diff --git a/packages/beacon-node/src/chain/blocks/index.ts b/packages/beacon-node/src/chain/blocks/index.ts index 3e324fa0f23b..df47824824de 100644 --- a/packages/beacon-node/src/chain/blocks/index.ts +++ b/packages/beacon-node/src/chain/blocks/index.ts @@ -5,7 +5,7 @@ import {JobItemQueue} from "../../util/queue/index.js"; import {BlockError, BlockErrorCode} from "../errors/index.js"; import {BlockProcessOpts} from "../options.js"; import {IBeaconChain} from "../interface.js"; -import {VerifyBlockModules, verifyBlockStateTransition} from "./verifyBlock.js"; +import {VerifyBlockModules, verifyBlocksInEpoch} from "./verifyBlock.js"; import {importBlock, ImportBlockModules} from "./importBlock.js"; import {assertLinearChainSegment} from "./utils/chainSegment.js"; import {FullyVerifiedBlock, ImportBlockOpts} from "./types.js"; @@ -70,21 +70,27 @@ export async function processBlocks( return; } - for (const [i, block] of relevantBlocks.entries()) { - // Fully verify a block to be imported immediately after. Does not produce any side-effects besides adding intermediate - // states in the state cache through regen. - const {postState, executionStatus, proposerBalanceDelta} = await verifyBlockStateTransition(chain, block, opts); + // Fully verify a block to be imported immediately after. Does not produce any side-effects besides adding intermediate + // states in the state cache through regen. + const {postStates, executionStatuses, proposerBalanceDeltas} = await verifyBlocksInEpoch( + chain, + relevantBlocks, + opts + ); - const fullyVerifiedBlock: FullyVerifiedBlock = { + const fullyVerifiedBlocks = relevantBlocks.map( + (block, i): FullyVerifiedBlock => ({ block, - postState, + postState: postStates[i], parentBlockSlot: parentSlots[i], - executionStatus, - proposerBalanceDelta, + executionStatus: executionStatuses[i], + proposerBalanceDelta: proposerBalanceDeltas[i], // TODO: Make this param mandatory and capture in gossip seenTimestampSec: opts.seenTimestampSec ?? Math.floor(Date.now() / 1000), - }; + }) + ); + for (const fullyVerifiedBlock of fullyVerifiedBlocks) { // No need to sleep(0) here since `importBlock` includes a disk write // TODO: Consider batching importBlock too if it takes significant time await importBlock(chain, fullyVerifiedBlock, opts); diff --git a/packages/beacon-node/src/chain/blocks/verifyBlock.ts b/packages/beacon-node/src/chain/blocks/verifyBlock.ts index fd429ad778e3..41babad566ea 100644 --- a/packages/beacon-node/src/chain/blocks/verifyBlock.ts +++ b/packages/beacon-node/src/chain/blocks/verifyBlock.ts @@ -1,15 +1,7 @@ -import { - CachedBeaconStateAllForks, - isBellatrixStateType, - isBellatrixBlockBodyType, - isMergeTransitionBlock as isMergeTransitionBlockFn, - isExecutionEnabled, - getBlockSignatureSets, - stateTransition, -} from "@lodestar/state-transition"; +import {CachedBeaconStateAllForks, computeEpochAtSlot} from "@lodestar/state-transition"; import {allForks, bellatrix} from "@lodestar/types"; import {toHexString} from "@chainsafe/ssz"; -import {IForkChoice, ExecutionStatus, assertValidTerminalPowBlock} from "@lodestar/fork-choice"; +import {IForkChoice, ExecutionStatus} from "@lodestar/fork-choice"; import {IChainForkConfig} from "@lodestar/config"; import {ILogger} from "@lodestar/utils"; import {IMetrics} from "../../metrics/index.js"; @@ -19,11 +11,12 @@ import {IBeaconClock} from "../clock/index.js"; import {BlockProcessOpts} from "../options.js"; import {IStateRegenerator, RegenCaller} from "../regen/index.js"; import {IBlsVerifier} from "../bls/index.js"; -import {ExecutePayloadStatus} from "../../execution/engine/interface.js"; -import {byteArrayEquals} from "../../util/bytes.js"; import {IEth1ForBlockProduction} from "../../eth1/index.js"; import {ImportBlockOpts} from "./types.js"; import {POS_PANDA_MERGE_TRANSITION_BANNER} from "./utils/pandaMergeTransitionBanner.js"; +import {verifyBlocksStateTransitionOnly} from "./verifyBlocksStateTransitionOnly.js"; +import {verifyBlocksSignatures} from "./verifyBlocksSignatures.js"; +import {verifyBlocksExecutionPayload} from "./verifyBlocksExecutionPayloads.js"; export type VerifyBlockModules = { bls: IBlsVerifier; @@ -38,249 +31,79 @@ export type VerifyBlockModules = { }; /** - * Verifies a block is fully valid running the full state transition. To relieve the main thread signatures are - * verified separately in workers with chain.bls worker pool. + * Verifies 1 or more blocks are fully valid; from a linear sequence of blocks. * - * - Advance state to block's slot - per_slot_processing() - * - STFN - per_block_processing() - * - Check state root matches + * To relieve the main thread signatures are verified separately in workers with chain.bls worker pool. + * In parallel it: + * - Run full state transition in sequence + * - Verify all block's signatures in parallel + * - Submit execution payloads to EL in sequence + * + * If there's an error during one of the steps, the rest are aborted with an AbortController. */ -export async function verifyBlockStateTransition( +export async function verifyBlocksInEpoch( chain: VerifyBlockModules, - block: allForks.SignedBeaconBlock, - opts: ImportBlockOpts & BlockProcessOpts -): Promise<{postState: CachedBeaconStateAllForks; executionStatus: ExecutionStatus; proposerBalanceDelta: number}> { - const {validProposerSignature, validSignatures} = opts; - - // TODO: Skip in process chain segment - // Retrieve preState from cache (regen) - const preState = await chain.regen.getPreState(block.message, RegenCaller.processBlocksInEpoch).catch((e) => { - throw new BlockError(block, {code: BlockErrorCode.PRESTATE_MISSING, error: e as Error}); - }); - - const isMergeTransitionBlock = - isBellatrixStateType(preState) && - isBellatrixBlockBodyType(block.message.body) && - isMergeTransitionBlockFn(preState, block.message.body); - - // STFN - per_slot_processing() + per_block_processing() - // NOTE: `regen.getPreState()` should have dialed forward the state already caching checkpoint states - const useBlsBatchVerify = !opts?.disableBlsBatchVerify; - const postState = stateTransition( - preState, - block, - { - // false because it's verified below with better error typing - verifyStateRoot: false, - // if block is trusted don't verify proposer or op signature - verifyProposer: !useBlsBatchVerify && !validSignatures && !validProposerSignature, - verifySignatures: !useBlsBatchVerify && !validSignatures, - assertCorrectProgressiveBalances: opts.assertCorrectProgressiveBalances, - }, - chain.metrics - ); - - /** Not null if execution is enabled */ - const executionPayloadEnabled = - isBellatrixStateType(postState) && - isBellatrixBlockBodyType(block.message.body) && - isExecutionEnabled(postState, block.message) - ? block.message.body.executionPayload - : null; + blocks: allForks.SignedBeaconBlock[], + opts: BlockProcessOpts & ImportBlockOpts +): Promise<{ + postStates: CachedBeaconStateAllForks[]; + executionStatuses: ExecutionStatus[]; + proposerBalanceDeltas: number[]; +}> { + if (blocks.length === 0) { + throw Error("Empty partiallyVerifiedBlocks"); + } - // Verify signatures after running state transition, so all SyncCommittee signed roots are known at this point. - // We must ensure block.slot <= state.slot before running getAllBlockSignatureSets(). - // NOTE: If in the future multiple blocks signatures are verified at once, all blocks must be in the same epoch - // so the attester and proposer shufflings are correct. - if (useBlsBatchVerify && !validSignatures) { - const signatureSets = getBlockSignatureSets(postState, block, { - skipProposerSignature: validProposerSignature, - }); + const block0 = blocks[0]; + const block0Epoch = computeEpochAtSlot(block0.message.slot); - if ( - signatureSets.length > 0 && - !(await chain.bls.verifySignatureSets(signatureSets, { - verifyOnMainThread: opts?.blsVerifyOnMainThread, - })) - ) { - throw new BlockError(block, {code: BlockErrorCode.INVALID_SIGNATURE, state: postState}); + // Ensure all blocks are in the same epoch + for (let i = 1; i < blocks.length; i++) { + const blockSlot = blocks[i].message.slot; + if (block0Epoch !== computeEpochAtSlot(blockSlot)) { + throw Error(`Block ${i} slot ${blockSlot} not in same epoch ${block0Epoch}`); } } - let executionStatus: ExecutionStatus; - if (executionPayloadEnabled) { - // TODO: Handle better notifyNewPayload() returning error is syncing - const execResult = await chain.executionEngine.notifyNewPayload(executionPayloadEnabled); - - switch (execResult.status) { - case ExecutePayloadStatus.VALID: - executionStatus = ExecutionStatus.Valid; - chain.forkChoice.validateLatestHash(execResult.latestValidHash, null); - break; // OK - - case ExecutePayloadStatus.INVALID: { - // If the parentRoot is not same as latestValidHash, then the branch from latestValidHash - // to parentRoot needs to be invalidated - const parentHashHex = toHexString(block.message.parentRoot); - chain.forkChoice.validateLatestHash( - execResult.latestValidHash, - parentHashHex !== execResult.latestValidHash ? parentHashHex : null - ); - throw new BlockError(block, { - code: BlockErrorCode.EXECUTION_ENGINE_ERROR, - execStatus: execResult.status, - errorMessage: execResult.validationError ?? "", - }); - } - - // 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(); + // TODO: Skip in process chain segment + // Retrieve preState from cache (regen) + const preState0 = await chain.regen.getPreState(block0.message, RegenCaller.processBlocksInEpoch).catch((e) => { + throw new BlockError(block0, {code: BlockErrorCode.PRESTATE_MISSING, error: e as Error}); + }); - 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) - ) { - 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}`, - }); - } + // Ensure the state is in the same epoch as block0 + if (block0Epoch !== computeEpochAtSlot(preState0.slot)) { + throw Error(`preState at slot ${preState0.slot} must be dialed to block epoch ${block0Epoch}`); + } - executionStatus = ExecutionStatus.Syncing; - break; - } + const abortController = new AbortController(); - // If the block has is not valid, or it referenced an invalid terminal block then the - // block is invalid, however it has no bearing on any forkChoice cleanup - // - // There can be other reasons for which EL failed some of the observed ones are - // 1. Connection refused / can't connect to EL port - // 2. EL Internal Error - // 3. Geth sometimes gives invalid merkel root error which means invalid - // but expects it to be handled in CL as of now. But we should log as warning - // and give it as optimistic treatment and expect any other non-geth CL<>EL - // combination to reject the invalid block and propose a block. - // On kintsugi devnet, this has been observed to cause contiguous proposal failures - // as the network is geth dominated, till a non geth node proposes and moves network - // forward - // For network/unreachable errors, an optimization can be added to replay these blocks - // back. But for now, lets assume other mechanisms like unknown parent block of a future - // child block will cause it to replay + try { + const [{postStates, proposerBalanceDeltas}, , {executionStatuses, mergeBlockFound}] = await Promise.all([ + // Run state transition only + // TODO: Ensure it yields to allow flushing to workers and engine API + verifyBlocksStateTransitionOnly(chain, preState0, blocks, abortController.signal, opts), - case ExecutePayloadStatus.INVALID_BLOCK_HASH: - case ExecutePayloadStatus.ELERROR: - case ExecutePayloadStatus.UNAVAILABLE: - throw new BlockError(block, { - code: BlockErrorCode.EXECUTION_ENGINE_ERROR, - execStatus: execResult.status, - errorMessage: execResult.validationError, - }); - } + // All signatures at once + verifyBlocksSignatures(chain, preState0, blocks, opts), - // 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 mergeBlockHash = toHexString( - chain.config.getForkTypes(mergeBlock.slot).BeaconBlock.hashTreeRoot(mergeBlock) - ); - const powBlockRootHex = toHexString(mergeBlock.body.executionPayload.parentHash); - const powBlock = await chain.eth1.getPowBlock(powBlockRootHex).catch((error) => { - // Lets just warn the user here, errors if any will be reported on - // `assertValidTerminalPowBlock` checks - chain.logger.warn( - "Error fetching terminal PoW block referred in the merge transition block", - {powBlockHash: powBlockRootHex, mergeBlockHash}, - error - ); - return null; - }); - const powBlockParent = - powBlock && - (await chain.eth1.getPowBlock(powBlock.parentHash).catch((error) => { - // Lets just warn the user here, errors if any will be reported on - // `assertValidTerminalPowBlock` checks - chain.logger.warn( - "Error fetching parent of the terminal PoW block referred in the merge transition block", - {powBlockParentHash: powBlock.parentHash, powBlock: powBlockRootHex, mergeBlockHash}, - error - ); - return null; - })); + // Execution payloads + verifyBlocksExecutionPayload(chain, blocks, preState0, abortController.signal, opts), + ]); - assertValidTerminalPowBlock(chain.config, mergeBlock, {executionStatus, powBlock, powBlockParent}); + if (mergeBlockFound !== null) { + // merge block found and is fully valid = state transition + signatures + execution payload. + // TODO: Will this banner be logged during syncing? + logOnPowBlock(chain, mergeBlockFound); } - } else { - // isExecutionEnabled() -> false - executionStatus = ExecutionStatus.PreMerge; - } - - // Check state root matches - if (!byteArrayEquals(block.message.stateRoot, postState.hashTreeRoot())) { - throw new BlockError(block, { - code: BlockErrorCode.INVALID_STATE_ROOT, - root: postState.hashTreeRoot(), - expectedRoot: block.message.stateRoot, - preState, - postState, - }); - } - // All checks have passed, if this is a merge transition block we can log - if (isMergeTransitionBlock) { - logOnPowBlock(chain, block as bellatrix.SignedBeaconBlock); + return {postStates, executionStatuses, proposerBalanceDeltas}; + } finally { + abortController.abort(); } - - // For metric block profitability - const proposerIndex = block.message.proposerIndex; - const proposerBalanceDelta = postState.balances.get(proposerIndex) - preState.balances.get(proposerIndex); - - return {postState, executionStatus, proposerBalanceDelta}; } -function logOnPowBlock(chain: VerifyBlockModules, block: bellatrix.SignedBeaconBlock): void { - const mergeBlock = block.message; +function logOnPowBlock(chain: VerifyBlockModules, mergeBlock: bellatrix.BeaconBlock): void { const mergeBlockHash = toHexString(chain.config.getForkTypes(mergeBlock.slot).BeaconBlock.hashTreeRoot(mergeBlock)); const mergeExecutionHash = toHexString(mergeBlock.body.executionPayload.blockHash); const mergePowHash = toHexString(mergeBlock.body.executionPayload.parentHash); diff --git a/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts b/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts new file mode 100644 index 000000000000..ffd1f946183a --- /dev/null +++ b/packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts @@ -0,0 +1,240 @@ +import { + CachedBeaconStateAllForks, + isBellatrixStateType, + isBellatrixBlockBodyType, + isMergeTransitionBlock as isMergeTransitionBlockFn, + isExecutionEnabled, +} from "@lodestar/state-transition"; +import {bellatrix, allForks} from "@lodestar/types"; +import {toHexString} from "@chainsafe/ssz"; +import {IForkChoice, ExecutionStatus, assertValidTerminalPowBlock} from "@lodestar/fork-choice"; +import {IChainForkConfig} from "@lodestar/config"; +import {ErrorAborted, ILogger} from "@lodestar/utils"; +import {IExecutionEngine} from "../../execution/engine/index.js"; +import {BlockError, BlockErrorCode} from "../errors/index.js"; +import {IBeaconClock} from "../clock/index.js"; +import {BlockProcessOpts} from "../options.js"; +import {ExecutePayloadStatus} from "../../execution/engine/interface.js"; +import {IEth1ForBlockProduction} from "../../eth1/index.js"; + +type VerifyBlockModules = { + eth1: IEth1ForBlockProduction; + executionEngine: IExecutionEngine; + clock: IBeaconClock; + logger: ILogger; + forkChoice: IForkChoice; + config: IChainForkConfig; +}; + +/** + * Verifies 1 or more execution payloads from a linear sequence of blocks. + * + * Since the EL client must be aware of each parent, all payloads must be submited in sequence. + */ +export async function verifyBlocksExecutionPayload( + chain: VerifyBlockModules, + blocks: allForks.SignedBeaconBlock[], + preState0: CachedBeaconStateAllForks, + signal: AbortSignal, + opts: BlockProcessOpts +): Promise<{executionStatuses: ExecutionStatus[]; mergeBlockFound: bellatrix.BeaconBlock | null}> { + const executionStatuses: ExecutionStatus[] = []; + let mergeBlockFound: bellatrix.BeaconBlock | null = null; + + 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 + if (signal.aborted) { + throw new ErrorAborted("verifyBlockExecutionPayloads"); + } + + const {executionStatus} = await verifyBlockExecutionPayload(chain, block, preState0, opts); + executionStatuses.push(executionStatus); + + const isMergeTransitionBlock = + isBellatrixStateType(preState0) && + isBellatrixBlockBodyType(block.message.body) && + isMergeTransitionBlockFn(preState0, block.message.body); + + // 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 mergeBlockHash = toHexString( + chain.config.getForkTypes(mergeBlock.slot).BeaconBlock.hashTreeRoot(mergeBlock) + ); + const powBlockRootHex = toHexString(mergeBlock.body.executionPayload.parentHash); + const powBlock = await chain.eth1.getPowBlock(powBlockRootHex).catch((error) => { + // Lets just warn the user here, errors if any will be reported on + // `assertValidTerminalPowBlock` checks + chain.logger.warn( + "Error fetching terminal PoW block referred in the merge transition block", + {powBlockHash: powBlockRootHex, mergeBlockHash}, + error + ); + return null; + }); + + const powBlockParent = + powBlock && + (await chain.eth1.getPowBlock(powBlock.parentHash).catch((error) => { + // Lets just warn the user here, errors if any will be reported on + // `assertValidTerminalPowBlock` checks + chain.logger.warn( + "Error fetching parent of the terminal PoW block referred in the merge transition block", + {powBlockParentHash: powBlock.parentHash, powBlock: powBlockRootHex, mergeBlockHash}, + error + ); + return null; + })); + + // executionStatus will never == ExecutionStatus.PreMerge if it's the mergeBlock. But gotta make TS happy =D + if (executionStatus === ExecutionStatus.PreMerge) { + throw Error("Merge block must not have executionStatus == PreMerge"); + } + + assertValidTerminalPowBlock(chain.config, mergeBlock, {executionStatus, powBlock, powBlockParent}); + + // Valid execution payload, but may not be in a valid beacon chain block. Delay printing the POS ACTIVATED banner + // to the end of the verify block routine, which confirms that this block is fully valid. + mergeBlockFound = mergeBlock; + } + } + + return {executionStatuses, mergeBlockFound}; +} + +/** + * Verifies a single block execution payload by sending it to the EL client (via HTTP). + */ +export async function verifyBlockExecutionPayload( + chain: VerifyBlockModules, + block: allForks.SignedBeaconBlock, + preState0: CachedBeaconStateAllForks, + opts: BlockProcessOpts +): Promise<{executionStatus: ExecutionStatus}> { + /** Not null if execution is enabled */ + const executionPayloadEnabled = + isBellatrixStateType(preState0) && + isBellatrixBlockBodyType(block.message.body) && + // Safe to use with a state previous to block's preState. isMergeComplete can only transition from false to true. + // - If preState0 is after merge block: condition is true, and will always be true + // - If preState0 is before merge block: the block could lie but then state transition function will throw above + // It is kinda safe to send non-trusted payloads to the execution client because at most it can trigger sync. + // TODO: If this becomes a problem, do some basic verification beforehand, like checking the proposer signature. + isExecutionEnabled(preState0, block.message) + ? block.message.body.executionPayload + : null; + + if (!executionPayloadEnabled) { + // isExecutionEnabled() -> false + return {executionStatus: ExecutionStatus.PreMerge}; + } + + // TODO: Handle better notifyNewPayload() returning error is syncing + const execResult = await chain.executionEngine.notifyNewPayload(executionPayloadEnabled); + + switch (execResult.status) { + case ExecutePayloadStatus.VALID: + chain.forkChoice.validateLatestHash(execResult.latestValidHash, null); + return {executionStatus: ExecutionStatus.Valid}; + + case ExecutePayloadStatus.INVALID: { + // If the parentRoot is not same as latestValidHash, then the branch from latestValidHash + // to parentRoot needs to be invalidated + const parentHashHex = toHexString(block.message.parentRoot); + chain.forkChoice.validateLatestHash( + execResult.latestValidHash, + parentHashHex !== execResult.latestValidHash ? parentHashHex : null + ); + throw new BlockError(block, { + code: BlockErrorCode.EXECUTION_ENGINE_ERROR, + execStatus: execResult.status, + errorMessage: execResult.validationError ?? "", + }); + } + + // 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(); + + 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) + ) { + 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}`, + }); + } + + return {executionStatus: ExecutionStatus.Syncing}; + } + + // If the block has is not valid, or it referenced an invalid terminal block then the + // block is invalid, however it has no bearing on any forkChoice cleanup + // + // There can be other reasons for which EL failed some of the observed ones are + // 1. Connection refused / can't connect to EL port + // 2. EL Internal Error + // 3. Geth sometimes gives invalid merkel root error which means invalid + // but expects it to be handled in CL as of now. But we should log as warning + // and give it as optimistic treatment and expect any other non-geth CL<>EL + // combination to reject the invalid block and propose a block. + // On kintsugi devnet, this has been observed to cause contiguous proposal failures + // as the network is geth dominated, till a non geth node proposes and moves network + // forward + // For network/unreachable errors, an optimization can be added to replay these blocks + // back. But for now, lets assume other mechanisms like unknown parent block of a future + // child block will cause it to replay + + case ExecutePayloadStatus.INVALID_BLOCK_HASH: + case ExecutePayloadStatus.ELERROR: + case ExecutePayloadStatus.UNAVAILABLE: + throw new BlockError(block, { + code: BlockErrorCode.EXECUTION_ENGINE_ERROR, + execStatus: execResult.status, + errorMessage: execResult.validationError, + }); + } +} diff --git a/packages/beacon-node/src/chain/blocks/verifyBlocksSanityChecks.ts b/packages/beacon-node/src/chain/blocks/verifyBlocksSanityChecks.ts index 141c17cb084e..4b3e70be8e02 100644 --- a/packages/beacon-node/src/chain/blocks/verifyBlocksSanityChecks.ts +++ b/packages/beacon-node/src/chain/blocks/verifyBlocksSanityChecks.ts @@ -31,8 +31,7 @@ export function verifyBlocksSanityChecks( const relevantBlocks: allForks.SignedBeaconBlock[] = []; const parentSlots: Slot[] = []; - for (let i = 0; i < blocks.length; i++) { - const block = blocks[i]; + for (const block of blocks) { const blockSlot = block.message.slot; // Not genesis block diff --git a/packages/beacon-node/src/chain/blocks/verifyBlocksSignatures.ts b/packages/beacon-node/src/chain/blocks/verifyBlocksSignatures.ts new file mode 100644 index 000000000000..94e65fc743d4 --- /dev/null +++ b/packages/beacon-node/src/chain/blocks/verifyBlocksSignatures.ts @@ -0,0 +1,80 @@ +import {CachedBeaconStateAllForks, getBlockSignatureSets} from "@lodestar/state-transition"; +import {allForks} from "@lodestar/types"; +import {sleep} from "@lodestar/utils"; +import {IBlsVerifier} from "../bls/index.js"; +import {BlockError, BlockErrorCode} from "../errors/blockError.js"; +import {ImportBlockOpts} from "./types.js"; + +/** + * Verifies 1 or more block's signatures from a group of blocks in the same epoch. + * getBlockSignatureSets() guarantees to return the correct signingRoots as long as all blocks belong in the same + * epoch as `preState0`. Otherwise the shufflings won't be correct. + * + * Since all data is known in advance all signatures are verified at once in parallel. + */ +export async function verifyBlocksSignatures( + chain: {bls: IBlsVerifier}, + preState0: CachedBeaconStateAllForks, + blocks: allForks.SignedBeaconBlock[], + opts: ImportBlockOpts +): Promise { + const isValidPromises: Promise[] = []; + + // Verifies signatures after running state transition, so all SyncCommittee signed roots are known at this point. + // We must ensure block.slot <= state.slot before running getAllBlockSignatureSets(). + // NOTE: If in the future multiple blocks signatures are verified at once, all blocks must be in the same epoch + // so the attester and proposer shufflings are correct. + for (const [i, block] of blocks.entries()) { + // Use [i] to make clear that the index has to be correct to blame the right block below on BlockError() + isValidPromises[i] = opts.validSignatures + ? // Skip all signature verification + Promise.resolve(true) + : // + // Verify signatures per block to track which block is invalid + chain.bls.verifySignatureSets( + getBlockSignatureSets(preState0, block, {skipProposerSignature: opts.validProposerSignature}) + ); + + // getBlockSignatureSets() takes 45ms in benchmarks for 2022Q2 mainnet blocks (100 sigs). When syncing a 32 blocks + // segments it will block the event loop for 1400 ms, which is too much. This sleep will allow the event loop to + // yield, which will cause one block's state transition to run. However, the tradeoff is okay and doesn't slow sync + if ((i + 1) % 8 === 0) { + await sleep(0); + } + } + + // `rejectFirstInvalidResolveAllValid()` returns on isValid result with its index + const res = await rejectFirstInvalidResolveAllValid(isValidPromises); + if (!res.allValid) { + throw new BlockError(blocks[res.index], {code: BlockErrorCode.INVALID_SIGNATURE, state: preState0}); + } +} + +type AllValidRes = {allValid: true} | {allValid: false; index: number}; + +/** + * From an array of promises that resolve a boolean isValid + * - if all valid, await all and return + * - if one invalid, abort immediately and return index of invalid + */ +export function rejectFirstInvalidResolveAllValid(isValidPromises: Promise[]): Promise { + return new Promise((resolve, reject) => { + let validCount = 0; + + for (let i = 0; i < isValidPromises.length; i++) { + isValidPromises[i] + .then((isValid) => { + if (isValid) { + if (++validCount >= isValidPromises.length) { + resolve({allValid: true}); + } + } else { + resolve({allValid: false, index: i}); + } + }) + .catch((e) => { + reject(e); + }); + } + }); +} diff --git a/packages/beacon-node/src/chain/blocks/verifyBlocksStateTransitionOnly.ts b/packages/beacon-node/src/chain/blocks/verifyBlocksStateTransitionOnly.ts new file mode 100644 index 000000000000..6e69142e6ffc --- /dev/null +++ b/packages/beacon-node/src/chain/blocks/verifyBlocksStateTransitionOnly.ts @@ -0,0 +1,79 @@ +import {CachedBeaconStateAllForks, stateTransition} from "@lodestar/state-transition"; +import {allForks} from "@lodestar/types"; +import {ErrorAborted, sleep} from "@lodestar/utils"; +import {IMetrics} from "../../metrics/index.js"; +import {BlockError, BlockErrorCode} from "../errors/index.js"; +import {BlockProcessOpts} from "../options.js"; +import {byteArrayEquals} from "../../util/bytes.js"; +import {ImportBlockOpts} from "./types.js"; + +/** + * Verifies 1 or more blocks are fully valid running the full state transition; from a linear sequence of blocks. + * + * - Advance state to block's slot - per_slot_processing() + * - For each block: + * - STFN - per_block_processing() + * - Check state root matches + */ +export async function verifyBlocksStateTransitionOnly( + chain: {metrics: IMetrics | null}, + preState0: CachedBeaconStateAllForks, + blocks: allForks.SignedBeaconBlock[], + signal: AbortSignal, + opts: BlockProcessOpts & ImportBlockOpts +): Promise<{postStates: CachedBeaconStateAllForks[]; proposerBalanceDeltas: number[]}> { + const postStates: CachedBeaconStateAllForks[] = []; + const proposerBalanceDeltas: number[] = []; + + for (let i = 0; i < blocks.length; i++) { + const {validProposerSignature, validSignatures} = opts; + const block = blocks[i]; + const preState = i === 0 ? preState0 : postStates[i - 1]; + + // STFN - per_slot_processing() + per_block_processing() + // NOTE: `regen.getPreState()` should have dialed forward the state already caching checkpoint states + const useBlsBatchVerify = !opts?.disableBlsBatchVerify; + const postState = stateTransition( + preState, + block, + { + // false because it's verified below with better error typing + verifyStateRoot: false, + // if block is trusted don't verify proposer or op signature + verifyProposer: !useBlsBatchVerify && !validSignatures && !validProposerSignature, + verifySignatures: !useBlsBatchVerify && !validSignatures, + }, + chain.metrics + ); + + // Check state root matches + if (!byteArrayEquals(block.message.stateRoot, postState.hashTreeRoot())) { + throw new BlockError(block, { + code: BlockErrorCode.INVALID_STATE_ROOT, + root: postState.hashTreeRoot(), + expectedRoot: block.message.stateRoot, + preState, + postState, + }); + } + + postStates[i] = postState; + + // For metric block profitability + const proposerIndex = block.message.proposerIndex; + proposerBalanceDeltas[i] = postState.balances.get(proposerIndex) - preState.balances.get(proposerIndex); + + // If blocks are invalid in execution the main promise could resolve before this loop ends. + // In that case stop processing blocks and return early. + if (signal.aborted) { + throw new ErrorAborted("verifyBlockStateTransitionOnly"); + } + + // this avoids keeping our node busy processing blocks + if (i < blocks.length - 1) { + await sleep(0); + } + } + + return {postStates, proposerBalanceDeltas}; +} diff --git a/packages/beacon-node/src/chain/regen/queued.ts b/packages/beacon-node/src/chain/regen/queued.ts index 87c7b7cd9857..3dae1c55265f 100644 --- a/packages/beacon-node/src/chain/regen/queued.ts +++ b/packages/beacon-node/src/chain/regen/queued.ts @@ -69,8 +69,15 @@ export class QueuedStateRegenerator implements IStateRegenerator { // Check the checkpoint cache (if the pre-state is a checkpoint state) if (parentEpoch < blockEpoch) { const checkpointState = this.checkpointStateCache.getLatest(parentRoot, blockEpoch); - if (checkpointState) { + if (checkpointState && computeEpochAtSlot(checkpointState.slot) === blockEpoch) { + // TODO: Miss-use of checkpointStateCache here return checkpointState; + // console.error({ + // "checkpointState.slot": checkpointState.slot, + // "block.slot": block.slot, + // blockEpoch, + // blockEpochStartSlot: computeStartSlotAtEpoch(blockEpoch), + // }); } } diff --git a/packages/beacon-node/src/sync/constants.ts b/packages/beacon-node/src/sync/constants.ts index c7672194ccbc..020e525d9ba1 100644 --- a/packages/beacon-node/src/sync/constants.ts +++ b/packages/beacon-node/src/sync/constants.ts @@ -43,7 +43,7 @@ export const EPOCHS_PER_BATCH = 1; /** * The maximum number of batches to queue before requesting more. * In good network conditions downloading batches is much faster than processing them - *A number > 10 epochs worth results in wasted progress when the chain completes syncing + * A number > 10 epochs worth results in wasted progress when the chain completes syncing * * TODO: When switching branches usually all batches in AwaitingProcessing are dropped, could it be optimized? */ diff --git a/packages/beacon-node/src/sync/range/chain.ts b/packages/beacon-node/src/sync/range/chain.ts index 3147b7a98a51..182fb781b39d 100644 --- a/packages/beacon-node/src/sync/range/chain.ts +++ b/packages/beacon-node/src/sync/range/chain.ts @@ -96,7 +96,8 @@ export class SyncChain { readonly firstBatchEpoch: Epoch; /** * The start of the chain segment. Any epoch previous to this one has been validated. - * But the `lastEpochWithProcessBlocks` may not be valid entirely. The + * Note: lastEpochWithProcessBlocks` signals the epoch at which 1 or more blocks have been processed + * successfully. So that epoch itself may or may not be valid. */ private lastEpochWithProcessBlocks: Epoch; private status = SyncChainStatus.Stopped; diff --git a/packages/beacon-node/test/unit/chain/blocks/rejectFirstInvalidResolveAllValid.test.ts b/packages/beacon-node/test/unit/chain/blocks/rejectFirstInvalidResolveAllValid.test.ts new file mode 100644 index 000000000000..eb92e3ff5713 --- /dev/null +++ b/packages/beacon-node/test/unit/chain/blocks/rejectFirstInvalidResolveAllValid.test.ts @@ -0,0 +1,79 @@ +import {expect} from "chai"; +import {rejectFirstInvalidResolveAllValid} from "../../../../src/chain/blocks/verifyBlocksSignatures.js"; + +/* eslint-disable @typescript-eslint/explicit-function-return-type */ + +describe("chain / blocks / rejectFirstInvalidResolveAllValid", () => { + it("Reject on first isValid = false", async () => { + const {resolves, log, logStrs} = prepareTest(); + await tick(); + + log("2_true"); + resolves[2](true); + await tick(); + + // Should resolve rejectFirstInvalidResolveAllValid() + log("1_false"); + resolves[1](false); + await tick(); + + // Already done + log("0_false"); + resolves[0](false); + await tick(); + + expect(logStrs).deep.equals(["2_true", "1_false", "invalid_1", "0_false"]); + }); + + it("Resolve when all isValid = true", async () => { + const {resolves, log, logStrs} = prepareTest(); + await tick(); + + for (const [i, resolve] of resolves.entries()) { + log(`${i}_true`); + resolve(true); + await tick(); + } + + expect(logStrs).deep.equals(["0_true", "1_true", "2_true", "all_valid"]); + }); +}); + +function tick() { + return new Promise((r) => process.nextTick(r)); +} + +function prepareTest() { + const promises: Promise[] = []; + const resolves: ((value: boolean) => void)[] = []; + for (let i = 0; i < 3; i++) { + const {promise, resolve} = resolvablePromise(); + promises.push(promise); + resolves.push(resolve); + } + + const logStrs: string[] = []; + + function log(str: string) { + logStrs.push(str); + } + + rejectFirstInvalidResolveAllValid(promises) + .then((res) => { + if (res.allValid) log("all_valid"); + else log(`invalid_${res.index}`); + }) + .catch(() => log("all_error")); + + return {resolves, log, logStrs}; +} + +// eslint-disable-next-line @typescript-eslint/explicit-function-return-type +function resolvablePromise() { + let resolve: ((value: T) => void) | null = null; + const promise = new Promise((_resolve) => { + resolve = _resolve; + }); + if (resolve === null) throw Error("resolve is null"); + return {promise, resolve}; +}