0.17 backport of #14602, but with named-arg account="*" compatibility
[0.17] Bugfix: Correctly calculate balances when min_conf is used, and for getbalance("*") #14782
pull luke-jr wants to merge 5 commits into bitcoin:0.17 from luke-jr:bugfix_rpc_getbalance_acctstar-0.17 changing 6 files +40 −29-
luke-jr commented at 5:36 AM on November 22, 2018: member
-
Wallet: Add trusted_only flag to GetBalance 73103ea7a0
-
Bugfix: RPC/Wallet: Restore getbalance("*") behaviour (include untrusted coins), while updating the API to accept a boolean as well 2141e62b6f
-
Bugfix: Wallet: When getting balance w/ min_conf, include UTXOs that were spent more recently c18bd54c75
-
lint: Remove incorrect check that all arguments are string or not eb013b350c
-
RPC/Wallet: Include "account" named arg alias for getbalance compatibility 32de31c1dd
- meshcollider added the label Wallet on Nov 22, 2018
- meshcollider added the label RPC/REST/ZMQ on Nov 22, 2018
- meshcollider added the label Backport on Nov 22, 2018
-
meshcollider commented at 5:37 AM on November 22, 2018: contributor
Concept ACK
- fanquake added this to the milestone 0.17.1 on Nov 22, 2018
-
gmaxwell commented at 8:48 AM on November 23, 2018: contributor
Concept ACK.
-
jonasschnelli commented at 5:47 AM on November 25, 2018: contributor
utACK 32de31c1dd3255702f655b0746cb4fac3cc1938f
-
jnewbery commented at 2:26 PM on November 28, 2018: member
I have the same comment for this PR as for the PR to master: #14602#pullrequestreview-179314689
This PR needs a few things before I'd consider it ready for merge:
- A rationale for why we'd want to provide this 'feature'. Summing up untrusted coins into a 'balance' can lead to very confusing results, such as negative balances and double-counting conflicting transactions. An uninformed user could see money 'disappear' or be tricked into thinking they've received money which they haven't.
- Full documentation of the usage. I removed
getbalance *here: https://github.com/bitcoin/bitcoin/pull/12953/files#diff-df7d84ff2f53fcb2a0dc15a3a51e55ceL822 because the option had been marked as deprecated. If we want to support this usage, then it should be properly documented, with appropriate warnings and caveats. - Full test coverage, so this doesn't get regressed again. I wrote some tests here: https://github.com/jnewbery/bitcoin/tree/untrusted_balance in the course of reviewing this PR.
- Not gratuitously breaking bitcoin-cli because you don't happen to use it.
Also for this backport:
- Not deleting linters because they fail on your branch(!)
-
luke-jr commented at 3:12 PM on November 28, 2018: member
The linter is broken by design.
- laanwj added this to the "Blockers" column in a project
- MarcoFalke removed this from the milestone 0.17.1 on Nov 30, 2018
- MarcoFalke added this to the milestone 0.17.2 on Nov 30, 2018
-
DrahtBot commented at 6:20 PM on December 1, 2018: member
<!--cf906140f33d8803c4a75a2196329ecb-->Needs rebase
- DrahtBot added the label Needs rebase on Dec 1, 2018
- MarcoFalke removed this from the "Blockers" column in a project
- luke-jr closed this on Jan 5, 2019
- laanwj removed the label Needs rebase on Oct 24, 2019
- DrahtBot locked this on Dec 16, 2021
Milestone
0.17.2