From 43a72a58bd0b5e6b3dc63de551ccb07794a77539 Mon Sep 17 00:00:00 2001 From: "Augusto F. Hack" Date: Fri, 7 Jun 2024 15:33:07 +0200 Subject: [PATCH 01/15] reuse the SLOT_LAYOUT_COMMITMENT_INDEX constant --- objects/src/testing/account.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/objects/src/testing/account.rs b/objects/src/testing/account.rs index 3dcd5fad5..34392b629 100644 --- a/objects/src/testing/account.rs +++ b/objects/src/testing/account.rs @@ -134,7 +134,7 @@ impl AccountBuilder { let inner_storage = self.storage_builder.build(); for (key, value) in inner_storage.slots().leaves() { - if key != 255 { + if key != AccountStorage::SLOT_LAYOUT_COMMITMENT_INDEX.into() { // don't copy the reserved key storage.set_item(key as u8, *value).map_err(AccountBuilderError::AccountError)?; } From bc8c6f73fbeb58e084b6acb2051e908ae7933995 Mon Sep 17 00:00:00 2001 From: "Augusto F. Hack" Date: Fri, 7 Jun 2024 15:39:28 +0200 Subject: [PATCH 02/15] type to -> too --- objects/src/accounts/storage/mod.rs | 6 +++--- objects/src/errors.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/objects/src/accounts/storage/mod.rs b/objects/src/accounts/storage/mod.rs index 48f24d281..412303ce4 100644 --- a/objects/src/accounts/storage/mod.rs +++ b/objects/src/accounts/storage/mod.rs @@ -160,9 +160,9 @@ impl AccountStorage { .count(); if maps.len() > count { - return Err(AccountError::StorageMapToManyMaps { - expected: (count), - actual: (maps.len()), + return Err(AccountError::StorageMapTooManyMaps { + expected: count, + actual: maps.len(), }); } diff --git a/objects/src/errors.rs b/objects/src/errors.rs index 2e33285c1..962127276 100644 --- a/objects/src/errors.rs +++ b/objects/src/errors.rs @@ -34,7 +34,7 @@ pub enum AccountError { StorageSlotInvalidValueArity { slot: u8, expected: u8, actual: u8 }, StorageSlotIsReserved(u8), StorageSlotNotValueSlot(u8, StorageSlotType), - StorageMapToManyMaps { expected: usize, actual: usize }, + StorageMapTooManyMaps { expected: usize, actual: usize }, StubDataIncorrectLength(usize, usize), } From a85cb20ffa9574a9622a52af8182696a1b69eaee Mon Sep 17 00:00:00 2001 From: "Augusto F. Hack" Date: Fri, 7 Jun 2024 16:01:56 +0200 Subject: [PATCH 03/15] add api to create SlotItem --- bench-tx/src/utils.rs | 12 +-- miden-lib/src/accounts/faucets/mod.rs | 12 +-- miden-lib/src/accounts/wallets/mod.rs | 12 +-- miden-tx/src/kernel_tests/test_account.rs | 20 +++-- miden-tx/tests/integration/main.rs | 12 +-- miden-tx/tests/integration/scripts/faucet.rs | 12 +-- miden-tx/tests/integration/wallet/mod.rs | 22 ++---- objects/src/accounts/storage/mod.rs | 82 ++++++++++---------- objects/src/testing/account.rs | 10 ++- objects/src/testing/storage.rs | 45 ++--------- 10 files changed, 89 insertions(+), 150 deletions(-) diff --git a/bench-tx/src/utils.rs b/bench-tx/src/utils.rs index 3af247223..cdcfdf142 100644 --- a/bench-tx/src/utils.rs +++ b/bench-tx/src/utils.rs @@ -3,7 +3,7 @@ pub use alloc::collections::BTreeMap; use miden_lib::transaction::TransactionKernel; use miden_objects::{ - accounts::{Account, AccountCode, AccountId, AccountStorage, SlotItem, StorageSlot}, + accounts::{Account, AccountCode, AccountId, AccountStorage, SlotItem}, assembly::ModuleAst, assets::{Asset, AssetVault}, Felt, Word, @@ -88,14 +88,8 @@ pub fn get_account_with_default_account_code( let account_assembler = TransactionKernel::assembler(); let account_code = AccountCode::new(account_code_ast.clone(), &account_assembler).unwrap(); - let account_storage = AccountStorage::new( - vec![SlotItem { - index: 0, - slot: StorageSlot::new_value(public_key), - }], - vec![], - ) - .unwrap(); + let account_storage = + AccountStorage::new(vec![SlotItem::new_value(0, 0, public_key)], vec![]).unwrap(); let account_vault = match assets { Some(asset) => AssetVault::new(&[asset]).unwrap(), diff --git a/miden-lib/src/accounts/faucets/mod.rs b/miden-lib/src/accounts/faucets/mod.rs index 6a48a43e2..3090e61ff 100644 --- a/miden-lib/src/accounts/faucets/mod.rs +++ b/miden-lib/src/accounts/faucets/mod.rs @@ -3,7 +3,6 @@ use alloc::string::ToString; use miden_objects::{ accounts::{ Account, AccountCode, AccountId, AccountStorage, AccountStorageType, AccountType, SlotItem, - StorageSlot, }, assembly::LibraryPath, assets::TokenSymbol, @@ -72,16 +71,7 @@ pub fn create_basic_fungible_faucet( // - slot 0: authentication data // - slot 1: token metadata as [max_supply, decimals, token_symbol, 0] let account_storage = AccountStorage::new( - vec![ - SlotItem { - index: 0, - slot: StorageSlot::new_value(auth_data), - }, - SlotItem { - index: 1, - slot: StorageSlot::new_value(metadata), - }, - ], + vec![SlotItem::new_value(0, 0, auth_data), SlotItem::new_value(1, 0, metadata)], vec![], )?; diff --git a/miden-lib/src/accounts/wallets/mod.rs b/miden-lib/src/accounts/wallets/mod.rs index ded42eab2..c1f8d0844 100644 --- a/miden-lib/src/accounts/wallets/mod.rs +++ b/miden-lib/src/accounts/wallets/mod.rs @@ -2,8 +2,7 @@ use alloc::string::{String, ToString}; use miden_objects::{ accounts::{ - Account, AccountCode, AccountId, AccountStorage, AccountStorageType, AccountType, - StorageSlot, + Account, AccountCode, AccountId, AccountStorage, AccountStorageType, AccountType, SlotItem, }, assembly::ModuleAst, AccountError, Word, @@ -58,13 +57,8 @@ pub fn create_basic_wallet( let account_assembler = TransactionKernel::assembler(); let account_code = AccountCode::new(account_code_ast.clone(), &account_assembler)?; - let account_storage = AccountStorage::new( - vec![miden_objects::accounts::SlotItem { - index: 0, - slot: StorageSlot::new_value(storage_slot_0_data), - }], - vec![], - )?; + let account_storage = + AccountStorage::new(vec![SlotItem::new_value(0, 0, storage_slot_0_data)], vec![])?; let account_seed = AccountId::get_account_seed( init_seed, diff --git a/miden-tx/src/kernel_tests/test_account.rs b/miden-tx/src/kernel_tests/test_account.rs index 43e69b129..8a7853cd8 100644 --- a/miden-tx/src/kernel_tests/test_account.rs +++ b/miden-tx/src/kernel_tests/test_account.rs @@ -10,7 +10,7 @@ use miden_objects::{ ACCOUNT_ID_REGULAR_ACCOUNT_IMMUTABLE_CODE_ON_CHAIN, ACCOUNT_ID_REGULAR_ACCOUNT_UPDATABLE_CODE_OFF_CHAIN, }, - AccountId, AccountType, StorageSlotType, + AccountId, AccountType, SlotItem, StorageSlotType, }, crypto::{hash::rpo::RpoDigest, merkle::LeafIndex}, testing::{ @@ -18,7 +18,8 @@ use miden_objects::{ notes::AssetPreservationStatus, prepare_word, storage::{ - storage_item_0, storage_item_1, storage_item_2, storage_map_2, STORAGE_LEAVES_2, + storage_map_2, STORAGE_INDEX_0, STORAGE_INDEX_1, STORAGE_INDEX_2, STORAGE_LEAVES_2, + STORAGE_VALUE_0, STORAGE_VALUE_1, }, }, }; @@ -246,7 +247,10 @@ fn test_is_faucet_procedure() { #[test] fn test_get_item() { - for storage_item in [storage_item_0(), storage_item_1()] { + for storage_item in [ + SlotItem::new_value(STORAGE_INDEX_0, 0, STORAGE_VALUE_0), + SlotItem::new_value(STORAGE_INDEX_1, 0, STORAGE_VALUE_1), + ] { let (tx_inputs, tx_args) = mock_inputs(MockAccountType::StandardExisting, AssetPreservationStatus::Preserved); @@ -335,7 +339,11 @@ fn test_set_item() { // Test different account storage types #[test] fn test_get_storage_data_type() { - for storage_item in [storage_item_0(), storage_item_1(), storage_item_2()] { + for storage_item in [ + SlotItem::new_value(STORAGE_INDEX_0, 0, STORAGE_VALUE_0), + SlotItem::new_value(STORAGE_INDEX_1, 0, STORAGE_VALUE_1), + SlotItem::new_map(STORAGE_INDEX_2, 0, storage_map_2().root().into()), + ] { let (tx_inputs, tx_args) = mock_inputs(MockAccountType::StandardExisting, AssetPreservationStatus::Preserved); @@ -385,7 +393,7 @@ fn test_get_map_item() { let (tx_inputs, tx_args) = mock_inputs(MockAccountType::StandardExisting, AssetPreservationStatus::Preserved); - let storage_item = storage_item_2(); + let storage_item = SlotItem::new_map(STORAGE_INDEX_2, 0, storage_map_2().root().into()); for (key, value) in STORAGE_LEAVES_2 { let code = format!( " @@ -433,7 +441,7 @@ fn test_set_map_item() { let (tx_inputs, tx_args) = mock_inputs(MockAccountType::StandardExisting, AssetPreservationStatus::Preserved); - let storage_item = storage_item_2(); + let storage_item = SlotItem::new_map(STORAGE_INDEX_2, 0, storage_map_2().root().into()); let code = format!( " diff --git a/miden-tx/tests/integration/main.rs b/miden-tx/tests/integration/main.rs index afa222ec2..f02d2f67a 100644 --- a/miden-tx/tests/integration/main.rs +++ b/miden-tx/tests/integration/main.rs @@ -5,7 +5,7 @@ use miden_lib::transaction::TransactionKernel; use miden_objects::{ accounts::{ account_id::testing::ACCOUNT_ID_SENDER, Account, AccountCode, AccountId, AccountStorage, - SlotItem, StorageSlot, + SlotItem, }, assembly::{ModuleAst, ProgramAst}, assets::{Asset, AssetVault, FungibleAsset}, @@ -78,14 +78,8 @@ pub fn get_account_with_default_account_code( let account_assembler = TransactionKernel::assembler(); let account_code = AccountCode::new(account_code_ast.clone(), &account_assembler).unwrap(); - let account_storage = AccountStorage::new( - vec![SlotItem { - index: 0, - slot: StorageSlot::new_value(public_key), - }], - vec![], - ) - .unwrap(); + let account_storage = + AccountStorage::new(vec![SlotItem::new_value(0, 0, public_key)], vec![]).unwrap(); let account_vault = match assets { Some(asset) => AssetVault::new(&[asset]).unwrap(), diff --git a/miden-tx/tests/integration/scripts/faucet.rs b/miden-tx/tests/integration/scripts/faucet.rs index 6f8394b05..8aec3a846 100644 --- a/miden-tx/tests/integration/scripts/faucet.rs +++ b/miden-tx/tests/integration/scripts/faucet.rs @@ -8,7 +8,7 @@ use miden_lib::{ use miden_objects::{ accounts::{ account_id::testing::ACCOUNT_ID_FUNGIBLE_FAUCET_OFF_CHAIN, Account, AccountCode, AccountId, - AccountStorage, AccountStorageType, SlotItem, StorageSlot, + AccountStorage, AccountStorageType, SlotItem, }, assembly::{ModuleAst, ProgramAst}, assets::{Asset, AssetVault, FungibleAsset, TokenSymbol}, @@ -294,14 +294,8 @@ fn get_faucet_account_with_max_supply_and_total_issuance( let faucet_storage_slot_1 = [Felt::new(max_supply), Felt::new(0), Felt::new(0), Felt::new(0)]; let mut faucet_account_storage = AccountStorage::new( vec![ - SlotItem { - index: 0, - slot: StorageSlot::new_value(public_key), - }, - SlotItem { - index: 1, - slot: StorageSlot::new_value(faucet_storage_slot_1), - }, + SlotItem::new_value(0, 0, public_key), + SlotItem::new_value(1, 0, faucet_storage_slot_1), ], vec![], ) diff --git a/miden-tx/tests/integration/wallet/mod.rs b/miden-tx/tests/integration/wallet/mod.rs index f8f24e68b..58db9cd39 100644 --- a/miden-tx/tests/integration/wallet/mod.rs +++ b/miden-tx/tests/integration/wallet/mod.rs @@ -5,7 +5,7 @@ use miden_objects::{ ACCOUNT_ID_FUNGIBLE_FAUCET_ON_CHAIN, ACCOUNT_ID_OFF_CHAIN_SENDER, ACCOUNT_ID_REGULAR_ACCOUNT_UPDATABLE_CODE_OFF_CHAIN, }, - Account, AccountId, AccountStorage, SlotItem, StorageSlot, + Account, AccountId, AccountStorage, SlotItem, }, assembly::ProgramAst, assets::{Asset, AssetVault, FungibleAsset}, @@ -85,14 +85,8 @@ fn prove_receive_asset_via_wallet() { assert_eq!(executed_transaction.account_delta().nonce(), Some(Felt::new(2))); // clone account info - let account_storage = AccountStorage::new( - vec![SlotItem { - index: 0, - slot: StorageSlot::new_value(target_pub_key), - }], - vec![], - ) - .unwrap(); + let account_storage = + AccountStorage::new(vec![SlotItem::new_value(0, 0, target_pub_key)], vec![]).unwrap(); let account_code = target_account.code().clone(); // vault delta let target_account_after: Account = Account::from_parts( @@ -167,14 +161,8 @@ fn prove_send_asset_via_wallet() { assert!(prove_and_verify_transaction(executed_transaction.clone()).is_ok()); // clones account info - let sender_account_storage = AccountStorage::new( - vec![SlotItem { - index: 0, - slot: StorageSlot::new_value(sender_pub_key), - }], - vec![], - ) - .unwrap(); + let sender_account_storage = + AccountStorage::new(vec![SlotItem::new_value(0, 0, sender_pub_key)], vec![]).unwrap(); let sender_account_code = sender_account.code().clone(); // vault delta diff --git a/objects/src/accounts/storage/mod.rs b/objects/src/accounts/storage/mod.rs index 412303ce4..482cb78e4 100644 --- a/objects/src/accounts/storage/mod.rs +++ b/objects/src/accounts/storage/mod.rs @@ -32,6 +32,41 @@ pub struct SlotItem { pub slot: StorageSlot, } +impl SlotItem { + /// Returns a new [SlotItem] with the [StorageSlotType::Value] type. + pub fn new_value(index: u8, arity: u8, value: Word) -> Self { + Self { + index, + slot: StorageSlot { + slot_type: StorageSlotType::Value { value_arity: arity }, + value, + }, + } + } + + /// Returns a new [SlotItem] with the [StorageSlotType::Map] type. + pub fn new_map(index: u8, arity: u8, value: Word) -> Self { + Self { + index, + slot: StorageSlot { + slot_type: StorageSlotType::Map { value_arity: arity }, + value, + }, + } + } + + /// Returns a new [SlotItem] with the [StorageSlotType::Array] type. + pub fn new_array(index: u8, arity: u8, depth: u8, value: Word) -> Self { + Self { + index, + slot: StorageSlot { + slot_type: StorageSlotType::Array { depth, value_arity: arity }, + value, + }, + } + } +} + /// Represents a single storage slot entry. #[derive(Debug, Clone)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] @@ -360,10 +395,7 @@ mod tests { use miden_crypto::hash::rpo::RpoDigest; - use super::{ - AccountStorage, Deserializable, Felt, Serializable, SlotItem, StorageMap, StorageSlot, - StorageSlotType, Word, - }; + use super::{AccountStorage, Deserializable, Felt, Serializable, SlotItem, StorageMap, Word}; use crate::{ONE, ZERO}; #[test] @@ -376,20 +408,8 @@ mod tests { // storage with values for default types let storage = AccountStorage::new( vec![ - SlotItem { - index: 0, - slot: StorageSlot { - slot_type: StorageSlotType::default(), - value: [ONE, ONE, ONE, ONE], - }, - }, - SlotItem { - index: 2, - slot: StorageSlot { - slot_type: StorageSlotType::default(), - value: [ONE, ONE, ONE, ZERO], - }, - }, + SlotItem::new_value(0, 0, [ONE, ONE, ONE, ONE]), + SlotItem::new_value(2, 0, [ONE, ONE, ONE, ZERO]), ], vec![], ) @@ -411,28 +431,10 @@ mod tests { let storage_map = StorageMap::with_entries(storage_map_leaves_2).unwrap(); let storage = AccountStorage::new( vec![ - SlotItem { - index: 0, - slot: StorageSlot { - slot_type: StorageSlotType::Value { value_arity: 1 }, - value: [ONE, ONE, ONE, ONE], - }, - }, - SlotItem { - index: 1, - slot: StorageSlot::new_value([ONE, ONE, ONE, ZERO]), - }, - SlotItem { - index: 2, - slot: StorageSlot::new_map(Word::from(storage_map.root())), - }, - SlotItem { - index: 3, - slot: StorageSlot { - slot_type: StorageSlotType::Array { depth: 4, value_arity: 3 }, - value: [ONE, ZERO, ZERO, ZERO], - }, - }, + SlotItem::new_value(0, 1, [ONE, ONE, ONE, ONE]), + SlotItem::new_value(1, 0, [ONE, ONE, ONE, ZERO]), + SlotItem::new_map(2, 0, storage_map.root().into()), + SlotItem::new_array(3, 3, 4, [ONE, ZERO, ZERO, ZERO]), ], vec![storage_map], ) diff --git a/objects/src/testing/account.rs b/objects/src/testing/account.rs index 34392b629..e5d543b97 100644 --- a/objects/src/testing/account.rs +++ b/objects/src/testing/account.rs @@ -14,8 +14,8 @@ use super::{ assets::non_fungible_asset, constants::FUNGIBLE_ASSET_AMOUNT, storage::{ - generate_account_seed, storage_item_0, storage_item_1, storage_item_2, storage_map_2, - AccountSeedType, AccountStorageBuilder, + generate_account_seed, storage_map_2, AccountSeedType, AccountStorageBuilder, + STORAGE_INDEX_0, STORAGE_INDEX_1, STORAGE_INDEX_2, STORAGE_VALUE_0, STORAGE_VALUE_1, }, }; use crate::{ @@ -235,7 +235,11 @@ pub fn mock_account_vault() -> AssetVault { pub fn mock_account_storage() -> AccountStorage { // create account storage AccountStorage::new( - vec![storage_item_0(), storage_item_1(), storage_item_2()], + vec![ + SlotItem::new_value(STORAGE_INDEX_0, 0, STORAGE_VALUE_0), + SlotItem::new_value(STORAGE_INDEX_1, 0, STORAGE_VALUE_1), + SlotItem::new_map(STORAGE_INDEX_2, 0, storage_map_2().root().into()), + ], vec![storage_map_2()], ) .unwrap() diff --git a/objects/src/testing/storage.rs b/objects/src/testing/storage.rs index 31580df3a..b281870e3 100644 --- a/objects/src/testing/storage.rs +++ b/objects/src/testing/storage.rs @@ -21,7 +21,7 @@ use crate::{ code::testing::make_account_code, get_account_seed_single, Account, AccountDelta, AccountId, AccountStorage, AccountStorageDelta, AccountStorageType, AccountType, AccountVaultDelta, SlotItem, - StorageMap, StorageSlot, StorageSlotType, + StorageMap, }, assets::{Asset, AssetVault, FungibleAsset}, notes::NoteAssets, @@ -91,31 +91,10 @@ pub const STORAGE_LEAVES_2: [(Digest, Word); 2] = [ ), ]; -pub fn storage_item_0() -> SlotItem { - SlotItem { - index: STORAGE_INDEX_0, - slot: StorageSlot::new_value(STORAGE_VALUE_0), - } -} - -pub fn storage_item_1() -> SlotItem { - SlotItem { - index: STORAGE_INDEX_1, - slot: StorageSlot::new_value(STORAGE_VALUE_1), - } -} - pub fn storage_map_2() -> StorageMap { StorageMap::with_entries(STORAGE_LEAVES_2).unwrap() } -pub fn storage_item_2() -> SlotItem { - SlotItem { - index: STORAGE_INDEX_2, - slot: StorageSlot::new_map(Word::from(storage_map_2().root())), - } -} - // MOCK FAUCET // ================================================================================================ @@ -131,10 +110,11 @@ pub fn mock_fungible_faucet( Felt::new(FUNGIBLE_FAUCET_INITIAL_BALANCE) }; let account_storage = AccountStorage::new( - vec![SlotItem { - index: FAUCET_STORAGE_DATA_SLOT, - slot: StorageSlot::new_value([ZERO, ZERO, ZERO, initial_balance]), - }], + vec![SlotItem::new_value( + FAUCET_STORAGE_DATA_SLOT, + 0, + [ZERO, ZERO, ZERO, initial_balance], + )], vec![], ) .unwrap(); @@ -163,10 +143,7 @@ pub fn mock_non_fungible_faucet( // TODO: add nft tree data to account storage? let account_storage = AccountStorage::new( - vec![SlotItem { - index: FAUCET_STORAGE_DATA_SLOT, - slot: StorageSlot::new_map(*nft_tree.root()), - }], + vec![SlotItem::new_map(FAUCET_STORAGE_DATA_SLOT, 0, *nft_tree.root())], vec![], ) .unwrap(); @@ -270,17 +247,11 @@ pub fn build_account(assets: Vec, nonce: Felt, storage_items: Vec) let id = AccountId::try_from(ACCOUNT_ID_REGULAR_ACCOUNT_IMMUTABLE_CODE_ON_CHAIN).unwrap(); let code = make_account_code(); - // build account data let vault = AssetVault::new(&assets).unwrap(); - - let slot_type = StorageSlotType::Value { value_arity: 0 }; let slot_items: Vec = storage_items .into_iter() .enumerate() - .map(|(index, item)| SlotItem { - index: index as u8, - slot: StorageSlot { slot_type, value: item }, - }) + .map(|(index, item)| SlotItem::new_value(index as u8, 0, item)) .collect(); let storage = AccountStorage::new(slot_items, vec![]).unwrap(); From 75350100f2e050abe3f15f2805c2a8031652a603 Mon Sep 17 00:00:00 2001 From: "Augusto F. Hack" Date: Fri, 7 Jun 2024 16:20:17 +0200 Subject: [PATCH 04/15] replaced castings in favor of conversions --- objects/src/accounts/storage/mod.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/objects/src/accounts/storage/mod.rs b/objects/src/accounts/storage/mod.rs index 482cb78e4..326110715 100644 --- a/objects/src/accounts/storage/mod.rs +++ b/objects/src/accounts/storage/mod.rs @@ -162,7 +162,7 @@ impl AccountStorage { let mut layout = vec![StorageSlotType::default(); Self::NUM_STORAGE_SLOTS]; // set the slot type for the layout commitment - layout[Self::SLOT_LAYOUT_COMMITMENT_INDEX as usize] = + layout[usize::from(Self::SLOT_LAYOUT_COMMITMENT_INDEX)] = StorageSlotType::Value { value_arity: 64 }; // process entries to extract type data @@ -173,14 +173,14 @@ impl AccountStorage { return Err(AccountError::StorageSlotIsReserved(item.index)); } - layout[item.index as usize] = item.slot.slot_type; - Ok((item.index as u64, item.slot.value)) + layout[usize::from(item.index)] = item.slot.slot_type; + Ok((item.index.into(), item.slot.value)) }) .collect::, AccountError>>()?; // add layout commitment entry entries.push(( - Self::SLOT_LAYOUT_COMMITMENT_INDEX as u64, + Self::SLOT_LAYOUT_COMMITMENT_INDEX.into(), *Hasher::hash_elements(&layout.iter().map(Felt::from).collect::>()), )); @@ -216,7 +216,7 @@ impl AccountStorage { /// /// If the item is not present in the storage, [ZERO; 4] is returned. pub fn get_item(&self, index: u8) -> Digest { - let item_index = NodeIndex::new(Self::STORAGE_TREE_DEPTH, index as u64) + let item_index = NodeIndex::new(Self::STORAGE_TREE_DEPTH, index.into()) .expect("index is u8 - index within range"); self.slots.get_node(item_index).expect("index is u8 - index within range") } @@ -278,7 +278,7 @@ impl AccountStorage { } // only value slots of basic arity can currently be updated - match self.layout[index as usize] { + match self.layout[usize::from(index)] { StorageSlotType::Value { value_arity } => { if value_arity > 0 { return Err(AccountError::StorageSlotInvalidValueArity { @@ -292,7 +292,7 @@ impl AccountStorage { } // update the slot and return - let index = LeafIndex::new(index as u64).expect("index is u8 - index within range"); + let index = LeafIndex::new(index.into()).expect("index is u8 - index within range"); let slot_value = self.slots.insert(index, value); Ok(slot_value) } From 2e0b7fde5a54f4ed08efa82a61ce6d86bdd34fb6 Mon Sep 17 00:00:00 2001 From: "Augusto F. Hack" Date: Fri, 7 Jun 2024 16:32:28 +0200 Subject: [PATCH 05/15] add serde for StorageSlotType --- objects/src/accounts/storage/mod.rs | 37 +++++++++++----------------- objects/src/accounts/storage/slot.rs | 21 ++++++++++++++++ 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/objects/src/accounts/storage/mod.rs b/objects/src/accounts/storage/mod.rs index 326110715..9c246e8cf 100644 --- a/objects/src/accounts/storage/mod.rs +++ b/objects/src/accounts/storage/mod.rs @@ -45,23 +45,25 @@ impl SlotItem { } /// Returns a new [SlotItem] with the [StorageSlotType::Map] type. - pub fn new_map(index: u8, arity: u8, value: Word) -> Self { + pub fn new_map(index: u8, arity: u8, root: Word) -> Self { Self { index, slot: StorageSlot { slot_type: StorageSlotType::Map { value_arity: arity }, - value, + value: root, }, } } /// Returns a new [SlotItem] with the [StorageSlotType::Array] type. - pub fn new_array(index: u8, arity: u8, depth: u8, value: Word) -> Self { + /// + /// The max size of the array is set to 2^log_n and the value arity for the slot is set to 0. + pub fn new_array(index: u8, arity: u8, log_n: u8, root: Word) -> Self { Self { index, slot: StorageSlot { - slot_type: StorageSlotType::Array { depth, value_arity: arity }, - value, + slot_type: StorageSlotType::Array { depth: log_n, value_arity: arity }, + value: root, }, } } @@ -303,20 +305,16 @@ impl AccountStorage { impl Serializable for AccountStorage { fn write_into(&self, target: &mut W) { - // serialize layout info; we don't serialize default type info as we'll assume that any - // slot type that wasn't serialized was a default slot type. also we skip the last slot - // type as it is a constant. - let complex_types = self.layout[..255] + // don't serialize last slot as it is a constant. + let complex_types = self.layout[..usize::from(AccountStorage::SLOT_LAYOUT_COMMITMENT_INDEX)] .iter() .enumerate() + // don't serialize default types, these are implied. .filter(|(_, slot_type)| !slot_type.is_default()) + .map(|(index, slot_type)| (u8::try_from(index).expect("Number of slot types is limited to u8"), slot_type)) .collect::>(); - target.write_u8(complex_types.len() as u8); - for (idx, slot_type) in complex_types { - target.write_u8(idx as u8); - target.write_u16(slot_type.into()); - } + complex_types.write_into(target); // serialize slot values; we serialize only non-empty values and also skip slot 255 as info // for this slot was already serialized as a part of serializing slot type info above @@ -348,15 +346,8 @@ impl Serializable for AccountStorage { impl Deserializable for AccountStorage { fn read_from(source: &mut R) -> Result { - // read complex types - let mut complex_types = BTreeMap::new(); - let num_complex_types = source.read_u8()?; - for _ in 0..num_complex_types { - let idx = source.read_u8()?; - let slot_type: StorageSlotType = - source.read_u16()?.try_into().map_err(DeserializationError::InvalidValue)?; - complex_types.insert(idx, slot_type); - } + let complex_types = >::read_from(source)?; + let mut complex_types = BTreeMap::from_iter(complex_types); // read filled slots and build a vector of slot items let mut items: Vec = Vec::new(); diff --git a/objects/src/accounts/storage/slot.rs b/objects/src/accounts/storage/slot.rs index 4ecae846c..cbb6f64c0 100644 --- a/objects/src/accounts/storage/slot.rs +++ b/objects/src/accounts/storage/slot.rs @@ -1,5 +1,8 @@ use alloc::string::{String, ToString}; +use vm_core::utils::{Deserializable, Serializable}; +use vm_processor::DeserializationError; + use super::Felt; // CONSTANTS @@ -139,3 +142,21 @@ impl From<&StorageSlotType> for Felt { Self::from(*value) } } + +// SERIALIZATION +// ================================================================================================ + +impl Serializable for StorageSlotType { + fn write_into(&self, target: &mut W) { + target.write_u16(self.into()); + } +} + +impl Deserializable for StorageSlotType { + fn read_from( + source: &mut R, + ) -> Result { + let encoded = source.read_u16()?; + StorageSlotType::try_from(encoded).map_err(DeserializationError::InvalidValue) + } +} From 824715f4663ff4763edf260054dbcf311af7d04a Mon Sep 17 00:00:00 2001 From: "Augusto F. Hack" Date: Fri, 7 Jun 2024 17:34:52 +0200 Subject: [PATCH 06/15] add comments to AccountStorage::new --- objects/src/accounts/storage/mod.rs | 54 +++++++++++++++-------------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/objects/src/accounts/storage/mod.rs b/objects/src/accounts/storage/mod.rs index 9c246e8cf..3be8d6e7b 100644 --- a/objects/src/accounts/storage/mod.rs +++ b/objects/src/accounts/storage/mod.rs @@ -160,29 +160,37 @@ impl AccountStorage { items: Vec, maps: Vec, ) -> Result { - // initialize storage layout - let mut layout = vec![StorageSlotType::default(); Self::NUM_STORAGE_SLOTS]; - - // set the slot type for the layout commitment - layout[usize::from(Self::SLOT_LAYOUT_COMMITMENT_INDEX)] = + // Empty layout + let mut layout = vec![StorageSlotType::default(); AccountStorage::NUM_STORAGE_SLOTS]; + layout[usize::from(AccountStorage::SLOT_LAYOUT_COMMITMENT_INDEX)] = StorageSlotType::Value { value_arity: 64 }; - // process entries to extract type data - let mut entries = items - .into_iter() - .map(|item| { - if item.index == Self::SLOT_LAYOUT_COMMITMENT_INDEX { - return Err(AccountError::StorageSlotIsReserved(item.index)); - } - - layout[usize::from(item.index)] = item.slot.slot_type; - Ok((item.index.into(), item.slot.value)) - }) - .collect::, AccountError>>()?; + // The following loop will: + // + // - Validate the slot and check it doesn't assign a value to a reserved slot. + // - Extract the slot value. + // - Count the number of maps to validate `maps`. + // + // It won't detect duplicates, that is later done by the `SimpleSmt` instantiation. + // + let mut entries = Vec::with_capacity(AccountStorage::NUM_STORAGE_SLOTS); + let mut num_maps = 0; + for item in items { + if item.index == AccountStorage::SLOT_LAYOUT_COMMITMENT_INDEX { + return Err(AccountError::StorageSlotIsReserved(item.index)); + } + + if matches!(item.slot.slot_type, StorageSlotType::Map { .. }) { + num_maps += 1; + } + + layout[usize::from(item.index)] = item.slot.slot_type; + entries.push((item.index.into(), item.slot.value)) + } // add layout commitment entry entries.push(( - Self::SLOT_LAYOUT_COMMITMENT_INDEX.into(), + AccountStorage::SLOT_LAYOUT_COMMITMENT_INDEX.into(), *Hasher::hash_elements(&layout.iter().map(Felt::from).collect::>()), )); @@ -190,15 +198,9 @@ impl AccountStorage { let slots = SimpleSmt::::with_leaves(entries) .map_err(AccountError::DuplicateStorageItems)?; - // check if the number of provided maps is bigger than the number of slots reserved for maps - let count = layout - .iter() - .filter(|&slot| matches!(slot, StorageSlotType::Map { .. })) - .count(); - - if maps.len() > count { + if maps.len() > num_maps { return Err(AccountError::StorageMapTooManyMaps { - expected: count, + expected: num_maps, actual: maps.len(), }); } From fdf9e6353d8c0c346e7503c91f1224facf2dfa83 Mon Sep 17 00:00:00 2001 From: "Augusto F. Hack" Date: Fri, 7 Jun 2024 17:45:52 +0200 Subject: [PATCH 07/15] added layout_commitment util --- objects/src/accounts/storage/mod.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/objects/src/accounts/storage/mod.rs b/objects/src/accounts/storage/mod.rs index 3be8d6e7b..6cbfe0df5 100644 --- a/objects/src/accounts/storage/mod.rs +++ b/objects/src/accounts/storage/mod.rs @@ -191,7 +191,7 @@ impl AccountStorage { // add layout commitment entry entries.push(( AccountStorage::SLOT_LAYOUT_COMMITMENT_INDEX.into(), - *Hasher::hash_elements(&layout.iter().map(Felt::from).collect::>()), + *layout_commitment(&layout), )); // construct storage slots smt and populate the types vector. @@ -237,7 +237,7 @@ impl AccountStorage { /// Returns a commitment to the storage layout. pub fn layout_commitment(&self) -> Digest { - Hasher::hash_elements(&self.layout.iter().map(Felt::from).collect::>()) + layout_commitment(&self.layout) } /// Returns the storage maps for this storage. @@ -302,6 +302,14 @@ impl AccountStorage { } } +// UTILITIES +// ------------------------------------------------------------------------------------------------ + +/// Computes the commitment to the given layout +fn layout_commitment(layout: &[StorageSlotType]) -> Digest { + Hasher::hash_elements(&layout.iter().map(Felt::from).collect::>()) +} + // SERIALIZATION // ================================================================================================ From 12f4692e5d69803515e49dd971895a896733b346 Mon Sep 17 00:00:00 2001 From: "Augusto F. Hack" Date: Fri, 7 Jun 2024 18:07:23 +0200 Subject: [PATCH 08/15] use StorageMap serde --- objects/src/accounts/storage/map.rs | 17 ++++++++- objects/src/accounts/storage/mod.rs | 52 ++++++++++------------------ objects/src/accounts/storage/slot.rs | 17 +++++++-- 3 files changed, 49 insertions(+), 37 deletions(-) diff --git a/objects/src/accounts/storage/map.rs b/objects/src/accounts/storage/map.rs index b769fd1e6..009a0ef24 100644 --- a/objects/src/accounts/storage/map.rs +++ b/objects/src/accounts/storage/map.rs @@ -1,3 +1,5 @@ +use vm_core::Felt; + use super::{ AccountError, ByteReader, ByteWriter, Deserializable, DeserializationError, Serializable, Word, }; @@ -9,6 +11,13 @@ use crate::crypto::{ // ACCOUNT STORAGE MAP // ================================================================================================ +pub const EMPTY_STORAGE_MAP: Word = [ + Felt::new(15321474589252129342), + Felt::new(17373224439259377994), + Felt::new(15071539326562317628), + Felt::new(3312677166725950353), +]; + /// Account storage map is a Sparse Merkle Tree of depth 64. It can be used to store more data as /// there is in plain usage of the storage slots. The root of the SMT consumes one account storage /// slot. @@ -119,7 +128,7 @@ impl Deserializable for StorageMap { mod tests { use miden_crypto::{hash::rpo::RpoDigest, Felt}; - use super::{Deserializable, Serializable, StorageMap, Word}; + use super::{Deserializable, Serializable, StorageMap, Word, EMPTY_STORAGE_MAP}; #[test] fn account_storage_serialization() { @@ -144,4 +153,10 @@ mod tests { let bytes = storage_map.to_bytes(); assert_eq!(storage_map, StorageMap::read_from_bytes(&bytes).unwrap()); } + + #[test] + fn test_empty_storage_map_constants() { + // If these values don't match, update the constants. + assert_eq!(*StorageMap::default().root(), EMPTY_STORAGE_MAP); + } } diff --git a/objects/src/accounts/storage/mod.rs b/objects/src/accounts/storage/mod.rs index 6cbfe0df5..e374e5fc0 100644 --- a/objects/src/accounts/storage/mod.rs +++ b/objects/src/accounts/storage/mod.rs @@ -10,7 +10,7 @@ mod slot; pub use slot::StorageSlotType; mod map; -pub use map::StorageMap; +pub use map::{StorageMap, EMPTY_STORAGE_MAP}; // CONSTANTS // ================================================================================================ @@ -326,45 +326,37 @@ impl Serializable for AccountStorage { complex_types.write_into(target); - // serialize slot values; we serialize only non-empty values and also skip slot 255 as info - // for this slot was already serialized as a part of serializing slot type info above let filled_slots = self .slots .leaves() - .filter(|(idx, &value)| { - // TODO: consider checking empty values for complex types as well - value != SimpleSmt::::EMPTY_VALUE - && *idx as u8 != AccountStorage::SLOT_LAYOUT_COMMITMENT_INDEX + // don't serialize the default values, these are implied. + .filter(|(index, &value)| { + let slot_type = self.layout + [usize::try_from(*index).expect("Number of slot types is limited to u8")]; + value != slot_type.empty_word() }) + .map(|(index, value)| (u8::try_from(index).expect("Number of slot types is limited to u8"), value)) + // don't serialized the layout commitment, it can be recomputed + .filter(|(index, _)| *index != AccountStorage::SLOT_LAYOUT_COMMITMENT_INDEX) .collect::>(); - target.write_u8(filled_slots.len() as u8); - for (idx, &value) in filled_slots { - target.write_u8(idx as u8); - target.write(value); - } - - // serialize the number of StorageMaps - target.write_u8(self.maps.len() as u8); + filled_slots.write_into(target); - // serialize storage maps - for storage_map in &self.maps { - storage_map.write_into(target); - } + // serialize the storage maps + self.maps.write_into(target); } } impl Deserializable for AccountStorage { fn read_from(source: &mut R) -> Result { + // read the non-default layout types let complex_types = >::read_from(source)?; let mut complex_types = BTreeMap::from_iter(complex_types); - // read filled slots and build a vector of slot items + // read the non-default entries + let filled_slots = >::read_from(source)?; let mut items: Vec = Vec::new(); - let num_filled_slots = source.read_u8()?; - for _ in 0..num_filled_slots { - let index = source.read_u8()?; - let value: Word = source.read()?; + for (index, value) in filled_slots { let slot_type = complex_types.remove(&index).unwrap_or_default(); items.push(SlotItem { index, @@ -372,16 +364,8 @@ impl Deserializable for AccountStorage { }); } - // read the number of StorageMap instances - let num_storage_maps = source.read_u8()?; - - let mut maps = Vec::with_capacity(num_storage_maps as usize); - for _ in 0..num_storage_maps { - maps.push( - StorageMap::read_from(source) - .map_err(|err| DeserializationError::InvalidValue(err.to_string()))?, - ); - } + // read the storage maps + let maps = >::read_from(source)?; Self::new(items, maps).map_err(|err| DeserializationError::InvalidValue(err.to_string())) } diff --git a/objects/src/accounts/storage/slot.rs b/objects/src/accounts/storage/slot.rs index cbb6f64c0..ba811ad1b 100644 --- a/objects/src/accounts/storage/slot.rs +++ b/objects/src/accounts/storage/slot.rs @@ -1,9 +1,13 @@ use alloc::string::{String, ToString}; -use vm_core::utils::{Deserializable, Serializable}; +use miden_crypto::merkle::SimpleSmt; +use vm_core::{ + utils::{Deserializable, Serializable}, + Word, EMPTY_WORD, +}; use vm_processor::DeserializationError; -use super::Felt; +use super::{Felt, EMPTY_STORAGE_MAP, STORAGE_TREE_DEPTH}; // CONSTANTS // ================================================================================================ @@ -56,6 +60,15 @@ impl StorageSlotType { _ => false, } } + + /// Returns the empty [Word] for a value of this type. + pub fn empty_word(&self) -> Word { + match self { + StorageSlotType::Value { .. } => SimpleSmt::::EMPTY_VALUE, + StorageSlotType::Map { .. } => EMPTY_STORAGE_MAP, + StorageSlotType::Array { .. } => EMPTY_WORD, + } + } } impl Default for StorageSlotType { From 456af4ec6789a9f4cf1c2bc4458104ab0c88a5df Mon Sep 17 00:00:00 2001 From: "Augusto F. Hack" Date: Fri, 7 Jun 2024 18:11:37 +0200 Subject: [PATCH 09/15] allow any slot item to be updated --- objects/src/accounts/storage/mod.rs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/objects/src/accounts/storage/mod.rs b/objects/src/accounts/storage/mod.rs index e374e5fc0..4b852cca7 100644 --- a/objects/src/accounts/storage/mod.rs +++ b/objects/src/accounts/storage/mod.rs @@ -281,20 +281,6 @@ impl AccountStorage { return Err(AccountError::StorageSlotIsReserved(index)); } - // only value slots of basic arity can currently be updated - match self.layout[usize::from(index)] { - StorageSlotType::Value { value_arity } => { - if value_arity > 0 { - return Err(AccountError::StorageSlotInvalidValueArity { - slot: index, - expected: 0, - actual: value_arity, - }); - } - }, - slot_type => Err(AccountError::StorageSlotNotValueSlot(index, slot_type))?, - } - // update the slot and return let index = LeafIndex::new(index.into()).expect("index is u8 - index within range"); let slot_value = self.slots.insert(index, value); From c56cddf62a4d863eaeb5656209bde708a892a44c Mon Sep 17 00:00:00 2001 From: Dominik Schmid <35031754+Dominik1999@users.noreply.github.com> Date: Tue, 11 Jun 2024 16:09:02 +0800 Subject: [PATCH 10/15] Apply account delta storage maps (#738) * fix: applying account delta for storage maps * rebasing on top * changing to BTreeMap * making tests pass * after review --- bench-tx/src/utils.rs | 2 +- miden-lib/src/accounts/faucets/mod.rs | 4 +- miden-lib/src/accounts/wallets/mod.rs | 7 ++- miden-lib/src/transaction/inputs.rs | 2 +- miden-tx/tests/integration/main.rs | 4 +- miden-tx/tests/integration/scripts/faucet.rs | 4 +- miden-tx/tests/integration/wallet/mod.rs | 8 ++- objects/src/accounts/data.rs | 4 +- objects/src/accounts/mod.rs | 48 ++++++++++++--- objects/src/accounts/storage/map.rs | 35 ++++++++--- objects/src/accounts/storage/mod.rs | 65 +++++++++++++++++--- objects/src/accounts/storage/testing.rs | 6 -- objects/src/errors.rs | 3 +- objects/src/testing/account.rs | 5 +- objects/src/testing/storage.rs | 39 +++++++----- 15 files changed, 176 insertions(+), 60 deletions(-) diff --git a/bench-tx/src/utils.rs b/bench-tx/src/utils.rs index cdcfdf142..55de68051 100644 --- a/bench-tx/src/utils.rs +++ b/bench-tx/src/utils.rs @@ -89,7 +89,7 @@ pub fn get_account_with_default_account_code( let account_code = AccountCode::new(account_code_ast.clone(), &account_assembler).unwrap(); let account_storage = - AccountStorage::new(vec![SlotItem::new_value(0, 0, public_key)], vec![]).unwrap(); + AccountStorage::new(vec![SlotItem::new_value(0, 0, public_key)], BTreeMap::new()).unwrap(); let account_vault = match assets { Some(asset) => AssetVault::new(&[asset]).unwrap(), diff --git a/miden-lib/src/accounts/faucets/mod.rs b/miden-lib/src/accounts/faucets/mod.rs index 3090e61ff..3fc871a86 100644 --- a/miden-lib/src/accounts/faucets/mod.rs +++ b/miden-lib/src/accounts/faucets/mod.rs @@ -1,4 +1,4 @@ -use alloc::string::ToString; +use alloc::{collections::BTreeMap, string::ToString}; use miden_objects::{ accounts::{ @@ -72,7 +72,7 @@ pub fn create_basic_fungible_faucet( // - slot 1: token metadata as [max_supply, decimals, token_symbol, 0] let account_storage = AccountStorage::new( vec![SlotItem::new_value(0, 0, auth_data), SlotItem::new_value(1, 0, metadata)], - vec![], + BTreeMap::new(), )?; let account_seed = AccountId::get_account_seed( diff --git a/miden-lib/src/accounts/wallets/mod.rs b/miden-lib/src/accounts/wallets/mod.rs index c1f8d0844..3c3232e2c 100644 --- a/miden-lib/src/accounts/wallets/mod.rs +++ b/miden-lib/src/accounts/wallets/mod.rs @@ -1,4 +1,7 @@ -use alloc::string::{String, ToString}; +use alloc::{ + collections::BTreeMap, + string::{String, ToString}, +}; use miden_objects::{ accounts::{ @@ -58,7 +61,7 @@ pub fn create_basic_wallet( let account_code = AccountCode::new(account_code_ast.clone(), &account_assembler)?; let account_storage = - AccountStorage::new(vec![SlotItem::new_value(0, 0, storage_slot_0_data)], vec![])?; + AccountStorage::new(vec![SlotItem::new_value(0, 0, storage_slot_0_data)], BTreeMap::new())?; let account_seed = AccountId::get_account_seed( init_seed, diff --git a/miden-lib/src/transaction/inputs.rs b/miden-lib/src/transaction/inputs.rs index 57477f5eb..08351c04b 100644 --- a/miden-lib/src/transaction/inputs.rs +++ b/miden-lib/src/transaction/inputs.rs @@ -223,7 +223,7 @@ fn add_account_to_advice_inputs( // If there are storage maps, we populate the merkle store and advice map if !account.storage().maps().is_empty() { - for map in account.storage().maps() { + for map in account.storage().maps().values() { // extend the merkle store and map with the storage maps inputs.extend_merkle_store(map.inner_nodes()); diff --git a/miden-tx/tests/integration/main.rs b/miden-tx/tests/integration/main.rs index f02d2f67a..38dacaa64 100644 --- a/miden-tx/tests/integration/main.rs +++ b/miden-tx/tests/integration/main.rs @@ -72,6 +72,8 @@ pub fn get_account_with_default_account_code( public_key: Word, assets: Option, ) -> Account { + use std::collections::BTreeMap; + use miden_objects::testing::account_code::DEFAULT_ACCOUNT_CODE; let account_code_src = DEFAULT_ACCOUNT_CODE; let account_code_ast = ModuleAst::parse(account_code_src).unwrap(); @@ -79,7 +81,7 @@ pub fn get_account_with_default_account_code( let account_code = AccountCode::new(account_code_ast.clone(), &account_assembler).unwrap(); let account_storage = - AccountStorage::new(vec![SlotItem::new_value(0, 0, public_key)], vec![]).unwrap(); + AccountStorage::new(vec![SlotItem::new_value(0, 0, public_key)], BTreeMap::new()).unwrap(); let account_vault = match assets { Some(asset) => AssetVault::new(&[asset]).unwrap(), diff --git a/miden-tx/tests/integration/scripts/faucet.rs b/miden-tx/tests/integration/scripts/faucet.rs index 8aec3a846..c6bc4cf1f 100644 --- a/miden-tx/tests/integration/scripts/faucet.rs +++ b/miden-tx/tests/integration/scripts/faucet.rs @@ -1,5 +1,7 @@ extern crate alloc; +use std::collections::BTreeMap; + use miden_lib::{ accounts::faucets::create_basic_fungible_faucet, transaction::{memory::FAUCET_STORAGE_DATA_SLOT, TransactionKernel}, @@ -297,7 +299,7 @@ fn get_faucet_account_with_max_supply_and_total_issuance( SlotItem::new_value(0, 0, public_key), SlotItem::new_value(1, 0, faucet_storage_slot_1), ], - vec![], + BTreeMap::new(), ) .unwrap(); diff --git a/miden-tx/tests/integration/wallet/mod.rs b/miden-tx/tests/integration/wallet/mod.rs index 58db9cd39..688c27491 100644 --- a/miden-tx/tests/integration/wallet/mod.rs +++ b/miden-tx/tests/integration/wallet/mod.rs @@ -1,3 +1,5 @@ +use std::collections::BTreeMap; + use miden_lib::{accounts::wallets::create_basic_wallet, AuthScheme}; use miden_objects::{ accounts::{ @@ -86,7 +88,8 @@ fn prove_receive_asset_via_wallet() { // clone account info let account_storage = - AccountStorage::new(vec![SlotItem::new_value(0, 0, target_pub_key)], vec![]).unwrap(); + AccountStorage::new(vec![SlotItem::new_value(0, 0, target_pub_key)], BTreeMap::new()) + .unwrap(); let account_code = target_account.code().clone(); // vault delta let target_account_after: Account = Account::from_parts( @@ -162,7 +165,8 @@ fn prove_send_asset_via_wallet() { // clones account info let sender_account_storage = - AccountStorage::new(vec![SlotItem::new_value(0, 0, sender_pub_key)], vec![]).unwrap(); + AccountStorage::new(vec![SlotItem::new_value(0, 0, sender_pub_key)], BTreeMap::new()) + .unwrap(); let sender_account_code = sender_account.code().clone(); // vault delta diff --git a/objects/src/accounts/data.rs b/objects/src/accounts/data.rs index 98d0a53f3..6d4ce1b03 100644 --- a/objects/src/accounts/data.rs +++ b/objects/src/accounts/data.rs @@ -94,6 +94,8 @@ impl Deserializable for AccountData { #[cfg(test)] mod tests { + use alloc::collections::BTreeMap; + use miden_crypto::{ dsa::rpo_falcon512::SecretKey, utils::{Deserializable, Serializable}, @@ -118,7 +120,7 @@ mod tests { // create account and auth let vault = AssetVault::new(&[]).unwrap(); - let storage = AccountStorage::new(vec![], vec![]).unwrap(); + let storage = AccountStorage::new(vec![], BTreeMap::new()).unwrap(); let nonce = Felt::new(0); let account = Account::from_parts(id, vault, storage, code, nonce); let account_seed = Some(Word::default()); diff --git a/objects/src/accounts/mod.rs b/objects/src/accounts/mod.rs index f7c299f65..3a884613a 100644 --- a/objects/src/accounts/mod.rs +++ b/objects/src/accounts/mod.rs @@ -315,10 +315,11 @@ mod tests { utils::{Deserializable, Serializable}, Felt, Word, }; + use vm_processor::Digest; use super::{AccountDelta, AccountStorageDelta, AccountVaultDelta}; use crate::{ - accounts::{delta::AccountStorageDeltaBuilder, Account}, + accounts::{delta::AccountStorageDeltaBuilder, Account, StorageMap, StorageMapDelta}, testing::storage::{build_account, build_account_delta, build_assets}, }; @@ -327,7 +328,7 @@ mod tests { let init_nonce = Felt::new(1); let (asset_0, _) = build_assets(); let word = [Felt::new(1), Felt::new(2), Felt::new(3), Felt::new(4)]; - let account = build_account(vec![asset_0], init_nonce, vec![word]); + let account = build_account(vec![asset_0], init_nonce, vec![word], None); let serialized = account.to_bytes(); let deserialized = Account::read_from_bytes(&serialized).unwrap(); @@ -357,13 +358,38 @@ mod tests { let init_nonce = Felt::new(1); let (asset_0, asset_1) = build_assets(); let word = [Felt::new(1), Felt::new(2), Felt::new(3), Felt::new(4)]; - let mut account = build_account(vec![asset_0], init_nonce, vec![word]); + // StorageMap with values + let storage_map_leaves_2: [(Digest, Word); 2] = [ + ( + Digest::new([Felt::new(101), Felt::new(102), Felt::new(103), Felt::new(104)]), + [Felt::new(1_u64), Felt::new(2_u64), Felt::new(3_u64), Felt::new(4_u64)], + ), + ( + Digest::new([Felt::new(105), Felt::new(106), Felt::new(107), Felt::new(108)]), + [Felt::new(5_u64), Felt::new(6_u64), Felt::new(7_u64), Felt::new(8_u64)], + ), + ]; + let mut storage_map = StorageMap::with_entries(storage_map_leaves_2).unwrap(); + let mut account = + build_account(vec![asset_0], init_nonce, vec![word], Some(storage_map.clone())); + + let new_map_entry = ( + Digest::new([Felt::new(101), Felt::new(102), Felt::new(103), Felt::new(104)]), + [Felt::new(9_u64), Felt::new(10_u64), Felt::new(11_u64), Felt::new(12_u64)], + ); + let updated_map = + StorageMapDelta::from(vec![], vec![(new_map_entry.0.into(), new_map_entry.1)]); + storage_map.insert(new_map_entry.0, new_map_entry.1); // build account delta let final_nonce = Felt::new(2); let storage_delta = AccountStorageDeltaBuilder::new() .add_cleared_items([0]) - .add_updated_items([(1_u8, [Felt::new(1), Felt::new(2), Felt::new(3), Felt::new(4)])]) + .add_updated_items([ + (1_u8, [Felt::new(1), Felt::new(2), Felt::new(3), Felt::new(4)]), + (254_u8, storage_map.root().into()), + ]) + .add_updated_maps([(254_u8, updated_map)]) .build() .unwrap(); let account_delta = @@ -371,7 +397,13 @@ mod tests { // apply delta and create final_account account.apply_delta(&account_delta).unwrap(); - let final_account = build_account(vec![asset_1], final_nonce, vec![Word::default(), word]); + + let final_account = build_account( + vec![asset_1], + final_nonce, + vec![Word::default(), word], + Some(storage_map), + ); // assert account is what it should be assert_eq!(account, final_account); @@ -383,7 +415,7 @@ mod tests { // build account let init_nonce = Felt::new(1); let (asset, _) = build_assets(); - let mut account = build_account(vec![asset], init_nonce, vec![Word::default()]); + let mut account = build_account(vec![asset], init_nonce, vec![Word::default()], None); // build account delta let storage_delta = AccountStorageDeltaBuilder::new() @@ -403,7 +435,7 @@ mod tests { // build account let init_nonce = Felt::new(2); let (asset, _) = build_assets(); - let mut account = build_account(vec![asset], init_nonce, vec![Word::default()]); + let mut account = build_account(vec![asset], init_nonce, vec![Word::default()], None); // build account delta let final_nonce = Felt::new(1); @@ -424,7 +456,7 @@ mod tests { // build account let init_nonce = Felt::new(1); let word = [Felt::new(1), Felt::new(2), Felt::new(3), Felt::new(4)]; - let mut account = build_account(vec![], init_nonce, vec![word]); + let mut account = build_account(vec![], init_nonce, vec![word], None); // build account delta let final_nonce = Felt::new(2); diff --git a/objects/src/accounts/storage/map.rs b/objects/src/accounts/storage/map.rs index 009a0ef24..82c5b5e6a 100644 --- a/objects/src/accounts/storage/map.rs +++ b/objects/src/accounts/storage/map.rs @@ -1,11 +1,15 @@ -use vm_core::Felt; +use vm_core::EMPTY_WORD; use super::{ - AccountError, ByteReader, ByteWriter, Deserializable, DeserializationError, Serializable, Word, + AccountError, ByteReader, ByteWriter, Deserializable, DeserializationError, Felt, Serializable, + Word, }; -use crate::crypto::{ - hash::rpo::RpoDigest, - merkle::{InnerNodeInfo, LeafIndex, Smt, SmtLeaf, SmtProof, SMT_DEPTH}, +use crate::{ + accounts::StorageMapDelta, + crypto::{ + hash::rpo::RpoDigest, + merkle::{InnerNodeInfo, LeafIndex, Smt, SmtLeaf, SmtProof, SMT_DEPTH}, + }, }; // ACCOUNT STORAGE MAP @@ -94,12 +98,29 @@ impl StorageMap { self.map.inner_nodes() // Delegate to Smt's inner_nodes method } - // STATE MUTATORS + // DATA MUTATORS // -------------------------------------------------------------------------------------------- - pub fn insert(&mut self, key: RpoDigest, value: Word) -> Word { self.map.insert(key, value) // Delegate to Smt's insert method } + + /// Applies the provided delta to this account storage. + /// + /// This method assumes that the delta has been validated by the calling method and so, no + /// additional validation of delta is performed. + pub(super) fn apply_delta(&mut self, map_delta: StorageMapDelta) -> Result<(), AccountError> { + // apply the updated leaves to the storage map + for (key, value) in map_delta.updated_leaves.iter() { + self.insert(key.into(), *value); + } + + // apply the cleared leaves to the storage map + for key in map_delta.cleared_leaves.iter() { + self.insert(key.into(), EMPTY_WORD); + } + + Ok(()) + } } impl Default for StorageMap { diff --git a/objects/src/accounts/storage/mod.rs b/objects/src/accounts/storage/mod.rs index 4b852cca7..0f955d66f 100644 --- a/objects/src/accounts/storage/mod.rs +++ b/objects/src/accounts/storage/mod.rs @@ -137,7 +137,7 @@ impl StorageSlot { pub struct AccountStorage { slots: SimpleSmt, layout: Vec, - maps: Vec, + maps: BTreeMap, } impl AccountStorage { @@ -158,7 +158,7 @@ impl AccountStorage { /// Returns a new instance of account storage initialized with the provided items. pub fn new( items: Vec, - maps: Vec, + maps: BTreeMap, ) -> Result { // Empty layout let mut layout = vec![StorageSlotType::default(); AccountStorage::NUM_STORAGE_SLOTS]; @@ -205,6 +205,13 @@ impl AccountStorage { }); } + // validate the map indices + for key in maps.keys() { + if *key == AccountStorage::SLOT_LAYOUT_COMMITMENT_INDEX { + return Err(AccountError::StorageSlotIsReserved(*key)); + } + } + Ok(Self { slots, layout, maps }) } @@ -241,7 +248,7 @@ impl AccountStorage { } /// Returns the storage maps for this storage. - pub fn maps(&self) -> &[StorageMap] { + pub fn maps(&self) -> &BTreeMap { &self.maps } @@ -253,10 +260,10 @@ impl AccountStorage { /// This method assumes that the delta has been validated by the calling method and so, no /// additional validation of delta is performed. /// - /// # Errors /// Returns an error if: /// - The delta implies an update to a reserved account slot. /// - The updates violate storage layout constraints. + /// - The updated value has an arity different from 0. pub(super) fn apply_delta(&mut self, delta: &AccountStorageDelta) -> Result<(), AccountError> { for &slot_idx in delta.cleared_items.iter() { self.set_item(slot_idx, Word::default())?; @@ -266,6 +273,18 @@ impl AccountStorage { self.set_item(slot_idx, slot_value)?; } + for &(slot_idx, ref map_delta) in delta.updated_maps.iter() { + // layout commitment slot cannot be updated + if slot_idx == Self::SLOT_LAYOUT_COMMITMENT_INDEX { + return Err(AccountError::StorageSlotIsReserved(slot_idx)); + } + + // pick the right storage map and apply delta + let storage_map = + self.maps.get_mut(&slot_idx).ok_or(AccountError::StorageMapNotFound(slot_idx))?; + storage_map.apply_delta(map_delta.clone())?; + } + Ok(()) } @@ -274,13 +293,37 @@ impl AccountStorage { /// # Errors /// Returns an error if: /// - The index specifies a reserved storage slot. - /// - The update violates storage layout constraints. + /// - The update tries to set a slot of type array. + /// - The update has a value arity different from 0. pub fn set_item(&mut self, index: u8, value: Word) -> Result { // layout commitment slot cannot be updated if index == Self::SLOT_LAYOUT_COMMITMENT_INDEX { return Err(AccountError::StorageSlotIsReserved(index)); } + // only value slots of basic arity can currently be updated + match self.layout[index as usize] { + StorageSlotType::Value { value_arity } => { + if value_arity > 0 { + return Err(AccountError::StorageSlotInvalidValueArity { + slot: index, + expected: 0, + actual: value_arity, + }); + } + }, + StorageSlotType::Map { value_arity } => { + if value_arity > 0 { + return Err(AccountError::StorageSlotInvalidValueArity { + slot: index, + expected: 0, + actual: value_arity, + }); + } + }, + slot_type => Err(AccountError::StorageSlotArrayNotAllowed(index, slot_type))?, + } + // update the slot and return let index = LeafIndex::new(index.into()).expect("index is u8 - index within range"); let slot_value = self.slots.insert(index, value); @@ -351,7 +394,7 @@ impl Deserializable for AccountStorage { } // read the storage maps - let maps = >::read_from(source)?; + let maps = >::read_from(source)?; Self::new(items, maps).map_err(|err| DeserializationError::InvalidValue(err.to_string())) } @@ -362,7 +405,7 @@ impl Deserializable for AccountStorage { #[cfg(test)] mod tests { - use alloc::vec::Vec; + use alloc::{collections::BTreeMap, vec::Vec}; use miden_crypto::hash::rpo::RpoDigest; @@ -372,7 +415,7 @@ mod tests { #[test] fn account_storage_serialization() { // empty storage - let storage = AccountStorage::new(Vec::new(), Vec::new()).unwrap(); + let storage = AccountStorage::new(Vec::new(), BTreeMap::new()).unwrap(); let bytes = storage.to_bytes(); assert_eq!(storage, AccountStorage::read_from_bytes(&bytes).unwrap()); @@ -382,7 +425,7 @@ mod tests { SlotItem::new_value(0, 0, [ONE, ONE, ONE, ONE]), SlotItem::new_value(2, 0, [ONE, ONE, ONE, ZERO]), ], - vec![], + BTreeMap::new(), ) .unwrap(); let bytes = storage.to_bytes(); @@ -400,6 +443,8 @@ mod tests { ), ]; let storage_map = StorageMap::with_entries(storage_map_leaves_2).unwrap(); + let mut maps = BTreeMap::new(); + maps.insert(2, storage_map.clone()); let storage = AccountStorage::new( vec![ SlotItem::new_value(0, 1, [ONE, ONE, ONE, ONE]), @@ -407,7 +452,7 @@ mod tests { SlotItem::new_map(2, 0, storage_map.root().into()), SlotItem::new_array(3, 3, 4, [ONE, ZERO, ZERO, ZERO]), ], - vec![storage_map], + maps, ) .unwrap(); let bytes = storage.to_bytes(); diff --git a/objects/src/accounts/storage/testing.rs b/objects/src/accounts/storage/testing.rs index ec0687208..c57e730ef 100644 --- a/objects/src/accounts/storage/testing.rs +++ b/objects/src/accounts/storage/testing.rs @@ -32,12 +32,6 @@ impl AccountStorageBuilder { self } - #[allow(dead_code)] - pub fn add_maps>(&mut self, maps: I) -> &mut Self { - self.maps.extend(maps); - self - } - pub fn build(&self) -> AccountStorage { AccountStorage::new(self.items.clone(), self.maps.clone()).unwrap() } diff --git a/objects/src/errors.rs b/objects/src/errors.rs index 962127276..8b83a83f9 100644 --- a/objects/src/errors.rs +++ b/objects/src/errors.rs @@ -33,7 +33,8 @@ pub enum AccountError { SeedDigestTooFewTrailingZeros { expected: u32, actual: u32 }, StorageSlotInvalidValueArity { slot: u8, expected: u8, actual: u8 }, StorageSlotIsReserved(u8), - StorageSlotNotValueSlot(u8, StorageSlotType), + StorageSlotArrayNotAllowed(u8, StorageSlotType), + StorageMapNotFound(u8), StorageMapTooManyMaps { expected: usize, actual: usize }, StubDataIncorrectLength(usize, usize), } diff --git a/objects/src/testing/account.rs b/objects/src/testing/account.rs index e5d543b97..07919bbbb 100644 --- a/objects/src/testing/account.rs +++ b/objects/src/testing/account.rs @@ -1,4 +1,5 @@ use alloc::{ + collections::BTreeMap, string::{String, ToString}, vec::Vec, }; @@ -234,13 +235,15 @@ pub fn mock_account_vault() -> AssetVault { pub fn mock_account_storage() -> AccountStorage { // create account storage + let mut maps = BTreeMap::new(); + maps.insert(STORAGE_INDEX_2, storage_map_2()); AccountStorage::new( vec![ SlotItem::new_value(STORAGE_INDEX_0, 0, STORAGE_VALUE_0), SlotItem::new_value(STORAGE_INDEX_1, 0, STORAGE_VALUE_1), SlotItem::new_map(STORAGE_INDEX_2, 0, storage_map_2().root().into()), ], - vec![storage_map_2()], + maps, ) .unwrap() } diff --git a/objects/src/testing/storage.rs b/objects/src/testing/storage.rs index b281870e3..46ce65bce 100644 --- a/objects/src/testing/storage.rs +++ b/objects/src/testing/storage.rs @@ -1,4 +1,4 @@ -use alloc::{string::String, vec::Vec}; +use alloc::{collections::BTreeMap, string::String, vec::Vec}; use assembly::Assembler; use miden_crypto::merkle::Smt; @@ -31,13 +31,13 @@ use crate::{ #[derive(Default, Debug, Clone)] pub struct AccountStorageBuilder { items: Vec, - maps: Vec, + maps: BTreeMap, } /// Builder for an `AccountStorage`, the builder can be configured and used multiple times. impl AccountStorageBuilder { pub fn new() -> Self { - Self { items: vec![], maps: vec![] } + Self { items: vec![], maps: BTreeMap::new() } } pub fn add_item(&mut self, item: SlotItem) -> &mut Self { @@ -53,14 +53,8 @@ impl AccountStorageBuilder { } #[allow(dead_code)] - pub fn add_map(&mut self, map: StorageMap) -> &mut Self { - self.maps.push(map); - self - } - - #[allow(dead_code)] - pub fn add_maps>(&mut self, maps: I) -> &mut Self { - self.maps.extend(maps); + pub fn add_map(&mut self, index: u8, map: StorageMap) -> &mut Self { + self.maps.insert(index, map); self } @@ -115,7 +109,7 @@ pub fn mock_fungible_faucet( 0, [ZERO, ZERO, ZERO, initial_balance], )], - vec![], + BTreeMap::new(), ) .unwrap(); let account_id = AccountId::try_from(account_id).unwrap(); @@ -144,7 +138,7 @@ pub fn mock_non_fungible_faucet( let account_storage = AccountStorage::new( vec![SlotItem::new_map(FAUCET_STORAGE_DATA_SLOT, 0, *nft_tree.root())], - vec![], + BTreeMap::new(), ) .unwrap(); let account_id = AccountId::try_from(account_id).unwrap(); @@ -243,17 +237,30 @@ pub fn generate_account_seed( // UTILITIES // -------------------------------------------------------------------------------------------- -pub fn build_account(assets: Vec, nonce: Felt, storage_items: Vec) -> Account { +pub fn build_account( + assets: Vec, + nonce: Felt, + storage_items: Vec, + map: Option, +) -> Account { let id = AccountId::try_from(ACCOUNT_ID_REGULAR_ACCOUNT_IMMUTABLE_CODE_ON_CHAIN).unwrap(); let code = make_account_code(); let vault = AssetVault::new(&assets).unwrap(); - let slot_items: Vec = storage_items + let mut slot_items: Vec = storage_items .into_iter() .enumerate() .map(|(index, item)| SlotItem::new_value(index as u8, 0, item)) .collect(); - let storage = AccountStorage::new(slot_items, vec![]).unwrap(); + + let mut maps = BTreeMap::new(); + if let Some(map) = map { + let slot_item_map = SlotItem::new_map(254, 0, *map.root()); + slot_items.push(slot_item_map); + maps.insert(254_u8, map); + } + + let storage = AccountStorage::new(slot_items, maps).unwrap(); Account::from_parts(id, vault, storage, code, nonce) } From 9c6f4ec8c5015d335beaec73d012b046103e8420 Mon Sep 17 00:00:00 2001 From: Dominik Schmid Date: Tue, 11 Jun 2024 12:14:31 +0200 Subject: [PATCH 11/15] last test: --- objects/src/accounts/storage/map.rs | 18 ++++----- objects/src/accounts/storage/mod.rs | 59 +++++++++++++++------------- objects/src/accounts/storage/slot.rs | 7 ++-- 3 files changed, 43 insertions(+), 41 deletions(-) diff --git a/objects/src/accounts/storage/map.rs b/objects/src/accounts/storage/map.rs index 82c5b5e6a..6fea3e9b0 100644 --- a/objects/src/accounts/storage/map.rs +++ b/objects/src/accounts/storage/map.rs @@ -1,5 +1,3 @@ -use vm_core::EMPTY_WORD; - use super::{ AccountError, ByteReader, ByteWriter, Deserializable, DeserializationError, Felt, Serializable, Word, @@ -14,8 +12,8 @@ use crate::{ // ACCOUNT STORAGE MAP // ================================================================================================ - -pub const EMPTY_STORAGE_MAP: Word = [ +/// Empty storage map root. +pub const EMPTY_STORAGE_MAP_ROOT: Word = [ Felt::new(15321474589252129342), Felt::new(17373224439259377994), Felt::new(15071539326562317628), @@ -108,15 +106,15 @@ impl StorageMap { /// /// This method assumes that the delta has been validated by the calling method and so, no /// additional validation of delta is performed. - pub(super) fn apply_delta(&mut self, map_delta: StorageMapDelta) -> Result<(), AccountError> { + pub fn apply_delta(&mut self, delta: &StorageMapDelta) -> Result<(), AccountError> { // apply the updated leaves to the storage map - for (key, value) in map_delta.updated_leaves.iter() { + for (key, value) in delta.updated_leaves.iter() { self.insert(key.into(), *value); } // apply the cleared leaves to the storage map - for key in map_delta.cleared_leaves.iter() { - self.insert(key.into(), EMPTY_WORD); + for key in delta.cleared_leaves.iter() { + self.insert(key.into(), Smt::EMPTY_VALUE); } Ok(()) @@ -149,7 +147,7 @@ impl Deserializable for StorageMap { mod tests { use miden_crypto::{hash::rpo::RpoDigest, Felt}; - use super::{Deserializable, Serializable, StorageMap, Word, EMPTY_STORAGE_MAP}; + use super::{Deserializable, Serializable, StorageMap, Word, EMPTY_STORAGE_MAP_ROOT}; #[test] fn account_storage_serialization() { @@ -178,6 +176,6 @@ mod tests { #[test] fn test_empty_storage_map_constants() { // If these values don't match, update the constants. - assert_eq!(*StorageMap::default().root(), EMPTY_STORAGE_MAP); + assert_eq!(*StorageMap::default().root(), EMPTY_STORAGE_MAP_ROOT); } } diff --git a/objects/src/accounts/storage/mod.rs b/objects/src/accounts/storage/mod.rs index 0f955d66f..a0f3082a7 100644 --- a/objects/src/accounts/storage/mod.rs +++ b/objects/src/accounts/storage/mod.rs @@ -1,5 +1,5 @@ use alloc::{collections::BTreeMap, string::ToString, vec::Vec}; - +use std::println; use super::{ AccountError, AccountStorageDelta, ByteReader, ByteWriter, Deserializable, DeserializationError, Digest, Felt, Hasher, Serializable, Word, @@ -10,7 +10,7 @@ mod slot; pub use slot::StorageSlotType; mod map; -pub use map::{StorageMap, EMPTY_STORAGE_MAP}; +pub use map::StorageMap; // CONSTANTS // ================================================================================================ @@ -169,6 +169,7 @@ impl AccountStorage { // // - Validate the slot and check it doesn't assign a value to a reserved slot. // - Extract the slot value. + // - Check that every map index has a corresponding map in `maps`. // - Count the number of maps to validate `maps`. // // It won't detect duplicates, that is later done by the `SimpleSmt` instantiation. @@ -181,6 +182,10 @@ impl AccountStorage { } if matches!(item.slot.slot_type, StorageSlotType::Map { .. }) { + // check that for every map index there is a map in maps + if !maps.contains_key(&item.index) { + return Err(AccountError::StorageMapNotFound(item.index)); + } num_maps += 1; } @@ -198,6 +203,7 @@ impl AccountStorage { let slots = SimpleSmt::::with_leaves(entries) .map_err(AccountError::DuplicateStorageItems)?; + println!("numbers: {:?}, num_maps: {:?}", maps.len(), num_maps); if maps.len() > num_maps { return Err(AccountError::StorageMapTooManyMaps { expected: num_maps, @@ -205,13 +211,6 @@ impl AccountStorage { }); } - // validate the map indices - for key in maps.keys() { - if *key == AccountStorage::SLOT_LAYOUT_COMMITMENT_INDEX { - return Err(AccountError::StorageSlotIsReserved(*key)); - } - } - Ok(Self { slots, layout, maps }) } @@ -282,7 +281,7 @@ impl AccountStorage { // pick the right storage map and apply delta let storage_map = self.maps.get_mut(&slot_idx).ok_or(AccountError::StorageMapNotFound(slot_idx))?; - storage_map.apply_delta(map_delta.clone())?; + storage_map.apply_delta(map_delta)?; } Ok(()) @@ -362,12 +361,14 @@ impl Serializable for AccountStorage { .filter(|(index, &value)| { let slot_type = self.layout [usize::try_from(*index).expect("Number of slot types is limited to u8")]; - value != slot_type.empty_word() + value != slot_type.default_word() }) .map(|(index, value)| (u8::try_from(index).expect("Number of slot types is limited to u8"), value)) // don't serialized the layout commitment, it can be recomputed .filter(|(index, _)| *index != AccountStorage::SLOT_LAYOUT_COMMITMENT_INDEX) .collect::>(); + println!("complex_type: {:?}", complex_types); + println!("filled_slots: {:?}", filled_slots); filled_slots.write_into(target); @@ -392,6 +393,7 @@ impl Deserializable for AccountStorage { slot: StorageSlot { slot_type, value }, }); } + println!("items: {:?}", items); // read the storage maps let maps = >::read_from(source)?; @@ -414,22 +416,22 @@ mod tests { #[test] fn account_storage_serialization() { - // empty storage - let storage = AccountStorage::new(Vec::new(), BTreeMap::new()).unwrap(); - let bytes = storage.to_bytes(); - assert_eq!(storage, AccountStorage::read_from_bytes(&bytes).unwrap()); - - // storage with values for default types - let storage = AccountStorage::new( - vec![ - SlotItem::new_value(0, 0, [ONE, ONE, ONE, ONE]), - SlotItem::new_value(2, 0, [ONE, ONE, ONE, ZERO]), - ], - BTreeMap::new(), - ) - .unwrap(); - let bytes = storage.to_bytes(); - assert_eq!(storage, AccountStorage::read_from_bytes(&bytes).unwrap()); + // // empty storage + // let storage = AccountStorage::new(Vec::new(), BTreeMap::new()).unwrap(); + // let bytes = storage.to_bytes(); + // assert_eq!(storage, AccountStorage::read_from_bytes(&bytes).unwrap()); + + // // storage with values for default types + // let storage = AccountStorage::new( + // vec![ + // SlotItem::new_value(0, 0, [ONE, ONE, ONE, ONE]), + // SlotItem::new_value(2, 0, [ONE, ONE, ONE, ZERO]), + // ], + // BTreeMap::new(), + // ) + // .unwrap(); + // let bytes = storage.to_bytes(); + // assert_eq!(storage, AccountStorage::read_from_bytes(&bytes).unwrap()); // storage with values for complex types let storage_map_leaves_2: [(RpoDigest, Word); 2] = [ @@ -455,7 +457,8 @@ mod tests { maps, ) .unwrap(); + let bytes = storage.to_bytes(); assert_eq!(storage, AccountStorage::read_from_bytes(&bytes).unwrap()); } -} +} \ No newline at end of file diff --git a/objects/src/accounts/storage/slot.rs b/objects/src/accounts/storage/slot.rs index ba811ad1b..f2b5c7035 100644 --- a/objects/src/accounts/storage/slot.rs +++ b/objects/src/accounts/storage/slot.rs @@ -7,7 +7,7 @@ use vm_core::{ }; use vm_processor::DeserializationError; -use super::{Felt, EMPTY_STORAGE_MAP, STORAGE_TREE_DEPTH}; +use super::{map::EMPTY_STORAGE_MAP_ROOT, Felt, STORAGE_TREE_DEPTH}; // CONSTANTS // ================================================================================================ @@ -57,15 +57,16 @@ impl StorageSlotType { pub fn is_default(&self) -> bool { match self { StorageSlotType::Value { value_arity } => *value_arity == 0, + StorageSlotType::Map { value_arity } => *value_arity == 0, _ => false, } } /// Returns the empty [Word] for a value of this type. - pub fn empty_word(&self) -> Word { + pub fn default_word(&self) -> Word { match self { StorageSlotType::Value { .. } => SimpleSmt::::EMPTY_VALUE, - StorageSlotType::Map { .. } => EMPTY_STORAGE_MAP, + StorageSlotType::Map { .. } => EMPTY_STORAGE_MAP_ROOT, StorageSlotType::Array { .. } => EMPTY_WORD, } } From c5e5fe76de15df1545ae19600dab172e45b28a1a Mon Sep 17 00:00:00 2001 From: Dominik Schmid Date: Tue, 11 Jun 2024 14:18:33 +0200 Subject: [PATCH 12/15] some refactorings --- objects/src/accounts/mod.rs | 55 ++++++++++++++++++++++------ objects/src/accounts/storage/mod.rs | 12 ++---- objects/src/accounts/storage/slot.rs | 1 - objects/src/testing/storage.rs | 29 +++++---------- 4 files changed, 57 insertions(+), 40 deletions(-) diff --git a/objects/src/accounts/mod.rs b/objects/src/accounts/mod.rs index 3a884613a..0299aa62f 100644 --- a/objects/src/accounts/mod.rs +++ b/objects/src/accounts/mod.rs @@ -311,6 +311,8 @@ pub fn hash_account( #[cfg(test)] mod tests { + use alloc::collections::BTreeMap; + use miden_crypto::{ utils::{Deserializable, Serializable}, Felt, Word, @@ -319,7 +321,9 @@ mod tests { use super::{AccountDelta, AccountStorageDelta, AccountVaultDelta}; use crate::{ - accounts::{delta::AccountStorageDeltaBuilder, Account, StorageMap, StorageMapDelta}, + accounts::{ + delta::AccountStorageDeltaBuilder, Account, SlotItem, StorageMap, StorageMapDelta, + }, testing::storage::{build_account, build_account_delta, build_assets}, }; @@ -328,7 +332,8 @@ mod tests { let init_nonce = Felt::new(1); let (asset_0, _) = build_assets(); let word = [Felt::new(1), Felt::new(2), Felt::new(3), Felt::new(4)]; - let account = build_account(vec![asset_0], init_nonce, vec![word], None); + let slot_item = SlotItem::new_value(0, 0, word); + let account = build_account(vec![asset_0], init_nonce, vec![slot_item], None); let serialized = account.to_bytes(); let deserialized = Account::read_from_bytes(&serialized).unwrap(); @@ -357,7 +362,11 @@ mod tests { // build account let init_nonce = Felt::new(1); let (asset_0, asset_1) = build_assets(); + + // Simple SlotItem let word = [Felt::new(1), Felt::new(2), Felt::new(3), Felt::new(4)]; + let slot_item = SlotItem::new_value(0, 0, word); + // StorageMap with values let storage_map_leaves_2: [(Digest, Word); 2] = [ ( @@ -370,8 +379,16 @@ mod tests { ), ]; let mut storage_map = StorageMap::with_entries(storage_map_leaves_2).unwrap(); - let mut account = - build_account(vec![asset_0], init_nonce, vec![word], Some(storage_map.clone())); + let mut maps = BTreeMap::new(); + maps.insert(2u8, storage_map.clone()); + let storage_map_root_as_slot_item = SlotItem::new_map(2u8, 0, storage_map.root().into()); + + let mut account = build_account( + vec![asset_0], + init_nonce, + vec![slot_item, storage_map_root_as_slot_item], + Some(maps.clone()), + ); let new_map_entry = ( Digest::new([Felt::new(101), Felt::new(102), Felt::new(103), Felt::new(104)]), @@ -380,6 +397,7 @@ mod tests { let updated_map = StorageMapDelta::from(vec![], vec![(new_map_entry.0.into(), new_map_entry.1)]); storage_map.insert(new_map_entry.0, new_map_entry.1); + maps.insert(2u8, storage_map.clone()); // build account delta let final_nonce = Felt::new(2); @@ -387,9 +405,9 @@ mod tests { .add_cleared_items([0]) .add_updated_items([ (1_u8, [Felt::new(1), Felt::new(2), Felt::new(3), Felt::new(4)]), - (254_u8, storage_map.root().into()), + (2_u8, storage_map.root().into()), ]) - .add_updated_maps([(254_u8, updated_map)]) + .add_updated_maps([(2_u8, updated_map)]) .build() .unwrap(); let account_delta = @@ -401,8 +419,12 @@ mod tests { let final_account = build_account( vec![asset_1], final_nonce, - vec![Word::default(), word], - Some(storage_map), + vec![ + SlotItem::new_value(0, 0, Word::default()), + SlotItem::new_value(1, 0, word), + SlotItem::new_map(2, 0, storage_map.root().into()), + ], + Some(maps), ); // assert account is what it should be @@ -415,7 +437,12 @@ mod tests { // build account let init_nonce = Felt::new(1); let (asset, _) = build_assets(); - let mut account = build_account(vec![asset], init_nonce, vec![Word::default()], None); + let mut account = build_account( + vec![asset], + init_nonce, + vec![SlotItem::new_value(0, 0, Word::default())], + None, + ); // build account delta let storage_delta = AccountStorageDeltaBuilder::new() @@ -435,7 +462,12 @@ mod tests { // build account let init_nonce = Felt::new(2); let (asset, _) = build_assets(); - let mut account = build_account(vec![asset], init_nonce, vec![Word::default()], None); + let mut account = build_account( + vec![asset], + init_nonce, + vec![SlotItem::new_value(0, 0, Word::default())], + None, + ); // build account delta let final_nonce = Felt::new(1); @@ -456,7 +488,8 @@ mod tests { // build account let init_nonce = Felt::new(1); let word = [Felt::new(1), Felt::new(2), Felt::new(3), Felt::new(4)]; - let mut account = build_account(vec![], init_nonce, vec![word], None); + let slot_item = SlotItem::new_value(0, 0, word); + let mut account = build_account(vec![], init_nonce, vec![slot_item], None); // build account delta let final_nonce = Felt::new(2); diff --git a/objects/src/accounts/storage/mod.rs b/objects/src/accounts/storage/mod.rs index a0f3082a7..7ca8c8df3 100644 --- a/objects/src/accounts/storage/mod.rs +++ b/objects/src/accounts/storage/mod.rs @@ -1,5 +1,5 @@ use alloc::{collections::BTreeMap, string::ToString, vec::Vec}; -use std::println; + use super::{ AccountError, AccountStorageDelta, ByteReader, ByteWriter, Deserializable, DeserializationError, Digest, Felt, Hasher, Serializable, Word, @@ -203,7 +203,6 @@ impl AccountStorage { let slots = SimpleSmt::::with_leaves(entries) .map_err(AccountError::DuplicateStorageItems)?; - println!("numbers: {:?}, num_maps: {:?}", maps.len(), num_maps); if maps.len() > num_maps { return Err(AccountError::StorageMapTooManyMaps { expected: num_maps, @@ -344,6 +343,7 @@ fn layout_commitment(layout: &[StorageSlotType]) -> Digest { impl Serializable for AccountStorage { fn write_into(&self, target: &mut W) { // don't serialize last slot as it is a constant. + // complex types are all types different from StorageSlotType::Value { value_arity: 0 } let complex_types = self.layout[..usize::from(AccountStorage::SLOT_LAYOUT_COMMITMENT_INDEX)] .iter() .enumerate() @@ -367,8 +367,6 @@ impl Serializable for AccountStorage { // don't serialized the layout commitment, it can be recomputed .filter(|(index, _)| *index != AccountStorage::SLOT_LAYOUT_COMMITMENT_INDEX) .collect::>(); - println!("complex_type: {:?}", complex_types); - println!("filled_slots: {:?}", filled_slots); filled_slots.write_into(target); @@ -393,8 +391,6 @@ impl Deserializable for AccountStorage { slot: StorageSlot { slot_type, value }, }); } - println!("items: {:?}", items); - // read the storage maps let maps = >::read_from(source)?; @@ -407,7 +403,7 @@ impl Deserializable for AccountStorage { #[cfg(test)] mod tests { - use alloc::{collections::BTreeMap, vec::Vec}; + use alloc::collections::BTreeMap; use miden_crypto::hash::rpo::RpoDigest; @@ -461,4 +457,4 @@ mod tests { let bytes = storage.to_bytes(); assert_eq!(storage, AccountStorage::read_from_bytes(&bytes).unwrap()); } -} \ No newline at end of file +} diff --git a/objects/src/accounts/storage/slot.rs b/objects/src/accounts/storage/slot.rs index f2b5c7035..515f34b05 100644 --- a/objects/src/accounts/storage/slot.rs +++ b/objects/src/accounts/storage/slot.rs @@ -57,7 +57,6 @@ impl StorageSlotType { pub fn is_default(&self) -> bool { match self { StorageSlotType::Value { value_arity } => *value_arity == 0, - StorageSlotType::Map { value_arity } => *value_arity == 0, _ => false, } } diff --git a/objects/src/testing/storage.rs b/objects/src/testing/storage.rs index 46ce65bce..c5e820219 100644 --- a/objects/src/testing/storage.rs +++ b/objects/src/testing/storage.rs @@ -1,7 +1,6 @@ use alloc::{collections::BTreeMap, string::String, vec::Vec}; use assembly::Assembler; -use miden_crypto::merkle::Smt; use vm_core::{Felt, FieldElement, Word, ZERO}; use vm_processor::Digest; @@ -132,13 +131,13 @@ pub fn mock_non_fungible_faucet( }; // construct nft tree - let nft_tree = Smt::with_entries(entries).unwrap(); - - // TODO: add nft tree data to account storage? + let nft_storage_map = StorageMap::with_entries(entries).unwrap(); + let mut maps = BTreeMap::new(); + maps.insert(FAUCET_STORAGE_DATA_SLOT, nft_storage_map.clone()); let account_storage = AccountStorage::new( - vec![SlotItem::new_map(FAUCET_STORAGE_DATA_SLOT, 0, *nft_tree.root())], - BTreeMap::new(), + vec![SlotItem::new_map(FAUCET_STORAGE_DATA_SLOT, 0, *nft_storage_map.root())], + maps, ) .unwrap(); let account_id = AccountId::try_from(account_id).unwrap(); @@ -240,25 +239,15 @@ pub fn generate_account_seed( pub fn build_account( assets: Vec, nonce: Felt, - storage_items: Vec, - map: Option, + slot_items: Vec, + maps: Option>, ) -> Account { let id = AccountId::try_from(ACCOUNT_ID_REGULAR_ACCOUNT_IMMUTABLE_CODE_ON_CHAIN).unwrap(); let code = make_account_code(); let vault = AssetVault::new(&assets).unwrap(); - let mut slot_items: Vec = storage_items - .into_iter() - .enumerate() - .map(|(index, item)| SlotItem::new_value(index as u8, 0, item)) - .collect(); - - let mut maps = BTreeMap::new(); - if let Some(map) = map { - let slot_item_map = SlotItem::new_map(254, 0, *map.root()); - slot_items.push(slot_item_map); - maps.insert(254_u8, map); - } + // Use the provided maps or create an empty BTreeMap if None is provided + let maps = maps.unwrap_or_default(); let storage = AccountStorage::new(slot_items, maps).unwrap(); From 33a16f44b3fbf3861787ee53cc49a546f52670b3 Mon Sep 17 00:00:00 2001 From: Dominik Schmid Date: Tue, 11 Jun 2024 21:01:17 +0200 Subject: [PATCH 13/15] adding a set_map_item in Rust --- miden-lib/asm/miden/kernels/tx/account.masm | 2 +- objects/src/accounts/storage/map.rs | 27 +++++++++++++--- objects/src/accounts/storage/mod.rs | 34 ++++++++++----------- 3 files changed, 41 insertions(+), 22 deletions(-) diff --git a/miden-lib/asm/miden/kernels/tx/account.masm b/miden-lib/asm/miden/kernels/tx/account.masm index e4f2b698b..75ff9bf9a 100644 --- a/miden-lib/asm/miden/kernels/tx/account.masm +++ b/miden-lib/asm/miden/kernels/tx/account.masm @@ -403,7 +403,7 @@ export.set_map_item.2 # 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, ...] - + # store OLD_VALUE and NEW_ROOT until the end of the procedure loc_storew.0 dropw loc_storew.1 dropw # => [KEY, NEW_VALUE, index, ...] diff --git a/objects/src/accounts/storage/map.rs b/objects/src/accounts/storage/map.rs index 6fea3e9b0..bea5883fd 100644 --- a/objects/src/accounts/storage/map.rs +++ b/objects/src/accounts/storage/map.rs @@ -1,3 +1,5 @@ +use vm_core::EMPTY_WORD; + use super::{ AccountError, ByteReader, ByteWriter, Deserializable, DeserializationError, Felt, Serializable, Word, @@ -108,17 +110,34 @@ impl StorageMap { /// additional validation of delta is performed. pub fn apply_delta(&mut self, delta: &StorageMapDelta) -> Result<(), AccountError> { // apply the updated leaves to the storage map - for (key, value) in delta.updated_leaves.iter() { - self.insert(key.into(), *value); + for &(key, value) in delta.updated_leaves.iter() { + self.set_map_item(key, value)?; } // apply the cleared leaves to the storage map - for key in delta.cleared_leaves.iter() { - self.insert(key.into(), Smt::EMPTY_VALUE); + // currently we cannot remove leaves from the storage map, so we just set them to empty + for &key in delta.cleared_leaves.iter() { + self.set_map_item(key, EMPTY_WORD)?; } Ok(()) } + + /// Sets a map item from the storage at the specified index. + pub fn set_map_item(&mut self, key: Word, value: Word) -> Result<(Word, Word), AccountError> { + let old_map_root = self.root(); + let old_value = self.get_value(&RpoDigest::from(key)); + + if value == EMPTY_WORD { + // if the value is empty, remove the leaf from the storage map + self.map.insert(key.into(), value); + } else { + // insert the value into the storage map + self.map.insert(key.into(), value); + } + + Ok((old_map_root.into(), old_value)) + } } impl Default for StorageMap { diff --git a/objects/src/accounts/storage/mod.rs b/objects/src/accounts/storage/mod.rs index 7ca8c8df3..e4f27e386 100644 --- a/objects/src/accounts/storage/mod.rs +++ b/objects/src/accounts/storage/mod.rs @@ -403,7 +403,7 @@ impl Deserializable for AccountStorage { #[cfg(test)] mod tests { - use alloc::collections::BTreeMap; + use alloc::{collections::BTreeMap, vec::Vec}; use miden_crypto::hash::rpo::RpoDigest; @@ -412,22 +412,22 @@ mod tests { #[test] fn account_storage_serialization() { - // // empty storage - // let storage = AccountStorage::new(Vec::new(), BTreeMap::new()).unwrap(); - // let bytes = storage.to_bytes(); - // assert_eq!(storage, AccountStorage::read_from_bytes(&bytes).unwrap()); - - // // storage with values for default types - // let storage = AccountStorage::new( - // vec![ - // SlotItem::new_value(0, 0, [ONE, ONE, ONE, ONE]), - // SlotItem::new_value(2, 0, [ONE, ONE, ONE, ZERO]), - // ], - // BTreeMap::new(), - // ) - // .unwrap(); - // let bytes = storage.to_bytes(); - // assert_eq!(storage, AccountStorage::read_from_bytes(&bytes).unwrap()); + // empty storage + let storage = AccountStorage::new(Vec::new(), BTreeMap::new()).unwrap(); + let bytes = storage.to_bytes(); + assert_eq!(storage, AccountStorage::read_from_bytes(&bytes).unwrap()); + + // storage with values for default types + let storage = AccountStorage::new( + vec![ + SlotItem::new_value(0, 0, [ONE, ONE, ONE, ONE]), + SlotItem::new_value(2, 0, [ONE, ONE, ONE, ZERO]), + ], + BTreeMap::new(), + ) + .unwrap(); + let bytes = storage.to_bytes(); + assert_eq!(storage, AccountStorage::read_from_bytes(&bytes).unwrap()); // storage with values for complex types let storage_map_leaves_2: [(RpoDigest, Word); 2] = [ From eafa5c3d7c69e85459710efb526efe879d6615ca Mon Sep 17 00:00:00 2001 From: Bobbin Threadbare Date: Tue, 11 Jun 2024 14:34:44 -0700 Subject: [PATCH 14/15] refactor: minor changes to storage mutator validation code --- objects/src/accounts/storage/mod.rs | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/objects/src/accounts/storage/mod.rs b/objects/src/accounts/storage/mod.rs index e4f27e386..78ac6a32c 100644 --- a/objects/src/accounts/storage/mod.rs +++ b/objects/src/accounts/storage/mod.rs @@ -203,7 +203,8 @@ impl AccountStorage { let slots = SimpleSmt::::with_leaves(entries) .map_err(AccountError::DuplicateStorageItems)?; - if maps.len() > num_maps { + // make sure the number of provide maps matches the number of map slots + if maps.len() != num_maps { return Err(AccountError::StorageMapTooManyMaps { expected: num_maps, actual: maps.len(), @@ -263,13 +264,8 @@ impl AccountStorage { /// - The updates violate storage layout constraints. /// - The updated value has an arity different from 0. pub(super) fn apply_delta(&mut self, delta: &AccountStorageDelta) -> Result<(), AccountError> { - for &slot_idx in delta.cleared_items.iter() { - self.set_item(slot_idx, Word::default())?; - } - for &(slot_idx, slot_value) in delta.updated_items.iter() { - self.set_item(slot_idx, slot_value)?; - } + // --- update storage maps -------------------------------------------- for &(slot_idx, ref map_delta) in delta.updated_maps.iter() { // layout commitment slot cannot be updated @@ -283,6 +279,18 @@ impl AccountStorage { storage_map.apply_delta(map_delta)?; } + // --- update storage slots ------------------------------------------- + // this will also update roots of updated storage maps, and thus should be run after we + // update storage maps - otherwise the roots won't match + + for &slot_idx in delta.cleared_items.iter() { + self.set_item(slot_idx, Word::default())?; + } + + for &(slot_idx, slot_value) in delta.updated_items.iter() { + self.set_item(slot_idx, slot_value)?; + } + Ok(()) } @@ -318,6 +326,12 @@ impl AccountStorage { actual: value_arity, }); } + + // make sure the value matches the root of the corresponding storage map + // TODO: we should remove handling of storage map updates from set_item(); + // once this is done, these checks would not be necessary + let storage_map = self.maps.get(&index).expect("storage map not found"); + assert_eq!(storage_map.root(), value.into()); }, slot_type => Err(AccountError::StorageSlotArrayNotAllowed(index, slot_type))?, } From ec24a37a38618d12d735f805c77f2c5fddc15bd6 Mon Sep 17 00:00:00 2001 From: Bobbin Threadbare Date: Tue, 11 Jun 2024 14:44:19 -0700 Subject: [PATCH 15/15] chore: pacified clippy --- objects/src/accounts/storage/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/objects/src/accounts/storage/mod.rs b/objects/src/accounts/storage/mod.rs index 78ac6a32c..58235b2a0 100644 --- a/objects/src/accounts/storage/mod.rs +++ b/objects/src/accounts/storage/mod.rs @@ -264,7 +264,6 @@ impl AccountStorage { /// - The updates violate storage layout constraints. /// - The updated value has an arity different from 0. pub(super) fn apply_delta(&mut self, delta: &AccountStorageDelta) -> Result<(), AccountError> { - // --- update storage maps -------------------------------------------- for &(slot_idx, ref map_delta) in delta.updated_maps.iter() {