From 8710211cb993b1a550a427b03d880d0679539744 Mon Sep 17 00:00:00 2001 From: harkamal Date: Mon, 9 May 2022 22:53:51 +0530 Subject: [PATCH] some comments to explain the merge block validations movement --- packages/fork-choice/src/forkChoice/forkChoice.ts | 10 ++++++++++ packages/lodestar/src/chain/blocks/verifyBlock.ts | 13 +++++++++++++ 2 files changed, 23 insertions(+) diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts index 5bcfb321832..a9de7e49d9f 100644 --- a/packages/fork-choice/src/forkChoice/forkChoice.ts +++ b/packages/fork-choice/src/forkChoice/forkChoice.ts @@ -328,6 +328,16 @@ export class ForkChoice implements IForkChoice { }); } + // As per specs, we should be validating here the terminal conditions of + // the PoW if this were a merge transition block. + // (https://github.com/ethereum/consensus-specs/blob/dev/specs/bellatrix/fork-choice.md#on_block) + // + // However this check has been moved to the `verifyBlockStateTransition` in + // `packages/lodestar/src/chain/blocks/verifyBlock.ts` as: + // + // 1. Its prudent to fail fast and not try importing a block in forkChoice. + // 2. Also the data to run such a validation is readily available there. + let shouldUpdateJustified = false; const {finalizedCheckpoint} = state; const currentJustifiedCheckpoint = toCheckpointWithHex(state.currentJustifiedCheckpoint); diff --git a/packages/lodestar/src/chain/blocks/verifyBlock.ts b/packages/lodestar/src/chain/blocks/verifyBlock.ts index ec510f2400f..302bde6c98c 100644 --- a/packages/lodestar/src/chain/blocks/verifyBlock.ts +++ b/packages/lodestar/src/chain/blocks/verifyBlock.ts @@ -280,6 +280,19 @@ export async function verifyBlockStateTransition( }); } + // If this is a merge transition block, check to ensure if it references + // a valid terminal PoW block. + // + // However specs define this check to be run inside forkChoice's onBlock + // (https://github.com/ethereum/consensus-specs/blob/dev/specs/bellatrix/fork-choice.md#on_block) + // but we perform the check here (as inspired from the lighthouse impl) + // + // Reasons: + // 1. If the block is not valid, we should fail early and not wait till + // forkChoice import. + // 2. It makes logical sense to pair it with the block validations and + // deal it with the external services like eth1 tracker here than + // in import block if (isMergeTransitionBlock) { const mergeBlock = block.message as bellatrix.BeaconBlock; const powBlockRootHex = toHexString(mergeBlock.body.executionPayload.parentHash);