From c93c547fce30edb6f4df3f0025beb26336b73297 Mon Sep 17 00:00:00 2001 From: Paul-Henry Kajfasz Date: Mon, 29 Jul 2024 14:56:37 +0100 Subject: [PATCH] Fixed check, added constants, removed testing account_procs --- miden-tx/src/host/account_procs.rs | 44 ++++++++++----- miden-tx/src/host/mod.rs | 2 +- miden-tx/src/testing/account_procs.rs | 79 --------------------------- miden-tx/src/testing/mock_host.rs | 4 +- miden-tx/src/testing/mod.rs | 1 - 5 files changed, 32 insertions(+), 98 deletions(-) delete mode 100644 miden-tx/src/testing/account_procs.rs diff --git a/miden-tx/src/host/account_procs.rs b/miden-tx/src/host/account_procs.rs index f078b8044..1644026a2 100644 --- a/miden-tx/src/host/account_procs.rs +++ b/miden-tx/src/host/account_procs.rs @@ -20,44 +20,51 @@ impl AccountProcedureIndexMap { adv_provider: &A, ) -> Result { // get the account procedures from the advice_map - let procs = adv_provider.get_mapped_values(&account_code_commitment).ok_or_else(|| { - TransactionHostError::AccountProcedureIndexMapError( - "Failed to read account procedure data from the advice provider".to_string(), - ) - })?; + let proc_data = + adv_provider.get_mapped_values(&account_code_commitment).ok_or_else(|| { + TransactionHostError::AccountProcedureIndexMapError( + "Failed to read account procedure data from the advice provider".to_string(), + ) + })?; let mut result = BTreeMap::new(); // sanity checks // check that there are procedures in the account code - if procs.is_empty() { + if proc_data.is_empty() { return Err(TransactionHostError::AccountProcedureIndexMapError( "The account code does not contain any procedures.".to_string(), )); } // check that the account code does not contain too many procedures - if procs.len() > AccountCode::MAX_NUM_PROCEDURES { + if proc_data[0].as_int() > AccountCode::MAX_NUM_PROCEDURES as u64 { return Err(TransactionHostError::AccountProcedureIndexMapError( "The account code contains too many procedures.".to_string(), )); } // check that the stored number of procedures matches the length of the procedures array - if procs[0].as_int() * 8 != procs.len() as u64 - 1 { + if proc_data[0].as_int() * AccountProcedureInfo::NUM_ELEMENTS_PER_PROC as u64 + != proc_data.len() as u64 - 1 + { return Err(TransactionHostError::AccountProcedureIndexMapError( "Invalid number of procedures.".to_string(), )); } - // we skip procs[0] because it's the number of procedures - for (proc_idx, proc_info) in procs[1..].chunks_exact(8).enumerate() { - let proc_info_array: [Felt; 8] = proc_info.try_into().map_err(|_| { - TransactionHostError::AccountProcedureIndexMapError( - "Invalid procedure info length.".to_string(), - ) - })?; + // we skip proc_data[0] because it's the number of procedures + for (proc_idx, proc_info) in proc_data[1..] + .chunks_exact(AccountProcedureInfo::NUM_ELEMENTS_PER_PROC) + .enumerate() + { + let proc_info_array: [Felt; AccountProcedureInfo::NUM_ELEMENTS_PER_PROC] = + proc_info.try_into().map_err(|_| { + TransactionHostError::AccountProcedureIndexMapError( + "Invalid procedure info length.".to_string(), + ) + })?; let procedure = AccountProcedureInfo::try_from(proc_info_array).map_err(|e| { TransactionHostError::AccountProcedureIndexMapError(format!( @@ -90,6 +97,13 @@ impl AccountProcedureIndexMap { process: &S, ) -> Result { let proc_root = process.get_stack_word(0).into(); + + // mock account method for testing from root context + // TODO: figure out if we can get rid of this + if proc_root == Digest::default() { + return Ok(255); + } + self.0 .get(&proc_root) .cloned() diff --git a/miden-tx/src/host/mod.rs b/miden-tx/src/host/mod.rs index c09d0481b..eeaeb7c37 100644 --- a/miden-tx/src/host/mod.rs +++ b/miden-tx/src/host/mod.rs @@ -18,7 +18,7 @@ use vm_processor::{ mod account_delta_tracker; use account_delta_tracker::AccountDeltaTracker; -mod account_procs; +pub mod account_procs; use account_procs::AccountProcedureIndexMap; mod note_builder; diff --git a/miden-tx/src/testing/account_procs.rs b/miden-tx/src/testing/account_procs.rs deleted file mode 100644 index daf695fe8..000000000 --- a/miden-tx/src/testing/account_procs.rs +++ /dev/null @@ -1,79 +0,0 @@ -use alloc::collections::BTreeMap; -use std::panic; - -use miden_lib::transaction::TransactionKernelError; -use miden_objects::accounts::{AccountCode, AccountProcedureInfo}; -use vm_processor::{AdviceProvider, Digest, Felt, ProcessState}; - -// ACCOUNT PROCEDURE INDEX MAP -// ================================================================================================ - -/// A map of proc_root |-> proc_index for all known procedures of an account interface. -pub struct AccountProcedureIndexMap(BTreeMap); - -impl AccountProcedureIndexMap { - /// Returns a new [AccountProcedureIndexMap] instantiated with account procedures present in - /// the provided advice provider. - pub fn new(account_code_commitment: Digest, adv_provider: &A) -> Self { - // get the account procedures from the advice_map - let procs = adv_provider - .get_mapped_values(&account_code_commitment) - .expect("Failed to read account procedure data from the advice provider"); - - let mut result = BTreeMap::new(); - - // sanity checks - - // check that there are procedures in the account code - if procs.is_empty() { - panic!("The account code does not contain any procedures."); - } - - // check that the account code does not contain too many procedures - if procs.len() > AccountCode::MAX_NUM_PROCEDURES { - panic!("The account code contains too many procedures."); - } - - // check that the stored number of procedures matches the length of the procedures array - if procs[0].as_int() * 8 != procs.len() as u64 - 1 { - panic!("Invalid number of procedures.") - } - - // we skip procs[0] because it's the number of procedures - for (proc_idx, proc_info) in procs[1..].chunks_exact(8).enumerate() { - let proc_info_array: [Felt; 8] = - proc_info.try_into().expect("Invalid procedure info length"); - - let procedure = AccountProcedureInfo::try_from(proc_info_array) - .expect("Failed to create AccountProcedure: {:?}"); - - let proc_idx = u16::try_from(proc_idx).expect("Invalid procedure index"); - - result.insert(*procedure.mast_root(), proc_idx); - } - - Self(result) - } - - /// Returns index of the procedure whose root is currently at the top of the operand stack in - /// the provided process. - /// - /// # Errors - /// Returns an error if the procedure at the top of the operand stack is not present in this - /// map. - pub fn get_proc_index( - &self, - process: &S, - ) -> Result { - let proc_root = process.get_stack_word(0).into(); - // mock account method for testing from root context - // TODO: figure out if we can get rid of this - if proc_root == Digest::default() { - return Ok(255); - } - self.0 - .get(&proc_root) - .cloned() - .ok_or(TransactionKernelError::UnknownAccountProcedure(proc_root)) - } -} diff --git a/miden-tx/src/testing/mock_host.rs b/miden-tx/src/testing/mock_host.rs index bb025e44a..3fd76dcc5 100644 --- a/miden-tx/src/testing/mock_host.rs +++ b/miden-tx/src/testing/mock_host.rs @@ -10,7 +10,7 @@ use vm_processor::{ ExecutionError, Host, HostResponse, MemAdviceProvider, ProcessState, }; -use super::account_procs::AccountProcedureIndexMap; +use crate::host::account_procs::AccountProcedureIndexMap; // MOCK HOST // ================================================================================================ @@ -32,7 +32,7 @@ impl MockHost { AccountProcedureIndexMap::new(account.code_commitment(), &adv_provider); Self { adv_provider, - acct_procedure_index_map: proc_index_map, + acct_procedure_index_map: proc_index_map.unwrap(), } } diff --git a/miden-tx/src/testing/mod.rs b/miden-tx/src/testing/mod.rs index 9d8cf2670..50efb4e12 100644 --- a/miden-tx/src/testing/mod.rs +++ b/miden-tx/src/testing/mod.rs @@ -1,4 +1,3 @@ -mod account_procs; pub mod executor; pub use mock_host::MockHost;