Further test new transaction consensus rules (#2246)

* Add a `at_least_one!` macro for testing

Similar to the `vec!` macro, but doesn't allow creating an empty list.

* Test if `has_inputs_and_outputs` considers actions

Create a dummy transaction with no inputs and no outputs, and add a
dummy Orchard action to it. The `check::has_inputs_and_outputs`
should succeed, because the consensus rule considers having Orchard
actions as having inputs and/or outputs.

* Refactor to create helper function

Move the code to create a fake Orchard shielded data instance to a
helper function in `zebra_chain::transaction::arbitrary`, so that other
tests can also use it.

* Test coinbase V5 transaction with enable spends

A V5 coinbase transaction that has Orchard shielded data MUST NOT have
the enable spends flag set.

* Test if coinbase without enable spends is valid

A coinbase transaction with Orchard shielded data and without the enable
spends flag set should be valid.

* Add a security comment about the `at_least_one!` macro

This macro must not be used outside tests, because it allows memory denial
of service.

Co-authored-by: teor <teor@riseup.net>

zebra-chain/src/serialization/constraint.rs

@@ -178,3 +178,36 @@ impl<T> AtLeastOne<T> {
 
 // TODO: consider implementing `push`, `append`, and `Extend`,
 // because adding elements can't break the constraint.
+
+/// Create an initialized [`AtLeastOne`] instance.
+///
+/// This macro is similar to the [`vec!`][`std::vec!`] macro, but doesn't support creating an empty
+/// `AtLeastOne` instance.
+///
+/// # Security
+///
+/// This macro must only be used in tests, because it skips the `TrustedPreallocate` memory
+/// denial of service checks.
+#[cfg(any(test, feature = "proptest-impl"))]
+#[macro_export]
+macro_rules! at_least_one {
+    ($element:expr; 0) => (
+        compile_error!("At least one element needed to create an `AtLeastOne<T>`")
+    );
+
+    ($element:expr; $count:expr) => (
+        {
+            <Vec<_> as std::convert::TryInto<$crate::serialization::AtLeastOne<_>>>::try_into(
+                vec![$element; $expr],
+            ).expect("at least one element in `AtLeastOne<_>`")
+        }
+    );
+
+    ($($element:expr),+ $(,)?) => (
+        {
+            <Vec<_> as std::convert::TryInto<$crate::serialization::AtLeastOne<_>>>::try_into(
+                vec![$($element),*],
+            ).expect("at least one element in `AtLeastOne<_>`")
+        }
+    );
+}

zebra-chain/src/transaction/arbitrary.rs

@@ -1,20 +1,23 @@
 //! Arbitrary data generation for transaction proptests
 
-use std::{convert::TryInto, sync::Arc};
+use std::{
+    convert::{TryFrom, TryInto},
+    sync::Arc,
+};
 
 use chrono::{TimeZone, Utc};
 use proptest::{arbitrary::any, array, collection::vec, option, prelude::*};
 
 use crate::{
     amount::Amount,
-    block, orchard,
+    at_least_one, block, orchard,
     parameters::{Network, NetworkUpgrade},
     primitives::{
         redpallas::{Binding, Signature},
         Bctv14Proof, Groth16Proof, Halo2Proof, ZkSnarkProof,
     },
     sapling,
-    serialization::ZcashDeserializeInto,
+    serialization::{ZcashDeserialize, ZcashDeserializeInto},
     sprout, transparent, LedgerState,
 };
 
@@ -573,3 +576,56 @@ pub fn fake_v5_transactions_for_network<'b>(
             .map(Transaction::from)
     })
 }
+
+/// Modify a V5 transaction to insert fake Orchard shielded data.
+///
+/// Creates a fake instance of [`orchard::ShieldedData`] with one fake action. Note that both the
+/// action and the shielded data are invalid and shouldn't be used in tests that require them to be
+/// valid.
+///
+/// A mutable reference to the inserted shielded data is returned, so that the caller can further
+/// customize it if required.
+///
+/// # Panics
+///
+/// Panics if the transaction to be modified is not V5.
+pub fn insert_fake_orchard_shielded_data(
+    transaction: &mut Transaction,
+) -> &mut orchard::ShieldedData {
+    // Create a dummy action, it doesn't need to be valid
+    let dummy_action_bytes = [0u8; 820];
+    let mut dummy_action_bytes_reader = &dummy_action_bytes[..];
+    let dummy_action = orchard::Action::zcash_deserialize(&mut dummy_action_bytes_reader)
+        .expect("Dummy action should deserialize");
+
+    // Pair the dummy action with a fake signature
+    let dummy_authorized_action = orchard::AuthorizedAction {
+        action: dummy_action,
+        spend_auth_sig: Signature::from([0u8; 64]),
+    };
+
+    // Place the dummy action inside the Orchard shielded data
+    let dummy_shielded_data = orchard::ShieldedData {
+        flags: orchard::Flags::empty(),
+        value_balance: Amount::try_from(0).expect("invalid transaction amount"),
+        shared_anchor: orchard::tree::Root::default(),
+        proof: Halo2Proof(vec![]),
+        actions: at_least_one![dummy_authorized_action],
+        binding_sig: Signature::from([0u8; 64]),
+    };
+
+    // Replace the shielded data in the transaction
+    match transaction {
+        Transaction::V5 {
+            orchard_shielded_data,
+            ..
+        } => {
+            *orchard_shielded_data = Some(dummy_shielded_data);
+
+            orchard_shielded_data
+                .as_mut()
+                .expect("shielded data was just inserted")
+        }
+        _ => panic!("Fake V5 transaction is not V5"),
+    }
+}

zebra-consensus/src/transaction/tests.rs

@@ -1,6 +1,10 @@
 use zebra_chain::{
+    orchard,
     parameters::Network,
-    transaction::{arbitrary::fake_v5_transactions_for_network, Transaction},
+    transaction::{
+        arbitrary::{fake_v5_transactions_for_network, insert_fake_orchard_shielded_data},
+        Transaction,
+    },
 };
 
 use super::check;
@@ -51,6 +55,32 @@ fn v5_fake_transactions() -> Result<(), Report> {
     Ok(())
 }
 
+#[test]
+fn fake_v5_transaction_with_orchard_actions_has_inputs_and_outputs() {
+    // Find a transaction with no inputs or outputs to use as base
+    let mut transaction = fake_v5_transactions_for_network(
+        Network::Mainnet,
+        zebra_test::vectors::MAINNET_BLOCKS.iter(),
+    )
+    .rev()
+    .find(|transaction| {
+        transaction.inputs().is_empty()
+            && transaction.outputs().is_empty()
+            && transaction.sapling_spends_per_anchor().next().is_none()
+            && transaction.sapling_outputs().next().is_none()
+            && transaction.joinsplit_count() == 0
+    })
+    .expect("At least one fake V5 transaction with no inputs and no outputs");
+
+    // Insert fake Orchard shielded data to the transaction, which has at least one action (this is
+    // guaranteed structurally by `orchard::ShieldedData`)
+    insert_fake_orchard_shielded_data(&mut transaction);
+
+    // If a transaction has at least one Orchard shielded action, it should be considered to have
+    // inputs and/or outputs
+    assert!(check::has_inputs_and_outputs(&transaction).is_ok());
+}
+
 #[test]
 fn v5_transaction_with_no_inputs_fails_validation() {
     let transaction = fake_v5_transactions_for_network(
@@ -95,3 +125,38 @@ fn v5_transaction_with_no_outputs_fails_validation() {
         Err(TransactionError::NoOutputs)
     );
 }
+
+#[test]
+fn v5_coinbase_transaction_without_enable_spends_flag_passes_validation() {
+    let mut transaction = fake_v5_transactions_for_network(
+        Network::Mainnet,
+        zebra_test::vectors::MAINNET_BLOCKS.iter(),
+    )
+    .rev()
+    .find(|transaction| transaction.is_coinbase())
+    .expect("At least one fake V5 coinbase transaction in the test vectors");
+
+    insert_fake_orchard_shielded_data(&mut transaction);
+
+    assert!(check::coinbase_tx_no_prevout_joinsplit_spend(&transaction).is_ok(),);
+}
+
+#[test]
+fn v5_coinbase_transaction_with_enable_spends_flag_fails_validation() {
+    let mut transaction = fake_v5_transactions_for_network(
+        Network::Mainnet,
+        zebra_test::vectors::MAINNET_BLOCKS.iter(),
+    )
+    .rev()
+    .find(|transaction| transaction.is_coinbase())
+    .expect("At least one fake V5 coinbase transaction in the test vectors");
+
+    let shielded_data = insert_fake_orchard_shielded_data(&mut transaction);
+
+    shielded_data.flags = orchard::Flags::ENABLE_SPENDS;
+
+    assert_eq!(
+        check::coinbase_tx_no_prevout_joinsplit_spend(&transaction),
+        Err(TransactionError::CoinbaseHasEnableSpendsOrchard)
+    );
+}