Skip to content

fix(wallet): keep exact spend metadata out of BRC100 action results - #616

Merged
ty-everett merged 1 commit into
mainfrom
codex/create-action-public-result-hotfix
Sep 25, 2026
Merged

ty-everett merged 1 commit into
mainfrom
codex/create-action-public-result-hotfix

Conversation

@ty-everett

@ty-everett ty-everett commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Program and scope

createAction can finish wallet work and then fail the binary BRC-100 response with Invalid createAction result object key: expected a string. The signer placed its internal exact-spend Symbol on the public result, which the SDK correctly rejects.

Keep exact-spend metadata in a shared local WeakMap, transfer it across permission-module result transforms, and consume the legacy local carrier before returning permission-managed results. This preserves service-charge authorization and strict SDK validation. Covers completed actions, actions awaiting signatures, separately loaded bundles, legacy local results, and real BRC-177 wallet results. Coordination: BotBoard #615, diagnosis: #613.

Unrelated #569 work, SDK validation changes, infrastructure deployments and live transaction retries are outside this PR. No BRC-100 app, wire, persisted-data, BRC-39 or account-recovery migration is needed. Older hosts can have created a transaction despite returning an error; reconcile history before retrying.

Impact

  • Public package source or manifest changed: @bsv/wallet-toolbox, @bsv/wallet-toolbox-client, @bsv/wallet-toolbox-mobile, all 2.14.1 patch.
  • Security-sensitive boundary reviewed: exact spending authorization remains intact; no SDK check is weakened.
  • Documentation, changelog, generated facts and migration guidance updated.
  • Existing exports and peer dependencies retained. No new dependency, override, suppression or skipped test.

Verification

Local validation:

  • Workspace pnpm build, typecheck, health:check, lint, format:check, audit:security: pass; zero high/critical advisories and zero contract/control findings.
  • Core test:coverage --runInBand: 282 suites, 3,008 passing tests, one unchanged skip. Patch coverage 100% (28/28), above the 90% gate.
  • Focused regression/integration run: 112 passing tests. The new binary-result tests first reproduced the original failure before the implementation changed.
  • Client 22 tests, mobile 46 tests, all three pack:check consumers: pass.
  • Packed browser/Vite consumer and React Native Metro/Hermes contract: pass on Node 24.18.0 / pnpm 10.33.2. Mobile bundle remains within existing budgets (Metro Brotli 458,561 bytes; Hermes Brotli 1,503,137 bytes).
  • Executable portable conformance: 6,491 passing cases, 211 unchanged governed skips. Eight documentation examples compile against 21 exact package tarballs.
  • Published SDK client compatibility: 48 passing binary createAction cases across SDK 2.4.0, 2.5.0, 2.7.1, 2.8.1, 2.8.2 and 2.8.6; completed/partial actions, response transforms and compiled core/client or legacy metadata all preserve exact response bytes and the 1,200-sat synthetic approval including the 100-sat service charge.
  • Real transaction fixtures and actual wallet/permission-manager paths are exercised offline; no live funds or account state are touched.
  • Self-reviewed correctness, spending authorization, compatibility, exports, artifacts, documentation and operational impact.
  • All hosted checks are terminal and successful on exact head 0c517e31265f313d04846b3380cfcde3ceac615d: CI and merge gate, CodeQL, Conformance, zero-new-Sonar findings and Codecov patch gate.

Security and dependencies

  • No dependency or lockfile change beyond the three package versions.
  • Legacy metadata compatibility and service-charge accounting tested; output verification and denial behavior remain enforced.
  • No new override, advisory dismissal, quality suppression or skipped test.
  • Exact-head CodeQL has no new alert: both Actions and JavaScript/TypeScript analyses have zero results on merge revision 0a2ebeb16df7f71c71ece7ddabbace19e98c3cd7.
  • Repository quality gate reports zero new Sonar findings and zero unreviewed hotspots for 0c517e31265f313d04846b3380cfcde3ceac615d.
  • Workflow permissions and lifecycle-script policy are unchanged.

Dependency evidence

This is a compatibility correction to existing published behavior. The package roots, SDK peer floor, wire encodings and storage schemas are unchanged. The shared WeakMap also preserves accounting when core/client/mobile modules are loaded separately in the same realm. Hosts should upgrade the wallet and permission manager together. Existing JSON bridges that remove internal metadata are unaffected by this binary response defect.

Release and operations

  • No npm publication from a workstation or this PR.
  • Three patch bumps, published 2.14.0 baselines, changelog, README and generated migration notes are included.
  • Protected release workflow must publish immutable 2.14.1 artifacts after merge and successful main checks. Consumer wallets then pin verified registry artifacts; this PR does not deploy them.
  • No database migration or infrastructure image rollout is required. Retain existing wallet data and reconcile ambiguous prior actions before retrying.

Completion evidence

  • Local implementation and hosted validation are complete; merge and publication remain pending.
  • No review threads; exact head and base verified immediately before merge.
  • Source merge, protected npm publication and consumer rollout verified independently.
  • One qualified maintainer approval is sufficient under repository policy; no last-pusher restriction is assumed.

@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecov Bot commented Sep 25, 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 September 25, 2026 02:58

@ty-everett ty-everett left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Maintainer self-review of 0c517e31265f313d04846b3380cfcde3ceac615d:

  • The internal metadata is removed at its producer instead of relaxing the SDK's public-result validator. The shared WeakMap preserves exact service-charge accounting across separately loaded core/client/mobile modules in the same realm. Permission response transforms explicitly carry the amount forward; the legacy carrier is consumed without leaking it to the public response.
  • Existing root/deep exports, SDK peer floors, BRC100 bytes, persisted schemas, BRC39 files and account recovery contracts are retained. The legacy metadata shape is supported for older local producers; upgrade the host and permission manager together.
  • Offline real-wallet and permission-manager regressions cover ordinary/two-step/BRC177 results. The compiled core/client matrix passes 48 cases against published SDK clients from 2.4.0 through 2.8.6, with exact wire bytes and full service-charge accounting preserved. Denied actions remain aborted before signing.
  • Full local core coverage: 282 suites/3,008 tests, one unchanged skip; 100% changed-line/branch coverage. Client/mobile tests, all packed consumers, browser/Metro/Hermes, executable conformance, workspace controls and documentation examples pass. No live funds were used.
  • Source CI, CodeQL, the exact-head zero-Sonar-findings gate and review-thread state are checked separately before merge. Publication is restricted to the protected workflow and the three 2.14.1 packages, followed by immutable registry verification and consumer validation.

No correctness or compatibility issue remains in this reviewed diff. Historical failing responses can correspond to completed transactions; operator guidance therefore requires reconciliation before retrying.

Final remote evidence: CI run 36087036429, including merge-gate, completed successfully. CodeQL run 36087036426 and Conformance run 36087036415 succeeded. Zero-new-Sonar-findings and Codecov patch gates passed. Re-read PR head 0c517e31265f313d04846b3380cfcde3ceac615d, base 231eaceef7fb9a3d494a3b23bffb3b01218e9493, and no review threads. This is an explicit maintainer self-review, not an independent second reviewer.

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.

1 participant