Repository navigation
feat: force the proposer to reorg unavailable blocks #9387
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
Changes from 5 commits
3e6efc0
54c200c
be04b1f
e0574a6
ab12dcd
acfe029
dee4c30
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,8 @@ import { | |
| G2_POINT_AT_INFINITY, | ||
| IBeaconStateView, | ||
| type IBeaconStateViewBellatrix, | ||
| type IBeaconStateViewGloas, | ||
| computeEpochAtSlot, | ||
| computeTimeAtSlot, | ||
| isStatePostBellatrix, | ||
| isStatePostCapella, | ||
|
|
@@ -58,10 +60,12 @@ import { | |
| PayloadId, | ||
| getExpectedGasLimit, | ||
| } from "../../execution/index.js"; | ||
| import {getShufflingDependentRoot} from "../../util/dependentRoot.js"; | ||
| import {fromGraffitiBytes} from "../../util/graffiti.js"; | ||
| import {kzg} from "../../util/kzg.js"; | ||
| import type {BeaconChain} from "../chain.js"; | ||
| import {CommonBlockBody} from "../interface.js"; | ||
| import {ProposerPreferencesPool} from "../opPools/index.js"; | ||
| import {validateBlobsAndKzgCommitments, validateCellsAndKzgCommitments} from "./validateBlobsAndKzgCommitments.js"; | ||
|
|
||
| // Time to provide the EL to generate a payload from new payload id | ||
|
|
@@ -204,6 +208,9 @@ export async function produceBlockBody<T extends BlockType>( | |
| // this into a completely separate function and have pre/post gloas more separated | ||
| const safeBlockHash = getSafeExecutionBlockHash(this.forkChoice); | ||
| const finalizedBlockHash = this.forkChoice.getFinalizedBlock().executionPayloadBlockHash ?? ZERO_HASH_HEX; | ||
| // TODO GLOAS: post-Gloas, proposer feeRecipient is also carried (signed) in | ||
| // ProposerPreferencesPool. Consider using this unified cache instead | ||
| // see https://github.com/ChainSafe/lodestar/issues/9379 | ||
| const feeRecipient = requestedFeeRecipient ?? this.beaconProposerCache.getOrDefault(proposerIndex); | ||
|
|
||
| const endExecutionPayload = this.metrics?.executionBlockProductionTimeSteps.startTimer(); | ||
|
|
@@ -219,8 +226,11 @@ export async function produceBlockBody<T extends BlockType>( | |
| let parentExecutionRequests: electra.ExecutionRequests; | ||
| // Apply parent payload once here as it's reused by EL prep and voluntary exit filtering below | ||
| let stateAfterParentPayload: IBeaconStateViewBellatrix = currentState; | ||
| const isExtendingPayload = this.forkChoice.shouldExtendPayload(toRootHex(parentBlockRoot)); | ||
| if (isExtendingPayload) { | ||
| // Spec: should_build_on_full(store, head). `parentBlock` is the proposer's head | ||
| // (set by chain.getProposerHead(slot)). Returns false when the PTC majority | ||
| // signalled the blob data is not available, forcing a build on EMPTY (reorg). | ||
| const isBuildingOnFull = this.forkChoice.shouldBuildOnFull(parentBlock); | ||
| if (isBuildingOnFull) { | ||
| parentBlockHash = currentState.latestExecutionPayloadBid.blockHash; | ||
| parentExecutionRequests = await this.getParentExecutionRequests(parentBlock.slot, parentBlock.blockRoot); | ||
| stateAfterParentPayload = currentState.withParentPayloadApplied(parentExecutionRequests); | ||
|
|
@@ -299,7 +309,7 @@ export async function produceBlockBody<T extends BlockType>( | |
| // Drop voluntary exits that parent_execution_requests have invalidated (e.g. a withdrawal | ||
| // request initiating an exit on the same validator). Op pool selected against the unapplied | ||
| // state, so re-validate against the post-apply state to avoid producing an invalid block. | ||
| if (isExtendingPayload && commonBlockBody.voluntaryExits.length > 0) { | ||
| if (isBuildingOnFull && commonBlockBody.voluntaryExits.length > 0) { | ||
| gloasBody.voluntaryExits = commonBlockBody.voluntaryExits.filter((signedVoluntaryExit) => | ||
| stateAfterParentPayload.isValidVoluntaryExit(signedVoluntaryExit, false) | ||
| ); | ||
|
|
@@ -633,6 +643,8 @@ export async function prepareExecutionPayload( | |
| chain: { | ||
| executionEngine: IExecutionEngine; | ||
| config: ChainForkConfig; | ||
| forkChoice: IForkChoice; | ||
| proposerPreferencesPool: ProposerPreferencesPool; | ||
| }, | ||
| logger: Logger, | ||
| fork: ForkPostBellatrix, | ||
|
|
@@ -733,6 +745,7 @@ export function getPayloadAttributesForSSE( | |
| chain: { | ||
| config: ChainForkConfig; | ||
| forkChoice: IForkChoice; | ||
| proposerPreferencesPool: ProposerPreferencesPool; | ||
| }, | ||
| { | ||
| prepareState, | ||
|
|
@@ -789,6 +802,8 @@ function preparePayloadAttributes( | |
| fork: ForkPostBellatrix, | ||
| chain: { | ||
| config: ChainForkConfig; | ||
| forkChoice: IForkChoice; | ||
| proposerPreferencesPool: ProposerPreferencesPool; | ||
| }, | ||
| { | ||
| prepareState, | ||
|
|
@@ -851,12 +866,59 @@ function preparePayloadAttributes( | |
| } | ||
|
|
||
| if (ForkSeq[fork] >= ForkSeq.gloas) { | ||
| if (!isStatePostGloas(prepareState)) { | ||
| throw new Error("Expected Gloas state for Gloas payload attributes"); | ||
| } | ||
| (payloadAttributes as gloas.SSEPayloadAttributes["payloadAttributes"]).slotNumber = prepareSlot; | ||
| (payloadAttributes as gloas.SSEPayloadAttributes["payloadAttributes"]).targetGasLimit = getProposerTargetGasLimit( | ||
| chain, | ||
| prepareState, | ||
| prepareSlot, | ||
| parentBlockRoot | ||
| ); | ||
| } | ||
|
|
||
| return payloadAttributes; | ||
| } | ||
|
|
||
| /** | ||
| * Resolve the proposer's preferred (target) gas limit for the Gloas `PayloadAttributesV4` | ||
| * `targetGasLimit` field (consensus-specs#5235, execution-apis#796). | ||
| * | ||
| * Sourced from the `SignedProposerPreferences` the proposer's VC submitted to the pool | ||
| * (same `(slot, dependent_root)` lookup as gossip bid validation). When no matching | ||
| * preferences are pooled, target the parent payload's gas limit so the gas limit stays | ||
| * unchanged (`is_gas_limit_target_compatible` then requires `gas_limit == parent_gas_limit`). | ||
| */ | ||
| function getProposerTargetGasLimit( | ||
| chain: {forkChoice: IForkChoice; proposerPreferencesPool: ProposerPreferencesPool}, | ||
| state: IBeaconStateViewGloas, | ||
| prepareSlot: Slot, | ||
| parentBlockRoot: Root | ||
| ): number { | ||
| const parentBlock = chain.forkChoice.getBlockHexDefaultStatus(toRootHex(parentBlockRoot)); | ||
| const dependentRootHex = (() => { | ||
| if (parentBlock === null) { | ||
| return null; | ||
| } | ||
| try { | ||
| return getShufflingDependentRoot( | ||
| chain.forkChoice, | ||
| computeEpochAtSlot(prepareSlot), | ||
| computeEpochAtSlot(parentBlock.slot), | ||
| parentBlock | ||
| ); | ||
| } catch { | ||
| return null; | ||
| } | ||
| })(); | ||
|
|
||
| const pref = dependentRootHex !== null ? chain.proposerPreferencesPool.get(prepareSlot, dependentRootHex) : null; | ||
| // TODO GLOAS: state.latestExecutionPayloadBid is the latest *bid*, not the latest *executed* | ||
| // payload — for EMPTY parents this drifts. Consider having a default value like Prysm's DefaultBuilderGasLimit. | ||
| return Number(pref ? pref.message.targetGasLimit : state.latestExecutionPayloadBid.gasLimit); | ||
|
Comment on lines
+917
to
+919
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fallback to |
||
| } | ||
|
|
||
| export async function produceCommonBlockBody<T extends BlockType>( | ||
| this: BeaconChain, | ||
| blockType: T, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
worth noting that the timing when this is called is not correct, unless I am missing some other changes we did in the meantime but #9164 (comment) and suggestion along the lines here #9164 (comment) are still relevant
we will still reorg the payload when we call
shouldBuildOnFull() == falseinproduceBlockBody.tsbut that's at the start of the next slot and the execution payload we prepared here will be on top of theblockHashand not theparentBlockHashlikely delaying this call to PTC deadline + 1 second is fine, but may not be ideal as we also don't wanna delay the FCU for too long as it gives less time to the execution client
likely the ideal approach is
shouldBuildOnFull()again at ~10 seconds (with PTC votes), or proactively if threshold is reached via event based trigger (something like here) and ifparentBlockHashchanges, call FCU again, overriding the previous callproduceBlockBody.tsat start of next slot but it should be rare that theparentBlockHashwill be different from the prepared block hash if we follow the abovealso I noticed we should probably add more logs here to know if we do a payload reorg