Merge bitcoin/bitcoin#28652: assumeutxo: fail early if snapshot block…

… hash doesn't match AssumeUTXO parameters

9620cb4 assumeutxo: fail early if snapshot block hash doesn't match AssumeUTXO parameters (Sebastian Falbesoner)

Pull request description:

  Right now the `loadtxoutset` RPC call treats literally all files with a minimum size of 40 bytes (=size of metadata) as potential valid snapshot candidates and the waiting loop for seeing the metadata block hash in the headers chain is always entered, e.g.:
  ```
  $ ./src/bitcoin-cli loadtxoutset ~/.vimrc
  <wait>

  bitcoind log:
  ...
  2023-10-15T14:55:45Z [snapshot] waiting to see blockheader 626174207465730a7265626d756e207465730a656c62616e65207861746e7973 in headers chain before snapshot activation
  ...
  ```
  There is no point in doing any further action though if we already know from the start that the UTXO snapshot loading won't be successful. This PR adds an assumeutxo parameter check immediately after the metadata is read in, so we can fail immediately on a mismatch:
  ```
  $ ./src/bitcoin-cli loadtxoutset ~/.vimrc
  error code: -32603
  error message:
  Unable to load UTXO snapshot, assumeutxo block hash in snapshot metadata not recognized (626174207465730a7265626d756e207465730a656c62616e
  65207861746e7973)
  ```
  This way, users who mistakenly try to load files that are not snapshots don't have to wait 10 minutes (=the block header waiting timeout) anymore to get a negative response. If a file is loaded which is a valid snapshot (referencing to an existing block hash), but one which doesn't match the parameters, the feedback is also faster, as we don't have to wait anymore to see the hash in the headers chain before getting an error.

  This is also partially fixes #28621.

ACKs for top commit:
  maflcko:
    lgtm ACK 9620cb4
  ryanofsky:
    Code review ACK 9620cb4. This should fix an annoyance and bad UX.
  pablomartin4btc:
    tACK 9620cb4

Tree-SHA512: f88b865e9d46254858e57c024463f389cd9d8760a7cb30c190aa1723a931e159987dfc2263a733825d700fa612e7416691e4d8aab64058f1aeb0a7fa9233ac9c

src/rpc/blockchain.cpp

@@ -2737,6 +2737,7 @@ static RPCHelpMan loadtxoutset()
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
     NodeContext& node = EnsureAnyNodeContext(request.context);
+    ChainstateManager& chainman = EnsureChainman(node);
     fs::path path{AbsPathForConfigVal(EnsureArgsman(node), fs::u8path(request.params[0].get_str()))};
 
     FILE* file{fsbridge::fopen(path, "rb")};
@@ -2751,14 +2752,16 @@ static RPCHelpMan loadtxoutset()
     afile >> metadata;
 
     uint256 base_blockhash = metadata.m_base_blockhash;
+    if (!chainman.GetParams().AssumeutxoForBlockhash(base_blockhash).has_value()) {
+        throw JSONRPCError(RPC_INTERNAL_ERROR, strprintf("Unable to load UTXO snapshot, "
+            "assumeutxo block hash in snapshot metadata not recognized (%s)", base_blockhash.ToString()));
+    }
     int max_secs_to_wait_for_headers = 60 * 10;
     CBlockIndex* snapshot_start_block = nullptr;
 
     LogPrintf("[snapshot] waiting to see blockheader %s in headers chain before snapshot activation\n",
         base_blockhash.ToString());
 
-    ChainstateManager& chainman = EnsureChainman(node);
-
     while (max_secs_to_wait_for_headers > 0) {
         snapshot_start_block = WITH_LOCK(::cs_main,
             return chainman.m_blockman.LookupBlockIndex(base_blockhash));

test/functional/feature_assumeutxo.py

@@ -73,17 +73,12 @@ def test_invalid_snapshot_scenarios(self, valid_snapshot_path):
         bad_snapshot_path = valid_snapshot_path + '.mod'
 
         self.log.info("  - snapshot file refering to a block that is not in the assumeutxo parameters")
-        # we can only test this with a block that is already known, as otherwise the `loadtxoutset` RPC
-        # would time out (waiting to see the hash in the headers chain), rather than error immediately
-        bad_snapshot_height = SNAPSHOT_BASE_HEIGHT - 1
+        bad_snapshot_block_hash = self.nodes[0].getblockhash(SNAPSHOT_BASE_HEIGHT - 1)
         with open(bad_snapshot_path, 'wb') as f:
-            bad_snapshot_block_hash = self.nodes[0].getblockhash(bad_snapshot_height)
             # block hash of the snapshot base is stored right at the start (first 32 bytes)
             f.write(bytes.fromhex(bad_snapshot_block_hash)[::-1] + valid_snapshot_contents[32:])
-
-        expected_log = f"assumeutxo height in snapshot metadata not recognized ({bad_snapshot_height}) - refusing to load snapshot"
-        with self.nodes[1].assert_debug_log([expected_log]):
-            assert_raises_rpc_error(-32603, "Unable to load UTXO snapshot", self.nodes[1].loadtxoutset, bad_snapshot_path)
+        error_details = f"assumeutxo block hash in snapshot metadata not recognized ({bad_snapshot_block_hash})"
+        assert_raises_rpc_error(-32603, f"Unable to load UTXO snapshot, {error_details}", self.nodes[1].loadtxoutset, bad_snapshot_path)
 
         self.log.info("  - snapshot file with wrong number of coins")
         valid_num_coins = struct.unpack("<I", valid_snapshot_contents[32:32 + 4])[0]