Skip to content

Commit d8441f3

Browse files
promagpromag
committed
wallet: IsMine overloads require cs_wallet lock
1 parent a13cafc commit d8441f3

3 files changed

Lines changed: 37 additions & 27 deletions

File tree

‎src/interfaces/wallet.cpp‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ namespace {
3737
//! Construct wallet tx struct.
3838
WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx)
3939
{
40+
LOCK(wallet.cs_wallet);
4041
WalletTx result;
4142
result.tx = wtx.tx;
4243
result.txin_is_mine.reserve(wtx.tx->vin.size());
@@ -132,7 +133,11 @@ class WalletImpl : public Wallet
132133
{
133134
return m_wallet->SignMessage(message, pkhash, str_sig);
134135
}
135-
bool isSpendable(const CTxDestination& dest) override { return m_wallet->IsMine(dest) & ISMINE_SPENDABLE; }
136+
bool isSpendable(const CTxDestination& dest) override
137+
{
138+
LOCK(m_wallet->cs_wallet);
139+
return m_wallet->IsMine(dest) & ISMINE_SPENDABLE;
140+
}
136141
bool haveWatchOnly() override
137142
{
138143
auto spk_man = m_wallet->GetLegacyScriptPubKeyMan();

‎src/wallet/wallet.cpp‎

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1210,15 +1210,13 @@ void CWallet::BlockUntilSyncedToCurrentChain() const {
12101210

12111211
isminetype CWallet::IsMine(const CTxIn &txin) const
12121212
{
1213+
AssertLockHeld(cs_wallet);
1214+
std::map<uint256, CWalletTx>::const_iterator mi = mapWallet.find(txin.prevout.hash);
1215+
if (mi != mapWallet.end())
12131216
{
1214-
LOCK(cs_wallet);
1215-
std::map<uint256, CWalletTx>::const_iterator mi = mapWallet.find(txin.prevout.hash);
1216-
if (mi != mapWallet.end())
1217-
{
1218-
const CWalletTx& prev = (*mi).second;
1219-
if (txin.prevout.n < prev.tx->vout.size())
1220-
return IsMine(prev.tx->vout[txin.prevout.n]);
1221-
}
1217+
const CWalletTx& prev = (*mi).second;
1218+
if (txin.prevout.n < prev.tx->vout.size())
1219+
return IsMine(prev.tx->vout[txin.prevout.n]);
12221220
}
12231221
return ISMINE_NO;
12241222
}
@@ -1243,16 +1241,19 @@ CAmount CWallet::GetDebit(const CTxIn &txin, const isminefilter& filter) const
12431241

12441242
isminetype CWallet::IsMine(const CTxOut& txout) const
12451243
{
1244+
AssertLockHeld(cs_wallet);
12461245
return IsMine(txout.scriptPubKey);
12471246
}
12481247

12491248
isminetype CWallet::IsMine(const CTxDestination& dest) const
12501249
{
1250+
AssertLockHeld(cs_wallet);
12511251
return IsMine(GetScriptForDestination(dest));
12521252
}
12531253

12541254
isminetype CWallet::IsMine(const CScript& script) const
12551255
{
1256+
AssertLockHeld(cs_wallet);
12561257
isminetype result = ISMINE_NO;
12571258
for (const auto& spk_man_pair : m_spk_managers) {
12581259
result = std::max(result, spk_man_pair.second->IsMine(script));
@@ -1264,6 +1265,7 @@ CAmount CWallet::GetCredit(const CTxOut& txout, const isminefilter& filter) cons
12641265
{
12651266
if (!MoneyRange(txout.nValue))
12661267
throw std::runtime_error(std::string(__func__) + ": value out of range");
1268+
LOCK(cs_wallet);
12671269
return ((IsMine(txout) & filter) ? txout.nValue : 0);
12681270
}
12691271

@@ -1281,13 +1283,12 @@ bool CWallet::IsChange(const CScript& script) const
12811283
// a better way of identifying which outputs are 'the send' and which are
12821284
// 'the change' will need to be implemented (maybe extend CWalletTx to remember
12831285
// which output, if any, was change).
1286+
LOCK(cs_wallet);
12841287
if (IsMine(script))
12851288
{
12861289
CTxDestination address;
12871290
if (!ExtractDestination(script, address))
12881291
return true;
1289-
1290-
LOCK(cs_wallet);
12911292
if (!FindAddressBookEntry(address)) {
12921293
return true;
12931294
}
@@ -1304,6 +1305,7 @@ CAmount CWallet::GetChange(const CTxOut& txout) const
13041305

13051306
bool CWallet::IsMine(const CTransaction& tx) const
13061307
{
1308+
AssertLockHeld(cs_wallet);
13071309
for (const CTxOut& txout : tx.vout)
13081310
if (IsMine(txout))
13091311
return true;
@@ -1597,6 +1599,7 @@ void CWalletTx::GetAmounts(std::list<COutputEntry>& listReceived,
15971599
nFee = nDebit - nValueOut;
15981600
}
15991601

1602+
LOCK(pwallet->cs_wallet);
16001603
// Sent/received.
16011604
for (unsigned int i = 0; i < tx->vout.size(); ++i)
16021605
{
@@ -3155,15 +3158,17 @@ DBErrors CWallet::ZapWalletTx(std::list<CWalletTx>& vWtx)
31553158
bool CWallet::SetAddressBookWithDB(WalletBatch& batch, const CTxDestination& address, const std::string& strName, const std::string& strPurpose)
31563159
{
31573160
bool fUpdated = false;
3161+
bool is_mine;
31583162
{
31593163
LOCK(cs_wallet);
31603164
std::map<CTxDestination, CAddressBookData>::iterator mi = m_address_book.find(address);
31613165
fUpdated = (mi != m_address_book.end() && !mi->second.IsChange());
31623166
m_address_book[address].SetLabel(strName);
31633167
if (!strPurpose.empty()) /* update purpose only if requested */
31643168
m_address_book[address].purpose = strPurpose;
3169+
is_mine = IsMine(address) != ISMINE_NO;
31653170
}
3166-
NotifyAddressBookChanged(this, address, strName, IsMine(address) != ISMINE_NO,
3171+
NotifyAddressBookChanged(this, address, strName, is_mine,
31673172
strPurpose, (fUpdated ? CT_UPDATED : CT_NEW) );
31683173
if (!strPurpose.empty() && !batch.WritePurpose(EncodeDestination(address), strPurpose))
31693174
return false;
@@ -3178,27 +3183,27 @@ bool CWallet::SetAddressBook(const CTxDestination& address, const std::string& s
31783183

31793184
bool CWallet::DelAddressBook(const CTxDestination& address)
31803185
{
3181-
// If we want to delete receiving addresses, we need to take care that DestData "used" (and possibly newer DestData) gets preserved (and the "deleted" address transformed into a change entry instead of actually being deleted)
3182-
// NOTE: This isn't a problem for sending addresses because they never have any DestData yet!
3183-
// When adding new DestData, it should be considered here whether to retain or delete it (or move it?).
3184-
if (IsMine(address)) {
3185-
WalletLogPrintf("%s called with IsMine address, NOT SUPPORTED. Please report this bug! %s\n", __func__, PACKAGE_BUGREPORT);
3186-
return false;
3187-
}
3188-
3186+
bool is_mine;
31893187
{
31903188
LOCK(cs_wallet);
3191-
3189+
// If we want to delete receiving addresses, we need to take care that DestData "used" (and possibly newer DestData) gets preserved (and the "deleted" address transformed into a change entry instead of actually being deleted)
3190+
// NOTE: This isn't a problem for sending addresses because they never have any DestData yet!
3191+
// When adding new DestData, it should be considered here whether to retain or delete it (or move it?).
3192+
if (IsMine(address)) {
3193+
WalletLogPrintf("%s called with IsMine address, NOT SUPPORTED. Please report this bug! %s\n", __func__, PACKAGE_BUGREPORT);
3194+
return false;
3195+
}
31923196
// Delete destdata tuples associated with address
31933197
std::string strAddress = EncodeDestination(address);
31943198
for (const std::pair<const std::string, std::string> &item : m_address_book[address].destdata)
31953199
{
31963200
WalletBatch(*database).EraseDestData(strAddress, item.first);
31973201
}
31983202
m_address_book.erase(address);
3203+
is_mine = IsMine(address) != ISMINE_NO;
31993204
}
32003205

3201-
NotifyAddressBookChanged(this, address, "", IsMine(address) != ISMINE_NO, "", CT_DELETED);
3206+
NotifyAddressBookChanged(this, address, "", is_mine, "", CT_DELETED);
32023207

32033208
WalletBatch(*database).ErasePurpose(EncodeDestination(address));
32043209
return WalletBatch(*database).EraseName(EncodeDestination(address));

‎src/wallet/wallet.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,20 +1038,20 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
10381038
bool GetNewDestination(const OutputType type, const std::string label, CTxDestination& dest, std::string& error);
10391039
bool GetNewChangeDestination(const OutputType type, CTxDestination& dest, std::string& error);
10401040

1041-
isminetype IsMine(const CTxDestination& dest) const;
1042-
isminetype IsMine(const CScript& script) const;
1043-
isminetype IsMine(const CTxIn& txin) const;
1041+
isminetype IsMine(const CTxDestination& dest) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
1042+
isminetype IsMine(const CScript& script) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
1043+
isminetype IsMine(const CTxIn& txin) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
10441044
/**
10451045
* Returns amount of debit if the input matches the
10461046
* filter, otherwise returns 0
10471047
*/
10481048
CAmount GetDebit(const CTxIn& txin, const isminefilter& filter) const;
1049-
isminetype IsMine(const CTxOut& txout) const;
1049+
isminetype IsMine(const CTxOut& txout) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
10501050
CAmount GetCredit(const CTxOut& txout, const isminefilter& filter) const;
10511051
bool IsChange(const CTxOut& txout) const;
10521052
bool IsChange(const CScript& script) const;
10531053
CAmount GetChange(const CTxOut& txout) const;
1054-
bool IsMine(const CTransaction& tx) const;
1054+
bool IsMine(const CTransaction& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
10551055
/** should probably be renamed to IsRelevantToMe */
10561056
bool IsFromMe(const CTransaction& tx) const;
10571057
CAmount GetDebit(const CTransaction& tx, const isminefilter& filter) const;

0 commit comments

Comments
 (0)