From 97df4e38879d2644aeec34c1eef241fed627333e Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 19 Dec 2023 10:30:14 -0300 Subject: [PATCH 1/6] net: store best block tip time inside PeerManager And implement 'ApproximateBestBlockDepth()' to estimate the distance, in blocks, between the best-known block and the network chain tip. Utilizing the best-block time and the chainparams blocks spacing to approximate it. --- src/init.cpp | 6 ++++-- src/net_processing.cpp | 21 +++++++++++++++++++-- src/net_processing.h | 4 ++-- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 481d5d398d3..f389ba2f9d0 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1754,13 +1754,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) { @@ -1769,7 +1771,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_processing.cpp b/src/net_processing.cpp index df54a62f28d..3abedf3e7e9 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -513,7 +513,11 @@ 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& interruptMsgProc) override @@ -721,6 +725,8 @@ private: /** The height of the best chain */ std::atomic m_best_height{-1}; + /** The time of the best chain tip block */ + std::atomic 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}; @@ -992,6 +998,12 @@ private: void UpdateBlockAvailability(NodeId nodeid, const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); 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, @@ -1311,6 +1323,11 @@ bool PeerManagerImpl::TipMayBeStale() return m_last_tip_update.load() < GetTime() - std::chrono::seconds{consensusParams.nPowTargetSpacing * 3} && mapBlocksInFlight.empty(); } +int64_t PeerManagerImpl::ApproximateBestBlockDepth() const +{ + return (GetTime() - 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; @@ -2047,7 +2064,7 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha */ void PeerManagerImpl::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) { - SetBestHeight(pindexNew->nHeight); + SetBestBlock(pindexNew->nHeight, std::chrono::seconds{pindexNew->GetBlockTime()}); SetServiceFlagsIBDCache(!fInitialDownload); // Don't relay inventory during initial block download. diff --git a/src/net_processing.h b/src/net_processing.h index afdef000674..f3aa2c84c62 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; From f9ac96b8d6f4eba23c88f302b22a2c676e351263 Mon Sep 17 00:00:00 2001 From: furszy Date: Wed, 26 Jul 2023 14:21:21 -0300 Subject: [PATCH 2/6] net: decouple state independent service flags from desirable ones This former one will be moved to the peer manager class in the following-up commit. --- contrib/seeds/README.md | 2 +- src/net.cpp | 4 ++-- src/protocol.h | 11 ++++++++--- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/contrib/seeds/README.md b/contrib/seeds/README.md index 6db77cbbea5..ad1ac4a64d3 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/net.cpp b/src/net.cpp index 102d81579f9..cb9f1582065 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -202,7 +202,7 @@ static std::vector ConvertSeeds(const std::vector &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() - one_week, -one_week); LogPrint(BCLog::NET, "Added hardcoded seed: %s\n", addr.ToStringAddrPort()); vSeedsOut.push_back(addr); @@ -2273,7 +2273,7 @@ void CConnman::ThreadDNSAddressSeed() AddAddrFetch(seed); } else { std::vector 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)) { diff --git a/src/protocol.h b/src/protocol.h index e405253632d..a3c472d0982 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -330,12 +330,17 @@ std::vector serviceFlagsToStr(uint64_t flags); * 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); +/** + * 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); } + /** Set the current IBD status in order to figure out the desirable service flags */ void SetServiceFlagsIBDCache(bool status); From 9f36e591c551ec2e58a6496334541bfdae8fdfe5 Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 19 Dec 2023 09:50:37 -0300 Subject: [PATCH 3/6] net: move state dependent peer services flags No behavior change. Just an intermediate refactoring. By relocating the peer desirable services flags into the peer manager, we allow the connections acceptance process to handle post-IBD potential stalling scenarios. In the follow-up commit(s), the desirable service flags will be dynamically adjusted to detect post-IBD stalling scenarios (such as a +48-hour inactive node that must prefer full node connections instead of limited peer connections because they cannot provide historical blocks). Additionally, this encapsulation enable us to customize the connections decision-making process based on new user's configurations in the future. --- src/net.cpp | 4 ++-- src/net.h | 6 ++++++ src/net_processing.cpp | 16 ++++++++++++++++ src/net_processing.h | 23 +++++++++++++++++++++++ src/protocol.cpp | 10 ++-------- src/protocol.h | 34 +--------------------------------- src/test/fuzz/integer.cpp | 1 - 7 files changed, 50 insertions(+), 44 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index cb9f1582065..6575441cadd 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2637,7 +2637,7 @@ void CConnman::ThreadOpenConnections(const std::vector 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()); @@ -2703,7 +2703,7 @@ void CConnman::ThreadOpenConnections(const std::vector 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; diff --git a/src/net.h b/src/net.h index 547e032ba61..78414340374 100644 --- a/src/net.h +++ b/src/net.h @@ -1005,6 +1005,12 @@ public: /** Handle removal of a peer (clear state) */ 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 * diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 3abedf3e7e9..3e09b543365 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -499,6 +499,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& 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 @@ -523,6 +524,7 @@ public: const std::chrono::microseconds time_received, const std::atomic& 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 */ @@ -1668,6 +1670,20 @@ 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 && GetServicesFlagsIBDCache()) { + return ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS); + } + return ServiceFlags(NODE_NETWORK | NODE_WITNESS); +} + PeerRef PeerManagerImpl::GetPeerRef(NodeId id) const { LOCK(m_peer_mutex); diff --git a/src/net_processing.h b/src/net_processing.h index f3aa2c84c62..f8d7a8f5115 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -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 27a0a2ffc18..694c13a7a88 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -125,18 +125,12 @@ 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; } +bool GetServicesFlagsIBDCache() { return g_initial_block_download_completed; } + CInv::CInv() { type = 0; diff --git a/src/protocol.h b/src/protocol.h index a3c472d0982..e19560e33be 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -310,29 +310,6 @@ enum ServiceFlags : uint64_t { */ std::vector 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. - */ -ServiceFlags GetDesirableServiceFlags(ServiceFlags services); - /** * State independent service flags. * If the return value is changed, contrib/seeds/makeseeds.py @@ -343,16 +320,7 @@ constexpr ServiceFlags SeedsServiceFlags() { return ServiceFlags(NODE_NETWORK | /** 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)); -} +bool GetServicesFlagsIBDCache(); /** * 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 9b97958a5d1..2577f9e97a7 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); } From 6ed53602ac7c565273b5722de167cb2569a0e381 Mon Sep 17 00:00:00 2001 From: furszy Date: Fri, 22 Dec 2023 10:32:32 -0300 Subject: [PATCH 4/6] net: peer manager, dynamically adjust desirable services flag Introduces functionality to detect when limited peers connections are desirable or not. Ensuring that the new connections desirable services flags stay relevant throughout the software's lifecycle. (Unlike the previous approach, where once the validation IBD flag was set, the desirable services flags remained constant forever). This will let us recover from stalling scenarios where the node had successfully synced, but subsequently dropped connections and remained inactive for a duration longer than the limited peers threshold (the timeframe within which limited peers can provide blocks). Then, upon reconnection to the network, the node may end up only establishing connections with limited peers, leading to an inability to synchronize the chain. This also fixes a possible limited peers threshold violation during IBD, when the user configures `-maxtipage` further than the BIP159's limits. This rule violation could lead to sync delays and, in the worst-case scenario, trigger the same post-IBD stalling scenario (mentioned above) but during IBD. --- src/net_processing.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 3e09b543365..84a7f4c4d5a 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 */ @@ -1678,8 +1680,11 @@ bool PeerManagerImpl::HasAllDesirableServiceFlags(ServiceFlags services) const ServiceFlags PeerManagerImpl::GetDesirableServiceFlags(ServiceFlags services) const { - if (services & NODE_NETWORK_LIMITED && GetServicesFlagsIBDCache()) { - return ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS); + 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); } From aff7d92b1500e2478ce36a7e86ae47df47dda178 Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 31 Oct 2023 11:15:34 -0300 Subject: [PATCH 5/6] test: add coverage for peerman adaptive connections service flags --- src/Makefile.test.include | 1 + src/test/peerman_tests.cpp | 76 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) create mode 100644 src/test/peerman_tests.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 416a11b0c01..3c341ef4a7a 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/test/peerman_tests.cpp b/src/test/peerman_tests.cpp new file mode 100644 index 00000000000..2c793293854 --- /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 +#include +#include +#include +#include +#include + +#include + +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(); + 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(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 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{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{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()); + 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{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() From 27f260aa6e04f82dad78e9a06d58927546143a27 Mon Sep 17 00:00:00 2001 From: furszy Date: Fri, 22 Dec 2023 10:32:37 -0300 Subject: [PATCH 6/6] net: remove now unused global 'g_initial_block_download_completed' --- src/net_processing.cpp | 1 - src/protocol.cpp | 8 -------- src/protocol.h | 4 ---- 3 files changed, 13 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 84a7f4c4d5a..9904790cea1 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2086,7 +2086,6 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha void PeerManagerImpl::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) { SetBestBlock(pindexNew->nHeight, std::chrono::seconds{pindexNew->GetBlockTime()}); - SetServiceFlagsIBDCache(!fInitialDownload); // Don't relay inventory during initial block download. if (fInitialDownload) return; diff --git a/src/protocol.cpp b/src/protocol.cpp index 694c13a7a88..0da160768d1 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -9,8 +9,6 @@ #include -static std::atomic g_initial_block_download_completed(false); - namespace NetMsgType { const char* VERSION = "version"; const char* VERACK = "verack"; @@ -125,12 +123,6 @@ bool CMessageHeader::IsCommandValid() const return true; } -void SetServiceFlagsIBDCache(bool state) { - g_initial_block_download_completed = state; -} - -bool GetServicesFlagsIBDCache() { return g_initial_block_download_completed; } - CInv::CInv() { type = 0; diff --git a/src/protocol.h b/src/protocol.h index e19560e33be..243cd23e6e6 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -318,10 +318,6 @@ std::vector serviceFlagsToStr(uint64_t flags); */ constexpr ServiceFlags SeedsServiceFlags() { return ServiceFlags(NODE_NETWORK | NODE_WITNESS); } -/** Set the current IBD status in order to figure out the desirable service flags */ -void SetServiceFlagsIBDCache(bool status); -bool GetServicesFlagsIBDCache(); - /** * Checks if a peer with the given service flags may be capable of having a * robust address-storage DB.