wallet: fix abandon crash on inconsistent state #35115

Fixes #34599.

CWallet::AbandonTransaction walks the abandoned tx's descendants via mapTxSpends and asserts that none of them is confirmed or in the mempool. That holds at the consensus level — if the parent isn't in the chain, no child can be — but the assertion runs against the wallet's in-memory per-tx state, which can transiently diverge:

• during reorgs, where per-tx state transitions are processed individually and intermediate states are observable;
• around RBF candidates, where multiple wallet wtxs share an input outpoint via mapTxSpends and so end up linked to the same parent output even though only one of them is the "real" descendant.

When the divergence is hit, the node aborts. The fix replaces the two assert calls with a TxUpdate::UNCHANGED early-return so the recursion safely halts at the inconsistent descendant instead of crashing — preserving the original intent ("don't abandon something that's confirmed or in mempool") and matching the no-assert pattern already used by the sibling lambdas in MarkConflicted, transactionAddedToMempool and transactionRemovedFromMempool.

A new wallet_tests/abandon_with_confirmed_descendant unit test synthesises the inconsistency (parent TxStateInactive, child TxStateConfirmed linked via mapTxSpends) and exercises the assertion site directly: pre-fix it aborts, post-fix AbandonTransaction returns true, marks the parent abandoned and leaves the child untouched.

Precedent: PR #31757 / commit 9ef429b6ae fixed an analogous wallet assertion crash on double block disconnection by handling the inconsistent-state case rather than relying on the assertion.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

There is a root issue that is not being handled in this pr which is that a child transaction cannot be confirmed if the parent is unconfirmed. This may solve the specific assertion failure here but it leaves the transactions in inconsistent state in the wallet

Thanks for reading. You're right that on-chain a child can't be confirmed while its parent is unconfirmed, but I think the "inconsistent state" here is narrower than it looks.

mapTxSpends is a std::unordered_multimap<COutPoint, Txid> that records every wtx claiming a given outpoint, not the canonical on-chain spend. An RBF pair (original + replacement) both live under the same input outpoint key, so the recursion in AbandonTransaction walks both — and when the replacement is confirmed while the original is TxStateBlockConflicted (which is exactly what MarkConflicted leaves behind after the replacement's block is connected in AddToWalletIfInvolvingMe), abandoning the original walks into a confirmed wtx and aborts the node. No per-wtx state is inconsistent in that moment — each wtx has a valid state — only the mapTxSpends-derived linkage looks like a parent→child edge when it is really a shared-input RBF edge.

The early-return halts the walk at that descendant without mutating it, which is the same shape the sibling descendant-walk lambdas in MarkConflicted, transactionAddedToMempool and transactionRemovedFromMempool already use. The precedent for tolerating rather than asserting on this kind of transient wallet-state observation is PR #31757 / 9ef429b6ae, which fixed an analogous crash on double block disconnection.

A root-cause fix would mean either making mapTxSpends logical (so RBF siblings don't collide) or reworking descendant discovery to cross-check against per-wtx state before recursing — both are substantially larger than the crash warrants, and neither is done by the sibling lambdas today. Happy to revisit if you see a specific reachable state where a per-wtx field ends up wrong as a result of this change.