diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index f53e0448c..8a2f9cbb1 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -21,7 +21,6 @@ testScripts=( 'wallet_shieldcoinbase_sprout.py' 'wallet_shieldcoinbase_sapling.py' 'wallet_listreceived.py' - 'wallet_mergetoaddress.py' 'wallet.py' 'wallet_overwintertx.py' 'wallet_persistence.py' @@ -30,6 +29,8 @@ testScripts=( 'wallet_addresses.py' 'wallet_sapling.py' 'wallet_listnotes.py' + 'mergetoaddress_sprout.py' + 'mergetoaddress_sapling.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 new file mode 100755 index 000000000..cc829f8b8 --- /dev/null +++ b/qa/rpc-tests/mergetoaddress_helper.py @@ -0,0 +1,366 @@ +# 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. + +# +# Common code for testing z_mergetoaddress before and after sapling activation +# + +from test_framework.authproxy import JSONRPCException +from test_framework.util import assert_equal, connect_nodes_bi, fail, \ + initialize_chain_clean, start_node, sync_blocks, sync_mempools, \ + wait_and_assert_operationid_status + +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)) + + +class MergeToAddressHelper: + + def __init__(self, addr_type, any_zaddr, utxos_to_generate, utxos_in_tx1, utxos_in_tx2, test_mempooltxinputlimit): + self.addr_type = addr_type + self.any_zaddr = [any_zaddr] + self.any_zaddr_or_utxo = [any_zaddr, "ANY_TADDR"] + # utxos_to_generate, utxos_in_tx1, utxos_in_tx2 have to do with testing transaction size limits + self.utxos_to_generate = utxos_to_generate + self.utxos_in_tx1 = utxos_in_tx1 + self.utxos_in_tx2 = utxos_in_tx2 + self.test_mempooltxinputlimit = test_mempooltxinputlimit + + def setup_chain(self, test): + print("Initializing test directory "+test.options.tmpdir) + initialize_chain_clean(test.options.tmpdir, 4) + + def setup_network(self, test, additional_args=[]): + args = ['-debug=zrpcunsafe', '-experimentalfeatures', '-zmergetoaddress'] + args += additional_args + test.nodes = [] + test.nodes.append(start_node(0, test.options.tmpdir, args)) + test.nodes.append(start_node(1, test.options.tmpdir, args)) + args2 = ['-debug=zrpcunsafe', '-experimentalfeatures', '-zmergetoaddress', '-mempooltxinputlimit=7'] + args2 += additional_args + test.nodes.append(start_node(2, test.options.tmpdir, args2)) + connect_nodes_bi(test.nodes, 0, 1) + connect_nodes_bi(test.nodes, 1, 2) + connect_nodes_bi(test.nodes, 0, 2) + test.is_network_split = False + test.sync_all() + + def run_test(self, test): + print "Mining blocks..." + + test.nodes[0].generate(1) + do_not_shield_taddr = test.nodes[0].getnewaddress() + + test.nodes[0].generate(4) + walletinfo = test.nodes[0].getwalletinfo() + assert_equal(walletinfo['immature_balance'], 50) + assert_equal(walletinfo['balance'], 0) + test.sync_all() + test.nodes[2].generate(1) + test.nodes[2].getnewaddress() + test.nodes[2].generate(1) + test.nodes[2].getnewaddress() + test.nodes[2].generate(1) + test.sync_all() + test.nodes[1].generate(101) + test.sync_all() + assert_equal(test.nodes[0].getbalance(), 50) + assert_equal(test.nodes[1].getbalance(), 10) + assert_equal(test.nodes[2].getbalance(), 30) + + # Shield the coinbase + myzaddr = test.nodes[0].z_getnewaddress(self.addr_type) + result = test.nodes[0].z_shieldcoinbase("*", myzaddr, 0) + wait_and_assert_operationid_status(test.nodes[0], result['opid']) + test.sync_all() + test.nodes[1].generate(1) + test.sync_all() + + # Prepare some UTXOs and notes for merging + mytaddr = test.nodes[0].getnewaddress() + mytaddr2 = test.nodes[0].getnewaddress() + mytaddr3 = test.nodes[0].getnewaddress() + result = test.nodes[0].z_sendmany(myzaddr, [ + {'address': do_not_shield_taddr, 'amount': 10}, + {'address': mytaddr, 'amount': 10}, + {'address': mytaddr2, 'amount': 10}, + {'address': mytaddr3, 'amount': 10}, + ], 1, 0) + wait_and_assert_operationid_status(test.nodes[0], result) + test.sync_all() + test.nodes[1].generate(1) + test.sync_all() + + # Merging will fail because from arguments need to be in an array + assert_mergetoaddress_exception( + "JSON value is not an array as expected", + lambda: test.nodes[0].z_mergetoaddress("notanarray", myzaddr)) + + # Merging will fail when trying to spend from watch-only address + test.nodes[2].importaddress(mytaddr) + assert_mergetoaddress_exception( + "Could not find any funds to merge.", + lambda: test.nodes[2].z_mergetoaddress([mytaddr], myzaddr)) + + # Merging will fail because fee is negative + assert_mergetoaddress_exception( + "Amount out of range", + lambda: test.nodes[0].z_mergetoaddress(self.any_zaddr_or_utxo, myzaddr, -1)) + + # Merging will fail because fee is larger than MAX_MONEY + assert_mergetoaddress_exception( + "Amount out of range", + lambda: test.nodes[0].z_mergetoaddress(self.any_zaddr_or_utxo, myzaddr, Decimal('21000000.00000001'))) + + # Merging will fail because fee is larger than sum of UTXOs + assert_mergetoaddress_exception( + "Insufficient funds, have 50.00, which is less than miners fee 999.00", + lambda: test.nodes[0].z_mergetoaddress(self.any_zaddr_or_utxo, myzaddr, 999)) + + # Merging will fail because transparent limit parameter must be at least 0 + assert_mergetoaddress_exception( + "Limit on maximum number of UTXOs cannot be negative", + lambda: test.nodes[0].z_mergetoaddress(self.any_zaddr_or_utxo, myzaddr, Decimal('0.001'), -1)) + + # Merging will fail because transparent limit parameter is absurdly large + assert_mergetoaddress_exception( + "JSON integer out of range", + lambda: test.nodes[0].z_mergetoaddress(self.any_zaddr_or_utxo, myzaddr, Decimal('0.001'), 99999999999999)) + + # Merging will fail because shielded limit parameter must be at least 0 + assert_mergetoaddress_exception( + "Limit on maximum number of notes cannot be negative", + lambda: test.nodes[0].z_mergetoaddress(self.any_zaddr_or_utxo, myzaddr, Decimal('0.001'), 50, -1)) + + # Merging will fail because shielded limit parameter is absurdly large + assert_mergetoaddress_exception( + "JSON integer out of range", + lambda: test.nodes[0].z_mergetoaddress(self.any_zaddr_or_utxo, myzaddr, Decimal('0.001'), 50, 99999999999999)) + + # Merging will fail for this specific case where it would spend a fee and do nothing + assert_mergetoaddress_exception( + "Destination address is also the only source address, and all its funds are already merged.", + lambda: test.nodes[0].z_mergetoaddress([mytaddr], 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']) + test.sync_all() + test.nodes[1].generate(1) + test.sync_all() + + # Confirm balances and that do_not_shield_taddr containing funds of 10 was left alone + assert_equal(test.nodes[0].getbalance(), 10) + assert_equal(test.nodes[0].z_getbalance(do_not_shield_taddr), Decimal('10.0')) + assert_equal(test.nodes[0].z_getbalance(myzaddr), Decimal('39.99990000')) + assert_equal(test.nodes[1].getbalance(), 40) + assert_equal(test.nodes[2].getbalance(), 30) + + # Shield all notes to another z-addr + myzaddr2 = test.nodes[0].z_getnewaddress(self.addr_type) + result = test.nodes[0].z_mergetoaddress(self.any_zaddr, myzaddr2, 0) + assert_equal(result["mergingUTXOs"], Decimal('0')) + assert_equal(result["remainingUTXOs"], Decimal('0')) + assert_equal(result["mergingNotes"], Decimal('2')) + assert_equal(result["remainingNotes"], Decimal('0')) + wait_and_assert_operationid_status(test.nodes[0], result['opid']) + test.sync_all() + blockhash = test.nodes[1].generate(1) + test.sync_all() + + assert_equal(len(test.nodes[0].getblock(blockhash[0])['tx']), 2) + assert_equal(test.nodes[0].z_getbalance(myzaddr), 0) + assert_equal(test.nodes[0].z_getbalance(myzaddr2), Decimal('39.99990000')) + + # Shield coinbase UTXOs from any node 2 taddr, and set fee to 0 + result = test.nodes[2].z_shieldcoinbase("*", myzaddr, 0) + wait_and_assert_operationid_status(test.nodes[2], result['opid']) + test.sync_all() + test.nodes[1].generate(1) + test.sync_all() + + assert_equal(test.nodes[0].getbalance(), 10) + assert_equal(test.nodes[0].z_getbalance(myzaddr), Decimal('30')) + assert_equal(test.nodes[0].z_getbalance(myzaddr2), Decimal('39.99990000')) + assert_equal(test.nodes[1].getbalance(), 60) + assert_equal(test.nodes[2].getbalance(), 0) + + # Merge all notes from node 0 into a node 0 taddr, and set fee to 0 + result = test.nodes[0].z_mergetoaddress(self.any_zaddr, mytaddr, 0) + wait_and_assert_operationid_status(test.nodes[0], result['opid']) + test.sync_all() + test.nodes[1].generate(1) + test.sync_all() + + assert_equal(test.nodes[0].getbalance(), Decimal('79.99990000')) + assert_equal(test.nodes[0].z_getbalance(do_not_shield_taddr), Decimal('10.0')) + assert_equal(test.nodes[0].z_getbalance(mytaddr), Decimal('69.99990000')) + assert_equal(test.nodes[0].z_getbalance(myzaddr), 0) + assert_equal(test.nodes[0].z_getbalance(myzaddr2), 0) + assert_equal(test.nodes[1].getbalance(), 70) + assert_equal(test.nodes[2].getbalance(), 0) + + # Merge all node 0 UTXOs together into a node 1 taddr, and set fee to 0 + test.nodes[1].getnewaddress() # Ensure we have an empty address + n1taddr = test.nodes[1].getnewaddress() + result = test.nodes[0].z_mergetoaddress(["ANY_TADDR"], n1taddr, 0) + wait_and_assert_operationid_status(test.nodes[0], result['opid']) + test.sync_all() + test.nodes[1].generate(1) + test.sync_all() + + assert_equal(test.nodes[0].getbalance(), 0) + assert_equal(test.nodes[0].z_getbalance(do_not_shield_taddr), 0) + assert_equal(test.nodes[0].z_getbalance(mytaddr), 0) + assert_equal(test.nodes[0].z_getbalance(myzaddr), 0) + assert_equal(test.nodes[1].getbalance(), Decimal('159.99990000')) + assert_equal(test.nodes[1].z_getbalance(n1taddr), Decimal('79.99990000')) + assert_equal(test.nodes[2].getbalance(), 0) + + # Generate self.utxos_to_generate regular UTXOs on node 0, and 20 regular UTXOs on node 2 + mytaddr = test.nodes[0].getnewaddress() + n2taddr = test.nodes[2].getnewaddress() + test.nodes[1].generate(1000) + test.sync_all() + for i in range(self.utxos_to_generate): + test.nodes[1].sendtoaddress(mytaddr, 1) + for i in range(20): + test.nodes[1].sendtoaddress(n2taddr, 1) + test.nodes[1].generate(1) + test.sync_all() + + # Merging the UTXOs will conditionally occur over two transactions, since max tx size is 100,000 bytes before Sapling and 2,000,000 after. + # We don't verify mergingTransparentValue as UTXOs are not selected in any specific order, so value can change on each test run. + # We set an unrealistically high limit parameter of 99999, to verify that max tx size will constrain the number of UTXOs. + result = test.nodes[0].z_mergetoaddress([mytaddr], myzaddr, 0, 99999) + assert_equal(result["mergingUTXOs"], self.utxos_in_tx1) + assert_equal(result["remainingUTXOs"], self.utxos_in_tx2) + assert_equal(result["mergingNotes"], Decimal('0')) + assert_equal(result["mergingShieldedValue"], Decimal('0')) + assert_equal(result["remainingNotes"], Decimal('0')) + assert_equal(result["remainingShieldedValue"], Decimal('0')) + remainingTransparentValue = result["remainingTransparentValue"] + wait_and_assert_operationid_status(test.nodes[0], result['opid']) + + # For sapling we do not check that this occurs over two transactions because of the time that it would take + if self.utxos_in_tx2 > 0: + # Verify that UTXOs are locked (not available for selection) by queuing up another merging operation + result = test.nodes[0].z_mergetoaddress([mytaddr], myzaddr, 0, 0) + assert_equal(result["mergingUTXOs"], self.utxos_in_tx2) + assert_equal(result["mergingTransparentValue"], Decimal(remainingTransparentValue)) + assert_equal(result["remainingUTXOs"], Decimal('0')) + assert_equal(result["remainingTransparentValue"], Decimal('0')) + assert_equal(result["mergingNotes"], Decimal('0')) + assert_equal(result["mergingShieldedValue"], Decimal('0')) + assert_equal(result["remainingNotes"], Decimal('0')) + assert_equal(result["remainingShieldedValue"], Decimal('0')) + wait_and_assert_operationid_status(test.nodes[0], result['opid']) + + # sync_all() invokes sync_mempool() but node 2's mempool limit will cause tx1 and tx2 to be rejected. + # So instead, we sync on blocks and mempool for node 0 and node 1, and after a new block is generated + # which mines tx1 and tx2, all nodes will have an empty mempool which can then be synced. + sync_blocks(test.nodes[:2]) + sync_mempools(test.nodes[:2]) + # Generate enough blocks to ensure all transactions are mined + while test.nodes[1].getmempoolinfo()['size'] > 0: + test.nodes[1].generate(1) + test.sync_all() + + # Verify maximum number of UTXOs which node 2 can shield is limited by option -mempooltxinputlimit + # This option is used when the limit parameter is set to 0. + + # -mempooltxinputlimit is not used after overwinter activation + if self.test_mempooltxinputlimit: + expected_to_merge = 7 + expected_remaining = 13 + else: + expected_to_merge = 20 + expected_remaining = 0 + + result = test.nodes[2].z_mergetoaddress([n2taddr], myzaddr, Decimal('0.0001'), 0) + assert_equal(result["mergingUTXOs"], expected_to_merge) + assert_equal(result["remainingUTXOs"], expected_remaining) + assert_equal(result["mergingNotes"], Decimal('0')) + assert_equal(result["remainingNotes"], Decimal('0')) + wait_and_assert_operationid_status(test.nodes[2], result['opid']) + test.sync_all() + test.nodes[1].generate(1) + test.sync_all() + + # Verify maximum number of UTXOs which node 0 can shield is set by default limit parameter of 50 + mytaddr = test.nodes[0].getnewaddress() + for i in range(100): + test.nodes[1].sendtoaddress(mytaddr, 1) + test.nodes[1].generate(1) + test.sync_all() + result = test.nodes[0].z_mergetoaddress([mytaddr], myzaddr, Decimal('0.0001')) + assert_equal(result["mergingUTXOs"], Decimal('50')) + assert_equal(result["remainingUTXOs"], Decimal('50')) + assert_equal(result["mergingNotes"], Decimal('0')) + # Remaining notes are only counted if we are trying to merge any notes + assert_equal(result["remainingNotes"], Decimal('0')) + wait_and_assert_operationid_status(test.nodes[0], result['opid']) + + # Verify maximum number of UTXOs which node 0 can shield can be set by the limit parameter + result = test.nodes[0].z_mergetoaddress([mytaddr], myzaddr, Decimal('0.0001'), 33) + assert_equal(result["mergingUTXOs"], Decimal('33')) + assert_equal(result["remainingUTXOs"], Decimal('17')) + assert_equal(result["mergingNotes"], Decimal('0')) + # Remaining notes are only counted if we are trying to merge any notes + assert_equal(result["remainingNotes"], Decimal('0')) + wait_and_assert_operationid_status(test.nodes[0], result['opid']) + # Don't sync node 2 which rejects the tx due to its mempooltxinputlimit + sync_blocks(test.nodes[:2]) + sync_mempools(test.nodes[:2]) + test.nodes[1].generate(1) + test.sync_all() + + # Verify maximum number of notes which node 0 can shield can be set by the limit parameter + # Also check that we can set off a second merge before the first one is complete + + # myzaddr will have 5 notes if testing before to Sapling activation and 4 otherwise + num_notes = len(test.nodes[0].z_listunspent(0)) + result1 = test.nodes[0].z_mergetoaddress([myzaddr], myzaddr, 0.0001, 50, 2) + result2 = test.nodes[0].z_mergetoaddress([myzaddr], myzaddr, 0.0001, 50, 2) + + # First merge should select from all notes + assert_equal(result1["mergingUTXOs"], Decimal('0')) + # Remaining UTXOs are only counted if we are trying to merge any UTXOs + assert_equal(result1["remainingUTXOs"], Decimal('0')) + assert_equal(result1["mergingNotes"], Decimal('2')) + assert_equal(result1["remainingNotes"], num_notes - 2) + + # Second merge should ignore locked notes + assert_equal(result2["mergingUTXOs"], Decimal('0')) + assert_equal(result2["remainingUTXOs"], Decimal('0')) + assert_equal(result2["mergingNotes"], Decimal('2')) + assert_equal(result2["remainingNotes"], num_notes - 4) + wait_and_assert_operationid_status(test.nodes[0], result1['opid']) + wait_and_assert_operationid_status(test.nodes[0], result2['opid']) + + test.sync_all() + test.nodes[1].generate(1) + test.sync_all() + + # Shield both UTXOs and notes to a z-addr + result = test.nodes[0].z_mergetoaddress(self.any_zaddr_or_utxo, myzaddr, 0, 10, 2) + assert_equal(result["mergingUTXOs"], Decimal('10')) + assert_equal(result["remainingUTXOs"], Decimal('7')) + assert_equal(result["mergingNotes"], Decimal('2')) + assert_equal(result["remainingNotes"], num_notes - 4) + wait_and_assert_operationid_status(test.nodes[0], result['opid']) + # Don't sync node 2 which rejects the tx due to its mempooltxinputlimit + sync_blocks(test.nodes[:2]) + sync_mempools(test.nodes[:2]) + test.nodes[1].generate(1) + test.sync_all() diff --git a/qa/rpc-tests/mergetoaddress_sapling.py b/qa/rpc-tests/mergetoaddress_sapling.py new file mode 100755 index 000000000..70930cc25 --- /dev/null +++ b/qa/rpc-tests/mergetoaddress_sapling.py @@ -0,0 +1,29 @@ +#!/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 mergetoaddress_helper import MergeToAddressHelper + + +class MergeToAddressSapling (BitcoinTestFramework): + # 13505 would be the maximum number of utxos based on the transaction size limits for Sapling + # but testing this causes the test to take an indeterminately long time to run. + helper = MergeToAddressHelper('sapling', 'ANY_SAPLING', 800, 800, 0, False) + + def setup_chain(self): + self.helper.setup_chain(self) + + def setup_network(self, split=False): + self.helper.setup_network(self, [ + '-nuparams=5ba81b19:100', # Overwinter + '-nuparams=76b809bb:100', # Sapling + ]) + + def run_test(self): + self.helper.run_test(self) + + +if __name__ == '__main__': + MergeToAddressSapling().main() diff --git a/qa/rpc-tests/mergetoaddress_sprout.py b/qa/rpc-tests/mergetoaddress_sprout.py new file mode 100755 index 000000000..6d7471a9b --- /dev/null +++ b/qa/rpc-tests/mergetoaddress_sprout.py @@ -0,0 +1,24 @@ +#!/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 mergetoaddress_helper import MergeToAddressHelper + + +class MergeToAddressSprout (BitcoinTestFramework): + helper = MergeToAddressHelper('sprout', 'ANY_SPROUT', 800, 662, 138, True) + + def setup_chain(self): + self.helper.setup_chain(self) + + def setup_network(self, split=False): + self.helper.setup_network(self) + + def run_test(self): + self.helper.run_test(self) + + +if __name__ == '__main__': + MergeToAddressSprout().main() diff --git a/qa/rpc-tests/test_framework/util.py b/qa/rpc-tests/test_framework/util.py index 1a6315ef5..0a57e6ddf 100644 --- a/qa/rpc-tests/test_framework/util.py +++ b/qa/rpc-tests/test_framework/util.py @@ -382,6 +382,9 @@ def assert_raises(exc, fun, *args, **kwds): else: raise AssertionError("No exception raised") +def fail(message=""): + raise AssertionError(message) + # Returns txid if operation was a success or None def wait_and_assert_operationid_status(node, myopid, in_status='success', in_errormsg=None, timeout=300): print('waiting for async operation {}'.format(myopid)) diff --git a/qa/rpc-tests/wallet_sapling.py b/qa/rpc-tests/wallet_sapling.py index 1abb1e8dc..82a151361 100755 --- a/qa/rpc-tests/wallet_sapling.py +++ b/qa/rpc-tests/wallet_sapling.py @@ -59,12 +59,15 @@ class WalletSaplingTest(BitcoinTestFramework): self.nodes[3].z_mergetoaddress([tmp_taddr], tmp_zaddr) raise AssertionError("Should have thrown an exception") except JSONRPCException as e: - assert_equal("Invalid parameter, Sapling is not supported yet by z_mergetoadress", e.error['message']) + assert_equal("Invalid parameter, Sapling has not activated", e.error['message']) try: self.nodes[3].z_mergetoaddress([tmp_zaddr], tmp_taddr) raise AssertionError("Should have thrown an exception") except JSONRPCException as e: - assert_equal("Invalid parameter, Sapling is not supported yet by z_mergetoadress", e.error['message']) + # When sending from a zaddr we check for sapling activation only if + # we find notes belonging to that address. Since sapling is not active + # none can be generated and none will be found. + assert_equal("Could not find any funds to merge.", e.error['message']) # Activate Sapling self.nodes[2].generate(2) diff --git a/src/test/rpc_wallet_tests.cpp b/src/test/rpc_wallet_tests.cpp index 4f1c1fb1a..b3ad832d2 100644 --- a/src/test/rpc_wallet_tests.cpp +++ b/src/test/rpc_wallet_tests.cpp @@ -1670,82 +1670,102 @@ BOOST_AUTO_TEST_CASE(rpc_z_shieldcoinbase_internals) } +void CheckRPCThrows(std::string rpcString, std::string expectedErrorMessage) { + try { + CallRPC(rpcString); + // Note: CallRPC catches (const UniValue& objError) and rethrows a runtime_error + BOOST_FAIL("Should have caused an error"); + } catch (const std::runtime_error& e) { + BOOST_CHECK_EQUAL(expectedErrorMessage, e.what()); + } catch(const std::exception& e) { + BOOST_FAIL(std::string("Unexpected exception: ") + typeid(e).name() + ", message=\"" + e.what() + "\""); + } +} + BOOST_AUTO_TEST_CASE(rpc_z_mergetoaddress_parameters) { SelectParams(CBaseChainParams::TESTNET); LOCK(pwalletMain->cs_wallet); + CheckRPCThrows("z_mergetoaddress 1 2", + "Error: z_mergetoaddress is disabled."); + + // Set global state required for z_mergetoaddress + fExperimentalMode = true; + mapArgs["-zmergetoaddress"] = "1"; + BOOST_CHECK_THROW(CallRPC("z_mergetoaddress"), runtime_error); BOOST_CHECK_THROW(CallRPC("z_mergetoaddress toofewargs"), runtime_error); BOOST_CHECK_THROW(CallRPC("z_mergetoaddress just too many args present for this method"), runtime_error); - // bad from address - BOOST_CHECK_THROW(CallRPC("z_mergetoaddress " - "[\"INVALIDtmRr6yJonqGK23UVhrKuyvTpF8qxQQjKigJ\"] tnpoQJVnYBZZqkFadj2bJJLThNCxbADGB5gSGeYTAGGrT5tejsxY9Zc1BtY8nnHmZkB"), runtime_error); + std::string taddr1 = "tmRr6yJonqGK23UVhrKuyvTpF8qxQQjKigJ"; + std::string taddr2 = "tmYmhvdKqEte49iohoB9utgL1kPbGgWSdNc"; + std::string aSproutAddr = "ztVtBC7vJFXPsZC8S3hXRu51rZysoJkSe6r1t9wk56bELrV9xTK6dx5TgSCH6RTw1dRD7HuApmcY1nhuQW9QfvE4MQXRRYU"; + std::string aSaplingAddr = "ztestsapling19rnyu293v44f0kvtmszhx35lpdug574twc0lwyf4s7w0umtkrdq5nfcauxrxcyfmh3m7slemqsj"; + + CheckRPCThrows("z_mergetoaddress [] " + taddr1, + "Invalid parameter, fromaddresses array is empty."); // bad from address - BOOST_CHECK_THROW(CallRPC("z_mergetoaddress " - "** tnpoQJVnYBZZqkFadj2bJJLThNCxbADGB5gSGeYTAGGrT5tejsxY9Zc1BtY8nnHmZkB"), runtime_error); + CheckRPCThrows("z_mergetoaddress [\"INVALID" + taddr1 + "\"] " + taddr2, + "Unknown address format: INVALID" + taddr1); // bad from address - BOOST_CHECK_THROW(CallRPC("z_mergetoaddress " - "[\"**\"] tnpoQJVnYBZZqkFadj2bJJLThNCxbADGB5gSGeYTAGGrT5tejsxY9Zc1BtY8nnHmZkB"), runtime_error); + CheckRPCThrows("z_mergetoaddress ** " + taddr2, + "Error parsing JSON:**"); // bad from address - BOOST_CHECK_THROW(CallRPC("z_mergetoaddress " - "tmRr6yJonqGK23UVhrKuyvTpF8qxQQjKigJ tnpoQJVnYBZZqkFadj2bJJLThNCxbADGB5gSGeYTAGGrT5tejsxY9Zc1BtY8nnHmZkB"), runtime_error); + CheckRPCThrows("z_mergetoaddress [\"**\"] " + taddr2, + "Unknown address format: **"); // bad from address - BOOST_CHECK_THROW(CallRPC("z_mergetoaddress " - "[tmRr6yJonqGK23UVhrKuyvTpF8qxQQjKigJ] tnpoQJVnYBZZqkFadj2bJJLThNCxbADGB5gSGeYTAGGrT5tejsxY9Zc1BtY8nnHmZkB"), runtime_error); + CheckRPCThrows("z_mergetoaddress " + taddr1 + " " + taddr2, + "Error parsing JSON:" + taddr1); + + // bad from address + CheckRPCThrows("z_mergetoaddress [" + taddr1 + "] " + taddr2, + "Error parsing JSON:[" + taddr1 + "]"); // bad to address - BOOST_CHECK_THROW(CallRPC("z_mergetoaddress " - "[\"tmRr6yJonqGK23UVhrKuyvTpF8qxQQjKigJ\"] INVALIDtnpoQJVnYBZZqkFadj2bJJLThNCxbADGB5gSGeYTAGGrT5tejsxY9Zc1BtY8nnHmZkB"), runtime_error); + CheckRPCThrows("z_mergetoaddress [\"" + taddr1 + "\"] INVALID" + taddr2, + "Invalid parameter, unknown address format: INVALID" + taddr2); // duplicate address - BOOST_CHECK_THROW(CallRPC("z_mergetoaddress " - "[\"tmRr6yJonqGK23UVhrKuyvTpF8qxQQjKigJ\", \"tmRr6yJonqGK23UVhrKuyvTpF8qxQQjKigJ\"] " - "tmQP9L3s31cLsghVYf2Jb5MhKj1jRBPoeQn" - ), runtime_error); + CheckRPCThrows("z_mergetoaddress [\"" + taddr1 + "\",\"" + taddr1 + "\"] " + taddr2, + "Invalid parameter, duplicated address: " + taddr1); // invalid fee amount, cannot be negative - BOOST_CHECK_THROW(CallRPC("z_mergetoaddress " - "[\"tmRr6yJonqGK23UVhrKuyvTpF8qxQQjKigJ\"] " - "tnpoQJVnYBZZqkFadj2bJJLThNCxbADGB5gSGeYTAGGrT5tejsxY9Zc1BtY8nnHmZkB " - "-0.0001" - ), runtime_error); + CheckRPCThrows("z_mergetoaddress [\"" + taddr1 + "\"] " + taddr2 + " -0.0001", + "Amount out of range"); // invalid fee amount, bigger than MAX_MONEY - BOOST_CHECK_THROW(CallRPC("z_mergetoaddress " - "[\"tmRr6yJonqGK23UVhrKuyvTpF8qxQQjKigJ\"] " - "tnpoQJVnYBZZqkFadj2bJJLThNCxbADGB5gSGeYTAGGrT5tejsxY9Zc1BtY8nnHmZkB " - "21000001" - ), runtime_error); + CheckRPCThrows("z_mergetoaddress [\"" + taddr1 + "\"] " + taddr2 + " 21000001", + "Amount out of range"); // invalid transparent limit, must be at least 0 - BOOST_CHECK_THROW(CallRPC("z_mergetoaddress " - "[\"tmRr6yJonqGK23UVhrKuyvTpF8qxQQjKigJ\"] " - "tnpoQJVnYBZZqkFadj2bJJLThNCxbADGB5gSGeYTAGGrT5tejsxY9Zc1BtY8nnHmZkB " - "0.0001 -1" - ), runtime_error); + CheckRPCThrows("z_mergetoaddress [\"" + taddr1 + "\"] " + taddr2 + " 0.0001 -1", + "Limit on maximum number of UTXOs cannot be negative"); // invalid shielded limit, must be at least 0 - BOOST_CHECK_THROW(CallRPC("z_mergetoaddress " - "[\"tmRr6yJonqGK23UVhrKuyvTpF8qxQQjKigJ\"] " - "tnpoQJVnYBZZqkFadj2bJJLThNCxbADGB5gSGeYTAGGrT5tejsxY9Zc1BtY8nnHmZkB " - "0.0001 100 -1" - ), runtime_error); + CheckRPCThrows("z_mergetoaddress [\"" + taddr1 + "\"] " + taddr2 + " 0.0001 100 -1", + "Limit on maximum number of notes cannot be negative"); + + CheckRPCThrows("z_mergetoaddress [\"ANY_TADDR\",\"" + taddr1 + "\"] " + taddr2, + "Cannot specify specific t-addrs when using \"ANY_TADDR\""); + + CheckRPCThrows("z_mergetoaddress [\"ANY_SPROUT\",\"" + aSproutAddr + "\"] " + taddr2, + "Cannot specify specific z-addrs when using \"ANY_SPROUT\" or \"ANY_SAPLING\""); + + CheckRPCThrows("z_mergetoaddress [\"ANY_SAPLING\",\"" + aSaplingAddr + "\"] " + taddr2, + "Cannot specify specific z-addrs when using \"ANY_SPROUT\" or \"ANY_SAPLING\""); // memo bigger than allowed length of ZC_MEMO_SIZE std::vector v (2 * (ZC_MEMO_SIZE+1)); // x2 for hexadecimal string format std::fill(v.begin(),v.end(), 'A'); std::string badmemo(v.begin(), v.end()); - auto pa = pwalletMain->GenerateNewSproutZKey(); - std::string zaddr1 = EncodePaymentAddress(pa); - BOOST_CHECK_THROW(CallRPC(string("z_mergetoaddress [\"tmRr6yJonqGK23UVhrKuyvTpF8qxQQjKigJ\"] ") - + zaddr1 + " 0.0001 100 100 " + badmemo), runtime_error); + CheckRPCThrows("z_mergetoaddress [\"" + taddr1 + "\"] " + aSproutAddr + " 0.0001 100 100 " + badmemo, + "Invalid parameter, size of memo is larger than maximum allowed 512"); // Mutable tx containing contextual information we need to build tx UniValue retValue = CallRPC("getblockcount"); @@ -1761,37 +1781,61 @@ BOOST_AUTO_TEST_CASE(rpc_z_mergetoaddress_parameters) "mainnet memo"); try { - std::shared_ptr operation(new AsyncRPCOperation_mergetoaddress(mtx, {}, {}, testnetzaddr, -1 )); + std::shared_ptr operation(new AsyncRPCOperation_mergetoaddress(boost::none, mtx, {}, {}, {}, testnetzaddr, -1 )); BOOST_FAIL("Should have caused an error"); } catch (const UniValue& objError) { BOOST_CHECK( find_error(objError, "Fee is out of range")); } try { - std::shared_ptr operation(new AsyncRPCOperation_mergetoaddress(mtx, {}, {}, testnetzaddr, 1)); + std::shared_ptr operation(new AsyncRPCOperation_mergetoaddress(boost::none, mtx, {}, {}, {}, testnetzaddr, 1)); BOOST_FAIL("Should have caused an error"); } catch (const UniValue& objError) { BOOST_CHECK( find_error(objError, "No inputs")); } - std::vector inputs = { MergeToAddressInputUTXO{ COutPoint{uint256(), 0}, 0} }; + std::vector inputs = { MergeToAddressInputUTXO{ COutPoint{uint256(), 0}, 0, CScript()} }; try { MergeToAddressRecipient badaddr("", "memo"); - std::shared_ptr operation(new AsyncRPCOperation_mergetoaddress(mtx, inputs, {}, badaddr, 1)); + std::shared_ptr operation(new AsyncRPCOperation_mergetoaddress(boost::none, mtx, inputs, {}, {}, badaddr, 1)); BOOST_FAIL("Should have caused an error"); } catch (const UniValue& objError) { BOOST_CHECK( find_error(objError, "Recipient parameter missing")); } + std::vector sproutNoteInputs = + {MergeToAddressInputSproutNote{JSOutPoint(), SproutNote(), 0, SproutSpendingKey()}}; + std::vector saplingNoteInputs = + {MergeToAddressInputSaplingNote{SaplingOutPoint(), SaplingNote(), 0, SaplingExpandedSpendingKey()}}; + + // Sprout and Sapling inputs -> throw + try { + auto operation = new AsyncRPCOperation_mergetoaddress(boost::none, mtx, inputs, sproutNoteInputs, saplingNoteInputs, testnetzaddr, 1); + BOOST_FAIL("Should have caused an error"); + } catch (const UniValue& objError) { + BOOST_CHECK(find_error(objError, "Cannot send from both Sprout and Sapling addresses using z_mergetoaddress")); + } + // Sprout inputs and TransactionBuilder -> throw + try { + auto operation = new AsyncRPCOperation_mergetoaddress(TransactionBuilder(), mtx, inputs, sproutNoteInputs, {}, testnetzaddr, 1); + BOOST_FAIL("Should have caused an error"); + } catch (const UniValue& objError) { + BOOST_CHECK(find_error(objError, "Sprout notes are not supported by the TransactionBuilder")); + } + // Testnet payment addresses begin with 'zt'. This test detects an incorrect prefix. try { - std::vector inputs = { MergeToAddressInputUTXO{ COutPoint{uint256(), 0}, 0} }; - std::shared_ptr operation( new AsyncRPCOperation_mergetoaddress(mtx, inputs, {}, mainnetzaddr, 1) ); + std::vector inputs = { MergeToAddressInputUTXO{ COutPoint{uint256(), 0}, 0, CScript()} }; + std::shared_ptr operation( new AsyncRPCOperation_mergetoaddress(boost::none, mtx, inputs, {}, {}, mainnetzaddr, 1) ); BOOST_FAIL("Should have caused an error"); } catch (const UniValue& objError) { BOOST_CHECK( find_error(objError, "Invalid recipient address")); } + + // Un-set global state + fExperimentalMode = false; + mapArgs.erase("-zmergetoaddress"); } @@ -1819,10 +1863,10 @@ BOOST_AUTO_TEST_CASE(rpc_z_mergetoaddress_internals) // Supply 2 inputs when mempool limit is 1 { std::vector inputs = { - MergeToAddressInputUTXO{COutPoint{uint256(),0},0}, - MergeToAddressInputUTXO{COutPoint{uint256(),0},0} + MergeToAddressInputUTXO{COutPoint{uint256(),0},0, CScript()}, + MergeToAddressInputUTXO{COutPoint{uint256(),0},0, CScript()} }; - std::shared_ptr operation( new AsyncRPCOperation_mergetoaddress(mtx, inputs, {}, zaddr1) ); + std::shared_ptr operation( new AsyncRPCOperation_mergetoaddress(boost::none, mtx, inputs, {}, {}, zaddr1) ); operation->main(); BOOST_CHECK(operation->isFailed()); std::string msg = operation->getErrorMessage(); @@ -1831,8 +1875,8 @@ BOOST_AUTO_TEST_CASE(rpc_z_mergetoaddress_internals) // Insufficient funds { - std::vector inputs = { MergeToAddressInputUTXO{COutPoint{uint256(),0},0} }; - std::shared_ptr operation( new AsyncRPCOperation_mergetoaddress(mtx, inputs, {}, zaddr1) ); + std::vector inputs = { MergeToAddressInputUTXO{COutPoint{uint256(),0},0, CScript()} }; + std::shared_ptr operation( new AsyncRPCOperation_mergetoaddress(boost::none, mtx, inputs, {}, {}, zaddr1) ); operation->main(); BOOST_CHECK(operation->isFailed()); std::string msg = operation->getErrorMessage(); @@ -1841,8 +1885,8 @@ BOOST_AUTO_TEST_CASE(rpc_z_mergetoaddress_internals) // get_memo_from_hex_string()) { - std::vector inputs = { MergeToAddressInputUTXO{COutPoint{uint256(),0},100000} }; - std::shared_ptr operation( new AsyncRPCOperation_mergetoaddress(mtx, inputs, {}, zaddr1) ); + std::vector inputs = { MergeToAddressInputUTXO{COutPoint{uint256(),0},100000, CScript()} }; + std::shared_ptr operation( new AsyncRPCOperation_mergetoaddress(boost::none, mtx, inputs, {}, {}, zaddr1) ); std::shared_ptr ptr = std::dynamic_pointer_cast (operation); TEST_FRIEND_AsyncRPCOperation_mergetoaddress proxy(ptr); @@ -1895,8 +1939,8 @@ BOOST_AUTO_TEST_CASE(rpc_z_mergetoaddress_internals) // Test the perform_joinsplit methods. { // Dummy input so the operation object can be instantiated. - std::vector inputs = { MergeToAddressInputUTXO{COutPoint{uint256(),0},100000} }; - std::shared_ptr operation( new AsyncRPCOperation_mergetoaddress(mtx, inputs, {}, zaddr1) ); + std::vector inputs = { MergeToAddressInputUTXO{COutPoint{uint256(),0},100000, CScript()} }; + std::shared_ptr operation( new AsyncRPCOperation_mergetoaddress(boost::none, mtx, inputs, {}, {}, zaddr1) ); std::shared_ptr ptr = std::dynamic_pointer_cast (operation); TEST_FRIEND_AsyncRPCOperation_mergetoaddress proxy(ptr); @@ -1956,8 +2000,8 @@ BOOST_AUTO_TEST_CASE(rpc_z_mergetoaddress_internals) obj.push_back(Pair("rawtxn", raw)); // we have the spending key for the dummy recipient zaddr1 - std::vector inputs = { MergeToAddressInputUTXO{COutPoint{uint256(),0},100000} }; - std::shared_ptr operation( new AsyncRPCOperation_mergetoaddress(mtx, inputs, {}, zaddr1) ); + std::vector inputs = { MergeToAddressInputUTXO{COutPoint{uint256(),0},100000, CScript()} }; + std::shared_ptr operation( new AsyncRPCOperation_mergetoaddress(boost::none, mtx, inputs, {}, {}, zaddr1) ); std::shared_ptr ptr = std::dynamic_pointer_cast (operation); TEST_FRIEND_AsyncRPCOperation_mergetoaddress proxy(ptr); diff --git a/src/wallet/asyncrpcoperation_mergetoaddress.cpp b/src/wallet/asyncrpcoperation_mergetoaddress.cpp index 80968e01c..af48cedd2 100644 --- a/src/wallet/asyncrpcoperation_mergetoaddress.cpp +++ b/src/wallet/asyncrpcoperation_mergetoaddress.cpp @@ -34,6 +34,8 @@ using namespace libzcash; +extern UniValue sendrawtransaction(const UniValue& params, bool fHelp); + int mta_find_output(UniValue obj, int n) { UniValue outputMapValue = find_value(obj, "outputmap"); @@ -53,20 +55,22 @@ int mta_find_output(UniValue obj, int n) } AsyncRPCOperation_mergetoaddress::AsyncRPCOperation_mergetoaddress( + boost::optional builder, CMutableTransaction contextualTx, std::vector utxoInputs, - std::vector noteInputs, + std::vector sproutNoteInputs, + std::vector saplingNoteInputs, MergeToAddressRecipient recipient, CAmount fee, UniValue contextInfo) : - tx_(contextualTx), utxoInputs_(utxoInputs), noteInputs_(noteInputs), - recipient_(recipient), fee_(fee), contextinfo_(contextInfo) + tx_(contextualTx), utxoInputs_(utxoInputs), sproutNoteInputs_(sproutNoteInputs), + saplingNoteInputs_(saplingNoteInputs), recipient_(recipient), fee_(fee), contextinfo_(contextInfo) { if (fee < 0 || fee > MAX_MONEY) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Fee is out of range"); } - if (utxoInputs.empty() && noteInputs.empty()) { + if (utxoInputs.empty() && sproutNoteInputs.empty() && saplingNoteInputs.empty()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "No inputs"); } @@ -74,6 +78,20 @@ AsyncRPCOperation_mergetoaddress::AsyncRPCOperation_mergetoaddress( throw JSONRPCError(RPC_INVALID_PARAMETER, "Recipient parameter missing"); } + if (sproutNoteInputs.size() > 0 && saplingNoteInputs.size() > 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot send from both Sprout and Sapling addresses using z_mergetoaddress"); + } + + if (sproutNoteInputs.size() > 0 && builder) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Sprout notes are not supported by the TransactionBuilder"); + } + + isUsingBuilder_ = false; + if (builder) { + isUsingBuilder_ = true; + builder_ = builder.get(); + } + toTaddr_ = DecodeDestination(std::get<0>(recipient)); isToTaddr_ = IsValidDestination(toTaddr_); isToZaddr_ = false; @@ -82,10 +100,6 @@ AsyncRPCOperation_mergetoaddress::AsyncRPCOperation_mergetoaddress( auto address = DecodePaymentAddress(std::get<0>(recipient)); if (IsValidPaymentAddress(address)) { isToZaddr_ = true; - // TODO: Add Sapling support. For now, return an error to the user. - if (boost::get(&address) == nullptr) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Currently, only Sprout zaddrs are supported"); - } toPaymentAddress_ = address; } else { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid recipient address"); @@ -203,7 +217,7 @@ bool AsyncRPCOperation_mergetoaddress::main_impl() { assert(isToTaddr_ != isToZaddr_); - bool isPureTaddrOnlyTx = (noteInputs_.empty() && isToTaddr_); + bool isPureTaddrOnlyTx = (sproutNoteInputs_.empty() && saplingNoteInputs_.empty() && isToTaddr_); CAmount minersFee = fee_; size_t numInputs = utxoInputs_.size(); @@ -228,7 +242,11 @@ bool AsyncRPCOperation_mergetoaddress::main_impl() } CAmount z_inputs_total = 0; - for (MergeToAddressInputNote& t : noteInputs_) { + for (const MergeToAddressInputSproutNote& t : sproutNoteInputs_) { + z_inputs_total += std::get<2>(t); + } + + for (const MergeToAddressInputSaplingNote& t : saplingNoteInputs_) { z_inputs_total += std::get<2>(t); } @@ -243,17 +261,19 @@ bool AsyncRPCOperation_mergetoaddress::main_impl() CAmount sendAmount = targetAmount - minersFee; // update the transaction with the UTXO inputs and output (if any) - CMutableTransaction rawTx(tx_); - for (MergeToAddressInputUTXO& t : utxoInputs_) { - CTxIn in(std::get<0>(t)); - rawTx.vin.push_back(in); + if (!isUsingBuilder_) { + CMutableTransaction rawTx(tx_); + for (const MergeToAddressInputUTXO& t : utxoInputs_) { + CTxIn in(std::get<0>(t)); + rawTx.vin.push_back(in); + } + if (isToTaddr_) { + CScript scriptPubKey = GetScriptForDestination(toTaddr_); + CTxOut out(sendAmount, scriptPubKey); + rawTx.vout.push_back(out); + } + tx_ = CTransaction(rawTx); } - if (isToTaddr_) { - CScript scriptPubKey = GetScriptForDestination(toTaddr_); - CTxOut out(sendAmount, scriptPubKey); - rawTx.vout.push_back(out); - } - tx_ = CTransaction(rawTx); LogPrint(isPureTaddrOnlyTx ? "zrpc" : "zrpcunsafe", "%s: spending %s to send %s with fee %s\n", getId(), FormatMoney(targetAmount), FormatMoney(sendAmount), FormatMoney(minersFee)); @@ -272,6 +292,127 @@ bool AsyncRPCOperation_mergetoaddress::main_impl() consensusBranchId_ = CurrentEpochBranchId(chainActive.Height() + 1, Params().GetConsensus()); } + /** + * SCENARIO #0 + * + * Sprout not involved, so we just use the TransactionBuilder and we're done. + * + * This is based on code from AsyncRPCOperation_sendmany::main_impl() and should be refactored. + */ + if (isUsingBuilder_) { + builder_.SetFee(minersFee); + + + for (const MergeToAddressInputUTXO& t : utxoInputs_) { + COutPoint outPoint = std::get<0>(t); + CAmount amount = std::get<1>(t); + CScript scriptPubKey = std::get<2>(t); + builder_.AddTransparentInput(outPoint, scriptPubKey, amount); + } + + boost::optional ovk; + // Select Sapling notes + std::vector saplingOPs; + std::vector saplingNotes; + std::vector expsks; + for (const MergeToAddressInputSaplingNote& saplingNoteInput: saplingNoteInputs_) { + saplingOPs.push_back(std::get<0>(saplingNoteInput)); + saplingNotes.push_back(std::get<1>(saplingNoteInput)); + auto expsk = std::get<3>(saplingNoteInput); + expsks.push_back(expsk); + if (!ovk) { + ovk = expsk.full_viewing_key().ovk; + } + } + + // Fetch Sapling anchor and witnesses + uint256 anchor; + std::vector> witnesses; + { + LOCK2(cs_main, pwalletMain->cs_wallet); + pwalletMain->GetSaplingNoteWitnesses(saplingOPs, witnesses, anchor); + } + + // Add Sapling spends + for (size_t i = 0; i < saplingNotes.size(); i++) { + if (!witnesses[i]) { + throw JSONRPCError(RPC_WALLET_ERROR, "Missing witness for Sapling note"); + } + assert(builder_.AddSaplingSpend(expsks[i], saplingNotes[i], anchor, witnesses[i].get())); + } + + if (isToTaddr_) { + if (!builder_.AddTransparentOutput(toTaddr_, sendAmount)) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid output address, not a valid taddr."); + } + } else { + std::string zaddr = std::get<0>(recipient_); + std::string memo = std::get<1>(recipient_); + std::array hexMemo = get_memo_from_hex_string(memo); + auto saplingPaymentAddress = boost::get(&toPaymentAddress_); + if (saplingPaymentAddress == nullptr) { + // This should never happen as we have already determined that the payment is to sapling + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Could not get Sapling payment address."); + } + if (saplingNoteInputs_.size() == 0 && utxoInputs_.size() > 0) { + // Sending from t-addresses, which we don't have ovks for. Instead, + // generate a common one from the HD seed. This ensures the data is + // recoverable, while keeping it logically separate from the ZIP 32 + // Sapling key hierarchy, which the user might not be using. + HDSeed seed; + if (!pwalletMain->GetHDSeed(seed)) { + throw JSONRPCError( + RPC_WALLET_ERROR, + "AsyncRPCOperation_sendmany: HD seed not found"); + } + ovk = ovkForShieldingFromTaddr(seed); + } + if (!ovk) { + throw JSONRPCError(RPC_WALLET_ERROR, "Sending to a Sapling address requires an ovk."); + } + builder_.AddSaplingOutput(ovk.get(), *saplingPaymentAddress, sendAmount, hexMemo); + } + + + // Build the transaction + auto maybe_tx = builder_.Build(); + if (!maybe_tx) { + throw JSONRPCError(RPC_WALLET_ERROR, "Failed to build transaction."); + } + tx_ = maybe_tx.get(); + + // Send the transaction + // TODO: Use CWallet::CommitTransaction instead of sendrawtransaction + auto signedtxn = EncodeHexTx(tx_); + if (!testmode) { + UniValue params = UniValue(UniValue::VARR); + params.push_back(signedtxn); + UniValue sendResultValue = sendrawtransaction(params, false); + if (sendResultValue.isNull()) { + throw JSONRPCError(RPC_WALLET_ERROR, "sendrawtransaction did not return an error or a txid."); + } + + auto txid = sendResultValue.get_str(); + + UniValue o(UniValue::VOBJ); + o.push_back(Pair("txid", txid)); + set_result(o); + } else { + // Test mode does not send the transaction to the network. + UniValue o(UniValue::VOBJ); + o.push_back(Pair("test", 1)); + o.push_back(Pair("txid", tx_.GetHash().ToString())); + o.push_back(Pair("hex", signedtxn)); + set_result(o); + } + + return true; + } + /** + * END SCENARIO #0 + */ + + /** * SCENARIO #1 * @@ -305,7 +446,7 @@ bool AsyncRPCOperation_mergetoaddress::main_impl() * * We only need a single JoinSplit. */ - if (noteInputs_.empty() && isToZaddr_) { + if (sproutNoteInputs_.empty() && isToZaddr_) { // Create JoinSplit to target z-addr. MergeToAddressJSInfo info; info.vpub_old = sendAmount; @@ -328,12 +469,8 @@ bool AsyncRPCOperation_mergetoaddress::main_impl() // Copy zinputs to more flexible containers - std::deque zInputsDeque; - for (auto o : noteInputs_) { - // TODO: Add Sapling support. For now, return an error to the user. - if (boost::get(&std::get<3>(o)) == nullptr) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Currently, only Sprout zaddrs are supported"); - } + std::deque zInputsDeque; + for (const auto& o : sproutNoteInputs_) { zInputsDeque.push_back(o); } @@ -342,7 +479,7 @@ bool AsyncRPCOperation_mergetoaddress::main_impl() // to happen as creating a chained joinsplit transaction can take longer than the block interval. { LOCK2(cs_main, pwalletMain->cs_wallet); - for (auto t : noteInputs_) { + for (auto t : sproutNoteInputs_) { JSOutPoint jso = std::get<0>(t); std::vector vOutPoints = {jso}; uint256 inputAnchor; @@ -373,7 +510,7 @@ bool AsyncRPCOperation_mergetoaddress::main_impl() // At this point, we are guaranteed to have at least one input note. // Use address of first input note as the temporary change address. - SproutSpendingKey changeKey = boost::get(std::get<3>(zInputsDeque.front())); + SproutSpendingKey changeKey = std::get<3>(zInputsDeque.front()); SproutPaymentAddress changeAddress = changeKey.address(); CAmount vpubOldTarget = 0; @@ -495,11 +632,11 @@ bool AsyncRPCOperation_mergetoaddress::main_impl() uint256 inputAnchor; int numInputsNeeded = (jsChange > 0) ? 1 : 0; while (numInputsNeeded++ < ZC_NUM_JS_INPUTS && zInputsDeque.size() > 0) { - MergeToAddressInputNote t = zInputsDeque.front(); + MergeToAddressInputSproutNote t = zInputsDeque.front(); JSOutPoint jso = std::get<0>(t); SproutNote note = std::get<1>(t); CAmount noteFunds = std::get<2>(t); - SproutSpendingKey zkey = boost::get(std::get<3>(t)); + SproutSpendingKey zkey = std::get<3>(t); zInputsDeque.pop_front(); MergeToAddressWitnessAnchorData wad = jsopWitnessAnchorMap[jso.ToString()]; @@ -628,7 +765,6 @@ bool AsyncRPCOperation_mergetoaddress::main_impl() extern UniValue signrawtransaction(const UniValue& params, bool fHelp); -extern UniValue sendrawtransaction(const UniValue& params, bool fHelp); /** * Sign and send a raw transaction. @@ -947,7 +1083,10 @@ void AsyncRPCOperation_mergetoaddress::unlock_utxos() { */ void AsyncRPCOperation_mergetoaddress::lock_notes() { LOCK2(cs_main, pwalletMain->cs_wallet); - for (auto note : noteInputs_) { + for (auto note : sproutNoteInputs_) { + pwalletMain->LockNote(std::get<0>(note)); + } + for (auto note : saplingNoteInputs_) { pwalletMain->LockNote(std::get<0>(note)); } } @@ -957,7 +1096,10 @@ void AsyncRPCOperation_mergetoaddress::unlock_utxos() { */ void AsyncRPCOperation_mergetoaddress::unlock_notes() { LOCK2(cs_main, pwalletMain->cs_wallet); - for (auto note : noteInputs_) { + for (auto note : sproutNoteInputs_) { + pwalletMain->UnlockNote(std::get<0>(note)); + } + for (auto note : saplingNoteInputs_) { pwalletMain->UnlockNote(std::get<0>(note)); } } diff --git a/src/wallet/asyncrpcoperation_mergetoaddress.h b/src/wallet/asyncrpcoperation_mergetoaddress.h index ef59ba67b..09ac53b79 100644 --- a/src/wallet/asyncrpcoperation_mergetoaddress.h +++ b/src/wallet/asyncrpcoperation_mergetoaddress.h @@ -9,6 +9,7 @@ #include "asyncrpcoperation.h" #include "paymentdisclosure.h" #include "primitives/transaction.h" +#include "transaction_builder.h" #include "wallet.h" #include "zcash/Address.hpp" #include "zcash/JoinSplit.hpp" @@ -24,11 +25,13 @@ using namespace libzcash; -// Input UTXO is a tuple of txid, vout, amount -typedef std::tuple MergeToAddressInputUTXO; +// Input UTXO is a tuple of txid, vout, amount, script +typedef std::tuple MergeToAddressInputUTXO; // Input JSOP is a tuple of JSOutpoint, note, amount, spending key -typedef std::tuple MergeToAddressInputNote; +typedef std::tuple MergeToAddressInputSproutNote; + +typedef std::tuple MergeToAddressInputSaplingNote; // A recipient is a tuple of address, memo (optional if zaddr) typedef std::tuple MergeToAddressRecipient; @@ -53,9 +56,11 @@ class AsyncRPCOperation_mergetoaddress : public AsyncRPCOperation { public: AsyncRPCOperation_mergetoaddress( + boost::optional builder, CMutableTransaction contextualTx, std::vector utxoInputs, - std::vector noteInputs, + std::vector sproutNoteInputs, + std::vector saplingNoteInputs, MergeToAddressRecipient recipient, CAmount fee = MERGE_TO_ADDRESS_OPERATION_DEFAULT_MINERS_FEE, UniValue contextInfo = NullUniValue); @@ -79,7 +84,8 @@ private: friend class TEST_FRIEND_AsyncRPCOperation_mergetoaddress; // class for unit testing UniValue contextinfo_; // optional data to include in return value from getStatus() - + + bool isUsingBuilder_; // Indicates that no Sprout addresses are involved uint32_t consensusBranchId_; CAmount fee_; int mindepth_; @@ -96,8 +102,10 @@ private: std::unordered_map jsopWitnessAnchorMap; std::vector utxoInputs_; - std::vector noteInputs_; + std::vector sproutNoteInputs_; + std::vector saplingNoteInputs_; + TransactionBuilder builder_; CTransaction tx_; std::array get_memo_from_hex_string(std::string s); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 820ab8cb4..38f117de0 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3734,17 +3734,11 @@ UniValue z_sendmany(const UniValue& params, bool fHelp) "Cannot send to both Sprout and Sapling addresses using z_sendmany"); } - // If we are sending from a shielded address, all recipient - // shielded addresses must be of the same type. - if (fromSprout && toSapling) { + // If sending between shielded addresses, they must be the same type + if ((fromSprout && toSapling) || (fromSapling && toSprout)) { throw JSONRPCError( RPC_INVALID_PARAMETER, - "Cannot send from a Sprout address to a Sapling address using z_sendmany"); - } - if (fromSapling && toSprout) { - throw JSONRPCError( - RPC_INVALID_PARAMETER, - "Cannot send from a Sapling address to a Sprout address using z_sendmany"); + "Cannot send between Sprout and Sapling addresses using z_sendmany"); } } else { throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, unknown address format: ")+address ); @@ -4130,9 +4124,12 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp) #define MERGE_TO_ADDRESS_DEFAULT_TRANSPARENT_LIMIT 50 -#define MERGE_TO_ADDRESS_DEFAULT_SHIELDED_LIMIT 10 +#define MERGE_TO_ADDRESS_DEFAULT_SPROUT_LIMIT 20 +#define MERGE_TO_ADDRESS_DEFAULT_SAPLING_LIMIT 200 #define JOINSPLIT_SIZE GetSerializeSize(JSDescription(), SER_NETWORK, PROTOCOL_VERSION) +#define OUTPUTDESCRIPTION_SIZE GetSerializeSize(OutputDescription(), SER_NETWORK, PROTOCOL_VERSION) +#define SPENDDESCRIPTION_SIZE GetSerializeSize(SpendDescription(), SER_NETWORK, PROTOCOL_VERSION) UniValue z_mergetoaddress(const UniValue& params, bool fHelp) { @@ -4162,9 +4159,9 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) "\nArguments:\n" "1. fromaddresses (string, required) A JSON array with addresses.\n" " The following special strings are accepted inside the array:\n" - " - \"*\": Merge both UTXOs and notes from all addresses belonging to the wallet.\n" - " - \"ANY_TADDR\": Merge UTXOs from all t-addrs belonging to the wallet.\n" - " - \"ANY_ZADDR\": Merge notes from all z-addrs belonging to the wallet.\n" + " - \"ANY_TADDR\": Merge UTXOs from any t-addrs belonging to the wallet.\n" + " - \"ANY_SPROUT\": Merge notes from any Sprout z-addrs belonging to the wallet.\n" + " - \"ANY_SAPLING\": Merge notes from any Sapling z-addrs belonging to the wallet.\n" " If a special string is given, any given addresses of that type will be ignored.\n" " [\n" " \"address\" (string) Can be a t-addr or a z-addr\n" @@ -4176,7 +4173,7 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) "4. transparent_limit (numeric, optional, default=" + strprintf("%d", MERGE_TO_ADDRESS_DEFAULT_TRANSPARENT_LIMIT) + ") Limit on the maximum number of UTXOs to merge. Set to 0 to use node option -mempooltxinputlimit (before Overwinter), or as many as will fit in the transaction (after Overwinter).\n" "4. shielded_limit (numeric, optional, default=" - + strprintf("%d", MERGE_TO_ADDRESS_DEFAULT_SHIELDED_LIMIT) + ") Limit on the maximum number of notes to merge. Set to 0 to merge as many as will fit in the transaction.\n" + + strprintf("%d Sprout or %d Sapling Notes", MERGE_TO_ADDRESS_DEFAULT_SPROUT_LIMIT, MERGE_TO_ADDRESS_DEFAULT_SAPLING_LIMIT) + ") Limit on the maximum number of notes to merge. Set to 0 to merge as many as will fit in the transaction.\n" "5. \"memo\" (string, optional) Encoded as hex. When toaddress is a z-addr, this will be stored in the memo field of the new note.\n" "\nResult:\n" "{\n" @@ -4201,9 +4198,9 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) LOCK2(cs_main, pwalletMain->cs_wallet); - bool useAny = false; bool useAnyUTXO = false; - bool useAnyNote = false; + bool useAnySprout = false; + bool useAnySapling = false; std::set taddrs = {}; std::set zaddrs = {}; @@ -4215,39 +4212,28 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) std::set setAddress; // Sources - bool containsSaplingZaddrSource = false; for (const UniValue& o : addresses.getValues()) { if (!o.isStr()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, expected string"); std::string address = o.get_str(); - if (address == "*") { - useAny = true; - } else if (address == "ANY_TADDR") { + + if (address == "ANY_TADDR") { useAnyUTXO = true; - } else if (address == "ANY_ZADDR") { - useAnyNote = true; + } else if (address == "ANY_SPROUT") { + useAnySprout = true; + } else if (address == "ANY_SAPLING") { + useAnySapling = true; } else { CTxDestination taddr = DecodeDestination(address); if (IsValidDestination(taddr)) { - // Ignore any listed t-addrs if we are using all of them - if (!(useAny || useAnyUTXO)) { - taddrs.insert(taddr); - } + taddrs.insert(taddr); } else { auto zaddr = DecodePaymentAddress(address); if (IsValidPaymentAddress(zaddr)) { - // Ignore listed z-addrs if we are using all of them - if (!(useAny || useAnyNote)) { - zaddrs.insert(zaddr); - } - // Check if z-addr is Sapling - bool isSapling = boost::get(&zaddr) != nullptr; - containsSaplingZaddrSource |= isSapling; + zaddrs.insert(zaddr); } else { - throw JSONRPCError( - RPC_INVALID_PARAMETER, - string("Invalid parameter, unknown address format: ") + address); + throw JSONRPCError(RPC_INVALID_PARAMETER, string("Unknown address format: ") + address); } } } @@ -4257,21 +4243,33 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) setAddress.insert(address); } + if (useAnyUTXO && taddrs.size() > 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify specific t-addrs when using \"ANY_TADDR\""); + } + if ((useAnySprout || useAnySapling) && zaddrs.size() > 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify specific z-addrs when using \"ANY_SPROUT\" or \"ANY_SAPLING\""); + } + + const int nextBlockHeight = chainActive.Height() + 1; + const bool overwinterActive = NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER); + const bool saplingActive = NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_SAPLING); + // Validate the destination address auto destaddress = params[1].get_str(); - bool isToZaddr = false; + bool isToSproutZaddr = false; bool isToSaplingZaddr = false; CTxDestination taddr = DecodeDestination(destaddress); if (!IsValidDestination(taddr)) { - if (IsValidPaymentAddressString(destaddress)) { - isToZaddr = true; - - // Is this a Sapling address? - auto res = DecodePaymentAddress(destaddress); - if (IsValidPaymentAddress(res)) { - isToSaplingZaddr = boost::get(&res) != nullptr; + auto decodeAddr = DecodePaymentAddress(destaddress); + if (IsValidPaymentAddress(decodeAddr)) { + if (boost::get(&decodeAddr) != nullptr) { + isToSaplingZaddr = true; + // If Sapling is not active, do not allow sending to a sapling addresses. + if (!saplingActive) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, Sapling has not activated"); + } } else { - throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, unknown address format: ") + destaddress ); + isToSproutZaddr = true; } } else { throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, unknown address format: ") + destaddress ); @@ -4296,18 +4294,21 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) } } - int nNoteLimit = MERGE_TO_ADDRESS_DEFAULT_SHIELDED_LIMIT; + int sproutNoteLimit = MERGE_TO_ADDRESS_DEFAULT_SPROUT_LIMIT; + int saplingNoteLimit = MERGE_TO_ADDRESS_DEFAULT_SAPLING_LIMIT; if (params.size() > 4) { - nNoteLimit = params[4].get_int(); + int nNoteLimit = params[4].get_int(); if (nNoteLimit < 0) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Limit on maximum number of notes cannot be negative"); } + sproutNoteLimit = nNoteLimit; + saplingNoteLimit = nNoteLimit; } std::string memo; if (params.size() > 5) { memo = params[5].get_str(); - if (!isToZaddr) { + if (!(isToSproutZaddr || isToSaplingZaddr)) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Memo can not be used with a taddr. It can only be used with a zaddr."); } else if (!IsHex(memo)) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, expected memo data in hexadecimal format."); @@ -4319,29 +4320,10 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) MergeToAddressRecipient recipient(destaddress, memo); - int nextBlockHeight = chainActive.Height() + 1; - bool overwinterActive = NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER); - unsigned int max_tx_size = MAX_TX_SIZE_AFTER_SAPLING; - if (!NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_SAPLING)) { - max_tx_size = MAX_TX_SIZE_BEFORE_SAPLING; - } - - // This RPC does not support Sapling yet. - if (isToSaplingZaddr || containsSaplingZaddrSource) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, Sapling is not supported yet by z_mergetoadress"); - } - - // If this RPC does support Sapling... - // If Sapling is not active, do not allow sending from or sending to Sapling addresses. - if (!NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_SAPLING)) { - if (isToSaplingZaddr || containsSaplingZaddrSource) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, Sapling has not activated"); - } - } - // Prepare to get UTXOs and notes std::vector utxoInputs; - std::vector noteInputs; + std::vector sproutNoteInputs; + std::vector saplingNoteInputs; CAmount mergedUTXOValue = 0; CAmount mergedNoteValue = 0; CAmount remainingUTXOValue = 0; @@ -4352,12 +4334,15 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) bool maxedOutNotesFlag = false; size_t mempoolLimit = (nUTXOLimit != 0) ? nUTXOLimit : (overwinterActive ? 0 : (size_t)GetArg("-mempooltxinputlimit", 0)); + unsigned int max_tx_size = saplingActive ? MAX_TX_SIZE_AFTER_SAPLING : MAX_TX_SIZE_BEFORE_SAPLING; size_t estimatedTxSize = 200; // tx overhead + wiggle room - if (isToZaddr) { + if (isToSproutZaddr) { estimatedTxSize += JOINSPLIT_SIZE; + } else if (isToSaplingZaddr) { + estimatedTxSize += OUTPUTDESCRIPTION_SIZE; } - if (useAny || useAnyUTXO || taddrs.size() > 0) { + if (useAnyUTXO || taddrs.size() > 0) { // Get available utxos vector vecOutputs; pwalletMain->AvailableCoins(vecOutputs, true, NULL, false, false); @@ -4368,8 +4353,10 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) continue; } + CScript scriptPubKey = out.tx->vout[out.i].scriptPubKey; + CTxDestination address; - if (!ExtractDestination(out.tx->vout[out.i].scriptPubKey, address)) { + if (!ExtractDestination(scriptPubKey, address)) { continue; } // If taddr is not wildcard "*", filter utxos @@ -4389,7 +4376,7 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) } else { estimatedTxSize += increase; COutPoint utxo(out.tx->GetHash(), out.i); - utxoInputs.emplace_back(utxo, nValue); + utxoInputs.emplace_back(utxo, nValue, scriptPubKey); mergedUTXOValue += nValue; } } @@ -4400,23 +4387,40 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) } } - if (useAny || useAnyNote || zaddrs.size() > 0) { + if (useAnySprout || useAnySapling || zaddrs.size() > 0) { // Get available notes std::vector sproutEntries; std::vector saplingEntries; pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, zaddrs); + // If Sapling is not active, do not allow sending from a sapling addresses. + if (!saplingActive && saplingEntries.size() > 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, Sapling has not activated"); + } + // Sending from both Sprout and Sapling is currently unsupported using z_mergetoaddress + if (sproutEntries.size() > 0 && saplingEntries.size() > 0) { + throw JSONRPCError( + RPC_INVALID_PARAMETER, + "Cannot send from both Sprout and Sapling addresses using z_mergetoaddress"); + } + // If sending between shielded addresses, they must be the same type + if ((saplingEntries.size() > 0 && isToSproutZaddr) || (sproutEntries.size() > 0 && isToSaplingZaddr)) { + throw JSONRPCError( + RPC_INVALID_PARAMETER, + "Cannot send between Sprout and Sapling addresses using z_mergetoaddress"); + } + // Find unspent notes and update estimated size - for (CSproutNotePlaintextEntry& entry : sproutEntries) { + for (const CSproutNotePlaintextEntry& entry : sproutEntries) { noteCounter++; CAmount nValue = entry.plaintext.value(); if (!maxedOutNotesFlag) { // If we haven't added any notes yet and the merge is to a // z-address, we have already accounted for the first JoinSplit. - size_t increase = (noteInputs.empty() && !isToZaddr) || (noteInputs.size() % 2 == 0) ? JOINSPLIT_SIZE : 0; + size_t increase = (sproutNoteInputs.empty() && !isToSproutZaddr) || (sproutNoteInputs.size() % 2 == 0) ? JOINSPLIT_SIZE : 0; if (estimatedTxSize + increase >= max_tx_size || - (nNoteLimit > 0 && noteCounter > nNoteLimit)) + (sproutNoteLimit > 0 && noteCounter > sproutNoteLimit)) { maxedOutNotesFlag = true; } else { @@ -4424,7 +4428,32 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) auto zaddr = entry.address; SproutSpendingKey zkey; pwalletMain->GetSproutSpendingKey(zaddr, zkey); - noteInputs.emplace_back(entry.jsop, entry.plaintext.note(zaddr), nValue, zkey); + sproutNoteInputs.emplace_back(entry.jsop, entry.plaintext.note(zaddr), nValue, zkey); + mergedNoteValue += nValue; + } + } + + if (maxedOutNotesFlag) { + remainingNoteValue += nValue; + } + } + + for (const SaplingNoteEntry& entry : saplingEntries) { + noteCounter++; + CAmount nValue = entry.note.value(); + if (!maxedOutNotesFlag) { + size_t increase = SPENDDESCRIPTION_SIZE; + if (estimatedTxSize + increase >= max_tx_size || + (saplingNoteLimit > 0 && noteCounter > saplingNoteLimit)) + { + maxedOutNotesFlag = true; + } else { + estimatedTxSize += increase; + libzcash::SaplingExtendedSpendingKey extsk; + if (!pwalletMain->GetSaplingExtendedSpendingKey(entry.address, extsk)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Could not find spending key for payment address."); + } + saplingNoteInputs.emplace_back(entry.op, entry.note, nValue, extsk.expsk); mergedNoteValue += nValue; } } @@ -4433,11 +4462,10 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) remainingNoteValue += nValue; } } - // TODO: Add Sapling support } size_t numUtxos = utxoInputs.size(); - size_t numNotes = noteInputs.size(); + size_t numNotes = sproutNoteInputs.size() + saplingNoteInputs.size(); if (numUtxos == 0 && numNotes == 0) { throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Could not find any funds to merge."); @@ -4474,15 +4502,20 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) CMutableTransaction contextualTx = CreateNewContextualCMutableTransaction( Params().GetConsensus(), nextBlockHeight); - bool isShielded = numNotes > 0 || isToZaddr; - if (contextualTx.nVersion == 1 && isShielded) { + bool isSproutShielded = sproutNoteInputs.size() > 0 || isToSproutZaddr; + if (contextualTx.nVersion == 1 && isSproutShielded) { contextualTx.nVersion = 2; // Tx format should support vjoinsplit } + // Builder (used if Sapling addresses are involved) + boost::optional builder; + if (isToSaplingZaddr || saplingNoteInputs.size() > 0) { + builder = TransactionBuilder(Params().GetConsensus(), nextBlockHeight, pwalletMain); + } // Create operation and add to global queue std::shared_ptr q = getAsyncRPCQueue(); std::shared_ptr operation( - new AsyncRPCOperation_mergetoaddress(contextualTx, utxoInputs, noteInputs, recipient, nFee, contextInfo) ); + new AsyncRPCOperation_mergetoaddress(builder, contextualTx, utxoInputs, sproutNoteInputs, saplingNoteInputs, recipient, nFee, contextInfo) ); q->addOperation(operation); AsyncRPCOperationId operationId = operation->getId(); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e033e1fac..eaecd63f0 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4512,10 +4512,9 @@ void CWallet::GetFilteredNotes( } // skip locked notes - // TODO: Add locking for Sapling notes - // if (ignoreLocked && IsLockedNote(op)) { - // continue; - // } + if (ignoreLocked && IsLockedNote(op)) { + continue; + } auto note = notePt.note(nd.ivk).get(); saplingEntries.push_back(SaplingNoteEntry {