From a8055cfe10d501248dbdb515a12e51f7fd40ed20 Mon Sep 17 00:00:00 2001 From: Eirik0 Date: Thu, 21 Mar 2019 18:12:50 -0600 Subject: [PATCH 1/2] Fix z_mergetoaddress sending from ANY_SPROUT/ANY_SAPLING when the wallet contains both note types --- qa/pull-tester/rpc-tests.sh | 1 + qa/rpc-tests/mergetoaddress_helper.py | 8 ++- qa/rpc-tests/mergetoaddress_mixednotes.py | 82 +++++++++++++++++++++++ src/wallet/rpcwallet.cpp | 9 ++- 4 files changed, 98 insertions(+), 2 deletions(-) create mode 100755 qa/rpc-tests/mergetoaddress_mixednotes.py diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index d072898b2..520c60837 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -31,6 +31,7 @@ testScripts=( 'wallet_listnotes.py' 'mergetoaddress_sprout.py' 'mergetoaddress_sapling.py' + 'mergetoaddress_mixednotes.py' 'listtransactions.py' 'mempool_resurrect_test.py' 'txn_doublespend.py' diff --git a/qa/rpc-tests/mergetoaddress_helper.py b/qa/rpc-tests/mergetoaddress_helper.py index cc829f8b8..962c2b7ff 100755 --- a/qa/rpc-tests/mergetoaddress_helper.py +++ b/qa/rpc-tests/mergetoaddress_helper.py @@ -17,11 +17,12 @@ from decimal import Decimal def assert_mergetoaddress_exception(expected_error_msg, merge_to_address_lambda): try: merge_to_address_lambda() - fail("Expected exception: %s" % expected_error_msg) except JSONRPCException as e: assert_equal(expected_error_msg, e.error['message']) except Exception as e: fail("Expected JSONRPCException. Found %s" % repr(e)) + else: + fail("Expected exception: %s" % expected_error_msg) class MergeToAddressHelper: @@ -152,6 +153,11 @@ class MergeToAddressHelper: "Destination address is also the only source address, and all its funds are already merged.", lambda: test.nodes[0].z_mergetoaddress([mytaddr], mytaddr)) + # Merging will fail for this specific case where it would spend a fee and do nothing + assert_mergetoaddress_exception( + "Cannot send from both Sprout and Sapling addresses using z_mergetoaddress", + lambda: test.nodes[0].z_mergetoaddress(["ANY_SPROUT", "ANY_SAPLING"], mytaddr)) + # Merge UTXOs from node 0 of value 30, standard fee of 0.00010000 result = test.nodes[0].z_mergetoaddress([mytaddr, mytaddr2, mytaddr3], myzaddr) wait_and_assert_operationid_status(test.nodes[0], result['opid']) diff --git a/qa/rpc-tests/mergetoaddress_mixednotes.py b/qa/rpc-tests/mergetoaddress_mixednotes.py new file mode 100755 index 000000000..b787429aa --- /dev/null +++ b/qa/rpc-tests/mergetoaddress_mixednotes.py @@ -0,0 +1,82 @@ +#!/usr/bin/env python +# Copyright (c) 2019 The Zcash developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +import sys; assert sys.version_info < (3,), ur"This script does not run under Python 3. Please use Python 2.7.x." + +from decimal import Decimal +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal, get_coinbase_address, \ + initialize_chain_clean, start_nodes, wait_and_assert_operationid_status +from mergetoaddress_helper import assert_mergetoaddress_exception + + +class MergeToAddressMixedNotes(BitcoinTestFramework): + def setup_nodes(self): + return start_nodes(4, self.options.tmpdir, [[ + '-nuparams=5ba81b19:100', # Overwinter + '-nuparams=76b809bb:100', # Sapling + '-experimentalfeatures', '-zmergetoaddress' + ]] * 4) + + def setup_chain(self): + print("Initializing test directory " + self.options.tmpdir) + initialize_chain_clean(self.options.tmpdir, 4) + + def run_test(self): + print "Mining blocks..." + self.nodes[0].generate(102) + self.sync_all() + + # Send some ZEC to Sprout/Sapling addresses + coinbase_addr = get_coinbase_address(self.nodes[0]) + sproutAddr = self.nodes[0].z_getnewaddress('sprout') + saplingAddr = self.nodes[0].z_getnewaddress('sapling') + t_addr = self.nodes[1].getnewaddress() + + opid = self.nodes[0].z_sendmany(coinbase_addr, [{"address": sproutAddr, "amount": Decimal('10')}], 1, 0) + wait_and_assert_operationid_status(self.nodes[0], opid) + self.nodes[0].generate(1) + self.sync_all() + assert_equal(self.nodes[0].z_getbalance(sproutAddr), Decimal('10')) + assert_equal(self.nodes[0].z_getbalance(saplingAddr), Decimal('0')) + assert_equal(Decimal(self.nodes[1].z_gettotalbalance()["transparent"]), Decimal('0')) + # Make sure we cannot use "ANY_SPROUT" and "ANY_SAPLING" even if we only have Sprout Notes + assert_mergetoaddress_exception( + "Cannot send from both Sprout and Sapling addresses using z_mergetoaddress", + lambda: self.nodes[0].z_mergetoaddress(["ANY_SPROUT", "ANY_SAPLING"], t_addr)) + opid = self.nodes[0].z_sendmany(coinbase_addr, [{"address": saplingAddr, "amount": Decimal('10')}], 1, 0) + wait_and_assert_operationid_status(self.nodes[0], opid) + self.nodes[0].generate(1) + self.sync_all() + + assert_equal(Decimal(self.nodes[1].z_gettotalbalance()["transparent"]), Decimal('0')) + + # Merge Sprout -> taddr + result = self.nodes[0].z_mergetoaddress(["ANY_SPROUT"], t_addr, 0) + wait_and_assert_operationid_status(self.nodes[0], result['opid']) + self.nodes[0].generate(1) + self.sync_all() + + assert_equal(self.nodes[0].z_getbalance(sproutAddr), Decimal('0')) + assert_equal(self.nodes[0].z_getbalance(saplingAddr), Decimal('10')) + assert_equal(Decimal(self.nodes[1].z_gettotalbalance()["transparent"]), Decimal('10')) + + # Make sure we cannot use "ANY_SPROUT" and "ANY_SAPLING" even if we only have Sapling Notes + assert_mergetoaddress_exception( + "Cannot send from both Sprout and Sapling addresses using z_mergetoaddress", + lambda: self.nodes[0].z_mergetoaddress(["ANY_SPROUT", "ANY_SAPLING"], t_addr)) + # Merge Sapling -> taddr + result = self.nodes[0].z_mergetoaddress(["ANY_SAPLING"], t_addr, 0) + wait_and_assert_operationid_status(self.nodes[0], result['opid']) + self.nodes[0].generate(1) + self.sync_all() + + assert_equal(self.nodes[0].z_getbalance(sproutAddr), Decimal('0')) + assert_equal(self.nodes[0].z_getbalance(saplingAddr), Decimal('0')) + assert_equal(Decimal(self.nodes[1].z_gettotalbalance()["transparent"]), Decimal('20')) + + +if __name__ == '__main__': + MergeToAddressMixedNotes().main() diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 4cac157be..c018c5410 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4532,8 +4532,15 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) if (!saplingActive && saplingEntries.size() > 0) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, Sapling has not activated"); } + // Do not include Sprout/Sapling notes if using "ANY_SAPLING"/"ANY_SPROUT" respectively + if (useAnySprout) { + saplingEntries.clear(); + } + if (useAnySapling) { + sproutEntries.clear(); + } // Sending from both Sprout and Sapling is currently unsupported using z_mergetoaddress - if (sproutEntries.size() > 0 && saplingEntries.size() > 0) { + if ((sproutEntries.size() > 0 && saplingEntries.size() > 0) || (useAnySprout && useAnySapling)) { throw JSONRPCError( RPC_INVALID_PARAMETER, "Cannot send from both Sprout and Sapling addresses using z_mergetoaddress"); From cf30355fc3a29ce75059fd57f158a9f9e5982989 Mon Sep 17 00:00:00 2001 From: Eirik0 Date: Thu, 6 Jun 2019 16:46:06 -0600 Subject: [PATCH 2/2] Clarify what combinations of from addresses can be used in z_mergetoaddress --- src/wallet/rpcwallet.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index c018c5410..309446dc3 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4297,7 +4297,9 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) " - \"ANY_TADDR\": Merge UTXOs from any taddrs belonging to the wallet.\n" " - \"ANY_SPROUT\": Merge notes from any Sprout zaddrs belonging to the wallet.\n" " - \"ANY_SAPLING\": Merge notes from any Sapling zaddrs belonging to the wallet.\n" - " If a special string is given, any given addresses of that type will be counted as duplicates and cause an error.\n" + " While it is possible to use a variety of different combinations of addresses and the above values,\n" + " it is not possible to send funds from both sprout and sapling addresses simultaneously. If a special\n" + " string is given, any given addresses of that type will be counted as duplicates and cause an error.\n" " [\n" " \"address\" (string) Can be a taddr or a zaddr\n" " ,...\n"