Skip to content

fix(mirror): resolve landed spends, feed the DHT collateral pointer, bound funding inputs - #457

Draft
MichaelTaylor3d wants to merge 5 commits into
mainfrom
loop/batch-mirror
Draft

fix(mirror): resolve landed spends, feed the DHT collateral pointer, bound funding inputs#457
MichaelTaylor3d wants to merge 5 commits into
mainfrom
loop/batch-mirror

Conversation

@MichaelTaylor3d

@MichaelTaylor3d MichaelTaylor3d commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

DRAFT — do not merge. The gate round has not returned. This is custody code that spends real $DIG and it takes the full triple gate.

Closes #412 (pieces 1 and 3) · Closes #435 · Closes #429 · Closes #427

Epic: https://github.com/DIG-Network/dig_ecosystem/issues/3166


1. #412 piece 1 — resolve landed mirror spends

journal.confirmed had zero production call sites (all 8 were #[cfg(test)]), and submitted() does not set settled, so every successfully broadcast mirror spend ended Unresolved on drop. dign spends showed a node whose money had demonstrably moved as a node that did not know what it had done.

The entry point, and why it is inside spend_audit. SpendJournal::resolve_landed(id, coin_id, height) -> Resolution (spend_audit.rs). RecordedSpend has no public constructor, SpendLog::append is module-private, and a RecordedSpend is dropped at pass end — so no later pass can resolve an earlier pass's record, and a resolver outside the module could not write the file at all. Making the write path public to let it would have been a second producer of Confirmed, the one status the module's honesty rules are built around.

Err is kept distinct from Ok(None) at three layers, deliberately:

layer shape
ChainSource::coin_record Result<Option<CoinRecord>, E>
MirrorEffects::coin_confirmation Result<Option<u32>, PassError>Err = could not ask; Ok(None) = absent or present with no confirmed_height yet
mirror::resolve Err → resolve nothing, count chain_unreadable, warn. Ok(None) → resolve nothing, silently.

Only Ok(Some(height)) reaches resolve_landed. lifecycle.rs's implementation propagates the source error rather than mapping it, and a malformed local coin id is deliberately NOT an Err — it is this node's bookkeeping being wrong, not the chain being down, and reporting it as an outage would hide a permanent defect behind a retry forever.

Keys, per the measurement:

  • reclaimintended_coin_id = Some(reclaimed_coin_id(coin)) already exists (lifecycle.rs:398). One coin_record read.
  • createintended_coin_id is None by design, so the key is the coin's appearance in the pass's own observe_chain, matched on all three of (store, root, epoch), plus one coin_record for the height. No coin id is ever derived or invented.

Disappearance is not used as a key, for the three reasons the measurement gives: nothing to pass to confirmed(); the mirror puzzle hash is shared, so a coin leaving the set proves someone spent it; and a short scan looks identical to a spend.

Ambiguity resolves nothing. Two open records can name one coin (two reclaim attempts of one coin derive the same child id; two open creates for one bond are reachable precisely because §25.4.6 does not suppress on Unresolved). At most one of those bundles landed and this node cannot tell which, so neither is resolved and both stay unresolved.

Did I change the suppression set? NO.

runner.rs's matches!(… Pending | Submitted) is untouched, and Unresolved is still not suppressed. A resolver only fires when the coin appears; a bundle that never lands leaves its record Unresolved forever, so adding Unresolved to the suppression would still be a suppression that never lifts and would still leave the bond permanently uncollateralised. runner.rs:404-414's asymmetry is unchanged and still correct. Resolution runs before in_flight_creates, so a create this sweep confirms stops suppressing itself in the same pass rather than one pass later — which is the whole gain, without touching the rule.

A resolver outage therefore causes no harm: it resolves nothing, suppresses nothing extra, and the next pass retries.

2. #435 / #412 piece 3 — something now supplies MirrorCoinPointers

The only impl was a test double at dht.rs:1734; production called DhtHandle::new, hard-coding None, so every live announce published unverified_mirror_coin_id = None and reannounce_on_epoch_rollover returned 0 on its first line every tick.

  • mirror::pointers::SnapshotMirrorPointers reads the BondSnapshot the last pass published — the same observation control.mirror.bondStates serves — rather than doing its own dig_mirror_coin::list. A second read would be a second answer, and an operator comparing the two would see a disagreement this node manufactured. (The measurement's "not PassReport.created" constraint is honoured: that is Vec<Bond> with no coin id; BondState::Bonded carries coin_id.)
  • Plumbed via a Node slot (set_mirror_coin_pointers) set in server.rs before spawn_peer_network, read at peer.rs's DhtHandle::with_mirror_pointers. A slot rather than an argument because the FFI/browser path constructs a Node with no mirror lifecycle and no peer network.
  • Only a current-epoch Bonded row yields a pointer, and a whole-store announce carries none.
  • dig-node-core now re-exports dig_dht so the consumer does not declare a second dig-dht constraint (§2.4b split-line risk on a trait's own type).
  • The Attach unverified_mirror_coin_id at the DHT announce (blocked on the dig-dht 0.15 cascade) #422 cascade is untouched, as instructed.

3. #429 — a Disabled bond IS bondable

Answered as the ticket's second permitted outcome, with the reasoning in the doc. Withheld is a property of the capsule (a stranger's, never advertisable); Disabled is a reversible node-wide switch. Excluding it would drop the buffer advice to zero for the whole node the moment collateralisation is switched off — telling an operator who is about to re-enable that they need no $DIG, and stranding them short on the next pass. Under-stating money the operator must hold is the reassuring direction. Reclaiming is documented as counted for the same forward-looking reason.

4. #427 — mirror funding inputs are bounded

MAX_SELECTED_FUNDING_COINS = 32, enforced after selection (free, in-memory) and before authenticate (the per-input coin_spend chain read). The scan address is publicly derivable, so unbounded, the number of chain reads one automated pass performs is chosen by whoever paid dust to it.

Direction: it fails CLOSED — the create is refused, the bond is uncollateralised for that pass, and a new FundingError::TooManyInputs { needed, limit } says the wallet is not short, its $DIG is in too many pieces. Recoverable by retry and permanently by consolidation. Failing open is not recoverable by anything the operator can do. Bounding the candidate set instead was rejected: it would refuse a fundable create because a stranger sent dust that was never selected.


Tests — what each catches, and each proven load-bearing

Eight mutations, each reverted individually against committed state; every one was killed by its own test.

test catches mutation that killed it
a_landed_reclaim_is_confirmed_at_the_height_the_chain_reported the shipped defect; and confirming on the broadcast rather than the landing (second spend, identically broadcast, no coin) resolver made inert
a_chain_that_cannot_answer_resolves_nothing_while_its_neighbour_still_resolves folding Err into Ok(None); the control rules out a resolver that does nothing at all .or_else(|_| Ok(None))
a_create_is_confirmed_only_against_a_coin_that_matches_store_root_and_epoch the invented coin id; decoys at the same store/different root and same root/previous epoch match on store_id alone
two_open_spends_claiming_one_coin_resolve_neither resolving both, and "resolve the first" ambiguity check disabled
a_spend_that_never_reached_the_network_is_not_confirmed_by_a_coin_that_matches_its_bond a resolver keyed on the bond alone writer's Pending refusal removed
a_current_epoch_bonded_capsule_publishes_its_coin_and_only_its_coin a source answering None for everything (= shipped behaviour); a third row rules out "return the only coin you have"
a_coin_from_a_previous_epoch_publishes_no_pointer a stale pointer, asserted directly rather than left to the re-announce epoch check disabled
the_whole_store_announce_carries_no_coin_because_no_coin_bonds_a_whole_store matching on the store id
before_the_first_pass_there_are_no_pointers_and_the_epoch_cannot_collide_with_a_real_one seeding the epoch sentinel to 0, a real epoch
bondable_pairs_counts_every_served_row_except_the_relayed_one_that_locks_nothing excluding Disabled/Reclaiming Disabled added to the exclusion
a_node_with_collateralisation_switched_off_still_advises_for_the_pairs_it_will_bond the consequence: every row Disabled → advice of zero same
a_create_needing_more_inputs_than_the_bound_is_refused_before_any_lineage_read unbounded selection; asserts both sides of the bound and that zero lineage reads happened > changed to >=

One finding from the revert-proof itself, recorded because it nearly shipped a false green. The Pending refusal existed in two places — the sweep's filter and the writer's guard — and the writer masks the sweep. Relaxing the sweep alone changed nothing observable and the test stayed green. The test now asserts SpendJournal::resolve_landed directly as well as through the sweep; the sweep's filter is kept deliberately (it stops a Pending record costing a chain read per pass to reach a refusal decidable for free) and is documented as redundant.

Blast radius

gitnexus impact was not usable: list_repos reports the registered dig-node index 301 commits behind and pointed at the primary checkout, and a stale index returns a false-safe impactedCount: 0 / risk UNKNOWN (dig_ecosystem#3188). Per §2.0 bound (2), done by grep + direct read instead, and stated here.

  • MirrorEffects — 2 impls, both #[cfg(test)] in runner.rs; both extended. Gained a required method (breaking for any external implementor; there are none).
  • SpendJournal — new method only; confirmed/submitted/failed/unresolved untouched.
  • FundingError — new variant. Exhaustive matches: the Display impl (updated) and the tests. No other matcher in the tree.
  • bondable_pairs — one caller, collateral_buffer.
  • DhtHandle::new — 6 call sites; only peer.rs:3004 changed, to with_mirror_pointers. The 5 test sites keep new, which still means "no pointers".
  • Node struct literals — 14, all updated with the new field.
  • dig_node_core::dig_dht re-export — additive.

detect_changes was likewise unavailable on a stale index; the diff was reviewed file-by-file and touches only mirror/, spend_audit.rs, control.rs (one function + its tests), peer.rs (one line), lib.rs (one slot), server.rs (one block), SPEC.md and the version.

No HIGH/CRITICAL risk warning is available to report, because the tool could not answer — which is itself worth the gate's attention on custody code.

SPEC

  • §25.4 gains step 7, normative: the two positive keys, the three-way coin read, the ban on disappearance-as-key, and the ambiguity rule.
  • §25.6 PENDING banner replaced with IMPLEMENTED, plus the current-epoch-only and no-whole-store-pointer clauses.

SemVer — minor, 0.189.0 → 0.190.0

New capability (resolution, the pointer source) plus two technically-breaking surfaces: a required method on the public MirrorEffects trait and a new FundingError variant. Under cargo's 0.x rules a minor bump is already semver-incompatible, so it carries the break correctly for any caret dependant. Stated rather than assumed.

Unverified

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

Evidence

  • cargo test -p dig-node-service -p dig-node-core --lib1023 passed / 0 failed and 657 passed / 0 failed, 0 filtered out (test COUNTS checked, not just the exit status).
  • cargo clippy -p dig-node-service -p dig-node-core --lib --tests --all-features -- -D warnings — clean.
  • cargo fmt --all — applied.
  • The 12 new/changed tests, run filtered: 12 passed, 645 filtered out.
  • Eight revert-proofs, each mutation applied individually against committed state and reverted with git checkout; every mutation was killed by its own test. Transcript summarised in the table above.

No mainnet spend was made by this lane. Commits are left unsquashed for the gate to read; they squash to one conventional commit before ready.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

loop-security — IN PROGRESS, not the verdict

Auditing head 4370990ebfd4bb24cbfe07372d749d664c53effd (resolved from gh pr view 457 --json headRefOid), against merge-base 3e480dd302ae0a8518ce535e5b1bc4a563b960ff. Read from git objects in a private worktree/scratch; the lane's worktree dn-mirror and the primary checkout (parked on pr409) were not touched.

Three of the six priority checks resolve CLEAN so far. Recording them now so they survive a kill.

1. The resolver does not invent a confirmation — CLEAR

SpendStatus::Confirmed { height, coin_id } is constructible from exactly one place in the diff: spend_audit.rs:249-252 inside resolve_landed. Both fields are the caller's observation, not an inference:

  • height reaches that line only through resolve.rs:162 — the Ok(Some(height)) arm of effects.coin_confirmation. There is no other call site of resolve_landed outside resolve_tests.rs.
  • coin_id is a TargetCoinId built at resolve.rs:163 from the claims key, which is derived at resolve.rs:130-143 as either the record's own intended_coin_id (reclaim) or a coin id read out of on_chain (create). Neither branch fabricates a string; a create with no matching (store_id, root, epoch) in the chain observation continues (resolve.rs:136-140) rather than guessing.

lifecycle.rs:31 is the production source of the height: Ok(record.and_then(|r| r.confirmed_height)). A coin the source knows about but has not seen in a block yields None, so mempool presence cannot become a height.

2. Err vs Ok(None) stay distinct, and an outage resolves nothing — CLEAR

lifecycle.rs:23-26 maps a chain-source failure to PassError::Chain via map_err — no unwrap_or, no ok(), no defaulting. In resolve.rs the three arms are structurally separate (:162 / :190 / :191), and the Err arm contains no call to resolve_landed — it only increments chain_unreadable and warns. A malformed local coin id is Ok(None) at lifecycle.rs:13-20 with an error! log, which is the right split: it is local bookkeeping being wrong, and treating it as an outage would hide a permanent defect behind an infinite retry.

Verified by construction rather than by reading the prose: the only path from coin_confirmation to a ledger append passes through the Ok(Some(_)) pattern.

3. The suppression set is UNCHANGED, and the gain is from ordering — CLEAR, proved mechanically

Extracted fn in_flight_creates from both revisions and diffed the function bodies:

base lines: 28  head lines: 28
IDENTICAL: in_flight_creates unchanged

Still matches!(r.status, SpendStatus::Pending | SpendStatus::Submitted) (runner.rs:474 at head), with the Unresolved-excluded rationale intact at runner.rs:442-453. The asymmetry the brief names is preserved: a duplicate create self-heals at rollover as EpochEnded, a never-lifting suppression leaves a bond permanently uncollateralised.

The ordering change is runner.rs:280-286resolve_landed_spends runs before let in_flight = in_flight_creates(...) at :287. It is called without ?, and its ResolveSummary is discarded, so a resolver outage cannot abort the pass. Traced the no-harm claim: an unreadable ledger returns an empty summary at resolve.rs:104 after a warn!; an unreadable chain leaves every record open; and claimants[0] at resolve.rs:159 cannot panic because a claims entry is only created by pushing into it. Records stay Unresolved, which is the state that already did not suppress. No harm.

journal.log() used at resolve.rs:93 is pre-existing (spend_audit.rs:773), not newly widened by this PR.

Still open: #435 pointers epoch/source, #427's bound, the intended_coin_id shape-change consumer grep, the dig_dht re-export, and a mutation spot-check of the resolver.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

loop-security — IN PROGRESS (2/3), not the verdict

Head 4370990ebfd4bb24cbfe07372d749d664c53effd. Three more checks resolved; one LOW defect found in the diff and one HIGH pre-existing issue named for a follow-up (not gating).

4. #435 — the production MirrorCoinPointers — CLEAR, with the source traced end to end

The brief asked whether it reads the held set from dig_mirror_coin::list and not from PassReport.created. It reads neither directly — it reads BondSnapshot. I traced whether that snapshot's coin ids ultimately come from the chain:

pointers.rs:104   BondState::Bonded { coin_id, epoch, .. }
  <- lifecycle.rs:758  publish() copies report.states verbatim (a TRANSLATION, never a recomputation)
  <- pass.rs:275       BondState::Bonded { coin_id: coin.coin_id.clone(), .. }, coin: &HeldMirror
  <- pass.rs:250       current_epoch_coins, filtered from PassInputs.on_chain
  <- runner.rs:280     self.effects.observe_chain()
  <- lifecycle.rs:338  dig_mirror_coin::list(self.source, self.owner_puzzle_hash)

So the coin id is the chain's, never PassReport.created (which is Vec<Bond> and could not supply one). observe_chain additionally fails closed on an incomplete scan (lifecycle.rs:343-348), so a truncated inventory cannot silently yield a wrong-pointer observation.

Epoch agreement — the thing the brief said is worse than no pointer — holds. In server.rs one local epoch binding feeds both PassContext { current_epoch: epoch, .. } (:2795) and lifecycle::publish(&snapshot, &report, epoch) (:2838). observe.rs:118 likewise sets BondObservation.epoch = ctx.current_epoch. And pointers.rs:107-109 refuses a row whose BondState::Bonded epoch differs from observation.epoch, so a previous epoch's coin publishes nothing. A failed pass is not published (server.rs:2840+), so a stale-but-consistent observation is kept rather than replaced by a worse one.

Wiring verified live, not just present: server.rs:2168 calls set_mirror_coin_pointers before spawn_peer_network at :2170, and peer.rs:3011-3015 now passes node.mirror_coin_pointers() into DhtHandle::with_mirror_pointers in place of the hard-coded None. The slot is a OnceLock whose set result is discarded, so the source cannot be swapped under a running node.

set_mirror_coin_pointers is a new pub method on Node — a genuine API widening — but the value it installs is untrusted by construction (a verifier judges the coin on the coin's own evidence, NC-12), the worst a wrong pointer costs is a lookup, and None stays a fully supported configuration. No authz consequence.

6. The intended_coin_id shape change — CLEAR, grepped independently

resolve_landed sets intended_coin_id = Some(coin_id) at spend_audit.rs:253, so a resolved create now carries an id where it used to carry None. I grepped every reference in crates/ (48 hits) and read each non-test reader. No consumer keys on a None check — every one keys on status first:

  • spend_audit.rs:409-421 chain_reference() — the Confirmed arm matches first and uses the variant's coin_id; the intended_coin_id fallback is only reachable for non-Confirmed records.
  • spend_audit.rs:1028-1063 the reconcile loop — same shape; the Confirmed arm inserts the variant's coin_id into accounted, and the intended_coin_id reads are confined to the Unresolved / Submitted / money-may-have-moved Failed arms.

Since resolve_landed writes the same value into both places, neither surface changes even incidentally. The remaining hits are resolve.rs itself and test fixtures.

5. #427 bound — the failure direction is acceptable; two LOW notes

The bound is selected.len() > MAX_SELECTED_FUNDING_COINS at funding.rs:263, placed after in-memory largest-first selection and before the per-input authenticate chain read at :270. That placement is right: the read it bounds is the one at :270, and bounding the candidate set instead would refuse a fundable create because a stranger sent dust.

Failure direction: acceptable. A legitimate operator whose $DIG is in more than 32 pieces gets a refused create — the bond is uncollateralised this pass, retried next pass, and permanently fixed by consolidating. TooManyInputs is its own variant so the surface does not tell a funded operator they are short. No funds are at risk and nothing is written. That is the recoverable direction.

An attacker cannot cheaply push a legitimate wallet over it. Selection is largest-first (funding.rs:255-259), so raising the selected count requires coins that are simultaneously larger than the honest ones (to be picked) and small enough that 32 do not cover the target — which means handing the operator roughly the collateral amount in genuine $DIG at their own address. Self-defeating.

LOW — the TooManyInputs message is a mangled string literal (funding.rs:128)

Verified with cat -A on the blob, not from the diff rendering. The literal is one physical line carrying three runs of 18 spaces where Rust line-continuations were lost, so the operator-facing text renders as may draw at most 32. The adjacent CommitmentsUnreadable arm uses a proper continuation, so this one arm was mangled. CI cannot catch it: rustfmt does not reformat string literals (format_strings = false by default) and cannot break a long one, so cargo fmt --check passes. Truthful, so not a money lie — but it is the operator-facing message on a custody refusal path and it is a one-line fix. Not gating.

LOW — MAX_SELECTED_FUNDING_COINS = 32 does not say it is unmeasured (funding.rs:69)

The doc block is thorough on why a bound, which direction it fails, and where it is placed — but it never says why 32, and the lane's own report states plainly that nobody has measured real operator wallets. A constant that fails closed on a legitimate operator should carry that on its face; a reader today takes 32 for a measured value. Not gating.

HIGH, PRE-EXISTING, NOT INTRODUCED BY THIS PR — one unauthenticatable coin kills every create

Naming it because #427 family is about exactly this attack surface and the new bound does not reach the stronger form. funding.rs:269-271 is unchanged context in this diff:

for record in &selected {
    cats.push(authenticate(source, record, owner_puzzle_hash)?);
}

The ? aborts the whole selection on the first refusal, and authenticate own doc says its job is to reject "a coin somebody paid to this puzzle hash". Anyone can create a plain coin at dig_cat_puzzle_hash(owner) — publicly derivable — with an amount one mojo larger than the operator largest honest $DIG coin. Largest-first picks it first, authenticate refuses it, and select_operator_dig_cats returns Unauthenticated every pass, forever. Cost to the attacker is the locked mojos; the node is then permanently unable to collateralise anything, which is a durable denial of the whole mirror-collateral function.

This PR strictly improves on the prior state and introduces no part of it, so it does not gate. Recommend a follow-up ticket: skip an unauthenticatable candidate and continue selecting, rather than aborting, while still refusing to spend it.

Remaining: the mutation spot-check of the resolver (build running), the dig_dht re-export, SemVer, and the no-mainnet-spend check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment