Use fork choice to determine finalized_checkpoint in gossip validation
#3543
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Update gossip validation rules to use the highest `finalized_checkpoint` across _all_ branches (`store.finalized_checkpoint`), instead of the one on the currently selected branch (`state.finalized_checkpoint`) when deciding whether to ignore a block / blob because they are already finalized.
etan-status
commented
Nov 6, 2023
Comment on lines
-694
to
+696
| - `finalized_root`: `state.finalized_checkpoint.root` for the state corresponding to the head block | ||
| - `finalized_root`: `store.finalized_checkpoint.root` according to [fork choice](./fork-choice.md). | ||
| (Note this defaults to `Root(b'\x00' * 32)` for the genesis finalized checkpoint). | ||
| - `finalized_epoch`: `state.finalized_checkpoint.epoch` for the state corresponding to the head block. | ||
| - `finalized_epoch`: `store.finalized_checkpoint.epoch` according to [fork choice](./fork-choice.md). |
There was a problem hiding this comment.
These are for the libp2p Status message
| i.e. validate that `signed_beacon_block.message.slot > compute_start_slot_at_epoch(state.finalized_checkpoint.epoch)` | ||
| i.e. validate that `signed_beacon_block.message.slot > compute_start_slot_at_epoch(store.finalized_checkpoint.epoch)` |
| - _[IGNORE]_ The sidecar is from a slot greater than the latest finalized slot -- i.e. validate that `block_header.slot > compute_start_slot_at_epoch(state.finalized_checkpoint.epoch)` | ||
| - _[IGNORE]_ The sidecar is from a slot greater than the latest finalized slot -- i.e. validate that `block_header.slot > compute_start_slot_at_epoch(store.finalized_checkpoint.epoch)` |
dapplion
approved these changes
Nov 9, 2023
ppopth
approved these changes
Nov 22, 2023
notably, it doesn't move backwards when you reorg |
hwwhww
approved these changes
Jan 15, 2024
djrtwo
approved these changes
Jan 16, 2024
| @@ -691,9 +691,9 @@ The fields are, as seen by the client at the time of sending the message: | |||
| - `current_fork_version` is the fork version at the node's current epoch defined by the wall-clock time | |||
| (not necessarily the epoch to which the node is sync) | |||
| - `genesis_validators_root` is the static `Root` found in `state.genesis_validators_root` | |||
| - `finalized_root`: `state.finalized_checkpoint.root` for the state corresponding to the head block | |||
| - `finalized_root`: `store.finalized_checkpoint.root` according to [fork choice](./fork-choice.md). | |||
There was a problem hiding this comment.
This is a no-op, right? The state here is implicitly your head which is going to share the store's finalized checkpoint by definition
There was a problem hiding this comment.
no, there may be a reorg that unrealizes finality in state that was already realized on a different branch - store.finalized_checkpoint tracks the highest finality across all branches, while state just tracks the one from the branch currently selected by fork choice (which may be lower).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update gossip validation rules to use the highest
finalized_checkpointacross all branches (store.finalized_checkpoint), instead of the one on the currently selected branch (state.finalized_checkpoint) when deciding whether to ignore a block / blob because they are already finalized.