From e0ae5362c9b9b9a17b9ae43b9a4e74251cbd2a1e Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 21 Oct 2021 08:21:13 -0600 Subject: [PATCH] Revert change to the type of GenerateNewKey --- src/wallet/test/rpc_wallet_tests.cpp | 8 +-- src/wallet/wallet.cpp | 80 ++++++++++++++-------------- src/wallet/wallet.h | 2 +- 3 files changed, 45 insertions(+), 45 deletions(-) diff --git a/src/wallet/test/rpc_wallet_tests.cpp b/src/wallet/test/rpc_wallet_tests.cpp index d3ccf872b..f415ec1c8 100644 --- a/src/wallet/test/rpc_wallet_tests.cpp +++ b/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)); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 335c6bd1c..67ef4f911 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -155,7 +155,6 @@ std::pair 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 CWallet::GenerateNewKey() +CPubKey CWallet::GenerateNewKey() { AssertLockHeld(cs_wallet); // mapKeyMetadata + auto seedOpt = GetMnemonicSeed(); - if (seedOpt.has_value()) { - if (!mnemonicHDChain.has_value()) { - mnemonicHDChain = CHDChain(seedOpt.value().Fingerprint(), GetTime()); - } - CHDChain& hdChain = mnemonicHDChain.value(); + if (!seedOpt.has_value()) { + throw std::runtime_error( + "CWallet::GenerateNewKey(): Wallet does not have a mnemonic seed."); + } + auto seed = seedOpt.value(); - // All mnemonic seeds are checked at construction to ensure that we can obtain - // a valid spending key for the account ZCASH_LEGACY_ACCOUNT; - // therefore, the `value()` call here is safe. - BIP32AccountChains accountChains = BIP32AccountChains::ForAccount( - seedOpt.value(), - BIP44CoinType(), - ZCASH_LEGACY_ACCOUNT).value(); + if (!mnemonicHDChain.has_value()) { + mnemonicHDChain = CHDChain(seedOpt.value().Fingerprint(), GetTime()); + } + CHDChain& hdChain = mnemonicHDChain.value(); - while (true) { - auto extKey = accountChains.DeriveExternal(hdChain.GetLegacyTKeyCounter()); - hdChain.IncrementLegacyTKeyCounter(); + // All mnemonic seeds are checked at construction to ensure that we can obtain + // a valid spending key for the account ZCASH_LEGACY_ACCOUNT; + // 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. - if (extKey.has_value()) { - CKey secret = extKey.value().first.key; - CPubKey pubkey = secret.GetPubKey(); - assert(secret.VerifyPubKey(pubkey)); + while (true) { + auto extKey = accountChains.DeriveExternal(hdChain.GetLegacyTKeyCounter()); + hdChain.IncrementLegacyTKeyCounter(); - // Create new metadata - const CKeyMetadata& keyMeta = extKey.value().second; - mapKeyMetadata[pubkey.GetID()] = keyMeta; - if (nTimeFirstKey == 0 || keyMeta.nCreateTime < nTimeFirstKey) - nTimeFirstKey = keyMeta.nCreateTime; + // if we did not successfully generate a key, try again. + if (extKey.has_value()) { + CKey secret = extKey.value().first.key; + CPubKey pubkey = secret.GetPubKey(); + assert(secret.VerifyPubKey(pubkey)); - if (!AddKeyPubKey(secret, pubkey)) - throw std::runtime_error("CWallet::GenerateNewKey(): AddKey failed"); + // Create new metadata + 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 (fFileBacked && !CWalletDB(strWalletFile).WriteMnemonicHDChain(hdChain)) { - throw std::runtime_error("CWallet::GenerateNewKey(): Writing HD chain model failed"); - } + if (!AddKeyPubKey(secret, pubkey)) + throw std::runtime_error("CWallet::GenerateNewKey(): AddKey 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()) - return false; // should have been caught by the `IsLocked` call. - walletdb.WritePool(nIndex, CKeyPool(key.value())); + walletdb.WritePool(nIndex, CKeyPool(key)); 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()); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index b98801685..cbfe6fce1 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -993,7 +993,7 @@ public: * keystore implementation * Generate a new key */ - std::optional 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)