test: Test new header sync behavior in loadtxoutset #29478

pull fjahr wants to merge 2 commits into bitcoin:master from fjahr:pr29345 changing 2 files +24 −17
  1. fjahr commented at 11:29 pm on February 25, 2024: contributor

    It adds a test for the change to loadtxoutset made in #29345. Before that change the test doesn’t fail right away but times out after 10 minutes.

    Also removes a sync_blocks call that didn’t seem to do anything valuable.

  2. DrahtBot commented at 11:29 pm on February 25, 2024: contributor

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

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK pablomartin4btc, BrandonOdiwuor, theStack, achow101

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #29612 (rpc: Optimize serialization disk space of dumptxoutset - Reloaded by fjahr)
    • #29553 (assumeutxo: Add dumptxoutset height param, remove shell scripts by fjahr)
    • #29370 (assumeutxo: Get rid of faked nTx and nChainTx values by ryanofsky)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  3. DrahtBot added the label Tests on Feb 25, 2024
  4. fjahr force-pushed on Feb 25, 2024
  5. fjahr force-pushed on Feb 26, 2024
  6. fjahr commented at 12:49 pm on February 26, 2024: contributor
    rebased
  7. in test/functional/feature_assumeutxo.py:64 in 7471435547 outdated 
    61         self.extra_args = [
62             [],
63             ["-fastprune", "-prune=1", "-blockfilterindex=1", "-coinstatsindex=1"],
64             ["-persistmempool=0","-txindex=1", "-blockfilterindex=1", "-coinstatsindex=1"],
65+            ["-txindex=1", "-blockfilterindex=1", "-coinstatsindex=1"],
66+            [],
    maflcko commented at 1:06 pm on February 26, 2024:
    Why add two nodes to test this? Why not just delay the headers, check for the error message, and then submit the headers to the existing node?
    fjahr commented at 4:05 pm on February 26, 2024:
    Yeah, I just messed up the rebase.
  8. test: Remove unnecessary sync_blocks in assumeutxo tests 
    The nodes are not connected at this point and no blocks have been mined, so it does not seem do anything
    2bc1ecfaa9
  9. in test/functional/feature_assumeutxo.py:170 in 9d7e0043fa outdated 
    165@@ -166,8 +166,6 @@ def run_test(self):
166         for n in self.nodes:
167             n.setmocktime(n.getblockheader(n.getbestblockhash())['time'])
168 
169-        self.sync_blocks()
170-
    maflcko commented at 1:07 pm on February 26, 2024:
    Same in wallet_assumeutxo.py ?
    fjahr commented at 4:05 pm on February 26, 2024:
    yepp, done
  10. fjahr force-pushed on Feb 26, 2024
  11. in test/functional/feature_assumeutxo.py:64 in 08c972d629 outdated 
    60         self.rpc_timeout = 120
61         self.extra_args = [
62             [],
63             ["-fastprune", "-prune=1", "-blockfilterindex=1", "-coinstatsindex=1"],
64             ["-persistmempool=0","-txindex=1", "-blockfilterindex=1", "-coinstatsindex=1"],
65+            [],
    maflcko commented at 4:17 pm on February 26, 2024:
    Why add a node to test this? Why not just delay the headers, check for the error message, and then submit the headers to the existing node?
    fjahr commented at 4:26 pm on February 26, 2024:
    Yeah, that works as well, then I might as well test all nodes.
    fjahr commented at 5:19 pm on February 26, 2024:
    Actually no, now I remember why I did it this way. The dump is created later and so to there is a bit of reorganization needed in the test, so the change is more complex with this now. But I also found that the docs needed further fixing as well anyway.
  12. fjahr force-pushed on Feb 26, 2024
  13. fjahr force-pushed on Feb 26, 2024
  14. DrahtBot added the label CI failed on Feb 26, 2024
  15. DrahtBot commented at 5:18 pm on February 26, 2024: contributor

    🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

    Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    Leave a comment here, if you need help tracking down a confusing failure.

    Debug: https://github.com/bitcoin/bitcoin/runs/21991913561

  16. fjahr force-pushed on Feb 26, 2024
  17. test: Add test for loadtxoutset when headers are not synced 1ec6684b08
  18. fjahr force-pushed on Feb 26, 2024
  19. DrahtBot removed the label CI failed on Feb 26, 2024
  20. pablomartin4btc approved
  21. pablomartin4btc commented at 5:19 pm on February 27, 2024: member

    tACK 1ec6684b08614f593b37e627a0a08e54b19318b6

    0git log --oneline src/rpc/blockchain.cpp
1faa30a4c56 (2401-rpc-loadtxoutset-) rpc: Do not wait for headers inside loadtxoutset
2fa9108941f rpc: Fix race in loadtxoutset
3
4git checkout fa9108941fa1a0e83484114e2d8a99d264c2ad09 -- src/rpc/blockchain.cpp
    • Note: a reviewer could extend the timeout changing self.rpc_timeout on test/functional/feature_assumeutxo.py
     0./test/functional/feature_assumeutxo.py 
 12024-02-27T16:30:31.156000Z TestFramework (INFO): PRNG seed is: 1857178557572365167
 22024-02-27T16:30:31.157000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_7fznd8ax
 32024-02-27T16:30:32.583000Z TestFramework (INFO): -- Testing assumeutxo + some indexes + pruning
 42024-02-27T16:30:32.584000Z TestFramework (INFO): Creating a UTXO snapshot at height 299
 52024-02-27T16:30:32.619000Z TestFramework (INFO): Test loading snapshot when headers are not synced
 62024-02-27T16:31:32.679000Z TestFramework (ERROR): Assertion failed
 7Traceback (most recent call last):
 8  File "/home/pablo/workspace/bitcoin/test/functional/test_framework/authproxy.py", line 151, in _get_response
 9    http_response = self.__conn.getresponse()
10  File "/usr/lib/python3.10/http/client.py", line 1375, in getresponse
11    response.begin()
12  File "/usr/lib/python3.10/http/client.py", line 318, in begin
13    version, status, reason = self._read_status()
14  File "/usr/lib/python3.10/http/client.py", line 279, in _read_status
15    line = str(self.fp.readline(_MAXLINE + 1), "iso-8859-1")
16  File "/usr/lib/python3.10/socket.py", line 705, in readinto
17    return self._sock.recv_into(b)
18TimeoutError: timed out
19
20During handling of the above exception, another exception occurred:
21
22Traceback (most recent call last):
23  File "/home/pablo/workspace/bitcoin/test/functional/test_framework/util.py", line 140, in try_rpc
24    fun(*args, **kwds)
25  File "/home/pablo/workspace/bitcoin/test/functional/test_framework/coverage.py", line 50, in __call__
26    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
27  File "/home/pablo/workspace/bitcoin/test/functional/test_framework/authproxy.py", line 127, in __call__
28    response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
29  File "/home/pablo/workspace/bitcoin/test/functional/test_framework/authproxy.py", line 106, in _request
30    return self._get_response()
31  File "/home/pablo/workspace/bitcoin/test/functional/test_framework/authproxy.py", line 153, in _get_response
32    raise JSONRPCException({
33test_framework.authproxy.JSONRPCException: 'loadtxoutset' RPC took longer than 60.000000 seconds. Consider using larger timeout for calls that take longer to return. (-344)
34
35During handling of the above exception, another exception occurred:
36
37Traceback (most recent call last):
38  File "/home/pablo/workspace/bitcoin/test/functional/test_framework/test_framework.py", line 131, in main
39    self.run_test()
40  File "/home/pablo/workspace/bitcoin/./test/functional/feature_assumeutxo.py", line 191, in run_test
41    self.test_headers_not_synced(dump_output['path'])
42  File "/home/pablo/workspace/bitcoin/./test/functional/feature_assumeutxo.py", line 118, in test_headers_not_synced
43    assert_raises_rpc_error(-32603, "The base block header (3bb7ce5eba0be48939b7a521ac1ba9316afee2c7bada3a0cca24188e6d7d96c0) must appear in the headers chain. Make sure all headers are syncing, and call this RPC again.",
44  File "/home/pablo/workspace/bitcoin/test/functional/test_framework/util.py", line 131, in assert_raises_rpc_error
45    assert try_rpc(code, message, fun, *args, **kwds), "No exception raised"
46  File "/home/pablo/workspace/bitcoin/test/functional/test_framework/util.py", line 144, in try_rpc
47    raise AssertionError("Unexpected JSONRPC error code %i" % e.error["code"])
48AssertionError: Unexpected JSONRPC error code -344
492024-02-27T16:31:32.741000Z TestFramework (INFO): Stopping nodes
502024-02-27T16:31:32.742000Z TestFramework.node1 (ERROR): Unable to stop node.
51Traceback (most recent call last):
52  File "/home/pablo/workspace/bitcoin/test/functional/test_framework/test_node.py", line 377, in stop_node
53    self.stop(wait=wait)
54  File "/home/pablo/workspace/bitcoin/test/functional/test_framework/coverage.py", line 50, in __call__
55    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
56  File "/home/pablo/workspace/bitcoin/test/functional/test_framework/authproxy.py", line 127, in __call__
57    response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
58  File "/home/pablo/workspace/bitcoin/test/functional/test_framework/authproxy.py", line 105, in _request
59    self.__conn.request(method, path, postdata, headers)
60  File "/usr/lib/python3.10/http/client.py", line 1283, in request
61    self._send_request(method, url, body, headers, encode_chunked)
62  File "/usr/lib/python3.10/http/client.py", line 1294, in _send_request
63    self.putrequest(method, url, **skips)
64  File "/usr/lib/python3.10/http/client.py", line 1120, in putrequest
65    raise CannotSendRequest(self.__state)
66http.client.CannotSendRequest: Request-sent
672024-02-27T16:32:32.889000Z TestFramework.utils (ERROR): wait_until() failed. Predicate: ''''
68        self.wait_until(lambda: self.is_node_stopped(expected_ret_code=expected_ret_code, **kwargs), timeout=timeout)
69'''
70Traceback (most recent call last):
71  File "/home/pablo/workspace/bitcoin/./test/functional/feature_assumeutxo.py", line 370, in <module>
72    AssumeutxoTest().main()
73  File "/home/pablo/workspace/bitcoin/test/functional/test_framework/test_framework.py", line 154, in main
74    exit_code = self.shutdown()
75  File "/home/pablo/workspace/bitcoin/test/functional/test_framework/test_framework.py", line 313, in shutdown
76    self.stop_nodes()
77  File "/home/pablo/workspace/bitcoin/test/functional/test_framework/test_framework.py", line 578, in stop_nodes
78    node.wait_until_stopped()
79  File "/home/pablo/workspace/bitcoin/test/functional/test_framework/test_node.py", line 425, in wait_until_stopped
80    self.wait_until(lambda: self.is_node_stopped(expected_ret_code=expected_ret_code, **kwargs), timeout=timeout)
81  File "/home/pablo/workspace/bitcoin/test/functional/test_framework/test_node.py", line 802, in wait_until
82    return wait_until_helper_internal(test_function, timeout=timeout, timeout_factor=self.timeout_factor)
83  File "/home/pablo/workspace/bitcoin/test/functional/test_framework/util.py", line 276, in wait_until_helper_internal
84    raise AssertionError("Predicate {} not true after {} seconds".format(predicate_source, timeout))
85AssertionError: Predicate ''''
86        self.wait_until(lambda: self.is_node_stopped(expected_ret_code=expected_ret_code, **kwargs), timeout=timeout)
87''' not true after 60 seconds
88[node 2] Cleaning up leftover process
89[node 1] Cleaning up leftover process
  22. maflcko commented at 5:41 pm on February 27, 2024: member
    lgtm
  23. fjahr renamed this:
    test: Test new header sync behavior in loadtxoutsetinfo
    test: Test new header sync behavior in loadtxoutset
    on Mar 10, 2024
  24. BrandonOdiwuor approved
  25. BrandonOdiwuor commented at 12:53 pm on March 12, 2024: contributor

    ACK 1ec6684b08614f593b37e627a0a08e54b19318b6

    Looks good to me

  26. in test/functional/feature_assumeutxo.py:201 in 1ec6684b08 
    196+        for i in range(1, 300):
197+            block = n0.getblock(n0.getblockhash(i), 0)
198+            # make n1 and n2 aware of the new header, but don't give them the
199+            # block.
200+            n1.submitheader(block)
201+            n2.submitheader(block)
    theStack commented at 11:05 am on March 13, 2024:

    nit: only fetching the headers rather than the full blocks seems to be sufficient here:

    0            block_header = n0.getblockheader(n0.getblockhash(i), verbose=False)
1            # make n1 and n2 aware of the new header, but don't give them the
2            # block.
3            n1.submitheader(block_header)
4            n2.submitheader(block_header)
    fjahr commented at 12:48 pm on March 13, 2024:
    I think that is already the case. Afaict getblock with verbosity 0 returns the same result as getblockheader with false. Inspired me to suggest this: #29646.
    fjahr commented at 1:33 pm on March 13, 2024:
    I will look into this as a follow-up, potentially improving getblockheader too since it’s confusing that this works
  27. theStack approved
  28. theStack commented at 11:05 am on March 13, 2024: contributor
    ACK 1ec6684b08614f593b37e627a0a08e54b19318b6
  29. achow101 commented at 3:33 pm on March 13, 2024: member
    ACK 1ec6684b08614f593b37e627a0a08e54b19318b6
  30. achow101 merged this on Mar 13, 2024
  31. achow101 closed this on Mar 13, 2024


fjahr DrahtBot maflcko pablomartin4btc BrandonOdiwuor theStack achow101

Labels
Tests

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-07-03 10:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me