diff --git a/src/bundle/commitments/issuance.rs b/src/bundle/commitments/issuance.rs index cb3dd7ce3..13fe4d14b 100644 --- a/src/bundle/commitments/issuance.rs +++ b/src/bundle/commitments/issuance.rs @@ -123,7 +123,8 @@ mod tests { }), true, &mut rng, - ); + ) + .unwrap(); let another_asset = bundle .add_recipient( diff --git a/src/issuance.rs b/src/issuance.rs index f7983249b..68bc34436 100644 --- a/src/issuance.rs +++ b/src/issuance.rs @@ -29,8 +29,9 @@ use crate::{ }; use Error::{ - AssetBaseCannotBeIdentityPoint, CannotBeFirstIssuance, IncorrectRhoDerivation, - InvalidIssueBundleSig, InvalidIssueValidatingKey, InvalidSighashKind, IssueActionNotFound, + AssetBaseCannotBeIdentityPoint, CannotBeFirstIssuance, CannotFinalizeOnFirstIssuance, + DuplicateIssueActionForAssetBase, IncorrectRhoDerivation, InvalidIssueBundleSig, + InvalidIssueValidatingKey, InvalidSighashKind, IssueActionNotFound, IssueActionPreviouslyFinalizedAssetBase, IssueActionWithoutNoteNotFinalized, IssueBundleIkMismatchAssetBase, MissingReferenceNoteOnFirstIssuance, ValueOverflow, }; @@ -404,21 +405,40 @@ impl IssueBundle { impl IssueBundle { /// Constructs a new `IssueBundle`. /// - /// If issue_info is None, the new `IssueBundle` will contain one `IssueAction` without notes - /// and with `finalize` set to true. - /// Otherwise, the new `IssueBundle` will contain one `IssueAction` with one note created from - /// issue_info values and with `finalize` set to false. In this created note, rho will be - /// set to zero. The rho value will be updated later by calling the `update_rho` method. + /// # Returns + /// + /// Returns the newly constructed `IssueBundle` and its corresponding `AssetBase`. + /// + /// The bundle contains a single `IssueAction`: + /// - if `issue_info` is `Some`, the action contains one issue note created from + /// `issue_info`, with `finalize` set to `false`; + /// - if `issue_info` is `None`, the action contains no issue note, with `finalize` + /// set to `true`. + /// + /// When `first_issuance` is `true`, a reference note for the asset defined by + /// (`asset_desc_hash`, `ik`) is also added. + /// + /// Created notes have `rho` set to `None` and use a temporary sampled `rseed`. + /// Their final `rho` and `rseed` values are set later by calling `update_rho`. + /// + /// # Errors /// - /// If `first_issuance` is true, the `IssueBundle` will contain a reference note for the asset - /// defined by (`asset_desc_hash`, `ik`). + /// Returns an error if `first_issuance` is `true` and `issue_info` is `None`, + /// since this would create a new asset and immediately finalize it without + /// creating any issue note for it. pub fn new( ik: IssueValidatingKey, asset_desc_hash: [u8; 32], issue_info: Option, first_issuance: bool, mut rng: impl RngCore, - ) -> (IssueBundle, AssetBase) { + ) -> Result<(IssueBundle, AssetBase), Error> { + // A first issuance cannot be finalized immediately, otherwise no issue note (except the + //reference note) can ever be created for this asset. + if first_issuance && issue_info.is_none() { + return Err(CannotFinalizeOnFirstIssuance); + } + let asset = AssetBase::custom(&AssetId::new_v0(&ik, &asset_desc_hash)); let mut notes = vec![]; @@ -446,21 +466,24 @@ impl IssueBundle { } }; - ( + Ok(( IssueBundle { ik, actions: NonEmpty::new(action), authorization: AwaitingNullifier, }, asset, - ) + )) } /// Add a new note to the `IssueBundle`. /// - /// Rho is set to zero. The rho value will be updated later by calling the `update_rho` method. + /// Rho is set to None and a temporary rseed is sampled. The rho and rseed values will be + /// updated later by calling the `update_rho` method. /// If `first_issuance` is true, we will also add a reference note for the asset defined by - /// (`asset_desc_hash`, `ik`). + /// (`asset_desc_hash`, `ik`). The reference note will also have a rho value set to + /// `None` and a temporary rseed value. The rho and rseed values of this reference note will be + /// updated later by calling the `update_rho` method. pub fn add_recipient( &mut self, asset_desc_hash: [u8; 32], @@ -506,27 +529,33 @@ impl IssueBundle { Ok(asset) } - /// Finalizes a given `IssueAction` - pub fn finalize_action(&mut self, asset_desc_hash: &[u8; 32]) -> Result<(), Error> { - match self + /// Finalizes issuance for the asset identified by (`asset_desc_hash`, `self.ik`). + /// + /// If an `IssueAction` already exists for this asset, its finalize flag is set. + /// Otherwise, a new finalize-only `IssueAction` is created. + pub fn finalize_action(&mut self, asset_desc_hash: &[u8; 32]) { + let issue_action = self .actions .iter_mut() - .find(|issue_action| issue_action.asset_desc_hash.eq(asset_desc_hash)) - { - Some(issue_action) => { - issue_action.flags = IssuanceFlags::from_parts(true); - } - None => { - return Err(IssueActionNotFound); - } - } + .find(|issue_action| issue_action.asset_desc_hash.eq(asset_desc_hash)); - Ok(()) + if let Some(issue_action) = issue_action { + issue_action.flags = IssuanceFlags::from_parts(true); + } else { + self.actions.push(IssueAction { + asset_desc_hash: *asset_desc_hash, + notes: vec![], + flags: IssuanceFlags::from_parts(true), + }); + } } /// Compute the correct rho value for each note in the bundle according to /// [ZIP-227: Issuance of Zcash Shielded Assets][zip227]. /// + /// This also updates each note's `rseed` by resampling it until a valid + /// commitment is produced. + /// /// [zip227]: https://zips.z.cash/zip-0227 pub fn update_rho( self, @@ -582,6 +611,10 @@ impl IssueBundle { pub fn sign(self, isk: &IssueAuthKey) -> Result, Error> { let expected_ik = IssueValidatingKey::from(isk); + if expected_ik != self.ik { + return Err(InvalidIssueValidatingKey); + } + // Make sure the `expected_ik` matches the `asset` for all notes. self.actions.iter().try_for_each(|action| { action.verify(&expected_ik)?; @@ -635,11 +668,12 @@ impl IssueBundle { IssueBundleAuthorizingCommitment(hash_issue_bundle_auth_data(self, sighash_info_for_kind)) } - /// Returns the note commitments for all notes in this bundle, in action order. + /// Returns the x-coordinates (`cmx`) of the note commitments for all notes + /// in this bundle, in action order. /// /// This exposes the commitments directly as `pasta_curves::pallas::Base` /// values for external crates, avoiding extra byte conversions. - pub fn note_commitments(&self) -> impl Iterator + '_ { + pub fn note_cmxs(&self) -> impl Iterator + '_ { self.actions().iter().flat_map(|action| { action .notes() @@ -675,14 +709,14 @@ pub fn check_issue_bundle_without_sighash( let (asset, amount) = action.verify(bundle.ik())?; + if new_records.contains_key(&asset) { + return Err(DuplicateIssueActionForAssetBase); + } + 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)) - { + let new_asset_record = match get_global_records(&asset) { // The first issuance of the asset None => AssetRecord::new( amount, @@ -752,6 +786,8 @@ pub fn check_issue_bundle_without_sighash( /// issuance of a new asset. /// * `IncorrectRhoDerivation`: If the `rho` value of any issuance note is not correctly derived /// from the `first_nullifier`. +/// * `DuplicateIssueActionForAssetBase`:If the bundle contains multiple `IssueAction`s for +/// the same asset. /// * **Other Errors**: Any additional errors returned by the `IssueAction::verify` method are /// propagated pub fn verify_issue_bundle( @@ -819,6 +855,9 @@ pub enum Error { AssetBaseCannotBeIdentityPoint, /// It cannot be first issuance because we have already some notes for this asset. CannotBeFirstIssuance, + /// A first issuance cannot be finalized immediately because no issue note would ever exist + /// for this asset. + CannotFinalizeOnFirstIssuance, /// Verification errors: /// Invalid issuance validating key. @@ -831,6 +870,8 @@ pub enum Error { IssueActionPreviouslyFinalizedAssetBase, /// The rho value of an issuance note is not correctly derived from the first nullifier. IncorrectRhoDerivation, + /// The bundle contains multiple `IssueAction`s for the same asset. + DuplicateIssueActionForAssetBase, /// Overflow error occurred while calculating the value of the asset ValueOverflow, @@ -869,6 +910,13 @@ impl fmt::Display for Error { "it cannot be first issuance because we have already some notes for this asset." ) } + CannotFinalizeOnFirstIssuance => { + write!( + f, + "a first issuance cannot be finalized immediately because no issue note would + ever exist for this asset." + ) + } InvalidIssueValidatingKey => { write!(f, "invalid issuance validating key") } @@ -884,6 +932,12 @@ impl fmt::Display for Error { IncorrectRhoDerivation => { write!(f, "incorrect rho value") } + DuplicateIssueActionForAssetBase => { + write!( + f, + "the bundle contains multiple `IssueAction`s for the same asset" + ) + } ValueOverflow => { write!( f, @@ -904,8 +958,8 @@ impl fmt::Display for Error { mod tests { use crate::{ issuance::Error::{ - CannotBeFirstIssuance, IncorrectRhoDerivation, InvalidIssueBundleSig, - InvalidIssueValidatingKey, IssueActionNotFound, + CannotBeFirstIssuance, CannotFinalizeOnFirstIssuance, IncorrectRhoDerivation, + InvalidIssueBundleSig, InvalidIssueValidatingKey, IssueActionPreviouslyFinalizedAssetBase, IssueActionWithoutNoteNotFinalized, IssueBundleIkMismatchAssetBase, MissingReferenceNoteOnFirstIssuance, ValueOverflow, }, @@ -1022,7 +1076,8 @@ mod tests { }), true, params.rng, - ); + ) + .unwrap(); (sign_bundle(bundle, params), asset) } @@ -1118,7 +1173,8 @@ mod tests { }), true, rng, - ); + ) + .unwrap(); let another_asset = bundle .add_recipient( @@ -1208,18 +1264,13 @@ mod tests { }), true, rng, - ); + ) + .unwrap(); - bundle - .finalize_action(&nft_asset_desc_hash) - .expect("Should finalize properly"); + bundle.finalize_action(&nft_asset_desc_hash); - assert_eq!( - bundle - .finalize_action(&another_nft_asset_desc_hash) - .unwrap_err(), - IssueActionNotFound - ); + // Finalize an asset that does not yet exist in the IssueBundle. + bundle.finalize_action(&another_nft_asset_desc_hash); } #[test] @@ -1244,7 +1295,8 @@ mod tests { }), true, rng, - ); + ) + .unwrap(); let prepared = bundle.update_rho(&first_nullifier, rng).prepare(sighash); assert_eq!(prepared.authorization().sighash, sighash); @@ -1275,7 +1327,8 @@ mod tests { }), true, params.rng, - ); + ) + .unwrap(); let wrong_isk = IssueAuthKey::::random(&mut rng); @@ -1285,7 +1338,7 @@ mod tests { .sign(&wrong_isk) .expect_err("should not be able to sign"); - assert_eq!(err, IssueBundleIkMismatchAssetBase); + assert_eq!(err, InvalidIssueValidatingKey); } #[test] @@ -1303,7 +1356,8 @@ mod tests { }), true, params.rng, - ); + ) + .unwrap(); let note = Note::new_issue_note( params.recipient, @@ -1358,9 +1412,10 @@ mod tests { }), true, params.rng, - ); + ) + .unwrap(); - bundle.finalize_action(&hash).unwrap(); + bundle.finalize_action(&hash); let signed = sign_bundle(bundle, ¶ms); @@ -1399,7 +1454,8 @@ mod tests { }), true, params.rng, - ); + ) + .unwrap(); bundle .add_recipient( @@ -1411,7 +1467,7 @@ mod tests { ) .unwrap(); - bundle.finalize_action(&asset1_desc_hash).unwrap(); + bundle.finalize_action(&asset1_desc_hash); bundle .add_recipient( @@ -1423,7 +1479,7 @@ mod tests { ) .unwrap(); - bundle.finalize_action(&asset2_desc_hash).unwrap(); + bundle.finalize_action(&asset2_desc_hash); bundle .add_recipient( @@ -1475,7 +1531,7 @@ mod tests { // Verify note_commitments() returns a correct number of non-zero, // unique pallas::Base values let mut unique_commitments = BTreeSet::new(); - for commitment in signed.note_commitments() { + for commitment in signed.note_cmxs() { assert_ne!(commitment, pallas::Base::zero()); assert!(unique_commitments.insert(commitment)); } @@ -1575,7 +1631,8 @@ mod tests { }), true, params.rng, - ); + ) + .unwrap(); // Sign with zeroed sighash, then verify with the random one let signed = bundle @@ -1684,7 +1741,8 @@ mod tests { }), true, rng, - ); + ) + .unwrap(); // NOTE: Equality between two IssueActions can only be tested once `rho` is initialized. // This call is required for the final `assert_eq!`. @@ -1732,7 +1790,8 @@ mod tests { }), false, // no reference note params.rng, - ); + ) + .unwrap(); let signed = sign_bundle(bundle, ¶ms); assert_eq!( @@ -1758,7 +1817,8 @@ mod tests { }), false, // not first issuance params.rng, - ); + ) + .unwrap(); let signed = sign_bundle(bundle, ¶ms); let mut rng = OsRng; @@ -1793,7 +1853,8 @@ mod tests { }), false, params.rng, - ); + ) + .unwrap(); let signed = sign_bundle(bundle, ¶ms); let mut rng = OsRng; @@ -1838,7 +1899,8 @@ mod tests { }), true, rng, - ); + ) + .unwrap(); assert_eq!( bundle @@ -1950,7 +2012,8 @@ mod tests { }), true, rng, - ); + ) + .unwrap(); let another_asset = bundle .add_recipient( @@ -2027,9 +2090,26 @@ mod tests { None, // no notes, finalize-only false, params.rng, - ); + ) + .unwrap(); let signed = sign_bundle(bundle, ¶ms); - assert_eq!(signed.note_commitments().count(), 0); + assert_eq!(signed.note_cmxs().count(), 0); + } + + #[test] + fn cannot_finalize_a_first_issuance_immediately() { + let params = setup_params(); + assert_eq!( + IssueBundle::new( + params.ik.clone(), + asset_desc_hash(b"asset1"), + None, // no notes, finalize-only + true, + params.rng, + ) + .unwrap_err(), + CannotFinalizeOnFirstIssuance + ); } } diff --git a/src/note/asset_base.rs b/src/note/asset_base.rs index 602cced94..5b4a26329 100644 --- a/src/note/asset_base.rs +++ b/src/note/asset_base.rs @@ -177,7 +177,6 @@ impl AssetBase { impl Hash for AssetBase { fn hash(&self, h: &mut H) { h.write(&self.to_bytes()); - let _ = h.finish(); } } diff --git a/tests/issuance_global_state.rs b/tests/issuance_global_state.rs index 58b2ed02c..13d1c1aea 100644 --- a/tests/issuance_global_state.rs +++ b/tests/issuance_global_state.rs @@ -141,10 +141,11 @@ fn build_issue_bundle(params: &TestParams, data: &[IssueTestNote]) -> IssueBundl }), first_issuance, rng, - ); + ) + .unwrap(); if is_finalized { - bundle.finalize_action(&asset_desc_hash).unwrap(); + bundle.finalize_action(&asset_desc_hash); } for IssueTestNote { @@ -166,7 +167,7 @@ fn build_issue_bundle(params: &TestParams, data: &[IssueTestNote]) -> IssueBundl .unwrap(); if is_finalized { - bundle.finalize_action(&asset_desc_hash).unwrap(); + bundle.finalize_action(&asset_desc_hash); } } diff --git a/tests/zsa.rs b/tests/zsa.rs index c3da73ecb..2e309c57c 100644 --- a/tests/zsa.rs +++ b/tests/zsa.rs @@ -163,7 +163,8 @@ fn issue_zsa_notes( }), true, &mut rng, - ); + ) + .unwrap(); assert!(awaiting_nullifier_bundle .add_recipient(