Skip to content

Commit 37719f3

Browse files
davidkaplanbitgodavidkaplanbitgo
authored
Merge pull request #8875 from BitGo/T1-3418
feat(sdk-core,abstract-utxo): add qr param and client-side output val…
2 parents 37cde79 + bc2e6b6 commit 37719f3

14 files changed

Lines changed: 256 additions & 11 deletions

File tree

‎modules/abstract-lightning/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
]
4040
},
4141
"dependencies": {
42-
"@bitgo/public-types": "6.21.0",
42+
"@bitgo/public-types": "6.22.0",
4343
"@bitgo/sdk-core": "^37.3.0",
4444
"@bitgo/statics": "^58.43.0",
4545
"@bitgo/utxo-lib": "^11.22.1",

‎modules/abstract-utxo/src/abstractUtxoCoin.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,7 @@ export interface TransactionParams extends BaseTransactionParams {
279279
rbfTxIds?: string[];
280280
/** Parameters for bridging intents (e.g. BTC -> sBTC peg-in), present when `type === 'bridging'`. */
281281
bridgingParams?: BridgingParams;
282+
qr?: boolean;
282283
}
283284

284285
export interface ParseTransactionOptions<TNumber extends number | bigint = number> extends BaseParseTransactionOptions {

‎modules/abstract-utxo/src/transaction/descriptor/verifyTransaction.ts‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,32 @@ export async function verifyTransaction<TNumber extends number | bigint>(
9494
);
9595
}
9696

97-
assertValidTransaction(psbt, descriptorMap, params.txParams.recipients ?? [], coin.name);
97+
const parsedOutputs = toBaseParsedTransactionOutputsFromPsbt(
98+
psbt,
99+
descriptorMap,
100+
params.txParams.recipients ?? [],
101+
coin.name
102+
);
103+
104+
if (params.txParams.qr) {
105+
const allExternalOutputs = [...parsedOutputs.explicitExternalOutputs, ...parsedOutputs.implicitExternalOutputs];
106+
if (allExternalOutputs.length > 0) {
107+
const txExplanation = await TxIntentMismatchError.tryGetTxExplanation(
108+
coin as unknown as IBaseCoin,
109+
params.txPrebuild
110+
);
111+
throw new TxIntentMismatchError(
112+
'quantum-resistant sweep transactions must only contain wallet-internal outputs',
113+
params.reqId,
114+
[params.txParams],
115+
params.txPrebuild.txHex,
116+
txExplanation
117+
);
118+
}
119+
return true;
120+
}
121+
122+
assertExpectedOutputDifference(parsedOutputs);
98123

99124
return true;
100125
}

‎modules/abstract-utxo/src/transaction/fixedScript/verifyTransaction.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,17 @@ export async function verifyTransaction<TNumber extends bigint | number>(
127127
debug('successfully verified user public key and custom change key signatures');
128128
}
129129

130+
if (txParams.qr) {
131+
const allExternalOutputs = [
132+
...parsedTransaction.explicitExternalOutputs,
133+
...parsedTransaction.implicitExternalOutputs,
134+
];
135+
if (allExternalOutputs.length > 0) {
136+
throwTxMismatch('quantum-resistant sweep transactions must only contain wallet-internal outputs');
137+
}
138+
return true;
139+
}
140+
130141
const missingOutputs = parsedTransaction.missingOutputs;
131142
if (missingOutputs.length !== 0) {
132143
// there are some outputs in the recipients list that have not made it into the actual transaction

‎modules/abstract-utxo/test/unit/transaction/descriptor/verifyTransactionQr.ts‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import assert from 'assert';
2+
3+
import * as testutils from '@bitgo/wasm-utxo/testutils';
4+
5+
import { verifyTransaction } from '../../../../src/transaction/descriptor/verifyTransaction';
6+
import { getUtxoCoin } from '../../util';
7+
8+
const { getDefaultXPubs, getDescriptor, getDescriptorMap, mockPsbt } = testutils.descriptor;
9+
10+
describe('descriptor verifyTransaction - quantum-resistant sweep', function () {
11+
const coin = getUtxoCoin('tbtc');
12+
13+
const xpubsSelf = getDefaultXPubs('a');
14+
const xpubsOther = getDefaultXPubs('b');
15+
const descriptorSelf = getDescriptor('Wsh2Of3', xpubsSelf);
16+
const descriptorOther = getDescriptor('Wsh2Of3', xpubsOther);
17+
const descriptorMap = getDescriptorMap('Wsh2Of3', xpubsSelf);
18+
19+
function buildPsbtAllInternal() {
20+
return mockPsbt(
21+
[
22+
{ descriptor: descriptorSelf, index: 0 },
23+
{ descriptor: descriptorSelf, index: 1, id: { vout: 1 } },
24+
],
25+
[
26+
{ descriptor: descriptorSelf, index: 0, value: BigInt(4e5) },
27+
{ descriptor: descriptorSelf, index: 1, value: BigInt(4e5) },
28+
]
29+
);
30+
}
31+
32+
function buildPsbtWithExternal() {
33+
return mockPsbt(
34+
[
35+
{ descriptor: descriptorSelf, index: 0 },
36+
{ descriptor: descriptorSelf, index: 1, id: { vout: 1 } },
37+
],
38+
[
39+
{ descriptor: descriptorOther, index: 0, value: BigInt(4e5), external: true },
40+
{ descriptor: descriptorSelf, index: 0, value: BigInt(4e5) },
41+
]
42+
);
43+
}
44+
45+
it('should reject when external outputs exist and qr is true', async function () {
46+
const psbt = buildPsbtWithExternal();
47+
48+
await assert.rejects(
49+
verifyTransaction(
50+
coin,
51+
{
52+
txParams: { qr: true, recipients: [] },
53+
txPrebuild: { txHex: Buffer.from(psbt.serialize()).toString('hex') },
54+
wallet: {} as any,
55+
},
56+
descriptorMap
57+
),
58+
/quantum-resistant sweep transactions must only contain wallet-internal outputs/
59+
);
60+
});
61+
62+
it('should pass when all outputs are internal and qr is true', async function () {
63+
const psbt = buildPsbtAllInternal();
64+
65+
const result = await verifyTransaction(
66+
coin,
67+
{
68+
txParams: { qr: true, recipients: [] },
69+
txPrebuild: { txHex: Buffer.from(psbt.serialize()).toString('hex') },
70+
wallet: {} as any,
71+
},
72+
descriptorMap
73+
);
74+
75+
assert.strictEqual(result, true);
76+
});
77+
78+
it('should not apply qr check when qr is not set (internal-only outputs pass normally)', async function () {
79+
const psbt = buildPsbtAllInternal();
80+
81+
const result = await verifyTransaction(
82+
coin,
83+
{
84+
txParams: { recipients: [] },
85+
txPrebuild: { txHex: Buffer.from(psbt.serialize()).toString('hex') },
86+
wallet: {} as any,
87+
},
88+
descriptorMap
89+
);
90+
91+
assert.strictEqual(result, true);
92+
});
93+
});

‎modules/abstract-utxo/test/unit/verifyTransaction.ts‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,119 @@ describe('Verify Transaction', function () {
465465
coinMock.restore();
466466
});
467467

468+
describe('quantum-resistant sweep (qr: true)', function () {
469+
it('should reject when explicit external outputs are present', async () => {
470+
const coinMock = sinon.stub(coin, 'parseTransaction').resolves({
471+
keychains: {} as any,
472+
keySignatures: {},
473+
outputs: [],
474+
missingOutputs: [],
475+
explicitExternalOutputs: [{ address: 'external_addr', amount: '5000' }],
476+
implicitExternalOutputs: [],
477+
changeOutputs: [{ address: 'change_addr', amount: '4000' }],
478+
explicitExternalSpendAmount: 5000,
479+
implicitExternalSpendAmount: 0,
480+
needsCustomChangeKeySignatureVerification: false,
481+
});
482+
483+
await assert.rejects(
484+
coin.verifyTransaction({
485+
txParams: { walletPassphrase: passphrase, qr: true },
486+
txPrebuild: {},
487+
wallet: unsignedSendingWallet as any,
488+
verification: {},
489+
}),
490+
/quantum-resistant sweep transactions must only contain wallet-internal outputs/
491+
);
492+
493+
coinMock.restore();
494+
});
495+
496+
it('should reject when implicit external outputs are present', async () => {
497+
const coinMock = sinon.stub(coin, 'parseTransaction').resolves({
498+
keychains: {} as any,
499+
keySignatures: {},
500+
outputs: [],
501+
missingOutputs: [],
502+
explicitExternalOutputs: [],
503+
implicitExternalOutputs: [{ address: 'paygo_addr', amount: '100' }],
504+
changeOutputs: [{ address: 'change_addr', amount: '9900' }],
505+
explicitExternalSpendAmount: 0,
506+
implicitExternalSpendAmount: 100,
507+
needsCustomChangeKeySignatureVerification: false,
508+
});
509+
510+
await assert.rejects(
511+
coin.verifyTransaction({
512+
txParams: { walletPassphrase: passphrase, qr: true },
513+
txPrebuild: {},
514+
wallet: unsignedSendingWallet as any,
515+
verification: {},
516+
}),
517+
/quantum-resistant sweep transactions must only contain wallet-internal outputs/
518+
);
519+
520+
coinMock.restore();
521+
});
522+
523+
it('should pass when all outputs are internal (change only)', async () => {
524+
const coinMock = sinon.stub(coin, 'parseTransaction').resolves({
525+
keychains: {} as any,
526+
keySignatures: {},
527+
outputs: [{ address: 'change_addr', amount: '10000' }],
528+
missingOutputs: [],
529+
explicitExternalOutputs: [],
530+
implicitExternalOutputs: [],
531+
changeOutputs: [{ address: 'change_addr', amount: '10000' }],
532+
explicitExternalSpendAmount: 0,
533+
implicitExternalSpendAmount: 0,
534+
needsCustomChangeKeySignatureVerification: false,
535+
});
536+
537+
const result = await coin.verifyTransaction({
538+
txParams: { walletPassphrase: passphrase, qr: true },
539+
txPrebuild: {},
540+
wallet: unsignedSendingWallet as any,
541+
verification: {},
542+
});
543+
544+
assert.strictEqual(result, true);
545+
546+
coinMock.restore();
547+
});
548+
549+
it('should not apply qr check when qr is not set', async () => {
550+
const coinMock = sinon.stub(coin, 'parseTransaction').resolves({
551+
keychains: {} as any,
552+
keySignatures: {},
553+
outputs: [],
554+
missingOutputs: [],
555+
explicitExternalOutputs: [{ address: 'external_addr', amount: '5000' }],
556+
implicitExternalOutputs: [],
557+
changeOutputs: [],
558+
explicitExternalSpendAmount: 5000,
559+
implicitExternalSpendAmount: 0,
560+
needsCustomChangeKeySignatureVerification: false,
561+
});
562+
563+
const bitcoinMock = sinon
564+
.stub(coin, 'createTransactionFromHex')
565+
.returns({ ins: [] } as unknown as utxolib.bitgo.UtxoTransaction);
566+
567+
const result = await coin.verifyTransaction({
568+
txParams: { walletPassphrase: passphrase },
569+
txPrebuild: { txHex: '00' },
570+
wallet: unsignedSendingWallet as any,
571+
verification: {},
572+
});
573+
574+
assert.strictEqual(result, true);
575+
576+
coinMock.restore();
577+
bitcoinMock.restore();
578+
});
579+
});
580+
468581
it('should work with bigint amounts', async () => {
469582
// need a coin that uses bigint
470583
const bigintCoin = getUtxoCoin('tdoge');

‎modules/bitgo/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@
140140
"superagent": "^9.0.1"
141141
},
142142
"devDependencies": {
143-
"@bitgo/public-types": "6.21.0",
143+
"@bitgo/public-types": "6.22.0",
144144
"@bitgo/sdk-opensslbytes": "^2.1.0",
145145
"@bitgo/sdk-test": "^9.1.46",
146146
"@openpgp/web-stream-tools": "0.0.14",

‎modules/express/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@
6060
"superagent": "^9.0.1"
6161
},
6262
"devDependencies": {
63-
"@bitgo/public-types": "6.21.0",
63+
"@bitgo/public-types": "6.22.0",
6464
"@bitgo/sdk-lib-mpc": "^10.14.0",
6565
"@bitgo/sdk-test": "^9.1.46",
6666
"@types/argparse": "^1.0.36",

‎modules/sdk-coin-flrp/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
"@bitgo/sdk-test": "^9.1.46"
4848
},
4949
"dependencies": {
50-
"@bitgo/public-types": "6.21.0",
50+
"@bitgo/public-types": "6.22.0",
5151
"@bitgo/sdk-core": "^37.3.0",
5252
"@bitgo/secp256k1": "^1.11.0",
5353
"@bitgo/statics": "^58.43.0",

‎modules/sdk-coin-sol/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@
5757
},
5858
"dependencies": {
5959
"@bitgo/logger": "^1.4.0",
60-
"@bitgo/public-types": "6.21.0",
60+
"@bitgo/public-types": "6.22.0",
6161
"@bitgo/sdk-core": "^37.3.0",
6262
"@bitgo/sdk-lib-mpc": "^10.14.0",
6363
"@bitgo/statics": "^58.43.0",

0 commit comments

Comments
 (0)