Removed unused confirmation target dropdown from the UI
ClosedPublic
Actions

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

Details

Reviewers

deadalnix
Fabien

Group Reviewers

Restricted Project

Commits

rSTAGING43d3860f4c0b: Removed unused confirmation target dropdown from the UI
rABC43d3860f4c0b: Removed unused confirmation target dropdown from the UI

Summary

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
test_runner.py
bitcoin-qt

Screenshot from 2019-03-04 10-35-58.png (598×834 px, 124 KB)

Diff Detail

Repository

rABC Bitcoin ABC

Branch

uiconftarget

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 5126
Build 8315: Bitcoin ABC Buildbot (legacy)
Build 8314: arc lint + arc unit

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

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

jasonbcox added inline comments.Mar 4 2019, 18:42

src/qt/sendcoinsdialog.cpp
734	This gets cleaned up in D2614.

jasonbcox added a child revision: D2614: Remove unused param from txmempool::estimateFee.Mar 4 2019, 18:42

jasonbcox mentioned this in D2614: Remove unused param from txmempool::estimateFee.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B5126: Diff 7571.Mar 4 2019, 18:54

Fabien requested changes to this revision.Mar 4 2019, 20:01

Fabien added inline comments.

src/qt/sendcoinsdialog.cpp

258

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) {
    settings.remove("nSmartFeeSliderPosition");
}
if (settings.value("nConfTarget").toInt() != 0) {
    settings.remove("nConfTarget");
}

This revision now requires changes to proceed.Mar 4 2019, 20:01

Cleanup nSmartFeeSliderPosition settings

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

jasonbcox added inline comments.Mar 4 2019, 20:34