From 3bfff6f219ca5cfba471c24513f4bb4835cce08f Mon Sep 17 00:00:00 2001 From: tersec Date: Wed, 5 Apr 2023 13:35:32 +0000 Subject: [PATCH] query EL and builder relay for bids in parallel (#4749) * outline for comparing bids from builder and engine API in BN * set up async * decision scaffold * clean up logging * Refactor proposeBlockMEV * Update beacon_chain/validators/validator_duties.nim Co-authored-by: zah * Update beacon_chain/validators/validator_duties.nim Co-authored-by: zah * use typedescs instead of explicit generic parameters --------- Co-authored-by: zah --- beacon_chain/validators/validator_duties.nim | 243 +++++++++++-------- 1 file changed, 148 insertions(+), 95 deletions(-) diff --git a/beacon_chain/validators/validator_duties.nim b/beacon_chain/validators/validator_duties.nim index 7c0e7c8e0..cecf54f08 100644 --- a/beacon_chain/validators/validator_duties.nim +++ b/beacon_chain/validators/validator_duties.nim @@ -85,7 +85,7 @@ declarePublicGauge(attached_validator_balance_total, logScope: topics = "beacval" type - ForkedBlockResult* = Result[ForkedBeaconBlock, string] + ForkedBlockResult = Result[ForkedBeaconBlock, string] SyncStatus* {.pure.} = enum synced @@ -392,7 +392,7 @@ proc makeBeaconBlockForHeadAndSlot*( get_expected_withdrawals(forkyState.data)) if withdrawals_root.isNone or hash_tree_root(withdrawals) != withdrawals_root.get: - # TODO: Why don't we fallback to the EL payload here? + # If engine API returned a block, will use that return err("Builder relay provided incorrect withdrawals root") # Otherwise, the state transition function notices that there are # too few withdrawals. @@ -675,12 +675,12 @@ proc getBlindedBlockParts[EPH: ForkyExecutionPayloadHeader]( return ok((executionPayloadHeader.get, forkedBlck)) -proc proposeBlockMEV[ +proc getBuilderBid[ SBBB: bellatrix_mev.SignedBlindedBeaconBlock | capella_mev.SignedBlindedBeaconBlock]( node: BeaconNode, head: BlockRef, validator: AttachedValidator, slot: Slot, randao: ValidatorSig, validator_index: ValidatorIndex): - Future[Opt[BlockRef]] {.async.} = + Future[Result[SBBB, string]] {.async.} = # Used by the BN's own validators, but not the REST server when SBBB is bellatrix_mev.SignedBlindedBeaconBlock: type EPH = bellatrix.ExecutionPayloadHeader @@ -695,7 +695,7 @@ proc proposeBlockMEV[ if blindedBlockParts.isErr: # Not signed yet, fine to try to fall back on EL beacon_block_builder_missed_with_fallback.inc() - return Opt.none BlockRef + return err blindedBlockParts.error() # These, together, get combined into the blinded block for signing and # proposal through the relay network. @@ -730,23 +730,18 @@ proc proposeBlockMEV[ Result[SBBB, string].err("getBlindedBlock timed out") if blindedBlock.isErr: - info "proposeBlockMEV: getBlindedBeaconBlock failed", - slot, head = shortLog(head), validator_index, blindedBlock, - error = blindedBlock.error - return Opt.none BlockRef + return err blindedBlock.error() - # Before unblindAndRouteBlockMEV, can fall back to EL; after, cannot + return blindedBlock + +proc proposeBlockMEV(node: BeaconNode, blindedBlock: auto): + Future[Result[BlockRef, string]] {.async.} = let unblindedBlockRef = await node.unblindAndRouteBlockMEV( - blindedBlock.get) + blindedBlock.get) return if unblindedBlockRef.isOk and unblindedBlockRef.get.isSome: beacon_blocks_proposed.inc() - unblindedBlockRef.get + ok(unblindedBlockRef.get.get) else: - # Signal to the caller that a signed, blinded beacon block was sent to the - # builder API server, at which point no local EL fallback can occur. Using - # non-`none` opt with the same head indicates this to proposeBlock(), with - # any non-`none` return value indicating this in general. - # # unblindedBlockRef.isOk and unblindedBlockRef.get.isNone indicates that # the block failed to validate and integrate into the DAG, which for the # purpose of this return value, is equivalent. It's used to drive Beacon @@ -756,11 +751,7 @@ proc proposeBlockMEV[ unblindedBlockRef.error else: "Unblinded block failed either to validate or integrate into validated store" - warn "proposeBlockMEV: blinded block either not successfully unblinded or not successfully proposed", - head = shortLog(head), slot, validator_index, - validator = shortLog(validator), - err = errMsg, blindedBlck = shortLog(blindedBlock.get) - Opt.some head + err errMsg proc makeBlindedBeaconBlockForHeadAndSlot*[ BBB: bellatrix_mev.BlindedBeaconBlock | capella_mev.BlindedBeaconBlock]( @@ -820,85 +811,104 @@ proc makeBlindedBeaconBlockForHeadAndSlot*[ else: return err("Attempt to create pre-Bellatrix blinded block") -proc proposeBlock(node: BeaconNode, - validator: AttachedValidator, - validator_index: ValidatorIndex, - head: BlockRef, - slot: Slot): Future[BlockRef] {.async.} = - if head.slot >= slot: - # We should normally not have a head newer than the slot we're proposing for - # but this can happen if block proposal is delayed - warn "Skipping proposal, have newer head already", - headSlot = shortLog(head.slot), - headBlockRoot = shortLog(head.root), - slot = shortLog(slot) - return head - - let - fork = node.dag.forkAtEpoch(slot.epoch) - genesis_validators_root = node.dag.genesis_validators_root - randao = - block: - let res = await validator.getEpochSignature( - fork, genesis_validators_root, slot.epoch) - if res.isErr(): - warn "Unable to generate randao reveal", - validator = shortLog(validator), error_msg = res.error() - return head - res.get() - - if node.config.payloadBuilderEnable: - let failsafeInEffect = +proc proposeBlockAux( + SBBB: typedesc, EPS: typedesc, node: BeaconNode, + validator: AttachedValidator, validator_index: ValidatorIndex, + head: BlockRef, slot: Slot, randao: ValidatorSig, fork: Fork, + genesis_validators_root: Eth2Digest): Future[BlockRef] {.async.} = + # Collect bids + let usePayloadBuilder = + if node.config.payloadBuilderEnable: withState(node.dag.headState): # Head slot, not proposal slot, matters here - livenessFailsafeInEffect( + # TODO it might make some sense to allow use of builder API if local + # EL fails -- i.e. it would change priorities, so any block from the + # execution layer client would override builder API. But it seems an + # odd requirement to produce no block at all in those conditions. + not livenessFailsafeInEffect( forkyState.data.block_roots.data, forkyState.data.slot) - if not failsafeInEffect: - let newBlockMEV = - if slot.epoch >= node.dag.cfg.DENEB_FORK_EPOCH: - debugRaiseAssert $denebImplementationMissing & ": proposeBlock" - await proposeBlockMEV[ - capella_mev.SignedBlindedBeaconBlock]( - node, head, validator, slot, randao, validator_index) - elif slot.epoch >= node.dag.cfg.CAPELLA_FORK_EPOCH: - await proposeBlockMEV[ - capella_mev.SignedBlindedBeaconBlock]( - node, head, validator, slot, randao, validator_index) - else: - await proposeBlockMEV[ - bellatrix_mev.SignedBlindedBeaconBlock]( - node, head, validator, slot, randao, validator_index) - - if newBlockMEV.isSome: - # This might be equivalent to the `head` passed in, but it signals that - # `submitBlindedBlock` ran, so don't do anything else. Otherwise, it is - # fine to try again with the local EL. - if newBlockMEV.get == head: - # Returning same block as head indicates failure to generate new block - beacon_block_builder_missed_without_fallback.inc() - return newBlockMEV.get - - # TODO Compare the value of the MEV block and the execution block - # obtained from the EL below: - - let newBlock = - if slot.epoch >= node.dag.cfg.DENEB_FORK_EPOCH: - await makeBeaconBlockForHeadAndSlot( - deneb.ExecutionPayloadForSigning, - node, randao, validator_index, node.graffitiBytes, head, slot) - elif slot.epoch >= node.dag.cfg.CAPELLA_FORK_EPOCH: - await makeBeaconBlockForHeadAndSlot( - capella.ExecutionPayloadForSigning, - node, randao, validator_index, node.graffitiBytes, head, slot) else: - await makeBeaconBlockForHeadAndSlot( - bellatrix.ExecutionPayloadForSigning, - node, randao, validator_index, node.graffitiBytes, head, slot) + false - if newBlock.isErr(): - return head # already logged elsewhere! + let + payloadBuilderBidFut = + if usePayloadBuilder: + getBuilderBid[SBBB](node, head, validator, slot, randao, validator_index) + else: + let fut = newFuture[Result[SBBB, string]]("builder-bid") + fut.complete(Result[SBBB, string].err( + "either payload builder disabled or liveness failsafe active")) + fut + engineBlockFut = makeBeaconBlockForHeadAndSlot( + EPS, node, randao, validator_index, node.graffitiBytes, head, slot) - var forkedBlck = newBlock.get() + # getBuilderBid times out after BUILDER_PROPOSAL_DELAY_TOLERANCE, with 1 more + # second for remote validators. makeBeaconBlockForHeadAndSlot times out after + # 1 second. + await allFutures(payloadBuilderBidFut, engineBlockFut) + doAssert payloadBuilderBidFut.finished and engineBlockFut.finished + + let builderBidAvailable = + if payloadBuilderBidFut.completed: + if payloadBuilderBidFut.read().isOk: + true + elif usePayloadBuilder: + info "Payload builder error", + slot, head = shortLog(head), validator = shortLog(validator), + err = payloadBuilderBidFut.read().error() + false + else: + # Effectively the same case, but without the log message + false + else: + info "Payload builder bid future failed", + slot, head = shortLog(head), validator = shortLog(validator), + err = payloadBuilderBidFut.error.msg + false + + let engineBidAvailable = + if engineBlockFut.completed: + if engineBlockFut.read().isOk: + true + else: + info "Engine block building error", + slot, head = shortLog(head), validator = shortLog(validator), + err = payloadBuilderBidFut.read().error() + false + else: + info "Engine block building failed", + slot, head = shortLog(head), validator = shortLog(validator), + err = engineBlockFut.error.msg + false + + let useBuilderBlock = + if builderBidAvailable: + if engineBidAvailable: + true + else: + true + else: + if not engineBidAvailable: + return head # errors logged in router + false + + if useBuilderBlock: + let + blindedBlock = payloadBuilderBidFut.read() + # Before proposeBlockMEV, can fall back to EL; after, cannot without + # risking slashing. + maybeUnblindedBlock = await proposeBlockMEV(node, blindedBlock) + + return maybeUnblindedBlock.valueOr: + warn "Blinded block proposal incomplete", + head = shortLog(head), slot, validator_index, + validator = shortLog(validator), + err = maybeUnblindedBlock.error, + blindedBlck = shortLog(blindedBlock.get) + beacon_block_builder_missed_without_fallback.inc() + return head + + var forkedBlck = engineBlockFut.read().get withBlck(forkedBlck): var blobs_sidecar = deneb.BlobsSidecar( @@ -974,6 +984,49 @@ proc proposeBlock(node: BeaconNode, return newBlockRef.get() +proc proposeBlock(node: BeaconNode, + validator: AttachedValidator, + validator_index: ValidatorIndex, + head: BlockRef, + slot: Slot): Future[BlockRef] {.async.} = + if head.slot >= slot: + # We should normally not have a head newer than the slot we're proposing for + # but this can happen if block proposal is delayed + warn "Skipping proposal, have newer head already", + headSlot = shortLog(head.slot), + headBlockRoot = shortLog(head.root), + slot = shortLog(slot) + return head + + let + fork = node.dag.forkAtEpoch(slot.epoch) + genesis_validators_root = node.dag.genesis_validators_root + randao = block: + let res = await validator.getEpochSignature( + fork, genesis_validators_root, slot.epoch) + if res.isErr(): + warn "Unable to generate randao reveal", + validator = shortLog(validator), error_msg = res.error() + return head + res.get() + + template proposeBlockContinuation(type1, type2: untyped): auto = + await proposeBlockAux( + type1, type2, node, validator, validator_index, head, slot, randao, fork, + genesis_validators_root) + + return + if slot.epoch >= node.dag.cfg.DENEB_FORK_EPOCH: + debugRaiseAssert $denebImplementationMissing & ": proposeBlock" + proposeBlockContinuation( + capella_mev.SignedBlindedBeaconBlock, deneb.ExecutionPayloadForSigning) + elif slot.epoch >= node.dag.cfg.CAPELLA_FORK_EPOCH: + proposeBlockContinuation( + capella_mev.SignedBlindedBeaconBlock, capella.ExecutionPayloadForSigning) + else: + proposeBlockContinuation( + bellatrix_mev.SignedBlindedBeaconBlock, bellatrix.ExecutionPayloadForSigning) + proc handleAttestations(node: BeaconNode, head: BlockRef, slot: Slot) = ## Perform all attestations that the validators attached to this node should ## perform during the given slot