p2p: avoid sending the same peer list over and over

Nodes remember which connections have been sent which peer addresses
and won't send it again. This causes more addresses to be sent as
the connection lifetime grows, since there is no duplication anymore,
which increases the diffusion speed of peer addresses. The whole
white list is now considered for sending, not just the most recent
seen peers. This further hardens against topology discovery, though
it will more readily send peers that have been last seen earlier
than it otherwise would. While this does save a fair amount of net
bandwidth, it makes heavy use of std::set lookups, which does bring
network_address::less up the profile, though not too aggressively.

3 files changed, 17 insertions, 4 deletions

diff --git a/src/p2p/net_node.h b/src/p2p/net_node.h
index 1ef6b9028..9b9ffbe2a 100644
--- a/src/p2p/net_node.h
+++ b/src/p2p/net_node.h
@@ -123,6 +123,7 @@ namespace nodetool
     peerid_type peer_id;
     uint32_t support_flags;
     bool m_in_timedsync;
+    std::set<epee::net_utils::network_address> sent_addresses;
   };
 
   template<class t_payload_net_handler>
diff --git a/src/p2p/net_node.inl b/src/p2p/net_node.inl
index 7a2feddc8..13da53b1d 100644
--- a/src/p2p/net_node.inl
+++ b/src/p2p/net_node.inl
@@ -2294,7 +2294,17 @@ namespace nodetool
     const epee::net_utils::zone zone_type = context.m_remote_address.get_zone();
     network_zone& zone = m_network_zones.at(zone_type);
 
-    zone.m_peerlist.get_peerlist_head(rsp.local_peerlist_new, true);
+    std::vector<peerlist_entry> local_peerlist_new;
+    zone.m_peerlist.get_peerlist_head(local_peerlist_new, true, P2P_DEFAULT_PEERS_IN_HANDSHAKE);
+
+    //only include out peers we did not already send
+    rsp.local_peerlist_new.reserve(local_peerlist_new.size());
+    for (auto &pe: local_peerlist_new)
+    {
+      if (!context.sent_addresses.insert(pe.adr).second)
+        continue;
+      rsp.local_peerlist_new.push_back(std::move(pe));
+    }
     m_payload_handler.get_payload_sync_data(rsp.payload_data);
 
     /* Tor/I2P nodes receiving connections via forwarding (from tor/i2p daemon)
@@ -2416,6 +2426,8 @@ namespace nodetool
 
     //fill response
     zone.m_peerlist.get_peerlist_head(rsp.local_peerlist_new, true);
+    for (const auto &e: rsp.local_peerlist_new)
+      context.sent_addresses.insert(e.adr);
     get_local_node_data(rsp.node_data, zone);
     m_payload_handler.get_payload_sync_data(rsp.payload_data);
     LOG_DEBUG_CC(context, "COMMAND_HANDSHAKE");
diff --git a/src/p2p/net_peerlist.h b/src/p2p/net_peerlist.h
index 58b704f73..8225ad2fa 100644
--- a/src/p2p/net_peerlist.h
+++ b/src/p2p/net_peerlist.h
@@ -269,19 +269,19 @@ namespace nodetool
     peers_indexed::index<by_time>::type& by_time_index=m_peers_white.get<by_time>();
     uint32_t cnt = 0;
 
-    // picks a random set of peers within the first 120%, rather than a set of the first 100%.
+    // picks a random set of peers within the whole set, rather pick the first depth elements.
     // The intent is that if someone asks twice, they can't easily tell:
     // - this address was not in the first list, but is in the second, so the only way this can be
     // is if its last_seen was recently reset, so this means the target node recently had a new
     // connection to that address
     // - this address was in the first list, and not in the second, which means either the address
-    // was moved to the gray list (if it's not accessibe, which the attacker can check if
+    // was moved to the gray list (if it's not accessible, which the attacker can check if
     // the address accepts incoming connections) or it was the oldest to still fit in the 250 items,
     // so its last_seen is old.
     //
     // See Cao, Tong et al. "Exploring the Monero Peer-to-Peer Network". https://eprint.iacr.org/2019/411
     //
-    const uint32_t pick_depth = anonymize ? depth + depth / 5 : depth;
+    const uint32_t pick_depth = anonymize ? m_peers_white.size() : depth;
     bs_head.reserve(pick_depth);
     for(const peers_indexed::value_type& vl: boost::adaptors::reverse(by_time_index))
     {