Skip to content

Commit a4ff0b2

Browse files
Merge pull request #7972 from BitGo/fix-utxo-threshold
fix(sdk-coin-flrp): correct change output thresholds for multisig wallets
2 parents 5158c6f + 289e1c9 commit a4ff0b2

3 files changed

Lines changed: 200 additions & 7 deletions

File tree

modules/sdk-coin-flrp/src/lib/ExportInPTxBuilder.ts

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,28 @@ export class ExportInPTxBuilder extends AtomicTransactionBuilder {
179179
const flareUnsignedTx = exportTx as UnsignedTx;
180180
const innerTx = flareUnsignedTx.getTx() as pvmSerial.ExportTx;
181181

182-
const utxosWithIndex = innerTx.baseTx.inputs.map((input) => {
182+
const changeOutputs = innerTx.baseTx.outputs;
183+
let correctedExportTx: pvmSerial.ExportTx = innerTx;
184+
185+
if (changeOutputs.length > 0) {
186+
const correctedChangeOutputs = changeOutputs.map((output) => {
187+
const transferOut = output.output as TransferOutput;
188+
const originalOwners = transferOut.outputOwners;
189+
190+
const assetIdStr = utils.flareIdString(Buffer.from(output.assetId.toBytes()).toString('hex')).toString();
191+
return TransferableOutput.fromNative(
192+
assetIdStr,
193+
transferOut.amount(),
194+
originalOwners.addrs.map((addr) => Buffer.from(addr.toBytes())),
195+
this.transaction._locktime,
196+
this.transaction._threshold
197+
);
198+
});
199+
200+
correctedExportTx = this.createCorrectedExportTx(innerTx, correctedChangeOutputs);
201+
}
202+
203+
const utxosWithIndex = correctedExportTx.baseTx.inputs.map((input) => {
183204
const inputTxid = utils.cb58Encode(Buffer.from(input.utxoID.txID.toBytes()));
184205
const inputOutputIdx = input.utxoID.outputIdx.value().toString();
185206

@@ -213,11 +234,46 @@ export class ExportInPTxBuilder extends AtomicTransactionBuilder {
213234
this.createAddressMapForUtxoWithSigIndices(utxo, utxo.threshold, utxo.actualSigIndices)
214235
);
215236

216-
const fixedUnsignedTx = new UnsignedTx(innerTx, [], new FlareUtils.AddressMaps(addressMaps), txCredentials);
237+
const fixedUnsignedTx = new UnsignedTx(
238+
correctedExportTx,
239+
[],
240+
new FlareUtils.AddressMaps(addressMaps),
241+
txCredentials
242+
);
217243

218244
this.transaction.setTransaction(fixedUnsignedTx);
219245
}
220246

247+
/**
248+
* Create a corrected ExportTx with the given change outputs.
249+
* This is necessary because FlareJS's newExportTx doesn't support setting
250+
* the threshold and locktime for change outputs.
251+
*
252+
* FlareJS declares baseTx.outputs as readonly, so we use Object.defineProperty
253+
* to override the property with the corrected outputs. This is a workaround until
254+
* FlareJS adds proper support for change output thresholds.
255+
*
256+
* @param originalTx - The original ExportTx
257+
* @param correctedOutputs - The corrected change outputs with proper threshold
258+
* @returns A new ExportTx with the corrected change outputs
259+
*/
260+
private createCorrectedExportTx(
261+
originalTx: pvmSerial.ExportTx,
262+
correctedOutputs: TransferableOutput[]
263+
): pvmSerial.ExportTx {
264+
// FlareJS declares baseTx.outputs as `public readonly outputs: readonly TransferableOutput[]`
265+
// We use Object.defineProperty to override the readonly property with our corrected outputs.
266+
// This is necessary because FlareJS's newExportTx doesn't support change output threshold/locktime.
267+
Object.defineProperty(originalTx.baseTx, 'outputs', {
268+
value: correctedOutputs,
269+
writable: false,
270+
enumerable: true,
271+
configurable: true,
272+
});
273+
274+
return originalTx;
275+
}
276+
221277
/**
222278
* Recover UTXOs from inputs.
223279
* Uses output addresses as proxy for UTXO addresses.

modules/sdk-coin-flrp/test/resources/transactionData/exportInP.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
11
// Test data for export with single UTXO
2-
// Transaction ID: 2R4iE6sX6BtAeTNrVdzifczs7qRGQw3yiaFTXaw9j9A4R6D5FW
32
export const EXPORT_IN_P = {
4-
txhash: '2R4iE6sX6BtAeTNrVdzifczs7qRGQw3yiaFTXaw9j9A4R6D5FW',
3+
txhash: '2HN4sSQeyS1T8ztWdsVv4Cyj3D8zJxGTMaSZeqWVmBfJEiYsiQ',
54
// Unsigned tx from script (with empty signatures + credential structure)
65
unsignedHex:
7-
'0x0000000000120000007200000000000000000000000000000000000000000000000000000000000000000000000158734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000070000000000daba24000000000000000000000001000000033329be7d01cd3ebaae6654d7327dd9f17a2e15817e918a5e8083ae4c9f2f0ed77055c24bf3665001c7324437c96c7c8a6a152da2385c1db5c3ab1f9100000002b489f3c616329c3d56a29c24702c348522e87e1ad0e23f02fbb405be599fbae30000000058734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd0000000500000000015d35f30000000100000000e65421a9e3307ed1f644e6e44855f94e01c35ea2bce2cdd9005a77e48decbd5c0000000058734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000050000000002e7b2b80000000200000000000000010000000078db5c30bed04c05ce209179812850bbb3fe6d46d7eef3744d814c0da55524790000000158734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000070000000003473bc0000000000000000000000002000000033329be7d01cd3ebaae6654d7327dd9f17a2e15817e918a5e8083ae4c9f2f0ed77055c24bf3665001c7324437c96c7c8a6a152da2385c1db5c3ab1f910000000200000009000000010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007e918a5e8083ae4c9f2f0ed77055c24bf3665001000000090000000200000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007e918a5e8083ae4c9f2f0ed77055c24bf36650012845ffc4',
6+
'0x0000000000120000007200000000000000000000000000000000000000000000000000000000000000000000000158734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000070000000000daba24000000000000000000000002000000033329be7d01cd3ebaae6654d7327dd9f17a2e15817e918a5e8083ae4c9f2f0ed77055c24bf3665001c7324437c96c7c8a6a152da2385c1db5c3ab1f9100000002b489f3c616329c3d56a29c24702c348522e87e1ad0e23f02fbb405be599fbae30000000058734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd0000000500000000015d35f30000000100000000e65421a9e3307ed1f644e6e44855f94e01c35ea2bce2cdd9005a77e48decbd5c0000000058734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000050000000002e7b2b80000000200000000000000010000000078db5c30bed04c05ce209179812850bbb3fe6d46d7eef3744d814c0da55524790000000158734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000070000000003473bc0000000000000000000000002000000033329be7d01cd3ebaae6654d7327dd9f17a2e15817e918a5e8083ae4c9f2f0ed77055c24bf3665001c7324437c96c7c8a6a152da2385c1db5c3ab1f910000000200000009000000010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007e918a5e8083ae4c9f2f0ed77055c24bf3665001000000090000000200000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007e918a5e8083ae4c9f2f0ed77055c24bf36650016aec5964',
87
halfSigntxHex:
9-
'0x0000000000120000007200000000000000000000000000000000000000000000000000000000000000000000000158734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000070000000000daba24000000000000000000000001000000033329be7d01cd3ebaae6654d7327dd9f17a2e15817e918a5e8083ae4c9f2f0ed77055c24bf3665001c7324437c96c7c8a6a152da2385c1db5c3ab1f9100000002b489f3c616329c3d56a29c24702c348522e87e1ad0e23f02fbb405be599fbae30000000058734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd0000000500000000015d35f30000000100000000e65421a9e3307ed1f644e6e44855f94e01c35ea2bce2cdd9005a77e48decbd5c0000000058734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000050000000002e7b2b80000000200000000000000010000000078db5c30bed04c05ce209179812850bbb3fe6d46d7eef3744d814c0da55524790000000158734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000070000000003473bc0000000000000000000000002000000033329be7d01cd3ebaae6654d7327dd9f17a2e15817e918a5e8083ae4c9f2f0ed77055c24bf3665001c7324437c96c7c8a6a152da2385c1db5c3ab1f9100000002000000090000000184918d7e399e192bf14262c9f9622feb13f412e987f6c411b90215175e4ef4d61c629593188d703e53579a88cef6973414b7c2bef10c582ea883b1767be2622a00000000090000000284918d7e399e192bf14262c9f9622feb13f412e987f6c411b90215175e4ef4d61c629593188d703e53579a88cef6973414b7c2bef10c582ea883b1767be2622a000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007e918a5e8083ae4c9f2f0ed77055c24bf3665001f945a1ff',
8+
'0x0000000000120000007200000000000000000000000000000000000000000000000000000000000000000000000158734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000070000000000daba24000000000000000000000002000000033329be7d01cd3ebaae6654d7327dd9f17a2e15817e918a5e8083ae4c9f2f0ed77055c24bf3665001c7324437c96c7c8a6a152da2385c1db5c3ab1f9100000002b489f3c616329c3d56a29c24702c348522e87e1ad0e23f02fbb405be599fbae30000000058734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd0000000500000000015d35f30000000100000000e65421a9e3307ed1f644e6e44855f94e01c35ea2bce2cdd9005a77e48decbd5c0000000058734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000050000000002e7b2b80000000200000000000000010000000078db5c30bed04c05ce209179812850bbb3fe6d46d7eef3744d814c0da55524790000000158734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000070000000003473bc0000000000000000000000002000000033329be7d01cd3ebaae6654d7327dd9f17a2e15817e918a5e8083ae4c9f2f0ed77055c24bf3665001c7324437c96c7c8a6a152da2385c1db5c3ab1f91000000020000000900000001ef504cec2443f9f68078976afa43eb518d7c6b8455389a743fe25fdde1b8e04f5bc94002efbdbd4a1feb3f5739f944970484336ae51d6aff3802493b0014e85c000000000900000002ef504cec2443f9f68078976afa43eb518d7c6b8455389a743fe25fdde1b8e04f5bc94002efbdbd4a1feb3f5739f944970484336ae51d6aff3802493b0014e85c000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007e918a5e8083ae4c9f2f0ed77055c24bf3665001329c4a92',
109
fullSigntxHex:
11-
'0x0000000000120000007200000000000000000000000000000000000000000000000000000000000000000000000158734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000070000000000daba24000000000000000000000001000000033329be7d01cd3ebaae6654d7327dd9f17a2e15817e918a5e8083ae4c9f2f0ed77055c24bf3665001c7324437c96c7c8a6a152da2385c1db5c3ab1f9100000002b489f3c616329c3d56a29c24702c348522e87e1ad0e23f02fbb405be599fbae30000000058734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd0000000500000000015d35f30000000100000000e65421a9e3307ed1f644e6e44855f94e01c35ea2bce2cdd9005a77e48decbd5c0000000058734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000050000000002e7b2b80000000200000000000000010000000078db5c30bed04c05ce209179812850bbb3fe6d46d7eef3744d814c0da55524790000000158734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000070000000003473bc0000000000000000000000002000000033329be7d01cd3ebaae6654d7327dd9f17a2e15817e918a5e8083ae4c9f2f0ed77055c24bf3665001c7324437c96c7c8a6a152da2385c1db5c3ab1f9100000002000000090000000184918d7e399e192bf14262c9f9622feb13f412e987f6c411b90215175e4ef4d61c629593188d703e53579a88cef6973414b7c2bef10c582ea883b1767be2622a00000000090000000284918d7e399e192bf14262c9f9622feb13f412e987f6c411b90215175e4ef4d61c629593188d703e53579a88cef6973414b7c2bef10c582ea883b1767be2622a0067902f8f061a628182a3ec1d768b100a3d13fcc01966a993c3300bf3cb5d3dc32870442f83c67741d22f5e7a1168f5ef7f22f26d7b62a183528fcbfd0fdede9300f1e3d1ac',
10+
'0x0000000000120000007200000000000000000000000000000000000000000000000000000000000000000000000158734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000070000000000daba24000000000000000000000002000000033329be7d01cd3ebaae6654d7327dd9f17a2e15817e918a5e8083ae4c9f2f0ed77055c24bf3665001c7324437c96c7c8a6a152da2385c1db5c3ab1f9100000002b489f3c616329c3d56a29c24702c348522e87e1ad0e23f02fbb405be599fbae30000000058734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd0000000500000000015d35f30000000100000000e65421a9e3307ed1f644e6e44855f94e01c35ea2bce2cdd9005a77e48decbd5c0000000058734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000050000000002e7b2b80000000200000000000000010000000078db5c30bed04c05ce209179812850bbb3fe6d46d7eef3744d814c0da55524790000000158734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000070000000003473bc0000000000000000000000002000000033329be7d01cd3ebaae6654d7327dd9f17a2e15817e918a5e8083ae4c9f2f0ed77055c24bf3665001c7324437c96c7c8a6a152da2385c1db5c3ab1f91000000020000000900000001ef504cec2443f9f68078976afa43eb518d7c6b8455389a743fe25fdde1b8e04f5bc94002efbdbd4a1feb3f5739f944970484336ae51d6aff3802493b0014e85c000000000900000002ef504cec2443f9f68078976afa43eb518d7c6b8455389a743fe25fdde1b8e04f5bc94002efbdbd4a1feb3f5739f944970484336ae51d6aff3802493b0014e85c000fd603ba6d1c63b84c0bfb50e8a2eef0a745379fbf687a09d15f2b411907eb886d725ff52060e7863258c5abe506a690446923ec9f00b97487c2df781aea88ab0181fe5c0a',
1211
amount: '55000000', // 0.055 (0.05 FLR + 0.005 FLR fee)
1312
pAddresses: [
1413
'P-costwo106gc5h5qswhye8e0pmthq4wzf0ekv5qppsrvpu',

modules/sdk-coin-flrp/test/unit/lib/exportInPTxBuilder.ts

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { EXPORT_IN_P as testData } from '../../resources/transactionData/exportI
44
import { TransactionBuilderFactory } from '../../../src/lib';
55
import { coins } from '@bitgo/statics';
66
import signFlowTest from './signFlowTestSuit';
7+
import { pvmSerial, UnsignedTx, TransferOutput } from '@flarenetwork/flarejs';
78

89
describe('Flrp Export In P Tx Builder', () => {
910
const coinConfig = coins.get('tflrp');
@@ -364,4 +365,141 @@ describe('Flrp Export In P Tx Builder', () => {
364365
fullSignedTx.toBroadcastFormat().should.be.a.String();
365366
});
366367
});
368+
369+
describe('Change output threshold fix', () => {
370+
/**
371+
* This test suite verifies the fix for the change output threshold bug.
372+
*
373+
* The issue: FlareJS's pvm.e.newExportTx() defaults change outputs to threshold=1,
374+
* but for multisig wallets we need threshold=2 to maintain proper security.
375+
*
376+
* The fix: After building the transaction, we correct the change outputs to use
377+
* the wallet's threshold (typically 2 for multisig).
378+
*/
379+
380+
it('should create change output with threshold=2 for multisig wallets', async () => {
381+
const txBuilder = factory
382+
.getExportInPBuilder()
383+
.threshold(testData.threshold)
384+
.locktime(testData.locktime)
385+
.fromPubKey(testData.pAddresses)
386+
.amount(testData.amount)
387+
.externalChainId(testData.sourceChainId)
388+
.feeState(testData.feeState)
389+
.context(testData.context)
390+
.decodedUtxos(testData.utxos);
391+
392+
const tx = await txBuilder.build();
393+
394+
const flareTransaction = (tx as any)._flareTransaction as UnsignedTx;
395+
const innerTx = flareTransaction.getTx() as pvmSerial.ExportTx;
396+
const changeOutputs = innerTx.baseTx.outputs;
397+
398+
changeOutputs.length.should.be.greaterThan(0);
399+
400+
changeOutputs.forEach((output, index) => {
401+
const transferOut = output.output as TransferOutput;
402+
const threshold = transferOut.outputOwners.threshold.value();
403+
404+
threshold.should.equal(
405+
testData.threshold,
406+
`Change output ${index} should have threshold=${testData.threshold}, but has threshold=${threshold}`
407+
);
408+
});
409+
});
410+
411+
it('should create change output with correct locktime for multisig wallets', async () => {
412+
const txBuilder = factory
413+
.getExportInPBuilder()
414+
.threshold(testData.threshold)
415+
.locktime(testData.locktime)
416+
.fromPubKey(testData.pAddresses)
417+
.amount(testData.amount)
418+
.externalChainId(testData.sourceChainId)
419+
.feeState(testData.feeState)
420+
.context(testData.context)
421+
.decodedUtxos(testData.utxos);
422+
423+
const tx = await txBuilder.build();
424+
425+
const flareTransaction = (tx as any)._flareTransaction as UnsignedTx;
426+
const innerTx = flareTransaction.getTx() as pvmSerial.ExportTx;
427+
const changeOutputs = innerTx.baseTx.outputs;
428+
429+
changeOutputs.length.should.be.greaterThan(0);
430+
431+
changeOutputs.forEach((output, index) => {
432+
const transferOut = output.output as TransferOutput;
433+
const locktime = transferOut.outputOwners.locktime.value();
434+
435+
locktime.should.equal(
436+
BigInt(testData.locktime),
437+
`Change output ${index} should have locktime=${testData.locktime}, but has locktime=${locktime}`
438+
);
439+
});
440+
});
441+
442+
it('should maintain threshold=2 after parsing and rebuilding', async () => {
443+
const txBuilder = factory
444+
.getExportInPBuilder()
445+
.threshold(testData.threshold)
446+
.locktime(testData.locktime)
447+
.fromPubKey(testData.pAddresses)
448+
.amount(testData.amount)
449+
.externalChainId(testData.sourceChainId)
450+
.feeState(testData.feeState)
451+
.context(testData.context)
452+
.decodedUtxos(testData.utxos);
453+
454+
const tx = await txBuilder.build();
455+
const txHex = tx.toBroadcastFormat();
456+
457+
const parsedTxBuilder = factory.from(txHex);
458+
const parsedTx = await parsedTxBuilder.build();
459+
460+
const flareTransaction = (parsedTx as any)._flareTransaction as UnsignedTx;
461+
const innerTx = flareTransaction.getTx() as pvmSerial.ExportTx;
462+
const changeOutputs = innerTx.baseTx.outputs;
463+
464+
changeOutputs.length.should.be.greaterThan(0);
465+
466+
changeOutputs.forEach((output, index) => {
467+
const transferOut = output.output as TransferOutput;
468+
const threshold = transferOut.outputOwners.threshold.value();
469+
470+
threshold.should.equal(
471+
testData.threshold,
472+
`After parsing, change output ${index} should have threshold=${testData.threshold}, but has threshold=${threshold}`
473+
);
474+
});
475+
});
476+
477+
it('should have change output addresses matching wallet addresses', async () => {
478+
const txBuilder = factory
479+
.getExportInPBuilder()
480+
.threshold(testData.threshold)
481+
.locktime(testData.locktime)
482+
.fromPubKey(testData.pAddresses)
483+
.amount(testData.amount)
484+
.externalChainId(testData.sourceChainId)
485+
.feeState(testData.feeState)
486+
.context(testData.context)
487+
.decodedUtxos(testData.utxos);
488+
489+
const tx = await txBuilder.build();
490+
491+
const flareTransaction = (tx as any)._flareTransaction as UnsignedTx;
492+
const innerTx = flareTransaction.getTx() as pvmSerial.ExportTx;
493+
const changeOutputs = innerTx.baseTx.outputs;
494+
495+
changeOutputs.length.should.be.greaterThan(0);
496+
497+
changeOutputs.forEach((output) => {
498+
const transferOut = output.output as TransferOutput;
499+
const addresses = transferOut.outputOwners.addrs;
500+
501+
addresses.length.should.equal(3, 'Change output should have 3 addresses for multisig wallet');
502+
});
503+
});
504+
});
367505
});

0 commit comments

Comments
 (0)