From ca0dad0a8c16b8bc4ae3e0ea7a46adaad5a2f0a5 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 14 Jan 2022 14:40:10 +0000 Subject: [PATCH 1/3] wallet: Implement `z_getbalanceforaccount` Closes zcash/zcash#5183. --- qa/rpc-tests/wallet_accounts.py | 33 +++++++++++++++++++++++ src/wallet/rpcwallet.cpp | 48 +++++++++++++++++++++++++++------ src/wallet/wallet.cpp | 16 +++++++++++ src/wallet/wallet.h | 14 ++++++++++ 4 files changed, 103 insertions(+), 8 deletions(-) diff --git a/qa/rpc-tests/wallet_accounts.py b/qa/rpc-tests/wallet_accounts.py index f693599c0..e8854ffef 100755 --- a/qa/rpc-tests/wallet_accounts.py +++ b/qa/rpc-tests/wallet_accounts.py @@ -4,6 +4,7 @@ # file COPYING or https://www.opensource.org/licenses/mit-license.php . from test_framework.authproxy import JSONRPCException +from test_framework.mininode import COIN from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -27,6 +28,18 @@ class WalletAccountsTest(BitcoinTestFramework): actual = self.nodes[0].z_listunifiedreceivers(ua) assert_equal(set(expected), set(actual)) + # Check we only have balances in the expected pools. + # Remember that empty pools are omitted from the output. + def check_account_balance(self, account, expected, minconf=None): + if minconf is None: + actual = self.nodes[0].z_getbalanceforaccount(account) + else: + actual = self.nodes[0].z_getbalanceforaccount(account, minconf) + assert_equal(set(expected), set(actual['pools'])) + for pool in expected: + assert_equal(expected[pool] * COIN, actual['pools'][pool]['valueZat']) + assert_equal(actual['minimum_confirmations'], 1 if minconf is None else minconf) + def run_test(self): # With a new wallet, the first account will be 0. account0 = self.nodes[0].z_getnewaccount() @@ -65,6 +78,10 @@ class WalletAccountsTest(BitcoinTestFramework): self.check_receiver_types(ua0, ['transparent', 'sapling']) self.check_receiver_types(ua1, ['transparent', 'sapling']) + # The balances of the accounts are all zero. + self.check_account_balance(0, {}) + self.check_account_balance(1, {}) + # Manually send funds to one of the receivers in the UA. # TODO: Once z_sendmany supports UAs, receive to the UA instead of the receiver. sapling0 = self.nodes[0].z_listunifiedreceivers(ua0)['sapling'] @@ -78,10 +95,18 @@ class WalletAccountsTest(BitcoinTestFramework): assert_equal(tx_details['outputs'][0]['type'], 'sapling') assert_equal(tx_details['outputs'][0]['address'], ua0) + # The new balance should not be visible with the default minconf, but should be + # visible with minconf=0. self.sync_all() + self.check_account_balance(0, {}) + self.check_account_balance(0, {'sapling': 10}, 0) + self.nodes[2].generate(1) self.sync_all() + # The default minconf should now detect the balance. + self.check_account_balance(0, {'sapling': 10}) + # Manually send funds from the UA receiver. # TODO: Once z_sendmany supports UAs, send from the UA instead of the receiver. node1sapling = self.nodes[1].z_getnewaddress('sapling') @@ -95,6 +120,14 @@ class WalletAccountsTest(BitcoinTestFramework): assert_equal(tx_details['spends'][0]['type'], 'sapling') assert_equal(tx_details['spends'][0]['address'], ua0) + # The balances of the account should reflect whether zero-conf transactions are + # being considered. We will show either 0 (because the spent 10-ZEC note is never + # shown, as that transaction has been created and broadcast, and _might_ get mined + # up until the transaction expires), or 9 (if we include the unmined transaction). + self.sync_all() + self.check_account_balance(0, {}) + self.check_account_balance(0, {'sapling': 9}, 0) + if __name__ == '__main__': WalletAccountsTest().main() diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index b645d6bd9..ad46851c3 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3679,9 +3679,6 @@ UniValue z_getbalanceforaccount(const UniValue& params, bool fHelp) " \"transparent\": {\n" " \"valueZat\": amount (numeric) The amount held in the transparent pool by this account\n" " \"},\n" - " \"sprout\": {\n" - " \"valueZat\": amount (numeric) The amount held in the sprout pool by this account\n" - " \"},\n" " \"sapling\": {\n" " \"valueZat\": amount (numeric) The amount held in the sapling pool by this account\n" " \"},\n" @@ -3705,7 +3702,13 @@ UniValue z_getbalanceforaccount(const UniValue& params, bool fHelp) if (!fExperimentalOrchardWallet) { throw JSONRPCError(RPC_WALLET_ENCRYPTION_FAILED, "Error: the Orchard wallet experimental extensions are disabled."); } - int64_t account = params[0].get_int64(); + + int64_t accountInt = params[0].get_int64(); + if (accountInt < 0 || accountInt >= ZCASH_LEGACY_ACCOUNT) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid account number, must be 0 <= account <= (2^31)-2."); + } + libzcash::AccountId account = accountInt; + int minconf = 1; if (params.size() > 1) { minconf = params[1].get_int(); @@ -3713,11 +3716,40 @@ UniValue z_getbalanceforaccount(const UniValue& params, bool fHelp) throw JSONRPCError(RPC_INVALID_PARAMETER, "Minimum number of confirmations cannot be less than 0"); } } + + LOCK(pwalletMain->cs_wallet); + + // Get the receivers for this account. + auto selector = pwalletMain->ZTXOSelectorForAccount(account, false); + if (!selector.has_value()) { + throw JSONRPCError( + RPC_INVALID_PARAMETER, + tfm::format("Error: account %d has not been generated by z_getnewaccount.", account)); + } + + auto spendableInputs = pwalletMain->FindSpendableInputs(selector.value(), true, minconf); + // Accounts never contain Sprout notes. + assert(spendableInputs.sproutNoteEntries.empty()); + + CAmount transparentBalance = 0; + CAmount saplingBalance = 0; + for (const auto& t : spendableInputs.utxos) { + transparentBalance += t.Value(); + } + for (const auto& t : spendableInputs.saplingNoteEntries) { + saplingBalance += t.note.value(); + } + UniValue pools(UniValue::VOBJ); - pools.pushKV("transparent", 99999.99); - pools.pushKV("sprout", 99999.99); - pools.pushKV("sapling", 99999.99); - pools.pushKV("orchard", 99999.99); + auto renderBalance = [&](std::string poolName, CAmount balance) { + if (balance > 0) { + UniValue pool(UniValue::VOBJ); + pool.pushKV("valueZat", balance); + pools.pushKV(poolName, pool); + } + }; + renderBalance("transparent", transparentBalance); + renderBalance("sapling", saplingBalance); UniValue result(UniValue::VOBJ); result.pushKV("pools", pools); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index aa216f994..ff5f840f2 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1363,6 +1363,22 @@ void CWallet::SyncMetaData(pair::iterator, typename TxSpe // Zcash transaction output selectors // +std::optional CWallet::ZTXOSelectorForAccount( + libzcash::AccountId account, + bool requireSpendingKey, + std::set receiverTypes) const +{ + if (mnemonicHDChain.has_value() && + mapUnifiedAccountKeys.count( + std::make_pair(mnemonicHDChain.value().GetSeedFingerprint(), account) + ) > 0) + { + return ZTXOSelector(AccountZTXOPattern(account, receiverTypes), requireSpendingKey); + } else { + return std::nullopt; + } +} + std::optional CWallet::ToZTXOSelector(const libzcash::PaymentAddress& addr, bool requireSpendingKey) const { auto self = this; std::optional pattern = std::nullopt; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 2dcb9e5b2..b70f799f6 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1222,6 +1222,20 @@ public: */ static bool SelectCoinsMinConf(const CAmount& nTargetValue, int nConfMine, int nConfTheirs, std::vector vCoins, std::set >& setCoinsRet, CAmount& nValueRet); + /** + * Obtain the ZTXO selector for the specified account ID. + * + * Returns `std::nullopt` if the account ID has not been generated yet by + * the wallet. + * + * If the `requireSpendingKey` flag is set, this will only return a selector + * that will choose outputs for which this wallet holds the spending keys. + */ + std::optional ZTXOSelectorForAccount( + libzcash::AccountId account, + bool requireSpendingKey, + std::set receiverTypes={}) const; + /** * Obtain the ZTXO selector for the specified payment address. If the * `requireSpendingKey` flag is set, this will only return a selector From 77c46933aa422390c25ad3d9f44ec1d5a1ab472b Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 26 Jan 2022 00:02:58 +0000 Subject: [PATCH 2/3] wallet: Fix account generation bug When generating a new account, we were setting up the map from account ID to UFVK ID, but were not setting up the reverse map from UFVK ID to account ID. The latter map is needed in order to verify that a note in the wallet belongs to the account. We were correctly loading it from the account metadata on node start, but between account generation and first restart, z_getbalanceforaccount would always return zero. We now initialize the UFVK address metadata map with the account ID when the account is generated. --- src/wallet/wallet.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ff5f840f2..195b259f6 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -469,8 +469,15 @@ std::optional // metadata that can be used to re-derive the spending key along with // the fingerprint of the associated full viewing key. + // Set up the bidirectional maps between the account ID and the UFVK ID. auto metaKey = std::make_pair(skmeta.GetSeedFingerprint(), skmeta.GetAccountId()); - mapUnifiedAccountKeys.insert({metaKey, skmeta.GetKeyID()}); + mapUnifiedAccountKeys.insert({metaKey, ufvkid}); + // We set up the UFVKAddressMetadata with the correct account ID (so we identify + // the UFVK as corresponding to this account) and empty receivers data (as we + // haven't generated any addresses yet). We don't need to persist this directly, + // because we persist skmeta below, and mapUfvkAddressMetadata is populated in + // LoadUnifiedAccountMetadata(). + mapUfvkAddressMetadata.insert({ufvkid, UFVKAddressMetadata(accountId)}); // Add Transparent component to the wallet AddTransparentSecretKey( From 756f4fc84057be486465e4f042d4b26becdbecc5 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 26 Jan 2022 00:34:45 +0000 Subject: [PATCH 3/3] wallet: Implement `z_getbalanceforaddress` Closes zcash/zcash#5182. --- qa/rpc-tests/wallet_accounts.py | 30 +++++++++++++++----- src/wallet/rpcwallet.cpp | 50 ++++++++++++++++++++++++++++++--- 2 files changed, 69 insertions(+), 11 deletions(-) diff --git a/qa/rpc-tests/wallet_accounts.py b/qa/rpc-tests/wallet_accounts.py index e8854ffef..79d92a518 100755 --- a/qa/rpc-tests/wallet_accounts.py +++ b/qa/rpc-tests/wallet_accounts.py @@ -40,6 +40,22 @@ class WalletAccountsTest(BitcoinTestFramework): assert_equal(expected[pool] * COIN, actual['pools'][pool]['valueZat']) assert_equal(actual['minimum_confirmations'], 1 if minconf is None else minconf) + # Check we only have balances in the expected pools. + # Remember that empty pools are omitted from the output. + def check_address_balance(self, address, expected, minconf=None): + if minconf is None: + actual = self.nodes[0].z_getbalanceforaddress(address) + else: + actual = self.nodes[0].z_getbalanceforaddress(address, minconf) + assert_equal(set(expected), set(actual['pools'])) + for pool in expected: + assert_equal(expected[pool] * COIN, actual['pools'][pool]['valueZat']) + assert_equal(actual['minimum_confirmations'], 1 if minconf is None else minconf) + + def check_balance(self, account, address, expected, minconf=None): + self.check_account_balance(account, expected, minconf) + self.check_address_balance(address, expected, minconf) + def run_test(self): # With a new wallet, the first account will be 0. account0 = self.nodes[0].z_getnewaccount() @@ -79,8 +95,8 @@ class WalletAccountsTest(BitcoinTestFramework): self.check_receiver_types(ua1, ['transparent', 'sapling']) # The balances of the accounts are all zero. - self.check_account_balance(0, {}) - self.check_account_balance(1, {}) + self.check_balance(0, ua0, {}) + self.check_balance(1, ua1, {}) # Manually send funds to one of the receivers in the UA. # TODO: Once z_sendmany supports UAs, receive to the UA instead of the receiver. @@ -98,14 +114,14 @@ class WalletAccountsTest(BitcoinTestFramework): # The new balance should not be visible with the default minconf, but should be # visible with minconf=0. self.sync_all() - self.check_account_balance(0, {}) - self.check_account_balance(0, {'sapling': 10}, 0) + self.check_balance(0, ua0, {}) + self.check_balance(0, ua0, {'sapling': 10}, 0) self.nodes[2].generate(1) self.sync_all() # The default minconf should now detect the balance. - self.check_account_balance(0, {'sapling': 10}) + self.check_balance(0, ua0, {'sapling': 10}) # Manually send funds from the UA receiver. # TODO: Once z_sendmany supports UAs, send from the UA instead of the receiver. @@ -125,8 +141,8 @@ class WalletAccountsTest(BitcoinTestFramework): # shown, as that transaction has been created and broadcast, and _might_ get mined # up until the transaction expires), or 9 (if we include the unmined transaction). self.sync_all() - self.check_account_balance(0, {}) - self.check_account_balance(0, {'sapling': 9}, 0) + self.check_balance(0, ua0, {}) + self.check_balance(0, ua0, {'sapling': 9}, 0) if __name__ == '__main__': diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index ad46851c3..6c29bad8c 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3641,6 +3641,14 @@ UniValue z_getbalanceforaddress(const UniValue& params, bool fHelp) if (!fExperimentalOrchardWallet) { throw JSONRPCError(RPC_WALLET_ENCRYPTION_FAILED, "Error: the Orchard wallet experimental extensions are disabled."); } + + KeyIO keyIO(Params()); + auto decoded = keyIO.DecodePaymentAddress(params[0].get_str()); + if (!decoded.has_value()) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address"); + } + auto address = decoded.value(); + int minconf = 1; if (params.size() > 1) { minconf = params[1].get_int(); @@ -3648,11 +3656,45 @@ UniValue z_getbalanceforaddress(const UniValue& params, bool fHelp) throw JSONRPCError(RPC_INVALID_PARAMETER, "Minimum number of confirmations cannot be less than 0"); } } + + LOCK(pwalletMain->cs_wallet); + + // Get the receivers for this address. + auto selector = pwalletMain->ToZTXOSelector(address, false); + if (!selector.has_value()) { + // The only way we'd reach this is if the address is a unified address for which + // we do not know its UFVK. + throw JSONRPCError( + RPC_INVALID_PARAMETER, + "Error: wallet does not have the Unified Full Viewing Key for the given address"); + } + + auto spendableInputs = pwalletMain->FindSpendableInputs(selector.value(), true, minconf); + + CAmount transparentBalance = 0; + CAmount sproutBalance = 0; + CAmount saplingBalance = 0; + for (const auto& t : spendableInputs.utxos) { + transparentBalance += t.Value(); + } + for (const auto& t : spendableInputs.sproutNoteEntries) { + sproutBalance += t.note.value(); + } + for (const auto& t : spendableInputs.saplingNoteEntries) { + saplingBalance += t.note.value(); + } + UniValue pools(UniValue::VOBJ); - pools.pushKV("transparent", 99999.99); - pools.pushKV("sprout", 99999.99); - pools.pushKV("sapling", 99999.99); - pools.pushKV("orchard", 99999.99); + auto renderBalance = [&](std::string poolName, CAmount balance) { + if (balance > 0) { + UniValue pool(UniValue::VOBJ); + pool.pushKV("valueZat", balance); + pools.pushKV(poolName, pool); + } + }; + renderBalance("transparent", transparentBalance); + renderBalance("sprout", sproutBalance); + renderBalance("sapling", saplingBalance); UniValue result(UniValue::VOBJ); result.pushKV("pools", pools);