Skip to content

Commit

Permalink
Reject upgrades if actor already on call stack
Browse files Browse the repository at this point in the history
  • Loading branch information
fridrik01 committed Oct 18, 2023
1 parent 53aa0d5 commit 1913868
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 0 deletions.
12 changes: 12 additions & 0 deletions fvm/src/call_manager/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -159,6 +162,7 @@ where
limits,
events: Default::default(),
state_access_tracker,
actor_call_stack: HashMap::new(),
})))
}

Expand Down Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions fvm/src/call_manager/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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>>;

Expand Down
14 changes: 14 additions & 0 deletions fvm/src/kernel/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>(
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions fvm/src/kernel/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<()>;
Expand Down
8 changes: 8 additions & 0 deletions fvm/src/syscalls/actor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 9 additions & 0 deletions fvm/tests/dummy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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!()
}
Expand Down
4 changes: 4 additions & 0 deletions testing/conformance/src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down

0 comments on commit 1913868

Please sign in to comment.