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

Implement conversion from PartialMerkleTree to TieredSmt #209

Closed
wants to merge 1 commit into from

Conversation

plafer
Copy link
Contributor

@plafer plafer commented Oct 30, 2023

There are a few things I don't like about the current implementation. This PR serves to get the conversation going about the best way to implement the PMT -> TSMT conversion.

Problem statement

It is not possible to convert a PartialMerkleTree into a TieredSmt because the PMT stores only values (accessed by NodeIndex), whereas the TieredSmt associates a key to every value. Therefore, to convert PMT -> TSMT, we must provide the key associated with each value in the PMT.

Current solution

The current solution uses TryFrom<(PMT, BTreeMap)> for TieredSmt, where the BTreeMap provides the "value -> key" information.

I also used an iterator-based approach (with a helper from itertools), as it is the most efficient approach; for all other solutions I tried, I was forced to create one or more Vec<> along the way, or traverse pmt.leaves() twice. However, the downside is that the expression in the try_from() is a little difficult to parse.

Finally, it is not clear from the type BTreeMap<Digest, Digest> what it is supposed to do. The problem I faced was that Word (the actual type for values in the tree) doesn't implement Ord, and so cannot be used as the key to a BTreeMap. The workaround is to store values as Digest, and convert them back to Word at some point in the process. I think we need a better solution for that.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@bobbinth
Copy link
Contributor

This is for the block producer in miden-node, right? If so, I think there is an alternative approach which may not require this change. The core idea is to perform TSMT update inside Miden VM and in this case, we won't need to construct the TSMT object explicitly.

Performing TSMT update within the VM would require the following:

  1. We need to load the Merkle store in the advice provider of the VM with the partial Merkle tree containing the relevant Merkle paths. For this, we should be able to use extend() method of MerkleStore and pass inner nodes of PartialMerkleTree into it (we can get these via PartialMerkleTree::inner_nodes() method).
  2. We need to load the advice map in the advice provider of the VM with the set of key-value pairs for each of the relevant leaves of the TSMT. For this, we need to have a mapping of the form leaf |-> (key, value), where leaf is the node value of a leaf in the partial Merkle tree, and (key, value) is the key value pair stored at that leaf (e.g., (account_id, account_hash) for the account TMST.
  3. We need to execute a program which uses std::collections::smt::insert (for nullifiers) and std::collections::smt64::set (for accounts) to update their corresponding TSMTs.

@plafer
Copy link
Contributor Author

plafer commented Nov 3, 2023

Closing, as no longer necessary

@plafer plafer closed this Nov 3, 2023
@bobbinth bobbinth deleted the plafer-pmt-to-tsmt branch February 14, 2024 20:24
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.

2 participants