From 828940b1634f80dcfed5491dc370324f209f2aaa Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 6 Feb 2018 12:39:20 +0000 Subject: [PATCH] Use a boost::optional for nCachedBranchId This enables us to distinguish between it being unset vs. being set to zero. --- src/chain.h | 20 +++++++++++++++----- src/main.cpp | 32 ++++++++++++++++++++------------ src/txdb.cpp | 2 +- 3 files changed, 36 insertions(+), 18 deletions(-) diff --git a/src/chain.h b/src/chain.h index c0239d5d2..937e20bda 100644 --- a/src/chain.h +++ b/src/chain.h @@ -147,9 +147,9 @@ public: unsigned int nStatus; //! Branch ID corresponding to the consensus rules used to validate this block. - //! Only accurate if block validity is BLOCK_VALID_CONSENSUS. + //! Only cached if block validity is BLOCK_VALID_CONSENSUS. //! Persisted at each activation height, memory-only for intervening blocks. - uint32_t nConsensusBranchId; + boost::optional nCachedBranchId; //! The anchor for the tree state up to the start of this block uint256 hashAnchor; @@ -191,7 +191,7 @@ public: nTx = 0; nChainTx = 0; nStatus = 0; - nConsensusBranchId = 0; + nCachedBranchId = boost::none; hashAnchor = uint256(); hashAnchorEnd = uint256(); nSequenceId = 0; @@ -353,8 +353,18 @@ public: READWRITE(VARINT(nDataPos)); if (nStatus & BLOCK_HAVE_UNDO) READWRITE(VARINT(nUndoPos)); - if (nStatus & BLOCK_ACTIVATES_UPGRADE) - READWRITE(nConsensusBranchId); + if (nStatus & BLOCK_ACTIVATES_UPGRADE) { + if (ser_action.ForRead()) { + uint32_t branchId; + READWRITE(branchId); + nCachedBranchId = branchId; + } else { + // nCachedBranchId must always be set if BLOCK_ACTIVATES_UPGRADE is set. + assert(nCachedBranchId); + uint32_t branchId = *nCachedBranchId; + READWRITE(branchId); + } + } READWRITE(hashAnchor); // block header diff --git a/src/main.cpp b/src/main.cpp index 9aed20d64..062507d0e 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2223,15 +2223,15 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin pindex->nStatus |= BLOCK_HAVE_UNDO; } - // Now that all consensus rules have been validated, set nConsensusBranchId. + // Now that all consensus rules have been validated, set nCachedBranchId. // Move this if BLOCK_VALID_CONSENSUS is ever altered. static_assert(BLOCK_VALID_CONSENSUS == BLOCK_VALID_SCRIPTS, - "nConsensusBranchId must be set after all consensus rules have been validated."); + "nCachedBranchId must be set after all consensus rules have been validated."); if (IsActivationHeightForAnyUpgrade(pindex->nHeight, Params().GetConsensus())) { pindex->nStatus |= BLOCK_ACTIVATES_UPGRADE; - pindex->nConsensusBranchId = CurrentEpochBranchId(pindex->nHeight, chainparams.GetConsensus()); + pindex->nCachedBranchId = CurrentEpochBranchId(pindex->nHeight, chainparams.GetConsensus()); } else if (pindex->pprev) { - pindex->nConsensusBranchId = pindex->pprev->nConsensusBranchId; + pindex->nCachedBranchId = pindex->pprev->nCachedBranchId; } pindex->RaiseValidity(BLOCK_VALID_SCRIPTS); @@ -3577,9 +3577,15 @@ bool static LoadBlockIndexDB() // Construct in-memory chain of branch IDs. // Relies on invariant: a block that does not activate a network upgrade // will always be valid under the same consensus rules as its parent. - // Activation blocks will have non-zero branch IDs (read from disk). - if (pindex->IsValid(BLOCK_VALID_CONSENSUS) && pindex->nConsensusBranchId == 0 && pindex->pprev) { - pindex->nConsensusBranchId = pindex->pprev->nConsensusBranchId; + // Genesis block has a branch ID of zero by definition, but has no + // validity status because it is side-loaded into a fresh chain. + // Activation blocks will have branch IDs set (read from disk). + if (pindex->pprev) { + if (pindex->IsValid(BLOCK_VALID_CONSENSUS) && !pindex->nCachedBranchId) { + pindex->nCachedBranchId = pindex->pprev->nCachedBranchId; + } + } else { + pindex->nCachedBranchId = NetworkUpgradeInfo[Consensus::BASE_SPROUT].nBranchId; } if (pindex->IsValid(BLOCK_VALID_TRANSACTIONS) && (pindex->nChainTx || pindex->pprev == NULL)) setBlockIndexCandidates.insert(pindex); @@ -3769,19 +3775,21 @@ bool RewindBlockIndex(const CChainParams& params) LOCK(cs_main); // RewindBlockIndex is called after LoadBlockIndex, so at this point every block - // index will have nConsensusBranchId set based on the values previously persisted - // to disk. By definition, a non-zero nConsensusBranchId means that the block was + // index will have nCachedBranchId set based on the values previously persisted + // to disk. By definition, a set nCachedBranchId means that the block was // fully-validated under the corresponding consensus rules. Thus we can quickly // identify whether the current active chain matches our expected sequence of // consensus rule changes, with two checks: // // - BLOCK_ACTIVATES_UPGRADE is set only on blocks that activate upgrades. - // - nConsensusBranchId for each block matches what we expect. + // - nCachedBranchId for each block matches what we expect. auto sufficientlyValidated = [¶ms](const CBlockIndex* pindex) { auto consensus = params.GetConsensus(); bool fFlagSet = pindex->nStatus & BLOCK_ACTIVATES_UPGRADE; bool fFlagExpected = IsActivationHeightForAnyUpgrade(pindex->nHeight, consensus); - return fFlagSet == fFlagExpected && pindex->nConsensusBranchId == CurrentEpochBranchId(pindex->nHeight, consensus); + return fFlagSet == fFlagExpected && + pindex->nCachedBranchId && + *pindex->nCachedBranchId == CurrentEpochBranchId(pindex->nHeight, consensus); }; int nHeight = 1; @@ -3832,7 +3840,7 @@ bool RewindBlockIndex(const CChainParams& params) pindexIter->nStatus &= ~(BLOCK_HAVE_DATA | BLOCK_HAVE_UNDO); // Remove branch ID pindexIter->nStatus &= ~BLOCK_ACTIVATES_UPGRADE; - pindexIter->nConsensusBranchId = 0; + pindexIter->nCachedBranchId = boost::none; // Remove storage location pindexIter->nFile = 0; pindexIter->nDataPos = 0; diff --git a/src/txdb.cpp b/src/txdb.cpp index ec0234ba2..1b66d70b4 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -309,7 +309,7 @@ bool CBlockTreeDB::LoadBlockIndexGuts() pindexNew->nNonce = diskindex.nNonce; pindexNew->nSolution = diskindex.nSolution; pindexNew->nStatus = diskindex.nStatus; - pindexNew->nConsensusBranchId = diskindex.nConsensusBranchId; + pindexNew->nCachedBranchId = diskindex.nCachedBranchId; pindexNew->nTx = diskindex.nTx; pindexNew->nSproutValue = diskindex.nSproutValue;