netinfo: add peer services column and outbound-only peers list

jonatack commented at 4:33 pm on September 19, 2024: member

Been using this since May 2023.

add a peer services column (considered displaying the p2p_v2 flag as “p” or “2”; proposing “2” here for continuity with the “v” column, but “p” is fine for me as well)
clarify in the help that “relaytxes” and “addr_relay_enabled” are from getpeerinfo
add a level 5 for an outbound-only peer list. Several people have requested this, to keep the output within screen limits when running netinfo as a live dashboard (i.e. with watch) on a long-running node with many peers.

DrahtBot commented at 4:33 pm on September 19, 2024: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	tdb3, 1440000bytes

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

sipa commented at 4:36 pm on September 19, 2024: member

It’s possible to be connected to a peer that supports the v2 service using the v1 protocol.

jonatack commented at 4:48 pm on September 19, 2024: member

It’s possible to be connected to a peer that supports the v2 service using the v1 protocol.

Thanks! (I must be missing something.)

sipa commented at 4:52 pm on September 19, 2024: member

I meant this as a comment on:

remove the “v” transport protocol column, as that info is provided in the new peer services column

They represent different information. The “v” transport protocol column currently conveys whether or not you are right now speaking to that peer through the v1 or v2 protocol.

Peer services are about what a peer supports, not what protocol you’re speaking with it.

If you run with -v2transport=0, the transport protocol will be v1 with all peers, includes ones that support v2. Similarly, If a peer recently upgraded to support v2, but you didn’t know about that when you connected to them, you may be using v1 with them.

jonatack force-pushed on Sep 19, 2024

jonatack commented at 5:04 pm on September 19, 2024: member

@sipa I guess it may make sense to only return the “v” column if the server node is -v2transport=0?

sipa commented at 5:06 pm on September 19, 2024: member

@jonatack I don’t think so. It’s possible that the service flag and transport type differ even with -v2transport=1.

jonatack commented at 5:09 pm on September 19, 2024: member

@sipa Thank you. Dropped the commit that removed the “v” column.

 0<->   type   net   serv  v  mping   ping send recv  txn  blk  hb addrp addrl  age  asmap  id 
 1 in        onion    nwl  1    360    360    1    7   19             38         19          4 
 2 in        onion    nwl  1    372    472    1    6                  19         17         45 
 3 in        onion    nwl  1    499    593   25   25                   2          4         89 
 4 in        onion   nbwl  1    512    608    1   34                   .         18         26 
 5 in        onion nbwcl2  2    523    610    1    0   18             37         19         11 
 6 in        onion         1    565    789   82   81    *              .         11         71 
 7 in        onion    nwl  1    579    579   18  906                             15         57 
 8 in          i2p   nwl2  1    585   1200    6    2    6             27         17         47 
 9 in        onion   nwl2  2    603    885    1    2                  29         17         38 
10 in        onion   nwl2  2    611   2714    1    2    4             39         18         20 
11 in        onion   nbwl  1    615    615    1   27                   .          4         87 
12 in        onion   nwl2  2    621    669    0    3   10             35         18         25 
13 in        onion    nwl  1    626   2463    1    6    5             39     1   19          7 
14 in        onion    nwl  1    647    651    1    4                  25         17         42 
15 in        onion   nwl2  2    659   1151    1    0                  37         17         48 
16 in          i2p   nwcl  1    867    867    1    0                              0        100 
17 in          i2p  nwcl2  2    936   1369    0    0                  16         15         56 
18 in          i2p   nwl2  1    972   1716    1    4                  30         17         39 
19 in        onion   nwl2  2   1020   2064    1    1    0             36         18         30 
20 in          i2p  nwcl2  2   1093   1176    1    4                  12          7         80 
21 in          i2p   nbwl  1   1094  14142    6    0   14             31         17         49 
22 in          i2p   nbwl  1   1245   1614    1    4                  16          6         82 
23 in          i2p  nbwl2  2   1425 340508   37  103                   2         11         69 
24 in          i2p    nwl  1   1546   1863    1    0                  24         18         21 
25 in          i2p   nbwl  1   2202   2202   74   68                              1         97 
26out  block  ipv4    nwl  1     91    100   88   87    *   10  .      .         19   7843   0 
27out   full  ipv4    wl2  2    102    108    2   13    1           1023         19  20115  10 
28out manual  ipv4    nwl  1    172    188    1    9    3           1017         19  47605   9 
29out manual cjdns   nwl2  2    183    183    1    4    0           1020         19          1 
30out   full  ipv4   nwl2  2    221    297    2    2    2           1016         18  35432  36 
31out  block onion    nwl  1    614    818   79   79    *              .         19          2 
32out   full onion   nwl2  2    645    772    0    0    0           1043         18         22 
33out   full onion    nwl  1    677   3582    0    0    0           1036         18         32 
34out   full   i2p  nbwcl  1    678   2217    2    0    3           1031         18         15 
35out manual onion    wl2  2    692    926    4    1    0           1010         19          3 
36out manual   i2p  nwcl2  2    748    910    4    0    0           1021         18         16 
37out manual onion  nwcl2  2    786    797    1    0    1           1022         17         40 
38out   full onion    nwl  1    795    916    0    0    0           1034         18         28 
39out  block onion    nwl  1    965    965   55   55    *              .          0         99 
40out   full   i2p    nwl  1   1042   1102    0    0    4           1019         11         70 
41out   full  ipv4    nwl  1  28462 527279    0   17                1001         18   1351  33 
42                               ms     ms  sec  sec  min  min                  min

tdb3 commented at 2:20 am on September 20, 2024: contributor

Approach ACK

Nice improvement. Performed a quick initial code review and exercised the changes interactively running on signet. I plan to circle back.

in src/bitcoin-cli.cpp:397 in 51206c693a outdated

395-    bool IsAddressSelected() const { return m_details_level == 2 || m_details_level == 4; }
396-    bool IsVersionSelected() const { return m_details_level == 3 || m_details_level == 4; }
397+    bool OutboundOnlySelected() const { return m_details_level == 5; }
398+    bool DetailsRequested() const { return m_details_level > 0 && m_details_level <= MAX_DETAIL_LEVEL; }
399+    bool IsAddressSelected() const { return m_details_level == 2 || m_details_level >= 4; }
400+    bool IsVersionSelected() const { return m_details_level == 3 || m_details_level >= 4; }

tdb3 commented at 2:38 pm on September 20, 2024:

nit: If touching the file again, looks like this could be simplified.

0- bool IsVersionSelected() const { return m_details_level == 3 || m_details_level >= 4; }
1+ bool IsVersionSelected() const { return m_details_level >= 3; }

jonatack commented at 7:43 pm on October 2, 2024:

done

in src/bitcoin-cli.cpp:465 in 8ca614d795 outdated

457@@ -456,6 +458,16 @@ class NetinfoRequestHandler : public BaseRequestHandler
458         if (conn_type == "addr-fetch") return "addr";
459         return "";
460     }
461+    std::string FormatServices(const UniValue& services)
462+    {
463+        std::string str;
464+        for (size_t i = 0; i < services.size(); ++i) {
465+            if (services[i].isNull()) continue;

tdb3 commented at 3:06 pm on September 20, 2024:

Do we expect to receive a null in servicesnames among other entries? If we don’t, maybe this should be something like assert(!services[i].isNull()); so the issue can be noticed?

jonatack commented at 9:34 pm on September 23, 2024:

Yes, I often see peers with no services, both new connections still setting up and longer-lasting ones, where getpeerinfo returns

0    "services": "0000000000000000",
1    "servicesnames": [
2    ],

jonatack commented at 9:43 pm on September 23, 2024:

Two examples right now. Longer-lasting ones…

0<->   type   net    serv  v  mping   ping send recv  txn  blk  hb addrp addrl  age  asmap    id address                                                             version
1 in        onion     nwl  1                 10   10                   .          0        16818 127.0.0.1:63651                                                     70016/Satoshi:26.1.0/
2 in        cjdns   nwcl2  2    282    295    5    3    4            377        246        15919 [fccb:248:11a6:1042:bca:1218:f7ce:7d3d]:40608                       70016/Satoshi:28.0.0/
3 in        onion          1    293    530   20   20    *              .        241        15949 127.0.0.1:64109                                                     70016
4 in        onion     nwl  1    316    349    5    1    0            414        237        15971 127.0.0.1:64399                                                     70016/Satoshi:26.0.0/
5 in        onion     nwl  1    323  89873    1   31    *              .        209        16068 127.0.0.1:49870                                                     70016/Satoshi:26.0.0/
6 in        onion    nwl2  2    326    634    5    1    5            428        237        15972 127.0.0.1:64405                                                     70016/Satoshi:27.1.0/
7 in        onion    nwl2  2    327    349    0    2    9            325        160        16232 127.0.0.1:54108                                                     70016/Satoshi:27.1.0/
8 in        onion          1    336    785    5   12                   .        237        15970 127.0.0.1:64391                                                     70016/Satoshi:0.16.3/

…and fresh connections

0<->   type   net    serv  v  mping   ping send recv  txn  blk  hb addrp addrl  age  asmap    id address                                                             version
1 in          i2p                                       *              .          0        16826 yn6fuglkgeib4runvfkdmsokxkcnaecvo3x6x3rpdm5eywsalbxa.b32.i2p:0
2 in        onion                                       *              .          0        16827 127.0.0.1:63746

tdb3 commented at 9:55 pm on September 23, 2024:

Sorry, I wasn’t as clear as I could have been. It seems possible to have no services (e.g. servicesnames has no elements, in which case the for loop wouldn’t be entered, right?). What I meant to ask is if we think we’ll see servicesnames with some non-zero number of elements, with at least one element satisfying the isNull() check? In other words, does the continue ever actually happen?

jonatack commented at 10:43 pm on September 23, 2024:

Good point, SGTM to use a CHECK_NONFATAL there in lieu of continue.

jonatack commented at 7:43 pm on October 2, 2024:

Looking at it more, I don’t think it’s needed and won’t be hit, so removed that line for now. (Thanks!)

tdb3 commented at 3:36 pm on September 20, 2024: contributor

Left a couple small comments. Tested that using 5 does include only outbound peers by connecting two nodes locally.

netinfo: add peer services column 50b4a0a5cd

netinfo: clarify relaytxes and addr_relay_enabled help docs 102995b381

netinfo: allow setting an outbound-only peer list

to keep the output within screen limits when running as a live dashboard,
i.e. with `watch`.

683b558a02

jonatack force-pushed on Oct 2, 2024

jonatack commented at 8:35 pm on October 2, 2024: member

Updated to take @tdb3 review feedback (thanks!) and make minor adjustments.

 0diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
 1index 3207a5d9c97..000f7695de6 100644
 2--- a/src/bitcoin-cli.cpp
 3+++ b/src/bitcoin-cli.cpp
 4@@ -391,10 +391,10 @@ private:
 5         return UNKNOWN_NETWORK;
 6     }
 7     uint8_t m_details_level{0}; //!< Optional user-supplied arg to set dashboard details level
 8-    bool OutboundOnlySelected() const { return m_details_level == 5; }
 9     bool DetailsRequested() const { return m_details_level > 0 && m_details_level <= MAX_DETAIL_LEVEL; }
10     bool IsAddressSelected() const { return m_details_level == 2 || m_details_level >= 4; }
11-    bool IsVersionSelected() const { return m_details_level == 3 || m_details_level >= 4; }
12+    bool IsVersionSelected() const { return m_details_level >= 3; }
13+    bool OutboundOnlySelected() const { return m_details_level == 5; }
14     bool m_is_asmap_on{false};
15     size_t m_max_addr_length{0};
16     size_t m_max_addr_processed_length{5};
17@@ -407,8 +407,8 @@ private:
18         std::string sub_version;
19         std::string conn_type;
20         std::string network;
21-        std::string services;
22         std::string age;
23+        std::string services;
24         std::string transport_protocol_type;
25         double min_ping;
26         double ping;
27@@ -463,7 +463,6 @@ private:
28     {
29         std::string str;
30         for (size_t i = 0; i < services.size(); ++i) {
31-            if (services[i].isNull()) continue;
32             const std::string s{services[i].get_str()};
33             str += s == "NETWORK_LIMITED" ? 'l' : s == "P2P_V2" ? '2' : ToLower(s[0]);
34         }
35@@ -539,7 +538,7 @@ public:
36                 const bool is_addr_relay_enabled{peer["addr_relay_enabled"].isNull() ? false : peer["addr_relay_enabled"].get_bool()};
37                 const bool is_bip152_hb_from{peer["bip152_hb_from"].get_bool()};
38                 const bool is_bip152_hb_to{peer["bip152_hb_to"].get_bool()};
39-                m_peers.push_back({addr, sub_version, conn_type, NETWORK_SHORT_NAMES[network_id], services, age, transport, min_ping, ping, addr_processed, addr_rate_limited, last_blck, last_recv, last_send, last_trxn, peer_id, mapped_as, version, is_addr_relay_enabled, is_bip152_hb_from, is_bip152_hb_to, is_outbound, is_tx_relay});
40+                m_peers.push_back({addr, sub_version, conn_type, NETWORK_SHORT_NAMES[network_id], age, services, transport, min_ping, ping, addr_processed, addr_rate_limited, last_blck, last_recv, last_send, last_trxn, peer_id, mapped_as, version, is_addr_relay_enabled, is_bip152_hb_from, is_bip152_hb_to, is_outbound, is_tx_relay});
41                 m_max_addr_length = std::max(addr.length() + 1, m_max_addr_length);
42                 m_max_addr_processed_length = std::max(ToString(addr_processed).length(), m_max_addr_processed_length);
43                 m_max_addr_rate_limited_length = std::max(ToString(addr_rate_limited).length(), m_max_addr_rate_limited_length);
44@@ -684,13 +683,13 @@ public:
45         "           \"addr\"   - address fetch; short-lived connection for requesting addresses\n"
46         "  net      Network the peer connected through (\"ipv4\", \"ipv6\", \"onion\", \"i2p\", \"cjdns\", or \"npr\" (not publicly routable))\n"
47         "  serv     Services offered by the peer\n"
48-        "           \"n\" - NETWORK, can serve the full block chain\n"
49-        "           \"b\" - BLOOM, can handle bloom-filtered connections (see BIP 111)\n"
50-        "           \"w\" - WITNESS, can be asked for blocks and transactions with witness data (SegWit)\n"
51-        "           \"c\" - COMPACT_FILTERS, can handle basic block filter requests (see BIPs 157 and 158)\n"
52-        "           \"l\" - NETWORK_LIMITED, limited to serving only the last 288 blocks (~2 days)\n"
53-        "           \"2\" - P2P_V2, supports version 2 P2P transport protocol, as defined in BIP324\n"
54-        "           \"u\" - UNKNOWN, unrecognized bit flag\n"
55+        "           \"n\" - NETWORK: peer can serve the full block chain\n"
56+        "           \"b\" - BLOOM: peer can handle bloom-filtered connections (see BIP 111)\n"
57+        "           \"w\" - WITNESS: peer can be asked for blocks and transactions with witness data (SegWit)\n"
58+        "           \"c\" - COMPACT_FILTERS: peer can handle basic block filter requests (see BIPs 157 and 158)\n"
59+        "           \"l\" - NETWORK_LIMITED: peer limited to serving only the last 288 blocks (~2 days)\n"
60+        "           \"2\" - P2P_V2: peer supports version 2 P2P transport protocol, as defined in BIP 324\n"
61+        "           \"u\" - UNKNOWN: unrecognized bit flag\n"
62         "  v        Version of transport protocol used for the connection\n"
63         "  mping    Minimum observed ping time, in milliseconds (ms)\n"
64         "  ping     Last observed ping time, in milliseconds (ms)\n"

tdb3 approved

tdb3 commented at 11:06 pm on October 2, 2024: contributor

code review and light test ACK 683b558a020f1632b0a3cbdaa165adbd1423281c

Good catch with age/services

1440000bytes approved

1440000bytes commented at 9:40 pm on October 3, 2024: none

ACK https://github.com/bitcoin/bitcoin/pull/30930/commits/683b558a020f1632b0a3cbdaa165adbd1423281c

netinfo: add peer services column and outbound-only peers list #30930

Code Coverage

Reviews

Conflicts