ProgressiveMerkleHasher and progressive container support #43
Conversation
Co-authored-by: michaelsproul <[email protected]>
Co-authored-by: michaelsproul <[email protected]>
Co-authored-by: michaelsproul <[email protected]>
Co-authored-by: michaelsproul <[email protected]>
… stream in Co-authored-by: michaelsproul <[email protected]>
…er method Co-authored-by: michaelsproul <[email protected]>
…inary tree hashing Co-authored-by: michaelsproul <[email protected]>
macladson
left a comment
macladson
left a comment
There was a problem hiding this comment.
I'm going to work on flipping the tree structure to the new spec
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #43 +/- ##
==========================================
+ Coverage 87.75% 88.88% +1.13%
==========================================
Files 6 7 +1
Lines 294 432 +138
==========================================
+ Hits 258 384 +126
- Misses 36 48 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| /// Number of chunks written to the current hasher. | ||
| current_level_chunks: usize, | ||
| /// Buffer for bytes that haven't been completed into a chunk yet. | ||
| buffer: Vec<u8>, |
There was a problem hiding this comment.
I think this being a Vec here is probably pretty inefficient so when we are looking at doing optimizations later we should consider changing how we handle the buffer
| // Check if current level is complete | ||
| if self.current_level_chunks == self.current_level_size { | ||
| // Move to next level (4x larger) | ||
| let next_level_size = self.current_level_size * 4; |
There was a problem hiding this comment.
This technically could overflow, we could use checked_mul instead and add a new error variant for the overflow, but practically the number of levels would you need to trigger this is enormous
| // Process any remaining bytes in the buffer as a final chunk | ||
| if !self.buffer.is_empty() { | ||
| let mut chunk = [0u8; BYTES_PER_CHUNK]; | ||
| chunk[..self.buffer.len()].copy_from_slice(&self.buffer); |
There was a problem hiding this comment.
This isn't really an issue but just noting that this will panic in cases where self.buffer.len() > BYTES_PER_CHUNK. This is guaranteed to be impossible since write will always return leaving buffer < BYTES_PER_CHUNK.
This PR contains all of the
tree_hashchanges required to support EIP-7916 and EIP-8016:ProgressiveMerkleHasher: an analog ofMerkleHasher, it has a lazy API where it can ingest bytes and hash them incrementally. It uses aMerkleHasherunder the hood to do the hashing of the binary subtrees.TreeHashimplementation forBitfield<Progressive>fromethereum_ssz.struct_behaviour = "progressive_container". This generates code that uses aProgressiveMerkleHasherfor the fields. It comes with a new attributeactive_fields({0,1},+)which is used to insert zero hashes for inactivated fields. The derive macro also generates a 32-byte array representing the active fields as a bitfield (at compile time), and generates code to mix this bitfield into the root, following the spec.enum_behaviour = "compatible_union". Compatible unions behave quite similarly to the existing (and now deprecated) union type, with the difference being that their g-indices and merkle tree structure is stable across all enum variants. For now the macro does not verify that enum variants are compatible, because this verification is not mandatory (it is safe if the only compatible unions happen to be compatible, which the spec should ensure). Verification is left for a follow-up PR: Compatible union validation ethereum_ssz#72.tree_hash(selector = "1")which can set the selector for compatible unions only. To avoid duplication withethereum_sszthis selector is read from thesszattribute, or thetree_hashattribute (they must be consistent if both are set). See discussion: Progressive data structures and tests EIP-7916, EIP-8016 lighthouse#8505 (comment).Depends on: