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

fa6fd3dd6a4e7f30eff5963836aed43fe01af078 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 fa4e088cbac035b8029a10b492849540150d0622).

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

  Fixes #22357

ACKs for top commit:
  ryanofsky:
    Code review ACK fa6fd3dd6a4e7f30eff5963836aed43fe01af078. 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 fa6fd3dd6a4e7f30eff5963836aed43fe01af078

Tree-SHA512: d690136a577f1f532aa1fee80d3f6600ff7fc61286fbf564a53d7938d5ae52d33f0dbb0fef8b8c041a4970fb424f0b9f1ee7ce791e0ff8354e0000ecc9e22b84

1 files changed, 13 insertions, 6 deletions

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 741540f724..7291f1d378 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -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.