Fixes #11344 and replaces #11428.
Before: <img width="588" alt="before" src="https://user-images.githubusercontent.com/10217/31984211-3299e81a-b993-11e7-94e9-bf63d2fed4bd.png">
After: <img width="578" alt="after" src="https://user-images.githubusercontent.com/10217/31984404-11f839da-b994-11e7-86ad-4c17a7d44b86.png">
I didn't replace bitcoin_en.ts because apparently it's generated by a script. If that's the case though, shouldn't it be removed from the repo?
This makes the Replace-by-fee aspect of it a bit too subtle IMO. Increasing the transaction fee for confirmations is not the only benefit, the fee increase is just the mechanism by which the transaction can be replaced. But still perhaps an improvement, the everyday user might not need to be bothered by the details of RBF. utACK on the tooltip change at least
@MeshCollider note that the counterpart to this feature #9697 also only refers to the fee bumping aspect of RBF.
The segwit.py tests failed in one of the Travis builds, presumably unrelated? Might be related to #10869?
Indeed unrelated, I've restarted it for you.
@MeshCollider note that the counterpart to this feature #9697 also only refers to the fee bumping aspect of RBF.
That is true, fair enough. I guess users of the GUI probably shouldn't be concerned, so utACK https://github.com/bitcoin/bitcoin/pull/11556/commits/115739acbe477e52716382e79732e38d34890d4c
1107 | @@ -1108,10 +1108,10 @@ 1108 | <item> 1109 | <widget class="QCheckBox" name="optInRBF"> 1110 | <property name="text"> 1111 | - <string>Request Replace-By-Fee</string> 1112 | + <string>Allow Fee Increase</string>
not exactly sure how it works in English, but shouldn't "fee increase" be lowercased? Capitalizing made sense for "Replace-By-Fee" since it is more of a name, but now we're using generic nouns
The UI uses a mix of "Upper lower case" and "All Upper Case". For example "Custom change address", "Subtract fee from amount" vs. "Pay To", "Coin Control Features". In addition some sentences end with a period, others don't.
I'm happy to change it either way, but it might be better to have a single pull request to implement (and document) a consistent style across the wallet.
:+1: lower case.
1107 | @@ -1108,10 +1108,10 @@ 1108 | <item> 1109 | <widget class="QCheckBox" name="optInRBF"> 1110 | <property name="text"> 1111 | - <string>Request Replace-By-Fee</string> 1112 | + <string>Allow Fee Increase</string> 1113 | </property> 1114 | <property name="toolTip"> 1115 | - <string>Indicates that the sender may wish to replace this transaction with a new one paying higher fees (prior to being confirmed).</string> 1116 | + <string>If the transaction takes a long time to confirm, this allows you to increase the fee later ("Replace-By-Fee", BIP 125). The recommended fee will be lower.</string>
maybe shorten it to "If the transaction takes too long to confirm" ?
This is one of the possible use cases. How about cancelling? And there is also the confirmations detail that concerns the recipient. There are too many details in the BIP to make this copy simple.
I'm more fond of the previous tooltip as it doesn't mention confirmation time. Maybe just add BIP 125 reference?
I'm more fond of the previous tooltip as it doesn't mention confirmation time
Agree, and would at least shift the emphasis away if this needs to be kept, changing from "If the transaction takes a long time to confirm, this allows you to increase the fee later" to "This allows you to increase the fee later if the transaction takes a long time to confirm."
It might also be clearer to change "The recommended fee will be lower" to ""This will cause the recommended fee to be lower."
@promag cancelling a transaction sounds like a very useful feature. My suggestion though would be to introduce a Cancel button in a future PR, and then modify the copy of this checkbox. If we change it to "Allow Fee Increase, Cancellation and More" people will expect those features to exist in the wallet.
Another option would be to have the label read "Mark as replaceable" (Electrum uses similar wording I think). This would be more future-proof for a potential cancellation feature, because then you'd only have to adapt the tooltip, and it's still an improvement over "Request Replace-By-Fee"
Sometimes I think this option shouldn't be in the fee section. From the user point of view, either make the transaction final/locked or allow it to be replaced/bumped. Something like:
@promag that is also a nice idea, but I would prefer a wording like "mark transaction as final" (or immutable or something to that effect). For not-so-technical users, "lock" has a security implication (like those little green locks you see on https pages, or lock symbols that indicate protected files), and I don't think we'd want users to think rbf transactions are somehow less secure
@flack right, my main point is to not mention fee as it can confuse the user as in what's the deal with fee and replace transaction.... For the curious/technical user the BIP reference can be enough?
Edit: for sure out of scope!
"Allow Fee Increase" is also out of context, that is not the purpose of the option, as in why would I want to be able to increase the fee?.
As it is, NACK.
@promag like I said, I like your suggestion to move the checkbox somewhere else, esp. when more rbf functionality gets added to the GUI. But (as mentioned above), the only thing the GUI currently lets you do with the rbf checkbox is to bump the transaction fee later on. So the idea of this PR is to help the user make the connection between the rbf checkbox and the bumpfee command.
If it's likely that more rbf functionality gets added to the GUI in the forseeable future, I'd say a relatively generic label (like "mark as replaceable", "mark as final") would be best, but for now I'd mention fees either way, since it's the only visible effect the checkbox currently has (i.e enables bumpfee, and gives you a lower estimate)
@petertodd any thoughts on this?
An alternative pull request could be one where we turn on RBF by default. In that case there's no need to explain what it's for or persuade users why they should tick this box. That would allow moving the button elsewhere in the send UI and just calling it "Disable Replace-By-Fee". If such a PR would be uncontentious, it would have my preference over this one, but I'm not sufficiently aware of the downsides.
Concept ACK. The new text is better understandable, though, it's not as prices as the current text (okay to me). "Allow fee increase" does not include that one could allow to add an output... but IMO neglect able.
I would also prefer @promag approach of flipping the default (PRs welcome).
Not sure if we should flip the bitcoind -walletrbf or if we should set -walletrbf=1 in GUI mode.
@jonasschnelli I wasn't suggesting to change the defaults. I was suggesting to think from the user perspective. I believe the use case is to choose whether the transaction should be final or not. As such that option shouldn't be in the "Transaction Fee" section.
Anyway, RBF enabled by default sounds good to me too.
@promag I don't think unexperienced users think that way. They already assume a transaction is final, because that's what they're used to from the legacy fiat world. Only when they send a transaction and find out it's stuck, they start looking for ways to fix that acute problem.
The way I see it (subjective of course) is that bumping fees is a feature, while the fact that this feature uses RBF is an implementation detail. If in the future we discover a different way to bump fees without RBF, the button could stay the same.
I'm trying to understand your NACK though (as opposed to just a preference). Do you believe something would break? Are you worried that users won't tick the box if it doesn't show the full range of potential uses? Or that it's unmaintainable (to me it seems trivial to change this again later).
It sounds like it's worth trying to flip the default first; if that PR doesn't raise objections then we can just close this one. If it does, we can continue the discussion.
utACK 115739acbe477e52716382e79732e38d34890d4c. Code change seems fine, and I think this is an improvement over the status quo, though also I agree with the concerns above. FWIW, I like the "Mark as replaceable" suggestion, and would also suggest "Allow fee adjustment" or "Allow increasing fees" as alternatives to "Allow Fee Increase" because "Fee Increase" sounds to me like something that could happen outside of your control.
Whatever text you decide to use for the checkbox, it would probably be good to repeat it in the confirmation dialog here, so it is clear that they are referring to the same option:
Rebased and tweaked the text based on feedback.
I also removed the confirmation dialog @ryanofsky mentioned. It doesn't make sense to me to show a warning, if we don't explain what the risk is. Alternatively, the dialog could explain that some merchants may not like RBF. In that case, that should also go in the tooltip.
<img width="585" alt="schermafbeelding 2017-11-04 om 12 48 58" src="https://user-images.githubusercontent.com/10217/32405121-42851e74-c15f-11e7-9a5f-cf5b90c88b39.png">
I should also have a PR that enables RBF by default soon.
Oh wait, I thought that was a separate "are you sure?" kind of confirmation dialog. But it's just a line in the larger confirmation window. I'll put it back with some improvement.
Screenshot of updated confirmation screen: <img width="439" alt="confirmation" src="https://user-images.githubusercontent.com/10217/32405190-8027db44-c160-11e7-84af-a137da9ea47d.png">
maybe out of scope for this PR, but it might make sense to move the rbf line in the confirmation dialog next to where the fee is displayed (and then show it in smaller print, like the mBTC line)
Opt-in RBF checkbox uses less technical jargon and emphasises
the fee bump functionality (at the expense of not mentioning
other uses of RBF).
The transaction confirmation screen uses copy consistent with this.
utACK
utACK db0b7373fc990806a06b6ba8a27ba2d710ce23c8
ACK db0b7373fc990806a06b6ba8a27ba2d710ce23c8