Reject Ironwood commitments for v5 transactions

CHANGELOG.md

@@ -46,9 +46,11 @@ when building proving/verifying keys).
     introspection for whether a circuit version (or a key's circuit version)
     constrains the `disableCrossAddress` public input.
   - `orchard::circuit::VerifyingKey::circuit_version`
-- `orchard::bundle::CommitmentError`, with its `UnrepresentableFlags` variant,
-  returned by `orchard::Bundle::commitment` when a bundle's flags cannot be
-  represented under the requested `BundlePoolRestrictions`.
+- `orchard::bundle::CommitmentError`, with its `UnrepresentableFlags` and
+  `InvalidTransactionVersion` variants, returned by bundle commitment APIs when
+  a bundle's flags cannot be represented under the requested
+  `BundlePoolRestrictions`, or when `IronwoodNu6_3Onward` is requested for a v5
+  transaction.
 - `orchard::bundle::BatchError` (requires the `circuit` feature), with its
   `RestrictionUnsupportedByKey` variant, returned by
   `orchard::bundle::BatchValidator::add_bundle` when a restricted bundle is added
@@ -160,14 +162,19 @@ when building proving/verifying keys).
     flag byte sets bit 2, and `TxVersion::V6` uses the v6 personalization strings
     and commits the anchor in the authorizing commitment instead of the effects
     commitment. Callers computing transaction IDs or sighashes must pass the
-    restrictions and version matching the transaction; these APIs do not validate
-    that the selected pool/era is consensus-valid for the selected transaction
-    version. `Bundle::commitment` now returns
-    `Result<BundleCommitment, CommitmentError>`, yielding
-    `Err(CommitmentError::UnrepresentableFlags)` if the flags are unrepresentable
-    under those restrictions.
+    restrictions and version matching the transaction; these APIs check only that
+    the combination is representable, not that it is consensus-valid for the
+    transaction version. `Bundle::commitment` now returns
+    `Result<BundleCommitment, CommitmentError>` and
+    `Bundle::<Authorized, V>::authorizing_commitment` returns
+    `Result<BundleAuthorizingCommitment, CommitmentError>`, with
+    `Err(CommitmentError::InvalidTransactionVersion)` for an Ironwood bundle in a
+    v5 transaction and `Err(CommitmentError::UnrepresentableFlags)` (from
+    `commitment` only) for flags the restrictions cannot represent.
   - `orchard::bundle::commitments::{hash_bundle_txid_empty, hash_bundle_auth_empty}`
-    now take a `BundlePoolRestrictions` and a `TxVersion`.
+    now take a `BundlePoolRestrictions` and a `TxVersion`, and return
+    `Result<Blake2bHash, CommitmentError>`, rejecting an Ironwood pool in a v5
+    transaction with `CommitmentError::InvalidTransactionVersion`.
 - Circuit APIs now require explicit circuit versions:
   - `orchard::circuit::Circuit::from_action_context` now takes an
     `OrchardCircuitVersion` instead of implicitly selecting `FixedPostNu6_2`.

src/bundle.rs

@@ -147,7 +147,7 @@ impl BundlePoolRestrictions {
 /// The two formats use different commitment personalization strings and include the bundle's
 /// anchor in different digests: v5 includes the anchor in the transaction-ID digest, while v6
 /// includes it in the authorizing digest. Ironwood bundles exist only in v6 transactions, so
-/// their commitment format ignores this value.
+/// attempting to compute an Ironwood commitment for a v5 transaction returns an error.
 ///
 /// This is independent of the [`BundlePoolRestrictions`] that govern construction: the same
 /// Orchard bundle can be committed under either version, and the caller must pass the one
@@ -659,15 +659,15 @@ impl<T: Authorization, V: Copy + Into<i64>> Bundle<T, V> {
     /// # Errors
     ///
     /// Returns [`CommitmentError::UnrepresentableFlags`] if the flags cannot
-    /// be encoded under the given pool restrictions.
+    /// be encoded under the given pool restrictions, or
+    /// [`CommitmentError::InvalidTransactionVersion`] if `tx_version` is not
+    /// valid for `pool_restrictions`.
     pub fn commitment(
         &self,
         pool_restrictions: BundlePoolRestrictions,
         tx_version: TxVersion,
     ) -> Result<BundleCommitment, CommitmentError> {
-        hash_bundle_txid_data(self, pool_restrictions, tx_version)
-            .map(BundleCommitment)
-            .ok_or(CommitmentError::UnrepresentableFlags)
+        hash_bundle_txid_data(self, pool_restrictions, tx_version).map(BundleCommitment)
     }
 
     /// Returns the transaction binding validating key for this bundle.
@@ -789,6 +789,13 @@ pub enum CommitmentError {
     /// * cross-address transfers are enabled but `pool_restrictions` specifies a post-NU6.3
     ///   Orchard pool (where cross-address transfers are forbidden).
     UnrepresentableFlags,
+    /// The requested transaction version is not valid for the requested pool
+    /// restrictions.
+    ///
+    /// Ironwood bundles exist only in v6 transactions, so
+    /// `BundlePoolRestrictions::IronwoodNu6_3Onward` cannot be committed with
+    /// `TxVersion::V5`.
+    InvalidTransactionVersion,
 }
 
 impl fmt::Display for CommitmentError {
@@ -798,6 +805,10 @@ impl fmt::Display for CommitmentError {
                 f,
                 "bundle flags are not representable according to the requested pool restrictions",
             ),
+            CommitmentError::InvalidTransactionVersion => write!(
+                f,
+                "Ironwood bundles can only be committed in a v6 transaction",
+            ),
         }
     }
 }
@@ -849,12 +860,17 @@ impl<V> Bundle<Authorized, V> {
     /// `tx_version` select the commitment personalization; in a v6 transaction this digest also
     /// includes the bundle anchor bytes (in a v5 transaction they are included by
     /// [`Bundle::commitment`] instead).
+    ///
+    /// # Errors
+    ///
+    /// Returns [`CommitmentError::InvalidTransactionVersion`] if `tx_version`
+    /// is not valid for `pool_restrictions`.
     pub fn authorizing_commitment(
         &self,
         pool_restrictions: BundlePoolRestrictions,
         tx_version: TxVersion,
-    ) -> BundleAuthorizingCommitment {
-        BundleAuthorizingCommitment(hash_bundle_auth_data(self, pool_restrictions, tx_version))
+    ) -> Result<BundleAuthorizingCommitment, CommitmentError> {
+        hash_bundle_auth_data(self, pool_restrictions, tx_version).map(BundleAuthorizingCommitment)
     }
 
     /// Verifies the proof for this bundle.
@@ -1325,15 +1341,24 @@ pub(crate) mod tests {
                 let (pi, ti) = formats[i];
                 let (pj, tj) = formats[j];
                 assert_ne!(
-                    hash_bundle_txid_empty(pi, ti).as_bytes(),
-                    hash_bundle_txid_empty(pj, tj).as_bytes()
+                    hash_bundle_txid_empty(pi, ti).unwrap().as_bytes(),
+                    hash_bundle_txid_empty(pj, tj).unwrap().as_bytes()
                 );
                 assert_ne!(
-                    hash_bundle_auth_empty(pi, ti).as_bytes(),
-                    hash_bundle_auth_empty(pj, tj).as_bytes()
+                    hash_bundle_auth_empty(pi, ti).unwrap().as_bytes(),
+                    hash_bundle_auth_empty(pj, tj).unwrap().as_bytes()
                 );
             }
         }
+
+        assert!(matches!(
+            hash_bundle_txid_empty(BundlePoolRestrictions::IronwoodNu6_3Onward, TxVersion::V5),
+            Err(CommitmentError::InvalidTransactionVersion)
+        ));
+        assert!(matches!(
+            hash_bundle_auth_empty(BundlePoolRestrictions::IronwoodNu6_3Onward, TxVersion::V5),
+            Err(CommitmentError::InvalidTransactionVersion)
+        ));
     }
 
     proptest! {
@@ -1420,6 +1445,29 @@ pub(crate) mod tests {
             ));
         }
 
+        #[test]
+        fn ironwood_rejects_v5_commitment_version(bundle in arb_bundle(3)) {
+            let bundle_i64 = Bundle::from_parts_unchecked(
+                bundle.actions().clone(),
+                *bundle.flags(),
+                0i64,
+                *bundle.anchor(),
+                bundle.authorization().clone(),
+            );
+
+            prop_assert!(matches!(
+                bundle_i64.commitment(BundlePoolRestrictions::IronwoodNu6_3Onward, TxVersion::V5),
+                Err(CommitmentError::InvalidTransactionVersion)
+            ));
+            prop_assert!(matches!(
+                bundle.authorizing_commitment(
+                    BundlePoolRestrictions::IronwoodNu6_3Onward,
+                    TxVersion::V5
+                ),
+                Err(CommitmentError::InvalidTransactionVersion)
+            ));
+        }
+
         /// The anchor bytes are included in the transaction-ID digest for the v5 format and in
         /// the authorizing digest for the v6 format, so changing only the anchor moves exactly
         /// one of the two digests. The v5 and v6 Orchard formats are also domain-separated, so
@@ -1450,8 +1498,8 @@ pub(crate) mod tests {
             ] {
                 let txid_a: [u8; 32] = a.commitment(pool_restrictions, tx).unwrap().into();
                 let txid_b: [u8; 32] = b.commitment(pool_restrictions, tx).unwrap().into();
-                let auth_a = a.authorizing_commitment(pool_restrictions, tx).0;
-                let auth_b = b.authorizing_commitment(pool_restrictions, tx).0;
+                let auth_a = a.authorizing_commitment(pool_restrictions, tx).unwrap().0;
+                let auth_b = b.authorizing_commitment(pool_restrictions, tx).unwrap().0;
                 if anchor_in_txid_digest {
                     prop_assert_ne!(txid_a, txid_b);
                     prop_assert_eq!(auth_a.as_bytes(), auth_b.as_bytes());

src/bundle/commitments.rs

@@ -2,7 +2,9 @@
 
 use blake2b_simd::{Hash as Blake2bHash, Params, State};
 
-use crate::bundle::{Authorization, Authorized, Bundle, BundlePoolRestrictions, TxVersion};
+use crate::bundle::{
+    Authorization, Authorized, Bundle, BundlePoolRestrictions, CommitmentError, TxVersion,
+};
 
 const ZCASH_ORCHARD_V5_HASH_PERSONALIZATION: &[u8; 16] = b"ZTxIdOrchardHash";
 const ZCASH_ORCHARD_V6_HASH_PERSONALIZATION: &[u8; 16] = b"ZTxIdOrchardH_v6";
@@ -70,17 +72,22 @@ enum BundleCommitmentFormat {
 }
 
 impl BundlePoolRestrictions {
-    fn commitment_format(self, tx_version: TxVersion) -> BundleCommitmentFormat {
-        match self {
+    fn commitment_format(
+        self,
+        tx_version: TxVersion,
+    ) -> Result<BundleCommitmentFormat, CommitmentError> {
+        Ok(match self {
             BundlePoolRestrictions::OrchardPreNu6_2
             | BundlePoolRestrictions::OrchardNu6_2Only
             | BundlePoolRestrictions::OrchardNu6_3Onward => match tx_version {
                 TxVersion::V5 => BundleCommitmentFormat::OrchardV5,
                 TxVersion::V6 => BundleCommitmentFormat::OrchardV6,
             },
-            // Ironwood exists only in v6 transactions, so `tx_version` is irrelevant here.
-            BundlePoolRestrictions::IronwoodNu6_3Onward => BundleCommitmentFormat::IronwoodV6,
-        }
+            BundlePoolRestrictions::IronwoodNu6_3Onward => match tx_version {
+                TxVersion::V5 => return Err(CommitmentError::InvalidTransactionVersion),
+                TxVersion::V6 => BundleCommitmentFormat::IronwoodV6,
+            },
+        })
     }
 }
 
@@ -123,15 +130,16 @@ fn hasher(personal: &[u8; 16]) -> State {
 /// personalization string. In the v6 format the anchor is included by
 /// `hash_bundle_auth_data` instead.
 ///
-/// Returns `None` if the bundle flags cannot be encoded in the domain's bundle format.
+/// Returns an error if the bundle flags cannot be encoded in the domain's bundle format,
+/// or if `tx_version` is not valid for `pool_restrictions`.
 ///
 /// [zip244]: https://zips.z.cash/zip-0244
 pub(crate) fn hash_bundle_txid_data<A: Authorization, V: Copy + Into<i64>>(
     bundle: &Bundle<A, V>,
     pool_restrictions: BundlePoolRestrictions,
     tx_version: TxVersion,
-) -> Option<Blake2bHash> {
-    let format = pool_restrictions.commitment_format(tx_version);
+) -> Result<Blake2bHash, CommitmentError> {
+    let format = pool_restrictions.commitment_format(tx_version)?;
     let personalizations = format.personalizations();
     let mut h = hasher(personalizations.bundle);
     let mut ch = hasher(personalizations.actions_compact);
@@ -155,12 +163,15 @@ pub(crate) fn hash_bundle_txid_data<A: Authorization, V: Copy + Into<i64>>(
     h.update(ch.finalize().as_bytes());
     h.update(mh.finalize().as_bytes());
     h.update(nh.finalize().as_bytes());
-    h.update(&[bundle.flags().to_byte(pool_restrictions)?]);
+    h.update(&[bundle
+        .flags()
+        .to_byte(pool_restrictions)
+        .ok_or(CommitmentError::UnrepresentableFlags)?]);
     h.update(&(*bundle.value_balance()).into().to_le_bytes());
     if format.includes_anchor_in_txid_digest() {
         h.update(&bundle.anchor().to_bytes());
     }
-    Some(h.finalize())
+    Ok(h.finalize())
 }
 
 /// Construct the commitment for the absent bundle as defined in
@@ -170,14 +181,14 @@ pub(crate) fn hash_bundle_txid_data<A: Authorization, V: Copy + Into<i64>>(
 pub fn hash_bundle_txid_empty(
     pool_restrictions: BundlePoolRestrictions,
     tx_version: TxVersion,
-) -> Blake2bHash {
-    hasher(
+) -> Result<Blake2bHash, CommitmentError> {
+    Ok(hasher(
         pool_restrictions
-            .commitment_format(tx_version)
+            .commitment_format(tx_version)?
             .personalizations()
             .bundle,
     )
-    .finalize()
+    .finalize())
 }
 
 /// Construct the commitment to the authorizing data of an
@@ -189,8 +200,8 @@ pub(crate) fn hash_bundle_auth_data<V>(
     bundle: &Bundle<Authorized, V>,
     pool_restrictions: BundlePoolRestrictions,
     tx_version: TxVersion,
-) -> Blake2bHash {
-    let format = pool_restrictions.commitment_format(tx_version);
+) -> Result<Blake2bHash, CommitmentError> {
+    let format = pool_restrictions.commitment_format(tx_version)?;
     let mut h = hasher(format.personalizations().auth);
     h.update(bundle.authorization().proof().as_ref());
     for action in bundle.actions().iter() {
@@ -202,7 +213,7 @@ pub(crate) fn hash_bundle_auth_data<V>(
     if format.includes_anchor_in_authorizing_digest() {
         h.update(&bundle.anchor().to_bytes());
     }
-    h.finalize()
+    Ok(h.finalize())
 }
 
 /// Construct the commitment for an absent bundle as defined in
@@ -212,12 +223,12 @@ pub(crate) fn hash_bundle_auth_data<V>(
 pub fn hash_bundle_auth_empty(
     pool_restrictions: BundlePoolRestrictions,
     tx_version: TxVersion,
-) -> Blake2bHash {
-    hasher(
+) -> Result<Blake2bHash, CommitmentError> {
+    Ok(hasher(
         pool_restrictions
-            .commitment_format(tx_version)
+            .commitment_format(tx_version)?
             .personalizations()
             .auth,
     )
-    .finalize()
+    .finalize())
 }