Skip to content

Commit

Permalink
[net processing] Default initialize recentRejects
Browse files Browse the repository at this point in the history 
Now that recentRejects is owned by PeerManagerImpl, and
PeerManagerImpl's lifetime is managed by the node context, we can just
default initialize recentRejects during object initialization. We can
also remove the unique_ptr indirection.
  • Loading branch information
@jnewbery
jnewbery committed Jul 20, 2021
1 parent a28bfd1 commit cd9902a
Showing 1 changed file with 10 additions and 16 deletions.
26 changes: 10 additions & 16 deletions src/net_processing.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ class PeerManagerImpl final : public PeerManager
*
* Memory used: 1.3 MB
*/
std::unique_ptr<CRollingBloomFilter> recentRejects GUARDED_BY(cs_main);
CRollingBloomFilter recentRejects GUARDED_BY(::cs_main){120'000, 0.000'001};
uint256 hashRecentRejectsChainTip GUARDED_BY(cs_main);

/*
Expand Down Expand Up @@ -1396,9 +1396,6 @@ PeerManagerImpl::PeerManagerImpl(const CChainParams& chainparams, CConnman& conn
m_mempool(pool),
m_ignore_incoming_txs(ignore_incoming_txs)
{
// Initialize global variables that cannot be constructed at startup.
recentRejects.reset(new CRollingBloomFilter(120000, 0.000001));

// Blocks don't typically have more than 4000 transactions, so this should
// be at least six blocks (~1 hr) worth of transactions that we can store,
// inserting both a txid and wtxid for every observed transaction.
Expand Down Expand Up @@ -1601,14 +1598,13 @@ void PeerManagerImpl::BlockChecked(const CBlock& block, const BlockValidationSta

bool PeerManagerImpl::AlreadyHaveTx(const GenTxid& gtxid)
{
assert(recentRejects);
if (m_chainman.ActiveChain().Tip()->GetBlockHash() != hashRecentRejectsChainTip) {
// If the chain tip has changed previously rejected transactions
// might be now valid, e.g. due to a nLockTime'd tx becoming valid,
// or a double-spend. Reset the rejects filter and give those
// txs a second chance.
hashRecentRejectsChainTip = m_chainman.ActiveChain().Tip()->GetBlockHash();
recentRejects->reset();
recentRejects.reset();
}

const uint256& hash = gtxid.GetHash();
Expand All @@ -1620,7 +1616,7 @@ bool PeerManagerImpl::AlreadyHaveTx(const GenTxid& gtxid)
if (m_recent_confirmed_transactions->contains(hash)) return true;
}

return recentRejects->contains(hash) || m_mempool.exists(gtxid);
return recentRejects.contains(hash) || m_mempool.exists(gtxid);
}

bool PeerManagerImpl::AlreadyHaveBlock(const uint256& block_hash)
Expand Down Expand Up @@ -2239,8 +2235,7 @@ void PeerManagerImpl::ProcessOrphanTx(std::set<uint256>& orphan_work_set)
// See also comments in https://github.com/bitcoin/bitcoin/pull/18044#discussion_r443419034
// for concerns around weakening security of unupgraded nodes
// if we start doing this too early.
assert(recentRejects);
recentRejects->insert(porphanTx->GetWitnessHash());
recentRejects.insert(porphanTx->GetWitnessHash());
// If the transaction failed for TX_INPUTS_NOT_STANDARD,
// then we know that the witness was irrelevant to the policy
// failure, since this check depends only on the txid
Expand All @@ -2252,7 +2247,7 @@ void PeerManagerImpl::ProcessOrphanTx(std::set<uint256>& orphan_work_set)
if (state.GetResult() == TxValidationResult::TX_INPUTS_NOT_STANDARD && porphanTx->GetWitnessHash() != porphanTx->GetHash()) {
// We only add the txid if it differs from the wtxid, to
// avoid wasting entries in the rolling bloom filter.
recentRejects->insert(porphanTx->GetHash());
recentRejects.insert(porphanTx->GetHash());
}
}
m_orphanage.EraseTx(orphanHash);
Expand Down Expand Up @@ -3255,7 +3250,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
std::sort(unique_parents.begin(), unique_parents.end());
unique_parents.erase(std::unique(unique_parents.begin(), unique_parents.end()), unique_parents.end());
for (const uint256& parent_txid : unique_parents) {
if (recentRejects->contains(parent_txid)) {
if (recentRejects.contains(parent_txid)) {
fRejectedParents = true;
break;
}
Expand Down Expand Up @@ -3296,8 +3291,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
// regardless of what witness is provided, we will not accept
// this, so we don't need to allow for redownload of this txid
// from any of our non-wtxidrelay peers.
recentRejects->insert(tx.GetHash());
recentRejects->insert(tx.GetWitnessHash());
recentRejects.insert(tx.GetHash());
recentRejects.insert(tx.GetWitnessHash());
m_txrequest.ForgetTxHash(tx.GetHash());
m_txrequest.ForgetTxHash(tx.GetWitnessHash());
}
Expand All @@ -3316,8 +3311,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
// See also comments in https://github.com/bitcoin/bitcoin/pull/18044#discussion_r443419034
// for concerns around weakening security of unupgraded nodes
// if we start doing this too early.
assert(recentRejects);
recentRejects->insert(tx.GetWitnessHash());
recentRejects.insert(tx.GetWitnessHash());
m_txrequest.ForgetTxHash(tx.GetWitnessHash());
// If the transaction failed for TX_INPUTS_NOT_STANDARD,
// then we know that the witness was irrelevant to the policy
Expand All @@ -3328,7 +3322,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
// transactions are later received (resulting in
// parent-fetching by txid via the orphan-handling logic).
if (state.GetResult() == TxValidationResult::TX_INPUTS_NOT_STANDARD && tx.GetWitnessHash() != tx.GetHash()) {
recentRejects->insert(tx.GetHash());
recentRejects.insert(tx.GetHash());
m_txrequest.ForgetTxHash(tx.GetHash());
}
if (RecursiveDynamicUsage(*ptx) < 100000) {
Expand Down

0 comments on commit cd9902a

Please sign in to comment.