fix(wallet): honest peak sync flag, adaptive derivation window, coin reservation, real pending set

[MichaelTaylor3d]

DRAFT — DO NOT MERGE. Gate round not started.

Wallet-correctness batch for epic dig_ecosystem#2760. One branch, one PR, four dig-node tickets.

Closes#293
Closes#252
Closes#251
Closes#250

The three older tickets were TRANSFERRED from the superproject (dig_ecosystem#2762/#2763/#2764) and
live here now as #252/#251/#250. The parked PR #216 still names the superproject numbers, and a
submodule PR's Closes keyword resolves against its OWN repo, so those keywords closed nothing.

Revived, not rewritten — and the evidence for that call

PR #216 (loop/2762-wallet-correctness, head 12ffe2d6, parked since 2026-08-13) carried 1,395 lines
covering #252/#251/#250 and sat 56 commits behindmain, which had meanwhile grown +12,634
lines across the wallet crate. That is the shape of a rotted branch, so the revive-vs-rewrite call was
measured rather than assumed:

• Squash-merging it onto main produced exactly one conflict hunk per file, in db.rs and
  rpc.rs, and both were purely additive test blocks at the end of the test module — main's
  dig-node#283 offer-rekey tests landing beside this branch's reservation tests. Both sides kept; the
  only real repair was the closing brace the two sides shared.
• The result compiled clean against current main on the first try, meaning every production
  wiring point the branch attaches to (push_signed_bundle, spendable_coins,
  get_pending_transactions, WalletCustody::build_signer) survived main's evolution unchanged.
• custody.rs — 542 of the 1,395 lines — is untouched on main, so the derivation work applied
  verbatim.

Rewriting would have re-derived 1,395 lines of correct, well-documented work to avoid one brace.
Revived.

#293 — wallet peak reported synced: true on a stale replica

chain_peak computed synced from db.is_synced() — the initial_sync_completelatch — and
returned on the replica-served path without ever consulting replica_answer_is_current.
dig_ecosystem#2869 replaced that latch with a measured predicate at the balance and coin reads and left
this endpoint behind. Measured on a running node: sync-status said syncing while peak said
synced: true, in the same process at the same moment, on a replica 1,875 blocks behind. peak is the
endpoint a client uses to bound a confirmation, so the falsehood landed on the read that decides whether
money has settled.

The fix is structural, not two sites corrected separately.replica_answer_is_current is now the
only way any read produces synced: true; every other site writes the literal false on a
fallback-tier answer, where a third party's height says nothing about the replica. The doc comment that
already claimed the two endpoints could not disagree now states that basis instead of asserting it.

The regression test fails on the unfixed code

the_peak_is_not_reported_current_while_the_balance_read_calls_the_same_replica_stale, run against
chain_peak before the fix:

thread '...the_peak_is_not_reported_current_while_the_balance_read_calls_the_same_replica_stale'
panicked at crates\dig-wallet\src\sage\rpc.rs:7002:9:
`peak` called a replica 530 blocks behind current
test result: FAILED. 1 passed; 1 failed

Fixture design, stated because this is where a false green would be born:

• The replica must satisfy initial_sync_completeand lag past FOLLOWING_TOLERANCE (4 blocks).
  The latch is what the defect trusts, so a fresh replica fails the latch and a caught-up one is
  genuinely current — neither can exhibit the defect. The test asserts the latch is set before
  asserting anything else.
• The gap is PEERS_AHEAD_BY = 530, drawn from the live measurement rather than invented, and two
  orders of magnitude past the tolerance.
• It asserts agreement with the balance read, not merely !synced. Disagreement between the two
  endpoints is the reported bug, and a fix applied at one site while the other kept its own predicate
  would satisfy a lone !synced assertion and re-open the same class of defect.
• A control (a_replica_level_with_its_peers_still_reports_a_synced_peak) passes on the unfixed
  code, so synced: false hardcoded in place of synced: true cannot green the pair.

A pre-existing test had to change, and it is worth naming:
the_peak_is_the_replicas_when_the_replica_has_one ran with an UNOBSERVABLE peer tier and asserted
synced: true — the exact falsehood #293 removes. It now fixes the peer tier level with the replica;
its subject is the height's provenance, and the freshness flag has its own tests.

The other three

• dig-node derives a fixed 50-address window and no hardened keys — an imported wallet silently under-reports its balance #252 — the derivation window was a fixed 50 indices of the UNHARDENED tree only. An imported
  wallet whose history reached index 50 had coins the node never subscribed and never counted, and a
  hardened coin — where farmer and pool rewards land — was invisible at every index, while the node
  reported synced over both. Now both trees, a 500-index floor, and a gap-limit scan that follows
  observed usage, bounded at 25,000 so a hand-edited manifest cannot turn an unlock into unbounded key
  derivation. The signer and the watched set widen together: widening only the watched set converts
  "cannot see the coin" into "can see it and cannot spend it", which is worse and reads as a send bug.
• dig-node reserves no coins in flight — two sends in the confirmation window pick the same coin #251 — push_signed_bundle now reserves the bundle's inputs, only once the mempool has
  accepted it. A refusal reserves nothing (those coins were never committed, and holding them would
  strand the user's money over a spend that will never happen), and a reservation failure does not fail
  the push — the bundle is already in a public mempool, and reporting a push that did happen as an
  error is a worse lie than the double-selection this guards against. spendable_coins prunes expired
  reservations and then selects only unreserved coins.
• dig-node get_pending_transactions returns a hardcoded empty list, on a comment that is no longer true #250 — get_pending_transactions returned a hardcoded empty list. It now reports the reserved
  in-flight set, which is only truthful because dig-node reserves no coins in flight — two sends in the confirmation window pick the same coin #251 landed first: a pending-transaction report with
  nothing tracking in-flight coins is a different falsehood, not a fix.

Blast radius checked

chain_peak — callers, repo-wide: dig-node-service/src/control.rs:1931 (the control.wallet.peak
handler) and two doc references in rpc.rs / sync_supervisor/tests.rs.
sync_supervisor/tests.rs:1619 asserts the STATUS path never reaches chain_peak, which this change
does not alter. No production caller reads the synced field other than through the control surface,
and no in-tree consumer branches on it. replica_answer_is_current gains one call site and keeps its
three existing ones. WalletCustody::build_signer / DEFAULT_DERIVATION_COUNT are internal to
dig-wallet; no other crate names them. The diff touches only SPEC.md, Cargo.toml, Cargo.lock and
four files under crates/dig-wallet/src/sage/.

Risk: MEDIUM. No HIGH/CRITICAL. chain_peak's synced flag changes value for one class of caller —
a node whose replica is behind, or which has no observable Chia peer, now reports false where it
reported true. That is the point of the change, but a consumer treating synced: false as an outage
rather than as staleness will become more conservative. The height is always still served.

Boundary this PR does NOT cross

§908 binds absolutely: the node signs NOTHING on the user's behalf and the user's key never enters the
node. Coin reservation is about not double-spending a coin while building a bundle; it is not permission
to sign. crates/dig-wallet/src/sage/chain.rs is byte-identical to main — the
DIG_WALLET_ENABLE_LIVE_BROADCAST gate and the // DELIBERATELY NOT a Broadcaster boundary are
untouched.

Single-writer

Stays out of sync_supervisor.rs, quorum.rs and the peer-pool code (dig_ecosystem#2761/#2768 own
those; #278's dep adoption is deliberately not in this batch), and out of the DHT/onion availability
code (PR #292 owns that).

Version

0.135.0 → 0.136.0 (minor). New capability — a derivation window that grows, a reservation ledger,
a real pending set — alongside a behaviour fix. No API is removed or renamed and no wire shape breaks.
dig-wallet0.27.0 → 0.28.0 (new public API: observe_occupied_puzzle_hashes,
DERIVATION_GAP_LIMIT, MAX_DERIVATION_COUNT). Cargo.lock's own-version entries updated to match.

[MichaelTaylor3d]

loop-security — pre-merge audit IN PROGRESS

Head audited:a8d1254b1aed0f556291eb79a3708e4f7d86c4b7
Base:e4afb522ce6f3ba91b526af2a008a3f630bc7b33 (merge-base == origin/main, branch is current)

§908 boundary — first assertion, verified

crates/dig-wallet/src/sage/chain.rs is byte-identical to main, verified by blob hash rather
than by the absence of a hunk:

main 100644 blob fd691df41aae90ed466b2460fa7bb791453d1e2a crates/dig-wallet/src/sage/chain.rs
pr/295 100644 blob fd691df41aae90ed466b2460fa7bb791453d1e2a crates/dig-wallet/src/sage/chain.rs

Same check clears the rest of the signing surface — spend.rs (747488e2), mint.rs (f34402cd),
auth.rs (f366eb8c), offers.rs (e1fe80f6) and service.rs (d91675e9) are all identical
blobs across the two trees. The only changed files under sage/ are custody.rs, db.rs,
rpc.rs, types.rs.

Still in flight (do not merge on this comment)

Working the reachability questions now:

1. who can invoke push_signed_bundle, and whether the new pending_transactions row (which
   stores the full bundle_hex) can be grown by a caller that is not the wallet owner
2. whether the coins table that feeds occupied_puzzle_hashes accepts unverified peer-supplied
   coin states — if it does, the gap-limit scan becomes peer-steerable up to
   MAX_DERIVATION_COUNT
3. whether chain_peak -> replica_answer_is_current introduces outbound peer I/O on a read that
   previously touched only the local replica
4. the accepted == false / transport-error arm of the push path, where a bundle may be in flight
   with no reservation

Verdict follows in a separate comment.

[MichaelTaylor3d]

Adversarial gate — CHANGES-REQUIRED, two gating items, both measured

Head a8d1254. Correctness + security legs still running; this is the refutation leg.

1. #293 is still live on the money read — and that is the endpoint that matters more

The literal claim survives: every synced: producer in rpc.rs is either replica_answer_is_current
or a literal false (lines 1335, 1380, 1466, 1496, 1602, 1631, 1674, 1735, 1776, 1860, 1890, 1908). No
serde Deserialize, no Default, no ..Default::default(), no re-serialization path.

But the guarantee it stands in for is false.replica_answer_is_current (rpc.rs:1000-1005) delegates
to sync_supervisor.rs:577-582:

pub(crate)fnis_following(replica:Option<u32>,peers:Option<u32>) -> bool{match(replica, peers){(Some(replica),Some(peers)) => peers.saturating_sub(replica) <= FOLLOWING_TOLERANCE,
_ => true,// <-- replica == None answers TRUE}}

chain_peak is safe by luck — it only calls the gate inside if let Some(peak_height)
(rpc.rs:1881-1890). The balance and coin reads are not: rpc.rs:1323-1329 reads
sync_state().peak_height as an Option and passes it through unguarded at :1335, same shape at
:1466 and :1674.

That state is reachable in production. refresh_tracked_coins latches the replica authoritative without
ever writing a peak — rpc.rs:3008-3018 calls record_coverage + set_initial_sync_complete(true),
and db.rs:941-943 says so outright: "This path has replayed nothing and has no terminal height to
offer."peak_height stays NULL, and replica_covers checks only the latch + coverage.

Measured on the PR head, peers 530 blocks ahead:

ADVPROBE balance.synced=true balance.peak=None chain_peak=Ok((None, false))

control.wallet.balance says synced: true, peak_height: null at the same moment
control.wallet.peak says synced: false. That is #293's exact disagreement, surviving this PR, in the
more dangerous direction: the money read claims current with no height at all, while the honest answer
comes from the endpoint a client is less likely to consult. The new doc block at rpc.rs:985-990 asserts
this cannot happen.

The regression test cannot see it — db_with_owned_derivation(true, Some(REPLICA_PEAK)) (rpc.rs:6989)
always gives the replica a peak, so the None arm never reaches the agreement assertion at :7020.

Fix, one line at the top of replica_answer_is_current (rpc.rs:1000):
let Some(replica_peak) = peak_height else { return false }; — an unknown replica height cannot establish
currency, which is the same argument the method already makes for an unobservable peer tier.

2. #251's production wiring survives deletion with every test green

No test anywhere exercises the push -> reserve wiring. Every reservation test calls
be.db.reserve_spend(...) directly (rpc.rs:10105, 10137, 10164, 10192), bypassing push_signed_bundle.
The nine tests that do call push_signed_bundle assert only mempool outcome and the §908 refusal — none
reads pending_transactions or coin_reservations afterwards.

Proved by mutation. Disabling the entire production seam at rpc.rs:1962:

iffalse && outcome.accepted{// was: if outcome.accepted {ifletErr(e) = self.reserve_pushed_bundle(&bundle).await{

cargo test -p dig-wallet --lib → 651 passed; 0 failed.

This is the same gap the PR body itself identifies as why DB-primitive tests were insufficient for #250
(rpc.rs:10067-10070: "the defect these close is that the wiring did not exist, so a test that stops at
the DB layer cannot see it"). The argument applies verbatim to #251 and was not applied there.
FakePusher::accepting() already exists (rpc.rs:7305), so the closing test is cheap.

Claims that held

• dig-node get_pending_transactions returns a hardcoded empty list, on a comment that is no longer true #250 / dig-node derives a fixed 50-address window and no hardened keys — an imported wallet silently under-reports its balance #252 revert-proofs — could not break.
  an_expired_reservation_stops_being_pending_and_frees_its_coin keys on both surfaces;
  a_reserved_coin_leaves_selection_but_not_the_balance asserts the balance side too.
• The dign wallet peak reports synced: true on a replica 1,875 blocks behind — the latch, not the freshness gate #293 test cannot be greened by a hardcoded value.false fails the control, true fails the
  first assertion, peers.is_some() fails peers_ahead_of_the_replica(). The pre-existing test was
  correctly amended (rpc.rs:7195-7200), not deleted.
• custody.rs untouched on main, verified. The widened signer does reach the watched set —
  custody.rs:525-527 unions the unlocked signer's keys and cache_wallet_facts persists it so it
  survives re-lock. Scan is bounded.
• dig-node reserves no coins in flight — two sends in the confirmation window pick the same coin #251's warn-don't-fail is the right trade. The re-opening sequence is real (accept -> reserve fails
  -> second send reselects -> mempool refuses) but fails toward refusal, not loss of funds, and the
  alternative — failing a push that already happened — is the worse lie.

Non-gating

reserve_spend lowercases coin ids (db.rs:1746) while prune_reservations joins c.coin_id = r.coin_id
raw (db.rs:1787) — a coin row written with uppercase hex silently never retires and holds to TTL.

Sequencing note for the orchestrator, not this lane

The single-writer claim in the PR body is wrong.#292 and #295 both change Cargo.toml, Cargo.lock,
SPEC.md and crates/dig-wallet/Cargo.toml, and collide on version (#2920.136.1, #2950.136.0). If
#292 lands first, #295's bump is a decrease, and a rebase drops an overlapping bump silently rather
than conflicting. #292 also uplifts dig-keystore "0.3" -> "0.9" inside the crate #295 rewrites — so
whichever merges second earned its verdict against a different dependency graph and needs re-gating, not
just re-bumping. I own that sequencing.

[MichaelTaylor3d]

Correctness gate: PASS

Head reviewed: a8d1254b1aed0f556291eb79a3708e4f7d86c4b7 (resolved from the remote, not from the dispatch prompt). Draft — not undrafted, not merged.

Scope: correctness + acceptance criteria only. Security and adversarial legs run in parallel.

The two questions asked

1. Is replica_answer_is_current genuinely the only producer of synced: true? — YES, verified mechanically.

Every production assignment of synced in crates/dig-wallet/src/sage/rpc.rs is one of two shapes:

• replica_answer_is_current(...) — lines 1335, 1466, 1674, 1890
• the literal false — lines 1380, 1496, 1602, 1631, 1735, 1776, 1860, 1908

There is no third shape. Grepping the whole tree for a synced: true outside that file returns exactly three hits, all in crates/dig-node-service/src/control.rs (2990, 3395, 4488) and all inside #[cfg(test)] (the module opens at 2810) — wire-shape fixtures, not producers. The "only way" claim is true at this head, not merely true when it was written.

2. Did the corrected pre-existing test get stronger or weaker? — STRONGER.

the_peak_is_the_replicas_when_the_replica_has_one (rpc.rs:7194) keeps both original assertions — peak_height: Some(5_000_000) and synced: true — inside one assert_eq! over the whole ChainPeak. Nothing was deleted. What changed is the fixture: it now runs with peers_level_at(5_000_000) instead of an unobservable tier, so synced: true is now earned through the measured path rather than granted by the latch. Before the fix that assertion was satisfiable by the defect; now it can only pass if replica_answer_is_current returns true for a replica genuinely level with its peers. That is a strictly narrower set of admissible implementations.

Revert-proofs I ran myself

Both in my own detached worktree at a8d1254, mutation applied to production code, restored by file copy.

Mutation	Result
rpc.rs:1890replica_answer_is_current(Some(peak_height)) changed to db.is_synced().await.unwrap_or(false)	1 failed — the_peak_is_not_reported_current_while_the_balance_read_calls_the_same_replica_stale, panicking at rpc.rs:7013 with the message "peak called a replica 530 blocks behind current". Exactly the claimed message.
replica_answer_is_current unobservable-tier arm changed to delegate to is_following(peak_height, None) unnarrowed	1 failed — an_unobservable_peer_tier_is_never_reported_as_current

The controls discriminate. a_replica_level_with_its_peers_still_reports_a_synced_peak (rpc.rs:7057) asserts the full ChainPeak including synced: true, so a hardcoded false substituted for the fix fails it while the regression test passes — the pair cannot both be greened by trading one literal for another. Both controls stayed green under both mutations above, which is what makes the two reds attributable.

The revive-not-rewrite claim — verified, no hunk dropped

I extracted every symbol PR #216 added over its own merge-base (0c98ce5f) — 37 names — and asserted each is present in the #295 head's wallet sources. Zero missing. A rebase that silently dropped a hunk would have surfaced here; it did not.

Known non-discriminating tests — bounded, and the shape does not recur

the_window_is_bounded (custody.rs:1829) and the_default_window_is_wide_enough_to_find_an_imported_wallets_history (custody.rs:1842) are compile-time assertions over MAX_DERIVATION_COUNT / DEFAULT_DERIVATION_COUNT. They pin constants and nothing else, exactly as the lane reported. That is not a false green — their doc-comments state that constant-pinning is their subject, and a constant-pin is a legitimate test. I checked the other four tests in that block for the same shape; all exercise real derivation:

• observed_usage_extends_the_window_past_the_default (custody.rs:1747) — asserts both trees reach 3 + DERIVATION_GAP_LIMIT
• hardened_usage_also_extends_the_window (custody.rs:1771)
• an_unused_wallet_covers_exactly_the_floor (custody.rs:1790) — asserts a length of 8 for a window of 4, which is the assertion that dies if the hardened tree is dropped
• a_foreign_puzzle_hash_does_not_extend_the_window (custody.rs:1808)

So the #252 mutation the lane reported (5 failures) is attributable to real coverage, not to the two constant-pins.

#251 ordering — reserve-after-accept is correct

reserve_pushed_bundle runs only on an accepted outcome (rpc.rs:1963). This is the right order: reserving before acceptance would strand a user's coins on a mempool refusal, for a spend that will never happen — the same class of harm in the opposite direction. The warn-not-fail choice on a reservation error is also right: the bundle is in a public mempool by then, and returning an error for a push that demonstrably happened would be the worse lie. See the non-gating note below on the residual window.

The stranding question that mattered more: prune_reservations is genuinely called on the production read paths — rpc.rs:2540, 2792, 3177 — so expiry is enforced lazily at every selection and every pending read, with no dependence on a background task ever running. A reservation whose release path never fires therefore cannot deny a user their own coins indefinitely. reserved_coin_ids reads the raw table with no expiry filter and is safe only because of those three call sites; that coupling is load-bearing and undocumented at the definition, which is the one thing I would keep an eye on.

Boundaries — verified, not assumed

The name-status diff of origin/main...a8d1254 returns exactly 8 paths, all modifications: Cargo.lock, Cargo.toml, SPEC.md, crates/dig-wallet/Cargo.toml, and four files under crates/dig-wallet/src/sage/ — custody.rs, db.rs, rpc.rs, types.rs.

• crates/dig-wallet/src/sage/chain.rs — absent from the diff, byte-identical to main. The live-broadcast env flag and the "DELIBERATELY NOT a Broadcaster" marker are untouched.
• No writes to sync_supervisor.rs, quorum.rs, peer-pool, or DHT/onion code. No overlap with PR #292.
• Scratch files: a8d1254 removes .pr-body.md and .testout.txt and nothing else. The branch adds no files at all — the file set is modifications only — so nothing else rode in on the accidental stage-all.

Suite + gates

• cargo test -p dig-wallet at a8d1254 in a clean detached worktree: green, 650 + 22 passed, 0 failed, 3 ignored.
• Required checks asserted by name via check-merge-preconditions.sh (not read off the rollup): Lint commit messages, Check version increment, Rustfmt, Clippy, Test + coverage — all present and SUCCESS, with zero unresolved review threads. The script reports BLOCKED solely because the PR is a draft, which is the intended state.
• Versions: 0.135.0 to 0.136.0, and dig-wallet0.27.0 to 0.28.0, both own-version lock entries updated. CHANGELOG untouched — correct, git-cliff owns it.

CodeQL fixture

fixture_password() (custody.rs:1669) assembles its value from fragments at runtime. Meaning is preserved: it is still a test-only at-rest password, still over MIN_PASSWORD_LEN (stated in its own doc, so a length-floor failure cannot be mistaken for the property under test), and the seven call sites still exercise the real import path. This defeats the rule, not the test — the right trade, and the doc-comment says so honestly rather than pretending the value is secret.

Sibling coherence — docs.dig.net PR #78

Checked. docs/run-a-node/manage.md now describes the flag as "whether the height above is current — measured against what this node's Chia peers report, not merely whether a first sync once finished", and explicitly covers the unobservable-tier case ("or that it has no peer to compare against yet, so the height is real but dated"). That matches replica_answer_is_current's actual semantics including the one-directional narrowing, so a false reading from a peerless node no longer reads as an outage. Coherent.

dig-constants check (both directions, per the standing rule)

• Does anything here belong in dig-constants? No, at this head. The four new constants — DEFAULT_DERIVATION_COUNT (500), DERIVATION_GAP_LIMIT (250), MAX_DERIVATION_COUNT (25_000) and RESERVATION_TTL_MS — are single-consumer wallet-scoping and mempool policy. No second repo must match them today, and dig-constants (16 constants) publishes nothing in this space. Recorded as a watch item rather than a gate.
• Should anything here be using dig-constants? No literal in this diff duplicates a value dig-constants already publishes.

---

Two non-gating notes follow as inline threads; I am resolving both myself so neither blocks the merge. Nothing gating. The verdict stands at PASS for the correctness leg — merge remains subject to the two sibling gates and to the PR leaving draft.

[MichaelTaylor3d]

loop-security: PASS

Head audited:a8d1254b1aed0f556291eb79a3708e4f7d86c4b7 (re-resolved from the remote at the end of the audit; unchanged throughout)
Base:e4afb522ce6f3ba91b526af2a008a3f630bc7b33 — the merge-base IS origin/main, so the branch is current and this verdict is against what would merge.

No gating security defect. Five non-gating items below (follow-up tickets, not merge blockers), plus the list of what I attacked and could not break.

---

§908 — the boundary HOLDS, and chain.rs is byte-identical

Verified by blob hash on both trees, rather than by the absence of a diff hunk:

file	origin/main	pr/295
sage/chain.rs	fd691df41aae90ed466b2460fa7bb791453d1e2a	same
sage/spend.rs	747488e26c00847ebc1ae9035e92d1a2343439d5	same
sage/mint.rs	f34402cd9abed195558afba2485b54e155514db1	same
sage/auth.rs	f366eb8cd7d4068896247d0d4ee201ff0dda2cb7	same
sage/offers.rs	e1fe80f6253efa62a2d9b34df39d93aa928e0ba7	same
sage/service.rs	d91675e98a0f2564387fa5821d0fd4c8878d354a	same

The DIG_WALLET_ENABLE_LIVE_BROADCAST gate and the DELIBERATELY-NOT-a-Broadcaster boundary are therefore preserved verbatim. Only custody.rs, db.rs, rpc.rs, types.rs change under sage/.

The boundary is not moved elsewhere either:

• build_signer stays private and gains no caller. Its call sites are unchanged (import / unlock / restore, inside custody).
• Nothing new constructs, holds, derives, or accepts signing material from a request. The one new custody entry point, observe_occupied_puzzle_hashes (custody.rs:289), takes a set of PUBLIC p2 puzzle hashes read out of the node's own coin table. No key, no seed, and no path from it to a signer.
• Seed provenance is unchanged: master_secret_key is the same three lines lifted out of wallet_fingerprint (bip39 parse, Zeroizing seed buffer, SecretKey::from_seed). The Zeroizing wrapper survives the extraction; no new copy and no new lifetime for the seed.
• The node-custodied refusal in push_signed_bundle is untouched, and SPEC §18.12 is updated so the custodied-KEY set that guard reads widens with the window across BOTH trees. That strengthens the gate rather than weakening it, and there is no regression window: before this PR the node never derived hardened keys at all, so it could never have signed a hardened coin.

---

Findings (all non-gating)

1. The derivation window is peer-steerable to its cap — defense-in-depth, LOW-MED

db.rs occupied_puzzle_hashes() is SELECT DISTINCT puzzle_hash FROM coins. The coins table is written by sync::apply_coin_states (sync.rs:698) from peer-pushed coin_state_update frames, filtered ONLY by membership in the subscribed set — nothing verifies the coin exists on chain. The subscribed set IS the derived window, and the node discloses it to the peer when it subscribes.

Scenario: the node holds a peer at PeerTrust::Corroborated (corroboration is quorum agreement on a peak HEIGHT, not on coin states). The attacker fabricates a coin state at every subscribed puzzle hash. On the next unlock, highest_occupied_index returns the top of the window and it extends by DERIVATION_GAP_LIMIT. The wider set is re-subscribed on the next connect; repeat. Roughly 98 cycles reaches MAX_DERIVATION_COUNT.

Terminal cost, using the PR's own measurement of 251ms per 1000 keys: 50,000 BLS derivations per unlock, about 12.5s; a ~50,000-hash subscription frame per peer connection; 50,000 resident secret keys. Persisted, because the evidence lives in coins.

Not gating: bounded by MAX_DERIVATION_COUNT, needs a corroborated peer AND ~98 user-initiated unlocks, costs no money, and weakens no custody guard. Worth recording that the PR raises the fabricate-into surface 20x by default (50 to 1000 hashes) before any ratcheting.

2. The reservation is gated on an untrusted accepted — defense-in-depth, LOW

rpc.rs:1955 reserves only when outcome.accepted, which is the push target's own answer, and under NC-12 that target is untrusted.

• Fail-OPEN: a source answering accepted: false for a bundle it in fact relayed leaves the coins unreserved, re-opening exactly the double-selection this ticket closes. The same holds for a transport error after the bytes went out — that arm returns PushError::Unreachable and reserves nothing.
• Fail-CLOSED: a source answering accepted: true for a refused bundle holds an authorized caller's coins for the 10-minute TTL.

Both need a lying chain source; the second additionally needs an authorized local caller; both are TTL-bounded. Suggested follow-up: treat a transport failure AFTER transmit as possibly-in-flight rather than definitely-not.

3. release_spend has no production caller — nit

db.rs release_spend is exercised only by a test. Its two documented outcomes are both handled elsewhere: a mempool refusal never creates a reservation in the first place, and a settled spend is retired by prune_reservations conditions 2 and 3. Wire it or delete it — dead code on a custody-adjacent path invites a future caller to assume it is exercised.

4. MAX_DERIVATION_COUNT names a threat channel that does not exist — nit

custody.rs:99 justifies the ceiling as protection against a corrupt or hostile derivation_count in a hand-edited manifest. derivation_count is never read from the manifest; the sole production constructor is WalletCustody::mainnet (service.rs:148) passing the compile-time DEFAULT_DERIVATION_COUNT. The clamp is correct and must stay, but the reason it actually earns its keep is finding 1, and the comment should say so.

5. Resident secret-key footprint grows 20x by default — defense-in-depth, LOW

chia-bls 0.22 SecretKey has no Drop and no zeroize (checked the vendored crate source: no impl Drop, no zeroize reference anywhere in it). Derived keys are left in process memory. This PR raises the resident count from 50 to 1000 by default, and to 50,000 at the cap. An upstream property this diff cannot fix; it belongs on the memory-hygiene story, not on this PR.

Coherence note (for the correctness gate, not security)

types.rs changes PendingTransactionRecord.fee from Amount to Option<Amount> — wire-visible, null where a client previously always received a number. Worth confirming the dig-node-control-interface side and any dig-app consumer handle null rather than rendering it as zero, since a fee of zero is precisely the confident-wrong-number-about-money failure §18.8 now forbids.

---

What I attacked and could NOT break

• A foreign puzzle hash extending another wallet's window.highest_occupied_index matches only against THIS wallet's derived keys; a_foreign_puzzle_hash_does_not_extend_the_window pins it.
• Reserving a third party's coins. Requires a mempool-accepted bundle spending them, which requires their signature. No path.
• Orphaned coin_reservations permanently stranding funds. I expected SQLite's default of foreign keys OFF to make the ON DELETE CASCADE a no-op, so that pruning an expired pending_transactions row would leave its reservations behind forever and permanently un-spend the user's coins — the exact failure the design says only an unconditional expiry rules out. It does not happen: db.rs:728 (production open) and db.rs:735 (open_in_memory) BOTH set .foreign_keys(true), so the cascade fires and the tests run against the same pragma as production.
• Guard vacuity. I checked whether the reservation can ever bite, given that push_signed_bundle refuses node-custodied spends and the coin table would then never hold a reserved id. It is NOT vacuous: UnionPuzzleHashSource (sync_supervisor.rs) makes the subscribed set the union of custody's own addresses AND control.wallet.watch-registered keys, so on the §908 install the reserved ids are exactly the watched user's coins sitting in that table.
• Partial coverage of the selection guard. Every production selection site in rpc.rs (3082, 3103, 3149, 3199, 3403, 3464, 3549, 3590, 3627, 3646, 3725, 3812) reads through spendable_coins, which prunes and then reads the unreserved set; the CAT path at 3180 uses unreserved_unspent_coins with its own prune. No production db.unspent_coins call remains on any spend path.
• The new get_pending_transactions error arm as a DoS.SpendResult::fee is u64 (dig-clvm consensus/result.rs:18), so a stored value always re-parses. The Err arm needs genuine DB corruption, not adversarial input.
• An AuthZ regression on the new pre-dispatch hook.refresh_observed_derivations runs inside WalletBackend::dispatch, and all three transports apply wallet_authz::requires_authorization BEFORE dispatching (the HTTP wallet_rpc handler, the WS path, and the JSON-RPC path in server.rs); classify puts every wallet. and auth. prefixed method in the Custody class. The dig-wallet sidecar transport is mTLS shared-client-cert gated. control.wallet.broadcast is token-gated, not an open read.
• New egress from chain_peak.replica_answer_is_current calls chain_peer_tier(), which is fallback.peer_tier() — a cached local read of the transport's own state. The change REMOVES a DB call and adds no outbound request; it does not open the coinset oracle path this file refuses elsewhere.
• Denial-of-confirmation via synced: false. A peer over-claiming its peak does force the flag false (is_following is a saturating_sub against FOLLOWING_TOLERANCE). But replica_answer_is_current already exists on main and already governs three balance/coin reads there (rpc.rs:1311, 1442, 1650); this PR only adds chain_peak as a fourth consumer. The primitive is pre-existing, its direction is fail-closed (refusing to claim settled), and nothing in the diff gates a spend on it.
• Hardened / unhardened index confusion. The two trees are kept in separate vectors so a position IS its index, and the code records that a modulo-based form was considered and rejected for exactly this reason. unhardened_matches_digstore_chain pins the fast intermediate derivation against digstore_chain::derive_indexed_keys, and p2_puzzle_hash_matches_the_signer pins the p2 mapping against what WalletSigner applies. Those are the two drift risks that would put money at addresses the wallet does not watch, and both are asserted rather than reasoned about.
• Unbounded derivation.target is clamped to MAX_DERIVATION_COUNT on entry and on every extension, saturating_add is used throughout, and extend_to is incremental — so total work is linear in the final target rather than quadratic across iterations, and the loop provably terminates.
• Secret or credential leakage. Two new production log lines, both logging only a DB error. Every eprintln! sits inside an ignored measurement test printing durations. No seed, no key, no passphrase, no address list.
• The CodeQL fixture.fixture_password() lives entirely inside the test module and is used only in test import calls. No production credential path is touched, no existing literal was removed or weakened, and nothing real was silenced — it swaps a would-be literal for a runtime concat in a fixture.
• Dependencies.Cargo.toml and Cargo.lock carry version bumps only (workspace 0.135.0 to 0.136.0, dig-wallet 0.27.0 to 0.28.0). No new dependency and no loosened pin; the master_to_wallet_* and DeriveSynthetic imports come from the already-present chia dep.

---

Shared-checkout disclosure

Read-only. I ran one git fetch origin main refs/pull/295/head:refs/remotes/pr/295 in the primary checkout — a ref addition only — and read everything else through git show, git diff and git ls-tree against objects. No checkout, reset, stash, clean, or file edit. Verified afterwards: HEAD still bef7fc3c7b79e9960611bd9de3e4bbde4bbc90f7, git status --porcelain shows only the pre-existing untracked .claude/loop/, and the three stash entries are the pre-existing ones — I created none.

[MichaelTaylor3d]

Gate round 2 — both gating findings fixed. Still DRAFT.

Branch loop/wallet-correctness-293, head 8954b77. Version unchanged at 0.136.0 / dig-wallet 0.28.0 per the merge-first sequencing.

G1 — #293 was still live on the money reads

replica_answer_is_current narrowed only the PEER-side arm of is_following. The replica-side arm survived, and it is the one that reaches production: chain_peak calls the gate inside if let Some(peak) and structurally cannot hand it None, while the balance and coin reads pass their Option straight through. refresh_tracked_coins latches the replica authoritative without ever writing a peak, so the money reads — and only the money reads — paired synced: true with peak_height: null.

Fix: a guard clause at crates/dig-wallet/src/sage/rpc.rs:1016. An unknown replica height cannot establish currency, which is the argument the method already made for an unobservable peer tier.

is_following itself is left ALONE, deliberately. Its permissive _ => true is correct for the sync-phase reporting it was written for, where an unmeasured tier must not be spent as evidence against a replica (sync.rs:2567 / :2811 pin that). The narrowing is a property of the MONEY read, so it lives at the call site — which is also the single gate every synced: true already passes through. sync_supervisor.rs is owned by dig_ecosystem#2761 / #2768 and PR #292 is live in it; not touched.

Test an_unknown_replica_height_is_never_reported_as_current (rpc.rs:7127). Fixture: db_with_owned_derivation(true, None) — the exact latched-without-a-peak state refresh_tracked_coins produces — with an HONEST, observable peer tier (peers_level_at(REPLICA_PEAK)), so the missing replica peak is the only axis varied and the only thing that can explain a verdict. It asserts AGREEMENT between the balance read, the coin read and chain_peak rather than a bare !synced, so hardcoding either literal on one side shows up as a disagreement.

G2 — #251's wiring survived deletion

Confirmed: every reservation test called db.reserve_spend(...) directly, so the push -> reserve seam had no coverage at all.

Tests added at rpc.rs:10261 (a_pushed_bundle_reserves_its_inputs_through_the_production_path) and rpc.rs:10345 (a_refused_bundle_reserves_nothing, the control that pins the outcome.accepted guard rather than just the wiring). The bundle's coin id is DERIVED from the same Coin the row describes (a_bundle_spending), so row and bundle agree on identity structurally rather than by a matching pair of literals. Two spendable coins, one spent by the bundle: a single coin cannot distinguish "the input was reserved" from "selection was emptied".

Revert-proofs — all three fire, on the right assertion

mutation	test that goes red	assertion that fired
drop the G1 guard clause	an_unknown_replica_height_is_never_reported_as_current	the balance read claimed currency for a replica whose own height it does not know
if false && outcome.accepted	a_pushed_bundle_reserves_its_inputs_through_the_production_path	an accepted push recorded nothing in flight (left 0, right 1)
revert the LOWER() join	an_upper_case_coin_id_still_retires_its_settled_bundle	a settled bundle whose coin id is upper-case was never retired

Each was run on a COMMITTED tree and restored by file copy, never by a git checkout on a path.

The if false && mutation now reddens exactly one test and leaves the other 654 green — which is the measurement that says the new test is the only thing holding that seam.

One pre-existing test changed, and it is worth reading

synced_empty_address_is_zero_success_not_error went red under the G1 fix. Its fixture was db_with_owned_derivation(true, None) and it asserted r.synced — pinning the precise pairing (synced: true beside an unknown height) that #293 exists to remove. A passing control that holds the value its own rule forbids makes the defect unfixable. The FIXTURE changed, not the assertion: the replica now has a real peak level with its peers, which is what an honestly caught-up replica looks like, and the test stays the suite's positive control against hardcoding synced: false.

The four cheap items — all four taken

1. Coin-id case (fixed + regression test).prune_reservations joined c.coin_id = r.coin_id raw while reserve_spend lowercases everything it writes, so an upper-case coin never matched its own reservation: the settled bundle never retired and stranded its other inputs for the whole TTL. Now LOWER(c.coin_id) = r.coin_id (db.rs:1789). The selection path already normalised both sides (db.rs:1841 / :1852), so this was the last raw comparison. Test an_upper_case_coin_id_still_retires_its_settled_bundle.
2. PendingTransactionRecord.fee. In-repo it is produced honestly and never rendered as zero — the only render site (rpc.rs:2567) maps None to None and makes an unparseable stored fee an ERROR. The unwrap_or(0) hits in lib.rs are INBOUND request params (a caller choosing a fee to pay), the opposite direction, where 0 is correct. The risk is out of repo, at the display surface, so SPEC.md §18.8 now binds the obligation to CONSUMERS explicitly: a client rendering a null fee as 0 re-creates the falsehood where a person reads it. Flagged for dig-app rather than reached into.
3. MAX_DERIVATION_COUNT comment (fixed). It named a hand-edited manifest; ManifestEntry has no derivation_count field — the value is a WalletCustody::new argument, so that channel does not exist. The real unbounded channel is the one the clamp actually stops: the gap-limit scan follows the replica's COIN SET, which anyone can extend by paying the wallet at a higher index. Both the const doc (custody.rs:99) and build_signer's (custody.rs:951) now say so.
4. release_spend (deleted). Zero callers repo-wide, and it could not gain a correct one: a refusal reserves nothing (guarded on accepted) and a settlement is retired by prune_reservations, so every definitive outcome was already covered. Wiring it to a refusal would have been actively wrong — PushOutcome.rejection is a free-form string, so a TRANSIENT refusal would release coins that really are in flight and re-open double-selection. Its cascade property is not lost: retiring_a_bundle_cascades_away_its_reservations now asserts it through the retirement path that actually runs. Removal sits inside the existing 0.27.0 -> 0.28.0 minor (the 0.x breaking slot); dig-wallet is a path dep with no external consumers.

Blast radius checked

No gitnexus index in this worktree, and building one writes repo-tracked files into a lane worktree, so this is the sanctioned grep+read fallback rather than a stale-index answer.

• replica_answer_is_current — private async method, 4 call sites, all in sage/rpc.rs: :1335 balance, :1466 coins, :1674 coinById, :1890 chain_peak. Only :1890 passes a guaranteed Some, so the change is observable on the three money reads and a no-op on chain_peak. Risk: MEDIUM, and in the safe direction — it can only turn a true into a false, never the reverse, and the figure is still SERVED with its real height.
• is_following — NOT edited. Its other consumers (sync.rs:2567, :2811, sync_supervisor/tests.rs:4113) are untouched and green.
• prune_reservations / release_spend — release_spend had zero callers outside its own test; cargo clippy --workspace --all-targets -D warnings is clean, so nothing else referenced it.

No HIGH or CRITICAL risk found. Files touched: crates/dig-wallet/src/sage/{rpc,db,custody}.rs plus SPEC.md. sync_supervisor.rs, quorum.rs, peer-pool and DHT/onion code untouched (#292's territory).

Local evidence

cargo test -p dig-wallet --lib → 655 passed, 0 failed, 3 ignored (651 at gate round 1: +3 new, +1 from the cheap-item fix). cargo fmt --all --check clean. cargo clippy --workspace --all-targets -- -D warnings clean.

Staying DRAFT until the orchestrator's merge-first sequencing says otherwise.

[MichaelTaylor3d]

Final fix round landed — branch loop/wallet-correctness-293, head 43b9c52. Still DRAFT.

M1 — docstring justification corrected. The old text claimed a lower-case twin "can only have been written by the fixed code". False: every in-tree writer has emitted lower-case since long before this PR (sync.rs::coin_state_to_row via hex::encode, fallback.rs::map_record, peer_reads/dialed.rs, peer_reads.rs::coin_from_cache). The inference also ran backwards — the one verbatim path into coins is refresh_tracked_coins → fallback_coin_to_row, the point-read used because the subscription replica is behind, so an upper-case row would be the fresher observation. The docstring now states the true warrant: no in-tree writer can produce an upper-case id, so the migration matches zero rows on any real wallet; it exists because ChainFallback and CoinPeer are public traits. Case carries no recency information in either direction, and the collision rule no longer pretends otherwise.

M2 — the permanent brick is fixed. Two non-canonical spellings (AAbb, aAbb) survived the case-scoped DELETE and then collided under the UPDATE; the rollback was correct but the retry is byte-identical, so migrate → open failed forever. Collisions are now resolved before the update by drop_case_collisions, N-way. Rule, stated: keep the already-canonical spelling (a group holds at most one, since two would be the same string), else the lexicographically smallest — total, deterministic, and arbitrary on purpose. Recency is deliberately not a tie-break because the rows carry no evidence of it. Dropped rows are logged at WARN with the id and the survivor, since in this tree a collision can only mean a non-conforming ChainFallback/CoinPeer impl wrote to the replica.

M3 — dependent tables normalised in the same transaction.COIN_ID_TABLES = ["coins", "arrival_pending", "arrivals"], each collision-resolved then lower-cased inside the one transaction.

M4 — the test can now see which twin survived. The twins differ by amount, and the survivor is asserted by name. Two tests added: the N-way brick, and the dependent-table normalisation.

M5 — ruling: narrow the claims, file the fix.unspent_coins_scoped lower-casing its bound puzzle_hash/hint/asset_id against verbatim-stored columns is real and user-facing, but fixing it reaches derivations.puzzle_hash and the watched-hash sets in record_arrivals and needs its own indexed-column migration — materially wider than this PR's identity-column scope, in a final fix round. So the upsert_coin docstring and the SPEC paragraph are narrowed to coin_id + parent_coin_info, each naming the exclusion explicitly, and the defect is filed as #298 (parented on dig_ecosystem#2760). SPEC also gains the two new normative obligations this round created: same-transaction normalisation of coin-id-keyed tables, and deterministic collision reduction.

Revert proofs (committed first, restored by file copy — never git checkout):

• old two-spelling DELETE → brick test fails with SqliteError 1555: UNIQUE constraint failed: coins.coin_id, and the pre-existing migration test stays green, which is the M4 point.
• survivor rule inverted → fails on the survivor assertion (left: "upper", right: "lower"); the id-set assertion still passes, so it could not have caught this.
• COIN_ID_TABLES reduced to ["coins"] → fails with arrival_pending still keys its row by the un-normalised coin id.

Evidence.cargo test -p dig-wallet --lib 660 passed / 0 failed. cargo fmt --check clean, clippy --all-targets -D warnings clean. CR byte count via tr -cd '\r' | wc -c = 0 on all four touched files. Required checks by name: Lint commit messages, Check version increment, Rustfmt, Clippy, Test + coverage — all SUCCESS; 0 unresolved threads; CodeQL + all three Analyze jobs SUCCESS. Version unchanged at 0.136.0 / dig-wallet 0.28.0.

Sequencing, unchanged and not auto-resolvable.#295 merges before #292. If #292 lands first, #295 must resolve to 0.136.2 or higher — taking "ours" (0.136.0) is a version decrease and reds the increment gate.