From 341b5a9714200b69cef3d66056605bcefef93e54 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Fri, 13 Jul 2018 19:40:31 -0700 Subject: [PATCH] Merge #13566: Fix get balance 702ae1e21a [RPC] [wallet] allow getbalance to use min_conf and watch_only without accounts. (John Newbery) cf15761f6d [wallet] GetBalance can take a min_depth argument. (John Newbery) 0f3d6e9ab7 [wallet] factor out GetAvailableWatchOnlyBalance() (John Newbery) 7110c830f8 [wallet] deduplicate GetAvailableCredit logic (John Newbery) ef7bc8893c [wallet] Factor out GetWatchOnlyBalance() (John Newbery) 4279da4785 [wallet] GetBalance can take an isminefilter filter. (John Newbery) Pull request description: #12953 inadvertently removed the functionality to call `getbalance "*" ` to get the wallet's balance with either minconfs or include_watchonly. This restores that functionality (when `-deprecatedrpc=accounts`), and also makes it possible to call ``getbalance minconf= include_watchonly=` when accounts are not being used. Tree-SHA512: 67e84de9291ed6d34b23c626f4dc5988ba0ae6c99708d02b87dd3aaad3f4b6baa6202a66cc2dadd30dd993a39de8036ee920fcaa8cbb1c5dfe606e6fac183344 --- src/interfaces/wallet.cpp | 2 +- src/wallet/rpcwallet.cpp | 77 ++++++++++++++++++-------------- src/wallet/wallet.cpp | 78 +++++++++++---------------------- src/wallet/wallet.h | 6 +-- test/functional/wallet_basic.py | 9 ++++ 5 files changed, 80 insertions(+), 92 deletions(-) diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 4dd73698a2..9f2aad3b92 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -382,7 +382,7 @@ public: result.anonymized_balance = m_wallet.GetAnonymizedBalance(); result.have_watch_only = m_wallet.HaveWatchOnly(); if (result.have_watch_only) { - result.watch_only_balance = m_wallet.GetWatchOnlyBalance(); + result.watch_only_balance = m_wallet.GetBalance(ISMINE_WATCH_ONLY); result.unconfirmed_watch_only_balance = m_wallet.GetUnconfirmedWatchOnlyBalance(); result.immature_watch_only_balance = m_wallet.GetImmatureWatchOnlyBalance(); } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 8bf469c007..ae8d9f7a4f 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -868,8 +868,9 @@ UniValue getbalance(const JSONRPCRequest& request) return NullUniValue; } - if (request.fHelp || (request.params.size() > 4 && IsDeprecatedRPCEnabled("accounts")) || (request.params.size() != 0 && !IsDeprecatedRPCEnabled("accounts"))) + if (request.fHelp || (request.params.size() > 4)) throw std::runtime_error( + (IsDeprecatedRPCEnabled("accounts") ? std::string( "getbalance ( \"account\" minconf addlocked include_watchonly )\n" "\nIf account is not specified, returns the server's total available balance.\n" "The available balance is what the wallet considers currently spendable, and is\n" @@ -881,9 +882,18 @@ UniValue getbalance(const JSONRPCRequest& request) "1. \"account\" (string, optional) DEPRECATED. This argument will be removed in V0.18. \n" " To use this deprecated argument, start dashd with -deprecatedrpc=accounts.\n" " The selected account, or \"*\" for entire wallet. It may be the default account using \"\".\n" - "2. minconf (numeric, optional, default=1) DEPRECATED. Only valid when an account is specified. This argument will be removed in V0.18. To use this deprecated argument, start dashd with -deprecatedrpc=accounts. Only include transactions confirmed at least this many times.\n" - "3. addlocked (bool, optional, default=false) Whether to include the value of transactions locked via InstantSend in the wallet's balance.\n" - "4. include_watchonly (bool, optional, default=false) DEPRECATED. Only valid when an account is specified. This argument will be removed in V0.18. To use this deprecated argument, start dashd with -deprecatedrpc=accounts. Also include balance in watch-only addresses (see 'importaddress')\n" + "2. minconf (numeric, optional) Only include transactions confirmed at least this many times.\n" + " The default is 1 if an account is provided or 0 if no account is provided\n") + : std::string( + "getbalance ( \"(dummy)\" minconf addlocked include_watchonly )\n" + "\nReturns the total available balance.\n" + "The available balance is what the wallet considers currently spendable, and is\n" + "thus affected by options which limit spendability such as -spendzeroconfchange.\n" + "\nArguments:\n" + "1. (dummy) (string, optional) Remains for backward compatibility. Must be excluded or set to \"*\".\n" + "2. minconf (numeric, optional, default=0) Only include transactions confirmed at least this many times.\n")) + + "3. addlocked (bool, optional, default=false) Whether to include the value of transactions locked via InstantSend in the wallet's balance.\n" + "4. include_watchonly (bool, optional, default=false) Also include balance in watch-only addresses (see 'importaddress')\n" "\nResult:\n" "amount (numeric) The total amount in " + CURRENCY_UNIT + " received for this account.\n" "\nExamples:\n" @@ -901,42 +911,41 @@ UniValue getbalance(const JSONRPCRequest& request) LOCK2(cs_main, pwallet->cs_wallet); - if (IsDeprecatedRPCEnabled("accounts")) { - const UniValue& account_value = request.params[0]; - const UniValue& minconf = request.params[1]; - const UniValue& addlocked = request.params[2]; - const UniValue& include_watchonly = request.params[3]; + const UniValue& account_value = request.params[0]; - if (account_value.isNull()) { - if (!minconf.isNull()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, - "getbalance minconf option is only currently supported if an account is specified"); - } - if (!include_watchonly.isNull()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, - "getbalance include_watchonly option is only currently supported if an account is specified"); - } - return ValueFromAmount(pwallet->GetBalance()); - } + int min_depth = 0; + if (IsDeprecatedRPCEnabled("accounts") && !account_value.isNull()) { + // Default min_depth to 1 when an account is provided. + min_depth = 1; + } + if (!request.params[1].isNull()) { + min_depth = request.params[1].get_int(); + } + + const UniValue& addlocked = request.params[2]; + bool fAddLocked = false; + if (!addlocked.isNull()) { + fAddLocked = addlocked.get_bool(); + } + + isminefilter filter = ISMINE_SPENDABLE; + if (!request.params[3].isNull() && request.params[3].get_bool()) { + filter = filter | ISMINE_WATCH_ONLY; + } + + if (!account_value.isNull()) { const std::string& account_param = account_value.get_str(); const std::string* account = account_param != "*" ? &account_param : nullptr; - int nMinDepth = 1; - if (!minconf.isNull()) - nMinDepth = minconf.get_int(); - bool fAddLocked = false; - if (!addlocked.isNull()) - fAddLocked = addlocked.get_bool(); - isminefilter filter = ISMINE_SPENDABLE; - if(!include_watchonly.isNull()) - if(include_watchonly.get_bool()) - filter = filter | ISMINE_WATCH_ONLY; - - return ValueFromAmount(pwallet->GetLegacyBalance(filter, nMinDepth, account, fAddLocked)); + if (!IsDeprecatedRPCEnabled("accounts") && account_param != "*") { + throw JSONRPCError(RPC_METHOD_DEPRECATED, "dummy first argument must be excluded or set to \"*\"."); + } else if (IsDeprecatedRPCEnabled("accounts")) { + return ValueFromAmount(pwallet->GetLegacyBalance(filter, min_depth, account, fAddLocked)); + } } - return ValueFromAmount(pwallet->GetBalance()); + return ValueFromAmount(pwallet->GetBalance(filter, min_depth)); } UniValue getunconfirmedbalance(const JSONRPCRequest &request) @@ -4107,7 +4116,7 @@ static const CRPCCommand commands[] = { "wallet", "dumpwallet", &dumpwallet, {"filename"} }, { "wallet", "encryptwallet", &encryptwallet, {"passphrase"} }, { "wallet", "getaddressinfo", &getaddressinfo, {"address"} }, - { "wallet", "getbalance", &getbalance, {"account","minconf","addlocked","include_watchonly"} }, + { "wallet", "getbalance", &getbalance, {"account|dummy","minconf","addlocked","include_watchonly"} }, { "wallet", "getnewaddress", &getnewaddress, {"label|account"} }, { "wallet", "getrawchangeaddress", &getrawchangeaddress, {} }, { "wallet", "getreceivedbyaddress", &getreceivedbyaddress, {"address","minconf","addlocked"} }, diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 9922f38e0d..07593e25e1 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2333,7 +2333,7 @@ CAmount CWalletTx::GetImmatureCredit(bool fUseCache) const return 0; } -CAmount CWalletTx::GetAvailableCredit(bool fUseCache) const +CAmount CWalletTx::GetAvailableCredit(bool fUseCache, const isminefilter& filter) const { if (pwallet == nullptr) return 0; @@ -2342,8 +2342,20 @@ CAmount CWalletTx::GetAvailableCredit(bool fUseCache) const if (IsCoinBase() && GetBlocksToMaturity() > 0) return 0; - if (fUseCache && fAvailableCreditCached) - return nAvailableCreditCached; + CAmount* cache = nullptr; + bool* cache_used = nullptr; + + if (filter == ISMINE_SPENDABLE) { + cache = &nAvailableCreditCached; + cache_used = &fAvailableCreditCached; + } else if (filter == ISMINE_WATCH_ONLY) { + cache = &nAvailableWatchCreditCached; + cache_used = &fAvailableWatchCreditCached; + } + + if (fUseCache && cache_used && *cache_used) { + return *cache; + } CAmount nCredit = 0; uint256 hashTx = GetHash(); @@ -2352,14 +2364,16 @@ CAmount CWalletTx::GetAvailableCredit(bool fUseCache) const if (!pwallet->IsSpent(hashTx, i)) { const CTxOut &txout = tx->vout[i]; - nCredit += pwallet->GetCredit(txout, ISMINE_SPENDABLE); + nCredit += pwallet->GetCredit(txout, filter); if (!MoneyRange(nCredit)) throw std::runtime_error(std::string(__func__) + ": value out of range"); } } - nAvailableCreditCached = nCredit; - fAvailableCreditCached = true; + if (cache) { + *cache = nCredit; + *cache_used = true; + } return nCredit; } @@ -2377,35 +2391,6 @@ CAmount CWalletTx::GetImmatureWatchOnlyCredit(const bool fUseCache) const return 0; } -CAmount CWalletTx::GetAvailableWatchOnlyCredit(const bool fUseCache) const -{ - if (pwallet == nullptr) - return 0; - - // Must wait until coinbase is safely deep enough in the chain before valuing it - if (IsCoinBase() && GetBlocksToMaturity() > 0) - return 0; - - if (fUseCache && fAvailableWatchCreditCached) - return nAvailableWatchCreditCached; - - CAmount nCredit = 0; - for (unsigned int i = 0; i < tx->vout.size(); i++) - { - if (!pwallet->IsSpent(GetHash(), i)) - { - const CTxOut &txout = tx->vout[i]; - nCredit += pwallet->GetCredit(txout, ISMINE_WATCH_ONLY); - if (!MoneyRange(nCredit)) - throw std::runtime_error(std::string(__func__) + ": value out of range"); - } - } - - nAvailableWatchCreditCached = nCredit; - fAvailableWatchCreditCached = true; - return nCredit; -} - CAmount CWalletTx::GetAnonymizedCredit(const CCoinControl* coinControl) const { if (!pwallet) @@ -2627,14 +2612,15 @@ std::unordered_set CWallet::GetSpendableTXs() return ret; } -CAmount CWallet::GetBalance() const +CAmount CWallet::GetBalance(const isminefilter& filter, const int min_depth) const { CAmount nTotal = 0; { LOCK2(cs_main, cs_wallet); for (auto pcoin : GetSpendableTXs()) { - if (pcoin->IsTrusted()) - nTotal += pcoin->GetAvailableCredit(); + if (pcoin->IsTrusted() && pcoin->GetDepthInMainChain() >= min_depth) { + nTotal += pcoin->GetAvailableCredit(true, filter); + } } } @@ -2764,20 +2750,6 @@ CAmount CWallet::GetImmatureBalance() const return nTotal; } -CAmount CWallet::GetWatchOnlyBalance() const -{ - CAmount nTotal = 0; - { - LOCK2(cs_main, cs_wallet); - for (auto pcoin : GetSpendableTXs()) { - if (pcoin->IsTrusted()) - nTotal += pcoin->GetAvailableWatchOnlyCredit(); - } - } - - return nTotal; -} - CAmount CWallet::GetUnconfirmedWatchOnlyBalance() const { CAmount nTotal = 0; @@ -2785,7 +2757,7 @@ CAmount CWallet::GetUnconfirmedWatchOnlyBalance() const LOCK2(cs_main, cs_wallet); for (auto pcoin : GetSpendableTXs()) { if (!pcoin->IsTrusted() && pcoin->GetDepthInMainChain() == 0 && !pcoin->IsLockedByInstantSend() && pcoin->InMempool()) - nTotal += pcoin->GetAvailableWatchOnlyCredit(); + nTotal += pcoin->GetAvailableCredit(true, ISMINE_WATCH_ONLY); } } return nTotal; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 55c34bb183..86a6b6366f 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -474,9 +474,8 @@ public: CAmount GetDebit(const isminefilter& filter) const; CAmount GetCredit(const isminefilter& filter) const; CAmount GetImmatureCredit(bool fUseCache=true) const; - CAmount GetAvailableCredit(bool fUseCache=true) const; + CAmount GetAvailableCredit(bool fUseCache=true, const isminefilter& filter=ISMINE_SPENDABLE) const; CAmount GetImmatureWatchOnlyCredit(const bool fUseCache=true) const; - CAmount GetAvailableWatchOnlyCredit(const bool fUseCache=true) const; CAmount GetChange() const; CAmount GetAnonymizedCredit(const CCoinControl* coinControl = nullptr) const; @@ -1047,10 +1046,9 @@ public: void ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman) override; // ResendWalletTransactionsBefore may only be called if fBroadcastTransactions! std::vector ResendWalletTransactionsBefore(int64_t nTime, CConnman* connman); - CAmount GetBalance() const; + CAmount GetBalance(const isminefilter& filter=ISMINE_SPENDABLE, const int min_depth=0) const; CAmount GetUnconfirmedBalance() const; CAmount GetImmatureBalance() const; - CAmount GetWatchOnlyBalance() const; CAmount GetUnconfirmedWatchOnlyBalance() const; CAmount GetImmatureWatchOnlyBalance() const; CAmount GetLegacyBalance(const isminefilter& filter, int minDepth, const std::string* account, const bool fAddLocked) const; diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 36ee2b4aa2..7c0f5c58c6 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -52,6 +52,15 @@ class WalletTest(BitcoinTestFramework): assert_equal(self.nodes[1].getbalance(), 500) assert_equal(self.nodes[2].getbalance(), 0) + # Check getbalance with different arguments + assert_equal(self.nodes[0].getbalance("*"), 500) + assert_equal(self.nodes[0].getbalance("*", 1), 500) + assert_equal(self.nodes[0].getbalance("*", 1, True), 500) + assert_equal(self.nodes[0].getbalance(minconf=1), 500) + + # first argument of getbalance must be excluded or set to "*" + assert_raises_rpc_error(-32, "dummy first argument must be excluded or set to \"*\"", self.nodes[0].getbalance, "") + # Check that only first and second nodes have UTXOs utxos = self.nodes[0].listunspent() assert_equal(len(utxos), 1)