From d7af8b89c871b78a8091eec18775b1a497114dc7 Mon Sep 17 00:00:00 2001 From: vyzo Date: Wed, 4 May 2022 12:27:59 +0300 Subject: [PATCH 01/21] propagate all gas outputs in ApplyRet --- fvm/src/executor/default.rs | 23 +++++++++++++++++------ fvm/src/executor/mod.rs | 16 +++++++++++++++- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/fvm/src/executor/default.rs b/fvm/src/executor/default.rs index 81f74d18e..97eb845d5 100644 --- a/fvm/src/executor/default.rs +++ b/fvm/src/executor/default.rs @@ -164,9 +164,14 @@ where }), ApplyKind::Implicit => Ok(ApplyRet { msg_receipt: receipt, - failure_info, penalty: TokenAmount::zero(), miner_tip: TokenAmount::zero(), + base_fee_burn: TokenAmount::from(0), + over_estimation_burn: TokenAmount::from(0), + refund: TokenAmount::from(0), + gas_refund: 0, + gas_burned: 0, + failure_info, exec_trace, }), } @@ -326,11 +331,12 @@ where // NOTE: we don't support old network versions in the FVM, so we always burn. let GasOutputs { base_fee_burn, - miner_tip, over_estimation_burn, - refund, miner_penalty, - .. + miner_tip, + refund, + gas_refund, + gas_burned, } = GasOutputs::compute( receipt.gas_used, msg.gas_limit, @@ -365,15 +371,20 @@ where // refund unused gas transfer_to_actor(&msg.from, &refund)?; - if (&base_fee_burn + over_estimation_burn + &refund + &miner_tip) != gas_cost { + if (&base_fee_burn + &over_estimation_burn + &refund + &miner_tip) != gas_cost { // Sanity check. This could be a fatal error. return Err(anyhow!("Gas handling math is wrong")); } Ok(ApplyRet { msg_receipt: receipt, - failure_info, penalty: miner_penalty, miner_tip, + base_fee_burn, + over_estimation_burn, + refund, + gas_refund, + gas_burned, + failure_info, exec_trace: vec![], }) } diff --git a/fvm/src/executor/mod.rs b/fvm/src/executor/mod.rs index d38e30ebf..4d430bf9f 100644 --- a/fvm/src/executor/mod.rs +++ b/fvm/src/executor/mod.rs @@ -5,6 +5,7 @@ use std::fmt::Display; pub use default::DefaultExecutor; use fvm_ipld_encoding::RawBytes; use fvm_shared::bigint::{BigInt, Sign}; +use fvm_shared::econ::TokenAmount; use fvm_shared::error::ExitCode; use fvm_shared::message::Message; use fvm_shared::receipt::Receipt; @@ -70,6 +71,14 @@ pub struct ApplyRet { pub penalty: BigInt, /// Tip given to miner from message. pub miner_tip: BigInt, + + // Gas stuffs + pub base_fee_burn: TokenAmount, + pub over_estimation_burn: TokenAmount, + pub refund: TokenAmount, + pub gas_refund: i64, + pub gas_burned: i64, + /// Additional failure information for debugging, if any. pub failure_info: Option, /// Execution trace information, for debugging. @@ -90,8 +99,13 @@ impl ApplyRet { gas_used: 0, }, penalty: miner_penalty, - failure_info: Some(ApplyFailure::PreValidation(message.into())), miner_tip: BigInt::zero(), + base_fee_burn: TokenAmount::from(0), + over_estimation_burn: TokenAmount::from(0), + refund: TokenAmount::from(0), + gas_refund: 0, + gas_burned: 0, + failure_info: Some(ApplyFailure::PreValidation(message.into())), exec_trace: vec![], } } From 21717136718a77b07d41e48a0c06c980fef5def0 Mon Sep 17 00:00:00 2001 From: vyzo Date: Wed, 4 May 2022 15:03:02 +0300 Subject: [PATCH 02/21] use tuple serialization for system actor state --- fvm/src/system_actor.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fvm/src/system_actor.rs b/fvm/src/system_actor.rs index bbb11a309..7a8cca34f 100644 --- a/fvm/src/system_actor.rs +++ b/fvm/src/system_actor.rs @@ -1,16 +1,16 @@ use anyhow::Context; use cid::Cid; use fvm_ipld_blockstore::Blockstore; +use fvm_ipld_encoding::tuple::{Deserialize_tuple, Serialize_tuple}; use fvm_ipld_encoding::{Cbor, CborStore}; use fvm_shared::address::Address; -use serde::{Deserialize, Serialize}; use crate::kernel::{ClassifyResult, Result}; use crate::state_tree::{ActorState, StateTree}; pub const SYSTEM_ACTOR_ADDR: Address = Address::new_id(0); -#[derive(Default, Deserialize, Serialize)] +#[derive(Default, Deserialize_tuple, Serialize_tuple)] pub struct State { // builtin actor registry: Vec<(String, Cid)> pub builtin_actors: Cid, From f4a74732560a08d750cdd497e7aa90dc123cfe31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Magiera?= Date: Wed, 4 May 2022 15:58:15 -0400 Subject: [PATCH 03/21] feat: fvm-wasm-instrument stack limiter and gas (#494) --- fvm/Cargo.toml | 1 + fvm/src/call_manager/default.rs | 68 ++++--- fvm/src/gas/mod.rs | 121 +++++++++-- fvm/src/gas/price_list.rs | 61 +++--- fvm/src/kernel/default.rs | 10 + fvm/src/kernel/mod.rs | 11 +- fvm/src/lib.rs | 10 +- fvm/src/machine/engine.rs | 190 ++++++++++++++---- fvm/src/machine/mod.rs | 7 +- fvm/src/syscalls/bind.rs | 90 +++++---- fvm/src/syscalls/mod.rs | 62 +----- .../conformance/benches/bench_conformance.rs | 6 +- .../benches/bench_conformance_overhead.rs | 16 +- testing/conformance/benches/bench_drivers.rs | 14 +- testing/conformance/src/driver.rs | 6 +- testing/conformance/src/vm.rs | 30 +-- testing/conformance/tests/runner.rs | 22 +- testing/integration/src/builtin.rs | 3 +- testing/integration/src/tester.rs | 13 +- testing/integration/tests/lib.rs | 114 +++++++++++ 20 files changed, 577 insertions(+), 278 deletions(-) diff --git a/fvm/Cargo.toml b/fvm/Cargo.toml index a442bd730..02392cc35 100644 --- a/fvm/Cargo.toml +++ b/fvm/Cargo.toml @@ -39,6 +39,7 @@ log = "0.4.14" byteorder = "1.4.3" anymap = "0.12.1" blake2b_simd = "1.0.0" +fvm-wasm-instrument = { version = "0.2.0", features = ["bulk"] } [dependencies.wasmtime] version = "0.35.2" diff --git a/fvm/src/call_manager/default.rs b/fvm/src/call_manager/default.rs index 9798e0a7d..838720cf8 100644 --- a/fvm/src/call_manager/default.rs +++ b/fvm/src/call_manager/default.rs @@ -1,5 +1,3 @@ -use std::cmp::max; - use anyhow::Context; use derive_more::{Deref, DerefMut}; use fvm_ipld_encoding::{RawBytes, DAG_CBOR}; @@ -7,9 +5,9 @@ use fvm_shared::actor::builtin::Type; use fvm_shared::address::{Address, Protocol}; use fvm_shared::econ::TokenAmount; use fvm_shared::error::{ErrorNumber, ExitCode}; -use fvm_shared::version::NetworkVersion; use fvm_shared::{ActorID, MethodNum, METHOD_SEND}; use num_traits::Zero; +use wasmtime::Val; use super::{Backtrace, CallManager, InvocationResult, NO_DATA_BLOCK_ID}; use crate::call_manager::backtrace::Frame; @@ -316,7 +314,6 @@ where // it returns a referenced copy. let engine = self.engine().clone(); - let gas_available = self.gas_tracker.gas_available(); log::trace!("calling {} -> {}::{}", from, to, method); self.map_mut(|cm| { // Make the kernel. @@ -334,21 +331,7 @@ where }; // Make a store. - let gas_used = kernel.gas_used(); - let exec_units_to_add = match kernel.network_version() { - NetworkVersion::V14 | NetworkVersion::V15 => i64::MAX, - _ => kernel - .price_list() - .gas_to_exec_units(max(gas_available.saturating_sub(gas_used), 0), false), - }; - let mut store = engine.new_store(kernel); - if let Err(err) = store.add_fuel(u64::try_from(exec_units_to_add).unwrap_or(0)) { - return ( - Err(ExecutionError::Fatal(err)), - store.into_data().kernel.into_call_manager(), - ); - } // Instantiate the module. let instance = match engine @@ -362,6 +345,26 @@ where // From this point on, there are no more syscall errors, only aborts. let result: std::result::Result = (|| { + let initial_milligas = + store + .data_mut() + .kernel + .borrow_milligas() + .map_err(|e| match e { + ExecutionError::OutOfGas => Abort::OutOfGas, + ExecutionError::Fatal(m) => Abort::Fatal(m), + _ => Abort::Fatal(anyhow::Error::msg( + "setting available gas on entering wasm", + )), + })?; + + store + .data() + .avail_gas_global + .clone() + .set(&mut store, Val::I64(initial_milligas)) + .map_err(Abort::Fatal)?; + // Lookup the invoke method. let invoke: wasmtime::TypedFunc<(u32,), u32> = instance .get_typed_func(&mut store, "invoke") @@ -371,19 +374,26 @@ where // Invoke it. let res = invoke.call(&mut store, (param_id,)); - // Charge gas for the "latest" use of execution units (all the exec units used since the most recent syscall) - // We do this by first loading the _total_ execution units consumed - let exec_units_consumed = store - .fuel_consumed() - .context("expected to find fuel consumed") - .map_err(Abort::Fatal)?; - // Then, pass the _total_ exec_units_consumed to the InvocationData, - // which knows how many execution units had been consumed at the most recent snapshot - // It will charge gas for the delta between the total units (the number we provide) and its snapshot + // Return gas tracking to GasTracker + let available_milligas = + match store.data_mut().avail_gas_global.clone().get(&mut store) { + Val::I64(g) => Ok(g), + _ => Err(Abort::Fatal(anyhow::Error::msg( + "failed to get available gas from wasm after returning", + ))), + }?; + store .data_mut() - .charge_gas_for_exec_units(exec_units_consumed) - .map_err(|e| Abort::from_error(ExitCode::SYS_ASSERTION_FAILED, e))?; + .kernel + .return_milligas("wasm_exec_last", available_milligas) + .map_err(|e| match e { + ExecutionError::OutOfGas => Abort::OutOfGas, + ExecutionError::Fatal(m) => Abort::Fatal(m), + _ => Abort::Fatal(anyhow::Error::msg( + "setting available gas on existing wasm", + )), + })?; // If the invocation failed due to running out of exec_units, we have already detected it and returned OutOfGas above. // Any other invocation failure is returned here as an Abort diff --git a/fvm/src/gas/mod.rs b/fvm/src/gas/mod.rs index b140fa2aa..dc462b8cf 100644 --- a/fvm/src/gas/mod.rs +++ b/fvm/src/gas/mod.rs @@ -3,61 +3,136 @@ pub use self::charge::GasCharge; pub(crate) use self::outputs::GasOutputs; -pub use self::price_list::{price_list_by_network_version, PriceList}; +pub use self::price_list::{price_list_by_network_version, PriceList, WasmGasPrices}; use crate::kernel::{ExecutionError, Result}; mod charge; mod outputs; mod price_list; +pub const MILLIGAS_PRECISION: i64 = 1000; + pub struct GasTracker { - gas_available: i64, - gas_used: i64, + milligas_limit: i64, + milligas_used: i64, + + /// A flag indicating whether this GasTracker is currently responsible for + /// gas accounting. A 'false' value indicates that gas accounting is + /// handled somewhere else (eg. in wasm execution). + /// + /// Creating gas charges is only allowed when own_limit is true. + own_limit: bool, } impl GasTracker { - pub fn new(gas_available: i64, gas_used: i64) -> Self { + pub fn new(gas_limit: i64, gas_used: i64) -> Self { Self { - gas_available, - gas_used, + milligas_limit: gas_to_milligas(gas_limit), + milligas_used: gas_to_milligas(gas_used), + own_limit: true, } } /// Safely consumes gas and returns an out of gas error if there is not sufficient /// enough gas remaining for charge. - pub fn charge_gas(&mut self, charge: GasCharge) -> Result<()> { - let to_use = charge.total(); - match self.gas_used.checked_add(to_use) { + fn charge_milligas(&mut self, name: &str, to_use: i64) -> Result<()> { + if !self.own_limit { + return Err(ExecutionError::Fatal(anyhow::Error::msg( + "charge_gas called when gas_limit owned by execution", + ))); + } + + match self.milligas_used.checked_add(to_use) { None => { - log::trace!("gas overflow: {}", charge.name); - self.gas_used = self.gas_available; + log::trace!("gas overflow: {}", name); + self.milligas_used = self.milligas_limit; Err(ExecutionError::OutOfGas) } Some(used) => { - log::trace!("charged {} gas: {}", to_use, charge.name); - if used > self.gas_available { - log::trace!("out of gas: {}", charge.name); - self.gas_used = self.gas_available; + log::trace!("charged {} gas: {}", to_use, name); + if used > self.milligas_limit { + log::trace!("out of gas: {}", name); + self.milligas_used = self.milligas_limit; Err(ExecutionError::OutOfGas) } else { - self.gas_used = used; + self.milligas_used = used; Ok(()) } } } } + pub fn charge_gas(&mut self, charge: GasCharge) -> Result<()> { + self.charge_milligas( + charge.name, + charge.total().saturating_mul(MILLIGAS_PRECISION), + ) + } + + /// returns available milligas; makes the gas tracker reject gas charges with + /// a fatal error until return_milligas is called. + pub fn borrow_milligas(&mut self) -> Result { + if !self.own_limit { + return Err(ExecutionError::Fatal(anyhow::Error::msg( + "borrow_milligas called on GasTracker which doesn't own gas limit", + ))); + } + self.own_limit = false; + + Ok(self.milligas_limit - self.milligas_used) + } + + /// sets new available gas, creating a new gas charge if needed + pub fn return_milligas(&mut self, name: &str, new_avail_mgas: i64) -> Result<()> { + if self.own_limit { + return Err(ExecutionError::Fatal(anyhow::Error::msg(format!( + "gastracker already owns gas_limit, charge: {}", + name + )))); + } + self.own_limit = true; + + let old_avail_milligas = self.milligas_limit - self.milligas_used; + let used = old_avail_milligas - new_avail_mgas; + + if used < 0 { + return Err(ExecutionError::Fatal(anyhow::Error::msg( + "negative gas charge in set_available_gas", + ))); + } + + self.charge_milligas(name, used) + } + /// Getter for gas available. - pub fn gas_available(&self) -> i64 { - self.gas_available + pub fn gas_limit(&self) -> i64 { + milligas_to_gas(self.milligas_limit, false) } /// Getter for gas used. pub fn gas_used(&self) -> i64 { - self.gas_used + milligas_to_gas(self.milligas_used, true) } } +/// Converts the specified gas into equivalent fractional gas units +#[inline] +fn gas_to_milligas(gas: i64) -> i64 { + gas.saturating_mul(MILLIGAS_PRECISION) +} + +/// Converts the specified fractional gas units into gas units +#[inline] +fn milligas_to_gas(milligas: i64, round_up: bool) -> i64 { + let mut div_result = milligas / MILLIGAS_PRECISION; + if milligas > 0 && round_up && milligas % MILLIGAS_PRECISION != 0 { + div_result = div_result.saturating_add(1); + } else if milligas < 0 && !round_up && milligas % MILLIGAS_PRECISION != 0 { + div_result = div_result.saturating_sub(1); + } + div_result +} + #[cfg(test)] mod tests { use super::*; @@ -71,4 +146,12 @@ mod tests { assert_eq!(t.gas_used(), 20); assert!(t.charge_gas(GasCharge::new("", 1, 0)).is_err()) } + + #[test] + fn milligas_to_gas_round() { + assert_eq!(milligas_to_gas(100, false), 0); + assert_eq!(milligas_to_gas(100, true), 1); + assert_eq!(milligas_to_gas(-100, false), -1); + assert_eq!(milligas_to_gas(-100, true), 0); + } } diff --git a/fvm/src/gas/price_list.rs b/fvm/src/gas/price_list.rs index a22b89379..a91c0ceac 100644 --- a/fvm/src/gas/price_list.rs +++ b/fvm/src/gas/price_list.rs @@ -11,6 +11,8 @@ use fvm_shared::sector::{ }; use fvm_shared::version::NetworkVersion; use fvm_shared::{MethodNum, METHOD_SEND}; +use fvm_wasm_instrument::gas_metering::{MemoryGrowCost, Rules}; +use fvm_wasm_instrument::parity_wasm::elements::Instruction; use lazy_static::lazy_static; use num_traits::Zero; @@ -118,7 +120,6 @@ lazy_static! { .copied() .collect(), - gas_per_exec_unit: 0, get_randomness_base: 0, get_randomness_per_byte: 0, @@ -131,6 +132,10 @@ lazy_static! { block_create_base: 0, block_link_base: 353640, block_stat: 0, + + wasm_rules: WasmGasPrices{ + exec_instruction_cost_milli: 0, + }, }; static ref SKYR_PRICES: PriceList = PriceList { @@ -240,8 +245,6 @@ lazy_static! { block_link_per_byte_cost: 1, // TODO: PARAM_FINISH - // TODO: PARAM_FINISH - gas_per_exec_unit: 2, // TODO: PARAM_FINISH get_randomness_base: 1, // TODO: PARAM_FINISH @@ -257,6 +260,11 @@ lazy_static! { block_link_base: 1, // TODO: PARAM_FINISH block_stat: 1, + + wasm_rules: WasmGasPrices{ + exec_instruction_cost_milli: 5000, + }, + // TODO: PARAM_FINISH }; } @@ -366,8 +374,6 @@ pub struct PriceList { pub(crate) verify_post_lookup: AHashMap, pub(crate) verify_consensus_fault: i64, pub(crate) verify_replica_update: i64, - // 1 Exec Unit = gas_per_exec_unit * 1 Gas - pub(crate) gas_per_exec_unit: i64, pub(crate) get_randomness_base: i64, pub(crate) get_randomness_per_byte: i64, @@ -381,6 +387,13 @@ pub struct PriceList { pub(crate) block_create_base: i64, pub(crate) block_link_base: i64, pub(crate) block_stat: i64, + + pub(crate) wasm_rules: WasmGasPrices, +} + +#[derive(Clone, Debug, Eq, PartialEq, Hash)] +pub struct WasmGasPrices { + pub(crate) exec_instruction_cost_milli: u64, } impl PriceList { @@ -537,33 +550,6 @@ impl PriceList { GasCharge::new("OnVerifyConsensusFault", self.verify_consensus_fault, 0) } - /// Returns the gas required for the specified exec_units. - #[inline] - pub fn on_consume_exec_units(&self, exec_units: u64) -> GasCharge<'static> { - GasCharge::new( - "OnConsumeExecUnits", - self.gas_per_exec_unit - .saturating_mul(i64::try_from(exec_units).unwrap_or(i64::MAX)), - 0, - ) - } - - /// Converts the specified gas into equivalent exec_units - /// Note: In rare cases the provided `gas` may be negative - #[inline] - pub fn gas_to_exec_units(&self, gas: i64, round_up: bool) -> i64 { - match self.gas_per_exec_unit { - 0 => 0, - v => { - let mut div_result = gas / v; - if round_up && gas % v != 0 { - div_result = div_result.saturating_add(1); - } - div_result - } - } - } - /// Returns the cost of the gas required for getting randomness from the client, based on the /// numebr of bytes of entropy. #[inline] @@ -648,3 +634,14 @@ pub fn price_list_by_network_version(network_version: NetworkVersion) -> &'stati _ => &SKYR_PRICES, } } + +impl Rules for WasmGasPrices { + fn instruction_cost(&self, _instruction: &Instruction) -> Option { + Some(self.exec_instruction_cost_milli) + } + + fn memory_grow_cost(&self) -> MemoryGrowCost { + // todo use pricelist + MemoryGrowCost::Free + } +} diff --git a/fvm/src/kernel/default.rs b/fvm/src/kernel/default.rs index 1aa894efc..c95761ce5 100644 --- a/fvm/src/kernel/default.rs +++ b/fvm/src/kernel/default.rs @@ -760,6 +760,16 @@ where self.call_manager.charge_gas(charge) } + fn return_milligas(&mut self, name: &str, new_gas: i64) -> Result<()> { + self.call_manager + .gas_tracker_mut() + .return_milligas(name, new_gas) + } + + fn borrow_milligas(&mut self) -> Result { + self.call_manager.gas_tracker_mut().borrow_milligas() + } + fn price_list(&self) -> &PriceList { self.call_manager.price_list() } diff --git a/fvm/src/kernel/mod.rs b/fvm/src/kernel/mod.rs index 0cdae1e3b..a9953a119 100644 --- a/fvm/src/kernel/mod.rs +++ b/fvm/src/kernel/mod.rs @@ -213,11 +213,6 @@ pub trait CircSupplyOps { } /// Operations for explicit gas charging. -/// -/// TODO this is unsafe; most gas charges should occur as part of syscalls, but -/// some built-in actors currently charge gas explicitly for concrete actions. -/// In the future (Phase 1), this should disappear and be replaced by gas instrumentation -/// at the WASM level. pub trait GasOps { /// GasUsed return the gas used by the transaction so far. fn gas_used(&self) -> i64; @@ -226,6 +221,12 @@ pub trait GasOps { /// `name` provides information about gas charging point fn charge_gas(&mut self, name: &str, compute: i64) -> Result<()>; + /// Returns available gas. + fn borrow_milligas(&mut self) -> Result; + + /// Sets available gas to a new value, creating a gas charge if needed + fn return_milligas(&mut self, name: &str, newgas: i64) -> Result<()>; + fn price_list(&self) -> &PriceList; } diff --git a/fvm/src/lib.rs b/fvm/src/lib.rs index 41cb409ca..705f8e67e 100644 --- a/fvm/src/lib.rs +++ b/fvm/src/lib.rs @@ -119,11 +119,13 @@ mod test { let actors_cid = bs.put_cbor(&(0, manifest_cid), Code::Blake2b256).unwrap(); + let mc = NetworkConfig::new(fvm_shared::version::NetworkVersion::V14) + .override_actors(actors_cid) + .for_epoch(0, root); + let machine = DefaultMachine::new( - &Engine::default(), - &NetworkConfig::new(fvm_shared::version::NetworkVersion::V14) - .override_actors(actors_cid) - .for_epoch(0, root), + &Engine::new_default((&mc.network).into()).unwrap(), + &mc, bs, DummyExterns, ) diff --git a/fvm/src/machine/engine.rs b/fvm/src/machine/engine.rs index 5fdb6062c..567da8e0b 100644 --- a/fvm/src/machine/engine.rs +++ b/fvm/src/machine/engine.rs @@ -1,13 +1,16 @@ -use std::collections::hash_map::Entry; +use std::collections::hash_map::Entry::{Occupied, Vacant}; use std::collections::HashMap; use std::ops::Deref; use std::sync::{Arc, Mutex}; -use anyhow::anyhow; +use anyhow::{anyhow, Context}; use cid::Cid; use fvm_ipld_blockstore::Blockstore; -use wasmtime::{Linker, Module}; +use fvm_wasm_instrument::gas_metering::GAS_COUNTER_NAME; +use wasmtime::{Global, GlobalType, Linker, Module, Mutability, Val, ValType}; +use crate::gas::WasmGasPrices; +use crate::machine::NetworkConfig; use crate::syscalls::{bind_syscalls, InvocationData}; use crate::Kernel; @@ -15,11 +18,56 @@ use crate::Kernel; #[derive(Clone)] pub struct Engine(Arc); +/// Container managing engines with different consensus-affecting configurations. +#[derive(Clone)] +pub struct MultiEngine(Arc>>); + +#[derive(Clone, Eq, PartialEq, Hash)] +pub struct EngineConfig { + pub max_wasm_stack: u32, + pub wasm_prices: &'static WasmGasPrices, +} + +impl From<&NetworkConfig> for EngineConfig { + fn from(nc: &NetworkConfig) -> Self { + EngineConfig { + max_wasm_stack: nc.max_wasm_stack, + wasm_prices: &nc.price_list.wasm_rules, + } + } +} + +impl MultiEngine { + pub fn new() -> MultiEngine { + MultiEngine(Arc::new(Mutex::new(HashMap::new()))) + } + + pub fn get(&self, nc: &NetworkConfig) -> anyhow::Result { + let mut engines = self + .0 + .lock() + .map_err(|_| anyhow::Error::msg("multiengine lock is poisoned"))?; + + let ec: EngineConfig = nc.into(); + + let engine = match engines.entry(ec.clone()) { + Occupied(entry) => entry.into_mut(), + Vacant(entry) => entry.insert(Engine::new_default(ec)?), + }; + + Ok(engine.clone()) + } +} + +impl Default for MultiEngine { + fn default() -> Self { + Self::new() + } +} + pub fn default_wasmtime_config() -> wasmtime::Config { let mut c = wasmtime::Config::default(); - // c.max_wasm_stack(); https://github.com/filecoin-project/ref-fvm/issues/424 - // wasmtime default: false c.wasm_threads(false); @@ -43,6 +91,8 @@ pub fn default_wasmtime_config() -> wasmtime::Config { // wasmtime default: depends on the arch // > This is true by default on x86-64, and false by default on other architectures. + // + // Not supported in wasm-instrument/parity-wasm c.wasm_reference_types(false); // wasmtime default: false @@ -55,23 +105,27 @@ pub fn default_wasmtime_config() -> wasmtime::Config { // > not enabled by default. c.cranelift_nan_canonicalization(true); - // c.cranelift_opt_level(Speed); ? + // Set to something much higher than the instrumented limiter. + c.max_wasm_stack(64 << 20).unwrap(); - c.consume_fuel(true); + // Execution cost accouting is done through wasm instrumentation, + c.consume_fuel(false); - c -} + // c.cranelift_opt_level(Speed); ? -impl Default for Engine { - fn default() -> Self { - Engine::new(&default_wasmtime_config()).unwrap() - } + c } struct EngineInner { engine: wasmtime::Engine, + + /// dummy gas global used in store costructor to avoid making + /// InvocationData.avail_gas_global an Option + dummy_gas_global: Global, + module_cache: Mutex>, instance_cache: Mutex>, + config: EngineConfig, } impl Deref for Engine { @@ -83,25 +137,30 @@ impl Deref for Engine { } impl Engine { - /// Create a new Engine from a wasmtime config. - pub fn new(c: &wasmtime::Config) -> anyhow::Result { - Ok(wasmtime::Engine::new(c)?.into()) + pub fn new_default(ec: EngineConfig) -> anyhow::Result { + Engine::new(&default_wasmtime_config(), ec) } -} -impl From for Engine { - fn from(engine: wasmtime::Engine) -> Self { - Engine(Arc::new(EngineInner { + /// Create a new Engine from a wasmtime config. + pub fn new(c: &wasmtime::Config, ec: EngineConfig) -> anyhow::Result { + let engine = wasmtime::Engine::new(c)?; + + let mut dummy_store = wasmtime::Store::new(&engine, ()); + let gg_type = GlobalType::new(ValType::I64, Mutability::Var); + let dummy_gg = Global::new(&mut dummy_store, gg_type, Val::I64(0)) + .expect("failed to create dummy gas global"); + + Ok(Engine(Arc::new(EngineInner { engine, + dummy_gas_global: dummy_gg, module_cache: Default::default(), instance_cache: Mutex::new(anymap::Map::new()), - })) + config: ec, + }))) } } - struct Cache { linker: wasmtime::Linker>, - instances: HashMap>>, } impl Engine { @@ -125,7 +184,7 @@ impl Engine { &cid.to_string() ) })?; - let module = Module::from_binary(&self.0.engine, wasm.as_slice())?; + let module = self.load_raw(wasm.as_slice())?; cache.insert(*cid, module); } Ok(()) @@ -137,7 +196,7 @@ impl Engine { let module = match cache.get(k) { Some(module) => module.clone(), None => { - let module = Module::from_binary(&self.0.engine, wasm)?; + let module = self.load_raw(wasm)?; cache.insert(*k, module.clone()); module } @@ -145,6 +204,44 @@ impl Engine { Ok(module) } + fn load_raw(&self, raw_wasm: &[u8]) -> anyhow::Result { + // First make sure that non-instrumented wasm is valid + Module::validate(&self.0.engine, raw_wasm) + .map_err(anyhow::Error::msg) + .with_context(|| "failed to validate actor wasm")?; + + // Note: when adding debug mode support (with recorded syscall replay) don't instrument to + // avoid breaking debug info + + use fvm_wasm_instrument::gas_metering::inject; + use fvm_wasm_instrument::inject_stack_limiter; + use fvm_wasm_instrument::parity_wasm::deserialize_buffer; + + let m = deserialize_buffer(raw_wasm)?; + + // stack limiter adds post/pre-ambles to call instructions; We want to do that + // before injecting gas accounting calls to avoid this overhead in every single + // block of code. + let m = + inject_stack_limiter(m, self.0.config.max_wasm_stack).map_err(anyhow::Error::msg)?; + + // inject gas metering based on a price list. This function will + // * add a new mutable i64 global import, gas.gas_counter + // * push a gas counter function which deduces gas from the global, and + // traps when gas.gas_counter is less than zero + // * optionally push a function which wraps memory.grow instruction + // making it charge gas based on memory requested + // * divide code into metered blocks, and add a call to the gas counter + // function before entering each metered block + let m = inject(m, self.0.config.wasm_prices, "gas") + .map_err(|_| anyhow::Error::msg("injecting gas counter failed"))?; + + let wasm = m.to_bytes()?; + let module = Module::from_binary(&self.0.engine, wasm.as_slice())?; + + Ok(module) + } + /// Load compiled wasm code into the engine. /// /// # Safety @@ -186,32 +283,39 @@ impl Engine { anymap::Entry::Occupied(e) => e.into_mut(), anymap::Entry::Vacant(e) => e.insert({ let mut linker = Linker::new(&self.0.engine); + linker.allow_shadowing(true); + bind_syscalls(&mut linker)?; - Cache { - linker, - instances: HashMap::new(), - } + Cache { linker } }), }; - let instance_pre = match cache.instances.entry(*k) { - Entry::Occupied(e) => e.into_mut(), - Entry::Vacant(e) => { - let module_cache = self.0.module_cache.lock().expect("module_cache poisoned"); - let module = match module_cache.get(k) { - Some(module) => module, - None => return Ok(None), - }; - // We can cache the "pre instance" because our linker only has host functions. - let pre = cache.linker.instantiate_pre(&mut *store, module)?; - e.insert(pre) - } + cache + .linker + .define("gas", GAS_COUNTER_NAME, store.data_mut().avail_gas_global)?; + + let module_cache = self.0.module_cache.lock().expect("module_cache poisoned"); + let module = match module_cache.get(k) { + Some(module) => module, + None => return Ok(None), }; - let instance = instance_pre.instantiate(&mut *store)?; + let instance = cache.linker.instantiate(&mut *store, module)?; Ok(Some(instance)) } /// Construct a new wasmtime "store" from the given kernel. pub fn new_store(&self, kernel: K) -> wasmtime::Store> { - wasmtime::Store::new(&self.0.engine, InvocationData::new(kernel)) + let id = InvocationData { + kernel, + last_error: None, + avail_gas_global: self.0.dummy_gas_global, + }; + + let mut store = wasmtime::Store::new(&self.0.engine, id); + let ggtype = GlobalType::new(ValType::I64, Mutability::Var); + let gg = Global::new(&mut store, ggtype, Val::I64(0)) + .expect("failed to create available_gas global"); + store.data_mut().avail_gas_global = gg; + + store } } diff --git a/fvm/src/machine/mod.rs b/fvm/src/machine/mod.rs index 23ee33c27..d89cc8f2b 100644 --- a/fvm/src/machine/mod.rs +++ b/fvm/src/machine/mod.rs @@ -20,7 +20,7 @@ pub use default::DefaultMachine; mod engine; -pub use engine::Engine; +pub use engine::{Engine, MultiEngine}; mod boxed; @@ -98,6 +98,10 @@ pub struct NetworkConfig { /// DEFAULT: 4096 pub max_call_depth: u32, + /// The maximum number of elements on wasm stack + /// DEFAULT: 64Ki (512KiB of u64 elements) + pub max_wasm_stack: u32, + /// An override for builtin-actors. If specified, this should be the CID of a builtin-actors /// "manifest". /// @@ -121,6 +125,7 @@ impl NetworkConfig { NetworkConfig { network_version, max_call_depth: 4096, + max_wasm_stack: 64 * 1024, actor_debugging: false, builtin_actors_override: None, price_list: price_list_by_network_version(network_version), diff --git a/fvm/src/syscalls/bind.rs b/fvm/src/syscalls/bind.rs index dfd641be8..32fb264d7 100644 --- a/fvm/src/syscalls/bind.rs +++ b/fvm/src/syscalls/bind.rs @@ -2,7 +2,7 @@ use std::mem; use fvm_shared::error::ErrorNumber; use fvm_shared::sys::SyscallSafe; -use wasmtime::{Caller, Linker, Trap, WasmTy}; +use wasmtime::{Caller, Linker, Trap, Val, WasmTy}; use super::context::Memory; use super::error::Abort; @@ -98,34 +98,39 @@ fn memory_and_data<'a, K: Kernel>( Ok((Memory::new(mem), data)) } -fn charge_exec_units_for_gas(caller: &mut Caller>) -> Result<(), Trap> { - let exec_units = caller +fn gastracker_to_wasmgas(caller: &mut Caller>) -> Result<(), Trap> { + let avail_milligas = caller .data_mut() - .calculate_exec_units_for_gas() - .map_err(|_| Trap::new("failed to calculate exec_units"))?; - if exec_units.is_negative() { - caller.add_fuel(u64::try_from(exec_units.saturating_neg()).unwrap_or(0))?; - } else { - caller.consume_fuel(u64::try_from(exec_units).unwrap_or(0))?; - } - - let gas_used = caller.data().kernel.gas_used(); - let fuel_consumed = caller - .fuel_consumed() - .ok_or_else(|| Trap::new("expected to find exec_units consumed"))?; - caller.data_mut().set_snapshots(gas_used, fuel_consumed); - Ok(()) + .kernel + .borrow_milligas() + .map_err(|_| Trap::new("borrowing available gas"))?; + + let gas_global = caller.data_mut().avail_gas_global; + gas_global + .set(caller, Val::I64(avail_milligas)) + .map_err(|_| Trap::new("failed to set available gas")) } -fn charge_gas_for_exec_units(caller: &mut Caller>) -> Result<(), Trap> { - let exec_units_consumed = caller - .fuel_consumed() - .ok_or_else(|| Trap::new("expected to find exec_units consumed"))?; +fn wasmgas_to_gastracker(caller: &mut Caller>) -> Result<(), Trap> { + let global = caller.data_mut().avail_gas_global; + let milligas = match global.get(&mut *caller) { + Val::I64(g) => Ok(g), + _ => Err(Trap::new("failed to get wasm gas")), + }?; + + // note: this should never error: + // * It can't return out-of-gas, because that would mean that we got + // negative available milligas returned from wasm - and wasm + // instrumentation will trap when it sees available gas go below zero + // * If it errors because gastracker thinks it already owns gas, something + // is really wrong caller .data_mut() - .charge_gas_for_exec_units(exec_units_consumed) - .map_err(|_| Trap::new("failed to charge gas for exec_units")) + .kernel + .return_milligas("wasm_exec", milligas) + .map_err(|e| Trap::new(format!("returning available gas: {}", e)))?; + Ok(()) } // Unfortunately, we can't implement this for _all_ functions. So we implement it for functions of up to 6 arguments. @@ -148,30 +153,35 @@ macro_rules! impl_bind_syscalls { if mem::size_of::() == 0 { // If we're returning a zero-sized "value", we return no value therefore and expect no out pointer. self.func_wrap(module, name, move |mut caller: Caller<'_, InvocationData> $(, $t: $t)*| { - charge_gas_for_exec_units(&mut caller)?; + wasmgas_to_gastracker(&mut caller)?; + let (mut memory, mut data) = memory_and_data(&mut caller)?; let ctx = Context{kernel: &mut data.kernel, memory: &mut memory}; - let result = match syscall(ctx $(, $t)*).into()? { - Ok(_) => { + let out = syscall(ctx $(, $t)*).into(); + + let result = match out { + Ok(Ok(_)) => { log::trace!("syscall {}::{}: ok", module, name); data.last_error = None; - 0 + Ok(0) }, - Err(err) => { + Ok(Err(err)) => { let code = err.1; log::trace!("syscall {}::{}: fail ({})", module, name, code as u32); data.last_error = Some(backtrace::Cause::new(module, name, err)); - code as u32 + Ok(code as u32) }, + Err(e) => Err(e.into()), }; - charge_exec_units_for_gas(&mut caller)?; - Ok(result) + gastracker_to_wasmgas(&mut caller)?; + + result }) } else { // If we're returning an actual value, we need to write it back into the wasm module's memory. self.func_wrap(module, name, move |mut caller: Caller<'_, InvocationData>, ret: u32 $(, $t: $t)*| { - charge_gas_for_exec_units(&mut caller)?; + wasmgas_to_gastracker(&mut caller)?; let (mut memory, mut data) = memory_and_data(&mut caller)?; // We need to check to make sure we can store the return value _before_ we do anything. @@ -183,23 +193,25 @@ macro_rules! impl_bind_syscalls { } let ctx = Context{kernel: &mut data.kernel, memory: &mut memory}; - let result = match syscall(ctx $(, $t)*).into()? { - Ok(value) => { + let result = match syscall(ctx $(, $t)*).into() { + Ok(Ok(value)) => { log::trace!("syscall {}::{}: ok", module, name); unsafe { *(memory.as_mut_ptr().offset(ret as isize) as *mut Ret::Value) = value }; data.last_error = None; - 0 + Ok(0) }, - Err(err) => { + Ok(Err(err)) => { let code = err.1; log::trace!("syscall {}::{}: fail ({})", module, name, code as u32); data.last_error = Some(backtrace::Cause::new(module, name, err)); - code as u32 + Ok(code as u32) }, + Err(e) => Err(e.into()), }; - charge_exec_units_for_gas(&mut caller)?; - Ok(result) + gastracker_to_wasmgas(&mut caller)?; + + result }) } } diff --git a/fvm/src/syscalls/mod.rs b/fvm/src/syscalls/mod.rs index dd599fb59..e5919f680 100644 --- a/fvm/src/syscalls/mod.rs +++ b/fvm/src/syscalls/mod.rs @@ -1,8 +1,7 @@ use cid::Cid; -use wasmtime::Linker; +use wasmtime::{Global, Linker}; use crate::call_manager::backtrace; -use crate::kernel::Result; use crate::Kernel; pub(crate) mod error; @@ -31,63 +30,8 @@ pub struct InvocationData { /// after receiving this error without calling any other syscalls. pub last_error: Option, - /// This snapshot is used to track changes in gas_used during syscall invocations. - /// The snapshot gets taken when execution exits WASM _after_ charging gas for any newly incurred fuel costs. - /// When execution moves back into WASM, we consume fuel for the delta between the snapshot and the new gas_used value. - pub gas_used_snapshot: i64, - - /// This snapshot is used to track changes in fuel_consumed during WASM execution. - /// The snapshot gets taken when execution enters WASM _after_ consuming fuel for any syscall gas consumption. - /// When execution exits WASM, we charge gas for the delta between the new fuel_consumed value and the snapshot. - pub exec_units_consumed_snapshot: u64, -} - -impl InvocationData { - pub(crate) fn new(kernel: K) -> Self { - let gas_used = kernel.gas_used(); - Self { - kernel, - last_error: None, - gas_used_snapshot: gas_used, - exec_units_consumed_snapshot: 0, - } - } - - /// This method: - /// 1) calculates the gas_used delta from the previous snapshot, - /// 2) converts this to the corresponding amount of exec_units. - /// 3) returns the value calculated in 2) for its caller to actually consume that exec_units_consumed - /// The caller should also update the snapshots after doing so. - pub(crate) fn calculate_exec_units_for_gas(&self) -> Result { - let gas_used = self.kernel.gas_used(); - let exec_units_to_consume = self - .kernel - .price_list() - .gas_to_exec_units(gas_used - self.gas_used_snapshot, true); - Ok(exec_units_to_consume) - } - - pub(crate) fn set_snapshots(&mut self, gas_used: i64, exec_units_consumed: u64) { - self.gas_used_snapshot = gas_used; - self.exec_units_consumed_snapshot = exec_units_consumed; - } - - /// This method: - /// 1) charges gas corresponding to the exec_units_consumed delta based on the previous snapshot - /// 2) updates the exec_units_consumed and gas_used snapshots - pub(crate) fn charge_gas_for_exec_units(&mut self, exec_units_consumed: u64) -> Result<()> { - self.kernel.charge_gas( - "exec_units", - self.kernel - .price_list() - .on_consume_exec_units( - exec_units_consumed.saturating_sub(self.exec_units_consumed_snapshot), - ) - .total(), - )?; - self.set_snapshots(self.kernel.gas_used(), exec_units_consumed); - Ok(()) - } + /// The global containing remaining available gas + pub avail_gas_global: Global, } use self::bind::BindSyscall; diff --git a/testing/conformance/benches/bench_conformance.rs b/testing/conformance/benches/bench_conformance.rs index d335c0a09..606dcffad 100644 --- a/testing/conformance/benches/bench_conformance.rs +++ b/testing/conformance/benches/bench_conformance.rs @@ -8,7 +8,7 @@ use std::time::Duration; use colored::Colorize; use criterion::*; -use fvm::machine::Engine; +use fvm::machine::MultiEngine; use fvm_conformance_tests::driver::*; use fvm_conformance_tests::report; use fvm_conformance_tests::vector::MessageVector; @@ -39,7 +39,7 @@ fn bench_conformance(c: &mut Criterion) { ), }; - let engine = Engine::default(); + let engines = MultiEngine::default(); // TODO: this is 30 seconds per benchmark... yeesh! once we get the setup running faster (by cloning VMs more efficiently), we can probably bring this down. let mut group = c.benchmark_group("conformance-tests"); @@ -74,7 +74,7 @@ fn bench_conformance(c: &mut Criterion) { &message_vector, CheckStrength::FullTest, &vector_path.display().to_string(), - &engine, + &engines, ) { Ok(()) => report!( "SUCCESSFULLY BENCHED TEST FILE".on_green(), diff --git a/testing/conformance/benches/bench_conformance_overhead.rs b/testing/conformance/benches/bench_conformance_overhead.rs index faa017ddc..ac7cc6543 100644 --- a/testing/conformance/benches/bench_conformance_overhead.rs +++ b/testing/conformance/benches/bench_conformance_overhead.rs @@ -4,7 +4,7 @@ use std::path::Path; use std::time::Duration; use criterion::*; -use fvm::machine::{Engine, BURNT_FUNDS_ACTOR_ADDR}; +use fvm::machine::{MultiEngine, BURNT_FUNDS_ACTOR_ADDR}; use fvm_conformance_tests::driver::*; use fvm_conformance_tests::vector::{ApplyMessage, MessageVector}; use fvm_ipld_encoding::{Cbor, RawBytes}; @@ -19,7 +19,7 @@ use crate::bench_drivers::{bench_vector_file, CheckStrength}; fn bench_init_only( group: &mut BenchmarkGroup, path_to_setup: &Path, - engine: &Engine, + engines: &MultiEngine, ) -> anyhow::Result<()> { // compute measurement overhead by benching running a single empty vector of zero messages let mut message_vector = MessageVector::from_file(path_to_setup)?; @@ -35,7 +35,7 @@ fn bench_init_only( &message_vector, CheckStrength::OnlyCheckSuccess, "bench_init_only", - engine, + engines, ) } @@ -43,7 +43,7 @@ fn bench_init_only( fn bench_500_simple_state_access( group: &mut BenchmarkGroup, path_to_setup: &Path, - engine: &Engine, + engines: &MultiEngine, ) -> anyhow::Result<()> { let five_hundred_state_accesses = (0..500) .map(|i| ApplyMessage { @@ -78,7 +78,7 @@ fn bench_500_simple_state_access( &message_vector, CheckStrength::OnlyCheckSuccess, "bench_500_simple_state_access", - engine, + engines, ) } /// runs overhead benchmarks, using the contents of the environment variable VECTOR as the starting FVM state @@ -101,9 +101,9 @@ fn bench_conformance_overhead(c: &mut Criterion) { group.measurement_time(Duration::new(30, 0)); // start by getting some baselines! - let engine = Engine::default(); - bench_init_only(&mut group, &path_to_setup, &engine).unwrap(); - bench_500_simple_state_access(&mut group, &path_to_setup, &engine).unwrap(); + let engines = MultiEngine::default(); + bench_init_only(&mut group, &path_to_setup, &engines).unwrap(); + bench_500_simple_state_access(&mut group, &path_to_setup, &engines).unwrap(); group.finish(); } diff --git a/testing/conformance/benches/bench_drivers.rs b/testing/conformance/benches/bench_drivers.rs index 8d8f8a978..4f7b9ea77 100644 --- a/testing/conformance/benches/bench_drivers.rs +++ b/testing/conformance/benches/bench_drivers.rs @@ -2,7 +2,7 @@ extern crate criterion; use criterion::*; use fvm::executor::{ApplyKind, DefaultExecutor, Executor}; -use fvm::machine::Engine; +use fvm::machine::MultiEngine; use fvm_conformance_tests::driver::*; use fvm_conformance_tests::vector::{MessageVector, Variant}; use fvm_conformance_tests::vm::{TestKernel, TestMachine}; @@ -37,7 +37,7 @@ pub fn bench_vector_variant( vector: &MessageVector, messages_with_lengths: Vec<(Message, usize)>, bs: &MemoryBlockstore, - engine: &Engine, + engines: &MultiEngine, ) { group.bench_function(name, move |b| { b.iter_batched( @@ -45,7 +45,7 @@ pub fn bench_vector_variant( let vector = &(*vector).clone(); let bs = bs.clone(); // TODO next few lines don't impact the benchmarks, but it might make them run waaaay more slowly... ought to make a base copy of the machine and exec and deepcopy them each time. - let machine = TestMachine::new_for_vector(vector, variant, bs, engine); + let machine = TestMachine::new_for_vector(vector, variant, bs, engines); // can assume this works because it passed a test before this ran let exec: DefaultExecutor = DefaultExecutor::new(machine); (messages_with_lengths.clone(), exec) @@ -80,7 +80,7 @@ pub fn bench_vector_file( vector: &MessageVector, check_strength: CheckStrength, name: &str, - engine: &Engine, + engines: &MultiEngine, ) -> anyhow::Result<()> { let (bs, _) = async_std::task::block_on(vector.seed_blockstore()).unwrap(); @@ -89,12 +89,12 @@ pub fn bench_vector_file( // this tests the variant before we run the benchmark and record the bench results to disk. // if we broke the test, it's not a valid optimization :P let testresult = match check_strength { - CheckStrength::FullTest => run_variant(bs.clone(), vector, variant, engine, true) + CheckStrength::FullTest => run_variant(bs.clone(), vector, variant, engines, true) .map_err(|e| { anyhow::anyhow!("run_variant failed (probably a test parsing bug): {}", e) })?, CheckStrength::OnlyCheckSuccess => { - run_variant(bs.clone(), vector, variant, engine, false).map_err(|e| { + run_variant(bs.clone(), vector, variant, engines, false).map_err(|e| { anyhow::anyhow!("run_variant failed (probably a test parsing bug): {}", e) })? } @@ -124,7 +124,7 @@ pub fn bench_vector_file( vector, messages_with_lengths, &bs, - engine, + engines, ); } else { return Err(anyhow::anyhow!("a test failed, get the tests passing/running before running benchmarks in {:?} mode: {}", check_strength, name)); diff --git a/testing/conformance/src/driver.rs b/testing/conformance/src/driver.rs index 545cdf404..435f16b17 100644 --- a/testing/conformance/src/driver.rs +++ b/testing/conformance/src/driver.rs @@ -5,7 +5,7 @@ use cid::Cid; use fmt::Display; use fvm::executor::{ApplyKind, ApplyRet, DefaultExecutor, Executor}; use fvm::kernel::Context; -use fvm::machine::{Engine, Machine}; +use fvm::machine::{Machine, MultiEngine}; use fvm::state_tree::{ActorState, StateTree}; use fvm_ipld_blockstore::MemoryBlockstore; use fvm_ipld_encoding::{Cbor, CborStore}; @@ -184,13 +184,13 @@ pub fn run_variant( bs: MemoryBlockstore, v: &MessageVector, variant: &Variant, - engine: &Engine, + engines: &MultiEngine, check_correctness: bool, ) -> anyhow::Result { let id = variant.id.clone(); // Construct the Machine. - let machine = TestMachine::new_for_vector(v, variant, bs, engine); + let machine = TestMachine::new_for_vector(v, variant, bs, engines); let mut exec: DefaultExecutor = DefaultExecutor::new(machine); // Apply all messages in the vector. diff --git a/testing/conformance/src/vm.rs b/testing/conformance/src/vm.rs index bf60d121f..640077a91 100644 --- a/testing/conformance/src/vm.rs +++ b/testing/conformance/src/vm.rs @@ -6,7 +6,7 @@ use futures::executor::block_on; use fvm::call_manager::{CallManager, DefaultCallManager, FinishRet, InvocationResult}; use fvm::gas::{GasTracker, PriceList}; use fvm::kernel::*; -use fvm::machine::{DefaultMachine, Engine, Machine, MachineContext, NetworkConfig}; +use fvm::machine::{DefaultMachine, Engine, Machine, MachineContext, MultiEngine, NetworkConfig}; use fvm::state_tree::{ActorState, StateTree}; use fvm::DefaultKernel; use fvm_ipld_blockstore::MemoryBlockstore; @@ -49,7 +49,7 @@ impl TestMachine>> { v: &MessageVector, variant: &Variant, blockstore: MemoryBlockstore, - engine: &Engine, + engines: &MultiEngine, ) -> TestMachine>> { let network_version = NetworkVersion::try_from(variant.nv).expect("unrecognized network version"); @@ -71,16 +71,14 @@ impl TestMachine>> { .get(&network_version) .expect("no builtin actors index for nv"); - let machine = DefaultMachine::new( - engine, - NetworkConfig::new(network_version) - .override_actors(builtin_actors) - .for_epoch(epoch, state_root) - .set_base_fee(base_fee), - blockstore, - externs, - ) - .unwrap(); + let mut nc = NetworkConfig::new(network_version); + nc.override_actors(builtin_actors); + let mut mc = nc.for_epoch(epoch, state_root); + mc.set_base_fee(base_fee); + + let engine = engines.get(&mc.network).expect("getting engine"); + + let machine = DefaultMachine::new(&engine, &mc, blockstore, externs).unwrap(); let price_list = machine.context().price_list.clone(); @@ -496,6 +494,14 @@ where self.0.charge_gas(name, compute) } + fn borrow_milligas(&mut self) -> Result { + self.0.borrow_milligas() + } + + fn return_milligas(&mut self, name: &str, newgas: i64) -> Result<()> { + self.0.return_milligas(name, newgas) + } + fn price_list(&self) -> &PriceList { self.0.price_list() } diff --git a/testing/conformance/tests/runner.rs b/testing/conformance/tests/runner.rs index 5eecbe1bd..1d4754326 100644 --- a/testing/conformance/tests/runner.rs +++ b/testing/conformance/tests/runner.rs @@ -12,7 +12,7 @@ use anyhow::{anyhow, Context as _}; use async_std::{stream, sync, task}; use colored::*; use futures::{Future, StreamExt, TryFutureExt, TryStreamExt}; -use fvm::machine::Engine; +use fvm::machine::MultiEngine; use fvm_conformance_tests::driver::*; use fvm_conformance_tests::report; use fvm_conformance_tests::vector::{MessageVector, Selector}; @@ -33,13 +33,13 @@ lazy_static! { async fn conformance_test_runner() -> anyhow::Result<()> { pretty_env_logger::init(); - let engine = Engine::default(); + let engines = MultiEngine::new(); let vector_results = match var("VECTOR") { Ok(v) => either::Either::Left( iter::once(async move { let path = Path::new(v.as_str()).to_path_buf(); - let res = run_vector(path.clone(), engine) + let res = run_vector(path.clone(), engines) .await .with_context(|| format!("failed to run vector: {}", path.display()))?; anyhow::Ok((path, res)) @@ -51,10 +51,10 @@ async fn conformance_test_runner() -> anyhow::Result<()> { .into_iter() .filter_ok(is_runnable) .map(|e| { - let engine = engine.clone(); + let engines = engines.clone(); async move { let path = e?.path().to_path_buf(); - let res = run_vector(path.clone(), engine) + let res = run_vector(path.clone(), engines) .await .with_context(|| format!("failed to run vector: {}", path.display()))?; Ok((path, res)) @@ -128,7 +128,7 @@ async fn conformance_test_runner() -> anyhow::Result<()> { /// one per variant. async fn run_vector( path: PathBuf, - engine: Engine, + engines: MultiEngine, ) -> anyhow::Result>>> { let file = File::open(&path)?; let reader = BufReader::new(file); @@ -199,14 +199,20 @@ async fn run_vector( (0..v.preconditions.variants.len()).map(move |i| { let v = v.clone(); let bs = bs.clone(); - let engine = engine.clone(); + let engines = engines.clone(); let name = format!("{} | {}", path.display(), &v.preconditions.variants[i].id); futures::future::Either::Right( task::Builder::new() .name(name) .spawn(async move { - run_variant(bs, &v, &v.preconditions.variants[i], &engine, true) + run_variant( + bs, + &v, + &v.preconditions.variants[i], + &engines, + true, + ) }) .unwrap(), ) diff --git a/testing/integration/src/builtin.rs b/testing/integration/src/builtin.rs index 61f8c4df5..9e2ae32df 100644 --- a/testing/integration/src/builtin.rs +++ b/testing/integration/src/builtin.rs @@ -16,9 +16,10 @@ use crate::error::Error::{ FailedToLoadManifest, FailedToSetActor, FailedToSetState, MultipleRootCid, NoCidInManifest, }; -const BUNDLES: [(NetworkVersion, &[u8]); 2] = [ +const BUNDLES: [(NetworkVersion, &[u8]); 3] = [ (NetworkVersion::V14, actors_v6::BUNDLE_CAR), (NetworkVersion::V15, actors_v7::BUNDLE_CAR), + (NetworkVersion::V16, actors_v7::BUNDLE_CAR), // todo bad hack ]; // Import built-in actors diff --git a/testing/integration/src/tester.rs b/testing/integration/src/tester.rs index 92cc9b591..30918a116 100644 --- a/testing/integration/src/tester.rs +++ b/testing/integration/src/tester.rs @@ -152,12 +152,15 @@ impl Tester { let blockstore = self.state_tree.store(); + let mut nc = NetworkConfig::new(self.nv); + nc.override_actors(self.builtin_actors); + + let mut mc = nc.for_epoch(0, state_root); + mc.set_base_fee(TokenAmount::from(DEFAULT_BASE_FEE)); + let machine = DefaultMachine::new( - &Engine::default(), - NetworkConfig::new(self.nv) - .override_actors(self.builtin_actors) - .for_epoch(0, state_root) - .set_base_fee(TokenAmount::from(DEFAULT_BASE_FEE)), + &Engine::new_default((&mc.network.clone()).into())?, + &mc, blockstore.clone(), dummy::DummyExterns, )?; diff --git a/testing/integration/tests/lib.rs b/testing/integration/tests/lib.rs index 73884a507..00f45a2c0 100644 --- a/testing/integration/tests/lib.rs +++ b/testing/integration/tests/lib.rs @@ -5,10 +5,12 @@ use fvm_integration_tests::tester::{Account, Tester}; use fvm_ipld_encoding::tuple::*; use fvm_shared::address::Address; use fvm_shared::bigint::BigInt; +use fvm_shared::error::ExitCode; use fvm_shared::message::Message; use fvm_shared::state::StateTreeVersion; use fvm_shared::version::NetworkVersion; use num_traits::Zero; +use wabt::wat2wasm; /// The state object. #[derive(Serialize_tuple, Deserialize_tuple, Clone, Debug, Default)] @@ -65,3 +67,115 @@ fn hello_world() { assert_eq!(res.msg_receipt.exit_code.value(), 16) } + +#[test] +fn out_of_gas() { + const WAT: &str = r#" + ;; Mock invoke function + (module + (func (export "invoke") (param $x i32) (result i32) + (loop + (br 0) + ) + (i32.const 1) + ) + ) + "#; + + // Instantiate tester + let mut tester = Tester::new(NetworkVersion::V16, StateTreeVersion::V4).unwrap(); + + let sender: [Account; 1] = tester.create_accounts().unwrap(); + + // Get wasm bin + let wasm_bin = wat2wasm(WAT).unwrap(); + + // Set actor state + let actor_state = State { count: 0 }; + let state_cid = tester.set_state(&actor_state).unwrap(); + + // Set actor + let actor_address = Address::new_id(10000); + + tester + .set_actor_from_bin(&wasm_bin, state_cid, actor_address, BigInt::zero()) + .unwrap(); + + // Instantiate machine + tester.instantiate_machine().unwrap(); + + // Send message + let message = Message { + from: sender[0].1, + to: actor_address, + gas_limit: 10_000_000, + method_num: 1, + ..Message::default() + }; + + let res = tester + .executor + .unwrap() + .execute_message(message, ApplyKind::Explicit, 100) + .unwrap(); + + assert_eq!(res.msg_receipt.exit_code, ExitCode::SYS_OUT_OF_GAS) +} + +#[test] +fn out_of_stack() { + const WAT: &str = r#" + ;; Mock invoke function + (module + (func (export "invoke") (param $x i32) (result i32) + (i64.const 123) + (call 1) + (drop) + (i32.const 0) + ) + (func (param $x i64) (result i64) + (local.get 0) + (call 1) + ) + ) + "#; + + // Instantiate tester + let mut tester = Tester::new(NetworkVersion::V16, StateTreeVersion::V4).unwrap(); + + let sender: [Account; 1] = tester.create_accounts().unwrap(); + + // Get wasm bin + let wasm_bin = wat2wasm(WAT).unwrap(); + + // Set actor state + let actor_state = State { count: 0 }; + let state_cid = tester.set_state(&actor_state).unwrap(); + + // Set actor + let actor_address = Address::new_id(10000); + + tester + .set_actor_from_bin(&wasm_bin, state_cid, actor_address, BigInt::zero()) + .unwrap(); + + // Instantiate machine + tester.instantiate_machine().unwrap(); + + // Send message + let message = Message { + from: sender[0].1, + to: actor_address, + gas_limit: 10_000_000, + method_num: 1, + ..Message::default() + }; + + let res = tester + .executor + .unwrap() + .execute_message(message, ApplyKind::Explicit, 100) + .unwrap(); + + assert_eq!(res.msg_receipt.exit_code, ExitCode::SYS_ILLEGAL_INSTRUCTION) +} From 090f838443623fd76124e16f49dfca90bdad77b9 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Wed, 4 May 2022 18:39:36 -0400 Subject: [PATCH 04/21] feat: refactor gas charging to remove "borrowing" (#527) --- fvm/src/call_manager/default.rs | 48 ++++------------------- fvm/src/gas/mod.rs | 69 +++++++++------------------------ fvm/src/kernel/default.rs | 22 +++++++---- fvm/src/kernel/mod.rs | 10 ++--- fvm/src/machine/engine.rs | 1 + fvm/src/syscalls/bind.rs | 48 ++++------------------- fvm/src/syscalls/mod.rs | 59 +++++++++++++++++++++++++++- testing/conformance/src/vm.rs | 20 +++++++--- 8 files changed, 125 insertions(+), 152 deletions(-) diff --git a/fvm/src/call_manager/default.rs b/fvm/src/call_manager/default.rs index 838720cf8..5546d84ed 100644 --- a/fvm/src/call_manager/default.rs +++ b/fvm/src/call_manager/default.rs @@ -7,7 +7,6 @@ use fvm_shared::econ::TokenAmount; use fvm_shared::error::{ErrorNumber, ExitCode}; use fvm_shared::{ActorID, MethodNum, METHOD_SEND}; use num_traits::Zero; -use wasmtime::Val; use super::{Backtrace, CallManager, InvocationResult, NO_DATA_BLOCK_ID}; use crate::call_manager::backtrace::Frame; @@ -16,6 +15,7 @@ use crate::gas::GasTracker; use crate::kernel::{ClassifyResult, ExecutionError, Kernel, Result, SyscallError}; use crate::machine::Machine; use crate::syscalls::error::Abort; +use crate::syscalls::{charge_for_exec, update_gas_available}; use crate::trace::{ExecutionEvent, ExecutionTrace, SendParams}; use crate::{account_actor, syscall_error}; @@ -345,55 +345,21 @@ where // From this point on, there are no more syscall errors, only aborts. let result: std::result::Result = (|| { - let initial_milligas = - store - .data_mut() - .kernel - .borrow_milligas() - .map_err(|e| match e { - ExecutionError::OutOfGas => Abort::OutOfGas, - ExecutionError::Fatal(m) => Abort::Fatal(m), - _ => Abort::Fatal(anyhow::Error::msg( - "setting available gas on entering wasm", - )), - })?; - - store - .data() - .avail_gas_global - .clone() - .set(&mut store, Val::I64(initial_milligas)) - .map_err(Abort::Fatal)?; - // Lookup the invoke method. let invoke: wasmtime::TypedFunc<(u32,), u32> = instance .get_typed_func(&mut store, "invoke") // All actors will have an invoke method. .map_err(Abort::Fatal)?; + // Set the available gas. + update_gas_available(&mut store)?; + // Invoke it. let res = invoke.call(&mut store, (param_id,)); - // Return gas tracking to GasTracker - let available_milligas = - match store.data_mut().avail_gas_global.clone().get(&mut store) { - Val::I64(g) => Ok(g), - _ => Err(Abort::Fatal(anyhow::Error::msg( - "failed to get available gas from wasm after returning", - ))), - }?; - - store - .data_mut() - .kernel - .return_milligas("wasm_exec_last", available_milligas) - .map_err(|e| match e { - ExecutionError::OutOfGas => Abort::OutOfGas, - ExecutionError::Fatal(m) => Abort::Fatal(m), - _ => Abort::Fatal(anyhow::Error::msg( - "setting available gas on existing wasm", - )), - })?; + // Charge for any remaining uncharged execution gas, returning an error if we run + // out. + charge_for_exec(&mut store)?; // If the invocation failed due to running out of exec_units, we have already detected it and returned OutOfGas above. // Any other invocation failure is returned here as an Abort diff --git a/fvm/src/gas/mod.rs b/fvm/src/gas/mod.rs index dc462b8cf..ec250ce51 100644 --- a/fvm/src/gas/mod.rs +++ b/fvm/src/gas/mod.rs @@ -15,13 +15,6 @@ pub const MILLIGAS_PRECISION: i64 = 1000; pub struct GasTracker { milligas_limit: i64, milligas_used: i64, - - /// A flag indicating whether this GasTracker is currently responsible for - /// gas accounting. A 'false' value indicates that gas accounting is - /// handled somewhere else (eg. in wasm execution). - /// - /// Creating gas charges is only allowed when own_limit is true. - own_limit: bool, } impl GasTracker { @@ -29,19 +22,12 @@ impl GasTracker { Self { milligas_limit: gas_to_milligas(gas_limit), milligas_used: gas_to_milligas(gas_used), - own_limit: true, } } /// Safely consumes gas and returns an out of gas error if there is not sufficient /// enough gas remaining for charge. - fn charge_milligas(&mut self, name: &str, to_use: i64) -> Result<()> { - if !self.own_limit { - return Err(ExecutionError::Fatal(anyhow::Error::msg( - "charge_gas called when gas_limit owned by execution", - ))); - } - + pub fn charge_milligas(&mut self, name: &str, to_use: i64) -> Result<()> { match self.milligas_used.checked_add(to_use) { None => { log::trace!("gas overflow: {}", name); @@ -69,50 +55,33 @@ impl GasTracker { ) } - /// returns available milligas; makes the gas tracker reject gas charges with - /// a fatal error until return_milligas is called. - pub fn borrow_milligas(&mut self) -> Result { - if !self.own_limit { - return Err(ExecutionError::Fatal(anyhow::Error::msg( - "borrow_milligas called on GasTracker which doesn't own gas limit", - ))); - } - self.own_limit = false; - - Ok(self.milligas_limit - self.milligas_used) - } - - /// sets new available gas, creating a new gas charge if needed - pub fn return_milligas(&mut self, name: &str, new_avail_mgas: i64) -> Result<()> { - if self.own_limit { - return Err(ExecutionError::Fatal(anyhow::Error::msg(format!( - "gastracker already owns gas_limit, charge: {}", - name - )))); - } - self.own_limit = true; - - let old_avail_milligas = self.milligas_limit - self.milligas_used; - let used = old_avail_milligas - new_avail_mgas; - - if used < 0 { - return Err(ExecutionError::Fatal(anyhow::Error::msg( - "negative gas charge in set_available_gas", - ))); - } - - self.charge_milligas(name, used) - } - /// Getter for gas available. pub fn gas_limit(&self) -> i64 { milligas_to_gas(self.milligas_limit, false) } + /// Getter for milligas available. + pub fn milligas_limit(&self) -> i64 { + self.milligas_limit + } + /// Getter for gas used. pub fn gas_used(&self) -> i64 { milligas_to_gas(self.milligas_used, true) } + + /// Getter for milligas used. + pub fn milligas_used(&self) -> i64 { + self.milligas_used + } + + pub fn gas_available(&self) -> i64 { + milligas_to_gas(self.milligas_available(), false) + } + + pub fn milligas_available(&self) -> i64 { + self.milligas_limit.saturating_sub(self.milligas_used) + } } /// Converts the specified gas into equivalent fractional gas units diff --git a/fvm/src/kernel/default.rs b/fvm/src/kernel/default.rs index c95761ce5..3a043f00b 100644 --- a/fvm/src/kernel/default.rs +++ b/fvm/src/kernel/default.rs @@ -755,19 +755,27 @@ where self.call_manager.gas_tracker().gas_used() } + fn milligas_used(&self) -> i64 { + self.call_manager.gas_tracker().milligas_used() + } + + fn gas_available(&self) -> i64 { + self.call_manager.gas_tracker().gas_available() + } + fn milligas_available(&self) -> i64 { + self.call_manager.gas_tracker().milligas_available() + } + fn charge_gas(&mut self, name: &str, compute: i64) -> Result<()> { let charge = GasCharge::new(name, compute, 0); - self.call_manager.charge_gas(charge) + self.call_manager.gas_tracker_mut().charge_gas(charge) } - fn return_milligas(&mut self, name: &str, new_gas: i64) -> Result<()> { + fn charge_milligas(&mut self, name: &str, compute: i64) -> Result<()> { self.call_manager .gas_tracker_mut() - .return_milligas(name, new_gas) - } - - fn borrow_milligas(&mut self) -> Result { - self.call_manager.gas_tracker_mut().borrow_milligas() + .charge_milligas(name, compute)?; + Ok(()) } fn price_list(&self) -> &PriceList { diff --git a/fvm/src/kernel/mod.rs b/fvm/src/kernel/mod.rs index a9953a119..5eee6911a 100644 --- a/fvm/src/kernel/mod.rs +++ b/fvm/src/kernel/mod.rs @@ -216,16 +216,16 @@ pub trait CircSupplyOps { pub trait GasOps { /// GasUsed return the gas used by the transaction so far. fn gas_used(&self) -> i64; + fn milligas_used(&self) -> i64; + + fn gas_available(&self) -> i64; + fn milligas_available(&self) -> i64; /// ChargeGas charges specified amount of `gas` for execution. /// `name` provides information about gas charging point fn charge_gas(&mut self, name: &str, compute: i64) -> Result<()>; - /// Returns available gas. - fn borrow_milligas(&mut self) -> Result; - - /// Sets available gas to a new value, creating a gas charge if needed - fn return_milligas(&mut self, name: &str, newgas: i64) -> Result<()>; + fn charge_milligas(&mut self, name: &str, compute: i64) -> Result<()>; fn price_list(&self) -> &PriceList; } diff --git a/fvm/src/machine/engine.rs b/fvm/src/machine/engine.rs index 567da8e0b..2137af1f6 100644 --- a/fvm/src/machine/engine.rs +++ b/fvm/src/machine/engine.rs @@ -308,6 +308,7 @@ impl Engine { kernel, last_error: None, avail_gas_global: self.0.dummy_gas_global, + last_milligas_available: 0, }; let mut store = wasmtime::Store::new(&self.0.engine, id); diff --git a/fvm/src/syscalls/bind.rs b/fvm/src/syscalls/bind.rs index 32fb264d7..7d7cdb737 100644 --- a/fvm/src/syscalls/bind.rs +++ b/fvm/src/syscalls/bind.rs @@ -2,11 +2,11 @@ use std::mem; use fvm_shared::error::ErrorNumber; use fvm_shared::sys::SyscallSafe; -use wasmtime::{Caller, Linker, Trap, Val, WasmTy}; +use wasmtime::{Caller, Linker, Trap, WasmTy}; use super::context::Memory; use super::error::Abort; -use super::{Context, InvocationData}; +use super::{charge_for_exec, update_gas_available, Context, InvocationData}; use crate::call_manager::backtrace; use crate::kernel::{self, ExecutionError, Kernel, SyscallError}; @@ -98,41 +98,6 @@ fn memory_and_data<'a, K: Kernel>( Ok((Memory::new(mem), data)) } -fn gastracker_to_wasmgas(caller: &mut Caller>) -> Result<(), Trap> { - let avail_milligas = caller - .data_mut() - .kernel - .borrow_milligas() - .map_err(|_| Trap::new("borrowing available gas"))?; - - let gas_global = caller.data_mut().avail_gas_global; - gas_global - .set(caller, Val::I64(avail_milligas)) - .map_err(|_| Trap::new("failed to set available gas")) -} - -fn wasmgas_to_gastracker(caller: &mut Caller>) -> Result<(), Trap> { - let global = caller.data_mut().avail_gas_global; - - let milligas = match global.get(&mut *caller) { - Val::I64(g) => Ok(g), - _ => Err(Trap::new("failed to get wasm gas")), - }?; - - // note: this should never error: - // * It can't return out-of-gas, because that would mean that we got - // negative available milligas returned from wasm - and wasm - // instrumentation will trap when it sees available gas go below zero - // * If it errors because gastracker thinks it already owns gas, something - // is really wrong - caller - .data_mut() - .kernel - .return_milligas("wasm_exec", milligas) - .map_err(|e| Trap::new(format!("returning available gas: {}", e)))?; - Ok(()) -} - // Unfortunately, we can't implement this for _all_ functions. So we implement it for functions of up to 6 arguments. macro_rules! impl_bind_syscalls { ($($t:ident)*) => { @@ -153,7 +118,7 @@ macro_rules! impl_bind_syscalls { if mem::size_of::() == 0 { // If we're returning a zero-sized "value", we return no value therefore and expect no out pointer. self.func_wrap(module, name, move |mut caller: Caller<'_, InvocationData> $(, $t: $t)*| { - wasmgas_to_gastracker(&mut caller)?; + charge_for_exec(&mut caller)?; let (mut memory, mut data) = memory_and_data(&mut caller)?; let ctx = Context{kernel: &mut data.kernel, memory: &mut memory}; @@ -174,14 +139,15 @@ macro_rules! impl_bind_syscalls { Err(e) => Err(e.into()), }; - gastracker_to_wasmgas(&mut caller)?; + update_gas_available(&mut caller)?; result }) } else { // If we're returning an actual value, we need to write it back into the wasm module's memory. self.func_wrap(module, name, move |mut caller: Caller<'_, InvocationData>, ret: u32 $(, $t: $t)*| { - wasmgas_to_gastracker(&mut caller)?; + charge_for_exec(&mut caller)?; + let (mut memory, mut data) = memory_and_data(&mut caller)?; // We need to check to make sure we can store the return value _before_ we do anything. @@ -209,7 +175,7 @@ macro_rules! impl_bind_syscalls { Err(e) => Err(e.into()), }; - gastracker_to_wasmgas(&mut caller)?; + update_gas_available(&mut caller)?; result }) diff --git a/fvm/src/syscalls/mod.rs b/fvm/src/syscalls/mod.rs index e5919f680..ec902d59f 100644 --- a/fvm/src/syscalls/mod.rs +++ b/fvm/src/syscalls/mod.rs @@ -1,7 +1,11 @@ +use std::mem; + +use anyhow::{anyhow, Context as _}; use cid::Cid; -use wasmtime::{Global, Linker}; +use wasmtime::{AsContextMut, Global, Linker, Val}; use crate::call_manager::backtrace; +use crate::kernel::ExecutionError; use crate::Kernel; pub(crate) mod error; @@ -30,11 +34,62 @@ pub struct InvocationData { /// after receiving this error without calling any other syscalls. pub last_error: Option, - /// The global containing remaining available gas + /// The global containing remaining available gas. pub avail_gas_global: Global, + /// The last-set milligas limit. When `charge_for_exec` is called, we charge for the + /// _difference_ between the current gas available (the wasm global) and the + /// `last_milligas_available`. + pub last_milligas_available: i64, +} + +pub fn update_gas_available( + ctx: &mut impl AsContextMut>, +) -> Result<(), Abort> { + let mut ctx = ctx.as_context_mut(); + let avail_milligas = ctx.data_mut().kernel.milligas_available(); + + let gas_global = ctx.data_mut().avail_gas_global; + gas_global + .set(&mut ctx, Val::I64(avail_milligas)) + .map_err(|e| Abort::Fatal(anyhow!("failed to set available gas global: {}", e)))?; + + ctx.data_mut().last_milligas_available = avail_milligas; + Ok(()) +} + +pub fn charge_for_exec( + ctx: &mut impl AsContextMut>, +) -> Result<(), Abort> { + let mut ctx = ctx.as_context_mut(); + let global = ctx.data_mut().avail_gas_global; + + let milligas_available = global + .get(&mut ctx) + .i64() + .context("failed to get wasm gas") + .map_err(Abort::Fatal)?; + + // Determine milligas used, and update the "o + let milligas_used = { + let data = ctx.data_mut(); + let last_milligas = mem::replace(&mut data.last_milligas_available, milligas_available); + // This should never be negative, but we might as well check. + last_milligas.saturating_sub(milligas_available) + }; + + ctx.data_mut() + .kernel + .charge_milligas("wasm_exec", milligas_used) + .map_err(|e| match e { + ExecutionError::OutOfGas => Abort::OutOfGas, + ExecutionError::Fatal(e) => Abort::Fatal(e), + ExecutionError::Syscall(e) => Abort::Fatal(anyhow!("unexpected syscall error: {}", e)), + })?; + Ok(()) } use self::bind::BindSyscall; +use self::error::Abort; /// The maximum supported CID size. (SPEC_AUDIT) pub const MAX_CID_LEN: usize = 100; diff --git a/testing/conformance/src/vm.rs b/testing/conformance/src/vm.rs index 640077a91..9d6471f06 100644 --- a/testing/conformance/src/vm.rs +++ b/testing/conformance/src/vm.rs @@ -494,16 +494,24 @@ where self.0.charge_gas(name, compute) } - fn borrow_milligas(&mut self) -> Result { - self.0.borrow_milligas() + fn price_list(&self) -> &PriceList { + self.0.price_list() } - fn return_milligas(&mut self, name: &str, newgas: i64) -> Result<()> { - self.0.return_milligas(name, newgas) + fn milligas_used(&self) -> i64 { + self.0.milligas_used() } - fn price_list(&self) -> &PriceList { - self.0.price_list() + fn gas_available(&self) -> i64 { + self.0.gas_available() + } + + fn milligas_available(&self) -> i64 { + self.0.milligas_available() + } + + fn charge_milligas(&mut self, name: &str, compute: i64) -> Result<()> { + self.0.charge_milligas(name, compute) } } From b4c0aa2f3a7a0efa0d9a6c0e662c95920c5814e1 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Wed, 4 May 2022 21:07:10 -0400 Subject: [PATCH 05/21] feat: resolve the memory once (#529) Instead of looking up the memory export on every syscall, resolve it when we construct the invocation container. fixes #510 --- fvm/src/call_manager/default.rs | 25 ++++++++++++++----------- fvm/src/machine/engine.rs | 16 +++++++++++++--- fvm/src/syscalls/bind.rs | 17 +++++++---------- fvm/src/syscalls/mod.rs | 7 ++++++- testing/integration/tests/lib.rs | 2 ++ 5 files changed, 42 insertions(+), 25 deletions(-) diff --git a/fvm/src/call_manager/default.rs b/fvm/src/call_manager/default.rs index 5546d84ed..793ce1f01 100644 --- a/fvm/src/call_manager/default.rs +++ b/fvm/src/call_manager/default.rs @@ -12,7 +12,7 @@ use super::{Backtrace, CallManager, InvocationResult, NO_DATA_BLOCK_ID}; use crate::call_manager::backtrace::Frame; use crate::call_manager::FinishRet; use crate::gas::GasTracker; -use crate::kernel::{ClassifyResult, ExecutionError, Kernel, Result, SyscallError}; +use crate::kernel::{ExecutionError, Kernel, Result, SyscallError}; use crate::machine::Machine; use crate::syscalls::error::Abort; use crate::syscalls::{charge_for_exec, update_gas_available}; @@ -333,18 +333,21 @@ where // Make a store. let mut store = engine.new_store(kernel); - // Instantiate the module. - let instance = match engine - .get_instance(&mut store, &state.code) - .and_then(|i| i.context("actor code not found")) - .or_fatal() - { - Ok(ret) => ret, - Err(err) => return (Err(err), store.into_data().kernel.into_call_manager()), - }; - // From this point on, there are no more syscall errors, only aborts. let result: std::result::Result = (|| { + // Instantiate the module. + let instance = engine + .get_instance(&mut store, &state.code) + .and_then(|i| i.context("actor code not found")) + .map_err(Abort::Fatal)?; + + // Resolve and store a reference to the exported memory. + let memory = instance + .get_memory(&mut store, "memory") + .context("actor has no memory export") + .map_err(Abort::Fatal)?; + store.data_mut().memory = memory; + // Lookup the invoke method. let invoke: wasmtime::TypedFunc<(u32,), u32> = instance .get_typed_func(&mut store, "invoke") diff --git a/fvm/src/machine/engine.rs b/fvm/src/machine/engine.rs index 2137af1f6..a405effe9 100644 --- a/fvm/src/machine/engine.rs +++ b/fvm/src/machine/engine.rs @@ -7,7 +7,7 @@ use anyhow::{anyhow, Context}; use cid::Cid; use fvm_ipld_blockstore::Blockstore; use fvm_wasm_instrument::gas_metering::GAS_COUNTER_NAME; -use wasmtime::{Global, GlobalType, Linker, Module, Mutability, Val, ValType}; +use wasmtime::{Global, GlobalType, Linker, Memory, MemoryType, Module, Mutability, Val, ValType}; use crate::gas::WasmGasPrices; use crate::machine::NetworkConfig; @@ -119,9 +119,13 @@ pub fn default_wasmtime_config() -> wasmtime::Config { struct EngineInner { engine: wasmtime::Engine, - /// dummy gas global used in store costructor to avoid making - /// InvocationData.avail_gas_global an Option + /// These two fields are used used in the store constructor to avoid resolve a chicken & egg + /// situation: We need the store before we can get the real values, but we need to create the + /// `InvocationData` before we can make the store. + /// + /// Alternatively, we could use `Option`s. But then we need to unwrap everywhere. dummy_gas_global: Global, + dummy_memory: Memory, module_cache: Mutex>, instance_cache: Mutex>, @@ -150,8 +154,12 @@ impl Engine { let dummy_gg = Global::new(&mut dummy_store, gg_type, Val::I64(0)) .expect("failed to create dummy gas global"); + let dummy_memory = Memory::new(&mut dummy_store, MemoryType::new(0, Some(0))) + .expect("failed to create dummy memory"); + Ok(Engine(Arc::new(EngineInner { engine, + dummy_memory, dummy_gas_global: dummy_gg, module_cache: Default::default(), instance_cache: Mutex::new(anymap::Map::new()), @@ -299,6 +307,7 @@ impl Engine { None => return Ok(None), }; let instance = cache.linker.instantiate(&mut *store, module)?; + Ok(Some(instance)) } @@ -309,6 +318,7 @@ impl Engine { last_error: None, avail_gas_global: self.0.dummy_gas_global, last_milligas_available: 0, + memory: self.0.dummy_memory, }; let mut store = wasmtime::Store::new(&self.0.engine, id); diff --git a/fvm/src/syscalls/bind.rs b/fvm/src/syscalls/bind.rs index 7d7cdb737..2afcb48dd 100644 --- a/fvm/src/syscalls/bind.rs +++ b/fvm/src/syscalls/bind.rs @@ -2,7 +2,7 @@ use std::mem; use fvm_shared::error::ErrorNumber; use fvm_shared::sys::SyscallSafe; -use wasmtime::{Caller, Linker, Trap, WasmTy}; +use wasmtime::{Caller, Linker, WasmTy}; use super::context::Memory; use super::error::Abort; @@ -89,13 +89,10 @@ where fn memory_and_data<'a, K: Kernel>( caller: &'a mut Caller<'_, InvocationData>, -) -> Result<(&'a mut Memory, &'a mut InvocationData), Trap> { - let (mem, data) = caller - .get_export("memory") - .and_then(|m| m.into_memory()) - .ok_or_else(|| Trap::new("failed to lookup actor memory"))? - .data_and_store_mut(caller); - Ok((Memory::new(mem), data)) +) -> (&'a mut Memory, &'a mut InvocationData) { + let memory_handle = caller.data().memory; + let (mem, data) = memory_handle.data_and_store_mut(caller); + (Memory::new(mem), data) } // Unfortunately, we can't implement this for _all_ functions. So we implement it for functions of up to 6 arguments. @@ -120,7 +117,7 @@ macro_rules! impl_bind_syscalls { self.func_wrap(module, name, move |mut caller: Caller<'_, InvocationData> $(, $t: $t)*| { charge_for_exec(&mut caller)?; - let (mut memory, mut data) = memory_and_data(&mut caller)?; + let (mut memory, mut data) = memory_and_data(&mut caller); let ctx = Context{kernel: &mut data.kernel, memory: &mut memory}; let out = syscall(ctx $(, $t)*).into(); @@ -148,7 +145,7 @@ macro_rules! impl_bind_syscalls { self.func_wrap(module, name, move |mut caller: Caller<'_, InvocationData>, ret: u32 $(, $t: $t)*| { charge_for_exec(&mut caller)?; - let (mut memory, mut data) = memory_and_data(&mut caller)?; + let (mut memory, mut data) = memory_and_data(&mut caller); // We need to check to make sure we can store the return value _before_ we do anything. if (ret as u64) > (memory.len() as u64) diff --git a/fvm/src/syscalls/mod.rs b/fvm/src/syscalls/mod.rs index ec902d59f..a368abe9b 100644 --- a/fvm/src/syscalls/mod.rs +++ b/fvm/src/syscalls/mod.rs @@ -2,7 +2,7 @@ use std::mem; use anyhow::{anyhow, Context as _}; use cid::Cid; -use wasmtime::{AsContextMut, Global, Linker, Val}; +use wasmtime::{AsContextMut, Global, Linker, Memory, Val}; use crate::call_manager::backtrace; use crate::kernel::ExecutionError; @@ -30,16 +30,21 @@ pub(self) use context::Context; pub struct InvocationData { /// The kernel on which this actor is being executed. pub kernel: K, + /// The last-seen syscall error. This error is considered the abort "cause" if an actor aborts /// after receiving this error without calling any other syscalls. pub last_error: Option, /// The global containing remaining available gas. pub avail_gas_global: Global, + /// The last-set milligas limit. When `charge_for_exec` is called, we charge for the /// _difference_ between the current gas available (the wasm global) and the /// `last_milligas_available`. pub last_milligas_available: i64, + + /// The invocation's imported "memory". + pub memory: Memory, } pub fn update_gas_available( diff --git a/testing/integration/tests/lib.rs b/testing/integration/tests/lib.rs index 00f45a2c0..328e90d5c 100644 --- a/testing/integration/tests/lib.rs +++ b/testing/integration/tests/lib.rs @@ -73,6 +73,7 @@ fn out_of_gas() { const WAT: &str = r#" ;; Mock invoke function (module + (memory (export "memory") 1) (func (export "invoke") (param $x i32) (result i32) (loop (br 0) @@ -127,6 +128,7 @@ fn out_of_stack() { const WAT: &str = r#" ;; Mock invoke function (module + (memory (export "memory") 1) (func (export "invoke") (param $x i32) (result i32) (i64.const 123) (call 1) From a1c87aea6745b2ffaabbfaea6dd138b9daeb05f2 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Thu, 5 May 2022 15:46:04 +0200 Subject: [PATCH 06/21] fix: remove unused chrono dep --- shared/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/shared/Cargo.toml b/shared/Cargo.toml index 6c8b5f4c3..122a76ffe 100644 --- a/shared/Cargo.toml +++ b/shared/Cargo.toml @@ -8,7 +8,6 @@ authors = ["ChainSafe Systems ", "Protocol Labs", "Filecoin C repository = "https://github.com/filecoin-project/ref-fvm" [dependencies] -chrono = "0.4.9" blake2b_simd = "1.0.0" thiserror = "1.0" num-traits = "0.2" From 297a7694ff9a584880632dde8826d9134d976fd9 Mon Sep 17 00:00:00 2001 From: raulk Date: Thu, 5 May 2022 16:54:38 +0100 Subject: [PATCH 07/21] update fvm changelog. --- fvm/CHANGELOG.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/fvm/CHANGELOG.md b/fvm/CHANGELOG.md index da3dd2ac1..6bf375590 100644 --- a/fvm/CHANGELOG.md +++ b/fvm/CHANGELOG.md @@ -4,9 +4,8 @@ Changes to the reference FVM implementation. ## Unreleased -- Added `testing` feature to change module visibility -- Changed visibility of `account_actor`, `init_actor` and `system_actor` to public to use them in the integration test -framework. +- Added `testing` feature to change module visibility; concretely changed visibility of `account_actor`, `init_actor` and `system_actor` to public to use them in the integration test framework. +- Propagate gas outputs in ApplyRet. ## 0.7.1 [2022-04-18] @@ -37,4 +36,4 @@ BREAKING: Updates the FVM to the latest syscall struct alignment - `StateTree::consume` -> `StateTree::into_store` - BREAKING: remove unused (by the FVM) `verify_post_discount` from the FVM PriceList. -[FIP0032]: https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0032.md \ No newline at end of file +[FIP0032]: https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0032.md From 5ec7ceeda6410e0fa680346436c27f8df5e378de Mon Sep 17 00:00:00 2001 From: Jim Pick Date: Mon, 9 May 2022 04:22:24 -0700 Subject: [PATCH 08/21] Spelling fix (#538) --- sdk/src/debug.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/debug.rs b/sdk/src/debug.rs index 74929495c..dd0980ae7 100644 --- a/sdk/src/debug.rs +++ b/sdk/src/debug.rs @@ -31,7 +31,7 @@ mod inner { sys::debug::log(msg.as_ptr(), msg.len() as u32).unwrap(); } } - /// Initialize logging if debuggig is enabled. + /// Initialize logging if debugging is enabled. pub fn init_logging() { if enabled() { log::set_logger(&Logger).expect("failed to enable logging"); From 049ff89f0e7e3d5470b1f5c3c7435b2d304bcbe6 Mon Sep 17 00:00:00 2001 From: raulk Date: Mon, 9 May 2022 11:32:56 +0000 Subject: [PATCH 09/21] fix clippy on nightly. (#539) --- ipld/amt/benches/amt_benchmark.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ipld/amt/benches/amt_benchmark.rs b/ipld/amt/benches/amt_benchmark.rs index c3827b722..54b5c5e7f 100644 --- a/ipld/amt/benches/amt_benchmark.rs +++ b/ipld/amt/benches/amt_benchmark.rs @@ -28,7 +28,9 @@ impl Serialize for BenchData { ().serialize(serializer) } } + impl<'de> Deserialize<'de> for BenchData { + #[allow(clippy::let_unit_value)] fn deserialize(deserializer: D) -> Result>::Error> where D: Deserializer<'de>, From 9beea15d47cb71a8b8bf7717eb23f9518ea7f70e Mon Sep 17 00:00:00 2001 From: raulk Date: Mon, 9 May 2022 22:28:59 +0000 Subject: [PATCH 10/21] fvm: release v0.7.2. (#546) --- fvm/CHANGELOG.md | 18 +++++++++++++++++- fvm/Cargo.toml | 2 +- testing/conformance/Cargo.toml | 2 +- testing/integration/Cargo.toml | 2 +- 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/fvm/CHANGELOG.md b/fvm/CHANGELOG.md index 6bf375590..734a4808f 100644 --- a/fvm/CHANGELOG.md +++ b/fvm/CHANGELOG.md @@ -3,9 +3,25 @@ Changes to the reference FVM implementation. ## Unreleased + +- ... + +## 0.7.2 [2022-05-09] -- Added `testing` feature to change module visibility; concretely changed visibility of `account_actor`, `init_actor` and `system_actor` to public to use them in the integration test framework. +- Add `testing` feature to change module visibility; concretely changed + visibility of `account_actor`, `init_actor` and `system_actor` to `pub` + to use them in the integration test framework. - Propagate gas outputs in ApplyRet. +- Migrate CBOR serde to [cbor4ii](https://github.com/quininer/cbor4ii). +- Instrument Wasm bytecode with [filecoin-project/fvm-wasm-instrument](https://github.com/filecoin-project/fvm-wasm-instrument), + a fork of [paritytech/wasm-instrument](https://github.com/paritytech/wasm-instrument) + for more accurate stack accounting and execution units metering. +- Abort when aborting fails. +- Fix syscall binding docs. +- Fix bugs in Wasm execution units gas accounting. +- Fix system actor state serialization. +- Remove unused dependencies from build graph. +- Optimize memory resolution so it only happens once. ## 0.7.1 [2022-04-18] diff --git a/fvm/Cargo.toml b/fvm/Cargo.toml index 02392cc35..cdd61dfbe 100644 --- a/fvm/Cargo.toml +++ b/fvm/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "fvm" description = "Filecoin Virtual Machine reference implementation" -version = "0.7.1" +version = "0.7.2" license = "MIT OR Apache-2.0" authors = ["Protocol Labs", "Filecoin Core Devs"] edition = "2021" diff --git a/testing/conformance/Cargo.toml b/testing/conformance/Cargo.toml index 3dc4593fc..90d935bf7 100644 --- a/testing/conformance/Cargo.toml +++ b/testing/conformance/Cargo.toml @@ -9,7 +9,7 @@ publish = false repository = "https://github.com/filecoin-project/ref-fvm" [dependencies] -fvm = { version = "0.7.1", path = "../../fvm", default-features = false } +fvm = { version = "0.7.2", path = "../../fvm", default-features = false } fvm_shared = { version = "0.6.1", path = "../../shared" } fvm_ipld_hamt = { version = "0.5.1", path = "../../ipld/hamt"} fvm_ipld_amt = { version = "0.4.1", path = "../../ipld/amt"} diff --git a/testing/integration/Cargo.toml b/testing/integration/Cargo.toml index 9404ee9e2..d2e73ddca 100644 --- a/testing/integration/Cargo.toml +++ b/testing/integration/Cargo.toml @@ -6,7 +6,7 @@ edition = "2021" repository = "https://github.com/filecoin-project/ref-fvm" [dependencies] -fvm = { version = "0.7.1", path = "../../fvm", default-features = false } +fvm = { version = "0.7.2", path = "../../fvm", default-features = false } fvm_shared = { version = "0.6.1", path = "../../shared" } fvm_ipld_hamt = { version = "0.5.1", path = "../../ipld/hamt"} fvm_ipld_amt = { version = "0.4.1", path = "../../ipld/amt"} From 6a3dec131e36fdeb66691de2e49207c0d26fc051 Mon Sep 17 00:00:00 2001 From: raulk Date: Tue, 10 May 2022 18:49:58 +0000 Subject: [PATCH 11/21] implement FIP-0032 gas parameters and charges (#534) * exec gas: apply only on non-structural instructions. * pricelist: add new gas fees. * gas: fully switch to milligas as canonical unit. * price list: update gas cost formulae. * price list: vertical spacing. * kernel: apply extern gas. * syscalls: charge for gas; small refactors. * remove Kernel#charge_milligas(). Canonical unit is now milligas everywhere. * fix tests. * fix param. * clippy. * add comment. * fix gas instrumentation. * rename GasTracker#{charge_gas=>apply_charge} to disambiguate. * add Gas and Milligas unit types to disambiguate. * convert public gas charges to milligas. * inclusion cost: adapt to milligas. * charge_gas: move conversion to milligas to syscall handler. * {to_milligas=>static_milligas}; make private. * clippy. * price list: remove compute_gas_multiplier (unused). * polish comment. * use saturation arithmetics for mem costs. * price list: use Milligas type for cost fields. * rename Kernel#charge_{=>milli}gas. * add extern cost in syscall pricing formulae. * charge for syscall gas at the right place. --- fvm/src/call_manager/mod.rs | 2 +- fvm/src/executor/default.rs | 4 +- fvm/src/gas/charge.rs | 15 +- fvm/src/gas/mod.rs | 46 ++-- fvm/src/gas/outputs.rs | 12 +- fvm/src/gas/price_list.rs | 418 ++++++++++++++++++++-------------- fvm/src/kernel/default.rs | 15 +- fvm/src/kernel/mod.rs | 21 +- fvm/src/syscalls/bind.rs | 12 + fvm/src/syscalls/error.rs | 10 + fvm/src/syscalls/gas.rs | 6 +- fvm/src/syscalls/mod.rs | 11 +- testing/conformance/src/vm.rs | 18 +- 13 files changed, 351 insertions(+), 239 deletions(-) diff --git a/fvm/src/call_manager/mod.rs b/fvm/src/call_manager/mod.rs index 141a205eb..e72975f73 100644 --- a/fvm/src/call_manager/mod.rs +++ b/fvm/src/call_manager/mod.rs @@ -116,7 +116,7 @@ pub trait CallManager: 'static { /// Charge gas. fn charge_gas(&mut self, charge: GasCharge) -> Result<()> { - self.gas_tracker_mut().charge_gas(charge)?; + self.gas_tracker_mut().apply_charge(charge)?; Ok(()) } } diff --git a/fvm/src/executor/default.rs b/fvm/src/executor/default.rs index 97eb845d5..ea0a605ac 100644 --- a/fvm/src/executor/default.rs +++ b/fvm/src/executor/default.rs @@ -15,7 +15,7 @@ use num_traits::Zero; use super::{ApplyFailure, ApplyKind, ApplyRet, Executor}; use crate::call_manager::{backtrace, CallManager, InvocationResult}; -use crate::gas::{GasCharge, GasOutputs}; +use crate::gas::{milligas_to_gas, GasCharge, GasOutputs}; use crate::kernel::{ClassifyResult, Context as _, ExecutionError, Kernel}; use crate::machine::{Machine, BURNT_FUNDS_ACTOR_ADDR, REWARD_ACTOR_ADDR}; @@ -220,7 +220,7 @@ where ApplyKind::Implicit => (GasCharge::new("none", 0, 0), Default::default()), ApplyKind::Explicit => { let inclusion_cost = pl.on_chain_message(raw_length); - let inclusion_total = inclusion_cost.total(); + let inclusion_total = milligas_to_gas(inclusion_cost.total(), true); // Verify the cost of the message is not over the message gas limit. if inclusion_total > msg.gas_limit { diff --git a/fvm/src/gas/charge.rs b/fvm/src/gas/charge.rs index cc52bd03f..f713b10cc 100644 --- a/fvm/src/gas/charge.rs +++ b/fvm/src/gas/charge.rs @@ -1,16 +1,20 @@ // Copyright 2019-2022 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT +use crate::gas::Milligas; + /// Single gas charge in the VM. Contains information about what gas was for, as well /// as the amount of gas needed for computation and storage respectively. pub struct GasCharge<'a> { pub name: &'a str, - pub compute_gas: i64, - pub storage_gas: i64, + /// Compute costs in milligas. + pub compute_gas: Milligas, + /// Storage costs in milligas. + pub storage_gas: Milligas, } impl<'a> GasCharge<'a> { - pub fn new(name: &'a str, compute_gas: i64, storage_gas: i64) -> Self { + pub fn new(name: &'a str, compute_gas: Milligas, storage_gas: Milligas) -> Self { Self { name, compute_gas, @@ -18,8 +22,9 @@ impl<'a> GasCharge<'a> { } } - /// Calculates total gas charge based on compute and storage multipliers. - pub fn total(&self) -> i64 { + /// Calculates total gas charge (in milligas) by summing compute and + /// storage gas associated with this charge. + pub fn total(&self) -> Milligas { self.compute_gas + self.storage_gas } } diff --git a/fvm/src/gas/mod.rs b/fvm/src/gas/mod.rs index ec250ce51..d296add39 100644 --- a/fvm/src/gas/mod.rs +++ b/fvm/src/gas/mod.rs @@ -12,13 +12,19 @@ mod price_list; pub const MILLIGAS_PRECISION: i64 = 1000; +// Type aliases to disambiguate units in interfaces. +pub type Gas = i64; +pub type Milligas = i64; + pub struct GasTracker { milligas_limit: i64, milligas_used: i64, } impl GasTracker { - pub fn new(gas_limit: i64, gas_used: i64) -> Self { + /// Gas limit and gas used are provided in protocol units (i.e. full units). + /// They are converted to milligas for internal canonical accounting. + pub fn new(gas_limit: Gas, gas_used: Gas) -> Self { Self { milligas_limit: gas_to_milligas(gas_limit), milligas_used: gas_to_milligas(gas_used), @@ -27,7 +33,7 @@ impl GasTracker { /// Safely consumes gas and returns an out of gas error if there is not sufficient /// enough gas remaining for charge. - pub fn charge_milligas(&mut self, name: &str, to_use: i64) -> Result<()> { + pub fn charge_milligas(&mut self, name: &str, to_use: Milligas) -> Result<()> { match self.milligas_used.checked_add(to_use) { None => { log::trace!("gas overflow: {}", name); @@ -48,51 +54,49 @@ impl GasTracker { } } - pub fn charge_gas(&mut self, charge: GasCharge) -> Result<()> { - self.charge_milligas( - charge.name, - charge.total().saturating_mul(MILLIGAS_PRECISION), - ) + /// Applies the specified gas charge, where quantities are supplied in milligas. + pub fn apply_charge(&mut self, charge: GasCharge) -> Result<()> { + self.charge_milligas(charge.name, charge.total()) } /// Getter for gas available. - pub fn gas_limit(&self) -> i64 { + pub fn gas_limit(&self) -> Gas { milligas_to_gas(self.milligas_limit, false) } /// Getter for milligas available. - pub fn milligas_limit(&self) -> i64 { + pub fn milligas_limit(&self) -> Milligas { self.milligas_limit } /// Getter for gas used. - pub fn gas_used(&self) -> i64 { + pub fn gas_used(&self) -> Gas { milligas_to_gas(self.milligas_used, true) } /// Getter for milligas used. - pub fn milligas_used(&self) -> i64 { + pub fn milligas_used(&self) -> Milligas { self.milligas_used } - pub fn gas_available(&self) -> i64 { + pub fn gas_available(&self) -> Gas { milligas_to_gas(self.milligas_available(), false) } - pub fn milligas_available(&self) -> i64 { + pub fn milligas_available(&self) -> Milligas { self.milligas_limit.saturating_sub(self.milligas_used) } } /// Converts the specified gas into equivalent fractional gas units #[inline] -fn gas_to_milligas(gas: i64) -> i64 { +pub(crate) fn gas_to_milligas(gas: i64) -> i64 { gas.saturating_mul(MILLIGAS_PRECISION) } /// Converts the specified fractional gas units into gas units #[inline] -fn milligas_to_gas(milligas: i64, round_up: bool) -> i64 { +pub(crate) fn milligas_to_gas(milligas: i64, round_up: bool) -> i64 { let mut div_result = milligas / MILLIGAS_PRECISION; if milligas > 0 && round_up && milligas % MILLIGAS_PRECISION != 0 { div_result = div_result.saturating_add(1); @@ -107,13 +111,17 @@ mod tests { use super::*; #[test] - fn basic_gas_tracker() { + #[allow(clippy::identity_op)] + fn basic_gas_tracker() -> Result<()> { let mut t = GasTracker::new(20, 10); - t.charge_gas(GasCharge::new("", 5, 0)).unwrap(); + t.apply_charge(GasCharge::new("", 5 * MILLIGAS_PRECISION, 0))?; assert_eq!(t.gas_used(), 15); - t.charge_gas(GasCharge::new("", 5, 0)).unwrap(); + t.apply_charge(GasCharge::new("", 5 * MILLIGAS_PRECISION, 0))?; assert_eq!(t.gas_used(), 20); - assert!(t.charge_gas(GasCharge::new("", 1, 0)).is_err()) + assert!(t + .apply_charge(GasCharge::new("", 1 * MILLIGAS_PRECISION, 0)) + .is_err()); + Ok(()) } #[test] diff --git a/fvm/src/gas/outputs.rs b/fvm/src/gas/outputs.rs index d52cec83a..ccb1b819f 100644 --- a/fvm/src/gas/outputs.rs +++ b/fvm/src/gas/outputs.rs @@ -3,6 +3,8 @@ use std::convert::TryFrom; use fvm_shared::bigint::BigInt; use fvm_shared::econ::TokenAmount; +use crate::gas::Gas; + #[derive(Clone, Default)] pub(crate) struct GasOutputs { pub base_fee_burn: TokenAmount, @@ -11,14 +13,14 @@ pub(crate) struct GasOutputs { pub miner_tip: TokenAmount, pub refund: TokenAmount, - pub gas_refund: i64, - pub gas_burned: i64, + pub gas_refund: Gas, + pub gas_burned: Gas, } impl GasOutputs { pub fn compute( - gas_used: i64, - gas_limit: i64, + gas_used: Gas, + gas_limit: Gas, base_fee: &TokenAmount, fee_cap: &TokenAmount, gas_premium: &TokenAmount, @@ -57,7 +59,7 @@ impl GasOutputs { } } -fn compute_gas_overestimation_burn(gas_used: i64, gas_limit: i64) -> (i64, i64) { +fn compute_gas_overestimation_burn(gas_used: Gas, gas_limit: Gas) -> (Gas, Gas) { const GAS_OVERUSE_NUM: i64 = 11; const GAS_OVERUSE_DENOM: i64 = 10; diff --git a/fvm/src/gas/price_list.rs b/fvm/src/gas/price_list.rs index a91c0ceac..3744c6a94 100644 --- a/fvm/src/gas/price_list.rs +++ b/fvm/src/gas/price_list.rs @@ -17,43 +17,51 @@ use lazy_static::lazy_static; use num_traits::Zero; use super::GasCharge; +use crate::gas::Milligas; + +/// Converts a static value to milligas. This operation does not saturate, +/// and should only be used with constant values in pricelists. +macro_rules! static_milligas { + ($ex:expr) => { + $ex * $crate::gas::MILLIGAS_PRECISION + }; +} lazy_static! { static ref OH_SNAP_PRICES: PriceList = PriceList { - compute_gas_multiplier: 1, storage_gas_multiplier: 1300, - on_chain_message_compute_base: 38863, - on_chain_message_storage_base: 36, - on_chain_message_storage_per_byte: 1, + on_chain_message_compute_base: static_milligas!(38863), + on_chain_message_storage_base: static_milligas!(36), + on_chain_message_storage_per_byte: static_milligas!(1), - on_chain_return_value_per_byte: 1, + on_chain_return_value_per_byte: static_milligas!(1), - send_base: 29233, - send_transfer_funds: 27500, - send_transfer_only_premium: 159672, - send_invoke_method: -5377, + send_base: static_milligas!(29233), + send_transfer_funds: static_milligas!(27500), + send_transfer_only_premium: static_milligas!(159672), + send_invoke_method: static_milligas!(-5377), - create_actor_compute: 1108454, - create_actor_storage: 36 + 40, - delete_actor: -(36 + 40), + create_actor_compute: static_milligas!(1108454), + create_actor_storage: static_milligas!(36 + 40), + delete_actor: static_milligas!(-(36 + 40)), - bls_sig_cost: 16598605, - secp256k1_sig_cost: 1637292, + bls_sig_cost: static_milligas!(16598605), + secp256k1_sig_cost: static_milligas!(1637292), - hashing_base: 31355, - compute_unsealed_sector_cid_base: 98647, - verify_seal_base: 2000, // TODO revisit potential removal of this + hashing_base: static_milligas!(31355), + compute_unsealed_sector_cid_base: static_milligas!(98647), + verify_seal_base: static_milligas!(2000), // TODO revisit potential removal of this verify_aggregate_seal_base: 0, verify_aggregate_seal_per: [ ( RegisteredSealProof::StackedDRG32GiBV1P1, - 449900 + static_milligas!(449900) ), ( RegisteredSealProof::StackedDRG64GiBV1P1, - 359272 + static_milligas!(359272) ) ].iter().copied().collect(), verify_aggregate_seal_steps: [ @@ -61,14 +69,14 @@ lazy_static! { RegisteredSealProof::StackedDRG32GiBV1P1, StepCost ( vec![ - Step{start: 4, cost: 103994170}, - Step{start: 7, cost: 112356810}, - Step{start: 13, cost: 122912610}, - Step{start: 26, cost: 137559930}, - Step{start: 52, cost: 162039100}, - Step{start: 103, cost: 210960780}, - Step{start: 205, cost: 318351180}, - Step{start: 410, cost: 528274980}, + Step{start: 4, cost: static_milligas!(103994170)}, + Step{start: 7, cost: static_milligas!(112356810)}, + Step{start: 13, cost: static_milligas!(122912610)}, + Step{start: 26, cost: static_milligas!(137559930)}, + Step{start: 52, cost: static_milligas!(162039100)}, + Step{start: 103, cost: static_milligas!(210960780)}, + Step{start: 205, cost: static_milligas!(318351180)}, + Step{start: 410, cost: static_milligas!(528274980)}, ] ) ), @@ -76,14 +84,14 @@ lazy_static! { RegisteredSealProof::StackedDRG64GiBV1P1, StepCost ( vec![ - Step{start: 4, cost: 102581240}, - Step{start: 7, cost: 110803030}, - Step{start: 13, cost: 120803700}, - Step{start: 26, cost: 134642130}, - Step{start: 52, cost: 157357890}, - Step{start: 103, cost: 203017690}, - Step{start: 205, cost: 304253590}, - Step{start: 410, cost: 509880640}, + Step{start: 4, cost: static_milligas!(102581240)}, + Step{start: 7, cost: static_milligas!(110803030)}, + Step{start: 13, cost: static_milligas!(120803700)}, + Step{start: 26, cost: static_milligas!(134642130)}, + Step{start: 52, cost: static_milligas!(157357890)}, + Step{start: 103, cost: static_milligas!(203017690)}, + Step{start: 205, cost: static_milligas!(304253590)}, + Step{start: 410, cost: static_milligas!(509880640)}, ] ) ) @@ -91,27 +99,27 @@ lazy_static! { .cloned() .collect(), - verify_consensus_fault: 495422, - verify_replica_update: 36316136, + verify_consensus_fault: static_milligas!(495422), + verify_replica_update: static_milligas!(36316136), verify_post_lookup: [ ( RegisteredPoStProof::StackedDRGWindow512MiBV1, ScalingCost { - flat: 117680921, + flat: static_milligas!(117680921), scale: 43780, }, ), ( RegisteredPoStProof::StackedDRGWindow32GiBV1, ScalingCost { - flat: 117680921, + flat: static_milligas!(117680921), scale: 43780, }, ), ( RegisteredPoStProof::StackedDRGWindow64GiBV1, ScalingCost { - flat: 117680921, + flat: static_milligas!(117680921), scale: 43780, }, ), @@ -124,14 +132,21 @@ lazy_static! { get_randomness_per_byte: 0, block_memcpy_per_byte_cost: 0, - block_io_per_byte_cost: 0, - block_link_per_byte_cost: 1, - block_open_base: 114617, - block_read_base: 0, + block_open_base: static_milligas!(114617), + block_open_memret_per_byte_cost: 0, + + block_link_base: static_milligas!(353640), + block_link_storage_per_byte_cost: static_milligas!(1), + block_create_base: 0, - block_link_base: 353640, - block_stat: 0, + block_create_memret_per_byte_cost: 0, + + block_read_base: 0, + block_stat_base: 0, + + syscall_cost: 0, + extern_cost: 0, wasm_rules: WasmGasPrices{ exec_instruction_cost_milli: 0, @@ -139,40 +154,39 @@ lazy_static! { }; static ref SKYR_PRICES: PriceList = PriceList { - compute_gas_multiplier: 1, storage_gas_multiplier: 1300, - on_chain_message_compute_base: 38863, - on_chain_message_storage_base: 36, - on_chain_message_storage_per_byte: 1, + on_chain_message_compute_base: static_milligas!(38863), + on_chain_message_storage_base: static_milligas!(36), + on_chain_message_storage_per_byte: static_milligas!(1), - on_chain_return_value_per_byte: 1, + on_chain_return_value_per_byte: static_milligas!(1), - send_base: 29233, - send_transfer_funds: 27500, - send_transfer_only_premium: 159672, - send_invoke_method: -5377, + send_base: static_milligas!(29233), + send_transfer_funds: static_milligas!(27500), + send_transfer_only_premium: static_milligas!(159672), + send_invoke_method: static_milligas!(-5377), - create_actor_compute: 1108454, - create_actor_storage: 36 + 40, - delete_actor: -(36 + 40), + create_actor_compute: static_milligas!(1108454), + create_actor_storage: static_milligas!(36 + 40), + delete_actor: static_milligas!(-(36 + 40)), - bls_sig_cost: 16598605, - secp256k1_sig_cost: 1637292, + bls_sig_cost: static_milligas!(16598605), + secp256k1_sig_cost: static_milligas!(1637292), - hashing_base: 31355, - compute_unsealed_sector_cid_base: 98647, - verify_seal_base: 2000, // TODO revisit potential removal of this + hashing_base: static_milligas!(31355), + compute_unsealed_sector_cid_base: static_milligas!(98647), + verify_seal_base: static_milligas!(2000), // TODO revisit potential removal of this verify_aggregate_seal_base: 0, verify_aggregate_seal_per: [ ( RegisteredSealProof::StackedDRG32GiBV1P1, - 449900 + static_milligas!(449900) ), ( RegisteredSealProof::StackedDRG64GiBV1P1, - 359272 + static_milligas!(359272) ) ].iter().copied().collect(), verify_aggregate_seal_steps: [ @@ -180,14 +194,14 @@ lazy_static! { RegisteredSealProof::StackedDRG32GiBV1P1, StepCost ( vec![ - Step{start: 4, cost: 103994170}, - Step{start: 7, cost: 112356810}, - Step{start: 13, cost: 122912610}, - Step{start: 26, cost: 137559930}, - Step{start: 52, cost: 162039100}, - Step{start: 103, cost: 210960780}, - Step{start: 205, cost: 318351180}, - Step{start: 410, cost: 528274980}, + Step{start: 4, cost: static_milligas!(103994170)}, + Step{start: 7, cost: static_milligas!(112356810)}, + Step{start: 13, cost: static_milligas!(122912610)}, + Step{start: 26, cost: static_milligas!(137559930)}, + Step{start: 52, cost: static_milligas!(162039100)}, + Step{start: 103, cost: static_milligas!(210960780)}, + Step{start: 205, cost: static_milligas!(318351180)}, + Step{start: 410, cost: static_milligas!(528274980)}, ] ) ), @@ -195,14 +209,14 @@ lazy_static! { RegisteredSealProof::StackedDRG64GiBV1P1, StepCost ( vec![ - Step{start: 4, cost: 102581240}, - Step{start: 7, cost: 110803030}, - Step{start: 13, cost: 120803700}, - Step{start: 26, cost: 134642130}, - Step{start: 52, cost: 157357890}, - Step{start: 103, cost: 203017690}, - Step{start: 205, cost: 304253590}, - Step{start: 410, cost: 509880640}, + Step{start: 4, cost: static_milligas!(102581240)}, + Step{start: 7, cost: static_milligas!(110803030)}, + Step{start: 13, cost: static_milligas!(120803700)}, + Step{start: 26, cost: static_milligas!(134642130)}, + Step{start: 52, cost: static_milligas!(157357890)}, + Step{start: 103, cost: static_milligas!(203017690)}, + Step{start: 205, cost: static_milligas!(304253590)}, + Step{start: 410, cost: static_milligas!(509880640)}, ] ) ) @@ -211,27 +225,27 @@ lazy_static! { .collect(), // TODO: PARAM_FINISH: this may need to be increased to account for the cost of an extern - verify_consensus_fault: 495422, - verify_replica_update: 36316136, + verify_consensus_fault: static_milligas!(495422), + verify_replica_update: static_milligas!(36316136), verify_post_lookup: [ ( RegisteredPoStProof::StackedDRGWindow512MiBV1, ScalingCost { - flat: 117680921, + flat: static_milligas!(117680921), scale: 43780, }, ), ( RegisteredPoStProof::StackedDRGWindow32GiBV1, ScalingCost { - flat: 117680921, + flat: static_milligas!(117680921), scale: 43780, }, ), ( RegisteredPoStProof::StackedDRGWindow64GiBV1, ScalingCost { - flat: 117680921, + flat: static_milligas!(117680921), scale: 43780, }, ), @@ -240,31 +254,29 @@ lazy_static! { .copied() .collect(), - block_memcpy_per_byte_cost: 4, - block_io_per_byte_cost: 2, - block_link_per_byte_cost: 1, - // TODO: PARAM_FINISH - - // TODO: PARAM_FINISH - get_randomness_base: 1, - // TODO: PARAM_FINISH - get_randomness_per_byte: 1, - - // TODO: PARAM_FINIuiSH - block_open_base: 1, - // TODO: PARAM_FINISH - block_read_base: 1, - // TODO: PARAM_FINISH - block_create_base: 1, - // TODO: PARAM_FINISH - block_link_base: 1, - // TODO: PARAM_FINISH - block_stat: 1, + get_randomness_base: 0, + get_randomness_per_byte: 0, + + block_memcpy_per_byte_cost: 500, + + block_open_base: static_milligas!(114617), + block_open_memret_per_byte_cost: static_milligas!(10), + + block_link_base: static_milligas!(353640), + block_link_storage_per_byte_cost: static_milligas!(1), + + block_create_base: 0, + block_create_memret_per_byte_cost: static_milligas!(10), + + block_read_base: 0, + block_stat_base: 0, + + syscall_cost: static_milligas!(14000), + extern_cost: static_milligas!(21000), wasm_rules: WasmGasPrices{ - exec_instruction_cost_milli: 5000, + exec_instruction_cost_milli: static_milligas!(4) as u64, }, - // TODO: PARAM_FINISH }; } @@ -300,14 +312,10 @@ impl StepCost { } } -/// Provides prices for operations in the VM +/// Provides prices for operations in the VM. +/// All costs are in milligas. #[derive(Clone, Debug)] pub struct PriceList { - /// Compute gas charge multiplier - // * This multiplier is not currently applied to anything, but is matching lotus. - // * If the possible values are non 1 or if Lotus adds, we should change also. - #[allow(unused)] - pub(crate) compute_gas_multiplier: i64, /// Storage gas charge multiplier pub(crate) storage_gas_multiplier: i64, @@ -317,77 +325,98 @@ pub struct PriceList { /// Together, these account for the cost of message propagation and validation, /// up to but excluding any actual processing by the VM. /// This is the cost a block producer burns when including an invalid message. - pub(crate) on_chain_message_compute_base: i64, - pub(crate) on_chain_message_storage_base: i64, - pub(crate) on_chain_message_storage_per_byte: i64, + pub(crate) on_chain_message_compute_base: Milligas, + pub(crate) on_chain_message_storage_base: Milligas, + pub(crate) on_chain_message_storage_per_byte: Milligas, /// Gas cost charged to the originator of a non-nil return value produced /// by an on-chain message is given by: /// len(return value)*OnChainReturnValuePerByte - pub(crate) on_chain_return_value_per_byte: i64, + pub(crate) on_chain_return_value_per_byte: Milligas, /// Gas cost for any message send execution(including the top-level one /// initiated by an on-chain message). /// This accounts for the cost of loading sender and receiver actors and /// (for top-level messages) incrementing the sender's sequence number. /// Load and store of actor sub-state is charged separately. - pub(crate) send_base: i64, + pub(crate) send_base: Milligas, /// Gas cost charged, in addition to SendBase, if a message send /// is accompanied by any nonzero currency amount. /// Accounts for writing receiver's new balance (the sender's state is /// already accounted for). - pub(crate) send_transfer_funds: i64, + pub(crate) send_transfer_funds: Milligas, /// Gas cost charged, in addition to SendBase, if message only transfers funds. - pub(crate) send_transfer_only_premium: i64, + pub(crate) send_transfer_only_premium: Milligas, /// Gas cost charged, in addition to SendBase, if a message invokes /// a method on the receiver. /// Accounts for the cost of loading receiver code and method dispatch. - pub(crate) send_invoke_method: i64, + pub(crate) send_invoke_method: Milligas, /// Gas cost for creating a new actor (via InitActor's Exec method). /// Note: this costs assume that the extra will be partially or totally refunded while /// the base is covering for the put. - pub(crate) create_actor_compute: i64, - pub(crate) create_actor_storage: i64, + pub(crate) create_actor_compute: Milligas, + pub(crate) create_actor_storage: Milligas, /// Gas cost for deleting an actor. /// Note: this partially refunds the create cost to incentivise the deletion of the actors. - pub(crate) delete_actor: i64, + pub(crate) delete_actor: Milligas, /// Gas cost for verifying bls signature - pub(crate) bls_sig_cost: i64, + pub(crate) bls_sig_cost: Milligas, /// Gas cost for verifying secp256k1 signature - pub(crate) secp256k1_sig_cost: i64, + pub(crate) secp256k1_sig_cost: Milligas, - pub(crate) hashing_base: i64, + pub(crate) hashing_base: Milligas, - pub(crate) compute_unsealed_sector_cid_base: i64, - pub(crate) verify_seal_base: i64, + pub(crate) compute_unsealed_sector_cid_base: Milligas, + pub(crate) verify_seal_base: Milligas, #[allow(unused)] - pub(crate) verify_aggregate_seal_base: i64, + pub(crate) verify_aggregate_seal_base: Milligas, pub(crate) verify_aggregate_seal_per: AHashMap, pub(crate) verify_aggregate_seal_steps: AHashMap, pub(crate) verify_post_lookup: AHashMap, - pub(crate) verify_consensus_fault: i64, - pub(crate) verify_replica_update: i64, - - pub(crate) get_randomness_base: i64, - pub(crate) get_randomness_per_byte: i64, - - pub(crate) block_memcpy_per_byte_cost: i64, - pub(crate) block_io_per_byte_cost: i64, - pub(crate) block_link_per_byte_cost: i64, - - pub(crate) block_open_base: i64, - pub(crate) block_read_base: i64, - pub(crate) block_create_base: i64, - pub(crate) block_link_base: i64, - pub(crate) block_stat: i64, - + pub(crate) verify_consensus_fault: Milligas, + pub(crate) verify_replica_update: Milligas, + + /// Gas cost for fetching randomness. + pub(crate) get_randomness_base: Milligas, + /// Gas cost per every byte of randomness fetched. + pub(crate) get_randomness_per_byte: Milligas, + + /// Gas cost per every block byte memcopied across boundaries. + pub(crate) block_memcpy_per_byte_cost: Milligas, + + /// Gas cost for opening a block. + pub(crate) block_open_base: Milligas, + /// Gas cost for every byte retained in FVM space when opening a block. + pub(crate) block_open_memret_per_byte_cost: Milligas, + + /// Gas cost for linking a block. + pub(crate) block_link_base: Milligas, + /// Multiplier for storage gas per byte. + pub(crate) block_link_storage_per_byte_cost: Milligas, + + /// Gas cost for creating a block. + pub(crate) block_create_base: Milligas, + /// Gas cost for every byte retained in FVM space when writing a block. + pub(crate) block_create_memret_per_byte_cost: Milligas, + + /// Gas cost for reading a block into actor space. + pub(crate) block_read_base: Milligas, + /// Gas cost for statting a block. + pub(crate) block_stat_base: Milligas, + + /// General gas cost for performing a syscall, accounting for the overhead thereof. + pub(crate) syscall_cost: Milligas, + /// General gas cost for calling an extern, accounting for the overhead thereof. + pub(crate) extern_cost: Milligas, + + /// Rules for execution gas. pub(crate) wasm_rules: WasmGasPrices, } @@ -408,6 +437,7 @@ impl PriceList { * self.storage_gas_multiplier, ) } + /// Returns the gas required for storing the response of a message in the chain. #[inline] pub fn on_chain_return_value(&self, data_size: usize) -> GasCharge<'static> { @@ -417,6 +447,7 @@ impl PriceList { data_size as i64 * self.on_chain_return_value_per_byte * self.storage_gas_multiplier, ) } + /// Returns the gas required when invoking a method. #[inline] pub fn on_method_invocation( @@ -436,6 +467,12 @@ impl PriceList { } GasCharge::new("OnMethodInvocation", ret, 0) } + + /// Returns the gas cost to be applied on a syscall. + pub fn on_syscall(&self) -> GasCharge<'static> { + GasCharge::new("OnSyscall", self.syscall_cost, 0) + } + /// Returns the gas required for creating an actor. #[inline] pub fn on_create_actor(&self) -> GasCharge<'static> { @@ -445,6 +482,7 @@ impl PriceList { self.create_actor_storage * self.storage_gas_multiplier, ) } + /// Returns the gas required for deleting an actor. #[inline] pub fn on_delete_actor(&self) -> GasCharge<'static> { @@ -454,6 +492,7 @@ impl PriceList { self.delete_actor * self.storage_gas_multiplier, ) } + /// Returns gas required for signature verification. #[inline] pub fn on_verify_signature(&self, sig_type: SignatureType) -> GasCharge<'static> { @@ -463,11 +502,13 @@ impl PriceList { }; GasCharge::new("OnVerifySignature", val, 0) } + /// Returns gas required for hashing data. #[inline] pub fn on_hashing(&self, _: usize) -> GasCharge<'static> { GasCharge::new("OnHashing", self.hashing_base, 0) } + /// Returns gas required for computing unsealed sector Cid. #[inline] pub fn on_compute_unsealed_sector_cid( @@ -481,6 +522,7 @@ impl PriceList { 0, ) } + /// Returns gas required for seal verification. #[inline] pub fn on_verify_seal(&self, _info: &SealVerifyInfo) -> GasCharge<'static> { @@ -521,11 +563,13 @@ impl PriceList { 0, ) } + /// Returns gas required for replica verification. #[inline] pub fn on_verify_replica_update(&self, _replica: &ReplicaUpdateInfo) -> GasCharge<'static> { GasCharge::new("OnVerifyReplicaUpdate", self.verify_replica_update, 0) } + /// Returns gas required for PoSt verification. #[inline] pub fn on_verify_post(&self, info: &WindowPoStVerifyInfo) -> GasCharge<'static> { @@ -544,10 +588,15 @@ impl PriceList { GasCharge::new("OnVerifyPost", gas_used, 0) } + /// Returns gas required for verifying consensus fault. #[inline] pub fn on_verify_consensus_fault(&self) -> GasCharge<'static> { - GasCharge::new("OnVerifyConsensusFault", self.verify_consensus_fault, 0) + GasCharge::new( + "OnVerifyConsensusFault", + self.extern_cost.saturating_add(self.verify_consensus_fault), + 0, + ) } /// Returns the cost of the gas required for getting randomness from the client, based on the @@ -556,10 +605,12 @@ impl PriceList { pub fn on_get_randomness(&self, entropy_size: usize) -> GasCharge<'static> { GasCharge::new( "OnGetRandomness", - self.get_randomness_base.saturating_add( - self.get_randomness_per_byte - .saturating_mul(entropy_size as i64), - ), + self.extern_cost + .saturating_add(self.get_randomness_base) + .saturating_add( + self.get_randomness_per_byte + .saturating_mul(entropy_size as i64), + ), 0, ) } @@ -567,19 +618,25 @@ impl PriceList { /// Returns the base gas required for loading an object, independent of the object's size. #[inline] pub fn on_block_open_base(&self) -> GasCharge<'static> { - GasCharge::new("OnBlockOpenBase", self.block_open_base, 0) + GasCharge::new( + "OnBlockOpenBase", + self.extern_cost.saturating_add(self.block_open_base), + 0, + ) } /// Returns the gas required for loading an object based on the size of the object. #[inline] pub fn on_block_open_per_byte(&self, data_size: usize) -> GasCharge<'static> { - // TODO: Should we also throw on a memcpy cost here (see https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0032.md#ipld-state-management-fees) + let size = data_size as i64; GasCharge::new( "OnBlockOpenPerByte", - self.block_io_per_byte_cost.saturating_mul(data_size as i64), + self.block_open_memret_per_byte_cost.saturating_mul(size) + + self.block_memcpy_per_byte_cost.saturating_mul(size), 0, ) } + /// Returns the gas required for reading a loaded object. #[inline] pub fn on_block_read(&self, data_size: usize) -> GasCharge<'static> { @@ -596,12 +653,14 @@ impl PriceList { /// Returns the gas required for adding an object to the FVM cache. #[inline] pub fn on_block_create(&self, data_size: usize) -> GasCharge<'static> { + let size = data_size as i64; + let mem_costs = self + .block_create_memret_per_byte_cost + .saturating_mul(size) + .saturating_add(self.block_memcpy_per_byte_cost.saturating_mul(size)); GasCharge::new( "OnBlockCreate", - self.block_create_base.saturating_add( - self.block_memcpy_per_byte_cost - .saturating_mul(data_size as i64), - ), + self.block_create_base.saturating_add(mem_costs), 0, ) } @@ -609,21 +668,26 @@ impl PriceList { /// Returns the gas required for committing an object to the state blockstore. #[inline] pub fn on_block_link(&self, data_size: usize) -> GasCharge<'static> { - // TODO: The FIP makes it sound like this would need 2 memcpys, is that what's desired? + let size = data_size as i64; + let memcpy = self.block_memcpy_per_byte_cost.saturating_mul(size); GasCharge::new( "OnBlockLink", - self.block_link_base, - // data_size as i64 * self.block_link_per_byte_cost * self.storage_gas_multiplier, - self.block_link_per_byte_cost + // twice the memcpy cost: + // - one from the block registry to the FVM BufferedBlockstore + // - one from the FVM BufferedBlockstore to the Node's Blockstore + // when the machine finishes. + self.block_link_base + .saturating_add((2_i64).saturating_mul(memcpy)), + self.block_link_storage_per_byte_cost .saturating_mul(self.storage_gas_multiplier) - .saturating_mul(data_size as i64), + .saturating_mul(size), ) } /// Returns the gas required for storing an object. #[inline] pub fn on_block_stat(&self) -> GasCharge<'static> { - GasCharge::new("OnBlockStat", self.block_stat, 0) + GasCharge::new("OnBlockStat", self.block_stat_base, 0) } } @@ -636,12 +700,30 @@ pub fn price_list_by_network_version(network_version: NetworkVersion) -> &'stati } impl Rules for WasmGasPrices { - fn instruction_cost(&self, _instruction: &Instruction) -> Option { - Some(self.exec_instruction_cost_milli) + fn instruction_cost(&self, instruction: &Instruction) -> Option { + if self.exec_instruction_cost_milli == 0 { + return Some(0); + } + + // Rules valid for nv16. We will need to be generic over Rules (massive + // generics tax), use &dyn Rules (which breaks other things), or pass + // in the network version, or rules version, to vary these prices going + // forward. + match instruction { + // FIP-0032: nop, drop, block, loop, unreachable, return, else, end are priced 0. + Instruction::Nop + | Instruction::Drop + | Instruction::Block(_) + | Instruction::Loop(_) + | Instruction::Unreachable + | Instruction::Return + | Instruction::Else + | Instruction::End => Some(0), + _ => Some(self.exec_instruction_cost_milli), + } } fn memory_grow_cost(&self) -> MemoryGrowCost { - // todo use pricelist MemoryGrowCost::Free } } diff --git a/fvm/src/kernel/default.rs b/fvm/src/kernel/default.rs index 3a043f00b..c59fcc6bb 100644 --- a/fvm/src/kernel/default.rs +++ b/fvm/src/kernel/default.rs @@ -599,6 +599,7 @@ where .externs() .verify_consensus_fault(h1, h2, extra) .or_illegal_argument()?; + if self.network_version() <= NetworkVersion::V15 { self.call_manager.charge_gas(GasCharge::new( "verify_consensus_fault_accesses", @@ -762,20 +763,14 @@ where fn gas_available(&self) -> i64 { self.call_manager.gas_tracker().gas_available() } + fn milligas_available(&self) -> i64 { self.call_manager.gas_tracker().milligas_available() } - fn charge_gas(&mut self, name: &str, compute: i64) -> Result<()> { + fn charge_milligas(&mut self, name: &str, compute: Milligas) -> Result<()> { let charge = GasCharge::new(name, compute, 0); - self.call_manager.gas_tracker_mut().charge_gas(charge) - } - - fn charge_milligas(&mut self, name: &str, compute: i64) -> Result<()> { - self.call_manager - .gas_tracker_mut() - .charge_milligas(name, compute)?; - Ok(()) + self.call_manager.gas_tracker_mut().apply_charge(charge) } fn price_list(&self) -> &PriceList { @@ -816,6 +811,7 @@ where .price_list() .on_get_randomness(entropy.len()), )?; + // TODO: Check error code self.call_manager .externs() @@ -835,6 +831,7 @@ where .price_list() .on_get_randomness(entropy.len()), )?; + // TODO: Check error code self.call_manager .externs() diff --git a/fvm/src/kernel/mod.rs b/fvm/src/kernel/mod.rs index 5eee6911a..ff65b3098 100644 --- a/fvm/src/kernel/mod.rs +++ b/fvm/src/kernel/mod.rs @@ -24,10 +24,9 @@ mod error; pub use error::{ClassifyResult, Context, ExecutionError, Result, SyscallError}; use crate::call_manager::{CallManager, InvocationResult}; -use crate::gas::PriceList; +use crate::gas::{Gas, Milligas, PriceList}; use crate::machine::Machine; -/// The "kernel" implements pub trait Kernel: ActorOps + BlockOps @@ -214,19 +213,19 @@ pub trait CircSupplyOps { /// Operations for explicit gas charging. pub trait GasOps { - /// GasUsed return the gas used by the transaction so far. - fn gas_used(&self) -> i64; - fn milligas_used(&self) -> i64; + /// Returns the gas used by the transaction so far. + fn gas_used(&self) -> Gas; + fn milligas_used(&self) -> Milligas; - fn gas_available(&self) -> i64; - fn milligas_available(&self) -> i64; + /// Returns the remaining gas for the transaction. + fn gas_available(&self) -> Gas; + fn milligas_available(&self) -> Milligas; /// ChargeGas charges specified amount of `gas` for execution. - /// `name` provides information about gas charging point - fn charge_gas(&mut self, name: &str, compute: i64) -> Result<()>; - - fn charge_milligas(&mut self, name: &str, compute: i64) -> Result<()>; + /// `name` provides information about gas charging point. + fn charge_milligas(&mut self, name: &str, compute: Milligas) -> Result<()>; + /// Returns the currently active gas price list. fn price_list(&self) -> &PriceList; } diff --git a/fvm/src/syscalls/bind.rs b/fvm/src/syscalls/bind.rs index 2afcb48dd..62889b91a 100644 --- a/fvm/src/syscalls/bind.rs +++ b/fvm/src/syscalls/bind.rs @@ -95,6 +95,15 @@ fn memory_and_data<'a, K: Kernel>( (Memory::new(mem), data) } +macro_rules! charge_syscall_gas { + ($kernel:expr) => { + let charge = $kernel.price_list().on_syscall(); + $kernel + .charge_milligas(charge.name, charge.compute_gas) + .map_err(Abort::from_error_as_fatal)?; + }; +} + // Unfortunately, we can't implement this for _all_ functions. So we implement it for functions of up to 6 arguments. macro_rules! impl_bind_syscalls { ($($t:ident)*) => { @@ -118,6 +127,8 @@ macro_rules! impl_bind_syscalls { charge_for_exec(&mut caller)?; let (mut memory, mut data) = memory_and_data(&mut caller); + charge_syscall_gas!(data.kernel); + let ctx = Context{kernel: &mut data.kernel, memory: &mut memory}; let out = syscall(ctx $(, $t)*).into(); @@ -146,6 +157,7 @@ macro_rules! impl_bind_syscalls { charge_for_exec(&mut caller)?; let (mut memory, mut data) = memory_and_data(&mut caller); + charge_syscall_gas!(data.kernel); // We need to check to make sure we can store the return value _before_ we do anything. if (ret as u64) > (memory.len() as u64) diff --git a/fvm/src/syscalls/error.rs b/fvm/src/syscalls/error.rs index 5872da9cd..d8a7b0257 100644 --- a/fvm/src/syscalls/error.rs +++ b/fvm/src/syscalls/error.rs @@ -1,6 +1,7 @@ //! This module contains code used to convert errors to and from wasmtime traps. use std::sync::Mutex; +use anyhow::anyhow; use derive_more::Display; use fvm_shared::error::ExitCode; use wasmtime::Trap; @@ -34,6 +35,15 @@ impl Abort { ExecutionError::Fatal(err) => Abort::Fatal(err), } } + + /// Just like from_error, but escalating syscall errors as fatal. + pub fn from_error_as_fatal(e: ExecutionError) -> Self { + match e { + ExecutionError::OutOfGas => Abort::OutOfGas, + ExecutionError::Fatal(e) => Abort::Fatal(e), + ExecutionError::Syscall(e) => Abort::Fatal(anyhow!("unexpected syscall error: {}", e)), + } + } } /// Wraps an execution error in a Trap. diff --git a/fvm/src/syscalls/gas.rs b/fvm/src/syscalls/gas.rs index 75bfd14c6..9009506d4 100644 --- a/fvm/src/syscalls/gas.rs +++ b/fvm/src/syscalls/gas.rs @@ -1,6 +1,7 @@ use std::str; use super::Context; +use crate::gas::gas_to_milligas; use crate::kernel::{ClassifyResult, Result}; use crate::Kernel; @@ -12,5 +13,8 @@ pub fn charge_gas( ) -> Result<()> { let name = str::from_utf8(context.memory.try_slice(name_off, name_len)?).or_illegal_argument()?; - context.kernel.charge_gas(name, compute) + // Gas charges from actors are always in full gas units. We use milligas internally, so convert here. + context + .kernel + .charge_milligas(name, gas_to_milligas(compute)) } diff --git a/fvm/src/syscalls/mod.rs b/fvm/src/syscalls/mod.rs index a368abe9b..440ea6f0f 100644 --- a/fvm/src/syscalls/mod.rs +++ b/fvm/src/syscalls/mod.rs @@ -5,7 +5,6 @@ use cid::Cid; use wasmtime::{AsContextMut, Global, Linker, Memory, Val}; use crate::call_manager::backtrace; -use crate::kernel::ExecutionError; use crate::Kernel; pub(crate) mod error; @@ -62,6 +61,7 @@ pub fn update_gas_available( Ok(()) } +/// Updates the FVM-side gas tracker with newly accrued execution gas charges. pub fn charge_for_exec( ctx: &mut impl AsContextMut>, ) -> Result<(), Abort> { @@ -74,7 +74,7 @@ pub fn charge_for_exec( .context("failed to get wasm gas") .map_err(Abort::Fatal)?; - // Determine milligas used, and update the "o + // Determine milligas used, and update the gas tracker. let milligas_used = { let data = ctx.data_mut(); let last_milligas = mem::replace(&mut data.last_milligas_available, milligas_available); @@ -85,11 +85,8 @@ pub fn charge_for_exec( ctx.data_mut() .kernel .charge_milligas("wasm_exec", milligas_used) - .map_err(|e| match e { - ExecutionError::OutOfGas => Abort::OutOfGas, - ExecutionError::Fatal(e) => Abort::Fatal(e), - ExecutionError::Syscall(e) => Abort::Fatal(anyhow!("unexpected syscall error: {}", e)), - })?; + .map_err(Abort::from_error_as_fatal)?; + Ok(()) } diff --git a/testing/conformance/src/vm.rs b/testing/conformance/src/vm.rs index 9d6471f06..ccf9fec70 100644 --- a/testing/conformance/src/vm.rs +++ b/testing/conformance/src/vm.rs @@ -426,14 +426,14 @@ where // NOT forwarded fn verify_seal(&mut self, vi: &SealVerifyInfo) -> Result { let charge = self.1.price_list.on_verify_seal(vi); - self.0.charge_gas(charge.name, charge.total())?; + self.0.charge_milligas(charge.name, charge.total())?; Ok(true) } // NOT forwarded fn verify_post(&mut self, vi: &WindowPoStVerifyInfo) -> Result { let charge = self.1.price_list.on_verify_post(vi); - self.0.charge_gas(charge.name, charge.total())?; + self.0.charge_milligas(charge.name, charge.total())?; Ok(true) } @@ -445,7 +445,7 @@ where _extra: &[u8], ) -> Result> { let charge = self.1.price_list.on_verify_consensus_fault(); - self.0.charge_gas(charge.name, charge.total())?; + self.0.charge_milligas(charge.name, charge.total())?; // TODO this seems wrong, should probably be parameterized. Ok(None) } @@ -453,14 +453,14 @@ where // NOT forwarded fn verify_aggregate_seals(&mut self, agg: &AggregateSealVerifyProofAndInfos) -> Result { let charge = self.1.price_list.on_verify_aggregate_seals(agg); - self.0.charge_gas(charge.name, charge.total())?; + self.0.charge_milligas(charge.name, charge.total())?; Ok(true) } // NOT forwarded fn verify_replica_update(&mut self, rep: &ReplicaUpdateInfo) -> Result { let charge = self.1.price_list.on_verify_replica_update(rep); - self.0.charge_gas(charge.name, charge.total())?; + self.0.charge_milligas(charge.name, charge.total())?; Ok(true) } } @@ -490,8 +490,8 @@ where self.0.gas_used() } - fn charge_gas(&mut self, name: &str, compute: i64) -> Result<()> { - self.0.charge_gas(name, compute) + fn charge_milligas(&mut self, name: &str, compute: i64) -> Result<()> { + self.0.charge_milligas(name, compute) } fn price_list(&self) -> &PriceList { @@ -509,10 +509,6 @@ where fn milligas_available(&self) -> i64 { self.0.milligas_available() } - - fn charge_milligas(&mut self, name: &str, compute: i64) -> Result<()> { - self.0.charge_milligas(name, compute) - } } impl MessageOps for TestKernel From 26c6f4c2248d99d4e7583478b4f365ddd9a9d2ae Mon Sep 17 00:00:00 2001 From: Volker Mische Date: Tue, 10 May 2022 19:50:47 +0100 Subject: [PATCH 12/21] chore: use a specific Rust nightly version (#541) --- ipld/amt/benches/amt_benchmark.rs | 4 +--- rust-toolchain | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/ipld/amt/benches/amt_benchmark.rs b/ipld/amt/benches/amt_benchmark.rs index 54b5c5e7f..773e778ef 100644 --- a/ipld/amt/benches/amt_benchmark.rs +++ b/ipld/amt/benches/amt_benchmark.rs @@ -30,13 +30,11 @@ impl Serialize for BenchData { } impl<'de> Deserialize<'de> for BenchData { - #[allow(clippy::let_unit_value)] fn deserialize(deserializer: D) -> Result>::Error> where D: Deserializer<'de>, { - let _s: () = Deserialize::deserialize(deserializer)?; - + Deserialize::deserialize(deserializer)?; Ok(Self::default()) } } diff --git a/rust-toolchain b/rust-toolchain index 07ade694b..67d1f3fd0 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1 +1 @@ -nightly \ No newline at end of file +nightly-2022-05-09 From c4f4acbe4f028a341f164d149e36d8ef5cb5617d Mon Sep 17 00:00:00 2001 From: Volker Mische Date: Wed, 11 May 2022 22:36:09 +0100 Subject: [PATCH 13/21] fix: make sure bitfield is within limits (#535) Error if one tries to decode a bitfield that is too large. --- ipld/bitfield/src/lib.rs | 6 +++ ipld/bitfield/src/rleplus/mod.rs | 8 +--- ipld/bitfield/src/unvalidated.rs | 8 +++- ipld/bitfield/tests/bitfield_tests.rs | 66 ++++++++++++++++++++++++++- 4 files changed, 79 insertions(+), 9 deletions(-) diff --git a/ipld/bitfield/src/lib.rs b/ipld/bitfield/src/lib.rs index 6935c50a6..4e04a9f78 100644 --- a/ipld/bitfield/src/lib.rs +++ b/ipld/bitfield/src/lib.rs @@ -21,6 +21,12 @@ pub use rleplus::Error; use thiserror::Error; pub use unvalidated::{UnvalidatedBitField, Validate}; +/// MaxEncodedSize is the maximum encoded size of a bitfield. When expanded into +/// a slice of runs, a bitfield of this size should not exceed 2MiB of memory. +/// +/// This bitfield can fit at least 3072 sparse elements. +pub(crate) const MAX_ENCODED_SIZE: usize = 32 << 10; + #[derive(Clone, Error, Debug)] #[error("bitfields may not include u64::MAX")] pub struct OutOfRangeError; diff --git a/ipld/bitfield/src/rleplus/mod.rs b/ipld/bitfield/src/rleplus/mod.rs index f3bc0e35c..dfd8539d5 100644 --- a/ipld/bitfield/src/rleplus/mod.rs +++ b/ipld/bitfield/src/rleplus/mod.rs @@ -75,13 +75,7 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; pub use writer::BitWriter; use super::BitField; -use crate::RangeSize; - -// MaxEncodedSize is the maximum encoded size of a bitfield. When expanded into -// a slice of runs, a bitfield of this size should not exceed 2MiB of memory. -// -// This bitfield can fit at least 3072 sparse elements. -const MAX_ENCODED_SIZE: usize = 32 << 10; +use crate::{RangeSize, MAX_ENCODED_SIZE}; impl Serialize for BitField { fn serialize(&self, serializer: S) -> std::result::Result diff --git a/ipld/bitfield/src/unvalidated.rs b/ipld/bitfield/src/unvalidated.rs index d2412f6e7..f29c0c9e2 100644 --- a/ipld/bitfield/src/unvalidated.rs +++ b/ipld/bitfield/src/unvalidated.rs @@ -7,7 +7,7 @@ use fvm_ipld_encoding::serde_bytes; use serde::{Deserialize, Deserializer, Serialize}; use super::BitField; -use crate::Error; +use crate::{Error, MAX_ENCODED_SIZE}; /// A trait for types that can produce a `&BitField` (or fail to do so). /// Generalizes over `&BitField` and `&mut UnvalidatedBitField`. @@ -77,6 +77,12 @@ impl<'de> Deserialize<'de> for UnvalidatedBitField { D: Deserializer<'de>, { let bytes: Vec = serde_bytes::deserialize(deserializer)?; + if bytes.len() > MAX_ENCODED_SIZE { + return Err(serde::de::Error::custom(format!( + "encoded bitfield was too large {}", + bytes.len() + ))); + } Ok(Self::Unvalidated(bytes)) } } diff --git a/ipld/bitfield/tests/bitfield_tests.rs b/ipld/bitfield/tests/bitfield_tests.rs index 4dab4bd55..ee0df4553 100644 --- a/ipld/bitfield/tests/bitfield_tests.rs +++ b/ipld/bitfield/tests/bitfield_tests.rs @@ -3,7 +3,7 @@ use std::collections::HashSet; -use fvm_ipld_bitfield::{bitfield, BitField}; +use fvm_ipld_bitfield::{bitfield, BitField, UnvalidatedBitField}; use rand::{Rng, SeedableRng}; use rand_xorshift::XorShiftRng; @@ -232,3 +232,67 @@ fn exceeds_bitfield_range() { BitField::try_from_bits([0, 1, 4, 99, u64::MAX - 1]) .expect("expected setting u64::MAX-1 to succeed"); } + +#[test] +fn bitfield_custom() { + let mut bf = BitField::new(); + + // Set alternating bits for worst-case size performance + let mut i = 0; + while i < 1_000_000 { + bf.set(i); + i += 2; + } + println!("# Set bits: {}", bf.len()); + + // Standard serialization catches MAX_ENCODING_SIZE issues + println!("Attempting to serialize..."); + match fvm_ipld_encoding::to_vec(&bf) { + Ok(_) => panic!("This should have failed!"), + Err(_) => println!("Standard serialization failed, as expected"), + } + + // Bypass to_vec enc size check so we can test deserialization + println!("Manually serializing..."); + // CBOR prefix for the bytes + let mut cbor = vec![0x5A, 0x00, 0x01, 0xE8, 0x49]; + cbor.extend_from_slice(&bf.to_bytes()); + println!("Success!"); + + println!("# bytes of cbor: {}", cbor.len()); + println!("Header: {:#010b}", cbor[0]); + println!("-- maj type {}", (cbor[0] & 0xe0) >> 5); + + // Get size of payload size + let info = cbor[0] & 0x1f; + println!("-- adtl info {}", info); + + // Get payload size + let size = match info { + 0..=23 => info as usize, + 24 => cbor[1] as usize, + 25 => u16::from_be_bytes([cbor[1], cbor[2]]) as usize, + 26 => u32::from_be_bytes([cbor[1], cbor[2], cbor[3], cbor[4]]) as usize, + 27 => u64::from_be_bytes([ + cbor[1], cbor[2], cbor[3], cbor[4], cbor[5], cbor[6], cbor[7], cbor[8], + ]) as usize, + _ => { + println!("OUT OF RANGE"); + 0 + } + }; + + println!("{} byte payload", size); + + // Deserialize and validate malicious payload + println!("Attempting to deserialize and validate..."); + match fvm_ipld_encoding::from_slice::(&cbor) { + Ok(mut bitfield) => { + bitfield.validate_mut().unwrap(); + panic!("Error - deserialized/validated payload over 32768 bytes."); + } + Err(_) => { + println!("Success - payload over 32768 bytes cannot be deserialized"); + } + } +} From 8d4d002dfef367f915388c4a157d6d751d378aae Mon Sep 17 00:00:00 2001 From: raulk Date: Wed, 11 May 2022 23:03:51 +0000 Subject: [PATCH 14/21] transmute fatal errors into `SYS_ASSERTION_FAILED` exit code. (#548) --- fvm/src/call_manager/backtrace.rs | 96 ++++++++--- fvm/src/call_manager/default.rs | 2 +- fvm/src/executor/default.rs | 26 ++- fvm/src/machine/default.rs | 3 + fvm/src/syscalls/bind.rs | 6 +- testing/integration/examples/integration.rs | 8 +- testing/integration/src/builtin.rs | 6 +- testing/integration/src/tester.rs | 59 ++++--- testing/integration/tests/lib.rs | 170 +++++++++++++++++++- 9 files changed, 312 insertions(+), 64 deletions(-) diff --git a/fvm/src/call_manager/backtrace.rs b/fvm/src/call_manager/backtrace.rs index fd5daa138..c45a58033 100644 --- a/fvm/src/call_manager/backtrace.rs +++ b/fvm/src/call_manager/backtrace.rs @@ -7,7 +7,9 @@ use fvm_shared::{ActorID, MethodNum}; use crate::kernel::SyscallError; -/// A call backtrace records _why_ an actor exited with a specific error code. +/// A call backtrace records the actors an error was propagated through, from +/// the moment it was emitted. The original error is the _cause_. Backtraces are +/// useful for identifying the root cause of an error. #[derive(Debug, Default, Clone)] pub struct Backtrace { /// The actors through which this error was propagated from bottom (source) to top. @@ -34,22 +36,35 @@ impl Backtrace { self.frames.is_empty() && self.cause.is_none() } - /// Clear the backtrace. This should be called: - /// - /// 1. Before all syscalls except "abort" - /// 2. After an actor returns with a 0 exit code. + /// Clear the backtrace. pub fn clear(&mut self) { self.cause = None; self.frames.clear(); } - /// Set the backtrace cause. If there is an existing backtrace, this will clear it. - pub fn set_cause(&mut self, cause: Cause) { + /// Begins a new backtrace. If there is an existing backtrace, this will clear it. + /// + /// Note: Backtraces are populated _backwards_. That is, a frame is inserted + /// every time an actor returns. That's why `begin()` resets any currently + /// accumulated state, as once an error occurs, we want to track its + /// propagation all the way up. + pub fn begin(&mut self, cause: Cause) { self.cause = Some(cause); self.frames.clear(); } + /// Sets the cause of a backtrace. + /// + /// This is useful to stamp a backtrace with its cause after the frames + /// have been collected, such as when we ultimately handle a fatal error at + /// the top of its propagation chain. + pub fn set_cause(&mut self, cause: Cause) { + self.cause = Some(cause); + } + /// Push a "frame" (actor exit) onto the backtrace. + /// + /// This should be called every time an actor exits. pub fn push_frame(&mut self, frame: Frame) { self.frames.push(frame) } @@ -85,34 +100,69 @@ impl Display for Frame { /// The ultimate "cause" of a failed message. #[derive(Clone, Debug)] -pub struct Cause { - /// The syscall "module". - pub module: &'static str, - /// The syscall function name. - pub function: &'static str, - /// The exact syscall error. - pub error: ErrorNumber, - /// The informational syscall message. - pub message: String, +pub enum Cause { + /// The original cause was a syscall error. + Syscall { + /// The syscall "module". + module: &'static str, + /// The syscall function name. + function: &'static str, + /// The exact syscall error. + error: ErrorNumber, + /// The informational syscall message. + message: String, + }, + /// The original cause was a fatal error. + Fatal { + /// The alternate-formatted message from the anyhow error. + error_msg: String, + /// The backtrace, captured if the relevant + /// [environment variables](https://doc.rust-lang.org/std/backtrace/index.html#environment-variables) are enabled. + backtrace: String, + }, } impl Cause { - pub fn new(module: &'static str, function: &'static str, err: SyscallError) -> Self { - Self { + /// Records a failing syscall as the cause of a backtrace. + pub fn from_syscall(module: &'static str, function: &'static str, err: SyscallError) -> Self { + Self::Syscall { module, function, error: err.1, message: err.0, } } + + /// Records a fatal error as the cause of a backtrace. + pub fn from_fatal(err: anyhow::Error) -> Self { + Self::Fatal { + error_msg: format!("{:#}", err), + backtrace: err.backtrace().to_string(), + } + } } impl Display for Cause { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!( - f, - "{}::{} -- {} ({}: {})", - self.module, self.function, &self.message, self.error as u32, self.error, - ) + match self { + Cause::Syscall { + module, + function, + error, + message, + } => { + write!( + f, + "{}::{} -- {} ({}: {})", + module, function, &message, *error as u32, error, + ) + } + Cause::Fatal { + error_msg, + backtrace, + } => { + write!(f, "[FATAL] Error: {}, Backtrace:\n{}", error_msg, backtrace) + } + } } } diff --git a/fvm/src/call_manager/default.rs b/fvm/src/call_manager/default.rs index 793ce1f01..c5c450e6c 100644 --- a/fvm/src/call_manager/default.rs +++ b/fvm/src/call_manager/default.rs @@ -393,7 +393,7 @@ where Ok(value) => Ok(InvocationResult::Return(value)), Err(abort) => { if let Some(err) = last_error { - cm.backtrace.set_cause(err); + cm.backtrace.begin(err); } let (code, message, res) = match abort { diff --git a/fvm/src/executor/default.rs b/fvm/src/executor/default.rs index ea0a605ac..e4cfa64c8 100644 --- a/fvm/src/executor/default.rs +++ b/fvm/src/executor/default.rs @@ -130,22 +130,36 @@ where ErrorNumber::Forbidden => ExitCode::SYS_ASSERTION_FAILED, }; - backtrace.set_cause(backtrace::Cause::new("send", "send", err)); + backtrace.begin(backtrace::Cause::from_syscall("send", "send", err)); Receipt { exit_code, return_data: Default::default(), gas_used, } } - Err(ExecutionError::Fatal(e)) => { - return Err(e.context(format!( - "[from={}, to={}, seq={}, m={}, h={}] fatal error", + Err(ExecutionError::Fatal(err)) => { + // We produce a receipt with SYS_ASSERTION_FAILED exit code, and + // we consume the full gas amount so that, in case of a network- + // wide fatal errors, all nodes behave deterministically. + // + // We set the backtrace from the fatal error to aid diagnosis. + // Note that we use backtrace#set_cause instead of backtrace#begin + // because we want to retain the propagation chain that we've + // accumulated on the way out. + let err = err.context(format!( + "[from={}, to={}, seq={}, m={}, h={}]", msg.from, msg.to, msg.sequence, msg.method_num, - self.context().epoch - ))); + self.context().epoch, + )); + backtrace.set_cause(backtrace::Cause::from_fatal(err)); + Receipt { + exit_code: ExitCode::SYS_ASSERTION_FAILED, + return_data: Default::default(), + gas_used: msg.gas_limit, + } } }; diff --git a/fvm/src/machine/default.rs b/fvm/src/machine/default.rs index 270e84037..82a586450 100644 --- a/fvm/src/machine/default.rs +++ b/fvm/src/machine/default.rs @@ -113,6 +113,9 @@ where // This interface works for now because we know all actor CIDs // ahead of time, but with user-supplied code, we won't have that // guarantee. + // Skip preloading all builtin actors when testing. This results in JIT + // bytecode to machine code compilation, and leads to faster tests. + #[cfg(not(feature = "testing"))] engine.preload(state_tree.store(), builtin_actors.left_values())?; Ok(DefaultMachine { diff --git a/fvm/src/syscalls/bind.rs b/fvm/src/syscalls/bind.rs index 62889b91a..5d93424d5 100644 --- a/fvm/src/syscalls/bind.rs +++ b/fvm/src/syscalls/bind.rs @@ -141,7 +141,7 @@ macro_rules! impl_bind_syscalls { Ok(Err(err)) => { let code = err.1; log::trace!("syscall {}::{}: fail ({})", module, name, code as u32); - data.last_error = Some(backtrace::Cause::new(module, name, err)); + data.last_error = Some(backtrace::Cause::from_syscall(module, name, err)); Ok(code as u32) }, Err(e) => Err(e.into()), @@ -163,7 +163,7 @@ macro_rules! impl_bind_syscalls { if (ret as u64) > (memory.len() as u64) || memory.len() - (ret as usize) < mem::size_of::() { let code = ErrorNumber::IllegalArgument; - data.last_error = Some(backtrace::Cause::new(module, name, SyscallError(format!("no space for return value"), code))); + data.last_error = Some(backtrace::Cause::from_syscall(module, name, SyscallError(format!("no space for return value"), code))); return Ok(code as u32); } @@ -178,7 +178,7 @@ macro_rules! impl_bind_syscalls { Ok(Err(err)) => { let code = err.1; log::trace!("syscall {}::{}: fail ({})", module, name, code as u32); - data.last_error = Some(backtrace::Cause::new(module, name, err)); + data.last_error = Some(backtrace::Cause::from_syscall(module, name, err)); Ok(code as u32) }, Err(e) => Err(e.into()), diff --git a/testing/integration/examples/integration.rs b/testing/integration/examples/integration.rs index fc61b4046..485fbe3a4 100644 --- a/testing/integration/examples/integration.rs +++ b/testing/integration/examples/integration.rs @@ -1,5 +1,6 @@ use fvm::executor::{ApplyKind, Executor}; use fvm_integration_tests::tester::{Account, Tester}; +use fvm_ipld_blockstore::MemoryBlockstore; use fvm_ipld_encoding::tuple::*; use fvm_shared::address::Address; use fvm_shared::bigint::BigInt; @@ -25,7 +26,12 @@ struct State { pub fn main() { // Instantiate tester - let mut tester = Tester::new(NetworkVersion::V15, StateTreeVersion::V4).unwrap(); + let mut tester = Tester::new( + NetworkVersion::V15, + StateTreeVersion::V4, + MemoryBlockstore::default(), + ) + .unwrap(); let sender: [Account; 1] = tester.create_accounts().unwrap(); diff --git a/testing/integration/src/builtin.rs b/testing/integration/src/builtin.rs index 9e2ae32df..fef8a3aba 100644 --- a/testing/integration/src/builtin.rs +++ b/testing/integration/src/builtin.rs @@ -5,7 +5,7 @@ use cid::Cid; use futures::executor::block_on; use fvm::state_tree::{ActorState, StateTree}; use fvm::{init_actor, system_actor}; -use fvm_ipld_blockstore::{Blockstore, MemoryBlockstore}; +use fvm_ipld_blockstore::Blockstore; use fvm_ipld_car::load_car; use fvm_ipld_encoding::CborStore; use fvm_shared::actor::builtin::{load_manifest, Type}; @@ -24,7 +24,7 @@ const BUNDLES: [(NetworkVersion, &[u8]); 3] = [ // Import built-in actors pub fn import_builtin_actors( - blockstore: &MemoryBlockstore, + blockstore: &impl Blockstore, ) -> Result> { BUNDLES .into_iter() @@ -40,7 +40,7 @@ pub fn import_builtin_actors( // Retrieve system, init and accounts actors code CID pub fn fetch_builtin_code_cid( - blockstore: &MemoryBlockstore, + blockstore: &impl Blockstore, builtin_actors: &Cid, ver: u32, ) -> Result<(Cid, Cid, Cid)> { diff --git a/testing/integration/src/tester.rs b/testing/integration/src/tester.rs index 30918a116..602990d59 100644 --- a/testing/integration/src/tester.rs +++ b/testing/integration/src/tester.rs @@ -5,7 +5,7 @@ use fvm::executor::DefaultExecutor; use fvm::machine::{DefaultMachine, Engine, Machine, NetworkConfig}; use fvm::state_tree::{ActorState, StateTree}; use fvm::{init_actor, system_actor, DefaultKernel}; -use fvm_ipld_blockstore::{Block, Blockstore, MemoryBlockstore}; +use fvm_ipld_blockstore::{Block, Blockstore}; use fvm_ipld_encoding::{ser, CborStore}; use fvm_ipld_hamt::Hamt; use fvm_shared::address::Address; @@ -24,15 +24,14 @@ use crate::error::Error::{FailedToFlushTree, NoManifestInformation, NoRootCid}; const DEFAULT_BASE_FEE: u64 = 100; -trait Store: Blockstore + Sized {} +pub trait Store: Blockstore + Sized + 'static {} -pub type IntegrationExecutor = DefaultExecutor< - DefaultKernel>>, ->; +pub type IntegrationExecutor = + DefaultExecutor>>>; pub type Account = (ActorID, Address); -pub struct Tester { +pub struct Tester { // Network version used in the test nv: NetworkVersion, // Builtin actors root Cid used in the Machine @@ -42,16 +41,16 @@ pub struct Tester { // Custom code cid deployed by developer code_cids: Vec, // Executor used to interact with deployed actors. - pub executor: Option, + pub executor: Option>, // State tree constructed before instantiating the Machine - pub state_tree: StateTree, + pub state_tree: Option>, } -impl Tester { - pub fn new(nv: NetworkVersion, stv: StateTreeVersion) -> Result { - // Initialize blockstore - let blockstore = MemoryBlockstore::default(); - +impl Tester +where + B: Blockstore, +{ + pub fn new(nv: NetworkVersion, stv: StateTreeVersion, blockstore: B) -> Result { // Load the builtin actors bundles into the blockstore. let nv_actors = import_builtin_actors(&blockstore)?; @@ -92,7 +91,7 @@ impl Tester { builtin_actors, executor: None, code_cids: vec![], - state_tree, + state_tree: Some(state_tree), accounts_code_cid, }) } @@ -100,13 +99,18 @@ impl Tester { /// Creates new accounts in the testing context pub fn create_accounts(&mut self) -> Result<[Account; N]> { // Create accounts. - put_secp256k1_accounts(&mut self.state_tree, self.accounts_code_cid) + put_secp256k1_accounts(self.state_tree.as_mut().unwrap(), self.accounts_code_cid) } /// Set a new state in the state tree pub fn set_state(&mut self, state: &S) -> Result { // Put state in tree - let state_cid = self.state_tree.store().put_cbor(state, Code::Blake2b256)?; + let state_cid = self + .state_tree + .as_mut() + .unwrap() + .store() + .put_cbor(state, Code::Blake2b256)?; Ok(state_cid) } @@ -121,11 +125,13 @@ impl Tester { ) -> Result<()> { // Register actor address self.state_tree + .as_mut() + .unwrap() .register_new_address(&actor_address) .unwrap(); // Put the WASM code into the blockstore. - let code_cid = put_wasm_code(self.state_tree.store(), wasm_bin)?; + let code_cid = put_wasm_code(self.state_tree.as_mut().unwrap().store(), wasm_bin)?; // Add code cid to list of deployed contract self.code_cids.push(code_cid); @@ -135,6 +141,8 @@ impl Tester { // Create actor self.state_tree + .as_mut() + .unwrap() .set_actor(&actor_address, actor_state) .map_err(anyhow::Error::from)?; @@ -143,14 +151,17 @@ impl Tester { /// Sets the Machine and the Executor in our Tester structure. pub fn instantiate_machine(&mut self) -> Result<()> { - // First flush tree and consume it - let state_root = self - .state_tree + // Take the state tree and leave None behind. + let mut state_tree = self.state_tree.take().unwrap(); + + // Calculate the state root. + let state_root = state_tree .flush() .map_err(anyhow::Error::from) .context(FailedToFlushTree)?; - let blockstore = self.state_tree.store(); + // Consume the state tree and take the blockstore. + let blockstore = state_tree.into_store(); let mut nc = NetworkConfig::new(self.nv); nc.override_actors(self.builtin_actors); @@ -161,7 +172,7 @@ impl Tester { let machine = DefaultMachine::new( &Engine::new_default((&mc.network.clone()).into())?, &mc, - blockstore.clone(), + blockstore, dummy::DummyExterns, )?; @@ -180,7 +191,7 @@ impl Tester { if self.executor.is_some() { self.executor.as_ref().unwrap().blockstore() } else { - self.state_tree.store() + self.state_tree.as_ref().unwrap().store() } } } @@ -224,7 +235,7 @@ fn put_secp256k1_accounts( } /// Inserts the WASM code for the actor into the blockstore. -fn put_wasm_code(blockstore: &MemoryBlockstore, wasm_binary: &[u8]) -> Result { +fn put_wasm_code(blockstore: &impl Blockstore, wasm_binary: &[u8]) -> Result { let cid = blockstore.put( Code::Blake2b256, &Block { diff --git a/testing/integration/tests/lib.rs b/testing/integration/tests/lib.rs index 328e90d5c..d87c10504 100644 --- a/testing/integration/tests/lib.rs +++ b/testing/integration/tests/lib.rs @@ -1,14 +1,21 @@ +use std::cell::RefCell; +use std::collections::HashSet; use std::env; +use cid::multihash::Multihash; +use cid::Cid; use fvm::executor::{ApplyKind, Executor}; use fvm_integration_tests::tester::{Account, Tester}; +use fvm_ipld_blockstore::{Blockstore, MemoryBlockstore}; use fvm_ipld_encoding::tuple::*; +use fvm_ipld_encoding::DAG_CBOR; use fvm_shared::address::Address; use fvm_shared::bigint::BigInt; use fvm_shared::error::ExitCode; use fvm_shared::message::Message; use fvm_shared::state::StateTreeVersion; use fvm_shared::version::NetworkVersion; +use fvm_shared::IDENTITY_HASH; use num_traits::Zero; use wabt::wat2wasm; @@ -24,7 +31,12 @@ const WASM_COMPILED_PATH: &str = #[test] fn hello_world() { // Instantiate tester - let mut tester = Tester::new(NetworkVersion::V15, StateTreeVersion::V4).unwrap(); + let mut tester = Tester::new( + NetworkVersion::V15, + StateTreeVersion::V4, + MemoryBlockstore::default(), + ) + .unwrap(); let sender: [Account; 1] = tester.create_accounts().unwrap(); @@ -84,7 +96,12 @@ fn out_of_gas() { "#; // Instantiate tester - let mut tester = Tester::new(NetworkVersion::V16, StateTreeVersion::V4).unwrap(); + let mut tester = Tester::new( + NetworkVersion::V16, + StateTreeVersion::V4, + MemoryBlockstore::default(), + ) + .unwrap(); let sender: [Account; 1] = tester.create_accounts().unwrap(); @@ -143,7 +160,12 @@ fn out_of_stack() { "#; // Instantiate tester - let mut tester = Tester::new(NetworkVersion::V16, StateTreeVersion::V4).unwrap(); + let mut tester = Tester::new( + NetworkVersion::V16, + StateTreeVersion::V4, + MemoryBlockstore::default(), + ) + .unwrap(); let sender: [Account; 1] = tester.create_accounts().unwrap(); @@ -181,3 +203,145 @@ fn out_of_stack() { assert_eq!(res.msg_receipt.exit_code, ExitCode::SYS_ILLEGAL_INSTRUCTION) } + +#[test] +fn backtraces() { + // Note: this test **does not actually assert anything**, but it's useful to + // let us peep into FVM backtrace generation under different scenarios. + const WAT_ABORT: &str = r#" + (module + ;; ipld::open + (type (;0;) (func (param i32 i32) (result i32))) + (import "ipld" "open" (func $fvm_sdk::sys::ipld::open::syscall (type 0))) + ;; vm::abort + (type (;1;) (func (param i32 i32 i32) (result i32))) + (import "vm" "abort" (func $fvm_sdk::sys::vm::abort::syscall (type 1))) + (memory (export "memory") 1) + (func (export "invoke") (param $x i32) (result i32) + (i32.const 123) + (i32.const 123) + (call $fvm_sdk::sys::ipld::open::syscall) + (i32.const 0) + (i32.const 0) + (call $fvm_sdk::sys::vm::abort::syscall) + unreachable + ) + ) + "#; + + const WAT_FATAL: &str = r#" + (module + ;; ipld::open + (type (;0;) (func (param i32 i32) (result i32))) + (import "ipld" "open" (func $fvm_sdk::sys::ipld::open::syscall (type 0))) + ;; vm::abort + (type (;1;) (func (param i32 i32 i32) (result i32))) + (import "vm" "abort" (func $fvm_sdk::sys::vm::abort::syscall (type 1))) + (memory (export "memory") 1) + (func (export "invoke") (param $x i32) (result i32) + (i32.const 128) + (memory.grow) + (i32.const 4) + (i32.const 25493505) + (i32.store) + (i32.const 8) + (i32.const 0) + (i32.store) + (i32.const 4) + (call $fvm_sdk::sys::ipld::open::syscall) + (i32.const 0) + (i32.const 0) + (call $fvm_sdk::sys::vm::abort::syscall) + unreachable + ) + ) + "#; + + let blockstore = FailingBlockstore::default(); + let identity_cid = Cid::new_v1(DAG_CBOR, Multihash::wrap(IDENTITY_HASH, &[0]).unwrap()); + blockstore.add_fail(identity_cid); + + // Instantiate tester + let mut tester = Tester::new( + NetworkVersion::V16, + StateTreeVersion::V4, + MemoryBlockstore::default(), + ) + .unwrap(); + + let sender: [Account; 1] = tester.create_accounts().unwrap(); + + let state_cid = tester.set_state(&State { count: 0 }).unwrap(); + + // Set an actor that aborts. + let (wasm_abort, wasm_fatal) = (wat2wasm(WAT_ABORT).unwrap(), wat2wasm(WAT_FATAL).unwrap()); + let (abort_address, fatal_address) = (Address::new_id(10000), Address::new_id(10001)); + tester + .set_actor_from_bin(&wasm_abort, state_cid, abort_address, BigInt::zero()) + .unwrap(); + tester + .set_actor_from_bin(&wasm_fatal, state_cid, fatal_address, BigInt::zero()) + .unwrap(); + + // Instantiate machine + tester.instantiate_machine().unwrap(); + + // Send message + let message = Message { + from: sender[0].1, + to: abort_address, + gas_limit: 10_000_000, + method_num: 1, + ..Message::default() + }; + + let res = tester + .executor + .as_mut() + .unwrap() + .execute_message(message, ApplyKind::Explicit, 100) + .unwrap(); + + println!("abort backtrace: {}", res.failure_info.unwrap()); + + // Send message + let message = Message { + from: sender[0].1, + to: fatal_address, + gas_limit: 10_000_000, + method_num: 1, + sequence: 1, + ..Message::default() + }; + + let res = tester + .executor + .as_mut() + .unwrap() + .execute_message(message, ApplyKind::Explicit, 100) + .unwrap(); + + println!("fatal backtrace: {}", res.failure_info.unwrap()); +} + +#[derive(Default)] +pub struct FailingBlockstore { + fail_for: RefCell>, + target: MemoryBlockstore, +} + +impl FailingBlockstore { + pub fn add_fail(&self, cid: Cid) { + self.fail_for.borrow_mut().insert(cid); + } +} + +impl Blockstore for FailingBlockstore { + fn get(&self, k: &Cid) -> anyhow::Result>> { + self.target.get(k) + } + + fn put_keyed(&self, k: &Cid, block: &[u8]) -> anyhow::Result<()> { + self.target.put_keyed(k, block) + } +} From e5e147a150a2462681f588ccf1343314b4a1ae62 Mon Sep 17 00:00:00 2001 From: raulk Date: Fri, 13 May 2022 15:50:26 +0000 Subject: [PATCH 15/21] remove support for nv14 and actors v6. (#556) --- fvm/src/gas/price_list.rs | 2 +- fvm/src/kernel/default.rs | 67 ++--------------------- fvm/src/lib.rs | 6 +- fvm/src/machine/default.rs | 2 +- fvm/src/market_actor.rs | 93 ------------------------------- fvm/src/power_actor.rs | 87 ----------------------------- fvm/src/reward_actor.rs | 94 -------------------------------- testing/conformance/Cargo.toml | 1 - testing/conformance/src/vm.rs | 5 +- testing/conformance/test-vectors | 2 +- 10 files changed, 9 insertions(+), 350 deletions(-) delete mode 100644 fvm/src/market_actor.rs delete mode 100644 fvm/src/power_actor.rs delete mode 100644 fvm/src/reward_actor.rs diff --git a/fvm/src/gas/price_list.rs b/fvm/src/gas/price_list.rs index 3744c6a94..05669c071 100644 --- a/fvm/src/gas/price_list.rs +++ b/fvm/src/gas/price_list.rs @@ -694,7 +694,7 @@ impl PriceList { /// Returns gas price list by NetworkVersion for gas consumption. pub fn price_list_by_network_version(network_version: NetworkVersion) -> &'static PriceList { match network_version { - NetworkVersion::V14 | NetworkVersion::V15 => &OH_SNAP_PRICES, + NetworkVersion::V15 => &OH_SNAP_PRICES, _ => &SKYR_PRICES, } } diff --git a/fvm/src/kernel/default.rs b/fvm/src/kernel/default.rs index c59fcc6bb..6b24a1220 100644 --- a/fvm/src/kernel/default.rs +++ b/fvm/src/kernel/default.rs @@ -33,15 +33,9 @@ use super::*; use crate::call_manager::{CallManager, InvocationResult}; use crate::externs::{Consensus, Rand}; use crate::gas::GasCharge; -use crate::market_actor::State as MarketActorState; -use crate::power_actor::State as PowerActorState; -use crate::reward_actor::State as RewardActorState; use crate::state_tree::ActorState; use crate::{syscall_error, EMPTY_ARR_CID}; -pub const BURN_ACTOR_ID: ActorID = 99; -pub const RESERVE_ACTOR_ID: ActorID = 90; - lazy_static! { static ref NUM_CPUS: usize = num_cpus::get(); static ref INITIAL_RESERVE_BALANCE: BigInt = BigInt::from(300_000_000) * FILECOIN_PRECISION; @@ -160,45 +154,6 @@ where Ok(state.address) } - fn get_burnt_funds(&self) -> Result { - Ok(self - .call_manager - .state_tree() - .get_actor_id(BURN_ACTOR_ID)? - .context("burn actor state couldn't be loaded") - .or_fatal()? - .balance) - } - - fn get_reserve_disbursed(&self) -> Result { - let reserve_balance = self - .call_manager - .state_tree() - .get_actor_id(RESERVE_ACTOR_ID)? - .context("failed to load reserve actor when determining reserve disbursed") - .or_fatal()? - .balance; - Ok(&*INITIAL_RESERVE_BALANCE - reserve_balance) - } - - fn get_fil_mined(&self) -> Result { - let (reward_state, _) = RewardActorState::load(self.call_manager.state_tree()) - .context("failed to load reward actor state when getting FIL mined")?; - Ok(reward_state.total_storage_power_reward()) - } - - fn power_locked(&self) -> Result { - let (power_state, _) = PowerActorState::load(self.call_manager.state_tree()) - .context("failed to load power actor state when determining locked FIL")?; - Ok(power_state.total_locked()) - } - - fn market_locked(&self) -> Result { - let (market_state, _) = MarketActorState::load(self.call_manager.state_tree()) - .context("failed to load market actor state when determining locked FIL")?; - Ok(market_state.total_locked()) - } - /// Returns `Some(actor_state)` or `None` if this actor has been deleted. fn get_self(&self) -> Result> { self.call_manager @@ -432,24 +387,10 @@ where C: CallManager, { fn total_fil_circ_supply(&self) -> Result { - let circ_supply = if self.network_version() <= NetworkVersion::V14 { - // Pre-v15 the circ supply was dynamically calculated on the Filecoin mainnet, - // meaning it fluctuated within an epoch (as messages were executed). This forced the FVM - // to do much of the calculation in order to support v14. - (&self.call_manager.context().circ_supply - + &self.get_fil_mined()? - + &self.get_reserve_disbursed()? - - &self.get_burnt_funds()? - - &self.power_locked()? - - &self.market_locked()?) - .max(Zero::zero()) - } else { - // From v15 and onwards, Filecoin mainnet was fixed to use a static circ supply per epoch. - // The value reported to the FVM from clients is now the static value, - // the FVM simply reports that value to actors. - self.call_manager.context().circ_supply.clone() - }; - Ok(circ_supply) + // From v15 and onwards, Filecoin mainnet was fixed to use a static circ supply per epoch. + // The value reported to the FVM from clients is now the static value, + // the FVM simply reports that value to actors. + Ok(self.call_manager.context().circ_supply.clone()) } } diff --git a/fvm/src/lib.rs b/fvm/src/lib.rs index 705f8e67e..2a4d8fb2d 100644 --- a/fvm/src/lib.rs +++ b/fvm/src/lib.rs @@ -37,10 +37,6 @@ pub mod init_actor; #[cfg(feature = "testing")] pub mod system_actor; -mod market_actor; -mod power_actor; -mod reward_actor; - pub mod trace; use cid::multihash::{Code, MultihashDigest}; @@ -119,7 +115,7 @@ mod test { let actors_cid = bs.put_cbor(&(0, manifest_cid), Code::Blake2b256).unwrap(); - let mc = NetworkConfig::new(fvm_shared::version::NetworkVersion::V14) + let mc = NetworkConfig::new(fvm_shared::version::NetworkVersion::V15) .override_actors(actors_cid) .for_epoch(0, root); diff --git a/fvm/src/machine/default.rs b/fvm/src/machine/default.rs index 82a586450..42b0538b9 100644 --- a/fvm/src/machine/default.rs +++ b/fvm/src/machine/default.rs @@ -60,7 +60,7 @@ where externs: E, ) -> anyhow::Result { const SUPPORTED_VERSIONS: RangeInclusive = - NetworkVersion::V14..=NetworkVersion::V16; + NetworkVersion::V15..=NetworkVersion::V16; debug!( "initializing a new machine, epoch={}, base_fee={}, nv={:?}, root={}", diff --git a/fvm/src/market_actor.rs b/fvm/src/market_actor.rs deleted file mode 100644 index 957d025ea..000000000 --- a/fvm/src/market_actor.rs +++ /dev/null @@ -1,93 +0,0 @@ -//! This module contains the types and functions to process the market actor's state. -//! This ONLY exists to support the circulating supply calc for version <= 14. -//! -//! It should be removed as soon as the Filecoin network updates to v15. - -use anyhow::Context; -use cid::Cid; -use fvm_ipld_blockstore::Blockstore; -use fvm_ipld_encoding::tuple::*; -use fvm_ipld_encoding::{Cbor, CborStore}; -use fvm_shared::address::Address; -use fvm_shared::bigint::bigint_ser; -use fvm_shared::clock::ChainEpoch; -use fvm_shared::deal::DealID; -use fvm_shared::econ::TokenAmount; - -use crate::kernel::{ClassifyResult, Result}; -use crate::state_tree::{ActorState, StateTree}; - -pub const MARKET_ACTOR_ADDR: Address = Address::new_id(5); - -/// Market power actor state - -impl Cbor for State {} -#[derive(Clone, Default, Serialize_tuple, Deserialize_tuple)] -pub struct State { - /// Proposals are deals that have been proposed and not yet cleaned up after expiry or termination. - /// Array - pub proposals: Cid, - - // States contains state for deals that have been activated and not yet cleaned up after expiry or termination. - // After expiration, the state exists until the proposal is cleaned up too. - // Invariant: keys(States) ⊆ keys(Proposals). - /// Array - pub states: Cid, - - /// PendingProposals tracks dealProposals that have not yet reached their deal start date. - /// We track them here to ensure that miners can't publish the same deal proposal twice - pub pending_proposals: Cid, - - /// Total amount held in escrow, indexed by actor address (including both locked and unlocked amounts). - pub escrow_table: Cid, - - /// Amount locked, indexed by actor address. - /// Note: the amounts in this table do not affect the overall amount in escrow: - /// only the _portion_ of the total escrow amount that is locked. - pub locked_table: Cid, - - /// Deal id state sequential incrementer - pub next_id: DealID, - - /// Metadata cached for efficient iteration over deals. - /// SetMultimap
- pub deal_ops_by_epoch: Cid, - pub last_cron: ChainEpoch, - - /// Total Client Collateral that is locked -> unlocked when deal is terminated - #[serde(with = "bigint_ser")] - pub total_client_locked_colateral: TokenAmount, - /// Total Provider Collateral that is locked -> unlocked when deal is terminated - #[serde(with = "bigint_ser")] - pub total_provider_locked_colateral: TokenAmount, - /// Total storage fee that is locked in escrow -> unlocked when payments are made - #[serde(with = "bigint_ser")] - pub total_client_storage_fee: TokenAmount, -} - -impl State { - /// Loads the market actor state with the supplied CID from the underlying store. - pub fn load(state_tree: &StateTree) -> Result<(Self, ActorState)> - where - B: Blockstore, - { - let market_act = state_tree - .get_actor(&MARKET_ACTOR_ADDR)? - .context("Market actor address could not be resolved") - .or_fatal()?; - - let state = state_tree - .store() - .get_cbor(&market_act.state) - .or_fatal()? - .context("market actor state not found") - .or_fatal()?; - Ok((state, market_act)) - } - - pub fn total_locked(&self) -> TokenAmount { - &self.total_client_locked_colateral - + &self.total_provider_locked_colateral - + &self.total_client_storage_fee - } -} diff --git a/fvm/src/power_actor.rs b/fvm/src/power_actor.rs deleted file mode 100644 index 221a53387..000000000 --- a/fvm/src/power_actor.rs +++ /dev/null @@ -1,87 +0,0 @@ -//! This module contains the types and functions to process the power actor's state. -//! This ONLY exists to support the circulating supply calc for version <= 14. -//! -//! It should be removed as soon as the Filecoin network updates to v15. - -use anyhow::Context; -use cid::Cid; -use fvm_ipld_blockstore::Blockstore; -use fvm_ipld_encoding::tuple::*; -use fvm_ipld_encoding::{Cbor, CborStore}; -use fvm_shared::address::Address; -use fvm_shared::bigint::bigint_ser; -use fvm_shared::clock::ChainEpoch; -use fvm_shared::econ::TokenAmount; -use fvm_shared::sector::StoragePower; -use fvm_shared::smooth::FilterEstimate; - -use crate::kernel::{ClassifyResult, Result}; -use crate::state_tree::{ActorState, StateTree}; - -pub const POWER_ACTOR_ADDR: Address = Address::new_id(4); - -/// Storage power actor state -#[derive(Default, Serialize_tuple, Deserialize_tuple)] -pub struct State { - #[serde(with = "bigint_ser")] - pub total_raw_byte_power: StoragePower, - #[serde(with = "bigint_ser")] - pub total_bytes_committed: StoragePower, - #[serde(with = "bigint_ser")] - pub total_quality_adj_power: StoragePower, - #[serde(with = "bigint_ser")] - pub total_qa_bytes_committed: StoragePower, - #[serde(with = "bigint_ser")] - pub total_pledge_collateral: TokenAmount, - - #[serde(with = "bigint_ser")] - pub this_epoch_raw_byte_power: StoragePower, - #[serde(with = "bigint_ser")] - pub this_epoch_quality_adj_power: StoragePower, - #[serde(with = "bigint_ser")] - pub this_epoch_pledge_collateral: TokenAmount, - pub this_epoch_qa_power_smoothed: FilterEstimate, - - pub miner_count: i64, - /// Number of miners having proven the minimum consensus power. - pub miner_above_min_power_count: i64, - - /// A queue of events to be triggered by cron, indexed by epoch. - pub cron_event_queue: Cid, // Multimap, (HAMT[ChainEpoch]AMT[CronEvent] - - /// First epoch in which a cron task may be stored. Cron will iterate every epoch between this - /// and the current epoch inclusively to find tasks to execute. - pub first_cron_epoch: ChainEpoch, - - /// Claimed power for each miner. - pub claims: Cid, // Map, HAMT[address]Claim - - pub proof_validation_batch: Option, -} - -impl Cbor for State {} - -impl State { - /// Loads the power actor state with the supplied CID from the underlying store. - pub fn load(state_tree: &StateTree) -> Result<(Self, ActorState)> - where - B: Blockstore, - { - let power_act = state_tree - .get_actor(&POWER_ACTOR_ADDR)? - .context("Power actor address could not be resolved") - .or_fatal()?; - - let state = state_tree - .store() - .get_cbor(&power_act.state) - .or_fatal()? - .context("power actor state not found") - .or_fatal()?; - Ok((state, power_act)) - } - - pub fn total_locked(self) -> TokenAmount { - self.total_pledge_collateral - } -} diff --git a/fvm/src/reward_actor.rs b/fvm/src/reward_actor.rs deleted file mode 100644 index 7e9472bdf..000000000 --- a/fvm/src/reward_actor.rs +++ /dev/null @@ -1,94 +0,0 @@ -use anyhow::Context; -use fvm_ipld_blockstore::Blockstore; -use fvm_ipld_encoding::tuple::*; -use fvm_ipld_encoding::{Cbor, CborStore}; -use fvm_shared::bigint::bigint_ser; -use fvm_shared::clock::ChainEpoch; -use fvm_shared::econ::TokenAmount; -use fvm_shared::sector::{Spacetime, StoragePower}; -use fvm_shared::smooth::FilterEstimate; -use fvm_shared::ActorID; - -use crate::kernel::{ClassifyResult, Result}; -use crate::state_tree::{ActorState, StateTree}; - -pub const REWARD_ACTOR_ID: ActorID = 2; - -impl Cbor for State {} -/// Reward actor state -#[derive(Serialize_tuple, Deserialize_tuple, Default)] -pub struct State { - /// Target CumsumRealized needs to reach for EffectiveNetworkTime to increase - /// Expressed in byte-epochs. - #[serde(with = "bigint_ser")] - pub cumsum_baseline: Spacetime, - - /// CumsumRealized is cumulative sum of network power capped by BaselinePower(epoch). - /// Expressed in byte-epochs. - #[serde(with = "bigint_ser")] - pub cumsum_realized: Spacetime, - - /// Ceiling of real effective network time `theta` based on - /// CumsumBaselinePower(theta) == CumsumRealizedPower - /// Theta captures the notion of how much the network has progressed in its baseline - /// and in advancing network time. - pub effective_network_time: ChainEpoch, - - /// EffectiveBaselinePower is the baseline power at the EffectiveNetworkTime epoch. - #[serde(with = "bigint_ser")] - pub effective_baseline_power: StoragePower, - - /// The reward to be paid in per WinCount to block producers. - /// The actual reward total paid out depends on the number of winners in any round. - /// This value is recomputed every non-null epoch and used in the next non-null epoch. - #[serde(with = "bigint_ser")] - pub this_epoch_reward: TokenAmount, - /// Smoothed `this_epoch_reward`. - pub this_epoch_reward_smoothed: FilterEstimate, - - /// The baseline power the network is targeting at st.Epoch. - #[serde(with = "bigint_ser")] - pub this_epoch_baseline_power: StoragePower, - - /// Epoch tracks for which epoch the Reward was computed. - pub epoch: ChainEpoch, - - // TotalStoragePowerReward tracks the total FIL awarded to block miners - #[serde(with = "bigint_ser")] - pub total_storage_power_reward: TokenAmount, - - // Simple and Baseline totals are constants used for computing rewards. - // They are on chain because of a historical fix resetting baseline value - // in a way that depended on the history leading immediately up to the - // migration fixing the value. These values can be moved from state back - // into a code constant in a subsequent upgrade. - #[serde(with = "bigint_ser")] - pub simple_total: TokenAmount, - #[serde(with = "bigint_ser")] - pub baseline_total: TokenAmount, -} - -impl State { - /// Loads the reward actor state with the supplied CID from the underlying store. - pub fn load(state_tree: &StateTree) -> Result<(Self, ActorState)> - where - B: Blockstore, - { - let reward_act = state_tree - .get_actor_id(REWARD_ACTOR_ID)? - .context("Reward actor address could not be resolved") - .or_fatal()?; - - let state = state_tree - .store() - .get_cbor(&reward_act.state) - .or_fatal()? - .context("reward actor state not found") - .or_fatal()?; - Ok((state, reward_act)) - } - - pub fn total_storage_power_reward(&self) -> TokenAmount { - self.total_storage_power_reward.clone() - } -} diff --git a/testing/conformance/Cargo.toml b/testing/conformance/Cargo.toml index 90d935bf7..fb222e956 100644 --- a/testing/conformance/Cargo.toml +++ b/testing/conformance/Cargo.toml @@ -47,7 +47,6 @@ serde_json = { version = "1.0", features = ["raw_value"] } walkdir = "2.3" regex = { version = "1.0" } ittapi-rs = { version = "0.1.6", optional = true } -actors-v6 = { version = "~6.3", package = "fil_builtin_actors_bundle" } actors-v7 = { version = "~7.3", package = "fil_builtin_actors_bundle" } libipld-core = { version = "0.13.1", features = ["serde-codec"] } diff --git a/testing/conformance/src/vm.rs b/testing/conformance/src/vm.rs index ccf9fec70..8069e1fa5 100644 --- a/testing/conformance/src/vm.rs +++ b/testing/conformance/src/vm.rs @@ -96,10 +96,7 @@ impl TestMachine>> { } pub fn import_actors(blockstore: &MemoryBlockstore) -> BTreeMap { - let bundles = [ - (NetworkVersion::V14, actors_v6::BUNDLE_CAR), - (NetworkVersion::V15, actors_v7::BUNDLE_CAR), - ]; + let bundles = [(NetworkVersion::V15, actors_v7::BUNDLE_CAR)]; bundles .into_iter() .map(|(nv, car)| { diff --git a/testing/conformance/test-vectors b/testing/conformance/test-vectors index 1cd64df0b..5a6e7ab9c 160000 --- a/testing/conformance/test-vectors +++ b/testing/conformance/test-vectors @@ -1 +1 @@ -Subproject commit 1cd64df0bbc7582d76acfff7b3534323604fc936 +Subproject commit 5a6e7ab9c2d7ffb5f54e5bd1a714ee5c1b7ad029 From f930a1ddf0197aa13da43e8aa3a39bf64084313f Mon Sep 17 00:00:00 2001 From: Mike <41407352+hunjixin@users.noreply.github.com> Date: Sat, 14 May 2022 02:45:35 +0800 Subject: [PATCH 16/21] testing/integration: allow manually-defined accounts (#549) --- testing/integration/src/tester.rs | 55 +++++++++++++++++-------------- 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/testing/integration/src/tester.rs b/testing/integration/src/tester.rs index 602990d59..5613cd9cd 100644 --- a/testing/integration/src/tester.rs +++ b/testing/integration/src/tester.rs @@ -1,4 +1,4 @@ -use anyhow::{Context, Result}; +use anyhow::{anyhow, Context, Result}; use cid::Cid; use fvm::call_manager::DefaultCallManager; use fvm::executor::DefaultExecutor; @@ -13,6 +13,7 @@ use fvm_shared::econ::TokenAmount; use fvm_shared::state::StateTreeVersion; use fvm_shared::version::NetworkVersion; use fvm_shared::{ActorID, IPLD_RAW}; +use libsecp256k1::{PublicKey, SecretKey}; use multihash::Code; use crate::builtin::{ @@ -97,9 +98,21 @@ where } /// Creates new accounts in the testing context + /// Inserts the specified number of accounts in the state tree, all with 1000 FIL，returning their IDs and Addresses. pub fn create_accounts(&mut self) -> Result<[Account; N]> { - // Create accounts. - put_secp256k1_accounts(self.state_tree.as_mut().unwrap(), self.accounts_code_cid) + use rand::SeedableRng; + + let rng = &mut rand_chacha::ChaCha8Rng::seed_from_u64(8); + + let mut ret: [Account; N] = [(0, Address::default()); N]; + for account in ret.iter_mut().take(N) { + let priv_key = SecretKey::random(rng); + *account = self.make_secp256k1_account( + priv_key, + TokenAmount::from(10u8) * TokenAmount::from(1000), + )?; + } + Ok(ret) } /// Set a new state in the state tree @@ -194,23 +207,20 @@ where self.state_tree.as_ref().unwrap().store() } } -} -/// Inserts the specified number of accounts in the state tree, all with 1000 FIL, -/// returning their IDs and Addresses. -fn put_secp256k1_accounts( - state_tree: &mut StateTree, - account_code_cid: Cid, -) -> Result<[Account; N]> { - use libsecp256k1::{PublicKey, SecretKey}; - use rand::SeedableRng; - - let rng = &mut rand_chacha::ChaCha8Rng::seed_from_u64(8); - - let mut ret: [Account; N] = [(0, Address::default()); N]; - for account in ret.iter_mut().take(N) { - let priv_key = SecretKey::random(rng); + + /// Put account with specified private key and balance + pub fn make_secp256k1_account( + &mut self, + priv_key: SecretKey, + init_balance: TokenAmount, + ) -> Result { let pub_key = PublicKey::from_secret_key(&priv_key); let pub_key_addr = Address::new_secp256k1(&pub_key.serialize())?; + + let state_tree = self + .state_tree + .as_mut() + .ok_or_else(|| anyhow!("unable get state tree"))?; let assigned_addr = state_tree.register_new_address(&pub_key_addr).unwrap(); let state = fvm::account_actor::State { address: pub_key_addr, @@ -219,21 +229,18 @@ fn put_secp256k1_accounts( let cid = state_tree.store().put_cbor(&state, Code::Blake2b256)?; let actor_state = ActorState { - code: account_code_cid, + code: self.accounts_code_cid, state: cid, sequence: 0, - balance: TokenAmount::from(10u8) * TokenAmount::from(1000), + balance: init_balance, }; state_tree .set_actor(&Address::new_id(assigned_addr), actor_state) .map_err(anyhow::Error::from)?; - - *account = (assigned_addr, pub_key_addr); + Ok((assigned_addr, pub_key_addr)) } - Ok(ret) } - /// Inserts the WASM code for the actor into the blockstore. fn put_wasm_code(blockstore: &impl Blockstore, wasm_binary: &[u8]) -> Result { let cid = blockstore.put( From 53deec5c0b5a9ea091008a351e339b5936404449 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 15 May 2022 13:39:22 -0400 Subject: [PATCH 17/21] chore: fix price-list milligas/gas mixup --- fvm/src/gas/price_list.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/fvm/src/gas/price_list.rs b/fvm/src/gas/price_list.rs index 05669c071..fdf724e96 100644 --- a/fvm/src/gas/price_list.rs +++ b/fvm/src/gas/price_list.rs @@ -106,21 +106,21 @@ lazy_static! { RegisteredPoStProof::StackedDRGWindow512MiBV1, ScalingCost { flat: static_milligas!(117680921), - scale: 43780, + scale: static_milligas!(43780), }, ), ( RegisteredPoStProof::StackedDRGWindow32GiBV1, ScalingCost { flat: static_milligas!(117680921), - scale: 43780, + scale: static_milligas!(43780), }, ), ( RegisteredPoStProof::StackedDRGWindow64GiBV1, ScalingCost { flat: static_milligas!(117680921), - scale: 43780, + scale: static_milligas!(43780), }, ), ] @@ -232,21 +232,21 @@ lazy_static! { RegisteredPoStProof::StackedDRGWindow512MiBV1, ScalingCost { flat: static_milligas!(117680921), - scale: 43780, + scale: static_milligas!(43780), }, ), ( RegisteredPoStProof::StackedDRGWindow32GiBV1, ScalingCost { flat: static_milligas!(117680921), - scale: 43780, + scale: static_milligas!(43780), }, ), ( RegisteredPoStProof::StackedDRGWindow64GiBV1, ScalingCost { flat: static_milligas!(117680921), - scale: 43780, + scale: static_milligas!(43780), }, ), ] From 5baa787a765eabe977cadc2a1a6b2a93f6914260 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 15 May 2022 15:19:33 -0400 Subject: [PATCH 18/21] feat: strongly typed gas units This adds a new gas type to avoid things like: 1. Multiplying gas by gas (not possible now). 2. Swapping gas/milligas. 3. Not using saturating arithmetic. This change also caught and fixed a bug where the "extra" consensus fault fee from lotus was being charged as milligas, not gas. Motivation: the previous commit that fixed a bug in window post cost scaling. --- fvm/src/call_manager/default.rs | 6 +- fvm/src/executor/default.rs | 9 +- fvm/src/gas/charge.rs | 14 +- fvm/src/gas/mod.rs | 219 ++++++++++++----- fvm/src/gas/outputs.rs | 14 +- fvm/src/gas/price_list.rs | 403 +++++++++++++++----------------- fvm/src/kernel/default.rs | 23 +- fvm/src/kernel/mod.rs | 6 +- fvm/src/syscalls/bind.rs | 2 +- fvm/src/syscalls/gas.rs | 6 +- fvm/src/syscalls/mod.rs | 5 +- testing/conformance/src/vm.rs | 28 +-- 12 files changed, 406 insertions(+), 329 deletions(-) diff --git a/fvm/src/call_manager/default.rs b/fvm/src/call_manager/default.rs index c5c450e6c..cee811b31 100644 --- a/fvm/src/call_manager/default.rs +++ b/fvm/src/call_manager/default.rs @@ -11,7 +11,7 @@ use num_traits::Zero; use super::{Backtrace, CallManager, InvocationResult, NO_DATA_BLOCK_ID}; use crate::call_manager::backtrace::Frame; use crate::call_manager::FinishRet; -use crate::gas::GasTracker; +use crate::gas::{Gas, GasTracker}; use crate::kernel::{ExecutionError, Kernel, Result, SyscallError}; use crate::machine::Machine; use crate::syscalls::error::Abort; @@ -71,7 +71,7 @@ where fn new(machine: M, gas_limit: i64, origin: Address, nonce: u64) -> Self { DefaultCallManager(Some(Box::new(InnerDefaultCallManager { machine, - gas_tracker: GasTracker::new(gas_limit, 0), + gas_tracker: GasTracker::new(Gas::new(gas_limit), Gas::zero()), origin, nonce, num_actors_created: 0, @@ -154,7 +154,7 @@ where } fn finish(mut self) -> (FinishRet, Self::Machine) { - let gas_used = self.gas_tracker.gas_used().max(0); + let gas_used = self.gas_tracker.gas_used().max(Gas::zero()).round_up(); let inner = self.0.take().expect("call manager is poisoned"); // TODO: Having to check against zero here is fishy, but this is what lotus does. diff --git a/fvm/src/executor/default.rs b/fvm/src/executor/default.rs index e4cfa64c8..a54acc785 100644 --- a/fvm/src/executor/default.rs +++ b/fvm/src/executor/default.rs @@ -15,7 +15,7 @@ use num_traits::Zero; use super::{ApplyFailure, ApplyKind, ApplyRet, Executor}; use crate::call_manager::{backtrace, CallManager, InvocationResult}; -use crate::gas::{milligas_to_gas, GasCharge, GasOutputs}; +use crate::gas::{Gas, GasCharge, GasOutputs}; use crate::kernel::{ClassifyResult, Context as _, ExecutionError, Kernel}; use crate::machine::{Machine, BURNT_FUNDS_ACTOR_ADDR, REWARD_ACTOR_ADDR}; @@ -231,10 +231,13 @@ where let pl = &self.context().price_list; let (inclusion_cost, miner_penalty_amount) = match apply_kind { - ApplyKind::Implicit => (GasCharge::new("none", 0, 0), Default::default()), + ApplyKind::Implicit => ( + GasCharge::new("none", Gas::zero(), Gas::zero()), + Default::default(), + ), ApplyKind::Explicit => { let inclusion_cost = pl.on_chain_message(raw_length); - let inclusion_total = milligas_to_gas(inclusion_cost.total(), true); + let inclusion_total = inclusion_cost.total().round_up(); // Verify the cost of the message is not over the message gas limit. if inclusion_total > msg.gas_limit { diff --git a/fvm/src/gas/charge.rs b/fvm/src/gas/charge.rs index f713b10cc..15ca323e7 100644 --- a/fvm/src/gas/charge.rs +++ b/fvm/src/gas/charge.rs @@ -1,20 +1,20 @@ // Copyright 2019-2022 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT -use crate::gas::Milligas; +use super::Gas; /// Single gas charge in the VM. Contains information about what gas was for, as well /// as the amount of gas needed for computation and storage respectively. pub struct GasCharge<'a> { pub name: &'a str, - /// Compute costs in milligas. - pub compute_gas: Milligas, - /// Storage costs in milligas. - pub storage_gas: Milligas, + /// Compute costs + pub compute_gas: Gas, + /// Storage costs + pub storage_gas: Gas, } impl<'a> GasCharge<'a> { - pub fn new(name: &'a str, compute_gas: Milligas, storage_gas: Milligas) -> Self { + pub fn new(name: &'a str, compute_gas: Gas, storage_gas: Gas) -> Self { Self { name, compute_gas, @@ -24,7 +24,7 @@ impl<'a> GasCharge<'a> { /// Calculates total gas charge (in milligas) by summing compute and /// storage gas associated with this charge. - pub fn total(&self) -> Milligas { + pub fn total(&self) -> Gas { self.compute_gas + self.storage_gas } } diff --git a/fvm/src/gas/mod.rs b/fvm/src/gas/mod.rs index d296add39..66da5cd7c 100644 --- a/fvm/src/gas/mod.rs +++ b/fvm/src/gas/mod.rs @@ -1,6 +1,9 @@ // Copyright 2019-2022 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT +use std::fmt::{Debug, Display}; +use std::ops::{Add, AddAssign, Mul, Sub, SubAssign}; + pub use self::charge::GasCharge; pub(crate) use self::outputs::GasOutputs; pub use self::price_list::{price_list_by_network_version, PriceList, WasmGasPrices}; @@ -12,13 +15,142 @@ mod price_list; pub const MILLIGAS_PRECISION: i64 = 1000; -// Type aliases to disambiguate units in interfaces. -pub type Gas = i64; -pub type Milligas = i64; +/// A typesafe representation of gas (internally stored as milligas). +/// +/// - All math operations are _saturating_ and never overflow. +/// - Enforces correct units by making it impossible to, e.g., get gas squared (by multiplying gas +/// by gas). +/// - Makes it harder to confuse gas and milligas. +#[derive(Hash, Eq, PartialEq, Ord, PartialOrd, Copy, Clone, Default)] +pub struct Gas(i64 /* milligas */); + +impl Debug for Gas { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if self.0 == 0 { + f.debug_tuple("Gas").field(&0 as &dyn Debug).finish() + } else { + let integral = self.0 / MILLIGAS_PRECISION; + let fractional = self.0 % MILLIGAS_PRECISION; + f.debug_tuple("Gas") + .field(&format_args!("{integral}.{fractional:03}")) + .finish() + } + } +} + +impl Display for Gas { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if self.0 == 0 { + f.write_str("0") + } else { + let integral = self.0 / MILLIGAS_PRECISION; + let fractional = self.0 % MILLIGAS_PRECISION; + write!(f, "{integral}.{fractional:03}") + } + } +} + +impl Gas { + /// Construct a `Gas` from milligas. + #[inline] + pub fn from_milligas(milligas: i64) -> Gas { + Gas(milligas) + } + + /// Construct a `Gas` from gas, scaling up. If this exceeds the width of an i64, it saturates at + /// `i64::MAX` milligas. + #[inline] + pub fn new(gas: i64) -> Gas { + Gas(gas.saturating_mul(MILLIGAS_PRECISION)) + } + + #[inline] + pub fn is_saturated(&self) -> bool { + self.0 == i64::MAX + } + + /// Returns the gas value as an integer, rounding the fractional part up. + #[inline] + pub fn round_up(&self) -> i64 { + milligas_to_gas(self.0, true) + } + + /// Returns the gas value as an integer, truncating the fractional part. + #[inline] + pub fn round_down(&self) -> i64 { + milligas_to_gas(self.0, false) + } + + /// Returns the gas value as milligas, without loss of precision. + #[inline] + pub fn as_milligas(&self) -> i64 { + self.0 + } +} + +impl num_traits::Zero for Gas { + fn zero() -> Self { + Gas(0) + } + + fn is_zero(&self) -> bool { + self.0 == 0 + } +} + +impl Add for Gas { + type Output = Gas; + + #[inline] + fn add(self, rhs: Self) -> Self::Output { + Self(self.0.saturating_add(rhs.0)) + } +} + +impl AddAssign for Gas { + #[inline] + fn add_assign(&mut self, rhs: Self) { + self.0 = self.0.saturating_add(rhs.0) + } +} + +impl SubAssign for Gas { + #[inline] + fn sub_assign(&mut self, rhs: Self) { + self.0 = self.0.saturating_sub(rhs.0) + } +} + +impl Sub for Gas { + type Output = Gas; + + #[inline] + fn sub(self, rhs: Self) -> Self::Output { + Self(self.0.saturating_sub(rhs.0)) + } +} + +impl Mul for Gas { + type Output = Gas; + + #[inline] + fn mul(self, rhs: i64) -> Self::Output { + Self(self.0.saturating_mul(rhs)) + } +} + +impl Mul for Gas { + type Output = Gas; + + #[inline] + fn mul(self, rhs: i32) -> Self::Output { + Self(self.0.saturating_mul(rhs.into())) + } +} pub struct GasTracker { - milligas_limit: i64, - milligas_used: i64, + gas_limit: Gas, + gas_used: Gas, } impl GasTracker { @@ -26,74 +158,47 @@ impl GasTracker { /// They are converted to milligas for internal canonical accounting. pub fn new(gas_limit: Gas, gas_used: Gas) -> Self { Self { - milligas_limit: gas_to_milligas(gas_limit), - milligas_used: gas_to_milligas(gas_used), + gas_limit, + gas_used, } } /// Safely consumes gas and returns an out of gas error if there is not sufficient /// enough gas remaining for charge. - pub fn charge_milligas(&mut self, name: &str, to_use: Milligas) -> Result<()> { - match self.milligas_used.checked_add(to_use) { - None => { - log::trace!("gas overflow: {}", name); - self.milligas_used = self.milligas_limit; - Err(ExecutionError::OutOfGas) - } - Some(used) => { - log::trace!("charged {} gas: {}", to_use, name); - if used > self.milligas_limit { - log::trace!("out of gas: {}", name); - self.milligas_used = self.milligas_limit; - Err(ExecutionError::OutOfGas) - } else { - self.milligas_used = used; - Ok(()) - } - } + pub fn charge_gas(&mut self, name: &str, to_use: Gas) -> Result<()> { + log::trace!("charging gas: {} {}", name, to_use); + // The gas type uses saturating math. + self.gas_used += to_use; + if self.gas_used > self.gas_limit { + log::trace!("gas limit reached"); + self.gas_used = self.gas_limit; + Err(ExecutionError::OutOfGas) + } else { + Ok(()) } } /// Applies the specified gas charge, where quantities are supplied in milligas. pub fn apply_charge(&mut self, charge: GasCharge) -> Result<()> { - self.charge_milligas(charge.name, charge.total()) + self.charge_gas(charge.name, charge.total()) } - /// Getter for gas available. + /// Getter for the maximum gas usable by this message. pub fn gas_limit(&self) -> Gas { - milligas_to_gas(self.milligas_limit, false) - } - - /// Getter for milligas available. - pub fn milligas_limit(&self) -> Milligas { - self.milligas_limit + self.gas_limit } /// Getter for gas used. pub fn gas_used(&self) -> Gas { - milligas_to_gas(self.milligas_used, true) - } - - /// Getter for milligas used. - pub fn milligas_used(&self) -> Milligas { - self.milligas_used + self.gas_used } + /// Getter for gas available. pub fn gas_available(&self) -> Gas { - milligas_to_gas(self.milligas_available(), false) - } - - pub fn milligas_available(&self) -> Milligas { - self.milligas_limit.saturating_sub(self.milligas_used) + self.gas_limit - self.gas_used } } -/// Converts the specified gas into equivalent fractional gas units -#[inline] -pub(crate) fn gas_to_milligas(gas: i64) -> i64 { - gas.saturating_mul(MILLIGAS_PRECISION) -} - /// Converts the specified fractional gas units into gas units #[inline] pub(crate) fn milligas_to_gas(milligas: i64, round_up: bool) -> i64 { @@ -108,18 +213,20 @@ pub(crate) fn milligas_to_gas(milligas: i64, round_up: bool) -> i64 { #[cfg(test)] mod tests { + use num_traits::Zero; + use super::*; #[test] #[allow(clippy::identity_op)] fn basic_gas_tracker() -> Result<()> { - let mut t = GasTracker::new(20, 10); - t.apply_charge(GasCharge::new("", 5 * MILLIGAS_PRECISION, 0))?; - assert_eq!(t.gas_used(), 15); - t.apply_charge(GasCharge::new("", 5 * MILLIGAS_PRECISION, 0))?; - assert_eq!(t.gas_used(), 20); + let mut t = GasTracker::new(Gas::new(20), Gas::new(10)); + t.apply_charge(GasCharge::new("", Gas::new(5), Gas::zero()))?; + assert_eq!(t.gas_used(), Gas::new(15)); + t.apply_charge(GasCharge::new("", Gas::new(5), Gas::zero()))?; + assert_eq!(t.gas_used(), Gas::new(20)); assert!(t - .apply_charge(GasCharge::new("", 1 * MILLIGAS_PRECISION, 0)) + .apply_charge(GasCharge::new("", Gas::new(1), Gas::zero())) .is_err()); Ok(()) } diff --git a/fvm/src/gas/outputs.rs b/fvm/src/gas/outputs.rs index ccb1b819f..3086d7843 100644 --- a/fvm/src/gas/outputs.rs +++ b/fvm/src/gas/outputs.rs @@ -3,8 +3,6 @@ use std::convert::TryFrom; use fvm_shared::bigint::BigInt; use fvm_shared::econ::TokenAmount; -use crate::gas::Gas; - #[derive(Clone, Default)] pub(crate) struct GasOutputs { pub base_fee_burn: TokenAmount, @@ -13,14 +11,16 @@ pub(crate) struct GasOutputs { pub miner_tip: TokenAmount, pub refund: TokenAmount, - pub gas_refund: Gas, - pub gas_burned: Gas, + // In whole gas units. + pub gas_refund: i64, + pub gas_burned: i64, } impl GasOutputs { pub fn compute( - gas_used: Gas, - gas_limit: Gas, + // In whole gas units. + gas_used: i64, + gas_limit: i64, base_fee: &TokenAmount, fee_cap: &TokenAmount, gas_premium: &TokenAmount, @@ -59,7 +59,7 @@ impl GasOutputs { } } -fn compute_gas_overestimation_burn(gas_used: Gas, gas_limit: Gas) -> (Gas, Gas) { +fn compute_gas_overestimation_burn(gas_used: i64, gas_limit: i64) -> (i64, i64) { const GAS_OVERUSE_NUM: i64 = 11; const GAS_OVERUSE_DENOM: i64 = 10; diff --git a/fvm/src/gas/price_list.rs b/fvm/src/gas/price_list.rs index fdf724e96..3a31d5df8 100644 --- a/fvm/src/gas/price_list.rs +++ b/fvm/src/gas/price_list.rs @@ -17,51 +17,43 @@ use lazy_static::lazy_static; use num_traits::Zero; use super::GasCharge; -use crate::gas::Milligas; - -/// Converts a static value to milligas. This operation does not saturate, -/// and should only be used with constant values in pricelists. -macro_rules! static_milligas { - ($ex:expr) => { - $ex * $crate::gas::MILLIGAS_PRECISION - }; -} +use crate::gas::Gas; lazy_static! { static ref OH_SNAP_PRICES: PriceList = PriceList { storage_gas_multiplier: 1300, - on_chain_message_compute_base: static_milligas!(38863), - on_chain_message_storage_base: static_milligas!(36), - on_chain_message_storage_per_byte: static_milligas!(1), + on_chain_message_compute_base: Gas::new(38863), + on_chain_message_storage_base: Gas::new(36), + on_chain_message_storage_per_byte: Gas::new(1), - on_chain_return_value_per_byte: static_milligas!(1), + on_chain_return_value_per_byte: Gas::new(1), - send_base: static_milligas!(29233), - send_transfer_funds: static_milligas!(27500), - send_transfer_only_premium: static_milligas!(159672), - send_invoke_method: static_milligas!(-5377), + send_base: Gas::new(29233), + send_transfer_funds: Gas::new(27500), + send_transfer_only_premium: Gas::new(159672), + send_invoke_method: Gas::new(-5377), - create_actor_compute: static_milligas!(1108454), - create_actor_storage: static_milligas!(36 + 40), - delete_actor: static_milligas!(-(36 + 40)), + create_actor_compute: Gas::new(1108454), + create_actor_storage: Gas::new(36 + 40), + delete_actor: Gas::new(-(36 + 40)), - bls_sig_cost: static_milligas!(16598605), - secp256k1_sig_cost: static_milligas!(1637292), + bls_sig_cost: Gas::new(16598605), + secp256k1_sig_cost: Gas::new(1637292), - hashing_base: static_milligas!(31355), - compute_unsealed_sector_cid_base: static_milligas!(98647), - verify_seal_base: static_milligas!(2000), // TODO revisit potential removal of this + hashing_base: Gas::new(31355), + compute_unsealed_sector_cid_base: Gas::new(98647), + verify_seal_base: Gas::new(2000), // TODO revisit potential removal of this - verify_aggregate_seal_base: 0, + verify_aggregate_seal_base: Zero::zero(), verify_aggregate_seal_per: [ ( RegisteredSealProof::StackedDRG32GiBV1P1, - static_milligas!(449900) + Gas::new(449900) ), ( RegisteredSealProof::StackedDRG64GiBV1P1, - static_milligas!(359272) + Gas::new(359272) ) ].iter().copied().collect(), verify_aggregate_seal_steps: [ @@ -69,14 +61,14 @@ lazy_static! { RegisteredSealProof::StackedDRG32GiBV1P1, StepCost ( vec![ - Step{start: 4, cost: static_milligas!(103994170)}, - Step{start: 7, cost: static_milligas!(112356810)}, - Step{start: 13, cost: static_milligas!(122912610)}, - Step{start: 26, cost: static_milligas!(137559930)}, - Step{start: 52, cost: static_milligas!(162039100)}, - Step{start: 103, cost: static_milligas!(210960780)}, - Step{start: 205, cost: static_milligas!(318351180)}, - Step{start: 410, cost: static_milligas!(528274980)}, + Step{start: 4, cost: Gas::new(103994170)}, + Step{start: 7, cost: Gas::new(112356810)}, + Step{start: 13, cost: Gas::new(122912610)}, + Step{start: 26, cost: Gas::new(137559930)}, + Step{start: 52, cost: Gas::new(162039100)}, + Step{start: 103, cost: Gas::new(210960780)}, + Step{start: 205, cost: Gas::new(318351180)}, + Step{start: 410, cost: Gas::new(528274980)}, ] ) ), @@ -84,14 +76,14 @@ lazy_static! { RegisteredSealProof::StackedDRG64GiBV1P1, StepCost ( vec![ - Step{start: 4, cost: static_milligas!(102581240)}, - Step{start: 7, cost: static_milligas!(110803030)}, - Step{start: 13, cost: static_milligas!(120803700)}, - Step{start: 26, cost: static_milligas!(134642130)}, - Step{start: 52, cost: static_milligas!(157357890)}, - Step{start: 103, cost: static_milligas!(203017690)}, - Step{start: 205, cost: static_milligas!(304253590)}, - Step{start: 410, cost: static_milligas!(509880640)}, + Step{start: 4, cost: Gas::new(102581240)}, + Step{start: 7, cost: Gas::new(110803030)}, + Step{start: 13, cost: Gas::new(120803700)}, + Step{start: 26, cost: Gas::new(134642130)}, + Step{start: 52, cost: Gas::new(157357890)}, + Step{start: 103, cost: Gas::new(203017690)}, + Step{start: 205, cost: Gas::new(304253590)}, + Step{start: 410, cost: Gas::new(509880640)}, ] ) ) @@ -99,28 +91,28 @@ lazy_static! { .cloned() .collect(), - verify_consensus_fault: static_milligas!(495422), - verify_replica_update: static_milligas!(36316136), + verify_consensus_fault: Gas::new(495422), + verify_replica_update: Gas::new(36316136), verify_post_lookup: [ ( RegisteredPoStProof::StackedDRGWindow512MiBV1, ScalingCost { - flat: static_milligas!(117680921), - scale: static_milligas!(43780), + flat: Gas::new(117680921), + scale: Gas::new(43780), }, ), ( RegisteredPoStProof::StackedDRGWindow32GiBV1, ScalingCost { - flat: static_milligas!(117680921), - scale: static_milligas!(43780), + flat: Gas::new(117680921), + scale: Gas::new(43780), }, ), ( RegisteredPoStProof::StackedDRGWindow64GiBV1, ScalingCost { - flat: static_milligas!(117680921), - scale: static_milligas!(43780), + flat: Gas::new(117680921), + scale: Gas::new(43780), }, ), ] @@ -128,65 +120,65 @@ lazy_static! { .copied() .collect(), - get_randomness_base: 0, - get_randomness_per_byte: 0, + get_randomness_base: Zero::zero(), + get_randomness_per_byte: Zero::zero(), - block_memcpy_per_byte_cost: 0, + block_memcpy_per_byte_cost: Zero::zero(), - block_open_base: static_milligas!(114617), - block_open_memret_per_byte_cost: 0, + block_open_base: Gas::new(114617), + block_open_memret_per_byte_cost: Zero::zero(), - block_link_base: static_milligas!(353640), - block_link_storage_per_byte_cost: static_milligas!(1), + block_link_base: Gas::new(353640), + block_link_storage_per_byte_cost: Gas::new(1), - block_create_base: 0, - block_create_memret_per_byte_cost: 0, + block_create_base: Zero::zero(), + block_create_memret_per_byte_cost: Zero::zero(), - block_read_base: 0, - block_stat_base: 0, + block_read_base: Zero::zero(), + block_stat_base: Zero::zero(), - syscall_cost: 0, - extern_cost: 0, + syscall_cost: Zero::zero(), + extern_cost: Zero::zero(), wasm_rules: WasmGasPrices{ - exec_instruction_cost_milli: 0, + exec_instruction_cost: Zero::zero(), }, }; static ref SKYR_PRICES: PriceList = PriceList { storage_gas_multiplier: 1300, - on_chain_message_compute_base: static_milligas!(38863), - on_chain_message_storage_base: static_milligas!(36), - on_chain_message_storage_per_byte: static_milligas!(1), + on_chain_message_compute_base: Gas::new(38863), + on_chain_message_storage_base: Gas::new(36), + on_chain_message_storage_per_byte: Gas::new(1), - on_chain_return_value_per_byte: static_milligas!(1), + on_chain_return_value_per_byte: Gas::new(1), - send_base: static_milligas!(29233), - send_transfer_funds: static_milligas!(27500), - send_transfer_only_premium: static_milligas!(159672), - send_invoke_method: static_milligas!(-5377), + send_base: Gas::new(29233), + send_transfer_funds: Gas::new(27500), + send_transfer_only_premium: Gas::new(159672), + send_invoke_method: Gas::new(-5377), - create_actor_compute: static_milligas!(1108454), - create_actor_storage: static_milligas!(36 + 40), - delete_actor: static_milligas!(-(36 + 40)), + create_actor_compute: Gas::new(1108454), + create_actor_storage: Gas::new(36 + 40), + delete_actor: Gas::new(-(36 + 40)), - bls_sig_cost: static_milligas!(16598605), - secp256k1_sig_cost: static_milligas!(1637292), + bls_sig_cost: Gas::new(16598605), + secp256k1_sig_cost: Gas::new(1637292), - hashing_base: static_milligas!(31355), - compute_unsealed_sector_cid_base: static_milligas!(98647), - verify_seal_base: static_milligas!(2000), // TODO revisit potential removal of this + hashing_base: Gas::new(31355), + compute_unsealed_sector_cid_base: Gas::new(98647), + verify_seal_base: Gas::new(2000), // TODO revisit potential removal of this - verify_aggregate_seal_base: 0, + verify_aggregate_seal_base: Zero::zero(), verify_aggregate_seal_per: [ ( RegisteredSealProof::StackedDRG32GiBV1P1, - static_milligas!(449900) + Gas::new(449900) ), ( RegisteredSealProof::StackedDRG64GiBV1P1, - static_milligas!(359272) + Gas::new(359272) ) ].iter().copied().collect(), verify_aggregate_seal_steps: [ @@ -194,14 +186,14 @@ lazy_static! { RegisteredSealProof::StackedDRG32GiBV1P1, StepCost ( vec![ - Step{start: 4, cost: static_milligas!(103994170)}, - Step{start: 7, cost: static_milligas!(112356810)}, - Step{start: 13, cost: static_milligas!(122912610)}, - Step{start: 26, cost: static_milligas!(137559930)}, - Step{start: 52, cost: static_milligas!(162039100)}, - Step{start: 103, cost: static_milligas!(210960780)}, - Step{start: 205, cost: static_milligas!(318351180)}, - Step{start: 410, cost: static_milligas!(528274980)}, + Step{start: 4, cost: Gas::new(103994170)}, + Step{start: 7, cost: Gas::new(112356810)}, + Step{start: 13, cost: Gas::new(122912610)}, + Step{start: 26, cost: Gas::new(137559930)}, + Step{start: 52, cost: Gas::new(162039100)}, + Step{start: 103, cost: Gas::new(210960780)}, + Step{start: 205, cost: Gas::new(318351180)}, + Step{start: 410, cost: Gas::new(528274980)}, ] ) ), @@ -209,14 +201,14 @@ lazy_static! { RegisteredSealProof::StackedDRG64GiBV1P1, StepCost ( vec![ - Step{start: 4, cost: static_milligas!(102581240)}, - Step{start: 7, cost: static_milligas!(110803030)}, - Step{start: 13, cost: static_milligas!(120803700)}, - Step{start: 26, cost: static_milligas!(134642130)}, - Step{start: 52, cost: static_milligas!(157357890)}, - Step{start: 103, cost: static_milligas!(203017690)}, - Step{start: 205, cost: static_milligas!(304253590)}, - Step{start: 410, cost: static_milligas!(509880640)}, + Step{start: 4, cost: Gas::new(102581240)}, + Step{start: 7, cost: Gas::new(110803030)}, + Step{start: 13, cost: Gas::new(120803700)}, + Step{start: 26, cost: Gas::new(134642130)}, + Step{start: 52, cost: Gas::new(157357890)}, + Step{start: 103, cost: Gas::new(203017690)}, + Step{start: 205, cost: Gas::new(304253590)}, + Step{start: 410, cost: Gas::new(509880640)}, ] ) ) @@ -225,28 +217,28 @@ lazy_static! { .collect(), // TODO: PARAM_FINISH: this may need to be increased to account for the cost of an extern - verify_consensus_fault: static_milligas!(495422), - verify_replica_update: static_milligas!(36316136), + verify_consensus_fault: Gas::new(495422), + verify_replica_update: Gas::new(36316136), verify_post_lookup: [ ( RegisteredPoStProof::StackedDRGWindow512MiBV1, ScalingCost { - flat: static_milligas!(117680921), - scale: static_milligas!(43780), + flat: Gas::new(117680921), + scale: Gas::new(43780), }, ), ( RegisteredPoStProof::StackedDRGWindow32GiBV1, ScalingCost { - flat: static_milligas!(117680921), - scale: static_milligas!(43780), + flat: Gas::new(117680921), + scale: Gas::new(43780), }, ), ( RegisteredPoStProof::StackedDRGWindow64GiBV1, ScalingCost { - flat: static_milligas!(117680921), - scale: static_milligas!(43780), + flat: Gas::new(117680921), + scale: Gas::new(43780), }, ), ] @@ -254,36 +246,36 @@ lazy_static! { .copied() .collect(), - get_randomness_base: 0, - get_randomness_per_byte: 0, + get_randomness_base: Zero::zero(), + get_randomness_per_byte: Zero::zero(), - block_memcpy_per_byte_cost: 500, + block_memcpy_per_byte_cost: Gas::from_milligas(500), - block_open_base: static_milligas!(114617), - block_open_memret_per_byte_cost: static_milligas!(10), + block_open_base: Gas::new(114617), + block_open_memret_per_byte_cost: Gas::new(10), - block_link_base: static_milligas!(353640), - block_link_storage_per_byte_cost: static_milligas!(1), + block_link_base: Gas::new(353640), + block_link_storage_per_byte_cost: Gas::new(1), - block_create_base: 0, - block_create_memret_per_byte_cost: static_milligas!(10), + block_create_base: Zero::zero(), + block_create_memret_per_byte_cost: Gas::new(10), - block_read_base: 0, - block_stat_base: 0, + block_read_base: Zero::zero(), + block_stat_base: Zero::zero(), - syscall_cost: static_milligas!(14000), - extern_cost: static_milligas!(21000), + syscall_cost: Gas::new(14000), + extern_cost: Gas::new(21000), wasm_rules: WasmGasPrices{ - exec_instruction_cost_milli: static_milligas!(4) as u64, + exec_instruction_cost: Gas::new(4), }, }; } #[derive(Clone, Debug, Copy)] pub(crate) struct ScalingCost { - flat: i64, - scale: i64, + flat: Gas, + scale: Gas, } #[derive(Clone, Debug)] @@ -292,11 +284,11 @@ pub(crate) struct StepCost(Vec); #[derive(Clone, Debug, Copy)] pub(crate) struct Step { start: i64, - cost: i64, + cost: Gas, } impl StepCost { - pub(crate) fn lookup(&self, x: i64) -> i64 { + pub(crate) fn lookup(&self, x: i64) -> Gas { let mut i: i64 = 0; while i < self.0.len() as i64 { if self.0[i as usize].start > x { @@ -306,7 +298,7 @@ impl StepCost { } i -= 1; if i < 0 { - return 0; + return Gas::zero(); } self.0[i as usize].cost } @@ -325,96 +317,96 @@ pub struct PriceList { /// Together, these account for the cost of message propagation and validation, /// up to but excluding any actual processing by the VM. /// This is the cost a block producer burns when including an invalid message. - pub(crate) on_chain_message_compute_base: Milligas, - pub(crate) on_chain_message_storage_base: Milligas, - pub(crate) on_chain_message_storage_per_byte: Milligas, + pub(crate) on_chain_message_compute_base: Gas, + pub(crate) on_chain_message_storage_base: Gas, + pub(crate) on_chain_message_storage_per_byte: Gas, /// Gas cost charged to the originator of a non-nil return value produced /// by an on-chain message is given by: /// len(return value)*OnChainReturnValuePerByte - pub(crate) on_chain_return_value_per_byte: Milligas, + pub(crate) on_chain_return_value_per_byte: Gas, /// Gas cost for any message send execution(including the top-level one /// initiated by an on-chain message). /// This accounts for the cost of loading sender and receiver actors and /// (for top-level messages) incrementing the sender's sequence number. /// Load and store of actor sub-state is charged separately. - pub(crate) send_base: Milligas, + pub(crate) send_base: Gas, /// Gas cost charged, in addition to SendBase, if a message send /// is accompanied by any nonzero currency amount. /// Accounts for writing receiver's new balance (the sender's state is /// already accounted for). - pub(crate) send_transfer_funds: Milligas, + pub(crate) send_transfer_funds: Gas, /// Gas cost charged, in addition to SendBase, if message only transfers funds. - pub(crate) send_transfer_only_premium: Milligas, + pub(crate) send_transfer_only_premium: Gas, /// Gas cost charged, in addition to SendBase, if a message invokes /// a method on the receiver. /// Accounts for the cost of loading receiver code and method dispatch. - pub(crate) send_invoke_method: Milligas, + pub(crate) send_invoke_method: Gas, /// Gas cost for creating a new actor (via InitActor's Exec method). /// Note: this costs assume that the extra will be partially or totally refunded while /// the base is covering for the put. - pub(crate) create_actor_compute: Milligas, - pub(crate) create_actor_storage: Milligas, + pub(crate) create_actor_compute: Gas, + pub(crate) create_actor_storage: Gas, /// Gas cost for deleting an actor. /// Note: this partially refunds the create cost to incentivise the deletion of the actors. - pub(crate) delete_actor: Milligas, + pub(crate) delete_actor: Gas, /// Gas cost for verifying bls signature - pub(crate) bls_sig_cost: Milligas, + pub(crate) bls_sig_cost: Gas, /// Gas cost for verifying secp256k1 signature - pub(crate) secp256k1_sig_cost: Milligas, + pub(crate) secp256k1_sig_cost: Gas, - pub(crate) hashing_base: Milligas, + pub(crate) hashing_base: Gas, - pub(crate) compute_unsealed_sector_cid_base: Milligas, - pub(crate) verify_seal_base: Milligas, + pub(crate) compute_unsealed_sector_cid_base: Gas, + pub(crate) verify_seal_base: Gas, #[allow(unused)] - pub(crate) verify_aggregate_seal_base: Milligas, - pub(crate) verify_aggregate_seal_per: AHashMap, + pub(crate) verify_aggregate_seal_base: Gas, + pub(crate) verify_aggregate_seal_per: AHashMap, pub(crate) verify_aggregate_seal_steps: AHashMap, pub(crate) verify_post_lookup: AHashMap, - pub(crate) verify_consensus_fault: Milligas, - pub(crate) verify_replica_update: Milligas, + pub(crate) verify_consensus_fault: Gas, + pub(crate) verify_replica_update: Gas, /// Gas cost for fetching randomness. - pub(crate) get_randomness_base: Milligas, + pub(crate) get_randomness_base: Gas, /// Gas cost per every byte of randomness fetched. - pub(crate) get_randomness_per_byte: Milligas, + pub(crate) get_randomness_per_byte: Gas, /// Gas cost per every block byte memcopied across boundaries. - pub(crate) block_memcpy_per_byte_cost: Milligas, + pub(crate) block_memcpy_per_byte_cost: Gas, /// Gas cost for opening a block. - pub(crate) block_open_base: Milligas, + pub(crate) block_open_base: Gas, /// Gas cost for every byte retained in FVM space when opening a block. - pub(crate) block_open_memret_per_byte_cost: Milligas, + pub(crate) block_open_memret_per_byte_cost: Gas, /// Gas cost for linking a block. - pub(crate) block_link_base: Milligas, + pub(crate) block_link_base: Gas, /// Multiplier for storage gas per byte. - pub(crate) block_link_storage_per_byte_cost: Milligas, + pub(crate) block_link_storage_per_byte_cost: Gas, /// Gas cost for creating a block. - pub(crate) block_create_base: Milligas, + pub(crate) block_create_base: Gas, /// Gas cost for every byte retained in FVM space when writing a block. - pub(crate) block_create_memret_per_byte_cost: Milligas, + pub(crate) block_create_memret_per_byte_cost: Gas, /// Gas cost for reading a block into actor space. - pub(crate) block_read_base: Milligas, + pub(crate) block_read_base: Gas, /// Gas cost for statting a block. - pub(crate) block_stat_base: Milligas, + pub(crate) block_stat_base: Gas, /// General gas cost for performing a syscall, accounting for the overhead thereof. - pub(crate) syscall_cost: Milligas, + pub(crate) syscall_cost: Gas, /// General gas cost for calling an extern, accounting for the overhead thereof. - pub(crate) extern_cost: Milligas, + pub(crate) extern_cost: Gas, /// Rules for execution gas. pub(crate) wasm_rules: WasmGasPrices, @@ -422,7 +414,7 @@ pub struct PriceList { #[derive(Clone, Debug, Eq, PartialEq, Hash)] pub struct WasmGasPrices { - pub(crate) exec_instruction_cost_milli: u64, + pub(crate) exec_instruction_cost: Gas, } impl PriceList { @@ -443,8 +435,8 @@ impl PriceList { pub fn on_chain_return_value(&self, data_size: usize) -> GasCharge<'static> { GasCharge::new( "OnChainReturnValue", - 0, - data_size as i64 * self.on_chain_return_value_per_byte * self.storage_gas_multiplier, + Zero::zero(), + self.on_chain_return_value_per_byte * data_size as i64 * self.storage_gas_multiplier, ) } @@ -465,12 +457,12 @@ impl PriceList { if method_num != METHOD_SEND { ret += self.send_invoke_method; } - GasCharge::new("OnMethodInvocation", ret, 0) + GasCharge::new("OnMethodInvocation", ret, Zero::zero()) } /// Returns the gas cost to be applied on a syscall. pub fn on_syscall(&self) -> GasCharge<'static> { - GasCharge::new("OnSyscall", self.syscall_cost, 0) + GasCharge::new("OnSyscall", self.syscall_cost, Zero::zero()) } /// Returns the gas required for creating an actor. @@ -488,7 +480,7 @@ impl PriceList { pub fn on_delete_actor(&self) -> GasCharge<'static> { GasCharge::new( "OnDeleteActor", - 0, + Zero::zero(), self.delete_actor * self.storage_gas_multiplier, ) } @@ -500,13 +492,13 @@ impl PriceList { SignatureType::BLS => self.bls_sig_cost, SignatureType::Secp256k1 => self.secp256k1_sig_cost, }; - GasCharge::new("OnVerifySignature", val, 0) + GasCharge::new("OnVerifySignature", val, Zero::zero()) } /// Returns gas required for hashing data. #[inline] pub fn on_hashing(&self, _: usize) -> GasCharge<'static> { - GasCharge::new("OnHashing", self.hashing_base, 0) + GasCharge::new("OnHashing", self.hashing_base, Zero::zero()) } /// Returns gas required for computing unsealed sector Cid. @@ -519,14 +511,14 @@ impl PriceList { GasCharge::new( "OnComputeUnsealedSectorCid", self.compute_unsealed_sector_cid_base, - 0, + Zero::zero(), ) } /// Returns gas required for seal verification. #[inline] pub fn on_verify_seal(&self, _info: &SealVerifyInfo) -> GasCharge<'static> { - GasCharge::new("OnVerifySeal", self.verify_seal_base, 0) + GasCharge::new("OnVerifySeal", self.verify_seal_base, Zero::zero()) } #[inline] pub fn on_verify_aggregate_seals( @@ -534,7 +526,7 @@ impl PriceList { aggregate: &AggregateSealVerifyProofAndInfos, ) -> GasCharge<'static> { let proof_type = aggregate.seal_proof; - let per_proof = self + let per_proof = *self .verify_aggregate_seal_per .get(&proof_type) .unwrap_or_else(|| { @@ -560,14 +552,18 @@ impl PriceList { GasCharge::new( "OnVerifyAggregateSeals", per_proof * num + step.lookup(num), - 0, + Zero::zero(), ) } /// Returns gas required for replica verification. #[inline] pub fn on_verify_replica_update(&self, _replica: &ReplicaUpdateInfo) -> GasCharge<'static> { - GasCharge::new("OnVerifyReplicaUpdate", self.verify_replica_update, 0) + GasCharge::new( + "OnVerifyReplicaUpdate", + self.verify_replica_update, + Zero::zero(), + ) } /// Returns gas required for PoSt verification. @@ -584,9 +580,9 @@ impl PriceList { .expect("512MiB lookup must exist in price table") }); - let gas_used = cost.flat + info.challenged_sectors.len() as i64 * cost.scale; + let gas_used = cost.flat + cost.scale * info.challenged_sectors.len() as i64; - GasCharge::new("OnVerifyPost", gas_used, 0) + GasCharge::new("OnVerifyPost", gas_used, Zero::zero()) } /// Returns gas required for verifying consensus fault. @@ -594,8 +590,8 @@ impl PriceList { pub fn on_verify_consensus_fault(&self) -> GasCharge<'static> { GasCharge::new( "OnVerifyConsensusFault", - self.extern_cost.saturating_add(self.verify_consensus_fault), - 0, + self.extern_cost + self.verify_consensus_fault, + Zero::zero(), ) } @@ -606,12 +602,9 @@ impl PriceList { GasCharge::new( "OnGetRandomness", self.extern_cost - .saturating_add(self.get_randomness_base) - .saturating_add( - self.get_randomness_per_byte - .saturating_mul(entropy_size as i64), - ), - 0, + + self.get_randomness_base + + (self.get_randomness_per_byte * entropy_size as i64), + Zero::zero(), ) } @@ -620,8 +613,8 @@ impl PriceList { pub fn on_block_open_base(&self) -> GasCharge<'static> { GasCharge::new( "OnBlockOpenBase", - self.extern_cost.saturating_add(self.block_open_base), - 0, + self.extern_cost + self.block_open_base, + Zero::zero(), ) } @@ -631,9 +624,9 @@ impl PriceList { let size = data_size as i64; GasCharge::new( "OnBlockOpenPerByte", - self.block_open_memret_per_byte_cost.saturating_mul(size) - + self.block_memcpy_per_byte_cost.saturating_mul(size), - 0, + (self.block_open_memret_per_byte_cost * size) + + (self.block_memcpy_per_byte_cost * size), + Zero::zero(), ) } @@ -642,11 +635,8 @@ impl PriceList { pub fn on_block_read(&self, data_size: usize) -> GasCharge<'static> { GasCharge::new( "OnBlockRead", - self.block_read_base.saturating_add( - self.block_memcpy_per_byte_cost - .saturating_mul(data_size as i64), - ), - 0, + self.block_read_base + (self.block_memcpy_per_byte_cost * data_size as i64), + Zero::zero(), ) } @@ -654,14 +644,12 @@ impl PriceList { #[inline] pub fn on_block_create(&self, data_size: usize) -> GasCharge<'static> { let size = data_size as i64; - let mem_costs = self - .block_create_memret_per_byte_cost - .saturating_mul(size) - .saturating_add(self.block_memcpy_per_byte_cost.saturating_mul(size)); + let mem_costs = (self.block_create_memret_per_byte_cost * size) + + (self.block_memcpy_per_byte_cost * size); GasCharge::new( "OnBlockCreate", - self.block_create_base.saturating_add(mem_costs), - 0, + self.block_create_base + mem_costs, + Zero::zero(), ) } @@ -669,25 +657,22 @@ impl PriceList { #[inline] pub fn on_block_link(&self, data_size: usize) -> GasCharge<'static> { let size = data_size as i64; - let memcpy = self.block_memcpy_per_byte_cost.saturating_mul(size); + let memcpy = self.block_memcpy_per_byte_cost * size; GasCharge::new( "OnBlockLink", // twice the memcpy cost: // - one from the block registry to the FVM BufferedBlockstore // - one from the FVM BufferedBlockstore to the Node's Blockstore // when the machine finishes. - self.block_link_base - .saturating_add((2_i64).saturating_mul(memcpy)), - self.block_link_storage_per_byte_cost - .saturating_mul(self.storage_gas_multiplier) - .saturating_mul(size), + self.block_link_base + (memcpy * 2), + self.block_link_storage_per_byte_cost * self.storage_gas_multiplier * size, ) } /// Returns the gas required for storing an object. #[inline] pub fn on_block_stat(&self) -> GasCharge<'static> { - GasCharge::new("OnBlockStat", self.block_stat_base, 0) + GasCharge::new("OnBlockStat", self.block_stat_base, Zero::zero()) } } @@ -701,7 +686,7 @@ pub fn price_list_by_network_version(network_version: NetworkVersion) -> &'stati impl Rules for WasmGasPrices { fn instruction_cost(&self, instruction: &Instruction) -> Option { - if self.exec_instruction_cost_milli == 0 { + if self.exec_instruction_cost.is_zero() { return Some(0); } @@ -719,7 +704,7 @@ impl Rules for WasmGasPrices { | Instruction::Return | Instruction::Else | Instruction::End => Some(0), - _ => Some(self.exec_instruction_cost_milli), + _ => Some(self.exec_instruction_cost.as_milligas() as u64), } } diff --git a/fvm/src/kernel/default.rs b/fvm/src/kernel/default.rs index 6b24a1220..53ff08692 100644 --- a/fvm/src/kernel/default.rs +++ b/fvm/src/kernel/default.rs @@ -544,8 +544,8 @@ where if self.network_version() <= NetworkVersion::V15 { self.call_manager.charge_gas(GasCharge::new( "verify_consensus_fault_accesses", - gas, - 0, + Gas::new(gas), + Gas::zero(), ))?; } @@ -693,25 +693,18 @@ impl GasOps for DefaultKernel where C: CallManager, { - fn gas_used(&self) -> i64 { + fn gas_used(&self) -> Gas { self.call_manager.gas_tracker().gas_used() } - fn milligas_used(&self) -> i64 { - self.call_manager.gas_tracker().milligas_used() - } - - fn gas_available(&self) -> i64 { + fn gas_available(&self) -> Gas { self.call_manager.gas_tracker().gas_available() } - fn milligas_available(&self) -> i64 { - self.call_manager.gas_tracker().milligas_available() - } - - fn charge_milligas(&mut self, name: &str, compute: Milligas) -> Result<()> { - let charge = GasCharge::new(name, compute, 0); - self.call_manager.gas_tracker_mut().apply_charge(charge) + fn charge_gas(&mut self, name: &str, compute: Gas) -> Result<()> { + self.call_manager + .gas_tracker_mut() + .charge_gas(name, compute) } fn price_list(&self) -> &PriceList { diff --git a/fvm/src/kernel/mod.rs b/fvm/src/kernel/mod.rs index ff65b3098..ab7a75e0e 100644 --- a/fvm/src/kernel/mod.rs +++ b/fvm/src/kernel/mod.rs @@ -24,7 +24,7 @@ mod error; pub use error::{ClassifyResult, Context, ExecutionError, Result, SyscallError}; use crate::call_manager::{CallManager, InvocationResult}; -use crate::gas::{Gas, Milligas, PriceList}; +use crate::gas::{Gas, PriceList}; use crate::machine::Machine; pub trait Kernel: @@ -215,15 +215,13 @@ pub trait CircSupplyOps { pub trait GasOps { /// Returns the gas used by the transaction so far. fn gas_used(&self) -> Gas; - fn milligas_used(&self) -> Milligas; /// Returns the remaining gas for the transaction. fn gas_available(&self) -> Gas; - fn milligas_available(&self) -> Milligas; /// ChargeGas charges specified amount of `gas` for execution. /// `name` provides information about gas charging point. - fn charge_milligas(&mut self, name: &str, compute: Milligas) -> Result<()>; + fn charge_gas(&mut self, name: &str, compute: Gas) -> Result<()>; /// Returns the currently active gas price list. fn price_list(&self) -> &PriceList; diff --git a/fvm/src/syscalls/bind.rs b/fvm/src/syscalls/bind.rs index 5d93424d5..14324b590 100644 --- a/fvm/src/syscalls/bind.rs +++ b/fvm/src/syscalls/bind.rs @@ -99,7 +99,7 @@ macro_rules! charge_syscall_gas { ($kernel:expr) => { let charge = $kernel.price_list().on_syscall(); $kernel - .charge_milligas(charge.name, charge.compute_gas) + .charge_gas(charge.name, charge.compute_gas) .map_err(Abort::from_error_as_fatal)?; }; } diff --git a/fvm/src/syscalls/gas.rs b/fvm/src/syscalls/gas.rs index 9009506d4..0c33f4977 100644 --- a/fvm/src/syscalls/gas.rs +++ b/fvm/src/syscalls/gas.rs @@ -1,7 +1,7 @@ use std::str; use super::Context; -use crate::gas::gas_to_milligas; +use crate::gas::Gas; use crate::kernel::{ClassifyResult, Result}; use crate::Kernel; @@ -14,7 +14,5 @@ pub fn charge_gas( let name = str::from_utf8(context.memory.try_slice(name_off, name_len)?).or_illegal_argument()?; // Gas charges from actors are always in full gas units. We use milligas internally, so convert here. - context - .kernel - .charge_milligas(name, gas_to_milligas(compute)) + context.kernel.charge_gas(name, Gas::new(compute)) } diff --git a/fvm/src/syscalls/mod.rs b/fvm/src/syscalls/mod.rs index 440ea6f0f..1e5bf50aa 100644 --- a/fvm/src/syscalls/mod.rs +++ b/fvm/src/syscalls/mod.rs @@ -5,6 +5,7 @@ use cid::Cid; use wasmtime::{AsContextMut, Global, Linker, Memory, Val}; use crate::call_manager::backtrace; +use crate::gas::Gas; use crate::Kernel; pub(crate) mod error; @@ -50,7 +51,7 @@ pub fn update_gas_available( ctx: &mut impl AsContextMut>, ) -> Result<(), Abort> { let mut ctx = ctx.as_context_mut(); - let avail_milligas = ctx.data_mut().kernel.milligas_available(); + let avail_milligas = ctx.data_mut().kernel.gas_available().as_milligas(); let gas_global = ctx.data_mut().avail_gas_global; gas_global @@ -84,7 +85,7 @@ pub fn charge_for_exec( ctx.data_mut() .kernel - .charge_milligas("wasm_exec", milligas_used) + .charge_gas("wasm_exec", Gas::from_milligas(milligas_used)) .map_err(Abort::from_error_as_fatal)?; Ok(()) diff --git a/testing/conformance/src/vm.rs b/testing/conformance/src/vm.rs index 8069e1fa5..05c59ae8c 100644 --- a/testing/conformance/src/vm.rs +++ b/testing/conformance/src/vm.rs @@ -4,7 +4,7 @@ use std::convert::TryFrom; use cid::Cid; use futures::executor::block_on; use fvm::call_manager::{CallManager, DefaultCallManager, FinishRet, InvocationResult}; -use fvm::gas::{GasTracker, PriceList}; +use fvm::gas::{Gas, GasTracker, PriceList}; use fvm::kernel::*; use fvm::machine::{DefaultMachine, Engine, Machine, MachineContext, MultiEngine, NetworkConfig}; use fvm::state_tree::{ActorState, StateTree}; @@ -423,14 +423,14 @@ where // NOT forwarded fn verify_seal(&mut self, vi: &SealVerifyInfo) -> Result { let charge = self.1.price_list.on_verify_seal(vi); - self.0.charge_milligas(charge.name, charge.total())?; + self.0.charge_gas(charge.name, charge.total())?; Ok(true) } // NOT forwarded fn verify_post(&mut self, vi: &WindowPoStVerifyInfo) -> Result { let charge = self.1.price_list.on_verify_post(vi); - self.0.charge_milligas(charge.name, charge.total())?; + self.0.charge_gas(charge.name, charge.total())?; Ok(true) } @@ -442,7 +442,7 @@ where _extra: &[u8], ) -> Result> { let charge = self.1.price_list.on_verify_consensus_fault(); - self.0.charge_milligas(charge.name, charge.total())?; + self.0.charge_gas(charge.name, charge.total())?; // TODO this seems wrong, should probably be parameterized. Ok(None) } @@ -450,14 +450,14 @@ where // NOT forwarded fn verify_aggregate_seals(&mut self, agg: &AggregateSealVerifyProofAndInfos) -> Result { let charge = self.1.price_list.on_verify_aggregate_seals(agg); - self.0.charge_milligas(charge.name, charge.total())?; + self.0.charge_gas(charge.name, charge.total())?; Ok(true) } // NOT forwarded fn verify_replica_update(&mut self, rep: &ReplicaUpdateInfo) -> Result { let charge = self.1.price_list.on_verify_replica_update(rep); - self.0.charge_milligas(charge.name, charge.total())?; + self.0.charge_gas(charge.name, charge.total())?; Ok(true) } } @@ -483,29 +483,21 @@ where C: CallManager>, K: Kernel>, { - fn gas_used(&self) -> i64 { + fn gas_used(&self) -> Gas { self.0.gas_used() } - fn charge_milligas(&mut self, name: &str, compute: i64) -> Result<()> { - self.0.charge_milligas(name, compute) + fn charge_gas(&mut self, name: &str, compute: Gas) -> Result<()> { + self.0.charge_gas(name, compute) } fn price_list(&self) -> &PriceList { self.0.price_list() } - fn milligas_used(&self) -> i64 { - self.0.milligas_used() - } - - fn gas_available(&self) -> i64 { + fn gas_available(&self) -> Gas { self.0.gas_available() } - - fn milligas_available(&self) -> i64 { - self.0.milligas_available() - } } impl MessageOps for TestKernel From 1c209429f41d2e24037925df8cae395c7919de63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Magiera?= Date: Sun, 15 May 2022 18:08:22 -0400 Subject: [PATCH 19/21] chore: Second pass on wasmtime config (#547) --- fvm/src/machine/engine.rs | 35 ++++++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/fvm/src/machine/engine.rs b/fvm/src/machine/engine.rs index a405effe9..233b0f0d1 100644 --- a/fvm/src/machine/engine.rs +++ b/fvm/src/machine/engine.rs @@ -7,6 +7,7 @@ use anyhow::{anyhow, Context}; use cid::Cid; use fvm_ipld_blockstore::Blockstore; use fvm_wasm_instrument::gas_metering::GAS_COUNTER_NAME; +use wasmtime::OptLevel::Speed; use wasmtime::{Global, GlobalType, Linker, Memory, MemoryType, Module, Mutability, Val, ValType}; use crate::gas::WasmGasPrices; @@ -69,9 +70,12 @@ pub fn default_wasmtime_config() -> wasmtime::Config { let mut c = wasmtime::Config::default(); // wasmtime default: false + // We don't want threads, there is no way to ensure determisism c.wasm_threads(false); // wasmtime default: true + // simd isn't supported in wasm-instrument, but if we add support there, we can probably enable this. + // Note: stack limits may need adjusting after this is enabled c.wasm_simd(false); // wasmtime default: false @@ -81,18 +85,23 @@ pub fn default_wasmtime_config() -> wasmtime::Config { c.wasm_memory64(false); // wasmtime default: true + // Note: wasm-instrument only supports this at a basic level, for M2 we will + // need to add more advanced support c.wasm_bulk_memory(true); // wasmtime default: false c.wasm_module_linking(false); // wasmtime default: true - c.wasm_multi_value(false); // ?? + // we should be able to enable this for M2, just need to make sure that it's + // handled correctly in wasm-instrument + c.wasm_multi_value(false); // wasmtime default: depends on the arch // > This is true by default on x86-64, and false by default on other architectures. // - // Not supported in wasm-instrument/parity-wasm + // Not supported in wasm-instrument/parity-wasm; adding support will be complicated. + // Note: stack limits may need adjusting after this is enabled c.wasm_reference_types(false); // wasmtime default: false @@ -105,13 +114,29 @@ pub fn default_wasmtime_config() -> wasmtime::Config { // > not enabled by default. c.cranelift_nan_canonicalization(true); + // wasmtime default: 512KiB // Set to something much higher than the instrumented limiter. - c.max_wasm_stack(64 << 20).unwrap(); + // Note: This is in bytes, while the instrumented limit is in stack elements + c.max_wasm_stack(4 << 20).unwrap(); // Execution cost accouting is done through wasm instrumentation, c.consume_fuel(false); - - // c.cranelift_opt_level(Speed); ? + c.epoch_interruption(false); + + // Disable debug-related things, wasm-instrument doesn't fix debug info + // yet, so those aren't useful, just add overhead + c.debug_info(false); + c.generate_address_map(false); + c.cranelift_debug_verifier(false); + + // Reiterate some defaults + c.guard_before_linear_memory(true); + c.interruptable(false); + c.parallel_compilation(true); + + // Doesn't seem to have significant impact on the time it takes to load code + // todo(M2): make sure this is guaranteed to run in linear time. + c.cranelift_opt_level(Speed); c } From 1183818b1a86190f7df0ed2b3d99f9c7400b6b9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Magiera?= Date: Sun, 15 May 2022 18:52:26 -0400 Subject: [PATCH 20/21] Better stack limits (#554) * wip better stack limits * stack overflow integration test * lower wasm stack / call depth limits * use a threadpool with 64MiB stacks for execution * Improve out-of-stack integration test --- fvm/Cargo.toml | 1 + fvm/src/executor/default.rs | 41 ++++++++-- fvm/src/machine/mod.rs | 4 +- testing/integration/Cargo.toml | 1 + .../tests/fil-stack-overflow-actor/Cargo.toml | 13 ++++ .../tests/fil-stack-overflow-actor/build.rs | 12 +++ .../fil-stack-overflow-actor/rust-toolchain | 1 + .../tests/fil-stack-overflow-actor/src/lib.rs | 64 +++++++++++++++ testing/integration/tests/lib.rs | 77 ++++++++++++++++++- 9 files changed, 205 insertions(+), 9 deletions(-) create mode 100644 testing/integration/tests/fil-stack-overflow-actor/Cargo.toml create mode 100644 testing/integration/tests/fil-stack-overflow-actor/build.rs create mode 100644 testing/integration/tests/fil-stack-overflow-actor/rust-toolchain create mode 100644 testing/integration/tests/fil-stack-overflow-actor/src/lib.rs diff --git a/fvm/Cargo.toml b/fvm/Cargo.toml index cdd61dfbe..3d26b89b5 100644 --- a/fvm/Cargo.toml +++ b/fvm/Cargo.toml @@ -40,6 +40,7 @@ byteorder = "1.4.3" anymap = "0.12.1" blake2b_simd = "1.0.0" fvm-wasm-instrument = { version = "0.2.0", features = ["bulk"] } +yastl = "0.1.2" [dependencies.wasmtime] version = "0.35.2" diff --git a/fvm/src/executor/default.rs b/fvm/src/executor/default.rs index a54acc785..21e8e97b3 100644 --- a/fvm/src/executor/default.rs +++ b/fvm/src/executor/default.rs @@ -11,6 +11,7 @@ use fvm_shared::error::{ErrorNumber, ExitCode}; use fvm_shared::message::Message; use fvm_shared::receipt::Receipt; use fvm_shared::ActorID; +use lazy_static::lazy_static; use num_traits::Zero; use super::{ApplyFailure, ApplyKind, ApplyRet, Executor}; @@ -38,9 +39,24 @@ impl DerefMut for DefaultExecutor { } } +lazy_static! { + static ref EXEC_POOL: yastl::Pool = yastl::Pool::with_config( + 8, + yastl::ThreadConfig::new() + .prefix("fvm-executor") + // fvm needs more than the deafault available stack (2MiB): + // - Max 2048 wasm stack elements, which is 16KiB of 64bit entries + // - Roughly 20KiB overhead per actor call + // - max 1024 nested calls, which means that in the worst case we need ~36MiB of stack + // We also want some more space just to be conservative, so 64MiB seems like a reasonable choice + .stack_size(64 << 20), + ); +} + impl Executor for DefaultExecutor where K: Kernel, + ::Machine: Send, { type Kernel = K; @@ -50,6 +66,26 @@ where msg: Message, apply_kind: ApplyKind, raw_length: usize, + ) -> anyhow::Result { + let mut ret = Err(anyhow!("failed to execute")); + + EXEC_POOL.scoped(|scope| { + scope.execute(|| ret = self.execute_message_inner(msg, apply_kind, raw_length)); + }); + + ret + } +} + +impl DefaultExecutor +where + K: Kernel, +{ + fn execute_message_inner( + &mut self, + msg: Message, + apply_kind: ApplyKind, + raw_length: usize, ) -> anyhow::Result { // Validate if the message was correct, charge for it, and extract some preliminary data. let (sender_id, gas_cost, inclusion_cost) = @@ -190,12 +226,7 @@ where }), } } -} -impl DefaultExecutor -where - K: Kernel, -{ /// Create a new [`DefaultExecutor`] for executing messages on the [`Machine`]. pub fn new(m: ::Machine) -> Self { Self(Some(m)) diff --git a/fvm/src/machine/mod.rs b/fvm/src/machine/mod.rs index d89cc8f2b..0e6afc538 100644 --- a/fvm/src/machine/mod.rs +++ b/fvm/src/machine/mod.rs @@ -124,8 +124,8 @@ impl NetworkConfig { pub fn new(network_version: NetworkVersion) -> Self { NetworkConfig { network_version, - max_call_depth: 4096, - max_wasm_stack: 64 * 1024, + max_call_depth: 1024, + max_wasm_stack: 2048, actor_debugging: false, builtin_actors_override: None, price_list: price_list_by_network_version(network_version), diff --git a/testing/integration/Cargo.toml b/testing/integration/Cargo.toml index d2e73ddca..3ceba7a98 100644 --- a/testing/integration/Cargo.toml +++ b/testing/integration/Cargo.toml @@ -39,6 +39,7 @@ features = ["cranelift", "pooling-allocator", "parallel-compilation"] wabt = "0.10.0" serde = { version = "1.0", features = ["derive"] } fil_hello_world_actor = { path = 'tests/fil-hello-world-actor', version = '0.1' } +fil_stack_overflow_actor = { path = 'tests/fil-stack-overflow-actor', version = '0.1' } [features] default = ["fvm/testing", "fvm_shared/testing"] diff --git a/testing/integration/tests/fil-stack-overflow-actor/Cargo.toml b/testing/integration/tests/fil-stack-overflow-actor/Cargo.toml new file mode 100644 index 000000000..333f57121 --- /dev/null +++ b/testing/integration/tests/fil-stack-overflow-actor/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "fil_stack_overflow_actor" +version = "0.1.0" +edition = "2021" + +[dependencies] +fvm_sdk = { version = "0.6.1", path = "../../../../sdk" } +fvm_shared = { version = "0.6.1", path = "../../../../shared" } + + +[build-dependencies] +wasm-builder = "3.0.1" +wasmtime = "0.33.0" diff --git a/testing/integration/tests/fil-stack-overflow-actor/build.rs b/testing/integration/tests/fil-stack-overflow-actor/build.rs new file mode 100644 index 000000000..0f2aa8a56 --- /dev/null +++ b/testing/integration/tests/fil-stack-overflow-actor/build.rs @@ -0,0 +1,12 @@ +fn main() { + use wasm_builder::WasmBuilder; + WasmBuilder::new() + .with_current_project() + .import_memory() + .append_to_rust_flags("-Ctarget-feature=+crt-static") + .append_to_rust_flags("-Cpanic=abort") + .append_to_rust_flags("-Coverflow-checks=true") + .append_to_rust_flags("-Clto=true") + .append_to_rust_flags("-Copt-level=z") + .build() +} diff --git a/testing/integration/tests/fil-stack-overflow-actor/rust-toolchain b/testing/integration/tests/fil-stack-overflow-actor/rust-toolchain new file mode 100644 index 000000000..07ade694b --- /dev/null +++ b/testing/integration/tests/fil-stack-overflow-actor/rust-toolchain @@ -0,0 +1 @@ +nightly \ No newline at end of file diff --git a/testing/integration/tests/fil-stack-overflow-actor/src/lib.rs b/testing/integration/tests/fil-stack-overflow-actor/src/lib.rs new file mode 100644 index 000000000..2b60cb8c5 --- /dev/null +++ b/testing/integration/tests/fil-stack-overflow-actor/src/lib.rs @@ -0,0 +1,64 @@ +use fvm_sdk as sdk; +use fvm_shared::address::Address; +use fvm_shared::error::ExitCode; + +#[no_mangle] +pub fn invoke(_: u32) -> u32 { + let m = sdk::message::method_number(); + // If we start with method 1, we'll be over recursive send limit, starting + // with method 2 should be fine + if m > 1026 { + sdk::vm::abort(0x42, None); + } + + if m == 1 { + // if method 0, we want to run out of stack + recurse(m, 1000) + } else { + // 5 stack elems per level (wasm-instrument charges for highest use in the + // function) + some overhead mean that with the 2048 element wasm limit we + // can do 396 recursive calls while still being able do do a send at that + // depth + recurse(m, 396) + } +} + +// we need two recurse functions; just one gets optimized into wasm loop + +#[inline(never)] +pub fn recurse(m: u64, n: u64) -> u32 { + if n > 0 { + call_extern(); + + return recurse2(m, n - 1); + } + do_send(m) +} + +#[inline(never)] +pub fn recurse2(m: u64, n: u64) -> u32 { + if n > 0 { + call_extern(); + + return recurse(m, n - 1); + } + do_send(m) +} + +// external call to prevent the compiler from doing smart things +#[inline(never)] +pub fn call_extern() { + let _ = sdk::message::method_number(); +} + +#[inline(never)] +pub fn do_send(m: u64) -> u32 { + let r = sdk::send::send(&Address::new_id(10000), m + 1, Vec::new().into(), 0.into()); + match r { + Ok(rec) => match rec.exit_code { + ExitCode::OK => 0, + e => sdk::vm::abort(e.value() | 0x80000000, None), + }, + Err(e) => sdk::vm::abort((e as u32) | 0xc0000000, None), + } +} diff --git a/testing/integration/tests/lib.rs b/testing/integration/tests/lib.rs index d87c10504..887c10216 100644 --- a/testing/integration/tests/lib.rs +++ b/testing/integration/tests/lib.rs @@ -5,13 +5,13 @@ use std::env; use cid::multihash::Multihash; use cid::Cid; use fvm::executor::{ApplyKind, Executor}; -use fvm_integration_tests::tester::{Account, Tester}; +use fvm_integration_tests::tester::{Account, IntegrationExecutor, Tester}; use fvm_ipld_blockstore::{Blockstore, MemoryBlockstore}; use fvm_ipld_encoding::tuple::*; use fvm_ipld_encoding::DAG_CBOR; use fvm_shared::address::Address; use fvm_shared::bigint::BigInt; -use fvm_shared::error::ExitCode; +use fvm_shared::error::{ErrorNumber, ExitCode}; use fvm_shared::message::Message; use fvm_shared::state::StateTreeVersion; use fvm_shared::version::NetworkVersion; @@ -28,6 +28,9 @@ pub struct State { const WASM_COMPILED_PATH: &str = "../../target/debug/wbuild/fil_hello_world_actor/fil_hello_world_actor.compact.wasm"; +const WASM_COMPILED_PATH_OVERFLOW: &str = + "../../target/debug/wbuild/fil_stack_overflow_actor/fil_stack_overflow_actor.compact.wasm"; + #[test] fn hello_world() { // Instantiate tester @@ -80,6 +83,76 @@ fn hello_world() { assert_eq!(res.msg_receipt.exit_code.value(), 16) } +#[test] +fn native_stack_overflow() { + // Instantiate tester + let mut tester = Tester::new( + NetworkVersion::V16, + StateTreeVersion::V4, + MemoryBlockstore::default(), + ) + .unwrap(); + + let sender: [Account; 1] = tester.create_accounts().unwrap(); + + // Get wasm bin + let wasm_path = env::current_dir() + .unwrap() + .join(WASM_COMPILED_PATH_OVERFLOW) + .canonicalize() + .unwrap(); + let wasm_bin = std::fs::read(wasm_path).expect("Unable to read file"); + + // Set actor state + let actor_state = State::default(); + let state_cid = tester.set_state(&actor_state).unwrap(); + + // Set actor + let actor_address = Address::new_id(10000); + + tester + .set_actor_from_bin(&wasm_bin, state_cid, actor_address, BigInt::zero()) + .unwrap(); + + // Instantiate machine + tester.instantiate_machine().unwrap(); + + let exec_test = |exec: &mut IntegrationExecutor, method| { + // Send message + let message = Message { + from: sender[0].1, + to: actor_address, + gas_limit: 10_000_000_000, + method_num: method, + sequence: method - 1, + ..Message::default() + }; + + let res = exec + .execute_message(message, ApplyKind::Explicit, 100) + .unwrap(); + + res.msg_receipt.exit_code.value() + }; + + let mut executor = tester.executor.unwrap(); + + // on method 0 the test actor should run out of stack + assert_eq!( + exec_test(&mut executor, 1), + ExitCode::SYS_ILLEGAL_INSTRUCTION.value() + ); + + // on method 1 the test actor should run out of recursive call limit + assert_eq!( + exec_test(&mut executor, 2), + 0xc0000000 + (ErrorNumber::LimitExceeded as u32) + ); + + // on method 2 the test actor should finish successfully + assert_eq!(exec_test(&mut executor, 3), 0x80000042); +} + #[test] fn out_of_gas() { const WAT: &str = r#" From 86f3c49b7632ef1cc05f7627ad121a1852c2218f Mon Sep 17 00:00:00 2001 From: Alfonso de la Rocha Date: Mon, 16 May 2022 10:50:44 +0200 Subject: [PATCH 21/21] rebased upstream --- shared/src/actor/builtin.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shared/src/actor/builtin.rs b/shared/src/actor/builtin.rs index 338a63405..eda60b112 100644 --- a/shared/src/actor/builtin.rs +++ b/shared/src/actor/builtin.rs @@ -84,8 +84,8 @@ impl TryFrom<&str> for Type { "multisig" => Type::Multisig, "reward" => Type::Reward, "verifiedregistry" => Type::VerifiedRegistry, - "sca" => Type::SCA, - "subnet" => Type::Subnet, + "hierarchical_sca" => Type::SCA, + "hierarchical_subnet" => Type::Subnet, _ => return Err(String::from("unrecognized actor type")), }; Ok(ret)