From 4dfc78a4d41a01d1e6597952eddbd79bcdccbafa Mon Sep 17 00:00:00 2001 From: austinabell Date: Fri, 17 Jul 2020 10:23:43 -0400 Subject: [PATCH 01/23] Setup network config params --- blockchain/state_manager/src/lib.rs | 4 +++- types/src/lib.rs | 25 +++++++++++++++++++++++++ vm/interpreter/Cargo.toml | 1 + vm/interpreter/src/default_runtime.rs | 19 +++++++++++++------ vm/interpreter/src/vm.rs | 14 +++++++++----- 5 files changed, 51 insertions(+), 12 deletions(-) diff --git a/blockchain/state_manager/src/lib.rs b/blockchain/state_manager/src/lib.rs index 48b65332acab..d8820753e087 100644 --- a/blockchain/state_manager/src/lib.rs +++ b/blockchain/state_manager/src/lib.rs @@ -16,6 +16,7 @@ use blockstore::BufferedBlockStore; use chain::{block_messages, ChainStore}; use cid::Cid; use encoding::de::DeserializeOwned; +use fil_types::DevnetParams; use forest_blocks::{Block, BlockHeader, FullTipset, Tipset, TipsetKeys}; use interpreter::{resolve_to_key_addr, ChainRand, DefaultSyscalls, VM}; use ipld_amt::Amt; @@ -124,7 +125,8 @@ where ) -> Result<(Cid, Cid), Box> { let mut buf_store = BufferedBlockStore::new(self.bs.as_ref()); // TODO possibly switch out syscalls to be saved at state manager level - let mut vm = VM::new( + // TODO change from statically using devnet params when needed + let mut vm = VM::<_, _, DevnetParams>::new( ts.parent_state(), &buf_store, ts.epoch(), diff --git a/types/src/lib.rs b/types/src/lib.rs index 82cc05c98bc9..2253743caf8f 100644 --- a/types/src/lib.rs +++ b/types/src/lib.rs @@ -6,3 +6,28 @@ pub mod sector; pub use self::piece::*; pub use self::sector::*; + +use num_bigint::BigInt; + +/// Config trait which handles different network configurations. +pub trait NetworkParams { + /// Total filecoin available to network. + const TOTAL_FILECOIN: u64; + + /// Available rewards for mining. + const MINING_REWARD_TOTAL: u64; + + /// Initial reward actor balance. This function is only called in genesis setting up state. + fn initial_reward_balance() -> BigInt { + BigInt::from(Self::MINING_REWARD_TOTAL) * Self::TOTAL_FILECOIN + } +} + +// Not yet finalized +pub struct DevnetParams; +impl NetworkParams for DevnetParams { + const TOTAL_FILECOIN: u64 = 2_000_000_000; + const MINING_REWARD_TOTAL: u64 = 1_400_000_000; +} + +pub const FILECOIN_PRECISION: u64 = 1_000_000_000_000_000_000; diff --git a/vm/interpreter/Cargo.toml b/vm/interpreter/Cargo.toml index d2971aef0b75..d10488d61d3c 100644 --- a/vm/interpreter/Cargo.toml +++ b/vm/interpreter/Cargo.toml @@ -24,5 +24,6 @@ log = "0.4.8" db = { path = "../../node/db" } chain = { path = "../../blockchain/chain" } fil_types = { path = "../../types" } + [dev-dependencies] ipld_hamt = { path = "../../ipld/hamt" } \ No newline at end of file diff --git a/vm/interpreter/src/default_runtime.rs b/vm/interpreter/src/default_runtime.rs index 5a3c28b8a456..a4df18faaa54 100644 --- a/vm/interpreter/src/default_runtime.rs +++ b/vm/interpreter/src/default_runtime.rs @@ -15,6 +15,7 @@ use byteorder::{BigEndian, WriteBytesExt}; use cid::{multihash::Blake2b256, Cid}; use clock::ChainEpoch; use crypto::DomainSeparationTag; +use fil_types::NetworkParams; use forest_encoding::to_vec; use forest_encoding::Cbor; use ipld_blockstore::BlockStore; @@ -23,13 +24,14 @@ use num_bigint::BigInt; use runtime::{ActorCode, Runtime, Syscalls}; use state_tree::StateTree; use std::cell::RefCell; +use std::marker::PhantomData; use std::rc::Rc; use vm::{ ActorError, ActorState, ExitCode, MethodNum, Randomness, Serialized, TokenAmount, METHOD_SEND, }; /// Implementation of the Runtime trait. -pub struct DefaultRuntime<'db, 'msg, 'st, 'sys, 'r, BS, SYS> { +pub struct DefaultRuntime<'db, 'msg, 'st, 'sys, 'r, BS, SYS, P> { state: &'st mut StateTree<'db, BS>, store: GasBlockStore<'db, BS>, syscalls: GasSyscalls<'sys, SYS>, @@ -41,12 +43,14 @@ pub struct DefaultRuntime<'db, 'msg, 'st, 'sys, 'r, BS, SYS> { num_actors_created: u64, price_list: PriceList, rand: &'r ChainRand, + params: PhantomData

, } -impl<'db, 'msg, 'st, 'sys, 'r, BS, SYS> DefaultRuntime<'db, 'msg, 'st, 'sys, 'r, BS, SYS> +impl<'db, 'msg, 'st, 'sys, 'r, BS, SYS, P> DefaultRuntime<'db, 'msg, 'st, 'sys, 'r, BS, SYS, P> where BS: BlockStore, SYS: Syscalls, + P: NetworkParams, { /// Constructs a new Runtime #[allow(clippy::too_many_arguments)] @@ -89,6 +93,7 @@ where num_actors_created, price_list, rand, + params: PhantomData, } } @@ -156,10 +161,11 @@ where } } -impl Runtime for DefaultRuntime<'_, '_, '_, '_, '_, BS, SYS> +impl Runtime for DefaultRuntime<'_, '_, '_, '_, '_, BS, SYS, P> where BS: BlockStore, SYS: Syscalls, + P: NetworkParams, { fn message(&self) -> &UnsignedMessage { &self.message @@ -337,7 +343,7 @@ where self.num_actors_created, self.rand, ); - internal_send::(&mut parent, &msg, 0) + internal_send::(&mut parent, &msg, 0) }; if send_res.is_err() { self.state @@ -417,14 +423,15 @@ where } /// Shared logic between the DefaultRuntime and the Interpreter. /// It invokes methods on different Actors based on the Message. -pub fn internal_send( - runtime: &mut DefaultRuntime<'_, '_, '_, '_, '_, BS, SYS>, +pub fn internal_send( + runtime: &mut DefaultRuntime<'_, '_, '_, '_, '_, BS, SYS, P>, msg: &UnsignedMessage, _gas_cost: i64, ) -> Result where BS: BlockStore, SYS: Syscalls, + P: NetworkParams, { runtime.charge_gas( runtime diff --git a/vm/interpreter/src/vm.rs b/vm/interpreter/src/vm.rs index 62c6b45d0045..6e0986521fcb 100644 --- a/vm/interpreter/src/vm.rs +++ b/vm/interpreter/src/vm.rs @@ -8,6 +8,7 @@ use actor::{ use blocks::FullTipset; use cid::Cid; use clock::ChainEpoch; +use fil_types::NetworkParams; use forest_encoding::Cbor; use ipld_blockstore::BlockStore; use log::warn; @@ -18,24 +19,26 @@ use runtime::Syscalls; use state_tree::StateTree; use std::collections::HashSet; use std::error::Error as StdError; +use std::marker::PhantomData; use vm::{ActorError, ExitCode, Serialized}; /// Interpreter which handles execution of state transitioning messages and returns receipts /// from the vm execution. -pub struct VM<'db, 'r, DB, SYS> { +pub struct VM<'db, 'r, DB, SYS, P> { state: StateTree<'db, DB>, // TODO revisit handling buffered store specifically in VM store: &'db DB, epoch: ChainEpoch, syscalls: SYS, rand: &'r ChainRand, - // TODO: missing fields + params: PhantomData

, } -impl<'db, 'r, DB, SYS> VM<'db, 'r, DB, SYS> +impl<'db, 'r, DB, SYS, P> VM<'db, 'r, DB, SYS, P> where DB: BlockStore, SYS: Syscalls, + P: NetworkParams, { pub fn new( root: &Cid, @@ -51,6 +54,7 @@ where epoch, syscalls, rand, + params: PhantomData, }) } @@ -126,8 +130,8 @@ where .value(BigInt::zero()) .gas_price(BigInt::zero()) .gas_limit(1 << 30) - .method_num(reward::Method::AwardBlockReward as u64) .params(params) + .method_num(reward::Method::AwardBlockReward as u64) .build()?; // TODO revisit this ApplyRet structure, doesn't match go logic 1:1 and can be cleaner @@ -350,7 +354,7 @@ where gas_cost: i64, ) -> ( Serialized, - DefaultRuntime<'db, 'm, '_, '_, '_, DB, SYS>, + DefaultRuntime<'db, 'm, '_, '_, '_, DB, SYS, P>, Option, ) { let mut rt = DefaultRuntime::new( From 2ea5e6ac4226315aad9517e17ab5043a45a92081 Mon Sep 17 00:00:00 2001 From: austinabell Date: Fri, 17 Jul 2020 10:52:59 -0400 Subject: [PATCH 02/23] Implement total circ supply for default runtime --- types/src/lib.rs | 16 +++++--- vm/interpreter/src/default_runtime.rs | 54 ++++++++++++++++++++++++--- 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/types/src/lib.rs b/types/src/lib.rs index 2253743caf8f..3c9ef91204cc 100644 --- a/types/src/lib.rs +++ b/types/src/lib.rs @@ -12,22 +12,28 @@ use num_bigint::BigInt; /// Config trait which handles different network configurations. pub trait NetworkParams { /// Total filecoin available to network. - const TOTAL_FILECOIN: u64; + const TOTAL_FILECOIN: i64; /// Available rewards for mining. - const MINING_REWARD_TOTAL: u64; + const MINING_REWARD_TOTAL: i64; /// Initial reward actor balance. This function is only called in genesis setting up state. fn initial_reward_balance() -> BigInt { BigInt::from(Self::MINING_REWARD_TOTAL) * Self::TOTAL_FILECOIN } + + /// Convert integer value of tokens into BigInt based on the token precision. + fn from_fil(i: i64) -> BigInt { + BigInt::from(i) * FILECOIN_PRECISION + } } // Not yet finalized pub struct DevnetParams; impl NetworkParams for DevnetParams { - const TOTAL_FILECOIN: u64 = 2_000_000_000; - const MINING_REWARD_TOTAL: u64 = 1_400_000_000; + const TOTAL_FILECOIN: i64 = 2_000_000_000; + const MINING_REWARD_TOTAL: i64 = 1_400_000_000; } -pub const FILECOIN_PRECISION: u64 = 1_000_000_000_000_000_000; +/// Ratio of integer values to token value. +pub const FILECOIN_PRECISION: i64 = 1_000_000_000_000_000_000; diff --git a/vm/interpreter/src/default_runtime.rs b/vm/interpreter/src/default_runtime.rs index a4df18faaa54..147064ad51c8 100644 --- a/vm/interpreter/src/default_runtime.rs +++ b/vm/interpreter/src/default_runtime.rs @@ -5,11 +5,7 @@ use super::gas_block_store::GasBlockStore; use super::gas_syscalls::GasSyscalls; use super::gas_tracker::{price_list_by_epoch, GasTracker, PriceList}; use super::ChainRand; -use actor::{ - self, account, ACCOUNT_ACTOR_CODE_ID, CRON_ACTOR_CODE_ID, INIT_ACTOR_CODE_ID, - MARKET_ACTOR_CODE_ID, MINER_ACTOR_CODE_ID, MULTISIG_ACTOR_CODE_ID, PAYCH_ACTOR_CODE_ID, - POWER_ACTOR_CODE_ID, REWARD_ACTOR_CODE_ID, SYSTEM_ACTOR_CODE_ID, VERIFIED_ACTOR_CODE_ID, -}; +use actor::*; use address::{Address, Protocol}; use byteorder::{BigEndian, WriteBytesExt}; use cid::{multihash::Blake2b256, Cid}; @@ -418,7 +414,53 @@ where &self.syscalls } fn total_fil_circ_supply(&self) -> Result { - todo!() + let get_actor_state = |addr: &Address| -> Result { + self.state + .get_actor(&addr) + .map_err(|e| { + ActorError::new( + ExitCode::ErrIllegalState, + format!( + "failed to get reward actor for cumputing total supply: {}", + e + ), + ) + })? + .ok_or_else(|| { + ActorError::new( + ExitCode::ErrIllegalState, + format!("Actor address ({}) does not exist", addr), + ) + }) + }; + + let rew = get_actor_state(&REWARD_ACTOR_ADDR)?; + let burnt = get_actor_state(&BURNT_FUNDS_ACTOR_ADDR)?; + let market = get_actor_state(&STORAGE_MARKET_ACTOR_ADDR)?; + let power = get_actor_state(&STORAGE_POWER_ACTOR_ADDR)?; + + let st: power::State = self + .store + .get(&power.state) + .map_err(|e| { + ActorError::new( + ExitCode::ErrIllegalState, + format!("failed to get storage power state: {}", e.to_string()), + ) + })? + .ok_or_else(|| { + ActorError::new( + ExitCode::ErrIllegalState, + "Failed to retrieve power state".to_owned(), + ) + })?; + + let total = P::from_fil(P::TOTAL_FILECOIN) + - rew.balance + - market.balance + - burnt.balance + - st.total_pledge_collateral; + Ok(total) } } /// Shared logic between the DefaultRuntime and the Interpreter. From f339a6dc4d2b29018d698ab7d0edc471d70e92a6 Mon Sep 17 00:00:00 2001 From: austinabell Date: Fri, 17 Jul 2020 11:39:25 -0400 Subject: [PATCH 03/23] Add actor error convenience macro --- vm/src/error.rs | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/vm/src/error.rs b/vm/src/error.rs index 2ac55c27c146..2e24be415e4d 100644 --- a/vm/src/error.rs +++ b/vm/src/error.rs @@ -7,7 +7,7 @@ use thiserror::Error; use crate::ExitCode; /// The error type that gets returned by actor method calls. -#[derive(Error, Debug)] +#[derive(Error, Debug, PartialEq)] #[error("ActorError(fatal: {fatal}, exit_code: {exit_code:?}, msg: {msg})")] pub struct ActorError { /// Is this a fatal error. @@ -57,3 +57,32 @@ impl From for ActorError { } } } + +/// Convenience macro for generating Actor Errors +#[macro_export] +macro_rules! actor_error { + // Error with only one stringable expression + ( $code:ident; $msg:expr ) => { ActorError::new(ExitCode::$code, $msg.to_string()) }; + + // String with positional arguments + ( $code:ident; $msg:literal $(, $ex:expr)+ ) => { + ActorError::new(ExitCode::$code, format!($msg, $($ex,)*)) + }; +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn serialize_symmetric() { + assert_eq!( + actor_error!(SysErrSenderInvalid; "test"), + ActorError::new(ExitCode::SysErrSenderInvalid, "test".to_owned()) + ); + assert_eq!( + actor_error!(SysErrSenderInvalid; "test {}, {}", 8, 10), + ActorError::new(ExitCode::SysErrSenderInvalid, format!("test {}, {}", 8, 10)) + ); + } +} From 3b176c362d81098be18704e8d6c1fba3f6b78210 Mon Sep 17 00:00:00 2001 From: austinabell Date: Fri, 17 Jul 2020 11:41:40 -0400 Subject: [PATCH 04/23] Switch actor error usage to macro in default runtime --- vm/interpreter/src/default_runtime.rs | 52 +++++++++------------------ 1 file changed, 16 insertions(+), 36 deletions(-) diff --git a/vm/interpreter/src/default_runtime.rs b/vm/interpreter/src/default_runtime.rs index 147064ad51c8..a43eefae5a4f 100644 --- a/vm/interpreter/src/default_runtime.rs +++ b/vm/interpreter/src/default_runtime.rs @@ -23,7 +23,8 @@ use std::cell::RefCell; use std::marker::PhantomData; use std::rc::Rc; use vm::{ - ActorError, ActorState, ExitCode, MethodNum, Randomness, Serialized, TokenAmount, METHOD_SEND, + actor_error, ActorError, ActorState, ExitCode, MethodNum, Randomness, Serialized, TokenAmount, + METHOD_SEND, }; /// Implementation of the Runtime trait. @@ -418,20 +419,12 @@ where self.state .get_actor(&addr) .map_err(|e| { - ActorError::new( - ExitCode::ErrIllegalState, - format!( - "failed to get reward actor for cumputing total supply: {}", - e - ), - ) + actor_error!(ErrIllegalState; + "failed to get reward actor for cumputing total supply: {}", e) })? - .ok_or_else(|| { - ActorError::new( - ExitCode::ErrIllegalState, - format!("Actor address ({}) does not exist", addr), - ) - }) + .ok_or_else( + || actor_error!(ErrIllegalState; "Actor address ({}) does not exist", addr), + ) }; let rew = get_actor_state(&REWARD_ACTOR_ADDR)?; @@ -443,17 +436,10 @@ where .store .get(&power.state) .map_err(|e| { - ActorError::new( - ExitCode::ErrIllegalState, - format!("failed to get storage power state: {}", e.to_string()), - ) + actor_error!(ErrIllegalState; + "failed to get storage power state: {}", e.to_string()) })? - .ok_or_else(|| { - ActorError::new( - ExitCode::ErrIllegalState, - "Failed to retrieve power state".to_owned(), - ) - })?; + .ok_or_else(|| actor_error!(ErrIllegalState; "Failed to retrieve power state"))?; let total = P::from_fil(P::TOTAL_FILECOIN) - rew.balance @@ -486,7 +472,7 @@ where if msg.value() != &0u8.into() { transfer(runtime.state, &msg.from(), &msg.to(), &msg.value()) - .map_err(|e| ActorError::new(ExitCode::SysErrSenderInvalid, e))?; + .map_err(|e| actor_error!(SysErrSenderInvalid; e))?; } let method_num = msg.method_num(); @@ -528,10 +514,9 @@ where x if x == *VERIFIED_ACTOR_CODE_ID => { actor::verifreg::Actor.invoke_method(runtime, method_num, msg.params()) } - _ => Err(ActorError::new( - ExitCode::SysErrorIllegalActor, - format!("no code for actor at address {}", msg.to()), - )), + _ => Err( + actor_error!(SysErrorIllegalActor; "no code for actor at address {}", msg.to()) + ), } }; return ret; @@ -585,13 +570,8 @@ where let act = st .get_actor(&addr) - .map_err(|e| ActorError::new(ExitCode::SysErrInternal, e))? - .ok_or_else(|| { - ActorError::new( - ExitCode::SysErrInternal, - format!("Failed to retrieve actor: {}", addr), - ) - })?; + .map_err(|e| actor_error!(SysErrInternal; e))? + .ok_or_else(|| actor_error!(SysErrInternal; "Failed to retrieve actor: {}", addr))?; if act.code != *ACCOUNT_ACTOR_CODE_ID { return Err(ActorError::new_fatal(format!( From 0d35a7d702630d6b1b455dcde4d1fd49797822d0 Mon Sep 17 00:00:00 2001 From: austinabell Date: Fri, 17 Jul 2020 16:49:17 -0400 Subject: [PATCH 05/23] updates to runtime functions --- vm/actor/src/builtin/init/mod.rs | 11 ++-- vm/actor/src/builtin/market/mod.rs | 21 +++---- vm/actor/src/builtin/miner/mod.rs | 61 ++++++------------- vm/actor/src/builtin/paych/mod.rs | 29 +++++---- vm/actor/tests/common/mod.rs | 10 +-- vm/interpreter/src/default_runtime.rs | 87 +++++++++++++++------------ vm/runtime/src/lib.rs | 2 +- vm/src/error.rs | 6 ++ 8 files changed, 108 insertions(+), 119 deletions(-) diff --git a/vm/actor/src/builtin/init/mod.rs b/vm/actor/src/builtin/init/mod.rs index e2a47c961e47..fe9169233e7c 100644 --- a/vm/actor/src/builtin/init/mod.rs +++ b/vm/actor/src/builtin/init/mod.rs @@ -17,7 +17,7 @@ use message::Message; use num_derive::FromPrimitive; use num_traits::FromPrimitive; use runtime::{ActorCode, Runtime}; -use vm::{ActorError, ExitCode, MethodNum, Serialized, METHOD_CONSTRUCTOR}; +use vm::{actor_error, ActorError, ExitCode, MethodNum, Serialized, METHOD_CONSTRUCTOR}; /// Init actor methods available #[derive(FromPrimitive)] @@ -59,15 +59,12 @@ impl Actor { { rt.validate_immediate_caller_accept_any(); let caller_code = rt - .get_actor_code_cid(rt.message().from()) - .expect("no code for actor"); + .get_actor_code_cid(rt.message().from())? + .ok_or_else(|| actor_error!(ErrForbidden; "No code for actor"))?; if !can_exec(&caller_code, ¶ms.code_cid) { - return Err(rt.abort( - ExitCode::ErrForbidden, - format!( + return Err(actor_error!(ErrForbidden; "called type {} cannot exec actor type {}", &caller_code, ¶ms.code_cid - ), )); } diff --git a/vm/actor/src/builtin/market/mod.rs b/vm/actor/src/builtin/market/mod.rs index d65924fd7fdc..c0fe10c1f7b8 100644 --- a/vm/actor/src/builtin/market/mod.rs +++ b/vm/actor/src/builtin/market/mod.rs @@ -29,7 +29,8 @@ use num_derive::FromPrimitive; use num_traits::{FromPrimitive, Zero}; use runtime::{ActorCode, Runtime}; use vm::{ - ActorError, ExitCode, MethodNum, Serialized, TokenAmount, METHOD_CONSTRUCTOR, METHOD_SEND, + actor_error, ActorError, ExitCode, MethodNum, Serialized, TokenAmount, METHOD_CONSTRUCTOR, + METHOD_SEND, }; /// Market actor methods available @@ -769,19 +770,13 @@ where RT: Runtime, { // Resolve the provided address to the canonical form against which the balance is held. - let nominal = rt.resolve_address(addr).map_err(|e| { - ActorError::new( - ExitCode::ErrIllegalArgument, - format!("Failed to resolve address provided: {}", e), - ) - })?; + let nominal = rt.resolve_address(addr).map_err( + |e| actor_error!(ErrIllegalArgument; "Failed to resolve address provided: {}", e), + )?; - let code_id = rt.get_actor_code_cid(&nominal).map_err(|e| { - ActorError::new( - ExitCode::ErrIllegalArgument, - format!("Failed to retrieve actor code cid: {}", e), - ) - })?; + let code_id = rt + .get_actor_code_cid(&nominal)? + .ok_or_else(|| actor_error!(ErrIllegalArgument; "no code for address {}", nominal))?; if code_id != *MINER_ACTOR_CODE_ID { // Ordinary account-style actor entry; funds recipient is just the entry address itself. diff --git a/vm/actor/src/builtin/miner/mod.rs b/vm/actor/src/builtin/miner/mod.rs index 28e802c6f5ec..b06456f642ba 100644 --- a/vm/actor/src/builtin/miner/mod.rs +++ b/vm/actor/src/builtin/miner/mod.rs @@ -55,8 +55,8 @@ use std::collections::HashMap; use std::error::Error as StdError; use std::ops::Neg; use vm::{ - ActorError, DealID, ExitCode, MethodNum, Serialized, TokenAmount, METHOD_CONSTRUCTOR, - METHOD_SEND, + actor_error, ActorError, DealID, ExitCode, MethodNum, Serialized, TokenAmount, + METHOD_CONSTRUCTOR, METHOD_SEND, }; // * Updated to specs-actors commit: 9e8c0d1c40d8b41de5dc727b6791c89e14fea4a8 @@ -2266,24 +2266,17 @@ where BS: BlockStore, RT: Runtime, { - let resolved = rt.resolve_address(&raw).map_err(|_| { - ActorError::new( - ExitCode::ErrIllegalArgument, - format!("unable to resolve address {}", raw), - ) - })?; + let resolved = rt + .resolve_address(&raw) + .map_err(|_| actor_error!(ErrIllegalArgument; "unable to resolve address {}", raw))?; assert!(resolved.protocol() == Protocol::ID); - let owner_code = rt.get_actor_code_cid(&resolved).map_err(|_| { - ActorError::new( - ExitCode::ErrIllegalArgument, - format!("no code for address: {}", resolved), - ) - })?; + let owner_code = rt + .get_actor_code_cid(&resolved)? + .ok_or_else(|| actor_error!(ErrIllegalArgument; "no code for address: {}", resolved))?; if !is_principal(&owner_code) { - return Err(ActorError::new( - ExitCode::ErrIllegalArgument, - format!("owner actor type must be a principal, was {}", owner_code), + return Err(actor_error!(ErrIllegalArgument; + "owner actor type must be a principal, was {}", owner_code )); } @@ -2297,25 +2290,17 @@ where BS: BlockStore, RT: Runtime, { - let resolved = rt.resolve_address(&raw).map_err(|e| { - ActorError::new( - ExitCode::ErrIllegalArgument, - format!("unable to resolve address: {},{}", raw, e), - ) - })?; + let resolved = rt.resolve_address(&raw).map_err( + |e| actor_error!(ErrIllegalArgument; "unable to resolve address: {},{}", raw, e), + )?; assert!(resolved.protocol() == Protocol::ID); - let owner_code = rt.get_actor_code_cid(&resolved).map_err(|e| { - ActorError::new( - ExitCode::ErrIllegalArgument, - format!("no code for address: {}, {}", resolved, e), - ) - })?; + let owner_code = rt + .get_actor_code_cid(&resolved)? + .ok_or_else(|| actor_error!(ErrIllegalArgument; "no code for address: {}", resolved))?; if owner_code != *ACCOUNT_ACTOR_CODE_ID { - return Err(ActorError::new( - ExitCode::ErrIllegalArgument, - format!("worker actor type must be an account, was {}", owner_code), - )); + return Err(actor_error!(ErrIllegalArgument; + "worker actor type must be an account, was {}", owner_code)); } if raw.protocol() != Protocol::BLS { @@ -2326,19 +2311,13 @@ where &TokenAmount::zero(), )?; let pub_key: Address = ret.deserialize().map_err(|e| { - ActorError::new( - ExitCode::ErrSerialization, - format!("failed to deserialize address result: {:?}, {}", ret, e), - ) + actor_error!(ErrSerialization; "failed to deserialize address result: {:?}, {}", ret, e) })?; if pub_key.protocol() != Protocol::BLS { - return Err(ActorError::new( - ExitCode::ErrIllegalArgument, - format!( + return Err(actor_error!(ErrIllegalArgument; "worker account {} must have BLS pubkey, was {}", resolved, pub_key.protocol() - ), )); } } diff --git a/vm/actor/src/builtin/paych/mod.rs b/vm/actor/src/builtin/paych/mod.rs index c26e351b023a..3d1d43c241e0 100644 --- a/vm/actor/src/builtin/paych/mod.rs +++ b/vm/actor/src/builtin/paych/mod.rs @@ -16,7 +16,8 @@ use num_derive::FromPrimitive; use num_traits::{FromPrimitive, Zero}; use runtime::{ActorCode, Runtime}; use vm::{ - ActorError, ExitCode, MethodNum, Serialized, TokenAmount, METHOD_CONSTRUCTOR, METHOD_SEND, + actor_error, ActorError, ExitCode, MethodNum, Serialized, TokenAmount, METHOD_CONSTRUCTOR, + METHOD_SEND, }; /// Payment Channel actor methods available @@ -43,9 +44,11 @@ impl Actor { rt.validate_immediate_caller_type(std::iter::once(&*INIT_ACTOR_CODE_ID))?; // Check both parties are capable of signing vouchers - let to = Self::resolve_account(rt, ¶ms.to)?; + let to = Self::resolve_account(rt, ¶ms.to) + .map_err(|e| actor_error!(ErrIllegalArgument; e))?; - let from = Self::resolve_account(rt, ¶ms.from)?; + let from = Self::resolve_account(rt, ¶ms.from) + .map_err(|e| actor_error!(ErrIllegalArgument; e))?; rt.create(&State::new(from, to))?; Ok(()) @@ -54,22 +57,24 @@ impl Actor { /// Resolves an address to a canonical ID address and requires it to address an account actor. /// The account actor constructor checks that the embedded address is associated with an appropriate key. /// An alternative (more expensive) would be to send a message to the actor to fetch its key. - fn resolve_account(rt: &RT, raw: &Address) -> Result + fn resolve_account(rt: &RT, raw: &Address) -> Result where BS: BlockStore, RT: Runtime, { - let resolved = rt.resolve_address(raw)?; + let resolved = rt + .resolve_address(raw) + .map_err(|e| format!("failed to resolve address {}", e))?; - let code_cid = rt.get_actor_code_cid(&resolved)?; + let code_cid = rt + .get_actor_code_cid(&resolved) + .expect("Failed to get code Cid") + .ok_or_else(|| format!("no code for address {}", raw))?; if code_cid != *ACCOUNT_ACTOR_CODE_ID { - Err(ActorError::new( - ExitCode::ErrIllegalArgument, - format!( - "actor {} must be an account ({}), was {}", - raw, &*ACCOUNT_ACTOR_CODE_ID, code_cid - ), + Err(format!( + "actor {} must be an account ({}), was {}", + raw, &*ACCOUNT_ACTOR_CODE_ID, code_cid )) } else { Ok(resolved) diff --git a/vm/actor/tests/common/mod.rs b/vm/actor/tests/common/mod.rs index 6a1ddf25e899..8dd3f34e54f8 100644 --- a/vm/actor/tests/common/mod.rs +++ b/vm/actor/tests/common/mod.rs @@ -514,16 +514,10 @@ where )) } - fn get_actor_code_cid(&self, addr: &Address) -> Result { + fn get_actor_code_cid(&self, addr: &Address) -> Result, ActorError> { self.require_in_call(); - self.actor_code_cids - .get(&addr) - .cloned() - .ok_or(ActorError::new( - ExitCode::ErrIllegalArgument, - "Actor address is not found".to_string(), - )) + Ok(self.actor_code_cids.get(&addr).cloned()) } fn get_randomness( diff --git a/vm/interpreter/src/default_runtime.rs b/vm/interpreter/src/default_runtime.rs index a43eefae5a4f..238e12b831b5 100644 --- a/vm/interpreter/src/default_runtime.rs +++ b/vm/interpreter/src/default_runtime.rs @@ -115,44 +115,34 @@ where self.price_list } - /// Gets the specified Actor from the state tree - fn get_actor(&self, addr: &Address) -> Result { - // TODO handle exit codes specifically, this leads to a broken implementation - self.state - .get_actor(&addr) - .map_err(|e| { - self.abort( - ExitCode::SysErrInternal, - format!("failed to load actor: {}", e), - ) - })? - .ok_or_else(|| self.abort(ExitCode::SysErrInternal, "actor not found")) - } - /// Get the balance of a particular Actor from their Address fn get_balance(&self, addr: &Address) -> Result { - // TODO fix this, not found should return 0 not error, on error should turn error into fatal - self.get_actor(&addr).map(|act| act.balance) + Ok(self + .state + .get_actor(&addr) + .map_err(ActorError::new_fatal)? + .map(|act| act.balance) + .unwrap_or_default()) } /// Update the state Cid of the Message receiver fn state_commit(&mut self, old_h: &Cid, new_h: Cid) -> Result<(), ActorError> { let to_addr = *self.message().to(); - let mut actor = self.get_actor(&to_addr)?; + let mut actor = self + .state + .get_actor(&to_addr) + .map_err(ActorError::new_fatal)? + .ok_or_else(|| actor_error!(fatal("failed to get actor to commit state")))?; if &actor.state != old_h { - return Err(self.abort( - ExitCode::ErrIllegalState, - "failed to update, inconsistent base reference".to_owned(), - )); + return Err(actor_error!(fatal( + "failed to update, inconsistent base reference" + ))); } actor.state = new_h; - self.state.set_actor(&to_addr, actor).map_err(|e| { - self.abort( - ExitCode::SysErrInternal, - format!("failed to set actor in state_commit: {}", e), - ) - })?; + self.state + .set_actor(&to_addr, actor) + .map_err(|e| actor_error!(fatal("failed to set actor in state_commit: {}", e)))?; Ok(()) } @@ -191,7 +181,9 @@ where where I: IntoIterator, { - let caller_cid = self.get_actor_code_cid(self.message().to())?; + let caller_cid = self + .get_actor_code_cid(self.message().to())? + .ok_or_else(|| actor_error!(fatal("failed to lookup code cid for caller")))?; if types.into_iter().any(|c| *c == caller_cid) { return Err(self.abort( ExitCode::SysErrForbidden, @@ -204,17 +196,25 @@ where } Ok(()) } + fn current_balance(&self) -> Result { self.get_balance(self.message.to()) } + fn resolve_address(&self, address: &Address) -> Result { self.state .lookup_id(&address) - .map_err(|e| self.abort(ExitCode::ErrPlaceholder, e)) + .map_err(ActorError::new_fatal) } - fn get_actor_code_cid(&self, addr: &Address) -> Result { - self.get_actor(&addr).map(|act| act.code) + + fn get_actor_code_cid(&self, addr: &Address) -> Result, ActorError> { + Ok(self + .state + .get_actor(&addr) + .map_err(ActorError::new_fatal)? + .map(|act| act.code)) } + fn get_randomness( &self, personalization: DomainSeparationTag, @@ -241,8 +241,9 @@ where // TODO: This is almost certainly wrong. Need to CBOR an empty slice and calculate Cid self.state_commit(&Cid::default(), c) } + fn state(&self) -> Result { - let actor = self.get_actor(self.message().to())?; + let actor = self.state.get_actor(self.message().to()).map_err(|e| actor_error!(SysErrorIllegalArgument; "failed to get actor for Readonly state: {}", e))?.ok_or_else(|| actor_error!(SysErrorIllegalArgument; "Actor readonly state does not exist"))?; self.store .get(&actor.state) .map_err(|e| { @@ -265,7 +266,7 @@ where F: FnOnce(&mut C, &mut Self) -> R, { // get actor - let act = self.get_actor(self.message().to())?; + let act = self.state.get_actor(self.message().to()).map_err(|e| actor_error!(SysErrorIllegalActor; "failed to get actor for transaction: {}", e))?.ok_or_else(|| actor_error!(SysErrorIllegalActor; "actor state for transaction doesn't exist"))?; // get state for actor based on generic C let mut state: C = self @@ -397,7 +398,14 @@ where } fn delete_actor(&mut self, _beneficiary: &Address) -> Result<(), ActorError> { self.charge_gas(self.price_list.on_delete_actor())?; - let balance = self.get_actor(self.message.to()).map(|act| act.balance)?; + let balance = self + .state + .get_actor(self.message.to()) + .map_err(|e| actor_error!(fatal("failed to get actor {}, {}", self.message.to(), e)))? + .ok_or_else( + || 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, @@ -436,7 +444,7 @@ where .store .get(&power.state) .map_err(|e| { - actor_error!(ErrIllegalState; + actor_error!(ErrIllegalState; "failed to get storage power state: {}", e.to_string()) })? .ok_or_else(|| actor_error!(ErrIllegalState; "Failed to retrieve power state"))?; @@ -468,7 +476,12 @@ where )?; // TODO: we need to try to recover here and try to create account actor - let to_actor = runtime.get_actor(msg.to())?; + // TODO: actually fix this and don't leave as unwrap for PR + let to_actor = runtime + .state + .get_actor(msg.to()) + .map_err(ActorError::new_fatal)? + .unwrap(); if msg.value() != &0u8.into() { transfer(runtime.state, &msg.from(), &msg.to(), &msg.value()) @@ -515,7 +528,7 @@ where actor::verifreg::Actor.invoke_method(runtime, method_num, msg.params()) } _ => Err( - actor_error!(SysErrorIllegalActor; "no code for actor at address {}", msg.to()) + actor_error!(SysErrorIllegalActor; "no code for actor at address {}", msg.to()), ), } }; diff --git a/vm/runtime/src/lib.rs b/vm/runtime/src/lib.rs index ba7bc9330375..307080e1e404 100644 --- a/vm/runtime/src/lib.rs +++ b/vm/runtime/src/lib.rs @@ -59,7 +59,7 @@ pub trait Runtime { fn resolve_address(&self, address: &Address) -> Result; /// Look up the code ID at an actor address. - fn get_actor_code_cid(&self, addr: &Address) -> Result; + fn get_actor_code_cid(&self, addr: &Address) -> Result, ActorError>; /// Randomness returns a (pseudo)random byte array drawing from a /// random beacon at a given epoch and incorporating reequisite entropy diff --git a/vm/src/error.rs b/vm/src/error.rs index 2e24be415e4d..653143c3d499 100644 --- a/vm/src/error.rs +++ b/vm/src/error.rs @@ -61,6 +61,12 @@ impl From for ActorError { /// Convenience macro for generating Actor Errors #[macro_export] macro_rules! actor_error { + // Fatal Errors + ( fatal($msg:expr) ) => { ActorError::new_fatal($msg.to_string()) }; + ( fatal($msg:literal $(, $ex:expr)+) ) => { + ActorError::new_fatal(format!($msg, $($ex,)*)) + }; + // Error with only one stringable expression ( $code:ident; $msg:expr ) => { ActorError::new(ExitCode::$code, $msg.to_string()) }; From ddd6d4311a69ac69b4efa1a9fd4b4b8dfc6ee5f0 Mon Sep 17 00:00:00 2001 From: austinabell Date: Mon, 20 Jul 2020 07:30:38 -0400 Subject: [PATCH 06/23] Switch message gas limit to i64 --- vm/interpreter/src/default_runtime.rs | 2 +- vm/interpreter/src/vm.rs | 6 ++++-- vm/interpreter/tests/transfer_test.rs | 3 ++- vm/message/src/lib.rs | 2 +- vm/message/src/message_receipt.rs | 6 +++--- vm/message/src/signed_message.rs | 2 +- vm/message/src/unsigned_message.rs | 6 +++--- 7 files changed, 15 insertions(+), 12 deletions(-) diff --git a/vm/interpreter/src/default_runtime.rs b/vm/interpreter/src/default_runtime.rs index 238e12b831b5..408847263c50 100644 --- a/vm/interpreter/src/default_runtime.rs +++ b/vm/interpreter/src/default_runtime.rs @@ -316,7 +316,7 @@ where .from(*self.message.from()) .method_num(method) .value(value.clone()) - .gas_limit(self.gas_available() as u64) + .gas_limit(self.gas_available()) .params(params.clone()) .build() .unwrap(); diff --git a/vm/interpreter/src/vm.rs b/vm/interpreter/src/vm.rs index 6e0986521fcb..abbdeabfe531 100644 --- a/vm/interpreter/src/vm.rs +++ b/vm/interpreter/src/vm.rs @@ -207,7 +207,7 @@ where let pl = price_list_by_epoch(self.epoch()); let ser_msg = &msg.marshal_cbor().map_err(|e| e.to_string())?; - let msg_gas_cost = pl.on_chain_message(ser_msg.len() as i64) as u64; + let msg_gas_cost = pl.on_chain_message(ser_msg.len() as i64); if msg_gas_cost > msg.gas_limit() { return Ok(ApplyRet::new( @@ -319,7 +319,9 @@ where } warn!("Send actor error: from:{}, to:{}", msg.from(), msg.to()); } - let gas_used = if gas_used < 0 { 0 } else { gas_used as u64 }; + // TODO recheck this + let gas_used = if gas_used < 0 { 0 } else { gas_used }; + // refund unused gas let refund = (msg.gas_limit() - gas_used) * msg.gas_price(); self.state.mutate_actor(msg.from(), |act| { diff --git a/vm/interpreter/tests/transfer_test.rs b/vm/interpreter/tests/transfer_test.rs index a9e203784c91..75f7a9ba91f9 100644 --- a/vm/interpreter/tests/transfer_test.rs +++ b/vm/interpreter/tests/transfer_test.rs @@ -6,6 +6,7 @@ use address::Address; use blocks::TipsetKeys; use cid::multihash::{Blake2b256, Identity}; use db::MemoryDB; +use fil_types::DevnetParams; use interpreter::{internal_send, ChainRand, DefaultRuntime, DefaultSyscalls}; use ipld_blockstore::BlockStore; use ipld_hamt::Hamt; @@ -95,7 +96,7 @@ fn transfer_test() { let default_syscalls = DefaultSyscalls::new(&store); let dummy_rand = ChainRand::new(TipsetKeys::new(vec![])); - let mut runtime = DefaultRuntime::new( + let mut runtime = DefaultRuntime::<_, _, DevnetParams>::new( &mut state, &store, &default_syscalls, diff --git a/vm/message/src/lib.rs b/vm/message/src/lib.rs index dd2d6f4eba1d..7b9020797edb 100644 --- a/vm/message/src/lib.rs +++ b/vm/message/src/lib.rs @@ -31,7 +31,7 @@ pub trait Message { /// gas_price returns gas price for the message fn gas_price(&self) -> &TokenAmount; /// Returns the gas limit for the message - fn gas_limit(&self) -> u64; + fn gas_limit(&self) -> i64; /// Returns the required funds for the message fn required_funds(&self) -> TokenAmount; } diff --git a/vm/message/src/message_receipt.rs b/vm/message/src/message_receipt.rs index 6b35d21ba8e3..ab3db956da4b 100644 --- a/vm/message/src/message_receipt.rs +++ b/vm/message/src/message_receipt.rs @@ -9,7 +9,7 @@ use vm::{ExitCode, Serialized}; pub struct MessageReceipt { pub exit_code: ExitCode, pub return_data: Serialized, - pub gas_used: u64, + pub gas_used: i64, } #[cfg(feature = "json")] @@ -44,7 +44,7 @@ pub mod json { exit_code: u64, #[serde(rename = "Return")] return_data: &'a [u8], - gas_used: u64, + gas_used: i64, } MessageReceiptSer { exit_code: m.exit_code as u64, @@ -64,7 +64,7 @@ pub mod json { exit_code: u64, #[serde(rename = "Return")] return_data: Vec, - gas_used: u64, + gas_used: i64, } let MessageReceiptDe { exit_code, diff --git a/vm/message/src/signed_message.rs b/vm/message/src/signed_message.rs index 2fa9e51b306c..5f103c66cad7 100644 --- a/vm/message/src/signed_message.rs +++ b/vm/message/src/signed_message.rs @@ -77,7 +77,7 @@ impl Message for SignedMessage { fn gas_price(&self) -> &TokenAmount { self.message.gas_price() } - fn gas_limit(&self) -> u64 { + fn gas_limit(&self) -> i64 { self.message.gas_limit() } fn required_funds(&self) -> TokenAmount { diff --git a/vm/message/src/unsigned_message.rs b/vm/message/src/unsigned_message.rs index 9bcf2d5b4f1d..95ccbca26a0c 100644 --- a/vm/message/src/unsigned_message.rs +++ b/vm/message/src/unsigned_message.rs @@ -57,7 +57,7 @@ pub struct UnsignedMessage { #[builder(default)] gas_price: TokenAmount, #[builder(default)] - gas_limit: u64, + gas_limit: i64, } impl UnsignedMessage { @@ -138,7 +138,7 @@ impl Message for UnsignedMessage { fn gas_price(&self) -> &TokenAmount { &self.gas_price } - fn gas_limit(&self) -> u64 { + fn gas_limit(&self) -> i64 { self.gas_limit } fn required_funds(&self) -> TokenAmount { @@ -180,7 +180,7 @@ pub mod json { sequence: u64, value: String, gas_price: String, - gas_limit: u64, + gas_limit: i64, #[serde(rename = "Method")] method_num: u64, params: String, From 609bce09cf6d72728a5835b309ce12aeb7b0bc56 Mon Sep 17 00:00:00 2001 From: austinabell Date: Mon, 20 Jul 2020 07:57:45 -0400 Subject: [PATCH 07/23] Remove unnecessary conversions --- vm/interpreter/src/default_runtime.rs | 2 +- vm/interpreter/src/gas_tracker/price_list.rs | 4 ++-- vm/interpreter/src/vm.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/vm/interpreter/src/default_runtime.rs b/vm/interpreter/src/default_runtime.rs index 408847263c50..23cc2cd8da6a 100644 --- a/vm/interpreter/src/default_runtime.rs +++ b/vm/interpreter/src/default_runtime.rs @@ -65,7 +65,7 @@ where ) -> Self { let price_list = price_list_by_epoch(epoch); let gas_tracker = Rc::new(RefCell::new(GasTracker::new( - message.gas_limit() as i64, + message.gas_limit(), gas_used, ))); let gas_block_store = GasBlockStore { diff --git a/vm/interpreter/src/gas_tracker/price_list.rs b/vm/interpreter/src/gas_tracker/price_list.rs index 501c2fa7ba35..1761a62a1b8f 100644 --- a/vm/interpreter/src/gas_tracker/price_list.rs +++ b/vm/interpreter/src/gas_tracker/price_list.rs @@ -77,8 +77,8 @@ pub struct PriceList { impl PriceList { /// Returns the gas required for storing a message of a given size in the chain. #[inline] - pub fn on_chain_message(&self, msg_size: i64) -> i64 { - self.on_chain_message_base + self.on_chain_message_per_byte * msg_size + pub fn on_chain_message(&self, msg_size: usize) -> i64 { + self.on_chain_message_base + self.on_chain_message_per_byte * msg_size as i64 } /// Returns the gas required for storing the response of a message in the chain. #[inline] diff --git a/vm/interpreter/src/vm.rs b/vm/interpreter/src/vm.rs index abbdeabfe531..2c9222fb0873 100644 --- a/vm/interpreter/src/vm.rs +++ b/vm/interpreter/src/vm.rs @@ -207,7 +207,7 @@ where let pl = price_list_by_epoch(self.epoch()); let ser_msg = &msg.marshal_cbor().map_err(|e| e.to_string())?; - let msg_gas_cost = pl.on_chain_message(ser_msg.len() as i64); + let msg_gas_cost = pl.on_chain_message(ser_msg.len()); if msg_gas_cost > msg.gas_limit() { return Ok(ApplyRet::new( @@ -301,7 +301,7 @@ where // scoped to deal with mutable reference borrowing let (ret_data, gas_used, act_err) = { - let (ret_data, mut rt, act_err) = self.send(msg, msg_gas_cost as i64); + let (ret_data, mut rt, act_err) = self.send(msg, msg_gas_cost); rt.charge_gas(rt.price_list().on_chain_return_value(ret_data.len())) .map_err(|e| e.to_string())?; (ret_data, rt.gas_used(), act_err) From 55ca3900e51e5cc8cf920f4ebbe3e3d37e80a9c5 Mon Sep 17 00:00:00 2001 From: austinabell Date: Mon, 20 Jul 2020 12:07:03 -0400 Subject: [PATCH 08/23] Switch to using MessageInfo trait for Runtime and refactor MockRuntime setup --- vm/actor/src/builtin/init/mod.rs | 5 +- vm/actor/src/builtin/market/mod.rs | 9 +- vm/actor/src/builtin/miner/mod.rs | 13 +- vm/actor/src/builtin/multisig/mod.rs | 19 +-- vm/actor/src/builtin/paych/mod.rs | 3 +- vm/actor/src/builtin/power/mod.rs | 21 ++- vm/actor/src/builtin/verifreg/mod.rs | 4 +- vm/actor/tests/account_actor_test.rs | 13 +- vm/actor/tests/common/mod.rs | 134 +++++++-------- vm/actor/tests/cron_actor_test.rs | 39 ++--- vm/actor/tests/init_actor_test.rs | 61 +++---- vm/actor/tests/market_actor_test.rs | 112 +++++------- vm/actor/tests/paych_actor_test.rs | 237 ++++++++++---------------- vm/actor/tests/reward_actor_test.rs | 26 +-- vm/interpreter/src/default_runtime.rs | 25 ++- vm/runtime/src/lib.rs | 31 +++- 16 files changed, 312 insertions(+), 440 deletions(-) diff --git a/vm/actor/src/builtin/init/mod.rs b/vm/actor/src/builtin/init/mod.rs index fe9169233e7c..c5e4e4510bac 100644 --- a/vm/actor/src/builtin/init/mod.rs +++ b/vm/actor/src/builtin/init/mod.rs @@ -13,7 +13,6 @@ use crate::{ use address::Address; use cid::Cid; use ipld_blockstore::BlockStore; -use message::Message; use num_derive::FromPrimitive; use num_traits::FromPrimitive; use runtime::{ActorCode, Runtime}; @@ -59,7 +58,7 @@ impl Actor { { rt.validate_immediate_caller_accept_any(); let caller_code = rt - .get_actor_code_cid(rt.message().from())? + .get_actor_code_cid(rt.message().caller())? .ok_or_else(|| actor_error!(ErrForbidden; "No code for actor"))?; if !can_exec(&caller_code, ¶ms.code_cid) { return Err(actor_error!(ErrForbidden; @@ -91,7 +90,7 @@ impl Actor { &id_address, METHOD_CONSTRUCTOR, ¶ms.constructor_params, - &rt.message().value().clone(), + &rt.message().value_received().clone(), ) .map_err(|err| rt.abort(err.exit_code(), "constructor failed"))?; diff --git a/vm/actor/src/builtin/market/mod.rs b/vm/actor/src/builtin/market/mod.rs index c0fe10c1f7b8..3a7f4c4c93df 100644 --- a/vm/actor/src/builtin/market/mod.rs +++ b/vm/actor/src/builtin/market/mod.rs @@ -23,7 +23,6 @@ use encoding::to_vec; use fil_types::PieceInfo; use ipld_amt::Amt; use ipld_blockstore::BlockStore; -use message::Message; use num_bigint::{BigInt, BigUint}; use num_derive::FromPrimitive; use num_traits::{FromPrimitive, Zero}; @@ -90,7 +89,7 @@ impl Actor { { let (nominal, _) = escrow_address(rt, &provider_or_client)?; - let msg_value = rt.message().value().clone(); + let msg_value = rt.message().value_received().clone(); rt.transaction::, _>(|st, rt| { st.add_escrow_balance(rt.store(), &nominal, msg_value) .map_err(|e| { @@ -195,7 +194,7 @@ impl Actor { let provider = rt.resolve_address(&provider_raw)?; let (_, worker) = request_miner_control_addrs(rt, &provider)?; - if &worker != rt.message().from() { + if &worker != rt.message().caller() { return Err(ActorError::new( ExitCode::ErrForbidden, format!("Caller is not provider {}", worker), @@ -297,7 +296,7 @@ impl Actor { RT: Runtime, { rt.validate_immediate_caller_type(std::iter::once(&*MINER_ACTOR_CODE_ID))?; - let miner_addr = *rt.message().from(); + let miner_addr = *rt.message().caller(); let mut total_deal_space_time = BigUint::zero(); let mut total_verified_deal_space_time = BigUint::zero(); rt.transaction::, _>(|st, rt| { @@ -387,7 +386,7 @@ impl Actor { RT: Runtime, { rt.validate_immediate_caller_type(std::iter::once(&*MINER_ACTOR_CODE_ID))?; - let miner_addr = *rt.message().from(); + let miner_addr = *rt.message().caller(); rt.transaction::, _>(|st, rt| { let prop = Amt::load(&st.proposals, rt.store()) diff --git a/vm/actor/src/builtin/miner/mod.rs b/vm/actor/src/builtin/miner/mod.rs index b06456f642ba..acaf1722e185 100644 --- a/vm/actor/src/builtin/miner/mod.rs +++ b/vm/actor/src/builtin/miner/mod.rs @@ -45,7 +45,6 @@ use fil_types::{ }; use ipld_amt::Amt; use ipld_blockstore::BlockStore; -use message::Message; use num_bigint::bigint_ser::{BigIntDe, BigIntSer}; use num_bigint::BigInt; use num_derive::FromPrimitive; @@ -130,7 +129,7 @@ impl Actor { let current_epoch = rt.curr_epoch(); let blake2b = |b: &[u8]| rt.syscalls().hash_blake2b(b); - let offset = assign_proving_period_offset(*rt.message().to(), current_epoch, blake2b) + let offset = assign_proving_period_offset(*rt.message().receiver(), current_epoch, blake2b) .map_err(|e| { ActorError::new( ExitCode::ErrSerialization, @@ -1210,7 +1209,7 @@ impl Actor { // Note: only the first reporter of any fault is rewarded. // Subsequent invocations fail because the target miner has been removed. rt.validate_immediate_caller_type(CALLER_TYPES_SIGNABLE.iter())?; - let reporter = *rt.message().from(); + let reporter = *rt.message().caller(); let fault = rt .syscalls() @@ -2109,14 +2108,14 @@ where BS: BlockStore, RT: Runtime, { - let miner_actor_id: u64 = if let Payload::ID(i) = rt.message().to().payload() { + let miner_actor_id: u64 = if let Payload::ID(i) = rt.message().receiver().payload() { *i } else { panic!("could not provide ID address"); }; // Regenerate challenge randomness, which must match that generated for the proof. - let entropy = rt.message().to().marshal_cbor().unwrap(); + let entropy = rt.message().receiver().marshal_cbor().unwrap(); let randomness: PoStRandomness = rt.get_randomness(WindowedPoStChallengeSeed, challenge_epoch, &entropy)?; @@ -2168,12 +2167,12 @@ where let commd = request_unsealed_sector_cid(rt, params.registered_proof, params.deal_ids.clone())?; - let miner_actor_id: u64 = if let Payload::ID(i) = rt.message().to().payload() { + let miner_actor_id: u64 = if let Payload::ID(i) = rt.message().receiver().payload() { *i } else { panic!("could not provide ID address"); }; - let entropy = rt.message().to().marshal_cbor().unwrap(); + let entropy = rt.message().receiver().marshal_cbor().unwrap(); let randomness: SealRandom = rt.get_randomness(SealRandomness, params.seal_rand_epoch, &entropy)?; let interactive_randomness: InteractiveSealRandomness = rt.get_randomness( diff --git a/vm/actor/src/builtin/multisig/mod.rs b/vm/actor/src/builtin/multisig/mod.rs index f11435c9ce13..ce72f2c04ae1 100644 --- a/vm/actor/src/builtin/multisig/mod.rs +++ b/vm/actor/src/builtin/multisig/mod.rs @@ -9,7 +9,6 @@ pub use self::types::*; use crate::{make_map, CALLER_TYPES_SIGNABLE, INIT_ACTOR_ADDR}; use address::Address; use ipld_blockstore::BlockStore; -use message::Message; use num_derive::FromPrimitive; use num_traits::FromPrimitive; use runtime::{ActorCode, Runtime}; @@ -67,7 +66,7 @@ impl Actor { }; if params.unlock_duration != 0 { - st.initial_balance = rt.message().value().clone(); + st.initial_balance = rt.message().value_received().clone(); st.unlock_duration = params.unlock_duration; st.start_epoch = rt.curr_epoch(); } @@ -83,7 +82,7 @@ impl Actor { RT: Runtime, { rt.validate_immediate_caller_type(CALLER_TYPES_SIGNABLE.iter())?; - let caller_addr: Address = *rt.message().from(); + let caller_addr: Address = *rt.message().caller(); let st: State = rt.state()?; Self::validate_signer(rt, &st, &caller_addr)?; @@ -129,7 +128,7 @@ impl Actor { RT: Runtime, { rt.validate_immediate_caller_type(CALLER_TYPES_SIGNABLE.iter())?; - let caller_addr: Address = *rt.message().from(); + let caller_addr: Address = *rt.message().caller(); // Validate signer let st = rt.state()?; @@ -145,7 +144,7 @@ impl Actor { RT: Runtime, { rt.validate_immediate_caller_type(CALLER_TYPES_SIGNABLE.iter())?; - let caller_addr: Address = *rt.message().from(); + let caller_addr: Address = *rt.message().caller(); let st: State = rt.state()?; Self::validate_signer(rt, &st, &caller_addr)?; @@ -187,7 +186,7 @@ impl Actor { BS: BlockStore, RT: Runtime, { - rt.validate_immediate_caller_is(std::iter::once(rt.message().to()))?; + rt.validate_immediate_caller_is(std::iter::once(rt.message().receiver()))?; rt.transaction::(|st, _| { // Check if signer to add is already signer @@ -214,7 +213,7 @@ impl Actor { BS: BlockStore, RT: Runtime, { - rt.validate_immediate_caller_is(std::iter::once(rt.message().to()))?; + rt.validate_immediate_caller_is(std::iter::once(rt.message().receiver()))?; rt.transaction::(|st, _| { // Check that signer to remove exists @@ -249,7 +248,7 @@ impl Actor { BS: BlockStore, RT: Runtime, { - rt.validate_immediate_caller_is(std::iter::once(rt.message().to()))?; + rt.validate_immediate_caller_is(std::iter::once(rt.message().receiver()))?; rt.transaction::(|st, _| { // Check that signer to remove exists @@ -287,7 +286,7 @@ impl Actor { BS: BlockStore, RT: Runtime, { - rt.validate_immediate_caller_is(std::iter::once(rt.message().to()))?; + rt.validate_immediate_caller_is(std::iter::once(rt.message().receiver()))?; rt.transaction::(|st, _| { // Check if valid threshold value @@ -309,7 +308,7 @@ impl Actor { BS: BlockStore, RT: Runtime, { - let from = *rt.message().from(); + let from = *rt.message().caller(); let curr_bal = rt.current_balance()?; let curr_epoch = rt.curr_epoch(); // Approval transaction diff --git a/vm/actor/src/builtin/paych/mod.rs b/vm/actor/src/builtin/paych/mod.rs index 3d1d43c241e0..2f5cf78e00cb 100644 --- a/vm/actor/src/builtin/paych/mod.rs +++ b/vm/actor/src/builtin/paych/mod.rs @@ -10,7 +10,6 @@ use crate::{check_empty_params, ACCOUNT_ACTOR_CODE_ID, INIT_ACTOR_CODE_ID}; use address::Address; use encoding::to_vec; use ipld_blockstore::BlockStore; -use message::Message; use num_bigint::BigInt; use num_derive::FromPrimitive; use num_traits::{FromPrimitive, Zero}; @@ -92,7 +91,7 @@ impl Actor { let st: State = rt.state()?; rt.validate_immediate_caller_is([st.from, st.to].iter())?; - let signer = if rt.message().from() == &st.from { + let signer = if rt.message().caller() == &st.from { st.to } else { st.from diff --git a/vm/actor/src/builtin/power/mod.rs b/vm/actor/src/builtin/power/mod.rs index 8fc0dba670f4..1033c1942a5f 100644 --- a/vm/actor/src/builtin/power/mod.rs +++ b/vm/actor/src/builtin/power/mod.rs @@ -17,7 +17,6 @@ use crate::{ use address::Address; use fil_types::{SealVerifyInfo, StoragePower}; use ipld_blockstore::BlockStore; -use message::Message; use num_bigint::bigint_ser::{BigIntDe, BigIntSer}; use num_bigint::BigInt; use num_derive::FromPrimitive; @@ -81,7 +80,7 @@ impl Actor { RT: Runtime, { rt.validate_immediate_caller_type(CALLER_TYPES_SIGNABLE.iter())?; - let value = rt.message().value().clone(); + let value = rt.message().value_received().clone(); // TODO update this send, is now outdated let addresses: init::ExecReturn = rt .send(&INIT_ACTOR_ADDR, init::Method::Exec as u64, params, &value)? @@ -176,7 +175,7 @@ impl Actor { rt.transaction(|st: &mut State, rt| { let rb_power = BigInt::from(params.weight.sector_size as u64); let qa_power = qa_power_for_weight(¶ms.weight); - st.add_to_claim(rt.store(), rt.message().from(), &rb_power, &qa_power) + st.add_to_claim(rt.store(), rt.message().caller(), &rb_power, &qa_power) .map_err(|e| { ActorError::new( ExitCode::ErrIllegalState, @@ -198,7 +197,7 @@ impl Actor { rt.transaction(|st: &mut State, rt| { let (rb_power, qa_power) = powers_for_weights(params.weights); - st.add_to_claim(rt.store(), rt.message().from(), &rb_power, &qa_power) + st.add_to_claim(rt.store(), rt.message().caller(), &rb_power, &qa_power) .map_err(|e| { ActorError::new( ExitCode::ErrIllegalState, @@ -219,7 +218,7 @@ impl Actor { rt.transaction(|st: &mut State, rt| { let (rb_power, qa_power) = powers_for_weights(params.weights); - st.add_to_claim(rt.store(), rt.message().from(), &rb_power, &qa_power) + st.add_to_claim(rt.store(), rt.message().caller(), &rb_power, &qa_power) .map_err(|e| { ActorError::new( ExitCode::ErrIllegalState, @@ -239,7 +238,7 @@ impl Actor { rt.transaction(|st: &mut State, rt| { let (rb_power, qa_power) = powers_for_weights(params.weights); - st.add_to_claim(rt.store(), rt.message().from(), &rb_power, &qa_power) + st.add_to_claim(rt.store(), rt.message().caller(), &rb_power, &qa_power) .map_err(|e| { ActorError::new( ExitCode::ErrIllegalState, @@ -269,7 +268,7 @@ impl Actor { st.add_to_claim( rt.store(), - rt.message().from(), + rt.message().caller(), &BigInt::from(prev_weight.sector_size as u64), &prev_power, ) @@ -283,7 +282,7 @@ impl Actor { let new_power = qa_power_for_weight(&new_weight); st.add_to_claim( rt.store(), - rt.message().from(), + rt.message().caller(), &BigInt::from(new_weight.sector_size as u64), &new_power, ) @@ -307,7 +306,7 @@ impl Actor { { rt.validate_immediate_caller_type(std::iter::once(&*MINER_ACTOR_CODE_ID))?; let miner_event = CronEvent { - miner_addr: *rt.message().from(), + miner_addr: *rt.message().caller(), callback_payload: params.payload.clone(), }; @@ -387,7 +386,7 @@ impl Actor { RT: Runtime, { rt.validate_immediate_caller_type(std::iter::once(&*MINER_ACTOR_CODE_ID))?; - let miner_addr = *rt.message().from(); + let miner_addr = *rt.message().caller(); let st: State = rt.state()?; let claim = st @@ -456,7 +455,7 @@ impl Actor { Multimap::new(rt.store()) }; - let miner_addr = rt.message().from(); + let miner_addr = rt.message().caller(); mmap.add(miner_addr.to_bytes().into(), seal_info) .map_err(|e| { ActorError::new( diff --git a/vm/actor/src/builtin/verifreg/mod.rs b/vm/actor/src/builtin/verifreg/mod.rs index 15f40b791eb7..30b4aad22f3b 100644 --- a/vm/actor/src/builtin/verifreg/mod.rs +++ b/vm/actor/src/builtin/verifreg/mod.rs @@ -11,7 +11,6 @@ use address::Address; use ipld_blockstore::BlockStore; use ipld_hamt::BytesKey; use ipld_hamt::Hamt; -use message::Message; use num_derive::FromPrimitive; use num_traits::{FromPrimitive, Zero}; use runtime::{ActorCode, Runtime}; @@ -116,8 +115,7 @@ impl Actor { rt.validate_immediate_caller_accept_any(); rt.transaction(|st: &mut State, rt| { // Validate caller is one of the verifiers. - let message: Box<&dyn Message> = Box::new(rt.message()); - let verify_addr = message.from(); + let verify_addr = rt.message().caller(); let verifier_cap = st .get_verifier(rt.store(), &verify_addr) diff --git a/vm/actor/tests/account_actor_test.rs b/vm/actor/tests/account_actor_test.rs index fb6edbea03e2..200d05cc738f 100644 --- a/vm/actor/tests/account_actor_test.rs +++ b/vm/actor/tests/account_actor_test.rs @@ -6,8 +6,6 @@ mod common; use actor::{account::State, ACCOUNT_ACTOR_CODE_ID, SYSTEM_ACTOR_ADDR, SYSTEM_ACTOR_CODE_ID}; use address::Address; use common::*; -use db::MemoryDB; -use message::UnsignedMessage; use vm::{ExitCode, Serialized}; macro_rules! account_tests { @@ -17,11 +15,12 @@ macro_rules! account_tests { fn $name() { let (addr, exit_code) = $value; - let bs = MemoryDB::default(); - let receiver = Address::new_id(100); - let message = UnsignedMessage::builder().to(receiver.clone()).from(SYSTEM_ACTOR_ADDR.clone()).build().unwrap(); - let mut rt = MockRuntime::new(&bs, message); - rt.caller_type = SYSTEM_ACTOR_CODE_ID.clone(); + let mut rt = MockRuntime { + receiver: Address::new_id(100), + caller: SYSTEM_ACTOR_ADDR.clone(), + caller_type: SYSTEM_ACTOR_CODE_ID.clone(), + ..Default::default() + }; rt.expect_validate_caller_addr(&[*SYSTEM_ACTOR_ADDR]); if exit_code.is_success() { diff --git a/vm/actor/tests/common/mod.rs b/vm/actor/tests/common/mod.rs index 8dd3f34e54f8..a0ad0d35d72d 100644 --- a/vm/actor/tests/common/mod.rs +++ b/vm/actor/tests/common/mod.rs @@ -10,25 +10,26 @@ use address::Address; use cid::{multihash::Blake2b256, Cid}; use clock::ChainEpoch; use crypto::{DomainSeparationTag, Signature}; +use db::MemoryDB; use encoding::{blake2b_256, de::DeserializeOwned, Cbor}; use fil_types::{PieceInfo, RegisteredSealProof, SealVerifyInfo, WindowPoStVerifyInfo}; use ipld_blockstore::BlockStore; -use message::{Message, UnsignedMessage}; -use runtime::{ActorCode, ConsensusFault, Runtime, Syscalls}; +use runtime::{ActorCode, ConsensusFault, MessageInfo, Runtime, Syscalls}; use std::cell::{Cell, RefCell}; use std::collections::{HashMap, VecDeque}; use std::error::Error as StdError; use vm::{ActorError, ExitCode, MethodNum, Randomness, Serialized, TokenAmount}; -pub struct MockRuntime<'a, BS: BlockStore> { +pub struct MockRuntime { pub epoch: ChainEpoch, - pub caller_type: Cid, pub miner: Address, - //pub value_received: TokenAmount, pub id_addresses: HashMap, pub actor_code_cids: HashMap, pub new_actor_addr: Option

, - pub message: UnsignedMessage, + pub receiver: Address, + pub caller: Address, + pub caller_type: Cid, + pub value_received: TokenAmount, // TODO: syscalls: syscaller @@ -39,7 +40,7 @@ pub struct MockRuntime<'a, BS: BlockStore> { // VM Impl pub in_call: bool, - pub store: &'a BS, + pub store: MemoryDB, pub in_transaction: bool, // Expectations @@ -55,6 +56,38 @@ pub struct MockRuntime<'a, BS: BlockStore> { pub expect_verify_consensus_fault: RefCell>, } +impl Default for MockRuntime { + fn default() -> Self { + Self { + epoch: Default::default(), + miner: Address::new_id(0), + id_addresses: Default::default(), + actor_code_cids: Default::default(), + new_actor_addr: Default::default(), + receiver: Address::new_id(0), + caller: Address::new_id(0), + caller_type: Default::default(), + value_received: Default::default(), + state: Default::default(), + balance: Default::default(), + received: Default::default(), + in_call: Default::default(), + store: Default::default(), + in_transaction: Default::default(), + expect_validate_caller_any: Default::default(), + expect_validate_caller_addr: Default::default(), + expect_validate_caller_type: Default::default(), + expect_sends: Default::default(), + expect_create_actor: Default::default(), + expect_verify_sigs: Default::default(), + expect_verify_seal: Default::default(), + expect_verify_post: Default::default(), + expect_compute_unsealed_sector_cid: Default::default(), + expect_verify_consensus_fault: Default::default(), + } + } +} + #[derive(Clone, Debug)] pub struct ExpectCreateActor { pub code_id: Cid, @@ -110,44 +143,7 @@ pub struct ExpectComputeUnsealedSectorCid { exit_code: ExitCode, } -impl<'a, BS> MockRuntime<'a, BS> -where - BS: BlockStore, -{ - pub fn new(bs: &'a BS, message: UnsignedMessage) -> Self { - MockRuntime { - epoch: 0, - caller_type: Cid::default(), - - miner: Address::new_id(0), - - id_addresses: HashMap::new(), - actor_code_cids: HashMap::new(), - new_actor_addr: None, - - message: message, - state: None, - balance: 0u8.into(), - received: 0u8.into(), - - // VM Impl - in_call: false, - store: bs, - in_transaction: false, - - // Expectations - expect_validate_caller_any: Cell::new(false), - expect_validate_caller_addr: RefCell::new(None), - expect_validate_caller_type: RefCell::new(None), - expect_sends: VecDeque::new(), - expect_create_actor: None, - expect_verify_sigs: RefCell::new(vec![]), - expect_verify_seal: RefCell::new(None), - expect_verify_post: RefCell::new(None), - expect_compute_unsealed_sector_cid: RefCell::new(None), - expect_verify_consensus_fault: RefCell::new(None), - } - } +impl MockRuntime { fn require_in_call(&self) { assert!( self.in_call, @@ -375,34 +371,33 @@ where #[allow(dead_code)] pub fn set_caller(&mut self, code_id: Cid, address: Address) { - self.message = UnsignedMessage::builder() - .to(self.message.to().clone()) - .from(address.clone()) - .value(self.message.value().clone()) - .build() - .unwrap(); + self.caller = address; self.caller_type = code_id.clone(); self.actor_code_cids.insert(address, code_id); } #[allow(dead_code)] pub fn set_value(&mut self, value: TokenAmount) { - self.message = UnsignedMessage::builder() - .to(self.message.to().clone()) - .from(self.message.from().clone()) - .value(value) - .build() - .unwrap(); + self.value_received = value; + } +} + +impl MessageInfo for MockRuntime { + fn caller(&self) -> &Address { + &self.caller + } + fn receiver(&self) -> &Address { + &self.receiver + } + fn value_received(&self) -> &TokenAmount { + &self.value_received } } -impl Runtime for MockRuntime<'_, BS> -where - BS: BlockStore, -{ - fn message(&self) -> &UnsignedMessage { +impl Runtime for MockRuntime { + fn message(&self) -> &dyn MessageInfo { self.require_in_call(); - &self.message + self } fn curr_epoch(&self) -> ChainEpoch { @@ -441,7 +436,7 @@ where ); for expected in &addrs { - if self.message().from() == expected { + if self.message().caller() == expected { *self.expect_validate_caller_addr.borrow_mut() = None; return Ok(()); } @@ -451,7 +446,7 @@ where ExitCode::ErrForbidden, format!( "caller address {:?} forbidden, allowed: {:?}", - self.message().from(), + self.message().caller(), &addrs ), )); @@ -563,8 +558,8 @@ where Ok(ret) } - fn store(&self) -> &BS { - self.store + fn store(&self) -> &MemoryDB { + &self.store } fn send( @@ -666,10 +661,7 @@ where } } -impl Syscalls for MockRuntime<'_, BS> -where - BS: BlockStore, -{ +impl Syscalls for MockRuntime { fn verify_signature( &self, signature: &Signature, diff --git a/vm/actor/tests/cron_actor_test.rs b/vm/actor/tests/cron_actor_test.rs index 651bdba078f7..460c94e2ea76 100644 --- a/vm/actor/tests/cron_actor_test.rs +++ b/vm/actor/tests/cron_actor_test.rs @@ -8,28 +8,19 @@ use actor::{ }; use address::Address; use common::*; -use db::MemoryDB; -use ipld_blockstore::BlockStore; -use message::UnsignedMessage; use vm::{ExitCode, Serialized}; -fn construct_runtime<'a, BS: BlockStore>(bs: &'a BS) -> MockRuntime<'a, BS> { - let receiver = Address::new_id(100); - - let message = UnsignedMessage::builder() - .from(SYSTEM_ACTOR_ADDR.clone()) - .to(receiver.clone()) - .build() - .unwrap(); - let mut rt = MockRuntime::new(bs, message); - rt.caller_type = SYSTEM_ACTOR_CODE_ID.clone(); - return rt; +fn construct_runtime() -> MockRuntime { + MockRuntime { + receiver: Address::new_id(100), + caller: SYSTEM_ACTOR_ADDR.clone(), + caller_type: SYSTEM_ACTOR_CODE_ID.clone(), + ..Default::default() + } } #[test] fn construct_with_empty_entries() { - let bs = MemoryDB::default(); - - let mut rt = construct_runtime(&bs); + let mut rt = construct_runtime(); construct_and_verify(&mut rt, &ConstructorParams { entries: vec![] }); let state: State = rt.get_state().unwrap(); @@ -39,9 +30,7 @@ fn construct_with_empty_entries() { #[test] fn construct_with_entries() { - let bs = MemoryDB::default(); - - let mut rt = construct_runtime(&bs); + let mut rt = construct_runtime(); let entry1 = Entry { receiver: Address::new_id(1001), @@ -73,16 +62,14 @@ fn construct_with_entries() { #[test] fn epoch_tick_with_empty_entries() { - let bs = MemoryDB::default(); - let mut rt = construct_runtime(&bs); + let mut rt = construct_runtime(); construct_and_verify(&mut rt, &ConstructorParams { entries: vec![] }); epoch_tick_and_verify(&mut rt); } #[test] fn epoch_tick_with_entries() { - let bs = MemoryDB::default(); - let mut rt = construct_runtime(&bs); + let mut rt = construct_runtime(); let entry1 = Entry { receiver: Address::new_id(1001), @@ -149,7 +136,7 @@ fn epoch_tick_with_entries() { epoch_tick_and_verify(&mut rt); } -fn construct_and_verify(rt: &mut MockRuntime<'_, BS>, params: &ConstructorParams) { +fn construct_and_verify(rt: &mut MockRuntime, params: &ConstructorParams) { rt.expect_validate_caller_addr(&[*SYSTEM_ACTOR_ADDR]); let ret = rt .call( @@ -162,7 +149,7 @@ fn construct_and_verify(rt: &mut MockRuntime<'_, BS>, params: &C rt.verify(); } -fn epoch_tick_and_verify(rt: &mut MockRuntime<'_, BS>) { +fn epoch_tick_and_verify(rt: &mut MockRuntime) { rt.expect_validate_caller_addr(&[*SYSTEM_ACTOR_ADDR]); let ret = rt .call(&*CRON_ACTOR_CODE_ID, 2, &Serialized::default()) diff --git a/vm/actor/tests/init_actor_test.rs b/vm/actor/tests/init_actor_test.rs index 3a7308f9a4d8..ded33e66a325 100644 --- a/vm/actor/tests/init_actor_test.rs +++ b/vm/actor/tests/init_actor_test.rs @@ -12,29 +12,22 @@ use actor::{ use address::Address; use cid::Cid; use common::*; -use db::MemoryDB; -use ipld_blockstore::BlockStore; -use message::{Message, UnsignedMessage}; use serde::Serialize; use vm::{ActorError, ExitCode, Serialized, TokenAmount, METHOD_CONSTRUCTOR}; -fn construct_runtime<'a, BS: BlockStore>(bs: &'a BS) -> MockRuntime<'a, BS> { - let receiver = Address::new_id(1000); - let message = UnsignedMessage::builder() - .to(receiver.clone()) - .from(SYSTEM_ACTOR_ADDR.clone()) - .build() - .unwrap(); - let mut rt = MockRuntime::new(bs, message); - rt.caller_type = SYSTEM_ACTOR_CODE_ID.clone(); - return rt; +fn construct_runtime() -> MockRuntime { + MockRuntime { + receiver: Address::new_id(1000), + caller: *SYSTEM_ACTOR_ADDR, + caller_type: SYSTEM_ACTOR_CODE_ID.clone(), + ..Default::default() + } } // Test to make sure we abort actors that can not call the exec function #[test] fn abort_cant_call_exec() { - let bs = MemoryDB::default(); - let mut rt = construct_runtime(&bs); + let mut rt = construct_runtime(); construct_and_verify(&mut rt); let anne = Address::new_id(1001); @@ -47,8 +40,7 @@ fn abort_cant_call_exec() { #[test] fn create_2_payment_channels() { - let bs = MemoryDB::default(); - let mut rt = construct_runtime(&bs); + let mut rt = construct_runtime(); construct_and_verify(&mut rt); let anne = Address::new_id(1001); @@ -58,14 +50,8 @@ fn create_2_payment_channels() { let pay_channel_string = format!("paych_{}", n); let paych = pay_channel_string.as_bytes(); - rt.balance = TokenAmount::from(100u8); - - rt.message = UnsignedMessage::builder() - .to(rt.message.to().clone()) - .from(rt.message.from().clone()) - .value(TokenAmount::from(100u8)) - .build() - .unwrap(); + rt.balance = TokenAmount::from(100); + rt.value_received = TokenAmount::from(100); let unique_address = Address::new_actor(paych); rt.new_actor_addr = Some(Address::new_actor(paych)); @@ -102,7 +88,7 @@ fn create_2_payment_channels() { let state: State = rt.get_state().unwrap(); let returned_address = state - .resolve_address(rt.store, &unique_address) + .resolve_address(&rt.store, &unique_address) .expect("Address should have been found"); assert_eq!(returned_address, expected_id_addr, "Wrong Address returned"); @@ -111,8 +97,7 @@ fn create_2_payment_channels() { #[test] fn create_storage_miner() { - let bs = MemoryDB::default(); - let mut rt = construct_runtime(&bs); + let mut rt = construct_runtime(); construct_and_verify(&mut rt); // only the storage power actor can create a miner @@ -149,21 +134,20 @@ fn create_storage_miner() { // Address should be resolved let state: State = rt.get_state().unwrap(); let returned_address = state - .resolve_address(rt.store, &unique_address) + .resolve_address(&rt.store, &unique_address) .expect("Address should have been found"); assert_eq!(expected_id_addr, returned_address); // Should return error since the address of flurbo is unknown let unknown_addr = Address::new_actor(b"flurbo"); state - .resolve_address(rt.store, &unknown_addr) + .resolve_address(&rt.store, &unknown_addr) .expect_err("Address should have not been found"); } #[test] fn create_multisig_actor() { - let bs = MemoryDB::default(); - let mut rt = construct_runtime(&bs); + let mut rt = construct_runtime(); construct_and_verify(&mut rt); // Actor creating multisig actor @@ -206,8 +190,7 @@ fn create_multisig_actor() { #[test] fn sending_constructor_failure() { - let bs = MemoryDB::default(); - let mut rt = construct_runtime(&bs); + let mut rt = construct_runtime(); construct_and_verify(&mut rt); // Only the storage power actor can create a miner @@ -250,7 +233,7 @@ fn sending_constructor_failure() { let state: State = rt.get_state().unwrap(); let returned_address = state - .resolve_address(rt.store, &unique_address) + .resolve_address(&rt.store, &unique_address) .expect_err("Address resolution should have failed"); assert_eq!( @@ -259,7 +242,7 @@ fn sending_constructor_failure() { ); } -fn construct_and_verify<'a, BS: BlockStore>(rt: &mut MockRuntime<'a, BS>) { +fn construct_and_verify(rt: &mut MockRuntime) { rt.expect_validate_caller_addr(&[SYSTEM_ACTOR_ADDR.clone()]); let params = ConstructorParams { network_name: "mock".to_string(), @@ -278,7 +261,7 @@ fn construct_and_verify<'a, BS: BlockStore>(rt: &mut MockRuntime<'a, BS>) { let state_data: State = rt.get_state().unwrap(); // Gets the Result(CID) - let empty_map = Multimap::from_root(rt.store, &state_data.address_map) + let empty_map = Multimap::from_root(&rt.store, &state_data.address_map) .unwrap() .root(); @@ -287,8 +270,8 @@ fn construct_and_verify<'a, BS: BlockStore>(rt: &mut MockRuntime<'a, BS>) { assert_eq!("mock".to_string(), state_data.network_name); } -fn exec_and_verify<'a, BS: BlockStore, S: Serialize>( - rt: &mut MockRuntime<'a, BS>, +fn exec_and_verify<'a, S: Serialize>( + rt: &mut MockRuntime, code_id: Cid, params: &S, ) -> Result diff --git a/vm/actor/tests/market_actor_test.rs b/vm/actor/tests/market_actor_test.rs index f54d422f7459..b2a6adfd1abc 100644 --- a/vm/actor/tests/market_actor_test.rs +++ b/vm/actor/tests/market_actor_test.rs @@ -13,10 +13,8 @@ use actor::{ use address::Address; use clock::EPOCH_UNDEFINED; use common::*; -use db::MemoryDB; use ipld_amt::Amt; -use ipld_blockstore::BlockStore; -use message::UnsignedMessage; +use std::collections::HashMap; use vm::{ExitCode, Serialized, TokenAmount, METHOD_CONSTRUCTOR, METHOD_SEND}; const OWNER_ID: u64 = 101; @@ -24,25 +22,20 @@ const PROVIDER_ID: u64 = 102; const WORKER_ID: u64 = 103; const CLIENT_ID: u64 = 104; -fn setup(bs: &BS) -> MockRuntime<'_, BS> { - let message = UnsignedMessage::builder() - .to(*STORAGE_MARKET_ACTOR_ADDR) - .from(*SYSTEM_ACTOR_ADDR) - .build() - .unwrap(); - - let mut rt = MockRuntime::new(bs, message); - - rt.caller_type = INIT_ACTOR_CODE_ID.clone(); - - rt.actor_code_cids - .insert(Address::new_id(OWNER_ID), ACCOUNT_ACTOR_CODE_ID.clone()); - rt.actor_code_cids - .insert(Address::new_id(WORKER_ID), ACCOUNT_ACTOR_CODE_ID.clone()); - rt.actor_code_cids - .insert(Address::new_id(PROVIDER_ID), MINER_ACTOR_CODE_ID.clone()); - rt.actor_code_cids - .insert(Address::new_id(CLIENT_ID), ACCOUNT_ACTOR_CODE_ID.clone()); +fn setup() -> MockRuntime { + let mut actor_code_cids = HashMap::default(); + actor_code_cids.insert(Address::new_id(OWNER_ID), ACCOUNT_ACTOR_CODE_ID.clone()); + actor_code_cids.insert(Address::new_id(WORKER_ID), ACCOUNT_ACTOR_CODE_ID.clone()); + actor_code_cids.insert(Address::new_id(PROVIDER_ID), MINER_ACTOR_CODE_ID.clone()); + actor_code_cids.insert(Address::new_id(CLIENT_ID), ACCOUNT_ACTOR_CODE_ID.clone()); + + let mut rt = MockRuntime { + receiver: *STORAGE_MARKET_ACTOR_ADDR, + caller: *SYSTEM_ACTOR_ADDR, + caller_type: INIT_ACTOR_CODE_ID.clone(), + actor_code_cids, + ..Default::default() + }; construct_and_verify(&mut rt); rt @@ -51,18 +44,12 @@ fn setup(bs: &BS) -> MockRuntime<'_, BS> { // TODO add array stuff #[test] fn simple_construction() { - let bs = MemoryDB::default(); - - let receiver: Address = Address::new_id(100); - - let message = UnsignedMessage::builder() - .to(receiver.clone()) - .from(*SYSTEM_ACTOR_ADDR) - .build() - .unwrap(); - - let mut rt = MockRuntime::new(&bs, message); - rt.caller_type = INIT_ACTOR_CODE_ID.clone(); + let mut rt = MockRuntime { + receiver: Address::new_id(100), + caller: *SYSTEM_ACTOR_ADDR, + caller_type: INIT_ACTOR_CODE_ID.clone(), + ..Default::default() + }; rt.expect_validate_caller_addr(&[SYSTEM_ACTOR_ADDR.clone()]); @@ -78,7 +65,7 @@ fn simple_construction() { rt.verify(); - let store = rt.store; + let store = &rt.store; let empty_map = Multimap::new(store).root().unwrap(); let empty_set = SetMultimap::new(store).root().unwrap(); let empty_array = Amt::::new(store).flush().unwrap(); @@ -103,8 +90,7 @@ fn add_provider_escrow_funds() { let provider_addr = Address::new_id(PROVIDER_ID); for caller_addr in vec![owner_addr, worker_addr] { - let bs = MemoryDB::default(); - let mut rt = setup(&bs); + let mut rt = setup(); for test_case in test_cases.clone() { rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), caller_addr); @@ -127,7 +113,7 @@ fn add_provider_escrow_funds() { let state_data: State = rt.get_state().unwrap(); assert_eq!( state_data - .get_escrow_balance(rt.store, &provider_addr) + .get_escrow_balance(&rt.store, &provider_addr) .unwrap(), TokenAmount::from(test_case.1 as u64) ); @@ -137,8 +123,7 @@ fn add_provider_escrow_funds() { #[test] fn account_actor_check() { - let bs = MemoryDB::default(); - let mut rt = setup(&bs); + let mut rt = setup(); let amount = TokenAmount::from(10u8); rt.set_value(amount); @@ -173,8 +158,7 @@ fn add_non_provider_funds() { let worker_addr = Address::new_id(WORKER_ID); for caller_addr in vec![client_addr, worker_addr] { - let bs = MemoryDB::default(); - let mut rt = setup(&bs); + let mut rt = setup(); for test_case in test_cases.clone() { rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), caller_addr); @@ -197,7 +181,7 @@ fn add_non_provider_funds() { let state_data: State = rt.get_state().unwrap(); assert_eq!( state_data - .get_escrow_balance(rt.store, &caller_addr) + .get_escrow_balance(&rt.store, &caller_addr) .unwrap(), TokenAmount::from(test_case.1 as u8) ); @@ -207,8 +191,7 @@ fn add_non_provider_funds() { #[test] fn withdraw_provider_to_owner() { - let bs = MemoryDB::default(); - let mut rt = setup(&bs); + let mut rt = setup(); let owner_addr = Address::new_id(OWNER_ID); let worker_addr = Address::new_id(WORKER_ID); @@ -227,7 +210,7 @@ fn withdraw_provider_to_owner() { assert_eq!( amount, state_data - .get_escrow_balance(rt.store, &provider_addr) + .get_escrow_balance(&rt.store, &provider_addr) .unwrap() ); @@ -264,7 +247,7 @@ fn withdraw_provider_to_owner() { assert_eq!( state_data - .get_escrow_balance(rt.store, &provider_addr) + .get_escrow_balance(&rt.store, &provider_addr) .unwrap(), TokenAmount::from(19u8) ); @@ -273,8 +256,7 @@ fn withdraw_provider_to_owner() { #[test] fn withdraw_non_provider() { // Test is currently failing because curr_epoch is 0. When subtracted by 1, it goe snmegative causing a overflow error - let bs = MemoryDB::default(); - let mut rt = setup(&bs); + let mut rt = setup(); let client_addr = Address::new_id(CLIENT_ID); @@ -285,7 +267,7 @@ fn withdraw_non_provider() { assert_eq!( amount, state_data - .get_escrow_balance(rt.store, &client_addr) + .get_escrow_balance(&rt.store, &client_addr) .unwrap() ); @@ -325,7 +307,7 @@ fn withdraw_non_provider() { assert_eq!( state_data - .get_escrow_balance(rt.store, &client_addr) + .get_escrow_balance(&rt.store, &client_addr) .unwrap(), TokenAmount::from(19u8) ); @@ -333,8 +315,7 @@ fn withdraw_non_provider() { #[test] fn client_withdraw_more_than_available() { - let bs = MemoryDB::default(); - let mut rt = setup(&bs); + let mut rt = setup(); let client_addr = Address::new_id(CLIENT_ID); @@ -377,7 +358,7 @@ fn client_withdraw_more_than_available() { assert_eq!( state_data - .get_escrow_balance(rt.store, &client_addr) + .get_escrow_balance(&rt.store, &client_addr) .unwrap(), TokenAmount::from(0u8) ); @@ -385,8 +366,7 @@ fn client_withdraw_more_than_available() { #[test] fn worker_withdraw_more_than_available() { - let bs = MemoryDB::default(); - let mut rt = setup(&bs); + let mut rt = setup(); let owner_addr = Address::new_id(OWNER_ID); let worker_addr = Address::new_id(WORKER_ID); @@ -405,7 +385,7 @@ fn worker_withdraw_more_than_available() { assert_eq!( amount, state_data - .get_escrow_balance(rt.store, &provider_addr) + .get_escrow_balance(&rt.store, &provider_addr) .unwrap() ); @@ -442,14 +422,14 @@ fn worker_withdraw_more_than_available() { assert_eq!( state_data - .get_escrow_balance(rt.store, &provider_addr) + .get_escrow_balance(&rt.store, &provider_addr) .unwrap(), TokenAmount::from(0u8) ); } -fn expect_provider_control_address( - rt: &mut MockRuntime<'_, BS>, +fn expect_provider_control_address( + rt: &mut MockRuntime, provider: Address, owner: Address, worker: Address, @@ -471,8 +451,8 @@ fn expect_provider_control_address( ); } -fn add_provider_funds( - rt: &mut MockRuntime<'_, BS>, +fn add_provider_funds( + rt: &mut MockRuntime, provider: Address, owner: Address, worker: Address, @@ -496,11 +476,7 @@ fn add_provider_funds( rt.balance = rt.balance.clone() + amount; } -fn add_participant_funds( - rt: &mut MockRuntime<'_, BS>, - addr: Address, - amount: TokenAmount, -) { +fn add_participant_funds(rt: &mut MockRuntime, addr: Address, amount: TokenAmount) { rt.set_value(amount.clone()); rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), addr.clone()); @@ -523,7 +499,7 @@ fn add_participant_funds( rt.balance = rt.balance.clone() + amount; } -fn construct_and_verify(rt: &mut MockRuntime<'_, BS>) { +fn construct_and_verify(rt: &mut MockRuntime) { rt.expect_validate_caller_addr(&[SYSTEM_ACTOR_ADDR.clone()]); assert_eq!( Serialized::default(), diff --git a/vm/actor/tests/paych_actor_test.rs b/vm/actor/tests/paych_actor_test.rs index 69ec9e8fca5c..ac10cdb7eeef 100644 --- a/vm/actor/tests/paych_actor_test.rs +++ b/vm/actor/tests/paych_actor_test.rs @@ -8,19 +8,17 @@ use actor::{ SignedVoucher, State as PState, UpdateChannelStateParams, LANE_LIMIT, SETTLE_DELAY, }, ACCOUNT_ACTOR_CODE_ID, INIT_ACTOR_ADDR, INIT_ACTOR_CODE_ID, MULTISIG_ACTOR_CODE_ID, - PAYCH_ACTOR_CODE_ID, SYSTEM_ACTOR_ADDR, + PAYCH_ACTOR_CODE_ID, }; use address::Address; use cid::Cid; use clock::ChainEpoch; use common::*; use crypto::Signature; -use db::MemoryDB; use derive_builder::Builder; use encoding::to_vec; -use ipld_blockstore::BlockStore; -use message::UnsignedMessage; use num_bigint::BigInt; +use std::collections::HashMap; use vm::{ExitCode, Serialized, TokenAmount, METHOD_CONSTRUCTOR, METHOD_SEND}; const R_PAYEE_ADDR: u64 = 103; @@ -34,16 +32,11 @@ struct LaneParams { nonce: u64, } -fn is_ok<'a, BS: BlockStore>(rt: &mut MockRuntime<'a, BS>, method_num: u64, ser: &Serialized) { +fn is_ok(rt: &mut MockRuntime, method_num: u64, ser: &Serialized) { assert!(rt.call(&*PAYCH_ACTOR_CODE_ID, method_num, ser).is_ok()); } -fn expect_error<'a, BS: BlockStore>( - rt: &mut MockRuntime<'a, BS>, - method_num: u64, - ser: &Serialized, - exp: ExitCode, -) { +fn expect_error(rt: &mut MockRuntime, method_num: u64, ser: &Serialized, exp: ExitCode) { assert_eq!( exp, rt.call(&*PAYCH_ACTOR_CODE_ID, method_num, ser,) @@ -59,27 +52,26 @@ mod paych_constructor { const TEST_PAYER_ADDR: u64 = 101; const TEST_CALLER_ADDR: u64 = 102; - fn construct_runtime<'a, BS: BlockStore>(bs: &'a BS) -> MockRuntime<'a, BS> { + fn construct_runtime() -> MockRuntime { let paych_addr = Address::new_id(TEST_PAYCH_ADDR); let payer_addr = Address::new_id(TEST_PAYER_ADDR); let caller_addr = Address::new_id(TEST_CALLER_ADDR); - let message = UnsignedMessage::builder() - .from(*SYSTEM_ACTOR_ADDR) - .to(paych_addr) - .build() - .unwrap(); - let mut rt = MockRuntime::new(bs, message); - rt.set_caller(INIT_ACTOR_CODE_ID.clone(), caller_addr); - rt.actor_code_cids - .insert(payer_addr, ACCOUNT_ACTOR_CODE_ID.clone()); - rt + let mut actor_code_cids = HashMap::default(); + actor_code_cids.insert(payer_addr, ACCOUNT_ACTOR_CODE_ID.clone()); + + MockRuntime { + receiver: paych_addr, + caller: caller_addr, + caller_type: INIT_ACTOR_CODE_ID.clone(), + actor_code_cids, + ..Default::default() + } } #[test] fn create_paych_actor_test() { let caller_addr = Address::new_id(TEST_CALLER_ADDR); - let bs = MemoryDB::default(); - let mut rt = construct_runtime(&bs); + let mut rt = construct_runtime(); rt.actor_code_cids .insert(caller_addr, ACCOUNT_ACTOR_CODE_ID.clone()); construct_and_verify(&mut rt, Address::new_id(TEST_PAYER_ADDR), caller_addr); @@ -87,8 +79,7 @@ mod paych_constructor { #[test] fn actor_doesnt_exist_test() { - let bs = MemoryDB::default(); - let mut rt = construct_runtime(&bs); + let mut rt = construct_runtime(); rt.expect_validate_caller_type(&[INIT_ACTOR_CODE_ID.clone()]); let params = ConstructorParams { to: Address::new_id(TEST_PAYCH_ADDR), @@ -134,18 +125,18 @@ mod paych_constructor { ]; for test_case in test_cases { - let bs = MemoryDB::default(); - let message = UnsignedMessage::builder() - .from(*SYSTEM_ACTOR_ADDR) - .to(paych_addr) - .build() - .unwrap(); - let mut rt = MockRuntime::new(&bs, message); - rt.set_caller(test_case.caller_code, caller_addr); + let mut actor_code_cids = HashMap::default(); + actor_code_cids.insert(test_case.paych_addr, test_case.new_actor_code); + actor_code_cids.insert(payer_addr, test_case.payer_code); + + let mut rt = MockRuntime { + receiver: paych_addr, + caller: caller_addr, + caller_type: test_case.caller_code, + actor_code_cids, + ..Default::default() + }; - rt.actor_code_cids - .insert(test_case.paych_addr, test_case.new_actor_code); - rt.actor_code_cids.insert(payer_addr, test_case.payer_code); rt.expect_validate_caller_type(&[INIT_ACTOR_CODE_ID.clone()]); let params = ConstructorParams { to: test_case.paych_addr, @@ -269,22 +260,21 @@ mod create_lane_tests { for test_case in test_cases { println!("Test Description {}", test_case.desc); - let bs = MemoryDB::default(); - let message = UnsignedMessage::builder() - .from(*SYSTEM_ACTOR_ADDR) - .to(paych_addr) - .gas_limit(1000) - .build() - .unwrap(); - let mut rt = MockRuntime::new(&bs, message); - rt.epoch = test_case.epoch; - rt.balance = TokenAmount::from(paych_balance.clone()); - rt.set_caller(INIT_ACTOR_CODE_ID.clone(), init_actor_addr); - - rt.actor_code_cids - .insert(payee_addr, ACCOUNT_ACTOR_CODE_ID.clone()); - rt.actor_code_cids - .insert(payer_addr, ACCOUNT_ACTOR_CODE_ID.clone()); + + let mut actor_code_cids = HashMap::default(); + actor_code_cids.insert(payee_addr, ACCOUNT_ACTOR_CODE_ID.clone()); + actor_code_cids.insert(payer_addr, ACCOUNT_ACTOR_CODE_ID.clone()); + + let mut rt = MockRuntime { + receiver: paych_addr, + caller: init_actor_addr, + caller_type: INIT_ACTOR_CODE_ID.clone(), + actor_code_cids, + epoch: test_case.epoch, + balance: paych_balance.clone(), + ..Default::default() + }; + construct_and_verify(&mut rt, payer_addr, payee_addr); let sv = SignedVoucher { @@ -348,8 +338,7 @@ mod update_channel_state_redeem { #[test] fn redeem_voucher_one_lane() { - let bs = MemoryDB::default(); - let (mut rt, mut sv) = require_create_cannel_with_lanes(&bs, 1); + let (mut rt, mut sv) = require_create_cannel_with_lanes(1); let state: PState = rt.get_state().unwrap(); let payee_addr = Address::new_id(R_PAYEE_ADDR); @@ -392,8 +381,7 @@ mod update_channel_state_redeem { #[test] fn redeem_voucher_correct_lane() { - let bs = MemoryDB::default(); - let (mut rt, mut sv) = require_create_cannel_with_lanes(&bs, 3); + let (mut rt, mut sv) = require_create_cannel_with_lanes(3); let state: PState = rt.get_state().unwrap(); let payee_addr = Address::new_id(R_PAYEE_ADDR); @@ -436,22 +424,15 @@ mod update_channel_state_redeem { mod merge_tests { use super::*; - fn construct_runtime<'a, BS: BlockStore>( - bs: &'a BS, - num_lanes: u64, - ) -> (MockRuntime<'a, BS>, SignedVoucher, PState) { - let (mut rt, sv) = require_create_cannel_with_lanes(bs, num_lanes); + fn construct_runtime(num_lanes: u64) -> (MockRuntime, SignedVoucher, PState) { + let (mut rt, sv) = require_create_cannel_with_lanes(num_lanes); let state: PState = rt.get_state().unwrap(); rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), state.from.clone()); rt.expect_validate_caller_addr(&[state.from, state.to]); (rt, sv, state) } - fn failure_end( - rt: &mut MockRuntime, - sv: SignedVoucher, - exp_exit_code: ExitCode, - ) { + fn failure_end(rt: &mut MockRuntime, sv: SignedVoucher, exp_exit_code: ExitCode) { let payee_addr = Address::new_id(R_PAYEE_ADDR); rt.expect_verify_signature(ExpectedVerifySig { sig: sv.clone().signature.unwrap(), @@ -471,8 +452,7 @@ mod merge_tests { #[test] fn merge_success() { let num_lanes = 3; - let bs = MemoryDB::default(); - let (mut rt, mut sv, mut state) = construct_runtime(&bs, num_lanes); + let (mut rt, mut sv, mut state) = construct_runtime(num_lanes); let merge_to: &LaneState = &state.lane_states[0]; let merge_from: &LaneState = &state.lane_states[1]; @@ -560,9 +540,8 @@ mod merge_tests { ]; for tc in test_cases { - let bs = MemoryDB::default(); let num_lanes = 2; - let (mut rt, mut sv, state) = construct_runtime(&bs, num_lanes); + let (mut rt, mut sv, state) = construct_runtime(num_lanes); rt.balance = TokenAmount::from(tc.balance as u64); @@ -580,9 +559,8 @@ mod merge_tests { #[test] fn invalid_merge_lane_999() { - let bs = MemoryDB::default(); let num_lanes = 2; - let (mut rt, mut sv, state) = construct_runtime(&bs, num_lanes); + let (mut rt, mut sv, state) = construct_runtime(num_lanes); let merge_to: &LaneState = &state.lane_states[0]; let merge_from = LaneState { @@ -602,9 +580,8 @@ mod merge_tests { #[test] fn lane_limit_exceeded() { - let bs = MemoryDB::default(); let num_lanes = LANE_LIMIT as u64; - let (mut rt, mut sv, _) = construct_runtime(&bs, num_lanes); + let (mut rt, mut sv, _) = construct_runtime(num_lanes); sv.lane += 1; sv.nonce += 1; @@ -617,11 +594,8 @@ mod update_channel_state_extra { use super::*; const OTHER_ADDR: u64 = 104; - fn construct_runtime<'a, BS: BlockStore>( - bs: &'a BS, - exit_code: ExitCode, - ) -> (MockRuntime<'a, BS>, SignedVoucher) { - let (mut rt, mut sv) = require_create_cannel_with_lanes(bs, 1); + fn construct_runtime(exit_code: ExitCode) -> (MockRuntime, SignedVoucher) { + let (mut rt, mut sv) = require_create_cannel_with_lanes(1); let state: PState = rt.get_state().unwrap(); let other_addr = Address::new_id(OTHER_ADDR); let fake_params = [1, 2, 3, 4]; @@ -656,8 +630,7 @@ mod update_channel_state_extra { } #[test] fn extra_call_succeed() { - let bs = MemoryDB::default(); - let (mut rt, sv) = construct_runtime(&bs, ExitCode::Ok); + let (mut rt, sv) = construct_runtime(ExitCode::Ok); is_ok( &mut rt, Method::UpdateChannelState as u64, @@ -668,8 +641,7 @@ mod update_channel_state_extra { #[test] fn extra_call_fail() { - let bs = MemoryDB::default(); - let (mut rt, sv) = construct_runtime(&bs, ExitCode::ErrPlaceholder); + let (mut rt, sv) = construct_runtime(ExitCode::ErrPlaceholder); expect_error( &mut rt, Method::UpdateChannelState as u64, @@ -684,8 +656,7 @@ mod update_channel_state_settling { use super::*; #[test] fn update_channel_setting() { - let bs = MemoryDB::default(); - let (mut rt, sv) = require_create_cannel_with_lanes(&bs, 1); + let (mut rt, sv) = require_create_cannel_with_lanes(1); rt.epoch = 10; let state: PState = rt.get_state().unwrap(); rt.expect_validate_caller_addr(&[state.from, state.to]); @@ -744,8 +715,7 @@ mod secret_preimage { use super::*; #[test] fn succeed_correct_secret() { - let bs = MemoryDB::default(); - let (mut rt, sv) = require_create_cannel_with_lanes(&bs, 1); + let (mut rt, sv) = require_create_cannel_with_lanes(1); let state: PState = rt.get_state().unwrap(); rt.expect_validate_caller_addr(&[state.from, state.to]); @@ -769,8 +739,7 @@ mod secret_preimage { #[test] fn incorrect_secret() { - let bs = MemoryDB::default(); - let (mut rt, sv) = require_create_cannel_with_lanes(&bs, 1); + let (mut rt, sv) = require_create_cannel_with_lanes(1); let state: PState = rt.get_state().unwrap(); rt.expect_validate_caller_addr(&[state.from, state.to]); @@ -806,8 +775,7 @@ mod actor_settle { const EP: i64 = 10; #[test] fn adjust_settling_at() { - let bs = MemoryDB::default(); - let (mut rt, _sv) = require_create_cannel_with_lanes(&bs, 1); + let (mut rt, _sv) = require_create_cannel_with_lanes(1); rt.epoch = EP; let mut state: PState = rt.get_state().unwrap(); rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), state.from); @@ -823,8 +791,7 @@ mod actor_settle { #[test] fn call_twice() { - let bs = MemoryDB::default(); - let (mut rt, _sv) = require_create_cannel_with_lanes(&bs, 1); + let (mut rt, _sv) = require_create_cannel_with_lanes(1); rt.epoch = EP; let state: PState = rt.get_state().unwrap(); rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), state.from); @@ -842,8 +809,7 @@ mod actor_settle { #[test] fn settle_if_height_less() { - let bs = MemoryDB::default(); - let (mut rt, mut sv) = require_create_cannel_with_lanes(&bs, 1); + let (mut rt, mut sv) = require_create_cannel_with_lanes(1); rt.epoch = EP; let mut state: PState = rt.get_state().unwrap(); @@ -876,11 +842,7 @@ mod actor_settle { mod actor_collect { use super::*; - fn exp_send_multiple( - rt: &mut MockRuntime, - state: &PState, - exit_codes: [ExitCode; 2], - ) { + fn exp_send_multiple(rt: &mut MockRuntime, state: &PState, exit_codes: [ExitCode; 2]) { rt.epoch = 12; let sent_to_from = &rt.balance - state.to_send.clone(); rt.expect_send( @@ -904,8 +866,7 @@ mod actor_collect { #[test] fn happy_path() { - let bs = MemoryDB::default(); - let (mut rt, _sv) = require_create_cannel_with_lanes(&bs, 1); + let (mut rt, _sv) = require_create_cannel_with_lanes(1); rt.epoch = 10; let mut state: PState = rt.get_state().unwrap(); rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), state.from); @@ -956,8 +917,7 @@ mod actor_collect { ]; for tc in test_cases { - let bs = MemoryDB::default(); - let (mut rt, _sv) = require_create_cannel_with_lanes(&bs, 1); + let (mut rt, _sv) = require_create_cannel_with_lanes(1); rt.epoch = 10; let mut state: PState = rt.get_state().unwrap(); @@ -983,35 +943,29 @@ mod actor_collect { } } -fn require_create_cannel_with_lanes<'a, BS: BlockStore>( - bs: &'a BS, - num_lanes: u64, -) -> (MockRuntime<'a, BS>, SignedVoucher) { - let paych_addr = Address::new_id(100 as u64); +fn require_create_cannel_with_lanes(num_lanes: u64) -> (MockRuntime, SignedVoucher) { + let paych_addr = Address::new_id(100); let payer_addr = Address::new_id(R_PAYER_ADDR); let payee_addr = Address::new_id(R_PAYEE_ADDR); - let balance = TokenAmount::from(100_000 as u64); - let recieved = TokenAmount::from(0 as u64); - + let balance = TokenAmount::from(100_000); + let received = TokenAmount::from(0); let curr_epoch = 2; - let message = UnsignedMessage::builder() - .from(*SYSTEM_ACTOR_ADDR) - .to(paych_addr) - .gas_limit(1000) - .build() - .unwrap(); - - let mut rt = MockRuntime::new(bs, message); - - rt.epoch = curr_epoch; - rt.balance = TokenAmount::from(balance); - rt.received = TokenAmount::from(recieved); - rt.set_caller(INIT_ACTOR_CODE_ID.clone(), *INIT_ACTOR_ADDR); - rt.actor_code_cids - .insert(payee_addr, ACCOUNT_ACTOR_CODE_ID.clone()); - rt.actor_code_cids - .insert(payer_addr, ACCOUNT_ACTOR_CODE_ID.clone()); + let mut actor_code_cids = HashMap::default(); + actor_code_cids.insert(payee_addr, ACCOUNT_ACTOR_CODE_ID.clone()); + actor_code_cids.insert(payer_addr, ACCOUNT_ACTOR_CODE_ID.clone()); + + let mut rt = MockRuntime { + receiver: paych_addr, + caller: *INIT_ACTOR_ADDR, + caller_type: INIT_ACTOR_CODE_ID.clone(), + actor_code_cids, + received, + balance, + epoch: curr_epoch, + ..Default::default() + }; + construct_and_verify(&mut rt, payer_addr, payee_addr); let mut last_sv = SignedVoucher::default(); @@ -1031,10 +985,7 @@ fn require_create_cannel_with_lanes<'a, BS: BlockStore>( (rt, last_sv) } -fn require_add_new_lane( - rt: &mut MockRuntime<'_, BS>, - param: LaneParams, -) -> SignedVoucher { +fn require_add_new_lane(rt: &mut MockRuntime, param: LaneParams) -> SignedVoucher { let payee_addr = Address::new_id(103 as u64); let sig = Signature::new_bls(vec![0, 1, 2, 3, 4, 5, 6, 7]); let sv = SignedVoucher { @@ -1071,11 +1022,7 @@ fn require_add_new_lane( } } -fn construct_and_verify( - rt: &mut MockRuntime<'_, BS>, - sender: Address, - receiver: Address, -) { +fn construct_and_verify(rt: &mut MockRuntime, sender: Address, receiver: Address) { let params = ConstructorParams { from: sender, to: receiver, @@ -1090,21 +1037,13 @@ fn construct_and_verify( verify_initial_state(rt, sender, receiver); } -fn verify_initial_state( - rt: &mut MockRuntime<'_, BS>, - sender: Address, - receiver: Address, -) { +fn verify_initial_state(rt: &mut MockRuntime, sender: Address, receiver: Address) { let _state: PState = rt.get_state().unwrap(); let expected_state = PState::new(sender, receiver); verify_state(rt, -1, expected_state) } -fn verify_state( - rt: &mut MockRuntime<'_, BS>, - exp_lanes: i64, - expected_state: PState, -) { +fn verify_state(rt: &mut MockRuntime, exp_lanes: i64, expected_state: PState) { let state: PState = rt.get_state().unwrap(); assert_eq!(expected_state.to, state.to); assert_eq!(expected_state.from, state.from); diff --git a/vm/actor/tests/reward_actor_test.rs b/vm/actor/tests/reward_actor_test.rs index 216b1307134f..e23321b99abe 100644 --- a/vm/actor/tests/reward_actor_test.rs +++ b/vm/actor/tests/reward_actor_test.rs @@ -9,29 +9,21 @@ use actor::{ }; use address::Address; use common::*; -use db::MemoryDB; -use ipld_blockstore::BlockStore; -use message::UnsignedMessage; -use std::panic; use vm::{Serialized, TokenAmount, METHOD_CONSTRUCTOR}; -fn construct_runtime<'a, BS: BlockStore>(bs: &'a BS) -> MockRuntime<'a, BS> { - let message = UnsignedMessage::builder() - .to(*REWARD_ACTOR_ADDR) - .from(*SYSTEM_ACTOR_ADDR) - .build() - .unwrap(); - - let mut rt = MockRuntime::new(bs, message); - rt.caller_type = SYSTEM_ACTOR_CODE_ID.clone(); - return rt; +fn construct_runtime() -> MockRuntime { + MockRuntime { + receiver: *REWARD_ACTOR_ADDR, + caller: *SYSTEM_ACTOR_ADDR, + caller_type: SYSTEM_ACTOR_CODE_ID.clone(), + ..Default::default() + } } #[test] #[should_panic(expected = "actor current balance 0 insufficient to pay gas reward 10")] fn balance_less_than_reward() { - let bs = MemoryDB::default(); - let mut rt = construct_runtime(&bs); + let mut rt = construct_runtime(); construct_and_verify(&mut rt); let miner = Address::new_id(1000); @@ -56,7 +48,7 @@ fn balance_less_than_reward() { rt.verify() } -fn construct_and_verify<'a, BS: BlockStore>(rt: &mut MockRuntime<'a, BS>) { +fn construct_and_verify(rt: &mut MockRuntime) { rt.expect_validate_caller_addr(&[SYSTEM_ACTOR_ADDR.clone()]); let ret = rt .call( diff --git a/vm/interpreter/src/default_runtime.rs b/vm/interpreter/src/default_runtime.rs index 23cc2cd8da6a..4c7b7f58f9dd 100644 --- a/vm/interpreter/src/default_runtime.rs +++ b/vm/interpreter/src/default_runtime.rs @@ -17,7 +17,7 @@ use forest_encoding::Cbor; use ipld_blockstore::BlockStore; use message::{Message, UnsignedMessage}; use num_bigint::BigInt; -use runtime::{ActorCode, Runtime, Syscalls}; +use runtime::{ActorCode, MessageInfo, Runtime, Syscalls}; use state_tree::StateTree; use std::cell::RefCell; use std::marker::PhantomData; @@ -64,10 +64,7 @@ where rand: &'r ChainRand, ) -> Self { let price_list = price_list_by_epoch(epoch); - let gas_tracker = Rc::new(RefCell::new(GasTracker::new( - message.gas_limit(), - gas_used, - ))); + let gas_tracker = Rc::new(RefCell::new(GasTracker::new(message.gas_limit(), gas_used))); let gas_block_store = GasBlockStore { price_list, gas: Rc::clone(&gas_tracker), @@ -127,7 +124,7 @@ where /// Update the state Cid of the Message receiver fn state_commit(&mut self, old_h: &Cid, new_h: Cid) -> Result<(), ActorError> { - let to_addr = *self.message().to(); + let to_addr = *self.message().receiver(); let mut actor = self .state .get_actor(&to_addr) @@ -154,8 +151,8 @@ where SYS: Syscalls, P: NetworkParams, { - fn message(&self) -> &UnsignedMessage { - &self.message + fn message(&self) -> &dyn MessageInfo { + self.message } fn curr_epoch(&self) -> ChainEpoch { self.epoch @@ -165,13 +162,13 @@ where where I: IntoIterator, { - let imm = self.resolve_address(self.message().from())?; + let imm = self.resolve_address(self.message().caller())?; // Check if theres is at least one match if !addresses.into_iter().any(|a| *a == imm) { return Err(self.abort( ExitCode::SysErrForbidden, - format!("caller is not one of {}", self.message().from()), + format!("caller is not one of {}", self.message().caller()), )); } Ok(()) @@ -182,7 +179,7 @@ where I: IntoIterator, { let caller_cid = self - .get_actor_code_cid(self.message().to())? + .get_actor_code_cid(self.message().receiver())? .ok_or_else(|| actor_error!(fatal("failed to lookup code cid for caller")))?; if types.into_iter().any(|c| *c == caller_cid) { return Err(self.abort( @@ -190,7 +187,7 @@ where format!( "caller cid type {} one of {}", caller_cid, - self.message().from() + self.message().caller() ), )); } @@ -243,7 +240,7 @@ where } fn state(&self) -> Result { - let actor = self.state.get_actor(self.message().to()).map_err(|e| actor_error!(SysErrorIllegalArgument; "failed to get actor for Readonly state: {}", e))?.ok_or_else(|| actor_error!(SysErrorIllegalArgument; "Actor readonly state does not exist"))?; + let actor = self.state.get_actor(self.message().receiver()).map_err(|e| actor_error!(SysErrorIllegalArgument; "failed to get actor for Readonly state: {}", e))?.ok_or_else(|| actor_error!(SysErrorIllegalArgument; "Actor readonly state does not exist"))?; self.store .get(&actor.state) .map_err(|e| { @@ -266,7 +263,7 @@ where F: FnOnce(&mut C, &mut Self) -> R, { // get actor - let act = self.state.get_actor(self.message().to()).map_err(|e| actor_error!(SysErrorIllegalActor; "failed to get actor for transaction: {}", e))?.ok_or_else(|| actor_error!(SysErrorIllegalActor; "actor state for transaction doesn't exist"))?; + let act = self.state.get_actor(self.message().receiver()).map_err(|e| actor_error!(SysErrorIllegalActor; "failed to get actor for transaction: {}", e))?.ok_or_else(|| actor_error!(SysErrorIllegalActor; "actor state for transaction doesn't exist"))?; // get state for actor based on generic C let mut state: C = self diff --git a/vm/runtime/src/lib.rs b/vm/runtime/src/lib.rs index 307080e1e404..ea0596f1288a 100644 --- a/vm/runtime/src/lib.rs +++ b/vm/runtime/src/lib.rs @@ -23,7 +23,7 @@ use filecoin_proofs_api::{ use forest_encoding::{blake2b_256, Cbor}; use ipld_blockstore::BlockStore; use log::warn; -use message::UnsignedMessage; +use message::Message; use rayon::prelude::*; use std::collections::BTreeMap; use std::collections::HashMap; @@ -35,7 +35,7 @@ use vm::{ActorError, ExitCode, MethodNum, Randomness, Serialized, TokenAmount}; /// this is everything that is accessible to actors, beyond parameters. pub trait Runtime { /// Information related to the current message being executed. - fn message(&self) -> &UnsignedMessage; + fn message(&self) -> &dyn MessageInfo; /// The current chain epoch number. The genesis block has epoch zero. fn curr_epoch(&self) -> ChainEpoch; @@ -134,14 +134,29 @@ pub trait Runtime { /// Message information available to the actor about executing message. pub trait MessageInfo { - // The address of the immediate calling actor. Always an ID-address. - fn caller(&self) -> Address; + /// The address of the immediate calling actor. Always an ID-address. + fn caller(&self) -> &Address; - // The address of the actor receiving the message. Always an ID-address. - fn receiver(&self) -> Address; + /// The address of the actor receiving the message. Always an ID-address. + fn receiver(&self) -> &Address; - // The value attached to the message being processed, implicitly added to current_balance() before method invocation. - fn value_received(&self) -> TokenAmount; + /// The value attached to the message being processed, implicitly added to current_balance() before method invocation. + fn value_received(&self) -> &TokenAmount; +} + +impl 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. From 5dceb2a3c68335882985f1870628890e918eec45 Mon Sep 17 00:00:00 2001 From: austinabell Date: Wed, 22 Jul 2020 12:03:47 -0400 Subject: [PATCH 09/23] Update runtime validations and refactor mock runtime usage --- vm/actor/src/builtin/account/mod.rs | 4 +-- vm/actor/src/builtin/init/mod.rs | 2 +- vm/actor/src/builtin/miner/mod.rs | 6 ++-- vm/actor/src/builtin/multisig/mod.rs | 12 ++++--- vm/actor/src/builtin/reward/mod.rs | 4 +-- vm/actor/src/builtin/system/mod.rs | 2 +- vm/actor/src/builtin/verifreg/mod.rs | 2 +- vm/actor/tests/account_actor_test.rs | 2 +- vm/actor/tests/common/mod.rs | 51 ++++++++++++++------------- vm/actor/tests/cron_actor_test.rs | 4 +-- vm/actor/tests/init_actor_test.rs | 2 +- vm/actor/tests/market_actor_test.rs | 15 ++++---- vm/actor/tests/paych_actor_test.rs | 44 +++++++++++------------ vm/actor/tests/reward_actor_test.rs | 4 +-- vm/interpreter/src/default_runtime.rs | 43 +++++++++++++--------- vm/runtime/src/lib.rs | 6 ++-- 16 files changed, 109 insertions(+), 94 deletions(-) diff --git a/vm/actor/src/builtin/account/mod.rs b/vm/actor/src/builtin/account/mod.rs index 9c3bfb3b8730..fe136ff77dfc 100644 --- a/vm/actor/src/builtin/account/mod.rs +++ b/vm/actor/src/builtin/account/mod.rs @@ -44,12 +44,12 @@ impl Actor { } // Fetches the pubkey-type address from this actor. - pub fn pubkey_address(rt: &RT) -> Result + pub fn pubkey_address(rt: &mut RT) -> Result where BS: BlockStore, RT: Runtime, { - rt.validate_immediate_caller_accept_any(); + rt.validate_immediate_caller_accept_any()?; let st: State = rt.state()?; Ok(st.address) } diff --git a/vm/actor/src/builtin/init/mod.rs b/vm/actor/src/builtin/init/mod.rs index c5e4e4510bac..81f3522d57f2 100644 --- a/vm/actor/src/builtin/init/mod.rs +++ b/vm/actor/src/builtin/init/mod.rs @@ -56,7 +56,7 @@ impl Actor { BS: BlockStore, RT: Runtime, { - rt.validate_immediate_caller_accept_any(); + rt.validate_immediate_caller_accept_any()?; let caller_code = rt .get_actor_code_cid(rt.message().caller())? .ok_or_else(|| actor_error!(ErrForbidden; "No code for actor"))?; diff --git a/vm/actor/src/builtin/miner/mod.rs b/vm/actor/src/builtin/miner/mod.rs index acaf1722e185..d7bd143a27e5 100644 --- a/vm/actor/src/builtin/miner/mod.rs +++ b/vm/actor/src/builtin/miner/mod.rs @@ -172,7 +172,7 @@ impl Actor { BS: BlockStore, RT: Runtime, { - rt.validate_immediate_caller_accept_any(); + rt.validate_immediate_caller_accept_any()?; let st: State = rt.state()?; Ok(GetControlAddressesReturn { owner: st.info.owner, @@ -581,7 +581,7 @@ impl Actor { BS: BlockStore, RT: Runtime, { - rt.validate_immediate_caller_accept_any(); + rt.validate_immediate_caller_accept_any()?; let sector_number = params.sector_number; let st: State = rt.state()?; @@ -782,7 +782,7 @@ impl Actor { BS: BlockStore, RT: Runtime, { - rt.validate_immediate_caller_accept_any(); + rt.validate_immediate_caller_accept_any()?; let st: State = rt.state()?; let sec = st.get_sector(rt.store(), params.sector_number); diff --git a/vm/actor/src/builtin/multisig/mod.rs b/vm/actor/src/builtin/multisig/mod.rs index ce72f2c04ae1..acd3aaea13db 100644 --- a/vm/actor/src/builtin/multisig/mod.rs +++ b/vm/actor/src/builtin/multisig/mod.rs @@ -186,7 +186,8 @@ impl Actor { BS: BlockStore, RT: Runtime, { - rt.validate_immediate_caller_is(std::iter::once(rt.message().receiver()))?; + let receiver = *rt.message().receiver(); + rt.validate_immediate_caller_is(std::iter::once(&receiver))?; rt.transaction::(|st, _| { // Check if signer to add is already signer @@ -213,7 +214,8 @@ impl Actor { BS: BlockStore, RT: Runtime, { - rt.validate_immediate_caller_is(std::iter::once(rt.message().receiver()))?; + let receiver = *rt.message().receiver(); + rt.validate_immediate_caller_is(std::iter::once(&receiver))?; rt.transaction::(|st, _| { // Check that signer to remove exists @@ -248,7 +250,8 @@ impl Actor { BS: BlockStore, RT: Runtime, { - rt.validate_immediate_caller_is(std::iter::once(rt.message().receiver()))?; + let receiver = *rt.message().receiver(); + rt.validate_immediate_caller_is(std::iter::once(&receiver))?; rt.transaction::(|st, _| { // Check that signer to remove exists @@ -286,7 +289,8 @@ impl Actor { BS: BlockStore, RT: Runtime, { - rt.validate_immediate_caller_is(std::iter::once(rt.message().receiver()))?; + let receiver = *rt.message().receiver(); + rt.validate_immediate_caller_is(std::iter::once(&receiver))?; rt.transaction::(|st, _| { // Check if valid threshold value diff --git a/vm/actor/src/builtin/reward/mod.rs b/vm/actor/src/builtin/reward/mod.rs index 8d5d5d15d990..aa99794f6b26 100644 --- a/vm/actor/src/builtin/reward/mod.rs +++ b/vm/actor/src/builtin/reward/mod.rs @@ -124,12 +124,12 @@ impl Actor { Ok(()) } - fn last_per_epoch_reward(rt: &RT) -> Result + fn last_per_epoch_reward(rt: &mut RT) -> Result where BS: BlockStore, RT: Runtime, { - rt.validate_immediate_caller_accept_any(); + rt.validate_immediate_caller_accept_any()?; let st: State = rt.state()?; Ok(st.last_per_epoch_reward) } diff --git a/vm/actor/src/builtin/system/mod.rs b/vm/actor/src/builtin/system/mod.rs index 278ea320b056..e34c7159b26b 100644 --- a/vm/actor/src/builtin/system/mod.rs +++ b/vm/actor/src/builtin/system/mod.rs @@ -20,7 +20,7 @@ pub enum Method { pub struct Actor; impl Actor { /// Init actor constructor - pub fn constructor(rt: &RT) -> Result<(), ActorError> + pub fn constructor(rt: &mut RT) -> Result<(), ActorError> where BS: BlockStore, RT: Runtime, diff --git a/vm/actor/src/builtin/verifreg/mod.rs b/vm/actor/src/builtin/verifreg/mod.rs index 30b4aad22f3b..2f60b4fe9d58 100644 --- a/vm/actor/src/builtin/verifreg/mod.rs +++ b/vm/actor/src/builtin/verifreg/mod.rs @@ -112,7 +112,7 @@ impl Actor { )); } - rt.validate_immediate_caller_accept_any(); + rt.validate_immediate_caller_accept_any()?; rt.transaction(|st: &mut State, rt| { // Validate caller is one of the verifiers. let verify_addr = rt.message().caller(); diff --git a/vm/actor/tests/account_actor_test.rs b/vm/actor/tests/account_actor_test.rs index 200d05cc738f..d49dddfec79e 100644 --- a/vm/actor/tests/account_actor_test.rs +++ b/vm/actor/tests/account_actor_test.rs @@ -21,7 +21,7 @@ macro_rules! account_tests { caller_type: SYSTEM_ACTOR_CODE_ID.clone(), ..Default::default() }; - rt.expect_validate_caller_addr(&[*SYSTEM_ACTOR_ADDR]); + rt.expect_validate_caller_addr(vec![*SYSTEM_ACTOR_ADDR]); if exit_code.is_success() { rt diff --git a/vm/actor/tests/common/mod.rs b/vm/actor/tests/common/mod.rs index a0ad0d35d72d..43b1b12d2de2 100644 --- a/vm/actor/tests/common/mod.rs +++ b/vm/actor/tests/common/mod.rs @@ -45,8 +45,8 @@ pub struct MockRuntime { // Expectations pub expect_validate_caller_any: Cell, - pub expect_validate_caller_addr: RefCell>>, - pub expect_validate_caller_type: RefCell>>, + pub expect_validate_caller_addr: Option>, + pub expect_validate_caller_type: Option>, pub expect_sends: VecDeque, pub expect_create_actor: Option, pub expect_verify_sigs: RefCell>, @@ -172,9 +172,9 @@ impl MockRuntime { .unwrap(); Ok(data) } - pub fn expect_validate_caller_addr(&self, addr: &[Address]) { + pub fn expect_validate_caller_addr(&mut self, addr: Vec
) { assert!(addr.len() > 0, "addrs must be non-empty"); - *self.expect_validate_caller_addr.borrow_mut() = Some(addr.to_vec()); + self.expect_validate_caller_addr = Some(addr); } #[allow(dead_code)] @@ -207,9 +207,9 @@ impl MockRuntime { } #[allow(dead_code)] - pub fn expect_validate_caller_type(&self, types: &[Cid]) { + pub fn expect_validate_caller_type(&mut self, types: Vec) { assert!(types.len() > 0, "addrs must be non-empty"); - *self.expect_validate_caller_type.borrow_mut() = Some(types.to_vec()); + self.expect_validate_caller_type = Some(types); } #[allow(dead_code)] @@ -278,14 +278,14 @@ impl MockRuntime { "expected ValidateCallerAny, not received" ); assert!( - self.expect_validate_caller_addr.borrow().as_ref().is_none(), + self.expect_validate_caller_addr.is_none(), "expected ValidateCallerAddr {:?}, not received", - self.expect_validate_caller_addr.borrow().as_ref().unwrap() + self.expect_validate_caller_addr ); assert!( - self.expect_validate_caller_type.borrow().as_ref().is_none(), + self.expect_validate_caller_type.is_none(), "expected ValidateCallerType {:?}, not received", - self.expect_validate_caller_type.borrow().as_ref().unwrap() + self.expect_validate_caller_type ); assert!( self.expect_sends.is_empty(), @@ -314,15 +314,15 @@ impl MockRuntime { .borrow() .as_ref() .is_none(), - "expect_compute_unsealed_sector_cid not received", + "expect_verify_consensus_fault not received", ); self.reset(); } pub fn reset(&mut self) { self.expect_validate_caller_any.set(false); - *self.expect_validate_caller_addr.borrow_mut() = None; - *self.expect_validate_caller_type.borrow_mut() = None; + self.expect_validate_caller_addr = None; + self.expect_validate_caller_type = None; self.expect_create_actor = None; self.expect_verify_sigs.borrow_mut().clear(); *self.expect_verify_seal.borrow_mut() = None; @@ -405,16 +405,17 @@ impl Runtime for MockRuntime { self.epoch } - fn validate_immediate_caller_accept_any(&self) { + fn validate_immediate_caller_accept_any(&mut self) -> Result<(), ActorError> { self.require_in_call(); assert!( self.expect_validate_caller_any.get(), "unexpected validate-caller-any" ); self.expect_validate_caller_any.set(false); + Ok(()) } - fn validate_immediate_caller_is<'a, I>(&self, addresses: I) -> Result<(), ActorError> + fn validate_immediate_caller_is<'a, I>(&mut self, addresses: I) -> Result<(), ActorError> where I: IntoIterator, { @@ -425,23 +426,23 @@ impl Runtime for MockRuntime { self.check_argument(addrs.len() > 0, "addrs must be non-empty".to_owned())?; assert!( - self.expect_validate_caller_addr.borrow().is_some(), + self.expect_validate_caller_addr.is_some(), "unexpected validate caller addrs" ); assert!( - &addrs == self.expect_validate_caller_addr.borrow().as_ref().unwrap(), + &addrs == self.expect_validate_caller_addr.as_ref().unwrap(), "unexpected validate caller addrs {:?}, expected {:?}", addrs, - self.expect_validate_caller_addr.borrow().as_ref() + self.expect_validate_caller_addr ); for expected in &addrs { if self.message().caller() == expected { - *self.expect_validate_caller_addr.borrow_mut() = None; + self.expect_validate_caller_addr = None; return Ok(()); } } - *self.expect_validate_caller_addr.borrow_mut() = None; + self.expect_validate_caller_addr = None; return Err(ActorError::new( ExitCode::ErrForbidden, format!( @@ -451,7 +452,7 @@ impl Runtime for MockRuntime { ), )); } - fn validate_immediate_caller_type<'a, I>(&self, types: I) -> Result<(), ActorError> + fn validate_immediate_caller_type<'a, I>(&mut self, types: I) -> Result<(), ActorError> where I: IntoIterator, { @@ -461,11 +462,11 @@ impl Runtime for MockRuntime { self.check_argument(types.len() > 0, "types must be non-empty".to_owned())?; assert!( - self.expect_validate_caller_type.borrow().is_some(), + self.expect_validate_caller_type.is_some(), "unexpected validate caller code" ); assert!( - &types == self.expect_validate_caller_type.borrow().as_ref().unwrap(), + &types == self.expect_validate_caller_type.as_ref().unwrap(), "unexpected validate caller code {:?}, expected {:?}", types, self.expect_validate_caller_type @@ -473,12 +474,12 @@ impl Runtime for MockRuntime { for expected in &types { if &self.caller_type == expected { - *self.expect_validate_caller_type.borrow_mut() = None; + self.expect_validate_caller_type = None; return Ok(()); } } - *self.expect_validate_caller_type.borrow_mut() = None; + self.expect_validate_caller_type = None; Err(self.abort( ExitCode::ErrForbidden, diff --git a/vm/actor/tests/cron_actor_test.rs b/vm/actor/tests/cron_actor_test.rs index 460c94e2ea76..7989954775ba 100644 --- a/vm/actor/tests/cron_actor_test.rs +++ b/vm/actor/tests/cron_actor_test.rs @@ -137,7 +137,7 @@ fn epoch_tick_with_entries() { } fn construct_and_verify(rt: &mut MockRuntime, params: &ConstructorParams) { - rt.expect_validate_caller_addr(&[*SYSTEM_ACTOR_ADDR]); + rt.expect_validate_caller_addr(vec![*SYSTEM_ACTOR_ADDR]); let ret = rt .call( &*CRON_ACTOR_CODE_ID, @@ -150,7 +150,7 @@ fn construct_and_verify(rt: &mut MockRuntime, params: &ConstructorParams) { } fn epoch_tick_and_verify(rt: &mut MockRuntime) { - rt.expect_validate_caller_addr(&[*SYSTEM_ACTOR_ADDR]); + rt.expect_validate_caller_addr(vec![*SYSTEM_ACTOR_ADDR]); let ret = rt .call(&*CRON_ACTOR_CODE_ID, 2, &Serialized::default()) .unwrap(); diff --git a/vm/actor/tests/init_actor_test.rs b/vm/actor/tests/init_actor_test.rs index ded33e66a325..f14f8116de03 100644 --- a/vm/actor/tests/init_actor_test.rs +++ b/vm/actor/tests/init_actor_test.rs @@ -243,7 +243,7 @@ fn sending_constructor_failure() { } fn construct_and_verify(rt: &mut MockRuntime) { - rt.expect_validate_caller_addr(&[SYSTEM_ACTOR_ADDR.clone()]); + rt.expect_validate_caller_addr(vec![SYSTEM_ACTOR_ADDR.clone()]); let params = ConstructorParams { network_name: "mock".to_string(), }; diff --git a/vm/actor/tests/market_actor_test.rs b/vm/actor/tests/market_actor_test.rs index b2a6adfd1abc..3218eb6c4f8c 100644 --- a/vm/actor/tests/market_actor_test.rs +++ b/vm/actor/tests/market_actor_test.rs @@ -51,7 +51,7 @@ fn simple_construction() { ..Default::default() }; - rt.expect_validate_caller_addr(&[SYSTEM_ACTOR_ADDR.clone()]); + rt.expect_validate_caller_addr(vec![SYSTEM_ACTOR_ADDR.clone()]); assert_eq!( Serialized::default(), @@ -164,9 +164,8 @@ fn add_non_provider_funds() { rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), caller_addr); let amount = TokenAmount::from(test_case.0 as u64); - //rt.balance = rt.balance + amount.clone(); rt.set_value(amount); - rt.expect_validate_caller_type(&CALLER_TYPES_SIGNABLE.clone()); + rt.expect_validate_caller_type(CALLER_TYPES_SIGNABLE.to_vec()); assert!(rt .call( @@ -272,7 +271,7 @@ fn withdraw_non_provider() { ); rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), client_addr.clone()); - rt.expect_validate_caller_type(&[ + rt.expect_validate_caller_type(vec![ ACCOUNT_ACTOR_CODE_ID.clone(), MULTISIG_ACTOR_CODE_ID.clone(), ]); @@ -323,7 +322,7 @@ fn client_withdraw_more_than_available() { add_participant_funds(&mut rt, client_addr.clone(), amount.clone()); rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), client_addr.clone()); - rt.expect_validate_caller_type(&[ + rt.expect_validate_caller_type(vec![ ACCOUNT_ACTOR_CODE_ID.clone(), MULTISIG_ACTOR_CODE_ID.clone(), ]); @@ -434,7 +433,7 @@ fn expect_provider_control_address( owner: Address, worker: Address, ) { - rt.expect_validate_caller_addr(&[owner.clone(), worker.clone()]); + rt.expect_validate_caller_addr(vec![owner.clone(), worker.clone()]); let return_value = GetControlAddressesReturn { owner: owner.clone(), @@ -481,7 +480,7 @@ fn add_participant_funds(rt: &mut MockRuntime, addr: Address, amount: TokenAmoun rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), addr.clone()); - rt.expect_validate_caller_type(&[ + rt.expect_validate_caller_type(vec![ ACCOUNT_ACTOR_CODE_ID.clone(), MULTISIG_ACTOR_CODE_ID.clone(), ]); @@ -500,7 +499,7 @@ fn add_participant_funds(rt: &mut MockRuntime, addr: Address, amount: TokenAmoun } fn construct_and_verify(rt: &mut MockRuntime) { - rt.expect_validate_caller_addr(&[SYSTEM_ACTOR_ADDR.clone()]); + rt.expect_validate_caller_addr(vec![SYSTEM_ACTOR_ADDR.clone()]); assert_eq!( Serialized::default(), rt.call( diff --git a/vm/actor/tests/paych_actor_test.rs b/vm/actor/tests/paych_actor_test.rs index ac10cdb7eeef..37000784a15b 100644 --- a/vm/actor/tests/paych_actor_test.rs +++ b/vm/actor/tests/paych_actor_test.rs @@ -80,7 +80,7 @@ mod paych_constructor { #[test] fn actor_doesnt_exist_test() { let mut rt = construct_runtime(); - rt.expect_validate_caller_type(&[INIT_ACTOR_CODE_ID.clone()]); + rt.expect_validate_caller_type(vec![INIT_ACTOR_CODE_ID.clone()]); let params = ConstructorParams { to: Address::new_id(TEST_PAYCH_ADDR), from: Address::new_id(TEST_PAYER_ADDR), @@ -137,7 +137,7 @@ mod paych_constructor { ..Default::default() }; - rt.expect_validate_caller_type(&[INIT_ACTOR_CODE_ID.clone()]); + rt.expect_validate_caller_type(vec![INIT_ACTOR_CODE_ID.clone()]); let params = ConstructorParams { to: test_case.paych_addr, from: Address::new_id(10001), @@ -290,7 +290,7 @@ mod create_lane_tests { let ucp = UpdateChannelStateParams::from(sv.clone()); rt.set_caller(test_case.target_code, payee_addr); - rt.expect_validate_caller_addr(&[payer_addr, payee_addr]); + rt.expect_validate_caller_addr(vec![payer_addr, payee_addr]); if test_case.sig.is_some() && test_case.secret_preimage.len() == 0 { let exp_exit_code = if !test_case.verify_sig { @@ -343,7 +343,7 @@ mod update_channel_state_redeem { let payee_addr = Address::new_id(R_PAYEE_ADDR); rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), payee_addr); - rt.expect_validate_caller_addr(&[state.from, state.to]); + rt.expect_validate_caller_addr(vec![state.from, state.to]); sv.amount = BigInt::from(9); @@ -386,7 +386,7 @@ mod update_channel_state_redeem { let payee_addr = Address::new_id(R_PAYEE_ADDR); rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), payee_addr); - rt.expect_validate_caller_addr(&[state.from, state.to]); + rt.expect_validate_caller_addr(vec![state.from, state.to]); let initial_amount = state.to_send; sv.amount = BigInt::from(9); @@ -428,7 +428,7 @@ mod merge_tests { let (mut rt, sv) = require_create_cannel_with_lanes(num_lanes); let state: PState = rt.get_state().unwrap(); rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), state.from.clone()); - rt.expect_validate_caller_addr(&[state.from, state.to]); + rt.expect_validate_caller_addr(vec![state.from, state.to]); (rt, sv, state) } @@ -600,7 +600,7 @@ mod update_channel_state_extra { let other_addr = Address::new_id(OTHER_ADDR); let fake_params = [1, 2, 3, 4]; rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), state.from); - rt.expect_validate_caller_addr(&[state.from, state.to]); + rt.expect_validate_caller_addr(vec![state.from, state.to]); sv.extra = Some(ModVerifyParams { actor: other_addr, @@ -659,7 +659,7 @@ mod update_channel_state_settling { let (mut rt, sv) = require_create_cannel_with_lanes(1); rt.epoch = 10; let state: PState = rt.get_state().unwrap(); - rt.expect_validate_caller_addr(&[state.from, state.to]); + rt.expect_validate_caller_addr(vec![state.from, state.to]); rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), state.from); is_ok(&mut rt, Method::Settle as u64, &Serialized::default()); @@ -693,7 +693,7 @@ mod update_channel_state_settling { for tc in test_cases { let mut ucp = UpdateChannelStateParams::from(sv.clone()); ucp.sv.min_settle_height = tc.min_settle; - rt.expect_validate_caller_addr(&[state.from, state.to]); + rt.expect_validate_caller_addr(vec![state.from, state.to]); rt.expect_verify_signature(ExpectedVerifySig { sig: sv.clone().signature.unwrap(), signer: state.to, @@ -717,7 +717,7 @@ mod secret_preimage { fn succeed_correct_secret() { let (mut rt, sv) = require_create_cannel_with_lanes(1); let state: PState = rt.get_state().unwrap(); - rt.expect_validate_caller_addr(&[state.from, state.to]); + rt.expect_validate_caller_addr(vec![state.from, state.to]); let ucp = UpdateChannelStateParams::from(sv.clone()); @@ -742,7 +742,7 @@ mod secret_preimage { let (mut rt, sv) = require_create_cannel_with_lanes(1); let state: PState = rt.get_state().unwrap(); - rt.expect_validate_caller_addr(&[state.from, state.to]); + rt.expect_validate_caller_addr(vec![state.from, state.to]); let mut ucp = UpdateChannelStateParams { proof: vec![], @@ -779,7 +779,7 @@ mod actor_settle { rt.epoch = EP; let mut state: PState = rt.get_state().unwrap(); rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), state.from); - rt.expect_validate_caller_addr(&[state.from, state.to]); + rt.expect_validate_caller_addr(vec![state.from, state.to]); is_ok(&mut rt, Method::Settle as u64, &Serialized::default()); @@ -795,10 +795,10 @@ mod actor_settle { rt.epoch = EP; let state: PState = rt.get_state().unwrap(); rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), state.from); - rt.expect_validate_caller_addr(&[state.from, state.to]); + rt.expect_validate_caller_addr(vec![state.from, state.to]); is_ok(&mut rt, Method::Settle as u64, &Serialized::default()); - rt.expect_validate_caller_addr(&[state.from, state.to]); + rt.expect_validate_caller_addr(vec![state.from, state.to]); expect_error( &mut rt, Method::Settle as u64, @@ -816,7 +816,7 @@ mod actor_settle { sv.min_settle_height = (EP + SETTLE_DELAY) + 1; let ucp = UpdateChannelStateParams::from(sv.clone()); - rt.expect_validate_caller_addr(&[state.from, state.to]); + rt.expect_validate_caller_addr(vec![state.from, state.to]); rt.expect_verify_signature(ExpectedVerifySig { sig: ucp.sv.clone().signature.unwrap(), signer: state.to, @@ -832,7 +832,7 @@ mod actor_settle { assert_eq!(state.settling_at, 0); assert_eq!(state.min_settle_height, ucp.sv.min_settle_height); rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), state.from); - rt.expect_validate_caller_addr(&[state.from, state.to]); + rt.expect_validate_caller_addr(vec![state.from, state.to]); is_ok(&mut rt, Method::Settle as u64, &Serialized::default()); state = rt.get_state().unwrap(); assert_eq!(state.settling_at, ucp.sv.min_settle_height); @@ -861,7 +861,7 @@ mod actor_collect { Serialized::default(), exit_codes[1], ); - rt.expect_validate_caller_addr(&[state.from, state.to]); + rt.expect_validate_caller_addr(vec![state.from, state.to]); } #[test] @@ -870,12 +870,12 @@ mod actor_collect { rt.epoch = 10; let mut state: PState = rt.get_state().unwrap(); rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), state.from); - rt.expect_validate_caller_addr(&[state.from, state.to]); + rt.expect_validate_caller_addr(vec![state.from, state.to]); is_ok(&mut rt, Method::Settle as u64, &Serialized::default()); state = rt.get_state().unwrap(); assert_eq!(state.settling_at, 11); - rt.expect_validate_caller_addr(&[state.from, state.to]); + rt.expect_validate_caller_addr(vec![state.from, state.to]); exp_send_multiple(&mut rt, &state, [ExitCode::Ok, ExitCode::Ok]); @@ -923,7 +923,7 @@ mod actor_collect { if !tc.dont_settle { rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), state.from); - rt.expect_validate_caller_addr(&[state.from, state.to]); + rt.expect_validate_caller_addr(vec![state.from, state.to]); is_ok(&mut rt, Method::Settle as u64, &Serialized::default()); state = rt.get_state().unwrap(); assert_eq!(state.settling_at, 11); @@ -998,7 +998,7 @@ fn require_add_new_lane(rt: &mut MockRuntime, param: LaneParams) -> SignedVouche ..SignedVoucher::default() }; rt.set_caller(ACCOUNT_ACTOR_CODE_ID.clone(), param.from); - rt.expect_validate_caller_addr(&[param.from, param.to]); + rt.expect_validate_caller_addr(vec![param.from, param.to]); rt.expect_verify_signature(ExpectedVerifySig { sig: sig.clone(), signer: payee_addr, @@ -1027,7 +1027,7 @@ fn construct_and_verify(rt: &mut MockRuntime, sender: Address, receiver: Address from: sender, to: receiver, }; - rt.expect_validate_caller_type(&[INIT_ACTOR_CODE_ID.clone()]); + rt.expect_validate_caller_type(vec![INIT_ACTOR_CODE_ID.clone()]); is_ok( rt, METHOD_CONSTRUCTOR, diff --git a/vm/actor/tests/reward_actor_test.rs b/vm/actor/tests/reward_actor_test.rs index e23321b99abe..b9d0b99dedbc 100644 --- a/vm/actor/tests/reward_actor_test.rs +++ b/vm/actor/tests/reward_actor_test.rs @@ -29,7 +29,7 @@ fn balance_less_than_reward() { let miner = Address::new_id(1000); let gas_reward = TokenAmount::from(10u8); - rt.expect_validate_caller_addr(&[*SYSTEM_ACTOR_ADDR]); + rt.expect_validate_caller_addr(vec![*SYSTEM_ACTOR_ADDR]); let params = AwardBlockRewardParams { miner: miner, @@ -49,7 +49,7 @@ fn balance_less_than_reward() { } fn construct_and_verify(rt: &mut MockRuntime) { - rt.expect_validate_caller_addr(&[SYSTEM_ACTOR_ADDR.clone()]); + rt.expect_validate_caller_addr(vec![SYSTEM_ACTOR_ADDR.clone()]); let ret = rt .call( &*REWARD_ACTOR_CODE_ID, diff --git a/vm/interpreter/src/default_runtime.rs b/vm/interpreter/src/default_runtime.rs index 4c7b7f58f9dd..ae6e7bc1900d 100644 --- a/vm/interpreter/src/default_runtime.rs +++ b/vm/interpreter/src/default_runtime.rs @@ -40,6 +40,7 @@ pub struct DefaultRuntime<'db, 'msg, 'st, 'sys, 'r, BS, SYS, P> { num_actors_created: u64, price_list: PriceList, rand: &'r ChainRand, + caller_validated: bool, params: PhantomData

, } @@ -87,6 +88,7 @@ where num_actors_created, price_list, rand, + caller_validated: false, params: PhantomData, } } @@ -143,6 +145,16 @@ where Ok(()) } + + fn abort_if_already_validated(&mut self) -> Result<(), ActorError> { + if self.caller_validated { + Err(actor_error!(SysErrorIllegalActor; + "Method must validate caller identity exactly once")) + } else { + self.caller_validated = true; + Ok(()) + } + } } impl Runtime for DefaultRuntime<'_, '_, '_, '_, '_, BS, SYS, P> @@ -157,39 +169,38 @@ where fn curr_epoch(&self) -> ChainEpoch { self.epoch } - fn validate_immediate_caller_accept_any(&self) {} - fn validate_immediate_caller_is<'db, I>(&self, addresses: I) -> Result<(), ActorError> + fn validate_immediate_caller_accept_any(&mut self) -> Result<(), ActorError> { + self.abort_if_already_validated() + } + fn validate_immediate_caller_is<'db, I>(&mut self, addresses: I) -> Result<(), ActorError> where I: IntoIterator, { + self.abort_if_already_validated()?; + let imm = self.resolve_address(self.message().caller())?; // Check if theres is at least one match if !addresses.into_iter().any(|a| *a == imm) { - return Err(self.abort( - ExitCode::SysErrForbidden, - format!("caller is not one of {}", self.message().caller()), + return Err(actor_error!(SysErrForbidden; + "caller {} is not one of supported", self.message().caller() )); } Ok(()) } - fn validate_immediate_caller_type<'db, I>(&self, types: I) -> Result<(), ActorError> + fn validate_immediate_caller_type<'db, I>(&mut self, types: I) -> Result<(), ActorError> where I: IntoIterator, { + self.abort_if_already_validated()?; + let caller_cid = self - .get_actor_code_cid(self.message().receiver())? + .get_actor_code_cid(self.message().caller())? .ok_or_else(|| actor_error!(fatal("failed to lookup code cid for caller")))?; - if types.into_iter().any(|c| *c == caller_cid) { - return Err(self.abort( - ExitCode::SysErrForbidden, - format!( - "caller cid type {} one of {}", - caller_cid, - self.message().caller() - ), - )); + if !types.into_iter().any(|c| *c == caller_cid) { + return Err(actor_error!(SysErrForbidden; + "caller cid type {} not one of supported", caller_cid)); } Ok(()) } diff --git a/vm/runtime/src/lib.rs b/vm/runtime/src/lib.rs index ea0596f1288a..e03da393b97f 100644 --- a/vm/runtime/src/lib.rs +++ b/vm/runtime/src/lib.rs @@ -42,11 +42,11 @@ pub trait Runtime { /// Validates the caller against some predicate. /// Exported actor methods must invoke at least one caller validation before returning. - fn validate_immediate_caller_accept_any(&self); - fn validate_immediate_caller_is<'a, I>(&self, addresses: I) -> Result<(), ActorError> + fn validate_immediate_caller_accept_any(&mut self) -> Result<(), ActorError>; + fn validate_immediate_caller_is<'a, I>(&mut self, addresses: I) -> Result<(), ActorError> where I: IntoIterator; - fn validate_immediate_caller_type<'a, I>(&self, types: I) -> Result<(), ActorError> + fn validate_immediate_caller_type<'a, I>(&mut self, types: I) -> Result<(), ActorError> where I: IntoIterator; From be1ca6282856793454506d78cca8975c3e7c2d74 Mon Sep 17 00:00:00 2001 From: austinabell Date: Wed, 22 Jul 2020 12:17:53 -0400 Subject: [PATCH 10/23] Update error handling in mock runtime --- vm/actor/tests/common/mod.rs | 150 +++++++++++------------------------ 1 file changed, 48 insertions(+), 102 deletions(-) diff --git a/vm/actor/tests/common/mod.rs b/vm/actor/tests/common/mod.rs index 43b1b12d2de2..d504906e85dd 100644 --- a/vm/actor/tests/common/mod.rs +++ b/vm/actor/tests/common/mod.rs @@ -18,7 +18,7 @@ use runtime::{ActorCode, ConsensusFault, MessageInfo, Runtime, Syscalls}; use std::cell::{Cell, RefCell}; use std::collections::{HashMap, VecDeque}; use std::error::Error as StdError; -use vm::{ActorError, ExitCode, MethodNum, Randomness, Serialized, TokenAmount}; +use vm::{actor_error, ActorError, ExitCode, MethodNum, Randomness, Serialized, TokenAmount}; pub struct MockRuntime { pub epoch: ChainEpoch, @@ -152,7 +152,7 @@ impl MockRuntime { } fn check_argument(&self, predicate: bool, msg: String) -> Result<(), ActorError> { if !predicate { - return Err(ActorError::new(ExitCode::SysErrorIllegalArgument, msg)); + return Err(actor_error!(SysErrorIllegalArgument; msg)); } Ok(()) } @@ -260,10 +260,7 @@ impl MockRuntime { x if x == &*VERIFIED_ACTOR_CODE_ID => { actor::verifreg::Actor.invoke_method(self, method_num, params) } - _ => Err(ActorError::new( - ExitCode::SysErrForbidden, - "invalid method id".to_owned(), - )), + _ => Err(actor_error!(SysErrForbidden; "invalid method id")), }; if res.is_err() { @@ -443,13 +440,9 @@ impl Runtime for MockRuntime { } } self.expect_validate_caller_addr = None; - return Err(ActorError::new( - ExitCode::ErrForbidden, - format!( + return Err(actor_error!(ErrForbidden; "caller address {:?} forbidden, allowed: {:?}", - self.message().caller(), - &addrs - ), + self.message().caller(), &addrs )); } fn validate_immediate_caller_type<'a, I>(&mut self, types: I) -> Result<(), ActorError> @@ -481,13 +474,10 @@ impl Runtime for MockRuntime { self.expect_validate_caller_type = None; - Err(self.abort( - ExitCode::ErrForbidden, - format!( - "caller type {:?} forbidden, allowed: {:?}", - self.caller_type, types - ), - )) + Err( + actor_error!(ErrForbidden; "caller type {:?} forbidden, allowed: {:?}", + self.caller_type, types), + ) } fn current_balance(&self) -> Result { @@ -504,10 +494,7 @@ impl Runtime for MockRuntime { self.id_addresses .get(&address) .cloned() - .ok_or(ActorError::new( - ExitCode::ErrIllegalArgument, - "Address not found".to_string(), - )) + .ok_or(actor_error!(ErrIllegalArgument; "Address not found")) } fn get_actor_code_cid(&self, addr: &Address) -> Result, ActorError> { @@ -527,10 +514,7 @@ impl Runtime for MockRuntime { fn create(&mut self, obj: &C) -> Result<(), ActorError> { if self.state.is_some() == true { - return Err(self.abort( - ExitCode::SysErrorIllegalActor, - "state already constructed".to_owned(), - )); + return Err(actor_error!(SysErrorIllegalActor; "state already constructed")); } self.state = Some(self.store.put(obj, Blake2b256).unwrap()); Ok(()) @@ -572,10 +556,7 @@ impl Runtime for MockRuntime { ) -> Result { self.require_in_call(); if self.in_transaction { - return Err(self.abort( - ExitCode::SysErrorIllegalActor, - "side-effect within transaction", - )); + return Err(actor_error!(SysErrorIllegalActor; "side-effect within transaction")); } assert!( @@ -592,12 +573,9 @@ impl Runtime for MockRuntime { assert!(&expected_msg.to == to && expected_msg.method == method && &expected_msg.params == params && &expected_msg.value == value, "expectedMessage being sent does not match expectation.\nMessage -\t to: {:?} method: {:?} value: {:?} params: {:?}\nExpected -\t {:?}", to, method, value, params, self.expect_sends[0]); if value > &self.balance { - return Err(self.abort( - ExitCode::SysErrSenderStateInvalid, - format!( + return Err(actor_error!(SysErrSenderStateInvalid; "cannot send value: {:?} exceeds balance: {:?}", value, self.balance - ), )); } self.balance -= value; @@ -628,10 +606,7 @@ impl Runtime for MockRuntime { fn create_actor(&mut self, code_id: &Cid, address: &Address) -> Result<(), ActorError> { self.require_in_call(); if self.in_transaction { - return Err(self.abort( - ExitCode::SysErrorIllegalActor, - "side-effect within transaction".to_owned(), - )); + return Err(actor_error!(SysErrorIllegalActor; "side-effect within transaction")); } let expect_create_actor = self .expect_create_actor @@ -645,10 +620,7 @@ impl Runtime for MockRuntime { fn delete_actor(&mut self, _beneficiary: &Address) -> Result<(), ActorError> { self.require_in_call(); if self.in_transaction { - return Err(self.abort( - ExitCode::SysErrorIllegalActor, - "side-effect within transaction".to_owned(), - )); + return Err(actor_error!(SysErrorIllegalActor; "side-effect within transaction")); } todo!("implement me???") } @@ -670,19 +642,15 @@ impl Syscalls for MockRuntime { plaintext: &[u8], ) -> Result<(), Box> { if self.expect_verify_sigs.borrow().len() == 0 { - return Err(Box::new(ActorError::new( - ExitCode::ErrIllegalState, - "Unexpected signature verification".to_string(), - ))); + return Err(Box::new( + actor_error!(ErrIllegalState; "Unexpected signature verification"), + )); } let exp = self .expect_verify_sigs .borrow_mut() .pop() - .ok_or(ActorError::new( - ExitCode::ErrIllegalState, - "Unexpected signature verification".to_string(), - ))?; + .ok_or(actor_error!(ErrIllegalState; "Unexpected signature verification"))?; if exp.sig == *signature && exp.signer == *signer && &exp.plaintext[..] == plaintext { if exp.result == ExitCode::Ok { return Ok(()); @@ -693,10 +661,9 @@ impl Syscalls for MockRuntime { ))); } } else { - return Err(Box::new(ActorError::new( - ExitCode::ErrIllegalState, - "Signatures did not match".to_string(), - ))); + return Err(Box::new( + actor_error!(ErrIllegalState; "Signatures did not match"), + )); } } @@ -711,22 +678,19 @@ impl Syscalls for MockRuntime { let exp = self .expect_compute_unsealed_sector_cid .replace(None) - .ok_or(Box::new(ActorError::new( - ExitCode::ErrIllegalState, - "Unexpected syscall to ComputeUnsealedSectorCID".to_string(), + .ok_or(Box::new(actor_error!(ErrIllegalState; + "Unexpected syscall to ComputeUnsealedSectorCID" )))?; if exp.reg != reg { - return Err(Box::new(ActorError::new( - ExitCode::ErrIllegalState, - "Unexpected compute_unsealed_sector_cid : reg mismatch".to_string(), + return Err(Box::new(actor_error!(ErrIllegalState; + "Unexpected compute_unsealed_sector_cid : reg mismatch" ))); } if exp.pieces[..].eq(pieces) { - return Err(Box::new(ActorError::new( - ExitCode::ErrIllegalState, - "Unexpected compute_unsealed_sector_cid : pieces mismatch".to_string(), + return Err(Box::new(actor_error!(ErrIllegalState; + "Unexpected compute_unsealed_sector_cid : pieces mismatch" ))); } @@ -739,19 +703,14 @@ impl Syscalls for MockRuntime { Ok(exp.cid) } fn verify_seal(&self, seal: &SealVerifyInfo) -> Result<(), Box> { - let exp = self - .expect_verify_seal - .replace(None) - .ok_or(Box::new(ActorError::new( - ExitCode::ErrIllegalState, - "Unexpected syscall to verify seal".to_string(), - )))?; + let exp = self.expect_verify_seal.replace(None).ok_or(Box::new( + actor_error!(ErrIllegalState; "Unexpected syscall to verify seal"), + ))?; if exp.seal != *seal { - return Err(Box::new(ActorError::new( - ExitCode::ErrIllegalState, - "Unexpected seal verification".to_string(), - ))); + return Err(Box::new( + actor_error!(ErrIllegalState; "Unexpected seal verification"), + )); } if exp.exit_code != ExitCode::Ok { return Err(Box::new(ActorError::new( @@ -762,19 +721,14 @@ impl Syscalls for MockRuntime { Ok(()) } fn verify_post(&self, post: &WindowPoStVerifyInfo) -> Result<(), Box> { - let exp = self - .expect_verify_post - .replace(None) - .ok_or(Box::new(ActorError::new( - ExitCode::ErrIllegalState, - "Unexpected syscall to verify PoSt ".to_string(), - )))?; + let exp = self.expect_verify_post.replace(None).ok_or(Box::new( + actor_error!(ErrIllegalState; "Unexpected syscall to verify PoSt"), + ))?; if exp.post != *post { - return Err(Box::new(ActorError::new( - ExitCode::ErrIllegalState, - "Unexpected PoSt verification".to_string(), - ))); + return Err(Box::new( + actor_error!(ErrIllegalState; "Unexpected PoSt verification"), + )); } if exp.exit_code != ExitCode::Ok { return Err(Box::new(ActorError::new( @@ -793,28 +747,20 @@ impl Syscalls for MockRuntime { let exp = self .expect_verify_consensus_fault .replace(None) - .ok_or(Box::new(ActorError::new( - ExitCode::ErrIllegalState, - "Unexpected syscall to verify_consensus_fault".to_string(), - )))?; + .ok_or(Box::new( + actor_error!(ErrIllegalState; "Unexpected syscall to verify_consensus_fault"), + ))?; if exp.require_correct_input { if exp.block_header_1 != h1 { - return Err(Box::new(ActorError::new( - ExitCode::ErrIllegalState, - "Header 1 mismatch".to_string(), - ))); + return Err(Box::new(actor_error!(ErrIllegalState; "Header 1 mismatch"))); } if exp.block_header_2 != h2 { - return Err(Box::new(ActorError::new( - ExitCode::ErrIllegalState, - "Header 2 mismatch".to_string(), - ))); + return Err(Box::new(actor_error!(ErrIllegalState; "Header 2 mismatch"))); } if exp.block_header_extra != extra { - return Err(Box::new(ActorError::new( - ExitCode::ErrIllegalState, - "Header extra mismatch".to_string(), - ))); + return Err(Box::new( + actor_error!(ErrIllegalState; "Header extra mismatch"), + )); } } if exp.exit_code != ExitCode::Ok { From 7261273988dc1ba02ae01a92b6995a5e4b905117 Mon Sep 17 00:00:00 2001 From: austinabell Date: Thu, 23 Jul 2020 09:11:39 -0400 Subject: [PATCH 11/23] Update runtime function signatures --- blockchain/state_manager/src/lib.rs | 6 ++++-- vm/actor/src/builtin/init/state.rs | 12 +++++------- vm/actor/src/builtin/market/mod.rs | 19 ++++++++++--------- vm/actor/src/builtin/miner/mod.rs | 10 +++++----- vm/actor/src/builtin/paych/mod.rs | 4 +++- vm/actor/src/builtin/power/mod.rs | 3 ++- vm/actor/src/builtin/reward/mod.rs | 10 +++++----- vm/actor/tests/common/mod.rs | 9 +++------ vm/actor/tests/init_actor_test.rs | 22 ++++++++++++---------- vm/interpreter/src/default_runtime.rs | 6 +++--- vm/runtime/src/lib.rs | 18 ++++++++++-------- vm/state_tree/src/lib.rs | 18 ++++++++++++------ 12 files changed, 74 insertions(+), 63 deletions(-) diff --git a/blockchain/state_manager/src/lib.rs b/blockchain/state_manager/src/lib.rs index d8820753e087..04b670298ea0 100644 --- a/blockchain/state_manager/src/lib.rs +++ b/blockchain/state_manager/src/lib.rs @@ -247,7 +247,7 @@ where } } - pub fn lookup_id(&self, addr: &Address, ts: &Tipset) -> Result { + pub fn lookup_id(&self, addr: &Address, ts: &Tipset) -> Result, Error> { let state_tree = StateTree::new_from_root(self.bs.as_ref(), ts.parent_state())?; state_tree.lookup_id(addr).map_err(Error::State) } @@ -256,7 +256,9 @@ where let market_state: market::State = self.load_actor_state(&*STORAGE_MARKET_ACTOR_ADDR, ts.parent_state())?; - let new_addr = self.lookup_id(addr, ts)?; + let new_addr = self + .lookup_id(addr, ts)? + .ok_or_else(|| Error::State(format!("Failed to resolve address {}", addr)))?; let out = MarketBalance { escrow: { diff --git a/vm/actor/src/builtin/init/state.rs b/vm/actor/src/builtin/init/state.rs index 1ac2ca4004b3..e35c6dba320a 100644 --- a/vm/actor/src/builtin/init/state.rs +++ b/vm/actor/src/builtin/init/state.rs @@ -55,19 +55,17 @@ impl State { &self, store: &BS, addr: &Address, - ) -> Result { + ) -> Result, String> { if addr.protocol() == Protocol::ID { - return Ok(*addr); + return Ok(Some(*addr)); } let map: Hamt = Hamt::load_with_bit_width(&self.address_map, store, HAMT_BIT_WIDTH)?; - let actor_id: ActorID = map - .get(&addr.to_bytes())? - .ok_or_else(|| "Address not found".to_owned())?; - - Ok(Address::new_id(actor_id)) + Ok(map + .get::<_, ActorID>(&addr.to_bytes())? + .map(Address::new_id)) } } diff --git a/vm/actor/src/builtin/market/mod.rs b/vm/actor/src/builtin/market/mod.rs index 3a7f4c4c93df..d54e0c79ce0b 100644 --- a/vm/actor/src/builtin/market/mod.rs +++ b/vm/actor/src/builtin/market/mod.rs @@ -190,8 +190,10 @@ impl Actor { } // All deals should have the same provider so get worker once - let provider_raw = ¶ms.deals[0].proposal.provider; - let provider = rt.resolve_address(&provider_raw)?; + let provider_raw = params.deals[0].proposal.provider; + let provider = rt.resolve_address(&provider_raw)?.ok_or_else( + || actor_error!(ErrNotFound; "failed to resolve provider address {}", provider_raw), + )?; let (_, worker) = request_miner_control_addrs(rt, &provider)?; if &worker != rt.message().caller() { @@ -219,10 +221,6 @@ impl Actor { } } - // All deals should have the same provider so get worker once - let provider_raw = params.deals[0].proposal.provider; - let provider = rt.resolve_address(&provider_raw)?; - let mut new_deal_ids: Vec = Vec::new(); rt.transaction(|st: &mut State, rt| { let mut prop = Amt::load(&st.proposals, rt.store()) @@ -241,7 +239,10 @@ impl Actor { )); } - let client = rt.resolve_address(&deal.proposal.client)?; + let client = rt.resolve_address(&deal.proposal.client)?.ok_or_else( + || actor_error!(ErrNotFound; + "failed to resolve provider address {}", provider_raw), + )?; // Normalise provider and client addresses in the proposal stored on chain (after signature verification). deal.proposal.provider = provider; deal.proposal.client = client; @@ -769,8 +770,8 @@ where RT: Runtime, { // Resolve the provided address to the canonical form against which the balance is held. - let nominal = rt.resolve_address(addr).map_err( - |e| actor_error!(ErrIllegalArgument; "Failed to resolve address provided: {}", e), + let nominal = rt.resolve_address(addr)?.ok_or_else( + || actor_error!(ErrIllegalArgument; "failed to resolve address {}", addr), )?; let code_id = rt diff --git a/vm/actor/src/builtin/miner/mod.rs b/vm/actor/src/builtin/miner/mod.rs index d7bd143a27e5..a88f33f5ed30 100644 --- a/vm/actor/src/builtin/miner/mod.rs +++ b/vm/actor/src/builtin/miner/mod.rs @@ -2266,8 +2266,8 @@ where RT: Runtime, { let resolved = rt - .resolve_address(&raw) - .map_err(|_| actor_error!(ErrIllegalArgument; "unable to resolve address {}", raw))?; + .resolve_address(&raw)? + .ok_or_else(|| actor_error!(ErrIllegalArgument; "unable to resolve address: {}", raw))?; assert!(resolved.protocol() == Protocol::ID); let owner_code = rt @@ -2289,9 +2289,9 @@ where BS: BlockStore, RT: Runtime, { - let resolved = rt.resolve_address(&raw).map_err( - |e| actor_error!(ErrIllegalArgument; "unable to resolve address: {},{}", raw, e), - )?; + let resolved = rt + .resolve_address(&raw)? + .ok_or_else(|| actor_error!(ErrIllegalArgument; "unable to resolve address: {}", raw))?; assert!(resolved.protocol() == Protocol::ID); let owner_code = rt diff --git a/vm/actor/src/builtin/paych/mod.rs b/vm/actor/src/builtin/paych/mod.rs index 2f5cf78e00cb..ff11ff520eaf 100644 --- a/vm/actor/src/builtin/paych/mod.rs +++ b/vm/actor/src/builtin/paych/mod.rs @@ -62,8 +62,10 @@ impl Actor { RT: Runtime, { let resolved = rt + // TODO: fatal error not handled here. To match go this will have to be refactored .resolve_address(raw) - .map_err(|e| format!("failed to resolve address {}", e))?; + .map_err(|e| e.to_string())? + .ok_or_else(|| format!("failed to resolve address {}", raw))?; let code_cid = rt .get_actor_code_cid(&resolved) diff --git a/vm/actor/src/builtin/power/mod.rs b/vm/actor/src/builtin/power/mod.rs index 1033c1942a5f..34e657aecaa3 100644 --- a/vm/actor/src/builtin/power/mod.rs +++ b/vm/actor/src/builtin/power/mod.rs @@ -110,7 +110,8 @@ impl Actor { BS: BlockStore, RT: Runtime, { - let nominal = rt.resolve_address(¶ms.miner)?; + // TODO this function does not exist anymore, make sure it is removed/replaced later + let nominal = rt.resolve_address(¶ms.miner)?.unwrap(); let st: State = rt.state()?; diff --git a/vm/actor/src/builtin/reward/mod.rs b/vm/actor/src/builtin/reward/mod.rs index aa99794f6b26..2d764a35f0e5 100644 --- a/vm/actor/src/builtin/reward/mod.rs +++ b/vm/actor/src/builtin/reward/mod.rs @@ -20,7 +20,8 @@ use num_derive::FromPrimitive; use num_traits::FromPrimitive; use runtime::{ActorCode, Runtime}; use vm::{ - ActorError, ExitCode, MethodNum, Serialized, TokenAmount, METHOD_CONSTRUCTOR, METHOD_SEND, + actor_error, ActorError, ExitCode, MethodNum, Serialized, TokenAmount, METHOD_CONSTRUCTOR, + METHOD_SEND, }; // * Updated to specs-actors commit: 52599b21919df07f44d7e61cc028e265ec18f700 @@ -84,10 +85,9 @@ impl Actor { "cannot give block reward for zero tickets" ); - let miner_addr = rt - .resolve_address(¶ms.miner) - // TODO revisit later if all address resolutions end up being the same exit code - .map_err(|e| ActorError::new(ExitCode::ErrIllegalState, e.msg().to_string()))?; + let miner_addr = rt.resolve_address(¶ms.miner)?.ok_or_else( + || actor_error!(ErrIllegalState; "failed to resolve given owner address"), + )?; let prior_balance = rt.current_balance()?; diff --git a/vm/actor/tests/common/mod.rs b/vm/actor/tests/common/mod.rs index d504906e85dd..027c6fc3ee6a 100644 --- a/vm/actor/tests/common/mod.rs +++ b/vm/actor/tests/common/mod.rs @@ -485,16 +485,13 @@ impl Runtime for MockRuntime { Ok(self.balance.clone()) } - fn resolve_address(&self, address: &Address) -> Result { + fn resolve_address(&self, address: &Address) -> Result, ActorError> { self.require_in_call(); if address.protocol() == address::Protocol::ID { - return Ok(address.clone()); + return Ok(Some(address.clone())); } - self.id_addresses - .get(&address) - .cloned() - .ok_or(actor_error!(ErrIllegalArgument; "Address not found")) + Ok(self.id_addresses.get(&address).cloned()) } fn get_actor_code_cid(&self, addr: &Address) -> Result, ActorError> { diff --git a/vm/actor/tests/init_actor_test.rs b/vm/actor/tests/init_actor_test.rs index f14f8116de03..7d81276ffa32 100644 --- a/vm/actor/tests/init_actor_test.rs +++ b/vm/actor/tests/init_actor_test.rs @@ -89,7 +89,8 @@ fn create_2_payment_channels() { let state: State = rt.get_state().unwrap(); let returned_address = state .resolve_address(&rt.store, &unique_address) - .expect("Address should have been found"); + .expect("Resolve should not error") + .expect("Address should be able to be resolved"); assert_eq!(returned_address, expected_id_addr, "Wrong Address returned"); } @@ -135,14 +136,18 @@ fn create_storage_miner() { let state: State = rt.get_state().unwrap(); let returned_address = state .resolve_address(&rt.store, &unique_address) - .expect("Address should have been found"); + .expect("Resolve should not error") + .expect("Address should be able to be resolved"); assert_eq!(expected_id_addr, returned_address); // Should return error since the address of flurbo is unknown let unknown_addr = Address::new_actor(b"flurbo"); - state - .resolve_address(&rt.store, &unknown_addr) - .expect_err("Address should have not been found"); + + let returned_address = state.resolve_address(&rt.store, &unknown_addr).unwrap(); + assert_eq!( + returned_address, None, + "Addresses should have not been found" + ); } #[test] @@ -232,12 +237,9 @@ fn sending_constructor_failure() { let state: State = rt.get_state().unwrap(); - let returned_address = state - .resolve_address(&rt.store, &unique_address) - .expect_err("Address resolution should have failed"); - + let returned_address = state.resolve_address(&rt.store, &unique_address).unwrap(); assert_eq!( - returned_address, "Address not found", + returned_address, None, "Addresses should have not been found" ); } diff --git a/vm/interpreter/src/default_runtime.rs b/vm/interpreter/src/default_runtime.rs index ae6e7bc1900d..ece20e88309c 100644 --- a/vm/interpreter/src/default_runtime.rs +++ b/vm/interpreter/src/default_runtime.rs @@ -178,10 +178,10 @@ where { self.abort_if_already_validated()?; - let imm = self.resolve_address(self.message().caller())?; + let imm = self.message().caller(); // Check if theres is at least one match - if !addresses.into_iter().any(|a| *a == imm) { + if !addresses.into_iter().any(|a| a == imm) { return Err(actor_error!(SysErrForbidden; "caller {} is not one of supported", self.message().caller() )); @@ -209,7 +209,7 @@ where self.get_balance(self.message.to()) } - fn resolve_address(&self, address: &Address) -> Result { + fn resolve_address(&self, address: &Address) -> Result, ActorError> { self.state .lookup_id(&address) .map_err(ActorError::new_fatal) diff --git a/vm/runtime/src/lib.rs b/vm/runtime/src/lib.rs index e03da393b97f..e6b72e4d1364 100644 --- a/vm/runtime/src/lib.rs +++ b/vm/runtime/src/lib.rs @@ -56,7 +56,7 @@ pub trait Runtime { /// Resolves an address of any protocol to an ID address (via the Init actor's table). /// This allows resolution of externally-provided SECP, BLS, or actor addresses to the canonical form. /// If the argument is an ID address it is returned directly. - fn resolve_address(&self, address: &Address) -> Result; + fn resolve_address(&self, address: &Address) -> Result, ActorError>; /// Look up the code ID at an actor address. fn get_actor_code_cid(&self, addr: &Address) -> Result, ActorError>; @@ -96,8 +96,8 @@ pub trait Runtime { fn store(&self) -> &BS; /// Sends a message to another actor, returning the exit code and return value envelope. - /// If the invoked method does not return successfully, its state changes (and that of any messages it sent in turn) - /// will be rolled back. + /// If the invoked method does not return successfully, its state changes + /// (and that of any messages it sent in turn) will be rolled back. fn send( &mut self, to: &Address, @@ -106,9 +106,9 @@ 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. + /// 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; @@ -118,7 +118,8 @@ pub trait Runtime { /// Always an ActorExec address. fn new_actor_address(&mut self) -> Result; - /// Creates an actor with code `codeID` and address `address`, with empty state. May only be called by Init actor. + /// 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>; /// Deletes the executing actor from the state tree, transferring any balance to beneficiary. @@ -140,7 +141,8 @@ pub trait MessageInfo { /// The address of the actor receiving the message. Always an ID-address. fn receiver(&self) -> &Address; - /// The value attached to the message being processed, implicitly added to current_balance() before method invocation. + /// The value attached to the message being processed, implicitly + /// added to current_balance() before method invocation. fn value_received(&self) -> &TokenAmount; } diff --git a/vm/state_tree/src/lib.rs b/vm/state_tree/src/lib.rs index d920835e1d94..d3aed1409700 100644 --- a/vm/state_tree/src/lib.rs +++ b/vm/state_tree/src/lib.rs @@ -49,7 +49,9 @@ where /// Get actor state from an address. Will be resolved to ID address. pub fn get_actor(&self, addr: &Address) -> Result, String> { - let addr = self.lookup_id(addr)?; + let addr = self + .lookup_id(addr)? + .ok_or_else(|| format!("Resolution lookup failed for {}", addr))?; // Check cache for actor state if let Some(actor_state) = self.actor_cache.read().get(&addr) { @@ -69,7 +71,9 @@ where /// Set actor state for an address. Will set state at ID address. pub fn set_actor(&mut self, addr: &Address, actor: ActorState) -> Result<(), String> { - let addr = self.lookup_id(addr)?; + let addr = self + .lookup_id(addr)? + .ok_or_else(|| format!("Resolution lookup failed for {}", addr))?; // Set actor state in cache if let Some(act) = self.actor_cache.write().insert(addr, actor.clone()) { @@ -88,9 +92,9 @@ where } /// Get an ID address from any Address - pub fn lookup_id(&self, addr: &Address) -> Result { + pub fn lookup_id(&self, addr: &Address) -> Result, String> { if addr.protocol() == Protocol::ID { - return Ok(*addr); + return Ok(Some(*addr)); } let init_act = self @@ -104,12 +108,14 @@ where .map_err(|e| e.to_string())? .ok_or("Could not resolve init actor state")?; - Ok(state.resolve_address(self.store(), addr)?) + state.resolve_address(self.store(), addr) } /// Delete actor for an address. Will resolve to ID address to delete. pub fn delete_actor(&mut self, addr: &Address) -> Result<(), String> { - let addr = self.lookup_id(addr)?; + let addr = self + .lookup_id(addr)? + .ok_or_else(|| format!("Resolution lookup failed for {}", addr))?; // Remove value from cache self.actor_cache.write().remove(&addr); From 0839434fcf67432f5e7d697c4305f9f647eb92f9 Mon Sep 17 00:00:00 2001 From: austinabell Date: Thu, 23 Jul 2020 11:49:49 -0400 Subject: [PATCH 12/23] Update empty array CID and bytes references and refactor more methods --- Cargo.lock | 1 + vm/Cargo.toml | 1 + vm/actor/src/builtin/market/mod.rs | 14 ++++++------ vm/interpreter/src/default_runtime.rs | 32 +++++++++++++++------------ vm/src/lib.rs | 27 ++++++++++++++++++++++ vm/src/method.rs | 5 ++++- 6 files changed, 58 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6beea6903335..4dfb3858d7db 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2523,6 +2523,7 @@ dependencies = [ "forest_bigint", "forest_cid", "forest_encoding", + "lazy_static", "num-derive", "num-traits 0.2.12", "serde", diff --git a/vm/Cargo.toml b/vm/Cargo.toml index d69a3a3b159a..93c76dfda35e 100644 --- a/vm/Cargo.toml +++ b/vm/Cargo.toml @@ -19,6 +19,7 @@ cid = { package = "forest_cid", path = "../ipld/cid", version = "0.1", features num-traits = "0.2" num-derive = "0.3.0" thiserror = "1.0.11" +lazy_static = "1.4" [features] json = [] \ No newline at end of file diff --git a/vm/actor/src/builtin/market/mod.rs b/vm/actor/src/builtin/market/mod.rs index d54e0c79ce0b..d401dbdb6e36 100644 --- a/vm/actor/src/builtin/market/mod.rs +++ b/vm/actor/src/builtin/market/mod.rs @@ -239,10 +239,10 @@ impl Actor { )); } - let client = rt.resolve_address(&deal.proposal.client)?.ok_or_else( - || actor_error!(ErrNotFound; - "failed to resolve provider address {}", provider_raw), - )?; + let client = rt.resolve_address(&deal.proposal.client)?.ok_or_else(|| { + actor_error!(ErrNotFound; + "failed to resolve provider address {}", provider_raw) + })?; // Normalise provider and client addresses in the proposal stored on chain (after signature verification). deal.proposal.provider = provider; deal.proposal.client = client; @@ -770,9 +770,9 @@ where RT: Runtime, { // Resolve the provided address to the canonical form against which the balance is held. - let nominal = rt.resolve_address(addr)?.ok_or_else( - || actor_error!(ErrIllegalArgument; "failed to resolve address {}", addr), - )?; + let nominal = rt + .resolve_address(addr)? + .ok_or_else(|| actor_error!(ErrIllegalArgument; "failed to resolve address {}", addr))?; let code_id = rt .get_actor_code_cid(&nominal)? diff --git a/vm/interpreter/src/default_runtime.rs b/vm/interpreter/src/default_runtime.rs index ece20e88309c..bf87b808651a 100644 --- a/vm/interpreter/src/default_runtime.rs +++ b/vm/interpreter/src/default_runtime.rs @@ -12,8 +12,8 @@ use cid::{multihash::Blake2b256, Cid}; use clock::ChainEpoch; use crypto::DomainSeparationTag; use fil_types::NetworkParams; -use forest_encoding::to_vec; use forest_encoding::Cbor; +use forest_encoding::{error::Error as EncodingError, to_vec}; use ipld_blockstore::BlockStore; use message::{Message, UnsignedMessage}; use num_bigint::BigInt; @@ -24,7 +24,7 @@ use std::marker::PhantomData; use std::rc::Rc; use vm::{ actor_error, ActorError, ActorState, ExitCode, MethodNum, Randomness, Serialized, TokenAmount, - METHOD_SEND, + EMPTY_ARR_CID, METHOD_SEND, }; /// Implementation of the Runtime trait. @@ -232,26 +232,30 @@ where let r = self .rand .get_randomness(&self.store, personalization, rand_epoch, entropy) - .map_err(|e| { - ActorError::new_fatal(format!("could not get randomness: {}", e.to_string())) - })?; + .map_err(|e| actor_error!(fatal("could not get randomness: {}", e.to_string())))?; Ok(Randomness(r)) } fn create(&mut self, obj: &C) -> Result<(), ActorError> { - let c = self.store.put(obj, Blake2b256).map_err(|e| { - self.abort( - ExitCode::ErrPlaceholder, - format!("storage put in create: {}", e.to_string()), - ) - })?; - // TODO: This is almost certainly wrong. Need to CBOR an empty slice and calculate Cid - self.state_commit(&Cid::default(), c) + let c = + self.store + .put(obj, Blake2b256) + .map_err(|e| match e.downcast::() { + Ok(ser_error) => actor_error!(ErrSerialization; + "failed to marshal cbor object {}", ser_error), + Err(other) => actor_error!(fatal("failed to put cbor object: {}", other)), + })?; + + self.state_commit(&EMPTY_ARR_CID, c) } fn state(&self) -> Result { - let actor = self.state.get_actor(self.message().receiver()).map_err(|e| actor_error!(SysErrorIllegalArgument; "failed to get actor for Readonly state: {}", e))?.ok_or_else(|| actor_error!(SysErrorIllegalArgument; "Actor readonly state does not exist"))?; + let actor = self.state.get_actor(self.message().receiver()) + .map_err(|e| actor_error!(SysErrorIllegalArgument; + "failed to get actor for Readonly state: {}", e))? + .ok_or_else(|| actor_error!(SysErrorIllegalArgument; + "Actor readonly state does not exist"))?; self.store .get(&actor.state) .map_err(|e| { diff --git a/vm/src/lib.rs b/vm/src/lib.rs index 513ad1038728..9566b38c5bbb 100644 --- a/vm/src/lib.rs +++ b/vm/src/lib.rs @@ -23,3 +23,30 @@ pub use self::invoc::*; pub use self::method::*; pub use self::randomness::*; pub use self::token::*; + +#[macro_use] +extern crate lazy_static; +use cid::{multihash::Blake2b256, Cid}; +use encoding::to_vec; + +lazy_static! { + /// Cbor bytes of an empty array serialized. + pub static ref EMPTY_ARR_BYTES: Vec = to_vec::<[(); 0]>(&[]).unwrap(); + + /// Cid of the empty array Cbor bytes (`EMPTY_ARR_BYTES`). + pub static ref EMPTY_ARR_CID: Cid = Cid::new_from_cbor(&EMPTY_ARR_BYTES, Blake2b256); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn empty_object_checks() { + assert_eq!(&*EMPTY_ARR_BYTES, &[128u8]); + assert_eq!( + EMPTY_ARR_CID.to_string(), + "bafy2bzacebc3bt6cedhoyw34drrmjvazhu4oj25er2ebk4u445pzycvq4ta4a" + ); + } +} diff --git a/vm/src/method.rs b/vm/src/method.rs index eff5ca8d576d..31b54714fb35 100644 --- a/vm/src/method.rs +++ b/vm/src/method.rs @@ -1,6 +1,7 @@ // Copyright 2020 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT +use super::EMPTY_ARR_BYTES; use encoding::{de, from_slice, ser, serde_bytes, to_vec, Cbor, Error as EncodingError}; use serde::{Deserialize, Serialize}; use std::ops::Deref; @@ -25,7 +26,9 @@ impl Default for Serialized { /// Default serialized bytes is an empty array serialized #[inline] fn default() -> Self { - Self::serialize::<[u8; 0]>([]).unwrap() + Self { + bytes: EMPTY_ARR_BYTES.clone(), + } } } From a07fa497f0f6771ae5fefebf2f5e80d8e4d7998a Mon Sep 17 00:00:00 2001 From: austinabell Date: Thu, 23 Jul 2020 12:25:59 -0400 Subject: [PATCH 13/23] Update state handles --- vm/interpreter/src/default_runtime.rs | 99 ++++++++++++++------------- 1 file changed, 53 insertions(+), 46 deletions(-) diff --git a/vm/interpreter/src/default_runtime.rs b/vm/interpreter/src/default_runtime.rs index bf87b808651a..490637e9fd2d 100644 --- a/vm/interpreter/src/default_runtime.rs +++ b/vm/interpreter/src/default_runtime.rs @@ -155,6 +155,34 @@ where Ok(()) } } + + /// Helper function for inserting into blockstore. + fn put(&self, obj: &T) -> Result + where + T: Cbor, + { + self.store + .put(obj, Blake2b256) + .map_err(|e| match e.downcast::() { + Ok(ser_error) => actor_error!(ErrSerialization; + "failed to marshal cbor object {}", ser_error), + Err(other) => actor_error!(fatal("failed to put cbor object: {}", other)), + }) + } + + /// Helper function for getting deserializable objects from blockstore. + fn get(&self, cid: &Cid) -> Result, ActorError> + where + T: Cbor, + { + self.store + .get(cid) + .map_err(|e| match e.downcast::() { + Ok(ser_error) => actor_error!(ErrSerialization; + "failed to unmarshal cbor object {}", ser_error), + Err(other) => actor_error!(fatal("failed to get cbor object: {}", other)), + }) + } } impl Runtime for DefaultRuntime<'_, '_, '_, '_, '_, BS, SYS, P> @@ -238,38 +266,30 @@ where } fn create(&mut self, obj: &C) -> Result<(), ActorError> { - let c = - self.store - .put(obj, Blake2b256) - .map_err(|e| match e.downcast::() { - Ok(ser_error) => actor_error!(ErrSerialization; - "failed to marshal cbor object {}", ser_error), - Err(other) => actor_error!(fatal("failed to put cbor object: {}", other)), - })?; + let c = self.put(obj)?; self.state_commit(&EMPTY_ARR_CID, c) } fn state(&self) -> Result { - let actor = self.state.get_actor(self.message().receiver()) - .map_err(|e| actor_error!(SysErrorIllegalArgument; - "failed to get actor for Readonly state: {}", e))? - .ok_or_else(|| actor_error!(SysErrorIllegalArgument; - "Actor readonly state does not exist"))?; - self.store - .get(&actor.state) + let actor = self + .state + .get_actor(self.message().receiver()) .map_err(|e| { - self.abort( - ExitCode::ErrPlaceholder, - format!("storage get error in read only state: {}", e.to_string()), - ) + actor_error!(SysErrorIllegalArgument; + "failed to get actor for Readonly state: {}", e) })? - .ok_or_else(|| { - self.abort( - ExitCode::ErrPlaceholder, - "storage get error in read only state".to_owned(), - ) - }) + .ok_or_else( + || actor_error!(SysErrorIllegalArgument; "Actor readonly state does not exist"), + )?; + + // TODO revisit as the go impl doesn't handle not exists and nil cases + self.get(&actor.state)?.ok_or_else(|| { + actor_error!(fatal( + "State does not exist for actor state cid: {}", + actor.state + )) + }) } fn transaction(&mut self, f: F) -> Result @@ -278,34 +298,21 @@ where F: FnOnce(&mut C, &mut Self) -> R, { // get actor - let act = self.state.get_actor(self.message().receiver()).map_err(|e| actor_error!(SysErrorIllegalActor; "failed to get actor for transaction: {}", e))?.ok_or_else(|| actor_error!(SysErrorIllegalActor; "actor state for transaction doesn't exist"))?; + let act = self.state.get_actor(self.message().receiver()) + .map_err(|e| actor_error!(SysErrorIllegalActor; "failed to get actor for transaction: {}", e))? + .ok_or_else(|| actor_error!(SysErrorIllegalActor; + "actor state for transaction doesn't exist"))?; // get state for actor based on generic C + // TODO Lotus is not handling the not exist case, revisit let mut state: C = self - .store - .get(&act.state) - .map_err(|e| { - self.abort( - ExitCode::ErrPlaceholder, - format!("storage get error in transaction: {}", e.to_string()), - ) - })? - .ok_or_else(|| { - self.abort( - ExitCode::ErrPlaceholder, - "storage get error in transaction".to_owned(), - ) - })?; + .get(&act.state)? + .ok_or_else(|| actor_error!(fatal("Actor state does not exist: {}", act.state)))?; // Update the state let r = f(&mut state, self); - let c = self.store.put(&state, Blake2b256).map_err(|e| { - self.abort( - ExitCode::ErrPlaceholder, - format!("storage put in create: {}", e.to_string()), - ) - })?; + let c = self.put(&state)?; // Committing that change self.state_commit(&act.state, c)?; From 98dc1b5a59d389296331244c3e004cf8d7eebd54 Mon Sep 17 00:00:00 2001 From: austinabell Date: Thu, 23 Jul 2020 14:32:13 -0400 Subject: [PATCH 14/23] Add allow internal flag in runtime --- vm/interpreter/src/default_runtime.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/vm/interpreter/src/default_runtime.rs b/vm/interpreter/src/default_runtime.rs index 490637e9fd2d..45e738bc1308 100644 --- a/vm/interpreter/src/default_runtime.rs +++ b/vm/interpreter/src/default_runtime.rs @@ -41,6 +41,7 @@ pub struct DefaultRuntime<'db, 'msg, 'st, 'sys, 'r, BS, SYS, P> { price_list: PriceList, rand: &'r ChainRand, caller_validated: bool, + allow_internal: bool, params: PhantomData

, } @@ -88,6 +89,7 @@ where num_actors_created, price_list, rand, + allow_internal: true, caller_validated: false, params: PhantomData, } @@ -310,7 +312,9 @@ where .ok_or_else(|| actor_error!(fatal("Actor state does not exist: {}", act.state)))?; // Update the state + self.allow_internal = false; let r = f(&mut state, self); + self.allow_internal = true; let c = self.put(&state)?; @@ -330,6 +334,10 @@ where params: &Serialized, value: &TokenAmount, ) -> Result { + if !self.allow_internal { + return Err(actor_error!(SysErrorIllegalActor; "runtime.send() is not allowed")); + } + let msg = UnsignedMessage::builder() .to(*to) .from(*self.message.from()) From 6e011ed069ecdfd66166e8aa6527d568d7e4bfa7 Mon Sep 17 00:00:00 2001 From: austinabell Date: Fri, 24 Jul 2020 09:28:28 -0400 Subject: [PATCH 15/23] Refactor internal send functions --- vm/actor/src/builtin/cron/mod.rs | 6 +- vm/actor/src/builtin/init/mod.rs | 6 +- vm/actor/src/builtin/market/mod.rs | 34 ++-- vm/actor/src/builtin/miner/mod.rs | 119 ++++++----- vm/actor/src/builtin/multisig/mod.rs | 2 +- vm/actor/src/builtin/paych/mod.rs | 10 +- vm/actor/src/builtin/power/mod.rs | 16 +- vm/actor/src/builtin/reward/mod.rs | 12 +- vm/actor/src/builtin/shared.rs | 6 +- vm/actor/tests/common/mod.rs | 10 +- vm/interpreter/src/default_runtime.rs | 278 +++++++++++++++----------- vm/interpreter/src/gas_tracker/mod.rs | 10 +- vm/interpreter/src/vm.rs | 11 +- vm/interpreter/tests/transfer_test.rs | 4 +- vm/runtime/src/lib.rs | 6 +- 15 files changed, 285 insertions(+), 245 deletions(-) diff --git a/vm/actor/src/builtin/cron/mod.rs b/vm/actor/src/builtin/cron/mod.rs index 4f874394139d..d8a0db5419ca 100644 --- a/vm/actor/src/builtin/cron/mod.rs +++ b/vm/actor/src/builtin/cron/mod.rs @@ -57,10 +57,10 @@ impl Actor { let st: State = rt.state()?; for entry in st.entries { let _v = rt.send( - &entry.receiver, + entry.receiver, entry.method_num, - &Serialized::default(), - &TokenAmount::from(0u8), + Serialized::default(), + TokenAmount::from(0u8), ); } Ok(()) diff --git a/vm/actor/src/builtin/init/mod.rs b/vm/actor/src/builtin/init/mod.rs index 81f3522d57f2..bd95b327e960 100644 --- a/vm/actor/src/builtin/init/mod.rs +++ b/vm/actor/src/builtin/init/mod.rs @@ -87,10 +87,10 @@ impl Actor { // Invoke constructor rt.send( - &id_address, + id_address, METHOD_CONSTRUCTOR, - ¶ms.constructor_params, - &rt.message().value_received().clone(), + params.constructor_params, + rt.message().value_received().clone(), ) .map_err(|err| rt.abort(err.exit_code(), "constructor failed"))?; diff --git a/vm/actor/src/builtin/market/mod.rs b/vm/actor/src/builtin/market/mod.rs index d401dbdb6e36..d4be278448e6 100644 --- a/vm/actor/src/builtin/market/mod.rs +++ b/vm/actor/src/builtin/market/mod.rs @@ -155,17 +155,17 @@ impl Actor { // TODO this will never be hit if amount_slashed_total > BigInt::zero() { rt.send( - &*BURNT_FUNDS_ACTOR_ADDR, + *BURNT_FUNDS_ACTOR_ADDR, METHOD_SEND, - &Serialized::default(), - &amount_slashed_total, + Serialized::default(), + amount_slashed_total, )?; } rt.send( - &recipient, + recipient, METHOD_SEND, - &Serialized::default(), - &amount_extracted, + Serialized::default(), + amount_extracted, )?; Ok(()) } @@ -195,7 +195,7 @@ impl Actor { || actor_error!(ErrNotFound; "failed to resolve provider address {}", provider_raw), )?; - let (_, worker) = request_miner_control_addrs(rt, &provider)?; + let (_, worker) = request_miner_control_addrs(rt, provider)?; if &worker != rt.message().caller() { return Err(ActorError::new( ExitCode::ErrForbidden, @@ -213,10 +213,10 @@ impl Actor { deal_size: BigInt::from(deal.proposal.piece_size.0), })?; rt.send( - &*VERIFIED_REGISTRY_ACTOR_ADDR, + *VERIFIED_REGISTRY_ACTOR_ADDR, VerifregMethod::UseBytes as u64, - &ser_params, - &TokenAmount::zero(), + ser_params, + TokenAmount::zero(), )?; } } @@ -617,18 +617,18 @@ impl Actor { deal_size: BigInt::from(d.piece_size.0), })?; rt.send( - &*VERIFIED_REGISTRY_ACTOR_ADDR, + *VERIFIED_REGISTRY_ACTOR_ADDR, VerifregMethod::RestoreBytes as u64, - &ser_params, - &TokenAmount::zero(), + ser_params, + TokenAmount::zero(), )?; } rt.send( - &*BURNT_FUNDS_ACTOR_ADDR, + *BURNT_FUNDS_ACTOR_ADDR, METHOD_SEND, - &Serialized::default(), - &amount_slashed, + Serialized::default(), + amount_slashed, )?; Ok(()) } @@ -785,7 +785,7 @@ where } // Storage miner actor entry; implied funds recipient is the associated owner address. - let (owner_addr, worker_addr) = request_miner_control_addrs(rt, &nominal)?; +let (owner_addr, worker_addr) = request_miner_control_addrs(rt, nominal)?; rt.validate_immediate_caller_is([owner_addr, worker_addr].iter())?; Ok((nominal, owner_addr)) } diff --git a/vm/actor/src/builtin/miner/mod.rs b/vm/actor/src/builtin/miner/mod.rs index a88f33f5ed30..861691c15a29 100644 --- a/vm/actor/src/builtin/miner/mod.rs +++ b/vm/actor/src/builtin/miner/mod.rs @@ -402,7 +402,7 @@ impl Actor { })??; // Remove power for new faults, and burn penalties. request_begin_faults(rt, sec_size, &detected_faults_sector)?; - burn_funds_and_notify_pledge_change(rt, &penalty)?; + burn_funds_and_notify_pledge_change(rt, penalty)?; // restore power for recovered sectors if !recovered_sectors.is_empty() { @@ -641,10 +641,10 @@ impl Actor { )?; rt.send( - &*STORAGE_POWER_ACTOR_ADDR, + *STORAGE_POWER_ACTOR_ADDR, PowerMethod::SubmitPoRepForBulkVerify as u64, - &Serialized::serialize(&svi)?, - &BigInt::zero(), + Serialized::serialize(&svi)?, + BigInt::zero(), )?; Ok(()) @@ -687,10 +687,10 @@ impl Actor { // TODO revisit spec TODOs let mut ret = rt.send( - &*STORAGE_MARKET_ACTOR_ADDR, + *STORAGE_MARKET_ACTOR_ADDR, MarketMethod::VerifyDealsOnSectorProveCommit as u64, - &ser_params, - &TokenAmount::zero(), + ser_params, + TokenAmount::zero(), )?; let deal_weights: VerifyDealsOnSectorProveCommitReturn = ret.deserialize()?; @@ -705,10 +705,10 @@ impl Actor { }, })?; ret = rt.send( - &*STORAGE_POWER_ACTOR_ADDR, + *STORAGE_POWER_ACTOR_ADDR, PowerMethod::OnSectorProveCommit as u64, - ¶m, - &TokenAmount::zero(), + param, + TokenAmount::zero(), )?; let BigIntDe(initial_pledge) = ret.deserialize()?; @@ -845,10 +845,10 @@ impl Actor { })?; rt.send( - &*STORAGE_POWER_ACTOR_ADDR, + *STORAGE_POWER_ACTOR_ADDR, PowerMethod::OnSectorModifyWeightDesc as u64, - &ser_params, - &BigInt::zero(), + ser_params, + BigInt::zero(), )?; // store new sector expiry @@ -1051,7 +1051,7 @@ impl Actor { // remove power for new faulty sectors detected_fault_sectors.append(&mut declared_fault_sectors); request_begin_faults(rt, sector_size, &detected_fault_sectors)?; - burn_funds_and_notify_pledge_change(rt, &penalty)?; + burn_funds_and_notify_pledge_change(rt, penalty)?; Ok(()) } @@ -1150,7 +1150,7 @@ impl Actor { // remove power for new faulty sectors request_begin_faults(rt, sector_size, &detected_fault_sectors)?; - burn_funds_and_notify_pledge_change(rt, &penalty)?; + burn_funds_and_notify_pledge_change(rt, penalty)?; // Power is not restored yet, but when the recovered sectors are successfully PoSted. Ok(()) @@ -1240,22 +1240,17 @@ impl Actor { let st: State = rt.state()?; rt.send( - &*STORAGE_POWER_ACTOR_ADDR, + *STORAGE_POWER_ACTOR_ADDR, PowerMethod::OnConsensusFault as u64, - &Serialized::serialize(BigIntSer(&st.locked_funds))?, - &BigInt::zero(), + Serialized::serialize(BigIntSer(&st.locked_funds))?, + BigInt::zero(), )?; // TODO: terminate deals with market actor, https://github.com/filecoin-project/specs-actors/issues/279 // Reward reporter with a share of the miner's current balance. let slasher_reward = reward_for_consensus_slash_report(fault_age, rt.current_balance()?); - rt.send( - &reporter, - METHOD_SEND, - &Serialized::default(), - &slasher_reward, - )?; + rt.send(reporter, METHOD_SEND, Serialized::default(), slasher_reward)?; // Delete the actor and burn all remaining funds rt.delete_actor(&*BURNT_FUNDS_ACTOR_ADDR)?; @@ -1297,10 +1292,10 @@ impl Actor { assert!(amount_withdrawn <= curr_balance); rt.send( - &st.info.owner, + st.info.owner, METHOD_SEND, - &Serialized::default(), - &amount_withdrawn, + Serialized::default(), + amount_withdrawn, )?; notify_pledge_change(rt, &vested_amount.neg())?; @@ -1391,7 +1386,7 @@ where // Remove power for new faults, and burn penalties. request_begin_faults(rt, sector_size, &detected_fault_sectors)?; - burn_funds_and_notify_pledge_change(rt, &penalty)?; + burn_funds_and_notify_pledge_change(rt, penalty)?; deadline }; @@ -1454,7 +1449,7 @@ where })??; terminate_sectors(rt, &expired_faults, SECTOR_TERMINATION_FAULTY)?; - burn_funds_and_notify_pledge_change(rt, &ongoing_fault_penalty)?; + burn_funds_and_notify_pledge_change(rt, ongoing_fault_penalty)?; } let proving_period_start = { @@ -1838,7 +1833,7 @@ where Ok(()) })??; // This deposit was locked separately to pledge collateral so there's no pledge change here. - burn_funds(rt, &deposit_burn)?; + burn_funds(rt, deposit_burn)?; Ok(()) } @@ -1940,7 +1935,7 @@ where request_terminate_deals(rt, deal_ids)?; request_terminate_power(rt, termination_type, state.info.sector_size, &all_sectors)?; - burn_funds_and_notify_pledge_change(rt, &penalty)?; + burn_funds_and_notify_pledge_change(rt, penalty)?; Ok(()) } @@ -1984,10 +1979,10 @@ where payload, })?; rt.send( - &*STORAGE_POWER_ACTOR_ADDR, + *STORAGE_POWER_ACTOR_ADDR, PowerMethod::EnrollCronEvent as u64, - &ser_params, - &TokenAmount::zero(), + ser_params, + TokenAmount::zero(), )?; Ok(()) @@ -2013,10 +2008,10 @@ where let ser_params = Serialized::serialize(OnFaultBeginParams { weights })?; rt.send( - &*STORAGE_POWER_ACTOR_ADDR, + *STORAGE_POWER_ACTOR_ADDR, PowerMethod::OnFaultBegin as u64, - &ser_params, - &TokenAmount::zero(), + ser_params, + TokenAmount::zero(), )?; Ok(()) } @@ -2041,10 +2036,10 @@ where let ser_params = Serialized::serialize(OnFaultEndParams { weights })?; rt.send( - &*STORAGE_POWER_ACTOR_ADDR, + *STORAGE_POWER_ACTOR_ADDR, PowerMethod::OnFaultEnd as u64, - &ser_params, - &TokenAmount::zero(), + ser_params, + TokenAmount::zero(), )?; Ok(()) } @@ -2059,10 +2054,10 @@ where } rt.send( - &*STORAGE_MARKET_ACTOR_ADDR, + *STORAGE_MARKET_ACTOR_ADDR, MarketMethod::OnMinerSectorsTerminate as u64, - &Serialized::serialize(OnMinerSectorsTerminateParams { deal_ids })?, - &TokenAmount::zero(), + Serialized::serialize(OnMinerSectorsTerminateParams { deal_ids })?, + TokenAmount::zero(), )?; Ok(()) } @@ -2090,10 +2085,10 @@ where })?; rt.send( - &*STORAGE_POWER_ACTOR_ADDR, + *STORAGE_POWER_ACTOR_ADDR, PowerMethod::OnSectorTerminate as u64, - &ser_params, - &TokenAmount::zero(), + ser_params, + TokenAmount::zero(), )?; Ok(()) } @@ -2206,13 +2201,13 @@ where RT: Runtime, { let ret = rt.send( - &*STORAGE_MARKET_ACTOR_ADDR, + *STORAGE_MARKET_ACTOR_ADDR, MarketMethod::ComputeDataCommitment as u64, - &Serialized::serialize(ComputeDataCommitmentParams { + Serialized::serialize(ComputeDataCommitmentParams { sector_type, deal_ids, })?, - &TokenAmount::zero(), + TokenAmount::zero(), )?; let unsealed_cid: Cid = ret.deserialize()?; Ok(unsealed_cid) @@ -2304,10 +2299,10 @@ where if raw.protocol() != Protocol::BLS { let ret = rt.send( - &resolved, + resolved, AccountMethod::PubkeyAddress as u64, - &Serialized::default(), - &TokenAmount::zero(), + Serialized::default(), + TokenAmount::zero(), )?; let pub_key: Address = ret.deserialize().map_err(|e| { actor_error!(ErrSerialization; "failed to deserialize address result: {:?}, {}", ret, e) @@ -2325,26 +2320,26 @@ where fn burn_funds_and_notify_pledge_change( rt: &mut RT, - amount: &TokenAmount, + amount: TokenAmount, ) -> Result<(), ActorError> where BS: BlockStore, RT: Runtime, { - burn_funds(rt, amount)?; - notify_pledge_change(rt, &amount.clone().neg()) + burn_funds(rt, amount.clone())?; + notify_pledge_change(rt, &amount.neg()) } -fn burn_funds(rt: &mut RT, amount: &TokenAmount) -> Result<(), ActorError> +fn burn_funds(rt: &mut RT, amount: TokenAmount) -> Result<(), ActorError> where BS: BlockStore, RT: Runtime, { - if amount > &BigInt::zero() { + if amount > BigInt::zero() { rt.send( - &*BURNT_FUNDS_ACTOR_ADDR, + *BURNT_FUNDS_ACTOR_ADDR, METHOD_SEND, - &Serialized::default(), + Serialized::default(), amount, )?; } @@ -2357,10 +2352,10 @@ where { if !pledge_delta.is_zero() { rt.send( - &*STORAGE_POWER_ACTOR_ADDR, + *STORAGE_POWER_ACTOR_ADDR, PowerMethod::UpdatePledgeTotal as u64, - &Serialized::serialize(BigIntSer(pledge_delta))?, - &TokenAmount::zero(), + Serialized::serialize(BigIntSer(pledge_delta))?, + TokenAmount::zero(), )?; } Ok(()) diff --git a/vm/actor/src/builtin/multisig/mod.rs b/vm/actor/src/builtin/multisig/mod.rs index acd3aaea13db..88605af9312a 100644 --- a/vm/actor/src/builtin/multisig/mod.rs +++ b/vm/actor/src/builtin/multisig/mod.rs @@ -375,7 +375,7 @@ impl Actor { // Sufficient number of approvals have arrived, relay message if threshold_met { - rt.send(&tx.to, tx.method, &tx.params, &tx.value)?; + rt.send(tx.to, tx.method, tx.params, tx.value)?; } Ok(()) diff --git a/vm/actor/src/builtin/paych/mod.rs b/vm/actor/src/builtin/paych/mod.rs index ff11ff520eaf..09244269e40d 100644 --- a/vm/actor/src/builtin/paych/mod.rs +++ b/vm/actor/src/builtin/paych/mod.rs @@ -147,13 +147,13 @@ impl Actor { if let Some(extra) = &sv.extra { rt.send( - &extra.actor, + extra.actor, extra.method, - &Serialized::serialize(PaymentVerifyParams { + Serialized::serialize(PaymentVerifyParams { extra: extra.data.clone(), proof: params.proof, })?, - &TokenAmount::from(0u8), + TokenAmount::from(0u8), )?; } @@ -307,10 +307,10 @@ impl Actor { })?; // send remaining balance to `from` - rt.send(&st.from, METHOD_SEND, &Serialized::default(), &rem_bal)?; + rt.send(st.from, METHOD_SEND, Serialized::default(), rem_bal)?; // send ToSend to `to` - rt.send(&st.to, METHOD_SEND, &Serialized::default(), &st.to_send)?; + rt.send(st.to, METHOD_SEND, Serialized::default(), st.to_send)?; rt.transaction(|st: &mut State, _| { st.to_send = TokenAmount::from(0u8); diff --git a/vm/actor/src/builtin/power/mod.rs b/vm/actor/src/builtin/power/mod.rs index 34e657aecaa3..2803721bdab3 100644 --- a/vm/actor/src/builtin/power/mod.rs +++ b/vm/actor/src/builtin/power/mod.rs @@ -83,7 +83,7 @@ impl Actor { let value = rt.message().value_received().clone(); // TODO update this send, is now outdated let addresses: init::ExecReturn = rt - .send(&INIT_ACTOR_ADDR, init::Method::Exec as u64, params, &value)? + .send(*INIT_ACTOR_ADDR, init::Method::Exec as u64, params.clone(), value)? .deserialize()?; rt.transaction::, _>(|st, rt| { @@ -115,7 +115,7 @@ impl Actor { let st: State = rt.state()?; - let (owner_addr, worker_addr) = request_miner_control_addrs(rt, &nominal)?; + let (owner_addr, worker_addr) = request_miner_control_addrs(rt, nominal)?; rt.validate_immediate_caller_is(&[owner_addr, worker_addr])?; let claim = st @@ -358,10 +358,10 @@ impl Actor { for event in cron_events { // TODO switch 12 to OnDeferredCronEvent on miner actor impl rt.send( - &event.miner_addr, + event.miner_addr, 12, - &event.callback_payload, - &TokenAmount::from(0u8), + event.callback_payload, + TokenAmount::from(0u8), )?; } @@ -491,10 +491,10 @@ where { let st: State = rt.state()?; let ret = rt.send( - &*REWARD_ACTOR_ADDR, + *REWARD_ACTOR_ADDR, RewardMethod::LastPerEpochReward as u64, - &Serialized::default(), - &TokenAmount::zero(), + Serialized::default(), + TokenAmount::zero(), )?; let BigIntDe(epoch_reward) = ret.deserialize()?; diff --git a/vm/actor/src/builtin/reward/mod.rs b/vm/actor/src/builtin/reward/mod.rs index 2d764a35f0e5..a95843e2dd69 100644 --- a/vm/actor/src/builtin/reward/mod.rs +++ b/vm/actor/src/builtin/reward/mod.rs @@ -107,18 +107,18 @@ impl Actor { ); rt.send( - &miner_addr, + miner_addr, miner::Method::AddLockedFund as u64, - &Serialized::serialize(&BigIntSer(&reward_payable))?, - &reward_payable, + Serialized::serialize(&BigIntSer(&reward_payable))?, + reward_payable, )?; // Burn the penalty rt.send( - &*BURNT_FUNDS_ACTOR_ADDR, + *BURNT_FUNDS_ACTOR_ADDR, METHOD_SEND, - &Serialized::default(), - &penalty, + Serialized::default(), + penalty.clone(), )?; Ok(()) diff --git a/vm/actor/src/builtin/shared.rs b/vm/actor/src/builtin/shared.rs index a7e461e18cf6..7b130d0080f3 100644 --- a/vm/actor/src/builtin/shared.rs +++ b/vm/actor/src/builtin/shared.rs @@ -11,7 +11,7 @@ use vm::{ActorError, Serialized, TokenAmount}; pub(crate) fn request_miner_control_addrs( rt: &mut RT, - miner_addr: &Address, + miner_addr: Address, ) -> Result<(Address, Address), ActorError> where BS: BlockStore, @@ -20,8 +20,8 @@ where let ret = rt.send( miner_addr, Method::ControlAddresses as u64, - &Serialized::default(), - &TokenAmount::zero(), + Serialized::default(), + TokenAmount::zero(), )?; let addrs: MinerAddrs = ret.deserialize()?; diff --git a/vm/actor/tests/common/mod.rs b/vm/actor/tests/common/mod.rs index 027c6fc3ee6a..fb4b220dbe9c 100644 --- a/vm/actor/tests/common/mod.rs +++ b/vm/actor/tests/common/mod.rs @@ -546,10 +546,10 @@ impl Runtime for MockRuntime { fn send( &mut self, - to: &Address, + to: Address, method: MethodNum, - params: &Serialized, - value: &TokenAmount, + params: Serialized, + value: TokenAmount, ) -> Result { self.require_in_call(); if self.in_transaction { @@ -567,9 +567,9 @@ impl Runtime for MockRuntime { let expected_msg = self.expect_sends.pop_front().unwrap(); - assert!(&expected_msg.to == to && expected_msg.method == method && &expected_msg.params == params && &expected_msg.value == value, "expectedMessage being sent does not match expectation.\nMessage -\t to: {:?} method: {:?} value: {:?} params: {:?}\nExpected -\t {:?}", to, method, value, params, self.expect_sends[0]); + assert!(expected_msg.to == to && expected_msg.method == method && expected_msg.params == params && expected_msg.value == value, "expectedMessage being sent does not match expectation.\nMessage -\t to: {:?} method: {:?} value: {:?} params: {:?}\nExpected -\t {:?}", to, method, value, params, self.expect_sends[0]); - if value > &self.balance { + if value > self.balance { return Err(actor_error!(SysErrSenderStateInvalid; "cannot send value: {:?} exceeds balance: {:?}", value, self.balance diff --git a/vm/interpreter/src/default_runtime.rs b/vm/interpreter/src/default_runtime.rs index 45e738bc1308..d07fda23e89d 100644 --- a/vm/interpreter/src/default_runtime.rs +++ b/vm/interpreter/src/default_runtime.rs @@ -15,6 +15,7 @@ use fil_types::NetworkParams; use forest_encoding::Cbor; use forest_encoding::{error::Error as EncodingError, to_vec}; use ipld_blockstore::BlockStore; +use log::warn; use message::{Message, UnsignedMessage}; use num_bigint::BigInt; use runtime::{ActorCode, MessageInfo, Runtime, Syscalls}; @@ -27,6 +28,9 @@ use vm::{ EMPTY_ARR_CID, METHOD_SEND, }; +// TODO this param isn't finalized +const ACTOR_EXEC_GAS: i64 = 0; + /// Implementation of the Runtime trait. pub struct DefaultRuntime<'db, 'msg, 'st, 'sys, 'r, BS, SYS, P> { state: &'st mut StateTree<'db, BS>, @@ -37,7 +41,7 @@ pub struct DefaultRuntime<'db, 'msg, 'st, 'sys, 'r, BS, SYS, P> { epoch: ChainEpoch, origin: Address, origin_nonce: u64, - num_actors_created: u64, + pub num_actors_created: u64, price_list: PriceList, rand: &'r ChainRand, caller_validated: bool, @@ -185,6 +189,45 @@ where Err(other) => actor_error!(fatal("failed to get cbor object: {}", other)), }) } + + fn internal_send( + &mut self, + from: Address, + to: Address, + method: MethodNum, + value: TokenAmount, + params: Serialized, + ) -> Result { + let msg = UnsignedMessage::builder() + .from(from) + .to(to) + .method_num(method) + .value(value) + .params(params) + .gas_limit(self.gas_available()) + .build() + .expect("Message creation fails"); + + // snapshot state tree + let snapshot = self + .state + .snapshot() + .map_err(|e| actor_error!(fatal("failed to create snapshot {}", e)))?; + + let send_res = vm_send::(self, &msg, None); + match send_res { + Ok(ret) => { + // self.num_actors_created = subrt.num_actors_created; + Ok(ret) + } + Err(e) => { + self.state + .revert_to_snapshot(&snapshot) + .map_err(|e| actor_error!(fatal("failed to revert snapshot: {}", e)))?; + Err(e) + } + } + } } impl Runtime for DefaultRuntime<'_, '_, '_, '_, '_, BS, SYS, P> @@ -329,53 +372,27 @@ where fn send( &mut self, - to: &Address, + to: Address, method: MethodNum, - params: &Serialized, - value: &TokenAmount, + params: Serialized, + value: TokenAmount, ) -> Result { if !self.allow_internal { return Err(actor_error!(SysErrorIllegalActor; "runtime.send() is not allowed")); } - let msg = UnsignedMessage::builder() - .to(*to) - .from(*self.message.from()) - .method_num(method) - .value(value.clone()) - .gas_limit(self.gas_available()) - .params(params.clone()) - .build() - .unwrap(); + let ret = self + .internal_send(*self.message.receiver(), to, method, value, params) + .map_err(|e| { + warn!( + "internal send failed: (to: {}) (method: {}) {}", + to, method, e + ); + e + })?; + self.charge_gas(ACTOR_EXEC_GAS)?; - // snapshot state tree - let snapshot = self - .state - .snapshot() - .map_err(|_e| self.abort(ExitCode::ErrPlaceholder, "failed to create snapshot"))?; - - let epoch = self.curr_epoch(); - let send_res = { - let mut parent = DefaultRuntime::new( - self.state, - self.store.store, - self.syscalls.syscalls, - self.gas_used(), - &msg, - epoch, - self.origin, - self.origin_nonce, - self.num_actors_created, - self.rand, - ); - internal_send::(&mut parent, &msg, 0) - }; - if send_res.is_err() { - self.state - .revert_to_snapshot(&snapshot) - .map_err(|_e| self.abort(ExitCode::ErrPlaceholder, "failed to revert snapshot"))?; - } - send_res + Ok(ret) } fn abort>(&self, exit_code: ExitCode, msg: S) -> ActorError { @@ -484,83 +501,63 @@ where Ok(total) } } + /// Shared logic between the DefaultRuntime and the Interpreter. /// It invokes methods on different Actors based on the Message. -pub fn internal_send( - runtime: &mut DefaultRuntime<'_, '_, '_, '_, '_, BS, SYS, P>, +pub fn vm_send<'db, 'msg, 'st, 'sys, 'r, BS, SYS, P>( + rt: &mut DefaultRuntime<'db, 'msg, 'st, 'sys, 'r, BS, SYS, P>, msg: &UnsignedMessage, - _gas_cost: i64, + gas_cost: Option, ) -> Result where BS: BlockStore, SYS: Syscalls, P: NetworkParams, { - runtime.charge_gas( - runtime - .price_list() + if let Some(cost) = gas_cost { + rt.charge_gas(cost)?; + } + + // TODO maybe move this + rt.charge_gas( + rt.price_list() .on_method_invocation(msg.value(), msg.method_num()), )?; - // TODO: we need to try to recover here and try to create account actor - // TODO: actually fix this and don't leave as unwrap for PR - let to_actor = runtime - .state - .get_actor(msg.to()) - .map_err(ActorError::new_fatal)? - .unwrap(); - - if msg.value() != &0u8.into() { - transfer(runtime.state, &msg.from(), &msg.to(), &msg.value()) - .map_err(|e| actor_error!(SysErrSenderInvalid; e))?; - } - - let method_num = msg.method_num(); - - if method_num != METHOD_SEND { - let ret = { - // TODO: make its own method/struct - match to_actor.code { - x if x == *SYSTEM_ACTOR_CODE_ID => { - actor::system::Actor.invoke_method(runtime, method_num, msg.params()) - } - x if x == *INIT_ACTOR_CODE_ID => { - actor::init::Actor.invoke_method(runtime, method_num, msg.params()) - } - x if x == *CRON_ACTOR_CODE_ID => { - actor::cron::Actor.invoke_method(runtime, method_num, msg.params()) - } - x if x == *ACCOUNT_ACTOR_CODE_ID => { - actor::account::Actor.invoke_method(runtime, method_num, msg.params()) - } - x if x == *POWER_ACTOR_CODE_ID => { - actor::power::Actor.invoke_method(runtime, method_num, msg.params()) - } - x if x == *MINER_ACTOR_CODE_ID => { - actor::miner::Actor.invoke_method(runtime, method_num, msg.params()) - } - x if x == *MARKET_ACTOR_CODE_ID => { - actor::market::Actor.invoke_method(runtime, method_num, msg.params()) - } - x if x == *PAYCH_ACTOR_CODE_ID => { - actor::paych::Actor.invoke_method(runtime, method_num, msg.params()) - } - x if x == *MULTISIG_ACTOR_CODE_ID => { - actor::multisig::Actor.invoke_method(runtime, method_num, msg.params()) - } - x if x == *REWARD_ACTOR_CODE_ID => { - actor::reward::Actor.invoke_method(runtime, method_num, msg.params()) - } - x if x == *VERIFIED_ACTOR_CODE_ID => { - actor::verifreg::Actor.invoke_method(runtime, method_num, msg.params()) - } - _ => Err( - actor_error!(SysErrorIllegalActor; "no code for actor at address {}", msg.to()), - ), + { + // On get actor gas charge + // TODO this value shouldn't be final + rt.charge_gas(0)?; + + // TODO: we need to try to recover here and try to create account actor + // TODO: actually fix this and don't leave as unwrap for PR + let to_actor = match rt + .state + .get_actor(msg.to()) + .map_err(ActorError::new_fatal)? + { + Some(act) => act, + None => { + // Try to create actor if not exist + todo!() } }; - return ret; + + 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())?; + } + + if msg.method_num() != METHOD_SEND { + rt.charge_gas(ACTOR_EXEC_GAS)?; + return invoke(rt, to_actor.code, msg.method_num(), msg.params(), msg.to()); + } } + Ok(Serialized::default()) } @@ -570,30 +567,83 @@ fn transfer( from: &Address, to: &Address, value: &TokenAmount, -) -> Result<(), String> { +) -> Result<(), ActorError> { if from == to { return Ok(()); } - if value < &0u8.into() { - return Err("Negative transfer value".to_owned()); + + let from_id = state + .lookup_id(from) + .map_err(ActorError::new_fatal)? + .ok_or_else(|| actor_error!(fatal("Failed to lookup from id for address {}", from)))?; + let to_id = state + .lookup_id(to) + .map_err(ActorError::new_fatal)? + .ok_or_else(|| actor_error!(fatal("Failed to lookup to id for address {}", to)))?; + + if from_id == to_id { + return Ok(()); + } + + if value < &0.into() { + return Err( + actor_error!(SysErrForbidden; "attempted to transfer negative transfer value {}", value), + ); } let mut f = state - .get_actor(from)? - .ok_or("Transfer failed when retrieving sender actor")?; + .get_actor(&from_id) + .map_err(ActorError::new_fatal)? + .ok_or_else(|| actor_error!(fatal( + "sender actor does not exist in state during transfer" + )))?; let mut t = state - .get_actor(to)? - .ok_or("Transfer failed when retrieving receiver actor")?; - - f.deduct_funds(&value)?; + .get_actor(&to_id) + .map_err(ActorError::new_fatal)? + .ok_or_else(|| actor_error!(fatal( + "receiver actor does not exist in state during transfer" + )))?; + + f.deduct_funds(&value).map_err(|e| { + actor_error!(SysErrInsufficientFunds; + "transfer failed when deducting funds ({}): {}", value, e) + })?; t.deposit_funds(&value); - state.set_actor(from, f)?; - state.set_actor(to, t)?; + state.set_actor(from, f).map_err(ActorError::new_fatal)?; + state.set_actor(to, t).map_err(ActorError::new_fatal)?; Ok(()) } +pub fn invoke<'db, 'msg, 'st, 'sys, 'r, BS, SYS, P>( + rt: &mut DefaultRuntime<'db, 'msg, 'st, 'sys, 'r, BS, SYS, P>, + code: Cid, + method_num: MethodNum, + params: &Serialized, + to: &Address, +) -> Result +where + BS: BlockStore, + SYS: Syscalls, + P: NetworkParams, +{ + match code { + x if x == *SYSTEM_ACTOR_CODE_ID => system::Actor.invoke_method(rt, method_num, params), + x if x == *INIT_ACTOR_CODE_ID => init::Actor.invoke_method(rt, method_num, params), + x if x == *CRON_ACTOR_CODE_ID => cron::Actor.invoke_method(rt, method_num, params), + x if x == *ACCOUNT_ACTOR_CODE_ID => account::Actor.invoke_method(rt, method_num, params), + x if x == *POWER_ACTOR_CODE_ID => power::Actor.invoke_method(rt, method_num, params), + x if x == *MINER_ACTOR_CODE_ID => miner::Actor.invoke_method(rt, method_num, params), + x if x == *MARKET_ACTOR_CODE_ID => market::Actor.invoke_method(rt, method_num, params), + x if x == *PAYCH_ACTOR_CODE_ID => paych::Actor.invoke_method(rt, method_num, params), + x if x == *MULTISIG_ACTOR_CODE_ID => multisig::Actor.invoke_method(rt, method_num, params), + x if x == *REWARD_ACTOR_CODE_ID => reward::Actor.invoke_method(rt, method_num, params), + x if x == *VERIFIED_ACTOR_CODE_ID => verifreg::Actor.invoke_method(rt, method_num, params), + _ => Err(actor_error!(SysErrorIllegalActor; "no code for actor at address {}", to)), + } +} + /// Returns public address of the specified actor address pub fn resolve_to_key_addr<'st, 'bs, BS, S>( st: &'st StateTree<'bs, S>, diff --git a/vm/interpreter/src/gas_tracker/mod.rs b/vm/interpreter/src/gas_tracker/mod.rs index 199210598f94..eae09864fd49 100644 --- a/vm/interpreter/src/gas_tracker/mod.rs +++ b/vm/interpreter/src/gas_tracker/mod.rs @@ -4,7 +4,7 @@ mod price_list; pub use self::price_list::{price_list_by_epoch, PriceList}; -use vm::{ActorError, ExitCode}; +use vm::{actor_error, ActorError, ExitCode}; pub struct GasTracker { gas_available: i64, @@ -23,13 +23,9 @@ impl GasTracker { pub fn charge_gas(&mut self, to_use: i64) -> Result<(), ActorError> { if self.gas_used + to_use > self.gas_available { self.gas_used = self.gas_available; - Err(ActorError::new( - ExitCode::SysErrOutOfGas, - format!( + Err(actor_error!(SysErrOutOfGas; "not enough gas (used={}) (available={})", - self.gas_used + to_use, - self.gas_available - ), + self.gas_used + to_use, self.gas_available )) } else { self.gas_used += to_use; diff --git a/vm/interpreter/src/vm.rs b/vm/interpreter/src/vm.rs index 2c9222fb0873..0d54540a1134 100644 --- a/vm/interpreter/src/vm.rs +++ b/vm/interpreter/src/vm.rs @@ -1,7 +1,7 @@ // Copyright 2020 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT -use super::{gas_tracker::price_list_by_epoch, internal_send, ChainRand, DefaultRuntime}; +use super::{gas_tracker::price_list_by_epoch, vm_send, ChainRand, DefaultRuntime}; use actor::{ cron, reward, ACCOUNT_ACTOR_CODE_ID, CRON_ACTOR_ADDR, REWARD_ACTOR_ADDR, SYSTEM_ACTOR_ADDR, }; @@ -372,11 +372,10 @@ where self.rand, ); - let ser = match internal_send(&mut rt, msg, gas_cost) { - Ok(ser) => ser, - Err(actor_err) => return (Serialized::default(), rt, Some(actor_err)), - }; - (ser, rt, None) + match vm_send(&mut rt, msg, Some(gas_cost)) { + Ok(ser) => (ser, rt, None), + Err(actor_err) => (Serialized::default(), rt, Some(actor_err)), + } } } diff --git a/vm/interpreter/tests/transfer_test.rs b/vm/interpreter/tests/transfer_test.rs index 75f7a9ba91f9..7577eb7643cf 100644 --- a/vm/interpreter/tests/transfer_test.rs +++ b/vm/interpreter/tests/transfer_test.rs @@ -7,7 +7,7 @@ use blocks::TipsetKeys; use cid::multihash::{Blake2b256, Identity}; use db::MemoryDB; use fil_types::DevnetParams; -use interpreter::{internal_send, ChainRand, DefaultRuntime, DefaultSyscalls}; +use interpreter::{vm_send, ChainRand, DefaultRuntime, DefaultSyscalls}; use ipld_blockstore::BlockStore; use ipld_hamt::Hamt; use message::UnsignedMessage; @@ -108,7 +108,7 @@ fn transfer_test() { 0, &dummy_rand, ); - let _serialized = internal_send(&mut runtime, &message, 0).unwrap(); + let _serialized = vm_send(&mut runtime, &message, None).unwrap(); let actor_state_result_1 = state.get_actor(&actor_addr_1).unwrap().unwrap(); let actor_state_result_2 = state.get_actor(&actor_addr_2).unwrap().unwrap(); diff --git a/vm/runtime/src/lib.rs b/vm/runtime/src/lib.rs index e6b72e4d1364..2b02fefeaeaf 100644 --- a/vm/runtime/src/lib.rs +++ b/vm/runtime/src/lib.rs @@ -100,10 +100,10 @@ pub trait Runtime { /// (and that of any messages it sent in turn) will be rolled back. fn send( &mut self, - to: &Address, + to: Address, method: MethodNum, - params: &Serialized, - value: &TokenAmount, + params: Serialized, + value: TokenAmount, ) -> Result; /// Halts execution upon an error from which the receiver cannot recover. From 6d10354cfe5882d4080d9d950a64150df329cbef Mon Sep 17 00:00:00 2001 From: austinabell Date: Fri, 24 Jul 2020 09:45:22 -0400 Subject: [PATCH 16/23] misc cleanup --- vm/actor/src/builtin/codes.rs | 2 +- vm/actor/src/builtin/market/mod.rs | 2 +- vm/actor/src/builtin/power/mod.rs | 7 ++++- vm/actor/tests/common/mod.rs | 4 +-- vm/interpreter/src/default_runtime.rs | 45 +++++++++++++++------------ 5 files changed, 35 insertions(+), 25 deletions(-) diff --git a/vm/actor/src/builtin/codes.rs b/vm/actor/src/builtin/codes.rs index b3d463b05282..bdeac5982cec 100644 --- a/vm/actor/src/builtin/codes.rs +++ b/vm/actor/src/builtin/codes.rs @@ -14,7 +14,7 @@ lazy_static! { pub static ref PAYCH_ACTOR_CODE_ID: Cid = make_builtin(b"fil/1/paymentchannel"); pub static ref MULTISIG_ACTOR_CODE_ID: Cid = make_builtin(b"fil/1/multisig"); pub static ref REWARD_ACTOR_CODE_ID: Cid = make_builtin(b"fil/1/reward"); - pub static ref VERIFIED_ACTOR_CODE_ID: Cid = make_builtin(b"fil/1/verifiedregistry"); + pub static ref VERIFREG_ACTOR_CODE_ID: Cid = make_builtin(b"fil/1/verifiedregistry"); // Set of actor code types that can represent external signing parties. pub static ref CALLER_TYPES_SIGNABLE: [Cid; 2] = diff --git a/vm/actor/src/builtin/market/mod.rs b/vm/actor/src/builtin/market/mod.rs index d4be278448e6..387f70de98e2 100644 --- a/vm/actor/src/builtin/market/mod.rs +++ b/vm/actor/src/builtin/market/mod.rs @@ -785,7 +785,7 @@ where } // Storage miner actor entry; implied funds recipient is the associated owner address. -let (owner_addr, worker_addr) = request_miner_control_addrs(rt, nominal)?; + let (owner_addr, worker_addr) = request_miner_control_addrs(rt, nominal)?; rt.validate_immediate_caller_is([owner_addr, worker_addr].iter())?; Ok((nominal, owner_addr)) } diff --git a/vm/actor/src/builtin/power/mod.rs b/vm/actor/src/builtin/power/mod.rs index 2803721bdab3..d444c9bf5720 100644 --- a/vm/actor/src/builtin/power/mod.rs +++ b/vm/actor/src/builtin/power/mod.rs @@ -83,7 +83,12 @@ impl Actor { let value = rt.message().value_received().clone(); // TODO update this send, is now outdated let addresses: init::ExecReturn = rt - .send(*INIT_ACTOR_ADDR, init::Method::Exec as u64, params.clone(), value)? + .send( + *INIT_ACTOR_ADDR, + init::Method::Exec as u64, + params.clone(), + value, + )? .deserialize()?; rt.transaction::, _>(|st, rt| { diff --git a/vm/actor/tests/common/mod.rs b/vm/actor/tests/common/mod.rs index fb4b220dbe9c..9608aa552f06 100644 --- a/vm/actor/tests/common/mod.rs +++ b/vm/actor/tests/common/mod.rs @@ -4,7 +4,7 @@ use actor::{ self, ACCOUNT_ACTOR_CODE_ID, CRON_ACTOR_CODE_ID, INIT_ACTOR_CODE_ID, MARKET_ACTOR_CODE_ID, MINER_ACTOR_CODE_ID, MULTISIG_ACTOR_CODE_ID, PAYCH_ACTOR_CODE_ID, POWER_ACTOR_CODE_ID, - REWARD_ACTOR_CODE_ID, SYSTEM_ACTOR_CODE_ID, VERIFIED_ACTOR_CODE_ID, + REWARD_ACTOR_CODE_ID, SYSTEM_ACTOR_CODE_ID, VERIFREG_ACTOR_CODE_ID, }; use address::Address; use cid::{multihash::Blake2b256, Cid}; @@ -257,7 +257,7 @@ impl MockRuntime { x if x == &*REWARD_ACTOR_CODE_ID => { actor::reward::Actor.invoke_method(self, method_num, params) } - x if x == &*VERIFIED_ACTOR_CODE_ID => { + x if x == &*VERIFREG_ACTOR_CODE_ID => { actor::verifreg::Actor.invoke_method(self, method_num, params) } _ => Err(actor_error!(SysErrForbidden; "invalid method id")), diff --git a/vm/interpreter/src/default_runtime.rs b/vm/interpreter/src/default_runtime.rs index d07fda23e89d..e6178691365e 100644 --- a/vm/interpreter/src/default_runtime.rs +++ b/vm/interpreter/src/default_runtime.rs @@ -215,18 +215,18 @@ where .map_err(|e| actor_error!(fatal("failed to create snapshot {}", e)))?; let send_res = vm_send::(self, &msg, None); - match send_res { - Ok(ret) => { - // self.num_actors_created = subrt.num_actors_created; - Ok(ret) - } - Err(e) => { - self.state - .revert_to_snapshot(&snapshot) - .map_err(|e| actor_error!(fatal("failed to revert snapshot: {}", e)))?; - Err(e) + send_res.map_err(|e| { + if let Err(e) = self.state.revert_to_snapshot(&snapshot) { + actor_error!(fatal("failed to revert snapshot: {}", e)) + } else { + e } - } + }) + } + + /// TryCreateAccountActor creates account actors from only BLS/SECP256K1 addresses. + fn try_create_account_actor(&mut self, addr: Address) -> Result { + todo!() } } @@ -539,7 +539,7 @@ where Some(act) => act, None => { // Try to create actor if not exist - todo!() + rt.try_create_account_actor(*msg.to())? } }; @@ -594,15 +594,19 @@ fn transfer( let mut f = state .get_actor(&from_id) .map_err(ActorError::new_fatal)? - .ok_or_else(|| actor_error!(fatal( - "sender actor does not exist in state during transfer" - )))?; + .ok_or_else(|| { + actor_error!(fatal( + "sender actor does not exist in state during transfer" + )) + })?; let mut t = state .get_actor(&to_id) .map_err(ActorError::new_fatal)? - .ok_or_else(|| actor_error!(fatal( - "receiver actor does not exist in state during transfer" - )))?; + .ok_or_else(|| { + actor_error!(fatal( + "receiver actor does not exist in state during transfer" + )) + })?; f.deduct_funds(&value).map_err(|e| { actor_error!(SysErrInsufficientFunds; @@ -616,7 +620,8 @@ fn transfer( Ok(()) } -pub fn invoke<'db, 'msg, 'st, 'sys, 'r, BS, SYS, P>( +/// Calls actor code with method and parameters. +fn invoke<'db, 'msg, 'st, 'sys, 'r, BS, SYS, P>( rt: &mut DefaultRuntime<'db, 'msg, 'st, 'sys, 'r, BS, SYS, P>, code: Cid, method_num: MethodNum, @@ -639,7 +644,7 @@ where x if x == *PAYCH_ACTOR_CODE_ID => paych::Actor.invoke_method(rt, method_num, params), x if x == *MULTISIG_ACTOR_CODE_ID => multisig::Actor.invoke_method(rt, method_num, params), x if x == *REWARD_ACTOR_CODE_ID => reward::Actor.invoke_method(rt, method_num, params), - x if x == *VERIFIED_ACTOR_CODE_ID => verifreg::Actor.invoke_method(rt, method_num, params), + x if x == *VERIFREG_ACTOR_CODE_ID => verifreg::Actor.invoke_method(rt, method_num, params), _ => Err(actor_error!(SysErrorIllegalActor; "no code for actor at address {}", to)), } } From ae705cad9eee38dbf6583cf8df1ae01c05e11fcb Mon Sep 17 00:00:00 2001 From: austinabell Date: Fri, 24 Jul 2020 10:44:52 -0400 Subject: [PATCH 17/23] update actor initialization --- vm/interpreter/src/default_runtime.rs | 73 +++++++++++++++++++++++-- vm/interpreter/tests/transfer_test.rs | 10 ++-- vm/state_tree/src/lib.rs | 9 +-- vm/state_tree/tests/state_tree_tests.rs | 8 +-- 4 files changed, 74 insertions(+), 26 deletions(-) diff --git a/vm/interpreter/src/default_runtime.rs b/vm/interpreter/src/default_runtime.rs index e6178691365e..8449350fe6a2 100644 --- a/vm/interpreter/src/default_runtime.rs +++ b/vm/interpreter/src/default_runtime.rs @@ -224,9 +224,43 @@ where }) } - /// TryCreateAccountActor creates account actors from only BLS/SECP256K1 addresses. - fn try_create_account_actor(&mut self, addr: Address) -> Result { - todo!() + /// creates account actors from only BLS/SECP256K1 addresses. + pub fn try_create_account_actor(&mut self, addr: &Address) -> Result { + self.charge_gas(self.price_list().on_create_actor())?; + + let addr_id = self + .state + .register_new_address(addr) + .map_err(ActorError::new_fatal)?; + + let act = make_actor(addr)?; + + self.state + .set_actor(&addr_id, act) + .map_err(ActorError::new_fatal)?; + + let p = Serialized::serialize(&addr).map_err(|e| { + actor_error!(fatal( + "couldn't serialize params for actor construction: {}", + e + )) + })?; + + self.internal_send( + *SYSTEM_ACTOR_ADDR, + addr_id, + account::Method::Constructor as u64, + TokenAmount::from(0), + p, + )?; + + let act = self + .state + .get_actor(&addr_id) + .map_err(ActorError::new_fatal)? + .ok_or_else(|| actor_error!(fatal("failed to retrieve created actor state")))?; + + Ok(act) } } @@ -539,7 +573,7 @@ where Some(act) => act, None => { // Try to create actor if not exist - rt.try_create_account_actor(*msg.to())? + rt.try_create_account_actor(msg.to())? } }; @@ -609,7 +643,7 @@ fn transfer( })?; f.deduct_funds(&value).map_err(|e| { - actor_error!(SysErrInsufficientFunds; + actor_error!(SysErrInsufficientFunds; "transfer failed when deducting funds ({}): {}", value, e) })?; t.deposit_funds(&value); @@ -691,3 +725,32 @@ where Ok(acc_st.address) } + +fn make_actor(addr: &Address) -> Result { + match addr.protocol() { + Protocol::BLS => Ok(new_bls_account_actor()), + Protocol::Secp256k1 => Ok(new_secp256k1_account_actor()), + Protocol::ID => { + Err(actor_error!(SysErrInvalidReceiver; "no actor with given id: {}", addr)) + } + Protocol::Actor => Err(actor_error!(SysErrInvalidReceiver; "no such actor: {}", addr)), + } +} + +fn new_bls_account_actor() -> ActorState { + ActorState { + code: ACCOUNT_ACTOR_CODE_ID.clone(), + balance: TokenAmount::from(0), + state: EMPTY_ARR_CID.clone(), + sequence: 0, + } +} + +fn new_secp256k1_account_actor() -> ActorState { + ActorState { + code: ACCOUNT_ACTOR_CODE_ID.clone(), + balance: TokenAmount::from(0), + state: EMPTY_ARR_CID.clone(), + sequence: 0, + } +} diff --git a/vm/interpreter/tests/transfer_test.rs b/vm/interpreter/tests/transfer_test.rs index 7577eb7643cf..886506ae8e16 100644 --- a/vm/interpreter/tests/transfer_test.rs +++ b/vm/interpreter/tests/transfer_test.rs @@ -76,12 +76,10 @@ fn transfer_test() { 0, ); - let actor_addr_1 = state - .register_new_address(&actor_addr_1, actor_state_1) - .unwrap(); - let actor_addr_2 = state - .register_new_address(&actor_addr_2, actor_state_2) - .unwrap(); + let actor_addr_1 = state.register_new_address(&actor_addr_1).unwrap(); + let actor_addr_2 = state.register_new_address(&actor_addr_2).unwrap(); + state.set_actor(&actor_addr_1, actor_state_1).unwrap(); + state.set_actor(&actor_addr_2, actor_state_2).unwrap(); let message = UnsignedMessage::builder() .to(actor_addr_1.clone()) diff --git a/vm/state_tree/src/lib.rs b/vm/state_tree/src/lib.rs index d3aed1409700..70c03a29f829 100644 --- a/vm/state_tree/src/lib.rs +++ b/vm/state_tree/src/lib.rs @@ -144,11 +144,7 @@ where } /// Register a new address through the init actor. - pub fn register_new_address( - &mut self, - addr: &Address, - actor: ActorState, - ) -> Result { + pub fn register_new_address(&mut self, addr: &Address) -> Result { let mut init_act: ActorState = self .get_actor(&INIT_ACTOR_ADDR)? .ok_or("Could not retrieve init actor")?; @@ -174,9 +170,6 @@ where self.set_actor(&INIT_ACTOR_ADDR, init_act)?; - // After mutating the init actor, set the state at the ID address created - self.set_actor(&new_addr, actor)?; - Ok(new_addr) } diff --git a/vm/state_tree/tests/state_tree_tests.rs b/vm/state_tree/tests/state_tree_tests.rs index 1cf165bf6d6e..7c7d70de9411 100644 --- a/vm/state_tree/tests/state_tree_tests.rs +++ b/vm/state_tree/tests/state_tree_tests.rs @@ -88,16 +88,10 @@ fn get_set_non_id() { // Register new address let addr = Address::new_secp256k1(&[2; SECP_PUB_LEN]).unwrap(); - let secp_state = ActorState::new(e_cid.clone(), e_cid.clone(), Default::default(), 0); - let assigned_addr = tree - .register_new_address(&addr, secp_state.clone()) - .unwrap(); + let assigned_addr = tree.register_new_address(&addr).unwrap(); assert_eq!(assigned_addr, Address::new_id(100)); - // Test resolution of Secp address - assert_eq!(tree.get_actor(&addr).unwrap(), Some(secp_state)); - // Test reverting snapshot to before init actor set tree.revert_to_snapshot(&snapshot).unwrap(); assert_eq!(tree.snapshot().unwrap(), snapshot); From 2b9ed09e0b8ceb6466cf68de786a437748963795 Mon Sep 17 00:00:00 2001 From: austinabell Date: Fri, 24 Jul 2020 10:52:57 -0400 Subject: [PATCH 18/23] Update gas interfaces --- vm/interpreter/src/gas_block_store.rs | 13 +++++++++---- vm/interpreter/src/gas_syscalls.rs | 26 +++++++++----------------- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/vm/interpreter/src/gas_block_store.rs b/vm/interpreter/src/gas_block_store.rs index 235ecef6f144..797e275b466f 100644 --- a/vm/interpreter/src/gas_block_store.rs +++ b/vm/interpreter/src/gas_block_store.rs @@ -9,6 +9,7 @@ use ipld_blockstore::BlockStore; use std::cell::RefCell; use std::error::Error as StdError; use std::rc::Rc; +use vm::{actor_error, ActorError}; /// Blockstore wrapper to charge gas on reads and writes pub(crate) struct GasBlockStore<'bs, BS> { @@ -23,8 +24,10 @@ where { /// Get bytes from block store by Cid fn get_bytes(&self, cid: &Cid) -> Result>, Box> { - // TODO investigate if should panic/exit here, should be fatal - let ret = self.store.get_bytes(cid)?; + let ret = self + .store + .get_bytes(cid) + .map_err(|e| actor_error!(fatal("failed to get block from blockstore: {}", e)))?; if let Some(bz) = &ret { self.gas .borrow_mut() @@ -54,8 +57,10 @@ where .borrow_mut() .charge_gas(self.price_list.on_ipld_put(to_vec(obj).unwrap().len()))?; - // TODO investigate if error here should be fatal - self.store.put(obj, hash) + Ok(self + .store + .put(obj, hash) + .map_err(|e| actor_error!(fatal("failed to write to store {}", e)))?) } } diff --git a/vm/interpreter/src/gas_syscalls.rs b/vm/interpreter/src/gas_syscalls.rs index 56c80a63b25c..ca9f81112ef8 100644 --- a/vm/interpreter/src/gas_syscalls.rs +++ b/vm/interpreter/src/gas_syscalls.rs @@ -29,20 +29,16 @@ where signer: &Address, plaintext: &[u8], ) -> Result<(), Box> { - self.gas - .borrow_mut() - .charge_gas( - self.price_list - .on_verify_signature(signature.signature_type(), plaintext.len()), - ) - .unwrap(); + self.gas.borrow_mut().charge_gas( + self.price_list + .on_verify_signature(signature.signature_type(), plaintext.len()), + )?; self.syscalls.verify_signature(signature, signer, plaintext) } fn hash_blake2b(&self, data: &[u8]) -> Result<[u8; 32], Box> { self.gas .borrow_mut() - .charge_gas(self.price_list.on_hashing(data.len())) - .unwrap(); + .charge_gas(self.price_list.on_hashing(data.len()))?; self.syscalls.hash_blake2b(data) } fn compute_unsealed_sector_cid( @@ -52,22 +48,19 @@ where ) -> Result> { self.gas .borrow_mut() - .charge_gas(self.price_list.on_compute_unsealed_sector_cid(reg, pieces)) - .unwrap(); + .charge_gas(self.price_list.on_compute_unsealed_sector_cid(reg, pieces))?; self.syscalls.compute_unsealed_sector_cid(reg, pieces) } fn verify_seal(&self, vi: &SealVerifyInfo) -> Result<(), Box> { self.gas .borrow_mut() - .charge_gas(self.price_list.on_verify_seal(vi)) - .unwrap(); + .charge_gas(self.price_list.on_verify_seal(vi))?; self.syscalls.verify_seal(vi) } fn verify_post(&self, vi: &WindowPoStVerifyInfo) -> Result<(), Box> { self.gas .borrow_mut() - .charge_gas(self.price_list.on_verify_post(vi)) - .unwrap(); + .charge_gas(self.price_list.on_verify_post(vi))?; self.syscalls.verify_post(vi) } fn verify_consensus_fault( @@ -78,8 +71,7 @@ where ) -> Result, Box> { self.gas .borrow_mut() - .charge_gas(self.price_list.on_verify_consensus_fault()) - .unwrap(); + .charge_gas(self.price_list.on_verify_consensus_fault())?; self.syscalls.verify_consensus_fault(h1, h2, extra) } From 340830654ef193d4512cf1d1526239e427f50906 Mon Sep 17 00:00:00 2001 From: austinabell Date: Fri, 24 Jul 2020 21:38:26 -0400 Subject: [PATCH 19/23] Update runtime generation --- vm/interpreter/src/default_runtime.rs | 47 +++++++++++++++++++++++---- vm/interpreter/src/vm.rs | 17 ++++++---- vm/interpreter/tests/transfer_test.rs | 2 +- 3 files changed, 53 insertions(+), 13 deletions(-) diff --git a/vm/interpreter/src/default_runtime.rs b/vm/interpreter/src/default_runtime.rs index 8449350fe6a2..753eafe56d9d 100644 --- a/vm/interpreter/src/default_runtime.rs +++ b/vm/interpreter/src/default_runtime.rs @@ -31,6 +31,24 @@ use vm::{ // TODO this param isn't finalized const ACTOR_EXEC_GAS: i64 = 0; +struct VMMsg { + caller: Address, + receiver: Address, + value_received: TokenAmount, +} + +impl MessageInfo for VMMsg { + fn caller(&self) -> &Address { + &self.caller + } + fn receiver(&self) -> &Address { + &self.receiver + } + fn value_received(&self) -> &TokenAmount { + &self.value_received + } +} + /// Implementation of the Runtime trait. pub struct DefaultRuntime<'db, 'msg, 'st, 'sys, 'r, BS, SYS, P> { state: &'st mut StateTree<'db, BS>, @@ -38,10 +56,11 @@ pub struct DefaultRuntime<'db, 'msg, 'st, 'sys, 'r, BS, SYS, P> { syscalls: GasSyscalls<'sys, SYS>, gas_tracker: Rc>, message: &'msg UnsignedMessage, + vm_msg: VMMsg, epoch: ChainEpoch, origin: Address, origin_nonce: u64, - pub num_actors_created: u64, + num_actors_created: u64, price_list: PriceList, rand: &'r ChainRand, caller_validated: bool, @@ -68,7 +87,7 @@ where origin_nonce: u64, num_actors_created: u64, rand: &'r ChainRand, - ) -> Self { + ) -> Result { let price_list = price_list_by_epoch(epoch); let gas_tracker = Rc::new(RefCell::new(GasTracker::new(message.gas_limit(), gas_used))); let gas_block_store = GasBlockStore { @@ -81,12 +100,27 @@ where gas: Rc::clone(&gas_tracker), syscalls, }; - DefaultRuntime { + + let caller_id = state + .lookup_id(&message.from()) + .map_err(|e| actor_error!(fatal("failed to lookup id: {}", e)))? + .ok_or_else( + || actor_error!(SysErrInvalidReceiver; "resolve msg from address failed"), + )?; + + let vm_msg = VMMsg { + caller: caller_id, + receiver: *message.receiver(), + value_received: message.value_received().clone(), + }; + + Ok(DefaultRuntime { state, store: gas_block_store, syscalls: gas_syscalls, gas_tracker, message, + vm_msg, epoch, origin, origin_nonce, @@ -96,7 +130,7 @@ where allow_internal: true, caller_validated: false, params: PhantomData, - } + }) } /// Adds to amount of used @@ -271,7 +305,7 @@ where P: NetworkParams, { fn message(&self) -> &dyn MessageInfo { - self.message + &self.vm_msg } fn curr_epoch(&self) -> ChainEpoch { self.epoch @@ -683,7 +717,8 @@ where } } -/// Returns public address of the specified actor address +/// returns the public key type of address (`BLS`/`SECP256K1`) of an account actor +/// identified by `addr`. pub fn resolve_to_key_addr<'st, 'bs, BS, S>( st: &'st StateTree<'bs, S>, store: &'bs BS, diff --git a/vm/interpreter/src/vm.rs b/vm/interpreter/src/vm.rs index 0d54540a1134..abddb1a4af15 100644 --- a/vm/interpreter/src/vm.rs +++ b/vm/interpreter/src/vm.rs @@ -301,7 +301,9 @@ where // scoped to deal with mutable reference borrowing let (ret_data, gas_used, act_err) = { - let (ret_data, mut rt, act_err) = self.send(msg, msg_gas_cost); + let (ret_data, rt, act_err) = self.send(msg, msg_gas_cost); + // TODO update this + let mut rt = rt.unwrap(); rt.charge_gas(rt.price_list().on_chain_return_value(ret_data.len())) .map_err(|e| e.to_string())?; (ret_data, rt.gas_used(), act_err) @@ -356,10 +358,10 @@ where gas_cost: i64, ) -> ( Serialized, - DefaultRuntime<'db, 'm, '_, '_, '_, DB, SYS, P>, + Option>, Option, ) { - let mut rt = DefaultRuntime::new( + let res = DefaultRuntime::new( &mut self.state, self.store, &self.syscalls, @@ -372,9 +374,12 @@ where self.rand, ); - match vm_send(&mut rt, msg, Some(gas_cost)) { - Ok(ser) => (ser, rt, None), - Err(actor_err) => (Serialized::default(), rt, Some(actor_err)), + match res { + Ok(mut rt) => match vm_send(&mut rt, msg, Some(gas_cost)) { + Ok(ser) => (ser, Some(rt), None), + Err(actor_err) => (Serialized::default(), Some(rt), Some(actor_err)), + }, + Err(e) => (Serialized::default(), None, Some(e)), } } } diff --git a/vm/interpreter/tests/transfer_test.rs b/vm/interpreter/tests/transfer_test.rs index 886506ae8e16..37ffdbcef733 100644 --- a/vm/interpreter/tests/transfer_test.rs +++ b/vm/interpreter/tests/transfer_test.rs @@ -105,7 +105,7 @@ fn transfer_test() { 0, 0, &dummy_rand, - ); + ).unwrap(); let _serialized = vm_send(&mut runtime, &message, None).unwrap(); let actor_state_result_1 = state.get_actor(&actor_addr_1).unwrap().unwrap(); From aa26fe351e4760807d22e7e320e38baf01dad83b Mon Sep 17 00:00:00 2001 From: austinabell Date: Sat, 25 Jul 2020 09:39:31 -0400 Subject: [PATCH 20/23] Update apply ret struct --- vm/interpreter/src/vm.rs | 112 +++++++++++--------------- vm/interpreter/tests/transfer_test.rs | 3 +- 2 files changed, 51 insertions(+), 64 deletions(-) diff --git a/vm/interpreter/src/vm.rs b/vm/interpreter/src/vm.rs index abddb1a4af15..cb3910ee6ef2 100644 --- a/vm/interpreter/src/vm.rs +++ b/vm/interpreter/src/vm.rs @@ -20,7 +20,7 @@ use state_tree::StateTree; use std::collections::HashSet; use std::error::Error as StdError; use std::marker::PhantomData; -use vm::{ActorError, ExitCode, Serialized}; +use vm::{actor_error, ActorError, ExitCode, Serialized}; /// Interpreter which handles execution of state transitioning messages and returns receipts /// from the vm execution. @@ -178,26 +178,26 @@ where let (ret_data, _, act_err) = self.send(msg, 0); if let Some(err) = act_err { - return ApplyRet::new( - MessageReceipt { + return ApplyRet { + msg_receipt: MessageReceipt { return_data: ret_data, exit_code: err.exit_code(), gas_used: 0, }, - BigInt::zero(), - Some(err), - ); + penalty: BigInt::zero(), + act_error: Some(err), + }; }; - ApplyRet::new( - MessageReceipt { + ApplyRet { + msg_receipt: MessageReceipt { return_data: ret_data, exit_code: ExitCode::Ok, gas_used: 0, }, - BigInt::zero(), - None, - ) + act_error: None, + penalty: BigInt::zero(), + } } /// Applies the state transition for a single message @@ -210,85 +210,83 @@ where let msg_gas_cost = pl.on_chain_message(ser_msg.len()); if msg_gas_cost > msg.gas_limit() { - return Ok(ApplyRet::new( - MessageReceipt { + return Ok(ApplyRet { + msg_receipt: MessageReceipt { return_data: Serialized::default(), exit_code: ExitCode::SysErrOutOfGas, gas_used: 0, }, - msg.gas_price() * msg_gas_cost, - Some(ActorError::new( - ExitCode::SysErrOutOfGas, - "Out of gas".to_owned(), - )), - )); + act_error: Some(actor_error!(SysErrOutOfGas; + "Out of gas ({} > {})", msg_gas_cost, msg.gas_limit())), + penalty: msg.gas_price() * msg_gas_cost, + }); } let miner_penalty_amount = msg.gas_price() * msg_gas_cost; let mut from_act = match self.state.get_actor(msg.from()) { Ok(from_act) => from_act.ok_or("Failed to retrieve actor state")?, Err(_) => { - return Ok(ApplyRet::new( - MessageReceipt { + return Ok(ApplyRet { + msg_receipt: MessageReceipt { return_data: Serialized::default(), exit_code: ExitCode::SysErrSenderInvalid, gas_used: 0, }, - msg.gas_price() * msg_gas_cost, - Some(ActorError::new( + penalty: msg.gas_price() * msg_gas_cost, + act_error: Some(ActorError::new( ExitCode::SysErrSenderInvalid, "Sender invalid".to_owned(), )), - )); + }); } }; if from_act.code != *ACCOUNT_ACTOR_CODE_ID { - return Ok(ApplyRet::new( - MessageReceipt { + return Ok(ApplyRet { + msg_receipt: MessageReceipt { return_data: Serialized::default(), exit_code: ExitCode::SysErrSenderInvalid, gas_used: 0, }, - miner_penalty_amount, - Some(ActorError::new( + penalty: miner_penalty_amount, + act_error: Some(ActorError::new( ExitCode::SysErrSenderInvalid, "Sender invalid".to_owned(), )), - )); + }); }; if msg.sequence() != from_act.sequence { - return Ok(ApplyRet::new( - MessageReceipt { + return Ok(ApplyRet { + msg_receipt: MessageReceipt { return_data: Serialized::default(), exit_code: ExitCode::SysErrSenderStateInvalid, gas_used: 0, }, - miner_penalty_amount, - Some(ActorError::new( + penalty: miner_penalty_amount, + act_error: Some(ActorError::new( ExitCode::SysErrSenderStateInvalid, "Sender state invalid".to_owned(), )), - )); + }); }; let gas_cost = msg.gas_price() * msg.gas_limit(); // TODO requires network_tx_fee to be added as per the spec let total_cost = &gas_cost + msg.value(); if from_act.balance < total_cost { - return Ok(ApplyRet::new( - MessageReceipt { + return Ok(ApplyRet { + msg_receipt: MessageReceipt { return_data: Serialized::default(), exit_code: ExitCode::SysErrSenderStateInvalid, gas_used: 0, }, - miner_penalty_amount, - Some(ActorError::new( + penalty: miner_penalty_amount, + act_error: Some(ActorError::new( ExitCode::SysErrSenderStateInvalid, "Sender state invalid".to_owned(), )), - )); + }); }; self.state.mutate_actor(msg.from(), |act| { @@ -341,15 +339,15 @@ where return Err("Gas handling math is wrong".to_owned()); } - Ok(ApplyRet::new( - MessageReceipt { + Ok(ApplyRet { + msg_receipt: MessageReceipt { return_data: ret_data, exit_code: ExitCode::Ok, gas_used, }, - BigInt::zero(), - None, - )) + penalty: BigInt::zero(), + act_error: None, + }) } /// Instantiates a new Runtime, and calls internal_send to do the execution. fn send<'m>( @@ -384,35 +382,23 @@ where } } -// TODO remove allow dead_code -#[allow(dead_code)] /// Apply message return data pub struct ApplyRet { - msg_receipt: MessageReceipt, - penalty: BigInt, - act_error: Option, -} - -impl ApplyRet { - fn new(msg_receipt: MessageReceipt, penalty: BigInt, act_error: Option) -> Self { - Self { - msg_receipt, - penalty, - act_error, - } - } + pub msg_receipt: MessageReceipt, + pub act_error: Option, + pub penalty: BigInt, } /// Does some basic checks on the Message to see if the fields are valid. -fn check_message(msg: &UnsignedMessage) -> Result<(), String> { +fn check_message(msg: &UnsignedMessage) -> Result<(), &'static str> { if msg.gas_limit() == 0 { - return Err("Message has no gas limit set".to_owned()); + return Err("Message has no gas limit set"); } if msg.value() == &BigInt::zero() { - return Err("Message has no value set".to_owned()); + return Err("Message has no value set"); } if msg.gas_price() == &BigInt::zero() { - return Err("Message has no gas price set".to_owned()); + return Err("Message has no gas price set"); } Ok(()) diff --git a/vm/interpreter/tests/transfer_test.rs b/vm/interpreter/tests/transfer_test.rs index 37ffdbcef733..6f5bea8d6ec0 100644 --- a/vm/interpreter/tests/transfer_test.rs +++ b/vm/interpreter/tests/transfer_test.rs @@ -105,7 +105,8 @@ fn transfer_test() { 0, 0, &dummy_rand, - ).unwrap(); + ) + .unwrap(); let _serialized = vm_send(&mut runtime, &message, None).unwrap(); let actor_state_result_1 = state.get_actor(&actor_addr_1).unwrap().unwrap(); From d1de8c3273cd22752b9769519c8d592431070fb4 Mon Sep 17 00:00:00 2001 From: austinabell Date: Mon, 27 Jul 2020 11:55:20 -0400 Subject: [PATCH 21/23] Small tweaks around message applying --- blockchain/state_manager/src/lib.rs | 2 +- vm/interpreter/src/vm.rs | 36 ++++++++++++----------------- 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/blockchain/state_manager/src/lib.rs b/blockchain/state_manager/src/lib.rs index 04b670298ea0..c74c7e0387b2 100644 --- a/blockchain/state_manager/src/lib.rs +++ b/blockchain/state_manager/src/lib.rs @@ -135,7 +135,7 @@ where )?; // Apply tipset messages - let receipts = vm.apply_tip_set_messages(ts)?; + let receipts = vm.apply_tipset_messages(ts)?; // Construct receipt root from receipts let rect_root = Amt::new_from_slice(self.bs.as_ref(), &receipts)?; diff --git a/vm/interpreter/src/vm.rs b/vm/interpreter/src/vm.rs index cb3910ee6ef2..f38e69febcc9 100644 --- a/vm/interpreter/src/vm.rs +++ b/vm/interpreter/src/vm.rs @@ -26,7 +26,6 @@ use vm::{actor_error, ActorError, ExitCode, Serialized}; /// from the vm execution. pub struct VM<'db, 'r, DB, SYS, P> { state: StateTree<'db, DB>, - // TODO revisit handling buffered store specifically in VM store: &'db DB, epoch: ChainEpoch, syscalls: SYS, @@ -70,7 +69,7 @@ where /// Apply all messages from a tipset /// Returns the receipts from the transactions. - pub fn apply_tip_set_messages( + pub fn apply_tipset_messages( &mut self, tipset: &FullTipset, ) -> Result, Box> { @@ -175,24 +174,16 @@ where } fn apply_implicit_message(&mut self, msg: &UnsignedMessage) -> ApplyRet { - let (ret_data, _, act_err) = self.send(msg, 0); - - if let Some(err) = act_err { - return ApplyRet { - msg_receipt: MessageReceipt { - return_data: ret_data, - exit_code: err.exit_code(), - gas_used: 0, - }, - penalty: BigInt::zero(), - act_error: Some(err), - }; - }; + let (return_data, _, act_err) = self.send(msg, None); ApplyRet { msg_receipt: MessageReceipt { - return_data: ret_data, - exit_code: ExitCode::Ok, + return_data, + exit_code: if let Some(err) = act_err { + err.exit_code() + } else { + ExitCode::Ok + }, gas_used: 0, }, act_error: None, @@ -299,7 +290,7 @@ where // scoped to deal with mutable reference borrowing let (ret_data, gas_used, act_err) = { - let (ret_data, rt, act_err) = self.send(msg, msg_gas_cost); + let (ret_data, rt, act_err) = self.send(msg, Some(msg_gas_cost)); // TODO update this let mut rt = rt.unwrap(); rt.charge_gas(rt.price_list().on_chain_return_value(ret_data.len())) @@ -353,7 +344,7 @@ where fn send<'m>( &mut self, msg: &'m UnsignedMessage, - gas_cost: i64, + gas_cost: Option, ) -> ( Serialized, Option>, @@ -363,7 +354,7 @@ where &mut self.state, self.store, &self.syscalls, - gas_cost, + gas_cost.unwrap_or_default(), &msg, self.epoch, *msg.from(), @@ -373,7 +364,7 @@ where ); match res { - Ok(mut rt) => match vm_send(&mut rt, msg, Some(gas_cost)) { + Ok(mut rt) => match vm_send(&mut rt, msg, gas_cost) { Ok(ser) => (ser, Some(rt), None), Err(actor_err) => (Serialized::default(), Some(rt), Some(actor_err)), }, @@ -394,6 +385,9 @@ fn check_message(msg: &UnsignedMessage) -> Result<(), &'static str> { if msg.gas_limit() == 0 { return Err("Message has no gas limit set"); } + if msg.gas_limit() < 0 { + return Err("Message has negative gas limit"); + } if msg.value() == &BigInt::zero() { return Err("Message has no value set"); } From feeb9949da08988c2e1f6f0dfcf91941c06b2803 Mon Sep 17 00:00:00 2001 From: austinabell Date: Mon, 27 Jul 2020 14:04:20 -0400 Subject: [PATCH 22/23] Update apply_message --- vm/interpreter/src/vm.rs | 107 +++++++++++++++++++++++---------------- vm/src/error.rs | 8 +++ vm/src/method.rs | 5 ++ 3 files changed, 76 insertions(+), 44 deletions(-) diff --git a/vm/interpreter/src/vm.rs b/vm/interpreter/src/vm.rs index f38e69febcc9..4d9d8958fc5a 100644 --- a/vm/interpreter/src/vm.rs +++ b/vm/interpreter/src/vm.rs @@ -203,7 +203,7 @@ where if msg_gas_cost > msg.gas_limit() { return Ok(ApplyRet { msg_receipt: MessageReceipt { - return_data: Serialized::default(), + return_data: Serialized::empty(), exit_code: ExitCode::SysErrOutOfGas, gas_used: 0, }, @@ -219,15 +219,12 @@ where Err(_) => { return Ok(ApplyRet { msg_receipt: MessageReceipt { - return_data: Serialized::default(), + return_data: Serialized::empty(), exit_code: ExitCode::SysErrSenderInvalid, gas_used: 0, }, penalty: msg.gas_price() * msg_gas_cost, - act_error: Some(ActorError::new( - ExitCode::SysErrSenderInvalid, - "Sender invalid".to_owned(), - )), + act_error: Some(actor_error!(SysErrSenderInvalid; "Sender invalid")), }); } }; @@ -235,48 +232,41 @@ where if from_act.code != *ACCOUNT_ACTOR_CODE_ID { return Ok(ApplyRet { msg_receipt: MessageReceipt { - return_data: Serialized::default(), + return_data: Serialized::empty(), exit_code: ExitCode::SysErrSenderInvalid, gas_used: 0, }, penalty: miner_penalty_amount, - act_error: Some(ActorError::new( - ExitCode::SysErrSenderInvalid, - "Sender invalid".to_owned(), - )), + act_error: Some(actor_error!(SysErrSenderInvalid; "send not from account actor")), }); }; + // TODO revisit if this is removed in future if msg.sequence() != from_act.sequence { return Ok(ApplyRet { msg_receipt: MessageReceipt { - return_data: Serialized::default(), + return_data: Serialized::empty(), exit_code: ExitCode::SysErrSenderStateInvalid, gas_used: 0, }, penalty: miner_penalty_amount, - act_error: Some(ActorError::new( - ExitCode::SysErrSenderStateInvalid, - "Sender state invalid".to_owned(), - )), + act_error: Some(actor_error!(SysErrSenderStateInvalid; + "actor sequence invalid: {} != {}", msg.sequence(), from_act.sequence)), }); }; let gas_cost = msg.gas_price() * msg.gas_limit(); - // TODO requires network_tx_fee to be added as per the spec let total_cost = &gas_cost + msg.value(); if from_act.balance < total_cost { return Ok(ApplyRet { msg_receipt: MessageReceipt { - return_data: Serialized::default(), + return_data: Serialized::empty(), exit_code: ExitCode::SysErrSenderStateInvalid, gas_used: 0, }, penalty: miner_penalty_amount, - act_error: Some(ActorError::new( - ExitCode::SysErrSenderStateInvalid, - "Sender state invalid".to_owned(), - )), + act_error: Some(actor_error!(SysErrSenderStateInvalid; + "actor balance less than needed: {} < {}", from_act.balance, total_cost)), }); }; @@ -288,30 +278,59 @@ where let snapshot = self.state.snapshot()?; - // scoped to deal with mutable reference borrowing - let (ret_data, gas_used, act_err) = { - let (ret_data, rt, act_err) = self.send(msg, Some(msg_gas_cost)); - // TODO update this - let mut rt = rt.unwrap(); - rt.charge_gas(rt.price_list().on_chain_return_value(ret_data.len())) - .map_err(|e| e.to_string())?; - (ret_data, rt.gas_used(), act_err) + let (mut ret_data, rt, mut act_err) = self.send(msg, Some(msg_gas_cost)); + if let Some(err) = &act_err { + if err.is_fatal() { + return Err(format!( + "[from={}, to={}, seq={}, m={}, h={}] fatal error: {}", + msg.from(), + msg.to(), + msg.sequence(), + msg.method_num(), + self.epoch, + err + )); + } else { + warn!( + "[from={}, to={}, seq={}, m={}] send error: {}", + msg.from(), + msg.to(), + msg.sequence(), + msg.method_num(), + err + ); + if !ret_data.is_empty() { + return Err(format!( + "message invocation errored, but had a return value anyway: {}", + err + )); + } + } + } + + let gas_used = if let Some(mut rt) = rt { + if !ret_data.is_empty() { + if let Err(e) = rt.charge_gas(rt.price_list().on_chain_return_value(ret_data.len())) + { + act_err = Some(e); + ret_data = Serialized::empty(); + } + } + if rt.gas_used() < 0 { + 0 + } else { + rt.gas_used() + } + } else { + return Err(format!("send returned None runtime: {:?}", act_err)); }; - if let Some(err) = act_err { - if err.is_fatal() { - return Err(format!("Fatal send actor error occurred, err: {:?}", err)); - }; - if err.exit_code() != ExitCode::Ok { - // revert all state changes since snapshot - if let Err(state_err) = self.state.revert_to_snapshot(&snapshot) { - return Err(format!("Revert state failed: {}", state_err)); - }; + if let Some(err) = &act_err { + if !err.is_ok() { + // Revert all state changes on error. + self.state.revert_to_snapshot(&snapshot)?; } - warn!("Send actor error: from:{}, to:{}", msg.from(), msg.to()); } - // TODO recheck this - let gas_used = if gas_used < 0 { 0 } else { gas_used }; // refund unused gas let refund = (msg.gas_limit() - gas_used) * msg.gas_price(); @@ -366,9 +385,9 @@ where match res { Ok(mut rt) => match vm_send(&mut rt, msg, gas_cost) { Ok(ser) => (ser, Some(rt), None), - Err(actor_err) => (Serialized::default(), Some(rt), Some(actor_err)), + Err(actor_err) => (Serialized::empty(), Some(rt), Some(actor_err)), }, - Err(e) => (Serialized::default(), None, Some(e)), + Err(e) => (Serialized::empty(), None, Some(e)), } } } diff --git a/vm/src/error.rs b/vm/src/error.rs index 653143c3d499..576e719be4c0 100644 --- a/vm/src/error.rs +++ b/vm/src/error.rs @@ -35,14 +35,22 @@ impl ActorError { } } + /// Returns true if error is fatal. pub fn is_fatal(&self) -> bool { self.fatal } + /// Returns the exit code of the error. pub fn exit_code(&self) -> ExitCode { self.exit_code } + /// Returns true when the exit code is `Ok`. + pub fn is_ok(&self) -> bool { + self.exit_code == ExitCode::Ok + } + + /// Error message of the actor error. pub fn msg(&self) -> &str { &self.msg } diff --git a/vm/src/method.rs b/vm/src/method.rs index 31b54714fb35..822698cab1e5 100644 --- a/vm/src/method.rs +++ b/vm/src/method.rs @@ -47,6 +47,11 @@ impl Serialized { Self { bytes } } + /// Empty bytes constructor. Used for empty return values. + pub fn empty() -> Self { + Self { bytes: Vec::new() } + } + /// Contructor for encoding Cbor encodable structure pub fn serialize(obj: O) -> Result { Ok(Self { From 01fe6d6af6af26648f013b76205f50b88e8489ed Mon Sep 17 00:00:00 2001 From: austinabell Date: Mon, 27 Jul 2020 15:00:45 -0400 Subject: [PATCH 23/23] fix conflicts from merge --- vm/actor/tests/account_actor_test.rs | 2 +- vm/actor/tests/cron_actor_test.rs | 4 ++-- vm/actor/tests/market_actor_test.rs | 2 +- vm/actor/tests/paych_actor_test.rs | 4 ++-- vm/actor/tests/reward_actor_test.rs | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/vm/actor/tests/account_actor_test.rs b/vm/actor/tests/account_actor_test.rs index 200d05cc738f..d49dddfec79e 100644 --- a/vm/actor/tests/account_actor_test.rs +++ b/vm/actor/tests/account_actor_test.rs @@ -21,7 +21,7 @@ macro_rules! account_tests { caller_type: SYSTEM_ACTOR_CODE_ID.clone(), ..Default::default() }; - rt.expect_validate_caller_addr(&[*SYSTEM_ACTOR_ADDR]); + rt.expect_validate_caller_addr(vec![*SYSTEM_ACTOR_ADDR]); if exit_code.is_success() { rt diff --git a/vm/actor/tests/cron_actor_test.rs b/vm/actor/tests/cron_actor_test.rs index 460c94e2ea76..7989954775ba 100644 --- a/vm/actor/tests/cron_actor_test.rs +++ b/vm/actor/tests/cron_actor_test.rs @@ -137,7 +137,7 @@ fn epoch_tick_with_entries() { } fn construct_and_verify(rt: &mut MockRuntime, params: &ConstructorParams) { - rt.expect_validate_caller_addr(&[*SYSTEM_ACTOR_ADDR]); + rt.expect_validate_caller_addr(vec![*SYSTEM_ACTOR_ADDR]); let ret = rt .call( &*CRON_ACTOR_CODE_ID, @@ -150,7 +150,7 @@ fn construct_and_verify(rt: &mut MockRuntime, params: &ConstructorParams) { } fn epoch_tick_and_verify(rt: &mut MockRuntime) { - rt.expect_validate_caller_addr(&[*SYSTEM_ACTOR_ADDR]); + rt.expect_validate_caller_addr(vec![*SYSTEM_ACTOR_ADDR]); let ret = rt .call(&*CRON_ACTOR_CODE_ID, 2, &Serialized::default()) .unwrap(); diff --git a/vm/actor/tests/market_actor_test.rs b/vm/actor/tests/market_actor_test.rs index 4030bdf22ebd..3218eb6c4f8c 100644 --- a/vm/actor/tests/market_actor_test.rs +++ b/vm/actor/tests/market_actor_test.rs @@ -499,7 +499,7 @@ fn add_participant_funds(rt: &mut MockRuntime, addr: Address, amount: TokenAmoun } fn construct_and_verify(rt: &mut MockRuntime) { - rt.expect_validate_caller_addr(&[SYSTEM_ACTOR_ADDR.clone()]); + rt.expect_validate_caller_addr(vec![SYSTEM_ACTOR_ADDR.clone()]); assert_eq!( Serialized::default(), rt.call( diff --git a/vm/actor/tests/paych_actor_test.rs b/vm/actor/tests/paych_actor_test.rs index c3ea993c7c64..37000784a15b 100644 --- a/vm/actor/tests/paych_actor_test.rs +++ b/vm/actor/tests/paych_actor_test.rs @@ -80,7 +80,7 @@ mod paych_constructor { #[test] fn actor_doesnt_exist_test() { let mut rt = construct_runtime(); - rt.expect_validate_caller_type(&[INIT_ACTOR_CODE_ID.clone()]); + rt.expect_validate_caller_type(vec![INIT_ACTOR_CODE_ID.clone()]); let params = ConstructorParams { to: Address::new_id(TEST_PAYCH_ADDR), from: Address::new_id(TEST_PAYER_ADDR), @@ -137,7 +137,7 @@ mod paych_constructor { ..Default::default() }; - rt.expect_validate_caller_type(&[INIT_ACTOR_CODE_ID.clone()]); + rt.expect_validate_caller_type(vec![INIT_ACTOR_CODE_ID.clone()]); let params = ConstructorParams { to: test_case.paych_addr, from: Address::new_id(10001), diff --git a/vm/actor/tests/reward_actor_test.rs b/vm/actor/tests/reward_actor_test.rs index a2be3947dc01..b9d0b99dedbc 100644 --- a/vm/actor/tests/reward_actor_test.rs +++ b/vm/actor/tests/reward_actor_test.rs @@ -49,7 +49,7 @@ fn balance_less_than_reward() { } fn construct_and_verify(rt: &mut MockRuntime) { - rt.expect_validate_caller_addr(&[SYSTEM_ACTOR_ADDR.clone()]); + rt.expect_validate_caller_addr(vec![SYSTEM_ACTOR_ADDR.clone()]); let ret = rt .call( &*REWARD_ACTOR_CODE_ID,