Skip to content

fix(wallet-toolbox): broadcast on internalize when the transaction is new to the user - #856

Merged
ty-everett merged 24 commits into
bsv-blockchain:mainfrom
shruggr:fix/internalize-broadcast-new-to-user
Oct 11, 2026
Merged

ty-everett merged 24 commits into
bsv-blockchain:mainfrom
shruggr:fix/internalize-broadcast-new-to-user

Conversation

@shruggr

@shruggr shruggr commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Program and scope

An unmined transaction first received by a wallet user is broadcast even when its existing shared proven-request record was created for another user or a no-send flow. Recipient outputs are persisted only after successful broadcasting; rejected broadcasts preserve the failure and do not create those outputs. Proven transactions retain their existing no-broadcast path.

Includes the preceding three merged David fixes. Exact reviewed head: 168053bf653e650f73374ce72bd0017ffaf8e272.

Impact

  • Node, browser/client and mobile wallet implementations affected; all three were qualified as the held 2.14.7 candidate and have now been published through the protected release workflow.
  • Reviewed network side effects, user ownership, existing shared request records and failure persistence. No public API, wire or database migration; active storage must be upgraded to receive the fix.
  • Changelog, three package guides, release-note/migration ledger and generated API documentation updated.

Verification

  • Node 24.18.0 / pnpm 10.33.2, frozen lifecycle-disabled installation and allowlisted build tooling.
  • Required health, lint, format, build, typechecks, documentation and wallet property tests passed.
  • Focused new-user suite 12 tests passed, including six new scenarios: new unproven input, a second user receiving ordinary/BRC-177 no-send transactions, rejected broadcasts without recipient outputs, and proven input without rebroadcast. The previous implementation fails five regressions. Mock-chain fixtures use no funded/live network.
  • The existing concurrent writer fixture now mocks successful submission and verifies the exact six payment transaction IDs; receiving those no-send payments deliberately broadcasts after this fix. No public-chain service is required by that fixture.
  • The first full local attempt failed the now-broadcasting concurrent writer fixture and a large-record latency test. After the deterministic fixture correction, the complete four-worker campaign passed; no governed exclusion, assertion or timeout was relaxed.
  • Complete wallet coverage: 292 suites / 3,177 tests passed, 7 existing skips. Client 22 and mobile 46 coverage tests passed, with unchanged governed exclusions and timeouts.
  • Local patch gate: 100.00% (4/4 changed line/branch points) against the unchanged 90% requirement.
  • All other workspace and conformance tests: passed, including 6,497 conformance tests and existing governed skips.
  • All three pack contracts passed: exports, declarations, maps, publint, strict resolution and clean consumers.
  • Browser Vite 2,228,637 raw / 533,690 gzip / 406,884 Brotli bytes, 132 modules; esbuild 1,737,552 / 483,038 / 379,034 bytes. Governed composition/budgets passed.
  • Metro 2,235,851 / 593,741 / 453,845 bytes; Hermes 4,658,000 / 1,946,618 / 1,515,326 bytes. Mobile artifact checks passed.
  • pnpm audit:security: zero high/critical advisories (3 low / 2 moderate retained). pnpm license:release-check: all package manifests, 39 incorporated-material records and docs bundle clearance passed.
  • Complete incremental diff reviewed for correctness, security, ownership, compatibility, package artifacts, dependencies, docs and operations. No generated test reports committed.
  • All hosted exact-head checks terminal and successful, including coverage, consumers, platform, conformance and aggregate/merge gates. No new skip, suppression or baseline.

The preceding IndexedDB PR passed the repository-owned 95.83% gate but its advisory external patch report counted one partially covered status condition as an uncovered line (75%). This final candidate adds a restrictive nonmatching-status assertion against both backends; the 15-test query suite passed. The weighting difference and advisory result remain explicitly recorded in #792. No gate or target was weakened.

Hosted CI 38096730256, CodeQL 38096730210 and conformance 38096730215 passed at the exact head. All 44 checks are successful or merge-gate-validated scope skips; repository and external patch checks both passed, with the owned patch gate 100.00% (4/4). Socket and Sonar zero-findings passed. The added nonmatching-status test covers every branch of the previously partial IndexedDB status condition.

Security and dependencies

  • No new dependency, override, exception or workflow/settings change.
  • Rejected-broadcast, shared-request, proven-input and second-user boundary regressions tested.
  • Exact-head CodeQL has no new alert; 0 open merge-ref alerts.
  • Exact-head Sonar has zero new findings and zero unreviewed hotspots.

Release and operations

  • All affected packed packages included in the held 2.14.7 candidate; no publication claimed by source merge.
  • Network behavior documented: accepting previously known unmined input can now broadcast it. Upgrade active storage as well as clients; no schema migration.
  • Publication completed through the protected, filtered npm OIDC workflow after all four qualified PRs merged. No workstation publication or unrelated service deployment.

Completion evidence

  • Final-head required checks and review conversations complete; 0 threads.
  • Normal protected merge verified: a2a6058, 2026-10-11T00:07:04Z. Remote main tree exactly equals the qualified candidate.

Original implementation explanation

newInternalize only broadcasts if (pr.isNew), but StorageProvider.getProvenOrReq never sets isNew, so internalizeAction never broadcasts. The outputs are stored as spendable whether or not the transaction reaches the network.

On a storage with one user, Monitor later sends the unsent req. On a storage shared by several users the req is one row per txid. When user B internalizes a noSend transaction that user A created on the same storage (for example a BRC-177 protected send), upsertProvenTxReq merges into A's nosend req and keeps its status. Nothing sends it, and B can spend outputs of a transaction that never reaches the network.

newInternalize only runs when the internalizing user has no transaction record for the txid, so the branch now runs whenever the BEEF has no proof for the transaction (pr.proven == null). On a failed broadcast the existing branch restores spent inputs and fails with WERR_REVIEW_ACTIONS without storing outputs.

Tests in test/wallet/action/internalizeAction.test.ts:

  • Unproven BEEF: postBeef is called and the req leaves unsent.
  • Shared storage, plain noSend and BRC-177 protected noSend: B's internalize broadcasts A's transaction, the req leaves nosend, and B's output is stored.
  • Same, with a rejected broadcast: B's internalize throws WERR_REVIEW_ACTIONS and stores no outputs.
  • BEEF with a merkle proof: no broadcast.

Five of the six new cases fail without the fix. test/storage + test/Wallet/action: 593 passed, 0 failed, apart from StorageKnexMigrationAtomicity, which needs a built out/. Lint, prettier and typecheck are clean.

Combined wallet publication milestone

All four scoped David PRs (#775, #774, #792, #856) are merged. Exact merged release source: a2a6058. Main CI 38097333329 and CodeQL 38097333297 passed; the six open main scanning records predate this work and were not changed or dismissed.

Protected Release 38097876026 completed successfully at that exact source with mode=single and filter=@bsv/wallet-toolbox*. The existing release-approval gate was approved for this authorized scope. Full canonical mutation checks, preparation, security, artifact and provenance gates passed. Publication and the complete post-release verification job passed. No repository security setting changed.

Published packages, each confirmed on npm with latest=2.14.7:

Independent reconciliation downloaded registry tarballs and matched their SHA-256 and SHA-512 integrity to the immutable release manifest. Registry signatures verified, npm provenance names source a2a6058361705cb02a30a86a6e07186a8c465d58 and this exact workflow invocation, and all six GitHub SLSA/CycloneDX attestations verified against that source and signer workflow. Separate lifecycle-disabled registry installs passed CommonJS and ESM import smoke tests and npm audit signatures for each package. The protected post-release job also verified every published workspace package and source-versus-registry reconciliation.

The combined package release is complete. No identified live wallet storage/service application was deployed; operators must upgrade active storage and clients to 2.14.7 to receive the relevant changes. Single-package release mode intentionally did not perform unrelated infrastructure publication or a repository-wide version cascade.

shruggr and others added 7 commits October 3, 2026 14:24
…ry JSON carries it

binaryRequests encodes only Uint8Array values, but the toolbox built its
largest storage payloads as number[]: StorageClientBase.createAction set
inputBEEF from pruned.toBinary(), and the signer's internalizeAction passed
ab.toBinaryAtomic(txid). They travelled as JSON number arrays even with
binary negotiated, and a BEEF over ~100 KB exceeded the auth middleware's
100,000-value request limit.

Use the SDK's toUint8Array() / toUint8ArrayAtomic() for createAction and
no-send-expiry inputBEEF and internalizeAction AtomicBEEF, and return
listOutputs BEEF, createAction sourceTransaction and competingBeef as
Uint8Array from Knex storage. BEEF and AtomicBEEF already allow Uint8Array,
and the server's byte validation accepts it. Legacy peers that have not
negotiated binary still receive number arrays.
BRC-177 Prefunding step 2 requires the anchor to be large enough both to
fund the protected action and to be reclaimed, net of a reclaim fee. The
anchor was sized for the protected action only, so value-neutral actions
(e.g. a 1-sat token transfer) were refused because the anchor could not
pay for its own reclaim.

Size the anchor as the larger of the protected action's need and the
reclaim floor (reclaim fee plus minimum reclaim output). The protected
action carries no change, so the funding planner now runs with a zero
change cap and pays any surplus as fee. generateChangeSdk accepts
maxChangeOutputs: 0 to mean "create no change".

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R53QzSzGv4LkfmxBd4URiD
Replace the "maxChangeOutputs: 0 means no change" overload with a
dedicated GenerateChangeSdkParams.surplusToFee flag. When set, the
planner creates no change output and pays any surplus beyond the
required fee as fee. maxChangeOutputs keeps its original rule (minimum
1, default 8); the BRC-177 protected branch goes back to a cap of 1 and
sets surplusToFee. The protected funding plan is the only setter.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R53QzSzGv4LkfmxBd4URiD
Compute the change-output cap as before, then zero it when surplusToFee
is set. No behaviour change (SonarCloud typescript:S3358).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R53QzSzGv4LkfmxBd4URiD
… JSON

StorageServer converts the byte columns of findOutputsAuth and
findProvenTxReqs results when the client negotiated binary JSON, as it
already does for getSyncChunk. Clients convert them back to number[] in
validateEntities.

Restore competingBeef to number[] in createAction's WERR_REVIEW_ACTIONS.
Thrown errors are serialized with stringifyBRC100, so it never reached
the binary codec.
filterCertificates (certifiers, types), filterOutputTagMaps (tagIds) and
filterTransactions (status) rejected every row when the array was empty.
StorageKnex and the other StorageIdb filters skip an empty array. Guard
the four checks with `.length > 0`.

Fixes bsv-blockchain#791

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R53QzSzGv4LkfmxBd4URiD
… new to the user

newInternalize gated its broadcast on pr.isNew, which getProvenOrReq never
sets, so internalizeAction never broadcast. On shared storage a noSend req
created by another user kept its nosend status and was never sent, while the
internalizing user's outputs were stored as spendable. Broadcast whenever the
BEEF has no proof for the transaction.
@shruggr
shruggr marked this pull request as ready for review October 6, 2026 06:34
shruggr added a commit to shruggr/ts-stack that referenced this pull request Oct 6, 2026
Adds bsv-blockchain#856 (internalize broadcasts transactions new to the user) to the interim repack. Not for upstream.
@ty-everett
ty-everett marked this pull request as draft October 10, 2026 23:54
@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecov Bot commented Oct 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@ty-everett
ty-everett marked this pull request as ready for review October 11, 2026 00:06
@ty-everett
ty-everett merged commit a2a6058 into bsv-blockchain:main Oct 11, 2026
44 checks passed
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