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

stringintech · 2025-10-28T21:44:26Z

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 ChainstateManager::AddChainstate() when prev_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 the bitcoin-chainstate tool and how the kernel API (#30595) behaved when loading a datadir containing assumeutxo data, during the time that PR was still under review. @sedited suggested opening a PR to add this test coverage.

DrahtBot · 2025-10-28T21:44:31Z

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	theStack, sedited
Concept ACK	yuvicc
Stale ACK	frankomosh

If your review is incorrectly listed, please copy-paste  into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#33847 (kernel: Improve logging API 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.

frankomosh

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

src/bitcoin-chainstate.cpp

frankomosh · 2025-10-29T06:41:03Z

src/validation.cpp

@@ -6321,7 +6321,7 @@ Chainstate& ChainstateManager::ActivateExistingSnapshot(uint256 base_blockhash)
    LogInfo("[snapshot] switching active chainstate to %s", m_snapshot_chainstate->ToString());

    // Mempool is empty at this point because we're still in IBD.
-    Assert(m_active_chainstate->m_mempool->size() == 0);
+    Assert(!m_active_chainstate->m_mempool || m_active_chainstate->m_mempool->size() == 0);


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

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.

sedited

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.

src/bitcoin-chainstate.cpp

sedited · 2025-10-29T12:33:22Z

src/validation.cpp

@@ -6321,7 +6321,7 @@ Chainstate& ChainstateManager::ActivateExistingSnapshot(uint256 base_blockhash)
    LogInfo("[snapshot] switching active chainstate to %s", m_snapshot_chainstate->ToString());

    // Mempool is empty at this point because we're still in IBD.
-    Assert(m_active_chainstate->m_mempool->size() == 0);
+    Assert(!m_active_chainstate->m_mempool || m_active_chainstate->m_mempool->size() == 0);


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.

stringintech · 2025-10-30T11:12:22Z

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.

sedited

ACK 3e7d272b2747f1937010b8fc1fefafcfa0ce3213

yuvicc · 2025-10-30T14:37:23Z

Concept ACK

frankomosh · 2025-11-04T06:40:41Z

ACK 3e7d272

stringintech · 2025-11-04T16:28:34Z

3e7d272 to 67740df - rebased and resolved conflict with #30595.

sedited

re-ACK 67740dfe6cfe3f3f91aaad41018a7da9004dcd8c

frankomosh · 2025-12-09T06:13:36Z

re-ACK 67740df

stringintech · 2025-12-09T10:38:51Z

67740df to 6cb63db - Rebased and removed unused variable block_tx in tool_bitcoin_chainstate.py.
Also updated PR description.

sedited

Re-ACK 6cb63db

sedited · 2025-12-10T14:37:05Z

src/bitcoin-chainstate.cpp

    // SETUP: Argument parsing and handling
-    if (argc != 2) {
+    const bool has_regtest_flag{argc == 3 && std::string(argv[1]) == "-regtest"};
+    if (argc < 2 || argc > 3 || (argc == 3 && !has_regtest_flag)) {


I've been thinking lately whether we can reuse some of our own common utilities for these small, kernel-based applications. These small, single use applications should be able to use our usual utilities, while still benefiting from a clearer kernel interface. I think we are at a point now, where we can confidently mix between the two without risking introducing an entanglement regression between what the kernel requires and what new code might introduce in it. For this reason, I added some basic argument handling on my branch over here: https://github.com/sedited/bitcoin/tree/bitcoin_chainstate_args , that might be interesting for this change here.

Makes sense, thanks! It seems I can cherry-pick your commit as is to replace bfceb4c?

Makes sense, thanks! It seems I can cherry-pick your commit as is to replace bfceb4c?

Sure, if you think it is a good idea too :)

For reviewers:

As discussed offline, adding ArgsManager and the G_TRANSLATION_FUN definition that should be added because of it, is causing different compile/link issues since the symbol is also defined in the kernel library and @sedited is suggesting to have the symbol removed from there.

So it seems adapting bitcoin-chainstate.cpp to use the ArgsManager could be done in a follow-up.

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

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

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

stringintech · 2025-12-18T17:14:17Z

6cb63db to 7b5d256 - Rebased and resolved conflict with #30214.

theStack

Concept and code-review ACK 7b5d256

As a follow-up idea, it might make sense to deduplicate the deterministic generation of the snapshot chain between this test and feature_assumeutxo.py, by moving the logic into a test framework module.

sedited

Re-ACK 7b5d256

DrahtBot added the Tests label Oct 28, 2025

frankomosh reviewed Oct 29, 2025

View reviewed changes

sedited reviewed Oct 29, 2025

View reviewed changes

DrahtBot mentioned this pull request Oct 29, 2025

refactor: Improve assumeutxo state representation #30214

Merged

stringintech force-pushed the 2025/10/test-bitcoin-chainstate-assumeutxo branch from 986eb77 to 3e7d272 Compare October 30, 2025 11:06

sedited approved these changes Oct 30, 2025

View reviewed changes

DrahtBot requested a review from frankomosh October 30, 2025 14:01

DrahtBot mentioned this pull request Oct 30, 2025

kernel: Introduce C header API #30595

Merged

stringintech force-pushed the 2025/10/test-bitcoin-chainstate-assumeutxo branch from 3e7d272 to 67740df Compare November 4, 2025 16:18

sedited approved these changes Nov 5, 2025

View reviewed changes

This was referenced Nov 11, 2025

kernel: Improve logging API #33847

Draft

kernel, logging: Pass Logger instances to kernel objects #30342

Draft

fanquake requested review from stickies-v and removed request for frankomosh November 20, 2025 17:42

sedited added this to The libbitcoinkernel Project Nov 24, 2025

sedited moved this to Ready for Review PRs in The libbitcoinkernel Project Nov 24, 2025

stringintech force-pushed the 2025/10/test-bitcoin-chainstate-assumeutxo branch from 67740df to 6cb63db Compare December 9, 2025 10:33

sedited approved these changes Dec 9, 2025

View reviewed changes

DrahtBot requested a review from frankomosh December 9, 2025 12:14

sedited reviewed Dec 10, 2025

View reviewed changes

sedited moved this from API Development to Client Flexibility in The libbitcoinkernel Project Dec 14, 2025

DrahtBot added the Needs rebase label Dec 16, 2025

kevkevinpal mentioned this pull request Dec 16, 2025

refactor: Improve assumeutxo state representation kevkevinpal/bitcoin#231

Closed

stringintech added 3 commits December 18, 2025 20:19

Add regtest support to bitcoin-chainstate tool

5f3d6bd

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

Fix ChainstateManager::AddChainstate() assertion crash

2bc3265

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

test: Add bitcoin-chainstate test for assumeutxo functionality

7b5d256

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

stringintech force-pushed the 2025/10/test-bitcoin-chainstate-assumeutxo branch from 6cb63db to 7b5d256 Compare December 18, 2025 17:12

DrahtBot removed the Needs rebase label Dec 18, 2025

theStack approved these changes Jan 5, 2026

View reviewed changes

DrahtBot requested a review from sedited January 5, 2026 08:27

sedited approved these changes Jan 5, 2026

View reviewed changes

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

Are you sure you want to change the base?

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

Conversation

stringintech commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

frankomosh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

frankomosh Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

sedited Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

sedited left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sedited Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

stringintech commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sedited left a comment

Choose a reason for hiding this comment

Uh oh!

yuvicc commented Oct 30, 2025

Uh oh!

frankomosh commented Nov 4, 2025

Uh oh!

stringintech commented Nov 4, 2025

Uh oh!

sedited left a comment

Choose a reason for hiding this comment

Uh oh!

frankomosh commented Dec 9, 2025

Uh oh!

stringintech commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sedited left a comment

Choose a reason for hiding this comment

Uh oh!

sedited Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

stringintech Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

sedited Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

stringintech Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stringintech commented Dec 18, 2025

Uh oh!

theStack left a comment

Choose a reason for hiding this comment

Uh oh!

sedited left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

stringintech commented Oct 28, 2025 •

edited

Loading

DrahtBot commented Oct 28, 2025 •

edited

Loading

stringintech commented Oct 30, 2025 •

edited

Loading

stringintech commented Dec 9, 2025 •

edited

Loading

stringintech Dec 18, 2025 •

edited

Loading