sikkienl/dash
1
0
Fork 0
mirror of https://github.com/dashpay/dash.git synced 2024-12-24 11:32:46 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#22106: refactor: address ProcessNewBlock comments from #21713

Browse Source 
e12f287498e5836bb5e32de5abaef02f3d20d868 net: cleanup newly added PeerManagerImpl::ProcessNewBlock (fanquake)
610151f5b076d4b1ab90c0dd2717e5410aba6b19 validation: change ProcessNewBlock() to take a CBlock reference (fanquake)

Pull request description:

  Addresses some [post-merge comments](https://github.com/bitcoin/bitcoin/pull/21713#pullrequestreview-638777410) from #21713. Also makes `ChainstateManager::ProcessNewBlock` take a const reference argument, as it [was asked](https://github.com/bitcoin/bitcoin/pull/21713#discussion_r615229548) why it was not the case in that PR.

ACKs for top commit:
  jnewbery:
    Code review ACK e12f287498e5836bb5e32de5abaef02f3d20d868
  MarcoFalke:
    review ACK e12f287498e5836bb5e32de5abaef02f3d20d868 🚚

Tree-SHA512: 9c3e7353240c862d50bce2a0f58741c109dd628040b56ed46250103f8ebe9009238b131da710486791e28e3a83c985057b7be0a32aed1a929269b43097c7425b
This commit is contained in:
fanquake 2021-06-02 10:44:16 +08:00 committed by Konstantin Akimov
parent c8725560c9
commit 3b05a99b50
No known key found for this signature in database
GPG Key ID: 2176C4A5D01EA524
3 changed files with 24 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
src/net_processing.cpp
View File

@ -935,7 +935,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 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); /** Process a new block. Perform any post-processing housekeeping */
void ProcessBlock(CNode& from, const std::shared_ptr<const CBlock>& pblock, bool force_processing);
/** Relay map (txid -> CTransactionRef) */ /** Relay map (txid -> CTransactionRef) */
typedef std::map<uint256, CTransactionRef> MapRelay; typedef std::map<uint256, CTransactionRef> MapRelay;
@ -3290,15 +3291,15 @@ std::pair<bool /*ret*/, bool /*do_return*/> static ValidateDSTX(CDeterministicMN
return {true, false}; return {true, false};
} }
void PeerManagerImpl::ProcessBlock(CNode& pfrom, const std::shared_ptr<const CBlock>& pblock, bool fForceProcessing) void PeerManagerImpl::ProcessBlock(CNode& node, const std::shared_ptr<const CBlock>& block, bool force_processing)
{ {
bool fNewBlock = false; bool new_block{false};
m_chainman.ProcessNewBlock(m_chainparams, pblock, fForceProcessing, &fNewBlock); m_chainman.ProcessNewBlock(m_chainparams, block, force_processing, &new_block);
if (fNewBlock) { if (new_block) {
pfrom.m_last_block_time = GetTime<std::chrono::seconds>(); node.m_last_block_time = GetTime<std::chrono::seconds>();
} else { } else {
LOCK(cs_main); LOCK(cs_main);
mapBlockSource.erase(pblock->GetHash()); mapBlockSource.erase(block->GetHash());
} }
} }
@ -4490,7 +4491,7 @@ void PeerManagerImpl::ProcessMessage(
LOCK(cs_main); LOCK(cs_main);
mapBlockSource.emplace(pblock->GetHash(), std::make_pair(pfrom.GetId(), false)); mapBlockSource.emplace(pblock->GetHash(), std::make_pair(pfrom.GetId(), false));
} }
// Setting fForceProcessing to true means that we bypass some of // Setting force_processing to true means that we bypass some of
// our anti-DoS protections in AcceptBlock, which filters // our anti-DoS protections in AcceptBlock, which filters
// unrequested blocks that might be trying to waste our resources // unrequested blocks that might be trying to waste our resources
// (eg disk space). Because we only try to reconstruct blocks when // (eg disk space). Because we only try to reconstruct blocks when
@ -4499,7 +4500,7 @@ void PeerManagerImpl::ProcessMessage(
// we have a chain with at least nMinimumChainWork), and we ignore // we have a chain with at least nMinimumChainWork), and we ignore
// compact blocks with less work than our tip, it is safe to treat // compact blocks with less work than our tip, it is safe to treat
// reconstructed compact blocks as having been requested. // reconstructed compact blocks as having been requested.
ProcessBlock(pfrom, pblock, /*fForceProcessing=*/true); ProcessBlock(pfrom, pblock, /*force_processing=*/true);
LOCK(cs_main); // hold cs_main for CBlockIndex::IsValid() LOCK(cs_main); // hold cs_main for CBlockIndex::IsValid()
if (pindex->IsValid(BLOCK_VALID_TRANSACTIONS)) { if (pindex->IsValid(BLOCK_VALID_TRANSACTIONS)) {
// Clear download state for this block, which is in // Clear download state for this block, which is in
@ -4582,7 +4583,7 @@ void PeerManagerImpl::ProcessMessage(
// disk-space attacks), but this should be safe due to the // disk-space attacks), but this should be safe due to the
// protections in the compact block handler -- see related comment // protections in the compact block handler -- see related comment
// in compact block optimistic reconstruction handling. // in compact block optimistic reconstruction handling.
ProcessBlock(pfrom, pblock, /*fForceProcessing=*/true); ProcessBlock(pfrom, pblock, /*force_processing=*/true);
} }
return; return;
} }

12
src/validation.cpp
View File

@ -3948,13 +3948,13 @@ bool CChainState::AcceptBlock(const std::shared_ptr<const CBlock>& pblock, Block
return true; return true;
} }
bool ChainstateManager::ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr<const CBlock> pblock, bool fForceProcessing, bool* fNewBlock) bool ChainstateManager::ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr<const CBlock>& block, bool force_processing, bool* new_block)
{ {
AssertLockNotHeld(cs_main); AssertLockNotHeld(cs_main);
{ {
CBlockIndex *pindex = nullptr; CBlockIndex *pindex = nullptr;
if (fNewBlock) *fNewBlock = false; if (new_block) *new_block = false;
BlockValidationState state; BlockValidationState state;
// CheckBlock() does not support multi-threaded block validation because CBlock::fChecked can cause data race. // CheckBlock() does not support multi-threaded block validation because CBlock::fChecked can cause data race.
@ -3966,13 +3966,13 @@ bool ChainstateManager::ProcessNewBlock(const CChainParams& chainparams, const s
// malleability that cause CheckBlock() to fail; see e.g. CVE-2012-2459 and // malleability that cause CheckBlock() to fail; see e.g. CVE-2012-2459 and
// https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2019-February/016697.html. Because CheckBlock() is // https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2019-February/016697.html. Because CheckBlock() is
// not very expensive, the anti-DoS benefits of caching failure (of a definitely-invalid block) are not substantial. // not very expensive, the anti-DoS benefits of caching failure (of a definitely-invalid block) are not substantial.
bool ret = CheckBlock(*pblock, state, chainparams.GetConsensus()); bool ret = CheckBlock(*block, state, chainparams.GetConsensus());
if (ret) { if (ret) {
// Store to disk // Store to disk
ret = ActiveChainstate().AcceptBlock(pblock, state, &pindex, fForceProcessing, nullptr, fNewBlock); ret = ActiveChainstate().AcceptBlock(block, state, &pindex, force_processing, nullptr, new_block);
} }
if (!ret) { if (!ret) {
GetMainSignals().BlockChecked(*pblock, state); GetMainSignals().BlockChecked(*block, state);
return error("%s: AcceptBlock FAILED: %s", __func__, state.ToString()); return error("%s: AcceptBlock FAILED: %s", __func__, state.ToString());
} }
} }
@ -3980,7 +3980,7 @@ bool ChainstateManager::ProcessNewBlock(const CChainParams& chainparams, const s
NotifyHeaderTip(ActiveChainstate()); NotifyHeaderTip(ActiveChainstate());
BlockValidationState state; // Only used to report errors, not invalidity - ignore it BlockValidationState state; // Only used to report errors, not invalidity - ignore it
if (!ActiveChainstate().ActivateBestChain(state, pblock)) if (!ActiveChainstate().ActivateBestChain(state, block))
return error("%s: ActivateBestChain failed: %s", __func__, state.ToString()); return error("%s: ActivateBestChain failed: %s", __func__, state.ToString());
LogPrintf("%s : ACCEPTED\n", __func__); LogPrintf("%s : ACCEPTED\n", __func__);

15
src/validation.h
View File

@ -995,22 +995,21 @@ public:
* block is made active. Note that it does not, however, guarantee that the * block is made active. Note that it does not, however, guarantee that the
* specific block passed to it has been checked for validity! * specific block passed to it has been checked for validity!
* *
* If you want to *possibly* get feedback on whether pblock is valid, you must * If you want to *possibly* get feedback on whether block is valid, you must
* install a CValidationInterface (see validationinterface.h) - this will have * install a CValidationInterface (see validationinterface.h) - this will have
* its BlockChecked method called whenever *any* block completes validation. * its BlockChecked method called whenever *any* block completes validation.
* *
* Note that we guarantee that either the proof-of-work is valid on pblock, or * Note that we guarantee that either the proof-of-work is valid on block, or
* (and possibly also) BlockChecked will have been called. * (and possibly also) BlockChecked will have been called.
* *
* May not be called in a * May not be called in a validationinterface callback.
* validationinterface callback.
* *
* @param[in] pblock The block we want to process. * @param[in] block The block we want to process.
* @param[in] fForceProcessing Process this block even if unrequested; used for non-network block sources. * @param[in] force_processing Process this block even if unrequested; used for non-network block sources.
* @param[out] fNewBlock A boolean which is set to indicate if the block was first received via this call * @param[out] new_block A boolean which is set to indicate if the block was first received via this call
* @returns If the block was processed, independently of block validity * @returns If the block was processed, independently of block validity
*/ */
bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr<const CBlock> pblock, bool fForceProcessing, bool* fNewBlock) LOCKS_EXCLUDED(cs_main); bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr<const CBlock>& block, bool force_processing, bool* new_block) LOCKS_EXCLUDED(cs_main);
/** /**
* Process incoming block headers. * Process incoming block headers.