v0.2 tracking PR #80
Merged
v0.2 tracking PR #80
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
docs: mention tree form order of NodeIndex docs
Add pre commit
…e-memory-implementation feat: merkle mountain range
Prior to this commit, there was an internal procedure with the merkle trees to compute empty sub-tree for arbitrary depths. However, this isn't ideal as this code can be reused in any merkle implementation that uses RPO as backend. This commit introduces a structure that will generate these empty subtrees values.
…pare feat: refactor simple smt to use empty subtree constants
Add merkle store
This commit introduce common derive proc macros to MerkleStore. These are required downstream as the in-memory storage can be cloned. It also introduces constructors common to the other types of the crate that will help to build a merkle store, using a build pattern.
refactor: add derive proc macros to merkle store
bugfix: check if the requested root is in the storage
ignore pre commit rev
Prior to this commit, the MerkleStore panicked under certain bounds. It will prevent such panics by using checked operations. ilog2, for instance, will panic when the operand is zero. However, there is a documentation rule enforcing the merkle tree to be size at least 2. If this rule is checked, then the panic is impossible.
The store builds the path from root to leaf, this updates the code to return a path from leaf to root, as it is done by the other structures. This also added custom error for missing root.
…bounds fix: merkle store panics on bounds
Storage benchmark
Returning tuples is often confusing as they don't convey meaning and it should be used only when there is no possible ambiguity. For `MerkleStore`, we had a couple of tuples being returned, and reading the implementation was required in order to distinguish if they were leaf values or computed roots. This commit introduces two containers that will self-document these returns: `RootPath` and `ValuePath`. It will also update `set_node` to return both the new root & the new path, so we can prevent duplicated traversals downstream when updating a node (one to update, the second to fetch the new path/root).
…ainers feat: add merkle path containers and return them on tree update
Fix benchmark code
And unit tests for each constructor type.
…message chore: clarified assert message
Example formatted MerkleTree:
```
880abe452320966617646e7740b014954300f19a28780a0889d62ff33f4b0534
1ade1369091efa31201e9b60c9c28874d0ddce5362b335135a6bb4c917285983
3e60a9c843b4bb19f7a0572102e6507195f5240767a396335fd21981b048b807
0100000000000000000000000000000000000000000000000000000000000000
0200000000000000000000000000000000000000000000000000000000000000
0300000000000000000000000000000000000000000000000000000000000000
0400000000000000000000000000000000000000000000000000000000000000
```
Example formatted MerklePath:
```
[0400000000000000000000000000000000000000000000000000000000000000, 1ade1369091efa31201e9b60c9c28874d0ddce5362b335135a6bb4c917285983]
```
Format merkle tree
store: added with_merkle_paths constructor
Prior to this commit, we limited the constants count to 64 for the empty subtrees depth computation. This is a hard-assumption that every tree of Miden will have a depth up to 64 - and will cause undefined behavior if it doesn't. With the introduction of `MerkleStore::merge_roots` and the deprecation of `mtree_cwm` instruction from the VM, this assumption is broken and the user might end with trees with depth greater than 64. This broken assumption could lead to attack vectors. We can easily fix that by extending the pre-computed hashes list to the maximum of `u8` (i.e. 255). This will have zero impact on functionality, and will be completely safe to use without hard assumptions.
Fix MerklePathSet issues
…ubtrees feat: add empty subtree constants to cover u8::MAX depth
…store Add serde to merklestore
The path returned by `EmptySubtreeRoots` starts at the root, and goes to the leaf. The MerkleStore constructor assumed the other direction, so the parent/child hashes were reversed. This fixes the bug and adds a test.
…pty-roots bugfix: fix internal nodes of for empty leafs of a SMT
Prepare v0.2 release
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR tracks progress towards v0.2 release.
Must have
Nice to have