From 948d4e6c1088fadabef71ae5bfa19dac773393aa Mon Sep 17 00:00:00 2001 From: Taylor Hornby Date: Thu, 23 Jun 2016 16:35:31 -0600 Subject: [PATCH] Split JoinSplit proof verification out of CheckTransaction. --- src/consensus/validation.h | 9 +---- src/gtest/test_checktransaction.cpp | 56 ++++++++++++++++++++--------- src/main.cpp | 27 ++++++++------ src/main.h | 1 + src/test/sighash_tests.cpp | 3 +- src/test/transaction_tests.cpp | 10 +++--- 6 files changed, 62 insertions(+), 44 deletions(-) diff --git a/src/consensus/validation.h b/src/consensus/validation.h index 8e1ac2ef3..d0ea30bf4 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -30,9 +30,8 @@ private: std::string strRejectReason; unsigned char chRejectCode; bool corruptionPossible; - bool pourVerify; public: - CValidationState() : mode(MODE_VALID), nDoS(0), chRejectCode(0), corruptionPossible(false), pourVerify(true) {} + CValidationState() : mode(MODE_VALID), nDoS(0), chRejectCode(0), corruptionPossible(false) {} virtual bool DoS(int level, bool ret = false, unsigned char chRejectCodeIn=0, std::string strRejectReasonIn="", bool corruptionIn=false) { @@ -45,12 +44,6 @@ public: mode = MODE_INVALID; return ret; } - virtual bool SetPerformPourVerification(bool pourVerifyIn) { - pourVerify = pourVerifyIn; - } - virtual bool PerformPourVerification() { - return pourVerify; - } virtual bool Invalid(bool ret = false, unsigned char _chRejectCode=0, std::string _strRejectReason="") { return DoS(0, ret, _chRejectCode, _strRejectReason); diff --git a/src/gtest/test_checktransaction.cpp b/src/gtest/test_checktransaction.cpp index 71b98e97f..1b7fadb6d 100644 --- a/src/gtest/test_checktransaction.cpp +++ b/src/gtest/test_checktransaction.cpp @@ -1,5 +1,6 @@ #include #include +#include #include "main.h" #include "primitives/transaction.h" @@ -13,7 +14,6 @@ TEST(checktransaction_tests, check_vpub_not_both_nonzero) { // Ensure that values within the pour are well-formed. CMutableTransaction newTx(tx); CValidationState state; - state.SetPerformPourVerification(false); // don't verify the snark newTx.vpour.push_back(CPourTx()); @@ -21,7 +21,7 @@ TEST(checktransaction_tests, check_vpub_not_both_nonzero) { pourtx->vpub_old = 1; pourtx->vpub_new = 1; - EXPECT_FALSE(CheckTransaction(newTx, state)); + EXPECT_FALSE(CheckTransactionWithoutProofVerification(newTx, state)); EXPECT_EQ(state.GetRejectReason(), "bad-txns-vpubs-both-nonzero"); } } @@ -31,8 +31,6 @@ public: MOCK_METHOD5(DoS, bool(int level, bool ret, unsigned char chRejectCodeIn, std::string strRejectReasonIn, bool corruptionIn)); - MOCK_METHOD1(SetPerformPourVerification, bool(bool pourVerifyIn)); - MOCK_METHOD0(PerformPourVerification, bool()); MOCK_METHOD3(Invalid, bool(bool ret, unsigned char _chRejectCode, std::string _strRejectReason)); MOCK_METHOD1(Error, bool(std::string strRejectReasonIn)); @@ -62,6 +60,30 @@ CMutableTransaction GetValidTransaction() { mtx.vpour[0].serials.at(1) = uint256S("0000000000000000000000000000000000000000000000000000000000000001"); mtx.vpour[1].serials.at(0) = uint256S("0000000000000000000000000000000000000000000000000000000000000002"); mtx.vpour[1].serials.at(1) = uint256S("0000000000000000000000000000000000000000000000000000000000000003"); + + + // Generate an ephemeral keypair. + uint256 joinSplitPubKey; + unsigned char joinSplitPrivKey[crypto_sign_SECRETKEYBYTES]; + crypto_sign_keypair(joinSplitPubKey.begin(), joinSplitPrivKey); + mtx.joinSplitPubKey = joinSplitPubKey; + + // Compute the correct hSig. + // TODO: #966. + static const uint256 one(uint256S("0000000000000000000000000000000000000000000000000000000000000001")); + // Empty output script. + CScript scriptCode; + CTransaction signTx(mtx); + uint256 dataToBeSigned = SignatureHash(scriptCode, signTx, NOT_AN_INPUT, SIGHASH_ALL); + if (dataToBeSigned == one) { + throw std::runtime_error("SignatureHash failed"); + } + + // Add the signature + assert(crypto_sign_detached(&mtx.joinSplitSig[0], NULL, + dataToBeSigned.begin(), 32, + joinSplitPrivKey + ) == 0); return mtx; } @@ -69,7 +91,7 @@ TEST(checktransaction_tests, valid_transaction) { CMutableTransaction mtx = GetValidTransaction(); CTransaction tx(mtx); MockCValidationState state; - EXPECT_TRUE(CheckTransaction(tx, state)); + EXPECT_TRUE(CheckTransactionWithoutProofVerification(tx, state)); } TEST(checktransaction_tests, bad_txns_vin_empty) { @@ -80,7 +102,7 @@ TEST(checktransaction_tests, bad_txns_vin_empty) { CTransaction tx(mtx); MockCValidationState state; EXPECT_CALL(state, DoS(10, false, REJECT_INVALID, "bad-txns-vin-empty", false)).Times(1); - CheckTransaction(tx, state); + CheckTransactionWithoutProofVerification(tx, state); } TEST(checktransaction_tests, bad_txns_vout_empty) { @@ -92,7 +114,7 @@ TEST(checktransaction_tests, bad_txns_vout_empty) { MockCValidationState state; EXPECT_CALL(state, DoS(10, false, REJECT_INVALID, "bad-txns-vout-empty", false)).Times(1); - CheckTransaction(tx, state); + CheckTransactionWithoutProofVerification(tx, state); } TEST(checktransaction_tests, bad_txns_oversize) { @@ -109,7 +131,7 @@ TEST(checktransaction_tests, bad_txns_oversize) { MockCValidationState state; EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-oversize", false)).Times(1); - CheckTransaction(tx, state); + CheckTransactionWithoutProofVerification(tx, state); } TEST(checktransaction_tests, bad_txns_vout_negative) { @@ -120,7 +142,7 @@ TEST(checktransaction_tests, bad_txns_vout_negative) { MockCValidationState state; EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-vout-negative", false)).Times(1); - CheckTransaction(tx, state); + CheckTransactionWithoutProofVerification(tx, state); } TEST(checktransaction_tests, bad_txns_vout_toolarge) { @@ -131,7 +153,7 @@ TEST(checktransaction_tests, bad_txns_vout_toolarge) { MockCValidationState state; EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-vout-toolarge", false)).Times(1); - CheckTransaction(tx, state); + CheckTransactionWithoutProofVerification(tx, state); } TEST(checktransaction_tests, bad_txns_txouttotal_toolarge_outputs) { @@ -143,7 +165,7 @@ TEST(checktransaction_tests, bad_txns_txouttotal_toolarge_outputs) { MockCValidationState state; EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-txouttotal-toolarge", false)).Times(1); - CheckTransaction(tx, state); + CheckTransactionWithoutProofVerification(tx, state); } TEST(checktransaction_tests, bad_txns_txouttotal_toolarge_joinsplit) { @@ -155,7 +177,7 @@ TEST(checktransaction_tests, bad_txns_txouttotal_toolarge_joinsplit) { MockCValidationState state; EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-txouttotal-toolarge", false)).Times(1); - CheckTransaction(tx, state); + CheckTransactionWithoutProofVerification(tx, state); } TEST(checktransaction_tests, bad_txns_vpub_old_negative) { @@ -166,7 +188,7 @@ TEST(checktransaction_tests, bad_txns_vpub_old_negative) { MockCValidationState state; EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-vpub_old-negative", false)).Times(1); - CheckTransaction(tx, state); + CheckTransactionWithoutProofVerification(tx, state); } TEST(checktransaction_tests, bad_txns_vpub_new_negative) { @@ -177,7 +199,7 @@ TEST(checktransaction_tests, bad_txns_vpub_new_negative) { MockCValidationState state; EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-vpub_new-negative", false)).Times(1); - CheckTransaction(tx, state); + CheckTransactionWithoutProofVerification(tx, state); } TEST(checktransaction_tests, bad_txns_vpub_old_toolarge) { @@ -188,7 +210,7 @@ TEST(checktransaction_tests, bad_txns_vpub_old_toolarge) { MockCValidationState state; EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-vpub_old-toolarge", false)).Times(1); - CheckTransaction(tx, state); + CheckTransactionWithoutProofVerification(tx, state); } TEST(checktransaction_tests, bad_txns_vpub_new_toolarge) { @@ -199,7 +221,7 @@ TEST(checktransaction_tests, bad_txns_vpub_new_toolarge) { MockCValidationState state; EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-vpub_new-toolarge", false)).Times(1); - CheckTransaction(tx, state); + CheckTransactionWithoutProofVerification(tx, state); } TEST(checktransaction_tests, bad_txns_vpubs_both_nonzero) { @@ -211,5 +233,5 @@ TEST(checktransaction_tests, bad_txns_vpubs_both_nonzero) { MockCValidationState state; EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-vpubs-both-nonzero", false)).Times(1); - CheckTransaction(tx, state); + CheckTransactionWithoutProofVerification(tx, state); } diff --git a/src/main.cpp b/src/main.cpp index ef3d8a8bf..6b41a6772 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -870,8 +870,23 @@ crypto_sign_check_S_lt_l(const unsigned char *S) } - bool CheckTransaction(const CTransaction& tx, CValidationState &state) +{ + if (!CheckTransactionWithoutProofVerification(tx, state)) { + return false; + } else { + // Ensure that zk-SNARKs verify + BOOST_FOREACH(const CPourTx &pour, tx.vpour) { + if (!pour.Verify(*pzcashParams, tx.joinSplitPubKey)) { + return state.DoS(100, error("CheckTransaction(): pour does not verify"), + REJECT_INVALID, "bad-txns-pour-verification-failed"); + } + } + return true; + } +} + +bool CheckTransactionWithoutProofVerification(const CTransaction& tx, CValidationState &state) { // Basic checks that don't depend on any context @@ -1008,16 +1023,6 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state) return state.DoS(100, error("CheckTransaction(): non-canonical ed25519 signature"), REJECT_INVALID, "non-canonical-ed25519-signature"); } - - if (state.PerformPourVerification()) { - // Ensure that zk-SNARKs verify - BOOST_FOREACH(const CPourTx &pour, tx.vpour) { - if (!pour.Verify(*pzcashParams, tx.joinSplitPubKey)) { - return state.DoS(100, error("CheckTransaction(): pour does not verify"), - REJECT_INVALID, "bad-txns-pour-verification-failed"); - } - } - } } } diff --git a/src/main.h b/src/main.h index 088c58be5..2ce1a5575 100644 --- a/src/main.h +++ b/src/main.h @@ -333,6 +333,7 @@ void UpdateCoins(const CTransaction& tx, CValidationState &state, CCoinsViewCach /** Context-independent validity checks */ bool CheckTransaction(const CTransaction& tx, CValidationState& state); +bool CheckTransactionWithoutProofVerification(const CTransaction& tx, CValidationState &state); /** Check for standard transaction types * @return True if all outputs (scriptPubKeys) use only standard transaction forms diff --git a/src/test/sighash_tests.cpp b/src/test/sighash_tests.cpp index f8732bde2..3d8714c0e 100644 --- a/src/test/sighash_tests.cpp +++ b/src/test/sighash_tests.cpp @@ -244,8 +244,7 @@ BOOST_AUTO_TEST_CASE(sighash_from_data) stream >> tx; CValidationState state; - state.SetPerformPourVerification(false); // don't verify the snark - BOOST_CHECK_MESSAGE(CheckTransaction(tx, state), strTest); + BOOST_CHECK_MESSAGE(CheckTransactionWithoutProofVerification(tx, state), strTest); BOOST_CHECK(state.IsValid()); std::vector raw = ParseHex(raw_script); diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 8bcd0eaa5..3c8d4c197 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -384,15 +384,13 @@ BOOST_AUTO_TEST_CASE(test_simple_pour_invalidity) unsigned char joinSplitPrivKey[crypto_sign_SECRETKEYBYTES]; crypto_sign_keypair(newTx.joinSplitPubKey.begin(), joinSplitPrivKey); - state.SetPerformPourVerification(false); // don't verify the snark - // No pours, vin and vout, means it should be invalid. - BOOST_CHECK(!CheckTransaction(newTx, state)); + BOOST_CHECK(!CheckTransactionWithoutProofVerification(newTx, state)); BOOST_CHECK(state.GetRejectReason() == "bad-txns-vin-empty"); newTx.vin.push_back(CTxIn(uint256S("0000000000000000000000000000000000000000000000000000000000000001"), 0)); - BOOST_CHECK(!CheckTransaction(newTx, state)); + BOOST_CHECK(!CheckTransactionWithoutProofVerification(newTx, state)); BOOST_CHECK(state.GetRejectReason() == "bad-txns-vout-empty"); newTx.vpour.push_back(CPourTx()); @@ -401,7 +399,7 @@ BOOST_AUTO_TEST_CASE(test_simple_pour_invalidity) pourtx->serials[0] = GetRandHash(); pourtx->serials[1] = GetRandHash(); - BOOST_CHECK(!CheckTransaction(newTx, state)); + BOOST_CHECK(!CheckTransactionWithoutProofVerification(newTx, state)); BOOST_CHECK(state.GetRejectReason() == "bad-txns-invalid-joinsplit-signature"); // TODO: #966. @@ -417,7 +415,7 @@ BOOST_AUTO_TEST_CASE(test_simple_pour_invalidity) joinSplitPrivKey ) == 0); - BOOST_CHECK(CheckTransaction(newTx, state)); + BOOST_CHECK(CheckTransactionWithoutProofVerification(newTx, state)); } { // Ensure that values within the pour are well-formed.