From 75a11522cfc50f5bbdb0a384dbf25bf08c42b78b Mon Sep 17 00:00:00 2001 From: Thomas Coratger <60488569+tcoratger@users.noreply.github.com> Date: Mon, 13 Apr 2026 16:28:42 +0200 Subject: [PATCH] refactor: extract resolve_block_root to utils and rename misleading test Extract the duplicated _resolve_label closure from StoreChecks and StateExpectation into a shared resolve_block_root function in utils.py. Both validation methods now use a thin _resolve wrapper that delegates to the shared function. Removes the unused hash_tree_root import from state_expectation.py. Rename test_mid_block_resolved_target_does_not_reopen_pending_votes to test_merged_attestations_for_same_target_justify_and_finalize_cleanly. The old name implied the "already-justified skip" guard fires, but the builder merges both attestation specs into one aggregated attestation before processing. The test verifies merging behavior, not the skip path. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../test_types/state_expectation.py | 43 ++++++------------- .../test_types/store_checks.py | 28 +++++------- .../src/consensus_testing/test_types/utils.py | 23 ++++++++++ .../state_transition/test_finalization.py | 11 +++-- 4 files changed, 55 insertions(+), 50 deletions(-) diff --git a/packages/testing/src/consensus_testing/test_types/state_expectation.py b/packages/testing/src/consensus_testing/test_types/state_expectation.py index 8c2acd995..ed7092ef2 100644 --- a/packages/testing/src/consensus_testing/test_types/state_expectation.py +++ b/packages/testing/src/consensus_testing/test_types/state_expectation.py @@ -12,9 +12,10 @@ JustificationValidators, JustifiedSlots, ) -from lean_spec.subspecs.ssz.hash import hash_tree_root from lean_spec.types import Bytes32, CamelModel +from .utils import resolve_block_root + class StateExpectation(CamelModel): """ @@ -158,17 +159,10 @@ def validate_against_state( """ fields = self.model_fields_set - def _resolve_label(field_name: str, label: str) -> Bytes32: + def _resolve(label: str) -> Bytes32: if block_registry is None: - raise ValueError( - f"{field_name}='{label}' specified but block_registry not provided" - ) - if label not in block_registry: - raise ValueError( - f"{field_name}='{label}' not found in block registry. " - f"Available: {list(block_registry.keys())}" - ) - return hash_tree_root(block_registry[label]) + raise ValueError(f"label '{label}' specified but block_registry not provided") + return resolve_block_root(label, block_registry) for field_name in fields & self._ACCESSORS.keys(): accessor = self._ACCESSORS[field_name] @@ -181,34 +175,25 @@ def _resolve_label(field_name: str, label: str) -> Bytes32: if "latest_justified_root_label" in fields: assert self.latest_justified_root_label is not None - expected = _resolve_label( - "latest_justified_root_label", self.latest_justified_root_label - ) - actual = state.latest_justified.root - if actual != expected: + expected = _resolve(self.latest_justified_root_label) + if state.latest_justified.root != expected: raise AssertionError( - f"State validation failed: latest_justified.root = {actual}, " - f"expected {expected}" + f"State validation failed: latest_justified.root = " + f"{state.latest_justified.root}, expected {expected}" ) if "latest_finalized_root_label" in fields: assert self.latest_finalized_root_label is not None - expected = _resolve_label( - "latest_finalized_root_label", self.latest_finalized_root_label - ) - actual = state.latest_finalized.root - if actual != expected: + expected = _resolve(self.latest_finalized_root_label) + if state.latest_finalized.root != expected: raise AssertionError( - f"State validation failed: latest_finalized.root = {actual}, " - f"expected {expected}" + f"State validation failed: latest_finalized.root = " + f"{state.latest_finalized.root}, expected {expected}" ) if "justifications_roots_labels" in fields: assert self.justifications_roots_labels is not None - expected_sorted = sorted( - _resolve_label("justifications_roots_labels", label) - for label in self.justifications_roots_labels - ) + expected_sorted = sorted(_resolve(label) for label in self.justifications_roots_labels) actual_sorted = sorted(state.justifications_roots.data) if actual_sorted != expected_sorted: raise AssertionError( diff --git a/packages/testing/src/consensus_testing/test_types/store_checks.py b/packages/testing/src/consensus_testing/test_types/store_checks.py index 4895b53a2..506e1e50b 100644 --- a/packages/testing/src/consensus_testing/test_types/store_checks.py +++ b/packages/testing/src/consensus_testing/test_types/store_checks.py @@ -10,6 +10,8 @@ from lean_spec.subspecs.ssz import hash_tree_root from lean_spec.types import Bytes32, CamelModel, Uint64 +from .utils import resolve_block_root + class AggregatedAttestationCheck(CamelModel): """ @@ -260,18 +262,12 @@ def _check(name: str, actual: object, expected: object) -> None: if actual != expected: raise AssertionError(f"Step {step_index}: {name} = {actual}, expected {expected}") - def _resolve_label(field_name: str, label: str) -> Bytes32: + def _resolve(label: str) -> Bytes32: if block_registry is None: raise ValueError( - f"Step {step_index}: {field_name}='{label}' specified " - f"but block_registry not provided" + f"Step {step_index}: label '{label}' specified but block_registry not provided" ) - if label not in block_registry: - raise ValueError( - f"Step {step_index}: {field_name}='{label}' not found " - f"in block registry. Available: {list(block_registry.keys())}" - ) - return hash_tree_root(block_registry[label]) + return resolve_block_root(label, block_registry) # Scalar store fields if "time" in fields: @@ -294,7 +290,7 @@ def _resolve_label(field_name: str, label: str) -> Bytes32: # Label-based root checks (resolve label -> root, then compare) if "head_root_label" in fields: assert self.head_root_label is not None - expected = _resolve_label("head_root_label", self.head_root_label) + expected = _resolve(self.head_root_label) _check("head.root", store.head, expected) if "filled_block_root_label" in fields: if filled_block is None: @@ -303,19 +299,15 @@ def _resolve_label(field_name: str, label: str) -> Bytes32: f"filled_block not provided" ) assert self.filled_block_root_label is not None - expected = _resolve_label("filled_block_root_label", self.filled_block_root_label) + expected = _resolve(self.filled_block_root_label) _check("filled_block.root", hash_tree_root(filled_block), expected) if "latest_justified_root_label" in fields: assert self.latest_justified_root_label is not None - expected = _resolve_label( - "latest_justified_root_label", self.latest_justified_root_label - ) + expected = _resolve(self.latest_justified_root_label) _check("latest_justified.root", store.latest_justified.root, expected) if "latest_finalized_root_label" in fields: assert self.latest_finalized_root_label is not None - expected = _resolve_label( - "latest_finalized_root_label", self.latest_finalized_root_label - ) + expected = _resolve(self.latest_finalized_root_label) _check("latest_finalized.root", store.latest_finalized.root, expected) # Attestation target checkpoint (slot + root consistency) @@ -416,7 +408,7 @@ def _resolve_label(field_name: str, label: str) -> Bytes32: if "labels_in_store" in fields: assert self.labels_in_store is not None for label in self.labels_in_store: - root = _resolve_label("labels_in_store", label) + root = _resolve(label) if root not in store.blocks: raise AssertionError( f"Step {step_index}: block '{label}' (root=0x{root.hex()}) " diff --git a/packages/testing/src/consensus_testing/test_types/utils.py b/packages/testing/src/consensus_testing/test_types/utils.py index 1f37c6cba..5b2d98097 100644 --- a/packages/testing/src/consensus_testing/test_types/utils.py +++ b/packages/testing/src/consensus_testing/test_types/utils.py @@ -6,6 +6,29 @@ from lean_spec.subspecs.containers.checkpoint import Checkpoint from lean_spec.subspecs.containers.slot import Slot from lean_spec.subspecs.ssz.hash import hash_tree_root +from lean_spec.types import Bytes32 + + +def resolve_block_root( + label: str, + block_registry: dict[str, Block], +) -> Bytes32: + """ + Resolve a block label to its hash tree root. + + Args: + label: Block label in the registry. + block_registry: Labeled blocks for lookup. + + Returns: + The block's hash tree root. + + Raises: + ValueError: If label not found in registry. + """ + if (block := block_registry.get(label)) is None: + raise ValueError(f"label '{label}' not found - available: {list(block_registry.keys())}") + return hash_tree_root(block) def resolve_checkpoint( diff --git a/tests/consensus/devnet/state_transition/test_finalization.py b/tests/consensus/devnet/state_transition/test_finalization.py index 6ae788033..8ca5e3eef 100644 --- a/tests/consensus/devnet/state_transition/test_finalization.py +++ b/tests/consensus/devnet/state_transition/test_finalization.py @@ -810,7 +810,7 @@ def test_mid_block_finalized_slot_rejects_target_that_loses_justifiability( ) -def test_mid_block_resolved_target_does_not_reopen_pending_votes( +def test_merged_attestations_for_same_target_justify_and_finalize_cleanly( state_transition_test: StateTransitionTestFiller, ) -> None: """ @@ -819,8 +819,13 @@ def test_mid_block_resolved_target_does_not_reopen_pending_votes( Scenario -------- 1. Justify block_1 in block_2 - 2. Process block_3 with one supermajority attestation to block_2 - and one later single-validator attestation to the same target + 2. Process block_3 with two attestation specs both targeting block_2: + one supermajority (V0-V2) and one single-validator (V3) + + The block builder merges both specs into a single aggregated + attestation covering all 4 validators (same AttestationData). + The merged attestation justifies slot 2 and finalizes slot 1 + in one step. No pending votes remain. Expected Behavior -----------------