From 267e9075bc1a52324bc7751421f5c9fbca6913cf Mon Sep 17 00:00:00 2001 From: polydez <155382956+polydez@users.noreply.github.com> Date: Thu, 22 Aug 2024 11:51:37 +0500 Subject: [PATCH] Refactored delta's vault and storage to use maps (#822) * refactor: use maps in storage and vault deltas * docs: update CHANGELOG.md * refactor: address review comments * tests: fix compilation errors * refactor: address review comments * refactor: address review comments --- CHANGELOG.md | 1 + miden-lib/src/transaction/errors.rs | 20 +- miden-tx/src/host/account_delta_tracker.rs | 235 +------- miden-tx/src/host/mod.rs | 20 +- miden-tx/src/tests/mod.rs | 81 ++- objects/src/accounts/delta/builder.rs | 57 -- objects/src/accounts/delta/mod.rs | 56 +- objects/src/accounts/delta/storage.rs | 504 +++++++--------- objects/src/accounts/delta/vault.rs | 637 ++++++++++++--------- objects/src/accounts/mod.rs | 33 +- objects/src/accounts/storage/map.rs | 20 +- objects/src/accounts/storage/mod.rs | 19 +- objects/src/assets/nonfungible.rs | 21 +- objects/src/assets/vault.rs | 36 +- objects/src/errors.rs | 18 +- objects/src/testing/storage.rs | 45 +- 16 files changed, 806 insertions(+), 997 deletions(-) delete mode 100644 objects/src/accounts/delta/builder.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e4a01a80..212514265 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ - Added serialization and equality comparison for `TransactionScript` (#824). - [BREAKING] Migrated to Miden VM v0.10 (#826). - Added conversions for `NoteExecutionHint` (#827). +- [BREAKING] Refactored account storage and vault deltas (#822). ## 0.4.0 (2024-07-03) diff --git a/miden-lib/src/transaction/errors.rs b/miden-lib/src/transaction/errors.rs index 6a9827435..eade4d37a 100644 --- a/miden-lib/src/transaction/errors.rs +++ b/miden-lib/src/transaction/errors.rs @@ -2,8 +2,8 @@ use alloc::{string::String, vec::Vec}; use core::fmt; use miden_objects::{ - accounts::AccountStorage, notes::NoteMetadata, AccountError, AssetError, Digest, Felt, - NoteError, + accounts::AccountStorage, notes::NoteMetadata, AccountDeltaError, AccountError, AssetError, + Digest, Felt, NoteError, }; // TRANSACTION KERNEL ERROR @@ -11,6 +11,7 @@ use miden_objects::{ #[derive(Debug, Clone, Eq, PartialEq)] pub enum TransactionKernelError { + AccountDeltaError(AccountDeltaError), FailedToAddAssetToNote(NoteError), InvalidNoteInputs { expected: Digest, @@ -39,7 +40,7 @@ impl fmt::Display for TransactionKernelError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { TransactionKernelError::FailedToAddAssetToNote(err) => { - write!(f, "failed to add asset to note: {err}") + write!(f, "Failed to add asset to note: {err}") }, TransactionKernelError::InvalidNoteInputs { expected, got, data } => { write!( @@ -50,7 +51,7 @@ impl fmt::Display for TransactionKernelError { }, TransactionKernelError::InvalidStorageSlotIndex(index) => { let num_slots = AccountStorage::NUM_STORAGE_SLOTS; - write!(f, "storage slot index {index} is invalid, must be smaller than {num_slots}") + write!(f, "Storage slot index {index} is invalid, must be smaller than {num_slots}") }, TransactionKernelError::MalformedAccountId(err) => { write!( f, "Account id data extracted from the stack by the event handler is not well formed {err}") @@ -59,7 +60,7 @@ impl fmt::Display for TransactionKernelError { write!(f, "Asset data extracted from the stack by the event handler is not well formed {err}") }, TransactionKernelError::MalformedAssetOnAccountVaultUpdate(err) => { - write!(f, "malformed asset during account vault update: {err}") + write!(f, "Malformed asset during account vault update: {err}") }, TransactionKernelError::MalformedNoteInputs(err) => { write!( f, "Note inputs data extracted from the advice map by the event handler is not well formed {err}") @@ -89,16 +90,19 @@ impl fmt::Display for TransactionKernelError { write!(f, "Public note missing or incomplete inputs in the advice provider") }, TransactionKernelError::MissingStorageSlotValue(index, err) => { - write!(f, "value for storage slot {index} could not be found: {err}") + write!(f, "Value for storage slot {index} could not be found: {err}") }, TransactionKernelError::TooFewElementsForNoteInputs => { write!( f, - "note input data in advice provider contains fewer elements than specified by its inputs length" + "Note input data in advice provider contains fewer elements than specified by its inputs length" ) }, TransactionKernelError::UnknownAccountProcedure(proc_root) => { - write!(f, "account procedure with root {proc_root} is not in the advice provider") + write!(f, "Account procedure with root {proc_root} is not in the advice provider") + }, + TransactionKernelError::AccountDeltaError(error) => { + write!(f, "Account delta error: {error}") }, } } diff --git a/miden-tx/src/host/account_delta_tracker.rs b/miden-tx/src/host/account_delta_tracker.rs index 1b019808a..7d41107f2 100644 --- a/miden-tx/src/host/account_delta_tracker.rs +++ b/miden-tx/src/host/account_delta_tracker.rs @@ -1,14 +1,7 @@ -use alloc::{collections::BTreeMap, vec::Vec}; - use miden_objects::{ - accounts::{ - AccountDelta, AccountId, AccountStorageDelta, AccountStub, AccountVaultDelta, - StorageMapDelta, - }, - assets::{Asset, FungibleAsset, NonFungibleAsset}, - Digest, Felt, Word, EMPTY_WORD, ZERO, + accounts::{AccountDelta, AccountStorageDelta, AccountStub, AccountVaultDelta}, + Felt, ZERO, }; - // ACCOUNT DELTA TRACKER // ================================================================================================ @@ -24,8 +17,8 @@ use miden_objects::{ /// - account code changes. #[derive(Debug, Clone, PartialEq, Eq)] pub struct AccountDeltaTracker { - storage: AccountStorageDeltaTracker, - vault: AccountVaultDeltaTracker, + storage: AccountStorageDelta, + vault: AccountVaultDelta, init_nonce: Felt, nonce_delta: Felt, } @@ -34,8 +27,8 @@ impl AccountDeltaTracker { /// Returns a new [AccountDeltaTracker] instantiated for the specified account. pub fn new(account: &AccountStub) -> Self { Self { - storage: AccountStorageDeltaTracker::default(), - vault: AccountVaultDeltaTracker::default(), + storage: AccountStorageDelta::default(), + vault: AccountVaultDelta::default(), init_nonce: account.nonce(), nonce_delta: ZERO, } @@ -43,15 +36,9 @@ impl AccountDeltaTracker { /// Consumes `self` and returns the resulting [AccountDelta]. pub fn into_delta(self) -> AccountDelta { - let storage_delta = self.storage.into_delta(); - let vault_delta = self.vault.into_delta(); - let nonce_delta = if self.nonce_delta == ZERO { - None - } else { - Some(self.init_nonce + self.nonce_delta) - }; + let nonce_delta = (self.nonce_delta != ZERO).then_some(self.init_nonce + self.nonce_delta); - AccountDelta::new(storage_delta, vault_delta, nonce_delta).expect("invalid account delta") + AccountDelta::new(self.storage, self.vault, nonce_delta).expect("invalid account delta") } /// Tracks nonce delta. @@ -59,211 +46,13 @@ impl AccountDeltaTracker { self.nonce_delta += value; } - /// Get the vault tracker - pub fn vault_tracker(&mut self) -> &mut AccountVaultDeltaTracker { + /// Get a mutable reference to the current vault delta + pub fn vault_delta(&mut self) -> &mut AccountVaultDelta { &mut self.vault } - /// Get the storage tracker - pub fn storage_tracker(&mut self) -> &mut AccountStorageDeltaTracker { + /// Get a mutable reference to the current storage delta + pub fn storage_delta(&mut self) -> &mut AccountStorageDelta { &mut self.storage } } - -// ACCOUNT STORAGE DELTA TRACKER -// ================================================================================================ - -/// The account storage delta tracker is responsible for tracking changes to the storage of the -/// account the transaction is being executed against. -/// -/// The delta tracker is composed of: -/// - A map which records the latest states for the updated storage slots. -/// - A map which records the latest states for the updates storage maps -#[derive(Default, Debug, Clone, PartialEq, Eq)] -pub struct AccountStorageDeltaTracker { - slot_updates: BTreeMap, - maps_updates: BTreeMap>, -} - -impl AccountStorageDeltaTracker { - /// Consumes `self` and returns the [AccountStorageDelta] that represents the changes to - /// the account's storage. - pub fn into_delta(self) -> AccountStorageDelta { - let mut cleared_items = Vec::new(); - let mut updated_items = Vec::new(); - let mut updated_maps: Vec<(u8, StorageMapDelta)> = Vec::new(); - - for (idx, value) in self.slot_updates { - if value == EMPTY_WORD { - cleared_items.push(idx); - } else { - updated_items.push((idx, value)); - } - } - - for (idx, map_deltas) in self.maps_updates { - let mut updated_leafs = Vec::new(); - let mut cleared_leafs = Vec::new(); - - for map_delta in map_deltas { - if map_delta.1 == EMPTY_WORD { - cleared_leafs.push(map_delta.0); - } else { - updated_leafs.push(map_delta); - } - } - let storage_map_delta = StorageMapDelta::from(cleared_leafs, updated_leafs); - updated_maps.push((idx, storage_map_delta)); - } - - AccountStorageDelta { - cleared_items, - updated_items, - updated_maps, - } - } - - /// Tracks a slot change - pub fn slot_update(&mut self, slot_index: u8, new_slot_value: [Felt; 4]) { - self.slot_updates.insert(slot_index, new_slot_value); - } - - /// Tracks a slot change - pub fn maps_update(&mut self, slot_index: u8, key: [Felt; 4], new_value: [Felt; 4]) { - self.maps_updates.entry(slot_index).or_default().push((key, new_value)); - } -} - -// ACCOUNT VAULT DELTA TRACKER -// ================================================================================================ - -/// The account vault delta tracker is responsible for tracking changes to the vault of the account -/// the transaction is being executed against. -/// -/// The delta tracker is composed of two maps: -/// - Fungible asset map: tracks changes to the vault's fungible assets, where the key is the -/// faucet ID of the asset, and the value is the amount of the asset being added or removed from -/// the vault (positive value for added assets, negative value for removed assets). -/// - Non-fungible asset map: tracks changes to the vault's non-fungible assets, where the key is -/// the non-fungible asset, and the value is either 1 or -1 depending on whether the asset is -/// being added or removed from the vault. -#[derive(Default, Debug, Clone, PartialEq, Eq)] -pub struct AccountVaultDeltaTracker { - fungible_assets: BTreeMap, - non_fungible_assets: BTreeMap, -} - -impl AccountVaultDeltaTracker { - // STATE MUTATORS - // -------------------------------------------------------------------------------------------- - - pub fn add_asset(&mut self, asset: Asset) { - match asset { - Asset::Fungible(asset) => { - update_asset_delta( - &mut self.fungible_assets, - asset.faucet_id(), - asset.amount() as i128, - ); - }, - Asset::NonFungible(asset) => { - update_asset_delta(&mut self.non_fungible_assets, asset.vault_key().into(), 1) - }, - } - } - - /// Track asset removal. - pub fn remove_asset(&mut self, asset: Asset) { - match asset { - Asset::Fungible(asset) => { - update_asset_delta( - &mut self.fungible_assets, - asset.faucet_id(), - -(asset.amount() as i128), - ); - }, - Asset::NonFungible(asset) => { - update_asset_delta(&mut self.non_fungible_assets, asset.vault_key().into(), -1) - }, - } - } - - // CONVERSIONS - // -------------------------------------------------------------------------------------------- - - /// Consumes `self` and returns the [AccountVaultDelta] that represents the changes to the - /// account's vault. - pub fn into_delta(self) -> AccountVaultDelta { - let mut added_assets = Vec::new(); - let mut removed_assets = Vec::new(); - - // process fungible assets - for (faucet_id, amount) in self.fungible_assets { - if amount > 0 { - added_assets.push(Asset::Fungible( - FungibleAsset::new( - AccountId::new_unchecked(faucet_id.into()), - amount.unsigned_abs() as u64, - ) - .expect("fungible asset is well formed"), - )); - } else { - removed_assets.push(Asset::Fungible( - FungibleAsset::new( - AccountId::new_unchecked(faucet_id.into()), - amount.unsigned_abs() as u64, - ) - .expect("fungible asset is well formed"), - )); - } - } - - // process non-fungible assets - for (non_fungible_asset, amount) in self.non_fungible_assets { - match amount { - 1 => { - added_assets.push(Asset::NonFungible(unsafe { - NonFungibleAsset::new_unchecked(*non_fungible_asset) - })); - }, - -1 => { - removed_assets.push(Asset::NonFungible(unsafe { - NonFungibleAsset::new_unchecked(*non_fungible_asset) - })); - }, - _ => unreachable!("non-fungible asset amount must be 1 or -1"), - } - } - - AccountVaultDelta { added_assets, removed_assets } - } -} - -// HELPER FUNCTIONS -// ================================================================================================ - -/// Updates the provided map with the provided key and amount. If the final amount is 0, the entry -/// is removed from the map. -fn update_asset_delta(delta_map: &mut BTreeMap, key: K, amount: V) -where - V: core::ops::Neg, - V: core::cmp::PartialEq<::Output>, - V: core::ops::AddAssign, - V: Copy, - K: Ord, -{ - use alloc::collections::btree_map::Entry; - - match delta_map.entry(key) { - Entry::Occupied(mut entry) => { - if entry.get() == &-amount { - entry.remove(); - } else { - *entry.get_mut() += amount; - } - }, - Entry::Vacant(entry) => { - entry.insert(amount); - }, - } -} diff --git a/miden-tx/src/host/mod.rs b/miden-tx/src/host/mod.rs index 5e80f7c43..cddb022ca 100644 --- a/miden-tx/src/host/mod.rs +++ b/miden-tx/src/host/mod.rs @@ -244,7 +244,7 @@ impl TransactionHost { // update the delta tracker only if the current and new values are different if current_slot_value != new_slot_value { let slot_index = slot_index.as_int() as u8; - self.account_delta.storage_tracker().slot_update(slot_index, new_slot_value); + self.account_delta.storage_delta().set_item(slot_index, new_slot_value); } Ok(()) @@ -281,9 +281,11 @@ impl TransactionHost { ]; let slot_index = slot_index.as_int() as u8; - self.account_delta - .storage_tracker() - .maps_update(slot_index, new_map_key, new_map_value); + self.account_delta.storage_delta().set_map_item( + slot_index, + new_map_key.into(), + new_map_value, + ); Ok(()) } @@ -304,7 +306,10 @@ impl TransactionHost { .try_into() .map_err(TransactionKernelError::MalformedAssetOnAccountVaultUpdate)?; - self.account_delta.vault_tracker().add_asset(asset); + self.account_delta + .vault_delta() + .add_asset(asset) + .map_err(TransactionKernelError::AccountDeltaError)?; Ok(()) } @@ -321,7 +326,10 @@ impl TransactionHost { .try_into() .map_err(TransactionKernelError::MalformedAssetOnAccountVaultUpdate)?; - self.account_delta.vault_tracker().remove_asset(asset); + self.account_delta + .vault_delta() + .remove_asset(asset) + .map_err(TransactionKernelError::AccountDeltaError)?; Ok(()) } diff --git a/miden-tx/src/tests/mod.rs b/miden-tx/src/tests/mod.rs index d41a9fb73..b9b19efd3 100644 --- a/miden-tx/src/tests/mod.rs +++ b/miden-tx/src/tests/mod.rs @@ -1,4 +1,4 @@ -use alloc::{rc::Rc, string::String, vec::Vec}; +use alloc::{collections::BTreeMap, rc::Rc, string::String, vec::Vec}; use miden_lib::transaction::TransactionKernel; use miden_objects::{ @@ -334,24 +334,24 @@ fn executed_transaction_account_delta() { // storage delta // -------------------------------------------------------------------------------------------- // We expect one updated item and one updated map - assert_eq!(executed_transaction.account_delta().storage().updated_items.len(), 1); + assert_eq!(executed_transaction.account_delta().storage().slots().len(), 1); assert_eq!( - executed_transaction.account_delta().storage().updated_items[0].0, - STORAGE_INDEX_0 - ); - assert_eq!( - executed_transaction.account_delta().storage().updated_items[0].1, - updated_slot_value + executed_transaction.account_delta().storage().slots().get(&STORAGE_INDEX_0), + Some(&updated_slot_value) ); - assert_eq!(executed_transaction.account_delta().storage().updated_maps.len(), 1); - assert_eq!( - executed_transaction.account_delta().storage().updated_maps[0].0, - STORAGE_INDEX_2 - ); + assert_eq!(executed_transaction.account_delta().storage().maps().len(), 1); assert_eq!( - executed_transaction.account_delta().storage().updated_maps[0].1.updated_leaves[0], - (updated_map_key, updated_map_value) + executed_transaction + .account_delta() + .storage() + .maps() + .get(&STORAGE_INDEX_2) + .unwrap() + .leaves(), + &Some((updated_map_key.into(), updated_map_value)) + .into_iter() + .collect::>() ); // vault delta @@ -372,24 +372,22 @@ fn executed_transaction_account_delta() { assert!(executed_transaction .account_delta() .vault() - .added_assets - .iter() - .all(|x| added_assets.contains(x))); + .added_assets() + .all(|x| added_assets.contains(&x))); assert_eq!( added_assets.len(), - executed_transaction.account_delta().vault().added_assets.len() + executed_transaction.account_delta().vault().added_assets().count() ); // assert that removed assets are tracked assert!(executed_transaction .account_delta() .vault() - .removed_assets - .iter() - .all(|x| removed_assets.contains(x))); + .removed_assets() + .all(|x| removed_assets.contains(&x))); assert_eq!( removed_assets.len(), - executed_transaction.account_delta().vault().removed_assets.len() + executed_transaction.account_delta().vault().removed_assets().count() ); } @@ -443,9 +441,9 @@ fn test_empty_delta_nonce_update() { // storage delta // -------------------------------------------------------------------------------------------- // We expect one updated item and one updated map - assert_eq!(executed_transaction.account_delta().storage().updated_items.len(), 0); + assert_eq!(executed_transaction.account_delta().storage().slots().len(), 0); - assert_eq!(executed_transaction.account_delta().storage().updated_maps.len(), 0); + assert_eq!(executed_transaction.account_delta().storage().maps().len(), 0); } #[test] @@ -505,7 +503,7 @@ fn test_send_note_proc() { # prepare the stack for the next call dropw - # push the asset to be removed + # push the asset to be removed push.{ASSET} # => [ASSET, note_idx, GARBAGE(11)] @@ -520,17 +518,17 @@ fn test_send_note_proc() { use.miden::account use.miden::contracts::wallets::basic->wallet use.miden::tx - + ## ACCOUNT PROCEDURE WRAPPERS ## ======================================================================================== proc.incr_nonce call.{ACCOUNT_INCR_NONCE_MAST_ROOT} # => [0] - + drop # => [] end - + ## TRANSACTION SCRIPT ## ======================================================================================== begin @@ -553,9 +551,9 @@ fn test_send_note_proc() { # => [GARBAGE(4), note_idx, GARBAGE(11)] {assets_to_remove} - + dropw dropw dropw dropw - + ## Update the account nonce ## ------------------------------------------------------------------------------------ push.1 exec.incr_nonce drop @@ -597,12 +595,11 @@ fn test_send_note_proc() { assert!(executed_transaction .account_delta() .vault() - .removed_assets - .iter() - .all(|x| removed_assets.contains(x))); + .removed_assets() + .all(|x| removed_assets.contains(&x))); assert_eq!( removed_assets.len(), - executed_transaction.account_delta().vault().removed_assets.len() + executed_transaction.account_delta().vault().removed_assets().count() ); } } @@ -704,29 +701,29 @@ fn executed_transaction_output_notes() { ## ACCOUNT PROCEDURE WRAPPERS ## ======================================================================================== proc.create_note - # pad the stack before the syscall to prevent accidental modification of the deeper stack - # elements + # pad the stack before the syscall to prevent accidental modification of the deeper stack + # elements padw padw swapdw # => [tag, aux, execution_hint, note_type, RECIPIENT, PAD(8)] call.wallet::create_note # => [note_idx, PAD(15)] - # remove excess PADs from the stack + # remove excess PADs from the stack swapdw dropw dropw movdn.7 dropw drop drop drop # => [note_idx] end proc.add_asset_to_note - # pad the stack before the syscall to prevent accidental modification of the deeper stack - # elements + # pad the stack before the syscall to prevent accidental modification of the deeper stack + # elements push.0.0.0 padw padw swapdw movup.7 swapw # => [ASSET, note_idx, PAD(11)] call.{ACCOUNT_ADD_ASSET_TO_NOTE_MAST_ROOT} # => [ASSET, note_idx, PAD(11)] - # remove excess PADs from the stack + # remove excess PADs from the stack swapdw dropw dropw swapw movdn.7 drop drop drop # => [ASSET, note_idx] @@ -762,7 +759,7 @@ fn executed_transaction_output_notes() { # => [note_idx] push.{REMOVED_ASSET_1} # asset - exec.remove_asset + exec.remove_asset # => [ASSET, note_ptr] exec.add_asset_to_note # => [note_idx] diff --git a/objects/src/accounts/delta/builder.rs b/objects/src/accounts/delta/builder.rs deleted file mode 100644 index 52b340a58..000000000 --- a/objects/src/accounts/delta/builder.rs +++ /dev/null @@ -1,57 +0,0 @@ -use alloc::vec::Vec; - -use super::{AccountStorageDelta, StorageMapDelta, Word}; -use crate::AccountDeltaError; - -#[derive(Clone, Debug, Default)] -pub struct AccountStorageDeltaBuilder { - pub cleared_items: Vec, - pub updated_items: Vec<(u8, Word)>, - pub updated_maps: Vec<(u8, StorageMapDelta)>, -} - -impl AccountStorageDeltaBuilder { - // CONSTRUCTORS - // ------------------------------------------------------------------------------------------- - pub fn new() -> Self { - Self::default() - } - - // MODIFIERS - // ------------------------------------------------------------------------------------------- - pub fn add_cleared_items(mut self, items: I) -> Self - where - I: IntoIterator, - { - self.cleared_items.extend(items); - self - } - - pub fn add_updated_items(mut self, items: I) -> Self - where - I: IntoIterator, - { - self.updated_items.extend(items); - self - } - - pub fn add_updated_maps(mut self, items: I) -> Self - where - I: IntoIterator, - { - self.updated_maps.extend(items); - self - } - - // BUILDERS - // ------------------------------------------------------------------------------------------- - pub fn build(self) -> Result { - let delta = AccountStorageDelta { - cleared_items: self.cleared_items, - updated_items: self.updated_items, - updated_maps: self.updated_maps, - }; - delta.validate()?; - Ok(delta) - } -} diff --git a/objects/src/accounts/delta/mod.rs b/objects/src/accounts/delta/mod.rs index c1b0e50a8..8702fca9b 100644 --- a/objects/src/accounts/delta/mod.rs +++ b/objects/src/accounts/delta/mod.rs @@ -4,16 +4,15 @@ use super::{ Account, ByteReader, ByteWriter, Deserializable, DeserializationError, Felt, Serializable, Word, ZERO, }; -use crate::{assets::Asset, AccountDeltaError}; - -mod builder; -pub use builder::AccountStorageDeltaBuilder; +use crate::AccountDeltaError; mod storage; pub use storage::{AccountStorageDelta, StorageMapDelta}; mod vault; -pub use vault::AccountVaultDelta; +pub use vault::{ + AccountVaultDelta, FungibleAssetDelta, NonFungibleAssetDelta, NonFungibleDeltaAction, +}; // ACCOUNT DELTA // ================================================================================================ @@ -39,18 +38,13 @@ impl AccountDelta { /// Returns new [AccountDelta] instantiated from the provided components. /// /// # Errors - /// Returns an error if: - /// - Storage or vault deltas are invalid. - /// - Storage or vault deltas are not empty, but nonce was not updated. + /// Returns an error if storage or vault were updated, but the nonce was either not updated + /// or set to 0. pub fn new( storage: AccountStorageDelta, vault: AccountVaultDelta, nonce: Option, ) -> Result { - // make sure storage and vault deltas are valid - storage.validate()?; - vault.validate()?; - // nonce must be updated if either account storage or vault were updated validate_nonce(nonce, &storage, &vault)?; @@ -58,19 +52,18 @@ impl AccountDelta { } /// Merge another [AccountDelta] into this one. - pub fn merge(self, other: Self) -> Result { - let nonce = match (self.nonce, other.nonce) { + pub fn merge(&mut self, other: Self) -> Result<(), AccountDeltaError> { + match (&mut self.nonce, other.nonce) { (Some(old), Some(new)) if new.as_int() <= old.as_int() => { return Err(AccountDeltaError::InconsistentNonceUpdate(format!( "New nonce {new} is not larger than the old nonce {old}" ))) }, // Incoming nonce takes precedence. - (old, new) => new.or(old), + (old, new) => *old = new.or(*old), }; - let storage = self.storage.merge(other.storage)?; - let vault = self.vault.merge(other.vault)?; - Self::new(storage, vault, nonce) + self.storage.merge(other.storage)?; + self.vault.merge(other.vault) } // PUBLIC ACCESSORS @@ -138,8 +131,9 @@ impl AccountUpdateDetails { AccountUpdateDetails::New(account) }, - (AccountUpdateDetails::Delta(initial), AccountUpdateDetails::Delta(new_delta)) => { - AccountUpdateDetails::Delta(initial.merge(new_delta)?) + (AccountUpdateDetails::Delta(mut delta), AccountUpdateDetails::Delta(new_delta)) => { + delta.merge(new_delta)?; + AccountUpdateDetails::Delta(delta) }, (left, right) => { return Err(AccountDeltaError::IncompatibleAccountUpdates(left, right)) @@ -211,8 +205,8 @@ impl Deserializable for AccountUpdateDetails { /// Checks if the nonce was updated correctly given the provided storage and vault deltas. /// /// # Errors -/// Returns an error if: -/// - Storage or vault were updated, but the nonce was either not updated or set to 0. +/// Returns an error if storage or vault were updated, but the nonce was either not updated +/// or set to 0. fn validate_nonce( nonce: Option, storage: &AccountStorageDelta, @@ -249,26 +243,14 @@ mod tests { #[test] fn account_delta_nonce_validation() { // empty delta - let storage_delta = AccountStorageDelta { - cleared_items: vec![], - updated_items: vec![], - updated_maps: vec![], - }; - - let vault_delta = AccountVaultDelta { - added_assets: vec![], - removed_assets: vec![], - }; + let storage_delta = AccountStorageDelta::default(); + let vault_delta = AccountVaultDelta::default(); assert!(AccountDelta::new(storage_delta.clone(), vault_delta.clone(), None).is_ok()); assert!(AccountDelta::new(storage_delta.clone(), vault_delta.clone(), Some(ONE)).is_ok()); // non-empty delta - let storage_delta = AccountStorageDelta { - cleared_items: vec![1], - updated_items: vec![], - updated_maps: vec![], - }; + let storage_delta = AccountStorageDelta::from_iters([1], [], []); assert!(AccountDelta::new(storage_delta.clone(), vault_delta.clone(), None).is_err()); assert!(AccountDelta::new(storage_delta.clone(), vault_delta.clone(), Some(ZERO)).is_err()); diff --git a/objects/src/accounts/delta/storage.rs b/objects/src/accounts/delta/storage.rs index 4378f0a27..f8244d75d 100644 --- a/objects/src/accounts/delta/storage.rs +++ b/objects/src/accounts/delta/storage.rs @@ -1,243 +1,174 @@ -use alloc::{collections::BTreeMap, string::ToString, vec::Vec}; +use alloc::{ + collections::{btree_map::Entry, BTreeMap}, + string::ToString, + vec::Vec, +}; + +use miden_crypto::EMPTY_WORD; use super::{ - AccountDeltaError, ByteReader, ByteWriter, Deserializable, DeserializationError, Felt, - Serializable, Word, + AccountDeltaError, ByteReader, ByteWriter, Deserializable, DeserializationError, Serializable, + Word, }; use crate::Digest; // CONSTANTS // ================================================================================================ -const MAX_MUTABLE_STORAGE_SLOT_IDX: u8 = 254; +const IMMUTABLE_STORAGE_SLOT: u8 = u8::MAX; // ACCOUNT STORAGE DELTA // ================================================================================================ /// [AccountStorageDelta] stores the differences between two states of account storage. /// -/// The differences are represented as follows: -/// - item updates: represented by `cleared_items` and `updated_items` field. +/// The delta consists of two maps: +/// - A map containing the updates to simple storage slots. The keys in this map are indexes of the +/// updated storage slots and the values are the new values for these slots. +/// - A map containing updates to storage maps. The keys in this map are also indexes of the +/// updated storage slots and the values are corresponding storage map delta objects. #[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct AccountStorageDelta { - pub cleared_items: Vec, - pub updated_items: Vec<(u8, Word)>, - pub updated_maps: Vec<(u8, StorageMapDelta)>, + slots: BTreeMap, + maps: BTreeMap, } impl AccountStorageDelta { - /// Creates an [AccountStorageDelta] from the given iterators. - #[cfg(test)] - fn from_iters(cleared_items: A, updated_items: B, updated_maps: C) -> Self - where - A: IntoIterator, - B: IntoIterator, - C: IntoIterator, - { - Self { - cleared_items: Vec::from_iter(cleared_items), - updated_items: Vec::from_iter(updated_items), - updated_maps: Vec::from_iter(updated_maps), - } + /// Creates a new storage delta from the provided fields. + pub fn new( + slots: BTreeMap, + maps: BTreeMap, + ) -> Result { + let result = Self { slots, maps }; + result.validate()?; + + Ok(result) } - /// Merges another delta into this one, overwriting any existing values. - pub fn merge(self, other: Self) -> Result { - self.validate()?; - other.validate()?; + /// Returns a reference to the updated slots in this storage delta. + pub fn slots(&self) -> &BTreeMap { + &self.slots + } - let items = - self.cleared_items - .into_iter() - .map(|slot| (slot, None)) - .chain(self.updated_items.into_iter().map(|(slot, value)| (slot, Some(value)))) - .chain(other.cleared_items.into_iter().map(|slot| (slot, None)).chain( - other.updated_items.into_iter().map(|(slot, value)| (slot, Some(value))), - )) - .collect::>(); - - let cleared_items = items - .iter() - .filter_map(|(slot, value)| value.is_none().then_some(*slot)) - .collect(); - let updated_items = - items.iter().filter_map(|(slot, value)| value.map(|v| (*slot, v))).collect(); + /// Returns a reference to the updated maps in this storage delta. + pub fn maps(&self) -> &BTreeMap { + &self.maps + } - let mut updated_maps = BTreeMap::::new(); - for (slot, update) in self.updated_maps.into_iter().chain(other.updated_maps.into_iter()) { - let entry = updated_maps.entry(slot).or_default(); - *entry = entry.clone().merge(update); - } + /// Returns true if storage delta contains no updates. + pub fn is_empty(&self) -> bool { + self.slots.is_empty() && self.maps.is_empty() + } - let updated_maps = updated_maps.into_iter().collect(); + /// Tracks a slot change + pub fn set_item(&mut self, slot_index: u8, new_slot_value: Word) { + self.slots.insert(slot_index, new_slot_value); + } - let result = Self { - cleared_items, - updated_items, - updated_maps, - }; - result.validate()?; + /// Tracks a map item change + pub fn set_map_item(&mut self, slot_index: u8, key: Digest, new_value: Word) { + self.maps.entry(slot_index).or_default().insert(key, new_value); + } - Ok(result) + /// Merges another delta into this one, overwriting any existing values. + pub fn merge(&mut self, other: Self) -> Result<(), AccountDeltaError> { + self.slots.extend(other.slots); + + // merge maps + for (slot, update) in other.maps.into_iter() { + match self.maps.entry(slot) { + Entry::Vacant(entry) => { + entry.insert(update); + }, + Entry::Occupied(mut entry) => entry.get_mut().merge(update), + } + } + + self.validate() } /// Checks whether this storage delta is valid. /// /// # Errors /// Returns an error if: - /// - The number of cleared or updated items is greater than 255. - /// - Any of cleared or updated items are at slot 255 (i.e., immutable slot). - /// - Any of the cleared or updated items is referenced more than once (e.g., updated twice). - /// - There is a storage map delta without a corresponding storage item update. - pub fn validate(&self) -> Result<(), AccountDeltaError> { - let num_cleared_items = self.cleared_items.len(); - let num_updated_items = self.updated_items.len(); - - if num_cleared_items > u8::MAX as usize { - return Err(AccountDeltaError::TooManyClearedStorageItems { - actual: num_cleared_items, - max: u8::MAX as usize, - }); - } else if num_updated_items > u8::MAX as usize { - return Err(AccountDeltaError::TooManyRemovedAssets { - actual: num_updated_items, - max: u8::MAX as usize, - }); + /// - Any of updated items are at slot 255 (i.e., immutable slot). + /// - Any of the updated slot is referenced from both maps (e.g., updated twice). + fn validate(&self) -> Result<(), AccountDeltaError> { + if self.slots.contains_key(&IMMUTABLE_STORAGE_SLOT) + || self.maps.contains_key(&IMMUTABLE_STORAGE_SLOT) + { + return Err(AccountDeltaError::ImmutableStorageSlot(IMMUTABLE_STORAGE_SLOT as usize)); } - // make sure cleared items vector does not contain errors - for (pos, &idx) in self.cleared_items.iter().enumerate() { - if idx > MAX_MUTABLE_STORAGE_SLOT_IDX { - return Err(AccountDeltaError::ImmutableStorageSlot(idx as usize)); - } - - if self.cleared_items[..pos].contains(&idx) { - return Err(AccountDeltaError::DuplicateStorageItemUpdate(idx as usize)); - } - } - - // make sure updates items vector does not contain errors - for (pos, (idx, _)) in self.updated_items.iter().enumerate() { - if *idx > MAX_MUTABLE_STORAGE_SLOT_IDX { - return Err(AccountDeltaError::ImmutableStorageSlot(*idx as usize)); - } - - if self.cleared_items.contains(idx) { - return Err(AccountDeltaError::DuplicateStorageItemUpdate(*idx as usize)); - } - - if self.updated_items[..pos].iter().any(|x| x.0 == *idx) { - return Err(AccountDeltaError::DuplicateStorageItemUpdate(*idx as usize)); - } - } - - // make sure storage map deltas are valid - for (index, storage_map_delta) in self.updated_maps.iter() { - if index > &MAX_MUTABLE_STORAGE_SLOT_IDX { - return Err(AccountDeltaError::ImmutableStorageSlot(*index as usize)); - } - if !storage_map_delta.is_empty() { - storage_map_delta.validate()?; + for slot in self.maps.keys() { + if self.slots.contains_key(slot) { + return Err(AccountDeltaError::DuplicateStorageItemUpdate(*slot as usize)); } } Ok(()) } +} - /// Returns true if storage delta contains no updates. - pub fn is_empty(&self) -> bool { - self.cleared_items.is_empty() - && self.updated_items.is_empty() - && self.updated_maps.is_empty() +#[cfg(any(feature = "testing", test))] +impl AccountStorageDelta { + /// Creates an [AccountStorageDelta] from the given iterators. + pub fn from_iters( + cleared_items: impl IntoIterator, + updated_items: impl IntoIterator, + updated_maps: impl IntoIterator, + ) -> Self { + Self { + slots: BTreeMap::from_iter( + cleared_items.into_iter().map(|key| (key, EMPTY_WORD)).chain(updated_items), + ), + maps: BTreeMap::from_iter(updated_maps), + } } } impl Serializable for AccountStorageDelta { fn write_into(&self, target: &mut W) { - assert!(self.cleared_items.len() <= u8::MAX as usize, "too many cleared storage items"); - target.write_u8(self.cleared_items.len() as u8); - target.write_many(self.cleared_items.iter()); + let cleared: Vec = self + .slots + .iter() + .filter(|&(_, value)| (value == &EMPTY_WORD)) + .map(|(slot, _)| *slot) + .collect(); + let updated: Vec<_> = + self.slots.iter().filter(|&(_, value)| value != &EMPTY_WORD).collect(); + + target.write_u8(cleared.len() as u8); + target.write_many(cleared.iter()); - assert!(self.updated_items.len() <= u8::MAX as usize, "too many updated storage items"); - target.write_u8(self.updated_items.len() as u8); - target.write_many(self.updated_items.iter()); + target.write_u8(updated.len() as u8); + target.write_many(updated.iter()); - assert!(self.updated_maps.len() <= u8::MAX as usize, "too many updated storage maps"); - target.write_u8(self.updated_maps.len() as u8); - target.write_many(self.updated_maps.iter()); + target.write_u8(self.maps.len() as u8); + target.write_many(self.maps.iter()); } } impl Deserializable for AccountStorageDelta { fn read_from(source: &mut R) -> Result { - // deserialize and validate cleared items + let mut slots = BTreeMap::new(); + let num_cleared_items = source.read_u8()? as usize; - let mut cleared_items = Vec::with_capacity(num_cleared_items); for _ in 0..num_cleared_items { - let idx = source.read_u8()?; - - // make sure index is valid - if idx > MAX_MUTABLE_STORAGE_SLOT_IDX { - return Err(DeserializationError::InvalidValue( - "immutable storage item cleared".to_string(), - )); - } - - // make sure the same item hasn't been cleared before - if cleared_items.contains(&idx) { - return Err(DeserializationError::InvalidValue( - "storage item cleared more than once".to_string(), - )); - } - - cleared_items.push(idx); + let cleared_slot = source.read_u8()?; + slots.insert(cleared_slot, EMPTY_WORD); } - // deserialize and validate updated items let num_updated_items = source.read_u8()? as usize; - let mut updated_items: Vec<(u8, Word)> = Vec::with_capacity(num_updated_items); for _ in 0..num_updated_items { - let idx = source.read_u8()?; - let value = Word::read_from(source)?; - - // make sure index is valid - if idx > MAX_MUTABLE_STORAGE_SLOT_IDX { - return Err(DeserializationError::InvalidValue( - "immutable storage item updated".to_string(), - )); - } - - // make sure the same item hasn't been updated before - if updated_items.iter().any(|x| x.0 == idx) { - return Err(DeserializationError::InvalidValue( - "storage item updated more than once".to_string(), - )); - } - - // make sure the storage item hasn't been cleared in the same delta - if cleared_items.contains(&idx) { - return Err(DeserializationError::InvalidValue( - "storage item both cleared and updated".to_string(), - )); - } - - updated_items.push((idx, value)); + let (updated_slot, updated_value) = source.read()?; + slots.insert(updated_slot, updated_value); } - // deserialize and validate storage map deltas - let num_updated_maps = source.read_u8()? as usize; - let mut updated_maps = Vec::with_capacity(num_updated_maps); - for _ in 0..num_updated_maps { - let idx = source.read_u8()?; - let value = StorageMapDelta::read_from(source)?; - updated_maps.push((idx, value)); - } + let num_maps = source.read_u8()? as usize; + let maps = source.read_many::<(u8, StorageMapDelta)>(num_maps)?.into_iter().collect(); - Ok(Self { - cleared_items, - updated_items, - updated_maps, - }) + Self::new(slots, maps).map_err(|err| DeserializationError::InvalidValue(err.to_string())) } } @@ -246,106 +177,91 @@ impl Deserializable for AccountStorageDelta { /// [StorageMapDelta] stores the differences between two states of account storage maps. /// -/// The differences are represented as follows: -/// - leave updates: represented by `cleared_leaves` and `updated_leaves` field. +/// The differences are represented as leaf updates: a map of updated item key ([Digest]) to +/// value ([Word]). For cleared items the value is [EMPTY_WORD]. #[derive(Clone, Debug, Default, PartialEq, Eq)] -pub struct StorageMapDelta { - pub cleared_leaves: Vec, - pub updated_leaves: Vec<(Word, Word)>, -} +pub struct StorageMapDelta(BTreeMap); impl StorageMapDelta { - /// Creates a new [StorageMapDelta] from the provided iteartors. - fn from_iters(cleared_leaves: A, updated_leaves: B) -> Self - where - A: IntoIterator, - B: IntoIterator, - { - Self { - cleared_leaves: Vec::from_iter(cleared_leaves), - updated_leaves: Vec::from_iter(updated_leaves), - } + /// Creates a new storage map delta from the provided leaves. + pub fn new(map: BTreeMap) -> Self { + Self(map) } - /// Creates a new storage map delta from the provided cleared and updated leaves. - pub fn from(cleared_leaves: Vec, updated_leaves: Vec<(Word, Word)>) -> Self { - Self::from_iters(cleared_leaves, updated_leaves) + /// Returns a reference to the updated leaves in this storage map delta. + pub fn leaves(&self) -> &BTreeMap { + &self.0 } - /// Checks whether this storage map delta is valid. - /// - /// # Errors - /// Returns an error if: - /// - Any of the cleared or updated leaves is referenced more than once (e.g., updated twice). - pub fn validate(&self) -> Result<(), AccountDeltaError> { - // we add all keys to a single vector and sort them to check for duplicates - // we don't use a hash set because we want to use no-std compatible code - let mut all_keys: Vec> = self - .cleared_leaves - .iter() - .chain(self.updated_leaves.iter().map(|(key, _)| key)) - .map(|x| x.iter().map(|x| x.as_int()).collect::>()) - .collect(); - - all_keys.sort_unstable(); - - if let Some(key) = all_keys.windows(2).find(|els| els[0] == els[1]) { - let mut iter = key[0].iter().map(|&x| Felt::new(x)); - // we know that the key is 4 elements long - let digest = Word::from([ - iter.next().unwrap(), - iter.next().unwrap(), - iter.next().unwrap(), - iter.next().unwrap(), - ]); - return Err(AccountDeltaError::DuplicateStorageMapLeaf { key: digest.into() }); - } - - Ok(()) + /// Inserts an item into the storage map delta. + pub fn insert(&mut self, key: Digest, value: Word) { + self.0.insert(key, value); } /// Returns true if storage map delta contains no updates. pub fn is_empty(&self) -> bool { - self.cleared_leaves.is_empty() && self.updated_leaves.is_empty() + self.0.is_empty() } /// Merge `other` into this delta, giving precedence to `other`. - pub fn merge(self, other: Self) -> Self { + pub fn merge(&mut self, other: Self) { // Aggregate the changes into a map such that `other` overwrites self. - let leaves = self.cleared_leaves.into_iter().map(|k| (k, None)); - let leaves = leaves - .chain(self.updated_leaves.into_iter().map(|(k, v)| (k, Some(v)))) - .chain(other.cleared_leaves.into_iter().map(|k| (k, None))) - .chain(other.updated_leaves.into_iter().map(|(k, v)| (k, Some(v)))) - .map(|(k, v)| (Digest::from(k), v.map(Digest::from))) - .collect::>(); - - let mut cleared = Vec::new(); - let mut updated = Vec::new(); - - for (key, value) in leaves { - match value { - Some(value) => updated.push((key.into(), value.into())), - None => cleared.push(key.into()), - } - } + self.0.extend(other.0); + } +} - Self::from(cleared, updated) +#[cfg(any(feature = "testing", test))] +impl StorageMapDelta { + /// Creates a new [StorageMapDelta] from the provided iterators. + pub fn from_iters( + cleared_leaves: impl IntoIterator, + updated_leaves: impl IntoIterator, + ) -> Self { + Self(BTreeMap::from_iter( + cleared_leaves + .into_iter() + .map(|key| (key.into(), EMPTY_WORD)) + .chain(updated_leaves.into_iter().map(|(key, value)| (key.into(), value))), + )) } } impl Serializable for StorageMapDelta { fn write_into(&self, target: &mut W) { - self.cleared_leaves.write_into(target); - self.updated_leaves.write_into(target); + let cleared: Vec<&Digest> = self + .0 + .iter() + .filter(|&(_, value)| value == &EMPTY_WORD) + .map(|(key, _)| key) + .collect(); + + let updated: Vec<_> = self.0.iter().filter(|&(_, value)| value != &EMPTY_WORD).collect(); + + target.write_usize(cleared.len()); + target.write_many(cleared.iter()); + + target.write_usize(updated.len()); + target.write_many(updated.iter()); } } impl Deserializable for StorageMapDelta { fn read_from(source: &mut R) -> Result { - let cleared_leaves = Vec::<_>::read_from(source)?; - let updated_leaves = Vec::<_>::read_from(source)?; - Ok(Self { cleared_leaves, updated_leaves }) + let mut map = BTreeMap::new(); + + let cleared_count = source.read_usize()?; + for _ in 0..cleared_count { + let cleared_key = source.read()?; + map.insert(cleared_key, EMPTY_WORD); + } + + let updated_count = source.read_usize()?; + for _ in 0..updated_count { + let (updated_key, updated_value) = source.read()?; + map.insert(updated_key, updated_value); + } + + Ok(Self::new(map)) } } @@ -354,11 +270,9 @@ impl Deserializable for StorageMapDelta { #[cfg(test)] mod tests { - use super::{AccountStorageDelta, Deserializable, Serializable}; use crate::{ - accounts::{delta::AccountStorageDeltaBuilder, StorageMapDelta}, - ONE, ZERO, + accounts::StorageMapDelta, testing::storage::AccountStorageDeltaBuilder, ONE, ZERO, }; #[test] @@ -380,10 +294,6 @@ mod tests { let bytes = delta.to_bytes(); assert!(AccountStorageDelta::read_from_bytes(&bytes).is_err()); - // duplicate in cleared items - let delta = AccountStorageDelta::from_iters([1, 2, 1], [], []); - assert!(delta.validate().is_err()); - let bytes = delta.to_bytes(); assert!(AccountStorageDelta::read_from_bytes(&bytes).is_err()); @@ -398,26 +308,25 @@ mod tests { let bytes = delta.to_bytes(); assert!(AccountStorageDelta::read_from_bytes(&bytes).is_err()); - // duplicate in updated items + let bytes = delta.to_bytes(); + assert!(AccountStorageDelta::read_from_bytes(&bytes).is_err()); + + // duplicate across cleared items and maps let delta = AccountStorageDelta::from_iters( - [], - [ - (4, [ONE, ONE, ONE, ONE]), - (5, [ONE, ONE, ONE, ZERO]), - (4, [ONE, ONE, ZERO, ZERO]), - ], - [], + [1, 2, 3], + [(2, [ONE, ONE, ONE, ONE]), (5, [ONE, ONE, ONE, ZERO])], + [(1, StorageMapDelta::default())], ); assert!(delta.validate().is_err()); let bytes = delta.to_bytes(); assert!(AccountStorageDelta::read_from_bytes(&bytes).is_err()); - // duplicate across cleared and updated items + // duplicate across updated items and maps let delta = AccountStorageDelta::from_iters( - [1, 2, 3], + [1, 3], [(2, [ONE, ONE, ONE, ONE]), (5, [ONE, ONE, ONE, ZERO])], - [], + [(2, StorageMapDelta::default())], ); assert!(delta.validate().is_err()); @@ -436,17 +345,8 @@ mod tests { let storage_delta = AccountStorageDelta::from_iters([], [(2, [ONE, ONE, ONE, ONE])], []); assert!(!storage_delta.is_empty()); - let storage_delta = AccountStorageDelta::from_iters( - [], - [], - [( - 3, - StorageMapDelta { - cleared_leaves: vec![], - updated_leaves: vec![], - }, - )], - ); + let storage_delta = + AccountStorageDelta::from_iters([], [], [(3, StorageMapDelta::default())]); assert!(!storage_delta.is_empty()); } @@ -467,17 +367,8 @@ mod tests { let deserialized = AccountStorageDelta::read_from_bytes(&serialized).unwrap(); assert_eq!(deserialized, storage_delta); - let storage_delta = AccountStorageDelta::from_iters( - [], - [], - [( - 3, - StorageMapDelta { - cleared_leaves: vec![], - updated_leaves: vec![], - }, - )], - ); + let storage_delta = + AccountStorageDelta::from_iters([], [], [(3, StorageMapDelta::default())]); let serialized = storage_delta.to_bytes(); let deserialized = AccountStorageDelta::read_from_bytes(&serialized).unwrap(); assert_eq!(deserialized, storage_delta); @@ -513,20 +404,20 @@ mod tests { const SLOT: u8 = 123; let item = item.map(|x| (SLOT, [vm_core::Felt::new(x), ZERO, ZERO, ZERO])); - AccountStorageDeltaBuilder::new() + AccountStorageDeltaBuilder::default() .add_cleared_items(item.is_none().then_some(SLOT)) .add_updated_items(item) .build() .unwrap() } - let delta_x = create_delta(x); + let mut delta_x = create_delta(x); let delta_y = create_delta(y); let expected = create_delta(expected); - let result = delta_x.merge(delta_y).unwrap(); + delta_x.merge(delta_y).unwrap(); - assert_eq!(result, expected); + assert_eq!(delta_x, expected); } #[rstest::rstest] @@ -538,23 +429,20 @@ mod tests { fn create_delta(value: Option) -> StorageMapDelta { let key = [vm_core::Felt::new(10), ZERO, ZERO, ZERO]; match value { - Some(value) => StorageMapDelta { - updated_leaves: vec![(key, [vm_core::Felt::new(value), ZERO, ZERO, ZERO])], - ..Default::default() - }, - None => StorageMapDelta { - cleared_leaves: vec![key], - ..Default::default() - }, + Some(value) => StorageMapDelta::from_iters( + [], + [(key, [vm_core::Felt::new(value), ZERO, ZERO, ZERO])], + ), + None => StorageMapDelta::from_iters([key], []), } } - let delta_x = create_delta(x); + let mut delta_x = create_delta(x); let delta_y = create_delta(y); let expected = create_delta(expected); - let result = delta_x.merge(delta_y); + delta_x.merge(delta_y); - assert_eq!(result, expected); + assert_eq!(delta_x, expected); } } diff --git a/objects/src/accounts/delta/vault.rs b/objects/src/accounts/delta/vault.rs index 3c0a400dc..bdd3fd549 100644 --- a/objects/src/accounts/delta/vault.rs +++ b/objects/src/accounts/delta/vault.rs @@ -1,12 +1,15 @@ -use alloc::{collections::BTreeMap, string::ToString, vec::Vec}; +use alloc::{ + collections::{btree_map::Entry, BTreeMap}, + string::ToString, + vec::Vec, +}; use super::{ - AccountDeltaError, Asset, ByteReader, ByteWriter, Deserializable, DeserializationError, - Serializable, + AccountDeltaError, ByteReader, ByteWriter, Deserializable, DeserializationError, Serializable, }; use crate::{ - accounts::AccountId, - assets::{FungibleAsset, NonFungibleAsset}, + accounts::{AccountId, AccountType}, + assets::{Asset, FungibleAsset, NonFungibleAsset}, }; // ACCOUNT VAULT DELTA @@ -15,314 +18,450 @@ use crate::{ /// [AccountVaultDelta] stores the difference between the initial and final account vault states. /// /// The difference is represented as follows: -/// - added_assets: a vector of assets that were added to the account vault. -/// - removed_assets: a vector of assets that were removed from the account vault. +/// - fungible: a binary tree map of fungible asset balance changes in the account vault. +/// - non_fungible: a binary tree map of non-fungible assets that were added to or removed from the +/// account vault. #[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct AccountVaultDelta { - pub added_assets: Vec, - pub removed_assets: Vec, + fungible: FungibleAssetDelta, + non_fungible: NonFungibleAssetDelta, } impl AccountVaultDelta { - /// Creates an empty [AccountVaultDelta]. - pub fn empty() -> Self { - Self::default() + /// Validates and creates an [AccountVaultDelta] with the given fungible and non-fungible asset + /// deltas. + /// + /// # Errors + /// Returns an error if the delta does not pass the validation. + pub const fn new(fungible: FungibleAssetDelta, non_fungible: NonFungibleAssetDelta) -> Self { + Self { fungible, non_fungible } } - /// Creates an [AccountVaultDelta] from the given iterators. - pub fn from_iterators(added_assets: A, removed_assets: B) -> Self - where - A: IntoIterator, - B: IntoIterator, - { - Self { - added_assets: Vec::from_iter(added_assets), - removed_assets: Vec::from_iter(removed_assets), + /// Returns a reference to the fungible asset delta. + pub fn fungible(&self) -> &FungibleAssetDelta { + &self.fungible + } + + /// Returns a reference to the non-fungible asset delta. + pub fn non_fungible(&self) -> &NonFungibleAssetDelta { + &self.non_fungible + } + + /// Returns true if this vault delta contains no updates. + pub fn is_empty(&self) -> bool { + self.fungible.is_empty() && self.non_fungible.is_empty() + } + + /// Tracks asset addition. + pub fn add_asset(&mut self, asset: Asset) -> Result<(), AccountDeltaError> { + match asset { + Asset::Fungible(asset) => self.fungible.add(asset), + Asset::NonFungible(asset) => self.non_fungible.add(asset), + } + } + + /// Tracks asset removal. + pub fn remove_asset(&mut self, asset: Asset) -> Result<(), AccountDeltaError> { + match asset { + Asset::Fungible(asset) => self.fungible.remove(asset), + Asset::NonFungible(asset) => self.non_fungible.remove(asset), } } /// Merges another delta into this one, overwriting any existing values. /// - /// Inputs and the result are validated as part of the merge. - pub fn merge(self, other: Self) -> Result { - self.validate()?; - other.validate()?; + /// The result is validated as part of the merge. + /// + /// # Errors + /// Returns an error if the resulted delta does not pass the validation. + pub fn merge(&mut self, other: Self) -> Result<(), AccountDeltaError> { + self.non_fungible.merge(other.non_fungible)?; + self.fungible.merge(other.fungible) + } +} - // Merge fungible and non-fungible assets separately. The former can be summed while the - // latter is more of a boolean affair. - // - // Track fungible asset amounts - positive and negative. i64 is not lossy because fungible's are - // restricted to 2^63-1. Overflow is still possible but we check for that. - let mut fungibles = BTreeMap::::new(); - let mut non_fungibles = BTreeMap::::new(); +#[cfg(any(feature = "testing", test))] +impl AccountVaultDelta { + /// Creates an [AccountVaultDelta] from the given iterators. + pub fn from_iters( + added_assets: impl IntoIterator, + removed_assets: impl IntoIterator, + ) -> Self { + use crate::assets::Asset; - let added = self.added_assets.into_iter().chain(other.added_assets); - let removed = self.removed_assets.into_iter().chain(other.removed_assets); + let mut fungible = FungibleAssetDelta::default(); + let mut non_fungible = NonFungibleAssetDelta::default(); - let assets = added.map(|asset| (asset, true)).chain(removed.map(|asset| (asset, false))); + for asset in added_assets { + match asset { + Asset::Fungible(asset) => { + fungible.add(asset).unwrap(); + }, + Asset::NonFungible(asset) => { + non_fungible.add(asset).unwrap(); + }, + } + } - for (asset, is_added) in assets { + for asset in removed_assets { match asset { - Asset::Fungible(fungible) => { - // Ensure overflow is not possible here. - const _: () = assert!(FungibleAsset::MAX_AMOUNT <= i64::MIN.unsigned_abs()); - const _: () = assert!(FungibleAsset::MAX_AMOUNT <= i64::MAX.unsigned_abs()); - let amount = i64::try_from(fungible.amount()).unwrap(); - - let entry = fungibles.entry(fungible.faucet_id()).or_default(); - *entry = if is_added { - entry.checked_add(amount) - } else { - entry.checked_sub(amount) - } - .ok_or_else(|| { - AccountDeltaError::AssetAmountTooBig( - entry.unsigned_abs() + amount.unsigned_abs(), - ) - })?; + Asset::Fungible(asset) => { + fungible.remove(asset).unwrap(); }, - Asset::NonFungible(non_fungible) => { - let previous = non_fungibles.insert(non_fungible, is_added); - if let Some(previous) = previous { - if previous == is_added { - // Asset cannot be added nor removed twice. - return Err(AccountDeltaError::DuplicateVaultUpdate(asset)); - } else { - // Otherwise they cancel out. - non_fungibles.remove(&non_fungible); - } - } + Asset::NonFungible(asset) => { + non_fungible.remove(asset).unwrap(); }, } } - let mut added = Vec::new(); - let mut removed = Vec::new(); + Self { fungible, non_fungible } + } - for (faucet_id, amount) in fungibles { - let is_positive = amount.is_positive(); - let amount: u64 = amount.abs().try_into().expect("i64::abs() always fits in u64"); + /// Returns an iterator over the added assets in this delta. + pub fn added_assets(&self) -> impl Iterator + '_ { + use crate::assets::{Asset, FungibleAsset, NonFungibleAsset}; + self.fungible + .0 + .iter() + .filter(|&(_, &value)| value >= 0) + .map(|(&faucet_id, &diff)| { + Asset::Fungible(FungibleAsset::new(faucet_id, diff.unsigned_abs()).unwrap()) + }) + .chain(self.non_fungible.filter_by_action(NonFungibleDeltaAction::Add).map(|key| { + Asset::NonFungible(unsafe { NonFungibleAsset::new_unchecked(key.into()) }) + })) + } - if amount == 0 { - continue; - } + /// Returns an iterator over the removed assets in this delta. + pub fn removed_assets(&self) -> impl Iterator + '_ { + use crate::assets::{Asset, FungibleAsset, NonFungibleAsset}; + self.fungible + .0 + .iter() + .filter(|&(_, &value)| value < 0) + .map(|(&faucet_id, &diff)| { + Asset::Fungible(FungibleAsset::new(faucet_id, diff.unsigned_abs()).unwrap()) + }) + .chain(self.non_fungible.filter_by_action(NonFungibleDeltaAction::Remove).map(|key| { + Asset::NonFungible(unsafe { NonFungibleAsset::new_unchecked(key.into()) }) + })) + } +} - // We know that the faucet ID is valid since this comes from an existing asset, so the only - // possible error case is the amount overflowing. - let asset = FungibleAsset::new(faucet_id, amount) - .map_err(|_| AccountDeltaError::AssetAmountTooBig(amount))?; +impl Serializable for AccountVaultDelta { + fn write_into(&self, target: &mut W) { + target.write(&self.fungible); + target.write(&self.non_fungible); + } +} - if is_positive { - added.push(Asset::Fungible(asset)); - } else { - removed.push(Asset::Fungible(asset)); - } - } +impl Deserializable for AccountVaultDelta { + fn read_from(source: &mut R) -> Result { + let fungible = source.read()?; + let non_fungible = source.read()?; - for (non_fungible, is_added) in non_fungibles { - let asset = Asset::NonFungible(non_fungible); - if is_added { - added.push(asset); - } else { - removed.push(asset); - } - } + Ok(Self::new(fungible, non_fungible)) + } +} - let delta = Self::from_iterators(added, removed); +// FUNGIBLE ASSET DELTA +// ================================================================================================ + +/// A binary tree map of fungible asset balance changes in the account vault. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct FungibleAssetDelta(BTreeMap); + +impl FungibleAssetDelta { + /// Validates and creates a new fungible asset delta. + /// + /// # Errors + /// Returns an error if the delta does not pass the validation. + pub fn new(map: BTreeMap) -> Result { + let delta = Self(map); delta.validate()?; Ok(delta) } - /// Checks whether this vault delta is valid. + /// Adds a new fungible asset to the delta. /// /// # Errors - /// Returns an error if: - /// - The number added assets is greater than [u16::MAX]. - /// - The number of removed assets is greater than [u16::MAX]. - /// - The same asset was added more than once, removed more than once, or both added and - /// removed. - pub fn validate(&self) -> Result<(), AccountDeltaError> { - if self.added_assets.len() > u16::MAX as usize { - return Err(AccountDeltaError::TooManyAddedAsset { - actual: self.added_assets.len(), - max: u16::MAX as usize, - }); - } else if self.removed_assets.len() > u16::MAX as usize { - return Err(AccountDeltaError::TooManyRemovedAssets { - actual: self.removed_assets.len(), - max: u16::MAX as usize, - }); + /// Returns an error if the delta would overflow. + pub fn add(&mut self, asset: FungibleAsset) -> Result<(), AccountDeltaError> { + let amount: i64 = asset.amount().try_into().expect("Amount it too high"); + self.add_delta(asset.faucet_id(), amount) + } + + /// Removes a fungible asset from the delta. + /// + /// # Errors + /// Returns an error if the delta would overflow. + pub fn remove(&mut self, asset: FungibleAsset) -> Result<(), AccountDeltaError> { + let amount: i64 = asset.amount().try_into().expect("Amount it too high"); + self.add_delta(asset.faucet_id(), -amount) + } + + /// Returns true if this vault delta contains no updates. + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + + /// Returns an iterator over the (key, value) pairs of the map. + pub fn iter(&self) -> impl Iterator { + self.0.iter() + } + + /// Merges another delta into this one, overwriting any existing values. + /// + /// The result is validated as part of the merge. + /// + /// # Errors + /// Returns an error if the result did not pass validation. + pub fn merge(&mut self, other: Self) -> Result<(), AccountDeltaError> { + // Merge fungible assets. + // + // Track fungible asset amounts - positive and negative. `i64` is not lossy while + // fungibles are restricted to 2^63-1. Overflow is still possible but we check for that. + + for (&faucet_id, &amount) in other.0.iter() { + self.add_delta(faucet_id, amount)?; } - // make sure all added assets are unique - for (pos, asset) in self.added_assets.iter().enumerate() { - if self.added_assets[..pos].iter().any(|a| a.is_same(asset)) { - return Err(AccountDeltaError::DuplicateVaultUpdate(*asset)); - } + Ok(()) + } + + // HELPER FUNCTIONS + // --------------------------------------------------------------------------------------------- + + /// Updates the provided map with the provided key and amount. If the final amount is 0, + /// the entry is removed. + /// + /// # Errors + /// Returns an error if the delta would overflow. + fn add_delta(&mut self, faucet_id: AccountId, delta: i64) -> Result<(), AccountDeltaError> { + match self.0.entry(faucet_id) { + Entry::Vacant(entry) => { + entry.insert(delta); + }, + Entry::Occupied(mut entry) => { + let old = *entry.get(); + let new = old.checked_add(delta).ok_or( + AccountDeltaError::FungibleAssetDeltaOverflow { + faucet_id, + this: old, + other: delta, + }, + )?; + + if new == 0 { + entry.remove(); + } else { + *entry.get_mut() = new; + } + }, } - // make sure all removed assets are the same - for (pos, asset) in self.removed_assets.iter().enumerate() { - if self.removed_assets[..pos].iter().any(|a| a.is_same(asset)) { - return Err(AccountDeltaError::DuplicateVaultUpdate(*asset)); - } + Ok(()) + } - if self.added_assets.iter().any(|a| a.is_same(asset)) { - return Err(AccountDeltaError::DuplicateVaultUpdate(*asset)); + /// Checks whether this vault delta is valid. + /// + /// # Errors + /// Returns an error if one or more fungible assets' faucet IDs are invalid. + fn validate(&self) -> Result<(), AccountDeltaError> { + for faucet_id in self.0.keys() { + if !matches!(faucet_id.account_type(), AccountType::FungibleFaucet) { + return Err(AccountDeltaError::NotAFungibleFaucetId(*faucet_id)); } } Ok(()) } +} + +impl Serializable for FungibleAssetDelta { + fn write_into(&self, target: &mut W) { + target.write_usize(self.0.len()); + // TODO: We save `i64` as `u64` since winter utils only support unsigned integers for now. + // We should update this code (and deserialization as well) once it support signed + // integers. + target.write_many(self.0.iter().map(|(&faucet_id, &delta)| (faucet_id, delta as u64))); + } +} + +impl Deserializable for FungibleAssetDelta { + fn read_from(source: &mut R) -> Result { + let num_fungible_assets = source.read_usize()?; + // TODO: We save `i64` as `u64` since winter utils only support unsigned integers for now. + // We should update this code (and serialization as well) once it support signed integers. + let map = source + .read_many::<(AccountId, u64)>(num_fungible_assets)? + .into_iter() + .map(|(account_id, delta_as_u64)| (account_id, delta_as_u64 as i64)) + .collect(); + + Self::new(map).map_err(|err| DeserializationError::InvalidValue(err.to_string())) + } +} + +// NON-FUNGIBLE ASSET DELTA +// ================================================================================================ + +/// A binary tree map of non-fungible asset changes (addition and removal) in the account vault. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct NonFungibleAssetDelta(BTreeMap); + +impl NonFungibleAssetDelta { + /// Creates a new non-fungible asset delta. + pub const fn new(map: BTreeMap) -> Self { + Self(map) + } + + /// Adds a new non-fungible asset to the delta. + /// + /// # Errors + /// Returns an error if the delta already contains the asset addition. + pub fn add(&mut self, asset: NonFungibleAsset) -> Result<(), AccountDeltaError> { + self.apply_action(asset, NonFungibleDeltaAction::Add) + } + + /// Removes a non-fungible asset from the delta. + /// + /// # Errors + /// Returns an error if the delta already contains the asset removal. + pub fn remove(&mut self, asset: NonFungibleAsset) -> Result<(), AccountDeltaError> { + self.apply_action(asset, NonFungibleDeltaAction::Remove) + } /// Returns true if this vault delta contains no updates. pub fn is_empty(&self) -> bool { - self.added_assets.is_empty() && self.removed_assets.is_empty() + self.0.is_empty() + } + + /// Returns an iterator over the (key, value) pairs of the map. + pub fn iter(&self) -> impl Iterator { + self.0.iter() + } + + /// Merges another delta into this one, overwriting any existing values. + /// + /// The result is validated as part of the merge. + /// + /// # Errors + /// Returns an error if duplicate non-fungible assets are added or removed. + pub fn merge(&mut self, other: Self) -> Result<(), AccountDeltaError> { + // Merge non-fungible assets. Each non-fungible asset can cancel others out. + for (&key, &action) in other.0.iter() { + self.apply_action(key, action)?; + } + + Ok(()) + } + + // HELPER FUNCTIONS + // --------------------------------------------------------------------------------------------- + + /// Updates the provided map with the provided key and action. + /// If the action is the opposite to the previous one, the entry is removed. + /// + /// # Errors + /// Returns an error if the delta already contains the provided key and action. + fn apply_action( + &mut self, + asset: NonFungibleAsset, + action: NonFungibleDeltaAction, + ) -> Result<(), AccountDeltaError> { + match self.0.entry(asset) { + Entry::Vacant(entry) => { + entry.insert(action); + }, + Entry::Occupied(entry) => { + let previous = *entry.get(); + if previous == action { + // Asset cannot be added nor removed twice. + return Err(AccountDeltaError::DuplicateNonFungibleVaultUpdate(asset)); + } + // Otherwise they cancel out. + entry.remove(); + }, + } + + Ok(()) + } + + /// Returns an iterator over all keys that have the provided action. + fn filter_by_action( + &self, + action: NonFungibleDeltaAction, + ) -> impl Iterator + '_ { + self.0 + .iter() + .filter(move |&(_, cur_action)| cur_action == &action) + .map(|(key, _)| *key) } } -impl Serializable for AccountVaultDelta { +impl Serializable for NonFungibleAssetDelta { fn write_into(&self, target: &mut W) { - assert!(self.added_assets.len() <= u16::MAX as usize, "too many added assets"); - target.write_u16(self.added_assets.len() as u16); - target.write_many(self.added_assets.iter()); + let added: Vec<_> = self.filter_by_action(NonFungibleDeltaAction::Add).collect(); + let removed: Vec<_> = self.filter_by_action(NonFungibleDeltaAction::Remove).collect(); + + target.write_usize(added.len()); + target.write_many(added.iter()); - assert!(self.removed_assets.len() <= u16::MAX as usize, "too many removed assets"); - target.write_u16(self.removed_assets.len() as u16); - target.write_many(self.removed_assets.iter()); + target.write_usize(removed.len()); + target.write_many(removed.iter()); } } -impl Deserializable for AccountVaultDelta { +impl Deserializable for NonFungibleAssetDelta { fn read_from(source: &mut R) -> Result { - // deserialize and validate added assets - let num_added_assets = source.read_u16()? as usize; - let mut added_assets: Vec = Vec::with_capacity(num_added_assets); - for _ in 0..num_added_assets { - let asset = Asset::read_from(source)?; - if added_assets.iter().any(|a| a.is_same(&asset)) { - return Err(DeserializationError::InvalidValue( - "asset added more than once".to_string(), - )); - } + let mut map = BTreeMap::new(); - added_assets.push(asset); + let num_added = source.read_usize()?; + for _ in 0..num_added { + let added_asset = source.read()?; + map.insert(added_asset, NonFungibleDeltaAction::Add); } - // deserialize and validate removed assets - let num_removed_assets = source.read_u16()? as usize; - let mut removed_assets: Vec = Vec::with_capacity(num_removed_assets); - for _ in 0..num_removed_assets { - let asset = Asset::read_from(source)?; - - if removed_assets.iter().any(|a| a.is_same(&asset)) { - return Err(DeserializationError::InvalidValue( - "asset added more than once".to_string(), - )); - } - - if added_assets.iter().any(|a| a.is_same(&asset)) { - return Err(DeserializationError::InvalidValue( - "asset both added and removed".to_string(), - )); - } - removed_assets.push(asset); + let num_removed = source.read_usize()?; + for _ in 0..num_removed { + let removed_asset = source.read()?; + map.insert(removed_asset, NonFungibleDeltaAction::Remove); } - Ok(Self { added_assets, removed_assets }) + Ok(Self::new(map)) } } +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum NonFungibleDeltaAction { + Add, + Remove, +} + // TESTS // ================================================================================================ #[cfg(test)] mod tests { - use super::{AccountVaultDelta, Asset, Deserializable, Serializable}; + use super::{AccountVaultDelta, Deserializable, Serializable}; use crate::{ accounts::{ account_id::testing::{ ACCOUNT_ID_FUNGIBLE_FAUCET_OFF_CHAIN, ACCOUNT_ID_FUNGIBLE_FAUCET_ON_CHAIN, - ACCOUNT_ID_NON_FUNGIBLE_FAUCET_OFF_CHAIN, ACCOUNT_ID_NON_FUNGIBLE_FAUCET_ON_CHAIN, + ACCOUNT_ID_NON_FUNGIBLE_FAUCET_OFF_CHAIN, }, AccountId, }, - assets::{FungibleAsset, NonFungibleAsset, NonFungibleAssetDetails}, + assets::{Asset, FungibleAsset, NonFungibleAsset, NonFungibleAssetDetails}, testing::storage::build_assets, }; - #[test] - fn account_vault_delta_validation() { - let ffid1 = AccountId::try_from(ACCOUNT_ID_FUNGIBLE_FAUCET_OFF_CHAIN).unwrap(); - let ffid2 = AccountId::try_from(ACCOUNT_ID_FUNGIBLE_FAUCET_ON_CHAIN).unwrap(); - let nffid1 = AccountId::try_from(ACCOUNT_ID_NON_FUNGIBLE_FAUCET_OFF_CHAIN).unwrap(); - let nffid2 = AccountId::try_from(ACCOUNT_ID_NON_FUNGIBLE_FAUCET_ON_CHAIN).unwrap(); - - let asset1: Asset = FungibleAsset::new(ffid1, 10).unwrap().into(); - let asset2: Asset = FungibleAsset::new(ffid1, 30).unwrap().into(); - let asset3: Asset = FungibleAsset::new(ffid2, 20).unwrap().into(); - - let asset4: Asset = - NonFungibleAsset::new(&NonFungibleAssetDetails::new(nffid1, vec![1, 2, 3]).unwrap()) - .unwrap() - .into(); - let asset5: Asset = - NonFungibleAsset::new(&NonFungibleAssetDetails::new(nffid1, vec![4, 5, 6]).unwrap()) - .unwrap() - .into(); - let asset6: Asset = - NonFungibleAsset::new(&NonFungibleAssetDetails::new(nffid2, vec![7, 8, 9]).unwrap()) - .unwrap() - .into(); - - assert_eq!(asset5, Asset::read_from_bytes(&asset5.to_bytes()).unwrap()); - - // control case - let delta = AccountVaultDelta { - added_assets: vec![asset1, asset4, asset5], - removed_assets: vec![asset3, asset6], - }; - assert!(delta.validate().is_ok()); - - let bytes = delta.to_bytes(); - assert_eq!(AccountVaultDelta::read_from_bytes(&bytes), Ok(delta)); - - // duplicate asset in added assets - let delta = AccountVaultDelta { - added_assets: vec![asset1, asset4, asset5, asset2], - removed_assets: vec![], - }; - assert!(delta.validate().is_err()); - - let bytes = delta.to_bytes(); - assert!(AccountVaultDelta::read_from_bytes(&bytes).is_err()); - - // duplicate asset in removed assets - let delta = AccountVaultDelta { - added_assets: vec![], - removed_assets: vec![asset1, asset4, asset5, asset2], - }; - assert!(delta.validate().is_err()); - - let bytes = delta.to_bytes(); - assert!(AccountVaultDelta::read_from_bytes(&bytes).is_err()); - - // duplicate asset across added and removed assets - let delta = AccountVaultDelta { - added_assets: vec![asset1, asset3], - removed_assets: vec![asset4, asset5, asset2], - }; - assert!(delta.validate().is_err()); - - let bytes = delta.to_bytes(); - assert!(AccountVaultDelta::read_from_bytes(&bytes).is_err()); - } - #[test] fn test_serde_account_vault() { let (asset_0, asset_1) = build_assets(); - let delta = AccountVaultDelta::from_iterators([asset_0], [asset_1]); + let delta = AccountVaultDelta::from_iters([asset_0], [asset_1]); let serialized = delta.to_bytes(); let deserialized = AccountVaultDelta::read_from_bytes(&serialized).unwrap(); @@ -334,9 +473,9 @@ mod tests { let faucet = AccountId::try_from(ACCOUNT_ID_FUNGIBLE_FAUCET_ON_CHAIN).unwrap(); let asset: Asset = FungibleAsset::new(faucet, 123).unwrap().into(); - assert!(AccountVaultDelta::empty().is_empty()); - assert!(!AccountVaultDelta::from_iterators([asset], []).is_empty()); - assert!(!AccountVaultDelta::from_iterators([], [asset]).is_empty()); + assert!(AccountVaultDelta::default().is_empty()); + assert!(!AccountVaultDelta::from_iters([asset], []).is_empty()); + assert!(!AccountVaultDelta::from_iters([], [asset]).is_empty()); } #[rstest::rstest] @@ -355,21 +494,15 @@ mod tests { fn create_delta_with_fungible(account_id: AccountId, amount: i64) -> AccountVaultDelta { let asset = FungibleAsset::new(account_id, amount.unsigned_abs()).unwrap().into(); match amount { - 0 => AccountVaultDelta::empty(), - x if x.is_positive() => AccountVaultDelta { - added_assets: vec![asset], - ..Default::default() - }, - _ => AccountVaultDelta { - removed_assets: vec![asset], - ..Default::default() - }, + 0 => AccountVaultDelta::default(), + x if x.is_positive() => AccountVaultDelta::from_iters([asset], []), + _ => AccountVaultDelta::from_iters([], [asset]), } } let account_id = AccountId::try_from(ACCOUNT_ID_FUNGIBLE_FAUCET_OFF_CHAIN).unwrap(); - let delta_x = create_delta_with_fungible(account_id, x); + let mut delta_x = create_delta_with_fungible(account_id, x); let delta_y = create_delta_with_fungible(account_id, y); let result = delta_x.merge(delta_y); @@ -377,7 +510,7 @@ mod tests { // None is used to indicate an error is expected. if let Some(expected) = expected { let expected = create_delta_with_fungible(account_id, expected); - assert_eq!(result.unwrap(), expected); + assert_eq!(result.map(|_| delta_x).unwrap(), expected); } else { assert!(result.is_err()); } @@ -409,28 +542,22 @@ mod tests { .into(); match added { - Some(true) => AccountVaultDelta { - added_assets: vec![asset], - ..Default::default() - }, - Some(false) => AccountVaultDelta { - removed_assets: vec![asset], - ..Default::default() - }, - None => AccountVaultDelta::empty(), + Some(true) => AccountVaultDelta::from_iters([asset], []), + Some(false) => AccountVaultDelta::from_iters([], [asset]), + None => AccountVaultDelta::default(), } } let account_id = AccountId::try_from(ACCOUNT_ID_NON_FUNGIBLE_FAUCET_OFF_CHAIN).unwrap(); - let delta_x = create_delta_with_non_fungible(account_id, x); + let mut delta_x = create_delta_with_non_fungible(account_id, x); let delta_y = create_delta_with_non_fungible(account_id, y); let result = delta_x.merge(delta_y); if let Ok(expected) = expected { let expected = create_delta_with_non_fungible(account_id, expected); - assert_eq!(result.unwrap(), expected); + assert_eq!(result.map(|_| delta_x).unwrap(), expected); } else { assert!(result.is_err()); } diff --git a/objects/src/accounts/mod.rs b/objects/src/accounts/mod.rs index 04948a992..fe4b35e5a 100644 --- a/objects/src/accounts/mod.rs +++ b/objects/src/accounts/mod.rs @@ -17,7 +17,10 @@ pub mod code; pub use code::{procedure::AccountProcedureInfo, AccountCode}; pub mod delta; -pub use delta::{AccountDelta, AccountStorageDelta, AccountVaultDelta, StorageMapDelta}; +pub use delta::{ + AccountDelta, AccountStorageDelta, AccountVaultDelta, FungibleAssetDelta, + NonFungibleAssetDelta, NonFungibleDeltaAction, StorageMapDelta, +}; mod seed; pub use seed::{get_account_seed, get_account_seed_single}; @@ -188,15 +191,11 @@ impl Account { /// - The nonce specified in the provided delta smaller than or equal to the current account /// nonce. pub fn apply_delta(&mut self, delta: &AccountDelta) -> Result<(), AccountError> { - // update vault; we don't check vault delta validity here because AccountDelta can contain + // update vault; we don't check vault delta validity here because `AccountDelta` can contain // only valid vault deltas - for &asset in delta.vault().added_assets.iter() { - self.vault.add_asset(asset).map_err(AccountError::AssetVaultUpdateError)?; - } - - for &asset in delta.vault().removed_assets.iter() { - self.vault.remove_asset(asset).map_err(AccountError::AssetVaultUpdateError)?; - } + self.vault + .apply_delta(delta.vault()) + .map_err(AccountError::AssetVaultUpdateError)?; // update storage self.storage.apply_delta(delta.storage())?; @@ -322,10 +321,10 @@ mod tests { use super::{AccountDelta, AccountStorageDelta, AccountVaultDelta}; use crate::{ - accounts::{ - delta::AccountStorageDeltaBuilder, Account, SlotItem, StorageMap, StorageMapDelta, + accounts::{Account, SlotItem, StorageMap, StorageMapDelta}, + testing::storage::{ + build_account, build_account_delta, build_assets, AccountStorageDeltaBuilder, }, - testing::storage::{build_account, build_account_delta, build_assets}, }; #[test] @@ -345,7 +344,7 @@ mod tests { fn test_serde_account_delta() { let final_nonce = Felt::new(2); let (asset_0, asset_1) = build_assets(); - let storage_delta = AccountStorageDeltaBuilder::new() + let storage_delta = AccountStorageDeltaBuilder::default() .add_cleared_items([0]) .add_updated_items([(1_u8, [Felt::new(1), Felt::new(2), Felt::new(3), Felt::new(4)])]) .build() @@ -396,13 +395,13 @@ mod tests { [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)]); + StorageMapDelta::from_iters([], [(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); - let storage_delta = AccountStorageDeltaBuilder::new() + let storage_delta = AccountStorageDeltaBuilder::default() .add_cleared_items([0]) .add_updated_items([(1_u8, [Felt::new(1), Felt::new(2), Felt::new(3), Felt::new(4)])]) .add_updated_maps([(2_u8, updated_map)]) @@ -443,7 +442,7 @@ mod tests { ); // build account delta - let storage_delta = AccountStorageDeltaBuilder::new() + let storage_delta = AccountStorageDeltaBuilder::default() .add_cleared_items([0]) .add_updated_items([(1_u8, [Felt::new(1), Felt::new(2), Felt::new(3), Felt::new(4)])]) .build() @@ -469,7 +468,7 @@ mod tests { // build account delta let final_nonce = Felt::new(1); - let storage_delta = AccountStorageDeltaBuilder::new() + let storage_delta = AccountStorageDeltaBuilder::default() .add_cleared_items([0]) .add_updated_items([(1_u8, [Felt::new(1), Felt::new(2), Felt::new(3), Felt::new(4)])]) .build() diff --git a/objects/src/accounts/storage/map.rs b/objects/src/accounts/storage/map.rs index 99c64b35d..4617aa462 100644 --- a/objects/src/accounts/storage/map.rs +++ b/objects/src/accounts/storage/map.rs @@ -8,7 +8,6 @@ use crate::{ hash::rpo::RpoDigest, merkle::{InnerNodeInfo, LeafIndex, Smt, SmtLeaf, SmtProof, SMT_DEPTH}, }, - EMPTY_WORD, }; // ACCOUNT STORAGE MAP @@ -107,22 +106,13 @@ impl StorageMap { } /// 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 fn apply_delta(&mut self, delta: &StorageMapDelta) -> Result { - // apply the updated leaves to the storage map - for &(key, value) in delta.updated_leaves.iter() { - self.insert(key.into(), value); - } - - // apply the cleared leaves to the storage map - // currently we cannot remove leaves from the storage map, so we just set them to empty - for &key in delta.cleared_leaves.iter() { - self.insert(key.into(), EMPTY_WORD); + pub fn apply_delta(&mut self, delta: &StorageMapDelta) -> Digest { + // apply the updated and cleared leaves to the storage map + for (&key, &value) in delta.leaves().iter() { + self.insert(key, value); } - Ok(self.root()) + self.root() } } diff --git a/objects/src/accounts/storage/mod.rs b/objects/src/accounts/storage/mod.rs index 838536640..50609819a 100644 --- a/objects/src/accounts/storage/mod.rs +++ b/objects/src/accounts/storage/mod.rs @@ -265,21 +265,16 @@ impl AccountStorage { /// 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. - /// - /// 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. + /// # Errors + /// Returns an error if the updates violate storage layout constraints. 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() { + for (&slot_idx, map_delta) in delta.maps().iter() { let storage_map = self.maps.get_mut(&slot_idx).ok_or(AccountError::StorageMapNotFound(slot_idx))?; - let new_root = storage_map.apply_delta(map_delta)?; + let new_root = storage_map.apply_delta(map_delta); let index = LeafIndex::new(slot_idx.into()).expect("index is u8 - index within range"); self.slots.insert(index, new_root.into()); @@ -287,11 +282,7 @@ impl AccountStorage { // --- update storage slots ------------------------------------------- - 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() { + for (&slot_idx, &slot_value) in delta.slots().iter() { self.set_item(slot_idx, slot_value)?; } diff --git a/objects/src/assets/nonfungible.rs b/objects/src/assets/nonfungible.rs index 1df4c0df1..12aa87de2 100644 --- a/objects/src/assets/nonfungible.rs +++ b/objects/src/assets/nonfungible.rs @@ -5,7 +5,10 @@ use super::{ parse_word, AccountId, AccountType, Asset, AssetError, Felt, Hasher, Word, ACCOUNT_ISFAUCET_MASK, }; -use crate::Digest; +use crate::{ + utils::{ByteReader, ByteWriter, Deserializable, DeserializationError, Serializable}, + Digest, +}; /// Position of the faucet_id inside the [NonFungibleAsset] word. const FAUCET_ID_POS: usize = 1; @@ -17,7 +20,7 @@ const FAUCET_ID_POS: usize = 1; /// The commitment is constructed as follows: /// /// - Hash the asset data producing `[d0, d1, d2, d3]`. -/// - Replace the value of `d1` with the fauce id producing `[d0, faucet_id, d2, d3]`. +/// - Replace the value of `d1` with the faucet id producing `[d0, faucet_id, d2, d3]`. /// - Force the bit position [ACCOUNT_ISFAUCET_MASK] of `d3` to be `0`. /// /// [NonFungibleAsset] itself does not contain the actual asset data. The container for this data @@ -180,6 +183,20 @@ impl fmt::Display for NonFungibleAsset { } } +impl Serializable for NonFungibleAsset { + fn write_into(&self, target: &mut W) { + target.write(self.0) + } +} + +impl Deserializable for NonFungibleAsset { + fn read_from(source: &mut R) -> Result { + let value: Word = source.read()?; + + Self::try_from(value).map_err(|err| DeserializationError::InvalidValue(err.to_string())) + } +} + // NON-FUNGIBLE ASSET DETAILS // ================================================================================================ diff --git a/objects/src/assets/vault.rs b/objects/src/assets/vault.rs index c18ab1c68..f41158a7e 100644 --- a/objects/src/assets/vault.rs +++ b/objects/src/assets/vault.rs @@ -4,8 +4,11 @@ use super::{ AccountId, AccountType, Asset, ByteReader, ByteWriter, Deserializable, DeserializationError, FungibleAsset, NonFungibleAsset, Serializable, ZERO, }; -use crate::{crypto::merkle::Smt, AssetVaultError, Digest}; - +use crate::{ + accounts::{AccountVaultDelta, NonFungibleDeltaAction}, + crypto::merkle::Smt, + AssetVaultError, Digest, +}; // ASSET VAULT // ================================================================================================ @@ -89,6 +92,35 @@ impl AssetVault { // PUBLIC MODIFIERS // -------------------------------------------------------------------------------------------- + /// Applies the specified delta to the asset vault. + /// + /// # Errors + /// Returns an error: + /// - If the total value of assets is greater than or equal to 2^63. + /// - If the delta contains an addition/subtraction for a fungible asset that is not stored in + /// the vault. + /// - If the delta contains a non-fungible asset removal that is not stored in the vault. + /// - If the delta contains a non-fungible asset addition that is already stored in the vault. + pub fn apply_delta(&mut self, delta: &AccountVaultDelta) -> Result<(), AssetVaultError> { + for (&faucet_id, &delta) in delta.fungible().iter() { + let asset = FungibleAsset::new(faucet_id, delta.unsigned_abs()) + .expect("Not a fungible faucet ID or delta is too large"); + match delta >= 0 { + true => self.add_fungible_asset(asset), + false => self.remove_fungible_asset(asset), + }?; + } + + for (&asset, &action) in delta.non_fungible().iter() { + match action { + NonFungibleDeltaAction::Add => self.add_non_fungible_asset(asset), + NonFungibleDeltaAction::Remove => self.remove_non_fungible_asset(asset), + }?; + } + + Ok(()) + } + // ADD ASSET // -------------------------------------------------------------------------------------------- /// Add the specified asset to the vault. diff --git a/objects/src/errors.rs b/objects/src/errors.rs index 0ff931638..e9f222985 100644 --- a/objects/src/errors.rs +++ b/objects/src/errors.rs @@ -6,7 +6,7 @@ use vm_processor::DeserializationError; use super::{ accounts::{AccountId, StorageSlotType}, assets::{Asset, FungibleAsset, NonFungibleAsset}, - crypto::{hash::rpo::RpoDigest, merkle::MerkleError}, + crypto::merkle::MerkleError, notes::NoteId, Digest, Word, MAX_BATCHES_PER_BLOCK, MAX_NOTES_PER_BATCH, }; @@ -60,16 +60,16 @@ impl std::error::Error for AccountError {} #[derive(Debug, Clone, PartialEq, Eq)] pub enum AccountDeltaError { DuplicateStorageItemUpdate(usize), - DuplicateVaultUpdate(Asset), - InconsistentNonceUpdate(String), + DuplicateNonFungibleVaultUpdate(NonFungibleAsset), + FungibleAssetDeltaOverflow { + faucet_id: AccountId, + this: i64, + other: i64, + }, ImmutableStorageSlot(usize), - TooManyAddedAsset { actual: usize, max: usize }, - TooManyClearedStorageItems { actual: usize, max: usize }, - TooManyRemovedAssets { actual: usize, max: usize }, - TooManyUpdatedStorageItems { actual: usize, max: usize }, - DuplicateStorageMapLeaf { key: RpoDigest }, - AssetAmountTooBig(u64), IncompatibleAccountUpdates(AccountUpdateDetails, AccountUpdateDetails), + InconsistentNonceUpdate(String), + NotAFungibleFaucetId(AccountId), } #[cfg(feature = "std")] diff --git a/objects/src/testing/storage.rs b/objects/src/testing/storage.rs index af042cd0f..dbc7b4a53 100644 --- a/objects/src/testing/storage.rs +++ b/objects/src/testing/storage.rs @@ -1,6 +1,7 @@ use alloc::{collections::BTreeMap, string::String, vec::Vec}; use assembly::Assembler; +use miden_crypto::EMPTY_WORD; use vm_core::{Felt, FieldElement, Word, ZERO}; use vm_processor::Digest; @@ -15,10 +16,11 @@ use crate::{ }, get_account_seed_single, Account, AccountCode, AccountDelta, AccountId, AccountStorage, AccountStorageDelta, AccountStorageType, AccountType, AccountVaultDelta, SlotItem, - StorageMap, StorageSlot, + StorageMap, StorageMapDelta, StorageSlot, }, assets::{Asset, AssetVault, FungibleAsset}, notes::NoteAssets, + AccountDeltaError, }; #[derive(Default, Debug, Clone)] @@ -56,6 +58,45 @@ impl AccountStorageBuilder { } } +// ACCOUNT STORAGE DELTA BUILDER +// ================================================================================================ + +#[derive(Clone, Debug, Default)] +pub struct AccountStorageDeltaBuilder { + slots: BTreeMap, + maps: BTreeMap, +} + +impl AccountStorageDeltaBuilder { + // MODIFIERS + // ------------------------------------------------------------------------------------------- + + pub fn add_cleared_items(mut self, items: impl IntoIterator) -> Self { + self.slots.extend(items.into_iter().map(|slot| (slot, EMPTY_WORD))); + self + } + + pub fn add_updated_items(mut self, items: impl IntoIterator) -> Self { + self.slots.extend(items); + self + } + + pub fn add_updated_maps( + mut self, + items: impl IntoIterator, + ) -> Self { + self.maps.extend(items); + self + } + + // BUILDERS + // ------------------------------------------------------------------------------------------- + + pub fn build(self) -> Result { + AccountStorageDelta::new(self.slots, self.maps) + } +} + // ACCOUNT STORAGE UTILS // ================================================================================================ @@ -240,7 +281,7 @@ pub fn build_account_delta( nonce: Felt, storage_delta: AccountStorageDelta, ) -> AccountDelta { - let vault_delta = AccountVaultDelta { added_assets, removed_assets }; + let vault_delta = AccountVaultDelta::from_iters(added_assets, removed_assets); AccountDelta::new(storage_delta, vault_delta, Some(nonce)).unwrap() }