Skip to content

Backport bitcoin#10831: Batch flushing operations to the walletdb during top up and increase keypool size #3045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
UdjinM6 merged 2 commits into from
Aug 16, 2019
Merged

Backport bitcoin#10831: Batch flushing operations to the walletdb during top up and increase keypool size #3045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6d39303
backport #10831
PastaPastaPasta Aug 6, 2019
daf40e3
Apply the same logic for SetHDChain/SetCryptedHDChain
UdjinM6 Aug 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 73 additions & 30 deletions src/wallet/wallet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ const CWalletTx* CWallet::GetWalletTx(const uint256& hash) const
return &(it->second);
}

CPubKey CWallet::GenerateNewKey(uint32_t nAccountIndex, bool fInternal)
CPubKey CWallet::GenerateNewKey(CWalletDB &walletdb, uint32_t nAccountIndex, bool fInternal)
{
AssertLockHeld(cs_wallet); // mapKeyMetadata
bool fCompressed = CanSupportFeature(FEATURE_COMPRPUBKEY); // default to compressed public keys if we want 0.6.0 wallets
Expand All @@ -122,14 +122,15 @@ CPubKey CWallet::GenerateNewKey(uint32_t nAccountIndex, bool fInternal)
CPubKey pubkey;
// use HD key derivation if HD was enabled during wallet creation
if (IsHDEnabled()) {
DeriveNewChildKey(metadata, secret, nAccountIndex, fInternal);
DeriveNewChildKey(walletdb, metadata, secret, nAccountIndex, fInternal);
pubkey = secret.GetPubKey();
} else {
secret.MakeNewKey(fCompressed);

// Compressed public keys were introduced in version 0.6.0
if (fCompressed)
if (fCompressed) {
SetMinVersion(FEATURE_COMPRPUBKEY);
}

pubkey = secret.GetPubKey();
assert(secret.VerifyPubKey(pubkey));
Expand All @@ -138,13 +139,14 @@ CPubKey CWallet::GenerateNewKey(uint32_t nAccountIndex, bool fInternal)
mapKeyMetadata[pubkey.GetID()] = metadata;
UpdateTimeFirstKey(nCreationTime);

if (!AddKeyPubKey(secret, pubkey))
if (!AddKeyPubKeyWithDB(walletdb, secret, pubkey)) {
throw std::runtime_error(std::string(__func__) + ": AddKey failed");
}
}
return pubkey;
}

void CWallet::DeriveNewChildKey(const CKeyMetadata& metadata, CKey& secretRet, uint32_t nAccountIndex, bool fInternal)
void CWallet::DeriveNewChildKey(CWalletDB &walletdb, const CKeyMetadata& metadata, CKey& secretRet, uint32_t nAccountIndex, bool fInternal)
{
CHDChain hdChainTmp;
if (!GetHDChain(hdChainTmp)) {
Expand Down Expand Up @@ -193,15 +195,15 @@ void CWallet::DeriveNewChildKey(const CKeyMetadata& metadata, CKey& secretRet, u
throw std::runtime_error(std::string(__func__) + ": SetAccount failed");

if (IsCrypted()) {
if (!SetCryptedHDChain(hdChainCurrent, false))
if (!SetCryptedHDChain(walletdb, hdChainCurrent, false))
throw std::runtime_error(std::string(__func__) + ": SetCryptedHDChain failed");
}
else {
if (!SetHDChain(hdChainCurrent, false))
if (!SetHDChain(walletdb, hdChainCurrent, false))
throw std::runtime_error(std::string(__func__) + ": SetHDChain failed");
Comment thread
UdjinM6 marked this conversation as resolved.
}

if (!AddHDPubKey(childKey.Neuter(), fInternal))
if (!AddHDPubKey(walletdb, childKey.Neuter(), fInternal))
throw std::runtime_error(std::string(__func__) + ": AddHDPubKey failed");
}

Expand Down Expand Up @@ -263,7 +265,7 @@ bool CWallet::LoadHDPubKey(const CHDPubKey &hdPubKey)
return true;
}

bool CWallet::AddHDPubKey(const CExtPubKey &extPubKey, bool fInternal)
bool CWallet::AddHDPubKey(CWalletDB &walletdb, const CExtPubKey &extPubKey, bool fInternal)
{
AssertLockHeld(cs_wallet);

Expand All @@ -285,32 +287,53 @@ bool CWallet::AddHDPubKey(const CExtPubKey &extPubKey, bool fInternal)
if (HaveWatchOnly(script))
RemoveWatchOnly(script);

return CWalletDB(*dbw).WriteHDPubKey(hdPubKey, mapKeyMetadata[extPubKey.pubkey.GetID()]);
return walletdb.WriteHDPubKey(hdPubKey, mapKeyMetadata[extPubKey.pubkey.GetID()]);
}

bool CWallet::AddKeyPubKey(const CKey& secret, const CPubKey &pubkey)
bool CWallet::AddKeyPubKeyWithDB(CWalletDB &walletdb, const CKey& secret, const CPubKey &pubkey)
{
AssertLockHeld(cs_wallet); // mapKeyMetadata
if (!CCryptoKeyStore::AddKeyPubKey(secret, pubkey))
return false;

// CCryptoKeyStore has no concept of wallet databases, but calls AddCryptedKey
// which is overridden below. To avoid flushes, the database handle is
// tunneled through to it.
bool needsDB = !pwalletdbEncryption;
if (needsDB) {
pwalletdbEncryption = &walletdb;
}
if (!CCryptoKeyStore::AddKeyPubKey(secret, pubkey)) {
if (needsDB) pwalletdbEncryption = NULL;
return false;
return false;
}
if (needsDB) pwalletdbEncryption = NULL;
// check if we need to remove from watch-only
CScript script;
script = GetScriptForDestination(pubkey.GetID());
if (HaveWatchOnly(script))
if (HaveWatchOnly(script)) {
RemoveWatchOnly(script);
}
script = GetScriptForRawPubKey(pubkey);
if (HaveWatchOnly(script))
if (HaveWatchOnly(script)) {
RemoveWatchOnly(script);
}

if (!IsCrypted()) {
return CWalletDB(*dbw).WriteKey(pubkey,
secret.GetPrivKey(),
mapKeyMetadata[pubkey.GetID()]);
return walletdb.WriteKey(pubkey,
secret.GetPrivKey(),
mapKeyMetadata[pubkey.GetID()]);
}
return true;
}


bool CWallet::AddKeyPubKey(const CKey& secret, const CPubKey &pubkey)
{
CWalletDB walletdb(*dbw);

return CWallet::AddKeyPubKeyWithDB(walletdb, secret, pubkey);
}

bool CWallet::AddCryptedKey(const CPubKey &vchPubKey,
const std::vector<unsigned char> &vchCryptedSecret)
{
Expand Down Expand Up @@ -802,7 +825,7 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase)
assert(hdChainCurrent.GetID() == hdChainCrypted.GetID());
assert(hdChainCurrent.GetSeedHash() != hdChainCrypted.GetSeedHash());

assert(SetCryptedHDChain(hdChainCrypted, false));
assert(SetCryptedHDChain(*pwalletdbEncryption, hdChainCrypted, false));
}

// Encryption was introduced in version 0.4.0
Expand Down Expand Up @@ -1598,29 +1621,29 @@ void CWallet::GenerateNewHDChain()
}
newHdChain.Debug(__func__);

if (!SetHDChain(newHdChain, false))
throw std::runtime_error(std::string(__func__) + ": SetHDChain failed");
if (!SetHDChainSingle(newHdChain, false))
throw std::runtime_error(std::string(__func__) + ": SetHDChainSingle failed");

// clean up
gArgs.ForceRemoveArg("-hdseed");
gArgs.ForceRemoveArg("-mnemonic");
gArgs.ForceRemoveArg("-mnemonicpassphrase");
}

bool CWallet::SetHDChain(const CHDChain& chain, bool memonly)
bool CWallet::SetHDChain(CWalletDB &walletdb, const CHDChain& chain, bool memonly)
{
LOCK(cs_wallet);

if (!CCryptoKeyStore::SetHDChain(chain))
return false;

if (!memonly && !CWalletDB(*dbw).WriteHDChain(chain))
if (!memonly && !walletdb.WriteHDChain(chain))
throw std::runtime_error(std::string(__func__) + ": WriteHDChain failed");

return true;
}

bool CWallet::SetCryptedHDChain(const CHDChain& chain, bool memonly)
bool CWallet::SetCryptedHDChain(CWalletDB &walletdb, const CHDChain& chain, bool memonly)
{
LOCK(cs_wallet);

Expand All @@ -1632,14 +1655,26 @@ bool CWallet::SetCryptedHDChain(const CHDChain& chain, bool memonly)
if (!pwalletdbEncryption->WriteCryptedHDChain(chain))
throw std::runtime_error(std::string(__func__) + ": WriteCryptedHDChain failed");
} else {
if (!CWalletDB(*dbw).WriteCryptedHDChain(chain))
if (!walletdb.WriteCryptedHDChain(chain))
throw std::runtime_error(std::string(__func__) + ": WriteCryptedHDChain failed");
}
}

return true;
}

bool CWallet::SetHDChainSingle(const CHDChain& chain, bool memonly)
{
CWalletDB walletdb(*dbw);
return SetHDChain(walletdb, chain, memonly);
}

bool CWallet::SetCryptedHDChainSingle(const CHDChain& chain, bool memonly)
{
CWalletDB walletdb(*dbw);
return SetCryptedHDChain(walletdb, chain, memonly);
}

bool CWallet::GetDecryptedHDChain(CHDChain& hdChainRet)
{
LOCK(cs_wallet);
Expand Down Expand Up @@ -4206,15 +4241,21 @@ bool CWallet::TopUpKeyPool(unsigned int kpSize)
nEnd = std::max(nEnd, *(--setExternalKeyPool.end()) + 1);
}
// TODO: implement keypools for all accounts?
if (!walletdb.WritePool(nEnd, CKeyPool(GenerateNewKey(0, fInternal), fInternal)))
if (!walletdb.WritePool(nEnd, CKeyPool(GenerateNewKey(walletdb, 0, fInternal), fInternal))) {
throw std::runtime_error(std::string(__func__) + ": writing generated key failed");
}

if (fInternal) {
setInternalKeyPool.insert(nEnd);
} else {
setExternalKeyPool.insert(nEnd);
}
LogPrintf("keypool added key %d, size=%u, internal=%d\n", nEnd, setInternalKeyPool.size() + setExternalKeyPool.size(), fInternal);

if (missingInternal + missingExternal > 0) {
LogPrintf("keypool added %d keys (%d internal), size=%u (%u internal)\n",
missingInternal + missingExternal, missingInternal,
setInternalKeyPool.size() + setExternalKeyPool.size(), setInternalKeyPool.size());
}

double dProgress = 100.f * nEnd / (nTargetSize + 1);
std::string strMsg = strprintf(_("Loading wallet... (%3.2f %%)"), dProgress);
Expand Down Expand Up @@ -4285,18 +4326,20 @@ void CWallet::ReturnKey(int64_t nIndex, bool fInternal)
LogPrintf("keypool return %d\n", nIndex);
}

bool CWallet::GetKeyFromPool(CPubKey& result, bool fInternal)
bool CWallet::GetKeyFromPool(CPubKey& result, bool internal)
{
CKeyPool keypool;
{
LOCK(cs_wallet);
int64_t nIndex = 0;
ReserveKeyFromKeyPool(nIndex, keypool, fInternal);
ReserveKeyFromKeyPool(nIndex, keypool, internal);
if (nIndex == -1)
{
if (IsLocked(true)) return false;
// TODO: implement keypool for all accouts?
result = GenerateNewKey(0, fInternal);

CWalletDB walletdb(*dbw);
result = GenerateNewKey(walletdb, 0, internal);
return true;
}
KeepKey(nIndex);
Expand Down
18 changes: 13 additions & 5 deletions src/wallet/wallet.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
void SyncTransaction(const CTransactionRef& tx, const CBlockIndex *pindex = NULL, int posInBlock = 0);

/* HD derive new child key (on internal or external chain) */
void DeriveNewChildKey(const CKeyMetadata& metadata, CKey& secretRet, uint32_t nAccountIndex, bool fInternal /*= false*/);
void DeriveNewChildKey(CWalletDB &walletdb, const CKeyMetadata& metadata, CKey& secretRet, uint32_t nAccountIndex, bool fInternal /*= false*/);

std::set<int64_t> setInternalKeyPool;
std::set<int64_t> setExternalKeyPool;
Expand Down Expand Up @@ -949,19 +949,20 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
* keystore implementation
* Generate a new key
*/
CPubKey GenerateNewKey(uint32_t nAccountIndex, bool fInternal /*= false*/);
CPubKey GenerateNewKey(CWalletDB& walletdb, uint32_t nAccountIndex, bool fInternal /*= false*/);
//! HaveKey implementation that also checks the mapHdPubKeys
bool HaveKey(const CKeyID &address) const override;
//! GetPubKey implementation that also checks the mapHdPubKeys
bool GetPubKey(const CKeyID &address, CPubKey& vchPubKeyOut) const override;
//! GetKey implementation that can derive a HD private key on the fly
bool GetKey(const CKeyID &address, CKey& keyOut) const override;
//! Adds a HDPubKey into the wallet(database)
bool AddHDPubKey(const CExtPubKey &extPubKey, bool fInternal);
bool AddHDPubKey(CWalletDB &walletdb, const CExtPubKey &extPubKey, bool fInternal);
//! loads a HDPubKey into the wallets memory
bool LoadHDPubKey(const CHDPubKey &hdPubKey);
//! Adds a key to the store, and saves it to disk.
bool AddKeyPubKey(const CKey& key, const CPubKey &pubkey) override;
bool AddKeyPubKeyWithDB(CWalletDB &walletdb, const CKey& key, const CPubKey &pubkey);
//! Adds a key to the store, without saving it to disk (used by LoadWallet)
bool LoadKey(const CKey& key, const CPubKey &pubkey) { return CCryptoKeyStore::AddKeyPubKey(key, pubkey); }
//! Load metadata (used by LoadWallet)
Expand Down Expand Up @@ -1232,8 +1233,15 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
/* Generates a new HD chain */
void GenerateNewHDChain();
/* Set the HD chain model (chain child index counters) */
bool SetHDChain(const CHDChain& chain, bool memonly);
bool SetCryptedHDChain(const CHDChain& chain, bool memonly);
bool SetHDChain(CWalletDB &walletdb, const CHDChain& chain, bool memonly);
bool SetCryptedHDChain(CWalletDB &walletdb, const CHDChain& chain, bool memonly);
/**
* Set the HD chain model (chain child index counters) using temporary wallet db object
* which causes db flush every time these methods are used
*/
bool SetHDChainSingle(const CHDChain& chain, bool memonly);
bool SetCryptedHDChainSingle(const CHDChain& chain, bool memonly);

bool GetDecryptedHDChain(CHDChain& hdChainRet);

void NotifyTransactionLock(const CTransaction &tx, const llmq::CInstantSendLock& islock) override;
Expand Down
4 changes: 2 additions & 2 deletions src/wallet/walletdb.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
{
CHDChain chain;
ssValue >> chain;
if (!pwallet->SetHDChain(chain, true))
if (!pwallet->SetHDChainSingle(chain, true))
{
strErr = "Error reading wallet database: SetHDChain failed";
return false;
Expand All @@ -512,7 +512,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
{
CHDChain chain;
ssValue >> chain;
if (!pwallet->SetCryptedHDChain(chain, true))
if (!pwallet->SetCryptedHDChainSingle(chain, true))
{
strErr = "Error reading wallet database: SetHDCryptedChain failed";
return false;
Expand Down