From 7ab733ede444aee61ab52670c228eec811268c6f Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Thu, 21 Nov 2024 10:41:30 +0100 Subject: [PATCH 1/3] rpc: rename coinbase_script to coinbase_output_script --- src/rpc/mining.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 60828c17115..e4e49a1b80b 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -158,11 +158,11 @@ static bool GenerateBlock(ChainstateManager& chainman, Mining& miner, CBlock&& b return true; } -static UniValue generateBlocks(ChainstateManager& chainman, Mining& miner, const CScript& coinbase_script, int nGenerate, uint64_t nMaxTries) +static UniValue generateBlocks(ChainstateManager& chainman, Mining& miner, const CScript& coinbase_output_script, int nGenerate, uint64_t nMaxTries) { UniValue blockHashes(UniValue::VARR); while (nGenerate > 0 && !chainman.m_interrupt) { - std::unique_ptr block_template(miner.createNewBlock(coinbase_script)); + std::unique_ptr block_template(miner.createNewBlock(coinbase_output_script)); CHECK_NONFATAL(block_template); std::shared_ptr block_out; @@ -236,9 +236,9 @@ static RPCHelpMan generatetodescriptor() const auto num_blocks{self.Arg("num_blocks")}; const auto max_tries{self.Arg("maxtries")}; - CScript coinbase_script; + CScript coinbase_output_script; std::string error; - if (!getScriptFromDescriptor(self.Arg("descriptor"), coinbase_script, error)) { + if (!getScriptFromDescriptor(self.Arg("descriptor"), coinbase_output_script, error)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, error); } @@ -246,7 +246,7 @@ static RPCHelpMan generatetodescriptor() Mining& miner = EnsureMining(node); ChainstateManager& chainman = EnsureChainman(node); - return generateBlocks(chainman, miner, coinbase_script, num_blocks, max_tries); + return generateBlocks(chainman, miner, coinbase_output_script, num_blocks, max_tries); }, }; } @@ -292,9 +292,9 @@ static RPCHelpMan generatetoaddress() Mining& miner = EnsureMining(node); ChainstateManager& chainman = EnsureChainman(node); - CScript coinbase_script = GetScriptForDestination(destination); + CScript coinbase_output_script = GetScriptForDestination(destination); - return generateBlocks(chainman, miner, coinbase_script, num_blocks, max_tries); + return generateBlocks(chainman, miner, coinbase_output_script, num_blocks, max_tries); }, }; } @@ -328,16 +328,16 @@ static RPCHelpMan generateblock() [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { const auto address_or_descriptor = request.params[0].get_str(); - CScript coinbase_script; + CScript coinbase_output_script; std::string error; - if (!getScriptFromDescriptor(address_or_descriptor, coinbase_script, error)) { + if (!getScriptFromDescriptor(address_or_descriptor, coinbase_output_script, error)) { const auto destination = DecodeDestination(address_or_descriptor); if (!IsValidDestination(destination)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Error: Invalid address or descriptor"); } - coinbase_script = GetScriptForDestination(destination); + coinbase_output_script = GetScriptForDestination(destination); } NodeContext& node = EnsureAnyNodeContext(request.context); @@ -371,7 +371,7 @@ static RPCHelpMan generateblock() ChainstateManager& chainman = EnsureChainman(node); { - std::unique_ptr block_template{miner.createNewBlock(coinbase_script, {.use_mempool = false})}; + std::unique_ptr block_template{miner.createNewBlock(coinbase_output_script, {.use_mempool = false})}; CHECK_NONFATAL(block_template); block = block_template->getBlock(); From ff41b9e296aba0237e068fab181523c50bf8c9d0 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Thu, 21 Nov 2024 10:48:26 +0100 Subject: [PATCH 2/3] Drop script_pub_key arg from createNewBlock Providing a script for the coinbase transaction is only done in test code and for CPU solo mining. Production miners use the getblocktemplate RPC which omits the coinbase transaction entirely from its block template, leaving it to external (pool) software to construct it. A coinbase script can still be passed via BlockCreateOptions instead. A temporary overload is added so that the test can be modified in the next commit. --- src/interfaces/mining.h | 3 +-- src/ipc/capnp/mining.capnp | 2 +- src/node/interfaces.cpp | 4 ++-- src/node/miner.cpp | 4 ++-- src/node/miner.h | 14 +++++++++++--- src/node/types.h | 21 +++++++++++++++++++-- src/rpc/mining.cpp | 7 +++---- 7 files changed, 39 insertions(+), 16 deletions(-) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index c77f3c30a26..5d1de316344 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -88,11 +88,10 @@ public: /** * Construct a new block template * - * @param[in] script_pub_key the coinbase output * @param[in] options options for creating the block * @returns a block template */ - virtual std::unique_ptr createNewBlock(const CScript& script_pub_key, const node::BlockCreateOptions& options = {}) = 0; + virtual std::unique_ptr createNewBlock(const node::BlockCreateOptions& options = {}) = 0; /** * Processes new block. A valid new block is automatically relayed to peers. diff --git a/src/ipc/capnp/mining.capnp b/src/ipc/capnp/mining.capnp index f8faaa0c424..5af37b725ab 100644 --- a/src/ipc/capnp/mining.capnp +++ b/src/ipc/capnp/mining.capnp @@ -17,7 +17,7 @@ interface Mining $Proxy.wrap("interfaces::Mining") { isInitialBlockDownload @1 (context :Proxy.Context) -> (result: Bool); getTip @2 (context :Proxy.Context) -> (result: Common.BlockRef, hasResult: Bool); waitTipChanged @3 (context :Proxy.Context, currentTip: Data, timeout: Float64) -> (result: Common.BlockRef); - createNewBlock @4 (scriptPubKey: Data, options: BlockCreateOptions) -> (result: BlockTemplate); + createNewBlock @4 (options: BlockCreateOptions) -> (result: BlockTemplate); processNewBlock @5 (context :Proxy.Context, block: Data) -> (newBlock: Bool, result: Bool); getTransactionsUpdated @6 (context :Proxy.Context) -> (result: UInt32); testBlockValidity @7 (context :Proxy.Context, block: Data, checkMerkleRoot: Bool) -> (state: BlockValidationState, result: Bool); diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index e4ae9400e37..45c9208727e 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -1004,11 +1004,11 @@ public: return TestBlockValidity(state, chainman().GetParams(), chainman().ActiveChainstate(), block, tip, /*fCheckPOW=*/false, check_merkle_root); } - std::unique_ptr createNewBlock(const CScript& script_pub_key, const BlockCreateOptions& options) override + std::unique_ptr createNewBlock(const BlockCreateOptions& options) override { BlockAssembler::Options assemble_options{options}; ApplyArgsManOptions(*Assert(m_node.args), assemble_options); - return std::make_unique(BlockAssembler{chainman().ActiveChainstate(), context()->mempool.get(), assemble_options}.CreateNewBlock(script_pub_key), m_node); + return std::make_unique(BlockAssembler{chainman().ActiveChainstate(), context()->mempool.get(), assemble_options}.CreateNewBlock(), m_node); } NodeContext* context() override { return &m_node; } diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 790ee1c1466..5d7304b597e 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -106,7 +106,7 @@ void BlockAssembler::resetBlock() nFees = 0; } -std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& scriptPubKeyIn) +std::unique_ptr BlockAssembler::CreateNewBlock() { const auto time_start{SteadyClock::now()}; @@ -151,7 +151,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc coinbaseTx.vin.resize(1); coinbaseTx.vin[0].prevout.SetNull(); coinbaseTx.vout.resize(1); - coinbaseTx.vout[0].scriptPubKey = scriptPubKeyIn; + coinbaseTx.vout[0].scriptPubKey = m_options.coinbase_output_script; coinbaseTx.vout[0].nValue = nFees + GetBlockSubsidy(nHeight, chainparams.GetConsensus()); coinbaseTx.vin[0].scriptSig = CScript() << nHeight << OP_0; pblock->vtx[0] = MakeTransactionRef(std::move(coinbaseTx)); diff --git a/src/node/miner.h b/src/node/miner.h index 25ce110b348..6b6079fa6ef 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -169,14 +169,22 @@ public: explicit BlockAssembler(Chainstate& chainstate, const CTxMemPool* mempool, const Options& options); - /** Construct a new block template with coinbase to scriptPubKeyIn */ - std::unique_ptr CreateNewBlock(const CScript& scriptPubKeyIn); + /** Construct a new block template */ + std::unique_ptr CreateNewBlock(); + + /** Temporary overload for tests */ + std::unique_ptr CreateNewBlock(const CScript& scriptPubKeyIn) + { + m_options.coinbase_output_script = scriptPubKeyIn; + return CreateNewBlock(); + }; inline static std::optional m_last_block_num_txs{}; inline static std::optional m_last_block_weight{}; private: - const Options m_options; + // TODO: make const again + Options m_options; // utility functions /** Clear the block's state and prepare for assembling a new block */ diff --git a/src/node/types.h b/src/node/types.h index 1302f1b127f..2fc66b892b5 100644 --- a/src/node/types.h +++ b/src/node/types.h @@ -3,8 +3,8 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. //! @file node/types.h is a home for public enum and struct type definitions -//! that are used by internally by node code, but also used externally by wallet -//! or GUI code. +//! that are used by internally by node code, but also used externally by wallet, +//! mining or GUI code. //! //! This file is intended to define only simple types that do not have external //! dependencies. More complicated types should be defined in dedicated header @@ -14,6 +14,7 @@ #define BITCOIN_NODE_TYPES_H #include +#include