cli -netinfo peer connections dashboard updates 🎄 ✨

jonatack commented at 7:35 pm on December 24, 2020: member

Merry Bitcoin Christmas! Ho ho ho 🎄 ✨

This PR updates -netinfo to:

use the getpeerinfo connection_type field (and no longer use getpeerinfo relaytxes for block-relay detection)
display manual peers count, if any, in the outbound row
display the block relay counts in the outbound row only
display high-bandwidth BIP152 compact block relay peers (hb column, to . and from *)
add support for displaying I2P network peers, if any are present

Testing and review welcome! How to test:

to run the full live dashboard (on Linux): $ watch --interval 1 --no-title ./src/bitcoin-cli -netinfo 4
to run the full dashboard: $ ./src/bitcoin-cli -netinfo 4
to see the help: $ ./src/bitcoin-cli -netinfo help
to see the help summary: $ ./src/bitcoin-cli -help | grep -A4 netinfo

DrahtBot commented at 0:10 am on December 25, 2020: member

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#20877 (netinfo: user help and argument parsing improvements by jonatack)
#20685 (Add I2P support using I2P SAM by vasild)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

jonatack force-pushed on Dec 26, 2020

sipa commented at 5:21 am on December 27, 2020: member

Concept ACK

michaelfolkson commented at 3:11 pm on December 27, 2020: contributor

Concept ACK. Is this ready to move out of draft? Would perhaps get more review if out of draft?

practicalswift commented at 7:58 pm on December 27, 2020: contributor

Concept ACK

-netinfo is great! :)

jonatack marked this as ready for review on Dec 27, 2020

jonatack force-pushed on Dec 27, 2020

jonatack commented at 11:07 pm on December 27, 2020: member

Moved out of draft and ready for review with updated PR description and “how to test” suggestions. The last commit that adds displaying BIP152 high-bandwidth peers can’t be backported to rc4, but the other commits, including the user help, could be if desired.

 0$ ./src/bitcoin-cli -netinfo help
 1-netinfo level "help" 
 2
 3Returns a network peer connections dashboard with information from the remote server.
 4Under the hood, -netinfo fetches the data by calling getpeerinfo and getnetworkinfo.
 5An optional integer argument from 0 to 4 can be passed for different peers listings.
 6Pass "help" to see this detailed help documentation.
 7If more than one argument is passed, only the first one is read and parsed.
 8Suggestion: use with the Linux watch command for a live dashboard; see example below.
 9
10Arguments:
111. level (integer 0-4, optional)  Specify the info level of the peers dashboard (default 0):
12                                  0 - Connection counts and local addresses
13                                  1 - Like 0 but with a detailed peers listing (without address or version columns)
14                                  2 - Like 1 but with an address column
15                                  3 - Like 1 but with a version column
16                                  4 - Like 1 but with both address and version columns
172. help (string "help", optional) Print this help documentation instead of the dashboard.
18
19The detailed peers listing in levels 1-4 displays all of the peers, sorted by direction and minimum ping time.
20
21Column   Description
22------   -----------
23<->      Direction
24         "in"  - inbound connections are those initiated by the peer
25         "out" - outbound connections are those initiated by us
26type     Type of peer connection
27         "full"   - full relay, the default
28         "block"  - block relay; like full relay but does not relay transactions or addresses
29         "manual" - peer we manually added using RPC addnode or the -addnode/-connect config options
30         "feeler" - short-lived connection for testing addresses
31         "addr"   - address fetch; short-lived connection for requesting addresses
32net      Network the peer connected through ("ipv4", "ipv6", "onion", "i2p", or "cjdns")
33mping    Minimum observed ping time in milliseconds (ms)
34ping     Ping time in milliseconds (ms)
35send     Time since last message sent to the peer, in seconds
36recv     Time since last message received from the peer, in seconds
37txn      Time since last novel transaction received from the peer and accepted into our mempool, in minutes
38blk      Time since last novel block passing initial validity checks received from the peer, in minutes
39hb       High-bandwidth BIP152 compact block relay
40         "." (to)   - we selected the peer as a high-bandwidth peer
41         "*" (from) - the peer selected us as a high-bandwidth peer
42age      Duration of connection to the peer, in minutes
43asmap    Mapped AS (Autonomous System) number in the BGP route to the peer, used for diversifying peer selection
44         (only displayed if the -asmap config option is set)
45id       Peer index, in increasing order of peer connections since node startup
46address  IP address and port of the peer
47version  Peer version and subversion concatenated, e.g. "70016/Satoshi:21.0.0/"
48
49Examples:
50
51Connection counts and local addresses only
52> bitcoin-cli -netinfo
53
54Compact peers listing
55> bitcoin-cli -netinfo 1
56
57Full dashboard
58> bitcoin-cli -netinfo 4
59
60Full live dashboard, adjust --interval or --no-title as needed (Linux)
61> watch --interval 1 --no-title ./src/bitcoin-cli -netinfo 4
62
63See this help
64> bitcoin-cli -netinfo help

jonasschnelli commented at 8:06 pm on December 28, 2020: contributor

Neat! Concept ACK

in src/bitcoin-cli.cpp:366 in f095dd33ab outdated

351@@ -349,6 +352,72 @@ class NetinfoRequestHandler : public BaseRequestHandler
352         const double milliseconds{round(1000 * seconds)};
353         return milliseconds > 999999 ? "-" : ToString(milliseconds);
354     }
355+    std::string ConnectionTypeForNetinfo(const std::string& conn_type) const
356+    {
357+        if (conn_type == "outbound-full-relay" || conn_type == "inbound") return "full";
358+        if (conn_type == "block-relay-only") return "block";
359+        if (conn_type == "manual" || conn_type == "feeler") return conn_type;
360+        if (conn_type == "addr-fetch") return "addr";

glozow commented at 11:50 pm on December 28, 2020:

nit: this can be a lot cleaner as a switch statement

jonatack commented at 0:27 am on December 29, 2020:

Yes, if an integral or enum connection type was exposed, but getpeerinfo provides it as a string, so this is the best we can do, I think.

https://en.cppreference.com/w/cpp/language/switch

jonatack commented at 0:30 am on December 29, 2020:

(Client side, I think we have to make do data-wise with what the server side rpc provides. We can’t hook in here to CNodeStats; it would be a layer violation).

laanwj commented at 12:22 pm on January 7, 2021:

Yea it’s too bad that C++ doesn’t have case pattern matching on strings, otherwise that’d be a lot cleaner. (this applies to some other places as well such as the P2P message dispatch)

in src/bitcoin-cli.cpp:395 in f095dd33ab outdated

390+            "         \"manual\" - peer we manually added using RPC addnode or the -addnode/-connect config options\n"
391+            "         \"feeler\" - short-lived connection for testing addresses\n"
392+            "         \"addr\"   - address fetch; short-lived connection for requesting addresses\n"
393+            "net      Network the peer connected through (\"ipv4\", \"ipv6\", \"onion\", \"i2p\", or \"cjdns\")\n"
394+            "mping    Minimum observed ping time in milliseconds (ms)\n"
395+            "ping     Ping time in milliseconds (ms)\n"

glozow commented at 11:51 pm on December 28, 2020:

“Latest ping time” would be more accurate imo

jonatack commented at 8:32 pm on December 29, 2020:

Thanks @glozow, changed to “Last observed ping time.” Should go well with its neighbor, “Minimum observed ping time.”

in src/bitcoin-cli.cpp:336 in f095dd33ab outdated

332@@ -331,7 +333,8 @@ class NetinfoRequestHandler : public BaseRequestHandler
333         int id;
334         int mapped_as;
335         int version;
336-        bool is_block_relay;

glozow commented at 11:55 pm on December 28, 2020:

Perhaps worth keeping, e.g. might need it again for something like #20726

jonatack commented at 7:23 pm on January 10, 2021:

Yes, dropped the commit in order to keep is_block_relay and reworked the connection type logic to distinguish between inbound full relay (based on fRelayTxes like in AttemptToEvictConnection) and outbound block relay (based on m_conn_type).

glozow commented at 0:18 am on December 29, 2020: member

Concept ACK, I tried it out - very nice :)

An idea - you could omit the txn column during IBD since the node isn’t validating (and shouldn’t be receiving) txns at all.

The second table should be mentioned in help imo, and in general just seems really weird to me. We have a column after the “overall totals” and it has a slot for inbound block-relay-only, which doesn’t exist afaik? Seems like it would make more sense for block/manual to be rows instead or have a separate table or something.

jonatack commented at 9:28 pm on December 29, 2020: member

Thanks for your ideas @glozow!

The second table should be mentioned in help imo, and in general just seems really weird to me. We have a column after the “overall totals” and it has a slot for inbound block-relay-only, which doesn’t exist afaik? Seems like it would make more sense for block/manual to be rows instead or have a separate table or something.

This is valuable, as I don’t have a fresh perspective. I’d like to keep the connection counts in as compact a format as we can, but any ideas welcome. How about this? Screenshot from 2020-12-29 22-21-12

Edit: updated the screenshot in the PR description to this

jonatack force-pushed on Dec 29, 2020

jonatack commented at 11:10 pm on December 29, 2020: member

Updated:

Rewrote the help, adding suggestions by @glozow and improved formatting
Display the block relay and manual counts in the outbound row only

jonatack commented at 11:13 pm on December 29, 2020: member

 0$ ./src/bitcoin-cli -netinfo help
 1-netinfo level|"help" 
 2
 3Returns a network peer connections dashboard with information from the remote server.
 4Under the hood, -netinfo fetches the data by calling getpeerinfo and getnetworkinfo.
 5An optional integer argument from 0 to 4 can be passed for different peers listings.
 6Pass "help" to see this detailed help documentation.
 7If more than one argument is passed, only the first one is read and parsed.
 8Suggestion: use with the Linux watch(1) command for a live dashboard; see example below.
 9
10Arguments:
111. level (integer 0-4, optional)  Specify the info level of the peers dashboard (default 0):
12                                  0 - Connection counts and local addresses
13                                  1 - Like 0 but with a peers listing (without address or version columns)
14                                  2 - Like 1 but with an address column
15                                  3 - Like 1 but with a version column
16                                  4 - Like 1 but with both address and version columns
172. help (string "help", optional) Print this help documentation instead of the dashboard.
18
19Result:
20
21* The peers listing in levels 1-4 displays all of the peers sorted by direction and minimum ping time:
22
23  Column   Description
24  ------   -----------
25  <->      Direction
26           "in"  - inbound connections are those initiated by the peer
27           "out" - outbound connections are those initiated by us
28  type     Type of peer connection
29           "full"   - full relay, the default
30           "block"  - block relay; like full relay but does not relay transactions or addresses
31           "manual" - peer we manually added using RPC addnode or the -addnode/-connect config options
32           "feeler" - short-lived connection for testing addresses
33           "addr"   - address fetch; short-lived connection for requesting addresses
34  net      Network the peer connected through ("ipv4", "ipv6", "onion", "i2p", or "cjdns")
35  mping    Minimum observed ping time, in milliseconds (ms)
36  ping     Last observed ping time, in milliseconds (ms)
37  send     Time since last message sent to the peer, in seconds
38  recv     Time since last message received from the peer, in seconds
39  txn      Time since last novel transaction received from the peer and accepted into our mempool, in minutes
40  blk      Time since last novel block passing initial validity checks received from the peer, in minutes
41  hb       High-bandwidth BIP152 compact block relay
42           "." (to)   - we selected the peer as a high-bandwidth peer
43           "*" (from) - the peer selected us as a high-bandwidth peer
44  age      Duration of connection to the peer, in minutes
45  asmap    Mapped AS (Autonomous System) number in the BGP route to the peer, used for diversifying
46           peer selection (only displayed if the -asmap config option is set)
47  id       Peer index, in increasing order of peer connections since node startup
48  address  IP address and port of the peer
49  version  Peer version and subversion concatenated, e.g. "70016/Satoshi:21.0.0/"
50
51* The connection counts table displays the number of peers by direction, network, and the totals
52  for each, as well as two special outbound columns for block relay peers and manual peers.
53
54* The local addresses table lists each local address broadcast by the node, the port, and the score.
55
56Examples:
57
58Connection counts and local addresses only
59> bitcoin-cli -netinfo
60
61Compact peers listing
62> bitcoin-cli -netinfo 1
63
64Full dashboard
65> bitcoin-cli -netinfo 4
66
67Full live dashboard, adjust --interval or --no-title as needed (Linux)
68> watch --interval 1 --no-title bitcoin-cli -netinfo 4
69
70See this help
71> bitcoin-cli -netinfo help

theStack commented at 1:28 pm on January 4, 2021: member

Very nice that Santa Claus also came to present a gift to Bitcoin Core ;) 🎅 Concept ACK, plan to review this within the next days.

jonatack force-pushed on Jan 5, 2021

jonatack commented at 10:58 pm on January 5, 2021: member

Rebased on top of #20829 (the first commit, “netinfo: add user help documentation”).

jonatack force-pushed on Jan 5, 2021

laanwj referenced this in commit 3b6d1b61d3 on Jan 6, 2021

sidhujag referenced this in commit b33831f81d on Jan 6, 2021

DrahtBot added the label Needs rebase on Jan 6, 2021

felipsoarez commented at 4:32 pm on January 6, 2021: none

Concept ACK, not tested

Good job, more information for better adaptations -netinfo is great

jonatack force-pushed on Jan 6, 2021

jonatack commented at 4:46 pm on January 6, 2021: member

Rebased after the merge of #20829. The diff is now much smaller and this should be ready for final review.

DrahtBot removed the label Needs rebase on Jan 6, 2021

jonatack force-pushed on Jan 10, 2021

jonatack commented at 7:31 pm on January 10, 2021: member

Dropped commit 97127a870 “netinfo: only display outbound block relay peers count.”

jonatack commented at 7:40 pm on January 18, 2021: member

Added a commit to support displaying connections to peers via the I2P network. An i2p peer counts column is displayed if the user is connected to any I2P peers. Otherwise there is no user-facing change, and this change is compatible with current master.

netinfo: add ConnectionTypeForNetinfo member helper function 62bf5b7850

jonatack force-pushed on Feb 2, 2021

jonatack commented at 3:47 pm on February 2, 2021: member

Updated connection type handling to display inbounds without a “full” or “block” type description from fRelayTxes.

Will give this a few days and if still no ACKs will close in favor of proposing the changes individually in separate pulls.

michaelfolkson commented at 5:31 pm on February 2, 2021: contributor

You haven’t started splitting this into separate pulls have you @jonatack? If no I will review this tomorrow. Otherwise I will wait for the separate pulls.

jonatack commented at 5:42 pm on February 2, 2021: member

Thanks 🙏 leaving as-is for now, one merge is easier than several 🐇 🐢

laanwj commented at 8:16 am on February 3, 2021: member

Code review and shallowly tested ACK ~~9628f9953d81c380c780046ef30a2f61c96d4966~~ re-ACK 747cb5b9949f80b3b4516f382a0ce80e41f3f5a6

I am a bit divided on adding I2P support here before #20685 is merged, but also don’t see it as a problem that the client can already handle this.

in src/bitcoin-cli.cpp:523 in 9628f9953d outdated

522                 result += strprintf(
523-                    "%3s %5s %5s%7s%7s%5s%5s%5s%5s %*s%*i %*s %-*s%s\n",
524+                    "%3s %6s %5s%7s%7s%5s%5s%5s%5s  %2s %*s%*i %*s %-*s%s\n",
525                     peer.is_outbound ? "out" : "in",
526-                    peer.is_block_relay ? "block" : "full",
527+                    ConnectionTypeForNetinfo(peer.conn_type),

MarcoFalke commented at 8:49 am on February 3, 2021:

In your screenshot an i2p inbound peer will print “full”, which is short for “outbound-full-relay”. Is this something wrong with i2p or with this pull?

jonatack commented at 8:51 am on February 3, 2021:

The screenshot in the PR description was not the latest version. Removed it.

jonatack commented at 8:55 am on February 3, 2021:

The inbound handling was updated after the i2p screenshot. Removed the i2p screenshot.

jonatack force-pushed on Feb 3, 2021

netinfo: update to use peer connection types d3cca3be63

netinfo: display manual peers count 5de7a6cf63

netinfo: add bip152 high-bandwidth to/from fields 9d6aeca2c5

netinfo: add i2p network

the i2p peer counts column is displayed iff the node is connected
to at least one i2p peer, so this doesn't add clutter for users
who are not running an i2p service

76d198a5c1

netinfo: display only outbound block relay counts 747cb5b994

jonatack force-pushed on Feb 3, 2021

jonatack commented at 1:42 pm on February 3, 2021: member

Updated the last commit to only display block relay peer counts in the outbound row, and simplified it and commit netinfo: display manual peers count by persisting the counts in a member variable instead of in the m_counts 2D array. @laanwj Thanks for reviewing! I apologize for pushing again after. I think this is finally done.

michaelfolkson commented at 5:39 pm on February 4, 2021: contributor

ACK 747cb5b9949f80b3b4516f382a0ce80e41f3f5a6

I really like it (obviously) and I’ve only done limited testing (no Tor or I2P connections)

display high-bandwidth BIP152 compact block relay peers (hb column, to . and from *)

Maybe it is just me but I feel as if I’m going to have to look at the help every time to remember what . and * represents. Presumably there were no better options than . and *? Obviously in and out are already taken and to and from took up too much space?

My understanding is a little hazy of BIP 152 selection process too so maybe I personally just wouldn’t care about this. I can look it up though!

jonatack commented at 9:40 pm on February 4, 2021: member

Thanks @michaelfolkson for reviewing! The idea is . for an outgoing signal (“to”; we selected the peer) and * for an inbound signal (“from”; the peer selected us). Kind of like a train with a bright headlight and a muted tail light. Though if you see them as starships instead I suppose an argument could be made for the inverse :smile:

Screenshot from 2020-12-29

jonasschnelli commented at 8:42 am on February 5, 2021: contributor

~~Should this also be compatible with -testnet (and eventually -regtest)?~~

never-mind: it is.

jonasschnelli approved

jonasschnelli commented at 8:45 am on February 5, 2021: contributor

Tested ACK 747cb5b9949f80b3b4516f382a0ce80e41f3f5a6 - works nicely. Great that this PR only changes bitcoin-cli.

laanwj merged this on Feb 5, 2021

laanwj closed this on Feb 5, 2021

jonatack deleted the branch on Feb 5, 2021

sidhujag referenced this in commit b456f6c50c on Feb 5, 2021

laanwj referenced this in commit cabe63759c on Mar 3, 2021

DrahtBot locked this on Aug 16, 2022

cli -netinfo peer connections dashboard updates 🎄 ✨ #20764

Conflicts