Further minor changes following intermediate code review

src/gtest/test_checkblock.cpp

@@ -39,7 +39,7 @@ TEST(CheckBlock, VersionTooLow) {
 
     MockCValidationState state;
     EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "version-invalid", false)).Times(1);
-    EXPECT_FALSE(CheckBlock(block, state, verifier, false, false));
+    EXPECT_FALSE(CheckBlock(block, state, verifier, flagCheckPow::OFF, flagCheckMerkleRoot::OFF));
 }
 
 
@@ -69,7 +69,7 @@ TEST(CheckBlock, BlockRejectsBadVersion) {
     auto verifier = libzcash::ProofVerifier::Strict();
 
     EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-version-too-low", false)).Times(1);
-    EXPECT_FALSE(CheckBlock(block, state, verifier, false, false));
+    EXPECT_FALSE(CheckBlock(block, state, verifier, flagCheckPow::OFF, flagCheckMerkleRoot::OFF));
 }
 
 

src/gtest/test_processTxBaseMsg.cpp

@@ -102,13 +102,13 @@ class FakeNode : public CNodeInterface
     bool fWhitelisted;
     std::string commandInvoked;
 
-    void AddInventoryKnown(const CInv& inv)  {}; //dummyImpl
-    NodeId GetId() const  {return 1987;};
-    virtual bool IsWhiteListed() const  {return fWhitelisted; };
-    std::string GetCleanSubVer() const  { return std::string{}; };
-    void StopAskingFor(const CInv& inv) { return; }
+    void AddInventoryKnown(const CInv& inv) override final  {}; //dummyImpl
+    NodeId GetId() const override final {return 1987;};
+    virtual bool IsWhiteListed() const override final {return fWhitelisted; };
+    std::string GetCleanSubVer() const override final { return std::string{}; };
+    void StopAskingFor(const CInv& inv) override final { return; }
     void PushMessage(const char* pszCommand, const std::string& param1, unsigned char param2,
-                     const std::string& param3, const uint256& param4)
+                     const std::string& param3, const uint256& param4) override final
     {
         commandInvoked = std::string(pszCommand) + param1;
         return;

src/main.cpp

@@ -2703,7 +2703,9 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin
     auto disabledVerifier = libzcash::ProofVerifier::Disabled();
 
     // Check it again to verify JoinSplit proofs, and in case a previous version let a bad block in
-    if (!CheckBlock(block, state, fExpensiveChecks ? verifier : disabledVerifier, !fJustCheck, !fJustCheck))
+    if (!CheckBlock(block, state, fExpensiveChecks ? verifier : disabledVerifier,
+                    !fJustCheck? flagCheckPow::ON : flagCheckPow::OFF,
+                    !fJustCheck? flagCheckMerkleRoot::ON: flagCheckMerkleRoot::OFF))
         return false;
 
     // verify that the view's current state corresponds to the previous block
@@ -4063,20 +4065,20 @@ bool FindUndoPos(CValidationState &state, int nFile, CDiskBlockPos &pos, unsigne
     return true;
 }
 
-bool CheckBlockHeader(const CBlockHeader& block, CValidationState& state, bool fCheckPOW)
+bool CheckBlockHeader(const CBlockHeader& block, CValidationState& state, flagCheckPow fCheckPOW)
 {
     // Check block version
     if (block.nVersion < MIN_BLOCK_VERSION)
         return state.DoS(100, error("CheckBlockHeader(): block version not valid"),
                          REJECT_INVALID, "version-invalid");
 
     // Check Equihash solution is valid
-    if (fCheckPOW && !CheckEquihashSolution(&block, Params()))
+    if (fCheckPOW == flagCheckPow::ON && !CheckEquihashSolution(&block, Params()))
         return state.DoS(100, error("CheckBlockHeader(): Equihash solution invalid"),
                          REJECT_INVALID, "invalid-solution");
 
     // Check proof of work matches claimed amount
-    if (fCheckPOW && !CheckProofOfWork(block.GetHash(), block.nBits, Params().GetConsensus()))
+    if (fCheckPOW == flagCheckPow::ON && !CheckProofOfWork(block.GetHash(), block.nBits, Params().GetConsensus()))
         return state.DoS(50, error("CheckBlockHeader(): proof of work failed"),
                          REJECT_INVALID, "high-hash");
 
@@ -4085,7 +4087,7 @@ bool CheckBlockHeader(const CBlockHeader& block, CValidationState& state, bool f
 
 bool CheckBlock(const CBlock& block, CValidationState& state,
                 libzcash::ProofVerifier& verifier,
-                bool fCheckPOW, bool fCheckMerkleRoot)
+                flagCheckPow fCheckPOW, flagCheckMerkleRoot fCheckMerkleRoot)
 {
     // These are checks that are independent of context.
 
@@ -4095,7 +4097,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state,
         return false;
 
     // Check the merkle root.
-    if (fCheckMerkleRoot) {
+    if (fCheckMerkleRoot == flagCheckMerkleRoot::ON) {
         bool mutated;
         uint256 hashMerkleRoot2 = block.BuildMerkleTree(&mutated);
         if (block.hashMerkleRoot != hashMerkleRoot2)
@@ -4461,7 +4463,7 @@ bool ProcessNewBlock(CValidationState &state, CNode* pfrom, CBlock* pblock, bool
 }
 
 bool TestBlockValidity(CValidationState &state, const CBlock& block, CBlockIndex * const pindexPrev,
-    bool fCheckPOW, bool fCheckMerkleRoot, bool fScRelatedChecks)
+        flagCheckPow fCheckPOW, flagCheckMerkleRoot fCheckMerkleRoot, flagScRelatedChecks fScRelatedChecks)
 {
     AssertLockHeld(cs_main);
     assert(pindexPrev == chainActive.Tip());
@@ -4482,7 +4484,7 @@ bool TestBlockValidity(CValidationState &state, const CBlock& block, CBlockIndex
         return false;
 
     static const bool JUST_CHECK_TRUE = true;
-    if (!ConnectBlock(block, state, &indexDummy, viewNew, chainActive, JUST_CHECK_TRUE, fScRelatedChecks))
+    if (!ConnectBlock(block, state, &indexDummy, viewNew, chainActive, JUST_CHECK_TRUE, fScRelatedChecks == flagScRelatedChecks::ON))
         return false;
     assert(state.IsValid());
 

src/main.h

@@ -473,23 +473,27 @@ bool DisconnectBlock(CBlock& block, CValidationState& state, CBlockIndex* pindex
     bool* pfClean = NULL, std::vector<CScCertificateStatusUpdateInfo>* pCertsStateInfo = nullptr);
 
 /** Apply the effects of this block (with given index) on the UTXO set represented by coins */
+enum class flagCheckPow        {ON, OFF};
+enum class flagCheckMerkleRoot {ON, OFF};
+enum class flagScRelatedChecks {ON, OFF};
 bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pindex,
     CCoinsViewCache& coins, const CChain& chain, bool fJustCheck, bool fScRelatedChecks,
     std::vector<CScCertificateStatusUpdateInfo>* pCertsStateInfo = nullptr);
 
 /** Context-independent validity checks */
-bool CheckBlockHeader(const CBlockHeader& block, CValidationState& state, bool fCheckPOW = true);
+bool CheckBlockHeader(const CBlockHeader& block, CValidationState& state, flagCheckPow fCheckPOW = flagCheckPow::ON);
 bool CheckBlock(const CBlock& block, CValidationState& state,
                 libzcash::ProofVerifier& verifier,
-                bool fCheckPOW = true, bool fCheckMerkleRoot = true);
+                flagCheckPow fCheckPOW = flagCheckPow::ON,
+                flagCheckMerkleRoot fCheckMerkleRoot = flagCheckMerkleRoot::ON);
 
 /** Context-dependent validity checks */
 bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& state, CBlockIndex *pindexPrev);
 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, bool fCheckMerkleRoot, bool fScRelatedChecks);
+        flagCheckPow fCheckPOW, flagCheckMerkleRoot fCheckMerkleRoot, flagScRelatedChecks fScRelatedChecks);
 
 /**
  * Store block on disk.

src/miner.cpp

@@ -760,7 +760,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn,  unsigned int nBlo
         pblocktemplate->vTxSigOps[0] = GetLegacySigOpCount(pblock->vtx[0]);
 
         CValidationState state;
-        if (!TestBlockValidity(state, *pblock, pindexPrev, /*fCheckPOW*/false, /*fCheckMerkleRoot*/false, /*fScRelatedChecks*/false))
+        if (!TestBlockValidity(state, *pblock, pindexPrev, flagCheckPow::OFF, flagCheckMerkleRoot::OFF, flagScRelatedChecks::OFF))
             throw std::runtime_error("CreateNewBlock(): TestBlockValidity failed");
     }
 

src/primitives/transaction.h

@@ -10,7 +10,6 @@
 #include "random.h"
 #include "script/script.h"
 #include "serialize.h"
-#include <univalue.h>
 #include "streams.h"
 #include "uint256.h"
 #include "consensus/consensus.h"
@@ -30,6 +29,13 @@
 #include <sc/sidechaintypes.h>
 #include <script/script_error.h>
 
+class UniValue;
+class CBackwardTransferOut;
+class CValidationState;
+class CChain;
+class CMutableTransactionBase;
+struct CMutableTransaction;
+
 static const int32_t SC_CERT_VERSION = 0xFFFFFFFB; // -5
 static const int32_t SC_TX_VERSION = 0xFFFFFFFC; // -4
 static const int32_t GROTH_TX_VERSION = 0xFFFFFFFD; // -3
@@ -401,8 +407,6 @@ class CTxCeasedSidechainWithdrawalInput
     CScript scriptPubKey() const;
 };
 
-class CBackwardTransferOut;
-
 /** An output of a transaction.  It contains the public key that the next input
  * must be able to sign with to claim it.
  */
@@ -669,11 +673,6 @@ class CBwtRequestOut : public CTxCrosschainOutBase
     std::string ToString() const override;
 };
 
-// forward declarations
-class CValidationState;
-class CChain;
-class CMutableTransactionBase;
-
 // abstract interface for CTransaction and CScCertificate
 class CTransactionBase
 {
@@ -800,8 +799,6 @@ class CTransactionBase
     }
 };
 
-struct CMutableTransaction;
-
 /** The basic transaction that is broadcasted on the network and contained in
  * blocks.  A transaction can contain multiple inputs and outputs.
  */

src/rpc/mining.cpp

@@ -550,7 +550,7 @@ UniValue getblocktemplate(const UniValue& params, bool fHelp)
             if (block.hashPrevBlock != pindexPrev->GetBlockHash())
                 return "inconclusive-not-best-prevblk";
             CValidationState state;
-            TestBlockValidity(state, block, pindexPrev, /*fCheckPOW*/false, /*fCheckMerkleRoot*/true, /*fScRelatedChecks*/true);
+            TestBlockValidity(state, block, pindexPrev, flagCheckPow::OFF, flagCheckMerkleRoot::ON, flagScRelatedChecks::ON);
             return BIP22ValidationResult(state);
         }
     }

src/test/checkblock_tests.cpp

@@ -58,7 +58,7 @@ BOOST_AUTO_TEST_CASE(May15)
         // After May 15'th, big blocks are OK:
         forkingBlock.nTime = tMay15; // Invalidates PoW
         auto verifier = libzcash::ProofVerifier::Strict();
-        BOOST_CHECK(CheckBlock(forkingBlock, state, verifier, false, false));
+        BOOST_CHECK(CheckBlock(forkingBlock, state, verifier, flagCheckPow::OFF, flagCheckMerkleRoot::OFF));
     }
 
     SetMockTime(0);