fix(mirror): a Bonded verdict must not rest on one uncorroborated chain read - #506
Conversation
Salvage anchor. Version bump only; the corroboration work follows. Co-Authored-By: Claude <noreply@anthropic.com>
Progress — #503 corroborated bond verdictBranch Done (pushed):
Blocked on: machine-wide ENOSPC (disk hit 0 bytes mid-run and truncated a source file, recovered from the pushed commit). Freed this lane's own Next action: |
`ChainBondVerifier::verify_against_chain` took its `ChainSource` from `ChainTransport::chain_source`, which is chia-query's router: with `coinset_fallback_enabled` it asks api.coinset.org FIRST and consults this node's dialled peers only when that read fails, and its own `ProviderInfo` records `trustless: false`. The four checks that promote a holder to `Bonded` are all internal consistency of a coin and its creating spend; none of them establishes chain MEMBERSHIP. A coin currying the real, public $DIG CAT puzzle around an invented parent satisfies every one of them, so a single source's word was enough to rank an attacker's peer at zero collateral cost. Verification-by-proof is not available -- `ChainSource` exposes no header, no merkle path, no inclusion proof, and a mirror coin is a CAT with no launcher. Corroboration is, and it is cheap here: the whole verdict reduces to two primitive reads, and `sage::peer_reads::PeerCorroboratedReads` already corroborates exactly those two over the node's own peers, inheriting `CORROBORATION_FLOOR` and `required_agreement`. This is a reuse and wiring change, not a second quorum. - new `sage::corroborated_source::CorroboratedChainSource`, a `ChainSource` over `PeerCorroboratedReads`. Three-way map on both reads; recomputes the coin id from the returned fields and refuses a mismatch; every read it cannot serve returns `Err`, never an empty `Vec` or `Ok(None)`, because an empty answer is an absence lie; `peak_height` maps "the peers did not agree" to `Err` rather than to `Ok(None)`, which means something else. - `ChainTransport::corroborated_chain_source` errs when no peer reads are attached and deliberately does NOT fall back to the router: falling through to one endpoint exactly when the peers failed to agree would let it overrule them. - `SPEC.md` §25.6a states the requirement normatively. Tests drive the chain half through a scripted peer sample: the bug (one peer vouching alone), the floor, an even split, a transport with no peer reads, a peerless source, the `Bonded` anti-vacuity control, and a permanent witness that the identical fabricated coin IS bonded by a single source. Closes #503
ee64283 to
9609e6d
Compare
Ready for the gate round — all 15 CI checks greenHead All six required contexts asserted BY NAME via
Non-required also green: Analyze rust/actions/js-ts, CodeQL, and all four package builds (deb amd64+arm64, msi, pkg). Local, with counts checked rather than exit status: 7 passed / 0 filtered ( Out of scope, stated rather than filed: the collateral census ( Not covered, stated rather than claimed: the |
|
IN PROGRESS — not the verdict. Correctness gate, fresh context, head Confirmed so far, by reading:
Still to check: no-fallback in |
loop-security — IN PROGRESS, not the verdictAudited head: Diff is small and reviewable: 8 files, 806 insertions — Axis 2 — independence: CLEARThe new path cannot be satisfied by one endpoint answering repeatedly, and I checked this at the draw rather than taking the module docs' word for it.
So corroboration here is genuinely irreducible to one source. Not a finding. Axis 1 — what an attacker must control to force
|
| answered | attacker peers needed |
|---|---|
| 5 | 4 |
| 4 | 3 |
| 3 | 3 |
| 2 | 2 (i.e. all of them) |
The exposure is the bottom row: silence lowers the denominator for free, so an attacker who lands 2 sybils in a sample and can outlast/suppress the rest reaches a full quorum at 2-of-2. That is inherited from PeerCorroboratedReads, not introduced here, and it is bounded by host-distinct discovery plus the 300s SAMPLE_LIFETIME redraw. Recording it as the shape of the residual risk, not as a finding against this PR.
Axis 4 — fail-closed direction: CLEAR on the four unsupported methods
coin_records_by_puzzle_hash (:170), coin_records_by_parent (:182), resolve_singleton_lineage (:193) and block_timestamp (:206) all return ChainSourceError::Unsupported. None returns Ok(vec![]) or Ok(None) — no absence lie. peak_height (:213) correctly maps PeerCorroboratedReads' "peers did not settle" None onto Err rather than onto ChainSource's Ok(None), which would have meant "this source has no peak" — a settled fact a caller may act on. That distinction is made explicitly at :207-212 and is right.
bond_verify.rs:409-414 reads the constructor's Err as Unverified, and chain_bond_verdict_and_coin maps every source Err to Unverified (:230, :253) and MirrorError::ChainUnavailable likewise (:260). No new path yields Bonded.
One genuine strengthening worth crediting: coin_bound_to (corroborated_source.rs:95) re-derives SHA256(parent|puzzle_hash|amount) and refuses an answer that is not about the requested coin. The bond path never asserted that itself, so this closes a gap rather than merely re-stating the layer below.
Still working: axis 3 (the stacked cache window), axis 5 (availability/demotion), axis 6 (fan-out), axis 7 (induced false absence).
|
orchestrator bc2a88 / loop-reviewer started on 9609e6d at 2026-09-02T17:32Z (fresh context, correctness gate) |
|
orchestrator bc2a88 / loop-security started on |
# Conflicts: # Cargo.lock # Cargo.toml
loop-reviewer — correctness gate STARTED (IN PROGRESS — not the verdict)Fresh-context correctness leg of the triple gate on PR #506.
Findings will be posted as inline threads as they are established; the verdict follows at the end. |
loop-security — audit STARTED (IN PROGRESS — not the verdict)Security leg of the triple gate for this PR. Auditing head Scope of this leg (custody-adjacent: a
Findings will be posted here as they are established, each marked IN PROGRESS; the verdict lands last as |
Adversarial gate: STARTEDThird leg of the triple gate on Refutation targets:
Verdict posted here as a separate comment when the evidence is in. I write no production code and will not merge or undraft. |
IN PROGRESS — not the verdict (confirmed items, head
|
loop-security — interim findings (IN PROGRESS — not the verdict)Head The headline is positive: the direction of this change is right
1. The corroboration floor is 2, and the reason it is 2 does not apply on this path — DEFENSE-IN-DEPTH
Attack: an attacker holding 2 of the 4 drawn peers wins any round in which the other two do not answer — both his peers return the fabricated coin record and its creating spend, The point: that floor was chosen for the replica sync path, where refusing a thin round re-creates a frozen replica — a self-inflicted denial. On the bond path refusing is free, because the fallback verdict is 2. The new SPEC clause is stronger than the code it describes — DOC ACCURACYSPEC.md's added paragraph says a 3. The whole corroborated path is DORMANT today, which bounds every finding above
This is why items 1 and 2 are not gating. It is also why they must not be lost: the short-circuit lifts itself the moment that accessor arrives, and bond_verify.rs:139-144 already carries a list of things that MUST land in that same change. Items 1 and 4 belong on that list. Still to check: the amplification figure when the path goes live, and whether the nested |
MichaelTaylor3d
left a comment
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
loop-reviewer — inline findings (IN PROGRESS, not the verdict)
Two findings anchored below. The verdict comment follows separately.
loop-security — interim findings, part 2 (IN PROGRESS — not the verdict)Head 4. Amplification: 16 third-party HTTPS reads becomes up to 64 peer queries, and a Sybil can make them UNCACHEABLE — MEDIUM, dormantThe chain per inbound locate:
So up to 8 x 2 x 4 = 64 peer chain queries per locate, against 8 x 2 = 16 outbound The part that is a genuine regression is the memoisation, and it is cheap to trigger:
Attack: a stranger publishes 8 provider records carrying 8 distinct fabricated coin ids, and holds one peer in this node's drawn sample. Every locate for that content pays the full 64 sequential peer queries, and none of the verdicts is ever memoised, so the cost repeats on every locate indefinitely. Before this PR the same 8 records cost 16 coinset reads once, because coinset returned a definite The fix is not to cache 5. The bridge drops
|
loop-security — VERDICT: PASSAudited head: No gating security defect in this diff. It closes a real forgery surface and it fails closed in the direction that matters. Five non-gating findings, ranked, all recorded in detail in the two interim comments above; none blocks the merge. Why PASSThe premise is correct and the fix is load-bearing. Fail-closed discipline holds at every branch I could find. Only-one-source answers, so Corroboration is keyed on the coin's identity, not on existence. No custody widening. No new The denial direction is closed by the lattice, which I checked rather than assumed. Findings — ranked, none gating
One hypothesis raised and falsified, recorded so it is not re-derived: I suspected the production nesting ( The one substantive trade, stated plainlyThis PR moves the bond verdict from one well-run centralized endpoint to two-of-four randomly drawn untrusted peers. That is contract-aligned — NC-12 holds every dialled peer untrusted and makes agreement the mechanism — and the router treated coinset as authoritative rather than corroborated, which is the hole. But the trade is only net-positive to the extent the floor is meaningful, which is exactly why finding 1 should not be lost behind the dormancy in finding 3. MechanicsEverything was read from git objects ( |
Adversarial gate: HOLDSHead 1. "Corroboration makes a forged coin id unBondable" -- HOLDSThe second source is genuinely a second source. The strongest attack I had is false, and I checked it directly. 2. Failure direction -- HOLDS. No arm returns
|
IN PROGRESS — revert-proof MEASURED (not the verdict)Two runs in an own detached worktree at Baseline — Mutation A — the call site reverted to the pre-fix single-source read. In my worktree only, The suite is fully green with this PR's decision reverted. That confirms the inline finding on This is not a claim that the pair is vacuous — it is not. Also confirmed clean, so nobody re-derives them:
|
MichaelTaylor3d
left a comment
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
One further finding, measured rather than read. Non-gating.
MichaelTaylor3d
left a comment
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
loop-reviewer — VERDICT: CHANGES-REQUIRED
Head reviewed: 1b44d9bf998921e50e8e516c39cb266ab0f735ca (resolved from the remote, not from the
dispatch brief). Correctness leg of the triple gate, fresh context.
The change itself is correct and I would pass it. The one blocker is merge sequencing: the branch
version is now BELOW main, and the green version gate is measuring a stale base.
Ranked
- GATING —
Cargo.toml:35, version0.251.0is belowmain's0.252.0.#500and#507
landed after the 18:54Z merge.Check version incrementreports pass only because it compares
against the merge-basebfaa79d(0.247.1). Rebase onto currentmain, re-read the version from
the file on disk, and take the next free value above0.252.0. Watch the rebase output for
dropping— an identical bump commit is discarded silently withRC=0. Thread left OPEN; it is
the only one. - Non-gating, resolved —
bond_verify.rs:408has no guard. Reverting this PR's decision to
.chain_source(...).awaitleaves the suite at 7 passed / 0 failed / 0 filtered. The repo
already has the idiom that would catch it (chain.rs:1404, aninclude_str!guard over the
module's own source); it must be scoped to the function body, since the doc comment above names
corroborated_chain_sourcethree times and a file-widecontainsstays green after a revert. - Non-gating, resolved —
corroborated_source.rs:133. Disabling the coin-id binding rejection
also leaves 7/7 green; the module has no#[cfg(test)]and nothing namescoin_bound_to.
Benign-fixture shape — every fixture is self-consistent, so the check never fires.
What I verified rather than took on trust
- Independence is real.
server.rs:2179→server.rs:596→service.rs:183
(with_peer_reads) →chain.rs:356(DialedPeerSample::mainnet) →dialed.rs:246
(assemble_distinct_sampleoverconnect_random_peer_excluding). Distinct, separately dialled
full nodes;api.coinset.orgis not in the path and there is no fallback to the router. - One source is not
Bonded, and disagreement fails closed.still_usable(dialed.rs:236)
requireslive >= CORROBORATION_FLOORandlive >= peers.len(), so attrition forces a redraw
rather than letting a sample ratchet to the two most eager repliers. Below the floor or on
disagreement →Err→Unverified, neverUnbondedand neverBonded. - The three-state distinction is preserved where it decides something. Every unservable read
returnsErrrather than an emptyOk(:216,:229,:240,:266), and "peers did not agree"
maps toErr, notOk(None)(:252). Those are different claims. - The differential pair is discriminating, not an outcome assertion. The same
fabricated_bond()
fixture yieldsBondedthrough one source andUnbondedthrough four, with
agreeing_peers_do_bond_a_genuine_coinas a real anti-vacuity control. The doubles are expressive:
distinct per-peer views and ids, and a peer that can refuse. - No new agreement mechanism was invented — the correct call under centralize-rival-implementations.
- The merge added nothing of the lane's own;
bfaa79d..1b44d9bis exactly the 8 declared files. - SPEC.md is normative, correctly placed in §25.6a, with no superseded wording surviving.
- §2.4b clean — every
dig-*/chia-*dep is at latest,chia-*uniformly on 0.36. - §2.5 clear pass on the new module.
Also honest and accurate: disclosed limitation 1 (the call site is unreachable through verdict_for
until #501 lands, because of the short-circuit at bond_verify.rs:305) checks out exactly as written.
Unresolved threads: 1 (the version). Merge is unblocked once that is rebased and re-bumped and my
thread is resolved; the other two legs of the gate report separately.
Mechanics: read via git objects plus an own detached worktree at C:/tmp/worktrees/e93-rev506, now
removed. dn-503, every sibling worktree and the primary checkout were never written to. gitnexus was
not used — its registered dig-node index is hundreds of commits behind and returns a false-safe
impactedCount: 0; radius was established by ripgrep plus direct reads and is reported as such.
|
orchestrator
Chain log: |
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
… chain source keeps chia-query's panic backstop dig-node#513 items 1 and 5, both of which need the file #506 created. Item 1 -- `CORROBORATION_FLOOR` is two, so two agreeing peers were a full quorum for a `Bonded` verdict. That constant is two for a LIVENESS reason belonging to the sync path: it writes the wallet's replica, and demanding more peers than a thin network offers is what froze a user's node for hours. The bond path writes nothing -- a refused round yields `Unverified`, the tier every record occupies with no verifier installed -- so refusing is free there and the floor can be higher. `BOND_CORROBORATION_FLOOR = 3` is therefore a SEPARATE constant, applied through `tally_with_floor` and selected by the bond path via `CorroboratedChainSource::requiring_corroboration`. It binds in BOTH dimensions -- answers and agreement -- so a wide round in which two voices agree does not buy its way past it. The cache is not consulted above the default floor: a cached row records the answer a round settled on, never how many peers settled it, and the sync path fills that cache at two. Item 5 -- this adapter replaces `chia-query`'s bridge on the bond path and had dropped its `guard_panics` backstop, keeping only the runtime-flavour check. That catches the misuse we can name and nothing else; a panic crossing a `ChainSource` method would unwind out of a `block_in_place` inside a locate. Restored, asserted through `block_on` rather than on the helper, so deleting it from the path turns the test red. Item 2 (memoising `Unverified`) stays rejected by design: `VerdictCache::remember` refuses `Unverified` and the path is bounded by `ReadAdmission` instead. Also merges origin/main and bumps dig-mirror-coin 0.8 -> 0.9 (§2.4b). Refs dig-node#513 Co-Authored-By: Claude <noreply@anthropic.com>
… chain source keeps chia-query's panic backstop dig-node#513 items 1 and 5, both of which need the file #506 created. Item 1 -- `CORROBORATION_FLOOR` is two, so two agreeing peers were a full quorum for a `Bonded` verdict. That constant is two for a LIVENESS reason belonging to the sync path: it writes the wallet's replica, and demanding more peers than a thin network offers is what froze a user's node for hours. The bond path writes nothing -- a refused round yields `Unverified`, the tier every record occupies with no verifier installed -- so refusing is free there and the floor can be higher. `BOND_CORROBORATION_FLOOR = 3` is therefore a SEPARATE constant, applied through `tally_with_floor` and selected by the bond path via `CorroboratedChainSource::requiring_corroboration`. It binds in BOTH dimensions -- answers and agreement -- so a wide round in which two voices agree does not buy its way past it. The cache is not consulted above the default floor: a cached row records the answer a round settled on, never how many peers settled it, and the sync path fills that cache at two. Item 5 -- this adapter replaces `chia-query`'s bridge on the bond path and had dropped its `guard_panics` backstop, keeping only the runtime-flavour check. That catches the misuse we can name and nothing else; a panic crossing a `ChainSource` method would unwind out of a `block_in_place` inside a locate. Restored, asserted through `block_on` rather than on the helper, so deleting it from the path turns the test red. Item 2 (memoising `Unverified`) stays rejected by design: `VerdictCache::remember` refuses `Unverified` and the path is bounded by `ReadAdmission` instead. Also merges origin/main and bumps dig-mirror-coin 0.8 -> 0.9 (§2.4b). Refs dig-node#513 Co-Authored-By: Claude <noreply@anthropic.com>
…s the panic backstop dig-node#513 items 1 and 5, both of which need the file #506 created. Item 1 -- `CORROBORATION_FLOOR` is two, so two agreeing peers were a full quorum for a `Bonded` verdict. That constant is two for a LIVENESS reason belonging to the sync path: it writes the wallet's replica, and demanding more peers than a thin network offers is what froze a user's node for hours. The bond path writes nothing -- a refused round yields `Unverified`, the tier every record occupies with no verifier installed -- so refusing is free there and the floor can be higher. `BOND_CORROBORATION_FLOOR = 3` is therefore a SEPARATE constant, applied through `tally_with_floor` and selected by the bond path via `CorroboratedChainSource::requiring_corroboration`. It binds in BOTH dimensions -- answers and agreement -- so a wide round in which two voices agree does not buy its way past it. The cache is not consulted above the default floor: a cached row records the answer a round settled on, never how many peers settled it, and the sync path fills that cache at two. Item 5 -- this adapter replaces `chia-query`'s bridge on the bond path and had dropped its `guard_panics` backstop, keeping only the runtime-flavour check. That catches the misuse we can name and nothing else; a panic crossing a `ChainSource` method would unwind out of a `block_in_place` inside a locate. Restored, asserted through `block_on` rather than on the helper, so deleting it from the path turns the test red. Item 2 (memoising `Unverified`) stays rejected by design: `VerdictCache::remember` refuses `Unverified` and the path is bounded by `ReadAdmission` instead. Also merges origin/main and bumps dig-mirror-coin 0.8 -> 0.9 (§2.4b). Refs dig-node#513 Co-Authored-By: Claude <noreply@anthropic.com>
…nputs (#505) * chore(wallet): open the #502 lane -- bound total reservation hold Salvage anchor for the dig-node#502 lane. Version assigned 0.251.0 (origin/main is 0.247.0; 0.248-0.250 are held by sibling lanes). Co-Authored-By: Claude <noreply@anthropic.com> * test(wallet): pin the total reservation hold a repushed bundle may take Four failing db-layer tests for dig-node#502, plus the constant they measure against. `MAX_RESERVATION_HOLD_MS` is defined as a multiple of `RESERVATION_TTL_MS` in one place so the two cannot drift; the TTL itself is unchanged. The acceptance test steps by less than a TTL past the cap and asserts its own iteration count: a one- or two-push fixture is satisfied by the unfixed code, because the first hold has not lapsed yet. Co-Authored-By: Claude <noreply@anthropic.com> * fix(wallet): bound the total hold a repushed bundle may take on its inputs `reserve_spend` re-armed `expires_at` to `now + RESERVATION_TTL_MS` on every push of a given transaction id, so a caller re-pushing the same signed bundle more often than the TTL renewed its hold forever and the inputs never returned. That is the lockout failure the TTL's own doc names as the worse of the two, reachable without a single dishonest answer. Two composed bounds, neither of which shortens the TTL: * A TOTAL cap anchored on the FIRST push. `MAX_RESERVATION_HOLD_MS` is defined as `6 * RESERVATION_TTL_MS` in one place, so lengthening the TTL scales the cap and the two cannot drift. `submitted_at` is not in the upsert's `DO UPDATE SET` list, so the stored value is a stable anchor; a test pins it. * A reason-conditional re-arm. `chain::refusal_forecloses_a_later_push` names the four CLVM-execution / cost refusals that complain about the bundle's own contents and that no better-synced node can turn into an acceptance. Those still HOLD -- the verdict is height-dependent, so this crate declines to trust one node's view of it -- but they may not RENEW the hold. Everything else extends, including an unrecognised reason, an `Err`, and a bare verdict. The two compose into the gate's "every observed refusal was foreclosing" case without a per-attempt history, because a re-push may never move the deadline EARLIER: if every attempt is non-extending the deadline never leaves the first `submitted_at + TTL`, and an extending attempt's grant survives every later one. The clamp lives in the SQL so it is atomic against the stored anchor; a read-then-write above this layer would race two concurrent pushes. `coin_reservations`' `ON CONFLICT(coin_id) DO NOTHING` first-claim-wins rule is untouched and pinned by a test. Closes #502 Co-Authored-By: Claude <noreply@anthropic.com> * chore: bump to 0.256.0, clear of #506's 0.251.0 * fix(wallet): drop the reason-conditional re-arm, keep the total-hold clamp The adversarial gate on #505 refuted the reason-conditional half and it is removed in full: VIEW_DEPENDENT_BUNDLE_CONTENT_REFUSALS, refusal_forecloses_a_later_push, attempt_may_extend_the_hold, the PendingTransactionRow::may_extend_expiry field and the CASE arm in reserve_spend. The four names it listed -- GENERATOR_RUNTIME_ERROR, BLOCK_COST_EXCEEDS_MAX, INVALID_BLOCK_COST, INVALID_SPEND_BUNDLE -- do not identify a bundle no destination is holding. push_tx relays to up to three destinations and only the LAST answer returns, so such a refusal from the last says nothing about the first, which may have admitted and gossiped the bundle. The removed code freed inputs up to 550s earlier than main for a bundle that lands, with no attacker: the double-spend direction #497 exists to close. What ships is the clamp alone: expires_at is bounded by submitted_at + 6 * RESERVATION_TTL_MS. The outer MAX is retained because a non-monotonic clock is the one case that can still drive an incoming deadline below a live one, and shortening a live hold is the dangerous direction. SPEC.md 18.9a gains the total-hold bound as a normative clause: without it a reimplementation built from the spec as written reproduces the unbounded re-arm this change fixes. Three limitations are now stated in MAX_RESERVATION_HOLD_MS' doc and two are pinned by tests: the bound is on CONTINUOUS hold and a re-push after the prune gets a fresh anchor; a bundle whose timelock matures past the cap has its inputs freed while the network genuinely still holds it; and inside the last TTL before the cap a re-push buys strictly less than a full TTL. The clamp also fails OPEN under an absurd clock, since SQLite promotes integer overflow to REAL. Refs #502 Co-Authored-By: Claude <noreply@anthropic.com> * chore: renumber to 0.252.8, under the MSI ProductVersion ceiling (#521) * style(wallet): rustfmt the two reserve_spend test call sites cargo fmt wanted the multi-line call form at both boundary tests. Formatted those two files only; the workspace-wide check is now clean at zero diffs. Co-Authored-By: Claude <noreply@anthropic.com> * docs(wallet): drop a comment left behind by the reverted re-arm field The comment sat on `reserved_coin_ids`, which IS assembled from a stored table and has no `true`. It described `may_extend_expiry`, the bool this branch removed in fce2358, and pointed at a field doc that no longer exists. Also tighten SPEC 18.9a: a re-push does not unconditionally 'update the deadline' -- at or past the cap, and under a backwards clock, it correctly leaves the deadline unchanged. State the re-arm as subject to the bound. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
…ion (#501) * chore(mirror): open the peer-binding lane for #473 Stub anchor so a session cap cannot lose the lane. Activation of the bond verifier follows. Co-Authored-By: Claude <noreply@anthropic.com> * feat(mirror): activate bond promotion on the coin's own peer declaration The bond verifier shipped in #467 was inert by construction: `peer_declaration` returned `NotReadable` unconditionally, so `Bonded` was unreachable, `verdict_for` short-circuited before any chain read, and every holder got one verdict. That was the correct posture while nothing could bind a coin to a claimant -- a coin proves that *a* bond exists and never that the peer offering the record holds it, and promoting on the chain half alone would rank a stranger republishing a public coin id first at zero collateral. `dig-mirror-coin` 0.8.0 supplies the missing half. A coin's owner may declare `dig-peer:<64-hex>` in the memo tail; only the owner's key can produce the spend that writes it, so the term is an owner attestation carried by executed on-chain code. `peer_declaration` now delegates to that crate's typed accessor rather than parsing the tail here, because a second parser for a security-critical format makes a divergence a silent authorization difference instead of a compile error. Promotion now requires BOTH bindings: coin -> content via `MirrorCoin::advertises`, and coin -> peer id via the declaration. `PeerDeclaration::NotReadable` is removed. It described a situation that no longer exists, and a variant nothing constructs is a state the type claims to model and does not. The address-substitution residual, resolved ---------------------------------------------------------------- The declaration binds coin -> peer id, never peer id -> address, so a record carrying an honest holder's peer id, that holder's real coin id and an ATTACKER's addresses satisfies every check here and IS promoted. SPEC 25.6a previously required closing that with an authoritative-record restriction, on the stated grounds that "a dialler is not by itself a backstop, because peer ids are derived from the presented certificate rather than pinned against the dialled identity". That premise is false for every path dig-node dials on: the download path makes the record's own `provider_peer_id` the `PeerTarget` pin, dig-nat passes it to dig-tls, and the verifier fails the handshake with `peer_id mismatch: expected .., got ..`. dig-peer re-checks after connect, and fetched content is merkle-verified against the caller's own requested root regardless. The attacker buys a refused connection, not a redirected reader. The restriction as written is also not implementable at this layer, and that is worth recording rather than rediscovering: dig-dht really does keep authoritative and hearsay records in two separate stores, but erases the distinction in `merge_dedup_by_provider` before `find_providers` returns, and a locator restricted to authoritative records would return almost nothing -- that store holds keys this node is k-closest to, not content it wants. What this layer owes instead is a BOUND, and it is added here: at most one record is promoted per claimed peer id, so one stolen identity cannot spend the whole verified budget. A duplicate falls back to the baseline tier it would have occupied with no verifier at all, never below it, so the lattice stays credit-only. Also corrects `peer.rs`'s note asserting no dial pins a peer id. The narrow fact behind it -- dig-gossip's legacy rustls outbound does not pin, and every `expected_peer_id` there is `#[cfg(test)]` -- is true; the generalisation to every dial was not, and dig-node never dials on that path. Closes #473 Closes #466 Co-Authored-By: Claude <noreply@anthropic.com> * docs(spec): 25.6a states the pin as the remedy, and two status bullets stop lying Three normative corrections in the section a reimplementation of the bond layer would be built from. No behaviour changes. 25.6a required an authoritative-record restriction and dismissed the alternative because "a dialler is not by itself a backstop, since peer ids are derived from the presented certificate rather than pinned against the dialled identity". That was FALSE. It generalised one true narrow fact -- dig-gossip's legacy rustls outbound does not pin, and every `expected_peer_id` there is test-only -- into a claim about every dial. dig-node never dials on that path: the download path makes a record's own `provider_peer_id` the pinned dial target, dig-nat passes it to dig-tls, and the verifier refuses the handshake on a mismatch. The restriction the clause preferred is also not available at the layer that ranks, and requiring it as though it were is worse than not requiring it. A DHT keeps attributed and hearsay records apart but flattens them into one untagged list when answering a lookup, and a reader's records for content it wants are overwhelmingly hearsay -- the attributed store covers the keys a node is closest to, not what it fetches. Restricting the locator to attributed records would return almost nothing. What the ranking layer owes instead is now stated as a MUST: at most one record promoted per claimed peer id per locate, so one stolen identity cannot occupy every promoted slot on the strength of a single bond. Credit-only is explicitly preserved. The two status bullets were separately stale. Verification is no longer inert. And the DHT pointer IS attached -- the `dig-dht ^0.13`/`0.15` semver split that blocked it is resolved, the announce passes a coin id (`dht.rs:493`), and `SnapshotMirrorPointers` is installed at `server.rs:2169`. A bullet saying a mirror coin id never reaches the DHT would have made the whole verification path read as unreachable. Refs #473 Co-Authored-By: Claude <noreply@anthropic.com> * fix(mirror): bound promotion by peer IDENTITY, and write this node's own declaration Three findings from the adversarial gate, one of them a live zero-cost attack on the bound added earlier in this branch. The bound was keyed on the raw wire string ---------------------------------------------------------------- `promoted_peers` used `record.provider_peer_id` verbatim, while every check that GRANTS a promotion is case-insensitive: the coin's declaration compares 32 decoded bytes, and the TLS pin compares 32 bytes of certificate hash. A peer id is fixed-length hex, so one identity has many spellings. So a stranger answering one lookup could return eight records carrying an honest holder's peer id in eight different hex cases, each with its own addresses. Each passes `advertises`, each passes `declares_peer`, each is a distinct `String` -- eight promotions and eight chain reads, consuming the whole `MAX_VERIFIED_PER_LOCATE` budget on the strength of one bond the attacker does not hold. Exactly the outcome the bound was written to prevent. Both the bound and `VerdictKey`'s claiming-peer component are now keyed on the ASCII-lowercased id. dig-dht applies this same normalisation to the neighbouring `unverified_mirror_coin_id`, for the reason its own doc gives: without it "dedup and equality would split on presentation". The wire-level gap is filed as DIG-Network/dig-dht#27 -- it also affects self-exclusion and the union address merge, both pre-existing. The regression test was vacuous when first written, and that is worth recording: with the honest holder LAST in the slate, a promoted respelling and a baseline one land in the same position under a stable sort, so it passed with the fix reverted. Moving the honest record between the two spellings makes the behaviours differ. Now revert-proven -- reverting the fix fails exactly one test, with the attacker's respelling ahead of the honest holder. A coin this node creates now names this node ---------------------------------------------------------------- `MirrorAdvertisement` gained the field, and dig-node had to answer it. A coin that declares nobody can never be promoted by any reader, so a node creating one pays collateral for a claim nothing can credit to it -- the feature would have been vacuous for every coin this node makes. `Node::own_peer_id` is threaded to the create, re-read per pass rather than captured at spawn, because the mirror task starts beside the peer network rather than after it. `None` still creates the coin and warns; refusing would leave a node unable to bond at all before its network is up. Two stale claims ---------------------------------------------------------------- `verdict_for`'s own doc still said "nothing is promoted today" and "No chain is read at all", in the commit that makes both false. And `peer.rs:2475` was an uncorrected twin of the dialler claim fixed at `:863` -- same refuted assertion, same file, in the code that configures the gossip pool. Fixing one copy of a false normative claim and leaving the other is how it comes back. Refs #473 Co-Authored-By: Claude <noreply@anthropic.com> * chore(mirror): salvage fix-lane WIP from the 15:11Z session cap -- uncompiled Uncommitted work left in the dead lane's worktree (7 files, +753/-41). Never compiled or tested by the lane that wrote it; the resuming implementer verifies it first. Co-Authored-By: Claude <noreply@anthropic.com> * fix(mirror): restore the spaces a lost continuation baked into six operator messages Six operator-facing literals carried the source indentation of a `\`-line-continuation that had been collapsed away, so an operator reading the log saw a 14-18 space gap in the middle of a sentence. The text is what was always meant; only the run is removed. Found by scanning every literal in the mirror modules rather than the one site the review named -- the reviewer reported it in `lifecycle.rs`, and the defect was actually in `advertise.rs` (3), `pass.rs` (2) and `runner.rs` (1). A defect class named at one site is not a defect class swept. Co-Authored-By: Claude <noreply@anthropic.com> * style(mirror): rustfmt the salvaged hunks The salvage commit was written into the tree without ever being built or formatted, so two lines it introduced were over width. Formatting only -- no behaviour change. Co-Authored-By: Claude <noreply@anthropic.com> * docs(spec): 25.6a states the aggregate read bound, and stops promising the eviction we removed Two coherence gaps between 25.6a and the code that now ships under it. The per-locate read bound was specified; the AGGREGATE bound was not. A per-locate ceiling bounds nothing on its own, because the gate admitting a locate is per-requestor over self-minted identities -- an adversary multiplies the ceiling by as many identities as it cares to mint. The clause now requires both limits the implementation holds (a process-wide verification budget and a per-claimant distinct-unproven-coin ledger), requires them to be consulted before the chain is touched, and states that exhaustion degrades to `unverified` rather than refusing service. The eviction clause said overflow "MUST evict rather than clear". The implementation deliberately does less than that: an `unbonded` is refused admission to a cache full of live entries rather than allowed to displace a `bonded`, because `unbonded` is the verdict a stranger elicits for free and per-insert eviction is therefore paced by the attacker. The clause said the code did something it no longer does. Also documents a `clippy::too_many_arguments` allow on `admitted_verdict_for`: its parameter list mirrors `verdict_for`'s exactly so that a transposition of one of the four opaque 32-byte arguments stays visible at the call site. Co-Authored-By: Claude <noreply@anthropic.com> * fix(mirror): the bond path takes its own corroboration floor and keeps the panic backstop dig-node#513 items 1 and 5, both of which need the file #506 created. Item 1 -- `CORROBORATION_FLOOR` is two, so two agreeing peers were a full quorum for a `Bonded` verdict. That constant is two for a LIVENESS reason belonging to the sync path: it writes the wallet's replica, and demanding more peers than a thin network offers is what froze a user's node for hours. The bond path writes nothing -- a refused round yields `Unverified`, the tier every record occupies with no verifier installed -- so refusing is free there and the floor can be higher. `BOND_CORROBORATION_FLOOR = 3` is therefore a SEPARATE constant, applied through `tally_with_floor` and selected by the bond path via `CorroboratedChainSource::requiring_corroboration`. It binds in BOTH dimensions -- answers and agreement -- so a wide round in which two voices agree does not buy its way past it. The cache is not consulted above the default floor: a cached row records the answer a round settled on, never how many peers settled it, and the sync path fills that cache at two. Item 5 -- this adapter replaces `chia-query`'s bridge on the bond path and had dropped its `guard_panics` backstop, keeping only the runtime-flavour check. That catches the misuse we can name and nothing else; a panic crossing a `ChainSource` method would unwind out of a `block_in_place` inside a locate. Restored, asserted through `block_on` rather than on the helper, so deleting it from the path turns the test red. Item 2 (memoising `Unverified`) stays rejected by design: `VerdictCache::remember` refuses `Unverified` and the path is bounded by `ReadAdmission` instead. Also merges origin/main and bumps dig-mirror-coin 0.8 -> 0.9 (§2.4b). Refs dig-node#513 Co-Authored-By: Claude <noreply@anthropic.com> * fix(mirror): adopt dig-mirror-coin 0.9.0 in the lock and correct the WalletDb test import The manifest already declared "0.9"; the lock still resolved 0.8.0, so the two disagreed. Resolve the lock to 0.9.0 so the declared and resolved versions agree. The corroborated_source test module imported WalletDb from a `crate::wallet_db` path that does not exist; the type lives at `crate::sage::db::WalletDb`. Co-Authored-By: Claude <noreply@anthropic.com> * build(deps): resolve dig-mirror-coin to 0.9.0 after the main merge The manifest declares "0.9"; the post-merge lock carried 0.7.0 from main. Re-resolve so declared and resolved agree on one line. Co-Authored-By: Claude <noreply@anthropic.com> * style(wallet): order the corroborated_source test imports as rustfmt wants The only `cargo fmt --all -- --check` diff on this branch: `db` must precede `peer_reads`. Fixed by hand rather than with `cargo fmt --all`, which has rewritten thousands of untouched lines on a sibling branch. * fix(mirror): supply a peer id in the create fixtures the identity guard now refuses Three tests went red on this branch for one reason: `create` now refuses before selecting any coin when the node has reported no peer id, because a coin naming no peer locks collateral for an epoch that no reader could ever credit. The fixtures predate that guard and passed `None`, whose comment ("what a node writes before its peer network is up") described a state that is no longer a supported input to a create. Each of the three now passes a well-formed id built as `"a1".repeat(32)`, so its length is right by construction rather than by counting 64 characters in a literal. The guard itself is unchanged and correct: it fails closed on money, which is the direction a wrong answer should fail in. The fourth call site keeps `None` deliberately. `an_all_rejected_value_refuses_and_spends_nothing` refuses at the advertisement guard, which returns before the identity guard is consulted, so its `None` is never reached. That test previously asserted only `is_err()`, which this PR's second early return makes ambiguous -- it would pass just as happily if the identity guard were reordered ahead of the URL one, leaving the URL guard it exists for unexercised. It now names the expected cause. * fix(mirror): guard the advertise literals a test could not reach The earlier pin for this defect class was real but covered the wrong module. `the_refusal_messages_read_as_sentences` drives `declaration_for_create`, so it asserts over `lifecycle.rs`'s own two refusal literals -- which were never the corrupted ones. The three genuinely broken runtime lines are inline `tracing` literals in this file with no test reachability at all, so they sat behind a green test that appeared to cover them. That is worse than the original defect, because it looks discharged. The two rejection reasons move into `rejection_reason()` and the two info lines into `ADVERTISING_AT_CONFIGURED_URLS` and `nothing_publishable()`, so a test can reach the rendered text. `nothing_publishable` is a function rather than a const because it names the environment variable and `concat!` cannot take a const; spelling the variable a second time as a literal would be a second source of truth for the same name. The walk is exhaustive BY CONSTRUCTION: a match maps each `Rejection` variant to the name a failure prints, so a new variant fails to compile until it is named in the walk. `rejection_reason`'s own match would force a new variant to be GIVEN a message, but nothing would force that message into the sweep meant to check it -- a gate over an enumeration can only check the enumeration it was handed. * test(mirror): control row 2 so its Unverified is attributable to the declaration The test claimed row 2 "differs only in the declared peer id", which was the reviewer's condition for it proving anything. On the fixture as built it does not: row 1 is minted by wallet(3) and row 2 by wallet(4), so they differ in owner puzzle hash AND declared peer id. That matters because `verdict_for` reaches `Unverified` from two disjoint places -- the chain half producing no coin, and `PeerDeclaration::Silent` at the final match. A row-2 coin malformed anywhere in the chain half would satisfy the assertion while proving nothing about the declaration: the same vacuity already closed for row 3 by its fourth row, and left open for row 2. The distinct owners are load-bearing and stay: `creating_spend` derives a coin's parent from (owner, asset, amount), so one wallet cannot publish two same-amount advertisements without the second overwriting the first. So rather than collapsing the rows, row 2's coin is asked the question it should answer positively -- same coin, same root, claimant `stranger` -- and must return `Bonded`. That proves the entire chain half passes, leaving the declaration as the only thing row 2 can be attributable to. The doc no longer states one-field difference as a property of the fixture. * docs(mirror): keep configured_urls' doc attached to configured_urls Lifting the message helpers put them BETWEEN `configured_urls`'s doc comment and its signature, so the doc block silently reattached to `rejection_reason` and the public function was left undocumented. Nothing catches this: it compiles, rustfmt and clippy are clean, and the rendered docs simply describe the wrong item. Moved the helpers above the doc block instead. * test(mirror): derive the guard's count from Rejection::ALL and cover the warn wrapper My own comment overclaimed, in the direction this guard exists to prevent. The exhaustive match is on the TYPE, so a new `Rejection` variant genuinely cannot compile without being named -- but naming is not walking. Add a variant, add the two match arms, and the array literal still compiles, `lines.len()` is still 4, and the hard-coded `assert_eq!(lines.len(), 4)` still passes while the new operator-facing message ships unguarded. A literal count does not merely fail to prevent that; it cements it. The walk is now driven from `Rejection::ALL`, declared beside the enum, with the expected count DERIVED as `Rejection::ALL.len() + 2` so the walk and the list cannot drift. `ALL`'s doc states what is actually enforced -- the match forces a developer into this module and forces the variant to be given a message; nothing forces it into the array, and the derived length is what ties them together -- rather than repeating the stronger claim. Also folds in a line that was outside the walk while the guard's name claimed `every`: the warn wrapper is operator-facing prose in its own right, so it becomes `not_advertised()` and the walk asserts the whole rendered sentence rather than the reason fragment it embeds. * fix(mirror): drop the stale None declared_peer field the merge duplicated The merge with origin/main spliced this branch's new `Some(declared_peer)` field alongside main's unmodified `declared_peer: None` block for the same struct literal -- a silent, non-conflicting 3-way merge that left the field specified twice (E0062), because dig-node#473 (this branch) inserted its field near the top of the literal while main's untouched block still carried the pre-#473 field lower down. Removed the stale block; the kept comment and `Some(declared_peer)` are this branch's real implementation. Co-Authored-By: Claude <noreply@anthropic.com> * fix(mirror): close the continuation-guard exclusion #501 was granted Empty EXCLUDED_DIRS now that this PR own its six lost-string-continuation sites are already fixed elsewhere in this diff (mirror/advertise.rs, mirror/pass.rs, mirror/runner.rs). Verified with the guard itself: no_lost_string_continuation_leaves_a_multi_space_run_mid_sentence passes scanning every directory under src, with files_scanned > 20 and zero offenses. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 participant
DRAFT — DO NOT MERGE. Gate round in progress (correctness + security + adversarial refutation, three fresh contexts). This body will be final before any merge is proposed.
Closes #503
What was wrong
ChainBondVerifier::verify_against_chaintook itsChainSourcefromChainTransport::chain_source(). That provider ischia-query's router: it asksapi.coinset.orgFIRST and consults this node's dialled Chia peers only when that read fails. Its ownProviderInfocarriestrustless: false, and its construction comment says outright that "the peers do not corroborate the answer".So a
Bondedverdict — which ranks a holder, and is worth rank 0 at zero collateral if forged — rested on one source's word.The four steps in
chain_bond_verdict_and_coinare all internal consistency, never chain membership. An attacker curries the real, public $DIG CAT puzzle around an invented parent coin, computes the child id, and publishes that id in a provider record. Every step passes, because nothing asks whether the coin exists on mainnet.Why corroboration, and not verification
Verification would have been the cheaper answer and was checked first. It is not reachable:
ChainSourceexposes no block header, no merkle path, no inclusion proof. Its only proof-shaped primitive isresolve_singleton_lineage, and a mirror coin is a CAT, not a singleton — it has no launcher to walk from. The alternative anchor, walking the CAT lineage to a real terminus, is unbounded and still needs a trusted anchor.MirrorCoin::from_creating_spend(spend, coin_id)derives puzzle hash, amount, asset id and advertised terms from executed on-chain code bound to the claimed coin id. A lying source cannot change what coinCis — only whether it exists.That is also why corroboration is cheap here: the whole verdict reduces to exactly two primitive reads,
coin_record(C)andcoin_spend(C.parent_coin_info).The change is wiring, not a second quorum
dig-wallet'ssage::peer_reads::PeerCorroboratedReadsalready corroborates exactly those two reads across the node's own concurrently-held peers, inheritingsage::quorum'sCORROBORATION_FLOOR(= 2, never one source) andrequired_agreement, and already failing closed (Erris UNKNOWN, never absence). No new agreement mechanism was introduced; building one would be the rival-implementation defect.crates/dig-wallet/src/sage/corroborated_source.rs(new) —CorroboratedChainSource, aChainSourceoverPeerCorroboratedReads. The trait comes fromchia_query::provider_registry::interface, which re-exports the wholedig-chainsource-interfacecrate, so no second declaration and no split-family risk. It recomputes the coin id from the returned(parent, puzzle_hash, amount)and rejects a mismatch, for records and spends. The methods it cannot serve returnErr, never an emptyOk— an empty answer would be an absence lie.peak_height'sNone(peers did not agree) maps toErr, not toOk(None)(source exposes no peak), because those are different claims.ChainTransport::corroborated_chain_source— errs when no peer reads are attached and never falls back to the router. Falling back would let one endpoint overrule the peers exactly when they failed to agree.ChainBondVerifier::verify_against_chainnow reads through it; an error yieldsUnverified.On independence:
sources::independence_group_foralready records that acoinset_fallback_enabledfabric is the oracle's group however many peers it holds. That is why N reads through the router are one read wearing a hat, and why this path must draw onDialedPeerSample.How it is verified
crates/dig-node-service/tests/mirror_bond_corroboration.rs, 7 tests. The differential proof is a pair driving the samefabricated_bond()fixture through different sources:a_single_source_bonds_the_fabricated_coin)Bondeda_coin_only_one_peer_has_ever_seen_is_not_bonded)Unbondedagreeing_peers_do_bond_a_genuine_coinis the anti-vacuity control: without it the suite could pass by never producingBondedat all. Also covered: one answering peer cannot bond a genuine coin (Unverified), an even split does not bond (Unverified), a transport without peer reads refuses rather than using the router, and a source with no peers errs rather than reporting an absence.Counts, checked as counts rather than exit status:
mirror_bond_corroboration7 passed / 0 failed / 0 filtered;mirror_bond_verify10 passed / 0 failed (existing suite unbroken);dig-wallet --lib769 passed / 0 failed / 1 ignored.Bump rationale
0.247.0 → 0.251.0, minor: new compatible capability (
CorroboratedChainSource,corroborated_chain_source) plus a behaviour change on the verdict path, no removed or renamed API. 0.248/0.249/0.250 are claimed by open PRs #499/#500/#498, so this takes the next free value rather than colliding.Blast radius
gitnexus' registered
dig-nodeindex is 338 commits behind HEAD, and a stale index returns a false-safeimpactedCount: 0, so it cannot answer here. Radius was established by ripgrep plus direct reads and is reported as such:verify_against_chainis private with exactly one caller;chain_sourceis unchanged and its two other callers (server.rs:2871,server.rs:3104, the collateral census) are untouched; the two new symbols had no prior callers. Additive plus one private call-site swap.Disclosed limitations
verify_against_chainis private and reachable only throughverdict_for, which short-circuits ondeclaration_source_is_readable() == falseuntil feat(mirror): activate bond promotion on the coin's own peer declaration #501 lands. Stated rather than papered over; it becomes testable when feat(mirror): activate bond promotion on the coin's own peer declaration #501 merges.quorum.rssays so about itself, and §25.6a is worded to match rather than to overclaim.chain_source()and is deliberately out of scope per the ticket.chain.rs'speak_heightdoc already records that as a tracked sequencing constraint;corroborated_chain_source()makes fixing it a one-liner when selected.Relation to #501
Orthogonal, and neither subsumes the other. #501 asks whose bond — does this coin declare this peer, answered by executed on-chain code and self-verifying given the coin. #503 asks whether the bond exists — chain membership, not self-verifying by any available means. They compose as two independent gates on one verdict.
Sequencing worth knowing: #503's defect only goes live once #501 merges, since until then the short-circuit means no chain read is paid and no
Bondedis reachable. Both editbond_verify.rs, so whichever lands second mergesmainin.