From e37618af70dd75e4ddd97768c03420e7914af3fe Mon Sep 17 00:00:00 2001 From: Otto Allmendinger Date: Mon, 16 Feb 2026 17:52:06 +0100 Subject: [PATCH] feat(abstract-utxo): add customChangeXpubs to explainTransaction Add support for custom change wallet functionality in explainTransaction and parseTransaction methods. This allows Bitcoin wallets to send change to a different wallet by providing customChangeXpubs. Includes validation for custom change key signatures in the parsing flow to ensure the change wallet keys were properly authorized by the sender. Added tests for signature verification and rejection of invalid signatures. Issue: BTC-2768 Co-authored-by: llm-git --- modules/abstract-utxo/src/abstractUtxoCoin.ts | 1 + .../src/transaction/explainTransaction.ts | 4 +- .../fixedScript/parseTransaction.ts | 33 ++++++++- .../test/unit/customChangeWallet.ts | 64 +++++++++++++++++ .../test/unit/customChangeWalletFixtures.ts | 68 +++++++++++++++++++ 5 files changed, 167 insertions(+), 3 deletions(-) create mode 100644 modules/abstract-utxo/test/unit/customChangeWalletFixtures.ts diff --git a/modules/abstract-utxo/src/abstractUtxoCoin.ts b/modules/abstract-utxo/src/abstractUtxoCoin.ts index 2c650acbe5..133229ffb8 100644 --- a/modules/abstract-utxo/src/abstractUtxoCoin.ts +++ b/modules/abstract-utxo/src/abstractUtxoCoin.ts @@ -243,6 +243,7 @@ export interface ExplainTransactionOptions; feeInfo?: string; pubs?: Triple; + customChangeXpubs?: Triple; decodeWith?: SdkBackend; } diff --git a/modules/abstract-utxo/src/transaction/explainTransaction.ts b/modules/abstract-utxo/src/transaction/explainTransaction.ts index 9ebe143ae4..8546431f25 100644 --- a/modules/abstract-utxo/src/transaction/explainTransaction.ts +++ b/modules/abstract-utxo/src/transaction/explainTransaction.ts @@ -27,6 +27,7 @@ export function explainTx( params: { wallet?: IWallet; pubs?: string[]; + customChangeXpubs?: Triple; txInfo?: { unspents?: Unspent[] }; changeInfo?: fixedScript.ChangeAddressInfo[]; }, @@ -49,7 +50,7 @@ export function explainTx( throw new Error('legacy transactions are not supported for descriptor wallets'); } if (tx instanceof utxolib.bitgo.UtxoPsbt) { - return fixedScript.explainPsbt(tx, params, coinName); + return fixedScript.explainPsbt(tx, { ...params, customChangePubs: params.customChangeXpubs }, coinName); } else if (tx instanceof fixedScriptWallet.BitGoPsbt) { const pubs = params.pubs; if (!pubs) { @@ -68,6 +69,7 @@ export function explainTx( replayProtection: { publicKeys: getReplayProtectionPubkeys(coinName), }, + customChangeWalletXpubs: params.customChangeXpubs, }); } else { return fixedScript.explainLegacyTx(tx, params, coinName); diff --git a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts index 7d6e57bccc..bfb78b729f 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts @@ -1,11 +1,19 @@ import assert from 'assert'; import _ from 'lodash'; -import { ITransactionRecipient, Triple, VerificationOptions, Wallet } from '@bitgo/sdk-core'; +import { ITransactionRecipient, KeyIndices, Triple, VerificationOptions, Wallet } from '@bitgo/sdk-core'; import type { AbstractUtxoCoin, ParseTransactionOptions } from '../../abstractUtxoCoin'; import type { FixedScriptWalletOutput, Output, ParsedTransaction } from '../types'; -import { fetchKeychains, getKeySignatures, toKeychainTriple, UtxoKeychain, UtxoNamedKeychains } from '../../keychains'; +import { + fetchKeychains, + getKeySignatures, + toKeychainTriple, + toXpubTriple, + UtxoKeychain, + UtxoNamedKeychains, +} from '../../keychains'; +import { verifyKeySignature } from '../../verifyKey'; import { assertValidTransactionRecipient, fromExtendedAddressFormatToScript, @@ -112,6 +120,20 @@ function toExpectedOutputs( return expectedOutputs; } +function verifyCustomChangeKeys(userKeychain: UtxoKeychain, customChange: CustomChangeOptions): void { + for (const keyIndex of [KeyIndices.USER, KeyIndices.BACKUP, KeyIndices.BITGO]) { + if ( + !verifyKeySignature({ + userKeychain, + keychainToVerify: customChange.keys[keyIndex], + keySignature: customChange.signatures[keyIndex], + }) + ) { + throw new Error(`failed to verify custom change ${KeyIndices[keyIndex].toLowerCase()} key signature`); + } + } +} + export async function parseTransaction( coin: AbstractUtxoCoin, params: ParseTransactionOptions @@ -177,11 +199,18 @@ export async function parseTransaction( } } + let customChangeXpubs: Triple | undefined; + if (customChange) { + verifyCustomChangeKeys(keychainArray[KeyIndices.USER], customChange); + customChangeXpubs = toXpubTriple(customChange.keys); + } + // obtain all outputs const explanation: TransactionExplanation = await coin.explainTransaction({ txHex: txPrebuild.txHex, txInfo: txPrebuild.txInfo, pubs: keychainArray.map((k) => k.pub) as Triple, + customChangeXpubs, }); const allOutputs = [...explanation.outputs, ...explanation.changeOutputs]; diff --git a/modules/abstract-utxo/test/unit/customChangeWallet.ts b/modules/abstract-utxo/test/unit/customChangeWallet.ts index e6a6ca4761..1b353c4b68 100644 --- a/modules/abstract-utxo/test/unit/customChangeWallet.ts +++ b/modules/abstract-utxo/test/unit/customChangeWallet.ts @@ -1,3 +1,5 @@ +import assert from 'assert'; + import should = require('should'); import * as sinon from 'sinon'; import { Wallet } from '@bitgo/sdk-core'; @@ -133,4 +135,66 @@ describe('Custom Change Wallets', () => { (coin.wallets as any).restore(); (coin.keychains as any).restore(); }); + + it('should reject invalid custom change key signatures before calling explainTransaction', async () => { + const wrongKey = coin.keychains().create(); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const sign = async ({ key }) => (await coin.signMessage({ prv: wrongKey.prv }, key.pub!)).toString('hex'); + const invalidSignatures = { + user: await sign(keys.change.user), + backup: await sign(keys.change.backup), + bitgo: await sign(keys.change.bitgo), + }; + + const signedSendingWallet = sinon.createStubInstance(Wallet, stubData.signedSendingWallet as any); + const changeWallet = sinon.createStubInstance(Wallet, stubData.changeWallet as any); + + sinon.stub(coin, 'keychains').returns({ + get: sinon.stub().callsFake(({ id }) => { + switch (id) { + case keys.send.user.id: + return Promise.resolve({ id, ...keys.send.user.key }); + case keys.send.backup.id: + return Promise.resolve({ id, ...keys.send.backup.key }); + case keys.send.bitgo.id: + return Promise.resolve({ id, ...keys.send.bitgo.key }); + case keys.change.user.id: + return Promise.resolve({ id, ...keys.change.user.key }); + case keys.change.backup.id: + return Promise.resolve({ id, ...keys.change.backup.key }); + case keys.change.bitgo.id: + return Promise.resolve({ id, ...keys.change.bitgo.key }); + } + }), + } as any); + + sinon.stub(coin, 'wallets').returns({ + get: sinon.stub().callsFake(() => Promise.resolve(changeWallet)), + } as any); + + const explainStub = sinon.stub(coin, 'explainTransaction'); + + signedSendingWallet._wallet = signedSendingWallet._wallet || { + customChangeKeySignatures: invalidSignatures, + }; + + try { + await coin.parseTransaction({ + txParams: { recipients: [] }, + txPrebuild: { txHex: '' }, + wallet: signedSendingWallet as any, + verification: {}, + }); + assert.fail('parseTransaction should have thrown for invalid custom change key signatures'); + } catch (e) { + assert.ok(e instanceof Error); + assert.match(e.message, /failed to verify custom change .* key signature/); + } + + assert.strictEqual(explainStub.called, false, 'explainTransaction should not have been called'); + + explainStub.restore(); + (coin.wallets as any).restore(); + (coin.keychains as any).restore(); + }); }); diff --git a/modules/abstract-utxo/test/unit/customChangeWalletFixtures.ts b/modules/abstract-utxo/test/unit/customChangeWalletFixtures.ts new file mode 100644 index 0000000000..6e6729a3ea --- /dev/null +++ b/modules/abstract-utxo/test/unit/customChangeWalletFixtures.ts @@ -0,0 +1,68 @@ +import assert from 'assert'; + +import { BIP32, message } from '@bitgo/wasm-utxo'; + +import { verifyKeySignature } from '../../src/verifyKey'; + +const KEY_ROLES = ['user', 'backup', 'bitgo'] as const; +type KeyRole = (typeof KEY_ROLES)[number]; + +function signKeySignature(signerPrivateKey: BIP32, pubToSign: string): string { + assert(signerPrivateKey.privateKey, 'signer must have a private key'); + return Buffer.from(message.signMessage(pubToSign, signerPrivateKey.privateKey)).toString('hex'); +} + +function createMockCustomChangeKeySignatures( + signerPrivateKey: BIP32, + changeKeychains: { pub: string }[] +): Record { + return { + user: signKeySignature(signerPrivateKey, changeKeychains[0].pub), + backup: signKeySignature(signerPrivateKey, changeKeychains[1].pub), + bitgo: signKeySignature(signerPrivateKey, changeKeychains[2].pub), + }; +} + +describe('Custom Change Wallet Key Signatures', function () { + const mainUserKey = BIP32.fromSeedSha256('main-user'); + const changeKeys = [ + BIP32.fromSeedSha256('change-user'), + BIP32.fromSeedSha256('change-backup'), + BIP32.fromSeedSha256('change-bitgo'), + ]; + const changeKeychains = changeKeys.map((k) => ({ pub: k.neutered().toBase58() })); + + it('should accept signatures from the correct key', function () { + const signatures = createMockCustomChangeKeySignatures(mainUserKey, changeKeychains); + + for (const role of KEY_ROLES) { + const index = KEY_ROLES.indexOf(role); + assert.ok( + verifyKeySignature({ + userKeychain: { pub: mainUserKey.neutered().toBase58() }, + keychainToVerify: changeKeychains[index], + keySignature: signatures[role], + }), + `failed to verify mock custom change ${role} key signature` + ); + } + }); + + it('should reject signatures from a different key', function () { + const wrongKey = BIP32.fromSeedSha256('wrong-key'); + const badSignatures = createMockCustomChangeKeySignatures(wrongKey, changeKeychains); + + for (const role of KEY_ROLES) { + const index = KEY_ROLES.indexOf(role); + assert.strictEqual( + verifyKeySignature({ + userKeychain: { pub: mainUserKey.neutered().toBase58() }, + keychainToVerify: changeKeychains[index], + keySignature: badSignatures[role], + }), + false, + `should have rejected custom change ${role} key signature from wrong key` + ); + } + }); +});