From 0da693f7e129fccaecf9a2c177083d2e80d37781 Mon Sep 17 00:00:00 2001 From: glozow Date: Fri, 26 Jul 2024 16:34:55 +0100 Subject: [PATCH] [functional test] orphan handling with multiple announcers --- test/functional/p2p_1p1c_network.py | 6 - test/functional/p2p_orphan_handling.py | 181 ++++++++++++++++++++++++- 2 files changed, 177 insertions(+), 10 deletions(-) diff --git a/test/functional/p2p_1p1c_network.py b/test/functional/p2p_1p1c_network.py index cdc4e1691d6..eef307d1eb0 100755 --- a/test/functional/p2p_1p1c_network.py +++ b/test/functional/p2p_1p1c_network.py @@ -143,12 +143,6 @@ class PackageRelayTest(BitcoinTestFramework): 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: diff --git a/test/functional/p2p_orphan_handling.py b/test/functional/p2p_orphan_handling.py index 0864deeb4fa..4a2d53cd2b3 100755 --- a/test/functional/p2p_orphan_handling.py +++ b/test/functional/p2p_orphan_handling.py @@ -58,6 +58,10 @@ def cleanup(func): 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 @@ class OrphanHandlingTest(BitcoinTestFramework): 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 @@ class OrphanHandlingTest(BitcoinTestFramework): 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 @@ class OrphanHandlingTest(BitcoinTestFramework): 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,8 +630,173 @@ class OrphanHandlingTest(BitcoinTestFramework): 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())) @@ -635,6 +804,7 @@ class OrphanHandlingTest(BitcoinTestFramework): 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 @@ class OrphanHandlingTest(BitcoinTestFramework): 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__':