Auto merge of #3220 - Eirik0:3209-fix-CheckTransaction, r=str4d

Implement consensus rules about the number of Spend and Output Descriptions in a transaction

Closes #3209.
Part of #3065.

src/main.cpp

@@ -1039,12 +1039,14 @@ bool CheckTransactionWithoutProofVerification(const CTransaction& tx, CValidatio
         }
     }
 
-    // Transactions can contain empty `vin` and `vout` so long as
+    // Transactions containing empty `vin` must have either non-empty
-    // either `vjoinsplit` or `vShieldedSpend` are non-empty.
+    // `vjoinsplit` or non-empty `vShieldedSpend`.
     if (tx.vin.empty() && tx.vjoinsplit.empty() && tx.vShieldedSpend.empty())
         return state.DoS(10, error("CheckTransaction(): vin empty"),
                          REJECT_INVALID, "bad-txns-vin-empty");
-    if (tx.vout.empty() && tx.vjoinsplit.empty() && tx.vShieldedSpend.empty())
+    // Transactions containing empty `vout` must have either non-empty
+    // `vjoinsplit` or non-empty `vShieldedOutput`.
+    if (tx.vout.empty() && tx.vjoinsplit.empty() && tx.vShieldedOutput.empty())
         return state.DoS(10, error("CheckTransaction(): vout empty"),
                          REJECT_INVALID, "bad-txns-vout-empty");
 
@@ -1169,6 +1171,14 @@ bool CheckTransactionWithoutProofVerification(const CTransaction& tx, CValidatio
             return state.DoS(100, error("CheckTransaction(): coinbase has joinsplits"),
                              REJECT_INVALID, "bad-cb-has-joinsplits");
 
+        // A coinbase transaction cannot have spend descriptions or output descriptions
+        if (tx.vShieldedSpend.size() > 0)
+            return state.DoS(100, error("CheckTransaction(): coinbase has spend descriptions"),
+                             REJECT_INVALID, "bad-cb-has-spend-description");
+        if (tx.vShieldedOutput.size() > 0)
+            return state.DoS(100, error("CheckTransaction(): coinbase has output descriptions"),
+                             REJECT_INVALID, "bad-cb-has-output-description");
+
         if (tx.vin[0].scriptSig.size() < 2 || tx.vin[0].scriptSig.size() > 100)
             return state.DoS(100, error("CheckTransaction(): coinbase script size"),
                              REJECT_INVALID, "bad-cb-length");

src/test/transaction_tests.cpp

@@ -408,6 +408,16 @@ void test_simple_sapling_invalidity(uint32_t consensusBranchId, CMutableTransact
         BOOST_CHECK(!CheckTransactionWithoutProofVerification(newTx, state));
         BOOST_CHECK(state.GetRejectReason() == "bad-txns-vin-empty");
     }
+    {
+        CMutableTransaction newTx(tx);
+        CValidationState state;
+
+        newTx.vShieldedSpend.push_back(SpendDescription());
+        newTx.vShieldedSpend[0].nullifier = GetRandHash();
+
+        BOOST_CHECK(!CheckTransactionWithoutProofVerification(newTx, state));
+        BOOST_CHECK(state.GetRejectReason() == "bad-txns-vout-empty");
+    }
     {
         // Ensure that nullifiers are never duplicated within a transaction.
         CMutableTransaction newTx(tx);
@@ -415,6 +425,9 @@ void test_simple_sapling_invalidity(uint32_t consensusBranchId, CMutableTransact
 
         newTx.vShieldedSpend.push_back(SpendDescription());
         newTx.vShieldedSpend[0].nullifier = GetRandHash();
+
+        newTx.vShieldedOutput.push_back(OutputDescription());
+
         newTx.vShieldedSpend.push_back(SpendDescription());
         newTx.vShieldedSpend[1].nullifier = newTx.vShieldedSpend[0].nullifier;
 
@@ -425,6 +438,28 @@ void test_simple_sapling_invalidity(uint32_t consensusBranchId, CMutableTransact
 
         BOOST_CHECK(CheckTransactionWithoutProofVerification(newTx, state));
     }
+    {
+        CMutableTransaction newTx(tx);
+        CValidationState state;
+
+        // Create a coinbase transaction
+        CTxIn vin;
+        vin.prevout = COutPoint();
+        newTx.vin.push_back(vin);
+        CTxOut vout;
+        vout.nValue = 1;
+        newTx.vout.push_back(vout);
+
+        newTx.vShieldedOutput.push_back(OutputDescription());
+
+        BOOST_CHECK(!CheckTransactionWithoutProofVerification(newTx, state));
+        BOOST_CHECK(state.GetRejectReason() == "bad-cb-has-output-description");
+
+        newTx.vShieldedSpend.push_back(SpendDescription());
+
+        BOOST_CHECK(!CheckTransactionWithoutProofVerification(newTx, state));
+        BOOST_CHECK(state.GetRejectReason() == "bad-cb-has-spend-description");
+    }
 }
 
 void test_simple_joinsplit_invalidity(uint32_t consensusBranchId, CMutableTransaction tx)