feat(control): serve a coin read by coin id, including spent coins

[MichaelTaylor3d]

Serves control.wallet.coinById — a chain read of ONE coin record BY COIN ID, including spent coins.

Closes the dig-node half of DIG-Network/dig_ecosystem#2392. The contract half is released: dig-node-control-interface 0.7.0 is on crates.io and declares WalletCoinById, so release-first is satisfied.

Why it exists

control.wallet.coins answers by address and unspent-only, so mint_status can observe neither the DID coin nor the funding coin the mint spends. Without this read a DID mint can be pushed — real XCH leaves the wallet — and never observed.

The unspent-only behaviour is a filter, not a source limit: rpc.rs:865 and rpc.rs:899 both drop spent coins after the read, and chia-query 0.6 already exposes an absence-aware get_coin_record_by_name_opt.

The money lie this fixes

CoinsetFallback::coin_record_by_id mapped Err(_) => Ok(None), so a dropped connection, a coinset 500, a TLS failure and an exhausted rate limit all returned "the coin does not exist." Under mint_status that means a mint reads Awaiting forever and can never report Failed.

Absence and unreachability are now distinct end to end:

• coin: null ⇔ Ok(None) ⇔ the chain answered {"success":true,"coin_record":null} — a real, checked absence;
• an error envelope, a transport failure or a timeout ⇔ Err(_) ⇔ a catalogued error, never a null coin.

Verified through every layer to the wire: coin_by_id (rpc.rs:961) maps the fallback error to BalanceError::ReadFailed via ? and constructs coin only from a real Some; the handler maps that through wallet_read_error to -32042. Nothing between re-collapses it — no .ok(), no unwrap_or_default, no catch-all arm.

Blast radius checked

coin_record_by_id has two production callers besides the new one, both in coins_by_ids (rpc.rs:1337, :1345), and their behaviour changes for the better. They read if let Some(fc) = …await?, so under the old mapping an unreachable chain silently omitted those coins and returned a truncated list as though it were complete — a caller asking "do these coins exist" would conclude they were absent. The error now propagates, so a partial answer can no longer masquerade as a complete one. This is the same defect class as the coinById fix and is fixed by the same change.

Six ChainFallback impls exist (CoinsetFallback, ChainTransport, EmptyFallback, and three test doubles); the trait signature is unchanged, so only CoinsetFallback's mapping moved.

Design

Served from coinset via chia-query 0.6 at the fallback tier, deliberately bypassing routing::route and never consulting the local SQLite replica. That table is filled only from the node's own subscriptions gated by db.derivation_exists, so a miss there means "this node doesn't watch that coin" — which is not absence, and answering from it would make the method an ownership oracle whose answer varied with node-local state.

Hence source is always "fallback", synced always false, peak_height always null; callers bound confirmations via control.wallet.peak.

It is an open, token-less read (is_open_control_read), like balance/coins/peak: the argument is a coin id — public chain data, never a seed, key, address or signature. Liveness and the shared coinset rate limiter (#1957) both gate it, and the parameter is validated before any network read.

§908 holds absolutely: this is a chain READ. The node signs nothing and no key material is anywhere on this path.

The green that proved nothing

The contract-conformance gate was pinned at dig-node-control-interface = "0.6.0" — a version predating this method. Every assertion in that suite iterates the CONTRACT, so a method the pinned version does not declare is not tested loosely; it is not tested at all. The suite reported success having checked nothing about the method this PR adds, including its auth posture.

Two changes, because the version alone fixes the instance and not the class:

1. the requirement becomes "0.7" (a caret range), so the pin moves with the published catalog instead of silently narrowing what CI can see;
2. a converse assertion — every control.* method the node SERVES is one the contract publishes — so a stale pin fails loudly instead of vacuously.

Proved load-bearing rather than asserted: restored to "0.6.0", the new test fails naming ["control.wallet.coinById"] while all four pre-existing tests still pass.

That direction also surfaced real drift the old suite structurally could not see: control.peers.ping is served but unpublished, now held in KNOWN_UNPUBLISHED with an honesty test mirroring KNOWN_PREEXISTING_DRIFT so the set can only shrink. Tracked as DIG-Network/dig_ecosystem#2455 — publishing it is its own reviewed change, not something this gate does blind.

How verified

• cargo test -p dig-wallet -p dig-node-service — green.
• The money-lie mapping is pinned by three tests over a real socket (fallback.rs): a dead port for unreachable, a one-shot local JSON server for provable absence, and a known coin mapped through with its spent_height. Both directions are pinned together on purpose — either alone is satisfiable by collapsing the other (always-error passes the first; always-Ok(None) passes the second).
• tests/server.rs exercises the validator and the pre-network ordering over the real HTTP surface.
• Contract conformance re-run against a genuine recompile (Compiling dig-node-control-interface v0.7.0), not a stale fingerprint.

Version

Workspace 0.104.0, dig-wallet 0.15.0 — minor: a new control method is a compatible new capability, and the contract surface is additive. The coins_by_ids change is a bug fix within that.

Closes DIG-Network/dig_ecosystem#2392

[MichaelTaylor3d]

CHANGES-REQUIRED — reviewed at head 9b89004ae0db586dde860a5c41bd34842cf561db.

The heart of this PR is right. Verified by reading:

• crates/dig-wallet/src/sage/fallback.rs:185-191 no longer collapses an error into an absence, and chia-query 0.6's get_coin_record_by_name_opt really does carry the distinction end to end (router.rs:337 -> peer_then_coinset_opt at router.rs:103-127 errors when both tiers fail; peer/mod.rs:518 treats only an empty coin-state list from a SUCCESSFUL response as absence, and request_coin_state returns spent coins, so the ticket's "including spent coins" criterion holds on the peer tier too, not just via coinset).
• I searched for the collapse pattern rather than trusting the two cited call sites. The only other Err(_) => Ok(None) on a chain read in dig-wallet is ChiaQueryLineage::parent_spend (fallback.rs:225), which is not on this path (non-gating note below). The two coins_by_ids callers use await? and propagate. ChainTransport::coin_record_by_id (chain.rs:179) delegates to CoinsetFallback, so it inherits the fix. EmptyFallback returns Ok(None) but reports is_live() == false, so coin_by_id refuses with NoChainSource before ever reaching it. No surviving collapse found.
• The node's coin_id validator matches the published contract's normalize_coin_id (dig-node-control-interface-0.7.0/src/params.rs:274-281) exactly, including the uppercase refusal the doc comment claims. That claim is true.
• Test load-bearingness (reasoned, not executed — I did not run the suite): an_unreachable_chain_is_an_error_never_a_missing_coin is load-bearing by construction (the reverted mapping returns Ok(None) on exactly the input it asserts is_err() for), and it is not vacuous because its paired absence test forbids satisfying it by always erroring. coins_wire_reports_a_spent_height_rather_than_asserting_null genuinely fails against the previous literal Value::Null. a_malformed_coin_id_is_refused_before_the_chain_is_ever_consulted discriminates via the two answers DIFFERING, which a reordered validator cannot satisfy.

Three findings block, ranked in the inline threads. None touch custody: §908 holds — this is a read, and no key material is anywhere on the path.

[Copilot]

@MichaelTaylor3d I've opened a new pull request, #202, to work on those changes. Once the pull request is ready, I'll request review from you.

[MichaelTaylor3d]

NON-GATING (does not block merge; resolving myself). Two observations from the same sweep, neither in this PR's scope:

1. The same error-into-absence collapse survives one file away, on a different read. crates/dig-wallet/src/sage/fallback.rs:225, in ChiaQueryLineage::parent_spend:

Ok(cs) => cs,
// The parent spend is not available (unspent / not found) — a clean "no lineage".
Err(_) => return Ok(None),

That is the identical shape this PR fixed in coin_record_by_id: a dropped connection, a coinset 500 or a rate-limit all report "there is no lineage". It is pre-existing, it is not on the coin_by_id path, and chia-query 0.6 already publishes the absence-aware get_puzzle_and_solution_opt that would fix it the same way. Worth its own ticket rather than widening this PR — the PR body's "no other path re-collapses it" is true of the coin_by_id path, which is what it claims.

2. The new CLI summary arm has no test and can print a false zero. crates/dig-node-service/src/control_cli.rs:327-337 adds a control.wallet.coinById arm to summarize, and crates/dig-node-service/src/control_cli.rs:530's summarize table was not extended to cover it — so the one new human-facing money line in this PR is unpinned. coin["amount"].as_u64().unwrap_or(0) would print 0 mojos for any amount that failed to parse, which is the same money-lie shape the PR exists to remove, in the human surface rather than the wire. Low severity (the wire mapper always emits a JSON number), but a row in the existing table asserting both the found-and-spent and the no such coin on chain lines would close it cheaply.

Both are tracked as follow-ups, not merge blockers.

[MichaelTaylor3d]

Correctness re-gate: PASS

Head reviewed: 5dc5a391bdb6da5f6b90800c897e73bb927477de (resolved from gh pr view --json headRefOid).

All five round-1 gating findings verified CLOSED at this head, by reading the code and running the tests, not by reading the commit messages.

1. Coin-id binding (the root) — CLOSED, and the test is non-vacuous.
The guard is crates/dig-wallet/src/sage/fallback.rs:206, comparing map_record's recomputed id against norm_hex(coin_id). coin_id_of (fallback.rs:107-127) genuinely recomputes SHA256(parent ‖ puzzle_hash ‖ amount) via chia_protocol::Coin::coin_id() — it does not echo a source-supplied field — so the check is self-certifying and covers substitution of any of the three components.

I probed vacuity in my own worktree (C:/tmp/worktrees/g2392p, detached at the same SHA; the review checkout was never mutated). Neutering the guard to if false && ...:

test ...a_record_for_a_different_coin_is_an_error_never_this_coins_record ... FAILED
  got Ok(Some(FallbackCoin { coin_id: "626443fb...", amount: 7, spent_height: Some(140), .. }))

The other three chain_failure_tests stayed green — so the test discriminates exactly this guard and nothing else.

On the max_peers: 0 trap raised in round 1: it does not apply to this guard, structurally.
The check does not live inside a tier. It is applied in dig-wallet to whatever chia_query::ChiaQuery::get_coin_record_by_name_opt returned, and that call is router.rs:337-347 → peer_then_coinset_opt — peer, peer-retry, then coinset. There is one call site and one check above the tier split, so a record substituted by the peer tier is rejected by the same line as one substituted by coinset. The 0-peer fixture is therefore a determinism/offline choice, not a coverage gap, for this property. (It would still be a gap for any property whose behaviour differs per tier — none is asserted here.)

2. False absence-guarantee wording — CLOSED. fallback.rs:177-193, rpc.rs:192-197, rpc.rs:944-..., control.rs:1424-... and the SPEC.md row all now say Ok(None)/coin: null means a source answered "no such coin" and explicitly that it is not proof of absence, deferring corroboration to dig_ecosystem#2456. I checked specifically for the over-claim the scope note warns about: the PR nowhere claims a guarantee only #2456 could provide. The positive direction is claimed as self-certifying, which is true locally and needs no second source.

3. SPEC closed-enumeration split — CLOSED at all four sites (SPEC.md §auth exceptions, the method table, the CLI verb list, and the -32040/-32042/-32043 error rows).

4. Duplicated coin-id rule — CLOSED. dig-node-control-interface promoted dev-dep → real dep at "0.7" (crates/dig-node-service/Cargo.toml:86; Cargo.lock resolves 0.7.0), and wallet_coin_id_param (control.rs:1346-1367) consumes WalletCoinByIdParams::validated(). The local function contributes only JSON extraction and error shaping — no second copy of the rule. The caret range plus the new the_contract_publishes_every_control_method_the_node_serves conformance test closes the "pinned at 0.6.0, so the gate iterated a catalog that had never heard of this method" hole.

5. pinned's doc comment — RESTORED (control_cli.rs:409); mojos/height each carry their own.

Extras verified: mojos() prints amount unknown rather than 0 mojos (pinned by coin_by_id_summary_never_prints_zero_for_an_unreadable_amount), and the record fields are read with Map::get rather than Map indexing — the panic path is real (serde_json::Map's Index<&str> panics on a missing key, unlike Value's) and is gone.

Tests run on this head (CARGO_TARGET_DIR outside the checkout):

• cargo test -p dig-wallet --lib sage::fallback — 6 passed
• cargo test -p dig-node-service --lib control:: --features testkit — 36 passed
• cargo test -p dig-node-service --test control_contract_conformance --features testkit — 5 passed
• cargo test -p dig-node-service --test server --features testkit coin — 1 passed
  All 15 required checks green, including Test + coverage and Clippy (coverage floor taken from the CI gate; I did not re-run llvm-cov locally).

dig-constants check. (1) Nothing in this diff is a shared/cross-repo value defined locally: the one cross-repo rule introduced (coin-id well-formedness) is consumed from dig-node-control-interface, which is the canonical home for the control-plane contract, not dig-constants. (2) Nothing here hardcodes a literal dig-constants publishes; DIG_ASSET_ID is untouched and still sourced from digstore_chain::dig. No finding.

§2.5 (beautiful code). The mappers are small, single-purpose and narratively ordered; every WHY comment states a real reason (the tier bypass, the asset: null non-classification, the spent_height literal removal). coins_wire emitting c.spent_height instead of a hardcoded null is a genuine improvement, and coins_wire_reports_a_spent_height_rather_than_asserting_null is a test that fails on reversion.

One non-gating nit posted inline and resolved by me.

[MichaelTaylor3d]

Independent re-gate at 5dc5a39 — reviewer PASS, security CHANGES-REQUIRED (1 HIGH), and 280e1d0 closes the HIGH.

Two fresh contexts re-gated this PR while 280e1d0 was being written; they converged on the same defect independently, which is worth stating plainly since it is the strongest evidence the finding was real.

Correctness leg — PASS. All five round-1 findings verified genuinely closed, not reworded. The coin-id binding was proved non-vacuous by a revert probe in a scratch worktree: neutering the check to if false && … makes exactly a_record_for_a_different_coin_is_an_error_never_this_coins_record fail (got Ok(Some(FallbackCoin{coin_id:"626443fb…"}))) while the other three chain_failure_tests stay green. The max_peers: 0 fixture trap does not apply to this property: the check sits in dig-wallet above the tier split, on whatever get_coin_record_by_name_opt returned, so a peer-substituted record and a coinset-substituted one hit the same line.

Security leg — the HIGH, now addressed by 280e1d0. The binding covers exactly parent_coin_info ‖ puzzle_hash ‖ amount; created_height and spent_height are not covered and are one unauthenticated peer's word. A peer that watched the mempool knows a pending coin's preimage, so it can return the genuine coin fields (binding passes) with created_height: <peak> for a bundle that never landed. 280e1d0 corrects the rustdoc, and its commit message names the same asymmetry: a false negative keeps polling, a false positive stops and records.

What 280e1d0 does not cover — filed as #2462 (MVP), sibling to #2456 and explicitly outside it (#2456 makes an absence trustworthy; #2462 makes a presence trustworthy):

• dig-node-control-interface 0.7.0 src/results.rs:620-623 still instructs the consumer to "read the created coin's id for a created_height, and the funding coin's id for a spent_height", and :563-566 states both as plain fact — the contract crate licenses exactly the read the node's rustdoc now warns against.
• control.rs:1439-1440 says "A non-null coin IS bound to the id asked for" and is silent on the limit; the SPEC.md control.wallet.coinById row discloses only the null caveat. Both are incomplete rather than false, so neither is gating.

Non-gating, for whenever the lines are next touched: wallet_read_error's third param is named address but only the InvalidAddress arm consumes it, which coin_by_id can never reach (control.rs:1448) — inert, latent mislabel. ChainFallback::coin_record_by_id's trait doc (fallback.rs:50) states no binding requirement; only CoinsetFallback enforces it, so a future impl inherits nothing. coinById is a third consumer of the shared fallback_rate bucket (rpc.rs:991) — a pre-existing class, no new primitive. Items already on #2457 were re-found and not re-raised.

Checked and clear: every Ok(Some(_)) exit passes the binding; no Err(_) => Ok(None) collapse remains on the by-id path; params validated before any network call via the contract's own WalletCoinByIdParams::validated(); no panic reachable from a hostile response (the serde_json::Map index sibling is closed, and mojos/height print amount unknown/pending rather than a fabricated 0); no peer-controlled string reaches a log; §908 intact — one hex string in, no key material anywhere.

Both gate checkouts were read-only and are removed. Posted by a second orchestrator session that has since stood down; this PR is yours.

[MichaelTaylor3d]

Reopening as a fresh PR from the same branch — GitHub has this PR permanently flagged as a stack parent and refuses every merge path with "this pull request is part of a stack and must be merged using the asynchronous merge REST API."

The stack came from #202 (a WIP Copilot SPEC PR based on this branch), which I closed, and whose branch I have now deleted. Neither cleared the flag; gh pr merge, the REST sync endpoint, --admin, and every documented async-endpoint spelling all refuse, and the repo has no merge queue.

Nothing is lost. Same branch, same head 280e1d0, all checks green, all three review threads resolved. The pre-merge security audit returned PASS on 5dc5a391, and its one doc finding landed as 280e1d0 (the coin-id binding authenticates identity, not chain state — see dig_ecosystem#2456).