-
Notifications
You must be signed in to change notification settings - Fork 27
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
Optimize merkle #90
base: stylus
Are you sure you want to change the base?
Optimize merkle #90
Conversation
Codecov Report
@@ Coverage Diff @@
## stylus #90 +/- ##
==========================================
+ Coverage 54.78% 55.11% +0.32%
==========================================
Files 247 247
Lines 37558 37604 +46
==========================================
+ Hits 20576 20725 +149
+ Misses 14614 14506 -108
- Partials 2368 2373 +5 |
empty_hash: Bytes32, | ||
min_depth: usize, | ||
) -> Merkle { | ||
pub fn get_empty(ty: MerkleType, layer: usize) -> Bytes32 { |
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.
note: moving from each markle storing it's own empty_layers vec to all sharing the same one didn't save any time on generating proofs. I still like it - but might be worth considering if added complexity is worth it.
MerkleType::Empty => panic!("attempted to fetch empty-layer value from empty merkle"), | ||
MerkleType::Memory => Memory::MEMORY_LAYERS, | ||
MerkleType::Opcode => 2, | ||
MerkleType::ArgumentData => 2, |
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.
Note: not sure if it's necessary. I gave opcode and argument data min_depth 2 to avoid short functions producing a "root" that's not hashed at all.
split func to a mostly-empty additional-data merkle and condensed opcode merkle.
Optimize mostly-empty merkles.
Note: the last commit, caching and reusing empty hashes across merkles, doesn't seem to save time.
contract upgrade required: OffchainLabs/stylus-contracts#6