Sapling support for z_shieldcoinbase and more

qa/pull-tester/rpc-tests.sh

@@ -19,7 +19,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_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()

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.
+        def verify_locking(first, second, limit):
-        # We don't verify shieldingValue as utxos are not selected in any specific order, so value can change on each test run.
+            result = self.nodes[0].z_shieldcoinbase(mytaddr, myzaddr, 0, limit)
-        # We set an unrealistically high limit parameter of 99999, to verify that max tx size will constrain the number of utxos.
+            assert_equal(result["shieldingUTXOs"], Decimal(first))
-        result = self.nodes[0].z_shieldcoinbase(mytaddr, myzaddr, 0, 99999)
+            assert_equal(result["remainingUTXOs"], Decimal(second))
-        assert_equal(result["shieldingUTXOs"], Decimal('662'))
+            remainingValue = result["remainingValue"]
-        assert_equal(result["remainingUTXOs"], Decimal('138'))
+            opid1 = result['opid']
-        remainingValue = result["remainingValue"]
-        opid1 = result['opid']
 
-        # Verify that utxos are locked (not available for selection) by queuing up another shielding operation
+            # 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)
+            result = self.nodes[0].z_shieldcoinbase(mytaddr, myzaddr, 0, 0)
-        assert_equal(result["shieldingValue"], Decimal(remainingValue))
+            assert_equal(result["shieldingValue"], Decimal(remainingValue))
-        assert_equal(result["shieldingUTXOs"], Decimal('138'))
+            assert_equal(result["shieldingUTXOs"], Decimal(second))
-        assert_equal(result["remainingValue"], Decimal('0'))
+            assert_equal(result["remainingValue"], Decimal('0'))
-        assert_equal(result["remainingUTXOs"], Decimal('0'))
+            assert_equal(result["remainingUTXOs"], Decimal('0'))
-        opid2 = result['opid']
+            opid2 = result['opid']
 
-        # wait for both aysnc operations to complete
+            # wait for both aysnc operations to complete
-        wait_and_assert_operationid_status(self.nodes[0], opid1)
+            wait_and_assert_operationid_status(self.nodes[0], opid1)
-        wait_and_assert_operationid_status(self.nodes[0], opid2)
+            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
+        if self.addr_type == 'sprout':
-        # This option is used when the limit parameter is set to 0.
+            # Verify maximum number of utxos which node 2 can shield is limited by option -mempooltxinputlimit
-        mytaddr = self.nodes[2].getnewaddress()
+            # This option is used when the limit parameter is set to 0.
-        result = self.nodes[2].z_shieldcoinbase(mytaddr, myzaddr, Decimal('0.0001'), 0)
+            mytaddr = self.nodes[2].getnewaddress()
-        assert_equal(result["shieldingUTXOs"], Decimal('7'))
+            result = self.nodes[2].z_shieldcoinbase(mytaddr, myzaddr, Decimal('0.0001'), 0)
-        assert_equal(result["remainingUTXOs"], Decimal('13'))
+            assert_equal(result["shieldingUTXOs"], Decimal('7'))
-        wait_and_assert_operationid_status(self.nodes[2], result['opid'])
+            assert_equal(result["remainingUTXOs"], Decimal('13'))
-        self.sync_all()
+            wait_and_assert_operationid_status(self.nodes[2], result['opid'])
-        self.nodes[1].generate(1)
+            self.sync_all()
-        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/chainparams.cpp

@@ -107,7 +107,7 @@ public:
         consensus.nPowMaxAdjustDown = 32; // 32% adjustment down
         consensus.nPowMaxAdjustUp = 16; // 16% adjustment up
         consensus.nPowTargetSpacing = 1 * 60;
-        consensus.fPowAllowMinDifficultyBlocks = true; //false;
+        consensus.nPowAllowMinDifficultyBlocksAfterHeight = boost::none;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nProtocolVersion = 170002;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nActivationHeight =
             Consensus::NetworkUpgrade::ALWAYS_ACTIVE;
@@ -488,6 +488,7 @@ public:
         consensus.nPowMaxAdjustDown = 32; // 32% adjustment down
         consensus.nPowMaxAdjustUp = 16; // 16% adjustment up
         consensus.nPowTargetSpacing = 2.5 * 60;
+        consensus.nPowAllowMinDifficultyBlocksAfterHeight = 299187;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nProtocolVersion = 170002;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nActivationHeight =
             Consensus::NetworkUpgrade::ALWAYS_ACTIVE;
@@ -502,7 +503,6 @@ public:
         // The best chain should have at least this much work.
         consensus.nMinimumChainWork = uint256S("0x00000000000000000000000000000000000000000000000000000001d0c4d9cd");
 
-        consensus.fPowAllowMinDifficultyBlocks = true;
         pchMessageStart[0] = 0x5A;
         pchMessageStart[1] = 0x1F;
         pchMessageStart[2] = 0x7E;
@@ -589,6 +589,7 @@ public:
         consensus.nPowMaxAdjustDown = 0; // Turn off adjustment down
         consensus.nPowMaxAdjustUp = 0; // Turn off adjustment up
         consensus.nPowTargetSpacing = 2.5 * 60;
+        consensus.nPowAllowMinDifficultyBlocksAfterHeight = 0;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nProtocolVersion = 170002;
         consensus.vUpgrades[Consensus::BASE_SPROUT].nActivationHeight =
             Consensus::NetworkUpgrade::ALWAYS_ACTIVE;

src/consensus/params.h

@@ -8,6 +8,8 @@
 
 #include "uint256.h"
 
+#include <boost/optional.hpp>
+
 namespace Consensus {
 
 /**
@@ -88,12 +90,12 @@ struct Params {
     int nMajorityEnforceBlockUpgrade;
     int nMajorityRejectBlockOutdated;
     int nMajorityWindow;
-    int fPowAllowMinDifficultyBlocks;
     NetworkUpgrade vUpgrades[MAX_NETWORK_UPGRADES];
 
     /** Proof of work parameters */
     uint256 powLimit;
     uint256 powAlternate;
+    boost::optional<uint32_t> nPowAllowMinDifficultyBlocksAfterHeight;
     int64_t nPowAveragingWindow;
     int64_t nPowMaxAdjustDown;
     int64_t nPowMaxAdjustUp;

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 = 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);
+    }
+
+    // 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());
+}

src/main.cpp

@@ -6796,8 +6796,11 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
             BOOST_FOREACH(uint256 hash, vEraseQueue)
             EraseOrphanTx(hash);
         }
-        // TODO: currently, prohibit joinsplits from entering mapOrphans
+        // TODO: currently, prohibit joinsplits and shielded spends/outputs from entering mapOrphans
-        else if (fMissingInputs && tx.vjoinsplit.size() == 0)
+        else if (fMissingInputs &&
+                 tx.vjoinsplit.empty() &&
+                 tx.vShieldedSpend.empty() &&
+                 tx.vShieldedOutput.empty())
         {
             AddOrphanTx(tx, pfrom->GetId());
             

src/miner.cpp

@@ -103,7 +103,12 @@ public:
 
 void UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev)
 {
-    pblock->nTime = 1 + std::max(pindexPrev->GetMedianTimePast()+1, GetAdjustedTime());
+    pblock->nTime = std::max(pindexPrev->GetMedianTimePast()+1, GetAdjustedTime());
+
+    // Updating time can change work required on testnet:
+    if (consensusParams.nPowAllowMinDifficultyBlocksAfterHeight != boost::none) {
+        pblock->nBits = GetNextWorkRequired(pindexPrev, pblock, consensusParams);
+    }
 }
 
 #include "komodo_defs.h"
@@ -1285,6 +1290,7 @@ void static BitcoinMiner_noeq()
                 printf("%lu mega hashes complete - working\n", (ASSETCHAINS_NONCEMASK[ASSETCHAINS_ALGO] + 1) / 1048576);
 #endif
                 break;
+
             }
         }
     }

src/pow.cpp

@@ -38,6 +38,20 @@ unsigned int GetNextWorkRequired(const CBlockIndex* pindexLast, const CBlockHead
     if (pindexLast == NULL )
         return nProofOfWorkLimit;
 
+    //{
+        // Comparing to pindexLast->nHeight with >= because this function
+        // returns the work required for the block after pindexLast.
+        //if (params.nPowAllowMinDifficultyBlocksAfterHeight != boost::none &&
+        //    pindexLast->nHeight >= params.nPowAllowMinDifficultyBlocksAfterHeight.get())
+        //{
+            // Special difficulty rule for testnet:
+            // If the new block's timestamp is more than 6 * 2.5 minutes
+            // then allow mining of a min-difficulty block.
+        //    if (pblock && pblock->GetBlockTime() > pindexLast->GetBlockTime() + params.nPowTargetSpacing * 6)
+        //        return nProofOfWorkLimit;
+        //}
+    //}
+
     // Find the first block in the averaging interval
     const CBlockIndex* pindexFirst = pindexLast;
     arith_uint256 bnTot {0};

src/test/rpc_wallet_tests.cpp

@@ -1518,13 +1518,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"));
     }
@@ -1532,7 +1532,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"));
     }
@@ -1565,7 +1565,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();
@@ -1575,7 +1575,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();
@@ -1586,7 +1586,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

@@ -56,12 +56,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
 
@@ -76,8 +77,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");
@@ -182,7 +181,6 @@ void AsyncRPCOperation_shieldcoinbase::main() {
     // !!! Payment disclosure END
 }
 
-
 bool AsyncRPCOperation_shieldcoinbase::main_impl() {
 
     CAmount minersFee = fee_;
@@ -218,32 +216,36 @@ 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_);
+    CMutableTransaction rawTx(m_op->tx_);
-    for (ShieldCoinbaseUTXO & t : inputs_) {
+    for (ShieldCoinbaseUTXO & t : m_op->inputs_) {
         CTxIn in(COutPoint(t.txid, t.vout));
         if (t.amount >= ASSETCHAINS_TIMELOCKGTE)
             in.nSequence = 0;
         rawTx.vin.push_back(in);
     }
-    tx_ = CTransaction(rawTx);
+    m_op->tx_ = CTransaction(rawTx);
 
     // Prepare raw transaction to handle JoinSplits
-    CMutableTransaction mtx(tx_);
+    CMutableTransaction mtx(m_op->tx_);
-    crypto_sign_keypair(joinSplitPubKey_.begin(), joinSplitPrivKey_);
+    crypto_sign_keypair(m_op->joinSplitPubKey_.begin(), m_op->joinSplitPrivKey_);
-    mtx.joinSplitPubKey = joinSplitPubKey_;
+    mtx.joinSplitPubKey = m_op->joinSplitPubKey_;
-    tx_ = CTransaction(mtx);
+    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;
 }
 
@@ -251,6 +253,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

@@ -4133,14 +4133,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
+        auto address = std::get<0>(zaddrRecipients[i]);
-        JSDescription jsdesc;
+        auto res = DecodePaymentAddress(address);
-
+        bool toSapling = boost::get<libzcash::SaplingPaymentAddress>(&res) != nullptr;
-        if (mtx.fOverwintered && (mtx.nVersion >= SAPLING_TX_VERSION)) {
+        if (toSapling) {
-            jsdesc.proof = GrothProof();
+            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);
@@ -4360,6 +4364,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) {
@@ -4370,7 +4375,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;
             }
@@ -4409,9 +4414,14 @@ 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
     int blockHeight = chainActive.LastTip()->GetHeight();
     nextBlockHeight = blockHeight + 1;
+    // (used if no Sapling addresses are involved)
     CMutableTransaction contextualTx = CreateNewContextualCMutableTransaction(
         Params().GetConsensus(), nextBlockHeight);
     contextualTx.nLockTime = blockHeight;
@@ -4421,7 +4431,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();
 

src/wallet/wallet.cpp

@@ -2660,7 +2660,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) {