Page MenuHomePhabricator

Remove unused param from txmempool::estimateFee
ClosedPublic

Authored by jasonbcox on Feb 26 2019, 19:32.

Details

Summary

Cleanup to help simplify the coincontrol logic being backported in T521.
Depends on D2613, D2641, D2662
Split out from D2543 for better review

Test Plan

ninja check
test_runner.py rpc_estimatefee

Diff Detail

Repository
rABC Bitcoin ABC
Branch
arcpatch-D2614
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 5128
Build 8319: Bitcoin ABC Buildbot (legacy)
Build 8318: arc lint + arc unit

Event Timeline

src/qt/sendcoinsdialog.cpp
806 ↗(On Diff #7490)

Even if this is correct, it can look like a bug for the user. If the target number of block selected is changed, the labels keeps displaying that the target is the next block (and the fee amount won't change).
Maybe this label should keep updating for now, and get removed together with this useless block target selection box ?

src/qt/sendcoinsdialog.cpp
806 ↗(On Diff #7490)

Screenshot would be nice.

Here are the screenshots

Before: fees not updating but block number matches the user selection

before.PNG (596×890 px, 67 KB)

After: neither fee nor block number gets updated

after.PNG (596×890 px, 67 KB)

deadalnix requested changes to this revision.Mar 2 2019, 00:43

Look like removing the option from the GUI is the first step here.

This revision now requires changes to proceed.Mar 2 2019, 00:43
jasonbcox edited the summary of this revision. (Show Details)

Done: D2641
Still need to rebase this diff.

src/qt/sendcoinsdialog.cpp
749 ↗(On Diff #7585)

Changing this requires to figure out the whole translation business. Maybe we do not want to translate it, but we need to at least update the translations files to reflect that change.

jasonbcox added inline comments.
src/qt/sendcoinsdialog.cpp
749 ↗(On Diff #7585)

hm I thought those files got auto generated as part of make but obviously that isn't true since there are none in this diff.

Updated translation strings

Going to split out refreshing the translation strings for unrelated changes since it's rather large.

Going to split out refreshing the translation strings for unrelated changes since it's rather large.

Rebase on D2662 to cleanly update translation strings

This revision is now accepted and ready to land.Mar 7 2019, 17:18
This revision was automatically updated to reflect the committed changes.