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.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
For detailed information about the code coverage, see the test coverage report.
<!--021abf342d371248e50ceaed478a90ca-->
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.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
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.
rebased
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 | + [],
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?
Yeah, I just messed up the rebase.
The nodes are not connected at this point and no blocks have been mined, so it does not seem do anything
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 | -
Same in wallet_assumeutxo.py ?
yepp, done
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 | + [],
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?
Yeah, that works as well, then I might as well test all nodes.
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.
<!--85328a0da195eb286784d51f73fa0af9-->
🚧 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.
<sub>Debug: https://github.com/bitcoin/bitcoin/runs/21991913561</sub>
tACK 1ec6684b08614f593b37e627a0a08e54b19318b6
<details>
<summary>Since this PR is based on top of [#29345](/bitcoin-bitcoin/29345/), I've backed it out from <code>src/rpc/blockchain.cpp</code> in order to hit the mentioned <code>timeout</code>.</summary>
git log --oneline src/rpc/blockchain.cpp
faa30a4c56 (2401-rpc-loadtxoutset-) rpc: Do not wait for headers inside loadtxoutset
fa9108941f rpc: Fix race in loadtxoutset
git checkout fa9108941fa1a0e83484114e2d8a99d264c2ad09 -- src/rpc/blockchain.cpp
self.rpc_timeout on test/functional/feature_assumeutxo.py./test/functional/feature_assumeutxo.py
2024-02-27T16:30:31.156000Z TestFramework (INFO): PRNG seed is: 1857178557572365167
2024-02-27T16:30:31.157000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_7fznd8ax
2024-02-27T16:30:32.583000Z TestFramework (INFO): -- Testing assumeutxo + some indexes + pruning
2024-02-27T16:30:32.584000Z TestFramework (INFO): Creating a UTXO snapshot at height 299
2024-02-27T16:30:32.619000Z TestFramework (INFO): Test loading snapshot when headers are not synced
2024-02-27T16:31:32.679000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/home/pablo/workspace/bitcoin/test/functional/test_framework/authproxy.py", line 151, in _get_response
http_response = self.__conn.getresponse()
File "/usr/lib/python3.10/http/client.py", line 1375, in getresponse
response.begin()
File "/usr/lib/python3.10/http/client.py", line 318, in begin
version, status, reason = self._read_status()
File "/usr/lib/python3.10/http/client.py", line 279, in _read_status
line = str(self.fp.readline(_MAXLINE + 1), "iso-8859-1")
File "/usr/lib/python3.10/socket.py", line 705, in readinto
return self._sock.recv_into(b)
TimeoutError: timed out
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/pablo/workspace/bitcoin/test/functional/test_framework/util.py", line 140, in try_rpc
fun(*args, **kwds)
File "/home/pablo/workspace/bitcoin/test/functional/test_framework/coverage.py", line 50, in __call__
return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
File "/home/pablo/workspace/bitcoin/test/functional/test_framework/authproxy.py", line 127, in __call__
response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
File "/home/pablo/workspace/bitcoin/test/functional/test_framework/authproxy.py", line 106, in _request
return self._get_response()
File "/home/pablo/workspace/bitcoin/test/functional/test_framework/authproxy.py", line 153, in _get_response
raise JSONRPCException({
test_framework.authproxy.JSONRPCException: 'loadtxoutset' RPC took longer than 60.000000 seconds. Consider using larger timeout for calls that take longer to return. (-344)
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/pablo/workspace/bitcoin/test/functional/test_framework/test_framework.py", line 131, in main
self.run_test()
File "/home/pablo/workspace/bitcoin/./test/functional/feature_assumeutxo.py", line 191, in run_test
self.test_headers_not_synced(dump_output['path'])
File "/home/pablo/workspace/bitcoin/./test/functional/feature_assumeutxo.py", line 118, in test_headers_not_synced
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.",
File "/home/pablo/workspace/bitcoin/test/functional/test_framework/util.py", line 131, in assert_raises_rpc_error
assert try_rpc(code, message, fun, *args, **kwds), "No exception raised"
File "/home/pablo/workspace/bitcoin/test/functional/test_framework/util.py", line 144, in try_rpc
raise AssertionError("Unexpected JSONRPC error code %i" % e.error["code"])
AssertionError: Unexpected JSONRPC error code -344
2024-02-27T16:31:32.741000Z TestFramework (INFO): Stopping nodes
2024-02-27T16:31:32.742000Z TestFramework.node1 (ERROR): Unable to stop node.
Traceback (most recent call last):
File "/home/pablo/workspace/bitcoin/test/functional/test_framework/test_node.py", line 377, in stop_node
self.stop(wait=wait)
File "/home/pablo/workspace/bitcoin/test/functional/test_framework/coverage.py", line 50, in __call__
return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
File "/home/pablo/workspace/bitcoin/test/functional/test_framework/authproxy.py", line 127, in __call__
response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
File "/home/pablo/workspace/bitcoin/test/functional/test_framework/authproxy.py", line 105, in _request
self.__conn.request(method, path, postdata, headers)
File "/usr/lib/python3.10/http/client.py", line 1283, in request
self._send_request(method, url, body, headers, encode_chunked)
File "/usr/lib/python3.10/http/client.py", line 1294, in _send_request
self.putrequest(method, url, **skips)
File "/usr/lib/python3.10/http/client.py", line 1120, in putrequest
raise CannotSendRequest(self.__state)
http.client.CannotSendRequest: Request-sent
2024-02-27T16:32:32.889000Z TestFramework.utils (ERROR): wait_until() failed. Predicate: ''''
self.wait_until(lambda: self.is_node_stopped(expected_ret_code=expected_ret_code, **kwargs), timeout=timeout)
'''
Traceback (most recent call last):
File "/home/pablo/workspace/bitcoin/./test/functional/feature_assumeutxo.py", line 370, in <module>
AssumeutxoTest().main()
File "/home/pablo/workspace/bitcoin/test/functional/test_framework/test_framework.py", line 154, in main
exit_code = self.shutdown()
File "/home/pablo/workspace/bitcoin/test/functional/test_framework/test_framework.py", line 313, in shutdown
self.stop_nodes()
File "/home/pablo/workspace/bitcoin/test/functional/test_framework/test_framework.py", line 578, in stop_nodes
node.wait_until_stopped()
File "/home/pablo/workspace/bitcoin/test/functional/test_framework/test_node.py", line 425, in wait_until_stopped
self.wait_until(lambda: self.is_node_stopped(expected_ret_code=expected_ret_code, **kwargs), timeout=timeout)
File "/home/pablo/workspace/bitcoin/test/functional/test_framework/test_node.py", line 802, in wait_until
return wait_until_helper_internal(test_function, timeout=timeout, timeout_factor=self.timeout_factor)
File "/home/pablo/workspace/bitcoin/test/functional/test_framework/util.py", line 276, in wait_until_helper_internal
raise AssertionError("Predicate {} not true after {} seconds".format(predicate_source, timeout))
AssertionError: Predicate ''''
self.wait_until(lambda: self.is_node_stopped(expected_ret_code=expected_ret_code, **kwargs), timeout=timeout)
''' not true after 60 seconds
[node 2] Cleaning up leftover process
[node 1] Cleaning up leftover process
</details>
lgtm
ACK 1ec6684b08614f593b37e627a0a08e54b19318b6
Looks good to me
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)
nit: only fetching the headers rather than the full blocks seems to be sufficient here:
block_header = n0.getblockheader(n0.getblockhash(i), verbose=False)
# make n1 and n2 aware of the new header, but don't give them the
# block.
n1.submitheader(block_header)
n2.submitheader(block_header)
I will look into this as a follow-up, potentially improving getblockheader too since it's confusing that this works
ACK 1ec6684b08614f593b37e627a0a08e54b19318b6
ACK 1ec6684b08614f593b37e627a0a08e54b19318b6