From 4b4662b06dd0f7aeb69be28e872eb492f9031fec Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Thu, 26 Apr 2018 15:24:59 -0600 Subject: [PATCH 1/2] Make sure transactions have non-empty outputs --- src/main.cpp | 8 +++++--- src/test/transaction_tests.cpp | 13 +++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 634dc177b..cb6cef5bb 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1039,12 +1039,14 @@ bool CheckTransactionWithoutProofVerification(const CTransaction& tx, CValidatio } } - // Transactions can contain empty `vin` and `vout` so long as - // either `vjoinsplit` or `vShieldedSpend` are non-empty. + // Transactions containing empty `vin` must have either 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"); diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 8524e1bc2..30c69882a 100644 --- a/src/test/transaction_tests.cpp +++ b/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; From 812098256f6522b46c78e131f4cee1316cd0daed Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Thu, 26 Apr 2018 15:53:26 -0600 Subject: [PATCH 2/2] Coinbase transactions can not have shielded spend or output --- src/main.cpp | 8 ++++++++ src/test/transaction_tests.cpp | 22 ++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/src/main.cpp b/src/main.cpp index cb6cef5bb..1569ff672 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1171,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"); diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 30c69882a..767f9f75f 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -438,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)