Merge #9622: [rpc] listsinceblock should include lost transactions when parameter is a reorg'd block

876e92b Testing: listsinceblock should display all transactions that were affected since the given block, including transactions that were removed due to a reorg. (Karl-Johan Alm)
f999c46 listsinceblock: optionally find and list any transactions that were undone due to reorg when requesting a non-main chain block in a new 'removed' array. (Karl-Johan Alm)

Pull request description:

  The following scenario will not notify the caller of the fact `tx0` has been dropped:

  1. User 1 receives BTC in tx0 from utxo1 in block aa1.
  2. User 2 receives BTC in tx1 from utxo1 (same) in block bb1
  3. User 1 sees 2 confirmations at block aa3.
  4. Reorg into bb chain.
  5. User 1 asks `listsinceblock aa3` and does not see that tx0 is now invalidated.

  See `listsinceblock.py` commit for related test.

  The proposed fix is to iterate from the given block down to the fork point, and to check each transaction in the blocks against the wallet, in addition to including all transactions from the fork point to the active chain tip (the current behavior). Any transactions that were present will now also be listed in the `listsinceblock` output in a new `replaced` array. This operation may be a bit heavy but the circumstances (and perceived frequency of occurrence) warrant it, I believe.

  Example output:
  ```Python
  {
    'transactions': [],
    'replaced': [
      {
        'walletconflicts': [],
        'vout': 1,
        'account': '',
        'timereceived': 1485234857,
        'time': 1485234857,
        'amount': '1.00000000',
        'bip125-replaceable': 'unknown',
        'trusted': False,
        'category': 'receive',
        'txid': 'ce673859a30dee1d2ebdb3c05f2eea7b1da54baf68f93bb8bfe37c5f09ed22ff',
        'address': 'miqEt4kWp9zSizwGGuUWLAmxEcTW9bFUnQ',
        'label': '',
        'confirmations': -7
      }
    ],
    'lastblock': '7a388f27d09e3699102a4ebf81597d974fc4c72093eeaa02adffbbf7527f6715'
  }
  ```

  I believe this addresses the comment by @luke-jr in https://github.com/bitcoin/bitcoin/pull/9516#issuecomment-274190081 but I could be wrong..

Tree-SHA512: 607b5dcaeccb9dc0d963d3de138c40490f3e923050b29821e6bd513d26beb587bddc748fbb194503fe618cfe34a6ed65d95e8d9c5764a882b6c5f976520cff35

1 files changed, 180 insertions, 10 deletions

diff --git a/test/functional/listsinceblock.py b/test/functional/listsinceblock.py
index f3d41e573e..ce2d556ef0 100755
--- a/test/functional/listsinceblock.py
+++ b/test/functional/listsinceblock.py
@@ -14,7 +14,15 @@ class ListSinceBlockTest (BitcoinTestFramework):
         self.setup_clean_chain = True
         self.num_nodes = 4
 
-    def run_test (self):
+    def run_test(self):
+        self.nodes[2].generate(101)
+        self.sync_all()
+
+        self.test_reorg()
+        self.test_double_spend()
+        self.test_double_send()
+
+    def test_reorg(self):
         '''
         `listsinceblock` did not behave correctly when handed a block that was
         no longer in the main chain:
@@ -43,14 +51,6 @@ class ListSinceBlockTest (BitcoinTestFramework):
         This test only checks that [tx0] is present.
         '''
 
-        self.nodes[2].generate(101)
-        self.sync_all()
-
-        assert_equal(self.nodes[0].getbalance(), 0)
-        assert_equal(self.nodes[1].getbalance(), 0)
-        assert_equal(self.nodes[2].getbalance(), 50)
-        assert_equal(self.nodes[3].getbalance(), 0)
-
         # Split network into two
         self.split_network()
 
@@ -73,7 +73,177 @@ class ListSinceBlockTest (BitcoinTestFramework):
             if tx['txid'] == senttx:
                 found = True
                 break
-        assert_equal(found, True)
+        assert found
+
+    def test_double_spend(self):
+        '''
+        This tests the case where the same UTXO is spent twice on two separate
+        blocks as part of a reorg.
+
+             ab0
+          /       \
+        aa1 [tx1]   bb1 [tx2]
+         |           |
+        aa2         bb2
+         |           |
+        aa3         bb3
+                     |
+                    bb4
+
+        Problematic case:
+
+        1. User 1 receives BTC in tx1 from utxo1 in block aa1.
+        2. User 2 receives BTC in tx2 from utxo1 (same) in block bb1
+        3. User 1 sees 2 confirmations at block aa3.
+        4. Reorg into bb chain.
+        5. User 1 asks `listsinceblock aa3` and does not see that tx1 is now
+           invalidated.
+
+        Currently the solution to this is to detect that a reorg'd block is
+        asked for in listsinceblock, and to iterate back over existing blocks up
+        until the fork point, and to include all transactions that relate to the
+        node wallet.
+        '''
+
+        self.sync_all()
+
+        # Split network into two
+        self.split_network()
+
+        # share utxo between nodes[1] and nodes[2]
+        utxos = self.nodes[2].listunspent()
+        utxo = utxos[0]
+        privkey = self.nodes[2].dumpprivkey(utxo['address'])
+        self.nodes[1].importprivkey(privkey)
+
+        # send from nodes[1] using utxo to nodes[0]
+        change = '%.8f' % (float(utxo['amount']) - 1.0003)
+        recipientDict = {
+            self.nodes[0].getnewaddress(): 1,
+            self.nodes[1].getnewaddress(): change,
+        }
+        utxoDicts = [{
+            'txid': utxo['txid'],
+            'vout': utxo['vout'],
+        }]
+        txid1 = self.nodes[1].sendrawtransaction(
+            self.nodes[1].signrawtransaction(
+                self.nodes[1].createrawtransaction(utxoDicts, recipientDict))['hex'])
+
+        # send from nodes[2] using utxo to nodes[3]
+        recipientDict2 = {
+            self.nodes[3].getnewaddress(): 1,
+            self.nodes[2].getnewaddress(): change,
+        }
+        self.nodes[2].sendrawtransaction(
+            self.nodes[2].signrawtransaction(
+                self.nodes[2].createrawtransaction(utxoDicts, recipientDict2))['hex'])
+
+        # generate on both sides
+        lastblockhash = self.nodes[1].generate(3)[2]
+        self.nodes[2].generate(4)
+
+        self.join_network()
+
+        self.sync_all()
+
+        # gettransaction should work for txid1
+        assert self.nodes[0].gettransaction(txid1)['txid'] == txid1, "gettransaction failed to find txid1"
+
+        # listsinceblock(lastblockhash) should now include txid1, as seen from nodes[0]
+        lsbres = self.nodes[0].listsinceblock(lastblockhash)
+        assert any(tx['txid'] == txid1 for tx in lsbres['removed'])
+
+        # but it should not include 'removed' if include_removed=false
+        lsbres2 = self.nodes[0].listsinceblock(blockhash=lastblockhash, include_removed=False)
+        assert 'removed' not in lsbres2
+
+    def test_double_send(self):
+        '''
+        This tests the case where the same transaction is submitted twice on two
+        separate blocks as part of a reorg. The former will vanish and the
+        latter will appear as the true transaction (with confirmations dropping
+        as a result).
+
+             ab0
+          /       \
+        aa1 [tx1]   bb1
+         |           |
+        aa2         bb2
+         |           |
+        aa3         bb3 [tx1]
+                     |
+                    bb4
+
+        Asserted:
+
+        1. tx1 is listed in listsinceblock.
+        2. It is included in 'removed' as it was removed, even though it is now
+           present in a different block.
+        3. It is listed with a confirmations count of 2 (bb3, bb4), not
+           3 (aa1, aa2, aa3).
+        '''
+
+        self.sync_all()
+
+        # Split network into two
+        self.split_network()
+
+        # create and sign a transaction
+        utxos = self.nodes[2].listunspent()
+        utxo = utxos[0]
+        change = '%.8f' % (float(utxo['amount']) - 1.0003)
+        recipientDict = {
+            self.nodes[0].getnewaddress(): 1,
+            self.nodes[2].getnewaddress(): change,
+        }
+        utxoDicts = [{
+            'txid': utxo['txid'],
+            'vout': utxo['vout'],
+        }]
+        signedtxres = self.nodes[2].signrawtransaction(
+                self.nodes[2].createrawtransaction(utxoDicts, recipientDict))
+        assert signedtxres['complete']
+
+        signedtx = signedtxres['hex']
+
+        # send from nodes[1]; this will end up in aa1
+        txid1 = self.nodes[1].sendrawtransaction(signedtx)
+
+        # generate bb1-bb2 on right side
+        self.nodes[2].generate(2)
+
+        # send from nodes[2]; this will end up in bb3
+        txid2 = self.nodes[2].sendrawtransaction(signedtx)
+
+        assert_equal(txid1, txid2)
+
+        # generate on both sides
+        lastblockhash = self.nodes[1].generate(3)[2]
+        self.nodes[2].generate(2)
+
+        self.join_network()
+
+        self.sync_all()
+
+        # gettransaction should work for txid1
+        self.nodes[0].gettransaction(txid1)
+
+        # listsinceblock(lastblockhash) should now include txid1 in transactions
+        # as well as in removed
+        lsbres = self.nodes[0].listsinceblock(lastblockhash)
+        assert any(tx['txid'] == txid1 for tx in lsbres['transactions'])
+        assert any(tx['txid'] == txid1 for tx in lsbres['removed'])
+
+        # find transaction and ensure confirmations is valid
+        for tx in lsbres['transactions']:
+            if tx['txid'] == txid1:
+                assert_equal(tx['confirmations'], 2)
+
+        # the same check for the removed array; confirmations should STILL be 2
+        for tx in lsbres['removed']:
+            if tx['txid'] == txid1:
+                assert_equal(tx['confirmations'], 2)
 
 if __name__ == '__main__':
     ListSinceBlockTest().main()