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.

Sign up for GitHub

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

Jump to bottom

Fee bumping Package #114

Open
wants to merge 63 commits into
base: main
Choose a base branch
from
Open

Fee bumping Package #114

Changes from 1 commit
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
5c233db
feat: Initialize @caravan/feebumping package
Legend101Zz Jun 23, 2024
592a875
changes in rbf file
Legend101Zz Jun 23, 2024
4f0755c
Add comprehensive fee bumping package for Caravan
Legend101Zz Jul 19, 2024
b00eaae
Implement fee bumping types and estimation functions
Legend101Zz Jul 22, 2024
9b0c967
Add transaction analysis functionality for fee bumping
Legend101Zz Jul 22, 2024
b7b2fa5
Refactor fee bumping package for improved functionality and integration
Legend101Zz Jul 22, 2024
63c129c
Changes to the RBF functionality in the fees package and added RBF si…
Legend101Zz Jul 26, 2024
4fce4fe
Implement CPFP and RBF classes with tests
Legend101Zz Aug 1, 2024
77f0cbe
Refactored RBF and CPFP classes, introduced TransactionAnalyzer class
Legend101Zz Aug 3, 2024
77f4db4
Refactor RBF, CPFP, and TransactionAnalyzer Classes and their Tests
Legend101Zz Aug 6, 2024
4f4f97a
Enhance RBF class functionality with improved state management and fe…
Legend101Zz Aug 14, 2024
07d5a84
Added methods to handle sequence numbers within PSBTs for better RBF …
Legend101Zz Aug 14, 2024
d71a0ec
minor corrections in requiredFee method
Legend101Zz Aug 14, 2024
ee553e7
minor corrections in requiredFee name , changed to requiredAbsoluteFee
Legend101Zz Aug 14, 2024
9237196
Refactor RBF class with improved state management and error handling
Legend101Zz Aug 16, 2024
63969ab
feat: Implement TransactionAnalyzer for raw transaction analysis
Legend101Zz Aug 19, 2024
0047714
Refactor of TransactionAnalyzer classfor Enhanced Fee Calculation and…
Legend101Zz Aug 22, 2024
28a1ae2
Added Bitcoin Transaction Components and Template
Legend101Zz Aug 22, 2024
8ca1b75
feat: Implemented new CPFP/RBF functions
Legend101Zz Aug 22, 2024
09cceac
chore: changes in utils,types and constants files
Legend101Zz Aug 29, 2024
47e598f
refactor: changes in transactionAnalyzer class and it's tests
Legend101Zz Aug 29, 2024
3c9f3ff
refactor : btcTransactionComponents and btcTransactionTemplate classes
Legend101Zz Aug 29, 2024
93f5e39
refactor: Refactored and corrected RBF and CPFP functions
Legend101Zz Aug 29, 2024
b247da4
changes: add util fn to convert btcTemplate to PSBT
Legend101Zz Sep 1, 2024
6662b2d
refactor : btcComponents and btcTemplate ... toPsbt() method , remove…
Legend101Zz Sep 1, 2024
f6aeea6
refactor: transactionAnalyzer ... removed redundant methods
Legend101Zz Sep 1, 2024
345d6fb
refactor: Changes in btcTransactionInput , Add methods for intiatiati…
Legend101Zz Sep 2, 2024
ad3c6ee
refactor : utils , added more util types for PSBT based calculations
Legend101Zz Sep 2, 2024
a215dea
refactor: RBF/CPFP functions to handle new toPSBT() method of templat…
Legend101Zz Sep 2, 2024
4597dc9
tests: added new btcTransactionComponents tests and refactored other …
Legend101Zz Sep 2, 2024
f39e26d
add README
Legend101Zz Sep 2, 2024
372f919
refactor btcTransactionComponents : Changes to inline docstring comme…
Legend101Zz Sep 9, 2024
d19fd1e
minor README changes
Legend101Zz Sep 9, 2024
272d946
refactor btcTransactionComponents : added methods to validate witness…
Legend101Zz Sep 9, 2024
c75270a
corrected and added new tests to btcTransactionComponents
Legend101Zz Sep 9, 2024
09d9bc5
refactored: btcTransactionTemplate class
Legend101Zz Sep 9, 2024
8da4962
Add ScriptType : Added a new script-type extending MULTISIG_ADDRESS_T…
Legend101Zz Sep 9, 2024
0acce5f
refactot transactionAnalyzer
Legend101Zz Sep 9, 2024
65717ec
minor fix: README link
Legend101Zz Sep 11, 2024
dc02fb5
Refactor: Utils file to remove unnecessary functions, move some util …
Legend101Zz Sep 12, 2024
73dd323
refactor: Transaction Analyzer ... correct rbf-fees calculation , ret…
Legend101Zz Sep 12, 2024
db3ae16
minor-refactor: Add detailed doc strings for ABSURDLY_HIGH_ABS_FEE & …
Legend101Zz Sep 12, 2024
fbd36a1
minor btcTransactionComponents changes
Legend101Zz Sep 12, 2024
82a3c5c
refactor: btcTransactionTemplate add addInputToPsbt & addOutputToPsbt…
Legend101Zz Sep 12, 2024
f142a7f
add: isCPFPFeeSatisfied method from utils to cpfp file
Legend101Zz Sep 12, 2024
104bc75
removed reverseTxid fn from utils
Legend101Zz Sep 16, 2024
e5c98c1
correction: Corrected code to extract input txid from input hash
Legend101Zz Sep 16, 2024
74511cf
correction-tests: Transaction Analyzer
Legend101Zz Sep 16, 2024
6f35442
correction-tests: BTC Components
Legend101Zz Sep 16, 2024
dcffd5e
correction tests: BTC Template
Legend101Zz Sep 22, 2024
788ff6f
correction tests: RBF
Legend101Zz Sep 22, 2024
c3f56c4
correction tests: CPFP
Legend101Zz Sep 22, 2024
74c643b
refactor : tsconfig
Legend101Zz Sep 22, 2024
c570faf
correction : RBF function , ensure that the new bumped tx ,pays gte o…
Legend101Zz Sep 24, 2024
539ac54
RBF : remove console.log statements
Legend101Zz Sep 24, 2024
bddee6e
correction : index file , added all exports
Legend101Zz Sep 24, 2024
ad43a38
correction: RBF calculations
Legend101Zz Sep 24, 2024
e8105cb
refactor: RBF fixtures
Legend101Zz Sep 24, 2024
7150a01
minor changes : Transaction Analyzer - RBF FEE CALCULATION COMMENTS .…
Legend101Zz Sep 27, 2024
a44bedc
added comments : types file
Legend101Zz Sep 27, 2024
663df1e
RBF : Add option in RBF tx creation to prevent
Legend101Zz Sep 27, 2024
09fba1f
RBF-Fixtures : minor comment fixes
Legend101Zz Sep 27, 2024
90b3227
changes: README
Legend101Zz Oct 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 63 additions & 11 deletions packages/caravan-fees/src/rbf.ts
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, could still use some work here but I think it's getting close. I think I understand a little better now the requirements on the rbf side and have a suggestion for how to clarify. Basically if the analyzer no longer thinks about rbf rates, and the rbf functions simply pass in the user defined target fee rate and then validates that value is higher than the original tx rate, that should clarify a lot.

I think you also need to make sure to add enough inputs for the cancelation tx so that the change is not dust. That should allow the change code and the validation at the end to be simplified.

Finally, if you try and modularize some more so that duplicated code can be shared, I think that will make things more readable.

Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,22 @@ export const createCancelRbfTransaction = (
newTxTemplate.addInput(BtcTxInputTemplate.fromUTXO(originalInput));

// Step 6: Add more inputs if necessary to meet fee requirements

// We continue adding inputs until both the fee rate is satisfied and
// the absolute fee meets the minimum RBF requirement
for (const utxo of availableInputs) {
if (newTxTemplate.feeRateSatisfied && newTxTemplate.areFeesPaid()) {
break; // Stop adding inputs once fee requirements are met
if (
newTxTemplate.feeRateSatisfied &&
new BigNumber(newTxTemplate.currentFee).gte(
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you still use “areFeesPaid” here, which should cover target fees, and then maybe there can be another method that does “are fees greater than” that you can pass the rbf value to for the comparison.

This is obviously a very important part of the logic so just thinking of ways to make it as readable as possible

Copy link
Author

Choose a reason for hiding this comment

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

also I had a doubt ... do you think that instead of using

txAnalyzer.minimumRBFFee we should check for max of the newTxTemplate.targetFeesToPay and absolute of original tx ... as minimumRBFFee is in turn calculated using the either targetFeeRate (set by user ) or old fee Rate + incrementalRelay FeeRate .... using the original tx size...

While newTxTemplate.targetFeesToPay calculates based on rbfFeeRate (which is calculated using minimumRBFFee) and current tx size (after bumping) ...

so

  const fee = BigNumber.max(
    newTxTemplate.targetFeesToPay,
    txAnalyzer.fee,
  );

would be correct code then ...

Copy link
Author

Choose a reason for hiding this comment

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

this is how we intialise the newTxTemplate

  // Step 3: Create a new transaction template
  const newTxTemplate = new BtcTransactionTemplate({
    inputs: [],
    outputs: [],
    targetFeeRate: Number(txAnalyzer.rbfFeeRate), // Use RBF-specific fee rate from analyzer
    dustThreshold,
    network,
    scriptType,
    requiredSigners,
    totalSigners,
  });

Copy link
Author

@Legend101Zz Legend101Zz Sep 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

like for this fixture ....

For this multisig transaction, we have two inputs and three outputs, each of P2WSH type. The original transaction pays 20,880 sats, and the transaction vsize is 348 vB. Therefore, the current fee rate is 60 sat/vB.

Scenario: RBF to Cancel Transaction

Suppose we use RBF to cancel this transaction and set a target fee rate of 80 sat/vB. In a cancellation, the size of the transaction typically decreases because we have only one output (cancellation address), and in most cases, a single input.

  • The new transaction size becomes 159 vB.
  • If we pay the original fee of 20,880 sats, the new fee rate becomes 131 sat/vB.

However, even though a fee of 12,720 sats would achieve the desired fee rate of 80 sat/vB, we still end up paying 20,880 sats to satisfy the RBF conditions.

now as per current code the newTxTemplate.targetFeesToPay gives 12,720 sats as 159vB * 80 sat/vB ... and txAnalyzer.minimumRBFFee gives ... 348vB (original tx size) * 80 sat/vB ==> 27_840sats and feeRate becomes 175 sat/vB ... so we overpay a lot

here

 // https://mempool.space/tx/be81b81620702718957d445611066bd596fe1840e219b84f6ea60e0114a7a305
    {
   
      case: "Create a valid cancel RBF transaction",
      originalTx:
        "010000000001022b2be3ac975c385defbda23a1eec30946b708c209981a5c07830fd726b3e25530200000000fdffffffe635a5d14ea5eb63d455948c0894d61ac35f7ba00f260f128b725f25855bd6550400000000fdffffff03400caa3b00000000220020701a8d401c84fb13e6baf169d59684e17abd9fa216c8cc5b9fc63d622ff8c58d886c5e1b000000002200200a618b712d918bb1ba59b737c2a37b40d557374754ef2575ce41d08d5f782df980960d04000000002200202122f4719add322f4d727f48379f8a8ba36a40ec4473fd99a2fdcfd89a16e048040047304402202c0be2d56da6f551b92e657a1f29bc410f244d68063bbc14297bae318d06a32d0220276ddf6da48d0dfdc717974aee32b94a45867393a663df8bc1e8382e1f7d9989014730440220649a73751ad934c9e517022bc8b510d53474772286e5678227ae71837ec0307d022031ec829069c7ab474759741c6acd1efb7e0918d5d63e4fbd1071ce3ea1bfc6ec01695221022dfa322241a4946b9ead36ab9c8c55bd4c4340a1290b5bf71d23a695aeb1240a21034d82610a17c332852205e063c64fee21a77fabc7ac0e6d7ada2a820922c9a5dc2103c96d495bfdd5ba4145e3e046fee45e84a8a48ad05bd8dbb395c011a32cf9f88053ae040047304402207361009ec2357c8d9f178a79772715d29308f1c265bf3c31f083d9253dcedd3c02203fbe90a73f2ca763233cacd9f6be1ebc24abdeff928d5ad454c6fd50b9bc869d014730440220416d0e1acfd95024dc31bc534fee0662ffd906dba98040ac4ed039bf4451176d02203b9416a93b4a116f4edcbfd256e5f232bc9b9f12f6270e84e23597ce68f24ca201695221022dfa322241a4946b9ead36ab9c8c55bd4c4340a1290b5bf71d23a695aeb1240a21034d82610a17c332852205e063c64fee21a77fabc7ac0e6d7ada2a820922c9a5dc2103c96d495bfdd5ba4145e3e046fee45e84a8a48ad05bd8dbb395c011a32cf9f88053ae00000000",
      network: Network.MAINNET,
      inputs: [
        {
          // https://mempool.space/tx/53253e6b72fd3078c0a58199208c706b9430ec1e3aa2bdef5d385c97ace32b2b#flow=&vout=2
          txid: "53253e6b72fd3078c0a58199208c706b9430ec1e3aa2bdef5d385c97ace32b2b",
          vout: 2,
          value: "932049200",
          sequence: 4294967293,
        },
        {
          // https://mempool.space/tx/55d65b85255f728b120f260fa07b5fc31ad694088c9455d463eba54ed1a535e6#flow=&vout=4
          txid: "55d65b85255f728b120f260fa07b5fc31ad694088c9455d463eba54ed1a535e6",
          vout: 4,
          value: "596144040",
          sequence: 4294967293,
        },
      ],
      availableUtxos: [
        // (Original Input) https://mempool.space/tx/53253e6b72fd3078c0a58199208c706b9430ec1e3aa2bdef5d385c97ace32b2b#flow=&vout=2
        {
          txid: "53253e6b72fd3078c0a58199208c706b9430ec1e3aa2bdef5d385c97ace32b2b",
          vout: 2,
          value: "932049200",
          witnessUtxo: {
            script: Buffer.from(
              "00202122f4719add322f4d727f48379f8a8ba36a40ec4473fd99a2fdcfd89a16e048",
              "hex",
            ),
            value: 932049200,
          },
          prevTxHex:
            "010000000145a8a366f1e073e4e1a03172b95e7db2f006d710d3585e5d14a137c218bf90ed03000000fc00473044022056da571c657d063d18217cb3d2cdb827d11fdd0fdef9ffc0ef69345628c0da260220092f7fe2b2d2cba7d1f4d53f0fb7db54dd15c6b8aac2f8e66102778aac0a62250147304402200bf2e4c82a2337168b7370deae483c88521367da8415c65ffa24b21ba4b13a7f02207a946b5c0f9f9213be130022d83f2ee91363c08de8cd2460765e020f59a3399a014c695221026fcec918a19aad4c92527f4bad924c7cc8dfdba935418f3ce217c1c839a58b952103c96d495bfdd5ba4145e3e046fee45e84a8a48ad05bd8dbb395c011a32cf9f8802103d1fc482d299248b97e38d9042a068eb102818a3556d5247d080849a0880a9e2653aefdffffff03400caa3b000000002200203eb5062a0b0850b23a599425289a091c374ca934101d03144f060c5b46a979be181d1a1a000000002200200a618b712d918bb1ba59b737c2a37b40d557374754ef2575ce41d08d5f782df930f18d37000000002200202122f4719add322f4d727f48379f8a8ba36a40ec4473fd99a2fdcfd89a16e04800000000",
        },
        {
          // (Original Input 2)  https://mempool.space/tx/55d65b85255f728b120f260fa07b5fc31ad694088c9455d463eba54ed1a535e6#flow=&vout=4
          txid: "55d65b85255f728b120f260fa07b5fc31ad694088c9455d463eba54ed1a535e6",
          vout: 4,
          value: "596144040",
          witnessUtxo: {
            script: Buffer.from(
              "00202122f4719add322f4d727f48379f8a8ba36a40ec4473fd99a2fdcfd89a16e048",
              "hex",
            ),
            value: 596144040,
          },
          prevTxHex:
            "010000000145a8a366f1e073e4e1a03172b95e7db2f006d710d3585e5d14a137c218bf90ed05000000fc004730440220414f5352d874fad4171326bdc563f2724eadbbe8e3199fbc83f9d6d04dc02176022072ec117a06d340209b44cef533d1a2f21c0ace64f653f454e745add290938e53014730440220367ff913573c50706003423b47648553730d0377c7707e505d1a4f9a08313ab6022073fad7c23abeb4ca1d48075bd276c7a5aa7b0d942ad741c9a8a4233a264f2df1014c695221026fcec918a19aad4c92527f4bad924c7cc8dfdba935418f3ce217c1c839a58b952103c96d495bfdd5ba4145e3e046fee45e84a8a48ad05bd8dbb395c011a32cf9f8802103d1fc482d299248b97e38d9042a068eb102818a3556d5247d080849a0880a9e2653aefdffffff05400caa3b000000002200200a618b712d918bb1ba59b737c2a37b40d557374754ef2575ce41d08d5f782df90084d71700000000220020701a8d401c84fb13e6baf169d59684e17abd9fa216c8cc5b9fc63d622ff8c58d0084d71700000000220020701a8d401c84fb13e6baf169d59684e17abd9fa216c8cc5b9fc63d622ff8c58d3819b1110000000022002014b288dca5d59caa8868d1668c97c971e58ab3ccf10534ac567ea51aa8aba299a86f8823000000002200202122f4719add322f4d727f48379f8a8ba36a40ec4473fd99a2fdcfd89a16e04800000000",
        },
        {
          // extra UTXO , NOT IN ORIGINAL TX
          // https://mempool.space/tx/89caa4dff1251f3a0e62b662ee1b22e8cffeb32bf2cf839ff959d62fe56a3562
          txid: "55d65b85255f728b120f260fa07b5fc31ad694088c9455d463eba54ed1a535e6",
          vout: 3,
          value: "‎296819000",
          witnessUtxo: {
            script: Buffer.from(
              "002014b288dca5d59caa8868d1668c97c971e58ab3ccf10534ac567ea51aa8aba299",
              "hex",
            ),
            value: 296819000,
          },
          prevTxHex:
            "010000000145a8a366f1e073e4e1a03172b95e7db2f006d710d3585e5d14a137c218bf90ed05000000fc004730440220414f5352d874fad4171326bdc563f2724eadbbe8e3199fbc83f9d6d04dc02176022072ec117a06d340209b44cef533d1a2f21c0ace64f653f454e745add290938e53014730440220367ff913573c50706003423b47648553730d0377c7707e505d1a4f9a08313ab6022073fad7c23abeb4ca1d48075bd276c7a5aa7b0d942ad741c9a8a4233a264f2df1014c695221026fcec918a19aad4c92527f4bad924c7cc8dfdba935418f3ce217c1c839a58b952103c96d495bfdd5ba4145e3e046fee45e84a8a48ad05bd8dbb395c011a32cf9f8802103d1fc482d299248b97e38d9042a068eb102818a3556d5247d080849a0880a9e2653aefdffffff05400caa3b000000002200200a618b712d918bb1ba59b737c2a37b40d557374754ef2575ce41d08d5f782df90084d71700000000220020701a8d401c84fb13e6baf169d59684e17abd9fa216c8cc5b9fc63d622ff8c58d0084d71700000000220020701a8d401c84fb13e6baf169d59684e17abd9fa216c8cc5b9fc63d622ff8c58d3819b1110000000022002014b288dca5d59caa8868d1668c97c971e58ab3ccf10534ac567ea51aa8aba299a86f8823000000002200202122f4719add322f4d727f48379f8a8ba36a40ec4473fd99a2fdcfd89a16e04800000000",
        },
      ],
      dustThreshold: "546",
      targetFeeRate: 80, // original was 60 sat/vB
      scriptType: SCRIPT_TYPES.P2WSH,
      requiredSigners: 2,
      totalSigners: 3,
      cancelAddress:
        "bc1qyy30guv6m5ez7ntj0ayr08u23w3k5s8vg3elmxdzlh8a3xskupyqn2lp5w", // https://mempool.space/address/bc1qyy30guv6m5ez7ntj0ayr08u23w3k5s8vg3elmxdzlh8a3xskupyqn2lp5w

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh interesting. That’s useful context. Then shouldn’t areFeesPaid cover it since it’s set to target the rbf rate?

I’m on mobile so I’m sure I’m missing something from all the different contexts but if the templates target is set from the analyzers rbf recommendation basically, then why doesn’t that catch all of the conditionals you’re looking at?

Copy link
Author

Choose a reason for hiding this comment

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

ahh ... yes actually ....
txAnalyzer.rbfFeeRate can be equal to targetFeeRate iff it is greater than BIP-125 based feeRate ... i.e
(original Fee Rate + incremental Relay)

which in this case is as RBF suggests a feeRate of 61 sats/vB

Copy link
Author

Choose a reason for hiding this comment

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

Oh interesting. That’s useful context. Then shouldn’t areFeesPaid cover it since it’s set to target the rbf rate?

I’m on mobile so I’m sure I’m missing something from all the different contexts but if the templates target is set from the analyzers rbf recommendation basically, then why doesn’t that catch all of the conditionals you’re looking at?

I am still trying to figure out how can we define this more clearly and remove the complexity ... surrounding it

Copy link
Contributor

Choose a reason for hiding this comment

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

iff it is greater than BIP-125 based feeRate ... i.e (original Fee Rate + incremental Relay)

But I thought the only rate mentioned in the bip was the minimum relay fee setting, so original fee rate doesn't really factor in here (but will for accelerations implicitly since you'll want a higher fee rate to fee bump).

Copy link
Contributor

Choose a reason for hiding this comment

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

So, as I've been re-reviewing and thinking about this more, I think part of the complexity comes from this comment you made:

minimumRBFFee is in turn calculated using the either targetFeeRate (set by user ) or old fee Rate + incrementalRelay FeeRate .... using the original tx size...

I commented on this in the analyzer as well. This is redirection that can be confusing. It's not clear that first of all that this check is being made which makes minimumRBFFee harder to understand. Have the property describing absolute fee, stick to the rules related to absolute fees.

Second, I think if you make the change at the top that I suggested, where the targetFeeRate from the user is validated to be at least as much as the original, then you can simplify to this:

if (
      newTxTemplate.feeRateSatisfied && 
      new BigNumber(newTxTemplate.currentFee).gte(txAnalyzer.minimumRbfFee)
    ) {
      // Stop adding inputs when both conditions are met:
      // 1. The fee rate satisfies the target rate (which must be gte original tx fee rate
      // 2. The current fee is gte the minimum required absolute RBF fee amount
      break;
    }

this should handle the situation you described above where you can hit the target fee rate but still not be paying enough absolute fees (because the replacement tx is smaller) and so you have to keep adding inputs to increase the current fee to be higher than minimumRbfFee.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, one problem here is that you haven't added anything to the change yet (which is why you could run into the issue below about dusty change). So this check should actually confirm that there is enough left over to have more than dust as change. if you add a property to the analyzer needsChange that just checks if there is leftover funds greater than the dust threshold, then you can do something like this:

if (
      newTxTemplate.feeRateSatisfied && 
      new BigNumber(newTxTemplate.currentFee).gte(txAnalyzer.minimumRbfFee) &&
      newTxTemplate.needsChange
    ) {
      // Stop adding inputs when the following conditions are met:
      // 1. The fee rate satisfies the target rate (which must be gte original tx fee rate
      // 2. The current fee is gte the minimum required absolute RBF fee amount
      // 3. There is enough left over for change
      break;
    }

With this the checks on the change aren't necessary and the extra validation afterwards also probably isn't necessary.

Note, this is probably only necessary for cancelation since you need at least one output. For accelerations, you probably already have a change output, or at least have at least one other output and can remove the change if necessary.

BigNumber.max(newTxTemplate.targetFeesToPay, txAnalyzer.minimumRBFFee),
)
) {
// Stop adding inputs when both conditions are met:
// 1. The fee rate satisfies the target rate
// 2. The current fee is greater than or equal to the maximum of:
// a) The target fee based on the desired rate by user
// b) The minimum fee required for a valid RBF replacement
break;
}
// Skip if this UTXO is already added
if (utxo.txid === originalInput.txid && utxo.vout === originalInput.vout) {
Expand All @@ -148,8 +161,14 @@ export const createCancelRbfTransaction = (
}

// Step 7: Calculate and set the cancellation output amount

const totalInputAmount = new BigNumber(newTxTemplate.getTotalInputAmount());
const fee = new BigNumber(newTxTemplate.targetFeesToPay);
// Ensure we're using the higher of the target fee and the minimum RBF fee
const fee = BigNumber.max(
newTxTemplate.targetFeesToPay,
txAnalyzer.minimumRBFFee,
);
// The cancel output receives all funds minus the fee
const cancelOutputAmount = totalInputAmount.minus(fee);

if (cancelOutputAmount.isLessThan(dustThreshold)) {
Expand All @@ -169,8 +188,29 @@ export const createCancelRbfTransaction = (

// Step 8: Ensure the new transaction meets RBF requirements
Copy link
Contributor

Choose a reason for hiding this comment

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

this could be its own function as well that's used in both.

Copy link
Contributor

Choose a reason for hiding this comment

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

A lot of this could probably be simplified too. This should throw if:

  1. new template fee is less than analyzer min RBF fee
  2. target fees are not paid
  3. the transaction creates dust
  4. the template doesn't validate.

const currentFee = new BigNumber(newTxTemplate.currentFee);
const minRequiredFee = new BigNumber(analysis.fee).plus(
txAnalyzer.minimumRBFFee,
const minRequiredFee = new BigNumber(txAnalyzer.minimumRBFFee);

console.log(
"testing",
"original tx size",
analysis.vsize,
"original tx fees",
analysis.fee,
"original tx feeRate",
analysis.feeRate,
);

console.log(
"\n modified",
"bumped tx size",
newTxTemplate.estimatedVsize,
"bumped tx fees",
newTxTemplate.currentFee,
"bumped tx targetFeesToPay",
newTxTemplate.targetFeesToPay,
"bumped tx feeRate",
newTxTemplate.estimatedFeeRate,
newTxTemplate,
);

if (currentFee.isLessThan(minRequiredFee)) {
Expand Down Expand Up @@ -336,8 +376,18 @@ export const createAcceleratedRbfTransaction = (

// Step 6: Add more inputs if necessary to meet fee requirements
for (const utxo of availableInputs) {
if (newTxTemplate.feeRateSatisfied && newTxTemplate.areFeesPaid()) {
break; // Stop adding inputs once fee requirements are met
if (
newTxTemplate.feeRateSatisfied &&
new BigNumber(newTxTemplate.currentFee).gte(
BigNumber.max(newTxTemplate.targetFeesToPay, txAnalyzer.minimumRBFFee),
)
) {
// Stop adding inputs when both conditions are met:
// 1. The fee rate satisfies the target rate
// 2. The current fee is greater than or equal to the maximum of:
// a) The target fee based on the desired rate by user
// b) The minimum fee required for a valid RBF replacement
break;
}
// Skip if this UTXO is already added
if (
Expand All @@ -356,7 +406,11 @@ export const createAcceleratedRbfTransaction = (
const totalOutputAmount = new BigNumber(
newTxTemplate.getTotalOutputAmount(),
);
const fee = new BigNumber(newTxTemplate.targetFeesToPay);
// Ensure we're using the higher of the target fee and the minimum RBF fee
const fee = BigNumber.max(
newTxTemplate.targetFeesToPay,
txAnalyzer.minimumRBFFee,
);
const changeAmount = totalInputAmount.minus(totalOutputAmount).minus(fee);

if (changeAmount.gt(dustThreshold)) {
Expand All @@ -381,9 +435,7 @@ export const createAcceleratedRbfTransaction = (

// Step 8: Ensure the new transaction meets RBF requirements
const currentFee = new BigNumber(newTxTemplate.currentFee);
const minRequiredFee = new BigNumber(analysis.fee).plus(
txAnalyzer.minimumRBFFee,
);
const minRequiredFee = new BigNumber(txAnalyzer.minimumRBFFee);

if (currentFee.isLessThan(minRequiredFee)) {
if (strict) {
Expand Down