Merge #10757: RPC: Introduce getblockstats to plot things #3058
Conversation
test/functional/rpc_getblockstats.py
Outdated
There was a problem hiding this comment.
What are the extra three lines added contrary to the initial pull
There was a problem hiding this comment.
Good question :) That's because I converted the test to use mocktime and not realtime when generating blocks for the test thus I needed https://github.com/dashpay/dash/pull/3058/files/0aa34ea89cf3630342e9ed6aa49291652cdccc83#diff-dbccfb6d2ff211cd44b8f5e532e3238aR13 and also, we have no set_test_params functionality yet so had to convert it to def __init__(self): + super().__init__() https://github.com/dashpay/dash/pull/3058/files/0aa34ea89cf3630342e9ed6aa49291652cdccc83#diff-dbccfb6d2ff211cd44b8f5e532e3238aR47.
get_mocktime is deprecated after #3049 however, so I rebased it (with no changes) and added 3a01dfb on top.
41d0476 Tests: Add data file (Anthony Towns) 4cbfb6a Tests: Test new getblockstats RPC (Jorge Timón) 35e77a0 RPC: Introduce getblockstats (Jorge Timón) cda8e36 Refactor: RPC: Separate GetBlockChecked() from getblock() (Jorge Timón) Pull request description: It returns per block statistics about several things. It should be easy to add more if people think of other things to add or remove some if I went too far (but once written, why not keep it? EDIT: answer: not to test or maintain them). The currently available options are: minfee,maxfee,totalfee,minfeerate,maxfeerate,avgfee,avgfeerate,txs,ins,outs (EDIT: see updated list in the rpc call documentation) For the x axis, one can use height or block.nTime (I guess I could add mediantime if there's interest [EDIT: nobody showed interest but I implemented mediantime nonetheless, in fact there's no distinction between x or y axis anymore, that's for the caller to judge]). To calculate fees, -txindex is required. Tree-SHA512: 2b2787a3c7dc4a11df1fce62c8a4c748f5347d7f7104205d5f0962ffec1e0370c825b49fd4d58ce8ce86bf39d8453f698bcd46206eea505f077541ca7d59b18c
| const uint256 hash = ParseHashV(request.params[0], "parameter 1"); | ||
| if (mapBlockIndex.count(hash) == 0) | ||
| throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); | ||
| pindex = mapBlockIndex[hash]; |
There was a problem hiding this comment.
Just a nit and I'm fine if you leave it this way, but I'm generally a big fan of .at(key) instead of the [] operator in cases you expect that the value is present 100% of the time and that non-presence is actually a bug. .at(key) will assert in case the value is not present while [] will create a default initialized value and return it (which will be much worse then an immediate crash).
There was a problem hiding this comment.
Yeah. But that's the way it's implemented in many other places as well and this thing is going to be refactored later, so I'd prefer to keep it as is.
5 participants
Fixes #2519
This rpc was introduced in Bitcoin Core 0.17 and we are still finishing 0.15 backports. However, it looks more or less trivial to backport this one and it shouldn't cause merge conflicts in the future as far as I can see. I dashified it already i.e.
s/satoshis/duffs/, dropped segwit related stats and switched some stats to usesize/bytesinstead ofweight/vbytesin calculations.Note:
getblockstats somehashis kind of broken which is a known issue bitcoin#15412 and it's not fixed yet (because it requires some deeper/controversial changes in rpc module itself).Original commits:
41d0476 Tests: Add data file (Anthony Towns)
4cbfb6a Tests: Test new getblockstats RPC (Jorge Timón)
35e77a0 RPC: Introduce getblockstats (Jorge Timón)
cda8e36 Refactor: RPC: Separate GetBlockChecked() from getblock() (Jorge Timón)
Original pull request description: