test: assumeutxo: check that UTXO-querying RPCs operate on snapshot chainstate #30636

pull theStack wants to merge 1 commits into bitcoin:master from theStack:202408-test-assumeutxo-check_utxo_querying_rpcs changing 1 files +25 −0

theStack commented at 11:52 AM on August 12, 2024: contributor

Inspired by some manual testing I did for #28553, this PR checks that RPCs which explicitly query the UTXO set database (i.e. gettxoutsetinfo, scantxoutset and gettxout) operate on the snapshot chainstate as expected.
DrahtBot commented at 11:52 AM on August 12, 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 fjahr, tdb3, achow101

Concept ACK BrandonOdiwuor

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
DrahtBot added the label Tests on Aug 12, 2024

Type	Reviewers
ACK	fjahr, tdb3, achow101
Concept ACK	BrandonOdiwuor

fanquake commented at 12:56 PM on August 12, 2024: member

https://github.com/bitcoin/bitcoin/actions/runs/10351325972/job/28649600800?pr=30636#step:7:23340:

 test  2024-08-12T12:48:42.142000Z TestFramework (ERROR): JSONRPC error 
                                   Traceback (most recent call last):
                                     File "/home/runner/work/_temp/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 132, in main
                                       self.run_test()
                                     File "/home/runner/work/_temp/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/feature_assumeutxo.py", line 354, in run_test
                                       utxo_info = n1.gettxoutsetinfo()
                                                   ^^^^^^^^^^^^^^^^^^^^
                                     File "/home/runner/work/_temp/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/coverage.py", line 50, in __call__
                                       return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
                                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                                     File "/home/runner/work/_temp/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/authproxy.py", line 146, in __call__
                                       raise JSONRPCException(response['error'], status)
                                   test_framework.authproxy.JSONRPCException: Unable to get data because coinstatsindex is still syncing. Current height: 0 (-32603)
 node1 2024-08-12T12:48:42.142251Z (mocktime: 2011-02-02T23:17:17Z) [scheduler] [validationinterface.cpp:246] [operator()] [validation] ChainStateFlushed: block hash=3bb7ce5eba0be48939b7a521ac1ba9316afee2c7bada3a0cca24188e6d7d96c0 
 test  2024-08-12T12:48:42.149000Z TestFramework (DEBUG): Closing down network thread

fjahr commented at 1:27 PM on August 12, 2024: contributor

Concept ACK
DrahtBot added the label CI failed on Aug 12, 2024
theStack force-pushed on Aug 12, 2024
in test/functional/feature_assumeutxo.py:362 in 2e9072c137 outdated
```
 357 | +        assert_equal(utxo_info['height'], loaded['base_height'])
 358 | +        assert_equal(utxo_info['bestblock'], loaded['tip_hash'])
 359 | +
 360 | +        # find coinbase output at snapshot height on node0 and scan for it on node1,
 361 | +        # where the block is not available, but the snapshot was loaded successfully
 362 | +        snapshot_hash = loaded['tip_hash']
```
fjahr commented at 2:46 PM on August 12, 2024:

nit: could have moved this up and used it in the gettxoutsetinfo as well

fjahr commented at 2:48 PM on August 12, 2024:

On second thought: maybe just pull all three of these loaded keys out in the beginning since you use each of them at least twice and they are used again further below... or you could deduplicate the assert_equal triplet but it's not a big deal either way...

theStack commented at 10:49 AM on August 20, 2024:

Introduced snapshot_hash and snapshot_num_coins in the beginning and used the already existing SNAPSHOT_BASE_HEIGHT for the height.

in test/functional/feature_assumeutxo.py:370 in 2e9072c137 outdated

 365 | +        coinbase_output_descriptor = coinbase_tx['vout'][0]['scriptPubKey']['desc']
 366 | +        scan_result = n1.scantxoutset('start', [coinbase_output_descriptor])
 367 | +        assert_equal(scan_result['success'], True)
 368 | +        assert_equal(scan_result['txouts'], loaded['coins_loaded'])
 369 | +        assert_equal(scan_result['height'], loaded['base_height'])
 370 | +        assert_equal(scan_result['bestblock'], loaded['tip_hash'])

fjahr commented at 2:47 PM on August 12, 2024:

nit

        assert_equal(scan_result['bestblock'], snapshot_hash)

in test/functional/feature_assumeutxo.py:371 in 2e9072c137 outdated

 366 | +        scan_result = n1.scantxoutset('start', [coinbase_output_descriptor])
 367 | +        assert_equal(scan_result['success'], True)
 368 | +        assert_equal(scan_result['txouts'], loaded['coins_loaded'])
 369 | +        assert_equal(scan_result['height'], loaded['base_height'])
 370 | +        assert_equal(scan_result['bestblock'], loaded['tip_hash'])
 371 | +        assert coinbase_tx['txid'] in [coin['txid'] for coin in scan_result['unspents']]

fjahr commented at 2:49 PM on August 12, 2024:

nit: Would be a bit more readable IMO if you would break it into two lines

fjahr commented at 2:57 PM on August 12, 2024: contributor

Code review ACK 2e9072c137e81c75c58d0c0788295c10fdafdc9b

This is a good regression test to have. I left a bunch of minor nits but this is fine to merge as-is so feel free to leave them unless you have to re-touch.
DrahtBot removed the label CI failed on Aug 12, 2024
tdb3 commented at 1:15 AM on August 13, 2024: contributor

Concept ACK

Great test additions. Light code review and manual test. I concur with the nits from @fjahr.
BrandonOdiwuor commented at 12:50 PM on August 14, 2024: contributor

Concept ACK
test: assumeutxo: check that UTXO-querying RPCs operate on snapshot chainstate 917e70a620
theStack force-pushed on Aug 20, 2024
theStack commented at 10:49 AM on August 20, 2024: contributor

Thanks for the reviews! Took the suggestions and refined the UTXO check against the scantxoutset result to also include the vout index (=0), rather than only verifying the txid.
fjahr commented at 11:09 AM on August 20, 2024: contributor

utACK 917e70a6206c62c4c492fa922425fc8e00d3f328

Only changes since last review were addressing my minor review comments.
DrahtBot requested review from tdb3 on Aug 20, 2024
DrahtBot requested review from BrandonOdiwuor on Aug 20, 2024
tdb3 approved
tdb3 commented at 11:27 AM on August 20, 2024: contributor

ACK 917e70a6206c62c4c492fa922425fc8e00d3f328

These tests are great additions. Changes incorporate comments nicely. Ran functionals locally (passed).
maflcko added this to the milestone 28.0 on Aug 20, 2024
achow101 commented at 5:20 PM on August 21, 2024: member

ACK 917e70a6206c62c4c492fa922425fc8e00d3f328
achow101 merged this on Aug 21, 2024
achow101 closed this on Aug 21, 2024
theStack deleted the branch on Aug 21, 2024
bitcoin locked this on Oct 3, 2025