fix(wallet): keep exact spend metadata out of BRC100 action results - #616
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ty-everett
left a comment
There was a problem hiding this comment.
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.



Program and scope
createActioncan finish wallet work and then fail the binary BRC-100 response withInvalid 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
@bsv/wallet-toolbox,@bsv/wallet-toolbox-client,@bsv/wallet-toolbox-mobile, all 2.14.1 patch.Verification
Local validation:
pnpm build,typecheck,health:check,lint,format:check,audit:security: pass; zero high/critical advisories and zero contract/control findings.test:coverage --runInBand: 282 suites, 3,008 passing tests, one unchanged skip. Patch coverage 100% (28/28), above the 90% gate.pack:checkconsumers: pass.0c517e31265f313d04846b3380cfcde3ceac615d: CI and merge gate, CodeQL, Conformance, zero-new-Sonar findings and Codecov patch gate.Security and dependencies
0a2ebeb16df7f71c71ece7ddabbace19e98c3cd7.0c517e31265f313d04846b3380cfcde3ceac615d.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
Completion evidence