Repository navigation
test: pin the wire format with golden vectors; keep inputs out of release.yml scripts (V2-1751) - #51
Nic-dorman wants to merge 3 commits into
Conversation
…ease.yml scripts (V2-1751) Every serialization test in this crate round-trips in-process, so a reordered field, an inserted variant or a serializer bump passes CI and only breaks when ant-node and ant-client from different versions talk. tests/golden_wire.rs checks the bytes captured from 5.0.1 (tests/golden/wire-v5.txt) for every ChunkMessage body and response arm, all ProtocolError variants, the rmp-encoded quote, candidate and commitment payloads, the commitment pin inputs, single-node and merkle payment proofs, and a real ML-DSA-65 pointer record. Each vector must encode to the captured bytes, decode to the expected value field by field (catches swapped same-typed fields), and re-encode identically. Deliberate re-capture: delete the vector and run once with ANT_PROTOCOL_WRITE_GOLDEN=1 (append-only). release.yml: the dispatch version input reaches the script through env, the format check runs before anything is written to $GITHUB_OUTPUT, and $GITHUB_OUTPUT is quoted. Closes V2-1751 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MaidSafe-Hermes
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES
2/2 seats reviewed (sol-1 gpt-6.1-sol xhigh; glm-1 z-ai/glm-5.3). The seats disagreed. I confirmed the change-request finding with an exact-head reproduction.
P2 — tests/golden_wire.rs:107-120. When ANT_PROTOCOL_WRITE_GOLDEN=1 and vectors are missing, golden() reads the vector file before taking WRITE_LOCK. Other test threads can read while a writer appends a row. I reproduced a failure on this head with all vectors removed and 21 test threads: chunk_put_request panicked because it read a partially written row (chunk.get_response.success was not a name hex line). It failed on run 4 of 50 attempts.
Minimum fix: serialize the read-and-append path under WRITE_LOCK. Keep the lock through the second read and append, without nested locking. Add a parallel regeneration test. The normal suite passed serially (21/21); the parallel missing-vector recovery path did not.
Security Audit also fails on inherited crypto-bigint 0.5.5 / RUSTSEC-2026-0387. Cargo.lock is identical to base. This audit is not a required merge status check; handle it as a separate release gate.
…-1751) Hermes round 1 (REQUEST_CHANGES on b5d6047): with ANT_PROTOCOL_WRITE_GOLDEN=1 and vectors missing, golden() read the vector file before taking WRITE_LOCK, so a parallel test could read a line another test was still appending ("... is not a `name hex` line"; reproduced 4/50 runs with all vectors removed and 21 threads). golden() now delegates to golden_in(path, name, fresh, write), which in write mode holds WRITE_LOCK across the read and the append (no nested locking; a poisoned lock is recovered, since the file is still intact). Each line is appended with a single write_all. parallel_regeneration_is_consistent: 32 threads regenerate 150 vectors into a temporary file (the real vector file is never touched), then check each vector was written exactly once and reads back intact. A race test can only be probabilistic. On dev2 the round-1 code failed 5/20 runs with the reported "`r` is not a `name hex` line", and the fixed code 0/50 (about 2 s per run). Also checked: regenerating every vector from scratch (30 runs, 21 threads) reproduces the captured bytes exactly, except pointer.record, whose ML-DSA signature is randomized and is captured once by design. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MaidSafe-Hermes
left a comment
There was a problem hiding this comment.
APPROVE
2/2 seats reviewed (sol-1 gpt-6.1-sol xhigh; glm-1 z-ai/glm-5.3); both seats agree (two model families).
The previous P2 race is fixed. In write mode, golden_in takes WRITE_LOCK before reading and holds it through the append. The new regression test checks 32 concurrent threads and 150 vectors.
Verified on this head: cargo test --locked --test golden_wire -- --test-threads=21 passed (22/22). I also cleared all 54 data rows in an isolated worktree, ran regeneration mode with 21 threads, and confirmed all rows were restored. All matched the captured fixtures except pointer.record, whose ML-DSA signature is randomized by design.
Non-blocking limits: the mutex covers threads in one test process, not separate regeneration processes. The temp-file test can leave its file if it panics. The unchanged release workflow echoes an invalid version into a ::error:: command; escaping or omitting the value would avoid injected log annotations. None blocks approval.
…echoed input (V2-1751) Hermes approved 092acd5 with three non-blocking notes: - WRITE_LOCK serializes appends between threads of one test process, not across processes. The module docs now say to regenerate with a single `cargo test` invocation. - parallel_regeneration_is_consistent removes its temporary file in a Drop guard, so a panicking run doesn't leave it behind. - release.yml no longer echoes a rejected dispatch version into its ::error:: line; text after a newline in it could have been read as another workflow command. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Round 2 follow-ups (125334e)
Hermes approved
092acd5with three non-blocking notes, all addressed in125334e(range092acd5..125334e):WRITE_LOCKserializes appends between threads of one test process. The module docs now say to regenerate with a singlecargo testinvocation.Dropguard. (dev2 had about 130 leftovers from my earlier runs on the round-1 code, which panicked before cleanup. The new run left none.)release.ymlno longer echoes a rejected dispatch version into its::error::line.On dev2, fmt and clippy (pedantic,
-D warnings) are clean and the golden suite passes 22/22. actionlint is clean.Round 1 fix (092acd5)
Hermes requested changes on
b5d60472. Review range:b5d60472..092acd5.ANT_PROTOCOL_WRITE_GOLDEN=1),golden()now holdsWRITE_LOCKacross both the read and the append, through a newgolden_in(path, name, fresh, write). There is no nested locking, and a poisoned lock is recovered (the file is still intact). Each line is appended with a singlewrite_all. The normal read-only mode doesn't lock, as before.parallel_regeneration_is_consistenthas 32 threads regenerate 150 vectors into a temporary file, never the real one. It then checks that each vector was written exactly once and reads back intact.`r` is not a `name hex` line. The fixed code failed 0/50 runs and takes about 2 s.pointer.record, whose ML-DSA signature is randomized and is captured once by design.-D warnings) are clean.--all-features: 109 lib + 22 golden tests pass. The golden suite run 30 times in parallel: 0 failures.The Security Audit red is still the inherited RUSTSEC-2026-0387 (V2-1743), and it isn't a required check.
Linear issue
Closes V2-1751
Summary
Follow-ups from the faux-CI audit (V2-1746). They cover items 1 and 3 of V2-1751; items 2 (release test gate) and 4 (ADR Governance) stay with V2-1746.
1. Golden wire-format vectors (
tests/golden_wire.rs,tests/golden/wire-v5.txt).Every serialization test in this crate round-trips in-process. A reordered field, an inserted variant or a serializer bump changes both sides of that round trip at once, so the test passes, and it only breaks when an ant-node and an ant-client from different versions talk to each other.
The new tests pin the bytes captured from 5.0.1 (main's
src/is identical to thev5.0.1tag). Every vector gets three checks:Covered:
ChunkMessage(postcard), every body variant and every response arm:ProtocolError: all 11 variants.PaymentQuote(baseline and committed);MerklePaymentCandidateNode(baseline and committed);StorageCommitment.StorageCommitment,commitment_hashandcommitment_signed_payload. The pin hashes postcard, not the rmp bytes on the wire, so a postcard change would silently re-pin every quote.PaymentProof, also read back throughdeserialize_proof) and merkle (tag +MerklePaymentProof, with a 16-candidate pool and a real branch from a fixed tree).addressandstate_idare pinned.Signatures and public keys inside quotes, candidates and commitments are short stand-in bytes. The wire carries them as opaque byte strings, and real ones would make each vector about 10 KB. Only the pointer needs a real signature, because decoding a pointer verifies it.
A failing vector means the wire changed. The module docs say so, and they describe the deliberate re-capture path for a coordinated cutover: delete the line and run once with
ANT_PROTOCOL_WRITE_GOLDEN=1. That mode only appends missing vectors; it never overwrites existing ones.2.
release.yml: no${{ }}interpolated intorun:scripts.versioninput reaches the script throughenv:(INPUT_VERSION). The event name comes from$GITHUB_EVENT_NAME, and the version check readsEXPECTEDfromenv:.$GITHUB_OUTPUT, inside the same step. Before, a malformed input could add or override step outputs, because the regex step ran after the outputs were written. The separate "Validate version format" step is folded in; no job or job name changes.$GITHUB_OUTPUTis quoted.Risk tier
Compatibility
src/changes.Semver impact
Test evidence
Every command in
ci.ymlwas run on dev2 (Linux) against this commit, on rustc 1.99.1 (latest stable, as CI'sdtolnay/rust-toolchain@stable):cargo fmt --all -- --check: pass.cargo clippy --all-targets --all-features -- -D warnings: pass.cargo test --all-features -- --test-threads=1, Anvil included: 130 passed. That's the 109 lib tests plus the 21 golden tests.cargo build --no-default-features: pass.cargo test --no-default-features: 126 passed.cargo test --locked --no-default-features --features rpc: 126 passed.cargo doc --all-features --no-deps: pass.cargo publish --dry-run: pass. The packaged crate builds, and the vector file isn't needed by the package.The vectors catch a real layout change. I swapped the declaration order of
data_sizeanddata_typeinChunkQuoteRequest.chunk_quote_requestfailed, the other 20 passed, and the change was reverted.The release.yml change is not exercised in PR CI: the workflow only runs on tags and dispatch.
New dependency
none (the tests use only existing dependencies and dev-dependencies)
ADR
n/a
Mitigation / rollback
Revert the PR. It adds a test file, a data file and a workflow env change, and touches no crate code.
🤖 Generated with Claude Code