Merge pull request #274 from ty-everett/codex/auto-change-policy

fix(wallet): enforce managed BRC-29 change policy

Author: ty-everett

packages/sdk/CHANGELOG.md

@@ -224,6 +224,12 @@ All notable changes to this project will be documented in this file. The format
 
 ### Fixed
 
+- Release prep for `2.1.7`: fix `Brc29RemittanceModule` settlement acceptance to always use the
+  `wallet payment` internalization protocol. The deprecated
+  `internalizeProtocol: 'basket insertion'` configuration now fails fast with
+  recovery guidance instead of constructing an invalid remittance and
+  misclassifying recipient funds as custom outputs.
+
 ### Security
 
 ---

packages/sdk/docs/reference/remittance.md

@@ -193,7 +193,9 @@ description?: string
 
 #### Property internalizeProtocol
 
-How wallet internalizes the payment.
+Deprecated. BRC-29 settlements must be internalized as `wallet payment` so the
+recipient can verify and manage the derived output. The module rejects
+`basket insertion`; it is reserved for application-managed custom outputs.
 
 ```ts
 internalizeProtocol?: "wallet payment" | "basket insertion"

packages/sdk/docs/remittance-getting-started.md

@@ -98,6 +98,12 @@ await manager.init()
   await testSyncThreads(manager)
 ```
 
+`Brc29RemittanceModule` internalizes a received settlement as a `wallet payment`.
+That classification is required: the wallet verifies the BRC-29 derivation and
+records the output as spendable managed change in its default balance. Do not
+use `basket insertion` for BRC-29 settlement funds; basket insertion is reserved
+for application-managed custom outputs and cannot target the default basket.
+
 ## Event Hooks
 
 Use `onEvent` or the per-event `events` callbacks to react to each step.

packages/sdk/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@bsv/sdk",
-  "version": "2.1.6",
+  "version": "2.1.7",
   "type": "module",
   "description": "BSV Blockchain Software Development Kit",
   "main": "dist/cjs/mod.js",

packages/sdk/src/remittance/__tests/BasicBRC29.additional.test.ts

@@ -570,22 +570,9 @@ describe('Brc29RemittanceModule – acceptSettlement validation', () => {
     )
   })
 
-  it('uses basket insertion internalizeProtocol when configured', async () => {
-    const internalizeAction = jest.fn(async () => ({ accepted: true as const }))
-    const wallet = makeWallet({ internalizeAction })
-    const module = new Brc29RemittanceModule({ internalizeProtocol: 'basket insertion' })
-
-    const result = await module.acceptSettlement(
-      { threadId: 'tid', settlement: { ...validSettlement }, sender: 'sender-key' },
-      makeContext(wallet)
-    )
-    expect(result.action).toBe('accept')
-    expect(internalizeAction).toHaveBeenCalledWith(
-      expect.objectContaining({
-        outputs: [expect.objectContaining({ protocol: 'basket insertion' })]
-      }),
-      'example.com'
-    )
+  it('rejects the unsafe basket insertion internalizeProtocol', () => {
+    expect(() => new Brc29RemittanceModule({ internalizeProtocol: 'basket insertion' }))
+      .toThrow('BRC-29 settlements cannot be internalized as basket insertions')
   })
 })
 
@@ -618,7 +605,7 @@ describe('Brc29RemittanceModule – constructor defaults', () => {
       outputDescription: 'out-desc',
       refundFeeSatoshis: 500,
       minRefundSatoshis: 200,
-      internalizeProtocol: 'basket insertion',
+      internalizeProtocol: 'wallet payment',
       nonceProvider: customNonce,
       lockingScriptProvider: customScript
     })
@@ -628,7 +615,7 @@ describe('Brc29RemittanceModule – constructor defaults', () => {
     expect((module as any).outputDescription).toBe('out-desc')
     expect((module as any).refundFeeSatoshis).toBe(500)
     expect((module as any).minRefundSatoshis).toBe(200)
-    expect((module as any).internalizeProtocol).toBe('basket insertion')
+    expect((module as any).internalizeProtocol).toBe('wallet payment')
     expect((module as any).nonceProvider).toBe(customNonce)
     expect((module as any).lockingScriptProvider).toBe(customScript)
   })

packages/sdk/src/remittance/modules/BasicBRC29.ts

@@ -112,7 +112,15 @@ export interface Brc29RemittanceModuleConfig {
    */
   minRefundSatoshis?: number
 
-  /** How wallet internalizes the payment. */
+  /**
+   * @deprecated BRC-29 settlements must be internalized as `wallet payment`.
+   * `basket insertion` never supplied the required insertion remittance and,
+   * more importantly, would classify recipient funds as custom application
+   * outputs that the wallet's automatic BRC-29 signer cannot spend.
+   *
+   * The property remains in the input type for source compatibility, but the
+   * constructor rejects `basket insertion` with an actionable error.
+   */
   internalizeProtocol?: 'wallet payment' | 'basket insertion'
 
   nonceProvider?: NonceProvider
@@ -138,7 +146,7 @@ export class Brc29RemittanceModule
   private readonly outputDescription: string
   private readonly refundFeeSatoshis: number
   private readonly minRefundSatoshis: number
-  private readonly internalizeProtocol: 'wallet payment' | 'basket insertion'
+  private readonly internalizeProtocol: 'wallet payment'
   private readonly nonceProvider: NonceProvider
   private readonly lockingScriptProvider: LockingScriptProvider
 
@@ -150,7 +158,13 @@ export class Brc29RemittanceModule
     this.outputDescription = cfg.outputDescription ?? 'Payment for remittance invoice'
     this.refundFeeSatoshis = cfg.refundFeeSatoshis ?? 1000
     this.minRefundSatoshis = cfg.minRefundSatoshis ?? 1000
-    this.internalizeProtocol = cfg.internalizeProtocol ?? 'wallet payment'
+    if (cfg.internalizeProtocol === 'basket insertion') {
+      throw new TypeError(
+        'BRC-29 settlements cannot be internalized as basket insertions. ' +
+        'Use wallet payment for spendable wallet balance, or implement a separate custom-output protocol with insertionRemittance.'
+      )
+    }
+    this.internalizeProtocol = 'wallet payment'
     this.nonceProvider = cfg.nonceProvider ?? DefaultNonceProvider
     this.lockingScriptProvider = cfg.lockingScriptProvider ?? DefaultLockingScriptProvider
   }
@@ -253,11 +267,9 @@ export class Brc29RemittanceModule
   ): Promise<{ action: 'accept'; receiptData?: Brc29ReceiptData } | { action: 'terminate'; termination: Termination }> {
     const { wallet, originator } = ctx
     const origin = originator as OriginatorDomainNameStringUnder250Bytes | undefined
-    console.log('acceptSettlement', args)
     try {
       const settlement = ensureValidSettlement(args.settlement)
       const outputIndex = settlement.outputIndex ?? 0
-      debugger
       const internalizeResult = await wallet.internalizeAction(
         {
           tx: settlement.transaction,

packages/wallet/wallet-toolbox/CHANGELOG.md

@@ -6,6 +6,15 @@ attention to changes that materially alter behavior or extend functionality.
 
 ## wallet-toolbox (unreleased)
 
+- Release prep for `2.4.1`: define one managed-change policy across Knex and IndexedDB allocation,
+  counting, default balance reporting, `balanceAndUtxos`, and `noSendChange`.
+  Only complete BRC-29 signer metadata is eligible; custom rows remain visible
+  through raw default-basket listing for recovery. `internalizeAction` now
+  rejects basket insertion into `default`, prevents managed change from being
+  reclassified, promotes verified legacy BRC-29 rows through `wallet payment`,
+  and permits incompatible custom rows to be swept to non-default baskets
+  without changing wallet balance.
+
 - Release prep for `2.4.0`: security fix for `GHSA-36f9-7rg5-cpf8`. `buildSignableTransaction` now verifies that storage-returned outputs match the caller's requested `args.outputs` before signing (rejects substituted/altered/reclassified/missing outputs), and rejects any unrequested output that isn't client-derived change or a single commission output bounded by `MAX_STORAGE_COMMISSION_SATOSHIS`. `WalletPermissionsManager.createAction` independently verifies requested outputs are present in the signable transaction before authorizing, as defense-in-depth. Protects against a malicious or compromised `StorageClient` operator substituting or injecting outputs that would be signed and broadcast without the caller's knowledge.
 
 - Release prep for `2.3.3`: add Arcade-first wallet service wiring, go-chaintracks

packages/wallet/wallet-toolbox/README.md

@@ -75,6 +75,10 @@ const result = await wallet.createAction({
 
 [Full API documentation](https://bsv-blockchain.github.io/wallet-toolbox) is available on GitHub Pages.
 
+See [Managed change, sweeping, and recovery](./docs/managed-change-policy.md)
+for the default-basket invariant, automatic funding policy, and supported
+`internalizeAction` repair paths.
+
 The codebase has detailed JSDoc annotations throughout — these will surface inline in editors like VS Code.
 
 ## Development

packages/wallet/wallet-toolbox/client/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@bsv/wallet-toolbox-client",
-  "version": "2.4.0",
+  "version": "2.4.1",
   "description": "Client only Wallet Storage",
   "main": "./out/src/index.client.js",
   "types": "./out/src/index.client.d.ts",

packages/wallet/wallet-toolbox/docs/client.md

@@ -17944,12 +17944,12 @@ When the transaction already exists, the description is updated. The isOutgoing
 
 "basket insertion" Merge Rules:
 1. The "default" basket may not be specified as the insertion basket.
-2. A change output in the "default" basket may not be target of an insertion into a different basket.
-3. These baskets do not affect the wallet's balance and are typed "custom".
+2. Managed change may not be reclassified as a basket insertion.
+3. Basket insertions do not affect wallet balance and are typed "custom".
 
 "wallet payment" Merge Rules:
-1. Targetting an existing change "default" basket output results in a no-op. No error. No alterations made.
-2. Targetting a previously "custom" non-change output converts it into a change output. This alters the transaction's `amount`, and the wallet balance.
+1. Targeting existing managed change in the default basket is idempotent.
+2. Targeting a custom output promotes it to managed BRC-29 change only after the locking script is verified. This is the supported recovery path for a misclassified BRC-29 payment.
 
 ```ts
 export async function internalizeAction(wallet: Wallet, auth: AuthId, args: InternalizeActionArgs): Promise<StorageInternalizeActionResult> 
@@ -17980,12 +17980,12 @@ When the transaction already exists, the description is updated. The isOutgoing
 
 "basket insertion" Merge Rules:
 1. The "default" basket may not be specified as the insertion basket.
-2. A change output in the "default" basket may not be target of an insertion into a different basket.
-3. These baskets do not affect the wallet's balance and are typed "custom".
+2. Managed change may not be reclassified as a basket insertion.
+3. Basket insertions do not affect wallet balance and are typed "custom".
 
 "wallet payment" Merge Rules:
-1. Targetting an existing change "default" basket output results in a no-op. No error. No alterations made.
-2. Targetting a previously "custom" non-change output converts it into a change output. This alters the transaction's `satoshis`, and the wallet balance.
+1. Targeting existing managed change in the default basket is idempotent.
+2. Targeting a custom output promotes it to managed BRC-29 change and increases wallet balance. This includes recovery of a legacy custom row incorrectly stored in the default basket.
 
 ```ts
 export async function internalizeAction(storage: StorageProvider, auth: AuthId, args: InternalizeActionArgs): Promise<StorageInternalizeActionResult>