Print peer counts for all reachable networks in -netinfo

jonatack commented at 2:22 pm on October 20, 2021: member

instead of only for networks we have peer connections to.

Users reported the previous behavior caused confusion, as no column was printed when a network was reachable but no peers were connected. Users expected a column to be printed with 0 peers. This commit aligns behavior with that expectation.

In addition, the ipv4, ipv6, and onion columns were always printed whether or not they were reachable. With this change, only the reachable ones will be returned.

Example with CJDNS reachable but no CJDNS peers (built on #23077 and #23175):

before

0        ipv4    ipv6   onion     i2p   total   block  manual
1in         0       0      12       5      17
2out        8       1       6       4      19       2       8
3total      8       1      18       9      36

after

0         ipv4    ipv6   onion     i2p   cjdns   total   block  manual
1in          0       0      12       5       0      17
2out         8       1       6       4       0      19       2       8
3total       8       1      18       9       0      36

There is one additional space between the in/out/total row headers and the network counts.

netinfo: print peer counts for all reachable networks

instead of only for networks we have peer connections to.

Users reported the previous behavior caused confusion,
as no column was printed when a network was reachable
but no peers were connected. Users expected a column
to be printed with 0 peers. This commit aligns
behavior with that expectation.

96f469f91b

laanwj added the label Utils/log/libs on Oct 20, 2021

laanwj commented at 2:23 pm on October 20, 2021: member

Concept ACK

dunxen commented at 2:31 pm on October 20, 2021: contributor

Concept ACK

Thanks for this! I think this behaviour makes it much clearer and more consistent for all networks.

practicalswift commented at 2:43 pm on October 20, 2021: contributor

Concept ACK

Nice work! Could include before/after examples in the OP to make it even nicer? :)

jonatack commented at 3:03 pm on October 20, 2021: member

Nice work! Could include before/after examples in the OP to make it even nicer? :)

Added!

jsarenik commented at 4:01 pm on October 20, 2021: none

Tested ACK 96f469f91

 0$ ./bitcoin-cli -netinfo                                                       
 1Bitcoin Core client v22.99.0-96f469f91bc0 - server 70016/Satoshi:22.99.0/
 2
 3         ipv4   onion     i2p   total   block
 4in          0       0       0       0
 5out         9       0       0       9       2
 6total       9       0       0       9
 7
 8Local addresses
 9c35gtgyf5y44mh3tpzxlx4lfqaus2worc6pkmv72soqebxsncytuy6id.onion     port   8333    score      4
10qvzki6iqjpgme4v3h3dno5jvytjj56zizokkvgjytkr3shgnlbxa.b32.i2p       port      0    score      4

Thank you @jonatack !

ghost commented at 5:07 pm on October 20, 2021: none

utACK https://github.com/bitcoin/bitcoin/commit/96f469f91bc02a19703344cc439eab064b72081a

I had similar confusion when reviewing an i2p PR: #22250 (comment)

laanwj commented at 7:06 pm on October 20, 2021: member

Code review and lightly tested ACK 96f469f91bc02a19703344cc439eab064b72081a

hebasto commented at 9:24 pm on October 20, 2021: member

Concept ACK.

hebasto approved

hebasto commented at 9:42 pm on October 20, 2021: member

ACK 96f469f91bc02a19703344cc439eab064b72081a, tested by comparing the output of watch src/bitcoin-cli -netinfo with the Peer tab of the Node window in the GUI :tiger2:

Also verified with -noonion command-line option:

0$ src/bitcoin-cli -netinfo
1Bitcoin Core client v22.99.0-96f469f91bc0 - server 70016/Satoshi:22.99.0/
2
3         ipv4    ipv6   total   block
4in          0       0       0
5out         6       0       6       2
6total       6       0       6
7
8Local addresses: n/a

UPDATE: with -onlynet=ipv4 command-line option:

0$ src/bitcoin-cli -netinfo
1Bitcoin Core client v22.99.0-96f469f91bc0 - server 70016/Satoshi:22.99.0/
2
3         ipv4   onion   total   block  manual
4in          0       0       0
5out         2       1       3       2       1
6total       2       1       3
7
8Local addresses
9d6jwdcoo2l3gbjps6asgg4nhp2gn5oao3wj333o43ssqnjaliehytfad.onion     port   8333    score      4

ghost commented at 3:00 am on October 21, 2021: none

UPDATE: with -onlynet=ipv4 command-line option: @hebasto

Did you see zero for ‘onion’ after few minutes or had 1 onion outgoing connection for a long time with onlynet=ipv4?

I think this won’t happen after #22834 gets merged.

DrahtBot commented at 5:34 am on October 21, 2021: 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:

#23175 (Add CJDNS network to -addrinfo and -netinfo by jonatack)

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.

hebasto commented at 6:47 am on October 21, 2021: member

@prayank23

UPDATE: with -onlynet=ipv4 command-line option:

@hebasto

Did you see zero for ‘onion’ after few minutes or had 1 onion outgoing connection for a long time with onlynet=ipv4?

I think this won’t happen after #22834 gets merged.

This onion connection is addnoded.

naumenkogs commented at 12:04 pm on October 21, 2021: member

ACK 96f469f91bc02a19703344cc439eab064b72081a. This change is good for understanding node connectivity.

laanwj merged this on Oct 21, 2021

laanwj closed this on Oct 21, 2021

sidhujag referenced this in commit b43d231c1d on Oct 21, 2021

jonatack deleted the branch on Oct 21, 2021

laanwj referenced this in commit caf8b26b52 on Nov 15, 2021

sidhujag referenced this in commit 5371b96f1e on Nov 16, 2021

MarcoFalke referenced this in commit 66636ca438 on Feb 18, 2022

DrahtBot locked this on Oct 30, 2022

Print peer counts for all reachable networks in -netinfo #23324

Conflicts