diff --git a/src/chain.h b/src/chain.h --- a/src/chain.h +++ b/src/chain.h @@ -201,7 +201,7 @@ BLOCK_FAILED_VALID = 32, // The block has an invalid parent. BLOCK_FAILED_CHILD = 64, - BLOCK_FAILED_MASK = BLOCK_FAILED_VALID | BLOCK_FAILED_CHILD, + BLOCK_INVALID_MASK = BLOCK_FAILED_VALID | BLOCK_FAILED_CHILD, }; struct BlockStatus { @@ -233,18 +233,37 @@ (hasUndo ? BLOCK_HAVE_UNDO : BLOCK_HAVE_NOTHING)); } + bool hasFailed() const { return status & BLOCK_FAILED_VALID; } + BlockStatus withFailed(bool hasFailed = true) const { + return BlockStatus( + (status & ~BLOCK_FAILED_VALID) | + (hasFailed ? BLOCK_FAILED_VALID : BLOCK_HAVE_NOTHING)); + } + + bool hasFailedParent() const { return status & BLOCK_FAILED_CHILD; } + BlockStatus withFailedParent(bool hasFailedParent = true) const { + return BlockStatus( + (status & ~BLOCK_FAILED_CHILD) | + (hasFailedParent ? BLOCK_FAILED_CHILD : BLOCK_HAVE_NOTHING)); + } + /** * Check whether this block index entry is valid up to the passed validity * level. */ bool isValid(enum BlockValidity nUpTo = BlockValidity::TRANSACTIONS) const { - if (status & BLOCK_FAILED_MASK) { + if (isInvalid()) { return false; } return getValidity() >= nUpTo; } + bool isInvalid() const { return status & BLOCK_INVALID_MASK; } + BlockStatus withClearedFailureFlags() const { + return withFailed(false).withFailedParent(false); + } + // To transition from this and the plain old intereger. // TODO: delete. uint32_t operator&(uint32_t rhs) const { return status & rhs; } @@ -253,6 +272,7 @@ this->status &= rhs; return *this; } + BlockStatus &operator|=(uint32_t rhs) { this->status |= rhs; return *this; @@ -447,11 +467,11 @@ //! Returns true if the validity was changed. bool RaiseValidity(enum BlockValidity nUpTo) { // Only validity flags allowed. - if (nStatus & BLOCK_FAILED_MASK) { + if (nStatus.isInvalid()) { return false; } - if (BlockValidity(nStatus & BLOCK_VALID_MASK) >= nUpTo) { + if (nStatus.getValidity() >= nUpTo) { return false; } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1438,7 +1438,7 @@ if (chainActive.Contains(block)) { // This block is part of the currently active chain. status = "active"; - } else if (block->nStatus & BLOCK_FAILED_MASK) { + } else if (block->nStatus.isInvalid()) { // This block or one of its ancestors is invalid. status = "invalid"; } else if (block->nChainTx == 0) { diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -504,7 +504,7 @@ if (pindex->IsValid(BlockValidity::SCRIPTS)) { return "duplicate"; } - if (pindex->nStatus & BLOCK_FAILED_MASK) { + if (pindex->nStatus.isInvalid()) { return "duplicate-invalid"; } return "duplicate-inconclusive"; @@ -859,7 +859,7 @@ if (pindex->IsValid(BlockValidity::SCRIPTS)) { return "duplicate"; } - if (pindex->nStatus & BLOCK_FAILED_MASK) { + if (pindex->nStatus.isInvalid()) { return "duplicate-invalid"; } // Otherwise, we might only have the header - process the block diff --git a/src/test/blockstatus_tests.cpp b/src/test/blockstatus_tests.cpp --- a/src/test/blockstatus_tests.cpp +++ b/src/test/blockstatus_tests.cpp @@ -11,16 +11,21 @@ BOOST_FIXTURE_TEST_SUITE(blockstatus_tests, BasicTestingSetup) -static void CheckBlockStatus(BlockStatus s, BlockValidity validity, - bool hasData, bool hasUndo) { +static void CheckBlockStatus(const BlockStatus s, BlockValidity validity, + bool hasData, bool hasUndo, bool hasFailed, + bool hasFailedParent) { BOOST_CHECK(s.getValidity() == validity); BOOST_CHECK_EQUAL(s.hasData(), hasData); BOOST_CHECK_EQUAL(s.hasUndo(), hasUndo); + BOOST_CHECK_EQUAL(s.hasFailed(), hasFailed); + BOOST_CHECK_EQUAL(s.hasFailedParent(), hasFailedParent); + BOOST_CHECK_EQUAL(s.isInvalid(), hasFailed || hasFailedParent); } BOOST_AUTO_TEST_CASE(sighash_construction_test) { // Check default values. - CheckBlockStatus(BlockStatus(), BlockValidity::UNKNOWN, false, false); + CheckBlockStatus(BlockStatus(), BlockValidity::UNKNOWN, false, false, false, + false); // Check all possible permutations. std::set baseValidities{ @@ -29,30 +34,59 @@ BlockValidity::CHAIN, BlockValidity::SCRIPTS}; std::set hasDataValues{false, true}; std::set hasUndoValues{false, true}; + std::set hasFailedValues{false, true}; + std::set hasFailedParentValues{false, true}; for (BlockValidity validity : baseValidities) { for (bool hasData : hasDataValues) { for (bool hasUndo : hasUndoValues) { - const BlockStatus s = BlockStatus() - .withValidity(validity) - .withData(hasData) - .withUndo(hasUndo); - - CheckBlockStatus(s, validity, hasData, hasUndo); - - // Also check all possible alterations. - CheckBlockStatus(s.withData(hasData), validity, hasData, - hasUndo); - CheckBlockStatus(s.withData(!hasData), validity, !hasData, - hasUndo); - CheckBlockStatus(s.withUndo(hasUndo), validity, hasData, - hasUndo); - CheckBlockStatus(s.withUndo(!hasUndo), validity, hasData, - !hasUndo); - - for (BlockValidity newValidity : baseValidities) { - CheckBlockStatus(s.withValidity(newValidity), newValidity, - hasData, hasUndo); + for (bool hasFailed : hasFailedValues) { + for (bool hasFailedParent : hasFailedParentValues) { + const BlockStatus s = + BlockStatus() + .withValidity(validity) + .withData(hasData) + .withUndo(hasUndo) + .withFailed(hasFailed) + .withFailedParent(hasFailedParent); + + CheckBlockStatus(s, validity, hasData, hasUndo, + hasFailed, hasFailedParent); + + // Clears failure flags. + CheckBlockStatus(s.withClearedFailureFlags(), validity, + hasData, hasUndo, false, false); + + // Also check all possible alterations. + CheckBlockStatus(s.withData(hasData), validity, hasData, + hasUndo, hasFailed, hasFailedParent); + CheckBlockStatus(s.withData(!hasData), validity, + !hasData, hasUndo, hasFailed, + hasFailedParent); + CheckBlockStatus(s.withUndo(hasUndo), validity, hasData, + hasUndo, hasFailed, hasFailedParent); + CheckBlockStatus(s.withUndo(!hasUndo), validity, + hasData, !hasUndo, hasFailed, + hasFailedParent); + CheckBlockStatus(s.withFailed(hasFailed), validity, + hasData, hasUndo, hasFailed, + hasFailedParent); + CheckBlockStatus(s.withFailed(!hasFailed), validity, + hasData, hasUndo, !hasFailed, + hasFailedParent); + CheckBlockStatus(s.withFailedParent(hasFailedParent), + validity, hasData, hasUndo, hasFailed, + hasFailedParent); + CheckBlockStatus(s.withFailedParent(!hasFailedParent), + validity, hasData, hasUndo, hasFailed, + !hasFailedParent); + + for (BlockValidity newValidity : baseValidities) { + CheckBlockStatus(s.withValidity(newValidity), + newValidity, hasData, hasUndo, + hasFailed, hasFailedParent); + } + } } } } diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2874,12 +2874,12 @@ // which block files have been deleted. Remove those as candidates // for the most work chain if we come across them; we can't switch // to a chain unless we have all the non-active-chain parent blocks. - bool fFailedChain = pindexTest->nStatus & BLOCK_FAILED_MASK; + bool fInvalidChain = pindexTest->nStatus.isInvalid(); bool fMissingData = !pindexTest->nStatus.hasData(); - if (fFailedChain || fMissingData) { + if (fInvalidChain || fMissingData) { // Candidate chain is not usable (either invalid or missing // data) - if (fFailedChain && + if (fInvalidChain && (pindexBestInvalid == nullptr || pindexNew->nChainWork > pindexBestInvalid->nChainWork)) { pindexBestInvalid = pindexNew; @@ -2887,7 +2887,7 @@ CBlockIndex *pindexFailed = pindexNew; // Remove the entire chain from the set. while (pindexTest != pindexFailed) { - if (fFailedChain) { + if (fInvalidChain) { pindexFailed->nStatus |= BLOCK_FAILED_CHILD; } else if (fMissingData) { // If we're missing data, then add back to @@ -3225,7 +3225,7 @@ while (it != mapBlockIndex.end()) { if (!it->second->IsValid() && it->second->GetAncestor(nHeight) == pindex) { - it->second->nStatus &= ~BLOCK_FAILED_MASK; + it->second->nStatus = it->second->nStatus.withClearedFailureFlags(); setDirtyBlockIndex.insert(it->second); if (it->second->IsValid(BlockValidity::TRANSACTIONS) && it->second->nChainTx && @@ -3244,8 +3244,8 @@ // Remove the invalidity flag from all ancestors too. while (pindex != nullptr) { - if (pindex->nStatus & BLOCK_FAILED_MASK) { - pindex->nStatus &= ~BLOCK_FAILED_MASK; + if (pindex->nStatus.isInvalid()) { + pindex->nStatus = pindex->nStatus.withClearedFailureFlags(); setDirtyBlockIndex.insert(pindex); } pindex = pindex->pprev; @@ -3793,7 +3793,7 @@ if (ppindex) { *ppindex = pindex; } - if (pindex->nStatus & BLOCK_FAILED_MASK) { + if (pindex->nStatus.isInvalid()) { return state.Invalid(error("%s: block %s is marked invalid", __func__, hash.ToString()), 0, "duplicate"); @@ -3815,7 +3815,7 @@ } pindexPrev = (*mi).second; - if (pindexPrev->nStatus & BLOCK_FAILED_MASK) { + if (pindexPrev->nStatus.isInvalid()) { return state.DoS(100, error("%s: prev block invalid", __func__), REJECT_INVALID, "bad-prevblk"); } @@ -4376,7 +4376,7 @@ (pindex->nChainTx || pindex->pprev == nullptr)) { setBlockIndexCandidates.insert(pindex); } - if (pindex->nStatus & BLOCK_FAILED_MASK && + if (pindex->nStatus.isInvalid() && (!pindexBestInvalid || pindex->nChainWork > pindexBestInvalid->nChainWork)) { pindexBestInvalid = pindex; @@ -5195,7 +5195,7 @@ if (pindexFirstInvalid == nullptr) { // Checks for not-invalid blocks. // The failed mask cannot be set for blocks without invalid parents. - assert((pindex->nStatus & BLOCK_FAILED_MASK) == 0); + assert(!pindex->nStatus.isInvalid()); } if (!CBlockIndexWorkComparator()(pindex, chainActive.Tip()) && pindexFirstNeverProcessed == nullptr) {