fix(wallet): adaptive+hardened derivation, real pending transactions, in-flight coin reservation by MichaelTaylor3d · Pull Request #216 · DIG-Network/dig-node

MichaelTaylor3d · 2026-08-12T14:41:11Z

DRAFT — DO NOT MERGE. Gate round not started.

Epic dig_ecosystem#2760 wallet-correctness batch. One branch, one PR, three tickets:

dig_ecosystem#2762 — fixed 50-address unhardened-only derivation window; an imported wallet silently under-reports its balance.
dig_ecosystem#2764 — get_pending_transactions returns a hardcoded empty list on a comment that is no longer true.
dig_ecosystem#2763 — no in-flight coin reservation; two sends inside the confirmation window pick the same coin.

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 make the node sign. The DIG_WALLET_ENABLE_LIVE_BROADCAST gate and the // DELIBERATELY NOT a Broadcaster boundary in chain.rs are preserved unchanged.

Single-writer

Stays out of sync_supervisor.rs, quorum.rs, and the peer-pool code (PR #213 / dig_ecosystem#2761/#2768 own those).

Status: implementation in progress. Version bump + Closes keywords added before the gate round.

Salvage anchor so the lane's branch exists on remote before implementation starts. Removed before merge. Co-Authored-By: Claude <noreply@anthropic.com>

…rified) Captured from a stalled lane's worktree so 909 lines of work survive the session that produced them. UNVERIFIED: not compiled, not tested, not linted since the last edit -- verify before trusting any of it. Adds an adaptive derivation window over both the unhardened and hardened trees (DEFAULT_DERIVATION_COUNT 500, DERIVATION_GAP_LIMIT 250, MAX_DERIVATION_COUNT 25_000) plus observe_occupied_puzzle_hashes so observed on-chain usage extends the window past the default, with a bounded ceiling. Fourteen tests accompany it, including a foreign-puzzle-hash control and a digstore-chain conformance check. Open question the lane was mid-measurement on when it stopped: the default window costs ~1.8s per unlock, which it judged too slow for a per-transaction unlock. measure_derivation_breakdown and measure_default_window_cost exist to locate that cost; the shape of the fix is not decided. Refs #2762 Co-Authored-By: Claude <noreply@anthropic.com>

+    #[test]
+    fn the_signer_covers_the_hardened_tree() {
+        let (c, dir) = custody_with_window(4);
+        c.import(ABANDON, "passphrase", None).unwrap();


+    #[test]
+    fn change_still_goes_to_unhardened_index_zero() {
+        let (c, dir) = custody_with_window(4);
+        c.import(ABANDON, "passphrase", None).unwrap();


+        let (c, dir) = custody_with_window(4);
+        // A coin at index 3 — the last covered index, so the gap is entirely unwatched.
+        c.observe_occupied_puzzle_hashes([unhardened_p2(ABANDON, 3)].into_iter().collect());
+        c.import(ABANDON, "passphrase", None).unwrap();


+    fn hardened_usage_also_extends_the_window() {
+        let (c, dir) = custody_with_window(4);
+        c.observe_occupied_puzzle_hashes([hardened_p2(ABANDON, 3)].into_iter().collect());
+        c.import(ABANDON, "passphrase", None).unwrap();


+    #[test]
+    fn an_unused_wallet_covers_exactly_the_floor() {
+        let (c, dir) = custody_with_window(4);
+        c.import(ABANDON, "passphrase", None).unwrap();


+                .into_iter()
+                .collect(),
+        );
+        c.import(ABANDON, "passphrase", None).unwrap();


Cosmetic only; no behaviour change. Verifies the salvaged dd8a37e against fmt + clippy, both of which were red on that commit. Co-Authored-By: Claude <noreply@anthropic.com>

+    fn measure_default_window_cost() {
+        let (c, dir) = custody_with_window(DEFAULT_DERIVATION_COUNT);
+        let t = std::time::Instant::now();
+        c.import(ABANDON, "passphrase", None).unwrap();


…pending set Makes the salvaged dd8a37e primitives actually reachable from production. - #2762: `observe_occupied_puzzle_hashes` had ZERO production callers, so the gap-limit scan was inert and the window could never grow. Feed it from the coin table before every key-loading method. - #2764: `get_pending_transactions` returns the real in-flight set instead of a hardcoded empty list, and errors rather than claiming empty on a DB failure. - #2763: spend-input selection reads the UNRESERVED unspent set, and an accepted push reserves its inputs for RESERVATION_TTL_MS. Fee is Option all the way to the wire: the node relays bundles it did not build, and must not answer a question about someone else's money with a confident zero. Not yet compiled — pushed as a checkpoint before the build. Co-Authored-By: Claude <noreply@anthropic.com>

The salvaged tests stopped at the db primitives and passed while nothing called them. These drive spendable_coins and get_pending_transactions, so they fail if the wiring is absent rather than only if the SQL is wrong. Co-Authored-By: Claude <noreply@anthropic.com>

Co-Authored-By: Claude <noreply@anthropic.com>

SPEC said get_pending_transactions is empty and described the custodied set as one 0..derivation_count range. Both are now false. Adds 18.7a (both trees, floor + gap-limit extension, signer and watched set widen together) and 18.9a (reserve on accepted push, selection-only, always expires). Co-Authored-By: Claude <noreply@anthropic.com>

MichaelTaylor3d · 2026-08-13T22:15:20Z

Sequenced behind dig-node PR #222 (#2868 — a unanimous quorum is discarded when the writer disagrees).

Both PRs edit the same money-bearing files and neither has passed a gate. #222 goes first because it is small, surgical, and it is the reason the replica is empty at all — measured on the installed 0.117.0 service: initial_sync_complete=0 and the coins table has 0 rows, so no coin state has ever been applied and the wallet shows nothing from the replica.

This PR is stale (no live lane) and will need a rebase onto #222 plus a fresh gate round. The work itself is not in question — just the ordering.

Overlap: crates/dig-wallet/src/sage/rpc.rs. #222 also adds a small honesty fix there for #2869 — synced is currently hardcoded true in the Source::Db arm, so a behind replica reports a stale figure as current. That lane has been told to align to this PR's shape rather than invent a second one.

MichaelTaylor3d · 2026-08-22T02:26:37Z

Superseded by #295, which REVIVES this branch's work onto current main rather than rewriting it.

This branch sat 56 commits behind, but the squash-merge produced exactly one conflict hunk per file — both purely additive test blocks at the end of the test modules — and compiled clean on the first try. custody.rs (542 of the 1,395 lines) is untouched on main, so the derivation work applied verbatim. All of it is in #295, plus dig-node#293.

Also worth recording, because it silently cost the close: this PR's body says Closes dig_ecosystem#2762 / #2763 / #2764. Those three were TRANSFERRED into this repo and are now #252 / #251 / #250. A submodule PR's Closes keyword resolves against its OWN repo, so those keywords close nothing — and a bare #2762 here points at a dig-node number. #295 uses the dig-node numbers.

Safe to close once #295 merges.

MichaelTaylor3d · 2026-08-22T02:30:21Z

Superseded by #295, which revived this branch rather than rewriting it.

Measured before deciding: this branch sat 56 commits behind a main that had grown +12,634 lines across the wallet crate — the shape of a rotted branch. But the squash produced exactly one conflict hunk per file (db.rs, rpc.rs), and both were purely additive test blocks at the end of the test module — main's #283 offer-rekey tests landing beside this branch's reservation tests. The only real repair was the closing brace the two sides shared, and the result compiled clean on the first try, so every production wiring point survived. custody.rs — 542 of the 1,395 lines — is untouched on main and applied verbatim.

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

The trap this PR was carrying

Its body said Closes dig_ecosystem#2762 / #2763 / #2764. Those three were transferred into this repo and are now dig-node #252 / #251 / #250. A submodule PR's Closes resolves against its OWN repo — so a bare #2762 here would have resolved to a different dig-node issue, and the three real tickets would have stayed open after merge. Fixed on #295, which closes #293 / #252 / #251 / #250 by their correct numbers.

Recording it because the failure is silent in both directions: nothing warns, and the tickets simply never close.

…reservation, real pending set (#295) * feat(wallet): revive the wallet-correctness batch onto main Salvages PR #216 (loop/2762-wallet-correctness) onto current main: adaptive hardened+unhardened derivation window, in-flight coin reservation, and a real pending-transaction set. Both rebase conflicts were purely additive test blocks at the end of the db.rs and rpc.rs test modules; both sides kept. Co-Authored-By: Claude <noreply@anthropic.com> * fix(wallet): measure the peak endpoint's synced flag instead of trusting the latch `chain_peak` computed `synced` from `db.is_synced()` — the `initial_sync_complete` latch — and returned on the replica-served path without ever consulting `replica_answer_is_current`. Measured on a running node: `sync-status` reported `syncing` while `peak` reported `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. Every read that can report `synced: true` now derives it from the one measured predicate; every other site writes the literal `false` on a fallback-tier answer. The agreement between `peak` and `syncStatus` is therefore structural rather than asserted, and the doc comment that already claimed it now states its basis. Co-Authored-By: Claude <noreply@anthropic.com> * chore(release): v0.136.0 Bumps the workspace artifact version and dig-wallet to 0.28.0 for the wallet-correctness batch, and specifies the freshness contract of `control.wallet.peak` in SPEC.md. Clears the clippy 1.98 findings the salvaged code carried (const-block assertions, `slice::from_ref`). Co-Authored-By: Claude <noreply@anthropic.com> * test(wallet): assemble the derivation fixtures' password instead of writing it literally CodeQL's hard-coded-cryptographic-value rule reads a string literal flowing into a password parameter as a credential and cannot tell a fixture from a real one, so seven copies raised seven findings that each needed a manual dismissal. Building the value from fragments keeps the fixtures as readable and leaves the rule free to mean something the next time it fires. Co-Authored-By: Claude <noreply@anthropic.com> * chore: drop two scratch files a lane staged by accident `.pr-body.md` and `.testout.txt` are working files of this lane and were swept in by a `git add -A`. Neither belongs in the repo. Co-Authored-By: Claude <noreply@anthropic.com> * fix(wallet): refuse to call a replica current when its own height is unknown `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 so 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` (dig-node#293). Also covers the `push -> reserve` seam (#251) through the production reads. Every existing reservation test called `db.reserve_spend` directly, so mutating the seam to `if false && outcome.accepted` left the suite green. `is_following` itself is left alone: its permissive arms are correct for the sync-phase reporting it was written for, and the narrowing is a property of the money read. Co-Authored-By: Claude <noreply@anthropic.com> * fix(wallet): retire a settled bundle whose coin id is upper-case `reserve_spend` normalises every coin id it writes; `coins` stores whatever hex the chain source handed over. `prune_reservations` joined the two RAW, so an upper-case coin never matched its own reservation: the settled bundle stayed pending and held its other inputs out of selection for the whole TTL. The selection path already normalised both sides, so this was the last raw comparison. Also removes `release_spend`, which had no production caller and could not gain a correct one — a refusal reserves nothing and a settlement is retired by `prune_reservations`, so every definitive outcome was already covered. Its cascade property is now asserted through the retirement path that actually runs. Also corrects `MAX_DERIVATION_COUNT`'s rationale: it named a hand-edited manifest, but the manifest carries no `derivation_count` (it is a `WalletCustody::new` argument). The real unbounded channel is the attacker-extensible coin set the gap-limit scan follows. Co-Authored-By: Claude <noreply@anthropic.com> * style(wallet): bind the refusing pusher before the backend rustfmt indents a builder chain that wraps a multi-line struct literal into a shape that reads worse than the value it builds. Naming the pusher first says what it is and leaves the chain one line. Co-Authored-By: Claude <noreply@anthropic.com> * style(wallet): restore LF line endings on four rewritten files Two commits on this branch rewrote SPEC.md, rpc.rs, db.rs and custody.rs whole-file through a Windows text-mode writer, which translated every LF to CRLF. The content was unchanged, but every line read as modified: the PR reported 23,329 additions / 21,565 deletions for a 1,823 / 59 delta. The cost was not cosmetic. CodeQL attributed 36 pre-existing test fixtures in custody.rs as newly introduced ("code changes were too large") and turned the GHAS gate red, and a shared SPEC.md would have conflicted wholesale with a concurrent PR whose hunks do not overlap these at all. Restored to LF byte-for-byte; `git diff --ignore-cr-at-eol` against the prior tree is empty. Deliberately no .gitattributes: a repo-wide text=auto would renormalise every file in the repository, a far larger change than the one being undone. Co-Authored-By: Claude <noreply@anthropic.com> * fix(wallet): store coin hex lower-case at the writer, not at one reader The `LOWER()` added to `prune_reservations` repaired one raw coin-id comparison and claimed to be the only one. Two others survived, and they fail in different directions: - `are_coins_spendable` binds caller-supplied ids on the Sage-parity `get_are_coins_spendable` endpoint. An upper-case id returned `false` for a genuinely spendable coin — a refusal rather than a loss, but a wrong answer about money to a parity consumer. - `record_arrivals` answers `parent_is_ours` with a raw comparison of the child's `parent_coin_info` against `coins.coin_id`. On a case mismatch the wallet's own change coin read as not-ours and was announced to the user as an incoming payment — the same money-display lie as #293. Fixed at the WRITER instead. `upsert_coin`/`upsert_coins` normalise `coin_id` and `parent_coin_info`, so all three sites are covered at once and a fourth reader added later inherits the guarantee. The read layer was already written as though this held — `reserved_coin_ids` lower-cases what it reads back, `unspent_coins_scoped` lower-cases the puzzle hash it binds — so this makes an existing assumption true rather than adding one. The `LOWER()` in `prune_reservations` is REMOVED, not kept as belt-and- braces: `coins.coin_id` is a PRIMARY KEY, SQLite cannot use an index through a function call, and the wrapper turned each retirement into a full scan of `coins` per reservation row. With both sides normalised by their writers the raw comparison is correct, so retaining it would buy only the scan. Existing rows are repaired by ladder step 2, in one transaction. A coin present under both cases is a PRIMARY KEY collision that would abort the step, so the upper-case row is dropped first: a lower-case twin can only have been written by the fixed code, making it the fresher observation. Three tests, each varying only the case the chain source handed over and each with a control a match-everything implementation would fail. All three fail on the code before this commit; the arrival one fails by announcing 2 arrivals where 1 is correct. Also corrects a false claim in `replica_answer_is_current`'s doc, which said it was "the single gate every read that can produce `synced: true` already passes through". True within `WalletBackend`, but `SyncHandle::status` reaches `SyncPhase::Synced` through its own `is_following` call and can still emit `{phase: "synced", peak_height: null}` on `control.wallet.sync-status`. That path is out of scope (a status endpoint, and a phase machine owned by dig_ecosystem#2761), but a false "this is the only gate" in custody-adjacent code is exactly what makes the next reader skip the check. Co-Authored-By: Claude <noreply@anthropic.com> * test(wallet): vary BOTH directions of the coin-hex case mismatch The arrival fixture wrote the parent upper-case and the child's `parent_coin_info` lower-case, and only that. Reverting the `parent_coin_info` normalisation alone left the test GREEN: with the stored `coin_id` normalised, "AABB" already became "aabb" and matched the child's pointer without the second normalisation ever running. The two directions are repaired by different binds, so a fixture carrying one cannot see the other. A second change coin now points at its parent in upper case while the parent is stored lower, which makes both binds load-bearing: reverting either one alone now fails, announcing 2 arrivals where 1 is correct. Co-Authored-By: Claude <noreply@anthropic.com> * fix(wallet): make the coin-hex migration unable to brick the wallet The two-spelling collision rule could not see a coin id stored under several NON-canonical spellings. `AAbb` and `aAbb` are both unequal to their own lower-casing, so the case-scoped DELETE removed neither and the UPDATE then collided them onto one unique key. The transaction rolled back correctly and the ladder mark stayed unset -- but the retry on the next open is byte-for-byte identical, so `migrate` failed forever and the wallet never opened again. A rollback is a safe failure only when the retry can succeed. Collisions are now resolved before the update, for any number of spellings, by a total and deterministic rule: keep the spelling that is already canonical (a group holds at most one), else the lexicographically smallest. Recency is deliberately not a tie-break -- the rows are identical apart from case, so the table carries no evidence of which was written last. Dropped rows are logged at WARN, since in this tree a collision can only mean a non-conforming `ChainFallback`/`CoinPeer` implementation wrote to the replica. `arrival_pending.coin_id` and `arrivals.coin_id` are normalised in the same transaction. They hold copies of `coins.coin_id` and are compared against it raw, so normalising the coin table alone was a desync with two money-visible consequences: `record_arrivals` prunes every held row whose id is no longer in `coins`, and losing the hold is how a deferred coin falls below the baseline watermark and is never announced; and `INSERT OR IGNORE INTO arrivals` stops recognising an id it already recorded, announcing a coin to the user twice. Correct the migration docstring, whose justification was false and inverted. A lower-case id is the ORDINARY pre-fix spelling -- every in-tree writer has used `hex::encode` or an explicit normalisation since long before this change -- so an upper-case row cannot be the staler observation. The only verbatim path into the table is `refresh_tracked_coins`, the point-read used precisely because the subscription replica is behind, so an upper-case row would be the FRESHER one. Case carries no recency information in either direction. The migration's real warrant is that `ChainFallback` and `CoinPeer` are public traits. Narrow the normalisation claim in `upsert_coin` and SPEC.md to the two coin identities that are actually normalised. `puzzle_hash`, `asset_id` and `hint` are stored verbatim while `unspent_coins_scoped` lower-cases what it binds against them; that is a real defect, filed separately, and not something either statement may imply is already handled. The migration test's twins were identical in every column but case, so a rule that kept the wrong twin passed it unchanged. The twins now differ, and the test names which survives. Co-Authored-By: Claude <noreply@anthropic.com> * docs(wallet): point the narrowed hex claim at its filed follow-up Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>

MichaelTaylor3d and others added 2 commits August 12, 2026 07:40

chore(wallet): open the #2762/#2764/#2763 wallet-correctness lane

8a3b6c3

Salvage anchor so the lane's branch exists on remote before implementation starts. Removed before merge. Co-Authored-By: Claude <noreply@anthropic.com>

github-advanced-security Bot found potential problems Aug 12, 2026

View reviewed changes

style(wallet): rustfmt the salvaged derivation window

97d34f5

Cosmetic only; no behaviour change. Verifies the salvaged dd8a37e against fmt + clippy, both of which were red on that commit. Co-Authored-By: Claude <noreply@anthropic.com>

github-advanced-security Bot found potential problems Aug 12, 2026

View reviewed changes

Comment thread crates/dig-wallet/src/sage/custody.rs

fn measure_default_window_cost() {

let (c, dir) = custody_with_window(DEFAULT_DERIVATION_COUNT);

let t = std::time::Instant::now();

c.import(ABANDON, "passphrase", None).unwrap();

MichaelTaylor3d and others added 4 commits August 12, 2026 08:53

fix(wallet): resolve custody path from the nested test module

f14d35e

Co-Authored-By: Claude <noreply@anthropic.com>

This was referenced Aug 22, 2026

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

Merged

dign wallet peak reports synced: true on a replica 1,875 blocks behind — the latch, not the freshness gate #293

Closed

MichaelTaylor3d closed this Aug 22, 2026

Navigation Menu

Uh oh!

fix(wallet): adaptive+hardened derivation, real pending transactions, in-flight coin reservation - #216

fix(wallet): adaptive+hardened derivation, real pending transactions, in-flight coin reservation#216
MichaelTaylor3d wants to merge 7 commits into
mainfrom
loop/2762-wallet-correctness

MichaelTaylor3d commented Aug 12, 2026

Uh oh!

Uh oh!

MichaelTaylor3d commented Aug 13, 2026

Uh oh!

MichaelTaylor3d commented Aug 22, 2026

Uh oh!

MichaelTaylor3d commented Aug 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

MichaelTaylor3d commented Aug 12, 2026

Boundary this PR does NOT cross

Single-writer

Uh oh!

Uh oh!

MichaelTaylor3d commented Aug 13, 2026

Uh oh!

MichaelTaylor3d commented Aug 22, 2026

Uh oh!

MichaelTaylor3d commented Aug 22, 2026

The trap this PR was carrying

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants