-
Notifications
You must be signed in to change notification settings - Fork 121
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(trie): Add support for trie V1 #3433
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## development #3433 +/- ##
===============================================
- Coverage 50.17% 50.07% -0.11%
===============================================
Files 229 229
Lines 28518 28604 +86
===============================================
+ Hits 14310 14323 +13
- Misses 12703 12753 +50
- Partials 1505 1528 +23 |
b816195
to
634f315
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just small observations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job!
🎉 This PR is included in version 0.8.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Changes
Basically the thing that I changed is the way we encode the trie, when we are using v1 we can have hashed values for both branches and leaves.
What I did is to keep our trie as it is storing the value in the way we were doing it but I only change the encode / decode method to, for example, hash the values (greater than 32bytes) to be hashed in the encoded representation.
Since we are using the encoded node to calculate the hash I had to modify those methods too.
I also I added the
IsHashedValue
flag inNode
structure for those cases where we are decoding nodes and they are encoded with the v1 format (usually used when verifying merkle proofs)Summary of changes
Tests
make test
Issues
close #2418
Primary Reviewer
@timwu20