Pass the block height through to CheckEquihashSolution()

This requires moving CheckEquihashSolution() to
ContextualCheckBlockHeader() for all but the genesis block, which has no
effect on consensus; it just means that an invalid Equihash solution is
rejected slightly later in the block validation process.

src/main.cpp

@@ -1740,7 +1740,7 @@ bool WriteBlockToDisk(const CBlock& block, CDiskBlockPos& pos, const CMessageHea
     return true;
 }
 
-bool ReadBlockFromDisk(CBlock& block, const CDiskBlockPos& pos, const Consensus::Params& consensusParams)
+bool ReadBlockFromDisk(CBlock& block, const CDiskBlockPos& pos, int nHeight, const Consensus::Params& consensusParams)
 {
     block.SetNull();
 
@@ -1758,7 +1758,7 @@ bool ReadBlockFromDisk(CBlock& block, const CDiskBlockPos& pos, const Consensus:
     }
 
     // Check the header
-    if (!(CheckEquihashSolution(&block, consensusParams) &&
+    if (!(CheckEquihashSolution(&block, nHeight, consensusParams) &&
           CheckProofOfWork(block.GetHash(), block.nBits, consensusParams)))
         return error("ReadBlockFromDisk: Errors in block header at %s", pos.ToString());
 
@@ -1767,7 +1767,7 @@ bool ReadBlockFromDisk(CBlock& block, const CDiskBlockPos& pos, const Consensus:
 
 bool ReadBlockFromDisk(CBlock& block, const CBlockIndex* pindex, const Consensus::Params& consensusParams)
 {
-    if (!ReadBlockFromDisk(block, pindex->GetBlockPos(), consensusParams))
+    if (!ReadBlockFromDisk(block, pindex->GetBlockPos(), pindex->nHeight, consensusParams))
         return false;
     if (block.GetHash() != pindex->GetBlockHash())
         return error("ReadBlockFromDisk(CBlock&, CBlockIndex*): GetHash() doesn't match index for %s at %s",
@@ -3855,13 +3855,19 @@ bool CheckBlockHeader(
     const CChainParams& chainparams,
     bool fCheckPOW)
 {
+    auto consensusParams = chainparams.GetConsensus();
+
     // Check block version
     if (block.nVersion < MIN_BLOCK_VERSION)
         return state.DoS(100, error("CheckBlockHeader(): block version too low"),
                          REJECT_INVALID, "version-too-low");
 
-    // Check Equihash solution is valid
-    if (fCheckPOW && !CheckEquihashSolution(&block, chainparams.GetConsensus()))
+    // Check Equihash solution is valid. The main check is in ContextualCheckBlockHeader,
+    // because we currently need to know the block height. That function skips the genesis
+    // block because it has no previous block, so we check it specifically here.
+    if (fCheckPOW &&
+        block.GetHash() == consensusParams.hashGenesisBlock &&
+        !CheckEquihashSolution(&block, 0, consensusParams))
         return state.DoS(100, error("CheckBlockHeader(): Equihash solution invalid"),
                          REJECT_INVALID, "invalid-solution");
 
@@ -3938,7 +3944,8 @@ bool CheckBlock(const CBlock& block, CValidationState& state,
 
 bool ContextualCheckBlockHeader(
     const CBlockHeader& block, CValidationState& state,
-    const CChainParams& chainParams, CBlockIndex * const pindexPrev)
+    const CChainParams& chainParams, CBlockIndex * const pindexPrev,
+    bool fCheckPOW)
 {
     const Consensus::Params& consensusParams = chainParams.GetConsensus();
     uint256 hash = block.GetHash();
@@ -3950,6 +3957,11 @@ bool ContextualCheckBlockHeader(
 
     int nHeight = pindexPrev->nHeight+1;
 
+    // Check Equihash solution is valid
+    if (fCheckPOW && !CheckEquihashSolution(&block, nHeight, consensusParams))
+        return state.DoS(100, error("CheckBlockHeader(): Equihash solution invalid"),
+                         REJECT_INVALID, "invalid-solution");
+
     // Check proof of work
     if (block.nBits != GetNextWorkRequired(pindexPrev, &block, consensusParams)) {
         return state.DoS(100, error("%s: incorrect proof of work", __func__),
@@ -4239,7 +4251,7 @@ bool TestBlockValidity(CValidationState& state, const CChainParams& chainparams,
     auto verifier = libzcash::ProofVerifier::Disabled();
 
     // NOTE: CheckBlockHeader is called by CheckBlock
-    if (!ContextualCheckBlockHeader(block, state, chainparams, pindexPrev))
+    if (!ContextualCheckBlockHeader(block, state, chainparams, pindexPrev, fCheckPOW))
         return false;
     if (!CheckBlock(block, state, chainparams, verifier, fCheckPOW, fCheckMerkleRoot))
         return false;
@@ -5031,7 +5043,7 @@ bool LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, CDiskB
                     std::pair<std::multimap<uint256, CDiskBlockPos>::iterator, std::multimap<uint256, CDiskBlockPos>::iterator> range = mapBlocksUnknownParent.equal_range(head);
                     while (range.first != range.second) {
                         std::multimap<uint256, CDiskBlockPos>::iterator it = range.first;
-                        if (ReadBlockFromDisk(block, it->second, chainparams.GetConsensus()))
+                        if (ReadBlockFromDisk(block, it->second, mapBlockIndex[head]->nHeight, chainparams.GetConsensus()))
                         {
                             LogPrintf("%s: Processing out of order child %s of %s\n", __func__, block.GetHash().ToString(),
                                     head.ToString());

src/main.h

@@ -436,7 +436,7 @@ bool GetTimestampIndex(unsigned int high, unsigned int low, bool fActiveOnly,
 
 /** Functions for disk access for blocks */
 bool WriteBlockToDisk(const CBlock& block, CDiskBlockPos& pos, const CMessageHeader::MessageStartChars& messageStart);
-bool ReadBlockFromDisk(CBlock& block, const CDiskBlockPos& pos, const Consensus::Params& consensusParams);
+bool ReadBlockFromDisk(CBlock& block, const CDiskBlockPos& pos, int nHeight, const Consensus::Params& consensusParams);
 bool ReadBlockFromDisk(CBlock& block, const CBlockIndex* pindex, const Consensus::Params& consensusParams);
 
 /** Functions for validating blocks and updating the block tree */
@@ -454,7 +454,8 @@ bool CheckBlock(const CBlock& block, CValidationState& state,
  *  By "context", we mean only the previous block headers, but not the UTXO
  *  set; UTXO-related validity checks are done in ConnectBlock(). */
 bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& state,
-                                const CChainParams& chainparams, CBlockIndex *pindexPrev);
+                                const CChainParams& chainparams, CBlockIndex *pindexPrev,
+                                bool fCheckPOW = true);
 bool ContextualCheckBlock(const CBlock& block, CValidationState& state,
                           const CChainParams& chainparams, CBlockIndex *pindexPrev);
 

src/pow.cpp

@@ -92,7 +92,7 @@ unsigned int CalculateNextWorkRequired(arith_uint256 bnAvg,
     return bnNew.GetCompact();
 }
 
-bool CheckEquihashSolution(const CBlockHeader *pblock, const Consensus::Params& params)
+bool CheckEquihashSolution(const CBlockHeader *pblock, int nHeight, const Consensus::Params& params)
 {
     unsigned int n = params.nEquihashN;
     unsigned int k = params.nEquihashK;

src/pow.h

@@ -23,7 +23,7 @@ unsigned int CalculateNextWorkRequired(arith_uint256 bnAvg,
                                        int nextHeight);
 
 /** Check whether the Equihash solution in a block header is valid */
-bool CheckEquihashSolution(const CBlockHeader *pblock, const Consensus::Params&);
+bool CheckEquihashSolution(const CBlockHeader *pblock, int nHeight, const Consensus::Params&);
 
 /** Check whether a block hash satisfies the proof-of-work requirement specified by nBits */
 bool CheckProofOfWork(uint256 hash, unsigned int nBits, const Consensus::Params&);

src/zcbenchmarks.cpp

@@ -204,7 +204,7 @@ double benchmark_verify_equihash()
     CBlockHeader genesis_header = genesis.GetBlockHeader();
     struct timeval tv_start;
     timer_start(tv_start);
-    CheckEquihashSolution(&genesis_header, params.GetConsensus());
+    assert(CheckEquihashSolution(&genesis_header, 1, params.GetConsensus()));
     return timer_stop(tv_start);
 }