Page MenuHomePhabricator

Remove unused param from txmempool::estimateFee
ClosedPublic

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

Details

Reviewers
deadalnix
Fabien
Group Reviewers
Restricted Project
Commits
rABCcf9a8488eb62: Remove unused param from txmempool::estimateFee
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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jasonbcox created this revision.Tue, Feb 26, 19:32
Herald added a reviewer: Restricted Project. · View Herald TranscriptTue, Feb 26, 19:32
Herald added a subscriber: schancel. · View Herald Transcript
Fabien added inline comments.Thu, Feb 28, 07:40
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 ?

deadalnix added inline comments.Fri, Mar 1, 00:39
src/qt/sendcoinsdialog.cpp
806 ↗(On Diff #7490)

Screenshot would be nice.

Fabien added a comment.Fri, Mar 1, 08:04

Here are the screenshots

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

After: neither fee nor block number gets updated

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

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

This revision now requires changes to proceed.Sat, Mar 2, 00:43
jasonbcox planned changes to this revision.Mon, Mar 4, 18:42
jasonbcox edited the summary of this revision. (Show Details)

Done: D2641
Still need to rebase this diff.

Fabien accepted this revision.Mon, Mar 4, 20:09
deadalnix added inline comments.Mon, Mar 4, 21:59
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 planned changes to this revision.Wed, Mar 6, 17:20
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.

jasonbcox updated this revision to Diff 7623.Wed, Mar 6, 18:20

Updated translation strings

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

jasonbcox planned changes to this revision.Wed, Mar 6, 18:31

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

jasonbcox updated this revision to Diff 7626.Wed, Mar 6, 18:38

Rebase on D2662 to cleanly update translation strings

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