From 2c93e4051fc468887976499dff6e6ce09e3d621e Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Tue, 21 Jan 2025 13:00:08 +0100 Subject: [PATCH 01/28] Modify verify_issue_bundle function to use get_asset_state instead of a finalization set, fix tests accordingly --- src/issuance.rs | 419 +++++++++++---------------------------------- src/supply_info.rs | 181 +------------------- tests/zsa.rs | 8 +- 3 files changed, 105 insertions(+), 503 deletions(-) diff --git a/src/issuance.rs b/src/issuance.rs index 78832fe7c..63fe5cc37 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -4,7 +4,7 @@ use group::Group; use k256::schnorr; use nonempty::NonEmpty; use rand::RngCore; -use std::collections::HashSet; +use std::collections::HashMap; use std::fmt; use crate::bundle::commitments::{hash_issue_bundle_auth_data, hash_issue_bundle_txid_data}; @@ -21,7 +21,7 @@ use crate::note::{AssetBase, Nullifier, Rho}; use crate::value::NoteValue; use crate::{Address, Note}; -use crate::supply_info::{AssetSupply, SupplyInfo}; +use crate::supply_info::AssetSupply; /// Checks if a given note is a reference note. /// @@ -555,70 +555,88 @@ impl IssueBundle { /// Validation for Orchard IssueBundles /// -/// A set of previously finalized asset types must be provided in `finalized` argument. -/// /// The following checks are performed: -/// * For the `IssueBundle`: -/// * the Signature on top of the provided `sighash` verifies correctly. -/// * For each `IssueAction`: -/// * Asset description size is correct. -/// * `AssetBase` for the `IssueAction` has not been previously finalized. -/// * For each `Note` inside an `IssueAction`: -/// * All notes have the same, correct `AssetBase`. +/// - **IssueBundle Verification**: +/// - The signature on the provided `sighash` is verified correctly. +/// - **IssueAction Verification**: +/// - The asset description size is correct. +/// - The `verify_supply` method checks pass, +/// - The new amount does not overflow the previous total supply value. +/// - The `AssetBase` for the `IssueAction` has not been previously finalized. +/// +/// # Arguments +/// +/// - `bundle` - A reference to the `IssueBundle` to be validated. +/// - `sighash` - A 32-byte array representing the sighash used to verify the bundle's signature. +/// - `get_asset_state` - A closure that takes a reference to an `AssetBase` and returns an +/// `Option`, representing the current state of the asset in the global store of +/// previously issued assets. /// /// # Returns /// -/// A Result containing a SupplyInfo struct, which stores supply information in a HashMap. -/// The HashMap `assets` uses AssetBase as the key, and an AssetSupply struct as the -/// value. The AssetSupply contains a NoteValue (representing the total value of all notes for -/// the asset), a bool indicating whether the asset is finalized and a Note (the reference note -/// for this asset). +/// A `Result` with a `HashMap` on success, which contains new values for updated or newly added +/// items in the global state. Each key is an `AssetBase`, and the corresponding value is a new +/// (updated) `AssetSupply`. /// /// # Errors /// -/// * `IssueBundleInvalidSignature`: This error occurs if the signature verification -/// for the provided `sighash` fails. -/// * `WrongAssetDescSize`: This error is raised if the asset description size for any -/// asset in the bundle is incorrect. -/// * `IssueActionPreviouslyFinalizedAssetBase`: This error occurs if the asset has already been -/// finalized (inserted into the `finalized` collection). -/// * `ValueOverflow`: This error occurs if an overflow happens during the calculation of -/// the value sum for the notes in the asset. -/// * `IssueBundleIkMismatchAssetBase`: This error is raised if the `AssetBase` derived from -/// the `ik` (Issuance Validating Key) and the `asset_desc` (Asset Description) does not match -/// the expected `AssetBase`. +/// - `IssueBundleInvalidSignature`: Occurs if the signature verification for the provided `sighash` +/// fails. +/// - `WrongAssetDescSize`: Raised if the asset description size for any asset in the bundle is +/// incorrect. +/// - `ValueOverflow`: Occurs if an overflow happens during the calculation of the total value for +/// the notes. +/// - `IssueActionPreviouslyFinalizedAssetBase`: Occurs if the asset has already been finalized. +/// - **Other Errors**: Any additional errors returned by the `verify_supply` method of `IssueAction` +/// will also be propagated. pub fn verify_issue_bundle( bundle: &IssueBundle, sighash: [u8; 32], - finalized: &HashSet, // The finalization set. -) -> Result { + // FIXME: consider using AssetStateStore trait with get_asset method instead + get_asset_state: impl Fn(&AssetBase) -> Option, +) -> Result, Error> { bundle - .ik - .verify(&sighash, &bundle.authorization.signature) + .ik() + .verify(&sighash, bundle.authorization().signature()) .map_err(|_| IssueBundleInvalidSignature)?; - let supply_info = + let verified_asset_states = bundle .actions() .iter() - .try_fold(SupplyInfo::new(), |mut supply_info, action| { + .try_fold(HashMap::new(), |mut verified_asset_states, action| { if !is_asset_desc_of_valid_size(action.asset_desc()) { return Err(WrongAssetDescSize); } - let (asset, supply) = action.verify_supply(bundle.ik())?; + let (asset, action_asset_state) = action.verify_supply(bundle.ik())?; - // Fail if the asset was previously finalized. - if finalized.contains(&asset) { - return Err(IssueActionPreviouslyFinalizedAssetBase(asset)); - } + let old_asset_state = verified_asset_states + .get(&asset) + .cloned() + .or_else(|| get_asset_state(&asset)) + .unwrap_or_default(); + + let amount = + (old_asset_state.amount + action_asset_state.amount).ok_or(ValueOverflow)?; - supply_info.add_supply(asset, supply)?; + let is_finalized = (!old_asset_state.is_finalized) + .then_some(action_asset_state.is_finalized) + .ok_or(IssueActionPreviouslyFinalizedAssetBase(asset))?; - Ok(supply_info) + let reference_note = old_asset_state + .reference_note + .or(action_asset_state.reference_note); + + verified_asset_states.insert( + asset, + AssetSupply::new(amount, is_finalized, reference_note), + ); + + Ok(verified_asset_states) })?; - Ok(supply_info) + Ok(verified_asset_states) } /// Errors produced during the issuance process @@ -688,13 +706,14 @@ impl fmt::Display for Error { } } +// FIXME: Add more tests for issued_assets (i.e. "global state") change after verify_issue_bundle +// is called: 1) check for expected output, 2) check for processing of existing assets etc. #[cfg(test)] mod tests { use super::{AssetSupply, IssueBundle, IssueInfo}; use crate::issuance::Error::{ - AssetBaseCannotBeIdentityPoint, IssueActionNotFound, - IssueActionPreviouslyFinalizedAssetBase, IssueBundleIkMismatchAssetBase, - IssueBundleInvalidSignature, WrongAssetDescSize, + AssetBaseCannotBeIdentityPoint, IssueActionPreviouslyFinalizedAssetBase, + IssueBundleIkMismatchAssetBase, IssueBundleInvalidSignature, WrongAssetDescSize, }; use crate::issuance::{ is_reference_note, verify_issue_bundle, IssueAction, Signed, Unauthorized, @@ -710,7 +729,7 @@ mod tests { use pasta_curves::pallas::{Point, Scalar}; use rand::rngs::OsRng; use rand::RngCore; - use std::collections::HashSet; + use std::collections::{HashMap, HashSet}; /// Validation for reference note /// @@ -824,6 +843,17 @@ mod tests { (isk, bundle, sighash) } + // Creates a set of finalized assets from a map of issued assets to support the new return type + // of the `verify_issue_bundle` function. + // TODO: Adapt issue bundle verification test functions to the new return type and remove this + // function. + fn get_finalization_set(issued_assets: &HashMap) -> HashSet { + issued_assets + .iter() + .filter_map(|(asset, asset_supply)| asset_supply.is_finalized.then(|| asset.clone())) + .collect::>() + } + #[test] fn verify_supply_valid() { let (ik, test_asset, action) = supply_test_params(10, 20, b"Asset 1", None, false); @@ -885,242 +915,6 @@ mod tests { ); } - #[test] - fn issue_bundle_basic() { - let (rng, _, ik, recipient, _) = setup_params(); - - let str = "Halo".to_string(); - let str2 = "Halo2".to_string(); - - assert_eq!( - IssueBundle::new( - ik.clone(), - vec![b'X'; 513], - Some(IssueInfo { - recipient, - value: NoteValue::unsplittable() - }), - true, - rng, - ) - .unwrap_err(), - WrongAssetDescSize - ); - - assert_eq!( - IssueBundle::new( - ik.clone(), - b"".to_vec(), - Some(IssueInfo { - recipient, - value: NoteValue::unsplittable() - }), - true, - rng, - ) - .unwrap_err(), - WrongAssetDescSize - ); - - let (mut bundle, asset) = IssueBundle::new( - ik.clone(), - str.clone().into_bytes(), - Some(IssueInfo { - recipient, - value: NoteValue::from_raw(5), - }), - true, - rng, - ) - .unwrap(); - - let another_asset = bundle - .add_recipient( - &str.into_bytes(), - recipient, - NoteValue::from_raw(10), - false, - rng, - ) - .unwrap(); - assert_eq!(asset, another_asset); - - let third_asset = bundle - .add_recipient( - str2.as_bytes(), - recipient, - NoteValue::from_raw(15), - true, - rng, - ) - .unwrap(); - assert_ne!(asset, third_asset); - - let actions = bundle.actions(); - assert_eq!(actions.len(), 2); - - let action = bundle.get_action_by_asset(&asset).unwrap(); - assert_eq!(action.notes.len(), 3); - let reference_note = action.notes.get(0).unwrap(); - verify_reference_note(reference_note, asset); - let first_note = action.notes.get(1).unwrap(); - assert_eq!(first_note.value().inner(), 5); - assert_eq!(first_note.asset(), asset); - assert_eq!(first_note.recipient(), recipient); - - let second_note = action.notes.get(2).unwrap(); - assert_eq!(second_note.value().inner(), 10); - assert_eq!(second_note.asset(), asset); - assert_eq!(second_note.recipient(), recipient); - - let action2 = bundle.get_action_by_desc(str2.as_bytes()).unwrap(); - assert_eq!(action2.notes.len(), 2); - let reference_note = action2.notes.get(0).unwrap(); - verify_reference_note(reference_note, AssetBase::derive(&ik, str2.as_bytes())); - let first_note = action2.notes().get(1).unwrap(); - assert_eq!(first_note.value().inner(), 15); - assert_eq!(first_note.asset(), third_asset); - - verify_reference_note(action.get_reference_note().unwrap(), asset); - verify_reference_note(action2.get_reference_note().unwrap(), third_asset); - } - - #[test] - fn issue_bundle_finalize_asset() { - let (rng, _, ik, recipient, _) = setup_params(); - - let (mut bundle, _) = IssueBundle::new( - ik, - b"NFT".to_vec(), - Some(IssueInfo { - recipient, - value: NoteValue::from_raw(u64::MIN), - }), - true, - rng, - ) - .expect("Should properly add recipient"); - - bundle - .finalize_action(b"NFT") - .expect("Should finalize properly"); - - assert_eq!( - bundle.finalize_action(b"Another NFT").unwrap_err(), - IssueActionNotFound - ); - - assert_eq!( - bundle.finalize_action(&vec![b'X'; 513]).unwrap_err(), - WrongAssetDescSize - ); - - assert_eq!(bundle.finalize_action(b"").unwrap_err(), WrongAssetDescSize); - } - - #[test] - fn issue_bundle_prepare() { - let (rng, _, ik, recipient, sighash) = setup_params(); - - let (bundle, _) = IssueBundle::new( - ik, - b"Frost".to_vec(), - Some(IssueInfo { - recipient, - value: NoteValue::from_raw(5), - }), - true, - rng, - ) - .unwrap(); - - let prepared = bundle.prepare(sighash); - assert_eq!(prepared.authorization().sighash, sighash); - } - - #[test] - fn issue_bundle_sign() { - let (rng, isk, ik, recipient, sighash) = setup_params(); - - let (bundle, _) = IssueBundle::new( - ik.clone(), - b"Sign".to_vec(), - Some(IssueInfo { - recipient, - value: NoteValue::from_raw(5), - }), - true, - rng, - ) - .unwrap(); - - let signed = bundle.prepare(sighash).sign(&isk).unwrap(); - - ik.verify(&sighash, &signed.authorization.signature) - .expect("signature should be valid"); - } - - #[test] - fn issue_bundle_invalid_isk_for_signature() { - let (rng, _, ik, recipient, _) = setup_params(); - - let (bundle, _) = IssueBundle::new( - ik, - b"IssueBundle".to_vec(), - Some(IssueInfo { - recipient, - value: NoteValue::from_raw(5), - }), - true, - rng, - ) - .unwrap(); - - let wrong_isk: IssuanceAuthorizingKey = IssuanceAuthorizingKey::random(); - - let err = bundle - .prepare([0; 32]) - .sign(&wrong_isk) - .expect_err("should not be able to sign"); - - assert_eq!(err, IssueBundleIkMismatchAssetBase); - } - - #[test] - fn issue_bundle_incorrect_asset_for_signature() { - let (mut rng, isk, ik, recipient, _) = setup_params(); - - // Create a bundle with "normal" note - let (mut bundle, _) = IssueBundle::new( - ik, - b"IssueBundle".to_vec(), - Some(IssueInfo { - recipient, - value: NoteValue::from_raw(5), - }), - true, - rng, - ) - .unwrap(); - - // Add "bad" note - let note = Note::new( - recipient, - NoteValue::from_raw(5), - AssetBase::derive(bundle.ik(), b"zsa_asset"), - Rho::from_nf_old(Nullifier::dummy(&mut rng)), - &mut rng, - ); - bundle.actions.first_mut().notes.push(note); - - let err = bundle - .prepare([0; 32]) - .sign(&isk) - .expect_err("should not be able to sign"); - - assert_eq!(err, IssueBundleIkMismatchAssetBase); - } - #[test] fn issue_bundle_verify() { let (rng, isk, ik, recipient, sighash) = setup_params(); @@ -1138,11 +932,9 @@ mod tests { .unwrap(); let signed = bundle.prepare(sighash).sign(&isk).unwrap(); - let prev_finalized = &mut HashSet::new(); - let supply_info = verify_issue_bundle(&signed, sighash, prev_finalized).unwrap(); - - supply_info.update_finalization_set(prev_finalized); + let issued_assets = verify_issue_bundle(&signed, sighash, |_| None).unwrap(); + let prev_finalized = get_finalization_set(&issued_assets); assert!(prev_finalized.is_empty()); } @@ -1166,18 +958,16 @@ mod tests { bundle.finalize_action(b"Verify with finalize").unwrap(); let signed = bundle.prepare(sighash).sign(&isk).unwrap(); - let prev_finalized = &mut HashSet::new(); - - let supply_info = verify_issue_bundle(&signed, sighash, prev_finalized).unwrap(); - supply_info.update_finalization_set(prev_finalized); + let issued_assets = verify_issue_bundle(&signed, sighash, |_| None).unwrap(); + let prev_finalized = get_finalization_set(&issued_assets); assert_eq!(prev_finalized.len(), 1); assert!(prev_finalized.contains(&AssetBase::derive(&ik, b"Verify with finalize"))); } #[test] - fn issue_bundle_verify_with_supply_info() { + fn issue_bundle_verify_with_issued_assets() { let (rng, isk, ik, recipient, sighash) = setup_params(); let asset1_desc = b"Verify with supply info 1".to_vec(); @@ -1217,11 +1007,9 @@ mod tests { .unwrap(); let signed = bundle.prepare(sighash).sign(&isk).unwrap(); - let prev_finalized = &mut HashSet::new(); - let supply_info = verify_issue_bundle(&signed, sighash, prev_finalized).unwrap(); - - supply_info.update_finalization_set(prev_finalized); + let issued_assets = verify_issue_bundle(&signed, sighash, |_| None).unwrap(); + let prev_finalized = get_finalization_set(&issued_assets); assert_eq!(prev_finalized.len(), 2); @@ -1229,14 +1017,14 @@ mod tests { assert!(prev_finalized.contains(&asset2_base)); assert!(!prev_finalized.contains(&asset3_base)); - assert_eq!(supply_info.assets.len(), 3); + assert_eq!(issued_assets.keys().len(), 3); let reference_note1 = signed.actions()[0].notes()[0]; let reference_note2 = signed.actions()[1].notes()[0]; let reference_note3 = signed.actions()[2].notes()[0]; assert_eq!( - supply_info.assets.get(&asset1_base), + issued_assets.get(&asset1_base), Some(&AssetSupply::new( NoteValue::from_raw(15), true, @@ -1244,7 +1032,7 @@ mod tests { )) ); assert_eq!( - supply_info.assets.get(&asset2_base), + issued_assets.get(&asset2_base), Some(&AssetSupply::new( NoteValue::from_raw(10), true, @@ -1252,7 +1040,7 @@ mod tests { )) ); assert_eq!( - supply_info.assets.get(&asset3_base), + issued_assets.get(&asset3_base), Some(&AssetSupply::new( NoteValue::from_raw(5), false, @@ -1278,14 +1066,19 @@ mod tests { .unwrap(); let signed = bundle.prepare(sighash).sign(&isk).unwrap(); - let prev_finalized = &mut HashSet::new(); let final_type = AssetBase::derive(&ik, b"already final"); - prev_finalized.insert(final_type); + let issued_assets = [( + final_type, + AssetSupply::new(NoteValue::from_raw(0), true, None), + )] + .into_iter() + .collect::>(); assert_eq!( - verify_issue_bundle(&signed, sighash, prev_finalized).unwrap_err(), + verify_issue_bundle(&signed, sighash, |asset| issued_assets.get(asset).copied()) + .unwrap_err(), IssueActionPreviouslyFinalizedAssetBase(final_type) ); } @@ -1321,10 +1114,8 @@ mod tests { signature: wrong_isk.try_sign(&sighash).unwrap(), }); - let prev_finalized = &HashSet::new(); - assert_eq!( - verify_issue_bundle(&signed, sighash, prev_finalized).unwrap_err(), + verify_issue_bundle(&signed, sighash, |_| None).unwrap_err(), IssueBundleInvalidSignature ); } @@ -1346,10 +1137,9 @@ mod tests { let sighash: [u8; 32] = bundle.commitment().into(); let signed = bundle.prepare(sighash).sign(&isk).unwrap(); - let prev_finalized = &HashSet::new(); assert_eq!( - verify_issue_bundle(&signed, random_sighash, prev_finalized).unwrap_err(), + verify_issue_bundle(&signed, random_sighash, |_| None).unwrap_err(), IssueBundleInvalidSignature ); } @@ -1383,10 +1173,8 @@ mod tests { signed.actions.first_mut().notes.push(note); - let prev_finalized = &HashSet::new(); - assert_eq!( - verify_issue_bundle(&signed, sighash, prev_finalized).unwrap_err(), + verify_issue_bundle(&signed, sighash, |_| None).unwrap_err(), IssueBundleIkMismatchAssetBase ); } @@ -1425,10 +1213,8 @@ mod tests { signed.actions.first_mut().notes = vec![note]; - let prev_finalized = &HashSet::new(); - assert_eq!( - verify_issue_bundle(&signed, sighash, prev_finalized).unwrap_err(), + verify_issue_bundle(&signed, sighash, |_| None).unwrap_err(), IssueBundleIkMismatchAssetBase ); } @@ -1457,13 +1243,12 @@ mod tests { .unwrap(); let mut signed = bundle.prepare(sighash).sign(&isk).unwrap(); - let prev_finalized = HashSet::new(); // 1. Try a description that is too long signed.actions.first_mut().modify_descr(vec![b'X'; 513]); assert_eq!( - verify_issue_bundle(&signed, sighash, &prev_finalized).unwrap_err(), + verify_issue_bundle(&signed, sighash, |_| None).unwrap_err(), WrongAssetDescSize ); @@ -1471,7 +1256,7 @@ mod tests { signed.actions.first_mut().modify_descr(b"".to_vec()); assert_eq!( - verify_issue_bundle(&signed, sighash, &prev_finalized).unwrap_err(), + verify_issue_bundle(&signed, sighash, |_| None).unwrap_err(), WrongAssetDescSize ); } @@ -1491,7 +1276,7 @@ mod tests { let (isk, bundle, sighash) = identity_point_test_params(10, 20); let signed = IssueBundle { - ik: bundle.ik, + ik: bundle.ik().clone(), actions: bundle.actions, authorization: Signed { signature: isk.try_sign(&sighash).unwrap(), @@ -1499,7 +1284,7 @@ mod tests { }; assert_eq!( - verify_issue_bundle(&signed, sighash, &HashSet::new()).unwrap_err(), + verify_issue_bundle(&signed, sighash, |_| None).unwrap_err(), AssetBaseCannotBeIdentityPoint ); } diff --git a/src/supply_info.rs b/src/supply_info.rs index 6444201ec..2969b1e95 100644 --- a/src/supply_info.rs +++ b/src/supply_info.rs @@ -1,11 +1,9 @@ //! Structs and logic related to supply information management for assets. -use std::collections::{hash_map, HashMap, HashSet}; - -use crate::{issuance::Error, note::AssetBase, value::NoteValue, Note}; +use crate::{value::NoteValue, Note}; /// Represents the amount of an asset, its finalization status and reference note. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, Default)] #[cfg_attr(test, derive(PartialEq, Eq))] pub struct AssetSupply { /// The amount of the asset. @@ -30,178 +28,3 @@ impl AssetSupply { } } } - -/// Contains information about the supply of assets. -#[derive(Debug, Clone)] -pub struct SupplyInfo { - /// A map of asset bases to their respective supply information. - pub assets: HashMap, -} - -impl SupplyInfo { - /// Creates a new, empty `SupplyInfo` instance. - pub fn new() -> Self { - Self { - assets: HashMap::new(), - } - } - - /// Inserts or updates an asset's supply information in the supply info map. - /// If the asset exists, adds the amounts (unconditionally) and updates the finalization status - /// (only if the new supply is finalized). If the asset is not found, inserts the new supply. - pub fn add_supply(&mut self, asset: AssetBase, new_supply: AssetSupply) -> Result<(), Error> { - match self.assets.entry(asset) { - hash_map::Entry::Occupied(entry) => { - let supply = entry.into_mut(); - supply.amount = (supply.amount + new_supply.amount).ok_or(Error::ValueOverflow)?; - supply.is_finalized |= new_supply.is_finalized; - supply.reference_note = supply.reference_note.or(new_supply.reference_note); - } - hash_map::Entry::Vacant(entry) => { - entry.insert(new_supply); - } - } - - Ok(()) - } - - /// Updates the set of finalized assets based on the supply information stored in - /// the `SupplyInfo` instance. - pub fn update_finalization_set(&self, finalization_set: &mut HashSet) { - finalization_set.extend( - self.assets - .iter() - .filter_map(|(asset, supply)| supply.is_finalized.then_some(asset)), - ); - } -} - -impl Default for SupplyInfo { - fn default() -> Self { - Self::new() - } -} - -#[cfg(test)] -mod tests { - use super::*; - - fn create_test_asset(asset_desc: &[u8]) -> AssetBase { - use crate::keys::{IssuanceAuthorizingKey, IssuanceValidatingKey}; - - let isk = IssuanceAuthorizingKey::from_bytes([1u8; 32]).unwrap(); - - AssetBase::derive(&IssuanceValidatingKey::from(&isk), asset_desc) - } - - fn sum<'a, T: IntoIterator>(supplies: T) -> Option { - supplies - .into_iter() - .map(|supply| supply.amount) - .try_fold(NoteValue::from_raw(0), |sum, value| sum + value) - } - - #[test] - fn test_add_supply_valid() { - let mut supply_info = SupplyInfo::new(); - - let asset1 = create_test_asset(b"Asset 1"); - let asset2 = create_test_asset(b"Asset 2"); - - let supply1 = AssetSupply::new(NoteValue::from_raw(20), false, None); - let supply2 = AssetSupply::new(NoteValue::from_raw(30), true, None); - let supply3 = AssetSupply::new(NoteValue::from_raw(10), false, None); - let supply4 = AssetSupply::new(NoteValue::from_raw(10), true, None); - let supply5 = AssetSupply::new(NoteValue::from_raw(50), false, None); - - assert_eq!(supply_info.assets.len(), 0); - - // Add supply1 - assert!(supply_info.add_supply(asset1, supply1).is_ok()); - assert_eq!(supply_info.assets.len(), 1); - assert_eq!( - supply_info.assets.get(&asset1), - Some(&AssetSupply::new(sum([&supply1]).unwrap(), false, None)) - ); - - // Add supply2 - assert!(supply_info.add_supply(asset1, supply2).is_ok()); - assert_eq!(supply_info.assets.len(), 1); - assert_eq!( - supply_info.assets.get(&asset1), - Some(&AssetSupply::new( - sum([&supply1, &supply2]).unwrap(), - true, - None - )) - ); - - // Add supply3 - assert!(supply_info.add_supply(asset1, supply3).is_ok()); - assert_eq!(supply_info.assets.len(), 1); - assert_eq!( - supply_info.assets.get(&asset1), - Some(&AssetSupply::new( - sum([&supply1, &supply2, &supply3]).unwrap(), - true, - None - )) - ); - - // Add supply4 - assert!(supply_info.add_supply(asset1, supply4).is_ok()); - assert_eq!(supply_info.assets.len(), 1); - assert_eq!( - supply_info.assets.get(&asset1), - Some(&AssetSupply::new( - sum([&supply1, &supply2, &supply3, &supply4]).unwrap(), - true, - None - )) - ); - - // Add supply5 - assert!(supply_info.add_supply(asset2, supply5).is_ok()); - assert_eq!(supply_info.assets.len(), 2); - assert_eq!( - supply_info.assets.get(&asset1), - Some(&AssetSupply::new( - sum([&supply1, &supply2, &supply3, &supply4]).unwrap(), - true, - None - )) - ); - assert_eq!( - supply_info.assets.get(&asset2), - Some(&AssetSupply::new(sum([&supply5]).unwrap(), false, None)) - ); - } - - #[test] - fn test_update_finalization_set() { - let mut supply_info = SupplyInfo::new(); - - let asset1 = create_test_asset(b"Asset 1"); - let asset2 = create_test_asset(b"Asset 2"); - let asset3 = create_test_asset(b"Asset 3"); - - let supply1 = AssetSupply::new(NoteValue::from_raw(10), false, None); - let supply2 = AssetSupply::new(NoteValue::from_raw(20), true, None); - let supply3 = AssetSupply::new(NoteValue::from_raw(40), false, None); - let supply4 = AssetSupply::new(NoteValue::from_raw(50), true, None); - - assert!(supply_info.add_supply(asset1, supply1).is_ok()); - assert!(supply_info.add_supply(asset1, supply2).is_ok()); - assert!(supply_info.add_supply(asset2, supply3).is_ok()); - assert!(supply_info.add_supply(asset3, supply4).is_ok()); - - let mut finalization_set = HashSet::new(); - - supply_info.update_finalization_set(&mut finalization_set); - - assert_eq!(finalization_set.len(), 2); - - assert!(finalization_set.contains(&asset1)); - assert!(finalization_set.contains(&asset3)); - } -} diff --git a/tests/zsa.rs b/tests/zsa.rs index 1efbeda38..6e1671f12 100644 --- a/tests/zsa.rs +++ b/tests/zsa.rs @@ -19,7 +19,6 @@ use orchard::{ Address, Anchor, Bundle, Note, }; use rand::rngs::OsRng; -use std::collections::HashSet; use zcash_note_encryption_zsa::try_note_decryption; #[derive(Debug)] @@ -177,12 +176,7 @@ fn issue_zsa_notes(asset_descr: &[u8], keys: &Keychain) -> (Note, Note, Note) { AssetBase::derive(&keys.ik().clone(), asset_descr), ); - assert!(verify_issue_bundle( - &issue_bundle, - issue_bundle.commitment().into(), - &HashSet::new(), - ) - .is_ok()); + assert!(verify_issue_bundle(&issue_bundle, issue_bundle.commitment().into(), |_| None).is_ok()); (*reference_note, *note1, *note2) } From c54d448565e2aa3595f67d3b7d5366c07f33ccf9 Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Mon, 27 Jan 2025 09:31:04 +0100 Subject: [PATCH 02/28] Rename IssueAction::verify_supply to IssueAction::verify --- src/issuance.rs | 40 +++++++++++++++++----------------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/src/issuance.rs b/src/issuance.rs index 1926e8059..5a02d7db6 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -104,12 +104,12 @@ impl IssueAction { self.finalize } - /// Verifies and computes the new asset supply for an `IssueAction`. + /// Verifies an `IssueAction` and computes the asset supply for it. /// /// This function calculates the total value (supply) of the asset by summing the values /// of all its notes and ensures that all note types are equal. It returns the asset and /// its supply as a tuple (`AssetBase`, `AssetSupply`) or an error if the asset was not - /// properly derived or an overflow occurred during the supply amount calculation. + /// properly derived or an overflow occurred during the supply amount calculation. /// /// # Arguments /// @@ -129,7 +129,7 @@ impl IssueAction { /// `AssetBase` for **all** internal notes. /// /// * `IssueActionWithoutNoteNotFinalized`:If the `IssueAction` contains no note and is not finalized. - fn verify_supply(&self, ik: &IssuanceValidatingKey) -> Result<(AssetBase, AssetSupply), Error> { + fn verify(&self, ik: &IssuanceValidatingKey) -> Result<(AssetBase, AssetSupply), Error> { if self.notes.is_empty() && !self.is_finalized() { return Err(IssueActionWithoutNoteNotFinalized); } @@ -539,7 +539,7 @@ impl IssueBundle { // Make sure the `expected_ik` matches the `asset` for all notes. self.actions.iter().try_for_each(|action| { - action.verify_supply(&expected_ik)?; + action.verify(&expected_ik)?; Ok(()) })?; @@ -591,7 +591,7 @@ impl IssueBundle { /// - The signature on the provided `sighash` is verified correctly. /// - **IssueAction Verification**: /// - The asset description size is correct. -/// - The `verify_supply` method checks pass, +/// - The `IssueAction::verify` method checks pass, /// - The new amount does not overflow the previous total supply value. /// - The `AssetBase` for the `IssueAction` has not been previously finalized. /// @@ -618,7 +618,7 @@ impl IssueBundle { /// - `ValueOverflow`: Occurs if an overflow happens during the calculation of the total value for /// the notes. /// - `IssueActionPreviouslyFinalizedAssetBase`: Occurs if the asset has already been finalized. -/// - **Other Errors**: Any additional errors returned by the `verify_supply` method of `IssueAction` +/// - **Other Errors**: Any additional errors returned by the `IssueAction::verify` method. /// will also be propagated. pub fn verify_issue_bundle( bundle: &IssueBundle, @@ -640,7 +640,7 @@ pub fn verify_issue_bundle( return Err(WrongAssetDescSize); } - let (asset, action_asset_state) = action.verify_supply(bundle.ik())?; + let (asset, action_asset_state) = action.verify(bundle.ik())?; let old_asset_state = verified_asset_states .get(&asset) @@ -911,10 +911,10 @@ mod tests { } #[test] - fn verify_supply_valid() { + fn action_verify_valid() { let (ik, test_asset, action) = supply_test_params(10, 20, b"Asset 1", None, false); - let result = action.verify_supply(&ik); + let result = action.verify(&ik); assert!(result.is_ok()); @@ -926,20 +926,20 @@ mod tests { } #[test] - fn verify_supply_invalid_for_asset_base_as_identity() { + fn action_verify_invalid_for_asset_base_as_identity() { let (_, bundle, _, _) = identity_point_test_params(10, 20); assert_eq!( - bundle.actions.head.verify_supply(&bundle.ik), + bundle.actions.head.verify(&bundle.ik), Err(AssetBaseCannotBeIdentityPoint) ); } #[test] - fn verify_supply_finalized() { + fn action_verify_finalized() { let (ik, test_asset, action) = supply_test_params(10, 20, b"Asset 1", None, true); - let result = action.verify_supply(&ik); + let result = action.verify(&ik); assert!(result.is_ok()); @@ -951,24 +951,18 @@ mod tests { } #[test] - fn verify_supply_incorrect_asset_base() { + fn action_verify_incorrect_asset_base() { let (ik, _, action) = supply_test_params(10, 20, b"Asset 1", Some(b"Asset 2"), false); - assert_eq!( - action.verify_supply(&ik), - Err(IssueBundleIkMismatchAssetBase) - ); + assert_eq!(action.verify(&ik), Err(IssueBundleIkMismatchAssetBase)); } #[test] - fn verify_supply_ik_mismatch_asset_base() { + fn action_verify_ik_mismatch_asset_base() { let (_, _, action) = supply_test_params(10, 20, b"Asset 1", None, false); let (_, _, ik, _, _, _) = setup_params(); - assert_eq!( - action.verify_supply(&ik), - Err(IssueBundleIkMismatchAssetBase) - ); + assert_eq!(action.verify(&ik), Err(IssueBundleIkMismatchAssetBase)); } #[test] From 9739253055f7ccab7dbc22d83f55c3e4392933d3 Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Mon, 27 Jan 2025 09:39:16 +0100 Subject: [PATCH 03/28] Simplify Error variant imports by referencing the local Error enum directly --- src/issuance.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/issuance.rs b/src/issuance.rs index 5a02d7db6..ad64ad3f7 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -9,11 +9,6 @@ use std::fmt; use crate::bundle::commitments::{hash_issue_bundle_auth_data, hash_issue_bundle_txid_data}; use crate::constants::reference_keys::ReferenceKeys; -use crate::issuance::Error::{ - AssetBaseCannotBeIdentityPoint, CannotBeFirstIssuance, IssueActionNotFound, - IssueActionPreviouslyFinalizedAssetBase, IssueActionWithoutNoteNotFinalized, - IssueBundleIkMismatchAssetBase, IssueBundleInvalidSignature, ValueOverflow, WrongAssetDescSize, -}; use crate::keys::{IssuanceAuthorizingKey, IssuanceValidatingKey}; use crate::note::asset_base::is_asset_desc_of_valid_size; use crate::note::{AssetBase, Nullifier, Rho}; @@ -23,6 +18,12 @@ use crate::{Address, Note}; use crate::supply_info::AssetSupply; +use Error::{ + AssetBaseCannotBeIdentityPoint, CannotBeFirstIssuance, IssueActionNotFound, + IssueActionPreviouslyFinalizedAssetBase, IssueActionWithoutNoteNotFinalized, + IssueBundleIkMismatchAssetBase, IssueBundleInvalidSignature, ValueOverflow, WrongAssetDescSize, +}; + /// Checks if a given note is a reference note. /// /// A reference note satisfies the following conditions: From 6f7ba6d1d3da46235c46093994e7f88708973480 Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Mon, 27 Jan 2025 11:42:56 +0100 Subject: [PATCH 04/28] Introduce AssetInfo generic and use it instead of AssetSupply struct, update verify_issue_bundle function to ensure the first asset issuance contains the reference note, update tests and docs accordingly --- src/issuance.rs | 161 ++++++++++++++++++++++++++------------------- src/supply_info.rs | 40 ++++++++--- 2 files changed, 125 insertions(+), 76 deletions(-) diff --git a/src/issuance.rs b/src/issuance.rs index ad64ad3f7..cbc576c24 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -16,12 +16,13 @@ use crate::note::{AssetBase, Nullifier, Rho}; use crate::value::NoteValue; use crate::{Address, Note}; -use crate::supply_info::AssetSupply; +use crate::supply_info::{ActionAssetInfo, BundleAssetInfo}; use Error::{ AssetBaseCannotBeIdentityPoint, CannotBeFirstIssuance, IssueActionNotFound, IssueActionPreviouslyFinalizedAssetBase, IssueActionWithoutNoteNotFinalized, - IssueBundleIkMismatchAssetBase, IssueBundleInvalidSignature, ValueOverflow, WrongAssetDescSize, + IssueBundleIkMismatchAssetBase, IssueBundleInvalidSignature, + MissingReferenceNoteOnFirstIssuance, ValueOverflow, WrongAssetDescSize, }; /// Checks if a given note is a reference note. @@ -105,12 +106,7 @@ impl IssueAction { self.finalize } - /// Verifies an `IssueAction` and computes the asset supply for it. - /// - /// This function calculates the total value (supply) of the asset by summing the values - /// of all its notes and ensures that all note types are equal. It returns the asset and - /// its supply as a tuple (`AssetBase`, `AssetSupply`) or an error if the asset was not - /// properly derived or an overflow occurred during the supply amount calculation. + /// Verifies this `IssueAction` and calculates the aggregated asset information. /// /// # Arguments /// @@ -118,7 +114,7 @@ impl IssueAction { /// /// # Returns /// - /// A `Result` containing a tuple with an `AssetBase` and an `AssetSupply`, or an `Error`. + /// A `Result` containing a tuple with an `AssetBase` and an `ActionAssetInfo`, or an `Error`. /// /// # Errors /// @@ -130,7 +126,7 @@ impl IssueAction { /// `AssetBase` for **all** internal notes. /// /// * `IssueActionWithoutNoteNotFinalized`:If the `IssueAction` contains no note and is not finalized. - fn verify(&self, ik: &IssuanceValidatingKey) -> Result<(AssetBase, AssetSupply), Error> { + fn verify(&self, ik: &IssuanceValidatingKey) -> Result<(AssetBase, ActionAssetInfo), Error> { if self.notes.is_empty() && !self.is_finalized() { return Err(IssueActionWithoutNoteNotFinalized); } @@ -160,7 +156,7 @@ impl IssueAction { Ok(( issue_asset, - AssetSupply::new( + ActionAssetInfo::new( value_sum, self.is_finalized(), self.get_reference_note().cloned(), @@ -585,48 +581,48 @@ impl IssueBundle { } } -/// Validation for Orchard IssueBundles +/// Validates an [`IssueBundle`] by performing the following checks: /// -/// The following checks are performed: -/// - **IssueBundle Verification**: -/// - The signature on the provided `sighash` is verified correctly. +/// - **Signature Verification**: +/// - Ensures the signature on the provided `sighash` matches the bundle’s authorization. /// - **IssueAction Verification**: -/// - The asset description size is correct. -/// - The `IssueAction::verify` method checks pass, -/// - The new amount does not overflow the previous total supply value. -/// - The `AssetBase` for the `IssueAction` has not been previously finalized. +/// - Checks that the asset description size is correct. +/// - Runs all checks within the `IssueAction::verify` method. +/// - 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 issuances may omit it. /// /// # Arguments /// -/// - `bundle` - A reference to the `IssueBundle` to be validated. -/// - `sighash` - A 32-byte array representing the sighash used to verify the bundle's signature. -/// - `get_asset_state` - A closure that takes a reference to an `AssetBase` and returns an -/// `Option`, representing the current state of the asset in the global store of -/// previously issued assets. +/// - `bundle`: A reference to the [`IssueBundle`] to be validated. +/// - `sighash`: A 32-byte array representing the `sighash` used to verify the bundle's signature. +/// - `get_asset_state`: A closure that takes a reference to an [`AssetBase`] and returns an +/// [`Option`], representing the current state of the asset from a global store +/// of previously issued assets. /// /// # Returns /// -/// A `Result` with a `HashMap` on success, which contains new values for updated or newly added -/// items in the global state. Each key is an `AssetBase`, and the corresponding value is a new -/// (updated) `AssetSupply`. +/// A `Result` containing a [`HashMap`] upon success, where each key-value +/// pair represents the new or updated state of an asset. The key is an [`AssetBase`], and the value +/// is the corresponding updated [`BundleAssetInfo`]. /// /// # Errors /// -/// - `IssueBundleInvalidSignature`: Occurs if the signature verification for the provided `sighash` -/// fails. -/// - `WrongAssetDescSize`: Raised if the asset description size for any asset in the bundle is -/// incorrect. -/// - `ValueOverflow`: Occurs if an overflow happens during the calculation of the total value for -/// the notes. -/// - `IssueActionPreviouslyFinalizedAssetBase`: Occurs if the asset has already been finalized. -/// - **Other Errors**: Any additional errors returned by the `IssueAction::verify` method. -/// will also be propagated. +/// - [`IssueBundleInvalidSignature`]: If the signature verification for the provided `sighash` fails. +/// - [`WrongAssetDescSize`]: If the asset description size is invalid. +/// - [`ValueOverflow`]: If adding the new amount to the existing total supply causes an overflow. +/// - [`IssueActionPreviouslyFinalizedAssetBase`]: If an action is attempted on an asset that has +/// already been finalized. +/// - [`MissingReferenceNoteOnFirstIssuance`]: If no reference note is provided for the first +/// issuance of a new asset. +/// - **Other Errors**: Any additional errors returned by the `IssueAction::verify` method are +/// propagated pub fn verify_issue_bundle( bundle: &IssueBundle, sighash: [u8; 32], // FIXME: consider using AssetStateStore trait with get_asset method instead - get_asset_state: impl Fn(&AssetBase) -> Option, -) -> Result, Error> { + get_asset_state: impl Fn(&AssetBase) -> Option, +) -> Result, Error> { bundle .ik() .verify(&sighash, bundle.authorization().signature()) @@ -641,29 +637,36 @@ pub fn verify_issue_bundle( return Err(WrongAssetDescSize); } - let (asset, action_asset_state) = action.verify(bundle.ik())?; + let (asset, action_asset_info) = action.verify(bundle.ik())?; - let old_asset_state = verified_asset_states + let verified_asset_state = match verified_asset_states .get(&asset) .cloned() .or_else(|| get_asset_state(&asset)) - .unwrap_or_default(); - - let amount = - (old_asset_state.amount + action_asset_state.amount).ok_or(ValueOverflow)?; - - let is_finalized = (!old_asset_state.is_finalized) - .then_some(action_asset_state.is_finalized) - .ok_or(IssueActionPreviouslyFinalizedAssetBase(asset))?; - - let reference_note = old_asset_state - .reference_note - .or(action_asset_state.reference_note); - - verified_asset_states.insert( - asset, - AssetSupply::new(amount, is_finalized, reference_note), - ); + { + // The first issuance of the asset + None => BundleAssetInfo::new( + action_asset_info.amount, + action_asset_info.is_finalized, + action_asset_info + .reference_note + .ok_or(MissingReferenceNoteOnFirstIssuance)?, + ), + + // Subsequent issuances of the asset + Some(old_asset_state) => { + let amount = (old_asset_state.amount + action_asset_info.amount) + .ok_or(ValueOverflow)?; + + let is_finalized = (!old_asset_state.is_finalized) + .then_some(action_asset_info.is_finalized) + .ok_or(IssueActionPreviouslyFinalizedAssetBase(asset))?; + + BundleAssetInfo::new(amount, is_finalized, old_asset_state.reference_note) + } + }; + + verified_asset_states.insert(asset, verified_asset_state); Ok(verified_asset_states) })?; @@ -671,6 +674,8 @@ pub fn verify_issue_bundle( Ok(verified_asset_states) } +// FIXME: IssueActionPreviouslyFinalizedAssetBase contains AssetBase but +// WrongAssetDescSize and new MissingReferenceNoteOnFirstIssuance don't? /// Errors produced during the issuance process #[derive(Debug, PartialEq, Eq)] pub enum Error { @@ -687,6 +692,7 @@ pub enum Error { /// It cannot be first issuance because we have already some notes for this asset. CannotBeFirstIssuance, + // FIXME: split the group comment ("Verification errors")? /// Verification errors: /// Invalid signature. IssueBundleInvalidSignature, @@ -695,6 +701,9 @@ pub enum Error { /// Overflow error occurred while calculating the value of the asset ValueOverflow, + + /// No reference note is provided for the first issuance of a new asset. + MissingReferenceNoteOnFirstIssuance, } impl fmt::Display for Error { @@ -742,6 +751,12 @@ impl fmt::Display for Error { "overflow error occurred while calculating the value of the asset" ) } + MissingReferenceNoteOnFirstIssuance => { + write!( + f, + "no reference note is provided for the first issuance of a new asset." + ) + } } } } @@ -750,7 +765,7 @@ impl fmt::Display for Error { // is called: 1) check for expected output, 2) check for processing of existing assets etc. #[cfg(test)] mod tests { - use super::{AssetSupply, IssueBundle, IssueInfo}; + use super::{BundleAssetInfo, IssueBundle, IssueInfo}; use crate::{ builder::{Builder, BundleType}, circuit::ProvingKey, @@ -904,7 +919,9 @@ mod tests { // of the `verify_issue_bundle` function. // TODO: Adapt issue bundle verification test functions to the new return type and remove this // function. - fn get_finalization_set(issued_assets: &HashMap) -> HashSet { + fn get_finalization_set( + issued_assets: &HashMap, + ) -> HashSet { issued_assets .iter() .filter_map(|(asset, asset_supply)| asset_supply.is_finalized.then(|| asset.clone())) @@ -1346,33 +1363,33 @@ mod tests { assert_eq!( issued_assets.get(&asset1_base), - Some(&AssetSupply::new( + Some(&BundleAssetInfo::new( NoteValue::from_raw(15), true, - Some(reference_note1) + reference_note1 )) ); assert_eq!( issued_assets.get(&asset2_base), - Some(&AssetSupply::new( + Some(&BundleAssetInfo::new( NoteValue::from_raw(10), true, - Some(reference_note2) + reference_note2 )) ); assert_eq!( issued_assets.get(&asset3_base), - Some(&AssetSupply::new( + Some(&BundleAssetInfo::new( NoteValue::from_raw(5), false, - Some(reference_note3) + reference_note3 )) ); } #[test] fn issue_bundle_verify_fail_previously_finalized() { - let (rng, isk, ik, recipient, sighash, first_nullifier) = setup_params(); + let (mut rng, isk, ik, recipient, sighash, first_nullifier) = setup_params(); let (bundle, _) = IssueBundle::new( ik.clone(), @@ -1396,7 +1413,17 @@ mod tests { let issued_assets = [( final_type, - AssetSupply::new(NoteValue::from_raw(0), true, None), + BundleAssetInfo::new( + NoteValue::from_raw(20), + true, + Note::new( + recipient, + NoteValue::from_raw(10), + final_type, + Rho::zero(), + &mut rng, + ), + ), )] .into_iter() .collect::>(); diff --git a/src/supply_info.rs b/src/supply_info.rs index 2969b1e95..c4a7fdb2a 100644 --- a/src/supply_info.rs +++ b/src/supply_info.rs @@ -1,25 +1,47 @@ -//! Structs and logic related to supply information management for assets. +//! Structs and logic related to aggregated information about an asset. use crate::{value::NoteValue, Note}; -/// Represents the amount of an asset, its finalization status and reference note. +/// Represents aggregated information about an asset, including its supply, finalization status, +/// and reference note. +/// +/// - For bundles or global state, the reference note is always [`Note`]. +/// - For actions, the reference note may be [`Option`] because some actions do not include it. #[derive(Debug, Clone, Copy, Default)] #[cfg_attr(test, derive(PartialEq, Eq))] -pub struct AssetSupply { +pub struct AssetInfo { /// The amount of the asset. pub amount: NoteValue, /// Whether or not the asset is finalized. pub is_finalized: bool, - /// The reference note, `None` if this `AssetSupply` instance is created from an issue bundle that does not include - /// a reference note (a non-first issuance) - pub reference_note: Option, + /// A reference note, which may be [`Note`] (for bundles/global state) + /// or [`Option`] (for actions). + pub reference_note: R, } -impl AssetSupply { - /// Creates a new AssetSupply instance with the given amount, finalization status and reference - /// note. +/// For bundles and global state, the reference note is always provided. +pub type BundleAssetInfo = AssetInfo; + +/// For actions, the reference note may be omitted if this asset is already known. +pub type ActionAssetInfo = AssetInfo>; + +impl BundleAssetInfo { + /// Creates a new [`AssetInfo`] instance for an `IssueBundle`, + /// where a reference note is always specified. + pub fn new(amount: NoteValue, is_finalized: bool, reference_note: Note) -> Self { + Self { + amount, + is_finalized, + reference_note, + } + } +} + +impl ActionAssetInfo { + /// Creates a new [`AssetInfo`] instance for an `IssueAction`, + /// where the reference note can be omitted if this is not the first issuance. pub fn new(amount: NoteValue, is_finalized: bool, reference_note: Option) -> Self { Self { amount, From 48abd861b18eabb1482443cfbed1fe412942d4b5 Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Mon, 27 Jan 2025 16:20:27 +0100 Subject: [PATCH 05/28] Remove FIXME --- src/issuance.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/issuance.rs b/src/issuance.rs index cbc576c24..52635ed7e 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -620,7 +620,6 @@ impl IssueBundle { pub fn verify_issue_bundle( bundle: &IssueBundle, sighash: [u8; 32], - // FIXME: consider using AssetStateStore trait with get_asset method instead get_asset_state: impl Fn(&AssetBase) -> Option, ) -> Result, Error> { bundle @@ -674,7 +673,7 @@ pub fn verify_issue_bundle( Ok(verified_asset_states) } -// FIXME: IssueActionPreviouslyFinalizedAssetBase contains AssetBase but +// FIXME: Why IssueActionPreviouslyFinalizedAssetBase contains AssetBase but // WrongAssetDescSize and new MissingReferenceNoteOnFirstIssuance don't? /// Errors produced during the issuance process #[derive(Debug, PartialEq, Eq)] From fb0326a895e9100030c5645bd035ff028987e0e1 Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Tue, 28 Jan 2025 11:58:43 +0100 Subject: [PATCH 06/28] Make AssetInfo a regular struct instead of generic, use it for bundle verification only, update IssueAction::verify and return (AssetBase, NoteValue) from it instead of (AssetBase, AssetInfo<...>) --- src/issuance.rs | 115 ++++++++++++++++++++++----------------------- src/supply_info.rs | 35 +++----------- 2 files changed, 61 insertions(+), 89 deletions(-) diff --git a/src/issuance.rs b/src/issuance.rs index 52635ed7e..730eb211e 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -16,7 +16,7 @@ use crate::note::{AssetBase, Nullifier, Rho}; use crate::value::NoteValue; use crate::{Address, Note}; -use crate::supply_info::{ActionAssetInfo, BundleAssetInfo}; +use crate::supply_info::AssetInfo; use Error::{ AssetBaseCannotBeIdentityPoint, CannotBeFirstIssuance, IssueActionNotFound, @@ -106,8 +106,12 @@ impl IssueAction { self.finalize } - /// Verifies this `IssueAction` and calculates the aggregated asset information. + /// Verifies and computes the new asset supply for an `IssueAction`. /// + /// This function calculates the total value (supply) of the asset by summing the values + /// of all its notes and ensures that all note types are equal. It returns the asset and + /// its supply as a tuple (`AssetBase`, `NoteValue`) or an error if the asset was not + /// properly derived or an overflow occurred during the supply amount calculation. /// # Arguments /// /// * `ik` - A reference to the `IssuanceValidatingKey` used for deriving the asset. @@ -126,7 +130,7 @@ impl IssueAction { /// `AssetBase` for **all** internal notes. /// /// * `IssueActionWithoutNoteNotFinalized`:If the `IssueAction` contains no note and is not finalized. - fn verify(&self, ik: &IssuanceValidatingKey) -> Result<(AssetBase, ActionAssetInfo), Error> { + fn verify(&self, ik: &IssuanceValidatingKey) -> Result<(AssetBase, NoteValue), Error> { if self.notes.is_empty() && !self.is_finalized() { return Err(IssueActionWithoutNoteNotFinalized); } @@ -145,23 +149,15 @@ impl IssueAction { } // All assets should be derived correctly - note.asset() - .eq(&issue_asset) - .then_some(()) - .ok_or(IssueBundleIkMismatchAssetBase)?; + if note.asset() != issue_asset { + return Err(IssueBundleIkMismatchAssetBase); + } // The total amount should not overflow (value_sum + note.value()).ok_or(ValueOverflow) })?; - Ok(( - issue_asset, - ActionAssetInfo::new( - value_sum, - self.is_finalized(), - self.get_reference_note().cloned(), - ), - )) + Ok((issue_asset, value_sum)) } /// Serialize `finalize` flag to a byte @@ -596,15 +592,15 @@ impl IssueBundle { /// /// - `bundle`: A reference to the [`IssueBundle`] to be validated. /// - `sighash`: A 32-byte array representing the `sighash` used to verify the bundle's signature. -/// - `get_asset_state`: A closure that takes a reference to an [`AssetBase`] and returns an -/// [`Option`], representing the current state of the asset from a global store +/// - `get_global_asset_state`: A closure that takes a reference to an [`AssetBase`] and returns an +/// [`Option`], representing the current state of the asset from a global store /// of previously issued assets. /// /// # Returns /// -/// A `Result` containing a [`HashMap`] upon success, where each key-value +/// A `Result` containing a [`HashMap`] upon success, where each key-value /// pair represents the new or updated state of an asset. The key is an [`AssetBase`], and the value -/// is the corresponding updated [`BundleAssetInfo`]. +/// is the corresponding updated [`AssetInfo`]. /// /// # Errors /// @@ -620,8 +616,8 @@ impl IssueBundle { pub fn verify_issue_bundle( bundle: &IssueBundle, sighash: [u8; 32], - get_asset_state: impl Fn(&AssetBase) -> Option, -) -> Result, Error> { + get_global_asset_state: impl Fn(&AssetBase) -> Option, +) -> Result, Error> { bundle .ik() .verify(&sighash, bundle.authorization().signature()) @@ -636,32 +632,32 @@ pub fn verify_issue_bundle( return Err(WrongAssetDescSize); } - let (asset, action_asset_info) = action.verify(bundle.ik())?; + let (asset, action_amount) = action.verify(bundle.ik())?; + let action_is_finalized = action.is_finalized(); + let action_reference_note = action.get_reference_note(); let verified_asset_state = match verified_asset_states .get(&asset) .cloned() - .or_else(|| get_asset_state(&asset)) + .or_else(|| get_global_asset_state(&asset)) { // The first issuance of the asset - None => BundleAssetInfo::new( - action_asset_info.amount, - action_asset_info.is_finalized, - action_asset_info - .reference_note - .ok_or(MissingReferenceNoteOnFirstIssuance)?, + None => AssetInfo::new( + action_amount, + action_is_finalized, + *action_reference_note.ok_or(MissingReferenceNoteOnFirstIssuance)?, ), // Subsequent issuances of the asset - Some(old_asset_state) => { - let amount = (old_asset_state.amount + action_asset_info.amount) - .ok_or(ValueOverflow)?; + Some(prev_asset_state) => { + let amount = + (prev_asset_state.amount + action_amount).ok_or(ValueOverflow)?; - let is_finalized = (!old_asset_state.is_finalized) - .then_some(action_asset_info.is_finalized) - .ok_or(IssueActionPreviouslyFinalizedAssetBase(asset))?; + if prev_asset_state.is_finalized { + return Err(IssueActionPreviouslyFinalizedAssetBase(asset)); + } - BundleAssetInfo::new(amount, is_finalized, old_asset_state.reference_note) + AssetInfo::new(amount, action_is_finalized, prev_asset_state.reference_note) } }; @@ -764,7 +760,7 @@ impl fmt::Display for Error { // is called: 1) check for expected output, 2) check for processing of existing assets etc. #[cfg(test)] mod tests { - use super::{BundleAssetInfo, IssueBundle, IssueInfo}; + use super::{AssetInfo, IssueBundle, IssueInfo}; use crate::{ builder::{Builder, BundleType}, circuit::ProvingKey, @@ -829,11 +825,11 @@ mod tests { (rng, isk, ik, recipient, sighash, first_nullifier) } - /// Sets up test parameters for supply tests. + /// Sets up test parameters for action verification tests. /// /// This function generates two notes with the specified values and asset descriptions, /// and returns the issuance validating key, the asset base, and the issue action. - fn supply_test_params( + fn action_verify_test_params( note1_value: u64, note2_value: u64, note1_asset_desc: &[u8], @@ -918,28 +914,26 @@ mod tests { // of the `verify_issue_bundle` function. // TODO: Adapt issue bundle verification test functions to the new return type and remove this // function. - fn get_finalization_set( - issued_assets: &HashMap, - ) -> HashSet { + fn get_finalization_set(issued_assets: &HashMap) -> HashSet { issued_assets .iter() - .filter_map(|(asset, asset_supply)| asset_supply.is_finalized.then(|| asset.clone())) + .filter_map(|(asset, asset_info)| asset_info.is_finalized.then(|| asset.clone())) .collect::>() } #[test] fn action_verify_valid() { - let (ik, test_asset, action) = supply_test_params(10, 20, b"Asset 1", None, false); + let (ik, test_asset, action) = action_verify_test_params(10, 20, b"Asset 1", None, false); let result = action.verify(&ik); assert!(result.is_ok()); - let (asset, supply) = result.unwrap(); + let (asset, amount) = result.unwrap(); assert_eq!(asset, test_asset); - assert_eq!(supply.amount, NoteValue::from_raw(30)); - assert!(!supply.is_finalized); + assert_eq!(amount, NoteValue::from_raw(30)); + assert!(!action.is_finalized()); } #[test] @@ -954,29 +948,30 @@ mod tests { #[test] fn action_verify_finalized() { - let (ik, test_asset, action) = supply_test_params(10, 20, b"Asset 1", None, true); + let (ik, test_asset, action) = action_verify_test_params(10, 20, b"Asset 1", None, true); let result = action.verify(&ik); assert!(result.is_ok()); - let (asset, supply) = result.unwrap(); + let (asset, amount) = result.unwrap(); assert_eq!(asset, test_asset); - assert_eq!(supply.amount, NoteValue::from_raw(30)); - assert!(supply.is_finalized); + assert_eq!(amount, NoteValue::from_raw(30)); + assert!(action.is_finalized()); } #[test] fn action_verify_incorrect_asset_base() { - let (ik, _, action) = supply_test_params(10, 20, b"Asset 1", Some(b"Asset 2"), false); + let (ik, _, action) = + action_verify_test_params(10, 20, b"Asset 1", Some(b"Asset 2"), false); assert_eq!(action.verify(&ik), Err(IssueBundleIkMismatchAssetBase)); } #[test] fn action_verify_ik_mismatch_asset_base() { - let (_, _, action) = supply_test_params(10, 20, b"Asset 1", None, false); + let (_, _, action) = action_verify_test_params(10, 20, b"Asset 1", None, false); let (_, _, ik, _, _, _) = setup_params(); assert_eq!(action.verify(&ik), Err(IssueBundleIkMismatchAssetBase)); @@ -1303,9 +1298,9 @@ mod tests { fn issue_bundle_verify_with_issued_assets() { let (rng, isk, ik, recipient, sighash, first_nullifier) = setup_params(); - let asset1_desc = b"Verify with supply info 1".to_vec(); - let asset2_desc = b"Verify with supply info 2".to_vec(); - let asset3_desc = b"Verify with supply info 3".to_vec(); + let asset1_desc = b"Verify with issued assets 1".to_vec(); + let asset2_desc = b"Verify with issued assets 2".to_vec(); + let asset3_desc = b"Verify with issued assets 3".to_vec(); let asset1_base = AssetBase::derive(&ik, &asset1_desc); let asset2_base = AssetBase::derive(&ik, &asset2_desc); @@ -1362,7 +1357,7 @@ mod tests { assert_eq!( issued_assets.get(&asset1_base), - Some(&BundleAssetInfo::new( + Some(&AssetInfo::new( NoteValue::from_raw(15), true, reference_note1 @@ -1370,7 +1365,7 @@ mod tests { ); assert_eq!( issued_assets.get(&asset2_base), - Some(&BundleAssetInfo::new( + Some(&AssetInfo::new( NoteValue::from_raw(10), true, reference_note2 @@ -1378,7 +1373,7 @@ mod tests { ); assert_eq!( issued_assets.get(&asset3_base), - Some(&BundleAssetInfo::new( + Some(&AssetInfo::new( NoteValue::from_raw(5), false, reference_note3 @@ -1412,7 +1407,7 @@ mod tests { let issued_assets = [( final_type, - BundleAssetInfo::new( + AssetInfo::new( NoteValue::from_raw(20), true, Note::new( diff --git a/src/supply_info.rs b/src/supply_info.rs index c4a7fdb2a..4707328ed 100644 --- a/src/supply_info.rs +++ b/src/supply_info.rs @@ -4,32 +4,21 @@ use crate::{value::NoteValue, Note}; /// Represents aggregated information about an asset, including its supply, finalization status, /// and reference note. -/// -/// - For bundles or global state, the reference note is always [`Note`]. -/// - For actions, the reference note may be [`Option`] because some actions do not include it. -#[derive(Debug, Clone, Copy, Default)] +#[derive(Debug, Clone, Copy)] #[cfg_attr(test, derive(PartialEq, Eq))] -pub struct AssetInfo { +pub struct AssetInfo { /// The amount of the asset. pub amount: NoteValue, /// Whether or not the asset is finalized. pub is_finalized: bool, - /// A reference note, which may be [`Note`] (for bundles/global state) - /// or [`Option`] (for actions). - pub reference_note: R, + /// A reference note + pub reference_note: Note, } -/// For bundles and global state, the reference note is always provided. -pub type BundleAssetInfo = AssetInfo; - -/// For actions, the reference note may be omitted if this asset is already known. -pub type ActionAssetInfo = AssetInfo>; - -impl BundleAssetInfo { - /// Creates a new [`AssetInfo`] instance for an `IssueBundle`, - /// where a reference note is always specified. +impl AssetInfo { + /// Creates a new [`AssetInfo`] instance. pub fn new(amount: NoteValue, is_finalized: bool, reference_note: Note) -> Self { Self { amount, @@ -38,15 +27,3 @@ impl BundleAssetInfo { } } } - -impl ActionAssetInfo { - /// Creates a new [`AssetInfo`] instance for an `IssueAction`, - /// where the reference note can be omitted if this is not the first issuance. - pub fn new(amount: NoteValue, is_finalized: bool, reference_note: Option) -> Self { - Self { - amount, - is_finalized, - reference_note, - } - } -} From acbb6b27a3cdbba7f1d43ca74701750e342f39a2 Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Tue, 28 Jan 2025 21:03:44 +0100 Subject: [PATCH 07/28] Add FIXME comment --- src/issuance.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/issuance.rs b/src/issuance.rs index 730eb211e..1d1834763 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -148,6 +148,9 @@ impl IssueAction { return Err(AssetBaseCannotBeIdentityPoint); } + // FIXME: Refactored this to use "if ..." instead of a chain with "then()" + // method to unify the approach with other "ifs" in this function and + // verify_issue_bundle function. // All assets should be derived correctly if note.asset() != issue_asset { return Err(IssueBundleIkMismatchAssetBase); From 4dce9d5ac627259597a6a6eaeed2efcd773deeed Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Tue, 28 Jan 2025 21:13:49 +0100 Subject: [PATCH 08/28] Update FIXME comments --- src/issuance.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/issuance.rs b/src/issuance.rs index 1d1834763..7445fb59a 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -148,9 +148,10 @@ impl IssueAction { return Err(AssetBaseCannotBeIdentityPoint); } - // FIXME: Refactored this to use "if ..." instead of a chain with "then()" - // method to unify the approach with other "ifs" in this function and - // verify_issue_bundle function. + // FIXME: I refactored this to use "if ..." instead of chaining with the "then()" + // method to unify the approach with other "if" statements in this function and + // the `verify_issue_bundle` function. + // All assets should be derived correctly if note.asset() != issue_asset { return Err(IssueBundleIkMismatchAssetBase); @@ -672,8 +673,8 @@ pub fn verify_issue_bundle( Ok(verified_asset_states) } -// FIXME: Why IssueActionPreviouslyFinalizedAssetBase contains AssetBase but -// WrongAssetDescSize and new MissingReferenceNoteOnFirstIssuance don't? +// FIXME: Why does IssueActionPreviouslyFinalizedAssetBase contain AssetBase, but +// WrongAssetDescSize and MissingReferenceNoteOnFirstIssuance do not? /// Errors produced during the issuance process #[derive(Debug, PartialEq, Eq)] pub enum Error { @@ -690,7 +691,7 @@ pub enum Error { /// It cannot be first issuance because we have already some notes for this asset. CannotBeFirstIssuance, - // FIXME: split the group comment ("Verification errors")? + // FIXME: Split the group comment ("Verification errors")? /// Verification errors: /// Invalid signature. IssueBundleInvalidSignature, @@ -759,8 +760,6 @@ impl fmt::Display for Error { } } -// FIXME: Add more tests for issued_assets (i.e. "global state") change after verify_issue_bundle -// is called: 1) check for expected output, 2) check for processing of existing assets etc. #[cfg(test)] mod tests { use super::{AssetInfo, IssueBundle, IssueInfo}; @@ -1297,6 +1296,8 @@ mod tests { assert!(prev_finalized.contains(&AssetBase::derive(&ik, b"Verify with finalize"))); } + // FIXME: Make it a workflow test: perform a series of bundle creations and verifications, + // with a global state simulation #[test] fn issue_bundle_verify_with_issued_assets() { let (rng, isk, ik, recipient, sighash, first_nullifier) = setup_params(); From 1fe70572274c632b8407007eeda3c81babe74837 Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Wed, 29 Jan 2025 09:33:16 +0100 Subject: [PATCH 09/28] get_finalization_set test helper function removed, issue bundle verification test functions updated to check the result of the verify_issue_bundle directly --- src/issuance.rs | 43 ++++++++++++++++++------------------------- 1 file changed, 18 insertions(+), 25 deletions(-) diff --git a/src/issuance.rs b/src/issuance.rs index 7445fb59a..dfe969c08 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -790,7 +790,7 @@ mod tests { use pasta_curves::pallas::{Point, Scalar}; use rand::rngs::OsRng; use rand::RngCore; - use std::collections::{HashMap, HashSet}; + use std::collections::HashMap; /// Validation for reference note /// @@ -912,17 +912,6 @@ mod tests { (isk, bundle, sighash, first_nullifier) } - // Creates a set of finalized assets from a map of issued assets to support the new return type - // of the `verify_issue_bundle` function. - // TODO: Adapt issue bundle verification test functions to the new return type and remove this - // function. - fn get_finalization_set(issued_assets: &HashMap) -> HashSet { - issued_assets - .iter() - .filter_map(|(asset, asset_info)| asset_info.is_finalized.then(|| asset.clone())) - .collect::>() - } - #[test] fn action_verify_valid() { let (ik, test_asset, action) = action_verify_test_params(10, 20, b"Asset 1", None, false); @@ -1242,7 +1231,7 @@ mod tests { let (rng, isk, ik, recipient, sighash, first_nullifier) = setup_params(); let (bundle, _) = IssueBundle::new( - ik, + ik.clone(), b"Verify".to_vec(), Some(IssueInfo { recipient, @@ -1260,9 +1249,15 @@ mod tests { .unwrap(); let issued_assets = verify_issue_bundle(&signed, sighash, |_| None).unwrap(); - let prev_finalized = get_finalization_set(&issued_assets); - assert!(prev_finalized.is_empty()); + let first_note = *signed.actions().first().notes().first().unwrap(); + assert_eq!( + issued_assets, + HashMap::from([( + AssetBase::derive(&ik, b"Verify"), + AssetInfo::new(NoteValue::from_raw(5), false, first_note) + )]) + ); } #[test] @@ -1290,10 +1285,15 @@ mod tests { .unwrap(); let issued_assets = verify_issue_bundle(&signed, sighash, |_| None).unwrap(); - let prev_finalized = get_finalization_set(&issued_assets); - assert_eq!(prev_finalized.len(), 1); - assert!(prev_finalized.contains(&AssetBase::derive(&ik, b"Verify with finalize"))); + let first_note = *signed.actions().first().notes().first().unwrap(); + assert_eq!( + issued_assets, + HashMap::from([( + AssetBase::derive(&ik, b"Verify with finalize"), + AssetInfo::new(NoteValue::from_raw(7), true, first_note) + )]) + ); } // FIXME: Make it a workflow test: perform a series of bundle creations and verifications, @@ -1345,13 +1345,6 @@ mod tests { .unwrap(); let issued_assets = verify_issue_bundle(&signed, sighash, |_| None).unwrap(); - let prev_finalized = get_finalization_set(&issued_assets); - - assert_eq!(prev_finalized.len(), 2); - - assert!(prev_finalized.contains(&asset1_base)); - assert!(prev_finalized.contains(&asset2_base)); - assert!(!prev_finalized.contains(&asset3_base)); assert_eq!(issued_assets.keys().len(), 3); From 991368b1e68f0e3d877f68db949563f0098a6554 Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Wed, 29 Jan 2025 14:48:21 +0100 Subject: [PATCH 10/28] Add an issuance workflow test (issue_bundle_verify_with_global_state) --- src/issuance.rs | 183 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 180 insertions(+), 3 deletions(-) diff --git a/src/issuance.rs b/src/issuance.rs index dfe969c08..a6a81505d 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -769,7 +769,7 @@ mod tests { issuance::Error::{ AssetBaseCannotBeIdentityPoint, IssueActionNotFound, IssueActionPreviouslyFinalizedAssetBase, IssueBundleIkMismatchAssetBase, - IssueBundleInvalidSignature, WrongAssetDescSize, + IssueBundleInvalidSignature, MissingReferenceNoteOnFirstIssuance, WrongAssetDescSize, }, issuance::{ is_reference_note, verify_issue_bundle, AwaitingNullifier, IssueAction, Signed, @@ -1296,8 +1296,6 @@ mod tests { ); } - // FIXME: Make it a workflow test: perform a series of bundle creations and verifications, - // with a global state simulation #[test] fn issue_bundle_verify_with_issued_assets() { let (rng, isk, ik, recipient, sighash, first_nullifier) = setup_params(); @@ -1378,6 +1376,185 @@ mod tests { ); } + // Issuance workflow test: performs a series of bundle creations and verifications, + // with a global state simulation + #[test] + fn issue_bundle_verify_with_global_state() { + type GlobalState = HashMap; + + fn first_note(bundle: &IssueBundle, action_index: usize) -> Note { + bundle.actions()[action_index].notes()[0] + } + + fn build_expected_global_state(data: &[(&AssetBase, u64, bool, Note)]) -> GlobalState { + data.into_iter() + .cloned() + .map(|(asset_base, amount, is_finalized, reference_note)| { + ( + asset_base.clone(), + AssetInfo::new(NoteValue::from_raw(amount), is_finalized, reference_note), + ) + }) + .collect::>() + } + + let (rng, isk, ik, recipient, sighash, first_nullifier) = setup_params(); + + // A closure to build an issue bundle using parameters from `setup_params`. + // Using a closure avoids passing `rng`, `ik`, etc. each time. + let build_issue_bundle = |data: &[(&Vec, u64, bool, bool)]| -> IssueBundle { + let (asset_desc, amount, first_issuance, is_finalized) = data.first().unwrap().clone(); + + let (mut bundle, _) = IssueBundle::new( + ik.clone(), + asset_desc.clone(), + Some(IssueInfo { + recipient, + value: NoteValue::from_raw(amount), + }), + first_issuance, + rng, + ) + .unwrap(); + + if is_finalized { + bundle.finalize_action(asset_desc).unwrap(); + } + + for (asset_desc, amount, first_issuance, is_finalized) in + data.into_iter().skip(1).cloned() + { + bundle + .add_recipient( + &asset_desc, + recipient, + NoteValue::from_raw(amount), + first_issuance, + rng, + ) + .unwrap(); + + if is_finalized { + bundle.finalize_action(asset_desc).unwrap(); + } + } + + bundle + .update_rho(&first_nullifier) + .prepare(sighash) + .sign(&isk) + .unwrap() + }; + + let asset1_desc = b"Verify with issued assets 1".to_vec(); + let asset2_desc = b"Verify with issued assets 2".to_vec(); + let asset3_desc = b"Verify with issued assets 3".to_vec(); + let asset4_desc = b"Verify with issued assets 4".to_vec(); + + let asset1_base = AssetBase::derive(&ik, &asset1_desc); + let asset2_base = AssetBase::derive(&ik, &asset2_desc); + let asset3_base = AssetBase::derive(&ik, &asset3_desc); + let asset4_base = AssetBase::derive(&ik, &asset4_desc); + + let mut global_state = GlobalState::new(); + + // We'll issue and verify a series of bundles. For valid bundles, the global + // state is updated and must match the expected result. For invalid bundles, + // we check the expected error, leaving the state unchanged. + + // ** Bundle1 (valid) ** + + let bundle1 = build_issue_bundle(&[ + (&asset1_desc, 7, true, false), + (&asset1_desc, 8, false, false), + (&asset2_desc, 10, true, true), + (&asset3_desc, 5, true, false), + ]); + + let expected_global_state1 = build_expected_global_state(&[ + (&asset1_base, 15, false, first_note(&bundle1, 0)), + (&asset2_base, 10, true, first_note(&bundle1, 1)), + (&asset3_base, 5, false, first_note(&bundle1, 2)), + ]); + + global_state.extend( + verify_issue_bundle(&bundle1, sighash, |asset| global_state.get(asset).cloned()) + .unwrap(), + ); + assert_eq!(global_state, expected_global_state1); + + // ** Bundle2 (valid) ** + + let bundle2 = build_issue_bundle(&[ + (&asset1_desc, 3, true, true), + (&asset3_desc, 20, false, false), + ]); + + let expected_global_state2 = build_expected_global_state(&[ + (&asset1_base, 18, true, first_note(&bundle1, 0)), + (&asset2_base, 10, true, first_note(&bundle1, 1)), + (&asset3_base, 25, false, first_note(&bundle1, 2)), + ]); + + global_state.extend( + verify_issue_bundle(&bundle2, sighash, |asset| global_state.get(asset).cloned()) + .unwrap(), + ); + assert_eq!(global_state, expected_global_state2); + + // ** Bundle3 (invalid) ** + + let bundle3 = build_issue_bundle(&[ + (&asset1_desc, 3, true, false), + (&asset3_desc, 20, false, false), + ]); + + let expected_global_state3 = expected_global_state2; + + assert_eq!( + verify_issue_bundle(&bundle3, sighash, |asset| global_state.get(asset).cloned()) + .unwrap_err(), + IssueActionPreviouslyFinalizedAssetBase(asset1_base), + ); + assert_eq!(global_state, expected_global_state3); + + // ** Bundle4 (invalid) ** + + let bundle4 = build_issue_bundle(&[ + (&asset3_desc, 50, true, true), + (&asset4_desc, 77, false, false), + ]); + + let expected_global_state4 = expected_global_state3; + + assert_eq!( + verify_issue_bundle(&bundle4, sighash, |asset| global_state.get(asset).cloned()) + .unwrap_err(), + MissingReferenceNoteOnFirstIssuance, + ); + assert_eq!(global_state, expected_global_state4); + + // ** Bundle5 (valid) ** + + let bundle5 = build_issue_bundle(&[ + (&asset3_desc, 50, true, true), + (&asset4_desc, 77, true, false), + ]); + + let expected_global_state5 = build_expected_global_state(&[ + (&asset1_base, 18, true, first_note(&bundle1, 0)), + (&asset2_base, 10, true, first_note(&bundle1, 1)), + (&asset3_base, 75, true, first_note(&bundle1, 2)), + (&asset4_base, 77, false, first_note(&bundle5, 1)), + ]); + + global_state.extend( + verify_issue_bundle(&bundle5, sighash, |asset| global_state.get(asset).cloned()) + .unwrap(), + ); + assert_eq!(global_state, expected_global_state5); + } + #[test] fn issue_bundle_verify_fail_previously_finalized() { let (mut rng, isk, ik, recipient, sighash, first_nullifier) = setup_params(); From 9016d1851e92dad2ca2480d7d46c95905420d280 Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Wed, 29 Jan 2025 17:20:01 +0100 Subject: [PATCH 11/28] Add a case for ValueOverflow error to the issuance workflow test --- src/issuance.rs | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/issuance.rs b/src/issuance.rs index a6a81505d..2508fd7f9 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -769,7 +769,8 @@ mod tests { issuance::Error::{ AssetBaseCannotBeIdentityPoint, IssueActionNotFound, IssueActionPreviouslyFinalizedAssetBase, IssueBundleIkMismatchAssetBase, - IssueBundleInvalidSignature, MissingReferenceNoteOnFirstIssuance, WrongAssetDescSize, + IssueBundleInvalidSignature, MissingReferenceNoteOnFirstIssuance, ValueOverflow, + WrongAssetDescSize, }, issuance::{ is_reference_note, verify_issue_bundle, AwaitingNullifier, IssueAction, Signed, @@ -1534,25 +1535,41 @@ mod tests { ); assert_eq!(global_state, expected_global_state4); - // ** Bundle5 (valid) ** + // ** Bundle5 (invalid) ** let bundle5 = build_issue_bundle(&[ + (&asset3_desc, u64::MAX - 20, true, true), + (&asset4_desc, 77, true, false), + ]); + + let expected_global_state5 = expected_global_state4; + + assert_eq!( + verify_issue_bundle(&bundle5, sighash, |asset| global_state.get(asset).cloned()) + .unwrap_err(), + ValueOverflow, + ); + assert_eq!(global_state, expected_global_state5); + + // ** Bundle6 (valid) ** + + let bundle6 = build_issue_bundle(&[ (&asset3_desc, 50, true, true), (&asset4_desc, 77, true, false), ]); - let expected_global_state5 = build_expected_global_state(&[ + let expected_global_state6 = build_expected_global_state(&[ (&asset1_base, 18, true, first_note(&bundle1, 0)), (&asset2_base, 10, true, first_note(&bundle1, 1)), (&asset3_base, 75, true, first_note(&bundle1, 2)), - (&asset4_base, 77, false, first_note(&bundle5, 1)), + (&asset4_base, 77, false, first_note(&bundle6, 1)), ]); global_state.extend( - verify_issue_bundle(&bundle5, sighash, |asset| global_state.get(asset).cloned()) + verify_issue_bundle(&bundle6, sighash, |asset| global_state.get(asset).cloned()) .unwrap(), ); - assert_eq!(global_state, expected_global_state5); + assert_eq!(global_state, expected_global_state6); } #[test] From 888022bbb2ef1ff17105a87be0ad0a8c2b045510 Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Thu, 30 Jan 2025 13:23:57 +0100 Subject: [PATCH 12/28] Update comments and remove asset from IssueActionPreviouslyFinalizedAssetBase error according to #138 PR review comments --- src/issuance.rs | 62 +++++++++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/src/issuance.rs b/src/issuance.rs index 2508fd7f9..f3bfcdb0d 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -1,4 +1,17 @@ -//! Structs related to issuance bundles and the associated logic. +//! Issuance logic for Zcash Shielded Assets (ZSAs). +//! +//! This module defines structures and methods for creating, authorizing, and verifying +//! issuance bundles, which introduce new shielded assets into the Orchard protocol. +//! +//! The core components include: +//! - `IssueBundle`: Represents a collection of issuance actions with authorization states. +//! - `IssueAction`: Defines an individual issuance event, including asset details and notes. +//! - `IssueAuth` variants: Track issuance states from creation to finalization. +//! - `verify_issue_bundle`: Ensures issuance validity and prevents unauthorized asset creation. +//! +//! Errors related to issuance, such as invalid signatures or supply overflows, +//! are handled through the `Error` enum. + use blake2b_simd::Hash as Blake2bHash; use group::Group; use k256::schnorr; @@ -118,18 +131,18 @@ impl IssueAction { /// /// # Returns /// - /// A `Result` containing a tuple with an `AssetBase` and an `ActionAssetInfo`, or an `Error`. + /// A `Result` containing a tuple with an `AssetBase` and an `NoteValue`, or an `Error`. /// /// # Errors /// /// This function may return an error in any of the following cases: /// - /// * `ValueOverflow`: If the total amount value of all notes in the `IssueAction` overflows. + /// * `ValueOverflow`: The total amount value of all notes in the `IssueAction` overflows. /// - /// * `IssueBundleIkMismatchAssetBase`: If the provided `ik` is not used to derive the + /// * `IssueBundleIkMismatchAssetBase`: The provided `ik` is not used to derive the /// `AssetBase` for **all** internal notes. /// - /// * `IssueActionWithoutNoteNotFinalized`:If the `IssueAction` contains no note and is not finalized. + /// * `IssueActionWithoutNoteNotFinalized`: The `IssueAction` contains no notes and is not finalized. fn verify(&self, ik: &IssuanceValidatingKey) -> Result<(AssetBase, NoteValue), Error> { if self.notes.is_empty() && !self.is_finalized() { return Err(IssueActionWithoutNoteNotFinalized); @@ -148,10 +161,6 @@ impl IssueAction { return Err(AssetBaseCannotBeIdentityPoint); } - // FIXME: I refactored this to use "if ..." instead of chaining with the "then()" - // method to unify the approach with other "if" statements in this function and - // the `verify_issue_bundle` function. - // All assets should be derived correctly if note.asset() != issue_asset { return Err(IssueBundleIkMismatchAssetBase); @@ -272,8 +281,8 @@ impl IssueBundle { /// /// # Returns /// - /// If a single matching action is found, it is returned as `Some(&IssueAction)`. - /// If no action matches the given Asset Base `asset`, it returns `None`. + /// Returns `Some(&IssueAction)` if a single matching action is found. + /// Returns `None` if no action matches the given asset base. /// /// # Panics /// @@ -340,7 +349,7 @@ impl IssueBundle { /// /// This function may return an error in any of the following cases: /// - /// * `WrongAssetDescSize`: If `asset_desc` is empty or longer than 512 bytes. + /// * `WrongAssetDescSize`: The `asset_desc` is empty or longer than 512 bytes. pub fn new( ik: IssuanceValidatingKey, asset_desc: Vec, @@ -404,7 +413,7 @@ impl IssueBundle { /// /// This function may return an error in any of the following cases: /// - /// * `WrongAssetDescSize`: If `asset_desc` is empty or longer than 512 bytes. + /// * `WrongAssetDescSize`: The `asset_desc` is empty or longer than 512 bytes. pub fn add_recipient( &mut self, asset_desc: &[u8], @@ -590,7 +599,7 @@ impl IssueBundle { /// - Runs all checks within the `IssueAction::verify` method. /// - 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 issuances may omit it. +/// - Requires a reference note for the *first issuance* of an asset; subsequent issuance may omit it. /// /// # Arguments /// @@ -608,12 +617,12 @@ impl IssueBundle { /// /// # Errors /// -/// - [`IssueBundleInvalidSignature`]: If the signature verification for the provided `sighash` fails. -/// - [`WrongAssetDescSize`]: If the asset description size is invalid. -/// - [`ValueOverflow`]: If adding the new amount to the existing total supply causes an overflow. -/// - [`IssueActionPreviouslyFinalizedAssetBase`]: If an action is attempted on an asset that has +/// - [`IssueBundleInvalidSignature`]: Signature verification for the provided `sighash` fails. +/// - [`WrongAssetDescSize`]: The asset description size is invalid. +/// - [`ValueOverflow`]: adding the new amount to the existing total supply causes an overflow. +/// - [`IssueActionPreviouslyFinalizedAssetBase`]: An action is attempted on an asset that has /// already been finalized. -/// - [`MissingReferenceNoteOnFirstIssuance`]: If no reference note is provided for the first +/// - [`MissingReferenceNoteOnFirstIssuance`]: No reference note is provided for the first /// issuance of a new asset. /// - **Other Errors**: Any additional errors returned by the `IssueAction::verify` method are /// propagated @@ -652,13 +661,13 @@ pub fn verify_issue_bundle( *action_reference_note.ok_or(MissingReferenceNoteOnFirstIssuance)?, ), - // Subsequent issuances of the asset + // Subsequent issuance of the asset Some(prev_asset_state) => { let amount = (prev_asset_state.amount + action_amount).ok_or(ValueOverflow)?; if prev_asset_state.is_finalized { - return Err(IssueActionPreviouslyFinalizedAssetBase(asset)); + return Err(IssueActionPreviouslyFinalizedAssetBase); } AssetInfo::new(amount, action_is_finalized, prev_asset_state.reference_note) @@ -673,8 +682,6 @@ pub fn verify_issue_bundle( Ok(verified_asset_states) } -// FIXME: Why does IssueActionPreviouslyFinalizedAssetBase contain AssetBase, but -// WrongAssetDescSize and MissingReferenceNoteOnFirstIssuance do not? /// Errors produced during the issuance process #[derive(Debug, PartialEq, Eq)] pub enum Error { @@ -691,12 +698,11 @@ pub enum Error { /// It cannot be first issuance because we have already some notes for this asset. CannotBeFirstIssuance, - // FIXME: Split the group comment ("Verification errors")? /// Verification errors: /// Invalid signature. IssueBundleInvalidSignature, /// The provided `AssetBase` has been previously finalized. - IssueActionPreviouslyFinalizedAssetBase(AssetBase), + IssueActionPreviouslyFinalizedAssetBase, /// Overflow error occurred while calculating the value of the asset ValueOverflow, @@ -741,7 +747,7 @@ impl fmt::Display for Error { IssueBundleInvalidSignature => { write!(f, "invalid signature") } - IssueActionPreviouslyFinalizedAssetBase(_) => { + IssueActionPreviouslyFinalizedAssetBase => { write!(f, "the provided `AssetBase` has been previously finalized") } ValueOverflow => { @@ -1515,7 +1521,7 @@ mod tests { assert_eq!( verify_issue_bundle(&bundle3, sighash, |asset| global_state.get(asset).cloned()) .unwrap_err(), - IssueActionPreviouslyFinalizedAssetBase(asset1_base), + IssueActionPreviouslyFinalizedAssetBase, ); assert_eq!(global_state, expected_global_state3); @@ -1616,7 +1622,7 @@ mod tests { assert_eq!( verify_issue_bundle(&signed, sighash, |asset| issued_assets.get(asset).copied()) .unwrap_err(), - IssueActionPreviouslyFinalizedAssetBase(final_type) + IssueActionPreviouslyFinalizedAssetBase ); } From 5a30c2f7c407237a43019633ebb96d450e5af169 Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Thu, 30 Jan 2025 13:28:11 +0100 Subject: [PATCH 13/28] Refactor verify_issue_bundle according to #138 PR review --- src/issuance.rs | 70 ++++++++++++++++++++++++------------------------- 1 file changed, 34 insertions(+), 36 deletions(-) diff --git a/src/issuance.rs b/src/issuance.rs index f3bfcdb0d..add8d820f 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -29,7 +29,7 @@ use crate::note::{AssetBase, Nullifier, Rho}; use crate::value::NoteValue; use crate::{Address, Note}; -use crate::supply_info::AssetInfo; +use crate::supply_info::AssetRecord; use Error::{ AssetBaseCannotBeIdentityPoint, CannotBeFirstIssuance, IssueActionNotFound, @@ -606,14 +606,14 @@ impl IssueBundle { /// - `bundle`: A reference to the [`IssueBundle`] to be validated. /// - `sighash`: A 32-byte array representing the `sighash` used to verify the bundle's signature. /// - `get_global_asset_state`: A closure that takes a reference to an [`AssetBase`] and returns an -/// [`Option`], representing the current state of the asset from a global store +/// [`Option`], representing the current state of the asset from a global store /// of previously issued assets. /// /// # Returns /// -/// A `Result` containing a [`HashMap`] upon success, where each key-value +/// A `Result` containing a [`HashMap`] upon success, where each key-value /// pair represents the new or updated state of an asset. The key is an [`AssetBase`], and the value -/// is the corresponding updated [`AssetInfo`]. +/// is the corresponding updated [`AssetRecord`]. /// /// # Errors /// @@ -629,59 +629,57 @@ impl IssueBundle { pub fn verify_issue_bundle( bundle: &IssueBundle, sighash: [u8; 32], - get_global_asset_state: impl Fn(&AssetBase) -> Option, -) -> Result, Error> { + get_global_records: impl Fn(&AssetBase) -> Option, +) -> Result, Error> { bundle .ik() .verify(&sighash, bundle.authorization().signature()) .map_err(|_| IssueBundleInvalidSignature)?; - let verified_asset_states = + let new_records = bundle .actions() .iter() - .try_fold(HashMap::new(), |mut verified_asset_states, action| { + .try_fold(HashMap::new(), |mut new_records, action| { if !is_asset_desc_of_valid_size(action.asset_desc()) { return Err(WrongAssetDescSize); } - let (asset, action_amount) = action.verify(bundle.ik())?; - let action_is_finalized = action.is_finalized(); - let action_reference_note = action.get_reference_note(); + let (asset, amount) = action.verify(bundle.ik())?; + let is_finalized = action.is_finalized(); + let ref_note = action.get_reference_note(); - let verified_asset_state = match verified_asset_states + let new_asset_record = match new_records .get(&asset) .cloned() - .or_else(|| get_global_asset_state(&asset)) + .or_else(|| get_global_records(&asset)) { // The first issuance of the asset - None => AssetInfo::new( - action_amount, - action_is_finalized, - *action_reference_note.ok_or(MissingReferenceNoteOnFirstIssuance)?, + None => AssetRecord::new( + amount, + is_finalized, + *ref_note.ok_or(MissingReferenceNoteOnFirstIssuance)?, ), - // Subsequent issuance of the asset - Some(prev_asset_state) => { - let amount = - (prev_asset_state.amount + action_amount).ok_or(ValueOverflow)?; + // Subsequent issuances of the asset + Some(current_record) => { + let amount = (current_record.amount + amount).ok_or(ValueOverflow)?; - if prev_asset_state.is_finalized { + if current_record.is_finalized { return Err(IssueActionPreviouslyFinalizedAssetBase); } - AssetInfo::new(amount, action_is_finalized, prev_asset_state.reference_note) + AssetRecord::new(amount, is_finalized, current_record.reference_note) } }; - verified_asset_states.insert(asset, verified_asset_state); + new_records.insert(asset, new_asset_record); - Ok(verified_asset_states) + Ok(new_records) })?; - Ok(verified_asset_states) + Ok(new_records) } - /// Errors produced during the issuance process #[derive(Debug, PartialEq, Eq)] pub enum Error { @@ -768,7 +766,7 @@ impl fmt::Display for Error { #[cfg(test)] mod tests { - use super::{AssetInfo, IssueBundle, IssueInfo}; + use super::{AssetRecord, IssueBundle, IssueInfo}; use crate::{ builder::{Builder, BundleType}, circuit::ProvingKey, @@ -1262,7 +1260,7 @@ mod tests { issued_assets, HashMap::from([( AssetBase::derive(&ik, b"Verify"), - AssetInfo::new(NoteValue::from_raw(5), false, first_note) + AssetRecord::new(NoteValue::from_raw(5), false, first_note) )]) ); } @@ -1298,7 +1296,7 @@ mod tests { issued_assets, HashMap::from([( AssetBase::derive(&ik, b"Verify with finalize"), - AssetInfo::new(NoteValue::from_raw(7), true, first_note) + AssetRecord::new(NoteValue::from_raw(7), true, first_note) )]) ); } @@ -1359,7 +1357,7 @@ mod tests { assert_eq!( issued_assets.get(&asset1_base), - Some(&AssetInfo::new( + Some(&AssetRecord::new( NoteValue::from_raw(15), true, reference_note1 @@ -1367,7 +1365,7 @@ mod tests { ); assert_eq!( issued_assets.get(&asset2_base), - Some(&AssetInfo::new( + Some(&AssetRecord::new( NoteValue::from_raw(10), true, reference_note2 @@ -1375,7 +1373,7 @@ mod tests { ); assert_eq!( issued_assets.get(&asset3_base), - Some(&AssetInfo::new( + Some(&AssetRecord::new( NoteValue::from_raw(5), false, reference_note3 @@ -1387,7 +1385,7 @@ mod tests { // with a global state simulation #[test] fn issue_bundle_verify_with_global_state() { - type GlobalState = HashMap; + type GlobalState = HashMap; fn first_note(bundle: &IssueBundle, action_index: usize) -> Note { bundle.actions()[action_index].notes()[0] @@ -1399,7 +1397,7 @@ mod tests { .map(|(asset_base, amount, is_finalized, reference_note)| { ( asset_base.clone(), - AssetInfo::new(NoteValue::from_raw(amount), is_finalized, reference_note), + AssetRecord::new(NoteValue::from_raw(amount), is_finalized, reference_note), ) }) .collect::>() @@ -1604,7 +1602,7 @@ mod tests { let issued_assets = [( final_type, - AssetInfo::new( + AssetRecord::new( NoteValue::from_raw(20), true, Note::new( From f87de8ea7becdf7dd59fce3c3db52882ab0ac206 Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Thu, 30 Jan 2025 13:29:37 +0100 Subject: [PATCH 14/28] Simplify verify_issue_bundle's return --- src/issuance.rs | 73 ++++++++++++++++++++++++------------------------- 1 file changed, 35 insertions(+), 38 deletions(-) diff --git a/src/issuance.rs b/src/issuance.rs index add8d820f..7c0817bd8 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -636,49 +636,46 @@ pub fn verify_issue_bundle( .verify(&sighash, bundle.authorization().signature()) .map_err(|_| IssueBundleInvalidSignature)?; - let new_records = - bundle - .actions() - .iter() - .try_fold(HashMap::new(), |mut new_records, action| { - if !is_asset_desc_of_valid_size(action.asset_desc()) { - return Err(WrongAssetDescSize); - } + bundle + .actions() + .iter() + .try_fold(HashMap::new(), |mut new_records, action| { + if !is_asset_desc_of_valid_size(action.asset_desc()) { + return Err(WrongAssetDescSize); + } + + let (asset, amount) = action.verify(bundle.ik())?; + let is_finalized = action.is_finalized(); + let ref_note = action.get_reference_note(); + + let new_asset_record = match new_records + .get(&asset) + .cloned() + .or_else(|| get_global_records(&asset)) + { + // The first issuance of the asset + None => AssetRecord::new( + amount, + is_finalized, + *ref_note.ok_or(MissingReferenceNoteOnFirstIssuance)?, + ), + + // Subsequent issuances of the asset + Some(current_record) => { + let amount = (current_record.amount + amount).ok_or(ValueOverflow)?; - let (asset, amount) = action.verify(bundle.ik())?; - let is_finalized = action.is_finalized(); - let ref_note = action.get_reference_note(); - - let new_asset_record = match new_records - .get(&asset) - .cloned() - .or_else(|| get_global_records(&asset)) - { - // The first issuance of the asset - None => AssetRecord::new( - amount, - is_finalized, - *ref_note.ok_or(MissingReferenceNoteOnFirstIssuance)?, - ), - - // Subsequent issuances of the asset - Some(current_record) => { - let amount = (current_record.amount + amount).ok_or(ValueOverflow)?; - - if current_record.is_finalized { - return Err(IssueActionPreviouslyFinalizedAssetBase); - } - - AssetRecord::new(amount, is_finalized, current_record.reference_note) + if current_record.is_finalized { + return Err(IssueActionPreviouslyFinalizedAssetBase); } - }; - new_records.insert(asset, new_asset_record); + AssetRecord::new(amount, is_finalized, current_record.reference_note) + } + }; - Ok(new_records) - })?; + new_records.insert(asset, new_asset_record); - Ok(new_records) + Ok(new_records) + }) } /// Errors produced during the issuance process #[derive(Debug, PartialEq, Eq)] From 422e2fc8b2aeba90d28888c53be9f89c5a10ad5d Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Thu, 30 Jan 2025 14:09:56 +0100 Subject: [PATCH 15/28] Make build_issue_bundle (in issuance.rs issue_bundle_verify_with_global_state test) a function instead of closure according to #138 PR review comment --- src/issuance.rs | 92 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 61 insertions(+), 31 deletions(-) diff --git a/src/issuance.rs b/src/issuance.rs index 7c0817bd8..e39391220 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -1400,11 +1400,19 @@ mod tests { .collect::>() } - let (rng, isk, ik, recipient, sighash, first_nullifier) = setup_params(); + fn build_issue_bundle( + params: &( + OsRng, + IssuanceAuthorizingKey, + IssuanceValidatingKey, + Address, + [u8; 32], + Nullifier, + ), + data: &[(&Vec, u64, bool, bool)], + ) -> IssueBundle { + let (rng, ref isk, ref ik, recipient, sighash, ref first_nullifier) = *params; - // A closure to build an issue bundle using parameters from `setup_params`. - // Using a closure avoids passing `rng`, `ik`, etc. each time. - let build_issue_bundle = |data: &[(&Vec, u64, bool, bool)]| -> IssueBundle { let (asset_desc, amount, first_issuance, is_finalized) = data.first().unwrap().clone(); let (mut bundle, _) = IssueBundle::new( @@ -1446,7 +1454,11 @@ mod tests { .prepare(sighash) .sign(&isk) .unwrap() - }; + } + + let params = setup_params(); + + let (_, _, ik, _, sighash, _) = params.clone(); let asset1_desc = b"Verify with issued assets 1".to_vec(); let asset2_desc = b"Verify with issued assets 2".to_vec(); @@ -1466,12 +1478,15 @@ mod tests { // ** Bundle1 (valid) ** - let bundle1 = build_issue_bundle(&[ - (&asset1_desc, 7, true, false), - (&asset1_desc, 8, false, false), - (&asset2_desc, 10, true, true), - (&asset3_desc, 5, true, false), - ]); + let bundle1 = build_issue_bundle( + ¶ms, + &[ + (&asset1_desc, 7, true, false), + (&asset1_desc, 8, false, false), + (&asset2_desc, 10, true, true), + (&asset3_desc, 5, true, false), + ], + ); let expected_global_state1 = build_expected_global_state(&[ (&asset1_base, 15, false, first_note(&bundle1, 0)), @@ -1487,10 +1502,13 @@ mod tests { // ** Bundle2 (valid) ** - let bundle2 = build_issue_bundle(&[ - (&asset1_desc, 3, true, true), - (&asset3_desc, 20, false, false), - ]); + let bundle2 = build_issue_bundle( + ¶ms, + &[ + (&asset1_desc, 3, true, true), + (&asset3_desc, 20, false, false), + ], + ); let expected_global_state2 = build_expected_global_state(&[ (&asset1_base, 18, true, first_note(&bundle1, 0)), @@ -1506,10 +1524,13 @@ mod tests { // ** Bundle3 (invalid) ** - let bundle3 = build_issue_bundle(&[ - (&asset1_desc, 3, true, false), - (&asset3_desc, 20, false, false), - ]); + let bundle3 = build_issue_bundle( + ¶ms, + &[ + (&asset1_desc, 3, true, false), + (&asset3_desc, 20, false, false), + ], + ); let expected_global_state3 = expected_global_state2; @@ -1522,10 +1543,13 @@ mod tests { // ** Bundle4 (invalid) ** - let bundle4 = build_issue_bundle(&[ - (&asset3_desc, 50, true, true), - (&asset4_desc, 77, false, false), - ]); + let bundle4 = build_issue_bundle( + ¶ms, + &[ + (&asset3_desc, 50, true, true), + (&asset4_desc, 77, false, false), + ], + ); let expected_global_state4 = expected_global_state3; @@ -1538,10 +1562,13 @@ mod tests { // ** Bundle5 (invalid) ** - let bundle5 = build_issue_bundle(&[ - (&asset3_desc, u64::MAX - 20, true, true), - (&asset4_desc, 77, true, false), - ]); + let bundle5 = build_issue_bundle( + ¶ms, + &[ + (&asset3_desc, u64::MAX - 20, true, true), + (&asset4_desc, 77, true, false), + ], + ); let expected_global_state5 = expected_global_state4; @@ -1554,10 +1581,13 @@ mod tests { // ** Bundle6 (valid) ** - let bundle6 = build_issue_bundle(&[ - (&asset3_desc, 50, true, true), - (&asset4_desc, 77, true, false), - ]); + let bundle6 = build_issue_bundle( + ¶ms, + &[ + (&asset3_desc, 50, true, true), + (&asset4_desc, 77, true, false), + ], + ); let expected_global_state6 = build_expected_global_state(&[ (&asset1_base, 18, true, first_note(&bundle1, 0)), From 9ecbeeaedc6ac1c17f68e963293762bbb551f72f Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Thu, 30 Jan 2025 14:12:19 +0100 Subject: [PATCH 16/28] Add updated supply_info.rs --- src/supply_info.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/supply_info.rs b/src/supply_info.rs index 4707328ed..4f8c4bf2d 100644 --- a/src/supply_info.rs +++ b/src/supply_info.rs @@ -6,7 +6,7 @@ use crate::{value::NoteValue, Note}; /// and reference note. #[derive(Debug, Clone, Copy)] #[cfg_attr(test, derive(PartialEq, Eq))] -pub struct AssetInfo { +pub struct AssetRecord { /// The amount of the asset. pub amount: NoteValue, @@ -17,8 +17,8 @@ pub struct AssetInfo { pub reference_note: Note, } -impl AssetInfo { - /// Creates a new [`AssetInfo`] instance. +impl AssetRecord { + /// Creates a new [`AssetRecord`] instance. pub fn new(amount: NoteValue, is_finalized: bool, reference_note: Note) -> Self { Self { amount, From 81d7d096d48817f1b09a52603a95e6eac5f7246d Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Thu, 30 Jan 2025 14:27:26 +0100 Subject: [PATCH 17/28] is_asset_desc_of_valid_size check moved from verify_issue_bundle to IssueAction::verify, according to #138 PR review comment --- src/issuance.rs | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/src/issuance.rs b/src/issuance.rs index e39391220..89f4bba8d 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -125,6 +125,7 @@ impl IssueAction { /// of all its notes and ensures that all note types are equal. It returns the asset and /// its supply as a tuple (`AssetBase`, `NoteValue`) or an error if the asset was not /// properly derived or an overflow occurred during the supply amount calculation. + /// /// # Arguments /// /// * `ik` - A reference to the `IssuanceValidatingKey` used for deriving the asset. @@ -137,13 +138,16 @@ impl IssueAction { /// /// This function may return an error in any of the following cases: /// + /// * `WrongAssetDescSize`: The asset description size is invalid. /// * `ValueOverflow`: The total amount value of all notes in the `IssueAction` overflows. - /// /// * `IssueBundleIkMismatchAssetBase`: The provided `ik` is not used to derive the /// `AssetBase` for **all** internal notes. - /// /// * `IssueActionWithoutNoteNotFinalized`: The `IssueAction` contains no notes and is not finalized. fn verify(&self, ik: &IssuanceValidatingKey) -> Result<(AssetBase, NoteValue), Error> { + if !is_asset_desc_of_valid_size(self.asset_desc()) { + return Err(WrongAssetDescSize); + } + if self.notes.is_empty() && !self.is_finalized() { return Err(IssueActionWithoutNoteNotFinalized); } @@ -592,20 +596,20 @@ impl IssueBundle { /// Validates an [`IssueBundle`] by performing the following checks: /// -/// - **Signature Verification**: +/// - **IssueBundle Auth signature verification**: /// - Ensures the signature on the provided `sighash` matches the bundle’s authorization. -/// - **IssueAction Verification**: -/// - Checks that the asset description size is correct. -/// - Runs all checks within the `IssueAction::verify` method. +/// - **Static IssueAction verification**: +/// - Runs checks using the `IssueAction::verify` method. +/// - **Node global state related verification**: /// - 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. /// /// # Arguments /// -/// - `bundle`: A reference to the [`IssueBundle`] to be validated. -/// - `sighash`: A 32-byte array representing the `sighash` used to verify the bundle's signature. -/// - `get_global_asset_state`: A closure that takes a reference to an [`AssetBase`] and returns an +/// * `bundle`: A reference to the [`IssueBundle`] to be validated. +/// * `sighash`: A 32-byte array representing the `sighash` used to verify the bundle's signature. +/// * `get_global_asset_state`: A closure that takes a reference to an [`AssetBase`] and returns an /// [`Option`], representing the current state of the asset from a global store /// of previously issued assets. /// @@ -617,14 +621,13 @@ impl IssueBundle { /// /// # Errors /// -/// - [`IssueBundleInvalidSignature`]: Signature verification for the provided `sighash` fails. -/// - [`WrongAssetDescSize`]: The asset description size is invalid. -/// - [`ValueOverflow`]: adding the new amount to the existing total supply causes an overflow. -/// - [`IssueActionPreviouslyFinalizedAssetBase`]: An action is attempted on an asset that has +/// * `IssueBundleInvalidSignature`: Signature verification for the provided `sighash` fails. +/// * `ValueOverflow`: adding the new amount to the existing total supply causes an overflow. +/// * `IssueActionPreviouslyFinalizedAssetBase`: An action is attempted on an asset that has /// already been finalized. -/// - [`MissingReferenceNoteOnFirstIssuance`]: No reference note is provided for the first +/// * `MissingReferenceNoteOnFirstIssuance`: No reference note is provided for the first /// issuance of a new asset. -/// - **Other Errors**: Any additional errors returned by the `IssueAction::verify` method are +/// * **Other Errors**: Any additional errors returned by the `IssueAction::verify` method are /// propagated pub fn verify_issue_bundle( bundle: &IssueBundle, @@ -640,10 +643,6 @@ pub fn verify_issue_bundle( .actions() .iter() .try_fold(HashMap::new(), |mut new_records, action| { - if !is_asset_desc_of_valid_size(action.asset_desc()) { - return Err(WrongAssetDescSize); - } - let (asset, amount) = action.verify(bundle.ik())?; let is_finalized = action.is_finalized(); let ref_note = action.get_reference_note(); From a8152fd17961823971bed0202339a7dd9d2ab8d0 Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Thu, 30 Jan 2025 14:52:38 +0100 Subject: [PATCH 18/28] Rename supply_info.rs to asset_record.rs --- src/{supply_info.rs => asset_record.rs} | 0 src/issuance.rs | 2 +- src/lib.rs | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename src/{supply_info.rs => asset_record.rs} (100%) diff --git a/src/supply_info.rs b/src/asset_record.rs similarity index 100% rename from src/supply_info.rs rename to src/asset_record.rs diff --git a/src/issuance.rs b/src/issuance.rs index 89f4bba8d..59706dff5 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -29,7 +29,7 @@ use crate::note::{AssetBase, Nullifier, Rho}; use crate::value::NoteValue; use crate::{Address, Note}; -use crate::supply_info::AssetRecord; +use crate::asset_record::AssetRecord; use Error::{ AssetBaseCannotBeIdentityPoint, CannotBeFirstIssuance, IssueActionNotFound, diff --git a/src/lib.rs b/src/lib.rs index 543c91825..52387f82c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,6 +18,7 @@ mod action; mod address; +pub mod asset_record; pub mod builder; pub mod bundle; pub mod circuit; @@ -29,7 +30,6 @@ pub mod note; pub mod orchard_flavor; pub mod primitives; mod spec; -pub mod supply_info; pub mod tree; pub mod value; pub mod zip32; From 09149a0c48d5f80512708cc157617c4dda04fa57 Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Thu, 30 Jan 2025 15:04:03 +0100 Subject: [PATCH 19/28] Minor: add new line --- src/issuance.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/issuance.rs b/src/issuance.rs index 59706dff5..c04db94cc 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -676,6 +676,7 @@ pub fn verify_issue_bundle( Ok(new_records) }) } + /// Errors produced during the issuance process #[derive(Debug, PartialEq, Eq)] pub enum Error { From 4c7d4848906d3d2ec202a476b4419975ee38c464 Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Fri, 31 Jan 2025 17:30:17 +0100 Subject: [PATCH 20/28] Minor code format fixes (empty line, comment) --- src/issuance.rs | 51 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/src/issuance.rs b/src/issuance.rs index c04db94cc..b69d4a77d 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -644,6 +644,7 @@ pub fn verify_issue_bundle( .iter() .try_fold(HashMap::new(), |mut new_records, action| { let (asset, amount) = action.verify(bundle.ik())?; + let is_finalized = action.is_finalized(); let ref_note = action.get_reference_note(); @@ -659,7 +660,7 @@ pub fn verify_issue_bundle( *ref_note.ok_or(MissingReferenceNoteOnFirstIssuance)?, ), - // Subsequent issuances of the asset + // Subsequent issuance of the asset Some(current_record) => { let amount = (current_record.amount + amount).ok_or(ValueOverflow)?; @@ -677,6 +678,54 @@ pub fn verify_issue_bundle( }) } +/// Attempts to revert ("undo") the issuance actions in `bundle`. +/// +/// ## How it works +/// 1. Calls [`verify_issue_bundle`] with a custom `get_global_asset_state` that +/// fabricates zero-amount, non-final asset states. This lets us compute the +/// net effect (`delta`) introduced by the bundle. +/// 2. For each asset in the resulting `delta` map, we subtract out its `delta.amount` +/// from the *real* global state. If finalization has changed, we abort. +/// 3. We set `is_finalized` to false in the returned map. +/// +/// Returns `None` if any step fails (e.g., asset doesn't exist, underflow on amount, +/// or a finalization mismatch). +pub fn revert_issue_bundle( + bundle: &IssueBundle, + sighash: [u8; 32], + get_global_asset_state: impl Fn(&AssetBase) -> Option, +) -> Option> { + let asset_state_deltas = verify_issue_bundle(bundle, sighash, |asset| { + get_global_asset_state(asset).map(|prev_asset_state| { + AssetRecord::new(NoteValue::zero(), false, prev_asset_state.reference_note) + }) + }) + .ok()?; + + asset_state_deltas + .into_iter() + .map(|(asset, asset_state_delta)| { + let prev_asset_state = get_global_asset_state(&asset)?; + + let new_amount = NoteValue::from_raw( + prev_asset_state + .amount + .inner() + .checked_sub(asset_state_delta.amount.inner())?, + ); + + if prev_asset_state.is_finalized != asset_state_delta.is_finalized { + return None; + } + + Some(( + asset, + AssetRecord::new(new_amount, false, prev_asset_state.reference_note), + )) + }) + .collect() +} + /// Errors produced during the issuance process #[derive(Debug, PartialEq, Eq)] pub enum Error { From 956e1d04ce1848a2d75b57d96945b2b7b852943b Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Fri, 31 Jan 2025 17:58:51 +0100 Subject: [PATCH 21/28] Introduce TestParams struct in the test part of issuance.rs and use it as a return value of setup_params instead of a tuple --- src/issuance.rs | 199 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 161 insertions(+), 38 deletions(-) diff --git a/src/issuance.rs b/src/issuance.rs index b69d4a77d..2f39db2f3 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -854,14 +854,17 @@ mod tests { assert_eq!(note.asset(), asset); } - fn setup_params() -> ( - OsRng, - IssuanceAuthorizingKey, - IssuanceValidatingKey, - Address, - [u8; 32], - Nullifier, - ) { + #[derive(Clone)] + struct TestParams { + rng: OsRng, + isk: IssuanceAuthorizingKey, + ik: IssuanceValidatingKey, + recipient: Address, + sighash: [u8; 32], + first_nullifier: Nullifier, + } + + fn setup_params() -> TestParams { let mut rng = OsRng; let isk = IssuanceAuthorizingKey::random(); @@ -875,7 +878,14 @@ mod tests { let first_nullifier = Nullifier::dummy(&mut rng); - (rng, isk, ik, recipient, sighash, first_nullifier) + TestParams { + rng, + isk, + ik, + recipient, + sighash, + first_nullifier, + } } /// Sets up test parameters for action verification tests. @@ -889,7 +899,12 @@ mod tests { note2_asset_desc: Option<&[u8]>, // if None, both notes use the same asset finalize: bool, ) -> (IssuanceValidatingKey, AssetBase, IssueAction) { - let (mut rng, _, ik, recipient, _, _) = setup_params(); + let TestParams { + mut rng, + ik, + recipient, + .. + } = setup_params(); let asset = AssetBase::derive(&ik, note1_asset_desc); let note2_asset = note2_asset_desc.map_or(asset, |desc| AssetBase::derive(&ik, desc)); @@ -937,7 +952,14 @@ mod tests { [u8; 32], Nullifier, ) { - let (mut rng, isk, ik, recipient, sighash, first_nullifier) = setup_params(); + let TestParams { + mut rng, + isk, + ik, + recipient, + sighash, + first_nullifier, + } = setup_params(); let note1 = Note::new( recipient, @@ -1014,14 +1036,20 @@ mod tests { #[test] fn action_verify_ik_mismatch_asset_base() { let (_, _, action) = action_verify_test_params(10, 20, b"Asset 1", None, false); - let (_, _, ik, _, _, _) = setup_params(); + let TestParams { ik, .. } = setup_params(); assert_eq!(action.verify(&ik), Err(IssueBundleIkMismatchAssetBase)); } #[test] fn issue_bundle_basic() { - let (rng, _, ik, recipient, _, first_nullifier) = setup_params(); + let TestParams { + rng, + ik, + recipient, + first_nullifier, + .. + } = setup_params(); let str = "Halo".to_string(); let str2 = "Halo2".to_string(); @@ -1137,7 +1165,9 @@ mod tests { #[test] fn issue_bundle_finalize_asset() { - let (rng, _, ik, recipient, _, _) = setup_params(); + let TestParams { + rng, ik, recipient, .. + } = setup_params(); let (mut bundle, _) = IssueBundle::new( ik, @@ -1170,7 +1200,14 @@ mod tests { #[test] fn issue_bundle_prepare() { - let (rng, _, ik, recipient, sighash, first_nullifier) = setup_params(); + let TestParams { + rng, + ik, + recipient, + sighash, + first_nullifier, + .. + } = setup_params(); let (bundle, _) = IssueBundle::new( ik, @@ -1190,7 +1227,14 @@ mod tests { #[test] fn issue_bundle_sign() { - let (rng, isk, ik, recipient, sighash, first_nullifier) = setup_params(); + let TestParams { + rng, + isk, + ik, + recipient, + sighash, + first_nullifier, + } = setup_params(); let (bundle, _) = IssueBundle::new( ik.clone(), @@ -1216,7 +1260,13 @@ mod tests { #[test] fn issue_bundle_invalid_isk_for_signature() { - let (rng, _, ik, recipient, _, first_nullifier) = setup_params(); + let TestParams { + rng, + ik, + recipient, + first_nullifier, + .. + } = setup_params(); let (bundle, _) = IssueBundle::new( ik, @@ -1243,7 +1293,14 @@ mod tests { #[test] fn issue_bundle_incorrect_asset_for_signature() { - let (mut rng, isk, ik, recipient, _, first_nullifier) = setup_params(); + let TestParams { + mut rng, + isk, + ik, + recipient, + first_nullifier, + .. + } = setup_params(); // Create a bundle with "normal" note let (mut bundle, _) = IssueBundle::new( @@ -1279,7 +1336,14 @@ mod tests { #[test] fn issue_bundle_verify() { - let (rng, isk, ik, recipient, sighash, first_nullifier) = setup_params(); + let TestParams { + rng, + isk, + ik, + recipient, + sighash, + first_nullifier, + } = setup_params(); let (bundle, _) = IssueBundle::new( ik.clone(), @@ -1313,7 +1377,14 @@ mod tests { #[test] fn issue_bundle_verify_with_finalize() { - let (rng, isk, ik, recipient, sighash, first_nullifier) = setup_params(); + let TestParams { + rng, + isk, + ik, + recipient, + sighash, + first_nullifier, + } = setup_params(); let (mut bundle, _) = IssueBundle::new( ik.clone(), @@ -1349,7 +1420,14 @@ mod tests { #[test] fn issue_bundle_verify_with_issued_assets() { - let (rng, isk, ik, recipient, sighash, first_nullifier) = setup_params(); + let TestParams { + rng, + isk, + ik, + recipient, + sighash, + first_nullifier, + } = setup_params(); let asset1_desc = b"Verify with issued assets 1".to_vec(); let asset2_desc = b"Verify with issued assets 2".to_vec(); @@ -1450,17 +1528,17 @@ mod tests { } fn build_issue_bundle( - params: &( - OsRng, - IssuanceAuthorizingKey, - IssuanceValidatingKey, - Address, - [u8; 32], - Nullifier, - ), + params: &TestParams, data: &[(&Vec, u64, bool, bool)], ) -> IssueBundle { - let (rng, ref isk, ref ik, recipient, sighash, ref first_nullifier) = *params; + let TestParams { + rng, + ref isk, + ref ik, + recipient, + sighash, + ref first_nullifier, + } = *params; let (asset_desc, amount, first_issuance, is_finalized) = data.first().unwrap().clone(); @@ -1507,7 +1585,7 @@ mod tests { let params = setup_params(); - let (_, _, ik, _, sighash, _) = params.clone(); + let TestParams { ik, sighash, .. } = params.clone(); let asset1_desc = b"Verify with issued assets 1".to_vec(); let asset2_desc = b"Verify with issued assets 2".to_vec(); @@ -1654,7 +1732,14 @@ mod tests { #[test] fn issue_bundle_verify_fail_previously_finalized() { - let (mut rng, isk, ik, recipient, sighash, first_nullifier) = setup_params(); + let TestParams { + mut rng, + isk, + ik, + recipient, + sighash, + first_nullifier, + } = setup_params(); let (bundle, _) = IssueBundle::new( ik.clone(), @@ -1709,7 +1794,14 @@ mod tests { } } - let (rng, isk, ik, recipient, sighash, first_nullifier) = setup_params(); + let TestParams { + rng, + isk, + ik, + recipient, + sighash, + first_nullifier, + } = setup_params(); let (bundle, _) = IssueBundle::new( ik, @@ -1743,7 +1835,15 @@ mod tests { #[test] fn issue_bundle_verify_fail_wrong_sighash() { - let (rng, isk, ik, recipient, random_sighash, first_nullifier) = setup_params(); + let TestParams { + rng, + isk, + ik, + recipient, + sighash: random_sighash, + first_nullifier, + } = setup_params(); + let (bundle, _) = IssueBundle::new( ik, b"Asset description".to_vec(), @@ -1771,7 +1871,14 @@ mod tests { #[test] fn issue_bundle_verify_fail_incorrect_asset_description() { - let (mut rng, isk, ik, recipient, sighash, first_nullifier) = setup_params(); + let TestParams { + mut rng, + isk, + ik, + recipient, + sighash, + first_nullifier, + } = setup_params(); let (bundle, _) = IssueBundle::new( ik, @@ -1812,7 +1919,14 @@ mod tests { fn issue_bundle_verify_fail_incorrect_ik() { let asset_description = b"Asset".to_vec(); - let (mut rng, isk, ik, recipient, sighash, first_nullifier) = setup_params(); + let TestParams { + mut rng, + isk, + ik, + recipient, + sighash, + first_nullifier, + } = setup_params(); let (bundle, _) = IssueBundle::new( ik, @@ -1861,7 +1975,14 @@ mod tests { } } - let (rng, isk, ik, recipient, sighash, first_nullifier) = setup_params(); + let TestParams { + rng, + isk, + ik, + recipient, + sighash, + first_nullifier, + } = setup_params(); let (bundle, _) = IssueBundle::new( ik, @@ -1949,7 +2070,9 @@ mod tests { #[test] fn issue_bundle_asset_desc_roundtrip() { - let (rng, _, ik, recipient, _, _) = setup_params(); + let TestParams { + rng, ik, recipient, .. + } = setup_params(); // Generated using https://onlinetools.com/utf8/generate-random-utf8 let asset_desc_1 = "󅞞 򬪗YV8𱈇m0{둛򙎠[㷊V֤]9Ծ̖l󾓨2닯򗏟iȰ䣄˃Oߺ񗗼🦄" From 322a55ab1634cf2013845ad39230a71de1cecb0e Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Fri, 31 Jan 2025 22:32:21 +0100 Subject: [PATCH 22/28] Moved issue_bundle_verify_with_global_state into a separate file and refactor it according the #138 review comments --- src/issuance.rs | 230 +------------------------- src/issuance/tests/global_state.rs | 256 +++++++++++++++++++++++++++++ 2 files changed, 259 insertions(+), 227 deletions(-) create mode 100644 src/issuance/tests/global_state.rs diff --git a/src/issuance.rs b/src/issuance.rs index 2f39db2f3..d8f512ab1 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -819,8 +819,7 @@ mod tests { issuance::Error::{ AssetBaseCannotBeIdentityPoint, IssueActionNotFound, IssueActionPreviouslyFinalizedAssetBase, IssueBundleIkMismatchAssetBase, - IssueBundleInvalidSignature, MissingReferenceNoteOnFirstIssuance, ValueOverflow, - WrongAssetDescSize, + IssueBundleInvalidSignature, WrongAssetDescSize, }, issuance::{ is_reference_note, verify_issue_bundle, AwaitingNullifier, IssueAction, Signed, @@ -843,6 +842,8 @@ mod tests { use rand::RngCore; use std::collections::HashMap; + mod global_state; + /// Validation for reference note /// /// The following checks are performed: @@ -1505,231 +1506,6 @@ mod tests { ); } - // Issuance workflow test: performs a series of bundle creations and verifications, - // with a global state simulation - #[test] - fn issue_bundle_verify_with_global_state() { - type GlobalState = HashMap; - - fn first_note(bundle: &IssueBundle, action_index: usize) -> Note { - bundle.actions()[action_index].notes()[0] - } - - fn build_expected_global_state(data: &[(&AssetBase, u64, bool, Note)]) -> GlobalState { - data.into_iter() - .cloned() - .map(|(asset_base, amount, is_finalized, reference_note)| { - ( - asset_base.clone(), - AssetRecord::new(NoteValue::from_raw(amount), is_finalized, reference_note), - ) - }) - .collect::>() - } - - fn build_issue_bundle( - params: &TestParams, - data: &[(&Vec, u64, bool, bool)], - ) -> IssueBundle { - let TestParams { - rng, - ref isk, - ref ik, - recipient, - sighash, - ref first_nullifier, - } = *params; - - let (asset_desc, amount, first_issuance, is_finalized) = data.first().unwrap().clone(); - - let (mut bundle, _) = IssueBundle::new( - ik.clone(), - asset_desc.clone(), - Some(IssueInfo { - recipient, - value: NoteValue::from_raw(amount), - }), - first_issuance, - rng, - ) - .unwrap(); - - if is_finalized { - bundle.finalize_action(asset_desc).unwrap(); - } - - for (asset_desc, amount, first_issuance, is_finalized) in - data.into_iter().skip(1).cloned() - { - bundle - .add_recipient( - &asset_desc, - recipient, - NoteValue::from_raw(amount), - first_issuance, - rng, - ) - .unwrap(); - - if is_finalized { - bundle.finalize_action(asset_desc).unwrap(); - } - } - - bundle - .update_rho(&first_nullifier) - .prepare(sighash) - .sign(&isk) - .unwrap() - } - - let params = setup_params(); - - let TestParams { ik, sighash, .. } = params.clone(); - - let asset1_desc = b"Verify with issued assets 1".to_vec(); - let asset2_desc = b"Verify with issued assets 2".to_vec(); - let asset3_desc = b"Verify with issued assets 3".to_vec(); - let asset4_desc = b"Verify with issued assets 4".to_vec(); - - let asset1_base = AssetBase::derive(&ik, &asset1_desc); - let asset2_base = AssetBase::derive(&ik, &asset2_desc); - let asset3_base = AssetBase::derive(&ik, &asset3_desc); - let asset4_base = AssetBase::derive(&ik, &asset4_desc); - - let mut global_state = GlobalState::new(); - - // We'll issue and verify a series of bundles. For valid bundles, the global - // state is updated and must match the expected result. For invalid bundles, - // we check the expected error, leaving the state unchanged. - - // ** Bundle1 (valid) ** - - let bundle1 = build_issue_bundle( - ¶ms, - &[ - (&asset1_desc, 7, true, false), - (&asset1_desc, 8, false, false), - (&asset2_desc, 10, true, true), - (&asset3_desc, 5, true, false), - ], - ); - - let expected_global_state1 = build_expected_global_state(&[ - (&asset1_base, 15, false, first_note(&bundle1, 0)), - (&asset2_base, 10, true, first_note(&bundle1, 1)), - (&asset3_base, 5, false, first_note(&bundle1, 2)), - ]); - - global_state.extend( - verify_issue_bundle(&bundle1, sighash, |asset| global_state.get(asset).cloned()) - .unwrap(), - ); - assert_eq!(global_state, expected_global_state1); - - // ** Bundle2 (valid) ** - - let bundle2 = build_issue_bundle( - ¶ms, - &[ - (&asset1_desc, 3, true, true), - (&asset3_desc, 20, false, false), - ], - ); - - let expected_global_state2 = build_expected_global_state(&[ - (&asset1_base, 18, true, first_note(&bundle1, 0)), - (&asset2_base, 10, true, first_note(&bundle1, 1)), - (&asset3_base, 25, false, first_note(&bundle1, 2)), - ]); - - global_state.extend( - verify_issue_bundle(&bundle2, sighash, |asset| global_state.get(asset).cloned()) - .unwrap(), - ); - assert_eq!(global_state, expected_global_state2); - - // ** Bundle3 (invalid) ** - - let bundle3 = build_issue_bundle( - ¶ms, - &[ - (&asset1_desc, 3, true, false), - (&asset3_desc, 20, false, false), - ], - ); - - let expected_global_state3 = expected_global_state2; - - assert_eq!( - verify_issue_bundle(&bundle3, sighash, |asset| global_state.get(asset).cloned()) - .unwrap_err(), - IssueActionPreviouslyFinalizedAssetBase, - ); - assert_eq!(global_state, expected_global_state3); - - // ** Bundle4 (invalid) ** - - let bundle4 = build_issue_bundle( - ¶ms, - &[ - (&asset3_desc, 50, true, true), - (&asset4_desc, 77, false, false), - ], - ); - - let expected_global_state4 = expected_global_state3; - - assert_eq!( - verify_issue_bundle(&bundle4, sighash, |asset| global_state.get(asset).cloned()) - .unwrap_err(), - MissingReferenceNoteOnFirstIssuance, - ); - assert_eq!(global_state, expected_global_state4); - - // ** Bundle5 (invalid) ** - - let bundle5 = build_issue_bundle( - ¶ms, - &[ - (&asset3_desc, u64::MAX - 20, true, true), - (&asset4_desc, 77, true, false), - ], - ); - - let expected_global_state5 = expected_global_state4; - - assert_eq!( - verify_issue_bundle(&bundle5, sighash, |asset| global_state.get(asset).cloned()) - .unwrap_err(), - ValueOverflow, - ); - assert_eq!(global_state, expected_global_state5); - - // ** Bundle6 (valid) ** - - let bundle6 = build_issue_bundle( - ¶ms, - &[ - (&asset3_desc, 50, true, true), - (&asset4_desc, 77, true, false), - ], - ); - - let expected_global_state6 = build_expected_global_state(&[ - (&asset1_base, 18, true, first_note(&bundle1, 0)), - (&asset2_base, 10, true, first_note(&bundle1, 1)), - (&asset3_base, 75, true, first_note(&bundle1, 2)), - (&asset4_base, 77, false, first_note(&bundle6, 1)), - ]); - - global_state.extend( - verify_issue_bundle(&bundle6, sighash, |asset| global_state.get(asset).cloned()) - .unwrap(), - ); - assert_eq!(global_state, expected_global_state6); - } - #[test] fn issue_bundle_verify_fail_previously_finalized() { let TestParams { diff --git a/src/issuance/tests/global_state.rs b/src/issuance/tests/global_state.rs new file mode 100644 index 000000000..4ddcdce9e --- /dev/null +++ b/src/issuance/tests/global_state.rs @@ -0,0 +1,256 @@ +use std::collections::HashMap; + +use crate::issuance::{ + verify_issue_bundle, AssetBase, AssetRecord, + Error::{ + IssueActionPreviouslyFinalizedAssetBase, MissingReferenceNoteOnFirstIssuance, ValueOverflow, + }, + IssueBundle, IssueInfo, Note, NoteValue, Signed, +}; + +use super::{setup_params, TestParams}; + +fn build_state_entry( + asset_base: &AssetBase, + amount: u64, + is_finalized: bool, + reference_note: &Note, +) -> (AssetBase, AssetRecord) { + ( + *asset_base, + AssetRecord::new(NoteValue::from_raw(amount), is_finalized, *reference_note), + ) +} + +#[derive(Clone)] +struct BundleTestData { + asset_desc: Vec, + amount: u64, + is_finalized: bool, + first_issuance: bool, +} + +impl BundleTestData { + fn new(asset_desc: &Vec, amount: u64, is_finalized: bool, first_issuance: bool) -> Self { + Self { + asset_desc: asset_desc.clone(), + amount, + is_finalized, + first_issuance, + } + } +} + +fn get_first_note(bundle: &IssueBundle, action_index: usize) -> &Note { + bundle.actions()[action_index].notes().first().unwrap() +} + +fn build_issue_bundle(params: &TestParams, data: &[BundleTestData]) -> IssueBundle { + let TestParams { + rng, + ref isk, + ref ik, + recipient, + sighash, + ref first_nullifier, + } = *params; + + let BundleTestData { + asset_desc, + amount, + is_finalized, + first_issuance, + } = data.first().unwrap().clone(); + + let (mut bundle, _) = IssueBundle::new( + ik.clone(), + asset_desc.clone(), + Some(IssueInfo { + recipient, + value: NoteValue::from_raw(amount), + }), + first_issuance, + rng, + ) + .unwrap(); + + if is_finalized { + bundle.finalize_action(&asset_desc).unwrap(); + } + + for BundleTestData { + asset_desc, + amount, + is_finalized, + first_issuance, + } in data.into_iter().skip(1).cloned() + { + bundle + .add_recipient( + &asset_desc, + recipient, + NoteValue::from_raw(amount), + first_issuance, + rng, + ) + .unwrap(); + + if is_finalized { + bundle.finalize_action(&asset_desc).unwrap(); + } + } + + bundle + .update_rho(&first_nullifier) + .prepare(sighash) + .sign(&isk) + .unwrap() +} + +// Issuance workflow test: performs a series of bundle creations and verifications, +// with a global state simulation +#[test] +fn issue_bundle_verify_with_global_state() { + let params = setup_params(); + + let TestParams { ik, sighash, .. } = params.clone(); + + let asset1_desc = b"Verify with issued assets 1".to_vec(); + let asset2_desc = b"Verify with issued assets 2".to_vec(); + let asset3_desc = b"Verify with issued assets 3".to_vec(); + let asset4_desc = b"Verify with issued assets 4".to_vec(); + + let asset1_base = AssetBase::derive(&ik, &asset1_desc); + let asset2_base = AssetBase::derive(&ik, &asset2_desc); + let asset3_base = AssetBase::derive(&ik, &asset3_desc); + let asset4_base = AssetBase::derive(&ik, &asset4_desc); + + let mut global_state = HashMap::new(); + + // We'll issue and verify a series of bundles. For valid bundles, the global + // state is updated and must match the expected result. For invalid bundles, + // we check the expected error, leaving the state unchanged. + + // ** Bundle1 (valid) ** + + let bundle1 = build_issue_bundle( + ¶ms, + &[ + BundleTestData::new(&asset1_desc, 7, false, true), + BundleTestData::new(&asset1_desc, 8, false, false), + BundleTestData::new(&asset2_desc, 10, true, true), + BundleTestData::new(&asset3_desc, 5, false, true), + ], + ); + + let expected_global_state1 = HashMap::from([ + build_state_entry(&asset1_base, 15, false, get_first_note(&bundle1, 0)), + build_state_entry(&asset2_base, 10, true, get_first_note(&bundle1, 1)), + build_state_entry(&asset3_base, 5, false, get_first_note(&bundle1, 2)), + ]); + + global_state.extend( + verify_issue_bundle(&bundle1, sighash, |asset| global_state.get(asset).cloned()).unwrap(), + ); + assert_eq!(global_state, expected_global_state1); + + // ** Bundle2 (valid) ** + + let bundle2 = build_issue_bundle( + ¶ms, + &[ + BundleTestData::new(&asset1_desc, 3, true, true), + BundleTestData::new(&asset3_desc, 20, false, false), + ], + ); + + let expected_global_state2 = HashMap::from([ + build_state_entry(&asset1_base, 18, true, get_first_note(&bundle1, 0)), + build_state_entry(&asset2_base, 10, true, get_first_note(&bundle1, 1)), + build_state_entry(&asset3_base, 25, false, get_first_note(&bundle1, 2)), + ]); + + global_state.extend( + verify_issue_bundle(&bundle2, sighash, |asset| global_state.get(asset).cloned()).unwrap(), + ); + assert_eq!(global_state, expected_global_state2); + + // ** Bundle3 (invalid) ** + + let bundle3 = build_issue_bundle( + ¶ms, + &[ + BundleTestData::new(&asset1_desc, 3, false, true), + BundleTestData::new(&asset3_desc, 20, false, false), + ], + ); + + let expected_global_state3 = expected_global_state2; + + assert_eq!( + verify_issue_bundle(&bundle3, sighash, |asset| global_state.get(asset).cloned()) + .unwrap_err(), + IssueActionPreviouslyFinalizedAssetBase, + ); + assert_eq!(global_state, expected_global_state3); + + // ** Bundle4 (invalid) ** + + let bundle4 = build_issue_bundle( + ¶ms, + &[ + BundleTestData::new(&asset3_desc, 50, true, true), + BundleTestData::new(&asset4_desc, 77, false, false), + ], + ); + + let expected_global_state4 = expected_global_state3; + + assert_eq!( + verify_issue_bundle(&bundle4, sighash, |asset| global_state.get(asset).cloned()) + .unwrap_err(), + MissingReferenceNoteOnFirstIssuance, + ); + assert_eq!(global_state, expected_global_state4); + + // ** Bundle5 (invalid) ** + + let bundle5 = build_issue_bundle( + ¶ms, + &[ + BundleTestData::new(&asset3_desc, u64::MAX - 20, true, true), + BundleTestData::new(&asset4_desc, 77, false, true), + ], + ); + + let expected_global_state5 = expected_global_state4; + + assert_eq!( + verify_issue_bundle(&bundle5, sighash, |asset| global_state.get(asset).cloned()) + .unwrap_err(), + ValueOverflow, + ); + assert_eq!(global_state, expected_global_state5); + + // ** Bundle6 (valid) ** + + let bundle6 = build_issue_bundle( + ¶ms, + &[ + BundleTestData::new(&asset3_desc, 50, true, true), + BundleTestData::new(&asset4_desc, 77, false, true), + ], + ); + + let expected_global_state6 = HashMap::from([ + build_state_entry(&asset1_base, 18, true, get_first_note(&bundle1, 0)), + build_state_entry(&asset2_base, 10, true, get_first_note(&bundle1, 1)), + build_state_entry(&asset3_base, 75, true, get_first_note(&bundle1, 2)), + build_state_entry(&asset4_base, 77, false, get_first_note(&bundle6, 1)), + ]); + + global_state.extend( + verify_issue_bundle(&bundle6, sighash, |asset| global_state.get(asset).cloned()).unwrap(), + ); + assert_eq!(global_state, expected_global_state6); +} From c8208d11769ed0c9f7b9668098f2610d56065651 Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Mon, 3 Feb 2025 14:54:45 +0100 Subject: [PATCH 23/28] Remove revert_issue_bundle function --- src/issuance.rs | 48 ------------------------------------------------ 1 file changed, 48 deletions(-) diff --git a/src/issuance.rs b/src/issuance.rs index d8f512ab1..dc21555ad 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -678,54 +678,6 @@ pub fn verify_issue_bundle( }) } -/// Attempts to revert ("undo") the issuance actions in `bundle`. -/// -/// ## How it works -/// 1. Calls [`verify_issue_bundle`] with a custom `get_global_asset_state` that -/// fabricates zero-amount, non-final asset states. This lets us compute the -/// net effect (`delta`) introduced by the bundle. -/// 2. For each asset in the resulting `delta` map, we subtract out its `delta.amount` -/// from the *real* global state. If finalization has changed, we abort. -/// 3. We set `is_finalized` to false in the returned map. -/// -/// Returns `None` if any step fails (e.g., asset doesn't exist, underflow on amount, -/// or a finalization mismatch). -pub fn revert_issue_bundle( - bundle: &IssueBundle, - sighash: [u8; 32], - get_global_asset_state: impl Fn(&AssetBase) -> Option, -) -> Option> { - let asset_state_deltas = verify_issue_bundle(bundle, sighash, |asset| { - get_global_asset_state(asset).map(|prev_asset_state| { - AssetRecord::new(NoteValue::zero(), false, prev_asset_state.reference_note) - }) - }) - .ok()?; - - asset_state_deltas - .into_iter() - .map(|(asset, asset_state_delta)| { - let prev_asset_state = get_global_asset_state(&asset)?; - - let new_amount = NoteValue::from_raw( - prev_asset_state - .amount - .inner() - .checked_sub(asset_state_delta.amount.inner())?, - ); - - if prev_asset_state.is_finalized != asset_state_delta.is_finalized { - return None; - } - - Some(( - asset, - AssetRecord::new(new_amount, false, prev_asset_state.reference_note), - )) - }) - .collect() -} - /// Errors produced during the issuance process #[derive(Debug, PartialEq, Eq)] pub enum Error { From df9d5ebe4cc319ba8f50e359e0e7fcb80f7ae164 Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Mon, 3 Feb 2025 14:55:17 +0100 Subject: [PATCH 24/28] Rename BundleTestData to IssueTestNote --- src/issuance/tests/global_state.rs | 38 +++++++++++++++--------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/issuance/tests/global_state.rs b/src/issuance/tests/global_state.rs index 4ddcdce9e..4fd635139 100644 --- a/src/issuance/tests/global_state.rs +++ b/src/issuance/tests/global_state.rs @@ -23,14 +23,14 @@ fn build_state_entry( } #[derive(Clone)] -struct BundleTestData { +struct IssueTestNote { asset_desc: Vec, amount: u64, is_finalized: bool, first_issuance: bool, } -impl BundleTestData { +impl IssueTestNote { fn new(asset_desc: &Vec, amount: u64, is_finalized: bool, first_issuance: bool) -> Self { Self { asset_desc: asset_desc.clone(), @@ -45,7 +45,7 @@ fn get_first_note(bundle: &IssueBundle, action_index: usize) -> &Note { bundle.actions()[action_index].notes().first().unwrap() } -fn build_issue_bundle(params: &TestParams, data: &[BundleTestData]) -> IssueBundle { +fn build_issue_bundle(params: &TestParams, data: &[IssueTestNote]) -> IssueBundle { let TestParams { rng, ref isk, @@ -55,7 +55,7 @@ fn build_issue_bundle(params: &TestParams, data: &[BundleTestData]) -> IssueBund ref first_nullifier, } = *params; - let BundleTestData { + let IssueTestNote { asset_desc, amount, is_finalized, @@ -78,7 +78,7 @@ fn build_issue_bundle(params: &TestParams, data: &[BundleTestData]) -> IssueBund bundle.finalize_action(&asset_desc).unwrap(); } - for BundleTestData { + for IssueTestNote { asset_desc, amount, is_finalized, @@ -136,10 +136,10 @@ fn issue_bundle_verify_with_global_state() { let bundle1 = build_issue_bundle( ¶ms, &[ - BundleTestData::new(&asset1_desc, 7, false, true), - BundleTestData::new(&asset1_desc, 8, false, false), - BundleTestData::new(&asset2_desc, 10, true, true), - BundleTestData::new(&asset3_desc, 5, false, true), + IssueTestNote::new(&asset1_desc, 7, false, true), + IssueTestNote::new(&asset1_desc, 8, false, false), + IssueTestNote::new(&asset2_desc, 10, true, true), + IssueTestNote::new(&asset3_desc, 5, false, true), ], ); @@ -159,8 +159,8 @@ fn issue_bundle_verify_with_global_state() { let bundle2 = build_issue_bundle( ¶ms, &[ - BundleTestData::new(&asset1_desc, 3, true, true), - BundleTestData::new(&asset3_desc, 20, false, false), + IssueTestNote::new(&asset1_desc, 3, true, true), + IssueTestNote::new(&asset3_desc, 20, false, false), ], ); @@ -180,8 +180,8 @@ fn issue_bundle_verify_with_global_state() { let bundle3 = build_issue_bundle( ¶ms, &[ - BundleTestData::new(&asset1_desc, 3, false, true), - BundleTestData::new(&asset3_desc, 20, false, false), + IssueTestNote::new(&asset1_desc, 3, false, true), + IssueTestNote::new(&asset3_desc, 20, false, false), ], ); @@ -199,8 +199,8 @@ fn issue_bundle_verify_with_global_state() { let bundle4 = build_issue_bundle( ¶ms, &[ - BundleTestData::new(&asset3_desc, 50, true, true), - BundleTestData::new(&asset4_desc, 77, false, false), + IssueTestNote::new(&asset3_desc, 50, true, true), + IssueTestNote::new(&asset4_desc, 77, false, false), ], ); @@ -218,8 +218,8 @@ fn issue_bundle_verify_with_global_state() { let bundle5 = build_issue_bundle( ¶ms, &[ - BundleTestData::new(&asset3_desc, u64::MAX - 20, true, true), - BundleTestData::new(&asset4_desc, 77, false, true), + IssueTestNote::new(&asset3_desc, u64::MAX - 20, true, true), + IssueTestNote::new(&asset4_desc, 77, false, true), ], ); @@ -237,8 +237,8 @@ fn issue_bundle_verify_with_global_state() { let bundle6 = build_issue_bundle( ¶ms, &[ - BundleTestData::new(&asset3_desc, 50, true, true), - BundleTestData::new(&asset4_desc, 77, false, true), + IssueTestNote::new(&asset3_desc, 50, true, true), + IssueTestNote::new(&asset4_desc, 77, false, true), ], ); From fce03d5bb9024f5dc1b18227e67e9bbbbf3780fb Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Mon, 3 Feb 2025 21:00:08 +0100 Subject: [PATCH 25/28] Move issuance/tests/global_state.rs to the integration_tests folder as issuance_global_state.rs, and use copies of setup_params and TestParams there. The code doesn't compile because some methods need to be public. Further fixes are required. --- src/asset_record.rs | 3 +- src/issuance.rs | 2 - .../issuance_global_state.rs | 54 ++++++++++++++++--- 3 files changed, 49 insertions(+), 10 deletions(-) rename src/issuance/tests/global_state.rs => tests/issuance_global_state.rs (85%) diff --git a/src/asset_record.rs b/src/asset_record.rs index 4f8c4bf2d..b3af557ca 100644 --- a/src/asset_record.rs +++ b/src/asset_record.rs @@ -4,8 +4,7 @@ use crate::{value::NoteValue, Note}; /// Represents aggregated information about an asset, including its supply, finalization status, /// and reference note. -#[derive(Debug, Clone, Copy)] -#[cfg_attr(test, derive(PartialEq, Eq))] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct AssetRecord { /// The amount of the asset. pub amount: NoteValue, diff --git a/src/issuance.rs b/src/issuance.rs index dc21555ad..531db1811 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -794,8 +794,6 @@ mod tests { use rand::RngCore; use std::collections::HashMap; - mod global_state; - /// Validation for reference note /// /// The following checks are performed: diff --git a/src/issuance/tests/global_state.rs b/tests/issuance_global_state.rs similarity index 85% rename from src/issuance/tests/global_state.rs rename to tests/issuance_global_state.rs index 4fd635139..729113356 100644 --- a/src/issuance/tests/global_state.rs +++ b/tests/issuance_global_state.rs @@ -1,14 +1,56 @@ use std::collections::HashMap; -use crate::issuance::{ - verify_issue_bundle, AssetBase, AssetRecord, - Error::{ - IssueActionPreviouslyFinalizedAssetBase, MissingReferenceNoteOnFirstIssuance, ValueOverflow, +use rand::{rngs::OsRng, RngCore}; + +use orchard::{ + asset_record::AssetRecord, + issuance::{ + verify_issue_bundle, + Error::{ + IssueActionPreviouslyFinalizedAssetBase, MissingReferenceNoteOnFirstIssuance, + ValueOverflow, + }, + IssueBundle, IssueInfo, Signed, }, - IssueBundle, IssueInfo, Note, NoteValue, Signed, + keys::{FullViewingKey, IssuanceAuthorizingKey, IssuanceValidatingKey, Scope, SpendingKey}, + note::{AssetBase, Nullifier}, + value::NoteValue, + Address, Note, }; -use super::{setup_params, TestParams}; +#[derive(Clone)] +struct TestParams { + rng: OsRng, + isk: IssuanceAuthorizingKey, + ik: IssuanceValidatingKey, + recipient: Address, + sighash: [u8; 32], + first_nullifier: Nullifier, +} + +fn setup_params() -> TestParams { + let mut rng = OsRng; + + let isk = IssuanceAuthorizingKey::random(); + let ik: IssuanceValidatingKey = (&isk).into(); + + let fvk = FullViewingKey::from(&SpendingKey::random(&mut rng)); + let recipient = fvk.address_at(0u32, Scope::External); + + let mut sighash = [0u8; 32]; + rng.fill_bytes(&mut sighash); + + let first_nullifier = Nullifier::dummy(&mut rng); + + TestParams { + rng, + isk, + ik, + recipient, + sighash, + first_nullifier, + } +} fn build_state_entry( asset_base: &AssetBase, From 3b03580f7879539c359c428340d4e9849ab7ec0b Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Tue, 4 Feb 2025 15:19:24 +0100 Subject: [PATCH 26/28] Update setup_params in issuance_global_state.rs to not use orchard pub(crate) method to generate random keys --- tests/issuance_global_state.rs | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/tests/issuance_global_state.rs b/tests/issuance_global_state.rs index 729113356..c06135992 100644 --- a/tests/issuance_global_state.rs +++ b/tests/issuance_global_state.rs @@ -18,6 +18,12 @@ use orchard::{ Address, Note, }; +fn random_bytes(mut rng: OsRng) -> [u8; N] { + let mut bytes = [0; N]; + rng.fill_bytes(&mut bytes); + bytes +} + #[derive(Clone)] struct TestParams { rng: OsRng, @@ -29,18 +35,21 @@ struct TestParams { } fn setup_params() -> TestParams { - let mut rng = OsRng; + use group::ff::{FromUniformBytes, PrimeField}; + use pasta_curves::pallas; + + let rng = OsRng; - let isk = IssuanceAuthorizingKey::random(); + let isk = IssuanceAuthorizingKey::from_bytes(random_bytes(rng)).unwrap(); let ik: IssuanceValidatingKey = (&isk).into(); - let fvk = FullViewingKey::from(&SpendingKey::random(&mut rng)); + let fvk = FullViewingKey::from(&SpendingKey::from_bytes(random_bytes(rng)).unwrap()); let recipient = fvk.address_at(0u32, Scope::External); - let mut sighash = [0u8; 32]; - rng.fill_bytes(&mut sighash); + let sighash = random_bytes(rng); - let first_nullifier = Nullifier::dummy(&mut rng); + let base = pallas::Base::from_uniform_bytes(&random_bytes(rng)); + let first_nullifier = Nullifier::from_bytes(&base.to_repr()).unwrap(); TestParams { rng, From f83e6ea30a7a04e3b95c90f1b94928c6e8d7938e Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Tue, 4 Feb 2025 16:30:29 +0100 Subject: [PATCH 27/28] Use another way to get a random Nullifier for testing in issuance_global_state.rs --- tests/issuance_global_state.rs | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tests/issuance_global_state.rs b/tests/issuance_global_state.rs index c06135992..4f476f7f1 100644 --- a/tests/issuance_global_state.rs +++ b/tests/issuance_global_state.rs @@ -35,8 +35,8 @@ struct TestParams { } fn setup_params() -> TestParams { - use group::ff::{FromUniformBytes, PrimeField}; - use pasta_curves::pallas; + use group::{ff::PrimeField, Curve, Group}; + use pasta_curves::{arithmetic::CurveAffine, pallas}; let rng = OsRng; @@ -48,8 +48,20 @@ fn setup_params() -> TestParams { let sighash = random_bytes(rng); - let base = pallas::Base::from_uniform_bytes(&random_bytes(rng)); - let first_nullifier = Nullifier::from_bytes(&base.to_repr()).unwrap(); + let first_nullifier = { + let point = pallas::Point::random(rng); + + // For testing purposes only: replicate the behavior of the + // `orchard::spec::extract_p` function, which is marked as `pub(crate)` in + // `orchard` and is therefore not visible here. + let base = point + .to_affine() + .coordinates() + .map(|c| *c.x()) + .unwrap_or_else(pallas::Base::zero); + + Nullifier::from_bytes(&base.to_repr()).unwrap() + }; TestParams { rng, From 409bdf5d00e6c4f819ca9e299064fcb4caba9938 Mon Sep 17 00:00:00 2001 From: Dmitry Demin Date: Tue, 4 Feb 2025 17:08:50 +0100 Subject: [PATCH 28/28] Improve comments inissuance_global_state.rs::setup_params --- tests/issuance_global_state.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/issuance_global_state.rs b/tests/issuance_global_state.rs index 4f476f7f1..a6170a21e 100644 --- a/tests/issuance_global_state.rs +++ b/tests/issuance_global_state.rs @@ -34,6 +34,7 @@ struct TestParams { first_nullifier: Nullifier, } +// For testing global state only - should not be used in an actual setting. fn setup_params() -> TestParams { use group::{ff::PrimeField, Curve, Group}; use pasta_curves::{arithmetic::CurveAffine, pallas}; @@ -48,12 +49,12 @@ fn setup_params() -> TestParams { let sighash = random_bytes(rng); + // For testing purposes only: replicate the behavior of orchard's `Nullifier::dummy` + // and `extract_p` functions, which are marked as `pub(crate)` in orchard and are therefore + // not visible here. let first_nullifier = { let point = pallas::Point::random(rng); - // For testing purposes only: replicate the behavior of the - // `orchard::spec::extract_p` function, which is marked as `pub(crate)` in - // `orchard` and is therefore not visible here. let base = point .to_affine() .coordinates()