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

[Core] Update module interface required for end-to-end StateHash implementation #251

Closed
8 tasks
Olshansk opened this issue Sep 27, 2022 · 0 comments
Closed
8 tasks
Assignees
Labels
consensus Consensus specific changes core Core infrastructure - protocol related persistence Persistence specific changes utility Utility specific changes

Comments

@Olshansk
Copy link
Member

Olshansk commented Sep 27, 2022

Objective

Document the cross-module flow and update the interfaces required to compute a stateHash.

Origin Document

The original ticket to implement the state hash (#147) was first tended to in (#152), but the scope grew and some exploratory work was done in #237.

Since the stateHash is a core component of any blockchain, it needs to be well understood and designed to make sure that it can be QAed against all possible scenarios.

Goals

  • Design & define the end-to-end flow to compute a state hash driving the task

Deliverable

  • A PR that updates the persistence, consensus and utility interfaces where needed
  • A mermaid sequence diagram showing which components & interfaces are necessary & used in this flow
  • Changes in the implementation that support the updates interfaces but without introducing new business logic

Non-goals / Non-deliverables

  • Implement the code necessary to compute a stateHash
  • Design a testing suite / approach for the state hash

General issue deliverables

  • Update the appropriate CHANGELOG
  • Update any relevant READMEs (local and/or global)
  • Update any relevant global documentation & references
  • If applicable, update the source code tree explanation
  • If applicable, add or update a state, sequence or flowchart diagram using mermaid

Testing Methodology

  • Task specific tests: make ...
  • All tests: make test_all
  • LocalNet: verify a LocalNet is still functioning correctly by following the instructions at docs/development/README.md

Creator: @Olshansk
Co-Owners: @andrewnguyen22

@Olshansk Olshansk added core Core infrastructure - protocol related consensus Consensus specific changes utility Utility specific changes persistence Persistence specific changes labels Sep 27, 2022
@Olshansk Olshansk self-assigned this Sep 27, 2022
Olshansk added a commit that referenced this issue Nov 28, 2022
## Description

Interface updates and diagrams to document the cross-module flow for stateHash implementation.

## Issue

Fixes part of #251

## Type of change

Please mark the relevant option(s):

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

## List of changes

The goal of this change is to show the interface changes and plan of action to implement the state hash implementation.

Documentation and external interface changes required for updating the state hash. Details related to the internals of the persistence module will be done in a follow up commit.

The major change is that the final `quorumCertificate` is only known at the time of committal, not proposal, so the corresponding changes were made as well as interface changes to make the terminology easier to follow with the corresponding documentation.

**General**
- Update some comments / TODOs throughout the code
- Handle errors where appropriate to support interface changes

**UtilityContext interface changes**
- Change `CommitPersistenceContext()` to `Commit(quorumCert)
- Change `ReleaseContext()` to `Release() error`

**PostgresContext interface changes**
- Removed `quorumCert` from the `SetProposalBlock` method signature
- Changed `Commit()` to `Commit(quorumCert)`
- Renamed `ResetContext` to `Release`
- Replace `AppHash` with `UpdateAppHash` 
- Added `ReleaseWriteContext` to the module level interface

**Persistence Module/Interface changes**
- Reduce the cope of the `StoreBlock` function
- Make `storeBlock` accept a `quorumCert`
- Remove `quorumCertificate` from local state and corresponding setters/getters
- Reduce the cope of the `IndexTransactions` function and remove from interface

**Consensus module changes**
- Add error handling where appropriate corresponding to the interface changes

## 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`

## 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
- [ ] If applicable, I have made corresponding changes to related local or global README
- [ ] If applicable, I have updated the corresponding CHANGELOG
- [ ] If applicable, I have added tests that prove my fix is effective or that my feature works
- [ ] If applicable, I have added new diagrams using [mermaid.js](https://mermaid-js.github.io)

---

Co-authored-by: Alessandro De Blasis <[email protected]>
Co-authored-by: Irving A.J. Rivas Z. <[email protected]>
Co-authored-by: Andrew Nguyen <[email protected]>
Co-authored-by: Daniel Olshansky <[email protected]>
Co-authored-by: Jason You <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus Consensus specific changes core Core infrastructure - protocol related persistence Persistence specific changes utility Utility specific changes
Projects
Status: Done
Development

No branches or pull requests

1 participant