Skip to content

Coordinate concurrent wallet receipts and complete broadcast results - #1066

Merged
ty-everett merged 4 commits into
mainfrom
codex/wallet-toolbox-acceptance-status-20261013
Oct 13, 2026
Merged

ty-everett merged 4 commits into
mainfrom
codex/wallet-toolbox-acceptance-status-20261013

Conversation

@BraydenLangley

@BraydenLangley BraydenLangley commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Program and scope

  • Tracker or issue: Wallet Toolbox receive reliability regression, with operational evidence retained privately.
  • Source correction is qualified for maintainer review: deterministic first receipt and same-recipient concurrency across SQL storage instances, with exact-head CI complete. PXC topology testing and production rollout remain deployment gates.
  • Why: a valid proofless payment can fail after broadcast with an internal unknown result; overlapping receipts can also count one on-chain output twice or credit it before broadcast succeeds.
  • Out of scope: Message Box acknowledgement retries, application accept-button behavior, automatic repair of historical wallet rows, package publication, and live deployment.
  • Exact head SHA reviewed: 0263a6566df547f7a89f07fce8a38c9ca4094ed1.

Root causes and resulting behavior

updateReqsFromAggregateResults preserves a durable request that has already advanced to unmined or completed, but previously returned before copying the provider result into the foreground response. aggregateActionResults then encountered unknown and threw after the recipient transaction header was committed. A retry could succeed because it took the existing-transaction path. The fix copies the actual provider status and competing-transaction evidence before that guard. Negative broadcast results still reject and do not credit new recipient outputs.

Separately, internalizeAction read for an existing user/TXID before its writer transaction. Two instances could both see no row, create separate transaction headers, and store the same user/outpoint twice because the old output constraint was keyed by transaction ID. A synthetic two-connection MySQL reproduction of one mined 5,000-satoshi payment returned two acceptances and a 10,000-satoshi local balance. Another interleaving let a second receive merge a committed header and credit the output while the first broadcast was pending; the first broadcast later failed. These are same-recipient, same-database failures; neither requires two people to act at once, and neither creates a second on-chain payment. All reproduction wallets, keys, payments, proofs, and broadcast results were synthetic.

SQL storage now acquires a database-backed (userId, txid) receipt claim before selecting the target transaction. A database-clock lease and owner token fence an interrupted or superseded attempt. Ownership outputs and claim completion commit together; the completed key persists to coordinate later merges. A committed header from a failed first broadcast retains a retry marker, so the next attempt rebroadcasts before crediting outputs; an unrelated existing no-send merge still follows its prior path. The migration adds only the receipt-claim table. It preserves historical transaction and output rows, including duplicates, and does not attempt financial-record repair. Existing ambiguous historical receipts remain a separate reconciliation task. IndexedDB does not use this SQL claim.

Operational boundary

This is not yet qualified for production. Local independent-connection MySQL is not a PXC multiwriter/failover exercise. Older serving pods do not honor claims, so the rollout needs an explicit mixed-version receipt-traffic plan. Back up and preflight the database, qualify staging on the deployed PXC topology, then use the protected package/image release and staging-first promotion. Historical duplicate rows should be audited and reconciled separately; the claim migration does not rewrite them. This PR does not repair earlier partial receipts or the separate Message Box acknowledgement symptom.

Impact

  • Public package source or manifest changed; affected packages are listed below
  • Security-sensitive wallet persistence boundary changed
  • Documentation and migration guidance changed

Patch candidates: @bsv/wallet-toolbox, @bsv/wallet-toolbox-client, and @bsv/wallet-toolbox-mobile 2.14.9. There is no public API, wire-format, runtime-target, or dependency-range change. SQL storage has a schema migration.

Verification

  • Local: all 41 workspace builds; pnpm health:check, pnpm lint, pnpm format:check, pnpm typecheck, and pnpm audit:security passed. The focused receipt, migration, and broadcast suites passed 39 tests. The repository's patch-coverage calculator reports 90.58% (125/138 changed line/branch points) from these focused suites alone against the 90% target.
  • Independent-connection MySQL 8.4.11 reproductions under REPEATABLE READ and READ COMMITTED now return one accepted new receipt plus one idempotent merge, one transaction, one output, and the correct 5,000-satoshi balance. During a pending negative broadcast, neither request credits an output and both reject. The old behavior was reproduced before this correction. The test database is disposable and synthetic.
  • Focused tests cover shared-storage first receipt, worker progress during result refresh, mined concurrent receipt, pending and negative broadcast, retry after failure, retry with a newly mined or already-stored proof, repeated receipt, expired-owner fencing before and after the first write, unauthorized receive, existing no-send merge, negative provider statuses, persistent completed claims, and migration with legacy duplicate rows. Three representative legacy-wallet suites passed 82 tests after the migration revision. A disposable MySQL migration test also preserved duplicate rows and enforced the receipt key.
  • Earlier first-commit local campaigns covered browser/mobile, packed consumers, property checks, affected dependents, and 6,497 conformance cases. A full local Node run passed 3,275 tests and hit one SQLite disk-I/O failure in an unrelated monitor test; the isolated monitor file then passed 14/14. Hosted CI for that first commit passed. The first concurrency head failed legacy-wallet suites because global unique indexes were incompatible with existing records; its revision passed all test lanes but measured 86.23% patch coverage. Both failures remain in CI history. Exact-head hosted CI on 0263a6566df547f7a89f07fce8a38c9ca4094ed1 is terminal and green: 36 successful checks, seven expected scope-based skips accepted by the merge gate, and 93.48% patch coverage (129/138 changed line/branch points).
  • The local MySQL tests do not prove deployed PXC multiwriter certification or failover behavior. No deployed wallet or database was written by these tests.
  • I self-reviewed the diff for correctness, security, compatibility, public API, package artifacts, performance, migration, docs, and operations
  • All applicable exact-head checks are terminal and successful, including the merge gate

Security and dependencies

  • No dependency or lockfile graph change; package-version metadata changed
  • Negative tests cover failed broadcast and duplicate-credit boundaries
  • Exact-head CodeQL has no new alert
  • Exact-head Sonar has zero new findings and unreviewed hotspots
  • No new override, advisory dismissal, quality suppression, or skipped test
  • Workflow permissions and lifecycle scripts are unchanged

Release and completion

  • No npm package, image, or deployment was published from this workstation or PR
  • Required patch versions, changelog, release notes, and migration guidance are included
  • Historical duplicate rows have an independent audit/reconciliation plan; the claim migration preserves them
  • PXC multiwriter/failover and mixed-version rollout have been qualified
  • Maintainer review, approval, and review-thread resolution are complete

Maintainer review and source merge may proceed after approval and required checks; merge does not publish a package or deploy a service. Hold package/image publication and live promotion until PXC topology and mixed-version rollout gates are complete.

@codecov

codecov Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.48936% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...s/wallet/wallet-toolbox/src/storage/StorageKnex.ts 85.41% 3 Missing and 4 partials ⚠️
...t-toolbox/src/storage/methods/internalizeAction.ts 96.66% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@BraydenLangley BraydenLangley changed the title Fix advanced wallet broadcast results and qualify concurrent receipts Coordinate concurrent wallet receipts and complete broadcast results Oct 13, 2026
@sonarqubecloud

Copy link
Copy Markdown

@BraydenLangley
BraydenLangley marked this pull request as ready for review October 13, 2026 21:00
@ty-everett
ty-everett merged commit ba479b1 into main Oct 13, 2026
44 checks passed
@ty-everett
ty-everett deleted the codex/wallet-toolbox-acceptance-status-20261013 branch October 13, 2026 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants