Merge #5952: backport: merge bitcoin#20146, bitcoin#20477, bitcoin#20624

, bitcoin#19972, bitcoin#20724, bitcoin#19884, bitcoin#21165, bitcoin#20721, bitcoin#21254, partial bitcoin#19829 (networking backports)

0d90465 merge bitcoin#21254: Avoid connecting to real network when running tests (Kittywhiskers Van Gogh)
2f672bd merge bitcoin#20721: Move ping data to net_processing (Kittywhiskers Van Gogh)
0d46acb merge bitcoin#21165: Use mocktime in test_seed_peers (Kittywhiskers Van Gogh)
8f40769 merge bitcoin#19884: No delay in adding fixed seeds if -dnsseed=0 and peers.dat is empty (Kittywhiskers Van Gogh)
446076d test: add missing `dnsseed=0` in configuration (Kittywhiskers Van Gogh)
081d8db mempool: remove stray boost::optional usage (Kittywhiskers Van Gogh)
bcd383c merge bitcoin#20724: Cleanup of -debug=net log messages (Kittywhiskers Van Gogh)
8c63868 merge bitcoin#19972: Add fuzzing harness for node eviction logic (Kittywhiskers Van Gogh)
18f2dc0 partial bitcoin#19829: Move block inventory state to net_processing (Kittywhiskers Van Gogh)
ec77bd3 net: move nLast{Block,TX}Time to match upstream location (Kittywhiskers Van Gogh)
f635e4a merge bitcoin#20624: Remove nStartingHeight check from block relay (Kittywhiskers Van Gogh)
9f1a3e5 merge bitcoin#20477: Add unit testing of node eviction logic (Kittywhiskers Van Gogh)
2d838a6 merge bitcoin#20146: Send post-verack handshake messages at most once (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * [bitcoin#19884](bitcoin#19884) doesn't seem to play nice on its own and necessitated two more backports (namely [bitcoin#21165](bitcoin#21165) and [bitcoin#21254](bitcoin#21254)) before it did.
    * Trying to find why that is has been time consuming but there doesn't seem to be a concrete answer. If running the daemon normally (`dashd --regtest --dnsseed=1`), the functionality behaves as expected but the test still fails.

    * The closest explanation is that our OOO backports with relation to mocking time could explain why it isn't working as expected due to debug statements I added that always shown the time delta between each "enough time has passed" check was 0 seconds even when the log was advancing forward in time.

  * The usage of `dnsseed=0` stems from [bitcoin#16551](bitcoin#16551) (link to diff in commit comment), a backport that was skipped due to complexity. Though, some aspects of the PR have made it with [dash#3946](#3946).

  * [bitcoin#19829](bitcoin#19829) does away with `CConnman::ForEachNode` usage in `PeerManagerImpl::UpdatedBlockTip` ([source](https://github.com/bitcoin/bitcoin/pull/19829/files#diff-6875de769e90cec84d2e8a9c1b962cdbcda44d870d42e4215827e599e11e90e3R1318-R1328)). Dash cannot do the same and continues to use `ForEachNode` as we use the `CNode::CanRelay` check, which is not accessible through the `Peer` struct.

      * It would be valuable to find values that are Dash-specific (like `m_masternode_connection`) and migrate them to `Peer` to avoid Dash-specific patches and closer alignment with upstream.

  ## Breaking Changes

  Potential change in behaviour in the GUI and RPC.

  In RPC, `getpeerinfo` will now display `pingwait` and `startingheight` if `fStateStats` is true (earlier behaviour was unconditional). `startingheight` has been placed below `banscore` (earlier behaviour placed it above). In the GUI (Qt), `peerHeight` and `peerPingWait` are subject to similar conditionality as mentioned earlier.

  No changes in protocol or consensus. Changes are primarily related to refactoring, cleaning up, improving networking code and adding a new flag (`-fixedseeds`).

  ## Checklist:

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  PastaPastaPasta:
    re-utACK 0d90465

Tree-SHA512: 79eedf47387a8715fc9f20c6bc051d4eae832266454445043e1478dc36daafc1679e002623917af43cf923735217622e3985f664123a1de23fadfdfece7e9b6b

src/Makefile.test.include

@@ -289,6 +289,7 @@ test_fuzz_fuzz_SOURCES = \
   test/fuzz/net_permissions.cpp \
   test/fuzz/netaddress.cpp \
   test/fuzz/netbase_dns_lookup.cpp \
+  test/fuzz/node_eviction.cpp \
   test/fuzz/p2p_transport_serialization.cpp \
   test/fuzz/parse_hd_keypath.cpp \
   test/fuzz/parse_iso8601.cpp \

src/init.cpp

@@ -577,8 +577,9 @@ void SetupServerArgs(NodeContext& node)
     argsman.AddArg("-connect=<ip>", "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);
     argsman.AddArg("-dns", strprintf("Allow DNS lookups for -addnode, -seednode and -connect (default: %u)", DEFAULT_NAME_LOOKUP), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
-    argsman.AddArg("-dnsseed", "Query for peer addresses via DNS lookup, if low on addresses (default: 1 unless -connect used)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
+    argsman.AddArg("-dnsseed", strprintf("Query for peer addresses via DNS lookup, if low on addresses (default: %u unless -connect used)", DEFAULT_DNSSEED), ArgsManager::ALLOW_BOOL, OptionsCategory::CONNECTION);
     argsman.AddArg("-externalip=<ip>", "Specify your own public address", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
+    argsman.AddArg("-fixedseeds", strprintf("Allow fixed seeds if DNS seeds don't provide peers (default: %u)", DEFAULT_FIXEDSEEDS), ArgsManager::ALLOW_BOOL, OptionsCategory::CONNECTION);
     argsman.AddArg("-forcednsseed", strprintf("Always query for peer addresses via DNS lookup (default: %u)", DEFAULT_FORCEDNSSEED), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
     argsman.AddArg("-listen", "Accept connections from outside (default: 1 if no -proxy or -connect)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
     argsman.AddArg("-listenonion", strprintf("Automatically create Tor onion service (default: %d)", DEFAULT_LISTEN_ONION), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);

src/net.h

@@ -40,6 +40,7 @@
 #include <thread>
 #include <optional>
 #include <queue>
+#include <vector>
 
 #ifndef WIN32
 #define USE_WAKEUP_PIPE
@@ -100,6 +101,8 @@ static const int64_t DEFAULT_PEER_CONNECT_TIMEOUT = 60;
 static const int NUM_FDS_MESSAGE_CAPTURE = 1;
 
 static const bool DEFAULT_FORCEDNSSEED = false;
+static const bool DEFAULT_DNSSEED = true;
+static const bool DEFAULT_FIXEDSEEDS = true;
 static const size_t DEFAULT_MAXRECEIVEBUFFER = 5 * 1000;
 static const size_t DEFAULT_MAXSENDBUFFER    = 1 * 1000;
 
@@ -278,15 +281,14 @@ class CNodeStats
     std::string cleanSubVer;
     bool fInbound;
     bool m_manual_connection;
-    int nStartingHeight;
+    int m_starting_height;
     uint64_t nSendBytes;
     mapMsgCmdSize mapSendBytesPerMsgCmd;
     uint64_t nRecvBytes;
     mapMsgCmdSize mapRecvBytesPerMsgCmd;
     NetPermissionFlags m_permissionFlags;
     bool m_legacyWhitelisted;
     int64_t m_ping_usec;
-    int64_t m_ping_wait_usec;
     int64_t m_min_ping_usec;
     // Our address, as reported by the peer
     std::string addrLocal;
@@ -589,9 +591,6 @@ class CNode
     mapMsgCmdSize mapRecvBytesPerMsgCmd GUARDED_BY(cs_vRecv);
 
 public:
-    uint256 hashContinue;
-    std::atomic<int> nStartingHeight{-1};
-
     // flood relay
     std::vector<CAddress> vAddrToSend;
     std::unique_ptr<CRollingBloomFilter> m_addr_known{nullptr};
@@ -601,24 +600,6 @@ class CNode
 
     bool IsBlockRelayOnly() const;
 
-    // List of block ids we still have announce.
-    // There is no final sorting before sending, as they are always sent immediately
-    // and in the order requested.
-    std::vector<uint256> vInventoryBlockToSend GUARDED_BY(cs_inventory);
-    Mutex cs_inventory;
-    /** UNIX epoch time of the last block received from this peer that we had
-     * not yet seen (e.g. not already received from another peer), that passed
-     * preliminary validity checks and was saved to disk, even if we don't
-     * connect the block or it eventually fails connection. Used as an inbound
-     * peer eviction criterium in CConnman::AttemptToEvictConnection. */
-    std::atomic<int64_t> nLastBlockTime{0};
-
-    /** UNIX epoch time of the last transaction received from this peer that we
-     * had not yet seen (e.g. not already received from another peer) and that
-     * was accepted into our mempool. Used as an inbound peer eviction criterium
-     * in CConnman::AttemptToEvictConnection. */
-    std::atomic<int64_t> nLastTXTime{0};
-
     struct TxRelay {
         mutable RecursiveMutex cs_filter;
         // We use fRelayTxes for two purposes -
@@ -647,20 +628,25 @@ class CNode
     // in dash: m_tx_relay should never be nullptr, use `RelayAddrsWithConn() == false` instead
     std::unique_ptr<TxRelay> m_tx_relay{std::make_unique<TxRelay>()};
 
-    // Used for headers announcements - unfiltered blocks to relay
-    std::vector<uint256> vBlockHashesToAnnounce GUARDED_BY(cs_inventory);
-
-    // Ping time measurement:
-    // The pong reply we're expecting, or 0 if no pong expected.
-    std::atomic<uint64_t> nPingNonceSent{0};
-    /** When the last ping was sent, or 0 if no ping was ever sent */
-    std::atomic<std::chrono::microseconds> m_ping_start{std::chrono::microseconds{0}};
-    // Last measured round-trip time.
-    std::atomic<int64_t> nPingUsecTime{0};
-    // Best measured round-trip time.
-    std::atomic<int64_t> nMinPingUsecTime{std::numeric_limits<int64_t>::max()};
-    // Whether a ping is requested.
-    std::atomic<bool> fPingQueued{false};
+    /** UNIX epoch time of the last block received from this peer that we had
+     * not yet seen (e.g. not already received from another peer), that passed
+     * preliminary validity checks and was saved to disk, even if we don't
+     * connect the block or it eventually fails connection. Used as an inbound
+     * peer eviction criterium in CConnman::AttemptToEvictConnection. */
+    std::atomic<int64_t> nLastBlockTime{0};
+
+    /** UNIX epoch time of the last transaction received from this peer that we
+     * had not yet seen (e.g. not already received from another peer) and that
+     * was accepted into our mempool. Used as an inbound peer eviction criterium
+     * in CConnman::AttemptToEvictConnection. */
+    std::atomic<int64_t> nLastTXTime{0};
+
+    /** Last measured round-trip time. Used only for RPC/GUI stats/debugging.*/
+    std::atomic<int64_t> m_last_ping_time{0};
+
+    /** Lowest measured round-trip time. Used as an inbound peer eviction
+     * criterium in CConnman::AttemptToEvictConnection. */
+    std::atomic<int64_t> m_min_ping_time{std::numeric_limits<int64_t>::max()};
 
     // If true, we will send him CoinJoin queue messages
     std::atomic<bool> fSendDSQueue{false};
@@ -846,6 +832,12 @@ class CNode
 
     std::string ConnectionTypeAsString() const;
 
+    /** A ping-pong round trip has completed successfully. Update latest and minimum ping times. */
+    void PongReceived(std::chrono::microseconds ping_time) {
+        m_last_ping_time = count_microseconds(ping_time);
+        m_min_ping_time = std::min(m_min_ping_time.load(), count_microseconds(ping_time));
+    }
+
     /** Whether this peer is an inbound onion, e.g. connected via our Tor onion service. */
     bool IsInboundOnion() const { return m_inbound_onion; }
     std::string GetLogString() const;
@@ -1296,6 +1288,9 @@ friend class CNode;
 
     void SetAsmap(std::vector<bool> asmap) { addrman.m_asmap = std::move(asmap); }
 
+    /** Return true if the peer has been connected for long enough to do inactivity checks. */
+    bool RunInactivityChecks(const CNode& node) const;
+
 private:
     struct ListenSocket {
     public:
@@ -1592,6 +1587,23 @@ inline std::chrono::microseconds PoissonNextSend(std::chrono::microseconds now,
 /** Dump binary message to file, with timestamp */
 void CaptureMessage(const CAddress& addr, const std::string& msg_type, const Span<const unsigned char>& data, bool is_incoming);
 
+struct NodeEvictionCandidate
+{
+    NodeId id;
+    int64_t nTimeConnected;
+    int64_t m_min_ping_time;
+    int64_t nLastBlockTime;
+    int64_t nLastTXTime;
+    bool fRelevantServices;
+    bool fRelayTxes;
+    bool fBloomFilter;
+    uint64_t nKeyedNetGroup;
+    bool prefer_evict;
+    bool m_is_local;
+};
+
+[[nodiscard]] std::optional<NodeId> SelectNodeToEvict(std::vector<NodeEvictionCandidate>&& vEvictionCandidates);
+
 class CExplicitNetCleanup
 {
 public: