Fix z_mergetoaddress sending from ANY_SPROUT/ANY_SAPLING when the wallet contains...

diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh
index d072898b26d2bb6127a564cbc0cd2372d92e8b8a..520c60837ef8065368d40af491f39b20716fe580 100755 (executable)
--- 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'
     'wallet_listnotes.py'
     'mergetoaddress_sprout.py'
     'mergetoaddress_sapling.py'

+    'mergetoaddress_mixednotes.py'

     'listtransactions.py'
     'mempool_resurrect_test.py'
     'txn_doublespend.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 cc829f8b8aad43f1304c21339dbe0c3a9e010b3e..962c2b7ff4976dce1ae7c6852758c5038a945c64 100755 (executable)
--- 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()
 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))
     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:
 
 
 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))
 
             "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'])
         # 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 (executable)
index 0000000..b787429
--- /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 4cac157be4dfe6671ceb319960a64095e1d3855f..c018c5410a6907e4320bd5aad65c7a6c1c747802 100644 (file)
--- 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");
         }
         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
         // 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");
             throw JSONRPCError(
                 RPC_INVALID_PARAMETER,
                 "Cannot send from both Sprout and Sapling addresses using z_mergetoaddress");