From 85416744988fe5dc6c13473b1d8071f4452f5227 Mon Sep 17 00:00:00 2001 From: tersec Date: Mon, 6 Mar 2023 16:19:15 +0000 Subject: [PATCH] simplify ELMonitor fcU payload attributes handling (#4696) --- beacon_chain/beacon_node_light_client.nim | 3 +- .../consensus_manager.nim | 59 +++---- beacon_chain/eth1/eth1_monitor.nim | 165 ++++++++---------- .../gossip_processing/block_processor.nim | 11 +- beacon_chain/nimbus_light_client.nim | 5 +- 5 files changed, 117 insertions(+), 126 deletions(-) diff --git a/beacon_chain/beacon_node_light_client.nim b/beacon_chain/beacon_node_light_client.nim index 4107845d1..b0b143ffc 100644 --- a/beacon_chain/beacon_node_light_client.nim +++ b/beacon_chain/beacon_node_light_client.nim @@ -67,7 +67,8 @@ proc initLightClient*( discard await node.elManager.forkchoiceUpdated( headBlockHash = payload.block_hash, safeBlockHash = beaconHead.safeExecutionPayloadHash, - finalizedBlockHash = beaconHead.finalizedExecutionPayloadHash) + finalizedBlockHash = beaconHead.finalizedExecutionPayloadHash, + payloadAttributes = NoPayloadAttributes) else: discard optimisticProcessor = initOptimisticProcessor( diff --git a/beacon_chain/consensus_object_pools/consensus_manager.nim b/beacon_chain/consensus_object_pools/consensus_manager.nim index dae559fd7..c32335679 100644 --- a/beacon_chain/consensus_object_pools/consensus_manager.nim +++ b/beacon_chain/consensus_object_pools/consensus_manager.nim @@ -112,10 +112,9 @@ proc expectBlock*(self: var ConsensusManager, expectedSlot: Slot): Future[bool] return fut -from eth/async_utils import awaitWithTimeout from web3/engine_api_types import - ForkchoiceUpdatedResponse, - PayloadExecutionStatus, PayloadStatusV1, PayloadAttributesV1 + ForkchoiceUpdatedResponse, PayloadExecutionStatus, PayloadStatusV1, + PayloadAttributesV1 func `$`(h: BlockHash): string = $h.asEth2Digest @@ -168,9 +167,10 @@ proc updateExecutionClientHead(self: ref ConsensusManager, # Can't use dag.head here because it hasn't been updated yet let (payloadExecutionStatus, latestValidHash) = await self.elManager.forkchoiceUpdated( - headExecutionPayloadHash, - newHead.safeExecutionPayloadHash, - newHead.finalizedExecutionPayloadHash) + headBlockHash = headExecutionPayloadHash, + safeBlockHash = newHead.safeExecutionPayloadHash, + finalizedBlockHash = newHead.finalizedExecutionPayloadHash, + payloadAttributes = NoPayloadAttributes) case payloadExecutionStatus of PayloadExecutionStatus.valid: @@ -340,32 +340,29 @@ proc runProposalForkchoiceUpdated*( if headBlockHash.isZero: return - let - payloadAttributes = withState(self.dag.headState): - when stateFork >= ConsensusFork.Capella: - ForkedPayloadAttributes( - kind: ForkedPayloadAttributesKind.v2, - v2: PayloadAttributesV2( - timestamp: Quantity timestamp, - prevRandao: FixedBytes[32] randomData, - suggestedFeeRecipient: feeRecipient, - withdrawals: toEngineWithdrawals get_expected_withdrawals(forkyState.data))) - else: - ForkedPayloadAttributes( - kind: ForkedPayloadAttributesKind.v1, - v1: PayloadAttributesV1( - timestamp: Quantity timestamp, - prevRandao: FixedBytes[32] randomData, - suggestedFeeRecipient: feeRecipient)) try: - let - safeBlockHash = beaconHead.safeExecutionPayloadHash - (status, _) = await self.elManager.forkchoiceUpdated( - headBlockHash, - safeBlockHash, - beaconHead.finalizedExecutionPayloadHash, - payloadAttributes = payloadAttributes) - debug "Fork-choice updated for proposal", status + let safeBlockHash = beaconHead.safeExecutionPayloadHash + + withState(self.dag.headState): + template callForkchoiceUpdated(fcPayloadAttributes: auto) = + let (status, _) = await self.elManager.forkchoiceUpdated( + headBlockHash, safeBlockHash, + beaconHead.finalizedExecutionPayloadHash, + payloadAttributes = fcPayloadAttributes) + debug "Fork-choice updated for proposal", status + + static: doAssert high(ConsensusFork) == ConsensusFork.Deneb + when stateFork >= ConsensusFork.Capella: + callForkchoiceUpdated(PayloadAttributesV2( + timestamp: Quantity timestamp, + prevRandao: FixedBytes[32] randomData, + suggestedFeeRecipient: feeRecipient, + withdrawals: toEngineWithdrawals get_expected_withdrawals(forkyState.data))) + else: + callForkchoiceUpdated(PayloadAttributesV1( + timestamp: Quantity timestamp, + prevRandao: FixedBytes[32] randomData, + suggestedFeeRecipient: feeRecipient)) except CatchableError as err: error "Engine API fork-choice update failed", err = err.msg diff --git a/beacon_chain/eth1/eth1_monitor.nim b/beacon_chain/eth1/eth1_monitor.nim index 385c4e73a..25b1e594b 100644 --- a/beacon_chain/eth1/eth1_monitor.nim +++ b/beacon_chain/eth1/eth1_monitor.nim @@ -112,22 +112,15 @@ type hasConsensusViolation: bool ## The local chain contradicts the observed consensus on the network - ForkedPayloadAttributesKind* {.pure.} = enum - v1 - v2 - - ForkedPayloadAttributes* = ref object - case kind*: ForkedPayloadAttributesKind - of ForkedPayloadAttributesKind.v1: - v1*: PayloadAttributesV1 - of ForkedPayloadAttributesKind.v2: - v2*: PayloadAttributesV2 + NoPayloadAttributesType = object + ## A type with exactly one value, and which is not constructed via a `nil` + ## value for a ref object, which which Nim 1.6 crashes with an ICE. NextExpectedPayloadParams* = object headBlockHash*: Eth2Digest safeBlockHash*: Eth2Digest finalizedBlockHash*: Eth2Digest - payloadAttributes: ForkedPayloadAttributes + payloadAttributes*: PayloadAttributesV2 ELManager* = ref object eth1Network: Option[Eth1Network] @@ -241,6 +234,9 @@ type GetPayloadV2Response | CancunExecutionPayloadAndBlobs +const + NoPayloadAttributes* = default(NoPayloadAttributesType) + declareCounter failed_web3_requests, "Failed web3 requests" @@ -616,10 +612,10 @@ proc close(connection: ELConnection): Future[void] {.async.} = awaitWithTimeout(connection.web3.get.close(), 30.seconds): debug "Failed to close data provider in time" -proc isConnected(connection: ELConnection): bool = +func isConnected(connection: ELConnection): bool = connection.web3.isSome -proc getJsonRpcRequestHeaders(jwtSecret: Option[seq[byte]]): +func getJsonRpcRequestHeaders(jwtSecret: Option[seq[byte]]): auto = if jwtSecret.isSome: let secret = jwtSecret.get @@ -694,65 +690,29 @@ func areSameAs(expectedParams: Option[NextExpectedPayloadParams], randomData: Eth2Digest, feeRecipient: Eth1Address, withdrawals: seq[WithdrawalV1]): bool = - if not(expectedParams.isSome and - expectedParams.get.headBlockHash == latestHead and - expectedParams.get.safeBlockHash == latestSafe and - expectedParams.get.finalizedBlockHash == latestFinalized): - return false - - if expectedParams.get.payloadAttributes == nil: - return false - - case expectedParams.get.payloadAttributes.kind - of ForkedPayloadAttributesKind.v1: - expectedParams.get.payloadAttributes.v1.timestamp.uint64 == timestamp and - expectedParams.get.payloadAttributes.v1.prevRandao.bytes == randomData.data and - expectedParams.get.payloadAttributes.v1.suggestedFeeRecipient == feeRecipient and - withdrawals.len == 0 - of ForkedPayloadAttributesKind.v2: - expectedParams.get.payloadAttributes.v2.timestamp.uint64 == timestamp and - expectedParams.get.payloadAttributes.v2.prevRandao.bytes == randomData.data and - expectedParams.get.payloadAttributes.v2.suggestedFeeRecipient == feeRecipient and - expectedParams.get.payloadAttributes.v2.withdrawals == withdrawals - -template makeForkedPayloadAttributes( - GetPayloadResponseType: type BellatrixExecutionPayloadWithValue, - timestamp: uint64, - randomData: Eth2Digest, - suggestedFeeRecipient: Eth1Address, - withdrawals: seq[WithdrawalV1]): ForkedPayloadAttributes = - ForkedPayloadAttributes( - kind: ForkedPayloadAttributesKind.v1, - v1: engine_api.PayloadAttributesV1( - timestamp: Quantity timestamp, - prevRandao: FixedBytes[32] randomData.data, - suggestedFeeRecipient: suggestedFeeRecipient)) - -template makeForkedPayloadAttributes( - GetPayloadResponseType: typedesc[engine_api.GetPayloadV2Response|CancunExecutionPayloadAndBlobs], - timestamp: uint64, - randomData: Eth2Digest, - suggestedFeeRecipient: Eth1Address, - withdrawals: seq[WithdrawalV1]): ForkedPayloadAttributes = - ForkedPayloadAttributes( - kind: ForkedPayloadAttributesKind.v2, - v2: engine_api.PayloadAttributesV2( - timestamp: Quantity timestamp, - prevRandao: FixedBytes[32] randomData.data, - suggestedFeeRecipient: suggestedFeeRecipient, - withdrawals: withdrawals)) + expectedParams.isSome and + expectedParams.get.headBlockHash == latestHead and + expectedParams.get.safeBlockHash == latestSafe and + expectedParams.get.finalizedBlockHash == latestFinalized and + expectedParams.get.payloadAttributes.timestamp.uint64 == timestamp and + expectedParams.get.payloadAttributes.prevRandao.bytes == randomData.data and + expectedParams.get.payloadAttributes.suggestedFeeRecipient == feeRecipient and + expectedParams.get.payloadAttributes.withdrawals == withdrawals proc forkchoiceUpdated(rpcClient: RpcClient, state: ForkchoiceStateV1, - payloadAttributes: ForkedPayloadAttributes): Future[ForkchoiceUpdatedResponse] = - if payloadAttributes == nil: + payloadAttributes: PayloadAttributesV1 | + PayloadAttributesV2 | + NoPayloadAttributesType): + Future[ForkchoiceUpdatedResponse] = + when payloadAttributes is NoPayloadAttributesType: rpcClient.engine_forkchoiceUpdatedV1(state, none PayloadAttributesV1) + elif payloadAttributes is PayloadAttributesV1: + rpcClient.engine_forkchoiceUpdatedV1(state, some payloadAttributes) + elif payloadAttributes is PayloadAttributesV2: + rpcClient.engine_forkchoiceUpdatedV2(state, some payloadAttributes) else: - case payloadAttributes.kind - of ForkedPayloadAttributesKind.v1: - rpcClient.engine_forkchoiceUpdatedV1(state, some payloadAttributes.v1) - of ForkedPayloadAttributesKind.v2: - rpcClient.engine_forkchoiceUpdatedV2(state, some payloadAttributes.v2) + static: doAssert false func computeBlockValue(blk: ExecutionPayloadV1): UInt256 {.raises: [RlpError, Defect].} = for transactionBytes in blk.transactions: @@ -777,17 +737,29 @@ proc getPayloadFromSingleEL( elif not headBlock.isZero: engine_api_last_minute_forkchoice_updates_sent.inc(1, [connection.engineUrl.url]) - let response = await rpcClient.forkchoiceUpdated( - ForkchoiceStateV1( - headBlockHash: headBlock.asBlockHash, - safeBlockHash: safeBlock.asBlockHash, - finalizedBlockHash: finalizedBlock.asBlockHash), - makeForkedPayloadAttributes( - GetPayloadResponseType, - timestamp, - randomData, - suggestedFeeRecipient, - withdrawals)) + when GetPayloadResponseType is BellatrixExecutionPayloadWithValue: + let response = await rpcClient.forkchoiceUpdated( + ForkchoiceStateV1( + headBlockHash: headBlock.asBlockHash, + safeBlockHash: safeBlock.asBlockHash, + finalizedBlockHash: finalizedBlock.asBlockHash), + PayloadAttributesV1( + timestamp: Quantity timestamp, + prevRandao: FixedBytes[32] randomData.data, + suggestedFeeRecipient: suggestedFeeRecipient)) + elif GetPayloadResponseType is engine_api.GetPayloadV2Response or GetPayloadResponseType is CancunExecutionPayloadAndBlobs: + let response = await rpcClient.forkchoiceUpdated( + ForkchoiceStateV1( + headBlockHash: headBlock.asBlockHash, + safeBlockHash: safeBlock.asBlockHash, + finalizedBlockHash: finalizedBlock.asBlockHash), + PayloadAttributesV2( + timestamp: Quantity timestamp, + prevRandao: FixedBytes[32] randomData.data, + suggestedFeeRecipient: suggestedFeeRecipient, + withdrawals: withdrawals)) + else: + static: doAssert false if response.payloadStatus.status != PayloadExecutionStatus.valid or response.payloadId.isNone: @@ -820,7 +792,7 @@ proc getPayloadFromSingleEL( else: return await engine_api.getPayload(rpcClient, GetPayloadResponseType, payloadId) -proc cmpGetPayloadResponses(lhs, rhs: SomeEnginePayloadWithValue): int = +func cmpGetPayloadResponses(lhs, rhs: SomeEnginePayloadWithValue): int = cmp(distinctBase lhs.blockValue, distinctBase rhs.blockValue) template EngineApiResponseType*(T: type bellatrix.ExecutionPayloadForSigning): type = @@ -960,7 +932,7 @@ proc waitELToSyncDeposits(connection: ELConnection, await sleepAsync(seconds(30)) rpcClient = await connection.connectedRpcClient() -proc networkHasDepositContract(m: ELManager): bool = +func networkHasDepositContract(m: ELManager): bool = not m.cfg.DEPOSIT_CONTRACT_ADDRESS.isDefaultValue func mostRecentKnownBlock(m: ELManager): BlockHash = @@ -1039,7 +1011,7 @@ type oldStatusIsOk disagreement -proc compareStatuses(prevStatus, newStatus: PayloadExecutionStatus): StatusRelation = +func compareStatuses(prevStatus, newStatus: PayloadExecutionStatus): StatusRelation = case prevStatus of PayloadExecutionStatus.syncing: if newStatus == PayloadExecutionStatus.syncing: @@ -1089,7 +1061,7 @@ type selectedResponse: Option[int] disagreementAlreadyDetected: bool -proc init(T: type ELConsensusViolationDetector): T = +func init(T: type ELConsensusViolationDetector): T = ELConsensusViolationDetector(selectedResponse: none int, disagreementAlreadyDetected: false) @@ -1168,7 +1140,8 @@ proc sendNewPayload*(m: ELManager, proc forkchoiceUpdatedForSingleEL( connection: ELConnection, state: ref ForkchoiceStateV1, - payloadAttributes: ForkedPayloadAttributes): + payloadAttributes: PayloadAttributesV1 | PayloadAttributesV2 | + NoPayloadAttributesType): Future[PayloadStatusV1] {.async.} = let rpcClient = await connection.connectedRpcClient() @@ -1187,7 +1160,8 @@ proc forkchoiceUpdatedForSingleEL( proc forkchoiceUpdated*(m: ELManager, headBlockHash, safeBlockHash, finalizedBlockHash: Eth2Digest, - payloadAttributes: ForkedPayloadAttributes = nil): + payloadAttributes: PayloadAttributesV1 | PayloadAttributesV2 | + NoPayloadAttributesType): Future[(PayloadExecutionStatus, Option[BlockHash])] {.async.} = doAssert not headBlockHash.isZero @@ -1205,11 +1179,26 @@ proc forkchoiceUpdated*(m: ELManager, if m.elConnections.len == 0: return (PayloadExecutionStatus.syncing, none BlockHash) + when payloadAttributes is PayloadAttributesV2: + template payloadAttributesV2(): auto = payloadAttributes + elif payloadAttributes is PayloadAttributesV1: + template payloadAttributesV2(): auto = PayloadAttributesV2( + timestamp: payloadAttributes.timestamp, + prevRandao: payloadAttributes.prevRandao, + suggestedFeeRecipient: payloadAttributes.suggestedFeeRecipient, + withdrawals: @[]) + elif payloadAttributes is NoPayloadAttributesType: + template payloadAttributesV2(): auto = + # Because timestamp and prevRandao are both 0, won't false-positive match + (static(default(PayloadAttributesV2))) + else: + static: doAssert false + m.nextExpectedPayloadParams = some NextExpectedPayloadParams( headBlockHash: headBlockHash, safeBlockHash: safeBlockHash, finalizedBlockHash: finalizedBlockHash, - payloadAttributes: payloadAttributes) + payloadAttributes: payloadAttributesV2) let state = newClone ForkchoiceStateV1( @@ -1258,12 +1247,12 @@ proc forkchoiceUpdated*(m: ELManager, proc forkchoiceUpdatedNoResult*(m: ELManager, headBlockHash, safeBlockHash, finalizedBlockHash: Eth2Digest, - payloadAttributes: ForkedPayloadAttributes = nil) {.async.} = + payloadAttributes: PayloadAttributesV1 | PayloadAttributesV2) {.async.} = discard await m.forkchoiceUpdated( headBlockHash, safeBlockHash, finalizedBlockHash, payloadAttributes) # TODO can't be defined within exchangeConfigWithSingleEL -proc `==`(x, y: Quantity): bool {.borrow, noSideEffect.} +func `==`(x, y: Quantity): bool {.borrow.} proc exchangeConfigWithSingleEL(m: ELManager, connection: ELConnection) {.async.} = let rpcClient = await connection.connectedRpcClient() @@ -1686,7 +1675,7 @@ template getBlockProposalData*(m: ELManager, getBlockProposalData( m.eth1Chain, state, finalizedEth1Data, finalizedStateDepositIndex) -proc new*(T: type ELConnection, +func new*(T: type ELConnection, engineUrl: EngineApiUrl): T = ELConnection( engineUrl: engineUrl, diff --git a/beacon_chain/gossip_processing/block_processor.nim b/beacon_chain/gossip_processing/block_processor.nim index 25327fa8f..ff7251857 100644 --- a/beacon_chain/gossip_processing/block_processor.nim +++ b/beacon_chain/gossip_processing/block_processor.nim @@ -213,7 +213,7 @@ proc storeBackfillBlock( from web3/engine_api_types import PayloadExecutionStatus, PayloadStatusV1 from ../eth1/eth1_monitor import - ELManager, asEngineExecutionPayload, sendNewPayload, + ELManager, NoPayloadAttributes, asEngineExecutionPayload, sendNewPayload, forkchoiceUpdated, forkchoiceUpdatedNoResult proc expectValidForkchoiceUpdated( @@ -222,7 +222,10 @@ proc expectValidForkchoiceUpdated( receivedBlock: ForkySignedBeaconBlock): Future[void] {.async.} = let (payloadExecutionStatus, _) = await elManager.forkchoiceUpdated( - headBlockHash, safeBlockHash, finalizedBlockHash) + headBlockHash = headBlockHash, + safeBlockHash = safeBlockHash, + finalizedBlockHash = finalizedBlockHash, + payloadAttributes = NoPayloadAttributes) receivedExecutionBlockHash = when typeof(receivedBlock).toFork >= ConsensusFork.Bellatrix: receivedBlock.message.body.execution_payload.block_hash @@ -260,7 +263,6 @@ from ../consensus_object_pools/spec_cache import get_attesting_indices from ../spec/datatypes/phase0 import TrustedSignedBeaconBlock from ../spec/datatypes/altair import SignedBeaconBlock -from eth/async_utils import awaitWithTimeout from ../spec/datatypes/bellatrix import ExecutionPayload, SignedBeaconBlock from ../spec/datatypes/capella import ExecutionPayload, SignedBeaconBlock, asTrusted, shortLog @@ -511,7 +513,8 @@ proc storeBlock*( discard await elManager.forkchoiceUpdated( headBlockHash = self.consensusManager[].optimisticExecutionPayloadHash, safeBlockHash = newHead.get.safeExecutionPayloadHash, - finalizedBlockHash = newHead.get.finalizedExecutionPayloadHash) + finalizedBlockHash = newHead.get.finalizedExecutionPayloadHash, + payloadAttributes = NoPayloadAttributes) else: let headExecutionPayloadHash = diff --git a/beacon_chain/nimbus_light_client.nim b/beacon_chain/nimbus_light_client.nim index d16fb62b0..f71e2d5d8 100644 --- a/beacon_chain/nimbus_light_client.nim +++ b/beacon_chain/nimbus_light_client.nim @@ -7,7 +7,7 @@ import std/os, - chronicles, chronicles/chronos_tools, chronos, stew/io2, + chronicles, chronos, stew/io2, eth/db/kvstore_sqlite3, eth/keys, ./eth1/eth1_monitor, ./gossip_processing/optimistic_processor, @@ -114,7 +114,8 @@ programMain: discard await elManager.forkchoiceUpdated( headBlockHash = payload.block_hash, safeBlockHash = payload.block_hash, # stub value - finalizedBlockHash = ZERO_HASH) + finalizedBlockHash = ZERO_HASH, + payloadAttributes = NoPayloadAttributes) else: discard optimisticProcessor = initOptimisticProcessor( getBeaconTime, optimisticHandler)