Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(dot/state): refactor NewTries #2782

Merged
merged 4 commits into from
Aug 29, 2022
Merged

chore(dot/state): refactor NewTries #2782

merged 4 commits into from
Aug 29, 2022

Conversation

qdm12
Copy link
Contributor

@qdm12 qdm12 commented Aug 24, 2022

Changes

  • NewTries does not return an error (always nil)
  • NewTries does not take an initial as argument (which won't get versioned)
  • Add (*Tries) SetEmptyTrie() method (which won't get versioned)
  • Add (*Tries) SetTrie(*trie.Trie) method (which will get versioned)

This PR is because the empty trie is the same for v0 and v1 state versions.
So we want a method SetEmptyTrie on Tries which doesn't require a state version, and another method SetTrie which does.

This allows to have way less code changes in the next chunky PR versioning trie calls.

Tests

go test -tags integration github.com/ChainSafe/gossamer

Issues

#2418

Primary Reviewer

@kishansagathiya

@qdm12 qdm12 force-pushed the qdm12/state/newtries branch 2 times, most recently from b7fe0eb to 8b8826d Compare August 24, 2022 21:37
@codecov
Copy link

codecov bot commented Aug 24, 2022

Codecov Report

Merging #2782 (973b149) into development (7d66ec0) will decrease coverage by 0.23%.
The diff coverage is 85.71%.

Additional details and impacted files
@@               Coverage Diff               @@
##           development    #2782      +/-   ##
===============================================
- Coverage        63.09%   62.86%   -0.24%     
===============================================
  Files              213      213              
  Lines            27061    27064       +3     
===============================================
- Hits             17074    17013      -61     
- Misses            8430     8495      +65     
+ Partials          1557     1556       -1     

@qdm12 qdm12 marked this pull request as ready for review August 25, 2022 13:33
@qdm12 qdm12 added the PR Easy label Aug 25, 2022
Copy link
Member

@EclesioMeloJunior EclesioMeloJunior left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should you rebase development into this branch?

dot/core/interface.go Outdated Show resolved Hide resolved
dot/core/mocks_test.go Outdated Show resolved Hide resolved
qdm12 added 3 commits August 25, 2022 15:13
- `SetEmptyTrie` to set an empty trie without taking a state version argument
- `SetTrie` to change `NewTries` constructor to NOT take a trie as input
@qdm12 qdm12 force-pushed the qdm12/state/newtries branch from f6adc97 to 2de5d47 Compare August 25, 2022 15:13
@qdm12 qdm12 force-pushed the qdm12/state/newtries branch from 2de5d47 to 973b149 Compare August 25, 2022 20:17
@qdm12 qdm12 merged commit 1f4fc20 into development Aug 29, 2022
@qdm12 qdm12 deleted the qdm12/state/newtries branch August 29, 2022 11:01
@github-actions
Copy link

🎉 This PR is included in version 0.7.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants