From 0e2b12b92a28a2949e75bf50f31563f52e647d6e Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Mon, 4 Nov 2024 18:24:45 -0500 Subject: [PATCH 01/30] net, init: derive default onion port if a user specified a -port After port collisions are no longer tolerated but lead to a startup failure in v28.0, local setups of multiple nodes, each with a different -port value would not be possible anymore due to collision of the onion default port - even if the nodes were using tor or not interested in receiving onion inbound connections. Fix this by deriving the onion listening port to be -port + 1. (idea by vasild / laanwj) Co-authored-by: Vasil Dimov --- src/chainparamsbase.cpp | 10 +++++----- src/chainparamsbase.h | 6 ++---- src/init.cpp | 10 ++++++---- src/torcontrol.cpp | 4 ++-- src/torcontrol.h | 2 +- test/functional/test_framework/test_node.py | 5 ++--- 6 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/chainparamsbase.cpp b/src/chainparamsbase.cpp index aadd04e509f22..060d519d9207c 100644 --- a/src/chainparamsbase.cpp +++ b/src/chainparamsbase.cpp @@ -41,15 +41,15 @@ std::unique_ptr CreateBaseChainParams(const ChainType chain) { switch (chain) { case ChainType::MAIN: - return std::make_unique("", 8332, 8334); + return std::make_unique("", 8332); case ChainType::TESTNET: - return std::make_unique("testnet3", 18332, 18334); + return std::make_unique("testnet3", 18332); case ChainType::TESTNET4: - return std::make_unique("testnet4", 48332, 48334); + return std::make_unique("testnet4", 48332); case ChainType::SIGNET: - return std::make_unique("signet", 38332, 38334); + return std::make_unique("signet", 38332); case ChainType::REGTEST: - return std::make_unique("regtest", 18443, 18445); + return std::make_unique("regtest", 18443); } assert(false); } diff --git a/src/chainparamsbase.h b/src/chainparamsbase.h index c75a70cb96b00..adbd6a5174695 100644 --- a/src/chainparamsbase.h +++ b/src/chainparamsbase.h @@ -22,15 +22,13 @@ class CBaseChainParams public: const std::string& DataDir() const { return strDataDir; } uint16_t RPCPort() const { return m_rpc_port; } - uint16_t OnionServiceTargetPort() const { return m_onion_service_target_port; } CBaseChainParams() = delete; - CBaseChainParams(const std::string& data_dir, uint16_t rpc_port, uint16_t onion_service_target_port) - : m_rpc_port(rpc_port), m_onion_service_target_port(onion_service_target_port), strDataDir(data_dir) {} + CBaseChainParams(const std::string& data_dir, uint16_t rpc_port) + : m_rpc_port(rpc_port), strDataDir(data_dir) {} private: const uint16_t m_rpc_port; - const uint16_t m_onion_service_target_port; std::string strDataDir; }; diff --git a/src/init.cpp b/src/init.cpp index 94a5a084638cc..8c4ae2e62d393 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -521,7 +521,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc) argsman.AddArg("-addnode=", strprintf("Add a node to connect to and attempt to keep the connection open (see the addnode RPC help for more info). This option can be specified multiple times to add multiple nodes; connections are limited to %u at a time and are counted separately from the -maxconnections limit.", MAX_ADDNODE_CONNECTIONS), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); argsman.AddArg("-asmap=", strprintf("Specify asn mapping used for bucketing of the peers (default: %s). Relative paths will be prefixed by the net-specific datadir location.", DEFAULT_ASMAP_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-bantime=", strprintf("Default duration (in seconds) of manually configured bans (default: %u)", DEFAULT_MISBEHAVING_BANTIME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-bind=[:][=onion]", strprintf("Bind to given address and always listen on it (default: 0.0.0.0). Use [host]:port notation for IPv6. Append =onion to tag any incoming connections to that address and port as incoming Tor connections (default: 127.0.0.1:%u=onion, testnet3: 127.0.0.1:%u=onion, testnet4: 127.0.0.1:%u=onion, signet: 127.0.0.1:%u=onion, regtest: 127.0.0.1:%u=onion)", defaultBaseParams->OnionServiceTargetPort(), testnetBaseParams->OnionServiceTargetPort(), testnet4BaseParams->OnionServiceTargetPort(), signetBaseParams->OnionServiceTargetPort(), regtestBaseParams->OnionServiceTargetPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); + argsman.AddArg("-bind=[:][=onion]", strprintf("Bind to given address and always listen on it (default: 0.0.0.0). Use [host]:port notation for IPv6. Append =onion to tag any incoming connections to that address and port as incoming Tor connections (default: 127.0.0.1:%u=onion, testnet3: 127.0.0.1:%u=onion, testnet4: 127.0.0.1:%u=onion, signet: 127.0.0.1:%u=onion, regtest: 127.0.0.1:%u=onion)", defaultChainParams->GetDefaultPort() + 1, testnetChainParams->GetDefaultPort() + 1, testnet4ChainParams->GetDefaultPort() + 1, signetChainParams->GetDefaultPort() + 1, regtestChainParams->GetDefaultPort() + 1), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); argsman.AddArg("-cjdnsreachable", "If set, then this host is configured for CJDNS (connecting to fc00::/8 addresses would lead us to the CJDNS network, see doc/cjdns.md) (default: 0)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-connect=", "Connect only to the specified node; -noconnect disables automatic connections (the rules for this peer are the same as for -addnode). This option can be specified multiple times to connect to multiple nodes.", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); argsman.AddArg("-discover", "Discover own IP addresses (default: 1 when listening and no -externalip or -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); @@ -548,7 +548,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc) argsman.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-txreconciliation", strprintf("Enable transaction reconciliations per BIP 330 (default: %d)", DEFAULT_TXRECONCILIATION_ENABLE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CONNECTION); - argsman.AddArg("-port=", strprintf("Listen for connections on (default: %u, testnet3: %u, testnet4: %u, signet: %u, regtest: %u). Not relevant for I2P (see doc/i2p.md).", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort(), testnet4ChainParams->GetDefaultPort(), signetChainParams->GetDefaultPort(), regtestChainParams->GetDefaultPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); + argsman.AddArg("-port=", strprintf("Listen for connections on (default: %u, testnet3: %u, testnet4: %u, signet: %u, regtest: %u). Not relevant for I2P (see doc/i2p.md). If set to a value x, the default onion listening port will be set to x+1.", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort(), testnet4ChainParams->GetDefaultPort(), signetChainParams->GetDefaultPort(), regtestChainParams->GetDefaultPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); #ifdef HAVE_SOCKADDR_UN argsman.AddArg("-proxy=", "Connect through SOCKS5 proxy, set -noproxy to disable (default: disabled). May be a local file path prefixed with 'unix:' if the proxy supports it.", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_ELISION, OptionsCategory::CONNECTION); #else @@ -1846,6 +1846,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) const uint16_t default_bind_port = static_cast(args.GetIntArg("-port", Params().GetDefaultPort())); + const uint16_t default_bind_port_onion = default_bind_port + 1; + const auto BadPortWarning = [](const char* prefix, uint16_t port) { return strprintf(_("%s request to listen on port %u. This port is considered \"bad\" and " "thus it is unlikely that any peer will connect to it. See " @@ -1870,7 +1872,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) const std::string network_type = bind_arg.substr(index + 1); if (network_type == "onion") { const std::string truncated_bind_arg = bind_arg.substr(0, index); - bind_addr = Lookup(truncated_bind_arg, BaseParams().OnionServiceTargetPort(), false); + bind_addr = Lookup(truncated_bind_arg, default_bind_port_onion, false); if (bind_addr.has_value()) { connOptions.onion_binds.push_back(bind_addr.value()); continue; @@ -1906,7 +1908,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } else if (!connOptions.vBinds.empty()) { onion_service_target = connOptions.vBinds.front(); } else { - onion_service_target = DefaultOnionServiceTarget(); + onion_service_target = DefaultOnionServiceTarget(default_bind_port_onion); connOptions.onion_binds.push_back(onion_service_target); } diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp index 58fc1bdf2aa02..60df916f166c6 100644 --- a/src/torcontrol.cpp +++ b/src/torcontrol.cpp @@ -711,9 +711,9 @@ void StopTorControl() } } -CService DefaultOnionServiceTarget() +CService DefaultOnionServiceTarget(uint16_t port) { struct in_addr onion_service_target; onion_service_target.s_addr = htonl(INADDR_LOOPBACK); - return {onion_service_target, BaseParams().OnionServiceTargetPort()}; + return {onion_service_target, port}; } diff --git a/src/torcontrol.h b/src/torcontrol.h index 4a0eef223e41d..0b66201cf1a46 100644 --- a/src/torcontrol.h +++ b/src/torcontrol.h @@ -27,7 +27,7 @@ void StartTorControl(CService onion_service_target); void InterruptTorControl(); void StopTorControl(); -CService DefaultOnionServiceTarget(); +CService DefaultOnionServiceTarget(uint16_t port); /** Reply from Tor, can be single or multi-line */ class TorControlReply diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index f1083dc1ce6bf..0749860c8e662 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -221,10 +221,9 @@ def start(self, extra_args=None, *, cwd=None, stdout=None, stderr=None, env=None extra_args = self.extra_args # If listening and no -bind is given, then bitcoind would bind P2P ports on - # 0.0.0.0:P and 127.0.0.1:18445 (for incoming Tor connections), where P is + # 0.0.0.0:P and 127.0.0.1:P+1 (for incoming Tor connections), where P is # a unique port chosen by the test framework and configured as port=P in - # bitcoin.conf. To avoid collisions on 127.0.0.1:18445, change it to - # 127.0.0.1:tor_port(). + # bitcoin.conf. To avoid collisions, change it to 127.0.0.1:tor_port(). will_listen = all(e != "-nolisten" and e != "-listen=0" for e in extra_args) has_explicit_bind = self.has_explicit_bind or any(e.startswith("-bind=") for e in extra_args) if will_listen and not has_explicit_bind: From 997757dd2b4d7b20b17299fbd21970b2efb8bbc8 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Fri, 15 Nov 2024 16:05:12 -0500 Subject: [PATCH 02/30] test: add functional test for -port behavior --- test/functional/feature_port.py | 60 +++++++++++++++++++++++++++++++++ test/functional/test_runner.py | 1 + 2 files changed, 61 insertions(+) create mode 100755 test/functional/feature_port.py diff --git a/test/functional/feature_port.py b/test/functional/feature_port.py new file mode 100755 index 0000000000000..2746d7d79c1d6 --- /dev/null +++ b/test/functional/feature_port.py @@ -0,0 +1,60 @@ +#!/usr/bin/env python3 +# Copyright (c) 2024-present The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +""" +Test the -port option and its interactions with +-bind. +""" + +from test_framework.test_framework import ( + BitcoinTestFramework, +) +from test_framework.util import ( + p2p_port, +) + + +class PortTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = True + # Avoid any -bind= on the command line. + self.bind_to_localhost_only = False + self.num_nodes = 1 + + def run_test(self): + node = self.nodes[0] + node.has_explicit_bind = True + port1 = p2p_port(self.num_nodes) + port2 = p2p_port(self.num_nodes + 5) + + self.log.info("When starting with -port, bitcoind binds to it and uses port + 1 for an onion bind") + with node.assert_debug_log(expected_msgs=[f'Bound to 0.0.0.0:{port1}', f'Bound to 127.0.0.1:{port1 + 1}']): + self.restart_node(0, extra_args=["-listen", f"-port={port1}"]) + + self.log.info("When specifying -port multiple times, only the last one is taken") + with node.assert_debug_log(expected_msgs=[f'Bound to 0.0.0.0:{port2}', f'Bound to 127.0.0.1:{port2 + 1}'], unexpected_msgs=[f'Bound to 0.0.0.0:{port1}']): + self.restart_node(0, extra_args=["-listen", f"-port={port1}", f"-port={port2}"]) + + self.log.info("When specifying ports with both -port and -bind, the one from -port is ignored") + with node.assert_debug_log(expected_msgs=[f'Bound to 0.0.0.0:{port2}'], unexpected_msgs=[f'Bound to 0.0.0.0:{port1}']): + self.restart_node(0, extra_args=["-listen", f"-port={port1}", f"-bind=0.0.0.0:{port2}"]) + + self.log.info("When -bind specifies no port, the values from -port and -bind are combined") + with self.nodes[0].assert_debug_log(expected_msgs=[f'Bound to 0.0.0.0:{port1}']): + self.restart_node(0, extra_args=["-listen", f"-port={port1}", "-bind=0.0.0.0"]) + + self.log.info("When an onion bind specifies no port, the value from -port, incremented by 1, is taken") + with self.nodes[0].assert_debug_log(expected_msgs=[f'Bound to 127.0.0.1:{port1 + 1}']): + self.restart_node(0, extra_args=["-listen", f"-port={port1}", "-bind=127.0.0.1=onion"]) + + self.log.info("Invalid values for -port raise errors") + self.stop_node(0) + node.extra_args = ["-listen", "-port=65536"] + node.assert_start_raises_init_error(expected_msg="Error: Invalid port specified in -port: '65536'") + node.extra_args = ["-listen", "-port=0"] + node.assert_start_raises_init_error(expected_msg="Error: Invalid port specified in -port: '0'") + + +if __name__ == '__main__': + PortTest(__file__).main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 701d81b9d25eb..cb621b537466f 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -339,6 +339,7 @@ 'feature_minchainwork.py', 'rpc_estimatefee.py', 'rpc_getblockstats.py', + 'feature_port.py', 'feature_bind_port_externalip.py', 'wallet_create_tx.py --legacy-wallet', 'wallet_send.py --legacy-wallet', From 1dd3af8fbc350c6f1efa8ae6449e67e1b42ccff4 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Tue, 5 Nov 2024 13:04:20 -0500 Subject: [PATCH 03/30] Add release note for #31223 Co-authored-by: Vasil Dimov --- doc/release-notes-31223.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 doc/release-notes-31223.md diff --git a/doc/release-notes-31223.md b/doc/release-notes-31223.md new file mode 100644 index 0000000000000..44f0552fd9335 --- /dev/null +++ b/doc/release-notes-31223.md @@ -0,0 +1,15 @@ +P2P and network changes +----------------------- +When the `-port` configuration option is used, the default onion listening port will now +be derived to be that port + 1 instead of being set to a fixed value (8334 on mainnet). +This re-allows setups with multiple local nodes using different `-port` and not using `-bind`, +which would lead to a startup failure in v28.0 due to a port collision. + +Note that a `HiddenServicePort` manually configured in `torrc` may need adjustment if used in +connection with the `-port` option. +For example, if you are using `-port=5555` with a non-standard value and not using `-bind=...=onion`, +previously Bitcoin Core would listen for incoming Tor connections on `127.0.0.1:8334`. +Now it would listen on `127.0.0.1:5556` (`-port` plus one). If you configured the hidden service manually +in torrc now you have to change it from `HiddenServicePort 8333 127.0.0.1:8334` to `HiddenServicePort 8333 +127.0.0.1:5556`, or configure bitcoind with `-bind=127.0.0.1:8334=onion` to get the previous behavior. +(#31223) From 7ab733ede444aee61ab52670c228eec811268c6f Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Thu, 21 Nov 2024 10:41:30 +0100 Subject: [PATCH 04/30] 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 60828c17115b4..e4e49a1b80b8b 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 05/30] 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 c77f3c30a2638..5d1de316344e0 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -88,11 +88,10 @@ class Mining /** * 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 f8faaa0c4246d..5af37b725ab25 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 e4ae9400e37aa..45c9208727e32 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -1004,11 +1004,11 @@ class MinerImpl : public Mining 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 790ee1c146653..5d7304b597ea3 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 25ce110b34886..6b6079fa6ef2c 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -169,14 +169,22 @@ class BlockAssembler 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 1302f1b127fa6..2fc66b892b53a 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