net, rpc, cli: expose peer network in getpeerinfo; simplify/improve -netinfo #20002

pull jonatack wants to merge 5 commits into bitcoin:master from jonatack:getpeerinfo-GetNetClass changing 5 files +97 −106
  1. jonatack commented at 11:47 AM on September 23, 2020: contributor

    This PR:

    • builds on #19991 and #19998
    • exposes peer networks via a new getpeerinfo network field ("ipv4", "ipv6", or "onion"), and adds functional tests
    • updates -netinfo to use getpeerinfo network rather than detecting the peer networks client-side
    • refactors -netinfo to easily add future networks
  2. practicalswift commented at 11:50 AM on September 23, 2020: contributor

    Concept ACK

    Thanks for improving -netinfo! :)

  3. jonatack force-pushed on Sep 23, 2020
  4. DrahtBot added the label P2P on Sep 23, 2020
  5. DrahtBot added the label RPC/REST/ZMQ on Sep 23, 2020
  6. jonatack cross-referenced this on Sep 23, 2020 from issue net: Add CNode::ConnectedThroughNetwork member function by hebasto
  7. jonatack force-pushed on Sep 23, 2020
  8. DrahtBot commented at 4:22 PM on September 23, 2020: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #20120 (net, rpc, test, bugfix: update GetNetworkName, GetNetworksInfo, regression tests by jonatack)
    • #20115 (cli: -netinfo quick updates/fixups and release note 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.

  9. jonatack force-pushed on Sep 23, 2020
  10. DrahtBot cross-referenced this on Sep 23, 2020 from issue net, rpc: expose connection type in getpeerinfo by jonatack
  11. DrahtBot cross-referenced this on Sep 23, 2020 from issue net processing: Move block inventory state to net_processing by jnewbery
  12. DrahtBot cross-referenced this on Sep 23, 2020 from issue net, rpc: expose high bandwidth mode state via getpeerinfo by theStack
  13. jonatack force-pushed on Sep 23, 2020
  14. jonatack force-pushed on Sep 24, 2020
  15. jonatack renamed this:
    net, rpc, cli: expose CNetAddr::GetNetClass in getpeerinfo, use in -netinfo
    net, rpc, cli: expose GetNetClass()/ConnectedViaTor() in getpeerinfo, use in -netinfo
    on Sep 24, 2020
  16. jonatack cross-referenced this on Sep 24, 2020 from issue net: Use alternative port for incoming Tor connections by hebasto
  17. hebasto commented at 2:53 PM on September 24, 2020: member

    Concept ACK.

  18. DrahtBot cross-referenced this on Sep 24, 2020 from issue net: guard vRecvGetData with cs_vRecv and orphan_work_set with g_cs_orphans by narula
  19. DrahtBot cross-referenced this on Sep 24, 2020 from issue [RPC] Add connection type to getpeerinfo, improve logs by amitiuttarwar
  20. DrahtBot cross-referenced this on Sep 24, 2020 from issue p2p: Move all header verification into the network layer, extend logging by troygiorshev
  21. DrahtBot cross-referenced this on Sep 24, 2020 from issue refactor: Cleanup thread ctor calls by hebasto
  22. DrahtBot cross-referenced this on Sep 24, 2020 from issue torcontrol: add -tortarget config by MDrollette
  23. DrahtBot cross-referenced this on Sep 26, 2020 from issue refactor: Add ALLOW_LIST flags and enforce usage in CheckArgFlags by ryanofsky
  24. DrahtBot added the label Needs rebase on Sep 26, 2020
  25. jonatack cross-referenced this on Sep 26, 2020 from issue Add -netinfo peer connections dashboard by jonatack
  26. jonatack force-pushed on Oct 2, 2020
  27. jonatack force-pushed on Oct 2, 2020
  28. DrahtBot removed the label Needs rebase on Oct 2, 2020
  29. jonatack renamed this:
    net, rpc, cli: expose GetNetClass()/ConnectedViaTor() in getpeerinfo, use in -netinfo
    net, rpc, cli: expose peer network in getpeerinfo; simplify/improve -netinfo
    on Oct 2, 2020
  30. DrahtBot cross-referenced this on Oct 3, 2020 from issue net: Add NAT-PMP port forwarding support by hebasto
  31. hebasto commented at 7:48 AM on October 3, 2020: member

    Tested 5604a61435c5dfdeed41db5eca489b353dee0b99 on Linux Mint 20 (x86_64):

    $ src/bitcoin-cli -netinfo 4
Bitcoin Core v0.20.99.0-5604a6143 - 70016/Satoshi:0.20.99/

Peer connections sorted by direction and min ping
<-> relay   net mping   ping send recv  txn  blk uptime  id address                     version
 in  full onion   178   1081   11   76              229 171 127.0.0.1:37328             70015/bitnodes.io:0.1/
 in block onion   241    558   29   29              265 157 127.0.0.1:34910             70015/Satoshi:0.20.1/
 in block onion   245    399  110  110              289 144 127.0.0.1:33322             70015/Satoshi:0.20.1/
 in  full onion   359    430   10    1    1   37     44 268 127.0.0.1:49482             70015/Satoshi:0.19.1/
out  full onion   251    450    1    3    0         468  78 jqsx6ag4hujpapxq.onion:8333 70015/Satoshi:0.20.0/
out  full onion   261    352    1    1    0   12    625   6 tuwj7ju4s25345pg.onion:8333 70015/Satoshi:0.19.0.1/
out block onion   280    528   58   58       624    625   9 kzrnyo7fadxihvgb.onion:8333 70015/Satoshi:0.20.0/
out  full onion   329    474    2    5    0  623    625   7 2mmxouhv6nebowkq.onion:8333 70015/Satoshi:0.18.1/
out  full onion   372    622    3    3    1  623    625   4 q467hmvzy5kfvoed.onion:8333 70015/Satoshi:0.20.1/
out  full onion   380    546    2   11    0  624    625   8 ee5skvb3ktbposl3.onion:8333 70015/Satoshi:0.20.0/
out  full onion   392    623    0    6    0  624    626   2 6xiqqr4fxnstd5fo.onion:8333 70015/Satoshi:0.20.1/
out  full onion   421    556    2    7    0  267    625   5 ohrieqovuxe6y4gl.onion:8333 70015/Satoshi:0.20.0/
out block onion   465    578  112  111              246 162 a53vtdm7uiet5vdl.onion:8333 70015/Satoshi:0.19.1/
out  full onion  1717   1717    5    0    0           1 290 mbciulcc3wuksb3f.onion:8333 70015/Satoshi:0.20.0/
                   ms     ms  sec  sec  min  min    min

        ipv4    ipv6   onion   total  block-relay
in         0       0       4       4       2
out        0       0      10      10       2
total      0       0      14      14       4
...
  32. in src/rpc/net.cpp:97 in bebd062eef outdated 
      93 | @@ -94,6 +94,7 @@ static RPCHelpMan getpeerinfo()
  94 |                              {RPCResult::Type::STR, "addr", "(host:port) The IP address and port of the peer"},
  95 |                              {RPCResult::Type::STR, "addrbind", "(ip:port) Bind address of the connection to the peer"},
  96 |                              {RPCResult::Type::STR, "addrlocal", "(ip:port) Local address as reported by the peer"},
  97 | +                            {RPCResult::Type::STR, "network", "Network (ipv4, ipv6, or onion) the peer connected through"},
    hebasto commented at 8:08 AM on October 3, 2020:

    bebd062eef3fa2d33c9216c368977dbedb70a443

    While here, mind making "addrbind" and "addrlocal" ordering consistent for help and output?

    jonatack commented at 7:16 PM on October 11, 2020:

    Good idea. Done.

  33. in test/functional/feature_proxy.py:165 in bebd062eef outdated 
     161 | @@ -143,6 +162,7 @@ def node_test(self, node, proxies, auth, test_onion=True):
 162 |              assert_equal(cmd.username, None)
 163 |              assert_equal(cmd.password, None)
 164 |          rv.append(cmd)
 165 | +        self.network_test(node, addr, network=NET_UNROUTABLE)
    hebasto commented at 8:31 AM on October 3, 2020:

    bebd062eef3fa2d33c9216c368977dbedb70a443

    Testing of network=NET_UNROUTABLE is actually skipped. Could we avoid such skipping?

    jonatack commented at 7:23 PM on October 11, 2020:

    When I run feature_proxy.py with -l debug, or with the following patch, it doesn't seem to be skipped in my testing. LMK if I'm misunderstanding.

    +++ b/test/functional/feature_proxy.py
@@ -101,6 +101,7 @@ class ProxyTest(BitcoinTestFramework):
         for peer in node.getpeerinfo():
             if peer["addr"] == addr:
                 assert_equal(peer["network"], network)
+                import pprint; pprint.pprint(network)
  34. in src/bitcoin-cli.cpp:299 in 5604a61435 outdated 
     310 | -        const size_t onion_pos{addr.rfind(ONION)};
 311 | -        return mapped_as == 0 && onion_pos != std::string::npos && addr_len > ONION_LEN &&
 312 | -               (onion_pos == addr_len - ONION_LEN || onion_pos == addr.find_last_of(":") - ONION_LEN);
 313 | -    }
 314 | -    uint8_t m_details_level{0}; //!< Optional user-supplied arg to set dashboard details level
 315 | +    static const int m_networks = 3; // Number of possible networks
    hebasto commented at 8:39 AM on October 3, 2020:

    5604a61435c5dfdeed41db5eca489b353dee0b99

        static constexpr int m_networks = 3; //!< Number of possible networks.
    jonatack commented at 7:17 PM on October 11, 2020:

    Done

  35. in src/bitcoin-cli.cpp:300 in 5604a61435 outdated 
     311 | -        return mapped_as == 0 && onion_pos != std::string::npos && addr_len > ONION_LEN &&
 312 | -               (onion_pos == addr_len - ONION_LEN || onion_pos == addr.find_last_of(":") - ONION_LEN);
 313 | -    }
 314 | -    uint8_t m_details_level{0}; //!< Optional user-supplied arg to set dashboard details level
 315 | +    static const int m_networks = 3; // Number of possible networks
 316 | +    uint8_t m_details_level = 0;     //!< Optional user-supplied arg to set dashboard details level
    hebasto commented at 8:40 AM on October 3, 2020:

    Why switch from modern {} initialization to the old style?

    jonatack commented at 7:18 PM on October 11, 2020:

    Done, retained modern style. Thanks.

  36. hebasto approved
  37. hebasto commented at 8:42 AM on October 3, 2020: member

    ACK 5604a61435c5dfdeed41db5eca489b353dee0b99

  38. jonatack cross-referenced this on Oct 9, 2020 from issue cli: -netinfo quick updates/fixups for 0.21 by jonatack
  39. jonatack force-pushed on Oct 11, 2020
  40. jonatack commented at 7:25 PM on October 11, 2020: contributor

    Thanks for reviewing @hebasto. I've addressed your feedback.

  41. DrahtBot cross-referenced this on Oct 11, 2020 from issue net, rpc, test, bugfix: update GetNetworkName, GetNetworksInfo, regression tests by jonatack
  42. laanwj referenced this in commit f79a4a8952 on Oct 12, 2020
  43. laanwj commented at 4:42 PM on October 12, 2020: member

    Needs rebase after #19998.

  44. DrahtBot added the label Needs rebase on Oct 12, 2020
  45. jonatack force-pushed on Oct 12, 2020
  46. DrahtBot removed the label Needs rebase on Oct 12, 2020
  47. jonatack commented at 10:12 AM on October 13, 2020: contributor

    Rebased. If merged for 0.21 will add a release note manually in the wiki.

  48. sidhujag referenced this in commit 2180fb4d7a on Oct 13, 2020
  49. jonatack force-pushed on Oct 14, 2020
  50. in src/bitcoin-cli.cpp:306 in 6c08a4b6e7 outdated 
     314 | -        return mapped_as == 0 && onion_pos != std::string::npos && addr_len > ONION_LEN &&
 315 | -               (onion_pos == addr_len - ONION_LEN || onion_pos == addr.find_last_of(":") - ONION_LEN);
 316 | +        for (const auto& network : m_networks) {
 317 | +            if (str == network.first) return network.second;
 318 | +        }
 319 | +        assert(false);
    laanwj commented at 11:27 AM on October 14, 2020:

    Let's not do assert(false) here. A crash on "invalid" input from the RPC is never good, it is good to be slightly forgiving with regard to forward-compatibility. Maybe return an 'unknown' category instead that is ignored with regard to the statistics/table.

    jonatack commented at 1:29 PM on October 14, 2020:

    Done!

  51. in src/interfaces/node.h:26 in 548f8c8e92 outdated 
      22 | @@ -23,7 +23,7 @@
  23 |  class BanMan;
  24 |  class CCoinControl;
  25 |  class CFeeRate;
  26 | -class CNodeStats;
  27 | +struct CNodeStats;
    laanwj commented at 11:31 AM on October 14, 2020:

    ~0 on the change from a class to a struct, I think calling it a struct is more consistent with how it's used but it doesn't seem to be a logical part of this change

    jonatack commented at 1:29 PM on October 14, 2020:

    Removed the struct change.

  52. laanwj commented at 11:37 AM on October 14, 2020: member

    Rebased. If merged for 0.21 will add a release note manually in the wiki.

    +1 in getting this into 0.21, it's a low risk change and is kind of the culmination of work that's been going on for the entire release cycle

  53. laanwj added this to the milestone 0.21.0 on Oct 14, 2020
  54. hebasto commented at 11:39 AM on October 14, 2020: member

    Tested 6c08a4b6e755efa40f021ff4e7ec0d69379cc773 on Linux Mint 20 (x86_64). LGTM.

  55. net: add peer network to CNodeStats 6df7882029
  56. rpc, test: expose CNodeStats network in RPC getpeerinfo 4938a109ad
  57. cli: simplify -netinfo using getpeerinfo network field 5133fab37e
  58. refactor: promote some -netinfo localvars to class members 82fd40216c
  59. in src/bitcoin-cli.cpp:299 in 6c08a4b6e7 outdated 
     295 | @@ -298,30 +296,22 @@ class GetinfoRequestHandler: public BaseRequestHandler
 296 |  class NetinfoRequestHandler : public BaseRequestHandler
 297 |  {
 298 |  private:
 299 | -    bool IsAddrIPv6(const std::string& addr) const
 300 | +    const std::vector<std::pair<std::string, uint8_t>> m_networks{{"ipv4", 0}, {"ipv6", 1}, {"onion", 2}};
    laanwj commented at 11:47 AM on October 14, 2020:

    As these IDs are always consecutively, and you iterate over it linearly, and you make the assumption that maxid is size()-1. why not simply

    const std::vector<std::string> m_networks{"ipv4","ipv6","onion"};

    alternatively, lookup would be easier with a std::map.

    jonatack commented at 1:32 PM on October 14, 2020:

    Good idea to flatten it--done, and changed the new data structures in the last commit to fixed-sized std::arrays.

  60. jonatack force-pushed on Oct 14, 2020
  61. jonatack commented at 1:38 PM on October 14, 2020: contributor

    Thanks for the feedback! Updated. (Edit: repushed to wrap array initialization in extra braces for one compiler on travis that needed appeasing.)

  62. jonatack force-pushed on Oct 14, 2020
  63. refactor: enable -netinfo to add future networks (i2p, cjdns)
    After this commit, a new network may be added by changing 4 lines:
- increment the value of `m_networks_size`
- add the network name to `m_networks`
- add the network name to this line: `result += "        ipv4    ipv6   onion   total  block-relay\n";`
- add "counts.at(i).at(<m_networks pos>)" to this line: `result += strprintf("%-5s  %5i   %5i   %5i   %5i   %5i\n...`
    6272604bef
  64. jonatack force-pushed on Oct 14, 2020
  65. laanwj commented at 3:43 PM on October 15, 2020: member

    ACK 6272604bef3b409455b010d134b4b62c8f6ff49f

  66. laanwj merged this on Oct 15, 2020
  67. laanwj closed this on Oct 15, 2020
  68. jonatack deleted the branch on Oct 15, 2020
  69. sidhujag referenced this in commit 37b55282d6 on Oct 16, 2020
  70. jonatack commented at 5:14 PM on November 2, 2020: contributor

    Rebased. If merged for 0.21 will add a release note manually in the wiki.

    Done, manually added a release note in the wiki for getpeerinfo#network.

  71. Fabcien referenced this in commit 58dc89eaaa on Nov 24, 2021
  72. Fabcien referenced this in commit 0e151b815a on Nov 24, 2021
  73. Fabcien referenced this in commit 937e21bca4 on Nov 24, 2021
  74. Fabcien referenced this in commit 47738f027e on Nov 24, 2021
  75. Fabcien referenced this in commit 46866bc04b on Nov 24, 2021
  76. bitcoin locked this on Feb 15, 2022
Contributors
jonatackpracticalswiftDrahtBothebastolaanwj
Labels
RPC/REST/ZMQP2P

Milestone
0.21.0

Linked (view graph)
#19643 Add -netinfo peer connections dashboard#19991 net: Use alternative port for incoming Tor connections#19998 net: Add CNode::ConnectedThroughNetwork member function#20115 cli: -netinfo quick updates/fixups for 0.21
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:54 UTC