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

Tracking: State Update RFC Implementation #1049

Closed
12 tasks done
yaahc opened this issue Sep 10, 2020 · 7 comments
Closed
12 tasks done

Tracking: State Update RFC Implementation #1049

yaahc opened this issue Sep 10, 2020 · 7 comments
Assignees
Labels
A-rust Area: Updates to Rust code C-tracking-issue Category: This is a tracking issue for other tasks S-needs-design Status: Needs a design decision

Comments

@yaahc
Copy link
Contributor

yaahc commented Sep 10, 2020

This is the tracking issue for the state update rfc that outlines how state updates are handled, in particular multiple chain handling, reorgs, and finalization of blocks, and how finalized vs un-finalized blocks are handled.

Tracking TODOs

Design

Tasks that are out of scope for the first alpha release

Related Work

@yaahc yaahc added A-rust Area: Updates to Rust code S-blocked Status: Blocked on other tasks S-needs-design Status: Needs a design decision C-tracking-issue Category: This is a tracking issue for other tasks labels Sep 10, 2020
@yaahc yaahc added this to the Validate transactions. milestone Sep 10, 2020
@hdevalence
Copy link
Contributor

To check that Chain operations are implemented correctly, I think that we should use the following proptests:

  1. For any sequence of n+m blocks, check that the following give the same chain:

    1. using push to build a chain of all n+m blocks and then calling fork to remove the top m blocks;
    2. using push to build a chain of the first n blocks;
  2. For any sequence of n+m blocks, check that the following give the same chain:

    1. using push to build a chain of all n+m blocks and then calling finalize to remove the bottom n blocks;
    2. using push to build a chain of the last m blocks;

This probably requires making a new proptest Strategy that can generate sequences of blocks with correct parent hashes. This has some overlap with #919. I don't think it's required for this work, but eventually we could evolve that strategy into one that generates not just sequences of blocks with correct parent hashes but with valid transactions, etc.

@teor2345 teor2345 modified the milestones: Validate transactions, State Service Sep 29, 2020
@teor2345 teor2345 modified the milestones: State Service, First Alpha Release Oct 8, 2020
@yaahc yaahc self-assigned this Oct 22, 2020
yaahc added a commit that referenced this issue Oct 26, 2020
## Motivation

The zebra-state service needs to be able to handle duplicate blocks.

## Solution

This implements changes already outlined by [The State
RFC](https://zebra.zfnd.org/dev/rfcs/0005-state-updates.html). We check for
successfully committed blocks first, since interacting with the queued blocks
struct at this point just complicates the implimentation. If the block has not
already been committed we then check if the block has already been queued, if
not we handle the block normally (normally here being the bit we already had
implemented).

## Documentation Changes

- [x] Update the state RFC to match the ways this fix departs from the design
	- the main thing is that I switched the order of checking for duplicates
- [x] ~~Add newly added functions to the state rfc~~ Decided not to do this because they're minor getters that don't influence the rest of the design and aren't exposed as part of the API
- [x] Document newly added functions inline

## Testing

## Related Issues

- fixes #1182
- tracking issue #1049

Co-authored-by: teor <teor@riseup.net>
@teor2345 teor2345 self-assigned this Oct 26, 2020
@teor2345
Copy link
Contributor

@hdevalence I moved Request::Transaction(transaction::Hash) to a new ticket #1220, because it doesn't need to be done in the first alpha. We can schedule that work whenever we have time.

@teor2345
Copy link
Contributor

teor2345 commented Nov 3, 2020

Notes from our testing catch-up:

Goals

  • Ensure we implement consensus rules

Scope

  • Consensus Rules
    • Focus on failure cases - unit and property tests
    • Use mainnet and testnet syncs to test success cases
    • Use zebra_test block test vectors to test finalized state and service

Consensus Rule List

  • Previous hash matches
  • Height is previous height + 1
  • Reorgs can only go back 99 blocks (less important - never tested)
  • Best chain is always chosen
    • PartialCumulativeWork order
    • block::Hash tie-breaker
  • PartialCumulativeWork is maintained correctly???
  • Ord
    • if PartialCumulativeWork is different, Ord matches work order
    • if PartialCumulativeWork is equal, Ord is not equal, Ord is consistent
    • unit test - panic if the two chains are equal
  • if the block is a genesis block, skip any transaction updates

Easy Tests

  • Sled conversions
  • assert that sled returns None (no existing key) on insert
  • test all requests work when the state is empty
    • context: the syncer commits the genesis block before making any other requests, but we also handle requests from peers - which can come in before we download and commit the genesis block

Design Tests - less important

  • CommitBlock / CommitFinalizedBlock
    • adding a block
    • finalizing a block
    • fork the chain
    • pruning a chain

TODOs:

  • split tracking issue up into Finalized, Non-Finalized, and Service

@mpguerra
Copy link
Contributor

mpguerra commented Nov 6, 2020

I see we now have new tracking issues created.

Do we want to wait until those are completed before closing this issue or are we going to track outstanding work in the Finalized, Non-Finalized, and Service tracking issues?

@yaahc
Copy link
Contributor Author

yaahc commented Nov 6, 2020

I was going to close those first and then close this one, using it as a low detail higher level tracking issue.

@mpguerra
Copy link
Contributor

Seems like all items are ticked off now, can we close this one? :D

@yaahc
Copy link
Contributor Author

yaahc commented Nov 18, 2020

sounds good to me :D

@yaahc yaahc closed this as completed Nov 18, 2020
@mpguerra mpguerra removed the S-blocked Status: Blocked on other tasks label Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust Area: Updates to Rust code C-tracking-issue Category: This is a tracking issue for other tasks S-needs-design Status: Needs a design decision
Projects
None yet
Development

No branches or pull requests

4 participants