From ad2db0b0f87b1be698ccbc3de0051636ecf8ff98 Mon Sep 17 00:00:00 2001 From: Advaita Saha Date: Mon, 13 Jan 2025 14:29:35 +0530 Subject: [PATCH 1/6] reorg handling --- nrpc/nrpc.nim | 42 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/nrpc/nrpc.nim b/nrpc/nrpc.nim index 816c91e12c..0c8d847220 100644 --- a/nrpc/nrpc.nim +++ b/nrpc/nrpc.nim @@ -22,6 +22,7 @@ import beacon_chain/el/el_manager, beacon_chain/el/engine_api_conversions, beacon_chain/spec/[forks, state_transition_block], + beacon_chain/spec/datatypes/bellatrix, beacon_chain/spec/eth2_apis/[rest_types, rest_beacon_calls], beacon_chain/networking/network_metadata, eth/async_utils @@ -45,7 +46,6 @@ template getCLBlockFromBeaconChain( var blck: ForkedSignedBeaconBlock if clBlock.isSome(): let blck = clBlock.get()[] - (blck, true) else: (blck, false) @@ -279,6 +279,12 @@ proc syncToEngineApi(conf: NRpcConf) {.async.} = info "newPayload Request sent", blockNumber = int(payload.blockNumber), response = payloadResponse.status + if payloadResponse.status != PayloadExecutionStatus.accepted: + error "Payload not accepted", + blockNumber = int(payload.blockNumber), + status = payloadResponse.status + quit(QuitFailure) + # Load the head hash from the execution payload, for forkchoice headHash = forkyBlck.message.body.execution_payload.block_hash @@ -303,9 +309,43 @@ proc syncToEngineApi(conf: NRpcConf) {.async.} = # Update the current block number from EL rest api # Shows that the fcu call has succeeded currentBlockNumber = elBlockNumber() + let oldHeadBlockNumber = headBlck.header.number (headBlck, _) = client.getELBlockFromBeaconChain(BlockIdent.init(BlockIdentType.Head), clConfig) + # Check for reorg + # No need to check for reorg if the EL head is behind the finalized block + if currentBlockNumber > finalizedBlck.header.number and oldHeadBlockNumber > headBlck.header.number: + warn "Head moved backwards : Possible reorg detected", + oldHead = oldHeadBlockNumber, + newHead = headBlck.header.number + + let (headClBlck, isAvailable) = client.getCLBlockFromBeaconChain( + BlockIdent.init(BlockIdentType.Head), clConfig + ) + + # move back the importedSlot to the finalized block + if isAvailable: + withBlck(headClBlck.asTrusted()): + when consensusFork >= ConsensusFork.Bellatrix: + importedSlot = forkyBlck.message.slot.uint64 + 1 + currentBlockNumber = forkyBlck.message.body.execution_payload.block_number + + # Load this head to the `headBlck` + if not getEthBlock(forkyBlck.message, headBlck): + error "Failed to get EL block from CL head" + quit(QuitFailure) + + (finalizedBlck, _) = client.getELBlockFromBeaconChain( + BlockIdent.init(BlockIdentType.Finalized), clConfig + ) + finalizedHash = finalizedBlck.header.blockHash.asEth2Digest + sendFCU(headClBlck) + else: + error "Failed to get CL head" + quit(QuitFailure) + + # fcU call for the last remaining payloads sendFCU(curBlck) From 0bf54a69ebd7a72a876b210fba14564d5b576828 Mon Sep 17 00:00:00 2001 From: Advaita Saha Date: Mon, 13 Jan 2025 15:04:53 +0530 Subject: [PATCH 2/6] formatting fix --- nrpc/nrpc.nim | 47 +++++++++++++++++++++++------------------------ 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/nrpc/nrpc.nim b/nrpc/nrpc.nim index 0c8d847220..01d3fc5595 100644 --- a/nrpc/nrpc.nim +++ b/nrpc/nrpc.nim @@ -1,5 +1,5 @@ # Nimbus -# Copyright (c) 2024 Status Research & Development GmbH +# Copyright (c) 2024-2025 Status Research & Development GmbH # Licensed under either of # * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE)) # * MIT license ([LICENSE-MIT](LICENSE-MIT)) @@ -199,11 +199,11 @@ proc syncToEngineApi(conf: NRpcConf) {.async.} = elif consensusFork == ConsensusFork.Capella: Opt.none(PayloadAttributesV2) elif consensusFork == ConsensusFork.Deneb or - consensusFork == ConsensusFork.Electra or - consensusFork == ConsensusFork.Fulu: + consensusFork == ConsensusFork.Electra or consensusFork == ConsensusFork.Fulu: Opt.none(PayloadAttributesV3) else: - static: doAssert(false, "Unsupported consensus fork") + static: + doAssert(false, "Unsupported consensus fork") Opt.none(PayloadAttributesV3) # Make the forkchoiceUpdated call based, after loading attributes based on the consensus fork @@ -249,18 +249,19 @@ proc syncToEngineApi(conf: NRpcConf) {.async.} = payload = payload, versionedHashes = versioned_hashes elif consensusFork == ConsensusFork.Electra or - consensusFork == ConsensusFork.Fulu: + consensusFork == ConsensusFork.Fulu: # Calculate the versioned hashes from the kzg commitments let versioned_hashes = mapIt( forkyBlck.message.body.blob_kzg_commitments, engine_api.VersionedHash(kzg_commitment_to_versioned_hash(it)), ) # Execution Requests for Electra - let execution_requests = @[ - SSZ.encode(forkyBlck.message.body.execution_requests.deposits), - SSZ.encode(forkyBlck.message.body.execution_requests.withdrawals), - SSZ.encode(forkyBlck.message.body.execution_requests.consolidations), - ] + let execution_requests = + @[ + SSZ.encode(forkyBlck.message.body.execution_requests.deposits), + SSZ.encode(forkyBlck.message.body.execution_requests.withdrawals), + SSZ.encode(forkyBlck.message.body.execution_requests.consolidations), + ] # TODO: Update to `newPayload()` once nim-web3 is updated payloadResponse = await rpcClient.engine_newPayloadV4( payload, @@ -274,15 +275,15 @@ proc syncToEngineApi(conf: NRpcConf) {.async.} = versionedHashes = versioned_hashes, executionRequests = execution_requests else: - static: doAssert(false, "Unsupported consensus fork") + static: + doAssert(false, "Unsupported consensus fork") info "newPayload Request sent", blockNumber = int(payload.blockNumber), response = payloadResponse.status if payloadResponse.status != PayloadExecutionStatus.accepted: - error "Payload not accepted", - blockNumber = int(payload.blockNumber), - status = payloadResponse.status + error "Payload not accepted", + blockNumber = int(payload.blockNumber), status = payloadResponse.status quit(QuitFailure) # Load the head hash from the execution payload, for forkchoice @@ -315,14 +316,13 @@ proc syncToEngineApi(conf: NRpcConf) {.async.} = # Check for reorg # No need to check for reorg if the EL head is behind the finalized block - if currentBlockNumber > finalizedBlck.header.number and oldHeadBlockNumber > headBlck.header.number: - warn "Head moved backwards : Possible reorg detected", - oldHead = oldHeadBlockNumber, - newHead = headBlck.header.number - - let (headClBlck, isAvailable) = client.getCLBlockFromBeaconChain( - BlockIdent.init(BlockIdentType.Head), clConfig - ) + if currentBlockNumber > finalizedBlck.header.number and + oldHeadBlockNumber > headBlck.header.number: + warn "Head moved backwards : Possible reorg detected", + oldHead = oldHeadBlockNumber, newHead = headBlck.header.number + + let (headClBlck, isAvailable) = + client.getCLBlockFromBeaconChain(BlockIdent.init(BlockIdentType.Head), clConfig) # move back the importedSlot to the finalized block if isAvailable: @@ -330,7 +330,7 @@ proc syncToEngineApi(conf: NRpcConf) {.async.} = when consensusFork >= ConsensusFork.Bellatrix: importedSlot = forkyBlck.message.slot.uint64 + 1 currentBlockNumber = forkyBlck.message.body.execution_payload.block_number - + # Load this head to the `headBlck` if not getEthBlock(forkyBlck.message, headBlck): error "Failed to get EL block from CL head" @@ -345,7 +345,6 @@ proc syncToEngineApi(conf: NRpcConf) {.async.} = error "Failed to get CL head" quit(QuitFailure) - # fcU call for the last remaining payloads sendFCU(curBlck) From a5910ca1e78b86e0a9d2f18022d304684573f161 Mon Sep 17 00:00:00 2001 From: Advaita Saha Date: Mon, 13 Jan 2025 15:26:22 +0530 Subject: [PATCH 3/6] fix validation condition --- nrpc/nrpc.nim | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nrpc/nrpc.nim b/nrpc/nrpc.nim index 01d3fc5595..e64c2e8534 100644 --- a/nrpc/nrpc.nim +++ b/nrpc/nrpc.nim @@ -281,8 +281,8 @@ proc syncToEngineApi(conf: NRpcConf) {.async.} = info "newPayload Request sent", blockNumber = int(payload.blockNumber), response = payloadResponse.status - if payloadResponse.status != PayloadExecutionStatus.accepted: - error "Payload not accepted", + if payloadResponse.status != PayloadExecutionStatus.valid: + error "Payload not validated", blockNumber = int(payload.blockNumber), status = payloadResponse.status quit(QuitFailure) From 3d12a919b2f457ade5fda1508376dc3d85a931aa Mon Sep 17 00:00:00 2001 From: Advaita Saha Date: Mon, 13 Jan 2025 17:03:07 +0530 Subject: [PATCH 4/6] eip-7685 compatible --- nrpc/nrpc.nim | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/nrpc/nrpc.nim b/nrpc/nrpc.nim index e64c2e8534..e8c420a92d 100644 --- a/nrpc/nrpc.nim +++ b/nrpc/nrpc.nim @@ -209,7 +209,12 @@ proc syncToEngineApi(conf: NRpcConf) {.async.} = # Make the forkchoiceUpdated call based, after loading attributes based on the consensus fork let fcuResponse = await rpcClient.forkchoiceUpdated(state, payloadAttributes) debug "forkchoiceUpdated", state = state, response = fcuResponse - info "forkchoiceUpdated Request sent", response = fcuResponse.payloadStatus.status + if fcuResponse.payloadStatus.status != PayloadExecutionStatus.valid: + error "Forkchoice not validated", status = fcuResponse.payloadStatus.status + quit(QuitFailure) + else: + info "forkchoiceUpdated Request sent", + response = fcuResponse.payloadStatus.status while running and currentBlockNumber < headBlck.header.number: var isAvailable = false @@ -250,19 +255,24 @@ proc syncToEngineApi(conf: NRpcConf) {.async.} = versionedHashes = versioned_hashes elif consensusFork == ConsensusFork.Electra or consensusFork == ConsensusFork.Fulu: - # Calculate the versioned hashes from the kzg commitments - let versioned_hashes = mapIt( - forkyBlck.message.body.blob_kzg_commitments, - engine_api.VersionedHash(kzg_commitment_to_versioned_hash(it)), - ) - # Execution Requests for Electra - let execution_requests = - @[ - SSZ.encode(forkyBlck.message.body.execution_requests.deposits), - SSZ.encode(forkyBlck.message.body.execution_requests.withdrawals), - SSZ.encode(forkyBlck.message.body.execution_requests.consolidations), - ] - # TODO: Update to `newPayload()` once nim-web3 is updated + let + # Calculate the versioned hashes from the kzg commitments + versioned_hashes = mapIt( + forkyBlck.message.body.blob_kzg_commitments, + engine_api.VersionedHash(kzg_commitment_to_versioned_hash(it)), + ) + # Execution Requests for Electra + execution_requests = block: + var requests: seq[seq[byte]] + for request_type, request_data in [ + SSZ.encode(forkyBlck.message.body.execution_requests.deposits), + SSZ.encode(forkyBlck.message.body.execution_requests.withdrawals), + SSZ.encode(forkyBlck.message.body.execution_requests.consolidations), + ]: + if request_data.len > 0: + requests.add @[request_type.byte] & request_data + requests + payloadResponse = await rpcClient.engine_newPayloadV4( payload, versioned_hashes, From b532bcf909b77e26a6eae9ef585f68a1be6a563c Mon Sep 17 00:00:00 2001 From: Advaita Saha Date: Mon, 13 Jan 2025 22:52:41 +0530 Subject: [PATCH 5/6] more exception handling --- nrpc/nrpc.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nrpc/nrpc.nim b/nrpc/nrpc.nim index e8c420a92d..ca2e80eead 100644 --- a/nrpc/nrpc.nim +++ b/nrpc/nrpc.nim @@ -148,7 +148,7 @@ proc syncToEngineApi(conf: NRpcConf) {.async.} = uint64(await rpcClient.eth_blockNumber()) except CatchableError as exc: error "Error getting block number", error = exc.msg - 0'u64 + quit(QuitFailure) # Load the EL state detials and create the beaconAPI client var From fa731c4ed069a3eac37a295fa15247ebb67dd083 Mon Sep 17 00:00:00 2001 From: Advaita Saha Date: Wed, 22 Jan 2025 22:22:45 +0530 Subject: [PATCH 6/6] remove strict checking --- nrpc/nrpc.nim | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/nrpc/nrpc.nim b/nrpc/nrpc.nim index ca2e80eead..82264c44b3 100644 --- a/nrpc/nrpc.nim +++ b/nrpc/nrpc.nim @@ -209,7 +209,8 @@ proc syncToEngineApi(conf: NRpcConf) {.async.} = # Make the forkchoiceUpdated call based, after loading attributes based on the consensus fork let fcuResponse = await rpcClient.forkchoiceUpdated(state, payloadAttributes) debug "forkchoiceUpdated", state = state, response = fcuResponse - if fcuResponse.payloadStatus.status != PayloadExecutionStatus.valid: + if fcuResponse.payloadStatus.status == PayloadExecutionStatus.invalid or + fcuResponse.payloadStatus.status == PayloadExecutionStatus.invalid_block_hash: error "Forkchoice not validated", status = fcuResponse.payloadStatus.status quit(QuitFailure) else: @@ -291,7 +292,8 @@ proc syncToEngineApi(conf: NRpcConf) {.async.} = info "newPayload Request sent", blockNumber = int(payload.blockNumber), response = payloadResponse.status - if payloadResponse.status != PayloadExecutionStatus.valid: + if payloadResponse.status == PayloadExecutionStatus.invalid or + payloadResponse.status == PayloadExecutionStatus.invalid_block_hash: error "Payload not validated", blockNumber = int(payload.blockNumber), status = payloadResponse.status quit(QuitFailure)