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 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 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))) 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 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) diff --git a/src/chainparamsbase.cpp b/src/chainparamsbase.cpp index 99619e9aee..d5ba27382e 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("", 8336, 8536); + return std::make_unique("", 8336); case ChainType::TESTNET: - return std::make_unique("testnet3", 18336, 18536); + return std::make_unique("testnet3", 18336); case ChainType::TESTNET4: - return std::make_unique("testnet4", 48336, 48536); + return std::make_unique("testnet4", 48336); case ChainType::SIGNET: - return std::make_unique("signet", 38336, 38536); + return std::make_unique("signet", 38336); 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 b6c73a8ad7..03e79fae7f 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -530,7 +530,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); @@ -557,7 +557,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 @@ -1800,7 +1800,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"); @@ -1823,8 +1823,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); @@ -1878,6 +1888,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 " @@ -1902,7 +1914,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; @@ -1938,7 +1950,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/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/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-"))); diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index c882e4c3e3..c8d6b7658d 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -435,6 +435,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/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 9d94419e97..ddf6938147 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -3045,7 +3045,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)", { 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 diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp index fbab9e7aba..340c650fe3 100644 --- a/src/test/util_string_tests.cpp +++ b/src/test/util_string_tests.cpp @@ -8,21 +8,36 @@ #include using namespace util; +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 -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)); + + // 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 -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) @@ -30,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"); @@ -110,6 +126,24 @@ 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); + + // 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 + // 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() diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp index 8d14b6ce2e..407d6085aa 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/src/validation.cpp b/src/validation.cpp index 5b028df5f7..50be5094d6 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4843,6 +4843,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; } diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 576752a777..12a12c9d62 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -1805,7 +1805,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/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 1dc75ee0f2..054b6be6ae 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4202,7 +4202,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) { 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/rpc_net.py b/test/functional/rpc_net.py index b0ca08346d..e8bbf52400 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): diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index c0bd559baf..16c18e364e 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -222,10 +222,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: diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 4f0f03cd08..0e5eb4ee4c 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -341,6 +341,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', diff --git a/test/functional/wallet_backwards_compatibility.py b/test/functional/wallet_backwards_compatibility.py index 775786fbb1..62db32d806 100755 --- a/test/functional/wallet_backwards_compatibility.py +++ b/test/functional/wallet_backwards_compatibility.py @@ -169,6 +169,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) diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index 5abb43e3b9..62b4756a8f 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -19,7 +19,8 @@ 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 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, assert_raises_rpc_error, @@ -27,6 +28,7 @@ ) from test_framework.wallet_util import ( get_generate_key, + generate_keypair, ) @@ -1006,6 +1008,57 @@ 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 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() + 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([combo_desc], 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] @@ -1032,6 +1085,9 @@ def run_test(self): self.test_avoidreuse() self.test_preserve_tx_extra_info() self.test_blank() + self.test_migrate_simple_watch_only() + self.test_manual_keys_import() + if __name__ == '__main__': WalletMigrationTest(__file__).main() 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.") 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")'), ]