util/check: Add CHECK_NONFATAL identity function and a NONFATAL_UNREA…

…CHABLE macro

Summary: This is a backport of [[bitcoin/bitcoin#24812 | core#24812]]

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, sdulfari

Reviewed By: #bitcoin_abc, sdulfari

Subscribers: sdulfari

Differential Revision: https://reviews.bitcoinabc.org/D13096

src/rpc/blockchain.cpp

@@ -35,6 +35,7 @@
 #include <txdb.h>
 #include <txmempool.h>
 #include <undo.h>
+#include <util/check.h>
 #include <util/strencodings.h>
 #include <util/translation.h>
 #include <validation.h>
@@ -1291,8 +1292,7 @@ static RPCHelpMan pruneblockchain() {
             }
 
             PruneBlockFilesManual(active_chainstate, height);
-            const CBlockIndex *block = active_chain.Tip();
-            CHECK_NONFATAL(block);
+            const CBlockIndex *block = CHECK_NONFATAL(active_chain.Tip());
             while (block->pprev && (block->pprev->nStatus.hasData())) {
                 block = block->pprev;
             }
@@ -1882,8 +1882,8 @@ RPCHelpMan getblockchaininfo() {
             LOCK(cs_main);
             CChainState &active_chainstate = chainman.ActiveChainstate();
 
-            const CBlockIndex *tip = active_chainstate.m_chain.Tip();
-            CHECK_NONFATAL(tip);
+            const CBlockIndex *tip =
+                CHECK_NONFATAL(active_chainstate.m_chain.Tip());
             const int height = tip->nHeight;
 
             UniValue obj(UniValue::VOBJ);
@@ -1906,8 +1906,7 @@ RPCHelpMan getblockchaininfo() {
             obj.pushKV("pruned", node::fPruneMode);
 
             if (node::fPruneMode) {
-                const CBlockIndex *block = tip;
-                CHECK_NONFATAL(block);
+                const CBlockIndex *block = CHECK_NONFATAL(tip);
                 while (block->pprev && (block->pprev->nStatus.hasData())) {
                     block = block->pprev;
                 }
@@ -3074,11 +3073,9 @@ static RPCHelpMan scantxoutset() {
                     CChainState &active_chainstate =
                         chainman.ActiveChainstate();
                     active_chainstate.ForceFlushStateToDisk();
-                    pcursor = std::unique_ptr<CCoinsViewCursor>(
-                        active_chainstate.CoinsDB().Cursor());
-                    CHECK_NONFATAL(pcursor);
-                    tip = active_chainstate.m_chain.Tip();
-                    CHECK_NONFATAL(tip);
+                    pcursor = CHECK_NONFATAL(std::unique_ptr<CCoinsViewCursor>(
+                        active_chainstate.CoinsDB().Cursor()));
+                    tip = CHECK_NONFATAL(active_chainstate.m_chain.Tip());
                 }
                 bool res = FindScriptPubKey(
                     g_scan_progress, g_should_abort_scan, count, pcursor.get(),
@@ -3301,8 +3298,8 @@ UniValue CreateUTXOSnapshot(NodeContext &node, CChainState &chainstate,
 
         pcursor =
             std::unique_ptr<CCoinsViewCursor>(chainstate.CoinsDB().Cursor());
-        tip = chainstate.m_blockman.LookupBlockIndex(stats.hashBlock);
-        CHECK_NONFATAL(tip);
+        tip = CHECK_NONFATAL(
+            chainstate.m_blockman.LookupBlockIndex(stats.hashBlock));
     }
 
     SnapshotMetadata metadata{tip->GetBlockHash(), stats.coins_count,

src/rpc/misc.cpp

@@ -512,9 +512,9 @@ static RPCHelpMan mockscheduler() {
                     "delta_time must be between 1 and 3600 seconds (1 hr)");
             }
 
-            auto node_context = util::AnyPtr<NodeContext>(request.context);
+            auto node_context =
+                CHECK_NONFATAL(util::AnyPtr<NodeContext>(request.context));
             // protect against null pointer dereference
-            CHECK_NONFATAL(node_context);
             CHECK_NONFATAL(node_context->scheduler);
             node_context->scheduler->MockForward(
                 std::chrono::seconds(delta_seconds));

src/rpc/rawtransaction.cpp

@@ -34,6 +34,7 @@
 #include <txmempool.h>
 #include <uint256.h>
 #include <util/bip32.h>
+#include <util/check.h>
 #include <util/error.h>
 #include <util/moneystr.h>
 #include <util/strencodings.h>

src/rpc/util.cpp

@@ -9,6 +9,7 @@
 #include <script/descriptor.h>
 #include <script/signingprovider.h>
 #include <tinyformat.h>
+#include <util/check.h>
 #include <util/strencodings.h>
 #include <util/string.h>
 #include <util/translation.h>
@@ -791,7 +792,7 @@ void RPCResult::ToSections(Sections &sections, const OuterType outer_type,
         }
         case Type::ANY: {
             // Only for testing
-            CHECK_NONFATAL(false);
+            NONFATAL_UNREACHABLE();
         }
         case Type::NONE: {
             sections.PushSection(
@@ -874,7 +875,7 @@ void RPCResult::ToSections(Sections &sections, const OuterType outer_type,
             return;
         } // no default case, so the compiler can warn about missing cases
     }
-    CHECK_NONFATAL(false);
+    NONFATAL_UNREACHABLE();
 }
 
 bool RPCResult::MatchesType(const UniValue &result) const {
@@ -910,7 +911,7 @@ bool RPCResult::MatchesType(const UniValue &result) const {
             return UniValue::VOBJ == result.getType();
         }
     } // no default case, so the compiler can warn about missing cases
-    CHECK_NONFATAL(false);
+    NONFATAL_UNREACHABLE();
 }
 
 std::string RPCArg::ToStringObj(const bool oneline) const {
@@ -944,11 +945,11 @@ std::string RPCArg::ToStringObj(const bool oneline) const {
         case Type::OBJ:
         case Type::OBJ_USER_KEYS:
             // Currently unused, so avoid writing dead code
-            CHECK_NONFATAL(false);
+            NONFATAL_UNREACHABLE();
 
             // no default case, so the compiler can warn about missing cases
     }
-    CHECK_NONFATAL(false);
+    NONFATAL_UNREACHABLE();
     return res + "unknown";
 }
 
@@ -987,7 +988,7 @@ std::string RPCArg::ToString(const bool oneline) const {
             return "[" + res + "...]";
         } // no default case, so the compiler can warn about missing cases
     }
-    CHECK_NONFATAL(false);
+    NONFATAL_UNREACHABLE();
 }
 
 static std::pair<int64_t, int64_t> ParseRange(const UniValue &value) {

src/util/check.h

@@ -17,8 +17,26 @@ class NonFatalCheckError : public std::runtime_error {
     using std::runtime_error::runtime_error;
 };
 
+#define format_internal_error(msg, file, line, func, report)                   \
+    strprintf("Internal bug detected: \"%s\"\n%s:%d (%s)\nPlease report this " \
+              "issue here: %s\n",                                              \
+              msg, file, line, func, report)
+
+/** Helper for CHECK_NONFATAL() */
+template <typename T>
+T &&inline_check_non_fatal(T &&val, const char *file, int line,
+                           const char *func, const char *assertion) {
+    if (!(val)) {
+        throw NonFatalCheckError(format_internal_error(
+            assertion, file, line, func, PACKAGE_BUGREPORT));
+    }
+
+    return std::forward<T>(val);
+}
+
 /**
- * Throw a NonFatalCheckError when the condition evaluates to false
+ * Identity function. Throw a NonFatalCheckError when the condition evaluates
+ * to false
  *
  * This should only be used
  * - where the condition is assumed to be true, not for error handling or
@@ -32,16 +50,7 @@ class NonFatalCheckError : public std::runtime_error {
  * caller, which can then report the issue to the developers.
  */
 #define CHECK_NONFATAL(condition)                                              \
-    do {                                                                       \
-        if (!(condition)) {                                                    \
-            throw NonFatalCheckError(                                          \
-                strprintf("%s:%d (%s)\n"                                       \
-                          "Internal bug detected: '%s'\n"                      \
-                          "You may report this issue here: %s\n",              \
-                          __FILE__, __LINE__, __func__, (#condition),          \
-                          PACKAGE_BUGREPORT));                                 \
-        }                                                                      \
-    } while (false)
+    inline_check_non_fatal(condition, __FILE__, __LINE__, __func__, #condition)
 
 #if defined(NDEBUG)
 #error "Cannot compile without assertions!"
@@ -80,4 +89,15 @@ template <typename T> T get_pure_r_value(T &&val) {
     }())
 #endif
 
+/**
+ * NONFATAL_UNREACHABLE() is a macro that is used to mark unreachable code.
+ * It throws a NonFatalCheckError.
+ * This is used to mark code that is not yet implemented or is not yet
+ * reachable.
+ */
+#define NONFATAL_UNREACHABLE()                                                 \
+    throw NonFatalCheckError(format_internal_error(                            \
+        "Unreachable code reached (non-fatal)", __FILE__, __LINE__, __func__,  \
+        PACKAGE_BUGREPORT))
+
 #endif // BITCOIN_UTIL_CHECK_H

src/wallet/rpcwallet.cpp

@@ -4117,8 +4117,7 @@ RPCHelpMan getaddressesbylabel() {
                     // CWallet::m_address_book is not expected to contain
                     // duplicate address strings, but build a separate set as a
                     // precaution just in case it does.
-                    bool unique = addresses.emplace(address).second;
-                    CHECK_NONFATAL(unique);
+                    CHECK_NONFATAL(addresses.emplace(address).second);
                     // UniValue::pushKV checks if the key exists in O(N)
                     // and since duplicate addresses are unexpected (checked
                     // with std::set in O(log(N))), UniValue::__pushKV is used

test/functional/rpc_misc.py

@@ -26,7 +26,7 @@ def run_test(self):
         self.log.info("test CHECK_NONFATAL")
         assert_raises_rpc_error(
             -1,
-            'Internal bug detected: \'request.params[9].get_str() != "trigger_internal_bug"\'',
+            'Internal bug detected: "request.params[9].get_str() != "trigger_internal_bug""',
             lambda: node.echo(arg9='trigger_internal_bug'),
         )