diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index dde4144c4..310b045a7 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -18,7 +18,8 @@ testScripts=( 'wallet_changeindicator.py' 'wallet_import_export.py' 'wallet_protectcoinbase.py' - 'wallet_shieldcoinbase.py' + 'wallet_shieldcoinbase_sprout.py' + 'wallet_shieldcoinbase_sapling.py' 'wallet_listreceived.py' 'wallet_mergetoaddress.py' 'wallet.py' diff --git a/qa/rpc-tests/wallet_shieldcoinbase.py b/qa/rpc-tests/wallet_shieldcoinbase.py index b77fedcf0..d8366c81d 100755 --- a/qa/rpc-tests/wallet_shieldcoinbase.py +++ b/qa/rpc-tests/wallet_shieldcoinbase.py @@ -12,6 +12,9 @@ from test_framework.util import assert_equal, initialize_chain_clean, \ from decimal import Decimal class WalletShieldCoinbaseTest (BitcoinTestFramework): + def __init__(self, addr_type): + super(WalletShieldCoinbaseTest, self).__init__() + self.addr_type = addr_type def setup_chain(self): print("Initializing test directory "+self.options.tmpdir) @@ -19,10 +22,17 @@ class WalletShieldCoinbaseTest (BitcoinTestFramework): def setup_network(self, split=False): args = ['-regtestprotectcoinbase', '-debug=zrpcunsafe'] + args2 = ['-regtestprotectcoinbase', '-debug=zrpcunsafe', "-mempooltxinputlimit=7"] + if self.addr_type != 'sprout': + nu = [ + '-nuparams=5ba81b19:0', # Overwinter + '-nuparams=76b809bb:1', # Sapling + ] + args.extend(nu) + args2 = args self.nodes = [] self.nodes.append(start_node(0, self.options.tmpdir, args)) self.nodes.append(start_node(1, self.options.tmpdir, args)) - args2 = ['-regtestprotectcoinbase', '-debug=zrpcunsafe', "-mempooltxinputlimit=7"] self.nodes.append(start_node(2, self.options.tmpdir, args2)) connect_nodes_bi(self.nodes,0,1) connect_nodes_bi(self.nodes,1,2) @@ -55,7 +65,7 @@ class WalletShieldCoinbaseTest (BitcoinTestFramework): # Prepare to send taddr->zaddr mytaddr = self.nodes[0].getnewaddress() - myzaddr = self.nodes[0].z_getnewaddress() + myzaddr = self.nodes[0].z_getnewaddress(self.addr_type) # Shielding will fail when trying to spend from watch-only address self.nodes[2].importaddress(mytaddr) @@ -135,26 +145,33 @@ class WalletShieldCoinbaseTest (BitcoinTestFramework): self.sync_all() mytaddr = self.nodes[0].getnewaddress() - # Shielding the 800 utxos will occur over two transactions, since max tx size is 100,000 bytes. - # We don't verify shieldingValue 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 = self.nodes[0].z_shieldcoinbase(mytaddr, myzaddr, 0, 99999) - assert_equal(result["shieldingUTXOs"], Decimal('662')) - assert_equal(result["remainingUTXOs"], Decimal('138')) - remainingValue = result["remainingValue"] - opid1 = result['opid'] + def verify_locking(first, second, limit): + result = self.nodes[0].z_shieldcoinbase(mytaddr, myzaddr, 0, limit) + assert_equal(result["shieldingUTXOs"], Decimal(first)) + assert_equal(result["remainingUTXOs"], Decimal(second)) + remainingValue = result["remainingValue"] + opid1 = result['opid'] - # Verify that utxos are locked (not available for selection) by queuing up another shielding operation - result = self.nodes[0].z_shieldcoinbase(mytaddr, myzaddr, 0, 0) - assert_equal(result["shieldingValue"], Decimal(remainingValue)) - assert_equal(result["shieldingUTXOs"], Decimal('138')) - assert_equal(result["remainingValue"], Decimal('0')) - assert_equal(result["remainingUTXOs"], Decimal('0')) - opid2 = result['opid'] + # Verify that utxos are locked (not available for selection) by queuing up another shielding operation + result = self.nodes[0].z_shieldcoinbase(mytaddr, myzaddr, 0, 0) + assert_equal(result["shieldingValue"], Decimal(remainingValue)) + assert_equal(result["shieldingUTXOs"], Decimal(second)) + assert_equal(result["remainingValue"], Decimal('0')) + assert_equal(result["remainingUTXOs"], Decimal('0')) + opid2 = result['opid'] - # wait for both aysnc operations to complete - wait_and_assert_operationid_status(self.nodes[0], opid1) - wait_and_assert_operationid_status(self.nodes[0], opid2) + # wait for both aysnc operations to complete + wait_and_assert_operationid_status(self.nodes[0], opid1) + wait_and_assert_operationid_status(self.nodes[0], opid2) + + if self.addr_type == 'sprout': + # Shielding the 800 utxos will occur over two transactions, since max tx size is 100,000 bytes. + # We don't verify shieldingValue 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. + verify_locking('662', '138', 99999) + else: + # Shield the 800 utxos over two transactions + verify_locking('500', '300', 500) # 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 @@ -164,16 +181,17 @@ class WalletShieldCoinbaseTest (BitcoinTestFramework): self.nodes[1].generate(1) self.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. - mytaddr = self.nodes[2].getnewaddress() - result = self.nodes[2].z_shieldcoinbase(mytaddr, myzaddr, Decimal('0.0001'), 0) - assert_equal(result["shieldingUTXOs"], Decimal('7')) - assert_equal(result["remainingUTXOs"], Decimal('13')) - wait_and_assert_operationid_status(self.nodes[2], result['opid']) - self.sync_all() - self.nodes[1].generate(1) - self.sync_all() + if self.addr_type == 'sprout': + # 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. + mytaddr = self.nodes[2].getnewaddress() + result = self.nodes[2].z_shieldcoinbase(mytaddr, myzaddr, Decimal('0.0001'), 0) + assert_equal(result["shieldingUTXOs"], Decimal('7')) + assert_equal(result["remainingUTXOs"], Decimal('13')) + wait_and_assert_operationid_status(self.nodes[2], result['opid']) + self.sync_all() + self.nodes[1].generate(1) + self.sync_all() # Verify maximum number of utxos which node 0 can shield is set by default limit parameter of 50 self.nodes[0].generate(200) @@ -194,6 +212,3 @@ class WalletShieldCoinbaseTest (BitcoinTestFramework): sync_mempools(self.nodes[:2]) self.nodes[1].generate(1) self.sync_all() - -if __name__ == '__main__': - WalletShieldCoinbaseTest().main() diff --git a/qa/rpc-tests/wallet_shieldcoinbase_sapling.py b/qa/rpc-tests/wallet_shieldcoinbase_sapling.py new file mode 100755 index 000000000..6aac42136 --- /dev/null +++ b/qa/rpc-tests/wallet_shieldcoinbase_sapling.py @@ -0,0 +1,13 @@ +#!/usr/bin/env python2 +import inspect +import os + +cwd = os.path.dirname(os.path.abspath(inspect.getfile(inspect.currentframe()))) +execfile(os.path.join(cwd, 'wallet_shieldcoinbase.py')) + +class WalletShieldCoinbaseSapling(WalletShieldCoinbaseTest): + def __init__(self): + super(WalletShieldCoinbaseSapling, self).__init__('sapling') + +if __name__ == '__main__': + WalletShieldCoinbaseSapling().main() diff --git a/qa/rpc-tests/wallet_shieldcoinbase_sprout.py b/qa/rpc-tests/wallet_shieldcoinbase_sprout.py new file mode 100755 index 000000000..77ca7487b --- /dev/null +++ b/qa/rpc-tests/wallet_shieldcoinbase_sprout.py @@ -0,0 +1,13 @@ +#!/usr/bin/env python2 +import inspect +import os + +cwd = os.path.dirname(os.path.abspath(inspect.getfile(inspect.currentframe()))) +execfile(os.path.join(cwd, 'wallet_shieldcoinbase.py')) + +class WalletShieldCoinbaseSprout(WalletShieldCoinbaseTest): + def __init__(self): + super(WalletShieldCoinbaseSprout, self).__init__('sprout') + +if __name__ == '__main__': + WalletShieldCoinbaseSprout().main() diff --git a/src/test/rpc_wallet_tests.cpp b/src/test/rpc_wallet_tests.cpp index 811b98bab..56e18221b 100644 --- a/src/test/rpc_wallet_tests.cpp +++ b/src/test/rpc_wallet_tests.cpp @@ -1520,13 +1520,13 @@ BOOST_AUTO_TEST_CASE(rpc_z_shieldcoinbase_parameters) std::string mainnetzaddr = "zcMuhvq8sEkHALuSU2i4NbNQxshSAYrpCExec45ZjtivYPbuiFPwk6WHy4SvsbeZ4siy1WheuRGjtaJmoD1J8bFqNXhsG6U"; try { - std::shared_ptr operation(new AsyncRPCOperation_shieldcoinbase(mtx, {}, testnetzaddr, -1 )); + std::shared_ptr operation(new AsyncRPCOperation_shieldcoinbase(TransactionBuilder(), mtx, {}, testnetzaddr, -1 )); } catch (const UniValue& objError) { BOOST_CHECK( find_error(objError, "Fee is out of range")); } try { - std::shared_ptr operation(new AsyncRPCOperation_shieldcoinbase(mtx, {}, testnetzaddr, 1)); + std::shared_ptr operation(new AsyncRPCOperation_shieldcoinbase(TransactionBuilder(), mtx, {}, testnetzaddr, 1)); } catch (const UniValue& objError) { BOOST_CHECK( find_error(objError, "Empty inputs")); } @@ -1534,7 +1534,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_shieldcoinbase_parameters) // Testnet payment addresses begin with 'zt'. This test detects an incorrect prefix. try { std::vector inputs = { ShieldCoinbaseUTXO{uint256(),0,0} }; - std::shared_ptr operation( new AsyncRPCOperation_shieldcoinbase(mtx, inputs, mainnetzaddr, 1) ); + std::shared_ptr operation( new AsyncRPCOperation_shieldcoinbase(TransactionBuilder(), mtx, inputs, mainnetzaddr, 1) ); } catch (const UniValue& objError) { BOOST_CHECK( find_error(objError, "Invalid to address")); } @@ -1567,7 +1567,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_shieldcoinbase_internals) // Supply 2 inputs when mempool limit is 1 { std::vector inputs = { ShieldCoinbaseUTXO{uint256(),0,0}, ShieldCoinbaseUTXO{uint256(),0,0} }; - std::shared_ptr operation( new AsyncRPCOperation_shieldcoinbase(mtx, inputs, zaddr) ); + std::shared_ptr operation( new AsyncRPCOperation_shieldcoinbase(TransactionBuilder(), mtx, inputs, zaddr) ); operation->main(); BOOST_CHECK(operation->isFailed()); std::string msg = operation->getErrorMessage(); @@ -1577,7 +1577,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_shieldcoinbase_internals) // Insufficient funds { std::vector inputs = { ShieldCoinbaseUTXO{uint256(),0,0} }; - std::shared_ptr operation( new AsyncRPCOperation_shieldcoinbase(mtx, inputs, zaddr) ); + std::shared_ptr operation( new AsyncRPCOperation_shieldcoinbase(TransactionBuilder(), mtx, inputs, zaddr) ); operation->main(); BOOST_CHECK(operation->isFailed()); std::string msg = operation->getErrorMessage(); @@ -1588,7 +1588,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_shieldcoinbase_internals) { // Dummy input so the operation object can be instantiated. std::vector inputs = { ShieldCoinbaseUTXO{uint256(),0,100000} }; - std::shared_ptr operation( new AsyncRPCOperation_shieldcoinbase(mtx, inputs, zaddr) ); + std::shared_ptr operation( new AsyncRPCOperation_shieldcoinbase(TransactionBuilder(), mtx, inputs, zaddr) ); std::shared_ptr ptr = std::dynamic_pointer_cast (operation); TEST_FRIEND_AsyncRPCOperation_shieldcoinbase proxy(ptr); static_cast(operation.get())->testmode = true; diff --git a/src/wallet/asyncrpcoperation_shieldcoinbase.cpp b/src/wallet/asyncrpcoperation_shieldcoinbase.cpp index 63f08f774..100434de5 100644 --- a/src/wallet/asyncrpcoperation_shieldcoinbase.cpp +++ b/src/wallet/asyncrpcoperation_shieldcoinbase.cpp @@ -55,12 +55,13 @@ static int find_output(UniValue obj, int n) { } AsyncRPCOperation_shieldcoinbase::AsyncRPCOperation_shieldcoinbase( + TransactionBuilder builder, CMutableTransaction contextualTx, std::vector inputs, std::string toAddress, CAmount fee, UniValue contextInfo) : - tx_(contextualTx), inputs_(inputs), fee_(fee), contextinfo_(contextInfo) + builder_(builder), tx_(contextualTx), inputs_(inputs), fee_(fee), contextinfo_(contextInfo) { assert(contextualTx.nVersion >= 2); // transaction format version must support vjoinsplit @@ -75,8 +76,6 @@ AsyncRPCOperation_shieldcoinbase::AsyncRPCOperation_shieldcoinbase( // Check the destination address is valid for this network i.e. not testnet being used on mainnet auto address = DecodePaymentAddress(toAddress); if (IsValidPaymentAddress(address)) { - // TODO: Add Sapling support. For now, ensure we can freely convert. - assert(boost::get(&address) != nullptr); tozaddr_ = address; } else { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid to address"); @@ -181,7 +180,6 @@ void AsyncRPCOperation_shieldcoinbase::main() { // !!! Payment disclosure END } - bool AsyncRPCOperation_shieldcoinbase::main_impl() { CAmount minersFee = fee_; @@ -217,30 +215,34 @@ bool AsyncRPCOperation_shieldcoinbase::main_impl() { LogPrint("zrpc", "%s: spending %s to shield %s with fee %s\n", getId(), FormatMoney(targetAmount), FormatMoney(sendAmount), FormatMoney(minersFee)); + return boost::apply_visitor(ShieldToAddress(this, sendAmount), tozaddr_); +} + +bool ShieldToAddress::operator()(const libzcash::SproutPaymentAddress &zaddr) const { // update the transaction with these inputs - CMutableTransaction rawTx(tx_); - for (ShieldCoinbaseUTXO & t : inputs_) { + CMutableTransaction rawTx(m_op->tx_); + for (ShieldCoinbaseUTXO & t : m_op->inputs_) { CTxIn in(COutPoint(t.txid, t.vout)); rawTx.vin.push_back(in); } - tx_ = CTransaction(rawTx); + m_op->tx_ = CTransaction(rawTx); // Prepare raw transaction to handle JoinSplits - CMutableTransaction mtx(tx_); - crypto_sign_keypair(joinSplitPubKey_.begin(), joinSplitPrivKey_); - mtx.joinSplitPubKey = joinSplitPubKey_; - tx_ = CTransaction(mtx); + CMutableTransaction mtx(m_op->tx_); + crypto_sign_keypair(m_op->joinSplitPubKey_.begin(), m_op->joinSplitPrivKey_); + mtx.joinSplitPubKey = m_op->joinSplitPubKey_; + m_op->tx_ = CTransaction(mtx); // Create joinsplit UniValue obj(UniValue::VOBJ); ShieldCoinbaseJSInfo info; info.vpub_old = sendAmount; info.vpub_new = 0; - JSOutput jso = JSOutput(boost::get(tozaddr_), sendAmount); + JSOutput jso = JSOutput(zaddr, sendAmount); info.vjsout.push_back(jso); - obj = perform_joinsplit(info); + obj = m_op->perform_joinsplit(info); - sign_send_raw_transaction(obj); + m_op->sign_send_raw_transaction(obj); return true; } @@ -248,6 +250,69 @@ bool AsyncRPCOperation_shieldcoinbase::main_impl() { extern UniValue signrawtransaction(const UniValue& params, bool fHelp); extern UniValue sendrawtransaction(const UniValue& params, bool fHelp); +bool ShieldToAddress::operator()(const libzcash::SaplingPaymentAddress &zaddr) const { + m_op->builder_.SetFee(m_op->fee_); + + // Sending from a t-address, which we don't have an ovk 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, + "CWallet::GenerateNewSaplingZKey(): HD seed not found"); + } + uint256 ovk = ovkForShieldingFromTaddr(seed); + + // Add transparent inputs + for (auto t : m_op->inputs_) { + m_op->builder_.AddTransparentInput(COutPoint(t.txid, t.vout), t.scriptPubKey, t.amount); + } + + // Send all value to the target z-addr + m_op->builder_.SendChangeTo(zaddr, ovk); + + // Build the transaction + auto maybe_tx = m_op->builder_.Build(); + if (!maybe_tx) { + throw JSONRPCError(RPC_WALLET_ERROR, "Failed to build transaction."); + } + m_op->tx_ = maybe_tx.get(); + + // Send the transaction + // TODO: Use CWallet::CommitTransaction instead of sendrawtransaction + auto signedtxn = EncodeHexTx(m_op->tx_); + if (!m_op->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)); + m_op->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", m_op->tx_.GetHash().ToString())); + o.push_back(Pair("hex", signedtxn)); + m_op->set_result(o); + } + + return true; +} + +bool ShieldToAddress::operator()(const libzcash::InvalidEncoding& no) const { + return false; +} + + /** * Sign and send a raw transaction. * Raw transaction as hex string should be in object field "rawtxn" diff --git a/src/wallet/asyncrpcoperation_shieldcoinbase.h b/src/wallet/asyncrpcoperation_shieldcoinbase.h index e63c54d7d..e9ee44fcd 100644 --- a/src/wallet/asyncrpcoperation_shieldcoinbase.h +++ b/src/wallet/asyncrpcoperation_shieldcoinbase.h @@ -8,6 +8,7 @@ #include "asyncrpcoperation.h" #include "amount.h" #include "primitives/transaction.h" +#include "transaction_builder.h" #include "zcash/JoinSplit.hpp" #include "zcash/Address.hpp" #include "wallet.h" @@ -27,6 +28,7 @@ using namespace libzcash; struct ShieldCoinbaseUTXO { uint256 txid; int vout; + CScript scriptPubKey; CAmount amount; }; @@ -41,7 +43,13 @@ struct ShieldCoinbaseJSInfo class AsyncRPCOperation_shieldcoinbase : public AsyncRPCOperation { public: - AsyncRPCOperation_shieldcoinbase(CMutableTransaction contextualTx, std::vector inputs, std::string toAddress, CAmount fee = SHIELD_COINBASE_DEFAULT_MINERS_FEE, UniValue contextInfo = NullUniValue); + AsyncRPCOperation_shieldcoinbase( + TransactionBuilder builder, + CMutableTransaction contextualTx, + std::vector inputs, + std::string toAddress, + CAmount fee = SHIELD_COINBASE_DEFAULT_MINERS_FEE, + UniValue contextInfo = NullUniValue); virtual ~AsyncRPCOperation_shieldcoinbase(); // We don't want to be copied or moved around @@ -59,6 +67,7 @@ public: bool paymentDisclosureMode = false; // Set to true to save esk for encrypted notes in payment disclosure database. private: + friend class ShieldToAddress; friend class TEST_FRIEND_AsyncRPCOperation_shieldcoinbase; // class for unit testing UniValue contextinfo_; // optional data to include in return value from getStatus() @@ -71,6 +80,7 @@ private: std::vector inputs_; + TransactionBuilder builder_; CTransaction tx_; bool main_impl(); @@ -88,6 +98,20 @@ private: std::vector paymentDisclosureData_; }; +class ShieldToAddress : public boost::static_visitor +{ +private: + AsyncRPCOperation_shieldcoinbase *m_op; + CAmount sendAmount; +public: + ShieldToAddress(AsyncRPCOperation_shieldcoinbase *op, CAmount sendAmount) : + m_op(op), sendAmount(sendAmount) {} + + bool operator()(const libzcash::SproutPaymentAddress &zaddr) const; + bool operator()(const libzcash::SaplingPaymentAddress &zaddr) const; + bool operator()(const libzcash::InvalidEncoding& no) const; +}; + // To test private methods, a friend class can act as a proxy class TEST_FRIEND_AsyncRPCOperation_shieldcoinbase { diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index d93279986..9c753f981 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3982,6 +3982,7 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp) } utxoCounter++; + auto scriptPubKey = out.tx->vout[out.i].scriptPubKey; CAmount nValue = out.tx->vout[out.i].nValue; if (!maxedOutFlag) { @@ -3992,7 +3993,7 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp) maxedOutFlag = true; } else { estimatedTxSize += increase; - ShieldCoinbaseUTXO utxo = {out.tx->GetHash(), out.i, nValue}; + ShieldCoinbaseUTXO utxo = {out.tx->GetHash(), out.i, scriptPubKey, nValue}; inputs.push_back(utxo); shieldedValue += nValue; } @@ -4027,7 +4028,12 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp) contextInfo.push_back(Pair("toaddress", params[1])); contextInfo.push_back(Pair("fee", ValueFromAmount(nFee))); + // Builder (used if Sapling addresses are involved) + TransactionBuilder builder = TransactionBuilder( + Params().GetConsensus(), nextBlockHeight, pwalletMain); + // Contextual transaction we will build on + // (used if no Sapling addresses are involved) CMutableTransaction contextualTx = CreateNewContextualCMutableTransaction( Params().GetConsensus(), nextBlockHeight); if (contextualTx.nVersion == 1) { @@ -4036,7 +4042,7 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp) // Create operation and add to global queue std::shared_ptr q = getAsyncRPCQueue(); - std::shared_ptr operation( new AsyncRPCOperation_shieldcoinbase(contextualTx, inputs, destaddress, nFee, contextInfo) ); + std::shared_ptr operation( new AsyncRPCOperation_shieldcoinbase(builder, contextualTx, inputs, destaddress, nFee, contextInfo) ); q->addOperation(operation); AsyncRPCOperationId operationId = operation->getId();