fix(wallet): stop reporting confident answers the replica has not earned

[MichaelTaylor3d]

DO NOT MERGE — gate round not yet run.

Stage 3 of the MVP feature path (epic DIG-Network/dig_ecosystem#2760): the wallet must not lie about money.

Closes nothing in this repo — both tickets live on the superproject and must be closed manually:

• DIG-Network/dig_ecosystem#2878
• DIG-Network/dig_ecosystem#2666

What a caller saw, and sees now

dig_ecosystem#2878 — identity-scoped reads routed on the bare sync flag

A client logs in with its puzzle hashes and holds 1.599 XCH on chain. The node's catch-up
completed over a different address, so the replica was never asked to follow this client.

	before	after
get_sync_status	selectable_balance: 0, synced_coins: 0, total_coins: 0 — every client renders this as "synced, balance 0"	synced_coins: 0, total_coins: 1 — renders as syncing, never a settled zero
get_coins	[] — reads downstream as "a chain was consulted and this wallet is empty"	the real chain coin, 1_599_000_000_000 mojos

get_sync_status and wallet_coins now call replica_covers_client_scope(&identity), which asks
the same containment question replica_is_authoritative already asks, but against the connected
client's scope rather than the node's followed set.

The scope choice is the fix, not an incidental. Under §908 a node holds no custody and may hold
no registrations, so its followed set is EMPTY, every recording trivially contains it, and routing
these reads through replica_is_authoritative would have been vacuous — serving the uncovered
client a synced zero exactly as before. See the revert-proof below.

dig_ecosystem#2666 — a refused writer reported synced

A node whose session corroboration refused (PeerTrust::Discovered) has every frame dropped before
any DB write, including the peak. Its replica falls behind by an unbounded, invisible amount.

• Before:{phase: "synced", peak_height: <frozen>}, indefinitely, provided a catch-up had
  completed in any earlier run. A user watching a node refused by every quorum round — a partition,
  or a hostile peer set — was told everything was fine.
• After:{phase: "syncing"}. The Synced arm now also requires observed.session_may_write.

No new phase token, so WalletSyncPhase in dig-node-control-interface is unchanged and no
consumer's whole-response parse can break. That was the explicit trade the ticket asked for, and it
avoids repeating dig_ecosystem#2609.

Blast radius checked (gitnexus, per-worktree index @ 5006593)

symbol	risk	direct callers	disposition
synced	CRITICAL (36 impacted)	wallet_coins, coins_by_ids, require_authoritative_coins	NOT MODIFIED. Only the identity-scoped call site (wallet_coins) was re-pointed. coins_by_ids (arbitrary ids, covered by #227) and require_authoritative_coins (spend inputs) still route on it, deliberately.
replica_is_authoritative	CRITICAL (44 impacted)	balance_for_address, coins_for_address, replica_coin_by_id	Refactored to delegate to the extracted replica_covers(&self.followed_set()). The expression is identical, so all three address-scoped reads are behaviour-unchanged.
get_sync_status	HIGH (41 impacted)	dispatch	Modified.
followed_set	CRITICAL (35 impacted)	replica_is_authoritative only	Unchanged.
scoped_identity	LOW (10 impacted)	4	Unchanged.
SyncHandle::status	—	supervisor + control status	One condition added to the Synced arm.

WARNING — HIGH/CRITICAL risk. replica_is_authoritative and synced both sit on the money read
path. The change to the former is a pure extraction; the latter was left alone precisely because of
its blast radius.

Diff scope confirmed against origin/main: two new private helpers, two routing call sites, one
ladder condition, tests, SPEC, version. No other symbol touched.

Fixture design — why these tests can see the defect

uncovered_but_funded_client needs three properties simultaneously, and dropping any one makes it blind:

1. The catch-up completed (db.is_synced() == true) — asserted in the fixture. Without it the
   pre-fix code already reports not-synced and the test proves nothing.
2. The recorded coverage names a real, DIFFERENT address (bb..) — an honest control. A fixture
   recording no coverage would also pass against an implementation that merely demanded some
   coverage exist, so the control is what forces the question to be asked about the client's scope.
3. The money is on the CHAIN, not in the replica. An unfunded fixture cannot distinguish
   "zero because unscoped" from "zero because empty" — which is the entire defect.

For #2666 the fixture is a_completed_catch_up_still_reports_synced_while_watching_addresses with
exactly one input varied (set_trust(false)); that control stays green, so the pair isolates the
refusal itself. The watched set is deliberately non-empty so the fixture sits on the path that
actually reaches Synced, and the peer tier is UNOBSERVABLE so is_following answers true and
cannot be the thing that fails the assertion.

Both fixes also carry a positive control, so neither is satisfied by an implementation that simply
never reports synced.

Revert-proof (committed first, then reverted — never git checkout on live work)

• Placement proof for #2878: swapping replica_covers_client_scope(&identity) to
  replica_is_authoritative() — the nearest wrong fix — turns both new tests red
  (synced=0 total=0 again). The tests pin the placement, not just the outcome.
• #2878 red before the fix: both tests failed for the right reason on 4acf298 (commit
  test(wallet): red ..., pushed before the implementation).
• #2666 red before the fix:left: Synced, right: Synced on the assert_ne!.

Fixtures corrected (they were under-specified, not wrong)

Four pre-existing fixtures latched a flag without stating what it meant, and the new predicates
correctly fail closed on them:

• synced_get_coins_reads_from_db_not_fallback and
  identity_scoped_reads_return_client_balance_never_other_wallets set initial_sync_complete with
  no recorded coverage; they now call record_coverage for the scope they intend to be covered.
• phase_is_syncing_when_caught_up_but_no_peer and phase_ladder_not_started_syncing_synced attach
  a peer without resolving its trust; they now call set_trust(true).

Verification (run locally — the badge is not the evidence)

• cargo test -p dig-wallet --lib → 600 passed, 0 failed (on the squashed tree).
• cargo test -p dig-node-service → all suites green, 0 failed.
• cargo clippy --all-targets --all-features -- -D warnings → clean.
• cargo fmt --all → applied.
• cargo llvm-cov -p dig-wallet --lib --summary-only → 81.67% lines (gate: --fail-under-lines 80).

Not run against a real chain. Every fixture here is WalletDb::open_in_memory() plus
MockFallback. The claim proven is that the routing no longer answers from an uncovered scope; the
claim NOT proven is that a live catch-up records coverage matching a real client's login scope on
mainnet. That needs a node with a chain source and a logged-in dig-app.

Version

0.126.1 → 0.126.2 (patch). Two behaviour-correcting fixes; no API added, removed, or renamed,
and no wire/contract token changed. CHANGELOG.md is left to git-cliff.

SPEC

SPEC.md §18.6f gains the identity-scope containment invariant (replacing the "does NOT yet hold /
tracked as #2878" sentence) and §18.6d gains the refused-writer invariant.

Not attempted in this batch, and why

• dig_ecosystem#2763 (no in-flight coin reservation) — real design work: reservation must hook
  three separate broadcast call sites in rpc.rs (:2707, :2808 with a caller-supplied
  broadcaster, :3130), plus a release lifecycle and resubmission. A reservation that hooks some of
  them is a partial guard on the money path, which is worse than none. status:paused.
• dig_ecosystem#2764 (get_pending_transactions hardcoded empty) — the ticket itself makes it
  depend on #2763. The alternative honest answer (an error instead of a false empty list) is a
  behaviour change on a polled surface and deserves its own decision, not a drive-by.
• dig_ecosystem#2762 (fixed 50-address window, no hardened keys) — independent of these two and
  substantial (custody derivation + subscription set + import scan-ahead). status:paused.

Epic #2760 must stay open: three of its five named children are unlanded.

[MichaelTaylor3d]

loop-security — interim finding 1 (audit in progress)

Audited head: 7deb27b97bd901238814bfafaddfc8171a89ec66 (resolved via gh pr view 246 --json headRefOid), merge-base 4acf2989.
Read from git objects + my own detached worktree at C:/tmp/worktrees/sec2760. The lane's worktree (dn-wallet-truth) was not touched.

Verified so far (no defect)

• The replica_covers extraction is behaviour-preserving.replica_is_authoritative (crates/dig-wallet/src/sage/rpc.rs:852) is now self.replica_covers(&self.followed_set()), and replica_covers (:872) is the identical expression with scope substituted. The two address-scoped callers (:1247, :1394) and replica_coin_by_id (:1629) are unchanged in meaning.
• The spelling claim holds.login (:1993) normalises every supplied puzzle hash through normalize_ph, and scoped_identity (:2026) normalises the config fallback, so the set handed to CoveredSet::from_hex is byte-identical to the one handed to db.coins_scoped / db.balance_scoped. No 0x/case door re-opens the defect.
• The empty-identity case is not a hole.CoveredSet::covers is vacuously true on an empty scope, so a not-logged-in session gets covered == true — but get_sync_status gates every use of db_synced behind tracking and forces (0, known.max(1)) when not tracking (:2114). Moot.
• is_following genuinely does not cover the refusal.sync_supervisor.rs:577 returns true whenever either peak is None, and otherwise tolerates a 4-block gap. On a node whose chain transport reports no peak, a refused writer would have reported Syncedforever, not merely for 4 blocks. Fix 2 closes a real hole.

FINDING 1 — a confident zero balance is still reachable, on a surface with nothing to qualify it

crates/dig-wallet/src/sage/rpc.rs:2231 — token_record

token_record reads the replica unconditionally. It asks neither the new replica_covers_client_scope nor even the old bare synced() flag; it calls self.db.balance_scoped(None, &identity) and returns the result as both balance and selectable_balance. It is reached from three dispatch-table methods — get_token (:3931), get_cats (:3923), get_all_cats (:3927) — all listed in the exported method set at :1037-1039.

Scenario (this PR's own fixture, one method over): the uncovered_but_funded_client state — completed catch-up, coverage recorded over bb..bb, client logged in at aa..aa, 1.599 XCH on chain, replica empty for that scope. After this PR:

call	answer
get_sync_status	synced_coins: 0, total_coins: 1 — honest (fixed here)
get_coins	the real 1_599_000_000_000 coin — honest (fixed here)
get_token {"asset_id": null}	balance: 0, selectable_balance: 0

TokenRecord carries no completeness field at all — no synced/total pair, no source, no peak. get_sync_status's zero is at least accompanied by a synced_coins < total_coins claim telling the client not to trust it; token_record's zero is a bare figure with nothing beside it. That is the stronger form of the same lie.

This is pre-existing, not introduced by this diff — token_record never routed. I am not gating the diff on it. But two things follow:

1. #2878's acceptance bar is not met by this PR alone. The ticket's user-visible symptom is "a wallet holding 1.599 XCH is told it has 0". A client that renders its XCH balance from get_token/get_cats rather than from get_sync_status.selectable_balance still sees exactly that. Which call dig-app makes needs checking before #2878 is closed.
2. The SPEC.md text added here reads more broadly than the code it describes. The new heading is "the IDENTITY-scoped reads MUST ask the same containment question about the CLIENT's scope", and the body then names two of the four (get_sync_status, wallet_coins). token_record and get_cats are identity-scoped reads that do not ask it. A second implementation reading that invariant would believe the class is closed. Either narrow the SPEC sentence to the two reads it actually governs, or state the residue explicitly the way the pre-fix text did for #2878.

Secondary note, same file: get_sync_status's own selectable_balance (:2106) is still the raw db.balance_scoped figure and is 0 for an uncovered scope. Defensible — it ships beside the synced_coins < total_coins qualifier — but it means the fix makes the completeness claim honest, not the number.

Continuing: reproducing the vacuity claim, judging the three refusals, and checking the four fixture repairs.

[MichaelTaylor3d]

loop-security — interim: EXECUTED PROBE RESULTS (head 7deb27b)

All four probes run in my own detached worktree C:/tmp/worktrees/sec2760, restored to a clean 7deb27b afterwards (git status --porcelain empty). The lane's worktree was never touched.

Baseline at head: 600 passed; 0 failed; 1 ignored.

Probe 1 — the vacuity claim. CONFIRMED, and it is the strongest thing on this PR.

Mutation: route both identity-scoped reads through replica_is_authoritative (the followed-set predicate) instead of replica_covers_client_scope, i.e. the "obvious" fix the lane says would have been a no-op.

an_uncovered_client_scope_reads_its_coins_from_the_chain ... FAILED
an_uncovered_client_scope_is_never_reported_as_a_synced_zero ... FAILED
a_covered_client_scope_still_reads_from_the_replica_and_reports_synced ... ok

and the failure text is verbatim the defect:

an uncovered scope was reported fully synced (synced=0 total=0)
assertion `left == right` failed: an uncovered scope was served the replica's empty coin set
left: 0
right: 1599000000000

So the claim holds exactly as written: the followed set is empty, every recording contains it, and the funded client is served a synced zero. The placement of this fix is load-bearing and the tests discriminate it. The control stays green, so the pair is not satisfied by an implementation that simply never reports synced.

Probe 2 — fixture property 2 (coverage over a DIFFERENT address). CONFIRMED.

Mutation: weaken replica_covers from containment to mere existence — state.initial_sync_complete && state.covered.is_some(). Both new tests FAIL with the same messages; the control passes. The fixture genuinely forces the containment question rather than "does some coverage exist", which is what recording bb..bb while the client logs in at aa..aa buys.

Probe 3 — fix 2's revert-proof. CLEAN AND PRECISELY TARGETED.

Mutation: delete && observed.session_may_write from the Synced arm (sync_supervisor.rs:481).

test result: FAILED. 76 passed; 1 failed
a_refused_writer_is_not_reported_as_synced

Exactly one test, and it is the new one. The guard is load-bearing and its test is the sole sentinel for it.

Probe 4 — FINDING 2 (defense-in-depth, gate-rot). Adding the guard made an existing #2501 guard vacuous.

Mutation: delete observed.peers >= 1 from the Synced arm, leaving session_may_write.

test result: ok. 600 passed; 0 failed

The entire suite survives.set_connected(0) (sync_supervisor.rs:531) also clears session_may_write, so the newly added clause now subsumes the peer-count clause in every fixture — including phase_is_syncing_when_caught_up_but_no_peer (sync_supervisor/tests.rs:1037), whose named purpose is "a caught-up replica with no peer is not currently synced" (T5, #2501) and whose second assertion this PR just made over-determined.

To be precise about what this is and is not:

• It is not a defect in shipped code. peers >= 1 is still present and correct.
• It is not the repair weakening an assertion in the ordinary sense — the added set_trust(true) was necessary, and without it the FIRST assertion in that test would have flipped to Syncing and the test would have proved nothing at all. The repair was right.
• It is gate rot: after this PR, no test in the 600 anywhere guards the #2501 peer-count property independently. A future refactor can delete observed.peers >= 1 and the suite stays green.

The state that would discriminate it — peers == 0 while session_may_write == true — is currently unreachable through SyncHandle's public API, because set_connected(0) force-clears the flag. So peers >= 1 is presently a redundant belt rather than an independent guard. That is a defensible design, but it should be recorded deliberately rather than discovered later: either note in the Synced arm that the peer clause is subsumed by session_may_write under the current set_connected contract, or give T5 a probe that survives it. Follow-up ticket, not a gate.

Still to do: the three refusals (#2763 / #2764 / #2762), and the reachability/cost question on the new fallback routing.

[MichaelTaylor3d]

loop-security VERDICT: PASS

Audited head: 7deb27b97bd901238814bfafaddfc8171a89ec66 — resolved from the remote at the start AND re-resolved immediately before posting this; it did not move during the audit. Merge-base 4acf29895953f7dd06570ea3ab4d87941ad82bdb. Still DRAFT; I am not merging and not undrafting.

No security defect in this diff. Every probe is recorded in the two comments above, executed in my own detached worktree C:/tmp/worktrees/sec2760 and restored to a clean 7deb27b. The lane's worktree was never touched; no shared checkout was mutated.

Areas checked, and why each is clear

area	verdict
Secrets / credentials	Nothing introduced, logged or committed. Cargo.lock changes exactly one line, the version. No dependency added, updated or loosened.
Custody / privilege	No key handling, no signing, no elevation, no service/registry/FS-permission write. 908 holds: the node still signs nothing. The Synced arm change only removes confidence; it grants nothing.
Input / boundary	The one new attacker-influenced input is the client login puzzle-hash list flowing into CoveredSet::from_hex. Normalised at rpc.rs:1993 before storage, from_hex normalises idempotently, and a malformed member simply fails containment and routes to the chain, which is fail-open toward the truthful tier. No unwrap on it, no path/shell/SQL surface, no deserialization change.
Crypto / protocol	Untouched. No new WalletSyncPhase token, so dig-node-control-interface is unchanged and no consumer whole-response parse can break (2609 lesson correctly applied).
AuthZ / exposure, WHO CAN INVOKE THIS	No new method; get_sync_status and get_coins were already on the dispatch table (rpc.rs:1030,1035). The control surface binds loopback only by default, non-loopback being opt-in (dig-node-service/src/config.rs:179, 1662), so the caller is the local user app, not an anonymous peer. No pre-existing guard was silently revoked.
Amplification / cost asymmetry	Real but bounded and non-remote. An uncovered scope now sends every get_coins poll to the chain-oracle fallback where it previously read the local DB. Because the invoker is loopback-only this is a local client poll rate, not attacker-elicited work, and it is the honest direction.
Guard specificity	The one-off variant is why this fix exists, and it was handled: coverage is asked as containment, not existence. I mutated it to mere existence (probe 2) and both new tests went red, so a one-address-short recording cannot slip past.
Persisted state as untrusted	covered is a local TEXT column written only by the node own catch-up. A missing or empty recording covers nothing and fails closed for every non-empty scope (coverage.rs:88). No unbounded growth, nothing forgeable.

The claims I was asked to verify hardest

1. The vacuity claim is TRUE and I reproduced it. Routing through replica_is_authoritative turns both new tests red with the literal defect text (synced=0 total=0, and left: 0 / right: 1599000000000) while the control stays green. This is a fix, not a fix-shaped no-op, and the tests pin the placement.
2. The extraction is behaviour-preserving.replica_covers (rpc.rs:872) is the identical expression with the scope made an argument; rpc.rs:1247, :1394 and :1629 are unchanged in meaning.
3. is_following genuinely does not cover the refusal.sync_supervisor.rs:577 answers true whenever either peak is None. On a node whose chain transport reports no peak, a refused writer reported Syncedforever, not for four blocks. Mutating the new guard away fails exactly one test, the new one.
4. The coins_by_ids deliberate leave is CORRECT, not an oversight.rpc.rs:2187-2206 backfills every id missing from the DB from the chain, so an uncovered scope coin is served truthfully by construction. The 2878 class cannot reach it.
5. The require_authoritative_coins deliberate leave is CORRECT for this ticket. It gates spend inputs read from db.unspent_coins, the node OWN custody, unscoped, so the client-identity question does not apply. Separate and pre-existing: it routes on the bare synced() rather than replica_is_authoritative(), so a widened followed set yields an incomplete spend-input set. The failure direction is insufficient-funds, and the chain enforces no-double-spend, so this is not a custody risk. Follow-up note only.
6. The four fixture repairs did not weaken any existing assertion. The two rpc repairs add record_coverage and retain every original assertion. The two supervisor repairs add set_trust(true), which was NECESSARY: without it the first assertion in phase_is_syncing_when_caught_up_but_no_peer would have flipped to Syncing and the test would have proved nothing at all. One side effect, FINDING 2 below.

The question you asked: can the wallet still report a confident figure it has not earned?

Yes, but not through anything this diff touches.token_record (rpc.rs:2231) reads db.balance_scoped with no gate at all, and is reached by get_token, get_cats and get_all_cats. TokenRecord carries no completeness field, so its zero has nothing beside it to qualify it, a strictly worse shape than the zero this PR just fixed. Detail in FINDING 1 above.

This diff makes the situation better and internally inconsistent at the same time: for the uncovered funded client, get_coins now returns the real 1.599 XCH while get_token and selectable_balance still return 0. Before, both were wrong together. That inconsistency is why FINDING 1 must be worked as part of this family rather than filed and forgotten.

One consequence that makes FINDING 1 permanent rather than transient:login (rpc.rs:1988) records identity and does not enrol. Enrolment is the separate control.wallet.watch door (dig-node-service/src/control.rs:2102), which takes G1 public keys, and the catch-up subscribes the FOLLOWED set. So for a client that logs in without ever enrolling, coverage will never contain its scope: get_sync_status reports syncing indefinitely and every balance figure stays 0 indefinitely. Honest, but non-converging. Syncing implies a convergence that will not happen, which is the same criticism the refused-writer question raises, with the same answer: less-wrong, not right.

Judging the three refusals

• 2763 (coin reservation): SOUND, and the argument is stronger than the lane stated. The inventory undercounts. There are sevenbroadcast( call sites at head, rpc.rs:2707, 2808, 3131, 3164, 3345, 3471, 3656, not three. A partial reservation across seven paths plus a release lifecycle plus resubmission is exactly the half-a-money-guard shape that gives false confidence, and refusing it here is right. The residue is bounded: without reservation, two sends in the confirmation window select the same coin and the chain rejects the loser. That is a reliability failure, not theft or fund loss. Correct to refuse; the ticket should carry the real site count.
• 2764 (get_pending_transactions): SOUND ON THE API, INCOMPLETE ON THE CHEAP HALF.transactions: Vec<_> genuinely cannot express unknown, and erroring on a polled surface is a real behaviour change against Sage parity. But the ticket title itself says on a comment that is no longer true, and that comment is still there at rpc.rs:2450: This PR has no spend/submission path, so there are no pending transactions. Seven broadcast sites say otherwise, and send_tip_with (:2808) takes a caller-supplied broadcaster that bypasses the unset field. Deleting a false justification from a money surface is a one-line change in a PR whose entire subject is money honesty. Not gating, but it should have been taken.
• 2762: CORRECTLY OUT OF SCOPE. It is the 50-address derivation window and missing hardened keys, it is status:paused, and it is a coverage-of-the-key-space defect rather than a routing-honesty one. Nothing in this diff makes it better or worse. Not avoidance.

Findings, ranked

#	severity	gates	where
1	Live money-honesty defect, pre-existing, OUTSIDE the diff	No	rpc.rs:2231token_record, reached by get_token / get_cats / get_all_cats, returns a bare confident zero balance with no completeness field.
1a	Low, IN-DIFF, doc honesty	No, but recommended before merge	SPEC.md. The new heading the IDENTITY-scoped reads MUST ask the same containment question is unqualified while the body fixes two of four. The text it replaced was scrupulously honest about its residue (this invariant does NOT yet hold on that surface; tracked as 2878). One sentence naming token_record / get_cats as the remaining residue restores that discipline. No code change.
2	Defense-in-depth, gate rot	No	sync_supervisor.rs:481. Deleting observed.peers >= 1 from the Synced arm now survives the entire 600-test suite, because set_connected(0) (:531) also clears session_may_write. The 2501 peer-count property is no longer independently guarded. Follow-up ticket.
3	Note	No	rpc.rs:2450, false comment on the pending-transactions surface.
4	Note	No	require_authoritative_coins (rpc.rs:2656) gates spend inputs on the bare synced() rather than coverage. Fails toward insufficient-funds; not custody-critical.

Why PASS rather than CHANGES-REQUIRED on FINDING 1

Because blocking this PR does not fix it and makes things worse. token_record lied before this diff and would lie identically if this diff never merged, while get_sync_status and get_coins would go on lying too. The surviving 2.6 stop is a change that MAKES a surface lie about money; this change makes two surfaces stop. Gating here would trade a real improvement for nothing.

Two conditions, asserted rather than requested, since they are the orchestrator to enforce:

1. Do not close dig_ecosystem 2878 on this PR alone. Its user-visible symptom is a wallet holding 1.599 XCH being told it has 0, and a client rendering its balance from get_token / get_cats still sees exactly that. File FINDING 1 as a sibling in this family and check which call dig-app actually makes before closing.
2. The 2.6 caveat stands as the lane stated it. Nothing here ran against a real chain; every fixture is open_in_memory() plus MockFallback. What is proven is that the routing no longer answers from an uncovered scope. What is NOT proven is that a live mainnet catch-up records coverage matching a real client login scope, and given that login does not enrol, that is the assumption most likely to be false in production. Stating it, not blocking on it.

Nothing was handed to Copilot. Money path throughout, per instruction.