Merge #19277: util: Add Assert identity function

fab80fef61 refactor: Remove unused EnsureChainman (MarcoFalke)
fa34587f1c scripted-diff: Replace EnsureChainman with Assert in unit tests (MarcoFalke)
fa6ef701ad util: Add Assert identity function (MarcoFalke)
fa457fbd33 move-only: Move NDEBUG compile time check to util/check (MarcoFalke)

Pull request description:

  The utility function is primarily useful to dereference pointer types, which are known to be not null at that time.

  For example, the ArgsManager is known to exist when the wallets are started: https://github.com/bitcoin/bitcoin/pull/18923/files#diff-fdb2a1a1d8bc790fcddeb6cf5a42ac55R503 . Instead of silently relying on that assumption, `Assert` can be used to abort the program and avoid UB should the assumption ever be violated.

ACKs for top commit:
  promag:
    Tested ACK fab80fef61.
  ryanofsky:
    Code review ACK fab80fef61

Tree-SHA512: 830fba10152ba17d47c4dd42809c7e26f9fe6d38e17a2d5b3f054fd644a5c4c9841286ac421ec9bb28cea9f5faeb659740fcf00de6cc589d423fee7694c42d16
This commit is contained in:
MarcoFalke
2020-07-04 08:42:57 -04:00
14 changed files with 46 additions and 39 deletions

View File

@@ -94,7 +94,7 @@ bool BuildChainTestingSetup::BuildChain(const CBlockIndex* pindex,
CBlockHeader header = block->GetBlockHeader();
BlockValidationState state;
if (!EnsureChainman(m_node).ProcessNewBlockHeaders({header}, state, Params(), &pindex)) {
if (!Assert(m_node.chainman)->ProcessNewBlockHeaders({header}, state, Params(), &pindex)) {
return false;
}
}
@@ -171,7 +171,7 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup)
uint256 chainA_last_header = last_header;
for (size_t i = 0; i < 2; i++) {
const auto& block = chainA[i];
BOOST_REQUIRE(EnsureChainman(m_node).ProcessNewBlock(Params(), block, true, nullptr));
BOOST_REQUIRE(Assert(m_node.chainman)->ProcessNewBlock(Params(), block, true, nullptr));
}
for (size_t i = 0; i < 2; i++) {
const auto& block = chainA[i];
@@ -189,7 +189,7 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup)
uint256 chainB_last_header = last_header;
for (size_t i = 0; i < 3; i++) {
const auto& block = chainB[i];
BOOST_REQUIRE(EnsureChainman(m_node).ProcessNewBlock(Params(), block, true, nullptr));
BOOST_REQUIRE(Assert(m_node.chainman)->ProcessNewBlock(Params(), block, true, nullptr));
}
for (size_t i = 0; i < 3; i++) {
const auto& block = chainB[i];
@@ -220,7 +220,7 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup)
// Reorg back to chain A.
for (size_t i = 2; i < 4; i++) {
const auto& block = chainA[i];
BOOST_REQUIRE(EnsureChainman(m_node).ProcessNewBlock(Params(), block, true, nullptr));
BOOST_REQUIRE(Assert(m_node.chainman)->ProcessNewBlock(Params(), block, true, nullptr));
}
// Check that chain A and B blocks can be retrieved.

View File

@@ -253,7 +253,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
pblock->nNonce = blockinfo[i].nonce;
}
std::shared_ptr<const CBlock> shared_pblock = std::make_shared<const CBlock>(*pblock);
BOOST_CHECK(EnsureChainman(m_node).ProcessNewBlock(chainparams, shared_pblock, true, nullptr));
BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlock(chainparams, shared_pblock, true, nullptr));
pblock->hashPrevBlock = pblock->GetHash();
}

View File

@@ -11,6 +11,7 @@
#include <node/context.h>
#include <pow.h>
#include <script/standard.h>
#include <util/check.h>
#include <validation.h>
CTxIn generatetoaddress(const NodeContext& node, const std::string& address)
@@ -31,7 +32,7 @@ CTxIn MineBlock(const NodeContext& node, const CScript& coinbase_scriptPubKey)
assert(block->nNonce);
}
bool processed{EnsureChainman(node).ProcessNewBlock(Params(), block, true, nullptr)};
bool processed{Assert(node.chainman)->ProcessNewBlock(Params(), block, true, nullptr)};
assert(processed);
return CTxIn{block->vtx[0]->GetHash(), 0};
@@ -39,9 +40,8 @@ CTxIn MineBlock(const NodeContext& node, const CScript& coinbase_scriptPubKey)
std::shared_ptr<CBlock> PrepareBlock(const NodeContext& node, const CScript& coinbase_scriptPubKey)
{
assert(node.mempool);
auto block = std::make_shared<CBlock>(
BlockAssembler{*node.mempool, Params()}
BlockAssembler{*Assert(node.mempool), Params()}
.CreateNewBlock(coinbase_scriptPubKey)
->block);

View File

@@ -231,7 +231,7 @@ CBlock TestChain100Setup::CreateAndProcessBlock(const std::vector<CMutableTransa
while (!CheckProofOfWork(block.GetHash(), block.nBits, chainparams.GetConsensus())) ++block.nNonce;
std::shared_ptr<const CBlock> shared_pblock = std::make_shared<const CBlock>(block);
EnsureChainman(m_node).ProcessNewBlock(chainparams, shared_pblock, true, nullptr);
Assert(m_node.chainman)->ProcessNewBlock(chainparams, shared_pblock, true, nullptr);
CBlock result = block;
return result;

View File

@@ -12,6 +12,7 @@
#include <pubkey.h>
#include <random.h>
#include <txmempool.h>
#include <util/check.h>
#include <util/string.h>
#include <type_traits>

View File

@@ -163,10 +163,10 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering)
std::transform(blocks.begin(), blocks.end(), std::back_inserter(headers), [](std::shared_ptr<const CBlock> b) { return b->GetBlockHeader(); });
// Process all the headers so we understand the toplogy of the chain
BOOST_CHECK(EnsureChainman(m_node).ProcessNewBlockHeaders(headers, state, Params()));
BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlockHeaders(headers, state, Params()));
// Connect the genesis block and drain any outstanding events
BOOST_CHECK(EnsureChainman(m_node).ProcessNewBlock(Params(), std::make_shared<CBlock>(Params().GenesisBlock()), true, &ignored));
BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlock(Params(), std::make_shared<CBlock>(Params().GenesisBlock()), true, &ignored));
SyncWithValidationInterfaceQueue();
// subscribe to events (this subscriber will validate event ordering)
@@ -188,13 +188,13 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering)
FastRandomContext insecure;
for (int i = 0; i < 1000; i++) {
auto block = blocks[insecure.randrange(blocks.size() - 1)];
EnsureChainman(m_node).ProcessNewBlock(Params(), block, true, &ignored);
Assert(m_node.chainman)->ProcessNewBlock(Params(), block, true, &ignored);
}
// to make sure that eventually we process the full chain - do it here
for (auto block : blocks) {
if (block->vtx.size() == 1) {
bool processed = EnsureChainman(m_node).ProcessNewBlock(Params(), block, true, &ignored);
bool processed = Assert(m_node.chainman)->ProcessNewBlock(Params(), block, true, &ignored);
assert(processed);
}
}
@@ -233,7 +233,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg)
{
bool ignored;
auto ProcessBlock = [&](std::shared_ptr<const CBlock> block) -> bool {
return EnsureChainman(m_node).ProcessNewBlock(Params(), block, /* fForceProcessing */ true, /* fNewBlock */ &ignored);
return Assert(m_node.chainman)->ProcessNewBlock(Params(), block, /* fForceProcessing */ true, /* fNewBlock */ &ignored);
};
// Process all mined blocks