Merge #21713: Refactor ProcessNewBlock to reduce code duplication

9a0653553a0ec403b4e7c6713466e0c7fa10ec94 Refactor ProcessNewBlock to reduce code duplication (R E Broadley)

Pull request description:

  There are probably a few issues with this code (maybe there's even a reason this code is duplicated as it currently is), so apologies in advance that I'm still a little (maybe very) bad with C++

ACKs for top commit:
  MarcoFalke:
    ACK 9a0653553a0ec403b4e7c6713466e0c7fa10ec94 💻
  promag:
    Code review ACK 9a0653553a0ec403b4e7c6713466e0c7fa10ec94.
  theStack:
    Code-review ACK 9a0653553a0ec403b4e7c6713466e0c7fa10ec94 🌴

Tree-SHA512: f8634ffad4b2370204d1a0945db4e27248b9e579d9912784da432b8ee3303cae424fa9f7500000dcfb31e6d29d04a8f7d322d17a6fe3d4adaddd10c539458a8c
This commit is contained in:
MarcoFalke 2021-04-19 09:01:43 +02:00 committed by Vijay Das Manikpuri
parent e649229046
commit f13be5085b
No known key found for this signature in database
GPG Key ID: DB1D81B01DB7C46E

41
src/net_processing.cpp Normal file → Executable file
View File

@ -473,6 +473,8 @@ private:
void ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic<bool>& interruptMsgProc) LOCKS_EXCLUDED(cs_main) EXCLUSIVE_LOCKS_REQUIRED(peer.m_getdata_requests_mutex);
void ProcessBlock(CNode& pfrom, const std::shared_ptr<const CBlock>& pblock, bool fForceProcessing);
/** Relay map */
typedef std::map<uint256, CTransactionRef> MapRelay;
MapRelay mapRelay GUARDED_BY(cs_main);
@ -2783,6 +2785,18 @@ std::pair<bool /*ret*/, bool /*do_return*/> static ValidateDSTX(CTxMemPool& memp
return {true, false};
}
void PeerManagerImpl::ProcessBlock(CNode& pfrom, const std::shared_ptr<const CBlock>& pblock, bool fForceProcessing)
{
bool fNewBlock = false;
m_chainman.ProcessNewBlock(m_chainparams, pblock, fForceProcessing, &fNewBlock);
if (fNewBlock) {
pfrom.nLastBlockTime = GetTime();
} else {
LOCK(cs_main);
mapBlockSource.erase(pblock->GetHash());
}
}
void PeerManagerImpl::ProcessMessage(
CNode& pfrom,
const std::string& msg_type,
@ -3850,7 +3864,6 @@ void PeerManagerImpl::ProcessMessage(
LOCK(cs_main);
mapBlockSource.emplace(pblock->GetHash(), std::make_pair(pfrom.GetId(), false));
}
bool fNewBlock = false;
// Setting fForceProcessing to true means that we bypass some of
// our anti-DoS protections in AcceptBlock, which filters
// unrequested blocks that might be trying to waste our resources
@ -3860,13 +3873,7 @@ void PeerManagerImpl::ProcessMessage(
// we have a chain with at least nMinimumChainWork), and we ignore
// compact blocks with less work than our tip, it is safe to treat
// reconstructed compact blocks as having been requested.
m_chainman.ProcessNewBlock(m_chainparams, pblock, /*fForceProcessing=*/true, &fNewBlock);
if (fNewBlock) {
pfrom.nLastBlockTime = GetTime();
} else {
LOCK(cs_main);
mapBlockSource.erase(pblock->GetHash());
}
ProcessBlock(pfrom, pblock, /*fForceProcessing=*/true);
LOCK(cs_main); // hold cs_main for CBlockIndex::IsValid()
if (pindex->IsValid(BLOCK_VALID_TRANSACTIONS)) {
// Clear download state for this block, which is in
@ -3943,20 +3950,13 @@ void PeerManagerImpl::ProcessMessage(
}
} // Don't hold cs_main when we call into ProcessNewBlock
if (fBlockRead) {
bool fNewBlock = false;
// Since we requested this block (it was in mapBlocksInFlight), force it to be processed,
// even if it would not be a candidate for new tip (missing previous block, chain not long enough, etc)
// This bypasses some anti-DoS logic in AcceptBlock (eg to prevent
// disk-space attacks), but this should be safe due to the
// protections in the compact block handler -- see related comment
// in compact block optimistic reconstruction handling.
m_chainman.ProcessNewBlock(m_chainparams, pblock, /*fForceProcessing=*/true, &fNewBlock);
if (fNewBlock) {
pfrom.nLastBlockTime = GetTime();
} else {
LOCK(cs_main);
mapBlockSource.erase(pblock->GetHash());
}
ProcessBlock(pfrom, pblock, /*fForceProcessing=*/true);
}
return;
}
@ -4021,14 +4021,7 @@ void PeerManagerImpl::ProcessMessage(
// cs_main in ProcessNewBlock is fine.
mapBlockSource.emplace(hash, std::make_pair(pfrom.GetId(), true));
}
bool fNewBlock = false;
m_chainman.ProcessNewBlock(m_chainparams, pblock, forceProcessing, &fNewBlock);
if (fNewBlock) {
pfrom.nLastBlockTime = GetTime();
} else {
LOCK(cs_main);
mapBlockSource.erase(pblock->GetHash());
}
ProcessBlock(pfrom, pblock, forceProcessing);
return;
}