Merge bitcoin/bitcoin#35379: test: Fix feature_dbcrash.py --usecli error

fad585b6e5 test: Wait for node exit after crash in verify_utxo_hash (MarcoFalke)
faf1475514 ci: Exclude feature_dbcrash.py under --v2transport --usecli (MarcoFalke)
fac27d702f test: Fix feature_dbcrash.py --usecli intermittent error (MarcoFalke)
fa09de8b68 test: [refactor] Simplify submit_block_catch_error (MarcoFalke)

Pull request description:

  This fixes a small intermittent issue that snuck in via commit fa8d4d5c35.

  Generally, it seems tedious and brittle trying to enumerate all possible exception types on all platforms and all test configs, all possible errno values, etc.

  So just treat any `Exception` as crash, and confirm it in the test. The test would still fail, if a crash did not happen after an Exception, but the failure may be minimally more tedious to debug. I think this is fine, because failures should be rare and having simpler and more flexible test code is preferable.

  ----

  Also, disable the test for now in CI, because it is quite slow.

ACKs for top commit:
  willcl-ark:
    ACK fad585b6e5

Tree-SHA512: 6ff69a53908b22904780f29def473f8e26c5b608d89420ac76768d06ea3e3394b5d3f4d488100f9d47f943ff5778a555302ccdaebc11fda7c009205b6a6ca05c

ci/test/00_setup_env_native_alpine_musl.sh

@@ -17,5 +17,5 @@ export BITCOIN_CONFIG="\
  -DREDUCE_EXPORTS=ON \
  -DCMAKE_BUILD_TYPE=Debug \
 "
-export TEST_RUNNER_EXTRA="--v2transport --usecli --extended"
+export TEST_RUNNER_EXTRA="--v2transport --usecli --extended --exclude feature_dbcrash"  # Run extended tests under --usecli and --v2transport, but exclude the very slow dbcrash
 export BITCOIN_CMD="bitcoin -m" # Used in functional tests

test/functional/feature_dbcrash.py

@@ -25,10 +25,7 @@
          - restart until recovery succeeds
          - check that utxo matches node3 using gettxoutsetinfo"""
 
-import errno
-import http.client
 import random
-import subprocess
 import time
 
 from test_framework.blocktools import COINBASE_MATURITY
@@ -73,7 +70,7 @@ class ChainstateWriteCrashTest(BitcoinTestFramework):
         self.start_nodes()
         # Leave them unconnected, we'll use submitblock directly in this test
 
-    def restart_node(self, node_index, expected_tip):
+    def restart_node(self, node_index, *, expected_tip):
         """Start up a given node id, wait for the tip to reach the given block hash, and calculate the utxo hash.
 
         Exceptions during startup or subsequent RPC calls should indicate a node crash (due to -dbcrashratio), in which case we try again. Give up
@@ -90,7 +87,7 @@ class ChainstateWriteCrashTest(BitcoinTestFramework):
             except Exception:
                 # An exception here should mean the node is about to crash.
                 # If bitcoind exits, then try again.  wait_for_node_exit()
-                # should raise an exception if bitcoind doesn't exit.
+                # enforces that bitcoind crashed.
                 self.wait_for_node_exit(node_index, timeout=10)
             self.crashed_on_restart += 1
 
@@ -105,25 +102,15 @@ class ChainstateWriteCrashTest(BitcoinTestFramework):
         """Try submitting a block to the given node.
 
         Catch any exceptions that indicate the node has crashed.
+        The caller will check that a crash happened.
         Returns true if the block was submitted successfully; false otherwise."""
 
         try:
             self.nodes[node_index].submitblock(block)
             return True
-        except (http.client.CannotSendRequest, http.client.RemoteDisconnected) as e:
+        except Exception as e:
             self.log.debug(f"node {node_index} submitblock raised exception: {e}")
             return False
-        except subprocess.CalledProcessError as e:
-            self.log.debug(f"node {node_index} submitblock raised CalledProcessError: {e}")
-            return False
-        except OSError as e:
-            self.log.debug(f"node {node_index} submitblock raised OSError exception: errno={e.errno}")
-            if e.errno in [errno.EPIPE, errno.ECONNREFUSED, errno.ECONNRESET]:
-                # The node has likely crashed
-                return False
-            else:
-                # Unexpected exception, raise
-                raise
 
     def sync_node3blocks(self, block_hashes):
         """Use submitblock to sync node3's chain with the other nodes
@@ -149,9 +136,10 @@ class ChainstateWriteCrashTest(BitcoinTestFramework):
                 if not self.submit_block_catch_error(i, block):
                     # TODO: more carefully check that the crash is due to -dbcrashratio
                     # (change the exit code perhaps, and check that here?)
+                    # wait_for_node_exit() enforces that bitcoind crashed.
                     self.wait_for_node_exit(i, timeout=30)
                     self.log.debug(f"Restarting node {i} after block hash {block_hash}")
-                    nodei_utxo_hash = self.restart_node(i, block_hash)
+                    nodei_utxo_hash = self.restart_node(i, expected_tip=block_hash)
                     assert nodei_utxo_hash is not None
                     self.restart_counts[i] += 1
                 else:
@@ -180,9 +168,10 @@ class ChainstateWriteCrashTest(BitcoinTestFramework):
         for i in range(3):
             try:
                 nodei_utxo_hash = self.nodes[i].gettxoutsetinfo()['hash_serialized_3']
-            except OSError:
+            except Exception:
                 # probably a crash on db flushing
-                nodei_utxo_hash = self.restart_node(i, self.nodes[3].getbestblockhash())
+                self.wait_for_node_exit(i, timeout=10)
+                nodei_utxo_hash = self.restart_node(i, expected_tip=self.nodes[3].getbestblockhash())
             assert_equal(nodei_utxo_hash, node3_utxo_hash)
 
     def generate_small_transactions(self, node, count, utxo_list):