From 79846ac1f33ad1c1343463ac83ba864128e0102c Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 14 Jul 2020 23:01:42 +1200 Subject: [PATCH 1/3] Use Rust Equihash validator unconditionally It correctly validates all blocks prior to Heartwood activation. --- src/pow.cpp | 31 +++++-------------------------- 1 file changed, 5 insertions(+), 26 deletions(-) diff --git a/src/pow.cpp b/src/pow.cpp index 0477a59cb..b6e72e13d 100644 --- a/src/pow.cpp +++ b/src/pow.cpp @@ -14,7 +14,6 @@ #include "uint256.h" #include -#include "sodium.h" unsigned int GetNextWorkRequired(const CBlockIndex* pindexLast, const CBlockHeader *pblock, const Consensus::Params& params) { @@ -98,37 +97,17 @@ bool CheckEquihashSolution(const CBlockHeader *pblock, int nHeight, const Consen unsigned int n = params.nEquihashN; unsigned int k = params.nEquihashK; - // Hash state - crypto_generichash_blake2b_state state; - EhInitialiseState(n, k, state); - // I = the block header minus nonce and solution. CEquihashInput I{*pblock}; // I||V CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); ss << I; - // From Heartwood activation, check with the Rust validator - if (params.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_HEARTWOOD)) { - return librustzcash_eh_isvalid( - n, k, - (unsigned char*)&ss[0], ss.size(), - pblock->nNonce.begin(), pblock->nNonce.size(), - pblock->nSolution.data(), pblock->nSolution.size()); - } - - // Before Heartwood activation, check with the C++ validator - ss << pblock->nNonce; - - // H(I||V||... - crypto_generichash_blake2b_update(&state, (unsigned char*)&ss[0], ss.size()); - - bool isValid; - EhIsValidSolution(n, k, state, pblock->nSolution, isValid); - if (!isValid) - return error("CheckEquihashSolution(): invalid solution"); - - return true; + return librustzcash_eh_isvalid( + n, k, + (unsigned char*)&ss[0], ss.size(), + pblock->nNonce.begin(), pblock->nNonce.size(), + pblock->nSolution.data(), pblock->nSolution.size()); } bool CheckProofOfWork(uint256 hash, unsigned int nBits, const Consensus::Params& params) From 96920e952d47e50bd5313b943fb7cb6f1c960866 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 14 Jul 2020 23:15:07 +1200 Subject: [PATCH 2/3] Remove C++ Equihash validator --- src/crypto/equihash.cpp | 66 +++---------------------------------- src/crypto/equihash.h | 19 +---------- src/test/equihash_tests.cpp | 20 +---------- src/test/miner_tests.cpp | 8 +++-- 4 files changed, 11 insertions(+), 102 deletions(-) diff --git a/src/crypto/equihash.cpp b/src/crypto/equihash.cpp index c72ce5e85..0f0945439 100644 --- a/src/crypto/equihash.cpp +++ b/src/crypto/equihash.cpp @@ -16,6 +16,8 @@ #include "config/bitcoin-config.h" #endif +#ifdef ENABLE_MINING + #include "compat/endian.h" #include "crypto/equihash.h" #include "util.h" @@ -324,7 +326,6 @@ std::shared_ptr TruncatedStepRow::GetTruncatedIndices(size_t le return p; } -#ifdef ENABLE_MINING template bool Equihash::BasicSolve(const eh_HashState& base_state, const std::function)> validBlock, @@ -717,100 +718,41 @@ invalidsolution: return false; } -#endif // ENABLE_MINING - -template -bool Equihash::IsValidSolution(const eh_HashState& base_state, std::vector soln) -{ - if (soln.size() != SolutionWidth) { - LogPrint("pow", "Invalid solution length: %d (expected %d)\n", - soln.size(), SolutionWidth); - return false; - } - - std::vector> X; - X.reserve(1 << K); - unsigned char tmpHash[HashOutput]; - for (eh_index i : GetIndicesFromMinimal(soln, CollisionBitLength)) { - GenerateHash(base_state, i/IndicesPerHashOutput, tmpHash, HashOutput); - X.emplace_back(tmpHash+((i % IndicesPerHashOutput) * N/8), - N/8, HashLength, CollisionBitLength, i); - } - - size_t hashLen = HashLength; - size_t lenIndices = sizeof(eh_index); - while (X.size() > 1) { - std::vector> Xc; - for (int i = 0; i < X.size(); i += 2) { - if (!HasCollision(X[i], X[i+1], CollisionByteLength)) { - LogPrint("pow", "Invalid solution: invalid collision length between StepRows\n"); - LogPrint("pow", "X[i] = %s\n", X[i].GetHex(hashLen)); - LogPrint("pow", "X[i+1] = %s\n", X[i+1].GetHex(hashLen)); - return false; - } - if (X[i+1].IndicesBefore(X[i], hashLen, lenIndices)) { - LogPrint("pow", "Invalid solution: Index tree incorrectly ordered\n"); - return false; - } - if (!DistinctIndices(X[i], X[i+1], hashLen, lenIndices)) { - LogPrint("pow", "Invalid solution: duplicate indices\n"); - return false; - } - Xc.emplace_back(X[i], X[i+1], hashLen, lenIndices, CollisionByteLength); - } - X = Xc; - hashLen -= CollisionByteLength; - lenIndices *= 2; - } - - assert(X.size() == 1); - return X[0].IsZero(hashLen); -} // Explicit instantiations for Equihash<96,3> template int Equihash<96,3>::InitialiseState(eh_HashState& base_state); -#ifdef ENABLE_MINING template bool Equihash<96,3>::BasicSolve(const eh_HashState& base_state, const std::function)> validBlock, const std::function cancelled); template bool Equihash<96,3>::OptimisedSolve(const eh_HashState& base_state, const std::function)> validBlock, const std::function cancelled); -#endif -template bool Equihash<96,3>::IsValidSolution(const eh_HashState& base_state, std::vector soln); // Explicit instantiations for Equihash<200,9> template int Equihash<200,9>::InitialiseState(eh_HashState& base_state); -#ifdef ENABLE_MINING template bool Equihash<200,9>::BasicSolve(const eh_HashState& base_state, const std::function)> validBlock, const std::function cancelled); template bool Equihash<200,9>::OptimisedSolve(const eh_HashState& base_state, const std::function)> validBlock, const std::function cancelled); -#endif -template bool Equihash<200,9>::IsValidSolution(const eh_HashState& base_state, std::vector soln); // Explicit instantiations for Equihash<96,5> template int Equihash<96,5>::InitialiseState(eh_HashState& base_state); -#ifdef ENABLE_MINING template bool Equihash<96,5>::BasicSolve(const eh_HashState& base_state, const std::function)> validBlock, const std::function cancelled); template bool Equihash<96,5>::OptimisedSolve(const eh_HashState& base_state, const std::function)> validBlock, const std::function cancelled); -#endif -template bool Equihash<96,5>::IsValidSolution(const eh_HashState& base_state, std::vector soln); // Explicit instantiations for Equihash<48,5> template int Equihash<48,5>::InitialiseState(eh_HashState& base_state); -#ifdef ENABLE_MINING template bool Equihash<48,5>::BasicSolve(const eh_HashState& base_state, const std::function)> validBlock, const std::function cancelled); template bool Equihash<48,5>::OptimisedSolve(const eh_HashState& base_state, const std::function)> validBlock, const std::function cancelled); -#endif -template bool Equihash<48,5>::IsValidSolution(const eh_HashState& base_state, std::vector soln); + +#endif // ENABLE_MINING diff --git a/src/crypto/equihash.h b/src/crypto/equihash.h index 7cd97ff60..03eb1a104 100644 --- a/src/crypto/equihash.h +++ b/src/crypto/equihash.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_EQUIHASH_H #define BITCOIN_EQUIHASH_H +#ifdef ENABLE_MINING #include "crypto/sha256.h" #include "utilstrencodings.h" @@ -183,15 +184,12 @@ public: Equihash() { } int InitialiseState(eh_HashState& base_state); -#ifdef ENABLE_MINING bool BasicSolve(const eh_HashState& base_state, const std::function)> validBlock, const std::function cancelled); bool OptimisedSolve(const eh_HashState& base_state, const std::function)> validBlock, const std::function cancelled); -#endif - bool IsValidSolution(const eh_HashState& base_state, std::vector soln); }; #include "equihash.tcc" @@ -214,7 +212,6 @@ static Equihash<48,5> Eh48_5; throw std::invalid_argument("Unsupported Equihash parameters"); \ } -#ifdef ENABLE_MINING inline bool EhBasicSolve(unsigned int n, unsigned int k, const eh_HashState& base_state, const std::function)> validBlock, const std::function cancelled) @@ -264,18 +261,4 @@ inline bool EhOptimisedSolveUncancellable(unsigned int n, unsigned int k, const } #endif // ENABLE_MINING -#define EhIsValidSolution(n, k, base_state, soln, ret) \ - if (n == 96 && k == 3) { \ - ret = Eh96_3.IsValidSolution(base_state, soln); \ - } else if (n == 200 && k == 9) { \ - ret = Eh200_9.IsValidSolution(base_state, soln); \ - } else if (n == 96 && k == 5) { \ - ret = Eh96_5.IsValidSolution(base_state, soln); \ - } else if (n == 48 && k == 5) { \ - ret = Eh48_5.IsValidSolution(base_state, soln); \ - } else { \ - ret = false; \ - throw std::invalid_argument("Unsupported Equihash parameters"); \ - } - #endif // BITCOIN_EQUIHASH_H diff --git a/src/test/equihash_tests.cpp b/src/test/equihash_tests.cpp index 8e202fc68..5eb41a640 100644 --- a/src/test/equihash_tests.cpp +++ b/src/test/equihash_tests.cpp @@ -91,22 +91,13 @@ void TestEquihashValidator(unsigned int n, unsigned int k, const std::string &I, size_t cBitLen { n/(k+1) }; auto minimal = GetMinimalFromIndices(soln, cBitLen); - // First test the C++ validator - crypto_generichash_blake2b_state state; - EhInitialiseState(n, k, state); uint256 V = ArithToUint256(nonce); - crypto_generichash_blake2b_update(&state, (unsigned char*)&I[0], I.size()); - crypto_generichash_blake2b_update(&state, V.begin(), V.size()); BOOST_TEST_MESSAGE("Running validator: n = " << n << ", k = " << k << ", I = " << I << ", V = " << V.GetHex() << ", expected = " << expected << ", soln ="); std::stringstream strm; PrintSolution(strm, soln); BOOST_TEST_MESSAGE(strm.str()); - bool isValid; - EhIsValidSolution(n, k, state, minimal, isValid); - BOOST_CHECK(isValid == expected); - // The Rust validator should have the exact same result - isValid = librustzcash_eh_isvalid( + bool isValid = librustzcash_eh_isvalid( n, k, (unsigned char*)&I[0], I.size(), V.begin(), V.size(), @@ -216,12 +207,8 @@ BOOST_AUTO_TEST_CASE(validator_allbitsmatter) { // Initialize the state according to one of the test vectors above. unsigned int n = 96; unsigned int k = 5; - crypto_generichash_blake2b_state state; - EhInitialiseState(n, k, state); uint256 V = ArithToUint256(1); std::string I = "Equihash is an asymmetric PoW based on the Generalised Birthday problem."; - crypto_generichash_blake2b_update(&state, (unsigned char*)&I[0], I.size()); - crypto_generichash_blake2b_update(&state, V.begin(), V.size()); // Encode the correct solution. std::vector soln = {2261, 15185, 36112, 104243, 23779, 118390, 118332, 130041, 32642, 69878, 76925, 80080, 45858, 116805, 92842, 111026, 15972, 115059, 85191, 90330, 68190, 122819, 81830, 91132, 23460, 49807, 52426, 80391, 69567, 114474, 104973, 122568}; @@ -229,9 +216,6 @@ BOOST_AUTO_TEST_CASE(validator_allbitsmatter) { std::vector sol_char = GetMinimalFromIndices(soln, cBitLen); // Prove that the solution is valid. - bool isValid; - EhIsValidSolution(n, k, state, sol_char, isValid); - BOOST_CHECK(isValid == true); BOOST_CHECK(librustzcash_eh_isvalid( n, k, (unsigned char*)&I[0], I.size(), @@ -242,8 +226,6 @@ BOOST_AUTO_TEST_CASE(validator_allbitsmatter) { for (size_t i = 0; i < sol_char.size() * 8; i++) { std::vector mutated = sol_char; mutated.at(i/8) ^= (1 << (i % 8)); - EhIsValidSolution(n, k, state, mutated, isValid); - BOOST_CHECK(isValid == false); BOOST_CHECK(!librustzcash_eh_isvalid( n, k, (unsigned char*)&I[0], I.size(), diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 7dc9773e2..99fa0c989 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -247,10 +247,12 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) solns.insert(sol_char); } - bool ret; for (auto soln : solns) { - EhIsValidSolution(n, k, curr_state, soln, ret); - if (!ret) continue; + if (!librustzcash_eh_isvalid( + n, k, + (unsigned char*)&ss[0], ss.size(), + pblock->nNonce.begin(), pblock->nNonce.size(), + soln.data(), soln.size())) continue; pblock->nSolution = soln; CValidationState state; From 3611f6881176d5fe8f3f0fd6261931c9aa3cefc2 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 15 Jul 2020 16:15:09 +1200 Subject: [PATCH 3/3] Revert "Pass the block height through to CheckEquihashSolution()" This reverts commit 49f9584613e2a5506508281b16427fa08c4ad11d. Now that we are depending unconditionally on the Rust Equihash validator, CheckEquihashSolution() can revert to being a non-contextual check. This also fixes a segfault that would occur during reindexing if the consensus rules were altered such that a previously-valid block would become invalid, and the node's block files contained blocks in a specific order. It was encountered while testing the Canopy NU on testnet (due to a bug in the implementation of ZIP 212 that was separately fixed in zcash/zcash#4604). --- src/main.cpp | 28 ++++++++-------------------- src/main.h | 5 ++--- src/pow.cpp | 2 +- src/pow.h | 2 +- src/zcbenchmarks.cpp | 2 +- 5 files changed, 13 insertions(+), 26 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index d593018f0..cceaaf9aa 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1824,7 +1824,7 @@ bool WriteBlockToDisk(const CBlock& block, CDiskBlockPos& pos, const CMessageHea return true; } -bool ReadBlockFromDisk(CBlock& block, const CDiskBlockPos& pos, int nHeight, const Consensus::Params& consensusParams) +bool ReadBlockFromDisk(CBlock& block, const CDiskBlockPos& pos, const Consensus::Params& consensusParams) { block.SetNull(); @@ -1842,7 +1842,7 @@ bool ReadBlockFromDisk(CBlock& block, const CDiskBlockPos& pos, int nHeight, con } // Check the header - if (!(CheckEquihashSolution(&block, nHeight, consensusParams) && + if (!(CheckEquihashSolution(&block, consensusParams) && CheckProofOfWork(block.GetHash(), block.nBits, consensusParams))) return error("ReadBlockFromDisk: Errors in block header at %s", pos.ToString()); @@ -1851,7 +1851,7 @@ bool ReadBlockFromDisk(CBlock& block, const CDiskBlockPos& pos, int nHeight, con bool ReadBlockFromDisk(CBlock& block, const CBlockIndex* pindex, const Consensus::Params& consensusParams) { - if (!ReadBlockFromDisk(block, pindex->GetBlockPos(), pindex->nHeight, consensusParams)) + if (!ReadBlockFromDisk(block, pindex->GetBlockPos(), consensusParams)) return false; if (block.GetHash() != pindex->GetBlockHash()) return error("ReadBlockFromDisk(CBlock&, CBlockIndex*): GetHash() doesn't match index for %s at %s", @@ -3979,19 +3979,13 @@ 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. 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)) + // Check Equihash solution is valid + if (fCheckPOW && !CheckEquihashSolution(&block, chainparams.GetConsensus())) return state.DoS(100, error("CheckBlockHeader(): Equihash solution invalid"), REJECT_INVALID, "invalid-solution"); @@ -4068,8 +4062,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool ContextualCheckBlockHeader( const CBlockHeader& block, CValidationState& state, - const CChainParams& chainParams, CBlockIndex * const pindexPrev, - bool fCheckPOW) + const CChainParams& chainParams, CBlockIndex * const pindexPrev) { const Consensus::Params& consensusParams = chainParams.GetConsensus(); uint256 hash = block.GetHash(); @@ -4081,11 +4074,6 @@ 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__), @@ -4377,7 +4365,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, fCheckPOW)) + if (!ContextualCheckBlockHeader(block, state, chainparams, pindexPrev)) return false; if (!CheckBlock(block, state, chainparams, verifier, fCheckPOW, fCheckMerkleRoot)) return false; @@ -5173,7 +5161,7 @@ bool LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, CDiskB std::pair::iterator, std::multimap::iterator> range = mapBlocksUnknownParent.equal_range(head); while (range.first != range.second) { std::multimap::iterator it = range.first; - if (ReadBlockFromDisk(block, it->second, mapBlockIndex[head]->nHeight, chainparams.GetConsensus())) + if (ReadBlockFromDisk(block, it->second, chainparams.GetConsensus())) { LogPrintf("%s: Processing out of order child %s of %s\n", __func__, block.GetHash().ToString(), head.ToString()); diff --git a/src/main.h b/src/main.h index ef515f1eb..c1c0f64d7 100644 --- a/src/main.h +++ b/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, int nHeight, const Consensus::Params& consensusParams); +bool ReadBlockFromDisk(CBlock& block, const CDiskBlockPos& pos, 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,8 +454,7 @@ 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, - bool fCheckPOW = true); + const CChainParams& chainparams, CBlockIndex *pindexPrev); bool ContextualCheckBlock(const CBlock& block, CValidationState& state, const CChainParams& chainparams, CBlockIndex *pindexPrev); diff --git a/src/pow.cpp b/src/pow.cpp index b6e72e13d..3c875a6eb 100644 --- a/src/pow.cpp +++ b/src/pow.cpp @@ -92,7 +92,7 @@ unsigned int CalculateNextWorkRequired(arith_uint256 bnAvg, return bnNew.GetCompact(); } -bool CheckEquihashSolution(const CBlockHeader *pblock, int nHeight, const Consensus::Params& params) +bool CheckEquihashSolution(const CBlockHeader *pblock, const Consensus::Params& params) { unsigned int n = params.nEquihashN; unsigned int k = params.nEquihashK; diff --git a/src/pow.h b/src/pow.h index 4a6f86b48..eb05c6e34 100644 --- a/src/pow.h +++ b/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, int nHeight, const Consensus::Params&); +bool CheckEquihashSolution(const CBlockHeader *pblock, 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&); diff --git a/src/zcbenchmarks.cpp b/src/zcbenchmarks.cpp index d66404814..23f31e7dd 100644 --- a/src/zcbenchmarks.cpp +++ b/src/zcbenchmarks.cpp @@ -204,7 +204,7 @@ double benchmark_verify_equihash() CBlockHeader genesis_header = genesis.GetBlockHeader(); struct timeval tv_start; timer_start(tv_start); - assert(CheckEquihashSolution(&genesis_header, 1, params.GetConsensus())); + CheckEquihashSolution(&genesis_header, params.GetConsensus()); return timer_stop(tv_start); }