feat(dht): attach the untrusted mirror-coin pointer to the provider announce - #431
Conversation
5f8528a to
b6fda8c
Compare
Sequencing: this PR is HELD behind #430, deliberately — and the hold is not about qualityThis PR is green and gate-ready. Five required checks SUCCESS at It is held because #430 and this PR both bump Order: #430 first (it is the epic's last create blocker and its security gate is running now), then Two findings from this lane worth carrying, neither blocking1. The pointer is plumbed but NOT injected in production. 2. What the mutation evidence actually provedThree reds, each from reverting only its own fix on committed work:
The second is the one that matters most for NC-12: the fake pointer source moves its epoch and its |
b6fda8c to
2754385
Compare
loop-security — IN PROGRESS, not the verdictAudited head: Posting as I go so nothing is lost to a watchdog. Nothing CRITICAL or HIGH so far. 1. NC-12 requirement 1 — nothing downstream treats the received pointer as evidence: SATISFIED, structurallyGrepped the whole head tree.
There is no production reader at all. The field is written only as 2. The documented literal-construction bypass is NOT reachable here
Checked whether dig-node is that consumer. It is not:
So the unnormalized path exists upstream but has no caller here. 3. Peer-supplied bytes cannot be echoed unescaped on the RPC surface
The redirect / 4. Dependency tier — recounted independently, matches the lane's claimCounted from the resolved
One line each for the three bumped crates — so there is one Still to check: the |
loop-security: PASS — nothing CRITICAL, nothing HIGH, no LIVE vulnerabilityAudited head: Scoped as briefed: an additive, behaviour-preserving diff in NC-12, the four properties that actually matter1. The pointer tells a verifier WHERE TO LOOK, never WHAT THE COIN IS — SATISFIED STRUCTURALLY
The bypass that would matter is documented upstream. dig-node is not that consumer. And serde is the load-bearing detail: the normalization is attached to the type, not to one call site — 2. Absence must not degrade discovery — SATISFIED, and by identity rather than by resemblance
The one-off-variant probe applies here as the mixed case rather than the empty one — "one id has a coin, the rest do not" — and that is exactly what the test covers, with the unbonded id as the truthful control. 3. A hostile value cannot hurt the receiver — SATISFIED
4. The epoch re-announce — the property holds in the CODE, and the fixture is not vacuous
I checked the lane's claim about its own fixture rather than accepting it. It holds: Restart is fail-safe, not fail-stale: Everything else checked
Merge preconditions at this SHA, asserted by name: 14 checks SUCCESS ( What would have made this failNamed so the PASS is checkable rather than decorative:
None of the six is present. Defense-in-depth — NOT gating, for the injection follow-up childBoth belong to whoever builds the real (a) (b) Also unchanged and correctly out of scope, per the brief: the pointer not being injected in production, the obsolete reason at Verdict: PASS. No LIVE vulnerability. I do not merge, undraft or edit — this verdict returns to the orchestrator. |
PR #431 took 0.178.0 on main, so this branch's original bump to 0.178.0 was dropped by the rebase as 'already upstream' -- leaving no effective version increase. 0.179.0 is the first number past what main holds. Cargo.lock regenerated; CI runs --locked.
…424) (#434) * chore(mirror): open lane for the production broadcaster (#424) * feat(mirror): wire a production broadcaster for the mirror lifecycle `production_broadcaster()` was a literal `None`, so `sign_and_broadcast` refused before signing and both the spend-record writer (#411) and the mirror signer (#410) were inert. `ChainTransport::broadcaster()` is the narrow counterpart to the existing public `chain_source()`: it returns a broadcaster built on the ONE shared client, so `shared_client` stays `pub(crate)` and no caller gains the client itself. The seam keeps its single-derivation role and gains a three-way answer, so an unreachable chain is no longer reported as a switched-off flag. On a default install no broadcaster is constructed and no chain is dialed. Refs #424 * chore(mirror): fmt, fix the remaining open_signer test call site, bump to 0.178.0 * chore: drop the WIP lane marker from CHANGELOG * chore: bump to 0.179.0 after rebasing onto 3d15118 PR #431 took 0.178.0 on main, so this branch's original bump to 0.178.0 was dropped by the rebase as 'already upstream' -- leaving no effective version increase. 0.179.0 is the first number past what main holds. Cargo.lock regenerated; CI runs --locked. * fix(mirror): a PENDING mempool ack is not an accepted broadcast Four findings from the adversarial gate on #434, all downstream of an operator having already enabled live broadcast. MONEY. `ChiaQueryBroadcaster::broadcast` read `TxStatus::success`, which `chia_query`'s `ack_to_tx_status` sets true for BOTH status 1 (SUCCESS) and status 2 (PENDING). PENDING means the full node did not admit the bundle to its mempool — held for an unknown parent, or a fee below the floor — so the broadcaster returned `Ok(())` for a submission that never happened, contradicting the `Broadcaster` contract in the same file. The journal then recorded it and the intra-pass reservation stranded the funding coin against a spend the network was not holding. Fixed locally by reading the status NAME the crate already produces, through a pure `accepted_by_mempool`. That consumes a distinction `chia-query` makes rather than re-deriving one, so it is not a rival implementation. The crate's own conflation is filed as DIG-Network/chia-query#48 and is a release-first cascade, not this PR. HONESTY. `ChainUnreachable` promised a reachability claim nothing makes: the client build succeeds with zero peers because the default config's coinset fallback tier makes the peer requirement optional. Renamed to `ChainClientUnavailable` and documented for what it observes — a failed CONSTRUCTION. `Available` now says plainly that it means a broadcaster was built, not that a chain answers; unreachability is discovered at the broadcast, by name. The three-way shape and the per-pass re-read are kept unchanged, so a node that starts offline still recovers. TESTS. The default-install test claimed a control no test exercised. Its fixture is now actually driven through `production_broadcaster(.., true)`, which both makes that test non-vacuous and pins what `Wired` means. MESSAGES. The refusal named a closed ticket instead of the flag an operator can reach; it now names both reasons it can fire. Four string literals had lost their line continuations. Co-Authored-By: Claude <noreply@anthropic.com> * docs(mirror): name the PENDING harm at the reservation, and correct two stale comments The reservation comment's 'extended only on a broadcast that REACHED the mempool' was false at the previous head and is what the PENDING fix makes true; it now says so and names the durable consequence. chain.rs no longer claims the transport is reachable only as a SignedBundlePusher, which stopped being true when broadcaster() was added. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
DRAFT — DO NOT MERGE — gate round not yet returned.
Closes #422.
Takes the
dig-dht 0.15tier and attaches the untrusted mirror-coin pointerProviderRecord::unverified_mirror_coin_idto this node's DHT provider announce, refreshed oncollateral-epoch rollover.
The tier, measured from the resolved lock
cargo update -p dig-dht -p dig-download -p dig-peer-selector— every crate named explicitly,because a caret-compatible transitive is invisible to
cargo fetchand can leave two lines alivewhile cargo prints success. Counted from
Cargo.lock, not from the caret:dig-dhtdig-downloaddig-peer-selectordig-natdig-tlsdig-peerchia-protocol(2 lines) andchia-bls(5) are multi-line and unchanged fromorigin/main—counted on both locks. Pre-existing, not introduced here.
§2.4b: every other
dig-*/chia-*declaration in this crate was checked againstindex.crates.ioand is already at the latest published version. The one exception is deliberate and left alone:
dig-ipc-protocol = "=0.3.0"is an exact pin on a wire contract (0.3.1 exists; taking it is areviewed change, not a sweep).
The pointer
dig-node-coreholds no collateral-epoch knowledge of its own, so the pointer arrives through a seam:MirrorCoinPointers—epoch()plus a per-contentcoin_id_for(). Per content because amirror coin bonds one
(store, root, owner, epoch)tuple, so a single node-wide pointer would sendverifiers to a coin that does not advertise the capsule they asked about.
announce_provider_with_collateral.announce_inventory_idsandsync_inventorykeep their signatures and delegate withNone.DhtHandle::reannounce_on_epoch_rollover()compares the source's epoch to the epoch thepublished pointers were drawn for.
run_maintenancecalls it beforerepublish— after wouldleave last epoch's coin id published for a further whole interval.
The four properties #422 states hold: the claim is untrusted (NC-12, where to look, never what the
coin is); absence is a normal state that neither suppresses an announce nor reads as a fault; a
verifier's judgement is unchanged and rests on the coin's own evidence; and the pointer is refreshed
on rollover.
Blast radius
Checked by grep + direct read (per-worktree gitnexus not analysed; a ~10-minute index would have
competed with the build, and §2.0 permits the fallback). Every callers-of edit is additive —
announce_inventory_ids,sync_inventoryandDhtHandle::newkeep their signatures:announce_inventory_ids—tests/dht_integration.rs:406, unchanged.sync_inventory— no production caller outsidedht.rs;tests/holdings_wire.rsreferences it.DhtHandle::new—peer.rs:3004,neighbourhood_probe.rs:736,tests/holdings_wire.rs:455; allnow delegate to
with_mirror_pointers(.., None).run_maintenance—peer.rs:3198.dig_dht::ProviderRecordgained a field, so six struct-literal test fixtures inunion_locator.rs/module_reshare.rstakeunverified_mirror_coin_id: None.How it was verified
cargo test -p dig-node-core --lib --locked— 1012 passed, 0 failed.cargo clippy --workspace --all-targets --locked -- -D warnings— clean.reannounce_on_epoch_rollover→return 0(the "publish once, let republish refresh it" implementation)The rollover test's fake moves its epoch and its coin ids together, as a real rollover does — a
fake that kept one coin across the rollover could not tell a fresh read from a replay. It also calls
service.republish()mid-test and asserts the record unchanged, so the later change can onlyhave come from the re-announce.
Not done here, stated rather than hidden
The pointer source is not yet injected in production. The mirror coins live in
dig-node-service, and constructing aMirrorCoinPointersfor the real node reachesserver.rs,which PRs #423 / #430 own. This PR plumbs the seam end-to-end inside
dig-node-coreand leavesDhtHandle::newpublishing no pointer — a normal, fully supported state. No user-facing surfaceclaims a pointer is published, so nothing is made to lie.
Consequently
SPEC.md§25.6 stays accurate and is deliberately untouched (another lane owns it):a reader still MUST NOT infer that a mirror coin id reaches the DHT. One sentence at
SPEC.md:7962gives a now-obsolete reason — that the dig-dht 0.15 bump is blocked — and wants a follow-up edit by
whichever lane owns §25.
Version: workspace
0.176.0,dig-node-core0.63.0(minor — new public API, no behaviour changefor an existing caller).
Cargo.tomlandCargo.lockagree.