diff --git a/src/node/txorphanage.h b/src/node/txorphanage.h index 6039b369b9f..5ce22d69a97 100644 --- a/src/node/txorphanage.h +++ b/src/node/txorphanage.h @@ -20,9 +20,7 @@ namespace node { static constexpr int64_t DEFAULT_RESERVED_ORPHAN_WEIGHT_PER_PEER{404'000}; /** Default value for TxOrphanage::m_max_global_latency_score. Helps limit the maximum latency for operations like * EraseForBlock and LimitOrphans. */ -static constexpr unsigned int DEFAULT_MAX_ORPHANAGE_LATENCY_SCORE{100}; -/** Default maximum number of orphan transactions kept in memory */ -static const uint32_t DEFAULT_MAX_ORPHAN_TRANSACTIONS{100}; +static constexpr unsigned int DEFAULT_MAX_ORPHANAGE_LATENCY_SCORE{3000}; /** A class to track orphan transactions (failed on TX_MISSING_INPUTS) * Since we cannot distinguish orphans from bad transactions with non-existent inputs, we heavily limit the amount of diff --git a/src/test/fuzz/txdownloadman.cpp b/src/test/fuzz/txdownloadman.cpp index 0c07e1010d2..bdca947c836 100644 --- a/src/test/fuzz/txdownloadman.cpp +++ b/src/test/fuzz/txdownloadman.cpp @@ -277,12 +277,9 @@ FUZZ_TARGET(txdownloadman, .init = initialize) // peer without tracking anything (this is only for the txdownload_impl target). static bool HasRelayPermissions(NodeId peer) { return peer == 0; } -static void CheckInvariants(const node::TxDownloadManagerImpl& txdownload_impl, size_t max_orphan_count) +static void CheckInvariants(const node::TxDownloadManagerImpl& txdownload_impl) { - // Orphanage usage should never exceed what is allowed - Assert(txdownload_impl.m_orphanage->Size() <= max_orphan_count); txdownload_impl.m_orphanage->SanityCheck(); - // We should never have more than the maximum in-flight requests out for a peer. for (NodeId peer = 0; peer < NUM_PEERS; ++peer) { if (!HasRelayPermissions(peer)) { @@ -301,7 +298,6 @@ FUZZ_TARGET(txdownloadman_impl, .init = initialize) // Initialize a TxDownloadManagerImpl bilingual_str error; CTxMemPool pool{MemPoolOptionsForTest(g_setup->m_node), error}; - const auto max_orphan_count = node::DEFAULT_MAX_ORPHAN_TRANSACTIONS; FastRandomContext det_rand{true}; node::TxDownloadManagerImpl txdownload_impl{node::TxDownloadOptions{pool, det_rand, true}}; @@ -434,7 +430,7 @@ FUZZ_TARGET(txdownloadman_impl, .init = initialize) if (fuzzed_data_provider.ConsumeBool()) time_skip *= -1; time += time_skip; } - CheckInvariants(txdownload_impl, max_orphan_count); + CheckInvariants(txdownload_impl); // Disconnect everybody, check that all data structures are empty. for (NodeId nodeid = 0; nodeid < NUM_PEERS; ++nodeid) { txdownload_impl.DisconnectedPeer(nodeid); diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp index f56fcbee803..56977548fde 100644 --- a/src/test/fuzz/txorphan.cpp +++ b/src/test/fuzz/txorphan.cpp @@ -52,7 +52,7 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage) std::vector tx_history; - LIMITED_WHILE(outpoints.size() < 200'000 && fuzzed_data_provider.ConsumeBool(), 10 * node::DEFAULT_MAX_ORPHAN_TRANSACTIONS) + LIMITED_WHILE(outpoints.size() < 200'000 && fuzzed_data_provider.ConsumeBool(), 1000) { // construct transaction const CTransactionRef tx = [&] { @@ -100,7 +100,7 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage) } // trigger orphanage functions - LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10 * node::DEFAULT_MAX_ORPHAN_TRANSACTIONS) + LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 1000) { NodeId peer_id = fuzzed_data_provider.ConsumeIntegral(); const auto total_bytes_start{orphanage->TotalOrphanUsage()}; @@ -220,7 +220,6 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage) // test mocktime and expiry SetMockTime(ConsumeTime(fuzzed_data_provider)); orphanage->LimitOrphans(); - Assert(orphanage->Size() <= node::DEFAULT_MAX_ORPHAN_TRANSACTIONS); }); } diff --git a/test/functional/p2p_invalid_tx.py b/test/functional/p2p_invalid_tx.py index ae9771e7cb4..634dbc9e26c 100755 --- a/test/functional/p2p_invalid_tx.py +++ b/test/functional/p2p_invalid_tx.py @@ -143,14 +143,14 @@ class InvalidTxRequestTest(BitcoinTestFramework): self.wait_until(lambda: 1 == len(node.getpeerinfo()), timeout=12) # p2ps[1] is no longer connected assert_equal(expected_mempool, set(node.getrawmempool())) - self.log.info('Test orphan pool overflow') + self.log.info('Test orphanage can store more than 100 transactions') orphan_tx_pool = [CTransaction() for _ in range(101)] for i in range(len(orphan_tx_pool)): orphan_tx_pool[i].vin.append(CTxIn(outpoint=COutPoint(i, 333))) orphan_tx_pool[i].vout.append(CTxOut(nValue=11 * COIN, scriptPubKey=SCRIPT_PUB_KEY_OP_TRUE)) - with node.assert_debug_log(['orphanage overflow, removed 1 tx']): - node.p2ps[0].send_txs_and_test(orphan_tx_pool, node, success=False) + node.p2ps[0].send_txs_and_test(orphan_tx_pool, node, success=False) + self.wait_until(lambda: len(node.getorphantxs()) >= 101) self.log.info('Test orphan with rejected parents') rejected_parent = CTransaction() @@ -160,8 +160,8 @@ class InvalidTxRequestTest(BitcoinTestFramework): node.p2ps[0].send_txs_and_test([rejected_parent], node, success=False) self.log.info('Test that a peer disconnection causes erase its transactions from the orphan pool') - with node.assert_debug_log(['Erased 100 orphan transaction(s) from peer=26']): - self.reconnect_p2p(num_connections=1) + self.reconnect_p2p(num_connections=1) + self.wait_until(lambda: len(node.getorphantxs()) == 0) self.log.info('Test that a transaction in the orphan pool is included in a new tip block causes erase this transaction from the orphan pool') tx_withhold_until_block_A = CTransaction() diff --git a/test/functional/p2p_orphan_handling.py b/test/functional/p2p_orphan_handling.py index d10c796acf7..803dff4df51 100755 --- a/test/functional/p2p_orphan_handling.py +++ b/test/functional/p2p_orphan_handling.py @@ -43,8 +43,6 @@ from test_framework.wallet import ( # for one peer and y seconds for another, use specific values instead. TXREQUEST_TIME_SKIP = NONPREF_PEER_TX_DELAY + TXID_RELAY_DELAY + OVERLOADED_PEER_TX_DELAY + 1 -DEFAULT_MAX_ORPHAN_TRANSACTIONS = 100 - def cleanup(func): # Time to fastfoward (using setmocktime) in between subtests to ensure they do not interfere with # one another, in seconds. Equal to 12 hours, which is enough to expire anything that may exist @@ -593,46 +591,6 @@ class OrphanHandlingTest(BitcoinTestFramework): assert_equal(node.getmempoolentry(tx_child["txid"])["wtxid"], tx_child["wtxid"]) self.wait_until(lambda: len(node.getorphantxs()) == 0) - @cleanup - def test_max_orphan_amount(self): - self.log.info("Check that we never exceed our storage limits for orphans") - - node = self.nodes[0] - self.generate(self.wallet, 1) - peer_1 = node.add_p2p_connection(P2PInterface()) - - self.log.info("Check that orphanage is empty on start of test") - assert len(node.getorphantxs()) == 0 - - self.log.info("Filling up orphanage with " + str(DEFAULT_MAX_ORPHAN_TRANSACTIONS) + "(DEFAULT_MAX_ORPHAN_TRANSACTIONS) orphans") - orphans = [] - parent_orphans = [] - for _ in range(DEFAULT_MAX_ORPHAN_TRANSACTIONS): - tx_parent_1 = self.wallet.create_self_transfer() - tx_child_1 = self.wallet.create_self_transfer(utxo_to_spend=tx_parent_1["new_utxo"]) - parent_orphans.append(tx_parent_1["tx"]) - orphans.append(tx_child_1["tx"]) - peer_1.send_without_ping(msg_tx(tx_child_1["tx"])) - - peer_1.sync_with_ping() - orphanage = node.getorphantxs() - self.wait_until(lambda: len(node.getorphantxs()) == DEFAULT_MAX_ORPHAN_TRANSACTIONS) - - for orphan in orphans: - assert tx_in_orphanage(node, orphan) - - self.log.info("Check that we do not add more than the max orphan amount") - tx_parent_1 = self.wallet.create_self_transfer() - tx_child_1 = self.wallet.create_self_transfer(utxo_to_spend=tx_parent_1["new_utxo"]) - peer_1.send_and_ping(msg_tx(tx_child_1["tx"])) - parent_orphans.append(tx_parent_1["tx"]) - orphanage = node.getorphantxs() - assert_equal(len(orphanage), DEFAULT_MAX_ORPHAN_TRANSACTIONS) - - self.log.info("Clearing the orphanage") - for index, parent_orphan in enumerate(parent_orphans): - peer_1.send_and_ping(msg_tx(parent_orphan)) - self.wait_until(lambda: len(node.getorphantxs()) == 0) @cleanup def test_orphan_handling_prefer_outbound(self): @@ -820,7 +778,6 @@ class OrphanHandlingTest(BitcoinTestFramework): self.test_same_txid_orphan() 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()