Skip to content

Update AssetSuply and SupplyInfo (new) #133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dmidem merged 21 commits into from
Jan 8, 2025
Merged

Update AssetSuply and SupplyInfo (new) #133

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
2bee134
Update AssetSupply
ConstanceBeguier Dec 19, 2024
01eb3e3
Add reference_notes into SupplyInfo
ConstanceBeguier Dec 19, 2024
60e1854
Apply suggestions from code review
PaulLaux Dec 25, 2024
6c540dd
Move get_reference_notes to the first IssueBundle impl block
dmidem Dec 26, 2024
c9f8ba3
Rename ValueSumOverflow issuance error variant to ValueOverflow as th…
dmidem Dec 26, 2024
c30e108
Pin yeslogic-fontconfig-sys to 5.0.0 as 6.0.0 requires fontconfig Ubu…
dmidem Dec 27, 2024
9d1fcfa
Revert "Pin yeslogic-fontconfig-sys to 5.0.0 as 6.0.0 requires fontco…
dmidem Dec 27, 2024
19ee69f
Update burn_validation to use NoteValue instead of i64
dmidem Dec 30, 2024
4f43896
Modify GitHub CI workflow to install libfontconfig1-dev on Ubuntu, wh…
dmidem Dec 31, 2024
a60428e
Use if runner.os in CI config instead of matrix.os check
dmidem Dec 31, 2024
d7973db
Merge branch 'zsa1' into update_asset_supply
dmidem Dec 31, 2024
2cce7db
Add reference_note field to AssetSupply struct, remove reference_note…
dmidem Jan 5, 2025
5b852f0
Remove get_reference_notes function as there's an action's menthod ge…
dmidem Jan 6, 2025
2bd20ab
Merge branch 'zsa1' into update_asset_supply
dmidem Jan 7, 2025
062dfca
Address review comments by updating function signatures and documenta…
dmidem Jan 8, 2025
c07e872
Minor fixes in burn_validation and supply_info.rs
dmidem Jan 8, 2025
1c7ef6c
Changed IssueAction::get_reference_note method logic to take the firs…
dmidem Jan 8, 2025
b3ed468
Return lifetime in supply info sum test
dmidem Jan 8, 2025
967cf84
Add is_reference_note function and use it in get_reference_note and t…
dmidem Jan 8, 2025
124dddf
Updated doc for get_reference_note
dmidem Jan 8, 2025
495c5d4
Use HashSet instead of HashMap in validate_bundle_burn
dmidem Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 25 additions & 23 deletions src/bundle/burn_validation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
//!
//! The module provides a function `validate_bundle_burn` that can be used to validate the burn values for the bundle.
//!
use std::fmt;
use std::{collections::HashMap, fmt};

use crate::note::AssetBase;
use crate::{note::AssetBase, value::NoteValue};

/// Possible errors that can occur during bundle burn validation.
#[derive(Debug)]
Expand All @@ -14,36 +14,36 @@ pub enum BurnError {
DuplicateAsset,
/// Cannot burn a native asset.
NativeAsset,
/// Cannot burn an asset with a non-positive value.
NonPositiveAmount,
/// Cannot burn an asset with a zero value.
ZeroAmount,
}

/// Validates burn for a bundle by ensuring each asset is unique, non-native, and has a positive value.
/// Validates burn for a bundle by ensuring each asset is unique, non-native, and has a non-zero value.
///
/// Each burn element is represented as a tuple of `AssetBase` and `i64` (value for the burn).
/// Each burn element is represented as a tuple of `AssetBase` and `NoteValue` (value for the burn).
///
/// # Arguments
///
/// * `burn` - A vector of assets, where each asset is represented as a tuple of `AssetBase` and `i64` (value the burn).
/// * `burn` - A vector of assets, where each asset is represented as a tuple of `AssetBase` and `NoteValue` (value the burn).
///
/// # Errors
///
/// Returns a `BurnError` if:
/// * Any asset in the `burn` vector is not unique (`BurnError::DuplicateAsset`).
/// * Any asset in the `burn` vector is native (`BurnError::NativeAsset`).
/// * Any asset in the `burn` vector has a non-positive value (`BurnError::NonPositiveAmount`).
pub fn validate_bundle_burn(bundle_burn: &Vec<(AssetBase, i64)>) -> Result<(), BurnError> {
let mut asset_set = std::collections::HashSet::<&AssetBase>::new();
/// * Any asset in the `burn` vector has a zero value (`BurnError::ZeroAmount`).
/// * Any asset in the `burn` vector is not unique (`BurnError::DuplicateAsset`).
pub fn validate_bundle_burn(burn: &[(AssetBase, NoteValue)]) -> Result<(), BurnError> {
let mut burn_set = HashMap::<AssetBase, NoteValue>::new();
Comment thread
PaulLaux marked this conversation as resolved.
Outdated

for (asset, value) in bundle_burn {
if !asset_set.insert(asset) {
return Err(BurnError::DuplicateAsset);
}
for (asset, value) in burn {
if asset.is_native().into() {
return Err(BurnError::NativeAsset);
}
if *value <= 0 {
return Err(BurnError::NonPositiveAmount);
if value.inner() == 0 {
return Err(BurnError::ZeroAmount);
}
if burn_set.insert(*asset, *value).is_some() {
return Err(BurnError::DuplicateAsset);
}
}

Expand All @@ -55,15 +55,17 @@ impl fmt::Display for BurnError {
match *self {
BurnError::DuplicateAsset => write!(f, "Encountered a duplicate asset to burn."),
BurnError::NativeAsset => write!(f, "Cannot burn a native asset."),
BurnError::NonPositiveAmount => {
write!(f, "Cannot burn an asset with a non-positive value.")
BurnError::ZeroAmount => {
write!(f, "Cannot burn an asset with a zero value.")
}
}
}
}

#[cfg(test)]
mod tests {
use crate::value::NoteValue;

use super::*;

/// Creates an item of bundle burn list for a given asset description and value.
Expand All @@ -80,14 +82,14 @@ mod tests {
///
/// A tuple `(AssetBase, Amount)` representing the burn list item.
///
pub fn get_burn_tuple(asset_desc: &[u8], value: i64) -> (AssetBase, i64) {
pub fn get_burn_tuple(asset_desc: &[u8], value: u64) -> (AssetBase, NoteValue) {
use crate::keys::{IssuanceAuthorizingKey, IssuanceValidatingKey};

let isk = IssuanceAuthorizingKey::from_bytes([1u8; 32]).unwrap();

(
AssetBase::derive(&IssuanceValidatingKey::from(&isk), asset_desc),
value,
NoteValue::from_raw(value),
)
}

Expand Down Expand Up @@ -121,7 +123,7 @@ mod tests {
fn validate_bundle_burn_native_asset() {
let bundle_burn = vec![
get_burn_tuple(b"Asset 1", 10),
(AssetBase::native(), 20),
(AssetBase::native(), NoteValue::from_raw(20)),
get_burn_tuple(b"Asset 3", 10),
];

Expand All @@ -140,6 +142,6 @@ mod tests {

let result = validate_bundle_burn(&bundle_burn);

assert_eq!(result, Err(BurnError::NonPositiveAmount));
assert_eq!(result, Err(BurnError::ZeroAmount));
}
}
91 changes: 50 additions & 41 deletions src/issuance.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,21 @@ use group::Group;
use k256::schnorr;
use nonempty::NonEmpty;
use rand::RngCore;
use std::collections::{HashMap, HashSet};
use std::collections::HashSet;
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, IssueActionNotFound, IssueActionPreviouslyFinalizedAssetBase,
IssueActionWithoutNoteNotFinalized, IssueBundleIkMismatchAssetBase,
IssueBundleInvalidSignature, ValueSumOverflow, WrongAssetDescSize,
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};

use crate::value::{NoteValue, ValueSum};
use crate::value::NoteValue;
use crate::{Address, Note};

use crate::supply_info::{AssetSupply, SupplyInfo};
Expand Down Expand Up @@ -114,7 +114,7 @@ impl IssueAction {
///
/// This function may return an error in any of the following cases:
///
/// * `ValueSumOverflow`: If the total amount value of all notes in the `IssueAction` overflows.
/// * `ValueOverflow`: If the total amount value of all notes in the `IssueAction` overflows.
///
/// * `IssueBundleIkMismatchAssetBase`: If the provided `ik` is not used to derive the
/// `AssetBase` for **all** internal notes.
Expand All @@ -132,7 +132,7 @@ impl IssueAction {
let value_sum = self
.notes
.iter()
.try_fold(ValueSum::zero(), |value_sum, &note| {
.try_fold(NoteValue::zero(), |value_sum, &note| {
//The asset base should not be the identity point of the Pallas curve.
if bool::from(note.asset().cv_base().is_identity()) {
return Err(AssetBaseCannotBeIdentityPoint);
Expand All @@ -145,12 +145,16 @@ impl IssueAction {
.ok_or(IssueBundleIkMismatchAssetBase)?;

// The total amount should not overflow
(value_sum + note.value()).ok_or(ValueSumOverflow)
(value_sum + note.value()).ok_or(ValueOverflow)
})?;

Ok((
issue_asset,
AssetSupply::new(value_sum, self.is_finalized()),
AssetSupply::new(
value_sum,
self.is_finalized(),
self.get_reference_note().cloned(),
),
))
}

Expand All @@ -163,6 +167,13 @@ impl IssueAction {
0b0000_0000
}
}

/// Returns the reference note.
pub fn get_reference_note(&self) -> Option<&Note> {
self.notes.iter().find(|note| {
(note.recipient() == ReferenceKeys::recipient()) && (note.value() == NoteValue::zero())
})
}
Comment on lines +172 to +187

@PaulLaux PaulLaux Jan 8, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So according to the finalized spec, the reference note must be the first note.
The logic should be:

  • get first note
  • check if it is the ref note
  • if yes, return.

with .find() we might get a non deterministic behavior.

@PaulLaux PaulLaux Jan 8, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use the existing verify_reference_note()

@dmidem dmidem Jan 8, 2025 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, if the first note doesn't match the reference note condition, it should return None even if, for example, there's the second note that does match the condition?

Now I changed it this way:

    /// Returns the reference note.
    pub fn get_reference_note(&self) -> Option<&Note> {
        self.notes.first().filter(|note| {
            (note.recipient() == ReferenceKeys::recipient()) && (note.value() == NoteValue::zero())
        })
    }

It considers the first note only, and Option::filter additionally checks the referenence note condition for the first note.

@PaulLaux PaulLaux Jan 8, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filter is fine
but the logic to verify a referance note repeat itself many times: see verify_reference_note() x2 in tests.
please extract and use the extracted fn ->bool here and in both tests.

Also update doc for this function.

@dmidem dmidem Jan 8, 2025 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I added new function:

fn is_reference_note(note: &Note) -> bool {
    note.value() == NoteValue::zero() && note.recipient() == ReferenceKeys::recipient()
}

and used it in get_reference_note and verify_reference_note, but verify_reference_note still has an additional check for asset matching, which does make sense for the tests, but looks duplicated for get_reference_note as we already checked this in IssueAction::verify_supply. So, I kept verify_reference_note as a separate function that calls is_reference_note and performs an additional check for the asset matching.

}

/// Defines the authorization type of an Issue bundle.
Expand Down Expand Up @@ -468,23 +479,6 @@ impl IssueBundle<Unauthorized> {
}
}

impl<T: IssueAuth> IssueBundle<T> {
/// Returns the reference notes for the `IssueBundle`.
pub fn get_reference_notes(self) -> HashMap<AssetBase, Note> {
let mut reference_notes = HashMap::new();
self.actions.iter().for_each(|action| {
action.notes.iter().for_each(|note| {
if (note.recipient() == ReferenceKeys::recipient())
&& (note.value() == NoteValue::zero())
{
reference_notes.insert(note.asset(), *note);
}
})
});
reference_notes
}
}

fn create_reference_note(asset: AssetBase, mut rng: impl RngCore) -> Note {
Note::new(
ReferenceKeys::recipient(),
Expand Down Expand Up @@ -561,12 +555,13 @@ impl IssueBundle<Signed> {
/// * For each `Note` inside an `IssueAction`:
/// * All notes have the same, correct `AssetBase`.
///
// # Returns
/// # Returns
///
/// A Result containing a SupplyInfo struct, which stores supply information in a HashMap.
/// The HashMap uses AssetBase as the key, and an AssetSupply struct as the value. The
/// AssetSupply contains a ValueSum (representing the total value of all notes for the asset)
/// and a bool indicating whether the asset is finalized.
/// 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).
///
/// # Errors
///
Expand All @@ -576,7 +571,7 @@ impl IssueBundle<Signed> {
/// asset in the bundle is incorrect.
/// * `IssueActionPreviouslyFinalizedAssetBase`: This error occurs if the asset has already been
/// finalized (inserted into the `finalized` collection).
/// * `ValueSumOverflow`: This error occurs if an overflow happens during the calculation of
/// * `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
Expand Down Expand Up @@ -636,7 +631,7 @@ pub enum Error {
IssueActionPreviouslyFinalizedAssetBase(AssetBase),

/// Overflow error occurred while calculating the value of the asset
ValueSumOverflow,
ValueOverflow,
}

impl fmt::Display for Error {
Expand Down Expand Up @@ -672,7 +667,7 @@ impl fmt::Display for Error {
IssueActionPreviouslyFinalizedAssetBase(_) => {
write!(f, "the provided `AssetBase` has been previously finalized")
}
ValueSumOverflow => {
ValueOverflow => {
write!(
f,
"overflow error occurred while calculating the value of the asset"
Expand All @@ -696,7 +691,7 @@ mod tests {
FullViewingKey, IssuanceAuthorizingKey, IssuanceValidatingKey, Scope, SpendingKey,
};
use crate::note::{AssetBase, Nullifier, Rho};
use crate::value::{NoteValue, ValueSum};
use crate::value::NoteValue;
use crate::{Address, Note};
use group::{Group, GroupEncoding};
use nonempty::NonEmpty;
Expand Down Expand Up @@ -829,7 +824,7 @@ mod tests {
let (asset, supply) = result.unwrap();

assert_eq!(asset, test_asset);
assert_eq!(supply.amount, ValueSum::from_raw(30));
assert_eq!(supply.amount, NoteValue::from_raw(30));
assert!(!supply.is_finalized);
}

Expand All @@ -854,7 +849,7 @@ mod tests {
let (asset, supply) = result.unwrap();

assert_eq!(asset, test_asset);
assert_eq!(supply.amount, ValueSum::from_raw(30));
assert_eq!(supply.amount, NoteValue::from_raw(30));
assert!(supply.is_finalized);
}

Expand Down Expand Up @@ -975,10 +970,8 @@ mod tests {
assert_eq!(first_note.value().inner(), 15);
assert_eq!(first_note.asset(), third_asset);

let reference_notes = bundle.get_reference_notes();
assert_eq!(reference_notes.len(), 2);
verify_reference_note(reference_notes.get(&asset).unwrap(), asset);
verify_reference_note(reference_notes.get(&third_asset).unwrap(), third_asset);
verify_reference_note(action.get_reference_note().unwrap(), asset);
verify_reference_note(action2.get_reference_note().unwrap(), third_asset);
}

#[test]
Expand Down Expand Up @@ -1227,17 +1220,33 @@ mod tests {

assert_eq!(supply_info.assets.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),
Some(&AssetSupply::new(ValueSum::from_raw(15), true))
Some(&AssetSupply::new(
NoteValue::from_raw(15),
true,
Some(reference_note1)
))
);
assert_eq!(
supply_info.assets.get(&asset2_base),
Some(&AssetSupply::new(ValueSum::from_raw(10), true))
Some(&AssetSupply::new(
NoteValue::from_raw(10),
true,
Some(reference_note2)
))
);
assert_eq!(
supply_info.assets.get(&asset3_base),
Some(&AssetSupply::new(ValueSum::from_raw(5), false))
Some(&AssetSupply::new(
NoteValue::from_raw(5),
false,
Some(reference_note3)
))
);
}

Expand Down
Loading