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.

    0# 2018-11-25T16:36:19Z Bitcoin Core version v0.17.99.0-edc715240-dirty (release build)
1seq 550100 550200  0.00s user 0.00s system 62% cpu 0.004 total
2xargs -n1 src/bitcoin-cli getblockstats  0.21s user 0.19s system 17% cpu 2.302 total
3
4
5# 2018-11-25T16:39:17Z Bitcoin Core version v0.17.0 (release build)
6seq 550100 550200  0.00s user 0.00s system 87% cpu 0.002 total
7xargs -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

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

    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 0: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:

    0    for (size_t i = 0; i < block.vtx.size(); ++i) {
1        const auto& tx = block.vtx.at(i);
2        ...// after coinbase check
3        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: 
    0    }
  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: 
    0The `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

    0    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: 
    0            for (const Coin& coin : txundo.vprevout) {
1                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)

    Signature:

     0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3utACK 293cb0e2ee (only some style comments)
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
 7pUg0LQv+J5j+966P/KLR0eT22HQJfZGL8PU8dMwq9WEjwhjrLFcccAwMEd4NPGOJ
 8GaEUbe5/r8V6lJKkXGJcY4Hs874nWKkmrUYVcF9IFGtqDde/75Xf6/cFGQFdMU+Z
 99KjKz3Lj1euVeYXCFJ8NNWKhxVwRMSN2+wbqy3/jCnNSjGUc2ob/vrHOXMBA4Peb
10gqfRJyUitjmVWB3vM13H0DP4N1gQc3Zd+ILWptZ3KOk147uJZ/YODbTUlbI0JknM
11/N8eGu4zbxUR9tMbaG8en3T0otT0/PYEPXy5eBAmjcGC4eHHZLDsbwLaYUkgRCvp
12M/WTkBBg5bk+tMbn+hYj4mGp1GCxnm+6D1JuyZsRUOVvu7Zkv87G41Ss+NBq2ozI
13SMsIgVJevTh9AeLpD1nPjS2qxWmmbX9WSJGCN/2dRmCi4tTe0dVWsXY8KtJTbVDl
14mqL4zWpxGXW0xjimKNDuPw8ONi9BzKKcCT2tn2MA3y4mXbVBeuqkWuMolUw6l3KW
154j4yc522
16=/Qza
17-----END PGP SIGNATURE-----

    Timestamp of file with hash 41e1a1ea262d1060a49ddc699ae7b5453144563ddbb328c375ebb745876632e1 -

  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: 
    0        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:

    0        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

    Signature:

     0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3re-utACK d20d7567528e216badb8475df298bb3cec008985
 4
 5Only changes are:
 6* Make tests more deterministic
 7* Add and use GetUndoChecked
 8* Minor stylistic changes
 9-----BEGIN PGP SIGNATURE-----
10
11iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
12pUh/4QwAwLZrUZJqLo1ctjMWO2SQ+ut4rxMDYaMNSIGOxw61u8J6OIUPsVfKBJLF
13s2Gi0Q7XwCwlttNVDARBCQFH4R8IhVXLaY//BCfVPamVwXCO3s1x7GDtLZZrjs2c
14Rng9VOhCSZ7wXZmgoQmYwj77pQOU2k1B6FXkMylzf/pt+W/jaANCuGtC4k4k8x/l
15sM2sH75nWs2URra42exUbmDoIfohOHWfAxcA0Ni31AWkeRYRazWGmsvA93qL6PtP
16/wN5rMNi10gCI83/N16ZZ+iq7djqRoCjkbkqSNd2D0NJOmquEIN4W3GGQJ1BciGF
17nu0gLQ12ShXCVf167E6s4xGPsKu9hsiUNJN9xursiD1RUhGtUQXdpIRjodL+lDj8
18CZhHeUj0whAXKkS+YddhPO385rzqChebvdWZL5stRlUi4TFKzJM+rvsjGNaEKgxO
19wjiUfi1L6v07zstlKFLc1pfri9Yws5b4MT6fO9+9dQrtUm1IcS4NeS4YrKKaiI4X
20B9R6OHC3
21=xoBb
22-----END PGP SIGNATURE-----

    Timestamp of file with hash 5dc2003a827858f9302ac84ff924bdf15ada47e85278dce1eee93d098c8e8abd -

  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


FelixWeis gmaxwell MarcoFalke DrahtBot promag jonasschnelli ajtowns laanwj marcinja

Labels
RPC/REST/ZMQ

Milestone
0.19.0

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: 2024-11-17 18:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me