util: Abort on failing CHECK_NONFATAL in debug builds

This requires adjusting some tests to force exceptions over aborts, or
accept either exceptions or aborts.

Also, remove a fuzz test in integer.cpp that is mostly redundant with
the unit test added in the prior commit.

src/test/fuzz/integer.cpp

@@ -255,9 +255,4 @@ FUZZ_TARGET(integer, .init = initialize_integer)
         } catch (const std::ios_base::failure&) {
         }
     }
-
-    try {
-        CHECK_NONFATAL(b);
-    } catch (const NonFatalCheckError&) {
-    }
 }

src/test/fuzz/rpc.cpp

@@ -381,6 +381,12 @@ FUZZ_TARGET(rpc, .init = initialize_rpc)
         arguments.push_back(ConsumeRPCArgument(fuzzed_data_provider, good_data));
     }
     try {
+        std::optional<test_only_CheckFailuresAreExceptionsNotAborts> maybe_mock{};
+        if (rpc_command == "echo") {
+            // Avoid aborting fuzzing for this specific test-only RPC with an
+            // intentional trigger_internal_bug
+            maybe_mock.emplace();
+        }
         rpc_testing_setup->CallRPC(rpc_command, arguments);
     } catch (const UniValue& json_rpc_error) {
         const std::string error_msg{json_rpc_error.find_value("message").get_str()};

src/test/rpc_tests.cpp

@@ -534,20 +534,23 @@ BOOST_AUTO_TEST_CASE(check_dup_param_names)
     make_rpc({{"p1", NAMED_ONLY}, {"p2", NAMED}});
     make_rpc({{"p1", NAMED_ONLY}, {"p2", NAMED_ONLY}});
 
-    // Error if parameters names are duplicates, unless one parameter is
+    {
-    // positional and the other is named and .also_positional is true.
+        test_only_CheckFailuresAreExceptionsNotAborts mock_checks{};
-    BOOST_CHECK_THROW(make_rpc({{"p1", POSITIONAL}, {"p1", POSITIONAL}}), NonFatalCheckError);
+        // Error if parameter names are duplicates, unless one parameter is
-    make_rpc({{"p1", POSITIONAL}, {"p1", NAMED}});
+        // positional and the other is named and .also_positional is true.
-    BOOST_CHECK_THROW(make_rpc({{"p1", POSITIONAL}, {"p1", NAMED_ONLY}}), NonFatalCheckError);
+        BOOST_CHECK_THROW(make_rpc({{"p1", POSITIONAL}, {"p1", POSITIONAL}}), NonFatalCheckError);
-    make_rpc({{"p1", NAMED}, {"p1", POSITIONAL}});
+        make_rpc({{"p1", POSITIONAL}, {"p1", NAMED}});
-    BOOST_CHECK_THROW(make_rpc({{"p1", NAMED}, {"p1", NAMED}}), NonFatalCheckError);
+        BOOST_CHECK_THROW(make_rpc({{"p1", POSITIONAL}, {"p1", NAMED_ONLY}}), NonFatalCheckError);
-    BOOST_CHECK_THROW(make_rpc({{"p1", NAMED}, {"p1", NAMED_ONLY}}), NonFatalCheckError);
+        make_rpc({{"p1", NAMED}, {"p1", POSITIONAL}});
-    BOOST_CHECK_THROW(make_rpc({{"p1", NAMED_ONLY}, {"p1", POSITIONAL}}), NonFatalCheckError);
+        BOOST_CHECK_THROW(make_rpc({{"p1", NAMED}, {"p1", NAMED}}), NonFatalCheckError);
-    BOOST_CHECK_THROW(make_rpc({{"p1", NAMED_ONLY}, {"p1", NAMED}}), NonFatalCheckError);
+        BOOST_CHECK_THROW(make_rpc({{"p1", NAMED}, {"p1", NAMED_ONLY}}), NonFatalCheckError);
-    BOOST_CHECK_THROW(make_rpc({{"p1", NAMED_ONLY}, {"p1", NAMED_ONLY}}), NonFatalCheckError);
+        BOOST_CHECK_THROW(make_rpc({{"p1", NAMED_ONLY}, {"p1", POSITIONAL}}), NonFatalCheckError);
+        BOOST_CHECK_THROW(make_rpc({{"p1", NAMED_ONLY}, {"p1", NAMED}}), NonFatalCheckError);
+        BOOST_CHECK_THROW(make_rpc({{"p1", NAMED_ONLY}, {"p1", NAMED_ONLY}}), NonFatalCheckError);
 
-    // Make sure duplicate aliases are detected too.
+        // Make sure duplicate aliases are detected too.
-    BOOST_CHECK_THROW(make_rpc({{"p1", POSITIONAL}, {"p2|p1", NAMED_ONLY}}), NonFatalCheckError);
+        BOOST_CHECK_THROW(make_rpc({{"p1", POSITIONAL}, {"p2|p1", NAMED_ONLY}}), NonFatalCheckError);
+    }
 }
 
 BOOST_AUTO_TEST_CASE(help_example)

src/util/check.h

@@ -60,11 +60,17 @@ public:
     NonFatalCheckError(std::string_view msg, std::string_view file, int line, std::string_view func);
 };
 
+/** Internal helper */
+void assertion_fail(std::string_view file, int line, std::string_view func, std::string_view assertion);
+
 /** Helper for CHECK_NONFATAL() */
 template <typename T>
 T&& inline_check_non_fatal(LIFETIMEBOUND T&& val, const char* file, int line, const char* func, const char* assertion)
 {
     if (!val) {
+        if constexpr (G_ABORT_ON_FAILED_ASSUME) {
+            assertion_fail(file, line, func, assertion);
+        }
         throw NonFatalCheckError{assertion, file, line, func};
     }
     return std::forward<T>(val);
@@ -74,9 +80,6 @@ T&& inline_check_non_fatal(LIFETIMEBOUND T&& val, const char* file, int line, co
 #error "Cannot compile without assertions!"
 #endif
 
-/** Helper for Assert() */
-void assertion_fail(std::string_view file, int line, std::string_view func, std::string_view assertion);
-
 /** Helper for Assert()/Assume() */
 template <bool IS_ASSERT, typename T>
 constexpr T&& inline_assertion_check(LIFETIMEBOUND T&& val, [[maybe_unused]] const char* file, [[maybe_unused]] int line, [[maybe_unused]] const char* func, [[maybe_unused]] const char* assertion)

test/functional/rpc_misc.py

@@ -15,6 +15,9 @@ from test_framework.util import (
 
 from test_framework.authproxy import JSONRPCException
 
+import http
+import subprocess
+
 
 class RpcMiscTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -24,11 +27,22 @@ class RpcMiscTest(BitcoinTestFramework):
         node = self.nodes[0]
 
         self.log.info("test CHECK_NONFATAL")
-        assert_raises_rpc_error(
+        msg_internal_bug = 'request.params[9].get_str() != "trigger_internal_bug"'
-            -1,
+        self.restart_node(0)  # Required to flush the chainstate
-            'Internal bug detected: request.params[9].get_str() != "trigger_internal_bug"',
+        try:
-            lambda: node.echo(arg9='trigger_internal_bug'),
+            node.echo(arg9="trigger_internal_bug")
-        )
+            assert False  # Must hit one of the exceptions below
+        except (
+                subprocess.CalledProcessError,
+                http.client.CannotSendRequest,
+                http.client.RemoteDisconnected,
+        ):
+            self.log.info("Restart node after crash")
+            assert_equal(-6, node.process.wait(timeout=10))
+            self.start_node(0)
+        except JSONRPCException as e:
+            assert_equal(e.error["code"], -1)
+            assert f"Internal bug detected: {msg_internal_bug}" in e.error["message"]
 
         self.log.info("test getmemoryinfo")
         memory = node.getmemoryinfo()['locked']