Add rho derivation check in verify_issue_bundle

src/issuance.rs

@@ -25,16 +25,16 @@ use crate::{
     bundle::commitments::{hash_issue_bundle_auth_data, hash_issue_bundle_txid_data},
     constants::reference_keys::ReferenceKeys,
     keys::{IssuanceAuthorizingKey, IssuanceValidatingKey},
-    note::{AssetBase, Nullifier, Rho},
+    note::{rho_for_issuance_note, AssetBase, Nullifier, Rho},
     value::NoteValue,
     Address, Note,
 };
 
 use Error::{
-    AssetBaseCannotBeIdentityPoint, CannotBeFirstIssuance, IssueActionNotFound,
-    IssueActionPreviouslyFinalizedAssetBase, IssueActionWithoutNoteNotFinalized,
-    IssueBundleIkMismatchAssetBase, IssueBundleInvalidSignature,
-    MissingReferenceNoteOnFirstIssuance, ValueOverflow,
+    AssetBaseCannotBeIdentityPoint, CannotBeFirstIssuance, IncorrectRhoDerivation,
+    IssueActionNotFound, IssueActionPreviouslyFinalizedAssetBase,
+    IssueActionWithoutNoteNotFinalized, IssueBundleIkMismatchAssetBase,
+    IssueBundleInvalidSignature, MissingReferenceNoteOnFirstIssuance, ValueOverflow,
 };
 
 /// Checks if a given note is a reference note.
@@ -599,6 +599,9 @@ impl IssueBundle<Signed> {
 ///   - Ensures the total supply value does not overflow when adding the new amount to the existing supply.
 ///   - Verifies that the `AssetBase` has not already been finalized.
 ///   - Requires a reference note for the *first issuance* of an asset; subsequent issuance may omit it.
+/// - **Rho computation**:
+///   - Ensures that the `rho` value of each issuance note is correctly computed from the given
+///     `first_nullifier`.
 ///
 /// # Arguments
 ///
@@ -607,6 +610,8 @@ impl IssueBundle<Signed> {
 /// * `get_global_asset_state`: A closure that takes a reference to an [`AssetBase`] and returns an
 ///   [`Option<AssetRecord>`], representing the current state of the asset from a global store
 ///   of previously issued assets.
+/// * `first_nullifier`: A reference to a [`Nullifier`] that is used to compute the `rho` value of
+///   each issuance note.
 ///
 /// # Returns
 ///
@@ -622,22 +627,33 @@ impl IssueBundle<Signed> {
 ///   already been finalized.
 /// * `MissingReferenceNoteOnFirstIssuance`: No reference note is provided for the first
 ///   issuance of a new asset.
+/// * `IncorrectRhoDerivation`: If the `rho` value of any issuance note is not correctly derived
+///   from the `first_nullifier`.
 /// * **Other Errors**: Any additional errors returned by the `IssueAction::verify` method are
 ///   propagated
 pub fn verify_issue_bundle(
     bundle: &IssueBundle<Signed>,
     sighash: [u8; 32],
     get_global_records: impl Fn(&AssetBase) -> Option<AssetRecord>,
+    first_nullifier: &Nullifier,
 ) -> Result<BTreeMap<AssetBase, AssetRecord>, Error> {
     bundle
         .ik()
         .verify(&sighash, bundle.authorization().signature())
         .map_err(|_| IssueBundleInvalidSignature)?;
 
-    bundle
-        .actions()
-        .iter()
-        .try_fold(BTreeMap::new(), |mut new_records, action| {
+    bundle.actions().iter().enumerate().try_fold(
+        BTreeMap::new(),
+        |mut new_records, (index_action, action)| {
+            // Check rho derivation for each note.
+            for (index_note, note) in action.notes.iter().enumerate() {
+                let expected_rho =
+                    rho_for_issuance_note(first_nullifier, index_action as u32, index_note as u32);
+                if note.rho() != expected_rho {
+                    return Err(IncorrectRhoDerivation);
+                }
+            }
+
             let (asset, amount) = action.verify(bundle.ik())?;
 
             let is_finalized = action.is_finalized();
@@ -670,7 +686,8 @@ pub fn verify_issue_bundle(
             new_records.insert(asset, new_asset_record);
 
             Ok(new_records)
-        })
+        },
+    )
 }
 
 /// Errors produced during the issuance process
@@ -692,6 +709,8 @@ pub enum Error {
     IssueBundleInvalidSignature,
     /// The provided `AssetBase` has been previously finalized.
     IssueActionPreviouslyFinalizedAssetBase,
+    /// The rho value of an issuance note is not correctly derived from the first nullifier.
+    IncorrectRhoDerivation,
 
     /// Overflow error occurred while calculating the value of the asset
     ValueOverflow,
@@ -736,6 +755,9 @@ impl fmt::Display for Error {
             IssueActionPreviouslyFinalizedAssetBase => {
                 write!(f, "the provided `AssetBase` has been previously finalized")
             }
+            IncorrectRhoDerivation => {
+                write!(f, "incorrect rho value")
+            }
             ValueOverflow => {
                 write!(
                     f,
@@ -759,7 +781,7 @@ mod tests {
         builder::{Builder, BundleType},
         circuit::ProvingKey,
         issuance::Error::{
-            IssueActionNotFound, IssueActionPreviouslyFinalizedAssetBase,
+            IncorrectRhoDerivation, IssueActionNotFound, IssueActionPreviouslyFinalizedAssetBase,
             IssueBundleIkMismatchAssetBase, IssueBundleInvalidSignature,
         },
         issuance::{
@@ -1232,7 +1254,8 @@ mod tests {
             .sign(&isk)
             .unwrap();
 
-        let issued_assets = verify_issue_bundle(&signed, sighash, |_| None).unwrap();
+        let issued_assets =
+            verify_issue_bundle(&signed, sighash, |_| None, &first_nullifier).unwrap();
 
         let first_note = *signed.actions().first().notes().first().unwrap();
         assert_eq!(
@@ -1277,7 +1300,8 @@ mod tests {
             .sign(&isk)
             .unwrap();
 
-        let issued_assets = verify_issue_bundle(&signed, sighash, |_| None).unwrap();
+        let issued_assets =
+            verify_issue_bundle(&signed, sighash, |_| None, &first_nullifier).unwrap();
 
         let first_note = *signed.actions().first().notes().first().unwrap();
         assert_eq!(
@@ -1362,7 +1386,8 @@ mod tests {
             .sign(&isk)
             .unwrap();
 
-        let issued_assets = verify_issue_bundle(&signed, sighash, |_| None).unwrap();
+        let issued_assets =
+            verify_issue_bundle(&signed, sighash, |_| None, &first_nullifier).unwrap();
 
         assert_eq!(issued_assets.keys().len(), 3);
 
@@ -1396,6 +1421,44 @@ mod tests {
         );
     }
 
+    #[test]
+    fn issue_bundle_verify_fail_incorrect_rho_derivation() {
+        let TestParams {
+            mut rng,
+            isk,
+            ik,
+            recipient,
+            sighash,
+            first_nullifier,
+        } = setup_params();
+
+        let asset_desc_hash =
+            compute_asset_desc_hash(&NonEmpty::from_slice(b"asset desc").unwrap());
+
+        let (bundle, _) = IssueBundle::new(
+            ik.clone(),
+            asset_desc_hash,
+            Some(IssueInfo {
+                recipient,
+                value: NoteValue::from_raw(5),
+            }),
+            true,
+            rng,
+        );
+
+        let signed = bundle
+            .update_rho(&first_nullifier)
+            .prepare(sighash)
+            .sign(&isk)
+            .unwrap();
+
+        // Verify that `verify_issue_bundle` returns an error if `first_nullifier` is incorrect.
+        assert_eq!(
+            verify_issue_bundle(&signed, sighash, |_| None, &Nullifier::dummy(&mut rng)),
+            Err(IncorrectRhoDerivation)
+        );
+    }
+
     #[test]
     fn issue_bundle_verify_fail_previously_finalized() {
         let TestParams {
@@ -1447,8 +1510,13 @@ mod tests {
         .collect::<BTreeMap<_, _>>();
 
         assert_eq!(
-            verify_issue_bundle(&signed, sighash, |asset| issued_assets.get(asset).copied())
-                .unwrap_err(),
+            verify_issue_bundle(
+                &signed,
+                sighash,
+                |asset| issued_assets.get(asset).copied(),
+                &first_nullifier
+            )
+            .unwrap_err(),
             IssueActionPreviouslyFinalizedAssetBase
         );
     }
@@ -1495,7 +1563,7 @@ mod tests {
         });
 
         assert_eq!(
-            verify_issue_bundle(&signed, sighash, |_| None).unwrap_err(),
+            verify_issue_bundle(&signed, sighash, |_| None, &first_nullifier).unwrap_err(),
             IssueBundleInvalidSignature
         );
     }
@@ -1530,7 +1598,7 @@ mod tests {
             .unwrap();
 
         assert_eq!(
-            verify_issue_bundle(&signed, random_sighash, |_| None).unwrap_err(),
+            verify_issue_bundle(&signed, random_sighash, |_| None, &first_nullifier).unwrap_err(),
             IssueBundleInvalidSignature
         );
     }
@@ -1571,14 +1639,14 @@ mod tests {
                 signed.ik(),
                 &compute_asset_desc_hash(&NonEmpty::from_slice(b"zsa_asset").unwrap()),
             ),
-            Rho::zero(),
+            rho_for_issuance_note(&first_nullifier, 0, 2),
             &mut rng,
         );
 
         signed.actions.first_mut().notes.push(note);
 
         assert_eq!(
-            verify_issue_bundle(&signed, sighash, |_| None).unwrap_err(),
+            verify_issue_bundle(&signed, sighash, |_| None, &first_nullifier).unwrap_err(),
             IssueBundleIkMismatchAssetBase
         );
     }
@@ -1621,14 +1689,14 @@ mod tests {
             recipient,
             NoteValue::from_raw(55),
             AssetBase::derive(&incorrect_ik, &asset_desc_hash),
-            Rho::zero(),
+            rho_for_issuance_note(&first_nullifier, 0, 0),
             &mut rng,
         );
 
         signed.actions.first_mut().notes = vec![note];
 
         assert_eq!(
-            verify_issue_bundle(&signed, sighash, |_| None).unwrap_err(),
+            verify_issue_bundle(&signed, sighash, |_| None, &first_nullifier).unwrap_err(),
             IssueBundleIkMismatchAssetBase
         );
     }

tests/issuance_global_state.rs

@@ -230,7 +230,13 @@ fn issue_bundle_verify_with_global_state() {
     ]);
 
     global_state.extend(
-        verify_issue_bundle(&bundle1, sighash, |asset| global_state.get(asset).cloned()).unwrap(),
+        verify_issue_bundle(
+            &bundle1,
+            sighash,
+            |asset| global_state.get(asset).cloned(),
+            &params.first_nullifier,
+        )
+        .unwrap(),
     );
     assert_eq!(global_state, expected_global_state1);
 
@@ -251,7 +257,13 @@ fn issue_bundle_verify_with_global_state() {
     ]);
 
     global_state.extend(
-        verify_issue_bundle(&bundle2, sighash, |asset| global_state.get(asset).cloned()).unwrap(),
+        verify_issue_bundle(
+            &bundle2,
+            sighash,
+            |asset| global_state.get(asset).cloned(),
+            &params.first_nullifier,
+        )
+        .unwrap(),
     );
     assert_eq!(global_state, expected_global_state2);
 
@@ -268,8 +280,13 @@ fn issue_bundle_verify_with_global_state() {
     let expected_global_state3 = expected_global_state2;
 
     assert_eq!(
-        verify_issue_bundle(&bundle3, sighash, |asset| global_state.get(asset).cloned())
-            .unwrap_err(),
+        verify_issue_bundle(
+            &bundle3,
+            sighash,
+            |asset| global_state.get(asset).cloned(),
+            &params.first_nullifier
+        )
+        .unwrap_err(),
         IssueActionPreviouslyFinalizedAssetBase,
     );
     assert_eq!(global_state, expected_global_state3);
@@ -287,8 +304,13 @@ fn issue_bundle_verify_with_global_state() {
     let expected_global_state4 = expected_global_state3;
 
     assert_eq!(
-        verify_issue_bundle(&bundle4, sighash, |asset| global_state.get(asset).cloned())
-            .unwrap_err(),
+        verify_issue_bundle(
+            &bundle4,
+            sighash,
+            |asset| global_state.get(asset).cloned(),
+            &params.first_nullifier
+        )
+        .unwrap_err(),
         MissingReferenceNoteOnFirstIssuance,
     );
     assert_eq!(global_state, expected_global_state4);
@@ -306,8 +328,13 @@ fn issue_bundle_verify_with_global_state() {
     let expected_global_state5 = expected_global_state4;
 
     assert_eq!(
-        verify_issue_bundle(&bundle5, sighash, |asset| global_state.get(asset).cloned())
-            .unwrap_err(),
+        verify_issue_bundle(
+            &bundle5,
+            sighash,
+            |asset| global_state.get(asset).cloned(),
+            &params.first_nullifier
+        )
+        .unwrap_err(),
         ValueOverflow,
     );
     assert_eq!(global_state, expected_global_state5);
@@ -330,7 +357,13 @@ fn issue_bundle_verify_with_global_state() {
     ]);
 
     global_state.extend(
-        verify_issue_bundle(&bundle6, sighash, |asset| global_state.get(asset).cloned()).unwrap(),
+        verify_issue_bundle(
+            &bundle6,
+            sighash,
+            |asset| global_state.get(asset).cloned(),
+            &params.first_nullifier,
+        )
+        .unwrap(),
     );
     assert_eq!(global_state, expected_global_state6);
 }

tests/zsa.rs

@@ -186,7 +186,13 @@ fn issue_zsa_notes(
         AssetBase::derive(&keys.ik().clone(), &asset_desc_hash),
     );
 
-    assert!(verify_issue_bundle(&issue_bundle, issue_bundle.commitment().into(), |_| None).is_ok());
+    assert!(verify_issue_bundle(
+        &issue_bundle,
+        issue_bundle.commitment().into(),
+        |_| None,
+        first_nullifier
+    )
+    .is_ok());
 
     (*reference_note, *note1, *note2)
 }