replace optimisticRoots table with field in BlockRef (#4969)

* replace optimisticRoots table with field in BlockRef

* copyright year

* mark finalized blocks as verified on load

* Update beacon_chain/consensus_object_pools/block_dag.nim

Co-authored-by: Etan Kissling <etan@status.im>

* expand non-optimistic block checking to all pre-merge blocks; refactor markBlockVerified to use BlockRef rather than block root and remove superfluous caller in newPayload path replaced by addResolvedHeadBlock BlockRef construction

* don't treat finalized block specially; VALID status is sticky

---------

Co-authored-by: Etan Kissling <etan@status.im>
This commit is contained in:
tersec 2023-05-20 12:18:51 +00:00 committed by GitHub
parent 0044c18c01
commit cd087b9a43
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 71 additions and 76 deletions

View File

@ -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:

View File

@ -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 =

View File

@ -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

View File

@ -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

View File

@ -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,

View File

@ -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:

View File

@ -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 =

View File

@ -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
)
)
)

View File

@ -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 =

View File

@ -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")

View File

@ -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:

View File

@ -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(

View File

@ -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