From 8cb8f0a76e734af8175974578655b75d79de0526 Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Wed, 18 Sep 2024 10:14:12 +0200 Subject: [PATCH 01/19] Change storage_offset from u16 to u8 --- objects/src/accounts/code/procedure.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/objects/src/accounts/code/procedure.rs b/objects/src/accounts/code/procedure.rs index bbe30347f..bf4944f86 100644 --- a/objects/src/accounts/code/procedure.rs +++ b/objects/src/accounts/code/procedure.rs @@ -19,7 +19,7 @@ use crate::AccountError; #[derive(Debug, PartialEq, Eq, Clone)] pub struct AccountProcedureInfo { mast_root: Digest, - storage_offset: u16, + storage_offset: u8, } impl AccountProcedureInfo { @@ -27,7 +27,7 @@ impl AccountProcedureInfo { pub const NUM_ELEMENTS_PER_PROC: usize = 8; /// Returns a new instance of an [AccountProcedureInfo]. - pub fn new(mast_root: Digest, storage_offset: u16) -> Self { + pub fn new(mast_root: Digest, storage_offset: u8) -> Self { Self { mast_root, storage_offset } } @@ -37,7 +37,7 @@ impl AccountProcedureInfo { } /// Returns a reference to the procedure's storage_offset. - pub fn storage_offset(&self) -> u16 { + pub fn storage_offset(&self) -> u8 { self.storage_offset } } @@ -64,7 +64,7 @@ impl TryFrom<[Felt; 8]> for AccountProcedureInfo { let mast_root = Digest::from(<[Felt; 4]>::try_from(&value[0..4]).unwrap()); // get storage_offset form value[4] - let storage_offset: u16 = value[4] + let storage_offset: u8 = value[4] .try_into() .map_err(|_| AccountError::AccountCodeProcedureInvalidStorageOffset)?; @@ -80,14 +80,14 @@ impl TryFrom<[Felt; 8]> for AccountProcedureInfo { impl Serializable for AccountProcedureInfo { fn write_into(&self, target: &mut W) { target.write(self.mast_root()); - target.write_u16(self.storage_offset()); + target.write_u8(self.storage_offset()); } } impl Deserializable for AccountProcedureInfo { fn read_from(source: &mut R) -> Result { let mast_root: Digest = source.read()?; - let storage_offset = source.read_u16()?; + let storage_offset = source.read_u8()?; Ok(Self::new(mast_root, storage_offset)) } From f3aa5dea2d3dbc9abd253683849a09dba7f425a9 Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Wed, 18 Sep 2024 11:13:59 +0200 Subject: [PATCH 02/19] Added storage size to AccountCode and AccountProcInfo --- .../src/tests/kernel_tests/test_account.rs | 8 +- objects/src/accounts/code/mod.rs | 16 ++-- objects/src/accounts/code/procedure.rs | 77 +++++++++++++++---- objects/src/errors.rs | 1 + 4 files changed, 76 insertions(+), 26 deletions(-) diff --git a/miden-tx/src/tests/kernel_tests/test_account.rs b/miden-tx/src/tests/kernel_tests/test_account.rs index 59526a54b..41dbd9af6 100644 --- a/miden-tx/src/tests/kernel_tests/test_account.rs +++ b/miden-tx/src/tests/kernel_tests/test_account.rs @@ -477,10 +477,10 @@ fn test_storage_offset() { // TODO: We manually set the offsets here because we do not have the ability to set the // offsets through MASM for now. Remove this code when we enable this functionality. let procedures_with_offsets = vec![ - AccountProcedureInfo::new(*code.procedures()[0].mast_root(), 2), - AccountProcedureInfo::new(*code.procedures()[1].mast_root(), 2), - AccountProcedureInfo::new(*code.procedures()[2].mast_root(), 1), - AccountProcedureInfo::new(*code.procedures()[3].mast_root(), 1), + AccountProcedureInfo::new(*code.procedures()[0].mast_root(), 2, 5), + AccountProcedureInfo::new(*code.procedures()[1].mast_root(), 2, 5), + AccountProcedureInfo::new(*code.procedures()[2].mast_root(), 1, 5), + AccountProcedureInfo::new(*code.procedures()[3].mast_root(), 1, 5), ]; let code = AccountCode::from_parts(code.mast().clone(), procedures_with_offsets.clone()); diff --git a/objects/src/accounts/code/mod.rs b/objects/src/accounts/code/mod.rs index 802f135b6..da8b9273d 100644 --- a/objects/src/accounts/code/mod.rs +++ b/objects/src/accounts/code/mod.rs @@ -19,17 +19,14 @@ use procedure::AccountProcedureInfo; /// Account's public interface consists of a set of account procedures, each procedure being a /// Miden VM program. Thus, MAST root of each procedure commits to the underlying program. /// -/// Each exported procedure is associated with a storage offset. This offset is applied to any -/// accesses made from within the procedure to the associated account's storage. For example, if -/// storage offset for a procedure is set ot 1, a call to the account::get_item(storage_slot=4) -/// made from this procedure would actually access storage slot with index 5. +/// Each exported procedure is associated with a storage offset and a storage size. /// /// We commit to the entire account interface by building a sequential hash of all procedure MAST /// roots and associated storage_offset's. Specifically, each procedure contributes exactly 8 field /// elements to the sequence of elements to be hashed. These elements are defined as follows: /// /// ```text -/// [PROCEDURE_MAST_ROOT, storage_offset, 0, 0, 0] +/// [PROCEDURE_MAST_ROOT, storage_offset, 0, 0, storage_size] /// ``` #[derive(Debug, Clone)] pub struct AccountCode { @@ -65,9 +62,14 @@ impl AccountCode { // be read from the Library metadata. let mut procedures: Vec = Vec::new(); let storage_offset = if is_faucet { 1 } else { 0 }; + let storage_size = 5; for module in library.module_infos() { for proc_mast_root in module.procedure_digests() { - procedures.push(AccountProcedureInfo::new(proc_mast_root, storage_offset)); + procedures.push(AccountProcedureInfo::new( + proc_mast_root, + storage_offset, + storage_size, + )); } } @@ -192,7 +194,7 @@ impl AccountCode { /// /// This is done by first converting each procedure into 8 field elements as follows: /// ```text - /// [PROCEDURE_MAST_ROOT, storage_offset, 0, 0, 0] + /// [PROCEDURE_MAST_ROOT, storage_offset, 0, 0, storage_size] /// ``` /// And then concatenating the resulting elements into a single vector. pub fn as_elements(&self) -> Vec { diff --git a/objects/src/accounts/code/procedure.rs b/objects/src/accounts/code/procedure.rs index bf4944f86..d3bcfee86 100644 --- a/objects/src/accounts/code/procedure.rs +++ b/objects/src/accounts/code/procedure.rs @@ -12,34 +12,54 @@ use crate::AccountError; /// Information about a procedure exposed in a public account interface. /// -/// The info included the MAST root of the procedure and the storage offset applied to all account -/// storage-related accesses made by this procedure. For example, if storage offset is set ot 1, a -/// call to the account::get_item(storage_slot=4) made from this procedure would actually access +/// The info included the MAST root of the procedure, the storage offset applied to all account +/// storage-related accesses made by this procedure and the storage size allowed to be accessed +/// by this procedure. +/// +/// The offset is applied to any accesses made from within the procedure to the associated +/// account's storage. For example, if storage offset for a procedure is set ot 1, a call +/// to the account::get_item(storage_slot=4) made from this procedure would actually access /// storage slot with index 5. +/// +/// The size is used to limit how many storage slots a given procedure can access in the associated +/// account's storage. For example, if storage size for a procedure is set to 3, the procedure will +/// be bounded to access storage slots in the range [storage_offset, storage_offset + 3]. #[derive(Debug, PartialEq, Eq, Clone)] pub struct AccountProcedureInfo { mast_root: Digest, storage_offset: u8, + storage_size: u8, } impl AccountProcedureInfo { /// The number of field elements needed to represent an [AccountProcedureInfo] in kernel memory. pub const NUM_ELEMENTS_PER_PROC: usize = 8; + // CONSTRUCTOR + // -------------------------------------------------------------------------------------------- + /// Returns a new instance of an [AccountProcedureInfo]. - pub fn new(mast_root: Digest, storage_offset: u8) -> Self { - Self { mast_root, storage_offset } + pub fn new(mast_root: Digest, storage_offset: u8, storage_size: u8) -> Self { + Self { mast_root, storage_offset, storage_size } } - /// Returns a reference to the procedure's mast_root. + // PUBLIC ACCESSORS + // -------------------------------------------------------------------------------------------- + + /// Returns a reference to the procedure's mast root. pub fn mast_root(&self) -> &Digest { &self.mast_root } - /// Returns a reference to the procedure's storage_offset. + /// Returns the procedure's storage offset. pub fn storage_offset(&self) -> u8 { self.storage_offset } + + /// Returns the procedure's storage size. + pub fn storage_size(&self) -> u8 { + self.storage_size + } } impl From for [Felt; 8] { @@ -50,7 +70,10 @@ impl From for [Felt; 8] { result[0..4].copy_from_slice(value.mast_root().as_elements()); // copy the storage offset into value[4] - result[4] = Felt::from(value.storage_offset()); + result[4] = Felt::from(value.storage_offset); + + // copy the storage size into value[7] + result[7] = Felt::from(value.storage_size); result } @@ -68,19 +91,25 @@ impl TryFrom<[Felt; 8]> for AccountProcedureInfo { .try_into() .map_err(|_| AccountError::AccountCodeProcedureInvalidStorageOffset)?; - // Check if the last three elements are zero - if value[5..].iter().any(|&x| x != Felt::ZERO) { + // Check if the next two elements are zero + if value[5] != Felt::ZERO || value[6] != Felt::ZERO { return Err(AccountError::AccountCodeProcedureInvalidPadding); } - Ok(Self { mast_root, storage_offset }) + // get storage_size form value[7] + let storage_size: u8 = value[7] + .try_into() + .map_err(|_| AccountError::AccountCodeProcedureInvalidStorageSize)?; + + Ok(Self { mast_root, storage_offset, storage_size }) } } impl Serializable for AccountProcedureInfo { fn write_into(&self, target: &mut W) { - target.write(self.mast_root()); - target.write_u8(self.storage_offset()); + target.write(self.mast_root); + target.write_u8(self.storage_offset); + target.write_u8(self.storage_size) } } @@ -88,8 +117,9 @@ impl Deserializable for AccountProcedureInfo { fn read_from(source: &mut R) -> Result { let mast_root: Digest = source.read()?; let storage_offset = source.read_u8()?; + let storage_size = source.read_u8()?; - Ok(Self::new(mast_root, storage_offset)) + Ok(Self::new(mast_root, storage_offset, storage_size)) } } @@ -98,10 +128,27 @@ impl Deserializable for AccountProcedureInfo { #[cfg(test)] mod tests { + use miden_crypto::utils::{Deserializable, Serializable}; + use vm_core::Felt; use crate::accounts::{AccountCode, AccountProcedureInfo}; + #[test] + fn test_from_to_account_procedure() { + let account_code = AccountCode::mock(); + + let procedure = account_code.procedures()[0].clone(); + + // from procedure to [Felt; 8] + let felts: [Felt; 8] = procedure.clone().into(); + + // try_from [Felt; 8] to procedure + let final_procedure: AccountProcedureInfo = felts.try_into().unwrap(); + + assert_eq!(procedure, final_procedure); + } + #[test] fn test_serde_account_procedure() { let account_code = AccountCode::mock(); @@ -109,6 +156,6 @@ mod tests { let serialized = account_code.procedures()[0].to_bytes(); let deserialized = AccountProcedureInfo::read_from_bytes(&serialized).unwrap(); - assert_eq!(deserialized, account_code.procedures()[0]); + assert_eq!(account_code.procedures()[0], deserialized); } } diff --git a/objects/src/errors.rs b/objects/src/errors.rs index fe72fb36f..53df17a4d 100644 --- a/objects/src/errors.rs +++ b/objects/src/errors.rs @@ -25,6 +25,7 @@ pub enum AccountError { AccountCodeNoProcedures, AccountCodeTooManyProcedures { max: usize, actual: usize }, AccountCodeProcedureInvalidStorageOffset, + AccountCodeProcedureInvalidStorageSize, AccountCodeProcedureInvalidPadding, AccountIdInvalidFieldElement(String), AccountIdTooFewOnes(u32, u32), From 2dbc6fc5d61d059b71cbd00dba1fb455c0cf6b50 Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Wed, 18 Sep 2024 12:26:27 +0200 Subject: [PATCH 03/19] Updated authenticate_procedure and get_procedure_info to support storage_size --- miden-lib/asm/kernels/transaction/api.masm | 96 +++++++++---------- .../asm/kernels/transaction/lib/account.masm | 20 ++-- 2 files changed, 60 insertions(+), 56 deletions(-) diff --git a/miden-lib/asm/kernels/transaction/api.masm b/miden-lib/asm/kernels/transaction/api.masm index feaab7253..d51f6ff04 100644 --- a/miden-lib/asm/kernels/transaction/api.masm +++ b/miden-lib/asm/kernels/transaction/api.masm @@ -54,16 +54,16 @@ const.ACCOUNT_VAULT_AFTER_REMOVE_ASSET_EVENT=131075 #! Panics: #! - if the invocation of the kernel procedure does not originate from the account context. #! -#! Stack: [...] -#! Output: [storage_offset, ...] +#! Stack: [] +#! Output: [storage_offset, storage_size] proc.authenticate_account_origin # get the hash of the caller padw caller - # => [CALLER, ...] + # => [CALLER] # assert that the caller is from the user context exec.account::authenticate_procedure - # => [storage_offset, ...] + # => [storage_offset, storage_size] end # KERNEL PROCEDURES @@ -96,7 +96,7 @@ end export.get_account_nonce # drop the procedure's hash dropw - + # get the account nonce exec.account::get_nonce # => [0, nonce] @@ -114,7 +114,7 @@ end export.get_initial_account_hash # drop the procedure's hash dropw - + # get the initial account hash exec.account::get_initial_hash # => [H, 0, 0, 0, 0] @@ -132,7 +132,7 @@ end export.get_current_account_hash # drop the procedure's hash dropw - + # get the current account hash exec.account::get_current_hash # => [ACCT_HASH, 0, 0, 0, 0] @@ -151,9 +151,9 @@ end export.incr_account_nonce # drop the procedure's hash dropw - + # authenticate that the procedure invocation originates from the account context - exec.authenticate_account_origin drop + exec.authenticate_account_origin drop drop # => [value] # arrange stack @@ -178,7 +178,7 @@ export.get_account_item # authenticate that the procedure invocation originates from the account context exec.authenticate_account_origin - # => [storage_offset, index, 0, 0, 0] + # => [storage_offset, storage_size, index, 0, 0, 0] # apply offset to storage slot index exec.account::apply_storage_offset @@ -205,7 +205,7 @@ end export.set_account_item # drop the procedure's hash dropw - + # if the transaction is being executed against a faucet account then assert # index != FAUCET_STORAGE_DATA_SLOT (reserved slot) dup exec.account::get_faucet_storage_data_slot eq @@ -215,7 +215,7 @@ export.set_account_item # authenticate that the procedure invocation originates from the account context exec.authenticate_account_origin - # => [storage_offset, index, V', 0, 0, 0] + # => [storage_offset, storage_size, index, V', 0, 0, 0] # apply offset to storage slot index exec.account::apply_storage_offset @@ -243,7 +243,7 @@ end export.get_account_map_item # drop the procedure's hash dropw - + # check if storage type is map dup exec.account::get_storage_slot_type # => [slot_type, index, KEY, ...] @@ -254,7 +254,7 @@ export.get_account_map_item # authenticate that the procedure invocation originates from the account context exec.authenticate_account_origin - # => [storage_offset, index, KEY, ...] + # => [storage_offset, storage_size, index, KEY, ...] # apply offset to storage slot index exec.account::apply_storage_offset @@ -291,10 +291,10 @@ end export.set_account_map_item.1 # drop the procedure's hash dropw - + # authenticate that the procedure invocation originates from the account context exec.authenticate_account_origin - # => [storage_offset, index, KEY, NEW_VALUE, ...] + # => [storage_offset, storage_size, index, KEY, NEW_VALUE, ...] # apply offset to storage slot index exec.account::apply_storage_offset @@ -327,9 +327,9 @@ end export.set_account_code # drop the procedure's hash dropw - + # authenticate that the procedure invocation originates from the account context - exec.authenticate_account_origin drop + exec.authenticate_account_origin drop drop # => [CODE_COMMITMENT] # arrange stack @@ -352,7 +352,7 @@ end export.account_vault_get_balance # drop the procedure's hash dropw - + # get the vault root exec.memory::get_acct_vault_root_ptr swap # => [faucet_id, acct_vault_root_ptr] @@ -373,7 +373,7 @@ end export.account_vault_has_non_fungible_asset # drop the procedure's hash dropw - + # arrange stack and get the vault root push.0 movdn.4 push.0 movdn.4 push.0 movdn.4 exec.memory::get_acct_vault_root_ptr movdn.4 # => [ASSET, 0, 0, 0] @@ -401,9 +401,9 @@ end export.account_vault_add_asset # drop the procedure's hash dropw - + # authenticate that the procedure invocation originates from the account context - exec.authenticate_account_origin drop + exec.authenticate_account_origin drop drop # => [ASSET] push.19891 drop # TODO: remove line, see miden-vm/#1122 @@ -443,9 +443,9 @@ end export.account_vault_remove_asset # drop the procedure's hash dropw - + # authenticate that the procedure invocation originates from the account context - exec.authenticate_account_origin drop + exec.authenticate_account_origin drop drop # => [ASSET] push.20071 drop # TODO: remove line, see miden-vm/#1122 @@ -477,7 +477,7 @@ end export.get_note_assets_info # drop the procedure's hash dropw - + # get the assets info exec.note::get_assets_info # => [ASSETS_HASH, num_assets, 0, 0, 0, 0, 0] @@ -497,7 +497,7 @@ end export.get_note_inputs_hash # drop the procedure's hash dropw - + exec.note::get_note_inputs_hash # => [NOTE_INPUTS_HASH, EMPTY_WORD] @@ -516,7 +516,7 @@ end export.get_note_sender # drop the procedure's hash dropw - + exec.note::get_sender swap drop # => [sender] end @@ -530,7 +530,7 @@ end export.get_block_number # drop the procedure's hash dropw - + # get the block number exec.tx::get_block_number # => [num, 0] @@ -550,7 +550,7 @@ end export.get_block_hash # drop the procedure's hash dropw - + dropw exec.tx::get_block_hash # => [BLOCK_HASH] end @@ -571,7 +571,7 @@ end export.get_input_notes_commitment # drop the procedure's hash dropw - + exec.tx::get_input_notes_commitment # => [COM, 0, 0, 0, 0] @@ -589,7 +589,7 @@ end export.get_output_notes_hash # drop the procedure's hash dropw - + # get the output notes hash exec.tx::get_output_notes_hash # => [COM, 0, 0, 0, 0] @@ -613,9 +613,9 @@ end export.create_note # drop the procedure's hash dropw - + # authenticate that the procedure invocation originates from the account context - exec.authenticate_account_origin drop + exec.authenticate_account_origin drop drop # => [tag, aux, note_type, execution_hint, RECIPIENT, pad(8)] exec.tx::create_note @@ -632,9 +632,9 @@ end export.add_asset_to_note # drop the procedure's hash dropw - + # authenticate that the procedure invocation originates from the account context - exec.authenticate_account_origin drop + exec.authenticate_account_origin drop drop # => [note_idx, ASSET] # duplicate the asset word to be able to return it @@ -654,7 +654,7 @@ end export.get_account_vault_commitment # drop the procedure's hash dropw - + # fetch the account vault root exec.memory::get_acct_vault_root # => [COM, 0, 0, 0, 0] @@ -682,9 +682,9 @@ end export.mint_asset # drop the procedure's hash dropw - + # authenticate that the procedure invocation originates from the account context - exec.authenticate_account_origin drop + exec.authenticate_account_origin drop drop # => [ASSET] # mint the asset @@ -711,9 +711,9 @@ end export.burn_asset # drop the procedure's hash dropw - + # authenticate that the procedure invocation originates from the account context - exec.authenticate_account_origin drop + exec.authenticate_account_origin drop drop # => [ASSET] # burn the asset @@ -734,7 +734,7 @@ end export.get_fungible_faucet_total_issuance # drop the procedure's hash dropw - + # assert that we are executing a transaction against a fungible faucet (access checks) exec.account::get_id exec.account::is_fungible_faucet assert.err=ERR_ACCT_MUST_BE_A_FAUCET # => [0] @@ -758,7 +758,7 @@ end export.get_note_serial_number # drop the procedure's hash dropw - + exec.note::get_serial_number # => [SERIAL_NUMBER] @@ -773,15 +773,15 @@ end #! Outputs: [, ] #! #! Where: -#! - procedure_offset is an offset of the kernel procedure, specified in the +#! - procedure_offset is an offset of the kernel procedure, specified in the #! `miden/kernel_proc_offsets.masm` file. -#! - procedure_inputs are inputs of the procedure to be executed, which is specified by the -#! procedure_offset. Note that the length of this inputs cannot exceed 12 elements, since the -#! first word on the stack will be occupied by the procedure hash. -#! - procedure_outputs are the outputs of the procedure to be executed. +#! - procedure_inputs are inputs of the procedure to be executed, which is specified by the +#! procedure_offset. Note that the length of this inputs cannot exceed 12 elements, since the +#! first word on the stack will be occupied by the procedure hash. +#! - procedure_outputs are the outputs of the procedure to be executed. export.exec_kernel_proc # check that the provided procedure offset is within expected bounds - dup exec.memory::get_num_kernel_procedures + dup exec.memory::get_num_kernel_procedures lt assert.err=ERR_KERNEL_PROCEDURE_OFFSET_OUT_OF_BOUNDS # => [procedure_pointer, , ] diff --git a/miden-lib/asm/kernels/transaction/lib/account.masm b/miden-lib/asm/kernels/transaction/lib/account.masm index 203346f8f..8856ea1c6 100644 --- a/miden-lib/asm/kernels/transaction/lib/account.masm +++ b/miden-lib/asm/kernels/transaction/lib/account.masm @@ -568,12 +568,16 @@ export.get_procedure_info # => [index] # get procedure pointer - mul.2 exec.memory::get_acct_procedures_section_offset add dup add.1 - # => [offset_ptr, proc_ptr] + mul.2 exec.memory::get_acct_procedures_section_offset add dup add.1 swap + # => [metadata_ptr, proc_ptr] # load procedure information from memory - mem_load swap padw movup.4 mem_loadw - # => [PROC_ROOT, storage_offset] + padw movup.4 mem_loadw padw movup.8 mem_loadw + # => [METADATA, PROC_ROOT] + + # keep only relevant data + swap drop swap drop movdn.5 movdn.5 + # => [PROC_ROOT, storage_offset, storage_size] end #! Verifies that the procedure root is part of the account code @@ -591,13 +595,13 @@ export.authenticate_procedure emit.ACCOUNT_PUSH_PROCEDURE_INDEX_EVENT adv_push.1 # => [index, PROC_ROOT] - # get procedure info (PROC_ROOT, storage_offset) from memory stored at index + # get procedure info (PROC_ROOT, storage_offset, storage_size) from memory stored at index exec.get_procedure_info - # => [PROC_ROOT, storage_offset, PROC_ROOT] + # => [MEM_PROC_ROOT, storage_offset, storage_size, PROC_ROOT] # verify that PROC_ROOT exists in memory at index - movup.4 movdn.8 assert_eqw.err=ERR_PROC_NOT_PART_OF_ACCOUNT_CODE - # => [storage_offset] + movup.4 movdn.9 movup.4 movdn.9 assert_eqw.err=ERR_PROC_NOT_PART_OF_ACCOUNT_CODE + # => [storage_offset, storage_size] end #! Validates that the account seed, provided via the advice map, satisfies the seed requirements. From 2e5984067f85faac3826e097e80fc8789d9390ed Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Wed, 18 Sep 2024 12:40:08 +0200 Subject: [PATCH 04/19] Remove unwanted changes --- miden-lib/asm/kernels/transaction/api.masm | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/miden-lib/asm/kernels/transaction/api.masm b/miden-lib/asm/kernels/transaction/api.masm index d51f6ff04..9892805dc 100644 --- a/miden-lib/asm/kernels/transaction/api.masm +++ b/miden-lib/asm/kernels/transaction/api.masm @@ -18,7 +18,7 @@ use.kernel::tx # the memory. # # ================================================================================================= -# ERRORS +# ERRORS # ================================================================================================= # For faucets the slot FAUCET_STORAGE_DATA_SLOT is reserved and can not be used with set_account_item @@ -33,7 +33,7 @@ const.ERR_READING_MAP_VALUE_FROM_NON_MAP_SLOT=0x00020049 # Provided kernel procedure offset is out of bounds const.ERR_KERNEL_PROCEDURE_OFFSET_OUT_OF_BOUNDS=0x00020053 -# EVENTS +# EVENTS # ================================================================================================= # Event emitted before an asset is added to the account vault. @@ -460,7 +460,7 @@ export.account_vault_remove_asset exec.asset_vault::remove_asset # => [ASSET] - # emit event to signal that an asset is being removed from the account vault + # emit event to signal that an asset is being removed from the account vault push.20149 drop # TODO: remove line, see miden-vm/#1122 emit.ACCOUNT_VAULT_AFTER_REMOVE_ASSET_EVENT # => [ASSET] @@ -535,7 +535,7 @@ export.get_block_number exec.tx::get_block_number # => [num, 0] - # organize the stack for return + # organize the stack for return swap drop # => [num] end @@ -687,7 +687,7 @@ export.mint_asset exec.authenticate_account_origin drop drop # => [ASSET] - # mint the asset + # mint the asset exec.faucet::mint # => [ASSET] end From 4f9a9d3ff8b9dcf3a2823ba98ee8e58e35668901 Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Wed, 18 Sep 2024 13:43:18 +0200 Subject: [PATCH 05/19] Added validate_storage_sizes --- .../asm/kernels/transaction/lib/account.masm | 56 +++++++++++++++---- .../asm/kernels/transaction/lib/prologue.masm | 39 ++++++------- 2 files changed, 66 insertions(+), 29 deletions(-) diff --git a/miden-lib/asm/kernels/transaction/lib/account.masm b/miden-lib/asm/kernels/transaction/lib/account.masm index 8856ea1c6..1d97e6345 100644 --- a/miden-lib/asm/kernels/transaction/lib/account.masm +++ b/miden-lib/asm/kernels/transaction/lib/account.masm @@ -352,7 +352,7 @@ export.validate_storage_offsets if.true while.true # get storage offset from memory - dup exec.get_procedure_storage_offset + dup exec.get_procedure_metadata swap drop # => [storage_offset, index, num_storage_slots, num_account_procedures] # assert that storage offset is not 0 @@ -384,7 +384,7 @@ export.validate_storage_offsets else while.true # get storage offset from memory - dup exec.get_procedure_storage_offset + dup exec.get_procedure_storage_metadata swap drop # => [storage_offset, index, num_storage_slots, num_account_procedures] # assert that storage offset is in bounds @@ -402,6 +402,41 @@ export.validate_storage_offsets # => [] end +#! Validates all account procedures storage sizes by +#! checking that all storage offsets are in bounds +#! +#! Stack: [] +#! Output: [] +export.validate_storage_sizes + # get number of account procedures and number of storage slots + exec.memory::get_num_account_procedures exec.memory::get_num_storage_slots + # => [num_storage_slots, num_account_procedures] + + # prepare stack for looping + push.0.1 + # => [start_loop, index, num_storage_slots, num_account_procedures] + + # we do not check if num_account_procedures == 0 here because a valid + # account has between 1 and 256 procedures with associated offsets + while.true + # get storage size from memory + dup exec.get_procedure_metadata add + # => [storage_limit, index, num_storage_slots, num_account_procedures] + + # assert that storage limit is in bounds + dup.2 lt assert.err=ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS + # => [index, num_storage_slots, get_num_account_procedures] + + # check if we should continue looping + add.1 dup dup.3 lt + # => [should_loop, index, num_storage_slots, num_account_procedures] + end + + # clean stack + drop drop drop + # => [] +end + #! Gets an item from the account storage #! #! Note: @@ -718,21 +753,22 @@ proc.set_item_raw # => [OLD_VALUE] end -#! Returns the procedure storage offset +#! Returns the procedure metadata #! #! Note: #! - We assume that index has been validated and is within bounds #! -#! Stack: [index, ...] -#! Output: [storage_offset, ...] +#! Stack: [index] +#! Output: [storage_offset, storage_size] #! #! - storage_offset is the procedure storage offset. -proc.get_procedure_storage_offset - # get procedure storage offset pointer +#! - storage_size is the procedure storage size. +proc.get_procedure_metadata + # get procedure storage metadata pointer mul.2 exec.memory::get_acct_procedures_section_offset add add.1 # => [storage_offset_ptr] - # load procedure storage offset from memory - mem_load - # => [storage_offset] + # load procedure storage offset from memory and keep relevant data + mem_loadw swap drop swap drop + # => [storage_offset, storage_size] end diff --git a/miden-lib/asm/kernels/transaction/lib/prologue.masm b/miden-lib/asm/kernels/transaction/lib/prologue.masm index 2d81338f3..6c37b4f6e 100644 --- a/miden-lib/asm/kernels/transaction/lib/prologue.masm +++ b/miden-lib/asm/kernels/transaction/lib/prologue.masm @@ -105,10 +105,10 @@ end # ================================================================================================= -#! Saves the procedure hashes of the chosen kernel to memory. Verifies that kernel root and kernel -#! hash match the sequential hash of all kernels and sequential hash of kernel procedures -#! respectively. -#! +#! Saves the procedure hashes of the chosen kernel to memory. Verifies that kernel root and kernel +#! hash match the sequential hash of all kernels and sequential hash of kernel procedures +#! respectively. +#! #! Inputs: #! Operand stack: [] #! Advice stack: [kernel_version] @@ -119,15 +119,15 @@ end #! Outputs: #! Operand stack: [] #! Advice stack: [] -#! +#! #! Where: -#! - kernel_version, index of the desired kernel in the array of all kernels available for the +#! - kernel_version, index of the desired kernel in the array of all kernels available for the #! current transaction #! - KERNEL_ROOT, accumulative hash from all kernel hashes. #! - [KERNEL_HASHES], array of each kernel hash #! - [KERNEL_PROCEDURE_HASHES], array of procedure hashes of the current kernel proc.process_kernel_data - # move the kernel offset to the operand stack + # move the kernel offset to the operand stack adv_push.1 # OS => [kernel_version] # AS => [] @@ -137,12 +137,12 @@ proc.process_kernel_data # OS => [KERNEL_ROOT, kernel_version] # AS => [] - # push the kernel hashes from the advice map to the advice stack + # push the kernel hashes from the advice map to the advice stack adv.push_mapvaln # OS => [KERNEL_ROOT, kernel_version] # AS => [len_felts, [KERNEL_HASHES]] - # move the number of felt elements in the [KERNEL_HASHES] array to the stack and get the + # move the number of felt elements in the [KERNEL_HASHES] array to the stack and get the # number of Words from it adv_push.1 div.4 # OS => [len_words, KERNEL_ROOT, kernel_version] @@ -150,7 +150,7 @@ proc.process_kernel_data # get the pointer to the memory where kernel hashes will be stored # Note: for now we use the same address for kernel hash and for kernel procedures since there is - # only one kernel and its hash will be overwritten by the procedures anyway. + # only one kernel and its hash will be overwritten by the procedures anyway. exec.memory::get_kernel_procedures_ptr swap # OS => [len_words, kernel_mem_ptr, KERNEL_ROOT, kernel_version] # AS => [[KERNEL_HASHES]] @@ -166,7 +166,7 @@ proc.process_kernel_data # AS => [] # get the hash of the kernel which will be used in the current transaction - exec.memory::get_kernel_procedures_ptr add + exec.memory::get_kernel_procedures_ptr add # OS => [kernel_ptr] # AS => [] @@ -174,12 +174,12 @@ proc.process_kernel_data # OS => [KERNEL_HASH] # AS => [] - # push the procedure hashes of the chosen kernel from the advice map to the advice stack + # push the procedure hashes of the chosen kernel from the advice map to the advice stack adv.push_mapvaln # OS => [KERNEL_HASH] # AS => [len_felts, [PROC_HASHES]] - # move the number of felt elements in the [PROC_HASHES] array to the stack and get the + # move the number of felt elements in the [PROC_HASHES] array to the stack and get the # number of Words from it adv_push.1 div.4 # OS => [len_words, KERNEL_HASH] @@ -612,8 +612,9 @@ proc.process_account_data # => [] end - # validate account procedure storage offsets + # validate account procedure metadata exec.account::validate_storage_offsets + exec.account::validate_storage_sizes end # INPUT NOTES DATA @@ -1161,10 +1162,10 @@ end #! - Any of the input notes do note exist in the note db. #! #! Operand stack: [ -#! BLOCK_HASH, -#! account_id, -#! INITIAL_ACCOUNT_HASH, -#! INPUT_NOTES_COMMITMENT, +#! BLOCK_HASH, +#! account_id, +#! INITIAL_ACCOUNT_HASH, +#! INPUT_NOTES_COMMITMENT, #! ] #! Advice stack: [ #! PREVIOUS_BLOCK_HASH, @@ -1208,7 +1209,7 @@ end #! - version, the current protocol version. #! - timestamp, the current timestamp. #! - NOTE_ROOT, root of the tree with all notes created in the block. -#! - kernel_version, index of the desired kernel in the array of all kernels available for the +#! - kernel_version, index of the desired kernel in the array of all kernels available for the #! current transaction. #! - account_nonce, account's nonce. #! - ACCOUNT_VAULT_ROOT, account's vault root. From 6024b596e9a6ac76634cb156f76e6c2ccef6c8b9 Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Wed, 18 Sep 2024 13:44:07 +0200 Subject: [PATCH 06/19] Fix masm typo --- miden-lib/asm/kernels/transaction/lib/account.masm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/miden-lib/asm/kernels/transaction/lib/account.masm b/miden-lib/asm/kernels/transaction/lib/account.masm index 1d97e6345..f9afcdd81 100644 --- a/miden-lib/asm/kernels/transaction/lib/account.masm +++ b/miden-lib/asm/kernels/transaction/lib/account.masm @@ -384,7 +384,7 @@ export.validate_storage_offsets else while.true # get storage offset from memory - dup exec.get_procedure_storage_metadata swap drop + dup exec.get_procedure_metadata swap drop # => [storage_offset, index, num_storage_slots, num_account_procedures] # assert that storage offset is in bounds From f621f6997398e77b35548e664a7b99d397701e0e Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Wed, 18 Sep 2024 17:11:09 +0200 Subject: [PATCH 07/19] Errors since changes --- miden-lib/asm/kernels/transaction/api.masm | 3 + .../asm/kernels/transaction/lib/account.masm | 4 +- .../asm/kernels/transaction/lib/prologue.masm | 2 +- miden-lib/src/transaction/mod.rs | 5 + miden-lib/src/transaction/procedures.rs | 96 +++++++++---------- miden-tx/src/testing/tx_context/builder.rs | 3 +- .../src/tests/kernel_tests/test_account.rs | 8 +- objects/src/accounts/code/mod.rs | 2 +- 8 files changed, 66 insertions(+), 57 deletions(-) diff --git a/miden-lib/asm/kernels/transaction/api.masm b/miden-lib/asm/kernels/transaction/api.masm index 9892805dc..7ee2d3981 100644 --- a/miden-lib/asm/kernels/transaction/api.masm +++ b/miden-lib/asm/kernels/transaction/api.masm @@ -64,6 +64,9 @@ proc.authenticate_account_origin # assert that the caller is from the user context exec.account::authenticate_procedure # => [storage_offset, storage_size] + + # TODO: Remove + drop end # KERNEL PROCEDURES diff --git a/miden-lib/asm/kernels/transaction/lib/account.masm b/miden-lib/asm/kernels/transaction/lib/account.masm index f9afcdd81..1acc315b6 100644 --- a/miden-lib/asm/kernels/transaction/lib/account.masm +++ b/miden-lib/asm/kernels/transaction/lib/account.masm @@ -611,7 +611,7 @@ export.get_procedure_info # => [METADATA, PROC_ROOT] # keep only relevant data - swap drop swap drop movdn.5 movdn.5 + swap drop swap drop swap movdn.5 movdn.5 # => [PROC_ROOT, storage_offset, storage_size] end @@ -769,6 +769,6 @@ proc.get_procedure_metadata # => [storage_offset_ptr] # load procedure storage offset from memory and keep relevant data - mem_loadw swap drop swap drop + padw movup.4 mem_loadw swap drop swap drop swap # => [storage_offset, storage_size] end diff --git a/miden-lib/asm/kernels/transaction/lib/prologue.masm b/miden-lib/asm/kernels/transaction/lib/prologue.masm index 6c37b4f6e..9489adf2e 100644 --- a/miden-lib/asm/kernels/transaction/lib/prologue.masm +++ b/miden-lib/asm/kernels/transaction/lib/prologue.masm @@ -614,7 +614,7 @@ proc.process_account_data # validate account procedure metadata exec.account::validate_storage_offsets - exec.account::validate_storage_sizes + # exec.account::validate_storage_sizes end # INPUT NOTES DATA diff --git a/miden-lib/src/transaction/mod.rs b/miden-lib/src/transaction/mod.rs index 5703ca79a..b786907a7 100644 --- a/miden-lib/src/transaction/mod.rs +++ b/miden-lib/src/transaction/mod.rs @@ -93,6 +93,11 @@ impl TransactionKernel { ) -> (StackInputs, AdviceInputs) { let account = tx_inputs.account(); + let (_, module_info, _) = Self::kernel().into_parts(); + module_info.procedures().for_each(|(idx, proc_info)| { + std::println!("{}. {}: {:?}", idx.as_usize(), proc_info.name, proc_info.digest) + }); + let stack_inputs = TransactionKernel::build_input_stack( account.id(), account.init_hash(), diff --git a/miden-lib/src/transaction/procedures.rs b/miden-lib/src/transaction/procedures.rs index 0761e5f67..6d8fcd8fc 100644 --- a/miden-lib/src/transaction/procedures.rs +++ b/miden-lib/src/transaction/procedures.rs @@ -51,10 +51,10 @@ impl TransactionKernel { const KERNEL0_PROCEDURES: [Digest; 28] = [ // account_vault_add_asset digest!( - 117074302502728688, - 11439878644778514598, - 16324818132154524894, - 6489512630979919440 + 2987683711948940239, + 9519466904295510013, + 10956924198747482602, + 17408004306125833279 ), // account_vault_get_balance digest!( @@ -72,10 +72,10 @@ const KERNEL0_PROCEDURES: [Digest; 28] = [ ), // account_vault_remove_asset digest!( - 2235246958022854005, - 5794405659267712135, - 12598697568377601936, - 10963092377629893642 + 7539834523041491467, + 397142937596055009, + 5327900295271643305, + 2375747046041964715 ), // get_account_id digest!( @@ -86,17 +86,17 @@ const KERNEL0_PROCEDURES: [Digest; 28] = [ ), // get_account_item digest!( - 18206004789224066622, - 4233449336812475978, - 6804658891075571436, - 3940070286581972689 + 1040475204795364964, + 17637695993153638679, + 14724580617279469106, + 3088455945005429498 ), // get_account_map_item digest!( - 9209967448327341770, - 8988024763842561887, - 12632818454415758249, - 8233400257714804605 + 11237872919213056173, + 3731131930525511376, + 10253981262026331292, + 6217009607744996686 ), // get_account_nonce digest!( @@ -128,38 +128,38 @@ const KERNEL0_PROCEDURES: [Digest; 28] = [ ), // incr_account_nonce digest!( - 14589349829020905629, - 1412999498410091194, - 17301618149076423693, - 2638573156781761162 + 5937604183620313248, + 7567561824574571659, + 7824648557680296928, + 7278151527460154168 ), // set_account_code digest!( - 13397042012380537032, - 174474080566637302, - 1465955330516409718, - 13427241200626333441 + 13512245974928463333, + 2707550160290340059, + 13327908938074034661, + 7587389307441888070 ), // set_account_item digest!( - 7028525769329264650, - 7531398982722010851, - 3695061772051382659, - 2998651828779176432 + 3188106049547813081, + 11204907395997093536, + 3176139606526386295, + 15885787955131587293 ), // set_account_map_item digest!( - 7037030220885902605, - 1540995878644451898, - 11995286967942035929, - 11976243733826929886 + 473184738092566701, + 18145721884467965302, + 1838523893940961211, + 9153492672281231858 ), // burn_asset digest!( - 10812504956203964835, - 17035791932747451701, - 8886876315554082935, - 6015659628759368174 + 3081554106218580737, + 8757783611969431741, + 3946640042947945507, + 11196647131471945635 ), // get_fungible_faucet_total_issuance digest!( @@ -170,24 +170,24 @@ const KERNEL0_PROCEDURES: [Digest; 28] = [ ), // mint_asset digest!( - 17329749049914215544, - 5633414059905366308, - 2519432440213570275, - 8693308573092701498 + 10975097057113762798, + 8452260563872196074, + 17505677823401929175, + 5193950258338411928 ), // add_asset_to_note digest!( - 16660224074633768406, - 3681728837439485251, - 11007804027515511275, - 7127888127578457912 + 7458115775341061429, + 1835996794399712711, + 6335051840432042762, + 2946127438913683307 ), // create_note digest!( - 386212833718199205, - 11471520476317876635, - 15232296418503481248, - 574740517948464248 + 15292587768052928649, + 11903456704622070043, + 15379387213739760556, + 13788361264876345584 ), // get_input_notes_commitment digest!( diff --git a/miden-tx/src/testing/tx_context/builder.rs b/miden-tx/src/testing/tx_context/builder.rs index 4ed5cf1f9..51b06e385 100644 --- a/miden-tx/src/testing/tx_context/builder.rs +++ b/miden-tx/src/testing/tx_context/builder.rs @@ -75,7 +75,8 @@ impl TransactionContextBuilder { TransactionKernel::testing_assembler(), ); - let assembler = TransactionKernel::testing_assembler_with_mock_account(); + let assembler = + TransactionKernel::testing_assembler_with_mock_account().with_debug_mode(true); Self { assembler: assembler.clone(), diff --git a/miden-tx/src/tests/kernel_tests/test_account.rs b/miden-tx/src/tests/kernel_tests/test_account.rs index 41dbd9af6..3401f9e33 100644 --- a/miden-tx/src/tests/kernel_tests/test_account.rs +++ b/miden-tx/src/tests/kernel_tests/test_account.rs @@ -477,10 +477,10 @@ fn test_storage_offset() { // TODO: We manually set the offsets here because we do not have the ability to set the // offsets through MASM for now. Remove this code when we enable this functionality. let procedures_with_offsets = vec![ - AccountProcedureInfo::new(*code.procedures()[0].mast_root(), 2, 5), - AccountProcedureInfo::new(*code.procedures()[1].mast_root(), 2, 5), - AccountProcedureInfo::new(*code.procedures()[2].mast_root(), 1, 5), - AccountProcedureInfo::new(*code.procedures()[3].mast_root(), 1, 5), + AccountProcedureInfo::new(*code.procedures()[0].mast_root(), 2, 0), + AccountProcedureInfo::new(*code.procedures()[1].mast_root(), 2, 0), + AccountProcedureInfo::new(*code.procedures()[2].mast_root(), 1, 0), + AccountProcedureInfo::new(*code.procedures()[3].mast_root(), 1, 0), ]; let code = AccountCode::from_parts(code.mast().clone(), procedures_with_offsets.clone()); diff --git a/objects/src/accounts/code/mod.rs b/objects/src/accounts/code/mod.rs index da8b9273d..ab8704116 100644 --- a/objects/src/accounts/code/mod.rs +++ b/objects/src/accounts/code/mod.rs @@ -62,7 +62,7 @@ impl AccountCode { // be read from the Library metadata. let mut procedures: Vec = Vec::new(); let storage_offset = if is_faucet { 1 } else { 0 }; - let storage_size = 5; + let storage_size = 0; for module in library.module_infos() { for proc_mast_root in module.procedure_digests() { procedures.push(AccountProcedureInfo::new( From f8b776dfe2549426712996d265d89f8fe5e7cc8e Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Wed, 18 Sep 2024 17:22:40 +0200 Subject: [PATCH 08/19] Fixed api.masm --- miden-lib/asm/kernels/transaction/api.masm | 15 ++-- miden-lib/src/transaction/mod.rs | 8 +- miden-lib/src/transaction/procedures.rs | 95 ++++++++++------------ 3 files changed, 57 insertions(+), 61 deletions(-) diff --git a/miden-lib/asm/kernels/transaction/api.masm b/miden-lib/asm/kernels/transaction/api.masm index 7ee2d3981..31153f721 100644 --- a/miden-lib/asm/kernels/transaction/api.masm +++ b/miden-lib/asm/kernels/transaction/api.masm @@ -64,9 +64,6 @@ proc.authenticate_account_origin # assert that the caller is from the user context exec.account::authenticate_procedure # => [storage_offset, storage_size] - - # TODO: Remove - drop end # KERNEL PROCEDURES @@ -180,7 +177,8 @@ export.get_account_item dropw # authenticate that the procedure invocation originates from the account context - exec.authenticate_account_origin + # TODO: remove swap drop and correctly impl apply_storage_offset + exec.authenticate_account_origin swap drop # => [storage_offset, storage_size, index, 0, 0, 0] # apply offset to storage slot index @@ -217,7 +215,8 @@ export.set_account_item # => [index, V', 0, 0, 0] # authenticate that the procedure invocation originates from the account context - exec.authenticate_account_origin + # TODO: remove swap drop and correctly impl apply_storage_offset + exec.authenticate_account_origin swap drop # => [storage_offset, storage_size, index, V', 0, 0, 0] # apply offset to storage slot index @@ -256,7 +255,8 @@ export.get_account_map_item # => [index, KEY, ...] # authenticate that the procedure invocation originates from the account context - exec.authenticate_account_origin + # TODO: remove swap drop and correctly impl apply_storage_offset + exec.authenticate_account_origin swap drop # => [storage_offset, storage_size, index, KEY, ...] # apply offset to storage slot index @@ -296,7 +296,8 @@ export.set_account_map_item.1 dropw # authenticate that the procedure invocation originates from the account context - exec.authenticate_account_origin + # TODO: remove swap drop and correctly impl apply_storage_offset + exec.authenticate_account_origin swap drop # => [storage_offset, storage_size, index, KEY, NEW_VALUE, ...] # apply offset to storage slot index diff --git a/miden-lib/src/transaction/mod.rs b/miden-lib/src/transaction/mod.rs index b786907a7..8e329422c 100644 --- a/miden-lib/src/transaction/mod.rs +++ b/miden-lib/src/transaction/mod.rs @@ -93,10 +93,10 @@ impl TransactionKernel { ) -> (StackInputs, AdviceInputs) { let account = tx_inputs.account(); - let (_, module_info, _) = Self::kernel().into_parts(); - module_info.procedures().for_each(|(idx, proc_info)| { - std::println!("{}. {}: {:?}", idx.as_usize(), proc_info.name, proc_info.digest) - }); + // let (_, module_info, _) = Self::kernel().into_parts(); + // module_info.procedures().for_each(|(idx, proc_info)| { + // std::println!("{}. {}: {:?}", idx.as_usize(), proc_info.name, proc_info.digest) + // }); let stack_inputs = TransactionKernel::build_input_stack( account.id(), diff --git a/miden-lib/src/transaction/procedures.rs b/miden-lib/src/transaction/procedures.rs index 6d8fcd8fc..026af2fc4 100644 --- a/miden-lib/src/transaction/procedures.rs +++ b/miden-lib/src/transaction/procedures.rs @@ -51,10 +51,10 @@ impl TransactionKernel { const KERNEL0_PROCEDURES: [Digest; 28] = [ // account_vault_add_asset digest!( - 2987683711948940239, - 9519466904295510013, - 10956924198747482602, - 17408004306125833279 + 7057810448624645611, + 8978561369920544397, + 17383363575393053933, + 8525279030477447608 ), // account_vault_get_balance digest!( @@ -72,10 +72,10 @@ const KERNEL0_PROCEDURES: [Digest; 28] = [ ), // account_vault_remove_asset digest!( - 7539834523041491467, - 397142937596055009, - 5327900295271643305, - 2375747046041964715 + 3906570158459297765, + 5685173729714936986, + 11907817359076246649, + 2218460969690966293 ), // get_account_id digest!( @@ -86,17 +86,17 @@ const KERNEL0_PROCEDURES: [Digest; 28] = [ ), // get_account_item digest!( - 1040475204795364964, - 17637695993153638679, - 14724580617279469106, - 3088455945005429498 + 13424289660465129162, + 11714494585523463335, + 6258080289656188530, + 11109254005343025041 ), // get_account_map_item digest!( - 11237872919213056173, - 3731131930525511376, - 10253981262026331292, - 6217009607744996686 + 4086597549949492919, + 17234429278667747774, + 2017480864876659824, + 5288909086187322732 ), // get_account_nonce digest!( @@ -128,38 +128,33 @@ const KERNEL0_PROCEDURES: [Digest; 28] = [ ), // incr_account_nonce digest!( - 5937604183620313248, - 7567561824574571659, - 7824648557680296928, - 7278151527460154168 + 13198506529285146698, + 8746315868356500606, + 8557523321460484022, + 14525697582570929169 ), // set_account_code - digest!( - 13512245974928463333, - 2707550160290340059, - 13327908938074034661, - 7587389307441888070 - ), + digest!(791239201240238984, 990477779242440333, 9153970741752682788, 4799626513113428911), // set_account_item digest!( - 3188106049547813081, - 11204907395997093536, - 3176139606526386295, - 15885787955131587293 + 3488970757239817556, + 2169657214816177798, + 10864207859383826292, + 15560226360313617810 ), // set_account_map_item digest!( - 473184738092566701, - 18145721884467965302, - 1838523893940961211, - 9153492672281231858 + 8895862041096145565, + 3313775403671697242, + 15357863624188673369, + 14860022603519750887 ), // burn_asset digest!( - 3081554106218580737, - 8757783611969431741, - 3946640042947945507, - 11196647131471945635 + 17972188987616095529, + 410605682642557022, + 5680335245116684784, + 3175208868792480334 ), // get_fungible_faucet_total_issuance digest!( @@ -170,24 +165,24 @@ const KERNEL0_PROCEDURES: [Digest; 28] = [ ), // mint_asset digest!( - 10975097057113762798, - 8452260563872196074, - 17505677823401929175, - 5193950258338411928 + 12097523897569151283, + 9195187621054123616, + 14731813546930010668, + 7795547866061778929 ), // add_asset_to_note digest!( - 7458115775341061429, - 1835996794399712711, - 6335051840432042762, - 2946127438913683307 + 16413492602303035109, + 17797523785153624998, + 12824576482857791553, + 5044696390097165171 ), // create_note digest!( - 15292587768052928649, - 11903456704622070043, - 15379387213739760556, - 13788361264876345584 + 17600293013402614422, + 5610185477332457018, + 11012826270384682854, + 6238191079182609894 ), // get_input_notes_commitment digest!( From 7103eaf7bb290e632bfd01614720444d9fb7adbc Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Thu, 19 Sep 2024 14:51:41 +0200 Subject: [PATCH 09/19] executed_transaction_account_delta errors out --- miden-lib/asm/kernels/transaction/api.masm | 12 ++-- .../asm/kernels/transaction/lib/account.masm | 57 +++++++++++++------ .../asm/kernels/transaction/lib/prologue.masm | 2 +- miden-lib/src/transaction/procedures.rs | 32 +++++------ miden-tx/src/errors/tx_kernel_errors.rs | 6 +- miden-tx/src/testing/tx_context/builder.rs | 5 +- .../src/tests/kernel_tests/test_account.rs | 37 ++++++++---- miden-tx/src/tests/mod.rs | 31 +++++++++- objects/src/accounts/code/mod.rs | 6 +- objects/src/accounts/code/procedure.rs | 2 +- objects/src/testing/storage.rs | 4 +- 11 files changed, 129 insertions(+), 65 deletions(-) diff --git a/miden-lib/asm/kernels/transaction/api.masm b/miden-lib/asm/kernels/transaction/api.masm index 31153f721..9892805dc 100644 --- a/miden-lib/asm/kernels/transaction/api.masm +++ b/miden-lib/asm/kernels/transaction/api.masm @@ -177,8 +177,7 @@ export.get_account_item dropw # authenticate that the procedure invocation originates from the account context - # TODO: remove swap drop and correctly impl apply_storage_offset - exec.authenticate_account_origin swap drop + exec.authenticate_account_origin # => [storage_offset, storage_size, index, 0, 0, 0] # apply offset to storage slot index @@ -215,8 +214,7 @@ export.set_account_item # => [index, V', 0, 0, 0] # authenticate that the procedure invocation originates from the account context - # TODO: remove swap drop and correctly impl apply_storage_offset - exec.authenticate_account_origin swap drop + exec.authenticate_account_origin # => [storage_offset, storage_size, index, V', 0, 0, 0] # apply offset to storage slot index @@ -255,8 +253,7 @@ export.get_account_map_item # => [index, KEY, ...] # authenticate that the procedure invocation originates from the account context - # TODO: remove swap drop and correctly impl apply_storage_offset - exec.authenticate_account_origin swap drop + exec.authenticate_account_origin # => [storage_offset, storage_size, index, KEY, ...] # apply offset to storage slot index @@ -296,8 +293,7 @@ export.set_account_map_item.1 dropw # authenticate that the procedure invocation originates from the account context - # TODO: remove swap drop and correctly impl apply_storage_offset - exec.authenticate_account_origin swap drop + exec.authenticate_account_origin # => [storage_offset, storage_size, index, KEY, NEW_VALUE, ...] # apply offset to storage slot index diff --git a/miden-lib/asm/kernels/transaction/lib/account.masm b/miden-lib/asm/kernels/transaction/lib/account.masm index 1acc315b6..89292db77 100644 --- a/miden-lib/asm/kernels/transaction/lib/account.masm +++ b/miden-lib/asm/kernels/transaction/lib/account.masm @@ -40,6 +40,9 @@ const.ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS=0x0002004E # Storage offset is invalid for a faucet account (0 is prohibited being the faucet reserved data slot) const.ERR_INVALID_FAUCET_STORAGE_OFFSET=0x0002004F +# Storage offset is invalid for 0 storage size (should be 0) +const.ERR_INVALID_STORAGE_OFFSET_FOR_SIZE=0x00020054 + # CONSTANTS # ================================================================================================= @@ -307,21 +310,20 @@ export.set_code # => [] end -#! Applies offset to provided storage slot index for storage access +#! Applies storage offset to provided storage slot index for storage access #! #! Panics: -#! - slot index is out of bounds #! - computed index is out of bounds #! -#! Stack: [storage_offset, slot_index] +#! Stack: [storage_offset, storage_size, slot_index] #! Output: [offset_slot_index] export.apply_storage_offset - # verify that slot_index is in bounds - dup.1 exec.memory::get_num_storage_slots dup movdn.4 lt assert.err=ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS - # => [storage_offset, slot_index, num_storage_slots] + # offset index + dup movup.3 add + # => [offset_slot_index, storage_offset, storage_size] - # verify that the computed offset_slot_index is in bounds - add dup movup.2 lt assert.err=ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS + # verify that slot_index is in bounds + movdn.2 add sub.1 dup.1 gte assert.err=ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS # => [offset_slot_index] end @@ -402,8 +404,9 @@ export.validate_storage_offsets # => [] end -#! Validates all account procedures storage sizes by -#! checking that all storage offsets are in bounds +#! Validates all account procedures sizes by checking that: +#! - All storage sizes are in bounds +#! - All procedures not accessing storage have offset and size set to 0 #! #! Stack: [] #! Output: [] @@ -419,13 +422,35 @@ export.validate_storage_sizes # we do not check if num_account_procedures == 0 here because a valid # account has between 1 and 256 procedures with associated offsets while.true - # get storage size from memory - dup exec.get_procedure_metadata add - # => [storage_limit, index, num_storage_slots, num_account_procedures] + # get storage offset and size from memory + dup exec.get_procedure_metadata + # => [storage_offset, storage_size, index, num_storage_slots, num_account_procedures] + + # check if size is 0 + dup.1 eq.0 + if.true + # assert that a procedure not accessing storage has offset and size 0 + dup eq.0 assert.err=ERR_INVALID_STORAGE_OFFSET_FOR_SIZE + drop drop + # => [index, num_storage_slots, get_num_account_procedures] + else + # TODO: This is a temporary check for faucets. We add 1 to all faucet offsets + # to prevent access to the reserved faucet data slot. When the assembler + # will support storage offsets remove this check. + # check if the storage offset and the number of storage slots are 1 + dup dup.4 eq.1 swap eq.1 and + # => [not_both_1, storage_offset, storage_size, index, num_storage_slots, num_account_procedures] - # assert that storage limit is in bounds - dup.2 lt assert.err=ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS - # => [index, num_storage_slots, get_num_account_procedures] + if.false + # assert that storage limit is in bounds + add sub.1 dup.2 lt assert.err=ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS + # => [index, num_storage_slots, get_num_account_procedures] + else + # clean stack + drop drop + # => [index, num_storage_slots, get_num_account_procedures] + end + end # check if we should continue looping add.1 dup dup.3 lt diff --git a/miden-lib/asm/kernels/transaction/lib/prologue.masm b/miden-lib/asm/kernels/transaction/lib/prologue.masm index 9489adf2e..6c37b4f6e 100644 --- a/miden-lib/asm/kernels/transaction/lib/prologue.masm +++ b/miden-lib/asm/kernels/transaction/lib/prologue.masm @@ -614,7 +614,7 @@ proc.process_account_data # validate account procedure metadata exec.account::validate_storage_offsets - # exec.account::validate_storage_sizes + exec.account::validate_storage_sizes end # INPUT NOTES DATA diff --git a/miden-lib/src/transaction/procedures.rs b/miden-lib/src/transaction/procedures.rs index 026af2fc4..65cbec15e 100644 --- a/miden-lib/src/transaction/procedures.rs +++ b/miden-lib/src/transaction/procedures.rs @@ -86,17 +86,17 @@ const KERNEL0_PROCEDURES: [Digest; 28] = [ ), // get_account_item digest!( - 13424289660465129162, - 11714494585523463335, - 6258080289656188530, - 11109254005343025041 + 12870368003864147532, + 135004044775175452, + 18341205336754684006, + 16511632309455617110 ), // get_account_map_item digest!( - 4086597549949492919, - 17234429278667747774, - 2017480864876659824, - 5288909086187322732 + 1722959968055615999, + 12509825287842064187, + 15844741291956278184, + 17329797087861318429 ), // get_account_nonce digest!( @@ -137,17 +137,17 @@ const KERNEL0_PROCEDURES: [Digest; 28] = [ digest!(791239201240238984, 990477779242440333, 9153970741752682788, 4799626513113428911), // set_account_item digest!( - 3488970757239817556, - 2169657214816177798, - 10864207859383826292, - 15560226360313617810 + 15764319909714146584, + 7841587466640850169, + 12756932643091096464, + 15802692116963007064 ), // set_account_map_item digest!( - 8895862041096145565, - 3313775403671697242, - 15357863624188673369, - 14860022603519750887 + 11056255565559326609, + 1357562972439875440, + 5479599748803854851, + 17981458485658454761 ), // burn_asset digest!( diff --git a/miden-tx/src/errors/tx_kernel_errors.rs b/miden-tx/src/errors/tx_kernel_errors.rs index 87144a9b6..07b71b44f 100644 --- a/miden-tx/src/errors/tx_kernel_errors.rs +++ b/miden-tx/src/errors/tx_kernel_errors.rs @@ -85,8 +85,9 @@ pub const ERR_NOTE_FUNGIBLE_MAX_AMOUNT_EXCEEDED: u32 = 131152; pub const ERR_NON_FUNGIBLE_ASSET_ALREADY_EXISTS: u32 = 131153; pub const ERR_INVALID_NOTE_IDX: u32 = 131154; pub const ERR_KERNEL_PROCEDURE_OFFSET_OUT_OF_BOUNDS: u32 = 131155; +pub const ERR_INVALID_STORAGE_OFFSET_FOR_SIZE: u32 = 131156; -pub const KERNEL_ERRORS: [(u32, &str); 84] = [ +pub const KERNEL_ERRORS: [(u32, &str); 85] = [ (ERR_FAUCET_RESERVED_DATA_SLOT, "For faucets, storage slot 254 is reserved and can not be used with set_account_item procedure"), (ERR_ACCT_MUST_BE_A_FAUCET, "Procedure can only be called from faucet accounts"), (ERR_P2ID_WRONG_NUMBER_OF_INPUTS, "P2ID scripts expect exactly 1 note input"), @@ -170,5 +171,6 @@ pub const KERNEL_ERRORS: [(u32, &str); 84] = [ (ERR_ACCT_TOO_MANY_PROCEDURES, "Number of account procedures exceeded the maximum limit of 256"), (ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS, "Provided storage slot index is out of bounds"), (ERR_INVALID_FAUCET_STORAGE_OFFSET, "Storage offset is invalid for a faucet account (0 is prohibited being the reserved faucet data slot)"), - (ERR_KERNEL_PROCEDURE_OFFSET_OUT_OF_BOUNDS, "Provided kernel procedure offset is out of bounds") + (ERR_KERNEL_PROCEDURE_OFFSET_OUT_OF_BOUNDS, "Provided kernel procedure offset is out of bounds"), + (ERR_INVALID_STORAGE_OFFSET_FOR_SIZE, "Storage offset is invalid for a procedure that does not access storage (should be 0)") ]; diff --git a/miden-tx/src/testing/tx_context/builder.rs b/miden-tx/src/testing/tx_context/builder.rs index 51b06e385..e38a48baa 100644 --- a/miden-tx/src/testing/tx_context/builder.rs +++ b/miden-tx/src/testing/tx_context/builder.rs @@ -52,7 +52,7 @@ pub struct TransactionContextBuilder { impl TransactionContextBuilder { pub fn new(account: Account) -> Self { Self { - assembler: TransactionKernel::testing_assembler(), + assembler: TransactionKernel::testing_assembler_with_mock_account(), account, account_seed: None, input_notes: Vec::new(), @@ -75,8 +75,7 @@ impl TransactionContextBuilder { TransactionKernel::testing_assembler(), ); - let assembler = - TransactionKernel::testing_assembler_with_mock_account().with_debug_mode(true); + let assembler = TransactionKernel::testing_assembler_with_mock_account(); Self { assembler: assembler.clone(), diff --git a/miden-tx/src/tests/kernel_tests/test_account.rs b/miden-tx/src/tests/kernel_tests/test_account.rs index 3401f9e33..4a9b8aa09 100644 --- a/miden-tx/src/tests/kernel_tests/test_account.rs +++ b/miden-tx/src/tests/kernel_tests/test_account.rs @@ -248,9 +248,16 @@ fn test_get_item() { #[test] fn test_get_map_item() { - let tx_context = TransactionContextBuilder::with_standard_account(ONE).build(); + let storage_slot = AccountStorage::mock_item_2().slot; + let (account, _) = AccountBuilder::new(ChaCha20Rng::from_entropy()) + .add_storage_slot(storage_slot) + .code(AccountCode::mock_account_code(TransactionKernel::testing_assembler(), false)) + .nonce(ONE) + .build() + .unwrap(); + + let tx_context = TransactionContextBuilder::new(account).build(); - let storage_item = AccountStorage::mock_item_2(); for (key, value) in STORAGE_LEAVES_2 { let code = format!( " @@ -265,7 +272,7 @@ fn test_get_map_item() { call.::test::account::get_map_item end ", - item_index = storage_item.index, + item_index = 0, map_key = prepare_word(&key), ); let process = tx_context.execute_code(&code).unwrap(); @@ -378,7 +385,15 @@ fn test_set_map_item() { [Felt::new(9_u64), Felt::new(10_u64), Felt::new(11_u64), Felt::new(12_u64)], ); - let tx_context = TransactionContextBuilder::with_standard_account(ONE).build(); + let storage_slot = AccountStorage::mock_item_2().slot; + let (account, _) = AccountBuilder::new(ChaCha20Rng::from_entropy()) + .add_storage_slot(storage_slot) + .code(AccountCode::mock_account_code(TransactionKernel::testing_assembler(), false)) + .nonce(ONE) + .build() + .unwrap(); + + let tx_context = TransactionContextBuilder::new(account).build(); let storage_item = AccountStorage::mock_item_2(); let code = format!( @@ -400,14 +415,14 @@ fn test_set_map_item() { call.account::get_item end ", - item_index = storage_item.index, + item_index = 0, new_key = prepare_word(&new_key), new_value = prepare_word(&new_value), ); let process = tx_context.execute_code(&code).unwrap(); - let mut new_storage_map = AccountStorage::mock_map_2(); + let mut new_storage_map = AccountStorage::mock_map(); new_storage_map.insert(new_key, new_value); assert_eq!( @@ -473,14 +488,14 @@ fn test_storage_offset() { // Setup account let code = AccountCode::compile(source_code, assembler.clone(), false).unwrap(); - // modify procedure offsets + // modify procedure storage offsets // TODO: We manually set the offsets here because we do not have the ability to set the // offsets through MASM for now. Remove this code when we enable this functionality. let procedures_with_offsets = vec![ - AccountProcedureInfo::new(*code.procedures()[0].mast_root(), 2, 0), - AccountProcedureInfo::new(*code.procedures()[1].mast_root(), 2, 0), - AccountProcedureInfo::new(*code.procedures()[2].mast_root(), 1, 0), - AccountProcedureInfo::new(*code.procedures()[3].mast_root(), 1, 0), + AccountProcedureInfo::new(*code.procedures()[0].mast_root(), 2, 1), + AccountProcedureInfo::new(*code.procedures()[1].mast_root(), 2, 1), + AccountProcedureInfo::new(*code.procedures()[2].mast_root(), 1, 1), + AccountProcedureInfo::new(*code.procedures()[3].mast_root(), 1, 1), ]; let code = AccountCode::from_parts(code.mast().clone(), procedures_with_offsets.clone()); diff --git a/miden-tx/src/tests/mod.rs b/miden-tx/src/tests/mod.rs index a47460ca5..c71a62c0d 100644 --- a/miden-tx/src/tests/mod.rs +++ b/miden-tx/src/tests/mod.rs @@ -8,7 +8,7 @@ use miden_objects::{ ACCOUNT_ID_NON_FUNGIBLE_FAUCET_ON_CHAIN, ACCOUNT_ID_REGULAR_ACCOUNT_IMMUTABLE_CODE_ON_CHAIN, }, - AccountCode, + AccountCode, AccountProcedureInfo, AccountStorage, }, assets::{Asset, FungibleAsset, NonFungibleAsset}, notes::{ @@ -16,6 +16,7 @@ use miden_objects::{ NoteMetadata, NoteRecipient, NoteScript, NoteTag, NoteType, }, testing::{ + account::AccountBuilder, constants::{FUNGIBLE_ASSET_AMOUNT, NON_FUNGIBLE_ASSET_DATA}, notes::DEFAULT_NOTE_CODE, prepare_word, @@ -25,6 +26,8 @@ use miden_objects::{ Felt, Word, MIN_PROOF_SECURITY_LEVEL, }; use miden_prover::ProvingOptions; +use rand::SeedableRng; +use rand_chacha::ChaCha20Rng; use vm_processor::{ utils::{Deserializable, Serializable}, Digest, MemAdviceProvider, ONE, @@ -104,7 +107,31 @@ fn transaction_executor_witness() { #[test] fn executed_transaction_account_delta() { - let mut tx_context = TransactionContextBuilder::with_standard_account(ONE) + let account_code = + AccountCode::mock_account_code(TransactionKernel::testing_assembler(), false); + + // modify procedure storage sizes + // TODO: We manually modify the sizes here to 3 because they are hardcoded as 1. + // Though we are accessing multiple storage slots up to 2 in this test. + // Remove this manual modification once we have the ability to set sizes using the assembler. + let procedures = account_code + .procedures() + .iter() + .map(|proc| AccountProcedureInfo::new(*proc.mast_root(), proc.storage_offset(), 3)) + .collect(); + let account_code = AccountCode::from_parts(account_code.mast(), procedures); + let (account, _) = AccountBuilder::new(ChaCha20Rng::from_entropy()) + .add_storage_slots([ + AccountStorage::mock_item_0().slot, + AccountStorage::mock_item_1().slot, + AccountStorage::mock_item_2().slot, + ]) + .code(account_code) + .nonce(ONE) + .build() + .unwrap(); + + let mut tx_context = TransactionContextBuilder::new(account) .with_mock_notes_preserved_with_account_vault_delta() .build(); diff --git a/objects/src/accounts/code/mod.rs b/objects/src/accounts/code/mod.rs index ab8704116..84e830284 100644 --- a/objects/src/accounts/code/mod.rs +++ b/objects/src/accounts/code/mod.rs @@ -58,11 +58,11 @@ impl AccountCode { pub fn new(library: Library, is_faucet: bool) -> Result { // extract procedure information from the library exports // TODO: currently, offsets for all regular account procedures are set to 0 - // and offsets for faucet accounts procedures are set to 1. Instead they should - // be read from the Library metadata. + // and offsets for faucet accounts procedures are set to 1. Furthermore sizes + // are set to 1 for all accounts. Instead they should be read from the Library metadata. let mut procedures: Vec = Vec::new(); let storage_offset = if is_faucet { 1 } else { 0 }; - let storage_size = 0; + let storage_size = 1; for module in library.module_infos() { for proc_mast_root in module.procedure_digests() { procedures.push(AccountProcedureInfo::new( diff --git a/objects/src/accounts/code/procedure.rs b/objects/src/accounts/code/procedure.rs index d3bcfee86..a468f6588 100644 --- a/objects/src/accounts/code/procedure.rs +++ b/objects/src/accounts/code/procedure.rs @@ -23,7 +23,7 @@ use crate::AccountError; /// /// The size is used to limit how many storage slots a given procedure can access in the associated /// account's storage. For example, if storage size for a procedure is set to 3, the procedure will -/// be bounded to access storage slots in the range [storage_offset, storage_offset + 3]. +/// be bounded to access storage slots in the range [storage_offset, storage_offset + 3 - 1]. #[derive(Debug, PartialEq, Eq, Clone)] pub struct AccountProcedureInfo { mast_root: Digest, diff --git a/objects/src/testing/storage.rs b/objects/src/testing/storage.rs index 8a4365821..f61bc69e0 100644 --- a/objects/src/testing/storage.rs +++ b/objects/src/testing/storage.rs @@ -157,12 +157,12 @@ impl AccountStorage { pub fn mock_item_2() -> SlotWithIndex { SlotWithIndex { - slot: StorageSlot::Map(Self::mock_map_2()), + slot: StorageSlot::Map(Self::mock_map()), index: STORAGE_INDEX_2, } } - pub fn mock_map_2() -> StorageMap { + pub fn mock_map() -> StorageMap { StorageMap::with_entries(STORAGE_LEAVES_2).unwrap() } } From 5ea268bf01882ee7d1b2ccc48d944bf3bed51c7a Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Thu, 19 Sep 2024 16:47:50 +0200 Subject: [PATCH 10/19] executed_transaction_account_delta works --- miden-tx/src/tests/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/miden-tx/src/tests/mod.rs b/miden-tx/src/tests/mod.rs index c71a62c0d..2d95ca142 100644 --- a/miden-tx/src/tests/mod.rs +++ b/miden-tx/src/tests/mod.rs @@ -10,7 +10,7 @@ use miden_objects::{ }, AccountCode, AccountProcedureInfo, AccountStorage, }, - assets::{Asset, FungibleAsset, NonFungibleAsset}, + assets::{Asset, AssetVault, FungibleAsset, NonFungibleAsset}, notes::{ Note, NoteAssets, NoteExecutionHint, NoteExecutionMode, NoteHeader, NoteId, NoteInputs, NoteMetadata, NoteRecipient, NoteScript, NoteTag, NoteType, @@ -109,6 +109,7 @@ fn transaction_executor_witness() { fn executed_transaction_account_delta() { let account_code = AccountCode::mock_account_code(TransactionKernel::testing_assembler(), false); + let account_assets = AssetVault::mock().assets().collect::>(); // modify procedure storage sizes // TODO: We manually modify the sizes here to 3 because they are hardcoded as 1. @@ -127,6 +128,7 @@ fn executed_transaction_account_delta() { AccountStorage::mock_item_2().slot, ]) .code(account_code) + .add_assets(account_assets) .nonce(ONE) .build() .unwrap(); From 27cb1d1137b75b55333d9ee1fe8a16e0633b9c8f Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Thu, 19 Sep 2024 17:32:51 +0200 Subject: [PATCH 11/19] All tests are passing --- miden-tx/tests/integration/scripts/faucet.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/miden-tx/tests/integration/scripts/faucet.rs b/miden-tx/tests/integration/scripts/faucet.rs index 4147c2dc1..3d219718b 100644 --- a/miden-tx/tests/integration/scripts/faucet.rs +++ b/miden-tx/tests/integration/scripts/faucet.rs @@ -4,7 +4,7 @@ use miden_lib::transaction::{memory::FAUCET_STORAGE_DATA_SLOT, TransactionKernel use miden_objects::{ accounts::{ account_id::testing::ACCOUNT_ID_FUNGIBLE_FAUCET_OFF_CHAIN, Account, AccountCode, AccountId, - AccountStorage, StorageSlot, + AccountProcedureInfo, AccountStorage, StorageSlot, }, assets::{Asset, AssetVault, FungibleAsset}, notes::{NoteAssets, NoteExecutionHint, NoteId, NoteMetadata, NoteTag, NoteType}, @@ -32,6 +32,20 @@ fn prove_faucet_contract_mint_fungible_asset_succeeds() { let (faucet_pub_key, falcon_auth) = get_new_pk_and_authenticator(); let faucet_account = get_faucet_account_with_max_supply_and_total_issuance(faucet_pub_key, 200, None); + let procedures = faucet_account + .code() + .procedures() + .iter() + .map(|proc| AccountProcedureInfo::new(*proc.mast_root(), proc.storage_offset(), 2)) + .collect(); + let account_code = AccountCode::from_parts(faucet_account.code().mast(), procedures); + let faucet_account = Account::from_parts( + faucet_account.id(), + faucet_account.vault().clone(), + faucet_account.storage().clone(), + account_code, + faucet_account.nonce(), + ); // CONSTRUCT AND EXECUTE TX (Success) // -------------------------------------------------------------------------------------------- From 0ecd4de35d4a6c5271ea7990b5049e5028e49401 Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Thu, 19 Sep 2024 17:43:32 +0200 Subject: [PATCH 12/19] updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd593c6d3..eb30740e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - [BREAKING] Refactored batch/block note trees (#834). - Set all procedures storage offsets of faucet accounts to `1` (#875). - Added `AccountStorageHeader` (#876). +- Implemented `storage_size`, updated storage bounds (#886). ## 0.5.1 (2024-08-28) - `miden-objects` crate only From a57a0ccf0e5930d9002f01646b8e7351229c55ee Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Fri, 20 Sep 2024 12:48:22 +0200 Subject: [PATCH 13/19] Move storage valdation logic to new accounts --- miden-lib/asm/kernels/transaction/lib/prologue.masm | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/miden-lib/asm/kernels/transaction/lib/prologue.masm b/miden-lib/asm/kernels/transaction/lib/prologue.masm index 6c37b4f6e..4ed6ca115 100644 --- a/miden-lib/asm/kernels/transaction/lib/prologue.masm +++ b/miden-lib/asm/kernels/transaction/lib/prologue.masm @@ -393,6 +393,11 @@ proc.validate_new_account # --------------------------------------------------------------------------------------------- exec.account::validate_seed # => [] + + # Assert the provided storage offsets and sizes satisfy storage requirements + # --------------------------------------------------------------------------------------------- + exec.account::validate_storage_offsets + exec.account::validate_storage_sizes end #! Validates that storage slots match storage commitment and saves storage slots into memory. @@ -611,10 +616,6 @@ proc.process_account_data assert.err=ERR_PROLOGUE_OLD_ACCT_NONCE_ZERO # => [] end - - # validate account procedure metadata - exec.account::validate_storage_offsets - exec.account::validate_storage_sizes end # INPUT NOTES DATA From 92f53d8e15306b2385fb23f7d5c470fca6a50f00 Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Fri, 20 Sep 2024 16:02:00 +0200 Subject: [PATCH 14/19] Added storage size and offset check on AccountProcedureInfo creation --- miden-lib/src/transaction/mod.rs | 5 ----- objects/src/accounts/code/procedure.rs | 7 +++++++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/miden-lib/src/transaction/mod.rs b/miden-lib/src/transaction/mod.rs index 7df42c321..623d6858d 100644 --- a/miden-lib/src/transaction/mod.rs +++ b/miden-lib/src/transaction/mod.rs @@ -95,11 +95,6 @@ impl TransactionKernel { ) -> (StackInputs, AdviceInputs) { let account = tx_inputs.account(); - // let (_, module_info, _) = Self::kernel().into_parts(); - // module_info.procedures().for_each(|(idx, proc_info)| { - // std::println!("{}. {}: {:?}", idx.as_usize(), proc_info.name, proc_info.digest) - // }); - let stack_inputs = TransactionKernel::build_input_stack( account.id(), account.init_hash(), diff --git a/objects/src/accounts/code/procedure.rs b/objects/src/accounts/code/procedure.rs index a468f6588..6d386ca0a 100644 --- a/objects/src/accounts/code/procedure.rs +++ b/objects/src/accounts/code/procedure.rs @@ -39,7 +39,14 @@ impl AccountProcedureInfo { // -------------------------------------------------------------------------------------------- /// Returns a new instance of an [AccountProcedureInfo]. + /// + /// # Panics + /// Panics if `storage_size` is 0 and `storage_offset` is not 0. pub fn new(mast_root: Digest, storage_offset: u8, storage_size: u8) -> Self { + if storage_size == 0 && storage_offset != 0 { + panic!("storage_offset must be 0 when storage_size is 0"); + } + Self { mast_root, storage_offset, storage_size } } From 816fbb7dc409c529be3557f1013bb28b152211e2 Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Tue, 1 Oct 2024 13:22:21 +0100 Subject: [PATCH 15/19] Lint --- bin/tx-prover/src/server/generated/api.rs | 83 ++++++++--------------- 1 file changed, 29 insertions(+), 54 deletions(-) diff --git a/bin/tx-prover/src/server/generated/api.rs b/bin/tx-prover/src/server/generated/api.rs index 082a941d1..9015ff32f 100644 --- a/bin/tx-prover/src/server/generated/api.rs +++ b/bin/tx-prover/src/server/generated/api.rs @@ -14,8 +14,7 @@ pub struct ProveTransactionResponse { /// Generated client implementations. pub mod api_client { #![allow(unused_variables, dead_code, missing_docs, clippy::let_unit_value)] - use tonic::codegen::*; - use tonic::codegen::http::Uri; + use tonic::codegen::{http::Uri, *}; #[derive(Debug, Clone)] pub struct ApiClient { inner: tonic::client::Grpc, @@ -46,10 +45,7 @@ pub mod api_client { let inner = tonic::client::Grpc::with_origin(inner, origin); Self { inner } } - pub fn with_interceptor( - inner: T, - interceptor: F, - ) -> ApiClient> + pub fn with_interceptor(inner: T, interceptor: F) -> ApiClient> where F: tonic::service::Interceptor, T::ResponseBody: Default, @@ -59,9 +55,8 @@ pub mod api_client { >::ResponseBody, >, >, - , - >>::Error: Into + Send + Sync, + >>::Error: + Into + Send + Sync, { ApiClient::new(InterceptedService::new(inner, interceptor)) } @@ -99,19 +94,14 @@ pub mod api_client { pub async fn prove_transaction( &mut self, request: impl tonic::IntoRequest, - ) -> std::result::Result< - tonic::Response, - tonic::Status, - > { - self.inner - .ready() - .await - .map_err(|e| { - tonic::Status::new( - tonic::Code::Unknown, - format!("Service was not ready: {}", e.into()), - ) - })?; + ) -> std::result::Result, tonic::Status> + { + self.inner.ready().await.map_err(|e| { + tonic::Status::new( + tonic::Code::Unknown, + format!("Service was not ready: {}", e.into()), + ) + })?; let codec = tonic::codec::ProstCodec::default(); let path = http::uri::PathAndQuery::from_static("/api.Api/ProveTransaction"); let mut req = request.into_request(); @@ -130,10 +120,7 @@ pub mod api_server { async fn prove_transaction( &self, request: tonic::Request, - ) -> std::result::Result< - tonic::Response, - tonic::Status, - >; + ) -> std::result::Result, tonic::Status>; } #[derive(Debug)] pub struct ApiServer { @@ -158,10 +145,7 @@ pub mod api_server { max_encoding_message_size: None, } } - pub fn with_interceptor( - inner: T, - interceptor: F, - ) -> InterceptedService + pub fn with_interceptor(inner: T, interceptor: F) -> InterceptedService where F: tonic::service::Interceptor, { @@ -217,23 +201,18 @@ pub mod api_server { "/api.Api/ProveTransaction" => { #[allow(non_camel_case_types)] struct ProveTransactionSvc(pub Arc); - impl< - T: Api, - > tonic::server::UnaryService - for ProveTransactionSvc { + impl tonic::server::UnaryService + for ProveTransactionSvc + { type Response = super::ProveTransactionResponse; - type Future = BoxFuture< - tonic::Response, - tonic::Status, - >; + type Future = BoxFuture, tonic::Status>; fn call( &mut self, request: tonic::Request, ) -> Self::Future { let inner = Arc::clone(&self.0); - let fut = async move { - ::prove_transaction(&inner, request).await - }; + let fut = + async move { ::prove_transaction(&inner, request).await }; Box::pin(fut) } } @@ -259,19 +238,15 @@ pub mod api_server { Ok(res) }; Box::pin(fut) - } - _ => { - Box::pin(async move { - Ok( - http::Response::builder() - .status(200) - .header("grpc-status", "12") - .header("content-type", "application/grpc") - .body(empty_body()) - .unwrap(), - ) - }) - } + }, + _ => Box::pin(async move { + Ok(http::Response::builder() + .status(200) + .header("grpc-status", "12") + .header("content-type", "application/grpc") + .body(empty_body()) + .unwrap()) + }), } } } From b1d38ada5fe65fb10bb9dafaaebeb76c2a05f950 Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Thu, 3 Oct 2024 16:28:26 +0100 Subject: [PATCH 16/19] Merge validate offsets and sizes --- .../asm/kernels/transaction/lib/account.masm | 115 ++++++------------ .../asm/kernels/transaction/lib/prologue.masm | 8 +- objects/src/accounts/code/procedure.rs | 3 +- 3 files changed, 42 insertions(+), 84 deletions(-) diff --git a/miden-lib/asm/kernels/transaction/lib/account.masm b/miden-lib/asm/kernels/transaction/lib/account.masm index 192d0f988..84b06d81a 100644 --- a/miden-lib/asm/kernels/transaction/lib/account.masm +++ b/miden-lib/asm/kernels/transaction/lib/account.masm @@ -327,16 +327,17 @@ export.apply_storage_offset # => [offset_slot_index] end -#! Validates all account procedures storage offsets by checking that: -#! - All storage offsets are in bounds +#! Validates all account procedures storage metadata by checking that: +#! - All storage offsets and sizes are in bounds #! - All storage offsets adhere to account type specific rules +#! - All procedures not accessing storage have offset and size set to 0 #! #! Notes: #! - For faucets checks that no storage offset is 0 (preventing reserved storage slot access) #! #! Stack: [] #! Output: [] -export.validate_storage_offsets +export.validate_storage_metadata # get number of account procedures and number of storage slots exec.memory::get_num_account_procedures exec.memory::get_num_storage_slots # => [num_storage_slots, num_account_procedures] @@ -353,30 +354,33 @@ export.validate_storage_offsets # account has between 1 and 256 procedures with associated offsets if.true while.true - # get storage offset from memory - dup exec.get_procedure_metadata swap drop - # => [storage_offset, index, num_storage_slots, num_account_procedures] + # get storage offset and size from memory + dup exec.get_procedure_metadata + # => [storage_offset, storage_size, index, num_storage_slots, num_account_procedures] # assert that storage offset is not 0 dup push.0 neq assert.err=ERR_INVALID_FAUCET_STORAGE_OFFSET - # => [storage_offset, index, num_storage_slots, num_account_procedures] + # => [storage_offset, storage_size, index, num_storage_slots, num_account_procedures] # TODO: This is a temporary check for faucets. We add 1 to all faucet offsets # to prevent access to the reserved faucet data slot. When the assembler - # will support storage offsets remove this check. + # will support storage offsets and sizes remove this check. # check if the storage offset and the number of storage slots are 1 - dup dup.3 eq.1 swap eq.1 and - # => [not_both_1, storage_offset, index, num_storage_slots, num_account_procedures] + dup dup.4 eq.1 swap eq.1 and + # => [both_1, storage_offset, storage_size, index, num_storage_slots, num_account_procedures] - if.false + if.true + # clean stack + drop drop + # => [index, num_storage_slots, get_num_account_procedures] + else # assert that storage offset is in bounds dup.2 lt assert.err=ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS # => [index, num_storage_slots, num_account_procedures] - else - # TODO: Remove this drop with the check above - # skip bound check and drop storage_offset - drop - # => [index, num_storage_slots, num_account_procedures] + + # assert that storage limit is in bounds + add sub.1 dup.2 lt assert.err=ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS + # => [index, num_storage_slots, get_num_account_procedures] end # check if we should continue looping @@ -385,76 +389,29 @@ export.validate_storage_offsets end else while.true - # get storage offset from memory - dup exec.get_procedure_metadata swap drop - # => [storage_offset, index, num_storage_slots, num_account_procedures] + # get storage offset and size from memory + dup exec.get_procedure_metadata + # => [storage_offset, storage_size, index, num_storage_slots, num_account_procedures] # assert that storage offset is in bounds - dup.2 lt assert.err=ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS - # => [index, num_storage_slots, num_account_procedures] + dup dup.4 lt assert.err=ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS + # => [storage_offset, storage_size, index, num_storage_slots, num_account_procedures] - # check if we should continue looping - add.1 dup dup.3 lt - # => [should_loop, index, num_storage_slots, num_account_procedures] - end - end - - # clean stack - drop drop drop - # => [] -end - -#! Validates all account procedures sizes by checking that: -#! - All storage sizes are in bounds -#! - All procedures not accessing storage have offset and size set to 0 -#! -#! Stack: [] -#! Output: [] -export.validate_storage_sizes - # get number of account procedures and number of storage slots - exec.memory::get_num_account_procedures exec.memory::get_num_storage_slots - # => [num_storage_slots, num_account_procedures] - - # prepare stack for looping - push.0.1 - # => [start_loop, index, num_storage_slots, num_account_procedures] + # assert that if size is 0 then offset is 0 + dup.1 eq.0 + if.true + dup eq.0 assert.err=ERR_INVALID_STORAGE_OFFSET_FOR_SIZE + end + # => [storage_offset, storage_size, index, num_storage_slots, num_account_procedures] - # we do not check if num_account_procedures == 0 here because a valid - # account has between 1 and 256 procedures with associated offsets - while.true - # get storage offset and size from memory - dup exec.get_procedure_metadata - # => [storage_offset, storage_size, index, num_storage_slots, num_account_procedures] - - # check if size is 0 - dup.1 eq.0 - if.true - # assert that a procedure not accessing storage has offset and size 0 - dup eq.0 assert.err=ERR_INVALID_STORAGE_OFFSET_FOR_SIZE - drop drop + # assert that the storage limit is in bounds + add sub.1 dup.2 lt assert.err=ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS # => [index, num_storage_slots, get_num_account_procedures] - else - # TODO: This is a temporary check for faucets. We add 1 to all faucet offsets - # to prevent access to the reserved faucet data slot. When the assembler - # will support storage offsets remove this check. - # check if the storage offset and the number of storage slots are 1 - dup dup.4 eq.1 swap eq.1 and - # => [not_both_1, storage_offset, storage_size, index, num_storage_slots, num_account_procedures] - if.false - # assert that storage limit is in bounds - add sub.1 dup.2 lt assert.err=ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS - # => [index, num_storage_slots, get_num_account_procedures] - else - # clean stack - drop drop - # => [index, num_storage_slots, get_num_account_procedures] - end + # check if we should continue looping + add.1 dup dup.3 lt + # => [should_loop, index, num_storage_slots, num_account_procedures] end - - # check if we should continue looping - add.1 dup dup.3 lt - # => [should_loop, index, num_storage_slots, num_account_procedures] end # clean stack diff --git a/miden-lib/asm/kernels/transaction/lib/prologue.masm b/miden-lib/asm/kernels/transaction/lib/prologue.masm index 4180eee16..c8ed52a0c 100644 --- a/miden-lib/asm/kernels/transaction/lib/prologue.masm +++ b/miden-lib/asm/kernels/transaction/lib/prologue.masm @@ -402,8 +402,8 @@ proc.validate_new_account # Assert the provided storage offsets and sizes satisfy storage requirements # --------------------------------------------------------------------------------------------- - exec.account::validate_storage_offsets - exec.account::validate_storage_sizes + exec.account::validate_storage_metadata + # => [] end #! Validates that storage slots match storage commitment and saves storage slots into memory. @@ -544,7 +544,7 @@ end #! - ACCOUNT_STORAGE_COMMITMENT, account's storage commitment. #! - ACCOUNT_CODE_COMMITMENT, account's code commitment. proc.process_account_data - # Initialize the current account data pointer in the bookkeeping section with the native offset + # Initialize the current account data pointer in the bookkeeping section with the native offset # (2048) exec.memory::set_current_account_data_ptr_to_native_account @@ -1241,6 +1241,6 @@ export.prepare_transaction exec.process_input_notes_data exec.process_tx_script_root # => [] - + push.MAX_BLOCK_NUM exec.memory::set_expiration_block_num end diff --git a/objects/src/accounts/code/procedure.rs b/objects/src/accounts/code/procedure.rs index d656841ae..5cfb772b5 100644 --- a/objects/src/accounts/code/procedure.rs +++ b/objects/src/accounts/code/procedure.rs @@ -24,6 +24,7 @@ use crate::AccountError; /// The size is used to limit how many storage slots a given procedure can access in the associated /// account's storage. For example, if storage size for a procedure is set to 3, the procedure will /// be bounded to access storage slots in the range [storage_offset, storage_offset + 3 - 1]. +/// Furthermore storage_size = 0 indicates that a procedure does not need to access storage. #[derive(Debug, PartialEq, Eq, Clone)] pub struct AccountProcedureInfo { mast_root: Digest, @@ -122,7 +123,7 @@ impl Serializable for AccountProcedureInfo { fn get_size_hint(&self) -> usize { self.mast_root.get_size_hint() + self.storage_offset.get_size_hint() - + self.storage_size().get_size_hint() + + self.storage_size.get_size_hint() } } From e42bd176a55d0e10108b05d039814ab5c552c2a6 Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Fri, 4 Oct 2024 17:25:32 +0100 Subject: [PATCH 17/19] Improved doc comments + Changed panic to Error --- .../asm/kernels/transaction/lib/account.masm | 11 ++++++---- .../src/transaction/procedures/kernel_v0.rs | 8 +++---- .../src/tests/kernel_tests/test_account.rs | 8 +++---- miden-tx/src/tests/mod.rs | 4 ++-- miden-tx/tests/integration/scripts/faucet.rs | 2 +- objects/src/accounts/code/mod.rs | 2 +- objects/src/accounts/code/procedure.rs | 22 ++++++++++++++----- objects/src/errors.rs | 2 ++ 8 files changed, 37 insertions(+), 22 deletions(-) diff --git a/miden-lib/asm/kernels/transaction/lib/account.masm b/miden-lib/asm/kernels/transaction/lib/account.masm index 84b06d81a..7265c7a1f 100644 --- a/miden-lib/asm/kernels/transaction/lib/account.masm +++ b/miden-lib/asm/kernels/transaction/lib/account.masm @@ -323,7 +323,7 @@ export.apply_storage_offset # => [offset_slot_index, storage_offset, storage_size] # verify that slot_index is in bounds - movdn.2 add sub.1 dup.1 gte assert.err=ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS + movdn.2 add dup.1 gt assert.err=ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS # => [offset_slot_index] end @@ -572,10 +572,11 @@ end #! Returns the procedure information #! #! Stack: [index, ...] -#! Output: [PROC_ROOT, storage_offset, ...] +#! Output: [PROC_ROOT, storage_offset, storage_size] #! #! - PROC_ROOT is the hash of the procedure. #! - storage_offset is the procedure storage offset. +#! - storage_size is the number of storage slots the procedure is allowed to access. #! #! Panics if #! - index is out of bounds @@ -600,9 +601,11 @@ end #! Verifies that the procedure root is part of the account code #! #! Stack: [PROC_ROOT] -#! Output: [storage_offset] +#! Output: [storage_offset, storage_size] #! #! - PROC_ROOT is the hash of the procedure to authenticate. +#! - storage_offset is the procedure storage offset. +#! - storage_size is the number of storage slots the procedure is allowed to access. #! #! Panics if #! - procedure root is not part of the account code. @@ -744,7 +747,7 @@ end #! Output: [storage_offset, storage_size] #! #! - storage_offset is the procedure storage offset. -#! - storage_size is the procedure storage size. +#! - storage_size is the number of storage slots the procedure is allowed to access. proc.get_procedure_metadata # get procedure storage metadata pointer mul.2 exec.memory::get_acct_procedures_section_ptr add add.1 diff --git a/miden-lib/src/transaction/procedures/kernel_v0.rs b/miden-lib/src/transaction/procedures/kernel_v0.rs index 8ca64b08b..fde910089 100644 --- a/miden-lib/src/transaction/procedures/kernel_v0.rs +++ b/miden-lib/src/transaction/procedures/kernel_v0.rs @@ -18,9 +18,9 @@ pub const KERNEL0_PROCEDURES: [Digest; 32] = [ // get_account_id digest!(0x386549d4435f79c1, 0x4a7add2e3b9f1b9e, 0x91c0af1138c14e77, 0xee8a5630e31bc74d), // get_account_item - digest!(0x9afb3c813c39cdba, 0x9bc4f37538512aed, 0xcc592e7820104593, 0x59a7fedcd9c30dc9), + digest!(0x614250d8c36af706, 0x46d39fb65480d1f3, 0xe0ebb7d5f46a6f32, 0x2bc18e17712bbbc5), // get_account_map_item - digest!(0x48cbbd91d5802b3d, 0x5a06de9f28e00bf3, 0xe4b05cb4d042df1d, 0x76a54f245408615d), + digest!(0xe055cca34d15fc7f, 0x815734bce550acd4, 0x50a827f81176640b, 0xb426738c7e29fb23), // get_account_nonce digest!(0x64d14d80f9eff37a, 0x7587e273b2d8a416, 0x3c041064332c03d3, 0xc327341072f4f1e8), // get_account_vault_commitment @@ -34,9 +34,9 @@ pub const KERNEL0_PROCEDURES: [Digest; 32] = [ // set_account_code digest!(0x90bc1f541f7adc63, 0xffa3daf2197fe496, 0xc72c5cedeb3482b, 0x5d6eac8e22abda40), // set_account_item - digest!(0xb60120026fe7a801, 0xe096b187ad23ebec, 0x86464c57975dd88, 0xe20e396dbc8a9649), + digest!(0xe77cd2a1c02ad66a, 0xa18d96ecd20c7ca8, 0x7114ec61e4db0bea, 0xe6b97475f1f4dcbc), // set_account_map_item - digest!(0xe085d5a90510c979, 0x9bd95341f0f07fb0, 0x4caf05f9ba8a5662, 0xd04560419bee4c83), + digest!(0x49092f6ea0d561f, 0x11528bb53882af83, 0x228c1352560481a, 0x79667f86e9a32dd), // burn_asset digest!(0x58e53cf050c1218e, 0x498f9b3f9904c03f, 0xbc341b7737247115, 0x3ea366d3bc90fe32), // get_fungible_faucet_total_issuance diff --git a/miden-tx/src/tests/kernel_tests/test_account.rs b/miden-tx/src/tests/kernel_tests/test_account.rs index a85f4e6af..433d2ac77 100644 --- a/miden-tx/src/tests/kernel_tests/test_account.rs +++ b/miden-tx/src/tests/kernel_tests/test_account.rs @@ -492,10 +492,10 @@ fn test_storage_offset() { // TODO: We manually set the offsets here because we do not have the ability to set the // offsets through MASM for now. Remove this code when we enable this functionality. let procedures_with_offsets = vec![ - AccountProcedureInfo::new(*code.procedures()[0].mast_root(), 2, 1), - AccountProcedureInfo::new(*code.procedures()[1].mast_root(), 2, 1), - AccountProcedureInfo::new(*code.procedures()[2].mast_root(), 1, 1), - AccountProcedureInfo::new(*code.procedures()[3].mast_root(), 1, 1), + AccountProcedureInfo::new(*code.procedures()[0].mast_root(), 2, 1).unwrap(), + AccountProcedureInfo::new(*code.procedures()[1].mast_root(), 2, 1).unwrap(), + AccountProcedureInfo::new(*code.procedures()[2].mast_root(), 1, 1).unwrap(), + AccountProcedureInfo::new(*code.procedures()[3].mast_root(), 1, 1).unwrap(), ]; let code = AccountCode::from_parts(code.mast().clone(), procedures_with_offsets.clone()); diff --git a/miden-tx/src/tests/mod.rs b/miden-tx/src/tests/mod.rs index 3cbd259a3..fdd3d5239 100644 --- a/miden-tx/src/tests/mod.rs +++ b/miden-tx/src/tests/mod.rs @@ -118,7 +118,7 @@ fn executed_transaction_account_delta() { let procedures = account_code .procedures() .iter() - .map(|proc| AccountProcedureInfo::new(*proc.mast_root(), proc.storage_offset(), 3)) + .map(|proc| AccountProcedureInfo::new(*proc.mast_root(), proc.storage_offset(), 3).unwrap()) .collect(); let account_code = AccountCode::from_parts(account_code.mast(), procedures); let (account, _) = AccountBuilder::new(ChaCha20Rng::from_entropy()) @@ -194,7 +194,7 @@ fn executed_transaction_account_delta() { send_asset_script.push_str(&format!( " ### note {i} - # prepare the stack for a new note creation + # prepare the stack for a new note creation push.0.1.2.3 # recipient push.{EXECUTION_HINT} # note_execution_hint push.{NOTETYPE} # note_type diff --git a/miden-tx/tests/integration/scripts/faucet.rs b/miden-tx/tests/integration/scripts/faucet.rs index 0e3fadb4b..d831c804a 100644 --- a/miden-tx/tests/integration/scripts/faucet.rs +++ b/miden-tx/tests/integration/scripts/faucet.rs @@ -36,7 +36,7 @@ fn prove_faucet_contract_mint_fungible_asset_succeeds() { .code() .procedures() .iter() - .map(|proc| AccountProcedureInfo::new(*proc.mast_root(), proc.storage_offset(), 2)) + .map(|proc| AccountProcedureInfo::new(*proc.mast_root(), proc.storage_offset(), 2).unwrap()) .collect(); let account_code = AccountCode::from_parts(faucet_account.code().mast(), procedures); let faucet_account = Account::from_parts( diff --git a/objects/src/accounts/code/mod.rs b/objects/src/accounts/code/mod.rs index 7e75ee956..2a904d144 100644 --- a/objects/src/accounts/code/mod.rs +++ b/objects/src/accounts/code/mod.rs @@ -69,7 +69,7 @@ impl AccountCode { proc_mast_root, storage_offset, storage_size, - )); + )?); } } diff --git a/objects/src/accounts/code/procedure.rs b/objects/src/accounts/code/procedure.rs index 5cfb772b5..539774ab8 100644 --- a/objects/src/accounts/code/procedure.rs +++ b/objects/src/accounts/code/procedure.rs @@ -1,10 +1,12 @@ +use std::string::ToString; + use vm_core::{ utils::{ByteReader, ByteWriter, Deserializable, Serializable}, FieldElement, }; use vm_processor::DeserializationError; -use super::{Digest, Felt}; +use super::{AccountCode, Digest, Felt}; use crate::AccountError; // ACCOUNT PROCEDURE INFO @@ -43,12 +45,20 @@ impl AccountProcedureInfo { /// /// # Panics /// Panics if `storage_size` is 0 and `storage_offset` is not 0. - pub fn new(mast_root: Digest, storage_offset: u8, storage_size: u8) -> Self { + pub fn new( + mast_root: Digest, + storage_offset: u8, + storage_size: u8, + ) -> Result { if storage_size == 0 && storage_offset != 0 { - panic!("storage_offset must be 0 when storage_size is 0"); + return Err(AccountError::ProcedureNotAccessingStorageHasOffsets); + } + + if (storage_offset + storage_size) as usize > AccountCode::MAX_NUM_PROCEDURES { + return Err(AccountError::StorageLimitOutOfBounds); } - Self { mast_root, storage_offset, storage_size } + Ok(Self { mast_root, storage_offset, storage_size }) } // PUBLIC ACCESSORS @@ -132,8 +142,8 @@ impl Deserializable for AccountProcedureInfo { let mast_root: Digest = source.read()?; let storage_offset = source.read_u8()?; let storage_size = source.read_u8()?; - - Ok(Self::new(mast_root, storage_offset, storage_size)) + Self::new(mast_root, storage_offset, storage_size) + .map_err(|err| DeserializationError::InvalidValue(err.to_string())) } } diff --git a/objects/src/errors.rs b/objects/src/errors.rs index bd1048203..ba2040ee9 100644 --- a/objects/src/errors.rs +++ b/objects/src/errors.rs @@ -45,6 +45,8 @@ pub enum AccountError { StorageSlotNotValue(u8), StorageIndexOutOfBounds { max: u8, actual: u8 }, StorageTooManySlots(u64), + StorageLimitOutOfBounds, + ProcedureNotAccessingStorageHasOffsets, } impl fmt::Display for AccountError { From 3a0d64bb84d46b05fc46c8ec65f6e9d7e96215e5 Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Fri, 4 Oct 2024 17:42:12 +0100 Subject: [PATCH 18/19] Fix no-std --- objects/src/accounts/code/procedure.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/objects/src/accounts/code/procedure.rs b/objects/src/accounts/code/procedure.rs index 539774ab8..c514b7506 100644 --- a/objects/src/accounts/code/procedure.rs +++ b/objects/src/accounts/code/procedure.rs @@ -1,4 +1,4 @@ -use std::string::ToString; +use alloc::string::ToString; use vm_core::{ utils::{ByteReader, ByteWriter, Deserializable, Serializable}, From 91dfb28d902a216ce0d67669377d58ba3db218e7 Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Mon, 7 Oct 2024 12:20:05 +0100 Subject: [PATCH 19/19] Add Errors, re-organize felts, improve namings --- .../asm/kernels/transaction/lib/account.masm | 27 ++++++++------- .../asm/kernels/transaction/lib/prologue.masm | 4 +-- .../src/transaction/procedures/kernel_v0.rs | 24 ++++++------- objects/src/accounts/code/mod.rs | 7 ++-- objects/src/accounts/code/procedure.rs | 34 +++++++++++-------- objects/src/errors.rs | 4 +-- 6 files changed, 53 insertions(+), 47 deletions(-) diff --git a/miden-lib/asm/kernels/transaction/lib/account.masm b/miden-lib/asm/kernels/transaction/lib/account.masm index 7265c7a1f..c813c2fcd 100644 --- a/miden-lib/asm/kernels/transaction/lib/account.masm +++ b/miden-lib/asm/kernels/transaction/lib/account.masm @@ -337,7 +337,7 @@ end #! #! Stack: [] #! Output: [] -export.validate_storage_metadata +export.validate_procedure_metadata # get number of account procedures and number of storage slots exec.memory::get_num_account_procedures exec.memory::get_num_storage_slots # => [num_storage_slots, num_account_procedures] @@ -372,15 +372,15 @@ export.validate_storage_metadata if.true # clean stack drop drop - # => [index, num_storage_slots, get_num_account_procedures] + # => [index, num_storage_slots, num_account_procedures] else # assert that storage offset is in bounds - dup.2 lt assert.err=ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS - # => [index, num_storage_slots, num_account_procedures] + dup dup.3 lt assert.err=ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS + # => [storage_offset, storage_size, index, num_storage_slots, num_account_procedures] # assert that storage limit is in bounds - add sub.1 dup.2 lt assert.err=ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS - # => [index, num_storage_slots, get_num_account_procedures] + add dup.2 lte assert.err=ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS + # => [index, num_storage_slots, num_account_procedures] end # check if we should continue looping @@ -397,6 +397,7 @@ export.validate_storage_metadata dup dup.4 lt assert.err=ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS # => [storage_offset, storage_size, index, num_storage_slots, num_account_procedures] + # TODO: Find a way to remove this `if` statement # assert that if size is 0 then offset is 0 dup.1 eq.0 if.true @@ -404,9 +405,9 @@ export.validate_storage_metadata end # => [storage_offset, storage_size, index, num_storage_slots, num_account_procedures] - # assert that the storage limit is in bounds - add sub.1 dup.2 lt assert.err=ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS - # => [index, num_storage_slots, get_num_account_procedures] + # assert that storage limit is in bounds + add dup.2 lte assert.err=ERR_STORAGE_SLOT_INDEX_OUT_OF_BOUNDS + # => [index, num_storage_slots, num_account_procedures] # check if we should continue looping add.1 dup dup.3 lt @@ -593,8 +594,8 @@ export.get_procedure_info padw movup.4 mem_loadw padw movup.8 mem_loadw # => [METADATA, PROC_ROOT] - # keep only relevant data - swap drop swap drop swap movdn.5 movdn.5 + # keep relevant data + drop drop swap movdn.5 movdn.5 # => [PROC_ROOT, storage_offset, storage_size] end @@ -753,7 +754,7 @@ proc.get_procedure_metadata mul.2 exec.memory::get_acct_procedures_section_ptr add add.1 # => [storage_offset_ptr] - # load procedure storage offset from memory and keep relevant data - padw movup.4 mem_loadw swap drop swap drop swap + # load procedure metadata from memory and keep relevant data + padw movup.4 mem_loadw drop drop swap # => [storage_offset, storage_size] end diff --git a/miden-lib/asm/kernels/transaction/lib/prologue.masm b/miden-lib/asm/kernels/transaction/lib/prologue.masm index c8ed52a0c..1da032421 100644 --- a/miden-lib/asm/kernels/transaction/lib/prologue.masm +++ b/miden-lib/asm/kernels/transaction/lib/prologue.masm @@ -400,9 +400,9 @@ proc.validate_new_account exec.account::validate_seed # => [] - # Assert the provided storage offsets and sizes satisfy storage requirements + # Assert the provided procedures offsets and sizes satisfy storage requirements # --------------------------------------------------------------------------------------------- - exec.account::validate_storage_metadata + exec.account::validate_procedure_metadata # => [] end diff --git a/miden-lib/src/transaction/procedures/kernel_v0.rs b/miden-lib/src/transaction/procedures/kernel_v0.rs index fde910089..a743730a9 100644 --- a/miden-lib/src/transaction/procedures/kernel_v0.rs +++ b/miden-lib/src/transaction/procedures/kernel_v0.rs @@ -8,19 +8,19 @@ use miden_objects::{digest, Digest, Felt}; /// Hashes of all dynamically executed procedures from the kernel 0. pub const KERNEL0_PROCEDURES: [Digest; 32] = [ // account_vault_add_asset - digest!(0x7215093ef8c739ae, 0xa2744b83f88eb2e, 0x4bca2840de404d50, 0x267ca24ec412e967), + digest!(0x77365035d901b352, 0x85d8042000096df, 0xa8531ec691f24d17, 0xc67a8fd2677bf558), // account_vault_get_balance digest!(0x92b81d20684fa47, 0x4920ee53425609b9, 0x2f8c32c56898141c, 0x9e4542839e34452f), // account_vault_has_non_fungible_asset digest!(0x1b1e6ec92fabca80, 0xbb3847ce15f98cac, 0x7152391739b5e0b3, 0x696aaf2c879c4fde), // account_vault_remove_asset - digest!(0x82c1762488a5fa06, 0x6d64bc2b046147ae, 0x89dec46bcea59bbd, 0xe0f03be7ffc4dcc3), + digest!(0xdf93ea4374fe098f, 0x63df56e7578d9661, 0xc5d3b1958456cc5, 0xbfeec68c1c6b4ca9), // get_account_id digest!(0x386549d4435f79c1, 0x4a7add2e3b9f1b9e, 0x91c0af1138c14e77, 0xee8a5630e31bc74d), // get_account_item - digest!(0x614250d8c36af706, 0x46d39fb65480d1f3, 0xe0ebb7d5f46a6f32, 0x2bc18e17712bbbc5), + digest!(0x83380522a33f8c7e, 0x1653bbd634d31107, 0x868fac07b1cb4005, 0x39bee294dac7fdc9), // get_account_map_item - digest!(0xe055cca34d15fc7f, 0x815734bce550acd4, 0x50a827f81176640b, 0xb426738c7e29fb23), + digest!(0xdf739f276157cf90, 0x4c94a55654d426b, 0xff2528216462fa83, 0x45797577ddc9a224), // get_account_nonce digest!(0x64d14d80f9eff37a, 0x7587e273b2d8a416, 0x3c041064332c03d3, 0xc327341072f4f1e8), // get_account_vault_commitment @@ -30,23 +30,23 @@ pub const KERNEL0_PROCEDURES: [Digest; 32] = [ // get_initial_account_hash digest!(0xe239391d2c860c53, 0x7a9d09c3015d7417, 0x111e9be3640d3848, 0xf2d442cf1e685a89), // incr_account_nonce - digest!(0xaa2904a4bec929ca, 0xc66e25b357383da8, 0x959f72ae1af83a30, 0x479718e0d289c3e), + digest!(0x6d75402ead2fe81c, 0x6e66c9ec980ec9cd, 0xe82e007b0eda78f1, 0xea9de83af0fc2634), // set_account_code - digest!(0x90bc1f541f7adc63, 0xffa3daf2197fe496, 0xc72c5cedeb3482b, 0x5d6eac8e22abda40), + digest!(0x62110f0b57e49ee5, 0xd961174262cd614a, 0x3459572bcf110091, 0x319291c6c18ad0db), // set_account_item - digest!(0xe77cd2a1c02ad66a, 0xa18d96ecd20c7ca8, 0x7114ec61e4db0bea, 0xe6b97475f1f4dcbc), + digest!(0xc279aa203249464, 0x464f69a21be47e7a, 0xb9161aaee45f0ff5, 0xbca81ff227c9ca03), // set_account_map_item - digest!(0x49092f6ea0d561f, 0x11528bb53882af83, 0x228c1352560481a, 0x79667f86e9a32dd), + digest!(0x85c7e78d8e33f81, 0x2392bd80e65f27a7, 0x69d4d656a994dd2c, 0xcb9be97522be5cf4), // burn_asset - digest!(0x58e53cf050c1218e, 0x498f9b3f9904c03f, 0xbc341b7737247115, 0x3ea366d3bc90fe32), + digest!(0x3c71836eaa5fba1b, 0xee719bcada360cd1, 0xad55420b925fd10d, 0x4d32e15e121e5e3e), // get_fungible_faucet_total_issuance digest!(0xd9310aaf087d0dc4, 0xdc834fff6ea325d2, 0x2c9d90a33b9a6d8a, 0xa381c27e49c538a8), // mint_asset - digest!(0xf1f416bd8fa21c94, 0x35c470f8c7a1eb1b, 0xc8c0b0b497dfe7a7, 0xea434f9afeb44bc1), + digest!(0x715eae96f4068cf1, 0x84ee32a7c64a85dd, 0x9b4d5a63fbd97064, 0xef0e81abf63aa2be), // add_asset_to_note - digest!(0x88f847f7b2f8f5f6, 0x301fa80970aea476, 0x1a09aa5379870f6, 0x76f121f8d5db2a0d), + digest!(0x9fbed6f52f2cc62d, 0xda9c2f699fac16fb, 0xeb6b8827beac6c95, 0xe27fc6900c673e2d), // create_note - digest!(0xd6ac7c95ba08b35e, 0x1ad1759445a51a95, 0x7cd321aa88e80729, 0x7804e81109ddc342), + digest!(0xa9e52dd343a6fa1d, 0xa54d666e10f34357, 0x7c53cc941096bd84, 0xe601314453890dfc), // get_input_notes_commitment digest!(0x1c078486abf976f5, 0xfce31a9f4b9687cd, 0xb1edb2edc115a619, 0xf1bb8c1bd9c7148b), // get_note_assets_info diff --git a/objects/src/accounts/code/mod.rs b/objects/src/accounts/code/mod.rs index 2a904d144..f275eccaa 100644 --- a/objects/src/accounts/code/mod.rs +++ b/objects/src/accounts/code/mod.rs @@ -53,14 +53,15 @@ impl AccountCode { /// location 0. /// /// # Errors - /// - If the number of procedures exported from the provided library is smaller than 1 or - /// greater than 256. + /// - If the number of procedures exported from the provided library is 0. + /// - If the number of procedures exported from the provided library is greater than 256. + /// - If the creation of a new `AccountProcedureInfo` fails. pub fn new(library: Library, is_faucet: bool) -> Result { // extract procedure information from the library exports // TODO: currently, offsets for all regular account procedures are set to 0 // and offsets for faucet accounts procedures are set to 1. Furthermore sizes // are set to 1 for all accounts. Instead they should be read from the Library metadata. - let mut procedures: Vec = Vec::new(); + let mut procedures = Vec::new(); let storage_offset = if is_faucet { 1 } else { 0 }; let storage_size = 1; for module in library.module_infos() { diff --git a/objects/src/accounts/code/procedure.rs b/objects/src/accounts/code/procedure.rs index c514b7506..ae4d4a4f5 100644 --- a/objects/src/accounts/code/procedure.rs +++ b/objects/src/accounts/code/procedure.rs @@ -6,8 +6,8 @@ use vm_core::{ }; use vm_processor::DeserializationError; -use super::{AccountCode, Digest, Felt}; -use crate::AccountError; +use super::{Digest, Felt}; +use crate::{accounts::AccountStorage, AccountError}; // ACCOUNT PROCEDURE INFO // ================================================================================================ @@ -43,19 +43,23 @@ impl AccountProcedureInfo { /// Returns a new instance of an [AccountProcedureInfo]. /// - /// # Panics - /// Panics if `storage_size` is 0 and `storage_offset` is not 0. + /// # Errors + /// - If `storage_size` is 0 and `storage_offset` is not 0. + /// - If `storage_size + storage_offset` is greater than `MAX_NUM_STORAGE_SLOTS`. pub fn new( mast_root: Digest, storage_offset: u8, storage_size: u8, ) -> Result { if storage_size == 0 && storage_offset != 0 { - return Err(AccountError::ProcedureNotAccessingStorageHasOffsets); + return Err(AccountError::PureProcedureWithStorageOffset); } - if (storage_offset + storage_size) as usize > AccountCode::MAX_NUM_PROCEDURES { - return Err(AccountError::StorageLimitOutOfBounds); + if (storage_offset + storage_size) as usize > AccountStorage::MAX_NUM_STORAGE_SLOTS { + return Err(AccountError::StorageOffsetOutOfBounds { + max: AccountStorage::MAX_NUM_STORAGE_SLOTS as u8, + actual: storage_offset + storage_size, + }); } Ok(Self { mast_root, storage_offset, storage_size }) @@ -91,7 +95,7 @@ impl From for [Felt; 8] { result[4] = Felt::from(value.storage_offset); // copy the storage size into value[7] - result[7] = Felt::from(value.storage_size); + result[5] = Felt::from(value.storage_size); result } @@ -109,16 +113,16 @@ impl TryFrom<[Felt; 8]> for AccountProcedureInfo { .try_into() .map_err(|_| AccountError::AccountCodeProcedureInvalidStorageOffset)?; - // Check if the next two elements are zero - if value[5] != Felt::ZERO || value[6] != Felt::ZERO { - return Err(AccountError::AccountCodeProcedureInvalidPadding); - } - - // get storage_size form value[7] - let storage_size: u8 = value[7] + // get storage_size form value[5] + let storage_size: u8 = value[5] .try_into() .map_err(|_| AccountError::AccountCodeProcedureInvalidStorageSize)?; + // Check if the remaining values are 0 + if value[6] != Felt::ZERO || value[7] != Felt::ZERO { + return Err(AccountError::AccountCodeProcedureInvalidPadding); + } + Ok(Self { mast_root, storage_offset, storage_size }) } } diff --git a/objects/src/errors.rs b/objects/src/errors.rs index ba2040ee9..2338c31a0 100644 --- a/objects/src/errors.rs +++ b/objects/src/errors.rs @@ -45,8 +45,8 @@ pub enum AccountError { StorageSlotNotValue(u8), StorageIndexOutOfBounds { max: u8, actual: u8 }, StorageTooManySlots(u64), - StorageLimitOutOfBounds, - ProcedureNotAccessingStorageHasOffsets, + StorageOffsetOutOfBounds { max: u8, actual: u8 }, + PureProcedureWithStorageOffset, } impl fmt::Display for AccountError {