Fix wallet storage self-healing for stuck change#188
Conversation
|
tonesnotes
left a comment
There was a problem hiding this comment.
None of this looks wrong. More belt and suspenders plus enhancements to idb where it lagged knex and additional recovery robustness.
In general, spendable should reflect the truth of whether an output has been spent or not. spentBy is a lame, incomplete hint that allows tracking spend flows in only a narrow class of situations and is also hard to maintain over some recovery flows. Adding the checks for spentBy also being null elevates the required validity threshold of the property which may now trigger recovery issues where spentBy is somehow not null but the output is spendable. As in, there was an attempt to spend this output by another transaction, but somehow that failed in an unusual way such that the spentBy didn't get reset to null.


Summary
This PR addresses a customer-observed wallet storage state that can brick
createActionin browser-local storage and leaves web apps unable to repair the wallet through BRC-100 calls.The problematic state is a contradictory change row where an output remains
spendable = truewhile also retainingspentBy = <failed transaction id>. In that state, change allocation can keep selecting an output that is still logically locked by a failed or otherwise unrecoverable transaction. If the holder transaction references a txid whose raw bytes/proof are no longer available, the double-spend review path can throwWERR_INVALID_PARAMETERfromgetBeefForTransactioninstead of returning a reviewable double-spend result. From the app side, admin-only basket access and hidden/unabortable action states mean there is no reliable BRC-100 repair path.Changes
spentByset from wallet balance and change allocation eligibility in both IndexedDB and Knex storage.reviewStatusrepair path so it now:reviewStatusbefore thespecOpInvalidChangerelease sweep, giving apps an existing non-admin repair trigger that can clear this stale failed-transaction lock class before chain-validating change outputs.WERR_INVALID_PARAMETERto the caller.spendablerows that still havespentByset;Why these changes are needed
The observed failure was not simply a bad UTXO. It was local storage drift that made the wallet believe an output was available for funding while the same row still pointed at a failed transaction as its spender. That is enough to trap repeated
createActionattempts in the funding/input validation path, and the current app-facing repair tools cannot target the hidden holder transaction or admin-only default basket directly.The defensive eligibility checks prevent the allocator from making the stuck state worse. The IndexedDB status review brings browser-local storage closer to the existing Knex repair behavior. Running review from the invalid-change release specOp gives applications a practical repair entry point without broadening basket permissions. The txid-only BEEF fallback preserves the intended
doubleSpendreview semantics even when the competing transaction is no longer locally reconstructable.Validation
corepack pnpm install --frozen-lockfilecorepack pnpm --filter @bsv/wallet-toolbox exec jest --runTestsByPath src/storage/__test/StorageIdb.test.ts --runInBandcorepack pnpm --filter @bsv/auth-express-middleware run buildcorepack pnpm --filter @bsv/payment-express-middleware run buildcorepack pnpm --filter @bsv/wallet-toolbox run buildcorepack pnpm --filter @bsv/wallet-toolbox exec ts-standard src/storage/StorageIdb.ts src/storage/StorageKnex.ts src/storage/StorageProvider.ts src/storage/__test/StorageIdb.test.ts src/storage/methods/reviewStatusIdb.tsNote: full
@bsv/wallet-toolboxlint currently fails on pre-existingno-unnecessary-type-assertionfindings insrc/__tests/WalletPermissionsManager.pmodules.test.ts; those files are unrelated to this patch.