Skip to content

fix(wallet): normalise stored hex at the writer and gate token balances on replica coverage - #300

Merged
MichaelTaylor3d merged 1 commit into
mainfrom
loop/298-247-balance-honesty
Aug 22, 2026
Merged

fix(wallet): normalise stored hex at the writer and gate token balances on replica coverage#300
MichaelTaylor3d merged 1 commit into
mainfrom
loop/298-247-balance-honesty

Conversation

@MichaelTaylor3d

@MichaelTaylor3dMichaelTaylor3d commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

DRAFT — DO NOT MERGE. Gate round has not run.

Also sequenced behind WU4 (the chia-protocol 0.36.1 / SDK 0.34 workspace uplift, epic #2761), which merges first. This branch expects to rebase onto it.

Closes#298
Closes#247

Parent epic: https://github.com/DIG-Network/dig_ecosystem/issues/2760 — these are its two remaining blockers.


Two falsehoods on one read path: the user's $DIG/XCH balance

#298 — an upper-case puzzle hash read as a zero balance

unspent_coins_scoped and five sibling scoped readers bind to_ascii_lowercase() values against columns their writers stored verbatim. A chain source (or a client) that spelled a value in upper case made the coin invisible, and balance_scoped — the user-facing balance — reported zero. Same shape as the "have 0 $DIG" failure recorded at fallback.rs:272-276.

Fixed at the WRITER, as #295 did for coin_id, so the class is closed rather than enumerated.

#247token_record reported a confident zero with no gate at all

rpc.rs returned balance / selectable_balance from db.balance_scoped with no completeness field and no coverage gate. A scope the replica does not cover answered 0, indistinguishable from genuinely holding none — and because login does not enrol (enrolment is control.wallet.watch), that state is permanent, not transient. Reached by get_token, get_cats, get_all_cats.


#298: the full enumeration

Every (table, column) holding a hex value compared against a lower-cased bind:

Table.columnKey?Indexed?Writers now normalisingExisting rows
coins.puzzle_hashnoidx_coins_phupsert_coin, upsert_coinsladder step 3
coins.asset_idnoidx_coins_asset, idx_coins_unspentupsert_coin, upsert_coins, attribute_cat_coinladder step 3
coins.hintnonoupsert_coin, upsert_coins, attribute_cat_coinladder step 3
derivations.puzzle_hashno (PK is hardened,idx)idx_derivations_phupsert_derivationladder step 3
cats.asset_idPRIMARY KEY(the PK)upsert_cat, update_cat_metadataladder step 3 + collision merge
arrivals.puzzle_hash, .asset_idnonocopies of coins (already normal)ladder step 3
chain_read_cache.puzzle_hash, chain_spend_cache.puzzle_hashnonoupsert_*_cacheladder step 3

Readers. Six scoped readers (unspent_coins_scoped, balance_scoped, pending_scoped, coins_scoped, coin_count_scoped, owned_cat_asset_ids_scoped) already bound lower-cased values and are fixed by the writer change alone — no read-side edit. Five readers taking a key from the CALLER bound it verbatim and now normalise: cat, clear_cat_metadata, update_cat_metadata, is_asset_owned, unspent_coins.

record_arrivals was already immune and needed no change: arrivals.rs:160 lower-cases the coin's puzzle hash before testing the watched set, and puzzle_hash_matching_is_case_insensitive already pinned it. Recorded because the issue flagged it as needing checking.

Index ruling — zero degradation, unlike #295's first attempt

No column is wrapped in LOWER() anywhere. The defect was never a read-side LOWER(column); it was a column that had not been normalised. The binds already lower-case the value, which reads straight down the index. So idx_coins_ph, idx_coins_asset, idx_coins_unspent, idx_derivations_ph and the cats primary key all stay fully usable — the writer fix is what makes that possible, and a reader-side fix is what would have destroyed it. Stated in the normalise_hex doc so the next person does not "add LOWER() as belt-and-braces".

Existing rows — and #295's warrant does NOT transfer

#295 could argue every in-tree writer already emitted hex::encode, so its migration matched zero rows on any wallet this ecosystem wrote and defended only against a third-party ChainFallback/CoinPeer.

That argument holds for the coins columns and derivations. It fails for cats.asset_id:actions.rs:21update_cat persists a caller-supplied TokenRecord, so the update_cat RPC is an in-tree, shipped, reachable way to write a shouted asset id today, with no out-of-tree implementation required. Such a row is unreachable by every canonical lookup, and its name/ticker/icon are not derivable from chain. So ladder step 3 is a genuine repair for at least one column, not defence in depth.

The migration cannot brick the wallet

Ladder step 3 (SCOPED_HEX_STORED_LOWER_CASE), one transaction, PRAGMA user_version gated.

  • Eight of the nine columns are not keys, so two spellings are two legitimate rows — plain UPDATE, nothing to collide.
  • cats.asset_idis a PK, so collisions are resolved first. The N-way group logic is shared with fix(wallet): honest peak sync flag, adaptive derivation window, coin reservation, real pending set #295 via a new case_groups helper (which drop_case_collisions now also uses), so the group is the complete lower-value equivalence class — the property that stops AAbb/aAbb/AAbB surviving the resolver and aborting the UPDATE. That abort is what left fix(wallet): honest peak sync flag, adaptive derivation window, coin reservation, real pending set #295's first version returning Err from WalletDb::open forever.
  • Losers are MERGED, not dropped.drop_case_collisions deletes, justified by its tables being chain-derived. A cats row is not: its metadata comes from a registry or the user. So non-NULL fields are COALESCEd onto the survivor first.
  • <> LOWER(col) is NULL-safe, so a NULL asset_id/hint survives as NULL — load-bearing, since asset_id IS NULL is how an XCH coin is told from a CAT. Pinned by its own test.

#247: the gate, and what an uncovered scope gets

Gate is replica_covers_client_scope — the same predicate get_coins/get_sync_status route on (#2878), so the balance and the coin list cannot disagree about completeness. Deliberately notreplica_is_authoritative, which is vacuous here: under §908 the followed set may be empty, every recording contains it, and every client would pass while nothing changed.

ReadUncovered scope returns
get_token (XCH)routed to the chain, confirmed-and-unspent re-filtered
get_token (XCH), no live chain source503
get_token (CAT)503
get_cats503
get_all_cats503 (propagates from token_record)

Why the two assets differ. XCH coins sit AT the identity's puzzle hashes, so the fallback can be asked directly — the get_coins remedy applies unchanged. A CAT is only hinted to its owner and needs puzzle uncurrying the fallback tier does not perform, so routing it to the chain returns an empty set — the same confident zero through a different door. TokenRecord cannot spell "unknown" without a dig-node-control-interface contract change, which must publish first (#2609), so refusing is the smallest honest option available today and, unlike a zero, is a shape no caller can mistake for a figure.

The chain arm re-applies the confirmed-and-unspent predicate rather than summing raw: coin_records_by_puzzle_hashes returns recently spent coins by design, and summing them would report already-spent money as held — a falsehood in the opposite direction and a worse one to act on.

get_cats is gated separately, and that is a placement decision, not a duplicate. The asset-id LIST is itself a replica read: an uncovered scope yields no ids, so every per-token gate downstream is never reached and the caller is told it owns no CATs. Proven below.

New ErrorKind::Unavailable503, distinct from Internal: Internal means the node tried and broke; Unavailable means the node is working correctly and is not entitled to the answer.


Evidence

Per-bind revert proofs — #298, 15/15, each isolating exactly ONE test

Committed first, then reverted by file copy (never git checkout -- <path>), one bind at a time, by REPLACEMENT so the revert still compiles.

[PROVEN] upsert_coin.puzzle_hash -> an_upper_case_puzzle_hash_still_counts_toward_the_xch_balance
[PROVEN] upsert_coin.asset_id -> an_upper_case_asset_id_still_counts_toward_the_cat_balance
[PROVEN] upsert_coin.hint -> an_upper_case_hint_still_counts_toward_the_cat_balance
[PROVEN] upsert_coins.puzzle_hash -> the_batch_writer_also_normalises_an_upper_case_puzzle_hash
[PROVEN] upsert_coins.asset_id -> the_batch_writer_also_normalises_an_upper_case_asset_id
[PROVEN] upsert_coins.hint -> the_batch_writer_also_normalises_an_upper_case_hint
[PROVEN] attribute_cat_coin.asset_id -> cat_attribution_normalises_an_upper_case_asset_id
[PROVEN] attribute_cat_coin.hint -> cat_attribution_normalises_an_upper_case_hint
[PROVEN] upsert_derivation.puzzle_hash -> an_upper_case_derivation_puzzle_hash_is_still_recognised_as_ours
[PROVEN] upsert_cat.asset_id -> cat_metadata_written_under_an_upper_case_asset_id_is_still_found
[PROVEN] update_cat_metadata.asset_id -> cat_metadata_updated_under_an_upper_case_asset_id_is_still_found
[PROVEN] cat.lookup -> a_caller_asking_for_cat_metadata_in_upper_case_is_still_answered
[PROVEN] clear_cat_metadata.asset_id -> a_caller_clearing_cat_metadata_in_upper_case_still_clears_it
[PROVEN] is_asset_owned.asset_id -> a_caller_asking_whether_an_upper_case_asset_is_owned_is_still_answered
[PROVEN] unspent_coins.asset_id -> an_upper_case_asset_id_from_a_caller_still_reads_its_coins
ALL BINDS PROVEN

Each fixture varies the case of exactly one column and spells every other canonically — which is why each revert reddens exactly one test rather than several. A fixture varying two at once stays green on a single revert, because the other mismatch keeps the row invisible for the wrong reason.

The proof run found two real gaps, both fixed:

  1. The first run silently skipped two binds — an 8-space anchor is a substring of a 12-space line, so upsert_coin.asset_id/.hint matched twice and were reported ambiguous rather than proven. Anchors now carry a leading newline.
  2. cat.lookup came back NOT LOAD-BEARING. Every test shouted at the writer and read back canonically, which is the direction a verbatim reader bind still gets right. Three tests were added that shout at the READER instead (cat, clear_cat_metadata, is_asset_owned), and all three are now proven.

Per-guard revert proofs — #247, 4/4

[PROVEN] token_record: the XCH coverage route -> 2 tests red
[PROVEN] token_record: the CAT refusal -> 1 test red
[PROVEN] scoped_xch_balance: no-chain-source refusal -> 1 test red
[PROVEN] get_cats: the list-level gate (PLACEMENT) -> 1 test red
ALL GUARDS PROVEN

The get_cats line is the placement proof: reverting it leaves token_record's CAT refusal fully intact, and the read still lies, because the list never reaches token_record. A test that could not tell those two positions apart would pin a coincidence.

RED-before-green, for the right reason

The #247 XCH test failed with left: Some(0), right: Some(1599000000000), and the CAT test with a literal 200 carrying "balance":0 — the defect verbatim, not a status-code mismatch.

Fixture correction found by the proofs

get_cats_returns_cat_tail_after_synced_cat_coin went red under the new gate. It writes the replica directly and never records coverage — describing a node that holds the owner's coins while claiming to follow no address at all, a state no real sync produces. Fixed by recording the catch-up the fixture implies, not by weakening the gate.

Tests, coverage, gates

  • Full dig-wallet suite: 686 lib + 22 conformance passed, 0 failed, 3 ignored.
  • New: 21 in stored_hex_is_case_insensitive + 5 for token_record reports a confident zero with no gate at all — get_token, get_cats, get_all_cats #247 = 26, including two controls (a genuinely foreign puzzle hash still counts for nothing; a covered scope still answers XCH, CAT and get_cats from the replica) so neither fix can be satisfied by "match everything" or "refuse everything".
  • cargo llvm-cov -p dig-wallet: 83.42% lines (floor 80).
  • cargo fmt --all -- --check clean; cargo clippy -p dig-wallet --all-targets -- -D warnings clean.

Version

0.136.2 → 0.137.0 (dig-wallet 0.28.0 → 0.29.0). Minor, and it is a judgement call worth checking: the response schema is unchanged and no API is removed or renamed, but get_token/get_cats now return 503 where they returned 200-with-a-zero, and ErrorKind gains a public variant. Read as minor because the 200-zero was a defect no correct client could depend on, and the Sage-parity model already requires clients to handle non-200 with a plain-text body. A reviewer who reads the 503 as a changed default should say so and this becomes 1.0.0-style major.


Blast radius checked

gitnexus fallback used, and stated as §2.0 bound (2) permits.analyze writes tracked files into the worktree, which would dirty a lane running beside three others in this repo. Blast radius was established by exhaustive grep over every SQL statement naming each column, plus reading each writer and reader — which for a "which statements touch this column" question is the precise instrument, not a weaker one.

  • normalise_hex — private, 2 callers before, 11 after; all in db.rs.
  • drop_case_collisions — 1 caller (normalise_stored_coin_hex); refactored to use the shared case_groups, behaviour unchanged, its own tests still green.
  • token_record — 3 callers (get_cats, get_all_cats, get_token); all three now inherit the gate, which is the intent.
  • ErrorKind — matched exhaustively in exactly one place, mod.rs:status(). Zero in-workspace exhaustive matches elsewhere (dig-node-service/dig-node-core use std::io::ErrorKind only).
  • balance_scoped — 4 callers. balance_for_address (rpc.rs:1347) was already correctly gated by routing::route and is deliberately untouched; get_sync_status was gated by fix(wallet): stop reporting confident answers the replica has not earned #246; token_record is what this PR gates.

Risk: MEDIUM, not high. No custody, no signing, no spend path — every change is a read gate or a stored-value normalisation. The one genuinely dangerous edge is the migration, because a migration that aborts is retried identically and bricks WalletDb::open forever; that specific failure has two dedicated tests (colliding_cat_spellings_..., many_non_canonical_cat_spellings_...) and is the reason the collision group must be the complete equivalence class.

git diff --stat origin/main..HEAD touches 7 files and nothing outside the expected set.


Deliberately out of scope

#253 (relay-reached peer renders [::]:0 — DIG-peer display, not the Chia path), #297, chia-query#30. Also: nfts.launcher_id / dids.launcher_id in is_asset_owned are a different column family, left verbatim and noted in the code.

…es on replica coverage
Two falsehoods on the same read path -- the user-facing $DIG/XCH balance.
dig-node#298: six scoped readers bind lower-cased values against columns their
writers stored verbatim, so a puzzle hash, asset id or hint spelled in upper case
made the coin invisible and `balance_scoped` reported ZERO. Fixed at the WRITER,
as #295 did for `coin_id`, which closes the class rather than enumerating it and
-- unlike a read-side `LOWER(column)` -- leaves every index usable.
Nine (table, column) pairs normalised across eleven bind sites: the three `coins`
scoping columns, `derivations.puzzle_hash`, `cats.asset_id`, both `arrivals`
columns and both chain caches. Ladder step 3 repairs rows already on disk in one
transaction. #295's "no in-tree writer can produce upper-case hex" warrant does
NOT transfer: `update_cat` persists a caller-supplied `TokenRecord`, so the RPC
is a shipped, reachable way to write a shouted `cats.asset_id` today. That column
is a PRIMARY KEY, so collisions are resolved over the COMPLETE lower-value
equivalence class before the update -- a partial group would collide a statement
later, abort the migration, and leave `WalletDb::open` failing forever. Losers
are MERGED rather than dropped, because CAT metadata is not derivable from chain.
dig-node#247: `token_record` returned `balance`/`selectable_balance` with no
completeness field and no gate, so a scope the replica does not cover answered a
confident zero indistinguishable from holding none -- permanently, since `login`
does not enrol. Gated on `replica_covers_client_scope`, the same predicate
`get_coins` routes on, so the balance and the coin list cannot disagree. XCH
falls to the chain; a CAT and the CAT LIST are REFUSED with a new 503
`ErrorKind::Unavailable`, because a CAT is only hinted to its owner and the
fallback tier does not uncurry, so routing it to the chain would return the same
zero through a different door.
Every bind and guard is revert-proofed individually: 15/15 for #298 and 4/4 for
#247, each reddening exactly one test. SPEC.md's narrowed hex-storage paragraph
is widened to match, and gains a normative requirement that no balance read may
report a figure for a scope the replica does not cover.
Closes#298Closes#247
Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d
MichaelTaylor3dforce-pushed the loop/298-247-balance-honesty branch from b81923e to 5d6dc54CompareAugust 22, 2026 16:36
@MichaelTaylor3dMichaelTaylor3d changed the title fix(wallet): normalise stored hex at the writer and gate the token balance on replica coveragefix(wallet): normalise stored hex at the writer and gate token balances on replica coverageAug 22, 2026
@MichaelTaylor3d

Copy link
Copy Markdown
ContributorAuthor

Gate evidence — required checks GREEN, asserted BY NAME

check-merge-preconditions.sh --repo DIG-Network/dig-node --pr 300 --allow-draft, at tip 5d6dc54:

draft=true mergeStateStatus=CLEAN unresolvedReviewThreads=0
Lint commit messages SUCCESS
Check version increment SUCCESS
Rustfmt SUCCESS
Clippy SUCCESS
Test + coverage SUCCESS
RESULT: preconditions MET

Workspace coverage from CI: 88.72% lines / 87.46% regions (gate --fail-under-lines 80).

Rebase note

Rebased onto f5cc47b (#301) after main moved. The only conflict was the version line — main took 0.136.3, this branch 0.137.0; resolved to 0.137.0, which is still an increment over main.

#301's content was verified BY NAME, not by size, per the --theirs incident on this repo: the retired-patch prose is present in both manifests (5 matching lines), and git diff f5cc47b..HEAD over the two files #301 touched is exactly one line — the version — so none of its prose was clobbered. This branch's own content re-verified the same way (normalise_hex_opt, SCOPED_HEX_COLUMNS, merge_cat_case_collisions, uncovered_scope_error, scoped_xch_balance, ErrorKind::Unavailable, both new SPEC clauses).

Note for the gate: while the PR was DIRTY the required checks read ABSENT — never ran, not red or pending. Worth knowing that a conflicted PR here reports as un-run rather than failing.

Squashed to one commit (the original header was 105 chars and commitlint caps at 100). Staying DRAFT until the gate round returns.

@MichaelTaylor3dMichaelTaylor3d left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PASS - correctness gate

Head reviewed: 5d6dc54f243a3e247a3469bd7a6135c69f1909eb (resolved from the remote; unchanged during the review). Reviewed in an isolated worktree; no shared checkout was mutated.

Verdict: PASS. No gating findings. Two non-gating observations at the end, both resolved by me so they cannot bar merge.

The two questions asked explicitly

1. Can the migration brick a wallet? No - and the brick risk it defends against is real, not theoretical.

  • Only one migrated column is a key.cats.asset_id is the sole PRIMARY KEY among the nine pairs. Of the eight in SCOPED_HEX_COLUMNS: coins is keyed on coin_id, derivations on (hardened, idx), arrivals on seq with UNIQUE on coin_id only, and both chain caches on coin_id. I checked every CREATE INDEX in the file - idx_derivations_ph, idx_coins_ph, idx_coins_asset, idx_coins_unspent - and none is UNIQUE. The eight plain UPDATEs have nothing to collide with, exactly as claimed.
  • No missing-column failure. A migration naming a column a legacy DB lacks fails hard, and arrivals.asset_id plus the two chain-cache puzzle_hash columns are NOT in ADD_COLUMN_MIGRATIONS. I read their introducing commits - b2f8f17 (arrivals) and 5382d86 (chain caches) - and all eight columns have existed since their table first appeared. Nothing can be absent.
  • The collision group is the complete equivalence class.case_groups keys on to_ascii_lowercase, so all spellings of one value land in one group and every non-survivor is DELETEd before the lower-casing UPDATE runs. surviving_spelling is total (already-canonical, else min; None only for an empty group, unreachable). The three-way AAbb/aAbb/AAbB case a pairwise rule misses has its own test.
  • No foreign key can turn a DELETE into a failure.foreign_keys(true) is set (crates/dig-wallet/src/sage/db.rs:766,773), so this mattered. The only REFERENCES in the schema is coin_reservations to pending_transactions; nothing references cats.
  • One transaction, mark after success. Everything runs inside one tx, and PRAGMA user_version is written only after it returns - so a failure rolls back whole and the next open retries.
  • Mutation-proved. Deleting the single line Self::merge_cat_case_collisions(&mut tx).await?; turns colliding_cat_spellings_neither_brick_the_wallet_nor_lose_metadata and many_non_canonical_cat_spellings_do_not_brick_the_walletred. The UNIQUE violation is genuine; the merge is what prevents it.

2. Does dig-node#295 drop-vs-merge warrant transfer? No - and merging is correct, not overbuild.

#295 warranted its drop with: every in-tree writer emits hex::encode, so the step defends only against a third-party impl, and a dropped row is re-observed from chain. Both halves fail for cats:

  • Reachability:upsert_cat (crates/dig-wallet/src/sage/db.rs:2458) and update_cat_metadata (crates/dig-wallet/src/sage/db.rs:2893) both write asset_id from a caller-supplied value, and actions.rs:21 update_cat is a shipped in-tree RPC reaching them. A shouted cats.asset_id needs no out-of-tree implementation. I confirmed this by reading the writers, independently of the PR body.
  • Recoverability: name, ticker, description and icon_url come from a registry or the user. Nothing on chain restores them. A drop here would have been silent user-data loss, not a re-syncable row.

So dropping would have been wrong, and the COALESCE-then-delete merge is the right shape. precision and visible are NOT NULL so the survivor keeps its own values - correctly documented.

The other claims, checked

3. Index ruling - zero degradation confirmed. No LOWER(column) appears in any predicate at head; every normalisation applies to the bound value (.bind(p.to_ascii_lowercase()) in the six scoped readers, Self::normalise_hex(...) at the writers). Every read still goes down idx_coins_ph / idx_coins_asset / idx_coins_unspent / idx_derivations_ph and the two primary keys. The reader-side alternative would indeed have been a full scan.

4. #247 uncovered-scope routing - the CAT argument holds.replica_covers_client_scope is the same predicate get_coins and get_sync_status route on, and the doc is right that replica_is_authoritative would be vacuous here: an empty followed set is trivially contained, so the gate would pass for every client. The XCH arm re-applies the confirmed-and-unspent predicate rather than summing the fallback raw - necessary, since coin_records_by_puzzle_hashes returns recently-spent coins. The CAT refusal is justified: a CAT coin is hinted, not sited, and attribution needs uncurrying the fallback does not do, so routing it would return an empty set - the same zero through a different door. Unavailable to 503 is a shape no Sage-parity caller can read as a figure.

5. Revert proofs spot-checked - three of them, and the labelling is better than claimed.

  • Reverting only the batch writerpuzzle_hash bind (crates/dig-wallet/src/sage/db.rs:1550) reddens exactly one test, the_batch_writer_also_normalises_an_upper_case_puzzle_hash. One bind, one test - the one-column-at-a-time fixture discipline does what it claims.
  • Reverting the merge call reddens exactly the two brick tests (above).
  • I tested the NOT-LOAD-BEARING label on the cat() lookup bind by reverting it - it now reddens a_caller_asking_for_cat_metadata_in_upper_case_is_still_answered. So the harness finding was acted on: a covering caller-side test was added and the bind is load-bearing at this head. That is the correct response to a not-load-bearing verdict, and it is evidence the AMBIGUOUS / NOT-LOAD-BEARING labelling is a real instrument rather than decoration - without it, that bind would have shipped unproven.

6. Both controls discriminate.a_coin_at_a_genuinely_foreign_puzzle_hash_still_counts_for_nothing refuses the match-everything shortcut - a worse defect, since it would report one wallet another wallet's money. a_covered_client_scope_still_reads_its_token_balances_from_the_replica refuses refuse-everything, and makes the CAT refusal a statement about coverage rather than about CATs.

7. The get_cats gate is a genuine placement proof. I removed the gate from get_cats while leaving the token_record refusal fully intact: an_uncovered_client_scope_refuses_to_list_its_cats still goes red. The empty-list path is genuinely reachable - no asset ids means the per-token gate is never called - so this is placement, not a duplicate of the token gate.

Suite, gates, coherence

  • 686 lib + 22 conformance, all green, run locally at this head (36.9s). Required checks asserted BY NAME via check-merge-preconditions.sh --allow-draft: Lint commit messages, Check version increment, Rustfmt, Clippy, Test + coverage - all SUCCESS; mergeStateStatus CLEAN, 0 unresolved threads.
  • The #301 rebase reverted nothing. The SPEC diff against base f5cc47b is exactly 3 hunks / +55 -16, all of them this PR's own: the 18.4 error model, the coverage rule, and the hex-storage rule. No #301 section is touched.
  • Stale-wording sweep is clean. The superseded sentence - that the rule is scoped to the two coin identities and does NOT presently extend to puzzle_hash, asset_id and hint - is gone from SPEC.md, and the normalise_hex doc-comment was rewritten rather than left contradicting the new behaviour. I grepped for surviving instances of the old rule's phrasing and found none. This is the contract-split trap and it was swept, not spot-checked.
  • Version bump 0.136.3 to 0.137.0, dig-wallet 0.28.0 to 0.29.0, is right.ErrorKind is a public enum gaining a variant - compatible for constructors, breaking only for an exhaustive external match - and a previously-200 path now returns 503. Minor is correct and the judgement is stated in the PR.
  • dig-constants: nothing here belongs there and nothing here duplicates it. Every value is a per-wallet SQLite column name, a table identifier, or an HTTP status - repo-local by nature, with no second implementation required to match. No finding either way.
  • No security leg needed. No key custody, no spend building, no signing; the node-signs-nothing boundary is untouched. The change makes a money surface more honest - a 503 instead of a fabricable zero - and the fail-closed direction is correct throughout, since an absent coverage recording covers nothing.

Two non-gating notes (resolved by me; not blockers)

  1. token_record sets selectable_balance to the same chain-derived figure as balance on the uncovered-XCH path. Selectable normally excludes reserved coins and this path is reservation-unaware - but an uncovered scope has no local reservations by construction, and the two fields were already coupled before this PR. No regression; worth remembering if reservations ever become chain-visible.
  2. The migration lower-cases the two chain caches' puzzle_hash, but those tables' writers (put_chain_read, put_chain_spend) still bind it verbatim, so post-migration rows are unnormalised again. Harmless today - nothing compares those columns, and a cached row's puzzle hash reaches coins only through the replay path, where the writer normalises it. A slight incoherence rather than a defect; the doc-comment already explains why the columns were included (legacy rows on disk).

Cleared to undraft and merge at the orchestrator's discretion. No findings were handed to Copilot - there are none to hand.

@MichaelTaylor3d
MichaelTaylor3d marked this pull request as ready for review August 22, 2026 17:48
@MichaelTaylor3d
MichaelTaylor3d merged commit 97183ca into mainAug 22, 2026
15 of 16 checks passed
@MichaelTaylor3d
MichaelTaylor3d deleted the loop/298-247-balance-honesty branch August 22, 2026 17:48
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant

@MichaelTaylor3d