Skip to content

Conversation

@mohd-kashif
Copy link
Contributor

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

WIN-8498: Refactor FLRP Transaction Builders for Better Code Organization

This PR refactors the FLRP transaction builders to consolidate common properties and methods into base classes, reducing code duplication and improving maintainability.


🔄 Changes Overview

1. Consolidated Common Properties to Base Classes

Property Moved From Moved To Storage Location
context() All 4 builders TransactionBuilder transaction._context
feeState() ExportInPTxBuilder, ImportInPTxBuilder AtomicTransactionBuilder transaction._feeState
amount() ExportInPTxBuilder, ExportInCTxBuilder AtomicTransactionBuilder transaction._amount
utxos() ExportInPTxBuilder, ImportInPTxBuilder Already in TransactionBuilder Removed duplicates

2. Added Validation for UTXO Balance

  • ExportInPTxBuilder: Validates that totalUtxoAmount >= amount before building
  • ImportInCTxBuilder: Validates that totalUtxoAmount > fee to ensure import can pay fees
  • ImportInPTxBuilder: Validates that totalUtxoAmount > 0

3. Standardized Error Types

All validation errors now consistently use BuildTransactionError instead of generic Error.


📁 Files Modified

File Changes
transaction.ts Added _feeState: FlrpFeeState and _amount: bigint public properties
atomicTransactionBuilder.ts Added feeState() and amount() setter methods
ExportInPTxBuilder.ts Removed duplicate _amount, _feeState, amount(), feeState(), utxos() - now uses base class
ExportInCTxBuilder.ts Removed duplicate _amount, _context, amount(), context() - now uses base class; fixed utxos() method signature
ImportInPTxBuilder.ts Removed duplicate _feeState, _context, feeState(), context(), utxos() - now uses base class
ImportInCTxBuilder.ts Removed duplicate _context, context() - now uses base class; added UTXO balance validation

✅ Benefits

  1. Reduced Code Duplication: Common setters like context(), feeState(), and amount() are now in base classes
  2. Consistent Validation: All builders use BuildTransactionError for validation failures
  3. Better UTXO Validation: Added balance checks to prevent transactions that would fail on-chain
  4. Easier Maintenance: Changes to common functionality only need to be made in one place

🧪 Testing

All existing unit tests pass. The refactoring maintains backward compatibility - the public API remains unchanged.

@mohd-kashif mohd-kashif changed the title Win 8498 WIN-8498: Refactor FLRP Transaction Builders for Better Code Organization Jan 5, 2026
@mohd-kashif mohd-kashif force-pushed the WIN-8498 branch 2 times, most recently from 542a8d5 to 0fda5db Compare January 5, 2026 18:59
@mohd-kashif mohd-kashif self-assigned this Jan 5, 2026
@mohd-kashif mohd-kashif requested a review from Copilot January 5, 2026 19:25
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 refactors the FLRP transaction builders to consolidate common properties (context, feeState, amount, utxos) into base classes, reducing code duplication. The refactoring also introduces standardized validation using BuildTransactionError and updates transaction building to use new FlareJS API methods (pvm.e.newImportTx, pvm.e.newExportTx, evm.newImportTx, evm.newExportTxFromBaseFee).

Key changes:

  • Moved common setters to TransactionBuilder and AtomicTransactionBuilder base classes
  • Changed UTXO handling from DecodedUtxoObj[] to hex string arrays with FlareJS native parsing
  • Introduced Context and FlrpFeeState for dynamic fee calculation
  • Updated all builders to use FlareJS helper methods instead of manual transaction construction

Reviewed changes

Copilot reviewed 26 out of 27 changed files in this pull request and generated no comments.

Show a summary per file
File Description
transaction.ts Added _context, _feeState, _amount public properties; changed fee type to FlrpTransactionFee
transactionBuilder.ts Moved context() setter and updated utxos() to accept hex strings; refactored validation
atomicTransactionBuilder.ts Added feeState() and amount() setters; removed createInputOutput() method
ImportInPTxBuilder.ts Refactored to use pvm.e.newImportTx; removed manual transaction construction
ImportInCTxBuilder.ts Refactored to use evm.newImportTx; simplified fee calculation
ExportInPTxBuilder.ts Refactored to use pvm.e.newExportTx; removed manual UTXO selection logic
ExportInCTxBuilder.ts Refactored to use evm.newExportTxFromBaseFee; simplified construction
utils.ts Added parseUtxoHex(), parseUtxoHexArray(), utxoToDecoded() for UTXO parsing
iface.ts Added Context, FlrpTransactionFee, FeeConfig, ExportEVMOptions interfaces
networks.ts Added flarePublicUrl property; changed txFee from 1261000 to 200000
package.json Added @bitgo/public-types@5.59.0 dependency
Test files Updated test data with new transaction formats, context, and feeState
Comments suppressed due to low confidence (1)

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

  mapOutputToEntry(network: FlareNetwork): (Output) => Entry {

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

@mohd-kashif mohd-kashif force-pushed the WIN-8498 branch 7 times, most recently from 2a00e0c to e66b7fa Compare January 8, 2026 04:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants