From 6caec7aef1b8e06b4e5fe95845bec703ad8d9589 Mon Sep 17 00:00:00 2001 From: therealyingtong Date: Mon, 14 Mar 2022 21:37:20 +0800 Subject: [PATCH 1/4] z_getbalance: Handle Unified Address case. --- src/wallet/rpcwallet.cpp | 15 ++++++++++++--- src/wallet/wallet.cpp | 3 +++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 3a1cc1e29..93a43875a 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3706,9 +3706,18 @@ UniValue z_getbalance(const UniValue& params, bool fHelp) nBalance = getBalanceZaddr(addr, nMinDepth, INT_MAX, false); }, [&](const libzcash::UnifiedAddress& addr) { - throw JSONRPCError( - RPC_INVALID_ADDRESS_OR_KEY, - "Unified addresses are not yet supported for z_getbalance."); + auto selector = pwalletMain->ZTXOSelectorForAddress(addr, true); + auto spendableInputs = pwalletMain->FindSpendableInputs(selector.value(), true, nMinDepth); + + for (const auto& t : spendableInputs.utxos) { + nBalance += t.Value(); + } + for (const auto& t : spendableInputs.saplingNoteEntries) { + nBalance += t.note.value(); + } + for (const auto& t : spendableInputs.orchardNoteMetadata) { + nBalance += t.GetNoteValue(); + } }, }, pa.value()); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index a84895780..ee9b3eeee 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -6341,6 +6341,9 @@ NoteFilter NoteFilter::ForPaymentAddresses(const std::vector Date: Tue, 15 Mar 2022 18:19:54 +0800 Subject: [PATCH 2/4] Adapt RPC tests to use z_getbalance for UAs. --- qa/rpc-tests/wallet_accounts.py | 12 +++++++++--- qa/rpc-tests/wallet_shieldcoinbase_ua_nu5.py | 5 ++++- qa/rpc-tests/wallet_shieldcoinbase_ua_sapling.py | 5 ++++- qa/rpc-tests/wallet_z_sendmany.py | 12 +++++++++--- 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/qa/rpc-tests/wallet_accounts.py b/qa/rpc-tests/wallet_accounts.py index a8f41b0c9..8d6d1cf33 100755 --- a/qa/rpc-tests/wallet_accounts.py +++ b/qa/rpc-tests/wallet_accounts.py @@ -35,14 +35,20 @@ class WalletAccountsTest(BitcoinTestFramework): # Remember that empty pools are omitted from the output. def _check_balance_for_rpc(self, rpcmethod, node, account, expected, minconf): rpc = getattr(self.nodes[node], rpcmethod) - actual = rpc(account) if minconf is None else rpc(account, minconf) + actual = rpc(account, minconf) assert_equal(set(expected), set(actual['pools'])) + total_balance = 0 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) + total_balance += expected[pool] + assert_equal(actual['minimum_confirmations'], minconf) + return total_balance def check_balance(self, node, account, address, expected, minconf=None): - self._check_balance_for_rpc('z_getbalanceforaccount', node, account, expected, minconf) + minconf = 1 if minconf is None else minconf + acct_balance = self._check_balance_for_rpc('z_getbalanceforaccount', node, account, expected, minconf) + z_getbalance = self.nodes[node].z_getbalance(address, minconf) + assert_equal(acct_balance, z_getbalance) fvk = self.nodes[node].z_exportviewingkey(address) self._check_balance_for_rpc('z_getbalanceforviewingkey', node, fvk, expected, minconf) diff --git a/qa/rpc-tests/wallet_shieldcoinbase_ua_nu5.py b/qa/rpc-tests/wallet_shieldcoinbase_ua_nu5.py index 81543ef62..0fbcc31c4 100755 --- a/qa/rpc-tests/wallet_shieldcoinbase_ua_nu5.py +++ b/qa/rpc-tests/wallet_shieldcoinbase_ua_nu5.py @@ -23,7 +23,8 @@ class WalletShieldCoinbaseUANU5(WalletShieldCoinbaseTest): assert('transparent' not in balances['pools']) assert('sprout' not in balances['pools']) # assert('sapling' not in balances['pools']) - assert_equal(balances['pools']['sapling']['valueZat'], expected * COIN) + sapling_balance = balances['pools']['sapling']['valueZat'] + assert_equal(sapling_balance, expected * COIN) # assert_equal(balances['pools']['orchard']['valueZat'], expected * COIN) # While we're at it, check that z_listunspent only shows outputs with @@ -35,6 +36,8 @@ class WalletShieldCoinbaseUANU5(WalletShieldCoinbaseTest): [{'type': x['type'], 'address': x['address']} for x in unspent], ) + total_balance = node.z_getbalance(self.addr) * COIN + assert(total_balance == sapling_balance) if __name__ == '__main__': print("Test shielding to a unified address with NU5 activated") diff --git a/qa/rpc-tests/wallet_shieldcoinbase_ua_sapling.py b/qa/rpc-tests/wallet_shieldcoinbase_ua_sapling.py index 6ef2baea6..fea567309 100755 --- a/qa/rpc-tests/wallet_shieldcoinbase_ua_sapling.py +++ b/qa/rpc-tests/wallet_shieldcoinbase_ua_sapling.py @@ -21,7 +21,8 @@ class WalletShieldCoinbaseUASapling(WalletShieldCoinbaseTest): balances = node.z_getbalanceforaccount(self.account) assert('transparent' not in balances['pools']) assert('sprout' not in balances['pools']) - assert_equal(balances['pools']['sapling']['valueZat'], expected * COIN) + sapling_balance = balances['pools']['sapling']['valueZat'] + assert_equal(sapling_balance, expected * COIN) assert('orchard' not in balances['pools']) # While we're at it, check that z_listunspent only shows outputs with @@ -33,6 +34,8 @@ class WalletShieldCoinbaseUASapling(WalletShieldCoinbaseTest): [{'type': x['type'], 'address': x['address']} for x in unspent], ) + total_balance = node.z_getbalance(self.addr) * COIN + assert(total_balance == sapling_balance) if __name__ == '__main__': print("Test shielding to a unified address with sapling activated (but not NU5)") diff --git a/qa/rpc-tests/wallet_z_sendmany.py b/qa/rpc-tests/wallet_z_sendmany.py index 2ba397ec9..8f4129036 100755 --- a/qa/rpc-tests/wallet_z_sendmany.py +++ b/qa/rpc-tests/wallet_z_sendmany.py @@ -27,14 +27,20 @@ class WalletZSendmanyTest(BitcoinTestFramework): # Remember that empty pools are omitted from the output. def _check_balance_for_rpc(self, rpcmethod, node, account, expected, minconf): rpc = getattr(self.nodes[node], rpcmethod) - actual = rpc(account) if minconf is None else rpc(account, minconf) + actual = rpc(account, minconf) assert_equal(set(expected), set(actual['pools'])) + total_balance = 0 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) + total_balance += expected[pool] + assert_equal(actual['minimum_confirmations'], minconf) + return total_balance def check_balance(self, node, account, address, expected, minconf=None): - self._check_balance_for_rpc('z_getbalanceforaccount', node, account, expected, minconf) + minconf = 1 if minconf is None else minconf + acct_balance = self._check_balance_for_rpc('z_getbalanceforaccount', node, account, expected, minconf) + z_getbalance = self.nodes[node].z_getbalance(address, minconf) + assert_equal(acct_balance, z_getbalance) fvk = self.nodes[node].z_exportviewingkey(address) self._check_balance_for_rpc('z_getbalanceforviewingkey', node, fvk, expected, minconf) From 1cd92e579cc3ce90baa4f6b4ecedd641764b078f Mon Sep 17 00:00:00 2001 From: ying tong Date: Tue, 15 Mar 2022 20:43:27 +0800 Subject: [PATCH 3/4] Style improvements in RPC tests. Co-authored-by: Daira Hopwood --- qa/rpc-tests/wallet_accounts.py | 3 +-- qa/rpc-tests/wallet_shieldcoinbase_ua_nu5.py | 2 +- qa/rpc-tests/wallet_shieldcoinbase_ua_sapling.py | 2 +- qa/rpc-tests/wallet_z_sendmany.py | 3 +-- 4 files changed, 4 insertions(+), 6 deletions(-) diff --git a/qa/rpc-tests/wallet_accounts.py b/qa/rpc-tests/wallet_accounts.py index 8d6d1cf33..4edc02618 100755 --- a/qa/rpc-tests/wallet_accounts.py +++ b/qa/rpc-tests/wallet_accounts.py @@ -44,8 +44,7 @@ class WalletAccountsTest(BitcoinTestFramework): assert_equal(actual['minimum_confirmations'], minconf) return total_balance - def check_balance(self, node, account, address, expected, minconf=None): - minconf = 1 if minconf is None else minconf + def check_balance(self, node, account, address, expected, minconf=1): acct_balance = self._check_balance_for_rpc('z_getbalanceforaccount', node, account, expected, minconf) z_getbalance = self.nodes[node].z_getbalance(address, minconf) assert_equal(acct_balance, z_getbalance) diff --git a/qa/rpc-tests/wallet_shieldcoinbase_ua_nu5.py b/qa/rpc-tests/wallet_shieldcoinbase_ua_nu5.py index 0fbcc31c4..30a7f22fb 100755 --- a/qa/rpc-tests/wallet_shieldcoinbase_ua_nu5.py +++ b/qa/rpc-tests/wallet_shieldcoinbase_ua_nu5.py @@ -37,7 +37,7 @@ class WalletShieldCoinbaseUANU5(WalletShieldCoinbaseTest): ) total_balance = node.z_getbalance(self.addr) * COIN - assert(total_balance == sapling_balance) + assert_equal(total_balance, sapling_balance) if __name__ == '__main__': print("Test shielding to a unified address with NU5 activated") diff --git a/qa/rpc-tests/wallet_shieldcoinbase_ua_sapling.py b/qa/rpc-tests/wallet_shieldcoinbase_ua_sapling.py index fea567309..f07ef656e 100755 --- a/qa/rpc-tests/wallet_shieldcoinbase_ua_sapling.py +++ b/qa/rpc-tests/wallet_shieldcoinbase_ua_sapling.py @@ -35,7 +35,7 @@ class WalletShieldCoinbaseUASapling(WalletShieldCoinbaseTest): ) total_balance = node.z_getbalance(self.addr) * COIN - assert(total_balance == sapling_balance) + assert_equal(total_balance, sapling_balance) if __name__ == '__main__': print("Test shielding to a unified address with sapling activated (but not NU5)") diff --git a/qa/rpc-tests/wallet_z_sendmany.py b/qa/rpc-tests/wallet_z_sendmany.py index 8f4129036..48df09cb0 100755 --- a/qa/rpc-tests/wallet_z_sendmany.py +++ b/qa/rpc-tests/wallet_z_sendmany.py @@ -36,8 +36,7 @@ class WalletZSendmanyTest(BitcoinTestFramework): assert_equal(actual['minimum_confirmations'], minconf) return total_balance - def check_balance(self, node, account, address, expected, minconf=None): - minconf = 1 if minconf is None else minconf + def check_balance(self, node, account, address, expected, minconf=1): acct_balance = self._check_balance_for_rpc('z_getbalanceforaccount', node, account, expected, minconf) z_getbalance = self.nodes[node].z_getbalance(address, minconf) assert_equal(acct_balance, z_getbalance) From 482f0e7e447339b9d45335de5e264b676fee645b Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 15 Mar 2022 16:17:46 -0600 Subject: [PATCH 4/4] Return the correct error for `z_getbalance` if a UA does not correspond to an account in the wallet.. Also, update the help text for z_getbalance to suggest that the user prefer z_getbalanceforviewingkey. Co-authored-by: str4d --- src/wallet/rpcwallet.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 93a43875a..4be01f570 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3649,11 +3649,10 @@ UniValue z_getbalance(const UniValue& params, bool fHelp) if (fHelp || params.size() == 0 || params.size() > 3) throw runtime_error( "z_getbalance \"address\" ( minconf inZat )\n" - "\nDEPRECATED\n" + "\nDEPRECATED; please use z_getbalanceforviewingkey instead.`\n" "\nReturns the balance of a taddr or zaddr belonging to the node's wallet.\n" "\nCAUTION: If the wallet has only an incoming viewing key for this address, then spends cannot be" "\ndetected, and so the returned balance may be larger than the actual balance." - "\nThe argument address may not be a Unified Address; please use z_getbalanceforviewingkey instead.\n" "\nArguments:\n" "1. \"address\" (string) The selected address. It may be a transparent or shielded address.\n" "2. minconf (numeric, optional, default=1) Only include transactions confirmed at least this many times.\n" @@ -3707,6 +3706,11 @@ UniValue z_getbalance(const UniValue& params, bool fHelp) }, [&](const libzcash::UnifiedAddress& addr) { auto selector = pwalletMain->ZTXOSelectorForAddress(addr, true); + if (!selector.has_value()) { + throw JSONRPCError( + RPC_INVALID_ADDRESS_OR_KEY, + "Unified address does not correspond to an account in the wallet"); + } auto spendableInputs = pwalletMain->FindSpendableInputs(selector.value(), true, nMinDepth); for (const auto& t : spendableInputs.utxos) {