diff options
| -rw-r--r-- | doc/release-notes.md | 2 | ||||
| -rw-r--r-- | src/bitcoin-cli.cpp | 113 | ||||
| -rwxr-xr-x | test/functional/interface_bitcoin_cli.py | 141 |
3 files changed, 211 insertions, 45 deletions
diff --git a/doc/release-notes.md b/doc/release-notes.md index 99f26fc943..cf9edd9b08 100644 --- 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 index 7a5f945511..1ec6411e32 100644 --- 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 @@ public: 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"]); } @@ -874,6 +885,100 @@ static void GetWalletBalances(UniValue& result) } /** + * 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 index 22eec59600..dfa448a1a8 100755 --- 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 @@ TOO_MANY_ARGS = 'error: too many arguments (maximum 2 for nblocks and maxtries)' 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 @@ class TestBitcoinCli(BitcoinTestFramework): 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 @@ class TestBitcoinCli(BitcoinTestFramework): 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 |