Update `transaction::check::coinbase_tx_no_joinsplit_or_spend` to validate V5 coinbase transactions with Orchard shielded data (#2236)

* Add a `Transaction::orchard_shielded_data` getter

Allows accessing the Orchard shielded data if it is present in the
transaction, regardless of the transaction version.

* Refactor `orchard_nullifiers` to use new getter

Allows making the method more concise.

* Add `CoinbaseHasEnableSpendsOrchard` error variant

Used when the validation rule is not met.

* Implement `enableSpendsOrchard` in coinbase check

The flag must not be set for the coinbase transaction.

* Refactor `Transaction::orchard_*` getters

Use the fact that `Option<T>` implements `Iterator<T>` to simplify the
code and remove the need for boxing the iterators.

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

zebra-chain/src/transaction.rs

@@ -395,47 +395,39 @@ impl Transaction {
 
     // orchard
 
-    /// Iterate over the [`orchard::Action`]s in this transaction, if there are any.
-    pub fn orchard_actions(&self) -> Box<dyn Iterator<Item = &orchard::Action> + '_> {
+    /// Access the [`orchard::ShieldedData`] in this transaction, if there are any,
+    /// regardless of version.
+    pub fn orchard_shielded_data(&self) -> Option<&orchard::ShieldedData> {
         match self {
-            // Actions
+            // Maybe Orchard shielded data
             Transaction::V5 {
-                orchard_shielded_data: Some(orchard_shielded_data),
+                orchard_shielded_data,
                 ..
-            } => Box::new(orchard_shielded_data.actions()),
+            } => orchard_shielded_data.as_ref(),
 
-            // No Actions
+            // No Orchard shielded data
             Transaction::V1 { .. }
             | Transaction::V2 { .. }
             | Transaction::V3 { .. }
-            | Transaction::V4 { .. }
-            | Transaction::V5 {
-                orchard_shielded_data: None,
-                ..
-            } => Box::new(std::iter::empty()),
+            | Transaction::V4 { .. } => None,
         }
     }
 
-    /// Access the [`orchard::Nullifier`]s in this transaction, regardless of version.
-    pub fn orchard_nullifiers(&self) -> Box<dyn Iterator<Item = &orchard::Nullifier> + '_> {
-        // This function returns a boxed iterator because the different
-        // transaction variants can have different iterator types
-        match self {
-            // Actions
-            Transaction::V5 {
-                orchard_shielded_data: Some(orchard_shielded_data),
-                ..
-            } => Box::new(orchard_shielded_data.nullifiers()),
+    /// Iterate over the [`orchard::Action`]s in this transaction, if there are any,
+    /// regardless of version.
+    pub fn orchard_actions(&self) -> impl Iterator<Item = &orchard::Action> {
+        self.orchard_shielded_data()
+            .into_iter()
+            .map(orchard::ShieldedData::actions)
+            .flatten()
+    }
 
-            // No Actions
-            Transaction::V1 { .. }
-            | Transaction::V2 { .. }
-            | Transaction::V3 { .. }
-            | Transaction::V4 { .. }
-            | Transaction::V5 {
-                orchard_shielded_data: None,
-                ..
-            } => Box::new(std::iter::empty()),
-        }
+    /// Access the [`orchard::Nullifier`]s in this transaction, if there are any,
+    /// regardless of version.
+    pub fn orchard_nullifiers(&self) -> impl Iterator<Item = &orchard::Nullifier> {
+        self.orchard_shielded_data()
+            .into_iter()
+            .map(orchard::ShieldedData::nullifiers)
+            .flatten()
     }
 }

zebra-consensus/src/error.rs

@@ -38,6 +38,9 @@ pub enum TransactionError {
     #[error("coinbase transaction MUST NOT have any Output descriptions pre-Heartwood")]
     CoinbaseHasOutputPreHeartwood,
 
+    #[error("coinbase transaction MUST NOT have the EnableSpendsOrchard flag set")]
+    CoinbaseHasEnableSpendsOrchard,
+
     #[error("coinbase transaction failed subsidy validation")]
     Subsidy(#[from] SubsidyError),
 

zebra-consensus/src/transaction/check.rs

@@ -3,6 +3,7 @@
 //! Code in this file can freely assume that no pre-V4 transactions are present.
 
 use zebra_chain::{
+    orchard::Flags,
     sapling::{AnchorVariant, Output, PerSpendAnchor, ShieldedData, Spend},
     transaction::Transaction,
 };
@@ -84,8 +85,11 @@ pub fn coinbase_tx_no_prevout_joinsplit_spend(tx: &Transaction) -> Result<(), Tr
             return Err(TransactionError::CoinbaseHasSpend);
         }
 
-        // TODO: Orchard validation (#1980)
-        // In a version 5 coinbase transaction, the enableSpendsOrchard flag MUST be 0.
+        if let Some(orchard_shielded_data) = tx.orchard_shielded_data() {
+            if orchard_shielded_data.flags.contains(Flags::ENABLE_SPENDS) {
+                return Err(TransactionError::CoinbaseHasEnableSpendsOrchard);
+            }
+        }
     }
 
     Ok(())