From 0dc99b84033a247ee8185c76c67dcdad3824a584 Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Mon, 3 Jul 2023 18:21:31 -0700 Subject: [PATCH] Turn dynamic borrow panics into HostErrors --- soroban-env-host/src/budget.rs | 86 +++---- .../src/cost_runner/cost_types/vm_ops.rs | 34 ++- soroban-env-host/src/cost_runner/runner.rs | 2 +- soroban-env-host/src/events/diagnostic.rs | 22 +- soroban-env-host/src/events/mod.rs | 4 +- soroban-env-host/src/host.rs | 217 ++++++++++++------ soroban-env-host/src/host/data_helper.rs | 18 +- soroban-env-host/src/host/error.rs | 48 +++- soroban-env-host/src/host/frame.rs | 53 +++-- soroban-env-host/src/host_object.rs | 6 +- soroban-env-host/src/storage.rs | 2 +- soroban-env-host/src/test/auth.rs | 6 +- soroban-env-host/src/test/budget_metering.rs | 26 +-- soroban-env-host/src/test/complex.rs | 4 +- soroban-env-host/src/test/event.rs | 24 +- soroban-env-host/src/test/hostile.rs | 6 +- soroban-env-host/src/test/invocation.rs | 6 +- soroban-env-host/src/test/ledger.rs | 2 +- soroban-env-host/src/test/lifecycle.rs | 5 +- soroban-env-host/src/test/prng.rs | 4 +- soroban-env-host/src/test/storage.rs | 2 +- soroban-env-host/src/test/token.rs | 7 +- soroban-env-host/src/test/util.rs | 27 +-- soroban-env-host/src/vm.rs | 24 +- soroban-env-host/tests/integration.rs | 2 +- 25 files changed, 384 insertions(+), 253 deletions(-) diff --git a/soroban-env-host/src/budget.rs b/soroban-env-host/src/budget.rs index 9a551b4b9..467f43e79 100644 --- a/soroban-env-host/src/budget.rs +++ b/soroban-env-host/src/budget.rs @@ -7,6 +7,7 @@ use std::{ use soroban_env_common::xdr::{ScErrorCode, ScErrorType}; use crate::{ + host::error::TryBorrowOrErr, xdr::{ContractCostParamEntry, ContractCostParams, ContractCostType, ExtensionPoint}, Host, HostError, }; @@ -412,13 +413,13 @@ pub struct Budget(pub(crate) Rc>); impl Debug for Budget { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - writeln!(f, "{:?}", self.0.borrow()) + writeln!(f, "{:?}", self.0.try_borrow().map_err(|_| std::fmt::Error)?) } } impl Display for Budget { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - writeln!(f, "{}", self.0.borrow()) + writeln!(f, "{}", self.0.try_borrow().map_err(|_| std::fmt::Error)?) } } @@ -465,7 +466,7 @@ impl Budget { where F: FnOnce(RefMut) -> Result, { - f(self.0.borrow_mut()) + f(self.0.try_borrow_mut_or_err()?) } fn charge_in_bulk( @@ -474,7 +475,7 @@ impl Budget { iterations: u64, input: Option, ) -> Result<(), HostError> { - if !self.0.borrow().enabled { + if !self.0.try_borrow_or_err()?.enabled { return Ok(()); } @@ -562,98 +563,97 @@ impl Budget { res } - pub fn get_tracker(&self, ty: ContractCostType) -> (u64, Option) { - self.0.borrow().tracker[ty as usize] + pub fn get_tracker(&self, ty: ContractCostType) -> Result<(u64, Option), HostError> { + Ok(self.0.try_borrow_or_err()?.tracker[ty as usize]) } pub(crate) fn get_tracker_mut(&self, ty: ContractCostType, f: F) -> Result<(), HostError> where F: FnOnce(&mut (u64, Option)) -> Result<(), HostError>, { - f(&mut self.0.borrow_mut().tracker[ty as usize]) + f(&mut self.0.try_borrow_mut_or_err()?.tracker[ty as usize]) } - pub fn get_cpu_insns_consumed(&self) -> u64 { - self.0.borrow().cpu_insns.get_total_count() + pub fn get_cpu_insns_consumed(&self) -> Result { + Ok(self.0.try_borrow_or_err()?.cpu_insns.get_total_count()) } - pub fn get_mem_bytes_consumed(&self) -> u64 { - self.0.borrow().mem_bytes.get_total_count() + pub fn get_mem_bytes_consumed(&self) -> Result { + Ok(self.0.try_borrow_or_err()?.mem_bytes.get_total_count()) } - pub fn get_cpu_insns_remaining(&self) -> u64 { - self.0.borrow().cpu_insns.get_remaining() + pub fn get_cpu_insns_remaining(&self) -> Result { + Ok(self.0.try_borrow_or_err()?.cpu_insns.get_remaining()) } - pub fn get_mem_bytes_remaining(&self) -> u64 { - self.0.borrow().mem_bytes.get_remaining() + pub fn get_mem_bytes_remaining(&self) -> Result { + Ok(self.0.try_borrow_or_err()?.mem_bytes.get_remaining()) } - pub fn reset_default(&self) { - *self.0.borrow_mut() = BudgetImpl::default() + pub fn reset_default(&self) -> Result<(), HostError> { + *self.0.try_borrow_mut_or_err()? = BudgetImpl::default(); + Ok(()) } - pub fn reset_unlimited(&self) { - self.reset_unlimited_cpu(); - self.reset_unlimited_mem(); + pub fn reset_unlimited(&self) -> Result<(), HostError> { + self.reset_unlimited_cpu()?; + self.reset_unlimited_mem()?; + Ok(()) } - pub fn reset_unlimited_cpu(&self) { + pub fn reset_unlimited_cpu(&self) -> Result<(), HostError> { self.mut_budget(|mut b| { b.cpu_insns.reset(u64::MAX); Ok(()) - }) - .unwrap(); // panic means multiple-mut-borrow bug + })?; // panic means multiple-mut-borrow bug self.reset_tracker() } - pub fn reset_unlimited_mem(&self) { + pub fn reset_unlimited_mem(&self) -> Result<(), HostError> { self.mut_budget(|mut b| { b.mem_bytes.reset(u64::MAX); Ok(()) - }) - .unwrap(); // panic means multiple-mut-borrow bug + })?; self.reset_tracker() } - pub fn reset_tracker(&self) { - for tracker in self.0.borrow_mut().tracker.iter_mut() { + pub fn reset_tracker(&self) -> Result<(), HostError> { + for tracker in self.0.try_borrow_mut_or_err()?.tracker.iter_mut() { tracker.0 = 0; tracker.1 = tracker.1.map(|_| 0); } + Ok(()) } - pub fn reset_limits(&self, cpu: u64, mem: u64) { + pub fn reset_limits(&self, cpu: u64, mem: u64) -> Result<(), HostError> { self.mut_budget(|mut b| { b.cpu_insns.reset(cpu); b.mem_bytes.reset(mem); Ok(()) - }) - .unwrap(); // impossible to panic - + })?; self.reset_tracker() } #[cfg(test)] - pub fn reset_models(&self) { + pub fn reset_models(&self) -> Result<(), HostError> { self.mut_budget(|mut b| { b.cpu_insns.reset_models(); b.mem_bytes.reset_models(); Ok(()) }) - .unwrap(); // impossible to panic } #[cfg(any(test, feature = "testutils"))] - pub fn reset_fuel_config(&self) { - self.0.borrow_mut().fuel_config.reset() + pub fn reset_fuel_config(&self) -> Result<(), HostError> { + self.0.try_borrow_mut_or_err()?.fuel_config.reset(); + Ok(()) } fn get_cpu_insns_remaining_as_fuel(&self) -> Result { - let cpu_remaining = self.get_cpu_insns_remaining(); + let cpu_remaining = self.get_cpu_insns_remaining()?; let cpu_per_fuel = self .0 - .borrow() + .try_borrow_or_err()? .cpu_insns .get_cost_model(ContractCostType::WasmInsnExec) .linear_term; @@ -676,10 +676,10 @@ impl Budget { } fn get_mem_bytes_remaining_as_fuel(&self) -> Result { - let bytes_remaining = self.get_mem_bytes_remaining(); + let bytes_remaining = self.get_mem_bytes_remaining()?; let bytes_per_fuel = self .0 - .borrow() + .try_borrow_or_err()? .mem_bytes .get_cost_model(ContractCostType::WasmMemAlloc) .linear_term; @@ -699,15 +699,15 @@ impl Budget { } // generate a wasmi fuel cost schedule based on our calibration - pub fn wasmi_fuel_costs(&self) -> FuelCosts { - let config = &self.0.borrow().fuel_config; + pub fn wasmi_fuel_costs(&self) -> Result { + let config = &self.0.try_borrow_or_err()?.fuel_config; let mut costs = FuelCosts::default(); costs.base = config.base; costs.entity = config.entity; costs.load = config.load; costs.store = config.store; costs.call = config.call; - costs + Ok(costs) } } diff --git a/soroban-env-host/src/cost_runner/cost_types/vm_ops.rs b/soroban-env-host/src/cost_runner/cost_types/vm_ops.rs index 661787434..4868b4448 100644 --- a/soroban-env-host/src/cost_runner/cost_types/vm_ops.rs +++ b/soroban-env-host/src/cost_runner/cost_types/vm_ops.rs @@ -52,10 +52,19 @@ impl CostRunner for VmMemReadRun { _iter: u64, mut sample: Self::SampleType, ) -> Self::RecycledType { - black_box(sample.vm.with_vmcaller(|caller| { - host.metered_vm_read_bytes_from_linear_memory(caller, &sample.vm, 0, &mut sample.buf) - .unwrap() - })); + black_box( + sample + .vm + .with_vmcaller(|caller| { + host.metered_vm_read_bytes_from_linear_memory( + caller, + &sample.vm, + 0, + &mut sample.buf, + ) + }) + .unwrap(), + ); sample } @@ -82,10 +91,19 @@ impl CostRunner for VmMemWriteRun { _iter: u64, mut sample: Self::SampleType, ) -> Self::RecycledType { - black_box(sample.vm.with_vmcaller(|caller| { - host.metered_vm_write_bytes_to_linear_memory(caller, &sample.vm, 0, &mut sample.buf) - .unwrap() - })); + black_box( + sample + .vm + .with_vmcaller(|caller| { + host.metered_vm_write_bytes_to_linear_memory( + caller, + &sample.vm, + 0, + &mut sample.buf, + ) + }) + .unwrap(), + ); sample } diff --git a/soroban-env-host/src/cost_runner/runner.rs b/soroban-env-host/src/cost_runner/runner.rs index 6acf20749..910d0b597 100644 --- a/soroban-env-host/src/cost_runner/runner.rs +++ b/soroban-env-host/src/cost_runner/runner.rs @@ -66,6 +66,6 @@ pub trait CostRunner: Sized { /// actual input from the host's perspective. So use it carefully. This should be /// after the `run`, outside of the CPU-and-memory tracking machineary. fn get_tracker(host: &Host) -> (u64, Option) { - host.0.budget.get_tracker(Self::COST_TYPE) + host.0.budget.get_tracker(Self::COST_TYPE).unwrap() } } diff --git a/soroban-env-host/src/events/diagnostic.rs b/soroban-env-host/src/events/diagnostic.rs index 1dbe79e2a..032a839a4 100644 --- a/soroban-env-host/src/events/diagnostic.rs +++ b/soroban-env-host/src/events/diagnostic.rs @@ -21,17 +21,21 @@ pub enum DiagnosticLevel { /// None of these functions are metered, which is why they're behind the is_debug check impl Host { - pub fn set_diagnostic_level(&self, diagnostic_level: DiagnosticLevel) { - *self.0.diagnostic_level.borrow_mut() = diagnostic_level; + pub fn set_diagnostic_level(&self, diagnostic_level: DiagnosticLevel) -> Result<(), HostError> { + *self.try_borrow_diagnostic_level_mut()? = diagnostic_level; + Ok(()) } // As above, avoids having to import DiagnosticLevel. - pub fn enable_debug(&self) { + pub fn enable_debug(&self) -> Result<(), HostError> { self.set_diagnostic_level(DiagnosticLevel::Debug) } - pub fn is_debug(&self) -> bool { - matches!(*self.0.diagnostic_level.borrow(), DiagnosticLevel::Debug) + pub fn is_debug(&self) -> Result { + Ok(matches!( + *self.try_borrow_diagnostic_level()?, + DiagnosticLevel::Debug + )) } /// Records a `System` contract event. `topics` is expected to be a `SCVec` @@ -70,7 +74,7 @@ impl Host { } pub fn log_diagnostics(&self, msg: &str, args: &[Val]) -> Result<(), HostError> { - if !self.is_debug() { + if !self.is_debug()? { return Ok(()); } let calling_contract = self.get_current_contract_id_unmetered()?; @@ -92,7 +96,7 @@ impl Host { msg: &str, args: &[Val], ) -> Result<(), HostError> { - if !self.is_debug() { + if !self.is_debug()? { return Ok(()); } @@ -130,7 +134,7 @@ impl Host { func: &Symbol, args: &[Val], ) -> Result<(), HostError> { - if !self.is_debug() { + if !self.is_debug()? { return Ok(()); } @@ -162,7 +166,7 @@ impl Host { func: &Symbol, res: &Val, ) -> Result<(), HostError> { - if !self.is_debug() { + if !self.is_debug()? { return Ok(()); } diff --git a/soroban-env-host/src/events/mod.rs b/soroban-env-host/src/events/mod.rs index 600c2e354..2e759819f 100644 --- a/soroban-env-host/src/events/mod.rs +++ b/soroban-env-host/src/events/mod.rs @@ -145,11 +145,11 @@ impl Host { where F: FnOnce(&mut InternalEventsBuffer) -> Result, { - f(&mut self.0.events.borrow_mut()) + f(&mut *self.try_borrow_events_mut()?) } pub fn get_events(&self) -> Result { - self.0.events.borrow().externalize(self) + self.try_borrow_events()?.externalize(self) } // Records a contract event. diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs index 0bbc69c5d..cf7061854 100644 --- a/soroban-env-host/src/host.rs +++ b/soroban-env-host/src/host.rs @@ -125,6 +125,94 @@ pub(crate) struct HostImpl { #[derive(Default, Clone)] pub struct Host(pub(crate) Rc); +macro_rules! impl_checked_borrow_helpers { + ($field:ident, $t:ty, $borrow:ident, $borrow_mut:ident) => { + impl Host { + pub(crate) fn $borrow(&self) -> Result, HostError> { + use crate::host::error::TryBorrowOrErr; + self.0.$field.try_borrow_or_err_with( + self, + concat!("host.0.", stringify!($field), ".try_borrow failed"), + ) + } + pub(crate) fn $borrow_mut(&self) -> Result, HostError> { + use crate::host::error::TryBorrowOrErr; + self.0.$field.try_borrow_mut_or_err_with( + self, + concat!("host.0.", stringify!($field), ".try_borrow_mut failed"), + ) + } + } + }; +} + +impl_checked_borrow_helpers!( + source_account, + Option, + try_borrow_source_account, + try_borrow_source_account_mut +); +impl_checked_borrow_helpers!( + ledger, + Option, + try_borrow_ledger, + try_borrow_ledger_mut +); +impl_checked_borrow_helpers!( + objects, + Vec, + try_borrow_objects, + try_borrow_objects_mut +); +impl_checked_borrow_helpers!(storage, Storage, try_borrow_storage, try_borrow_storage_mut); +impl_checked_borrow_helpers!( + context, + Vec, + try_borrow_context, + try_borrow_context_mut +); +impl_checked_borrow_helpers!( + events, + InternalEventsBuffer, + try_borrow_events, + try_borrow_events_mut +); +impl_checked_borrow_helpers!( + authorization_manager, + AuthorizationManager, + try_borrow_authorization_manager, + try_borrow_authorization_manager_mut +); +impl_checked_borrow_helpers!( + diagnostic_level, + DiagnosticLevel, + try_borrow_diagnostic_level, + try_borrow_diagnostic_level_mut +); +impl_checked_borrow_helpers!( + base_prng, + Option, + try_borrow_base_prng, + try_borrow_base_prng_mut +); +impl_checked_borrow_helpers!( + expiration_bumps, + ExpirationLedgerBumps, + try_borrow_expiration_bumps, + try_borrow_expiration_bumps_mut +); + +#[cfg(any(test, feature = "testutils"))] +impl_checked_borrow_helpers!(contracts, std::collections::HashMap>, try_borrow_contracts, try_borrow_contracts_mut); + +#[cfg(any(test, feature = "testutils"))] +impl_checked_borrow_helpers!( + previous_authorization_manager, + Option, + try_borrow_previous_authorization_manager, + try_borrow_previous_authorization_manager_mut +); + impl Debug for HostImpl { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "HostImpl(...)") @@ -163,22 +251,24 @@ impl Host { })) } - pub fn set_source_account(&self, source_account: AccountId) { - *self.0.source_account.borrow_mut() = Some(source_account); + pub fn set_source_account(&self, source_account: AccountId) -> Result<(), HostError> { + *self.try_borrow_source_account_mut()? = Some(source_account); + Ok(()) } #[cfg(any(test, feature = "testutils"))] - pub fn remove_source_account(&self) { - *self.0.source_account.borrow_mut() = None; + pub fn remove_source_account(&self) -> Result<(), HostError> { + *self.try_borrow_source_account_mut()? = None; + Ok(()) } #[cfg(test)] - pub(crate) fn source_account_id(&self) -> Option { - self.0.source_account.borrow().clone() + pub(crate) fn source_account_id(&self) -> Result, HostError> { + Ok(self.try_borrow_source_account()?.clone()) } pub fn source_account_address(&self) -> Result, HostError> { - if let Some(acc) = self.0.source_account.borrow().as_ref() { + if let Some(acc) = self.try_borrow_source_account()?.as_ref() { Ok(Some(self.add_host_object(ScAddress::Account( acc.metered_clone(self.budget_ref())?, ))?)) @@ -187,8 +277,9 @@ impl Host { } } - pub fn switch_to_recording_auth(&self) { - *self.0.authorization_manager.borrow_mut() = AuthorizationManager::new_recording(); + pub fn switch_to_recording_auth(&self) -> Result<(), HostError> { + *self.try_borrow_authorization_manager_mut()? = AuthorizationManager::new_recording(); + Ok(()) } pub fn set_authorization_entries( @@ -196,23 +287,25 @@ impl Host { auth_entries: Vec, ) -> Result<(), HostError> { let new_auth_manager = AuthorizationManager::new_enforcing(self, auth_entries)?; - *self.0.authorization_manager.borrow_mut() = new_auth_manager; + *self.try_borrow_authorization_manager_mut()? = new_auth_manager; Ok(()) } - pub fn set_base_prng_seed(&self, seed: prng::Seed) { - *self.0.base_prng.borrow_mut() = Some(Prng::new_from_seed(seed)) + pub fn set_base_prng_seed(&self, seed: prng::Seed) -> Result<(), HostError> { + *self.try_borrow_base_prng_mut()? = Some(Prng::new_from_seed(seed)); + Ok(()) } - pub fn set_ledger_info(&self, info: LedgerInfo) { - *self.0.ledger.borrow_mut() = Some(info) + pub fn set_ledger_info(&self, info: LedgerInfo) -> Result<(), HostError> { + *self.try_borrow_ledger_mut()? = Some(info); + Ok(()) } pub fn with_ledger_info(&self, f: F) -> Result where F: FnOnce(&LedgerInfo) -> Result, { - match self.0.ledger.borrow().as_ref() { + match self.try_borrow_ledger()?.as_ref() { None => Err(self.err( ScErrorType::Context, ScErrorCode::InternalError, @@ -227,7 +320,7 @@ impl Host { where F: FnMut(&mut LedgerInfo), { - match self.0.ledger.borrow_mut().as_mut() { + match self.try_borrow_ledger_mut()?.as_mut() { None => Err(self.err( ScErrorType::Context, ScErrorCode::InternalError, @@ -267,7 +360,7 @@ impl Host { where F: FnOnce(&mut Storage) -> Result, { - f(&mut self.0.storage.borrow_mut()) + f(&mut *self.try_borrow_storage_mut()?) } /// Immutable accessor to the instance storage of the currently running @@ -324,9 +417,8 @@ impl Host { self, ) -> Result<(Storage, Budget, Events, ExpirationLedgerBumps), (Self, HostError)> { let events = self - .0 - .events - .borrow() + .try_borrow_events() + .map_err(|e| (self.clone(), e))? .externalize(&self) .map_err(|e| (self.clone(), e))?; @@ -389,8 +481,8 @@ impl Host { /// Use this in conjunction with `set_auth_manager` to do authorized /// operations without breaking the current authorization state (useful for /// preserving the auth state while doing the generic test setup). - pub fn snapshot_auth_manager(&self) -> AuthorizationManager { - self.0.authorization_manager.borrow().clone() + pub fn snapshot_auth_manager(&self) -> Result { + Ok(self.try_borrow_authorization_manager()?.clone()) } /// Replaces authorization manager with the provided new instance. @@ -399,8 +491,9 @@ impl Host { /// operations without breaking the current authorization state (useful for /// preserving the auth state while doing the generic test setup). #[cfg(any(test, feature = "testutils"))] - pub fn set_auth_manager(&self, auth_manager: AuthorizationManager) { - *self.0.authorization_manager.borrow_mut() = auth_manager; + pub fn set_auth_manager(&self, auth_manager: AuthorizationManager) -> Result<(), HostError> { + *self.try_borrow_authorization_manager_mut()? = auth_manager; + Ok(()) } // Testing interface to create values directly for later use via Env functions. @@ -417,9 +510,7 @@ impl Host { ) -> Result<(), HostError> { let storage_key = self.contract_instance_ledger_key(&contract_id)?; if self - .0 - .storage - .borrow_mut() + .try_borrow_storage_mut()? .has(&storage_key, self.as_budget())? { return Err(self.err( @@ -482,9 +573,7 @@ impl Host { ) -> Result { let has_deployer = deployer.is_some(); if has_deployer { - self.0 - .authorization_manager - .borrow_mut() + self.try_borrow_authorization_manager_mut()? .push_create_contract_host_fn_frame(args.metered_clone(self.budget_ref())?); } // Make sure that even in case of operation failure we still pop the @@ -495,7 +584,7 @@ impl Host { // for them just to make auth work in a single case). let res = self.create_contract_with_optional_auth(deployer, args); if has_deployer { - self.0.authorization_manager.borrow_mut().pop_frame(); + self.try_borrow_authorization_manager_mut()?.pop_frame(); } res } @@ -506,7 +595,7 @@ impl Host { args: CreateContractArgs, ) -> Result { if let Some(deployer_address) = deployer { - self.0.authorization_manager.borrow_mut().require_auth( + self.try_borrow_authorization_manager_mut()?.require_auth( self, deployer_address, Default::default(), @@ -554,7 +643,7 @@ impl Host { contract_fns: Rc, ) -> Result<(), HostError> { let hash = self.contract_id_from_address(contract_address)?; - let mut contracts = self.0.contracts.borrow_mut(); + let mut contracts = self.try_borrow_contracts_mut()?; contracts.insert(hash, contract_fns); Ok(()) } @@ -582,9 +671,7 @@ impl Host { &self, ) -> Result, HostError> { Ok(self - .0 - .previous_authorization_manager - .borrow_mut() + .try_borrow_previous_authorization_manager_mut()? .as_mut() .ok_or_else(|| { self.err( @@ -615,9 +702,7 @@ impl Host { body_type: ContractEntryBodyType::DataEntry, })); if !self - .0 - .storage - .borrow_mut() + .try_borrow_storage_mut()? .has(&code_key, self.as_budget())? { let body = ContractCodeEntryBody::DataEntry(wasm.try_into().map_err(|_| { @@ -654,16 +739,12 @@ impl Host { pub fn get_recorded_auth_payloads(&self) -> Result, HostError> { #[cfg(not(any(test, feature = "testutils")))] { - self.0 - .authorization_manager - .borrow() + self.try_borrow_authorization_manager()? .get_recorded_auth_payloads(self) } #[cfg(any(test, feature = "testutils"))] { - self.0 - .previous_authorization_manager - .borrow() + self.try_borrow_previous_authorization_manager()? .as_ref() .ok_or_else(|| { self.err( @@ -731,8 +812,8 @@ impl Host { let durability: ContractDataDurability = t.try_into()?; let key = self.contract_data_key_from_rawval(k, durability)?; - if self.0.storage.borrow_mut().has(&key, self.as_budget())? { - let mut current = (*self.0.storage.borrow_mut().get(&key, self.as_budget())?) + if self.try_borrow_storage_mut()?.has(&key, self.as_budget())? { + let mut current = (*self.try_borrow_storage_mut()?.get(&key, self.as_budget())?) .metered_clone(&self.0.budget)?; match current.data { @@ -761,9 +842,7 @@ impl Host { )); } } - self.0 - .storage - .borrow_mut() + self.try_borrow_storage_mut()? .put(&key, &Rc::new(current), self.as_budget())?; } else { let body = ContractDataEntryBody::DataEntry(ContractDataEntryData { @@ -777,7 +856,7 @@ impl Host { expiration_ledger_seq: self.get_min_expiration_ledger(durability)?, durability, }); - self.0.storage.borrow_mut().put( + self.try_borrow_storage_mut()?.put( &key, &Host::ledger_entry_from_data(data), self.as_budget(), @@ -837,7 +916,9 @@ impl EnvBase for Host { fn escalate_error_to_panic(&self, e: Self::Error) -> ! { let _ = self.with_current_frame_opt(|f| { if let Some(Frame::TestContract(frame)) = f { - *frame.panic.borrow_mut() = Some(e.error); + if let Ok(mut panic) = frame.panic.try_borrow_mut() { + *panic = Some(e.error); + } } Ok(()) }); @@ -1025,7 +1106,7 @@ impl VmCallerEnv for Host { vals_pos: U32Val, vals_len: U32Val, ) -> Result { - if self.is_debug() { + if self.is_debug()? { self.as_budget().with_free_budget(|| { let VmSlice { vm, pos, len } = self.decode_vmslice(msg_pos, msg_len)?; let mut msg: Vec = vec![0u8; len as usize]; @@ -1915,7 +1996,7 @@ impl VmCallerEnv for Host { let res = match t { StorageType::Temporary | StorageType::Persistent => { let key = self.storage_key_from_rawval(k, t.try_into()?)?; - self.0.storage.borrow_mut().has(&key, self.as_budget())? + self.try_borrow_storage_mut()?.has(&key, self.as_budget())? } StorageType::Instance => { self.with_instance_storage(|s| Ok(s.map.get(&k, self)?.is_some()))? @@ -1935,7 +2016,7 @@ impl VmCallerEnv for Host { match t { StorageType::Temporary | StorageType::Persistent => { let key = self.storage_key_from_rawval(k, t.try_into()?)?; - let entry = self.0.storage.borrow_mut().get(&key, self.as_budget())?; + let entry = self.try_borrow_storage_mut()?.get(&key, self.as_budget())?; match &entry.data { LedgerEntryData::ContractData(ContractDataEntry { body, .. }) => match body { ContractDataEntryBody::DataEntry(data) => Ok(self.to_host_val(&data.val)?), @@ -1980,7 +2061,7 @@ impl VmCallerEnv for Host { match t { StorageType::Temporary | StorageType::Persistent => { let key = self.contract_data_key_from_rawval(k, t.try_into()?)?; - self.0.storage.borrow_mut().del(&key, self.as_budget())?; + self.try_borrow_storage_mut()?.del(&key, self.as_budget())?; } StorageType::Instance => { self.with_mut_instance_storage(|s| { @@ -2026,9 +2107,7 @@ impl VmCallerEnv for Host { // The host doesn't load the key, but we want to make sure that // it's in the footprint, because the system embedding the host // (e.g. stellar-core) will to perform the bump. - self.0 - .storage - .borrow_mut() + self.try_borrow_storage_mut()? .touch_key(&key, self.as_budget())?; let min_expiration = self.with_ledger_info(|li| { @@ -2037,7 +2116,7 @@ impl VmCallerEnv for Host { .saturating_sub(1) .saturating_add(min.into())) })?; - self.0.expiration_bumps.borrow_mut().metered_push( + self.try_borrow_expiration_bumps_mut()?.metered_push( self, LedgerBump { key, @@ -2061,7 +2140,7 @@ impl VmCallerEnv for Host { let contract_id = self.get_current_contract_id_internal()?; let key = self.contract_instance_ledger_key(&contract_id)?; - self.0.expiration_bumps.borrow_mut().metered_push( + self.try_borrow_expiration_bumps_mut()?.metered_push( self, LedgerBump { key: Rc::clone(&key), @@ -2075,7 +2154,7 @@ impl VmCallerEnv for Host { { ContractExecutable::Wasm(wasm_hash) => { let key = self.contract_code_ledger_key(&wasm_hash)?; - self.0.expiration_bumps.borrow_mut().metered_push( + self.try_borrow_expiration_bumps_mut()?.metered_push( self, LedgerBump { key, @@ -2721,7 +2800,7 @@ impl VmCallerEnv for Host { &self, _vmcaller: &mut VmCaller, ) -> Result { - let contexts = self.0.context.borrow(); + let contexts = self.try_borrow_context()?; let get_host_val_tuple = |id: &Hash, function: &Symbol| -> Result<[Val; 2], HostError> { let addr_val = self @@ -2781,9 +2860,7 @@ impl VmCallerEnv for Host { ) -> Result { let args = self.visit_obj(args, |a: &HostVec| a.to_vec(self.budget_ref()))?; Ok(self - .0 - .authorization_manager - .borrow_mut() + .try_borrow_authorization_manager_mut()? .require_auth(self, address, args)? .into()) } @@ -2812,9 +2889,7 @@ impl VmCallerEnv for Host { })?; Ok(self - .0 - .authorization_manager - .borrow_mut() + .try_borrow_authorization_manager_mut()? .require_auth(self, address, args)? .into()) } @@ -2825,9 +2900,7 @@ impl VmCallerEnv for Host { auth_entries: VecObject, ) -> Result { Ok(self - .0 - .authorization_manager - .borrow_mut() + .try_borrow_authorization_manager_mut()? .add_invoker_contract_auth(self, auth_entries)? .into()) } diff --git a/soroban-env-host/src/host/data_helper.rs b/soroban-env-host/src/host/data_helper.rs index 8a4734a5b..3910ecc7b 100644 --- a/soroban-env-host/src/host/data_helper.rs +++ b/soroban-env-host/src/host/data_helper.rs @@ -39,7 +39,7 @@ impl Host { &self, key: &Rc, ) -> Result { - let entry = self.0.storage.borrow_mut().get(key, self.as_budget())?; + let entry = self.try_borrow_storage_mut()?.get(key, self.as_budget())?; match &entry.data { LedgerEntryData::ContractData(ContractDataEntry { body, .. }) => match body { ContractDataEntryBody::DataEntry(data) => match &data.val { @@ -80,9 +80,7 @@ impl Host { pub(crate) fn retrieve_wasm_from_storage(&self, wasm_hash: &Hash) -> Result { let key = self.contract_code_ledger_key(wasm_hash)?; match &self - .0 - .storage - .borrow_mut() + .try_borrow_storage_mut()? .get(&key, self.as_budget())? .data { @@ -105,7 +103,7 @@ impl Host { pub(crate) fn wasm_exists(&self, wasm_hash: &Hash) -> Result { let key = self.contract_code_ledger_key(wasm_hash)?; - self.0.storage.borrow_mut().has(&key, self.as_budget()) + self.try_borrow_storage_mut()?.has(&key, self.as_budget()) } // Notes on metering: `from_host_obj` and `put` to storage covered, rest are free. @@ -120,8 +118,8 @@ impl Host { flags: 0, }); - if self.0.storage.borrow_mut().has(&key, self.as_budget())? { - let mut current = (*self.0.storage.borrow_mut().get(&key, self.as_budget())?) + if self.try_borrow_storage_mut()?.has(&key, self.as_budget())? { + let mut current = (*self.try_borrow_storage_mut()?.get(&key, self.as_budget())?) .metered_clone(&self.0.budget)?; match current.data { @@ -137,9 +135,7 @@ impl Host { )); } } - self.0 - .storage - .borrow_mut() + self.try_borrow_storage_mut()? .put(&key, &Rc::new(current), self.as_budget())?; } else { let data = LedgerEntryData::ContractData(ContractDataEntry { @@ -153,7 +149,7 @@ impl Host { .saturating_add(li.min_persistent_entry_expiration)) })?, }); - self.0.storage.borrow_mut().put( + self.try_borrow_storage_mut()?.put( key, &Host::ledger_entry_from_data(data), self.as_budget(), diff --git a/soroban-env-host/src/host/error.rs b/soroban-env-host/src/host/error.rs index 556ff797f..f9c4cd790 100644 --- a/soroban-env-host/src/host/error.rs +++ b/soroban-env-host/src/host/error.rs @@ -10,7 +10,10 @@ use soroban_env_common::{ xdr::{ScErrorCode, ScErrorType}, ConversionError, TryFromVal, U32Val, Val, }; -use std::ops::DerefMut; +use std::{ + cell::{Ref, RefCell, RefMut}, + ops::DerefMut, +}; #[derive(Clone)] pub(crate) struct DebugInfo { @@ -137,6 +140,37 @@ impl From for std::io::Error { } } +pub(crate) trait TryBorrowOrErr { + fn try_borrow_or_err(&self) -> Result, Error>; + fn try_borrow_mut_or_err(&self) -> Result, Error>; + fn try_borrow_or_err_with(&self, host: &Host, msg: &str) -> Result, HostError> { + self.try_borrow_or_err() + .map_err(|e| host.error(e, msg, &[])) + } + fn try_borrow_mut_or_err_with( + &self, + host: &Host, + msg: &str, + ) -> Result, HostError> { + self.try_borrow_mut_or_err() + .map_err(|e| host.error(e, msg, &[])) + } +} + +impl TryBorrowOrErr for RefCell { + fn try_borrow_or_err(&self) -> Result, Error> { + self.try_borrow().map_err(|_| { + Error::from_type_and_code(ScErrorType::Context, ScErrorCode::InternalError) + }) + } + + fn try_borrow_mut_or_err(&self) -> Result, Error> { + self.try_borrow_mut().map_err(|_| { + Error::from_type_and_code(ScErrorType::Context, ScErrorCode::InternalError) + }) + } +} + impl Host { /// Convenience function that only evaluates the auxiliary debug arguments /// to [Host::error] when [Host::is_debug] is `true`. @@ -146,7 +180,7 @@ impl Host { msg: &str, debug_args: impl FnOnce() -> &'a [Val] + 'a, ) -> HostError { - if self.is_debug() { + if let Ok(true) = self.is_debug() { self.error(error, msg, debug_args()) } else { self.error(error, msg, &[]) @@ -165,7 +199,7 @@ impl Host { /// enriches the returned [Error] with [DebugInfo] in the form of a /// [Backtrace] and snapshot of the [Events] buffer. pub fn error(&self, error: Error, msg: &str, args: &[Val]) -> HostError { - if self.is_debug() { + if let Ok(true) = self.is_debug() { // We _try_ to take a mutable borrow of the events buffer refcell // while building up the event we're going to emit into the events // log, failing gracefully (just emitting a no-debug-info @@ -244,7 +278,7 @@ impl Host { E: Debug, { res.map_err(|e| { - if self.is_debug() { + if let Ok(true) = self.is_debug() { let msg = format!("{:?}", e); self.error(e.into(), &msg, &[]) } else { @@ -315,10 +349,10 @@ impl DebugArg for usize { macro_rules! err { ($host:expr, $error:expr, $msg:literal, $($args:expr),*) => { { - if $host.is_debug() { - $host.error($error.into(), $msg, &[]) - } else { + if let Ok(true) = $host.is_debug() { $host.error($error.into(), $msg, &[$(<_ as $crate::host::error::DebugArg>::debug_arg($host, &$args)),*]) + } else { + $host.error($error.into(), $msg, &[]) } } }; diff --git a/soroban-env-host/src/host/frame.rs b/soroban-env-host/src/host/frame.rs index 238803e2f..bb23b6e31 100644 --- a/soroban-env-host/src/host/frame.rs +++ b/soroban-env-host/src/host/frame.rs @@ -131,10 +131,10 @@ impl Host { prng: None, storage: None, }; - self.0.context.borrow_mut().push(ctx); + self.try_borrow_context_mut()?.push(ctx); Ok(RollbackPoint { - storage: self.0.storage.borrow().map.clone(), - events: self.0.events.borrow().vec.len(), + storage: self.try_borrow_storage()?.map.clone(), + events: self.try_borrow_events()?.vec.len(), auth: auth_snapshot, }) } @@ -149,9 +149,7 @@ impl Host { if orp.is_none() { self.flush_instance_storage()?; } - self.0 - .context - .borrow_mut() + self.try_borrow_context_mut()? .pop() .expect("unmatched host frame push/pop"); // This is a bit hacky, as it relies on re-borrow to occur only doing @@ -162,12 +160,10 @@ impl Host { auth_manager.pop_frame(); } - if self.0.context.borrow().is_empty() { + if self.try_borrow_context()?.is_empty() { // When there are no frames left, emulate authentication for the // recording auth mode. This is a no-op for the enforcing mode. - self.0 - .authorization_manager - .borrow_mut() + self.try_borrow_authorization_manager_mut()? .maybe_emulate_authentication(self)?; // Empty call stack in tests means that some contract function call // has been finished and hence the authorization manager can be reset. @@ -176,17 +172,18 @@ impl Host { // shared between the contract invocations. #[cfg(any(test, feature = "testutils"))] { - *self.0.previous_authorization_manager.borrow_mut() = - Some(self.0.authorization_manager.borrow().clone()); - self.0.authorization_manager.borrow_mut().reset(); + *self.try_borrow_previous_authorization_manager_mut()? = + Some(self.try_borrow_authorization_manager()?.clone()); + self.try_borrow_authorization_manager_mut()?.reset(); } } if let Some(rp) = orp { - self.0.storage.borrow_mut().map = rp.storage; - self.0.events.borrow_mut().rollback(rp.events)?; + self.try_borrow_storage_mut()?.map = rp.storage; + self.try_borrow_events_mut()?.rollback(rp.events)?; if let Some(auth_rp) = rp.auth { - self.0.authorization_manager.borrow_mut().rollback(auth_rp); + self.try_borrow_authorization_manager_mut()? + .rollback(auth_rp); } } Ok(()) @@ -276,7 +273,7 @@ impl Host { if let Some(p) = &mut curr_prng_opt { res = f(p) } else { - let mut base_guard = self.0.base_prng.borrow_mut(); + let mut base_guard = self.try_borrow_base_prng_mut()?; if let Some(base) = base_guard.as_mut() { match base.sub_prng(self.as_budget()) { Ok(mut sub_prng) => { @@ -313,7 +310,7 @@ impl Host { F: FnOnce() -> Result, { self.charge_budget(ContractCostType::GuardFrame, None)?; - let start_depth = self.0.context.borrow().len(); + let start_depth = self.try_borrow_context()?.len(); let rp = self.push_frame(frame)?; let res = f(); let res = if let Ok(v) = res { @@ -333,7 +330,7 @@ impl Host { self.pop_frame(None)?; } // Every push and pop should be matched; if not there is a bug. - let end_depth = self.0.context.borrow().len(); + let end_depth = self.try_borrow_context()?.len(); assert_eq!(start_depth, end_depth); res } @@ -368,7 +365,7 @@ impl Host { } pub(crate) fn get_invoking_contract_internal(&self) -> Result { - let frames = self.0.context.borrow(); + let frames = self.try_borrow_context()?; // the previous frame must exist and must be a contract let hash = match frames.as_slice() { [.., c2, _] => match &c2.frame { @@ -395,7 +392,7 @@ impl Host { // Metering: mostly free or already covered by components (e.g. err_general) pub(super) fn get_invoker_type(&self) -> Result { - let frames = self.0.context.borrow(); + let frames = self.try_borrow_context()?; // If the previous frame exists and is a contract, return its ID, otherwise return // the account invoking. let st = match frames.as_slice() { @@ -494,7 +491,7 @@ impl Host { } if !matches!(reentry_mode, ContractReentryMode::Allowed) { let mut is_last_non_host_frame = true; - for ctx in self.0.context.borrow().iter().rev() { + for ctx in self.try_borrow_context()?.iter().rev() { let exist_id = match &ctx.frame { Frame::ContractVM(vm, ..) => &vm.contract_id, Frame::Token(id, ..) => id, @@ -528,10 +525,10 @@ impl Host { // This looks a little un-idiomatic, but this avoids maintaining a borrow of // self.0.contracts. Implementing it as // - // if let Some(cfs) = self.0.contracts.borrow().get(&id).cloned() { ... } + // if let Some(cfs) = self.try_borrow_contracts()?.get(&id).cloned() { ... } // // maintains a borrow of self.0.contracts, which can cause borrow errors. - let cfs_option = self.0.contracts.borrow().get(&id).cloned(); + let cfs_option = self.try_borrow_contracts()?.get(&id).cloned(); if let Some(cfs) = cfs_option { let instance_key = self.contract_instance_ledger_key(&id)?; let storage = self @@ -589,13 +586,15 @@ impl Host { ScErrorCode::InternalError, ); - if let Some(err) = *panic.borrow() { - error = err; + if let Ok(panic) = panic.try_borrow() { + if let Some(err) = *panic { + error = err; + } } // If we're allowed to record dynamic strings (which happens // when diagnostics are active), also log the panic payload into // the Debug buffer. - else if self.is_debug() { + else if self.is_debug()? { if let Some(str) = panic_payload.downcast_ref::<&str>() { let msg: String = format!( "caught panic '{}' from contract function '{:?}'", diff --git a/soroban-env-host/src/host_object.rs b/soroban-env-host/src/host_object.rs index 60c5bbde7..0c921f45c 100644 --- a/soroban-env-host/src/host_object.rs +++ b/soroban-env-host/src/host_object.rs @@ -177,14 +177,14 @@ impl Host { &self, hot: HOT, ) -> Result { - let prev_len = self.0.objects.borrow().len(); + let prev_len = self.try_borrow_objects()?.len(); if prev_len > u32::MAX as usize { return Err(self.err_arith_overflow()); } // charge for the new host object, which is just the amortized cost of a single // `HostObject` allocation metered_clone::charge_heap_alloc::(1, self.as_budget())?; - self.0.objects.borrow_mut().push(HOT::inject(hot)); + self.try_borrow_objects_mut()?.push(HOT::inject(hot)); let handle = prev_len as u32; Ok(HOT::new_from_handle(handle)) } @@ -200,7 +200,7 @@ impl Host { F: FnOnce(Option<&HostObject>) -> Result, { self.charge_budget(ContractCostType::VisitObject, None)?; - let r = self.0.objects.borrow(); + let r = self.try_borrow_objects()?; let obj: Object = obj.into(); let handle: u32 = obj.get_handle(); f(r.get(handle as usize)) diff --git a/soroban-env-host/src/storage.rs b/soroban-env-host/src/storage.rs index 1cf257567..2db037ed4 100644 --- a/soroban-env-host/src/storage.rs +++ b/soroban-env-host/src/storage.rs @@ -329,7 +329,7 @@ mod test_footprint { #[test] fn footprint_record_access() -> Result<(), HostError> { let budget = Budget::default(); - budget.reset_unlimited(); + budget.reset_unlimited()?; let mut fp = Footprint::default(); // record when key not exist let key = Rc::new(LedgerKey::ContractData(LedgerKeyContractData { diff --git a/soroban-env-host/src/test/auth.rs b/soroban-env-host/src/test/auth.rs index 7e71c4c7f..a6bf76c0b 100644 --- a/soroban-env-host/src/test/auth.rs +++ b/soroban-env-host/src/test/auth.rs @@ -89,8 +89,8 @@ impl AuthTest { let host = Host::test_host_with_recording_footprint(); // TODO: remove the `reset_unlimited` and instead reset inputs wherever appropriate // to respect the budget limit. - host.as_budget().reset_unlimited(); - host.enable_debug(); + host.as_budget().reset_unlimited().unwrap(); + host.enable_debug().unwrap(); host.with_mut_ledger_info(|li| { li.sequence_number = 100; @@ -251,7 +251,7 @@ impl AuthTest { fn_name: Symbol, args: HostVec, ) -> Vec { - self.host.switch_to_recording_auth(); + self.host.switch_to_recording_auth().unwrap(); self.host .call(contract_address.clone().into(), fn_name, args.into()) .unwrap(); diff --git a/soroban-env-host/src/test/budget_metering.rs b/soroban-env-host/src/test/budget_metering.rs index ba3a349dd..280cb3260 100644 --- a/soroban-env-host/src/test/budget_metering.rs +++ b/soroban-env-host/src/test/budget_metering.rs @@ -34,9 +34,9 @@ fn xdr_object_conversion() -> Result<(), HostError> { // a "value" on separate paths that both need metering, // we wind up double-counting the conversion of "objects". // Possibly this should be improved in the future. - assert_eq!(budget.get_tracker(ContractCostType::ValXdrConv).0, 6); - assert_eq!(budget.get_cpu_insns_consumed(), 60); - assert_eq!(budget.get_mem_bytes_consumed(), 6); + assert_eq!(budget.get_tracker(ContractCostType::ValXdrConv)?.0, 6); + assert_eq!(budget.get_cpu_insns_consumed()?, 60); + assert_eq!(budget.get_mem_bytes_consumed()?, 6); Ok(()) })?; Ok(()) @@ -59,13 +59,13 @@ fn vm_hostfn_invocation() -> Result<(), HostError> { // try_call host.try_call(id_obj, sym, args)?; host.with_budget(|budget| { - assert_eq!(budget.get_tracker(ContractCostType::InvokeVmFunction).0, 1); + assert_eq!(budget.get_tracker(ContractCostType::InvokeVmFunction)?.0, 1); assert_eq!( - budget.get_tracker(ContractCostType::InvokeHostFunction).0, + budget.get_tracker(ContractCostType::InvokeHostFunction)?.0, 2 ); - assert_eq!(budget.get_cpu_insns_consumed(), 30); - assert_eq!(budget.get_mem_bytes_consumed(), 3); + assert_eq!(budget.get_cpu_insns_consumed()?, 30); + assert_eq!(budget.get_mem_bytes_consumed()?, 3); Ok(()) })?; @@ -95,7 +95,7 @@ fn metered_xdr() -> Result<(), HostError> { host.metered_write_xdr(&scmap, &mut w)?; host.with_budget(|budget| { assert_eq!( - budget.get_tracker(ContractCostType::ValSer).1, + budget.get_tracker(ContractCostType::ValSer)?.1, Some(w.len() as u64) ); Ok(()) @@ -104,7 +104,7 @@ fn metered_xdr() -> Result<(), HostError> { host.metered_from_xdr::(w.as_slice())?; host.with_budget(|budget| { assert_eq!( - budget.get_tracker(ContractCostType::ValDeser).1, + budget.get_tracker(ContractCostType::ValDeser)?.1, Some(w.len() as u64) ); Ok(()) @@ -157,9 +157,9 @@ fn map_insert_key_vec_obj() -> Result<(), HostError> { host.with_budget(|budget| { // 6 = 1 visit map + 1 visit k1 + (obj_cmp which needs to) 1 visit both k0 and k1 during lookup, // and then 2 more to validate order of resulting map. - assert_eq!(budget.get_tracker(ContractCostType::VisitObject).0, 6); + assert_eq!(budget.get_tracker(ContractCostType::VisitObject)?.0, 6); // upper bound of number of map-accesses, counting both binary-search, point-access and validate-scan. - assert_eq!(budget.get_tracker(ContractCostType::MapEntry).0, 8); + assert_eq!(budget.get_tracker(ContractCostType::MapEntry)?.0, 8); Ok(()) })?; @@ -200,7 +200,7 @@ fn test_recursive_type_clone() -> Result<(), HostError> { //*********************************************************************************************************************************************/ expect!["864"].assert_eq( host.as_budget() - .get_tracker(ContractCostType::HostMemAlloc) + .get_tracker(ContractCostType::HostMemAlloc)? .1 .unwrap() .to_string() @@ -210,7 +210,7 @@ fn test_recursive_type_clone() -> Result<(), HostError> { // memory layout of the top level type (Vec). expect!["888"].assert_eq( host.as_budget() - .get_tracker(ContractCostType::HostMemCpy) + .get_tracker(ContractCostType::HostMemCpy)? .1 .unwrap() .to_string() diff --git a/soroban-env-host/src/test/complex.rs b/soroban-env-host/src/test/complex.rs index 20c633651..909db4ac5 100644 --- a/soroban-env-host/src/test/complex.rs +++ b/soroban-env-host/src/test/complex.rs @@ -24,7 +24,7 @@ fn run_complex() -> Result<(), HostError> { // Run 1: record footprint, emulating "preflight". let foot = { let host = Host::test_host_with_recording_footprint(); - host.set_ledger_info(info.clone()); + host.set_ledger_info(info.clone())?; let contract_id_obj = host.register_test_contract_wasm_from_source_account( COMPLEX, account_id.clone(), @@ -43,7 +43,7 @@ fn run_complex() -> Result<(), HostError> { { let store = Storage::with_enforcing_footprint_and_map(foot, MeteredOrdMap::default()); let host = Host::with_storage_and_budget(store, Budget::default()); - host.set_ledger_info(info); + host.set_ledger_info(info)?; let contract_id_obj = host.register_test_contract_wasm_from_source_account(COMPLEX, account_id, salt); host.call( diff --git a/soroban-env-host/src/test/event.rs b/soroban-env-host/src/test/event.rs index 625bc672e..22c7893a0 100644 --- a/soroban-env-host/src/test/event.rs +++ b/soroban-env-host/src/test/event.rs @@ -95,10 +95,10 @@ fn test_event_rollback() -> Result<(), HostError> { host.call(id, sym, args)?.get_payload(), Val::from_void().to_val().get_payload() ); - host.0.events.borrow_mut().rollback(1)?; + host.try_borrow_events_mut()?.rollback(1)?; // run `UPDATE_EXPECT=true cargo test` to update this. let expected = expect!["[HostEvent { event: ContractEvent { ext: V0, contract_id: Some(Hash(0000000000000000000000000000000000000000000000000000000000000000)), type_: Contract, body: V0(ContractEventV0 { topics: ScVec(VecM([I32(0), I32(1)])), data: U32(0) }) }, failed_call: false }, HostEvent { event: ContractEvent { ext: V0, contract_id: Some(Hash(0000000000000000000000000000000000000000000000000000000000000000)), type_: System, body: V0(ContractEventV0 { topics: ScVec(VecM([I32(0), I32(1)])), data: U32(0) }) }, failed_call: true }]"]; - let actual = format!("{:?}", host.0.events.borrow().externalize(&host)?.0); + let actual = format!("{:?}", host.try_borrow_events()?.externalize(&host)?.0); expected.assert_eq(&actual); Ok(()) } @@ -122,12 +122,12 @@ fn test_internal_contract_events_metering_not_free() -> Result<(), HostError> { let _ = host.with_events_mut(|events| { Ok(events.record(InternalEvent::Contract(ce), host.as_budget())) })?; - assert_eq!(host.as_budget().get_cpu_insns_consumed(), 30); - assert_eq!(host.as_budget().get_mem_bytes_consumed(), 3); + assert_eq!(host.as_budget().get_cpu_insns_consumed()?, 30); + assert_eq!(host.as_budget().get_mem_bytes_consumed()?, 3); - let _ = host.0.events.borrow().externalize(&host)?; - assert_eq!(host.as_budget().get_cpu_insns_consumed(), 80); - assert_eq!(host.as_budget().get_mem_bytes_consumed(), 8); + let _ = host.try_borrow_events()?.externalize(&host)?; + assert_eq!(host.as_budget().get_cpu_insns_consumed()?, 80); + assert_eq!(host.as_budget().get_mem_bytes_consumed()?, 8); Ok(()) } @@ -155,11 +155,11 @@ fn test_internal_diagnostic_event_metering_free() -> Result<(), HostError> { let _ = host.with_events_mut(|events| { Ok(events.record(InternalEvent::Diagnostic(de), host.as_budget())) })?; - assert_eq!(host.as_budget().get_cpu_insns_consumed(), 0); - assert_eq!(host.as_budget().get_mem_bytes_consumed(), 0); + assert_eq!(host.as_budget().get_cpu_insns_consumed()?, 0); + assert_eq!(host.as_budget().get_mem_bytes_consumed()?, 0); - let _ = host.0.events.borrow().externalize(&host)?; - assert_eq!(host.as_budget().get_cpu_insns_consumed(), 0); - assert_eq!(host.as_budget().get_mem_bytes_consumed(), 0); + let _ = host.try_borrow_events()?.externalize(&host)?; + assert_eq!(host.as_budget().get_cpu_insns_consumed()?, 0); + assert_eq!(host.as_budget().get_mem_bytes_consumed()?, 0); Ok(()) } diff --git a/soroban-env-host/src/test/hostile.rs b/soroban-env-host/src/test/hostile.rs index 2ca452eaa..10b759028 100644 --- a/soroban-env-host/src/test/hostile.rs +++ b/soroban-env-host/src/test/hostile.rs @@ -99,9 +99,9 @@ fn hostile_objs_traps() -> Result<(), HostError> { let host = Host::test_host_with_recording_footprint(); let contract_id_obj = host.register_test_contract_wasm(HOSTILE); - host.set_diagnostic_level(crate::DiagnosticLevel::Debug); - host.with_budget(|b| Ok(b.reset_default()))?; - host.with_budget(|b| Ok(b.reset_unlimited_cpu()))?; + host.set_diagnostic_level(crate::DiagnosticLevel::Debug)?; + host.with_budget(|b| b.reset_default())?; + host.with_budget(|b| b.reset_unlimited_cpu())?; // This one should just run out of memory let res = host.call( diff --git a/soroban-env-host/src/test/invocation.rs b/soroban-env-host/src/test/invocation.rs index c69ded3e0..9b9af7e22 100644 --- a/soroban-env-host/src/test/invocation.rs +++ b/soroban-env-host/src/test/invocation.rs @@ -32,7 +32,7 @@ fn invoke_single_contract_function() -> Result<(), HostError> { fn invoke_cross_contract(diagnostics: bool) -> Result<(), HostError> { let host = Host::test_host_with_recording_footprint(); if diagnostics { - host.set_diagnostic_level(crate::DiagnosticLevel::Debug); + host.set_diagnostic_level(crate::DiagnosticLevel::Debug)?; } let id_obj = host.register_test_contract_wasm(ADD_I32); @@ -59,7 +59,7 @@ fn invoke_cross_contract_with_diagnostics() -> Result<(), HostError> { #[test] fn invoke_cross_contract_with_err() -> Result<(), HostError> { let host = Host::test_host_with_recording_footprint(); - host.enable_debug(); + host.enable_debug()?; let id_obj = host.register_test_contract_wasm(VEC); // prepare arguments let sym = Symbol::try_from_small_str("vec_err").unwrap(); @@ -118,7 +118,7 @@ fn invoke_cross_contract_indirect() -> Result<(), HostError> { #[test] fn invoke_cross_contract_indirect_err() -> Result<(), HostError> { let host = Host::test_host_with_recording_footprint(); - host.enable_debug(); + host.enable_debug()?; let id0_obj = host.register_test_contract_wasm(INVOKE_CONTRACT); let sym = Symbol::try_from_small_str("add_with").unwrap(); let args = host.test_vec_obj::(&[i32::MAX, 1])?; diff --git a/soroban-env-host/src/test/ledger.rs b/soroban-env-host/src/test/ledger.rs index 670cd0b27..e98b02bd3 100644 --- a/soroban-env-host/src/test/ledger.rs +++ b/soroban-env-host/src/test/ledger.rs @@ -22,7 +22,7 @@ fn ledger_network_id() -> Result<(), HostError> { min_persistent_entry_expiration: 4096, min_temp_entry_expiration: 16, max_entry_expiration: 6312000, - }); + })?; let obj = host.get_ledger_network_id()?; let np = host.visit_obj(obj, |np: &ScBytes| Ok(np.to_vec()))?; assert_eq!(np, vec![7; 32],); diff --git a/soroban-env-host/src/test/lifecycle.rs b/soroban-env-host/src/test/lifecycle.rs index e9cbfef53..51826ae97 100644 --- a/soroban-env-host/src/test/lifecycle.rs +++ b/soroban-env-host/src/test/lifecycle.rs @@ -75,7 +75,8 @@ fn test_host() -> Host { host.set_ledger_info(LedgerInfo { network_id: generate_bytes_array(), ..Default::default() - }); + }) + .unwrap(); host } @@ -83,7 +84,7 @@ fn test_host() -> Host { fn test_create_contract_from_source_account(host: &Host, wasm: &[u8]) -> Hash { let source_account = generate_account_id(); let salt = generate_bytes_array(); - host.set_source_account(source_account.clone()); + host.set_source_account(source_account.clone()).unwrap(); let contract_id_preimage = ContractIdPreimage::Address(ContractIdPreimageFromAddress { address: ScAddress::Account(source_account.clone()), salt: Uint256(salt.to_vec().try_into().unwrap()), diff --git a/soroban-env-host/src/test/prng.rs b/soroban-env-host/src/test/prng.rs index 8166c6869..f442c1b23 100644 --- a/soroban-env-host/src/test/prng.rs +++ b/soroban-env-host/src/test/prng.rs @@ -60,8 +60,8 @@ impl ContractFunctionSet for PRNGUsingTest { fn prng_test() -> Result<(), HostError> { let host = Host::default(); - host.enable_debug(); - host.set_base_prng_seed([0; 32]); + host.enable_debug()?; + host.set_base_prng_seed([0; 32])?; let dummy_id = [0; 32]; let dummy_address = ScAddress::Contract(Hash(dummy_id.clone())); diff --git a/soroban-env-host/src/test/storage.rs b/soroban-env-host/src/test/storage.rs index df2a5f61a..377cda401 100644 --- a/soroban-env-host/src/test/storage.rs +++ b/soroban-env-host/src/test/storage.rs @@ -206,7 +206,7 @@ fn test_storage_mix() { // This makes sure the keyspaces are not mixed between storage types. let host = Host::test_host_with_recording_footprint(); host.with_budget(|b| { - b.reset_unlimited(); + b.reset_unlimited().unwrap(); Ok(()) }) .unwrap(); diff --git a/soroban-env-host/src/test/token.rs b/soroban-env-host/src/test/token.rs index e9f4593e1..1885d3842 100644 --- a/soroban-env-host/src/test/token.rs +++ b/soroban-env-host/src/test/token.rs @@ -58,7 +58,8 @@ impl TokenTest { min_persistent_entry_expiration: 4096, min_temp_entry_expiration: 16, max_entry_expiration: 10000, - }); + }) + .unwrap(); Self { host, issuer_key: generate_keypair(), @@ -290,7 +291,7 @@ impl TokenTest { T: Into, F: FnOnce() -> Result, { - self.host.set_source_account(account_id); + self.host.set_source_account(account_id)?; self.host.with_frame( Frame::HostFunction(HostFunctionType::InvokeContract), || { @@ -2748,7 +2749,7 @@ fn test_recording_auth_for_token() { let user = TestSigner::account(&test.user_key); test.create_default_account(&user); test.create_default_trustline(&user); - test.host.switch_to_recording_auth(); + test.host.switch_to_recording_auth().unwrap(); let args = host_vec![&test.host, user.address(&test.host), 100_i128]; test.host diff --git a/soroban-env-host/src/test/util.rs b/soroban-env-host/src/test/util.rs index 00591da38..854f1acb5 100644 --- a/soroban-env-host/src/test/util.rs +++ b/soroban-env-host/src/test/util.rs @@ -11,6 +11,7 @@ use soroban_env_common::{ use crate::{ budget::Budget, + host::error::TryBorrowOrErr, storage::{test_storage::MockSnapshotSource, Storage}, xdr, Host, HostError, }; @@ -60,14 +61,14 @@ impl Host { let snapshot_source = Rc::::new(MockSnapshotSource::new()); let storage = Storage::with_recording_footprint(snapshot_source); let host = Host::with_storage_and_budget(storage, Budget::default()); - host.set_ledger_info(Default::default()); + host.set_ledger_info(Default::default()).unwrap(); host } pub(crate) fn test_budget(self, cpu: u64, mem: u64) -> Self { self.with_budget(|budget| { - budget.reset_limits(cpu, mem); // something big but finite that we may exceed - budget.reset_models(); + budget.reset_limits(cpu, mem)?; // something big but finite that we may exceed + budget.reset_models()?; Ok(()) }) .unwrap(); @@ -85,26 +86,26 @@ impl Host { self.with_budget(|budget| { budget .0 - .borrow_mut() + .try_borrow_mut_or_err()? .cpu_insns .get_cost_model_mut(ty) .const_term = const_cpu as i64; budget .0 - .borrow_mut() + .try_borrow_mut_or_err()? .cpu_insns .get_cost_model_mut(ty) .linear_term = lin_cpu as i64; budget .0 - .borrow_mut() + .try_borrow_mut_or_err()? .mem_bytes .get_cost_model_mut(ty) .const_term = const_mem as i64; budget .0 - .borrow_mut() + .try_borrow_mut_or_err()? .mem_bytes .get_cost_model_mut(ty) .linear_term = lin_mem as i64; @@ -174,15 +175,15 @@ impl Host { ) -> AddressObject { // Use source account-based auth in order to avoid using nonces which // won't work well with enforcing ledger footprint. - let prev_source_account = self.source_account_id(); + let prev_source_account = self.source_account_id().unwrap(); // Use recording auth to skip specifying the auth payload. - let prev_auth_manager = self.snapshot_auth_manager(); - self.switch_to_recording_auth(); + let prev_auth_manager = self.snapshot_auth_manager().unwrap(); + self.switch_to_recording_auth().unwrap(); let wasm_hash = self .upload_wasm(self.bytes_new_from_slice(contract_wasm).unwrap()) .unwrap(); - self.set_source_account(account.clone()); + self.set_source_account(account.clone()).unwrap(); let contract_address = self .create_contract( self.add_host_object(ScAddress::Account(account.clone())) @@ -192,9 +193,9 @@ impl Host { ) .unwrap(); if let Some(prev_account) = prev_source_account { - self.set_source_account(prev_account); + self.set_source_account(prev_account).unwrap(); } - self.set_auth_manager(prev_auth_manager); + self.set_auth_manager(prev_auth_manager).unwrap(); contract_address } diff --git a/soroban-env-host/src/vm.rs b/soroban-env-host/src/vm.rs index 5cc40ddcb..a0de31fae 100644 --- a/soroban-env-host/src/vm.rs +++ b/soroban-env-host/src/vm.rs @@ -12,7 +12,7 @@ mod dispatch; mod fuel_refillable; mod func_info; -use crate::{budget::AsBudget, err, xdr::ContractCostType, HostError}; +use crate::{budget::AsBudget, err, host::error::TryBorrowOrErr, xdr::ContractCostType, HostError}; use std::{cell::RefCell, io::Cursor, rc::Rc}; use super::{xdr::Hash, Host, Symbol, Val}; @@ -142,7 +142,7 @@ impl Vm { )?; let mut config = wasmi::Config::default(); - let fuel_costs = host.as_budget().wasmi_fuel_costs(); + let fuel_costs = host.as_budget().wasmi_fuel_costs()?; // Turn off all optional wasm features. config @@ -221,14 +221,18 @@ impl Vm { inputs: &[Value], ) -> Result { let mut wasm_ret: [Value; 1] = [Value::I64(0)]; - self.store.borrow_mut().fill_fuels(host)?; - let res = func.call(&mut *self.store.borrow_mut(), inputs, &mut wasm_ret); + self.store.try_borrow_mut_or_err()?.fill_fuels(host)?; + let res = func.call( + &mut *self.store.try_borrow_mut_or_err()?, + inputs, + &mut wasm_ret, + ); // Due to the way wasmi's fuel metering works (it does `remaining.checked_sub(delta).ok_or(Trap)`), // there may be a small amount of fuel (less than delta -- the fuel cost of that failing // wasmi instruction) remaining when the `OutOfFuel` trap occurs. This is only observable // if the contract traps with `OutOfFuel`, which may appear confusing if they look closely // at the budget amount consumed. So it should be fine. - self.store.borrow_mut().return_fuels(host)?; + self.store.try_borrow_mut_or_err()?.return_fuels(host)?; if let Err(e) = res { // When a call fails with a wasmi::Error::Trap that carries a HostError @@ -254,7 +258,7 @@ impl Vm { )); } e => { - return Err(if host.is_debug() { + return Err(if host.is_debug()? { // With diagnostics on: log as much detail as we can from wasmi. let msg = format!("VM call failed: {:?}", &e); host.error(e.into(), &msg, &[func_sym.to_val()]) @@ -284,7 +288,7 @@ impl Vm { let func_ss: SymbolStr = func_sym.try_into_val(host)?; let ext = match self .instance - .get_export(&*self.store.borrow(), func_ss.as_ref()) + .get_export(&*self.store.try_borrow_or_err()?, func_ss.as_ref()) { None => { return Err(host.err( @@ -331,11 +335,11 @@ impl Vm { /// to this VM's `Store` and `Instance`, and calls the provided function /// back with it. Mainly used for testing. #[cfg(any(test, feature = "testutils"))] - pub fn with_vmcaller(&self, f: F) -> T + pub fn with_vmcaller(&self, f: F) -> Result where - F: FnOnce(&mut VmCaller) -> T, + F: FnOnce(&mut VmCaller) -> Result, { - let store: &mut Store = &mut self.store.borrow_mut(); + let store: &mut Store = &mut *self.store.try_borrow_mut_or_err()?; let mut ctx: StoreContextMut = store.into(); let caller: Caller = Caller::new(&mut ctx, Some(&self.instance)); let mut vmcaller: VmCaller = VmCaller(Some(caller)); diff --git a/soroban-env-host/tests/integration.rs b/soroban-env-host/tests/integration.rs index 8c9227aa6..c7eae35cc 100644 --- a/soroban-env-host/tests/integration.rs +++ b/soroban-env-host/tests/integration.rs @@ -39,7 +39,7 @@ fn map_host_fn() -> Result<(), HostError> { #[test] fn debug_log() { let host = Host::default(); - host.set_diagnostic_level(DiagnosticLevel::Debug); + host.set_diagnostic_level(DiagnosticLevel::Debug).unwrap(); // Call a debug-log helper. host.log_from_slice("can't convert value", &[Val::from_i32(1).to_val()]) .unwrap();