Merge pull request #7823 from BitGo/WIN-8488

refactor(sdk-coin-flrp): update import transaction fee calculation method

Author: mohd-kashif

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

@@ -63,7 +63,8 @@ export class ImportInCTxBuilder extends AtomicInCTransactionBuilder {
 
     // Calculate fee based on input/output difference
     const fee = totalInputAmount - totalOutputAmount;
-    const feeSize = this.calculateFeeSize(baseTx);
+    // Calculate cost units using the same method as buildFlareTransaction
+    const feeSize = this.calculateImportCost(baseTx);
     // Use integer division to ensure feeRate can be converted back to BigInt
     const feeRate = Math.floor(Number(fee) / feeSize);
 
@@ -165,7 +166,7 @@ export class ImportInCTxBuilder extends AtomicInCTransactionBuilder {
     const { inputs, amount, credentials } = this.createInputs();
 
     // Calculate import cost units (matching AVAXP's costImportTx approach)
-    // Create a temporary transaction to calculate the actual cost units
+    // Create a temporary transaction with full amount to calculate fee size
     const tempOutput = new evmSerial.Output(
       new Address(this.transaction._to[0]),
       new BigIntPr(amount),
@@ -179,13 +180,18 @@ export class ImportInCTxBuilder extends AtomicInCTransactionBuilder {
       [tempOutput]
     );
 
-    // Calculate the import cost units (matches AVAXP's feeSize from costImportTx)
+    // Calculate feeSize once using full amount (matching AVAXP approach)
     const feeSize = this.calculateImportCost(tempImportTx);
-
-    // Multiply feeRate by cost units (matching AVAXP: fee = feeRate.muln(feeSize))
     const feeRate = BigInt(this.transaction._fee.feeRate);
     const fee = feeRate * BigInt(feeSize);
 
+    // Validate that we have enough funds to cover the fee
+    if (amount <= fee) {
+      throw new BuildTransactionError(
+        `Insufficient funds: have ${amount.toString()}, need more than ${fee.toString()} for fee`
+      );
+    }
+
     this.transaction._fee.fee = fee.toString();
     this.transaction._fee.size = feeSize;
 
@@ -327,21 +333,6 @@ export class ImportInCTxBuilder extends AtomicInCTransactionBuilder {
     return totalCost;
   }
 
-  /**
-   * Calculate the fee size for the transaction (for backwards compatibility)
-   * For C-chain imports, the feeRate is treated as an absolute fee value
-   */
-  private calculateFeeSize(tx?: evmSerial.ImportTx): number {
-    // If tx is provided, calculate based on actual transaction size
-    if (tx) {
-      const codec = avmSerial.getAVMManager().getDefaultCodec();
-      return tx.toBytes(codec).length;
-    }
-
-    // For C-chain imports, treat feeRate as the absolute fee (multiplier of 1)
-    return 1;
-  }
-
   /**
    * Recover UTXOs from imported inputs
    * @param importedInputs Array of transferable inputs

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

@@ -115,6 +115,130 @@ describe('Flrp Import In C Tx Builder', () => {
       const calculatedOutput = inputAmount - calculatedFee;
       assert(outputAmount === calculatedOutput, 'Output should equal input minus total fee');
     });
+
+    it('should use consistent fee calculation in initBuilder and buildFlareTransaction', async () => {
+      const inputAmount = '100000000'; // 100M nanoFLRP (matches real-world transaction)
+      const expectedFeeRate = 500; // Real feeRate from working transaction
+      const threshold = 2;
+
+      const utxo: DecodedUtxoObj = {
+        outputID: 0,
+        amount: inputAmount,
+        txid: '2vPMx8P63adgBae7GAWFx7qvJDwRmMnDCyKddHRBXWhysjX4BP',
+        outputidx: '0',
+        addresses: [
+          '0x3329be7d01cd3ebaae6654d7327dd9f17a2e1581',
+          '0x7e918a5e8083ae4c9f2f0ed77055c24bf3665001',
+          '0xc7324437c96c7c8a6a152da2385c1db5c3ab1f91',
+        ],
+        threshold: threshold,
+      };
+
+      const txBuilder = factory
+        .getImportInCBuilder()
+        .threshold(threshold)
+        .fromPubKey(testData.pAddresses)
+        .utxos([utxo])
+        .to(testData.to)
+        .feeRate(expectedFeeRate.toString());
+
+      const tx = await txBuilder.build();
+      const calculatedFee = BigInt((tx as any).fee.fee);
+      const feeInfo = (tx as any).fee;
+
+      const maxReasonableFee = BigInt(inputAmount) / BigInt(10); // Max 10% of input
+      assert(
+        calculatedFee < maxReasonableFee,
+        `Fee ${calculatedFee} should be less than 10% of input (${maxReasonableFee})`
+      );
+
+      const expectedMinFee = BigInt(expectedFeeRate) * BigInt(12000);
+      const expectedMaxFee = BigInt(expectedFeeRate) * BigInt(13000);
+
+      assert(calculatedFee >= expectedMinFee, `Fee ${calculatedFee} should be at least ${expectedMinFee}`);
+      assert(calculatedFee <= expectedMaxFee, `Fee ${calculatedFee} should not exceed ${expectedMaxFee}`);
+
+      const outputAmount = BigInt(tx.outputs[0].value);
+      assert(outputAmount > BigInt(0), 'Output should be positive');
+
+      const expectedOutput = BigInt(inputAmount) - calculatedFee;
+      assert(
+        outputAmount === expectedOutput,
+        `Output ${outputAmount} should equal input ${inputAmount} minus fee ${calculatedFee}`
+      );
+
+      const txHex = tx.toBroadcastFormat();
+      const parsedBuilder = factory.from(txHex);
+      const parsedTx = await parsedBuilder.build();
+      const parsedFeeRate = (parsedTx as any).fee.feeRate;
+
+      assert(parsedFeeRate !== undefined && parsedFeeRate > 0, 'Parsed feeRate should be defined and positive');
+
+      const feeRateDiff = Math.abs(parsedFeeRate! - expectedFeeRate);
+      const maxAllowedDiff = 10;
+      assert(
+        feeRateDiff <= maxAllowedDiff,
+        `Parsed feeRate ${parsedFeeRate} should be close to original ${expectedFeeRate} (diff: ${feeRateDiff})`
+      );
+
+      const feeSize = feeInfo.size!;
+      assert(feeSize > 10000, `Fee size ${feeSize} should include fixed cost (10000) + input costs`);
+      assert(feeSize < 20000, `Fee size ${feeSize} should be reasonable (< 20000)`);
+    });
+
+    it('should prevent artificially inflated feeRate from using wrong calculation', async () => {
+      const inputAmount = '100000000'; // 100M nanoFLRP
+      const threshold = 2;
+
+      const utxo: DecodedUtxoObj = {
+        outputID: 0,
+        amount: inputAmount,
+        txid: '2vPMx8P63adgBae7GAWFx7qvJDwRmMnDCyKddHRBXWhysjX4BP',
+        outputidx: '0',
+        addresses: [
+          '0x3329be7d01cd3ebaae6654d7327dd9f17a2e1581',
+          '0x7e918a5e8083ae4c9f2f0ed77055c24bf3665001',
+          '0xc7324437c96c7c8a6a152da2385c1db5c3ab1f91',
+        ],
+        threshold: threshold,
+      };
+
+      const feeRate = 500;
+
+      const txBuilder = factory
+        .getImportInCBuilder()
+        .threshold(threshold)
+        .fromPubKey(testData.pAddresses)
+        .utxos([utxo])
+        .to(testData.to)
+        .feeRate(feeRate.toString());
+
+      let tx;
+      try {
+        tx = await txBuilder.build();
+      } catch (error: any) {
+        throw new Error(
+          `Transaction build failed (this was the OLD bug behavior): ${error.message}. ` +
+            `The fix ensures calculateImportCost() is used consistently.`
+        );
+      }
+
+      const calculatedFee = BigInt((tx as any).fee.fee);
+
+      const oldBugFee = BigInt(328000000);
+      const reasonableFee = BigInt(10000000);
+
+      assert(
+        calculatedFee < reasonableFee,
+        `Fee ${calculatedFee} should be reasonable (< ${reasonableFee}), not inflated like OLD bug (~${oldBugFee})`
+      );
+
+      const outputAmount = BigInt(tx.outputs[0].value);
+      assert(
+        outputAmount > BigInt(0),
+        `Output ${outputAmount} should be positive. OLD bug would make output negative due to excessive fee.`
+      );
+    });
   });
 
   describe('on-chain verified transactions', () => {