fix(sdk-coin-flrp): compute Flare C-chain import fee from real gas usage

[ArunBala-Bitgo]

Summary

• Flare (FLR/coston2) C-chain atomic imports were built with evm.newImportTx(fee), treating the caller-supplied fee as a literal amount and never accounting for the ~10,000 gas AtomicTxBaseCost that flarejs's gas estimator applies internally.
• When a caller priced the import using an underestimated gas figure (e.g. ~5,700 gas instead of the real ~11,300), the network rejected the broadcast with insufficient funds — independent of transfer amount, matching the reported failure mode.
• Added an opt-in baseFee() builder method that instead calls evm.newImportTxFromBaseFee(...), letting flarejs compute the real gas cost (including AtomicTxBaseCost) from the actual tx, and applies a 10% padding buffer on the supplied base fee to absorb volatility between signing and broadcast. This mirrors the pattern the export builder (ExportInCTxBuilder) already uses via newExportTxFromBaseFee.
• The existing literal fee() path is untouched for backward compatibility with existing (on-chain verified) test vectors.

Test plan

• npx mocha --exclude test/unit/flrp.ts 'test/unit/**/*.ts' in modules/sdk-coin-flrp — 275 passing
• New tests confirm the base-fee-derived fee exceeds a naive gas*baseFee estimate and includes padding
• tsc --noEmit and eslint clean (no new warnings)

Ticket: CECHO-1821

🤖 Generated with Claude Code

[linear-code]

CECHO-1821

[ArunBala-Bitgo]

@claude review this PR

[github-actions]

Claude finished @ArunBala-Bitgo's task —— View job

---

Code Review for Flare C-chain Import Fee Fix

Todo List:

• Read and analyze all modified files
• Review the core implementation changes in ImportInCTxBuilder.ts
• Examine interface changes and type definitions
• Review test coverage and test quality
• Check for potential security or performance issues
• Provide comprehensive review feedback

---

Overall Assessment

This PR successfully addresses a critical issue in Flare C-chain atomic import transactions where underestimated gas fees were causing broadcast failures. The implementation is well-architected, maintains backward compatibility, and includes comprehensive test coverage.

✅ Strengths

1. Excellent Problem Analysis & Solution Design

• Root Cause Identification: Correctly identified that evm.newImportTx(fee) treats the fee as literal, missing the ~10,000 gas AtomicTxBaseCost that flarejs applies internally
• Optimal Solution: Mirrors the existing ExportInCTxBuilder pattern by using newImportTxFromBaseFee() instead of newImportTx()
• Volatility Protection: 10% padding buffer (BASE_FEE_PADDING_BPS = 1000n) protects against base fee fluctuations between signing and broadcast

2. Robust Implementation Architecture

• Dual Path Design: Clean conditional logic supports both new baseFee() and legacy fee() methods in ImportInCTxBuilder.ts:171-209
• Type Safety: Proper BigInt arithmetic and type casting throughout
• Error Handling: Maintains existing validation while adding new path-specific logic
• Code Organization: Well-structured with clear separation of concerns

3. Comprehensive Test Coverage

The new test suite (importInCTxBuilder.ts:224-281) is particularly strong:

• Functional Verification: Confirms gas-aware fees exceed naive estimates
• Padding Validation: Ensures 10% buffer is applied correctly
• Edge Case Testing: Uses realistic values from reported failure scenarios
• Determinism: Verifies consistent behavior across identical inputs

4. Backward Compatibility

• Preserved API: Existing fee() method unchanged, ensuring no breaking changes
• Test Compatibility: All 275 existing tests pass without modification
• Migration Path: Opt-in design allows gradual adoption of new baseFee() method

🔍 Detailed Code Analysis

Interface Changes (iface.ts:166-172)

/**
 * C-chain base fee (in nFLR/wei per gas) used to derive the atomic-tx fee.
 * When set, the import builder computes the required fee from actual gas usage
 * (including the AtomicTxBaseCost) instead of treating `fee` as a fixed amount.
 */
baseFee?: string;

• ✅ Well-documented optional field
• ✅ Clear explanation of behavior difference
• ✅ Consistent with existing FlrpTransactionFee structure

Builder Method (atomicTransactionBuilder.ts:82-94)

baseFee(baseFeeValue: string | bigint): this {
  const baseFee = typeof baseFeeValue === 'string' ? baseFeeValue : baseFeeValue.toString();
  (this.transaction as Transaction)._fee.baseFee = baseFee;
  return this;
}

• ✅ Consistent with existing fee() method signature
• ✅ Proper type conversion handling
• ✅ Fluent interface pattern maintained

Core Logic (ImportInCTxBuilder.ts:171-209)

The conditional implementation is clean and logical:

• Gas-Aware Path: Uses evm.newImportTxFromBaseFee() with padding calculation
• Legacy Path: Maintains original evm.newImportTx() behavior
• Fee Calculation: Properly derives final fee from UTXO inputs minus outputs

💡 Minor Suggestions

1. Enhanced Error Context (ImportInCTxBuilder.ts:136-138)

Consider enhancing the error message:

// Current
if (!this.transaction._fee.fee && !this.transaction._fee.baseFee) {
  throw new BuildTransactionError('fee is required');
}

// Suggested
if (!this.transaction._fee.fee && !this.transaction._fee.baseFee) {
  throw new BuildTransactionError('Either fee or baseFee is required');
}

2. Constant Documentation

The BASE_FEE_PADDING_BPS constant is well-placed, but consider adding a reference:

/**
 * Buffer applied to a caller-supplied base fee to absorb C-chain base-fee volatility
 * between transaction signing and broadcast. Expressed in basis points (1000 = 10%).
 * @see ExportInCTxBuilder for similar padding strategy
 */
const BASE_FEE_PADDING_BPS = 1000n;

3. Test Enhancement Opportunity

Consider adding a test that verifies the exact gas calculation matches the expected ~11,300 gas cost mentioned in the PR description.

🔒 Security & Performance Assessment

Security

• ✅ No Injection Risks: All inputs properly validated and typed
• ✅ Overflow Protection: BigInt arithmetic prevents integer overflow
• ✅ Balance Validation: UTXO sufficiency checks maintained
• ✅ Padding Bounds: 10% padding is reasonable and bounded

Performance

• ✅ Minimal Overhead: New path only active when baseFee specified
• ✅ Efficient Calculation: Simple arithmetic operations
• ✅ No Breaking Changes: Legacy path unchanged

📋 Final Verdict

APPROVED ✅

This is a high-quality fix that:

• ✅ Solves the reported gas estimation issue effectively
• ✅ Maintains full backward compatibility
• ✅ Includes excellent test coverage
• ✅ Follows established patterns in the codebase
• ✅ Implements appropriate safety measures (padding)
• ✅ Has clear, well-documented code

The implementation correctly addresses CECHO-1821 by allowing flarejs to compute the real gas cost including AtomicTxBaseCost, preventing the "insufficient funds" errors on broadcast.

---

Job run: #12465 | Branch: cecho-1821-flare-atomic-import-fee

[Copilot]

Pull request overview

This PR updates the Flare (FLR/coston2) C-chain atomic import transaction builder to support deriving the import fee from actual gas usage (including FlareJS’s AtomicTxBaseCost) by introducing an opt-in base-fee-driven build path, while keeping the legacy literal fee() behavior for backward compatibility.

Changes:

• Added a baseFee() builder input to compute import fees from real gas usage via evm.newImportTxFromBaseFee(...), including a 10% base-fee padding buffer.
• Updated import builder validation/build logic to accept baseFee as an alternative to a literal fee.
• Added unit tests covering the base-fee-driven path and expected fee characteristics.

Reviewed changes

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

File	Description
modules/sdk-coin-flrp/src/lib/ImportInCTxBuilder.ts	Adds base-fee-derived import fee computation via FlareJS and applies base-fee padding.
modules/sdk-coin-flrp/src/lib/atomicTransactionBuilder.ts	Introduces a baseFee() setter on atomic transaction builders.
modules/sdk-coin-flrp/src/lib/iface.ts	Extends fee shape to include an optional baseFee field for C-chain imports.
modules/sdk-coin-flrp/test/unit/lib/importInCTxBuilder.ts	Adds unit tests for the base-fee-driven import builder behavior.

---

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

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (2)

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

• In the baseFee path, the computed fee is only validated as > 0. This is weaker than the legacy fixed-fee path, which requires totalUtxoAmount > fee (ensuring some amount is actually imported). As written, a tx where the computed fee consumes the entire UTXO amount (outputs sum to 0) would pass validation and set fee to the full input amount.

      const innerImportTx = importTx.getTx() as evmSerial.ImportTx;
      const totalOutputAmount = innerImportTx.Outs.reduce((sum, out) => sum + out.amount.value(), BigInt(0));
      const computedFee = totalUtxoAmount - totalOutputAmount;
      if (computedFee <= BigInt(0)) {
        throw new BuildTransactionError(

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

• This comment is misleading: baseFeeWei is already in wei-per-gas, but the note says "gwei-equivalent" even though the literal value is 500n (wei). Updating the comment avoids confusion about what unit the builder expects.

      const baseFeeWei = 500n; // gwei-equivalent base fee used in the reported failure
      const underpricedGasEstimate = 5700n; // naive/legacy gas estimate missing AtomicTxBaseCost
      const underpricedFee = baseFeeWei * underpricedGasEstimate;

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (2)

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

• The error message here only mentions fee, but the builder now also accepts baseFee. When neither is provided, the message should reflect the new valid option to reduce confusion for callers.

    if (!hasFee && !hasBaseFee) {
      throw new BuildTransactionError('fee is required');
    }

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

• After computing the fee from baseFee, the transaction ends up with both fee (now computed) and baseFee set. That contradicts the earlier mutual-exclusivity validation and can confuse downstream consumers/serialization. Consider clearing baseFee once the final fee is derived to preserve the invariant.

      this.transaction._fee.fee = computedFee.toString();