Skip to content

Commit e53615b

Browse files
achow101achow101
committed
Remove vchDefaultKey and have better first run detection
Removes vchDefaultKey which was only used for first run detection. Improves wallet first run detection by checking to see if any keys were read from the database. This will now also check for a valid defaultkey for backwards compatibility reasons and to check for any corruption. Keys will stil be generated on the first one, but there won't be any shown in the address book as was previously done.
1 parent ae47724 commit e53615b

File tree

7 files changed

+23
-38
lines changed

7 files changed

+23
-38
lines changed

‎src/wallet/crypter.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,6 @@ friend class wallet_crypto::TestCrypter; // for test access to chKey/chIV
113113
class CCryptoKeyStore : public CBasicKeyStore
114114
{
115115
private:
116-
CryptedKeyMap mapCryptedKeys;
117116

118117
CKeyingMaterial vMasterKey;
119118

@@ -131,6 +130,7 @@ class CCryptoKeyStore : public CBasicKeyStore
131130
bool EncryptKeys(CKeyingMaterial& vMasterKeyIn);
132131

133132
bool Unlock(const CKeyingMaterial& vMasterKeyIn);
133+
CryptedKeyMap mapCryptedKeys;
134134

135135
public:
136136
CCryptoKeyStore() : fUseCrypto(false), fDecryptionThoroughlyChecked(false)

‎src/wallet/wallet.cpp‎

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3114,9 +3114,11 @@ DBErrors CWallet::LoadWallet(bool& fFirstRunRet)
31143114
}
31153115
}
31163116

3117+
// This wallet is in its first run if all of these are empty
3118+
fFirstRunRet = mapKeys.empty() && mapCryptedKeys.empty() && mapWatchKeys.empty() && setWatchOnly.empty() && mapScripts.empty();
3119+
31173120
if (nLoadWalletRet != DB_LOAD_OK)
31183121
return nLoadWalletRet;
3119-
fFirstRunRet = !vchDefaultKey.IsValid();
31203122

31213123
uiInterface.LoadWallet(this);
31223124

@@ -3126,7 +3128,6 @@ DBErrors CWallet::LoadWallet(bool& fFirstRunRet)
31263128
DBErrors CWallet::ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256>& vHashOut)
31273129
{
31283130
AssertLockHeld(cs_wallet); // mapWallet
3129-
vchDefaultKey = CPubKey();
31303131
DBErrors nZapSelectTxRet = CWalletDB(*dbw,"cr+").ZapSelectTx(vHashIn, vHashOut);
31313132
for (uint256 hash : vHashOut)
31323133
mapWallet.erase(hash);
@@ -3155,7 +3156,6 @@ DBErrors CWallet::ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256
31553156

31563157
DBErrors CWallet::ZapWalletTx(std::vector<CWalletTx>& vWtx)
31573158
{
3158-
vchDefaultKey = CPubKey();
31593159
DBErrors nZapWalletTxRet = CWalletDB(*dbw,"cr+").ZapWalletTx(vWtx);
31603160
if (nZapWalletTxRet == DB_NEED_REWRITE)
31613161
{
@@ -3231,14 +3231,6 @@ const std::string& CWallet::GetAccountName(const CScript& scriptPubKey) const
32313231
return DEFAULT_ACCOUNT_NAME;
32323232
}
32333233

3234-
bool CWallet::SetDefaultKey(const CPubKey &vchPubKey)
3235-
{
3236-
if (!CWalletDB(*dbw).WriteDefaultKey(vchPubKey))
3237-
return false;
3238-
vchDefaultKey = vchPubKey;
3239-
return true;
3240-
}
3241-
32423234
/**
32433235
* Mark old keypool keys as used,
32443236
* and generate all new keys
@@ -4014,13 +4006,11 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile)
40144006
if (!walletInstance->SetHDMasterKey(masterPubKey))
40154007
throw std::runtime_error(std::string(__func__) + ": Storing master key failed");
40164008
}
4017-
CPubKey newDefaultKey;
4018-
if (walletInstance->GetKeyFromPool(newDefaultKey, false)) {
4019-
walletInstance->SetDefaultKey(newDefaultKey);
4020-
if (!walletInstance->SetAddressBook(walletInstance->vchDefaultKey.GetID(), "", "receive")) {
4021-
InitError(_("Cannot write default address") += "\n");
4022-
return nullptr;
4023-
}
4009+
4010+
// Top up the keypool
4011+
if (!walletInstance->TopUpKeyPool()) {
4012+
InitError(_("Unable to generate initial keys") += "\n");
4013+
return NULL;
40244014
}
40254015

40264016
walletInstance->SetBestChain(chainActive.GetLocator());

‎src/wallet/wallet.h‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -807,8 +807,6 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
807807

808808
std::map<CTxDestination, CAddressBookData> mapAddressBook;
809809

810-
CPubKey vchDefaultKey;
811-
812810
std::set<COutPoint> setLockedCoins;
813811

814812
const CWalletTx* GetWalletTx(const uint256& hash) const;
@@ -1040,8 +1038,6 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
10401038
return setInternalKeyPool.size() + setExternalKeyPool.size();
10411039
}
10421040

1043-
bool SetDefaultKey(const CPubKey &vchPubKey);
1044-
10451041
//! signify that a particular wallet feature is now used. this may change nWalletVersion and nWalletMaxVersion if those are lower
10461042
bool SetMinVersion(enum WalletFeature, CWalletDB* pwalletdbIn = nullptr, bool fExplicit = false);
10471043

‎src/wallet/walletdb.cpp‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -130,11 +130,6 @@ bool CWalletDB::WriteOrderPosNext(int64_t nOrderPosNext)
130130
return WriteIC(std::string("orderposnext"), nOrderPosNext);
131131
}
132132

133-
bool CWalletDB::WriteDefaultKey(const CPubKey& vchPubKey)
134-
{
135-
return WriteIC(std::string("defaultkey"), vchPubKey);
136-
}
137-
138133
bool CWalletDB::ReadPool(int64_t nPool, CKeyPool& keypool)
139134
{
140135
return batch.Read(std::make_pair(std::string("pool"), nPool), keypool);
@@ -452,7 +447,14 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
452447
}
453448
else if (strType == "defaultkey")
454449
{
455-
ssValue >> pwallet->vchDefaultKey;
450+
// We don't want or need the default key, but if there is one set,
451+
// we want to make sure that it is valid so that we can detect corruption
452+
CPubKey vchPubKey;
453+
ssValue >> vchPubKey;
454+
if (!vchPubKey.IsValid()) {
455+
strErr = "Error reading wallet database: Default Key corrupt";
456+
return false;
457+
}
456458
}
457459
else if (strType == "pool")
458460
{
@@ -522,7 +524,6 @@ bool CWalletDB::IsKeyType(const std::string& strType)
522524

523525
DBErrors CWalletDB::LoadWallet(CWallet* pwallet)
524526
{
525-
pwallet->vchDefaultKey = CPubKey();
526527
CWalletScanState wss;
527528
bool fNoncriticalErrors = false;
528529
DBErrors result = DB_LOAD_OK;
@@ -565,7 +566,7 @@ DBErrors CWalletDB::LoadWallet(CWallet* pwallet)
565566
{
566567
// losing keys is considered a catastrophic error, anything else
567568
// we assume the user can live with:
568-
if (IsKeyType(strType))
569+
if (IsKeyType(strType) || strType == "defaultkey")
569570
result = DB_CORRUPT;
570571
else
571572
{

‎src/wallet/walletdb.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,6 @@ class CWalletDB
191191

192192
bool WriteOrderPosNext(int64_t nOrderPosNext);
193193

194-
bool WriteDefaultKey(const CPubKey& vchPubKey);
195-
196194
bool ReadPool(int64_t nPool, CKeyPool& keypool);
197195
bool WritePool(int64_t nPool, const CKeyPool& keypool);
198196
bool ErasePool(int64_t nPool);

‎test/functional/keypool-topup.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ def run_test(self):
6969
assert_equal(self.nodes[1].listtransactions()[0]['category'], "receive")
7070

7171
# Check that we have marked all keys up to the used keypool key as used
72-
assert_equal(self.nodes[1].validateaddress(self.nodes[1].getnewaddress())['hdkeypath'], "m/0'/0'/111'")
72+
assert_equal(self.nodes[1].validateaddress(self.nodes[1].getnewaddress())['hdkeypath'], "m/0'/0'/110'")
7373

7474
if __name__ == '__main__':
7575
KeypoolRestoreTest().main()

‎test/functional/wallet-hd.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ def run_test (self):
4242
non_hd_add = self.nodes[0].getnewaddress()
4343
self.nodes[1].importprivkey(self.nodes[0].dumpprivkey(non_hd_add))
4444

45-
# This should be enough to keep the master key and the non-HD key
45+
# This should be enough to keep the master key and the non-HD key
4646
self.nodes[1].backupwallet(tmpdir + "/hd.bak")
4747
#self.nodes[1].dumpwallet(tmpdir + "/hd.dump")
4848

@@ -54,7 +54,7 @@ def run_test (self):
5454
for i in range(num_hd_adds):
5555
hd_add = self.nodes[1].getnewaddress()
5656
hd_info = self.nodes[1].validateaddress(hd_add)
57-
assert_equal(hd_info["hdkeypath"], "m/0'/0'/"+str(i+1)+"'")
57+
assert_equal(hd_info["hdkeypath"], "m/0'/0'/"+str(i)+"'")
5858
assert_equal(hd_info["hdmasterkeyid"], masterkeyid)
5959
self.nodes[0].sendtoaddress(hd_add, 1)
6060
self.nodes[0].generate(1)
@@ -83,7 +83,7 @@ def run_test (self):
8383
for _ in range(num_hd_adds):
8484
hd_add_2 = self.nodes[1].getnewaddress()
8585
hd_info_2 = self.nodes[1].validateaddress(hd_add_2)
86-
assert_equal(hd_info_2["hdkeypath"], "m/0'/0'/"+str(_+1)+"'")
86+
assert_equal(hd_info_2["hdkeypath"], "m/0'/0'/"+str(_)+"'")
8787
assert_equal(hd_info_2["hdmasterkeyid"], masterkeyid)
8888
assert_equal(hd_add, hd_add_2)
8989
connect_nodes_bi(self.nodes, 0, 1)
@@ -101,7 +101,7 @@ def run_test (self):
101101
for out in outs:
102102
if out['value'] != 1:
103103
keypath = self.nodes[1].validateaddress(out['scriptPubKey']['addresses'][0])['hdkeypath']
104-
104+
105105
assert_equal(keypath[0:7], "m/0'/1'")
106106

107107
if __name__ == '__main__':

0 commit comments

Comments
 (0)