Merge #17258: Fix issue with conflicted mempool tx in listsinceblock

436ad436434b94982bcb7dc1d13a21949263ef73 Fix issue with conflicted mempool tx in listsinceblock (Adam Jonas)

Pull request description:

  Closes #8752 by bringing back abandoned #10470.

  This now checks that returned transactions are not conflicting with any transactions that are filtered out by the given blockhash and add a functional test to prevent this in the future.

  For more context, #8757 was closed in favor of #10470.

ACKs for top commit:
  instagibbs:
    utACK https://github.com/bitcoin/bitcoin/pull/17258/commits/436ad436434b94982bcb7dc1d13a21949263ef73
  kallewoof:
    utACK 436ad436434b94982bcb7dc1d13a21949263ef73
  jonatack:
    I'm not qualifed to give an ACK here but 436ad436434b94982bcb7dc1d13a21949263ef73 appears reasonable. Built/ran tests/verified that this test fails without the change in rpcwallet.cpp:

Tree-SHA512: 63d75cd3d3f19fc84dc38899b200c96179b82b24db263cd0116ee5b715265be647157855c2e35912d2fbc49c7b37db9375d6aab0ac672f0f09bece8431de5ea9

1 files changed, 49 insertions, 0 deletions

diff --git a/test/functional/wallet_listsinceblock.py b/test/functional/wallet_listsinceblock.py
index 4aeb393255..455e89e310 100755
--- a/test/functional/wallet_listsinceblock.py
+++ b/test/functional/wallet_listsinceblock.py
@@ -5,6 +5,7 @@
 """Test the listsincelast RPC."""
 
 from test_framework.test_framework import BitcoinTestFramework
+from test_framework.messages import BIP125_SEQUENCE_NUMBER
 from test_framework.util import (
     assert_array_result,
     assert_equal,
@@ -12,6 +13,7 @@ from test_framework.util import (
     connect_nodes,
 )
 
+from decimal import Decimal
 
 class ListSinceBlockTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -33,6 +35,7 @@ class ListSinceBlockTest(BitcoinTestFramework):
         self.test_reorg()
         self.test_double_spend()
         self.test_double_send()
+        self.double_spends_filtered()
 
     def test_no_blockhash(self):
         txid = self.nodes[2].sendtoaddress(self.nodes[0].getnewaddress(), 1)
@@ -291,5 +294,51 @@ class ListSinceBlockTest(BitcoinTestFramework):
             if tx['txid'] == txid1:
                 assert_equal(tx['confirmations'], 2)
 
+    def double_spends_filtered(self):
+        '''
+        `listsinceblock` was returning conflicted transactions even if they
+        occurred before the specified cutoff blockhash
+        '''
+        spending_node = self.nodes[2]
+        dest_address = spending_node.getnewaddress()
+
+        tx_input = dict(
+            sequence=BIP125_SEQUENCE_NUMBER, **next(u for u in spending_node.listunspent()))
+        rawtx = spending_node.createrawtransaction(
+            [tx_input], {dest_address: tx_input["amount"] - Decimal("0.00051000"),
+                         spending_node.getrawchangeaddress(): Decimal("0.00050000")})
+        signedtx = spending_node.signrawtransactionwithwallet(rawtx)
+        orig_tx_id = spending_node.sendrawtransaction(signedtx["hex"])
+        original_tx = spending_node.gettransaction(orig_tx_id)
+
+        double_tx = spending_node.bumpfee(orig_tx_id)
+
+        # check that both transactions exist
+        block_hash = spending_node.listsinceblock(
+            spending_node.getblockhash(spending_node.getblockcount()))
+        original_found = False
+        double_found = False
+        for tx in block_hash['transactions']:
+            if tx['txid'] == original_tx['txid']:
+                original_found = True
+            if tx['txid'] == double_tx['txid']:
+                double_found = True
+        assert_equal(original_found, True)
+        assert_equal(double_found, True)
+
+        lastblockhash = spending_node.generate(1)[0]
+
+        # check that neither transaction exists
+        block_hash = spending_node.listsinceblock(lastblockhash)
+        original_found = False
+        double_found = False
+        for tx in block_hash['transactions']:
+            if tx['txid'] == original_tx['txid']:
+                original_found = True
+            if tx['txid'] == double_tx['txid']:
+                double_found = True
+        assert_equal(original_found, False)
+        assert_equal(double_found, False)
+
 if __name__ == '__main__':
     ListSinceBlockTest().main()