diff --git a/src/gtest/test_checkblock.cpp b/src/gtest/test_checkblock.cpp index 9f41d8088..2937c4553 100644 --- a/src/gtest/test_checkblock.cpp +++ b/src/gtest/test_checkblock.cpp @@ -7,6 +7,8 @@ #include "utiltest.h" #include "zcash/Proof.hpp" +#include + class MockCValidationState : public CValidationState { public: MOCK_METHOD5(DoS, bool(int level, bool ret, @@ -26,13 +28,14 @@ public: TEST(CheckBlock, VersionTooLow) { auto verifier = ProofVerifier::Strict(); + auto orchardAuth = orchard::AuthValidator::Batch(); CBlock block; block.nVersion = 1; MockCValidationState state; EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "version-too-low", false)).Times(1); - EXPECT_FALSE(CheckBlock(block, state, Params(), verifier, false, false, true)); + EXPECT_FALSE(CheckBlock(block, state, Params(), verifier, orchardAuth, false, false, true)); } @@ -72,9 +75,10 @@ TEST(CheckBlock, BlockSproutRejectsBadVersion) { CBlockIndex indexPrev {Params().GenesisBlock()}; auto verifier = ProofVerifier::Strict(); + auto orchardAuth = orchard::AuthValidator::Batch(); EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-version-too-low", false)).Times(1); - EXPECT_FALSE(CheckBlock(block, state, Params(), verifier, false, false, true)); + EXPECT_FALSE(CheckBlock(block, state, Params(), verifier, orchardAuth, false, false, true)); } diff --git a/src/main.cpp b/src/main.cpp index faca7e1d3..3f4c26dad 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1188,7 +1188,7 @@ bool ContextualCheckTransaction( bool CheckTransaction(const CTransaction& tx, CValidationState &state, - ProofVerifier& verifier) + ProofVerifier& verifier, orchard::AuthValidator& orchardAuth) { // Don't count coinbase transactions because mining skews the count if (!tx.IsCoinBase()) { @@ -1209,6 +1209,9 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, // Sapling zk-SNARK proofs are checked in librustzcash_sapling_check_{spend,output}, // called from ContextualCheckTransaction. + // Queue Orchard signatures to be batch-validated. + tx.GetOrchardBundle().QueueSignatureValidation(orchardAuth, tx.GetHash()); + return true; } } @@ -1504,8 +1507,12 @@ bool AcceptToMemoryPool( return false; } + // This will be a single-transaction batch, which is still more efficient as every + // Orchard bundle contains at least two signatures. + auto orchardAuth = orchard::AuthValidator::Batch(); + auto verifier = ProofVerifier::Strict(); - if (!CheckTransaction(tx, state, verifier)) + if (!CheckTransaction(tx, state, verifier, orchardAuth)) return error("AcceptToMemoryPool: CheckTransaction failed"); // Check transaction contextually against the set of consensus rules which apply in the next block to be mined. @@ -1646,9 +1653,9 @@ bool AcceptToMemoryPool( } } - // We don't yet know if the transaction commits to consensusBranchId, - // but if the entry gets added to the mempool, then it has passed - // ContextualCheckInputs and therefore this is correct. + // For v1-v4 transactions, we don't yet know if the transaction commits + // to consensusBranchId, but if the entry gets added to the mempool, then + // it has passed ContextualCheckInputs and therefore this is correct. CTxMemPoolEntry entry(tx, nFees, GetTime(), dPriority, chainActive.Height(), mempool.HasNoInputsOf(tx), fSpendsCoinbase, consensusBranchId); unsigned int nSize = entry.GetTxSize(); @@ -1701,6 +1708,13 @@ bool AcceptToMemoryPool( return state.Error("AcceptToMemoryPool: " + errmsg); } + // Check Orchard bundle authorizations. + // This is done near the end to help prevent CPU exhaustion + // denial-of-service attacks. + if (!orchardAuth.Validate()) { + return state.DoS(100, false, REJECT_INVALID, "bad-orchard-bundle-authorization"); + } + // Check against previous transactions // This is done last to help prevent CPU exhaustion denial-of-service attacks. PrecomputedTransactionData txdata(tx); @@ -2732,13 +2746,20 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin // proof verification is expensive, disable if possible auto verifier = fExpensiveChecks ? ProofVerifier::Strict() : ProofVerifier::Disabled(); + // Disable Orchard batch signature validation if possible. + auto orchardAuth = fExpensiveChecks ? + orchard::AuthValidator::Batch() : orchard::AuthValidator::Disabled(); + // If in initial block download, and this block is an ancestor of a checkpoint, // and -ibdskiptxverification is set, disable all transaction checks. bool fCheckTransactions = ShouldCheckTransactions(chainparams, pindex); // Check it again to verify JoinSplit proofs, and in case a previous version let a bad block in - if (!CheckBlock(block, state, chainparams, verifier, !fJustCheck, !fJustCheck, fCheckTransactions)) + if (!CheckBlock(block, state, chainparams, verifier, orchardAuth, + !fJustCheck, !fJustCheck, fCheckTransactions)) + { return false; + } // verify that the view's current state corresponds to the previous block uint256 hashPrevBlock = pindex->pprev == NULL ? uint256() : pindex->pprev->GetBlockHash(); @@ -3051,6 +3072,13 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin block.vtx[0].GetValueOut(), blockReward), REJECT_INVALID, "bad-cb-amount"); + // Ensure Orchard signatures are valid (if we are checking them) + if (!orchardAuth.Validate()) { + return state.DoS(100, + error("ConnectBlock(): an Orchard bundle within the block is invalid"), + REJECT_INVALID, "bad-orchard-bundle-authorization"); + } + if (!control.Wait()) return state.DoS(100, false); int64_t nTime2 = GetTimeMicros(); nTimeVerify += nTime2 - nTimeStart; @@ -4142,6 +4170,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const CChainParams& chainparams, ProofVerifier& verifier, + orchard::AuthValidator& orchardAuth, bool fCheckPOW, bool fCheckMerkleRoot, bool fCheckTransactions) @@ -4192,7 +4221,7 @@ bool CheckBlock(const CBlock& block, // Check transactions for (const CTransaction& tx : block.vtx) - if (!CheckTransaction(tx, state, verifier)) + if (!CheckTransaction(tx, state, verifier, orchardAuth)) return error("CheckBlock(): CheckTransaction failed"); unsigned int nSigOps = 0; @@ -4394,9 +4423,9 @@ static bool AcceptBlockHeader(const CBlockHeader& block, CValidationState& state * Store block on disk. * If dbp is non-NULL, the file is known to already reside on disk. * - * JoinSplit proofs are not verified here; the only caller of AcceptBlock - * (ProcessNewBlock) later invokes ActivateBestChain, which ultimately calls - * ConnectBlock in a manner that can verify the proofs + * JoinSplit proofs and Orchard authorizations are not verified here; the only + * caller of AcceptBlock (ProcessNewBlock) later invokes ActivateBestChain, + * which ultimately calls ConnectBlock in a manner that can verify the proofs. */ static bool AcceptBlock(const CBlock& block, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex, bool fRequested, CDiskBlockPos* dbp) { @@ -4428,10 +4457,12 @@ static bool AcceptBlock(const CBlock& block, CValidationState& state, const CCha if (fTooFarAhead) return true; // Block height is too high } - // See method docstring for why this is always disabled. + // See method docstring for why these are always disabled. auto verifier = ProofVerifier::Disabled(); + auto orchardAuth = orchard::AuthValidator::Disabled(); + bool fCheckTransactions = ShouldCheckTransactions(chainparams, pindex); - if ((!CheckBlock(block, state, chainparams, verifier, true, true, fCheckTransactions)) || + if ((!CheckBlock(block, state, chainparams, verifier, orchardAuth, true, true, fCheckTransactions)) || !ContextualCheckBlock(block, state, chainparams, pindex->pprev, fCheckTransactions)) { if (state.IsInvalid() && !state.CorruptionPossible()) { pindex->nStatus |= BLOCK_FAILED_VALID; @@ -4517,14 +4548,16 @@ bool TestBlockValidity(CValidationState& state, const CChainParams& chainparams, CBlockIndex indexDummy(block); indexDummy.pprev = pindexPrev; indexDummy.nHeight = pindexPrev->nHeight + 1; - // JoinSplit proofs are verified in ConnectBlock + + // JoinSplit proofs and Orchard authorizations are verified in ConnectBlock auto verifier = ProofVerifier::Disabled(); + auto orchardAuth = orchard::AuthValidator::Disabled(); // NOTE: CheckBlockHeader is called by CheckBlock if (!ContextualCheckBlockHeader(block, state, chainparams, pindexPrev)) return false; // The following may be duplicative of the `CheckBlock` call within `ConnectBlock` - if (!CheckBlock(block, state, chainparams, verifier, false, fCheckMerkleRoot, true)) + if (!CheckBlock(block, state, chainparams, verifier, orchardAuth, false, fCheckMerkleRoot, true)) return false; if (!ContextualCheckBlock(block, state, chainparams, pindexPrev, true)) return false; @@ -4919,6 +4952,9 @@ bool CVerifyDB::VerifyDB(const CChainParams& chainparams, CCoinsView *coinsview, // Flags used to permit skipping checks for efficiency auto verifier = ProofVerifier::Disabled(); // No need to verify JoinSplits twice bool fCheckTransactions = true; + // We may as well check Orchard authorizations if we are checking + // transactions, since we can batch-validate them. + auto orchardAuth = orchard::AuthValidator::Batch(); for (CBlockIndex* pindex = chainActive.Tip(); pindex && pindex->pprev; pindex = pindex->pprev) { @@ -4934,7 +4970,7 @@ bool CVerifyDB::VerifyDB(const CChainParams& chainparams, CCoinsView *coinsview, // check level 1: verify block validity fCheckTransactions = ShouldCheckTransactions(chainparams, pindex); - if (nCheckLevel >= 1 && !CheckBlock(block, state, chainparams, verifier, true, true, fCheckTransactions)) + if (nCheckLevel >= 1 && !CheckBlock(block, state, chainparams, verifier, orchardAuth, true, true, fCheckTransactions)) return error("VerifyDB(): *** found bad block at %d, hash=%s\n", pindex->nHeight, pindex->GetBlockHash().ToString()); // check level 2: verify undo validity diff --git a/src/main.h b/src/main.h index 5b79143c0..ff0f0ddd2 100644 --- a/src/main.h +++ b/src/main.h @@ -42,6 +42,8 @@ #include #include +#include + #include class CBlockIndex; @@ -358,7 +360,8 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight); /** Transaction validation functions */ /** Context-independent validity checks */ -bool CheckTransaction(const CTransaction& tx, CValidationState& state, ProofVerifier& verifier); +bool CheckTransaction(const CTransaction& tx, CValidationState& state, + ProofVerifier& verifier, orchard::AuthValidator& orchardAuth); bool CheckTransactionWithoutProofVerification(const CTransaction& tx, CValidationState &state); namespace Consensus { @@ -464,6 +467,7 @@ bool CheckBlockHeader(const CBlockHeader& block, CValidationState& state, bool CheckBlock(const CBlock& block, CValidationState& state, const CChainParams& chainparams, ProofVerifier& verifier, + orchard::AuthValidator& orchardAuth, bool fCheckPOW, bool fCheckMerkleRoot, bool fCheckTransactions); diff --git a/src/test/checkblock_tests.cpp b/src/test/checkblock_tests.cpp index ae4c53ce2..641b54864 100644 --- a/src/test/checkblock_tests.cpp +++ b/src/test/checkblock_tests.cpp @@ -11,6 +11,8 @@ #include "utiltime.h" #include "zcash/Proof.hpp" +#include + #include #include @@ -57,7 +59,8 @@ BOOST_AUTO_TEST_CASE(May15) // After May 15'th, big blocks are OK: forkingBlock.nTime = tMay15; // Invalidates PoW auto verifier = ProofVerifier::Strict(); - BOOST_CHECK(CheckBlock(forkingBlock, state, Params(), verifier, false, false, true)); + auto orchardAuth = orchard::AuthValidator::Disabled(); // Block is before NU5 + BOOST_CHECK(CheckBlock(forkingBlock, state, Params(), verifier, orchardAuth, false, false, true)); } SetMockTime(0); diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 8451fd31c..787008b8d 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -34,6 +34,7 @@ #include #include +#include #include @@ -68,6 +69,7 @@ BOOST_AUTO_TEST_CASE(tx_valid) std::string comment(""); auto verifier = ProofVerifier::Strict(); + auto orchardAuth = orchard::AuthValidator::Batch(); ScriptError err; for (size_t idx = 0; idx < tests.size(); idx++) { UniValue test = tests[idx]; @@ -111,7 +113,7 @@ BOOST_AUTO_TEST_CASE(tx_valid) stream >> tx; CValidationState state; - BOOST_CHECK_MESSAGE(CheckTransaction(tx, state, verifier), strTest + comment); + BOOST_CHECK_MESSAGE(CheckTransaction(tx, state, verifier, orchardAuth), strTest + comment); BOOST_CHECK_MESSAGE(state.IsValid(), comment); PrecomputedTransactionData txdata(tx); @@ -156,6 +158,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid) std::string comment(""); auto verifier = ProofVerifier::Strict(); + auto orchardAuth = orchard::AuthValidator::Batch(); ScriptError err; for (size_t idx = 0; idx < tests.size(); idx++) { UniValue test = tests[idx]; @@ -204,7 +207,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid) } CValidationState state; - fValid = CheckTransaction(tx, state, verifier) && state.IsValid(); + fValid = CheckTransaction(tx, state, verifier, orchardAuth) && state.IsValid(); PrecomputedTransactionData txdata(tx); for (unsigned int i = 0; i < tx.vin.size() && fValid; i++) @@ -243,11 +246,12 @@ BOOST_AUTO_TEST_CASE(basic_transaction_tests) stream >> tx; CValidationState state; auto verifier = ProofVerifier::Strict(); - BOOST_CHECK_MESSAGE(CheckTransaction(tx, state, verifier) && state.IsValid(), "Simple deserialized transaction should be valid."); + auto orchardAuth = orchard::AuthValidator::Disabled(); // No Orchard component + BOOST_CHECK_MESSAGE(CheckTransaction(tx, state, verifier, orchardAuth) && state.IsValid(), "Simple deserialized transaction should be valid."); // Check that duplicate txins fail tx.vin.push_back(tx.vin[0]); - BOOST_CHECK_MESSAGE(!CheckTransaction(tx, state, verifier) || !state.IsValid(), "Transaction with duplicate txins should be invalid."); + BOOST_CHECK_MESSAGE(!CheckTransaction(tx, state, verifier, orchardAuth) || !state.IsValid(), "Transaction with duplicate txins should be invalid."); } // @@ -424,6 +428,7 @@ void test_simple_sapling_invalidity(uint32_t consensusBranchId, CMutableTransact void test_simple_joinsplit_invalidity(uint32_t consensusBranchId, CMutableTransaction tx) { + auto orchardAuth = orchard::AuthValidator::Disabled(); // No Orchard components auto verifier = ProofVerifier::Strict(); { // Ensure that empty vin/vout remain invalid without @@ -477,26 +482,26 @@ void test_simple_joinsplit_invalidity(uint32_t consensusBranchId, CMutableTransa jsdesc->vpub_old = -1; BOOST_CHECK_THROW((CTransaction(newTx)), std::ios_base::failure); - BOOST_CHECK(!CheckTransaction(UNSAFE_CTransaction(newTx), state, verifier)); + BOOST_CHECK(!CheckTransaction(UNSAFE_CTransaction(newTx), state, verifier, orchardAuth)); BOOST_CHECK(state.GetRejectReason() == "bad-txns-vpub_old-negative"); jsdesc->vpub_old = MAX_MONEY + 1; BOOST_CHECK_THROW((CTransaction(newTx)), std::ios_base::failure); - BOOST_CHECK(!CheckTransaction(UNSAFE_CTransaction(newTx), state, verifier)); + BOOST_CHECK(!CheckTransaction(UNSAFE_CTransaction(newTx), state, verifier, orchardAuth)); BOOST_CHECK(state.GetRejectReason() == "bad-txns-vpub_old-toolarge"); jsdesc->vpub_old = 0; jsdesc->vpub_new = -1; BOOST_CHECK_THROW((CTransaction(newTx)), std::ios_base::failure); - BOOST_CHECK(!CheckTransaction(UNSAFE_CTransaction(newTx), state, verifier)); + BOOST_CHECK(!CheckTransaction(UNSAFE_CTransaction(newTx), state, verifier, orchardAuth)); BOOST_CHECK(state.GetRejectReason() == "bad-txns-vpub_new-negative"); jsdesc->vpub_new = MAX_MONEY + 1; BOOST_CHECK_THROW((CTransaction(newTx)), std::ios_base::failure); - BOOST_CHECK(!CheckTransaction(UNSAFE_CTransaction(newTx), state, verifier)); + BOOST_CHECK(!CheckTransaction(UNSAFE_CTransaction(newTx), state, verifier, orchardAuth)); BOOST_CHECK(state.GetRejectReason() == "bad-txns-vpub_new-toolarge"); jsdesc->vpub_new = (MAX_MONEY / 2) + 10; @@ -506,7 +511,7 @@ void test_simple_joinsplit_invalidity(uint32_t consensusBranchId, CMutableTransa JSDescription *jsdesc2 = &newTx.vJoinSplit[1]; jsdesc2->vpub_new = (MAX_MONEY / 2) + 10; - BOOST_CHECK(!CheckTransaction(newTx, state, verifier)); + BOOST_CHECK(!CheckTransaction(newTx, state, verifier, orchardAuth)); BOOST_CHECK(state.GetRejectReason() == "bad-txns-txintotal-toolarge"); } { @@ -520,7 +525,7 @@ void test_simple_joinsplit_invalidity(uint32_t consensusBranchId, CMutableTransa jsdesc->nullifiers[0] = GetRandHash(); jsdesc->nullifiers[1] = jsdesc->nullifiers[0]; - BOOST_CHECK(!CheckTransaction(newTx, state, verifier)); + BOOST_CHECK(!CheckTransaction(newTx, state, verifier, orchardAuth)); BOOST_CHECK(state.GetRejectReason() == "bad-joinsplits-nullifiers-duplicate"); jsdesc->nullifiers[1] = GetRandHash(); @@ -532,7 +537,7 @@ void test_simple_joinsplit_invalidity(uint32_t consensusBranchId, CMutableTransa jsdesc2->nullifiers[0] = GetRandHash(); jsdesc2->nullifiers[1] = jsdesc->nullifiers[0]; - BOOST_CHECK(!CheckTransaction(newTx, state, verifier)); + BOOST_CHECK(!CheckTransaction(newTx, state, verifier, orchardAuth)); BOOST_CHECK(state.GetRejectReason() == "bad-joinsplits-nullifiers-duplicate"); } { @@ -551,7 +556,7 @@ void test_simple_joinsplit_invalidity(uint32_t consensusBranchId, CMutableTransa CTransaction finalNewTx(newTx); BOOST_CHECK(finalNewTx.IsCoinBase()); } - BOOST_CHECK(!CheckTransaction(newTx, state, verifier)); + BOOST_CHECK(!CheckTransaction(newTx, state, verifier, orchardAuth)); BOOST_CHECK(state.GetRejectReason() == "bad-cb-has-joinsplits"); } } diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index c1d64919e..c65c0c328 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -18,6 +18,8 @@ #include "wallet/wallet.h" #include "zcash/Proof.hpp" +#include + #include #include #include @@ -487,8 +489,15 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, ssValue >> wtx; CValidationState state; auto verifier = ProofVerifier::Strict(); - if (!(CheckTransaction(wtx, state, verifier) && (wtx.GetHash() == hash) && state.IsValid())) + auto orchardAuth = orchard::AuthValidator::Batch(); + if (!( + CheckTransaction(wtx, state, verifier, orchardAuth) && + (wtx.GetHash() == hash) && + orchardAuth.Validate() && + state.IsValid()) + ) { return false; + } // Undo serialize changes in 31600 if (31404 <= wtx.fTimeReceivedIsTxTime && wtx.fTimeReceivedIsTxTime <= 31703)