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

[Utility] Use TreeStore as source of truth #937

Merged
merged 2 commits into from
Jul 27, 2023
Merged

Conversation

h5law
Copy link
Contributor

@h5law h5law commented Jul 25, 2023

Description

Summary generated by Reviewpad on 26 Jul 23 11:24 UTC

This pull request includes the following changes:

  1. In the trees.go file:

    • Added a new method Prove that generates and verifies a proof against a tree name stored in the TreeStore.
    • Updated the GetTreeHashes method to exclude the root tree from the map of tree names to their root hashes.
  2. In the treestore_module.go file:

    • Added a new method Prove that generates and verifies a proof against a tree with a matching name.
    • No other changes have been made to the existing methods in the file.
  3. In the transaction_test.go file:

    • Modified the import statements to use different package names.
    • Modified the test cases TestHandleTransaction_ErrorAlreadyInMempool, TestHandleTransaction_ErrorAlreadyCommitted, and TestHandleTransaction_BasicValidation.
    • Modified the function prepareEmptyIndexedTransaction.
    • Changed some variable names.
  4. In the ibc_msg_mempool_test.go file:

    • Added a new test function TestHandleMessage_ErrorAlreadyCommitted.
    • Added environment preparation code, code for indexing a test transaction, and handling the error.
    • Modified the error handling code.
  5. In the utility/transaction.go file:

    • Added an import statement for the github.com/pokt-network/pocket/shared/crypto package.
    • Updated the assignment of the txHash variable.
    • Updated the condition for checking if the transaction is already in the mempool.
    • Updated the call to u.GetBus().GetPersistenceModule().TransactionExists.
    • Updated the check for the existence of the transaction.
  6. In the persistence/trees/trees_test.go file:

    • Updated the package imports.
    • Added a new test function TestTreeStore_Prove and several test cases.
    • Updated the treeStore struct and the existing test function.
  7. In the persistence_module.go file:

    • Added a new method TransactionExists and commented out the old method.

Please let me know if you would like a more detailed review of any specific part of the diff.

Issue

Fixes #875

Type of change

Please mark the relevant option(s):

  • New feature, functionality or library
  • Bug fix
  • Code health or cleanup
  • Major breaking change
  • Documentation
  • Other

List of changes

  • Update TransactionExists to use TreeStore
  • Add Prove method to TreeStore
  • Update tests

Testing

  • make develop_test; if any code changes were made
  • make test_e2e on k8s LocalNet; if any code changes were made
  • e2e-devnet-test passes tests on DevNet; if any code was changed
  • Docker Compose LocalNet; if any major functionality was changed or introduced
  • k8s LocalNet; if any infrastructure or configuration changes were made

Required Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added, or updated, godoc format comments on touched members (see: tip.golang.org/doc/comment)
  • I have tested my changes using the available tooling
  • I have updated the corresponding CHANGELOG

If Applicable Checklist

  • I have updated the corresponding README(s); local and/or global
  • I have added tests that prove my fix is effective or that my feature works
  • I have added, or updated, mermaid.js diagrams in the corresponding README(s)
  • I have added, or updated, documentation and mermaid.js diagrams in shared/docs/* if I updated shared/*README(s)

@h5law h5law added utility Utility specific changes persistence Persistence specific changes e2e-devnet-test Runs E2E tests on devnet labels Jul 25, 2023
@h5law h5law added this to the M1: Pocket PoS (Proof of Stake) milestone Jul 25, 2023
@h5law h5law requested review from Olshansk and dylanlott July 25, 2023 21:55
@h5law h5law self-assigned this Jul 25, 2023
@reviewpad reviewpad bot added medium Pull request is medium waiting-for-review labels Jul 25, 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.

Just a few nits

utility/transaction_test.go Outdated Show resolved Hide resolved
shared/modules/treestore_module.go Outdated Show resolved Hide resolved
persistence/trees/trees.go Outdated Show resolved Hide resolved
persistence/block.go Outdated Show resolved Hide resolved
persistence/block.go Outdated Show resolved Hide resolved
ibc/ibc_msg_mempool_test.go Show resolved Hide resolved
utility/transaction_test.go Show resolved Hide resolved
persistence/trees/trees_test.go Outdated Show resolved Hide resolved
@reviewpad reviewpad bot added large Pull request is large and removed medium Pull request is medium labels Jul 26, 2023
@h5law h5law requested a review from Olshansk July 26, 2023 15:33
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.

flowers-tree

@h5law h5law merged commit bfc57eb into main Jul 27, 2023
12 checks passed
@h5law h5law deleted the utility/prove_stores branch July 27, 2023 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e2e-devnet-test Runs E2E tests on devnet large Pull request is large persistence Persistence specific changes utility Utility specific changes waiting-for-review
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Utility] Address the discrepancy between the state trees and txIndexer
3 participants