Skip to content

Commit a162edf

Browse files
committed
Merge bitcoin/bitcoin#22359: wallet: Do not set fInMempool in transactionAddedToMempool when tx is not in the mempool
fa6fd3d 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 fa4e088). Avoid setting it back to true (incorrectly) in the validation interface background thread. Fixes #22357 ACKs for top commit: ryanofsky: Code review ACK fa6fd3d. 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 fa6fd3d Tree-SHA512: d690136a577f1f532aa1fee80d3f6600ff7fc61286fbf564a53d7938d5ae52d33f0dbb0fef8b8c041a4970fb424f0b9f1ee7ce791e0ff8354e0000ecc9e22b84
2 parents 8fa03c4 + fa6fd3d commit a162edf

File tree

1 file changed

+13
-6
lines changed

1 file changed

+13
-6
lines changed

src/wallet/wallet.cpp

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,16 @@ static void UpdateWalletSetting(interfaces::Chain& chain,
9494
}
9595
}
9696

97+
/**
98+
* Refresh mempool status so the wallet is in an internally consistent state and
99+
* immediately knows the transaction's status: Whether it can be considered
100+
* trusted and is eligible to be abandoned ...
101+
*/
102+
static void RefreshMempoolStatus(CWalletTx& tx, interfaces::Chain& chain)
103+
{
104+
tx.fInMempool = chain.isInMempool(tx.GetHash());
105+
}
106+
97107
bool AddWallet(const std::shared_ptr<CWallet>& wallet)
98108
{
99109
LOCK(cs_wallets);
@@ -803,10 +813,7 @@ bool CWallet::MarkReplaced(const uint256& originalHash, const uint256& newHash)
803813
wtx.mapValue["replaced_by_txid"] = newHash.ToString();
804814

805815
// Refresh mempool status without waiting for transactionRemovedFromMempool
806-
// notification so the wallet is in an internally consistent state and
807-
// immediately knows the old transaction should not be considered trusted
808-
// and is eligible to be abandoned
809-
wtx.fInMempool = chain().isInMempool(originalHash);
816+
RefreshMempoolStatus(wtx, chain());
810817

811818
WalletBatch batch(GetDatabase());
812819

@@ -1206,15 +1213,15 @@ void CWallet::transactionAddedToMempool(const CTransactionRef& tx, uint64_t memp
12061213

12071214
auto it = mapWallet.find(tx->GetHash());
12081215
if (it != mapWallet.end()) {
1209-
it->second.fInMempool = true;
1216+
RefreshMempoolStatus(it->second, chain());
12101217
}
12111218
}
12121219

12131220
void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) {
12141221
LOCK(cs_wallet);
12151222
auto it = mapWallet.find(tx->GetHash());
12161223
if (it != mapWallet.end()) {
1217-
it->second.fInMempool = false;
1224+
RefreshMempoolStatus(it->second, chain());
12181225
}
12191226
// Handle transactions that were removed from the mempool because they
12201227
// conflict with transactions in a newly connected block.

0 commit comments

Comments
 (0)