highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-04-07 19:48:58 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#28698: assumeutxo, blockstorage: Prevent core dump on invalid hash

Browse Source 
811067ca1cbbd4a697791cbe3ecd4bee19fe6193 test: add coverage for snapshot chainstate not matching AssumeUTXO parameters (pablomartin4btc)
4a5be10b928d4ed33d223972537c1cb79163e79c assumeutxo, blockstorage: prevent core dump on invalid hash (pablomartin4btc)

Pull request description:

  While reviewing #27596 (ran `loadtxoutset` in `mainnet` before `m_assumeutxo_data` is empty as [currently](434495a8c1/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](24deb2022b).
  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 811067ca1cbbd4a697791cbe3ecd4bee19fe6193
  theStack:
    ACK 811067ca1cbbd4a697791cbe3ecd4bee19fe6193
  ryanofsky:
    Code review ACK 811067ca1cbbd4a697791cbe3ecd4bee19fe6193.

Tree-SHA512: cfc137b0a4f638b99fd7dac2c35cc729ef71ae1166a2a8960a91055ec90841cb33aed589834012cfe0e157937e2a76a88d1020ea1df2bc98e1114eb1fc8eaae4
This commit is contained in:
fanquake 2023-10-29 10:21:51 +01:00
commit feae4e0438
No known key found for this signature in database
GPG Key ID: 2EEB9F5CC09526C1
2 changed files with 25 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/node/blockstorage.cpp
View File

@ -387,7 +387,12 @@ bool BlockManager::LoadBlockIndex(const std::optional<uint256>& snapshot_blockha
}
if (snapshot_blockhash) {
const AssumeutxoData au_data = *Assert(GetParams().AssumeutxoForBlockhash(*snapshot_blockhash));
const std::optional<AssumeutxoData> maybe_au_data = GetParams().AssumeutxoForBlockhash(*snapshot_blockhash);
if (!maybe_au_data) {
m_opts.notifications.fatalError(strprintf("Assumeutxo data not found for the given blockhash '%s'.", snapshot_blockhash->ToString()));
return false;
}
const AssumeutxoData& au_data = *Assert(maybe_au_data);
m_snapshot_height = au_data.height;
CBlockIndex* base{LookupBlockIndex(*snapshot_blockhash)};

19
test/functional/feature_assumeutxo.py
View File

@ -33,6 +33,8 @@ Interesting starting states could be loading a snapshot when the current chain t
- TODO: Not an ancestor or a descendant of the snapshot block and has more work
"""
from shutil import rmtree
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
@ -107,6 +109,22 @@ class AssumeutxoTest(BitcoinTestFramework):
f.write(valid_snapshot_contents[(32 + 8 + offset + len(content)):])
expected_error(log_msg=f"[snapshot] bad snapshot content hash: expected 61d9c2b29a2571a5fe285fe2d8554f91f93309666fc9b8223ee96338de25ff53, got {wrong_hash}")
def test_invalid_chainstate_scenarios(self):
self.log.info("Test different scenarios of invalid snapshot chainstate in datadir")
self.log.info(" - snapshot chainstate refering to a block that is not in the assumeutxo parameters")
self.stop_node(0)
chainstate_snapshot_path = self.nodes[0].chain_path / "chainstate_snapshot"
chainstate_snapshot_path.mkdir()
with open(chainstate_snapshot_path / "base_blockhash", 'wb') as f:
f.write(b'z' * 32)
expected_error = f"Error: A fatal internal error occurred, see debug.log for details"
self.nodes[0].assert_start_raises_init_error(expected_msg=expected_error)
# resurrect node again
rmtree(chainstate_snapshot_path)
self.start_node(0)
def run_test(self):
"""
Bring up two (disconnected) nodes, mine some new blocks on the first,
@ -166,6 +184,7 @@ class AssumeutxoTest(BitcoinTestFramework):
assert_equal(n0.getblockchaininfo()["blocks"], FINAL_HEIGHT)
self.test_invalid_snapshot_scenarios(dump_output['path'])
self.test_invalid_chainstate_scenarios()
self.log.info(f"Loading snapshot into second node from {dump_output['path']}")
loaded = n1.loadtxoutset(dump_output['path'])