diff --git a/src/node/txdownloadman_impl.cpp b/src/node/txdownloadman_impl.cpp index e9fe958f5d..804c00494f 100644 --- a/src/node/txdownloadman_impl.cpp +++ b/src/node/txdownloadman_impl.cpp @@ -348,6 +348,12 @@ node::RejectedTxTodo TxDownloadManagerImpl::MempoolRejectedTx(const CTransaction } } if (!fRejectedParents) { + // Filter parents that we already have. + // Exclude m_lazy_recent_rejects_reconsiderable: the missing parent may have been + // previously rejected for being too low feerate. This orphan might CPFP it. + std::erase_if(unique_parents, [&](const auto& txid){ + return AlreadyHaveTx(GenTxid::Txid(txid), /*include_reconsiderable=*/false); + }); const auto current_time{GetTime()}; for (const uint256& parent_txid : unique_parents) { @@ -357,11 +363,7 @@ node::RejectedTxTodo TxDownloadManagerImpl::MempoolRejectedTx(const CTransaction // Eventually we should replace this with an improved // protocol for getting all unconfirmed parents. const auto gtxid{GenTxid::Txid(parent_txid)}; - // Exclude m_lazy_recent_rejects_reconsiderable: the missing parent may have been - // previously rejected for being too low feerate. This orphan might CPFP it. - if (!AlreadyHaveTx(gtxid, /*include_reconsiderable=*/false)) { - AddTxAnnouncement(nodeid, gtxid, current_time, /*p2p_inv=*/false); - } + AddTxAnnouncement(nodeid, gtxid, current_time, /*p2p_inv=*/false); } // Potentially flip add_extra_compact_tx to false if AddTx returns false because the tx was already there diff --git a/src/test/txdownload_tests.cpp b/src/test/txdownload_tests.cpp index 6d277b5629..88064b5a5f 100644 --- a/src/test/txdownload_tests.cpp +++ b/src/test/txdownload_tests.cpp @@ -231,10 +231,14 @@ BOOST_FIXTURE_TEST_CASE(handle_missing_inputs, TestChain100Setup) // it's in RecentRejectsFilter. Specifically, the parent is allowed to be in // RecentRejectsReconsiderableFilter, but it cannot be in RecentRejectsFilter. const bool expect_keep_orphan = !parent_recent_rej; + const unsigned int expected_parents = parent_recent_rej || parent_recent_conf || parent_in_mempool ? 0 : 1; + // If we don't expect to keep the orphan then expected_parents is 0. + // !expect_keep_orphan => (expected_parents == 0) + BOOST_CHECK(expect_keep_orphan || expected_parents == 0); const auto ret_1p1c = txdownload_impl.MempoolRejectedTx(orphan, state_orphan, nodeid, /*first_time_failure=*/true); std::string err_msg; const bool ok = CheckOrphanBehavior(txdownload_impl, orphan, ret_1p1c, err_msg, - /*expect_orphan=*/expect_keep_orphan, /*expect_keep=*/true, /*expected_parents=*/expect_keep_orphan ? 1 : 0); + /*expect_orphan=*/expect_keep_orphan, /*expect_keep=*/true, /*expected_parents=*/expected_parents); BOOST_CHECK_MESSAGE(ok, err_msg); } @@ -278,11 +282,12 @@ BOOST_FIXTURE_TEST_CASE(handle_missing_inputs, TestChain100Setup) for (int32_t i = 1; i < num_parents; ++i) { txdownload_impl.RecentConfirmedTransactionsFilter().insert(parents[i]->GetHash().ToUint256()); } + const unsigned int expected_parents = 1; const auto ret_1recon_conf = txdownload_impl.MempoolRejectedTx(orphan, state_orphan, nodeid, /*first_time_failure=*/true); std::string err_msg; const bool ok = CheckOrphanBehavior(txdownload_impl, orphan, ret_1recon_conf, err_msg, - /*expect_orphan=*/true, /*expect_keep=*/true, /*expected_parents=*/num_parents); + /*expect_orphan=*/true, /*expect_keep=*/true, /*expected_parents=*/expected_parents); BOOST_CHECK_MESSAGE(ok, err_msg); }