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

Fix TSMT serialization #235

Closed
wants to merge 47 commits into from
Closed

Fix TSMT serialization #235

wants to merge 47 commits into from

Conversation

plafer
Copy link
Contributor

@plafer plafer commented Dec 15, 2023

The current serde serialization of NodeIndex makes serializing data structures that contain a HashMap<NodeIndex, ...> fail with the error: "key must be a string" (more details here).

This implements a workaround: NodeIndex is serialized as "depth-value".

bobbinth and others added 30 commits October 19, 2023 12:16
mmr: support arbitrary from/to delta updates
…r-versions

mmr: support proofs with older forest versions
…-forest

mmr: support accumulator of older forest versions
When a prefix is pushed to the depth 64, the entry list includes only
the values different than ZERO. This is required, since each block
represents a 2^192 values.

The bug was in the proof membership code, that failed to handle the case
of a key that was not in the list, because the depth is 64 and the value
was not set.
…ic-data

simple_smt: reduce serialized size, use static hashes of the empty word
Implement `IntoIterator` for `RpoDigest`
feat: memoize Signature polynomial decoding
hackaugusto and others added 17 commits November 22, 2023 17:02
…ndex-bug

simplesmt: bugfix, index must be validated before modifying the tree
* with_contiguous_leaves

* test
…duplicate-check

simplesmt: simplify duplicate check
…#228)

* Change InvalidNumEntries error

* max computation

* remove length check

* remove ExactSizeIterator constraint

* fix InvalidNumEntries error condition

* 2_usize
…value-depth-64

bugfix: TSMT failed to verify empty word for depth 64.
* recompute_nodes_from_indeX_to_root

* MerkleError variant

* set_subtree

* test_simplesmt_set_subtree

* test_simplesmt_set_subtree_entire_tree

* test

* set_subtree: return root
serde: for MerklePath, ValuePath, and RootPath
@plafer plafer requested a review from bobbinth December 15, 2023 21:25
Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@bobbinth
Copy link
Contributor

In what context do we need this serialization? Originally, it was meant to be used only for mock data purposes.

@plafer
Copy link
Contributor Author

plafer commented Dec 18, 2023

I can't speak for why it was added in the first place, but I implemented this to be able to serialize an Account, which contains AccountVault, which wraps TieredSmt - and the TieredSmt serialization to JSON was broken.

However with 0xPolygonMiden/miden-base#369 we no longer need this change (for the purpose of Account at least).

@plafer
Copy link
Contributor Author

plafer commented Dec 18, 2023

Actually maybe it would be better to import the serialization strategy from that PR - see 0xPolygonMiden/miden-base#369 (comment)

@bobbinth
Copy link
Contributor

Closing this as we re removing TSMT.

@bobbinth bobbinth closed this Jan 19, 2024
@bobbinth bobbinth deleted the plafer-fix-tsmt-serialization branch February 14, 2024 20:07
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.

5 participants