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

Use lazy-loaded cached tree implementation #6

Merged
merged 5 commits into from
May 24, 2023
Merged

Use lazy-loaded cached tree implementation #6

merged 5 commits into from
May 24, 2023

Conversation

h5law
Copy link
Collaborator

@h5law h5law commented May 23, 2023

This is a port of this PR whilst retaining the squashed commit history of @roysc

This introduces a largely rewritten implementation of the SMT which operates on a cached, lazily-loaded tree structure, as opposed to the current paradigm of reading from/writing to the DB directly for each operation. This gives a significant performance cosmos/cosmos-sdk#11444 (comment) on all operations while preserving full compatibility of the produced hashes.

This also refactors the hasher objects to allow tree paths and stored values to be hashed independently, and Options to configure them. celestiaorg/smt#40 can be satisfied by passing an identity function as the path hasher.

As noted cosmos/cosmos-sdk#11444 (comment), the DeepSubtree code supporting state-transition fraud proofs was not compatible as-is with this implementation and so is removed; but there should be no technical block to adapting that code to this impl.; it was simply outside the scope of Cosmos-SDK ADR-40 for which this was originally built.

Although using the latest vulcanize/smt lazy_loading branch some changes had to be made:

  • The hashers.go file was missing from the repo and had to be copied in
  • All references to celestiaorg/smt were replaced with pokt-network/smt
  • Go version was updated to 1.18

This solves and merge conflicts and enables lazy loading to be used in the repo.

roysc and others added 5 commits April 11, 2023 13:34
This introduces a largely rewritten implementation of the SMT which operates on a cached,
lazily-loaded tree structure which must be explicitly committed to DB, replacing the existing pattern
of performing DB reads and writes on for each operation. This gives a significant performance
improvement on all operations while preserving backwards compatibility of commitment hashes.

This also:

* refactors the hasher objects to allow tree paths and stored values to be hashed independently,
and Options to configure them. By passing an identity function as the path hasher, the raw key can
be used directly as the leaf.

* removes value storage from the tree; the caller must maintain their own value mapping.

* removes the DeepSubtree code supporting state-transition fraud proofs, as it was not compatible
as-is with this implementation (but there should be no technical block to adapting it to this pattern).

* renames various types and decouples the path and value hashing functions.

* expands tests to cover corner cases, e.g. orphan node removal.
@h5law h5law added the enhancement New feature or request label May 23, 2023
@h5law h5law requested a review from Olshansk May 23, 2023 14:34
@h5law h5law self-assigned this May 23, 2023
Copy link
Member

@Olshansk Olshansk left a comment

Choose a reason for hiding this comment

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

The hashers.go file was missing from the repo and had to be copied in

Damn...


Going to approve this just so we can merge it in and start iterating on top of the lazy-loaded PR. A couple things to keep in mind (before we clean up the code and such):

  1. Can you try to integrate the lazy-loaded PR into the pocket repo and see if everything still works as expected?
  2. Note that the README in this repo will also need to be updated, but happy to leave that for a future PR as well.

With that said

giphy

@h5law h5law merged commit 6e691a6 into master May 24, 2023
@h5law h5law deleted the lazy_loading branch May 24, 2023 08:41
@roysc
Copy link

roysc commented May 28, 2023

Thanks for pulling from the new branch, and pointing out the missing file - sorry about that!

@h5law
Copy link
Collaborator Author

h5law commented May 28, 2023

Thanks for pulling from the new branch, and pointing out the missing file - sorry about that!

No worries thanks for all your hard work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants