merge bitcoin#17192: Add CHECK_NONFATAL and use it in src/rpc

src/Makefile.am

@@ -289,6 +289,7 @@ BITCOIN_CORE_H = \
  unordered_lru_cache.h \
  util/bip32.h \
  util/bytevectorhash.h \
+ util/check.h \
  util/error.h \
  util/fees.h \
  util/spanparsing.h \

src/rpc/blockchain.cpp

@@ -11,12 +11,12 @@
 #include <chain.h>
 #include <chainparams.h>
 #include <coins.h>
-#include <node/coinstats.h>
 #include <core_io.h>
 #include <consensus/validation.h>
 #include <index/blockfilterindex.h>
-#include <key_io.h>
 #include <index/txindex.h>
+#include <key_io.h>
+#include <node/coinstats.h>
 #include <policy/feerate.h>
 #include <policy/policy.h>
 #include <primitives/transaction.h>
@@ -40,7 +40,6 @@
 #include <llmq/chainlocks.h>
 #include <llmq/instantsend.h>
 
-#include <assert.h>
 #include <stdint.h>
 
 #include <univalue.h>

src/rpc/misc.cpp

@@ -15,8 +15,9 @@
 #include <rpc/util.h>
 #include <script/descriptor.h>
 #include <txmempool.h>
-#include <util/system.h>
+#include <util/check.h>
 #include <util/strencodings.h>
+#include <util/system.h>
 #include <validation.h>
 #include <util/validation.h>
 
@@ -1270,6 +1271,7 @@ static UniValue echo(const JSONRPCRequest& request)
  throw std::runtime_error(
  RPCHelpMan{"echo|echojson ...",
  "\nSimply echo back the input arguments. This command is for testing.\n"
+ "\nIt will return an internal bug report when exactly 100 arguments are passed.\n"
  "\nThe difference between echo and echojson is that echojson has argument conversion enabled in the client-side table in "
  "dash-cli and the GUI. There is no server-side difference.",
  {},
@@ -1278,6 +1280,8 @@ static UniValue echo(const JSONRPCRequest& request)
  }.ToString()
  );
 
+ CHECK_NONFATAL(request.params.size() != 100);
+
  return request.params;
 }
 

src/rpc/util.h

@@ -6,12 +6,13 @@
 #define BITCOIN_RPC_UTIL_H
 
 #include <node/transaction.h>
-#include <pubkey.h>
 #include <protocol.h>
+#include <pubkey.h>
 #include <rpc/protocol.h>
 #include <rpc/request.h>
 #include <script/standard.h>
 #include <univalue.h>
+#include <util/check.h>
 #include <util/strencodings.h>
 
 #include <string>
@@ -143,7 +144,7 @@ struct RPCArg {
  m_oneline_description{oneline_description},
  m_type_str{type_str}
  {
- assert(type != Type::ARR && type != Type::OBJ);
+ CHECK_NONFATAL(type != Type::ARR && type != Type::OBJ);
  }
 
  RPCArg(
@@ -162,7 +163,7 @@ struct RPCArg {
  m_oneline_description{oneline_description},
  m_type_str{type_str}
  {
- assert(type == Type::ARR || type == Type::OBJ);
+ CHECK_NONFATAL(type == Type::ARR || type == Type::OBJ);
  }
 
  bool IsOptional() const;
@@ -191,14 +192,14 @@ struct RPCResult {
  explicit RPCResult(std::string result)
  : m_cond{}, m_result{std::move(result)}
  {
- assert(!m_result.empty());
+ CHECK_NONFATAL(!m_result.empty());
  }
 
  RPCResult(std::string cond, std::string result)
  : m_cond{std::move(cond)}, m_result{std::move(result)}
  {
- assert(!m_cond.empty());
- assert(!m_result.empty());
+ CHECK_NONFATAL(!m_cond.empty());
+ CHECK_NONFATAL(!m_result.empty());
  }
 };
 

src/util/check.h

@@ -0,0 +1,41 @@
+// Copyright (c) 2019 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_UTIL_CHECK_H
+#define BITCOIN_UTIL_CHECK_H
+
+#include <tinyformat.h>
+
+#include <stdexcept>
+
+class NonFatalCheckError : public std::runtime_error
+{
+ using std::runtime_error::runtime_error;
+};
+
+/**
+ * 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 validating user input
+ * - where a failure to fulfill the condition is recoverable and does not abort the program
+ *
+ * For example in RPC code, where it is undersirable to crash the whole program, this can be generally used to replace
+ * asserts or recoverable logic errors. A NonFatalCheckError in RPC code is caught and passed as a string to the RPC
+ * 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)
+
+#endif // BITCOIN_UTIL_CHECK_H

test/functional/rpc_misc.py

@@ -23,6 +23,13 @@ def set_test_params(self):
  def run_test(self):
  node = self.nodes[0]
 
+ self.log.info("test CHECK_NONFATAL")
+ assert_raises_rpc_error(
+ -1,
+ "Internal bug detected: 'request.params.size() != 100'",
+ lambda: node.echo(*[0] * 100),
+ )
+
  self.log.info("test getmemoryinfo")
  memory = node.getmemoryinfo()['locked']
  assert_greater_than(memory['used'], 0)