Skip to content

Commit

Permalink
[validation] RewindBlockIndex no longer needed
Browse files Browse the repository at this point in the history 
Instead of rewinding blocks, we request that the user deletes blocks dir and chain state dir to IBD
  • Loading branch information
@dhruv
dhruv committed Feb 8, 2021
1 parent b401b09 commit 416854c
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 164 deletions.
31 changes: 11 additions & 20 deletions src/init.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1686,27 +1686,18 @@ bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockA
break;
}

bool failed_rewind{false};
// Can't hold cs_main while calling RewindBlockIndex, so retrieve the relevant
// chainstates beforehand.
for (CChainState* chainstate : WITH_LOCK(::cs_main, return chainman.GetAll())) {
if (!fReset) {
// Note that RewindBlockIndex MUST run even if we're about to -reindex-chainstate.
// It both disconnects blocks based on the chainstate, and drops block data in
// BlockIndex() based on lack of available witness data.
uiInterface.InitMessage(_("Rewinding blocks...").translated);
if (!chainstate->RewindBlockIndex(chainparams)) {
strLoadError = _(
"Unable to rewind the database to a pre-fork state. "
"You will need to redownload the blockchain");
failed_rewind = true;
break; // out of the per-chainstate loop
}
}
}

if (failed_rewind) {
break; // out of the chainstate activation do-while
if (!fReset) {
uiInterface.InitMessage(_("Checking if redownload is needed...").translated);
LOCK(cs_main);
auto chainstates{chainman.GetAll()};
if (std::any_of(chainstates.begin(), chainstates.end(),
[&chainparams](CChainState* cs) { return cs->NeedsRedownload(chainparams); })) {
strLoadError = _(
"Segwit blocks are insufficiently validated. "
"Please delete blocks dir and chain state dir and restart.");
break;
}
}

bool failed_verification = false;
Expand Down
139 changes: 9 additions & 130 deletions src/validation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4390,143 +4390,22 @@ bool CChainState::ReplayBlocks(const CChainParams& params)
return true;
}

//! Helper for CChainState::RewindBlockIndex
void CChainState::EraseBlockData(CBlockIndex* index)
bool CChainState::NeedsRedownload(const CChainParams& params)
{
AssertLockHeld(cs_main);
assert(!m_chain.Contains(index)); // Make sure this block isn't active

// Reduce validity
index->nStatus = std::min<unsigned int>(index->nStatus & BLOCK_VALID_MASK, BLOCK_VALID_TREE) | (index->nStatus & ~BLOCK_VALID_MASK);
// Remove have-data flags.
index->nStatus &= ~(BLOCK_HAVE_DATA | BLOCK_HAVE_UNDO);
// Remove storage location.
index->nFile = 0;
index->nDataPos = 0;
index->nUndoPos = 0;
// Remove various other things
index->nTx = 0;
index->nChainTx = 0;
index->nSequenceId = 0;
// Make sure it gets written.
setDirtyBlockIndex.insert(index);
// Update indexes
setBlockIndexCandidates.erase(index);
auto ret = m_blockman.m_blocks_unlinked.equal_range(index->pprev);
while (ret.first != ret.second) {
if (ret.first->second == index) {
m_blockman.m_blocks_unlinked.erase(ret.first++);
} else {
++ret.first;
}
}
// Mark parent as eligible for main chain again
if (index->pprev && index->pprev->IsValid(BLOCK_VALID_TRANSACTIONS) && index->pprev->HaveTxsDownloaded()) {
setBlockIndexCandidates.insert(index->pprev);
}
}

bool CChainState::RewindBlockIndex(const CChainParams& params)
{
// Note that during -reindex-chainstate we are called with an empty m_chain!

// First erase all post-segwit blocks without witness not in the main chain,
// as this can we done without costly DisconnectTip calls. Active
// blocks will be dealt with below (releasing cs_main in between).
{
LOCK(cs_main);
for (const auto& entry : m_blockman.m_block_index) {
if (IsWitnessEnabled(entry.second->pprev, params.GetConsensus()) && !(entry.second->nStatus & BLOCK_OPT_WITNESS) && !m_chain.Contains(entry.second)) {
EraseBlockData(entry.second);
}
}
}
// At and above params.SegwitHeight, segwit consensus rules must be validated
CBlockIndex* block = m_chain[params.GetConsensus().SegwitHeight];

// Find what height we need to reorganize to.
CBlockIndex *tip;
int nHeight = 1;
{
LOCK(cs_main);
while (nHeight <= m_chain.Height()) {
// Although SCRIPT_VERIFY_WITNESS is now generally enforced on all
// blocks in ConnectBlock, we don't need to go back and
// re-download/re-verify blocks from before segwit actually activated.
if (IsWitnessEnabled(m_chain[nHeight - 1], params.GetConsensus()) && !(m_chain[nHeight]->nStatus & BLOCK_OPT_WITNESS)) {
break;
}
nHeight++;
}

tip = m_chain.Tip();
}
// nHeight is now the height of the first insufficiently-validated block, or tipheight + 1

BlockValidationState state;
// Loop until the tip is below nHeight, or we reach a pruned block.
while (!ShutdownRequested()) {
{
LOCK(cs_main);
LOCK(m_mempool.cs);
// Make sure nothing changed from under us (this won't happen because RewindBlockIndex runs before importing/network are active)
assert(tip == m_chain.Tip());
if (tip == nullptr || tip->nHeight < nHeight) break;
if (fPruneMode && !(tip->nStatus & BLOCK_HAVE_DATA)) {
// If pruning, don't try rewinding past the HAVE_DATA point;
// since older blocks can't be served anyway, there's
// no need to walk further, and trying to DisconnectTip()
// will fail (and require a needless reindex/redownload
// of the blockchain).
break;
}

// Disconnect block
if (!DisconnectTip(state, params, nullptr)) {
return error("RewindBlockIndex: unable to disconnect block at height %i (%s)", tip->nHeight, state.ToString());
}

// Reduce validity flag and have-data flags.
// We do this after actual disconnecting, otherwise we'll end up writing the lack of data
// to disk before writing the chainstate, resulting in a failure to continue if interrupted.
// Note: If we encounter an insufficiently validated block that
// is on m_chain, it must be because we are a pruning node, and
// this block or some successor doesn't HAVE_DATA, so we were unable to
// rewind all the way. Blocks remaining on m_chain at this point
// must not have their validity reduced.
EraseBlockData(tip);

tip = tip->pprev;
}
// Make sure the queue of validation callbacks doesn't grow unboundedly.
LimitValidationInterfaceQueue();

// Occasionally flush state to disk.
if (!FlushStateToDisk(params, state, FlushStateMode::PERIODIC)) {
LogPrintf("RewindBlockIndex: unable to flush state to disk (%s)\n", state.ToString());
return false;
}
}

{
LOCK(cs_main);
if (m_chain.Tip() != nullptr) {
// We can't prune block index candidates based on our tip if we have
// no tip due to m_chain being empty!
PruneBlockIndexCandidates();

CheckBlockIndex(params.GetConsensus());

// FlushStateToDisk can possibly read ::ChainActive(). Be conservative
// and skip it here, we're about to -reindex-chainstate anyway, so
// it'll get called a bunch real soon.
BlockValidationState state;
if (!FlushStateToDisk(params, state, FlushStateMode::ALWAYS)) {
LogPrintf("RewindBlockIndex: unable to flush state to disk (%s)\n", state.ToString());
return false;
}
while (block != nullptr) {
if (!(block->nStatus & BLOCK_OPT_WITNESS)) {
// block is insufficiently validated for a segwit client
return true;
}
block = m_chain.Next(block);
}

return true;
return false;
}

void CChainState::UnloadBlockIndex() {
Expand Down
5 changes: 1 addition & 4 deletions src/validation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,7 @@ class CChainState

/** Replay blocks that aren't fully applied to the database. */
bool ReplayBlocks(const CChainParams& params);
bool RewindBlockIndex(const CChainParams& params) LOCKS_EXCLUDED(cs_main);
[[nodiscard]] bool NeedsRedownload(const CChainParams& params) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
bool LoadGenesisBlock(const CChainParams& chainparams);

void PruneBlockIndexCandidates();
Expand Down Expand Up @@ -730,9 +730,6 @@ class CChainState

bool RollforwardBlock(const CBlockIndex* pindex, CCoinsViewCache& inputs, const CChainParams& params) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

//! Mark a block as not having block data
void EraseBlockData(CBlockIndex* index) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

friend ChainstateManager;
};

Expand Down
41 changes: 31 additions & 10 deletions test/functional/p2p_segwit.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
"""Test segwit transactions and blocks on P2P network."""
from decimal import Decimal
import math
import os
import random
import shutil
import struct
import time

Expand Down Expand Up @@ -83,6 +85,7 @@
softfork_active,
hex_str_to_bytes,
assert_raises_rpc_error,
get_datadir_path,
)

# The versionbit bit used to signal activation of SegWit
Expand Down Expand Up @@ -1956,23 +1959,41 @@ def test_non_standard_witness(self):
def test_upgrade_after_activation(self):
"""Test the behavior of starting up a segwit-aware node after the softfork has activated."""

self.restart_node(2, extra_args=["-segwitheight={}".format(SEGWIT_HEIGHT)])
# All nodes are caught up and node 2 is a pre-segwit node that will soon upgrade.
assert_equal(self.nodes[0].getblockcount(), self.nodes[2].getblockcount())
assert_equal(self.nodes[1].getblockcount(), self.nodes[2].getblockcount())
assert SEGWIT_HEIGHT < self.nodes[2].getblockcount()
assert softfork_active(self.nodes[0], 'segwit')
assert softfork_active(self.nodes[1], 'segwit')
assert 'segwit' not in self.nodes[2].getblockchaininfo()['softforks']

# Restarting node 2 should result in a shutdown because the blockchain consists of
# insufficiently validated blocks per segwit consensus rules.
self.stop_node(2)
with self.nodes[2].assert_debug_log(expected_msgs=["Segwit blocks are insufficiently validated. Please delete blocks dir and chain state dir and restart."], timeout=10):
self.nodes[2].start(["-segwitheight={}".format(SEGWIT_HEIGHT)])

# As directed, the user deletes the blocks and chainstate directories and restarts the node
datadir = get_datadir_path(self.options.tmpdir, 2)
blocks_dir = os.path.join(datadir, "regtest", "blocks")
chainstate_dir = os.path.join(datadir, "regtest", "chainstate")
shutil.rmtree(blocks_dir)
shutil.rmtree(chainstate_dir)

self.start_node(2, extra_args=["-segwitheight={}".format(SEGWIT_HEIGHT)])
assert_equal(self.nodes[2].getblockcount(), 0)
self.connect_nodes(0, 2)

# We reconnect more than 100 blocks, give it plenty of time
self.sync_blocks(timeout=240)

# Make sure that this peer thinks segwit has activated.
# The upgraded node syncs headers and performs redownload
assert_equal(self.nodes[0].getblockcount(), self.nodes[2].getblockcount())
assert_equal(self.nodes[1].getblockcount(), self.nodes[2].getblockcount())
assert softfork_active(self.nodes[0], 'segwit')
assert softfork_active(self.nodes[1], 'segwit')
assert softfork_active(self.nodes[2], 'segwit')

# Make sure this peer's blocks match those of node0.
height = self.nodes[2].getblockcount()
while height >= 0:
block_hash = self.nodes[2].getblockhash(height)
assert_equal(block_hash, self.nodes[0].getblockhash(height))
assert_equal(self.nodes[0].getblock(block_hash), self.nodes[2].getblock(block_hash))
height -= 1

@subtest # type: ignore
def test_witness_sigops(self):
"""Test sigop counting is correct inside witnesses."""
Expand Down

0 comments on commit 416854c

Please sign in to comment.