From 38860f93b680f152fc6fc3d9ae574a4c0659e775 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Tue, 18 Jan 2022 21:49:30 +1000 Subject: validation: remove redundant CChainParams params from ChainstateManager methods --- src/bitcoin-chainstate.cpp | 2 +- src/net_processing.cpp | 6 +++--- src/rpc/mining.cpp | 6 +++--- src/test/blockfilter_index_tests.cpp | 8 ++++---- src/test/fuzz/utxo_snapshot.cpp | 2 +- src/test/miner_tests.cpp | 2 +- src/test/util/mining.cpp | 2 +- src/test/util/setup_common.cpp | 3 +-- src/test/validation_block_tests.cpp | 10 +++++----- src/validation.cpp | 20 ++++++++++---------- src/validation.h | 7 ++----- 11 files changed, 32 insertions(+), 36 deletions(-) diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 2ba8aaa2c9..5e5272228b 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -190,7 +190,7 @@ int main(int argc, char* argv[]) bool new_block; auto sc = std::make_shared(block.GetHash()); RegisterSharedValidationInterface(sc); - bool accepted = chainman.ProcessNewBlock(chainparams, blockptr, /*force_processing=*/true, /*new_block=*/&new_block); + bool accepted = chainman.ProcessNewBlock(blockptr, /*force_processing=*/true, /*new_block=*/&new_block); UnregisterSharedValidationInterface(sc); if (!new_block && accepted) { std::cerr << "duplicate" << std::endl; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 478368b673..55ff2a4837 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2214,7 +2214,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, const Peer& peer, } BlockValidationState state; - if (!m_chainman.ProcessNewBlockHeaders(headers, state, m_chainparams, &pindexLast)) { + if (!m_chainman.ProcessNewBlockHeaders(headers, state, &pindexLast)) { if (state.IsInvalid()) { MaybePunishNodeForBlock(pfrom.GetId(), state, via_compact_block, "invalid header received"); return; @@ -2591,7 +2591,7 @@ void PeerManagerImpl::ProcessGetCFCheckPt(CNode& peer, CDataStream& vRecv) void PeerManagerImpl::ProcessBlock(CNode& node, const std::shared_ptr& block, bool force_processing) { bool new_block{false}; - m_chainman.ProcessNewBlock(m_chainparams, block, force_processing, &new_block); + m_chainman.ProcessNewBlock(block, force_processing, &new_block); if (new_block) { node.m_last_block_time = GetTime(); } else { @@ -3569,7 +3569,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, const CBlockIndex *pindex = nullptr; BlockValidationState state; - if (!m_chainman.ProcessNewBlockHeaders({cmpctblock.header}, state, m_chainparams, &pindex)) { + if (!m_chainman.ProcessNewBlockHeaders({cmpctblock.header}, state, &pindex)) { if (state.IsInvalid()) { MaybePunishNodeForBlock(pfrom.GetId(), state, /*via_compact_block=*/true, "invalid header via cmpctblock"); return; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index b552528951..2585c28fb8 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -133,7 +133,7 @@ static bool GenerateBlock(ChainstateManager& chainman, CBlock& block, uint64_t& } std::shared_ptr shared_pblock = std::make_shared(block); - if (!chainman.ProcessNewBlock(chainparams, shared_pblock, true, nullptr)) { + if (!chainman.ProcessNewBlock(shared_pblock, true, nullptr)) { throw JSONRPCError(RPC_INTERNAL_ERROR, "ProcessNewBlock, block not accepted"); } @@ -1000,7 +1000,7 @@ static RPCHelpMan submitblock() bool new_block; auto sc = std::make_shared(block.GetHash()); RegisterSharedValidationInterface(sc); - bool accepted = chainman.ProcessNewBlock(Params(), blockptr, /*force_processing=*/true, /*new_block=*/&new_block); + bool accepted = chainman.ProcessNewBlock(blockptr, /*force_processing=*/true, /*new_block=*/&new_block); UnregisterSharedValidationInterface(sc); if (!new_block && accepted) { return "duplicate"; @@ -1042,7 +1042,7 @@ static RPCHelpMan submitheader() } BlockValidationState state; - chainman.ProcessNewBlockHeaders({h}, state, Params()); + chainman.ProcessNewBlockHeaders({h}, state); if (state.IsValid()) return NullUniValue; if (state.IsError()) { throw JSONRPCError(RPC_VERIFY_ERROR, state.ToString()); diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index 82b9617384..ffb9fb55d9 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -101,7 +101,7 @@ bool BuildChainTestingSetup::BuildChain(const CBlockIndex* pindex, CBlockHeader header = block->GetBlockHeader(); BlockValidationState state; - if (!Assert(m_node.chainman)->ProcessNewBlockHeaders({header}, state, Params(), &pindex)) { + if (!Assert(m_node.chainman)->ProcessNewBlockHeaders({header}, state, &pindex)) { return false; } } @@ -178,7 +178,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(Assert(m_node.chainman)->ProcessNewBlock(Params(), block, true, nullptr)); + BOOST_REQUIRE(Assert(m_node.chainman)->ProcessNewBlock(block, true, nullptr)); } for (size_t i = 0; i < 2; i++) { const auto& block = chainA[i]; @@ -196,7 +196,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(Assert(m_node.chainman)->ProcessNewBlock(Params(), block, true, nullptr)); + BOOST_REQUIRE(Assert(m_node.chainman)->ProcessNewBlock(block, true, nullptr)); } for (size_t i = 0; i < 3; i++) { const auto& block = chainB[i]; @@ -227,7 +227,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(Assert(m_node.chainman)->ProcessNewBlock(Params(), block, true, nullptr)); + BOOST_REQUIRE(Assert(m_node.chainman)->ProcessNewBlock(block, true, nullptr)); } // Check that chain A and B blocks can be retrieved. diff --git a/src/test/fuzz/utxo_snapshot.cpp b/src/test/fuzz/utxo_snapshot.cpp index e513f1883c..33496a457e 100644 --- a/src/test/fuzz/utxo_snapshot.cpp +++ b/src/test/fuzz/utxo_snapshot.cpp @@ -58,7 +58,7 @@ FUZZ_TARGET_INIT(utxo_snapshot, initialize_chain) if (fuzzed_data_provider.ConsumeBool()) { for (const auto& block : *g_chain) { BlockValidationState dummy; - bool processed{chainman.ProcessNewBlockHeaders({*block}, dummy, ::Params())}; + bool processed{chainman.ProcessNewBlockHeaders({*block}, dummy)}; Assert(processed); const auto* index{WITH_LOCK(::cs_main, return chainman.m_blockman.LookupBlockIndex(block->GetHash()))}; Assert(index); diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index cafa5710fa..0c3a13c5f8 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -587,7 +587,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) pblock->nNonce = bi.nonce; } std::shared_ptr shared_pblock = std::make_shared(*pblock); - BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlock(chainparams, shared_pblock, true, nullptr)); + BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlock(shared_pblock, true, nullptr)); pblock->hashPrevBlock = pblock->GetHash(); } diff --git a/src/test/util/mining.cpp b/src/test/util/mining.cpp index 5ed8598e8e..52e02c4fa4 100644 --- a/src/test/util/mining.cpp +++ b/src/test/util/mining.cpp @@ -68,7 +68,7 @@ CTxIn MineBlock(const NodeContext& node, const CScript& coinbase_scriptPubKey) assert(block->nNonce); } - bool processed{Assert(node.chainman)->ProcessNewBlock(Params(), block, true, nullptr)}; + bool processed{Assert(node.chainman)->ProcessNewBlock(block, true, nullptr)}; assert(processed); return CTxIn{block->vtx[0]->GetHash(), 0}; diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index aa7b73e6ac..6891629f8e 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -300,10 +300,9 @@ CBlock TestChain100Setup::CreateAndProcessBlock( chainstate = &Assert(m_node.chainman)->ActiveChainstate(); } - const CChainParams& chainparams = Params(); const CBlock block = this->CreateBlock(txns, scriptPubKey, *chainstate); std::shared_ptr shared_pblock = std::make_shared(block); - Assert(m_node.chainman)->ProcessNewBlock(chainparams, shared_pblock, true, nullptr); + Assert(m_node.chainman)->ProcessNewBlock(shared_pblock, true, nullptr); return block; } diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index c5b1dabcb7..f6cf7e7331 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -100,7 +100,7 @@ std::shared_ptr MinerTestingSetup::FinalizeBlock(std::shared_ptr // submit block header, so that miner can get the block height from the // global state and the node has the topology of the chain BlockValidationState ignored; - BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlockHeaders({pblock->GetBlockHeader()}, ignored, Params())); + BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlockHeaders({pblock->GetBlockHeader()}, ignored)); return pblock; } @@ -157,7 +157,7 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering) bool ignored; // Connect the genesis block and drain any outstanding events - BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlock(Params(), std::make_shared(Params().GenesisBlock()), true, &ignored)); + BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlock(std::make_shared(Params().GenesisBlock()), true, &ignored)); SyncWithValidationInterfaceQueue(); // subscribe to events (this subscriber will validate event ordering) @@ -179,13 +179,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)]; - Assert(m_node.chainman)->ProcessNewBlock(Params(), block, true, &ignored); + Assert(m_node.chainman)->ProcessNewBlock(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 = Assert(m_node.chainman)->ProcessNewBlock(Params(), block, true, &ignored); + bool processed = Assert(m_node.chainman)->ProcessNewBlock(block, true, &ignored); assert(processed); } } @@ -224,7 +224,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg) { bool ignored; auto ProcessBlock = [&](std::shared_ptr block) -> bool { - return Assert(m_node.chainman)->ProcessNewBlock(Params(), block, /*force_processing=*/true, /*new_block=*/&ignored); + return Assert(m_node.chainman)->ProcessNewBlock(block, /*force_processing=*/true, /*new_block=*/&ignored); }; // Process all mined blocks diff --git a/src/validation.cpp b/src/validation.cpp index b5d6a66088..fb05b9c18e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3576,13 +3576,13 @@ static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& stat return true; } -bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex) +bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationState& state, CBlockIndex** ppindex) { AssertLockHeld(cs_main); // Check for duplicate uint256 hash = block.GetHash(); BlockMap::iterator miSelf{m_blockman.m_block_index.find(hash)}; - if (hash != chainparams.GetConsensus().hashGenesisBlock) { + if (hash != GetConsensus().hashGenesisBlock) { if (miSelf != m_blockman.m_block_index.end()) { // Block header is already known. CBlockIndex* pindex = &(miSelf->second); @@ -3595,7 +3595,7 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida return true; } - if (!CheckBlockHeader(block, state, chainparams.GetConsensus())) { + if (!CheckBlockHeader(block, state, GetConsensus())) { LogPrint(BCLog::VALIDATION, "%s: Consensus::CheckBlockHeader: %s, %s\n", __func__, hash.ToString(), state.ToString()); return false; } @@ -3612,7 +3612,7 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida LogPrint(BCLog::VALIDATION, "%s: %s prev block invalid\n", __func__, hash.ToString()); return state.Invalid(BlockValidationResult::BLOCK_INVALID_PREV, "bad-prevblk"); } - if (!ContextualCheckBlockHeader(block, state, m_blockman, chainparams, pindexPrev, GetAdjustedTime())) { + if (!ContextualCheckBlockHeader(block, state, m_blockman, GetParams(), pindexPrev, GetAdjustedTime())) { LogPrint(BCLog::VALIDATION, "%s: Consensus::ContextualCheckBlockHeader: %s, %s\n", __func__, hash.ToString(), state.ToString()); return false; } @@ -3665,14 +3665,14 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida } // Exposed wrapper for AcceptBlockHeader -bool ChainstateManager::ProcessNewBlockHeaders(const std::vector& headers, BlockValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex) +bool ChainstateManager::ProcessNewBlockHeaders(const std::vector& headers, BlockValidationState& state, const CBlockIndex** ppindex) { AssertLockNotHeld(cs_main); { LOCK(cs_main); for (const CBlockHeader& header : headers) { CBlockIndex *pindex = nullptr; // Use a temp pindex instead of ppindex to avoid a const_cast - bool accepted{AcceptBlockHeader(header, state, chainparams, &pindex)}; + bool accepted{AcceptBlockHeader(header, state, &pindex)}; ActiveChainstate().CheckBlockIndex(); if (!accepted) { @@ -3686,7 +3686,7 @@ bool ChainstateManager::ProcessNewBlockHeaders(const std::vector& if (NotifyHeaderTip(ActiveChainstate())) { if (ActiveChainstate().IsInitialBlockDownload() && ppindex && *ppindex) { const CBlockIndex& last_accepted{**ppindex}; - const int64_t blocks_left{(GetTime() - last_accepted.GetBlockTime()) / chainparams.GetConsensus().nPowTargetSpacing}; + const int64_t blocks_left{(GetTime() - last_accepted.GetBlockTime()) / GetConsensus().nPowTargetSpacing}; const double progress{100.0 * last_accepted.nHeight / (last_accepted.nHeight + blocks_left)}; LogPrintf("Synchronizing blockheaders, height: %d (~%.2f%%)\n", last_accepted.nHeight, progress); } @@ -3705,7 +3705,7 @@ bool CChainState::AcceptBlock(const std::shared_ptr& pblock, Block CBlockIndex *pindexDummy = nullptr; CBlockIndex *&pindex = ppindex ? *ppindex : pindexDummy; - bool accepted_header{m_chainman.AcceptBlockHeader(block, state, m_params, &pindex)}; + bool accepted_header{m_chainman.AcceptBlockHeader(block, state, &pindex)}; CheckBlockIndex(); if (!accepted_header) @@ -3778,7 +3778,7 @@ bool CChainState::AcceptBlock(const std::shared_ptr& pblock, Block return true; } -bool ChainstateManager::ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr& block, bool force_processing, bool* new_block) +bool ChainstateManager::ProcessNewBlock(const std::shared_ptr& block, bool force_processing, bool* new_block) { AssertLockNotHeld(cs_main); @@ -3796,7 +3796,7 @@ bool ChainstateManager::ProcessNewBlock(const CChainParams& chainparams, const s // malleability that cause CheckBlock() to fail; see e.g. CVE-2012-2459 and // https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2019-February/016697.html. Because CheckBlock() is // not very expensive, the anti-DoS benefits of caching failure (of a definitely-invalid block) are not substantial. - bool ret = CheckBlock(*block, state, chainparams.GetConsensus()); + bool ret = CheckBlock(*block, state, GetConsensus()); if (ret) { // Store to disk ret = ActiveChainstate().AcceptBlock(block, state, &pindex, force_processing, nullptr, new_block); diff --git a/src/validation.h b/src/validation.h index 1852ae6a54..d23f0a3716 100644 --- a/src/validation.h +++ b/src/validation.h @@ -41,7 +41,6 @@ class CChainState; class CBlockTreeDB; -class CChainParams; class CTxMemPool; class ChainstateManager; struct ChainTxData; @@ -853,7 +852,6 @@ private: bool AcceptBlockHeader( const CBlockHeader& block, BlockValidationState& state, - const CChainParams& chainparams, CBlockIndex** ppindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); friend CChainState; @@ -971,7 +969,7 @@ public: * @param[out] new_block A boolean which is set to indicate if the block was first received via this call * @returns If the block was processed, independently of block validity */ - bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr& block, bool force_processing, bool* new_block) LOCKS_EXCLUDED(cs_main); + bool ProcessNewBlock(const std::shared_ptr& block, bool force_processing, bool* new_block) LOCKS_EXCLUDED(cs_main); /** * Process incoming block headers. @@ -981,10 +979,9 @@ public: * * @param[in] block The block headers themselves * @param[out] state This may be set to an Error state if any error occurred processing them - * @param[in] chainparams The params for the chain we want to connect to * @param[out] ppindex If set, the pointer will be set to point to the last new block index object for the given headers */ - bool ProcessNewBlockHeaders(const std::vector& block, BlockValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex = nullptr) LOCKS_EXCLUDED(cs_main); + bool ProcessNewBlockHeaders(const std::vector& block, BlockValidationState& state, const CBlockIndex** ppindex = nullptr) LOCKS_EXCLUDED(cs_main); /** * Try to add a transaction to the memory pool. -- cgit v1.2.3