Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fee bumping Package #114

Merged
merged 84 commits into from
Oct 23, 2024
Merged

Fee bumping Package #114

merged 84 commits into from
Oct 23, 2024

Conversation

Legend101Zz
Copy link
Contributor

Fee Bumping Package

Overview

Comprehensive refactor of the fee bumping package to enhance functionality, improve bitcoinjs-lib integration, and streamline code structure.

Key Improvements

  • RBF: Dynamic handling, cancellation support, output fee subtraction
  • CPFP: Multi-parent output handling, dynamic calculations, additional outputs
  • Utils: New deriveAddress function, updated RBF checks, removed estimateVirtualSize
  • Analysis: New analyzeTransaction function for fee bumping recommendations
  • Types: Expanded definitions for better bitcoinjs-lib integration

Changes

  • Updated RBF and CPFP to work directly with bitcoinjs-lib Transaction objects
  • Implemented more accurate fee calculations
  • Added comprehensive test suite for new functionality
  • Removed estimator.ts, integrating core functions elsewhere

Notes

Significant changes to fee bumping logic. Careful review needed, especially for transaction analysis and RBF/CPFP implementations.

Create a new package for fee bumping functionality in Caravan. This package
provides modular and reusable utilities for implementing RBF (Replace-By-Fee)
and CPFP (Child-Pays-For-Parent) fee bumping strategies.

Key additions:
- Basic package structure and configuration files
- Core modules for RBF and CPFP implementations
- Fee estimation utilities
- Type definitions for fee bumping operations
- Constants and utility functions

This package aims to enhance Caravan's transaction management capabilities
by providing a flexible and extensible fee bumping solution that can be
easily integrated into the Caravan coordinator wallet and other Bitcoin
projects.
This commit introduces a new @caravan/fee-bumping package that provides robust functionality for handling Replace-By-Fee (RBF) and Child-Pays-For-Parent (CPFP) fee bumping strategies in Bitcoin transactions. The package is designed to work seamlessly with other Caravan packages and offers a flexible, modular approach to fee bumping.

Key features:
- Implement RBFHandler for Replace-By-Fee transactions
- Implement CPFPHandler for Child-Pays-For-Parent transactions
- Create FeeEstimator for dynamic fee rate calculations
- Provide utility functions for transaction analysis and manipulation
- Define comprehensive types for improved type safety and developer experience
- Implement constants for fee-related thresholds and limits

The package structure includes:
- src/index.ts: Main entry point and FeeBumping class
- src/types.ts: TypeScript interfaces and types
- src/estimator.ts: FeeEstimator class for fee calculations
- src/rbf.ts: RBFHandler class for RBF operations
- src/cpfp.ts: CPFPHandler class for CPFP operations
- src/utils.ts: Utility functions for transaction handling
- src/constants.ts: Constant values used throughout the package

This package provides a solid foundation for implementing fee bumping strategies in Caravan-based applications. It offers:
1. Flexible fee estimation based on network conditions
2. Configurable options for both RBF and CPFP operations
3. Built-in safeguards against excessive fees
4. Utility functions for transaction analysis and modification

The implementation is designed with extensibility in mind, allowing for easy updates and additions to fee bumping strategies in the future.

Next steps:
- Implement comprehensive unit tests for all components
- Integrate the package with the main Caravan UI
- Create documentation and usage examples
- Perform thorough testing with various transaction scenarios

This addition significantly enhances Caravan's capabilities in handling dynamic fee environments, improving overall user experience and transaction reliability.
- Add new types for UTXO, TransactionOutput, TransactionAnalysis, FeeRate,
  FeeEstimate, TransactionDetails, BlockchainClientInterface, RBFOptions,
  and CPFPOptions in types.ts
- Create getFeeEstimates function to fetch fee estimates from blockchain client
- Implement calculateFee function to compute transaction fee based on vsize and fee rate
- Use BigNumber for precise calculations
- Import necessary constants and interfaces

This commit lays the groundwork for RBF and CPFP implementations by defining
essential types and providing core fee estimation functionality.
- Create new file analyzer.ts with analyzeTransaction function
- Implement logic to determine if a transaction can use RBF or CPFP
- Calculate current fee rate of the transaction
- Provide a recommended fee bumping method based on transaction properties
- Utilize utility functions isRBFSignaled and calculateEffectiveFeeRate
- Return a TransactionAnalysis object with fee bumping options

This commit adds crucial functionality for analyzing transactions to determine
the best fee bumping strategy, supporting the implementation of RBF and CPFP
features in the Caravan wallet.
This commit significantly refactors the fee bumping package to improve its functionality,
integration with bitcoinjs-lib, and overall code structure. Key changes include:

RBF (Replace-By-Fee):
- Updated to work with bitcoinjs-lib Transaction objects
- Implemented dynamic input and output handling
- Added support for transaction cancellation and fee subtraction from outputs

CPFP (Child-Pays-For-Parent):
- Refactored to handle multiple parent outputs
- Implemented dynamic input and output calculation
- Added support for additional outputs in child transactions
- Improved fee calculation accuracy

Utils:
- Added deriveAddress function for working with bitcoinjs-lib Output objects
- Updated isRBFSignaled and calculateEffectiveFeeRate functions
- Removed estimateVirtualSize in favor of using Transaction.virtualSize()

Transaction Analysis:
- Implemented analyzeTransaction function for recommending fee bumping strategies
- Added logic to compare current fee rate with network fee rate

Removed:
- Eliminated estimator.ts file, integrating necessary functionality into other modules

Tests:
- Added comprehensive test suite for analyzeTransaction function
- Updated existing tests to reflect new implementations of RBF and CPFP functions

Types:
- Updated and expanded type definitions to better integrate with bitcoinjs-lib
- Added new interfaces for RBFOptions, CPFPOptions, and TransactionAnalysis
Copy link

changeset-bot bot commented Jul 23, 2024

🦋 Changeset detected

Latest commit: 8f2335b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@caravan/psbt Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Jul 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
caravan-coordinator ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 23, 2024 2:58pm

Copy link
Contributor

@bucko13 bucko13 left a comment

Choose a reason for hiding this comment

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

Really good stuff here so far! Only reviewed cpfp so far, but wanted to submit some comments to start.

High level feedback is that I would be curious to see if you think you could reduce the dependence on a consumer of this package from having to use other external dependencies as much as possible. Right now someone that wants to interact and leverage this package outside of this monorepo MUST have some awareness of the use of bitcoinjs-lib and bignumber library and use the same libraries themselves. Ideally any use of bignumbers is contained to within the functions themselves. This means that any consumer of the function can use whatever BN library they prefer and just convert to/from strings when interacting with the function.

Same with Transaction from bitcoinjs-lib. It would be nice if someone that wanted to estimate fees or construct a CPFP didn't have to first build a bitcoinjs-lib Transaction object (and what happens if they're using a different/incompatible version??)

packages/caravan-fees/src/constants.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/constants.ts Show resolved Hide resolved
packages/caravan-fees/src/constants.ts Outdated Show resolved Hide resolved
packages/caravan-fees/package.json Outdated Show resolved Hide resolved
packages/caravan-fees/src/cpfp.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/cpfp.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/cpfp.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/types.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/cpfp.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/cpfp.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@bucko13 bucko13 left a comment

Choose a reason for hiding this comment

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

Some more comments on the other files. Excited about the progress!

packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/transactionAnalyzer.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/transactionAnalyzer.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/transactionAnalyzer.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/utils.ts Outdated Show resolved Hide resolved
…gnalling functionality to PSBTV2

Key changes:

1. rbf.ts:
   - Introduced RbfTransaction class to encapsulate RBF logic
   - Implemented methods for preparing accelerated and canceled transactions
   - Added helper methods for fee calculations and PSBT manipulations
   - Created public functions prepareRbfTransaction and prepareCancelTransaction
     as entry points for RBF operations

2. psbtv2.ts:
   - Added isRbfSignaled method to check if a transaction is RBF-enabled
Copy link
Contributor

@Shadouts Shadouts left a comment

Choose a reason for hiding this comment

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

I've only reviewed the PsbtV2 class changes. I like the addition of another value setter and getter. There are so many of these unbuilt which could make this package really powerful. However, I take issue with the bip125-specific changes. See line comments.

Additionally, adding a test for setInputSequence would be good.

packages/caravan-psbt/src/psbtv2/psbtv2.ts Outdated Show resolved Hide resolved
packages/caravan-psbt/src/psbtv2/psbtv2.ts Outdated Show resolved Hide resolved
packages/caravan-psbt/src/psbtv2/psbtv2.ts Outdated Show resolved Hide resolved
packages/caravan-psbt/src/psbtv2/psbtv2.ts Show resolved Hide resolved
packages/caravan-psbt/src/psbtv2/psbtv2.ts Outdated Show resolved Hide resolved
packages/caravan-psbt/src/psbtv2/psbtv2.ts Outdated Show resolved Hide resolved
packages/caravan-psbt/src/psbtv2/psbtv2.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@bucko13 bucko13 left a comment

Choose a reason for hiding this comment

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

Couple more comments. I like the move to the class for the RBF!

packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/transactionAnalyzer.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/transactionAnalyzer.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/package.json Outdated Show resolved Hide resolved
This commit introduces new classes for Child-Pays-for-Parent (CPFP) and refactors majority of Replace-By-Fee (RBF)
transaction creation, along with comprehensive tests.

Key changes:
- Add CPFPTransaction class for creating child transactions
- Refactored RbfTransaction class for preparing replacement transactions
- Create tests for CPFP, RBF
- Update existing transaction utilities to support new classes

The new classes support both PSBTv2 and PSBTv0 formats, handle various edge cases,
and provide methods for fee calculation and transaction preparation.
Copy link
Contributor

@bucko13 bucko13 left a comment

Choose a reason for hiding this comment

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

leaving some more comments, going to wait for a full review as it looks like there are some changes still in progress.

packages/caravan-fees/src/constants.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/cpfp.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/cpfp.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/cpfp.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/types.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@bucko13 bucko13 left a comment

Choose a reason for hiding this comment

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

The classes are much much better and the tests are improving as well. Still some things that need to be cleaned up though.

packages/caravan-psbt/src/psbtv2/psbtv2.ts Show resolved Hide resolved
packages/caravan-fees/src/constants.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/cpfp.fixtures.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/cpfp.test.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/cpfp.test.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
…e calculation

- Add state management to track acceleration and cancellation status
- Implement caching for fee increase and vsize calculations
- Introduce incremental relay fee for accurate fee bump calculations
- Refactor fee calculation logic to comply with BIP 125 rules
- Optimize input selection by sorting UTXOs before addition
- Improve RBF signaling by updating only one input sequence
- Added more comments for better code understanding
…(Replace-By-Fee) support

- setInputSequence(inputIndex: number, sequence: number): A method for setting the sequence number of a specific input, critical for signaling RBF and handling nLockTime.
- get isRBFSignaled(): boolean: A method to check if the transaction signals RBF, as per BIP125 guidelines.
- Added tests
Copy link
Contributor

@bucko13 bucko13 left a comment

Choose a reason for hiding this comment

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

Looks great! Mostly just some aesthetic nits here and there and then I think we're good!

packages/caravan-fees/src/constants.ts Show resolved Hide resolved
packages/caravan-fees/src/cpfp.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/cpfp.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/btcTransactionTemplate.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/rbf.ts Outdated Show resolved Hide resolved
packages/caravan-fees/src/transactionAnalyzer.ts Outdated Show resolved Hide resolved
@bucko13 bucko13 dismissed Shadouts’s stale review October 23, 2024 15:20

Dismissed as resolved in 787be0d

@bucko13 bucko13 merged commit 8d79fc6 into caravan-bitcoin:main Oct 23, 2024
3 checks passed
@github-actions github-actions bot mentioned this pull request Nov 7, 2024
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.

4 participants