diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -315,7 +315,8 @@ result.immature_balance = m_wallet.GetImmatureBalance(); 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 = diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1018,61 +1018,69 @@ return NullUniValue; } - if (request.fHelp || - (request.params.size() > 3 && - IsDeprecatedRPCEnabled(gArgs, "accounts")) || - (request.params.size() != 0 && - !IsDeprecatedRPCEnabled(gArgs, "accounts"))) { + if (request.fHelp || (request.params.size() > 3)) { throw std::runtime_error( - "getbalance ( \"account\" minconf 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,\n" - "and is thus affected by options which limit spendability such as " - "-spendzeroconfchange.\n" - "If account is specified (DEPRECATED), returns the balance in the " - "account.\n" - "Note that the account \"\" is not the same as leaving the " - "parameter out.\n" - "The server total may be different to the balance in the default " - "\"\" account.\n" - "\nArguments:\n" - "1. \"account\" (string, optional) DEPRECATED. This " - "argument will be removed in v0.21. \n" - " To use this deprecated argument, start " - "bitcoind with -deprecatedrpc=accounts. The account " - "string may be given as a\n" - " specific account name to find the balance " - "associated with wallet keys in\n" - " a named account, or as the empty string " - "(\"\") to find the balance\n" - " associated with wallet keys not in any named " - "account, or as \"*\" to find\n" - " the balance associated with all wallet keys " - "regardless of account.\n" - " When this option is specified, it calculates " - "the balance in a different\n" - " way than when it is not specified, and which " - "can count spends twice when\n" - " there are conflicting pending transactions " - "temporarily resulting in low\n" - " or even negative balances.\n" - " In general, account balance calculation is " - "not considered reliable and\n" - " has resulted in confusing outcomes, so it is " - "recommended to avoid passing\n" - " this argument.\n" - "2. minconf (numeric, optional, default=1) DEPRECATED. " - "Only valid when an account is specified. This argument will be " - "removed in V0.21. To use this deprecated argument, start bitcoind " - "with -deprecatedrpc=accounts. Only include transactions confirmed " - "at least this many times.\n" - "3. include_watchonly (bool, optional, default=false) DEPRECATED. " - "Only valid when an account is specified. This argument will be " - "removed in V0.21. To use this deprecated argument, start bitcoind " - "with -deprecatedrpc=accounts. Also include balance in watch-only " - "addresses (see 'importaddress')\n" + (IsDeprecatedRPCEnabled(gArgs, "accounts") + ? std::string( + "getbalance ( \"account\" minconf 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,\nand is thus affected by options " + "which limit spendability such as " + "-spendzeroconfchange.\n" + "If account is specified (DEPRECATED), returns the " + "balance in the account.\n" + "Note that the account \"\" is not the same as leaving " + "the parameter out.\nThe server total may be different " + "to the balance in the default " + "\"\" account.\n" + "\nArguments:\n" + "1. \"account\" (string, optional) DEPRECATED. " + "This argument will be removed in v0.21. \n" + " To use this deprecated argument, " + "start bitcoind with -deprecatedrpc=accounts. The " + "account string may be given as a\n" + " specific account name to find the " + "balance associated with wallet keys in\n" + " a named account, or as the empty " + "string (\"\") to find the balance\n" + " associated with wallet keys not " + "in any named account, or as \"*\" to find\n" + " the balance associated with all " + "wallet keys regardless of account.\n" + " When this option is specified, it " + "calculates the balance in a different\n" + " way than when it is not " + "specified, and which can count spends twice when\n" + " there are conflicting pending " + "transactions temporarily resulting in low\n" + " or even negative balances.\n" + " In general, account balance " + "calculation is not considered reliable and\n" + " has resulted in confusing " + "outcomes, so it is recommended to avoid passing\n" + " this argument.\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 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. 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 + @@ -1091,45 +1099,39 @@ LOCK2(cs_main, pwallet->cs_wallet); - if (IsDeprecatedRPCEnabled(gArgs, "accounts")) { - const UniValue &account_value = request.params[0]; - const UniValue &minconf = request.params[1]; - const UniValue &include_watchonly = request.params[2]; + 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(gArgs, "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(); + } + + isminefilter filter = ISMINE_SPENDABLE; + if (!request.params[2].isNull() && request.params[2].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(); - } - isminefilter filter = ISMINE_SPENDABLE; - if (!include_watchonly.isNull() && include_watchonly.get_bool()) { - filter = filter | ISMINE_WATCH_ONLY; + if (!IsDeprecatedRPCEnabled(gArgs, "accounts") && + account_param != "*") { + throw JSONRPCError( + RPC_METHOD_DEPRECATED, + "dummy first argument must be excluded or set to \"*\"."); + } else if (IsDeprecatedRPCEnabled(gArgs, "accounts")) { + return ValueFromAmount( + pwallet->GetLegacyBalance(filter, min_depth, account)); } - - return ValueFromAmount( - pwallet->GetLegacyBalance(filter, nMinDepth, account)); } - return ValueFromAmount(pwallet->GetBalance()); + return ValueFromAmount(pwallet->GetBalance(filter, min_depth)); } static UniValue getunconfirmedbalance(const Config &config, @@ -5399,7 +5401,7 @@ { "wallet", "createwallet", createwallet, {"wallet_name", "disable_private_keys"} }, { "wallet", "encryptwallet", encryptwallet, {"passphrase"} }, { "wallet", "getaddressinfo", getaddressinfo, {"address"} }, - { "wallet", "getbalance", getbalance, {"account","minconf","include_watchonly"} }, + { "wallet", "getbalance", getbalance, {"account|dummy","minconf","include_watchonly"} }, { "wallet", "getnewaddress", getnewaddress, {"label|account", "address_type"} }, { "wallet", "getrawchangeaddress", getrawchangeaddress, {"address_type"} }, { "wallet", "getreceivedbyaddress", getreceivedbyaddress, {"address","minconf"} }, diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -473,12 +473,12 @@ EXCLUSIVE_LOCKS_REQUIRED(cs_main); Amount GetImmatureCredit(bool fUseCache = true) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); - Amount GetAvailableCredit(bool fUseCache = true) const + Amount + GetAvailableCredit(bool fUseCache = true, + const isminefilter &filter = ISMINE_SPENDABLE) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); Amount GetImmatureWatchOnlyCredit(const bool fUseCache = true) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); - Amount GetAvailableWatchOnlyCredit(const bool fUseCache = true) const - EXCLUSIVE_LOCKS_REQUIRED(cs_main); Amount GetChange() const; // Get the marginal bytes if spending the specified output from this @@ -1072,10 +1072,10 @@ std::vector ResendWalletTransactionsBefore(int64_t nTime, CConnman *connman) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - Amount GetBalance() const; + Amount GetBalance(const isminefilter &filter = ISMINE_SPENDABLE, + const int min_depth = 0) const; Amount GetUnconfirmedBalance() const; Amount GetImmatureBalance() const; - Amount GetWatchOnlyBalance() const; Amount GetUnconfirmedWatchOnlyBalance() const; Amount GetImmatureWatchOnlyBalance() const; Amount GetLegacyBalance(const isminefilter &filter, int minDepth, diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2081,7 +2081,8 @@ return Amount::zero(); } -Amount CWalletTx::GetAvailableCredit(bool fUseCache) const { +Amount CWalletTx::GetAvailableCredit(bool fUseCache, + const isminefilter &filter) const { if (pwallet == nullptr) { return Amount::zero(); } @@ -2092,8 +2093,19 @@ return Amount::zero(); } - if (fUseCache && fAvailableCreditCached) { - return nAvailableCreditCached; + Amount *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; } Amount nCredit = Amount::zero(); @@ -2101,7 +2113,7 @@ for (uint32_t i = 0; i < tx->vout.size(); i++) { if (!pwallet->IsSpent(COutPoint(txid, 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"); @@ -2109,8 +2121,10 @@ } } - nAvailableCreditCached = nCredit; - fAvailableCreditCached = true; + if (cache) { + *cache = nCredit; + *cache_used = true; + } return nCredit; } @@ -2128,39 +2142,6 @@ return Amount::zero(); } -Amount CWalletTx::GetAvailableWatchOnlyCredit(const bool fUseCache) const { - if (pwallet == nullptr) { - return Amount::zero(); - } - - // Must wait until coinbase is safely deep enough in the chain before - // valuing it. - if (IsCoinBase() && GetBlocksToMaturity() > 0) { - return Amount::zero(); - } - - if (fUseCache && fAvailableWatchCreditCached) { - return nAvailableWatchCreditCached; - } - - Amount nCredit = Amount::zero(); - const TxId &txid = GetId(); - for (uint32_t i = 0; i < tx->vout.size(); i++) { - if (!pwallet->IsSpent(COutPoint(txid, 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; -} - Amount CWalletTx::GetChange() const { if (fChangeCached) { return nChangeCached; @@ -2298,14 +2279,15 @@ * * @{ */ -Amount CWallet::GetBalance() const { +Amount CWallet::GetBalance(const isminefilter &filter, + const int min_depth) const { LOCK2(cs_main, cs_wallet); Amount nTotal = Amount::zero(); for (const auto &entry : mapWallet) { const CWalletTx *pcoin = &entry.second; - if (pcoin->IsTrusted()) { - nTotal += pcoin->GetAvailableCredit(); + if (pcoin->IsTrusted() && pcoin->GetDepthInMainChain() >= min_depth) { + nTotal += pcoin->GetAvailableCredit(true, filter); } } @@ -2339,20 +2321,6 @@ return nTotal; } -Amount CWallet::GetWatchOnlyBalance() const { - LOCK2(cs_main, cs_wallet); - - Amount nTotal = Amount::zero(); - for (const auto &entry : mapWallet) { - const CWalletTx *pcoin = &entry.second; - if (pcoin->IsTrusted()) { - nTotal += pcoin->GetAvailableWatchOnlyCredit(); - } - } - - return nTotal; -} - Amount CWallet::GetUnconfirmedWatchOnlyBalance() const { LOCK2(cs_main, cs_wallet); @@ -2361,7 +2329,7 @@ const CWalletTx *pcoin = &entry.second; if (!pcoin->IsTrusted() && pcoin->GetDepthInMainChain() == 0 && pcoin->InMempool()) { - nTotal += pcoin->GetAvailableWatchOnlyCredit(); + nTotal += pcoin->GetAvailableCredit(true, ISMINE_WATCH_ONLY); } } diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -64,6 +64,16 @@ assert_equal(self.nodes[1].getbalance(), 50) assert_equal(self.nodes[2].getbalance(), 0) + # Check getbalance with different arguments + assert_equal(self.nodes[0].getbalance("*"), 50) + assert_equal(self.nodes[0].getbalance("*", 1), 50) + assert_equal(self.nodes[0].getbalance("*", 1, True), 50) + assert_equal(self.nodes[0].getbalance(minconf=1), 50) + + # 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)