[functional test] orphan handling with multiple announcers

Author: glozow

test/functional/p2p_1p1c_network.py

@@ -143,12 +143,6 @@ def run_test(self):
         for (i, peer) in enumerate(self.peers):
             for tx in transactions_to_presend[i]:
                 peer.send_and_ping(msg_tx(tx))
-            # This disconnect removes any sent orphans from the orphanage (EraseForPeer) and times
-            # out the in-flight requests.  It is currently required for the test to pass right now,
-            # because the node will not reconsider an orphan tx and will not (re)try requesting
-            # orphan parents from multiple peers if the first one didn't respond.
-            # TODO: remove this in the future if the node tries orphan resolution with multiple peers.
-            peer.peer_disconnect()
 
         self.log.info("Submit full packages to node0")
         for package_hex in packages_to_submit:

test/functional/p2p_orphan_handling.py

@@ -58,6 +58,10 @@ def wrapper(self):
             self.generate(self.nodes[0], 1)
             self.nodes[0].disconnect_p2ps()
             self.nodes[0].bumpmocktime(LONG_TIME_SKIP)
+            # Check that mempool and orphanage have been cleared
+            assert_equal(0, len(self.nodes[0].getorphantxs()))
+            assert_equal(0, len(self.nodes[0].getrawmempool()))
+            self.wallet.rescan_utxos(include_mempool=True)
     return wrapper
 
 class PeerTxRelayer(P2PTxInvStore):
@@ -533,7 +537,7 @@ def test_same_txid_orphan_of_orphan(self):
         assert tx_middle["txid"] in node_mempool
         assert tx_grandchild["txid"] in node_mempool
         assert_equal(node.getmempoolentry(tx_middle["txid"])["wtxid"], tx_middle["wtxid"])
-        assert_equal(len(node.getorphantxs()), 0)
+        self.wait_until(lambda: len(node.getorphantxs()) == 0)
 
     @cleanup
     def test_orphan_txid_inv(self):
@@ -585,7 +589,7 @@ def test_orphan_txid_inv(self):
         assert tx_parent["txid"] in node_mempool
         assert tx_child["txid"] in node_mempool
         assert_equal(node.getmempoolentry(tx_child["txid"])["wtxid"], tx_child["wtxid"])
-        assert_equal(len(node.getorphantxs()), 0)
+        self.wait_until(lambda: len(node.getorphantxs()) == 0)
 
     @cleanup
     def test_max_orphan_amount(self):
@@ -610,7 +614,7 @@ def test_max_orphan_amount(self):
 
         peer_1.sync_with_ping()
         orphanage = node.getorphantxs()
-        assert_equal(len(orphanage), DEFAULT_MAX_ORPHAN_TRANSACTIONS)
+        self.wait_until(lambda: len(node.getorphantxs()) == DEFAULT_MAX_ORPHAN_TRANSACTIONS)
 
         for orphan in orphans:
             assert tx_in_orphanage(node, orphan)
@@ -626,15 +630,181 @@ def test_max_orphan_amount(self):
         self.log.info("Clearing the orphanage")
         for index, parent_orphan in enumerate(parent_orphans):
             peer_1.send_and_ping(msg_tx(parent_orphan))
-        assert_equal(len(node.getorphantxs()),0)
+        self.wait_until(lambda: len(node.getorphantxs()) == 0)
+
+    @cleanup
+    def test_orphan_handling_prefer_outbound(self):
+        self.log.info("Test that the node prefers requesting from outbound peers")
+        node = self.nodes[0]
+        orphan_wtxid, orphan_tx, parent_tx = self.create_parent_and_child()
+        orphan_inv = CInv(t=MSG_WTX, h=int(orphan_wtxid, 16))
+
+        peer_inbound = node.add_p2p_connection(PeerTxRelayer())
+        peer_outbound = node.add_outbound_p2p_connection(PeerTxRelayer(), p2p_idx=1)
+
+        # Inbound peer relays the transaction.
+        peer_inbound.send_and_ping(msg_inv([orphan_inv]))
+        self.nodes[0].bumpmocktime(TXREQUEST_TIME_SKIP)
+        peer_inbound.wait_for_getdata([int(orphan_wtxid, 16)])
+
+        # Both peers send invs for the orphan, so the node can expect both to know its ancestors.
+        peer_outbound.send_and_ping(msg_inv([orphan_inv]))
+
+        peer_inbound.send_and_ping(msg_tx(orphan_tx))
+
+        # There should be 1 orphan with 2 announcers (we don't know what their peer IDs are)
+        orphanage = node.getorphantxs(verbosity=2)
+        assert_equal(orphanage[0]["wtxid"], orphan_wtxid)
+        assert_equal(len(orphanage[0]["from"]), 2)
+
+        # The outbound peer should be preferred for getting orphan parents
+        self.nodes[0].bumpmocktime(TXID_RELAY_DELAY)
+        peer_outbound.wait_for_parent_requests([int(parent_tx.rehash(), 16)])
+
+        # There should be no request to the inbound peer
+        peer_inbound.assert_never_requested(int(parent_tx.rehash(), 16))
+
+        self.log.info("Test that, if the preferred peer doesn't respond, the node sends another request")
+        self.nodes[0].bumpmocktime(GETDATA_TX_INTERVAL)
+        peer_inbound.sync_with_ping()
+        peer_inbound.wait_for_parent_requests([int(parent_tx.rehash(), 16)])
+
+    @cleanup
+    def test_announcers_before_and_after(self):
+        self.log.info("Test that the node uses all peers who announced the tx prior to realizing it's an orphan")
+        node = self.nodes[0]
+        orphan_wtxid, orphan_tx, parent_tx = self.create_parent_and_child()
+        orphan_inv = CInv(t=MSG_WTX, h=int(orphan_wtxid, 16))
+
+        # Announces before tx is sent, disconnects while node is requesting parents
+        peer_early_disconnected = node.add_outbound_p2p_connection(PeerTxRelayer(), p2p_idx=3)
+        # Announces before tx is sent, doesn't respond to parent request
+        peer_early_unresponsive = node.add_p2p_connection(PeerTxRelayer())
+
+        # Announces after tx is sent
+        peer_late_announcer = node.add_p2p_connection(PeerTxRelayer())
+
+        # Both peers send invs for the orphan, so the node can expect both to know its ancestors.
+        peer_early_disconnected.send_and_ping(msg_inv([orphan_inv]))
+        self.nodes[0].bumpmocktime(TXREQUEST_TIME_SKIP)
+        peer_early_disconnected.wait_for_getdata([int(orphan_wtxid, 16)])
+        peer_early_unresponsive.send_and_ping(msg_inv([orphan_inv]))
+        peer_early_disconnected.send_and_ping(msg_tx(orphan_tx))
+
+        # There should be 1 orphan with 2 announcers (we don't know what their peer IDs are)
+        orphanage = node.getorphantxs(verbosity=2)
+        assert_equal(len(orphanage), 1)
+        assert_equal(orphanage[0]["wtxid"], orphan_wtxid)
+        assert_equal(len(orphanage[0]["from"]), 2)
+
+        # Peer disconnects before responding to request
+        self.nodes[0].bumpmocktime(TXID_RELAY_DELAY)
+        peer_early_disconnected.wait_for_parent_requests([int(parent_tx.rehash(), 16)])
+        peer_early_disconnected.peer_disconnect()
 
+        # The orphan should have 1 announcer left after the node finishes disconnecting peer_early_disconnected.
+        self.wait_until(lambda: len(node.getorphantxs(verbosity=2)[0]["from"]) == 1)
+
+        # The node should retry with the other peer that announced the orphan earlier.
+        # This node's request was additionally delayed because it's an inbound peer.
+        self.nodes[0].bumpmocktime(NONPREF_PEER_TX_DELAY)
+        peer_early_unresponsive.wait_for_parent_requests([int(parent_tx.rehash(), 16)])
+
+        self.log.info("Test that the node uses peers who announce the tx after realizing it's an orphan")
+        peer_late_announcer.send_and_ping(msg_inv([orphan_inv]))
+
+        # The orphan should have 2 announcers now
+        orphanage = node.getorphantxs(verbosity=2)
+        assert_equal(orphanage[0]["wtxid"], orphan_wtxid)
+        assert_equal(len(orphanage[0]["from"]), 2)
+
+        self.nodes[0].bumpmocktime(GETDATA_TX_INTERVAL)
+        peer_late_announcer.wait_for_parent_requests([int(parent_tx.rehash(), 16)])
+
+    @cleanup
+    def test_parents_change(self):
+        self.log.info("Test that, if a parent goes missing during orphan reso, it is requested")
+        node = self.nodes[0]
+        # Orphan will have 2 parents, 1 missing and 1 already in mempool when received.
+        # Create missing parent.
+        parent_missing = self.wallet.create_self_transfer()
+
+        # Create parent that will already be in mempool, but become missing during orphan resolution.
+        # Get 3 UTXOs for replacement-cycled parent, UTXOS A, B, C
+        coin_A = self.wallet.get_utxo(confirmed_only=True)
+        coin_B = self.wallet.get_utxo(confirmed_only=True)
+        coin_C = self.wallet.get_utxo(confirmed_only=True)
+        # parent_peekaboo_AB spends A and B. It is replaced by tx_replacer_BC (conflicting UTXO B),
+        # and then replaced by tx_replacer_C (conflicting UTXO C). This replacement cycle is used to
+        # ensure that parent_peekaboo_AB can be reintroduced without requiring package RBF.
+        FEE_INCREMENT = 2400
+        parent_peekaboo_AB = self.wallet.create_self_transfer_multi(
+            utxos_to_spend=[coin_A, coin_B],
+            num_outputs=1,
+            fee_per_output=FEE_INCREMENT
+        )
+        tx_replacer_BC = self.wallet.create_self_transfer_multi(
+            utxos_to_spend=[coin_B, coin_C],
+            num_outputs=1,
+            fee_per_output=2*FEE_INCREMENT
+        )
+        tx_replacer_C = self.wallet.create_self_transfer(
+            utxo_to_spend=coin_C,
+            fee_per_output=3*FEE_INCREMENT
+        )
+
+        # parent_peekaboo_AB starts out in the mempool
+        node.sendrawtransaction(parent_peekaboo_AB["hex"])
+
+        orphan = self.wallet.create_self_transfer_multi(utxos_to_spend=[parent_peekaboo_AB["new_utxos"][0], parent_missing["new_utxo"]])
+        orphan_wtxid = orphan["wtxid"]
+        orphan_inv = CInv(t=MSG_WTX, h=int(orphan_wtxid, 16))
+
+        # peer1 sends the orphan and gets a request for the missing parent
+        peer1 = node.add_p2p_connection(PeerTxRelayer())
+        peer1.send_and_ping(msg_inv([orphan_inv]))
+        node.bumpmocktime(TXREQUEST_TIME_SKIP)
+        peer1.wait_for_getdata([int(orphan_wtxid, 16)])
+        peer1.send_and_ping(msg_tx(orphan["tx"]))
+        self.wait_until(lambda: node.getorphantxs(verbosity=0) == [orphan["txid"]])
+        node.bumpmocktime(NONPREF_PEER_TX_DELAY + TXID_RELAY_DELAY)
+        peer1.wait_for_getdata([int(parent_missing["txid"], 16)])
+
+        # Replace parent_peekaboo_AB so that is is a newly missing parent.
+        # Then, replace the replacement so that it can be resubmitted.
+        node.sendrawtransaction(tx_replacer_BC["hex"])
+        assert tx_replacer_BC["txid"] in node.getrawmempool()
+        node.sendrawtransaction(tx_replacer_C["hex"])
+        assert tx_replacer_BC["txid"] not in node.getrawmempool()
+        assert tx_replacer_C["txid"] in node.getrawmempool()
+
+        # Second peer is an additional announcer for this orphan
+        peer2 = node.add_p2p_connection(PeerTxRelayer())
+        peer2.send_and_ping(msg_inv([orphan_inv]))
+        assert_equal(len(node.getorphantxs(verbosity=2)[0]["from"]), 2)
+
+        # Disconnect peer1. peer2 should become the new candidate for orphan resolution.
+        peer1.peer_disconnect()
+        node.bumpmocktime(NONPREF_PEER_TX_DELAY + TXID_RELAY_DELAY)
+        self.wait_until(lambda: len(node.getorphantxs(verbosity=2)[0]["from"]) == 1)
+        # Both parents should be requested, now that they are both missing.
+        peer2.wait_for_parent_requests([int(parent_peekaboo_AB["txid"], 16), int(parent_missing["txid"], 16)])
+        peer2.send_and_ping(msg_tx(parent_missing["tx"]))
+        peer2.send_and_ping(msg_tx(parent_peekaboo_AB["tx"]))
+
+        final_mempool = node.getrawmempool()
+        assert parent_missing["txid"] in final_mempool
+        assert parent_peekaboo_AB["txid"] in final_mempool
+        assert orphan["txid"] in final_mempool
+        assert tx_replacer_C["txid"] in final_mempool
 
     def run_test(self):
         self.nodes[0].setmocktime(int(time.time()))
         self.wallet_nonsegwit = MiniWallet(self.nodes[0], mode=MiniWalletMode.RAW_P2PK)
         self.generate(self.wallet_nonsegwit, 10)
         self.wallet = MiniWallet(self.nodes[0])
         self.generate(self.wallet, 160)
+
         self.test_arrival_timing_orphan()
         self.test_orphan_rejected_parents_exceptions()
         self.test_orphan_multiple_parents()
@@ -645,6 +815,9 @@ def run_test(self):
         self.test_same_txid_orphan_of_orphan()
         self.test_orphan_txid_inv()
         self.test_max_orphan_amount()
+        self.test_orphan_handling_prefer_outbound()
+        self.test_announcers_before_and_after()
+        self.test_parents_change()
 
 
 if __name__ == '__main__':