diff --git a/qa/rpc-tests/abc-ec.py b/qa/rpc-tests/abc-ec.py --- a/qa/rpc-tests/abc-ec.py +++ b/qa/rpc-tests/abc-ec.py @@ -358,11 +358,18 @@ assert_equal(self.nodes[0].getbestblockhash(), big_block_hash) assert_equal(self.nodes[1].getbestblockhash(), node_1_last_hash) - # Update the excessive size and resync - self.nodes[1].setexcessiveblock(self.excessive_block_size_16) - self.sync_all() - assert_equal(self.nodes[0].getbestblockhash(), big_block_hash) - assert_equal(self.nodes[1].getbestblockhash(), big_block_hash) + # The Block was stored but it's not the active chain + + assert_equal(self.nodes[1].getblock( + big_block_hash)["hash"], big_block_hash) + assert_equal(self.nodes[1].getblock( + big_block_hash)["confirmations"], -1) + + # TODO: make the node revalidate excessive blocks when the policy is changed + # self.nodes[1].setexcessiveblock(self.excessive_block_size_16) + # self.sync_all() + # assert_equal(self.nodes[0].getbestblockhash(), big_block_hash) + # assert_equal(self.nodes[1].getbestblockhash(), big_block_hash) if __name__ == '__main__': FullBlockTest().main() diff --git a/src/chain.h b/src/chain.h --- a/src/chain.h +++ b/src/chain.h @@ -155,6 +155,9 @@ //!< excessive block BLOCK_EXCESSIVE = 128, + //!< excessive block or have data + BLOCK_EXCESSIVE_OR_STORED_DATA = BLOCK_EXCESSIVE | BLOCK_HAVE_DATA, + //!< mask for all the errors BLOCK_NOT_VALID_MASK = BLOCK_EXCESSIVE | BLOCK_FAILED_MASK }; @@ -258,7 +261,7 @@ CDiskBlockPos GetBlockPos() const { CDiskBlockPos ret; - if (nStatus & BLOCK_HAVE_DATA) { + if (nStatus & BLOCK_EXCESSIVE_OR_STORED_DATA) { ret.nFile = nFile; ret.nPos = nDataPos; } @@ -322,6 +325,12 @@ return ((nStatus & BLOCK_VALID_MASK) >= nUpTo); } + //! Check whether this block index entry is excessive + bool IsExcessive() const { + if (nStatus & BLOCK_EXCESSIVE) return false; + return true; + } + //! Raise the validity level of this block index entry. //! Returns true if the validity was changed. bool RaiseValidity(enum BlockStatus nUpTo) { @@ -373,9 +382,11 @@ READWRITE(VARINT(nHeight)); READWRITE(VARINT(nStatus)); READWRITE(VARINT(nTx)); - if (nStatus & (BLOCK_HAVE_DATA | BLOCK_HAVE_UNDO)) + if ((nStatus & (BLOCK_HAVE_DATA | BLOCK_HAVE_UNDO)) || + nStatus & BLOCK_EXCESSIVE) READWRITE(VARINT(nFile)); - if (nStatus & BLOCK_HAVE_DATA) READWRITE(VARINT(nDataPos)); + if (nStatus & BLOCK_EXCESSIVE_OR_STORED_DATA) + READWRITE(VARINT(nDataPos)); if (nStatus & BLOCK_HAVE_UNDO) READWRITE(VARINT(nUndoPos)); // block header diff --git a/src/net_processing.cpp b/src/net_processing.cpp --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2336,7 +2336,7 @@ mapBlocksInFlight.find(pindex->GetBlockHash()); bool fAlreadyInFlight = blockInFlightIt != mapBlocksInFlight.end(); - if (pindex->nStatus & BLOCK_HAVE_DATA) { + if (pindex->nStatus & BLOCK_EXCESSIVE_OR_STORED_DATA) { // Nothing to do here return true; } diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2483,6 +2483,7 @@ * invalid (it's however far from certain to be valid). */ static CBlockIndex *FindMostWorkChain() { + int iterator_offset = 0; do { CBlockIndex *pindexNew = nullptr; @@ -2490,7 +2491,24 @@ { std::set::reverse_iterator it = setBlockIndexCandidates.rbegin(); - if (it == setBlockIndexCandidates.rend()) return nullptr; + if (iterator_offset > 0) { + // Avoid using EC chains but do not remove them from the index + // map to not break the CheckBlockIndex + // This code will be upgraded to allow the node to select an EC + // chain as the MostWorkChain + int i = 0; + while (i < iterator_offset) { + it++; + i++; + // If the others chains have missing data or have bad blocks + // their blocks are removed + if (it == setBlockIndexCandidates.rend()) { + return nullptr; + } + } + } else { + if (it == setBlockIndexCandidates.rend()) return nullptr; + } pindexNew = *it; } @@ -2499,6 +2517,7 @@ // is an optimization, as we know all blocks in it are valid already. CBlockIndex *pindexTest = pindexNew; bool fInvalidAncestor = false; + bool fCurrentChainIsEC = false; while (pindexTest && !chainActive.Contains(pindexTest)) { assert(pindexTest->nChainTx || pindexTest->nHeight == 0); @@ -2506,8 +2525,16 @@ // 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_NOT_VALID_MASK; + bool fFailedChain = pindexTest->nStatus & BLOCK_FAILED_MASK; bool fMissingData = !(pindexTest->nStatus & BLOCK_HAVE_DATA); + + if (pindexTest->nStatus & BLOCK_EXCESSIVE) { + // Ignore this chain and continue + ++iterator_offset; + fCurrentChainIsEC = true; + break; + } + if (fFailedChain || fMissingData) { // Candidate chain is not usable (either invalid or missing // data) @@ -2537,7 +2564,7 @@ } pindexTest = pindexTest->pprev; } - if (!fInvalidAncestor) return pindexNew; + if (!fCurrentChainIsEC && !fInvalidAncestor) return pindexNew; } while (true); } @@ -2940,7 +2967,10 @@ pindexNew->nDataPos = pos.nPos; pindexNew->nUndoPos = 0; pindexNew->nStatus |= BLOCK_HAVE_DATA; - pindexNew->RaiseValidity(BLOCK_VALID_TRANSACTIONS); + if (!(pindexNew->nStatus & BLOCK_EXCESSIVE)) { + // The validations are only executed when the blocks are not excessive + pindexNew->RaiseValidity(BLOCK_VALID_TRANSACTIONS); + } setDirtyBlockIndex.insert(pindexNew); if (pindexNew->pprev == nullptr || pindexNew->pprev->nChainTx) { @@ -2977,7 +3007,9 @@ } } } else { - if (pindexNew->pprev && pindexNew->pprev->IsValid(BLOCK_VALID_TREE)) { + if (pindexNew->pprev && (pindexNew->pprev->IsValid(BLOCK_VALID_TREE) || + pindexNew->pprev->IsExcessive())) { + // Allow parent blocks to be excessive mapBlocksUnlinked.insert( std::make_pair(pindexNew->pprev, pindexNew)); } @@ -3498,6 +3530,7 @@ // process an unrequested block if it's new and has enough work to // advance our tip, and isn't too many blocks ahead. bool fAlreadyHave = pindex->nStatus & BLOCK_HAVE_DATA; + bool fIsExcessive = pindex->nStatus & BLOCK_EXCESSIVE; bool fHasMoreWork = (chainActive.Tip() ? pindex->nChainWork > chainActive.Tip()->nChainWork : true); @@ -3517,7 +3550,7 @@ // TODO: deal better with return value and error conditions for duplicate // and unrequested blocks. - if (fAlreadyHave) { + if (fAlreadyHave || fIsExcessive) { return true; } @@ -3543,6 +3576,8 @@ *fNewBlock = true; } + bool fIsBlockExcessive = false; + const CChainParams &chainparams = config.GetChainParams(); if (!CheckBlock(config, block, state, chainparams.GetConsensus()) || !ContextualCheckBlock(config, block, state, chainparams.GetConsensus(), @@ -3552,16 +3587,22 @@ setDirtyBlockIndex.insert(pindex); } else if (state.IsExcessive() && !state.CorruptionPossible()) { pindex->nStatus |= BLOCK_EXCESSIVE; - setDirtyBlockIndex.insert(pindex); + fIsBlockExcessive = true; + } + if (!fIsBlockExcessive) { + // If the block is excessive delay the return until the block is + // stored + return error("%s: %s (block %s)", __func__, + FormatStateMessage(state), block.GetHash().ToString()); } - return error("%s: %s (block %s)", __func__, FormatStateMessage(state), - block.GetHash().ToString()); } - // Header is valid/has work, merkle tree and segwit merkle tree are // good...RELAY NOW (but if it does not build on our best tip, let the // SendMessages loop relay it) - if (!IsInitialBlockDownload() && chainActive.Tip() == pindex->pprev) { + + // Do not relay excessive blocks, they may be invalid + if (!IsInitialBlockDownload() && chainActive.Tip() == pindex->pprev && + !fIsBlockExcessive) { GetMainSignals().NewPoWValidBlock(pindex, pblock); } @@ -3597,6 +3638,13 @@ FlushStateToDisk(state, FLUSH_STATE_NONE); } + if (fIsBlockExcessive) { + // After saving the excessive block to disk, inform that the block is + // bigger than the defined max size + return error("%s: %s (block %s)", __func__, FormatStateMessage(state), + block.GetHash().ToString()); + } + return true; } @@ -3617,8 +3665,8 @@ LOCK(cs_main); - if (ret) { - // Store to disk + if (ret || state.IsExcessive()) { + // Store to disk if there are not errors or the block is excessive ret = AcceptBlock(config, pblock, state, &pindex, fForceProcessing, nullptr, fNewBlock); } @@ -4542,31 +4590,39 @@ pindex->nStatus & BLOCK_FAILED_VALID) { pindexFirstInvalid = pindex; } - if (pindexFirstMissing == nullptr && - !(pindex->nStatus & BLOCK_HAVE_DATA)) { - pindexFirstMissing = pindex; - } - if (pindexFirstNeverProcessed == nullptr && pindex->nTx == 0) { - pindexFirstNeverProcessed = pindex; - } - if (pindex->pprev != nullptr && pindexFirstNotTreeValid == nullptr && - (pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_TREE) { - pindexFirstNotTreeValid = pindex; - } - if (pindex->pprev != nullptr && - pindexFirstNotTransactionsValid == nullptr && - (pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_TRANSACTIONS) { - pindexFirstNotTransactionsValid = pindex; - } - if (pindex->pprev != nullptr && pindexFirstNotChainValid == nullptr && - (pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_CHAIN) { - pindexFirstNotChainValid = pindex; - } - if (pindex->pprev != nullptr && pindexFirstNotScriptsValid == nullptr && - (pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_SCRIPTS) { - pindexFirstNotScriptsValid = pindex; + if (!(pindex->nStatus & BLOCK_EXCESSIVE)) { + // If the block is excessive this validations were not executed + // This need to be validated before making the Excessive chain an + // active chain + if (pindexFirstMissing == nullptr && + !(pindex->nStatus & BLOCK_HAVE_DATA)) { + pindexFirstMissing = pindex; + } + if (pindexFirstNeverProcessed == nullptr && pindex->nTx == 0) { + pindexFirstNeverProcessed = pindex; + } + if (pindex->pprev != nullptr && + pindexFirstNotTreeValid == nullptr && + (pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_TREE) { + pindexFirstNotTreeValid = pindex; + } + if (pindex->pprev != nullptr && + pindexFirstNotTransactionsValid == nullptr && + (pindex->nStatus & BLOCK_VALID_MASK) < + BLOCK_VALID_TRANSACTIONS) { + pindexFirstNotTransactionsValid = pindex; + } + if (pindex->pprev != nullptr && + pindexFirstNotChainValid == nullptr && + (pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_CHAIN) { + pindexFirstNotChainValid = pindex; + } + if (pindex->pprev != nullptr && + pindexFirstNotScriptsValid == nullptr && + (pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_SCRIPTS) { + pindexFirstNotScriptsValid = pindex; + } } - // Begin: actual consistency checks. if (pindex->pprev == nullptr) { // Genesis block checks. @@ -4586,7 +4642,8 @@ if (!fHavePruned) { // If we've never pruned, then HAVE_DATA should be equivalent to nTx // > 0 - assert(!(pindex->nStatus & BLOCK_HAVE_DATA) == (pindex->nTx == 0)); + assert(!(pindex->nStatus & BLOCK_EXCESSIVE_OR_STORED_DATA) == + (pindex->nTx == 0)); assert(pindexFirstMissing == pindexFirstNeverProcessed); } else { // If we have pruned, then we can only say that HAVE_DATA implies @@ -4597,8 +4654,9 @@ assert(pindex->nStatus & BLOCK_HAVE_DATA); } // This is pruning-independent. - assert(((pindex->nStatus & BLOCK_VALID_MASK) >= - BLOCK_VALID_TRANSACTIONS) == (pindex->nTx > 0)); + assert((((pindex->nStatus & BLOCK_VALID_MASK) >= + BLOCK_VALID_TRANSACTIONS || + pindex->nStatus & BLOCK_EXCESSIVE)) == (pindex->nTx > 0)); // All parents having had data (at some point) is equivalent to all // parents being VALID_TRANSACTIONS, which is equivalent to nChainTx // being set. @@ -4671,7 +4729,8 @@ } rangeUnlinked.first++; } - if (pindex->pprev && (pindex->nStatus & BLOCK_HAVE_DATA) && + if (pindex->pprev && + (pindex->nStatus & BLOCK_EXCESSIVE_OR_STORED_DATA) && pindexFirstNeverProcessed != nullptr && pindexFirstInvalid == nullptr) { // If this block has block data available, some parent was never @@ -4679,7 +4738,7 @@ // mapBlocksUnlinked. assert(foundInUnlinked); } - if (!(pindex->nStatus & BLOCK_HAVE_DATA)) { + if (!(pindex->nStatus & BLOCK_EXCESSIVE_OR_STORED_DATA)) { // Can't be in mapBlocksUnlinked if we don't HAVE_DATA assert(!foundInUnlinked); } @@ -4688,7 +4747,8 @@ // mapBlocksUnlinked. assert(!foundInUnlinked); } - if (pindex->pprev && (pindex->nStatus & BLOCK_HAVE_DATA) && + if (pindex->pprev && + (pindex->nStatus & BLOCK_EXCESSIVE_OR_STORED_DATA) && pindexFirstNeverProcessed == nullptr && pindexFirstMissing != nullptr) { // We HAVE_DATA for this block, have received data for all parents