Skip to content

Commit feae4e0

Browse files
committed
Merge bitcoin#28698: assumeutxo, blockstorage: Prevent core dump on invalid hash
811067c test: add coverage for snapshot chainstate not matching AssumeUTXO parameters (pablomartin4btc) 4a5be10 assumeutxo, blockstorage: prevent core dump on invalid hash (pablomartin4btc) Pull request description: While reviewing bitcoin#27596 (ran `loadtxoutset` in `mainnet` before `m_assumeutxo_data` is empty as [currently](https://github.com/jamesob/bitcoin/blob/434495a8c1496ca23fe35b84499f3daf668d76b8/src/kernel/chainparams.cpp#L175-L177) in master - back to 1b1d711), got a `core dumped`, so it seems there's a potential issue if new releases ever remove snapshot details or a semi-experienced user performs a `loadtxoutset` on a different "customised" binary version (not sure if this is a real use case). ``` 2023-10-18T17:42:52Z [init] Using obfuscation key for /tmp/.test_utxo_2/blocks/index: 0000000000000000 node/blockstorage.cpp:390 LoadBlockIndex: Assertion `GetParams().AssumeutxoForBlockhash(*snapshot_blockhash)' failed. Aborted (core dumped) ``` <details> <summary>This is also happening before IBD is completed (<code>background validation</code> still being performed as it can be seen in rpc <code>getchainstates</code>)</summary> ``` /src/bitcoin-cli -datadir=${AU_DATADIR} getchainstates { "headers": 813097, "chainstates": [ { "blocks": 368249, "bestblockhash": "00000000000000000b7a08224a1cb00d337100ba7a46c03d04b2c2d8964efc37", "difficulty": 52278304845.59168, "verificationprogress": 0.086288278873286, "coins_db_cache_bytes": 7969177, "coins_tip_cache_bytes": 14908338995, "validated": true }, { "blocks": 813097, "bestblockhash": "0000000000000000000270c9fdce7b17db64cca91f90106964b58e33a4d91089", "difficulty": 61030681983175.59, "verificationprogress": 0.999997140098457, "coins_db_cache_bytes": 419430, "coins_tip_cache_bytes": 784649420, "snapshot_blockhash": "00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054", "validated": false } ] } ``` </details> <details> <summary>Steps to reproduce the core dump error and its output:</summary> 1. Perform a `loadtxoutset` in `mainnet` on compiled `bitcoind` adding the block hash from Sjors's [commit](Sjors@24deb20). 2. Once step 1 finishes, remove the added code from step 1 and compile again or just compile `master` without any changes on top. 3. Run `bitcoind`, soon it'll crash with: ``` 2023-10-18T17:42:52Z [init] init message: Loading block index… 2023-10-18T17:42:52Z [init] Assuming ancestors of block 00000000000000000001a0a448d6cf2546b06801389cc030b2b18c6491266815 have valid signatures. 2023-10-18T17:42:52Z [init] Setting nMinimumChainWork=000000000000000000000000000000000000000052b2559353df4117b7348b64 2023-10-18T17:42:52Z [init] Prune configured to target 3000 MiB on disk for block and undo files. 2023-10-18T17:42:52Z [init] [snapshot] detected active snapshot chainstate (/tmp/.test_utxo_2/chainstate_snapshot) - loading 2023-10-18T17:42:52Z [init] [snapshot] switching active chainstate to Chainstate [snapshot] @ height -1 (null) 2023-10-18T17:42:52Z [init] Opening LevelDB in /tmp/.test_utxo_2/blocks/index 2023-10-18T17:42:52Z [init] Opened LevelDB successfully 2023-10-18T17:42:52Z [init] Using obfuscation key for /tmp/.test_utxo_2/blocks/index: 0000000000000000 node/blockstorage.cpp:390 LoadBlockIndex: Assertion `GetParams().AssumeutxoForBlockhash(*snapshot_blockhash)' failed. Aborted (core dumped) ``` </details> <details> <summary>After original change, error message output:</summary> ``` 2023-10-20T15:49:12Z [init] init message: Loading block index… 2023-10-20T15:49:12Z [init] Assuming ancestors of block 00000000000000000001a0a448d6cf2546b06801389cc030b2b18c6491266815 have valid signatures. 2023-10-20T15:49:12Z [init] Setting nMinimumChainWork=000000000000000000000000000000000000000052b2559353df4117b7348b64 2023-10-20T15:49:12Z [init] Prune configured to target 3000 MiB on disk for block and undo files. 2023-10-20T15:49:12Z [init] [snapshot] detected active snapshot chainstate (/tmp/.test_utxo_2/chainstate_snapshot) - loading 2023-10-20T15:49:12Z [init] [snapshot] switching active chainstate to Chainstate [snapshot] @ height -1 (null) 2023-10-20T15:49:12Z [init] Opening LevelDB in /tmp/.test_utxo_2/blocks/index 2023-10-20T15:49:12Z [init] Opened LevelDB successfully 2023-10-20T15:49:12Z [init] Using obfuscation key for /tmp/.test_utxo_2/blocks/index: 0000000000000000 2023-10-20T15:49:13Z [init] *** Assumeutxo data not found for the given blockhash '00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054'. 2023-10-20T15:49:13Z [init] Error: Assumeutxo data not found for the given blockhash '00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054'. Error: Assumeutxo data not found for the given blockhash '00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054'. 2023-10-20T15:49:13Z [init] Shutdown requested. Exiting. 2023-10-20T15:49:13Z [init] Shutdown: In progress... 2023-10-20T15:49:13Z [scheduler] scheduler thread exit 2023-10-20T15:49:13Z [shutoff] Flushed fee estimates to fee_estimates.dat. 2023-10-20T15:49:13Z [shutoff] Shutdown: done ``` </details> <details> <summary>Alternative on error handling using <code>return error()</code> instead of <code>return FatalError()</code> used in this PR, which produces a different output and perhaps confusing:</summary> ``` 2023-10-20T21:45:58Z [init] Using obfuscation key for /tmp/.test_utxo_2/blocks/index: 0000000000000000 2023-10-20T21:45:59Z [init] ERROR: Assumeutxo data not found for the given blockhash '00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054'. 2023-10-20T21:45:59Z [init] : Error loading block database. Please restart with -reindex or -reindex-chainstate to recover. : Error loading block database. Please restart with -reindex or -reindex-chainstate to recover. 2023-10-20T21:45:59Z [init] Aborted block database rebuild. Exiting. 2023-10-20T21:45:59Z [init] Shutdown: In progress... 2023-10-20T21:45:59Z [scheduler] scheduler thread exit 2023-10-20T21:45:59Z [shutoff] Flushed fee estimates to fee_estimates.dat. 2023-10-20T21:45:59Z [shutoff] Shutdown: done ``` </details> <details> <summary>Current state (including ryanofsky <a href="https://github.com/bitcoin/bitcoin/pull/28698#discussion_r1368635965">suggestion</a>), after code change, error message output:</summary> ``` 2023-10-25T02:29:57Z [init] Using obfuscation key for /home/pablo/.test_utxo_2/regtest/blocks/index: 0000000000000000 2023-10-25T02:29:57Z [init] *** Assumeutxo data not found for the given blockhash 'f09b5835f3f8b39481f2af3257bbc2e82845552d4d2d6d31cf520fc24263ed5b'. 2023-10-25T02:29:57Z [init] Error: A fatal internal error occurred, see debug.log for details Error: A fatal internal error occurred, see debug.log for details 2023-10-25T02:29:57Z [init] Shutdown requested. Exiting. 2023-10-25T02:29:57Z [init] Shutdown: In progress... 2023-10-25T02:29:57Z [scheduler] scheduler thread exit 2023-10-25T02:29:57Z [shutoff] Flushed fee estimates to fee_estimates.dat. 2023-10-25T02:29:57Z [shutoff] Shutdown: done ``` </details> ACKs for top commit: naumenkogs: ACK 811067c theStack: ACK 811067c ryanofsky: Code review ACK 811067c. Tree-SHA512: cfc137b0a4f638b99fd7dac2c35cc729ef71ae1166a2a8960a91055ec90841cb33aed589834012cfe0e157937e2a76a88d1020ea1df2bc98e1114eb1fc8eaae4
2 parents f028470 + 811067c commit feae4e0

File tree

2 files changed

+25
-1
lines changed

2 files changed

+25
-1
lines changed

src/node/blockstorage.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,12 @@ bool BlockManager::LoadBlockIndex(const std::optional<uint256>& snapshot_blockha
387387
}
388388

389389
if (snapshot_blockhash) {
390-
const AssumeutxoData au_data = *Assert(GetParams().AssumeutxoForBlockhash(*snapshot_blockhash));
390+
const std::optional<AssumeutxoData> maybe_au_data = GetParams().AssumeutxoForBlockhash(*snapshot_blockhash);
391+
if (!maybe_au_data) {
392+
m_opts.notifications.fatalError(strprintf("Assumeutxo data not found for the given blockhash '%s'.", snapshot_blockhash->ToString()));
393+
return false;
394+
}
395+
const AssumeutxoData& au_data = *Assert(maybe_au_data);
391396
m_snapshot_height = au_data.height;
392397
CBlockIndex* base{LookupBlockIndex(*snapshot_blockhash)};
393398

test/functional/feature_assumeutxo.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
- TODO: Not an ancestor or a descendant of the snapshot block and has more work
3434
3535
"""
36+
from shutil import rmtree
37+
3638
from test_framework.test_framework import BitcoinTestFramework
3739
from test_framework.util import (
3840
assert_equal,
@@ -107,6 +109,22 @@ def expected_error(log_msg="", rpc_details=""):
107109
f.write(valid_snapshot_contents[(32 + 8 + offset + len(content)):])
108110
expected_error(log_msg=f"[snapshot] bad snapshot content hash: expected 61d9c2b29a2571a5fe285fe2d8554f91f93309666fc9b8223ee96338de25ff53, got {wrong_hash}")
109111

112+
def test_invalid_chainstate_scenarios(self):
113+
self.log.info("Test different scenarios of invalid snapshot chainstate in datadir")
114+
115+
self.log.info(" - snapshot chainstate refering to a block that is not in the assumeutxo parameters")
116+
self.stop_node(0)
117+
chainstate_snapshot_path = self.nodes[0].chain_path / "chainstate_snapshot"
118+
chainstate_snapshot_path.mkdir()
119+
with open(chainstate_snapshot_path / "base_blockhash", 'wb') as f:
120+
f.write(b'z' * 32)
121+
expected_error = f"Error: A fatal internal error occurred, see debug.log for details"
122+
self.nodes[0].assert_start_raises_init_error(expected_msg=expected_error)
123+
124+
# resurrect node again
125+
rmtree(chainstate_snapshot_path)
126+
self.start_node(0)
127+
110128
def run_test(self):
111129
"""
112130
Bring up two (disconnected) nodes, mine some new blocks on the first,
@@ -166,6 +184,7 @@ def run_test(self):
166184
assert_equal(n0.getblockchaininfo()["blocks"], FINAL_HEIGHT)
167185

168186
self.test_invalid_snapshot_scenarios(dump_output['path'])
187+
self.test_invalid_chainstate_scenarios()
169188

170189
self.log.info(f"Loading snapshot into second node from {dump_output['path']}")
171190
loaded = n1.loadtxoutset(dump_output['path'])

0 commit comments

Comments
 (0)