Page MenuHomePhabricator

Removed unused confirmation target dropdown from the UI

Authored by jasonbcox on Mar 4 2019, 18:40.



Confirmation targets do not affect tx fees, so having this on the UI is useless at best and confusing at worst. I'm also removing confirmation targets from txmempool::estimateFee's API in D2614, so there is no plan to use this in the future.

Test Plan

ninja check

Diff Detail

rABC Bitcoin ABC
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jasonbcox created this revision.Mar 4 2019, 18:40
Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 4 2019, 18:40
Herald added a subscriber: schancel. · View Herald Transcript
jasonbcox edited the test plan for this revision. (Show Details)Mar 4 2019, 18:41
jasonbcox added inline comments.Mar 4 2019, 18:42
734 ↗(On Diff #7571)

This gets cleaned up in D2614.

Fabien requested changes to this revision.Mar 4 2019, 20:01
Fabien added inline comments.
258 ↗(On Diff #7571)

This block of code is no longer needed.
If remove() deletes the entry from the configuration file, this could be also applied to nConfTarget in order to clean up the file and avoid confusion.
Maybe something like this:

// nSmartFeeSliderPosition and nConfTarget are obsolete.
// Remove them from the settings if they exist
if (settings.value("nSmartFeeSliderPosition").toInt() != 0) {
if (settings.value("nConfTarget").toInt() != 0) {
This revision now requires changes to proceed.Mar 4 2019, 20:01
jasonbcox updated this revision to Diff 7576.Mar 4 2019, 20:33

Cleanup nSmartFeeSliderPosition settings

jasonbcox added inline comments.Mar 4 2019, 20:34
258 ↗(On Diff #7571)

I agree. This should also be cleaned up in the next major version, so I've noted that in the comment.

Fabien accepted this revision.Mar 4 2019, 21:21
This revision is now accepted and ready to land.Mar 4 2019, 21:21
This revision was automatically updated to reflect the committed changes.