Add information to "Confirm fee bump" window #186

nit: Just <br> is used in the most codebase in this repository.

Done. Replaced <br /> with <br>

nit: Could a full stop at the sentence end make it better?

This seems to confusing to normal people. They don't know what inputs or outputs are, nor necessarily care how Bitcoin works internally.

This seems to confusing to normal people.

:D

They don't know what inputs or outputs are, nor necessarily care how Bitcoin works internally.

Educational purpose, no?

Could it be re-worded without revealing of internal tx details?

nit: Could a full stop at the sentence end make it better?

Done

This seems to confusing to normal people. They don't know what inputs or outputs are, nor necessarily care how Bitcoin works internally.

"Inputs" is already mentioned in GUI in 'Coin Control Features", user who did the initial transaction by selecting inputs obviously cares about it and "Outputs" can be replaced with "Change Address" although it can mean different things but easier to understand.

Users also shouldn't need to know about internals.

Coin Control Features are an advanced feature.

Maybe just only append this when it's enabled?

Maybe just only append this when it's enabled?

This makes sense. What is the best way to know if coin control was used for a transaction? So that it can be used in if statement for this warning.

Also any thoughts on this issue? https://github.com/bitcoin/bitcoin/issues/20598

Extra inputs and outputs might be added even if "Coin Control" wasn't used?

AFAIK it isn't possible right now, we'd just need to assume the current checkbox state is what the user wants behaviour based on

IMO a generic warning message based on bumpfee docs is good enough for now. In long term we need to fix few other issues related to coin selection algorithm, coin control for RBF etc. It can add random inputs even when coin control is disabled.

With CC disabled, it was created with random inputs already...

Added if statement to check if CC is enabled

grammar nit, "which" needs a preceding comma

    questionString.append(tr("WARNING: New inputs and outputs can be added in replacement transaction if necessary, which affects privacy."));

An article ("a", "the", etc.) is also missing before the word "replacement"

"can be added...if necessary" is vague, would suggest being more precise: added by what, why would it be necessary.

An article ("a", "the", etc.) is also missing before the word "replacement"

Will add "the"

"can be added...if necessary" is vague, would suggest being more precise: added by what, why would it be necessary.

I rephrased 'adding inputs when necessary. It may add a new change output if one does not already exist.' from https://bitcoincore.org/en/doc/0.20.0/rpc/wallet/bumpfee/ Not sure what else can I add which even normal users can understand

Thanks for updating! (very kind but no need to credit me for a drive-by comment) can you squash to one commit?

Oh I see, sorry, this is based somewhat on the bumpfee help.

The command will pay the additional fee by reducing change outputs or adding inputs when necessary.
It may add a new change output if one does not already exist.
All inputs in the original transaction will be included in the replacement transaction.

I'm not sure this should have a screaming WARNING.

Maybe:

This will pay the additional fee by reducing change outputs or
adding inputs when necessary. It may add a new change output if one
does not already exist. These changes may potentially leak privacy.

can you squash to one commit?

Sure will do once we finalize the warning message. Does the below mentioned thing look okay?

WARNING: This will pay the additional fee by reducing change outputs or adding inputs when necessary. It may add a new change output if one does not already exist. These may potentially leak privacy.

IMO a generic message would work better with other issues open and less clarity on what's happening with coin selection. A long term solution will be:

1. Improve Coin Selection Algorithm
2. Provide details of replacement transaction with all inputs and outputs similar to fees.
3. Provide an option to use coin control while doing replacement transaction

But the above suggested message will be incorrect because the issue is more than just user not aware of how RBF works in bitcoin core wallet. It is also the coin selection algorithm when using a custom change address in the initial transaction. Original Issue: https://github.com/bitcoin/bitcoin/issues/20795

No extra inputs or outputs were added in this: https://tbtc.bitaps.com/f35a69b066d1ff79cb13bc6dba2eb345f239b2bfa268e3fb0ee06b28748bea99 when I didn't use "custom change address" for initial transaction

If you setlabel the change address before bumping the fee it causes Core not to recognize the change address as a change address and to therefore add a new input. I think it makes sense to label all your change UTXOs if you plan to use coin control for privacy. Without labelling your change how do you hope to prevent undesired UTXO linkage?

I wrote about this in bitcoin/bitcoin#20935.

I could fix the issue but I doubt it would see enough review; my fee rate work has just been sitting there for the past couple months.

As for the message, could s/will/may/, e.g.:

"Warning: This may pay the additional fee by reducing change outputs or adding inputs, when necessary. It may add a new change output if one does not already exist. These may potentially leak privacy."

@dooglus Thanks for sharing. Will do some test and respond in https://github.com/bitcoin/bitcoin/issues/20935 @jonatack Made changes in https://github.com/bitcoin-core/gui/commit/570ebc6238639d40c7d54bbf39857de9416f2ba2

you can run clang-format -i src/qt/walletmodel.cpp --lines=518:522 to format your code automatically

     if (getOptionsModel()->getCoinControlFeatures() == true) {
-    questionString.append("<br><br>");
-    questionString.append(tr("Warning: This may pay the additional fee by reducing change outputs or adding inputs, when necessary. It may add a new change output if one does not already exist. These may potentially leak privacy."));
+        questionString.append("<br><br>");
+        questionString.append(tr("Warning: This may pay the additional fee by reducing change outputs or adding inputs, when necessary. It may add a new change output if one does not already exist. These may potentially leak privacy."));
     }

Thanks

nit, not sure this comment is needed, but if you keep it

    // Display warning in the "Confirm fee bump" window if the "Coin Control Features" option is enabled

    if (getOptionsModel()->getCoinControlFeatures()) {

        questionString.append(tr("Warning: This may pay the additional fee by reducing change outputs or adding inputs, when necessary. It may add a new change output if one does not already exist. These changes may potentially leak privacy."));