From fa8481bdd8195915c6a1e14aa4d4e0911dfcc92b Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Mon, 7 Oct 2024 15:31:48 +0200 Subject: [PATCH 1/3] chore: Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d835030c9..2d35f55ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ - [BREAKING] Moved `MAX_NUM_FOREIGN_ACCOUNTS` into `miden-objects` (#904). - Implemented `storage_size`, updated storage bounds (#886). - [BREAKING] Auto-generate `KERNEL_ERRORS` list from the transaction kernel's MASM files and rework error constant names (#906). +- Implement `Serializable` for `FungibleAsset` (#907). ## 0.5.1 (2024-08-28) - `miden-objects` crate only From 0bbe3e694e998b52fd3f32188b8548bd7b148e43 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Wed, 9 Oct 2024 11:21:13 +0200 Subject: [PATCH 2/3] feat(objects): Rework and optimize asset serialization --- objects/src/assets/fungible.rs | 62 +++++++++++++-------- objects/src/assets/mod.rs | 89 ++++++++++--------------------- objects/src/assets/nonfungible.rs | 52 +++++++++--------- 3 files changed, 94 insertions(+), 109 deletions(-) diff --git a/objects/src/assets/fungible.rs b/objects/src/assets/fungible.rs index 1622ed1d0..9b6e8267d 100644 --- a/objects/src/assets/fungible.rs +++ b/objects/src/assets/fungible.rs @@ -1,11 +1,14 @@ use alloc::string::ToString; use core::fmt; -use vm_core::FieldElement; +use vm_core::{ + utils::{ByteReader, ByteWriter, Deserializable, Serializable}, + FieldElement, +}; +use vm_processor::DeserializationError; use super::{ - is_not_a_non_fungible_asset, parse_word, AccountId, AccountType, Asset, AssetError, Felt, Word, - ZERO, + is_not_a_non_fungible_asset, AccountId, AccountType, Asset, AssetError, Felt, Word, ZERO, }; // FUNGIBLE ASSET @@ -145,16 +148,6 @@ impl From for Word { } } -impl From for [u8; 32] { - fn from(asset: FungibleAsset) -> Self { - let mut result = [0_u8; 32]; - let id_bytes: [u8; 8] = asset.faucet_id.into(); - result[..8].copy_from_slice(&asset.amount.to_le_bytes()); - result[24..].copy_from_slice(&id_bytes); - result - } -} - impl From for Asset { fn from(asset: FungibleAsset) -> Self { Asset::Fungible(asset) @@ -175,17 +168,44 @@ impl TryFrom for FungibleAsset { } } -impl TryFrom<[u8; 32]> for FungibleAsset { - type Error = AssetError; +impl fmt::Display for FungibleAsset { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:?}", self) + } +} - fn try_from(value: [u8; 32]) -> Result { - let word = parse_word(value)?; - Self::try_from(word) +// SERIALIZATION +// ================================================================================================ + +impl Serializable for FungibleAsset { + fn write_into(&self, target: &mut W) { + // All assets should serialize their faucet ID at the first position to allow them to be + // easily distinguishable during deserialization. + target.write(self.faucet_id); + target.write(self.amount); + } + + fn get_size_hint(&self) -> usize { + self.faucet_id.get_size_hint() + self.amount.get_size_hint() } } -impl fmt::Display for FungibleAsset { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{:?}", self) +impl Deserializable for FungibleAsset { + fn read_from(source: &mut R) -> Result { + let faucet_id: AccountId = source.read()?; + FungibleAsset::deserialize_with_account_id(faucet_id, source) + } +} + +impl FungibleAsset { + /// Deserializes a [`FungibleAsset`] from an [`AccountId`] and the remaining data from the given + /// `source`. + pub(super) fn deserialize_with_account_id( + faucet_id: AccountId, + source: &mut R, + ) -> Result { + let amount: u64 = source.read()?; + FungibleAsset::new(faucet_id, amount) + .map_err(|err| DeserializationError::InvalidValue(err.to_string())) } } diff --git a/objects/src/assets/mod.rs b/objects/src/assets/mod.rs index 59251acd6..fdf215843 100644 --- a/objects/src/assets/mod.rs +++ b/objects/src/assets/mod.rs @@ -1,5 +1,3 @@ -use alloc::string::ToString; - use super::{ accounts::{AccountId, AccountType, ACCOUNT_ISFAUCET_MASK}, utils::serde::{ByteReader, ByteWriter, Deserializable, DeserializationError, Serializable}, @@ -70,12 +68,6 @@ pub enum Asset { } impl Asset { - // CONSTANTS - // -------------------------------------------------------------------------------------------- - - /// The serialized size of an [`Asset`] in bytes. - pub const SERIALIZED_SIZE: usize = 32; - /// Creates a new [Asset] without checking its validity. pub(crate) fn new_unchecked(value: Word) -> Asset { if is_not_a_non_fungible_asset(value) { @@ -152,21 +144,6 @@ impl From<&Asset> for Word { } } -impl From for [u8; Asset::SERIALIZED_SIZE] { - fn from(asset: Asset) -> Self { - match asset { - Asset::Fungible(asset) => asset.into(), - Asset::NonFungible(asset) => asset.into(), - } - } -} - -impl From<&Asset> for [u8; Asset::SERIALIZED_SIZE] { - fn from(value: &Asset) -> Self { - (*value).into() - } -} - impl TryFrom<&Word> for Asset { type Error = AssetError; @@ -187,64 +164,51 @@ impl TryFrom for Asset { } } -impl TryFrom<[u8; Asset::SERIALIZED_SIZE]> for Asset { - type Error = AssetError; - - fn try_from(value: [u8; Asset::SERIALIZED_SIZE]) -> Result { - parse_word(value)?.try_into() - } -} - -impl TryFrom<&[u8; Asset::SERIALIZED_SIZE]> for Asset { - type Error = AssetError; - - fn try_from(value: &[u8; Asset::SERIALIZED_SIZE]) -> Result { - (*value).try_into() - } -} - // SERIALIZATION // ================================================================================================ impl Serializable for Asset { fn write_into(&self, target: &mut W) { - let data: [u8; Asset::SERIALIZED_SIZE] = self.into(); - target.write_bytes(&data); + match self { + Asset::Fungible(fungible_asset) => fungible_asset.write_into(target), + Asset::NonFungible(non_fungible_asset) => non_fungible_asset.write_into(target), + } } fn get_size_hint(&self) -> usize { - Asset::SERIALIZED_SIZE + match self { + Asset::Fungible(fungible_asset) => fungible_asset.get_size_hint(), + Asset::NonFungible(non_fungible_asset) => non_fungible_asset.get_size_hint(), + } } } impl Deserializable for Asset { fn read_from(source: &mut R) -> Result { - let data_vec = source.read_vec(Asset::SERIALIZED_SIZE)?; - let data_array: [u8; Asset::SERIALIZED_SIZE] = - data_vec.try_into().expect("Vec must be of size 32"); - - let asset = Asset::try_from(&data_array) - .map_err(|error| DeserializationError::InvalidValue(format!("{error}")))?; - Ok(asset) + // Both asset types have their faucet ID as the first element, so we can use it to inspect + // what type of asset it is. + let account_id: AccountId = source.read()?; + let account_type = account_id.account_type(); + + match account_type { + AccountType::FungibleFaucet => { + FungibleAsset::deserialize_with_account_id(account_id, source).map(Asset::from) + }, + AccountType::NonFungibleFaucet => { + NonFungibleAsset::deserialize_with_account_id(account_id, source).map(Asset::from) + }, + other_type => { + Err(DeserializationError::InvalidValue(format!( + "failed to deserialize asset: expected an account ID of type faucet, found {other_type:?}" + ))) + }, + } } } // HELPER FUNCTIONS // ================================================================================================ -fn parse_word(bytes: [u8; Asset::SERIALIZED_SIZE]) -> Result { - Ok([ - parse_felt(&bytes[..8])?, - parse_felt(&bytes[8..16])?, - parse_felt(&bytes[16..24])?, - parse_felt(&bytes[24..])?, - ]) -} - -fn parse_felt(bytes: &[u8]) -> Result { - Felt::try_from(bytes).map_err(|err| AssetError::InvalidFieldElement(err.to_string())) -} - /// Returns `true` if asset in [Word] is not a non-fungible asset. /// /// Note: this does not mean that the word is a fungible asset as the word may contain an value @@ -260,6 +224,7 @@ fn is_not_a_non_fungible_asset(asset: Word) -> bool { #[cfg(test)] mod tests { + use miden_crypto::{ utils::{Deserializable, Serializable}, Word, diff --git a/objects/src/assets/nonfungible.rs b/objects/src/assets/nonfungible.rs index 7de393e26..fa9c2a3a2 100644 --- a/objects/src/assets/nonfungible.rs +++ b/objects/src/assets/nonfungible.rs @@ -3,10 +3,7 @@ use core::fmt; use vm_core::{FieldElement, WORD_SIZE}; -use super::{ - parse_word, AccountId, AccountType, Asset, AssetError, Felt, Hasher, Word, - ACCOUNT_ISFAUCET_MASK, -}; +use super::{AccountId, AccountType, Asset, AssetError, Felt, Hasher, Word, ACCOUNT_ISFAUCET_MASK}; use crate::{ utils::{ByteReader, ByteWriter, Deserializable, DeserializationError, Serializable}, Digest, @@ -149,17 +146,6 @@ impl From for Word { } } -impl From for [u8; 32] { - fn from(asset: NonFungibleAsset) -> Self { - let mut result = [0_u8; 32]; - result[..8].copy_from_slice(&asset.0[0].as_int().to_le_bytes()); - result[8..16].copy_from_slice(&asset.0[FAUCET_ID_POS].as_int().to_le_bytes()); - result[16..24].copy_from_slice(&asset.0[2].as_int().to_le_bytes()); - result[24..].copy_from_slice(&asset.0[3].as_int().to_le_bytes()); - result - } -} - impl From for Asset { fn from(asset: NonFungibleAsset) -> Self { Asset::NonFungible(asset) @@ -176,24 +162,23 @@ impl TryFrom for NonFungibleAsset { } } -impl TryFrom<[u8; 32]> for NonFungibleAsset { - type Error = AssetError; - - fn try_from(value: [u8; 32]) -> Result { - let word = parse_word(value)?; - Self::try_from(word) - } -} - impl fmt::Display for NonFungibleAsset { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{:?}", self) } } +// SERIALIZATION +// ================================================================================================ + impl Serializable for NonFungibleAsset { fn write_into(&self, target: &mut W) { - target.write(self.0) + // All assets should serialize their faucet ID at the first position to allow them to be + // easily distinguishable during deserialization. + target.write(self.0[FAUCET_ID_POS]); + target.write(self.0[0]); + target.write(self.0[2]); + target.write(self.0[3]); } fn get_size_hint(&self) -> usize { @@ -204,11 +189,26 @@ impl Serializable for NonFungibleAsset { impl Deserializable for NonFungibleAsset { fn read_from(source: &mut R) -> Result { let value: Word = source.read()?; - Self::try_from(value).map_err(|err| DeserializationError::InvalidValue(err.to_string())) } } +impl NonFungibleAsset { + /// Deserializes a [`NonFungibleAsset`] from an [`AccountId`] and the remaining data from the + /// given `source`. + pub(super) fn deserialize_with_account_id( + faucet_id: AccountId, + source: &mut R, + ) -> Result { + let hash_0: Felt = source.read()?; + let hash_2: Felt = source.read()?; + let hash_3: Felt = source.read()?; + + NonFungibleAsset::from_parts(faucet_id, [hash_0, Felt::ZERO, hash_2, hash_3]) + .map_err(|err| DeserializationError::InvalidValue(err.to_string())) + } +} + // NON-FUNGIBLE ASSET DETAILS // ================================================================================================ From 2ea51387bd5be3ac295fd07c4a3cb4694761f8b6 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Mon, 14 Oct 2024 09:28:40 +0200 Subject: [PATCH 3/3] feat(objects): Fix Deserializable for Non Fungible Asset --- objects/src/assets/fungible.rs | 39 ++++++++++++++++++++++++++ objects/src/assets/nonfungible.rs | 46 +++++++++++++++++++++++++++++-- 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/objects/src/assets/fungible.rs b/objects/src/assets/fungible.rs index 9b6e8267d..d9a9bd78b 100644 --- a/objects/src/assets/fungible.rs +++ b/objects/src/assets/fungible.rs @@ -209,3 +209,42 @@ impl FungibleAsset { .map_err(|err| DeserializationError::InvalidValue(err.to_string())) } } + +// TESTS +// ================================================================================================ + +#[cfg(test)] +mod tests { + use super::*; + use crate::accounts::account_id::testing::{ + ACCOUNT_ID_FUNGIBLE_FAUCET_OFF_CHAIN, ACCOUNT_ID_FUNGIBLE_FAUCET_ON_CHAIN, + ACCOUNT_ID_FUNGIBLE_FAUCET_ON_CHAIN_1, ACCOUNT_ID_FUNGIBLE_FAUCET_ON_CHAIN_2, + ACCOUNT_ID_FUNGIBLE_FAUCET_ON_CHAIN_3, ACCOUNT_ID_NON_FUNGIBLE_FAUCET_OFF_CHAIN, + }; + + #[test] + fn test_fungible_asset_serde() { + for fungible_account_id in [ + ACCOUNT_ID_FUNGIBLE_FAUCET_OFF_CHAIN, + ACCOUNT_ID_FUNGIBLE_FAUCET_ON_CHAIN, + ACCOUNT_ID_FUNGIBLE_FAUCET_ON_CHAIN_1, + ACCOUNT_ID_FUNGIBLE_FAUCET_ON_CHAIN_2, + ACCOUNT_ID_FUNGIBLE_FAUCET_ON_CHAIN_3, + ] { + let account_id = AccountId::try_from(fungible_account_id).unwrap(); + let fungible_asset = FungibleAsset::new(account_id, 10).unwrap(); + assert_eq!( + fungible_asset, + FungibleAsset::read_from_bytes(&fungible_asset.to_bytes()).unwrap() + ); + } + + let account_id = AccountId::try_from(ACCOUNT_ID_FUNGIBLE_FAUCET_ON_CHAIN_3).unwrap(); + let asset = FungibleAsset::new(account_id, 50).unwrap(); + let mut asset_bytes = asset.to_bytes(); + // Set invalid Faucet ID. + asset_bytes[0..8].copy_from_slice(&ACCOUNT_ID_NON_FUNGIBLE_FAUCET_OFF_CHAIN.to_le_bytes()); + let err = FungibleAsset::read_from_bytes(&asset_bytes).unwrap_err(); + assert!(matches!(err, DeserializationError::InvalidValue(_))); + } +} diff --git a/objects/src/assets/nonfungible.rs b/objects/src/assets/nonfungible.rs index fa9c2a3a2..9f18692d0 100644 --- a/objects/src/assets/nonfungible.rs +++ b/objects/src/assets/nonfungible.rs @@ -188,8 +188,10 @@ impl Serializable for NonFungibleAsset { impl Deserializable for NonFungibleAsset { fn read_from(source: &mut R) -> Result { - let value: Word = source.read()?; - Self::try_from(value).map_err(|err| DeserializationError::InvalidValue(err.to_string())) + let faucet_id: AccountId = source.read()?; + + Self::deserialize_with_account_id(faucet_id, source) + .map_err(|err| DeserializationError::InvalidValue(err.to_string())) } } @@ -204,6 +206,8 @@ impl NonFungibleAsset { let hash_2: Felt = source.read()?; let hash_3: Felt = source.read()?; + // The second felt in the data_hash will be replaced by the faucet id, so we can set it to + // zero here. NonFungibleAsset::from_parts(faucet_id, [hash_0, Felt::ZERO, hash_2, hash_3]) .map_err(|err| DeserializationError::InvalidValue(err.to_string())) } @@ -244,3 +248,41 @@ impl NonFungibleAssetDetails { &self.asset_data } } + +// TESTS +// ================================================================================================ + +#[cfg(test)] +mod tests { + use super::*; + use crate::accounts::account_id::testing::{ + ACCOUNT_ID_FUNGIBLE_FAUCET_OFF_CHAIN, ACCOUNT_ID_NON_FUNGIBLE_FAUCET_OFF_CHAIN, + ACCOUNT_ID_NON_FUNGIBLE_FAUCET_ON_CHAIN, ACCOUNT_ID_NON_FUNGIBLE_FAUCET_ON_CHAIN_1, + }; + + #[test] + fn test_non_fungible_asset_serde() { + for non_fungible_account_id in [ + ACCOUNT_ID_NON_FUNGIBLE_FAUCET_OFF_CHAIN, + ACCOUNT_ID_NON_FUNGIBLE_FAUCET_ON_CHAIN, + ACCOUNT_ID_NON_FUNGIBLE_FAUCET_ON_CHAIN_1, + ] { + let account_id = AccountId::try_from(non_fungible_account_id).unwrap(); + let details = NonFungibleAssetDetails::new(account_id, vec![1, 2, 3]).unwrap(); + let non_fungible_asset = NonFungibleAsset::new(&details).unwrap(); + assert_eq!( + non_fungible_asset, + NonFungibleAsset::read_from_bytes(&non_fungible_asset.to_bytes()).unwrap() + ); + } + + let account = AccountId::try_from(ACCOUNT_ID_NON_FUNGIBLE_FAUCET_OFF_CHAIN).unwrap(); + let details = NonFungibleAssetDetails::new(account, vec![4, 5, 6, 7]).unwrap(); + let asset = NonFungibleAsset::new(&details).unwrap(); + let mut asset_bytes = asset.to_bytes(); + // Set invalid Faucet ID. + asset_bytes[0..8].copy_from_slice(&ACCOUNT_ID_FUNGIBLE_FAUCET_OFF_CHAIN.to_le_bytes()); + let err = NonFungibleAsset::read_from_bytes(&asset_bytes).unwrap_err(); + assert!(matches!(err, DeserializationError::InvalidValue(_))); + } +}