Source the Excessive block size from BlockValidationOptions

Summary: After all, it's a block validation option.

Test Plan:
  make check
  ./test/functional/test_runner.py --extended

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D4048

src/bench/checkblock.cpp

@@ -41,6 +41,7 @@ static void DeserializeAndCheckBlockTest(benchmark::State &state) {
     stream.write(&a, 1); // Prevent compaction
 
     const Config &config = GetConfig();
+    BlockValidationOptions options(config);
     while (state.KeepRunning()) {
         // Note that CBlock caches its checked state, so we need to recreate it
         // here.
@@ -49,7 +50,8 @@ static void DeserializeAndCheckBlockTest(benchmark::State &state) {
         assert(stream.Rewind(sizeof(block_bench::block413567)));
 
         CValidationState validationState;
-        assert(CheckBlock(config, block, validationState));
+        bool ret = CheckBlock(config, block, validationState, options);
+        assert(ret);
     }
 }
 

src/blockencodings.cpp

@@ -241,7 +241,7 @@ ReadStatus PartiallyDownloadedBlock::FillBlock(
     }
 
     CValidationState state;
-    if (!CheckBlock(*config, block, state)) {
+    if (!CheckBlock(*config, block, state, BlockValidationOptions(*config))) {
         // TODO: We really want to just check merkle tree manually here, but
         // that is expensive, and CheckBlock caches a block's "checked-status"
         // (in the CBlock?). CBlock should be able to check its own merkle root

src/miner.cpp

@@ -224,9 +224,10 @@ BlockAssembler::CreateNewBlock(const CScript &scriptPubKeyIn) {
     }
 
     CValidationState state;
-    BlockValidationOptions validationOptions(false, false);
     if (!TestBlockValidity(*config, state, *pblock, pindexPrev,
-                           validationOptions)) {
+                           BlockValidationOptions(*config)
+                               .withCheckPoW(false)
+                               .withCheckMerkleRoot(false))) {
         throw std::runtime_error(strprintf("%s: TestBlockValidity failed: %s",
                                            __func__,
                                            FormatStateMessage(state)));

src/rpc/mining.cpp

@@ -490,10 +490,10 @@ static UniValue getblocktemplate(const Config &config,
                 return "inconclusive-not-best-prevblk";
             }
             CValidationState state;
-            BlockValidationOptions validationOptions =
-                BlockValidationOptions(false, true);
             TestBlockValidity(config, state, block, pindexPrev,
-                              validationOptions);
+                              BlockValidationOptions(config)
+                                  .withCheckPoW(false)
+                                  .withCheckMerkleRoot(true));
             return BIP22ValidationResult(config, state);
         }
     }

src/test/blockcheck_tests.cpp

@@ -17,9 +17,10 @@ BOOST_FIXTURE_TEST_SUITE(blockcheck_tests, BasicTestingSetup)
 static void RunCheckOnBlockImpl(const GlobalConfig &config, const CBlock &block,
                                 CValidationState &state, bool expected) {
     block.fChecked = false;
-    BlockValidationOptions validationOptions =
-        BlockValidationOptions(false, false);
-    bool fValid = CheckBlock(config, block, state, validationOptions);
+    bool fValid = CheckBlock(
+        config, block, state,
+        BlockValidationOptions(config).withCheckPoW(false).withCheckMerkleRoot(
+            false));
 
     BOOST_CHECK_EQUAL(fValid, expected);
     BOOST_CHECK_EQUAL(fValid, state.IsValid());

src/validation.cpp

@@ -278,6 +278,10 @@ std::set<const CBlockIndex *> setDirtyBlockIndex;
 std::set<int> setDirtyFileInfo;
 } // namespace
 
+BlockValidationOptions::BlockValidationOptions(const Config &config)
+    : checkPoW(true), checkMerkleRoot(true),
+      excessiveBlockSize(config.GetMaxBlockSize()) {}
+
 CBlockIndex *FindForkInGlobalIndex(const CChain &chain,
                                    const CBlockLocator &locator) {
     AssertLockHeld(cs_main);
@@ -1675,9 +1679,10 @@ bool CChainState::ConnectBlock(const Config &config, const CBlock &block,
     // 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).
-    BlockValidationOptions validationOptions =
-        BlockValidationOptions(!fJustCheck, !fJustCheck);
-    if (!CheckBlock(config, block, state, validationOptions)) {
+    if (!CheckBlock(config, block, state,
+                    BlockValidationOptions(config)
+                        .withCheckPoW(!fJustCheck)
+                        .withCheckMerkleRoot(!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
@@ -3438,9 +3443,9 @@ static bool FindUndoPos(CValidationState &state, int nFile, FlatFilePos &pos,
  * Do not call this for any check that depends on the context.
  * For context-dependent calls, see ContextualCheckBlockHeader.
  */
-static bool CheckBlockHeader(
-    const Config &config, const CBlockHeader &block, CValidationState &state,
-    BlockValidationOptions validationOptions = BlockValidationOptions()) {
+static bool CheckBlockHeader(const Config &config, const CBlockHeader &block,
+                             CValidationState &state,
+                             BlockValidationOptions validationOptions) {
     // Check proof of work matches claimed amount
     if (validationOptions.shouldValidatePoW() &&
         !CheckProofOfWork(block.GetHash(), block.nBits,
@@ -3495,7 +3500,7 @@ bool CheckBlock(const Config &config, const CBlock &block,
     }
 
     // Size limits.
-    auto nMaxBlockSize = config.GetMaxBlockSize();
+    auto nMaxBlockSize = validationOptions.getExcessiveBlockSize();
 
     // Bail early if there is no way this block is of reasonable size.
     if ((block.vtx.size() * MIN_TRANSACTION_SIZE) > nMaxBlockSize) {
@@ -3791,7 +3796,8 @@ bool CChainState::AcceptBlockHeader(const Config &config,
             return true;
         }
 
-        if (!CheckBlockHeader(config, block, state)) {
+        if (!CheckBlockHeader(config, block, state,
+                              BlockValidationOptions(config))) {
             return error("%s: Consensus::CheckBlockHeader: %s, %s", __func__,
                          hash.ToString(), FormatStateMessage(state));
         }
@@ -4013,7 +4019,7 @@ bool CChainState::AcceptBlock(const Config &config,
         *fNewBlock = true;
     }
 
-    if (!CheckBlock(config, block, state) ||
+    if (!CheckBlock(config, block, state, BlockValidationOptions(config)) ||
         !ContextualCheckBlock(config, block, state, pindex->pprev)) {
         if (state.IsInvalid() && !state.CorruptionPossible()) {
             pindex->nStatus = pindex->nStatus.withFailed();
@@ -4091,7 +4097,8 @@ bool ProcessNewBlock(const Config &config,
 
         // Ensure that CheckBlock() passes before calling AcceptBlock, as
         // belt-and-suspenders.
-        bool ret = CheckBlock(config, *pblock, state);
+        bool ret =
+            CheckBlock(config, *pblock, state, BlockValidationOptions(config));
         if (ret) {
             // Store to disk
             ret = g_chainstate.AcceptBlock(
@@ -4642,7 +4649,8 @@ bool CVerifyDB::VerifyDB(const Config &config, CCoinsView *coinsview,
         }
 
         // check level 1: verify block validity
-        if (nCheckLevel >= 1 && !CheckBlock(config, block, state)) {
+        if (nCheckLevel >= 1 &&
+            !CheckBlock(config, block, state, BlockValidationOptions(config))) {
             return error("%s: *** found bad block at %d, hash=%s (%s)\n",
                          __func__, pindex->nHeight,
                          pindex->GetBlockHash().ToString(),

src/validation.h

@@ -294,14 +294,28 @@ class BlockValidationOptions {
     bool checkPoW : 1;
     bool checkMerkleRoot : 1;
 
+    uint64_t excessiveBlockSize;
+
 public:
     // Do full validation by default
-    BlockValidationOptions() : checkPoW(true), checkMerkleRoot(true) {}
-    BlockValidationOptions(bool checkPoWIn, bool checkMerkleRootIn)
-        : checkPoW(checkPoWIn), checkMerkleRoot(checkMerkleRootIn) {}
+    BlockValidationOptions(const Config &config);
+
+    BlockValidationOptions withCheckPoW(bool _checkPoW = true) const {
+        BlockValidationOptions ret = *this;
+        ret.checkPoW = _checkPoW;
+        return ret;
+    }
+
+    BlockValidationOptions
+    withCheckMerkleRoot(bool _checkMerkleRoot = true) const {
+        BlockValidationOptions ret = *this;
+        ret.checkMerkleRoot = _checkMerkleRoot;
+        return ret;
+    }
 
     bool shouldValidatePoW() const { return checkPoW; }
     bool shouldValidateMerkleRoot() const { return checkMerkleRoot; }
+    uint64_t getExcessiveBlockSize() const { return excessiveBlockSize; }
 };
 
 /**
@@ -573,9 +587,9 @@ bool ReadBlockFromDisk(CBlock &block, const CBlockIndex *pindex,
  * Returns true if the provided block is valid (has valid header,
  * transactions are valid, block is a valid size, etc.)
  */
-bool CheckBlock(
-    const Config &Config, const CBlock &block, CValidationState &state,
-    BlockValidationOptions validationOptions = BlockValidationOptions());
+bool CheckBlock(const Config &Config, const CBlock &block,
+                CValidationState &state,
+                BlockValidationOptions validationOptions);
 
 /**
  * This is a variant of ContextualCheckTransaction which computes the contextual
@@ -592,10 +606,9 @@ bool ContextualCheckTransactionForCurrentBlock(const Consensus::Params &params,
  * 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(
-    const Config &config, CValidationState &state, const CBlock &block,
-    CBlockIndex *pindexPrev,
-    BlockValidationOptions validationOptions = BlockValidationOptions());
+bool TestBlockValidity(const Config &config, CValidationState &state,
+                       const CBlock &block, CBlockIndex *pindexPrev,
+                       BlockValidationOptions validationOptions);
 
 /**
  * When there are blocks in the active chain with missing data, rewind the