[Merged by Bors] - Retrospective invalidation of exec. payloads for opt. sync #2837
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
paulhauner
changed the title
|
Thanks @realbigsean and @michaelsproul! That was a great review, thanks for the improvements and bug fixes 😬 I have addressed all your comments and am waiting for CI to pass
I must have lost this one between my desktop and laptop, thanks! Added here 2ae6d78. |
paulhauner
added
ready-for-review
This was referenced Feb 27, 2022
michaelsproul
approved these changes
Feb 27, 2022
Comment on lines
+134
to
+136
| 0_i64 | ||
| .checked_sub(node.weight as i64) | ||
| .ok_or(Error::InvalidExecutionDeltaOverflow(node_index))? |
Co-authored-by: Michael Sproul <[email protected]>
|
Thanks for the approval @michaelsproul 🙏 I'll wait for approval/comments from @realbigsean before merging |
Start adding execution_status tests Complete test 01 Add tests 2, 3 Appease clippy
|
I added some non-substantive tests for the I was testing to check balance propagation with and without proposer boost. I didn't find any bugs. |
realbigsean
approved these changes
Feb 28, 2022
|
bors r+ 🎉 |
paulhauner
added
ready-for-merge
bors bot
pushed a commit
that referenced
this pull request
Feb 28, 2022
## Issue Addressed NA ## Proposed Changes Adds the functionality to allow blocks to be validated/invalidated after their import as per the [optimistic sync spec](https://github.com/ethereum/consensus-specs/blob/dev/sync/optimistic.md#how-to-optimistically-import-blocks). This means: - Updating `ProtoArray` to allow flipping the `execution_status` of ancestors/descendants based on payload validity updates. - Creating separation between `execution_layer` and the `beacon_chain` by creating a `PayloadStatus` struct. - Refactoring how the `execution_layer` selects a `PayloadStatus` from the multiple statuses returned from multiple EEs. - Adding testing framework for optimistic imports. - Add `ExecutionBlockHash(Hash256)` new-type struct to avoid confusion between *beacon block roots* and *execution payload hashes*. - Add `merge` to [`FORKS`](https://github.com/sigp/lighthouse/blob/c3a793fd73a3b11b130b82032904d39c952869e4/Makefile#L17) in the `Makefile` to ensure we test the beacon chain with merge settings. - Fix some tests here that were failing due to a missing execution layer. ## TODO - [ ] Balance tests Co-authored-by: Mark Mackey <[email protected]>
2 tasks
|
Pull request successfully merged into unstable. Build succeeded: |
bors
bot
changed the title
1 task
bors bot
pushed a commit
that referenced
this pull request
Mar 8, 2022
## Issue Addressed There has been an [`engine_exchangetransitionconfigurationv1`](https://github.com/ethereum/execution-apis/blob/main/src/engine/specification.md#engine_exchangetransitionconfigurationv1) method added to the execution API specs. The `engine_exchangetransitionconfigurationv1` will be polled every 60s as per this PR: ethereum/execution-apis#189. If that PR is merged as-is, then we will be matching the spec. If that PR *is not* merged, we are still fully compatible with the spec, but just doing more than we are required. ## Additional Info - [x] ~~Blocked on #2837~~ - [x] Add method to EE integration tests
bors bot
pushed a commit
that referenced
this pull request
Mar 9, 2022
## Issue Addressed Resolves #2936 ## Proposed Changes Adds functionality for calling [`validator/prepare_beacon_proposer`](https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/Validator/prepareBeaconProposer) in advance. There is a `BeaconChain::prepare_beacon_proposer` method which, which called, computes the proposer for the next slot. If that proposer has been registered via the `validator/prepare_beacon_proposer` API method, then the `beacon_chain.execution_layer` will be provided the `PayloadAttributes` for us in all future forkchoiceUpdated calls. An artificial forkchoiceUpdated call will be created 4s before each slot, when the head updates and when a validator updates their information. Additionally, I added strict ordering for calls from the `BeaconChain` to the `ExecutionLayer`. I'm not certain the `ExecutionLayer` will always maintain this ordering, but it's a good start to have consistency from the `BeaconChain`. There are some deadlock opportunities introduced, they are documented in the code. ## Additional Info - ~~Blocked on #2837~~ Co-authored-by: realbigsean <[email protected]>
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.
Issue Addressed
NA
Proposed Changes
Adds the functionality to allow blocks to be validated/invalidated after their import as per the optimistic sync spec. This means:
ProtoArrayto allow flipping theexecution_statusof ancestors/descendants based on payload validity updates.execution_layerand thebeacon_chainby creating aPayloadStatusstruct.execution_layerselects aPayloadStatusfrom the multiple statuses returned from multiple EEs.ExecutionBlockHash(Hash256)new-type struct to avoid confusion between beacon block roots and execution payload hashes.mergetoFORKSin theMakefileto ensure we test the beacon chain with merge settings.TODO