From 979d86ea768903e0325af3122e8369e3ee7562e7 Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Mon, 13 Feb 2023 10:15:23 -0800
Subject: [PATCH] fix: fvm: remove all unwraps

We were catching panics here, but the unwraps ended up hiding the actual
error message because rust backtraces are disabled by default.
---
 rust/src/fvm/engine.rs  | 16 ++++++++++++----
 rust/src/fvm/machine.rs | 38 +++++++++++++++++++++++++++++---------
 2 files changed, 41 insertions(+), 13 deletions(-)

diff --git a/rust/src/fvm/engine.rs b/rust/src/fvm/engine.rs
index 86d393f2..e54e0b79 100644
--- a/rust/src/fvm/engine.rs
+++ b/rust/src/fvm/engine.rs
@@ -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
@@ -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,
@@ -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()),
diff --git a/rust/src/fvm/machine.rs b/rust/src/fvm/machine.rs
index 3045a435..6b4d1e75 100644
--- a/rust/src/fvm/machine.rs
+++ b/rust/src/fvm/machine.rs
@@ -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() {
@@ -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;
 
@@ -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())