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

jonatack · 2021-10-20T14:22:51Z

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

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

after

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

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

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.

laanwj · 2021-10-20T14:23:47Z

Concept ACK

dunxen · 2021-10-20T14:31:23Z

Concept ACK

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

practicalswift · 2021-10-20T14:43:03Z

Concept ACK

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

jonatack · 2021-10-20T15:03:03Z

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

Added!

jsarenik · 2021-10-20T16:01:47Z

Tested ACK 96f469f

$ ./bitcoin-cli -netinfo                                                       
Bitcoin Core client v22.99.0-96f469f91bc0 - server 70016/Satoshi:22.99.0/

         ipv4   onion     i2p   total   block
in          0       0       0       0
out         9       0       0       9       2
total       9       0       0       9

Local addresses
c35gtgyf5y44mh3tpzxlx4lfqaus2worc6pkmv72soqebxsncytuy6id.onion     port   8333    score      4
qvzki6iqjpgme4v3h3dno5jvytjj56zizokkvgjytkr3shgnlbxa.b32.i2p       port      0    score      4

Thank you @jonatack !

ghost · 2021-10-20T17:07:31Z

utACK 96f469f

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

laanwj · 2021-10-20T19:06:31Z

Code review and lightly tested ACK 96f469f

hebasto · 2021-10-20T21:24:36Z

Concept ACK.

hebasto

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

Also verified with -noonion command-line option:

$ src/bitcoin-cli -netinfo
Bitcoin Core client v22.99.0-96f469f91bc0 - server 70016/Satoshi:22.99.0/

         ipv4    ipv6   total   block
in          0       0       0
out         6       0       6       2
total       6       0       6

Local addresses: n/a

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

$ src/bitcoin-cli -netinfo
Bitcoin Core client v22.99.0-96f469f91bc0 - server 70016/Satoshi:22.99.0/

         ipv4   onion   total   block  manual
in          0       0       0
out         2       1       3       2       1
total       2       1       3

Local addresses
d6jwdcoo2l3gbjps6asgg4nhp2gn5oao3wj333o43ssqnjaliehytfad.onion     port   8333    score      4

ghost · 2021-10-21T03:00:46Z

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 · 2021-10-21T05:34:05Z

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 · 2021-10-21T06:47:55Z

@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 · 2021-10-21T12:04:17Z

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

…-netinfo 96f469f netinfo: print peer counts for all reachable networks (Jon Atack) Pull request description: 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 bitcoin#23077 and bitcoin#23175): before ``` ipv4 ipv6 onion i2p total block manual in 0 0 12 5 17 out 8 1 6 4 19 2 8 total 8 1 18 9 36 ``` after ``` ipv4 ipv6 onion i2p cjdns total block manual in 0 0 12 5 0 17 out 8 1 6 4 0 19 2 8 total 8 1 18 9 0 36 ``` There is one additional space between the in/out/total row headers and the network counts. ACKs for top commit: jsarenik: Tested ACK 96f469f prayank23: utACK bitcoin@96f469f laanwj: Code review and lightly tested ACK 96f469f naumenkogs: ACK 96f469f. hebasto: ACK 96f469f, tested by comparing the output of `watch src/bitcoin-cli -netinfo` with the Peer tab of the Node window in the GUI 🐅 Tree-SHA512: 3489f40148a2bd0afc9eef1e1577d44150c1fccec8dbf2a675bc23aa9343bfcae6c4039f5b96e54730668c83f40bc932fb6808f5540e86ff7249fde8dc0fff67

7b65287 cli: hoist networks class data members to a constant (Jon Atack) 5bd40a3 cli: add cjdns network to -addrinfo and -netinfo (Jon Atack) Pull request description: Follow-up to #23077 and #23324. ``` $ ./src/bitcoin-cli -addrinfo { "addresses_known": { "ipv4": 47782, "ipv6": 10307, "onion": 8030, "i2p": 41, "cjdns": 1, "total": 66161 } } $ ./src/bitcoin-cli -netinfo Bitcoin Core client v22.99.0-deb6223d4c55 - server 70016/Satoshi:22.99.0(jon)/ ipv4 ipv6 onion i2p cjdns total block manual in 0 5 12 5 1 23 out 2 2 9 5 2 20 2 10 total 2 7 21 10 3 43 ``` ``` $ ./src/bitcoin-cli -netinfo 1 ``` ![Screenshot from 2021-10-10 12-01-58](https://user-images.githubusercontent.com/2415484/136691258-8b3fa7aa-3edb-4428-854a-adadfef302e3.png) ACKs for top commit: laanwj: Code review ACK 7b65287 Tree-SHA512: 9c740d394d9842d38a1c01a824271b25277baac11ed090f0430daa15b580c2bf3d114ac6b8254b19d6aaee57cbe1ca6a414996e6994e0bf4a577bed771382eca

7b65287 cli: hoist networks class data members to a constant (Jon Atack) 5bd40a3 cli: add cjdns network to -addrinfo and -netinfo (Jon Atack) Pull request description: Follow-up to bitcoin#23077 and bitcoin#23324. ``` $ ./src/bitcoin-cli -addrinfo { "addresses_known": { "ipv4": 47782, "ipv6": 10307, "onion": 8030, "i2p": 41, "cjdns": 1, "total": 66161 } } $ ./src/bitcoin-cli -netinfo Bitcoin Core client v22.99.0-deb6223d4c55 - server 70016/Satoshi:22.99.0(jon)/ ipv4 ipv6 onion i2p cjdns total block manual in 0 5 12 5 1 23 out 2 2 9 5 2 20 2 10 total 2 7 21 10 3 43 ``` ``` $ ./src/bitcoin-cli -netinfo 1 ``` ![Screenshot from 2021-10-10 12-01-58](https://user-images.githubusercontent.com/2415484/136691258-8b3fa7aa-3edb-4428-854a-adadfef302e3.png) ACKs for top commit: laanwj: Code review ACK 7b65287 Tree-SHA512: 9c740d394d9842d38a1c01a824271b25277baac11ed090f0430daa15b580c2bf3d114ac6b8254b19d6aaee57cbe1ca6a414996e6994e0bf4a577bed771382eca

… and devs a4da16f Improve -netinfo help based on feedback from users and devs (Jon Atack) Pull request description: Clarify which networks are displayed by the peer counts table (*reachable* networks; follow-up to #23324) in response to questions received over the past months, and a few other improvements. ACKs for top commit: laanwj: Code review ACK a4da16f w0xlt: ACK a4da16f kristapsk: utACK a4da16f Tree-SHA512: e6522c08421aa7f10d50723156d0a8fc5ec82cad2f0bd931bbec603077fcd4921c6505ef743d57386fba81c95dcfc77df75abf3378319886368e4ae33f9a6d73

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

hebasto approved these changes Oct 20, 2021

View reviewed changes

DrahtBot mentioned this pull request Oct 21, 2021

Add CJDNS network to -addrinfo and -netinfo #23175

Closed

laanwj merged commit ee1294f into bitcoin:master Oct 21, 2021

jonatack deleted the netinfo-print-peer-counts-for-all-reachable-networks branch October 21, 2021 16:28

prusnak mentioned this pull request Nov 19, 2021

Add Yggdrasil support #23531

Closed

jonatack mentioned this pull request Feb 16, 2022

doc: improve -netinfo help based on feedback from users and devs #24360

Merged

bitcoin locked and limited conversation to collaborators Oct 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

jonatack commented Oct 20, 2021 •

edited

Loading

Uh oh!

laanwj commented Oct 20, 2021

Uh oh!

dunxen commented Oct 20, 2021 •

edited

Loading

Uh oh!

practicalswift commented Oct 20, 2021 •

edited

Loading

Uh oh!

jonatack commented Oct 20, 2021

Uh oh!

jsarenik commented Oct 20, 2021 •

edited

Loading

Uh oh!

ghost commented Oct 20, 2021 •

edited by ghost

Loading

Uh oh!

laanwj commented Oct 20, 2021

Uh oh!

hebasto commented Oct 20, 2021

Uh oh!

hebasto left a comment •

edited

Loading

Uh oh!

ghost commented Oct 21, 2021 •

edited by ghost

Loading

Uh oh!

DrahtBot commented Oct 21, 2021

Uh oh!

hebasto commented Oct 21, 2021

Uh oh!

naumenkogs commented Oct 21, 2021

Uh oh!

Uh oh!

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

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

Uh oh!

Conversation

jonatack commented Oct 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj commented Oct 20, 2021

Uh oh!

dunxen commented Oct 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Oct 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonatack commented Oct 20, 2021

Uh oh!

jsarenik commented Oct 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Oct 20, 2021 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj commented Oct 20, 2021

Uh oh!

hebasto commented Oct 20, 2021

Uh oh!

hebasto left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Oct 21, 2021 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Oct 21, 2021

Conflicts

Uh oh!

hebasto commented Oct 21, 2021

Uh oh!

naumenkogs commented Oct 21, 2021

Uh oh!

Uh oh!

jonatack commented Oct 20, 2021 •

edited

Loading

dunxen commented Oct 20, 2021 •

edited

Loading

practicalswift commented Oct 20, 2021 •

edited

Loading

jsarenik commented Oct 20, 2021 •

edited

Loading

ghost commented Oct 20, 2021 •

edited by ghost

Loading

hebasto left a comment •

edited

Loading

ghost commented Oct 21, 2021 •

edited by ghost

Loading