Refactor: move CheckBlockSignature function call inside CheckBlock.

furszy · furszy · commit 374f6c9efce9 · 2021-04-09T18:09:15.000-03:00
Github-Pull: #2295 Rebased-From: 47cae28
diff --git a/src/blockassembler.cpp b/src/blockassembler.cpp
@@ -226,7 +226,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
         if (chainActive.Tip() != pindexPrev) return nullptr; // new block came in, move on
 
         CValidationState state;
-        if (!TestBlockValidity(state, *pblock, pindexPrev, false, false)) {
+        if (!TestBlockValidity(state, *pblock, pindexPrev, false, false, false)) {
             throw std::runtime_error(
                     strprintf("%s: TestBlockValidity failed: %s", __func__, FormatStateMessage(state)));
         }
diff --git a/src/primitives/block.h b/src/primitives/block.h
@@ -94,7 +94,7 @@ class CBlock : public CBlockHeader
     std::vector<unsigned char> vchBlockSig;
 
     // memory only
-    mutable bool fChecked;
+    mutable bool fChecked{false};
 
     CBlock()
     {
diff --git a/src/validation.cpp b/src/validation.cpp
@@ -1376,7 +1376,7 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
 {
     AssertLockHeld(cs_main);
     // Check it again in case a previous version let a bad block in
-    if (!fAlreadyChecked && !CheckBlock(block, state, !fJustCheck, !fJustCheck)) {
+    if (!fAlreadyChecked && !CheckBlock(block, state, !fJustCheck, !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
@@ -2776,6 +2776,12 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo
         return state.DoS(100, error("%s : out-of-bounds SigOpCount", __func__),
             REJECT_INVALID, "bad-blk-sigops", true);
 
+    // Check PoS signature.
+    if (fCheckSig && !CheckBlockSignature(block)) {
+        return state.DoS(100, error("%s : bad proof-of-stake block signature", __func__),
+                         REJECT_INVALID, "bad-PoS-sig", true);
+    }
+
     if (fCheckPOW && fCheckMerkleRoot && fCheckSig)
         block.fChecked = true;
 
@@ -3268,9 +3274,6 @@ bool ProcessNewBlock(CValidationState& state, CNode* pfrom, const std::shared_pt
     // check block
     bool checked = CheckBlock(*pblock, state);
 
-    if (!CheckBlockSignature(*pblock))
-        return error("%s : bad proof-of-stake block signature", __func__);
-
     if (pblock->GetHash() != consensus.hashGenesisBlock && pfrom != NULL) {
         //if we get this far, check if the prev block is our prev block, if not then request sync and return false
         BlockMap::iterator mi = mapBlockIndex.find(pblock->hashPrevBlock);
@@ -3315,7 +3318,7 @@ bool ProcessNewBlock(CValidationState& state, CNode* pfrom, const std::shared_pt
     return true;
 }
 
-bool TestBlockValidity(CValidationState& state, const CBlock& block, CBlockIndex* const pindexPrev, bool fCheckPOW, bool fCheckMerkleRoot)
+bool TestBlockValidity(CValidationState& state, const CBlock& block, CBlockIndex* const pindexPrev, bool fCheckPOW, bool fCheckMerkleRoot, bool fCheckBlockSig)
 {
     AssertLockHeld(cs_main);
     assert(pindexPrev);
@@ -3332,7 +3335,7 @@ bool TestBlockValidity(CValidationState& state, const CBlock& block, CBlockIndex
     // NOTE: CheckBlockHeader is called by CheckBlock
     if (!ContextualCheckBlockHeader(block, state, pindexPrev))
         return error("%s: ContextualCheckBlockHeader failed: %s", __func__, FormatStateMessage(state));
-    if (!CheckBlock(block, state, fCheckPOW, fCheckMerkleRoot))
+    if (!CheckBlock(block, state, fCheckPOW, fCheckMerkleRoot, fCheckBlockSig))
         return error("%s: CheckBlock failed: %s", __func__, FormatStateMessage(state));
     if (!ContextualCheckBlock(block, state, pindexPrev))
         return error("%s: ContextualCheckBlock failed: %s", __func__, FormatStateMessage(state));
diff --git a/src/validation.h b/src/validation.h
@@ -332,7 +332,7 @@ bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& sta
 bool ContextualCheckBlock(const CBlock& block, CValidationState& state, CBlockIndex* pindexPrev);
 
 /** Check a block is completely valid from start to finish (only works on top of our current best block, with cs_main held) */
-bool TestBlockValidity(CValidationState& state, const CBlock& block, CBlockIndex* pindexPrev, bool fCheckPOW = true, bool fCheckMerkleRoot = true);
+bool TestBlockValidity(CValidationState& state, const CBlock& block, CBlockIndex* pindexPrev, bool fCheckPOW = true, bool fCheckMerkleRoot = true, bool fCheckBlockSig = true);
 
 /** Store block on disk. If dbp is provided, the file is known to already reside on disk */
 bool AcceptBlock(const CBlock& block, CValidationState& state, CBlockIndex** pindex, CDiskBlockPos* dbp = NULL, bool fAlreadyCheckedBlock = false);

Original file line number	Diff line number	Diff line change
`@@ -226,7 +226,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc`
`226`	`226`	`if (chainActive.Tip() != pindexPrev) return nullptr; // new block came in, move on`
`227`	`227`
`228`	`228`	`CValidationState state;`
`229`		`- if (!TestBlockValidity(state, *pblock, pindexPrev, false, false)) {`
	`229`	`+ if (!TestBlockValidity(state, *pblock, pindexPrev, false, false, false)) {`
`230`	`230`	`throw std::runtime_error(`
`231`	`231`	`strprintf("%s: TestBlockValidity failed: %s", __func__, FormatStateMessage(state)));`
`232`	`232`	`}`
Original file line number	Diff line number	Diff line change
`@@ -94,7 +94,7 @@ class CBlock : public CBlockHeader`
`94`	`94`	`std::vector<unsigned char> vchBlockSig;`
`95`	`95`
`96`	`96`	`// memory only`
`97`		`- mutable bool fChecked;`
	`97`	`+ mutable bool fChecked{false};`
`98`	`98`
`99`	`99`	`CBlock()`
`100`	`100`	`{`