foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-02 09:46:52 -05:00
Code Issues Activity

Merge #18594: cli: display multiwallet balances in -getinfo

Browse source 
5edad5ce5d test: add -getinfo multiwallet functional tests (Jon Atack)
903b6c117f rpc: drop unused JSONRPCProcessBatchReply size arg, refactor (Jon Atack)
afce85eb99 cli: use GetWalletBalances() functionality for -getinfo (Jon Atack)
9f01849a49 cli: create GetWalletBalances() to fetch multiwallet balances (Jon Atack)
743077544b cli: lift -rpcwallet logic up to CommandLineRPC() (Jon Atack)
29f2cbdeb7 cli: extract connection exception handler, -rpcwait logic (Jon Atack)

Pull request description:

  This PR is a client-side version of #18453, per review feedback there and [review club discussions](https://bitcoincore.reviews/18453#meeting-log). It updates `bitcoin-cli -getinfo` on the client side to display wallet name and balance for the loaded wallets when more than one is loaded (e.g. you are in "multiwallet mode") and `-rpcwallet=` is not passed; otherwise, behavior is unchanged.

  before
  ```json
  $ bitcoin-cli -getinfo -regtest
  {
    "version": 199900,
    "blocks": 15599,
    "headers": 15599,
    "verificationprogress": 1,
    "timeoffset": 0,
    "connections": 0,
    "proxy": "",
    "difficulty": 4.656542373906925e-10,
    "chain": "regtest",
    "balance": 0.00001000,
    "relayfee": 0.00001000
  }

  ```
  after
  ```json
  $ bitcoin-cli -getinfo -regtest
  {
    "version": 199900,
    "blocks": 15599,
    "headers": 15599,
    "verificationprogress": 1,
    "timeoffset": 0,
    "connections": 0,
    "proxy": "",
    "difficulty": 4.656542373906925e-10,
    "chain": "regtest",
    "balances": {
      "": 0.00001000,
      "Encrypted": 0.00003500,
      "day-to-day": 0.00000120,
      "side project": 0.00000094
    }
  }
  ```
  -----

  `Review club` discussion about this PR is here: https://bitcoincore.reviews/18453

  This PR can be manually tested by building, creating/loading/unloading several wallets with `bitcoin-cli createwallet/loadwallet/unloadwallet` and running `bitcoin-cli -getinfo` and `bitcoin-cli -rpcwallet=<wallet-name> -getinfo`.

  `wallet_multiwallet.py --usecli` provides regression test coverage on this change, along with `interface_bitcoin_cli.py` where this PR adds test coverage.

  Credit to Wladimir J. van der Laan for the idea in https://github.com/bitcoin/bitcoin/issues/17314 and https://github.com/bitcoin/bitcoin/pull/18453#issuecomment-605431806.

ACKs for top commit:
  promag:
    Tested ACK 5edad5ce5d.
  jnewbery:
    utACK 5edad5ce5d
  meshcollider:
    Code review ACK 5edad5ce5d

Tree-SHA512: 4ca36c5f6c49936b40afb605c44459c1d5b80b5bd84df634007ca276b3f6c102a0cb382f9d528370363ee32c94b0d7ffa15184578eaf8de74179e566c5c5cee5
This commit is contained in:
Samuel Dobson 2020-05-24 00:06:19 +12:00
commit 24f7029064
No known key found for this signature in database
GPG key ID: D300116E1C875A3D
4 changed files with 139 additions and 79 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

159
src/bitcoin-cli.cpp
View file

@ -9,6 +9,7 @@
#include <chainparamsbase.h>
#include <clientversion.h>
#include <optional.h>
#include <rpc/client.h>
#include <rpc/protocol.h>
#include <rpc/request.h>
@ -250,7 +251,7 @@ public:
UniValue ProcessReply(const UniValue &batch_in) override
{
UniValue result(UniValue::VOBJ);
std::vector<UniValue> batch = JSONRPCProcessBatchReply(batch_in, batch_in.size());
const std::vector<UniValue> batch = JSONRPCProcessBatchReply(batch_in);
// Errors in getnetworkinfo() and getblockchaininfo() are fatal, pass them on;
// getwalletinfo() and getbalances() are allowed to fail if there is no wallet.
if (!batch[ID_NETWORKINFO]["error"].isNull()) {
@ -304,7 +305,7 @@ public:
}
};
static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, const std::vector<std::string>& args)
static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, const std::vector<std::string>& args, const Optional<std::string>& rpcwallet = {})
{
std::string host;
// In preference order, we choose the following for the port:
@ -369,14 +370,12 @@ static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, co
// check if we should use a special wallet endpoint
std::string endpoint = "/";
if (!gArgs.GetArgs("-rpcwallet").empty()) {
std::string walletName = gArgs.GetArg("-rpcwallet", "");
char *encodedURI = evhttp_uriencode(walletName.data(), walletName.size(), false);
if (rpcwallet) {
char* encodedURI = evhttp_uriencode(rpcwallet->data(), rpcwallet->size(), false);
if (encodedURI) {
endpoint = "/wallet/"+ std::string(encodedURI);
endpoint = "/wallet/" + std::string(encodedURI);
free(encodedURI);
}
else {
} else {
throw CConnectionFailed("uri-encode failed");
}
}
@ -418,6 +417,65 @@ static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, co
return reply;
}
/**
* ConnectAndCallRPC wraps CallRPC with -rpcwait and an exception handler.
*
* @param[in] rh Pointer to RequestHandler.
* @param[in] strMethod Reference to const string method to forward to CallRPC.
* @param[in] rpcwallet Reference to const optional string wallet name to forward to CallRPC.
* @returns the RPC response as a UniValue object.
* @throws a CConnectionFailed std::runtime_error if connection failed or RPC server still in warmup.
*/
static UniValue ConnectAndCallRPC(BaseRequestHandler* rh, const std::string& strMethod, const std::vector<std::string>& args, const Optional<std::string>& rpcwallet = {})
{
UniValue response(UniValue::VOBJ);
// Execute and handle connection failures with -rpcwait.
const bool fWait = gArgs.GetBoolArg("-rpcwait", false);
do {
try {
response = CallRPC(rh, strMethod, args, rpcwallet);
if (fWait) {
const UniValue& error = find_value(response, "error");
if (!error.isNull() && error["code"].get_int() == RPC_IN_WARMUP) {
throw CConnectionFailed("server in warmup");
}
}
break; // Connection succeeded, no need to retry.
} catch (const CConnectionFailed&) {
if (fWait) {
UninterruptibleSleep(std::chrono::milliseconds{1000});
} else {
throw;
}
}
} while (fWait);
return response;
}
/**
* GetWalletBalances calls listwallets; if more than one wallet is loaded, it then
* fetches mine.trusted balances for each loaded wallet and pushes them to `result`.
*
* @param result Reference to UniValue object the wallet names and balances are pushed to.
*/
static void GetWalletBalances(UniValue& result)
{
std::unique_ptr<BaseRequestHandler> rh{MakeUnique<DefaultRequestHandler>()};
const UniValue listwallets = ConnectAndCallRPC(rh.get(), "listwallets", /* args=*/{});
if (!find_value(listwallets, "error").isNull()) return;
const UniValue& wallets = find_value(listwallets, "result");
if (wallets.size() <= 1) return;
UniValue balances(UniValue::VOBJ);
for (const UniValue& wallet : wallets.getValues()) {
const std::string wallet_name = wallet.get_str();
const UniValue getbalances = ConnectAndCallRPC(rh.get(), "getbalances", /* args=*/{}, wallet_name);
const UniValue& balance = find_value(getbalances, "result")["mine"]["trusted"];
balances.pushKV(wallet_name, balance);
}
result.pushKV("balances", balances);
}
static int CommandLineRPC(int argc, char *argv[])
{
std::string strPrint;
@ -474,9 +532,8 @@ static int CommandLineRPC(int argc, char *argv[])
}
std::unique_ptr<BaseRequestHandler> rh;
std::string method;
if (gArgs.GetBoolArg("-getinfo", false)) {
if (gArgs.IsArgSet("-getinfo")) {
rh.reset(new GetinfoRequestHandler());
method = "";
} else {
rh.reset(new DefaultRequestHandler());
if (args.size() < 1) {
@ -485,62 +542,46 @@ static int CommandLineRPC(int argc, char *argv[])
method = args[0];
args.erase(args.begin()); // Remove trailing method name from arguments vector
}
Optional<std::string> wallet_name{};
if (gArgs.IsArgSet("-rpcwallet")) wallet_name = gArgs.GetArg("-rpcwallet", "");
const UniValue reply = ConnectAndCallRPC(rh.get(), method, args, wallet_name);
// Execute and handle connection failures with -rpcwait
const bool fWait = gArgs.GetBoolArg("-rpcwait", false);
do {
try {
const UniValue reply = CallRPC(rh.get(), method, args);
// Parse reply
UniValue result = find_value(reply, "result");
const UniValue& error = find_value(reply, "error");
if (!error.isNull()) {
// Error
strPrint = "error: " + error.write();
nRet = abs(error["code"].get_int());
if (error.isObject()) {
const UniValue& errCode = find_value(error, "code");
const UniValue& errMsg = find_value(error, "message");
strPrint = errCode.isNull() ? "" : ("error code: " + errCode.getValStr() + "\n");
// Parse reply
const UniValue& result = find_value(reply, "result");
const UniValue& error = find_value(reply, "error");
if (!error.isNull()) {
// Error
int code = error["code"].get_int();
if (fWait && code == RPC_IN_WARMUP)
throw CConnectionFailed("server in warmup");
strPrint = "error: " + error.write();
nRet = abs(code);
if (error.isObject())
{
UniValue errCode = find_value(error, "code");
UniValue errMsg = find_value(error, "message");
strPrint = errCode.isNull() ? "" : "error code: "+errCode.getValStr()+"\n";
if (errMsg.isStr())
strPrint += "error message:\n"+errMsg.get_str();
if (errCode.isNum() && errCode.get_int() == RPC_WALLET_NOT_SPECIFIED) {
strPrint += "\nTry adding \"-rpcwallet=<filename>\" option to bitcoin-cli command line.";
}
}
} else {
// Result
if (result.isNull())
strPrint = "";
else if (result.isStr())
strPrint = result.get_str();
else
strPrint = result.write(2);
if (errMsg.isStr()) {
strPrint += ("error message:\n" + errMsg.get_str());
}
if (errCode.isNum() && errCode.get_int() == RPC_WALLET_NOT_SPECIFIED) {
strPrint += "\nTry adding \"-rpcwallet=<filename>\" option to bitcoin-cli command line.";
}
// Connection succeeded, no need to retry.
break;
}
catch (const CConnectionFailed&) {
if (fWait)
UninterruptibleSleep(std::chrono::milliseconds{1000});
else
throw;
} else {
if (gArgs.IsArgSet("-getinfo") && !gArgs.IsArgSet("-rpcwallet")) {
GetWalletBalances(result); // fetch multiwallet balances and append to result
}
} while (fWait);
}
catch (const std::exception& e) {
// Result
if (result.isNull()) {
strPrint = "";
} else if (result.isStr()) {
strPrint = result.get_str();
} else {
strPrint = result.write(2);
}
}
} catch (const std::exception& e) {
strPrint = std::string("error: ") + e.what();
nRet = EXIT_FAILURE;
}
catch (...) {
} catch (...) {
PrintExceptionContinue(nullptr, "CommandLineRPC()");
throw;
}

10
src/rpc/request.cpp
View file

@ -130,20 +130,20 @@ void DeleteAuthCookie()
}
}
std::vector<UniValue> JSONRPCProcessBatchReply(const UniValue &in, size_t num)
std::vector<UniValue> JSONRPCProcessBatchReply(const UniValue& in)
{
if (!in.isArray()) {
throw std::runtime_error("Batch must be an array");
}
const size_t num {in.size()};
std::vector<UniValue> batch(num);
for (size_t i=0; i<in.size(); ++i) {
const UniValue &rec = in[i];
for (const UniValue& rec : in.getValues()) {
if (!rec.isObject()) {
throw std::runtime_error("Batch member must be object");
throw std::runtime_error("Batch member must be an object");
}
size_t id = rec["id"].get_int();
if (id >= num) {
throw std::runtime_error("Batch member id larger than size");
throw std::runtime_error("Batch member id is larger than batch size");
}
batch[id] = rec;
}

2
src/rpc/request.h
View file

@ -26,7 +26,7 @@ bool GetAuthCookie(std::string *cookie_out);
/** Delete RPC authentication cookie from disk */
void DeleteAuthCookie();
/** Parse JSON-RPC batch reply into a vector */
std::vector<UniValue> JSONRPCProcessBatchReply(const UniValue &in, size_t num);
std::vector<UniValue> JSONRPCProcessBatchReply(const UniValue& in);
class JSONRPCRequest
{

47
test/functional/interface_bitcoin_cli.py
View file

@ -67,6 +67,7 @@ class TestBitcoinCli(BitcoinTestFramework):
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()
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'])
@ -76,42 +77,60 @@ class TestBitcoinCli(BitcoinTestFramework):
# Setup to test -getinfo and -rpcwallet= with multiple wallets.
wallets = ['', 'Encrypted', 'secret']
amounts = [Decimal('59.999928'), Decimal(9), Decimal(31)]
amounts = [BALANCE + Decimal('9.999928'), Decimal(9), Decimal(31)]
self.nodes[0].createwallet(wallet_name=wallets[1])
self.nodes[0].createwallet(wallet_name=wallets[2])
w1 = self.nodes[0].get_wallet_rpc(wallets[0])
w2 = self.nodes[0].get_wallet_rpc(wallets[1])
w3 = self.nodes[0].get_wallet_rpc(wallets[2])
w1.walletpassphrase(password, self.rpc_timeout)
w2.encryptwallet(password)
w1.sendtoaddress(w2.getnewaddress(), amounts[1])
w1.sendtoaddress(w3.getnewaddress(), amounts[2])
# Mine a block to confirm; adds a block reward (50 BTC) to the default wallet.
self.nodes[0].generate(1)
self.log.info("Test -getinfo with multiple wallets loaded returns no balance")
assert_equal(set(self.nodes[0].listwallets()), set(wallets))
assert 'balance' not in self.nodes[0].cli('-getinfo').send_cli().keys()
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').send_cli('-rpcwallet={}'.format(wallets[i]))
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])
self.log.info("Test -getinfo with multiple wallets and -rpcwallet=non-existing-wallet returns no balance")
assert 'balance' not in self.nodes[0].cli('-getinfo').send_cli('-rpcwallet=does-not-exist').keys()
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
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)})
# 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:])})
self.log.info("Test -getinfo after unloading all wallets except a non-default one returns its balance")
self.nodes[0].unloadwallet(wallets[0])
self.nodes[0].unloadwallet(wallets[2])
assert_equal(self.nodes[0].listwallets(), [wallets[1]])
assert_equal(self.nodes[0].cli('-getinfo').send_cli()['balance'], amounts[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])
self.log.info("Test -getinfo -rpcwallet=remaining-non-default-wallet returns its balance")
assert_equal(self.nodes[0].cli('-getinfo').send_cli('-rpcwallet={}'.format(wallets[1]))['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', '-rpcwallet={}'.format(wallets[1])).send_cli()
assert 'balances' not in cli_get_info.keys()
assert_equal(cli_get_info['balance'], amounts[1])
self.log.info("Test -getinfo with -rpcwallet=unloaded wallet returns no balance")
assert 'balance' not in self.nodes[0].cli('-getinfo').send_cli('-rpcwallet={}'.format(wallets[2])).keys()
self.log.info("Test -getinfo with -rpcwallet=unloaded wallet returns no balances")
cli_get_info = self.nodes[0].cli('-getinfo', '-rpcwallet={}'.format(wallets[2])).send_cli()
assert 'balance' not in cli_get_info_keys
assert 'balances' not in cli_get_info_keys
else:
self.log.info("*** Wallet not compiled; cli getwalletinfo and -getinfo wallet tests skipped")
self.nodes[0].generate(1) # maintain block parity with the wallet_compiled conditional branch