fix(dig-node): restore the getContent envelope fields *.on.dig.net reassembles from - #177
Conversation
…ublicManifest/getManifest)
…envelope
Every *.on.dig.net subdomain has been dark since the rpc.dig.net cutover, and the
cause is three absent fields rather than an absent method.
A dig.getContent result is ONE window of a resource, and the client reassembles the
windows itself. The retired dighub-retrieval Lambda stated the resource's full
total_length plus each window's own offset and length on every window; dig-node's
build_result stated none of them, and omitted next_offset entirely when complete
rather than sending an explicit null.
The on.dig.net service worker sizes its reassembly buffer from that field
(assets/sw.js fetchVerifiedPost):
const total = first.total_length >>> 0; // undefined >>> 0 === 0
const buf = new Uint8Array(total); // zero-length buffer
so the ciphertext was copied into a 0-byte buffer and discarded, the subsequent
sum(chunk_lens) == ciphertext.length check failed, and the worker answered its own
404. Verified live in a real browser against chia-offer.on.dig.net.
Nothing on the wire was an error. Driving the resolver's own pipeline by hand shows
the node already serves this correctly: dig.getContent returns real ciphertext and a
real Merkle inclusion proof that VERIFIES against the chain-anchored root
a88ec43737ee9ae7708ae62b5ad52d77764f4c8cc0175bfb0213678840d021b2, and the bytes
decrypt to the page HTML. The read was sound; only the envelope describing it was
incomplete, which is why every probe run during the cutover passed.
Both producers of this envelope now go through ONE shared builder,
content_window_envelope — the locally-held read path and the peer fetch-through path.
A second implementation of a shared wire shape is what allowed them to drift in the
first place.
next_offset is now always present, as an explicit null on the last window, so a
client ending its loop on `next_offset == null` can tell "the resource is complete"
apart from "this server omitted the field".
Tests: the three presence tests fail without the fields (falsified by removing the
emission and re-running); a reassembly test performs the resolver's exact arithmetic
and asserts sum(chunk_lens) == buffer length; a consistency test pins the local and
fetch-through envelopes field-for-field.
SPEC.md gains §5.5.0 stating the envelope as a contract.
Refs DIG-Network/dig_ecosystem#2071
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
d623952 to
cefb076
Compare
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
CHANGES-REQUIRED - reviewed at head cefb0763b88442cf6a341570d08fe9883953fd66.
The core fix is correct and the tests are non-vacuous. I verified the four restored fields against the retired Lambda (hub.dig.net/services/retrieval/src/bin/bootstrap.rs:673-697 rpc_chunk_result), against ChunkObject in docs.dig.net/static/openrpc.json, and against the resolver's actual reassembly (on.dig.net/assets/sw.js:466-495). Field names, types and the next_offset: null-when-complete rule all match. Multi-window arithmetic is correct at window 0, a middle window and the last window, and an offset past the end clamps to an empty, complete: true, next_offset: null window. The build_result / content_result merge loses nothing: build_result always passed Some(proof), so making the proof conditional is a no-op for it, and content_result already had the if let Some gate. The only in-repo reader of these fields (seams/capsule/capsule_download.rs:213) uses .and_then(Value::as_u64), which treats an explicit null exactly like an absent field. The falsification claim holds by construction: with the emission removed, tests 1/2 compare Null against a number, test 3 sizes a zero-length buffer and panics on the copy_from_slice, and the download.rs assertion fails; only the local-vs-fetch-through consistency test legitimately stays green.
Two findings block. Both are cheap, and both are about the PUBLISHED wire contract rather than the code path this PR fixes. Given the ticket is a P0 outage caused by exactly this class of contract drift, I am not willing to wave a second live divergence through in the same envelope.
inclusion_proofis a REQUIRED field inChunkObjectand the Lambda emitted it on EVERY window; this node emits it on window 0 only, and the new SPEC section codifies that divergence rather than resolving it.- The 3-MiB window size is a cross-repo shared value defined as a bare literal in three repos, and the new SPEC section states a windowing contract without stating the window.
Non-gating notes are posted separately and self-resolved.
|
Non-gating notes (posted for the record as an issue comment, not a review thread, so they cannot block merge under
Readable-code (CLAUDE.md 2.5): the extracted builder is a clear improvement - one intent-revealing name, WHY-comments that explain the outage rather than restating the code, and a genuine de-duplication of a shared wire shape. No objection. Verified green at |
Review of the envelope fix caught that `inclusion_proof` was gated on `start == 0`,
which is the same defect this change exists to remove, relocated to large resources.
`inclusion_proof` is in `ChunkObject.required` in docs.dig.net's openrpc.json and is
documented there as "Sent on every window for getContent/getManifest"; the retired
dighub-retrieval Lambda emitted it unconditionally, gating only `chunk_lens` on the
first window. Gating the proof meant windows 1..N of any resource over 3 MiB carried
no way to verify, and a client that begins mid-resource — a resumed or ranged read —
never received one at all. A well-formed response the client cannot verify, with no
error raised anywhere, is exactly the failure mode of #2071. chia-offer.on.dig.net is
15962 bytes and fits in a single window, which is why testing against it could not
have surfaced this.
`chunk_lens` keeps its first-window-only rule, which both normative sources agree on:
it describes how to split the reassembled resource, which a client cannot act on
until it holds every window.
SPEC.md §5.5.0 is corrected and completed:
* the field table now matches ChunkObject field-for-field, and says so, so the two
normative sources cannot drift again without one of them contradicting the other
in writing
* states the 3 MiB window, and that this node IGNORES the `length` request
parameter while openrpc documents it as honoured-then-clamped, so a client sizes
its stride from the length it is GIVEN
* names all three independent definitions of the window size (WINDOW here,
RPC_MAX_CHUNK in hub's retrieval Lambda, RPC_CHUNK in on.dig.net's service
worker) and references DIG-Network/dig_ecosystem#2076 to consolidate them into
dig-constants
* drops the inaccurate "every path, one shared builder" claim: the response-window
cache replays a proxied upstream result verbatim rather than rebuilding it, which
is deliberate (it preserves provenance) and is now stated as the exception
The window-size consolidation is deliberately NOT done here. dig-node carries four
versions of dig-constants (#2072) and PR #178 adds a release gate requiring it to be
single and current, so adding a constant means a version reconciliation this outage
fix should not be coupled to.
Tests: a new test asserts the proof on both the first and last window of a
multi-window resource, and that only the first carries chunk_lens. Falsified by
re-gating the proof on `start == 0` — the last-window assertion fails. 670/670
dig-node-core lib tests pass.
Refs DIG-Network/dig_ecosystem#2071
Co-Authored-By: Claude <noreply@anthropic.com>
…le locally WIP for the #2071 method sweep — parked while PR #177 (the P0 envelope fix) is re-gated. Co-Authored-By: Claude <noreply@anthropic.com>
Re-gate: CHANGES-REQUIRED — two blocking, plus a correction to what this PR actually fixesThe hoist itself is verified correct: the proof is emitted on every window including the clamped BLOCKING 1 — the producer's own rustdoc still asserts the defect (
|
…cache key Three findings from the re-gate. The producer's own rustdoc still asserted the retired rule — "they ride the first window only (offset == 0)" — while the code 33 lines below emitted the proof unconditionally and SPEC.md said the opposite. That is the exact mechanism this change exists to remove: two written descriptions of one shape disagreeing, with the stale one in the first place a maintainer looks. Corrected. `inclusion_proof` was omitted entirely when the source carried none, while SPEC.md §5.5.0 — added by this same branch — declares it REQUIRED and calls a node that omits it non-conforming. The parity reference emits the key always, as "" (the Lambda's unwrap_or_default), and openrpc types it ["string","null"] and required. The absent case is reachable rather than theoretical: a fetch-through serve of a capsule with no per-resource commitment has inclusion_proof: None, which download.rs documents as a legitimate accepted state. Now emitted as "" — present-and-empty is a fact a client can act on, absent is one it has to guess at. The response-cache key gains an envelope SCHEMA version. A cached window is replayed verbatim and stamped source:"local", so it is indistinguishable on the wire from a freshly built one; with no version in the key, an upgraded node would keep serving pre-fix windows until they aged out of the LRU, and "the fix is deployed" would not imply "the fix is what clients receive". Bumping the version strands prior entries by construction — no eviction pass, no migration. Tests. The mutation `if start == 0 || complete` previously passed the full suite because the multi-window test sampled only offsets 0 and WINDOW — first and last, no middle, which that mutation also satisfies. Both multi-window tests now use 2*WINDOW+500 and assert the middle window, and the mutation fails them. The reassembly test — the one the file itself calls "the contract that actually matters" — ran at 15962 bytes, one window, and never looped, which is the same shape that could not have surfaced the finding; it now drives the resolver's real loop across three windows, following next_offset and checking the proof on each. SPEC.md §5.5.0 drops a false justification. "A client may verify per window" is impossible and contradicted by this crate: DigstoreProofVerifier requires proof.leaf == resource_leaf, the hash of the WHOLE reassembled ciphertext. The correct reason is that a client resuming or starting mid-resource has no other source for the proof, which the section already said. A false reason in a normative document is what lets the next person argue the rule away. Also drops the unreachable `!chunk_lens.is_null()` guard — both callers on this branch render a concrete Vec, and the caller that motivated it (dig.getCapsule) is not on this branch. It returns with its caller. 671/671 dig-node-core lib tests pass. Refs DIG-Network/dig_ecosystem#2071 Co-Authored-By: Claude <noreply@anthropic.com>
…le locally WIP for the #2071 method sweep — parked while PR #177 (the P0 envelope fix) is re-gated. Co-Authored-By: Claude <noreply@anthropic.com>
Rebased onto the #177 re-gate fixes. inclusion_proof is now always emitted (as "" when absent), so the capsule test's absence assertion becomes an empty-string assertion: a capsule window has no per-resource proof and says so explicitly rather than by omission. chunk_lens keeps its omission, and the guard returns with the caller that motivated it. The two are deliberately different: empty-vs-absent distinguishes two states of an APPLICABLE field (inclusion_proof), while chunk_lens is INAPPLICABLE to a whole module and absent says exactly that. Co-Authored-By: Claude <noreply@anthropic.com>
|
All three blocking + non-blocking findings are fixed in Both blocking findings were right1. The producer's rustdoc. Corrected. You are right that this is the identical mechanism the PR names as root cause, and leaving it in the first place a maintainer looks was the worst place to leave it. 2. Test gap — confirmed and closed. I reproduced your mutation exactly: Non-blocking, all three taken. The "verify per window" clause was false — The correction: this PR IS the outage fixYour narrower point is correct and I have adopted it — the proof hoist never broke the subdomains, because #179 cannot be the fix — the resolver never calls those methods. Every And this PR's fields alone restore it. I ran the deployed service worker's own Same bytes, same proof, same root, same So the deploy sequencing should go back to: merge + release #177 restores 671/671 lib tests pass. |
…le locally WIP for the #2071 method sweep — parked while PR #177 (the P0 envelope fix) is re-gated. Co-Authored-By: Claude <noreply@anthropic.com>
Rebased onto the #177 re-gate fixes. inclusion_proof is now always emitted (as "" when absent), so the capsule test's absence assertion becomes an empty-string assertion: a capsule window has no per-resource proof and says so explicitly rather than by omission. chunk_lens keeps its omission, and the guard returns with the caller that motivated it. The two are deliberately different: empty-vs-absent distinguishes two states of an APPLICABLE field (inclusion_proof), while chunk_lens is INAPPLICABLE to a whole module and absent says exactly that. Co-Authored-By: Claude <noreply@anthropic.com>
…le locally WIP for the #2071 method sweep — parked while PR #177 (the P0 envelope fix) is re-gated. Co-Authored-By: Claude <noreply@anthropic.com>
Rebased onto the #177 re-gate fixes. inclusion_proof is now always emitted (as "" when absent), so the capsule test's absence assertion becomes an empty-string assertion: a capsule window has no per-resource proof and says so explicitly rather than by omission. chunk_lens keeps its omission, and the guard returns with the caller that motivated it. The two are deliberately different: empty-vs-absent distinguishes two states of an APPLICABLE field (inclusion_proof), while chunk_lens is INAPPLICABLE to a whole module and absent says exactly that. Co-Authored-By: Claude <noreply@anthropic.com>
…le locally WIP for the #2071 method sweep — parked while PR #177 (the P0 envelope fix) is re-gated. Co-Authored-By: Claude <noreply@anthropic.com>
Rebased onto the #177 re-gate fixes. inclusion_proof is now always emitted (as "" when absent), so the capsule test's absence assertion becomes an empty-string assertion: a capsule window has no per-resource proof and says so explicitly rather than by omission. chunk_lens keeps its omission, and the guard returns with the caller that motivated it. The two are deliberately different: empty-vs-absent distinguishes two states of an APPLICABLE field (inclusion_proof), while chunk_lens is INAPPLICABLE to a whole module and absent says exactly that. Co-Authored-By: Claude <noreply@anthropic.com>
…le locally WIP for the #2071 method sweep — parked while PR #177 (the P0 envelope fix) is re-gated. Co-Authored-By: Claude <noreply@anthropic.com>
Rebased onto the #177 re-gate fixes. inclusion_proof is now always emitted (as "" when absent), so the capsule test's absence assertion becomes an empty-string assertion: a capsule window has no per-resource proof and says so explicitly rather than by omission. chunk_lens keeps its omission, and the guard returns with the caller that motivated it. The two are deliberately different: empty-vs-absent distinguishes two states of an APPLICABLE field (inclusion_proof), while chunk_lens is INAPPLICABLE to a whole module and absent says exactly that. Co-Authored-By: Claude <noreply@anthropic.com>
…le locally WIP for the #2071 method sweep — parked while PR #177 (the P0 envelope fix) is re-gated. Co-Authored-By: Claude <noreply@anthropic.com>
Rebased onto the #177 re-gate fixes. inclusion_proof is now always emitted (as "" when absent), so the capsule test's absence assertion becomes an empty-string assertion: a capsule window has no per-resource proof and says so explicitly rather than by omission. chunk_lens keeps its omission, and the guard returns with the caller that motivated it. The two are deliberately different: empty-vs-absent distinguishes two states of an APPLICABLE field (inclusion_proof), while chunk_lens is INAPPLICABLE to a whole module and absent says exactly that. Co-Authored-By: Claude <noreply@anthropic.com>
Task
Restore
*.on.dig.net. Every subdomain has been dark since the rpc.dig.net cutover(DIG-Network/dig_ecosystem#2071).
This PR IS the outage fix — measured, not argued
The re-gate concluded this PR is "conformance only" and that #179 is the restore. That is wrong,
and I can show it both ways. The gate's narrower point is right and I have adopted it: the proof
hoist (commit 2) never broke the subdomains, because
fetchVerifiedPostalways awaits window 0alone and keeps
first.inclusion_proof. But this PR is two commits, and the gate generalised fromcommit 2 to the whole PR. Commit 1 —
total_length/offset— is the outage.The live resolver calls exactly one RPC method. Every
rpcCallin the deployedsw.jsanddig-embed.js:No
getProof, nogetMetadata, nogetPublicManifest, nogetCapsule. #179 adds methods theresolver never calls, so it cannot restore the subdomains.
And adding only this PR's fields fixes it. Running the deployed service worker's own
fetchVerifiedPost+serveUrnarithmetic against the liverpc.dig.netresponse, twice — theonly difference between the runs is the three fields this PR adds:
Same bytes, same proof, same root, same
chunk_lens.const total = first.total_length >>> 0evaluates to
0when the field is absent, so the ciphertext lands in a zero-length buffer and theworker answers its own
404(sw.js:759) — HTTP 200 on the outer document throughout, which is whythis reads as "doesn't resolve".
Deploy expectation: merging and releasing this restores
*.on.dig.net. #179 does not, and is nota prerequisite.
What changed
content_window_envelopefor both producers — the locally-held read (build_result)and the peer fetch-through (
FetchedResource::content_result). A second implementation of ashared wire shape is what let them drift.
total_length, its ownoffsetandlength, an explicitnext_offset: nullwhen complete, and the
inclusion_proof— the latter always PRESENT, as""when the source hasnone, matching the Lambda's
unwrap_or_default()and openrpc's required["string","null"].chunk_lenskeeps its first-window-only rule, which both sources agree on.and stamped
source: "local", so without this an upgraded node would keep serving pre-fix windowsuntil they aged out — "deployed" would not imply "what clients receive".
SPEC.md§5.5.0 states the envelope as a contract, requires it to agree field-for-field withChunkObject, and records the 3 MiB window plus its three independent definitions (#2076).Verification
dig-node-corelib tests.if start == 0 || completeat the proof emit previously passed the fullsuite; it now fails two tests. The old multi-window test sampled only offsets 0 and WINDOW — first
and last, which that mutation also satisfies — so both now use
2*WINDOW+500and assert themiddle.
surface a windows-1..N defect. It now drives the resolver's real loop across three windows.
Bump
0.93.8 → 0.93.9 (patch). A defect fix restoring fields the published contract already required.
Refs DIG-Network/dig_ecosystem#2071