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 stdlib smt module for Word key/values #373

Closed
wants to merge 13 commits into from

Conversation

maxgillett
Copy link
Contributor

@maxgillett maxgillett commented Aug 23, 2022

Partially addresses issue #258 by introducing the procedures smt::get, smt::update and smt::insert in a new smt standard library module. These procedures allow manipulation of compacted sparse Merkle trees with Word-sized keys. The sparse Merkle tree advice set is modified to permit a compacted representation, where leaf values can be stored at depths that are smaller than the maximum depth of the tree.

Integration tests are present for these three procedures, with some associated documentation describing the structure of the compact representation.

The following tasks remain to be implemented:

  • Handle insertion case when the branching point is an existing compacted leaf node, not an internal node (referred to as a type 1 insertion in the smt_insert_key integration test documentation)
  • Use empty hashes that assume a maximum tree depth of 256
  • Fix get_path test case error for non-compacted SMT advice set
  • Fully document the procedures (including soundness checks and required advice tape)
  • Refactor Miden assembly to improve readability and reduce cycle count by optimizing stack accesses

@maxgillett maxgillett marked this pull request as draft August 23, 2022 15:54
@maxgillett maxgillett changed the title [DRAFT] Implement stdlib smt module for Word key/values Implement stdlib smt module for Word key/values Aug 23, 2022
@bobbinth
Copy link
Contributor

Thank you! This looks great! A few comments about the general approach and structure (i.e., this is not an in-depth review).

  1. I think the overall approach works, and I don't see any soundness issues. But I'd like more people to review the general construction (not Miden assembly implementation) to see if maybe you and I missed anything.
  2. I would probably put compact SMT advice set into a separate module. It will result in some code duplication - but shouldn't be too bad since you can reuse Store in both w/o modifications. I think it might be cleaner that way. We could call them SimpleSmt and CompactSmpt or something similar.
  3. The comments you have in integration tests are awesome! But I'd probably move them either to .masm file or to the SMT module. I, personally, tried to understand how SMT works by reading .masm file and only then went to look at tests.
  4. For the .masm module, I'd probably have just two external interfaces: get and set. Internally, set could still split out into update vs. insert (and the split could be determined using advice input), but externally, I think it would be much more convenient to have only these two procedures exposed.
  5. We can probably simplify management of the advice tape by creating one or more specialized decorators. Such decorator could be executed once at the beginning of the procedure and it would inject all required values (in the right order) into the advice tape.
  6. I wonder if we can make insertions simpler if we allow leaf nodes to be only at specific depth. For example, we could say that a leaf nodes must be at depth which are multiples of 8 (e.g., 8, 16, 24 etc.). This way, depth of a given branch always increases in increments of 8. But I haven't really thought about this much - so, it is possible that this actually complicates things more.

@bobbinth
Copy link
Contributor

Superseded by #1046, #1048, #1036 and other related PRs.

@bobbinth bobbinth closed this Aug 29, 2023
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