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

[Persistence] State Hash Computation Implementation #147

Closed
15 tasks
Olshansk opened this issue Aug 4, 2022 · 2 comments
Closed
15 tasks

[Persistence] State Hash Computation Implementation #147

Olshansk opened this issue Aug 4, 2022 · 2 comments
Assignees
Labels
core Core infrastructure - protocol related persistence Persistence specific changes

Comments

@Olshansk
Copy link
Member

Olshansk commented Aug 4, 2022

Objective

Compute a hash in order to take a snapshot of the V1 state via a single hash.

Origin Document

See the entire conversation around the state hash in https://github.com/pokt-network/pocket-network-protocol/tree/main/persistence.

Goals / Deliverables

  • Determine if we should use a Merkle Tree or Vector Committments
  • Pick a library for the merkle Tree or commitments
  • Compute and expose the state hash after every block commit
  • Incorporate the schemas (i.e. protobuf message definitions) in the state hash
  • Create a foundation to enable fast sync and slow sync by synching the different state trees
  • Do not limit the trees to only be keyed by addresses

General issue checklist

  • Update the appropriate CHANGELOG
  • Update the README
  • If applicable, update the source code tree explanation
  • If applicable, add or update a state, sequence or flowchart diagram using mermaid
  • Update any relevant global documentation & references
  • Document small issues / TODOs along the way

Non-goals

  • TODO

Testing Methodology

  • Develop an appropriate test suite
  • All tests: make test_all
  • LocalNet: verify a LocalNet is still functioning correctly by following the instructions at docs/development/README.md
    Remove

Creator: @Olshansk
Co-Owners: @andrewnguyen22

@Olshansk Olshansk added core Core infrastructure - protocol related persistence Persistence specific changes priority:high labels Aug 4, 2022
@Olshansk Olshansk self-assigned this Aug 4, 2022
@Olshansk
Copy link
Member Author

Olshansk commented Aug 4, 2022

The team's internal research is being aggregated here and will be shared publically once complete. If you would like access aprior, please reach out to the team and we'd be happy to share.

We are currently planning to use [https://github.com/celestiaorg/smt] originally implemented by @musalbas because:

  • It is a simple implementation of Libra's Jellyfish Merkle Tree
  • Takes advantage of the two major Sparse Merkle Tree optimizations: empty subtrees and single-node subtrees
  • Fully implemented in Go
  • Much simpler than Ethereum's modified Merkle Patricia Trie in both implementation and logic complexity

The work is currently ongoing in this branch: https://github.com/pokt-network/pocket/pull/new/issues/147/statehash

@Olshansk Olshansk changed the title [Persistence] Compute State Hash [Persistence] State Hash Computation Implementation Sep 4, 2022
andrewnguyen22 pushed a commit that referenced this issue Sep 12, 2022
andrewnguyen22 pushed a commit that referenced this issue Sep 14, 2022
## Description
Proper structure ownership delegated to the respective packages in order to maintain the modularity of the design.

closes #163

##### Shared
- Moved all shared structures out of the shared module
- Moved structure responsibility of config and genesis to the respective modules
- Shared interfaces and general 'base' configuration located here
- Moved make client code to 'debug' to clarify that the event distribution is for the temporary local net
- Left multiple `TODO` for remaining code in test_artifacts to think on removal of shared testing code

##### Consensus
- Ensured proto structures implement shared interfaces
- ConsensusConfig uses shared interfaces in order to accept MockConsensusConfig in test_artifacts
- ConsensusGenesisState uses shared interfaces in order to accept MockConsensusGenesisState in test_artifacts
- Implemented shared validator interface for `validator_map` functionality

##### P2P
- Ensured proto structures implement shared interfaces
- P2PConfig uses shared interfaces in order to accept MockP2PConfig in test_artifacts
- Moved connection_type to bool for simplicity (need to figure out how to do Enums without sharing the structure)

##### Persistence
- Renamed schema -> types
- Added genesis, config, and unstaking proto files from shared
- Ensured proto structures implement shared interfaces
- Populate genesis state uses shared interfaces in order to accept MockPersistenceGenesisState
- ^ Same applies for PersistenceConfig
- Bumped cleanup TODOs to #147 due to scope size of #163

##### Utility
- Ensured proto structures implement shared interfaces
- UtilityConfig uses shared interfaces in order to accept MockUtilityConfig in test_artifacts
- Moved all utilty tests from shared to tests package
- Left `TODO` for tests package still importing persistence for `NewTestPersistenceModule`
  - This is one of the last places where cross-module importing exists

## Type of change
Please mark the options that are relevant.
- [x] Code Health (non-breaking change which cleans up scoping)

## Author Note to reviewer
Though this appears to expand the code footprint, most of the expansion was inevitable setup
from the overbloated shared module. The intention here is to have one last large PR in order
to properly scope the structures / interfaces to better enable parllel work.

Much cleanup expected to follow this PR

## Before Merge
@Olshansk See TODO in makefile under proto_gen_local. I need assistance with this

## How Has This Been Tested?
- [x] Unit tests

___REPLACE_ME_: Describe the tests and that you ran to verify your changes. If applicable, provide steps to reproduce. Bonus points for images and videos or gifs._

- [x] `make test_all`
- [x] [LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md)

## Checklist
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have tested my changes using the available tooling

---

Co-authored-by: Daniel Olshansky <[email protected]>
Co-authored-by: Andrew Nguyen <[email protected]>
Co-authored-by: Andrew Nguyen <[email protected]>
Co-authored-by: Daniel Olshansky <[email protected]>
deblasis pushed a commit to deblasis/pocket that referenced this issue Sep 15, 2022
…etwork#178)

Proper structure ownership delegated to the respective packages in order to maintain the modularity of the design.

closes pokt-network#163

- Moved all shared structures out of the shared module
- Moved structure responsibility of config and genesis to the respective modules
- Shared interfaces and general 'base' configuration located here
- Moved make client code to 'debug' to clarify that the event distribution is for the temporary local net
- Left multiple `TODO` for remaining code in test_artifacts to think on removal of shared testing code

- Ensured proto structures implement shared interfaces
- ConsensusConfig uses shared interfaces in order to accept MockConsensusConfig in test_artifacts
- ConsensusGenesisState uses shared interfaces in order to accept MockConsensusGenesisState in test_artifacts
- Implemented shared validator interface for `validator_map` functionality

- Ensured proto structures implement shared interfaces
- P2PConfig uses shared interfaces in order to accept MockP2PConfig in test_artifacts
- Moved connection_type to bool for simplicity (need to figure out how to do Enums without sharing the structure)

- Renamed schema -> types
- Added genesis, config, and unstaking proto files from shared
- Ensured proto structures implement shared interfaces
- Populate genesis state uses shared interfaces in order to accept MockPersistenceGenesisState
- ^ Same applies for PersistenceConfig
- Bumped cleanup TODOs to pokt-network#147 due to scope size of pokt-network#163

- Ensured proto structures implement shared interfaces
- UtilityConfig uses shared interfaces in order to accept MockUtilityConfig in test_artifacts
- Moved all utilty tests from shared to tests package
- Left `TODO` for tests package still importing persistence for `NewTestPersistenceModule`
  - This is one of the last places where cross-module importing exists

Please mark the options that are relevant.
- [x] Code Health (non-breaking change which cleans up scoping)

Though this appears to expand the code footprint, most of the expansion was inevitable setup
from the overbloated shared module. The intention here is to have one last large PR in order
to properly scope the structures / interfaces to better enable parllel work.

Much cleanup expected to follow this PR

@Olshansk See TODO in makefile under proto_gen_local. I need assistance with this

- [x] Unit tests

___REPLACE_ME_: Describe the tests and that you ran to verify your changes. If applicable, provide steps to reproduce. Bonus points for images and videos or gifs._

- [x] `make test_all`
- [x] [LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md)

- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have tested my changes using the available tooling

---

Co-authored-by: Daniel Olshansky <[email protected]>
Co-authored-by: Andrew Nguyen <[email protected]>
Co-authored-by: Andrew Nguyen <[email protected]>
Co-authored-by: Daniel Olshansky <[email protected]>
@Olshansk
Copy link
Member Author

Olshansk commented Oct 5, 2022

The learnings/discussions from this PR will be implemented in #284.

@Olshansk Olshansk closed this as completed Oct 5, 2022
andrewnguyen22 pushed a commit that referenced this issue Oct 19, 2022
## Description

A general cleanup issue is needed to tackle TODO's and ensure the persistence module is usable/readable by consolidating `Actor` with `BaseActor` as part of #172. 

Follows issue-#128, issue-#105, issue-#147 and issue-#148 the persistence module is messier and more difficult to understand than the developers would want for organic external contribution.

## Issue

Fixes #210 

## Type of change

Please mark the relevant option(s):

- [ ] New feature, functionality or library
- [ ] Bug fix
- [x] Code health or cleanup
- [ ] Major breaking change
- [ ] Documentation
- [ ] Other <!-- add details here if it a different type of change -->

## List of changes

- [x] Consolidate `Actor` in `persistence/schema/base_actor.go` with `BaseActor` in `persistence/schema/base_actor.go`.


## Testing

- [x] `make develop_test`
- [x] [LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md) w/ all of the steps outlined in the `README`

## Required Checklist

- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have tested my changes using the available tooling
- [x] I have updated the corresponding CHANGELOG

### If Applicable Checklist
- [x] 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](https://mermaid-js.github.io) diagrams in the corresponding README(s)
- [ ] I have added, or updated, documentation and [mermaid.js](https://mermaid-js.github.io) diagrams in `shared/docs/*` if I updated `shared/*`README(s)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core infrastructure - protocol related persistence Persistence specific changes
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant