Skip to content

Commit

Permalink
Fix chain tip discovery (#974)
Browse files Browse the repository at this point in the history 
* Update CBlockIndex to include blockwork getter

* Replacing critical place to include the new blockwork getter and add more methods for correctly handling the best block work
  • Loading branch information
@marpme @justinvforvendetta
marpme authored and justinvforvendetta committed Aug 27, 2019
1 parent 683ac7b commit 14d456f
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 20 deletions.
10 changes: 10 additions & 0 deletions src/chain.h
View file
Expand Up @@ -364,6 +364,16 @@ class CBlockIndex
return false;
}

//! Defines the base for checking the validity and
//! maintaining the best chain and searching for the best
arith_uint256 GetBlockWork() const
{
// this shall return the real block work that has been done
// in order to create this work. Due to older code we are forced
// to use this for now. (FIXME VIP-1)
return nHeight;
}

//! Build the skiplist pointer for this entry.
void BuildSkip();

Expand Down
24 changes: 12 additions & 12 deletions src/net_processing.cpp
View file
Expand Up @@ -379,8 +379,8 @@ static void ProcessBlockAvailability(NodeId nodeid) {

if (!state->hashLastUnknownBlock.IsNull()) {
const CBlockIndex* pindex = LookupBlockIndex(state->hashLastUnknownBlock);
if (pindex && pindex->nChainWork > 0) {
if (state->pindexBestKnownBlock == nullptr || pindex->nChainWork >= state->pindexBestKnownBlock->nChainWork) {
if (pindex && pindex->GetBlockWork() > 0) {
if (state->pindexBestKnownBlock == nullptr || pindex->GetBlockWork() >= state->pindexBestKnownBlock->GetBlockWork()) {
state->pindexBestKnownBlock = pindex;
}
state->hashLastUnknownBlock.SetNull();
Expand All @@ -396,9 +396,9 @@ static void UpdateBlockAvailability(NodeId nodeid, const uint256 &hash) {
ProcessBlockAvailability(nodeid);

const CBlockIndex* pindex = LookupBlockIndex(hash);
if (pindex && pindex->nChainWork > 0) {
if (pindex && pindex->GetBlockWork() > 0) {
// An actually better block was announced.
if (state->pindexBestKnownBlock == nullptr || pindex->nChainWork >= state->pindexBestKnownBlock->nChainWork) {
if (state->pindexBestKnownBlock == nullptr || pindex->GetBlockWork() >= state->pindexBestKnownBlock->GetBlockWork()) {
state->pindexBestKnownBlock = pindex;
}
} else {
Expand Down Expand Up @@ -484,7 +484,7 @@ static void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vec
// Make sure pindexBestKnownBlock is up to date, we'll need it.
ProcessBlockAvailability(nodeid);

if (state->pindexBestKnownBlock == nullptr || state->pindexBestKnownBlock->nChainWork < chainActive.Tip()->nChainWork || state->pindexBestKnownBlock->nChainWork < nMinimumChainWork) {
if (state->pindexBestKnownBlock == nullptr || state->pindexBestKnownBlock->GetBlockWork() < chainActive.Tip()->GetBlockWork() || state->pindexBestKnownBlock->nChainWork < nMinimumChainWork) {
// This peer has nothing interesting.
return;
}
Expand Down Expand Up @@ -1452,7 +1452,7 @@ bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::ve
// because it is set in UpdateBlockAvailability. Some nullptr checks
// are still present, however, as belt-and-suspenders.

if (received_new_header && pindexLast->nChainWork > chainActive.Tip()->nChainWork) {
if (received_new_header && pindexLast->GetBlockWork() > chainActive.Tip()->GetBlockWork()) {
nodestate->m_last_block_announcement = GetTime();
}

Expand Down Expand Up @@ -1492,7 +1492,7 @@ bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::ve
bool fCanDirectFetch = CanDirectFetch(chainparams.GetConsensus());
// If this set of headers is valid and ends in a block with at least as
// much work as our tip, download as much as possible.
if (fCanDirectFetch && pindexLast->IsValid(BLOCK_VALID_TREE) && chainActive.Tip()->nChainWork <= pindexLast->nChainWork) {
if (fCanDirectFetch && pindexLast->IsValid(BLOCK_VALID_TREE) && chainActive.Tip()->GetBlockWork() <= pindexLast->GetBlockWork()) {
std::vector<const CBlockIndex*> vToFetch;
const CBlockIndex *pindexWalk = pindexLast;
// Calculate all the blocks we'd need to switch to pindexLast, up to a limit.
Expand Down Expand Up @@ -1565,7 +1565,7 @@ bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::ve
if (!pfrom->fDisconnect && IsOutboundDisconnectionCandidate(pfrom) && nodestate->pindexBestKnownBlock != nullptr) {
// If this is an outbound peer, check to see if we should protect
// it from the bad/lagging chain logic.
if (g_outbound_peers_with_protect_from_disconnect < MAX_OUTBOUND_PEERS_TO_PROTECT_FROM_DISCONNECT && nodestate->pindexBestKnownBlock->nChainWork >= chainActive.Tip()->nChainWork && !nodestate->m_chain_sync.m_protect) {
if (g_outbound_peers_with_protect_from_disconnect < MAX_OUTBOUND_PEERS_TO_PROTECT_FROM_DISCONNECT && nodestate->pindexBestKnownBlock->GetBlockWork() >= chainActive.Tip()->GetBlockWork() && !nodestate->m_chain_sync.m_protect) {
LogPrint(BCLog::NET, "Protecting outbound peer=%d from eviction\n", pfrom->GetId());
nodestate->m_chain_sync.m_protect = true;
++g_outbound_peers_with_protect_from_disconnect;
Expand Down Expand Up @@ -2477,7 +2477,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr

// If this was a new header with more work than our tip, update the
// peer's last block announcement time
if (received_new_header && pindex->nChainWork > chainActive.Tip()->nChainWork) {
if (received_new_header && pindex->GetBlockWork() > chainActive.Tip()->GetBlockWork()) {
nodestate->m_last_block_announcement = GetTime();
}

Expand All @@ -2487,7 +2487,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
if (pindex->nStatus & BLOCK_HAVE_DATA) // Nothing to do here
return true;

if (pindex->nChainWork <= chainActive.Tip()->nChainWork || // We know something better
if (pindex->GetBlockWork() <= chainActive.Tip()->GetBlockWork() || // We know something better
pindex->nTx != 0) { // We had this block at some point, but pruned it
if (fAlreadyInFlight) {
// We requested this block for some reason, but our mempool will probably be useless
Expand Down Expand Up @@ -3148,13 +3148,13 @@ void PeerLogicValidation::ConsiderEviction(CNode *pto, int64_t time_in_seconds)
// their chain has more work than ours, we should sync to it,
// unless it's invalid, in which case we should find that out and
// disconnect from them elsewhere).
if (state.pindexBestKnownBlock != nullptr && state.pindexBestKnownBlock->nChainWork >= chainActive.Tip()->nChainWork) {
if (state.pindexBestKnownBlock != nullptr && state.pindexBestKnownBlock->GetBlockWork() >= chainActive.Tip()->GetBlockWork()) {
if (state.m_chain_sync.m_timeout != 0) {
state.m_chain_sync.m_timeout = 0;
state.m_chain_sync.m_work_header = nullptr;
state.m_chain_sync.m_sent_getheaders = false;
}
} else if (state.m_chain_sync.m_timeout == 0 || (state.m_chain_sync.m_work_header != nullptr && state.pindexBestKnownBlock != nullptr && state.pindexBestKnownBlock->nChainWork >= state.m_chain_sync.m_work_header->nChainWork)) {
} else if (state.m_chain_sync.m_timeout == 0 || (state.m_chain_sync.m_work_header != nullptr && state.pindexBestKnownBlock != nullptr && state.pindexBestKnownBlock->GetBlockWork() >= state.m_chain_sync.m_work_header->GetBlockWork())) {
// Our best block known by this peer is behind our tip, and we're either noticing
// that for the first time, OR this peer was able to catch up to some earlier point
// where we checked against our tip.
Expand Down
33 changes: 25 additions & 8 deletions src/validation.cpp
View file
Expand Up @@ -65,8 +65,8 @@ namespace {
{
bool operator()(const CBlockIndex *pa, const CBlockIndex *pb) const {
// First sort by most total work, ...
if (pa->nChainWork > pb->nChainWork) return false;
if (pa->nChainWork < pb->nChainWork) return true;
if (pa->GetBlockWork() > pb->GetBlockWork()) return false;
if (pa->GetBlockWork() < pb->GetBlockWork()) return true;

// ... then by earliest time received, ...
if (pa->nSequenceId < pb->nSequenceId) return false;
Expand Down Expand Up @@ -125,7 +125,9 @@ class CChainState {
/** Decreasing counter (used by subsequent preciousblock calls). */
int32_t nBlockReverseSequenceId = -1;
/** chainwork for the last block that preciousblock has been applied to. */
arith_uint256 nLastPreciousChainwork = 0;
// FIXME VIP-1
// arith_uint256 nLastPreciousChainwork = 0;
arith_uint256 nLastPreciousBlockHeight = 0;

/** In order to efficiently track invalidity of headers, we keep the set of
* blocks which we tried to connect and found to be invalid here (ie which
Expand Down Expand Up @@ -179,6 +181,7 @@ class CChainState {
bool DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions *disconnectpool);

// Manual block validity manipulation:
arith_uint256 GetLastPreciousBlockWork() const;
bool PreciousBlock(CValidationState& state, const CChainParams& params, CBlockIndex *pindex);
bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, CBlockIndex *pindex);
bool ResetBlockFailureFlags(CBlockIndex *pindex);
Expand All @@ -204,7 +207,7 @@ class CChainState {
* By default this only executes fully when using the Regtest chain; see: fCheckBlockIndex.
*/
void CheckBlockIndex(const Consensus::Params& consensusParams);

void SetNewPreciousBlockWork(const arith_uint256& newWork);
void InvalidBlockFound(CBlockIndex *pindex, const CValidationState &state);
CBlockIndex* FindMostWorkChain();
void ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pindexNew, const FlatFilePos& pos, const Consensus::Params& consensusParams);
Expand Down Expand Up @@ -2532,7 +2535,7 @@ CBlockIndex* CChainState::FindMostWorkChain() {
bool fMissingData = !(pindexTest->nStatus & BLOCK_HAVE_DATA);
if (fFailedChain || fMissingData) {
// Candidate chain is not usable (either invalid or missing data)
if (fFailedChain && (pindexBestInvalid == nullptr || pindexNew->nChainWork > pindexBestInvalid->nChainWork))
if (fFailedChain && (pindexBestInvalid == nullptr || pindexNew->GetBlockWork() > pindexBestInvalid->GetBlockWork()))
pindexBestInvalid = pindexNew;
CBlockIndex *pindexFailed = pindexNew;
// Remove the entire chain from the set.
Expand Down Expand Up @@ -2792,19 +2795,33 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams,
return g_chainstate.ActivateBestChain(state, chainparams, std::move(pblock));
}

arith_uint256 CChainState::GetLastPreciousBlockWork() const
{
// FIXME VIP-1
return nLastPreciousBlockHeight;
}

void CChainState::SetNewPreciousBlockWork(const arith_uint256& newWork)
{
// FIXME VIP-1
nLastPreciousBlockHeight = newWork;
}

bool CChainState::PreciousBlock(CValidationState& state, const CChainParams& params, CBlockIndex *pindex)
{
{
LOCK(cs_main);
if (pindex->nChainWork < chainActive.Tip()->nChainWork) {
if (pindex->GetBlockWork() < chainActive.Tip()->GetBlockWork()) {
// Nothing to do, this block is not at the tip.
return true;
}
if (chainActive.Tip()->nChainWork > nLastPreciousChainwork) {
if (chainActive.Tip()->GetBlockWork() > GetLastPreciousBlockWork()) {
// The chain has been extended since the last call, reset the counter.
nBlockReverseSequenceId = -1;
}
nLastPreciousChainwork = chainActive.Tip()->nChainWork;

SetNewPreciousBlockWork(chainActive.Tip()->GetBlockWork());

setBlockIndexCandidates.erase(pindex);
pindex->nSequenceId = nBlockReverseSequenceId;
if (nBlockReverseSequenceId > std::numeric_limits<int32_t>::min()) {
Expand Down

0 comments on commit 14d456f

Please sign in to comment.