diff --git a/src/gtest/test_checktransaction.cpp b/src/gtest/test_checktransaction.cpp index 206e73227..15e5d24b1 100644 --- a/src/gtest/test_checktransaction.cpp +++ b/src/gtest/test_checktransaction.cpp @@ -545,18 +545,20 @@ TEST(ContextualCheckShieldedInputsTest, BadTxnsInvalidJoinsplitSignature) { const PrecomputedTransactionData txdata(tx, allPrevOutputs); MockCValidationState state; + AssumeShieldedInputsExistAndAreSpendable baseView; + CCoinsViewCache view(&baseView); // during initial block download, for transactions being accepted into the // mempool (and thus not mined), DoS ban score should be zero, else 10 EXPECT_CALL(state, DoS(0, false, REJECT_INVALID, "bad-txns-invalid-joinsplit-signature", false, "")).Times(1); - ContextualCheckShieldedInputs(tx, txdata, state, orchardAuth, consensus, 0, false, false, [](const Consensus::Params&) { return true; }); + ContextualCheckShieldedInputs(tx, txdata, state, view, orchardAuth, consensus, 0, false, false, [](const Consensus::Params&) { return true; }); EXPECT_CALL(state, DoS(10, false, REJECT_INVALID, "bad-txns-invalid-joinsplit-signature", false, "")).Times(1); - ContextualCheckShieldedInputs(tx, txdata, state, orchardAuth, consensus, 0, false, false, [](const Consensus::Params&) { return false; }); + ContextualCheckShieldedInputs(tx, txdata, state, view, orchardAuth, consensus, 0, false, false, [](const Consensus::Params&) { return false; }); // for transactions that have been mined in a block, DoS ban score should // always be 100. EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-invalid-joinsplit-signature", false, "")).Times(1); - ContextualCheckShieldedInputs(tx, txdata, state, orchardAuth, consensus, 0, false, true, [](const Consensus::Params&) { return true; }); + ContextualCheckShieldedInputs(tx, txdata, state, view, orchardAuth, consensus, 0, false, true, [](const Consensus::Params&) { return true; }); EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-invalid-joinsplit-signature", false, "")).Times(1); - ContextualCheckShieldedInputs(tx, txdata, state, orchardAuth, consensus, 0, false, true, [](const Consensus::Params&) { return false; }); + ContextualCheckShieldedInputs(tx, txdata, state, view, orchardAuth, consensus, 0, false, true, [](const Consensus::Params&) { return false; }); } TEST(ContextualCheckShieldedInputsTest, JoinsplitSignatureDetectsOldBranchId) { @@ -578,9 +580,11 @@ TEST(ContextualCheckShieldedInputsTest, JoinsplitSignatureDetectsOldBranchId) { const PrecomputedTransactionData txdata(tx, allPrevOutputs); MockCValidationState state; + AssumeShieldedInputsExistAndAreSpendable baseView; + CCoinsViewCache view(&baseView); // Ensure that the transaction validates against Sapling. EXPECT_TRUE(ContextualCheckShieldedInputs( - tx, txdata, state, orchardAuth, consensus, saplingBranchId, false, false, + tx, txdata, state, view, orchardAuth, consensus, saplingBranchId, false, false, [](const Consensus::Params&) { return false; })); // Attempt to validate the inputs against Blossom. We should be notified @@ -592,7 +596,7 @@ TEST(ContextualCheckShieldedInputsTest, JoinsplitSignatureDetectsOldBranchId) { HexInt(saplingBranchId)), false, "")).Times(1); EXPECT_FALSE(ContextualCheckShieldedInputs( - tx, txdata, state, orchardAuth, consensus, blossomBranchId, false, false, + tx, txdata, state, view, orchardAuth, consensus, blossomBranchId, false, false, [](const Consensus::Params&) { return false; })); // Attempt to validate the inputs against Heartwood. All we should learn is @@ -602,7 +606,7 @@ TEST(ContextualCheckShieldedInputsTest, JoinsplitSignatureDetectsOldBranchId) { 10, false, REJECT_INVALID, "bad-txns-invalid-joinsplit-signature", false, "")).Times(1); EXPECT_FALSE(ContextualCheckShieldedInputs( - tx, txdata, state, orchardAuth, consensus, heartwoodBranchId, false, false, + tx, txdata, state, view, orchardAuth, consensus, heartwoodBranchId, false, false, [](const Consensus::Params&) { return false; })); } @@ -611,6 +615,9 @@ TEST(ContextualCheckShieldedInputsTest, NonCanonicalEd25519Signature) { auto consensus = Params().GetConsensus(); auto orchardAuth = orchard::AuthValidator::Disabled(); + AssumeShieldedInputsExistAndAreSpendable baseView; + CCoinsViewCache view(&baseView); + auto saplingBranchId = NetworkUpgradeInfo[Consensus::UPGRADE_SAPLING].nBranchId; CMutableTransaction mtx = GetValidTransaction(saplingBranchId); @@ -623,7 +630,7 @@ TEST(ContextualCheckShieldedInputsTest, NonCanonicalEd25519Signature) { CTransaction tx(mtx); const PrecomputedTransactionData txdata(tx, allPrevOutputs); MockCValidationState state; - EXPECT_TRUE(ContextualCheckShieldedInputs(tx, txdata, state, orchardAuth, consensus, saplingBranchId, false, true)); + EXPECT_TRUE(ContextualCheckShieldedInputs(tx, txdata, state, view, orchardAuth, consensus, saplingBranchId, false, true)); } // Copied from libsodium/crypto_sign/ed25519/ref10/open.c @@ -647,15 +654,15 @@ TEST(ContextualCheckShieldedInputsTest, NonCanonicalEd25519Signature) { // during initial block download, for transactions being accepted into the // mempool (and thus not mined), DoS ban score should be zero, else 10 EXPECT_CALL(state, DoS(0, false, REJECT_INVALID, "bad-txns-invalid-joinsplit-signature", false, "")).Times(1); - ContextualCheckShieldedInputs(tx, txdata, state, orchardAuth, consensus, saplingBranchId, false, false, [](const Consensus::Params&) { return true; }); + ContextualCheckShieldedInputs(tx, txdata, state, view, orchardAuth, consensus, saplingBranchId, false, false, [](const Consensus::Params&) { return true; }); EXPECT_CALL(state, DoS(10, false, REJECT_INVALID, "bad-txns-invalid-joinsplit-signature", false, "")).Times(1); - ContextualCheckShieldedInputs(tx, txdata, state, orchardAuth, consensus, saplingBranchId, false, false, [](const Consensus::Params&) { return false; }); + ContextualCheckShieldedInputs(tx, txdata, state, view, orchardAuth, consensus, saplingBranchId, false, false, [](const Consensus::Params&) { return false; }); // for transactions that have been mined in a block, DoS ban score should // always be 100. EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-invalid-joinsplit-signature", false, "")).Times(1); - ContextualCheckShieldedInputs(tx, txdata, state, orchardAuth, consensus, saplingBranchId, false, true, [](const Consensus::Params&) { return true; }); + ContextualCheckShieldedInputs(tx, txdata, state, view, orchardAuth, consensus, saplingBranchId, false, true, [](const Consensus::Params&) { return true; }); EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-invalid-joinsplit-signature", false, "")).Times(1); - ContextualCheckShieldedInputs(tx, txdata, state, orchardAuth, consensus, saplingBranchId, false, true, [](const Consensus::Params&) { return false; }); + ContextualCheckShieldedInputs(tx, txdata, state, view, orchardAuth, consensus, saplingBranchId, false, true, [](const Consensus::Params&) { return false; }); } TEST(ChecktransactionTests, OverwinterConstructors) { @@ -1322,9 +1329,11 @@ TEST(ChecktransactionTests, HeartwoodEnforcesSaplingRulesOnShieldedCoinbase) { // their transparent input; ZIP 244 handles this by making the coinbase // sighash the txid. PrecomputedTransactionData txdata(tx, {}); + AssumeShieldedInputsExistAndAreSpendable baseView; + CCoinsViewCache view(&baseView); EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-sapling-binding-signature-invalid", false, "")).Times(1); EXPECT_FALSE(ContextualCheckShieldedInputs( - tx, txdata, state, orchardAuth, chainparams.GetConsensus(), heartwoodBranchId, false, true)); + tx, txdata, state, view, orchardAuth, chainparams.GetConsensus(), heartwoodBranchId, false, true)); RegtestDeactivateHeartwood(); } diff --git a/src/main.cpp b/src/main.cpp index d9d4cae8c..99fd395da 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1233,6 +1233,7 @@ bool ContextualCheckShieldedInputs( const CTransaction& tx, const PrecomputedTransactionData& txdata, CValidationState &state, + const CCoinsViewCache &view, orchard::AuthValidator& orchardAuth, const Consensus::Params& consensus, uint32_t consensusBranchId, @@ -1240,6 +1241,12 @@ bool ContextualCheckShieldedInputs( bool isMined, bool (*isInitBlockDownload)(const Consensus::Params&)) { + // This doesn't trigger the DoS code on purpose; if it did, it would make it easier + // for an attacker to attempt to split the network. + if (!Consensus::CheckTxShieldedInputs(tx, state, view, 0)) { + return false; + } + const int DOS_LEVEL_BLOCK = 100; // DoS level set to 10 to be more forgiving. const int DOS_LEVEL_MEMPOOL = 10; @@ -1868,16 +1875,10 @@ bool AcceptToMemoryPool( return state.Invalid(false, REJECT_DUPLICATE, "bad-txns-inputs-spent"); // Are the shielded spends' requirements met? - auto unmetShieldedReq = view.HaveShieldedRequirements(tx); - if (unmetShieldedReq) { - auto txid = tx.GetHash().ToString(); - auto rejectCode = ShieldedReqRejectCode(*unmetShieldedReq); - auto rejectReason = ShieldedReqRejectReason(*unmetShieldedReq); - TracingError( - "main", "AcceptToMemoryPool(): shielded requirements not met", - "txid", txid.c_str(), - "reason", rejectReason.c_str()); - return state.Invalid(false, rejectCode, rejectReason); + // This doesn't trigger the DoS code on purpose; if it did, it would make it easier + // for an attacker to attempt to split the network. + if (!Consensus::CheckTxShieldedInputs(tx, state, view, 0)) { + return false; } // Bring the best block into scope @@ -1885,6 +1886,7 @@ bool AcceptToMemoryPool( nValueIn = view.GetValueIn(tx); + // we have all inputs cached now, so switch back to dummy view.SetBackend(dummy); // Check for non-standard pay-to-script-hash in inputs @@ -2004,6 +2006,7 @@ bool AcceptToMemoryPool( tx, txdata, state, + view, orchardAuth, chainparams.GetConsensus(), consensusBranchId, @@ -2551,26 +2554,39 @@ int GetSpendHeight(const CCoinsViewCache& inputs) } namespace Consensus { +bool CheckTxShieldedInputs( + const CTransaction& tx, + CValidationState& state, + const CCoinsViewCache& view, + int dosLevel) +{ + // Are the shielded spends' requirements met? + auto unmetShieldedReq = view.HaveShieldedRequirements(tx); + if (unmetShieldedReq) { + auto txid = tx.GetHash().ToString(); + auto rejectCode = ShieldedReqRejectCode(*unmetShieldedReq); + auto rejectReason = ShieldedReqRejectReason(*unmetShieldedReq); + TracingError( + "main", "CheckTxShieldedInputs(): shielded requirements not met", + "txid", txid.c_str(), + "reason", rejectReason.c_str()); + return state.DoS(dosLevel, false, rejectCode, rejectReason); + } + + return true; +} + bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, const Consensus::Params& consensusParams) { + assert(!tx.IsCoinBase()); + + // Indented to reduce conflicts with upstream. + { // This doesn't trigger the DoS code on purpose; if it did, it would make it easier // for an attacker to attempt to split the network. if (!inputs.HaveInputs(tx)) return state.Invalid(false, 0, "", "Inputs unavailable"); - // Are the shielded spends' requirements met? - auto unmetShieldedReq = inputs.HaveShieldedRequirements(tx); - if (unmetShieldedReq) { - auto txid = tx.GetHash().ToString(); - auto rejectCode = ShieldedReqRejectCode(*unmetShieldedReq); - auto rejectReason = ShieldedReqRejectReason(*unmetShieldedReq); - TracingError( - "main", "CheckInputs(): shielded requirements not met", - "txid", txid.c_str(), - "reason", rejectReason.c_str()); - return state.Invalid(false, rejectCode, rejectReason); - } - CAmount nValueIn = 0; CAmount nFees = 0; for (unsigned int i = 0; i < tx.vin.size(); i++) @@ -2621,6 +2637,7 @@ bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoins nFees += nTxFee; if (!MoneyRange(nFees)) return state.DoS(100, false, REJECT_INVALID, "bad-txns-fee-outofrange"); + } return true; } }// namespace Consensus @@ -3220,25 +3237,17 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin // txid. std::vector allPrevOutputs; + // Are the shielded spends' requirements met? + if (!Consensus::CheckTxShieldedInputs(tx, state, view, 100)) { + return false; + } + if (!tx.IsCoinBase()) { if (!view.HaveInputs(tx)) return state.DoS(100, error("ConnectBlock(): inputs missing/spent"), REJECT_INVALID, "bad-txns-inputs-missingorspent"); - // Are the shielded spends' requirements met? - auto unmetShieldedReq = view.HaveShieldedRequirements(tx); - if (unmetShieldedReq) { - auto txid = tx.GetHash().ToString(); - auto rejectCode = ShieldedReqRejectCode(*unmetShieldedReq); - auto rejectReason = ShieldedReqRejectReason(*unmetShieldedReq); - TracingError( - "main", "ConnectBlock(): shielded requirements not met", - "txid", txid.c_str(), - "reason", rejectReason.c_str()); - return state.DoS(100, false, rejectCode, rejectReason); - } - for (const auto& input : tx.vin) { allPrevOutputs.push_back(view.GetOutputFor(input)); } @@ -3303,6 +3312,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin tx, txdata.back(), state, + view, orchardAuth, chainparams.GetConsensus(), consensusBranchId, diff --git a/src/main.h b/src/main.h index a5b6142e3..9335ed5ff 100644 --- a/src/main.h +++ b/src/main.h @@ -373,19 +373,29 @@ bool ContextualCheckInputs(const CTransaction& tx, CValidationState &state, cons std::vector *pvChecks = NULL); /** - * Checks the signatures for a transaction's shielded components. + * Check whether all shielded inputs of this transaction are valid. + * + * This checks that: + * - The anchors in the transaction exist in the given view. + * - The nullifiers in the transaction do not exist in the given view. + * - The signatures for the transaction's shielded components are valid. * * This also currently checks the Sapling proofs, due to the way the Rust verification * code is written. Sprout and Orchard proofs are currently checked in CheckTransaction(). * Once we have batch proof validation implemented, these will all be accumulated in * CheckTransaction(). * + * To skip checking signatures, use `Consensus::CheckTxShieldedInputs` instead. + * + * This does not modify the view to add the nullifiers to the spent set. + * * The `isInitBlockDownload` argument is a function parameter to assist with testing. */ bool ContextualCheckShieldedInputs( const CTransaction& tx, const PrecomputedTransactionData& txdata, CValidationState &state, + const CCoinsViewCache &view, orchard::AuthValidator& orchardAuth, const Consensus::Params& consensus, uint32_t consensusBranchId, @@ -410,6 +420,22 @@ bool CheckTransactionWithoutProofVerification(const CTransaction& tx, CValidatio namespace Consensus { +/** + * Check whether all shielded inputs of this transaction are valid. + * + * This checks that: + * - The anchors in the transaction exist in the given view. + * - The nullifiers in the transaction do not exist in the given view. + * + * This does not modify the view to add the nullifiers to the spent set. + * This does not check proofs or signatures. + */ +bool CheckTxShieldedInputs( + const CTransaction& tx, + CValidationState& state, + const CCoinsViewCache& view, + int dosLevel); + /** * Check whether all inputs of this transaction are valid (no double spends and amounts) * This does not modify the UTXO set. This does not check scripts and sigs. diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 6e970e000..eaf5e9d8d 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -24,6 +24,7 @@ #include "test/test_util.h" #include "primitives/transaction.h" #include "transaction_builder.h" +#include "utiltest.h" #include #include @@ -436,6 +437,8 @@ void test_simple_joinsplit_invalidity(uint32_t consensusBranchId, CMutableTransa // joinsplits. CMutableTransaction newTx(tx); CValidationState state; + AssumeShieldedInputsExistAndAreSpendable baseView; + CCoinsViewCache view(&baseView); Ed25519SigningKey joinSplitPrivKey; ed25519_generate_keypair(&joinSplitPrivKey, &newTx.joinSplitPubKey); @@ -462,7 +465,7 @@ void test_simple_joinsplit_invalidity(uint32_t consensusBranchId, CMutableTransa BOOST_CHECK(CheckTransactionWithoutProofVerification(newTx, state)); BOOST_CHECK(ContextualCheckTransaction(newTx, state, Params(), 0, true)); - BOOST_CHECK(!ContextualCheckShieldedInputs(newTx, txdata, state, orchardAuth, Params().GetConsensus(), consensusBranchId, false, true)); + BOOST_CHECK(!ContextualCheckShieldedInputs(newTx, txdata, state, view, orchardAuth, Params().GetConsensus(), consensusBranchId, false, true)); BOOST_CHECK(state.GetRejectReason() == "bad-txns-invalid-joinsplit-signature"); // Empty output script. @@ -478,7 +481,7 @@ void test_simple_joinsplit_invalidity(uint32_t consensusBranchId, CMutableTransa state = CValidationState(); BOOST_CHECK(CheckTransactionWithoutProofVerification(newTx, state)); BOOST_CHECK(ContextualCheckTransaction(newTx, state, Params(), 0, true)); - BOOST_CHECK(ContextualCheckShieldedInputs(newTx, txdata, state, orchardAuth, Params().GetConsensus(), consensusBranchId, false, true)); + BOOST_CHECK(ContextualCheckShieldedInputs(newTx, txdata, state, view, orchardAuth, Params().GetConsensus(), consensusBranchId, false, true)); BOOST_CHECK_EQUAL(state.GetRejectReason(), ""); } { diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 3dcf1bec2..823d476a4 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -583,42 +583,13 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const i++; } - // The SaltedTxidHasher is fine to use here; it salts the map keys automatically - // with randomness generated on construction. - boost::unordered_map intermediates; - - for (const JSDescription &joinsplit : tx.vJoinSplit) { - for (const uint256 &nf : joinsplit.nullifiers) { - assert(!pcoins->GetNullifier(nf, SPROUT)); - } - - SproutMerkleTree tree; - auto it = intermediates.find(joinsplit.anchor); - if (it != intermediates.end()) { - tree = it->second; - } else { - assert(pcoins->GetSproutAnchorAt(joinsplit.anchor, tree)); - } - - for (const uint256& commitment : joinsplit.commitments) - { - tree.append(commitment); - } - - intermediates.insert(std::make_pair(tree.root(), tree)); - } - for (const SpendDescription &spendDescription : tx.vShieldedSpend) { - SaplingMerkleTree tree; - - assert(pcoins->GetSaplingAnchorAt(spendDescription.anchor, tree)); - assert(!pcoins->GetNullifier(spendDescription.nullifier, SAPLING)); - } if (fDependsWait) waitingOnDependants.push_back(&(*it)); else { CValidationState state; bool fCheckResult = tx.IsCoinBase() || Consensus::CheckTxInputs(tx, state, mempoolDuplicate, nSpendHeight, Params().GetConsensus()); + fCheckResult &= Consensus::CheckTxShieldedInputs(tx, state, mempoolDuplicate, 0); assert(fCheckResult); UpdateCoins(tx, mempoolDuplicate, 1000000); } @@ -635,6 +606,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const } else { bool fCheckResult = entry->GetTx().IsCoinBase() || Consensus::CheckTxInputs(entry->GetTx(), state, mempoolDuplicate, nSpendHeight, Params().GetConsensus()); + fCheckResult &= Consensus::CheckTxShieldedInputs(entry->GetTx(), state, mempoolDuplicate, 0); assert(fCheckResult); UpdateCoins(entry->GetTx(), mempoolDuplicate, 1000000); stepsSinceLastRemove = 0; diff --git a/src/utiltest.h b/src/utiltest.h index d289d0dd5..2ab8e222b 100644 --- a/src/utiltest.h +++ b/src/utiltest.h @@ -5,12 +5,36 @@ #ifndef ZCASH_UTILTEST_H #define ZCASH_UTILTEST_H +#include "coins.h" #include "key_io.h" #include "wallet/wallet.h" #include "zcash/Address.hpp" #include "zcash/Note.hpp" #include "zcash/NoteEncryption.hpp" +// A fake chain state view where anchors and nullifiers are assumed to exist. +class AssumeShieldedInputsExistAndAreSpendable : public CCoinsView { +public: + AssumeShieldedInputsExistAndAreSpendable() {} + + bool GetSproutAnchorAt(const uint256 &rt, SproutMerkleTree &tree) const { + return true; + } + + bool GetSaplingAnchorAt(const uint256 &rt, SaplingMerkleTree &tree) const { + return true; + } + + bool GetOrchardAnchorAt(const uint256 &rt, OrchardMerkleFrontier &tree) const { + return true; + } + + bool GetNullifier(const uint256 &nf, ShieldedType type) const { + // Always return false so we treat every nullifier as being unspent. + return false; + } +}; + // Sprout CWalletTx GetValidSproutReceive(const libzcash::SproutSpendingKey& sk, CAmount value,