Skip to content

Commit 7dadd36

Browse files
tcoratgerclaude
andcommitted
fix(testing): make consensus vector generation deterministic across hash seeds
Some emitted vectors depended on set or dict iteration order, which Python seeds per process, so two fills of identical input could produce different bytes and break cross-client reproducibility. - aggregation: build the new aggregate pool in insertion order rather than a hash-seeded set union, mirroring the fork-choice merge fix, so fork-choice weights are reproducible - testing: collect a block's valid attestations in a list and emit gossipsub forwards in recipient order, so block bodies and outbound RPCs are stable Add an order_sensitive marker on the vectors that exercise these orderings and a fill-determinism check that regenerates the marked subset under two hash seeds and diffs them, wired into CI so a regression in this class fails fast. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent b17c82d commit 7dadd36

8 files changed

Lines changed: 54 additions & 10 deletions

File tree

.github/workflows/ci.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,9 @@ jobs:
133133
- name: Fill test fixtures
134134
run: just fill-ci
135135

136+
- name: Check fixture determinism (order-sensitive vectors)
137+
run: just fill-determinism
138+
136139
interop-tests:
137140
name: Interop tests - Multi-node consensus
138141
runs-on: macos-latest

Justfile

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,31 @@ test-consensus *args:
8585
fill-ci *args:
8686
uv run --group test fill --fork=Lstar --clean -n auto --dist=worksteal "$@"
8787

88+
# Generate the order-sensitive vectors twice under different hash seeds and diff.
89+
# Only the vectors marked order_sensitive run, so this stays cheap.
90+
# A difference means an emitted vector depends on set or dict iteration order,
91+
# which is hash-seeded and would break cross-client reproducibility.
92+
# Pass a path to widen the scope (for example the whole tests/consensus tree).
93+
[group('tests')]
94+
fill-determinism *args:
95+
#!/usr/bin/env bash
96+
set -euo pipefail
97+
target="{{args}}"
98+
[ -z "$target" ] && target="tests/consensus -m order_sensitive"
99+
first="$(mktemp -d)"
100+
second="$(mktemp -d)"
101+
trap 'rm -rf "$first" "$second"' EXIT
102+
# Single process: the marked subset is small, so xdist worker startup would
103+
# cost more than it saves, and one process pins the hash seed cleanly.
104+
PYTHONHASHSEED=1 uv run --group test fill --fork=Lstar --clean -n 0 -o "$first" $target -q
105+
PYTHONHASHSEED=2 uv run --group test fill --fork=Lstar --clean -n 0 -o "$second" $target -q
106+
if diff -rq "$first" "$second"; then
107+
echo "Determinism check passed: fixtures are byte-identical across hash seeds."
108+
else
109+
echo "Determinism check FAILED: emitted vectors differ across PYTHONHASHSEED." >&2
110+
exit 1
111+
fi
112+
88113
# Run API conformance tests against an external client
89114
[group('tests')]
90115
apitest server_url *args:

packages/testing/src/consensus_testing/pytest_plugins/filler.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,11 @@ def pytest_configure(config: pytest.Config) -> None:
194194
"real_crypto(smoke=False): build and verify with the real prover, never the mock; "
195195
"smoke=True also keeps it in the fast mocked lane",
196196
)
197+
config.addinivalue_line(
198+
"markers",
199+
"order_sensitive: emission could depend on set or dict iteration order; "
200+
"the determinism check generates this vector twice and diffs the output",
201+
)
197202

198203
# Crypto mode is chosen explicitly and applies to either scheme.
199204
AggregationProver.set_mode(CryptoMode(config.getoption("--crypto")))

packages/testing/src/consensus_testing/test_fixtures/gossipsub_handler.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -476,8 +476,14 @@ async def _execute(self) -> GossipsubExpectation:
476476
# Each entry represents one RPC sent to a peer.
477477
# The structure mirrors the gossipsub RPC wire format.
478478
# Fixture consumers use this to assert exact outbound behavior.
479+
# Emit outbound RPCs in canonical recipient order.
480+
# The send order to mesh peers is set-driven.
481+
# It is not consensus-relevant.
482+
# Sorting by recipient keeps the emitted vector reproducible across runs.
479483
sent_rpcs = []
480-
for recipient_peer_id, rpc in capture.sent:
484+
for recipient_peer_id, rpc in sorted(
485+
capture.sent, key=lambda entry: peer_names.get(entry[0], str(entry[0]))
486+
):
481487
subscriptions = (
482488
[
483489
SentSubscription(

packages/testing/src/consensus_testing/test_types/block_spec.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ def build_attestations(
178178
) -> tuple[
179179
list[Attestation],
180180
dict[AttestationData, dict[ValidatorIndex, Signature]],
181-
set[Attestation],
181+
list[Attestation],
182182
]:
183183
"""
184184
Build attestations and signatures from this block's attestation specs.
@@ -195,11 +195,14 @@ def build_attestations(
195195
- Subset of attestations that have valid (non-dummy) signatures
196196
"""
197197
if self.attestations is None:
198-
return [], {}, set()
198+
return [], {}, []
199199

200200
attestations: list[Attestation] = []
201201
signature_lookup: dict[AttestationData, dict[ValidatorIndex, Signature]] = {}
202-
valid_attestations: set[Attestation] = set()
202+
# A list, not a set.
203+
# Build order is the canonical order consumers gossip in.
204+
# That keeps the emitted block body reproducible across runs.
205+
valid_attestations: list[Attestation] = []
203206

204207
for aggregated_spec in self.attestations:
205208
# Build attestation data once.
@@ -224,7 +227,7 @@ def build_attestations(
224227
validator_index,
225228
attestation_data,
226229
)
227-
valid_attestations.add(attestation)
230+
valid_attestations.append(attestation)
228231
else:
229232
signature = create_dummy_signature()
230233

src/lean_spec/spec/forks/lstar/aggregation.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,11 @@ def aggregate(self, store: LstarStore) -> tuple[LstarStore, list[SignedAggregate
113113
#
114114
# Known payloads cannot start a round alone, since re-aggregating them adds nothing.
115115
# They serve only as fallback building blocks once fresh evidence exists.
116-
for attestation_data in (
117-
store.latest_new_aggregated_payloads.keys() | store.attestation_signatures.keys()
118-
):
116+
# Iterate in insertion order, in order to have a deterministic pool order.
117+
for attestation_data in {
118+
**store.latest_new_aggregated_payloads,
119+
**store.attestation_signatures,
120+
}:
119121
# Phase 1: Select.
120122
#
121123
# Reuse existing proofs first to keep the proof tree shallow.

tests/consensus/lstar/fork_choice/test_block_attestation_limits.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
from lean_spec.spec.forks import RejectionReason, Slot, ValidatorIndex
1515
from lean_spec.spec.forks.lstar.config import MAX_ATTESTATIONS_DATA
1616

17-
pytestmark = pytest.mark.valid_until("Lstar")
17+
pytestmark = [pytest.mark.valid_until("Lstar"), pytest.mark.order_sensitive]
1818

1919

2020
def _justifiable_slots(n: int) -> list[Slot]:

tests/consensus/lstar/networking/test_gossipsub_handlers.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
PeerConfiguration,
1717
)
1818

19-
pytestmark = pytest.mark.valid_until("Lstar")
19+
pytestmark = [pytest.mark.valid_until("Lstar"), pytest.mark.order_sensitive]
2020

2121
TOPIC = "test_topic"
2222
PARAMS = GossipsubMeshParameters(d=4, d_low=3, d_high=6, d_lazy=3)

0 commit comments

Comments
 (0)