Skip to content

fix(wallet): allow fee bump without opt-in RBF signaling#430

Open
alienx5499 wants to merge 1 commit intobitcoindevkit:masterfrom
alienx5499:fix/65-fee-bump-without-rbf
Open

fix(wallet): allow fee bump without opt-in RBF signaling#430
alienx5499 wants to merge 1 commit intobitcoindevkit:masterfrom
alienx5499:fix/65-fee-bump-without-rbf

Conversation

@alienx5499
Copy link
Copy Markdown

Description

Fixes #65.

Wallet::build_fee_bump previously refused to build a replacement when no input signaled opt-in RBF (nSequence0xFFFFFFFD). That guard is removed so the wallet can construct a fee-bump transaction even if the original did not opt in. Whether a replacement is accepted is a mempool / relay policy concern (e.g. default full-RBF in recent Bitcoin Core), not something this API can enforce by requiring a specific nSequence on the prior tx.

Also removes BuildFeeBumpError::IrreplaceableTransaction, since that path is no longer reachable.

Notes to the reviewers

  • API: BuildFeeBumpError::IrreplaceableTransaction is removed (breaking public enum). If you prefer a deprecation cycle for the variant only, say so and we can adjust.
  • Semantics: Callers could previously rely on an error when the original tx did not signal opt-in RBF; they must not assume that anymore.
  • Tests: tests/build_fee_bump.rs — the old test expected failure for non–opt-in-RBF; it now asserts a successful fee bump with a higher fee rate.

Changelog notice

fix(wallet): allow build_fee_bump without opt-in RBF on the original tx; remove IrreplaceableTransaction (#65)

Checklists

All Submissions:

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@alienx5499 alienx5499 force-pushed the fix/65-fee-bump-without-rbf branch from 9ee74f9 to db7feac Compare April 3, 2026 21:09
@ValuedMammal ValuedMammal added the api A breaking API change label Apr 3, 2026
@ValuedMammal ValuedMammal moved this to In Progress in BDK Wallet Apr 3, 2026
@ValuedMammal ValuedMammal added this to the Wallet 4.0.0 milestone Apr 3, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.04%. Comparing base (fb7681a) to head (db7feac).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #430   +/-   ##
=======================================
  Coverage   80.04%   80.04%           
=======================================
  Files          24       24           
  Lines        5336     5326   -10     
  Branches      242      241    -1     
=======================================
- Hits         4271     4263    -8     
+ Misses        987      985    -2     
  Partials       78       78           
Flag Coverage Δ
rust 80.04% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@alienx5499
Copy link
Copy Markdown
Author

Hi @ValuedMammal, gentle ping for review when you have time. Thanks!

@ValuedMammal
Copy link
Copy Markdown
Collaborator

Approach ACK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api A breaking API change

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

Allow bump fee on transactions without RBF sequence

2 participants