From c63862cf88f66851c546b93abdfdee7e380f8106 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Fri, 10 Jan 2025 14:34:49 +0100 Subject: [PATCH] validate EL block hash when running consensus block tests (#6406) * validate EL block hash when running consensus block tests We currently don't have an easy way to test EL block hash computation. As the EL block hash in consensus-spec-tests is computed correctly, update the test runners that load block from test files to also verify the EL block hash. This increases missing test coverage. Requires https://github.com/ethereum/consensus-specs/pull/3829 * fix * resolve merge conflicts * fix genesis case, and deal with `incorrect_block_hash` test * add missing export marker * fix import * htr mutates underlying data, messing with differ, create copy in test * Handle payloads with empty tx (unsupported in ordered trie tool) * Update copyright years --------- Co-authored-by: tersec --- beacon_chain/spec/beaconstate.nim | 2 +- beacon_chain/spec/helpers.nim | 38 ++++++++++++------- .../bellatrix/test_fixture_operations.nim | 18 +++++++-- .../capella/test_fixture_operations.nim | 18 +++++++-- .../deneb/test_fixture_operations.nim | 18 +++++++-- .../electra/test_fixture_operations.nim | 18 +++++++-- tests/consensus_spec/fixtures_utils.nim | 26 +++++++++++-- .../test_fixture_fork_choice.nim | 7 +--- tests/consensus_spec/test_fixture_kzg.nim | 4 +- .../test_fixture_sanity_blocks.nim | 15 ++++---- .../test_fixture_transition.nim | 14 +++---- tests/testblockutil.nim | 4 +- 12 files changed, 124 insertions(+), 58 deletions(-) diff --git a/beacon_chain/spec/beaconstate.nim b/beacon_chain/spec/beaconstate.nim index 6c5c3ce07..89061a709 100644 --- a/beacon_chain/spec/beaconstate.nim +++ b/beacon_chain/spec/beaconstate.nim @@ -2408,7 +2408,7 @@ func upgrade_to_fulu*( post -func latest_block_root(state: ForkyBeaconState, state_root: Eth2Digest): +func latest_block_root*(state: ForkyBeaconState, state_root: Eth2Digest): Eth2Digest = # The root of the last block that was successfully applied to this state - # normally, when a block is applied, the data from the header is stored in diff --git a/beacon_chain/spec/helpers.nim b/beacon_chain/spec/helpers.nim index fdc4817b0..bf60fae9f 100644 --- a/beacon_chain/spec/helpers.nim +++ b/beacon_chain/spec/helpers.nim @@ -389,14 +389,17 @@ func is_merge_transition_complete*( state.latest_execution_payload_header != defaultExecutionPayloadHeader # https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.9/sync/optimistic.md#helpers -func is_execution_block*(blck: SomeForkyBeaconBlock): bool = - when typeof(blck).kind >= ConsensusFork.Bellatrix: +func is_execution_block*(body: SomeForkyBeaconBlockBody): bool = + when typeof(body).kind >= ConsensusFork.Bellatrix: const defaultExecutionPayload = - default(typeof(blck.body.execution_payload)) - blck.body.execution_payload != defaultExecutionPayload + default(typeof(body.execution_payload)) + body.execution_payload != defaultExecutionPayload else: false +func is_execution_block*(blck: SomeForkyBeaconBlock): bool = + blck.body.is_execution_block + # https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.8/specs/bellatrix/beacon-chain.md#is_merge_transition_block func is_merge_transition_block( state: bellatrix.BeaconState | capella.BeaconState | deneb.BeaconState | @@ -480,9 +483,10 @@ func computeRequestsHash( requestsHash.to(EthHash32) -proc blockToBlockHeader*(blck: ForkyBeaconBlock): EthHeader = - template payload: auto = blck.body.execution_payload - +proc toExecutionBlockHeader( + payload: ForkyExecutionPayload, + parentRoot: Eth2Digest, + requestsHash = Opt.none(EthHash32)): EthHeader = static: # `GasInt` is signed. We only use it for hashing. doAssert sizeof(GasInt) == sizeof(payload.gas_limit) doAssert sizeof(GasInt) == sizeof(payload.gas_used) @@ -506,12 +510,7 @@ proc blockToBlockHeader*(blck: ForkyBeaconBlock): EthHeader = Opt.none(uint64) parentBeaconBlockRoot = when typeof(payload).kind >= ConsensusFork.Deneb: - Opt.some EthHash32(blck.parent_root.data) - else: - Opt.none(EthHash32) - requestsHash = - when typeof(payload).kind >= ConsensusFork.Electra: - Opt.some blck.body.execution_requests.computeRequestsHash() + Opt.some EthHash32(parentRoot.data) else: Opt.none(EthHash32) @@ -538,8 +537,19 @@ proc blockToBlockHeader*(blck: ForkyBeaconBlock): EthHeader = parentBeaconBlockRoot : parentBeaconBlockRoot, # EIP-4788 requestsHash : requestsHash) # EIP-7685 +proc compute_execution_block_hash*( + body: ForkyBeaconBlockBody, + parentRoot: Eth2Digest): Eth2Digest = + when typeof(body).kind >= ConsensusFork.Electra: + body.execution_payload.toExecutionBlockHeader( + parentRoot, Opt.some body.execution_requests.computeRequestsHash()) + .rlpHash().to(Eth2Digest) + else: + body.execution_payload.toExecutionBlockHeader(parentRoot) + .rlpHash().to(Eth2Digest) + proc compute_execution_block_hash*(blck: ForkyBeaconBlock): Eth2Digest = - rlpHash(blockToBlockHeader(blck)).to(Eth2Digest) + blck.body.compute_execution_block_hash(blck.parent_root) from std/math import exp, ln from std/sequtils import foldl diff --git a/tests/consensus_spec/bellatrix/test_fixture_operations.nim b/tests/consensus_spec/bellatrix/test_fixture_operations.nim index ad307e3ff..b2cfb33ff 100644 --- a/tests/consensus_spec/bellatrix/test_fixture_operations.nim +++ b/tests/consensus_spec/bellatrix/test_fixture_operations.nim @@ -20,11 +20,11 @@ import ../fixtures_utils, ../os_ops, ../../helpers/debug_state -from std/sequtils import mapIt, toSeq +from std/sequtils import anyIt, mapIt, toSeq from std/strutils import contains from ../../../beacon_chain/spec/beaconstate import get_base_reward_per_increment, get_state_exit_queue_info, - get_total_active_balance, process_attestation + get_total_active_balance, latest_block_root, process_attestation const OpDir = SszTestsDir/const_preset/"bellatrix"/"operations" @@ -110,9 +110,12 @@ suite baseDescription & "Attester Slashing " & preset(): applyAttesterSlashing, path) suite baseDescription & "Block Header " & preset(): - func applyBlockHeader( + proc applyBlockHeader( preState: var bellatrix.BeaconState, blck: bellatrix.BeaconBlock): Result[void, cstring] = + if blck.is_execution_block: + check blck.body.execution_payload.block_hash == + blck.compute_execution_block_hash() var cache: StateCache process_block_header(preState, blck, {}, cache) @@ -144,6 +147,13 @@ suite baseDescription & "Execution Payload " & preset(): let payloadValid = os_ops.readFile( OpExecutionPayloadDir/"pyspec_tests"/path/"execution.yaml" ).contains("execution_valid: true") + if payloadValid and body.is_execution_block and + not body.execution_payload.transactions.anyIt(it.len == 0): + let expectedOk = (path != "incorrect_block_hash") + check expectedOk == (body.execution_payload.block_hash == + body.compute_execution_block_hash( + preState.latest_block_root( + assignClone(preState)[].hash_tree_root()))) func executePayload(_: bellatrix.ExecutionPayload): bool = payloadValid process_execution_payload( preState, body.execution_payload, executePayload) @@ -199,4 +209,4 @@ suite baseDescription & "Voluntary Exit " & preset(): for path in walkTests(OpVoluntaryExitDir): runTest[SignedVoluntaryExit, typeof applyVoluntaryExit]( OpVoluntaryExitDir, suiteName, "Voluntary Exit", "voluntary_exit", - applyVoluntaryExit, path) \ No newline at end of file + applyVoluntaryExit, path) diff --git a/tests/consensus_spec/capella/test_fixture_operations.nim b/tests/consensus_spec/capella/test_fixture_operations.nim index 39f2d4ad0..c5a0ec17e 100644 --- a/tests/consensus_spec/capella/test_fixture_operations.nim +++ b/tests/consensus_spec/capella/test_fixture_operations.nim @@ -20,11 +20,11 @@ import ../fixtures_utils, ../os_ops, ../../helpers/debug_state -from std/sequtils import mapIt, toSeq +from std/sequtils import anyIt, mapIt, toSeq from std/strutils import contains from ../../../beacon_chain/spec/beaconstate import get_base_reward_per_increment, get_state_exit_queue_info, - get_total_active_balance, process_attestation + get_total_active_balance, latest_block_root, process_attestation const OpDir = SszTestsDir/const_preset/"capella"/"operations" @@ -114,9 +114,12 @@ suite baseDescription & "Attester Slashing " & preset(): applyAttesterSlashing, path) suite baseDescription & "Block Header " & preset(): - func applyBlockHeader( + proc applyBlockHeader( preState: var capella.BeaconState, blck: capella.BeaconBlock): Result[void, cstring] = + if blck.is_execution_block: + check blck.body.execution_payload.block_hash == + blck.compute_execution_block_hash() var cache: StateCache process_block_header(preState, blck, {}, cache) @@ -161,6 +164,13 @@ suite baseDescription & "Execution Payload " & preset(): let payloadValid = os_ops.readFile( OpExecutionPayloadDir/"pyspec_tests"/path/"execution.yaml" ).contains("execution_valid: true") + if payloadValid and body.is_execution_block and + not body.execution_payload.transactions.anyIt(it.len == 0): + let expectedOk = (path != "incorrect_block_hash") + check expectedOk == (body.execution_payload.block_hash == + body.compute_execution_block_hash( + preState.latest_block_root( + assignClone(preState)[].hash_tree_root()))) func executePayload(_: capella.ExecutionPayload): bool = payloadValid process_execution_payload( preState, body.execution_payload, executePayload) @@ -227,4 +237,4 @@ suite baseDescription & "Withdrawals " & preset(): for path in walkTests(OpWithdrawalsDir): runTest[capella.ExecutionPayload, typeof applyWithdrawals]( OpWithdrawalsDir, suiteName, "Withdrawals", "execution_payload", - applyWithdrawals, path) \ No newline at end of file + applyWithdrawals, path) diff --git a/tests/consensus_spec/deneb/test_fixture_operations.nim b/tests/consensus_spec/deneb/test_fixture_operations.nim index e1c2d0a61..d2a58f015 100644 --- a/tests/consensus_spec/deneb/test_fixture_operations.nim +++ b/tests/consensus_spec/deneb/test_fixture_operations.nim @@ -20,11 +20,11 @@ import ../fixtures_utils, ../os_ops, ../../helpers/debug_state -from std/sequtils import mapIt, toSeq +from std/sequtils import anyIt, mapIt, toSeq from std/strutils import contains from ../../../beacon_chain/spec/beaconstate import get_base_reward_per_increment, get_state_exit_queue_info, - get_total_active_balance, process_attestation + get_total_active_balance, latest_block_root, process_attestation const OpDir = SszTestsDir/const_preset/"deneb"/"operations" @@ -114,9 +114,12 @@ suite baseDescription & "Attester Slashing " & preset(): applyAttesterSlashing, path) suite baseDescription & "Block Header " & preset(): - func applyBlockHeader( + proc applyBlockHeader( preState: var deneb.BeaconState, blck: deneb.BeaconBlock): Result[void, cstring] = + if blck.is_execution_block: + check blck.body.execution_payload.block_hash == + blck.compute_execution_block_hash() var cache: StateCache process_block_header(preState, blck, {}, cache) @@ -164,6 +167,13 @@ suite baseDescription & "Execution Payload " & preset(): let payloadValid = os_ops.readFile( OpExecutionPayloadDir/"pyspec_tests"/path/"execution.yaml" ).contains("execution_valid: true") + if payloadValid and body.is_execution_block and + not body.execution_payload.transactions.anyIt(it.len == 0): + let expectedOk = (path != "incorrect_block_hash") + check expectedOk == (body.execution_payload.block_hash == + body.compute_execution_block_hash( + preState.latest_block_root( + assignClone(preState)[].hash_tree_root()))) func executePayload(_: deneb.ExecutionPayload): bool = payloadValid process_execution_payload(preState, body, executePayload) @@ -229,4 +239,4 @@ suite baseDescription & "Withdrawals " & preset(): for path in walkTests(OpWithdrawalsDir): runTest[deneb.ExecutionPayload, typeof applyWithdrawals]( OpWithdrawalsDir, suiteName, "Withdrawals", "execution_payload", - applyWithdrawals, path) \ No newline at end of file + applyWithdrawals, path) diff --git a/tests/consensus_spec/electra/test_fixture_operations.nim b/tests/consensus_spec/electra/test_fixture_operations.nim index d16be9796..109dbf939 100644 --- a/tests/consensus_spec/electra/test_fixture_operations.nim +++ b/tests/consensus_spec/electra/test_fixture_operations.nim @@ -20,11 +20,11 @@ import ../fixtures_utils, ../os_ops, ../../helpers/debug_state -from std/sequtils import mapIt, toSeq +from std/sequtils import anyIt, mapIt, toSeq from std/strutils import contains from ../../../beacon_chain/spec/beaconstate import get_base_reward_per_increment, get_state_exit_queue_info, - get_total_active_balance, process_attestation + get_total_active_balance, latest_block_root, process_attestation const OpDir = SszTestsDir/const_preset/"electra"/"operations" @@ -121,9 +121,12 @@ suite baseDescription & "Attester Slashing " & preset(): applyAttesterSlashing, path) suite baseDescription & "Block Header " & preset(): - func applyBlockHeader( + proc applyBlockHeader( preState: var electra.BeaconState, blck: electra.BeaconBlock): Result[void, cstring] = + if blck.is_execution_block: + check blck.body.execution_payload.block_hash == + blck.compute_execution_block_hash() var cache: StateCache process_block_header(preState, blck, {}, cache) @@ -199,6 +202,13 @@ suite baseDescription & "Execution Payload " & preset(): let payloadValid = os_ops.readFile( OpExecutionPayloadDir/"pyspec_tests"/path/"execution.yaml" ).contains("execution_valid: true") + if payloadValid and body.is_execution_block and + not body.execution_payload.transactions.anyIt(it.len == 0): + let expectedOk = (path != "incorrect_block_hash") + check expectedOk == (body.execution_payload.block_hash == + body.compute_execution_block_hash( + preState.latest_block_root( + assignClone(preState)[].hash_tree_root()))) func executePayload(_: electra.ExecutionPayload): bool = payloadValid process_execution_payload( defaultRuntimeConfig, preState, body, executePayload) @@ -281,4 +291,4 @@ suite baseDescription & "Withdrawals " & preset(): for path in walkTests(OpWithdrawalsDir): runTest[electra.ExecutionPayload, typeof applyWithdrawals]( OpWithdrawalsDir, suiteName, "Withdrawals", "execution_payload", - applyWithdrawals, path) \ No newline at end of file + applyWithdrawals, path) diff --git a/tests/consensus_spec/fixtures_utils.nim b/tests/consensus_spec/fixtures_utils.nim index eb5e8632c..b28df613d 100644 --- a/tests/consensus_spec/fixtures_utils.nim +++ b/tests/consensus_spec/fixtures_utils.nim @@ -9,18 +9,18 @@ import # Standard library - std/[strutils, typetraits], + std/[sequtils, strutils, typetraits], # Internals ./os_ops, ../../beacon_chain/spec/datatypes/[phase0, altair, bellatrix], ../../beacon_chain/spec/[ - eth2_merkleization, eth2_ssz_serialization, forks], + eth2_merkleization, eth2_ssz_serialization, forks, helpers], # Status libs, snappy, stew/byteutils export - eth2_merkleization, eth2_ssz_serialization + eth2_merkleization, eth2_ssz_serialization, helpers # Process current EF test format # --------------------------------------------- @@ -173,4 +173,22 @@ proc loadForkedState*( withState(state[]): forkyState.data = parseTest(path, SSZ, consensusFork.BeaconState) forkyState.root = hash_tree_root(forkyState.data) - state \ No newline at end of file + state + +proc loadBlock*( + path: string, + consensusFork: static ConsensusFork, + validateBlockHash = true): auto = + var blck = parseTest(path, SSZ, consensusFork.SignedBeaconBlock) + blck.root = hash_tree_root(blck.message) + when consensusFork >= ConsensusFork.Bellatrix: + if blck.message.is_execution_block and + not blck.message.body.execution_payload.transactions.anyIt(it.len == 0): + if blck.message.body.execution_payload.block_hash != + blck.message.compute_execution_block_hash(): + try: + stderr.write "Invalid `block_hash`: ", path, "\n" + except IOError: + discard + quit 1 + blck diff --git a/tests/consensus_spec/test_fixture_fork_choice.nim b/tests/consensus_spec/test_fixture_fork_choice.nim index b735640c3..b93890165 100644 --- a/tests/consensus_spec/test_fixture_fork_choice.nim +++ b/tests/consensus_spec/test_fixture_fork_choice.nim @@ -1,5 +1,5 @@ # beacon_chain -# Copyright (c) 2018-2024 Status Research & Development GmbH +# Copyright (c) 2018-2025 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). @@ -119,10 +119,7 @@ proc loadOps( doAssert step.hasKey"blobs" == step.hasKey"proofs" withConsensusFork(fork): let - blck = parseTest( - path/filename & ".ssz_snappy", - SSZ, consensusFork.SignedBeaconBlock) - + blck = loadBlock(path/filename & ".ssz_snappy", consensusFork) blobData = when consensusFork >= ConsensusFork.Deneb: if step.hasKey"blobs": diff --git a/tests/consensus_spec/test_fixture_kzg.nim b/tests/consensus_spec/test_fixture_kzg.nim index ba18180e0..6d6ee2f4c 100644 --- a/tests/consensus_spec/test_fixture_kzg.nim +++ b/tests/consensus_spec/test_fixture_kzg.nim @@ -221,7 +221,7 @@ proc runComputeCellsAndKzgProofsTest(suiteName, suitePath, path: string) = check output.kind == JNull else: let p_val = p[].get - for i in 0..