rpc: faster getblockstats using BlockUndo data #14802

pull FelixWeis wants to merge 1 commits into bitcoin:master from FelixWeis:201811_blockstats_undoblock changing 4 files +174 −187
  1. FelixWeis commented at 9:14 PM on November 25, 2018: contributor

    Using undo data for a block (rev?????.dat) we can retrieve value information about prevouts and calculate the final transaction fee (rate). This approach is about 80x faster, drops the requirement for -txindex, and works for all non-pruned blocks.

    # 2018-11-25T16:36:19Z Bitcoin Core version v0.17.99.0-edc715240-dirty (release build)
seq 550100 550200  0.00s user 0.00s system 62% cpu 0.004 total
xargs -n1 src/bitcoin-cli getblockstats  0.21s user 0.19s system 17% cpu 2.302 total


# 2018-11-25T16:39:17Z Bitcoin Core version v0.17.0 (release build)
seq 550100 550200  0.00s user 0.00s system 87% cpu 0.002 total
xargs -n1 src/bitcoin-cli getblockstats  0.24s user 0.22s system 0% cpu 3:19.42 total
  2. gmaxwell commented at 9:17 PM on November 25, 2018: contributor

    <3 I like this direction, certainly having a RPC that requires txindex is unfortunate, as does having rpcs that don't work (well) with pruning. I don't think it would have been worth it to store extra data to make stats fast, but since we already are...

  3. fanquake added the label RPC/REST/ZMQ on Nov 25, 2018
  4. in test/functional/rpc_getblockstats.py:115 in 602dd542a8 outdated 
     112 | @@ -128,8 +113,8 @@ def run_test(self):
 113 |              assert_equal(stats_by_hash, self.expected_stats[i])
 114 |  
 115 |              # Check with the node that has no txindex
    MarcoFalke commented at 10:59 PM on November 25, 2018:

    The getblockstats code seems to no longer call into any txindex code (not even as fallback), so might as well remove the txindexed node from the whole test?

    FelixWeis commented at 5:21 AM on February 9, 2019:

    test and testdata changed

  5. MarcoFalke commented at 11:01 PM on November 25, 2018: member

    Concept ACK

  6. DrahtBot commented at 11:25 PM on November 25, 2018: member

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    No conflicts as of last run.

  7. in src/validation.cpp:1477 in 602dd542a8 outdated 
    1450 | @@ -1451,6 +1451,36 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi
1451 |      return true;
1452 |  }
1453 |  
1454 | +bool UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex* pindex)
    promag commented at 12:00 PM on November 26, 2018:

    Could split move only to a different commit.

    FelixWeis commented at 5:22 AM on February 9, 2019:

    it will get squashed anyway no?

  8. in src/validation.h:392 in 602dd542a8 outdated 
     388 | @@ -388,12 +389,15 @@ class CScriptCheck
 389 |  void InitScriptExecutionCache();
 390 |  
 391 |  
 392 | +
    promag commented at 12:01 PM on November 26, 2018:

    Remove change.

  9. promag commented at 12:03 PM on November 26, 2018: member

    Great change!

    Mind adding a release note?

  10. jonasschnelli commented at 11:21 PM on December 9, 2018: contributor

    Clever idea! Concept ACK

  11. ajtowns commented at 12:17 AM on January 2, 2019: member

    ConceptACK, code looks good at first glance

  12. laanwj commented at 1:00 PM on January 22, 2019: member

    Good to remove reliance on the transaction index. utACK 602dd542a80d76225285ad9d52ebfc2a4d4e7c54

  13. DrahtBot added the label Needs rebase on Jan 30, 2019
  14. FelixWeis force-pushed on Feb 6, 2019
  15. DrahtBot removed the label Needs rebase on Feb 6, 2019
  16. FelixWeis force-pushed on Feb 8, 2019
  17. in src/validation.h:22 in 76fef2639e outdated 
      18 | @@ -19,6 +19,7 @@
  19 |  #include <script/script_error.h>
  20 |  #include <sync.h>
  21 |  #include <versionbits.h>
  22 | +#include <undo.h>
    MarcoFalke commented at 2:40 PM on February 9, 2019:

    style-nit: Could use a forward decl instead?

  18. in src/rpc/blockchain.cpp:1881 in 76fef2639e outdated 
    1875 | @@ -1876,6 +1876,10 @@ static UniValue getblockstats(const JSONRPCRequest& request)
1876 |      }
1877 |  
1878 |      const CBlock block = GetBlockChecked(pindex);
1879 | +    CBlockUndo blockUndo;
1880 | +    if (!UndoReadFromDisk(blockUndo, pindex)) {
1881 | +        error("BlockStats(): failure reading undo data");
    marcinja commented at 1:21 AM on February 14, 2019:

    nit: This could be throw JSONRPCError to stay consistent with GetBlockChecked and most of the other functions in this file. I'm not sure if there is a preferred approach though, so feel free to ignore.

  19. in doc/release-notes.md:303 in 76fef2639e outdated 
     297 | @@ -298,6 +298,9 @@ RPC
 298 |  - A new `submitheader` RPC allows submitting block headers independently
 299 |    from their block.  This is likely only useful for testing.
 300 |  
 301 | +- `getblockstats` speedup and drops `-txindex` requirement, now works on
 302 | +  all non-pruned blocks.
 303 | +
    marcinja commented at 2:03 AM on February 14, 2019:

    nit: This should be a full sentence.

    MarcoFalke commented at 2:19 PM on March 2, 2019:

    And a separate file ./doc/release-notes-14802.md

  20. marcinja commented at 2:08 AM on February 14, 2019: contributor

    utACK 76fef2639e

    Nice work!

  21. jonasschnelli added this to the milestone 0.19.0 on Feb 15, 2019
  22. DrahtBot added the label Needs rebase on Mar 2, 2019
  23. in src/rpc/blockchain.cpp:1985 in 76fef2639e outdated 
    1981 | @@ -1987,6 +1982,7 @@ static UniValue getblockstats(const JSONRPCRequest& request)
1982 |              maxfeerate = std::max(maxfeerate, feerate);
1983 |              minfeerate = std::min(minfeerate, feerate);
1984 |          }
1985 | +        tx_n++;
    MarcoFalke commented at 6:49 PM on March 19, 2019:

    looks fragile. (If some paths in the for loop continue, this would be skipped)

  24. in src/rpc/blockchain.cpp:1917 in 76fef2639e outdated 
    1912 | @@ -1913,6 +1913,7 @@ static UniValue getblockstats(const JSONRPCRequest& request)
1913 |      std::vector<std::pair<CAmount, int64_t>> feerate_array;
1914 |      std::vector<int64_t> txsize_array;
1915 |  
1916 | +    size_t tx_n = 1;
1917 |      for (const auto& tx : block.vtx) {
    MarcoFalke commented at 6:51 PM on March 19, 2019:

    style-nit:

        for (size_t i = 0; i < block.vtx.size(); ++i) {
        const auto& tx = block.vtx.at(i);
        ...// after coinbase check
        const auto& txundo = blockUndo.vtxundo.at(i + 1);
  25. MarcoFalke referenced this in commit 93623eea71 on Mar 20, 2019
  26. MarcoFalke commented at 5:04 PM on March 25, 2019: member

    @FelixWeis Are you still working on this?

  27. jonasschnelli commented at 5:40 AM on April 17, 2019: contributor

    Don't let this go down unmaintained... please pick this up @FelixWeis

  28. FelixWeis force-pushed on Apr 24, 2019
  29. FelixWeis referenced this in commit cc72185867 on Apr 24, 2019
  30. DrahtBot removed the label Needs rebase on Apr 24, 2019
  31. in src/rpc/blockchain.cpp:1883 in cc72185867 outdated 
    1878 | @@ -1879,6 +1879,9 @@ static UniValue getblockstats(const JSONRPCRequest& request)
1879 |      }
1880 |  
1881 |      const CBlock block = GetBlockChecked(pindex);
1882 | +    CBlockUndo blockUndo;
1883 | +    if (!UndoReadFromDisk(blockUndo, pindex))
    MarcoFalke commented at 12:43 PM on April 28, 2019:

    style-nit in commit ab4ed66cab35f5339921721cb85b57a1fd378f12:

    Could you please add {} according to the developer notes and then squash the fixup commit into the first one?

    https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

  32. FelixWeis force-pushed on Apr 29, 2019
  33. FelixWeis force-pushed on Apr 29, 2019
  34. in src/rpc/blockchain.cpp:1885 in a565355d8f outdated 
    1878 | @@ -1879,6 +1879,10 @@ static UniValue getblockstats(const JSONRPCRequest& request)
1879 |      }
1880 |  
1881 |      const CBlock block = GetBlockChecked(pindex);
1882 | +    CBlockUndo blockUndo;
1883 | +    if (!UndoReadFromDisk(blockUndo, pindex)) {
1884 | +        throw JSONRPCError(RPC_INTERNAL_ERROR, "Can't read undo data from disk");
1885 | +    }
    MarcoFalke commented at 12:43 PM on April 29, 2019: 
        }
  35. FelixWeis force-pushed on Apr 29, 2019
  36. in doc/release-notes-14802.md:3 in 293cb0e2ee outdated 
       0 | @@ -0,0 +1,3 @@
   1 | +RPC changes
   2 | +-----------
   3 | +Substantial `getblockstats` speedup for fee calculation by using BlockUndo data. This change also drops `-txindex` requirement and now works for all non-pruned blocks.
    MarcoFalke commented at 5:49 PM on April 29, 2019: 
    The `getblockstats` RPC is faster for fee calculation by using BlockUndo data. Also, `-txindex` is no longer required and `getblockstats` works for all non-pruned blocks.
  37. in src/rpc/blockchain.cpp:1885 in 293cb0e2ee outdated 
    1878 | @@ -1879,6 +1879,10 @@ static UniValue getblockstats(const JSONRPCRequest& request)
1879 |      }
1880 |  
1881 |      const CBlock block = GetBlockChecked(pindex);
1882 | +    CBlockUndo blockUndo;
1883 | +    if (!UndoReadFromDisk(blockUndo, pindex)) {
1884 | +        throw JSONRPCError(RPC_INTERNAL_ERROR, "Can't read undo data from disk");
1885 | +    }
    MarcoFalke commented at 5:50 PM on April 29, 2019:

    Move the code into a helper and call

        const CBlockUndo = GetUndoChecked(pindex);
  38. in src/rpc/blockchain.cpp:1965 in 293cb0e2ee outdated 
    1968 | -                    throw JSONRPCError(RPC_INTERNAL_ERROR, std::string("Unexpected internal error (tx index seems corrupt)"));
1969 | -                }
1970 | -
1971 | -                CTxOut prevoutput = tx_in->vout[in.prevout.n];
1972 | +            const auto& txundo = blockUndo.vtxundo.at(i - 1);
1973 | +            for (size_t txi_n = 0; txi_n < tx->vin.size(); ++txi_n) {
    MarcoFalke commented at 5:58 PM on April 29, 2019: 
                for (const Coin& coin : txundo.vprevout) {
                const CTxOut& prevoutput = coin.out;
  39. in src/rpc/blockchain.h:11 in 293cb0e2ee outdated 
       7 | @@ -8,6 +8,7 @@
   8 |  #include <vector>
   9 |  #include <stdint.h>
  10 |  #include <amount.h>
  11 | +#include <undo.h>
    MarcoFalke commented at 5:58 PM on April 29, 2019:

    Why include this here?

  40. MarcoFalke approved
  41. MarcoFalke commented at 6:02 PM on April 29, 2019: member

    utACK 293cb0e2ee (only some style comments)

    <details><summary>Show signature and timestamp</summary>

    Signature:

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

utACK 293cb0e2ee (only some style comments)
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUg0LQv+J5j+966P/KLR0eT22HQJfZGL8PU8dMwq9WEjwhjrLFcccAwMEd4NPGOJ
GaEUbe5/r8V6lJKkXGJcY4Hs874nWKkmrUYVcF9IFGtqDde/75Xf6/cFGQFdMU+Z
9KjKz3Lj1euVeYXCFJ8NNWKhxVwRMSN2+wbqy3/jCnNSjGUc2ob/vrHOXMBA4Peb
gqfRJyUitjmVWB3vM13H0DP4N1gQc3Zd+ILWptZ3KOk147uJZ/YODbTUlbI0JknM
/N8eGu4zbxUR9tMbaG8en3T0otT0/PYEPXy5eBAmjcGC4eHHZLDsbwLaYUkgRCvp
M/WTkBBg5bk+tMbn+hYj4mGp1GCxnm+6D1JuyZsRUOVvu7Zkv87G41Ss+NBq2ozI
SMsIgVJevTh9AeLpD1nPjS2qxWmmbX9WSJGCN/2dRmCi4tTe0dVWsXY8KtJTbVDl
mqL4zWpxGXW0xjimKNDuPw8ONi9BzKKcCT2tn2MA3y4mXbVBeuqkWuMolUw6l3KW
4j4yc522
=/Qza
-----END PGP SIGNATURE-----

    Timestamp of file with hash 41e1a1ea262d1060a49ddc699ae7b5453144563ddbb328c375ebb745876632e1 -

    </details>

  42. in test/functional/rpc_getblockstats.py:42 in 293cb0e2ee outdated 
      41 | @@ -55,13 +42,14 @@ def generate_test_data(self, filename):
  42 |          mocktime = time.time()
    MarcoFalke commented at 6:10 PM on April 29, 2019: 
            mocktime = 1525107225

    nit: could set mocktime to the previous value, so that similar data is generated?

  43. in test/functional/rpc_getblockstats.py:52 in 293cb0e2ee outdated 
      49 |  
  50 |          self.nodes[0].sendtoaddress(address=self.nodes[0].getnewaddress(), amount=10, subtractfeefromamount=True)
  51 |          self.nodes[0].sendtoaddress(address=self.nodes[0].getnewaddress(), amount=10, subtractfeefromamount=False)
  52 | -        self.nodes[1].sendtoaddress(address=self.nodes[0].getnewaddress(), amount=1, subtractfeefromamount=True)
  53 | +        self.nodes[0].settxfee(amount=0.003)
  54 | +        self.nodes[0].sendtoaddress(address=self.nodes[0].getnewaddress(), amount=1, subtractfeefromamount=True)
    MarcoFalke commented at 6:20 PM on April 29, 2019:

    nit could make data gen (slightly more) deterministic:

            self.nodes[0].sendtoaddress(address=self.nodes[0].get_deterministic_priv_key().address, amount=1, subtractfeefromamount=True)
  44. MarcoFalke commented at 6:21 PM on April 29, 2019: member

    more test nits

  45. DrahtBot added the label Needs rebase on May 3, 2019
  46. MarcoFalke commented at 12:55 PM on May 3, 2019: member

    @FelixWeis You don't actually need to rebase. If you address my style-nits, the conflict will solve itself.

  47. FelixWeis force-pushed on May 9, 2019
  48. DrahtBot removed the label Needs rebase on May 9, 2019
  49. FelixWeis force-pushed on May 9, 2019
  50. FelixWeis force-pushed on May 9, 2019
  51. MarcoFalke commented at 12:18 PM on May 10, 2019: member

    Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

  52. FelixWeis force-pushed on May 10, 2019
  53. rpc: faster getblockstats using BlockUndo data
    Using undo data for a block (rev?????.dat) we can retrieve value information about prevouts and calculate the final transaction fee (rate). This approach is about 80x faster, drops the requirement for -txindex, and works for all non-pruned blocks.
    d20d756752
  54. FelixWeis force-pushed on May 10, 2019
  55. MarcoFalke commented at 5:19 PM on May 10, 2019: member

    re-utACK d20d7567528e216badb8475df298bb3cec008985

    Only changes are:

    • Make tests more deterministic
    • Add and use GetUndoChecked
    • Minor stylistic changes

    <details><summary>Show signature and timestamp</summary>

    Signature:

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

re-utACK d20d7567528e216badb8475df298bb3cec008985

Only changes are:
* Make tests more deterministic
* Add and use GetUndoChecked
* Minor stylistic changes
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUh/4QwAwLZrUZJqLo1ctjMWO2SQ+ut4rxMDYaMNSIGOxw61u8J6OIUPsVfKBJLF
s2Gi0Q7XwCwlttNVDARBCQFH4R8IhVXLaY//BCfVPamVwXCO3s1x7GDtLZZrjs2c
Rng9VOhCSZ7wXZmgoQmYwj77pQOU2k1B6FXkMylzf/pt+W/jaANCuGtC4k4k8x/l
sM2sH75nWs2URra42exUbmDoIfohOHWfAxcA0Ni31AWkeRYRazWGmsvA93qL6PtP
/wN5rMNi10gCI83/N16ZZ+iq7djqRoCjkbkqSNd2D0NJOmquEIN4W3GGQJ1BciGF
nu0gLQ12ShXCVf167E6s4xGPsKu9hsiUNJN9xursiD1RUhGtUQXdpIRjodL+lDj8
CZhHeUj0whAXKkS+YddhPO385rzqChebvdWZL5stRlUi4TFKzJM+rvsjGNaEKgxO
wjiUfi1L6v07zstlKFLc1pfri9Yws5b4MT6fO9+9dQrtUm1IcS4NeS4YrKKaiI4X
B9R6OHC3
=xoBb
-----END PGP SIGNATURE-----

    Timestamp of file with hash 5dc2003a827858f9302ac84ff924bdf15ada47e85278dce1eee93d098c8e8abd -

    </details>

  56. MarcoFalke merged this on May 10, 2019
  57. MarcoFalke closed this on May 10, 2019
  58. MarcoFalke referenced this in commit e2371f842f on May 10, 2019
  59. luke-jr referenced this in commit 8a0df12a3f on Sep 3, 2019
  60. deadalnix referenced this in commit e553714344 on Jun 2, 2020
  61. ftrader referenced this in commit ef21184354 on Dec 1, 2020
  62. MarcoFalke referenced this in commit f656165e9c on Dec 24, 2020
  63. vijaydasmp referenced this in commit 82e17ba598 on Dec 11, 2021
  64. vijaydasmp referenced this in commit 89b4f7f52a on Dec 13, 2021
  65. vijaydasmp referenced this in commit 9b9c149e3a on Dec 14, 2021
  66. vijaydasmp referenced this in commit 350423a45a on Dec 14, 2021
  67. Munkybooty referenced this in commit b6785dac9f on Feb 6, 2022
  68. DrahtBot locked this on Feb 15, 2022
Contributors
FelixWeisgmaxwellMarcoFalkeDrahtBotpromagjonasschnelliajtownslaanwjmarcinja
Labels
RPC/REST/ZMQ

Milestone
0.19.0

Linked (view graph)
#15623 refactor: Expose UndoReadFromDisk in header#16083 [WIP] transaction fees in getblock#18772 rpc: calculate fees in getblock using BlockUndo data
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-01 06:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me