Skip to content

test: pin the wire format with golden vectors; keep inputs out of release.yml scripts (V2-1751) - #51

Open
Nic-dorman wants to merge 3 commits into
mainfrom
nic/v2-1751-ant-protocol-golden-vectors
Open

Nic-dorman wants to merge 3 commits into
mainfrom
nic/v2-1751-ant-protocol-golden-vectors

Conversation

@Nic-dorman

@Nic-dorman Nic-dorman commented Nov 2, 2026 •

Copy link
Copy Markdown
Member

Round 2 follow-ups (125334e)

Hermes approved 092acd5 with three non-blocking notes, all addressed in 125334e (range 092acd5..125334e):

  1. One process only: WRITE_LOCK serializes appends between threads of one test process. The module docs now say to regenerate with a single cargo test invocation.
  2. Temp file left on panic: the test removes its file in a Drop guard. (dev2 had about 130 leftovers from my earlier runs on the round-1 code, which panicked before cleanup. The new run left none.)
  3. Injected annotations: release.yml no 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.

  • Fix: in write mode (ANT_PROTOCOL_WRITE_GOLDEN=1), golden() now holds WRITE_LOCK across both the read and the append, through a new golden_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 single write_all. The normal read-only mode doesn't lock, as before.
  • Parallel regeneration test: parallel_regeneration_is_consistent has 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.
  • Evidence, all on dev2:
    • A race test can only be probabilistic. The round-1 code failed 5/20 runs, with your exact panic `r` is not a `name hex` line. The fixed code failed 0/50 runs and takes about 2 s.
    • Your scenario: all vectors removed, write mode, 21 threads, 30 runs, 0 failures. Every regenerated vector matched the captured bytes except pointer.record, whose ML-DSA signature is randomized and is captured once by design.
    • fmt and clippy (pedantic, -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 the v5.0.1 tag). Every vector gets three checks:

  1. Encoding a fixed value gives exactly the captured bytes.
  2. The captured bytes decode to exactly that value, field by field. This is what catches two same-typed fields being swapped: postcard and rmp encode by position, so re-encoding alone can't see it. Every field gets a distinct value.
  3. Re-encoding gives the captured bytes again.

Covered:

  • ChunkMessage (postcard), every body variant and every response arm:
    • Put request (unpaid/paid); put response (Success, AlreadyExists, PaymentRequired, Error).
    • Get request; get response (Success, NotFound, Error).
    • Quote request, quote request V2; quote response (Success with commitment, baseline Success, Error).
    • Merkle candidate quote request and V2; merkle candidate quote response (Success, Error).
    • Pointer put request (unpaid/paid); pointer put response (Success, Unchanged, Stale, PaymentRequired, Error).
    • Pointer get request; pointer get response (Success, NotFound, Error).
  • ProtocolError: all 11 variants.
  • Payloads carried inside those messages (MessagePack):
    • PaymentQuote (baseline and committed);
    • MerklePaymentCandidateNode (baseline and committed);
    • StorageCommitment.
  • Commitment pin inputs: the postcard encoding of StorageCommitment, commitment_hash and commitment_signed_payload. The pin hashes postcard, not the rmp bytes on the wire, so a postcard change would silently re-pin every quote.
  • Payment proofs: single-node (tag + PaymentProof, also read back through deserialize_proof) and merkle (tag + MerklePaymentProof, with a 16-candidate pool and a real branch from a fixed tree).
  • Pointer record: a real 5,303-byte ML-DSA-65 record from a seeded key. Decoding verifies the captured signature, the signed body is rebuilt and compared byte for byte, and address and state_id are 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 into run: scripts.

  • The dispatch version input reaches the script through env: (INPUT_VERSION). The event name comes from $GITHUB_EVENT_NAME, and the version check reads EXPECTED from env:.
  • The format check now runs before anything is written to $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_OUTPUT is quoted.

Risk tier

  • T0 — docs / tooling / CI / pure UX-output. Repo CI only.
  • T1 — client-only, no network-facing behavior change. CI + prod compat smoke.
  • T2 — node/client logic with behavioral surface, no protocol/format/economics change. Dev testnet + ADR.
  • T3 — protocol / storage format / payments / routing. T2 evidence + adversarial testing.

Compatibility

  • Wire: none. These tests pin the current wire format; nothing in src/ changes.
  • Storage: none
  • API: none

Semver impact

  • breaking
  • feature
  • fix

Test evidence

Every command in ci.yml was run on dev2 (Linux) against this commit, on rustc 1.99.1 (latest stable, as CI's dtolnay/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_size and data_type in ChunkQuoteRequest. chunk_quote_request failed, 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

…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 MaidSafe-Hermes left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 MaidSafe-Hermes left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants