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

fix(wallet): use map_keychain in Wallet::build_fee_bump #1812

Merged
merged 2 commits into from
Jan 27, 2025
Merged

fix(wallet): use map_keychain in Wallet::build_fee_bump #1812

merged 2 commits into from
Jan 27, 2025

Conversation

ValuedMammal
Copy link
Contributor

@ValuedMammal ValuedMammal commented Jan 24, 2025 •
edited
Loading

Previously we failed to remove the change output if the wallet has no internal keychain which caused tx building to fail at the new higher feerate. Fix this by mapping the internal keychain to the de-facto change keychain so that the drain output can be recalculated.

fixes #1807

Changelog notice

Fixed

  • wallet: Fixed an issue preventing build_fee_bump from re-targeting the drain value for some wallets

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

Bugfixes:

  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@ValuedMammal
fix(wallet): use map_keychain in Wallet::build_fee_bump
53d16e1 
Previously we failed to remove the change output if the wallet has
no internal keychain which caused tx building to fail at the new
higher feerate. Fix this by mapping the internal keychain to the
de-facto change keychain so that the drain output can be
recalculated.
@ValuedMammal
test(wallet): test build_fee_bump with single descriptor wallet
515309e
@ValuedMammal ValuedMammal self-assigned this Jan 24, 2025
@ValuedMammal ValuedMammal added this to the 1.1.0 milestone Jan 24, 2025
@notmandatory notmandatory added the bug Something isn't working label Jan 24, 2025
notmandatory
notmandatory approved these changes Jan 24, 2025
View reviewed changes
Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

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

Thanks for catching this!

ACK 515309e

evanlinjin
evanlinjin approved these changes Jan 26, 2025
View reviewed changes
Copy link
Member

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

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

ACK 515309e

@ValuedMammal ValuedMammal merged commit ab4df29 into bitcoindevkit:master Jan 27, 2025
23 checks passed
@ValuedMammal ValuedMammal deleted the fix/build-fee-bump-single-desc branch January 27, 2025 15:16
@ValuedMammal ValuedMammal mentioned this pull request Feb 4, 2025
Closed
41 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@notmandatory notmandatory notmandatory approved these changes

@evanlinjin evanlinjin evanlinjin approved these changes

Assignees

@ValuedMammal ValuedMammal

Labels
bug Something isn't working
Projects
BDK
Archived in project
Milestone
1.1.0
Development

Successfully merging this pull request may close these issues.

[wallet] build_fee_bump fails to remove the change output for single descriptor wallet
3 participants
@ValuedMammal @notmandatory @evanlinjin