Do not un-mark fInMempool on wallet txn if ATMP fails. #11866

Irrespective of the failure reason, un-marking fInMempool out-of-order is incorrect - it should be unmarked when TransactionRemovedFromMempool fires.

Clean up of #11839, which I think was the wrong fix.

ACK the |= ret change. I would keep the test here.

At the moment ATMP errors if the transaction is in the mempool:

https://github.com/bitcoin/bitcoin/blob/d48ab83f00538a5135b2c448809260a9d46ca31c/src/validation.cpp#L502-L505

So in this case we know how to avoid that error, which is the test added above in #11839.

Exactly, no reason to double-check when ATMP is going to check it for us anyway.

But in the ATMP context being in the mempool is an error, and IMO we shouldn't hit that.

Its no different from a mempool conflict of any other form, and the error is discarded/not printed almost everywhere in wallet (and where it may be, we want the state object to be populated with the appropriate error, not ignored). Worse, its weirdly race-y, with wallet checking something and then calling something else which will check the same thing but with the appropriate locks.

utACK 6ef86c92e7fcba866160d7a346fb260d7e4ab5bb

utACK