Skip to content

Conversation

@mohd-kashif
Copy link
Contributor

@mohd-kashif mohd-kashif commented Jan 28, 2026

Summary: FLRP Signature Index Bug Fix

The Bug

Problem: P-chain import/export transactions for FLRP were failing on-chain with "wrong signature" errors, while identical AVAXP transactions succeeded.

Root Cause: The FlareJS library's pvm.e.newImportTx / pvm.e.newExportTx methods use matchOwners() internally to determine sigIndices (signature positions). When all 3 wallet addresses were passed to FlareJS, matchOwners() would select different signature indices than intended, causing a mismatch between:

  • Which addresses the SDK embedded signatures for
  • Which signature slots the on-chain verifier expected

Example of the mismatch:

UTXO addresses (sorted by hex on-chain):
  [0] BitGo  (hex: 3094db9d...)
  [1] Backup (hex: 7f807c5e...)  
  [2] User   (hex: c20336a9...)

Expected sigIndices for User+BitGo signing: [0, 2]
FlareJS was sometimes selecting: [0, 1] or other combinations

The Fix

Solution: Pass only the 2 intended signing addresses to FlareJS builders, not all 3 wallet addresses.

Key Changes:

  1. getSigningAddresses() method - Added to atomicTransactionBuilder.ts to determine the correct 2 signers:

    • Normal flow: User (index 0) + BitGo (index 1)
    • Recovery flow: BitGo (index 1) + Backup (index 2)
  2. Updated transaction builders (ImportInPTxBuilder.ts, ExportInPTxBuilder.ts, ImportInCTxBuilder.ts):

    • Pass only 2 signing addresses to FlareJS's newImportTx/newExportTx
    • FlareJS's matchOwners() now correctly selects the 2 signers from the UTXO's address list
  3. Change output fix (ExportInPTxBuilder.ts):

    • FlareJS was setting change outputs with only 2 addresses (the signing addresses)
    • Fixed to explicitly set all 3 wallet addresses on change outputs for proper multisig
  4. C-chain import fee (ImportInCTxBuilder.ts):

    • Ensured the 1,000,000 nFLR baseTxFee is correctly applied for EVM gas costs

How It Works in Production

Signing Flow (User signs first, BitGo signs second):

1. Wallet Platform sends transaction request with:
   - UTXOs (each with 3 addresses: user, bitgo, backup)
   - fromAddresses: [user, bitgo, backup] (convention order)
   - recoverSigner: false (normal) or true (recovery)

2. SDK determines signing addresses:
   - Normal: [user_address, bitgo_address]
   - Recovery: [bitgo_address, backup_address]

3. FlareJS builds transaction with sigIndices pointing to the
   correct 2 positions in the sorted UTXO address list

4. User signs → embedded signature placed in correct slot
   BitGo signs → finds empty slot (embedded signature matching)

5. Transaction submitted on-chain → signatures verify correctly

On-Chain Verified Transactions

These transactions were built with the fix and accepted on Flare Coston2 testnet:

Transaction Type TX ID sigIndices
Import to P-chain bgHnEJ64td8u31aZrGDaWcDqxZ8vDV5qGd7bmSifgvUnUW8v2 [0, 2]
Export from P-chain nSBwNcgfLbk5S425b1qaYaqTTCiMCV75KU4Fbnq8SPUUqLq2 [0, 2]
Import to C-chain 2t4gxEAdPLiiy9HsbjaQun1mVFewMoixNS64eZ56C38L4mpP1j [0, 2]

sigIndices [0, 2] meaning:

  • Position 0 = BitGo (hex 3094db9d... sorts first)
  • Position 2 = User (hex c20336a9... sorts third)

This matches the expected signing pair (User + BitGo) for normal transactions.


@mohd-kashif mohd-kashif self-assigned this Jan 28, 2026
@mohd-kashif mohd-kashif marked this pull request as ready for review January 28, 2026 20:40
@mohd-kashif mohd-kashif requested a review from a team as a code owner January 28, 2026 20:40
@mohd-kashif mohd-kashif requested a review from Copilot January 29, 2026 07:18
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a critical bug in FLRP (Flare P-chain) atomic transactions where import/export transactions were failing on-chain with "wrong signature" errors. The root cause was that FlareJS's matchOwners() function was selecting different signature indices than intended when all 3 wallet addresses were passed to the transaction builders.

Changes:

  • Added getSigningAddresses() method to pass only 2 signing addresses (user+BitGo or backup+BitGo) to FlareJS instead of all 3 wallet addresses
  • Fixed change output creation in ExportInPTxBuilder to ensure all 3 addresses are included for proper multisig
  • Updated test data with new transaction hashes reflecting the corrected sigIndices
  • Added on-chain verified test cases for Import P, Import C, and Export P transactions

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
modules/sdk-coin-flrp/src/lib/atomicTransactionBuilder.ts Added getSigningAddresses() method and simplified credential/addressMap creation logic
modules/sdk-coin-flrp/src/lib/ImportInPTxBuilder.ts Updated to use getSigningAddresses() and pass only 2 addresses to FlareJS
modules/sdk-coin-flrp/src/lib/ImportInCTxBuilder.ts Updated to use getSigningAddresses() and pass only 2 addresses to FlareJS
modules/sdk-coin-flrp/src/lib/ExportInPTxBuilder.ts Updated to use getSigningAddresses() for signing but all 3 addresses for change outputs
modules/sdk-coin-flrp/test/unit/lib/signatureIndex.ts Deleted comprehensive 1377-line test file with extensive signature index coverage
modules/sdk-coin-flrp/test/unit/lib/importInPTxBuilder.ts Added on-chain verified test case, removed extensive address ordering tests
modules/sdk-coin-flrp/test/unit/lib/importInCTxBuilder.ts Added on-chain verified test case, removed extensive address ordering tests
modules/sdk-coin-flrp/test/unit/lib/exportInPTxBuilder.ts Added on-chain verified test case with change output verification, removed address ordering tests
modules/sdk-coin-flrp/test/unit/flrp.ts Updated change amount assertions to match new transaction data
modules/sdk-coin-flrp/test/resources/transactionData/*.ts Updated test data with new transaction hashes and hex values
modules/sdk-coin-flrp/test/resources/account.ts Added ON_CHAIN_TEST_WALLET with verified test keys

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@mohd-kashif mohd-kashif merged commit 16c89b4 into master Jan 29, 2026
20 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants