From 86cf60b5c8b4225a2168038d0a00d65298b92cd2 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 20 Sep 2016 16:22:30 -0700 Subject: [PATCH] Fixes #1345 so that UTXO debit and credits are computed correctly for a transaction. --- qa/rpc-tests/wallet.py | 109 ++++++++++++++++++++++++++++++++++++++++- src/wallet/wallet.cpp | 76 ++++++++++++++++++++++++++-- 2 files changed, 178 insertions(+), 7 deletions(-) diff --git a/qa/rpc-tests/wallet.py b/qa/rpc-tests/wallet.py index 7f6645b99..f0f7c8ca9 100755 --- a/qa/rpc-tests/wallet.py +++ b/qa/rpc-tests/wallet.py @@ -6,6 +6,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import * +from time import * class WalletTest (BitcoinTestFramework): @@ -37,6 +38,9 @@ class WalletTest (BitcoinTestFramework): assert_equal(self.nodes[0].getbalance(), 40) assert_equal(self.nodes[1].getbalance(), 10) assert_equal(self.nodes[2].getbalance(), 0) + assert_equal(self.nodes[0].getbalance("*"), 40) + assert_equal(self.nodes[1].getbalance("*"), 10) + assert_equal(self.nodes[2].getbalance("*"), 0) # Send 21 BTC from 0 to 2 using sendtoaddress call. # Second transaction will be child of first, and will require a fee @@ -58,6 +62,8 @@ class WalletTest (BitcoinTestFramework): # minus the 21 plus fees sent to node2 assert_equal(self.nodes[0].getbalance(), 50-21) assert_equal(self.nodes[2].getbalance(), 21) + assert_equal(self.nodes[0].getbalance("*"), 50-21) + assert_equal(self.nodes[2].getbalance("*"), 21) # Node0 should have three unspent outputs. # Create a couple of transactions to send them to node2, submit them through @@ -87,6 +93,8 @@ class WalletTest (BitcoinTestFramework): assert_equal(self.nodes[0].getbalance(), 0) assert_equal(self.nodes[2].getbalance(), 50) assert_equal(self.nodes[2].getbalance("from1"), 50-21) + assert_equal(self.nodes[0].getbalance("*"), 0) + assert_equal(self.nodes[2].getbalance("*"), 50) # Send 10 BTC normal address = self.nodes[0].getnewaddress("test") @@ -96,6 +104,8 @@ class WalletTest (BitcoinTestFramework): self.sync_all() assert_equal(self.nodes[2].getbalance(), Decimal('39.99900000')) assert_equal(self.nodes[0].getbalance(), Decimal('10.00000000')) + assert_equal(self.nodes[2].getbalance("*"), Decimal('39.99900000')) + assert_equal(self.nodes[0].getbalance("*"), Decimal('10.00000000')) # Send 10 BTC with subtract fee from amount txid = self.nodes[2].sendtoaddress(address, 10, "", "", True) @@ -103,6 +113,8 @@ class WalletTest (BitcoinTestFramework): self.sync_all() assert_equal(self.nodes[2].getbalance(), Decimal('29.99900000')) assert_equal(self.nodes[0].getbalance(), Decimal('19.99900000')) + assert_equal(self.nodes[2].getbalance("*"), Decimal('29.99900000')) + assert_equal(self.nodes[0].getbalance("*"), Decimal('19.99900000')) # Sendmany 10 BTC txid = self.nodes[2].sendmany('from1', {address: 10}, 0, "", []) @@ -110,6 +122,8 @@ class WalletTest (BitcoinTestFramework): self.sync_all() assert_equal(self.nodes[2].getbalance(), Decimal('19.99800000')) assert_equal(self.nodes[0].getbalance(), Decimal('29.99900000')) + assert_equal(self.nodes[2].getbalance("*"), Decimal('19.99800000')) + assert_equal(self.nodes[0].getbalance("*"), Decimal('29.99900000')) # Sendmany 10 BTC with subtract fee from amount txid = self.nodes[2].sendmany('from1', {address: 10}, 0, "", [address]) @@ -117,6 +131,8 @@ class WalletTest (BitcoinTestFramework): self.sync_all() assert_equal(self.nodes[2].getbalance(), Decimal('9.99800000')) assert_equal(self.nodes[0].getbalance(), Decimal('39.99800000')) + assert_equal(self.nodes[2].getbalance("*"), Decimal('9.99800000')) + assert_equal(self.nodes[0].getbalance("*"), Decimal('39.99800000')) # Test ResendWalletTransactions: # Create a couple of transactions, then start up a fourth @@ -178,13 +194,15 @@ class WalletTest (BitcoinTestFramework): self.nodes[1].generate(1) #mine a block, tx should not be in there self.sync_all() assert_equal(self.nodes[2].getbalance(), Decimal('9.99800000')); #should not be changed because tx was not broadcasted - + assert_equal(self.nodes[2].getbalance("*"), Decimal('9.99800000')); #should not be changed because tx was not broadcasted + #now broadcast from another node, mine a block, sync, and check the balance self.nodes[1].sendrawtransaction(txObjNotBroadcasted['hex']) self.nodes[1].generate(1) self.sync_all() txObjNotBroadcasted = self.nodes[0].gettransaction(txIdNotBroadcasted) assert_equal(self.nodes[2].getbalance(), Decimal('11.99800000')); #should not be + assert_equal(self.nodes[2].getbalance("*"), Decimal('11.99800000')); #should not be #create another tx txIdNotBroadcasted = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 2); @@ -203,6 +221,93 @@ class WalletTest (BitcoinTestFramework): #tx should be added to balance because after restarting the nodes tx should be broadcastet assert_equal(self.nodes[2].getbalance(), Decimal('13.99800000')); #should not be - + assert_equal(self.nodes[2].getbalance("*"), Decimal('13.99800000')); #should not be + + # send from node 0 to node 2 taddr + mytaddr = self.nodes[2].getnewaddress(); + self.nodes[0].sendtoaddress(mytaddr, 10.0); + self.nodes[0].generate(1) + self.sync_all() + + mybalance = self.nodes[2].z_getbalance(mytaddr) + assert_equal(self.nodes[2].z_getbalance(mytaddr), Decimal('10.0')); + + # add zaddr to node 2 + # payment address: tneMWwNSjPkPaz7p5ed3XJbrz8XwpfBvaha3jaGU26EstNN8HKMYVyzgwMmVcmtaw7b5uuaF4Hr8P4UPZEMkuTkXQa8STzF + # spending key: TKWRfN47drnaFDbHBSYT2McbPjbmFjjTrGuntj3tAMnnGDg2Kp19 + self.nodes[2].z_importkey("TKWRfN47drnaFDbHBSYT2McbPjbmFjjTrGuntj3tAMnnGDg2Kp19") + myzaddr = "tneMWwNSjPkPaz7p5ed3XJbrz8XwpfBvaha3jaGU26EstNN8HKMYVyzgwMmVcmtaw7b5uuaF4Hr8P4UPZEMkuTkXQa8STzF" + + # send node 2 taddr to zaddr + recipients = [] + recipients.append({"address":myzaddr, "amount":7.0}) + myopid = self.nodes[2].z_sendmany(mytaddr, recipients) + + opids = [] + opids.append(myopid) + + timeout = 120 + status = None + for x in xrange(1, timeout): + results = self.nodes[2].z_getoperationresult(opids) + if len(results)==0: + sleep(1) + else: + status = results[0]["status"] + break + + assert_equal("success", status) + self.nodes[2].generate(1) + self.sync_all() + + # check balances + zsendmanynotevalue = Decimal('7.0') + zsendmanyfee = Decimal('0.0001') + node2utxobalance = Decimal('23.998') - zsendmanynotevalue - zsendmanyfee + + assert_equal(self.nodes[2].getbalance(), node2utxobalance) + assert_equal(self.nodes[2].getbalance("*"), node2utxobalance) + + # check zaddr balance + assert_equal(self.nodes[2].z_getbalance(myzaddr), zsendmanynotevalue); + + # check via z_gettotalbalance + resp = self.nodes[2].z_gettotalbalance() + assert_equal(Decimal(resp["transparent"]), node2utxobalance) + assert_equal(Decimal(resp["private"]), zsendmanynotevalue) + assert_equal(Decimal(resp["total"]), node2utxobalance + zsendmanynotevalue) + + + # send from private note to node 0 and node 2 + node0balance = self.nodes[0].getbalance() # 25.99794745 + node2balance = self.nodes[2].getbalance() # 16.99790000 + + recipients = [] + recipients.append({"address":self.nodes[0].getnewaddress(), "amount":1.0}) + recipients.append({"address":self.nodes[2].getnewaddress(), "amount":1.0}) + myopid = self.nodes[2].z_sendmany(myzaddr, recipients) + + status = None + opids = [] + opids.append(myopid) + for x in xrange(1, timeout): + results = self.nodes[2].z_getoperationresult(opids) + if len(results)==0: + sleep(1) + else: + status = results[0]["status"] + break + + assert_equal("success", status) + self.nodes[2].generate(1) + self.sync_all() + + node0balance += Decimal('1.0') + node2balance += Decimal('1.0') + assert_equal(Decimal(self.nodes[0].getbalance()), node0balance) + assert_equal(Decimal(self.nodes[0].getbalance("*")), node0balance) + assert_equal(Decimal(self.nodes[2].getbalance()), node2balance) + assert_equal(Decimal(self.nodes[2].getbalance("*")), node2balance) + if __name__ == '__main__': WalletTest ().main () diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 2e1449fc2..90e9ac56f 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1328,6 +1328,7 @@ int CWalletTx::GetRequestCount() const return nRequests; } +// GetAmounts will determine the transparent debits and credits for a given wallet tx. void CWalletTx::GetAmounts(list& listReceived, list& listSent, CAmount& nFee, string& strSentAccount, const isminefilter& filter) const { @@ -1336,12 +1337,77 @@ void CWalletTx::GetAmounts(list& listReceived, listSent.clear(); strSentAccount = strFromAccount; - // Compute fee: + // Is this tx sent/signed by me? CAmount nDebit = GetDebit(filter); - if (nDebit > 0) // debit>0 means we signed/sent this transaction - { - CAmount nValueOut = GetValueOut(); - nFee = nDebit - nValueOut; + bool isFromMyTaddr = nDebit > 0; // debit>0 means we signed/sent this transaction + + // Does this tx spend my notes? + bool isFromMyZaddr = false; + for (const JSDescription& js : vjoinsplit) { + for (const uint256& nullifier : js.nullifiers) { + if (pwallet->IsFromMe(nullifier)) { + isFromMyZaddr = true; + break; + } + } + if (isFromMyZaddr) { + break; + } + } + + // Compute fee if we sent this transaction. + if (isFromMyTaddr) { + CAmount nValueOut = GetValueOut(); // transparent outputs plus all vpub_old + CAmount nValueIn = 0; + for (const JSDescription & js : vjoinsplit) { + nValueIn += js.vpub_new; + } + nFee = nDebit - nValueOut + nValueIn; + } + + // Create output entry for vpub_old/new, if we sent utxos from this transaction + if (isFromMyTaddr) { + CAmount myVpubOld = 0; + CAmount myVpubNew = 0; + for (const JSDescription& js : vjoinsplit) { + bool fMyJSDesc = false; + + // Check input side + for (const uint256& nullifier : js.nullifiers) { + if (pwallet->IsFromMe(nullifier)) { + fMyJSDesc = true; + break; + } + } + + // Check output side + if (!fMyJSDesc) { + for (const std::pair nd : this->mapNoteData) { + if (nd.first.js < vjoinsplit.size() && nd.first.n < vjoinsplit[nd.first.js].ciphertexts.size()) { + fMyJSDesc = true; + break; + } + } + } + + if (fMyJSDesc) { + myVpubOld += js.vpub_old; + myVpubNew += js.vpub_new; + } + + if (!MoneyRange(js.vpub_old) || !MoneyRange(js.vpub_new) || !MoneyRange(myVpubOld) || !MoneyRange(myVpubNew)) { + throw std::runtime_error("CWalletTx::GetAmounts: value out of range"); + } + } + + // Create an output for the value taken from or added to the transparent value pool by JoinSplits + if (myVpubOld > myVpubNew) { + COutputEntry output = {CNoDestination(), myVpubOld - myVpubNew, (int)vout.size()}; + listSent.push_back(output); + } else if (myVpubNew > myVpubOld) { + COutputEntry output = {CNoDestination(), myVpubNew - myVpubOld, (int)vout.size()}; + listReceived.push_back(output); + } } // Sent/received.