Fix internal send bug, remove message ref from runtime

vm/interpreter/src/default_runtime.rs

@@ -36,6 +36,7 @@ const ACTOR_EXEC_GAS: GasCharge = GasCharge {
     storage_gas: 0,
 };
 
+#[derive(Debug, Clone)]
 struct VMMsg {
     caller: Address,
     receiver: Address,
@@ -55,12 +56,11 @@ impl MessageInfo for VMMsg {
 }
 
 /// Implementation of the Runtime trait.
-pub struct DefaultRuntime<'db, 'msg, 'st, 'sys, 'r, 'act, BS, SYS, R, P = DevnetParams> {
+pub struct DefaultRuntime<'db, 'st, 'sys, 'r, 'act, BS, SYS, R, P = DevnetParams> {
     state: &'st mut StateTree<'db, BS>,
     store: GasBlockStore<'db, BS>,
     syscalls: GasSyscalls<'sys, SYS>,
     gas_tracker: Rc<RefCell<GasTracker>>,
-    message: &'msg UnsignedMessage,
     vm_msg: VMMsg,
     epoch: ChainEpoch,
     origin: Address,
@@ -74,8 +74,8 @@ pub struct DefaultRuntime<'db, 'msg, 'st, 'sys, 'r, 'act, BS, SYS, R, P = Devnet
     params: PhantomData<P>,
 }
 
-impl<'db, 'msg, 'st, 'sys, 'r, 'act, BS, SYS, R, P>
-    DefaultRuntime<'db, 'msg, 'st, 'sys, 'r, 'act, BS, SYS, R, P>
+impl<'db, 'st, 'sys, 'r, 'act, BS, SYS, R, P>
+    DefaultRuntime<'db, 'st, 'sys, 'r, 'act, BS, SYS, R, P>
 where
     BS: BlockStore,
     SYS: Syscalls,
@@ -89,7 +89,7 @@ where
         store: &'db BS,
         syscalls: &'sys SYS,
         gas_used: i64,
-        message: &'msg UnsignedMessage,
+        message: &UnsignedMessage,
         epoch: ChainEpoch,
         origin: Address,
         origin_nonce: u64,
@@ -119,16 +119,15 @@ where
 
         let vm_msg = VMMsg {
             caller: caller_id,
-            receiver: *message.receiver(),
-            value_received: message.value_received().clone(),
+            receiver: *message.to(),
+            value_received: message.value().clone(),
         };
 
         Ok(DefaultRuntime {
             state,
             store: gas_block_store,
             syscalls: gas_syscalls,
             gas_tracker,
-            message,
             vm_msg,
             epoch,
             origin,
@@ -242,11 +241,18 @@ where
         value: TokenAmount,
         params: Serialized,
     ) -> Result<Serialized, ActorError> {
+        // ID must be resolved because otherwise would be done in creation of new runtime.
+        // TODO revisit this later, it's possible there are no code paths this is needed.
+        let from_id = self.resolve_address(&from)?.ok_or_else(|| {
+            actor_error!(SysErrInvalidReceiver;
+            "resolving from address in internal send failed")
+        })?;
+
         let msg = UnsignedMessage {
             from,
             to,
             method_num: method,
-            value,
+            value: value.clone(),
             params,
             gas_limit: self.gas_available(),
             version: Default::default(),
@@ -255,13 +261,21 @@ where
             gas_premium: Default::default(),
         };
 
+        let prev_msg = self.vm_msg.clone();
+        self.vm_msg = VMMsg {
+            caller: from_id,
+            receiver: to,
+            value_received: value,
+        };
+
         // snapshot state tree
         let snapshot = self
             .state
             .snapshot()
             .map_err(|e| actor_error!(fatal("failed to create snapshot {}", e)))?;
 
         let send_res = vm_send::<BS, SYS, R, P>(self, &msg, None);
+        self.vm_msg = prev_msg;
         send_res.map_err(|e| {
             if let Err(e) = self.state.revert_to_snapshot(&snapshot) {
                 actor_error!(fatal("failed to revert snapshot: {}", e))
@@ -311,7 +325,7 @@ where
     }
 }
 
-impl<BS, SYS, R, P> Runtime<BS> for DefaultRuntime<'_, '_, '_, '_, '_, '_, BS, SYS, R, P>
+impl<BS, SYS, R, P> Runtime<BS> for DefaultRuntime<'_, '_, '_, '_, '_, BS, SYS, R, P>
 where
     BS: BlockStore,
     SYS: Syscalls,
@@ -361,7 +375,7 @@ where
     }
 
     fn current_balance(&self) -> Result<TokenAmount, ActorError> {
-        self.get_balance(self.message.to())
+        self.get_balance(self.message().receiver())
     }
 
     fn resolve_address(&self, address: &Address) -> Result<Option<Address>, ActorError> {
@@ -478,7 +492,7 @@ where
         }
 
         let ret = self
-            .internal_send(*self.message.receiver(), to, method, value, params)
+            .internal_send(*self.message().receiver(), to, method, value, params)
             .map_err(|e| {
                 warn!(
                     "internal send failed: (to: {}) (method: {}) {}",
@@ -538,15 +552,15 @@ where
     /// May only be called by the actor itself.
     fn delete_actor(&mut self, beneficiary: &Address) -> Result<(), ActorError> {
         self.charge_gas(self.price_list.on_delete_actor())?;
+        let receiver = *self.message().receiver();
         let balance = self
             .state
-            .get_actor(self.message.to())
-            .map_err(|e| actor_error!(fatal("failed to get actor {}, {}", self.message.to(), e)))?
+            .get_actor(&receiver)
+            .map_err(|e| actor_error!(fatal("failed to get actor {}, {}", receiver, e)))?
             .ok_or_else(
                 || actor_error!(SysErrorIllegalActor; "failed to load actor in delete actor"),
             )
             .map(|act| act.balance)?;
-        let receiver = *self.message().receiver();
         if balance != 0.into() {
             // Transfer the executing actor's balance to the beneficiary
             transfer(self.state, &receiver, beneficiary, &balance).map_err(|e| {
@@ -606,8 +620,8 @@ where
 
 /// Shared logic between the DefaultRuntime and the Interpreter.
 /// It invokes methods on different Actors based on the Message.
-pub fn vm_send<'db, 'msg, 'st, 'sys, 'r, 'act, BS, SYS, R, P>(
-    rt: &mut DefaultRuntime<'db, 'msg, 'st, 'sys, 'r, 'act, BS, SYS, R, P>,
+pub fn vm_send<'db, 'st, 'sys, 'r, 'act, BS, SYS, R, P>(
+    rt: &mut DefaultRuntime<'db, 'st, 'sys, 'r, 'act, BS, SYS, R, P>,
     msg: &UnsignedMessage,
     gas_cost: Option<GasCharge>,
 ) -> Result<Serialized, ActorError>
@@ -621,12 +635,6 @@ where
         rt.charge_gas(cost)?;
     }
 
-    // TODO maybe move this
-    rt.charge_gas(
-        rt.price_list()
-            .on_method_invocation(msg.value(), msg.method_num()),
-    )?;
-
     let to_actor = match rt
         .state
         .get_actor(msg.to())
@@ -639,6 +647,11 @@ where
         }
     };
 
+    rt.charge_gas(
+        rt.price_list()
+            .on_method_invocation(msg.value(), msg.method_num()),
+    )?;
+
     if msg.value() > &TokenAmount::from(0) {
         transfer(rt.state, &msg.from(), &msg.to(), &msg.value())
             .map_err(|e| e.wrap("failed to transfer funds"))?;
@@ -712,8 +725,8 @@ fn transfer<BS: BlockStore>(
 }
 
 /// Calls actor code with method and parameters.
-fn invoke<'db, 'msg, 'st, 'sys, 'r, 'act, BS, SYS, R, P>(
-    rt: &mut DefaultRuntime<'db, 'msg, 'st, 'sys, 'r, 'act, BS, SYS, R, P>,
+fn invoke<'db, 'st, 'sys, 'r, 'act, BS, SYS, R, P>(
+    rt: &mut DefaultRuntime<'db, 'st, 'sys, 'r, 'act, BS, SYS, R, P>,
     code: Cid,
     method_num: MethodNum,
     params: &Serialized,

vm/interpreter/src/vm.rs

@@ -431,13 +431,13 @@ where
     }
     /// Instantiates a new Runtime, and calls internal_send to do the execution.
     #[allow(clippy::type_complexity)]
-    fn send<'m>(
+    fn send(
         &mut self,
-        msg: &'m UnsignedMessage,
+        msg: &UnsignedMessage,
         gas_cost: Option<GasCharge>,
     ) -> (
         Serialized,
-        Option<DefaultRuntime<'db, 'm, '_, '_, '_, '_, DB, SYS, R, P>>,
+        Option<DefaultRuntime<'db, '_, '_, '_, '_, DB, SYS, R, P>>,
         Option<ActorError>,
     ) {
         let res = DefaultRuntime::new(

vm/runtime/src/lib.rs

@@ -23,13 +23,12 @@ use filecoin_proofs_api::{
 use forest_encoding::{blake2b_256, Cbor};
 use ipld_blockstore::BlockStore;
 use log::warn;
-use message::Message;
 use rayon::prelude::*;
 use std::collections::BTreeMap;
 use std::collections::HashMap;
 use std::convert::{TryFrom, TryInto};
 use std::error::Error as StdError;
-use vm::{actor_error, ActorError, ExitCode, MethodNum, Randomness, Serialized, TokenAmount};
+use vm::{ActorError, MethodNum, Randomness, Serialized, TokenAmount};
 
 /// Runtime is the VM's internal runtime object.
 /// this is everything that is accessible to actors, beyond parameters.
@@ -163,21 +162,6 @@ pub trait MessageInfo {
     fn value_received(&self) -> &TokenAmount;
 }
 
-impl<M> MessageInfo for M
-where
-    M: Message,
-{
-    fn caller(&self) -> &Address {
-        Message::from(self)
-    }
-    fn receiver(&self) -> &Address {
-        Message::to(self)
-    }
-    fn value_received(&self) -> &TokenAmount {
-        Message::value(self)
-    }
-}
-
 /// Pure functions implemented as primitives by the runtime.
 pub trait Syscalls {
     /// Verifies that a signature is valid for an address and plaintext.
@@ -206,8 +190,7 @@ pub trait Syscalls {
         let mut fcp_pieces: Vec<proofs::PieceInfo> = pieces
             .iter()
             .map(proofs::PieceInfo::try_from)
-            .collect::<Result<_, &'static str>>()
-            .map_err(|e| actor_error!(ErrPlaceholder; e))?;
+            .collect::<Result<_, &'static str>>()?;
 
         // pad remaining space with 0 piece commitments
         {
@@ -225,8 +208,7 @@ pub trait Syscalls {
             }
         }
 
-        let comm_d = compute_comm_d(proof_type.try_into()?, &fcp_pieces)
-            .map_err(|e| actor_error!(ErrPlaceholder; e))?;
+        let comm_d = compute_comm_d(proof_type.try_into()?, &fcp_pieces)?;
 
         Ok(data_commitment_v1_to_cid(&comm_d)?)
     }