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

store: support adding existing structures #139

Merged
merged 6 commits into from
Apr 21, 2023

Conversation

hackaugusto
Copy link
Contributor

Describe your changes

fixes #136

implemented the extend trait for the merkle store, consume an iterator over inner nodes. this can be used to add existing structures into the store.

Checklist before requesting a review

  • Repo forked and branch created from next according to naming convention.
  • Commit messages and codestyle follow conventions.
  • Relevant issues are linked in the PR description.
  • Tests added for new functionality.
  • Documentation/comments updated according to changes.

Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I left one comment inline. One other thing: I was thinking that we'd remove most with_* and add_* methods after this. Is there a reason to keep them around?

@hackaugusto
Copy link
Contributor Author

hackaugusto commented Apr 20, 2023

One other thing: I was thinking that we'd remove most with_* and add_* methods after this. Is there a reason to keep them around?

MerklePath and MerklePathSet need it (these don't have the inner node iterator), should I add them?

edit: btw, the merkle path doesn't iterate over the inner nodes (it doesn't have them), it actually needs to compute instead. I have a PR for that already #122 , once we get that it should be easy to transform it to a InnerNodeInfo

edit2: forgot to mention, we can probably remove the methods for the MMR / SMT, let me know if I should remove these.

@hackaugusto hackaugusto force-pushed the hacka-support-adding-existing-structures-to-store branch from 57a9583 to 8161477 Compare April 20, 2023 11:45
@bobbinth
Copy link
Contributor

One other thing: I was thinking that we'd remove most with_* and add_* methods after this. Is there a reason to keep them around?

MerklePath and MerklePathSet need it (these don't have the inner node iterator), should I add them?

Let's remove all methods that we can and we'll remove the remaining ones later.

@hackaugusto hackaugusto force-pushed the hacka-support-adding-existing-structures-to-store branch from 9761115 to 59595a2 Compare April 21, 2023 12:48
@bobbinth bobbinth merged commit ae4e27b into next Apr 21, 2023
@bobbinth bobbinth deleted the hacka-support-adding-existing-structures-to-store branch April 21, 2023 21:32
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