diff --git a/modules/abstract-utxo/src/transaction/fixedScript/explainPsbtWasm.ts b/modules/abstract-utxo/src/transaction/fixedScript/explainPsbtWasm.ts index 1183197d7d..206f45c1d7 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/explainPsbtWasm.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/explainPsbtWasm.ts @@ -40,13 +40,13 @@ function toExternalOutput(output: ParsedExternalOutput): Output { export function explainPsbtWasm( psbt: fixedScriptWallet.BitGoPsbt, - walletXpubs: Triple, + walletXpubs: Triple | fixedScriptWallet.RootWalletKeys, params: { replayProtection: { checkSignature?: boolean; publicKeys: Buffer[]; }; - customChangeWalletXpubs?: Triple; + customChangeWalletXpubs?: Triple | fixedScriptWallet.RootWalletKeys; } ): TransactionExplanationWasm { const parsed = psbt.parseTransactionWithWalletKeys(walletXpubs, { replayProtection: params.replayProtection }); diff --git a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts index bfb78b729f..f5ff447050 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts @@ -209,6 +209,7 @@ export async function parseTransaction( const explanation: TransactionExplanation = await coin.explainTransaction({ txHex: txPrebuild.txHex, txInfo: txPrebuild.txInfo, + decodeWith: txPrebuild.decodeWith, pubs: keychainArray.map((k) => k.pub) as Triple, customChangeXpubs, }); diff --git a/modules/abstract-utxo/test/unit/customChangeWallet.ts b/modules/abstract-utxo/test/unit/customChangeWallet.ts index 1b353c4b68..a67cc301d3 100644 --- a/modules/abstract-utxo/test/unit/customChangeWallet.ts +++ b/modules/abstract-utxo/test/unit/customChangeWallet.ts @@ -1,200 +1,241 @@ -import assert from 'assert'; - -import should = require('should'); -import * as sinon from 'sinon'; -import { Wallet } from '@bitgo/sdk-core'; - -import { generateAddress } from '../../src'; - -import { getUtxoCoin } from './util'; - -describe('Custom Change Wallets', () => { - const coin = getUtxoCoin('tbtc'); - - const keys = { - send: { - user: { id: '0', key: coin.keychains().create() }, - backup: { id: '1', key: coin.keychains().create() }, - bitgo: { id: '2', key: coin.keychains().create() }, - }, - change: { - user: { id: '3', key: coin.keychains().create() }, - backup: { id: '4', key: coin.keychains().create() }, - bitgo: { id: '5', key: coin.keychains().create() }, - }, - }; - - const customChangeKeySignatures = { - user: '', - backup: '', - bitgo: '', - }; - - const addressData = { - chain: 11, - index: 1, - addressType: 'p2shP2wsh' as const, - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - keychains: [ - { pub: keys.change.user.key.pub! }, - { pub: keys.change.backup.key.pub! }, - { pub: keys.change.bitgo.key.pub! }, - ], - threshold: 2, - }; - - const changeAddress = generateAddress(coin.name, addressData); - - const changeWalletId = 'changeWalletId'; - const stubData = { - signedSendingWallet: { - keyIds: sinon.stub().returns([keys.send.user.id, keys.send.backup.id, keys.send.bitgo.id]), - coinSpecific: sinon.stub().returns({ customChangeWalletId: changeWalletId }), - }, - changeWallet: { - keyIds: sinon.stub().returns([keys.change.user.id, keys.change.backup.id, keys.change.bitgo.id]), - createAddress: sinon.stub().resolves(changeAddress), - }, - }; - - before(async () => { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const sign = async ({ key }) => (await coin.signMessage({ prv: keys.send.user.key.prv }, key.pub!)).toString('hex'); - customChangeKeySignatures.user = await sign(keys.change.user); - customChangeKeySignatures.backup = await sign(keys.change.backup); - customChangeKeySignatures.bitgo = await sign(keys.change.bitgo); - }); +import assert from 'node:assert/strict'; + +import nock = require('nock'); +import { CoinName, fixedScriptWallet, BIP32, message } from '@bitgo/wasm-utxo'; +import * as utxolib from '@bitgo/utxo-lib'; +import { testutil } from '@bitgo/utxo-lib'; +import { common, Wallet } from '@bitgo/sdk-core'; +import { getSeed } from '@bitgo/sdk-test'; + +import { explainPsbt as explainPsbtUtxolib, explainPsbtWasm } from '../../src/transaction/fixedScript'; +import { verifyKeySignature } from '../../src/verifyKey'; +import { SdkBackend } from '../../src/transaction'; + +import { defaultBitGo, getUtxoCoin } from './util'; + +function explainPsbt( + psbt: utxolib.bitgo.UtxoPsbt | fixedScriptWallet.BitGoPsbt, + walletKeys: utxolib.bitgo.RootWalletKeys, + customChangeWalletKeys: utxolib.bitgo.RootWalletKeys | undefined, + coin: CoinName +) { + if (psbt instanceof fixedScriptWallet.BitGoPsbt) { + return explainPsbtWasm(psbt, fixedScriptWallet.RootWalletKeys.from(walletKeys), { + replayProtection: { publicKeys: [] }, + customChangeWalletXpubs: customChangeWalletKeys + ? fixedScriptWallet.RootWalletKeys.from(customChangeWalletKeys) + : undefined, + }); + } else { + return explainPsbtUtxolib(psbt, { pubs: walletKeys, customChangePubs: customChangeWalletKeys }, coin); + } +} + +function describeWithBackend(sdkBackend: SdkBackend) { + describe(`Custom Change Wallets (sdkBackend=${sdkBackend})`, function () { + const coin = getUtxoCoin('btc'); + const network = utxolib.networks.bitcoin; + const bgUrl = common.Environments[defaultBitGo.getEnv()].uri; + const rootWalletKeys = testutil.getDefaultWalletKeys(); + const customChangeWalletKeys = testutil.getWalletKeysForSeed('custom change'); + const userPrivateKey = BIP32.fromBase58(rootWalletKeys.triple[0].toBase58()).privateKey!; + + const mainKeyIds = rootWalletKeys.triple.map((k) => getSeed(k.neutered().toBase58()).toString('hex')); + const customChangeKeyIds = customChangeWalletKeys.triple.map((k) => + getSeed(k.neutered().toBase58()).toString('hex') + ); + const customChangeKeySignatures = Object.fromEntries( + (['user', 'backup', 'bitgo'] as const).map((name, i) => [ + name, + Buffer.from( + message.signMessage(customChangeWalletKeys.triple[i].neutered().toBase58(), userPrivateKey) + ).toString('hex'), + ]) + ) as Record<'user' | 'backup' | 'bitgo', string>; + + const inputs: testutil.Input[] = [{ scriptType: 'p2sh', value: BigInt(10000) }]; + const outputs: testutil.Output[] = [ + // regular change (uses rootWalletKeys via default) + { scriptType: 'p2sh', value: BigInt(3000) }, + // custom change (bip32Derivation from customChangeWalletKeys, not added as global xpubs) + { scriptType: 'p2sh', value: BigInt(3000), walletKeys: customChangeWalletKeys }, + // external (no derivation info) + { scriptType: 'p2sh', value: BigInt(3000), walletKeys: null }, + ]; + + const utxolibPsbt = testutil.constructPsbt(inputs, outputs, network, rootWalletKeys, 'unsigned', { + addGlobalXPubs: true, + }); + const psbt: utxolib.bitgo.UtxoPsbt | fixedScriptWallet.BitGoPsbt = + sdkBackend === 'wasm-utxo' ? fixedScriptWallet.BitGoPsbt.fromBytes(utxolibPsbt.toBuffer(), 'btc') : utxolibPsbt; + + const externalAddress = utxolib.address.fromOutputScript(utxolibPsbt.txOutputs[2].script, network); + const customChangeWalletId = 'custom-change-wallet-id'; + const mainWalletId = 'main-wallet-id'; + + function nockKeyFetch(keyIds: string[], keys: utxolib.bitgo.RootWalletKeys): nock.Scope[] { + return keyIds.map((id, i) => + nock(bgUrl) + .get(`/api/v2/${coin.getChain()}/key/${id}`) + .reply(200, { pub: keys.triple[i].neutered().toBase58() }) + ); + } - it('should consider addresses derived from the custom change keys as internal spends', async () => { - 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 outputAmount = 10000; - const recipients = []; - - sinon.stub(coin, 'explainTransaction').resolves({ - outputs: [], - changeOutputs: [ - { - address: changeAddress, - amount: outputAmount, - }, - ], - } as any); - - signedSendingWallet._wallet = signedSendingWallet._wallet || { - customChangeKeySignatures, - }; - - const parsedTransaction = await coin.parseTransaction({ - txParams: { changeAddress, recipients }, - txPrebuild: { txHex: '' }, - wallet: signedSendingWallet as any, - verification: { - addresses: { - [changeAddress]: { - chain: addressData.chain, - index: addressData.index, - }, - }, - }, + function nockCustomChangeWallet(): nock.Scope { + return nock(bgUrl).get(`/api/v2/${coin.getChain()}/wallet/${customChangeWalletId}`).reply(200, { + id: customChangeWalletId, + keys: customChangeKeyIds, + coin: coin.getChain(), + }); + } + + afterEach(() => nock.cleanAll()); + + it('classifies custom change output when customChangePubs is provided', function () { + const explanation = explainPsbt(psbt, rootWalletKeys, customChangeWalletKeys, 'btc'); + + assert.strictEqual(explanation.changeOutputs.length, 1); + assert.strictEqual(explanation.changeOutputs[0].amount, '3000'); + + assert.ok(explanation.customChangeOutputs); + assert.strictEqual(explanation.customChangeOutputs.length, 1); + assert.strictEqual(explanation.customChangeOutputs[0].amount, '3000'); + assert.strictEqual(explanation.customChangeAmount, '3000'); + + assert.strictEqual(explanation.outputs.length, 1); + assert.strictEqual(explanation.outputs[0].amount, '3000'); + }); + + it('classifies custom change output as external without customChangePubs', function () { + const explanation = explainPsbt(psbt, rootWalletKeys, undefined, 'btc'); + + assert.strictEqual(explanation.changeOutputs.length, 1); + assert.strictEqual(explanation.changeOutputs[0].amount, '3000'); + + assert.strictEqual(explanation.customChangeOutputs?.length ?? 0, 0); + + // custom change + external both treated as external outputs + assert.strictEqual(explanation.outputs.length, 2); }); - should.exist(parsedTransaction.outputs[0]); - parsedTransaction.outputs[0].should.deepEqual({ - address: changeAddress, - amount: outputAmount, - external: false, - needsCustomChangeKeySignatureVerification: true, + it('verifies valid custom change key signatures', function () { + const userPub = rootWalletKeys.triple[0].neutered().toBase58(); + + for (const key of customChangeWalletKeys.triple) { + const pub = key.neutered().toBase58(); + const signature = Buffer.from(message.signMessage(pub, userPrivateKey)).toString('hex'); + assert.ok( + verifyKeySignature({ userKeychain: { pub: userPub }, keychainToVerify: { pub }, keySignature: signature }) + ); + } }); - (coin.explainTransaction as any).restore(); - (coin.wallets as any).restore(); - (coin.keychains as any).restore(); - }); + it('rejects invalid custom change key signatures', function () { + const wrongKey = BIP32.fromBase58(testutil.getWalletKeysForSeed('wrong').triple[0].toBase58()); + const userPub = rootWalletKeys.triple[0].neutered().toBase58(); + + for (const key of customChangeWalletKeys.triple) { + const pub = key.neutered().toBase58(); + const badSignature = Buffer.from(message.signMessage(pub, wrongKey.privateKey!)).toString('hex'); + assert.strictEqual( + verifyKeySignature({ userKeychain: { pub: userPub }, keychainToVerify: { pub }, keySignature: badSignature }), + false + ); + } + }); - 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 }); + describe('parseTransaction', function () { + it('fetches custom change wallet keys and verifies signatures', async function () { + const wallet = new Wallet(defaultBitGo, coin, { + id: mainWalletId, + keys: mainKeyIds, + coin: coin.getChain(), + coinSpecific: { customChangeWalletId }, + customChangeKeySignatures, + }); + + const nocks = [ + ...nockKeyFetch(mainKeyIds, rootWalletKeys), + nockCustomChangeWallet(), + ...nockKeyFetch(customChangeKeyIds, customChangeWalletKeys), + ]; + + const parsed = await coin.parseTransaction({ + txParams: { recipients: [{ address: externalAddress, amount: '3000' }] }, + txPrebuild: { txHex: utxolibPsbt.toHex(), decodeWith: sdkBackend }, + wallet: wallet as unknown as import('../../src').UtxoWallet, + }); + + for (const n of nocks) assert.ok(n.isDone()); + + assert.ok(parsed.customChange); + assert.strictEqual(parsed.customChange.keys.length, 3); + for (let i = 0; i < 3; i++) { + assert.strictEqual(parsed.customChange.keys[i].pub, customChangeWalletKeys.triple[i].neutered().toBase58()); } - }), - } as any); - sinon.stub(coin, 'wallets').returns({ - get: sinon.stub().callsFake(() => Promise.resolve(changeWallet)), - } as any); + assert.strictEqual(parsed.explicitExternalOutputs.length, 1); + assert.strictEqual(parsed.explicitExternalOutputs[0].amount, '3000'); + }); + + it('has no custom change when wallet lacks customChangeWalletId', async function () { + const wallet = new Wallet(defaultBitGo, coin, { + id: mainWalletId, + keys: mainKeyIds, + coin: coin.getChain(), + coinSpecific: {}, + }); - const explainStub = sinon.stub(coin, 'explainTransaction'); + const nocks = nockKeyFetch(mainKeyIds, rootWalletKeys); - signedSendingWallet._wallet = signedSendingWallet._wallet || { - customChangeKeySignatures: invalidSignatures, - }; + const parsed = await coin.parseTransaction({ + txParams: { recipients: [{ address: externalAddress, amount: '3000' }] }, + txPrebuild: { txHex: utxolibPsbt.toHex(), decodeWith: sdkBackend }, + wallet: wallet as unknown as import('../../src').UtxoWallet, + }); - 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/); - } + for (const n of nocks) assert.ok(n.isDone()); - assert.strictEqual(explainStub.called, false, 'explainTransaction should not have been called'); + assert.strictEqual(parsed.customChange, undefined); + assert.strictEqual(parsed.needsCustomChangeKeySignatureVerification, false); + }); - explainStub.restore(); - (coin.wallets as any).restore(); - (coin.keychains as any).restore(); + it('rejects invalid custom change key signatures', async function () { + const wrongKey = BIP32.fromBase58(testutil.getWalletKeysForSeed('wrong').triple[0].toBase58()); + const badSignatures = Object.fromEntries( + (['user', 'backup', 'bitgo'] as const).map((name, i) => [ + name, + Buffer.from( + message.signMessage(customChangeWalletKeys.triple[i].neutered().toBase58(), wrongKey.privateKey!) + ).toString('hex'), + ]) + ) as Record<'user' | 'backup' | 'bitgo', string>; + + const wallet = new Wallet(defaultBitGo, coin, { + id: mainWalletId, + keys: mainKeyIds, + coin: coin.getChain(), + coinSpecific: { customChangeWalletId }, + customChangeKeySignatures: badSignatures, + }); + + nockKeyFetch(mainKeyIds, rootWalletKeys); + nockCustomChangeWallet(); + nockKeyFetch(customChangeKeyIds, customChangeWalletKeys); + + await assert.rejects( + () => + coin.parseTransaction({ + txParams: { recipients: [{ address: externalAddress, amount: '3000' }] }, + txPrebuild: { txHex: utxolibPsbt.toHex(), decodeWith: sdkBackend }, + wallet: wallet as unknown as import('../../src').UtxoWallet, + }), + /failed to verify custom change .* key signature/ + ); + }); + }); }); -}); +} + +describeWithBackend('utxolib'); +describeWithBackend('wasm-utxo');