Merge branch 'master' of https://github.com/zcash/zcash into dev

qa/pull-tester/rpc-tests.sh

@@ -25,7 +25,8 @@ testScripts=(
     'wallet_mergetoaddress.py'
     'wallet.py'
     'wallet_overwintertx.py'
-#    'wallet_nullifiers.py'
+    'wallet_persistence.py'
+    'wallet_nullifiers.py'
     'wallet_1941.py'
     'wallet_addresses.py'
     'wallet_sapling.py'
@@ -46,7 +47,7 @@ testScripts=(
     'zapwallettxes.py'
     'proxy_test.py'
     'merkle_blocks.py'
-#    'fundrawtransaction.py'
+    'fundrawtransaction.py'
     'signrawtransactions.py'
     'signrawtransaction_offline.py'
     'walletbackup.py'

qa/rpc-tests/wallet_persistence.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,
+    start_nodes, stop_nodes,
+    initialize_chain_clean, connect_nodes_bi, wait_bitcoinds,
+    wait_and_assert_operationid_status
+)
+from decimal import Decimal
+
+class WalletPersistenceTest (BitcoinTestFramework):
+
+    def setup_chain(self):
+        print("Initializing test directory " + self.options.tmpdir)
+        initialize_chain_clean(self.options.tmpdir, 2)
+
+    def setup_network(self, split=False):
+        self.nodes = start_nodes(2, self.options.tmpdir,
+            extra_args=[[
+                '-nuparams=5ba81b19:100', # Overwinter
+                '-nuparams=76b809bb:201', # Sapling
+            ]] * 2)
+        connect_nodes_bi(self.nodes,0,1)
+        self.is_network_split=False
+        self.sync_all()
+
+    def run_test(self):
+        # Sanity-check the test harness
+        self.nodes[0].generate(200)
+        assert_equal(self.nodes[0].getblockcount(), 200)
+        self.sync_all()
+
+        # Verify Sapling address is persisted in wallet (even when Sapling is not yet active)
+        sapling_addr = self.nodes[0].z_getnewaddress('sapling')
+
+        # Make sure the node has the addresss
+        addresses = self.nodes[0].z_listaddresses()
+        assert_true(sapling_addr in addresses, "Should contain address before restart")
+
+        # Restart the nodes
+        stop_nodes(self.nodes)
+        wait_bitcoinds()
+        self.setup_network()
+
+        # Make sure we still have the address after restarting
+        addresses = self.nodes[0].z_listaddresses()
+        assert_true(sapling_addr in addresses, "Should contain address after restart")
+
+        # Activate Sapling
+        self.nodes[0].generate(1)
+        self.sync_all()
+
+        # Node 0 shields funds to Sapling address
+        taddr0 = self.nodes[0].getnewaddress()
+        recipients = []
+        recipients.append({"address": sapling_addr, "amount": Decimal('20')})
+        myopid = self.nodes[0].z_sendmany(taddr0, recipients, 1, 0)
+        wait_and_assert_operationid_status(self.nodes[0], myopid)
+
+        self.sync_all()
+        self.nodes[0].generate(1)
+        self.sync_all()
+
+        # Verify shielded balance
+        assert_equal(self.nodes[0].z_getbalance(sapling_addr), Decimal('20'))
+
+        # Restart the nodes
+        stop_nodes(self.nodes)
+        wait_bitcoinds()
+        self.setup_network()
+
+        # Node 0 sends some shielded funds to Node 1
+        dest_addr = self.nodes[1].z_getnewaddress('sapling')
+        recipients = []
+        recipients.append({"address": dest_addr, "amount": Decimal('15')})
+        myopid = self.nodes[0].z_sendmany(sapling_addr, recipients, 1, 0)
+        wait_and_assert_operationid_status(self.nodes[0], myopid)
+
+        self.sync_all()
+        self.nodes[0].generate(1)
+        self.sync_all()
+
+        # Verify balances
+        assert_equal(self.nodes[0].z_getbalance(sapling_addr), Decimal('5'))
+        assert_equal(self.nodes[1].z_getbalance(dest_addr), Decimal('15'))
+
+        # Restart the nodes
+        stop_nodes(self.nodes)
+        wait_bitcoinds()
+        self.setup_network()
+
+        # Verify balances
+        assert_equal(self.nodes[0].z_getbalance(sapling_addr), Decimal('5'))
+        assert_equal(self.nodes[1].z_getbalance(dest_addr), Decimal('15'))
+
+if __name__ == '__main__':
+    WalletPersistenceTest().main()

src/keystore.cpp

@@ -157,7 +157,7 @@ bool CBasicKeyStore::AddSaplingFullViewingKey(
     auto ivk = fvk.in_viewing_key();
     mapSaplingFullViewingKeys[ivk] = fvk;
 
-    return AddSaplingIncomingViewingKey(ivk, defaultAddr);
+    return CBasicKeyStore::AddSaplingIncomingViewingKey(ivk, defaultAddr);
 }
 
 // This function updates the wallet's internal address->ivk map. 

src/keystore.h

@@ -312,6 +312,6 @@ typedef std::map<CKeyID, std::pair<CPubKey, std::vector<unsigned char> > > Crypt
 typedef std::map<libzcash::SproutPaymentAddress, std::vector<unsigned char> > CryptedSproutSpendingKeyMap;
 
 //! Sapling 
-typedef std::map<libzcash::SaplingFullViewingKey, std::vector<unsigned char> > CryptedSaplingSpendingKeyMap;
+typedef std::map<libzcash::SaplingExtendedFullViewingKey, std::vector<unsigned char> > CryptedSaplingSpendingKeyMap;
 
 #endif // BITCOIN_KEYSTORE_H

src/test/rpc_wallet_tests.cpp

@@ -1354,7 +1354,6 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_taddr_to_sapling)
 /*
  * This test covers storing encrypted zkeys in the wallet.
  */
-/* TODO: Uncomment during PR for #3388
 BOOST_AUTO_TEST_CASE(rpc_wallet_encrypted_wallet_zkeys)
 {
     LOCK2(cs_main, pwalletMain->cs_wallet);
@@ -1410,7 +1409,67 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_encrypted_wallet_zkeys)
     // We can't simulate over RPC the wallet closing and being reloaded
     // but there are tests for this in gtest.
 }
-*/
+
+BOOST_AUTO_TEST_CASE(rpc_wallet_encrypted_wallet_sapzkeys)
+{
+    LOCK2(cs_main, pwalletMain->cs_wallet);
+    UniValue retValue;
+    int n = 100;
+
+    if(!pwalletMain->HaveHDSeed())
+    {
+        pwalletMain->GenerateNewSeed();
+    }
+
+    // wallet should currently be empty
+    std::set<libzcash::SaplingPaymentAddress> addrs;
+    pwalletMain->GetSaplingPaymentAddresses(addrs);
+    BOOST_CHECK(addrs.size()==0);
+
+    // create keys
+    for (int i = 0; i < n; i++) {
+        CallRPC("z_getnewaddress sapling");
+    }
+
+    // Verify we can list the keys imported
+    BOOST_CHECK_NO_THROW(retValue = CallRPC("z_listaddresses"));
+    UniValue arr = retValue.get_array();
+    BOOST_CHECK(arr.size() == n);
+
+    // Verify that the wallet encryption RPC is disabled
+    BOOST_CHECK_THROW(CallRPC("encryptwallet passphrase"), runtime_error);
+
+    // Encrypt the wallet (we can't call RPC encryptwallet as that shuts down node)
+    SecureString strWalletPass;
+    strWalletPass.reserve(100);
+    strWalletPass = "hello";
+
+    boost::filesystem::current_path(GetArg("-datadir","/tmp/thisshouldnothappen"));
+    BOOST_CHECK(pwalletMain->EncryptWallet(strWalletPass));
+
+    // Verify we can still list the keys imported
+    BOOST_CHECK_NO_THROW(retValue = CallRPC("z_listaddresses"));
+    arr = retValue.get_array();
+    BOOST_CHECK(arr.size() == n);
+
+    // Try to add a new key, but we can't as the wallet is locked
+    BOOST_CHECK_THROW(CallRPC("z_getnewaddress sapling"), runtime_error);
+
+    // We can't call RPC walletpassphrase as that invokes RPCRunLater which breaks tests.
+    // So we manually unlock.
+    BOOST_CHECK(pwalletMain->Unlock(strWalletPass));
+
+    // Now add a key
+    BOOST_CHECK_NO_THROW(CallRPC("z_getnewaddress sapling"));
+
+    // Verify the key has been added
+    BOOST_CHECK_NO_THROW(retValue = CallRPC("z_listaddresses"));
+    arr = retValue.get_array();
+    BOOST_CHECK(arr.size() == n+1);
+
+    // We can't simulate over RPC the wallet closing and being reloaded
+    // but there are tests for this in gtest.
+}
 
 
 BOOST_AUTO_TEST_CASE(rpc_z_listunspent_parameters)

src/wallet/crypter.cpp

@@ -172,11 +172,11 @@ static bool DecryptSproutSpendingKey(const CKeyingMaterial& vMasterKey,
 
 static bool DecryptSaplingSpendingKey(const CKeyingMaterial& vMasterKey,
                                const std::vector<unsigned char>& vchCryptedSecret,
-                               const libzcash::SaplingFullViewingKey& fvk,
+                               const libzcash::SaplingExtendedFullViewingKey& extfvk,
                                libzcash::SaplingExtendedSpendingKey& sk)
 {
     CKeyingMaterial vchSecret;
-    if (!DecryptSecret(vMasterKey, vchCryptedSecret, fvk.GetFingerprint(), vchSecret))
+    if (!DecryptSecret(vMasterKey, vchCryptedSecret, extfvk.fvk.GetFingerprint(), vchSecret))
         return false;
 
     if (vchSecret.size() != ZIP32_XSK_SIZE)
@@ -184,7 +184,7 @@ static bool DecryptSaplingSpendingKey(const CKeyingMaterial& vMasterKey,
 
     CSecureDataStream ss(vchSecret, SER_NETWORK, PROTOCOL_VERSION);
     ss >> sk;
-    return sk.expsk.full_viewing_key() == fvk;
+    return sk.expsk.full_viewing_key() == extfvk.fvk;
 }
 
 bool CCryptoKeyStore::SetCrypted()
@@ -263,10 +263,10 @@ bool CCryptoKeyStore::Unlock(const CKeyingMaterial& vMasterKeyIn)
         CryptedSaplingSpendingKeyMap::const_iterator miSapling = mapCryptedSaplingSpendingKeys.begin();
         for (; miSapling != mapCryptedSaplingSpendingKeys.end(); ++miSapling)
         {
-            const libzcash::SaplingFullViewingKey &fvk = (*miSapling).first;
+            const libzcash::SaplingExtendedFullViewingKey &extfvk = (*miSapling).first;
             const std::vector<unsigned char> &vchCryptedSecret = (*miSapling).second;
             libzcash::SaplingExtendedSpendingKey sk;
-            if (!DecryptSaplingSpendingKey(vMasterKeyIn, vchCryptedSecret, fvk, sk))
+            if (!DecryptSaplingSpendingKey(vMasterKeyIn, vchCryptedSecret, extfvk, sk))
             {
                 keyFail = true;
                 break;
@@ -467,12 +467,12 @@ bool CCryptoKeyStore::AddSaplingSpendingKey(
         CSecureDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
         ss << sk;
         CKeyingMaterial vchSecret(ss.begin(), ss.end());
-        auto fvk = sk.expsk.full_viewing_key();
+        auto extfvk = sk.ToXFVK();
-        if (!EncryptSecret(vMasterKey, vchSecret, fvk.GetFingerprint(), vchCryptedSecret)) {
+        if (!EncryptSecret(vMasterKey, vchSecret, extfvk.fvk.GetFingerprint(), vchCryptedSecret)) {
             return false;
         }
 
-        if (!AddCryptedSaplingSpendingKey(fvk, vchCryptedSecret, defaultAddr)) {
+        if (!AddCryptedSaplingSpendingKey(extfvk, vchCryptedSecret, defaultAddr)) {
             return false;
         }
     }
@@ -496,7 +496,7 @@ bool CCryptoKeyStore::AddCryptedSproutSpendingKey(
 }
 
 bool CCryptoKeyStore::AddCryptedSaplingSpendingKey(
-    const libzcash::SaplingFullViewingKey &fvk,
+    const libzcash::SaplingExtendedFullViewingKey &extfvk,
     const std::vector<unsigned char> &vchCryptedSecret,
     const libzcash::SaplingPaymentAddress &defaultAddr)
 {
@@ -507,11 +507,11 @@ bool CCryptoKeyStore::AddCryptedSaplingSpendingKey(
         }
 
         // if SaplingFullViewingKey is not in SaplingFullViewingKeyMap, add it
-        if (!AddSaplingFullViewingKey(fvk, defaultAddr)) {
+        if (!AddSaplingFullViewingKey(extfvk.fvk, defaultAddr)) {
             return false;
         }
 
-        mapCryptedSaplingSpendingKeys[fvk] = vchCryptedSecret;
+        mapCryptedSaplingSpendingKeys[extfvk] = vchCryptedSecret;
     }
     return true;
 }
@@ -540,11 +540,11 @@ bool CCryptoKeyStore::GetSaplingSpendingKey(const libzcash::SaplingFullViewingKe
         if (!IsCrypted())
             return CBasicKeyStore::GetSaplingSpendingKey(fvk, skOut);
 
-        CryptedSaplingSpendingKeyMap::const_iterator mi = mapCryptedSaplingSpendingKeys.find(fvk);
+        for (auto entry : mapCryptedSaplingSpendingKeys) {
-        if (mi != mapCryptedSaplingSpendingKeys.end())
+            if (entry.first.fvk == fvk) {
-        {
+                const std::vector<unsigned char> &vchCryptedSecret = entry.second;
-            const std::vector<unsigned char> &vchCryptedSecret = (*mi).second;
+                return DecryptSaplingSpendingKey(vMasterKey, vchCryptedSecret, entry.first, skOut);
-            return DecryptSaplingSpendingKey(vMasterKey, vchCryptedSecret, fvk, skOut);
+            }
         }
     }
     return false;
@@ -611,12 +611,12 @@ bool CCryptoKeyStore::EncryptKeys(CKeyingMaterial& vMasterKeyIn)
             CSecureDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
             ss << sk;
             CKeyingMaterial vchSecret(ss.begin(), ss.end());
-            libzcash::SaplingFullViewingKey fvk = sk.expsk.full_viewing_key();
+            auto extfvk = sk.ToXFVK();
             std::vector<unsigned char> vchCryptedSecret;
-            if (!EncryptSecret(vMasterKeyIn, vchSecret, fvk.GetFingerprint(), vchCryptedSecret)) {
+            if (!EncryptSecret(vMasterKeyIn, vchSecret, extfvk.fvk.GetFingerprint(), vchCryptedSecret)) {
                 return false;
             }
-            if (!AddCryptedSaplingSpendingKey(fvk, vchCryptedSecret, sk.DefaultAddress())) {
+            if (!AddCryptedSaplingSpendingKey(extfvk, vchCryptedSecret, sk.DefaultAddress())) {
                 return false;
             }
         }

src/wallet/crypter.h

@@ -241,7 +241,7 @@ public:
     }
     //! Sapling 
     virtual bool AddCryptedSaplingSpendingKey(
-        const libzcash::SaplingFullViewingKey &fvk,
+        const libzcash::SaplingExtendedFullViewingKey &extfvk,
         const std::vector<unsigned char> &vchCryptedSecret,
         const libzcash::SaplingPaymentAddress &defaultAddr);
     bool AddSaplingSpendingKey(
@@ -253,7 +253,11 @@ public:
             LOCK(cs_SpendingKeyStore);
             if (!IsCrypted())
                 return CBasicKeyStore::HaveSaplingSpendingKey(fvk);
-            return mapCryptedSaplingSpendingKeys.count(fvk) > 0;
+            for (auto entry : mapCryptedSaplingSpendingKeys) {
+                if (entry.first.fvk == fvk) {
+                    return true;
+                }
+            }
         }
         return false;
     }

src/wallet/gtest/test_wallet_zkeys.cpp

@@ -10,8 +10,13 @@
 /**
  * This test covers Sapling methods on CWallet
  * GenerateNewSaplingZKey()
+ * AddSaplingZKey()
+ * AddSaplingIncomingViewingKey()
+ * LoadSaplingZKey()
+ * LoadSaplingIncomingViewingKey()
+ * LoadSaplingZKeyMetadata()
  */
-TEST(wallet_zkeys_tests, store_and_load_sapling_zkeys) {
+TEST(wallet_zkeys_tests, StoreAndLoadSaplingZkeys) {
     SelectParams(CBaseChainParams::MAIN);
 
     CWallet wallet;
@@ -58,6 +63,44 @@ TEST(wallet_zkeys_tests, store_and_load_sapling_zkeys) {
     EXPECT_EQ(2, addrs.size());
     EXPECT_EQ(1, addrs.count(address));
     EXPECT_EQ(1, addrs.count(sk.DefaultAddress()));
+
+    // Generate a diversified address different to the default
+    // If we can't get an early diversified address, we are very unlucky
+    blob88 diversifier;
+    diversifier.begin()[0] = 10;
+    auto dpa = sk.ToXFVK().Address(diversifier).get().second;
+
+    // verify wallet only has the default address
+    EXPECT_TRUE(wallet.HaveSaplingIncomingViewingKey(sk.DefaultAddress()));
+    EXPECT_FALSE(wallet.HaveSaplingIncomingViewingKey(dpa));
+
+    // manually add a diversified address
+    auto ivk = fvk.in_viewing_key();
+    EXPECT_TRUE(wallet.AddSaplingIncomingViewingKey(ivk, dpa));
+
+    // verify wallet did add it
+    EXPECT_TRUE(wallet.HaveSaplingIncomingViewingKey(sk.DefaultAddress()));
+    EXPECT_TRUE(wallet.HaveSaplingIncomingViewingKey(dpa));
+
+    // Load a third key into the wallet
+    auto sk2 = m.Derive(1);
+    ASSERT_TRUE(wallet.LoadSaplingZKey(sk2));
+
+    // attach metadata to this third key
+    auto ivk2 = sk2.expsk.full_viewing_key().in_viewing_key();
+    int64_t now = GetTime();
+    CKeyMetadata meta(now);
+    ASSERT_TRUE(wallet.LoadSaplingZKeyMetadata(ivk2, meta));
+
+    // check metadata is the same
+    ASSERT_EQ(wallet.mapSaplingZKeyMetadata[ivk2].nCreateTime, now);
+
+    // Load a diversified address for the third key into the wallet
+    auto dpa2 = sk2.ToXFVK().Address(diversifier).get().second;
+    EXPECT_TRUE(wallet.HaveSaplingIncomingViewingKey(sk2.DefaultAddress()));
+    EXPECT_FALSE(wallet.HaveSaplingIncomingViewingKey(dpa2));
+    EXPECT_TRUE(wallet.LoadSaplingPaymentAddress(dpa2, ivk2));
+    EXPECT_TRUE(wallet.HaveSaplingIncomingViewingKey(dpa2));
 }
 
 /**
@@ -288,7 +331,6 @@ TEST(wallet_zkeys_tests, WriteViewingKeyDirectToDB) {
 /**
  * This test covers methods on CWalletDB to load/save crypted z keys.
  */
-/* TODO: Uncomment during PR for #3388
 TEST(wallet_zkeys_tests, write_cryptedzkey_direct_to_db) {
     SelectParams(CBaseChainParams::TESTNET);
 
@@ -363,5 +405,99 @@ TEST(wallet_zkeys_tests, write_cryptedzkey_direct_to_db) {
     wallet2.GetSproutSpendingKey(paymentAddress2, keyOut);
     ASSERT_EQ(paymentAddress2, keyOut.address());
 }
-*/
 
+/**
+ * This test covers methods on CWalletDB to load/save crypted sapling z keys.
+ */
+TEST(wallet_zkeys_tests, WriteCryptedSaplingZkeyDirectToDb) {
+    SelectParams(CBaseChainParams::TESTNET);
+
+    // Get temporary and unique path for file.
+    // Note: / operator to append paths
+    boost::filesystem::path pathTemp = boost::filesystem::temp_directory_path() / boost::filesystem::unique_path();
+    boost::filesystem::create_directories(pathTemp);
+    mapArgs["-datadir"] = pathTemp.string();
+
+    bool fFirstRun;
+    CWallet wallet("wallet_crypted_sapling.dat");
+    ASSERT_EQ(DB_LOAD_OK, wallet.LoadWallet(fFirstRun));
+
+     // No default CPubKey set
+    ASSERT_TRUE(fFirstRun);
+
+    ASSERT_FALSE(wallet.HaveHDSeed());
+    wallet.GenerateNewSeed();
+
+    // wallet should be empty
+    std::set<libzcash::SaplingPaymentAddress> addrs;
+    wallet.GetSaplingPaymentAddresses(addrs);
+    ASSERT_EQ(0, addrs.size());
+
+    // Add random key to the wallet
+    auto address = wallet.GenerateNewSaplingZKey();
+
+    // wallet should have one key
+    wallet.GetSaplingPaymentAddresses(addrs);
+    ASSERT_EQ(1, addrs.size());
+
+    // Generate a diversified address different to the default
+    // If we can't get an early diversified address, we are very unlucky
+    libzcash::SaplingExtendedSpendingKey extsk;
+    EXPECT_TRUE(wallet.GetSaplingExtendedSpendingKey(address, extsk));
+    blob88 diversifier;
+    diversifier.begin()[0] = 10;
+    auto dpa = extsk.ToXFVK().Address(diversifier).get().second;
+
+    // Add diversified address to the wallet
+    auto ivk = extsk.expsk.full_viewing_key().in_viewing_key();
+    EXPECT_TRUE(wallet.AddSaplingIncomingViewingKey(ivk, dpa));
+
+    // encrypt wallet
+    SecureString strWalletPass;
+    strWalletPass.reserve(100);
+    strWalletPass = "hello";
+    ASSERT_TRUE(wallet.EncryptWallet(strWalletPass));
+
+    // adding a new key will fail as the wallet is locked
+    EXPECT_ANY_THROW(wallet.GenerateNewSaplingZKey());
+
+    // unlock wallet and then add
+    wallet.Unlock(strWalletPass);
+    auto address2 = wallet.GenerateNewSaplingZKey();
+
+    // Create a new wallet from the existing wallet path
+    CWallet wallet2("wallet_crypted_sapling.dat");
+    ASSERT_EQ(DB_LOAD_OK, wallet2.LoadWallet(fFirstRun));
+
+    // Confirm it's not the same as the other wallet
+    ASSERT_TRUE(&wallet != &wallet2);
+    ASSERT_TRUE(wallet2.HaveHDSeed());
+
+    // wallet should have three addresses
+    wallet2.GetSaplingPaymentAddresses(addrs);
+    ASSERT_EQ(3, addrs.size());
+
+    //check we have entries for our payment addresses
+    ASSERT_TRUE(addrs.count(address));
+    ASSERT_TRUE(addrs.count(address2));
+    ASSERT_TRUE(addrs.count(dpa));
+
+    // spending key is crypted, so we can't extract valid payment address
+    libzcash::SaplingExtendedSpendingKey keyOut;
+    EXPECT_FALSE(wallet2.GetSaplingExtendedSpendingKey(address, keyOut));
+    ASSERT_FALSE(address == keyOut.DefaultAddress());
+
+    // address -> ivk mapping is not crypted
+    libzcash::SaplingIncomingViewingKey ivkOut;
+    EXPECT_TRUE(wallet2.GetSaplingIncomingViewingKey(dpa, ivkOut));
+    EXPECT_EQ(ivk, ivkOut);
+
+    // unlock wallet to get spending keys and verify payment addresses
+    wallet2.Unlock(strWalletPass);
+
+    EXPECT_TRUE(wallet2.GetSaplingExtendedSpendingKey(address, keyOut));
+    ASSERT_EQ(address, keyOut.DefaultAddress());
+
+    EXPECT_TRUE(wallet2.GetSaplingExtendedSpendingKey(address2, keyOut));
+    ASSERT_EQ(address2, keyOut.DefaultAddress());
+}

src/wallet/wallet.cpp

@@ -177,11 +177,36 @@ bool CWallet::AddSaplingZKey(
         return true;
     }
 
-    // TODO: Persist to disk
+    if (!IsCrypted()) {
+        auto ivk = sk.expsk.full_viewing_key().in_viewing_key();
+        return CWalletDB(strWalletFile).WriteSaplingZKey(ivk, sk, mapSaplingZKeyMetadata[ivk]);
+    }
     
     return true;
 }
 
+// Add payment address -> incoming viewing key map entry
+bool CWallet::AddSaplingIncomingViewingKey(
+    const libzcash::SaplingIncomingViewingKey &ivk,
+    const libzcash::SaplingPaymentAddress &addr)
+{
+    AssertLockHeld(cs_wallet); // mapSaplingZKeyMetadata
+
+    if (!CCryptoKeyStore::AddSaplingIncomingViewingKey(ivk, addr)) {
+        return false;
+    }
+
+    if (!fFileBacked) {
+        return true;
+    }
+
+    if (!IsCrypted()) {
+        return CWalletDB(strWalletFile).WriteSaplingPaymentAddress(addr, ivk);
+    }
+
+    return true;
+}
+
 
 // Add spending key to keystore and persist to disk
 bool CWallet::AddSproutZKey(const libzcash::SproutSpendingKey &key)
@@ -304,16 +329,25 @@ bool CWallet::AddCryptedSproutSpendingKey(
     return false;
 }
 
-bool CWallet::AddCryptedSaplingSpendingKey(const libzcash::SaplingFullViewingKey &fvk,
+bool CWallet::AddCryptedSaplingSpendingKey(const libzcash::SaplingExtendedFullViewingKey &extfvk,
                                            const std::vector<unsigned char> &vchCryptedSecret,
                                            const libzcash::SaplingPaymentAddress &defaultAddr)
 {
-    if (!CCryptoKeyStore::AddCryptedSaplingSpendingKey(fvk, vchCryptedSecret, defaultAddr))
+    if (!CCryptoKeyStore::AddCryptedSaplingSpendingKey(extfvk, vchCryptedSecret, defaultAddr))
         return false;
     if (!fFileBacked)
         return true;
     {
-        // TODO: Sapling - Write to disk
+        LOCK(cs_wallet);
+        if (pwalletdbEncryption) {
+            return pwalletdbEncryption->WriteCryptedSaplingZKey(extfvk,
+                                                         vchCryptedSecret,
+                                                         mapSaplingZKeyMetadata[extfvk.fvk.in_viewing_key()]);
+        } else {
+            return CWalletDB(strWalletFile).WriteCryptedSaplingZKey(extfvk,
+                                                         vchCryptedSecret,
+                                                         mapSaplingZKeyMetadata[extfvk.fvk.in_viewing_key()]);
+        }
     }
     return false;
 }
@@ -345,6 +379,32 @@ bool CWallet::LoadCryptedZKey(const libzcash::SproutPaymentAddress &addr, const
     return CCryptoKeyStore::AddCryptedSproutSpendingKey(addr, rk, vchCryptedSecret);
 }
 
+bool CWallet::LoadCryptedSaplingZKey(
+    const libzcash::SaplingExtendedFullViewingKey &extfvk,
+    const std::vector<unsigned char> &vchCryptedSecret)
+{
+     return CCryptoKeyStore::AddCryptedSaplingSpendingKey(extfvk, vchCryptedSecret, extfvk.DefaultAddress());
+}
+
+bool CWallet::LoadSaplingZKeyMetadata(const libzcash::SaplingIncomingViewingKey &ivk, const CKeyMetadata &meta)
+{
+    AssertLockHeld(cs_wallet); // mapSaplingZKeyMetadata
+    mapSaplingZKeyMetadata[ivk] = meta;
+    return true;
+}
+
+bool CWallet::LoadSaplingZKey(const libzcash::SaplingExtendedSpendingKey &key)
+{
+    return CCryptoKeyStore::AddSaplingSpendingKey(key, key.DefaultAddress());
+}
+
+bool CWallet::LoadSaplingPaymentAddress(
+    const libzcash::SaplingPaymentAddress &addr,
+    const libzcash::SaplingIncomingViewingKey &ivk)
+{
+    return CCryptoKeyStore::AddSaplingIncomingViewingKey(ivk, addr);
+}
+
 bool CWallet::LoadZKey(const libzcash::SproutSpendingKey &key)
 {
     return CCryptoKeyStore::AddSproutSpendingKey(key);
@@ -2273,14 +2333,12 @@ bool CWallet::SetHDSeed(const HDSeed& seed)
         return true;
     }
 
-    /* TODO: Uncomment during PR for #3388
     {
         LOCK(cs_wallet);
         if (!IsCrypted()) {
             return CWalletDB(strWalletFile).WriteHDSeed(seed);
         }
     }
-    */
     return true;
 }
 
@@ -2294,7 +2352,6 @@ bool CWallet::SetCryptedHDSeed(const uint256& seedFp, const std::vector<unsigned
         return true;
     }
 
-    /* TODO: Uncomment during PR for #3388
     {
         LOCK(cs_wallet);
         if (pwalletdbEncryption)
@@ -2302,17 +2359,14 @@ bool CWallet::SetCryptedHDSeed(const uint256& seedFp, const std::vector<unsigned
         else
             return CWalletDB(strWalletFile).WriteCryptedHDSeed(seedFp, vchCryptedSecret);
     }
-    */
     return false;
 }
 
 void CWallet::SetHDChain(const CHDChain& chain, bool memonly)
 {
     LOCK(cs_wallet);
-    /* TODO: Uncomment during PR for #3388
     if (!memonly && fFileBacked && !CWalletDB(strWalletFile).WriteHDChain(chain))
         throw std::runtime_error(std::string(__func__) + ": writing chain failed");
-    */
 
     hdChain = chain;
 }

src/wallet/wallet.h

@@ -284,6 +284,20 @@ public:
     libzcash::SaplingIncomingViewingKey ivk;
     boost::optional<uint256> nullifier;
 
+    ADD_SERIALIZE_METHODS;
+
+    template <typename Stream, typename Operation>
+    inline void SerializationOp(Stream& s, Operation ser_action) {
+        int nVersion = s.GetVersion();
+        if (!(s.GetType() & SER_GETHASH)) {
+            READWRITE(nVersion);
+        }
+        READWRITE(ivk);
+        READWRITE(nullifier);
+        READWRITE(witnesses);
+        READWRITE(witnessHeight);
+    }
+
     friend bool operator==(const SaplingNoteData& a, const SaplingNoteData& b) {
         return (a.ivk == b.ivk && a.nullifier == b.nullifier && a.witnessHeight == b.witnessHeight);
     }
@@ -510,8 +524,10 @@ public:
         READWRITE(nTimeReceived);
         READWRITE(fFromMe);
         READWRITE(fSpent);
-        // TODO:
+
-        //READWRITE(mapSaplingNoteData);
+        if (fOverwintered && nVersion >= SAPLING_TX_VERSION) {
+            READWRITE(mapSaplingNoteData);
+        }
 
         if (ser_action.ForRead())
         {
@@ -1060,10 +1076,25 @@ public:
     bool AddSaplingZKey(
         const libzcash::SaplingExtendedSpendingKey &key,
         const libzcash::SaplingPaymentAddress &defaultAddr);
+    bool AddSaplingIncomingViewingKey(
+        const libzcash::SaplingIncomingViewingKey &ivk,
+        const libzcash::SaplingPaymentAddress &addr);
     bool AddCryptedSaplingSpendingKey(
-        const libzcash::SaplingFullViewingKey &fvk,
+        const libzcash::SaplingExtendedFullViewingKey &extfvk,
         const std::vector<unsigned char> &vchCryptedSecret,
         const libzcash::SaplingPaymentAddress &defaultAddr);
+    //! Adds spending key to the store, without saving it to disk (used by LoadWallet)
+    bool LoadSaplingZKey(const libzcash::SaplingExtendedSpendingKey &key);
+    //! Load spending key metadata (used by LoadWallet)
+    bool LoadSaplingZKeyMetadata(const libzcash::SaplingIncomingViewingKey &ivk, const CKeyMetadata &meta);
+    //! Adds a Sapling payment address -> incoming viewing key map entry,
+    //! without saving it to disk (used by LoadWallet)
+    bool LoadSaplingPaymentAddress(
+        const libzcash::SaplingPaymentAddress &addr,
+        const libzcash::SaplingIncomingViewingKey &ivk);
+    //! Adds an encrypted spending key to the store, without saving it to disk (used by LoadWallet)
+    bool LoadCryptedSaplingZKey(const libzcash::SaplingExtendedFullViewingKey &extfvk,
+                                const std::vector<unsigned char> &vchCryptedSecret);
 
     /**
      * Increment the next transaction order id

src/wallet/walletdb.cpp

@@ -125,6 +125,28 @@ bool CWalletDB::WriteCryptedZKey(const libzcash::SproutPaymentAddress & addr,
     return true;
 }
 
+bool CWalletDB::WriteCryptedSaplingZKey(
+    const libzcash::SaplingExtendedFullViewingKey &extfvk,
+    const std::vector<unsigned char>& vchCryptedSecret,
+    const CKeyMetadata &keyMeta)
+{
+    const bool fEraseUnencryptedKey = true;
+    nWalletDBUpdated++;
+    auto ivk = extfvk.fvk.in_viewing_key();
+
+    if (!Write(std::make_pair(std::string("sapzkeymeta"), ivk), keyMeta))
+        return false;
+
+    if (!Write(std::make_pair(std::string("csapzkey"), ivk), std::make_pair(extfvk, vchCryptedSecret), false))
+        return false;
+
+    if (fEraseUnencryptedKey)
+    {
+        Erase(std::make_pair(std::string("sapzkey"), ivk));
+    }
+    return true;
+}
+
 bool CWalletDB::WriteMasterKey(unsigned int nID, const CMasterKey& kMasterKey)
 {
     nWalletDBUpdated++;
@@ -141,6 +163,26 @@ bool CWalletDB::WriteZKey(const libzcash::SproutPaymentAddress& addr, const libz
     // pair is: tuple_key("zkey", paymentaddress) --> secretkey
     return Write(std::make_pair(std::string("zkey"), addr), key, false);
 }
+bool CWalletDB::WriteSaplingZKey(const libzcash::SaplingIncomingViewingKey &ivk,
+                const libzcash::SaplingExtendedSpendingKey &key,
+                const CKeyMetadata &keyMeta)
+{
+    nWalletDBUpdated++;
+
+    if (!Write(std::make_pair(std::string("sapzkeymeta"), ivk), keyMeta))
+        return false;
+
+    return Write(std::make_pair(std::string("sapzkey"), ivk), key, false);
+}
+
+bool CWalletDB::WriteSaplingPaymentAddress(
+    const libzcash::SaplingPaymentAddress &addr,
+    const libzcash::SaplingIncomingViewingKey &ivk)
+{
+    nWalletDBUpdated++;
+
+    return Write(std::make_pair(std::string("sapzaddr"), addr), ivk, false);
+}
 
 bool CWalletDB::WriteSproutViewingKey(const libzcash::SproutViewingKey &vk)
 {
@@ -383,13 +425,14 @@ public:
     unsigned int nZKeys;
     unsigned int nCZKeys;
     unsigned int nZKeyMeta;
+    unsigned int nSapZAddrs;
     bool fIsEncrypted;
     bool fAnyUnordered;
     int nFileVersion;
     vector<uint256> vWalletUpgrade;
 
     CWalletScanState() {
-        nKeys = nCKeys = nKeyMeta = nZKeys = nCZKeys = nZKeyMeta = 0;
+        nKeys = nCKeys = nKeyMeta = nZKeys = nCZKeys = nZKeyMeta = nSapZAddrs = 0;
         fIsEncrypted = false;
         fAnyUnordered = false;
         nFileVersion = 0;
@@ -511,6 +554,23 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
 
             wss.nZKeys++;
         }
+        else if (strType == "sapzkey")
+        {
+            libzcash::SaplingIncomingViewingKey ivk;
+            ssKey >> ivk;
+            libzcash::SaplingExtendedSpendingKey key;
+            ssValue >> key;
+
+            if (!pwallet->LoadSaplingZKey(key))
+            {
+                strErr = "Error reading wallet database: LoadSaplingZKey failed";
+                return false;
+            }
+
+            //add checks for integrity
+            wss.nZKeys++;
+        }
+
         else if (strType == "key" || strType == "wkey")
         {
             CPubKey vchPubKey;
@@ -624,6 +684,23 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
             }
             wss.fIsEncrypted = true;
         }
+        else if (strType == "csapzkey")
+        {
+            libzcash::SaplingIncomingViewingKey ivk;
+            ssKey >> ivk;
+            libzcash::SaplingExtendedFullViewingKey extfvk;
+            ssValue >> extfvk;
+            vector<unsigned char> vchCryptedSecret;
+            ssValue >> vchCryptedSecret;
+            wss.nCKeys++;
+
+            if (!pwallet->LoadCryptedSaplingZKey(extfvk, vchCryptedSecret))
+            {
+                strErr = "Error reading wallet database: LoadCryptedSaplingZKey failed";
+                return false;
+            }
+            wss.fIsEncrypted = true;
+        }
         else if (strType == "keymeta")
         {
             CPubKey vchPubKey;
@@ -651,6 +728,32 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
 
             // ignore earliest key creation time as taddr will exist before any zaddr
         }
+        else if (strType == "sapzkeymeta")
+        {
+            libzcash::SaplingIncomingViewingKey ivk;
+            ssKey >> ivk;
+            CKeyMetadata keyMeta;
+            ssValue >> keyMeta;
+
+            wss.nZKeyMeta++;
+
+            pwallet->LoadSaplingZKeyMetadata(ivk, keyMeta);
+        }
+        else if (strType == "sapzaddr")
+        {
+            libzcash::SaplingPaymentAddress addr;
+            ssKey >> addr;
+            libzcash::SaplingIncomingViewingKey ivk;
+            ssValue >> ivk;
+
+            wss.nSapZAddrs++;
+
+            if (!pwallet->LoadSaplingPaymentAddress(addr, ivk))
+            {
+                strErr = "Error reading wallet database: LoadSaplingPaymentAddress failed";
+                return false;
+            }
+        }
         else if (strType == "defaultkey")
         {
             ssValue >> pwallet->vchDefaultKey;
@@ -736,7 +839,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
             ssValue >> vchCryptedSecret;
             if (!pwallet->LoadCryptedHDSeed(seedFp, vchCryptedSecret))
             {
-                strErr = "Error reading wallet database: LoadCryptedSeed failed";
+                strErr = "Error reading wallet database: LoadCryptedHDSeed failed";
                 return false;
             }
             wss.fIsEncrypted = true;
@@ -759,6 +862,7 @@ static bool IsKeyType(string strType)
     return (strType== "key" || strType == "wkey" ||
             strType == "hdseed" || strType == "chdseed" ||
             strType == "zkey" || strType == "czkey" ||
+            strType == "sapzkey" || strType == "csapzkey" ||
             strType == "vkey" ||
             strType == "mkey" || strType == "ckey");
 }
@@ -911,11 +1015,20 @@ DBErrors CWalletDB::FindWalletTx(CWallet* pwallet, vector<uint256>& vTxHash, vec
                 uint256 hash;
                 ssKey >> hash;
 
-                CWalletTx wtx;
+                std::vector<unsigned char> txData(ssValue.begin(), ssValue.end());
-                ssValue >> wtx;
+                try {
+                    CWalletTx wtx;
+                    ssValue >> wtx;
+                    vWtx.push_back(wtx);
+                } catch (...) {
+                    // Decode failure likely due to Sapling v4 transaction format change
+                    // between 2.0.0 and 2.0.1. As user is requesting deletion, log the
+                    // transaction entry and then mark it for deletion anyway.
+                    LogPrintf("Failed to decode wallet transaction; logging it here before deletion:\n");
+                    LogPrintf("txid: %s\n%s\n", hash.GetHex(), HexStr(txData));
+                }
 
                 vTxHash.push_back(hash);
-                vWtx.push_back(wtx);
             }
         }
         pcursor->close();

src/wallet/walletdb.h

@@ -184,10 +184,18 @@ public:
 
     /// Write spending key to wallet database, where key is payment address and value is spending key.
     bool WriteZKey(const libzcash::SproutPaymentAddress& addr, const libzcash::SproutSpendingKey& key, const CKeyMetadata &keyMeta);
+    bool WriteSaplingZKey(const libzcash::SaplingIncomingViewingKey &ivk,
+                          const libzcash::SaplingExtendedSpendingKey &key,
+                          const CKeyMetadata  &keyMeta);
+    bool WriteSaplingPaymentAddress(const libzcash::SaplingPaymentAddress &addr,
+                                    const libzcash::SaplingIncomingViewingKey &ivk);
     bool WriteCryptedZKey(const libzcash::SproutPaymentAddress & addr,
                           const libzcash::ReceivingKey & rk,
                           const std::vector<unsigned char>& vchCryptedSecret,
                           const CKeyMetadata &keyMeta);
+    bool WriteCryptedSaplingZKey(const libzcash::SaplingExtendedFullViewingKey &extfvk,
+                          const std::vector<unsigned char>& vchCryptedSecret,
+                          const CKeyMetadata &keyMeta);
 
     bool WriteSproutViewingKey(const libzcash::SproutViewingKey &vk);
     bool EraseSproutViewingKey(const libzcash::SproutViewingKey &vk);

src/zcash/zip32.h

@@ -78,6 +78,21 @@ struct SaplingExtendedFullViewingKey {
         Address(diversifier_index_t j) const;
 
     libzcash::SaplingPaymentAddress DefaultAddress() const;
+
+    friend inline bool operator==(const SaplingExtendedFullViewingKey& a, const SaplingExtendedFullViewingKey& b) {
+        return (
+            a.depth == b.depth &&
+            a.parentFVKTag == b.parentFVKTag &&
+            a.childIndex == b.childIndex &&
+            a.chaincode == b.chaincode &&
+            a.fvk == b.fvk &&
+            a.dk == b.dk);
+    }
+    friend inline bool operator<(const SaplingExtendedFullViewingKey& a, const SaplingExtendedFullViewingKey& b) {
+        return (a.depth < b.depth ||
+            (a.depth == b.depth && a.childIndex < b.childIndex) ||
+            (a.depth == b.depth && a.childIndex == b.childIndex && a.fvk < b.fvk));
+    }
 };
 
 struct SaplingExtendedSpendingKey {