Dialog for allowing the user to choose the change output when bumping a tx

[achow101]

Based on bitcoin/bitcoin#26467

Implements a GUI dialog for allowing the user to choose the output to reduce when bumping a transaction. This adds the functionality that was added to the RPC.

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	luke-jr, hebasto
Approach ACK	john-moffett, w0xlt

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

Conflicts

No conflicts as of last run.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

• "Choose which output is change." -> "Choose which output is the change." [The phrase "is change" is ungrammatical; "is the change" clarifies meaning.]
• "/** Dialog for choosing the change output when bumping fee" -> "/** Dialog for choosing the change output when bumping the fee" [Missing article "the" makes the phrase awkward and less clear.]

^2025-12-11

[Sjors]

Can you add a screenshot to the PR description (and one for the single-output case)?

[john-moffett]

Approach ACK

I think I'd prefer if the user had some more information about the outputs in the dialog. Maybe something like this, but worded better:

Maybe the actual change address isn't even needed, or could be revealed in a tooltip.

Also, if there's a single change output with sufficient funds for feebumping, then selecting "none" has basically no effect compared to selecting that output, as the wallet will always(?) deduct from that existing change output. Maybe this common case can be simplified by removing the "None" option?

[Sjors]

Can we be a bit more bold and just assume that a change address is change, and not show the dialog in that case? Ideally we don't show such a dialog more often than necessary.

[john-moffett]

That would prevent the case where you wanted to deduct the extra fee from the recipient, such as when you initially selected "Subtract fee from amount".

Ideally, you'd only show the dialog if that were selected previously, but I don't think it's persisted anywhere. Eg -

gui/src/qt/sendcoinsrecipient.h

Line 42 in 30f553d

bool fSubtractFeeFromAmount; // memory only

I could be wrong, though.

[luke-jr]

Concept ACK. I'd just show "Change" for the change, though. End users shouldn't need to know about change addresses.

[hebasto]

The first commit e1ea0ba fails to compile:

  CXX      qt/libbitcoinqt_a-walletmodel.o
qt/walletmodel.cpp: In member function ‘bool WalletModel::bumpFee(uint256, uint256&)’:
qt/walletmodel.cpp:486:41: error: no matching function for call to ‘interfaces::Wallet::createBumpTransaction(uint256&, wallet::CCoinControl&, std::vector<bilingual_str>&, CAmount&, CAmount&, CMutableTransaction&)’
  486 |     if (!m_wallet->createBumpTransaction(hash, coin_control, errors, old_fee, new_fee, mtx)) {
      |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ./qt/walletmodel.h:16,
                 from qt/walletmodel.cpp:9:
./interfaces/wallet.h:166:18: note: candidate: ‘virtual bool interfaces::Wallet::createBumpTransaction(const uint256&, const wallet::CCoinControl&, std::vector<bilingual_str>&, CAmount&, CAmount&, CMutableTransaction&, std::optional<unsigned int>)’
  166 |     virtual bool createBumpTransaction(const uint256& txid,
      |                  ^~~~~~~~~~~~~~~~~~~~~
./interfaces/wallet.h:166:18: note:   candidate expects 7 arguments, 6 provided
make: *** [Makefile:13550: qt/libbitcoinqt_a-walletmodel.o] Error 1

[achow101]

Rebased and fixed the build issue in the first commit.

[hebasto]

test_bitcoin-qt fails in the CI: https://github.com/bitcoin-core/gui/actions/runs/11186485220.

[w0xlt]

Approach ACK

[DrahtBot]

🚧 At least one of the CI tasks failed.
_{Task previous releases, depends DEBUG: https://github.com/bitcoin-core/gui/runs/31101606803
LLM reason (✨ experimental): The CI failure is due to a build error in bumpfeechoosechangedialog.cpp.}

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

• Possibly due to a silent merge conflict (the changes in this pull request being
  incompatible with the current code in the target branch). If so, make sure to rebase on the latest
  commit of the target branch.

• A sanitizer issue, which can only be found by compiling with the sanitizer and running the
  affected test.

• An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

[achow101]

Are you still working on this?

[hebasto]

Concept ACK.