diff options
| -rw-r--r-- | contrib/seeds/README.md | 2 | ||||
| -rw-r--r-- | src/Makefile.test.include | 1 | ||||
| -rw-r--r-- | src/init.cpp | 6 | ||||
| -rw-r--r-- | src/net.cpp | 8 | ||||
| -rw-r--r-- | src/net.h | 6 | ||||
| -rw-r--r-- | src/net_processing.cpp | 43 | ||||
| -rw-r--r-- | src/net_processing.h | 27 | ||||
| -rw-r--r-- | src/protocol.cpp | 14 | ||||
| -rw-r--r-- | src/protocol.h | 43 | ||||
| -rw-r--r-- | src/test/fuzz/integer.cpp | 1 | ||||
| -rw-r--r-- | src/test/peerman_tests.cpp | 76 |
11 files changed, 163 insertions, 64 deletions
diff --git a/contrib/seeds/README.md b/contrib/seeds/README.md index 6db77cbbea..ad1ac4a64d 100644 --- a/contrib/seeds/README.md +++ b/contrib/seeds/README.md @@ -4,7 +4,7 @@ Utility to generate the seeds.txt list that is compiled into the client (see [src/chainparamsseeds.h](/src/chainparamsseeds.h) and other utilities in [contrib/seeds](/contrib/seeds)). Be sure to update `PATTERN_AGENT` in `makeseeds.py` to include the current version, -and remove old versions as necessary (at a minimum when GetDesirableServiceFlags +and remove old versions as necessary (at a minimum when SeedsServiceFlags() changes its default return value, as those are the services which seeds are added to addrman with). diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 46b9495fc1..69d44dbde6 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -117,6 +117,7 @@ BITCOIN_TESTS =\ test/net_tests.cpp \ test/netbase_tests.cpp \ test/orphanage_tests.cpp \ + test/peerman_tests.cpp \ test/pmt_tests.cpp \ test/policy_fee_tests.cpp \ test/policyestimator_tests.cpp \ diff --git a/src/init.cpp b/src/init.cpp index b825c8ce21..57a1fd87d1 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1743,13 +1743,15 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // ********************************************************* Step 12: start node //// debug print + int64_t best_block_time{}; { LOCK(cs_main); LogPrintf("block tree size = %u\n", chainman.BlockIndex().size()); chain_active_height = chainman.ActiveChain().Height(); + best_block_time = chainman.ActiveChain().Tip() ? chainman.ActiveChain().Tip()->GetBlockTime() : chainman.GetParams().GenesisBlock().GetBlockTime(); if (tip_info) { tip_info->block_height = chain_active_height; - tip_info->block_time = chainman.ActiveChain().Tip() ? chainman.ActiveChain().Tip()->GetBlockTime() : chainman.GetParams().GenesisBlock().GetBlockTime(); + tip_info->block_time = best_block_time; tip_info->verification_progress = GuessVerificationProgress(chainman.GetParams().TxData(), chainman.ActiveChain().Tip()); } if (tip_info && chainman.m_best_header) { @@ -1758,7 +1760,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } } LogPrintf("nBestHeight = %d\n", chain_active_height); - if (node.peerman) node.peerman->SetBestHeight(chain_active_height); + if (node.peerman) node.peerman->SetBestBlock(chain_active_height, std::chrono::seconds{best_block_time}); // Map ports with UPnP or NAT-PMP. StartMapPort(args.GetBoolArg("-upnp", DEFAULT_UPNP), args.GetBoolArg("-natpmp", DEFAULT_NATPMP)); diff --git a/src/net.cpp b/src/net.cpp index 10af6e7e91..7c82f01d75 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -203,7 +203,7 @@ static std::vector<CAddress> ConvertSeeds(const std::vector<uint8_t> &vSeedsIn) while (!s.eof()) { CService endpoint; s >> endpoint; - CAddress addr{endpoint, GetDesirableServiceFlags(NODE_NONE)}; + CAddress addr{endpoint, SeedsServiceFlags()}; addr.nTime = rng.rand_uniform_delay(Now<NodeSeconds>() - one_week, -one_week); LogPrint(BCLog::NET, "Added hardcoded seed: %s\n", addr.ToStringAddrPort()); vSeedsOut.push_back(addr); @@ -2267,7 +2267,7 @@ void CConnman::ThreadDNSAddressSeed() AddAddrFetch(seed); } else { std::vector<CAddress> vAdd; - ServiceFlags requiredServiceBits = GetDesirableServiceFlags(NODE_NONE); + constexpr ServiceFlags requiredServiceBits{SeedsServiceFlags()}; std::string host = strprintf("x%x.%s", requiredServiceBits, seed); CNetAddr resolveSource; if (!resolveSource.SetInternal(host)) { @@ -2638,7 +2638,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) const CAddress addr = m_anchors.back(); m_anchors.pop_back(); if (!addr.IsValid() || IsLocal(addr) || !g_reachable_nets.Contains(addr) || - !HasAllDesirableServiceFlags(addr.nServices) || + !m_msgproc->HasAllDesirableServiceFlags(addr.nServices) || outbound_ipv46_peer_netgroups.count(m_netgroupman.GetGroup(addr))) continue; addrConnect = addr; LogPrint(BCLog::NET, "Trying to make an anchor connection to %s\n", addrConnect.ToStringAddrPort()); @@ -2704,7 +2704,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) // for non-feelers, require all the services we'll want, // for feelers, only require they be a full node (only because most // SPV clients don't have a good address DB available) - if (!fFeeler && !HasAllDesirableServiceFlags(addr.nServices)) { + if (!fFeeler && !m_msgproc->HasAllDesirableServiceFlags(addr.nServices)) { continue; } else if (fFeeler && !MayHaveUsefulAddressDB(addr.nServices)) { continue; @@ -1006,6 +1006,12 @@ public: virtual void FinalizeNode(const CNode& node) = 0; /** + * Callback to determine whether the given set of service flags are sufficient + * for a peer to be "relevant". + */ + virtual bool HasAllDesirableServiceFlags(ServiceFlags services) const = 0; + + /** * Process protocol messages received from a given node * * @param[in] pnode The node which we have received messages from. diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 80cf610a0d..d46ecd89d4 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -133,6 +133,8 @@ static const unsigned int MAX_BLOCKS_TO_ANNOUNCE = 8; static const int MAX_NUM_UNCONNECTING_HEADERS_MSGS = 10; /** Minimum blocks required to signal NODE_NETWORK_LIMITED */ static const unsigned int NODE_NETWORK_LIMITED_MIN_BLOCKS = 288; +/** Window, in blocks, for connecting to NODE_NETWORK_LIMITED peers */ +static const unsigned int NODE_NETWORK_LIMITED_ALLOW_CONN_BLOCKS = 144; /** Average delay between local address broadcasts */ static constexpr auto AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL{24h}; /** Average delay between peer address broadcasts */ @@ -499,6 +501,7 @@ public: /** Implement NetEventsInterface */ void InitializeNode(CNode& node, ServiceFlags our_services) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void FinalizeNode(const CNode& node) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_headers_presync_mutex); + bool HasAllDesirableServiceFlags(ServiceFlags services) const override; bool ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, !m_headers_presync_mutex, g_msgproc_mutex); bool SendMessages(CNode* pto) override @@ -513,12 +516,17 @@ public: bool IgnoresIncomingTxs() override { return m_opts.ignore_incoming_txs; } void SendPings() override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void RelayTransaction(const uint256& txid, const uint256& wtxid) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); - void SetBestHeight(int height) override { m_best_height = height; }; + void SetBestBlock(int height, std::chrono::seconds time) override + { + m_best_height = height; + m_best_block_time = time; + }; void UnitTestMisbehaving(NodeId peer_id, int howmuch) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex) { Misbehaving(*Assert(GetPeerRef(peer_id)), howmuch, ""); }; void ProcessMessage(CNode& pfrom, const std::string& msg_type, DataStream& vRecv, const std::chrono::microseconds time_received, const std::atomic<bool>& interruptMsgProc) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, !m_headers_presync_mutex, g_msgproc_mutex); void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) override; + ServiceFlags GetDesirableServiceFlags(ServiceFlags services) const override; private: /** Consider evicting an outbound peer based on the amount of time they've been behind our tip */ @@ -721,6 +729,8 @@ private: /** The height of the best chain */ std::atomic<int> m_best_height{-1}; + /** The time of the best chain tip block */ + std::atomic<std::chrono::seconds> m_best_block_time{0s}; /** Next time to check for stale tip */ std::chrono::seconds m_stale_tip_check_time GUARDED_BY(cs_main){0s}; @@ -993,6 +1003,12 @@ private: bool CanDirectFetch() EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** + * Estimates the distance, in blocks, between the best-known block and the network chain tip. + * Utilizes the best-block time and the chainparams blocks spacing to approximate it. + */ + int64_t ApproximateBestBlockDepth() const; + + /** * To prevent fingerprinting attacks, only send blocks/headers outside of * the active chain if they are no more than a month older (both in time, * and in best equivalent proof of work) than the best header chain we know @@ -1311,6 +1327,11 @@ bool PeerManagerImpl::TipMayBeStale() return m_last_tip_update.load() < GetTime<std::chrono::seconds>() - std::chrono::seconds{consensusParams.nPowTargetSpacing * 3} && mapBlocksInFlight.empty(); } +int64_t PeerManagerImpl::ApproximateBestBlockDepth() const +{ + return (GetTime<std::chrono::seconds>() - m_best_block_time.load()).count() / m_chainparams.GetConsensus().nPowTargetSpacing; +} + bool PeerManagerImpl::CanDirectFetch() { return m_chainman.ActiveChain().Tip()->Time() > GetAdjustedTime() - m_chainparams.GetConsensus().PowTargetSpacing() * 20; @@ -1651,6 +1672,23 @@ void PeerManagerImpl::FinalizeNode(const CNode& node) LogPrint(BCLog::NET, "Cleared nodestate for peer=%d\n", nodeid); } +bool PeerManagerImpl::HasAllDesirableServiceFlags(ServiceFlags services) const +{ + // Shortcut for (services & GetDesirableServiceFlags(services)) == GetDesirableServiceFlags(services) + return !(GetDesirableServiceFlags(services) & (~services)); +} + +ServiceFlags PeerManagerImpl::GetDesirableServiceFlags(ServiceFlags services) const +{ + if (services & NODE_NETWORK_LIMITED) { + // Limited peers are desirable when we are close to the tip. + if (ApproximateBestBlockDepth() < NODE_NETWORK_LIMITED_ALLOW_CONN_BLOCKS) { + return ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS); + } + } + return ServiceFlags(NODE_NETWORK | NODE_WITNESS); +} + PeerRef PeerManagerImpl::GetPeerRef(NodeId id) const { LOCK(m_peer_mutex); @@ -2047,8 +2085,7 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha */ void PeerManagerImpl::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) { - SetBestHeight(pindexNew->nHeight); - SetServiceFlagsIBDCache(!fInitialDownload); + SetBestBlock(pindexNew->nHeight, std::chrono::seconds{pindexNew->GetBlockTime()}); // Don't relay inventory during initial block download. if (fInitialDownload) return; diff --git a/src/net_processing.h b/src/net_processing.h index afdef00067..f8d7a8f511 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -92,8 +92,8 @@ public: /** Send ping message to all peers */ virtual void SendPings() = 0; - /** Set the best height */ - virtual void SetBestHeight(int height) = 0; + /** Set the height of the best block and its time (seconds since epoch). */ + virtual void SetBestBlock(int height, std::chrono::seconds time) = 0; /* Public for unit testing. */ virtual void UnitTestMisbehaving(NodeId peer_id, int howmuch) = 0; @@ -110,6 +110,29 @@ public: /** This function is used for testing the stale tip eviction logic, see denialofservice_tests.cpp */ virtual void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) = 0; + + /** + * Gets the set of service flags which are "desirable" for a given peer. + * + * These are the flags which are required for a peer to support for them + * to be "interesting" to us, ie for us to wish to use one of our few + * outbound connection slots for or for us to wish to prioritize keeping + * their connection around. + * + * Relevant service flags may be peer- and state-specific in that the + * version of the peer may determine which flags are required (eg in the + * case of NODE_NETWORK_LIMITED where we seek out NODE_NETWORK peers + * unless they set NODE_NETWORK_LIMITED and we are out of IBD, in which + * case NODE_NETWORK_LIMITED suffices). + * + * Thus, generally, avoid calling with 'services' == NODE_NONE, unless + * state-specific flags must absolutely be avoided. When called with + * 'services' == NODE_NONE, the returned desirable service flags are + * guaranteed to not change dependent on state - ie they are suitable for + * use when describing peers which we know to be desirable, but for which + * we do not have a confirmed set of service flags. + */ + virtual ServiceFlags GetDesirableServiceFlags(ServiceFlags services) const = 0; }; #endif // BITCOIN_NET_PROCESSING_H diff --git a/src/protocol.cpp b/src/protocol.cpp index 27a0a2ffc1..0da160768d 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -9,8 +9,6 @@ #include <atomic> -static std::atomic<bool> g_initial_block_download_completed(false); - namespace NetMsgType { const char* VERSION = "version"; const char* VERACK = "verack"; @@ -125,18 +123,6 @@ bool CMessageHeader::IsCommandValid() const return true; } - -ServiceFlags GetDesirableServiceFlags(ServiceFlags services) { - if ((services & NODE_NETWORK_LIMITED) && g_initial_block_download_completed) { - return ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS); - } - return ServiceFlags(NODE_NETWORK | NODE_WITNESS); -} - -void SetServiceFlagsIBDCache(bool state) { - g_initial_block_download_completed = state; -} - CInv::CInv() { type = 0; diff --git a/src/protocol.h b/src/protocol.h index e405253632..243cd23e6e 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -311,43 +311,12 @@ enum ServiceFlags : uint64_t { std::vector<std::string> serviceFlagsToStr(uint64_t flags); /** - * Gets the set of service flags which are "desirable" for a given peer. - * - * These are the flags which are required for a peer to support for them - * to be "interesting" to us, ie for us to wish to use one of our few - * outbound connection slots for or for us to wish to prioritize keeping - * their connection around. - * - * Relevant service flags may be peer- and state-specific in that the - * version of the peer may determine which flags are required (eg in the - * case of NODE_NETWORK_LIMITED where we seek out NODE_NETWORK peers - * unless they set NODE_NETWORK_LIMITED and we are out of IBD, in which - * case NODE_NETWORK_LIMITED suffices). - * - * Thus, generally, avoid calling with peerServices == NODE_NONE, unless - * state-specific flags must absolutely be avoided. When called with - * peerServices == NODE_NONE, the returned desirable service flags are - * guaranteed to not change dependent on state - ie they are suitable for - * use when describing peers which we know to be desirable, but for which - * we do not have a confirmed set of service flags. - * - * If the NODE_NONE return value is changed, contrib/seeds/makeseeds.py - * should be updated appropriately to filter for the same nodes. - */ -ServiceFlags GetDesirableServiceFlags(ServiceFlags services); - -/** Set the current IBD status in order to figure out the desirable service flags */ -void SetServiceFlagsIBDCache(bool status); - -/** - * A shortcut for (services & GetDesirableServiceFlags(services)) - * == GetDesirableServiceFlags(services), ie determines whether the given - * set of service flags are sufficient for a peer to be "relevant". - */ -static inline bool HasAllDesirableServiceFlags(ServiceFlags services) -{ - return !(GetDesirableServiceFlags(services) & (~services)); -} + * State independent service flags. + * If the return value is changed, contrib/seeds/makeseeds.py + * should be updated appropriately to filter for nodes with + * desired service flags (compatible with our new flags). + */ +constexpr ServiceFlags SeedsServiceFlags() { return ServiceFlags(NODE_NETWORK | NODE_WITNESS); } /** * Checks if a peer with the given service flags may be capable of having a diff --git a/src/test/fuzz/integer.cpp b/src/test/fuzz/integer.cpp index 9b97958a5d..2577f9e97a 100644 --- a/src/test/fuzz/integer.cpp +++ b/src/test/fuzz/integer.cpp @@ -213,7 +213,6 @@ FUZZ_TARGET(integer, .init = initialize_integer) { const ServiceFlags service_flags = (ServiceFlags)u64; - (void)HasAllDesirableServiceFlags(service_flags); (void)MayHaveUsefulAddressDB(service_flags); } diff --git a/src/test/peerman_tests.cpp b/src/test/peerman_tests.cpp new file mode 100644 index 0000000000..2c79329385 --- /dev/null +++ b/src/test/peerman_tests.cpp @@ -0,0 +1,76 @@ +// Copyright (c) 2024-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php. + +#include <chainparams.h> +#include <node/miner.h> +#include <net_processing.h> +#include <pow.h> +#include <test/util/setup_common.h> +#include <validation.h> + +#include <boost/test/unit_test.hpp> + +BOOST_FIXTURE_TEST_SUITE(peerman_tests, RegTestingSetup) + +/** Window, in blocks, for connecting to NODE_NETWORK_LIMITED peers */ +static constexpr int64_t NODE_NETWORK_LIMITED_ALLOW_CONN_BLOCKS = 144; + +static void mineBlock(const node::NodeContext& node, std::chrono::seconds block_time) +{ + auto curr_time = GetTime<std::chrono::seconds>(); + SetMockTime(block_time); // update time so the block is created with it + CBlock block = node::BlockAssembler{node.chainman->ActiveChainstate(), nullptr}.CreateNewBlock(CScript() << OP_TRUE)->block; + while (!CheckProofOfWork(block.GetHash(), block.nBits, node.chainman->GetConsensus())) ++block.nNonce; + block.fChecked = true; // little speedup + SetMockTime(curr_time); // process block at current time + Assert(node.chainman->ProcessNewBlock(std::make_shared<const CBlock>(block), /*force_processing=*/true, /*min_pow_checked=*/true, nullptr)); + SyncWithValidationInterfaceQueue(); // drain events queue +} + +// Verifying when network-limited peer connections are desirable based on the node's proximity to the tip +BOOST_AUTO_TEST_CASE(connections_desirable_service_flags) +{ + std::unique_ptr<PeerManager> peerman = PeerManager::make(*m_node.connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, {}); + auto consensus = m_node.chainman->GetParams().GetConsensus(); + + // Check we start connecting to full nodes + ServiceFlags peer_flags{NODE_WITNESS | NODE_NETWORK_LIMITED}; + BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK | NODE_WITNESS)); + + // Make peerman aware of the initial best block and verify we accept limited peers when we start close to the tip time. + auto tip = WITH_LOCK(::cs_main, return m_node.chainman->ActiveChain().Tip()); + uint64_t tip_block_time = tip->GetBlockTime(); + int tip_block_height = tip->nHeight; + peerman->SetBestBlock(tip_block_height, std::chrono::seconds{tip_block_time}); + + SetMockTime(tip_block_time + 1); // Set node time to tip time + BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS)); + + // Check we don't disallow limited peers connections when we are behind but still recoverable (below the connection safety window) + SetMockTime(GetTime<std::chrono::seconds>() + std::chrono::seconds{consensus.nPowTargetSpacing * (NODE_NETWORK_LIMITED_ALLOW_CONN_BLOCKS - 1)}); + BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS)); + + // Check we disallow limited peers connections when we are further than the limited peers safety window + SetMockTime(GetTime<std::chrono::seconds>() + std::chrono::seconds{consensus.nPowTargetSpacing * 2}); + BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK | NODE_WITNESS)); + + // By now, we tested that the connections desirable services flags change based on the node's time proximity to the tip. + // Now, perform the same tests for when the node receives a block. + RegisterValidationInterface(peerman.get()); + + // First, verify a block in the past doesn't enable limited peers connections + // At this point, our time is (NODE_NETWORK_LIMITED_ALLOW_CONN_BLOCKS + 1) * 10 minutes ahead the tip's time. + mineBlock(m_node, /*block_time=*/std::chrono::seconds{tip_block_time + 1}); + BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK | NODE_WITNESS)); + + // Verify a block close to the tip enables limited peers connections + mineBlock(m_node, /*block_time=*/GetTime<std::chrono::seconds>()); + BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS)); + + // Lastly, verify the stale tip checks can disallow limited peers connections after not receiving blocks for a prolonged period. + SetMockTime(GetTime<std::chrono::seconds>() + std::chrono::seconds{consensus.nPowTargetSpacing * NODE_NETWORK_LIMITED_ALLOW_CONN_BLOCKS + 1}); + BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK | NODE_WITNESS)); +} + +BOOST_AUTO_TEST_SUITE_END() |