Auto merge of #1341 - ebfull:fix-checktransaction-bug, r=ebfull

Fix CheckTransaction bugs.

Closes #1319.

Does not address the name of `vpub_old` or `vpub_new`.

src/gtest/test_checktransaction.cpp

@@ -171,7 +171,7 @@ TEST(checktransaction_tests, bad_txns_txouttotal_toolarge_outputs) {
 TEST(checktransaction_tests, bad_txns_txouttotal_toolarge_joinsplit) {
     CMutableTransaction mtx = GetValidTransaction();
     mtx.vout[0].nValue = 1;
-    mtx.vjoinsplit[0].vpub_new = MAX_MONEY;
+    mtx.vjoinsplit[0].vpub_old = MAX_MONEY;
 
     CTransaction tx(mtx);
 
@@ -180,6 +180,18 @@ TEST(checktransaction_tests, bad_txns_txouttotal_toolarge_joinsplit) {
     CheckTransactionWithoutProofVerification(tx, state);
 }
 
+TEST(checktransaction_tests, bad_txns_txintotal_toolarge_joinsplit) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vjoinsplit[0].vpub_new = MAX_MONEY - 1;
+    mtx.vjoinsplit[1].vpub_new = MAX_MONEY - 1;
+
+    CTransaction tx(mtx);
+
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-txintotal-toolarge", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
 TEST(checktransaction_tests, bad_txns_vpub_old_negative) {
     CMutableTransaction mtx = GetValidTransaction();
     mtx.vjoinsplit[0].vpub_old = -1;

src/main.cpp

@@ -919,13 +919,30 @@ bool CheckTransactionWithoutProofVerification(const CTransaction& tx, CValidatio
                              REJECT_INVALID, "bad-txns-vpubs-both-nonzero");
         }
 
-        nValueOut += joinsplit.vpub_new;
+        nValueOut += joinsplit.vpub_old;
         if (!MoneyRange(nValueOut)) {
             return state.DoS(100, error("CheckTransaction(): txout total out of range"),
                              REJECT_INVALID, "bad-txns-txouttotal-toolarge");
         }
     }
 
+    // Ensure input values do not exceed MAX_MONEY
+    // We have not resolved the txin values at this stage,
+    // but we do know what the joinsplits claim to add
+    // to the value pool.
+    {
+        CAmount nValueIn = 0;
+        for (std::vector<JSDescription>::const_iterator it(tx.vjoinsplit.begin()); it != tx.vjoinsplit.end(); ++it)
+        {
+            nValueIn += it->vpub_new;
+
+            if (!MoneyRange(it->vpub_new) || !MoneyRange(nValueIn)) {
+                return state.DoS(100, error("CheckTransaction(): txin total out of range"),
+                                 REJECT_INVALID, "bad-txns-txintotal-toolarge");
+            }
+        }
+    }
+
 
     // Check for duplicate inputs
     set<COutPoint> vInOutPoints;

src/test/transaction_tests.cpp

@@ -451,7 +451,7 @@ BOOST_AUTO_TEST_CASE(test_simple_joinsplit_invalidity)
         jsdesc2->vpub_new = (MAX_MONEY / 2) + 10;
 
         BOOST_CHECK(!CheckTransaction(newTx, state));
-        BOOST_CHECK(state.GetRejectReason() == "bad-txns-txouttotal-toolarge");
+        BOOST_CHECK(state.GetRejectReason() == "bad-txns-txintotal-toolarge");
     }
     {
         // Ensure that nullifiers are never duplicated within a transaction.