sikkienl/dash
1
0
Fork 0
mirror of https://github.com/dashpay/dash.git synced 2024-12-26 12:32:48 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge #17373: wallet: Various fixes and cleanup to keypool handling in LegacyScriptPubKeyMan and CWallet

Browse Source 
886f1731bec4393dd342403ac34069a3a4f95eea Key pool: Fix omitted pre-split count in GetKeyPoolSize (Andrew Chow)
386a994b853bc5b3a2ed0d812673465b8ffa4849 Key pool: Change ReturnDestination interface to take address instead of key (Andrew Chow)
ba41aa4969169cd73d6b4f57444ed7d8d875de10 Key pool: Move LearnRelated and GetDestination calls (Andrew Chow)
65833a74076cddf986037c6eb3b29a9b9dbe31c5 Add OutputType and CPubKey parameters to KeepDestination (Andrew Chow)
9fcf8ce7ae02bf170b9bf0c2887fd709d752cbf7 Rename Keep/ReturnKey to Keep/ReturnDestination and remove the wrapper (Andrew Chow)
596f6460f9fd8273665c8754ccd673d93a4f25f0 Key pool: Move CanGetAddresses call (Andrew Chow)

Pull request description:

  * The `pwallet->CanGetAddresses()` call in `ReserveDestination::GetReservedDestination` to `LegacyScriptPubKeyMan::GetReservedDestination` so that the sanity check results in a failure when a `ScriptPubKeyMan` individually cannot get a destination, not when any of the `ScriptPubKeyMan`s can't.
  * `ScriptPubKeyMan::GetReservedDestination` is changed to return the destination so that future `ScriptPubKeyMan`s can return destinations constructed in other ways. This is implemented for `LegacyScriptPubKeyMan` by moving key-to-destination code from `CWallet` to `LegacyScriptPubKeyMan`
  * In order for `ScriptPubKeyMan` to be generic and work with future `ScriptPubKeyMan`s, `ScriptPubKeyMan::ReturnDestination` is changed to take a `CTxDestination` instead of a `CPubKey`. Since `LegacyScriptPubKeyMan` still deals with keys internally, a new map `m_reserved_key_to_index` is added in order to track the keypool indexes that have been reserved.
  * A bug is fixed in how the total keypool size is calculated as it was omitting `set_pre_split_keypool` which is a bug.

  Split from #17261

ACKs for top commit:
  ryanofsky:
    Code review ACK 886f1731bec4393dd342403ac34069a3a4f95eea. Only change is moving earlier fix to a better commit (same end result).
  promag:
    Code review ACK 886f1731bec4393dd342403ac34069a3a4f95eea.
  instagibbs:
    code review re-ACK 886f1731be
  Sjors:
    Code review re-ACK 886f1731bec4393dd342403ac34069a3a4f95eea

Tree-SHA512: f4be290759f63fdc920d5c02bd0d09acc4b06a5f053787d4afcd3c921b2e35d2bd97617fadae015da853dc189f559fb8d2c6e58d53e4cabfac9af151cd97ad19
This commit is contained in:
fanquake 2019-12-06 13:37:12 -05:00 committed by PastaPastaPasta
parent f798da8e92
commit 4b6a09bbd3
4 changed files with 30 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
src/wallet/scriptpubkeyman.cpp
View File

@ -263,24 +263,19 @@ bool LegacyScriptPubKeyMan::EncryptKeys(CKeyingMaterial& vMasterKeyIn)
return true;
}
bool LegacyScriptPubKeyMan::GetReservedDestination(bool internal, int64_t& index, CKeyPool& keypool)
bool LegacyScriptPubKeyMan::GetReservedDestination(bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool)
{
if (!CanGetAddresses(internal)) {
return false;
}
if (!ReserveKeyFromKeyPool(index, keypool, internal)) {
return false;
}
address = keypool.vchPubKey.GetID();
return true;
}
void LegacyScriptPubKeyMan::KeepDestination(int64_t index)
{
KeepKey(index);
}
void LegacyScriptPubKeyMan::ReturnDestination(int64_t index, bool internal, const CPubKey& pubkey)
{
ReturnKey(index, internal, pubkey);
}
void LegacyScriptPubKeyMan::MarkUnusedAddresses(WalletBatch &batch, const CScript& script, const uint256& hashBlock)
{
AssertLockHeld(cs_wallet);
@ -1447,7 +1442,7 @@ void LegacyScriptPubKeyMan::AddKeypoolPubkeyWithDB(const CPubKey& pubkey, const
m_pool_key_to_index[pubkey.GetID()] = index;
}
void LegacyScriptPubKeyMan::KeepKey(int64_t nIndex)
void LegacyScriptPubKeyMan::KeepDestination(int64_t nIndex)
{
// Remove from key pool
{
@ -1457,11 +1452,16 @@ void LegacyScriptPubKeyMan::KeepKey(int64_t nIndex)
--m_wallet.nKeysLeftSinceAutoBackup;
if (!nWalletBackups)
m_wallet.nKeysLeftSinceAutoBackup = 0;
CPubKey pubkey;
bool have_pk = GetPubKey(m_index_to_reserved_key.at(nIndex), pubkey);
assert(have_pk);
m_index_to_reserved_key.erase(nIndex);
}
WalletLogPrintf("keypool keep %d\n", nIndex);
}
void LegacyScriptPubKeyMan::ReturnKey(int64_t nIndex, bool fInternal, const CPubKey& pubkey)
void LegacyScriptPubKeyMan::ReturnDestination(int64_t nIndex, bool fInternal, const CTxDestination&)
{
// Return to key pool
{
@ -1471,7 +1471,9 @@ void LegacyScriptPubKeyMan::ReturnKey(int64_t nIndex, bool fInternal, const CPub
} else {
setExternalKeyPool.insert(nIndex);
}
m_pool_key_to_index[pubkey.GetID()] = nIndex;
CKeyID& pubkey_id = m_index_to_reserved_key.at(nIndex);
m_pool_key_to_index[pubkey_id] = nIndex;
m_index_to_reserved_key.erase(nIndex);
NotifyCanGetAddressesChanged();
}
WalletLogPrintf("keypool return %d\n", nIndex);
@ -1494,7 +1496,7 @@ bool LegacyScriptPubKeyMan::GetKeyFromPool(CPubKey& result, bool internal)
result = GenerateNewKey(batch, 0, internal);
return true;
}
KeepKey(nIndex);
KeepDestination(nIndex);
result = keypool.vchPubKey;
}
return true;
@ -1536,6 +1538,8 @@ bool LegacyScriptPubKeyMan::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& key
throw std::runtime_error(std::string(__func__) + ": keypool entry invalid");
}
assert(m_index_to_reserved_key.count(nIndex) == 0);
m_index_to_reserved_key[nIndex] = keypool.vchPubKey.GetID();
m_pool_key_to_index.erase(keypool.vchPubKey.GetID());
WalletLogPrintf("keypool reserve %d\n", nIndex);
}

14
src/wallet/scriptpubkeyman.h
View File

@ -145,9 +145,9 @@ public:
virtual isminetype IsMine(const CScript& script) const { return ISMINE_NO; }
virtual isminetype IsMine(const CTxDestination& dest) const { return ISMINE_NO; }
virtual bool GetReservedDestination(bool internal, int64_t& index, CKeyPool& keypool) { return false; }
virtual bool GetReservedDestination(bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool) { return false; }
virtual void KeepDestination(int64_t index) {}
virtual void ReturnDestination(int64_t index, bool internal, const CPubKey& pubkey) {}
virtual void ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) {}
virtual bool TopUp(unsigned int size = 0) { return false; }
@ -232,9 +232,12 @@ private:
std::set<int64_t> setExternalKeyPool GUARDED_BY(cs_wallet);
int64_t m_max_keypool_index GUARDED_BY(cs_wallet) = 0;
std::map<CKeyID, int64_t> m_pool_key_to_index;
// Tracks keypool indexes to CKeyIDs of keys that have been taken out of the keypool but may be returned to it
std::map<int64_t, CKeyID> m_index_to_reserved_key;
//! Fetches a key from the keypool
bool GetKeyFromPool(CPubKey &key, bool fInternal /*= false*/);
/**
* Reserves a key from the keypool and sets nIndex to its index
*
@ -251,9 +254,6 @@ private:
*/
bool ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool fRequestedInternal);
void KeepKey(int64_t nIndex);
void ReturnKey(int64_t nIndex, bool fInternal, const CPubKey& pubkey);
public:
bool GetNewDestination(CTxDestination& dest, std::string& error) override;
isminetype IsMine(const CScript& script) const override;
@ -262,9 +262,9 @@ public:
//! will encrypt previously unencrypted keys
bool EncryptKeys(CKeyingMaterial& vMasterKeyIn);
bool GetReservedDestination(bool internal, int64_t& index, CKeyPool& keypool) override;
bool GetReservedDestination(bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool) override;
void KeepDestination(int64_t index) override;
void ReturnDestination(int64_t index, bool internal, const CPubKey& pubkey) override;
void ReturnDestination(int64_t index, bool internal, const CTxDestination&) override;
bool TopUp(unsigned int size = 0) override;

13
src/wallet/wallet.cpp
View File

@ -3966,21 +3966,14 @@ bool ReserveDestination::GetReservedDestination(CTxDestination& dest, bool fInte
return false;
}
if (!pwallet->CanGetAddresses(fInternalIn)) {
return false;
}
if (nIndex == -1)
{
CKeyPool keypool;
if (!m_spk_man->GetReservedDestination(fInternalIn, nIndex, keypool)) {
if (!m_spk_man->GetReservedDestination(fInternalIn, address, nIndex, keypool)) {
return false;
}
vchPubKey = keypool.vchPubKey;
fInternal = keypool.fInternal;
}
assert(vchPubKey.IsValid());
address = vchPubKey.GetID();
dest = address;
return true;
}
@ -3991,17 +3984,15 @@ void ReserveDestination::KeepDestination()
m_spk_man->KeepDestination(nIndex);
}
nIndex = -1;
vchPubKey = CPubKey();
address = CNoDestination();
}
void ReserveDestination::ReturnDestination()
{
if (nIndex != -1) {
m_spk_man->ReturnDestination(nIndex, fInternal, vchPubKey);
m_spk_man->ReturnDestination(nIndex, fInternal, address);
}
nIndex = -1;
vchPubKey = CPubKey();
address = CNoDestination();
}

5
src/wallet/wallet.h
View File

@ -149,12 +149,11 @@ class ReserveDestination : public CReserveScript
protected:
//! The wallet to reserve from
CWallet* const pwallet;
LegacyScriptPubKeyMan* m_spk_man{nullptr};
//! The ScriptPubKeyMan to reserve from. Based on type when GetReservedDestination is called
ScriptPubKeyMan* m_spk_man{nullptr};
//! The index of the address's key in the keypool
int64_t nIndex{-1};
//! The public key for the address
CPubKey vchPubKey;
//! The destination
CTxDestination address;
//! Whether this is from the internal (change output) keypool