Auto merge of #1051 - ThisIsNotOfficialCodeItsJustForks:t863-mock-coverage-of-checktransaction, r=ebfull

Add mock test coverage of CheckTransaction Closes #863.
2016-07-14 18:43:07 +00:00 · 2016-07-14 18:43:07 +00:00 · fc127fc2db
parent 4bcdd85583 a4944ffcb0
commit fc127fc2db
6 changed files with 328 additions and 34 deletions
--- 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);
--- a/src/gtest/test_checktransaction.cpp
+++ b/src/gtest/test_checktransaction.cpp
@ -1,5 +1,6 @@
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
+#include <sodium.h>

 #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));
@ -45,9 +43,309 @@ public:
    MOCK_CONST_METHOD0(GetRejectReason, std::string());
 };

-TEST(checktransaction_tests, mock_proof_of_concept) {
-    CTransaction tx;
+
+CMutableTransaction GetValidTransaction() {
+    CMutableTransaction mtx;
+    mtx.vin.resize(2);
+    mtx.vin[0].prevout.hash = uint256S("0000000000000000000000000000000000000000000000000000000000000001");
+    mtx.vin[0].prevout.n = 0;
+    mtx.vin[1].prevout.hash = uint256S("0000000000000000000000000000000000000000000000000000000000000002");
+    mtx.vin[1].prevout.n = 0;
+    mtx.vout.resize(2);
+    // mtx.vout[0].scriptPubKey = 
+    mtx.vout[0].nValue = 0;
+    mtx.vout[1].nValue = 0;
+    mtx.vpour.resize(2);
+    mtx.vpour[0].serials.at(0) = uint256S("0000000000000000000000000000000000000000000000000000000000000000");
+    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;
+}
+
+TEST(checktransaction_tests, valid_transaction) {
+    CMutableTransaction mtx = GetValidTransaction();
+    CTransaction tx(mtx);
+    MockCValidationState state;
+    EXPECT_TRUE(CheckTransactionWithoutProofVerification(tx, state));
+}
+
+TEST(checktransaction_tests, bad_txns_vin_empty) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vpour.resize(0);
+    mtx.vin.resize(0);
+
+    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) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vpour.resize(0);
+    mtx.vout.resize(0);
+
+    CTransaction tx(mtx);
+
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(10, false, REJECT_INVALID, "bad-txns-vout-empty", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
+TEST(checktransaction_tests, bad_txns_oversize) {
+    CMutableTransaction mtx = GetValidTransaction();
+
+    mtx.vin[0].scriptSig = CScript();
+    // 18 * (520char + DROP) + OP_1 = 9433 bytes
+    std::vector<unsigned char> vchData(520);
+    for (unsigned int i = 0; i < 4000; ++i)
+        mtx.vin[0].scriptSig << vchData << OP_DROP;
+    mtx.vin[0].scriptSig << OP_1;
+
+    CTransaction tx(mtx);
+
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-oversize", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
+TEST(checktransaction_tests, bad_txns_vout_negative) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vout[0].nValue = -1;
+
+    CTransaction tx(mtx);
+
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-vout-negative", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
+TEST(checktransaction_tests, bad_txns_vout_toolarge) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vout[0].nValue = MAX_MONEY + 1;
+
+    CTransaction tx(mtx);
+
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-vout-toolarge", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
+TEST(checktransaction_tests, bad_txns_txouttotal_toolarge_outputs) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vout[0].nValue = MAX_MONEY;
+    mtx.vout[1].nValue = 1;
+
+    CTransaction tx(mtx);
+
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-txouttotal-toolarge", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
+TEST(checktransaction_tests, bad_txns_txouttotal_toolarge_joinsplit) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vout[0].nValue = 1;
+    mtx.vpour[0].vpub_new = MAX_MONEY;
+
+    CTransaction tx(mtx);
+
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-txouttotal-toolarge", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
+TEST(checktransaction_tests, bad_txns_vpub_old_negative) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vpour[0].vpub_old = -1;
+
+    CTransaction tx(mtx);
+
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-vpub_old-negative", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
+TEST(checktransaction_tests, bad_txns_vpub_new_negative) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vpour[0].vpub_new = -1;
+
+    CTransaction tx(mtx);
+
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-vpub_new-negative", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
+TEST(checktransaction_tests, bad_txns_vpub_old_toolarge) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vpour[0].vpub_old = MAX_MONEY + 1;
+
+    CTransaction tx(mtx);
+
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-vpub_old-toolarge", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
+TEST(checktransaction_tests, bad_txns_vpub_new_toolarge) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vpour[0].vpub_new = MAX_MONEY + 1;
+
+    CTransaction tx(mtx);
+
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-vpub_new-toolarge", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
+TEST(checktransaction_tests, bad_txns_vpubs_both_nonzero) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vpour[0].vpub_old = 1;
+    mtx.vpour[0].vpub_new = 1;
+
+    CTransaction tx(mtx);
+
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-vpubs-both-nonzero", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
+TEST(checktransaction_tests, bad_txns_inputs_duplicate) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vin[1].prevout.hash = mtx.vin[0].prevout.hash;
+    mtx.vin[1].prevout.n = mtx.vin[0].prevout.n;
+
+    CTransaction tx(mtx);
+
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-inputs-duplicate", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
+TEST(checktransaction_tests, bad_pours_serials_duplicate_same_pour) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vpour[0].serials.at(0) = uint256S("0000000000000000000000000000000000000000000000000000000000000000");
+    mtx.vpour[0].serials.at(1) = uint256S("0000000000000000000000000000000000000000000000000000000000000000");
+
+    CTransaction tx(mtx);
+
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-pours-serials-duplicate", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
+TEST(checktransaction_tests, bad_pours_serials_duplicate_different_pour) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vpour[0].serials.at(0) = uint256S("0000000000000000000000000000000000000000000000000000000000000000");
+    mtx.vpour[1].serials.at(0) = uint256S("0000000000000000000000000000000000000000000000000000000000000000");
+
+    CTransaction tx(mtx);
+
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-pours-serials-duplicate", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
+TEST(checktransaction_tests, bad_cb_has_pours) {
+    CMutableTransaction mtx = GetValidTransaction();
+    // Make it a coinbase.
+    mtx.vin.resize(1);
+    mtx.vin[0].prevout.SetNull();
+
+    mtx.vpour.resize(1);
+
+    CTransaction tx(mtx);
+    EXPECT_TRUE(tx.IsCoinBase());
+
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-cb-has-pours", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
+TEST(checktransaction_tests, bad_cb_empty_scriptsig) {
+    CMutableTransaction mtx = GetValidTransaction();
+    // Make it a coinbase.
+    mtx.vin.resize(1);
+    mtx.vin[0].prevout.SetNull();
+
+    mtx.vpour.resize(0);
+
+    CTransaction tx(mtx);
+    EXPECT_TRUE(tx.IsCoinBase());
+
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-cb-length", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
+TEST(checktransaction_tests, bad_txns_prevout_null) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vin[1].prevout.SetNull();
+
+    CTransaction tx(mtx);
+    EXPECT_FALSE(tx.IsCoinBase());
+
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(10, false, REJECT_INVALID, "bad-txns-prevout-null", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
+TEST(checktransaction_tests, bad_txns_invalid_joinsplit_signature) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.joinSplitSig[0] += 1;
+    CTransaction tx(mtx);
+
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-invalid-joinsplit-signature", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
+TEST(checktransaction_tests, non_canonical_ed25519_signature) {
+    CMutableTransaction mtx = GetValidTransaction();
+
+    // Copied from libsodium/crypto_sign/ed25519/ref10/open.c
+    static const unsigned char L[32] =
+      { 0xed, 0xd3, 0xf5, 0x5c, 0x1a, 0x63, 0x12, 0x58,
+        0xd6, 0x9c, 0xf7, 0xa2, 0xde, 0xf9, 0xde, 0x14,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10 };
+
+    // Add L to S, which starts at mtx.joinSplitSig[32].
+    unsigned int s = 0;
+    for (size_t i = 0; i < 32; i++) {
+        s = mtx.joinSplitSig[32 + i] + L[i] + (s >> 8);
+        mtx.joinSplitSig[32 + i] = s & 0xff;
+    }
+
+    CTransaction tx(mtx);
+
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "non-canonical-ed25519-signature", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
 }
--- 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");
-                    }
-                }
-            }
        }
    }

--- 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
--- 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<unsigned char> raw = ParseHex(raw_script);
--- 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.