Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: fvm-wasm-instrument stack limiter and gas #494

Merged
merged 31 commits into from
May 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
096ae9d
feat: fvm-wasm-instrument stack limiter
magik6k Apr 14, 2022
f5b4f7f
Swap wasmtime fuel to instrumented gas
magik6k Apr 22, 2022
d093b53
Instrumentation rules on pricelist
magik6k Apr 23, 2022
48d78e0
Don't consume wasmtime fuel
magik6k Apr 23, 2022
a85bc94
Fix gas handoff on aborts
magik6k Apr 24, 2022
e3868bf
Make out-of-gas in execution work
magik6k Apr 25, 2022
79b7a3a
Cleanup frgas logic
magik6k Apr 25, 2022
0183944
More reasonable initial exec cost
magik6k Apr 25, 2022
689f8a1
Validate raw wasm before instrumenting
magik6k Apr 25, 2022
ef868f2
handle negative nubers in frgas_to_gas correctly
magik6k Apr 25, 2022
d926f97
miligas for wasm execution
magik6k Apr 25, 2022
7fe3153
convert gastracker to miligas
magik6k Apr 25, 2022
e4282ee
mili means 1/1000
magik6k Apr 25, 2022
9d2e98c
basic exec gas test
magik6k Apr 25, 2022
fc651b3
Add basic integration test for out-of-gas
magik6k Apr 27, 2022
d1a7ecd
mili->milli
magik6k Apr 27, 2022
8880415
Address review
magik6k Apr 27, 2022
95f4de3
Fine clippy
magik6k Apr 27, 2022
ced762b
stack limit to network config
magik6k Apr 27, 2022
4f4cd7e
gas integration test
magik6k Apr 27, 2022
75376c6
fix: avoid unwrap by using a dummy gas global
Stebalien Apr 27, 2022
5841c2a
Avoid one gas return path
magik6k Apr 28, 2022
06dda99
More review addressing
magik6k Apr 28, 2022
d776857
Engine per network
magik6k Apr 28, 2022
72ce6df
MultiEngine
magik6k Apr 29, 2022
b7f6b88
Fix conformance
magik6k Apr 29, 2022
43127db
Make clippy happy
magik6k Apr 29, 2022
c2b5e38
Integration test out-of-stack instrumentation
magik6k May 4, 2022
0adb846
Apply suggestions from code review
magik6k May 4, 2022
0c69efc
Review addressing
magik6k May 4, 2022
ea08252
Fix comment
magik6k May 4, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions fvm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
68 changes: 39 additions & 29 deletions fvm/src/call_manager/default.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
use std::cmp::max;

use anyhow::Context;
use derive_more::{Deref, DerefMut};
use fvm_ipld_encoding::{RawBytes, DAG_CBOR};
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;
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -362,6 +345,26 @@ where

// From this point on, there are no more syscall errors, only aborts.
let result: std::result::Result<RawBytes, Abort> = (|| {
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")
Expand All @@ -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
Expand Down
121 changes: 102 additions & 19 deletions fvm/src/gas/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
magik6k marked this conversation as resolved.
Show resolved Hide resolved
}

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",
)));
Comment on lines +40 to +42
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Stebalien do we not have sugar for creating Fatal errors with string interpolations?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

anyhow!("foo").or_fatal()

}

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<i64> {
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;
Comment on lines +75 to +80
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit weird, it feels like we're doing borrow checking at runtime. Can't we write this code in a way that the Rust borrow checker guarantees the correctness?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, no. But we could probably avoid this dance by remembering the starting gas in the InvocationContext?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish we could, but I don't see a great way to do this, especially with the dance that happens in syscall handlers.


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::*;
Expand All @@ -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);
}
}
Loading