Merge #20877: netinfo: user help and argument parsing improvements

7d3343fb8e775527e6afc2b183a62ad76e62347e cli: update -netinfo help doc following the merge of 882ce251 (Jon Atack)
ef614bb408f0547d79bfe9b39278b197e040c683 cli: small -netinfo simplification and performance improvement (Jon Atack)
6b45ef32331dd3efb863d1d2f7405e2665565905 cli: improve -netinfo invalid argument error message (Jon Atack)
3732404afaa15f7a86846327e48cdb9a2027f80c cli: warn in help that -netinfo is not intended to be a stable API (Jon Atack)
7afdd7225842dcff2708dedb8131bb082673f228 cli: enable -netinfo help to run without a remote server (Jon Atack)

Pull request description:

  A few updates, some per IRC discussion today at http://www.erisian.com.au/bitcoin-core-dev/log-2021-01-07.html#l-87 with respect to -netinfo:

  - enable `-netinfo help` to run without a remote server
  - warn in `-netinfo help` that -netinfo is not intended to be a stable API
  - improve the -netinfo invalid argument error message
  - make a performance improvement and simplification I noticed after the merge of #20764
  - update the -netinfo help doc following the merge of #21192
  -----

  How to test manually:  :microscope: :test_tube:  :chart_with_upwards_trend:

  1. check out and build this branch locally; if you need help, don't hesitate to refer to https://jonatack.github.io/articles/how-to-review-pull-requests-in-bitcoin-core#pull-down-the-code-locally or https://jonatack.github.io/articles/how-to-compile-bitcoin-core-and-run-the-tests
  2. while it is compiling, look at the code changes
  3. stop signet (if it is running) with `./src/bitcoin-cli -signet stop`
  4. once the build is completed, run `./src/bitcoin-cli -signet -netinfo help`
  5. the help should be printed even though the signet server is not running
  6. near the top you should see the new warning, "This human-readable interface will change regularly and is not intended to be a stable API" as well as a bit more description about the integer argument values.
  7. start signet with `./src/bitcoind -signet`
  8. test the improved invalid argument error message if you run `./src/bitcoin-cli -signet -netinfo 256` or `./src/bitcoin-cli -signet -netinfo a` (valid values are from 0 to 255)
  9. leave review feedback or `ACK <commit hash>` -- done :beers:

ACKs for top commit:
  michaelfolkson:
    Re-ACK 7d3343fb8e775527e6afc2b183a62ad76e62347e
  pinheadmz:
    RE-ACK 7d3343fb8e775527e6afc2b183a62ad76e62347e

Tree-SHA512: 28c5e9f295ffccba5c2a70faac4987d45f35d4758cf8f10daa767e83212316c4cfc65930e4066f7ad627e9d15b92d43439d1ba9c2f755dfde61885c6a70aa155

1 files changed, 70 insertions, 75 deletions

diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
index 0830cb54cb..dc4b142f83 100644
--- a/src/bitcoin-cli.cpp
+++ b/src/bitcoin-cli.cpp
@@ -300,7 +300,6 @@ class NetinfoRequestHandler : public BaseRequestHandler
 {
 private:
     static constexpr int8_t UNKNOWN_NETWORK{-1};
-    static constexpr int8_t NET_I2P{3}; // pos of "i2p" in m_networks
     static constexpr uint8_t m_networks_size{4};
     static constexpr uint8_t MAX_DETAIL_LEVEL{4};
     const std::array<std::string, m_networks_size> m_networks{{"ipv4", "ipv6", "onion", "i2p"}};
@@ -314,13 +313,11 @@ private:
         }
         return UNKNOWN_NETWORK;
     }
-    uint8_t m_details_level{0};      //!< Optional user-supplied arg to set dashboard details level
-    bool m_is_help_requested{false}; //!< Optional user-supplied arg to print help documentation
+    uint8_t m_details_level{0}; //!< Optional user-supplied arg to set dashboard details level
     bool DetailsRequested() const { return m_details_level > 0 && m_details_level < 5; }
     bool IsAddressSelected() const { return m_details_level == 2 || m_details_level == 4; }
     bool IsVersionSelected() const { return m_details_level == 3 || m_details_level == 4; }
     bool m_is_asmap_on{false};
-    bool m_is_i2p_on{false};
     size_t m_max_addr_length{0};
     size_t m_max_age_length{3};
     size_t m_max_id_length{2};
@@ -367,68 +364,6 @@ private:
         if (conn_type == "addr-fetch") return "addr";
         return "";
     }
-    const UniValue NetinfoHelp()
-    {
-        return std::string{
-            "-netinfo level|\"help\" \n\n"
-            "Returns a network peer connections dashboard with information from the remote server.\n"
-            "Under the hood, -netinfo fetches the data by calling getpeerinfo and getnetworkinfo.\n"
-            "An optional integer argument from 0 to 4 can be passed for different peers listings.\n"
-            "Pass \"help\" to see this detailed help documentation.\n"
-            "If more than one argument is passed, only the first one is read and parsed.\n"
-            "Suggestion: use with the Linux watch(1) command for a live dashboard; see example below.\n\n"
-            "Arguments:\n"
-            "1. level (integer 0-4, optional)  Specify the info level of the peers dashboard (default 0):\n"
-            "                                  0 - Connection counts and local addresses\n"
-            "                                  1 - Like 0 but with a peers listing (without address or version columns)\n"
-            "                                  2 - Like 1 but with an address column\n"
-            "                                  3 - Like 1 but with a version column\n"
-            "                                  4 - Like 1 but with both address and version columns\n"
-            "2. help (string \"help\", optional) Print this help documentation instead of the dashboard.\n\n"
-            "Result:\n\n"
-            "* The peers listing in levels 1-4 displays all of the peers sorted by direction and minimum ping time:\n\n"
-            "  Column   Description\n"
-            "  ------   -----------\n"
-            "  <->      Direction\n"
-            "           \"in\"  - inbound connections are those initiated by the peer\n"
-            "           \"out\" - outbound connections are those initiated by us\n"
-            "  type     Type of peer connection\n"
-            "           \"full\"   - full relay, the default\n"
-            "           \"block\"  - block relay; like full relay but does not relay transactions or addresses\n"
-            "           \"manual\" - peer we manually added using RPC addnode or the -addnode/-connect config options\n"
-            "           \"feeler\" - short-lived connection for testing addresses\n"
-            "           \"addr\"   - address fetch; short-lived connection for requesting addresses\n"
-            "  net      Network the peer connected through (\"ipv4\", \"ipv6\", \"onion\", \"i2p\", or \"cjdns\")\n"
-            "  mping    Minimum observed ping time, in milliseconds (ms)\n"
-            "  ping     Last observed ping time, in milliseconds (ms)\n"
-            "  send     Time since last message sent to the peer, in seconds\n"
-            "  recv     Time since last message received from the peer, in seconds\n"
-            "  txn      Time since last novel transaction received from the peer and accepted into our mempool, in minutes\n"
-            "  blk      Time since last novel block passing initial validity checks received from the peer, in minutes\n"
-            "  hb       High-bandwidth BIP152 compact block relay\n"
-            "           \".\" (to)   - we selected the peer as a high-bandwidth peer\n"
-            "           \"*\" (from) - the peer selected us as a high-bandwidth peer\n"
-            "  age      Duration of connection to the peer, in minutes\n"
-            "  asmap    Mapped AS (Autonomous System) number in the BGP route to the peer, used for diversifying\n"
-            "           peer selection (only displayed if the -asmap config option is set)\n"
-            "  id       Peer index, in increasing order of peer connections since node startup\n"
-            "  address  IP address and port of the peer\n"
-            "  version  Peer version and subversion concatenated, e.g. \"70016/Satoshi:21.0.0/\"\n\n"
-            "* The connection counts table displays the number of peers by direction, network, and the totals\n"
-            "  for each, as well as two special outbound columns for block relay peers and manual peers.\n\n"
-            "* The local addresses table lists each local address broadcast by the node, the port, and the score.\n\n"
-            "Examples:\n\n"
-            "Connection counts and local addresses only\n"
-            "> bitcoin-cli -netinfo\n\n"
-            "Compact peers listing\n"
-            "> bitcoin-cli -netinfo 1\n\n"
-            "Full dashboard\n"
-            "> bitcoin-cli -netinfo 4\n\n"
-            "Full live dashboard, adjust --interval or --no-title as needed (Linux)\n"
-            "> watch --interval 1 --no-title bitcoin-cli -netinfo 4\n\n"
-            "See this help\n"
-            "> bitcoin-cli -netinfo help\n"};
-    }
     const int64_t m_time_now{GetSystemTimeInSeconds()};
 
 public:
@@ -441,10 +376,8 @@ public:
             uint8_t n{0};
             if (ParseUInt8(args.at(0), &n)) {
                 m_details_level = std::min(n, MAX_DETAIL_LEVEL);
-            } else if (args.at(0) == "help") {
-                m_is_help_requested = true;
             } else {
-                throw std::runtime_error(strprintf("invalid -netinfo argument: %s", args.at(0)));
+                throw std::runtime_error(strprintf("invalid -netinfo argument: %s\nFor more information, run: bitcoin-cli -netinfo help", args.at(0)));
             }
         }
         UniValue result(UniValue::VARR);
@@ -455,9 +388,6 @@ public:
 
     UniValue ProcessReply(const UniValue& batch_in) override
     {
-        if (m_is_help_requested) {
-            return JSONRPCReplyObj(NetinfoHelp(), NullUniValue, 1);
-        }
         const std::vector<UniValue> batch{JSONRPCProcessBatchReply(batch_in)};
         if (!batch[ID_PEERINFO]["error"].isNull()) return batch[ID_PEERINFO];
         if (!batch[ID_NETWORKINFO]["error"].isNull()) return batch[ID_NETWORKINFO];
@@ -472,7 +402,6 @@ public:
             const std::string network{peer["network"].get_str()};
             const int8_t network_id{NetworkStringToId(network)};
             if (network_id == UNKNOWN_NETWORK) continue;
-            m_is_i2p_on |= (network_id == NET_I2P);
             const bool is_outbound{!peer["inbound"].get_bool()};
             const bool is_block_relay{!peer["relaytxes"].get_bool()};
             const std::string conn_type{peer["connection_type"].get_str()};
@@ -545,13 +474,14 @@ public:
 
         // Report peer connection totals by type.
         result += "        ipv4    ipv6   onion";
-        if (m_is_i2p_on) result += "     i2p";
+        const bool any_i2p_peers = m_counts.at(2).at(3); // false if total i2p peers count is 0, otherwise true
+        if (any_i2p_peers) result += "     i2p";
         result += "   total   block";
         if (m_manual_peers_count) result += "  manual";
         const std::array<std::string, 3> rows{{"in", "out", "total"}};
         for (uint8_t i = 0; i < 3; ++i) {
             result += strprintf("\n%-5s  %5i   %5i   %5i", rows.at(i), m_counts.at(i).at(0), m_counts.at(i).at(1), m_counts.at(i).at(2)); // ipv4/ipv6/onion peers counts
-            if (m_is_i2p_on) result += strprintf("   %5i", m_counts.at(i).at(3)); // i2p peers count
+            if (any_i2p_peers) result += strprintf("   %5i", m_counts.at(i).at(3)); // i2p peers count
             result += strprintf("   %5i", m_counts.at(i).at(m_networks_size)); // total peers count
             if (i == 1) { // the outbound row has two extra columns for block relay and manual peer counts
                 result += strprintf("   %5i", m_block_relay_peers_count);
@@ -576,6 +506,67 @@ public:
 
         return JSONRPCReplyObj(UniValue{result}, NullUniValue, 1);
     }
+
+    const std::string m_help_doc{
+        "-netinfo level|\"help\" \n\n"
+        "Returns a network peer connections dashboard with information from the remote server.\n"
+        "This human-readable interface will change regularly and is not intended to be a stable API.\n"
+        "Under the hood, -netinfo fetches the data by calling getpeerinfo and getnetworkinfo.\n"
+        + strprintf("An optional integer argument from 0 to %d can be passed for different peers listings; %d to 255 are parsed as %d.\n", MAX_DETAIL_LEVEL, MAX_DETAIL_LEVEL, MAX_DETAIL_LEVEL) +
+        "Pass \"help\" to see this detailed help documentation.\n"
+        "If more than one argument is passed, only the first one is read and parsed.\n"
+        "Suggestion: use with the Linux watch(1) command for a live dashboard; see example below.\n\n"
+        "Arguments:\n"
+        + strprintf("1. level (integer 0-%d, optional)  Specify the info level of the peers dashboard (default 0):\n", MAX_DETAIL_LEVEL) +
+        "                                  0 - Connection counts and local addresses\n"
+        "                                  1 - Like 0 but with a peers listing (without address or version columns)\n"
+        "                                  2 - Like 1 but with an address column\n"
+        "                                  3 - Like 1 but with a version column\n"
+        "                                  4 - Like 1 but with both address and version columns\n"
+        "2. help (string \"help\", optional) Print this help documentation instead of the dashboard.\n\n"
+        "Result:\n\n"
+        + strprintf("* The peers listing in levels 1-%d displays all of the peers sorted by direction and minimum ping time:\n\n", MAX_DETAIL_LEVEL) +
+        "  Column   Description\n"
+        "  ------   -----------\n"
+        "  <->      Direction\n"
+        "           \"in\"  - inbound connections are those initiated by the peer\n"
+        "           \"out\" - outbound connections are those initiated by us\n"
+        "  type     Type of peer connection\n"
+        "           \"full\"   - full relay, the default\n"
+        "           \"block\"  - block relay; like full relay but does not relay transactions or addresses\n"
+        "           \"manual\" - peer we manually added using RPC addnode or the -addnode/-connect config options\n"
+        "           \"feeler\" - short-lived connection for testing addresses\n"
+        "           \"addr\"   - address fetch; short-lived connection for requesting addresses\n"
+        "  net      Network the peer connected through (\"ipv4\", \"ipv6\", \"onion\", \"i2p\", or \"cjdns\")\n"
+        "  mping    Minimum observed ping time, in milliseconds (ms)\n"
+        "  ping     Last observed ping time, in milliseconds (ms)\n"
+        "  send     Time since last message sent to the peer, in seconds\n"
+        "  recv     Time since last message received from the peer, in seconds\n"
+        "  txn      Time since last novel transaction received from the peer and accepted into our mempool, in minutes\n"
+        "  blk      Time since last novel block passing initial validity checks received from the peer, in minutes\n"
+        "  hb       High-bandwidth BIP152 compact block relay\n"
+        "           \".\" (to)   - we selected the peer as a high-bandwidth peer\n"
+        "           \"*\" (from) - the peer selected us as a high-bandwidth peer\n"
+        "  age      Duration of connection to the peer, in minutes\n"
+        "  asmap    Mapped AS (Autonomous System) number in the BGP route to the peer, used for diversifying\n"
+        "           peer selection (only displayed if the -asmap config option is set)\n"
+        "  id       Peer index, in increasing order of peer connections since node startup\n"
+        "  address  IP address and port of the peer\n"
+        "  version  Peer version and subversion concatenated, e.g. \"70016/Satoshi:21.0.0/\"\n\n"
+        "* The connection counts table displays the number of peers by direction, network, and the totals\n"
+        "  for each, as well as two special outbound columns for block relay peers and manual peers.\n\n"
+        "* The local addresses table lists each local address broadcast by the node, the port, and the score.\n\n"
+        "Examples:\n\n"
+        "Connection counts and local addresses only\n"
+        "> bitcoin-cli -netinfo\n\n"
+        "Compact peers listing\n"
+        "> bitcoin-cli -netinfo 1\n\n"
+        "Full dashboard\n"
+        + strprintf("> bitcoin-cli -netinfo %d\n\n", MAX_DETAIL_LEVEL) +
+        "Full live dashboard, adjust --interval or --no-title as needed (Linux)\n"
+        + strprintf("> watch --interval 1 --no-title bitcoin-cli -netinfo %d\n\n", MAX_DETAIL_LEVEL) +
+        "See this help\n"
+        "> bitcoin-cli -netinfo help\n"};
 };
 
 /** Process RPC generatetoaddress request. */
@@ -907,6 +898,10 @@ static int CommandLineRPC(int argc, char *argv[])
         if (gArgs.IsArgSet("-getinfo")) {
             rh.reset(new GetinfoRequestHandler());
         } else if (gArgs.GetBoolArg("-netinfo", false)) {
+            if (!args.empty() && args.at(0) == "help") {
+                tfm::format(std::cout, "%s\n", NetinfoRequestHandler().m_help_doc);
+                return 0;
+            }
             rh.reset(new NetinfoRequestHandler());
         } else if (gArgs.GetBoolArg("-generate", false)) {
             const UniValue getnewaddress{GetNewAddress()};