Skip to content

Commit 5bb25d9

Browse files
Merge pull request #7968 from BitGo/WIN-8692
fix(sdk-coin-flrp): signature ordering according to flareJS lib
2 parents 2d816e4 + 415f7c5 commit 5bb25d9

File tree

10 files changed

+601
-19
lines changed

10 files changed

+601
-19
lines changed

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

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,16 +76,29 @@ export class ExportInPTxBuilder extends AtomicTransactionBuilder {
7676

7777
this.computeAddressesIndexFromParsed();
7878

79+
// Use parsed credentials if available, otherwise create new ones based on sigIndices
80+
// The sigIndices from the parsed transaction (stored in addressesIndex) determine
81+
// the correct credential ordering for on-chain verification
7982
const txCredentials =
8083
credentials.length > 0
8184
? credentials
8285
: this.transaction._utxos.map((utxo) => {
8386
const utxoThreshold = utxo.threshold || this.transaction._threshold;
87+
const sigIndices = utxo.addressesIndex ?? [];
88+
// Use sigIndices-based method if we have valid sigIndices from parsed transaction
89+
if (sigIndices.length >= utxoThreshold && sigIndices.every((idx) => idx >= 0)) {
90+
return this.createCredentialForUtxoWithSigIndices(utxo, utxoThreshold, sigIndices);
91+
}
8492
return this.createCredentialForUtxo(utxo, utxoThreshold);
8593
});
8694

95+
// Create addressMaps using sigIndices from parsed transaction for consistency
8796
const addressMaps = this.transaction._utxos.map((utxo) => {
8897
const utxoThreshold = utxo.threshold || this.transaction._threshold;
98+
const sigIndices = utxo.addressesIndex ?? [];
99+
if (sigIndices.length >= utxoThreshold && sigIndices.every((idx) => idx >= 0)) {
100+
return this.createAddressMapForUtxoWithSigIndices(utxo, utxoThreshold, sigIndices);
101+
}
89102
return this.createAddressMapForUtxo(utxo, utxoThreshold);
90103
});
91104

@@ -178,19 +191,27 @@ export class ExportInPTxBuilder extends AtomicTransactionBuilder {
178191
throw new BuildTransactionError(`Could not find matching UTXO for input ${inputTxid}:${inputOutputIdx}`);
179192
}
180193

194+
const transferInput = input.input as TransferInput;
195+
const actualSigIndices = transferInput.sigIndicies();
196+
181197
return {
182198
...originalUtxo,
183199
addressesIndex: originalUtxo.addressesIndex,
184200
addresses: originalUtxo.addresses,
185201
threshold: originalUtxo.threshold || this.transaction._threshold,
202+
actualSigIndices,
186203
};
187204
});
188205

189206
this.transaction._utxos = utxosWithIndex;
190207

191-
const txCredentials = utxosWithIndex.map((utxo) => this.createCredentialForUtxo(utxo, utxo.threshold));
208+
const txCredentials = utxosWithIndex.map((utxo) =>
209+
this.createCredentialForUtxoWithSigIndices(utxo, utxo.threshold, utxo.actualSigIndices)
210+
);
192211

193-
const addressMaps = utxosWithIndex.map((utxo) => this.createAddressMapForUtxo(utxo, utxo.threshold));
212+
const addressMaps = utxosWithIndex.map((utxo) =>
213+
this.createAddressMapForUtxoWithSigIndices(utxo, utxo.threshold, utxo.actualSigIndices)
214+
);
194215

195216
const fixedUnsignedTx = new UnsignedTx(innerTx, [], new FlareUtils.AddressMaps(addressMaps), txCredentials);
196217

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

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,20 +76,31 @@ export class ImportInCTxBuilder extends AtomicInCTransactionBuilder {
7676

7777
this.computeAddressesIndexFromParsed();
7878

79+
// Create addressMaps using sigIndices from parsed transaction for consistency
7980
const addressMaps = this.transaction._utxos.map((utxo) => {
8081
const utxoThreshold = utxo.threshold || this.transaction._threshold;
82+
const sigIndices = utxo.addressesIndex ?? [];
83+
if (sigIndices.length >= utxoThreshold && sigIndices.every((idx) => idx >= 0)) {
84+
return this.createAddressMapForUtxoWithSigIndices(utxo, utxoThreshold, sigIndices);
85+
}
8186
return this.createAddressMapForUtxo(utxo, utxoThreshold);
8287
});
8388

8489
const flareAddressMaps = new FlareUtils.AddressMaps(addressMaps);
8590

91+
// Use parsed credentials if available, otherwise create new ones based on sigIndices
8692
let txCredentials: Credential[];
8793
if (credentials.length > 0) {
8894
txCredentials = credentials;
8995
} else {
90-
txCredentials = this.transaction._utxos.map((utxo) =>
91-
this.createCredentialForUtxo(utxo, utxo.threshold || this.transaction._threshold)
92-
);
96+
txCredentials = this.transaction._utxos.map((utxo) => {
97+
const utxoThreshold = utxo.threshold || this.transaction._threshold;
98+
const sigIndices = utxo.addressesIndex ?? [];
99+
if (sigIndices.length >= utxoThreshold && sigIndices.every((idx) => idx >= 0)) {
100+
return this.createCredentialForUtxoWithSigIndices(utxo, utxoThreshold, sigIndices);
101+
}
102+
return this.createCredentialForUtxo(utxo, utxoThreshold);
103+
});
93104
}
94105

95106
const unsignedTx = new UnsignedTx(baseTx, [], flareAddressMaps, txCredentials);
@@ -178,19 +189,27 @@ export class ImportInCTxBuilder extends AtomicInCTransactionBuilder {
178189
throw new BuildTransactionError(`Could not find matching UTXO for input ${inputTxid}:${inputOutputIdx}`);
179190
}
180191

192+
const transferInput = input.input as TransferInput;
193+
const actualSigIndices = transferInput.sigIndicies();
194+
181195
return {
182196
...originalUtxo,
183197
addressesIndex: originalUtxo.addressesIndex,
184198
addresses: originalUtxo.addresses,
185199
threshold: originalUtxo.threshold || this.transaction._threshold,
200+
actualSigIndices,
186201
};
187202
});
188203

189204
this.transaction._utxos = utxosWithIndex;
190205

191-
const txCredentials = utxosWithIndex.map((utxo) => this.createCredentialForUtxo(utxo, utxo.threshold));
206+
const txCredentials = utxosWithIndex.map((utxo) =>
207+
this.createCredentialForUtxoWithSigIndices(utxo, utxo.threshold, utxo.actualSigIndices)
208+
);
192209

193-
const addressMaps = utxosWithIndex.map((utxo) => this.createAddressMapForUtxo(utxo, utxo.threshold));
210+
const addressMaps = utxosWithIndex.map((utxo) =>
211+
this.createAddressMapForUtxoWithSigIndices(utxo, utxo.threshold, utxo.actualSigIndices)
212+
);
194213

195214
const fixedUnsignedTx = new UnsignedTx(innerTx, [], new FlareUtils.AddressMaps(addressMaps), txCredentials);
196215

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

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,16 +95,29 @@ export class ImportInPTxBuilder extends AtomicTransactionBuilder {
9595

9696
this.computeAddressesIndexFromParsed();
9797

98+
// Use parsed credentials if available, otherwise create new ones based on sigIndices
99+
// The sigIndices from the parsed transaction (stored in addressesIndex) determine
100+
// the correct credential ordering for on-chain verification
98101
const txCredentials =
99102
credentials.length > 0
100103
? credentials
101104
: this.transaction._utxos.map((utxo) => {
102105
const utxoThreshold = utxo.threshold || this.transaction._threshold;
106+
const sigIndices = utxo.addressesIndex ?? [];
107+
// Use sigIndices-based method if we have valid sigIndices from parsed transaction
108+
if (sigIndices.length >= utxoThreshold && sigIndices.every((idx) => idx >= 0)) {
109+
return this.createCredentialForUtxoWithSigIndices(utxo, utxoThreshold, sigIndices);
110+
}
103111
return this.createCredentialForUtxo(utxo, utxoThreshold);
104112
});
105113

114+
// Create addressMaps using sigIndices from parsed transaction for consistency
106115
const addressMaps = this.transaction._utxos.map((utxo) => {
107116
const utxoThreshold = utxo.threshold || this.transaction._threshold;
117+
const sigIndices = utxo.addressesIndex ?? [];
118+
if (sigIndices.length >= utxoThreshold && sigIndices.every((idx) => idx >= 0)) {
119+
return this.createAddressMapForUtxoWithSigIndices(utxo, utxoThreshold, sigIndices);
120+
}
108121
return this.createAddressMapForUtxo(utxo, utxoThreshold);
109122
});
110123

@@ -209,19 +222,27 @@ export class ImportInPTxBuilder extends AtomicTransactionBuilder {
209222
throw new BuildTransactionError(`Could not find matching UTXO for input ${inputTxid}:${inputOutputIdx}`);
210223
}
211224

225+
const transferInput = input.input as TransferInput;
226+
const actualSigIndices = transferInput.sigIndicies();
227+
212228
return {
213229
...originalUtxo,
214230
addressesIndex: originalUtxo.addressesIndex,
215231
addresses: originalUtxo.addresses,
216232
threshold: originalUtxo.threshold || this.transaction._threshold,
233+
actualSigIndices,
217234
};
218235
});
219236

220237
this.transaction._utxos = utxosWithIndex;
221238

222-
const txCredentials = utxosWithIndex.map((utxo) => this.createCredentialForUtxo(utxo, utxo.threshold));
239+
const txCredentials = utxosWithIndex.map((utxo) =>
240+
this.createCredentialForUtxoWithSigIndices(utxo, utxo.threshold, utxo.actualSigIndices)
241+
);
223242

224-
const addressMaps = utxosWithIndex.map((utxo) => this.createAddressMapForUtxo(utxo, utxo.threshold));
243+
const addressMaps = utxosWithIndex.map((utxo) =>
244+
this.createAddressMapForUtxoWithSigIndices(utxo, utxo.threshold, utxo.actualSigIndices)
245+
);
225246

226247
const fixedUnsignedTx = new UnsignedTx(innerTx, [], new FlareUtils.AddressMaps(addressMaps), txCredentials);
227248

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

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,4 +288,128 @@ export abstract class AtomicTransactionBuilder extends TransactionBuilder {
288288

289289
return addressMap;
290290
}
291+
292+
/**
293+
* Create credential using the ACTUAL sigIndices from FlareJS.
294+
*
295+
* This method determines which sender addresses correspond to which sigIndex positions,
296+
* then creates the credential with signatures in the correct order matching the sigIndices.
297+
*
298+
* sigIndices tell us which positions in the UTXO's owner addresses need to sign.
299+
* We need to figure out which sender addresses are at those positions and create
300+
* signature slots in the same order as sigIndices.
301+
*
302+
* @param utxo - The UTXO to create credential for
303+
* @param threshold - Number of signatures required
304+
* @param actualSigIndices - The actual sigIndices from FlareJS's built input
305+
* @returns Credential with signatures ordered to match sigIndices
306+
* @protected
307+
*/
308+
protected createCredentialForUtxoWithSigIndices(
309+
utxo: DecodedUtxoObj,
310+
threshold: number,
311+
actualSigIndices: number[]
312+
): Credential {
313+
const sender = this.transaction._fromAddresses;
314+
const addressesIndex = utxo.addressesIndex ?? [];
315+
316+
// either user (0) or recovery (2)
317+
const firstIndex = this.recoverSigner ? 2 : 0;
318+
319+
if (threshold === 1) {
320+
if (sender && sender.length > firstIndex && addressesIndex[firstIndex] !== undefined) {
321+
return new Credential([utils.createEmptySigWithAddress(Buffer.from(sender[firstIndex]).toString('hex'))]);
322+
}
323+
return new Credential([utils.createNewSig('')]);
324+
}
325+
326+
// For threshold >= 2, use the actual sigIndices order from FlareJS
327+
// sigIndices[i] = position in UTXO's owner addresses that needs to sign
328+
// addressesIndex[senderIdx] = position in UTXO's owner addresses for that sender
329+
//
330+
// We need to find which sender corresponds to each sigIndex and create signatures
331+
// in the sigIndices order.
332+
if (actualSigIndices.length >= 2 && addressesIndex.length >= 2 && sender && sender.length >= threshold) {
333+
const emptySignatures: ReturnType<typeof utils.createNewSig>[] = [];
334+
335+
for (const sigIdx of actualSigIndices) {
336+
// Find which sender address is at this UTXO position
337+
// addressesIndex[senderIdx] tells us which UTXO position each sender is at
338+
const senderIdx = addressesIndex.findIndex((utxoPos) => utxoPos === sigIdx);
339+
340+
if (senderIdx === firstIndex) {
341+
// This sigIndex slot is for user/recovery - embed their address
342+
emptySignatures.push(utils.createEmptySigWithAddress(Buffer.from(sender[firstIndex]).toString('hex')));
343+
} else {
344+
// BitGo (HSM) or unknown sender - empty signature
345+
emptySignatures.push(utils.createNewSig(''));
346+
}
347+
}
348+
349+
return new Credential(emptySignatures);
350+
}
351+
352+
// Fallback: create threshold empty signatures
353+
const emptySignatures: ReturnType<typeof utils.createNewSig>[] = [];
354+
for (let i = 0; i < threshold; i++) {
355+
emptySignatures.push(utils.createNewSig(''));
356+
}
357+
return new Credential(emptySignatures);
358+
}
359+
360+
/**
361+
* Create AddressMap using the ACTUAL sigIndices from FlareJS.
362+
*
363+
* Maps sender addresses to signature slots based on the actual sigIndices order.
364+
*
365+
* @param utxo - The UTXO to create AddressMap for
366+
* @param threshold - Number of signatures required
367+
* @param actualSigIndices - The actual sigIndices from FlareJS's built input
368+
* @returns AddressMap that maps addresses to signature slots
369+
* @protected
370+
*/
371+
protected createAddressMapForUtxoWithSigIndices(
372+
utxo: DecodedUtxoObj,
373+
threshold: number,
374+
actualSigIndices: number[]
375+
): FlareUtils.AddressMap {
376+
const addressMap = new FlareUtils.AddressMap();
377+
const sender = this.transaction._fromAddresses;
378+
const addressesIndex = utxo.addressesIndex ?? [];
379+
380+
const firstIndex = this.recoverSigner ? 2 : 0;
381+
const bitgoIndex = 1;
382+
383+
if (threshold === 1) {
384+
if (sender && sender.length > firstIndex) {
385+
addressMap.set(new Address(sender[firstIndex]), 0);
386+
} else if (sender && sender.length > 0) {
387+
addressMap.set(new Address(sender[0]), 0);
388+
}
389+
return addressMap;
390+
}
391+
392+
// For threshold >= 2, map addresses based on actual sigIndices order
393+
if (actualSigIndices.length >= 2 && addressesIndex.length >= 2 && sender && sender.length >= threshold) {
394+
actualSigIndices.forEach((sigIdx, slotIdx) => {
395+
// Find which sender is at this UTXO position
396+
const senderIdx = addressesIndex.findIndex((utxoPos) => utxoPos === sigIdx);
397+
398+
if (senderIdx === bitgoIndex || senderIdx === firstIndex) {
399+
addressMap.set(new Address(sender[senderIdx]), slotIdx);
400+
}
401+
});
402+
403+
return addressMap;
404+
}
405+
406+
// Fallback
407+
if (sender && sender.length >= threshold) {
408+
sender.slice(0, threshold).forEach((addr, i) => {
409+
addressMap.set(new Address(addr), i);
410+
});
411+
}
412+
413+
return addressMap;
414+
}
291415
}

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,16 @@ export class Transaction extends BaseTransaction {
182182
const signature = await secp256k1.sign(unsignedBytes, prv);
183183
let signatureSet = false;
184184

185-
if (hasMatchingAddress) {
185+
// Check if any credential has embedded addresses - if so, we MUST use address-based matching
186+
const hasEmbeddedAddressesInCredentials = unsignedTx.credentials.some((credential) => {
187+
const signatures = credential.getSignatures();
188+
return signatures.some((sig) => isEmptySignature(sig) && hasEmbeddedAddress(sig));
189+
});
190+
191+
// Use address-based slot matching if:
192+
// 1. addressMap contains matching address, OR
193+
// 2. credentials have embedded addresses (prioritize this to ensure correct slot placement)
194+
if (hasMatchingAddress || hasEmbeddedAddressesInCredentials) {
186195
// Use address-based slot matching (like AVAX-P)
187196
let checkSign: CheckSignature | undefined = undefined;
188197

@@ -209,8 +218,9 @@ export class Transaction extends BaseTransaction {
209218
}
210219

211220
// Fallback: If address-based matching didn't work (e.g., ImportInC loaded from unsigned tx
212-
// where P-chain addresses aren't in addressMaps), sign ALL empty slots across ALL credentials.
213-
// This handles multisig where each UTXO needs a credential signed by the same key.
221+
// where P-chain addresses aren't in addressMaps AND no embedded addresses), sign ALL empty
222+
// slots across ALL credentials. This handles multisig where each UTXO needs a credential
223+
// signed by the same key.
214224
if (!signatureSet) {
215225
for (const credential of unsignedTx.credentials) {
216226
const signatures = credential.getSignatures();

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,8 +377,14 @@ export class Utils implements BaseUtils {
377377
/**
378378
* Sort addresses lexicographically by their byte representation.
379379
* This matches how addresses are stored on-chain in Avalanche/Flare P-chain UTXOs.
380+
*
381+
* IMPORTANT: This sorting MUST be consistent with FlareJS's internal address sorting.
382+
* FlareJS uses the same lexicographic comparison: `hexA.localeCompare(hexB)`.
383+
* The sigIndices in transaction inputs depend on this sorted order, so any deviation
384+
* would cause signature order mismatches and on-chain verification failures.
385+
*
380386
* @param addresses - Array of bech32 address strings (e.g., "P-costwo1...")
381-
* @returns Array of addresses sorted by hex value
387+
* @returns Array of addresses sorted by hex value (ascending lexicographic order)
382388
*/
383389
public sortAddressesByHex(addresses: string[]): string[] {
384390
return [...addresses].sort((a, b) => {

0 commit comments

Comments
 (0)