fix(relay): make a cold relayed capsule fetch reachable in one command - #334
fix(relay): make a cold relayed capsule fetch reachable in one command#334MichaelTaylor3d wants to merge 18 commits into
Conversation
Progress — resume-readyBranch Done
The finding that mattered, and what it changedThe first revert-proof pass ran a revert called The fix was structural rather than a bigger test. The three steps — ask, escalate, wait — are one In progressRe-running the five revert-proofs against the folded shape, individually rather than as a batch — Next action
|
Revert-proof evidenceEach revert applied ALONE to a committed tree, run alone, restored afterward. Committing first is not
The one that mattered:
|
0d2dc76 to
5305989
Compare
Implementation complete — handing back to the review gateBranch Local evidence
CI: every required check green except Test + coverage, still running at the time of writing. The acceptance bar this PR does NOT clear, stated plainly§2.6's bar is a person watching a What that means concretely: the timing constants — a 3 s hop grace, a 10 s poll, a 60 s stall window, One correction to the brief I was givenThe two manifest comments were stale, but not in the way reported. The claim was that the |
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
CHANGES-REQUIRED — correctness gate
Head reviewed: 6e2b6730a772f6fc92b39cd16c8273a666baed0b (resolved from the remote, not from the dispatch).
Recorded as a --comment review: a verdict event from the PR author's identity is a 422 for both APPROVE and REQUEST_CHANGES. The four open inline threads are what bar the merge, via required_conversation_resolution.
What I verified myself, by mutation, in my own worktree
Baseline at head: 968 passed, 0 failed.
| Probe | Result |
|---|---|
no-wait (delete the RelayPending arm) |
CONFIRMED — all four #333 tests fail. The compiler also reported wait_for_relayed_descriptor as dead code, proving the mutation reached production. |
#332 Discard (drop state_store.clear) |
CONFIRMED — a_verification_failure_still_discards_what_it_staged fails. |
#332 Preserve (make Preserve erase) |
CONFIRMED — a_severed_warm_resumes_from_its_partial_instead_of_refetching_the_capsule fails. |
| ceiling removed | CONFIRMED — a_hop_that_fabricates_endless_progress_is_bounded_by_the_ceiling fails in 0.01 s with Elapsed(()). The tokio::time::timeout bound works; it fails fast, it does not hang. |
get_module_info bypasses descriptor_via_rounds |
DEFECT — 968/968 still pass. See thread 1. |
The revert-proofs the PR claims are real. The gap is one level up: they protect the helpers, not the production call path.
The ceiling incident — verified, with one correction
RELAY_MAX_WAIT is present and correct at head (const module_transport.rs:564, the check at :624 before any further sleep, three tests). I traced the history independently: the check was removed in 27c97cd and restored in 203661c. Nothing else from that batch is missing — the only other deletions in 27c97cd are the intended descriptor_with_escalation → descriptor_via_rounds refactor, and all three RelayWaitEnd endings survive in both code and tests.
One correction to the write-up: 2a27fd3 is not an ancestor of this branch. The branch was rewritten; its on-branch twin is 27c97cd. The narrative is otherwise accurate, but a reader chasing 2a27fd3 will not find it in the history.
Verdict on the four constants
- 3 s grace — sound. Comfortably inside the 5 s first rung, so the small-capsule case stays byte-for-byte one round trip.
- 10 s poll — sound, with a doc nit: it is described as "one small round trip", but each poll is
round(true)= a re-dial plus the full 65 s ladder. Cheap for a hop that answersPendingpromptly; a hop that goes silent costs a 65 s ladder before being classedAbandoned. Non-gating. - 60 s stall — sound. Correctly sized above the poll interval, and the monotonic-max
bestmakes downward lying useless (it self-stalls at 60 s). - 30 min ceiling — right instrument, wrong scope. See thread 2: it bounds one peer, while dig-download's documented worst case is
3 × holders × the transport's per-ask timeout, and this PR raises that term from 65 s to 30 min.
Also confirmed
- NC-12 core claim holds.
staged_bytesis read only forbest/last_advanceand onetracing::debug!. It never gates acceptance, and the capsule still clears the chain anchor + merkle verification inwarm_claiming. - §2.4b spot-checked and the lane's claim is correct.
dig-nat 0.20.0→dig-tls ^0.4;dig-peer 0.12.0→chia-protocol ^0.36+dig-tls ^0.4;dig-dht 0.12.1→dig-nat ^0.20, so it is blocked transitively too.dig-peer-selector 0.9.1is already inside the declared caret. Nothing else is takeable without the chia-0.36 cascade (#3152). The two corrected Cargo.toml blocker comments are an improvement and match the index. capsule_warm_locator_tests— the set-based assertion keeps both halves: an empty set still catches a pull that asked nobody, and a second id still catches reaching past the pool. Correct call, correctly reasoned.- #332 / dig-download 0.19.2 composition — the
StagingDispositionjustification holds.for_failureis the only producer ofDiscard,erase()is the only path that clears the checkpoint, and I found no second eraser.
Not blocking
.github/** is untouched, so there is no workflow collision with #336 — but Cargo.toml, Cargo.lock and SPEC.md all overlap. Thread 5, self-resolved.
Four threads open. I will re-verify and resolve each against the new head.
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…ifest blockers The ceiling check was removed by a revert-proof batch that was killed before it could restore the file, and swept into 2a27fd3. It is the NC-12 bound: a hop's staged byte count is its own claim, so a stall window alone cannot catch a hop that fabricates rising progress forever. The two Cargo.toml comments named a blocker that never existed: chia-peer 0.2.0 skipped dig-chainsource-interface 0.2 and went straight to dci ^0.3 with chia-protocol ^0.36.1, where chia-query 0.15.0 also sits. The real blocker is the chia-0.36 cascade (dig_ecosystem#3152). Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…ion proof dig-download 0.19.2 adds an across-round descriptor re-ask (dig-download#37), so a never-answering holder is now asked MAX_DESCRIPTOR_ATTEMPTS times. The test pinned the exact ask sequence, which was never its subject -- it proves the warm locator unions the connected pool. Both halves of that property survive the change to a set: an empty set still fails, and a second peer still fails. Co-Authored-By: Claude <noreply@anthropic.com>
…NTS.md gitnexus analyze rewrites its index name into both files, which are TRACKED, so a per-worktree index silently lands in the diff. Co-Authored-By: Claude <noreply@anthropic.com>
…ay waiting per PULL The ladder, the second-pass escalation and the progress-bounded wait now live in a private child module. The parent supplies only a policy-free one-shot ask, so a call site cannot reach the middle of the decision: the gate's bypass -- calling the inner round directly, which deleted both features with 968 tests green -- no longer compiles. RELAY_MAX_WAIT bounds one PEER. The pull asks many, so ten hops each fabricating a byte of progress per poll stayed inside their ceilings while holding one pull for hours. RelayWaitBudget charges relay waiting against the CAPSULE, so one honest hop may still spend the whole budget and liars share it. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…its name Co-Authored-By: Claude <noreply@anthropic.com>
96957a1 to
62b3bd2
Compare
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
All four gate findings addressed — re-gate requestedRebased onto Finding 1 — you were right, and my fix had closed the wrong jointUnifying the three steps made them inseparable from each other while leaving the inner ladder The fix is structural, not another assertion. The ladder, the escalation and the relay wait now Proved the way you broke it. I reproduced your bypass exactly — replaced
The second is new and is the one that pins the call site rather than the helper: a One thing worth flagging: on the first attempt your bypass made the ladder test hang rather than Finding 2 — per-pull budget added; the per-hop ceiling untouched
Three tests, all revert-proved. The load-bearing one is Also written into Finding 3 —
|
| gate | result |
|---|---|
cargo fmt --all -- --check |
0 |
cargo clippy --workspace --all-targets -- -D warnings |
0 |
cargo test --workspace |
0 — 973 in dig-node-core lib, 0 failed |
| revert-proofs | 9 total, each run alone, each naming the assertion that fired |
Still DRAFT. Re-gate requested.
DO NOT MERGE — gate round not yet run.
dig-node's whole remaining share of dig_ecosystem#3128, as one batch (CLAUDE.md §1.4 batch-per-repo).
Closes #322
Closes #332
Closes #333
The shape decision for #333, and its argument
A blocking ask must be capped tightly; a non-blocking one need not be. The 65 s descriptor ladder
is not too short — it is the right bound for the operation it was built for, and the operation was
wrong.
The hop held the requestor's stream open for the whole duration of a third-party 135 MB transfer, so
the cost of waiting was a held stream on both ends. That is exactly the cost a wall-clock cap exists to
bound, which is why the cap is correct and why the capability was unreachable behind it.
Neither obvious fix works:
two-phase proxy and
proxy_rate_limiterexist to bound.the thing being waited for.
So the fix decouples the two waits rather than lengthening one:
relay_capsulereturnsRelayStatus { Landed | Pending { staged_bytes } | Refused }.The pull is spawned in the background and the ask waits only a 3 s grace — comfortably inside the
requestor's first 5 s rung, so a capsule that lands quickly is still answered in one round trip and
the case that already worked is unchanged. One that does not is answered
ContentMissInconclusive(
-32017) witherror.data.relay_staged_bytes, and the pull keeps running.re-ask every 10 s, continue while the staged count strictly advances, abandon after a 60 s stall
window with no advance, and abandon at a 30 min ceiling regardless.
Once the ask is non-blocking, the instrument changes. Each further poll costs one small round trip
instead of a held stream, so "how long may I wait" stops being a resource question and becomes a
liveness question. Forward progress needs no size; a wall clock needs a size the requestor cannot have.
Why the ceiling is not optional (NC-12).
relay_staged_bytesis a HOP'S CLAIM about itself. Astall window cannot catch a hostile hop that fabricates a counter rising forever, so the ceiling is
what makes the worst case finite: a lying hop can waste one pull's time from one peer, and no more.
The claim decides only how long to wait — never whether a byte is accepted. The capsule is still
verified against the chain-anchored root exactly as a direct holder's would be, so a hop that
fabricates its way through a wait still cannot produce content that passes.
Backwards compatible by construction. The code is the taxonomy's existing inconclusive-miss code,
not a new number (
SYSTEM.md: the taxonomy is owned bydig-rpc-protocoland adopted, neverrestated — this repo has paid for restating it twice). A requestor that ignores
relay_staged_bytessees an ordinary inconclusive miss and retries later, which is precisely what #333's field observers
were doing by hand.
The diff, per ticket
#333 —
module_relay.rs(RelayStatus, the 3 s grace, background spawn),module_reshare.rs(
spawn_relayed_capsule_warm,holds,staged_bytes,await_landing),lib.rs(
relay_pending_err,RELAY_PROGRESS_FIELD, both serve sites),peer.rs+module_serve.rs(module_relay_pending_framefor the streaming site),module_transport.rs(
DescriptorAnswer,wait_for_relayed_descriptor, the three bounds).#322 —
module_transport.rs:LadderEndnow distinguishesRefused(the peer ANSWERED, and theanswer was no) from
Exhausted(it never answered). A refusal escalates within the same invocation;an exhausted ladder does not, because a peer that could not answer a plain ask will not answer a relay
ask and escalating would double the invocation's bound for nothing.
#332 —
module_reshare.rs:StagedPullholds the staged bytes and the resume checkpoint as ONEvalue with one
erase, so no failure path can remove half a partial.Discarderases both;Preservetouches neither.Decisions the coordinator asked me to state
StagingDispositionis KEPT, not removed. With dig-download 0.19.2's demotion wipe gone, thedisposition is not belt-and-braces — it is now the ONLY thing that erases a partial attributable to a
proven-false descriptor. The lower layer deliberately PRESERVES on demotion; something still has to
decide that a verification failure is different, and that decision has exactly one home.
dig-downloadis bumped to 0.19.2 in the lock (lock-only; the"0.19"caret already admits it).Verified from the index:
vers=0.19.2, yanked=false, checksum02a51e9858830dcc…, and thenon-dev dependency set is byte-identical to 0.19.1, so the single
[[package]]block swap is the wholecorrect change.
cargo metadata --lockedpasses.No other
dig-*/chia-*bump is takeable here (§2.4b), and this is measured, not assumed. Everylatest release reaches chia 0.36 —
dig-download 0.20.3,dig-nat 0.20.0anddig-dht 0.12.1allrequire
dig-tls ^0.4(chia-bls ^0.36.1), anddig-peer 0.12.0requires chia-protocol ^0.36 — whilethis crate is on chia-protocol 0.26 / chia-bls 0.26 / chia-wallet-sdk 0.30. Taking any one of them
would put two chia lines inside one crate, which is the internally-split state §2.4b exists to prevent
and which shipped twice on 2026-08-22. The uplift is the chia-0.36 cascade (dig_ecosystem#3152), not
this PR.
Two false manifest comments corrected (
dig-node-core/Cargo.toml,dig-wallet/Cargo.toml). Theysaid the
chia-peerunpin awaited a release built againstdig-chainsource-interface0.2. Measured onthe index: no such release was ever needed —
chia-peer 0.2.0skipped dci 0.2 and went straight todci
^0.3+ chia-protocol^0.36.1, wherechia-query 0.15.0also sits, so the twoChainSourceProvidertraits already unify again. They unify at CHIA 0.36, so the real blocker is thecascade above. A stale blocker is worse than none, because it stops people looking.
Blast radius (§2.0)
gitnexus indexed per-worktree (
dn-3128, 114 MB) and queried; paired with ripgrep, because gitnexusunder-counted here — it found 1 caller of
relay_capsulewhere grep found 3.relay_capsulepeer.rs::stream_module_range,lib.rs::get_module_info,lib.rs::fetch_module_range_frame. All three updated.discard_staging→StagedPull::eraseStagingDisposition::apply,warm_with_config)descriptor_overget_module_info)ask_within_deadlinesNo public API of
dig-node-corechanges shape; every edited symbol ispub(crate)or private, and thewire change is one additive
error.datafield.Verification
Revert-proofs, each run alone, naming the assertion that fired — see the PR comments for the table.