Uh oh!
There was an error while loading. Please reload this page.
fix(wallet): scope the chain-fallback CAT read to its asset id - #224
Conversation
The $DIG balance row reports a holding the user does not have: the chain-fallback branch of the balance read drops the asset id and sums every coin hinted to the address. Stub commit to open the lane (dig_ecosystem#2879). Co-Authored-By: Claude <noreply@anthropic.com>
`control.wallet.balance` reported a $DIG holding the user does not have. Both `balance_for_address` and `coins_for_address` computed the CAT asset id, applied it in their DB branch, and DROPPED it in their chain-fallback branch: they called `coin_records_by_hints` and treated the whole answer as $DIG. A hint is not an asset. `get_coin_records_by_hints` takes no asset argument and answers with every coin hinted to a puzzle hash -- any CAT, and any plain XCH coin whose spend carried a hint memo. The reported figure is that sum rendered at the wrong scale: one hinted XCH coin of 10^8 mojos (0.0001 XCH) shows as 100000 $DIG at $DIG's 3 decimals. Scope the hint read to the asset by keeping only coins that live where that asset's coins live -- `cat_puzzle_hash(owner_ph, asset_id)`, the canonical curry `digstore-chain` already uses for CAT balance, coin reconstruction and sends. That is the exact fallback equivalent of the DB tier's `hint IN (...) AND asset_id = ?`, so the two tiers of one read now agree about which asset they are reading. The two branches share ONE helper, since duplicating the scoping is how one copy came to be missing. Refs dig_ecosystem#2879. Co-Authored-By: Claude <noreply@anthropic.com>
Puts the rule where the next caller will read it, since the defect was a correct-looking call to a method whose name implies a scoping it does not do: * `ChainFallback::coin_records_by_hints` documents that it answers with every coin hinted to an address -- any CAT of any TAIL, and any plain XCH coin carrying a hint memo -- and that a caller wanting ONE asset must filter by that asset's CAT puzzle hash. Both legitimate shapes are named, so the sync pass that uses the answer whole still reads as correct. * SPEC.md sharpens `control.wallet.balance`/`.coins`: the asset scoping binds BOTH tiers, the fallback tier's equivalent of `hint IN (...) AND asset_id = ?` is the canonical CAT puzzle-hash filter, and over-filtering is called out as the same money lie mirrored. Patch, not minor: a read that reported the wrong figure now reports the right one. No method, parameter, field or type changes, and no caller has to adapt. Refs dig_ecosystem#2879. Co-Authored-By: Claude <noreply@anthropic.com>
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
VERDICT: CHANGES-REQUIRED (recorded as a comment review — GitHub refuses REQUEST_CHANGES on a PR sharing the review identity, HTTP 422; the inline thread below is the blocker and bars merge under required_conversation_resolution.)
Correctness gate — CHANGES-REQUIRED (one gating finding, mechanical)
Head reviewed: 4d860c102dd3bed3e4edcf85165396139d25863d (resolved from the remote, not from the dispatch prompt).
The money correctness of this fix is CLEAN, and I verified it by execution, not by reading. The single blocker below is a documentation-placement regression the diff introduces; it has no behavioural effect and is one block-move to fix. Nothing about the fix itself needs to change.
Verified BY EXECUTION (own worktree, no shared checkout mutated)
| probe | result |
|---|---|
| full suite at head | 549 passed; 0 failed; 1 ignored — matches the PR body |
| revert-proof: filter removed, everything else intact | a_fallback_dig_balance_counts_only_dig_coins -> left: 107012345, right: 12345; a_fallback_dig_coin_list_... -> left: ["foreign-cat","hinted-xch","pending-dig","real-dig"]. Both fail for exactly the right reason, and the figures are the PR body's, independently reproduced |
over-filter direction (the one that gates): curry arguments SWAPPED to cat_puzzle_hash(asset_id, owner) — the wrong-but-plausible curry | both tests fail with left: 0, right: 12345 and left: [], right: ["pending-dig","real-dig"] |
That third probe is the answer to "is this a fix, or is the number merely smaller?" The tests discriminate both inequalities: an under-filter answers 107012345, an over-filter answers 0, and only the correct curry answers 12345. A fixture of 12_345 / 678 / 100_000_000 also means a scale confusion cannot pass — significant digits sit low in the range, which is right for a defect that is a scale confusion.
Verified BY READING
- The curry is the canonical one, applied in the right order.
digstore-chain/src/cat.rs:39iscat_puzzle_hash(owner_puzzle_hash: Bytes32, asset_id: Bytes32); the call passes(owner, asset_id). Same helper the crate's own CAT paths use —cat_balance(cat.rs:57) andreconstruct_cat_coins(cat.rs:91) both enumerateunspent_coins(cat_puzzle_hash(owner_ph, asset_id)). Nothing is hand-rolled. - No spend path can now fail to fund itself.
coins_for_addresshas one production consumer,wallet_coins(crates/dig-node-service/src/control.rs:1589) — an RPC read; the node builds no spend from it. The set it now returns is exactly the set the canonical CAT spend builder enumerates (same puzzle hash, item 1), so the list is now consistent with what is actually spendable. Pre-fix it was a superset carrying hinted-XCH and foreign-TAIL coins — a$DIGspend built from that list would have been built on foreign inputs. The change strictly removes unspendable inputs; it removes none that a spend could have used. - Case/prefix cannot cause an accidental over-filter in production. The filter compares
normalize_ph(&c.puzzle_hash)(rpc.rs:4415— strips0x, lowercases) againsthex::encode(...); and the real tier'sCoinsetFallback::map_record(fallback.rs:285) already normalizes vianorm_hex(fallback.rs:210, identical rule). Double and production agree. - The double is faithful, and the old one genuinely could not host this.
MockFallback::coin_records_by_hintsreturnsOk(vec![])(fallback.rs:1102) — it cannot express a multi-asset hint set, so every pre-existing$DIG-fallback test was blind to the hint path by construction.EveryHintedCoinoverstates nothing: the real read isget_coin_records_by_hints(&hints, None, None, true)(fallback.rs:324), asset-blind and spent-inclusive, which is what the double models. - Sweep re-done independently (not taken on the lane's word):
pending_scoped(db.rs:1245) does takeasset_idand filtersAND asset_id = ?/AND asset_id IS NULL— the brief's named suspect is genuinely clean, and it is DB-only.refresh_tracked_coins(rpc.rs:2637) uses both reads whole, writes rows withasset_id/hintunset, and attributes TAILs afterwards viasingleton::reconstruct_all— it never claims one asset, so it is the other legitimate shape.chain.rs:256is a bare delegate.
§2.0: gitnexus had no index for this worktree, so blast radius was established by caller enumeration + direct reads — stated, as the fallback requires.
The one blocker
See the inline thread on crates/dig-wallet/src/sage/rpc.rs. Two non-gating notes are posted separately and I resolve those myself so they do not bar merge.
Re-gate after the fix will re-verify only that delta.
Uh oh!
There was an error while loading. Please reload this page.
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
Non-gating note (see inline) — posted for the record and resolved by the reviewer so it does not bar merge.
Uh oh!
There was an error while loading. Please reload this page.
MichaelTaylor3d
commented
Aug 14, 2026
CORRECTNESS GATE: CHANGES-REQUIRED Head reviewed: The money fix itself is CORRECT. The one blocker is a documentation defect the diff introduced on the very read it fixes; it is already handed to 1. GATING — |
MichaelTaylor3d
commented
Aug 14, 2026
SECURITY GATE: PASS Head audited: No BLOCKING security defect in the diff. The change strictly reduces the money-lie surface it was written to fix, and I could not construct a path by which an attacker turns it into a false zero. The three threat questionsQ1 -- can an attacker make the filter EXCLUDE real $DIG? No path found. The filter key set is derived only from (the requested address p2 hash, the compile-time I specifically hunted the hint-flood truncation vector, since a hint is a permissionless write and truncation would silently drop the real coin: the peer leg PAGINATES to Q2 -- is the fix complete? Yes for this surface. Both fallback consumers now route through the single helper ( Q3 -- fail open or fail closed? CLOSED, and that is the right direction here.
Boundaries
Findings -- all NON-GATING (phase-1 posture, CLAUDE.md 2.6)1. MEDIUM (pre-existing, narrowed not closed) -- the filter is a puzzle-hash filter, not a lineage proof, so an unlineaged "fake CAT" still inflates the fallback $DIG figure. Scenario: an attacker spends their own XCH creating a coin with 2. LOW / gate-integrity -- the three new tests cannot catch a WRONG curry.
Cheap hardening, one line: add an 3. LOW -- a revocable CAT, or any non-p2 inner puzzle, is excluded by construction. 4. INFO -- tier divergence, and SPEC slightly overstates the equivalence. DB tier = hint IN (p2) AND asset_id = dig (TAIL-attributed, inner-puzzle agnostic); fallback tier = puzzle_hash == cat_puzzle_hash(p2, dig) (curry-exact, lineage-blind). They differ for (i) a $DIG coin hinted to us with a non-standard inner puzzle (DB includes, fallback excludes) and (ii) an unlineaged fake at the canonical hash (fallback includes, DB excludes). 5. INFO -- shared error code checked, NOT live. Measured vs reasonedMEASURED (read at source): the two call sites and the single helper; REASONED (arithmetic, not compiled): the new tests are non-vacuous -- the fixture sums to 107,012,345 unfiltered against an asserted 12,345 / 678, so deleting the filter cannot leave them green. I did not build a worktree; the claim most at risk (Q1 false zero) is not settleable by a unit test that reuses the production curry, which is finding 2. Merge preconditions (for the orchestrator, not part of this verdict)Required checks asserted BY NAME at 4d860c1 -- Lint commit messages, Check version increment, Rustfmt, Clippy, Test + coverage: all SUCCESS. unresolvedReviewThreads = 1 (the correctness leg GATING doc-placement finding at |
…ress Relocate the asset_scoped_fallback_coins function with its own doc comment to precede the balance_for_address doc comment, restoring the direct association between the documentation and the function it describes. This fixes the misplaced doc comment that was inserted between balance_for_address's documentation and its function definition. Fixes dig_ecosystem#2879. Co-Authored-By: Claude <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
DO NOT MERGE — draft. The orchestrator gates and merges.
Closes DIG-Network/dig_ecosystem#2879.
TLDR
The
$DIGbalance row reported a holding the user does not have. Bothbalance_for_addressand
coins_for_addresscomputed the CAT asset id, applied it in their DB branch, anddropped it in their chain-fallback branch — summing every coin hinted to the address as
$DIG. Fixed by scoping the hint read to the asset via the canonical CAT puzzle hash, inone helper both branches share.
Root cause — confirmed, and how
Confirmed at source level, and measured in a test. A hint is not an asset:
get_coin_records_by_hintstakes no asset id and answers with every coin hinted to a puzzlehash — any CAT of any TAIL, and any plain XCH coin whose spend carried a hint memo. It
reads like "the CAT read" and is not one.
The arithmetic corroborates the report exactly:
0.0001 XCHis 10^8 mojos, and 10^8 baseunits rendered at
$DIG's 3 decimals is precisely100000— the figure the user saw besideit. One hinted XCH coin, counted twice, once at each asset's scale.
Measured before/after on the fixture (revert-proof: the fix reverted in place, tests re-run,
then restored — the tree is byte-identical to the committed state):
$DIGbalance$DIGcoin list["foreign-cat", "hinted-xch", "pending-dig", "real-dig"]["pending-dig", "real-dig"]$DIGcoinsdig-app is innocent and untouched. Its two per-asset reads, per-asset wire mapping and
per-asset formatting are all correct; a divisor change there would have made the screenshot
look right and hidden this permanently.
The fix
A CAT coin does not sit at its owner's p2 puzzle hash — it sits at the OUTER hash currying its
TAIL around that hash, and is merely hinted to it. So keeping only coins at
digstore_chain::cat::cat_puzzle_hash(owner_ph, asset_id)is the exact fallback-tierequivalent of the DB tier's
hint IN (…) AND asset_id = ?, and the two tiers of one read nowagree about which asset they are reading.
That curry is the canonical construction the crate's CAT balance, coin reconstruction and send
paths already use — not hand-rolled here; a second spelling of a curry that decides whether
money is counted is a future byte-drift bug.
Both branches call one shared
asset_scoped_fallback_coins, because duplicating thescoping is how one copy came to be missing.
Both directions are tested, deliberately
Over-filtering is the same money lie mirrored — a real
$DIGholder shown a zero. The fixtureis built to fail both wrong implementations:
100_000_000mojos — the reported case; must not count;$DIGCAT coin,12_345— must still count (significant digits low down onpurpose: a round fixture passes under several scales, and this defect is a scale confusion);
7_000_000— a second asset, so a filter atthe wrong layer changes the answer instead of preserving it;
$DIGcoin,678— the pending figure is asset-scoped too, not only the sum.Plus an XCH control over the same fixture returning a real non-zero
100_000_000, proving thefilter did not simply suppress the fallback tier.
MockFallbackcould not host this test at all: its hint read returns an empty list, so it cannotexpress a multi-asset hint set. A double faithful to the tier (
EveryHintedCoin) replaces it.Sweep — the same defect shape, repo-wide
Every caller of the asset-blind hint read, and every asset-scoped DB read whose fallback peer
could disagree:
rpc.rsbalance_for_addressrpc.rscoins_for_addressrpc.rs:2636refresh_tracked_coinschain.rs:257rpc.rswallet_coinsfallback CAT armOk(vec![])for any CAT — an under-report, not an over-reportdb.rspending_scopedasset_idand filters on it, and is only reached from the DB branchtoken_record,get_sync_statusselectable balanceCoherence
SPEC.mdcontrol.wallet.balance/.coinsnow state that the asset scoping binds bothtiers, name the CAT-puzzle-hash filter as the fallback equivalent of the DB predicate, and call
out over-filtering as the mirrored lie. The old text claimed
$DIG"scopes by the canonical CATasset id" — true of one tier only, which is exactly the bug.
ChainFallback::coin_records_by_hintsdocuments its asset-blindness on the method itself, so thenext caller reads the rule at the callsite. Both legitimate shapes are named, so the sync pass
that uses the answer whole still reads as correct.
canonicalskill gains "a hint is not an asset" (§2.0 append-on-discovery).Blast radius
gitnexus was not available (fresh worktree, no index; §2.0 permits the fallback and requires
saying so), so the radius was established by exhaustive caller enumeration + direct reads:
balance_for_address→ one production caller,dig-node-service/src/control.rs:1337(
control.wallet.balance), plus one e2e test that reads XCH (unaffected).coins_for_address→ one production caller,control.rs:1595(control.wallet.coins).sage/rpc.rswith no other callers. Nothing was renamed.(
dig-wallet/src/lib.rshas its own unrelatedcat_asset_id/parse_asset_id_hexfree functions— a name coincidence in a different module, which is why a find-and-replace was not used.)
only the figure changes, which is the point. Risk: low, and confined to what the two
control.wallet.*reads report when served by the fallback tier.Evidence
cargo test -p dig-wallet --lib→ 549 passed, 0 failed, 1 ignored.cargo check --workspace --all-targets→ clean;cargo clippy -p dig-wallet --lib --all-features→ no warnings.derivationsandcoinsboth 0 rows), which is why the read fell back in the first place, so no live figure could be
taken. The claim that this is the reported defect rests on the source-level proof plus the exact
arithmetic match — stated as such, not as a live reproduction.
Version
0.118.0→ 0.118.1, patch: a read that reported the wrong figure now reports the right one,with no API, wire or type change and no caller adaptation.