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

[Consensus] State Sync - Request, download and apply blocks #352

Open
8 tasks
Olshansk opened this issue Nov 14, 2022 · 1 comment
Open
8 tasks

[Consensus] State Sync - Request, download and apply blocks #352

Olshansk opened this issue Nov 14, 2022 · 1 comment
Assignees
Labels
consensus Consensus specific changes core Core infrastructure - protocol related

Comments

@Olshansk
Copy link
Member

Olshansk commented Nov 14, 2022

Objective

Implement the interface that exposes the maximum & minimum blocks a node can expose and share

Origin Document

State sync block by block design: #125

Goals

  • A foundation for advertising/exposing data a single node has to offer to others during state sync
  • Allow another node to request, download and apply a block from the advertising node

Deliverables

  • Implementation of the interface (fully integrated) that exposes the data a node has available
  • Tooling and/or tests that call these endpoints to retrieve the advertised information

Non-goals

  • Actual implementation of state sync

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

Creators: @jessicadaugherty @Olshansk

@Olshansk Olshansk moved this from Rescope to Up Next in V1 Dashboard Nov 23, 2022
@Olshansk Olshansk assigned gokutheengineer and unassigned Olshansk Dec 27, 2022
@jessicadaugherty jessicadaugherty changed the title [Consensus] State Sync - Implement & expose interface to advertise and provide height + blocks [Consensus] State Sync - Request, download and apply blocks Jan 30, 2023
@jessicadaugherty jessicadaugherty moved this from Up Next to In Progress in V1 Dashboard Feb 20, 2023
@Olshansk Olshansk added core Core infrastructure - protocol related consensus Consensus specific changes labels Apr 4, 2023
@Olshansk
Copy link
Member Author

Olshansk commented Apr 4, 2023

After a first attempt at #594, we've got some new learnings and are going to start from scratch, splitting the work into smaller PRs

Notes:

  • Try to avoid maintaining state in the consensus module as much as possible
  • Try to avoid small boolean vars (e.g. IsSynching, etc...) in the consensus module
  • Utilize persistence for the state of the node
  • Use the FSM and main event bus for event handling
  • Create channels to accept state sync metadata responses and block responses
  • Create very small PRs, one by one

As a starting point, we will create the following micro PRs:

PR1: [Testing] Channel for FSM events

Don't mock FSM in testing. Use the existing testing infrastructure and pipe the events into the shared channel. This enables waiting & controlling the clock.

PR2: [Testing] Helpers

Avoid boiler plate code needed in hotstuff within state sync E.g. Create WaitForBlock, WaitForTransaction, etc, type helpers.

PR3: [Consensus] Adding transactions via transaction indexer

  • Use the txIndexer to populate the transactions in the blocks

PR4: [Consensus] Create Block and Metadata Response Channels

  • One channel to accept new metadata responses.
  • One channel to accept new block responses.
  • FSM events will trigger operations for the channels (e.g. aggregating the metadata responses, processing received blocks, etc...)
  • Asynchronously, goroutines will be handling, processing of these channels; leave a placeholder for this in PR3.
    • One goroutine will be processing the metadata response channel.
    • Another goroutine will be processing received blocks, committing.

PR5: [Consensus] Investigate locks

  • Implement the placeholders from PR4 to actually apply the block
  • This will require some work / trickery related to locks

This is a starting point and further PRs will be needed.

gokutheengineer added a commit that referenced this issue Apr 12, 2023
## Description

Populate the transactions in the persisted blocks using `txIndexer`.

## Issue

Fixes the following sub-issues of #352:

- PR3: [Consensus] Adding transactions via transaction indexer

## Type of change

Please mark the relevant option(s):

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

## List of changes

- Update `prepareBlock()` function to include transactions with
`txIndexer`.


## Testing

- [X] `make develop_test`; if any code changes were made
- [X] [Docker Compose
LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md);
if any major functionality was changed or introduced
- [ ] [k8s
LocalNet](https://github.com/pokt-network/pocket/blob/main/build/localnet/README.md);
if any infrastructure or configuration changes were made


## Required Checklist

- [ ] 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 added, or updated, [`godoc` format
comments](https://go.dev/blog/godoc) on touched members (see:
[tip.golang.org/doc/comment](https://tip.golang.org/doc/comment))
- [X] I have tested my changes using the available tooling
- [X] 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](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)
gokutheengineer added a commit that referenced this issue Apr 12, 2023
## Description
Includes changes that primarily focus on code refactoring,
simplification, and improvements to the testing in the consensus module.
Key modifications entail renaming functions, altering error messages and
logging, streamlining and updating imports, and introducing new helper
functions. It also sets the new structure for the state sync module,
although doesn't include full implementation, which is to come in the
next PRs.



## Issue
Fixes the following sub-issues of #352:

- (Partially) PR1: [Testing] Channel for FSM events: removed FSM
mocking. `waitForFSMEvents()` function to be added.
- PR2: [Testing] Helpers: helpers for hotstuff messages, and state sync
events.
- (Partially) PR4: [Consensus] Create Block and Metadata Response
Channels:

## Type of change

Please mark the relevant option(s):

- [X] 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
- Refactored state sync module,
- Implemented two new channels for received metadata and blocks,
- Refactored consensus module's testing framework by removing state
machine mock in testing,
- Introduced, as placeholders, new helper functions for testing state
sync stages.



## Testing

- [X] `make develop_test`; if any code changes were made
- [X] [Docker Compose
LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md);
if any major functionality was changed or introduced
- [ ] [k8s
LocalNet](https://github.com/pokt-network/pocket/blob/main/build/localnet/README.md);
if any infrastructure or configuration changes were made

<!-- REMOVE this comment block after following the instructions
 If you added additional tests or infrastructure, describe it here.
 Bonus points for images and videos or gifs.
-->

## 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 added, or updated, [`godoc` format
comments](https://go.dev/blog/godoc) on touched members (see:
[tip.golang.org/doc/comment](https://tip.golang.org/doc/comment))
- [X] I have tested my changes using the available tooling
- [X] 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](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)

---------

Co-authored-by: Daniel Olshansky <[email protected]>
@gokutheengineer gokutheengineer mentioned this issue May 4, 2023
18 tasks
@Olshansk Olshansk assigned Olshansk and unassigned gokutheengineer May 17, 2023
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
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

4 participants