Pass in smart fee slider value to coin control dialog #10582

1. 
   morcos commented at 7:58 PM on June 13, 2017: member

   Since cfe77ef41 the global nTxConfirmTarget wasn't being updated by the smart fee slider and thus the coin control dialog and labels were not being updated.

   I'm not sure this is the ideal way to pass this information around, I'm open to suggestions.

2. 
   morcos commented at 7:59 PM on June 13, 2017: member

   Forgot to mention this should definitely be tagged 0.14 Not sure if it's worth holding up 0.14.2 @laanwj

   It's broken in the entire 0.14 branch

3. in src/qt/sendcoinsdialog.cpp:826 in 3e7cbeb23a outdated

    821 | @@ -822,6 +822,12 @@ void SendCoinsDialog::coinControlUpdateLabels()
    822 |      // set pay amounts
    823 |      CoinControlDialog::payAmounts.clear();
    824 |      CoinControlDialog::fSubtractFeeFromAmount = false;
    825 | +    if (ui->radioSmartFee->isChecked()) {
    826 | +        CoinControlDialog::coinControl->nConfirmTarget = ui->sliderSmartFee->maximum() - ui->sliderSmartFee->value() + 2;
   

   ---

   ---

   

   ryanofsky commented at 8:08 PM on June 13, 2017:

   Seems strange that all these CoinControlDialog members are static, but of course the pr isn't changing that. This seems like a clean fix given the current organization.

   ---

   

   laanwj commented at 8:13 PM on June 13, 2017:

   Yes the use of static here has always annoyed me too, but indeed that's not the focus here.

4. laanwj added this to the milestone 0.14.3 on Jun 13, 2017
5. 
   laanwj commented at 8:09 PM on June 13, 2017: member

   I've tagged it for 0.14.3. If there's any issues with 0.14.2 we can include it in the next rc, but it seems that rc2 can go straight to release.

6. 
   jonasschnelli commented at 8:26 PM on June 13, 2017: contributor

   Thanks for the fix. Tested ACK: 3e7cbeb23aa97c55d2fd9656662a593a2769ec5c

7. fanquake added the label GUI on Jun 14, 2017
8. jonasschnelli added the label Needs backport on Jun 14, 2017
9. 
   morcos commented at 2:35 PM on June 14, 2017: member

   @laanwj is it too late to change release notes for 0.14.2? do you think it's at least worth mentioning that it is a known bug. might save some people some confusion?

10. Pass in smart fee slider value to coin control dialog

    Since cfe77ef41 the global nTxConfirmTarget wasn't being updated by the smart
    fee slider and thus the coin control dialog and labels were not being updated.

    e9cd7786da
11. morcos force-pushed on Jun 14, 2017
12. 
    morcos commented at 2:59 PM on June 14, 2017: member

    oops, forgot to replace the global nTxConfirmTarget in one more place

13. 
    MarcoFalke commented at 6:04 PM on June 14, 2017: member

    @morcos As far as I see, 0.14.2 is not tagged, so you are still free to fixup the release notes. Just make sure to ping wumpus on irc when you have opened the pull.

    I haven't looked at the code, but if this is just a display issue, it is probably fine to only mention in the release notes. If this is actually ignoring the value from the slider, and users that chose a slow confirm target are effectively overpaying, we should hold back 0.14.2 for a few days.

    On Wed, Jun 14, 2017 at 4:35 PM, Alex Morcos notifications@github.com wrote:

    @laanwj https://github.com/laanwj is it too late to change release notes for 0.14.2? do you think it's at least worth mentioning that it is a known bug. might save some people some confusion?

    — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/pull/10582#issuecomment-308450949, or mute the thread https://github.com/notifications/unsubscribe-auth/AGGmv1uHYI_4EZpOeGA28bhh1dcIbZ_Zks5sD--5gaJpZM4N5AZe .

14. 
    morcos commented at 6:36 PM on June 14, 2017: member

    @MarcoFalke yes, just display

    opened #10588

15. 
    laanwj commented at 10:52 AM on June 15, 2017: member

    utACK e9cd778

16. laanwj merged this on Jun 15, 2017
17. laanwj closed this on Jun 15, 2017

    ---

18. laanwj referenced this in commit 7c72fb99af on Jun 15, 2017
19. fanquake removed the label Needs backport on Mar 7, 2018
20. PastaPastaPasta referenced this in commit 11a23bec7f on Jul 5, 2019
21. PastaPastaPasta referenced this in commit 0d8411b053 on Jul 5, 2019
22. PastaPastaPasta referenced this in commit 5284d6ae87 on Jul 6, 2019
23. PastaPastaPasta referenced this in commit 98a92dc209 on Jul 8, 2019
24. PastaPastaPasta referenced this in commit 7f6dbcbc7b on Jul 9, 2019
25. PastaPastaPasta referenced this in commit 3bae86b62d on Jul 9, 2019
26. barrystyle referenced this in commit 4a28e3084f on Jan 22, 2020
27. DrahtBot locked this on Sep 8, 2021

Contributors

morcosryanofskylaanwjjonasschnelliMarcoFalke

Milestone
0.14.3