From ba63dacbbbc647e4a84824a7aee5838c5bcabab4 Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@z.cash>
Date: Thu, 13 Sep 2018 22:04:00 +0100
Subject: [PATCH 01/14] Use the correct empty memo for Sapling outputs

---
 src/transaction_builder.cpp | 4 ++--
 src/transaction_builder.h   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/transaction_builder.cpp b/src/transaction_builder.cpp
index e137cbdb8..880edbbc7 100644
--- a/src/transaction_builder.cpp
+++ b/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;
         }
diff --git a/src/transaction_builder.h b/src/transaction_builder.h
index 560898381..c7eca80f1 100644
--- a/src/transaction_builder.h
+++ b/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);

From 5f91a9564183fc81724fd2615fbf762571a0500e Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@z.cash>
Date: Thu, 13 Sep 2018 23:12:29 +0100
Subject: [PATCH 02/14] Add Sapling support to z_shieldcoinbase

Part of #3216.
---
 qa/pull-tester/rpc-tests.sh                   |  3 +-
 qa/rpc-tests/wallet_shieldcoinbase.py         | 81 +++++++++-------
 qa/rpc-tests/wallet_shieldcoinbase_sapling.py | 13 +++
 qa/rpc-tests/wallet_shieldcoinbase_sprout.py  | 13 +++
 src/test/rpc_wallet_tests.cpp                 | 12 +--
 .../asyncrpcoperation_shieldcoinbase.cpp      | 93 ++++++++++++++++---
 src/wallet/asyncrpcoperation_shieldcoinbase.h | 26 +++++-
 src/wallet/rpcwallet.cpp                      | 10 +-
 8 files changed, 194 insertions(+), 57 deletions(-)
 create mode 100755 qa/rpc-tests/wallet_shieldcoinbase_sapling.py
 create mode 100755 qa/rpc-tests/wallet_shieldcoinbase_sprout.py

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<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;
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<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"
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<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 {
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<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();
 

From 5ead4b171388b5fe96d6639a0ffccb0c390a3ccd Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@z.cash>
Date: Wed, 3 Oct 2018 16:08:00 +0100
Subject: [PATCH 03/14] Revert "Get rid of
 consensus.fPowAllowMinDifficultyBlocks."

This reverts commit dffc025d38bb12b655bfde53de6dd237840c7d8e.
---
 src/chainparams.cpp    | 3 +++
 src/consensus/params.h | 1 +
 src/miner.cpp          | 9 +++++++++
 3 files changed, 13 insertions(+)

diff --git a/src/chainparams.cpp b/src/chainparams.cpp
index 46777f54e..b0352a66d 100644
--- a/src/chainparams.cpp
+++ b/src/chainparams.cpp
@@ -95,6 +95,7 @@ public:
         consensus.nPowMaxAdjustDown = 32; // 32% adjustment down
         consensus.nPowMaxAdjustUp = 16; // 16% adjustment up
         consensus.nPowTargetSpacing = 2.5 * 60;
+        consensus.fPowAllowMinDifficultyBlocks = false;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nProtocolVersion = 170002;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nActivationHeight =
             Consensus::NetworkUpgrade::ALWAYS_ACTIVE;
@@ -271,6 +272,7 @@ public:
         consensus.nPowMaxAdjustDown = 32; // 32% adjustment down
         consensus.nPowMaxAdjustUp = 16; // 16% adjustment up
         consensus.nPowTargetSpacing = 2.5 * 60;
+        consensus.fPowAllowMinDifficultyBlocks = true;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nProtocolVersion = 170002;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nActivationHeight =
             Consensus::NetworkUpgrade::ALWAYS_ACTIVE;
@@ -391,6 +393,7 @@ public:
         consensus.nPowMaxAdjustDown = 0; // Turn off adjustment down
         consensus.nPowMaxAdjustUp = 0; // Turn off adjustment up
         consensus.nPowTargetSpacing = 2.5 * 60;
+        consensus.fPowAllowMinDifficultyBlocks = true;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nProtocolVersion = 170002;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nActivationHeight =
             Consensus::NetworkUpgrade::ALWAYS_ACTIVE;
diff --git a/src/consensus/params.h b/src/consensus/params.h
index c21607046..50eb69a45 100644
--- a/src/consensus/params.h
+++ b/src/consensus/params.h
@@ -91,6 +91,7 @@ struct Params {
     NetworkUpgrade vUpgrades[MAX_NETWORK_UPGRADES];
     /** Proof of work parameters */
     uint256 powLimit;
+    bool fPowAllowMinDifficultyBlocks;
     int64_t nPowAveragingWindow;
     int64_t nPowMaxAdjustDown;
     int64_t nPowMaxAdjustUp;
diff --git a/src/miner.cpp b/src/miner.cpp
index e22ac2c62..19912eb67 100644
--- a/src/miner.cpp
+++ b/src/miner.cpp
@@ -101,6 +101,10 @@ public:
 void UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev)
 {
     pblock->nTime = std::max(pindexPrev->GetMedianTimePast()+1, GetAdjustedTime());
+
+    // Updating time can change work required on testnet:
+    if (consensusParams.fPowAllowMinDifficultyBlocks)
+        pblock->nBits = GetNextWorkRequired(pindexPrev, pblock, consensusParams);
 }
 
 CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn)
@@ -722,6 +726,11 @@ void static BitcoinMiner()
                 // Update nNonce and nTime
                 pblock->nNonce = ArithToUint256(UintToArith256(pblock->nNonce) + 1);
                 UpdateTime(pblock, chainparams.GetConsensus(), pindexPrev);
+                if (chainparams.GetConsensus().fPowAllowMinDifficultyBlocks)
+                {
+                    // Changing pblock->nTime can change work required on testnet:
+                    hashTarget.SetCompact(pblock->nBits);
+                }
             }
         }
     }

From 639e46b4d7c6d9fcaab1a76433fb174364c84413 Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@z.cash>
Date: Wed, 3 Oct 2018 16:15:03 +0100
Subject: [PATCH 04/14] Revert "Remove testnet-only difficulty rules"

This reverts commit 333ea3c4266179da5d40e496ae60bcce0259c790.
---
 src/gtest/test_rpc.cpp | 27 +++++++++++++++++++++++++++
 src/pow.cpp            | 17 +++++++++++++++++
 src/rpc/blockchain.cpp | 26 +++++++++++++++++---------
 3 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/src/gtest/test_rpc.cpp b/src/gtest/test_rpc.cpp
index 3733379a8..ad80393f5 100644
--- a/src/gtest/test_rpc.cpp
+++ b/src/gtest/test_rpc.cpp
@@ -9,6 +9,33 @@
 #include "streams.h"
 #include "utilstrencodings.h"
 
+TEST(rpc, GetDifficultyTestnetRules) {
+    SelectParams(CBaseChainParams::TESTNET);
+
+    CBlockIndex prev;
+    prev.nTime = 1472700000;
+    prev.nBits = 0x201fffff;
+
+    CBlockIndex curr;
+    curr.pprev = &prev;
+    curr.nTime = 1472700300;
+    curr.nBits = 0x207fffff;
+
+    // Time interval is within 5 minutes, so the min-difficulty block should be
+    // interpreted as a valid network difficulty.
+    EXPECT_EQ(1, GetDifficulty(&curr));
+    EXPECT_EQ(1, GetNetworkDifficulty(&curr));
+
+    curr.nTime += 1;
+
+    // Time interval is over 5 minutes, so the min-difficulty block should be
+    // ignored for network difficulty determination.
+    EXPECT_EQ(1, GetDifficulty(&curr));
+    // We have to check this directly, because of some combination of rounding
+    // and truncation issues that result in Google Test displaying 4 != 4
+    EXPECT_EQ((double)0x7fffff/(double)0x1fffff, GetNetworkDifficulty(&curr));
+}
+
 extern UniValue blockToJSON(const CBlock& block, const CBlockIndex* blockindex, bool txDetails = false);
 
 TEST(rpc, check_blockToJSON_returns_minified_solution) {
diff --git a/src/pow.cpp b/src/pow.cpp
index 18873d4ee..0eb81414a 100644
--- a/src/pow.cpp
+++ b/src/pow.cpp
@@ -24,6 +24,23 @@ unsigned int GetNextWorkRequired(const CBlockIndex* pindexLast, const CBlockHead
     if (pindexLast == NULL)
         return nProofOfWorkLimit;
 
+    const CBlockIndex* pindexBits = pindexLast;
+    {
+        if (params.fPowAllowMinDifficultyBlocks)
+        {
+            // Special difficulty rule for testnet:
+            // If the new block's timestamp is more than 2* 2.5 minutes
+            // then allow mining of a min-difficulty block.
+            if (pblock->GetBlockTime() > pindexLast->GetBlockTime() + params.nPowTargetSpacing*2)
+                return nProofOfWorkLimit;
+            else {
+                // Get the last non-min-difficulty (or at worst the genesis difficulty)
+                while (pindexBits->pprev && pindexBits->nBits == nProofOfWorkLimit)
+                    pindexBits = pindexBits->pprev;
+            }
+        }
+    }
+
     // Find the first block in the averaging interval
     const CBlockIndex* pindexFirst = pindexLast;
     arith_uint256 bnTot {0};
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 9a9b0fd57..c7afe7af8 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -38,21 +38,29 @@ double GetDifficultyINTERNAL(const CBlockIndex* blockindex, bool networkDifficul
             blockindex = chainActive.Tip();
     }
 
-    uint32_t bits;
-    if (networkDifficulty) {
-        bits = GetNextWorkRequired(blockindex, nullptr, Params().GetConsensus());
-    } else {
-        bits = blockindex->nBits;
-    }
-
     uint32_t powLimit =
         UintToArith256(Params().GetConsensus().powLimit).GetCompact();
-    int nShift = (bits >> 24) & 0xff;
+    {
+        if (networkDifficulty && Params().GetConsensus().fPowAllowMinDifficultyBlocks)
+        {
+            // Special difficulty rule for testnet:
+            // If a block's timestamp is more than 2*nPowTargetSpacing minutes after
+            // the previous block, then it is permitted to be min-difficulty. So
+            // get the last non-min-difficulty (or at worst the genesis difficulty).
+            auto window = Params().GetConsensus().nPowTargetSpacing*2;
+            while (blockindex->pprev && blockindex->nBits == powLimit &&
+                    blockindex->GetBlockTime() > blockindex->pprev->GetBlockTime() + window) {
+                blockindex = blockindex->pprev;
+            }
+        }
+    }
+
+    int nShift = (blockindex->nBits >> 24) & 0xff;
     int nShiftAmount = (powLimit >> 24) & 0xff;
 
     double dDiff =
         (double)(powLimit & 0x00ffffff) / 
-        (double)(bits & 0x00ffffff);
+        (double)(blockindex->nBits & 0x00ffffff);
 
     while (nShift < nShiftAmount)
     {

From 1702a86455c426da46473770c990257c4a1e087d Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@z.cash>
Date: Wed, 3 Oct 2018 17:50:17 +0100
Subject: [PATCH 05/14] Allow minimum-difficulty blocks on testnet and regtest

A block may be mined with nBits set to the minimum difficulty if its
nTime is set more than six block intervals (15 minutes) after its parent
block.

This is a consensus rule change on testnet that will result in a chain
split (as desired).
---
 src/gtest/test_pow.cpp | 41 +++++++++++++++++++++++++++++++++++++++++
 src/gtest/test_rpc.cpp | 27 ---------------------------
 src/pow.cpp            | 10 ++--------
 src/rpc/blockchain.cpp | 26 +++++++++-----------------
 4 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/src/gtest/test_pow.cpp b/src/gtest/test_pow.cpp
index ac3ce59e1..80e0d0921 100644
--- a/src/gtest/test_pow.cpp
+++ b/src/gtest/test_pow.cpp
@@ -68,3 +68,44 @@ TEST(PoW, DifficultyAveraging) {
                                         params),
               GetNextWorkRequired(&blocks[lastBlk], nullptr, params));
 }
+
+TEST(PoW, MinDifficultyRules) {
+    SelectParams(CBaseChainParams::TESTNET);
+    const Consensus::Params& params = Params().GetConsensus();
+    size_t lastBlk = 2*params.nPowAveragingWindow;
+    size_t firstBlk = lastBlk - params.nPowAveragingWindow;
+
+    // Start with blocks evenly-spaced and equal difficulty
+    std::vector<CBlockIndex> blocks(lastBlk+1);
+    for (int i = 0; i <= lastBlk; i++) {
+        blocks[i].pprev = i ? &blocks[i - 1] : nullptr;
+        blocks[i].nHeight = i;
+        blocks[i].nTime = 1269211443 + i * params.nPowTargetSpacing;
+        blocks[i].nBits = 0x1e7fffff; /* target 0x007fffff000... */
+        blocks[i].nChainWork = i ? blocks[i - 1].nChainWork + GetBlockProof(blocks[i - 1]) : arith_uint256(0);
+    }
+
+    // Create a new block at the target spacing
+    CBlockHeader next;
+    next.nTime = blocks[lastBlk].nTime + params.nPowTargetSpacing;
+
+    // Result should be unchanged, modulo integer division precision loss
+    arith_uint256 bnRes;
+    bnRes.SetCompact(0x1e7fffff);
+    bnRes /= params.AveragingWindowTimespan();
+    bnRes *= params.AveragingWindowTimespan();
+    EXPECT_EQ(GetNextWorkRequired(&blocks[lastBlk], &next, params), bnRes.GetCompact());
+
+    // Delay last block up to the edge of the min-difficulty limit
+    next.nTime += params.nPowTargetSpacing * 5;
+
+    // Result should be unchanged, modulo integer division precision loss
+    EXPECT_EQ(GetNextWorkRequired(&blocks[lastBlk], &next, params), bnRes.GetCompact());
+
+    // Delay last block over the min-difficulty limit
+    next.nTime += 1;
+
+    // Result should be the minimum difficulty
+    EXPECT_EQ(GetNextWorkRequired(&blocks[lastBlk], &next, params),
+              UintToArith256(params.powLimit).GetCompact());
+}
diff --git a/src/gtest/test_rpc.cpp b/src/gtest/test_rpc.cpp
index ad80393f5..3733379a8 100644
--- a/src/gtest/test_rpc.cpp
+++ b/src/gtest/test_rpc.cpp
@@ -9,33 +9,6 @@
 #include "streams.h"
 #include "utilstrencodings.h"
 
-TEST(rpc, GetDifficultyTestnetRules) {
-    SelectParams(CBaseChainParams::TESTNET);
-
-    CBlockIndex prev;
-    prev.nTime = 1472700000;
-    prev.nBits = 0x201fffff;
-
-    CBlockIndex curr;
-    curr.pprev = &prev;
-    curr.nTime = 1472700300;
-    curr.nBits = 0x207fffff;
-
-    // Time interval is within 5 minutes, so the min-difficulty block should be
-    // interpreted as a valid network difficulty.
-    EXPECT_EQ(1, GetDifficulty(&curr));
-    EXPECT_EQ(1, GetNetworkDifficulty(&curr));
-
-    curr.nTime += 1;
-
-    // Time interval is over 5 minutes, so the min-difficulty block should be
-    // ignored for network difficulty determination.
-    EXPECT_EQ(1, GetDifficulty(&curr));
-    // We have to check this directly, because of some combination of rounding
-    // and truncation issues that result in Google Test displaying 4 != 4
-    EXPECT_EQ((double)0x7fffff/(double)0x1fffff, GetNetworkDifficulty(&curr));
-}
-
 extern UniValue blockToJSON(const CBlock& block, const CBlockIndex* blockindex, bool txDetails = false);
 
 TEST(rpc, check_blockToJSON_returns_minified_solution) {
diff --git a/src/pow.cpp b/src/pow.cpp
index 0eb81414a..40e69bb52 100644
--- a/src/pow.cpp
+++ b/src/pow.cpp
@@ -24,20 +24,14 @@ unsigned int GetNextWorkRequired(const CBlockIndex* pindexLast, const CBlockHead
     if (pindexLast == NULL)
         return nProofOfWorkLimit;
 
-    const CBlockIndex* pindexBits = pindexLast;
     {
         if (params.fPowAllowMinDifficultyBlocks)
         {
             // Special difficulty rule for testnet:
-            // If the new block's timestamp is more than 2* 2.5 minutes
+            // If the new block's timestamp is more than 6 * 2.5 minutes
             // then allow mining of a min-difficulty block.
-            if (pblock->GetBlockTime() > pindexLast->GetBlockTime() + params.nPowTargetSpacing*2)
+            if (pblock && pblock->GetBlockTime() > pindexLast->GetBlockTime() + params.nPowTargetSpacing * 6)
                 return nProofOfWorkLimit;
-            else {
-                // Get the last non-min-difficulty (or at worst the genesis difficulty)
-                while (pindexBits->pprev && pindexBits->nBits == nProofOfWorkLimit)
-                    pindexBits = pindexBits->pprev;
-            }
         }
     }
 
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index c7afe7af8..9a9b0fd57 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -38,29 +38,21 @@ double GetDifficultyINTERNAL(const CBlockIndex* blockindex, bool networkDifficul
             blockindex = chainActive.Tip();
     }
 
-    uint32_t powLimit =
-        UintToArith256(Params().GetConsensus().powLimit).GetCompact();
-    {
-        if (networkDifficulty && Params().GetConsensus().fPowAllowMinDifficultyBlocks)
-        {
-            // Special difficulty rule for testnet:
-            // If a block's timestamp is more than 2*nPowTargetSpacing minutes after
-            // the previous block, then it is permitted to be min-difficulty. So
-            // get the last non-min-difficulty (or at worst the genesis difficulty).
-            auto window = Params().GetConsensus().nPowTargetSpacing*2;
-            while (blockindex->pprev && blockindex->nBits == powLimit &&
-                    blockindex->GetBlockTime() > blockindex->pprev->GetBlockTime() + window) {
-                blockindex = blockindex->pprev;
-            }
-        }
+    uint32_t bits;
+    if (networkDifficulty) {
+        bits = GetNextWorkRequired(blockindex, nullptr, Params().GetConsensus());
+    } else {
+        bits = blockindex->nBits;
     }
 
-    int nShift = (blockindex->nBits >> 24) & 0xff;
+    uint32_t powLimit =
+        UintToArith256(Params().GetConsensus().powLimit).GetCompact();
+    int nShift = (bits >> 24) & 0xff;
     int nShiftAmount = (powLimit >> 24) & 0xff;
 
     double dDiff =
         (double)(powLimit & 0x00ffffff) / 
-        (double)(blockindex->nBits & 0x00ffffff);
+        (double)(bits & 0x00ffffff);
 
     while (nShift < nShiftAmount)
     {

From f0dcfceb81b8a373eea5900a6fd352209441f119 Mon Sep 17 00:00:00 2001
From: Simon <simon@bitcartel.com>
Date: Wed, 3 Oct 2018 15:40:50 -0700
Subject: [PATCH 06/14] Closes #3560. Update Sapling note data correctly when
 importing a key.

---
 qa/rpc-tests/wallet_sapling.py | 8 ++++++++
 src/wallet/wallet.cpp          | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/qa/rpc-tests/wallet_sapling.py b/qa/rpc-tests/wallet_sapling.py
index bba426868..c3b17af50 100755
--- a/qa/rpc-tests/wallet_sapling.py
+++ b/qa/rpc-tests/wallet_sapling.py
@@ -136,5 +136,13 @@ class WalletSaplingTest(BitcoinTestFramework):
         assert('outCiphertext' in shieldedOutput)
         assert('proof' in shieldedOutput)
 
+        # Verify importing a spending key will update the nullifiers and witnesses correctly
+        sk0 = self.nodes[0].z_exportkey(saplingAddr0)
+        self.nodes[2].z_importkey(sk0, "yes")
+        assert_equal(self.nodes[2].z_getbalance(saplingAddr0), Decimal('10'))
+        sk1 = self.nodes[1].z_exportkey(saplingAddr1)
+        self.nodes[2].z_importkey(sk1, "yes")
+        assert_equal(self.nodes[2].z_getbalance(saplingAddr1), Decimal('5'))
+
 if __name__ == '__main__':
     WalletSaplingTest().main()
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index d838f485e..2125f0d1c 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2367,7 +2367,7 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate)
                 }
             }
             // Increment note witness caches
-            IncrementNoteWitnesses(pindex, &block, sproutTree, saplingTree);
+            ChainTip(pindex, &block, sproutTree, saplingTree, true);
 
             pindex = chainActive.Next(pindex);
             if (GetTime() >= nNow + 60) {

From b86dc98047a1cf373c11cbb5bb9cfde65aa4a67c Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@z.cash>
Date: Thu, 4 Oct 2018 14:05:27 +0100
Subject: [PATCH 07/14] Only enable min-difficulty blocks on testnet from a
 particular height

The min-difficulty change is a bilateral consensus rule change, and so
must be conditionally enabled in order for the earlier section of the
chain to synchronise.

Technically this could be implemented as a network upgrade, but as this will
never be deployed to mainnet, a targeted fork will suffice.
---
 src/chainparams.cpp    | 6 +++---
 src/consensus/params.h | 4 +++-
 src/miner.cpp          | 5 +++--
 src/pow.cpp            | 3 ++-
 4 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/chainparams.cpp b/src/chainparams.cpp
index b0352a66d..25f9f45f3 100644
--- a/src/chainparams.cpp
+++ b/src/chainparams.cpp
@@ -95,7 +95,7 @@ public:
         consensus.nPowMaxAdjustDown = 32; // 32% adjustment down
         consensus.nPowMaxAdjustUp = 16; // 16% adjustment up
         consensus.nPowTargetSpacing = 2.5 * 60;
-        consensus.fPowAllowMinDifficultyBlocks = false;
+        consensus.nPowAllowMinDifficultyBlocksFromHeight = boost::none;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nProtocolVersion = 170002;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nActivationHeight =
             Consensus::NetworkUpgrade::ALWAYS_ACTIVE;
@@ -272,7 +272,7 @@ public:
         consensus.nPowMaxAdjustDown = 32; // 32% adjustment down
         consensus.nPowMaxAdjustUp = 16; // 16% adjustment up
         consensus.nPowTargetSpacing = 2.5 * 60;
-        consensus.fPowAllowMinDifficultyBlocks = true;
+        consensus.nPowAllowMinDifficultyBlocksFromHeight = 299187;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nProtocolVersion = 170002;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nActivationHeight =
             Consensus::NetworkUpgrade::ALWAYS_ACTIVE;
@@ -393,7 +393,7 @@ public:
         consensus.nPowMaxAdjustDown = 0; // Turn off adjustment down
         consensus.nPowMaxAdjustUp = 0; // Turn off adjustment up
         consensus.nPowTargetSpacing = 2.5 * 60;
-        consensus.fPowAllowMinDifficultyBlocks = true;
+        consensus.nPowAllowMinDifficultyBlocksFromHeight = 0;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nProtocolVersion = 170002;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nActivationHeight =
             Consensus::NetworkUpgrade::ALWAYS_ACTIVE;
diff --git a/src/consensus/params.h b/src/consensus/params.h
index 50eb69a45..4e6f7b86a 100644
--- a/src/consensus/params.h
+++ b/src/consensus/params.h
@@ -8,6 +8,8 @@
 
 #include "uint256.h"
 
+#include <boost/optional.hpp>
+
 namespace Consensus {
 
 /**
@@ -91,7 +93,7 @@ struct Params {
     NetworkUpgrade vUpgrades[MAX_NETWORK_UPGRADES];
     /** Proof of work parameters */
     uint256 powLimit;
-    bool fPowAllowMinDifficultyBlocks;
+    boost::optional<uint32_t> nPowAllowMinDifficultyBlocksFromHeight;
     int64_t nPowAveragingWindow;
     int64_t nPowMaxAdjustDown;
     int64_t nPowMaxAdjustUp;
diff --git a/src/miner.cpp b/src/miner.cpp
index 19912eb67..960ac60de 100644
--- a/src/miner.cpp
+++ b/src/miner.cpp
@@ -103,8 +103,9 @@ void UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams,
     pblock->nTime = std::max(pindexPrev->GetMedianTimePast()+1, GetAdjustedTime());
 
     // Updating time can change work required on testnet:
-    if (consensusParams.fPowAllowMinDifficultyBlocks)
+    if (consensusParams.nPowAllowMinDifficultyBlocksFromHeight) {
         pblock->nBits = GetNextWorkRequired(pindexPrev, pblock, consensusParams);
+    }
 }
 
 CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn)
@@ -726,7 +727,7 @@ void static BitcoinMiner()
                 // Update nNonce and nTime
                 pblock->nNonce = ArithToUint256(UintToArith256(pblock->nNonce) + 1);
                 UpdateTime(pblock, chainparams.GetConsensus(), pindexPrev);
-                if (chainparams.GetConsensus().fPowAllowMinDifficultyBlocks)
+                if (chainparams.GetConsensus().nPowAllowMinDifficultyBlocksFromHeight)
                 {
                     // Changing pblock->nTime can change work required on testnet:
                     hashTarget.SetCompact(pblock->nBits);
diff --git a/src/pow.cpp b/src/pow.cpp
index 40e69bb52..7bc2fa79f 100644
--- a/src/pow.cpp
+++ b/src/pow.cpp
@@ -25,7 +25,8 @@ unsigned int GetNextWorkRequired(const CBlockIndex* pindexLast, const CBlockHead
         return nProofOfWorkLimit;
 
     {
-        if (params.fPowAllowMinDifficultyBlocks)
+        if (params.nPowAllowMinDifficultyBlocksFromHeight &&
+            pindexLast->nHeight >= params.nPowAllowMinDifficultyBlocksFromHeight.get())
         {
             // Special difficulty rule for testnet:
             // If the new block's timestamp is more than 6 * 2.5 minutes

From f09aae037c9e13469859ee7e658a09710d1a57ce Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@z.cash>
Date: Thu, 4 Oct 2018 22:46:15 +0100
Subject: [PATCH 08/14] Update wallet_listreceived test for now-correct empty
 Sapling memos

---
 qa/rpc-tests/wallet_listreceived.py | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/qa/rpc-tests/wallet_listreceived.py b/qa/rpc-tests/wallet_listreceived.py
index 88d4f8cc7..9ece27772 100755
--- a/qa/rpc-tests/wallet_listreceived.py
+++ b/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()

From 1f7ee4af70ece8759f7a4e6d8df9c39284bb1482 Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@z.cash>
Date: Thu, 4 Oct 2018 23:26:05 +0100
Subject: [PATCH 09/14] Rename min-difficulty flag to remove off-by-one in the
 name

---
 src/chainparams.cpp    | 6 +++---
 src/consensus/params.h | 2 +-
 src/miner.cpp          | 4 ++--
 src/pow.cpp            | 6 ++++--
 4 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/src/chainparams.cpp b/src/chainparams.cpp
index 25f9f45f3..8ebf04c6f 100644
--- a/src/chainparams.cpp
+++ b/src/chainparams.cpp
@@ -95,7 +95,7 @@ public:
         consensus.nPowMaxAdjustDown = 32; // 32% adjustment down
         consensus.nPowMaxAdjustUp = 16; // 16% adjustment up
         consensus.nPowTargetSpacing = 2.5 * 60;
-        consensus.nPowAllowMinDifficultyBlocksFromHeight = boost::none;
+        consensus.nPowAllowMinDifficultyBlocksAfterHeight = boost::none;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nProtocolVersion = 170002;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nActivationHeight =
             Consensus::NetworkUpgrade::ALWAYS_ACTIVE;
@@ -272,7 +272,7 @@ public:
         consensus.nPowMaxAdjustDown = 32; // 32% adjustment down
         consensus.nPowMaxAdjustUp = 16; // 16% adjustment up
         consensus.nPowTargetSpacing = 2.5 * 60;
-        consensus.nPowAllowMinDifficultyBlocksFromHeight = 299187;
+        consensus.nPowAllowMinDifficultyBlocksAfterHeight = 299187;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nProtocolVersion = 170002;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nActivationHeight =
             Consensus::NetworkUpgrade::ALWAYS_ACTIVE;
@@ -393,7 +393,7 @@ public:
         consensus.nPowMaxAdjustDown = 0; // Turn off adjustment down
         consensus.nPowMaxAdjustUp = 0; // Turn off adjustment up
         consensus.nPowTargetSpacing = 2.5 * 60;
-        consensus.nPowAllowMinDifficultyBlocksFromHeight = 0;
+        consensus.nPowAllowMinDifficultyBlocksAfterHeight = 0;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nProtocolVersion = 170002;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nActivationHeight =
             Consensus::NetworkUpgrade::ALWAYS_ACTIVE;
diff --git a/src/consensus/params.h b/src/consensus/params.h
index 4e6f7b86a..26288f243 100644
--- a/src/consensus/params.h
+++ b/src/consensus/params.h
@@ -93,7 +93,7 @@ struct Params {
     NetworkUpgrade vUpgrades[MAX_NETWORK_UPGRADES];
     /** Proof of work parameters */
     uint256 powLimit;
-    boost::optional<uint32_t> nPowAllowMinDifficultyBlocksFromHeight;
+    boost::optional<uint32_t> nPowAllowMinDifficultyBlocksAfterHeight;
     int64_t nPowAveragingWindow;
     int64_t nPowMaxAdjustDown;
     int64_t nPowMaxAdjustUp;
diff --git a/src/miner.cpp b/src/miner.cpp
index 960ac60de..b76226ecc 100644
--- a/src/miner.cpp
+++ b/src/miner.cpp
@@ -103,7 +103,7 @@ void UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams,
     pblock->nTime = std::max(pindexPrev->GetMedianTimePast()+1, GetAdjustedTime());
 
     // Updating time can change work required on testnet:
-    if (consensusParams.nPowAllowMinDifficultyBlocksFromHeight) {
+    if (consensusParams.nPowAllowMinDifficultyBlocksAfterHeight) {
         pblock->nBits = GetNextWorkRequired(pindexPrev, pblock, consensusParams);
     }
 }
@@ -727,7 +727,7 @@ void static BitcoinMiner()
                 // Update nNonce and nTime
                 pblock->nNonce = ArithToUint256(UintToArith256(pblock->nNonce) + 1);
                 UpdateTime(pblock, chainparams.GetConsensus(), pindexPrev);
-                if (chainparams.GetConsensus().nPowAllowMinDifficultyBlocksFromHeight)
+                if (chainparams.GetConsensus().nPowAllowMinDifficultyBlocksAfterHeight)
                 {
                     // Changing pblock->nTime can change work required on testnet:
                     hashTarget.SetCompact(pblock->nBits);
diff --git a/src/pow.cpp b/src/pow.cpp
index 7bc2fa79f..82c817efb 100644
--- a/src/pow.cpp
+++ b/src/pow.cpp
@@ -25,8 +25,10 @@ unsigned int GetNextWorkRequired(const CBlockIndex* pindexLast, const CBlockHead
         return nProofOfWorkLimit;
 
     {
-        if (params.nPowAllowMinDifficultyBlocksFromHeight &&
-            pindexLast->nHeight >= params.nPowAllowMinDifficultyBlocksFromHeight.get())
+        // Comparing to pindexLast->nHeight with >= because this function
+        // returns the work required for the block after pindexLast.
+        if (params.nPowAllowMinDifficultyBlocksAfterHeight &&
+            pindexLast->nHeight >= params.nPowAllowMinDifficultyBlocksAfterHeight.get())
         {
             // Special difficulty rule for testnet:
             // If the new block's timestamp is more than 6 * 2.5 minutes

From 4c90270469e32cbf3459ec2fd25d46a089bd5c58 Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@z.cash>
Date: Thu, 4 Oct 2018 23:29:11 +0100
Subject: [PATCH 10/14] Explicitly check the min-difficulty flag against
 boost::none

It isn't clear how a boost::optional that holds 0 (which is the case for
regtest) is coerced to a boolean, unless you pore over the Boost
documentation. An explicit check is clearer.
---
 src/miner.cpp | 4 ++--
 src/pow.cpp   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/miner.cpp b/src/miner.cpp
index b76226ecc..e2e5d2f7e 100644
--- a/src/miner.cpp
+++ b/src/miner.cpp
@@ -103,7 +103,7 @@ void UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams,
     pblock->nTime = std::max(pindexPrev->GetMedianTimePast()+1, GetAdjustedTime());
 
     // Updating time can change work required on testnet:
-    if (consensusParams.nPowAllowMinDifficultyBlocksAfterHeight) {
+    if (consensusParams.nPowAllowMinDifficultyBlocksAfterHeight != boost::none) {
         pblock->nBits = GetNextWorkRequired(pindexPrev, pblock, consensusParams);
     }
 }
@@ -727,7 +727,7 @@ void static BitcoinMiner()
                 // Update nNonce and nTime
                 pblock->nNonce = ArithToUint256(UintToArith256(pblock->nNonce) + 1);
                 UpdateTime(pblock, chainparams.GetConsensus(), pindexPrev);
-                if (chainparams.GetConsensus().nPowAllowMinDifficultyBlocksAfterHeight)
+                if (chainparams.GetConsensus().nPowAllowMinDifficultyBlocksAfterHeight != boost::none)
                 {
                     // Changing pblock->nTime can change work required on testnet:
                     hashTarget.SetCompact(pblock->nBits);
diff --git a/src/pow.cpp b/src/pow.cpp
index 82c817efb..743df08ee 100644
--- a/src/pow.cpp
+++ b/src/pow.cpp
@@ -27,7 +27,7 @@ unsigned int GetNextWorkRequired(const CBlockIndex* pindexLast, const CBlockHead
     {
         // Comparing to pindexLast->nHeight with >= because this function
         // returns the work required for the block after pindexLast.
-        if (params.nPowAllowMinDifficultyBlocksAfterHeight &&
+        if (params.nPowAllowMinDifficultyBlocksAfterHeight != boost::none &&
             pindexLast->nHeight >= params.nPowAllowMinDifficultyBlocksAfterHeight.get())
         {
             // Special difficulty rule for testnet:

From 2b47b0de7d3a809170d239386a291e954cb3f32b Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@z.cash>
Date: Thu, 4 Oct 2018 23:46:50 +0100
Subject: [PATCH 11/14] Position PoW.MinDifficultyRules test after rule
 activates

---
 src/gtest/test_pow.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gtest/test_pow.cpp b/src/gtest/test_pow.cpp
index 80e0d0921..4557f6db1 100644
--- a/src/gtest/test_pow.cpp
+++ b/src/gtest/test_pow.cpp
@@ -79,7 +79,7 @@ TEST(PoW, MinDifficultyRules) {
     std::vector<CBlockIndex> blocks(lastBlk+1);
     for (int i = 0; i <= lastBlk; i++) {
         blocks[i].pprev = i ? &blocks[i - 1] : nullptr;
-        blocks[i].nHeight = i;
+        blocks[i].nHeight = params.nPowAllowMinDifficultyBlocksAfterHeight.get() + i;
         blocks[i].nTime = 1269211443 + i * params.nPowTargetSpacing;
         blocks[i].nBits = 0x1e7fffff; /* target 0x007fffff000... */
         blocks[i].nChainWork = i ? blocks[i - 1].nChainWork + GetBlockProof(blocks[i - 1]) : arith_uint256(0);

From 089ec92e7b1f40e24601acbf60bfaee9c042f8a2 Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@z.cash>
Date: Fri, 5 Oct 2018 11:32:22 +0100
Subject: [PATCH 12/14] Fix pyflakes warnings

---
 qa/rpc-tests/wallet_shieldcoinbase_sapling.py | 3 +++
 qa/rpc-tests/wallet_shieldcoinbase_sprout.py  | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/qa/rpc-tests/wallet_shieldcoinbase_sapling.py b/qa/rpc-tests/wallet_shieldcoinbase_sapling.py
index 6aac42136..d5d130329 100755
--- a/qa/rpc-tests/wallet_shieldcoinbase_sapling.py
+++ b/qa/rpc-tests/wallet_shieldcoinbase_sapling.py
@@ -2,6 +2,9 @@
 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'))
 
diff --git a/qa/rpc-tests/wallet_shieldcoinbase_sprout.py b/qa/rpc-tests/wallet_shieldcoinbase_sprout.py
index 77ca7487b..619ae7caf 100755
--- a/qa/rpc-tests/wallet_shieldcoinbase_sprout.py
+++ b/qa/rpc-tests/wallet_shieldcoinbase_sprout.py
@@ -2,6 +2,9 @@
 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'))
 

From c94f4477e8551314b386c36ab92bcbde8bbb5641 Mon Sep 17 00:00:00 2001
From: Simon <simon@bitcartel.com>
Date: Fri, 5 Oct 2018 10:48:03 -0700
Subject: [PATCH 13/14] For #3546. Shielded tx with missing inputs are not
 treated as orphans.

---
 src/main.cpp | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/main.cpp b/src/main.cpp
index 2921f657d..40ab31cdd 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -5483,8 +5483,11 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
             BOOST_FOREACH(uint256 hash, vEraseQueue)
                 EraseOrphanTx(hash);
         }
-        // TODO: currently, prohibit joinsplits from entering mapOrphans
-        else if (fMissingInputs && tx.vjoinsplit.size() == 0)
+        // TODO: currently, prohibit joinsplits and shielded spends/outputs from entering mapOrphans
+        else if (fMissingInputs &&
+                 tx.vjoinsplit.empty() &&
+                 tx.vShieldedSpend.empty() &&
+                 tx.vShieldedOutput.empty())
         {
             AddOrphanTx(tx, pfrom->GetId());
 

From 4dcc48b8cb63d0a88bde9d32abeaf1cc6fe366d1 Mon Sep 17 00:00:00 2001
From: Simon <simon@bitcartel.com>
Date: Fri, 5 Oct 2018 11:16:20 -0700
Subject: [PATCH 14/14] For #3546. Improve estimated tx size for Sapling
 outputs.

---
 src/wallet/rpcwallet.cpp | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index 84892543a..b85ab8f89 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -3786,14 +3786,18 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
     // Depending on the input notes, the actual tx size may turn out to be larger and perhaps invalid.
     size_t txsize = 0;
     for (int i = 0; i < zaddrRecipients.size(); i++) {
-        // TODO Check whether the recipient is a Sprout or Sapling address
-        JSDescription jsdesc;
-
-        if (mtx.fOverwintered && (mtx.nVersion >= SAPLING_TX_VERSION)) {
-            jsdesc.proof = GrothProof();
+        auto address = std::get<0>(zaddrRecipients[i]);
+        auto res = DecodePaymentAddress(address);
+        bool toSapling = boost::get<libzcash::SaplingPaymentAddress>(&res) != nullptr;
+        if (toSapling) {
+            mtx.vShieldedOutput.push_back(OutputDescription());
+        } else {
+            JSDescription jsdesc;
+            if (mtx.fOverwintered && (mtx.nVersion >= SAPLING_TX_VERSION)) {
+                jsdesc.proof = GrothProof();
+            }
+            mtx.vjoinsplit.push_back(jsdesc);
         }
-
-        mtx.vjoinsplit.push_back(jsdesc);
     }
     CTransaction tx(mtx);
     txsize += GetSerializeSize(tx, SER_NETWORK, tx.nVersion);