diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index 13ddc461..821a637c 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -11,6 +11,7 @@ export BITCOIND=${REAL_BITCOIND} #Run the tests testScripts=( + 'wallet_treestate.py' 'wallet_protectcoinbase.py' 'wallet.py' 'wallet_nullifiers.py' diff --git a/qa/rpc-tests/wallet_treestate.py b/qa/rpc-tests/wallet_treestate.py new file mode 100755 index 00000000..ae55368f --- /dev/null +++ b/qa/rpc-tests/wallet_treestate.py @@ -0,0 +1,128 @@ +#!/usr/bin/env python2 +# Copyright (c) 2016 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 * +from time import * + +import sys + +class WalletTreeStateTest (BitcoinTestFramework): + + def setup_chain(self): + print("Initializing test directory "+self.options.tmpdir) + initialize_chain_clean(self.options.tmpdir, 4) + + # Start nodes with -regtestprotectcoinbase to set fCoinbaseMustBeProtected to true. + def setup_network(self, split=False): + self.nodes = start_nodes(3, self.options.tmpdir, extra_args=[['-regtestprotectcoinbase','-debug=zrpc']] * 3 ) + connect_nodes_bi(self.nodes,0,1) + connect_nodes_bi(self.nodes,1,2) + connect_nodes_bi(self.nodes,0,2) + self.is_network_split=False + self.sync_all() + + def wait_and_assert_operationid_status(self, myopid, in_status='success', in_errormsg=None): + print('waiting for async operation {}'.format(myopid)) + opids = [] + opids.append(myopid) + timeout = 300 + status = None + errormsg = None + for x in xrange(1, timeout): + results = self.nodes[0].z_getoperationresult(opids) + if len(results)==0: + sleep(1) + else: + status = results[0]["status"] + if status == "failed": + errormsg = results[0]['error']['message'] + break + print('...returned status: {}'.format(status)) + print('...error msg: {}'.format(errormsg)) + assert_equal(in_status, status) + if errormsg is not None: + assert(in_errormsg is not None) + assert_equal(in_errormsg in errormsg, True) + print('...returned error: {}'.format(errormsg)) + + def run_test (self): + print "Mining blocks..." + + self.nodes[0].generate(100) + self.sync_all() + self.nodes[1].generate(101) + self.sync_all() + + mytaddr = self.nodes[0].getnewaddress() # where coins were mined + myzaddr = self.nodes[0].z_getnewaddress() + + # Spend coinbase utxos to create three notes of 9.99990000 each + recipients = [] + recipients.append({"address":myzaddr, "amount":Decimal('10.0') - Decimal('0.0001')}) + myopid = self.nodes[0].z_sendmany(mytaddr, recipients) + self.wait_and_assert_operationid_status(myopid) + self.sync_all() + self.nodes[1].generate(1) + self.sync_all() + myopid = self.nodes[0].z_sendmany(mytaddr, recipients) + self.wait_and_assert_operationid_status(myopid) + self.sync_all() + self.nodes[1].generate(1) + self.sync_all() + myopid = self.nodes[0].z_sendmany(mytaddr, recipients) + self.wait_and_assert_operationid_status(myopid) + self.sync_all() + self.nodes[1].generate(1) + self.sync_all() + + # Check balance + resp = self.nodes[0].z_getbalance(myzaddr) + assert_equal(Decimal(resp), Decimal('9.9999') * 3 ) + + # We want to test a real-world situation where during the time spent creating a transaction + # with joinsplits, other transactions containing joinsplits have been mined into new blocks, + # which result in the treestate changing whilst creating the transaction. + + # Tx 1 will change the treestate while Tx 2 containing chained joinsplits is still being generated + recipients = [] + recipients.append({"address":self.nodes[2].z_getnewaddress(), "amount":Decimal('10.0') - Decimal('0.0001')}) + myopid = self.nodes[0].z_sendmany(mytaddr, recipients) + self.wait_and_assert_operationid_status(myopid) + + # Tx 2 will consume all three notes, which must take at least two joinsplits. This is regardless of + # the z_sendmany implementation because there are only two inputs per joinsplit. + recipients = [] + recipients.append({"address":self.nodes[2].z_getnewaddress(), "amount":Decimal('18')}) + recipients.append({"address":self.nodes[2].z_getnewaddress(), "amount":Decimal('11.9997') - Decimal('0.0001')}) + myopid = self.nodes[0].z_sendmany(myzaddr, recipients) + + # Wait for Tx 2 to begin executing... + for x in xrange(1, 60): + results = self.nodes[0].z_getoperationstatus([myopid]) + status = results[0]["status"] + if status == "executing": + break + sleep(1) + + # Now mine Tx 1 which will change global treestate before Tx 2's second joinsplit begins processing + self.sync_all() + self.nodes[1].generate(1) + self.sync_all() + + # Wait for Tx 2 to be created + self.wait_and_assert_operationid_status(myopid) + + # Note that a bug existed in v1.0.0-1.0.3 where Tx 2 creation would fail with an error: + # "Witness for spendable note does not have same anchor as change input" + + # Check balance + resp = self.nodes[0].z_getbalance(myzaddr) + assert_equal(Decimal(resp), Decimal('0.0')) + + +if __name__ == '__main__': + WalletTreeStateTest().main() diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index 9e27b971..ad3a415d 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -320,7 +320,22 @@ bool AsyncRPCOperation_sendmany::main_impl() { zOutputsDeque.push_back(o); } - + // When spending notes, take a snapshot of note witnesses and anchors as the treestate will + // change upon arrival of new blocks which contain joinsplit transactions. This is likely + // to happen as creating a chained joinsplit transaction can take longer than the block interval. + if (z_inputs_.size() > 0) { + LOCK2(cs_main, pwalletMain->cs_wallet); + for (auto t : z_inputs_) { + JSOutPoint jso = std::get<0>(t); + std::vector vOutPoints = { jso }; + uint256 inputAnchor; + std::vector> vInputWitnesses; + pwalletMain->GetNoteWitnesses(vOutPoints, vInputWitnesses, inputAnchor); + jsopWitnessAnchorMap[ jso.ToString() ] = WitnessAnchorData{ vInputWitnesses, inputAnchor }; + } + } + + /** * SCENARIO #2 * @@ -570,7 +585,9 @@ bool AsyncRPCOperation_sendmany::main_impl() { // std::vector vInputNotes; std::vector vOutPoints; + std::vector> vInputWitnesses; uint256 inputAnchor; + WitnessAnchorData wad; int numInputsNeeded = (jsChange>0) ? 1 : 0; while (numInputsNeeded++ < ZC_NUM_JS_INPUTS && zInputsDeque.size() > 0) { SendManyInputJSOP t = zInputsDeque.front(); @@ -579,6 +596,16 @@ bool AsyncRPCOperation_sendmany::main_impl() { CAmount noteFunds = std::get<2>(t); zInputsDeque.pop_front(); + wad = jsopWitnessAnchorMap[ jso.ToString() ]; + for (auto & w : wad.witnesses) { + vInputWitnesses.push_back(w); + } + if (inputAnchor.IsNull()) { + inputAnchor = wad.anchor; + } else if (inputAnchor != wad.anchor) { + throw JSONRPCError(RPC_WALLET_ERROR, "Selected input notes do not share the same anchor"); + } + vOutPoints.push_back(jso); vInputNotes.push_back(note); @@ -595,12 +622,7 @@ bool AsyncRPCOperation_sendmany::main_impl() { // Add history of previous commitments to witness if (vInputNotes.size() > 0) { - std::vector> vInputWitnesses; - { - LOCK(cs_main); - pwalletMain->GetNoteWitnesses(vOutPoints, vInputWitnesses, inputAnchor); - } - + if (vInputWitnesses.size()==0) { throw JSONRPCError(RPC_WALLET_ERROR, "Could not find witness for note commitment"); } diff --git a/src/wallet/asyncrpcoperation_sendmany.h b/src/wallet/asyncrpcoperation_sendmany.h index df9c3d8d..1c0cbbbf 100644 --- a/src/wallet/asyncrpcoperation_sendmany.h +++ b/src/wallet/asyncrpcoperation_sendmany.h @@ -14,6 +14,7 @@ #include "json/json_spirit_value.h" #include "wallet.h" +#include #include // TODO: Compute fee based on a heuristic, e.g. (num tx output * dust threshold) + joinsplit bytes * ? @@ -41,6 +42,12 @@ struct AsyncJoinSplitInfo CAmount vpub_new = 0; }; +// A struct to help us track the witnesses and anchor for a given JSOutPoint +struct WitnessAnchorData { + std::vector> witnesses; + uint256 anchor; +}; + class AsyncRPCOperation_sendmany : public AsyncRPCOperation { public: AsyncRPCOperation_sendmany(std::string fromAddress, std::vector tOutputs, std::vector zOutputs, int minDepth); @@ -70,7 +77,9 @@ private: uint256 joinSplitPubKey_; unsigned char joinSplitPrivKey_[crypto_sign_SECRETKEYBYTES]; - + // The key is the result string from calling JSOutPoint::ToString() + std::unordered_map jsopWitnessAnchorMap; + std::vector t_outputs_; std::vector z_outputs_; std::vector t_inputs_;