Merge bitcoin/bitcoin#22359: wallet: Do not set fInMempool in transactionAddedToMempool when tx is not in the mempool

fa6fd3dd6a wallet: Properly set fInMempool in mempool notifications (MarcoFalke)

Pull request description:

  A wallet method (like bumping the fee) might have set `fInMempool` to false because the transaction was removed from the mempool (See commit fa4e088cba).

  Avoid setting it back to true (incorrectly) in the validation interface background thread.

  Fixes #22357

ACKs for top commit:
  ryanofsky:
    Code review ACK fa6fd3dd6a. Only change since last review is extending workaround to `transactionRemovedFromMempool`. Since we know this workaround is imperfect and the goal of this PR is mainly to fix CI errors, I would probably be inclined to limit the workaround to as few places as possible where we have seen actual failures, instead of adding the workaround to as many places as possible, where there is some chance it might trigger new failures. But since this workaround is so straightforward and almost looks like a real fix, probably it doesn't matter.
  meshcollider:
    utACK fa6fd3dd6a

Tree-SHA512: d690136a577f1f532aa1fee80d3f6600ff7fc61286fbf564a53d7938d5ae52d33f0dbb0fef8b8c041a4970fb424f0b9f1ee7ce791e0ff8354e0000ecc9e22b84
This commit is contained in:
Samuel Dobson 2021-08-09 14:10:22 +12:00
commit a162edfdd1
No known key found for this signature in database
GPG key ID: D300116E1C875A3D

View file

@ -94,6 +94,16 @@ static void UpdateWalletSetting(interfaces::Chain& chain,
}
}
/**
* Refresh mempool status so the wallet is in an internally consistent state and
* immediately knows the transaction's status: Whether it can be considered
* trusted and is eligible to be abandoned ...
*/
static void RefreshMempoolStatus(CWalletTx& tx, interfaces::Chain& chain)
{
tx.fInMempool = chain.isInMempool(tx.GetHash());
}
bool AddWallet(const std::shared_ptr<CWallet>& wallet)
{
LOCK(cs_wallets);
@ -803,10 +813,7 @@ bool CWallet::MarkReplaced(const uint256& originalHash, const uint256& newHash)
wtx.mapValue["replaced_by_txid"] = newHash.ToString();
// Refresh mempool status without waiting for transactionRemovedFromMempool
// notification so the wallet is in an internally consistent state and
// immediately knows the old transaction should not be considered trusted
// and is eligible to be abandoned
wtx.fInMempool = chain().isInMempool(originalHash);
RefreshMempoolStatus(wtx, chain());
WalletBatch batch(GetDatabase());
@ -1206,7 +1213,7 @@ void CWallet::transactionAddedToMempool(const CTransactionRef& tx, uint64_t memp
auto it = mapWallet.find(tx->GetHash());
if (it != mapWallet.end()) {
it->second.fInMempool = true;
RefreshMempoolStatus(it->second, chain());
}
}
@ -1214,7 +1221,7 @@ void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRe
LOCK(cs_wallet);
auto it = mapWallet.find(tx->GetHash());
if (it != mapWallet.end()) {
it->second.fInMempool = false;
RefreshMempoolStatus(it->second, chain());
}
// Handle transactions that were removed from the mempool because they
// conflict with transactions in a newly connected block.