From 06f2a8f9b61684fca6b0f2e834fabc6353947c7c Mon Sep 17 00:00:00 2001 From: Jay Graber Date: Thu, 13 Sep 2018 16:27:15 -0700 Subject: [PATCH 1/9] s/jsoutindex/outindex for sapling outputs --- src/wallet/rpcwallet.cpp | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index d93279986..d46ba7b85 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2473,7 +2473,8 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) " {\n" " \"txid\" : \"txid\", (string) the transaction id \n" " \"jsindex\" : n (numeric) the joinsplit index\n" - " \"jsoutindex\" : n (numeric) the output index of the joinsplit\n" + " \"jsoutindex\" (sprout) : n (numeric) the output index of the joinsplit\n" + " \"outindex\" (sapling) : n (numeric) the output index\n" " \"confirmations\" : n (numeric) the number of confirmations\n" " \"spendable\" : true|false (boolean) true if note can be spent by wallet, false if note has zero confirmations, false if address is watchonly\n" " \"address\" : \"address\", (string) the shielded address\n" @@ -2584,6 +2585,25 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) results.push_back(obj); } // TODO: Sapling + for (UnspentSaplingNoteEntry & entry : saplingEntries) { + UniValue obj(UniValue::VOBJ); + obj.push_back(Pair("txid", entry.op.hash.ToString())); + obj.push_back(Pair("outindex", (int)entry.op.n)); + obj.push_back(Pair("confirmations", entry.nHeight)); + libzcash::SaplingIncomingViewingKey ivk; + libzcash::SaplingFullViewingKey fvk; + pwalletMain->GetSaplingIncomingViewingKey(boost::get(entry.address), ivk); + pwalletMain->GetSaplingFullViewingKey(ivk, fvk); + bool hasSaplingSpendingKey = pwalletMain->HaveSaplingSpendingKey(fvk); + obj.push_back(Pair("spendable", hasSaplingSpendingKey)); + obj.push_back(Pair("address", EncodePaymentAddress(entry.address))); + obj.push_back(Pair("amount", ValueFromAmount(CAmount(entry.note.value())))); // note.value() == plaintext.value() + obj.push_back(Pair("memo", HexStr(entry.memo))); + if (hasSaplingSpendingKey) { + obj.push_back(Pair("change", pwalletMain->IsNoteSaplingChange(nullifierSet, entry.address, entry.op))); + } + results.push_back(obj); + } } return results; From 66795a408b557be002a14da9ab1782f184b92311 Mon Sep 17 00:00:00 2001 From: Jay Graber Date: Mon, 27 Aug 2018 18:04:38 -0700 Subject: [PATCH 2/9] z_listunspent sapling support - needs refactor --- src/wallet/rpcwallet.cpp | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index d46ba7b85..b0807e1b6 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2463,7 +2463,7 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) "1. minconf (numeric, optional, default=1) The minimum confirmations to filter\n" "2. maxconf (numeric, optional, default=9999999) The maximum confirmations to filter\n" "3. includeWatchonly (bool, optional, default=false) Also include watchonly addresses (see 'z_importviewingkey')\n" - "4. \"addresses\" (string) A json array of zaddrs to filter on. Duplicate addresses not allowed.\n" + "4. \"addresses\" (string) A json array of zaddrs (both Sprout and Sapling) to filter on. Duplicate addresses not allowed.\n" " [\n" " \"address\" (string) zaddr\n" " ,...\n" @@ -2536,12 +2536,25 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) auto zaddr = DecodePaymentAddress(address); if (IsValidPaymentAddress(zaddr)) { // TODO: Add Sapling support. For now, ensure we can freely convert. - assert(boost::get(&zaddr) != nullptr); - libzcash::SproutPaymentAddress addr = boost::get(zaddr); - if (!fIncludeWatchonly && !pwalletMain->HaveSproutSpendingKey(addr)) { + if (boost::get(&zaddr) != nullptr){ + libzcash::SproutPaymentAddress sproutAddr = boost::get(zaddr); + if (!fIncludeWatchonly && !pwalletMain->HaveSproutSpendingKey(sproutAddr)) { throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, spending key for address does not belong to wallet: ") + address); + } + zaddrs.insert(sproutAddr); + } else if (boost::get(&zaddr) != nullptr) { + libzcash::SaplingPaymentAddress saplingAddr = boost::get(zaddr); + libzcash::SaplingIncomingViewingKey ivk; + libzcash::SaplingFullViewingKey fvk; + if (!fIncludeWatchonly && + !pwalletMain->GetSaplingIncomingViewingKey(saplingAddr, ivk) && + !pwalletMain->GetSaplingFullViewingKey(ivk, fvk) && + !pwalletMain->HaveSaplingSpendingKey(fvk)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, spending key for address does not belong to wallet: ") + address); + } + zaddrs.insert(saplingAddr); } - zaddrs.insert(addr); + } else { throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, address is not a valid zaddr: ") + address); } @@ -2557,7 +2570,12 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) // TODO: Add Sapling support std::set sproutzaddrs = {}; pwalletMain->GetSproutPaymentAddresses(sproutzaddrs); + + std::set saplingzaddrs = {}; + pwalletMain->GetSaplingPaymentAddresses(saplingzaddrs); + zaddrs.insert(sproutzaddrs.begin(), sproutzaddrs.end()); + zaddrs.insert(saplingzaddrs.begin(), saplingzaddrs.end()); } UniValue results(UniValue::VARR); @@ -2567,6 +2585,7 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) std::vector saplingEntries; pwalletMain->GetUnspentFilteredNotes(sproutEntries, saplingEntries, zaddrs, nMinDepth, nMaxDepth, !fIncludeWatchonly); std::set> nullifierSet = pwalletMain->GetNullifiersForAddresses(zaddrs); + for (CUnspentSproutNotePlaintextEntry & entry : sproutEntries) { UniValue obj(UniValue::VOBJ); obj.push_back(Pair("txid", entry.jsop.hash.ToString())); @@ -2584,6 +2603,7 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) } results.push_back(obj); } + // TODO: Sapling for (UnspentSaplingNoteEntry & entry : saplingEntries) { UniValue obj(UniValue::VOBJ); From 011f9a02ef10eb1429c2aa697e2c4511ef698ee7 Mon Sep 17 00:00:00 2001 From: Jay Graber Date: Tue, 28 Aug 2018 17:16:04 -0700 Subject: [PATCH 3/9] Add rpc test for sprout txs z_listunspent --- qa/pull-tester/rpc-tests.sh | 1 + qa/rpc-tests/wallet_listnotes.py | 84 ++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) create mode 100755 qa/rpc-tests/wallet_listnotes.py diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index dde4144c4..50244440e 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -27,6 +27,7 @@ testScripts=( 'wallet_1941.py' 'wallet_addresses.py' 'wallet_sapling.py' + 'wallet_listnotes.py' 'listtransactions.py' 'mempool_resurrect_test.py' 'txn_doublespend.py' diff --git a/qa/rpc-tests/wallet_listnotes.py b/qa/rpc-tests/wallet_listnotes.py new file mode 100755 index 000000000..f839b1ffd --- /dev/null +++ b/qa/rpc-tests/wallet_listnotes.py @@ -0,0 +1,84 @@ +#!/usr/bin/env python2 +# Copyright (c) 2018 The Zcash developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal, start_nodes, wait_and_assert_operationid_status + +from decimal import Decimal + +# Test wallet z_listunspent and z_listreceivedbyaddress behaviour across network upgrades +# TODO: Test z_listreceivedbyaddress +class WalletListNotes(BitcoinTestFramework): + + def setup_nodes(self): + return start_nodes(4, self.options.tmpdir, [[ + '-nuparams=5ba81b19:202', # Overwinter + '-nuparams=76b809bb:204', # Sapling + ]] * 4) + + def run_test(self): + # Current height = 200 -> Sprout + sproutzaddr = self.nodes[0].z_getnewaddress('sprout') + saplingzaddr = self.nodes[0].z_getnewaddress('sapling') + print "sprout zaddr", sproutzaddr + print "saplingzaddr", saplingzaddr + + # Current height = 201 -> Sprout + self.nodes[0].generate(1) + self.sync_all() + + mining_addr = self.nodes[0].listunspent()[0]['address'] + # Shield coinbase funds + recipients = [] + recipients.append({"address":sproutzaddr, "amount":Decimal('10.0')-Decimal('0.0001')}) # utxo amount less fee + myopid = self.nodes[0].z_sendmany(mining_addr, recipients) + txid_1 = wait_and_assert_operationid_status(self.nodes[0], myopid) + + # No funds in sproutzaddr yet + assert_equal(set(self.nodes[0].z_listunspent()), set()) + + print self.nodes[0].z_gettotalbalance() # no private balance displayed bc no confirmations yet + + # list unspent, allowing 0 confirmations + unspent_cb = self.nodes[0].z_listunspent(0) + # list unspent, filtering by address + unspent_cb_filter = self.nodes[0].z_listunspent(0, 9999, False, [sproutzaddr]) + assert_equal(unspent_cb, unspent_cb_filter) + assert_equal(len(unspent_cb), 1) + assert_equal(unspent_cb[0]['change'], False) + assert_equal(unspent_cb[0]['txid'], txid_1) + assert_equal(unspent_cb[0]['spendable'], True) + assert_equal(unspent_cb[0]['address'], sproutzaddr) + assert_equal(unspent_cb[0]['amount'], Decimal('10.0')-Decimal('0.0001')) + + # Generate a block to confirm shield coinbase tx + # Current height = 202 -> Overwinter. Default address type is Sprout + self.nodes[0].generate(1) + self.sync_all() + + sproutzaddr2 = self.nodes[0].z_getnewaddress('sprout') + recipients = [{"address": sproutzaddr2, "amount":Decimal('1.0')-Decimal('0.0001')}] + myopid = self.nodes[0].z_sendmany(sproutzaddr, recipients) + txid_2 = wait_and_assert_operationid_status(self.nodes[0], myopid) + + # list unspent, allowing 0conf txs + unspent_tx = self.nodes[0].z_listunspent(0) + unspent_tx_filter = self.nodes[0].z_listunspent(0, 9999, False, [sproutzaddr2]) + assert_equal(len(unspent_tx), 2) + assert_equal(unspent_tx[0]['change'], False) + assert_equal(unspent_tx[0]['txid'], txid_2) + assert_equal(unspent_tx[0]['spendable'], True) + assert_equal(unspent_tx[0]['address'], sproutzaddr2) + assert_equal(unspent_tx[0]['amount'], Decimal('1.0')-Decimal('0.0001')) + # TODO: test change + + # No funds in saplingzaddr yet + assert_equal(set(self.nodes[0].z_listunspent(0, 9999, False, [saplingzaddr])), set()) + + # Current height = 204 -> Sapling + # self.nodes[0].generate(2) + +if __name__ == '__main__': + WalletListNotes().main() \ No newline at end of file From cd1c6e3767d3524bbd2609976eae8d16bd987c6a Mon Sep 17 00:00:00 2001 From: Jay Graber Date: Thu, 6 Sep 2018 13:59:45 -0700 Subject: [PATCH 4/9] Modify comments --- src/wallet/rpcwallet.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index b0807e1b6..02c899187 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2535,7 +2535,7 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) string address = o.get_str(); auto zaddr = DecodePaymentAddress(address); if (IsValidPaymentAddress(zaddr)) { - // TODO: Add Sapling support. For now, ensure we can freely convert. + // TODO: Add visitor to handle support for Sprout and Sapling addrs. if (boost::get(&zaddr) != nullptr){ libzcash::SproutPaymentAddress sproutAddr = boost::get(zaddr); if (!fIncludeWatchonly && !pwalletMain->HaveSproutSpendingKey(sproutAddr)) { @@ -2567,10 +2567,10 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) } else { // User did not provide zaddrs, so use default i.e. all addresses - // TODO: Add Sapling support std::set sproutzaddrs = {}; pwalletMain->GetSproutPaymentAddresses(sproutzaddrs); + // Sapling support std::set saplingzaddrs = {}; pwalletMain->GetSaplingPaymentAddresses(saplingzaddrs); From 27b3cce94f92e2842c4fcab576c3059e9ee2aea8 Mon Sep 17 00:00:00 2001 From: Jay Graber Date: Thu, 6 Sep 2018 15:51:02 -0700 Subject: [PATCH 5/9] Modify GetNullifiersForAddresses for Sapling --- src/wallet/wallet.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d838f485e..d4ef17616 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -548,6 +548,12 @@ std::set> CWallet::GetNullifiersFor } } } + // Sapling + for (const auto & noteDataPair : txPair.second.mapSaplingNoteData) { + if (noteDataPair.second.nullifier && ivkMap.count(noteDataPair.second.ivk)) { + nullifierSet.insert(std::make_pair(ivkMap[noteDataPair.second.ivk], noteDataPair.second.nullifier.get())); + } + } } return nullifierSet; } From d7d6480ce35893de1342f050f793cba81c853692 Mon Sep 17 00:00:00 2001 From: Larry Ruane Date: Tue, 11 Sep 2018 14:51:38 -0600 Subject: [PATCH 6/9] z_listunspent rpc unit test: add testing for Sapling --- qa/rpc-tests/wallet_listnotes.py | 155 ++++++++++++++++++++++++------- 1 file changed, 120 insertions(+), 35 deletions(-) diff --git a/qa/rpc-tests/wallet_listnotes.py b/qa/rpc-tests/wallet_listnotes.py index f839b1ffd..32997df43 100755 --- a/qa/rpc-tests/wallet_listnotes.py +++ b/qa/rpc-tests/wallet_listnotes.py @@ -20,65 +20,150 @@ class WalletListNotes(BitcoinTestFramework): def run_test(self): # Current height = 200 -> Sprout + assert_equal(200, self.nodes[0].getblockcount()) sproutzaddr = self.nodes[0].z_getnewaddress('sprout') + + # test that we can create a sapling zaddr before sapling activates saplingzaddr = self.nodes[0].z_getnewaddress('sapling') - print "sprout zaddr", sproutzaddr - print "saplingzaddr", saplingzaddr + + # we've got lots of coinbase (taddr) but no shielded funds yet + assert_equal(0, Decimal(self.nodes[0].z_gettotalbalance()['private'])) - # Current height = 201 -> Sprout + # Set current height to 201 -> Sprout self.nodes[0].generate(1) self.sync_all() - + assert_equal(201, self.nodes[0].getblockcount()) + mining_addr = self.nodes[0].listunspent()[0]['address'] - # Shield coinbase funds - recipients = [] - recipients.append({"address":sproutzaddr, "amount":Decimal('10.0')-Decimal('0.0001')}) # utxo amount less fee + + # Shield coinbase funds (must be a multiple of 10, no change allowed pre-sapling) + receive_amount_10 = Decimal('10.0') - Decimal('0.0001') + recipients = [{"address":sproutzaddr, "amount":receive_amount_10}] myopid = self.nodes[0].z_sendmany(mining_addr, recipients) txid_1 = wait_and_assert_operationid_status(self.nodes[0], myopid) + self.sync_all() - # No funds in sproutzaddr yet - assert_equal(set(self.nodes[0].z_listunspent()), set()) + # No funds (with (default) one or more confirmations) in sproutzaddr yet + assert_equal(0, len(self.nodes[0].z_listunspent())) + assert_equal(0, len(self.nodes[0].z_listunspent(1))) - print self.nodes[0].z_gettotalbalance() # no private balance displayed bc no confirmations yet + # no private balance because no confirmations yet + assert_equal(0, Decimal(self.nodes[0].z_gettotalbalance()['private'])) - # list unspent, allowing 0 confirmations + # list private unspent, this time allowing 0 confirmations unspent_cb = self.nodes[0].z_listunspent(0) - # list unspent, filtering by address - unspent_cb_filter = self.nodes[0].z_listunspent(0, 9999, False, [sproutzaddr]) + assert_equal(1, len(unspent_cb)) + assert_equal(False, unspent_cb[0]['change']) + assert_equal(txid_1, unspent_cb[0]['txid']) + assert_equal(True, unspent_cb[0]['spendable']) + assert_equal(sproutzaddr, unspent_cb[0]['address']) + assert_equal(receive_amount_10, unspent_cb[0]['amount']) + + # list unspent, filtering by address, should produce same result + unspent_cb_filter = self.nodes[0].z_listunspent(0, 9999, False, [sproutzaddr]) assert_equal(unspent_cb, unspent_cb_filter) - assert_equal(len(unspent_cb), 1) - assert_equal(unspent_cb[0]['change'], False) - assert_equal(unspent_cb[0]['txid'], txid_1) - assert_equal(unspent_cb[0]['spendable'], True) - assert_equal(unspent_cb[0]['address'], sproutzaddr) - assert_equal(unspent_cb[0]['amount'], Decimal('10.0')-Decimal('0.0001')) - # Generate a block to confirm shield coinbase tx - # Current height = 202 -> Overwinter. Default address type is Sprout + # Generate a block to confirm shield coinbase tx self.nodes[0].generate(1) self.sync_all() - sproutzaddr2 = self.nodes[0].z_getnewaddress('sprout') - recipients = [{"address": sproutzaddr2, "amount":Decimal('1.0')-Decimal('0.0001')}] + # Current height = 202 -> Overwinter. Default address type remains Sprout + assert_equal(202, self.nodes[0].getblockcount()) + + # Send 1.0 (actually 0.9999) from sproutzaddr to a new zaddr + sproutzaddr2 = self.nodes[0].z_getnewaddress() + receive_amount_1 = Decimal('1.0') - Decimal('0.0001') + change_amount_9 = receive_amount_10 - Decimal('1.0') + assert_equal('sprout', self.nodes[0].z_validateaddress(sproutzaddr2)['type']) + recipients = [{"address": sproutzaddr2, "amount":receive_amount_1}] myopid = self.nodes[0].z_sendmany(sproutzaddr, recipients) txid_2 = wait_and_assert_operationid_status(self.nodes[0], myopid) + self.sync_all() # list unspent, allowing 0conf txs unspent_tx = self.nodes[0].z_listunspent(0) - unspent_tx_filter = self.nodes[0].z_listunspent(0, 9999, False, [sproutzaddr2]) assert_equal(len(unspent_tx), 2) - assert_equal(unspent_tx[0]['change'], False) - assert_equal(unspent_tx[0]['txid'], txid_2) - assert_equal(unspent_tx[0]['spendable'], True) - assert_equal(unspent_tx[0]['address'], sproutzaddr2) - assert_equal(unspent_tx[0]['amount'], Decimal('1.0')-Decimal('0.0001')) - # TODO: test change + # sort low-to-high by amount (order of returned entries is not guaranteed) + unspent_tx = sorted(unspent_tx, key=lambda k: k['amount']) + assert_equal(False, unspent_tx[0]['change']) + assert_equal(txid_2, unspent_tx[0]['txid']) + assert_equal(True, unspent_tx[0]['spendable']) + assert_equal(sproutzaddr2, unspent_tx[0]['address']) + assert_equal(receive_amount_1, unspent_tx[0]['amount']) + + assert_equal(True, unspent_tx[1]['change']) + assert_equal(txid_2, unspent_tx[1]['txid']) + assert_equal(True, unspent_tx[1]['spendable']) + assert_equal(sproutzaddr, unspent_tx[1]['address']) + assert_equal(change_amount_9, unspent_tx[1]['amount']) + + unspent_tx_filter = self.nodes[0].z_listunspent(0, 9999, False, [sproutzaddr2]) + assert_equal(1, len(unspent_tx_filter)) + assert_equal(unspent_tx[0], unspent_tx_filter[0]) + + unspent_tx_filter = self.nodes[0].z_listunspent(0, 9999, False, [sproutzaddr]) + assert_equal(1, len(unspent_tx_filter)) + assert_equal(unspent_tx[1], unspent_tx_filter[0]) + + # Set current height to 204 -> Sapling + self.nodes[0].generate(2) + self.sync_all() + assert_equal(204, self.nodes[0].getblockcount()) # No funds in saplingzaddr yet - assert_equal(set(self.nodes[0].z_listunspent(0, 9999, False, [saplingzaddr])), set()) - - # Current height = 204 -> Sapling - # self.nodes[0].generate(2) + assert_equal(0, len(self.nodes[0].z_listunspent(0, 9999, False, [saplingzaddr]))) + + # Send 0.9999 to our sapling zaddr + # (sending from a sprout zaddr to a sapling zaddr is disallowed, + # so send from coin base) + receive_amount_2 = Decimal('2.0') - Decimal('0.0001') + recipients = [{"address": saplingzaddr, "amount":receive_amount_2}] + myopid = self.nodes[0].z_sendmany(mining_addr, recipients) + txid_3 = wait_and_assert_operationid_status(self.nodes[0], myopid) + self.sync_all() + unspent_tx = self.nodes[0].z_listunspent(0) + assert_equal(3, len(unspent_tx)) + + # low-to-high in amount + unspent_tx = sorted(unspent_tx, key=lambda k: k['amount']) + + assert_equal(False, unspent_tx[0]['change']) + assert_equal(txid_2, unspent_tx[0]['txid']) + assert_equal(True, unspent_tx[0]['spendable']) + assert_equal(sproutzaddr2, unspent_tx[0]['address']) + assert_equal(receive_amount_1, unspent_tx[0]['amount']) + + assert_equal(False, unspent_tx[1]['change']) + assert_equal(txid_3, unspent_tx[1]['txid']) + assert_equal(True, unspent_tx[1]['spendable']) + assert_equal(saplingzaddr, unspent_tx[1]['address']) + assert_equal(receive_amount_2, unspent_tx[1]['amount']) + + assert_equal(True, unspent_tx[2]['change']) + assert_equal(txid_2, unspent_tx[2]['txid']) + assert_equal(True, unspent_tx[2]['spendable']) + assert_equal(sproutzaddr, unspent_tx[2]['address']) + assert_equal(change_amount_9, unspent_tx[2]['amount']) + + unspent_tx_filter = self.nodes[0].z_listunspent(0, 9999, False, [saplingzaddr]) + assert_equal(1, len(unspent_tx_filter)) + assert_equal(unspent_tx[1], unspent_tx_filter[0]) + + # test that pre- and post-sapling can be filtered in a single call + unspent_tx_filter = self.nodes[0].z_listunspent(0, 9999, False, + [sproutzaddr, saplingzaddr]) + assert_equal(2, len(unspent_tx_filter)) + unspent_tx_filter = sorted(unspent_tx_filter, key=lambda k: k['amount']) + assert_equal(unspent_tx[1], unspent_tx_filter[0]) + assert_equal(unspent_tx[2], unspent_tx_filter[1]) + + # so far, this node has no watchonly addresses, so results are the same + unspent_tx_watchonly = self.nodes[0].z_listunspent(0, 9999, True) + unspent_tx_watchonly = sorted(unspent_tx_watchonly, key=lambda k: k['amount']) + assert_equal(unspent_tx, unspent_tx_watchonly) + + # TODO: use z_exportviewingkey, z_importviewingkey to test includeWatchonly + # but this requires Sapling support for those RPCs if __name__ == '__main__': - WalletListNotes().main() \ No newline at end of file + WalletListNotes().main() From c0f7e4059db2221473ecb4f94e6fd5966260bb5c Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 28 Sep 2018 22:08:07 -0700 Subject: [PATCH 7/9] Fix rebasing of CWallet::GetNullifiersForAddresses --- src/wallet/wallet.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d4ef17616..d838f485e 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -548,12 +548,6 @@ std::set> CWallet::GetNullifiersFor } } } - // Sapling - for (const auto & noteDataPair : txPair.second.mapSaplingNoteData) { - if (noteDataPair.second.nullifier && ivkMap.count(noteDataPair.second.ivk)) { - nullifierSet.insert(std::make_pair(ivkMap[noteDataPair.second.ivk], noteDataPair.second.nullifier.get())); - } - } } return nullifierSet; } From 5f57babd08ac29db317c4e8352ee2997c03d1bc5 Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 28 Sep 2018 22:11:05 -0700 Subject: [PATCH 8/9] Cleanup to address review comments. --- qa/rpc-tests/wallet_listnotes.py | 3 +- src/wallet/rpcwallet.cpp | 60 ++++++++++++-------------------- 2 files changed, 23 insertions(+), 40 deletions(-) diff --git a/qa/rpc-tests/wallet_listnotes.py b/qa/rpc-tests/wallet_listnotes.py index 32997df43..5cd89c661 100755 --- a/qa/rpc-tests/wallet_listnotes.py +++ b/qa/rpc-tests/wallet_listnotes.py @@ -8,8 +8,7 @@ from test_framework.util import assert_equal, start_nodes, wait_and_assert_opera from decimal import Decimal -# Test wallet z_listunspent and z_listreceivedbyaddress behaviour across network upgrades -# TODO: Test z_listreceivedbyaddress +# Test wallet z_listunspent behaviour across network upgrades class WalletListNotes(BitcoinTestFramework): def setup_nodes(self): diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 02c899187..78226452c 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2458,7 +2458,8 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) "Optionally filter to only include notes sent to specified addresses.\n" "When minconf is 0, unspent notes with zero confirmations are returned, even though they are not immediately spendable.\n" "Results are an array of Objects, each of which has:\n" - "{txid, jsindex, jsoutindex, confirmations, address, amount, memo}\n" + "{txid, jsindex, jsoutindex, confirmations, address, amount, memo} (Sprout)\n" + "{txid, outindex, confirmations, address, amount, memo} (Sapling)\n" "\nArguments:\n" "1. minconf (numeric, optional, default=1) The minimum confirmations to filter\n" "2. maxconf (numeric, optional, default=9999999) The maximum confirmations to filter\n" @@ -2535,26 +2536,10 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) string address = o.get_str(); auto zaddr = DecodePaymentAddress(address); if (IsValidPaymentAddress(zaddr)) { - // TODO: Add visitor to handle support for Sprout and Sapling addrs. - if (boost::get(&zaddr) != nullptr){ - libzcash::SproutPaymentAddress sproutAddr = boost::get(zaddr); - if (!fIncludeWatchonly && !pwalletMain->HaveSproutSpendingKey(sproutAddr)) { - throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, spending key for address does not belong to wallet: ") + address); - } - zaddrs.insert(sproutAddr); - } else if (boost::get(&zaddr) != nullptr) { - libzcash::SaplingPaymentAddress saplingAddr = boost::get(zaddr); - libzcash::SaplingIncomingViewingKey ivk; - libzcash::SaplingFullViewingKey fvk; - if (!fIncludeWatchonly && - !pwalletMain->GetSaplingIncomingViewingKey(saplingAddr, ivk) && - !pwalletMain->GetSaplingFullViewingKey(ivk, fvk) && - !pwalletMain->HaveSaplingSpendingKey(fvk)) { - throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, spending key for address does not belong to wallet: ") + address); - } - zaddrs.insert(saplingAddr); + auto hasSpendingKey = boost::apply_visitor(HaveSpendingKeyForPaymentAddress(pwalletMain), zaddr); + if (hasSpendingKey) { + zaddrs.insert(zaddr); } - } else { throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, address is not a valid zaddr: ") + address); } @@ -2604,25 +2589,24 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) results.push_back(obj); } - // TODO: Sapling for (UnspentSaplingNoteEntry & entry : saplingEntries) { - UniValue obj(UniValue::VOBJ); - obj.push_back(Pair("txid", entry.op.hash.ToString())); - obj.push_back(Pair("outindex", (int)entry.op.n)); - obj.push_back(Pair("confirmations", entry.nHeight)); - libzcash::SaplingIncomingViewingKey ivk; - libzcash::SaplingFullViewingKey fvk; - pwalletMain->GetSaplingIncomingViewingKey(boost::get(entry.address), ivk); - pwalletMain->GetSaplingFullViewingKey(ivk, fvk); - bool hasSaplingSpendingKey = pwalletMain->HaveSaplingSpendingKey(fvk); - obj.push_back(Pair("spendable", hasSaplingSpendingKey)); - obj.push_back(Pair("address", EncodePaymentAddress(entry.address))); - obj.push_back(Pair("amount", ValueFromAmount(CAmount(entry.note.value())))); // note.value() == plaintext.value() - obj.push_back(Pair("memo", HexStr(entry.memo))); - if (hasSaplingSpendingKey) { - obj.push_back(Pair("change", pwalletMain->IsNoteSaplingChange(nullifierSet, entry.address, entry.op))); - } - results.push_back(obj); + UniValue obj(UniValue::VOBJ); + obj.push_back(Pair("txid", entry.op.hash.ToString())); + obj.push_back(Pair("outindex", (int)entry.op.n)); + obj.push_back(Pair("confirmations", entry.nHeight)); + libzcash::SaplingIncomingViewingKey ivk; + libzcash::SaplingFullViewingKey fvk; + pwalletMain->GetSaplingIncomingViewingKey(boost::get(entry.address), ivk); + pwalletMain->GetSaplingFullViewingKey(ivk, fvk); + bool hasSaplingSpendingKey = pwalletMain->HaveSaplingSpendingKey(fvk); + obj.push_back(Pair("spendable", hasSaplingSpendingKey)); + obj.push_back(Pair("address", EncodePaymentAddress(entry.address))); + obj.push_back(Pair("amount", ValueFromAmount(CAmount(entry.note.value())))); // note.value() is equivalent to plaintext.value() + obj.push_back(Pair("memo", HexStr(entry.memo))); + if (hasSaplingSpendingKey) { + obj.push_back(Pair("change", pwalletMain->IsNoteSaplingChange(nullifierSet, entry.address, entry.op))); + } + results.push_back(obj); } } From 27a6a99cb05127f8f30df222b5bd1e11f853fcdb Mon Sep 17 00:00:00 2001 From: Larry Ruane Date: Thu, 4 Oct 2018 12:26:36 -0600 Subject: [PATCH 9/9] fix z_listunspent includeWatchonly logic --- src/wallet/rpcwallet.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 78226452c..9d8ae70a6 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2535,14 +2535,14 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) } string address = o.get_str(); auto zaddr = DecodePaymentAddress(address); - if (IsValidPaymentAddress(zaddr)) { - auto hasSpendingKey = boost::apply_visitor(HaveSpendingKeyForPaymentAddress(pwalletMain), zaddr); - if (hasSpendingKey) { - zaddrs.insert(zaddr); - } - } else { + if (!IsValidPaymentAddress(zaddr)) { throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, address is not a valid zaddr: ") + address); } + auto hasSpendingKey = boost::apply_visitor(HaveSpendingKeyForPaymentAddress(pwalletMain), zaddr); + if (!fIncludeWatchonly && !hasSpendingKey) { + throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, spending key for address does not belong to wallet: ") + address); + } + zaddrs.insert(zaddr); if (setAddress.count(address)) { throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, duplicated address: ") + address);