Skip to content

Commit c98bd13

Browse files
adamjonasadamjonas
committed
replace asserts in RPC code with CHECK_NONFATAL and add linter
1 parent edd9d07 commit c98bd13

File tree

6 files changed

+34
-23
lines changed

6 files changed

+34
-23
lines changed

‎src/rpc/blockchain.cpp‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ static CUpdatedBlock latestblock;
5858
*/
5959
double GetDifficulty(const CBlockIndex* blockindex)
6060
{
61-
assert(blockindex);
61+
CHECK_NONFATAL(blockindex);
6262

6363
int nShift = (blockindex->nBits >> 24) & 0xff;
6464
double dDiff =
@@ -957,7 +957,7 @@ static UniValue pruneblockchain(const JSONRPCRequest& request)
957957

958958
PruneBlockFilesManual(height);
959959
const CBlockIndex* block = ::ChainActive().Tip();
960-
assert(block);
960+
CHECK_NONFATAL(block);
961961
while (block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA)) {
962962
block = block->pprev;
963963
}
@@ -1252,7 +1252,7 @@ UniValue getblockchaininfo(const JSONRPCRequest& request)
12521252
obj.pushKV("pruned", fPruneMode);
12531253
if (fPruneMode) {
12541254
const CBlockIndex* block = tip;
1255-
assert(block);
1255+
CHECK_NONFATAL(block);
12561256
while (block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA)) {
12571257
block = block->pprev;
12581258
}
@@ -1598,7 +1598,7 @@ static UniValue getchaintxstats(const JSONRPCRequest& request)
15981598
}
15991599
}
16001600

1601-
assert(pindex != nullptr);
1601+
CHECK_NONFATAL(pindex != nullptr);
16021602

16031603
if (request.params[0].isNull()) {
16041604
blockcount = std::max(0, std::min(blockcount, pindex->nHeight - 1));
@@ -1771,7 +1771,7 @@ static UniValue getblockstats(const JSONRPCRequest& request)
17711771
}
17721772
}
17731773

1774-
assert(pindex != nullptr);
1774+
CHECK_NONFATAL(pindex != nullptr);
17751775

17761776
std::set<std::string> stats;
17771777
if (!request.params[1].isNull()) {
@@ -1871,7 +1871,7 @@ static UniValue getblockstats(const JSONRPCRequest& request)
18711871
}
18721872

18731873
CAmount txfee = tx_total_in - tx_total_out;
1874-
assert(MoneyRange(txfee));
1874+
CHECK_NONFATAL(MoneyRange(txfee));
18751875
if (do_medianfee) {
18761876
fee_array.push_back(txfee);
18771877
}
@@ -2008,7 +2008,7 @@ class CoinsViewScanReserver
20082008
explicit CoinsViewScanReserver() : m_could_reserve(false) {}
20092009

20102010
bool reserve() {
2011-
assert (!m_could_reserve);
2011+
CHECK_NONFATAL(!m_could_reserve);
20122012
std::lock_guard<std::mutex> lock(g_utxosetscan);
20132013
if (g_scan_in_progress) {
20142014
return false;
@@ -2135,9 +2135,9 @@ UniValue scantxoutset(const JSONRPCRequest& request)
21352135
LOCK(cs_main);
21362136
::ChainstateActive().ForceFlushStateToDisk();
21372137
pcursor = std::unique_ptr<CCoinsViewCursor>(::ChainstateActive().CoinsDB().Cursor());
2138-
assert(pcursor);
2138+
CHECK_NONFATAL(pcursor);
21392139
tip = ::ChainActive().Tip();
2140-
assert(tip);
2140+
CHECK_NONFATAL(tip);
21412141
}
21422142
bool res = FindScriptPubKey(g_scan_progress, g_should_abort_scan, count, pcursor.get(), needles, coins);
21432143
result.pushKV("success", res);

‎src/rpc/mining.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)
512512
// Need to update only after we know CreateNewBlock succeeded
513513
pindexPrev = pindexPrevNew;
514514
}
515-
assert(pindexPrev);
515+
CHECK_NONFATAL(pindexPrev);
516516
CBlock* pblock = &pblocktemplate->block; // pointer for convenience
517517
const Consensus::Params& consensusParams = Params().GetConsensus();
518518

@@ -554,7 +554,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)
554554
entry.pushKV("fee", pblocktemplate->vTxFees[index_in_template]);
555555
int64_t nTxSigOps = pblocktemplate->vTxSigOpsCost[index_in_template];
556556
if (fPreSegWit) {
557-
assert(nTxSigOps % WITNESS_SCALE_FACTOR == 0);
557+
CHECK_NONFATAL(nTxSigOps % WITNESS_SCALE_FACTOR == 0);
558558
nTxSigOps /= WITNESS_SCALE_FACTOR;
559559
}
560560
entry.pushKV("sigops", nTxSigOps);
@@ -643,9 +643,9 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)
643643
int64_t nSigOpLimit = MAX_BLOCK_SIGOPS_COST;
644644
int64_t nSizeLimit = MAX_BLOCK_SERIALIZED_SIZE;
645645
if (fPreSegWit) {
646-
assert(nSigOpLimit % WITNESS_SCALE_FACTOR == 0);
646+
CHECK_NONFATAL(nSigOpLimit % WITNESS_SCALE_FACTOR == 0);
647647
nSigOpLimit /= WITNESS_SCALE_FACTOR;
648-
assert(nSizeLimit % WITNESS_SCALE_FACTOR == 0);
648+
CHECK_NONFATAL(nSizeLimit % WITNESS_SCALE_FACTOR == 0);
649649
nSizeLimit /= WITNESS_SCALE_FACTOR;
650650
}
651651
result.pushKV("sigoplimit", nSigOpLimit);

‎src/rpc/util.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,7 @@ RPCHelpMan::RPCHelpMan(std::string name, std::string description, std::vector<RP
428428
std::set<std::string> named_args;
429429
for (const auto& arg : m_args) {
430430
// Should have unique named arguments
431-
assert(named_args.insert(arg.m_name).second);
431+
CHECK_NONFATAL(named_args.insert(arg.m_name).second);
432432
}
433433
}
434434

@@ -620,11 +620,11 @@ std::string RPCArg::ToStringObj(const bool oneline) const
620620
case Type::OBJ:
621621
case Type::OBJ_USER_KEYS:
622622
// Currently unused, so avoid writing dead code
623-
assert(false);
623+
CHECK_NONFATAL(false);
624624

625625
// no default case, so the compiler can warn about missing cases
626626
}
627-
assert(false);
627+
CHECK_NONFATAL(false);
628628
}
629629

630630
std::string RPCArg::ToString(const bool oneline) const
@@ -661,7 +661,7 @@ std::string RPCArg::ToString(const bool oneline) const
661661

662662
// no default case, so the compiler can warn about missing cases
663663
}
664-
assert(false);
664+
CHECK_NONFATAL(false);
665665
}
666666

667667
static std::pair<int64_t, int64_t> ParseRange(const UniValue& value)

‎src/wallet/rpcdump.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ UniValue importprivkey(const JSONRPCRequest& request)
162162
if (!key.IsValid()) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding");
163163

164164
CPubKey pubkey = key.GetPubKey();
165-
assert(key.VerifyPubKey(pubkey));
165+
CHECK_NONFATAL(key.VerifyPubKey(pubkey));
166166
CKeyID vchAddress = pubkey.GetID();
167167
{
168168
pwallet->MarkDirty();
@@ -642,7 +642,7 @@ UniValue importwallet(const JSONRPCRequest& request)
642642
std::string label = std::get<3>(key_tuple);
643643

644644
CPubKey pubkey = key.GetPubKey();
645-
assert(key.VerifyPubKey(pubkey));
645+
CHECK_NONFATAL(key.VerifyPubKey(pubkey));
646646
CKeyID keyid = pubkey.GetID();
647647

648648
pwallet->WalletLogPrintf("Importing %s...\n", EncodeDestination(PKHash(keyid)));
@@ -915,7 +915,7 @@ static std::string RecurseImportData(const CScript& script, ImportData& import_d
915915
case TX_SCRIPTHASH: {
916916
if (script_ctx == ScriptContext::P2SH) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Trying to nest P2SH inside another P2SH");
917917
if (script_ctx == ScriptContext::WITNESS_V0) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Trying to nest P2SH inside a P2WSH");
918-
assert(script_ctx == ScriptContext::TOP);
918+
CHECK_NONFATAL(script_ctx == ScriptContext::TOP);
919919
CScriptID id = CScriptID(uint160(solverdata[0]));
920920
auto subscript = std::move(import_data.redeemscript); // Remove redeemscript from import_data to check for superfluous script later.
921921
if (!subscript) return "missing redeemscript";

‎src/wallet/rpcwallet.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ static void WalletTxToJSON(interfaces::Chain& chain, interfaces::Chain::Lock& lo
136136
entry.pushKV("blockindex", wtx.m_confirm.nIndex);
137137
int64_t block_time;
138138
bool found_block = chain.findBlock(wtx.m_confirm.hashBlock, nullptr /* block */, &block_time);
139-
assert(found_block);
139+
CHECK_NONFATAL(found_block);
140140
entry.pushKV("blocktime", block_time);
141141
} else {
142142
entry.pushKV("trusted", wtx.IsTrusted(locked_chain));
@@ -2943,7 +2943,7 @@ static UniValue listunspent(const JSONRPCRequest& request)
29432943
CTxDestination witness_destination;
29442944
if (redeemScript.IsPayToWitnessScriptHash()) {
29452945
bool extracted = ExtractDestination(redeemScript, witness_destination);
2946-
assert(extracted);
2946+
CHECK_NONFATAL(extracted);
29472947
// Also return the witness script
29482948
const WitnessV0ScriptHash& whash = boost::get<WitnessV0ScriptHash>(witness_destination);
29492949
CScriptID id;
@@ -3833,7 +3833,7 @@ static UniValue getaddressesbylabel(const JSONRPCRequest& request)
38333833
// address strings, but build a separate set as a precaution just in
38343834
// case it does.
38353835
bool unique = addresses.emplace(address).second;
3836-
assert(unique);
3836+
CHECK_NONFATAL(unique);
38373837
// UniValue::pushKV checks if the key exists in O(N)
38383838
// and since duplicate addresses are unexpected (checked with
38393839
// std::set in O(log(N))), UniValue::__pushKV is used instead,

‎test/lint/lint-assertions.sh‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,15 @@ if [[ ${OUTPUT} != "" ]]; then
2020
EXIT_CODE=1
2121
fi
2222

23+
# Macro CHECK_NONFATAL(condition) should be used instead of assert for RPC code, where it
24+
# is undesirable to crash the whole program. See: src/util/check.h
25+
# src/rpc/server.cpp is excluded from this check since it's mostly meta-code.
26+
OUTPUT=$(git grep -nE 'assert *\(.*\);' -- "src/rpc/" "src/wallet/rpc*" ":(exclude)src/rpc/server.cpp")
27+
if [[ ${OUTPUT} != "" ]]; then
28+
echo "CHECK_NONFATAL(condition) should be used instead of assert for RPC code."
29+
echo
30+
echo "${OUTPUT}"
31+
EXIT_CODE=1
32+
fi
33+
2334
exit ${EXIT_CODE}

0 commit comments

Comments
 (0)