From 98868633d1d5cd2591b9613545bd2ce2e81af212 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Wed, 12 Jan 2022 22:02:21 +0000 Subject: [PATCH 01/21] Bugfix: configure: bitcoin-{cli,tx,util} don't need UPnP, NAT-PMP, or ZMQ --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 9b080eb0c8..b9567c9780 100644 --- a/configure.ac +++ b/configure.ac @@ -1349,7 +1349,7 @@ if test "$use_usdt" != "no"; then ) fi -if test "$build_bitcoin_cli$build_bitcoin_tx$build_bitcoin_util$build_bitcoind$bitcoin_enable_qt$use_bench$use_tests" = "nonononononono"; then +if test "$build_bitcoind$bitcoin_enable_qt$use_bench$use_tests" = "nononono"; then use_upnp=no use_natpmp=no use_zmq=no From e049fd76f0d57c1e6400fbfbaf4cc6ebe540f16f Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Tue, 5 Jul 2022 23:41:38 +0000 Subject: [PATCH 02/21] Bugfix: Check for readlink buffer overflow and handle gracefully If readlink returns the size of the buffer, an overflow may have (safely) occurred. Pass a buffer size of MAX_PATH+1 (the size of the actual buffer) to detect this scenario. --- src/qt/guiutil.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index 558d4f108c..cb8be5b9ba 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -743,9 +743,10 @@ bool SetStartOnSystemStartup(bool fAutoStart) else { char pszExePath[MAX_PATH+1]; - ssize_t r = readlink("/proc/self/exe", pszExePath, sizeof(pszExePath) - 1); - if (r == -1) + ssize_t r = readlink("/proc/self/exe", pszExePath, sizeof(pszExePath)); + if (r == -1 || r > MAX_PATH) { return false; + } pszExePath[r] = '\0'; fs::create_directories(GetAutostartDir()); From fa3ea81c3e7d962715788ab5525958a532c51414 Mon Sep 17 00:00:00 2001 From: MacroFake Date: Mon, 30 May 2022 16:10:32 +0200 Subject: [PATCH 03/21] refactor: Add LIFETIMEBOUND / -Wdangling-gsl to Assert() --- src/test/util_tests.cpp | 5 +++++ src/util/check.h | 5 +++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index fda56ccff7..5766fff92d 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -122,6 +122,11 @@ BOOST_AUTO_TEST_CASE(util_check) // Check nested Asserts BOOST_CHECK_EQUAL(Assert((Assert(x).test() ? 3 : 0)), 3); + + // Check -Wdangling-gsl does not trigger when copying the int. (It would + // trigger on "const int&") + const int nine{*Assert(std::optional{9})}; + BOOST_CHECK_EQUAL(9, nine); } BOOST_AUTO_TEST_CASE(util_criticalsection) diff --git a/src/util/check.h b/src/util/check.h index aca957925a..49f07de9dd 100644 --- a/src/util/check.h +++ b/src/util/check.h @@ -9,6 +9,7 @@ #include #endif +#include #include #include @@ -24,7 +25,7 @@ class NonFatalCheckError : public std::runtime_error /** Helper for CHECK_NONFATAL() */ template -T&& inline_check_non_fatal(T&& val, const char* file, int line, const char* func, const char* assertion) +T&& inline_check_non_fatal(LIFETIMEBOUND T&& val, const char* file, int line, const char* func, const char* assertion) { if (!(val)) { throw NonFatalCheckError( @@ -56,7 +57,7 @@ void assertion_fail(const char* file, int line, const char* func, const char* as /** Helper for Assert()/Assume() */ template -T&& inline_assertion_check(T&& val, [[maybe_unused]] const char* file, [[maybe_unused]] int line, [[maybe_unused]] const char* func, [[maybe_unused]] const char* assertion) +T&& inline_assertion_check(LIFETIMEBOUND T&& val, [[maybe_unused]] const char* file, [[maybe_unused]] int line, [[maybe_unused]] const char* func, [[maybe_unused]] const char* assertion) { if constexpr (IS_ASSERT #ifdef ABORT_ON_FAILED_ASSUME From afbcd227dd0889002eeeec6148538ed2e2c9aadb Mon Sep 17 00:00:00 2001 From: fanquake Date: Fri, 21 Oct 2022 16:37:11 +0800 Subject: [PATCH 04/21] build: remove threadinterrupt from libbitcoinkernel --- src/Makefile.am | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Makefile.am b/src/Makefile.am index 70a0ca8915..85038232f4 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -933,7 +933,6 @@ libbitcoinkernel_la_SOURCES = \ support/cleanse.cpp \ support/lockedpool.cpp \ sync.cpp \ - threadinterrupt.cpp \ txdb.cpp \ txmempool.cpp \ uint256.cpp \ From e9f1d8c27261070d209a28570ad36fe91531cdd2 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sun, 23 Oct 2022 15:00:20 +0100 Subject: [PATCH 05/21] Squashed 'src/minisketch/' changes from 47f0a2d26f..a571ba20f9 a571ba20f9 Merge sipa/minisketch#68: Add missed `#include ` b9a7f7e2bc Merge sipa/minisketch#69: refactor: Drop unused `total` local variables 8a5af94edc Merge sipa/minisketch#70: build: Remove `-Qunused-arguments` workaround for clang + ccache c36f1f03a3 Merge sipa/minisketch#72: Fix MSVC implementation of `CountBits()` function 0078bedda6 Ignore `HAVE_CLZ` macro when building with MSVC 1c772918c4 Fix MSVC implementation of `CountBits()` function 98f87c55f4 build: Remove `-Qunused-arguments` workaround for clang + ccache 11a1e25c81 refactor: Drop unused `total` local variables ed6c8fcfd9 Add missed `#include ` git-subtree-dir: src/minisketch git-subtree-split: a571ba20f9dd1accab6a2309d066369878042ca6 --- configure.ac | 3 --- src/bench.cpp | 4 ---- src/int_utils.h | 24 ++++++++++++------------ src/test.cpp | 1 + 4 files changed, 13 insertions(+), 19 deletions(-) diff --git a/configure.ac b/configure.ac index 9dc66e7fd2..83910448a2 100644 --- a/configure.ac +++ b/configure.ac @@ -124,9 +124,6 @@ if test "x$use_ccache" != "xno"; then fi AC_MSG_RESULT($use_ccache) fi -if test "x$use_ccache" = "xyes"; then - AX_CHECK_COMPILE_FLAG([-Qunused-arguments],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Qunused-arguments"],,[[$CXXFLAG_WERROR]]) -fi VERIFY_DEFINES=-DMINISKETCH_VERIFY RELEASE_DEFINES= diff --git a/src/bench.cpp b/src/bench.cpp index f55944a448..dc44379fdb 100644 --- a/src/bench.cpp +++ b/src/bench.cpp @@ -62,13 +62,11 @@ int main(int argc, char** argv) { if (!states[0]) { printf(" -\t"); } else { - double total = 0.0; for (auto& state : states) { auto start = std::chrono::steady_clock::now(); minisketch_decode(state, 2 * syndromes, roots.data()); auto stop = std::chrono::steady_clock::now(); std::chrono::duration dur(stop - start); - total += dur.count(); benches.push_back(dur.count()); } std::sort(benches.begin(), benches.end()); @@ -98,7 +96,6 @@ int main(int argc, char** argv) { if (!states[0]) { printf(" -\t"); } else { - double total = 0.0; for (auto& state : states) { auto start = std::chrono::steady_clock::now(); for (auto val : data) { @@ -106,7 +103,6 @@ int main(int argc, char** argv) { } auto stop = std::chrono::steady_clock::now(); std::chrono::duration dur(stop - start); - total += dur.count(); benches.push_back(dur.count()); } std::sort(benches.begin(), benches.end()); diff --git a/src/int_utils.h b/src/int_utils.h index 62b2c38a29..d21ba56f33 100644 --- a/src/int_utils.h +++ b/src/int_utils.h @@ -129,17 +129,7 @@ constexpr inline I Mask() { return ((I((I(-1)) << (std::numeric_limits::digit /** Compute the smallest power of two that is larger than val. */ template static inline int CountBits(I val, int max) { -#ifdef HAVE_CLZ - (void)max; - if (val == 0) return 0; - if (std::numeric_limits::digits >= std::numeric_limits::digits) { - return std::numeric_limits::digits - __builtin_clz(val); - } else if (std::numeric_limits::digits >= std::numeric_limits::digits) { - return std::numeric_limits::digits - __builtin_clzl(val); - } else { - return std::numeric_limits::digits - __builtin_clzll(val); - } -#elif _MSC_VER +#ifdef _MSC_VER (void)max; unsigned long index; unsigned char ret; @@ -149,7 +139,17 @@ static inline int CountBits(I val, int max) { ret = _BitScanReverse64(&index, val); } if (!ret) return 0; - return index; + return index + 1; +#elif HAVE_CLZ + (void)max; + if (val == 0) return 0; + if (std::numeric_limits::digits >= std::numeric_limits::digits) { + return std::numeric_limits::digits - __builtin_clz(val); + } else if (std::numeric_limits::digits >= std::numeric_limits::digits) { + return std::numeric_limits::digits - __builtin_clzl(val); + } else { + return std::numeric_limits::digits - __builtin_clzll(val); + } #else while (max && (val >> (max - 1) == 0)) --max; return max; diff --git a/src/test.cpp b/src/test.cpp index 417937ea5f..85b9e9e396 100644 --- a/src/test.cpp +++ b/src/test.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include "../include/minisketch.h" From 884304e6c6a250719a64752fe52d1283cd5e36ab Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 24 Oct 2022 13:36:04 +0100 Subject: [PATCH 06/21] test: Make `system_tests/run_command` locale agnostic --- src/test/system_tests.cpp | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp index 11f4be7fef..d5b65b9c08 100644 --- a/src/test/system_tests.cpp +++ b/src/test/system_tests.cpp @@ -51,15 +51,9 @@ BOOST_AUTO_TEST_CASE(run_command) } { // An invalid command is handled by Boost -#ifdef WIN32 - const std::string expected{"The system cannot find the file specified."}; -#else - const std::string expected{"No such file or directory"}; -#endif BOOST_CHECK_EXCEPTION(RunCommandParseJSON("invalid_command"), boost::process::process_error, [&](const boost::process::process_error& e) { - const std::string what(e.what()); - BOOST_CHECK(what.find("RunCommandParseJSON error:") == std::string::npos); - BOOST_CHECK(what.find(expected) != std::string::npos); + BOOST_CHECK(std::string(e.what()).find("RunCommandParseJSON error:") == std::string::npos); + BOOST_CHECK_EQUAL(e.code().value(), 2); return true; }); } From e891aabf5a4992a65b9c5ae8606f8dd08515b310 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Mon, 24 Oct 2022 22:03:48 +0100 Subject: [PATCH 07/21] [net processing] Fixup TryLowWorkHeadersSync comment --- src/net_processing.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 3f344005bc..544acede03 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -638,9 +638,8 @@ private: * @param[in] chain_start_header Where these headers connect in our index. * @param[in,out] headers The headers to be processed. * - * @return True if chain was low work and a headers sync was - * initiated (and headers will be empty after calling); false - * otherwise. + * @return True if chain was low work (headers will be empty after + * calling); false otherwise. */ bool TryLowWorkHeadersSync(Peer& peer, CNode& pfrom, const CBlockIndex* chain_start_header, From 784b02319128988038d4bd82f05736be22f14ee9 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Wed, 26 Oct 2022 11:11:36 +0100 Subject: [PATCH 08/21] [net processing] Simplify use of IsContinuationOfLowWorkHeadersSync in TryLowWorkHeaderSync `m_headers_sync` is already reset in IsContinuationOfLowWorkHeadersSync if there is a failure, so there is no need to also reset in TryLowWorkHeaderSync. --- src/net_processing.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 544acede03..4774d58d37 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2562,14 +2562,10 @@ bool PeerManagerImpl::TryLowWorkHeadersSync(Peer& peer, CNode& pfrom, const CBlo peer.m_headers_sync.reset(new HeadersSyncState(peer.m_id, m_chainparams.GetConsensus(), chain_start_header, minimum_chain_work)); - // Now a HeadersSyncState object for tracking this synchronization is created, - // process the headers using it as normal. - if (!IsContinuationOfLowWorkHeadersSync(peer, pfrom, headers)) { - // Something went wrong, reset the headers sync. - peer.m_headers_sync.reset(nullptr); - LOCK(m_headers_presync_mutex); - m_headers_presync_stats.erase(peer.m_id); - } + // Now a HeadersSyncState object for tracking this synchronization + // is created, process the headers using it as normal. Failures are + // handled inside of IsContinuationOfLowWorkHeadersSync. + (void)IsContinuationOfLowWorkHeadersSync(peer, pfrom, headers); } else { LogPrint(BCLog::NET, "Ignoring low-work chain (height=%u) from peer=%d\n", chain_start_header->nHeight + headers.size(), pfrom.GetId()); } From fa24239a1c2281f61ab70a62228e88f4c7e72701 Mon Sep 17 00:00:00 2001 From: MacroFake Date: Wed, 26 Oct 2022 18:17:01 +0200 Subject: [PATCH 09/21] net: Avoid SetTxRelay for feeler connections --- src/net.h | 6 ++---- src/net_processing.cpp | 8 ++++---- test/functional/p2p_sendtxrcncl.py | 11 +++++++++++ 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/net.h b/src/net.h index 11bfc4c9fb..245f14731b 100644 --- a/src/net.h +++ b/src/net.h @@ -489,10 +489,8 @@ public: /** Whether this peer provides all services that we want. Used for eviction decisions */ std::atomic_bool m_has_all_wanted_services{false}; - /** Whether we should relay transactions to this peer (their version - * message did not include fRelay=false and this is not a block-relay-only - * connection). This only changes from false to true. It will never change - * back to false. Used only in inbound eviction logic. */ + /** Whether we should relay transactions to this peer. This only changes + * from false to true. It will never change back to false. */ std::atomic_bool m_relays_txs{false}; /** Whether this peer has loaded a bloom filter. Used only in inbound diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 363f2fde71..18992a6fb2 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -395,9 +395,7 @@ struct Peer { private: Mutex m_tx_relay_mutex; - /** Transaction relay data. Will be a nullptr if we're not relaying - * transactions with this peer (e.g. if it's a block-relay-only peer or - * the peer has sent us fRelay=false with bloom filters disabled). */ + /** Transaction relay data. May be a nullptr. */ std::unique_ptr m_tx_relay GUARDED_BY(m_tx_relay_mutex); }; @@ -3261,12 +3259,14 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } peer->m_starting_height = starting_height; - // We only initialize the Peer::TxRelay m_relay_txs data structure if: + // Only initialize the Peer::TxRelay m_relay_txs data structure if: // - this isn't an outbound block-relay-only connection, and + // - this isn't an outbound feeler connection, and // - fRelay=true (the peer wishes to receive transaction announcements) // or we're offering NODE_BLOOM to this peer. NODE_BLOOM means that // the peer may turn on transaction relay later. if (!pfrom.IsBlockOnlyConn() && + !pfrom.IsFeelerConn() && (fRelay || (peer->m_our_services & NODE_BLOOM))) { auto* const tx_relay = peer->SetTxRelay(); { diff --git a/test/functional/p2p_sendtxrcncl.py b/test/functional/p2p_sendtxrcncl.py index 14cd815a30..50a5127f25 100755 --- a/test/functional/p2p_sendtxrcncl.py +++ b/test/functional/p2p_sendtxrcncl.py @@ -39,6 +39,12 @@ class SendTxrcnclReceiver(P2PInterface): def on_sendtxrcncl(self, message): self.sendtxrcncl_msg_received = message + +class P2PFeelerReceiver(SendTxrcnclReceiver): + def on_version(self, message): + pass # feeler connections can not send any message other than their own version + + class PeerTrackMsgOrder(P2PInterface): def __init__(self): super().__init__() @@ -163,6 +169,11 @@ class SendTxRcnclTest(BitcoinTestFramework): assert not peer.sendtxrcncl_msg_received peer.peer_disconnect() + self.log.info("SENDTXRCNCL should not be sent if feeler") + peer = self.nodes[0].add_outbound_p2p_connection(P2PFeelerReceiver(), p2p_idx=2, connection_type="feeler") + assert not peer.sendtxrcncl_msg_received + peer.peer_disconnect() + self.log.info('SENDTXRCNCL if block-relay-only triggers a disconnect') peer = self.nodes[0].add_outbound_p2p_connection( PeerNoVerack(), wait_for_verack=False, p2p_idx=3, connection_type="block-relay-only") From ef97b89902f03d710f6111a1d8deaaa87d92a817 Mon Sep 17 00:00:00 2001 From: Jeff Ruane Date: Wed, 26 Oct 2022 05:05:52 -0600 Subject: [PATCH 10/21] Exclude rand from debug log Currently, debug.log is spammed with messages from random.cpp when functional tests are run. These logs are not useful for debugging, and decrease the signal to noise ratio of the logs. --- test/functional/test_framework/test_node.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 2367a9a8fa..83f9f253fd 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -102,6 +102,7 @@ class TestNode(): "-debug", "-debugexclude=libevent", "-debugexclude=leveldb", + "-debugexclude=rand", "-uacomment=testnode%d" % i, ] if use_valgrind: From 180eac0f73b7d96861f9626ebb701d9a46585a5b Mon Sep 17 00:00:00 2001 From: omahs <73983677+omahs@users.noreply.github.com> Date: Thu, 27 Oct 2022 20:59:01 +0200 Subject: [PATCH 11/21] Fix: typos Fix: typos Fix: typos Fix: typos --- doc/tracing.md | 8 ++++---- src/net_processing.cpp | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/tracing.md b/doc/tracing.md index b6e3b9263a..6e60901496 100644 --- a/doc/tracing.md +++ b/doc/tracing.md @@ -76,7 +76,7 @@ the passed message. #### Tracepoint `net:outbound_message` -Is called when a message is send to a peer over the P2P network. Passes +Is called when a message is sent to a peer over the P2P network. Passes information about our peer, the connection and the message as arguments. Arguments passed: @@ -116,7 +116,7 @@ added to and removed (spent) from the cache when we connect a new block. (`chainstate.CoinsTip()`). For example, the RPCs `generateblock` and `getblocktemplate` call `TestBlockValidity()`, which applies the UTXO set changes to a temporary cache. Similarly, mempool consistency checks, which are -frequent on regtest, also apply the the UTXO set changes to a temporary cache. +frequent on regtest, also apply the UTXO set changes to a temporary cache. Changes to the _main_ UTXO cache and to temporary caches trigger the tracepoints. We can't tell if a temporary cache or the _main_ cache was changed. @@ -253,8 +253,8 @@ TRACE6(net, inbound_message, ### Guidelines and best practices -#### Clear motivation and use-case -Tracepoints need a clear motivation and use-case. The motivation should +#### Clear motivation and use case +Tracepoints need a clear motivation and use case. The motivation should outweigh the impact on, for example, code readability. There is no point in adding tracepoints that don't end up being used. diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 363f2fde71..e2d70fc2c8 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2714,7 +2714,7 @@ void PeerManagerImpl::UpdatePeerStateForReceivedHeaders(CNode& pfrom, // This peer has too little work on their headers chain to help // us sync -- disconnect if it is an outbound disconnection // candidate. - // Note: We compare their tip to the minumum chain work (rather than + // Note: We compare their tip to the minimum chain work (rather than // m_chainman.ActiveChain().Tip()) because we won't start block download // until we have a headers chain that has at least // the minimum chain work, even if a peer has a chain past our tip, @@ -3901,7 +3901,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // Note that if we were to be on a chain that forks from the checkpointed // chain, then serving those headers to a peer that has seen the // checkpointed chain would cause that peer to disconnect us. Requiring - // that our chainwork exceed the mimimum chain work is a protection against + // that our chainwork exceed the minimum chain work is a protection against // being fed a bogus chain when we started up for the first time and // getting partitioned off the honest network for serving that chain to // others. From fa29ef00adac6f0842acdd38344820a1ce0e3087 Mon Sep 17 00:00:00 2001 From: MacroFake Date: Fri, 28 Oct 2022 11:14:02 +0200 Subject: [PATCH 12/21] refactor: Silence GCC Wmissing-field-initializers in ChainstateManagerOpts --- src/kernel/chainstatemanager_opts.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/kernel/chainstatemanager_opts.h b/src/kernel/chainstatemanager_opts.h index 020ae24c11..226bb6031e 100644 --- a/src/kernel/chainstatemanager_opts.h +++ b/src/kernel/chainstatemanager_opts.h @@ -31,9 +31,9 @@ struct ChainstateManagerOpts { std::optional check_block_index{}; bool checkpoints_enabled{DEFAULT_CHECKPOINTS_ENABLED}; //! If set, it will override the minimum work we will assume exists on some valid chain. - std::optional minimum_chain_work; + std::optional minimum_chain_work{}; //! If set, it will override the block hash whose ancestors we will assume to have valid scripts without checking them. - std::optional assumed_valid_block; + std::optional assumed_valid_block{}; //! If the tip is older than this, the node is considered to be in initial block download. std::chrono::seconds max_tip_age{DEFAULT_MAX_TIP_AGE}; }; From 8a9f1e4d18e2b94509548af2aa3ad14185793f73 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Thu, 27 Oct 2022 17:56:58 -0400 Subject: [PATCH 13/21] test: fix intermittent failure in rpc_getblockfrompeer.py by adding a sync to make sure the node has received the header before we query it for the block --- test/functional/rpc_getblockfrompeer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/rpc_getblockfrompeer.py b/test/functional/rpc_getblockfrompeer.py index fd4d1992eb..8bd3366e36 100755 --- a/test/functional/rpc_getblockfrompeer.py +++ b/test/functional/rpc_getblockfrompeer.py @@ -100,7 +100,7 @@ class GetBlockFromPeerTest(BitcoinTestFramework): # Connect a P2PInterface to the pruning node and have it submit only the header of the # block that the pruning node has not seen node1_interface = self.nodes[1].add_p2p_connection(P2PInterface()) - node1_interface.send_message(msg_headers([block])) + node1_interface.send_and_ping(msg_headers([block])) # Get the peer id of the P2PInterface from the pruning node node1_peers = self.nodes[1].getpeerinfo() From 201b9a02fd9885ce40999e383cf75b8354d6ef26 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Fri, 28 Oct 2022 15:24:22 -0400 Subject: [PATCH 14/21] test: fix intermittent failure in feature_index_prune.py After syncing the blocks, we didn't check that the indexes have caught up to the tip before manually pruning. This could lead to prune blockers lower thatn the expected height. --- test/functional/feature_index_prune.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/functional/feature_index_prune.py b/test/functional/feature_index_prune.py index bc85e43a57..4b7a50c1c7 100755 --- a/test/functional/feature_index_prune.py +++ b/test/functional/feature_index_prune.py @@ -138,6 +138,7 @@ class FeatureIndexPruneTest(BitcoinTestFramework): self.connect_nodes(i, 3) self.sync_blocks(timeout=300) + self.sync_index(height=2500) for node in self.nodes[:2]: with node.assert_debug_log(['limited pruning to height 2489']): From 0f38524c31da4cf69d8e904569fe56292e4325b9 Mon Sep 17 00:00:00 2001 From: Bitcoin Hodler Date: Sun, 30 Oct 2022 15:14:45 +0000 Subject: [PATCH 15/21] doc: correct deriveaddresses RPC name --- doc/descriptors.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/descriptors.md b/doc/descriptors.md index de92e3dbcf..1baf652f30 100644 --- a/doc/descriptors.md +++ b/doc/descriptors.md @@ -266,6 +266,6 @@ ones. For larger numbers of errors, or other types of errors, there is a roughly 1 in a trillion chance of not detecting the errors. All RPCs in Bitcoin Core will include the checksum in their output. Only -certain RPCs require checksums on input, including `deriveaddress` and +certain RPCs require checksums on input, including `deriveaddresses` and `importmulti`. The checksum for a descriptor without one can be computed using the `getdescriptorinfo` RPC. From 058eb69ce47c17205a41fc667ce66429f09a332a Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 11 Oct 2022 16:03:35 +0800 Subject: [PATCH 16/21] build: add missing event cflags to libbitcoin_util The fact that this is missing is currently masked by the inclusion of BOOST_CPPFLAGS. --- src/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Makefile.am b/src/Makefile.am index 6d10f86d57..19b131a5f4 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -710,6 +710,7 @@ libbitcoin_util_a_SOURCES = \ $(BITCOIN_CORE_H) if USE_LIBEVENT +libbitcoin_util_a_CPPFLAGS += $(EVENT_CFLAGS) libbitcoin_util_a_SOURCES += util/url.cpp endif # From 3a0b352c63db543833e0f919a004cf2c5093fe9c Mon Sep 17 00:00:00 2001 From: fanquake Date: Fri, 14 Oct 2022 13:55:53 +0800 Subject: [PATCH 17/21] refactor: move url.h/cpp from lib util to lib common --- build_msvc/bitcoin-cli/bitcoin-cli.vcxproj | 3 +++ .../libbitcoin_common/libbitcoin_common.vcxproj.in | 1 + .../libbitcoin_util/libbitcoin_util.vcxproj.in | 1 - ci/test/06_script_b.sh | 2 +- src/Makefile.am | 13 +++++++------ src/bitcoin-cli.cpp | 2 +- src/bitcoin-wallet.cpp | 2 +- src/bitcoind.cpp | 2 +- src/{util => common}/url.cpp | 2 +- src/{util => common}/url.h | 6 +++--- src/qt/main.cpp | 2 +- src/test/fuzz/string.cpp | 2 +- src/test/util/setup_common.cpp | 2 +- src/wallet/rpc/util.cpp | 2 +- 14 files changed, 23 insertions(+), 19 deletions(-) rename src/{util => common}/url.cpp (95%) rename src/{util => common}/url.h (79%) diff --git a/build_msvc/bitcoin-cli/bitcoin-cli.vcxproj b/build_msvc/bitcoin-cli/bitcoin-cli.vcxproj index e5e0e978f8..738884fb41 100644 --- a/build_msvc/bitcoin-cli/bitcoin-cli.vcxproj +++ b/build_msvc/bitcoin-cli/bitcoin-cli.vcxproj @@ -15,6 +15,9 @@ {0667528c-d734-4009-adf9-c0d6c4a5a5a6} + + {7c87e378-df58-482e-aa2f-1bc129bc19ce} + {6190199c-6cf4-4dad-bfbd-93fa72a760c1} diff --git a/build_msvc/libbitcoin_common/libbitcoin_common.vcxproj.in b/build_msvc/libbitcoin_common/libbitcoin_common.vcxproj.in index b47d62b295..482e4333f7 100644 --- a/build_msvc/libbitcoin_common/libbitcoin_common.vcxproj.in +++ b/build_msvc/libbitcoin_common/libbitcoin_common.vcxproj.in @@ -8,6 +8,7 @@ StaticLibrary + @SOURCE_FILES@ diff --git a/build_msvc/libbitcoin_util/libbitcoin_util.vcxproj.in b/build_msvc/libbitcoin_util/libbitcoin_util.vcxproj.in index 6ec40461c2..adf4fa0354 100644 --- a/build_msvc/libbitcoin_util/libbitcoin_util.vcxproj.in +++ b/build_msvc/libbitcoin_util/libbitcoin_util.vcxproj.in @@ -8,7 +8,6 @@ StaticLibrary - @SOURCE_FILES@ diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh index c0b7a969c0..89e517d83c 100755 --- a/ci/test/06_script_b.sh +++ b/ci/test/06_script_b.sh @@ -40,6 +40,7 @@ if [ "${RUN_TIDY}" = "true" ]; then ( CI_EXEC run-clang-tidy -quiet "${MAKEJOBS}" ) | grep -C5 "error" export P_CI_DIR="${BASE_BUILD_DIR}/bitcoin-$HOST/" CI_EXEC "python3 ${DIR_IWYU}/include-what-you-use/iwyu_tool.py"\ + " src/common/url.cpp"\ " src/compat"\ " src/dbwrapper.cpp"\ " src/init"\ @@ -68,7 +69,6 @@ if [ "${RUN_TIDY}" = "true" ]; then " src/util/strencodings.cpp"\ " src/util/string.cpp"\ " src/util/syserror.cpp"\ - " src/util/url.cpp"\ " src/zmq"\ " -p . ${MAKEJOBS} -- -Xiwyu --cxx17ns -Xiwyu --mapping_file=${BASE_BUILD_DIR}/bitcoin-$HOST/contrib/devtools/iwyu/bitcoin.core.imp" fi diff --git a/src/Makefile.am b/src/Makefile.am index 19b131a5f4..d50247b52f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -134,6 +134,7 @@ BITCOIN_CORE_H = \ coins.h \ common/bloom.h \ common/run_command.h \ + common/url.h \ compat/assumptions.h \ compat/byteswap.h \ compat/compat.h \ @@ -303,7 +304,6 @@ BITCOIN_CORE_H = \ util/translation.h \ util/types.h \ util/ui_change_type.h \ - util/url.h \ util/vector.h \ validation.h \ validationinterface.h \ @@ -662,6 +662,11 @@ libbitcoin_common_a_SOURCES = \ script/standard.cpp \ warnings.cpp \ $(BITCOIN_CORE_H) + +if USE_LIBEVENT +libbitcoin_common_a_CPPFLAGS += $(EVENT_CFLAGS) +libbitcoin_common_a_SOURCES += common/url.cpp +endif # # util # @@ -708,11 +713,6 @@ libbitcoin_util_a_SOURCES = \ util/time.cpp \ util/tokenpipe.cpp \ $(BITCOIN_CORE_H) - -if USE_LIBEVENT -libbitcoin_util_a_CPPFLAGS += $(EVENT_CFLAGS) -libbitcoin_util_a_SOURCES += util/url.cpp -endif # # cli # @@ -776,6 +776,7 @@ endif bitcoin_cli_LDADD = \ $(LIBBITCOIN_CLI) \ $(LIBUNIVALUE) \ + $(LIBBITCOIN_COMMON) \ $(LIBBITCOIN_UTIL) \ $(LIBBITCOIN_CRYPTO) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 6d77385584..c06488dbe9 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -21,7 +22,6 @@ #include #include #include -#include #include #include diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index d556300ee2..78c1a2060c 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -17,7 +18,6 @@ #include #include #include -#include #include #include diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index 9f81640ddb..d8d4e34e47 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -25,7 +26,6 @@ #include #include #include -#include #include #include diff --git a/src/util/url.cpp b/src/common/url.cpp similarity index 95% rename from src/util/url.cpp rename to src/common/url.cpp index ea9323e666..5200d55096 100644 --- a/src/util/url.cpp +++ b/src/common/url.cpp @@ -2,7 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include +#include #include diff --git a/src/util/url.h b/src/common/url.h similarity index 79% rename from src/util/url.h rename to src/common/url.h index 5a7b11fa04..7bbd8b60de 100644 --- a/src/util/url.h +++ b/src/common/url.h @@ -2,8 +2,8 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#ifndef BITCOIN_UTIL_URL_H -#define BITCOIN_UTIL_URL_H +#ifndef BITCOIN_COMMON_URL_H +#define BITCOIN_COMMON_URL_H #include @@ -11,4 +11,4 @@ using UrlDecodeFn = std::string(const std::string& url_encoded); UrlDecodeFn urlDecode; extern UrlDecodeFn* const URL_DECODE; -#endif // BITCOIN_UTIL_URL_H +#endif // BITCOIN_COMMON_URL_H diff --git a/src/qt/main.cpp b/src/qt/main.cpp index e8f39584ad..45131a1cf5 100644 --- a/src/qt/main.cpp +++ b/src/qt/main.cpp @@ -4,9 +4,9 @@ #include +#include #include #include -#include #include diff --git a/src/test/fuzz/string.cpp b/src/test/fuzz/string.cpp index 94399faf04..f6373351d8 100644 --- a/src/test/fuzz/string.cpp +++ b/src/test/fuzz/string.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -27,7 +28,6 @@ #include #include #include -#include #include #include diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 0d0db176f9..bdcff1076b 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -46,7 +47,6 @@ #include #include #include -#include #include #include #include diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp index 1aa2a87e99..26270f23ed 100644 --- a/src/wallet/rpc/util.cpp +++ b/src/wallet/rpc/util.cpp @@ -4,9 +4,9 @@ #include +#include #include #include -#include #include #include From f1ee974e8e18a6a4fa50e0062d0531936dac32ae Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Tue, 1 Nov 2022 13:48:06 +0100 Subject: [PATCH 18/21] test: remove unused `CHANGE_{XPRV,XPUB}` constants These constants exist since the introduction of the functional test wallet_taproot.py (2667366aaa69447a9de4d819669d254a5ebd4d4b), but they have never been used. --- test/functional/wallet_taproot.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/functional/wallet_taproot.py b/test/functional/wallet_taproot.py index 3c630ba433..41420238d6 100755 --- a/test/functional/wallet_taproot.py +++ b/test/functional/wallet_taproot.py @@ -156,9 +156,6 @@ KEYS = [ } ] -CHANGE_XPRV = "tprv8ZgxMBicQKsPcyDrWwiecVnTtFmfRwbfFqEfR4ZGWvq5aTTwLBWmAm5zrbMcYtb9gQNFfhRfqhhrBG37U3nhmXxEgeEPBJGHAPrHCrAd1WX" -CHANGE_XPUB = "tpubD6NzVbkrYhZ4WSFeQbPF1uSaTHHbbGnZq8qShabZwCdUQwihxaLMMFhs2kidGF2qrRKiQVqw8VoyuTHj1bZqmMXMeciaU1gBjWA1sim2zUB" - def key(hex_key): """Construct an x-only pubkey from its hex representation.""" From 303fb8ff459e67baf97c2ab0b6dd6fa25aff76e3 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Tue, 1 Nov 2022 20:50:51 +0100 Subject: [PATCH 19/21] doc: mention BIP86 in doc/bips.md --- doc/bips.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/bips.md b/doc/bips.md index 25381818e4..1d5c91b8bd 100644 --- a/doc/bips.md +++ b/doc/bips.md @@ -28,6 +28,7 @@ BIPs that are implemented by Bitcoin Core (up-to-date up to **v24.0**): and it is disabled by default at build time since **v0.19.0** ([PR #15584](https://github.com/bitcoin/bitcoin/pull/15584)). It has been removed as of **v0.20.0** ([PR 17165](https://github.com/bitcoin/bitcoin/pull/17165)). * [`BIP 84`](https://github.com/bitcoin/bips/blob/master/bip-0084.mediawiki): The experimental descriptor wallets introduced in **v0.21.0** by default use the Hierarchical Deterministic Wallet derivation proposed by BIP 84. ([PR #16528](https://github.com/bitcoin/bitcoin/pull/16528)) +* [`BIP 86`](https://github.com/bitcoin/bips/blob/master/bip-0086.mediawiki): Descriptor wallets by default use the Hierarchical Deterministic Wallet derivation proposed by BIP 86 since **v23.0** ([PR #22364](https://github.com/bitcoin/bitcoin/pull/22364)). * [`BIP 90`](https://github.com/bitcoin/bips/blob/master/bip-0090.mediawiki): Trigger mechanism for activation of BIPs 34, 65, and 66 has been simplified to block height checks since **v0.14.0** ([PR #8391](https://github.com/bitcoin/bitcoin/pull/8391)). * [`BIP 111`](https://github.com/bitcoin/bips/blob/master/bip-0111.mediawiki): `NODE_BLOOM` service bit added, and enforced for all peer versions as of **v0.13.0** ([PR #6579](https://github.com/bitcoin/bitcoin/pull/6579) and [PR #6641](https://github.com/bitcoin/bitcoin/pull/6641)). * [`BIP 112`](https://github.com/bitcoin/bips/blob/master/bip-0112.mediawiki): The CHECKSEQUENCEVERIFY opcode has been implemented since **v0.12.1** ([PR #7524](https://github.com/bitcoin/bitcoin/pull/7524)), and has been *buried* since **v0.19.0** ([PR #16060](https://github.com/bitcoin/bitcoin/pull/16060)). From 29fa38a41aa1844768d4b90df739eec3e66ea25f Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Thu, 3 Nov 2022 09:26:27 +0100 Subject: [PATCH 20/21] .python-version: bump patch version to 3.6.15 --- .python-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.python-version b/.python-version index 8b7b0b52e5..cd337510be 100644 --- a/.python-version +++ b/.python-version @@ -1 +1 @@ -3.6.12 +3.6.15 From 74d975318a1443aebfbcee36e331df4e54ec1fbe Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Thu, 3 Nov 2022 11:46:07 -0400 Subject: [PATCH 21/21] test: fix intermittent failure in p2p_sendtxrcncl.py Using disconnect_p2ps instead of peer_disconnect makes the node wait for the disconnect to complete. As a result, we can reuse p2p_idx=0 in the add_outbound_p2p_connection calls. --- test/functional/p2p_sendtxrcncl.py | 30 ++++++++++----------- test/functional/test_framework/test_node.py | 4 +++ 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/test/functional/p2p_sendtxrcncl.py b/test/functional/p2p_sendtxrcncl.py index 50a5127f25..fed9832a7d 100755 --- a/test/functional/p2p_sendtxrcncl.py +++ b/test/functional/p2p_sendtxrcncl.py @@ -74,7 +74,7 @@ class SendTxRcnclTest(BitcoinTestFramework): assert not peer.sendtxrcncl_msg_received.initiator assert peer.sendtxrcncl_msg_received.responder assert_equal(peer.sendtxrcncl_msg_received.version, 1) - peer.peer_disconnect() + self.nodes[0].disconnect_p2ps() self.log.info('SENDTXRCNCL should be sent before VERACK') peer = self.nodes[0].add_p2p_connection(PeerTrackMsgOrder(), send_version=True, wait_for_verack=True) @@ -82,7 +82,7 @@ class SendTxRcnclTest(BitcoinTestFramework): verack_index = [i for i, msg in enumerate(peer.messages) if msg.msgtype == b'verack'][0] sendtxrcncl_index = [i for i, msg in enumerate(peer.messages) if msg.msgtype == b'sendtxrcncl'][0] assert(sendtxrcncl_index < verack_index) - peer.peer_disconnect() + self.nodes[0].disconnect_p2ps() self.log.info('SENDTXRCNCL on pre-WTXID version should not be sent') peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=False, wait_for_verack=False) @@ -94,7 +94,7 @@ class SendTxRcnclTest(BitcoinTestFramework): peer.send_message(pre_wtxid_version_msg) peer.wait_for_verack() assert not peer.sendtxrcncl_msg_received - peer.peer_disconnect() + self.nodes[0].disconnect_p2ps() self.log.info('SENDTXRCNCL for fRelay=false should not be sent') peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=False, wait_for_verack=False) @@ -106,7 +106,7 @@ class SendTxRcnclTest(BitcoinTestFramework): peer.send_message(no_txrelay_version_msg) peer.wait_for_verack() assert not peer.sendtxrcncl_msg_received - peer.peer_disconnect() + self.nodes[0].disconnect_p2ps() self.log.info('valid SENDTXRCNCL received') peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=True, wait_for_verack=False) @@ -152,31 +152,31 @@ class SendTxRcnclTest(BitcoinTestFramework): with self.nodes[0].assert_debug_log(['Forget txreconciliation state of peer']): peer.send_message(create_sendtxrcncl_msg()) peer.send_message(msg_verack()) - peer.peer_disconnect() + self.nodes[0].disconnect_p2ps() self.log.info('SENDTXRCNCL sent to an outbound') peer = self.nodes[0].add_outbound_p2p_connection( - SendTxrcnclReceiver(), wait_for_verack=True, p2p_idx=1, connection_type="outbound-full-relay") + SendTxrcnclReceiver(), wait_for_verack=True, p2p_idx=0, connection_type="outbound-full-relay") assert peer.sendtxrcncl_msg_received assert peer.sendtxrcncl_msg_received.initiator assert not peer.sendtxrcncl_msg_received.responder assert_equal(peer.sendtxrcncl_msg_received.version, 1) - peer.peer_disconnect() + self.nodes[0].disconnect_p2ps() self.log.info('SENDTXRCNCL should not be sent if block-relay-only') peer = self.nodes[0].add_outbound_p2p_connection( - SendTxrcnclReceiver(), wait_for_verack=True, p2p_idx=2, connection_type="block-relay-only") + SendTxrcnclReceiver(), wait_for_verack=True, p2p_idx=0, connection_type="block-relay-only") assert not peer.sendtxrcncl_msg_received - peer.peer_disconnect() + self.nodes[0].disconnect_p2ps() self.log.info("SENDTXRCNCL should not be sent if feeler") - peer = self.nodes[0].add_outbound_p2p_connection(P2PFeelerReceiver(), p2p_idx=2, connection_type="feeler") + peer = self.nodes[0].add_outbound_p2p_connection(P2PFeelerReceiver(), p2p_idx=0, connection_type="feeler") assert not peer.sendtxrcncl_msg_received - peer.peer_disconnect() + self.nodes[0].disconnect_p2ps() self.log.info('SENDTXRCNCL if block-relay-only triggers a disconnect') peer = self.nodes[0].add_outbound_p2p_connection( - PeerNoVerack(), wait_for_verack=False, p2p_idx=3, connection_type="block-relay-only") + PeerNoVerack(), wait_for_verack=False, p2p_idx=0, connection_type="block-relay-only") with self.nodes[0].assert_debug_log(["we indicated no tx relay; disconnecting"]): peer.send_message(create_sendtxrcncl_msg(initiator=False)) peer.wait_for_disconnect() @@ -184,7 +184,7 @@ class SendTxRcnclTest(BitcoinTestFramework): self.log.info('SENDTXRCNCL with initiator=1 and responder=0 from outbound triggers a disconnect') sendtxrcncl_wrong_role = create_sendtxrcncl_msg(initiator=True) peer = self.nodes[0].add_outbound_p2p_connection( - PeerNoVerack(), wait_for_verack=False, p2p_idx=4, connection_type="outbound-full-relay") + PeerNoVerack(), wait_for_verack=False, p2p_idx=0, connection_type="outbound-full-relay") with self.nodes[0].assert_debug_log(["txreconciliation protocol violation"]): peer.send_message(sendtxrcncl_wrong_role) peer.wait_for_disconnect() @@ -193,13 +193,13 @@ class SendTxRcnclTest(BitcoinTestFramework): self.restart_node(0, []) peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=True, wait_for_verack=True) assert not peer.sendtxrcncl_msg_received - peer.peer_disconnect() + self.nodes[0].disconnect_p2ps() self.log.info('SENDTXRCNCL not sent if blocksonly is set') self.restart_node(0, ["-txreconciliation", "-blocksonly"]) peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=True, wait_for_verack=True) assert not peer.sendtxrcncl_msg_received - peer.peer_disconnect() + self.nodes[0].disconnect_p2ps() if __name__ == '__main__': diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 83f9f253fd..0075fa0996 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -625,6 +625,10 @@ class TestNode(): This method adds the p2p connection to the self.p2ps list and returns the connection to the caller. + + p2p_idx must be different for simultaneously connected peers. When reusing it for the next peer + after disconnecting the previous one, it is necessary to wait for the disconnect to finish to avoid + a race condition. """ def addconnection_callback(address, port):