cli: display multiwallet balances in -getinfo #18594

This PR is a client-side version of #18453, per review feedback there and review club discussions. 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

$ 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

$ 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 #17314 and #18453 (comment).

Concept ACK

interface_bitcoin_cli.py fails in no wallet job.

It seems this is in part reverting changes that were just merged in #18574 (split out of #18453), and refactoring to handle multiwallet.

I understand splitting up changes, and maybe it doesn't matter so much in this instance, but it's a bit of code/review churn if we're PR'ing and merging refactors only to essentially undo the changes and do something else in a different PR a few hours later.

I empathise. See #18453 (comment). #18574 was the only change that seemed to have consensus, so it seemed best to split it out for merge and propose client-side code in this PR to test and compare with the server-side code in #18453.

EDIT: This PR no longer touches the changes merged in #18574.

<!--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:

• #16439 (RPC: support "@height" in place of blockhash for getblock etc by ajtowns)

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.

Updated with the following:

• ensure it works when built without the wallet
• extract the connection try/wait/failure logic to be callable for each connection, so we can use this for calling listwallets and getbalances to be more robust
• drop an unused arg in JSONRPCProcessBatchReply as requested in #18574 (review)

Concept and approach ACK.

ACK ea59b18387c4b26240e1f087c568408922b3c940

Built, ran tests, tested manually by creating new wallet, transferring funds to it and unloading. Light code review.

PR updated per @promag's review feedback to display wallet names and balances for all loaded wallets when more than one wallet is loaded (e.g. you are in "multiwallet mode") and -rpcwallet= is not passed; otherwise, behavior is unchanged. @fanquake, this PR no longer touches the changes in #18574. @jnewbery, @promag, @michaelfolkson, can you please re-review?

Concept ACK, but I wonder why in 208be5a811eee4f4d4d336a9e47c2b36760621b5 this is hardcoded to getinfo? Couldn't this be generalized to any command with almost no extra work (return results grouped by wallet name), or am I missing something?

edit: hmm after reading some of the linked issues it looks like this is trying to achieve something very specific, but my suggestion could be a future improvement on top of this.

Thanks @jb55, I agree, but there isn't currently consensus on what the server-side API interface should be and if one should be done at all. I learned from the first PR that a client-side version embedded into -getinfo as proposed here will be easier to merge due to more clear consensus and because it isn't subject to API constraints. I see this as a solution that can be merged now, and maybe a server-side one might emerge (or not) later, and could be called from -getinfo as well to batch the calls to getbalances. Don't hesitate to ACK if you'd like to see it move forward!

ACK https://github.com/bitcoin/bitcoin/commit/9fa9882fbbf4324f36cbf1008642adf43436be53

Built, run and tested on macOS Catalina 10.15.4 Manual testing with several wallets successful:

./src/bitcoin-cli -regtest -getinfo
{
  "version": 209900,
  "blocks": 205,
  "headers": 205,
  "verificationprogress": 1,
  "timeoffset": 0,
  "connections": 0,
  "proxy": "",
  "difficulty": 4.656542373906925e-10,
  "chain": "regtest",
  "relayfee": 0.00001000,
  "warnings": "This is a pre-release test build - use at your own risk - do not use for mining or merchant applications",
  "balances": {
    "": 5000.00000000,
    "other": 250.00000000,
    "secret_wallet": 0.00000000
  }
}

Execution of ./test/functional/wallet_multiwallet.py --usecli was successful.

Good stuff Jon. A few style nits, but otherwise looks great

Thanks everyone for the reviews and @jnewbery for the excellent suggestions -- done and also moved the GetWalletBalances code to its own function and added Doxygen documentation for it and ConnectAndCallRPC.

<details> <summary>Changes: git diff 9fa9882 2e7d8b9</summary> <p>

diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
index 86271c4271..a3302835c3 100644
--- a/src/bitcoin-cli.cpp
+++ b/src/bitcoin-cli.cpp
@@ -17,6 +17,7 @@
 #include <util/translation.h>
 #include <util/url.h>
 
+#include <cstring>
 #include <functional>
 #include <memory>
 #include <stdio.h>
@@ -304,7 +305,7 @@ public:
     }
 };
 
-static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, const std::vector<std::string>& args, const char *rpcwallet = nullptr)
+static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, const std::vector<std::string>& args, const char * const rpcwallet = nullptr)
 {
     std::string host;
     // In preference order, we choose the following for the port:
@@ -369,9 +370,8 @@ static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, co
 
     // check if we should use a special wallet endpoint
     std::string endpoint = "/";
-    if (rpcwallet || !gArgs.GetArgs("-rpcwallet").empty()) {
-        std::string walletName = rpcwallet ? rpcwallet : gArgs.GetArg("-rpcwallet", "");
-        char *encodedURI = evhttp_uriencode(walletName.data(), walletName.size(), false);
+    if (rpcwallet) {
+        char *encodedURI = evhttp_uriencode(rpcwallet, strlen(rpcwallet), false);
         if (encodedURI) {
             endpoint = "/wallet/"+ std::string(encodedURI);
             free(encodedURI);
@@ -417,7 +417,16 @@ static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, co
     return reply;
 }
 
-static UniValue ConnectAndCallRPC(BaseRequestHandler *rh, const std::string& strMethod, const std::vector<std::string>& args, const char *rpcwallet = nullptr)
+/**
+ * ConnectAndCallRPC wraps CallRPC with -rpcwait and an exception handler.
+ *
+ * [@param](/bitcoin-bitcoin/contributor/param/)[in] rh         Pointer to RequestHandler.
+ * [@param](/bitcoin-bitcoin/contributor/param/)[in] strMethod  Reference to const string method to forward to CallRPC.
+ * [@param](/bitcoin-bitcoin/contributor/param/)[in] rpcwallet  Pointer to const c-string rpcwallet arg to forward to CallRPC.
+ * [@returns](/bitcoin-bitcoin/contributor/returns/) the RPC response as a UniValue object.
+ * [@throws](/bitcoin-bitcoin/contributor/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 char * const rpcwallet = nullptr)
 {
     UniValue response(UniValue::VOBJ);
     // Execute and handle connection failures with -rpcwait.
@@ -425,7 +434,6 @@ static UniValue ConnectAndCallRPC(BaseRequestHandler *rh, const std::string& str
     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) {
@@ -442,10 +450,35 @@ static UniValue ConnectAndCallRPC(BaseRequestHandler *rh, const std::string& str
             }
         }
     } 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](/bitcoin-bitcoin/contributor/param/)[in] result  Reference to UniValue object the wallet names and balances are pushed to.
+ */
+static void GetWalletBalances(UniValue& result)
+{
+    std::unique_ptr<BaseRequestHandler> rh;
+    rh.reset(new DefaultRequestHandler());
+    const std::vector<std::string> args;
+    const UniValue listwallets = ConnectAndCallRPC(rh.get(), "listwallets", args);
+    const UniValue& wallets = find_value(listwallets, "result");
+
+    if (find_value(listwallets, "error").isNull() && wallets.size() > 1) {
+        UniValue balances(UniValue::VOBJ);
+        for (const UniValue& wallet : wallets.getValues()) {
+            const char * const wallet_name = wallet.get_str().c_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;
@@ -502,7 +535,7 @@ 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());
         } else {
             rh.reset(new DefaultRequestHandler());
@@ -512,7 +545,13 @@ static int CommandLineRPC(int argc, char *argv[])
             method = args[0];
             args.erase(args.begin()); // Remove trailing method name from arguments vector
         }
-        const UniValue reply = ConnectAndCallRPC(rh.get(), method, args);
+
+        UniValue reply;
+        if (gArgs.IsArgSet("-rpcwallet")) {
+            reply = ConnectAndCallRPC(rh.get(), method, args, gArgs.GetArg("-rpcwallet", "").c_str());
+        } else {
+            reply = ConnectAndCallRPC(rh.get(), method, args);
+        }
 
         // Parse reply
         UniValue result = find_value(reply, "result");
@@ -534,22 +573,8 @@ static int CommandLineRPC(int argc, char *argv[])
                 }
             }
         } else {
-            // Display mine.trusted balances for all loaded wallets if -getinfo
-            // was called without -rpcwallet and more than one wallet is loaded.
-            if (gArgs.GetBoolArg("-getinfo", false) && gArgs.GetArgs("-rpcwallet").empty()) {
-                rh.reset(new DefaultRequestHandler());
-                const UniValue listwallets = ConnectAndCallRPC(rh.get(), "listwallets", args);
-                const UniValue& wallets = find_value(listwallets, "result");
-                    if (find_value(listwallets, "error").isNull() && wallets.size() > 1) {
-                    UniValue balances(UniValue::VOBJ);
-                    for (const UniValue& wallet : wallets.getValues()) {
-                        const char *wallet_name = wallet.get_str().c_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);
-                }
+            if (gArgs.IsArgSet("-getinfo") && !gArgs.IsArgSet("-rpcwallet")) {
+                GetWalletBalances(result); // fetch multiwallet balances and append to result
             }
             // Result
             if (result.isNull()) {
diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py
index 66bbd05d0d..7530e7daf6 100755
--- a/test/functional/interface_bitcoin_cli.py
+++ b/test/functional/interface_bitcoin_cli.py
@@ -77,7 +77,7 @@ 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])

</p> </details>

utACK 2e7d8b9bf584ab61295795f0521537fc3f1057df. I've left some more style nits inline, but feel free to ignore. I think this is ready for merge as-is.

There's no need to credit reviewers in the commit logs (if we did that everywhere, the commit logs would get very long!)

Thank you @jnewbery, I appreciate your outstanding reviewing and applied your suggestions. I also applied Clang formatting to the PR changeset. Here are the changes since the last push:

<details><summary><code>git diff 2e7d8b9 0af7d3a</code></summary><p>

diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
index a3302835c3..feea064be9 100644
--- a/src/bitcoin-cli.cpp
+++ b/src/bitcoin-cli.cpp
@@ -305,7 +305,7 @@ public:
     }
 };
 
-static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, const std::vector<std::string>& args, const char * const rpcwallet = nullptr)
+static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, const std::vector<std::string>& args, const char* const rpcwallet = nullptr)
 {
     std::string host;
     // In preference order, we choose the following for the port:
@@ -371,9 +371,9 @@ static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, co
     // check if we should use a special wallet endpoint
     std::string endpoint = "/";
     if (rpcwallet) {
-        char *encodedURI = evhttp_uriencode(rpcwallet, strlen(rpcwallet), false);
+        char* encodedURI = evhttp_uriencode(rpcwallet, strlen(rpcwallet), false);
         if (encodedURI) {
-            endpoint = "/wallet/"+ std::string(encodedURI);
+            endpoint = "/wallet/" + std::string(encodedURI);
             free(encodedURI);
         } else {
             throw CConnectionFailed("uri-encode failed");
@@ -426,7 +426,7 @@ static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, co
  * [@returns](/bitcoin-bitcoin/contributor/returns/) the RPC response as a UniValue object.
  * [@throws](/bitcoin-bitcoin/contributor/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 char * const rpcwallet = nullptr)
+static UniValue ConnectAndCallRPC(BaseRequestHandler* rh, const std::string& strMethod, const std::vector<std::string>& args, const char* const rpcwallet = nullptr)
 {
     UniValue response(UniValue::VOBJ);
     // Execute and handle connection failures with -rpcwait.
@@ -441,8 +441,7 @@ static UniValue ConnectAndCallRPC(BaseRequestHandler *rh, const std::string& str
                 }
             }
             break; // Connection succeeded, no need to retry.
-        }
-        catch (const CConnectionFailed&) {
+        } catch (const CConnectionFailed&) {
             if (fWait) {
                 UninterruptibleSleep(std::chrono::milliseconds{1000});
             } else {
@@ -457,26 +456,24 @@ static UniValue ConnectAndCallRPC(BaseRequestHandler *rh, const std::string& str
  * 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](/bitcoin-bitcoin/contributor/param/)[in] result  Reference to UniValue object the wallet names and balances are pushed to.
+ * [@param](/bitcoin-bitcoin/contributor/param/) result  Reference to UniValue object the wallet names and balances are pushed to.
  */
 static void GetWalletBalances(UniValue& result)
 {
-    std::unique_ptr<BaseRequestHandler> rh;
-    rh.reset(new DefaultRequestHandler());
-    const std::vector<std::string> args;
-    const UniValue listwallets = ConnectAndCallRPC(rh.get(), "listwallets", args);
+    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;

-    if (find_value(listwallets, "error").isNull() && wallets.size() > 1) {
-        UniValue balances(UniValue::VOBJ);
-        for (const UniValue& wallet : wallets.getValues()) {
-            const char * const wallet_name = wallet.get_str().c_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);

+    UniValue balances(UniValue::VOBJ);
+    for (const UniValue& wallet : wallets.getValues()) {
+        const char* const wallet_name = wallet.get_str().c_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[])
@@ -545,13 +542,7 @@ static int CommandLineRPC(int argc, char *argv[])
             method = args[0];
             args.erase(args.begin()); // Remove trailing method name from arguments vector
         }
-
-        UniValue reply;
-        if (gArgs.IsArgSet("-rpcwallet")) {
-            reply = ConnectAndCallRPC(rh.get(), method, args, gArgs.GetArg("-rpcwallet", "").c_str());
-        } else {
-            reply = ConnectAndCallRPC(rh.get(), method, args);
-        }
+        const UniValue reply = ConnectAndCallRPC(rh.get(), method, args, gArgs.IsArgSet("-rpcwallet") ? gArgs.GetArg("-rpcwallet", "").c_str() : nullptr);
 
@@ -585,12 +576,10 @@ static int CommandLineRPC(int argc, char *argv[])
                 strPrint = result.write(2);
             }
         }
-    }
-    catch (const std::exception& e) {
+    } catch (const std::exception& e) {
         strPrint = std::string("error: ") + e.what();
         nRet = EXIT_FAILURE;
-    }
-    catch (...) {
+    } catch (...) {
         PrintExceptionContinue(nullptr, "CommandLineRPC()");
         throw;
     }

</p></details>

@jonatack thanks for being so responsive to review, and well done for catching the nasty bug in my previous suggestion! (https://github.com/bitcoin/bitcoin/pull/18594#discussion_r428327325).

I have one final suggestion: switch out the c-style char * arguments for Optional<std::string>s. Using std::string seems better in almost all cases.

Thank you @jnewbery and @luke-jr for your excellent reviews and suggestions. I'll take more heed henceforth of the developer notes' warning about c_str(). Updated to use Optional<std::string> as per the following diff. Aside from a release note if merged, this PR should hopefully be ready.

<details><summary><code>git diff 0af7d3a 5edad5c</code></summary><p>

diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
 #include <clientversion.h>
+#include <optional.h>
 #include <rpc/client.h>
@@ -17,7 +18,6 @@
 #include <util/url.h>
 
-#include <cstring>
 #include <functional>
@@ -305,7 +305,7 @@ public:
     }
 };
 
-static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, const std::vector<std::string>& args, const char* const rpcwallet = nullptr)
+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:
@@ -371,7 +371,7 @@ static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, co
     // check if we should use a special wallet endpoint
     std::string endpoint = "/";
     if (rpcwallet) {
-        char* encodedURI = evhttp_uriencode(rpcwallet, strlen(rpcwallet), false);
+        char* encodedURI = evhttp_uriencode(rpcwallet->data(), rpcwallet->size(), false);
         if (encodedURI) {
             endpoint = "/wallet/" + std::string(encodedURI);
             free(encodedURI);
@@ -422,11 +422,11 @@ static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, co
  *
  * [@param](/bitcoin-bitcoin/contributor/param/)[in] rh         Pointer to RequestHandler.
  * [@param](/bitcoin-bitcoin/contributor/param/)[in] strMethod  Reference to const string method to forward to CallRPC.
- * [@param](/bitcoin-bitcoin/contributor/param/)[in] rpcwallet  Pointer to const c-string rpcwallet arg to forward to CallRPC.
+ * [@param](/bitcoin-bitcoin/contributor/param/)[in] rpcwallet  Reference to const optional string wallet name to forward to CallRPC.
  * [@returns](/bitcoin-bitcoin/contributor/returns/) the RPC response as a UniValue object.
  * [@throws](/bitcoin-bitcoin/contributor/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 char* const rpcwallet = nullptr)
+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.
@@ -468,7 +468,7 @@ static void GetWalletBalances(UniValue& result)
 
     UniValue balances(UniValue::VOBJ);
     for (const UniValue& wallet : wallets.getValues()) {
-        const char* const wallet_name = wallet.get_str().c_str();
+        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);
@@ -542,7 +542,9 @@ static int CommandLineRPC(int argc, char *argv[])
             method = args[0];
             args.erase(args.begin()); // Remove trailing method name from arguments vector
         }
-        const UniValue reply = ConnectAndCallRPC(rh.get(), method, args, gArgs.IsArgSet("-rpcwallet") ? gArgs.GetArg("-rpcwallet", "").c_str() : nullptr);
+        Optional<std::string> wallet_name{};
+        if (gArgs.IsArgSet("-rpcwallet")) wallet_name = gArgs.GetArg("-rpcwallet", "");
+        const UniValue reply = ConnectAndCallRPC(rh.get(), method, args, wallet_name);

</p></details>

Tested ACK 5edad5ce5d3f15b694bf3fad0300c6446674b554.

Does it make sense sum up balances and always have the balance?

@promag why not (for a follow-up):

  "balance": 0.00003714,
  "balances": {
    "": 0.00001000,
    "Encrypted": 0.00003500,
    "day-to-day": 0.00000120,
    "side project": 0.00000094
  }
}

What I miss with -getinfo in single-wallet mode is showing the wallet name. At some point a common interface like the above for both modes might be good, to always see the wallet name even when there is only one wallet loaded.

utACK 5edad5ce5d3f15b694bf3fad0300c6446674b554

Thanks for being patient with my iterative reviews!

Thanks @jnewbery, @promag and @luke-jr for reviewing. I'll add a commit to the release note follow-up that tightens up the -getinfo command parsing and adds a test as per #18594 (review).

concept ACK

Code review ACK 5edad5ce5d3f15b694bf3fad0300c6446674b554

Agree that a release note followup which can include a few other things would be good, this is RTM as is.

Agree that a release note followup which can include a few other things would be good

Thanks -- done in #19089 and #19354.