Merge pull request #25 from miketout/dev-zcash

Dev zcash

qa/pull-tester/rpc-tests.sh

@@ -20,6 +20,7 @@ testScripts=(
     'wallet_import_export.py'
     'wallet_protectcoinbase.py'
     'wallet_shieldcoinbase.py'
+    'wallet_listreceived.py'
     'wallet_mergetoaddress.py'
     'wallet.py'
     'wallet_overwintertx.py'
@@ -27,6 +28,7 @@ testScripts=(
     'wallet_1941.py'
     'wallet_addresses.py'
     'wallet_sapling.py'
+    'wallet_listnotes.py'
     'listtransactions.py'
     'mempool_resurrect_test.py'
     'txn_doublespend.py'
@@ -67,6 +69,7 @@ testScripts=(
     'rewind_index.py'
     'p2p_txexpiry_dos.py'
     'p2p_node_bloom.py'
+    'regtest_signrawtransaction.py'
 );
 testScriptsExt=(
     'getblocktemplate_longpoll.py'

qa/rpc-tests/regtest_signrawtransaction.py

@@ -0,0 +1,34 @@
+#!/usr/bin/env python2
+# Copyright (c) 2018 The Zcash developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import start_nodes, wait_and_assert_operationid_status
+
+class RegtestSignrawtransactionTest (BitcoinTestFramework):
+
+    def setup_nodes(self):
+        return start_nodes(4, self.options.tmpdir, [[
+            "-nuparams=5ba81b19:200", # Overwinter
+            "-nuparams=76b809bb:204", # Sapling
+        ]] * 4)
+
+    def run_test(self):
+        self.nodes[0].generate(1)
+        self.sync_all()
+        taddr = self.nodes[1].getnewaddress()
+        zaddr1 = self.nodes[1].z_getnewaddress('sprout')
+
+        self.nodes[0].sendtoaddress(taddr, 2.0)
+        self.nodes[0].generate(1)
+        self.sync_all()
+
+        # Create and sign Overwinter transaction.
+        # If the incorrect consensus branch id is selected, there will be a signing error. 
+        opid = self.nodes[1].z_sendmany(taddr,
+            [{'address': zaddr1, 'amount': 1}])
+        wait_and_assert_operationid_status(self.nodes[1], opid)
+
+if __name__ == '__main__':
+    RegtestSignrawtransactionTest().main()

qa/rpc-tests/signrawtransaction_offline.py

@@ -2,6 +2,7 @@
 
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import assert_equal, assert_true, initialize_chain_clean, start_node
+from test_framework.authproxy import JSONRPCException
 
 class SignOfflineTest (BitcoinTestFramework):
     # Setup Methods
@@ -36,7 +37,17 @@ class SignOfflineTest (BitcoinTestFramework):
 
         create_hex = self.nodes[0].createrawtransaction(create_inputs, {taddr: 9.9999})
 
-        signed_tx = offline_node.signrawtransaction(create_hex, sign_inputs, privkeys)
+        # An offline regtest node does not rely on the approx release height of the software
+        # to determine the consensus rules to be used for signing.
+        try:
+            signed_tx = offline_node.signrawtransaction(create_hex, sign_inputs, privkeys)
+            self.nodes[0].sendrawtransaction(signed_tx['hex'])
+            assert(False)
+        except JSONRPCException:
+            pass
+
+        # Passing in the consensus branch id resolves the issue for offline regtest nodes.
+        signed_tx = offline_node.signrawtransaction(create_hex, sign_inputs, privkeys, "ALL", "5ba81b19")
 
         # If we return the transaction hash, then we have have not thrown an error (success)
         online_tx_hash = self.nodes[0].sendrawtransaction(signed_tx['hex'])

qa/rpc-tests/wallet_listnotes.py

@@ -0,0 +1,168 @@
+#!/usr/bin/env python2
+# Copyright (c) 2018 The Zcash developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import assert_equal, start_nodes, wait_and_assert_operationid_status
+
+from decimal import Decimal
+
+# Test wallet z_listunspent behaviour across network upgrades
+class WalletListNotes(BitcoinTestFramework):
+
+    def setup_nodes(self):
+        return start_nodes(4, self.options.tmpdir, [[
+            '-nuparams=5ba81b19:202', # Overwinter
+            '-nuparams=76b809bb:204', # Sapling
+        ]] * 4)
+
+    def run_test(self):
+        # Current height = 200 -> Sprout
+        assert_equal(200, self.nodes[0].getblockcount())
+        sproutzaddr = self.nodes[0].z_getnewaddress('sprout')
+
+        # test that we can create a sapling zaddr before sapling activates
+        saplingzaddr = self.nodes[0].z_getnewaddress('sapling')
+
+        # we've got lots of coinbase (taddr) but no shielded funds yet
+        assert_equal(0, Decimal(self.nodes[0].z_gettotalbalance()['private']))
+        
+        # Set current height to 201 -> Sprout
+        self.nodes[0].generate(1)
+        self.sync_all()
+        assert_equal(201, self.nodes[0].getblockcount())
+
+        mining_addr = self.nodes[0].listunspent()[0]['address']
+
+        # Shield coinbase funds (must be a multiple of 10, no change allowed pre-sapling)
+        receive_amount_10 = Decimal('10.0') - Decimal('0.0001')
+        recipients = [{"address":sproutzaddr, "amount":receive_amount_10}]
+        myopid = self.nodes[0].z_sendmany(mining_addr, recipients)
+        txid_1 = wait_and_assert_operationid_status(self.nodes[0], myopid)
+        self.sync_all()
+        
+        # No funds (with (default) one or more confirmations) in sproutzaddr yet
+        assert_equal(0, len(self.nodes[0].z_listunspent()))
+        assert_equal(0, len(self.nodes[0].z_listunspent(1)))
+        
+        # no private balance because no confirmations yet
+        assert_equal(0, Decimal(self.nodes[0].z_gettotalbalance()['private']))
+        
+        # list private unspent, this time allowing 0 confirmations
+        unspent_cb = self.nodes[0].z_listunspent(0)
+        assert_equal(1, len(unspent_cb))
+        assert_equal(False,             unspent_cb[0]['change'])
+        assert_equal(txid_1,            unspent_cb[0]['txid'])
+        assert_equal(True,              unspent_cb[0]['spendable'])
+        assert_equal(sproutzaddr,       unspent_cb[0]['address'])
+        assert_equal(receive_amount_10, unspent_cb[0]['amount'])
+
+        # list unspent, filtering by address, should produce same result
+        unspent_cb_filter = self.nodes[0].z_listunspent(0, 9999, False, [sproutzaddr])
+        assert_equal(unspent_cb, unspent_cb_filter)
+        
+        # Generate a block to confirm shield coinbase tx
+        self.nodes[0].generate(1)
+        self.sync_all()
+        
+        # Current height = 202 -> Overwinter. Default address type remains Sprout
+        assert_equal(202, self.nodes[0].getblockcount())
+
+        # Send 1.0 (actually 0.9999) from sproutzaddr to a new zaddr
+        sproutzaddr2 = self.nodes[0].z_getnewaddress()
+        receive_amount_1 = Decimal('1.0') - Decimal('0.0001')
+        change_amount_9 = receive_amount_10 - Decimal('1.0')
+        assert_equal('sprout', self.nodes[0].z_validateaddress(sproutzaddr2)['type'])
+        recipients = [{"address": sproutzaddr2, "amount":receive_amount_1}]
+        myopid = self.nodes[0].z_sendmany(sproutzaddr, recipients)
+        txid_2 = wait_and_assert_operationid_status(self.nodes[0], myopid)
+        self.sync_all()
+        
+        # list unspent, allowing 0conf txs
+        unspent_tx = self.nodes[0].z_listunspent(0)
+        assert_equal(len(unspent_tx), 2)
+        # sort low-to-high by amount (order of returned entries is not guaranteed)
+        unspent_tx = sorted(unspent_tx, key=lambda k: k['amount'])
+        assert_equal(False,             unspent_tx[0]['change'])
+        assert_equal(txid_2,            unspent_tx[0]['txid'])
+        assert_equal(True,              unspent_tx[0]['spendable'])
+        assert_equal(sproutzaddr2,      unspent_tx[0]['address'])
+        assert_equal(receive_amount_1,  unspent_tx[0]['amount'])
+
+        assert_equal(True,              unspent_tx[1]['change'])
+        assert_equal(txid_2,            unspent_tx[1]['txid'])
+        assert_equal(True,              unspent_tx[1]['spendable'])
+        assert_equal(sproutzaddr,       unspent_tx[1]['address'])
+        assert_equal(change_amount_9,   unspent_tx[1]['amount'])
+
+        unspent_tx_filter = self.nodes[0].z_listunspent(0, 9999, False, [sproutzaddr2])
+        assert_equal(1, len(unspent_tx_filter))
+        assert_equal(unspent_tx[0], unspent_tx_filter[0])
+
+        unspent_tx_filter = self.nodes[0].z_listunspent(0, 9999, False, [sproutzaddr])
+        assert_equal(1, len(unspent_tx_filter))
+        assert_equal(unspent_tx[1], unspent_tx_filter[0])
+        
+        # Set current height to 204 -> Sapling
+        self.nodes[0].generate(2)
+        self.sync_all()
+        assert_equal(204, self.nodes[0].getblockcount())
+        
+        # No funds in saplingzaddr yet
+        assert_equal(0, len(self.nodes[0].z_listunspent(0, 9999, False, [saplingzaddr])))
+
+        # Send 0.9999 to our sapling zaddr
+        # (sending from a sprout zaddr to a sapling zaddr is disallowed,
+        # so send from coin base)
+        receive_amount_2 = Decimal('2.0') - Decimal('0.0001')
+        recipients = [{"address": saplingzaddr, "amount":receive_amount_2}]
+        myopid = self.nodes[0].z_sendmany(mining_addr, recipients)
+        txid_3 = wait_and_assert_operationid_status(self.nodes[0], myopid)
+        self.sync_all()
+        unspent_tx = self.nodes[0].z_listunspent(0)
+        assert_equal(3, len(unspent_tx))
+
+        # low-to-high in amount
+        unspent_tx = sorted(unspent_tx, key=lambda k: k['amount'])
+
+        assert_equal(False,             unspent_tx[0]['change'])
+        assert_equal(txid_2,            unspent_tx[0]['txid'])
+        assert_equal(True,              unspent_tx[0]['spendable'])
+        assert_equal(sproutzaddr2,      unspent_tx[0]['address'])
+        assert_equal(receive_amount_1,  unspent_tx[0]['amount'])
+
+        assert_equal(False,             unspent_tx[1]['change'])
+        assert_equal(txid_3,            unspent_tx[1]['txid'])
+        assert_equal(True,              unspent_tx[1]['spendable'])
+        assert_equal(saplingzaddr,      unspent_tx[1]['address'])
+        assert_equal(receive_amount_2,  unspent_tx[1]['amount'])
+
+        assert_equal(True,              unspent_tx[2]['change'])
+        assert_equal(txid_2,            unspent_tx[2]['txid'])
+        assert_equal(True,              unspent_tx[2]['spendable'])
+        assert_equal(sproutzaddr,       unspent_tx[2]['address'])
+        assert_equal(change_amount_9,   unspent_tx[2]['amount'])
+
+        unspent_tx_filter = self.nodes[0].z_listunspent(0, 9999, False, [saplingzaddr])
+        assert_equal(1, len(unspent_tx_filter))
+        assert_equal(unspent_tx[1], unspent_tx_filter[0])
+
+        # test that pre- and post-sapling can be filtered in a single call
+        unspent_tx_filter = self.nodes[0].z_listunspent(0, 9999, False,
+            [sproutzaddr, saplingzaddr])
+        assert_equal(2, len(unspent_tx_filter))
+        unspent_tx_filter = sorted(unspent_tx_filter, key=lambda k: k['amount'])
+        assert_equal(unspent_tx[1], unspent_tx_filter[0])
+        assert_equal(unspent_tx[2], unspent_tx_filter[1])
+
+        # so far, this node has no watchonly addresses, so results are the same
+        unspent_tx_watchonly = self.nodes[0].z_listunspent(0, 9999, True)
+        unspent_tx_watchonly = sorted(unspent_tx_watchonly, key=lambda k: k['amount'])
+        assert_equal(unspent_tx, unspent_tx_watchonly)
+
+        # TODO: use z_exportviewingkey, z_importviewingkey to test includeWatchonly
+        # but this requires Sapling support for those RPCs
+
+if __name__ == '__main__':
+    WalletListNotes().main()

qa/rpc-tests/wallet_listreceived.py

@@ -0,0 +1,101 @@
+#!/usr/bin/env python2
+# Copyright (c) 2018 The Zcash developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import assert_equal, assert_true, assert_false
+from test_framework.util import start_nodes, wait_and_assert_operationid_status
+from decimal import Decimal
+
+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')
+
+class ListReceivedTest (BitcoinTestFramework):
+
+    def setup_nodes(self):
+        return start_nodes(4, self.options.tmpdir, [[
+            "-nuparams=5ba81b19:201", # Overwinter
+            "-nuparams=76b809bb:204", # Sapling
+        ]] * 4)
+
+    def generate_and_sync(self, new_height):
+        self.nodes[0].generate(1)
+        self.sync_all()
+        assert_equal(new_height, self.nodes[0].getblockcount())
+
+    def run_test_release(self, release, expected_memo, height):
+        self.generate_and_sync(height+1)
+        taddr = self.nodes[1].getnewaddress()
+        zaddr1 = self.nodes[1].z_getnewaddress(release)
+
+        self.nodes[0].sendtoaddress(taddr, 2.0)
+        self.generate_and_sync(height+2)
+
+        # Send 1 ZEC to zaddr1
+        opid = self.nodes[1].z_sendmany(taddr,
+            [{'address': zaddr1, 'amount': 1, 'memo': my_memo}])
+        txid = wait_and_assert_operationid_status(self.nodes[1], opid)
+        self.sync_all()
+        r = self.nodes[1].z_listreceivedbyaddress(zaddr1)
+        assert_equal(0, len(r), "Should have received no confirmed note")
+
+        # No confirmation required, one note should be present
+        r = self.nodes[1].z_listreceivedbyaddress(zaddr1, 0)
+        assert_equal(1, len(r), "Should have received one (unconfirmed) note")
+        assert_equal(txid, r[0]['txid'])
+        assert_equal(1, r[0]['amount'])
+        assert_false(r[0]['change'], "Note should not be change")
+        assert_equal(my_memo, r[0]['memo'])
+
+        # Confirm transaction (1 ZEC from taddr to zaddr1)
+        self.generate_and_sync(height+3)
+
+        # Require one confirmation, note should be present
+        assert_equal(r, self.nodes[1].z_listreceivedbyaddress(zaddr1))
+
+        # Generate some change by sending part of zaddr1 to zaddr2
+        zaddr2 = self.nodes[1].z_getnewaddress(release)
+        opid = self.nodes[1].z_sendmany(zaddr1,
+            [{'address': zaddr2, 'amount': 0.6, 'memo': my_memo}])
+        txid = wait_and_assert_operationid_status(self.nodes[1], opid)
+        self.sync_all()
+        self.generate_and_sync(height+4)
+
+        # zaddr1 should have a note with change
+        r = self.nodes[1].z_listreceivedbyaddress(zaddr1, 0)
+        r = sorted(r, key = lambda received: received['amount'])
+        assert_equal(2, len(r), "zaddr1 Should have received 2 notes")
+
+        assert_equal(txid, r[0]['txid'])
+        assert_equal(Decimal('0.4')-fee, r[0]['amount'])
+        assert_true(r[0]['change'], "Note valued at (0.4-fee) should be change")
+        assert_equal(expected_memo, r[0]['memo'])
+
+        # The old note still exists (it's immutable), even though it is spent
+        assert_equal(Decimal('1.0'), r[1]['amount'])
+        assert_false(r[1]['change'], "Note valued at 1.0 should not be change")
+        assert_equal(expected_memo, r[0]['memo'])
+
+        # zaddr2 should not have change
+        r = self.nodes[1].z_listreceivedbyaddress(zaddr2, 0)
+        r = sorted(r, key = lambda received: received['amount'])
+        assert_equal(1, len(r), "zaddr2 Should have received 1 notes")
+        assert_equal(txid, r[0]['txid'])
+        assert_equal(Decimal('0.6'), r[0]['amount'])
+        assert_false(r[0]['change'], "Note valued at 0.6 should not be change")
+        assert_equal(my_memo, r[0]['memo'])
+
+    def run_test(self):
+        self.run_test_release('sprout', no_memo, 200)
+        self.run_test_release('sapling', zero_memo, 204)
+
+if __name__ == '__main__':
+    ListReceivedTest().main()

qa/rpc-tests/wallet_sapling.py

@@ -53,9 +53,14 @@ class WalletSaplingTest(BitcoinTestFramework):
         recipients = []
         recipients.append({"address": saplingAddr0, "amount": Decimal('20')})
         myopid = self.nodes[0].z_sendmany(taddr0, recipients, 1, 0)
-        wait_and_assert_operationid_status(self.nodes[0], myopid)
+        mytxid = wait_and_assert_operationid_status(self.nodes[0], myopid)
 
         self.sync_all()
+
+        # Verify priority of tx is MAX_PRIORITY, defined as 1E+16 (10000000000000000)
+        mempool = self.nodes[0].getrawmempool(True)
+        assert(Decimal(mempool[mytxid]['startingpriority']) == Decimal('1E+16'))
+
         self.nodes[2].generate(1)
         self.sync_all()
 
@@ -70,9 +75,14 @@ class WalletSaplingTest(BitcoinTestFramework):
         recipients = []
         recipients.append({"address": saplingAddr1, "amount": Decimal('15')})
         myopid = self.nodes[0].z_sendmany(saplingAddr0, recipients, 1, 0)
-        wait_and_assert_operationid_status(self.nodes[0], myopid)
+        mytxid = wait_and_assert_operationid_status(self.nodes[0], myopid)
 
         self.sync_all()
+
+        # Verify priority of tx is MAX_PRIORITY, defined as 1E+16 (10000000000000000)
+        mempool = self.nodes[0].getrawmempool(True)
+        assert(Decimal(mempool[mytxid]['startingpriority']) == Decimal('1E+16'))
+
         self.nodes[2].generate(1)
         self.sync_all()
 
@@ -92,6 +102,11 @@ class WalletSaplingTest(BitcoinTestFramework):
         mytxid = wait_and_assert_operationid_status(self.nodes[1], myopid)
 
         self.sync_all()
+
+        # Verify priority of tx is MAX_PRIORITY, defined as 1E+16 (10000000000000000)
+        mempool = self.nodes[1].getrawmempool(True)
+        assert(Decimal(mempool[mytxid]['startingpriority']) == Decimal('1E+16'))
+
         self.nodes[2].generate(1)
         self.sync_all()
 

src/cc/CoinbaseGuard.cpp

@@ -18,11 +18,16 @@
 
 extern int32_t VERUS_MIN_STAKEAGE;
 
+bool IsData(opcodetype opcode)
+{
+    return (opcode >= 0 && opcode <= OP_PUSHDATA4) || (opcode >= OP_1 && opcode <= OP_16);
+}
+
 bool UnpackStakeOpRet(const CTransaction &stakeTx, std::vector<std::vector<unsigned char>> &vData)
 {
     bool isValid = stakeTx.vout[stakeTx.vout.size() - 1].scriptPubKey.GetOpretData(vData);
 
-    if (isValid && vData.size() == 1 && vData[0][0] > 0 && vData[0][0] < 4)
+    if (isValid && vData.size() == 1)
     {
         CScript data = CScript(vData[0].begin(), vData[0].end());
         vData.clear();
@@ -34,9 +39,14 @@ bool UnpackStakeOpRet(const CTransaction &stakeTx, std::vector<std::vector<unsig
         bool moreData = true;
 
         for (bytesTotal = vch.size(); 
-             bytesTotal <= nMaxDatacarrierBytes && !(isValid = (pc == data.end())) && (moreData = data.GetOp(pc, op, vch)) && (op > 0 && op <= OP_PUSHDATA4); 
+             bytesTotal <= nMaxDatacarrierBytes && !(isValid = (pc == data.end())) && (moreData = data.GetOp(pc, op, vch)) && IsData(op); 
              bytesTotal += vch.size())
         {
+            if (op >= OP_1 && op <= OP_16)
+            {
+                vch.resize(1);
+                vch[0] = (op - OP_1) + 1;
+            }
             vData.push_back(vch);
         }
         
@@ -49,13 +59,13 @@ bool UnpackStakeOpRet(const CTransaction &stakeTx, std::vector<std::vector<unsig
     return false;
 }
 
-CStakeParams::CStakeParams(std::vector<std::vector<unsigned char>> vData)
+CStakeParams::CStakeParams(const std::vector<std::vector<unsigned char>> &vData)
 {
     // A stake OP_RETURN contains:
     // 1. source block height in little endian 32 bit
     // 2. target block height in little endian 32 bit
     // 3. 32 byte prev block hash
-    // 4. alternate 20 byte pubkey hash, 33 byte pubkey, or not present to use same as stake destination
+    // 4. 33 byte pubkey, or not present to use same as stake destination
 
     srcHeight = 0;
     blkHeight = 0;
@@ -65,13 +75,13 @@ CStakeParams::CStakeParams(std::vector<std::vector<unsigned char>> vData)
         vData[3].size() == sizeof(prevHash) &&
         (vData.size() == STAKE_MINPARAMS || (vData.size() == STAKE_MAXPARAMS && vData[4].size() == 33)))
     {
-        for (auto ch : vData[1])
+        for (int i = 0, size = vData[1].size(); i < size; i++)
         {
-            srcHeight = srcHeight << 8 | ch;
+            srcHeight = srcHeight | vData[1][i] << (8 * i);
         }
-        for (auto ch : vData[2])
+        for (int i = 0, size = vData[2].size(); i < size; i++)
         {
-            blkHeight = blkHeight << 8 | ch;
+            blkHeight = blkHeight | vData[2][i] << (8 * i);
         }
 
         prevHash = uint256(vData[3]);
@@ -101,6 +111,8 @@ bool GetStakeParams(const CTransaction &stakeTx, CStakeParams &stakeParams)
 {
     std::vector<std::vector<unsigned char>> vData = std::vector<std::vector<unsigned char>>();
 
+    //printf("opret stake script: %s\nvalue at scriptPubKey[0]: %x\n", stakeTx.vout[1].scriptPubKey.ToString().c_str(), stakeTx.vout[1].scriptPubKey[0]);
+
     if (stakeTx.vin.size() == 1 && 
         stakeTx.vout.size() == 2 && 
         stakeTx.vout[0].nValue > 0 && 
@@ -108,7 +120,7 @@ bool GetStakeParams(const CTransaction &stakeTx, CStakeParams &stakeParams)
         UnpackStakeOpRet(stakeTx, vData))
     {
         stakeParams = CStakeParams(vData);
-        return true;
+        return stakeParams.IsValid();
     }
     return false;
 }
@@ -151,8 +163,9 @@ bool ValidateStakeTransaction(const CTransaction &stakeTx, CStakeParams &stakePa
                         auto consensusBranchId = CurrentEpochBranchId(stakeParams.blkHeight, Params().GetConsensus());
                         if (VerifyScript(stakeTx.vin[0].scriptSig, 
                                          srcTx.vout[stakeTx.vin[0].prevout.n].scriptPubKey, 
-                                         STANDARD_SCRIPT_VERIFY_FLAGS + SCRIPT_VERIFY_SIGPUSHONLY, 
+                                         MANDATORY_SCRIPT_VERIFY_FLAGS, 
-                                         BaseSignatureChecker(), 
+                                         TransactionSignatureChecker(&stakeTx, 0, srcTx.vout[stakeTx.vin[0].prevout.n].nValue,
+                                                                     PrecomputedTransactionData(stakeTx)),
                                          consensusBranchId))
                         {
                             return true;

src/cc/CoinbaseGuard.h

@@ -18,7 +18,7 @@
 #include "streams.h"
 #include "script/script.h"
 
-#define DEFAULT_STAKE_TXFEE 10000
+#define DEFAULT_STAKE_TXFEE 0
 
 class CStakeParams
 {
@@ -33,7 +33,7 @@ class CStakeParams
     
         CStakeParams() : srcHeight(0), blkHeight(0), prevHash(), pk() {}
 
-        CStakeParams(std::vector<std::vector<unsigned char>> vData);
+        CStakeParams(const std::vector<std::vector<unsigned char>> &vData);
 
         CStakeParams(uint32_t _srcHeight, uint32_t _blkHeight, const uint256 &_prevHash, const CPubKey &_pk) :
             srcHeight(_srcHeight), blkHeight(_blkHeight), prevHash(_prevHash), pk(_pk) {}

src/coins.cpp

@@ -675,19 +675,17 @@ double CCoinsViewCache::GetPriority(const CTransaction &tx, int nHeight) const
 {
     if (tx.IsCoinBase())
         return 0.0;
-    // Joinsplits do not reveal any information about the value or age of a note, so we
+
+    // Shielded transfers do not reveal any information about the value or age of a note, so we
     // cannot apply the priority algorithm used for transparent utxos.  Instead, we just
-    // use the maximum priority whenever a transaction contains any JoinSplits.
+    // use the maximum priority for all (partially or fully) shielded transactions.
-    // (Note that coinbase transactions cannot contain JoinSplits.)
+    // (Note that coinbase transactions cannot contain JoinSplits, or Sapling shielded Spends or Outputs.)
-    // FIXME: this logic is partially duplicated between here and CreateNewBlock in miner.cpp.
+
-    
+    if (tx.vjoinsplit.size() > 0 || tx.vShieldedSpend.size() > 0 || tx.vShieldedOutput.size() > 0 || tx.IsCoinImport()) {
-    if (tx.vjoinsplit.size() > 0) {
-            return MAX_PRIORITY;
-        }
-    if (tx.IsCoinImport()) {
         return MAX_PRIORITY;
     }
 
+    // FIXME: this logic is partially duplicated between here and CreateNewBlock in miner.cpp.
     double dResult = 0.0;
     BOOST_FOREACH(const CTxIn& txin, tx.vin)
     {

src/gtest/test_keys.cpp

@@ -5,7 +5,7 @@
 
 #include <gtest/gtest.h>
 
-TEST(Keys, DISABLED_EncodeAndDecodeSapling)
+TEST(Keys, EncodeAndDecodeSapling)
 {
     SelectParams(CBaseChainParams::MAIN);
 

src/gtest/test_keystore.cpp

@@ -215,12 +215,6 @@ TEST(keystore_tests, StoreAndRetrieveSaplingSpendingKey) {
     EXPECT_FALSE(keyStore.HaveSaplingIncomingViewingKey(addr));
     EXPECT_FALSE(keyStore.GetSaplingIncomingViewingKey(addr, ivkOut));
 
-    // If we don't specify the default address, that mapping isn't created
-    keyStore.AddSaplingSpendingKey(sk);
-    EXPECT_TRUE(keyStore.HaveSaplingSpendingKey(fvk));
-    EXPECT_TRUE(keyStore.HaveSaplingFullViewingKey(ivk));
-    EXPECT_FALSE(keyStore.HaveSaplingIncomingViewingKey(addr));
-
     // When we specify the default address, we get the full mapping
     keyStore.AddSaplingSpendingKey(sk, addr);
     EXPECT_TRUE(keyStore.HaveSaplingSpendingKey(fvk));

src/key_io.cpp

@@ -294,8 +294,7 @@ libzcash::PaymentAddress DecodePaymentAddress(const std::string& str)
     }
     data.clear();
     auto bech = bech32::Decode(str);
-    bool allowSapling = true;
+    if (bech.first == Params().Bech32HRP(CChainParams::SAPLING_PAYMENT_ADDRESS) &&
-    if (allowSapling && bech.first == Params().Bech32HRP(CChainParams::SAPLING_PAYMENT_ADDRESS) &&
         bech.second.size() == ConvertedSaplingPaymentAddressSize) {
         // Bech32 decoding
         data.reserve((bech.second.size() * 5) / 8);
@@ -361,8 +360,7 @@ libzcash::SpendingKey DecodeSpendingKey(const std::string& str)
     }
     data.clear();
     auto bech = bech32::Decode(str);
-    bool allowSapling = true;
+    if (bech.first == Params().Bech32HRP(CChainParams::SAPLING_EXTENDED_SPEND_KEY) &&
-    if (allowSapling && bech.first == Params().Bech32HRP(CChainParams::SAPLING_EXTENDED_SPEND_KEY) &&
         bech.second.size() == ConvertedSaplingExtendedSpendingKeySize) {
         // Bech32 decoding
         data.reserve((bech.second.size() * 5) / 8);

src/keystore.cpp

@@ -125,13 +125,13 @@ bool CBasicKeyStore::AddSproutSpendingKey(const libzcash::SproutSpendingKey &sk)
 //! Sapling 
 bool CBasicKeyStore::AddSaplingSpendingKey(
     const libzcash::SaplingExtendedSpendingKey &sk,
-    const boost::optional<libzcash::SaplingPaymentAddress> &defaultAddr)
+    const libzcash::SaplingPaymentAddress &defaultAddr)
 {
     LOCK(cs_SpendingKeyStore);
     auto fvk = sk.expsk.full_viewing_key();
 
     // if SaplingFullViewingKey is not in SaplingFullViewingKeyMap, add it
-    if (!AddSaplingFullViewingKey(fvk, defaultAddr)){
+    if (!AddSaplingFullViewingKey(fvk, defaultAddr)) {
         return false;
     }
 
@@ -151,17 +151,27 @@ bool CBasicKeyStore::AddSproutViewingKey(const libzcash::SproutViewingKey &vk)
 
 bool CBasicKeyStore::AddSaplingFullViewingKey(
     const libzcash::SaplingFullViewingKey &fvk,
-    const boost::optional<libzcash::SaplingPaymentAddress> &defaultAddr)
+    const libzcash::SaplingPaymentAddress &defaultAddr)
 {
     LOCK(cs_SpendingKeyStore);
     auto ivk = fvk.in_viewing_key();
     mapSaplingFullViewingKeys[ivk] = fvk;
 
-    if (defaultAddr) {
+    return AddSaplingIncomingViewingKey(ivk, defaultAddr);
-        // Add defaultAddr -> SaplingIncomingViewing to SaplingIncomingViewingKeyMap
+}
-        mapSaplingIncomingViewingKeys[defaultAddr.get()] = ivk;
+
-    }
+// This function updates the wallet's internal address->ivk map. 
-    
+// If we add an address that is already in the map, the map will
+// remain unchanged as each address only has one ivk.
+bool CBasicKeyStore::AddSaplingIncomingViewingKey(
+    const libzcash::SaplingIncomingViewingKey &ivk,
+    const libzcash::SaplingPaymentAddress &addr)
+{
+    LOCK(cs_SpendingKeyStore);
+
+    // Add addr -> SaplingIncomingViewing to SaplingIncomingViewingKeyMap
+    mapSaplingIncomingViewingKeys[addr] = ivk;
+
     return true;
 }
 

src/keystore.h

@@ -67,7 +67,7 @@ public:
     //! Add a Sapling spending key to the store.
     virtual bool AddSaplingSpendingKey(
         const libzcash::SaplingExtendedSpendingKey &sk,
-        const boost::optional<libzcash::SaplingPaymentAddress> &defaultAddr = boost::none) =0;
+        const libzcash::SaplingPaymentAddress &defaultAddr) =0;
     
     //! Check whether a Sapling spending key corresponding to a given Sapling viewing key is present in the store.
     virtual bool HaveSaplingSpendingKey(const libzcash::SaplingFullViewingKey &fvk) const =0;
@@ -76,13 +76,16 @@ public:
     //! Support for Sapling full viewing keys
     virtual bool AddSaplingFullViewingKey(
         const libzcash::SaplingFullViewingKey &fvk,
-        const boost::optional<libzcash::SaplingPaymentAddress> &defaultAddr = boost::none) =0;
+        const libzcash::SaplingPaymentAddress &defaultAddr) =0;
     virtual bool HaveSaplingFullViewingKey(const libzcash::SaplingIncomingViewingKey &ivk) const =0;
     virtual bool GetSaplingFullViewingKey(
         const libzcash::SaplingIncomingViewingKey &ivk, 
         libzcash::SaplingFullViewingKey& fvkOut) const =0;
     
     //! Sapling incoming viewing keys 
+    virtual bool AddSaplingIncomingViewingKey(
+        const libzcash::SaplingIncomingViewingKey &ivk,
+        const libzcash::SaplingPaymentAddress &addr) =0;
     virtual bool HaveSaplingIncomingViewingKey(const libzcash::SaplingPaymentAddress &addr) const =0;
     virtual bool GetSaplingIncomingViewingKey(
         const libzcash::SaplingPaymentAddress &addr, 
@@ -237,7 +240,7 @@ public:
     //! Sapling 
     bool AddSaplingSpendingKey(
         const libzcash::SaplingExtendedSpendingKey &sk,
-        const boost::optional<libzcash::SaplingPaymentAddress> &defaultAddr = boost::none);
+        const libzcash::SaplingPaymentAddress &defaultAddr);
     bool HaveSaplingSpendingKey(const libzcash::SaplingFullViewingKey &fvk) const
     {
         bool result;
@@ -264,12 +267,15 @@ public:
 
     virtual bool AddSaplingFullViewingKey(
         const libzcash::SaplingFullViewingKey &fvk,
-        const boost::optional<libzcash::SaplingPaymentAddress> &defaultAddr = boost::none);
+        const libzcash::SaplingPaymentAddress &defaultAddr);
     virtual bool HaveSaplingFullViewingKey(const libzcash::SaplingIncomingViewingKey &ivk) const;
     virtual bool GetSaplingFullViewingKey(
         const libzcash::SaplingIncomingViewingKey &ivk, 
         libzcash::SaplingFullViewingKey& fvkOut) const;
     
+    virtual bool AddSaplingIncomingViewingKey(
+        const libzcash::SaplingIncomingViewingKey &ivk,
+        const libzcash::SaplingPaymentAddress &addr);
     virtual bool HaveSaplingIncomingViewingKey(const libzcash::SaplingPaymentAddress &addr) const;
     virtual bool GetSaplingIncomingViewingKey(
         const libzcash::SaplingPaymentAddress &addr, 

src/rpc/rawtransaction.cpp

@@ -1137,8 +1137,9 @@ UniValue signrawtransaction(const UniValue& params, bool fHelp)
     bool fHashSingle = ((nHashType & ~SIGHASH_ANYONECANPAY) == SIGHASH_SINGLE);
     // Use the approximate release height if it is greater so offline nodes 
     // have a better estimation of the current height and will be more likely to
-    // determine the correct consensus branch ID.
+    // determine the correct consensus branch ID.  Regtest mode ignores release height.
-    int chainHeight = std::max(chainActive.Height() + 1, APPROX_RELEASE_HEIGHT);
+    int chainHeight = chainActive.Height() + 1;
+
     // Grab the current consensus branch ID
     auto consensusBranchId = CurrentEpochBranchId(chainHeight, Params().GetConsensus());
 

src/script/script.cpp

@@ -264,11 +264,30 @@ bool CScript::GetBalancedData(const_iterator& pc, std::vector<std::vector<unsign
                 // this should never pop what it hasn't pushed (like a success code)
                 if (--netPushes < 0)
                     return false;
+            } 
+            else 
+            {
+                // push or fail
+                netPushes++;
+                if (opcode == OP_0)
+                {
+                    data.resize(1);
+                    data[0] = 0;
+                    vSolutions.push_back(data);
+                }
+                else if (opcode >= OP_1 && opcode <= OP_16)
+                {
+                    data.resize(1);
+                    data[0] = (opcode - OP_1) + 1;
+                    vSolutions.push_back(data);
+                }
+                else if (opcode > 0 && opcode <= OP_PUSHDATA4 && data.size() > 0)
+                {
+                    vSolutions.push_back(data);
+                }
+                else
+                    return false;
             }
-            if (opcode < 1 || opcode > OP_PUSHDATA4)
-                return false;
-            netPushes++;
-            vSolutions.push_back(data);
         }
         else
             return false;
@@ -276,14 +295,14 @@ bool CScript::GetBalancedData(const_iterator& pc, std::vector<std::vector<unsign
     return netPushes == 0;
 }
 
-// this returns true if either there is nothing left and pc points at the end, or 
+// this returns true if either there is nothing left and pc points at the end
-// all instructions from the pc to the end of the script are balanced pushes and pops
 // if there is data, it also returns all the values as byte vectors in a list of vectors
 bool CScript::GetOpretData(std::vector<std::vector<unsigned char>>& vData) const
 {
     vector<unsigned char> data;
     opcodetype opcode;
     const_iterator pc = begin();
+    std::vector<unsigned char> vch1 = std::vector<unsigned char>(1);
 
     vData.clear();
 
@@ -293,7 +312,20 @@ bool CScript::GetOpretData(std::vector<std::vector<unsigned char>>& vData) const
         {
             if (GetOp(pc, opcode, data))
             {
-                vData.push_back(data);
+                if (opcode == OP_0)
+                {
+                    vch1[0] = 0;
+                    vData.push_back(vch1);
+                }
+                else if (opcode >= OP_1 && opcode <= OP_16)
+                {
+                    vch1[0] = (opcode - OP_1) + 1;
+                    vData.push_back(data);
+                }
+                else
+                {
+                    vData.push_back(data);
+                }
             }
         }
         return vData.size() != 0;
@@ -316,7 +348,7 @@ bool CScript::IsPayToCryptoCondition(CScript *pCCSubScript, std::vector<std::vec
                     {
                         if (pCCSubScript)
                             *pCCSubScript = CScript(begin(),pc);
-                        return 1;
+                        return true;
                     }
                 }
     return false;

src/script/standard.cpp

@@ -35,7 +35,19 @@ COptCCParams::COptCCParams(std::vector<unsigned char> &vch)
             param.clear();
             if (inScr.GetOp(pc, opcode, param))
             {
-                if (opcode > 0 && opcode <= OP_PUSHDATA4 && param.size() > 0)
+                if (opcode == OP_0)
+                {
+                    param.resize(1);
+                    param[0] = 0;
+                    data.push_back(param);
+                }
+                else if (opcode >= OP_1 && opcode <= OP_16)
+                {
+                    param.resize(1);
+                    param[0] = (opcode - OP_1) + 1;
+                    data.push_back(param);
+                }
+                else if (opcode > 0 && opcode <= OP_PUSHDATA4 && param.size() > 0)
                 {
                     data.push_back(param);
                 }
@@ -55,8 +67,8 @@ COptCCParams::COptCCParams(std::vector<unsigned char> &vch)
             {
                 version = param[0];
                 evalCode = param[1];
-                n = param[2];
+                m = param[2];
-                m = param[3];
+                n = param[3];
                 if (version != VERSION || m != 1 || (n != 1 && n != 2) || data.size() <= n)
                 {
                     // we only support one version, and 1 of 1 or 1 of 2 now, so set invalid

src/test/rpc_wallet_tests.cpp

@@ -383,13 +383,12 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_z_validateaddress)
     BOOST_CHECK_NO_THROW(retValue = CallRPC("z_validateaddress zs1z7rejlpsa98s2rrrfkwmaxu53e4ue0ulcrw0h4x5g8jl04tak0d3mm47vdtahatqrlkngh9slya"));
     resultObj = retValue.get_obj();
     b = find_value(resultObj, "isvalid").get_bool();
-    // TODO: Revert when we re-enable Sapling addresses on mainnet
+    BOOST_CHECK_EQUAL(b, true);
+    BOOST_CHECK_EQUAL(find_value(resultObj, "type").get_str(), "sapling");
+    b = find_value(resultObj, "ismine").get_bool();
     BOOST_CHECK_EQUAL(b, false);
-    // BOOST_CHECK_EQUAL(find_value(resultObj, "type").get_str(), "sapling");
+    BOOST_CHECK_EQUAL(find_value(resultObj, "diversifier").get_str(), "1787997c30e94f050c634d");
-    // b = find_value(resultObj, "ismine").get_bool();
+    BOOST_CHECK_EQUAL(find_value(resultObj, "diversifiedtransmissionkey").get_str(), "34ed1f60f5db5763beee1ddbb37dd5f7e541d4d4fbdcc09fbfcc6b8e949bbe9d");
-    // BOOST_CHECK_EQUAL(b, false);
-    // BOOST_CHECK_EQUAL(find_value(resultObj, "diversifier").get_str(), "1787997c30e94f050c634d");
-    // BOOST_CHECK_EQUAL(find_value(resultObj, "diversifiedtransmissionkey").get_str(), "34ed1f60f5db5763beee1ddbb37dd5f7e541d4d4fbdcc09fbfcc6b8e949bbe9d");
 }
 
 /*
@@ -537,8 +536,6 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_z_importwallet)
  */
 BOOST_AUTO_TEST_CASE(rpc_wallet_z_importexport)
 {
-    SelectParams(CBaseChainParams::REGTEST);
-
     LOCK2(cs_main, pwalletMain->cs_wallet);
     UniValue retValue;
     int n1 = 1000; // number of times to import/export

src/wallet/asyncrpcoperation_mergetoaddress.cpp

@@ -82,8 +82,10 @@ AsyncRPCOperation_mergetoaddress::AsyncRPCOperation_mergetoaddress(
         auto address = DecodePaymentAddress(std::get<0>(recipient));
         if (IsValidPaymentAddress(address)) {
             isToZaddr_ = true;
-            // TODO: Add Sapling support. For now, ensure we can later convert freely.
+            // TODO: Add Sapling support. For now, return an error to the user.
-            assert(boost::get<libzcash::SproutPaymentAddress>(&address) != nullptr);
+            if (boost::get<libzcash::SproutPaymentAddress>(&address) == nullptr) {
+                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Currently, only Sprout zaddrs are supported");
+            }
             toPaymentAddress_ = address;
         } else {
             throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid recipient address");
@@ -328,8 +330,10 @@ bool AsyncRPCOperation_mergetoaddress::main_impl()
     // Copy zinputs to more flexible containers
     std::deque<MergeToAddressInputNote> zInputsDeque;
     for (auto o : noteInputs_) {
-        // TODO: Add Sapling support. For now, ensure we can later convert freely.
+        // TODO: Add Sapling support. For now, return an error to the user.
-        assert(boost::get<libzcash::SproutSpendingKey>(&std::get<3>(o)) != nullptr);
+        if (boost::get<libzcash::SproutSpendingKey>(&std::get<3>(o)) == nullptr) {
+            throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Currently, only Sprout zaddrs are supported");
+        }
         zInputsDeque.push_back(o);
     }
 

src/wallet/crypter.cpp

@@ -451,7 +451,7 @@ bool CCryptoKeyStore::AddSproutSpendingKey(const libzcash::SproutSpendingKey &sk
 
 bool CCryptoKeyStore::AddSaplingSpendingKey(
     const libzcash::SaplingExtendedSpendingKey &sk,
-    const boost::optional<libzcash::SaplingPaymentAddress> &defaultAddr)
+    const libzcash::SaplingPaymentAddress &defaultAddr)
 {
     {
         LOCK(cs_SpendingKeyStore);
@@ -498,7 +498,7 @@ bool CCryptoKeyStore::AddCryptedSproutSpendingKey(
 bool CCryptoKeyStore::AddCryptedSaplingSpendingKey(
     const libzcash::SaplingFullViewingKey &fvk,
     const std::vector<unsigned char> &vchCryptedSecret,
-    const boost::optional<libzcash::SaplingPaymentAddress> &defaultAddr)
+    const libzcash::SaplingPaymentAddress &defaultAddr)
 {
     {
         LOCK(cs_SpendingKeyStore);
@@ -507,7 +507,7 @@ bool CCryptoKeyStore::AddCryptedSaplingSpendingKey(
         }
 
         // if SaplingFullViewingKey is not in SaplingFullViewingKeyMap, add it
-        if (!AddSaplingFullViewingKey(fvk, defaultAddr)){
+        if (!AddSaplingFullViewingKey(fvk, defaultAddr)) {
             return false;
         }
 
@@ -616,7 +616,7 @@ bool CCryptoKeyStore::EncryptKeys(CKeyingMaterial& vMasterKeyIn)
             if (!EncryptSecret(vMasterKeyIn, vchSecret, fvk.GetFingerprint(), vchCryptedSecret)) {
                 return false;
             }
-            if (!AddCryptedSaplingSpendingKey(fvk, vchCryptedSecret)) {
+            if (!AddCryptedSaplingSpendingKey(fvk, vchCryptedSecret, sk.DefaultAddress())) {
                 return false;
             }
         }

src/wallet/crypter.h

@@ -243,10 +243,10 @@ public:
     virtual bool AddCryptedSaplingSpendingKey(
         const libzcash::SaplingFullViewingKey &fvk,
         const std::vector<unsigned char> &vchCryptedSecret,
-        const boost::optional<libzcash::SaplingPaymentAddress> &defaultAddr = boost::none);
+        const libzcash::SaplingPaymentAddress &defaultAddr);
     bool AddSaplingSpendingKey(
         const libzcash::SaplingExtendedSpendingKey &sk,
-        const boost::optional<libzcash::SaplingPaymentAddress> &defaultAddr = boost::none);
+        const libzcash::SaplingPaymentAddress &defaultAddr);
     bool HaveSaplingSpendingKey(const libzcash::SaplingFullViewingKey &fvk) const
     {
         {

src/wallet/gtest/test_wallet.cpp

@@ -512,13 +512,13 @@ TEST(WalletTests, FindMySaplingNotes) {
     // No Sapling notes can be found in tx which does not belong to the wallet
     CWalletTx wtx {&wallet, tx};
     ASSERT_FALSE(wallet.HaveSaplingSpendingKey(fvk));
-    auto noteMap = wallet.FindMySaplingNotes(wtx);
+    auto noteMap = wallet.FindMySaplingNotes(wtx).first;
     EXPECT_EQ(0, noteMap.size());
 
     // Add spending key to wallet, so Sapling notes can be found
-    ASSERT_TRUE(wallet.AddSaplingZKey(sk));
+    ASSERT_TRUE(wallet.AddSaplingZKey(sk, pk));
     ASSERT_TRUE(wallet.HaveSaplingSpendingKey(fvk));
-    noteMap = wallet.FindMySaplingNotes(wtx);
+    noteMap = wallet.FindMySaplingNotes(wtx).first;
     EXPECT_EQ(2, noteMap.size());
 
     // Revert to default
@@ -630,7 +630,7 @@ TEST(WalletTests, GetConflictedSaplingNotes) {
     auto ivk = fvk.in_viewing_key();
     auto pk = sk.DefaultAddress();
 
-    ASSERT_TRUE(wallet.AddSaplingZKey(sk));
+    ASSERT_TRUE(wallet.AddSaplingZKey(sk, pk));
     ASSERT_TRUE(wallet.HaveSaplingSpendingKey(fvk));
 
     // Generate note A
@@ -664,7 +664,7 @@ TEST(WalletTests, GetConflictedSaplingNotes) {
     EXPECT_EQ(0, chainActive.Height());
 
     // Simulate SyncTransaction which calls AddToWalletIfInvolvingMe
-    auto saplingNoteData = wallet.FindMySaplingNotes(wtx);
+    auto saplingNoteData = wallet.FindMySaplingNotes(wtx).first;
     ASSERT_TRUE(saplingNoteData.size() > 0);
     wtx.SetSaplingNoteData(saplingNoteData);
     wtx.SetMerkleBranch(block);
@@ -815,7 +815,7 @@ TEST(WalletTests, SaplingNullifierIsSpent) {
     auto tx = maybe_tx.get();
 
     CWalletTx wtx {&wallet, tx};
-    ASSERT_TRUE(wallet.AddSaplingZKey(sk));
+    ASSERT_TRUE(wallet.AddSaplingZKey(sk, pk));
     ASSERT_TRUE(wallet.HaveSaplingSpendingKey(fvk));
 
     // Manually compute the nullifier based on the known position
@@ -912,7 +912,7 @@ TEST(WalletTests, NavigateFromSaplingNullifierToNote) {
     auto tx = maybe_tx.get();
 
     CWalletTx wtx {&wallet, tx};
-    ASSERT_TRUE(wallet.AddSaplingZKey(sk));
+    ASSERT_TRUE(wallet.AddSaplingZKey(sk, pk));
     ASSERT_TRUE(wallet.HaveSaplingSpendingKey(fvk));
 
     // Manually compute the nullifier based on the expected position
@@ -938,7 +938,7 @@ TEST(WalletTests, NavigateFromSaplingNullifierToNote) {
 
     // Simulate SyncTransaction which calls AddToWalletIfInvolvingMe
     wtx.SetMerkleBranch(block);
-    auto saplingNoteData = wallet.FindMySaplingNotes(wtx);
+    auto saplingNoteData = wallet.FindMySaplingNotes(wtx).first;
     ASSERT_TRUE(saplingNoteData.size() > 0);
     wtx.SetSaplingNoteData(saplingNoteData);
     wallet.AddToWallet(wtx, true, NULL);
@@ -1048,7 +1048,7 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) {
     auto tx = maybe_tx.get();
 
     CWalletTx wtx {&wallet, tx};
-    ASSERT_TRUE(wallet.AddSaplingZKey(sk));
+    ASSERT_TRUE(wallet.AddSaplingZKey(sk, pk));
     ASSERT_TRUE(wallet.HaveSaplingSpendingKey(fvk));
 
     // Fake-mine the transaction
@@ -1064,7 +1064,7 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) {
     EXPECT_TRUE(chainActive.Contains(&fakeIndex));
     EXPECT_EQ(0, chainActive.Height());
 
-    auto saplingNoteData = wallet.FindMySaplingNotes(wtx);
+    auto saplingNoteData = wallet.FindMySaplingNotes(wtx).first;
     ASSERT_TRUE(saplingNoteData.size() > 0);
     wtx.SetSaplingNoteData(saplingNoteData);
     wtx.SetMerkleBranch(block);
@@ -1141,7 +1141,7 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) {
     EXPECT_TRUE(chainActive.Contains(&fakeIndex2));
     EXPECT_EQ(1, chainActive.Height());
 
-    auto saplingNoteData2 = wallet.FindMySaplingNotes(wtx2);
+    auto saplingNoteData2 = wallet.FindMySaplingNotes(wtx2).first;
     ASSERT_TRUE(saplingNoteData2.size() > 0);
     wtx2.SetSaplingNoteData(saplingNoteData2);
     wtx2.SetMerkleBranch(block2);
@@ -1751,7 +1751,7 @@ TEST(WalletTests, UpdatedSaplingNoteData) {
 
     // Wallet contains fvk1 but not fvk2
     CWalletTx wtx {&wallet, tx};
-    ASSERT_TRUE(wallet.AddSaplingZKey(sk));
+    ASSERT_TRUE(wallet.AddSaplingZKey(sk, pk));
     ASSERT_TRUE(wallet.HaveSaplingSpendingKey(fvk));
     ASSERT_FALSE(wallet.HaveSaplingSpendingKey(fvk2));
 
@@ -1769,7 +1769,7 @@ TEST(WalletTests, UpdatedSaplingNoteData) {
     EXPECT_EQ(0, chainActive.Height());
 
     // Simulate SyncTransaction which calls AddToWalletIfInvolvingMe
-    auto saplingNoteData = wallet.FindMySaplingNotes(wtx);
+    auto saplingNoteData = wallet.FindMySaplingNotes(wtx).first;
     ASSERT_TRUE(saplingNoteData.size() == 1); // wallet only has key for change output
     wtx.SetSaplingNoteData(saplingNoteData);
     wtx.SetMerkleBranch(block);
@@ -1784,10 +1784,10 @@ TEST(WalletTests, UpdatedSaplingNoteData) {
     wtx = wallet.mapWallet[hash];
 
     // Now lets add key fvk2 so wallet can find the payment note sent to pk2
-    ASSERT_TRUE(wallet.AddSaplingZKey(sk2));
+    ASSERT_TRUE(wallet.AddSaplingZKey(sk2, pk2));
     ASSERT_TRUE(wallet.HaveSaplingSpendingKey(fvk2));
     CWalletTx wtx2 = wtx;
-    auto saplingNoteData2 = wallet.FindMySaplingNotes(wtx2);
+    auto saplingNoteData2 = wallet.FindMySaplingNotes(wtx2).first;
     ASSERT_TRUE(saplingNoteData2.size() == 2);
     wtx2.SetSaplingNoteData(saplingNoteData2);
 
@@ -1881,7 +1881,7 @@ TEST(WalletTests, MarkAffectedSaplingTransactionsDirty) {
     auto ivk = fvk.in_viewing_key();
     auto pk = sk.DefaultAddress();
 
-    ASSERT_TRUE(wallet.AddSaplingZKey(sk));
+    ASSERT_TRUE(wallet.AddSaplingZKey(sk, pk));
     ASSERT_TRUE(wallet.HaveSaplingSpendingKey(fvk));
 
     // Set up transparent address
@@ -1923,7 +1923,7 @@ TEST(WalletTests, MarkAffectedSaplingTransactionsDirty) {
     EXPECT_EQ(0, chainActive.Height());
 
     // Simulate SyncTransaction which calls AddToWalletIfInvolvingMe
-    auto saplingNoteData = wallet.FindMySaplingNotes(wtx);
+    auto saplingNoteData = wallet.FindMySaplingNotes(wtx).first;
     ASSERT_TRUE(saplingNoteData.size() > 0);
     wtx.SetSaplingNoteData(saplingNoteData);
     wtx.SetMerkleBranch(block);

src/wallet/rpcdump.cpp

@@ -724,8 +724,10 @@ UniValue z_importviewingkey(const UniValue& params, bool fHelp)
     if (!IsValidViewingKey(viewingkey)) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid viewing key");
     }
-    // TODO: Add Sapling support. For now, ensure we can freely convert.
+    // TODO: Add Sapling support. For now, return an error to the user.
-    assert(boost::get<libzcash::SproutViewingKey>(&viewingkey) != nullptr);
+    if (boost::get<libzcash::SproutViewingKey>(&viewingkey) == nullptr) {
+        throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Currently, only Sprout viewing keys are supported");
+    }
     auto vkey = boost::get<libzcash::SproutViewingKey>(viewingkey);
     auto addr = vkey.address();
 
@@ -824,8 +826,10 @@ UniValue z_exportviewingkey(const UniValue& params, bool fHelp)
     if (!IsValidPaymentAddress(address)) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid zaddr");
     }
-    // TODO: Add Sapling support. For now, ensure we can freely convert.
+    // TODO: Add Sapling support. For now, return an error to the user.
-    assert(boost::get<libzcash::SproutPaymentAddress>(&address) != nullptr);
+    if (boost::get<libzcash::SproutPaymentAddress>(&address) == nullptr) {
+        throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Currently, only Sprout zaddrs are supported");
+    }
     auto addr = boost::get<libzcash::SproutPaymentAddress>(address);
 
     libzcash::SproutViewingKey vk;

src/wallet/rpcwallet.cpp

@@ -2799,12 +2799,13 @@ UniValue z_listunspent(const UniValue& params, bool fHelp)
             "Optionally filter to only include notes sent to specified addresses.\n"
             "When minconf is 0, unspent notes with zero confirmations are returned, even though they are not immediately spendable.\n"
             "Results are an array of Objects, each of which has:\n"
-            "{txid, jsindex, jsoutindex, confirmations, address, amount, memo}\n"
+            "{txid, jsindex, jsoutindex, confirmations, address, amount, memo} (Sprout)\n"
+            "{txid, outindex, confirmations, address, amount, memo} (Sapling)\n"
             "\nArguments:\n"
             "1. minconf          (numeric, optional, default=1) The minimum confirmations to filter\n"
             "2. maxconf          (numeric, optional, default=9999999) The maximum confirmations to filter\n"
             "3. includeWatchonly (bool, optional, default=false) Also include watchonly addresses (see 'z_importviewingkey')\n"
-            "4. \"addresses\"      (string) A json array of zaddrs to filter on.  Duplicate addresses not allowed.\n"
+            "4. \"addresses\"      (string) A json array of zaddrs (both Sprout and Sapling) to filter on.  Duplicate addresses not allowed.\n"
             "    [\n"
             "      \"address\"     (string) zaddr\n"
             "      ,...\n"
@@ -2814,7 +2815,8 @@ UniValue z_listunspent(const UniValue& params, bool fHelp)
             "  {\n"
             "    \"txid\" : \"txid\",          (string) the transaction id \n"
             "    \"jsindex\" : n             (numeric) the joinsplit index\n"
-            "    \"jsoutindex\" : n          (numeric) the output index of the joinsplit\n"
+            "    \"jsoutindex\" (sprout) : n          (numeric) the output index of the joinsplit\n"
+            "    \"outindex\" (sapling) : n          (numeric) the output index\n"
             "    \"confirmations\" : n       (numeric) the number of confirmations\n"
             "    \"spendable\" : true|false  (boolean) true if note can be spent by wallet, false if note has zero confirmations, false if address is watchonly\n"
             "    \"address\" : \"address\",    (string) the shielded address\n"
@@ -2874,17 +2876,14 @@ UniValue z_listunspent(const UniValue& params, bool fHelp)
             }
             string address = o.get_str();
             auto zaddr = DecodePaymentAddress(address);
-            if (IsValidPaymentAddress(zaddr)) {
+            if (!IsValidPaymentAddress(zaddr)) {
-                // TODO: Add Sapling support. For now, ensure we can freely convert.
-                assert(boost::get<libzcash::SproutPaymentAddress>(&zaddr) != nullptr);
-                libzcash::SproutPaymentAddress addr = boost::get<libzcash::SproutPaymentAddress>(zaddr);
-                if (!fIncludeWatchonly && !pwalletMain->HaveSproutSpendingKey(addr)) {
-                    throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, spending key for address does not belong to wallet: ") + address);
-                }
-                zaddrs.insert(addr);
-            } else {
                 throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, address is not a valid zaddr: ") + address);
             }
+            auto hasSpendingKey = boost::apply_visitor(HaveSpendingKeyForPaymentAddress(pwalletMain), zaddr);
+            if (!fIncludeWatchonly && !hasSpendingKey) {
+                throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, spending key for address does not belong to wallet: ") + address);
+            }
+            zaddrs.insert(zaddr);
 
             if (setAddress.count(address)) {
                 throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, duplicated address: ") + address);
@@ -2894,10 +2893,15 @@ UniValue z_listunspent(const UniValue& params, bool fHelp)
     }
     else {
         // User did not provide zaddrs, so use default i.e. all addresses
-        // TODO: Add Sapling support
         std::set<libzcash::SproutPaymentAddress> sproutzaddrs = {};
         pwalletMain->GetSproutPaymentAddresses(sproutzaddrs);
+        
+        // Sapling support
+        std::set<libzcash::SaplingPaymentAddress> saplingzaddrs = {};
+        pwalletMain->GetSaplingPaymentAddresses(saplingzaddrs);
+        
         zaddrs.insert(sproutzaddrs.begin(), sproutzaddrs.end());
+        zaddrs.insert(saplingzaddrs.begin(), saplingzaddrs.end());
     }
 
     UniValue results(UniValue::VARR);
@@ -2907,6 +2911,7 @@ UniValue z_listunspent(const UniValue& params, bool fHelp)
         std::vector<UnspentSaplingNoteEntry> saplingEntries;
         pwalletMain->GetUnspentFilteredNotes(sproutEntries, saplingEntries, zaddrs, nMinDepth, nMaxDepth, !fIncludeWatchonly);
         std::set<std::pair<PaymentAddress, uint256>> nullifierSet = pwalletMain->GetNullifiersForAddresses(zaddrs);
+        
         for (CUnspentSproutNotePlaintextEntry & entry : sproutEntries) {
             UniValue obj(UniValue::VOBJ);
             obj.push_back(Pair("txid", entry.jsop.hash.ToString()));
@@ -2920,11 +2925,30 @@ UniValue z_listunspent(const UniValue& params, bool fHelp)
             std::string data(entry.plaintext.memo().begin(), entry.plaintext.memo().end());
             obj.push_back(Pair("memo", HexStr(data)));
             if (hasSproutSpendingKey) {
-                obj.push_back(Pair("change", pwalletMain->IsNoteChange(nullifierSet, entry.address, entry.jsop)));
+                obj.push_back(Pair("change", pwalletMain->IsNoteSproutChange(nullifierSet, entry.address, entry.jsop)));
+            }
+            results.push_back(obj);
+        }
+        
+        for (UnspentSaplingNoteEntry & entry : saplingEntries) {
+            UniValue obj(UniValue::VOBJ);
+            obj.push_back(Pair("txid", entry.op.hash.ToString()));
+            obj.push_back(Pair("outindex", (int)entry.op.n));
+            obj.push_back(Pair("confirmations", entry.nHeight));
+            libzcash::SaplingIncomingViewingKey ivk;
+            libzcash::SaplingFullViewingKey fvk;
+            pwalletMain->GetSaplingIncomingViewingKey(boost::get<libzcash::SaplingPaymentAddress>(entry.address), ivk);
+            pwalletMain->GetSaplingFullViewingKey(ivk, fvk);
+            bool hasSaplingSpendingKey = pwalletMain->HaveSaplingSpendingKey(fvk);
+            obj.push_back(Pair("spendable", hasSaplingSpendingKey));
+            obj.push_back(Pair("address", EncodePaymentAddress(entry.address)));
+            obj.push_back(Pair("amount", ValueFromAmount(CAmount(entry.note.value())))); // note.value() is equivalent to plaintext.value()
+            obj.push_back(Pair("memo", HexStr(entry.memo)));
+            if (hasSaplingSpendingKey) {
+                obj.push_back(Pair("change", pwalletMain->IsNoteSaplingChange(nullifierSet, entry.address, entry.op)));
             }
             results.push_back(obj);
         }
-        // TODO: Sapling
     }
 
     return results;
@@ -3486,7 +3510,7 @@ UniValue z_getnewaddress(const UniValue& params, bool fHelp)
 
     if (addrType == ADDR_TYPE_SPROUT) {
         return EncodePaymentAddress(pwalletMain->GenerateNewZKey());
-    } else if (addrType == ADDR_TYPE_SAPLING && allowSapling) {
+    } else if (addrType == ADDR_TYPE_SAPLING) {
         return EncodePaymentAddress(pwalletMain->GenerateNewSaplingZKey());
     } else {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid address type");
@@ -3650,12 +3674,9 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp)
     if (!IsValidPaymentAddress(zaddr)) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid zaddr.");
     }
-    // TODO: Add Sapling support. For now, ensure we can freely convert.
-    assert(boost::get<libzcash::SproutPaymentAddress>(&zaddr) != nullptr);
-    auto sproutzaddr = boost::get<libzcash::SproutPaymentAddress>(zaddr);
 
-    bool hasSproutSpendingKey = pwalletMain->HaveSproutSpendingKey(sproutzaddr);
+    // Visitor to support Sprout and Sapling addrs
-    if (!(hasSproutSpendingKey || pwalletMain->HaveSproutViewingKey(sproutzaddr))) {
+    if (!boost::apply_visitor(PaymentAddressBelongsToWallet(pwalletMain), zaddr)) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "From address does not belong to this node, zaddr spending key or viewing key not found.");
     }
 
@@ -3663,22 +3684,40 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp)
     std::vector<CSproutNotePlaintextEntry> sproutEntries;
     std::vector<SaplingNoteEntry> saplingEntries;
     pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, fromaddress, nMinDepth, false, false);
-    auto nullifierSet = hasSproutSpendingKey ? pwalletMain->GetNullifiersForAddresses({zaddr}) : std::set<std::pair<PaymentAddress, uint256>>();
+
-    for (CSproutNotePlaintextEntry & entry : sproutEntries) {
+    std::set<std::pair<PaymentAddress, uint256>> nullifierSet;
-        UniValue obj(UniValue::VOBJ);
+    auto hasSpendingKey = boost::apply_visitor(HaveSpendingKeyForPaymentAddress(pwalletMain), zaddr);
-        obj.push_back(Pair("txid", entry.jsop.hash.ToString()));
+    if (hasSpendingKey) {
-        obj.push_back(Pair("amount", ValueFromAmount(CAmount(entry.plaintext.value()))));
+        nullifierSet = pwalletMain->GetNullifiersForAddresses({zaddr});
-        std::string data(entry.plaintext.memo().begin(), entry.plaintext.memo().end());
+    }
-        obj.push_back(Pair("memo", HexStr(data)));
+
-        // (txid, jsindex, jsoutindex) is needed to globally identify a note
+    if (boost::get<libzcash::SproutPaymentAddress>(&zaddr) != nullptr) {
-        obj.push_back(Pair("jsindex", entry.jsop.js));
+        for (CSproutNotePlaintextEntry & entry : sproutEntries) {
-        obj.push_back(Pair("jsoutindex", entry.jsop.n));
+            UniValue obj(UniValue::VOBJ);
-        if (hasSproutSpendingKey) {
+            obj.push_back(Pair("txid", entry.jsop.hash.ToString()));
-            obj.push_back(Pair("change", pwalletMain->IsNoteChange(nullifierSet, entry.address, entry.jsop)));
+            obj.push_back(Pair("amount", ValueFromAmount(CAmount(entry.plaintext.value()))));
-        }
+            std::string data(entry.plaintext.memo().begin(), entry.plaintext.memo().end());
-        result.push_back(obj);
+            obj.push_back(Pair("memo", HexStr(data)));
+            obj.push_back(Pair("jsindex", entry.jsop.js));
+            obj.push_back(Pair("jsoutindex", entry.jsop.n));
+            if (hasSpendingKey) {
+                obj.push_back(Pair("change", pwalletMain->IsNoteSproutChange(nullifierSet, entry.address, entry.jsop)));
+            }
+            result.push_back(obj);
+        }
+    } else if (boost::get<libzcash::SaplingPaymentAddress>(&zaddr) != nullptr) {
+        for (SaplingNoteEntry & entry : saplingEntries) {
+            UniValue obj(UniValue::VOBJ);
+            obj.push_back(Pair("txid", entry.op.hash.ToString()));
+            obj.push_back(Pair("amount", ValueFromAmount(CAmount(entry.note.value()))));
+            obj.push_back(Pair("memo", HexStr(entry.memo)));
+            obj.push_back(Pair("outindex", (int)entry.op.n));
+            if (hasSpendingKey) {
+              obj.push_back(Pair("change", pwalletMain->IsNoteSaplingChange(nullifierSet, entry.address, entry.op)));
+            }
+            result.push_back(obj);
+        }
     }
-    // TODO: Sapling
     return result;
 }
 

src/wallet/wallet.cpp

@@ -165,7 +165,7 @@ SaplingPaymentAddress CWallet::GenerateNewSaplingZKey()
 // Add spending key to keystore 
 bool CWallet::AddSaplingZKey(
     const libzcash::SaplingExtendedSpendingKey &sk,
-    const boost::optional<libzcash::SaplingPaymentAddress> &defaultAddr)
+    const libzcash::SaplingPaymentAddress &defaultAddr)
 {
     AssertLockHeld(cs_wallet); // mapSaplingZKeyMetadata
 
@@ -306,7 +306,7 @@ bool CWallet::AddCryptedSproutSpendingKey(
 
 bool CWallet::AddCryptedSaplingSpendingKey(const libzcash::SaplingFullViewingKey &fvk,
                                            const std::vector<unsigned char> &vchCryptedSecret,
-                                           const boost::optional<libzcash::SaplingPaymentAddress> &defaultAddr)
+                                           const libzcash::SaplingPaymentAddress &defaultAddr)
 {
     if (!CCryptoKeyStore::AddCryptedSaplingSpendingKey(fvk, vchCryptedSecret, defaultAddr))
         return false;
@@ -523,20 +523,50 @@ void CWallet::SetBestChain(const CBlockLocator& loc)
     SetBestChainINTERNAL(walletdb, loc);
 }
 
-std::set<std::pair<libzcash::PaymentAddress, uint256>> CWallet::GetNullifiersForAddresses(const std::set<libzcash::PaymentAddress> & addresses)
+std::set<std::pair<libzcash::PaymentAddress, uint256>> CWallet::GetNullifiersForAddresses(
+        const std::set<libzcash::PaymentAddress> & addresses)
 {
     std::set<std::pair<libzcash::PaymentAddress, uint256>> nullifierSet;
+    // Sapling ivk -> list of addrs map
+    // (There may be more than one diversified address for a given ivk.)
+    std::map<libzcash::SaplingIncomingViewingKey, std::vector<libzcash::SaplingPaymentAddress>> ivkMap;
+    for (const auto & addr : addresses) {
+        auto saplingAddr = boost::get<libzcash::SaplingPaymentAddress>(&addr);
+        if (saplingAddr != nullptr) {
+            libzcash::SaplingIncomingViewingKey ivk;
+            this->GetSaplingIncomingViewingKey(*saplingAddr, ivk);
+            ivkMap[ivk].push_back(*saplingAddr);
+        }
+    }
     for (const auto & txPair : mapWallet) {
+        // Sprout
         for (const auto & noteDataPair : txPair.second.mapSproutNoteData) {
-            if (noteDataPair.second.nullifier && addresses.count(noteDataPair.second.address)) {
+            auto & noteData = noteDataPair.second;
-                nullifierSet.insert(std::make_pair(noteDataPair.second.address, noteDataPair.second.nullifier.get()));
+            auto & nullifier = noteData.nullifier;
+            auto & address = noteData.address;
+            if (nullifier && addresses.count(address)) {
+                nullifierSet.insert(std::make_pair(address, nullifier.get()));
+            }
+        }
+        // Sapling
+        for (const auto & noteDataPair : txPair.second.mapSaplingNoteData) {
+            auto & noteData = noteDataPair.second;
+            auto & nullifier = noteData.nullifier;
+            auto & ivk = noteData.ivk;
+            if (nullifier && ivkMap.count(ivk)) {
+                for (const auto & addr : ivkMap[ivk]) {
+                    nullifierSet.insert(std::make_pair(addr, nullifier.get()));
+                }
             }
         }
     }
     return nullifierSet;
 }
 
-bool CWallet::IsNoteChange(const std::set<std::pair<libzcash::PaymentAddress, uint256>> & nullifierSet, const PaymentAddress & address, const JSOutPoint & jsop)
+bool CWallet::IsNoteSproutChange(
+        const std::set<std::pair<libzcash::PaymentAddress, uint256>> & nullifierSet,
+        const PaymentAddress & address,
+        const JSOutPoint & jsop)
 {
     // A Note is marked as "change" if the address that received it
     // also spent Notes in the same transaction. This will catch,
@@ -557,6 +587,26 @@ bool CWallet::IsNoteChange(const std::set<std::pair<libzcash::PaymentAddress, ui
     return false;
 }
 
+bool CWallet::IsNoteSaplingChange(const std::set<std::pair<libzcash::PaymentAddress, uint256>> & nullifierSet,
+        const libzcash::PaymentAddress & address,
+        const SaplingOutPoint & op)
+{
+    // A Note is marked as "change" if the address that received it
+    // also spent Notes in the same transaction. This will catch,
+    // for instance:
+    // - Change created by spending fractions of Notes (because
+    //   z_sendmany sends change to the originating z-address).
+    // - Notes created by consolidation transactions (e.g. using
+    //   z_mergetoaddress).
+    // - Notes sent from one address to itself.
+    for (const SpendDescription &spend : mapWallet[op.hash].vShieldedSpend) {
+        if (nullifierSet.count(std::make_pair(address, spend.nullifier))) {
+            return true;
+        }
+    }
+    return false;
+}
+
 bool CWallet::SetMinVersion(enum WalletFeature nVersion, CWalletDB* pwalletdbIn, bool fExplicit)
 {
     LOCK(cs_wallet); // nWalletVersion
@@ -1256,9 +1306,9 @@ int32_t CWallet::VerusStakeTransaction(CBlock *pBlock, CMutableTransaction &txNe
     txnouttype whichType;
     std::vector<std::vector<unsigned char>> vSolutions;
 
-    CBlockIndex *tipindex;
+    CBlockIndex *tipindex = chainActive.LastTip();
-    tipindex = chainActive.LastTip();
+    uint32_t stakeHeight = tipindex->GetHeight() + 1;
-    bool extendedStake = tipindex->GetHeight() >= Params().GetConsensus().vUpgrades[Consensus::UPGRADE_SAPLING].nActivationHeight;
+    bool extendedStake = stakeHeight >= Params().GetConsensus().vUpgrades[Consensus::UPGRADE_SAPLING].nActivationHeight;
 
     if (!extendedStake)
         pk = CPubKey();
@@ -1275,7 +1325,6 @@ int32_t CWallet::VerusStakeTransaction(CBlock *pBlock, CMutableTransaction &txNe
     bool signSuccess; 
     SignatureData sigdata; 
     uint64_t txfee;
-    uint32_t stakeHeight = chainActive.Height() + 1;
     auto consensusBranchId = CurrentEpochBranchId(stakeHeight, Params().GetConsensus());
 
     const CKeyStore& keystore = *pwalletMain;
@@ -1321,10 +1370,11 @@ int32_t CWallet::VerusStakeTransaction(CBlock *pBlock, CMutableTransaction &txNe
             return 0;
 
         txOut1.scriptPubKey << OP_RETURN 
-            << CStakeParams(pSrcIndex->GetHeight(), tipindex->GetHeight() + 1, pBlock->hashPrevBlock, pk).AsVector();
+            << CStakeParams(pSrcIndex->GetHeight(), tipindex->GetHeight() + 1, tipindex->GetBlockHash(), pk).AsVector();
     }
 
     nValue = txNew.vout[0].nValue = stakeSource.vout[voutNum].nValue - txfee;
+
     txNew.nLockTime = 0;
     CTransaction txNewConst(txNew);
     signSuccess = ProduceSignature(TransactionSignatureCreator(&keystore, &txNewConst, 0, nValue, SIGHASH_ALL), stakeSource.vout[voutNum].scriptPubKey, sigdata, consensusBranchId);
@@ -1645,7 +1695,14 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl
         bool fExisted = mapWallet.count(tx.GetHash()) != 0;
         if (fExisted && !fUpdate) return false;
         auto sproutNoteData = FindMySproutNotes(tx);
-        auto saplingNoteData = FindMySaplingNotes(tx);
+        auto saplingNoteDataAndAddressesToAdd = FindMySaplingNotes(tx);
+        auto saplingNoteData = saplingNoteDataAndAddressesToAdd.first;
+        auto addressesToAdd = saplingNoteDataAndAddressesToAdd.second;
+        for (const auto &addressToAdd : addressesToAdd) {
+            if (!AddSaplingIncomingViewingKey(addressToAdd.second, addressToAdd.first)) {
+                return false;
+            }
+        }
         if (fExisted || IsMine(tx) || IsFromMe(tx) || sproutNoteData.size() > 0 || saplingNoteData.size() > 0)
         {
             CWalletTx wtx(this,tx);
@@ -1806,12 +1863,13 @@ mapSproutNoteData_t CWallet::FindMySproutNotes(const CTransaction &tx) const
  * the result of FindMySaplingNotes (for the addresses available at the time) will
  * already have been cached in CWalletTx.mapSaplingNoteData.
  */
-mapSaplingNoteData_t CWallet::FindMySaplingNotes(const CTransaction &tx) const
+std::pair<mapSaplingNoteData_t, SaplingIncomingViewingKeyMap> CWallet::FindMySaplingNotes(const CTransaction &tx) const
 {
     LOCK(cs_SpendingKeyStore);
     uint256 hash = tx.GetHash();
 
     mapSaplingNoteData_t noteData;
+    SaplingIncomingViewingKeyMap viewingKeysToAdd;
 
     // Protocol Spec: 4.19 Block Chain Scanning (Sapling)
     for (uint32_t i = 0; i < tx.vShieldedOutput.size(); ++i) {
@@ -1822,6 +1880,10 @@ mapSaplingNoteData_t CWallet::FindMySaplingNotes(const CTransaction &tx) const
             if (!result) {
                 continue;
             }
+            auto address = ivk.address(result.get().d);
+            if (address && mapSaplingIncomingViewingKeys.count(address.get()) == 0) {
+                viewingKeysToAdd[address.get()] = ivk;
+            }
             // We don't cache the nullifier here as computing it requires knowledge of the note position
             // in the commitment tree, which can only be determined when the transaction has been mined.
             SaplingOutPoint op {hash, i};
@@ -1832,7 +1894,7 @@ mapSaplingNoteData_t CWallet::FindMySaplingNotes(const CTransaction &tx) const
         }
     }
 
-    return noteData;
+    return std::make_pair(noteData, viewingKeysToAdd);
 }
 
 bool CWallet::IsSproutNullifierFromMe(const uint256& nullifier) const

src/wallet/wallet.h

@@ -1059,11 +1059,11 @@ public:
     //! Adds Sapling spending key to the store, and saves it to disk
     bool AddSaplingZKey(
         const libzcash::SaplingExtendedSpendingKey &key,
-        const boost::optional<libzcash::SaplingPaymentAddress> &defaultAddr = boost::none);
+        const libzcash::SaplingPaymentAddress &defaultAddr);
     bool AddCryptedSaplingSpendingKey(
         const libzcash::SaplingFullViewingKey &fvk,
         const std::vector<unsigned char> &vchCryptedSecret,
-        const boost::optional<libzcash::SaplingPaymentAddress> &defaultAddr = boost::none);
+        const libzcash::SaplingPaymentAddress &defaultAddr);
 
     /**
      * Increment the next transaction order id
@@ -1133,7 +1133,7 @@ public:
         const uint256& hSig,
         uint8_t n) const;
     mapSproutNoteData_t FindMySproutNotes(const CTransaction& tx) const;
-    mapSaplingNoteData_t FindMySaplingNotes(const CTransaction& tx) const;
+    std::pair<mapSaplingNoteData_t, SaplingIncomingViewingKeyMap> FindMySaplingNotes(const CTransaction& tx) const;
     bool IsSproutNullifierFromMe(const uint256& nullifier) const;
     bool IsSaplingNullifierFromMe(const uint256& nullifier) const;
 
@@ -1164,7 +1164,8 @@ public:
     /** Saves witness caches and best block locator to disk. */
     void SetBestChain(const CBlockLocator& loc);
     std::set<std::pair<libzcash::PaymentAddress, uint256>> GetNullifiersForAddresses(const std::set<libzcash::PaymentAddress> & addresses);
-    bool IsNoteChange(const std::set<std::pair<libzcash::PaymentAddress, uint256>> & nullifierSet, const libzcash::PaymentAddress & address, const JSOutPoint & entry);
+    bool IsNoteSproutChange(const std::set<std::pair<libzcash::PaymentAddress, uint256>> & nullifierSet, const libzcash::PaymentAddress & address, const JSOutPoint & entry);
+    bool IsNoteSaplingChange(const std::set<std::pair<libzcash::PaymentAddress, uint256>> & nullifierSet, const libzcash::PaymentAddress & address, const SaplingOutPoint & entry);
 
     DBErrors LoadWallet(bool& fFirstRunRet);
     DBErrors ZapWalletTx(std::vector<CWalletTx>& vWtx);