diff --git a/src/gtest/test_keystore.cpp b/src/gtest/test_keystore.cpp index d767967ba..5660f369b 100644 --- a/src/gtest/test_keystore.cpp +++ b/src/gtest/test_keystore.cpp @@ -55,9 +55,7 @@ TEST(keystore_tests, sapling_keys) { EXPECT_EQ(in_viewing_key, in_viewing_key_2); // Check that the default address from primitives and from sk method are the same - auto addrOpt = sk.default_address(); - EXPECT_TRUE(addrOpt); - auto default_addr = addrOpt.value(); + auto default_addr = sk.default_address(); auto addrOpt2 = in_viewing_key.address(default_d); EXPECT_TRUE(addrOpt2); auto default_addr_2 = addrOpt2.value(); @@ -175,9 +173,7 @@ TEST(keystore_tests, StoreAndRetrieveSaplingSpendingKey) { auto sk = libzcash::SaplingSpendingKey::random(); auto fvk = sk.full_viewing_key(); auto ivk = fvk.in_viewing_key(); - auto addrOpt = sk.default_address(); - EXPECT_TRUE(addrOpt); - auto addr = addrOpt.value(); + auto addr = sk.default_address(); // Sanity-check: we can't get a key we haven't added EXPECT_FALSE(keyStore.HaveSaplingSpendingKey(fvk)); diff --git a/src/gtest/test_sapling_note.cpp b/src/gtest/test_sapling_note.cpp index de06b54c1..06f2d6188 100644 --- a/src/gtest/test_sapling_note.cpp +++ b/src/gtest/test_sapling_note.cpp @@ -56,7 +56,7 @@ TEST(SaplingNote, TestVectors) TEST(SaplingNote, Random) { // Test creating random notes using the same spending key - auto address = SaplingSpendingKey::random().default_address().get(); + auto address = SaplingSpendingKey::random().default_address(); SaplingNote note1(address, GetRand(MAX_MONEY)); SaplingNote note2(address, GetRand(MAX_MONEY)); @@ -66,7 +66,7 @@ TEST(SaplingNote, Random) ASSERT_NE(note1.r, note2.r); // Test diversifier and pk_d are not the same for different spending keys - SaplingNote note3(SaplingSpendingKey::random().default_address().get(), GetRand(MAX_MONEY)); + SaplingNote note3(SaplingSpendingKey::random().default_address(), GetRand(MAX_MONEY)); ASSERT_NE(note1.d, note3.d); ASSERT_NE(note1.pk_d, note3.pk_d); } diff --git a/src/keystore.cpp b/src/keystore.cpp index d0e5b8ec6..cac20a1ae 100644 --- a/src/keystore.cpp +++ b/src/keystore.cpp @@ -98,16 +98,17 @@ bool CBasicKeyStore::AddSaplingSpendingKey(const libzcash::SaplingSpendingKey &s { LOCK(cs_SpendingKeyStore); auto fvk = sk.full_viewing_key(); - mapSaplingSpendingKeys[fvk] = sk; // if SaplingFullViewingKey is not in SaplingFullViewingKeyMap, add it - AddSaplingFullViewingKey(fvk); + if (!AddSaplingFullViewingKey(fvk)){ + return false; + } + mapSaplingSpendingKeys[fvk] = sk; + // Add addr -> SaplingIncomingViewing to SaplingIncomingViewingKeyMap auto ivk = fvk.in_viewing_key(); - auto addrOpt = sk.default_address(); - assert(addrOpt != boost::none); - auto addr = addrOpt.value(); + auto addr = sk.default_address(); mapSaplingIncomingViewingKeys[addr] = ivk; return true; diff --git a/src/keystore.h b/src/keystore.h index 3d230dc37..541c0bca5 100644 --- a/src/keystore.h +++ b/src/keystore.h @@ -92,6 +92,7 @@ typedef std::map NoteDecryptor // Full viewing key has equivalent functionality to a transparent address // When encrypting wallet, encrypt SaplingSpendingKeyMap, while leaving SaplingFullViewingKeyMap unencrypted +// When implementing ZIP 32, add another map from SaplingFullViewingKey -> SaplingExpandedSpendingKey typedef std::map SaplingSpendingKeyMap; typedef std::map SaplingFullViewingKeyMap; // Only maps from default addresses to ivk, may need to be reworked when adding diversified addresses. diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 410aec564..420246f0b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -103,21 +103,16 @@ libzcash::PaymentAddress CWallet::GenerateNewZKey() // Generate a new Sapling spending key and return its public payment address SaplingPaymentAddress CWallet::GenerateNewSaplingZKey() { - AssertLockHeld(cs_wallet); // mapZKeyMetadata + AssertLockHeld(cs_wallet); // mapSaplingZKeyMetadata - SaplingSpendingKey sk; - boost::optional addrOpt; - while (!addrOpt){ - sk = SaplingSpendingKey::random(); - addrOpt = sk.default_address(); - } - - auto addr = addrOpt.value(); + auto sk = SaplingSpendingKey::random(); auto fvk = sk.full_viewing_key(); + auto addr = sk.default_address(); // Check for collision, even though it is unlikely to ever occur - if (CCryptoKeyStore::HaveSaplingSpendingKey(fvk)) - throw std::runtime_error("CWallet::GenerateNewSaplingZKey(): Collision detected"); + if (CCryptoKeyStore::HaveSaplingSpendingKey(fvk)) { + throw std::runtime_error("CWallet::GenerateNewSaplingZKey(): Collision detected"); + } // Create new metadata int64_t nCreationTime = GetTime(); diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 554f1672b..318993a65 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -141,7 +141,7 @@ public: bool WriteViewingKey(const libzcash::SproutViewingKey &vk); bool EraseViewingKey(const libzcash::SproutViewingKey &vk); - + private: CWalletDB(const CWalletDB&); void operator=(const CWalletDB&); diff --git a/src/zcash/Address.cpp b/src/zcash/Address.cpp index 43bd45ecd..e6dd0125a 100644 --- a/src/zcash/Address.cpp +++ b/src/zcash/Address.cpp @@ -75,8 +75,11 @@ boost::optional SaplingIncomingViewingKey::address(divers } } -boost::optional SaplingSpendingKey::default_address() const { - return full_viewing_key().in_viewing_key().address(default_diversifier(*this)); +SaplingPaymentAddress SaplingSpendingKey::default_address() const { + // Iterates within default_diversifier to ensure a valid address is returned + auto addrOpt = full_viewing_key().in_viewing_key().address(default_diversifier(*this)); + assert(addrOpt != boost::none); + return addrOpt.value(); } } diff --git a/src/zcash/Address.hpp b/src/zcash/Address.hpp index 614defdde..2a0d7a481 100644 --- a/src/zcash/Address.hpp +++ b/src/zcash/Address.hpp @@ -202,7 +202,7 @@ public: SaplingFullViewingKey full_viewing_key() const; // Can derive Sapling addr from default diversifier - boost::optional default_address() const; + SaplingPaymentAddress default_address() const; }; typedef boost::variant PaymentAddress;