Skip to content

Commit

Permalink
Avoid one gas return path
Browse files Browse the repository at this point in the history
  • Loading branch information
magik6k committed Apr 28, 2022
1 parent 34ab703 commit 20d3ed1
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 31 deletions.
40 changes: 16 additions & 24 deletions fvm/src/call_manager/default.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::cmp::max;

use anyhow::Context;
use derive_more::{Deref, DerefMut};
use fvm_ipld_encoding::{RawBytes, DAG_CBOR};
Expand All @@ -9,6 +7,7 @@ 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;
Expand Down Expand Up @@ -331,13 +330,8 @@ where
super::NO_DATA_BLOCK_ID
};

let initial_milligas = match kernel.borrow_milligas() {
Ok(mg) => max(mg, 0),
Err(err) => return (Err(err), kernel.into_call_manager()),
};

// Make a store and available gas
let mut store = engine.new_store(kernel, initial_milligas);
let mut store = engine.new_store(kernel);

// Instantiate the module.
let instance = match engine
Expand All @@ -346,24 +340,23 @@ where
.or_fatal()
{
Ok(ret) => ret,
Err(err) => {
// return gas before returning the error
if let Err(e) = store
.data_mut()
.kernel
.return_milligas("getinstance_fail", initial_milligas)
{
// this shouldn't ever fail as we didn't charge any gas since borrowing above
// but just in case, log the error
log::error!("failed to return gas after failed get_instance: {}", e)
};

return (Err(err), store.into_data().kernel.into_call_manager());
}
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<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 avaialable gas")),
})?;

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 @@ -373,8 +366,7 @@ where
// Invoke it.
let res = invoke.call(&mut store, (param_id,));

// Update GasTracker gas
use wasmtime::Val;
// Return gas tracking to GasTracker
let available_milligas =
match store.data_mut().avail_gas_global.clone().get(&mut store) {
Val::I64(g) => Ok(g),
Expand Down
2 changes: 1 addition & 1 deletion fvm/src/gas/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ impl GasTracker {
pub fn borrow_milligas(&mut self) -> Result<i64> {
if !self.own_limit {
return Err(ExecutionError::Fatal(anyhow::Error::msg(
"get_gas called when gas_limit owned by execution",
"borrow_milligas called on GasTracker which doesn't own gas limit",
)));
}
self.own_limit = false;
Expand Down
8 changes: 2 additions & 6 deletions fvm/src/machine/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,11 +254,7 @@ impl Engine {
}

/// Construct a new wasmtime "store" from the given kernel.
pub fn new_store<K: Kernel>(
&self,
kernel: K,
milligas: i64,
) -> wasmtime::Store<InvocationData<K>> {
pub fn new_store<K: Kernel>(&self, kernel: K) -> wasmtime::Store<InvocationData<K>> {
let id = InvocationData {
kernel,
last_error: None,
Expand All @@ -267,7 +263,7 @@ impl Engine {

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(milligas))
let gg = Global::new(&mut store, ggtype, Val::I64(0))
.expect("failed to create available_gas global");
store.data_mut().avail_gas_global = gg;

Expand Down

0 comments on commit 20d3ed1

Please sign in to comment.