Reject upgrades if actor already on call stack

Author: fridrik01

fvm/src/call_manager/default.rs

@@ -14,6 +14,7 @@ use fvm_shared::event::StampedEvent;
 use fvm_shared::sys::BlockId;
 use fvm_shared::{ActorID, METHOD_SEND};
 use num_traits::Zero;
+use std::collections::HashMap;
 
 use super::state_access_tracker::{ActorAccessState, StateAccessTracker};
 use super::{Backtrace, CallManager, Entrypoint, InvocationResult, NO_DATA_BLOCK_ID};
@@ -75,6 +76,8 @@ pub struct InnerDefaultCallManager<M: Machine> {
     limits: M::Limiter,
     /// Accumulator for events emitted in this call stack.
     events: EventsAccumulator,
+    /// A map of ActorID and how often they appear on the call stack.
+    actor_call_stack: HashMap<ActorID, i32>,
 }
 
 #[doc(hidden)]
@@ -159,6 +162,7 @@ where
             limits,
             events: Default::default(),
             state_access_tracker,
+            actor_call_stack: HashMap::new(),
         })))
     }
 
@@ -327,6 +331,14 @@ where
         self.nonce
     }
 
+    fn get_actor_call_stack(&self) -> &HashMap<ActorID, i32> {
+        &self.actor_call_stack
+    }
+
+    fn get_actor_call_stack_mut(&mut self) -> &mut HashMap<ActorID, i32> {
+        &mut self.actor_call_stack
+    }
+
     fn next_actor_address(&self) -> Address {
         // Base the next address on the address specified as the message origin. This lets us use,
         // e.g., an f2 address even if we can't look it up anywhere.

fvm/src/call_manager/mod.rs

@@ -7,6 +7,7 @@ use fvm_shared::econ::TokenAmount;
 use fvm_shared::error::ExitCode;
 use fvm_shared::upgrade::UpgradeInfo;
 use fvm_shared::{ActorID, MethodNum};
+use std::collections::HashMap;
 
 use crate::engine::Engine;
 use crate::gas::{Gas, GasCharge, GasTimer, GasTracker, PriceList};
@@ -119,6 +120,9 @@ pub trait CallManager: 'static {
         delegated_address: Option<Address>,
     ) -> Result<()>;
 
+    fn get_actor_call_stack(&self) -> &HashMap<ActorID, i32>;
+    fn get_actor_call_stack_mut(&mut self) -> &mut HashMap<ActorID, i32>;
+
     /// Resolve an address into an actor ID, charging gas as appropriate.
     fn resolve_address(&self, address: &Address) -> Result<Option<ActorID>>;
 

fvm/src/kernel/default.rs

@@ -137,6 +137,12 @@ where
             return Err(syscall_error!(LimitExceeded; "cannot store return block").into());
         }
 
+        self.call_manager
+            .get_actor_call_stack_mut()
+            .entry(self.actor_id)
+            .and_modify(|count| *count += 1)
+            .or_insert(1);
+
         // Send.
         let result = self.call_manager.with_transaction(|cm| {
             cm.call_actor::<K>(
@@ -873,6 +879,14 @@ where
             .create_actor(code_id, actor_id, delegated_address)
     }
 
+    fn is_actor_on_call_stack(&self) -> bool {
+        self.call_manager
+            .get_actor_call_stack()
+            .get(&self.actor_id)
+            .map(|count| *count > 0)
+            .unwrap_or(false)
+    }
+
     fn upgrade_actor<K: Kernel>(
         &mut self,
         new_code_cid: Cid,

fvm/src/kernel/mod.rs

@@ -215,6 +215,8 @@ pub trait ActorOps {
         params_id: BlockId,
     ) -> Result<SendResult>;
 
+    fn is_actor_on_call_stack(&self) -> bool;
+
     /// Installs actor code pointed by cid
     #[cfg(feature = "m2-native")]
     fn install_actor(&mut self, code_cid: Cid) -> Result<()>;

fvm/src/syscalls/actor.rs

@@ -121,6 +121,14 @@ pub fn upgrade_actor<K: Kernel>(
         Err(err) => return err.into(),
     };
 
+    // actor cannot be upgraded if its already on the call stack
+    if context.kernel.is_actor_on_call_stack() {
+        return ControlFlow::Error(syscall_error!(
+            Forbidden;
+            "cannot upgrade actor if it is already on the call stack"
+        ));
+    };
+
     match context.kernel.upgrade_actor::<K>(cid, params_id) {
         Ok(SendResult {
             block_id,

fvm/tests/dummy.rs

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0, MIT
 use std::borrow::Borrow;
 use std::cell::RefCell;
+use std::collections::HashMap;
 use std::rc::Rc;
 
 use anyhow::Context;
@@ -358,6 +359,14 @@ impl CallManager for DummyCallManager {
         todo!()
     }
 
+    fn get_actor_call_stack(&self) -> &HashMap<ActorID, i32> {
+        todo!()
+    }
+
+    fn get_actor_call_stack_mut(&mut self) -> &mut HashMap<ActorID, i32> {
+        todo!()
+    }
+
     fn invocation_count(&self) -> u64 {
         todo!()
     }

testing/conformance/src/vm.rs

@@ -302,6 +302,10 @@ where
     fn upgrade_actor<KK>(&mut self, new_code_cid: Cid, params_id: BlockId) -> Result<SendResult> {
         self.0.upgrade_actor::<Self>(new_code_cid, params_id)
     }
+
+    fn is_actor_on_call_stack(&self) -> bool {
+        self.0.is_actor_on_call_stack()
+    }
 }
 
 impl<M, C, K> IpldBlockOps for TestKernel<K>