Change default_address to return SaplingPaymentAddr and not boost::optional

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));

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);
 }

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;

src/keystore.h

@@ -92,6 +92,7 @@ typedef std::map<libzcash::SproutPaymentAddress, ZCNoteDecryption> 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<libzcash::SaplingFullViewingKey, libzcash::SaplingSpendingKey> SaplingSpendingKeyMap;
 typedef std::map<libzcash::SaplingIncomingViewingKey, libzcash::SaplingFullViewingKey> SaplingFullViewingKeyMap;
 // Only maps from default addresses to ivk, may need to be reworked when adding diversified addresses. 

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
-    
-    SaplingSpendingKey sk;
-    boost::optional<SaplingPaymentAddress> addrOpt;
-    while (!addrOpt){
-        sk = SaplingSpendingKey::random();
-        addrOpt = sk.default_address();
-    }
+    AssertLockHeld(cs_wallet); // mapSaplingZKeyMetadata
     
-    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();

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&);

src/zcash/Address.cpp

@@ -75,8 +75,11 @@ boost::optional<SaplingPaymentAddress> SaplingIncomingViewingKey::address(divers
     }
 }
 
-boost::optional<SaplingPaymentAddress> 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();
 }
 
 }

src/zcash/Address.hpp

@@ -202,7 +202,7 @@ public:
     SaplingFullViewingKey full_viewing_key() const;
     
     // Can derive Sapling addr from default diversifier 
-    boost::optional<SaplingPaymentAddress> default_address() const;
+    SaplingPaymentAddress default_address() const;
 };
 
 typedef boost::variant<InvalidEncoding, SproutPaymentAddress, SaplingPaymentAddress> PaymentAddress;