Skip to content

Commit

Permalink
Merge bitcoin#30429: rpc: Use CHECK_NONFATAL over Assert
Browse files Browse the repository at this point in the history
fa62707 rpc: Use CHECK_NONFATAL over Assert (MarcoFalke)

Pull request description:

  Any RPC method should not abort the whole node when an internal logic error happens.

  Fix it by just aborting this single RPC method call when an error happens.

  Also, fix the linter to find the fixed cases.

ACKs for top commit:
  achow101:
    ACK fa62707
  stickies-v:
    ACK fa62707
  tdb3:
    ACK fa62707
  hodlinator:
    ACK fa62707

Tree-SHA512: dad2f31b01a66578949009499e4385fb4d72f0f897419f2a6e0ea02e799b9a31e6ecb5a67fa5d27fcbc7939fe8acd62dc04e877b35831493b7f2c604dec7dc64
  • Loading branch information
achow101 committed Jul 16, 2024
2 parents 1d24d38 + fa62707 commit ad5579e
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 6 deletions.
4 changes: 2 additions & 2 deletions src/rpc/blockchain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -801,14 +801,14 @@ std::optional<int> GetPruneHeight(const BlockManager& blockman, const CChain& ch
// If the chain tip is pruned, everything is pruned.
if (!((chain_tip->nStatus & BLOCK_HAVE_MASK) == BLOCK_HAVE_MASK)) return chain_tip->nHeight;

const auto& first_unpruned{*Assert(blockman.GetFirstBlock(*chain_tip, /*status_mask=*/BLOCK_HAVE_MASK, first_block))};
const auto& first_unpruned{*CHECK_NONFATAL(blockman.GetFirstBlock(*chain_tip, /*status_mask=*/BLOCK_HAVE_MASK, first_block))};
if (&first_unpruned == first_block) {
// All blocks between first_block and chain_tip have data, so nothing is pruned.
return std::nullopt;
}

// Block before the first unpruned block is the last pruned block.
return Assert(first_unpruned.pprev)->nHeight;
return CHECK_NONFATAL(first_unpruned.pprev)->nHeight;
}

static RPCHelpMan pruneblockchain()
Expand Down
10 changes: 6 additions & 4 deletions test/lint/lint-assertions.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ def main():
# checks should be used over assert. See: src/util/check.h
# src/rpc/server.cpp is excluded from this check since it's mostly meta-code.
exit_code = git_grep([
"-nE",
r"\<(A|a)ss(ume|ert) *\(.*\);",
"--line-number",
"--extended-regexp",
r"\<(A|a)ss(ume|ert)\(",
"--",
"src/rpc/",
"src/wallet/rpc*",
Expand All @@ -38,8 +39,9 @@ def main():
# The `BOOST_ASSERT` macro requires to `#include boost/assert.hpp`,
# which is an unnecessary Boost dependency.
exit_code |= git_grep([
"-E",
r"BOOST_ASSERT *\(.*\);",
"--line-number",
"--extended-regexp",
r"BOOST_ASSERT\(",
"--",
"*.cpp",
"*.h",
Expand Down

0 comments on commit ad5579e

Please sign in to comment.