From 10b5281c11830597b41310bd71ac79e109d4e691 Mon Sep 17 00:00:00 2001 From: Ignacio Amigo Date: Thu, 16 Jan 2025 14:56:07 -0300 Subject: [PATCH 1/9] feat: Type storage placeholders and support for templated maps --- CHANGELOG.md | 1 + .../src/accounts/component/template/mod.rs | 36 ++++- .../component/template/storage/mod.rs | 123 +++++++++++------- .../component/template/storage/placeholder.rs | 10 ++ .../component/template/storage/toml.rs | 22 ++-- .../storage/{word.rs => value_type.rs} | 123 ++++++++++++++++-- objects/src/errors.rs | 4 + 7 files changed, 241 insertions(+), 78 deletions(-) rename objects/src/accounts/component/template/storage/{word.rs => value_type.rs} (72%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 910ef417d..35abfdef5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ - Made `AccountIdError` public (#1067). - Made `BasicFungibleFaucet::MAX_DECIMALS` public (#1063). - [BREAKING] Removed `miden-tx-prover` crate and created `miden-proving-service` and `miden-remote-provers` (#1047). +- Added storage placeholder types and support for templated map (#1074). ## 0.6.2 (2024-11-20) diff --git a/objects/src/accounts/component/template/mod.rs b/objects/src/accounts/component/template/mod.rs index 18e269175..4b70dd0a8 100644 --- a/objects/src/accounts/component/template/mod.rs +++ b/objects/src/accounts/component/template/mod.rs @@ -1,5 +1,5 @@ use alloc::{ - collections::BTreeSet, + collections::{btree_map::Entry, BTreeMap, BTreeSet}, string::{String, ToString}, vec::Vec, }; @@ -188,14 +188,14 @@ impl AccountComponentMetadata { /// be used for initializing storage slot values, or storage map entries. /// For a full example on how a placeholder may be utilized, refer to the docs /// for [AccountComponentMetadata]. - pub fn get_storage_placeholders(&self) -> BTreeSet { - let mut placeholder_set = BTreeSet::new(); + pub fn get_storage_placeholders(&self) -> BTreeMap { + let mut placeholder_map = BTreeMap::new(); for storage_entry in &self.storage { - for key in storage_entry.placeholders() { - placeholder_set.insert(key.clone()); + for (placeholder, placeholder_type) in storage_entry.placeholders() { + placeholder_map.insert(placeholder.clone(), placeholder_type); } } - placeholder_set + placeholder_map } /// Returns the name of the account component. @@ -254,6 +254,29 @@ impl AccountComponentMetadata { return Err(AccountComponentTemplateError::NonContiguousSlots(slots[0], slots[1])); } } + + // Check that placeholders do not appear more than once with a different type + let mut placeholders = BTreeMap::new(); + for storage_entry in &self.storage { + for (placeholder, placeholder_type) in storage_entry.placeholders() { + match placeholders.entry(placeholder.clone()) { + Entry::Occupied(entry) => { + // if already exists, make sure it's the same type + if *entry.get() != placeholder_type { + return Err( + AccountComponentTemplateError::StoragePlaceholderDuplicate( + placeholder.clone(), + ), + ); + } + }, + Entry::Vacant(slot) => { + slot.insert(placeholder_type); + }, + } + } + } + Ok(()) } } @@ -290,7 +313,6 @@ impl Deserializable for AccountComponentMetadata { #[cfg(test)] mod tests { - use assembly::Assembler; use assert_matches::assert_matches; use storage::WordRepresentation; diff --git a/objects/src/accounts/component/template/storage/mod.rs b/objects/src/accounts/component/template/storage/mod.rs index 4ff64ddea..3a5b70fe1 100644 --- a/objects/src/accounts/component/template/storage/mod.rs +++ b/objects/src/accounts/component/template/storage/mod.rs @@ -1,19 +1,16 @@ use alloc::{boxed::Box, string::String, vec::Vec}; -use vm_core::{ - utils::{ByteReader, ByteWriter, Deserializable, Serializable}, - Word, -}; -use vm_processor::{DeserializationError, Digest}; +use vm_core::utils::{ByteReader, ByteWriter, Deserializable, Serializable}; +use vm_processor::DeserializationError; -mod word; -pub use word::*; +mod value_type; +pub use value_type::*; use super::AccountComponentTemplateError; -use crate::accounts::{StorageMap, StorageSlot}; +use crate::accounts::StorageSlot; mod placeholder; -pub use placeholder::{StoragePlaceholder, StorageValue}; +pub use placeholder::{PlaceholderType, StoragePlaceholder, StorageValue}; mod init_storage_data; pub use init_storage_data::InitStorageData; @@ -54,7 +51,7 @@ pub enum StorageEntry { /// The numeric index of this map slot in the component's storage. slot: u8, /// A list of key-value pairs to initialize in this map slot. - map_entries: Vec, + map_entries: MapRepresentation, }, /// A multi-slot entry, representing a single logical value across multiple slots. @@ -91,13 +88,13 @@ impl StorageEntry { name: impl Into, description: Option>, slot: u8, - map_entries: Vec, + map_representation: MapRepresentation, ) -> Self { StorageEntry::Map { name: name.into(), description: description.map(Into::::into), slot, - map_entries, + map_entries: map_representation, } } @@ -167,24 +164,14 @@ impl StorageEntry { } } - /// Returns the map entries for a `Map` variant as a slice. - /// Returns an empty slice for non-map variants. - pub fn map_entries(&self) -> &[MapEntry] { - match self { - StorageEntry::Map { map_entries: values, .. } => values.as_slice(), - StorageEntry::Value { .. } => &[], - StorageEntry::MultiSlot { .. } => &[], - } - } - - /// Returns an iterator over all of the storage entries's placeholder keys. - // TODO: Should placeholders be typed? - pub fn placeholders(&self) -> Box + '_> { + /// Returns an iterator over all of the storage entries's placeholder keys, alongside their + /// expected type. + pub fn placeholders( + &self, + ) -> Box + '_> { match self { StorageEntry::Value { value, .. } => value.placeholders(), - StorageEntry::Map { map_entries: values, .. } => { - Box::new(values.iter().flat_map(|word| word.placeholders())) - }, + StorageEntry::Map { map_entries, .. } => map_entries.placeholders(), StorageEntry::MultiSlot { values, .. } => { Box::new(values.iter().flat_map(|word| word.placeholders())) }, @@ -209,16 +196,7 @@ impl StorageEntry { Ok(vec![StorageSlot::Value(slot)]) }, StorageEntry::Map { map_entries: values, .. } => { - let entries = values - .iter() - .map(|map_entry| { - let key = map_entry.key().try_build_word(init_storage_data)?; - let value = map_entry.value().try_build_word(init_storage_data)?; - Ok((key.into(), value)) - }) - .collect::, AccountComponentTemplateError>>()?; // Collect into a Vec and propagate errors - - let storage_map = StorageMap::with_entries(entries); + let storage_map = values.try_build_map(init_storage_data)?; Ok(vec![StorageSlot::Map(storage_map)]) }, StorageEntry::MultiSlot { values, .. } => Ok(values @@ -285,13 +263,13 @@ impl Deserializable for StorageEntry { // Map 1 => { let slot = source.read_u8()?; - let values: Vec = source.read()?; + let map_representation: MapRepresentation = source.read()?; Ok(StorageEntry::Map { name, description, slot, - map_entries: values, + map_entries: map_representation, }) }, @@ -335,7 +313,9 @@ impl MapEntry { &self.value } - pub fn placeholders(&self) -> impl Iterator { + /// Returns an iterator over all of the storage entries's placeholder keys, alongside their + /// expected type. + pub fn placeholders(&self) -> impl Iterator { self.key.placeholders().chain(self.value.placeholders()) } @@ -365,12 +345,14 @@ impl Deserializable for MapEntry { #[cfg(test)] mod tests { + use core::panic; use std::collections::BTreeSet; use assembly::Assembler; use assert_matches::assert_matches; use semver::Version; - use vm_core::{Felt, FieldElement}; + use vm_core::{Felt, FieldElement, Word}; + use vm_processor::Digest; use super::*; use crate::{ @@ -402,7 +384,7 @@ mod tests { name: "map".into(), description: Some("A storage map entry".into()), slot: 1, - map_entries: vec![ + map_entries: MapRepresentation::List(vec![ MapEntry { key: WordRepresentation::Template( StoragePlaceholder::new("foo.bar").unwrap(), @@ -419,7 +401,7 @@ mod tests { key: WordRepresentation::Hexadecimal(digest!("0x3").into()), value: WordRepresentation::Hexadecimal(digest!("0x4").into()), }, - ], + ]), }, StorageEntry::MultiSlot { name: "multi".into(), @@ -470,7 +452,7 @@ mod tests { slot = 0 values = [ { key = "0x1", value = ["{{value.test}}", "0x1", "0x2", "0x3"] }, - { key = "{{key.test}}", value = "0x3" }, + { key = "{{map.key.test}}", value = "0x3" }, { key = "0x3", value = "0x4" } ] @@ -488,12 +470,25 @@ mod tests { "{{word.test}}", ["1", "0", "0", "0"], ] + + [[storage]] + name = "map-template" + description = "a templated map" + slot = 4 + values = "{{map.template}}" "#; let component_metadata = AccountComponentMetadata::from_toml(toml_text).unwrap(); - let library = Assembler::default().assemble_library([CODE]).unwrap(); + for (key, placeholder_type) in component_metadata.get_storage_placeholders() { + match key.inner() { + "map.key.test" | "word.test" => assert_eq!(placeholder_type, PlaceholderType::Word), + "value.test" => assert_eq!(placeholder_type, PlaceholderType::Felt), + "map.template" => assert_eq!(placeholder_type, PlaceholderType::Map), + _ => panic!("all cases are covered"), + } + } - assert_eq!(component_metadata.storage_entries().first().unwrap().map_entries().len(), 3); + let library = Assembler::default().assemble_library([CODE]).unwrap(); let template = AccountComponentTemplate::new(component_metadata, library); @@ -504,7 +499,7 @@ mod tests { let storage_placeholders = InitStorageData::new([ ( - StoragePlaceholder::new("key.test").unwrap(), + StoragePlaceholder::new("map.key.test").unwrap(), StorageValue::Word(Default::default()), ), ( @@ -515,6 +510,10 @@ mod tests { StoragePlaceholder::new("word.test").unwrap(), StorageValue::Word([Felt::ZERO, Felt::ZERO, Felt::ZERO, Felt::new(128)]), ), + ( + StoragePlaceholder::new("map.template").unwrap(), + StorageValue::Map(vec![(Digest::default(), Word::default())]), + ), ]); let component = AccountComponent::from_template(&template, &storage_placeholders).unwrap(); @@ -542,4 +541,32 @@ mod tests { )) ); } + + #[test] + pub fn fail_duplicate_key() { + let toml_text = r#" + name = "Test Component" + description = "This is a test component" + version = "1.0.1" + targets = ["FungibleFaucet"] + + [[storage]] + name = "map" + description = "A storage map entry" + slot = 0 + values = [ + { key = "0x1", value = ["{{value.test}}", "0x1", "0x2", "0x3"] }, + ] + + [[storage]] + name = "word" + slot = 1 + value = "{{value.test}}" + "#; + let component_metadata = AccountComponentMetadata::from_toml(toml_text); + assert_matches!( + component_metadata, + Err(AccountComponentTemplateError::StoragePlaceholderDuplicate(_)) + ); + } } diff --git a/objects/src/accounts/component/template/storage/placeholder.rs b/objects/src/accounts/component/template/storage/placeholder.rs index fcef1c52a..421286e26 100644 --- a/objects/src/accounts/component/template/storage/placeholder.rs +++ b/objects/src/accounts/component/template/storage/placeholder.rs @@ -28,6 +28,16 @@ pub struct StoragePlaceholder { key: String, } +/// An identifier for the expected type for a storage placeholder. +/// These indicate which variant of [StorageValue] should be provided when instantiating a +/// component. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub enum PlaceholderType { + Felt, + Map, + Word, +} + impl StoragePlaceholder { /// Creates a new [StoragePlaceholder] from the provided string. /// diff --git a/objects/src/accounts/component/template/storage/toml.rs b/objects/src/accounts/component/template/storage/toml.rs index a7bebaa39..4852b1b85 100644 --- a/objects/src/accounts/component/template/storage/toml.rs +++ b/objects/src/accounts/component/template/storage/toml.rs @@ -8,10 +8,11 @@ use serde::{de::Error, Deserialize, Deserializer, Serialize, Serializer}; use vm_core::Felt; use vm_processor::Digest; -use super::{FeltRepresentation, StorageEntry, StoragePlaceholder, WordRepresentation}; +use super::{ + FeltRepresentation, MapRepresentation, StorageEntry, StoragePlaceholder, WordRepresentation, +}; use crate::{ - accounts::{component::template::MapEntry, AccountComponentMetadata}, - errors::AccountComponentTemplateError, + accounts::AccountComponentMetadata, errors::AccountComponentTemplateError, utils::parse_hex_string_as_word, }; @@ -40,6 +41,7 @@ impl AccountComponentMetadata { Ok(toml) } } + // WORD REPRESENTATION SERIALIZATION // ================================================================================================ @@ -207,9 +209,7 @@ enum StorageValues { /// List of individual words (for multi-slot entries). Words(Vec), /// List of key-value entries (for map storage slots). - MapEntries(Vec), - /// A placeholder value, represented as "{{key}}". - Template(StoragePlaceholder), + MapEntries(MapRepresentation), } impl StorageValues { @@ -217,7 +217,6 @@ impl StorageValues { match self { StorageValues::Words(_) => true, StorageValues::MapEntries(_) => false, - StorageValues::Template(_) => false, } } @@ -225,23 +224,20 @@ impl StorageValues { match self { StorageValues::Words(vec) => Some(vec), StorageValues::MapEntries(_) => None, - StorageValues::Template(_) => None, } } - pub fn into_map_entries(self) -> Option> { + pub fn into_map_entries(self) -> Option { match self { StorageValues::Words(_) => None, - StorageValues::MapEntries(vec) => Some(vec), - StorageValues::Template(_) => None, + StorageValues::MapEntries(map) => Some(map), } } pub fn len(&self) -> Option { match self { StorageValues::Words(vec) => Some(vec.len()), - StorageValues::MapEntries(vec) => Some(vec.len()), - StorageValues::Template(_) => None, + StorageValues::MapEntries(map) => map.len(), } } } diff --git a/objects/src/accounts/component/template/storage/word.rs b/objects/src/accounts/component/template/storage/value_type.rs similarity index 72% rename from objects/src/accounts/component/template/storage/word.rs rename to objects/src/accounts/component/template/storage/value_type.rs index 5e4912d76..5997ac3b2 100644 --- a/objects/src/accounts/component/template/storage/word.rs +++ b/objects/src/accounts/component/template/storage/value_type.rs @@ -1,13 +1,13 @@ -use alloc::boxed::Box; +use alloc::{boxed::Box, vec::Vec}; use vm_core::{ utils::{ByteReader, ByteWriter, Deserializable, Serializable}, Felt, FieldElement, Word, }; -use vm_processor::DeserializationError; +use vm_processor::{DeserializationError, Digest}; -use super::{InitStorageData, StoragePlaceholder}; -use crate::accounts::component::template::AccountComponentTemplateError; +use super::{placeholder::PlaceholderType, InitStorageData, MapEntry, StoragePlaceholder}; +use crate::accounts::{component::template::AccountComponentTemplateError, StorageMap}; // WORDS // ================================================================================================ @@ -24,14 +24,17 @@ pub enum WordRepresentation { } impl WordRepresentation { - /// Returns an iterator over all storage placeholder references within the [WordRepresentation]. - pub fn placeholders(&self) -> Box + '_> { + /// Returns an iterator over all storage placeholder references within the [WordRepresentation] + /// along with their expected types + pub fn placeholders( + &self, + ) -> Box + '_> { match self { WordRepresentation::Array(array) => { - Box::new(array.iter().flat_map(|felt| felt.storage_placeholders())) + Box::new(array.iter().flat_map(|felt| felt.placeholders())) }, WordRepresentation::Template(storage_placeholder) => { - Box::new(core::iter::once(storage_placeholder)) + Box::new(core::iter::once((storage_placeholder, PlaceholderType::Word))) }, WordRepresentation::Hexadecimal(_) => Box::new(core::iter::empty()), } @@ -144,9 +147,12 @@ pub enum FeltRepresentation { } impl FeltRepresentation { - pub fn storage_placeholders(&self) -> impl Iterator { + /// Returns the storage placeholders within this representation, alongside their expected type. + pub fn placeholders(&self) -> impl Iterator { let maybe_key = match self { - FeltRepresentation::Template(storage_placeholder) => Some(storage_placeholder), + FeltRepresentation::Template(storage_placeholder) => { + Some((storage_placeholder, PlaceholderType::Felt)) + }, _ => None, }; @@ -228,6 +234,103 @@ impl Deserializable for FeltRepresentation { } } +// MAP REPRESENTATION +// ================================================================================================ + +/// Supported map representations for a component's storage entries. +#[derive(Debug, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "std", derive(::serde::Deserialize, ::serde::Serialize))] +#[cfg_attr(feature = "std", serde(untagged))] +pub enum MapRepresentation { + List(Vec), + Template(StoragePlaceholder), +} + +impl MapRepresentation { + /// Returns an iterator over all of the storage entries's placeholder keys, alongside their + /// expected type. + pub fn placeholders( + &self, + ) -> Box + '_> { + match self { + MapRepresentation::Template(storage_placeholder) => { + Box::new(core::iter::once((storage_placeholder, PlaceholderType::Map))) + }, + MapRepresentation::List(entries) => { + Box::new(entries.iter().flat_map(|entry| entry.placeholders())) + }, + } + } + + /// Returns the amount of key-value pairs in the entry, or `None` if the representation is a + /// placeholder. + pub fn len(&self) -> Option { + match self { + MapRepresentation::List(vec) => Some(vec.len()), + MapRepresentation::Template(_) => None, + } + } + + /// Attempts to convert the [MapRepresentation] into a [StorageMap]. + /// + /// If the representation is a template, the value is retrieved from + /// `init_storage_data`, identified by its key. If any of the inner elements + /// within the value are a template, they are retrieved in the same way. + pub fn try_build_map( + &self, + init_storage_data: &InitStorageData, + ) -> Result { + let entries = match self { + MapRepresentation::List(vec) => vec + .iter() + .map(|map_entry| { + let key = map_entry.key().try_build_word(init_storage_data)?; + let value = map_entry.value().try_build_word(init_storage_data)?; + Ok((key.into(), value)) + }) + .collect::, _>>()?, + MapRepresentation::Template(storage_placeholder) => init_storage_data + .get(storage_placeholder) + .ok_or_else(|| { + AccountComponentTemplateError::PlaceholderValueNotProvided( + storage_placeholder.clone(), + ) + })? + .as_map() + .cloned()?, + }; + Ok(StorageMap::with_entries(entries)) + } +} + +impl Serializable for MapRepresentation { + fn write_into(&self, target: &mut W) { + match self { + MapRepresentation::List(entries) => { + target.write_u8(0); + entries.write_into(target); + }, + MapRepresentation::Template(storage_placeholder) => { + target.write_u8(1); + storage_placeholder.write_into(target); + }, + } + } +} + +impl Deserializable for MapRepresentation { + fn read_from(source: &mut R) -> Result { + match source.read_u8()? { + 0 => Ok(MapRepresentation::List(Vec::::read_from(source)?)), + 1 => Ok(MapRepresentation::Template(StoragePlaceholder::read_from(source)?)), + other => Err(DeserializationError::InvalidValue(format!( + "Unknown variant tag for MapRepresentation: {}", + other + ))), + } + } +} + // TESTS // ================================================================================================ diff --git a/objects/src/errors.rs b/objects/src/errors.rs index ff823d4a8..11f078260 100644 --- a/objects/src/errors.rs +++ b/objects/src/errors.rs @@ -35,6 +35,10 @@ pub enum AccountComponentTemplateError { DuplicateSlot(u8), #[error("component storage slots have to start at 0")] StorageSlotsMustStartAtZero, + #[error( + "storage placeholder `{0}` appears more than once, representing different types of values" + )] + StoragePlaceholderDuplicate(StoragePlaceholder), #[error("storage value was not of the expected type {0}")] IncorrectStorageValue(String), #[error("multi-slot entry should contain as many values as storage slots indices")] From a608ea69a0011ab29764e944f98d56b602a9a69b Mon Sep 17 00:00:00 2001 From: Ignacio Amigo Date: Thu, 16 Jan 2025 19:17:43 -0300 Subject: [PATCH 2/9] fix: Expose placeholder type --- objects/src/accounts/component/mod.rs | 2 +- .../accounts/component/template/storage/placeholder.rs | 10 ++++++++++ objects/src/accounts/mod.rs | 3 ++- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/objects/src/accounts/component/mod.rs b/objects/src/accounts/component/mod.rs index 199fc8383..730da56bf 100644 --- a/objects/src/accounts/component/mod.rs +++ b/objects/src/accounts/component/mod.rs @@ -6,7 +6,7 @@ use vm_processor::MastForest; mod template; pub use template::{ AccountComponentMetadata, AccountComponentTemplate, FeltRepresentation, InitStorageData, - StorageEntry, StoragePlaceholder, StorageValue, WordRepresentation, + PlaceholderType, StorageEntry, StoragePlaceholder, StorageValue, WordRepresentation, }; use crate::{ diff --git a/objects/src/accounts/component/template/storage/placeholder.rs b/objects/src/accounts/component/template/storage/placeholder.rs index 421286e26..ae42c9878 100644 --- a/objects/src/accounts/component/template/storage/placeholder.rs +++ b/objects/src/accounts/component/template/storage/placeholder.rs @@ -38,6 +38,16 @@ pub enum PlaceholderType { Word, } +impl core::fmt::Display for PlaceholderType { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + PlaceholderType::Felt => f.write_str("Felt"), + PlaceholderType::Map => f.write_str("Map"), + PlaceholderType::Word => f.write_str("Word"), + } + } +} + impl StoragePlaceholder { /// Creates a new [StoragePlaceholder] from the provided string. /// diff --git a/objects/src/accounts/mod.rs b/objects/src/accounts/mod.rs index 0c1101e21..486120905 100644 --- a/objects/src/accounts/mod.rs +++ b/objects/src/accounts/mod.rs @@ -23,7 +23,8 @@ pub use code::{procedure::AccountProcedureInfo, AccountCode}; mod component; pub use component::{ AccountComponent, AccountComponentMetadata, AccountComponentTemplate, FeltRepresentation, - InitStorageData, StorageEntry, StoragePlaceholder, StorageValue, WordRepresentation, + InitStorageData, PlaceholderType, StorageEntry, StoragePlaceholder, StorageValue, + WordRepresentation, }; pub mod delta; From 7bd48d529fdbeb473f2c7e34aeccdd2b9bcba645 Mon Sep 17 00:00:00 2001 From: Ignacio Amigo Date: Thu, 16 Jan 2025 19:36:22 -0300 Subject: [PATCH 3/9] refactor: Expose MapRepresentation --- objects/src/accounts/component/mod.rs | 3 ++- objects/src/accounts/mod.rs | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/objects/src/accounts/component/mod.rs b/objects/src/accounts/component/mod.rs index 730da56bf..bed5f837c 100644 --- a/objects/src/accounts/component/mod.rs +++ b/objects/src/accounts/component/mod.rs @@ -6,7 +6,8 @@ use vm_processor::MastForest; mod template; pub use template::{ AccountComponentMetadata, AccountComponentTemplate, FeltRepresentation, InitStorageData, - PlaceholderType, StorageEntry, StoragePlaceholder, StorageValue, WordRepresentation, + MapRepresentation, PlaceholderType, StorageEntry, StoragePlaceholder, StorageValue, + WordRepresentation, }; use crate::{ diff --git a/objects/src/accounts/mod.rs b/objects/src/accounts/mod.rs index 486120905..6cdaeacfc 100644 --- a/objects/src/accounts/mod.rs +++ b/objects/src/accounts/mod.rs @@ -23,8 +23,8 @@ pub use code::{procedure::AccountProcedureInfo, AccountCode}; mod component; pub use component::{ AccountComponent, AccountComponentMetadata, AccountComponentTemplate, FeltRepresentation, - InitStorageData, PlaceholderType, StorageEntry, StoragePlaceholder, StorageValue, - WordRepresentation, + InitStorageData, MapRepresentation, PlaceholderType, StorageEntry, StoragePlaceholder, + StorageValue, WordRepresentation, }; pub mod delta; From 0a30796ed75e6a4f8943f9a14ed67c31655d7a5c Mon Sep 17 00:00:00 2001 From: Ignacio Amigo Date: Fri, 17 Jan 2025 13:49:46 -0300 Subject: [PATCH 4/9] reviews: Doc updates, validation for map --- .../src/accounts/component/template/mod.rs | 153 ++++++++++++++++-- .../component/template/storage/mod.rs | 49 +++--- .../component/template/storage/toml.rs | 13 +- .../component/template/storage/value_type.rs | 81 ++++++++-- objects/src/errors.rs | 19 ++- 5 files changed, 242 insertions(+), 73 deletions(-) diff --git a/objects/src/accounts/component/template/mod.rs b/objects/src/accounts/component/template/mod.rs index 4b70dd0a8..0f1c891fc 100644 --- a/objects/src/accounts/component/template/mod.rs +++ b/objects/src/accounts/component/template/mod.rs @@ -92,6 +92,14 @@ impl Deserializable for AccountComponentTemplate { /// When the `std` feature is enabled, this struct allows for serialization and deserialization to /// and from a TOML file. /// +/// # Guarantees +/// +/// - The metadata's storage layout does not contain duplicate slots, and it always starts at slot +/// index 0. +/// - Storage slots are laid out in a contiguous manner. +/// - Storage placeholders can appear multiple times, but only if the expected [StorageValue] is of +/// the same type in all instances. +/// /// # Example /// /// ``` @@ -122,8 +130,8 @@ impl Deserializable for AccountComponentTemplate { /// )]); /// /// let component_template = AccountComponentMetadata::new( -/// "test".into(), -/// "desc".into(), +/// "test name".into(), +/// "description of the component".into(), /// Version::parse("0.1.0")?, /// BTreeSet::new(), /// vec![], @@ -183,15 +191,19 @@ impl AccountComponentMetadata { Ok(component) } - /// Retrieves the set of storage placeholder keys (identified by a string) that - /// require a value at the moment of component instantiation. These values will - /// be used for initializing storage slot values, or storage map entries. - /// For a full example on how a placeholder may be utilized, refer to the docs - /// for [AccountComponentMetadata]. - pub fn get_storage_placeholders(&self) -> BTreeMap { + /// Retrieves a map of unique storage placeholders mapped to their expected type that require + /// a value at the moment of component instantiation. These values will be used for + /// initializing storage slot values, or storage map entries.For a full example on how a + /// placeholder may be utilized, please refer to the docs for [AccountComponentMetadata]. + /// + /// Types for the returned storage placeholders are inferred based on their location in the + /// storage layout structure. + pub fn get_unique_storage_placeholders(&self) -> BTreeMap { let mut placeholder_map = BTreeMap::new(); for storage_entry in &self.storage { - for (placeholder, placeholder_type) in storage_entry.placeholders() { + for (placeholder, placeholder_type) in storage_entry.all_placeholders_iter() { + // The constructors of this type guarantee each placeholder has the same type, so + // reinserting them multiple times is fine. placeholder_map.insert(placeholder.clone(), placeholder_type); } } @@ -228,6 +240,7 @@ impl AccountComponentMetadata { /// # Errors /// /// - If the specified storage slots contain duplicates. + /// - If the template contains multiple storage placeholders with of different type. /// - If the slot numbers do not start at zero. /// - If the slots are not contiguous. fn validate(&self) -> Result<(), AccountComponentTemplateError> { @@ -241,7 +254,9 @@ impl AccountComponentMetadata { all_slots.sort_unstable(); if let Some(&first_slot) = all_slots.first() { if first_slot != 0 { - return Err(AccountComponentTemplateError::StorageSlotsMustStartAtZero); + return Err(AccountComponentTemplateError::StorageSlotsDoNotStartAtZero( + first_slot, + )); } } @@ -258,14 +273,16 @@ impl AccountComponentMetadata { // Check that placeholders do not appear more than once with a different type let mut placeholders = BTreeMap::new(); for storage_entry in &self.storage { - for (placeholder, placeholder_type) in storage_entry.placeholders() { + for (placeholder, placeholder_type) in storage_entry.all_placeholders_iter() { match placeholders.entry(placeholder.clone()) { Entry::Occupied(entry) => { // if already exists, make sure it's the same type if *entry.get() != placeholder_type { return Err( - AccountComponentTemplateError::StoragePlaceholderDuplicate( + AccountComponentTemplateError::StoragePlaceholderTypeMismatch( placeholder.clone(), + *entry.get(), + placeholder_type, ), ); } @@ -277,6 +294,38 @@ impl AccountComponentMetadata { } } + self.validate_map_keys()?; + + Ok(()) + } + + /// Validates map keys. + /// Because keys can be represented in a variety of ways, the `to_string()` implementation is + /// used to check for duplicates. + fn validate_map_keys(&self) -> Result<(), AccountComponentTemplateError> { + for storage_entry in self.storage_entries() { + match storage_entry { + StorageEntry::Map { map, .. } => { + match map { + MapRepresentation::List(map_entries) => { + let mut seen_keys = BTreeSet::new(); + for entry in map_entries { + if !seen_keys.insert(entry.key().to_string()) { + return Err( + AccountComponentTemplateError::StorageMapHasDuplicateKeys( + entry.key().to_string(), + ), + ); + } + } + }, + // Can't validate placeholders since we don't know the keys and values yet + MapRepresentation::Template(_) => continue, + } + }, + _ => continue, + } + } Ok(()) } } @@ -316,9 +365,10 @@ mod tests { use assembly::Assembler; use assert_matches::assert_matches; use storage::WordRepresentation; + use vm_core::{Felt, FieldElement}; use super::*; - use crate::{accounts::AccountComponent, testing::account_code::CODE}; + use crate::{accounts::AccountComponent, testing::account_code::CODE, AccountError}; #[test] fn test_contiguous_value_slots() { @@ -327,7 +377,7 @@ mod tests { name: "slot0".into(), description: None, slot: 0, - value: WordRepresentation::Hexadecimal(Default::default()), + value: WordRepresentation::Value(Default::default()), }, StorageEntry::MultiSlot { name: "slot1".into(), @@ -335,7 +385,7 @@ mod tests { slots: vec![1, 2], values: vec![ WordRepresentation::Array(Default::default()), - WordRepresentation::Hexadecimal(Default::default()), + WordRepresentation::Value(Default::default()), ], }, ]; @@ -391,14 +441,14 @@ mod tests { slots: vec![1, 2], values: vec![ WordRepresentation::Array(Default::default()), - WordRepresentation::Hexadecimal(Default::default()), + WordRepresentation::Value(Default::default()), ], }, StorageEntry::Value { name: "slot0".into(), description: None, slot: 0, - value: WordRepresentation::Hexadecimal(Default::default()), + value: WordRepresentation::Value(Default::default()), }, ]; @@ -420,4 +470,73 @@ mod tests { assert_eq!(deserialized, template) } + + #[test] + pub fn fail_duplicate_key() { + let toml_text = r#" + name = "Test Component" + description = "This is a test component" + version = "1.0.1" + targets = ["FungibleFaucet"] + + [[storage]] + name = "map" + description = "A storage map entry" + slot = 0 + values = [ + { key = "0x1", value = ["{{value.test}}", "0x1", "0x2", "0x3"] }, + { key = "0x1", value = ["0x1", "0x2", "0x3", "{{value.test}}"] }, + ] + "#; + + let result = AccountComponentMetadata::from_toml(toml_text); + assert_matches!(result, Err(AccountComponentTemplateError::StorageMapHasDuplicateKeys(_))); + } + + #[test] + pub fn fail_duplicate_key_instance() { + let toml_text = r#" + name = "Test Component" + description = "This is a test component" + version = "1.0.1" + targets = ["FungibleFaucet"] + + [[storage]] + name = "map" + description = "A storage map entry" + slot = 0 + values = [ + { key = ["0","0","0","1"], value = ["{{value.test}}", "0x1", "0x2", "0x3"] }, + { key = "{{word.test}}", value = ["0x1", "0x2", "0x3", "{{value.test}}"] }, + ] + "#; + + let metadata = AccountComponentMetadata::from_toml(toml_text).unwrap(); + let library = Assembler::default().assemble_library([CODE]).unwrap(); + let template = AccountComponentTemplate::new(metadata, library); + + let init_storage_data = InitStorageData::new([ + ( + StoragePlaceholder::new("word.test").unwrap(), + StorageValue::Word([Felt::ZERO, Felt::ZERO, Felt::ZERO, Felt::ONE]), + ), + (StoragePlaceholder::new("value.test").unwrap(), StorageValue::Felt(Felt::ONE)), + ]); + let account_component = AccountComponent::from_template(&template, &init_storage_data); + assert_matches!( + account_component, + Err(AccountError::AccountComponentTemplateInstantiationError( + AccountComponentTemplateError::StorageMapHasDuplicateKeys(_) + )) + ); + + let valid_init_storage_data = InitStorageData::new([ + ( + StoragePlaceholder::new("word.test").unwrap(), + StorageValue::Word([Felt::new(30), Felt::new(20), Felt::new(10), Felt::ZERO]), + ), + (StoragePlaceholder::new("value.test").unwrap(), StorageValue::Felt(Felt::ONE)), + ]); + AccountComponent::from_template(&template, &valid_init_storage_data).unwrap(); + } } diff --git a/objects/src/accounts/component/template/storage/mod.rs b/objects/src/accounts/component/template/storage/mod.rs index 3a5b70fe1..c45377330 100644 --- a/objects/src/accounts/component/template/storage/mod.rs +++ b/objects/src/accounts/component/template/storage/mod.rs @@ -51,7 +51,7 @@ pub enum StorageEntry { /// The numeric index of this map slot in the component's storage. slot: u8, /// A list of key-value pairs to initialize in this map slot. - map_entries: MapRepresentation, + map: MapRepresentation, }, /// A multi-slot entry, representing a single logical value across multiple slots. @@ -94,7 +94,7 @@ impl StorageEntry { name: name.into(), description: description.map(Into::::into), slot, - map_entries: map_representation, + map: map_representation, } } @@ -166,14 +166,14 @@ impl StorageEntry { /// Returns an iterator over all of the storage entries's placeholder keys, alongside their /// expected type. - pub fn placeholders( + pub fn all_placeholders_iter( &self, ) -> Box + '_> { match self { - StorageEntry::Value { value, .. } => value.placeholders(), - StorageEntry::Map { map_entries, .. } => map_entries.placeholders(), + StorageEntry::Value { value, .. } => value.all_placeholders_iter(), + StorageEntry::Map { map: map_entries, .. } => map_entries.all_placeholders_iter(), StorageEntry::MultiSlot { values, .. } => { - Box::new(values.iter().flat_map(|word| word.placeholders())) + Box::new(values.iter().flat_map(|word| word.all_placeholders_iter())) }, } } @@ -195,7 +195,7 @@ impl StorageEntry { let slot = value.try_build_word(init_storage_data)?; Ok(vec![StorageSlot::Value(slot)]) }, - StorageEntry::Map { map_entries: values, .. } => { + StorageEntry::Map { map: values, .. } => { let storage_map = values.try_build_map(init_storage_data)?; Ok(vec![StorageSlot::Map(storage_map)]) }, @@ -222,12 +222,7 @@ impl Serializable for StorageEntry { target.write_u8(*slot); target.write(value); }, - StorageEntry::Map { - name, - description, - slot, - map_entries: values, - } => { + StorageEntry::Map { name, description, slot, map: values } => { target.write_u8(1u8); target.write(name); target.write(description); @@ -269,7 +264,7 @@ impl Deserializable for StorageEntry { name, description, slot, - map_entries: map_representation, + map: map_representation, }) }, @@ -315,8 +310,10 @@ impl MapEntry { /// Returns an iterator over all of the storage entries's placeholder keys, alongside their /// expected type. - pub fn placeholders(&self) -> impl Iterator { - self.key.placeholders().chain(self.value.placeholders()) + pub fn all_placeholders_iter( + &self, + ) -> impl Iterator { + self.key.all_placeholders_iter().chain(self.value.all_placeholders_iter()) } pub fn into_parts(self) -> (WordRepresentation, WordRepresentation) { @@ -378,28 +375,28 @@ mod tests { name: "slot0".into(), description: Some("First slot".into()), slot: 0, - value: WordRepresentation::Hexadecimal(digest!("0x333123").into()), + value: WordRepresentation::Value(digest!("0x333123").into()), }, StorageEntry::Map { name: "map".into(), description: Some("A storage map entry".into()), slot: 1, - map_entries: MapRepresentation::List(vec![ + map: MapRepresentation::List(vec![ MapEntry { key: WordRepresentation::Template( StoragePlaceholder::new("foo.bar").unwrap(), ), - value: WordRepresentation::Hexadecimal(digest!("0x2").into()), + value: WordRepresentation::Value(digest!("0x2").into()), }, MapEntry { - key: WordRepresentation::Hexadecimal(digest!("0x2").into()), + key: WordRepresentation::Value(digest!("0x2").into()), value: WordRepresentation::Template( StoragePlaceholder::new("bar.baz").unwrap(), ), }, MapEntry { - key: WordRepresentation::Hexadecimal(digest!("0x3").into()), - value: WordRepresentation::Hexadecimal(digest!("0x4").into()), + key: WordRepresentation::Value(digest!("0x3").into()), + value: WordRepresentation::Value(digest!("0x4").into()), }, ]), }, @@ -410,7 +407,7 @@ mod tests { values: vec![ WordRepresentation::Template(StoragePlaceholder::new("test.Template").unwrap()), WordRepresentation::Array(array), - WordRepresentation::Hexadecimal(digest!("0xabcdef123abcdef123").into()), + WordRepresentation::Value(digest!("0xabcdef123abcdef123").into()), ], }, StorageEntry::Value { @@ -479,7 +476,7 @@ mod tests { "#; let component_metadata = AccountComponentMetadata::from_toml(toml_text).unwrap(); - for (key, placeholder_type) in component_metadata.get_storage_placeholders() { + for (key, placeholder_type) in component_metadata.get_unique_storage_placeholders() { match key.inner() { "map.key.test" | "word.test" => assert_eq!(placeholder_type, PlaceholderType::Word), "value.test" => assert_eq!(placeholder_type, PlaceholderType::Felt), @@ -543,7 +540,7 @@ mod tests { } #[test] - pub fn fail_duplicate_key() { + pub fn fail_placeholder_type_mismatch() { let toml_text = r#" name = "Test Component" description = "This is a test component" @@ -566,7 +563,7 @@ mod tests { let component_metadata = AccountComponentMetadata::from_toml(toml_text); assert_matches!( component_metadata, - Err(AccountComponentTemplateError::StoragePlaceholderDuplicate(_)) + Err(AccountComponentTemplateError::StoragePlaceholderTypeMismatch(_, _, _)) ); } } diff --git a/objects/src/accounts/component/template/storage/toml.rs b/objects/src/accounts/component/template/storage/toml.rs index 4852b1b85..73851ca32 100644 --- a/objects/src/accounts/component/template/storage/toml.rs +++ b/objects/src/accounts/component/template/storage/toml.rs @@ -52,7 +52,7 @@ impl serde::Serialize for WordRepresentation { { use serde::ser::SerializeSeq; match self { - WordRepresentation::Hexadecimal(word) => { + WordRepresentation::Value(word) => { // Ensure that the length of the vector is exactly 4 let word = Digest::from(word); serializer.serialize_str(&word.to_string()) @@ -101,7 +101,7 @@ impl<'de> serde::Deserialize<'de> for WordRepresentation { ) })?; - Ok(WordRepresentation::Hexadecimal(word)) + Ok(WordRepresentation::Value(word)) } fn visit_seq(self, mut seq: A) -> Result @@ -266,12 +266,7 @@ impl From for RawStorageEntry { value: Some(value), ..Default::default() }, - StorageEntry::Map { - name, - description, - slot, - map_entries: values, - } => RawStorageEntry { + StorageEntry::Map { name, description, slot, map: values } => RawStorageEntry { name, description, slot: Some(slot), @@ -349,7 +344,7 @@ impl<'de> Deserialize<'de> for StorageEntry { name: raw.name, description: raw.description, slot: raw.slot.ok_or(D::Error::missing_field("slot"))?, - map_entries, + map: map_entries, }) }, (Some(slots), Some(values)) => { diff --git a/objects/src/accounts/component/template/storage/value_type.rs b/objects/src/accounts/component/template/storage/value_type.rs index 5997ac3b2..e9f4c9fc5 100644 --- a/objects/src/accounts/component/template/storage/value_type.rs +++ b/objects/src/accounts/component/template/storage/value_type.rs @@ -1,4 +1,4 @@ -use alloc::{boxed::Box, vec::Vec}; +use alloc::{boxed::Box, collections::BTreeSet, string::ToString, vec::Vec}; use vm_core::{ utils::{ByteReader, ByteWriter, Deserializable, Serializable}, @@ -16,7 +16,7 @@ use crate::accounts::{component::template::AccountComponentTemplateError, Storag #[derive(Debug, Clone, PartialEq, Eq)] pub enum WordRepresentation { /// A word represented by a hexadecimal string. - Hexadecimal([Felt; 4]), + Value([Felt; 4]), /// A word represented by its four base elements. Array([FeltRepresentation; 4]), /// A placeholder value, represented as "{{key}}". @@ -26,17 +26,17 @@ pub enum WordRepresentation { impl WordRepresentation { /// Returns an iterator over all storage placeholder references within the [WordRepresentation] /// along with their expected types - pub fn placeholders( + pub fn all_placeholders_iter( &self, ) -> Box + '_> { match self { WordRepresentation::Array(array) => { - Box::new(array.iter().flat_map(|felt| felt.placeholders())) + Box::new(array.iter().flat_map(|felt| felt.all_placeholders_iter())) }, WordRepresentation::Template(storage_placeholder) => { Box::new(core::iter::once((storage_placeholder, PlaceholderType::Word))) }, - WordRepresentation::Hexadecimal(_) => Box::new(core::iter::empty()), + WordRepresentation::Value(_) => Box::new(core::iter::empty()), } } @@ -50,7 +50,7 @@ impl WordRepresentation { init_storage_data: &InitStorageData, ) -> Result { match self { - WordRepresentation::Hexadecimal(word) => Ok(*word), + WordRepresentation::Value(word) => Ok(*word), WordRepresentation::Array(array) => { let mut result = [Felt::ZERO; 4]; for (index, felt_repr) in array.iter().enumerate() { @@ -76,14 +76,14 @@ impl WordRepresentation { impl From for WordRepresentation { fn from(value: Word) -> Self { - WordRepresentation::Hexadecimal(value) + WordRepresentation::Value(value) } } impl Serializable for WordRepresentation { fn write_into(&self, target: &mut W) { match self { - WordRepresentation::Hexadecimal(value) => { + WordRepresentation::Value(value) => { target.write_u8(0); target.write(value); }, @@ -107,7 +107,7 @@ impl Deserializable for WordRepresentation { 0 => { // Hexadecimal let value = <[Felt; 4]>::read_from(source)?; - Ok(WordRepresentation::Hexadecimal(value)) + Ok(WordRepresentation::Value(value)) }, 1 => { // Array @@ -128,7 +128,27 @@ impl Deserializable for WordRepresentation { impl Default for WordRepresentation { fn default() -> Self { - WordRepresentation::Hexadecimal(Default::default()) + WordRepresentation::Value(Default::default()) + } +} + +impl core::fmt::Display for WordRepresentation { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + WordRepresentation::Value(hex) => f.write_str(&Digest::from(hex).to_hex()), + WordRepresentation::Array(array) => { + f.write_str("[")?; + f.write_str(&format!("{}, ", array[0]))?; + f.write_str(&format!("{}, ", array[1]))?; + f.write_str(&format!("{}, ", array[2]))?; + f.write_str(&format!("{}, ", array[3]))?; + + f.write_str("]") + }, + WordRepresentation::Template(storage_placeholder) => { + f.write_str(&storage_placeholder.to_string()) + }, + } } } @@ -148,7 +168,9 @@ pub enum FeltRepresentation { impl FeltRepresentation { /// Returns the storage placeholders within this representation, alongside their expected type. - pub fn placeholders(&self) -> impl Iterator { + pub fn all_placeholders_iter( + &self, + ) -> impl Iterator { let maybe_key = match self { FeltRepresentation::Template(storage_placeholder) => { Some((storage_placeholder, PlaceholderType::Felt)) @@ -234,6 +256,18 @@ impl Deserializable for FeltRepresentation { } } +impl core::fmt::Display for FeltRepresentation { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + FeltRepresentation::Hexadecimal(base_element) => f.write_str(&base_element.to_string()), + FeltRepresentation::Decimal(base_element) => f.write_str(&base_element.to_string()), + FeltRepresentation::Template(storage_placeholder) => { + f.write_str(&storage_placeholder.to_string()) + }, + } + } +} + // MAP REPRESENTATION // ================================================================================================ @@ -249,7 +283,7 @@ pub enum MapRepresentation { impl MapRepresentation { /// Returns an iterator over all of the storage entries's placeholder keys, alongside their /// expected type. - pub fn placeholders( + pub fn all_placeholders_iter( &self, ) -> Box + '_> { match self { @@ -257,7 +291,7 @@ impl MapRepresentation { Box::new(core::iter::once((storage_placeholder, PlaceholderType::Map))) }, MapRepresentation::List(entries) => { - Box::new(entries.iter().flat_map(|entry| entry.placeholders())) + Box::new(entries.iter().flat_map(|entry| entry.all_placeholders_iter())) }, } } @@ -271,6 +305,15 @@ impl MapRepresentation { } } + /// Returns `true` if the map is represented by a list of key-value entries, and the list is + /// empty. Returns false otherwise + pub fn is_empty(&self) -> bool { + match self { + MapRepresentation::List(vec) => vec.is_empty(), + MapRepresentation::Template(_) => false, + } + } + /// Attempts to convert the [MapRepresentation] into a [StorageMap]. /// /// If the representation is a template, the value is retrieved from @@ -299,6 +342,16 @@ impl MapRepresentation { .as_map() .cloned()?, }; + + // validate that no key appears multiple times + let mut seen_keys = BTreeSet::new(); + for (map_key, _map_value) in entries.iter() { + if !seen_keys.insert(map_key) { + return Err(AccountComponentTemplateError::StorageMapHasDuplicateKeys( + map_key.to_hex(), + )); + } + } Ok(StorageMap::with_entries(entries)) } } @@ -418,7 +471,7 @@ mod tests { #[test] fn test_word_representation_serde() { let word = Word::default(); - let original = WordRepresentation::Hexadecimal(word); + let original = WordRepresentation::Value(word); let serialized = original.to_bytes(); let deserialized = WordRepresentation::read_from_bytes(&serialized).unwrap(); assert_eq!(original, deserialized); diff --git a/objects/src/errors.rs b/objects/src/errors.rs index 11f078260..ed7a0967a 100644 --- a/objects/src/errors.rs +++ b/objects/src/errors.rs @@ -16,7 +16,10 @@ use super::{ MAX_OUTPUT_NOTES_PER_BATCH, MAX_OUTPUT_NOTES_PER_BLOCK, }; use crate::{ - accounts::{AccountCode, AccountIdPrefix, AccountStorage, AccountType, StoragePlaceholder}, + accounts::{ + AccountCode, AccountIdPrefix, AccountStorage, AccountType, PlaceholderType, + StoragePlaceholder, + }, block::block_num_from_epoch, notes::{NoteAssets, NoteExecutionHint, NoteTag, NoteType, Nullifier}, BlockHeader, ACCOUNT_UPDATE_MAX_SIZE, MAX_INPUTS_PER_NOTE, MAX_INPUT_NOTES_PER_TX, @@ -33,12 +36,6 @@ pub enum AccountComponentTemplateError { DeserializationError(#[source] toml::de::Error), #[error("slot {0} is defined multiple times")] DuplicateSlot(u8), - #[error("component storage slots have to start at 0")] - StorageSlotsMustStartAtZero, - #[error( - "storage placeholder `{0}` appears more than once, representing different types of values" - )] - StoragePlaceholderDuplicate(StoragePlaceholder), #[error("storage value was not of the expected type {0}")] IncorrectStorageValue(String), #[error("multi-slot entry should contain as many values as storage slots indices")] @@ -49,6 +46,14 @@ pub enum AccountComponentTemplateError { NonContiguousSlots(u8, u8), #[error("storage value for placeholder `{0}` was not provided in the map")] PlaceholderValueNotProvided(StoragePlaceholder), + #[error("storage map contains duplicate key `{0}`")] + StorageMapHasDuplicateKeys(String), + #[error("component storage slots have to start at 0, but they start at {0}")] + StorageSlotsDoNotStartAtZero(u8), + #[error( + "storage placeholder `{0}` appears more than once representing different types `{0}` and `{1}`" + )] + StoragePlaceholderTypeMismatch(StoragePlaceholder, PlaceholderType, PlaceholderType), } // ACCOUNT ERROR From 5b24ba589969d568194f4e6fe8047cf12253fa5e Mon Sep 17 00:00:00 2001 From: Ignacio Amigo Date: Fri, 17 Jan 2025 15:12:49 -0300 Subject: [PATCH 5/9] chore: Merge --- CHANGELOG.md | 3 +- miden-lib/asm/kernels/transaction/api.masm | 41 ++----- .../asm/kernels/transaction/lib/account.masm | 76 ++++++++---- .../kernels/transaction/lib/asset_vault.masm | 4 +- .../asm/kernels/transaction/lib/epilogue.masm | 6 +- .../asm/kernels/transaction/lib/prologue.masm | 3 +- miden-lib/asm/kernels/transaction/lib/tx.masm | 6 +- miden-lib/asm/kernels/transaction/main.masm | 10 -- miden-lib/asm/miden/note.masm | 4 +- .../src/transaction/procedures/kernel_v0.rs | 28 ++--- miden-tx/src/executor/data_store.rs | 6 +- miden-tx/src/executor/mod.rs | 3 +- miden-tx/src/host/mod.rs | 2 +- miden-tx/src/testing/mock_chain/mod.rs | 18 +-- miden-tx/src/testing/tx_context/mod.rs | 3 +- .../src/tests/kernel_tests/test_epilogue.rs | 3 +- .../src/tests/kernel_tests/test_prologue.rs | 2 +- objects/src/accounts/account_id/id_anchor.rs | 14 +-- objects/src/accounts/account_id/mod.rs | 4 +- objects/src/accounts/builder/mod.rs | 6 +- objects/src/block/block_number.rs | 114 ++++++++++++++++++ objects/src/block/header.rs | 38 ++---- objects/src/block/mod.rs | 4 +- objects/src/errors.rs | 31 +++-- objects/src/notes/file.rs | 4 +- objects/src/notes/location.rs | 16 ++- objects/src/testing/block.rs | 6 +- objects/src/transaction/chain_mmr.rs | 27 +++-- objects/src/transaction/inputs.rs | 8 +- 29 files changed, 301 insertions(+), 189 deletions(-) create mode 100644 objects/src/block/block_number.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 268069c3a..b2e4fb3c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,6 @@ ### Changes -- Added health check endpoints to the prover service (#1006). - Implemented serialization for `AccountHeader` (#996). - Updated Pingora crates to 0.4 and added polling time to the configuration file (#997). - Added support for `miden-tx-prover` proxy to update workers on a running proxy (#989). @@ -17,11 +16,13 @@ - Added conversion from `Account` to `AccountDelta` for initial account state representation as delta (#983). - [BREAKING] Added `miden::note::get_script_hash` procedure (#995). - [BREAKING] Refactor error messages in `miden-lib` and `miden-tx` and use `thiserror` 2.0 (#1005). +- Added health check endpoints to the prover service (#1006). - [BREAKING] Extend `AccountId` to two `Felt`s and require block hash in derivation (#982). - Removed workers list from the proxy configuration file (#1018). - Added tracing to the `miden-tx-prover` CLI (#1014). - Added metrics to the `miden-tx-prover` proxy (#1017). - Implemented `to_hex` for `AccountIdPrefix` and `epoch_block_num` for `BlockHeader` (#1039). +- Add `BlockNumber` struct (#1043). - Introduce `AccountIdBuilder` to simplify `AccountId` generation in tests (#1045). - Introduced `AccountComponentTemplate` with TOML serialization and templating (#1015, #1027). - [BREAKING] Updated the names and values of the kernel procedure offsets and corresponding kernel procedures (#1037). diff --git a/miden-lib/asm/kernels/transaction/api.masm b/miden-lib/asm/kernels/transaction/api.masm index db93d7460..249cca995 100644 --- a/miden-lib/asm/kernels/transaction/api.masm +++ b/miden-lib/asm/kernels/transaction/api.masm @@ -1,4 +1,3 @@ -use.std::collections::smt use.std::sys use.kernel::account @@ -27,9 +26,6 @@ const.ERR_FAUCET_STORAGE_DATA_SLOT_IS_RESERVED=0x00020000 # The get_fungible_faucet_total_issuance procedure can only be called on a fungible faucet const.ERR_ACCOUNT_TOTAL_ISSUANCE_PROC_CAN_ONLY_BE_CALLED_ON_FUNGIBLE_FAUCET=0x00020001 -# Failed to read an account map item from a non-map storage slot -const.ERR_ACCOUNT_READING_MAP_VALUE_FROM_NON_MAP_SLOT=0x00020002 - # Provided kernel procedure offset is out of bounds const.ERR_KERNEL_PROCEDURE_OFFSET_OUT_OF_BOUNDS=0x00020003 @@ -298,14 +294,15 @@ export.account_set_item # => [R', V, pad(8)] end -#! Returns VALUE located under specified KEY in map in specified account storage slot. +#! Returns the VALUE located under the specified KEY within the map contained in the given +#! account storage slot. #! #! Inputs: [index, KEY, pad(11)] #! Outputs: [VALUE, pad(12)] #! #! Where: -#! - index is the index of the item to get. -#! - VALUE is the value of the item. +#! - index is the index of the storage slot that contains the the map root. +#! - VALUE is the value of the map item at KEY. #! #! Panics if: #! - the index is out of bounds (>255). @@ -313,15 +310,6 @@ end #! #! Invocation: dynexec export.account_get_map_item - # check if storage type is map - dup exec.account::get_storage_slot_type - # => [slot_type, index, KEY, pad(11)] - - # check if type == map - exec.constants::get_storage_slot_type_map eq - assert.err=ERR_ACCOUNT_READING_MAP_VALUE_FROM_NON_MAP_SLOT - # => [index, KEY, pad(11)] - # authenticate that the procedure invocation originates from the account context exec.authenticate_account_origin # => [storage_offset, storage_size, index, KEY, pad(11)] @@ -330,31 +318,22 @@ export.account_get_map_item exec.account::apply_storage_offset # => [index_with_offset, KEY, pad(11)] - # fetch the account storage item, which is ROOT of the map - exec.account::get_item swapw - # => [KEY, ROOT, pad(11)] - - # fetch the VALUE located under KEY in the tree - exec.smt::get - # => [VALUE, ROOT, pad(11)] - - # remove the ROOT from the stack - swapw dropw + # fetch the map item from account storage + exec.account::get_map_item # => [VALUE, pad(12)] end -#! Inserts specified NEW_VALUE under specified KEY in map in specified account storage slot. +#! Stores NEW_VALUE under the specified KEY within the map contained in the given account storage slot. #! #! Inputs: [index, KEY, NEW_VALUE, pad(7)] #! Outputs: [OLD_MAP_ROOT, OLD_MAP_VALUE, pad(8)] #! #! Where: -#! - index is the index of the item to get. +#! - index is the index of the storage slot which contains the map root. #! - NEW_VALUE is the value of the new map item for the respective KEY. #! - OLD_VALUE is the value of the old map item for the respective KEY. #! - KEY is the key of the new item. #! - OLD_MAP_ROOT is the root of the old map before insertion -#! - NEW_MAP_ROOT is the root of the new map after insertion. #! #! Panics if: #! - the index is out of bounds (>255). @@ -436,7 +415,6 @@ export.account_add_asset exec.authenticate_account_origin drop drop # => [ASSET, pad(12)] - push.19891 drop # TODO: remove line, see miden-vm/#1122 emit.ACCOUNT_VAULT_BEFORE_ADD_ASSET_EVENT # => [ASSET, pad(12)] @@ -454,7 +432,6 @@ export.account_add_asset # emit event to signal that an asset is being added to the account vault swapw - push.21383 drop # TODO: remove line, see miden-vm/#1122 emit.ACCOUNT_VAULT_AFTER_ADD_ASSET_EVENT dropw # => [ASSET', pad(12)] end @@ -483,7 +460,6 @@ export.account_remove_asset exec.authenticate_account_origin drop drop # => [ASSET, pad(12)] - push.20071 drop # TODO: remove line, see miden-vm/#1122 emit.ACCOUNT_VAULT_BEFORE_REMOVE_ASSET_EVENT # => [ASSET, pad(12)] @@ -496,7 +472,6 @@ export.account_remove_asset # => [ASSET, pad(12)] # emit event to signal that an asset is being removed from the account vault - push.20149 drop # TODO: remove line, see miden-vm/#1122 emit.ACCOUNT_VAULT_AFTER_REMOVE_ASSET_EVENT # => [ASSET, pad(12)] end diff --git a/miden-lib/asm/kernels/transaction/lib/account.masm b/miden-lib/asm/kernels/transaction/lib/account.masm index ec750a36a..26d929c11 100644 --- a/miden-lib/asm/kernels/transaction/lib/account.masm +++ b/miden-lib/asm/kernels/transaction/lib/account.masm @@ -76,6 +76,9 @@ const.ERR_ACCOUNT_ID_EPOCH_MUST_BE_LESS_THAN_U16_MAX=0x00020058 # Unknown account storage mode in account ID. const.ERR_ACCOUNT_ID_UNKNOWN_STORAGE_MODE=0x00020059 +# Failed to read an account map item from a non-map storage slot +const.ERR_ACCOUNT_READING_MAP_VALUE_FROM_NON_MAP_SLOT=0x00020002 + # CONSTANTS # ================================================================================================= @@ -235,13 +238,11 @@ export.incr_nonce u32assert.err=ERR_ACCOUNT_NONCE_INCREASE_MUST_BE_U32 # emit event to signal that account nonce is being incremented - push.20261 drop # TODO: remove line, see miden-vm/#1122 emit.ACCOUNT_BEFORE_INCREMENT_NONCE_EVENT exec.memory::get_acct_nonce add exec.memory::set_acct_nonce - push.20357 drop # TODO: remove line, see miden-vm/#1122 emit.ACCOUNT_AFTER_INCREMENT_NONCE_EVENT end @@ -528,7 +529,6 @@ end #! Panics if: #! - the storage slot type is not value. export.set_item - push.20443 drop # TODO: remove line, see miden-vm/#1122 emit.ACCOUNT_STORAGE_BEFORE_SET_ITEM_EVENT # => [index, V'] @@ -552,25 +552,63 @@ export.set_item # emit event to signal that an account storage item is being updated swapw movup.8 - push.20551 drop # TODO: remove line, see miden-vm/#1122 emit.ACCOUNT_STORAGE_AFTER_SET_ITEM_EVENT drop dropw # => [V] end -#! Sets an item in the specified account storage map. +#! Returns the VALUE located under the specified KEY within the map contained in the given +#! account storage slot. +#! +#! Inputs: [index, KEY] +#! Outputs: [VALUE] +#! +#! Note: +#! - We assume that index has been validated and is within bounds. +#! +#! Where: +#! - index is the index of the storage slot that contains the the map root. +#! - VALUE is the value of the map item at KEY. +#! +#! Panics if: +#! - the requested storage slot type is not map. +export.get_map_item + # check if storage slot type is map + dup exec.get_storage_slot_type + # => [slot_type, index, KEY] + + # check if type == map + exec.constants::get_storage_slot_type_map eq + assert.err=ERR_ACCOUNT_READING_MAP_VALUE_FROM_NON_MAP_SLOT + # => [index, KEY] + + # fetch the account storage item, which is ROOT of the map + exec.get_item swapw + # => [KEY, ROOT] + + # fetch the VALUE located under KEY in the tree + exec.smt::get + # => [VALUE, ROOT] + + # remove the ROOT from the stack + swapw dropw + # => [VALUE] +end + +#! Stores NEW_VALUE under the specified KEY within the map contained in the given account storage slot. #! #! Note: #! - We assume that index has been validated and is within bounds. #! #! Inputs: [index, KEY, NEW_VALUE, OLD_ROOT] -#! Outputs: [OLD_VALUE, NEW_ROOT] +#! Outputs: [OLD_MAP_ROOT, OLD_MAP_VALUE] #! #! Where: +#! - index is the index of the storage slot which contains the map root. #! - OLD_ROOT is the root of the map to set the KEY NEW_VALUE pair. #! - NEW_VALUE is the value to set under KEY. #! - KEY is the key to set. -#! - OLD_VALUE is the previous value of the item. -#! - NEW_ROOT is the new root of the map. +#! - OLD_MAP_VALUE is the previous value of the item. +#! - OLD_MAP_ROOT is the root of the old map before insertion #! #! Panics if: #! - the storage slot type is not map. @@ -585,11 +623,10 @@ export.set_map_item.3 # => [slot_type, index, KEY, NEW_VALUE, OLD_ROOT] # check if slot_type == map - exec.constants::get_storage_slot_type_map eq + exec.constants::get_storage_slot_type_map eq assert.err=ERR_ACCOUNT_SETTING_MAP_ITEM_ON_NON_MAP_SLOT # => [index, KEY, NEW_VALUE, OLD_ROOT] - push.20693 drop # TODO: remove line, see miden-vm/#1122 emit.ACCOUNT_STORAGE_BEFORE_SET_MAP_ITEM_EVENT # => [index, KEY, NEW_VALUE, OLD_ROOT] @@ -601,25 +638,24 @@ export.set_map_item.3 # set the NEW_VALUE under KEY in the tree # note smt::set expects the stack to be [NEW_VALUE, KEY, OLD_ROOT, ...] swapw exec.smt::set - # => [OLD_VALUE, NEW_ROOT, KEY, NEW_VALUE, index, ...] + # => [OLD_MAP_VALUE, NEW_ROOT, KEY, NEW_VALUE, index, ...] - # store OLD_VALUE and NEW_ROOT until the end of the procedure + # store OLD_MAP_VALUE and NEW_ROOT until the end of the procedure loc_storew.1 dropw loc_storew.2 dropw # => [KEY, NEW_VALUE, index, ...] # emit event to signal that an account storage item is being updated movup.8 - push.20771 drop # TODO: remove line, see miden-vm/#1122 emit.ACCOUNT_STORAGE_AFTER_SET_MAP_ITEM_EVENT drop # => [KEY, NEW_VALUE, ...] - # load OLD_VALUE and NEW_ROOT on the top of the stack + # load OLD_MAP_VALUE and NEW_ROOT on the top of the stack loc_loadw.2 swapw loc_loadw.1 swapw - # => [NEW_ROOT, OLD_VALUE, ...] + # => [NEW_ROOT, OLD_MAP_VALUE, ...] # set the root of the map in the respective account storage slot loc_load.0 exec.set_item_raw - # => [OLD_MAP_ROOT, OLD_VALUE, ...] + # => [OLD_MAP_ROOT, OLD_MAP_VALUE, ...] end #! Returns the type of the requested storage slot. @@ -691,7 +727,6 @@ end #! - the procedure root is not part of the account code. export.authenticate_procedure # load procedure index - push.20897 drop # TODO: remove line, see miden-vm/#1122 emit.ACCOUNT_PUSH_PROCEDURE_INDEX_EVENT adv_push.1 # => [index, PROC_ROOT] @@ -746,8 +781,7 @@ export.validate_seed # => [0, 0, account_id_prefix, account_id_suffix, ANCHOR_BLOCK_HASH] # populate first four elements of the rate with the account ID seed - adv.push_mapval push.15263 drop # TODO: remove line, see miden-vm/#1122 - adv_loadw + adv.push_mapval adv_loadw # => [SEED, ANCHOR_BLOCK_HASH] # pad capacity element of hasher @@ -825,7 +859,7 @@ end #! - the computed account storage commitment does not match the provided account storage commitment export.save_account_storage_data # move storage slot data from the advice map to the advice stack - adv.push_mapvaln push.15160 drop # TODO: remove line, see miden-vm/#1122 + adv.push_mapvaln # OS => [STORAGE_COMMITMENT] # AS => [storage_slot_data_len, [STORAGE_SLOT_DATA]] @@ -894,7 +928,7 @@ end #! - the computed account code commitment does not match the provided account code commitment export.save_account_procedure_data # move procedure data from the advice map to the advice stack - adv.push_mapvaln push.15161 drop # TODO: remove line, see miden-vm/#1122 + adv.push_mapvaln # OS => [CODE_COMMITMENT] # AS => [account_procedure_data_len, [ACCOUNT_PROCEDURE_DATA]] diff --git a/miden-lib/asm/kernels/transaction/lib/asset_vault.masm b/miden-lib/asm/kernels/transaction/lib/asset_vault.masm index 0f63f7f0e..203a5902b 100644 --- a/miden-lib/asm/kernels/transaction/lib/asset_vault.masm +++ b/miden-lib/asm/kernels/transaction/lib/asset_vault.masm @@ -151,7 +151,7 @@ export.add_fungible_asset # we therefore overwrite the faucet id with the faucet id from ASSET to account for this edge case mem_loadw swapw # => [ASSET_KEY, VAULT_ROOT, faucet_id_prefix, faucet_id_suffix, amount, vault_root_ptr] - adv.push_smtpeek push.15329 drop # TODO: remove line, see miden-vm/#1122 + adv.push_smtpeek adv_loadw # => [CUR_VAULT_VALUE, VAULT_ROOT, faucet_id_prefix, faucet_id_suffix, amount, vault_root_ptr] swapw @@ -312,7 +312,7 @@ export.remove_fungible_asset # To account for the edge case in which CUR_VAULT_VALUE is an EMPTY_WORD, we replace the most # significant element with the faucet_id to construct the CUR_ASSET. padw dup.14 mem_loadw swapw - adv.push_smtpeek push.15413 drop # TODO: remove line, see miden-vm/#1122 + adv.push_smtpeek adv_loadw dupw movdnw.2 drop movup.11 # => [CUR_ASSET, VAULT_ROOT, CUR_VAULT_VALUE, amount, ASSET, vault_root_ptr] diff --git a/miden-lib/asm/kernels/transaction/lib/epilogue.masm b/miden-lib/asm/kernels/transaction/lib/epilogue.masm index 409b53698..6f6f6b05a 100644 --- a/miden-lib/asm/kernels/transaction/lib/epilogue.masm +++ b/miden-lib/asm/kernels/transaction/lib/epilogue.masm @@ -58,7 +58,7 @@ proc.copy_output_notes_to_advice_map # => [OUTPUT_NOTES_COMMITMENT, output_note_ptr, output_notes_end_ptr] # insert created data into the advice map - adv.insert_mem push.15511 drop # TODO: remove line, see miden-vm/#1122 + adv.insert_mem # => [OUTPUT_NOTES_COMMITMENT, output_note_ptr, output_notes_end_ptr] # drop output note pointers @@ -274,7 +274,7 @@ export.finalize_transaction # => [FINAL_ACCOUNT_HASH, acct_data_ptr, acct_data_end_ptr, INIT_ACCT_HASH] # insert final account data into the advice map - adv.insert_mem push.15619 drop # TODO: remove line, see miden-vm/#1122 + adv.insert_mem # => [FINAL_ACCOUNT_HASH, acct_data_ptr, acct_data_end_ptr, INIT_ACCT_HASH] # drop account data section pointers @@ -295,8 +295,6 @@ export.finalize_transaction exec.memory::get_acct_nonce # => [current_nonce, init_nonce, FINAL_ACCOUNT_HASH, INIT_ACCT_HASH] - push.1005 drop # TODO: remove line, see miden-vm/#1122 - # assert that initial nonce is less than current nonce lt assert.err=ERR_ACCOUNT_NONCE_DID_NOT_INCREASE_AFTER_STATE_CHANGE # => [FINAL_ACCOUNT_HASH, INIT_ACCT_HASH] diff --git a/miden-lib/asm/kernels/transaction/lib/prologue.masm b/miden-lib/asm/kernels/transaction/lib/prologue.masm index 83c2e3059..1a3323be4 100644 --- a/miden-lib/asm/kernels/transaction/lib/prologue.masm +++ b/miden-lib/asm/kernels/transaction/lib/prologue.masm @@ -374,7 +374,6 @@ proc.validate_new_account exec.constants::get_empty_smt_root assert_eqw.err=ERR_PROLOGUE_NEW_NON_FUNGIBLE_FAUCET_RESERVED_SLOT_MUST_BE_VALID_EMPY_SMT # => [] - push.1001 drop # TODO: remove line, see miden-vm/#1122 # get the faucet reserved storage data slot type exec.account::get_faucet_storage_data_slot exec.account::get_storage_slot_type @@ -982,7 +981,7 @@ proc.process_input_notes_data dup neq.0 if.true exec.memory::get_input_notes_commitment - adv.push_mapval push.15787 drop # TODO: remove line, see miden-vm/#1122 + adv.push_mapval dropw end # => [num_notes] diff --git a/miden-lib/asm/kernels/transaction/lib/tx.masm b/miden-lib/asm/kernels/transaction/lib/tx.masm index dbaa41b09..1edef8668 100644 --- a/miden-lib/asm/kernels/transaction/lib/tx.masm +++ b/miden-lib/asm/kernels/transaction/lib/tx.masm @@ -477,7 +477,6 @@ end #! - the note_tag starts with anything but 0b11 and note_type is not public. #! - the number of output notes exceeds the maximum limit of 1024. export.create_note - push.20983 drop # TODO: remove line, see miden-vm/#1122 emit.NOTE_BEFORE_CREATED_EVENT exec.build_note_metadata @@ -495,7 +494,6 @@ export.create_note # => [NOTE_METADATA, note_ptr, RECIPIENT, note_idx] # emit event to signal that a new note is created - push.21067 drop # TODO: remove line, see miden-vm/#1122 emit.NOTE_AFTER_CREATED_EVENT # set the metadata for the output note @@ -539,9 +537,8 @@ export.add_asset_to_note # => [ASSET, note_ptr, num_of_assets, note_idx] # emit event to signal that a new asset is going to be added to the note. - push.21169 drop # TODO: remove line, see miden-vm/#1122 emit.NOTE_BEFORE_ADD_ASSET_EVENT - # => [ASSET, note_ptr] + # => [ASSET, note_ptr, num_of_assets, note_idx] # Check if ASSET to add is fungible exec.asset::is_fungible_asset @@ -559,7 +556,6 @@ export.add_asset_to_note # => [note_ptr, note_idx] # emit event to signal that a new asset was added to the note. - push.21277 drop # TODO: remove line, see miden-vm/#1122 emit.NOTE_AFTER_ADD_ASSET_EVENT drop diff --git a/miden-lib/asm/kernels/transaction/main.masm b/miden-lib/asm/kernels/transaction/main.masm index 9e065bcec..256bf0399 100644 --- a/miden-lib/asm/kernels/transaction/main.masm +++ b/miden-lib/asm/kernels/transaction/main.masm @@ -61,19 +61,16 @@ const.EPILOGUE_END=131081 proc.main.1 # Prologue # --------------------------------------------------------------------------------------------- - push.0 drop # TODO: remove line, see miden-vm/#1122 trace.PROLOGUE_START exec.prologue::prepare_transaction # => [] - push.1 drop # TODO: remove line, see miden-vm/#1122 trace.PROLOGUE_END # Note Processing # --------------------------------------------------------------------------------------------- - push.2 drop # TODO: remove line, see miden-vm/#1122 trace.NOTES_PROCESSING_START exec.memory::get_num_input_notes @@ -87,7 +84,6 @@ proc.main.1 # => [should_loop] while.true - push.4 drop # TODO: remove line, see miden-vm/#1122 trace.NOTE_EXECUTION_START # => [] @@ -109,20 +105,17 @@ proc.main.1 loc_load.0 neq # => [should_loop] - push.5 drop # TODO: remove line, see miden-vm/#1122 trace.NOTE_EXECUTION_END end exec.note::note_processing_teardown # => [] - push.3 drop # TODO: remove line, see miden-vm/#1122 trace.NOTES_PROCESSING_END # Transaction Script Processing # --------------------------------------------------------------------------------------------- - push.6 drop # TODO: remove line, see miden-vm/#1122 trace.TX_SCRIPT_PROCESSING_START # get the memory address of the transaction script root and load it to the stack @@ -147,13 +140,11 @@ proc.main.1 # => [] end - push.7 drop # TODO: remove line, see miden-vm/#1122 trace.TX_SCRIPT_PROCESSING_END # Epilogue # --------------------------------------------------------------------------------------------- - push.8 drop # TODO: remove line, see miden-vm/#1122 trace.EPILOGUE_START # execute the transaction epilogue @@ -163,7 +154,6 @@ proc.main.1 # truncate the stack movupw.3 dropw movupw.3 dropw movup.9 drop - push.9 drop # TODO: remove line, see miden-vm/#1122 trace.EPILOGUE_END # => [OUTPUT_NOTES_COMMITMENT, FINAL_ACCOUNT_HASH, tx_expiration_block_num] end diff --git a/miden-lib/asm/miden/note.masm b/miden-lib/asm/miden/note.masm index 67ccb4d5f..f06f92984 100644 --- a/miden-lib/asm/miden/note.masm +++ b/miden-lib/asm/miden/note.masm @@ -66,7 +66,7 @@ export.get_assets # => [ASSETS_HASH, num_assets, dest_ptr] # load the asset data from the advice map to the advice stack - adv.push_mapval push.15887 drop # TODO: remove line, see miden-vm/#1122 + adv.push_mapval # => [ASSETS_HASH, num_assets, dest_ptr] # calculate number of assets rounded up to an even number @@ -113,7 +113,7 @@ export.get_inputs # => [INPUTS_HASH, dest_ptr] # load the inputs from the advice map to the advice stack - adv.push_mapval push.15973 drop # TODO: remove line, see miden-vm/#1122 + adv.push_mapval # => [INPUTS_HASH, dest_ptr] adv_push.1 diff --git a/miden-lib/src/transaction/procedures/kernel_v0.rs b/miden-lib/src/transaction/procedures/kernel_v0.rs index 0d0e2cdfb..9d52c4398 100644 --- a/miden-lib/src/transaction/procedures/kernel_v0.rs +++ b/miden-lib/src/transaction/procedures/kernel_v0.rs @@ -16,33 +16,33 @@ pub const KERNEL0_PROCEDURES: [Digest; 35] = [ // account_get_nonce digest!("0x7456aa74a3bddeacfbcf02eb04130b8aea0772b5b15f0aeb49f91b1269d16bf7"), // account_incr_nonce - digest!("0xfc2da7d93bd401f08dfa9dd24523db712ea17d528db992e29fd3a6504f5aafe5"), + digest!("0x1760c860042c6e36b6b1b1f9f177c86b113db1d7a7c6445ac5414284fcc6f311"), // account_get_code_commitment - digest!("0xc1b532722380c4235dd91f23f5733177288d8f295d8471657e37de6f9e472e08"), + digest!("0xa19e305766cd81293460dddada4f885edd7a1f301cdefc2d004f3fcb69fcba77"), // account_get_storage_commitment - digest!("0x69d9f332e44293e7eb5bb7e5e6abbe98f7c8abfb681ac05c8ac69cdb9ffe71b6"), + digest!("0x04ed2802cd12739fcb8ff83500301accbf078e8eb89807825c4c18ee5b6c00cb"), // account_get_item - digest!("0x1a73504fc477eb17c9c9fcedd97ba4f397c9827c562aff1822a2fd08f8e9da18"), + digest!("0x677538ad2411983435f0b942b1fc6b3e5df0cf0d10bae34d56b48c54311b773a"), // account_set_item - digest!("0x4d2ef9eac1e0560f763cc09c1d669f6f47cf4d0c3deea54e4cb77bfcda9a3602"), + digest!("0xebfa25d6653e1ed501d4af35a37b00071097ff9dadf10c5a7d87f3dba65d757c"), // account_get_map_item - digest!("0x2b3ad4c92f7cbce843eae3ef56e061317b41a46116cdc7bdd9da1b5318228523"), + digest!("0xac89c3cd71f8f96e15b8f4c83b6c5b4abd9e161d3b195ebb4e1f885d7ce94b94"), // account_set_map_item - digest!("0x02befd8e777bacc5f4c3b14267b7e5558c9557d82970e74612c5aa6d7febf9c2"), + digest!("0x64cf602d692a3efbd72f2b4031c0a384cbc56e5a8929b4031aeafbee6a9ff8a3"), // account_get_vault_commitment digest!("0xbfa1956f7b944f3c8e4e6fdb4e4ace50cda03f94cac33420c8b7e97a0c39981c"), // account_add_asset - digest!("0x78d6a2544fcb8d8749165ece83cd72f73ffed433f39543f87090691f5c052838"), + digest!("0x2dc318c6659b20f5d145fd4153fe48a14f2c8dc4573fa98bf80d2df1db0e63bc"), // account_remove_asset - digest!("0x80c0cdf81ff7f845ee45a6473627611e1b251b3c8368c3764cdc8d48f2e7b806"), + digest!("0xbda7b544551c540ef9e67f3b1ee00d4292c419812c035c5bf53b9de28bcdeb48"), // account_get_balance digest!("0xbe5819bf5aabf1a7c9d8ebfc330a7869f610ca4f86e3708265d8d361b5b82f14"), // account_has_non_fungible_asset digest!("0xce07ab13a9320a39642412ee6673eeee228a0173e00047c8b05bc259343b209a"), // faucet_mint_asset - digest!("0x83521ee1aec0ffb09cfc65c47bb08aa5ad3924da42ab5a4bff9152d6534d4918"), + digest!("0x475215f1e401eb72d4760f860fd7ac022e92472b767fd4a5e9c60e8483cb8764"), // faucet_burn_asset - digest!("0x27f721dba5a5710e295ac9404e17a9497f2052bbbb73293cd0fe0ba1ad63bad6"), + digest!("0x94686503a323b4fbefb758210a41c2e27d9c22f14cc07e9a28be2e564508eb20"), // faucet_get_total_fungible_asset_issuance digest!("0xe56c3538757d5e2cee385028a5ba4e78eca3982698a4e6a3d5ddb17ea59fc13a"), // faucet_is_non_fungible_asset_issued @@ -50,7 +50,7 @@ pub const KERNEL0_PROCEDURES: [Digest; 35] = [ // note_get_assets_info digest!("0xcca266d382dfdd980ad1884bdf78525cc090fe05f4d6839d1df067382b120e2f"), // note_add_asset - digest!("0xf0ae3bde840b005661422bcf64d1700602aae19f5bcff038ed7fed95b384a42f"), + digest!("0x0018adf033ed13ae21864b028a959c7ca358fb5b34d305f9f1aaf96a0fded537"), // note_get_serial_number digest!("0xad91130ec219756213c6cadeaf8a38de8768e50c620cb7c347c531874c6054b6"), // note_get_inputs_hash @@ -60,7 +60,7 @@ pub const KERNEL0_PROCEDURES: [Digest; 35] = [ // note_get_script_hash digest!("0x82a32c90566e7de09e99b2151851c77a6b4117aa05aba20fe7d8244f065d6870"), // tx_create_note - digest!("0x1f4f867434fc2704a16a15edd783f3825b1b5d932e04412b46eee3c85adcf1d2"), + digest!("0x1df987f8f3bb9fbfec3f3252031e146c0318772d45efcde56cc50275c766276a"), // tx_get_input_notes_commitment digest!("0x44a86433c9a03c7ee99d046b0cd16e05df275f05ebb176b60193207229eae8b5"), // tx_get_output_notes_commitment @@ -70,7 +70,7 @@ pub const KERNEL0_PROCEDURES: [Digest; 35] = [ // tx_get_block_number digest!("0x17da2a77b878820854bfff2b5f9eb969a4e2e76a998f97f4967b2b1a7696437c"), // tx_start_foreign_context - digest!("0x8c3adb3aff1686205283a59d3908e074c1f4768ac4c7a778ef2b873693ca9101"), + digest!("0xa44071231dad1165c9e785a780044347da436cfd97def777d4af4771f3de8c43"), // tx_end_foreign_context digest!("0x132b50feca8ecec10937c740640c59733e643e89c1d8304cf4523120e27a0428"), // tx_get_expiration_delta diff --git a/miden-tx/src/executor/data_store.rs b/miden-tx/src/executor/data_store.rs index 83a5aae29..989dd64f0 100644 --- a/miden-tx/src/executor/data_store.rs +++ b/miden-tx/src/executor/data_store.rs @@ -1,7 +1,9 @@ #[cfg(feature = "async")] use alloc::boxed::Box; -use miden_objects::{accounts::AccountId, notes::NoteId, transaction::TransactionInputs}; +use miden_objects::{ + accounts::AccountId, block::BlockNumber, notes::NoteId, transaction::TransactionInputs, +}; use winter_maybe_async::*; use crate::DataStoreError; @@ -32,7 +34,7 @@ pub trait DataStore { fn get_transaction_inputs( &self, account_id: AccountId, - block_ref: u32, + block_ref: BlockNumber, notes: &[NoteId], ) -> Result; } diff --git a/miden-tx/src/executor/mod.rs b/miden-tx/src/executor/mod.rs index 4b477daac..51ad4fbff 100644 --- a/miden-tx/src/executor/mod.rs +++ b/miden-tx/src/executor/mod.rs @@ -4,6 +4,7 @@ use miden_lib::transaction::TransactionKernel; use miden_objects::{ accounts::{AccountCode, AccountId}, assembly::Library, + block::BlockNumber, notes::NoteId, transaction::{ExecutedTransaction, TransactionArgs, TransactionInputs}, vm::StackOutputs, @@ -128,7 +129,7 @@ impl TransactionExecutor { pub fn execute_transaction( &self, account_id: AccountId, - block_ref: u32, + block_ref: BlockNumber, notes: &[NoteId], tx_args: TransactionArgs, ) -> Result { diff --git a/miden-tx/src/host/mod.rs b/miden-tx/src/host/mod.rs index f0951b185..e3c21c6d8 100644 --- a/miden-tx/src/host/mod.rs +++ b/miden-tx/src/host/mod.rs @@ -176,7 +176,7 @@ impl TransactionHost { /// Adds an asset at the top of the [OutputNoteBuilder] identified by the note pointer. /// - /// Expected stack state: [ASSET, note_ptr, ...] + /// Expected stack state: [ASSET, note_ptr, num_of_assets, note_idx] fn on_note_before_add_asset( &mut self, process: &S, diff --git a/miden-tx/src/testing/mock_chain/mod.rs b/miden-tx/src/testing/mock_chain/mod.rs index 3a2f8ed6f..5c63d2aae 100644 --- a/miden-tx/src/testing/mock_chain/mod.rs +++ b/miden-tx/src/testing/mock_chain/mod.rs @@ -12,8 +12,8 @@ use miden_objects::{ }, assets::{Asset, FungibleAsset, TokenSymbol}, block::{ - block_num_from_epoch, compute_tx_hash, Block, BlockAccountUpdate, BlockNoteIndex, - BlockNoteTree, NoteBatch, + compute_tx_hash, Block, BlockAccountUpdate, BlockNoteIndex, BlockNoteTree, BlockNumber, + NoteBatch, }, crypto::{ dsa::rpo_falcon512::SecretKey, @@ -602,14 +602,14 @@ impl MockChain { let block = self.blocks.last().unwrap(); let mut input_notes = vec![]; - let mut block_headers_map: BTreeMap = BTreeMap::new(); + let mut block_headers_map: BTreeMap = BTreeMap::new(); for note in notes { let input_note = self.available_notes.get(note).expect("Note not found").clone(); let note_block_num = input_note.location().unwrap().block_num(); if note_block_num != block.header().block_num() { block_headers_map.insert( note_block_num, - self.blocks.get(note_block_num as usize).unwrap().header(), + self.blocks.get(note_block_num.as_usize()).unwrap().header(), ); } input_notes.push(input_note); @@ -618,13 +618,13 @@ impl MockChain { // If the account is new, add the anchor block's header from which the account ID is derived // to the MMR. if account.is_new() { - let epoch_block_num = block_num_from_epoch(account.id().anchor_epoch()); + let epoch_block_num = BlockNumber::from_epoch(account.id().anchor_epoch()); // The reference block of the transaction is added to the MMR in // prologue::process_chain_data so we can skip adding it to the block headers here. if epoch_block_num != block.header().block_num() { block_headers_map.insert( epoch_block_num, - self.blocks.get(epoch_block_num as usize).unwrap().header(), + self.blocks.get(epoch_block_num.as_usize()).unwrap().header(), ); } } @@ -653,7 +653,9 @@ impl MockChain { /// This will also make all the objects currently pending available for use. /// If `block_num` is `Some(number)`, blocks will be generated up to `number`. pub fn seal_block(&mut self, block_num: Option) -> Block { - let next_block_num = self.blocks.last().map_or(0, |b| b.header().block_num() + 1); + let next_block_num = + self.blocks.last().map_or(0, |b| b.header().block_num().child().as_u32()); + let target_block_num = block_num.unwrap_or(next_block_num); if target_block_num < next_block_num { @@ -711,7 +713,7 @@ impl MockChain { let header = BlockHeader::new( version, prev_hash, - current_block_num, + BlockNumber::from(current_block_num), chain_root, account_root, nullifier_root, diff --git a/miden-tx/src/testing/tx_context/mod.rs b/miden-tx/src/testing/tx_context/mod.rs index e3cd17e9c..657f33080 100644 --- a/miden-tx/src/testing/tx_context/mod.rs +++ b/miden-tx/src/testing/tx_context/mod.rs @@ -7,6 +7,7 @@ use miden_lib::transaction::TransactionKernel; use miden_objects::{ accounts::{Account, AccountCode, AccountId}, assembly::Assembler, + block::BlockNumber, notes::{Note, NoteId}, transaction::{ExecutedTransaction, InputNote, InputNotes, TransactionArgs, TransactionInputs}, }; @@ -139,7 +140,7 @@ impl DataStore for TransactionInputs { fn get_transaction_inputs( &self, account_id: AccountId, - block_num: u32, + block_num: BlockNumber, notes: &[NoteId], ) -> Result { assert_eq!(account_id, self.account().id()); diff --git a/miden-tx/src/tests/kernel_tests/test_epilogue.rs b/miden-tx/src/tests/kernel_tests/test_epilogue.rs index b294624c6..9887307e8 100644 --- a/miden-tx/src/tests/kernel_tests/test_epilogue.rs +++ b/miden-tx/src/tests/kernel_tests/test_epilogue.rs @@ -244,7 +244,8 @@ fn test_block_expiration_height_monotonically_decreases() { // Expiry block should be set to transaction's block + the stored expiration delta // (which can only decrease, not increase) - let expected_expiry = v1.min(v2) + tx_context.tx_inputs().block_header().block_num() as u64; + let expected_expiry = + v1.min(v2) + tx_context.tx_inputs().block_header().block_num().as_u64(); assert_eq!(process.get_stack_item(8).as_int(), expected_expiry); } } diff --git a/miden-tx/src/tests/kernel_tests/test_prologue.rs b/miden-tx/src/tests/kernel_tests/test_prologue.rs index 43f9de9ab..718ca7f3a 100644 --- a/miden-tx/src/tests/kernel_tests/test_prologue.rs +++ b/miden-tx/src/tests/kernel_tests/test_prologue.rs @@ -233,7 +233,7 @@ fn chain_mmr_memory_assertions(process: &Process, prepared_tx: &Transa assert_eq!( read_root_mem_value(process, CHAIN_MMR_NUM_LEAVES_PTR)[0], - Felt::new(chain_mmr.chain_length() as u64), + Felt::new(chain_mmr.chain_length().as_u64()), "The number of leaves should be stored at the CHAIN_MMR_NUM_LEAVES_PTR" ); diff --git a/objects/src/accounts/account_id/id_anchor.rs b/objects/src/accounts/account_id/id_anchor.rs index 1fdb2baf5..c2572f8ee 100644 --- a/objects/src/accounts/account_id/id_anchor.rs +++ b/objects/src/accounts/account_id/id_anchor.rs @@ -1,6 +1,4 @@ -use crate::{ - block::block_epoch_from_number, errors::AccountIdError, BlockHeader, Digest, EMPTY_WORD, -}; +use crate::{block::BlockNumber, errors::AccountIdError, BlockHeader, Digest, EMPTY_WORD}; // ACCOUNT ID ANCHOR // ================================================================================================ @@ -14,8 +12,8 @@ use crate::{ /// # Constraints /// /// This type enforces the following constraints. -/// - The `anchor_block_number` % 2^[`BlockHeader::EPOCH_LENGTH_EXPONENT`] must be zero. In other -/// words, the block number must a multiple of 2^[`BlockHeader::EPOCH_LENGTH_EXPONENT`]. +/// - The `anchor_block_number` % 2^[`BlockNumber::EPOCH_LENGTH_EXPONENT`] must be zero. In other +/// words, the block number must a multiple of 2^[`BlockNumber::EPOCH_LENGTH_EXPONENT`]. /// - The epoch derived from the `anchor_block_number` must be strictly less than [`u16::MAX`]. #[derive(Debug, Clone, Copy)] pub struct AccountIdAnchor { @@ -51,14 +49,14 @@ impl AccountIdAnchor { /// Returns an error if any of the anchor constraints are not met. See the [type /// documentation](AccountIdAnchor) for details. pub fn new( - anchor_block_number: u32, + anchor_block_number: BlockNumber, anchor_block_hash: Digest, ) -> Result { - if anchor_block_number & 0x0000_ffff != 0 { + if anchor_block_number.as_u32() & 0x0000_ffff != 0 { return Err(AccountIdError::AnchorBlockMustBeEpochBlock); } - let anchor_epoch = block_epoch_from_number(anchor_block_number); + let anchor_epoch = anchor_block_number.block_epoch(); if anchor_epoch == u16::MAX { return Err(AccountIdError::AnchorEpochMustNotBeU16Max); diff --git a/objects/src/accounts/account_id/mod.rs b/objects/src/accounts/account_id/mod.rs index 841321f0c..e746dae19 100644 --- a/objects/src/accounts/account_id/mod.rs +++ b/objects/src/accounts/account_id/mod.rs @@ -92,7 +92,7 @@ use crate::{errors::AccountIdError, AccountError, ACCOUNT_TREE_DEPTH}; /// as the [`NoteMetadata`](crate::notes::NoteMetadata). In such cases, it can happen that all /// bits of the encoded suffix would be one, so having the epoch constraint is important. /// - The ID is dependent on the hash of an epoch block. This is a block whose number is a multiple -/// of 2^[`BlockHeader::EPOCH_LENGTH_EXPONENT`][epoch_len_exp], e.g. `0`, `65536`, `131072`, ... +/// of 2^[`BlockNumber::EPOCH_LENGTH_EXPONENT`][epoch_len_exp], e.g. `0`, `65536`, `131072`, ... /// These are the first blocks of epoch 0, 1, 2, ... We call this dependence _anchoring_ because /// the ID is anchored to that epoch block's hash. Anchoring makes it practically impossible for /// an attacker to construct a rainbow table of account IDs whose epoch is X, if the block for @@ -108,7 +108,7 @@ use crate::{errors::AccountIdError, AccountError, ACCOUNT_TREE_DEPTH}; /// hashes to the user's ID can claim the assets sent to the user's ID. Adding the anchor /// block hash to ID generation process makes this attack practically impossible. /// -/// [epoch_len_exp]: crate::block::BlockHeader::EPOCH_LENGTH_EXPONENT +/// [epoch_len_exp]: crate::block::BlockNumber::EPOCH_LENGTH_EXPONENT #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum AccountId { V0(AccountIdV0), diff --git a/objects/src/accounts/builder/mod.rs b/objects/src/accounts/builder/mod.rs index 1ab8eadff..70c6241ef 100644 --- a/objects/src/accounts/builder/mod.rs +++ b/objects/src/accounts/builder/mod.rs @@ -251,7 +251,7 @@ mod tests { use vm_core::FieldElement; use super::*; - use crate::accounts::StorageSlot; + use crate::{accounts::StorageSlot, block::BlockNumber}; const CUSTOM_CODE1: &str = " export.foo @@ -317,7 +317,9 @@ mod tests { let anchor_block_hash = Digest::new([Felt::new(42); 4]); let anchor_block_number = 1 << 16; - let id_anchor = AccountIdAnchor::new(anchor_block_number, anchor_block_hash).unwrap(); + let id_anchor = + AccountIdAnchor::new(BlockNumber::from(anchor_block_number), anchor_block_hash) + .unwrap(); let (account, seed) = Account::builder([5; 32]) .anchor(id_anchor) diff --git a/objects/src/block/block_number.rs b/objects/src/block/block_number.rs new file mode 100644 index 000000000..51efc33de --- /dev/null +++ b/objects/src/block/block_number.rs @@ -0,0 +1,114 @@ +use core::{fmt, ops::Add}; + +use super::Felt; +use crate::utils::serde::{ + ByteReader, ByteWriter, Deserializable, DeserializationError, Serializable, +}; + +// BLOCK NUMBER +// ================================================================================================ + +/// A convenience wrapper around a `u32` representing the number of a block. +/// +/// Each block has a unique number and block numbers increase monotonically by `1`. +#[derive(Debug, Default, Eq, PartialEq, Copy, Clone, PartialOrd, Ord, Hash)] +pub struct BlockNumber(u32); + +impl BlockNumber { + /// The length of an epoch expressed as a power of two. `2^(EPOCH_LENGTH_EXPONENT)` is the + /// number of blocks in an epoch. + /// + /// The epoch of a block can be obtained by shifting the block number to the right by this + /// exponent. + pub const EPOCH_LENGTH_EXPONENT: u8 = 16; + + /// Genesis BlockNumber + pub const GENESIS: Self = Self(0); + + /// Returns the previous block number + pub fn parent(self) -> Option { + self.checked_sub(1) + } + + /// Returns the next block number + pub fn child(self) -> BlockNumber { + self + 1 + } + + /// Creates the [`BlockNumber`] corresponding to the epoch block for the provided `epoch`. + pub const fn from_epoch(epoch: u16) -> BlockNumber { + BlockNumber((epoch as u32) << BlockNumber::EPOCH_LENGTH_EXPONENT) + } + + /// Creates a `BlockNumber` from a `usize`. + pub fn from_usize(value: usize) -> Self { + BlockNumber(value as u32) + } + + /// Returns the epoch to which this block number belongs. + pub const fn block_epoch(&self) -> u16 { + (self.0 >> BlockNumber::EPOCH_LENGTH_EXPONENT) as u16 + } + + /// Returns the block number as a `u32`. + pub fn as_u32(&self) -> u32 { + self.0 + } + + /// Returns the block number as a `u64`. + pub fn as_u64(&self) -> u64 { + self.0 as u64 + } + + /// Returns the block number as a `usize`. + pub fn as_usize(&self) -> usize { + self.0 as usize + } + + /// Checked integer subtraction. Computes `self - rhs`, returning `None` if underflow occurred. + pub fn checked_sub(&self, rhs: u32) -> Option { + self.0.checked_sub(rhs).map(Self) + } +} + +impl Add for BlockNumber { + type Output = Self; + + fn add(self, other: u32) -> Self::Output { + BlockNumber(self.0 + other) + } +} + +impl Serializable for BlockNumber { + fn write_into(&self, target: &mut W) { + target.write_u32(self.0); + } + + fn get_size_hint(&self) -> usize { + core::mem::size_of::() + } +} + +impl Deserializable for BlockNumber { + fn read_from(source: &mut R) -> Result { + source.read::().map(BlockNumber::from) + } +} + +impl From for Felt { + fn from(value: BlockNumber) -> Self { + Felt::from(value.as_u32()) + } +} + +impl From for BlockNumber { + fn from(value: u32) -> Self { + BlockNumber(value) + } +} + +impl fmt::Display for BlockNumber { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.0) + } +} diff --git a/objects/src/block/header.rs b/objects/src/block/header.rs index 7ae216b26..4a0f57ed3 100644 --- a/objects/src/block/header.rs +++ b/objects/src/block/header.rs @@ -1,6 +1,6 @@ use alloc::vec::Vec; -use super::{Digest, Felt, Hasher, ZERO}; +use super::{BlockNumber, Digest, Felt, Hasher, ZERO}; use crate::utils::serde::{ ByteReader, ByteWriter, Deserializable, DeserializationError, Serializable, }; @@ -29,7 +29,7 @@ use crate::utils::serde::{ pub struct BlockHeader { version: u32, prev_hash: Digest, - block_num: u32, + block_num: BlockNumber, chain_root: Digest, account_root: Digest, nullifier_root: Digest, @@ -43,19 +43,12 @@ pub struct BlockHeader { } impl BlockHeader { - /// The length of an epoch expressed as a power of two. `2^(EPOCH_LENGTH_EXPONENT)` is the - /// number of blocks in an epoch. - /// - /// The epoch of a block can be obtained by shifting the block number to the right by this - /// exponent. - pub const EPOCH_LENGTH_EXPONENT: u8 = 16; - /// Creates a new block header. #[allow(clippy::too_many_arguments)] pub fn new( version: u32, prev_hash: Digest, - block_num: u32, + block_num: BlockNumber, chain_root: Digest, account_root: Digest, nullifier_root: Digest, @@ -76,7 +69,7 @@ impl BlockHeader { kernel_root, proof_hash, timestamp, - block_num, + block_num.as_u32(), ); // The sub hash is merged with the note_root - hash(sub_hash, note_root) to produce the @@ -129,15 +122,15 @@ impl BlockHeader { } /// Returns the block number. - pub fn block_num(&self) -> u32 { + pub fn block_num(&self) -> BlockNumber { self.block_num } /// Returns the epoch to which this block belongs. /// - /// This is the block number shifted right by [`Self::EPOCH_LENGTH_EXPONENT`]. + /// This is the block number shifted right by [`BlockNumber::EPOCH_LENGTH_EXPONENT`]. pub fn block_epoch(&self) -> u16 { - block_epoch_from_number(self.block_num) + self.block_num.block_epoch() } /// Returns the chain root. @@ -187,8 +180,8 @@ impl BlockHeader { } /// Returns the block number of the epoch block to which this block belongs. - pub fn epoch_block_num(&self) -> u32 { - block_num_from_epoch(self.block_epoch()) + pub fn epoch_block_num(&self) -> BlockNumber { + BlockNumber::from_epoch(self.block_epoch()) } // HELPERS @@ -275,19 +268,6 @@ impl Deserializable for BlockHeader { } } -// UTILITIES -// ================================================================================================ - -/// Returns the block number of the epoch block for the given `epoch`. -pub const fn block_num_from_epoch(epoch: u16) -> u32 { - (epoch as u32) << BlockHeader::EPOCH_LENGTH_EXPONENT -} - -/// Returns the epoch of the given block number. -pub const fn block_epoch_from_number(block_number: u32) -> u16 { - (block_number >> BlockHeader::EPOCH_LENGTH_EXPONENT) as u16 -} - #[cfg(test)] mod tests { use vm_core::Word; diff --git a/objects/src/block/mod.rs b/objects/src/block/mod.rs index d915dea8a..77c0f8992 100644 --- a/objects/src/block/mod.rs +++ b/objects/src/block/mod.rs @@ -6,7 +6,9 @@ use super::{ }; mod header; -pub use header::{block_epoch_from_number, block_num_from_epoch, BlockHeader}; +pub use header::BlockHeader; +mod block_number; +pub use block_number::BlockNumber; mod note_tree; pub use note_tree::{BlockNoteIndex, BlockNoteTree}; diff --git a/objects/src/errors.rs b/objects/src/errors.rs index ed7a0967a..523577e7a 100644 --- a/objects/src/errors.rs +++ b/objects/src/errors.rs @@ -20,10 +20,9 @@ use crate::{ AccountCode, AccountIdPrefix, AccountStorage, AccountType, PlaceholderType, StoragePlaceholder, }, - block::block_num_from_epoch, + block::BlockNumber, notes::{NoteAssets, NoteExecutionHint, NoteTag, NoteType, Nullifier}, - BlockHeader, ACCOUNT_UPDATE_MAX_SIZE, MAX_INPUTS_PER_NOTE, MAX_INPUT_NOTES_PER_TX, - MAX_OUTPUT_NOTES_PER_TX, + ACCOUNT_UPDATE_MAX_SIZE, MAX_INPUTS_PER_NOTE, MAX_INPUT_NOTES_PER_TX, MAX_OUTPUT_NOTES_PER_TX, }; // ACCOUNT COMPONENT TEMPLATE ERROR @@ -148,7 +147,7 @@ pub enum AccountIdError { AccountIdSuffixLeastSignificantByteMustBeZero, #[error( "anchor block must be an epoch block, that is, its block number must be a multiple of 2^{}", - BlockHeader::EPOCH_LENGTH_EXPONENT + BlockNumber::EPOCH_LENGTH_EXPONENT )] AnchorBlockMustBeEpochBlock, } @@ -305,23 +304,26 @@ pub enum NoteError { #[derive(Debug, Error)] pub enum ChainMmrError { #[error("block num {block_num} exceeds chain length {chain_length} implied by the chain MMR")] - BlockNumTooBig { chain_length: usize, block_num: u32 }, + BlockNumTooBig { + chain_length: usize, + block_num: BlockNumber, + }, #[error("duplicate block {block_num} in chain MMR")] - DuplicateBlock { block_num: u32 }, + DuplicateBlock { block_num: BlockNumber }, #[error("chain MMR does not track authentication paths for block {block_num}")] - UntrackedBlock { block_num: u32 }, + UntrackedBlock { block_num: BlockNumber }, } impl ChainMmrError { - pub fn block_num_too_big(chain_length: usize, block_num: u32) -> Self { + pub fn block_num_too_big(chain_length: usize, block_num: BlockNumber) -> Self { Self::BlockNumTooBig { chain_length, block_num } } - pub fn duplicate_block(block_num: u32) -> Self { + pub fn duplicate_block(block_num: BlockNumber) -> Self { Self::DuplicateBlock { block_num } } - pub fn untracked_block(block_num: u32) -> Self { + pub fn untracked_block(block_num: BlockNumber) -> Self { Self::UntrackedBlock { block_num } } } @@ -346,7 +348,7 @@ pub enum TransactionInputError { AccountSeedProvidedForExistingAccount, #[error( "anchor block header for epoch {0} (block number = {block_number}) must be provided in the chain mmr for the new account", - block_number = block_num_from_epoch(*.0), + block_number = BlockNumber::from_epoch(*.0), )] AnchorBlockHeaderNotProvidedForNewAccount(u16), #[error("transaction input note with nullifier {0} is a duplicate")] @@ -354,13 +356,16 @@ pub enum TransactionInputError { #[error("ID {expected} of the new account does not match the ID {actual} computed from the provided seed")] InconsistentAccountSeed { expected: AccountId, actual: AccountId }, #[error("chain mmr has length {actual} which does not match block number {expected} ")] - InconsistentChainLength { expected: u32, actual: u32 }, + InconsistentChainLength { + expected: BlockNumber, + actual: BlockNumber, + }, #[error("chain mmr has root {actual} which does not match block header's root {expected}")] InconsistentChainRoot { expected: Digest, actual: Digest }, #[error("block in which input note with id {0} was created is not in chain mmr")] InputNoteBlockNotInChainMmr(NoteId), #[error("input note with id {0} was not created in block {1}")] - InputNoteNotInBlock(NoteId, u32), + InputNoteNotInBlock(NoteId, BlockNumber), #[error("account ID computed from seed is invalid")] InvalidAccountIdSeed(#[source] AccountIdError), #[error( diff --git a/objects/src/notes/file.rs b/objects/src/notes/file.rs index 5e6188f5d..39899de75 100644 --- a/objects/src/notes/file.rs +++ b/objects/src/notes/file.rs @@ -108,6 +108,7 @@ mod tests { use crate::{ accounts::AccountId, assets::{Asset, FungibleAsset}, + block::BlockNumber, notes::{ Note, NoteAssets, NoteFile, NoteInclusionProof, NoteInputs, NoteMetadata, NoteRecipient, NoteScript, NoteTag, NoteType, @@ -195,7 +196,8 @@ mod tests { #[test] fn serialize_with_proof() { let note = create_example_note(); - let mock_inclusion_proof = NoteInclusionProof::new(0, 0, Default::default()).unwrap(); + let mock_inclusion_proof = + NoteInclusionProof::new(BlockNumber::from(0), 0, Default::default()).unwrap(); let file = NoteFile::NoteWithProof(note.clone(), mock_inclusion_proof.clone()); let mut buffer = Vec::new(); file.write_into(&mut buffer); diff --git a/objects/src/notes/location.rs b/objects/src/notes/location.rs index 242c269c0..a71f287c4 100644 --- a/objects/src/notes/location.rs +++ b/objects/src/notes/location.rs @@ -1,13 +1,16 @@ use super::{ ByteReader, ByteWriter, Deserializable, DeserializationError, NoteError, Serializable, }; -use crate::{crypto::merkle::MerklePath, MAX_BATCHES_PER_BLOCK, MAX_OUTPUT_NOTES_PER_BATCH}; +use crate::{ + block::BlockNumber, crypto::merkle::MerklePath, MAX_BATCHES_PER_BLOCK, + MAX_OUTPUT_NOTES_PER_BATCH, +}; /// Contains information about the location of a note. #[derive(Clone, Debug, PartialEq, Eq)] pub struct NoteLocation { /// The block number the note was created in. - block_num: u32, + block_num: BlockNumber, /// The index of the note in the note Merkle tree of the block the note was created in. node_index_in_block: u16, @@ -15,7 +18,7 @@ pub struct NoteLocation { impl NoteLocation { /// Returns the block number the note was created in. - pub fn block_num(&self) -> u32 { + pub fn block_num(&self) -> BlockNumber { self.block_num } @@ -43,7 +46,7 @@ pub struct NoteInclusionProof { impl NoteInclusionProof { /// Returns a new [NoteInclusionProof]. pub fn new( - block_num: u32, + block_num: BlockNumber, node_index_in_block: u16, note_path: MerklePath, ) -> Result { @@ -54,6 +57,7 @@ impl NoteInclusionProof { highest_index: HIGHEST_INDEX, }); } + let location = NoteLocation { block_num, node_index_in_block }; Ok(Self { location, note_path }) @@ -79,14 +83,14 @@ impl NoteInclusionProof { impl Serializable for NoteLocation { fn write_into(&self, target: &mut W) { - target.write_u32(self.block_num); + target.write(self.block_num); target.write_u16(self.node_index_in_block); } } impl Deserializable for NoteLocation { fn read_from(source: &mut R) -> Result { - let block_num = source.read_u32()?; + let block_num = source.read()?; let node_index_in_block = source.read_u16()?; Ok(Self { block_num, node_index_in_block }) diff --git a/objects/src/testing/block.rs b/objects/src/testing/block.rs index 5b3054e86..483bfebd6 100644 --- a/objects/src/testing/block.rs +++ b/objects/src/testing/block.rs @@ -6,7 +6,7 @@ use vm_processor::Digest; #[cfg(not(target_family = "wasm"))] use winter_rand_utils::{rand_array, rand_value}; -use crate::{accounts::Account, BlockHeader, ACCOUNT_TREE_DEPTH}; +use crate::{accounts::Account, block::BlockNumber, BlockHeader, ACCOUNT_TREE_DEPTH}; impl BlockHeader { /// Creates a mock block. The account tree is formed from the provided `accounts`, @@ -16,7 +16,7 @@ impl BlockHeader { /// For non-WASM targets, the remaining header values are initialized randomly. For WASM /// targets, values are initialized to [Default::default()] pub fn mock( - block_num: u32, + block_num: impl Into, chain_root: Option, note_root: Option, accounts: &[Account], @@ -67,7 +67,7 @@ impl BlockHeader { BlockHeader::new( 0, prev_hash, - block_num, + block_num.into(), chain_root, account_root, nullifier_root, diff --git a/objects/src/transaction/chain_mmr.rs b/objects/src/transaction/chain_mmr.rs index de0e1af61..427a84a84 100644 --- a/objects/src/transaction/chain_mmr.rs +++ b/objects/src/transaction/chain_mmr.rs @@ -3,6 +3,7 @@ use alloc::{collections::BTreeMap, vec::Vec}; use vm_core::utils::{Deserializable, Serializable}; use crate::{ + block::BlockNumber, crypto::merkle::{InnerNodeInfo, MmrPeaks, PartialMmr}, BlockHeader, ChainMmrError, }; @@ -27,7 +28,7 @@ pub struct ChainMmr { mmr: PartialMmr, /// A map of block_num |-> block_header for all blocks for which the partial MMR contains /// authentication paths. - blocks: BTreeMap, + blocks: BTreeMap, } impl ChainMmr { @@ -47,7 +48,7 @@ impl ChainMmr { let mut block_map = BTreeMap::new(); for block in blocks.into_iter() { - if block.block_num() as usize >= chain_length { + if block.block_num().as_usize() >= chain_length { return Err(ChainMmrError::block_num_too_big(chain_length, block.block_num())); } @@ -55,7 +56,7 @@ impl ChainMmr { return Err(ChainMmrError::duplicate_block(block.block_num())); } - if !mmr.is_tracked(block.block_num() as usize) { + if !mmr.is_tracked(block.block_num().as_usize()) { return Err(ChainMmrError::untracked_block(block.block_num())); } } @@ -72,18 +73,21 @@ impl ChainMmr { } /// Returns total number of blocks contain in the chain described by this MMR. - pub fn chain_length(&self) -> usize { - self.mmr.forest() + pub fn chain_length(&self) -> BlockNumber { + BlockNumber::from( + u32::try_from(self.mmr.forest()) + .expect("chain mmr should never contain more than u32::MAX blocks"), + ) } /// Returns true if the block is present in this chain MMR. - pub fn contains_block(&self, block_num: u32) -> bool { + pub fn contains_block(&self, block_num: BlockNumber) -> bool { self.blocks.contains_key(&block_num) } /// Returns the block header for the specified block, or None if the block is not present in /// this chain MMR. - pub fn get_block(&self, block_num: u32) -> Option<&BlockHeader> { + pub fn get_block(&self, block_num: BlockNumber) -> Option<&BlockHeader> { self.blocks.get(&block_num) } @@ -100,7 +104,7 @@ impl ChainMmr { /// Panics if the `block_header.block_num` is not equal to the current chain length (i.e., the /// provided block header is not the next block in the chain). pub fn add_block(&mut self, block_header: BlockHeader, track: bool) { - assert_eq!(block_header.block_num(), self.chain_length() as u32); + assert_eq!(block_header.block_num(), self.chain_length()); self.mmr.add(block_header.hash(), track); } @@ -111,7 +115,7 @@ impl ChainMmr { /// MMR. pub fn inner_nodes(&self) -> impl Iterator + '_ { self.mmr.inner_nodes( - self.blocks.values().map(|block| (block.block_num() as usize, block.hash())), + self.blocks.values().map(|block| (block.block_num().as_usize(), block.hash())), ) } } @@ -150,6 +154,7 @@ mod tests { use super::ChainMmr; use crate::{ alloc::vec::Vec, + block::BlockNumber, crypto::merkle::{Mmr, PartialMmr}, BlockHeader, Digest, }; @@ -216,11 +221,11 @@ mod tests { assert_eq!(chain_mmr, deserialized); } - fn int_to_block_header(block_num: u32) -> BlockHeader { + fn int_to_block_header(block_num: impl Into) -> BlockHeader { BlockHeader::new( 0, Digest::default(), - block_num, + block_num.into(), Digest::default(), Digest::default(), Digest::default(), diff --git a/objects/src/transaction/inputs.rs b/objects/src/transaction/inputs.rs index 33c16833f..b011cbc10 100644 --- a/objects/src/transaction/inputs.rs +++ b/objects/src/transaction/inputs.rs @@ -4,7 +4,7 @@ use core::fmt::Debug; use super::{BlockHeader, ChainMmr, Digest, Felt, Hasher, Word}; use crate::{ accounts::{Account, AccountId, AccountIdAnchor}, - block::block_num_from_epoch, + block::BlockNumber, notes::{Note, NoteId, NoteInclusionProof, NoteLocation, Nullifier}, utils::serde::{ByteReader, ByteWriter, Deserializable, DeserializationError, Serializable}, TransactionInputError, MAX_INPUT_NOTES_PER_TX, @@ -44,10 +44,10 @@ impl TransactionInputs { // check the block_chain and block_header are consistent let block_num = block_header.block_num(); - if block_chain.chain_length() != block_header.block_num() as usize { + if block_chain.chain_length() != block_header.block_num() { return Err(TransactionInputError::InconsistentChainLength { expected: block_header.block_num(), - actual: block_chain.chain_length() as u32, + actual: block_chain.chain_length(), }); } @@ -477,7 +477,7 @@ pub fn validate_account_seed( ) -> Result<(), TransactionInputError> { match (account.is_new(), account_seed) { (true, Some(seed)) => { - let anchor_block_number = block_num_from_epoch(account.id().anchor_epoch()); + let anchor_block_number = BlockNumber::from_epoch(account.id().anchor_epoch()); let anchor_block_hash = if block_header.block_num() == anchor_block_number { block_header.hash() From 4d5a7d8432c93a13b44314275cdb28623afb8791 Mon Sep 17 00:00:00 2001 From: Ignacio Amigo Date: Fri, 17 Jan 2025 15:13:59 -0300 Subject: [PATCH 6/9] chore: Merge fixes --- objects/src/errors.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/objects/src/errors.rs b/objects/src/errors.rs index 379d3baf9..523577e7a 100644 --- a/objects/src/errors.rs +++ b/objects/src/errors.rs @@ -16,14 +16,10 @@ use super::{ MAX_OUTPUT_NOTES_PER_BATCH, MAX_OUTPUT_NOTES_PER_BLOCK, }; use crate::{ -<<<<<<< HEAD accounts::{ AccountCode, AccountIdPrefix, AccountStorage, AccountType, PlaceholderType, StoragePlaceholder, }, -======= - accounts::{AccountCode, AccountIdPrefix, AccountStorage, AccountType, StoragePlaceholder}, ->>>>>>> 33263067959a14e3c230a2de9453bcee834075ae block::BlockNumber, notes::{NoteAssets, NoteExecutionHint, NoteTag, NoteType, Nullifier}, ACCOUNT_UPDATE_MAX_SIZE, MAX_INPUTS_PER_NOTE, MAX_INPUT_NOTES_PER_TX, MAX_OUTPUT_NOTES_PER_TX, From b230aebb44060cf4f8f07ea6340965d2d4d54d28 Mon Sep 17 00:00:00 2001 From: Ignacio Amigo Date: Sun, 19 Jan 2025 21:01:55 -0300 Subject: [PATCH 7/9] refactor: change inner type of StorageValue::Map --- .../component/template/storage/mod.rs | 7 ++- .../component/template/storage/placeholder.rs | 17 +++----- .../component/template/storage/value_type.rs | 43 +++++++++++-------- 3 files changed, 34 insertions(+), 33 deletions(-) diff --git a/objects/src/accounts/component/template/storage/mod.rs b/objects/src/accounts/component/template/storage/mod.rs index c45377330..36e51f504 100644 --- a/objects/src/accounts/component/template/storage/mod.rs +++ b/objects/src/accounts/component/template/storage/mod.rs @@ -348,14 +348,13 @@ mod tests { use assembly::Assembler; use assert_matches::assert_matches; use semver::Version; - use vm_core::{Felt, FieldElement, Word}; - use vm_processor::Digest; + use vm_core::{Felt, FieldElement}; use super::*; use crate::{ accounts::{ component::template::{AccountComponentMetadata, AccountComponentTemplate}, - AccountComponent, AccountType, + AccountComponent, AccountType, StorageMap, }, digest, testing::account_code::CODE, @@ -509,7 +508,7 @@ mod tests { ), ( StoragePlaceholder::new("map.template").unwrap(), - StorageValue::Map(vec![(Digest::default(), Word::default())]), + StorageValue::Map(StorageMap::default()), ), ]); diff --git a/objects/src/accounts/component/template/storage/placeholder.rs b/objects/src/accounts/component/template/storage/placeholder.rs index ae42c9878..aee15a64c 100644 --- a/objects/src/accounts/component/template/storage/placeholder.rs +++ b/objects/src/accounts/component/template/storage/placeholder.rs @@ -1,16 +1,13 @@ -use alloc::{ - string::{String, ToString}, - vec::Vec, -}; +use alloc::string::{String, ToString}; use thiserror::Error; use vm_core::{ utils::{ByteReader, ByteWriter, Deserializable, Serializable}, Felt, Word, }; -use vm_processor::{DeserializationError, Digest}; +use vm_processor::DeserializationError; -use crate::accounts::component::template::AccountComponentTemplateError; +use crate::accounts::{component::template::AccountComponentTemplateError, StorageMap}; // STORAGE PLACEHOLDER // ================================================================================================ @@ -164,14 +161,14 @@ impl Deserializable for StoragePlaceholder { /// A [StorageValue] can be one of: /// - `Felt(Felt)`: a single [Felt] value /// - `Word(Word)`: a single [Word] value -/// - `Map(Vec<(Digest, Word)>)`: a list of storage map entries, mapping [Digest] to [Word] +/// - `Map(StorageMap)`: a storage map entries that maps [Digest] to [Word] /// /// These values are used to resolve dynamic placeholders at component instantiation. #[derive(Clone, Debug)] pub enum StorageValue { Felt(Felt), Word(Word), - Map(Vec<(Digest, Word)>), + Map(StorageMap), } impl StorageValue { @@ -193,8 +190,8 @@ impl StorageValue { } } - /// Returns `Ok(&Vec<(Digest, Word)>>` if the variant is `Map`, otherwise errors. - pub fn as_map(&self) -> Result<&Vec<(Digest, Word)>, AccountComponentTemplateError> { + /// Returns `Ok(&StorageMap>` if the variant is `Map`, otherwise errors. + pub fn as_map(&self) -> Result<&StorageMap, AccountComponentTemplateError> { if let StorageValue::Map(map) = self { Ok(map) } else { diff --git a/objects/src/accounts/component/template/storage/value_type.rs b/objects/src/accounts/component/template/storage/value_type.rs index e9f4c9fc5..4730f41c7 100644 --- a/objects/src/accounts/component/template/storage/value_type.rs +++ b/objects/src/accounts/component/template/storage/value_type.rs @@ -323,15 +323,29 @@ impl MapRepresentation { &self, init_storage_data: &InitStorageData, ) -> Result { - let entries = match self { - MapRepresentation::List(vec) => vec - .iter() - .map(|map_entry| { - let key = map_entry.key().try_build_word(init_storage_data)?; - let value = map_entry.value().try_build_word(init_storage_data)?; - Ok((key.into(), value)) - }) - .collect::, _>>()?, + let map = match self { + MapRepresentation::List(vec) => { + let entries = vec + .iter() + .map(|map_entry| { + let key = map_entry.key().try_build_word(init_storage_data)?; + let value = map_entry.value().try_build_word(init_storage_data)?; + Ok((key.into(), value)) + }) + .collect::, _>>()?; + + // validate that no key appears multiple times + let mut seen_keys = BTreeSet::new(); + for (map_key, _map_value) in entries.iter() { + if !seen_keys.insert(map_key) { + return Err(AccountComponentTemplateError::StorageMapHasDuplicateKeys( + map_key.to_hex(), + )); + } + } + + StorageMap::with_entries(entries) + }, MapRepresentation::Template(storage_placeholder) => init_storage_data .get(storage_placeholder) .ok_or_else(|| { @@ -343,16 +357,7 @@ impl MapRepresentation { .cloned()?, }; - // validate that no key appears multiple times - let mut seen_keys = BTreeSet::new(); - for (map_key, _map_value) in entries.iter() { - if !seen_keys.insert(map_key) { - return Err(AccountComponentTemplateError::StorageMapHasDuplicateKeys( - map_key.to_hex(), - )); - } - } - Ok(StorageMap::with_entries(entries)) + Ok(map) } } From 07c5e278aaa68ec9a660b45e983875d33965a9f4 Mon Sep 17 00:00:00 2001 From: Ignacio Amigo Date: Sun, 19 Jan 2025 21:10:25 -0300 Subject: [PATCH 8/9] docs: Change description --- objects/src/accounts/component/template/storage/placeholder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/objects/src/accounts/component/template/storage/placeholder.rs b/objects/src/accounts/component/template/storage/placeholder.rs index aee15a64c..a7daf381e 100644 --- a/objects/src/accounts/component/template/storage/placeholder.rs +++ b/objects/src/accounts/component/template/storage/placeholder.rs @@ -161,7 +161,7 @@ impl Deserializable for StoragePlaceholder { /// A [StorageValue] can be one of: /// - `Felt(Felt)`: a single [Felt] value /// - `Word(Word)`: a single [Word] value -/// - `Map(StorageMap)`: a storage map entries that maps [Digest] to [Word] +/// - `Map(StorageMap)`: a storage map /// /// These values are used to resolve dynamic placeholders at component instantiation. #[derive(Clone, Debug)] From b7672e4ed849cb5b4101bbfdc0b54e0271786bb2 Mon Sep 17 00:00:00 2001 From: Ignacio Amigo Date: Mon, 20 Jan 2025 13:53:08 -0300 Subject: [PATCH 9/9] reviews: Docs and avoid allocation when formatting --- .../src/accounts/component/template/mod.rs | 11 ++++++---- .../component/template/storage/value_type.rs | 22 +++++++++++-------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/objects/src/accounts/component/template/mod.rs b/objects/src/accounts/component/template/mod.rs index 0f1c891fc..361bc06e3 100644 --- a/objects/src/accounts/component/template/mod.rs +++ b/objects/src/accounts/component/template/mod.rs @@ -98,7 +98,10 @@ impl Deserializable for AccountComponentTemplate { /// index 0. /// - Storage slots are laid out in a contiguous manner. /// - Storage placeholders can appear multiple times, but only if the expected [StorageValue] is of -/// the same type in all instances. +/// the same type in all instances. The expected placeholders can be retrieved with +/// [AccountComponentMetadata::get_unique_storage_placeholders()], which returns a map from +/// [StoragePlaceholder] to [PlaceholderType] (which, in turn, indicates the expected value type +/// for the placeholder). /// /// # Example /// @@ -193,7 +196,7 @@ impl AccountComponentMetadata { /// Retrieves a map of unique storage placeholders mapped to their expected type that require /// a value at the moment of component instantiation. These values will be used for - /// initializing storage slot values, or storage map entries.For a full example on how a + /// initializing storage slot values, or storage map entries. For a full example on how a /// placeholder may be utilized, please refer to the docs for [AccountComponentMetadata]. /// /// Types for the returned storage placeholders are inferred based on their location in the @@ -240,7 +243,7 @@ impl AccountComponentMetadata { /// # Errors /// /// - If the specified storage slots contain duplicates. - /// - If the template contains multiple storage placeholders with of different type. + /// - If the template contains multiple storage placeholders of different type. /// - If the slot numbers do not start at zero. /// - If the slots are not contiguous. fn validate(&self) -> Result<(), AccountComponentTemplateError> { @@ -299,7 +302,7 @@ impl AccountComponentMetadata { Ok(()) } - /// Validates map keys. + /// Validates map keys by checking for duplicates. /// Because keys can be represented in a variety of ways, the `to_string()` implementation is /// used to check for duplicates. fn validate_map_keys(&self) -> Result<(), AccountComponentTemplateError> { diff --git a/objects/src/accounts/component/template/storage/value_type.rs b/objects/src/accounts/component/template/storage/value_type.rs index 4730f41c7..efa43e596 100644 --- a/objects/src/accounts/component/template/storage/value_type.rs +++ b/objects/src/accounts/component/template/storage/value_type.rs @@ -1,4 +1,4 @@ -use alloc::{boxed::Box, collections::BTreeSet, string::ToString, vec::Vec}; +use alloc::{boxed::Box, collections::BTreeSet, vec::Vec}; use vm_core::{ utils::{ByteReader, ByteWriter, Deserializable, Serializable}, @@ -138,15 +138,15 @@ impl core::fmt::Display for WordRepresentation { WordRepresentation::Value(hex) => f.write_str(&Digest::from(hex).to_hex()), WordRepresentation::Array(array) => { f.write_str("[")?; - f.write_str(&format!("{}, ", array[0]))?; - f.write_str(&format!("{}, ", array[1]))?; - f.write_str(&format!("{}, ", array[2]))?; - f.write_str(&format!("{}, ", array[3]))?; + f.write_fmt(format_args!("{}, ", array[0]))?; + f.write_fmt(format_args!("{}, ", array[1]))?; + f.write_fmt(format_args!("{}, ", array[2]))?; + f.write_fmt(format_args!("{}, ", array[3]))?; f.write_str("]") }, WordRepresentation::Template(storage_placeholder) => { - f.write_str(&storage_placeholder.to_string()) + f.write_fmt(format_args!("{}", storage_placeholder)) }, } } @@ -259,10 +259,14 @@ impl Deserializable for FeltRepresentation { impl core::fmt::Display for FeltRepresentation { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { match self { - FeltRepresentation::Hexadecimal(base_element) => f.write_str(&base_element.to_string()), - FeltRepresentation::Decimal(base_element) => f.write_str(&base_element.to_string()), + FeltRepresentation::Hexadecimal(base_element) => { + f.write_fmt(format_args!("{}", base_element)) + }, + FeltRepresentation::Decimal(base_element) => { + f.write_fmt(format_args!("{}", base_element)) + }, FeltRepresentation::Template(storage_placeholder) => { - f.write_str(&storage_placeholder.to_string()) + f.write_fmt(format_args!("{}", storage_placeholder)) }, } }