From 83c4e360daba7ea9e3a6c24a1d65acf656819b2f Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Thu, 27 Sep 2018 15:44:04 -0600 Subject: [PATCH] Address need not be optional when adding sapling keys --- src/gtest/test_keystore.cpp | 6 ------ src/keystore.cpp | 14 ++++++-------- src/keystore.h | 8 ++++---- src/wallet/crypter.cpp | 8 ++++---- src/wallet/crypter.h | 4 ++-- src/wallet/gtest/test_wallet.cpp | 16 ++++++++-------- src/wallet/wallet.cpp | 4 ++-- src/wallet/wallet.h | 4 ++-- 8 files changed, 28 insertions(+), 36 deletions(-) diff --git a/src/gtest/test_keystore.cpp b/src/gtest/test_keystore.cpp index 6b4d08a59..ccf9cb9ba 100644 --- a/src/gtest/test_keystore.cpp +++ b/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)); diff --git a/src/keystore.cpp b/src/keystore.cpp index 8262c8aab..be5e0b82f 100644 --- a/src/keystore.cpp +++ b/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 &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,15 @@ bool CBasicKeyStore::AddSproutViewingKey(const libzcash::SproutViewingKey &vk) bool CBasicKeyStore::AddSaplingFullViewingKey( const libzcash::SaplingFullViewingKey &fvk, - const boost::optional &defaultAddr) + const libzcash::SaplingPaymentAddress &defaultAddr) { LOCK(cs_SpendingKeyStore); auto ivk = fvk.in_viewing_key(); mapSaplingFullViewingKeys[ivk] = fvk; - - if (defaultAddr) { - // Add defaultAddr -> SaplingIncomingViewing to SaplingIncomingViewingKeyMap - mapSaplingIncomingViewingKeys[defaultAddr.get()] = ivk; - } + // Add defaultAddr -> SaplingIncomingViewing to SaplingIncomingViewingKeyMap + mapSaplingIncomingViewingKeys[defaultAddr] = ivk; + return true; } diff --git a/src/keystore.h b/src/keystore.h index a8a8f62f5..5b064f27e 100644 --- a/src/keystore.h +++ b/src/keystore.h @@ -66,7 +66,7 @@ public: //! Add a Sapling spending key to the store. virtual bool AddSaplingSpendingKey( const libzcash::SaplingExtendedSpendingKey &sk, - const boost::optional &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; @@ -75,7 +75,7 @@ public: //! Support for Sapling full viewing keys virtual bool AddSaplingFullViewingKey( const libzcash::SaplingFullViewingKey &fvk, - const boost::optional &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, @@ -236,7 +236,7 @@ public: //! Sapling bool AddSaplingSpendingKey( const libzcash::SaplingExtendedSpendingKey &sk, - const boost::optional &defaultAddr = boost::none); + const libzcash::SaplingPaymentAddress &defaultAddr); bool HaveSaplingSpendingKey(const libzcash::SaplingFullViewingKey &fvk) const { bool result; @@ -263,7 +263,7 @@ public: virtual bool AddSaplingFullViewingKey( const libzcash::SaplingFullViewingKey &fvk, - const boost::optional &defaultAddr = boost::none); + const libzcash::SaplingPaymentAddress &defaultAddr); virtual bool HaveSaplingFullViewingKey(const libzcash::SaplingIncomingViewingKey &ivk) const; virtual bool GetSaplingFullViewingKey( const libzcash::SaplingIncomingViewingKey &ivk, diff --git a/src/wallet/crypter.cpp b/src/wallet/crypter.cpp index 88807598a..25d69cc3d 100644 --- a/src/wallet/crypter.cpp +++ b/src/wallet/crypter.cpp @@ -449,7 +449,7 @@ bool CCryptoKeyStore::AddSproutSpendingKey(const libzcash::SproutSpendingKey &sk bool CCryptoKeyStore::AddSaplingSpendingKey( const libzcash::SaplingExtendedSpendingKey &sk, - const boost::optional &defaultAddr) + const libzcash::SaplingPaymentAddress &defaultAddr) { { LOCK(cs_SpendingKeyStore); @@ -496,7 +496,7 @@ bool CCryptoKeyStore::AddCryptedSproutSpendingKey( bool CCryptoKeyStore::AddCryptedSaplingSpendingKey( const libzcash::SaplingFullViewingKey &fvk, const std::vector &vchCryptedSecret, - const boost::optional &defaultAddr) + const libzcash::SaplingPaymentAddress &defaultAddr) { { LOCK(cs_SpendingKeyStore); @@ -505,7 +505,7 @@ bool CCryptoKeyStore::AddCryptedSaplingSpendingKey( } // if SaplingFullViewingKey is not in SaplingFullViewingKeyMap, add it - if (!AddSaplingFullViewingKey(fvk, defaultAddr)){ + if (!AddSaplingFullViewingKey(fvk, defaultAddr)) { return false; } @@ -614,7 +614,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; } } diff --git a/src/wallet/crypter.h b/src/wallet/crypter.h index 7c326fbc4..b751ce300 100644 --- a/src/wallet/crypter.h +++ b/src/wallet/crypter.h @@ -243,10 +243,10 @@ public: virtual bool AddCryptedSaplingSpendingKey( const libzcash::SaplingFullViewingKey &fvk, const std::vector &vchCryptedSecret, - const boost::optional &defaultAddr = boost::none); + const libzcash::SaplingPaymentAddress &defaultAddr); bool AddSaplingSpendingKey( const libzcash::SaplingExtendedSpendingKey &sk, - const boost::optional &defaultAddr = boost::none); + const libzcash::SaplingPaymentAddress &defaultAddr); bool HaveSaplingSpendingKey(const libzcash::SaplingFullViewingKey &fvk) const { { diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index a67c3b491..fa066061d 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -516,7 +516,7 @@ TEST(WalletTests, FindMySaplingNotes) { 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); EXPECT_EQ(2, noteMap.size()); @@ -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 @@ -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 @@ -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 @@ -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)); @@ -1784,7 +1784,7 @@ 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); @@ -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 diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d931f7384..2093fadb9 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -150,7 +150,7 @@ SaplingPaymentAddress CWallet::GenerateNewSaplingZKey() // Add spending key to keystore bool CWallet::AddSaplingZKey( const libzcash::SaplingExtendedSpendingKey &sk, - const boost::optional &defaultAddr) + const libzcash::SaplingPaymentAddress &defaultAddr) { AssertLockHeld(cs_wallet); // mapSaplingZKeyMetadata @@ -291,7 +291,7 @@ bool CWallet::AddCryptedSproutSpendingKey( bool CWallet::AddCryptedSaplingSpendingKey(const libzcash::SaplingFullViewingKey &fvk, const std::vector &vchCryptedSecret, - const boost::optional &defaultAddr) + const libzcash::SaplingPaymentAddress &defaultAddr) { if (!CCryptoKeyStore::AddCryptedSaplingSpendingKey(fvk, vchCryptedSecret, defaultAddr)) return false; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index dda1e4dfe..1deb9b3aa 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1058,11 +1058,11 @@ public: //! Adds Sapling spending key to the store, and saves it to disk bool AddSaplingZKey( const libzcash::SaplingExtendedSpendingKey &key, - const boost::optional &defaultAddr = boost::none); + const libzcash::SaplingPaymentAddress &defaultAddr); bool AddCryptedSaplingSpendingKey( const libzcash::SaplingFullViewingKey &fvk, const std::vector &vchCryptedSecret, - const boost::optional &defaultAddr = boost::none); + const libzcash::SaplingPaymentAddress &defaultAddr); /** * Increment the next transaction order id