From 87ae893d7570192a67afa0cd26ca2b324fd74378 Mon Sep 17 00:00:00 2001 From: mdr0id Date: Wed, 19 Sep 2018 19:40:51 -0700 Subject: [PATCH 01/12] Revert "wallet: Comment out HDSeed and CHDChain persistence to disk" This reverts commit b7f9a7ae02ffcd4bce3df9e7c9e231b0d64ed0d4. --- qa/pull-tester/rpc-tests.sh | 4 ++-- src/test/rpc_wallet_tests.cpp | 2 -- src/wallet/gtest/test_wallet_zkeys.cpp | 2 -- src/wallet/wallet.cpp | 6 ------ 4 files changed, 2 insertions(+), 12 deletions(-) diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index 50244440e..cbc711844 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -23,7 +23,7 @@ testScripts=( 'wallet_mergetoaddress.py' 'wallet.py' 'wallet_overwintertx.py' -# 'wallet_nullifiers.py' + 'wallet_nullifiers.py' 'wallet_1941.py' 'wallet_addresses.py' 'wallet_sapling.py' @@ -44,7 +44,7 @@ testScripts=( 'zapwallettxes.py' 'proxy_test.py' 'merkle_blocks.py' -# 'fundrawtransaction.py' + 'fundrawtransaction.py' 'signrawtransactions.py' 'signrawtransaction_offline.py' 'walletbackup.py' diff --git a/src/test/rpc_wallet_tests.cpp b/src/test/rpc_wallet_tests.cpp index 811b98bab..1ce4ea71a 100644 --- a/src/test/rpc_wallet_tests.cpp +++ b/src/test/rpc_wallet_tests.cpp @@ -1356,7 +1356,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); @@ -1412,7 +1411,6 @@ 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_z_listunspent_parameters) diff --git a/src/wallet/gtest/test_wallet_zkeys.cpp b/src/wallet/gtest/test_wallet_zkeys.cpp index e4a8eeeb4..85f74ab7b 100644 --- a/src/wallet/gtest/test_wallet_zkeys.cpp +++ b/src/wallet/gtest/test_wallet_zkeys.cpp @@ -288,7 +288,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 +362,4 @@ TEST(wallet_zkeys_tests, write_cryptedzkey_direct_to_db) { wallet2.GetSproutSpendingKey(paymentAddress2, keyOut); ASSERT_EQ(paymentAddress2, keyOut.address()); } -*/ diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d6eea3c13..b53691fbb 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1991,14 +1991,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; } @@ -2012,7 +2010,6 @@ bool CWallet::SetCryptedHDSeed(const uint256& seedFp, const std::vector Date: Fri, 5 Oct 2018 23:13:32 +0100 Subject: [PATCH 02/12] Store ExtFVK with encrypted Sapling spending key instead of FVK This ensures that even when the wallet is encrypted, we can derive the default Sapling payment address for our spending keys. --- src/keystore.h | 2 +- src/wallet/crypter.cpp | 38 +++++++++++++++++++------------------- src/wallet/crypter.h | 8 ++++++-- src/wallet/wallet.cpp | 4 ++-- src/wallet/wallet.h | 2 +- src/zcash/zip32.h | 15 +++++++++++++++ 6 files changed, 44 insertions(+), 25 deletions(-) diff --git a/src/keystore.h b/src/keystore.h index b369dec78..6ff34b20d 100644 --- a/src/keystore.h +++ b/src/keystore.h @@ -311,6 +311,6 @@ typedef std::map > > Crypt typedef std::map > CryptedSproutSpendingKeyMap; //! Sapling -typedef std::map > CryptedSaplingSpendingKeyMap; +typedef std::map > CryptedSaplingSpendingKeyMap; #endif // BITCOIN_KEYSTORE_H diff --git a/src/wallet/crypter.cpp b/src/wallet/crypter.cpp index 25d69cc3d..cf87c01d4 100644 --- a/src/wallet/crypter.cpp +++ b/src/wallet/crypter.cpp @@ -170,11 +170,11 @@ static bool DecryptSproutSpendingKey(const CKeyingMaterial& vMasterKey, static bool DecryptSaplingSpendingKey(const CKeyingMaterial& vMasterKey, const std::vector& 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) @@ -182,7 +182,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() @@ -261,10 +261,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 &vchCryptedSecret = (*miSapling).second; libzcash::SaplingExtendedSpendingKey sk; - if (!DecryptSaplingSpendingKey(vMasterKeyIn, vchCryptedSecret, fvk, sk)) + if (!DecryptSaplingSpendingKey(vMasterKeyIn, vchCryptedSecret, extfvk, sk)) { keyFail = true; break; @@ -465,12 +465,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(); - if (!EncryptSecret(vMasterKey, vchSecret, fvk.GetFingerprint(), vchCryptedSecret)) { + auto extfvk = sk.ToXFVK(); + if (!EncryptSecret(vMasterKey, vchSecret, extfvk.fvk.GetFingerprint(), vchCryptedSecret)) { return false; } - if (!AddCryptedSaplingSpendingKey(fvk, vchCryptedSecret, defaultAddr)) { + if (!AddCryptedSaplingSpendingKey(extfvk, vchCryptedSecret, defaultAddr)) { return false; } } @@ -494,7 +494,7 @@ bool CCryptoKeyStore::AddCryptedSproutSpendingKey( } bool CCryptoKeyStore::AddCryptedSaplingSpendingKey( - const libzcash::SaplingFullViewingKey &fvk, + const libzcash::SaplingExtendedFullViewingKey &extfvk, const std::vector &vchCryptedSecret, const libzcash::SaplingPaymentAddress &defaultAddr) { @@ -505,11 +505,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; } @@ -538,11 +538,11 @@ bool CCryptoKeyStore::GetSaplingSpendingKey(const libzcash::SaplingFullViewingKe if (!IsCrypted()) return CBasicKeyStore::GetSaplingSpendingKey(fvk, skOut); - CryptedSaplingSpendingKeyMap::const_iterator mi = mapCryptedSaplingSpendingKeys.find(fvk); - if (mi != mapCryptedSaplingSpendingKeys.end()) - { - const std::vector &vchCryptedSecret = (*mi).second; - return DecryptSaplingSpendingKey(vMasterKey, vchCryptedSecret, fvk, skOut); + for (auto entry : mapCryptedSaplingSpendingKeys) { + if (entry.first.fvk == fvk) { + const std::vector &vchCryptedSecret = entry.second; + return DecryptSaplingSpendingKey(vMasterKey, vchCryptedSecret, entry.first, skOut); + } } } return false; @@ -609,12 +609,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 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; } } diff --git a/src/wallet/crypter.h b/src/wallet/crypter.h index b751ce300..1cfefe886 100644 --- a/src/wallet/crypter.h +++ b/src/wallet/crypter.h @@ -241,7 +241,7 @@ public: } //! Sapling virtual bool AddCryptedSaplingSpendingKey( - const libzcash::SaplingFullViewingKey &fvk, + const libzcash::SaplingExtendedFullViewingKey &extfvk, const std::vector &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; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b53691fbb..09a9d15d5 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -293,11 +293,11 @@ bool CWallet::AddCryptedSproutSpendingKey( return false; } -bool CWallet::AddCryptedSaplingSpendingKey(const libzcash::SaplingFullViewingKey &fvk, +bool CWallet::AddCryptedSaplingSpendingKey(const libzcash::SaplingExtendedFullViewingKey &extfvk, const std::vector &vchCryptedSecret, const libzcash::SaplingPaymentAddress &defaultAddr) { - if (!CCryptoKeyStore::AddCryptedSaplingSpendingKey(fvk, vchCryptedSecret, defaultAddr)) + if (!CCryptoKeyStore::AddCryptedSaplingSpendingKey(extfvk, vchCryptedSecret, defaultAddr)) return false; if (!fFileBacked) return true; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index a08a8c782..95cd7b63e 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1060,7 +1060,7 @@ public: const libzcash::SaplingExtendedSpendingKey &key, const libzcash::SaplingPaymentAddress &defaultAddr); bool AddCryptedSaplingSpendingKey( - const libzcash::SaplingFullViewingKey &fvk, + const libzcash::SaplingExtendedFullViewingKey &extfvk, const std::vector &vchCryptedSecret, const libzcash::SaplingPaymentAddress &defaultAddr); diff --git a/src/zcash/zip32.h b/src/zcash/zip32.h index 0fc5785bd..44bc58598 100644 --- a/src/zcash/zip32.h +++ b/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 { From 2fcf06077ff44b112a92505c33ee17766940b521 Mon Sep 17 00:00:00 2001 From: mdr0id Date: Thu, 13 Sep 2018 14:22:00 -0700 Subject: [PATCH 03/12] Persist Sapling key material in the wallet to disk --- qa/pull-tester/rpc-tests.sh | 1 + qa/rpc-tests/wallet_persistence.py | 31 ++++++++++++ src/wallet/wallet.cpp | 35 ++++++++++++- src/wallet/wallet.h | 7 +++ src/wallet/walletdb.cpp | 81 +++++++++++++++++++++++++++++- src/wallet/walletdb.h | 6 +++ 6 files changed, 158 insertions(+), 3 deletions(-) create mode 100644 qa/rpc-tests/wallet_persistence.py diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index cbc711844..0819a89ad 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -23,6 +23,7 @@ testScripts=( 'wallet_mergetoaddress.py' 'wallet.py' 'wallet_overwintertx.py' + 'wallet_persistence.py' 'wallet_nullifiers.py' 'wallet_1941.py' 'wallet_addresses.py' diff --git a/qa/rpc-tests/wallet_persistence.py b/qa/rpc-tests/wallet_persistence.py new file mode 100644 index 000000000..364e7b450 --- /dev/null +++ b/qa/rpc-tests/wallet_persistence.py @@ -0,0 +1,31 @@ +#!/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_true, start_nodes, stop_nodes, \ + wait_bitcoinds + +class WalletPersistenceTest (BitcoinTestFramework): + + def setup_network(self, split=False): + self.nodes = start_nodes(1, self.options.tmpdir) + self.is_network_split = False + self.sync_all() + + def run_test(self): + sapling_addr = self.nodes[0].z_getnewaddress('sapling') + addresses = self.nodes[0].z_listaddresses() + # make sure the node has the addresss + assert_true(sapling_addr in addresses, "Should contain address before restart") + # restart the nodes + stop_nodes(self.nodes) + wait_bitcoinds() + self.nodes = self.setup_nodes() + addresses = self.nodes[0].z_listaddresses() + # make sure we still have the address after restarting + assert_true(sapling_addr in addresses, "Should contain address after restart") + +if __name__ == '__main__': + WalletPersistenceTest().main() \ No newline at end of file diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 09a9d15d5..ee242c233 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -166,7 +166,10 @@ 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; } @@ -302,7 +305,16 @@ bool CWallet::AddCryptedSaplingSpendingKey(const libzcash::SaplingExtendedFullVi 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; } @@ -334,6 +346,25 @@ bool CWallet::LoadCryptedZKey(const libzcash::SproutPaymentAddress &addr, const return CCryptoKeyStore::AddCryptedSproutSpendingKey(addr, rk, vchCryptedSecret); } +bool CWallet::LoadCryptedSaplingZKey( + const libzcash::SaplingExtendedFullViewingKey &extfvk, + const std::vector &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::LoadZKey(const libzcash::SproutSpendingKey &key) { return CCryptoKeyStore::AddSproutSpendingKey(key); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 95cd7b63e..642898827 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1063,6 +1063,13 @@ public: const libzcash::SaplingExtendedFullViewingKey &extfvk, const std::vector &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 an encrypted spending key to the store, without saving it to disk (used by LoadWallet) + bool LoadCryptedSaplingZKey(const libzcash::SaplingExtendedFullViewingKey &extfvk, + const std::vector &vchCryptedSecret); /** * Increment the next transaction order id diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 699d71946..9bab20858 100644 --- a/src/wallet/walletdb.cpp +++ b/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& 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,17 @@ 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::WriteSproutViewingKey(const libzcash::SproutViewingKey &vk) { @@ -511,6 +544,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 +674,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 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 +718,17 @@ 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 == "defaultkey") { ssValue >> pwallet->vchDefaultKey; @@ -736,7 +814,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 +837,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"); } diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index dc58adb65..a009b1ab7 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -184,10 +184,16 @@ 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 WriteCryptedZKey(const libzcash::SproutPaymentAddress & addr, const libzcash::ReceivingKey & rk, const std::vector& vchCryptedSecret, const CKeyMetadata &keyMeta); + bool WriteCryptedSaplingZKey(const libzcash::SaplingExtendedFullViewingKey &extfvk, + const std::vector& vchCryptedSecret, + const CKeyMetadata &keyMeta); bool WriteSproutViewingKey(const libzcash::SproutViewingKey &vk); bool EraseSproutViewingKey(const libzcash::SproutViewingKey &vk); From 48a93fd2e099c15be4169fc883eb3b2de4cb5325 Mon Sep 17 00:00:00 2001 From: mdr0id Date: Mon, 17 Sep 2018 12:41:26 -0700 Subject: [PATCH 04/12] Serialize Sapling data in CWalletTx If 2.0.0 nodes upgrade to 2.0.1 after Sapling has activated, the v4 Sapling transactions in their wallet will be treated as corrupt, and a rescan will be triggered which will overwrite the old-format transactions with the new Sapling-aware format. --- src/wallet/wallet.h | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 642898827..1b11b6349 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -283,6 +283,20 @@ public: libzcash::SaplingIncomingViewingKey ivk; boost::optional nullifier; + ADD_SERIALIZE_METHODS; + + template + 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); } @@ -509,8 +523,10 @@ public: READWRITE(nTimeReceived); READWRITE(fFromMe); READWRITE(fSpent); - // TODO: - //READWRITE(mapSaplingNoteData); + + if (fOverwintered && nVersion >= SAPLING_TX_VERSION) { + READWRITE(mapSaplingNoteData); + } if (ser_action.ForRead()) { From fe92bc700a08797992e712cc3b2ad3bbe0db9a12 Mon Sep 17 00:00:00 2001 From: mdr0id Date: Thu, 27 Sep 2018 13:06:33 -0700 Subject: [PATCH 05/12] Adding in rpc wallet sap for test_bitcoin --- src/test/rpc_wallet_tests.cpp | 61 +++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/test/rpc_wallet_tests.cpp b/src/test/rpc_wallet_tests.cpp index 1ce4ea71a..850701eb3 100644 --- a/src/test/rpc_wallet_tests.cpp +++ b/src/test/rpc_wallet_tests.cpp @@ -1412,6 +1412,67 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_encrypted_wallet_zkeys) // 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 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) { From 9ce6f8425bb301c97cbd7d469786829d01a8c2b2 Mon Sep 17 00:00:00 2001 From: mdr0id Date: Wed, 3 Oct 2018 10:16:30 -0700 Subject: [PATCH 06/12] Add gtest coverage of Sapling wallet persistence --- src/wallet/gtest/test_wallet_zkeys.cpp | 95 +++++++++++++++++++++++++- 1 file changed, 94 insertions(+), 1 deletion(-) diff --git a/src/wallet/gtest/test_wallet_zkeys.cpp b/src/wallet/gtest/test_wallet_zkeys.cpp index 85f74ab7b..ebd6c3bf0 100644 --- a/src/wallet/gtest/test_wallet_zkeys.cpp +++ b/src/wallet/gtest/test_wallet_zkeys.cpp @@ -10,8 +10,11 @@ /** * This test covers Sapling methods on CWallet * GenerateNewSaplingZKey() + * AddSaplingZKey() + * LoadSaplingZKey() + * LoadSaplingZKeyMetadata() */ -TEST(wallet_zkeys_tests, store_and_load_sapling_zkeys) { +TEST(wallet_zkeys_tests, StoreAndLoadSaplingZkeys) { SelectParams(CBaseChainParams::MAIN); CWallet wallet; @@ -58,6 +61,19 @@ 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())); + + // 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); } /** @@ -363,3 +379,80 @@ TEST(wallet_zkeys_tests, write_cryptedzkey_direct_to_db) { 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 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()); + + // 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 two keys + wallet2.GetSaplingPaymentAddresses(addrs); + ASSERT_EQ(2, addrs.size()); + + //check we have entries for our payment addresses + ASSERT_TRUE(addrs.count(address)); + ASSERT_TRUE(addrs.count(address2)); + + // 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()); + + // 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()); +} From 3e471410f11029b2161006656464a95d818acd19 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 6 Oct 2018 00:45:39 +0100 Subject: [PATCH 07/12] Persist Sapling payment address to IVK map This ensures we remember any diversified addresses manually generated outside the wallet. --- src/wallet/gtest/test_wallet_zkeys.cpp | 49 ++++++++++++++++++++++++-- src/wallet/wallet.cpp | 29 +++++++++++++++ src/wallet/wallet.h | 8 +++++ src/wallet/walletdb.cpp | 27 +++++++++++++- src/wallet/walletdb.h | 2 ++ 5 files changed, 112 insertions(+), 3 deletions(-) diff --git a/src/wallet/gtest/test_wallet_zkeys.cpp b/src/wallet/gtest/test_wallet_zkeys.cpp index ebd6c3bf0..0dcec1077 100644 --- a/src/wallet/gtest/test_wallet_zkeys.cpp +++ b/src/wallet/gtest/test_wallet_zkeys.cpp @@ -11,7 +11,9 @@ * This test covers Sapling methods on CWallet * GenerateNewSaplingZKey() * AddSaplingZKey() + * AddSaplingIncomingViewingKey() * LoadSaplingZKey() + * LoadSaplingIncomingViewingKey() * LoadSaplingZKeyMetadata() */ TEST(wallet_zkeys_tests, StoreAndLoadSaplingZkeys) { @@ -62,6 +64,24 @@ TEST(wallet_zkeys_tests, StoreAndLoadSaplingZkeys) { 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)); @@ -74,6 +94,13 @@ TEST(wallet_zkeys_tests, StoreAndLoadSaplingZkeys) { // 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)); } /** @@ -413,6 +440,18 @@ TEST(wallet_zkeys_tests, WriteCryptedSaplingZkeyDirectToDb) { 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); @@ -434,19 +473,25 @@ TEST(wallet_zkeys_tests, WriteCryptedSaplingZkeyDirectToDb) { ASSERT_TRUE(&wallet != &wallet2); ASSERT_TRUE(wallet2.HaveHDSeed()); - // wallet should have two keys + // wallet should have three addresses wallet2.GetSaplingPaymentAddresses(addrs); - ASSERT_EQ(2, addrs.size()); + 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); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ee242c233..3690ed166 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -174,6 +174,28 @@ bool CWallet::AddSaplingZKey( 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) @@ -365,6 +387,13 @@ 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); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 1b11b6349..e7d30f467 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1075,6 +1075,9 @@ 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::SaplingExtendedFullViewingKey &extfvk, const std::vector &vchCryptedSecret, @@ -1083,6 +1086,11 @@ public: 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 &vchCryptedSecret); diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 9bab20858..9a878491c 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -175,6 +175,15 @@ bool CWalletDB::WriteSaplingZKey(const libzcash::SaplingIncomingViewingKey &ivk, 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) { nWalletDBUpdated++; @@ -416,13 +425,14 @@ public: unsigned int nZKeys; unsigned int nCZKeys; unsigned int nZKeyMeta; + unsigned int nSapZAddrs; bool fIsEncrypted; bool fAnyUnordered; int nFileVersion; vector vWalletUpgrade; CWalletScanState() { - nKeys = nCKeys = nKeyMeta = nZKeys = nCZKeys = nZKeyMeta = 0; + nKeys = nCKeys = nKeyMeta = nZKeys = nCZKeys = nZKeyMeta = nSapZAddrs = 0; fIsEncrypted = false; fAnyUnordered = false; nFileVersion = 0; @@ -729,6 +739,21 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, 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; diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index a009b1ab7..b3210bbc0 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -187,6 +187,8 @@ public: 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& vchCryptedSecret, From cb9cff5fdc1d13851b791923b91efe2aae561e88 Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 5 Oct 2018 21:15:08 -0700 Subject: [PATCH 08/12] Fix deadlock from calling CWallet::AddSaplingIncomingViewingKey instead of CBasicKeyStore::AddSaplingIncomingViewingKey --- src/keystore.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/keystore.cpp b/src/keystore.cpp index 54fb02590..e1e3ae89b 100644 --- a/src/keystore.cpp +++ b/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. From 5513faccf66f99f073dfc17b8f8cf6649b8acf2d Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 6 Oct 2018 12:18:56 +0100 Subject: [PATCH 09/12] Ignore decoding errors during -zapwallettxes The undecoded wallet transaction is logged before proceeding, so later recovery of metadata might be possible. But the fact that the user is using -zapwallettxes is a clear indicator that they want transactions removed from their wallet, so this is the priority. --- src/wallet/walletdb.cpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 9a878491c..1081e9d09 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -1015,11 +1015,20 @@ DBErrors CWalletDB::FindWalletTx(CWallet* pwallet, vector& vTxHash, vec uint256 hash; ssKey >> hash; - CWalletTx wtx; - ssValue >> wtx; + std::vector txData(ssValue.begin(), ssValue.end()); + 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(); From 5537bf5cdb6346d8b1eee7a376cde0cf7b3cfa26 Mon Sep 17 00:00:00 2001 From: Simon Date: Sat, 6 Oct 2018 21:05:30 -0700 Subject: [PATCH 10/12] Fix file permissions of QA test wallet_persistence.py --- qa/rpc-tests/wallet_persistence.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 qa/rpc-tests/wallet_persistence.py diff --git a/qa/rpc-tests/wallet_persistence.py b/qa/rpc-tests/wallet_persistence.py old mode 100644 new mode 100755 From 08f1baaaca612645312e95c3b542bacc98f90b72 Mon Sep 17 00:00:00 2001 From: Simon Date: Sat, 6 Oct 2018 22:53:24 -0700 Subject: [PATCH 11/12] Update wallet_persistence test to verify wallet txs are persisted across restarts. --- qa/rpc-tests/wallet_persistence.py | 81 +++++++++++++++++++++++++++--- 1 file changed, 73 insertions(+), 8 deletions(-) diff --git a/qa/rpc-tests/wallet_persistence.py b/qa/rpc-tests/wallet_persistence.py index 364e7b450..540fd87e9 100755 --- a/qa/rpc-tests/wallet_persistence.py +++ b/qa/rpc-tests/wallet_persistence.py @@ -4,28 +4,93 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_true, start_nodes, stop_nodes, \ - wait_bitcoinds +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(1, self.options.tmpdir) - self.is_network_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() - # make sure the node has the addresss assert_true(sapling_addr in addresses, "Should contain address before restart") - # restart the nodes + + # Restart the nodes stop_nodes(self.nodes) wait_bitcoinds() - self.nodes = self.setup_nodes() + self.setup_network() + + # Make sure we still have the address after restarting addresses = self.nodes[0].z_listaddresses() - # make sure we still have the address after restarting 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')) + + # 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() \ No newline at end of file From 01282e4fa63ab251b28d77bdd05df18aaf6d0595 Mon Sep 17 00:00:00 2001 From: Simon Date: Sun, 7 Oct 2018 16:22:33 -0700 Subject: [PATCH 12/12] Update wallet_persistence test to verify spending notes after restart. --- qa/rpc-tests/wallet_persistence.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/qa/rpc-tests/wallet_persistence.py b/qa/rpc-tests/wallet_persistence.py index 540fd87e9..612226275 100755 --- a/qa/rpc-tests/wallet_persistence.py +++ b/qa/rpc-tests/wallet_persistence.py @@ -68,6 +68,11 @@ class WalletPersistenceTest (BitcoinTestFramework): # 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 = []