wallet: Unrelated conflicted parent txs do not cause child txs to be marked as conflicted #29435

1. 
   achow101 commented at 5:45 pm on February 14, 2024: member

   Is there an existing issue for this?

   • I have searched the existing issues

   Current behaviour

   If we receive a transaction which spends from an unconfirmed parent, and that parent is replaced, the child does not get marked as conflicted but rather only as inactive. If the child contains an input from our wallet, that input cannot be reused until the child is abandoned.

   See also https://github.com/ACINQ/eclair/pull/2818

   Expected behaviour

   We should try to detect if a such a child transaction becomes conflicted due to a parent becoming conflicted and mark it as such.

   Steps to reproduce

   A (failing) test for this behavior can be found at https://github.com/achow101/bitcoin/commit/213fa91d5c289204e3f1e01194b1fccd08a69c6c

   Relevant log output

   No response

   How did you obtain Bitcoin Core

   Compiled from source

   What version of Bitcoin Core are you using?

   master@128b4a80387d322a7810d8716eccdac95ff9b8cd

   Operating system and version

   Arch

   Machine specifications

   No response

2. 
   Eunovo commented at 10:33 pm on March 2, 2024: none

   @achow101 What about situations where the parent goes from conflicted to unconfirmed?
3. 
   ryanofsky commented at 5:35 pm on March 12, 2024: contributor

   We should try to detect if a such a child transaction becomes conflicted due to a parent becoming conflicted and mark it as such.

   How could we do this? Would the idea be to add non-wallet transactions to mapTxSpends? This doesn’t seem like a straightforward problem to solve.

4. 
   achow101 commented at 6:19 pm on March 12, 2024: member

   How could we do this? Would the idea be to add non-wallet transactions to mapTxSpends? This doesn’t seem like a straightforward problem to solve.

   My initial thought was that we could just look at every tx that we get a TransactionRemovedFromMempool signal for and see if there are any descendants that are in the wallet.

   But I agree that this is nontrivial to solve.

5. 
   ryanofsky commented at 6:54 pm on March 12, 2024: contributor

   My initial thought was that we could just look at every tx that we get a TransactionRemovedFromMempool signal for and see if there are any descendants that are in the wallet.

   I think that makes sense if it’s enough to detect conflicts where the parent transaction conflicts with a new block and is already in the mempool. The issue description doesn’t seem to say whether or not the parent transaction is in the mempool, but maybe that’s implied.

   Maybe one complication with using the TransactionRemovedFromMempool notification to detect block conflicts is that the notification doesn’t include the hash of the conflicting block. But maybe this information could be added, or we could add back the vtxConflicted array argument to BlockConnected that was removed in #17477. I think recording the hash of the conflicting block is just mostly just important for reverting the conflicted state if there is a reorg.

6. 
   Eunovo commented at 7:46 pm on March 12, 2024: none

   Maybe one complication with using the TransactionRemovedFromMempool notification to detect block conflicts is that the notification doesn’t include the hash of the conflicting block. But maybe this information could be added, or we could add back the vtxConflicted array argument to BlockConnected that was removed in #17477. I think recording the hash of the conflicting block is just mostly just important for reverting the conflicted state if there is a reorg. @ryanofsky I’m currently testing a rough PoC, https://github.com/Eunovo/bitcoin/commit/517e39cf1e88af18d82dfbb49292ddd89cad2761, that I think follows a similar idea but without using vtxConflicted array. I created a new map to keep track of non-wallet transactions, then I modified the REPLACED signal for TransactionRemovedFromMempool to include the txid of the double-spending transaction. By tracking this double-spend transactions, the wallet can detect conflicts in the BlockConnected signal and mark the wallet tx as conflicted.

7. 
   Eunovo commented at 7:49 pm on March 12, 2024: none

   My initial thought was that we could just look at every tx that we get a TransactionRemovedFromMempool signal for and see if there are any descendants that are in the wallet.

   @achow101 Not sure how we can access a mempool transaction’s ancestry in the wallet. The mempool doesn’t seem to be directly connected to the wallet.

Contributors
achow101 Eunovo ryanofsky