Auto merge of #3518 - str4d:3216-z_shieldcoinbase, r=str4d

Add Sapling support to z_shieldcoinbase

Part of #3216.

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'

qa/rpc-tests/wallet_listreceived.py

@@ -12,9 +12,6 @@ my_memo = 'c0ffee' # stay awake
 my_memo = my_memo + '0'*(1024-len(my_memo))
 
 no_memo = 'f6' + ('0'*1022) # see section 5.5 of the protocol spec
-# sapling generates zero_memo, but this may be fixed soon (to no_memo)
-# then this test can be simplified
-zero_memo = '0'*1024
 
 fee = Decimal('0.0001')
 
@@ -95,7 +92,7 @@ class ListReceivedTest (BitcoinTestFramework):
 
     def run_test(self):
         self.run_test_release('sprout', no_memo, 200)
-        self.run_test_release('sapling', zero_memo, 204)
+        self.run_test_release('sapling', no_memo, 204)
 
 if __name__ == '__main__':
     ListReceivedTest().main()

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']
-
-        # 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']
-
-        # 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)
+        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(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)
+
+        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()

qa/rpc-tests/wallet_shieldcoinbase_sapling.py

@@ -0,0 +1,16 @@
+#!/usr/bin/env python2
+import inspect
+import os
+
+# To keep pyflakes happy
+WalletShieldCoinbaseTest = object
+
+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()

qa/rpc-tests/wallet_shieldcoinbase_sprout.py

@@ -0,0 +1,16 @@
+#!/usr/bin/env python2
+import inspect
+import os
+
+# To keep pyflakes happy
+WalletShieldCoinbaseTest = object
+
+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()

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<AsyncRPCOperation> operation(new AsyncRPCOperation_shieldcoinbase(mtx, {}, testnetzaddr, -1 ));
+        std::shared_ptr<AsyncRPCOperation> 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<AsyncRPCOperation> operation(new AsyncRPCOperation_shieldcoinbase(mtx, {}, testnetzaddr, 1));
+        std::shared_ptr<AsyncRPCOperation> 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<ShieldCoinbaseUTXO> inputs = { ShieldCoinbaseUTXO{uint256(),0,0} };
-        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_shieldcoinbase(mtx, inputs, mainnetzaddr, 1) );
+        std::shared_ptr<AsyncRPCOperation> 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<ShieldCoinbaseUTXO> inputs = { ShieldCoinbaseUTXO{uint256(),0,0}, ShieldCoinbaseUTXO{uint256(),0,0} };
-        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_shieldcoinbase(mtx, inputs, zaddr) );
+        std::shared_ptr<AsyncRPCOperation> 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<ShieldCoinbaseUTXO> inputs = { ShieldCoinbaseUTXO{uint256(),0,0} };
-        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_shieldcoinbase(mtx, inputs, zaddr) );
+        std::shared_ptr<AsyncRPCOperation> 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<ShieldCoinbaseUTXO> inputs = { ShieldCoinbaseUTXO{uint256(),0,100000} };
-        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_shieldcoinbase(mtx, inputs, zaddr) );
+        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_shieldcoinbase(TransactionBuilder(), mtx, inputs, zaddr) );
         std::shared_ptr<AsyncRPCOperation_shieldcoinbase> ptr = std::dynamic_pointer_cast<AsyncRPCOperation_shieldcoinbase> (operation);
         TEST_FRIEND_AsyncRPCOperation_shieldcoinbase proxy(ptr);
         static_cast<AsyncRPCOperation_shieldcoinbase *>(operation.get())->testmode = true;

src/transaction_builder.cpp

@@ -128,7 +128,7 @@ boost::optional<CTransaction> TransactionBuilder::Build()
         // Send change to the specified change address. If no change address
         // was set, send change to the first Sapling address given as input.
         if (zChangeAddr) {
-            AddSaplingOutput(zChangeAddr->first, zChangeAddr->second, change, {});
+            AddSaplingOutput(zChangeAddr->first, zChangeAddr->second, change);
         } else if (tChangeAddr) {
             // tChangeAddr has already been validated.
             assert(AddTransparentOutput(tChangeAddr.value(), change));
@@ -136,7 +136,7 @@ boost::optional<CTransaction> TransactionBuilder::Build()
             auto fvk = spends[0].expsk.full_viewing_key();
             auto note = spends[0].note;
             libzcash::SaplingPaymentAddress changeAddr(note.d, note.pk_d);
-            AddSaplingOutput(fvk.ovk, changeAddr, change, {});
+            AddSaplingOutput(fvk.ovk, changeAddr, change);
         } else {
             return boost::none;
         }

src/transaction_builder.h

@@ -86,7 +86,7 @@ public:
         uint256 ovk,
         libzcash::SaplingPaymentAddress to,
         CAmount value,
-        std::array<unsigned char, ZC_MEMO_SIZE> memo);
+        std::array<unsigned char, ZC_MEMO_SIZE> memo = {{0xF6}});
 
     // Assumes that the value correctly corresponds to the provided UTXO.
     void AddTransparentInput(COutPoint utxo, CScript scriptPubKey, CAmount value);

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<ShieldCoinbaseUTXO> 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<libzcash::SproutPaymentAddress>(&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<libzcash::SproutPaymentAddress>(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"

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<ShieldCoinbaseUTXO> inputs, std::string toAddress, CAmount fee = SHIELD_COINBASE_DEFAULT_MINERS_FEE, UniValue contextInfo = NullUniValue);
+    AsyncRPCOperation_shieldcoinbase(
+        TransactionBuilder builder,
+        CMutableTransaction contextualTx,
+        std::vector<ShieldCoinbaseUTXO> 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<ShieldCoinbaseUTXO> inputs_;
 
+    TransactionBuilder builder_;
     CTransaction tx_;
 
     bool main_impl();
@@ -88,6 +98,20 @@ private:
     std::vector<PaymentDisclosureKeyInfo> paymentDisclosureData_;
 };
 
+class ShieldToAddress : public boost::static_visitor<bool>
+{
+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 {

src/wallet/rpcwallet.cpp

@@ -4006,6 +4006,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) {
@@ -4016,7 +4017,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;
             }
@@ -4051,7 +4052,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) {
@@ -4060,7 +4066,7 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp)
 
     // Create operation and add to global queue
     std::shared_ptr<AsyncRPCQueue> q = getAsyncRPCQueue();
-    std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_shieldcoinbase(contextualTx, inputs, destaddress, nFee, contextInfo) );
+    std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_shieldcoinbase(builder, contextualTx, inputs, destaddress, nFee, contextInfo) );
     q->addOperation(operation);
     AsyncRPCOperationId operationId = operation->getId();