Pass in smart fee slider value to coin control dialog #10582
Conversation
|
Forgot to mention this should definitely be tagged 0.14 It's broken in the entire 0.14 branch |
| CoinControlDialog::payAmounts.clear(); | ||
| CoinControlDialog::fSubtractFeeFromAmount = false; | ||
| if (ui->radioSmartFee->isChecked()) { | ||
| CoinControlDialog::coinControl->nConfirmTarget = ui->sliderSmartFee->maximum() - ui->sliderSmartFee->value() + 2; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Yes the use of static here has always annoyed me too, but indeed that's not the focus here.
|
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. |
|
Thanks for the fix. |
|
@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? |
Since cfe77ef the global nTxConfirmTarget wasn't being updated by the smart fee slider and thus the coin control dialog and labels were not being updated.
morcos
force-pushed
the
fixcoincontrolfee
branch
from
3e7cbeb to
e9cd778
Compare
|
oops, forgot to replace the global nTxConfirmTarget in one more place |
|
@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 ***@***.***> 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
<#10582 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGGmv1uHYI_4EZpOeGA28bhh1dcIbZ_Zks5sD--5gaJpZM4N5AZe>
.
|
|
@MarcoFalke yes, just display opened #10588 |
|
utACK e9cd778 |
e9cd778 Pass in smart fee slider value to coin control dialog (Alex Morcos) Tree-SHA512: 3899c3eb89b06e9cc842b33fabcce40a84fcc3a88ac2b02861f63419925312ac2a9f632567c02b0a060f5c5cd55f337e35e99b80535d1c1b8fcb7fd0c539f3c0
…ialog e9cd778 Pass in smart fee slider value to coin control dialog (Alex Morcos) Tree-SHA512: 3899c3eb89b06e9cc842b33fabcce40a84fcc3a88ac2b02861f63419925312ac2a9f632567c02b0a060f5c5cd55f337e35e99b80535d1c1b8fcb7fd0c539f3c0
6 participants
Since cfe77ef 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.