From b3fe288e5013112c83f71b1b18bfaffa531b4989 Mon Sep 17 00:00:00 2001 From: harkamal Date: Sat, 23 Jul 2022 16:51:46 +0530 Subject: [PATCH 1/5] Fix safe optimistic import --- .../blocks/verifyBlocksExecutionPayloads.ts | 131 +++++++++++++----- 1 file changed, 97 insertions(+), 34 deletions(-) diff --git a/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts b/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts index ffd1f946183a..3f98b8478b81 100644 --- a/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts +++ b/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts @@ -4,6 +4,8 @@ import { isBellatrixBlockBodyType, isMergeTransitionBlock as isMergeTransitionBlockFn, isExecutionEnabled, + isMergeTransitionComplete, + BeaconStateBellatrix, } from "@lodestar/state-transition"; import {bellatrix, allForks} from "@lodestar/types"; import {toHexString} from "@chainsafe/ssz"; @@ -41,6 +43,88 @@ export async function verifyBlocksExecutionPayload( const executionStatuses: ExecutionStatus[] = []; let mergeBlockFound: bellatrix.BeaconBlock | null = null; + // We need to track and keep updating if its safe to optimistically import these blocks. + // The following is how we determine for a block if its safe: + // + // (but we need to modify this check for this segment of blocks because it checks if the + // parent of any block imported in forkchoice is post-merge and currently we could only + // have blocks[0]'s parent imported in the chain as this is no longer one by one verify + + // import.) + // + // For a block with SYNCING status (called optimistic block), it's okay to import with + // SYNCING status as EL could switch into syncing + // + // 1. On intial startup/restart + // 2. When some reorg might have occured and EL doesn't has a parent root + // (observed on devnets) + // 3. Because of some unavailable (and potentially invalid) root but there is no way + // of knowing if this is invalid/unavailable. For unavailable block, some proposer + // will (sooner or later) build on the available parent head which will + // eventually win in fork-choice as other validators vote on VALID blocks. + // + // Once EL catches up again and respond VALID, the fork choice will be updated which + // will either validate or prune invalid blocks + // + // When to import such blocks: + // From: https://github.com/ethereum/consensus-specs/pull/2844 + // A block MUST NOT be optimistically imported, unless either of the following + // conditions are met: + // + // 1. Parent of the block has execution + // + // Since with the sync optimizations, the previous block might not have been in the + // forkChoice yet, so the below check could fail for safeSlotsToImportOptimistically + // + // Luckily, we can depend on the preState0 to see if we are already post merge w.r.t + // the blocks we are importing. + // + // Or in other words if + // - block status is syncing + // - and we are not in a post merge world and is parent is not optimistically safe + // - and we are syncing close to the chain head i.e. clock slot + // - and parent is optimistically safe + // + // then throw error + // + // + // - if we are are not yet in post merge world (preState0's isMergeTransitionComplete + // is false) and + // - if we haven't yet imported a post merge ancestor in forkchoice i.e. + // - and we are syncing close to the clockSlot, i.e. merge Transition could be underway + // + // + // 2. The current slot (as per the system clock) is at least + // SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY ahead of the slot of the block being + // imported. + // This means that the merge transition could be underway and we can't afford to import + // a block which is not fully validated as it could affect liveliness of the network. + // + + // For this segment of blocks: + // We are optimistically safe with respec to this entire block segment if: + // - we have transitioned to post-merge world or + // - all the blocks are way behind the current slot + // - or we have already imported a post-merge parent of first block of this chain in forkchoice + const lastBlock = blocks[blocks.length - 1]; + let isOptimisticallySafe = + isMergeTransitionComplete(preState0 as BeaconStateBellatrix) || + lastBlock.message.slot + opts.safeSlotsToImportOptimistically < chain.clock.currentSlot; + const firstBlock = blocks[0]; + if (!isOptimisticallySafe) { + const parentRoot = toHexString(firstBlock.message.parentRoot); + const parentBlock = chain.forkChoice.getBlockHex(parentRoot); + if (!parentBlock) { + throw new BlockError(firstBlock, { + code: BlockErrorCode.PARENT_UNKNOWN, + parentRoot, + }); + } + // isOptimisticallySafe if we have already imported a post-merge parent of this block + if (parentBlock.executionStatus !== ExecutionStatus.PreMerge) { + isOptimisticallySafe = true; + } + } + for (const block of blocks) { // If blocks are invalid in consensus the main promise could resolve before this loop ends. // In that case stop sending blocks to execution engine @@ -48,7 +132,11 @@ export async function verifyBlocksExecutionPayload( throw new ErrorAborted("verifyBlockExecutionPayloads"); } - const {executionStatus} = await verifyBlockExecutionPayload(chain, block, preState0, opts); + const {executionStatus} = await verifyBlockExecutionPayload(chain, block, preState0, opts, isOptimisticallySafe); + // It becomes optimistically safefor following blocks if a post-merge block is deemed fit + // for import. If it would not have been safe verifyBlockExecutionPayload would throw error + // and we would not be here. + isOptimisticallySafe ||= executionStatus !== ExecutionStatus.PreMerge; executionStatuses.push(executionStatus); const isMergeTransitionBlock = @@ -122,7 +210,8 @@ export async function verifyBlockExecutionPayload( chain: VerifyBlockModules, block: allForks.SignedBeaconBlock, preState0: CachedBeaconStateAllForks, - opts: BlockProcessOpts + opts: BlockProcessOpts, + isOptimisticallySafe: boolean ): Promise<{executionStatus: ExecutionStatus}> { /** Not null if execution is enabled */ const executionPayloadEnabled = @@ -168,43 +257,17 @@ export async function verifyBlockExecutionPayload( // Accepted and Syncing have the same treatment, as final validation of block is pending case ExecutePayloadStatus.ACCEPTED: case ExecutePayloadStatus.SYNCING: { - // It's okay to ignore SYNCING status as EL could switch into syncing - // 1. On intial startup/restart - // 2. When some reorg might have occured and EL doesn't has a parent root - // (observed on devnets) - // 3. Because of some unavailable (and potentially invalid) root but there is no way - // of knowing if this is invalid/unavailable. For unavailable block, some proposer - // will (sooner or later) build on the available parent head which will - // eventually win in fork-choice as other validators vote on VALID blocks. - // Once EL catches up again and respond VALID, the fork choice will be updated which - // will either validate or prune invalid blocks - // - // When to import such blocks: - // From: https://github.com/ethereum/consensus-specs/pull/2844 - // A block MUST NOT be optimistically imported, unless either of the following - // conditions are met: - // - // 1. Parent of the block has execution - // 2. The justified checkpoint has execution enabled - // 3. The current slot (as per the system clock) is at least - // SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY ahead of the slot of the block being - // imported. - - const parentRoot = toHexString(block.message.parentRoot); - const parentBlock = chain.forkChoice.getBlockHex(parentRoot); - const justifiedBlock = chain.forkChoice.getJustifiedBlock(); - + // Check if the entire segment was deemed safe or, this block specifically itself if not in + // the safeSlotsToImportOptimistically window of current slot, then we can import else + // we need to throw and not import his block if ( - !parentBlock || - // Following condition is the !(Not) of the safe import condition - (parentBlock.executionStatus === ExecutionStatus.PreMerge && - justifiedBlock.executionStatus === ExecutionStatus.PreMerge && - block.message.slot + opts.safeSlotsToImportOptimistically > chain.clock.currentSlot) + !isOptimisticallySafe && + block.message.slot + opts.safeSlotsToImportOptimistically >= chain.clock.currentSlot ) { throw new BlockError(block, { code: BlockErrorCode.EXECUTION_ENGINE_ERROR, execStatus: ExecutePayloadStatus.UNSAFE_OPTIMISTIC_STATUS, - errorMessage: `not safe to import ${execResult.status} payload within ${opts.safeSlotsToImportOptimistically} of currentSlot, status=${execResult.status}`, + errorMessage: `not safe to import ${execResult.status} payload within ${opts.safeSlotsToImportOptimistically} of currentSlot`, }); } From c6c08f8c72df46ddf7db985d7e46c98b79ba431b Mon Sep 17 00:00:00 2001 From: harkamal Date: Sat, 23 Jul 2022 18:09:13 +0530 Subject: [PATCH 2/5] fix issues --- .../src/chain/blocks/verifyBlocksExecutionPayloads.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts b/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts index 3f98b8478b81..4b89146f4322 100644 --- a/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts +++ b/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts @@ -107,10 +107,11 @@ export async function verifyBlocksExecutionPayload( // - or we have already imported a post-merge parent of first block of this chain in forkchoice const lastBlock = blocks[blocks.length - 1]; let isOptimisticallySafe = - isMergeTransitionComplete(preState0 as BeaconStateBellatrix) || + (isBellatrixStateType(preState0) && isMergeTransitionComplete(preState0 as BeaconStateBellatrix)) || lastBlock.message.slot + opts.safeSlotsToImportOptimistically < chain.clock.currentSlot; - const firstBlock = blocks[0]; + if (!isOptimisticallySafe) { + const firstBlock = blocks[0]; const parentRoot = toHexString(firstBlock.message.parentRoot); const parentBlock = chain.forkChoice.getBlockHex(parentRoot); if (!parentBlock) { @@ -119,6 +120,7 @@ export async function verifyBlocksExecutionPayload( parentRoot, }); } + // isOptimisticallySafe if we have already imported a post-merge parent of this block if (parentBlock.executionStatus !== ExecutionStatus.PreMerge) { isOptimisticallySafe = true; From e2b8d052e1ceed11c92e64f7b03ba8e427829479 Mon Sep 17 00:00:00 2001 From: g11tech Date: Tue, 26 Jul 2022 12:49:58 +0530 Subject: [PATCH 3/5] correct comment text Co-authored-by: Lion - dapplion <35266934+dapplion@users.noreply.github.com> --- .../src/chain/blocks/verifyBlocksExecutionPayloads.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts b/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts index 4b89146f4322..21909dc946bc 100644 --- a/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts +++ b/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts @@ -135,7 +135,7 @@ export async function verifyBlocksExecutionPayload( } const {executionStatus} = await verifyBlockExecutionPayload(chain, block, preState0, opts, isOptimisticallySafe); - // It becomes optimistically safefor following blocks if a post-merge block is deemed fit + // It becomes optimistically safe for following blocks if a post-merge block is deemed fit // for import. If it would not have been safe verifyBlockExecutionPayload would throw error // and we would not be here. isOptimisticallySafe ||= executionStatus !== ExecutionStatus.PreMerge; From c7ea53e6fec591bc4628a8c28ab06883c4e421d9 Mon Sep 17 00:00:00 2001 From: harkamal Date: Tue, 26 Jul 2022 13:44:41 +0530 Subject: [PATCH 4/5] propagate the parent of the segment from sanity checks --- .../beacon-node/src/chain/blocks/index.ts | 3 +- .../src/chain/blocks/verifyBlock.ts | 5 +- .../blocks/verifyBlocksExecutionPayloads.ts | 56 +++++++------------ .../chain/blocks/verifyBlocksSanityChecks.ts | 23 ++++---- 4 files changed, 38 insertions(+), 49 deletions(-) diff --git a/packages/beacon-node/src/chain/blocks/index.ts b/packages/beacon-node/src/chain/blocks/index.ts index df47824824de..00ee70d0aef8 100644 --- a/packages/beacon-node/src/chain/blocks/index.ts +++ b/packages/beacon-node/src/chain/blocks/index.ts @@ -63,7 +63,7 @@ export async function processBlocks( } try { - const {relevantBlocks, parentSlots} = verifyBlocksSanityChecks(chain, blocks, opts); + const {relevantBlocks, parentSlots, parentBlock} = verifyBlocksSanityChecks(chain, blocks, opts); // No relevant blocks, skip verifyBlocksInEpoch() if (relevantBlocks.length === 0) { @@ -74,6 +74,7 @@ export async function processBlocks( // states in the state cache through regen. const {postStates, executionStatuses, proposerBalanceDeltas} = await verifyBlocksInEpoch( chain, + parentBlock, relevantBlocks, opts ); diff --git a/packages/beacon-node/src/chain/blocks/verifyBlock.ts b/packages/beacon-node/src/chain/blocks/verifyBlock.ts index 41babad566ea..580e10887e80 100644 --- a/packages/beacon-node/src/chain/blocks/verifyBlock.ts +++ b/packages/beacon-node/src/chain/blocks/verifyBlock.ts @@ -1,7 +1,7 @@ import {CachedBeaconStateAllForks, computeEpochAtSlot} from "@lodestar/state-transition"; import {allForks, bellatrix} from "@lodestar/types"; import {toHexString} from "@chainsafe/ssz"; -import {IForkChoice, ExecutionStatus} from "@lodestar/fork-choice"; +import {IForkChoice, ExecutionStatus, ProtoBlock} from "@lodestar/fork-choice"; import {IChainForkConfig} from "@lodestar/config"; import {ILogger} from "@lodestar/utils"; import {IMetrics} from "../../metrics/index.js"; @@ -43,6 +43,7 @@ export type VerifyBlockModules = { */ export async function verifyBlocksInEpoch( chain: VerifyBlockModules, + parentBlock: ProtoBlock, blocks: allForks.SignedBeaconBlock[], opts: BlockProcessOpts & ImportBlockOpts ): Promise<{ @@ -88,7 +89,7 @@ export async function verifyBlocksInEpoch( verifyBlocksSignatures(chain, preState0, blocks, opts), // Execution payloads - verifyBlocksExecutionPayload(chain, blocks, preState0, abortController.signal, opts), + verifyBlocksExecutionPayload(chain, parentBlock, blocks, preState0, abortController.signal, opts), ]); if (mergeBlockFound !== null) { diff --git a/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts b/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts index 21909dc946bc..df78a28bddf3 100644 --- a/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts +++ b/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts @@ -4,12 +4,10 @@ import { isBellatrixBlockBodyType, isMergeTransitionBlock as isMergeTransitionBlockFn, isExecutionEnabled, - isMergeTransitionComplete, - BeaconStateBellatrix, } from "@lodestar/state-transition"; import {bellatrix, allForks} from "@lodestar/types"; import {toHexString} from "@chainsafe/ssz"; -import {IForkChoice, ExecutionStatus, assertValidTerminalPowBlock} from "@lodestar/fork-choice"; +import {IForkChoice, ExecutionStatus, assertValidTerminalPowBlock, ProtoBlock} from "@lodestar/fork-choice"; import {IChainForkConfig} from "@lodestar/config"; import {ErrorAborted, ILogger} from "@lodestar/utils"; import {IExecutionEngine} from "../../execution/engine/index.js"; @@ -35,6 +33,7 @@ type VerifyBlockModules = { */ export async function verifyBlocksExecutionPayload( chain: VerifyBlockModules, + parentBlock: ProtoBlock, blocks: allForks.SignedBeaconBlock[], preState0: CachedBeaconStateAllForks, signal: AbortSignal, @@ -43,14 +42,11 @@ export async function verifyBlocksExecutionPayload( const executionStatuses: ExecutionStatus[] = []; let mergeBlockFound: bellatrix.BeaconBlock | null = null; - // We need to track and keep updating if its safe to optimistically import these blocks. - // The following is how we determine for a block if its safe: - // - // (but we need to modify this check for this segment of blocks because it checks if the - // parent of any block imported in forkchoice is post-merge and currently we could only - // have blocks[0]'s parent imported in the chain as this is no longer one by one verify + - // import.) - // + // Error in the same way as verifyBlocksSanityChecks if empty blocks + if (blocks.length === 0) { + throw Error("Empty partiallyVerifiedBlocks"); + } + // For a block with SYNCING status (called optimistic block), it's okay to import with // SYNCING status as EL could switch into syncing // @@ -65,6 +61,15 @@ export async function verifyBlocksExecutionPayload( // Once EL catches up again and respond VALID, the fork choice will be updated which // will either validate or prune invalid blocks // + // We need to track and keep updating if its safe to optimistically import these blocks. + // The following is how we determine for a block if its safe: + // + // (but we need to modify this check for this segment of blocks because it checks if the + // parent of any block imported in forkchoice is post-merge and currently we could only + // have blocks[0]'s parent imported in the chain as this is no longer one by one verify + + // import.) + // + // // When to import such blocks: // From: https://github.com/ethereum/consensus-specs/pull/2844 // A block MUST NOT be optimistically imported, unless either of the following @@ -87,8 +92,6 @@ export async function verifyBlocksExecutionPayload( // then throw error // // - // - if we are are not yet in post merge world (preState0's isMergeTransitionComplete - // is false) and // - if we haven't yet imported a post merge ancestor in forkchoice i.e. // - and we are syncing close to the clockSlot, i.e. merge Transition could be underway // @@ -99,34 +102,16 @@ export async function verifyBlocksExecutionPayload( // This means that the merge transition could be underway and we can't afford to import // a block which is not fully validated as it could affect liveliness of the network. // - + // // For this segment of blocks: // We are optimistically safe with respec to this entire block segment if: - // - we have transitioned to post-merge world or // - all the blocks are way behind the current slot // - or we have already imported a post-merge parent of first block of this chain in forkchoice const lastBlock = blocks[blocks.length - 1]; let isOptimisticallySafe = - (isBellatrixStateType(preState0) && isMergeTransitionComplete(preState0 as BeaconStateBellatrix)) || + parentBlock.executionStatus !== ExecutionStatus.PreMerge || lastBlock.message.slot + opts.safeSlotsToImportOptimistically < chain.clock.currentSlot; - if (!isOptimisticallySafe) { - const firstBlock = blocks[0]; - const parentRoot = toHexString(firstBlock.message.parentRoot); - const parentBlock = chain.forkChoice.getBlockHex(parentRoot); - if (!parentBlock) { - throw new BlockError(firstBlock, { - code: BlockErrorCode.PARENT_UNKNOWN, - parentRoot, - }); - } - - // isOptimisticallySafe if we have already imported a post-merge parent of this block - if (parentBlock.executionStatus !== ExecutionStatus.PreMerge) { - isOptimisticallySafe = true; - } - } - for (const block of blocks) { // If blocks are invalid in consensus the main promise could resolve before this loop ends. // In that case stop sending blocks to execution engine @@ -138,8 +123,9 @@ export async function verifyBlocksExecutionPayload( // It becomes optimistically safe for following blocks if a post-merge block is deemed fit // for import. If it would not have been safe verifyBlockExecutionPayload would throw error // and we would not be here. - isOptimisticallySafe ||= executionStatus !== ExecutionStatus.PreMerge; - executionStatuses.push(executionStatus); + if (executionStatus !== ExecutionStatus.PreMerge) { + isOptimisticallySafe = true; + } const isMergeTransitionBlock = isBellatrixStateType(preState0) && diff --git a/packages/beacon-node/src/chain/blocks/verifyBlocksSanityChecks.ts b/packages/beacon-node/src/chain/blocks/verifyBlocksSanityChecks.ts index 4b3e70be8e02..38230da50d48 100644 --- a/packages/beacon-node/src/chain/blocks/verifyBlocksSanityChecks.ts +++ b/packages/beacon-node/src/chain/blocks/verifyBlocksSanityChecks.ts @@ -1,6 +1,6 @@ import {computeStartSlotAtEpoch} from "@lodestar/state-transition"; import {IChainForkConfig} from "@lodestar/config"; -import {IForkChoice} from "@lodestar/fork-choice"; +import {IForkChoice, ProtoBlock} from "@lodestar/fork-choice"; import {allForks, Slot} from "@lodestar/types"; import {toHexString} from "@lodestar/utils"; import {IBeaconClock} from "../clock/interface.js"; @@ -23,7 +23,7 @@ export function verifyBlocksSanityChecks( chain: {forkChoice: IForkChoice; clock: IBeaconClock; config: IChainForkConfig}, blocks: allForks.SignedBeaconBlock[], opts: ImportBlockOpts -): {relevantBlocks: allForks.SignedBeaconBlock[]; parentSlots: Slot[]} { +): {relevantBlocks: allForks.SignedBeaconBlock[]; parentSlots: Slot[]; parentBlock: ProtoBlock} { if (blocks.length === 0) { throw Error("Empty partiallyVerifiedBlocks"); } @@ -31,6 +31,14 @@ export function verifyBlocksSanityChecks( const relevantBlocks: allForks.SignedBeaconBlock[] = []; const parentSlots: Slot[] = []; + // When importing a block segment, only the first NON-IGNORED block must be known to the fork-choice. + const firstBlock = blocks[0]; + const parentRoot = toHexString(firstBlock.message.parentRoot); + const parentBlock = chain.forkChoice.getBlockHex(parentRoot); + if (!parentBlock) { + throw new BlockError(firstBlock, {code: BlockErrorCode.PARENT_UNKNOWN, parentRoot}); + } + for (const block of blocks) { const blockSlot = block.message.slot; @@ -57,18 +65,11 @@ export function verifyBlocksSanityChecks( let parentBlockSlot: Slot; - // When importing a block segment, only the first NON-IGNORED block must be known to the fork-choice. if (relevantBlocks.length > 0) { parentBlockSlot = relevantBlocks[relevantBlocks.length - 1].message.slot; } else { // Parent is known to the fork-choice - const parentRoot = toHexString(block.message.parentRoot); - const parentBlock = chain.forkChoice.getBlockHex(parentRoot); - if (!parentBlock) { - throw new BlockError(block, {code: BlockErrorCode.PARENT_UNKNOWN, parentRoot}); - } else { - parentBlockSlot = parentBlock.slot; - } + parentBlockSlot = parentBlock.slot; } // Block not in the future, also checks for infinity @@ -95,5 +96,5 @@ export function verifyBlocksSanityChecks( parentSlots.push(parentBlockSlot); } - return {relevantBlocks, parentSlots}; + return {relevantBlocks, parentSlots, parentBlock}; } From d14a1ff739eecef49b5ece120874dc89ac17c2da Mon Sep 17 00:00:00 2001 From: harkamal Date: Tue, 26 Jul 2022 15:10:54 +0530 Subject: [PATCH 5/5] fixes --- .../beacon-node/src/chain/blocks/index.ts | 3 +- .../blocks/verifyBlocksExecutionPayloads.ts | 1 + .../chain/blocks/verifyBlocksSanityChecks.ts | 28 +++++++++++-------- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/packages/beacon-node/src/chain/blocks/index.ts b/packages/beacon-node/src/chain/blocks/index.ts index 00ee70d0aef8..4d0b777670a3 100644 --- a/packages/beacon-node/src/chain/blocks/index.ts +++ b/packages/beacon-node/src/chain/blocks/index.ts @@ -66,7 +66,8 @@ export async function processBlocks( const {relevantBlocks, parentSlots, parentBlock} = verifyBlocksSanityChecks(chain, blocks, opts); // No relevant blocks, skip verifyBlocksInEpoch() - if (relevantBlocks.length === 0) { + if (relevantBlocks.length === 0 || parentBlock === null) { + // parentBlock can only be null if relevantBlocks are empty return; } diff --git a/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts b/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts index df78a28bddf3..4c5d65fa2c7d 100644 --- a/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts +++ b/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts @@ -126,6 +126,7 @@ export async function verifyBlocksExecutionPayload( if (executionStatus !== ExecutionStatus.PreMerge) { isOptimisticallySafe = true; } + executionStatuses.push(executionStatus); const isMergeTransitionBlock = isBellatrixStateType(preState0) && diff --git a/packages/beacon-node/src/chain/blocks/verifyBlocksSanityChecks.ts b/packages/beacon-node/src/chain/blocks/verifyBlocksSanityChecks.ts index 38230da50d48..1203845532e3 100644 --- a/packages/beacon-node/src/chain/blocks/verifyBlocksSanityChecks.ts +++ b/packages/beacon-node/src/chain/blocks/verifyBlocksSanityChecks.ts @@ -23,21 +23,14 @@ export function verifyBlocksSanityChecks( chain: {forkChoice: IForkChoice; clock: IBeaconClock; config: IChainForkConfig}, blocks: allForks.SignedBeaconBlock[], opts: ImportBlockOpts -): {relevantBlocks: allForks.SignedBeaconBlock[]; parentSlots: Slot[]; parentBlock: ProtoBlock} { +): {relevantBlocks: allForks.SignedBeaconBlock[]; parentSlots: Slot[]; parentBlock: ProtoBlock | null} { if (blocks.length === 0) { throw Error("Empty partiallyVerifiedBlocks"); } const relevantBlocks: allForks.SignedBeaconBlock[] = []; const parentSlots: Slot[] = []; - - // When importing a block segment, only the first NON-IGNORED block must be known to the fork-choice. - const firstBlock = blocks[0]; - const parentRoot = toHexString(firstBlock.message.parentRoot); - const parentBlock = chain.forkChoice.getBlockHex(parentRoot); - if (!parentBlock) { - throw new BlockError(firstBlock, {code: BlockErrorCode.PARENT_UNKNOWN, parentRoot}); - } + let parentBlock: ProtoBlock | null = null; for (const block of blocks) { const blockSlot = block.message.slot; @@ -68,8 +61,15 @@ export function verifyBlocksSanityChecks( if (relevantBlocks.length > 0) { parentBlockSlot = relevantBlocks[relevantBlocks.length - 1].message.slot; } else { - // Parent is known to the fork-choice - parentBlockSlot = parentBlock.slot; + // When importing a block segment, only the first NON-IGNORED block must be known to the fork-choice. + const parentRoot = toHexString(block.message.parentRoot); + parentBlock = chain.forkChoice.getBlockHex(parentRoot); + if (!parentBlock) { + throw new BlockError(block, {code: BlockErrorCode.PARENT_UNKNOWN, parentRoot}); + } else { + // Parent is known to the fork-choice + parentBlockSlot = parentBlock.slot; + } } // Block not in the future, also checks for infinity @@ -96,5 +96,11 @@ export function verifyBlocksSanityChecks( parentSlots.push(parentBlockSlot); } + // Just assert to be over cautious and for purposes to be more explicit for someone + // going through the code segment + if (parentBlock === null && relevantBlocks.length > 0) { + throw Error(`Internal error, parentBlock should not be null for relevantBlocks=${relevantBlocks.length}`); + } + return {relevantBlocks, parentSlots, parentBlock}; }