loadtxoutset
#28791
Transaction counts aren’t known for block history loaded from a snapshot. If you start with -checkblockindex after loading a snapshot, the bitcoin daemon will core dump. The test suite does not check for this because all the snapshots have no non-coinbase transactions (all blocks prior to the snapshot are assumed to have nTx = 1).
Recommend for backport to 26.x
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For detailed information about the code coverage, see the test coverage report.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | pablomartin4btc, fjahr, achow101 |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
4858@@ -4859,7 +4859,9 @@ void ChainstateManager::CheckBlockIndex()
4859 // For testing, allow transaction counts to be completely unset.
4860 || (pindex->nChainTx == 0 && pindex->nTx == 0)
4861 // For testing, allow this nChainTx to be unset if previous is also unset.
Yes, the entire assert block is wrong. When a snapshot is loaded, the pindex->nTx value is set to 1. So I’m not really sure what these exceptions would be allowing through.
But since this issue affects the v26 release, I’ve tried to keep the code delta as small and minimally impactful as possible.
Removing the testing assert conditions, as explained by @maflcko in the comment link provided above, even with this fix, bitcoind still crashes.
02023-11-09T15:54:40Z [dnsseed] dnsseed thread exit
1bitcoind: validation.cpp:4880: void ChainstateManager::CheckBlockIndex(): Assertion `(pindex->nChainTx == pindex->nTx + prev_chain_tx) || pindex->IsAssumedValid()' failed.
2Aborted (core dumped)
tACK cdc6ac4126b31426261605a757c52ea2dbfb2a81
0./src/bitcoin-cli -datadir=${AU_DATADIR} getblockchaininfo
1{
2 "chain": "main",
3 "blocks": 800000,
4 "headers": 815917,
5 "bestblockhash": "00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054",
6 "difficulty": 53911173001054.59,
7 "time": 1690168629,
8 "mediantime": 1690165851,
9 "verificationprogress": 0.9438294771289651,
10 "initialblockdownload": true,
11 "chainwork": "00000000000000000000000000000000000000004fc85ab3390629e495bf13d5",
12 "size_on_disk": 31288815,
13 "pruned": true,
14 "pruneheight": 800001,
15 "automatic_pruning": true,
16 "prune_target_size": 20971520000,
17 "warnings": "This is a pre-release test build - use at your own risk - do not use for mining or merchant applications"
18}
0./src/bitcoin-cli -datadir=${AU_DATADIR} getchainstates
1{
2 "headers": 815917,
3 "chainstates": [
4 {
5 "blocks": 71968,
6 "bestblockhash": "0000000000d485b326ff1f88f2cb66fcc24c4345d1b151fa5a4d00476f74a977",
7 "difficulty": 244.2132230923753,
8 "verificationprogress": 0.0001072847948972671,
9 "coins_db_cache_bytes": 419430,
10 "coins_tip_cache_bytes": 784649420,
11 "validated": true
12 },
13 {
14 "blocks": 800000,
15 "bestblockhash": "00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054",
16 "difficulty": 53911173001054.59,
17 "verificationprogress": 0.9438294714681589,
18 "coins_db_cache_bytes": 7969177,
19 "coins_tip_cache_bytes": 14908338995,
20 "snapshot_blockhash": "00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054",
21 "validated": false
22 }
23 ]
24}
02023-11-09T14:44:55Z [msghand] New block-relay-only v1 peer connected: version: 70016, blocks=815999, peer=1
1bitcoind: validation.cpp:4879: void ChainstateManager::CheckBlockIndex(): Assertion `(pindex->nChainTx == pindex->nTx + prev_chain_tx) || (pindex->nChainTx == 0 && pindex->nTx == 0) || (pindex->nChainTx == 0 && prev_chain_tx == 0 && pindex->pprev)' failed.
2Aborted (core dumped)
I’ve verified this PR fixes the problem.
Since we know that loaded snapshots don’t know tx count (*), should we not also avoid this by not letting the user call -checkblockindex under these circumstances and return an error? I’d also add this warning in the documentation if that makes sense.
(*) Is this also happening after background validation is completed?
No, that would be likely to cause problems in various workflows. It is entirely reasonable that -checkblockindex is set in a bitcoin.conf file, just as it is done by default in the RPC tests. In that case restarting the daemon after loading a snapshot but before validation of the historical blockchain is complete will cause it to be in an unstartable state, even though there is nothing wrong with the block index.
checkblockindex is useful for checking the integrity of the database, even before the whole historical blockchain is downloaded.
utACK cdc6ac4126b31426261605a757c52ea2dbfb2a81
This seems to be the best solution for this issue.
