mirrors/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2024-11-20 02:25:40 +01:00
Code Issues Projects Releases Wiki Activity

Merge #19905: Remove dead CheckForkWarningConditionsOnNewFork

Browse Source 
fa7eed5be7 doc: Clarify that vpindexToConnect is in reverse order (MarcoFalke)
fa62304c97 Remove dead CheckForkWarningConditionsOnNewFork (MarcoFalke)

Pull request description:

  The function has several code and logic bugs, which prevent it from working at all:

  * `vpindexToConnect.back()` is passed to `CheckForkWarningConditionsOnNewFork`, which is the earliest connected block (least work block), *not* the new fork tip
  * `ActivateBestChainStep` will never try to connect a block that descends from an invalid block, so the invalid fork will only ever be of height 1, never hitting the 7 block minimum condition

  Instead of dragging the dead and wrong code around through every change in validation, remove it. In the future it could make sense to add a fork detection somewhere outside of the `ActivateBestChainStep` logic (maybe net_processing).

ACKs for top commit:
  jnewbery:
    utACK fa7eed5be7
  fjahr:
    Code review ACK fa7eed5be7
  glozow:
    utACK fa7eed5be7 I see that it's dead code

Tree-SHA512: 815bdbac7c1eb5b7594b0866a2dbd3c7619797afaadb03a5269fb96739ffb83b05b8e4f7c1e68d48d7886132dd0b12c14c3fb4ee0e72de1074726050ed203e1a
This commit is contained in:
MarcoFalke 2020-11-18 11:23:50 +01:00
commit a64ff1c4d3
No known key found for this signature in database
GPG Key ID: D2EA4850E7528B25
3 changed files with 13 additions and 94 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

87
src/validation.cpp
View File

@ -1312,8 +1312,6 @@ bool CChainState::IsInitialBlockDownload() const
return false;
}
static CBlockIndex *pindexBestForkTip = nullptr, *pindexBestForkBase = nullptr;
static void AlertNotify(const std::string& strMessage)
{
uiInterface.NotifyAlertChanged();
@ -1339,75 +1337,18 @@ static void CheckForkWarningConditions() EXCLUSIVE_LOCKS_REQUIRED(cs_main)
AssertLockHeld(cs_main);
// Before we get past initial download, we cannot reliably alert about forks
// (we assume we don't get stuck on a fork before finishing our initial sync)
if (::ChainstateActive().IsInitialBlockDownload())
if (::ChainstateActive().IsInitialBlockDownload()) {
return;
// If our best fork is no longer within 72 blocks (+/- 12 hours if no one mines it)
// of our head, drop it
if (pindexBestForkTip && ::ChainActive().Height() - pindexBestForkTip->nHeight >= 72)
pindexBestForkTip = nullptr;
if (pindexBestForkTip || (pindexBestInvalid && pindexBestInvalid->nChainWork > ::ChainActive().Tip()->nChainWork + (GetBlockProof(*::ChainActive().Tip()) * 6)))
{
if (!GetfLargeWorkForkFound() && pindexBestForkBase)
{
std::string warning = std::string("'Warning: Large-work fork detected, forking after block ") +
pindexBestForkBase->phashBlock->ToString() + std::string("'");
AlertNotify(warning);
}
if (pindexBestForkTip && pindexBestForkBase)
{
LogPrintf("%s: Warning: Large valid fork found\n forking the chain at height %d (%s)\n lasting to height %d (%s).\nChain state database corruption likely.\n", __func__,
pindexBestForkBase->nHeight, pindexBestForkBase->phashBlock->ToString(),
pindexBestForkTip->nHeight, pindexBestForkTip->phashBlock->ToString());
SetfLargeWorkForkFound(true);
}
else
{
LogPrintf("%s: Warning: Found invalid chain at least ~6 blocks longer than our best chain.\nChain state database corruption likely.\n", __func__);
SetfLargeWorkInvalidChainFound(true);
}
}
else
{
SetfLargeWorkForkFound(false);
if (pindexBestInvalid && pindexBestInvalid->nChainWork > ::ChainActive().Tip()->nChainWork + (GetBlockProof(*::ChainActive().Tip()) * 6)) {
LogPrintf("%s: Warning: Found invalid chain at least ~6 blocks longer than our best chain.\nChain state database corruption likely.\n", __func__);
SetfLargeWorkInvalidChainFound(true);
} else {
SetfLargeWorkInvalidChainFound(false);
}
}
static void CheckForkWarningConditionsOnNewFork(CBlockIndex* pindexNewForkTip) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
AssertLockHeld(cs_main);
// If we are on a fork that is sufficiently large, set a warning flag
CBlockIndex* pfork = pindexNewForkTip;
CBlockIndex* plonger = ::ChainActive().Tip();
while (pfork && pfork != plonger)
{
while (plonger && plonger->nHeight > pfork->nHeight)
plonger = plonger->pprev;
if (pfork == plonger)
break;
pfork = pfork->pprev;
}
// We define a condition where we should warn the user about as a fork of at least 7 blocks
// with a tip within 72 blocks (+/- 12 hours if no one mines it) of ours
// We use 7 blocks rather arbitrarily as it represents just under 10% of sustained network
// hash rate operating on the fork.
// or a chain that is entirely longer than ours and invalid (note that this should be detected by both)
// We define it this way because it allows us to only store the highest fork tip (+ base) which meets
// the 7-block condition and from this always have the most-likely-to-cause-warning fork
if (pfork && (!pindexBestForkTip || pindexNewForkTip->nHeight > pindexBestForkTip->nHeight) &&
pindexNewForkTip->nChainWork - pfork->nChainWork > (GetBlockProof(*pfork) * 7) &&
::ChainActive().Height() - pindexNewForkTip->nHeight < 72)
{
pindexBestForkTip = pindexNewForkTip;
pindexBestForkBase = pfork;
}
CheckForkWarningConditions();
}
// Called both upon regular invalid block discovery *and* InvalidateBlock
void static InvalidChainFound(CBlockIndex* pindexNew) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
@ -2746,8 +2687,8 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChai
AssertLockHeld(cs_main);
AssertLockHeld(m_mempool.cs);
const CBlockIndex *pindexOldTip = m_chain.Tip();
const CBlockIndex *pindexFork = m_chain.FindFork(pindexMostWork);
const CBlockIndex* pindexOldTip = m_chain.Tip();
const CBlockIndex* pindexFork = m_chain.FindFork(pindexMostWork);
// Disconnect active blocks which are no longer in the best chain.
bool fBlocksDisconnected = false;
@ -2767,7 +2708,7 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChai
fBlocksDisconnected = true;
}
// Build list of new blocks to connect.
// Build list of new blocks to connect (in descending height order).
std::vector<CBlockIndex*> vpindexToConnect;
bool fContinue = true;
int nHeight = pindexFork ? pindexFork->nHeight : -1;
@ -2777,7 +2718,7 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChai
int nTargetHeight = std::min(nHeight + 32, pindexMostWork->nHeight);
vpindexToConnect.clear();
vpindexToConnect.reserve(nTargetHeight - nHeight);
CBlockIndex *pindexIter = pindexMostWork->GetAncestor(nTargetHeight);
CBlockIndex* pindexIter = pindexMostWork->GetAncestor(nTargetHeight);
while (pindexIter && pindexIter->nHeight != nHeight) {
vpindexToConnect.push_back(pindexIter);
pindexIter = pindexIter->pprev;
@ -2785,7 +2726,7 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChai
nHeight = nTargetHeight;
// Connect new blocks.
for (CBlockIndex *pindexConnect : reverse_iterate(vpindexToConnect)) {
for (CBlockIndex* pindexConnect : reverse_iterate(vpindexToConnect)) {
if (!ConnectTip(state, chainparams, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr<const CBlock>(), connectTrace, disconnectpool)) {
if (state.IsInvalid()) {
// The block violates a consensus rule.
@ -2821,11 +2762,7 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChai
}
m_mempool.check(&CoinsTip());
// Callbacks/notifications for a new best chain.
if (fInvalidFound)
CheckForkWarningConditionsOnNewFork(vpindexToConnect.back());
else
CheckForkWarningConditions();
CheckForkWarningConditions();
return true;
}

18
src/warnings.cpp
View File

@ -14,7 +14,6 @@
static Mutex g_warnings_mutex;
static bilingual_str g_misc_warnings GUARDED_BY(g_warnings_mutex);
static bool fLargeWorkForkFound GUARDED_BY(g_warnings_mutex) = false;
static bool fLargeWorkInvalidChainFound GUARDED_BY(g_warnings_mutex) = false;
void SetMiscWarning(const bilingual_str& warning)
@ -23,18 +22,6 @@ void SetMiscWarning(const bilingual_str& warning)
g_misc_warnings = warning;
}
void SetfLargeWorkForkFound(bool flag)
{
LOCK(g_warnings_mutex);
fLargeWorkForkFound = flag;
}
bool GetfLargeWorkForkFound()
{
LOCK(g_warnings_mutex);
return fLargeWorkForkFound;
}
void SetfLargeWorkInvalidChainFound(bool flag)
{
LOCK(g_warnings_mutex);
@ -60,10 +47,7 @@ bilingual_str GetWarnings(bool verbose)
warnings_verbose.emplace_back(warnings_concise);
}
if (fLargeWorkForkFound) {
warnings_concise = _("Warning: The network does not appear to fully agree! Some miners appear to be experiencing issues.");
warnings_verbose.emplace_back(warnings_concise);
} else if (fLargeWorkInvalidChainFound) {
if (fLargeWorkInvalidChainFound) {
warnings_concise = _("Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade.");
warnings_verbose.emplace_back(warnings_concise);
}

2
src/warnings.h
View File

@ -11,8 +11,6 @@
struct bilingual_str;
void SetMiscWarning(const bilingual_str& warning);
void SetfLargeWorkForkFound(bool flag);
bool GetfLargeWorkForkFound();
void SetfLargeWorkInvalidChainFound(bool flag);
/** Format a string that describes several potential problems detected by the core.
* @param[in] verbose bool