From c0be886d3cbe349bab47deb1455d1121b5d69d5a Mon Sep 17 00:00:00 2001 From: Jay Geng Date: Wed, 12 Jul 2023 22:40:04 -0400 Subject: [PATCH] WIP: Use ResourceLimiter --- Cargo.lock | 10 -- Cargo.toml | 5 +- soroban-env-common/src/error.rs | 4 +- soroban-env-host/src/budget.rs | 145 ++++++++++++------- soroban-env-host/src/host.rs | 1 + soroban-env-host/src/host/metered_map.rs | 10 +- soroban-env-host/src/host/metered_vector.rs | 10 +- soroban-env-host/src/test/budget_metering.rs | 2 +- soroban-env-host/src/vm.rs | 16 +- soroban-env-host/src/vm/dispatch.rs | 4 +- soroban-env-host/src/vm/fuel_refillable.rs | 65 ++++----- 11 files changed, 152 insertions(+), 120 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ae9ab4a95..a17edc38b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -694,12 +694,6 @@ dependencies = [ "syn 1.0.109", ] -[[package]] -name = "intx" -version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f6f38a50a899dc47a6d0ed5508e7f601a2e34c3a85303514b5d137f3c10a0c75" - [[package]] name = "itertools" version = "0.10.5" @@ -1412,9 +1406,7 @@ version = "0.0.17" [[package]] name = "soroban-wasmi" version = "0.30.0-soroban" -source = "git+https://github.com/stellar/wasmi?rev=3dc639fde3bebf0bf364a9fa4ac2f0efb7ee9995#3dc639fde3bebf0bf364a9fa4ac2f0efb7ee9995" dependencies = [ - "intx", "smallvec", "spin", "wasmi_arena", @@ -1840,12 +1832,10 @@ dependencies = [ [[package]] name = "wasmi_arena" version = "0.4.0" -source = "git+https://github.com/stellar/wasmi?rev=3dc639fde3bebf0bf364a9fa4ac2f0efb7ee9995#3dc639fde3bebf0bf364a9fa4ac2f0efb7ee9995" [[package]] name = "wasmi_core" version = "0.12.0" -source = "git+https://github.com/stellar/wasmi?rev=3dc639fde3bebf0bf364a9fa4ac2f0efb7ee9995#3dc639fde3bebf0bf364a9fa4ac2f0efb7ee9995" dependencies = [ "downcast-rs", "libm", diff --git a/Cargo.toml b/Cargo.toml index 6e5025ffa..f5668f8bf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,9 +43,8 @@ rev = "e6ba45c60c16de28c7522586b80ed0150157df73" # [patch."https://github.com/stellar/rs-stellar-xdr"] # stellar-xdr = { path = "../rs-stellar-xdr/" } -# [patch."https://github.com/stellar/wasmi"] -# soroban-wasmi = { path = "../wasmi/crates/wasmi/" } -# soroban-wasmi_core = { path = "../wasmi/crates/core/" } +[patch."https://github.com/stellar/wasmi"] +soroban-wasmi = { path = "../wasmi/crates/wasmi/" } [profile.release] codegen-units = 1 diff --git a/soroban-env-common/src/error.rs b/soroban-env-common/src/error.rs index 02d426344..f89f72b02 100644 --- a/soroban-env-common/src/error.rs +++ b/soroban-env-common/src/error.rs @@ -159,7 +159,9 @@ impl From for Error { wasmi::core::TrapCode::BadSignature => ScErrorCode::UnexpectedType, - wasmi::core::TrapCode::StackOverflow | wasmi::core::TrapCode::OutOfFuel => { + wasmi::core::TrapCode::StackOverflow + | wasmi::core::TrapCode::OutOfFuel + | wasmi::core::TrapCode::GrowthOperationLimited => { return Error::from_type_and_code(ScErrorType::Budget, ScErrorCode::ExceededLimit) } }; diff --git a/soroban-env-host/src/budget.rs b/soroban-env-host/src/budget.rs index 467f43e79..0934f4786 100644 --- a/soroban-env-host/src/budget.rs +++ b/soroban-env-host/src/budget.rs @@ -12,7 +12,10 @@ use crate::{ Host, HostError, }; -use wasmi::FuelCosts; +use wasmi::{ + errors::{MemoryError, TableError}, + FuelCosts, ResourceLimiter, StoreLimits, StoreLimitsBuilder, +}; /// We provide a "cost model" object that evaluates a linear expression: /// @@ -68,7 +71,7 @@ impl HostCostModel for ContractCostParamEntry { } } -#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(Clone)] pub struct BudgetDimension { /// A set of cost models that map input values (eg. event counts, object /// sizes) from some CostType to whatever concrete resource type is being @@ -204,7 +207,7 @@ impl BudgetDimension { /// doesn't derive all the traits we want. These fields (coarsely) define the /// relative costs of different wasm instruction types and are for wasmi internal /// fuel metering use only. Units are in "fuels". -#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(Clone)] pub(crate) struct FuelConfig { /// The base fuel costs for all instructions. pub base: u64, @@ -249,7 +252,7 @@ impl FuelConfig { } } -#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(Clone)] pub(crate) struct BudgetImpl { pub cpu_insns: BudgetDimension, pub mem_bytes: BudgetDimension, @@ -258,6 +261,7 @@ pub(crate) struct BudgetImpl { tracker: Vec<(u64, Option)>, enabled: bool, fuel_config: FuelConfig, + store_limits: StoreLimits, } impl BudgetImpl { @@ -274,6 +278,8 @@ impl BudgetImpl { tracker: vec![(0, None); ContractCostType::variants().len()], enabled: true, fuel_config: Default::default(), + // TODO: make this a const. Do we need to limit the number of memories too? + store_limits: StoreLimitsBuilder::new().memory_size(0x200000).build(), }; b.init_tracker(); @@ -408,7 +414,7 @@ impl Display for BudgetImpl { } } -#[derive(Default, Clone, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Default, Clone)] pub struct Budget(pub(crate) Rc>); impl Debug for Budget { @@ -469,7 +475,13 @@ impl Budget { f(self.0.try_borrow_mut_or_err()?) } - fn charge_in_bulk( + /// Performs a bulk charge to the budget under the specified [`CostType`]. + /// The `iterations` is the batch size. The caller needs to ensure: + /// 1. the batched charges have identical costs (having the same + /// [`CostType`] and `input`) + /// 2. The input passed in (Some/None) is consistent with the [`CostModel`] + /// underneath the [`CostType`] (linear/constant). + pub fn bulk_charge( &self, ty: ContractCostType, iterations: u64, @@ -520,27 +532,7 @@ impl Budget { /// Otherwise it is a linear model. The caller needs to ensure the input /// passed is consistent with the inherent model underneath. pub fn charge(&self, ty: ContractCostType, input: Option) -> Result<(), HostError> { - self.charge_in_bulk(ty, 1, input) - } - - pub fn apply_wasmi_fuels(&self, cpu_fuel: u64, mem_fuel: u64) -> Result<(), HostError> { - self.charge_in_bulk(ContractCostType::WasmInsnExec, cpu_fuel, None)?; - self.charge_in_bulk(ContractCostType::WasmMemAlloc, mem_fuel, None) - } - - /// Performs a bulk charge to the budget under the specified [`CostType`]. - /// The `iterations` is the batch size. The caller needs to ensure: - /// 1. the batched charges have identical costs (having the same - /// [`CostType`] and `input`) - /// 2. The input passed in (Some/None) is consistent with the [`CostModel`] - /// underneath the [`CostType`] (linear/constant). - pub fn batched_charge( - &self, - ty: ContractCostType, - iterations: u64, - input: Option, - ) -> Result<(), HostError> { - self.charge_in_bulk(ty, iterations, input) + self.bulk_charge(ty, 1, input) } pub fn with_free_budget(&self, f: F) -> Result @@ -649,7 +641,7 @@ impl Budget { Ok(()) } - fn get_cpu_insns_remaining_as_fuel(&self) -> Result { + pub(crate) fn get_cpu_insns_remaining_as_fuel(&self) -> Result { let cpu_remaining = self.get_cpu_insns_remaining()?; let cpu_per_fuel = self .0 @@ -675,29 +667,6 @@ impl Budget { Ok(cpu_remaining / cpu_per_fuel) } - fn get_mem_bytes_remaining_as_fuel(&self) -> Result { - let bytes_remaining = self.get_mem_bytes_remaining()?; - let bytes_per_fuel = self - .0 - .try_borrow_or_err()? - .mem_bytes - .get_cost_model(ContractCostType::WasmMemAlloc) - .linear_term; - - if bytes_per_fuel < 0 { - return Err((ScErrorType::Context, ScErrorCode::InvalidInput).into()); - } - let bytes_per_fuel = (bytes_per_fuel as u64).max(1); - // See comment about rounding above. - Ok(bytes_remaining / bytes_per_fuel) - } - - pub fn get_fuels_budget(&self) -> Result<(u64, u64), HostError> { - let cpu_fuel = self.get_cpu_insns_remaining_as_fuel()?; - let mem_fuel = self.get_mem_bytes_remaining_as_fuel()?; - Ok((cpu_fuel, mem_fuel)) - } - // generate a wasmi fuel cost schedule based on our calibration pub fn wasmi_fuel_costs(&self) -> Result { let config = &self.0.try_borrow_or_err()?.fuel_config; @@ -709,6 +678,26 @@ impl Budget { costs.call = config.call; Ok(costs) } + + pub fn wasmi_store_limits(&self) -> Result { + Ok(self.0.try_borrow_or_err()?.store_limits.clone()) + } + + pub fn get_store_limits_or_return_dummy(&self) -> StoreLimits { + match self.0.try_borrow() { + Ok(b) => b.store_limits.clone(), + // if borrow error happens, something is wrong so we create a dummy + // with minimal limits to fail asap + Err(_) => StoreLimitsBuilder::new() + .memory_size(0) + .table_elements(0) + .instances(0) + .tables(0) + .memories(0) + .trap_on_grow_failure(true) + .build(), + } + } } /// Default settings for local/sandbox testing only. The actual operations will use parameters @@ -721,6 +710,7 @@ impl Default for BudgetImpl { tracker: vec![(0, None); ContractCostType::variants().len()], enabled: true, fuel_config: Default::default(), + store_limits: Default::default(), }; for ct in ContractCostType::variants() { @@ -1014,3 +1004,56 @@ impl Default for BudgetImpl { b } } + +impl ResourceLimiter for Host { + fn memory_growing( + &mut self, + current: usize, + desired: usize, + maximum: Option, + ) -> Result { + let mut store_limits = self + .as_budget() + .wasmi_store_limits() + .map_err(|_| MemoryError::OutOfBoundsGrowth)?; + store_limits.memory_growing(current, desired, maximum) + } + + fn table_growing( + &mut self, + current: u32, + desired: u32, + maximum: Option, + ) -> Result { + let mut store_limits = + self.as_budget() + .wasmi_store_limits() + .map_err(|_| TableError::GrowOutOfBounds { + maximum: 0, + current: 0, + delta: 0, + })?; + store_limits.table_growing(current, desired, maximum) + } + + fn instances(&self) -> usize { + match self.as_budget().wasmi_store_limits() { + Ok(s) => s.instances(), + Err(_) => 0, + } + } + + fn tables(&self) -> usize { + match self.as_budget().wasmi_store_limits() { + Ok(s) => s.tables(), + Err(_) => 0, + } + } + + fn memories(&self) -> usize { + match self.as_budget().wasmi_store_limits() { + Ok(s) => s.memories(), + Err(_) => 0, + } + } +} diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs index b36842c64..127fa054e 100644 --- a/soroban-env-host/src/host.rs +++ b/soroban-env-host/src/host.rs @@ -52,6 +52,7 @@ use soroban_env_common::xdr::{ ContractEntryBodyType, ContractIdPreimage, ContractIdPreimageFromAddress, ScContractInstance, ScErrorCode, MASK_CONTRACT_DATA_FLAGS_V20, }; +use wasmi::ResourceLimiter; use self::metered_clone::MeteredClone; use self::{ diff --git a/soroban-env-host/src/host/metered_map.rs b/soroban-env-host/src/host/metered_map.rs index 223bcb902..ce7f60906 100644 --- a/soroban-env-host/src/host/metered_map.rs +++ b/soroban-env-host/src/host/metered_map.rs @@ -37,18 +37,18 @@ where { fn charge_access(&self, count: usize, b: &B) -> Result<(), HostError> { b.as_budget() - .batched_charge(ContractCostType::MapEntry, count as u64, None) + .bulk_charge(ContractCostType::MapEntry, count as u64, None) } fn charge_scan(&self, b: &B) -> Result<(), HostError> { b.as_budget() - .batched_charge(ContractCostType::MapEntry, self.map.len() as u64, None) + .bulk_charge(ContractCostType::MapEntry, self.map.len() as u64, None) } fn charge_binsearch(&self, b: &B) -> Result<(), HostError> { let mag = 64 - (self.map.len() as u64).leading_zeros(); b.as_budget() - .batched_charge(ContractCostType::MapEntry, 1 + mag as u64, None) + .bulk_charge(ContractCostType::MapEntry, 1 + mag as u64, None) } } @@ -337,7 +337,7 @@ where a: &MeteredOrdMap, b: &MeteredOrdMap, ) -> Result { - self.as_budget().batched_charge( + self.as_budget().bulk_charge( ContractCostType::MapEntry, a.map.len().min(b.map.len()) as u64, None, @@ -357,7 +357,7 @@ where a: &MeteredOrdMap, b: &MeteredOrdMap, ) -> Result { - self.batched_charge( + self.bulk_charge( ContractCostType::MapEntry, a.map.len().min(b.map.len()) as u64, None, diff --git a/soroban-env-host/src/host/metered_vector.rs b/soroban-env-host/src/host/metered_vector.rs index 057196351..c9416d260 100644 --- a/soroban-env-host/src/host/metered_vector.rs +++ b/soroban-env-host/src/host/metered_vector.rs @@ -31,16 +31,16 @@ where A: DeclaredSizeForMetering, { fn charge_access(&self, count: usize, budget: &Budget) -> Result<(), HostError> { - budget.batched_charge(ContractCostType::VecEntry, count as u64, None) + budget.bulk_charge(ContractCostType::VecEntry, count as u64, None) } fn charge_scan(&self, budget: &Budget) -> Result<(), HostError> { - budget.batched_charge(ContractCostType::VecEntry, self.vec.len() as u64, None) + budget.bulk_charge(ContractCostType::VecEntry, self.vec.len() as u64, None) } fn charge_binsearch(&self, budget: &Budget) -> Result<(), HostError> { let mag = 64 - (self.vec.len() as u64).leading_zeros(); - budget.batched_charge(ContractCostType::VecEntry, 1 + mag as u64, None) + budget.bulk_charge(ContractCostType::VecEntry, 1 + mag as u64, None) } } @@ -335,7 +335,7 @@ where a: &MeteredVector, b: &MeteredVector, ) -> Result { - self.as_budget().batched_charge( + self.as_budget().bulk_charge( ContractCostType::VecEntry, a.vec.len().min(b.vec.len()) as u64, None, @@ -355,7 +355,7 @@ where a: &MeteredVector, b: &MeteredVector, ) -> Result { - self.as_budget().batched_charge( + self.as_budget().bulk_charge( ContractCostType::VecEntry, a.vec.len().min(b.vec.len()) as u64, None, diff --git a/soroban-env-host/src/test/budget_metering.rs b/soroban-env-host/src/test/budget_metering.rs index 2364f7561..e54571521 100644 --- a/soroban-env-host/src/test/budget_metering.rs +++ b/soroban-env-host/src/test/budget_metering.rs @@ -262,7 +262,7 @@ fn total_amount_charged_from_random_inputs() -> Result<(), HostError> { ]; for ty in ContractCostType::variants() { - host.with_budget(|b| b.batched_charge(ty, tracker[ty as usize].0, tracker[ty as usize].1))?; + host.with_budget(|b| b.bulk_charge(ty, tracker[ty as usize].0, tracker[ty as usize].1))?; } let actual = format!("{:?}", host.as_budget()); expect![[r#" diff --git a/soroban-env-host/src/vm.rs b/soroban-env-host/src/vm.rs index a0de31fae..2619fffd9 100644 --- a/soroban-env-host/src/vm.rs +++ b/soroban-env-host/src/vm.rs @@ -24,7 +24,10 @@ use soroban_env_common::{ ConversionError, SymbolStr, TryIntoVal, WasmiMarshal, }; -use wasmi::{Engine, FuelConsumptionMode, Func, Instance, Linker, Memory, Module, Store, Value}; +use wasmi::{ + Engine, FuelConsumptionMode, Func, Instance, Linker, Memory, Module, Store, StoreLimitsBuilder, + Value, +}; #[cfg(any(test, feature = "testutils"))] use crate::VmCaller; @@ -161,6 +164,11 @@ impl Vm { Self::check_meta_section(host, &module)?; let mut store = Store::new(&engine, host.clone()); + + // install the limiter, which currently only limits the wasm linear memory size + store.limiter(|host| host); + // TODO: charge the max memory size limit upfront as WasmMemAlloc + let mut linker = >::new(&engine); for hf in HOST_FUNCTIONS { @@ -221,7 +229,7 @@ impl Vm { inputs: &[Value], ) -> Result { let mut wasm_ret: [Value; 1] = [Value::I64(0)]; - self.store.try_borrow_mut_or_err()?.fill_fuels(host)?; + self.store.try_borrow_mut_or_err()?.add_fuel_to_vm(host)?; let res = func.call( &mut *self.store.try_borrow_mut_or_err()?, inputs, @@ -232,7 +240,9 @@ impl Vm { // 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.try_borrow_mut_or_err()?.return_fuels(host)?; + self.store + .try_borrow_mut_or_err()? + .return_fuel_to_host(host)?; if let Err(e) = res { // When a call fails with a wasmi::Error::Trap that carries a HostError diff --git a/soroban-env-host/src/vm/dispatch.rs b/soroban-env-host/src/vm/dispatch.rs index 505e97d67..2c2707420 100644 --- a/soroban-env-host/src/vm/dispatch.rs +++ b/soroban-env-host/src/vm/dispatch.rs @@ -77,7 +77,7 @@ macro_rules! generate_dispatch_functions { // This is where the VM -> Host boundary is crossed. // We first return all fuels from the VM back to the host such that // the host maintains control of the budget. - FuelRefillable::return_fuels(&mut caller, &host).map_err(|he| Trap::from(he))?; + FuelRefillable::return_fuel_to_host(&mut caller, &host).map_err(|he| Trap::from(he))?; host.charge_budget(ContractCostType::InvokeHostFunction, None)?; let mut vmcaller = VmCaller(Some(caller)); @@ -115,7 +115,7 @@ macro_rules! generate_dispatch_functions { // This is where the Host->VM boundary is crossed. // We supply the remaining host budget as fuel to the VM. let caller = vmcaller.try_mut().map_err(|e| Trap::from(HostError::from(e)))?; - FuelRefillable::fill_fuels(caller, &host).map_err(|he| Trap::from(he))?; + FuelRefillable::add_fuel_to_vm(caller, &host).map_err(|he| Trap::from(he))?; res } diff --git a/soroban-env-host/src/vm/fuel_refillable.rs b/soroban-env-host/src/vm/fuel_refillable.rs index ccf98837c..9b57a8d85 100644 --- a/soroban-env-host/src/vm/fuel_refillable.rs +++ b/soroban-env-host/src/vm/fuel_refillable.rs @@ -1,25 +1,25 @@ use crate::{ budget::AsBudget, - xdr::{ScErrorCode, ScErrorType}, + xdr::{ContractCostType, ScErrorCode, ScErrorType}, Host, HostError, }; use wasmi::{errors::FuelError, Caller, Store}; pub(crate) trait FuelRefillable { - fn fuels_consumed(&self) -> Result<(u64, u64), HostError>; + fn fuel_consumed(&self) -> Result; - fn fuels_total(&self) -> Result<(u64, u64), HostError>; + fn fuel_total(&self) -> Result; - fn add_fuels(&mut self, cpu: u64, mem: u64) -> Result<(), HostError>; + fn add_fuel(&mut self, fuel: u64) -> Result<(), HostError>; - fn reset_fuels(&mut self) -> Result<(), HostError>; + fn reset_fuel(&mut self) -> Result<(), HostError>; fn is_clean(&self) -> Result { - Ok(self.fuels_consumed()? == (0, 0) && self.fuels_total()? == (0, 0)) + Ok(self.fuel_consumed()? == 0 && self.fuel_total()? == 0) } - fn fill_fuels(&mut self, host: &Host) -> Result<(), HostError> { + fn add_fuel_to_vm(&mut self, host: &Host) -> Result<(), HostError> { if !self.is_clean()? { return Err(host.err( ScErrorType::WasmVm, @@ -28,54 +28,41 @@ pub(crate) trait FuelRefillable { &[], )); } - let (cpu, mem) = host.as_budget().get_fuels_budget()?; - self.add_fuels(cpu, mem) + let fuel = host.as_budget().get_cpu_insns_remaining_as_fuel()?; + self.add_fuel(fuel) } - fn return_fuels(&mut self, host: &Host) -> Result<(), HostError> { - let (cpu, mem) = self.fuels_consumed()?; - host.as_budget().apply_wasmi_fuels(cpu, mem)?; - self.reset_fuels() + fn return_fuel_to_host(&mut self, host: &Host) -> Result<(), HostError> { + let fuel = self.fuel_consumed()?; + host.as_budget() + .bulk_charge(ContractCostType::WasmInsnExec, fuel, None)?; + self.reset_fuel() } } -//wasmi::Store + macro_rules! impl_refillable_for_store { ($store: ty) => { impl<'a> FuelRefillable for $store { - fn fuels_consumed(&self) -> Result<(u64, u64), HostError> { - let cpu = self.fuel_consumed().ok_or_else(|| { - HostError::from(wasmi::Error::Store(FuelError::FuelMeteringDisabled)) - })?; - let mem = self.mem_fuel_consumed().ok_or_else(|| { + fn fuel_consumed(&self) -> Result { + self.fuel_consumed().ok_or_else(|| { HostError::from(wasmi::Error::Store(FuelError::FuelMeteringDisabled)) - })?; - Ok((cpu, mem)) + }) } - fn fuels_total(&self) -> Result<(u64, u64), HostError> { - let cpu = self.fuel_total().ok_or_else(|| { - HostError::from(wasmi::Error::Store(FuelError::FuelMeteringDisabled)) - })?; - let mem = self.mem_fuel_total().ok_or_else(|| { + fn fuel_total(&self) -> Result { + self.fuel_total().ok_or_else(|| { HostError::from(wasmi::Error::Store(FuelError::FuelMeteringDisabled)) - })?; - Ok((cpu, mem)) + }) } - fn add_fuels(&mut self, cpu: u64, mem: u64) -> Result<(), HostError> { - self.add_fuel(cpu) - .map_err(|fe| HostError::from(wasmi::Error::Store(fe)))?; - self.add_mem_fuel(mem) - .map_err(|fe| HostError::from(wasmi::Error::Store(fe)))?; - Ok(()) + fn add_fuel(&mut self, fuel: u64) -> Result<(), HostError> { + self.add_fuel(fuel) + .map_err(|fe| HostError::from(wasmi::Error::Store(fe))) } - fn reset_fuels(&mut self) -> Result<(), HostError> { + fn reset_fuel(&mut self) -> Result<(), HostError> { self.reset_fuel() - .map_err(|fe| HostError::from(wasmi::Error::Store(fe)))?; - self.reset_mem_fuel() - .map_err(|fe| HostError::from(wasmi::Error::Store(fe)))?; - Ok(()) + .map_err(|fe| HostError::from(wasmi::Error::Store(fe))) } } };