bitcoin-chainstate utility is not shipped in a release, it is the only current utility directly using the bitcoin kernel library. Adding a simple test for it is useful for checking that the library is actually usable. The test is also useful in future to demonstrate that the bitcoin-chainstate binary using the API for the kernel library introduced in #30595 actually works and offers similar features.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32145.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | laanwj, maflcko, kevkevinpal |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
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.
Needs rebase, as the base is too old?
Thanks, wrote this some time ago and forgot to rebase.
32+ raise AssertionError(f'Expected stderr output {expected_stderr} does not partially match stderr:\n{stderr}')
33+
34+ def run_test(self):
35+ node = self.nodes[0]
36+ datadir = node.cli.datadir
37+ # This might trigger a warning that there is not enough disk space, since this node runs on mainnet. Ignore it.
37+ # This might trigger a warning that there is not enough disk space, since this node runs on mainnet. Ignore it.
38+ node.stop_node(check_stderr=False)
39+
40+ self.log.info(f"Testing bitcoin-chainstate {self.get_binaries().chainstate_argv()} with datadir: {datadir}")
41+ block_one = "010000006fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000982051fd1e4ba744bbbe680e1fee14677ba1a3c3540bf7b1cdb606e857233e0e61bc6649ffff001d01e362990101000000010000000000000000000000000000000000000000000000000000000000000000ffffffff0704ffff001d0104ffffffff0100f2052a0100000043410496b538e853519c726a2c91e61ec11600ae1390813a627c66fb8be7947be63c52da7589379515d4e0a604f8141781e62294721166bf621e73a82cbf2342c858eeac00000000"
42+ self.add_block(datadir, block_one + "\n", "Block has not yet been rejected")
"\n" to add_block?
Adds basic coverage for successfully validating a mainnet block as well
as some duplicate and invalid data.
Thank you for the review @laanwj,
Updated 76449b9cdb367eed05cfdf5fe550fc41e477a7fe -> ca55613fd1596c3e9e5c3cc5c4a7ea4591841fc0 (BitcoinChainstateFunctionalTest_0 -> BitcoinChainstateFunctionalTest_1, compare)
"\n" for arguments passed to add_block.
29+ stdout, stderr = proc.communicate(input=input + "\n", timeout=5)
30+ self.log.debug("STDOUT: {0}".format(stdout.strip("\n")))
31+ self.log.info("STDERR: {0}".format(stderr.strip("\n")))
32+
33+ if expected_stderr not in stderr:
34+ raise AssertionError(f"Expected stderr output {expected_stderr} does not partially match stderr:\n{stderr}")
assert_equal first, but then saw that partial matches use a similar approach here: https://github.com/bitcoin/bitcoin/blob/master/test/functional/test_framework/test_node.py#L523.
ACK ca55613fd1596c3e9e5c3cc5c4a7ea4591841fc0 🎭
Signature:
0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: ACK ca55613fd1596c3e9e5c3cc5c4a7ea4591841fc0 🎭
3QChzeK5vs9igTYzIEqEksUtAvYIchTYkSzNBQRrlsSse8lfj94JWklqOD42JDPoGNvO5iqQVgmiYMx2735hABA==
41+ self.log.info(f"Testing bitcoin-chainstate {self.get_binaries().chainstate_argv()} with datadir: {datadir}")
42+ block_one = "010000006fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000982051fd1e4ba744bbbe680e1fee14677ba1a3c3540bf7b1cdb606e857233e0e61bc6649ffff001d01e362990101000000010000000000000000000000000000000000000000000000000000000000000000ffffffff0704ffff001d0104ffffffff0100f2052a0100000043410496b538e853519c726a2c91e61ec11600ae1390813a627c66fb8be7947be63c52da7589379515d4e0a604f8141781e62294721166bf621e73a82cbf2342c858eeac00000000"
43+ self.add_block(datadir, block_one, "Block has not yet been rejected")
44+ self.add_block(datadir, block_one, "duplicate")
45+ self.add_block(datadir, "00", "Block decode failed")
46+ self.add_block(datadir, "", "Empty line found")
not something that may block this PR but I see other error messages in bitcoin-chainstate.cpp
maybe can be addressed in a followup PR
0grep -nri "std::cerr" ./src/bitcoin-chainstate.cpp
1
2(manually removed the ones that are already in this PR)
3
453: std::cerr
5 54 << "Usage: " << argv[0] << " DATADIR" << std::endl
6 55 << "Display DATADIR information, and process hex-encoded blocks on standard input." << std::endl
7 56 << std::endl
8 57 << "IMPORTANT: THIS EXECUTABLE IS EXPERIMENTAL, FOR TESTING ONLY, AND EXPECTED TO" << std::endl
9 58 << " BREAK IN FUTURE VERSIONS. DO NOT USE ON YOUR ACTUAL DATADIR." << std::endl;
10 59 return 1;
11 60 }
12100: std::cerr << "Error flushing block data to disk: " << message.original << std::endl;
13104: std::cerr << "Error: " << message.original << std::endl;
14134: std::cerr << "Failed to load Chain state from your datadir." << std::endl;
15139: std::cerr << "Failed to verify loaded Chain state from your datadir." << std::endl;
16147: std::cerr << "Failed to connect best block (" << state.ToString() << ")" << std::endl;
17222: std::cerr << "inconclusive" << std::endl;
18231: std::cerr << "the block header may be on a too-little-work chain" << std::endl;
19234: std::cerr << "invalid by consensus rules (excluding any below reasons)" << std::endl;
20237: std::cerr << "this block was cached as being invalid and we didn't store the reason why" << std::endl;
21240: std::cerr << "invalid proof of work or time too old" << std::endl;
22243: std::cerr << "the block's data didn't match the data committed to by the PoW" << std::endl;
23246: std::cerr << "We don't have the previous block the checked one is built on" << std::endl;
24249: std::cerr << "A block this one builds on is invalid" << std::endl;
25252: std::cerr << "block timestamp was > 2 hours in the future (or our clock is bad)" << std::endl;
ACK ca55613fd1596c3e9e5c3cc5c4a7ea4591841fc0
looks good to me I think we can expand on the test, but not blocking this change