Skip to content

Commit

Permalink
[validation] Refactor ConnectTrace and PerBlockConnectTrace
Browse files Browse the repository at this point in the history 
Rename PerBlockConnectTrace to BlockAndIndex now that it's just a struct
of two members.

Now that ConnectTrace is basically just a struct containing a vector of
BlockAndIndex, replace it with a typedef vector called BlocksTrace.

Co-authored by Antoine Riard <ariard@student.42.fr>
  • Loading branch information
@jnewbery
jnewbery committed Nov 22, 2019
1 parent fb394a9 commit cb155c9
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 40 deletions.
57 changes: 20 additions & 37 deletions src/validation.cpp
View file
Expand Up @@ -2465,43 +2465,24 @@ static int64_t nTimeFlush = 0;
static int64_t nTimeChainState = 0;
static int64_t nTimePostConnect = 0;

struct PerBlockConnectTrace {
CBlockIndex* pindex = nullptr;
std::shared_ptr<const CBlock> pblock;
PerBlockConnectTrace(CBlockIndex* pindexIn, std::shared_ptr<const CBlock> pblockIn) : pindex(pindexIn), pblock(pblockIn) {}
struct BlockAndIndex {
CBlockIndex* index = nullptr;
std::shared_ptr<const CBlock> block;
BlockAndIndex(CBlockIndex* index_in, std::shared_ptr<const CBlock> block_in) : index(index_in), block(block_in) {}
};

/**
* Used to track blocks whose transactions were applied to the UTXO state as a
* part of a single ActivateBestChainStep call.
*
* This class is single-use, once you call GetBlocksConnected() you have to throw
* it away and make a new one.
* Used to track blocks that were connected as part of a single ActivateBestChainStep call.
*/
class ConnectTrace {
private:
std::vector<PerBlockConnectTrace> blocksConnected;

public:
explicit ConnectTrace() {}

void BlockConnected(CBlockIndex* pindex, std::shared_ptr<const CBlock> pblock) {
assert(pindex);
assert(pblock);
blocksConnected.emplace_back(pindex, std::move(pblock));
}

std::vector<PerBlockConnectTrace>& GetBlocksConnected() {
return blocksConnected;
}
};
typedef std::vector<BlockAndIndex> BlocksTrace;

/**
* Connect a new block to m_chain. pblock is either nullptr or a pointer to a CBlock
* corresponding to pindexNew, to bypass loading it again from disk.
*
* The block is added to connectTrace if connection succeeds.
* The block is added to BlocksTrace if connection succeeds.
*/
bool CChainState::ConnectTip(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool)
bool CChainState::ConnectTip(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, BlocksTrace& blocks_trace, DisconnectedBlockTransactions &disconnectpool)
{
assert(pindexNew->pprev == m_chain.Tip());
// Read block from disk.
Expand Down Expand Up @@ -2552,7 +2533,9 @@ bool CChainState::ConnectTip(BlockValidationState& state, const CChainParams& ch
LogPrint(BCLog::BENCH, " - Connect postprocess: %.2fms [%.2fs (%.2fms/blk)]\n", (nTime6 - nTime5) * MILLI, nTimePostConnect * MICRO, nTimePostConnect * MILLI / nBlocksTotal);
LogPrint(BCLog::BENCH, "- Connect block: %.2fms [%.2fs (%.2fms/blk)]\n", (nTime6 - nTime1) * MILLI, nTimeTotal * MICRO, nTimeTotal * MILLI / nBlocksTotal);

connectTrace.BlockConnected(pindexNew, std::move(pthisBlock));
assert(pindexNew);
assert(pthisBlock);
blocks_trace.emplace_back(pindexNew, std::move(pthisBlock));
return true;
}

Expand Down Expand Up @@ -2633,7 +2616,7 @@ void CChainState::PruneBlockIndexCandidates() {
*
* @returns true unless a system error occurred
*/
bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace)
bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, BlocksTrace& blocks_trace)
{
AssertLockHeld(cs_main);

Expand Down Expand Up @@ -2677,7 +2660,7 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChai

// Connect new blocks.
for (CBlockIndex *pindexConnect : reverse_iterate(vpindexToConnect)) {
if (!ConnectTip(state, chainparams, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr<const CBlock>(), connectTrace, disconnectpool)) {
if (!ConnectTip(state, chainparams, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr<const CBlock>(), blocks_trace, disconnectpool)) {
if (state.IsInvalid()) {
// The block violates a consensus rule.
if (state.GetResult() != BlockValidationResult::BLOCK_MUTATED) {
Expand Down Expand Up @@ -2779,13 +2762,13 @@ bool CChainState::ActivateBestChain(BlockValidationState &state, const CChainPar
LimitValidationInterfaceQueue();

{
LOCK2(cs_main, ::mempool.cs); // Lock transaction pool for at least as long as it takes for connectTrace to be consumed
LOCK2(cs_main, ::mempool.cs); // Lock transaction pool for at least as long as it takes for blocks_trace to be consumed
CBlockIndex* starting_tip = m_chain.Tip();
bool blocks_connected = false;
do {
// We absolutely may not unlock cs_main until we've made forward progress
// (with the exception of shutdown due to hardware issues, low disk space, etc).
ConnectTrace connectTrace; // Destructed before cs_main is unlocked
BlocksTrace blocks_trace; // Destructed before cs_main is unlocked

if (pindexMostWork == nullptr) {
pindexMostWork = FindMostWorkChain();
Expand All @@ -2798,7 +2781,7 @@ bool CChainState::ActivateBestChain(BlockValidationState &state, const CChainPar

bool fInvalidFound = false;
std::shared_ptr<const CBlock> nullBlockPtr;
if (!ActivateBestChainStep(state, chainparams, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : nullBlockPtr, fInvalidFound, connectTrace)) {
if (!ActivateBestChainStep(state, chainparams, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : nullBlockPtr, fInvalidFound, blocks_trace)) {
// A system error occurred
return false;
}
Expand All @@ -2810,9 +2793,9 @@ bool CChainState::ActivateBestChain(BlockValidationState &state, const CChainPar
}
pindexNewTip = m_chain.Tip();

for (const PerBlockConnectTrace& trace : connectTrace.GetBlocksConnected()) {
assert(trace.pblock && trace.pindex);
GetMainSignals().BlockConnected(trace.pblock, trace.pindex);
for (const BlockAndIndex trace : blocks_trace) {
assert(trace.block && trace.index);
GetMainSignals().BlockConnected(trace.block, trace.index);
}
} while (!m_chain.Tip() || (starting_tip && CBlockIndexWorkComparator()(m_chain.Tip(), starting_tip)));
if (!blocks_connected) return true;
Expand Down
7 changes: 4 additions & 3 deletions src/validation.h
View file
Expand Up @@ -412,7 +412,8 @@ enum DisconnectResult
DISCONNECT_FAILED // Something else went wrong.
};

class ConnectTrace;
struct BlockAndIndex;
typedef std::vector<BlockAndIndex> BlocksTrace;

/** @see CChainState::FlushStateToDisk */
enum class FlushStateMode {
Expand Down Expand Up @@ -723,8 +724,8 @@ class CChainState {
bool LoadChainTip(const CChainParams& chainparams) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

private:
bool ActivateBestChainStep(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs);
bool ConnectTip(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs);
bool ActivateBestChainStep(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, BlocksTrace& blocks_trace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs);
bool ConnectTip(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, BlocksTrace& blocks_trace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs);

void InvalidBlockFound(CBlockIndex *pindex, const BlockValidationState &state) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
CBlockIndex* FindMostWorkChain() EXCLUSIVE_LOCKS_REQUIRED(cs_main);
Expand Down

0 comments on commit cb155c9

Please sign in to comment.