From 6958f67af3f477b5dc106aa2f267afb047c26205 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Sat, 4 Nov 2023 20:35:41 +0100 Subject: [PATCH 1/3] add test runner for v1.4.0-beta.4 Merkle proof tests (#5567) Create a test runner for validating the new `Merkle proof` Deneb tests that are added with `v1.4.0-beta.4` specs. --- .../consensus_spec_tests_preset.nim | 1 + .../test_fixture_merkle_proof.nim | 74 +++++++++++++++++++ 2 files changed, 75 insertions(+) create mode 100644 tests/consensus_spec/test_fixture_merkle_proof.nim diff --git a/tests/consensus_spec/consensus_spec_tests_preset.nim b/tests/consensus_spec/consensus_spec_tests_preset.nim index 24bd8bf01e..cc3e3785da 100644 --- a/tests/consensus_spec/consensus_spec_tests_preset.nim +++ b/tests/consensus_spec/consensus_spec_tests_preset.nim @@ -20,6 +20,7 @@ import ./test_fixture_light_client_single_merkle_proof, ./test_fixture_light_client_sync, ./test_fixture_light_client_update_ranking, + ./test_fixture_merkle_proof, ./test_fixture_sanity_blocks, ./test_fixture_sanity_slots, ./test_fixture_transition diff --git a/tests/consensus_spec/test_fixture_merkle_proof.nim b/tests/consensus_spec/test_fixture_merkle_proof.nim new file mode 100644 index 0000000000..828a4e048a --- /dev/null +++ b/tests/consensus_spec/test_fixture_merkle_proof.nim @@ -0,0 +1,74 @@ +# beacon_chain +# Copyright (c) 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. + +{.used.} + +import + # Standard library + std/[sequtils, streams], + # Status libraries + stew/bitops2, + # Third-party + yaml, + # Beacon chain internals + ../../beacon_chain/spec/helpers, + # Test utilities + ../testutil, + ./fixtures_utils, ./os_ops + +proc runTest[T](suiteName, path: string, objType: typedesc[T]) = + test "Merkle proof - Single merkle proof - " & path.relativePath(SszTestsDir): + type + TestProof = object + leaf: string + leaf_index: GeneralizedIndex + branch: seq[string] + + let + proof = block: + let s = openFileStream(path/"proof.yaml") + defer: close(s) + var res: TestProof + yaml.load(s, res) + res + + obj = newClone(parseTest(path/"object.ssz_snappy", SSZ, T)) + + var computedProof = newSeq[Eth2Digest](log2trunc(proof.leaf_index)) + build_proof(obj[], proof.leaf_index, computedProof).get + + check: + computedProof == proof.branch.mapIt(Eth2Digest.fromHex(it)) + is_valid_merkle_branch( + Eth2Digest.fromHex(proof.leaf), + computedProof, + log2trunc(proof.leaf_index), + get_subtree_index(proof.leaf_index), + hash_tree_root(obj[])) + +suite "EF - Merkle proof" & preset(): + const presetPath = SszTestsDir/const_preset + for kind, path in walkDir(presetPath, relative = true, checkDir = true): + let testsPath = presetPath/path/"merkle_proof"/"single_merkle_proof" + if kind != pcDir or not dirExists(testsPath): + continue + let fork = forkForPathComponent(path).valueOr: + test "Merkle proof - Single merkle proof - " & path: + skip() + continue + for kind, path in walkDir(testsPath, relative = true, checkDir = true): + let suitePath = testsPath/path + if kind != pcDir or not dirExists(suitePath): + continue + let objName = path + withConsensusFork(fork): + for kind, path in walkDir(suitePath, relative = true, checkDir = true): + case objName + of "BeaconBlockBody": + runTest(suiteName, suitePath/path, consensusFork.BeaconBlockBody) + else: + raiseAssert "Unknown test object: " & suitePath/path From 8d46809a5c8488ef13728beb5853d9d780d00c77 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Sat, 4 Nov 2023 20:36:01 +0100 Subject: [PATCH 2/3] skip upcoming FC tests for intentional block reorgs until implemented (#5566) v1.4.0-beta.4 adds tests for intentional block reorgs. To reflect the implementation status, skip those tests for now and mark them as such. --- tests/consensus_spec/test_fixture_fork_choice.nim | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/consensus_spec/test_fixture_fork_choice.nim b/tests/consensus_spec/test_fixture_fork_choice.nim index c831652796..f4edfc4c40 100644 --- a/tests/consensus_spec/test_fixture_fork_choice.nim +++ b/tests/consensus_spec/test_fixture_fork_choice.nim @@ -354,6 +354,12 @@ proc runTest(suiteName: static[string], path: string, fork: ConsensusFork) = "too_late_for_merge", "block_lookup_failed", "all_valid", + + # TODO intentional reorgs + "should_override_forkchoice_update__false", + "should_override_forkchoice_update__true", + "basic_is_parent_root", + "basic_is_head_root", ] test suiteName & " - " & path.relativePath(SszTestsDir): From f14389bb8440f437e9cea6f2ba6d6386fbdd2219 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Sat, 4 Nov 2023 20:36:12 +0100 Subject: [PATCH 3/3] avoid perpetually sending blobs to peers (#5563) Fix regression from #4808 where blobs that are already known are issued ACCEPT verdict, propagating them to peers over and over again. `validateBlobSidecar` contains the correct IGNORE logic. Moved it above the expensive checks to retain the performance of the check. --- beacon_chain/gossip_processing/eth2_processor.nim | 7 +------ beacon_chain/gossip_processing/gossip_validation.nim | 12 ++++++------ 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/beacon_chain/gossip_processing/eth2_processor.nim b/beacon_chain/gossip_processing/eth2_processor.nim index 71f22f66d0..87406275b5 100644 --- a/beacon_chain/gossip_processing/eth2_processor.nim +++ b/beacon_chain/gossip_processing/eth2_processor.nim @@ -287,12 +287,7 @@ proc processSignedBlobSidecar*( # Potential under/overflows are fine; would just create odd metrics and logs let delay = wallTime - signedBlobSidecar.message.slot.start_beacon_time - - if self.blobQuarantine[].hasBlob(signedBlobSidecar.message): - debug "Blob received, already in quarantine", delay - return ValidationRes.ok - else: - debug "Blob received", delay + debug "Blob received", delay let v = self.dag.validateBlobSidecar(self.quarantine, self.blobQuarantine, diff --git a/beacon_chain/gossip_processing/gossip_validation.nim b/beacon_chain/gossip_processing/gossip_validation.nim index 8ec4538f0e..5874a8349a 100644 --- a/beacon_chain/gossip_processing/gossip_validation.nim +++ b/beacon_chain/gossip_processing/gossip_validation.nim @@ -331,6 +331,12 @@ proc validateBlobSidecar*( if not (sbs.message.slot > dag.finalizedHead.slot): return errIgnore("SignedBlobSidecar: slot already finalized") + # [IGNORE] The sidecar is the only sidecar with valid signature + # received for the tuple (sidecar.block_root, sidecar.index). + if blobQuarantine[].hasBlob(sbs.message): + return errIgnore( + "SignedBlobSidecar: already have blob with valid signature") + # [IGNORE] The block's parent (defined by block.parent_root) has # been seen (via both gossip and non-gossip sources) (a client MAY # queue blocks for processing once the parent block is retrieved). @@ -376,12 +382,6 @@ proc validateBlobSidecar*( sbs.signature): return dag.checkedReject("SignedBlobSidecar: invalid blob signature") - # [IGNORE] The sidecar is the only sidecar with valid signature - # received for the tuple (sidecar.block_root, sidecar.index). - if blobQuarantine[].hasBlob(sbs.message): - return errIgnore( - "SignedBlobSidecar: already have blob with valid signature") - ok()