Skip to content

Commit

Permalink
fix: fvm: remove all unwraps
Browse files Browse the repository at this point in the history
We were catching panics here, but the unwraps ended up hiding the actual
error message because rust backtraces are disabled by default.
  • Loading branch information
Stebalien authored and arajasek committed Feb 14, 2023
1 parent 4c503e5 commit 979d86e
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 13 deletions.
16 changes: 12 additions & 4 deletions rust/src/fvm/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ impl MultiEngineContainer {
}

pub fn get(&self, nv: u32) -> anyhow::Result<Arc<dyn AbstractMultiEngine + 'static>> {
let mut locked = self.engines.lock().unwrap();
let mut locked = self
.engines
.lock()
.map_err(|e| anyhow!("engine lock poisoned: {e}"))?;
Ok(match locked.entry(nv) {
Entry::Occupied(v) => v.get().clone(),
Entry::Vacant(v) => v
Expand Down Expand Up @@ -254,8 +257,10 @@ mod v2 {
self,
Message2 {
version: msg.version.try_into().context("invalid message version")?,
from: Address2::from_bytes(&msg.from.to_bytes()).unwrap(),
to: Address2::from_bytes(&msg.to.to_bytes()).unwrap(),
from: Address2::from_bytes(&msg.from.to_bytes())
.context("unsupported from address")?,
to: Address2::from_bytes(&msg.to.to_bytes())
.context("unsupported to address")?,
sequence: msg.sequence,
value: TokenAmount2::from_atto(msg.value.atto().clone()),
method_num: msg.method_num,
Expand Down Expand Up @@ -365,7 +370,10 @@ mod v2 {
value,
} => Some(ExecutionEvent::Call {
from,
to: Address::from_bytes(&to.to_bytes()).unwrap(),
to: Address::from_bytes(&to.to_bytes())
// There's nothing we can do here, so we use the "chaos" actor
// ID.
.unwrap_or_else(|_| Address::new_id(98)),
method,
params: bytes_to_block(params),
value: TokenAmount::from_atto(value.atto().clone()),
Expand Down
38 changes: 29 additions & 9 deletions rust/src/fvm/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,9 @@ fn fvm_machine_execute_message(
let mut executor = executor
.machine
.as_ref()
.expect("missing executor")
.context("missing executor")?
.lock()
.unwrap();
.map_err(|e| anyhow!("executor lock poisoned: {e}"))?;
let apply_ret = executor.execute_message(message, apply_kind, chain_len as usize)?;

let exec_trace = if !apply_ret.exec_trace.is_empty() {
Expand Down Expand Up @@ -243,11 +243,31 @@ fn fvm_machine_execute_message(
.map(|info| info.to_string().into_boxed_str().into());

// TODO: use the non-bigint token amount everywhere in the FVM
let penalty: u128 = apply_ret.penalty.atto().try_into().unwrap();
let miner_tip: u128 = apply_ret.miner_tip.atto().try_into().unwrap();
let base_fee_burn: u128 = apply_ret.base_fee_burn.atto().try_into().unwrap();
let over_estimation_burn: u128 = apply_ret.over_estimation_burn.atto().try_into().unwrap();
let refund: u128 = apply_ret.refund.atto().try_into().unwrap();
let penalty: u128 = apply_ret
.penalty
.atto()
.try_into()
.context("penalty exceeds u128 attoFIL")?;
let miner_tip: u128 = apply_ret
.miner_tip
.atto()
.try_into()
.context("miner tip exceeds u128 attoFIL")?;
let base_fee_burn: u128 = apply_ret
.base_fee_burn
.atto()
.try_into()
.context("base fee burn exceeds u128 attoFIL")?;
let over_estimation_burn: u128 = apply_ret
.over_estimation_burn
.atto()
.try_into()
.context("overestimation burn exceeds u128 attoFIL")?;
let refund: u128 = apply_ret
.refund
.atto()
.try_into()
.context("refund exceeds u128 attoFIL")?;
let gas_refund = apply_ret.gas_refund;
let gas_burned = apply_ret.gas_burned;

Expand Down Expand Up @@ -313,9 +333,9 @@ fn fvm_machine_flush(executor: &'_ InnerFvmMachine) -> repr_c::Box<Result<c_slic
let mut executor = executor
.machine
.as_ref()
.expect("missing executor")
.context("missing executor")?
.lock()
.unwrap();
.map_err(|e| anyhow!("executor lock poisoned: {e}"))?;
let cid = executor.flush()?;

Ok(cid.to_bytes().into_boxed_slice().into())
Expand Down

0 comments on commit 979d86e

Please sign in to comment.