Merge bitcoin#18454: net: Make addr relay mockable, add test

NOTE: There is slight difference with original backport due to future changes
in bitcoin#19272, bitcoin#19763 - otherwise functional test p2p_addr_relay.py fails

fa1da3d test: Add basic addr relay test (MarcoFalke)
fa1793c net: Pass connman const when relaying address (MarcoFalke)
fa47a0b net: Make addr relay mockable (MarcoFalke)

Pull request description:

  As usual:

  * Switch to std::chrono time to be type-safe and mockable
  * Add basic test that relies on mocktime to add code coverage

ACKs for top commit:
  naumenkogs:
    utACK  fa1da3d
  promag:
    ACK fa1da3d (fabe56e before bitcoin#18454 (comment)), fa5bf23 was dropped since last review.

Tree-SHA512: 0552bf8fcbe375baa3cab62acd8c23b2994efa47daff818ad1116d0ffaa0b9e520dc1bca2bbc68369b25584e85e54861fe6fd0968de4f503b95439c099df9bd7

fixup - see bitcoin#19272, bitcoin#19763

Author: MarcoFalke

src/net.h

@@ -1134,8 +1134,8 @@ class CNode
     std::vector<CAddress> vAddrToSend;
     const std::unique_ptr<CRollingBloomFilter> m_addr_known;
     bool fGetAddr{false};
-    int64_t nNextAddrSend GUARDED_BY(cs_sendProcessing){0};
-    int64_t nNextLocalAddrSend GUARDED_BY(cs_sendProcessing){0};
+    std::chrono::microseconds m_next_addr_send GUARDED_BY(cs_sendProcessing){0};
+    std::chrono::microseconds m_next_local_addr_send GUARDED_BY(cs_sendProcessing){0};
 
     // Don't relay addr messages to peers that we connect to as block-relay-only
     // peers (to prevent adversaries from inferring these links from addr

src/net_processing.cpp

@@ -138,9 +138,9 @@ static const int MAX_UNCONNECTING_HEADERS = 10;
 static const unsigned int NODE_NETWORK_LIMITED_MIN_BLOCKS = 288;
 
 /** Average delay between local address broadcasts in seconds. */
-static constexpr unsigned int AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL = 24 * 60 * 60;
+static constexpr std::chrono::hours AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL{24};
 /** Average delay between peer address broadcasts in seconds. */
-static const unsigned int AVG_ADDRESS_BROADCAST_INTERVAL = 30;
+static constexpr std::chrono::seconds AVG_ADDRESS_BROADCAST_INTERVAL{30};
 /** Average delay between trickled inventory transmissions in seconds.
  *  Blocks and peers with noban permission bypass this, regular outbound peers get half this delay,
  *  Masternode outbound peers get quarter this delay. */
@@ -1889,13 +1889,13 @@ void RelayTransaction(const uint256& txid, const CConnman& connman)
     });
 }
 
-static void RelayAddress(const CAddress& addr, bool fReachable, CConnman& connman)
+static void RelayAddress(const CAddress& addr, bool fReachable, const CConnman& connman)
 {
     // Relay to a limited number of other nodes
     // Use deterministic randomness to send to the same nodes for 24 hours
     // at a time so the m_addr_knowns of the chosen nodes prevent repeats
     uint64_t hashAddr = addr.GetHash();
-    const CSipHasher hasher = connman.GetDeterministicRandomizer(RANDOMIZER_ID_ADDRESS_RELAY).Write(hashAddr << 32).Write((GetTime() + hashAddr) / (24*60*60));
+    const CSipHasher hasher = connman.GetDeterministicRandomizer(RANDOMIZER_ID_ADDRESS_RELAY).Write(hashAddr << 32).Write((GetTime() + hashAddr) / (24 * 60 * 60));
     FastRandomContext insecure_rand;
 
     // Relay reachable addresses to 2 peers. Unreachable addresses are relayed randomly to 1 or 2 peers.
@@ -4678,16 +4678,16 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
         int64_t nNow = GetTimeMicros();
         auto current_time = GetTime<std::chrono::microseconds>();
 
-        if (pto->IsAddrRelayPeer() && !m_chainman.ActiveChainstate().IsInitialBlockDownload() && pto->nNextLocalAddrSend < nNow) {
+        if (pto->IsAddrRelayPeer() && !m_chainman.ActiveChainstate().IsInitialBlockDownload() && pto->m_next_local_addr_send < current_time) {
             AdvertiseLocal(pto);
-            pto->nNextLocalAddrSend = PoissonNextSend(nNow, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL);
+            pto->m_next_local_addr_send = PoissonNextSend(current_time, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL);
         }
 
         //
         // Message: addr
         //
-        if (pto->IsAddrRelayPeer() && pto->nNextAddrSend < nNow) {
-            pto->nNextAddrSend = PoissonNextSend(nNow, AVG_ADDRESS_BROADCAST_INTERVAL);
+        if (pto->IsAddrRelayPeer() && pto->m_next_addr_send < current_time) {
+            pto->m_next_addr_send = PoissonNextSend(current_time, AVG_ADDRESS_BROADCAST_INTERVAL);
             std::vector<CAddress> vAddr;
             vAddr.reserve(pto->vAddrToSend.size());
 

test/functional/p2p_addr_relay.py

@@ -0,0 +1,69 @@
+#!/usr/bin/env python3
+# Copyright (c) 2020 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""
+Test addr relay
+"""
+
+from test_framework.messages import (
+    CAddress,
+    NODE_NETWORK,
+    msg_addr,
+)
+from test_framework.mininode import (
+    P2PInterface,
+)
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import (
+    assert_equal,
+)
+import time
+
+ADDRS = []
+for i in range(10):
+    addr = CAddress()
+    addr.time = int(time.time()) + i
+    addr.nServices = NODE_NETWORK
+    addr.ip = "123.123.123.{}".format(i % 256)
+    addr.port = 8333 + i
+    ADDRS.append(addr)
+
+
+class AddrReceiver(P2PInterface):
+    def on_addr(self, message):
+        for addr in message.addrs:
+            assert_equal(addr.nServices, 9)
+            assert addr.ip.startswith('123.123.123.')
+            assert (8333 <= addr.port < 8343)
+
+
+class AddrTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.setup_clean_chain = False
+        self.num_nodes = 1
+
+    def run_test(self):
+        self.log.info('Create connection that sends addr messages')
+        addr_source = self.nodes[0].add_p2p_connection(P2PInterface())
+        msg = msg_addr()
+
+        self.log.info('Send too large addr message')
+        msg.addrs = ADDRS * 101
+        with self.nodes[0].assert_debug_log(['addr message size = 1010']):
+            addr_source.send_and_ping(msg)
+
+        self.log.info('Check that addr message content is relayed and added to addrman')
+        addr_receiver = self.nodes[0].add_p2p_connection(AddrReceiver())
+        msg.addrs = ADDRS
+        with self.nodes[0].assert_debug_log([
+                'Added 10 addresses from 127.0.0.1: 0 tried',
+                'received: addr (301 bytes) peer=0',
+        ]):
+            addr_source.send_and_ping(msg)
+            self.nodes[0].setmocktime(int(time.time()) + 30 * 60)
+            addr_receiver.sync_with_ping()
+
+
+if __name__ == '__main__':
+    AddrTest().main()

test/functional/test_runner.py

@@ -174,6 +174,7 @@
     'rpc_blockchain.py',
     'rpc_deprecated.py',
     'wallet_disable.py',
+    'p2p_addr_relay.py',
     'p2p_getaddr_caching.py',
     'p2p_getdata.py',
     'rpc_net.py',