test: Add bitcoin-chainstate test for assumeutxo functionality

stringintech commented at 9:44 pm on October 28, 2025: contributor

This PR adds functional test coverage for the bitcoin-chainstate tool loading a datadir initialized with an assumeutxo snapshot.

The PR also includes:

Fix for assertion crash in ActivateExistingSnapshot() when active chainstate has no initialized mempool (required for the test to pass)
-regtest flag support for bitcoin-chainstate to enable the testing

This work started while experimenting with how the current state of the kernel API (#30595) behaves when loading a datadir containing assumeutxo data, and @TheCharlatan suggested adding the same test coverage for the current state of the bitcoin-chainstate tool.

DrahtBot added the label Tests on Oct 28, 2025

DrahtBot commented at 9:44 pm on October 28, 2025: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33728.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	TheCharlatan
Concept ACK	frankomosh, yuvicc

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:

#30595 (kernel: Introduce C header API by TheCharlatan)
#30214 (refactor: Improve assumeutxo state representation 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.

in src/bitcoin-chainstate.cpp:65 in 986eb77560

61@@ -61,6 +62,8 @@ int main(int argc, char* argv[])
62     fs::path abs_datadir{fs::absolute(argv[1])};
63     fs::create_directories(abs_datadir);
64 
65+    const bool use_regtest{(argc == 3 && std::string(argv[2]) == "-regtest")};

frankomosh commented at 6:38 am on October 29, 2025:

If user passes 3 args where argv[2] != "-regtest"(or generally malformed argument combinations), could it silently set use_regtest = false without warning?

If thats the case then I think there should atleast be a warning

stringintech commented at 3:14 pm on October 29, 2025:

Yes I think it is better to have validation on this.

in src/validation.cpp:6324 in 986eb77560 outdated

6320@@ -6321,7 +6321,7 @@ Chainstate& ChainstateManager::ActivateExistingSnapshot(uint256 base_blockhash)
6321     LogInfo("[snapshot] switching active chainstate to %s", m_snapshot_chainstate->ToString());
6322 
6323     // Mempool is empty at this point because we're still in IBD.
6324-    Assert(m_active_chainstate->m_mempool->size() == 0);
6325+    Assert(!m_active_chainstate->m_mempool || m_active_chainstate->m_mempool->size() == 0);

frankomosh commented at 6:41 am on October 29, 2025:

Is this now weaker that the original assertion, because I think it will now silently accepts a null mempool pointer?

TheCharlatan commented at 12:33 pm on October 29, 2025:

It is weaker, yes, but we similarly guard in other situations. We also still have the assertion in ActivateSnapshot to guard this condition at a more critical point in time.

frankomosh commented at 6:49 am on October 29, 2025: contributor

Concept ACK I think its a nice addition as it adds valuable functional coverage for bitcoin-chainstate. Few nits and inquiries inline;

in src/bitcoin-chainstate.cpp:54 in 986eb77560

48@@ -49,10 +49,11 @@ int main(int argc, char* argv[])
49     LogInstance().DisableLogging();
50 
51     // SETUP: Argument parsing and handling
52-    if (argc != 2) {
53+    if (argc < 2 || argc > 3) {
54         std::cerr
55-            << "Usage: " << argv[0] << " DATADIR" << std::endl
56+            << "Usage: " << argv[0] << " DATADIR [-regtest]" << std::endl

TheCharlatan commented at 11:19 am on October 29, 2025:

Since this is an optional positional argument, I think I would drop the - prefix.

stringintech commented at 3:13 pm on October 29, 2025:

I thought if I want to treat it as boolean, it might be better to introduce it as a flag with the dash prefix. Would it make more sense to you if I drop the dash and treat it as a chain name instead (where only regtest and mainnet are supported, and if absent, defaults to mainnet)?

TheCharlatan commented at 9:34 pm on October 29, 2025:

Ah, I see. I think I would change the order then, since typically named optional args precede positional arguments / operands in command line applications: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html#tag_12_01 .

TheCharlatan commented at 12:34 pm on October 29, 2025: contributor

Concept ACK

Consolidating the tests into the regtest case seems fine to me, especially since this is just a utility to showcase and doogfood some of the kernel work.

Add regtest support to bitcoin-chainstate tool

Adds -regtest flag to enable testing with regtest chain parameters.

a21f2d6dd2

Fix `ActivateExistingSnapshot()` assertion crash

Check mempool exists before accessing size when active chainstate doesn't have initialized mempool.

8ad120ae59

test: Add bitcoin-chainstate test for assumeutxo functionality

Adds functional test coverage for bitcoin-chainstate tool loading a datadir initialized with an assumeutxo snapshot

3e7d272b27

stringintech force-pushed on Oct 30, 2025

stringintech commented at 11:12 am on October 30, 2025: contributor

986eb77 to 3e7d272 - reversed the order of -regtest flag and input DATADIR as suggested by @TheCharlatan and added simple validation for the flag as suggested by @frankomosh.

TheCharlatan approved

TheCharlatan commented at 2:01 pm on October 30, 2025: contributor

ACK 3e7d272b2747f1937010b8fc1fefafcfa0ce3213

DrahtBot requested review from frankomosh on Oct 30, 2025

yuvicc commented at 2:37 pm on October 30, 2025: contributor

Concept ACK

test: Add bitcoin-chainstate test for assumeutxo functionality #33728

Code Coverage & Benchmarks

Reviews

Conflicts