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

Support late block re-orging #6595

Open
ajsutton opened this issue Dec 13, 2022 · 2 comments
Open

Support late block re-orging #6595

ajsutton opened this issue Dec 13, 2022 · 2 comments

Comments

@ajsutton
Copy link
Contributor

Description

To discourage the late publication of blocks, implement support for late block re-orging: ethereum/consensus-specs#3034

@benjaminion
Copy link
Contributor

benjaminion commented Nov 7, 2023

This has now been merged, so we can get on with implementing it.

@rolfyone rolfyone self-assigned this Nov 10, 2023
@tbenr
Copy link
Contributor

tbenr commented Nov 13, 2023

Don't forget to re-enable related reference tests (disabled here: #7695)

rolfyone added a commit to rolfyone/teku that referenced this issue Nov 16, 2023
 - added  `REORG_HEAD_WEIGHT_THRESHOLD`, `REORG_PARENT_WEIGHT_THRESHOLD`, and  `REORG_MAX_EPOCHS_SINCE_FINALIZATION` to phase0 configuration, and defaulted

 - added utility functions:
   - `isShufflingStable`
   - `isFinalizationOk`
   - `calculateCommitteeFraction`

which were part of the changes in ethereum/consensus-specs#3034

partially addresses Consensys#6595

Signed-off-by: Paul Harris <[email protected]>
rolfyone added a commit that referenced this issue Nov 17, 2023
* Added config items for late block reorg

partially addresses #6595

Signed-off-by: Paul Harris <[email protected]>
rolfyone added a commit to rolfyone/teku that referenced this issue Nov 21, 2023
 - Added validator_is_connected
 - added is_timely

 Currently Timeliness is going to be enabled, adding to a small cache. This is minimal overhead.

 partially addresses Consensys#6595

Signed-off-by: Paul Harris <[email protected]>
courtneyeh pushed a commit to courtneyeh/teku that referenced this issue Nov 22, 2023
* Added more utilities for late block reorg

 - Added validator_is_connected
 - added is_timely

 Currently Timeliness is going to be enabled, adding to a small cache. This is minimal overhead.

 partially addresses Consensys#6595

Signed-off-by: Paul Harris <[email protected]>
rolfyone added a commit to rolfyone/teku that referenced this issue Nov 23, 2023
 - is_parent_strong
 - is_head_weak
 - is_proposing_on_time

The head_weak and parent_strong functions I put into store ultimately, as it has all the context needed. It did mean a little restructuring to enable testing, but I think it currently is a sensible home for it.

partially addresses Consensys#6595

Signed-off-by: Paul Harris <[email protected]>
rolfyone added a commit to rolfyone/teku that referenced this issue Nov 23, 2023
partially addresses Consensys#6595

Signed-off-by: Paul Harris <[email protected]>
rolfyone added a commit to rolfyone/teku that referenced this issue Nov 23, 2023
* Added more utilities for late block reorg

 - is_parent_strong
 - is_head_weak
 - is_proposing_on_time
 - is_ffg_competitive

The head_weak and parent_strong functions I put into store ultimately, as it has all the context needed. It did mean a little restructuring to enable testing, but I think it currently is a sensible home for it.

partially addresses Consensys#6595

Signed-off-by: Paul Harris <[email protected]>
rolfyone added a commit to rolfyone/teku that referenced this issue Nov 24, 2023
Implemented get_proposer_head function, and tested results via the reftests.

It's going to be more efficient to get the justified state once, so have refactored isHeadWeak and isParentStrong.

The ordering of checks was cheapest -> most expensive, we can avoid fetching state if any of the lighter tests fail.

partially addresses Consensys#6595

Signed-off-by: Paul Harris <[email protected]>
rolfyone added a commit to rolfyone/teku that referenced this issue Nov 27, 2023
Implemented get_proposer_head function, and tested results via the reftests.

It's going to be more efficient to get the justified state once, so have refactored isHeadWeak and isParentStrong.

The ordering of checks was cheapest -> most expensive, we can avoid fetching state if any of the lighter tests fail.

partially addresses Consensys#6595

Signed-off-by: Paul Harris <[email protected]>
rolfyone added a commit to rolfyone/teku that referenced this issue Nov 29, 2023
Implemented should_override_fork_choice function, and tested results via the reftests.

There's a level of duplication in the 'should_override_fork_choice' and 'get_proposer_head', but I've stayed true to the spec, while fixing ordering a little to do cheaper tests first.

Ultimately, the function will exit fast once it finds a mis-match, and if it gets to the end then things pass.

Also had to adjust timeliness to use store time, and I made the Store implement the TimeProvider to fix an issue with using a system time provider when store time was needed.

Finally, while I did successfully run the reftests, I have to leave the reftests disabled until we get a release that doesnt contain backwards ticks. It's possible to remove the ticks to run the tests, but they won't run out of the box.

partially addresses Consensys#6595

Signed-off-by: Paul Harris <[email protected]>
rolfyone added a commit that referenced this issue Nov 30, 2023
Implemented should_override_fork_choice function, and tested results via the reftests.

There's a level of duplication in the 'should_override_fork_choice' and 'get_proposer_head', but I've stayed true to the spec, while fixing ordering a little to do cheaper tests first.

Ultimately, the function will exit fast once it finds a mis-match, and if it gets to the end then things pass.

Also had to adjust timeliness to use store time, and I made the Store implement the TimeProvider to fix an issue with using a system time provider when store time was needed.

Finally, while I did successfully run the reftests, I have to leave the reftests disabled until we get a release that doesnt contain backwards ticks. It's possible to remove the ticks to run the tests, but they won't run out of the box.

partially addresses #6595

Signed-off-by: Paul Harris <[email protected]>
rolfyone added a commit to rolfyone/teku that referenced this issue Dec 5, 2023
 Currently not enabled, just the flag, and defaulting to false, and a test that shows it can be toggled on.

 partially addresses Consensys#6595

Signed-off-by: Paul Harris <[email protected]>
rolfyone added a commit that referenced this issue Dec 5, 2023
 Currently not enabled, just the flag, and defaulting to false, and a test that shows it can be toggled on.

 partially addresses #6595

Signed-off-by: Paul Harris <[email protected]>
rolfyone added a commit to rolfyone/teku that referenced this issue Jan 24, 2024
Removed state from isHeadWeak and isParentStrong - these values now get cached on updateHeadTransaction

Renamed newSlot to proposalSlot

Added a function to cache weights, as they're expensive to compute and we shouldn't need to compute each call.

debug logging added in recentChainData at decission points.

partially addresses Consensys#6595 - this is a subset of Consensys#7789

Signed-off-by: Paul Harris <[email protected]>
rolfyone added a commit to rolfyone/teku that referenced this issue Jan 28, 2024
Currently late block reorg is disabled so this defers to `getStateAtSlotExact`

This functionality diverges from `getStateAtSlotExact` with late block reorg, as sometimes the state we're wanting to use is the parent of the head. In this scenario we actually need to check the proposer head, and get the appropriate state based on that instead.

The fallback given all things being equal currently is to call getStateAtSlotExact, which is basically falling back to the old functionality.
 - if late block reorg is disabled
 - if the head root can't be read
 - if the headRoot is the same as the root we're told to use from getProposerHead.

 In late block reorg case where we're using parent, we need to call `regenerateBeaconState` to apply empty slots.

 partially addresses Consensys#6595

Signed-off-by: Paul Harris <[email protected]>
rolfyone added a commit that referenced this issue Jan 31, 2024
Currently late block reorg is disabled so this defers to `getStateAtSlotExact`

This functionality diverges from `getStateAtSlotExact` with late block reorg, as sometimes the state we're wanting to use is the parent of the head. In this scenario we actually need to check the proposer head, and get the appropriate state based on that instead.

The fallback given all things being equal currently is to call getStateAtSlotExact, which is basically falling back to the old functionality.
 - if late block reorg is disabled
 - if the head root can't be read
 - if the headRoot is the same as the root we're told to use from getProposerHead.

 In late block reorg case where we're using parent, we need to call `regenerateBeaconState` to apply empty slots.

 partially addresses #6595

Signed-off-by: Paul Harris <[email protected]>
rolfyone added a commit to rolfyone/teku that referenced this issue Jan 31, 2024
ForkChoiceNotifier needed to be supplier because it wasn't intiialised from BeaconChainController.

Also renamed `newSlot` to `proposalSlot` in line with a previous change.

adjunct to Consensys#6595

Signed-off-by: Paul Harris <[email protected]>
rolfyone added a commit that referenced this issue Feb 1, 2024
rolfyone added a commit to rolfyone/teku that referenced this issue Feb 11, 2024
Moved to the tracker class and renamed, which allowed the possibility of writing better coverage tests, and that allowed a level of refactoring.

Had to instrument the class for testing still, but its a lot better than it was testing in RecentChainData.

partially addresses Consensys#6595

Signed-off-by: Paul Harris <[email protected]>
rolfyone added a commit that referenced this issue Feb 17, 2024
* Added tests for Late block reorg

Moved to the tracker class and renamed, which allowed the possibility of writing better coverage tests, and that allowed a level of refactoring.

Had to instrument the class for testing still, but its a lot better than it was testing in RecentChainData.

partially addresses #6595

Signed-off-by: Paul Harris <[email protected]>
@rolfyone rolfyone removed their assignment Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants