rpc, cli: add network in/out connections to `getnetworkinfo` and `-getinfo` by jonatack · Pull Request #19405 · bitcoin/bitcoin

jonatack · 2020-06-29T09:00:03Z

This is basic info that is present in the GUI that I've been wishing to have exposed via the RPC and CLI without needing a bash workaround or script. For human users it would also be useful to have it in -getinfo.

bitcoin-cli getnetworkinfo

  "connections": 15,
  "connections_in": 6,
  "connections_out": 9,

bitcoin-cli -getinfo

  "connections": {
    "in": 6,
    "out": 9,
    "total": 15
  },

Update the tests, RPC help, and release notes for the changes. Also fixup the getnettotals timemillis help while touching rpc/net.cpp.

Reviewers can manually test this PR by building from source, launching bitcoind, and then running bitcoin-cli -getinfo, bitcoin-cli getnetworkinfo, bitcoin-cli help getnetworkinfo, and bitcoin-cli help getnettotals (for the UNIX epoch time change).

src/rpc/net.cpp

laanwj · 2020-06-29T13:52:43Z

This is info I've been wishing to have for some time -- probably there is an RPC I'm overlooking? But for human users it seems good to have it in -getinfo.

Yeah, I've personally always used a stats script that goes through getpeerinfo and categorizes them by in/out × ipv4/ipv6/tor.

in:  ipv4 139 ipv6  20 tor   0
out: ipv4  10 ipv6   0 tor   0

Seems like something that could easily be done client-side in bitcoin-cli itself. But no objections to adding this server-side, it's a small code change.

practicalswift · 2020-06-29T14:00:27Z

Concept ACK

jonatack · 2020-06-29T15:14:40Z

Re-pushed with release notes.

Yeah, I've personally always used a stats script that goes through getpeerinfo and categorizes them by in/out × ipv4/ipv6/tor.
in:  ipv4 139 ipv6  20 tor   0
out: ipv4  10 ipv6   0 tor   0
Seems like something that could easily be done client-side in bitcoin-cli itself. But no objections to adding this server-side, it's a small code change.

That's pretty nice. In terms of vertical space for human use, something like that might be better as string values

  "connections": 149,
  "in": "ipv4 139 ipv6 20 tor 0",
  "out": "ipv4 10 ipv6  0 tor 0",

rather than a JSON object

  "connections": {
    "in": {
      "ipv4": 139,
      "ipv6": 20,
      "tor": 0
    },
    "out": {
      "ipv4": 10,
      "ipv6": 0,
      "tor": 0
    },
    "total": 149
  },

We could perhaps create a new CLI -netinfo option to provide a more detailed report of the network peer connections.

maflcko

Concept ACK, but I am wondering if instead the connection type (#19316) should be returned?

src/rpc/net.cpp

jonatack · 2020-06-29T18:01:39Z

Concept ACK, but I am wondering if instead the connection type (#19316) should be returned?

That PR seems orthogonal, as it proposes to detail the outbounds into various subtypes, but the basic distinction remains between inbound and outbound connections.

I reckon that exposing to users and RPC clients the counts of the outbound subtypes in addition to in/out/total would be enough detail and screen space to warrant a separate client-side -netinfo option in a future PR that could also provide @laanwj's ip4/6/tor breakdown.

jonatack · 2020-06-29T18:27:54Z

Addressed @MarcoFalke's suggestions. Thanks everyone. Rebased per git diff 9290a98 94a792cc.

jnewbery · 2020-07-01T13:35:54Z

I have no objection to adding this to getnetworkinfo, but just noting that jq is your friend here:

bitcoin-cli getpeerinfo | jq "group_by(.inbound) | map({Inbound: .[0].inbound, Count: length}) | .[]"
{
  "Inbound": false,
  "Count": 9
}
{
  "Inbound": true,
  "Count": 6
}

willcl-ark · 2020-07-02T14:25:42Z

tACK.

Changes seem simple-enough and are working (and passing tests) on MacOS 10.15.

Also pleased to learn jnewbery's advanced jq pipe from above :)

eriknylund · 2020-07-15T09:22:10Z

ACK 94a792cc19f26754ac5d60a1b733d4b520f7d0c7

Ran unit tests and extended functional tests, all pass. Did not run fuzz tests. Started bitcoind on a clean datadir and ran the bitcoin-cli commands to verify the new output:

$ ./bitcoind -datadir=/tmp/pr19405

Tested -getinfo

$ ./bitcoin-cli -datadir=/tmp/pr19405 -getinfo
{
  "version": 209900,
  "blocks": 0,
  "headers": 192000,
  "verificationprogress": 1.827923226519457e-09,
  "timeoffset": 0,
  "connections": {
    "in": 0,
    "out": 6,
    "total": 6
  },
  "proxy": "",
  "difficulty": 1,
  "chain": "main",
  "keypoolsize": 1000,
  "paytxfee": 0.00000000,
  "balance": 0.00000000,
  "relayfee": 0.00001000,
  "warnings": "This is a pre-release test build - use at your own risk - do not use for mining or merchant applications"
}

Tested getnetworkinfo:

$ ./bitcoin-cli -datadir=/tmp/pr19405 getnetworkinfo
{
  "version": 209900,
  "subversion": "/Satoshi:0.20.99/",
  "protocolversion": 70015,
  "localservices": "0000000000000409",
  "localservicesnames": [
    "NETWORK",
    "WITNESS",
    "NETWORK_LIMITED"
  ],
  "localrelay": true,
  "timeoffset": 0,
  "networkactive": true,
  "connections": 7,
  "connections_in": 0,
  "connections_out": 7,
  "networks": [
    {
      "name": "ipv4",
      "limited": false,
      "reachable": true,
      "proxy": "",
      "proxy_randomize_credentials": false
    },
    {
      "name": "ipv6",
      "limited": false,
      "reachable": true,
      "proxy": "",
      "proxy_randomize_credentials": false
    },
    {
      "name": "onion",
      "limited": true,
      "reachable": false,
      "proxy": "",
      "proxy_randomize_credentials": false
    }
  ],
  "relayfee": 0.00001000,
  "incrementalfee": 0.00001000,
  "localaddresses": [
  ],
  "warnings": "This is a pre-release test build - use at your own risk - do not use for mining or merchant applications"
}

Verified that the help contains the new fields as well:

$ ./bitcoin-cli -datadir=/tmp/pr19405 help getnetworkinfo
getnetworkinfo
Returns an object containing various state info regarding P2P networking.

Result:
{                                                    (json object)
  "version" : n,                                     (numeric) the server version
  "subversion" : "str",                              (string) the server subversion string
  "protocolversion" : n,                             (numeric) the protocol version
  "localservices" : "hex",                           (string) the services we offer to the network
  "localservicesnames" : [                           (json array) the services we offer to the network, in human-readable form
    "str",                                           (string) the service name
    ...
  ],
  "localrelay" : true|false,                         (boolean) true if transaction relay is requested from peers
  "timeoffset" : n,                                  (numeric) the time offset
  "connections" : n,                                 (numeric) the total number of connections
  "connections_in" : n,                              (numeric) the number of inbound connections
  "connections_out" : n,                             (numeric) the number of outbound connections
  "networkactive" : true|false,                      (boolean) whether p2p networking is enabled
  "networks" : [                                     (json array) information per network
    {                                                (json object)
      "name" : "str",                                (string) network (ipv4, ipv6 or onion)
      "limited" : true|false,                        (boolean) is the network limited using -onlynet?
      "reachable" : true|false,                      (boolean) is the network reachable?
      "proxy" : "str",                               (string) ("host:port") the proxy that is used for this network, or empty if none
      "proxy_randomize_credentials" : true|false     (boolean) Whether randomized credentials are used
    },
    ...
  ],
  "relayfee" : n,                                    (numeric) minimum relay fee for transactions in BTC/kB
  "incrementalfee" : n,                              (numeric) minimum fee increment for mempool limiting or BIP 125 replacement in BTC/kB
  "localaddresses" : [                               (json array) list of local addresses
    {                                                (json object)
      "address" : "str",                             (string) network address
      "port" : n,                                    (numeric) network port
      "score" : n                                    (numeric) relative score
    },
    ...
  ],
  "warnings" : "str"                                 (string) any network and blockchain warnings
}
`

test/functional/rpc_net.py

DrahtBot · 2020-07-18T21:23:50Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

net: Replace enum CConnMan::NumConnections with enum class ConnectionDirection #19771 (net: Replace enum CConnMan::NumConnections with enum class ConnectionDirection by luke-jr)
Allow whitelisting outgoing connections #17167 (Allow whitelisting outgoing connections by luke-jr)

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 · 2020-07-23T17:01:03Z

Current status:

1 Concept ACK
2 "No objections"
2 Tested ACKs

Is anything else needed? The changes are simple and have test coverage.

Github-Pull: bitcoin#19405 Rebased-From: 1c667b4080c6d18ea65c52c252d8d9a551ecd5ae

Github-Pull: bitcoin#19405 Rebased-From: 94a792cc19f26754ac5d60a1b733d4b520f7d0c7

darosior

Concept ACK

jonatack · 2020-08-20T09:35:32Z

Rebased and re-opened per IRC discussion yesterday with shesek at http://www.erisian.com.au/bitcoin-core-dev/log-2020-08-19.html#l-375. Proposing with both getnetworkinfo and -getinfo, open to adjust per review feedback.

jonatack · 2020-08-24T16:44:20Z

Rebased. Testing and review welcome! 🥰

eriknylund · 2020-08-24T19:24:36Z

ACK 581b343

Repeated steps from #19405 (comment) on the re-opened and rebased PR. Tests and extended functional tests pass.

Tested bitcoin-cli -getinfo, bitcoin-cli getnetworkinfo and bitcoin-cli help getnetworkinfo and the output is unchanged from the previous test. Also tested the UNIX epoch time fixup in bitcoin-cli help getnettotals this looks alright as well.

eriknylund · 2020-08-27T05:57:57Z

@shesek have you had a chance to test and review to see if this works for your purposes?

benthecarman · 2020-08-27T06:33:01Z

tACK 581b343

willcl-ark · 2020-08-28T11:05:07Z

tACK for 581b343, this time rebased onto master at 862fde8.

shesek · 2020-08-28T14:42:11Z

tACK 581b343. This provides what I needed, thanks!

eriknylund · 2020-08-28T14:47:49Z

tACK 581b343. This provides what I needed, thanks!

Thanks @shesek!

n-thumann · 2020-09-04T10:35:33Z

tACK 581b343 on master at a0a422c, ran unit & functional tests and and confirmed changes on an existing datadir ✌️

eriknylund · 2020-09-04T10:38:46Z

tACK 581b343 on master at a0a422c, ran unit & functional tests and and confirmed changes on an existing datadir ✌️

Thanks for reviewing @n-thumann and happy to hear that you tested on an existing datadir, which covers the other side of my test! 👍

…networkinfo` and `-getinfo` 581b343 Add in/out connections to cli -getinfo (Jon Atack) d9cc13e UNIX_EPOCH_TIME fixup in rpc getnettotals (Jon Atack) 1ab49b8 Add in/out connections to rpc getnetworkinfo (Jon Atack) Pull request description: This is basic info that is present in the GUI that I've been wishing to have exposed via the RPC and CLI without needing a bash workaround or script. For human users it would also be useful to have it in `-getinfo`. `bitcoin-cli getnetworkinfo` ``` "connections": 15, "connections_in": 6, "connections_out": 9, ``` `bitcoin-cli -getinfo` ``` "connections": { "in": 6, "out": 9, "total": 15 }, ``` Update the tests, RPC help, and release notes for the changes. Also fixup the `getnettotals` timemillis help while touching `rpc/net.cpp`. ----- Reviewers can manually test this PR by [building from source](https://jonatack.github.io/articles/how-to-compile-bitcoin-core-and-run-the-tests), launching bitcoind, and then running `bitcoin-cli -getinfo`, `bitcoin-cli getnetworkinfo`, `bitcoin-cli help getnetworkinfo`, and `bitcoin-cli help getnettotals` (for the UNIX epoch time change). ACKs for top commit: eriknylund: > tACK [581b343](bitcoin@581b343) on master at [a0a422c](bitcoin@a0a422c), ran unit & functional tests and and confirmed changes on an existing datadir ✌️ benthecarman: tACK `581b343` willcl-ark: tACK for 581b343, this time rebased onto master at 862fde8. shesek: tACK `581b343`. This provides what I needed, thanks! n-thumann: tACK 581b343 on master at a0a422c, ran unit & functional tests and and confirmed changes on an existing datadir ✌️ Tree-SHA512: 08dd3ac8fefae401bd8253ff3ac027603c528eeccba53cedcb127771316173a7052fce44af8fa33ac98ebc4cf2a2b11cdefd949995d55e9b9a5942b876d00dc5

bf1f913 cli -netinfo: display multiple levels of details (Jon Atack) 077b3ac cli: change -netinfo optional arg from bool to int (Jon Atack) 4e2f2dd cli: add getpeerinfo last_{block,transaction} to -netinfo (Jon Atack) 644be65 cli: add -netinfo server version check and error message (Jon Atack) ce57bf6 cli: create peer connections report sorted by dir, minping (Jon Atack) f5edd66 cli: create vector of Peer structs for peers data (Jon Atack) 3a0ab93 cli: add NetType enum struct and NetTypeEnumToString() (Jon Atack) c227100 cli: create local addresses, ports, and scores report (Jon Atack) d3f77b7 cli: create inbound/outbound peer connections report (Jon Atack) 19377b2 cli: start dashboard report with chain and version header (Jon Atack) a3653c1 cli: tally peer connections by type (Jon Atack) 54799b6 cli: add ipv6 and onion address type detection helpers (Jon Atack) 12242b1 cli: create initial -netinfo option, NetinfoRequestHandler class (Jon Atack) Pull request description: This PR is inspired by laanwj's python script mentioned in #19405, which it turns out I ended up using every day and extending because I got hooked on using it to monitor Bitcoin peer connections. For the full experience, run `./src/bitcoin-cli -netinfo 4` On Linux, try it with watch `watch ./src/bitcoin-cli -netinfo 4` Help doc ``` $ ./src/bitcoin-cli -help | grep -A3 netinfo -netinfo Get network peer connection information from the remote server. An optional integer argument from 0 to 4 can be passed for different peers listings (default: 0). ``` ACKs for top commit: vasild: ACK bf1f913 0xB10C: ACK bf1f913 practicalswift: ACK bf1f913 -- patch looks correct and is limited to `src/bitcoin-cli.cpp` Tree-SHA512: b9d18e5cc2ffd2bb9f0295b5ac7609da8a9bbecaf823a26dfa706b5f07d5d1a8343081dad98b16aa9dc8efd8f41bc1a4acdc40259727de622dc7195ccf59c572

bf1f913 cli -netinfo: display multiple levels of details (Jon Atack) 077b3ac cli: change -netinfo optional arg from bool to int (Jon Atack) 4e2f2dd cli: add getpeerinfo last_{block,transaction} to -netinfo (Jon Atack) 644be65 cli: add -netinfo server version check and error message (Jon Atack) ce57bf6 cli: create peer connections report sorted by dir, minping (Jon Atack) f5edd66 cli: create vector of Peer structs for peers data (Jon Atack) 3a0ab93 cli: add NetType enum struct and NetTypeEnumToString() (Jon Atack) c227100 cli: create local addresses, ports, and scores report (Jon Atack) d3f77b7 cli: create inbound/outbound peer connections report (Jon Atack) 19377b2 cli: start dashboard report with chain and version header (Jon Atack) a3653c1 cli: tally peer connections by type (Jon Atack) 54799b6 cli: add ipv6 and onion address type detection helpers (Jon Atack) 12242b1 cli: create initial -netinfo option, NetinfoRequestHandler class (Jon Atack) Pull request description: This PR is inspired by laanwj's python script mentioned in bitcoin#19405, which it turns out I ended up using every day and extending because I got hooked on using it to monitor Bitcoin peer connections. For the full experience, run `./src/bitcoin-cli -netinfo 4` On Linux, try it with watch `watch ./src/bitcoin-cli -netinfo 4` Help doc ``` $ ./src/bitcoin-cli -help | grep -A3 netinfo -netinfo Get network peer connection information from the remote server. An optional integer argument from 0 to 4 can be passed for different peers listings (default: 0). ``` ACKs for top commit: vasild: ACK bf1f913 0xB10C: ACK bf1f913 practicalswift: ACK bf1f913 -- patch looks correct and is limited to `src/bitcoin-cli.cpp` Tree-SHA512: b9d18e5cc2ffd2bb9f0295b5ac7609da8a9bbecaf823a26dfa706b5f07d5d1a8343081dad98b16aa9dc8efd8f41bc1a4acdc40259727de622dc7195ccf59c572

Summary: > Add in/out connections to rpc getnetworkinfo > UNIX_EPOCH_TIME fixup in rpc getnettotals > Add in/out connections to cli -getinfo This is a backport of [[bitcoin/bitcoin#19405 | core#19405]] Test Plan: `ninja check-functional` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D10180

fanquake added the RPC/REST/ZMQ label Jun 29, 2020

jonatack commented Jun 29, 2020

View reviewed changes

src/rpc/net.cpp Outdated Show resolved Hide resolved

jonatack force-pushed the in-and-out-connections branch from e7c7809 to 9290a98 Compare June 29, 2020 14:53

maflcko reviewed Jun 29, 2020

View reviewed changes

src/rpc/net.cpp Outdated Show resolved Hide resolved

src/rpc/net.cpp Outdated Show resolved Hide resolved

jonatack force-pushed the in-and-out-connections branch from 9290a98 to 94a792c Compare June 29, 2020 18:25

jonatack mentioned this pull request Jul 1, 2020

[net] Cleanup logic around connection types #19316

Merged

eriknylund reviewed Jul 15, 2020

View reviewed changes

test/functional/rpc_net.py Outdated Show resolved Hide resolved

eriknylund approved these changes Jul 15, 2020

View reviewed changes

DrahtBot mentioned this pull request Jul 18, 2020

net: Add -networkactive option #19473

Merged

DrahtBot added the Needs rebase label Jul 23, 2020

jonatack closed this Jul 23, 2020

jonatack mentioned this pull request Aug 2, 2020

Add -netinfo peer connections dashboard #19643

Merged

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Aug 15, 2020

Add in/out connections to rpc getnetworkinfo

2709f14

Github-Pull: bitcoin#19405 Rebased-From: 1c667b4080c6d18ea65c52c252d8d9a551ecd5ae

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Aug 15, 2020

Add in/out connections to cli -getinfo

f97fe2b

Github-Pull: bitcoin#19405 Rebased-From: 94a792cc19f26754ac5d60a1b733d4b520f7d0c7

jonatack reopened this Aug 19, 2020

darosior reviewed Aug 20, 2020

View reviewed changes

jonatack force-pushed the in-and-out-connections branch from 94a792c to d96c13f Compare August 20, 2020 09:31

DrahtBot removed the Needs rebase label Aug 20, 2020

DrahtBot mentioned this pull request Aug 20, 2020

net, rpc: expose nLastBlockTime/nLastTXTime as last block/last_transaction in getpeerinfo #19731

Merged

This was referenced Aug 21, 2020

Allow whitelisting outgoing connections #17167

Closed

net: Replace enum CConnMan::NumConnections with enum class ConnectionDirection #19771

Merged

DrahtBot added the Needs rebase label Aug 24, 2020

jonatack added 3 commits August 24, 2020 18:41

Add in/out connections to rpc getnetworkinfo

1ab49b8

UNIX_EPOCH_TIME fixup in rpc getnettotals

d9cc13e

Add in/out connections to cli -getinfo

581b343

jonatack force-pushed the in-and-out-connections branch from d96c13f to 581b343 Compare August 24, 2020 16:42

DrahtBot removed the Needs rebase label Aug 24, 2020

n-thumann approved these changes Sep 4, 2020

View reviewed changes

laanwj merged commit 23d3ae7 into bitcoin:master Sep 4, 2020

jonatack deleted the in-and-out-connections branch September 4, 2020 13:16

jonatack mentioned this pull request Sep 4, 2020

cli: degrade -getinfo gracefully for older servers #19874

Closed

dekokun mentioned this pull request Jan 31, 2022

btcjson: Update fields in GetNetworkInfoResult btcsuite/btcd#1800

Merged

bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022

Conversation

jonatack commented Jun 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

laanwj commented Jun 29, 2020

Uh oh!

practicalswift commented Jun 29, 2020

Uh oh!

jonatack commented Jun 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jonatack commented Jun 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonatack commented Jun 29, 2020

Uh oh!

jnewbery commented Jul 1, 2020

Uh oh!

willcl-ark commented Jul 2, 2020

Uh oh!

eriknylund commented Jul 15, 2020

Uh oh!

Uh oh!

DrahtBot commented Jul 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

jonatack commented Jul 23, 2020

Uh oh!

darosior left a comment

Choose a reason for hiding this comment

Uh oh!

jonatack commented Aug 20, 2020

Uh oh!

jonatack commented Aug 24, 2020

Uh oh!

eriknylund commented Aug 24, 2020

Uh oh!

eriknylund commented Aug 27, 2020

Uh oh!

benthecarman commented Aug 27, 2020

Uh oh!

willcl-ark commented Aug 28, 2020

Uh oh!

shesek commented Aug 28, 2020

Uh oh!

eriknylund commented Aug 28, 2020

Uh oh!

n-thumann commented Sep 4, 2020

Uh oh!

eriknylund commented Sep 4, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

jonatack commented Jun 29, 2020 •

edited

Loading

jonatack commented Jun 29, 2020 •

edited

Loading

jonatack commented Jun 29, 2020 •

edited

Loading

DrahtBot commented Jul 18, 2020 •

edited

Loading