From 64ebb0f97178687517c2060bf6b9931064607888 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Mon, 10 Jun 2024 17:27:12 +0200 Subject: [PATCH] Always pass options to BlockAssembler constructor This makes the options argument for BlockAssembler constructor mandatory, dropping implicit use of ArgsManager. The caller i.e. the Mining interface implementation now handles this. In a future Stratum v2 change the Options object needs to be mofified after arguments have been processed. Specifically the pool communicates how many extra bytes it needs for its own outputs (payouts, extra commitments, etc). This will need to be substracted from what the user set as -blockmaxweight. Such a change can be implemented in createNewBlock, after ApplyArgsManOptions. --- src/node/interfaces.cpp | 6 +++++- src/node/miner.cpp | 9 --------- src/node/miner.h | 1 - src/test/blockfilter_index_tests.cpp | 3 ++- src/test/peerman_tests.cpp | 3 ++- src/test/util/setup_common.cpp | 3 ++- src/test/validation_block_tests.cpp | 6 ++++-- 7 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 215dbf5e17..356a01b286 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -75,6 +75,7 @@ using interfaces::MakeSignalHandler; using interfaces::Mining; using interfaces::Node; using interfaces::WalletLoader; +using node::BlockAssembler; using util::Join; namespace node { @@ -862,8 +863,11 @@ public: std::unique_ptr createNewBlock(const CScript& script_pub_key, bool use_mempool) override { + BlockAssembler::Options options; + ApplyArgsManOptions(gArgs, options); + LOCK(::cs_main); - return BlockAssembler{chainman().ActiveChainstate(), use_mempool ? context()->mempool.get() : nullptr}.CreateNewBlock(script_pub_key); + return BlockAssembler{chainman().ActiveChainstate(), use_mempool ? context()->mempool.get() : nullptr, options}.CreateNewBlock(script_pub_key); } NodeContext* context() override { return &m_node; } diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 87f40e993f..03c6d74deb 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -80,15 +80,6 @@ void ApplyArgsManOptions(const ArgsManager& args, BlockAssembler::Options& optio if (const auto parsed{ParseMoney(*blockmintxfee)}) options.blockMinFeeRate = CFeeRate{*parsed}; } } -static BlockAssembler::Options ConfiguredOptions() -{ - BlockAssembler::Options options; - ApplyArgsManOptions(gArgs, options); - return options; -} - -BlockAssembler::BlockAssembler(Chainstate& chainstate, const CTxMemPool* mempool) - : BlockAssembler(chainstate, mempool, ConfiguredOptions()) {} void BlockAssembler::resetBlock() { diff --git a/src/node/miner.h b/src/node/miner.h index 06a917228d..c3178a7532 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -161,7 +161,6 @@ public: bool test_block_validity{true}; }; - explicit BlockAssembler(Chainstate& chainstate, const CTxMemPool* mempool); explicit BlockAssembler(Chainstate& chainstate, const CTxMemPool* mempool, const Options& options); /** Construct a new block template with coinbase to scriptPubKeyIn */ diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index d44d84af93..067a32d6a4 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -67,7 +67,8 @@ CBlock BuildChainTestingSetup::CreateBlock(const CBlockIndex* prev, const std::vector& txns, const CScript& scriptPubKey) { - std::unique_ptr pblocktemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get()}.CreateNewBlock(scriptPubKey); + BlockAssembler::Options options; + std::unique_ptr pblocktemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get(), options}.CreateNewBlock(scriptPubKey); CBlock& block = pblocktemplate->block; block.hashPrevBlock = prev->GetBlockHash(); block.nTime = prev->nTime + 1; diff --git a/src/test/peerman_tests.cpp b/src/test/peerman_tests.cpp index 397b1d8c2d..6de373eef2 100644 --- a/src/test/peerman_tests.cpp +++ b/src/test/peerman_tests.cpp @@ -20,7 +20,8 @@ static void mineBlock(const node::NodeContext& node, std::chrono::seconds block_ { auto curr_time = GetTime(); SetMockTime(block_time); // update time so the block is created with it - CBlock block = node::BlockAssembler{node.chainman->ActiveChainstate(), nullptr}.CreateNewBlock(CScript() << OP_TRUE)->block; + node::BlockAssembler::Options options; + CBlock block = node::BlockAssembler{node.chainman->ActiveChainstate(), nullptr, options}.CreateNewBlock(CScript() << OP_TRUE)->block; while (!CheckProofOfWork(block.GetHash(), block.nBits, node.chainman->GetConsensus())) ++block.nNonce; block.fChecked = true; // little speedup SetMockTime(curr_time); // process block at current time diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 79e33eacec..cc7b2d6546 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -374,7 +374,8 @@ CBlock TestChain100Setup::CreateBlock( const CScript& scriptPubKey, Chainstate& chainstate) { - CBlock block = BlockAssembler{chainstate, nullptr}.CreateNewBlock(scriptPubKey)->block; + BlockAssembler::Options options; + CBlock block = BlockAssembler{chainstate, nullptr, options}.CreateNewBlock(scriptPubKey)->block; Assert(block.vtx.size() == 1); for (const CMutableTransaction& tx : txns) { diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 69f4e305ab..588ac60498 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -65,7 +65,8 @@ std::shared_ptr MinerTestingSetup::Block(const uint256& prev_hash) static int i = 0; static uint64_t time = Params().GenesisBlock().nTime; - auto ptemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get()}.CreateNewBlock(CScript{} << i++ << OP_TRUE); + BlockAssembler::Options options; + auto ptemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get(), options}.CreateNewBlock(CScript{} << i++ << OP_TRUE); auto pblock = std::make_shared(ptemplate->block); pblock->hashPrevBlock = prev_hash; pblock->nTime = ++time; @@ -329,7 +330,8 @@ BOOST_AUTO_TEST_CASE(witness_commitment_index) LOCK(Assert(m_node.chainman)->GetMutex()); CScript pubKey; pubKey << 1 << OP_TRUE; - auto ptemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get()}.CreateNewBlock(pubKey); + BlockAssembler::Options options; + auto ptemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get(), options}.CreateNewBlock(pubKey); CBlock pblock = ptemplate->block; CTxOut witness;