Merge bitcoin/bitcoin#25168: refactor: Avoid passing params where not needed

fa1b76aeb0 Do not call global Params() when chainman is in scope (MacroFake)
fa30234be8 Do not pass CChainParams& to PeerManager::make (MacroFake)
fafe5c0ca2 Do not pass CChainParams& to BlockAssembler constructor (MacroFake)
faf012b438 Do not pass Consensus::Params& to Chainstate helpers (MacroFake)
fa4ee53dca Do not pass time getter to Chainstate helpers (MacroFake)

Pull request description:

  It seems confusing to pass chain params, consensus params, or a time function around when it is not needed.

  Fix this by:

  * Inlining the passed time getter function. I don't see a use case why this should be mockable.
  * Using `chainman.GetConsensus()` or `chainman.GetParams()`, where possible.

ACKs for top commit:
  promag:
    Code review ACK fa1b76aeb0.
  vincenzopalazzo:
    ACK fa1b76aeb0

Tree-SHA512: 1abff5cba4b4871d97f17dbcdf67bc9255ff21fa4150a79a74e39b28f0610eab3e7dee24d56872dd6e111f003b55e288958cdd467e6218368d896f191e4ec9cd
This commit is contained in:
MacroFake
2022-05-20 13:35:09 +01:00
21 changed files with 66 additions and 92 deletions

View File

@@ -119,9 +119,7 @@ static bool GenerateBlock(ChainstateManager& chainman, CBlock& block, uint64_t&
block_hash.SetNull();
block.hashMerkleRoot = BlockMerkleRoot(block);
CChainParams chainparams(Params());
while (max_tries > 0 && block.nNonce < std::numeric_limits<uint32_t>::max() && !CheckProofOfWork(block.GetHash(), block.nBits, chainparams.GetConsensus()) && !ShutdownRequested()) {
while (max_tries > 0 && block.nNonce < std::numeric_limits<uint32_t>::max() && !CheckProofOfWork(block.GetHash(), block.nBits, chainman.GetConsensus()) && !ShutdownRequested()) {
++block.nNonce;
--max_tries;
}
@@ -145,7 +143,7 @@ static UniValue generateBlocks(ChainstateManager& chainman, const CTxMemPool& me
{
UniValue blockHashes(UniValue::VARR);
while (nGenerate > 0 && !ShutdownRequested()) {
std::unique_ptr<CBlockTemplate> pblocktemplate(BlockAssembler(chainman.ActiveChainstate(), mempool, Params()).CreateNewBlock(coinbase_script));
std::unique_ptr<CBlockTemplate> pblocktemplate(BlockAssembler{chainman.ActiveChainstate(), mempool}.CreateNewBlock(coinbase_script));
if (!pblocktemplate.get())
throw JSONRPCError(RPC_INTERNAL_ERROR, "Couldn't create new block");
CBlock *pblock = &pblocktemplate->block;
@@ -349,7 +347,6 @@ static RPCHelpMan generateblock()
}
}
CChainParams chainparams(Params());
CBlock block;
ChainstateManager& chainman = EnsureChainman(node);
@@ -357,7 +354,7 @@ static RPCHelpMan generateblock()
LOCK(cs_main);
CTxMemPool empty_mempool;
std::unique_ptr<CBlockTemplate> blocktemplate(BlockAssembler(chainman.ActiveChainstate(), empty_mempool, chainparams).CreateNewBlock(coinbase_script));
std::unique_ptr<CBlockTemplate> blocktemplate(BlockAssembler{chainman.ActiveChainstate(), empty_mempool}.CreateNewBlock(coinbase_script));
if (!blocktemplate) {
throw JSONRPCError(RPC_INTERNAL_ERROR, "Couldn't create new block");
}
@@ -374,7 +371,7 @@ static RPCHelpMan generateblock()
LOCK(cs_main);
BlockValidationState state;
if (!TestBlockValidity(state, chainparams, chainman.ActiveChainstate(), block, chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock), false, false)) {
if (!TestBlockValidity(state, chainman.GetParams(), chainman.ActiveChainstate(), block, chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock), false, false)) {
throw JSONRPCError(RPC_VERIFY_ERROR, strprintf("TestBlockValidity failed: %s", state.ToString()));
}
}
@@ -429,7 +426,7 @@ static RPCHelpMan getmininginfo()
obj.pushKV("difficulty", (double)GetDifficulty(active_chain.Tip()));
obj.pushKV("networkhashps", getnetworkhashps().HandleRequest(request));
obj.pushKV("pooledtx", (uint64_t)mempool.size());
obj.pushKV("chain", Params().NetworkIDString());
obj.pushKV("chain", chainman.GetParams().NetworkIDString());
obj.pushKV("warnings", GetWarnings(false).original);
return obj;
},
@@ -643,7 +640,7 @@ static RPCHelpMan getblocktemplate()
if (block.hashPrevBlock != pindexPrev->GetBlockHash())
return "inconclusive-not-best-prevblk";
BlockValidationState state;
TestBlockValidity(state, Params(), active_chainstate, block, pindexPrev, false, true);
TestBlockValidity(state, chainman.GetParams(), active_chainstate, block, pindexPrev, false, true);
return BIP22ValidationResult(state);
}
@@ -665,7 +662,7 @@ static RPCHelpMan getblocktemplate()
if (strMode != "template")
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid mode");
if (!Params().IsTestChain()) {
if (!chainman.GetParams().IsTestChain()) {
const CConnman& connman = EnsureConnman(node);
if (connman.GetNodeCount(ConnectionDirection::Both) == 0) {
throw JSONRPCError(RPC_CLIENT_NOT_CONNECTED, PACKAGE_NAME " is not connected!");
@@ -726,7 +723,7 @@ static RPCHelpMan getblocktemplate()
// TODO: Maybe recheck connections/IBD and (if something wrong) send an expires-immediately template to stop miners?
}
const Consensus::Params& consensusParams = Params().GetConsensus();
const Consensus::Params& consensusParams = chainman.GetParams().GetConsensus();
// GBT must be called with 'signet' set in the rules for signet chains
if (consensusParams.signet_blocks && setClientRules.count("signet") != 1) {
@@ -755,7 +752,7 @@ static RPCHelpMan getblocktemplate()
// Create new block
CScript scriptDummy = CScript() << OP_TRUE;
pblocktemplate = BlockAssembler(active_chainstate, mempool, Params()).CreateNewBlock(scriptDummy);
pblocktemplate = BlockAssembler{active_chainstate, mempool}.CreateNewBlock(scriptDummy);
if (!pblocktemplate)
throw JSONRPCError(RPC_OUT_OF_MEMORY, "Out of memory");