diff --git a/vm/actor/src/builtin/codes.rs b/vm/actor/src/builtin/codes.rs index 44bb1c98a1d3..4cdcba8f8367 100644 --- a/vm/actor/src/builtin/codes.rs +++ b/vm/actor/src/builtin/codes.rs @@ -26,6 +26,32 @@ fn make_builtin(bz: &[u8]) -> Cid { Cid::new_v1(Codec::Raw, Identity::digest(bz)) } +/// Returns true if the code `Cid` belongs to a builtin actor. +pub fn is_builtin_actor(code: &Cid) -> bool { + code == &*SYSTEM_ACTOR_CODE_ID + || code == &*INIT_ACTOR_CODE_ID + || code == &*CRON_ACTOR_CODE_ID + || code == &*ACCOUNT_ACTOR_CODE_ID + || code == &*POWER_ACTOR_CODE_ID + || code == &*MINER_ACTOR_CODE_ID + || code == &*MARKET_ACTOR_CODE_ID + || code == &*PAYCH_ACTOR_CODE_ID + || code == &*MULTISIG_ACTOR_CODE_ID + || code == &*REWARD_ACTOR_CODE_ID + || code == &*VERIFREG_ACTOR_CODE_ID +} + +/// Returns true if the code belongs to a singleton actor. +pub fn is_singleton_actor(code: &Cid) -> bool { + code == &*SYSTEM_ACTOR_CODE_ID + || code == &*INIT_ACTOR_CODE_ID + || code == &*REWARD_ACTOR_CODE_ID + || code == &*CRON_ACTOR_CODE_ID + || code == &*POWER_ACTOR_CODE_ID + || code == &*MARKET_ACTOR_CODE_ID + || code == &*VERIFREG_ACTOR_CODE_ID +} + // Tests whether a code CID represents an actor that can be an external principal: i.e. an account or multisig. // We could do something more sophisticated here: https://github.com/filecoin-project/specs-actors/issues/178 pub fn is_principal(code: &Cid) -> bool { diff --git a/vm/actor/src/builtin/init/mod.rs b/vm/actor/src/builtin/init/mod.rs index 35a3078a88d7..6d41e64154d0 100644 --- a/vm/actor/src/builtin/init/mod.rs +++ b/vm/actor/src/builtin/init/mod.rs @@ -80,7 +80,7 @@ impl Actor { })??; // Create an empty actor - rt.create_actor(¶ms.code_cid, &id_address)?; + rt.create_actor(params.code_cid, &id_address)?; // Invoke constructor rt.send( diff --git a/vm/actor/src/builtin/miner/mod.rs b/vm/actor/src/builtin/miner/mod.rs index acbbc78dee88..239616529f10 100644 --- a/vm/actor/src/builtin/miner/mod.rs +++ b/vm/actor/src/builtin/miner/mod.rs @@ -2562,10 +2562,7 @@ impl ActorCode for Actor { Self::change_multi_address(rt, params.deserialize()?)?; Ok(Serialized::default()) } - None => { - // Method number does not match available, abort in runtime - Err(rt.abort(ExitCode::SysErrInvalidMethod, "Invalid method".to_owned())) - } + None => Err(actor_error!(SysErrInvalidMethod; "Invalid method")), } } } diff --git a/vm/actor/tests/common/mod.rs b/vm/actor/tests/common/mod.rs index e974b36f590d..8353d8fe7939 100644 --- a/vm/actor/tests/common/mod.rs +++ b/vm/actor/tests/common/mod.rs @@ -550,7 +550,7 @@ impl Runtime for MockRuntime { F: FnOnce(&mut C, &mut Self) -> R, { if self.in_transaction { - return Err(self.abort(ExitCode::SysErrorIllegalActor, "nested transaction")); + return Err(actor_error!(SysErrorIllegalActor; "nested transaction")); } let mut read_only = self.state()?; self.in_transaction = true; @@ -605,10 +605,6 @@ impl Runtime for MockRuntime { } } - fn abort>(&self, exit_code: ExitCode, msg: S) -> ActorError { - ActorError::new(exit_code, msg.as_ref().to_owned()) - } - fn new_actor_address(&mut self) -> Result { self.require_in_call(); let ret = self @@ -620,7 +616,7 @@ impl Runtime for MockRuntime { return Ok(ret); } - fn create_actor(&mut self, code_id: &Cid, address: &Address) -> Result<(), ActorError> { + fn create_actor(&mut self, code_id: Cid, address: &Address) -> Result<(), ActorError> { self.require_in_call(); if self.in_transaction { return Err(actor_error!(SysErrorIllegalActor; "side-effect within transaction")); @@ -630,7 +626,7 @@ impl Runtime for MockRuntime { .take() .expect("unexpected call to create actor"); - assert!(&expect_create_actor.code_id == code_id && &expect_create_actor.address == address, "unexpected actor being created, expected code: {:?} address: {:?}, actual code: {:?} address: {:?}", expect_create_actor.code_id, expect_create_actor.address, code_id, address); + assert!(&expect_create_actor.code_id == &code_id && &expect_create_actor.address == address, "unexpected actor being created, expected code: {:?} address: {:?}, actual code: {:?} address: {:?}", expect_create_actor.code_id, expect_create_actor.address, code_id, address); Ok(()) } diff --git a/vm/interpreter/src/default_runtime.rs b/vm/interpreter/src/default_runtime.rs index 8cc95701c1db..23f5b635e820 100644 --- a/vm/interpreter/src/default_runtime.rs +++ b/vm/interpreter/src/default_runtime.rs @@ -482,53 +482,54 @@ where Ok(ret) } - - fn abort>(&self, exit_code: ExitCode, msg: S) -> ActorError { - ActorError::new(exit_code, msg.as_ref().to_owned()) - } fn new_actor_address(&mut self) -> Result { let oa = resolve_to_key_addr(self.state, &self.store, &self.origin)?; let mut b = to_vec(&oa).map_err(|e| { - self.abort( - ExitCode::ErrSerialization, - format!("Could not serialize address in new_actor_address: {}", e), - ) - })?; - b.write_u64::(self.origin_nonce).map_err(|e| { - self.abort( - ExitCode::ErrSerialization, - format!("Writing nonce address into a buffer: {}", e.to_string()), - ) + actor_error!(fatal( + "Could not serialize address in new_actor_address: {}", + e + )) })?; + b.write_u64::(self.origin_nonce) + .map_err(|e| actor_error!(fatal("Writing nonce address into a buffer: {}", e)))?; b.write_u64::(self.num_actors_created) .map_err(|e| { - self.abort( - ExitCode::ErrSerialization, - format!( - "Writing number of actors created into a buffer: {}", - e.to_string() - ), - ) + actor_error!(fatal( + "Writing number of actors created into a buffer: {}", + e + )) })?; let addr = Address::new_actor(&b); self.num_actors_created += 1; Ok(addr) } - fn create_actor(&mut self, code_id: &Cid, address: &Address) -> Result<(), ActorError> { + fn create_actor(&mut self, code_id: Cid, address: &Address) -> Result<(), ActorError> { + if !is_builtin_actor(&code_id) { + return Err(actor_error!(SysErrorIllegalArgument; "Can only create built-in actors.")); + } + if is_singleton_actor(&code_id) { + return Err(actor_error!(SysErrorIllegalArgument; + "Can only have one instance of singleton actors.")); + } + + if let Ok(Some(_)) = self.state.get_actor(address) { + return Err(actor_error!(SysErrorIllegalArgument; "Actor address already exists")); + } + self.charge_gas(self.price_list.on_create_actor())?; self.state .set_actor( &address, - ActorState::new(code_id.clone(), Cid::default(), 0u64.into(), 0), + ActorState::new(code_id, EMPTY_ARR_CID.clone(), 0.into(), 0), ) - .map_err(|e| { - self.abort( - ExitCode::SysErrInternal, - format!("creating actor entry: {}", e), - ) - }) + .map_err(|e| actor_error!(fatal("creating actor entry: {}", e))) } - fn delete_actor(&mut self, _beneficiary: &Address) -> Result<(), ActorError> { + + /// DeleteActor deletes the executing actor from the state tree, transferring + /// any balance to beneficiary. + /// Aborts if the beneficiary does not exist. + /// 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 balance = self .state @@ -538,18 +539,21 @@ where || actor_error!(SysErrorIllegalActor; "failed to load actor in delete actor"), ) .map(|act| act.balance)?; - if !balance.eq(&0u64.into()) { - return Err(self.abort( - ExitCode::SysErrInternal, - "cannot delete actor with non-zero 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| { + actor_error!(fatal( + "failed to transfer balance to beneficiary actor: {}", + e.msg() + )) + })?; } - self.state.delete_actor(self.message.to()).map_err(|e| { - self.abort( - ExitCode::SysErrInternal, - format!("failed to delete actor: {}", e), - ) - }) + + // Delete the executing actor + self.state + .delete_actor(&receiver) + .map_err(|e| actor_error!(fatal("failed to delete actor: {}", e))) } fn syscalls(&self) -> &dyn Syscalls { &self.syscalls diff --git a/vm/runtime/src/lib.rs b/vm/runtime/src/lib.rs index 87740dbb97fd..af3b43c27b67 100644 --- a/vm/runtime/src/lib.rs +++ b/vm/runtime/src/lib.rs @@ -117,12 +117,6 @@ pub trait Runtime { value: TokenAmount, ) -> Result; - /// Halts execution upon an error from which the receiver cannot recover. - /// The caller will receive the exitcode and an empty return value. - /// State changes made within this call will be rolled back. This method does not return. - /// The message and args are for diagnostic purposes and do not persist on chain. - fn abort>(&self, exit_code: ExitCode, msg: S) -> ActorError; - /// Computes an address for a new actor. The returned address is intended to uniquely refer to /// the actor even in the event of a chain re-org (whereas an ID-address might refer to a /// different actor after messages are re-ordered). @@ -131,7 +125,7 @@ pub trait Runtime { /// Creates an actor with code `codeID` and address `address`, with empty state. /// May only be called by Init actor. - fn create_actor(&mut self, code_id: &Cid, address: &Address) -> Result<(), ActorError>; + fn create_actor(&mut self, code_id: Cid, address: &Address) -> Result<(), ActorError>; /// Deletes the executing actor from the state tree, transferring any balance to beneficiary. /// Aborts if the beneficiary does not exist. diff --git a/vm/src/code.rs b/vm/src/code.rs deleted file mode 100644 index d3d6254eb556..000000000000 --- a/vm/src/code.rs +++ /dev/null @@ -1,89 +0,0 @@ -// Copyright 2020 ChainSafe Systems -// SPDX-License-Identifier: Apache-2.0, MIT - -use cid::Cid; - -/// CodeID is the reference to the code which is attached to the Actor state. -/// There are builtin IDs and the option for custom code with a Cid -#[derive(PartialEq, Eq, Clone, Debug)] -pub enum CodeID { - Init, - Cron, - Account, - Reward, - PaymentChannel, - StoragePower, - StorageMarket, - StorageMiner, - System, - Verifreg, - Multisig, - CustomCode(Cid), -} - -// TODO define builtin Cids - -impl CodeID { - /// Returns true if cid is builtin Actor - pub fn is_builtin(&self) -> bool { - match *self { - CodeID::CustomCode(_) => false, - _ => true, - } - } - /// Returns true if cid is singleton Actor - pub fn is_singleton(&self) -> bool { - match *self { - CodeID::StorageMarket - | CodeID::Init - | CodeID::StoragePower - | CodeID::Cron - | CodeID::Reward - | CodeID::System - | CodeID::Verifreg => true, - _ => false, - } - } -} - -#[cfg(test)] -mod test { - use super::*; - use cid::Cid; - - #[test] - fn builtin_checks() { - // Tests all builtins will return true - assert!(CodeID::Init.is_builtin()); - assert!(CodeID::System.is_builtin()); - assert!(CodeID::StorageMarket.is_builtin()); - assert!(CodeID::StoragePower.is_builtin()); - assert!(CodeID::Cron.is_builtin()); - assert!(CodeID::Account.is_builtin()); - assert!(CodeID::PaymentChannel.is_builtin()); - assert!(CodeID::StorageMiner.is_builtin()); - assert!(CodeID::Verifreg.is_builtin()); - assert!(CodeID::Multisig.is_builtin()); - assert!(CodeID::Reward.is_builtin()); - - assert!(!CodeID::CustomCode(Cid::default()).is_builtin()); - } - - #[test] - fn singleton_checks() { - // singletons - assert!(CodeID::Init.is_singleton()); - assert!(CodeID::StorageMarket.is_singleton()); - assert!(CodeID::StoragePower.is_singleton()); - assert!(CodeID::Verifreg.is_singleton()); - assert!(CodeID::Reward.is_singleton()); - assert!(CodeID::System.is_singleton()); - assert!(CodeID::Cron.is_singleton()); - // non-singletons - assert!(!CodeID::Account.is_singleton()); - assert!(!CodeID::PaymentChannel.is_singleton()); - assert!(!CodeID::StorageMiner.is_singleton()); - assert!(!CodeID::Multisig.is_singleton()); - assert!(!CodeID::CustomCode(Cid::default()).is_singleton()); - } -} diff --git a/vm/src/lib.rs b/vm/src/lib.rs index 9566b38c5bbb..79012a8f4337 100644 --- a/vm/src/lib.rs +++ b/vm/src/lib.rs @@ -5,7 +5,6 @@ extern crate serde; mod actor_state; -mod code; mod deal_id; mod error; mod exit_code; @@ -15,7 +14,6 @@ mod randomness; mod token; pub use self::actor_state::*; -pub use self::code::*; pub use self::deal_id::*; pub use self::error::*; pub use self::exit_code::*;