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

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage

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

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

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

    <!--174a7506f384e20aa4161008e828411d-->

    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

    <!--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>

  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

    <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
    
    • Note: a reviewer could extend the timeout changing 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>

  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:

                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)
    

    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

  32. bitcoin locked this on Mar 13, 2025

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-02 03:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me