Skip to content

Commit f28eb80

Browse files
luke-jrluke-jr
committed
Bugfix: wallet: Increment "update counter" only after actually making the applicable db changes to avoid potential races
Also does all "update counter" access via IncrementUpdateCounter
1 parent 9fec4da commit f28eb80

File tree

2 files changed

+50
-49
lines changed

2 files changed

+50
-49
lines changed

‎src/wallet/walletdb.cpp‎

Lines changed: 29 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -32,47 +32,39 @@ static std::atomic<unsigned int> nWalletDBUpdateCounter;
3232

3333
bool CWalletDB::WriteName(const std::string& strAddress, const std::string& strName)
3434
{
35-
nWalletDBUpdateCounter++;
36-
return batch.Write(std::make_pair(std::string("name"), strAddress), strName);
35+
return WriteIC(std::make_pair(std::string("name"), strAddress), strName);
3736
}
3837

3938
bool CWalletDB::EraseName(const std::string& strAddress)
4039
{
4140
// This should only be used for sending addresses, never for receiving addresses,
4241
// receiving addresses must always have an address book entry if they're not change return.
43-
nWalletDBUpdateCounter++;
44-
return batch.Erase(std::make_pair(std::string("name"), strAddress));
42+
return EraseIC(std::make_pair(std::string("name"), strAddress));
4543
}
4644

4745
bool CWalletDB::WritePurpose(const std::string& strAddress, const std::string& strPurpose)
4846
{
49-
nWalletDBUpdateCounter++;
50-
return batch.Write(std::make_pair(std::string("purpose"), strAddress), strPurpose);
47+
return WriteIC(std::make_pair(std::string("purpose"), strAddress), strPurpose);
5148
}
5249

5350
bool CWalletDB::ErasePurpose(const std::string& strPurpose)
5451
{
55-
nWalletDBUpdateCounter++;
56-
return batch.Erase(std::make_pair(std::string("purpose"), strPurpose));
52+
return EraseIC(std::make_pair(std::string("purpose"), strPurpose));
5753
}
5854

5955
bool CWalletDB::WriteTx(const CWalletTx& wtx)
6056
{
61-
nWalletDBUpdateCounter++;
62-
return batch.Write(std::make_pair(std::string("tx"), wtx.GetHash()), wtx);
57+
return WriteIC(std::make_pair(std::string("tx"), wtx.GetHash()), wtx);
6358
}
6459

6560
bool CWalletDB::EraseTx(uint256 hash)
6661
{
67-
nWalletDBUpdateCounter++;
68-
return batch.Erase(std::make_pair(std::string("tx"), hash));
62+
return EraseIC(std::make_pair(std::string("tx"), hash));
6963
}
7064

7165
bool CWalletDB::WriteKey(const CPubKey& vchPubKey, const CPrivKey& vchPrivKey, const CKeyMetadata& keyMeta)
7266
{
73-
nWalletDBUpdateCounter++;
74-
75-
if (!batch.Write(std::make_pair(std::string("keymeta"), vchPubKey),
67+
if (!WriteIC(std::make_pair(std::string("keymeta"), vchPubKey),
7668
keyMeta, false))
7769
return false;
7870

@@ -82,63 +74,58 @@ bool CWalletDB::WriteKey(const CPubKey& vchPubKey, const CPrivKey& vchPrivKey, c
8274
vchKey.insert(vchKey.end(), vchPubKey.begin(), vchPubKey.end());
8375
vchKey.insert(vchKey.end(), vchPrivKey.begin(), vchPrivKey.end());
8476

85-
return batch.Write(std::make_pair(std::string("key"), vchPubKey), std::make_pair(vchPrivKey, Hash(vchKey.begin(), vchKey.end())), false);
77+
return WriteIC(std::make_pair(std::string("key"), vchPubKey), std::make_pair(vchPrivKey, Hash(vchKey.begin(), vchKey.end())), false);
8678
}
8779

8880
bool CWalletDB::WriteCryptedKey(const CPubKey& vchPubKey,
8981
const std::vector<unsigned char>& vchCryptedSecret,
9082
const CKeyMetadata &keyMeta)
9183
{
9284
const bool fEraseUnencryptedKey = true;
93-
nWalletDBUpdateCounter++;
9485

95-
if (!batch.Write(std::make_pair(std::string("keymeta"), vchPubKey),
86+
if (!WriteIC(std::make_pair(std::string("keymeta"), vchPubKey),
9687
keyMeta))
9788
return false;
9889

99-
if (!batch.Write(std::make_pair(std::string("ckey"), vchPubKey), vchCryptedSecret, false))
90+
if (!WriteIC(std::make_pair(std::string("ckey"), vchPubKey), vchCryptedSecret, false))
10091
return false;
10192
if (fEraseUnencryptedKey)
10293
{
103-
batch.Erase(std::make_pair(std::string("key"), vchPubKey));
104-
batch.Erase(std::make_pair(std::string("wkey"), vchPubKey));
94+
EraseIC(std::make_pair(std::string("key"), vchPubKey));
95+
EraseIC(std::make_pair(std::string("wkey"), vchPubKey));
10596
}
97+
10698
return true;
10799
}
108100

109101
bool CWalletDB::WriteMasterKey(unsigned int nID, const CMasterKey& kMasterKey)
110102
{
111-
nWalletDBUpdateCounter++;
112-
return batch.Write(std::make_pair(std::string("mkey"), nID), kMasterKey, true);
103+
return WriteIC(std::make_pair(std::string("mkey"), nID), kMasterKey, true);
113104
}
114105

115106
bool CWalletDB::WriteCScript(const uint160& hash, const CScript& redeemScript)
116107
{
117-
nWalletDBUpdateCounter++;
118-
return batch.Write(std::make_pair(std::string("cscript"), hash), *(const CScriptBase*)(&redeemScript), false);
108+
return WriteIC(std::make_pair(std::string("cscript"), hash), *(const CScriptBase*)(&redeemScript), false);
119109
}
120110

121111
bool CWalletDB::WriteWatchOnly(const CScript &dest, const CKeyMetadata& keyMeta)
122112
{
123-
nWalletDBUpdateCounter++;
124-
if (!batch.Write(std::make_pair(std::string("watchmeta"), *(const CScriptBase*)(&dest)), keyMeta))
113+
if (!WriteIC(std::make_pair(std::string("watchmeta"), *(const CScriptBase*)(&dest)), keyMeta))
125114
return false;
126-
return batch.Write(std::make_pair(std::string("watchs"), *(const CScriptBase*)(&dest)), '1');
115+
return WriteIC(std::make_pair(std::string("watchs"), *(const CScriptBase*)(&dest)), '1');
127116
}
128117

129118
bool CWalletDB::EraseWatchOnly(const CScript &dest)
130119
{
131-
nWalletDBUpdateCounter++;
132-
if (!batch.Erase(std::make_pair(std::string("watchmeta"), *(const CScriptBase*)(&dest))))
120+
if (!EraseIC(std::make_pair(std::string("watchmeta"), *(const CScriptBase*)(&dest))))
133121
return false;
134-
return batch.Erase(std::make_pair(std::string("watchs"), *(const CScriptBase*)(&dest)));
122+
return EraseIC(std::make_pair(std::string("watchs"), *(const CScriptBase*)(&dest)));
135123
}
136124

137125
bool CWalletDB::WriteBestBlock(const CBlockLocator& locator)
138126
{
139-
nWalletDBUpdateCounter++;
140-
batch.Write(std::string("bestblock"), CBlockLocator()); // Write empty block locator so versions that require a merkle branch automatically rescan
141-
return batch.Write(std::string("bestblock_nomerkle"), locator);
127+
WriteIC(std::string("bestblock"), CBlockLocator()); // Write empty block locator so versions that require a merkle branch automatically rescan
128+
return WriteIC(std::string("bestblock_nomerkle"), locator);
142129
}
143130

144131
bool CWalletDB::ReadBestBlock(CBlockLocator& locator)
@@ -149,14 +136,12 @@ bool CWalletDB::ReadBestBlock(CBlockLocator& locator)
149136

150137
bool CWalletDB::WriteOrderPosNext(int64_t nOrderPosNext)
151138
{
152-
nWalletDBUpdateCounter++;
153-
return batch.Write(std::string("orderposnext"), nOrderPosNext);
139+
return WriteIC(std::string("orderposnext"), nOrderPosNext);
154140
}
155141

156142
bool CWalletDB::WriteDefaultKey(const CPubKey& vchPubKey)
157143
{
158-
nWalletDBUpdateCounter++;
159-
return batch.Write(std::string("defaultkey"), vchPubKey);
144+
return WriteIC(std::string("defaultkey"), vchPubKey);
160145
}
161146

162147
bool CWalletDB::ReadPool(int64_t nPool, CKeyPool& keypool)
@@ -166,19 +151,17 @@ bool CWalletDB::ReadPool(int64_t nPool, CKeyPool& keypool)
166151

167152
bool CWalletDB::WritePool(int64_t nPool, const CKeyPool& keypool)
168153
{
169-
nWalletDBUpdateCounter++;
170-
return batch.Write(std::make_pair(std::string("pool"), nPool), keypool);
154+
return WriteIC(std::make_pair(std::string("pool"), nPool), keypool);
171155
}
172156

173157
bool CWalletDB::ErasePool(int64_t nPool)
174158
{
175-
nWalletDBUpdateCounter++;
176-
return batch.Erase(std::make_pair(std::string("pool"), nPool));
159+
return EraseIC(std::make_pair(std::string("pool"), nPool));
177160
}
178161

179162
bool CWalletDB::WriteMinVersion(int nVersion)
180163
{
181-
return batch.Write(std::string("minversion"), nVersion);
164+
return WriteIC(std::string("minversion"), nVersion);
182165
}
183166

184167
bool CWalletDB::ReadAccount(const std::string& strAccount, CAccount& account)
@@ -854,21 +837,18 @@ bool CWalletDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path
854837

855838
bool CWalletDB::WriteDestData(const std::string &address, const std::string &key, const std::string &value)
856839
{
857-
nWalletDBUpdateCounter++;
858-
return batch.Write(std::make_pair(std::string("destdata"), std::make_pair(address, key)), value);
840+
return WriteIC(std::make_pair(std::string("destdata"), std::make_pair(address, key)), value);
859841
}
860842

861843
bool CWalletDB::EraseDestData(const std::string &address, const std::string &key)
862844
{
863-
nWalletDBUpdateCounter++;
864-
return batch.Erase(std::make_pair(std::string("destdata"), std::make_pair(address, key)));
845+
return EraseIC(std::make_pair(std::string("destdata"), std::make_pair(address, key)));
865846
}
866847

867848

868849
bool CWalletDB::WriteHDChain(const CHDChain& chain)
869850
{
870-
nWalletDBUpdateCounter++;
871-
return batch.Write(std::string("hdchain"), chain);
851+
return WriteIC(std::string("hdchain"), chain);
872852
}
873853

874854
void CWalletDB::IncrementUpdateCounter()

‎src/wallet/walletdb.h‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,27 @@ class CKeyMetadata
140140
*/
141141
class CWalletDB
142142
{
143+
private:
144+
template <typename K, typename T>
145+
bool WriteIC(const K& key, const T& value, bool fOverwrite = true)
146+
{
147+
if (!batch.Write(key, value, fOverwrite)) {
148+
return false;
149+
}
150+
IncrementUpdateCounter();
151+
return true;
152+
}
153+
154+
template <typename K>
155+
bool EraseIC(const K& key)
156+
{
157+
if (!batch.Erase(key)) {
158+
return false;
159+
}
160+
IncrementUpdateCounter();
161+
return true;
162+
}
163+
143164
public:
144165
CWalletDB(CWalletDBWrapper& dbw, const char* pszMode = "r+", bool _fFlushOnClose = true) :
145166
batch(dbw, pszMode, _fFlushOnClose)

0 commit comments

Comments
 (0)