diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 74f36e9ab..4a35474f1 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3427,7 +3427,7 @@ bool CWallet::TopUpKeyPool(unsigned int kpSize) return true; } -void CWallet::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool fRequestedInternal) +bool CWallet::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool fRequestedInternal) { nIndex = -1; keypool.vchPubKey = CPubKey(); @@ -3438,11 +3438,13 @@ void CWallet::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool fRe TopUpKeyPool(); bool fReturningInternal = IsHDEnabled() && CanSupportFeature(FEATURE_HD_SPLIT) && fRequestedInternal; - std::set& setKeyPool = set_pre_split_keypool.empty() ? (fReturningInternal ? setInternalKeyPool : setExternalKeyPool) : set_pre_split_keypool; + bool use_split_keypool = set_pre_split_keypool.empty(); + std::set& setKeyPool = use_split_keypool ? (fReturningInternal ? setInternalKeyPool : setExternalKeyPool) : set_pre_split_keypool; // Get the oldest key - if(setKeyPool.empty()) - return; + if (setKeyPool.empty()) { + return false; + } WalletBatch batch(*database); @@ -3456,14 +3458,17 @@ void CWallet::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool fRe throw std::runtime_error(std::string(__func__) + ": unknown key in key pool"); } // If the key was pre-split keypool, we don't care about what type it is - if (set_pre_split_keypool.size() == 0 && keypool.fInternal != fReturningInternal) { + if (use_split_keypool && keypool.fInternal != fReturningInternal) { throw std::runtime_error(std::string(__func__) + ": keypool entry misclassified"); } + if (!keypool.vchPubKey.IsValid()) { + throw std::runtime_error(std::string(__func__) + ": keypool entry invalid"); + } - assert(keypool.vchPubKey.IsValid()); m_pool_key_to_index.erase(keypool.vchPubKey.GetID()); LogPrintf("keypool reserve %d\n", nIndex); } + return true; } void CWallet::KeepKey(int64_t nIndex) @@ -3496,10 +3501,8 @@ bool CWallet::GetKeyFromPool(CPubKey& result, bool internal) CKeyPool keypool; { LOCK(cs_wallet); - int64_t nIndex = 0; - ReserveKeyFromKeyPool(nIndex, keypool, internal); - if (nIndex == -1) - { + int64_t nIndex; + if (!ReserveKeyFromKeyPool(nIndex, keypool, internal)) { if (IsLocked()) return false; WalletBatch batch(*database); result = GenerateNewKey(batch, internal); @@ -3701,12 +3704,10 @@ bool CReserveKey::GetReservedKey(CPubKey& pubkey, bool internal) if (nIndex == -1) { CKeyPool keypool; - pwallet->ReserveKeyFromKeyPool(nIndex, keypool, internal); - if (nIndex != -1) - vchPubKey = keypool.vchPubKey; - else { + if (!pwallet->ReserveKeyFromKeyPool(nIndex, keypool, internal)) { return false; } + vchPubKey = keypool.vchPubKey; fInternal = keypool.fInternal; } assert(vchPubKey.IsValid()); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index fecc2178e..5e05ed3c8 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -999,7 +999,22 @@ public: bool NewKeyPool(); size_t KeypoolCountExternalKeys() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool TopUpKeyPool(unsigned int kpSize = 0); - void ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool fRequestedInternal); + + /** + * Reserves a key from the keypool and sets nIndex to its index + * + * @param[out] nIndex the index of the key in keypool + * @param[out] keypool the keypool the key was drawn from, which could be the + * the pre-split pool if present, or the internal or external pool + * @param fRequestedInternal true if the caller would like the key drawn + * from the internal keypool, false if external is preferred + * + * @return true if succeeded, false if failed due to empty keypool + * @throws std::runtime_error if keypool read failed, key was invalid, + * was not found in the wallet, or was misclassified in the internal + * or external keypool + */ + bool ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool fRequestedInternal); void KeepKey(int64_t nIndex); void ReturnKey(int64_t nIndex, bool fInternal, const CPubKey& pubkey); bool GetKeyFromPool(CPubKey &key, bool internal = false);