Skip to content

Commit a8e7402

Browse files
mohd-kashifmohd-kashif
authored
Merge pull request #7964 from BitGo/WIN-8686-utxo-sorting
feat(sdk-coin-flrp): implement UTXO address sorting to ensure signature verification
2 parents f8886d1 + 003ec57 commit a8e7402

9 files changed

Lines changed: 672 additions & 57 deletions

File tree

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

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -233,18 +233,4 @@ export class ExportInPTxBuilder extends AtomicTransactionBuilder {
233233
return utxo;
234234
});
235235
}
236-
237-
private computeAddressesIndexFromParsed(): void {
238-
const sender = this.transaction._fromAddresses;
239-
if (!sender || sender.length === 0) return;
240-
241-
this.transaction._utxos.forEach((utxo) => {
242-
if (utxo.addresses && utxo.addresses.length > 0) {
243-
const utxoAddresses = utxo.addresses.map((a) => utils.parseAddress(a));
244-
utxo.addressesIndex = sender.map((senderAddr) =>
245-
utxoAddresses.findIndex((utxoAddr) => Buffer.compare(Buffer.from(utxoAddr), Buffer.from(senderAddr)) === 0)
246-
);
247-
}
248-
});
249-
}
250236
}

‎modules/sdk-coin-flrp/src/lib/ImportInCTxBuilder.ts‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -230,18 +230,4 @@ export class ImportInCTxBuilder extends AtomicInCTransactionBuilder {
230230
};
231231
});
232232
}
233-
234-
private computeAddressesIndexFromParsed(): void {
235-
const sender = this.transaction._fromAddresses;
236-
if (!sender || sender.length === 0) return;
237-
238-
this.transaction._utxos.forEach((utxo) => {
239-
if (utxo.addresses && utxo.addresses.length > 0) {
240-
const utxoAddresses = utxo.addresses.map((a) => utils.parseAddress(a));
241-
utxo.addressesIndex = sender.map((senderAddr) =>
242-
utxoAddresses.findIndex((utxoAddr) => Buffer.compare(Buffer.from(utxoAddr), Buffer.from(senderAddr)) === 0)
243-
);
244-
}
245-
});
246-
}
247233
}

‎modules/sdk-coin-flrp/src/lib/ImportInPTxBuilder.ts‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -268,27 +268,4 @@ export class ImportInPTxBuilder extends AtomicTransactionBuilder {
268268
return utxo;
269269
});
270270
}
271-
272-
/**
273-
* Compute proper addressesIndex from parsed transaction's sigIndicies.
274-
* Following AVAX P approach: addressesIndex[senderIdx] = position of sender in UTXO addresses.
275-
*
276-
* For parsed transactions:
277-
* - sigIndicies tells us which UTXO positions need to sign for each slot
278-
* - We use output addresses as proxy for UTXO addresses
279-
* - We compute addressesIndex as sender -> utxo position mapping
280-
*/
281-
private computeAddressesIndexFromParsed(): void {
282-
const sender = this.transaction._fromAddresses;
283-
if (!sender || sender.length === 0) return;
284-
285-
this.transaction._utxos.forEach((utxo) => {
286-
if (utxo.addresses && utxo.addresses.length > 0) {
287-
const utxoAddresses = utxo.addresses.map((a) => utils.parseAddress(a));
288-
utxo.addressesIndex = sender.map((senderAddr) =>
289-
utxoAddresses.findIndex((utxoAddr) => Buffer.compare(Buffer.from(utxoAddr), Buffer.from(senderAddr)) === 0)
290-
);
291-
}
292-
});
293-
}
294271
}

‎modules/sdk-coin-flrp/src/lib/atomicTransactionBuilder.ts‎

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,12 +105,17 @@ export abstract class AtomicTransactionBuilder extends TransactionBuilder {
105105
* Compute addressesIndex for UTXOs following AVAX P approach.
106106
* addressesIndex[senderIdx] = position of sender[senderIdx] in UTXO's address list
107107
*
108+
* IMPORTANT: UTXO addresses are sorted lexicographically by byte value to match
109+
* on-chain storage order. The API may return addresses in arbitrary order, but
110+
* on-chain UTXOs always store addresses in sorted order.
111+
*
108112
* Example:
109113
* A = user key, B = hsm key, C = backup key
110114
* sender (bitgoAddresses) = [ A, B, C ]
111-
* utxo.addresses (IMS addresses) = [ B, C, A ]
112-
* addressesIndex = [ 2, 0, 1 ]
113-
* (sender[0]=A is at position 2 in UTXO, sender[1]=B is at position 0, etc.)
115+
* utxo.addresses (from API) = [ B, C, A ]
116+
* sorted utxo.addresses = [ A, B, C ] (sorted by hex value)
117+
* addressesIndex = [ 0, 1, 2 ]
118+
* (sender[0]=A is at position 0 in sorted UTXO, sender[1]=B is at position 1, etc.)
114119
*
115120
* @protected
116121
*/
@@ -123,14 +128,44 @@ export abstract class AtomicTransactionBuilder extends TransactionBuilder {
123128
}
124129

125130
if (utxo.addresses && utxo.addresses.length > 0) {
126-
const utxoAddresses = utxo.addresses.map((a) => utils.parseAddress(a));
131+
const sortedAddresses = utils.sortAddressesByHex(utxo.addresses);
132+
utxo.addresses = sortedAddresses;
133+
134+
const utxoAddresses = sortedAddresses.map((a) => utils.parseAddress(a));
127135
utxo.addressesIndex = sender.map((a) =>
128136
utxoAddresses.findIndex((u) => Buffer.compare(Buffer.from(u), Buffer.from(a)) === 0)
129137
);
130138
}
131139
});
132140
}
133141

142+
/**
143+
* Compute addressesIndex from parsed transaction data.
144+
* Similar to computeAddressesIndex() but used when parsing existing transactions
145+
* via initBuilder().
146+
*
147+
* IMPORTANT: UTXO addresses are sorted lexicographically by byte value to match
148+
* on-chain storage order, ensuring consistency with fresh builds.
149+
*
150+
* @protected
151+
*/
152+
protected computeAddressesIndexFromParsed(): void {
153+
const sender = this.transaction._fromAddresses;
154+
if (!sender || sender.length === 0) return;
155+
156+
this.transaction._utxos.forEach((utxo) => {
157+
if (utxo.addresses && utxo.addresses.length > 0) {
158+
const sortedAddresses = utils.sortAddressesByHex(utxo.addresses);
159+
utxo.addresses = sortedAddresses;
160+
161+
const utxoAddresses = sortedAddresses.map((a) => utils.parseAddress(a));
162+
utxo.addressesIndex = sender.map((senderAddr) =>
163+
utxoAddresses.findIndex((utxoAddr) => Buffer.compare(Buffer.from(utxoAddr), Buffer.from(senderAddr)) === 0)
164+
);
165+
}
166+
});
167+
}
168+
134169
/**
135170
* Validate UTXOs have consistent addresses.
136171
* Note: UTXO threshold can differ from transaction threshold - each UTXO has its own

‎modules/sdk-coin-flrp/src/lib/utils.ts‎

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,32 @@ export class Utils implements BaseUtils {
374374
return new Id(Buffer.from(value, 'hex'));
375375
}
376376

377+
/**
378+
* Sort addresses lexicographically by their byte representation.
379+
* This matches how addresses are stored on-chain in Avalanche/Flare P-chain UTXOs.
380+
* @param addresses - Array of bech32 address strings (e.g., "P-costwo1...")
381+
* @returns Array of addresses sorted by hex value
382+
*/
383+
public sortAddressesByHex(addresses: string[]): string[] {
384+
return [...addresses].sort((a, b) => {
385+
const aHex = this.parseAddress(a).toString('hex');
386+
const bHex = this.parseAddress(b).toString('hex');
387+
return aHex.localeCompare(bHex);
388+
});
389+
}
390+
391+
/**
392+
* Sort address buffers lexicographically by their byte representation.
393+
* This matches how addresses are stored on-chain in Avalanche/Flare P-chain UTXOs.
394+
* @param addressBuffers - Array of address byte buffers
395+
* @returns Array of address buffers sorted by hex value
396+
*/
397+
public sortAddressBuffersByHex(addressBuffers: Buffer[]): Buffer[] {
398+
return [...addressBuffers].sort((a, b) => {
399+
return a.toString('hex').localeCompare(b.toString('hex'));
400+
});
401+
}
402+
377403
/**
378404
* Recover public key from signature
379405
* @param messageHash - The SHA256 hash of the message (e.g., signablePayload)
@@ -496,6 +522,11 @@ export class Utils implements BaseUtils {
496522
/**
497523
* Convert DecodedUtxoObj to native FlareJS Utxo object
498524
* This is the reverse of utxoToDecoded
525+
*
526+
* IMPORTANT: Addresses are sorted lexicographically by byte value to match
527+
* on-chain storage order. The API may return addresses in arbitrary order, but
528+
* on-chain UTXOs always store addresses in sorted order.
529+
*
499530
* @param decoded - DecodedUtxoObj to convert
500531
* @param assetId - Asset ID as cb58 encoded string
501532
* @returns Native FlareJS Utxo object
@@ -507,9 +538,14 @@ export class Utils implements BaseUtils {
507538
// Parse addresses from bech32 strings to byte buffers
508539
const addressBytes = decoded.addresses.map((addr) => this.parseAddress(addr));
509540

510-
// Create OutputOwners with locktime, threshold, and addresses
541+
// Sort addresses lexicographically by byte value to match on-chain order
542+
// This is critical because the P-chain stores addresses in sorted order,
543+
// and sigIndices must reference the correct positions.
544+
const sortedAddressBytes = this.sortAddressBuffersByHex(addressBytes);
545+
546+
// Create OutputOwners with locktime, threshold, and SORTED addresses
511547
const locktime = decoded.locktime ? BigInt(decoded.locktime) : BigInt(0);
512-
const outputOwners = OutputOwners.fromNative(addressBytes, locktime, decoded.threshold);
548+
const outputOwners = OutputOwners.fromNative(sortedAddressBytes, locktime, decoded.threshold);
513549

514550
// Create TransferOutput with amount and owners
515551
const amount = BigInt(decoded.amount);

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

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,141 @@ describe('Flrp Export In P Tx Builder', () => {
148148
});
149149
});
150150

151+
describe('UTXO address sorting fix - addresses in non-sorted order for ExportInP', () => {
152+
/**
153+
* This test suite verifies the fix for the address ordering bug in ExportInP.
154+
*
155+
* The issue: When the API returns UTXO addresses in a different order than how they're
156+
* stored on-chain (lexicographically sorted by byte value), the sigIndices would be
157+
* computed incorrectly, causing signature verification to fail.
158+
*
159+
* The fix: Sort UTXO addresses before computing addressesIndex to match on-chain order.
160+
*/
161+
162+
// Helper to create UTXO with specific address order
163+
const createUtxoWithAddressOrder = (utxo: (typeof testData.utxos)[0], addresses: string[]) => ({
164+
...utxo,
165+
addresses: addresses,
166+
});
167+
168+
it('should correctly sort UTXO addresses when building ExportInP transaction', async () => {
169+
// Create UTXOs with addresses in reversed order (simulating API returning unsorted)
170+
const reversedUtxos = testData.utxos.map((utxo) =>
171+
createUtxoWithAddressOrder(utxo, [...utxo.addresses].reverse())
172+
);
173+
174+
const txBuilder = factory
175+
.getExportInPBuilder()
176+
.threshold(testData.threshold)
177+
.locktime(testData.locktime)
178+
.fromPubKey(testData.pAddresses)
179+
.amount(testData.amount)
180+
.externalChainId(testData.sourceChainId)
181+
.feeState(testData.feeState)
182+
.context(testData.context)
183+
.decodedUtxos(reversedUtxos);
184+
185+
// Should not throw - the fix ensures addresses are sorted before computing sigIndices
186+
const tx = await txBuilder.build();
187+
const txJson = tx.toJson();
188+
189+
txJson.type.should.equal(22); // Export In P type
190+
txJson.threshold.should.equal(2);
191+
});
192+
193+
it('should produce same transaction hex regardless of input UTXO address order for ExportInP', async () => {
194+
// Build with original address order
195+
const txBuilder1 = factory
196+
.getExportInPBuilder()
197+
.threshold(testData.threshold)
198+
.locktime(testData.locktime)
199+
.fromPubKey(testData.pAddresses)
200+
.amount(testData.amount)
201+
.externalChainId(testData.sourceChainId)
202+
.feeState(testData.feeState)
203+
.context(testData.context)
204+
.decodedUtxos(testData.utxos);
205+
206+
const tx1 = await txBuilder1.build();
207+
const hex1 = tx1.toBroadcastFormat();
208+
209+
// Build with reversed address order in UTXOs
210+
const reversedUtxos = testData.utxos.map((utxo) =>
211+
createUtxoWithAddressOrder(utxo, [...utxo.addresses].reverse())
212+
);
213+
214+
const txBuilder2 = factory
215+
.getExportInPBuilder()
216+
.threshold(testData.threshold)
217+
.locktime(testData.locktime)
218+
.fromPubKey(testData.pAddresses)
219+
.amount(testData.amount)
220+
.externalChainId(testData.sourceChainId)
221+
.feeState(testData.feeState)
222+
.context(testData.context)
223+
.decodedUtxos(reversedUtxos);
224+
225+
const tx2 = await txBuilder2.build();
226+
const hex2 = tx2.toBroadcastFormat();
227+
228+
// Both should produce the same hex since addresses get sorted
229+
hex1.should.equal(hex2);
230+
});
231+
232+
it('should handle signing correctly with unsorted UTXO addresses for ExportInP', async () => {
233+
const reversedUtxos = testData.utxos.map((utxo) =>
234+
createUtxoWithAddressOrder(utxo, [...utxo.addresses].reverse())
235+
);
236+
237+
const txBuilder = factory
238+
.getExportInPBuilder()
239+
.threshold(testData.threshold)
240+
.locktime(testData.locktime)
241+
.fromPubKey(testData.pAddresses)
242+
.amount(testData.amount)
243+
.externalChainId(testData.sourceChainId)
244+
.feeState(testData.feeState)
245+
.context(testData.context)
246+
.decodedUtxos(reversedUtxos);
247+
248+
txBuilder.sign({ key: testData.privateKeys[2] });
249+
txBuilder.sign({ key: testData.privateKeys[0] });
250+
251+
const tx = await txBuilder.build();
252+
const txJson = tx.toJson();
253+
254+
// Should have signatures after signing (count depends on UTXO thresholds)
255+
txJson.signatures.length.should.be.greaterThan(0);
256+
tx.toBroadcastFormat().should.be.a.String();
257+
});
258+
259+
it('should produce valid signed transaction matching expected output with unsorted addresses for ExportInP', async () => {
260+
const reversedUtxos = testData.utxos.map((utxo) =>
261+
createUtxoWithAddressOrder(utxo, [...utxo.addresses].reverse())
262+
);
263+
264+
const txBuilder = factory
265+
.getExportInPBuilder()
266+
.threshold(testData.threshold)
267+
.locktime(testData.locktime)
268+
.fromPubKey(testData.pAddresses)
269+
.amount(testData.amount)
270+
.externalChainId(testData.sourceChainId)
271+
.feeState(testData.feeState)
272+
.context(testData.context)
273+
.decodedUtxos(reversedUtxos);
274+
275+
txBuilder.sign({ key: testData.privateKeys[2] });
276+
txBuilder.sign({ key: testData.privateKeys[0] });
277+
278+
const tx = await txBuilder.build();
279+
280+
// The signed tx should match the expected fullSigntxHex from testData
281+
tx.toBroadcastFormat().should.equal(testData.fullSigntxHex);
282+
tx.id.should.equal(testData.txhash);
283+
});
284+
});
285+
151286
describe('addressesIndex extraction and signature ordering', () => {
152287
it('should extract addressesIndex from parsed transaction inputs', async () => {
153288
const txBuilder = factory.from(testData.halfSigntxHex);

0 commit comments

Comments
 (0)