Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RPC] add 'getblockindexstats' function #911

Merged
merged 5 commits into from
Jun 18, 2019

Conversation

random-zebra
Copy link

This introduces a new RPC method getblockindexstats to get aggregated BlockIndex data for a range of blocks:

  • transaction count
  • transaction count (including coinbase/coinstake txes)
  • zPIV per-denom mint count
  • zPIV per-denom spend count
  • total transaction bytes
  • total fees in block range
  • average fee per kB

Since it expands the functionality of getfeeinfo, this method has been redirected to call getblockindexstats (over the last N blocks).

To avoid code duplication with getserials and getmintsinblocks, the initial validation of the input params has been refactored in the function validaterange().
This also contains a fix to prevent the wallet from crashing when calling these functions over out-of-range blocks.

@Fuzzbawls
Copy link
Collaborator

Concept ACK

Would love to see this covered in a functional test and possibly brought over to the REST API at some point. Will give it some testing soon.

@random-zebra
Copy link
Author

7980f23 fixes the fee calculation excluding zerocoin spends (which have no fee) and zerocoin mints (which have a fixed fee per-output).

26b1f0c introduces a separate counter for the upcoming (with v.0.3.3) "public spend" outputs (ref: #891)

Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found some nits in the RPC command help texts. not a big deal, and not the only ones that have styling inconsistencies overall. Can be fixed in a separate PR that unifies all RPC command help texts.

for reference, the formatting should be

"command \"required_string_args\" required_numbool_args ( \"optional_string_args\" optional_numbool_args )\n"

src/rpc/blockchain.cpp Outdated Show resolved Hide resolved
src/rpc/blockchain.cpp Outdated Show resolved Hide resolved
src/rpc/blockchain.cpp Outdated Show resolved Hide resolved
@Fuzzbawls Fuzzbawls added this to the 3.3.0 milestone Jun 14, 2019
@random-zebra
Copy link
Author

Fixed help texts styling

- getblockindexstats
- getmintsinblocks
- getserials
@Fuzzbawls Fuzzbawls added the Needs Release Notes Placeholder tag for anything needing mention in the "Notable Changes" section of release notes label Jun 16, 2019
Copy link

@Warrows Warrows left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK. I have a few remarks about the locking strategy.

src/rpc/blockchain.cpp Show resolved Hide resolved
src/rpc/blockchain.cpp Outdated Show resolved Hide resolved
src/rpc/blockchain.cpp Outdated Show resolved Hide resolved
src/rpc/blockchain.cpp Outdated Show resolved Hide resolved
src/rpc/blockchain.cpp Outdated Show resolved Hide resolved
- validaterange
- getfeeinfo
- geblockindexstats
- getmintsinblocks
- getserials
@random-zebra
Copy link
Author

Locking strategy has been updated guarding only the access to chainActive.
Performance is comparable with the previous implementation.
E.g. calling getblockindexstats on the same ranges of blocks:

- 2K blocks: 0.045 s
- 20K blocks:  0.562 s
- 200K blocks: 8.859 s
  • after
- 2K blocks: 0.038 s
- 20K blocks: 0.581 s
- 200K blocks: 8.831 s

Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK eea9915

Copy link

@Warrows Warrows left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK eea9915

@Fuzzbawls Fuzzbawls removed Needs Release Notes Placeholder tag for anything needing mention in the "Notable Changes" section of release notes labels Jun 17, 2019
@Fuzzbawls Fuzzbawls merged commit eea9915 into PIVX-Project:master Jun 18, 2019
Fuzzbawls added a commit that referenced this pull request Jun 18, 2019
eea9915 [RPC] fix locking strategy (random-zebra)
49ef846 [RPC] fix help texts (random-zebra)
26b1f0c [RPC] 'getblockindexstats': count public spends separately (random-zebra)
7980f23 [RPC] fix fee calculation in 'getblockindexstats' and 'getfeeinfo' (random-zebra)
689ac23 [RPC] add 'getblockindexstats' function (random-zebra)

Pull request description:

  This introduces a new RPC method `getblockindexstats` to get aggregated BlockIndex data for a range of blocks:
  - transaction count
  - transaction count (including coinbase/coinstake txes)
  - zPIV per-denom mint count
  - zPIV per-denom spend count
  - total transaction bytes
  - total fees in block range
  - average fee per kB

  Since it expands the functionality of `getfeeinfo`, this method has been redirected to call `getblockindexstats` (over the **last** N blocks).

  To avoid code duplication with `getserials` and `getmintsinblocks`, the initial validation of the input params has been refactored in the function `validaterange()`.
  This also contains a fix to prevent the wallet from crashing when calling these functions over out-of-range blocks.

ACKs for commit eea991:
  Fuzzbawls:
    ACK eea9915
  Warrows:
    ACK eea9915

Tree-SHA512: fd424d27b09133f8e1b646044f6e48cb7be8e7b7f6bbe51797b229e7e1c5d721226022915876d3810b38e7c2eb6bb089a7e3b8df2b0487ac251bde405e1f964a
@random-zebra random-zebra deleted the 2019_getblockindexstats branch September 24, 2020 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants