cli: Implement human readable -getinfo.

klementtan · klementtan · commit a37e29d32fde · 2021-07-21T19:27:04.000+08:00
diff --git a/doc/release-notes.md b/doc/release-notes.md
@@ -75,6 +75,8 @@ Updated settings
 Tools and Utilities
 -------------------
 
+- Update `-getinfo` to return data in a user-friendly format that also reduces vertical space. (#21832)
+
 Wallet
 ------
 
diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
@@ -9,6 +9,7 @@
 
 #include <chainparamsbase.h>
 #include <clientversion.h>
+#include <policy/feerate.h>
 #include <rpc/client.h>
 #include <rpc/mining.h>
 #include <rpc/protocol.h>
@@ -28,6 +29,10 @@
 #include <string>
 #include <tuple>
 
+#ifndef WIN32
+#include <unistd.h>
+#endif
+
 #include <event2/buffer.h>
 #include <event2/keyvalq_struct.h>
 #include <support/events.h>
@@ -48,6 +53,9 @@ static constexpr int8_t UNKNOWN_NETWORK{-1};
 /** Default number of blocks to generate for RPC generatetoaddress. */
 static const std::string DEFAULT_NBLOCKS = "1";
 
+/** Default -color setting. */
+static const std::string DEFAULT_COLOR_SETTING{"auto"};
+
 static void SetupCliArgs(ArgsManager& argsman)
 {
     SetupHelpOptions(argsman);
@@ -66,6 +74,7 @@ static void SetupCliArgs(ArgsManager& argsman)
     argsman.AddArg("-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). Pass \"help\" for detailed help documentation.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
 
     SetupChainParamsBaseOptions(argsman);
+    argsman.AddArg("-color=<when>", strprintf("Color setting for CLI output (default: %s). Valid values: always, auto (add color codes when standard output is connected to a terminal and OS is not WIN32), never.", DEFAULT_COLOR_SETTING), ArgsManager::ALLOW_STRING, OptionsCategory::OPTIONS);
     argsman.AddArg("-named", strprintf("Pass named instead of positional arguments (default: %s)", DEFAULT_NAMED), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-rpcclienttimeout=<n>", strprintf("Timeout in seconds during HTTP requests, or 0 for no timeout. (default: %d)", DEFAULT_HTTP_CLIENT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-rpcconnect=<ip>", strprintf("Send commands to node running on <ip> (default: %s)", DEFAULT_RPCCONNECT), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
@@ -338,7 +347,9 @@ class GetinfoRequestHandler: public BaseRequestHandler
         result.pushKV("difficulty", batch[ID_BLOCKCHAININFO]["result"]["difficulty"]);
         result.pushKV("chain", UniValue(batch[ID_BLOCKCHAININFO]["result"]["chain"]));
         if (!batch[ID_WALLETINFO]["result"].isNull()) {
+            result.pushKV("has_wallet", true);
             result.pushKV("keypoolsize", batch[ID_WALLETINFO]["result"]["keypoolsize"]);
+            result.pushKV("walletname", batch[ID_WALLETINFO]["result"]["walletname"]);
             if (!batch[ID_WALLETINFO]["result"]["unlocked_until"].isNull()) {
                 result.pushKV("unlocked_until", batch[ID_WALLETINFO]["result"]["unlocked_until"]);
             }
@@ -873,6 +884,100 @@ static void GetWalletBalances(UniValue& result)
     result.pushKV("balances", balances);
 }
 
+/**
+ * ParseGetInfoResult takes in -getinfo result in UniValue object and parses it
+ * into a user friendly UniValue string to be printed on the console.
+ * @param[out] result  Reference to UniValue result containing the -getinfo output.
+ */
+static void ParseGetInfoResult(UniValue& result)
+{
+    if (!find_value(result, "error").isNull()) return;
+
+    std::string RESET, GREEN, BLUE, YELLOW, MAGENTA, CYAN;
+    bool should_colorize = false;
+
+#ifndef WIN32
+    if (isatty(fileno(stdout))) {
+        // By default, only print colored text if OS is not WIN32 and stdout is connected to a terminal.
+        should_colorize = true;
+    }
+#endif
+
+    if (gArgs.IsArgSet("-color")) {
+        const std::string color{gArgs.GetArg("-color", DEFAULT_COLOR_SETTING)};
+        if (color == "always") {
+            should_colorize = true;
+        } else if (color == "never") {
+            should_colorize = false;
+        } else if (color != "auto") {
+            throw std::runtime_error("Invalid value for -color option. Valid values: always, auto, never.");
+        }
+    }
+
+    if (should_colorize) {
+        RESET = "\x1B[0m";
+        GREEN = "\x1B[32m";
+        BLUE = "\x1B[34m";
+        YELLOW = "\x1B[33m";
+        MAGENTA = "\x1B[35m";
+        CYAN = "\x1B[36m";
+    }
+
+    std::string result_string = strprintf("%sChain: %s%s\n", BLUE, result["chain"].getValStr(), RESET);
+    result_string += strprintf("Blocks: %s\n", result["blocks"].getValStr());
+    result_string += strprintf("Headers: %s\n", result["headers"].getValStr());
+    result_string += strprintf("Verification progress: %.4f%%\n", result["verificationprogress"].get_real() * 100);
+    result_string += strprintf("Difficulty: %s\n\n", result["difficulty"].getValStr());
+
+    result_string += strprintf(
+        "%sNetwork: in %s, out %s, total %s%s\n",
+        GREEN,
+        result["connections"]["in"].getValStr(),
+        result["connections"]["out"].getValStr(),
+        result["connections"]["total"].getValStr(),
+        RESET);
+    result_string += strprintf("Version: %s\n", result["version"].getValStr());
+    result_string += strprintf("Time offset (s): %s\n", result["timeoffset"].getValStr());
+    const std::string proxy = result["proxy"].getValStr();
+    result_string += strprintf("Proxy: %s\n", proxy.empty() ? "N/A" : proxy);
+    result_string += strprintf("Min tx relay fee rate (%s/kvB): %s\n\n", CURRENCY_UNIT, result["relayfee"].getValStr());
+
+    if (!result["has_wallet"].isNull()) {
+        const std::string walletname = result["walletname"].getValStr();
+        result_string += strprintf("%sWallet: %s%s\n", MAGENTA, walletname.empty() ? "\"\"" : walletname, RESET);
+
+        result_string += strprintf("Keypool size: %s\n", result["keypoolsize"].getValStr());
+        if (!result["unlocked_until"].isNull()) {
+            result_string += strprintf("Unlocked until: %s\n", result["unlocked_until"].getValStr());
+        }
+        result_string += strprintf("Transaction fee rate (-paytxfee) (%s/kvB): %s\n\n", CURRENCY_UNIT, result["paytxfee"].getValStr());
+    }
+    if (!result["balance"].isNull()) {
+        result_string += strprintf("%sBalance:%s %s\n\n", CYAN, RESET, result["balance"].getValStr());
+    }
+
+    if (!result["balances"].isNull()) {
+        result_string += strprintf("%sBalances%s\n", CYAN, RESET);
+
+        size_t max_balance_length{10};
+
+        for (const std::string& wallet : result["balances"].getKeys()) {
+            max_balance_length = std::max(result["balances"][wallet].getValStr().length(), max_balance_length);
+        }
+
+        for (const std::string& wallet : result["balances"].getKeys()) {
+            result_string += strprintf("%*s %s\n",
+                                       max_balance_length,
+                                       result["balances"][wallet].getValStr(),
+                                       wallet.empty() ? "\"\"" : wallet);
+        }
+        result_string += "\n";
+    }
+
+    result_string += strprintf("%sWarnings:%s %s", YELLOW, RESET, result["warnings"].getValStr());
+    result.setStr(result_string);
+}
+
 /**
  * Call RPC getnewaddress.
  * @returns getnewaddress response as a UniValue object.
@@ -994,9 +1099,13 @@ static int CommandLineRPC(int argc, char *argv[])
             UniValue result = find_value(reply, "result");
             const UniValue& error = find_value(reply, "error");
             if (error.isNull()) {
-                if (gArgs.IsArgSet("-getinfo") && !gArgs.IsArgSet("-rpcwallet")) {
-                    GetWalletBalances(result); // fetch multiwallet balances and append to result
+                if (gArgs.GetBoolArg("-getinfo", false)) {
+                    if (!gArgs.IsArgSet("-rpcwallet")) {
+                        GetWalletBalances(result); // fetch multiwallet balances and append to result
+                    }
+                    ParseGetInfoResult(result);
                 }
+
                 ParseResult(result, strPrint);
             } else {
                 ParseError(error, strPrint, nRet);
diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py
@@ -5,6 +5,7 @@
 """Test bitcoin-cli"""
 
 from decimal import Decimal
+import re
 
 from test_framework.blocktools import COINBASE_MATURITY
 from test_framework.test_framework import BitcoinTestFramework
@@ -29,6 +30,41 @@
 WALLET_NOT_LOADED = 'Requested wallet does not exist or is not loaded'
 WALLET_NOT_SPECIFIED = 'Wallet file not specified'
 
+
+def cli_get_info_string_to_dict(cli_get_info_string):
+    """Helper method to convert human-readable -getinfo into a dictionary"""
+    cli_get_info = {}
+    lines = cli_get_info_string.splitlines()
+    line_idx = 0
+    ansi_escape = re.compile(r'(\x9B|\x1B\[)[0-?]*[ -\/]*[@-~]')
+    while line_idx < len(lines):
+        # Remove ansi colour code
+        line = ansi_escape.sub('', lines[line_idx])
+        if "Balances" in line:
+            # When "Balances" appears in a line, all of the following lines contain "balance: wallet" until an empty line
+            cli_get_info["Balances"] = {}
+            while line_idx < len(lines) and not (lines[line_idx + 1] == ''):
+                line_idx += 1
+                balance, wallet = lines[line_idx].strip().split(" ")
+                # Remove right justification padding
+                wallet = wallet.strip()
+                if wallet == '""':
+                    # Set default wallet("") to empty string
+                    wallet = ''
+                cli_get_info["Balances"][wallet] = balance.strip()
+        elif ": " in line:
+            key, value = line.split(": ")
+            if key == 'Wallet' and value == '""':
+                # Set default wallet("") to empty string
+                value = ''
+            if key == "Proxy" and value == "N/A":
+                # Set N/A to empty string to represent no proxy
+                value = ''
+            cli_get_info[key.strip()] = value.strip()
+        line_idx += 1
+    return cli_get_info
+
+
 class TestBitcoinCli(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
@@ -67,37 +103,43 @@ def run_test(self):
         self.log.info("Test -getinfo with arguments fails")
         assert_raises_process_error(1, "-getinfo takes no arguments", self.nodes[0].cli('-getinfo').help)
 
+        self.log.info("Test -getinfo with -color=never does not return ANSI escape codes")
+        assert "\u001b[0m" not in self.nodes[0].cli('-getinfo', '-color=never').send_cli()
+
+        self.log.info("Test -getinfo with -color=always returns ANSI escape codes")
+        assert "\u001b[0m" in self.nodes[0].cli('-getinfo', '-color=always').send_cli()
+
+        self.log.info("Test -getinfo with invalid value for -color option")
+        assert_raises_process_error(1, "Invalid value for -color option. Valid values: always, auto, never.", self.nodes[0].cli('-getinfo', '-color=foo').send_cli)
+
         self.log.info("Test -getinfo returns expected network and blockchain info")
         if self.is_wallet_compiled():
             self.nodes[0].encryptwallet(password)
-        cli_get_info = self.nodes[0].cli('-getinfo').send_cli()
+        cli_get_info_string = self.nodes[0].cli('-getinfo').send_cli()
+        cli_get_info = cli_get_info_string_to_dict(cli_get_info_string)
+
         network_info = self.nodes[0].getnetworkinfo()
         blockchain_info = self.nodes[0].getblockchaininfo()
-        assert_equal(cli_get_info['version'], network_info['version'])
-        assert_equal(cli_get_info['blocks'], blockchain_info['blocks'])
-        assert_equal(cli_get_info['headers'], blockchain_info['headers'])
-        assert_equal(cli_get_info['timeoffset'], network_info['timeoffset'])
-        assert_equal(
-            cli_get_info['connections'],
-            {
-                'in': network_info['connections_in'],
-                'out': network_info['connections_out'],
-                'total': network_info['connections']
-            }
-        )
-        assert_equal(cli_get_info['proxy'], network_info['networks'][0]['proxy'])
-        assert_equal(cli_get_info['difficulty'], blockchain_info['difficulty'])
-        assert_equal(cli_get_info['chain'], blockchain_info['chain'])
+        assert_equal(int(cli_get_info['Version']), network_info['version'])
+        assert_equal(cli_get_info['Verification progress'], "%.4f%%" % (blockchain_info['verificationprogress'] * 100))
+        assert_equal(int(cli_get_info['Blocks']), blockchain_info['blocks'])
+        assert_equal(int(cli_get_info['Headers']), blockchain_info['headers'])
+        assert_equal(int(cli_get_info['Time offset (s)']), network_info['timeoffset'])
+        expected_network_info = f"in {network_info['connections_in']}, out {network_info['connections_out']}, total {network_info['connections']}"
+        assert_equal(cli_get_info["Network"], expected_network_info)
+        assert_equal(cli_get_info['Proxy'], network_info['networks'][0]['proxy'])
+        assert_equal(Decimal(cli_get_info['Difficulty']), blockchain_info['difficulty'])
+        assert_equal(cli_get_info['Chain'], blockchain_info['chain'])
 
         if self.is_wallet_compiled():
             self.log.info("Test -getinfo and bitcoin-cli getwalletinfo return expected wallet info")
-            assert_equal(cli_get_info['balance'], BALANCE)
-            assert 'balances' not in cli_get_info.keys()
+            assert_equal(Decimal(cli_get_info['Balance']), BALANCE)
+            assert 'Balances' not in cli_get_info_string
             wallet_info = self.nodes[0].getwalletinfo()
-            assert_equal(cli_get_info['keypoolsize'], wallet_info['keypoolsize'])
-            assert_equal(cli_get_info['unlocked_until'], wallet_info['unlocked_until'])
-            assert_equal(cli_get_info['paytxfee'], wallet_info['paytxfee'])
-            assert_equal(cli_get_info['relayfee'], network_info['relayfee'])
+            assert_equal(int(cli_get_info['Keypool size']), wallet_info['keypoolsize'])
+            assert_equal(int(cli_get_info['Unlocked until']), wallet_info['unlocked_until'])
+            assert_equal(Decimal(cli_get_info['Transaction fee rate (-paytxfee) (BTC/kvB)']), wallet_info['paytxfee'])
+            assert_equal(Decimal(cli_get_info['Min tx relay fee rate (BTC/kvB)']), network_info['relayfee'])
             assert_equal(self.nodes[0].cli.getwalletinfo(), wallet_info)
 
             # Setup to test -getinfo, -generate, and -rpcwallet= with multiple wallets.
@@ -120,44 +162,57 @@ def run_test(self):
 
             self.log.info("Test -getinfo with multiple wallets and -rpcwallet returns specified wallet balance")
             for i in range(len(wallets)):
-                cli_get_info = self.nodes[0].cli('-getinfo', '-rpcwallet={}'.format(wallets[i])).send_cli()
-                assert 'balances' not in cli_get_info.keys()
-                assert_equal(cli_get_info['balance'], amounts[i])
+                cli_get_info_string = self.nodes[0].cli('-getinfo', '-rpcwallet={}'.format(wallets[i])).send_cli()
+                cli_get_info = cli_get_info_string_to_dict(cli_get_info_string)
+                assert 'Balances' not in cli_get_info_string
+                assert_equal(cli_get_info["Wallet"], wallets[i])
+                assert_equal(Decimal(cli_get_info['Balance']), amounts[i])
 
             self.log.info("Test -getinfo with multiple wallets and -rpcwallet=non-existing-wallet returns no balances")
-            cli_get_info_keys = self.nodes[0].cli('-getinfo', '-rpcwallet=does-not-exist').send_cli().keys()
-            assert 'balance' not in cli_get_info_keys
-            assert 'balances' not in cli_get_info_keys
+            cli_get_info_string = self.nodes[0].cli('-getinfo', '-rpcwallet=does-not-exist').send_cli()
+            assert 'Balance' not in cli_get_info_string
+            assert 'Balances' not in cli_get_info_string
 
             self.log.info("Test -getinfo with multiple wallets returns all loaded wallet names and balances")
             assert_equal(set(self.nodes[0].listwallets()), set(wallets))
-            cli_get_info = self.nodes[0].cli('-getinfo').send_cli()
-            assert 'balance' not in cli_get_info.keys()
-            assert_equal(cli_get_info['balances'], {k: v for k, v in zip(wallets, amounts)})
+            cli_get_info_string = self.nodes[0].cli('-getinfo').send_cli()
+            cli_get_info = cli_get_info_string_to_dict(cli_get_info_string)
+            assert 'Balance' not in cli_get_info
+            for k, v in zip(wallets, amounts):
+                assert_equal(Decimal(cli_get_info['Balances'][k]), v)
 
             # Unload the default wallet and re-verify.
             self.nodes[0].unloadwallet(wallets[0])
             assert wallets[0] not in self.nodes[0].listwallets()
-            cli_get_info = self.nodes[0].cli('-getinfo').send_cli()
-            assert 'balance' not in cli_get_info.keys()
-            assert_equal(cli_get_info['balances'], {k: v for k, v in zip(wallets[1:], amounts[1:])})
+            cli_get_info_string = self.nodes[0].cli('-getinfo').send_cli()
+            cli_get_info = cli_get_info_string_to_dict(cli_get_info_string)
+            assert 'Balance' not in cli_get_info
+            assert 'Balances' in cli_get_info_string
+            for k, v in zip(wallets[1:], amounts[1:]):
+                assert_equal(Decimal(cli_get_info['Balances'][k]), v)
+            assert wallets[0] not in cli_get_info
 
             self.log.info("Test -getinfo after unloading all wallets except a non-default one returns its balance")
             self.nodes[0].unloadwallet(wallets[2])
             assert_equal(self.nodes[0].listwallets(), [wallets[1]])
-            cli_get_info = self.nodes[0].cli('-getinfo').send_cli()
-            assert 'balances' not in cli_get_info.keys()
-            assert_equal(cli_get_info['balance'], amounts[1])
+            cli_get_info_string = self.nodes[0].cli('-getinfo').send_cli()
+            cli_get_info = cli_get_info_string_to_dict(cli_get_info_string)
+            assert 'Balances' not in cli_get_info_string
+            assert_equal(cli_get_info['Wallet'], wallets[1])
+            assert_equal(Decimal(cli_get_info['Balance']), amounts[1])
 
             self.log.info("Test -getinfo with -rpcwallet=remaining-non-default-wallet returns only its balance")
-            cli_get_info = self.nodes[0].cli('-getinfo', rpcwallet2).send_cli()
-            assert 'balances' not in cli_get_info.keys()
-            assert_equal(cli_get_info['balance'], amounts[1])
+            cli_get_info_string = self.nodes[0].cli('-getinfo', rpcwallet2).send_cli()
+            cli_get_info = cli_get_info_string_to_dict(cli_get_info_string)
+            assert 'Balances' not in cli_get_info_string
+            assert_equal(cli_get_info['Wallet'], wallets[1])
+            assert_equal(Decimal(cli_get_info['Balance']), amounts[1])
 
             self.log.info("Test -getinfo with -rpcwallet=unloaded wallet returns no balances")
-            cli_get_info_keys = self.nodes[0].cli('-getinfo', rpcwallet3).send_cli().keys()
-            assert 'balance' not in cli_get_info_keys
-            assert 'balances' not in cli_get_info_keys
+            cli_get_info_string = self.nodes[0].cli('-getinfo', rpcwallet3).send_cli()
+            cli_get_info_keys = cli_get_info_string_to_dict(cli_get_info_string)
+            assert 'Balance' not in cli_get_info_keys
+            assert 'Balances' not in cli_get_info_string
 
             # Test bitcoin-cli -generate.
             n1 = 3
