Skip to content

Commit b1c8ea4

Browse files
author
MacroFake
committed
Merge bitcoin/bitcoin#25683: refactor: log nEvicted message in LimitOrphans then return void
b4b657b refactor: log `nEvicted` message in `LimitOrphans` then return void (chinggg) Pull request description: Fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=49347 LimitOrphans() can log expired tx and it should log evicted tx as well instead of returning the `nEvicted` number for caller to print the message. Since `LimitOrphans()` now returns void, the redundant assertion check in fuzz test is also removed. Top commit has no ACKs. Tree-SHA512: 18c41702321b0e59812590cd389f3163831d431f4ebdc3b3e1e0698496a6bdbac52288f28f779237a58813c6717da1a35e8933d509822978ff726c1b13cfc778
2 parents 1abbae6 + b4b657b commit b1c8ea4

File tree

4 files changed

+5
-10
lines changed

4 files changed

+5
-10
lines changed

src/net_processing.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3631,10 +3631,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
36313631

36323632
// DoS prevention: do not allow m_orphanage to grow unbounded (see CVE-2012-3789)
36333633
unsigned int nMaxOrphanTx = (unsigned int)std::max((int64_t)0, gArgs.GetIntArg("-maxorphantx", DEFAULT_MAX_ORPHAN_TRANSACTIONS));
3634-
unsigned int nEvicted = m_orphanage.LimitOrphans(nMaxOrphanTx);
3635-
if (nEvicted > 0) {
3636-
LogPrint(BCLog::MEMPOOL, "orphanage overflow, removed %u tx\n", nEvicted);
3637-
}
3634+
m_orphanage.LimitOrphans(nMaxOrphanTx);
36383635
} else {
36393636
LogPrint(BCLog::MEMPOOL, "not keeping orphan with rejected parents %s\n",tx.GetHash().ToString());
36403637
// We will continue to reject this tx since it has rejected

src/test/fuzz/txorphan.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,10 +138,8 @@ FUZZ_TARGET_INIT(txorphan, initialize_orphanage)
138138
[&] {
139139
// test mocktime and expiry
140140
SetMockTime(ConsumeTime(fuzzed_data_provider));
141-
auto size_before = orphanage.Size();
142141
auto limit = fuzzed_data_provider.ConsumeIntegral<unsigned int>();
143-
auto n_evicted = WITH_LOCK(g_cs_orphans, return orphanage.LimitOrphans(limit));
144-
Assert(size_before - n_evicted <= limit);
142+
WITH_LOCK(g_cs_orphans, orphanage.LimitOrphans(limit));
145143
Assert(orphanage.Size() <= limit);
146144
});
147145
}

src/txorphanage.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ void TxOrphanage::EraseForPeer(NodeId peer)
102102
if (nErased > 0) LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx from peer=%d\n", nErased, peer);
103103
}
104104

105-
unsigned int TxOrphanage::LimitOrphans(unsigned int max_orphans)
105+
void TxOrphanage::LimitOrphans(unsigned int max_orphans)
106106
{
107107
AssertLockHeld(g_cs_orphans);
108108

@@ -135,7 +135,7 @@ unsigned int TxOrphanage::LimitOrphans(unsigned int max_orphans)
135135
EraseTx(m_orphan_list[randompos]->first);
136136
++nEvicted;
137137
}
138-
return nEvicted;
138+
if (nEvicted > 0) LogPrint(BCLog::MEMPOOL, "orphanage overflow, removed %u tx\n", nEvicted);
139139
}
140140

141141
void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx, std::set<uint256>& orphan_work_set) const

src/txorphanage.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class TxOrphanage {
4141
void EraseForBlock(const CBlock& block) LOCKS_EXCLUDED(::g_cs_orphans);
4242

4343
/** Limit the orphanage to the given maximum */
44-
unsigned int LimitOrphans(unsigned int max_orphans) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
44+
void LimitOrphans(unsigned int max_orphans) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
4545

4646
/** Add any orphans that list a particular tx as a parent into a peer's work set
4747
* (ie orphans that may have found their final missing parent, and so should be reconsidered for the mempool) */

0 commit comments

Comments
 (0)