wallet: IsMine overloads require cs_wallet lock

This commit is contained in:
João Barbosa 2020-06-15 23:03:17 +01:00
parent a13cafc6c6
commit d8441f30ff
3 changed files with 37 additions and 27 deletions

View File

@ -37,6 +37,7 @@ namespace {
//! Construct wallet tx struct.
WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx)
{
LOCK(wallet.cs_wallet);
WalletTx result;
result.tx = wtx.tx;
result.txin_is_mine.reserve(wtx.tx->vin.size());
@ -132,7 +133,11 @@ public:
{
return m_wallet->SignMessage(message, pkhash, str_sig);
}
bool isSpendable(const CTxDestination& dest) override { return m_wallet->IsMine(dest) & ISMINE_SPENDABLE; }
bool isSpendable(const CTxDestination& dest) override
{
LOCK(m_wallet->cs_wallet);
return m_wallet->IsMine(dest) & ISMINE_SPENDABLE;
}
bool haveWatchOnly() override
{
auto spk_man = m_wallet->GetLegacyScriptPubKeyMan();

View File

@ -1210,15 +1210,13 @@ void CWallet::BlockUntilSyncedToCurrentChain() const {
isminetype CWallet::IsMine(const CTxIn &txin) const
{
AssertLockHeld(cs_wallet);
std::map<uint256, CWalletTx>::const_iterator mi = mapWallet.find(txin.prevout.hash);
if (mi != mapWallet.end())
{
LOCK(cs_wallet);
std::map<uint256, CWalletTx>::const_iterator mi = mapWallet.find(txin.prevout.hash);
if (mi != mapWallet.end())
{
const CWalletTx& prev = (*mi).second;
if (txin.prevout.n < prev.tx->vout.size())
return IsMine(prev.tx->vout[txin.prevout.n]);
}
const CWalletTx& prev = (*mi).second;
if (txin.prevout.n < prev.tx->vout.size())
return IsMine(prev.tx->vout[txin.prevout.n]);
}
return ISMINE_NO;
}
@ -1243,16 +1241,19 @@ CAmount CWallet::GetDebit(const CTxIn &txin, const isminefilter& filter) const
isminetype CWallet::IsMine(const CTxOut& txout) const
{
AssertLockHeld(cs_wallet);
return IsMine(txout.scriptPubKey);
}
isminetype CWallet::IsMine(const CTxDestination& dest) const
{
AssertLockHeld(cs_wallet);
return IsMine(GetScriptForDestination(dest));
}
isminetype CWallet::IsMine(const CScript& script) const
{
AssertLockHeld(cs_wallet);
isminetype result = ISMINE_NO;
for (const auto& spk_man_pair : m_spk_managers) {
result = std::max(result, spk_man_pair.second->IsMine(script));
@ -1264,6 +1265,7 @@ CAmount CWallet::GetCredit(const CTxOut& txout, const isminefilter& filter) cons
{
if (!MoneyRange(txout.nValue))
throw std::runtime_error(std::string(__func__) + ": value out of range");
LOCK(cs_wallet);
return ((IsMine(txout) & filter) ? txout.nValue : 0);
}
@ -1281,13 +1283,12 @@ bool CWallet::IsChange(const CScript& script) const
// a better way of identifying which outputs are 'the send' and which are
// 'the change' will need to be implemented (maybe extend CWalletTx to remember
// which output, if any, was change).
LOCK(cs_wallet);
if (IsMine(script))
{
CTxDestination address;
if (!ExtractDestination(script, address))
return true;
LOCK(cs_wallet);
if (!FindAddressBookEntry(address)) {
return true;
}
@ -1304,6 +1305,7 @@ CAmount CWallet::GetChange(const CTxOut& txout) const
bool CWallet::IsMine(const CTransaction& tx) const
{
AssertLockHeld(cs_wallet);
for (const CTxOut& txout : tx.vout)
if (IsMine(txout))
return true;
@ -1597,6 +1599,7 @@ void CWalletTx::GetAmounts(std::list<COutputEntry>& listReceived,
nFee = nDebit - nValueOut;
}
LOCK(pwallet->cs_wallet);
// Sent/received.
for (unsigned int i = 0; i < tx->vout.size(); ++i)
{
@ -3155,6 +3158,7 @@ DBErrors CWallet::ZapWalletTx(std::list<CWalletTx>& vWtx)
bool CWallet::SetAddressBookWithDB(WalletBatch& batch, const CTxDestination& address, const std::string& strName, const std::string& strPurpose)
{
bool fUpdated = false;
bool is_mine;
{
LOCK(cs_wallet);
std::map<CTxDestination, CAddressBookData>::iterator mi = m_address_book.find(address);
@ -3162,8 +3166,9 @@ bool CWallet::SetAddressBookWithDB(WalletBatch& batch, const CTxDestination& add
m_address_book[address].SetLabel(strName);
if (!strPurpose.empty()) /* update purpose only if requested */
m_address_book[address].purpose = strPurpose;
is_mine = IsMine(address) != ISMINE_NO;
}
NotifyAddressBookChanged(this, address, strName, IsMine(address) != ISMINE_NO,
NotifyAddressBookChanged(this, address, strName, is_mine,
strPurpose, (fUpdated ? CT_UPDATED : CT_NEW) );
if (!strPurpose.empty() && !batch.WritePurpose(EncodeDestination(address), strPurpose))
return false;
@ -3178,17 +3183,16 @@ bool CWallet::SetAddressBook(const CTxDestination& address, const std::string& s
bool CWallet::DelAddressBook(const CTxDestination& address)
{
// 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)
// NOTE: This isn't a problem for sending addresses because they never have any DestData yet!
// When adding new DestData, it should be considered here whether to retain or delete it (or move it?).
if (IsMine(address)) {
WalletLogPrintf("%s called with IsMine address, NOT SUPPORTED. Please report this bug! %s\n", __func__, PACKAGE_BUGREPORT);
return false;
}
bool is_mine;
{
LOCK(cs_wallet);
// 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)
// NOTE: This isn't a problem for sending addresses because they never have any DestData yet!
// When adding new DestData, it should be considered here whether to retain or delete it (or move it?).
if (IsMine(address)) {
WalletLogPrintf("%s called with IsMine address, NOT SUPPORTED. Please report this bug! %s\n", __func__, PACKAGE_BUGREPORT);
return false;
}
// Delete destdata tuples associated with address
std::string strAddress = EncodeDestination(address);
for (const std::pair<const std::string, std::string> &item : m_address_book[address].destdata)
@ -3196,9 +3200,10 @@ bool CWallet::DelAddressBook(const CTxDestination& address)
WalletBatch(*database).EraseDestData(strAddress, item.first);
}
m_address_book.erase(address);
is_mine = IsMine(address) != ISMINE_NO;
}
NotifyAddressBookChanged(this, address, "", IsMine(address) != ISMINE_NO, "", CT_DELETED);
NotifyAddressBookChanged(this, address, "", is_mine, "", CT_DELETED);
WalletBatch(*database).ErasePurpose(EncodeDestination(address));
return WalletBatch(*database).EraseName(EncodeDestination(address));

View File

@ -1038,20 +1038,20 @@ public:
bool GetNewDestination(const OutputType type, const std::string label, CTxDestination& dest, std::string& error);
bool GetNewChangeDestination(const OutputType type, CTxDestination& dest, std::string& error);
isminetype IsMine(const CTxDestination& dest) const;
isminetype IsMine(const CScript& script) const;
isminetype IsMine(const CTxIn& txin) const;
isminetype IsMine(const CTxDestination& dest) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
isminetype IsMine(const CScript& script) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
isminetype IsMine(const CTxIn& txin) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
/**
* Returns amount of debit if the input matches the
* filter, otherwise returns 0
*/
CAmount GetDebit(const CTxIn& txin, const isminefilter& filter) const;
isminetype IsMine(const CTxOut& txout) const;
isminetype IsMine(const CTxOut& txout) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
CAmount GetCredit(const CTxOut& txout, const isminefilter& filter) const;
bool IsChange(const CTxOut& txout) const;
bool IsChange(const CScript& script) const;
CAmount GetChange(const CTxOut& txout) const;
bool IsMine(const CTransaction& tx) const;
bool IsMine(const CTransaction& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
/** should probably be renamed to IsRelevantToMe */
bool IsFromMe(const CTransaction& tx) const;
CAmount GetDebit(const CTransaction& tx, const isminefilter& filter) const;