Revert change to the type of GenerateNewKey

src/wallet/test/rpc_wallet_tests.cpp

@@ -134,7 +134,7 @@ BOOST_AUTO_TEST_CASE(rpc_wallet)
 
     LOCK2(cs_main, pwalletMain->cs_wallet);
 
-    CPubKey demoPubkey = pwalletMain->GenerateNewKey().value();
+    CPubKey demoPubkey = pwalletMain->GenerateNewKey();
     CTxDestination demoAddress(CTxDestination(demoPubkey.GetID()));
     UniValue retValue;
     string strPurpose = "receive";
@@ -143,7 +143,7 @@ BOOST_AUTO_TEST_CASE(rpc_wallet)
         pwalletMain->SetAddressBook(demoPubkey.GetID(), "", strPurpose);
     });
 
-    CPubKey setaccountDemoPubkey = pwalletMain->GenerateNewKey().value();
+    CPubKey setaccountDemoPubkey = pwalletMain->GenerateNewKey();
     CTxDestination setaccountDemoAddress(CTxDestination(setaccountDemoPubkey.GetID()));
 
     /*********************************
@@ -1469,7 +1469,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_taddr_to_sapling)
 
     KeyIO keyIO(Params());
     // add keys manually
-    auto taddr = pwalletMain->GenerateNewKey().value().GetID();
+    auto taddr = pwalletMain->GenerateNewKey().GetID();
     std::string taddr1 = keyIO.EncodeDestination(taddr);
     auto pa = DefaultSaplingAddress(pwalletMain);
     std::string zaddr1 = keyIO.EncodePaymentAddress(pa);
@@ -2173,7 +2173,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_mergetoaddress_internals)
 void TestWTxStatus(const Consensus::Params consensusParams, const int delta) {
 
     auto AddTrx = [&consensusParams]() {
-        auto taddr = pwalletMain->GenerateNewKey().value().GetID();
+        auto taddr = pwalletMain->GenerateNewKey().GetID();
         CMutableTransaction mtx = CreateNewContextualCMutableTransaction(consensusParams, 1);
         CScript scriptPubKey = CScript() << OP_DUP << OP_HASH160 << ToByteVector(taddr) << OP_EQUALVERIFY << OP_CHECKSIG;
         mtx.vout.push_back(CTxOut(5 * COIN, scriptPubKey));

src/wallet/wallet.cpp

@@ -155,7 +155,6 @@ std::pair<SaplingExtendedSpendingKey, bool> CWallet::GenerateLegacySaplingZKey(u
         throw std::runtime_error(
                 "CWallet::GenerateLegacySaplingZKey(): Wallet does not have a mnemonic seed.");
     }
-
     auto seed = seedOpt.value();
 
     auto xsk = libzcash::SaplingExtendedSpendingKey::Legacy(seed, BIP44CoinType(), addrIndex);
@@ -254,53 +253,56 @@ bool CWallet::AddSproutZKey(const libzcash::SproutSpendingKey &key)
     return true;
 }
 
-std::optional<CPubKey> CWallet::GenerateNewKey()
+CPubKey CWallet::GenerateNewKey()
 {
     AssertLockHeld(cs_wallet); // mapKeyMetadata
+
     auto seedOpt = GetMnemonicSeed();
-    if (seedOpt.has_value()) {
+    if (!seedOpt.has_value()) {
-        if (!mnemonicHDChain.has_value()) {
+        throw std::runtime_error(
-            mnemonicHDChain = CHDChain(seedOpt.value().Fingerprint(), GetTime());
+                "CWallet::GenerateNewKey(): Wallet does not have a mnemonic seed.");
-        }
+    }
-        CHDChain& hdChain = mnemonicHDChain.value();
+    auto seed = seedOpt.value();
 
-        // All mnemonic seeds are checked at construction to ensure that we can obtain
+    if (!mnemonicHDChain.has_value()) {
-        // a valid spending key for the account ZCASH_LEGACY_ACCOUNT;
+        mnemonicHDChain = CHDChain(seedOpt.value().Fingerprint(), GetTime());
-        // therefore, the `value()` call here is safe.
+    }
-        BIP32AccountChains accountChains = BIP32AccountChains::ForAccount(
+    CHDChain& hdChain = mnemonicHDChain.value();
-                seedOpt.value(),
-                BIP44CoinType(),
-                ZCASH_LEGACY_ACCOUNT).value();
 
-        while (true) {
+    // All mnemonic seeds are checked at construction to ensure that we can obtain
-            auto extKey = accountChains.DeriveExternal(hdChain.GetLegacyTKeyCounter());
+    // a valid spending key for the account ZCASH_LEGACY_ACCOUNT;
-            hdChain.IncrementLegacyTKeyCounter();
+    // therefore, the `value()` call here is safe.
+    BIP32AccountChains accountChains = BIP32AccountChains::ForAccount(
+            seedOpt.value(),
+            BIP44CoinType(),
+            ZCASH_LEGACY_ACCOUNT).value();
 
-            // if we did not successfully generate a key, try again.
+    while (true) {
-            if (extKey.has_value()) {
+        auto extKey = accountChains.DeriveExternal(hdChain.GetLegacyTKeyCounter());
-                CKey secret = extKey.value().first.key;
+        hdChain.IncrementLegacyTKeyCounter();
-                CPubKey pubkey = secret.GetPubKey();
-                assert(secret.VerifyPubKey(pubkey));
 
-                // Create new metadata
+        // if we did not successfully generate a key, try again.
-                const CKeyMetadata& keyMeta = extKey.value().second;
+        if (extKey.has_value()) {
-                mapKeyMetadata[pubkey.GetID()] = keyMeta;
+            CKey secret = extKey.value().first.key;
-                if (nTimeFirstKey == 0 || keyMeta.nCreateTime < nTimeFirstKey)
+            CPubKey pubkey = secret.GetPubKey();
-                    nTimeFirstKey = keyMeta.nCreateTime;
+            assert(secret.VerifyPubKey(pubkey));
 
-                if (!AddKeyPubKey(secret, pubkey))
+            // Create new metadata
-                    throw std::runtime_error("CWallet::GenerateNewKey(): AddKey failed");
+            const CKeyMetadata& keyMeta = extKey.value().second;
+            mapKeyMetadata[pubkey.GetID()] = keyMeta;
+            if (nTimeFirstKey == 0 || keyMeta.nCreateTime < nTimeFirstKey)
+                nTimeFirstKey = keyMeta.nCreateTime;
 
-                // Update the persisted chain information
+            if (!AddKeyPubKey(secret, pubkey))
-                if (fFileBacked && !CWalletDB(strWalletFile).WriteMnemonicHDChain(hdChain)) {
+                throw std::runtime_error("CWallet::GenerateNewKey(): AddKey failed");
-                    throw std::runtime_error("CWallet::GenerateNewKey(): Writing HD chain model failed");
-                }
 
-                return pubkey;
+            // Update the persisted chain information
+            if (fFileBacked && !CWalletDB(strWalletFile).WriteMnemonicHDChain(hdChain)) {
+                throw std::runtime_error("CWallet::GenerateNewKey(): Writing HD chain model failed");
             }
+
+            return pubkey;
         }
-    } else {
-        return std::nullopt;
     }
 }
 
@@ -4207,9 +4209,7 @@ bool CWallet::NewKeyPool()
         {
             int64_t nIndex = i+1;
             auto key = GenerateNewKey();
-            if (!key.has_value())
+            walletdb.WritePool(nIndex, CKeyPool(key));
-                return false; // should have been caught by the `IsLocked` call.
-            walletdb.WritePool(nIndex, CKeyPool(key.value()));
             setKeyPool.insert(nIndex);
         }
         LogPrintf("CWallet::NewKeyPool wrote %d new keys\n", nKeys);
@@ -4240,7 +4240,7 @@ bool CWallet::TopUpKeyPool(unsigned int kpSize)
             if (!setKeyPool.empty())
                 nEnd = *(--setKeyPool.end()) + 1;
             auto newKey = GenerateNewKey();
-            if (!newKey.has_value() || !walletdb.WritePool(nEnd, CKeyPool(newKey.value())))
+            if (!walletdb.WritePool(nEnd, CKeyPool(newKey)))
                 throw runtime_error("TopUpKeyPool(): writing generated key failed");
             setKeyPool.insert(nEnd);
             LogPrintf("keypool added key %d, size=%u\n", nEnd, setKeyPool.size());

src/wallet/wallet.h

@@ -993,7 +993,7 @@ public:
      * keystore implementation
      * Generate a new key
      */
-    std::optional<CPubKey> GenerateNewKey();
+    CPubKey GenerateNewKey();
     //! Adds a key to the store, and saves it to disk.
     bool AddKeyPubKey(const CKey& key, const CPubKey &pubkey);
     //! Adds a key to the store, without saving it to disk (used by LoadWallet)