foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-08 10:31:50 -05:00
Code Issues Activity

Merge #17318: replace asserts in RPC code with CHECK_NONFATAL and add linter

Browse source 
c98bd13e67 replace asserts in RPC code with CHECK_NONFATAL and add linter (Adam Jonas)

Pull request description:

  - Replace instances of assert in /rpc files and rpcwallet with CHECK_NONFATAL(condition)
  - Add a linter to prevent future usage of assert being used in RPC code

  ref https://github.com/bitcoin/bitcoin/pull/17192

ACKs for top commit:
  practicalswift:
    ACK c98bd13e67 -- diff looks correct

Tree-SHA512: a16036b6bbcca73a5334665f66e17e1756377d582317568291da1d727fc9cf8c84bac9d9bd099534e1be315345336e5f7b66b93793135155f320dc5862a2d875
This commit is contained in:
MarcoFalke 2019-11-04 11:33:36 -05:00
commit 94a26b192f
No known key found for this signature in database
GPG key ID: D2EA4850E7528B25
6 changed files with 34 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

18
src/rpc/blockchain.cpp
View file

@ -58,7 +58,7 @@ static CUpdatedBlock latestblock;
*/
double GetDifficulty(const CBlockIndex* blockindex)
{
assert(blockindex);
CHECK_NONFATAL(blockindex);
int nShift = (blockindex->nBits >> 24) & 0xff;
double dDiff =
@ -957,7 +957,7 @@ static UniValue pruneblockchain(const JSONRPCRequest& request)
PruneBlockFilesManual(height);
const CBlockIndex* block = ::ChainActive().Tip();
assert(block);
CHECK_NONFATAL(block);
while (block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA)) {
block = block->pprev;
}
@ -1252,7 +1252,7 @@ UniValue getblockchaininfo(const JSONRPCRequest& request)
obj.pushKV("pruned", fPruneMode);
if (fPruneMode) {
const CBlockIndex* block = tip;
assert(block);
CHECK_NONFATAL(block);
while (block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA)) {
block = block->pprev;
}
@ -1598,7 +1598,7 @@ static UniValue getchaintxstats(const JSONRPCRequest& request)
}
}
assert(pindex != nullptr);
CHECK_NONFATAL(pindex != nullptr);
if (request.params[0].isNull()) {
blockcount = std::max(0, std::min(blockcount, pindex->nHeight - 1));
@ -1771,7 +1771,7 @@ static UniValue getblockstats(const JSONRPCRequest& request)
}
}
assert(pindex != nullptr);
CHECK_NONFATAL(pindex != nullptr);
std::set<std::string> stats;
if (!request.params[1].isNull()) {
@ -1871,7 +1871,7 @@ static UniValue getblockstats(const JSONRPCRequest& request)
}
CAmount txfee = tx_total_in - tx_total_out;
assert(MoneyRange(txfee));
CHECK_NONFATAL(MoneyRange(txfee));
if (do_medianfee) {
fee_array.push_back(txfee);
}
@ -2008,7 +2008,7 @@ public:
explicit CoinsViewScanReserver() : m_could_reserve(false) {}
bool reserve() {
assert (!m_could_reserve);
CHECK_NONFATAL(!m_could_reserve);
std::lock_guard<std::mutex> lock(g_utxosetscan);
if (g_scan_in_progress) {
return false;
@ -2135,9 +2135,9 @@ UniValue scantxoutset(const JSONRPCRequest& request)
LOCK(cs_main);
::ChainstateActive().ForceFlushStateToDisk();
pcursor = std::unique_ptr<CCoinsViewCursor>(::ChainstateActive().CoinsDB().Cursor());
assert(pcursor);
CHECK_NONFATAL(pcursor);
tip = ::ChainActive().Tip();
assert(tip);
CHECK_NONFATAL(tip);
}
bool res = FindScriptPubKey(g_scan_progress, g_should_abort_scan, count, pcursor.get(), needles, coins);
result.pushKV("success", res);

8
src/rpc/mining.cpp
View file

@ -555,7 +555,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)
// Need to update only after we know CreateNewBlock succeeded
pindexPrev = pindexPrevNew;
}
assert(pindexPrev);
CHECK_NONFATAL(pindexPrev);
CBlock* pblock = &pblocktemplate->block; // pointer for convenience
const Consensus::Params& consensusParams = Params().GetConsensus();
@ -597,7 +597,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)
entry.pushKV("fee", pblocktemplate->vTxFees[index_in_template]);
int64_t nTxSigOps = pblocktemplate->vTxSigOpsCost[index_in_template];
if (fPreSegWit) {
assert(nTxSigOps % WITNESS_SCALE_FACTOR == 0);
CHECK_NONFATAL(nTxSigOps % WITNESS_SCALE_FACTOR == 0);
nTxSigOps /= WITNESS_SCALE_FACTOR;
}
entry.pushKV("sigops", nTxSigOps);
@ -686,9 +686,9 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)
int64_t nSigOpLimit = MAX_BLOCK_SIGOPS_COST;
int64_t nSizeLimit = MAX_BLOCK_SERIALIZED_SIZE;
if (fPreSegWit) {
assert(nSigOpLimit % WITNESS_SCALE_FACTOR == 0);
CHECK_NONFATAL(nSigOpLimit % WITNESS_SCALE_FACTOR == 0);
nSigOpLimit /= WITNESS_SCALE_FACTOR;
assert(nSizeLimit % WITNESS_SCALE_FACTOR == 0);
CHECK_NONFATAL(nSizeLimit % WITNESS_SCALE_FACTOR == 0);
nSizeLimit /= WITNESS_SCALE_FACTOR;
}
result.pushKV("sigoplimit", nSigOpLimit);

8
src/rpc/util.cpp
View file

@ -428,7 +428,7 @@ RPCHelpMan::RPCHelpMan(std::string name, std::string description, std::vector<RP
std::set<std::string> named_args;
for (const auto& arg : m_args) {
// Should have unique named arguments
assert(named_args.insert(arg.m_name).second);
CHECK_NONFATAL(named_args.insert(arg.m_name).second);
}
}
@ -620,11 +620,11 @@ std::string RPCArg::ToStringObj(const bool oneline) const
case Type::OBJ:
case Type::OBJ_USER_KEYS:
// Currently unused, so avoid writing dead code
assert(false);
CHECK_NONFATAL(false);
// no default case, so the compiler can warn about missing cases
}
assert(false);
CHECK_NONFATAL(false);
}
std::string RPCArg::ToString(const bool oneline) const
@ -661,7 +661,7 @@ std::string RPCArg::ToString(const bool oneline) const
// no default case, so the compiler can warn about missing cases
}
assert(false);
CHECK_NONFATAL(false);
}
static std::pair<int64_t, int64_t> ParseRange(const UniValue& value)

6
src/wallet/rpcdump.cpp
View file

@ -168,7 +168,7 @@ UniValue importprivkey(const JSONRPCRequest& request)
if (!key.IsValid()) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding");
CPubKey pubkey = key.GetPubKey();
assert(key.VerifyPubKey(pubkey));
CHECK_NONFATAL(key.VerifyPubKey(pubkey));
CKeyID vchAddress = pubkey.GetID();
{
pwallet->MarkDirty();
@ -639,7 +639,7 @@ UniValue importwallet(const JSONRPCRequest& request)
std::string label = std::get<3>(key_tuple);
CPubKey pubkey = key.GetPubKey();
assert(key.VerifyPubKey(pubkey));
CHECK_NONFATAL(key.VerifyPubKey(pubkey));
CKeyID keyid = pubkey.GetID();
pwallet->WalletLogPrintf("Importing %s...\n", EncodeDestination(PKHash(keyid)));
@ -906,7 +906,7 @@ static std::string RecurseImportData(const CScript& script, ImportData& import_d
case TX_SCRIPTHASH: {
if (script_ctx == ScriptContext::P2SH) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Trying to nest P2SH inside another P2SH");
if (script_ctx == ScriptContext::WITNESS_V0) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Trying to nest P2SH inside a P2WSH");
assert(script_ctx == ScriptContext::TOP);
CHECK_NONFATAL(script_ctx == ScriptContext::TOP);
CScriptID id = CScriptID(uint160(solverdata[0]));
auto subscript = std::move(import_data.redeemscript); // Remove redeemscript from import_data to check for superfluous script later.
if (!subscript) return "missing redeemscript";

6
src/wallet/rpcwallet.cpp
View file

@ -136,7 +136,7 @@ static void WalletTxToJSON(interfaces::Chain& chain, interfaces::Chain::Lock& lo
entry.pushKV("blockindex", wtx.m_confirm.nIndex);
int64_t block_time;
bool found_block = chain.findBlock(wtx.m_confirm.hashBlock, nullptr /* block */, &block_time);
assert(found_block);
CHECK_NONFATAL(found_block);
entry.pushKV("blocktime", block_time);
} else {
entry.pushKV("trusted", wtx.IsTrusted(locked_chain));
@ -2943,7 +2943,7 @@ static UniValue listunspent(const JSONRPCRequest& request)
CTxDestination witness_destination;
if (redeemScript.IsPayToWitnessScriptHash()) {
bool extracted = ExtractDestination(redeemScript, witness_destination);
assert(extracted);
CHECK_NONFATAL(extracted);
// Also return the witness script
const WitnessV0ScriptHash& whash = boost::get<WitnessV0ScriptHash>(witness_destination);
CScriptID id;
@ -3831,7 +3831,7 @@ static UniValue getaddressesbylabel(const JSONRPCRequest& request)
// address strings, but build a separate set as a precaution just in
// case it does.
bool unique = addresses.emplace(address).second;
assert(unique);
CHECK_NONFATAL(unique);
// 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 instead,

11
test/lint/lint-assertions.sh
View file

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