lighthouse
lighthouse copied to clipboard
Remove alignment limitations from checkpoint sync
Description
Currently Lighthouse's checkpoint sync requires that the checkpoint state is not from a skipped slot, see: https://lighthouse-book.sigmaprime.io/checkpoint-sync.html#alignment-requirements
This limitation is somewhat artificial, and was taken in order to simplify the initial implementation. I've forgotten exactly why but I'll post more info on this issue when I dive back into it again.
Another desirable property: checkpoint sync from a state alone, without having to load the block.
I think the simplest way to solve this would be to advance the provided state to the nearest epoch boundary, although I might wait for the discussion here to be resolved until implementing that change.
Regarding the use of unaligned states, I went looking for some places where we look up states for block roots, which might fail if the only state stored is the advanced version of the state (e.g. block was at slot 30 and we've stored its state skipped through to 32).
- The main place where this is relevant is in the on-finalization database migration here:
https://github.com/sigp/lighthouse/blob/c547a11b0da48db6fdd03bca2c6ce2448bbcc3a9/beacon_node/beacon_chain/src/canonical_head.rs#L1019-L1041
That code takes care to use the advanced form of the finalized block's state, which is exactly what we want.
- This usage in the committee cache lookup could be problematic if the fork choice node stores the unadvanced state root:
https://github.com/sigp/lighthouse/blob/c547a11b0da48db6fdd03bca2c6ce2448bbcc3a9/beacon_node/beacon_chain/src/beacon_chain.rs#L5551-L5559
- In fork choice itself where we load the balances from the justified state could also be an issue. When we checkpoint sync we artificially set the justified and finalized blocks equal to each other, so this could result in a lookup of the finalized block's state:
https://github.com/sigp/lighthouse/blob/c547a11b0da48db6fdd03bca2c6ce2448bbcc3a9/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs#L324-L328
- We load the state for the parent block's state root in block verification:
https://github.com/sigp/lighthouse/blob/c547a11b0da48db6fdd03bca2c6ce2448bbcc3a9/beacon_node/beacon_chain/src/block_verification.rs#L1771-L1778
This would cause blocks that are descended from the finalized block (with skips) to error incorrectly. We should definitely fix this.
- We load the state for the head block's state root on start-up:
https://github.com/sigp/lighthouse/blob/c547a11b0da48db6fdd03bca2c6ce2448bbcc3a9/beacon_node/beacon_chain/src/canonical_head.rs#L298-L304
This could error if the finalized block is the head (e.g. restarting immediately after checkpoint sync).
There are probably other places too.
Some ideas for fixes:
A. Handle each case independently using bespoke logic. E.g. for fork choice we could make sure the state_root
stored in the nodes is the advanced state's state root (when initialising from a checkpoint). This might work, but might also break other things.
B. Store the unaligned state in the database, and keep track of this. @paulhauner suggested a separate column, but I think the BeaconState
column would be fine (using store_full_state
). We'd just need to keep track of the state root for this unaligned finalized state in the store, and have a special case for loading the unaligned state when get_state
requests a state root that matches.
C. Alternatively we could do a variant of B where we don't actually store the unaligned state, but we do store its state root and redirect requests for this state root to the aligned/advanced state. I think this is strictly worse, aside from the disk usage, as it breaks a pretty fundamental property of get_state
.
So far I'm in favour of B, with no new column, and a DB schema migration to add the unaligned_finalized_state_root
to the DB. I think it could reasonably live in the AnchorInfo
, as it is only relevant when the anchor is non-null (i.e. for non-archive nodes).
Adding the tree-states tag, because sorting out the alignment issue is required for https://github.com/sigp/lighthouse/issues/4481, which is required for a tree-states
database migration.