[Persistence] TreeStore Refactor

[dylanlott]

Description

Refactors the stateTrees component behind the TreeStore interface.

Issue

Part of a refactor effort towards implementing #562

Part of a series of refactors in the persistence package, with #736 being the first.

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

• Pulls the stateTrees component down into its own component behind a dedicated interface for managing the SMT store.

Testing

• make develop_test; if any code changes were made
• 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)

[codecov]

Codecov Report

Patch coverage has no change and project coverage change: -0.67 ⚠️

Comparison is base (2074a1b) 31.52% compared to head (e64af30) 30.86%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #756      +/-   ##
==========================================
- Coverage   31.52%   30.86%   -0.67%     
==========================================
  Files         107      108       +1     
  Lines        9034     9228     +194     
==========================================
  Hits         2848     2848              
- Misses       5846     6040     +194     
  Partials      340      340              

Impacted Files	Coverage Δ
persistence/trees/trees.go	0.00% <0.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[github-actions]

The code patch provided appears to be part of a project's CHANGELOG. This file is meant to keep track of changes made to the project over time, rather than containing actual code for review. Nevertheless, I can give comments on this particular entry:

1. The date format and versioning seem consistent with the previous entries, based on the visible fragment.
2. There are two changes described:
   a) Refactoring stateTrees implementation: This could be positive, as it mentions moving the functionality into its own module, potentially improving code organization and maintainability. However, without examining the specific code changes, it's impossible to determine if this refactoring improved the code quality or introduced any bugs.
   b) Implementing Update and Commit pattern with SMT trees: Without more context on what the "Update and Commit" pattern is and how it relates to SMT trees, I cannot give specific feedback on whether this is an improvement or risk. It would be helpful to provide additional details or references regarding these concepts for better understanding.

In summary, while there doesn't seem to be anything inherently risky in these descriptions, it would be necessary to examine the actual code changes themselves to give relevant feedback on bug risks and improvements.

[github-actions]

The provided code patch shows a small update in the project's changelog, detailing changes in version 0.0.0.41. Here's a brief review:

1. Typo: There appears to be a typo in the description of the change - "consitency" should be spelled as "consistency". Please fix this to avoid confusion.

2. Clarity: The description of the change, "Rename package import for consistency and clarity," is slightly vague. It could be more informative by specifically mentioning which package imports were renamed. This would provide better understanding and context related to the changes.

Overall, the code patch seems minimal and straightforward, but it can be improved by fixing the spelling error and providing more specific details about the package imports being renamed.

[github-actions]

The provided code patch represents the addition of a new release/version (0.0.0.14) to a changelog for a project that adheres to Semantic Versioning. Here are my feedback and suggestions:

1. It seems you're following an incorrect version numbering format; semantic versioning typically follows the pattern MAJOR.MINOR.PATCH. You may consider updating the version number to 0.14.0.

2. The entry in the changelog indicates that the TreeStore interface has been defined, but there's no actual code snippet available for inspection. To provide a thorough review, please share the relevant code changes.

3. When providing information about updates in the changelog, ensure they describe the impact with sufficient detail to help users understand what's new or changed. For example, mentioning the purpose/benefits of the TreeStore interface and any key methods introduced would be helpful.

[Olshansk]

@dylanlott Do you plan on adding any unit tests to this?

[github-actions]

Overall, the code patch seems to be well-structured and clear. However, here's a brief review with some suggestions for improvement:

1. Comment consistency:
   Some comments are formatted as full sentences with capitalization and punctuation, while others are not. For consistency, it would be better to choose one style and apply it throughout the comments. For example, update,

   // BlockStore maps a block height to an *coreTypes.IndexedTransaction
   

   to

   // BlockStore maps a block height to an *coreTypes.IndexedTransaction.
   

2. Import organization:
   Although not mandatory, it's a best practice to organize imports into three groups: standard library imports, third-party imports, and local (same project) imports. In your case, consider organizing the imports like this:

   import (
      // Standard library imports if any
   
      "github.com/jackc/pgx/v5"
      // ... other third-party imports
      
      "github.com/pokt-network/pocket/persistence/blockstore"
      "github.com/pokt-network/pocket/persistence/indexer"
      "github.com/pokt-network/pocket/runtime/genesis"
      // ... other local imports
   )

3. Error handling:
   The addition of new methods to the PersistenceModule interface and the new TreeStore interface brings more opportunities for errors. Ensure that you have appropriate error handling in place when these methods are implemented in concrete implementations of these interfaces.

4. Interface segregation:
   Consider whether it's necessary to extend the PersistenceModule interface with the GetTreeStore() TreeStore method or if creating a separate interface that embeds both PersistenceModule and TreeStore would achieve better separation of concerns. This may help you keep the code modular, making it easier to maintain and test.

Aside from these suggestions, the code patch seems well-structured and presents low risk of introducing bugs.

[dylanlott]

@dylanlott Do you plan on adding any unit tests to this?

Yes, was planning on future PRs with more test coverage. This PR was meant to bring the refactor online and achieve test parity.

[Olshansk]

Let's get this over the finish line!