From 25dada932aaa763a6795d55cc1114d944d1b4e1d Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Mon, 5 Mar 2018 10:42:26 -0500 Subject: [PATCH 1/2] Shut down if trying to connect a corrupted block The call to CheckBlock() in ConnectBlock() is redundant with calls to it prior to storing a block on disk. If CheckBlock() fails with an error indicating the block is potentially corrupted, then shut down immediately, as this is an indication that the node is experiencing hardware issues. (If we didn't shut down, we could go into an infinite loop trying to reconnect this same bad block, as we're not setting the block's status to FAILED in the case where there is potential corruption.) If CheckBlock() fails for some other reason, we'll end up flagging this block as bad (perhaps some prior software version "let a bad block in", as the comment indicates), and not trying to connect it again, so this case should be properly handled. --- src/validation.cpp | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/validation.cpp b/src/validation.cpp index 6b5dcb188..780495c57 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1768,8 +1768,27 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin int64_t nTimeStart = GetTimeMicros(); // Check it again in case a previous version let a bad block in - if (!CheckBlock(block, state, !fJustCheck, !fJustCheck)) + // NOTE: We don't currently (re-)invoke ContextualCheckBlock() or + // ContextualCheckBlockHeader() here. This means that if we add a new + // consensus rule that is enforced in one of those two functions, then we + // may have let in a block that violates the rule prior to updating the + // software, and we would NOT be enforcing the rule here. Fully solving + // upgrade from one software version to the next after a consensus rule + // change is potentially tricky and issue-specific (see RewindBlockIndex() + // for one general approach that was used for BIP 141 deployment). + // Also, currently the rule against blocks more than 2 hours in the future + // is enforced in ContextualCheckBlockHeader(); we wouldn't want to + // re-enforce that rule here (at least until we make it impossible for + // GetAdjustedTime() to go backward). + if (!CheckBlock(block, state, !fJustCheck, !fJustCheck)) { + if (state.CorruptionPossible()) { + // We don't write down blocks to disk if they may have been + // corrupted, so this should be impossible unless we're having hardware + // problems. + return AbortNode(state, "Corrupt block found indicating potential hardware failure; shutting down"); + } return error("%s: Consensus::CheckBlock: %s", __func__, FormatStateMessage(state)); + } // verify that the view's current state corresponds to the previous block uint256 hashPrevBlock = pindex->pprev == NULL ? uint256() : pindex->pprev->GetBlockHash(); From d5446c1a27fd3137d4d39c9514a01347e4765d31 Mon Sep 17 00:00:00 2001 From: Ross Nicoll Date: Mon, 21 Jun 2021 21:23:36 +0100 Subject: [PATCH 2/2] Log failing block hash Log failing block hash if an attempt is made to connect a corrupt block. --- src/validation.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/validation.cpp b/src/validation.cpp index 780495c57..12f795fe9 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1782,6 +1782,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin // GetAdjustedTime() to go backward). if (!CheckBlock(block, state, !fJustCheck, !fJustCheck)) { if (state.CorruptionPossible()) { + LogPrintf("%s: Attempt to connect corrupted block %s.\n", __func__, block.GetHash().ToString()); // We don't write down blocks to disk if they may have been // corrupted, so this should be impossible unless we're having hardware // problems.