Skip to content

Commit 2145271

Browse files
mohd-kashifmohd-kashif
authored
Merge pull request #7811 from BitGo/WIN-8279
Refactor: Centralize signature ordering logic in FLRP transaction builders
2 parents 53166e8 + cebeb07 commit 2145271

File tree

7 files changed

+844
-211
lines changed

7 files changed

+844
-211
lines changed

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

Lines changed: 27 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -110,12 +110,10 @@ export class ExportInPTxBuilder extends AtomicTransactionBuilder {
110110
this.transaction._rawSignedBytes = rawBytes;
111111
}
112112

113-
// Create proper UnsignedTx wrapper with credentials
114-
const sortedAddresses = [...this.transaction._fromAddresses].sort((a, b) => Buffer.compare(a, b));
115-
116113
// When credentials were extracted, use them directly to preserve existing signatures
117114
// Otherwise, create empty credentials with dynamic ordering based on addressesIndex
118115
// Match avaxp behavior: order depends on UTXO address positions
116+
// Use centralized method for credential creation
119117
const txCredentials =
120118
credentials.length > 0
121119
? credentials
@@ -126,39 +124,11 @@ export class ExportInPTxBuilder extends AtomicTransactionBuilder {
126124
// Get UTXO for this input to determine addressesIndex
127125
const utxo = this.transaction._utxos[inputIdx];
128126

129-
// either user (0) or recovery (2)
130-
const firstIndex = this.recoverSigner ? 2 : 0;
131-
const bitgoIndex = 1;
132-
133-
// If UTXO has addresses, compute dynamic ordering
134-
if (utxo && utxo.addresses && utxo.addresses.length > 0) {
135-
const utxoAddresses = utxo.addresses.map((a) => utils.parseAddress(a));
136-
const addressesIndex = this.transaction._fromAddresses.map((a) =>
137-
utxoAddresses.findIndex((u) => Buffer.compare(Buffer.from(u), Buffer.from(a)) === 0)
138-
);
139-
140-
// Dynamic ordering based on addressesIndex
141-
let sigSlots: ReturnType<typeof utils.createNewSig>[];
142-
if (addressesIndex[bitgoIndex] < addressesIndex[firstIndex]) {
143-
// Bitgo comes first: [zeros, userAddress]
144-
sigSlots = [
145-
utils.createNewSig(''),
146-
utils.createEmptySigWithAddress(
147-
Buffer.from(this.transaction._fromAddresses[firstIndex]).toString('hex')
148-
),
149-
];
150-
} else {
151-
// User comes first: [userAddress, zeros]
152-
sigSlots = [
153-
utils.createEmptySigWithAddress(
154-
Buffer.from(this.transaction._fromAddresses[firstIndex]).toString('hex')
155-
),
156-
utils.createNewSig(''),
157-
];
158-
}
159-
return new Credential(sigSlots);
127+
// Use centralized method, but handle case where inputThreshold might differ
128+
if (inputThreshold === this.transaction._threshold) {
129+
return this.createCredentialForUtxo(utxo, this.transaction._threshold);
160130
} else {
161-
// Fallback: use all zeros if no UTXO addresses available
131+
// Fallback: use all zeros if threshold differs (shouldn't happen normally)
162132
const sigSlots: ReturnType<typeof utils.createNewSig>[] = [];
163133
for (let i = 0; i < inputThreshold; i++) {
164134
sigSlots.push(utils.createNewSig(''));
@@ -167,15 +137,13 @@ export class ExportInPTxBuilder extends AtomicTransactionBuilder {
167137
}
168138
});
169139

170-
// Create address maps for signing - one per input/credential
171-
// Each address map contains all addresses mapped to their indices
172-
const addressMaps = txCredentials.map(() => {
173-
const addressMap = new FlareUtils.AddressMap();
174-
sortedAddresses.forEach((addr, i) => {
175-
addressMap.set(new Address(addr), i);
176-
});
177-
return addressMap;
178-
});
140+
// Create AddressMaps based on signature slot order (matching credential order), not sorted addresses
141+
// This matches the approach used in credentials: addressesIndex determines signature order
142+
// AddressMaps should map addresses to signature slots in the same order as credentials
143+
// Use centralized method for AddressMap creation
144+
const addressMaps = txCredentials.map((credential, credIdx) =>
145+
this.createAddressMapForUtxo(this.transaction._utxos[credIdx], this.transaction._threshold)
146+
);
179147

180148
// Always create a new UnsignedTx with properly structured credentials
181149
const unsignedTx = new UnsignedTx(exportTx, [], new FlareUtils.AddressMaps(addressMaps), txCredentials);
@@ -227,15 +195,13 @@ export class ExportInPTxBuilder extends AtomicTransactionBuilder {
227195
this.exportedOutputs() // exportedOutputs
228196
);
229197

230-
// Create address maps for signing - one per input/credential
231-
const sortedAddresses = [...this.transaction._fromAddresses].sort((a, b) => Buffer.compare(a, b));
232-
const addressMaps = credentials.map(() => {
233-
const addressMap = new FlareUtils.AddressMap();
234-
sortedAddresses.forEach((addr, i) => {
235-
addressMap.set(new Address(addr), i);
236-
});
237-
return addressMap;
238-
});
198+
// Create AddressMaps based on signature slot order (matching credential order), not sorted addresses
199+
// This matches the approach used in credentials: addressesIndex determines signature order
200+
// AddressMaps should map addresses to signature slots in the same order as credentials
201+
// Use centralized method for AddressMap creation
202+
const addressMaps = credentials.map((credential, credIdx) =>
203+
this.createAddressMapForUtxo(this.transaction._utxos[credIdx], this.transaction._threshold)
204+
);
239205

240206
// Create unsigned transaction
241207
const unsignedTx = new UnsignedTx(
@@ -316,39 +282,15 @@ export class ExportInPTxBuilder extends AtomicTransactionBuilder {
316282

317283
// Create credential with empty signatures for slot identification
318284
// Match avaxp behavior: dynamic ordering based on addressesIndex from UTXO
319-
const hasAddresses =
320-
this.transaction._fromAddresses && this.transaction._fromAddresses.length >= this.transaction._threshold;
321-
322-
if (!hasAddresses) {
323-
// If addresses not available, use all zeros
324-
const emptySignatures = sigIndices.map(() => utils.createNewSig(''));
325-
credentials.push(new Credential(emptySignatures));
285+
// Use centralized method for credential creation
286+
// Note: Use utxoThreshold if it differs from transaction threshold (should be rare)
287+
const thresholdToUse =
288+
utxoThreshold === this.transaction._threshold ? this.transaction._threshold : utxoThreshold;
289+
if (thresholdToUse === this.transaction._threshold) {
290+
credentials.push(this.createCredentialForUtxo(utxo, thresholdToUse));
326291
} else {
327-
// Compute addressesIndex: position of each _fromAddresses in UTXO's address list
328-
const utxoAddresses = utxo.addresses.map((a) => utils.parseAddress(a));
329-
const addressesIndex = this.transaction._fromAddresses.map((a) =>
330-
utxoAddresses.findIndex((u) => Buffer.compare(Buffer.from(u), Buffer.from(a)) === 0)
331-
);
332-
333-
// either user (0) or recovery (2)
334-
const firstIndex = this.recoverSigner ? 2 : 0;
335-
const bitgoIndex = 1;
336-
337-
// Dynamic ordering based on addressesIndex
338-
let emptySignatures: ReturnType<typeof utils.createNewSig>[];
339-
if (addressesIndex[bitgoIndex] < addressesIndex[firstIndex]) {
340-
// Bitgo comes first in signature order: [zeros, userAddress]
341-
emptySignatures = [
342-
utils.createNewSig(''),
343-
utils.createEmptySigWithAddress(Buffer.from(this.transaction._fromAddresses[firstIndex]).toString('hex')),
344-
];
345-
} else {
346-
// User comes first in signature order: [userAddress, zeros]
347-
emptySignatures = [
348-
utils.createEmptySigWithAddress(Buffer.from(this.transaction._fromAddresses[firstIndex]).toString('hex')),
349-
utils.createNewSig(''),
350-
];
351-
}
292+
// Fallback: use all zeros if threshold differs (shouldn't happen normally)
293+
const emptySignatures = sigIndices.map(() => utils.createNewSig(''));
352294
credentials.push(new Credential(emptySignatures));
353295
}
354296
}

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

Lines changed: 42 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,37 @@ export class ImportInCTxBuilder extends AtomicInCTransactionBuilder {
8787
const inputThreshold = firstInput.sigIndicies().length || this.transaction._threshold;
8888
this.transaction._threshold = inputThreshold;
8989

90-
// Create proper UnsignedTx wrapper with credentials
91-
const toAddress = new Address(output.address.toBytes());
92-
const addressMap = new FlareUtils.AddressMap([[toAddress, 0]]);
90+
// Create AddressMaps based on signature slot order (matching credential order), not sorted addresses
91+
// This matches the approach used in credentials: addressesIndex determines signature order
92+
// AddressMaps should map addresses to signature slots in the same order as credentials
93+
// If _fromAddresses is available, create AddressMap based on UTXO order (matching credential order)
94+
// Otherwise, fall back to mapping just the output address
95+
const firstUtxo = this.transaction._utxos[0];
96+
let addressMap: FlareUtils.AddressMap;
97+
if (
98+
firstUtxo &&
99+
firstUtxo.addresses &&
100+
firstUtxo.addresses.length > 0 &&
101+
this.transaction._fromAddresses &&
102+
this.transaction._fromAddresses.length >= this.transaction._threshold
103+
) {
104+
// Use centralized method for AddressMap creation
105+
addressMap = this.createAddressMapForUtxo(firstUtxo, this.transaction._threshold);
106+
} else {
107+
// Fallback: map output address to slot 0 (for C-chain imports, output is the destination)
108+
// Or map addresses sequentially if _fromAddresses is available but UTXO addresses are not
109+
addressMap = new FlareUtils.AddressMap();
110+
if (this.transaction._fromAddresses && this.transaction._fromAddresses.length >= this.transaction._threshold) {
111+
this.transaction._fromAddresses.slice(0, this.transaction._threshold).forEach((addr, i) => {
112+
addressMap.set(new Address(addr), i);
113+
});
114+
} else {
115+
// Last resort: map output address
116+
const toAddress = new Address(output.address.toBytes());
117+
addressMap.set(toAddress, 0);
118+
}
119+
}
120+
93121
const addressMaps = new FlareUtils.AddressMaps([addressMap]);
94122

95123
// When credentials were extracted, use them directly to preserve existing signatures
@@ -159,11 +187,15 @@ export class ImportInCTxBuilder extends AtomicInCTransactionBuilder {
159187
[output]
160188
);
161189

162-
// Create unsigned transaction with all potential signers in address map
163-
const addressMap = new FlareUtils.AddressMap();
164-
this.transaction._fromAddresses.forEach((addr, i) => {
165-
addressMap.set(new Address(addr), i);
166-
});
190+
// Create AddressMaps based on signature slot order (matching credential order), not sorted addresses
191+
// This matches the approach used in credentials: addressesIndex determines signature order
192+
// AddressMaps should map addresses to signature slots in the same order as credentials
193+
// For C-chain imports, we typically have one input, so use the first UTXO
194+
// Use centralized method for AddressMap creation
195+
const firstUtxo = this.transaction._utxos[0];
196+
const addressMap = firstUtxo
197+
? this.createAddressMapForUtxo(firstUtxo, this.transaction._threshold)
198+
: new FlareUtils.AddressMap();
167199
const addressMaps = new FlareUtils.AddressMaps([addressMap]);
168200

169201
const unsignedTx = new UnsignedTx(
@@ -230,41 +262,8 @@ export class ImportInCTxBuilder extends AtomicInCTransactionBuilder {
230262

231263
// Create credential with empty signatures for slot identification
232264
// Match avaxp behavior: dynamic ordering based on addressesIndex from UTXO
233-
const hasAddresses =
234-
this.transaction._fromAddresses && this.transaction._fromAddresses.length >= this.transaction._threshold;
235-
236-
if (!hasAddresses) {
237-
// If addresses not available, use all zeros
238-
const emptySignatures = sigIndices.map(() => utils.createNewSig(''));
239-
credentials.push(new Credential(emptySignatures));
240-
} else {
241-
// Compute addressesIndex: position of each _fromAddresses in UTXO's address list
242-
const utxoAddresses = utxo.addresses.map((a) => utils.parseAddress(a));
243-
const addressesIndex = this.transaction._fromAddresses.map((a) =>
244-
utxoAddresses.findIndex((u) => Buffer.compare(Buffer.from(u), Buffer.from(a)) === 0)
245-
);
246-
247-
// either user (0) or recovery (2)
248-
const firstIndex = this.recoverSigner ? 2 : 0;
249-
const bitgoIndex = 1;
250-
251-
// Dynamic ordering based on addressesIndex
252-
let emptySignatures: ReturnType<typeof utils.createNewSig>[];
253-
if (addressesIndex[bitgoIndex] < addressesIndex[firstIndex]) {
254-
// Bitgo comes first in signature order: [zeros, userAddress]
255-
emptySignatures = [
256-
utils.createNewSig(''),
257-
utils.createEmptySigWithAddress(Buffer.from(this.transaction._fromAddresses[firstIndex]).toString('hex')),
258-
];
259-
} else {
260-
// User comes first in signature order: [userAddress, zeros]
261-
emptySignatures = [
262-
utils.createEmptySigWithAddress(Buffer.from(this.transaction._fromAddresses[firstIndex]).toString('hex')),
263-
utils.createNewSig(''),
264-
];
265-
}
266-
credentials.push(new Credential(emptySignatures));
267-
}
265+
// Use centralized method for credential creation
266+
credentials.push(this.createCredentialForUtxo(utxo, this.transaction._threshold));
268267
});
269268

270269
return {

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

Lines changed: 18 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -92,55 +92,19 @@ export class ImportInPTxBuilder extends AtomicTransactionBuilder {
9292
}
9393

9494
// Create proper UnsignedTx wrapper with credentials
95-
const sortedAddresses = [...this.transaction._fromAddresses].sort((a, b) => Buffer.compare(a, b));
96-
const addressMaps = sortedAddresses.map((a, i) => new FlareUtils.AddressMap([[new Address(a), i]]));
97-
98-
// When credentials were extracted, use them directly to preserve existing signatures
9995
// Match avaxp behavior: dynamic ordering based on addressesIndex from UTXO
96+
// Use centralized methods for credential and AddressMap creation
10097
const txCredentials =
10198
credentials.length > 0
10299
? credentials
103-
: this.transaction._utxos.map((utxo) => {
104-
// either user (0) or recovery (2)
105-
const firstIndex = this.recoverSigner ? 2 : 0;
106-
const bitgoIndex = 1;
107-
108-
// If UTXO has addresses, compute dynamic ordering
109-
if (utxo && utxo.addresses && utxo.addresses.length > 0) {
110-
const utxoAddresses = utxo.addresses.map((a) => utils.parseAddress(a));
111-
const addressesIndex = this.transaction._fromAddresses.map((a) =>
112-
utxoAddresses.findIndex((u) => Buffer.compare(Buffer.from(u), Buffer.from(a)) === 0)
113-
);
114-
115-
// Dynamic ordering based on addressesIndex
116-
let sigSlots: ReturnType<typeof utils.createNewSig>[];
117-
if (addressesIndex[bitgoIndex] < addressesIndex[firstIndex]) {
118-
// Bitgo comes first: [zeros, userAddress]
119-
sigSlots = [
120-
utils.createNewSig(''),
121-
utils.createEmptySigWithAddress(
122-
Buffer.from(this.transaction._fromAddresses[firstIndex]).toString('hex')
123-
),
124-
];
125-
} else {
126-
// User comes first: [userAddress, zeros]
127-
sigSlots = [
128-
utils.createEmptySigWithAddress(
129-
Buffer.from(this.transaction._fromAddresses[firstIndex]).toString('hex')
130-
),
131-
utils.createNewSig(''),
132-
];
133-
}
134-
return new Credential(sigSlots);
135-
} else {
136-
// Fallback: use all zeros if no UTXO addresses available
137-
const sigSlots: ReturnType<typeof utils.createNewSig>[] = [];
138-
for (let i = 0; i < this.transaction._threshold; i++) {
139-
sigSlots.push(utils.createNewSig(''));
140-
}
141-
return new Credential(sigSlots);
142-
}
143-
});
100+
: this.transaction._utxos.map((utxo) => this.createCredentialForUtxo(utxo, this.transaction._threshold));
101+
102+
// Create AddressMaps based on signature slot order (matching credential order), not sorted addresses
103+
// This matches the approach used in credentials: addressesIndex determines signature order
104+
// AddressMaps should map addresses to signature slots in the same order as credentials
105+
const addressMaps = this.transaction._utxos.map((utxo) =>
106+
this.createAddressMapForUtxo(utxo, this.transaction._threshold)
107+
);
144108

145109
const unsignedTx = new UnsignedTx(importTx, [], new FlareUtils.AddressMaps(addressMaps), txCredentials);
146110

@@ -200,8 +164,13 @@ export class ImportInPTxBuilder extends AtomicTransactionBuilder {
200164
inputs // importedInputs (ins)
201165
);
202166

203-
// Create address maps for signing
204-
const addressMaps = this.transaction._fromAddresses.map((a, i) => new FlareUtils.AddressMap([[new Address(a), i]]));
167+
// Create AddressMaps based on signature slot order (matching credential order), not sorted addresses
168+
// This matches the approach used in credentials: addressesIndex determines signature order
169+
// AddressMaps should map addresses to signature slots in the same order as credentials
170+
// Use centralized method for AddressMap creation
171+
const addressMaps = credentials.map((credential, credIdx) =>
172+
this.createAddressMapForUtxo(this.transaction._utxos[credIdx], this.transaction._threshold)
173+
);
205174

206175
// Create unsigned transaction
207176
const unsignedTx = new UnsignedTx(
@@ -264,41 +233,8 @@ export class ImportInPTxBuilder extends AtomicTransactionBuilder {
264233

265234
// Create credential with empty signatures for slot identification
266235
// Match avaxp behavior: dynamic ordering based on addressesIndex from UTXO
267-
const hasAddresses =
268-
this.transaction._fromAddresses && this.transaction._fromAddresses.length >= this.transaction._threshold;
269-
270-
if (!hasAddresses) {
271-
// If addresses not available, use all zeros
272-
const emptySignatures = sigIndices.map(() => utils.createNewSig(''));
273-
credentials.push(new Credential(emptySignatures));
274-
} else {
275-
// Compute addressesIndex: position of each _fromAddresses in UTXO's address list
276-
const utxoAddresses = utxo.addresses.map((a) => utils.parseAddress(a));
277-
const addressesIndex = this.transaction._fromAddresses.map((a) =>
278-
utxoAddresses.findIndex((u) => Buffer.compare(Buffer.from(u), Buffer.from(a)) === 0)
279-
);
280-
281-
// either user (0) or recovery (2)
282-
const firstIndex = this.recoverSigner ? 2 : 0;
283-
const bitgoIndex = 1;
284-
285-
// Dynamic ordering based on addressesIndex
286-
let emptySignatures: ReturnType<typeof utils.createNewSig>[];
287-
if (addressesIndex[bitgoIndex] < addressesIndex[firstIndex]) {
288-
// Bitgo comes first in signature order: [zeros, userAddress]
289-
emptySignatures = [
290-
utils.createNewSig(''),
291-
utils.createEmptySigWithAddress(Buffer.from(this.transaction._fromAddresses[firstIndex]).toString('hex')),
292-
];
293-
} else {
294-
// User comes first in signature order: [userAddress, zeros]
295-
emptySignatures = [
296-
utils.createEmptySigWithAddress(Buffer.from(this.transaction._fromAddresses[firstIndex]).toString('hex')),
297-
utils.createNewSig(''),
298-
];
299-
}
300-
credentials.push(new Credential(emptySignatures));
301-
}
236+
// Use centralized method for credential creation
237+
credentials.push(this.createCredentialForUtxo(utxo, this.transaction._threshold));
302238
});
303239

304240
return {

0 commit comments

Comments
 (0)