diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index 1f25465fa99..dde4144c45a 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -19,6 +19,7 @@ testScripts=( 'wallet_import_export.py' 'wallet_protectcoinbase.py' 'wallet_shieldcoinbase.py' + 'wallet_listreceived.py' 'wallet_mergetoaddress.py' 'wallet.py' 'wallet_overwintertx.py' diff --git a/qa/rpc-tests/wallet_listreceived.py b/qa/rpc-tests/wallet_listreceived.py new file mode 100755 index 00000000000..88d4f8cc7df --- /dev/null +++ b/qa/rpc-tests/wallet_listreceived.py @@ -0,0 +1,101 @@ +#!/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, assert_true, assert_false +from test_framework.util import start_nodes, wait_and_assert_operationid_status +from decimal import Decimal + +my_memo = 'c0ffee' # stay awake +my_memo = my_memo + '0'*(1024-len(my_memo)) + +no_memo = 'f6' + ('0'*1022) # see section 5.5 of the protocol spec +# sapling generates zero_memo, but this may be fixed soon (to no_memo) +# then this test can be simplified +zero_memo = '0'*1024 + +fee = Decimal('0.0001') + +class ListReceivedTest (BitcoinTestFramework): + + def setup_nodes(self): + return start_nodes(4, self.options.tmpdir, [[ + "-nuparams=5ba81b19:201", # Overwinter + "-nuparams=76b809bb:204", # Sapling + ]] * 4) + + def generate_and_sync(self, new_height): + self.nodes[0].generate(1) + self.sync_all() + assert_equal(new_height, self.nodes[0].getblockcount()) + + def run_test_release(self, release, expected_memo, height): + self.generate_and_sync(height+1) + taddr = self.nodes[1].getnewaddress() + zaddr1 = self.nodes[1].z_getnewaddress(release) + + self.nodes[0].sendtoaddress(taddr, 2.0) + self.generate_and_sync(height+2) + + # Send 1 ZEC to zaddr1 + opid = self.nodes[1].z_sendmany(taddr, + [{'address': zaddr1, 'amount': 1, 'memo': my_memo}]) + txid = wait_and_assert_operationid_status(self.nodes[1], opid) + self.sync_all() + r = self.nodes[1].z_listreceivedbyaddress(zaddr1) + assert_equal(0, len(r), "Should have received no confirmed note") + + # No confirmation required, one note should be present + r = self.nodes[1].z_listreceivedbyaddress(zaddr1, 0) + assert_equal(1, len(r), "Should have received one (unconfirmed) note") + assert_equal(txid, r[0]['txid']) + assert_equal(1, r[0]['amount']) + assert_false(r[0]['change'], "Note should not be change") + assert_equal(my_memo, r[0]['memo']) + + # Confirm transaction (1 ZEC from taddr to zaddr1) + self.generate_and_sync(height+3) + + # Require one confirmation, note should be present + assert_equal(r, self.nodes[1].z_listreceivedbyaddress(zaddr1)) + + # Generate some change by sending part of zaddr1 to zaddr2 + zaddr2 = self.nodes[1].z_getnewaddress(release) + opid = self.nodes[1].z_sendmany(zaddr1, + [{'address': zaddr2, 'amount': 0.6, 'memo': my_memo}]) + txid = wait_and_assert_operationid_status(self.nodes[1], opid) + self.sync_all() + self.generate_and_sync(height+4) + + # zaddr1 should have a note with change + r = self.nodes[1].z_listreceivedbyaddress(zaddr1, 0) + r = sorted(r, key = lambda received: received['amount']) + assert_equal(2, len(r), "zaddr1 Should have received 2 notes") + + assert_equal(txid, r[0]['txid']) + assert_equal(Decimal('0.4')-fee, r[0]['amount']) + assert_true(r[0]['change'], "Note valued at (0.4-fee) should be change") + assert_equal(expected_memo, r[0]['memo']) + + # The old note still exists (it's immutable), even though it is spent + assert_equal(Decimal('1.0'), r[1]['amount']) + assert_false(r[1]['change'], "Note valued at 1.0 should not be change") + assert_equal(expected_memo, r[0]['memo']) + + # zaddr2 should not have change + r = self.nodes[1].z_listreceivedbyaddress(zaddr2, 0) + r = sorted(r, key = lambda received: received['amount']) + assert_equal(1, len(r), "zaddr2 Should have received 1 notes") + assert_equal(txid, r[0]['txid']) + assert_equal(Decimal('0.6'), r[0]['amount']) + assert_false(r[0]['change'], "Note valued at 0.6 should not be change") + assert_equal(my_memo, r[0]['memo']) + + def run_test(self): + self.run_test_release('sprout', no_memo, 200) + self.run_test_release('sapling', zero_memo, 204) + +if __name__ == '__main__': + ListReceivedTest().main() diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 82b98304a5b..d932799861a 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2579,7 +2579,7 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) std::string data(entry.plaintext.memo().begin(), entry.plaintext.memo().end()); obj.push_back(Pair("memo", HexStr(data))); if (hasSproutSpendingKey) { - obj.push_back(Pair("change", pwalletMain->IsNoteChange(nullifierSet, entry.address, entry.jsop))); + obj.push_back(Pair("change", pwalletMain->IsNoteSproutChange(nullifierSet, entry.address, entry.jsop))); } results.push_back(obj); } @@ -3307,12 +3307,9 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp) if (!IsValidPaymentAddress(zaddr)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid zaddr."); } - // TODO: Add Sapling support. For now, ensure we can freely convert. - assert(boost::get(&zaddr) != nullptr); - auto sproutzaddr = boost::get(zaddr); - bool hasSproutSpendingKey = pwalletMain->HaveSproutSpendingKey(sproutzaddr); - if (!(hasSproutSpendingKey || pwalletMain->HaveSproutViewingKey(sproutzaddr))) { + // Visitor to support Sprout and Sapling addrs + if (!boost::apply_visitor(PaymentAddressBelongsToWallet(pwalletMain), zaddr)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "From address does not belong to this node, zaddr spending key or viewing key not found."); } @@ -3320,22 +3317,40 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp) std::vector sproutEntries; std::vector saplingEntries; pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, fromaddress, nMinDepth, false, false); - auto nullifierSet = hasSproutSpendingKey ? pwalletMain->GetNullifiersForAddresses({zaddr}) : std::set>(); - for (CSproutNotePlaintextEntry & entry : sproutEntries) { - UniValue obj(UniValue::VOBJ); - obj.push_back(Pair("txid", entry.jsop.hash.ToString())); - obj.push_back(Pair("amount", ValueFromAmount(CAmount(entry.plaintext.value())))); - std::string data(entry.plaintext.memo().begin(), entry.plaintext.memo().end()); - obj.push_back(Pair("memo", HexStr(data))); - // (txid, jsindex, jsoutindex) is needed to globally identify a note - obj.push_back(Pair("jsindex", entry.jsop.js)); - obj.push_back(Pair("jsoutindex", entry.jsop.n)); - if (hasSproutSpendingKey) { - obj.push_back(Pair("change", pwalletMain->IsNoteChange(nullifierSet, entry.address, entry.jsop))); + + std::set> nullifierSet; + auto hasSpendingKey = boost::apply_visitor(HaveSpendingKeyForPaymentAddress(pwalletMain), zaddr); + if (hasSpendingKey) { + nullifierSet = pwalletMain->GetNullifiersForAddresses({zaddr}); + } + + if (boost::get(&zaddr) != nullptr) { + for (CSproutNotePlaintextEntry & entry : sproutEntries) { + UniValue obj(UniValue::VOBJ); + obj.push_back(Pair("txid", entry.jsop.hash.ToString())); + obj.push_back(Pair("amount", ValueFromAmount(CAmount(entry.plaintext.value())))); + std::string data(entry.plaintext.memo().begin(), entry.plaintext.memo().end()); + obj.push_back(Pair("memo", HexStr(data))); + obj.push_back(Pair("jsindex", entry.jsop.js)); + obj.push_back(Pair("jsoutindex", entry.jsop.n)); + if (hasSpendingKey) { + obj.push_back(Pair("change", pwalletMain->IsNoteSproutChange(nullifierSet, entry.address, entry.jsop))); + } + result.push_back(obj); + } + } else if (boost::get(&zaddr) != nullptr) { + for (SaplingNoteEntry & entry : saplingEntries) { + UniValue obj(UniValue::VOBJ); + obj.push_back(Pair("txid", entry.op.hash.ToString())); + obj.push_back(Pair("amount", ValueFromAmount(CAmount(entry.note.value())))); + obj.push_back(Pair("memo", HexStr(entry.memo))); + obj.push_back(Pair("outindex", (int)entry.op.n)); + if (hasSpendingKey) { + obj.push_back(Pair("change", pwalletMain->IsNoteSaplingChange(nullifierSet, entry.address, entry.op))); + } + result.push_back(obj); } - result.push_back(obj); } - // TODO: Sapling return result; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6e2631db363..d838f485e3a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -512,20 +512,50 @@ void CWallet::SetBestChain(const CBlockLocator& loc) SetBestChainINTERNAL(walletdb, loc); } -std::set> CWallet::GetNullifiersForAddresses(const std::set & addresses) +std::set> CWallet::GetNullifiersForAddresses( + const std::set & addresses) { std::set> nullifierSet; + // Sapling ivk -> list of addrs map + // (There may be more than one diversified address for a given ivk.) + std::map> ivkMap; + for (const auto & addr : addresses) { + auto saplingAddr = boost::get(&addr); + if (saplingAddr != nullptr) { + libzcash::SaplingIncomingViewingKey ivk; + this->GetSaplingIncomingViewingKey(*saplingAddr, ivk); + ivkMap[ivk].push_back(*saplingAddr); + } + } for (const auto & txPair : mapWallet) { + // Sprout for (const auto & noteDataPair : txPair.second.mapSproutNoteData) { - if (noteDataPair.second.nullifier && addresses.count(noteDataPair.second.address)) { - nullifierSet.insert(std::make_pair(noteDataPair.second.address, noteDataPair.second.nullifier.get())); + auto & noteData = noteDataPair.second; + auto & nullifier = noteData.nullifier; + auto & address = noteData.address; + if (nullifier && addresses.count(address)) { + nullifierSet.insert(std::make_pair(address, nullifier.get())); + } + } + // Sapling + for (const auto & noteDataPair : txPair.second.mapSaplingNoteData) { + auto & noteData = noteDataPair.second; + auto & nullifier = noteData.nullifier; + auto & ivk = noteData.ivk; + if (nullifier && ivkMap.count(ivk)) { + for (const auto & addr : ivkMap[ivk]) { + nullifierSet.insert(std::make_pair(addr, nullifier.get())); + } } } } return nullifierSet; } -bool CWallet::IsNoteChange(const std::set> & nullifierSet, const PaymentAddress & address, const JSOutPoint & jsop) +bool CWallet::IsNoteSproutChange( + const std::set> & nullifierSet, + const PaymentAddress & address, + const JSOutPoint & jsop) { // A Note is marked as "change" if the address that received it // also spent Notes in the same transaction. This will catch, @@ -546,6 +576,26 @@ bool CWallet::IsNoteChange(const std::set> & nullifierSet, + const libzcash::PaymentAddress & address, + const SaplingOutPoint & op) +{ + // A Note is marked as "change" if the address that received it + // also spent Notes in the same transaction. This will catch, + // for instance: + // - Change created by spending fractions of Notes (because + // z_sendmany sends change to the originating z-address). + // - Notes created by consolidation transactions (e.g. using + // z_mergetoaddress). + // - Notes sent from one address to itself. + for (const SpendDescription &spend : mapWallet[op.hash].vShieldedSpend) { + if (nullifierSet.count(std::make_pair(address, spend.nullifier))) { + return true; + } + } + return false; +} + bool CWallet::SetMinVersion(enum WalletFeature nVersion, CWalletDB* pwalletdbIn, bool fExplicit) { LOCK(cs_wallet); // nWalletVersion diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 50acb239479..3589f9629fb 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1161,7 +1161,8 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface /** Saves witness caches and best block locator to disk. */ void SetBestChain(const CBlockLocator& loc); std::set> GetNullifiersForAddresses(const std::set & addresses); - bool IsNoteChange(const std::set> & nullifierSet, const libzcash::PaymentAddress & address, const JSOutPoint & entry); + bool IsNoteSproutChange(const std::set> & nullifierSet, const libzcash::PaymentAddress & address, const JSOutPoint & entry); + bool IsNoteSaplingChange(const std::set> & nullifierSet, const libzcash::PaymentAddress & address, const SaplingOutPoint & entry); DBErrors LoadWallet(bool& fFirstRunRet); DBErrors ZapWalletTx(std::vector& vWtx);