feat(mirror): operator-scoped $DIG CAT coin selector for mirror creates (#421) - #423
Conversation
loop-security — IN PROGRESS, not the verdictAuditing at head Scope correction, before any findingThe dispatch brief says this branches from
MERGE-ORDER HAZARD (not a defect in this diff, flagged for the orchestrator)#419's If #419 merges first and #423 is merged without rebasing onto it, the three-way merge can resolve Recommendation: rebase #423 onto merged-#419 and re-run this gate on the rebased head before merging second. I will state in the verdict whether the funding logic itself is clean; that verdict cannot cover a merge resolution that has not happened yet. Continuing: scoping claim, double-spend surface, lineage authentication, the |
loop-security — FINDING 1, GATING — IN PROGRESS, not the verdictHead audited: The reservation mechanism is INERT for the one operation this PR exists to enable
A mirror The chain of evidence
So on every create outcome — broadcast OK ( Concrete scenarioState: operator wallet open, live broadcast on, broadcaster wired (#424), advertised URLs configured (#426). Stores
Severity and why I am gating rather than ticketingThis is not a fund-loss primitive — the chain refuses the conflicting spend, so no coin is consumed twice. It is also dormant in production today: I am gating anyway, on three grounds:
Fix shape (not a request for a specific patch)The record needs its funding coins even when no A regression test would need two consecutive passes over one chain fixture, asserting the second pass does not re-select the first's coins. Continuing the audit: lineage authentication, the |
loop-security — FINDINGS 2-4 — IN PROGRESS, verdict nextHead audited: Line-number correction to Finding 1: the create's FINDING 2 — GATING (normative twin of Finding 1):
|
loop-security VERDICT: CHANGES-REQUIREDHead audited: Read-only audit. No file edited, no shared checkout mutated — The lead question: can any path fund a create from a wallet other than the operator's?No. The scoping claim holds, and it holds structurally as the lane argues. This is the strongest part of the diff.
The fixture-collapse fix is real: Lineage all-or-nothing holds: The extraction is clean and behaviour-preserving. Both pre-existing callers keep byte-identical refusal messages, each in its original field order. Why this is CHANGES-REQUIRED anyway1. GATING — the reservation mechanism is inert for creates
So on every create outcome the record is correctly retained by the
Not a fund-loss primitive — the chain rejects the conflicting spend. It is a liveness defect, a false safety claim, and a money-surface inaccuracy: Note the test shape. Every integration probe supplies 2. GATING —
|
Findings 1 and 2 addressed at
|
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…o coins Co-Authored-By: Claude <noreply@anthropic.com>
Repairs the previous commit's whole-file line-ending flip on Cargo.toml: sed -i rewrote the CRLF file as LF, turning a one-line version bump into an 86-line diff. Restored and re-bumped byte-wise. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
`SpendJournal::submitted` was the sole writer of `funding_coin_ids`, and `sign_and_broadcast` called it only when the created coin was derivable. A mirror create is exactly the case where it is not — it passes `intended: None` — so the create path dropped the record entirely and every create contributed an EMPTY id list to `committed_funding_coin_ids`. The `!is_terminal()` filter was never wrong; it was never fed. The reservation therefore held nothing for creates: two creates in one confirmation window re-selected the same coins and broadcast conflicting bundles, and `control.mirror.*` and `dign spend-audit` showed every create as consuming no coins. The two facts a submission carries are independent and are now recorded independently. `Submission::intended_coin_id` becomes `Option<TargetCoinId>` and `sign_and_broadcast` records the submission UNCONDITIONALLY. The coins CONSUMED are read from the signed bundle and are always known; the coin CREATED is `None` for a create, which stays `None` — naming a plausible coin would let the reconcile confirm a spend against a coin it never created, the defect `TargetCoinId` exists to make inexpressible. The discarding branch is now unrepresentable rather than merely unused. `SpendRecord::intended_coin_id` was already `Option` and every reader — `reconcile`'s four arms and `chain_reference` — already handled `None`, so no reader changes. A successful broadcast now resolves `Submitted` before the drop guard rather than `Unresolved` with an empty list; both are non-terminal, so the coins are withheld either way, and the entry no longer understates what the node knows. SPEC.md §25 states the mechanism and its consequence rather than asserting the reservation property abstractly, so the clause is true of the code in this diff. Co-Authored-By: Claude <noreply@anthropic.com>
…sion The runtime proof of the fix lives at the journal seam (`funding::tests`), because reaching `sign_and_broadcast` needs an opened `OperatorWallet` and a real signed `MirrorSpends`. What that cannot see is the PLACEMENT: whether the create path calls `submitted` at all. The defect was a placement, so a test asserting only the outcome would pin a coincidence. Guarded structurally, in the idiom this file already uses for the `.with_signer(` rule — one unconditional `self.journal.submitted(` call and no branch on whether the target coin is derivable — with the companion test that proves the needles match a real reintroduction and that the `include_str!` still resolves to the file owning the broadcast path. Both new `funding` tests were proved load-bearing by reverting only the fix: `a_spend_with_no_derivable_target_still_withholds_its_funding_coins` and `recording_a_creates_funding_coins_does_not_invent_a_target_coin` both go red while the five surrounding controls stay green. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
1727d28 to
b961f10
Compare
Rebased onto merged-#419 — head
|
…spelling them CodeQL flagged eleven `salt: u8` byte literals in `tests/mirror_operator_funding.rs` as hard-coded cryptographic values used as a salt. They reach a hash for real — `ordinary_dig_coins` seeds a grandparent coin with `[salt; 32]` and takes that coin's id — so the alert is well-founded on the dataflow even though the values are fixture discriminators and never key material. Deriving them clears the finding at SOURCE across every call site. Justifying them thread by thread does not, and has already cost two extra fix-passes on this repo (dig-node#917, #950 are the same false positive twice). `salt(step)` digests a fixed string and offsets by `step`, which keeps both properties the fixtures depend on: * deterministic — same bytes every run, so a failing fixture stays reproducible; nothing random is used; * distinct per step — `wrapping_add` over distinct steps yields distinct salts, which is what keeps an operator coin and a replica coin from collapsing to one id. `Chain::fund` asserts against that collapse, so a collision would fail loudly rather than silently commit both coins. `chia-sha2` is added as a dev-dependency on the SAME 0.36.1 line as every other chia primitive in this manifest; a second line would be a second `Sha256` type. The lock gains exactly one line — the package was already present transitively at 0.36.1, so no new chia line is resolved. Verified on the integration target explicitly, which `--lib` does not compile: `cargo test -p dig-node-service --locked --test mirror_operator_funding` — 10 passed, the collapse assertion silent. Co-Authored-By: Claude <noreply@anthropic.com>
GHAS: fixed at source by deriving the salts —
|
Correction: the 11 GHAS threads DID auto-resolveMy previous comment said they "survived the rescan as OUTDATED" and handed the decision back. That
What I got wrong: I read the thread state during the rescan window and reported it as the The The caution about the code-scanning API also still stands: Remaining state at
|
| check | state |
|---|---|
| Lint commit messages | SUCCESS |
| Check version increment | SUCCESS |
| Rustfmt | SUCCESS |
| Clippy | SUCCESS |
| Test + coverage | in_progress (started 15:56:36Z) |
Unresolved threads: 0. Still DRAFT, still not merged.
Record correction: the 11 GHAS threads did NOT resolve themselvesA comment above withdraws an earlier "they survived as OUTDATED" report and says the threads The threads were resolved by an explicit Why this is worth a comment rather than letting it stand. The wrong lesson here is durable and
The correct procedure, both steps: derive the value so the finding is gone at source (done — the Nothing about the fix itself is in question. |
loop-security re-gate — IN PROGRESS, not the verdictAuditing head Posting as I establish things so nothing is lost to a stall. Finding 1's fix — the four checks in the brief, verified1. Any remaining path that records a submission without its consumed coins? No.
2. Do the readers hold? Yes — no reader assumes Every non-test read of
No 3. Does
Worth recording, because it is a small correction to the lane's own framing: 4. Same log on both ends? Yes. Writer Still to check: end-to-end reachability of the guard, the GHAS salt derivation, the |
loop-security re-gate — IN PROGRESS, not the verdict (2/2)Head Finding 2's fix does NOT hold — the new SPEC clause is false on arrival, again
Two creates in the same PASS do select the same coin. That is the tightest possible confirmation window, and it is the ordinary multi-bond case rather than a corner. The chain of evidence, all at this head:
The scenario. Node holds bonds A and B needing collateral, and one 100k $DIG coin C at the operator address;
What it is not. Both bundles are What it is. The mechanism the SPEC describes does not deliver the property the SPEC states, and the SPEC states it as a Also affected, one line down at Two remedies, either sufficient:
I prefer the first: the second leaves a signed, guaranteed-to-be-rejected bundle broadcast from the operator wallet on every multi-bond pass, which is a real spend attempt on a path documented as reservation-protected. Everything else I checked is clean; verdict comment to follow. |
loop-security scoped re-gate — CHANGES-REQUIREDHead audited: One gating finding. Finding 1's fix is sound and verified; Finding 2's fix is not. GATING —
|
…ly across passes A pass emits N creates (runner loops the affordable prefix, which plan derives as balance / per_coin), and every one of them was handed the same committed-coin snapshot, read once before the pass. Neither of the other two sources could correct it: the durable journal is re-read once per pass, and the chain shows a broadcast coin as unspent for the whole confirmation window -- the premise the funding module is built on. So the second create in a pass re-selected the first's coin and broadcast a bundle double-spending it, reported as two successful creates. SPEC.md 25 states the opposite as a MUST NOT. The committed set gains interior mutability and sign_and_broadcast extends it, on a broadcast that reached the mempool only, from the same value the durable journal receives -- so the two cannot disagree, and a failed broadcast strands nothing. Refs: #423 Co-Authored-By: Claude <noreply@anthropic.com>
Two probes driving the REAL NodeMirrorEffects::create twice against genuine CAT coins. They differ in exactly one thing -- whether a second coin exists -- because either alone is blind: with one coin, 'the second refused' is also what a selector that refuses everything produces; with two, 'both succeeded' is what the defective implementation produces, since it broadcasts twice quite happily. The assertion is on the coins each broadcast bundle actually spends, not on the in-memory reservation set: asserting the set would pin the mechanism rather than the property, and would stay green if the extension moved somewhere the selector never consults. Refs: #423 Co-Authored-By: Claude <noreply@anthropic.com>
… bump to 0.175.1 The clause read as a consequence of the journal recording, which is an ACROSS-pass mechanism only -- so it asserted a within-pass property the code did not have. It now names both halves and says why the journal alone cannot cover the second: it is read once, before the pass, and the chain reports a broadcast coin as unspent for the whole confirmation window. runner's stop-cleanly rationale said a create 'failed for want of a coin', which did not describe the path it was defending: create #2 selected fine and would have failed at the mempool. The reservation is what makes the reasoning sound, and the comment now says so. Refs: #423 Co-Authored-By: Claude <noreply@anthropic.com>
Gating finding FIXED in code — intra-pass reservation, not a narrowed sentenceHead What changed
Revert proof — the test is load-bearingNew target Both fail on their own assertion — not a compile error, not an unrelated panic — and the diagnostic prints the same coin id in both bundles, which is the double-spend itself. Restored: Why two probes rather than oneEither alone is blind, which is the trap this repo has hit before:
So the probes vary exactly one thing (whether a second coin exists) and keep a truthful control. The assertion is on the coins each broadcast bundle actually spends, not on the in-memory set: asserting the set would pin the mechanism and stay green if the extension were moved somewhere the selector never consults. The fixture deliberately leaves the funding coin unspent on chain after the first broadcast — that is the production premise ( Blast radius checked
Local: |
DO NOT MERGE — gate round has not returned. Draft until then.
Closes #421
What this is
dig_mirror_coin::createtakes itsVec<Cat>from the caller. The only $DIG selector this processhad was
WalletBackend::select_cats, which reads the node-custodied replica's coin table — adifferent wallet from the §16.4 operator wallet the mirror signer signs with. Funding a mirror
coin from those coins is a real spend of the wrong wallet's money that returns
Okand looksentirely successful, which is why step 7 (#419) made
createrefuse by name.This adds the correct selector rather than relaxing that refusal.
Which coin set it reads, and how that is proven
mirror::funding::select_operator_dig_catsreadsChainSource::coin_records_by_puzzle_hash(dig_cat_puzzle_hash(operator_ph), include_spent = false).dig_cat_puzzle_hashisCatArgs::curry_tree_hash(DIG_ASSET_ID, operator_ph)— the canonical CATwrapping, the same construction
reclaimed_coin_idwas already using inline and now shares.The scope is structural, not a filter: a coin at any other owner's puzzle hash is never read.
It is proven by fixtures that fund two different wallets on one chain:
the_selector_funds_from_the_operator_wallet_and_never_from_the_replica— the replica holds10× the requirement, the operator exactly 1×; the selection is the operator's coins and contains
none of the replica's.
an_operator_with_no_coins_refuses_even_when_the_replica_is_rich— the mirror image, and the onethat fails loudly against the defect: only the replica is funded, and the correct answer is
Insufficient { have: 0 }.A single-wallet fixture would pass against a selector that ignored its owner argument entirely,
which is precisely the implementation under suspicion.
Extracted, not duplicated
select_cat_rowswas not parameterised in place — its algorithm was extracted todig_wallet::sage::selection::select_largest_first, generic over the item and its(amount, tiebreak)key. Three rivals existed:rpc::select_cat_rowsover DB rows,offers::select_catsover resolvedCats, and this new one over chain records. The first two nowcall the shared function, with byte-identical refusal messages (on a shortfall the walked total and
the whole-set total are equal, so
haveis unchanged).Reservations
The chain cannot offer an unreserved set: a broadcast coin stays unspent in the chain's view for the
whole confirmation window, and the mirror pass runs on a round timer inside it.
The equivalent record already exists and is durable across restarts — the spend audit journal
records
funding_coin_idsfor every bundle it submits.committed_funding_coin_idswithholds thefunding coins of every record where
SpendStatus::is_terminal()is false, reusing thatpredicate rather than restating it: a
Submittedspend may still consume its coins, anUnresolvedone may already have, and a
Failedone at a stage thatmoney_may_have_moved()is an unknownwearing a failure's name. An audit file with unreadable lines refuses, because a silently
smaller reservation set is exactly how one coin funds two bundles.
Lineage
Each selected candidate is authenticated by reading
coin_spend(parent_coin_info)— the spend thatCREATED it — and running it through
dig_wallet::sage::singleton::resolve_cat. A candidate whosecreating spend is absent, or which yields no matching CAT child, or whose resolved CAT disagrees
about the asset id or the owner, refuses the whole selection. Dropping it and proceeding would
fund the create from a short set, and a mirror coin below the epoch's requirement is collateral
genuinely locked against a bond it does not satisfy.
A chain
Errmaps toPassError::Chain, never toInsufficient— an unreadable source is UNKNOWNand is in no position to claim the wallet is empty.
The amount
createpasses itsamount_dig_base_unitsargument straight through as the selection target.Nothing here re-derives
apply_safety_margin(required_per_store, margin_bp).the_number_of_coins_drawn_follows_the_requirement_it_was_givenasserts the selection tracks theargument, so a selector that ignored it would be visible.
What step 7 established, unchanged
MirrorSigner::sign(&MirrorSpends, &SpendJournal)is untouched..with_signer(/.with_broadcaster(guard stilldiscriminates — this PR adds no such call, and
funding.rsis not in its include set because itnever mentions
WalletBackend.Err-carrying field consumed only bycreate, exactly likedig_balance.Relayedcapsules remain invisible to the create path (split_by_provenanceis untouched).Blast radius checked
gitnexus was not used: a per-worktree
analyzeon this tree exceeds the §2.0 ten-minute bound,so the radius was taken by grep + direct read and is stated here per §2.0's fallback clause.
select_cat_rows— one caller (WalletBackend::select_cats). Behaviour and message preserved.offers::select_cats— one caller (offers.rs:134). Behaviour and message preserved.reclaimed_coin_id— one caller; now shares the CAT derivation instead of its own curry.NodeMirrorEffects::new— one caller (server::spawn_mirror_passes), updated in this PR.dig-walletordig-node-serviceis removed or changed in meaning; bothadditions are new items.
§2.4b
dig-mirror-coin0.7.0,dig-chainsource-interface0.3.2 anddig-mirror-collateral0.3.0 werechecked against the index and are already at latest; the
chia-*set stays together on the 0.36line. The ~30 declarations deferred to #418 are not absorbed here.
What still blocks a create on a funded wallet
dig_mirror_coin::createrequires at least one URL its store can befetched from, and this node has no configured public name.
NodeMirrorEffects::createrefuses byname, ahead of any chain read, and
server.rspasses an empty URL set with that stated. This isan advertisement gap, not a funding one.
spawn_mirror_passesstill passesbroadcaster: None.DIG_WALLET_ENABLE_LIVE_BROADCASTmust be on for a signer to exist at all.Base
Branched from
loop/412-step7-observation(PR #419), which is not yet merged and whichintroduces the
mirror/lifecycle.rsthis changes. Until #419 merges, this PR's diff againstmainincludes #419's commits.
Version: workspace
0.173.0→0.175.0(minor — new capability;spend_audit::Submission::intended_coin_idbecomes
Option<TargetCoinId>, a crate-internal type with no published consumers).Reservation fix — security gate findings 1 and 2 (
d9d4f61)The gate found the reservation mechanism INERT for creates.
SpendJournal::submittedis the solewriter of
funding_coin_ids, andsign_and_broadcastcalled it only in the arm where the createdcoin was derivable — which a create never is, so it passed
Noneand the record was dropped. The!is_terminal()filter was correct; it was never FED. Two creates in one confirmation windowre-selected the same coins and broadcast conflicting bundles, and
control.rs:3352/spend_audit_cli.rs:227showed every create as consuming no coins.Submission::intended_coin_idis nowOption<TargetCoinId>and the submission is recordedUNCONDITIONALLY. The two facts are independent and are recorded independently: the coins CONSUMED
are read from the signed bundle and are always known; the coin CREATED is
Nonefor a create andstays
None— naming a plausible coin would let the reconcile confirm a spend against a coin itnever created, the defect
TargetCoinIdexists to make inexpressible. The discarding branch is nowunrepresentable rather than merely unused.
SpendRecord::intended_coin_idwas alreadyOption, andreconcile's four arms andchain_reference()already handledNone, so no reader changed. A successful broadcast nowresolves
Submittedbefore the drop guard rather thanUnresolvedwith an empty list; both arenon-terminal, so the coins are withheld either way.
SPEC.md§25 states the mechanism and its consequence rather than asserting the reservationproperty abstractly, so the clause is true of the code in this same diff.
Blast radius checked
gitnexus's
dig-nodeindex predates this branch and could not resolve the symbol, so the radius wastaken by grep + direct read (§2.0 fallback, stated as required rather than silently substituted).
Every construction site of
Submission(14) and every call site of.submitted((13) lie insidedig-node-service—spend_audit.rs,spend_audit_cli.rs,mirror/funding.rs,mirror/lifecycle.rs,tests/spend_audit_e2e.rs— plus every reader ofintended_coin_id, all ofwhich already took an
Option. Nothing outside the crate names either symbol.Evidence
Both new tests were proved load-bearing by reverting ONLY the fix (committed first; reverted via a
file copy, not
git checkout): both go red while the five surrounding controls stay green. Thefirst attempt planted the mutation after the funding-ids assignment and all 7 passed — a targeting
error in the harness, not a weak test; corrected and re-run. All 91
mirror::tests green atd9d4f61.The placement is guarded separately, because the defect WAS a placement and a test asserting only
the outcome would pin a coincidence:
lifecycle.rscarries a structural guard — in the idiom thatfile already uses for the
.with_signer(rule — that there is exactly one unconditionalself.journal.submitted(call and no branch on whether the target coin is derivable, plus thecompanion test proving the needles match a real reintroduction.
Follow-ups filed, deliberately NOT absorbed
input (
mirror/funding.rs:194-215).dig-wallet/src/sage/offers.rs:412-429select_xchis a fourth largest-first rivaland disagrees with its three siblings on overflow direction (
sum += c.amount, which wraps in arelease build).
Still open, and it blocks landing
server.rs:2789also passesbroadcaster: None; #419's8f93bd6is the fix, not this PR. ThisPR must be rebased onto merged-#419 before it lands — both edit the same
mirror/lifecycle.rsregion, and merging second without a rebase risks resolving away #419's honesty fix. Not rebased
yet; #419 is not merged.
Co-Authored-By: Claude noreply@anthropic.com