test(relay): make the module relay's NC-1 payload exclusion falsifiable - #353
Conversation
0534929 to
ef7c8bf
Compare
module_relay states the NC-1 / 5.4 exclusion in prose: the hop is handed (store_id, root) and never a retrieval key, so it relays capsule ciphertext it cannot read. The eco#3128 measurement recorded that no test fails if a directed, recipient-addressed message becomes routable through that leg. Prose only. Add the guard. Two DIFFERENT retrieval keys ride a fully-open relay - one on the describe, one on the window - and both must select nothing: the same capsule described, a byte-identical window served, and exactly one cached artifact, keyed by (store, root) and never by a key. One key could not distinguish an ignored key from a threaded one, so the inequality is asserted rather than assumed. Verified load-bearing by the nearest wrong implementation: threading params[retrieval_key] into relay_capsule leaves all 988 other tests in the crate green and fails only this one. Refs DIG-Network/dig_ecosystem#3128 Co-Authored-By: Claude <noreply@anthropic.com>
ef7c8bf to
9e63d04
Compare
|
Correctness gate — interim evidence (independent reproduction) @ Re-running the PR's four measurement rows myself, in a private worktree, one cargo process at a time, reading
The Mutation applied and asserted present at The guard fails for the RIGHT reason. The assertion that fires is the describe's i.e. the mutated hop went off pulling a capsule keyed by the retrieval key instead of serving the one |
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
Correctness gate: PASS
Head reviewed: 9e63d04b06d0eaad3fc7881354776e6b6f4a50ab (resolved from the remote, not from the dispatch brief). Read-only review; all probing done in a private worktree at C:\tmp\worktrees\gate353. The shared dig-node checkout was not mutated.
The four measurement rows — INDEPENDENTLY REPRODUCED, all four exact
Run one cargo process at a time, reading the test result: passed-counts, never ok and never stderr.
| row | claimed | measured here |
|---|---|---|
| guard alone, clean | 1 passed |
ok. 1 passed; 0 failed; 988 filtered out |
| guard alone, mutated | 0 passed; 1 failed |
FAILED. 0 passed; 1 failed |
| crate MINUS guard, mutated | 988 passed |
ok. 988 passed; 0 failed; 1 filtered out (442.68s) |
| crate restored | 989 passed |
ok. 989 passed; 0 failed; 0 filtered out (418.36s) |
The 988 filtered out on row 1 corroborates the 989 crate total from a second, independent direction.
The mutation was asserted applied, and asserted removed. One line, inserted ahead of gate (1) in relay_capsule:
let root_hex = params.get("retrieval_key").and_then(|v| v.as_str()).unwrap_or(root_hex);
grep -c retrieval_key module_relay.rs read 2 while mutated (module doc line + the mutation) and 1 after restore, and git status --porcelain was empty before row 4. So the 988-green row was measured against a tree that genuinely carried the defect — it is not a mutation that failed to apply reading as a guard that never fires.
This row is the whole value of the PR and it holds: 988 pre-existing tests, including the shipped relay gate test, are blind to a one-line change that turns a courier of opaque capsule bytes into one steerable by a recipient-addressed key.
The guard fails for the RIGHT reason
Not an incidental panic. The assertion that fires is the describe's total_size (left: Null, right: Number(314)), and the response body names the mechanism:
{"error":{"code":-32017,"data":{"relay_staged_bytes":0},
"message":"relaying the requested capsule on your behalf; not yet complete"}}
The mutated hop went off pulling a capsule keyed by the retrieval key instead of serving the one (store, root) names; that pull never lands, so the hop answers Pending. That is precisely the threading behaviour module_relay's docs forbid.
The two-key fixture — both halves verified
- The keys genuinely differ.
id_hex(0xc1)/id_hex(0xc2)(lib.rs:7129) expand toc1c1…andc2c2…. The reasoning is right and worth keeping on the record: with a single key, ignoring and threading are indistinguishable, because there is nothing for a threaded key to select differently. The fixture could not have exhibited the property under test. - The
assert_ne!can fail — measured, not assumed. Collapsing the pair to(id_hex(0xc1), id_hex(0xc1))fires it atlib.rs:7437withassertion left != right failed: the two keys MUST differ…, and it fires first, ahead of every other assertion. So a later edit that collapses the constants breaks loudly instead of silently degrading the test into the false green it was built to avoid. That is a genuine tripwire.
Scope, wiring, and version
- Zero production symbols edited. The full diff is 96 test lines in
lib.rs, 6 doc lines inmodule_relay.rs:34-39, and the version bumps. Read line by line, not inferred from the description — nothing behavioural rode in. - The two-defaults design is intact and was not rebuilt.
declare_proxyremains atmodule_transport.rs:492/515with call sites at:1507/:1548; NC-4 is untouched (no SPEC or normative file appears in the diff at all). The lane correctly did not re-implement a producer that had already shipped. wire_relay_hopreused, not duplicated. Definedlib.rs:7260, now called from exactly two places —:7341(the prior gate test) and:7431(this one). No parallel fixture.- 0.151.0 → 0.151.1 patch is correct per §2.4 for a test+doc change.
- Required checks asserted BY NAME at
9e63d04viacheck-merge-preconditions.sh, not off the rollup:Lint commit messages,Check version increment,Rustfmt,Clippy,Test + coverage— all five present and SUCCESS.mergeStateStatus=CLEAN(not theDIRTY/absent-contexts state the lane hit earlier), 0 unresolved threads. The script'sBLOCKEDis solelydraft=true, which is the intended state — do not undraft here.
Non-gating observation, confirmed WARRANTED (do not fix in this PR)
The lane's note that other NC-12 claims in seams/dig_peer/ are doc-comment-only looks right from what I can see: 15 NC-12 assertions across 7 files — ask_routing.rs (1), forwarded_ask.rs (3), holder_cache.rs (1), module_relay.rs (2), module_reshare.rs (3), module_serve.rs (1), module_transport.rs (4). Several sit next to heavy existing coverage in module_reshare, so the count is an upper bound, not a defect list. The sharpest candidate is forwarded_ask.rs:78 — a wire budget clamped from an untrusted peer, which is exactly the shape where the ecosystem has already recorded a rival implementation disagreeing on failure direction. Worth scoping as its own audit under eco#3128 rather than guessing; not a blocker here.
Claimed nothing it has not shown
The PR does not assert an end-to-end result. Every relay proof in the crate still runs against MockModuleTransport / MockProviderLocator, and nobody has yet watched a real .dig arrive over a real hop between two real hosts. That is a fleet job recorded on eco#3128 and is correctly not claimed here.
No gating findings. No open threads opened by this review.
Outcome first: the eco#3128 measurement is STALE — requirement 4 is LIVE
Parent epic: https://github.com/DIG-Network/dig_ecosystem/issues/3128
The brief for this lane said requirement 4 (onion streaming) was "shipped as code,
vacuous as behaviour — nothing in the tree sends
proxy: true", and asked for onereal producer.
That is no longer true. Re-measured against
origin/main@90ed299:NatModuleTransport— the productionModuleTransportimpl — sendsproxy: trueon a SECOND-pass escalation, from two call sites:
module_transport.rs:1507(get_module_info)module_transport.rs:1548(fetch_module_range)RelayEscalation::escalate_for(module_transport.rs:284),which latches per
(capsule, peer)pair: the first ask goes plain, every later askfor that pair escalates. That is exactly the two-defaults design the epic settled —
the bulk
.digpull escalates to onion, individual resource requests are untouchedand still default DIRECT (NC-4).
declare_proxy→ask_with_proxy(
module_transport.rs:486,517).Landed by #299 (the leg), #314 (warm locator reaches the pool), #334 (cold relayed
fetch reachable in one command). No rebuild was needed and none was done.
What this PR actually does
The one gap the measurement named that is STILL open:
module_relay.rs:26-31states the NC-1 / §5.4 exclusion in prose — the hop is handed(store_id, root)and never a retrieval key, so it relays ciphertext it cannot read.Prose only. This PR makes that exclusion falsifiable.
Blast radius
Test-only. Zero production symbols edited, so the call-graph radius of the change is
empty. The contract PINNED is
module_relay::relay_capsule, whose production callersare three and were read in full:
lib.rs:2817(dig.getModuleInfo),lib.rs:2903(dig.fetchModuleRange),peer.rs:1461(the peer-facing half).gitnexus fallback declared per CLAUDE.md §2.0 bound (2): radius established by grep +
direct read rather than a fresh per-worktree index, because the diff edits no symbol.
Release hazard
dig-node's cron cuts a stable tag at midnight UTC unattended. This PR is a gate,
not a feature, but the same rule applies: do not merge on red.
Evidence
The guard passes —
test result: ok. 1 passed(
tests::a_relay_ignores_recipient_specific_params_entirely).It is load-bearing. The nearest wrong implementation is one line in
relay_capsule—threading the requestor's own selector into the pull target:
That is precisely the NC-1 widening the module docs forbid: it makes the hop able to
carry a recipient-addressed payload rather than opaque capsule bytes. Under it:
test result: FAILED. 0 passed; 1 failedtest result: ok. 988 passed; 0 failedtest result: ok. 989 passed; 0 failed988 green with the widening in place. The entire pre-existing suite — including the
shipped
a_relay_serves_a_capsule_it_does_not_hold_only_when_both_gates_are_opengatetest — is blind to it. This guard is the only thing that catches it, which is exactly
the gap the measurement named.
The RED is for the right reason, not an incidental one: the mutated hop answers
-32017("relaying the requested capsule on your behalf; not yet complete") because it went off
pulling a capsule keyed by the retrieval key and never landed the real one.
Other gates:
cargo fmt --all -- --checkclean,cargo clippy -p dig-node-core --lib --all-featuresclean.Fixture design — why it is not a false green
The recurring trap here is asserting an outcome the wrong implementation also produces.
A single retrieval key would have done exactly that: with one value there is nothing
for a threaded key to select differently, so an ignoring hop and a threading hop answer
identically and the fixture cannot exhibit the property under test. Two different keys,
one on the describe and one on the window, make them disagree. The inequality is
assert_ne!d rather than assumed, so a later edit that collapses the two constantsfails loudly instead of silently blinding the test.
The third assertion is the placement half: a hop that keyed its cache or staging by the
requestor's key leaves a second artifact on disk, which is what a relayed DIRECTED
payload would physically look like.
Scope — what this PR does NOT claim
cheap widening falsifiable, which is one more than before.
dig-*/chia-*declaration is touched by this diff. The chia0.26 -> 0.36.1 uplift remains unsatisfiable in
dig-wallet— thechiaumbrellacrate has no 0.36 line (dig-node#308). Measured and reported, not shimmed.
Version: 0.150.0 -> 0.150.1 (patch — test + docs, no behaviour change).
CI — required checks asserted BY NAME at
9e63d04UNSTABLEreflects two non-required installer builds (build .msi,build .pkg) and doesnot gate. Left DRAFT deliberately — the gate round has not returned, and an undrafted PR
with green checks and zero threads is a merge invitation to any sibling lane.
Rebase note: the first push landed while
mainmoved to015b1c6(#344), which left thePR
DIRTY— and a conflicting PR runs NO required checks at all, so the rollup showed fourCodeQL passes and the five required contexts simply ABSENT rather than red. Rebased; the
version bump was re-resolved on top of #344's (0.151.0 -> 0.151.1) rather than allowed to
collapse, and
Cargo.lockwas regenerated frommainso its diff is the two version linesand nothing else. The guard was re-run green on the rebased tree.