Skip to content

Commit e37618a

Browse files
OttoAllmendingerllm-git
andcommitted
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 <llm-git@ttll.de>
1 parent 32e748f commit e37618a

5 files changed

Lines changed: 167 additions & 3 deletions

File tree

modules/abstract-utxo/src/abstractUtxoCoin.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@ export interface ExplainTransactionOptions<TNumber extends number | bigint = num
243243
txInfo?: TransactionInfo<TNumber>;
244244
feeInfo?: string;
245245
pubs?: Triple<string>;
246+
customChangeXpubs?: Triple<string>;
246247
decodeWith?: SdkBackend;
247248
}
248249

modules/abstract-utxo/src/transaction/explainTransaction.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ export function explainTx<TNumber extends number | bigint>(
2727
params: {
2828
wallet?: IWallet;
2929
pubs?: string[];
30+
customChangeXpubs?: Triple<string>;
3031
txInfo?: { unspents?: Unspent<TNumber>[] };
3132
changeInfo?: fixedScript.ChangeAddressInfo[];
3233
},
@@ -49,7 +50,7 @@ export function explainTx<TNumber extends number | bigint>(
4950
throw new Error('legacy transactions are not supported for descriptor wallets');
5051
}
5152
if (tx instanceof utxolib.bitgo.UtxoPsbt) {
52-
return fixedScript.explainPsbt(tx, params, coinName);
53+
return fixedScript.explainPsbt(tx, { ...params, customChangePubs: params.customChangeXpubs }, coinName);
5354
} else if (tx instanceof fixedScriptWallet.BitGoPsbt) {
5455
const pubs = params.pubs;
5556
if (!pubs) {
@@ -68,6 +69,7 @@ export function explainTx<TNumber extends number | bigint>(
6869
replayProtection: {
6970
publicKeys: getReplayProtectionPubkeys(coinName),
7071
},
72+
customChangeWalletXpubs: params.customChangeXpubs,
7173
});
7274
} else {
7375
return fixedScript.explainLegacyTx(tx, params, coinName);

modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,19 @@
11
import assert from 'assert';
22

33
import _ from 'lodash';
4-
import { ITransactionRecipient, Triple, VerificationOptions, Wallet } from '@bitgo/sdk-core';
4+
import { ITransactionRecipient, KeyIndices, Triple, VerificationOptions, Wallet } from '@bitgo/sdk-core';
55

66
import type { AbstractUtxoCoin, ParseTransactionOptions } from '../../abstractUtxoCoin';
77
import type { FixedScriptWalletOutput, Output, ParsedTransaction } from '../types';
8-
import { fetchKeychains, getKeySignatures, toKeychainTriple, UtxoKeychain, UtxoNamedKeychains } from '../../keychains';
8+
import {
9+
fetchKeychains,
10+
getKeySignatures,
11+
toKeychainTriple,
12+
toXpubTriple,
13+
UtxoKeychain,
14+
UtxoNamedKeychains,
15+
} from '../../keychains';
16+
import { verifyKeySignature } from '../../verifyKey';
917
import {
1018
assertValidTransactionRecipient,
1119
fromExtendedAddressFormatToScript,
@@ -112,6 +120,20 @@ function toExpectedOutputs(
112120
return expectedOutputs;
113121
}
114122

123+
function verifyCustomChangeKeys(userKeychain: UtxoKeychain, customChange: CustomChangeOptions): void {
124+
for (const keyIndex of [KeyIndices.USER, KeyIndices.BACKUP, KeyIndices.BITGO]) {
125+
if (
126+
!verifyKeySignature({
127+
userKeychain,
128+
keychainToVerify: customChange.keys[keyIndex],
129+
keySignature: customChange.signatures[keyIndex],
130+
})
131+
) {
132+
throw new Error(`failed to verify custom change ${KeyIndices[keyIndex].toLowerCase()} key signature`);
133+
}
134+
}
135+
}
136+
115137
export async function parseTransaction<TNumber extends bigint | number>(
116138
coin: AbstractUtxoCoin,
117139
params: ParseTransactionOptions<TNumber>
@@ -177,11 +199,18 @@ export async function parseTransaction<TNumber extends bigint | number>(
177199
}
178200
}
179201

202+
let customChangeXpubs: Triple<string> | undefined;
203+
if (customChange) {
204+
verifyCustomChangeKeys(keychainArray[KeyIndices.USER], customChange);
205+
customChangeXpubs = toXpubTriple(customChange.keys);
206+
}
207+
180208
// obtain all outputs
181209
const explanation: TransactionExplanation = await coin.explainTransaction<TNumber>({
182210
txHex: txPrebuild.txHex,
183211
txInfo: txPrebuild.txInfo,
184212
pubs: keychainArray.map((k) => k.pub) as Triple<string>,
213+
customChangeXpubs,
185214
});
186215

187216
const allOutputs = [...explanation.outputs, ...explanation.changeOutputs];

modules/abstract-utxo/test/unit/customChangeWallet.ts

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import assert from 'assert';
2+
13
import should = require('should');
24
import * as sinon from 'sinon';
35
import { Wallet } from '@bitgo/sdk-core';
@@ -133,4 +135,66 @@ describe('Custom Change Wallets', () => {
133135
(coin.wallets as any).restore();
134136
(coin.keychains as any).restore();
135137
});
138+
139+
it('should reject invalid custom change key signatures before calling explainTransaction', async () => {
140+
const wrongKey = coin.keychains().create();
141+
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
142+
const sign = async ({ key }) => (await coin.signMessage({ prv: wrongKey.prv }, key.pub!)).toString('hex');
143+
const invalidSignatures = {
144+
user: await sign(keys.change.user),
145+
backup: await sign(keys.change.backup),
146+
bitgo: await sign(keys.change.bitgo),
147+
};
148+
149+
const signedSendingWallet = sinon.createStubInstance(Wallet, stubData.signedSendingWallet as any);
150+
const changeWallet = sinon.createStubInstance(Wallet, stubData.changeWallet as any);
151+
152+
sinon.stub(coin, 'keychains').returns({
153+
get: sinon.stub().callsFake(({ id }) => {
154+
switch (id) {
155+
case keys.send.user.id:
156+
return Promise.resolve({ id, ...keys.send.user.key });
157+
case keys.send.backup.id:
158+
return Promise.resolve({ id, ...keys.send.backup.key });
159+
case keys.send.bitgo.id:
160+
return Promise.resolve({ id, ...keys.send.bitgo.key });
161+
case keys.change.user.id:
162+
return Promise.resolve({ id, ...keys.change.user.key });
163+
case keys.change.backup.id:
164+
return Promise.resolve({ id, ...keys.change.backup.key });
165+
case keys.change.bitgo.id:
166+
return Promise.resolve({ id, ...keys.change.bitgo.key });
167+
}
168+
}),
169+
} as any);
170+
171+
sinon.stub(coin, 'wallets').returns({
172+
get: sinon.stub().callsFake(() => Promise.resolve(changeWallet)),
173+
} as any);
174+
175+
const explainStub = sinon.stub(coin, 'explainTransaction');
176+
177+
signedSendingWallet._wallet = signedSendingWallet._wallet || {
178+
customChangeKeySignatures: invalidSignatures,
179+
};
180+
181+
try {
182+
await coin.parseTransaction({
183+
txParams: { recipients: [] },
184+
txPrebuild: { txHex: '' },
185+
wallet: signedSendingWallet as any,
186+
verification: {},
187+
});
188+
assert.fail('parseTransaction should have thrown for invalid custom change key signatures');
189+
} catch (e) {
190+
assert.ok(e instanceof Error);
191+
assert.match(e.message, /failed to verify custom change .* key signature/);
192+
}
193+
194+
assert.strictEqual(explainStub.called, false, 'explainTransaction should not have been called');
195+
196+
explainStub.restore();
197+
(coin.wallets as any).restore();
198+
(coin.keychains as any).restore();
199+
});
136200
});
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import assert from 'assert';
2+
3+
import { BIP32, message } from '@bitgo/wasm-utxo';
4+
5+
import { verifyKeySignature } from '../../src/verifyKey';
6+
7+
const KEY_ROLES = ['user', 'backup', 'bitgo'] as const;
8+
type KeyRole = (typeof KEY_ROLES)[number];
9+
10+
function signKeySignature(signerPrivateKey: BIP32, pubToSign: string): string {
11+
assert(signerPrivateKey.privateKey, 'signer must have a private key');
12+
return Buffer.from(message.signMessage(pubToSign, signerPrivateKey.privateKey)).toString('hex');
13+
}
14+
15+
function createMockCustomChangeKeySignatures(
16+
signerPrivateKey: BIP32,
17+
changeKeychains: { pub: string }[]
18+
): Record<KeyRole, string> {
19+
return {
20+
user: signKeySignature(signerPrivateKey, changeKeychains[0].pub),
21+
backup: signKeySignature(signerPrivateKey, changeKeychains[1].pub),
22+
bitgo: signKeySignature(signerPrivateKey, changeKeychains[2].pub),
23+
};
24+
}
25+
26+
describe('Custom Change Wallet Key Signatures', function () {
27+
const mainUserKey = BIP32.fromSeedSha256('main-user');
28+
const changeKeys = [
29+
BIP32.fromSeedSha256('change-user'),
30+
BIP32.fromSeedSha256('change-backup'),
31+
BIP32.fromSeedSha256('change-bitgo'),
32+
];
33+
const changeKeychains = changeKeys.map((k) => ({ pub: k.neutered().toBase58() }));
34+
35+
it('should accept signatures from the correct key', function () {
36+
const signatures = createMockCustomChangeKeySignatures(mainUserKey, changeKeychains);
37+
38+
for (const role of KEY_ROLES) {
39+
const index = KEY_ROLES.indexOf(role);
40+
assert.ok(
41+
verifyKeySignature({
42+
userKeychain: { pub: mainUserKey.neutered().toBase58() },
43+
keychainToVerify: changeKeychains[index],
44+
keySignature: signatures[role],
45+
}),
46+
`failed to verify mock custom change ${role} key signature`
47+
);
48+
}
49+
});
50+
51+
it('should reject signatures from a different key', function () {
52+
const wrongKey = BIP32.fromSeedSha256('wrong-key');
53+
const badSignatures = createMockCustomChangeKeySignatures(wrongKey, changeKeychains);
54+
55+
for (const role of KEY_ROLES) {
56+
const index = KEY_ROLES.indexOf(role);
57+
assert.strictEqual(
58+
verifyKeySignature({
59+
userKeychain: { pub: mainUserKey.neutered().toBase58() },
60+
keychainToVerify: changeKeychains[index],
61+
keySignature: badSignatures[role],
62+
}),
63+
false,
64+
`should have rejected custom change ${role} key signature from wrong key`
65+
);
66+
}
67+
});
68+
});

0 commit comments

Comments
 (0)