diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index 729c92b1d..6a2348012 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -15,6 +15,7 @@ testScripts=( 'prioritisetransaction.py' 'wallet_treestate.py' 'wallet_anchorfork.py' + 'wallet_changeindicator.py' 'wallet_protectcoinbase.py' 'wallet_shieldcoinbase.py' 'wallet_mergetoaddress.py' diff --git a/qa/rpc-tests/test_framework/util.py b/qa/rpc-tests/test_framework/util.py index ea6e8c832..79130c9bd 100644 --- a/qa/rpc-tests/test_framework/util.py +++ b/qa/rpc-tests/test_framework/util.py @@ -355,9 +355,18 @@ def random_transaction(nodes, amount, min_fee, fee_increment, fee_variants): return (txid, signresult["hex"], fee) -def assert_equal(thing1, thing2): - if thing1 != thing2: - raise AssertionError("%s != %s"%(str(thing1),str(thing2))) +def assert_equal(expected, actual, message = ""): + if expected != actual: + if message: + message = "%s; " % message + raise AssertionError("%sexpected: <%s> but was: <%s>" % (message, str(expected), str(actual))) + +def assert_true(condition, message = ""): + if not condition: + raise AssertionError(message) + +def assert_false(condition, message = ""): + assert_true(not condition, message) def assert_greater_than(thing1, thing2): if thing1 <= thing2: diff --git a/qa/rpc-tests/wallet_changeindicator.py b/qa/rpc-tests/wallet_changeindicator.py new file mode 100755 index 000000000..58b431fc2 --- /dev/null +++ b/qa/rpc-tests/wallet_changeindicator.py @@ -0,0 +1,64 @@ +#!/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, wait_and_assert_operationid_status + +from decimal import Decimal + +class WalletChangeIndicatorTest (BitcoinTestFramework): + # Helper Methods + def generate_and_sync(self): + self.sync_all() + self.nodes[0].generate(1) + self.sync_all() + + # Tests + def run_test(self): + taddr = self.nodes[1].getnewaddress() + zaddr1 = self.nodes[1].z_getnewaddress() + zaddr2 = self.nodes[1].z_getnewaddress() + + self.nodes[0].sendtoaddress(taddr, Decimal('1.0')) + self.generate_and_sync() + + # Send 1 ZEC to a zaddr + wait_and_assert_operationid_status(self.nodes[1], self.nodes[1].z_sendmany(taddr, [{'address': zaddr1, 'amount': 1.0, 'memo': 'c0ffee01'}], 1, 0)) + self.generate_and_sync() + + # Check that we have received 1 note which is not change + receivedbyaddress = self.nodes[1].z_listreceivedbyaddress(zaddr1, 0) + listunspent = self.nodes[1].z_listunspent() + assert_equal(1, len(receivedbyaddress), "Should have received 1 note") + assert_false(receivedbyaddress[0]['change'], "Note should not be change") + assert_equal(1, len(listunspent), "Should have 1 unspent note") + assert_false(listunspent[0]['change'], "Unspent note should not be change") + + # Generate some change + wait_and_assert_operationid_status(self.nodes[1], self.nodes[1].z_sendmany(zaddr1, [{'address': zaddr2, 'amount': 0.6, 'memo': 'c0ffee02'}], 1, 0)) + self.generate_and_sync() + + # Check zaddr1 received + sortedreceived1 = sorted(self.nodes[1].z_listreceivedbyaddress(zaddr1, 0), key = lambda received: received['amount']) + assert_equal(2, len(sortedreceived1), "zaddr1 Should have received 2 notes") + assert_equal(Decimal('0.4'), sortedreceived1[0]['amount']) + assert_true(sortedreceived1[0]['change'], "Note valued at 0.4 should be change") + assert_equal(Decimal('1.0'), sortedreceived1[1]['amount']) + assert_false(sortedreceived1[1]['change'], "Note valued at 1.0 should not be change") + # Check zaddr2 received + sortedreceived2 = sorted(self.nodes[1].z_listreceivedbyaddress(zaddr2, 0), key = lambda received: received['amount']) + assert_equal(1, len(sortedreceived2), "zaddr2 Should have received 1 notes") + assert_equal(Decimal('0.6'), sortedreceived2[0]['amount']) + assert_false(sortedreceived2[0]['change'], "Note valued at 0.6 should not be change") + # Check unspent + sortedunspent = sorted(self.nodes[1].z_listunspent(), key = lambda received: received['amount']) + assert_equal(2, len(sortedunspent), "Should have 2 unspent notes") + assert_equal(Decimal('0.4'), sortedunspent[0]['amount']) + assert_true(sortedunspent[0]['change'], "Unspent note valued at 0.4 should be change") + assert_equal(Decimal('0.6'), sortedunspent[1]['amount']) + assert_false(sortedunspent[1]['change'], "Unspent note valued at 0.6 should not be change") + +if __name__ == '__main__': + WalletChangeIndicatorTest().main() diff --git a/qa/rpc-tests/wallet_nullifiers.py b/qa/rpc-tests/wallet_nullifiers.py index 207631efb..db8940d3b 100755 --- a/qa/rpc-tests/wallet_nullifiers.py +++ b/qa/rpc-tests/wallet_nullifiers.py @@ -188,10 +188,23 @@ class WalletNullifiersTest (BitcoinTestFramework): assert_equal(myzaddr in self.nodes[3].z_listaddresses(), False) assert_equal(myzaddr in self.nodes[3].z_listaddresses(True), True) - # Node 3 should see the same received notes as node 2 - assert_equal( - self.nodes[2].z_listreceivedbyaddress(myzaddr), - self.nodes[3].z_listreceivedbyaddress(myzaddr)) + # Node 3 should see the same received notes as node 2; however, + # some of the notes were change for note 2 but not for note 3. + # Aside from that the recieved notes should be the same. So, + # group by txid and then check that all properties aside from + # change are equal. + node2Received = dict([r['txid'], r] for r in self.nodes[2].z_listreceivedbyaddress(myzaddr)) + node3Received = dict([r['txid'], r] for r in self.nodes[3].z_listreceivedbyaddress(myzaddr)) + assert_equal(len(node2Received), len(node2Received)) + for txid in node2Received: + received2 = node2Received[txid] + received3 = node3Received[txid] + assert_equal(len(received2), len(received3)) + for key in received2: + # check all the properties except for change + if key != 'change': + assert_equal(received2[key], received3[key]) + # Node 3's balances should be unchanged without explicitly requesting # to include watch-only balances diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index eca9b49e8..451a162d5 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2469,6 +2469,7 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) " \"address\" : \"address\", (string) the shielded address\n" " \"amount\": xxxxx, (numeric) the amount of value in the note\n" " \"memo\": xxxxx, (string) hexademical string representation of memo field\n" + " \"change\": true|false, (boolean) true if the address that received the note is also one of the sending addresses\n" " }\n" " ,...\n" "]\n" @@ -2553,9 +2554,10 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) if (zaddrs.size() > 0) { std::vector entries; pwalletMain->GetUnspentFilteredNotes(entries, zaddrs, nMinDepth, nMaxDepth, !fIncludeWatchonly); + std::set> nullifierSet = pwalletMain->GetNullifiersForAddresses(zaddrs); for (CUnspentSproutNotePlaintextEntry & entry : entries) { UniValue obj(UniValue::VOBJ); - obj.push_back(Pair("txid",entry.jsop.hash.ToString())); + obj.push_back(Pair("txid", entry.jsop.hash.ToString())); obj.push_back(Pair("jsindex", (int)entry.jsop.js )); obj.push_back(Pair("jsoutindex", (int)entry.jsop.n)); obj.push_back(Pair("confirmations", entry.nHeight)); @@ -2564,6 +2566,7 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) 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("change", pwalletMain->IsNoteChange(nullifierSet, entry.address, entry.jsop))); results.push_back(obj); } } @@ -3226,9 +3229,10 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp) "2. minconf (numeric, optional, default=1) Only include transactions confirmed at least this many times.\n" "\nResult:\n" "{\n" - " \"txid\": xxxxx, (string) the transaction id\n" - " \"amount\": xxxxx, (numeric) the amount of value in the note\n" - " \"memo\": xxxxx, (string) hexademical string representation of memo field\n" + " \"txid\": xxxxx, (string) the transaction id\n" + " \"amount\": xxxxx, (numeric) the amount of value in the note\n" + " \"memo\": xxxxx, (string) hexademical string representation of memo field\n" + " \"change\": true|false, (boolean) true if the address that received the note is also one of the sending addresses\n" "}\n" "\nExamples:\n" + HelpExampleCli("z_listreceivedbyaddress", "\"ztfaW34Gj9FrnGUEf833ywDVL62NWXBM81u6EQnM6VR45eYnXhwztecW1SjxA7JrmAXKJhxhj3vDNEpVCQoSvVoSpmbhtjf\"") @@ -3264,21 +3268,22 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp) UniValue result(UniValue::VARR); std::vector entries; pwalletMain->GetFilteredNotes(entries, fromaddress, nMinDepth, false, false); + std::set> nullifierSet = pwalletMain->GetNullifiersForAddresses({zaddr}); for (CSproutNotePlaintextEntry & entry : entries) { UniValue obj(UniValue::VOBJ); - obj.push_back(Pair("txid",entry.jsop.hash.ToString())); + 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)); + obj.push_back(Pair("change", pwalletMain->IsNoteChange(nullifierSet, entry.address, entry.jsop))); result.push_back(obj); } return result; } - UniValue z_getbalance(const UniValue& params, bool fHelp) { if (!EnsureWalletIsAvailable(fHelp)) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 420246f0b..4d57d04c2 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -465,6 +465,40 @@ void CWallet::SetBestChain(const CBlockLocator& loc) SetBestChainINTERNAL(walletdb, loc); } +std::set> CWallet::GetNullifiersForAddresses(const std::set & addresses) +{ + std::set> nullifierSet; + for (const auto & txPair : mapWallet) { + for (const auto & noteDataPair : txPair.second.mapNoteData) { + if (noteDataPair.second.nullifier && addresses.count(noteDataPair.second.address)) { + nullifierSet.insert(std::make_pair(noteDataPair.second.address, noteDataPair.second.nullifier.get())); + } + } + } + return nullifierSet; +} + +bool CWallet::IsNoteChange(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, + // for instance: + // - Change created by spending fractions of Notes (because + // z_sendmany sends change to the originating z-address). + // - "Chaining Notes" used to connect JoinSplits together. + // - Notes created by consolidation transactions (e.g. using + // z_mergetoaddress). + // - Notes sent from one address to itself. + for (const JSDescription & jsd : mapWallet[jsop.hash].vjoinsplit) { + for (const uint256 & nullifier : jsd.nullifiers) { + if (nullifierSet.count(std::make_pair(address, 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 3fe8e95e5..a473cbd17 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1074,6 +1074,8 @@ public: void ChainTip(const CBlockIndex *pindex, const CBlock *pblock, ZCIncrementalMerkleTree tree, bool added); /** 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); DBErrors LoadWallet(bool& fFirstRunRet); DBErrors ZapWalletTx(std::vector& vWtx);