[Qt] Improved copy for RBF checkbox and tooltip #11556

pull Sjors wants to merge 1 commits into bitcoin:master from Sjors:rbf-ui-text changing 2 files +3 −3
  1. Sjors commented at 6:56 am on October 25, 2017: member

    Fixes #11344 and replaces #11428.

    Before:

    After:

  2. Sjors commented at 7:00 am on October 25, 2017: member
    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?
  3. fanquake added the label GUI on Oct 25, 2017
  4. meshcollider commented at 7:08 am on October 25, 2017: contributor
    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
  5. Sjors commented at 7:42 am on October 25, 2017: member
    @MeshCollider note that the counterpart to this feature #9697 also only refers to the fee bumping aspect of RBF.
  6. Sjors commented at 7:47 am on October 25, 2017: member
    The segwit.py tests failed in one of the Travis builds, presumably unrelated? Might be related to #10869?
  7. meshcollider commented at 8:15 am on October 25, 2017: contributor

    @Sjors

    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

  8. in src/qt/forms/sendcoinsdialog.ui:1111 in 115739acbe outdated
    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>
    


    flack commented at 8:43 am on October 25, 2017:
    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

    Sjors commented at 8:55 am on October 25, 2017:

    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.


    promag commented at 2:00 pm on October 25, 2017:
    :+1: lower case.
  9. in src/qt/forms/sendcoinsdialog.ui:1114 in 115739acbe outdated
    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>
    


    flack commented at 8:47 am on October 25, 2017:
    maybe shorten it to “If the transaction takes too long to confirm” ?

    promag commented at 2:09 pm on October 25, 2017:
    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.

    promag commented at 3:06 pm on October 25, 2017:
    I’m more fond of the previous tooltip as it doesn’t mention confirmation time. Maybe just add BIP 125 reference?

    ryanofsky commented at 5:00 pm on October 30, 2017:

    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.”

  10. Sjors commented at 2:45 pm on October 25, 2017: member
    @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.
  11. flack commented at 3:08 pm on October 25, 2017: contributor
    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”
  12. promag commented at 3:08 pm on October 25, 2017: member

    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:

    • Lock transaction
  13. flack commented at 3:13 pm on October 25, 2017: contributor
    @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
  14. promag commented at 3:28 pm on October 25, 2017: member

    @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.

  15. flack commented at 3:44 pm on October 25, 2017: contributor

    @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)

  16. Sjors commented at 3:47 pm on October 25, 2017: member
    @petertodd any thoughts on this?
  17. promag commented at 10:35 pm on October 25, 2017: member
    @flack honestly I wouldn’t change the UI if there is consensus in adding such RBF functionality in an foreseeable future.
  18. Sjors commented at 2:29 am on October 26, 2017: member
    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.
  19. jonasschnelli commented at 6:22 am on October 26, 2017: contributor

    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.

  20. promag commented at 1:33 pm on October 26, 2017: member

    @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.

  21. Sjors commented at 2:05 am on October 27, 2017: member

    @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.

  22. ryanofsky commented at 5:19 pm on October 30, 2017: member
    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.
  23. ryanofsky commented at 2:38 pm on November 1, 2017: member

    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:

    https://github.com/bitcoin/bitcoin/blob/cffa5ee132f3ef6a8d138eb98d15da2b61999ca4/src/qt/sendcoinsdialog.cpp#L348

  24. Sjors force-pushed on Nov 4, 2017
  25. Sjors commented at 11:55 am on November 4, 2017: member

    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.

    I should also have a PR that enables RBF by default soon.

  26. Sjors commented at 11:58 am on November 4, 2017: member
    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.
  27. Sjors force-pushed on Nov 4, 2017
  28. Sjors commented at 12:05 pm on November 4, 2017: member
    Screenshot of updated confirmation screen:
  29. flack commented at 12:42 pm on November 4, 2017: contributor
    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)
  30. Sjors commented at 1:01 pm on November 4, 2017: member
    PR to turn RBF on by default: #11605
  31. [Qt] Improved copy: RBF checkbox, tooltip and confirmation screen
    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.
    db0b7373fc
  32. Sjors force-pushed on Nov 18, 2017
  33. Sjors commented at 12:57 pm on November 18, 2017: member

    Rebased. Not sure why Travis is unhappy.

    #11605 is causing even more discussion than this PR. Although I haven’t given up on it, it might take a while. In the mean time I suggest we consider merging this one.

  34. MarcoFalke added the label Docs and Output on Nov 27, 2017
  35. petertodd commented at 10:44 am on November 29, 2017: contributor
    utACK
  36. TheBlueMatt commented at 5:26 pm on December 4, 2017: member
    utACK db0b7373fc990806a06b6ba8a27ba2d710ce23c8
  37. jonasschnelli commented at 7:30 am on December 5, 2017: contributor
    ACK db0b7373fc990806a06b6ba8a27ba2d710ce23c8
  38. jonasschnelli merged this on Dec 5, 2017
  39. jonasschnelli closed this on Dec 5, 2017

  40. jonasschnelli referenced this in commit 91eeaa0335 on Dec 5, 2017
  41. Sjors deleted the branch on Dec 5, 2017
  42. laanwj referenced this in commit f19ca129ff on Dec 22, 2017
  43. DrahtBot locked this on Sep 8, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-18 21:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me