diff --git a/beacon_chain/consensus_object_pools/block_clearance.nim b/beacon_chain/consensus_object_pools/block_clearance.nim index 692dbbcca..a6fad24bd 100644 --- a/beacon_chain/consensus_object_pools/block_clearance.nim +++ b/beacon_chain/consensus_object_pools/block_clearance.nim @@ -45,7 +45,8 @@ proc addResolvedHeadBlock( let blockRoot = trustedBlock.root - blockRef = BlockRef.init(blockRoot, trustedBlock.message) + blockRef = BlockRef.init( + blockRoot, executionValid = blockVerified, trustedBlock.message) startTick = Moment.now() link(parent, blockRef) @@ -95,9 +96,6 @@ proc addResolvedHeadBlock( dag.putShufflingRef( ShufflingRef.init(state, cache, blockRef.slot.epoch + 1)) - if not blockVerified: - dag.optimisticRoots.incl blockRoot - # Notify others of the new block before processing the quarantine, such that # notifications for parents happens before those of the children if onBlockAdded != nil: diff --git a/beacon_chain/consensus_object_pools/block_dag.nim b/beacon_chain/consensus_object_pools/block_dag.nim index eda657fe5..8eca5aed7 100644 --- a/beacon_chain/consensus_object_pools/block_dag.nim +++ b/beacon_chain/consensus_object_pools/block_dag.nim @@ -35,6 +35,7 @@ type ## Root that can be used to retrieve block data from database executionBlockHash*: Opt[Eth2Digest] + executionValid*: bool parent*: BlockRef ##\ ## Not nil, except for the finalized head @@ -54,24 +55,29 @@ template slot*(blck: BlockRef): Slot = blck.bid.slot func init*( T: type BlockRef, root: Eth2Digest, - executionBlockHash: Opt[Eth2Digest], slot: Slot): BlockRef = + executionBlockHash: Opt[Eth2Digest], executionValid: bool, slot: Slot): + BlockRef = BlockRef( bid: BlockId(root: root, slot: slot), - executionBlockHash: executionBlockHash) + executionBlockHash: executionBlockHash, executionValid: executionValid) func init*( - T: type BlockRef, root: Eth2Digest, + T: type BlockRef, root: Eth2Digest, executionValid: bool, blck: phase0.SomeBeaconBlock | altair.SomeBeaconBlock | phase0.TrustedBeaconBlock | altair.TrustedBeaconBlock): BlockRef = - BlockRef.init(root, Opt.some ZERO_HASH, blck.slot) + # Use same formal parameters for simplicity, but it's impossible for these + # blocks to be optimistic. + BlockRef.init(root, Opt.some ZERO_HASH, executionValid = true, blck.slot) func init*( - T: type BlockRef, root: Eth2Digest, + T: type BlockRef, root: Eth2Digest, executionValid: bool, blck: bellatrix.SomeBeaconBlock | bellatrix.TrustedBeaconBlock | capella.SomeBeaconBlock | capella.TrustedBeaconBlock | deneb.SomeBeaconBlock | deneb.TrustedBeaconBlock): BlockRef = BlockRef.init( root, Opt.some Eth2Digest(blck.body.execution_payload.block_hash), + executionValid = + blck.body.execution_payload.block_hash == ZERO_HASH or executionValid, blck.slot) func parent*(bs: BlockSlot): BlockSlot = diff --git a/beacon_chain/consensus_object_pools/block_pools_types.nim b/beacon_chain/consensus_object_pools/block_pools_types.nim index a3fd6c820..bdc8c32dc 100644 --- a/beacon_chain/consensus_object_pools/block_pools_types.nim +++ b/beacon_chain/consensus_object_pools/block_pools_types.nim @@ -240,9 +240,6 @@ type ## EPOCHS_PER_SYNC_COMMITTEE_PERIOD is happening, some valid sync ## committee messages will be rejected - optimisticRoots*: HashSet[Eth2Digest] - ## https://github.com/ethereum/consensus-specs/blob/v1.3.0/sync/optimistic.md#helpers - EpochKey* = object ## The epoch key fully determines the shuffling for proposers and ## committees in a beacon state - the epoch level information in the state diff --git a/beacon_chain/consensus_object_pools/blockchain_dag.nim b/beacon_chain/consensus_object_pools/blockchain_dag.nim index 588c2080f..99aaf7cd5 100644 --- a/beacon_chain/consensus_object_pools/blockchain_dag.nim +++ b/beacon_chain/consensus_object_pools/blockchain_dag.nim @@ -1015,9 +1015,13 @@ proc init*(T: type ChainDAGRef, cfg: RuntimeConfig, db: BeaconChainDB, # Load head -> finalized, or all summaries in case the finalized block table # hasn't been written yet for blck in db.getAncestorSummaries(head.root): - # The execution block root gets filled in as needed - let newRef = - BlockRef.init(blck.root, Opt.none Eth2Digest, blck.summary.slot) + # The execution block root gets filled in as needed. Nonfinalized Bellatrix + # and later blocks are loaded as optimistic, which gets adjusted that first + # `VALID` fcU from an EL plus markBlockVerified. Pre-merge blocks still get + # marked as `VALID`. + let newRef = BlockRef.init( + blck.root, Opt.none Eth2Digest, executionValid = false, + blck.summary.slot) if headRef == nil: headRef = newRef @@ -1253,17 +1257,6 @@ proc init*(T: type ChainDAGRef, cfg: RuntimeConfig, db: BeaconChainDB, dag.initLightClientDataCache() - # If these aren't actually optimistic, the first fcU will resolve that - withState(dag.headState): - when consensusFork >= ConsensusFork.Bellatrix: - template executionPayloadHeader(): auto = - forkyState().data.latest_execution_payload_header - const emptyExecutionPayloadHeader = - default(type(executionPayloadHeader)) - if executionPayloadHeader != emptyExecutionPayloadHeader: - dag.optimisticRoots.incl dag.head.root - dag.optimisticRoots.incl dag.finalizedHead.blck.root - dag template genesis_validators_root*(dag: ChainDAGRef): Eth2Digest = @@ -1918,7 +1911,7 @@ proc pruneBlockSlot(dag: ChainDAGRef, bs: BlockSlot) = # Update light client data dag.deleteLightClientData(bs.blck.bid) - dag.optimisticRoots.excl bs.blck.root + bs.blck.executionValid = true dag.forkBlocks.excl(KeyedBlockRef.init(bs.blck)) discard dag.db.delBlock( dag.cfg.consensusForkAtEpoch(bs.blck.slot.epoch), bs.blck.root) @@ -1969,34 +1962,32 @@ proc pruneBlocksDAG(dag: ChainDAGRef) = # https://github.com/ethereum/consensus-specs/blob/v1.3.0/sync/optimistic.md#helpers template is_optimistic*(dag: ChainDAGRef, root: Eth2Digest): bool = - root in dag.optimisticRoots + let blck = dag.getBlockRef(root) + if blck.isSome: + not blck.get.executionValid + else: + # Either it doesn't exist at all, or it's finalized + false -proc markBlockVerified*( - dag: ChainDAGRef, quarantine: var Quarantine, root: Eth2Digest) = - # Might be called when block was not optimistic to begin with, or had been - # but already had been marked verified. - if not dag.is_optimistic(root): - return - - var cur = dag.getBlockRef(root).valueOr: - return - logScope: blck = shortLog(cur) - - debug "markBlockVerified" +proc markBlockVerified*(dag: ChainDAGRef, blck: BlockRef) = + var cur = blck while true: - if not dag.is_optimistic(cur.bid.root): - return + cur.executionValid = true - dag.optimisticRoots.excl cur.bid.root - - debug "markBlockVerified ancestor" + debug "markBlockVerified", blck = shortLog(cur) if cur.parent.isNil: break cur = cur.parent + # Always check at least as far back as the parent so that when a new block + # is added with executionValid already set, it stil sets the ancestors, to + # the next valid in the chain. + if cur.executionValid: + return + iterator syncSubcommittee*( syncCommittee: openArray[ValidatorIndex], subcommitteeIdx: SyncSubcommitteeIndex): ValidatorIndex = @@ -2435,7 +2426,7 @@ proc updateHead*( justified = shortLog(getStateField( dag.headState, current_justified_checkpoint)), finalized = shortLog(getStateField(dag.headState, finalized_checkpoint)), - isOptHead = dag.is_optimistic(newHead.root) + isOptHead = not newHead.executionValid if not(isNil(dag.onReorgHappened)): let @@ -2457,7 +2448,7 @@ proc updateHead*( justified = shortLog(getStateField( dag.headState, current_justified_checkpoint)), finalized = shortLog(getStateField(dag.headState, finalized_checkpoint)), - isOptHead = dag.is_optimistic(newHead.root) + isOptHead = newHead.executionValid if not(isNil(dag.onHeadChanged)): let diff --git a/beacon_chain/consensus_object_pools/consensus_manager.nim b/beacon_chain/consensus_object_pools/consensus_manager.nim index 12d5c2c78..2a1fb0422 100644 --- a/beacon_chain/consensus_object_pools/consensus_manager.nim +++ b/beacon_chain/consensus_object_pools/consensus_manager.nim @@ -159,7 +159,7 @@ proc updateExecutionClientHead(self: ref ConsensusManager, if headExecutionPayloadHash.isZero: # Blocks without execution payloads can't be optimistic. - self.dag.markBlockVerified(self.quarantine[], newHead.blck.root) + self.dag.markBlockVerified(newHead.blck) return Opt[void].ok() template callForkchoiceUpdated(attributes: untyped): auto = @@ -184,13 +184,23 @@ proc updateExecutionClientHead(self: ref ConsensusManager, case payloadExecutionStatus of PayloadExecutionStatus.valid: - self.dag.markBlockVerified(self.quarantine[], newHead.blck.root) + self.dag.markBlockVerified(newHead.blck) of PayloadExecutionStatus.invalid, PayloadExecutionStatus.invalid_block_hash: self.attestationPool[].forkChoice.mark_root_invalid(newHead.blck.root) self.quarantine[].addUnviable(newHead.blck.root) return Opt.none(void) of PayloadExecutionStatus.accepted, PayloadExecutionStatus.syncing: - self.dag.optimisticRoots.incl newHead.blck.root + # Don't do anything. Either newHead.blck.executionValid was already false, + # in which case it'd be superfluous to set it to false again, or the block + # was marked as `VALID` in the `newPayload` path already, in which case it + # is fine to keep it as valid here. Conceptually, were this to be lines of + # code, it'd be something like + # if newHead.blck.executionValid: + # do nothing because of latter case + # else: + # do nothing because it's a no-op + # So, either way, do nothing. + discard return Opt[void].ok() @@ -241,7 +251,7 @@ proc updateHead*(self: var ConsensusManager, wallSlot: Slot) = if self.dag.loadExecutionBlockHash(newHead.blck).isZero: # Blocks without execution payloads can't be optimistic. - self.dag.markBlockVerified(self.quarantine[], newHead.blck.root) + self.dag.markBlockVerified(newHead.blck) self.updateHead(newHead.blck) @@ -259,7 +269,7 @@ func isSynced(dag: ChainDAGRef, wallSlot: Slot): bool = if dag.head.slot + defaultSyncHorizon < wallSlot: false else: - not dag.is_optimistic(dag.head.root) + dag.head.executionValid proc checkNextProposer( dag: ChainDAGRef, actionTracker: ActionTracker, diff --git a/beacon_chain/gossip_processing/block_processor.nim b/beacon_chain/gossip_processing/block_processor.nim index fc94e7b3d..91cb3c60a 100644 --- a/beacon_chain/gossip_processing/block_processor.nim +++ b/beacon_chain/gossip_processing/block_processor.nim @@ -20,7 +20,7 @@ from ../consensus_object_pools/consensus_manager import runProposalForkchoiceUpdated, shouldSyncOptimistically, updateHead, updateHeadWithExecution from ../consensus_object_pools/blockchain_dag import - getBlockRef, getProposer, forkAtEpoch, is_optimistic, loadExecutionBlockHash, + getBlockRef, getProposer, forkAtEpoch, loadExecutionBlockHash, markBlockVerified, validatorKey from ../beacon_clock import GetBeaconTimeFn, toFloatSeconds from ../consensus_object_pools/block_dag import BlockRef, root, shortLog, slot @@ -560,9 +560,6 @@ proc storeBlock*( # This reduces in-flight fcU spam, which both reduces EL load and decreases # otherwise somewhat unpredictable CL head movement. - if payloadValid: - dag.markBlockVerified(self.consensusManager.quarantine[], signedBlock.root) - # Grab the new head according to our latest attestation data; determines how # async this needs to be. let newHead = attestationPool[].selectOptimisticHead( @@ -618,10 +615,8 @@ proc storeBlock*( # Blocks without execution payloads can't be optimistic, and don't try # to fcU to a block the EL hasn't seen self.consensusManager[].updateHead(newHead.get.blck) - elif not dag.is_optimistic newHead.get.blck.root: - # Not `NOT_VALID`; either `VALID` or `INVALIDATED`, but latter wouldn't - # be selected as head, so `VALID`. `forkchoiceUpdated` necessary for EL - # client only. + elif newHead.get.blck.executionValid: + # `forkchoiceUpdated` necessary for EL client only. self.consensusManager[].updateHead(newHead.get.blck) if self.consensusManager.checkNextProposer(wallSlot).isNone: diff --git a/beacon_chain/nimbus_beacon_node.nim b/beacon_chain/nimbus_beacon_node.nim index 03df39405..0fa9f9654 100644 --- a/beacon_chain/nimbus_beacon_node.nim +++ b/beacon_chain/nimbus_beacon_node.nim @@ -1270,7 +1270,7 @@ proc onSlotEnd(node: BeaconNode, slot: Slot) {.async.} = await node.updateGossipStatus(slot + 1) func syncStatus(node: BeaconNode, wallSlot: Slot): string = - let optimistic_head = node.dag.is_optimistic(node.dag.head.root) + let optimistic_head = not node.dag.head.executionValid if node.syncManager.inProgress: let optimisticSuffix = diff --git a/beacon_chain/rpc/rest_debug_api.nim b/beacon_chain/rpc/rest_debug_api.nim index 804f2c671..c867bbe8f 100644 --- a/beacon_chain/rpc/rest_debug_api.nim +++ b/beacon_chain/rpc/rest_debug_api.nim @@ -76,7 +76,7 @@ proc installDebugApiHandlers*(router: var RestRouter, node: BeaconNode) = ( root: it.root, slot: it.slot, - execution_optimistic: node.getBlockRefOptimistic(it) + execution_optimistic: not it.executionValid ) ) ) diff --git a/beacon_chain/rpc/rest_node_api.nim b/beacon_chain/rpc/rest_node_api.nim index 7a9711f8f..f547db90f 100644 --- a/beacon_chain/rpc/rest_node_api.nim +++ b/beacon_chain/rpc/rest_node_api.nim @@ -1,3 +1,9 @@ +# Copyright (c) 2021-2023 Status Research & Development GmbH +# Licensed and distributed under either of +# * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT). +# * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). +# at your option. This file may not be copied, modified, or distributed except according to those terms. + import stew/[byteutils, results], chronicles, @@ -264,7 +270,7 @@ proc installNodeApiHandlers*(router: var RestRouter, node: BeaconNode) = node.syncManager.inProgress isOptimistic = if node.currentSlot().epoch() >= node.dag.cfg.BELLATRIX_FORK_EPOCH: - some(node.dag.is_optimistic(node.dag.head.root)) + some(not node.dag.head.executionValid) else: none[bool]() elOffline = diff --git a/beacon_chain/rpc/rest_utils.nim b/beacon_chain/rpc/rest_utils.nim index 5712288ec..2079baa55 100644 --- a/beacon_chain/rpc/rest_utils.nim +++ b/beacon_chain/rpc/rest_utils.nim @@ -315,14 +315,6 @@ proc getBlockOptimistic*(node: BeaconNode, else: none[bool]() -proc getBlockRefOptimistic*(node: BeaconNode, blck: BlockRef): bool = - let blck = node.dag.getForkedBlock(blck.bid).get() - case blck.kind - of ConsensusFork.Phase0, ConsensusFork.Altair: - false - of ConsensusFork.Bellatrix, ConsensusFork.Capella, ConsensusFork.Deneb: - node.dag.is_optimistic(blck.root) - const jsonMediaType* = MediaType.init("application/json") sszMediaType* = MediaType.init("application/octet-stream") diff --git a/beacon_chain/sync/sync_protocol.nim b/beacon_chain/sync/sync_protocol.nim index 10923e711..93b388f44 100644 --- a/beacon_chain/sync/sync_protocol.nim +++ b/beacon_chain/sync/sync_protocol.nim @@ -340,7 +340,7 @@ p2pProtocol BeaconSync(version = 1, # blocks all being optimistic and none of them being optimistic. The # EL catches up, tells the CL the head is verified, and that's it. if blocks[i].slot.epoch >= dag.cfg.BELLATRIX_FORK_EPOCH and - dag.is_optimistic(dag.head.root): + not dag.head.executionValid: continue let uncompressedLen = uncompressedLenFramed(bytes).valueOr: @@ -402,7 +402,7 @@ p2pProtocol BeaconSync(version = 1, # blocks all being optimistic and none of them being optimistic. The # EL catches up, tells the CL the head is verified, and that's it. if blockRef.slot.epoch >= dag.cfg.BELLATRIX_FORK_EPOCH and - dag.is_optimistic(dag.head.root): + not dag.head.executionValid: continue let uncompressedLen = uncompressedLenFramed(bytes).valueOr: @@ -526,7 +526,7 @@ p2pProtocol BeaconSync(version = 1, # blocks all being optimistic and none of them being optimistic. The # EL catches up, tells the CL the head is verified, and that's it. if blockIds[i].slot.epoch >= dag.cfg.BELLATRIX_FORK_EPOCH and - dag.is_optimistic(dag.head.root): + not dag.head.executionValid: continue let uncompressedLen = uncompressedLenFramed(bytes).valueOr: diff --git a/beacon_chain/validators/keystore_management.nim b/beacon_chain/validators/keystore_management.nim index 5a31f8469..bb0935e60 100644 --- a/beacon_chain/validators/keystore_management.nim +++ b/beacon_chain/validators/keystore_management.nim @@ -1416,7 +1416,7 @@ func getPerValidatorDefaultFeeRecipient*( proc getSuggestedFeeRecipient*( host: KeymanagerHost, pubkey: ValidatorPubKey, defaultFeeRecipient: Eth1Address): - Result[Eth1Address, ValidatorConfigFileStatus] {.deprecated.} = + Result[Eth1Address, ValidatorConfigFileStatus] = host.validatorsDir.getSuggestedFeeRecipient(pubkey, defaultFeeRecipient) proc getSuggestedFeeRecipient( diff --git a/beacon_chain/validators/validator_duties.nim b/beacon_chain/validators/validator_duties.nim index 352ff8a27..d41036032 100644 --- a/beacon_chain/validators/validator_duties.nim +++ b/beacon_chain/validators/validator_duties.nim @@ -164,7 +164,7 @@ proc isSynced*(node: BeaconNode, head: BlockRef): SyncStatus = head.slot + node.config.syncHorizon < wallSlot.slot: SyncStatus.unsynced else: - if node.dag.is_optimistic(head.root): + if not head.executionValid: SyncStatus.optimistic else: SyncStatus.synced