Add histunspent RPC #10804

pull promag wants to merge 2 commits into bitcoin:master from promag:2017-07-rpc-add-histunspent changing 4 files +104 −2

promag commented at 3:44 PM on July 12, 2017: member
This PR introduces histunspent RPC. It calculates an histogram for the current unspent transaction output amounts.

This allows to have a better perspective of how the total balance is distributed. This also avoids transmitting the whole UTXO to create the histogram on client side, specially when it has thousands of unspents.

For the moment, the histogram bins are defined with an array of amounts. Later there can be other constructors to define the bins.

Example:
```
./bitcoin-cli -regtest  listunspent | grep amount
    "amount": 50.00000000,
    "amount": 50.00000000,
    "amount": 4.00000000,
    "amount": 45.99996160,
    "amount": 50.00000000,
    "amount": 50.00000000,
    "amount": 36.99996160,
    "amount": 13.00000000,
    "amount": 50.00000000,

./bitcoin-cli -regtest  histunspent '[0,1,5,10,15,100]'
{
  "bins": [
    0, 
    1, 
    0, 
    1, 
    7
  ],
  "ranges": [
    0.00000000, 
    1.00000000, 
    5.00000000, 
    10.00000000, 
    15.00000000, 
    100.00000000
  ]
}
```
Note for reviewers, WIP and missing tests.
Handle null values in RPCTypeCheckArgument 052f97563f
[WIP] Add histunspent RPC b680b5b9ce
fanquake added the label RPC/REST/ZMQ on Jul 12, 2017
jnewbery commented at 9:44 PM on July 13, 2017: member

My immediate reaction to this is the same as #10757 (comment) and #9733 (comment). I'm not a fan of these statistical RPCs, which seem to me to be very interesting, but really only provide niche functionality. My issue is that they add maintenance burden for features that are only likely to be used by very few people. For me, they're better served by a fork of the project or a patch set. To my mind, this project should try to minimize functionality to what's essential for end users.

I understand that I may be in the minority in that view, and that it's probably discouraging for contributors for proposed PRs to be rejected, so I'm not going to push the point.
promag commented at 10:18 PM on July 14, 2017: member
I generally agree with you, this is easily done out but the problem is when the UTXO set is very large:
- listunspent can take minutes to complete;
- the result is a huge array;
- no pagination support
- locks held too much time.
The goal with this RPC, is to create a flexible UTXO counting to solve the above problems in one-shot.

Typically this is useful for online wallets to monitor the UTXO set, to trigger sweep transactions, to understand if there are good unspents for the upcoming transactions to have the least input count (hence low fee), etc..
promag commented at 10:21 PM on July 14, 2017: member

For me, they're better served by a fork of the project or a patch set. To my mind, this project should try to minimize functionality to what's essential for end users. @jnewbery IMO there is space for a basic statistical feature like this in core, which is by far one of the easiest around in code.
laanwj commented at 12:46 PM on July 17, 2017: member

I don't share @jnewbery's general resent against statistical calls. Sometimes the information to compute the statistics is available only server-side.

In this case, however, all the information is available in the listunspent result. So this can just as well be computed in an external script. Building it into bitcoind is an unnecessary burden.

For me, they're better served by a fork of the project or a patch set. To my mind, this project should try to minimize functionality to what's essential for end users.

So my point is that this doesn't even require a fork. All the information to compute is is already available.
promag commented at 1:03 PM on July 17, 2017: member

all the information is available in the listunspent @laanwj sure but it's not that feasible when it's a huge data set, even with listunspent query options.
promag commented at 2:49 PM on July 17, 2017: member

Closing since the general opinion is to compute client side. I guess the above bottlenecks should be tackled instead.
promag closed this on Jul 17, 2017
promag deleted the branch on Jul 17, 2017

in src/wallet/rpcwallet.cpp:2702 in b680b5b9ce

2697 | +
2698 | +    if (!request.params[0].isNull()) {
2699 | +        const std::vector<UniValue>& values = request.params[0].getValues();
2700 | +
2701 | +        if (values.size() < 2) {
2702 | +            // throw invalid range (minimum 2 amounts);

pedrobranco commented at 12:54 PM on July 19, 2017:

👀

DrahtBot locked this on Sep 8, 2021

Contributors

Labels