From ec777917d6eba0b417dbc90b9b891240a44b7ec4 Mon Sep 17 00:00:00 2001 From: Randall Naar Date: Sat, 27 Apr 2024 23:50:18 -0400 Subject: [PATCH 01/22] test: Fix intermittent issue in wallet_backwards_compatibility.py --- test/functional/wallet_backwards_compatibility.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/functional/wallet_backwards_compatibility.py b/test/functional/wallet_backwards_compatibility.py index ab008a40cd..f66ab5f52c 100755 --- a/test/functional/wallet_backwards_compatibility.py +++ b/test/functional/wallet_backwards_compatibility.py @@ -172,6 +172,7 @@ def run_test(self): # Create another conflicting transaction using RBF tx3_id = node_master.sendtoaddress(return_address, 1) tx4_id = node_master.bumpfee(tx3_id)["txid"] + self.sync_mempools() # Abandon transaction, but don't confirm node_master.abandontransaction(tx3_id) From 0e2b12b92a28a2949e75bf50f31563f52e647d6e Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Mon, 4 Nov 2024 18:24:45 -0500 Subject: [PATCH 02/22] 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 aadd04e509..060d519d92 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 c75a70cb96..adbd6a5174 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 94a5a08463..8c4ae2e62d 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 58fc1bdf2a..60df916f16 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 4a0eef223e..0b66201cf1 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 f1083dc1ce..0749860c8e 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 03/22] 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 0000000000..2746d7d79c --- /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 701d81b9d2..cb621b5374 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 04/22] 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 0000000000..44f0552fd9 --- /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 988721d37a3c65e4ef86945133207fda243f2fba Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Thu, 21 Nov 2024 22:59:09 +0100 Subject: [PATCH 05/22] test: avoid internet traffic in rpc_net.py Can be tested by running ``` $ sudo tcpdump -i eth0 host 11.22.33.44 ``` and verifying that no packets appear in the tcpdump output. Co-authored-by: Vasil Dimov --- test/functional/rpc_net.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py index b63059b1ee..41ecbbed22 100755 --- a/test/functional/rpc_net.py +++ b/test/functional/rpc_net.py @@ -63,6 +63,9 @@ class NetTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 self.extra_args = [["-minrelaytxfee=0.00001000"], ["-minrelaytxfee=0.00000500"]] + # Specify a non-working proxy to make sure no actual connections to public IPs are attempted + for args in self.extra_args: + args.append("-proxy=127.0.0.1:1") self.supports_cli = False def run_test(self): From 37946c0aafeebc1585f1316fb05f252f7fb51e91 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Fri, 6 Dec 2024 14:24:21 +0700 Subject: [PATCH 06/22] Set notifications m_tip_block in LoadChainTip() Ensure KernelNotifications m_tip_block is set even if no new block arrives. Additionally, have node init always wait for this to happen. --- src/init.cpp | 16 +++++++++++++--- src/interfaces/mining.h | 4 ++-- src/node/blockstorage.h | 1 + src/node/kernel_notifications.h | 6 +++--- src/validation.cpp | 7 +++++++ 5 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 19efc69aaa..f6c3879b98 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1770,7 +1770,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) node.background_init_thread = std::thread(&util::TraceThread, "initload", [=, &chainman, &args, &node] { ScheduleBatchPriority(); - // Import blocks + // Import blocks and ActivateBestChain() ImportBlocks(chainman, vImportFiles); if (args.GetBoolArg("-stopafterblockimport", DEFAULT_STOPAFTERBLOCKIMPORT)) { LogPrintf("Stopping after block import\n"); @@ -1793,8 +1793,18 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } }); - // Wait for genesis block to be processed - if (WITH_LOCK(chainman.GetMutex(), return chainman.ActiveTip() == nullptr)) { + /* + * Wait for genesis block to be processed. Typically kernel_notifications.m_tip_block + * has already been set by a call to LoadChainTip() in CompleteChainstateInitialization(). + * But this is skipped if the chainstate doesn't exist yet or is being wiped: + * + * 1. first startup with an empty datadir + * 2. reindex + * 3. reindex-chainstate + * + * In these case it's connected by a call to ActivateBestChain() in the initload thread. + */ + { WAIT_LOCK(kernel_notifications.m_tip_block_mutex, lock); kernel_notifications.m_tip_block_cv.wait(lock, [&]() EXCLUSIVE_LOCKS_REQUIRED(kernel_notifications.m_tip_block_mutex) { return !kernel_notifications.m_tip_block.IsNull() || ShutdownRequested(node); diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index c77f3c30a2..6f23bf3bb5 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -75,8 +75,8 @@ class Mining virtual std::optional getTip() = 0; /** - * Waits for the connected tip to change. If the tip was not connected on - * startup, this will wait. + * Waits for the connected tip to change. During node initialization, this will + * wait until the tip is connected. * * @param[in] current_tip block hash of the current chain tip. Function waits * for the chain tip to differ from this. diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 03bc5f4600..ac6db0558d 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -430,6 +430,7 @@ class BlockManager void CleanupBlockRevFiles() const; }; +// Calls ActivateBestChain() even if no blocks are imported. void ImportBlocks(ChainstateManager& chainman, std::span import_paths); } // namespace node diff --git a/src/node/kernel_notifications.h b/src/node/kernel_notifications.h index 296b9c426d..35070b5285 100644 --- a/src/node/kernel_notifications.h +++ b/src/node/kernel_notifications.h @@ -56,9 +56,9 @@ class KernelNotifications : public kernel::Notifications Mutex m_tip_block_mutex; std::condition_variable m_tip_block_cv GUARDED_BY(m_tip_block_mutex); - //! The block for which the last blockTip notification was received for. - //! The initial ZERO means that no block has been connected yet, which may - //! be true even long after startup, until shutdown. + //! The block for which the last blockTip notification was received. + //! It's first set when the tip is connected during node initialization. + //! Might be unset during an early shutdown. uint256 m_tip_block GUARDED_BY(m_tip_block_mutex){uint256::ZERO}; private: diff --git a/src/validation.cpp b/src/validation.cpp index 74f4e80485..e1bddf8bcb 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4721,6 +4721,13 @@ bool Chainstate::LoadChainTip() m_chain.Height(), FormatISO8601DateTime(tip->GetBlockTime()), GuessVerificationProgress(m_chainman.GetParams().TxData(), tip)); + + // Ensure KernelNotifications m_tip_block is set even if no new block arrives. + if (this->GetRole() != ChainstateRole::BACKGROUND) { + // Ignoring return value for now. + (void)m_chainman.GetNotifications().blockTip(GetSynchronizationState(/*init=*/true, m_chainman.m_blockman.m_blockfiles_indexed), *pindex); + } + return true; } From 932cd1e92b6d39c6879f546867698bc8441d09cd Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 26 Nov 2024 11:18:18 -0500 Subject: [PATCH 07/22] wallet: fix crash during watch-only wallet migration The crash occurs because we assume the cached scripts structure will not be empty, but it can be empty when the legacy wallet contained only watch-only and solvable but not spendable scripts --- src/wallet/wallet.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 0961aebc7c..b971be5ddd 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4085,7 +4085,11 @@ util::Result CWallet::ApplyMigrationData(WalletBatch& local_wallet_batch, if (ExtractDestination(script, dest)) not_migrated_dests.emplace(dest); } - Assume(!m_cached_spks.empty()); + // When the legacy wallet has no spendable scripts, the main wallet will be empty, leaving its script cache empty as well. + // The watch-only and/or solvable wallet(s) will contain the scripts in their respective caches. + if (!data.desc_spkms.empty()) Assume(!m_cached_spks.empty()); + if (!data.watch_descs.empty()) Assume(!data.watchonly_wallet->m_cached_spks.empty()); + if (!data.solvable_descs.empty()) Assume(!data.solvable_wallet->m_cached_spks.empty()); for (auto& desc_spkm : data.desc_spkms) { if (m_spk_managers.count(desc_spkm->GetID()) > 0) { From 297a876c9809e267e37481fc776cbec90056b078 Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 26 Nov 2024 11:31:14 -0500 Subject: [PATCH 08/22] test: add coverage for migrating watch-only script --- test/functional/wallet_migration.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index 5abb43e3b9..e22a316f37 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -19,7 +19,7 @@ from test_framework.key import ECPubKey from test_framework.test_framework import BitcoinTestFramework from test_framework.messages import COIN, CTransaction, CTxOut -from test_framework.script_util import key_to_p2pkh_script, script_to_p2sh_script, script_to_p2wsh_script +from test_framework.script_util import key_to_p2pkh_script, key_to_p2pk_script, script_to_p2sh_script, script_to_p2wsh_script from test_framework.util import ( assert_equal, assert_raises_rpc_error, @@ -27,6 +27,7 @@ ) from test_framework.wallet_util import ( get_generate_key, + generate_keypair, ) @@ -1006,6 +1007,17 @@ def check_comments(): wallet.unloadwallet() + def test_migrate_simple_watch_only(self): + self.log.info("Test migrating a watch-only p2pk script") + wallet = self.create_legacy_wallet("bare_p2pk", blank=True) + _, pubkey = generate_keypair() + p2pk_script = key_to_p2pk_script(pubkey) + wallet.importaddress(address=p2pk_script.hex()) + # Migrate wallet in the latest node + res, _ = self.migrate_and_get_rpc("bare_p2pk") + wo_wallet = self.master_node.get_wallet_rpc(res['watchonly_name']) + assert_equal(wo_wallet.listdescriptors()['descriptors'][0]['desc'], descsum_create(f'pk({pubkey.hex()})')) + wo_wallet.unloadwallet() def run_test(self): self.master_node = self.nodes[0] @@ -1032,6 +1044,8 @@ def run_test(self): self.test_avoidreuse() self.test_preserve_tx_extra_info() self.test_blank() + self.test_migrate_simple_watch_only() + if __name__ == '__main__': WalletMigrationTest(__file__).main() From cdd207c0e48081ab13e2c8c9886f3e2d5da1857e Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 26 Nov 2024 14:21:30 -0500 Subject: [PATCH 09/22] test: add coverage for migrating standalone imported keys --- test/functional/wallet_migration.py | 46 +++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index e22a316f37..fd6ec16065 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -19,6 +19,7 @@ from test_framework.key import ECPubKey from test_framework.test_framework import BitcoinTestFramework from test_framework.messages import COIN, CTransaction, CTxOut +from test_framework.script import hash160 from test_framework.script_util import key_to_p2pkh_script, key_to_p2pk_script, script_to_p2sh_script, script_to_p2wsh_script from test_framework.util import ( assert_equal, @@ -1019,6 +1020,50 @@ def test_migrate_simple_watch_only(self): assert_equal(wo_wallet.listdescriptors()['descriptors'][0]['desc'], descsum_create(f'pk({pubkey.hex()})')) wo_wallet.unloadwallet() + def test_manual_keys_import(self): + self.log.info("Test migrating standalone private keys") + wallet = self.create_legacy_wallet("import_privkeys", blank=True) + privkey, pubkey = generate_keypair(wif=True) + wallet.importprivkey(privkey=privkey, label="hi", rescan=False) + + # Migrate and verify + res, wallet = self.migrate_and_get_rpc("import_privkeys") + + # There should be descriptors containing the imported key for: pk(), pkh(), sh(wpkh()), wpkh() + key_origin = hash160(pubkey)[:4].hex() + pubkey_hex = pubkey.hex() + pk_desc = descsum_create(f'pk([{key_origin}]{pubkey_hex})') + pkh_desc = descsum_create(f'pkh([{key_origin}]{pubkey_hex})') + sh_wpkh_desc = descsum_create(f'sh(wpkh([{key_origin}]{pubkey_hex}))') + wpkh_desc = descsum_create(f'wpkh([{key_origin}]{pubkey_hex})') + expected_descs = [pk_desc, pkh_desc, sh_wpkh_desc, wpkh_desc] + + # Verify all expected descriptors were migrated + migrated_desc = [item['desc'] for item in wallet.listdescriptors()['descriptors'] if pubkey.hex() in item['desc']] + assert_equal(expected_descs, migrated_desc) + wallet.unloadwallet() + + ###################################################### + self.log.info("Test migrating standalone public keys") + wallet = self.create_legacy_wallet("import_pubkeys", blank=True) + wallet.importpubkey(pubkey=pubkey_hex, rescan=False) + + res, _ = self.migrate_and_get_rpc("import_pubkeys") + + # Same as before, there should be descriptors in the watch-only wallet for the imported pubkey + wo_wallet = self.nodes[0].get_wallet_rpc(res['watchonly_name']) + # As we imported the pubkey only, there will be no key origin in the following descriptors + pk_desc = descsum_create(f'pk({pubkey_hex})') + pkh_desc = descsum_create(f'pkh({pubkey_hex})') + sh_wpkh_desc = descsum_create(f'sh(wpkh({pubkey_hex}))') + wpkh_desc = descsum_create(f'wpkh({pubkey_hex})') + expected_descs = [pk_desc, pkh_desc, sh_wpkh_desc, wpkh_desc] + + # Verify all expected descriptors were migrated + migrated_desc = [item['desc'] for item in wo_wallet.listdescriptors()['descriptors']] + assert_equal(expected_descs, migrated_desc) + wo_wallet.unloadwallet() + def run_test(self): self.master_node = self.nodes[0] self.old_node = self.nodes[1] @@ -1045,6 +1090,7 @@ def run_test(self): self.test_preserve_tx_extra_info() self.test_blank() self.test_migrate_simple_watch_only() + self.test_manual_keys_import() if __name__ == '__main__': From b81a4659950a6c4e22316f66b55cae8afc4f4d9a Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Fri, 6 Dec 2024 21:45:18 +0100 Subject: [PATCH 10/22] refactor test: Profit from using namespace + using detail function Also adds BOOST_CHECK_NO_THROW() while touching that line, clarifying part of what we are checking for. Also removed redundant inline from template functions in .cpp file. --- src/test/util_string_tests.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp index fbab9e7aba..eaf179f640 100644 --- a/src/test/util_string_tests.cpp +++ b/src/test/util_string_tests.cpp @@ -8,21 +8,22 @@ #include using namespace util; +using util::detail::CheckNumFormatSpecifiers; BOOST_AUTO_TEST_SUITE(util_string_tests) // Helper to allow compile-time sanity checks while providing the number of // args directly. Normally PassFmt would be used. template -inline void PassFmt(util::ConstevalFormatString fmt) +void PassFmt(ConstevalFormatString fmt) { // Execute compile-time check again at run-time to get code coverage stats - util::detail::CheckNumFormatSpecifiers(fmt.fmt); + BOOST_CHECK_NO_THROW(CheckNumFormatSpecifiers(fmt.fmt)); } template -inline void FailFmtWithError(const char* wrong_fmt, std::string_view error) +void FailFmtWithError(const char* wrong_fmt, std::string_view error) { - BOOST_CHECK_EXCEPTION(util::detail::CheckNumFormatSpecifiers(wrong_fmt), const char*, HasReason{error}); + BOOST_CHECK_EXCEPTION(CheckNumFormatSpecifiers(wrong_fmt), const char*, HasReason{error}); } BOOST_AUTO_TEST_CASE(ConstevalFormatString_NumSpec) From 533013cba20664e3581a8e4633cc188d5be3175a Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Fri, 6 Dec 2024 16:08:34 +0100 Subject: [PATCH 11/22] test: Prove+document ConstevalFormatString/tinyformat parity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Lőrinc Co-Authored-By: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Co-Authored-By: Ryan Ofsky --- src/test/util_string_tests.cpp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp index eaf179f640..d90539af47 100644 --- a/src/test/util_string_tests.cpp +++ b/src/test/util_string_tests.cpp @@ -12,6 +12,14 @@ using util::detail::CheckNumFormatSpecifiers; BOOST_AUTO_TEST_SUITE(util_string_tests) +template +void TfmFormatZeroes(const std::string& fmt) +{ + std::apply([&](auto... args) { + (void)tfm::format(fmt, args...); + }, std::array{}); +} + // Helper to allow compile-time sanity checks while providing the number of // args directly. Normally PassFmt would be used. template @@ -19,6 +27,12 @@ void PassFmt(ConstevalFormatString fmt) { // Execute compile-time check again at run-time to get code coverage stats BOOST_CHECK_NO_THROW(CheckNumFormatSpecifiers(fmt.fmt)); + + // If ConstevalFormatString didn't throw above, make sure tinyformat doesn't + // throw either for the same format string and parameter count combination. + // Proves that we have some extent of protection from runtime errors + // (tinyformat may still throw for some type mismatches). + BOOST_CHECK_NO_THROW(TfmFormatZeroes(fmt.fmt)); } template void FailFmtWithError(const char* wrong_fmt, std::string_view error) @@ -111,6 +125,15 @@ BOOST_AUTO_TEST_CASE(ConstevalFormatString_NumSpec) FailFmtWithError<2>("%1$*2$", err_term); FailFmtWithError<2>("%1$.*2$", err_term); FailFmtWithError<2>("%1$9.*2$", err_term); + + // Ensure that tinyformat throws if format string contains wrong number + // of specifiers. PassFmt relies on this to verify tinyformat successfully + // formats the strings, and will need to be updated if tinyformat is changed + // not to throw on failure. + BOOST_CHECK_EXCEPTION(TfmFormatZeroes<2>("%s"), tfm::format_error, + HasReason{"tinyformat: Not enough conversion specifiers in format string"}); + BOOST_CHECK_EXCEPTION(TfmFormatZeroes<1>("%s %s"), tfm::format_error, + HasReason{"tinyformat: Too many conversion specifiers in format string"}); } BOOST_AUTO_TEST_SUITE_END() From 76cca4aa6fcd363dee821c837b1b627ea137b7a4 Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Fri, 6 Dec 2024 21:56:16 +0100 Subject: [PATCH 12/22] test: Document non-parity between tinyformat and ConstevalFormatstring - For "%n", which is supposed to write to the argument for printf. - For string/integer mismatches of width/precision specifiers. Co-Authored-By: Ryan Ofsky --- src/test/util_string_tests.cpp | 9 +++++++++ test/lint/run-lint-format-strings.py | 2 ++ 2 files changed, 11 insertions(+) diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp index d90539af47..e40bef6379 100644 --- a/src/test/util_string_tests.cpp +++ b/src/test/util_string_tests.cpp @@ -126,6 +126,15 @@ BOOST_AUTO_TEST_CASE(ConstevalFormatString_NumSpec) FailFmtWithError<2>("%1$.*2$", err_term); FailFmtWithError<2>("%1$9.*2$", err_term); + // Non-parity between tinyformat and ConstevalFormatString. + // tinyformat throws but ConstevalFormatString does not. + BOOST_CHECK_EXCEPTION(tfm::format(ConstevalFormatString<1>{"%n"}, 0), tfm::format_error, + HasReason{"tinyformat: %n conversion spec not supported"}); + BOOST_CHECK_EXCEPTION(tfm::format(ConstevalFormatString<2>{"%*s"}, "hi", "hi"), tfm::format_error, + HasReason{"tinyformat: Cannot convert from argument type to integer for use as variable width or precision"}); + BOOST_CHECK_EXCEPTION(tfm::format(ConstevalFormatString<2>{"%.*s"}, "hi", "hi"), tfm::format_error, + HasReason{"tinyformat: Cannot convert from argument type to integer for use as variable width or precision"}); + // Ensure that tinyformat throws if format string contains wrong number // of specifiers. PassFmt relies on this to verify tinyformat successfully // formats the strings, and will need to be updated if tinyformat is changed diff --git a/test/lint/run-lint-format-strings.py b/test/lint/run-lint-format-strings.py index 4402ec2d57..0e08c9f974 100755 --- a/test/lint/run-lint-format-strings.py +++ b/test/lint/run-lint-format-strings.py @@ -15,6 +15,8 @@ FALSE_POSITIVES = [ ("src/clientversion.cpp", "strprintf(_(COPYRIGHT_HOLDERS), COPYRIGHT_HOLDERS_SUBSTITUTION)"), ("src/test/translation_tests.cpp", "strprintf(format, arg)"), + ("src/test/util_string_tests.cpp", 'tfm::format(ConstevalFormatString<2>{"%*s"}, "hi", "hi")'), + ("src/test/util_string_tests.cpp", 'tfm::format(ConstevalFormatString<2>{"%.*s"}, "hi", "hi")'), ] From c93bf0e6e2cd0e9dc979ed996e41cc1034bc1ad1 Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Thu, 5 Dec 2024 14:50:06 +0100 Subject: [PATCH 13/22] test: Add missing %c character test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Proves tinyformat doesn't trigger an exception for \0 characters. Co-Authored-By: Lőrinc --- src/test/util_string_tests.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp index e40bef6379..340c650fe3 100644 --- a/src/test/util_string_tests.cpp +++ b/src/test/util_string_tests.cpp @@ -45,6 +45,7 @@ BOOST_AUTO_TEST_CASE(ConstevalFormatString_NumSpec) PassFmt<0>(""); PassFmt<0>("%%"); PassFmt<1>("%s"); + PassFmt<1>("%c"); PassFmt<0>("%%s"); PassFmt<0>("s%%"); PassFmt<1>("%%%s"); From f6496a838828aa6d5a2a40380c2338fda2e0d812 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 20 Nov 2024 11:24:30 +0000 Subject: [PATCH 14/22] guix: disable gcov in base-linux-gcc In a `x86_64-linux-gnu` build, this drops: ```bash x86_64-linux-gnu/bin/x86_64-linux-gnu-gcov x86_64-linux-gnu/bin/x86_64-linux-gnu-gcov-dump x86_64-linux-gnu/bin/x86_64-linux-gnu-gcov-tool x86_64-linux-gnu/lib/gcc/x86_64-linux-gnu/12.4.0: libgcov.a ``` For mingw-w64-gcc, `--disable-gcov` is currently passed for this target in Guix, due to issues with mingw-w64, see https://github.com/fanquake/guix/blob/8bed031e58c9cf895d243790e7c80808b0075de7/gnu/packages/gcc.scm#L99-L102. However we'll add it in any case, in case it's re-enabled in future, when the underlying issues are fixed. --- contrib/guix/manifest.scm | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contrib/guix/manifest.scm b/contrib/guix/manifest.scm index eccb408559..8854cfef30 100644 --- a/contrib/guix/manifest.scm +++ b/contrib/guix/manifest.scm @@ -420,6 +420,7 @@ inspecting signatures in Mach-O binaries.") ;; https://gcc.gnu.org/install/configure.html (list "--enable-threads=posix", "--enable-default-ssp=yes", + "--disable-gcov", building-on))))))) (define-public linux-base-gcc @@ -435,6 +436,7 @@ inspecting signatures in Mach-O binaries.") "--enable-default-pie=yes", "--enable-standard-branch-protection=yes", "--enable-cet=yes", + "--disable-gcov", building-on))) ((#:phases phases) `(modify-phases ,phases From bb7e686341e437b2e7aae887827710918c00ae0f Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 9 Dec 2024 17:01:10 +0000 Subject: [PATCH 15/22] fuzz: add cstdlib to FuzzedDataProvider Same as https://github.com/llvm/llvm-project/pull/113951. Avoids compile failures under clang-20 & `D_LIBCPP_REMOVE_TRANSITIVE_INCLUDES`: ```bash In file included from /bitcoin/src/test/fuzz/addition_overflow.cpp:5: /bitcoin/src/test/fuzz/FuzzedDataProvider.h:209:5: error: use of undeclared identifier 'abort' 209 | abort(); | ^ /bitcoin/src/test/fuzz/FuzzedDataProvider.h:250:5: error: use of undeclared identifier 'abort' 250 | abort(); ``` --- src/test/fuzz/FuzzedDataProvider.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/fuzz/FuzzedDataProvider.h b/src/test/fuzz/FuzzedDataProvider.h index 5903ed8379..e57b95b630 100644 --- a/src/test/fuzz/FuzzedDataProvider.h +++ b/src/test/fuzz/FuzzedDataProvider.h @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include From 62b2d23edbad76c1d68bb8c0fc8af54e7c0d7dc8 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 9 Dec 2024 14:55:38 -0500 Subject: [PATCH 16/22] wallet: Migrate non-HD keys to combo() descriptor Non-HD keys in legacy wallets without a HD seed ID were being migrated to separate pk(), pkh(), sh(wpkh()), and wpkh() descriptors for each key. These could be more compactly represented as combo() descriptors, so migration should make combo() for them. It is possible that existing non-HD wallets that were migrated, or wallets that started blank and had private keys imported into them have run into this issue. However, as the 4 descriptors produce the same output scripts as the single combo(), so any previously migrated wallets are not missing any output scripts. The only observable difference should be performance related, and the wallet size on disk. --- src/wallet/scriptpubkeyman.cpp | 2 +- test/functional/wallet_migration.py | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 62384056dc..23e2257b1e 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -1799,7 +1799,7 @@ std::optional LegacyDataSPKM::MigrateToDescriptor() keyid_it++; continue; } - if (m_hd_chain.seed_id == meta.hd_seed_id || m_inactive_hd_chains.count(meta.hd_seed_id) > 0) { + if (!meta.hd_seed_id.IsNull() && (m_hd_chain.seed_id == meta.hd_seed_id || m_inactive_hd_chains.count(meta.hd_seed_id) > 0)) { keyid_it = keyids.erase(keyid_it); continue; } diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index fd6ec16065..62b4756a8f 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -1032,15 +1032,11 @@ def test_manual_keys_import(self): # There should be descriptors containing the imported key for: pk(), pkh(), sh(wpkh()), wpkh() key_origin = hash160(pubkey)[:4].hex() pubkey_hex = pubkey.hex() - pk_desc = descsum_create(f'pk([{key_origin}]{pubkey_hex})') - pkh_desc = descsum_create(f'pkh([{key_origin}]{pubkey_hex})') - sh_wpkh_desc = descsum_create(f'sh(wpkh([{key_origin}]{pubkey_hex}))') - wpkh_desc = descsum_create(f'wpkh([{key_origin}]{pubkey_hex})') - expected_descs = [pk_desc, pkh_desc, sh_wpkh_desc, wpkh_desc] + combo_desc = descsum_create(f"combo([{key_origin}]{pubkey_hex})") # Verify all expected descriptors were migrated migrated_desc = [item['desc'] for item in wallet.listdescriptors()['descriptors'] if pubkey.hex() in item['desc']] - assert_equal(expected_descs, migrated_desc) + assert_equal([combo_desc], migrated_desc) wallet.unloadwallet() ###################################################### From b7ec69c25cf316c21bafa075dd895bf90dc4e59b Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 10 Dec 2024 15:20:46 +0000 Subject: [PATCH 17/22] depends: add -g to *BSD_debug flags To match the other HOST_debug_flags. --- depends/hosts/freebsd.mk | 2 +- depends/hosts/netbsd.mk | 2 +- depends/hosts/openbsd.mk | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/depends/hosts/freebsd.mk b/depends/hosts/freebsd.mk index 8cef32e231..009d215f82 100644 --- a/depends/hosts/freebsd.mk +++ b/depends/hosts/freebsd.mk @@ -4,7 +4,7 @@ freebsd_CXXFLAGS=-pipe -std=$(CXX_STANDARD) freebsd_release_CFLAGS=-O2 freebsd_release_CXXFLAGS=$(freebsd_release_CFLAGS) -freebsd_debug_CFLAGS=-O1 +freebsd_debug_CFLAGS=-O1 -g freebsd_debug_CXXFLAGS=$(freebsd_debug_CFLAGS) ifeq (86,$(findstring 86,$(build_arch))) diff --git a/depends/hosts/netbsd.mk b/depends/hosts/netbsd.mk index 16dff92d42..838b58e7ba 100644 --- a/depends/hosts/netbsd.mk +++ b/depends/hosts/netbsd.mk @@ -12,7 +12,7 @@ netbsd_CXXFLAGS=$(netbsd_CFLAGS) netbsd_release_CFLAGS=-O2 netbsd_release_CXXFLAGS=$(netbsd_release_CFLAGS) -netbsd_debug_CFLAGS=-O1 +netbsd_debug_CFLAGS=-O1 -g netbsd_debug_CXXFLAGS=$(netbsd_debug_CFLAGS) ifeq (86,$(findstring 86,$(build_arch))) diff --git a/depends/hosts/openbsd.mk b/depends/hosts/openbsd.mk index 63f6d73d55..53595689b6 100644 --- a/depends/hosts/openbsd.mk +++ b/depends/hosts/openbsd.mk @@ -4,7 +4,7 @@ openbsd_CXXFLAGS=-pipe -std=$(CXX_STANDARD) openbsd_release_CFLAGS=-O2 openbsd_release_CXXFLAGS=$(openbsd_release_CFLAGS) -openbsd_debug_CFLAGS=-O1 +openbsd_debug_CFLAGS=-O1 -g openbsd_debug_CXXFLAGS=$(openbsd_debug_CFLAGS) ifeq (86,$(findstring 86,$(build_arch))) From 015aad8d6a69c3bceb124ae7cf87990531d15eef Mon Sep 17 00:00:00 2001 From: RiceChuan Date: Thu, 12 Dec 2024 16:36:06 +0800 Subject: [PATCH 18/22] docs: remove repetitive words Signed-off-by: RiceChuan --- src/rpc/blockchain.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 6a57a3c487..41673ee21d 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -2942,7 +2942,7 @@ static RPCHelpMan dumptxoutset() return RPCHelpMan{ "dumptxoutset", "Write the serialized UTXO set to a file. This can be used in loadtxoutset afterwards if this snapshot height is supported in the chainparams as well.\n\n" - "Unless the the \"latest\" type is requested, the node will roll back to the requested height and network activity will be suspended during this process. " + "Unless the \"latest\" type is requested, the node will roll back to the requested height and network activity will be suspended during this process. " "Because of this it is discouraged to interact with the node in any other way during the execution of this call to avoid inconsistent results and race conditions, particularly RPCs that interact with blockstorage.\n\n" "This call may take several minutes. Make sure to use no RPC timeout (bitcoin-cli -rpcclienttimeout=0)", { From fa47baa03bcfcf44fb2ed05f009a32d32f860c45 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 12 Dec 2024 09:24:03 +0100 Subject: [PATCH 19/22] ci: Bump centos gcc --- ci/test/00_setup_env_i686_centos.sh | 6 +++--- ci/test/03_test_script.sh | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/ci/test/00_setup_env_i686_centos.sh b/ci/test/00_setup_env_i686_centos.sh index f63adce610..22657742d7 100755 --- a/ci/test/00_setup_env_i686_centos.sh +++ b/ci/test/00_setup_env_i686_centos.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # -# Copyright (c) 2020-2022 The Bitcoin Core developers +# Copyright (c) 2020-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. @@ -9,9 +9,9 @@ export LC_ALL=C.UTF-8 export HOST=i686-pc-linux-gnu export CONTAINER_NAME=ci_i686_centos export CI_IMAGE_NAME_TAG="quay.io/centos/amd64:stream9" -export CI_BASE_PACKAGES="gcc-c++ glibc-devel.x86_64 libstdc++-devel.x86_64 glibc-devel.i686 libstdc++-devel.i686 ccache make git python3 python3-pip which patch lbzip2 xz procps-ng dash rsync coreutils bison e2fsprogs cmake" +export STREAM_GCC_V="12" +export CI_BASE_PACKAGES="gcc-toolset-${STREAM_GCC_V}-gcc-c++ glibc-devel.x86_64 gcc-toolset-${STREAM_GCC_V}-libstdc++-devel.x86_64 glibc-devel.i686 gcc-toolset-${STREAM_GCC_V}-libstdc++-devel.i686 ccache make git python3 python3-pip which patch lbzip2 xz procps-ng dash rsync coreutils bison e2fsprogs cmake" export PIP_PACKAGES="pyzmq" export GOAL="install" -export NO_WERROR=1 # Suppress error: #warning _FORTIFY_SOURCE > 2 is treated like 2 on this platform [-Werror=cpp] export BITCOIN_CONFIG="-DWITH_ZMQ=ON -DBUILD_GUI=ON -DREDUCE_EXPORTS=ON" export CONFIG_SHELL="/bin/dash" diff --git a/ci/test/03_test_script.sh b/ci/test/03_test_script.sh index f9064d177a..6e77a8927c 100755 --- a/ci/test/03_test_script.sh +++ b/ci/test/03_test_script.sh @@ -93,6 +93,8 @@ fi if [ -z "$NO_DEPENDS" ]; then if [[ $CI_IMAGE_NAME_TAG == *centos* ]]; then SHELL_OPTS="CONFIG_SHELL=/bin/dash" + # shellcheck disable=SC1090 + source "/opt/rh/gcc-toolset-${STREAM_GCC_V}/enable" else SHELL_OPTS="CONFIG_SHELL=" fi From e2d3372e558d227aa6dd604b67724685d62e547e Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Wed, 11 Dec 2024 23:10:11 +0100 Subject: [PATCH 20/22] lint: Disable signature output in git log Necessary for users that have signature output enabled by default, since the script would stumble on them and error out. --- test/lint/lint-git-commit-check.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/lint/lint-git-commit-check.py b/test/lint/lint-git-commit-check.py index 5dc30cc755..f5636f22ec 100755 --- a/test/lint/lint-git-commit-check.py +++ b/test/lint/lint-git-commit-check.py @@ -36,10 +36,10 @@ def main(): assert os.getenv("COMMIT_RANGE") # E.g. COMMIT_RANGE='HEAD~n..HEAD' commit_range = os.getenv("COMMIT_RANGE") - commit_hashes = check_output(["git", "log", commit_range, "--format=%H"], text=True, encoding="utf8").splitlines() + commit_hashes = check_output(["git", "-c", "log.showSignature=false", "log", commit_range, "--format=%H"], text=True, encoding="utf8").splitlines() for hash in commit_hashes: - commit_info = check_output(["git", "log", "--format=%B", "-n", "1", hash], text=True, encoding="utf8").splitlines() + commit_info = check_output(["git", "-c", "log.showSignature=false", "log", "--format=%B", "-n", "1", hash], text=True, encoding="utf8").splitlines() if len(commit_info) >= 2: if commit_info[1]: print(f"The subject line of commit hash {hash} is followed by a non-empty line. Subject lines should always be followed by a blank line.") From df27ee9f024f69a8bdac8b0ee3bd7882f6a54294 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Thu, 12 Dec 2024 10:27:24 +0000 Subject: [PATCH 21/22] refactor: Fix "modernize-use-starts-ends-with" clang-tidy warning --- src/ipc/process.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ipc/process.cpp b/src/ipc/process.cpp index 432c365d8f..bdc541b654 100644 --- a/src/ipc/process.cpp +++ b/src/ipc/process.cpp @@ -72,7 +72,7 @@ static bool ParseAddress(std::string& address, struct sockaddr_un& addr, std::string& error) { - if (address.compare(0, 4, "unix") == 0 && (address.size() == 4 || address[4] == ':')) { + if (address == "unix" || address.starts_with("unix:")) { fs::path path; if (address.size() <= 5) { path = data_dir / fs::PathFromString(strprintf("%s.sock", RemovePrefixView(dest_exe_name, "bitcoin-"))); From 5cd9e95eea12fddd2a1a668d509f0a5b6e267516 Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 12 Dec 2024 17:14:42 +0000 Subject: [PATCH 22/22] depends: update capnproto to 1.0.2 This fixes compilation on FreeBSD. See: https://github.com/capnproto/capnproto/commit/1c19c362b4733023dc38042481a21c22502ed593. --- depends/packages/native_capnp.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/depends/packages/native_capnp.mk b/depends/packages/native_capnp.mk index 484e78d5d9..d6e6b4cadb 100644 --- a/depends/packages/native_capnp.mk +++ b/depends/packages/native_capnp.mk @@ -1,9 +1,9 @@ package=native_capnp -$(package)_version=1.0.1 +$(package)_version=1.0.2 $(package)_download_path=https://capnproto.org/ $(package)_download_file=capnproto-c++-$($(package)_version).tar.gz $(package)_file_name=capnproto-cxx-$($(package)_version).tar.gz -$(package)_sha256_hash=0f7f4b8a76a2cdb284fddef20de8306450df6dd031a47a15ac95bc43c3358e09 +$(package)_sha256_hash=9057dbc0223366b74bbeca33a05de164a229b0377927f1b7ef3828cdd8cb1d7e define $(package)_set_vars $(package)_config_opts := -DBUILD_TESTING=OFF