From b4b657ba57a2ce31b3c21ea9245aad26d5b06a57 Mon Sep 17 00:00:00 2001 From: chinggg <24590067+chinggg@users.noreply.github.com> Date: Sat, 23 Jul 2022 20:51:07 +0800 Subject: [PATCH] refactor: log `nEvicted` message in `LimitOrphans` then return void `LimitOrphans()` can log expired tx and it should log evicted tx as well instead of returning the number for caller to print the message. Since `LimitOrphans()` now return void, the redundant assertion check in fuzz test is also removed. --- src/net_processing.cpp | 5 +---- src/test/fuzz/txorphan.cpp | 4 +--- src/txorphanage.cpp | 4 ++-- src/txorphanage.h | 2 +- 4 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 74d1bf44d29..a845be9e623 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3630,10 +3630,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // DoS prevention: do not allow m_orphanage to grow unbounded (see CVE-2012-3789) unsigned int nMaxOrphanTx = (unsigned int)std::max((int64_t)0, gArgs.GetIntArg("-maxorphantx", DEFAULT_MAX_ORPHAN_TRANSACTIONS)); - unsigned int nEvicted = m_orphanage.LimitOrphans(nMaxOrphanTx); - if (nEvicted > 0) { - LogPrint(BCLog::MEMPOOL, "orphanage overflow, removed %u tx\n", nEvicted); - } + m_orphanage.LimitOrphans(nMaxOrphanTx); } else { LogPrint(BCLog::MEMPOOL, "not keeping orphan with rejected parents %s\n",tx.GetHash().ToString()); // We will continue to reject this tx since it has rejected diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp index cc20f89bbfa..9278f034233 100644 --- a/src/test/fuzz/txorphan.cpp +++ b/src/test/fuzz/txorphan.cpp @@ -138,10 +138,8 @@ FUZZ_TARGET_INIT(txorphan, initialize_orphanage) [&] { // test mocktime and expiry SetMockTime(ConsumeTime(fuzzed_data_provider)); - auto size_before = orphanage.Size(); auto limit = fuzzed_data_provider.ConsumeIntegral(); - auto n_evicted = WITH_LOCK(g_cs_orphans, return orphanage.LimitOrphans(limit)); - Assert(size_before - n_evicted <= limit); + WITH_LOCK(g_cs_orphans, orphanage.LimitOrphans(limit)); Assert(orphanage.Size() <= limit); }); } diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp index ed4783f1a5c..69ae8ea5821 100644 --- a/src/txorphanage.cpp +++ b/src/txorphanage.cpp @@ -102,7 +102,7 @@ void TxOrphanage::EraseForPeer(NodeId peer) if (nErased > 0) LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx from peer=%d\n", nErased, peer); } -unsigned int TxOrphanage::LimitOrphans(unsigned int max_orphans) +void TxOrphanage::LimitOrphans(unsigned int max_orphans) { AssertLockHeld(g_cs_orphans); @@ -135,7 +135,7 @@ unsigned int TxOrphanage::LimitOrphans(unsigned int max_orphans) EraseTx(m_orphan_list[randompos]->first); ++nEvicted; } - return nEvicted; + if (nEvicted > 0) LogPrint(BCLog::MEMPOOL, "orphanage overflow, removed %u tx\n", nEvicted); } void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx, std::set& orphan_work_set) const diff --git a/src/txorphanage.h b/src/txorphanage.h index 24c8318f369..9363e6f7336 100644 --- a/src/txorphanage.h +++ b/src/txorphanage.h @@ -41,7 +41,7 @@ public: void EraseForBlock(const CBlock& block) LOCKS_EXCLUDED(::g_cs_orphans); /** Limit the orphanage to the given maximum */ - unsigned int LimitOrphans(unsigned int max_orphans) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); + void LimitOrphans(unsigned int max_orphans) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); /** Add any orphans that list a particular tx as a parent into a peer's work set * (ie orphans that may have found their final missing parent, and so should be reconsidered for the mempool) */