Merge #20002: net, rpc, cli: expose peer network in getpeerinfo; simplify/improve -netinfo

6272604 refactor: enable -netinfo to add future networks (i2p, cjdns) (Jon Atack)
82fd402 refactor: promote some -netinfo localvars to class members (Jon Atack)
5133fab cli: simplify -netinfo using getpeerinfo network field (Jon Atack)
4938a10 rpc, test: expose CNodeStats network in RPC getpeerinfo (Jon Atack)
6df7882 net: add peer network to CNodeStats (Jon Atack)

Pull request description:

  This PR:

  - builds on #19991 and #19998
  - exposes peer networks via a new getpeerinfo `network` field ("ipv4", "ipv6", or "onion"), and adds functional tests
  - updates -netinfo to use getpeerinfo `network` rather than detecting the peer networks client-side
  - refactors -netinfo to easily add future networks

ACKs for top commit:
  laanwj:
    ACK 6272604

Tree-SHA512: 28883487585135ceaaf84ce09131f2336e3193407f2e3df0960e3f4ac340f500ab94ffecb9d06a4c49bc05e3cca4f914ea4379860bea0bd5df2f834f74616015

Author: laanwj

src/bitcoin-cli.cpp

@@ -39,8 +39,6 @@ static const char DEFAULT_RPCCONNECT[] = "127.0.0.1";
 static const int DEFAULT_HTTP_CLIENT_TIMEOUT=900;
 static const bool DEFAULT_NAMED=false;
 static const int CONTINUE_EXECUTION=-1;
-static const std::string ONION{".onion"};
-static const size_t ONION_LEN{ONION.size()};
 
 /** Default number of blocks to generate for RPC generatetoaddress. */
 static const std::string DEFAULT_NBLOCKS = "1";
@@ -298,30 +296,24 @@ class GetinfoRequestHandler: public BaseRequestHandler
 class NetinfoRequestHandler : public BaseRequestHandler
 {
 private:
-    bool IsAddrIPv6(const std::string& addr) const
+    static constexpr int8_t UNKNOWN_NETWORK{-1};
+    static constexpr size_t m_networks_size{3};
+    const std::array<std::string, m_networks_size> m_networks{{"ipv4", "ipv6", "onion"}};
+    std::array<std::array<uint16_t, m_networks_size + 2>, 3> m_counts{{{}}}; //!< Peer counts by (in/out/total, networks/total/block-relay)
+    int8_t NetworkStringToId(const std::string& str) const
     {
-        return !addr.empty() && addr.front() == '[';
-    }
-    bool IsInboundOnion(const std::string& addr_local, int mapped_as) const
-    {
-        return mapped_as == 0 && addr_local.find(ONION) != std::string::npos;
-    }
-    bool IsOutboundOnion(const std::string& addr, int mapped_as) const
-    {
-        const size_t addr_len{addr.size()};
-        const size_t onion_pos{addr.rfind(ONION)};
-        return mapped_as == 0 && onion_pos != std::string::npos && addr_len > ONION_LEN &&
-               (onion_pos == addr_len - ONION_LEN || onion_pos == addr.find_last_of(":") - ONION_LEN);
+        for (size_t i = 0; i < m_networks_size; ++i) {
+            if (str == m_networks.at(i)) return i;
+        }
+        return UNKNOWN_NETWORK;
     }
     uint8_t m_details_level{0}; //!< Optional user-supplied arg to set dashboard details level
     bool DetailsRequested() const { return m_details_level > 0 && m_details_level < 5; }
     bool IsAddressSelected() const { return m_details_level == 2 || m_details_level == 4; }
     bool IsVersionSelected() const { return m_details_level == 3 || m_details_level == 4; }
-    enum struct NetType {
-        ipv4,
-        ipv6,
-        onion,
-    };
+    bool m_is_asmap_on{false};
+    size_t m_max_addr_length{0};
+    size_t m_max_id_length{2};
     struct Peer {
         int id;
         int mapped_as;
@@ -334,30 +326,24 @@ class NetinfoRequestHandler : public BaseRequestHandler
         double min_ping;
         double ping;
         std::string addr;
+        std::string network;
         std::string sub_version;
-        NetType net_type;
         bool is_block_relay;
         bool is_outbound;
         bool operator<(const Peer& rhs) const { return std::tie(is_outbound, min_ping) < std::tie(rhs.is_outbound, rhs.min_ping); }
     };
-    std::string NetTypeEnumToString(NetType t)
-    {
-        switch (t) {
-        case NetType::ipv4: return "ipv4";
-        case NetType::ipv6: return "ipv6";
-        case NetType::onion: return "onion";
-        } // no default case, so the compiler can warn about missing cases
-        assert(false);
-    }
+    std::vector<Peer> m_peers;
     std::string ChainToString() const
     {
         if (gArgs.GetChainName() == CBaseChainParams::TESTNET) return " testnet";
         if (gArgs.GetChainName() == CBaseChainParams::REGTEST) return " regtest";
         return "";
     }
+    const int64_t m_time_now{GetSystemTimeInSeconds()};
+
 public:
-    const int ID_PEERINFO = 0;
-    const int ID_NETWORKINFO = 1;
+    static constexpr int ID_PEERINFO = 0;
+    static constexpr int ID_NETWORKINFO = 1;
 
     UniValue PrepareRequest(const std::string& method, const std::vector<std::string>& args) override
     {
@@ -385,104 +371,81 @@ class NetinfoRequestHandler : public BaseRequestHandler
         }
 
         // Count peer connection totals, and if DetailsRequested(), store peer data in a vector of structs.
-        const int64_t time_now{GetSystemTimeInSeconds()};
-        int ipv4_i{0}, ipv6_i{0}, onion_i{0}, block_relay_i{0}, total_i{0}; // inbound conn counters
-        int ipv4_o{0}, ipv6_o{0}, onion_o{0}, block_relay_o{0}, total_o{0}; // outbound conn counters
-        size_t max_peer_id_length{2}, max_addr_length{0};
-        bool is_asmap_on{false};
-        std::vector<Peer> peers;
-        const UniValue& getpeerinfo{batch[ID_PEERINFO]["result"]};
-
-        for (const UniValue& peer : getpeerinfo.getValues()) {
-            const std::string addr{peer["addr"].get_str()};
-            const std::string addr_local{peer["addrlocal"].isNull() ? "" : peer["addrlocal"].get_str()};
-            const int mapped_as{peer["mapped_as"].isNull() ? 0 : peer["mapped_as"].get_int()};
+        for (const UniValue& peer : batch[ID_PEERINFO]["result"].getValues()) {
+            const std::string network{peer["network"].get_str()};
+            const int8_t network_id{NetworkStringToId(network)};
+            if (network_id == UNKNOWN_NETWORK) continue;
+            const bool is_outbound{!peer["inbound"].get_bool()};
             const bool is_block_relay{!peer["relaytxes"].get_bool()};
-            const bool is_inbound{peer["inbound"].get_bool()};
-            NetType net_type{NetType::ipv4};
-            if (is_inbound) {
-                if (IsAddrIPv6(addr)) {
-                    net_type = NetType::ipv6;
-                    ++ipv6_i;
-                } else if (IsInboundOnion(addr_local, mapped_as)) {
-                    net_type = NetType::onion;
-                    ++onion_i;
-                } else {
-                    ++ipv4_i;
-                }
-                if (is_block_relay) ++block_relay_i;
-            } else {
-                if (IsAddrIPv6(addr)) {
-                    net_type = NetType::ipv6;
-                    ++ipv6_o;
-                } else if (IsOutboundOnion(addr, mapped_as)) {
-                    net_type = NetType::onion;
-                    ++onion_o;
-                } else {
-                    ++ipv4_o;
-                }
-                if (is_block_relay) ++block_relay_o;
+            ++m_counts.at(is_outbound).at(network_id);      // in/out by network
+            ++m_counts.at(is_outbound).at(m_networks_size); // in/out overall
+            ++m_counts.at(2).at(network_id);                // total by network
+            ++m_counts.at(2).at(m_networks_size);           // total overall
+            if (is_block_relay) {
+                ++m_counts.at(is_outbound).at(m_networks_size + 1); // in/out block-relay
+                ++m_counts.at(2).at(m_networks_size + 1);           // total block-relay
             }
             if (DetailsRequested()) {
                 // Push data for this peer to the peers vector.
                 const int peer_id{peer["id"].get_int()};
+                const int mapped_as{peer["mapped_as"].isNull() ? 0 : peer["mapped_as"].get_int()};
                 const int version{peer["version"].get_int()};
-                const std::string sub_version{peer["subver"].get_str()};
                 const int64_t conn_time{peer["conntime"].get_int64()};
                 const int64_t last_blck{peer["last_block"].get_int64()};
                 const int64_t last_recv{peer["lastrecv"].get_int64()};
                 const int64_t last_send{peer["lastsend"].get_int64()};
                 const int64_t last_trxn{peer["last_transaction"].get_int64()};
                 const double min_ping{peer["minping"].isNull() ? -1 : peer["minping"].get_real()};
                 const double ping{peer["pingtime"].isNull() ? -1 : peer["pingtime"].get_real()};
-                peers.push_back({peer_id, mapped_as, version, conn_time, last_blck, last_recv, last_send, last_trxn, min_ping, ping, addr, sub_version, net_type, is_block_relay, !is_inbound});
-                max_peer_id_length = std::max(ToString(peer_id).length(), max_peer_id_length);
-                max_addr_length = std::max(addr.length() + 1, max_addr_length);
-                is_asmap_on |= (mapped_as != 0);
+                const std::string addr{peer["addr"].get_str()};
+                const std::string sub_version{peer["subver"].get_str()};
+                m_peers.push_back({peer_id, mapped_as, version, conn_time, last_blck, last_recv, last_send, last_trxn, min_ping, ping, addr, network, sub_version, is_block_relay, is_outbound});
+                m_max_id_length = std::max(ToString(peer_id).length(), m_max_id_length);
+                m_max_addr_length = std::max(addr.length() + 1, m_max_addr_length);
+                m_is_asmap_on |= (mapped_as != 0);
             }
         }
 
         // Generate report header.
         std::string result{strprintf("%s %s%s - %i%s\n\n", PACKAGE_NAME, FormatFullVersion(), ChainToString(), networkinfo["protocolversion"].get_int(), networkinfo["subversion"].get_str())};
 
         // Report detailed peer connections list sorted by direction and minimum ping time.
-        if (DetailsRequested() && !peers.empty()) {
-            std::sort(peers.begin(), peers.end());
+        if (DetailsRequested() && !m_peers.empty()) {
+            std::sort(m_peers.begin(), m_peers.end());
             result += "Peer connections sorted by direction and min ping\n<-> relay   net mping   ping send recv  txn  blk uptime ";
-            if (is_asmap_on) result += " asmap ";
-            result += strprintf("%*s %-*s%s\n", max_peer_id_length, "id", IsAddressSelected() ? max_addr_length : 0, IsAddressSelected() ? "address" : "", IsVersionSelected() ? "version" : "");
-            for (const Peer& peer : peers) {
+            if (m_is_asmap_on) result += " asmap ";
+            result += strprintf("%*s %-*s%s\n", m_max_id_length, "id", IsAddressSelected() ? m_max_addr_length : 0, IsAddressSelected() ? "address" : "", IsVersionSelected() ? "version" : "");
+            for (const Peer& peer : m_peers) {
                 std::string version{ToString(peer.version) + peer.sub_version};
                 result += strprintf(
                     "%3s %5s %5s%6s%7s%5s%5s%5s%5s%7s%*i %*s %-*s%s\n",
                     peer.is_outbound ? "out" : "in",
                     peer.is_block_relay ? "block" : "full",
-                    NetTypeEnumToString(peer.net_type),
+                    peer.network,
                     peer.min_ping == -1 ? "" : ToString(round(1000 * peer.min_ping)),
                     peer.ping == -1 ? "" : ToString(round(1000 * peer.ping)),
-                    peer.last_send == 0 ? "" : ToString(time_now - peer.last_send),
-                    peer.last_recv == 0 ? "" : ToString(time_now - peer.last_recv),
-                    peer.last_trxn == 0 ? "" : ToString((time_now - peer.last_trxn) / 60),
-                    peer.last_blck == 0 ? "" : ToString((time_now - peer.last_blck) / 60),
-                    peer.conn_time == 0 ? "" : ToString((time_now - peer.conn_time) / 60),
-                    is_asmap_on ? 7 : 0, // variable spacing
-                    is_asmap_on && peer.mapped_as != 0 ? ToString(peer.mapped_as) : "",
-                    max_peer_id_length, // variable spacing
+                    peer.last_send == 0 ? "" : ToString(m_time_now - peer.last_send),
+                    peer.last_recv == 0 ? "" : ToString(m_time_now - peer.last_recv),
+                    peer.last_trxn == 0 ? "" : ToString((m_time_now - peer.last_trxn) / 60),
+                    peer.last_blck == 0 ? "" : ToString((m_time_now - peer.last_blck) / 60),
+                    peer.conn_time == 0 ? "" : ToString((m_time_now - peer.conn_time) / 60),
+                    m_is_asmap_on ? 7 : 0, // variable spacing
+                    m_is_asmap_on && peer.mapped_as != 0 ? ToString(peer.mapped_as) : "",
+                    m_max_id_length, // variable spacing
                     peer.id,
-                    IsAddressSelected() ? max_addr_length : 0, // variable spacing
+                    IsAddressSelected() ? m_max_addr_length : 0, // variable spacing
                     IsAddressSelected() ? peer.addr : "",
                     IsVersionSelected() && version != "0" ? version : "");
             }
             result += "                   ms     ms  sec  sec  min  min    min\n\n";
         }
 
         // Report peer connection totals by type.
-        total_i = ipv4_i + ipv6_i + onion_i;
-        total_o = ipv4_o + ipv6_o + onion_o;
         result += "        ipv4    ipv6   onion   total  block-relay\n";
-        result += strprintf("in     %5i   %5i   %5i   %5i   %5i\n", ipv4_i, ipv6_i, onion_i, total_i, block_relay_i);
-        result += strprintf("out    %5i   %5i   %5i   %5i   %5i\n", ipv4_o, ipv6_o, onion_o, total_o, block_relay_o);
-        result += strprintf("total  %5i   %5i   %5i   %5i   %5i\n", ipv4_i + ipv4_o, ipv6_i + ipv6_o, onion_i + onion_o, total_i + total_o, block_relay_i + block_relay_o);
+        const std::array<std::string, 3> rows{{"in", "out", "total"}};
+        for (size_t i = 0; i < m_networks_size; ++i) {
+            result += strprintf("%-5s  %5i   %5i   %5i   %5i   %5i\n", rows.at(i), m_counts.at(i).at(0), m_counts.at(i).at(1), m_counts.at(i).at(2), m_counts.at(i).at(m_networks_size), m_counts.at(i).at(m_networks_size + 1));
+        }
 
         // Report local addresses, ports, and scores.
         result += "\nLocal addresses";

src/net.cpp

@@ -552,6 +552,7 @@ void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap)
     X(nServices);
     X(addr);
     X(addrBind);
+    stats.m_network = GetNetworkName(ConnectedThroughNetwork());
     stats.m_mapped_as = addr.GetMappedAS(m_asmap);
     if (m_tx_relay != nullptr) {
         LOCK(m_tx_relay->cs_filter);

src/net.h

@@ -705,6 +705,8 @@ class CNodeStats
     CAddress addr;
     // Bind address of our side of the connection
     CAddress addrBind;
+    // Name of the network the peer connected through
+    std::string m_network;
     uint32_t m_mapped_as;
     std::string m_conn_type_string;
 };

src/rpc/net.cpp

@@ -94,6 +94,7 @@ static RPCHelpMan getpeerinfo()
                             {RPCResult::Type::STR, "addr", "(host:port) The IP address and port of the peer"},
                             {RPCResult::Type::STR, "addrbind", "(ip:port) Bind address of the connection to the peer"},
                             {RPCResult::Type::STR, "addrlocal", "(ip:port) Local address as reported by the peer"},
+                            {RPCResult::Type::STR, "network", "Network (ipv4, ipv6, or onion) the peer connected through"},
                             {RPCResult::Type::NUM, "mapped_as", "The AS in the BGP route to the peer used for diversifying\n"
                                                                 "peer selection (only available if the asmap config flag is set)"},
                             {RPCResult::Type::STR_HEX, "services", "The services offered"},
@@ -167,10 +168,13 @@ static RPCHelpMan getpeerinfo()
         bool fStateStats = GetNodeStateStats(stats.nodeid, statestats);
         obj.pushKV("id", stats.nodeid);
         obj.pushKV("addr", stats.addrName);
-        if (!(stats.addrLocal.empty()))
-            obj.pushKV("addrlocal", stats.addrLocal);
-        if (stats.addrBind.IsValid())
+        if (stats.addrBind.IsValid()) {
             obj.pushKV("addrbind", stats.addrBind.ToString());
+        }
+        if (!(stats.addrLocal.empty())) {
+            obj.pushKV("addrlocal", stats.addrLocal);
+        }
+        obj.pushKV("network", stats.m_network);
         if (stats.m_mapped_as != 0) {
             obj.pushKV("mapped_as", uint64_t(stats.m_mapped_as));
         }

test/functional/feature_proxy.py

@@ -18,8 +18,9 @@
     - proxy on IPv6
 
 - Create various proxies (as threads)
-- Create bitcoinds that connect to them
-- Manipulate the bitcoinds using addnode (onetry) an observe effects
+- Create nodes that connect to them
+- Manipulate the peer connections using addnode (onetry) and observe effects
+- Test the getpeerinfo `network` field for the peer
 
 addnode connect to IPv4
 addnode connect to IPv6
@@ -40,6 +41,12 @@
 from test_framework.netutil import test_ipv6_local
 
 RANGE_BEGIN = PORT_MIN + 2 * PORT_RANGE  # Start after p2p and rpc ports
+# From GetNetworkName() in netbase.cpp:
+NET_UNROUTABLE = ""
+NET_IPV4 = "ipv4"
+NET_IPV6 = "ipv6"
+NET_ONION = "onion"
+
 
 class ProxyTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -90,10 +97,16 @@ def setup_nodes(self):
         self.add_nodes(self.num_nodes, extra_args=args)
         self.start_nodes()
 
+    def network_test(self, node, addr, network):
+        for peer in node.getpeerinfo():
+            if peer["addr"] == addr:
+                assert_equal(peer["network"], network)
+
     def node_test(self, node, proxies, auth, test_onion=True):
         rv = []
-        # Test: outgoing IPv4 connection through node
-        node.addnode("15.61.23.23:1234", "onetry")
+        addr = "15.61.23.23:1234"
+        self.log.debug("Test: outgoing IPv4 connection through node for address {}".format(addr))
+        node.addnode(addr, "onetry")
         cmd = proxies[0].queue.get()
         assert isinstance(cmd, Socks5Command)
         # Note: bitcoind's SOCKS5 implementation only sends atyp DOMAINNAME, even if connecting directly to IPv4/IPv6
@@ -104,10 +117,12 @@ def node_test(self, node, proxies, auth, test_onion=True):
             assert_equal(cmd.username, None)
             assert_equal(cmd.password, None)
         rv.append(cmd)
+        self.network_test(node, addr, network=NET_IPV4)
 
         if self.have_ipv6:
-            # Test: outgoing IPv6 connection through node
-            node.addnode("[1233:3432:2434:2343:3234:2345:6546:4534]:5443", "onetry")
+            addr = "[1233:3432:2434:2343:3234:2345:6546:4534]:5443"
+            self.log.debug("Test: outgoing IPv6 connection through node for address {}".format(addr))
+            node.addnode(addr, "onetry")
             cmd = proxies[1].queue.get()
             assert isinstance(cmd, Socks5Command)
             # Note: bitcoind's SOCKS5 implementation only sends atyp DOMAINNAME, even if connecting directly to IPv4/IPv6
@@ -118,10 +133,12 @@ def node_test(self, node, proxies, auth, test_onion=True):
                 assert_equal(cmd.username, None)
                 assert_equal(cmd.password, None)
             rv.append(cmd)
+            self.network_test(node, addr, network=NET_IPV6)
 
         if test_onion:
-            # Test: outgoing onion connection through node
-            node.addnode("bitcoinostk4e4re.onion:8333", "onetry")
+            addr = "bitcoinostk4e4re.onion:8333"
+            self.log.debug("Test: outgoing onion connection through node for address {}".format(addr))
+            node.addnode(addr, "onetry")
             cmd = proxies[2].queue.get()
             assert isinstance(cmd, Socks5Command)
             assert_equal(cmd.atyp, AddressType.DOMAINNAME)
@@ -131,9 +148,11 @@ def node_test(self, node, proxies, auth, test_onion=True):
                 assert_equal(cmd.username, None)
                 assert_equal(cmd.password, None)
             rv.append(cmd)
+            self.network_test(node, addr, network=NET_ONION)
 
-        # Test: outgoing DNS name connection through node
-        node.addnode("node.noumenon:8333", "onetry")
+        addr = "node.noumenon:8333"
+        self.log.debug("Test: outgoing DNS name connection through node for address {}".format(addr))
+        node.addnode(addr, "onetry")
         cmd = proxies[3].queue.get()
         assert isinstance(cmd, Socks5Command)
         assert_equal(cmd.atyp, AddressType.DOMAINNAME)
@@ -143,6 +162,7 @@ def node_test(self, node, proxies, auth, test_onion=True):
             assert_equal(cmd.username, None)
             assert_equal(cmd.password, None)
         rv.append(cmd)
+        self.network_test(node, addr, network=NET_UNROUTABLE)
 
         return rv
 
@@ -197,5 +217,6 @@ def networks_dict(d):
                 assert_equal(n3[net]['proxy_randomize_credentials'], False)
             assert_equal(n3['onion']['reachable'], False)
 
+
 if __name__ == '__main__':
     ProxyTest().main()