From 7ca0edaac08e4899d39e1121ee18837a9852dff8 Mon Sep 17 00:00:00 2001 From: austinabell Date: Sun, 29 Mar 2020 12:03:29 -0400 Subject: [PATCH 1/5] Implement rewward state --- vm/actor/src/builtin/reward/state.rs | 189 ++++++++++++++++++++++++--- vm/actor/src/util/multimap.rs | 4 +- 2 files changed, 175 insertions(+), 18 deletions(-) diff --git a/vm/actor/src/builtin/reward/state.rs b/vm/actor/src/builtin/reward/state.rs index d2aa44f99454..72313f2c2b99 100644 --- a/vm/actor/src/builtin/reward/state.rs +++ b/vm/actor/src/builtin/reward/state.rs @@ -1,36 +1,101 @@ // Copyright 2020 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT +use crate::Multimap; use address::Address; use cid::Cid; use clock::ChainEpoch; use ipld_blockstore::BlockStore; use num_bigint::biguint_ser::{BigUintDe, BigUintSer}; -use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use num_derive::FromPrimitive; +use num_traits::{CheckedSub, FromPrimitive}; +use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; use vm::TokenAmount; -pub struct Reward { - // TODO update to new spec - pub start_epoch: ChainEpoch, - pub value: TokenAmount, - pub release_rate: TokenAmount, - pub amount_withdrawn: TokenAmount, -} - /// Reward actor state pub struct State { + /// Reward multimap indexing addresses. pub reward_map: Cid, + /// Sum of un-withdrawn rewards. pub reward_total: TokenAmount, } impl State { - pub fn withdraw_reward( - _store: &BS, - _owner: Address, - _curr_epoch: ChainEpoch, - ) -> TokenAmount { - // TODO - todo!() + pub fn new(empty_multimap: Cid) -> Self { + Self { + reward_map: empty_multimap, + reward_total: TokenAmount::default(), + } + } + + #[allow(dead_code)] + pub(super) fn add_reward( + &mut self, + store: &BS, + owner: &Address, + reward: Reward, + ) -> Result<(), String> { + let mut rewards = Multimap::from_root(store, &self.reward_map)?; + let value = reward.value.clone(); + + rewards.add(owner.hash_key(), reward)?; + + self.reward_map = rewards.root()?; + self.reward_total += value; + Ok(()) + } + + /// Calculates and subtracts the total withdrawable reward for an owner. + #[allow(dead_code)] + pub(super) fn withdraw_reward( + &mut self, + store: &BS, + owner: &Address, + curr_epoch: ChainEpoch, + ) -> Result { + let mut rewards = Multimap::from_root(store, &self.reward_map)?; + let key = owner.hash_key(); + + // Iterate rewards, accumulate total and remaining reward state + let mut remaining_rewards = Vec::new(); + let mut withdrawable_sum = TokenAmount::from(0u8); + rewards.for_each(&key, |_, reward: Reward| { + let unlocked = reward.amount_vested(curr_epoch); + let withdrawable = unlocked + .checked_sub(&reward.amount_withdrawn) + .ok_or(format!( + "Unlocked amount {} less than amount withdrawn {} at epoch {}", + unlocked, reward.amount_withdrawn, curr_epoch + ))?; + + withdrawable_sum += withdrawable; + if unlocked < reward.value { + remaining_rewards.push(Reward { + vesting_function: reward.vesting_function, + start_epoch: reward.start_epoch, + end_epoch: reward.end_epoch, + value: reward.value, + amount_withdrawn: unlocked, + }); + } + Ok(()) + })?; + + assert!( + withdrawable_sum < self.reward_total, + "withdrawable amount cannot exceed previous total" + ); + + // Replace old reward table with remaining rewards + rewards.remove_all(&key)?; + for rew in remaining_rewards { + rewards.add(key.clone(), rew)?; + } + + // Replace old values with updated rewards and total + self.reward_map = rewards.root()?; + self.reward_total -= &withdrawable_sum; + Ok(withdrawable_sum) } } @@ -55,3 +120,95 @@ impl<'de> Deserialize<'de> for State { }) } } + +/// Defines vestion function type for reward actor +#[derive(Clone, Debug, PartialEq, Copy, FromPrimitive)] +#[repr(u8)] +pub enum VestingFunction { + None = 0, + Linear = 1, +} + +impl Serialize for VestingFunction { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + (*self as u8).serialize(serializer) + } +} + +impl<'de> Deserialize<'de> for VestingFunction { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let b: u8 = Deserialize::deserialize(deserializer)?; + Ok(FromPrimitive::from_u8(b) + .ok_or_else(|| de::Error::custom("Invalid registered proof byte"))?) + } +} + +#[derive(Clone, Debug, PartialEq)] +pub struct Reward { + pub vesting_function: VestingFunction, + pub start_epoch: ChainEpoch, + pub end_epoch: ChainEpoch, + pub value: TokenAmount, + pub amount_withdrawn: TokenAmount, +} + +impl Reward { + pub fn amount_vested(&self, curr_epoch: ChainEpoch) -> TokenAmount { + match self.vesting_function { + VestingFunction::None => self.value.clone(), + VestingFunction::Linear => { + let elapsed = curr_epoch - self.start_epoch; + let vest_duration = self.end_epoch - self.start_epoch; + if elapsed >= vest_duration { + self.value.clone() + } else { + self.value.clone() * elapsed / vest_duration + } + } + } + } +} + +impl Serialize for Reward { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + ( + &self.vesting_function, + &self.start_epoch, + &self.end_epoch, + BigUintSer(&self.value), + BigUintSer(&self.amount_withdrawn), + ) + .serialize(serializer) + } +} + +impl<'de> Deserialize<'de> for Reward { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let ( + vesting_function, + start_epoch, + end_epoch, + BigUintDe(value), + BigUintDe(amount_withdrawn), + ) = Deserialize::deserialize(deserializer)?; + Ok(Self { + vesting_function, + start_epoch, + end_epoch, + value, + amount_withdrawn, + }) + } +} diff --git a/vm/actor/src/util/multimap.rs b/vm/actor/src/util/multimap.rs index 229fbcd470a6..2cb400217404 100644 --- a/vm/actor/src/util/multimap.rs +++ b/vm/actor/src/util/multimap.rs @@ -65,9 +65,9 @@ where /// Removes all values for a key. #[inline] - pub fn remove_all(&mut self, key: String) -> Result<(), String> { + pub fn remove_all(&mut self, key: &str) -> Result<(), String> { // Remove entry from table - self.0.delete(&key)?; + self.0.delete(key)?; Ok(()) } From 9f9ff2ebeb2d349aff760aea9307670cde911c6e Mon Sep 17 00:00:00 2001 From: austinabell Date: Sun, 29 Mar 2020 15:08:40 -0400 Subject: [PATCH 2/5] Implement reward actor --- vm/actor/src/builtin/cron/mod.rs | 3 +- vm/actor/src/builtin/init/mod.rs | 3 +- vm/actor/src/builtin/mod.rs | 2 + vm/actor/src/builtin/multisig/mod.rs | 3 +- vm/actor/src/builtin/paych/mod.rs | 7 +- vm/actor/src/builtin/reward/mod.rs | 154 ++++++++++++++++++++++++--- vm/actor/src/builtin/reward/state.rs | 6 +- vm/actor/src/builtin/reward/types.rs | 62 +++++++++++ vm/actor/src/builtin/shared.rs | 19 ++++ vm/actor/tests/multimap_test.rs | 4 +- vm/runtime/src/lib.rs | 4 +- vm/src/error.rs | 8 ++ 12 files changed, 243 insertions(+), 32 deletions(-) create mode 100644 vm/actor/src/builtin/reward/types.rs create mode 100644 vm/actor/src/builtin/shared.rs diff --git a/vm/actor/src/builtin/cron/mod.rs b/vm/actor/src/builtin/cron/mod.rs index c9da63df8347..d86a93bac3d2 100644 --- a/vm/actor/src/builtin/cron/mod.rs +++ b/vm/actor/src/builtin/cron/mod.rs @@ -6,7 +6,6 @@ mod state; pub use self::state::{Entry, State}; use crate::{check_empty_params, SYSTEM_ACTOR_ADDR}; use address::Address; -use forest_ipld::Ipld; use ipld_blockstore::BlockStore; use num_derive::FromPrimitive; use num_traits::FromPrimitive; @@ -85,7 +84,7 @@ impl Actor { let st: State = rt.state(); for entry in st.entries { - rt.send::( + rt.send( &entry.receiver, entry.method_num, &Serialized::default(), diff --git a/vm/actor/src/builtin/init/mod.rs b/vm/actor/src/builtin/init/mod.rs index 76fbea52e29b..f336bf01cc82 100644 --- a/vm/actor/src/builtin/init/mod.rs +++ b/vm/actor/src/builtin/init/mod.rs @@ -12,7 +12,6 @@ use crate::{ }; use address::Address; use cid::Cid; -use forest_ipld::Ipld; use ipld_blockstore::BlockStore; use message::Message; use num_derive::FromPrimitive; @@ -101,7 +100,7 @@ impl Actor { rt.create_actor(¶ms.code_cid, &id_address); // Invoke constructor - rt.send::( + rt.send( &id_address, METHOD_CONSTRUCTOR, ¶ms.constructor_params, diff --git a/vm/actor/src/builtin/mod.rs b/vm/actor/src/builtin/mod.rs index 13fc46c59c89..f324a27488fb 100644 --- a/vm/actor/src/builtin/mod.rs +++ b/vm/actor/src/builtin/mod.rs @@ -10,8 +10,10 @@ pub mod multisig; pub mod paych; pub mod power; pub mod reward; +mod shared; mod singletons; pub mod system; pub use self::codes::*; +pub(crate) use self::shared::*; pub use self::singletons::*; diff --git a/vm/actor/src/builtin/multisig/mod.rs b/vm/actor/src/builtin/multisig/mod.rs index c3a1e890fb66..23bf1e52c7d2 100644 --- a/vm/actor/src/builtin/multisig/mod.rs +++ b/vm/actor/src/builtin/multisig/mod.rs @@ -8,7 +8,6 @@ pub use self::state::State; pub use self::types::*; use crate::{make_map, CALLER_TYPES_SIGNABLE, INIT_ACTOR_ADDR}; use address::Address; -use forest_ipld::Ipld; use ipld_blockstore::BlockStore; use message::Message; use num_derive::FromPrimitive; @@ -366,7 +365,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 ebf8bed1e653..51499da19da7 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 cid::Cid; use encoding::to_vec; -use forest_ipld::Ipld; use ipld_blockstore::BlockStore; use message::Message; use num_bigint::BigInt; @@ -148,7 +147,7 @@ impl Actor { } if let Some(extra) = &sv.extra { - rt.send::( + rt.send( &extra.actor, extra.method, &Serialized::serialize(PaymentVerifyParams { @@ -300,10 +299,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/reward/mod.rs b/vm/actor/src/builtin/reward/mod.rs index ff7c20ffa0af..71324383a380 100644 --- a/vm/actor/src/builtin/reward/mod.rs +++ b/vm/actor/src/builtin/reward/mod.rs @@ -2,15 +2,22 @@ // SPDX-License-Identifier: Apache-2.0, MIT mod state; +mod types; -pub use self::state::{Reward, State}; -use crate::check_empty_params; +pub use self::state::{Reward, State, VestingFunction}; +pub use self::types::*; +use crate::{ + check_empty_params, request_miner_control_addrs, Multimap, BURNT_FUNDS_ACTOR_ADDR, + SYSTEM_ACTOR_ADDR, +}; use address::Address; use ipld_blockstore::BlockStore; use num_derive::FromPrimitive; -use num_traits::FromPrimitive; +use num_traits::{FromPrimitive, Zero}; use runtime::{ActorCode, Runtime}; -use vm::{ActorError, ExitCode, MethodNum, Serialized, METHOD_CONSTRUCTOR}; +use vm::{ + ActorError, ExitCode, MethodNum, Serialized, TokenAmount, METHOD_CONSTRUCTOR, METHOD_SEND, +}; /// Reward actor methods available #[derive(FromPrimitive)] @@ -32,31 +39,147 @@ impl Method { pub struct Actor; impl Actor { /// Constructor for Reward actor - fn constructor(_rt: &RT) -> Result<(), ActorError> + fn constructor(rt: &RT) -> Result<(), ActorError> where BS: BlockStore, RT: Runtime, { - // TODO - todo!(); + rt.validate_immediate_caller_is(std::iter::once(&*SYSTEM_ACTOR_ADDR)); + + let empty_root = Multimap::new(rt.store()).root().map_err(|e| { + ActorError::new( + ExitCode::ErrIllegalState, + format!("failed to construct state: {}", e), + ) + })?; + + rt.create(&State::new(empty_root)); + Ok(()) } + /// Mints a reward and puts into state reward map - fn award_block_reward(_rt: &RT) -> Result<(), ActorError> + fn award_block_reward(rt: &RT, params: AwardBlockRewardParams) -> Result<(), ActorError> where BS: BlockStore, RT: Runtime, { - // TODO add params type and implement - todo!(); + rt.validate_immediate_caller_is(std::iter::once(&*SYSTEM_ACTOR_ADDR)); + if rt.current_balance() >= params.gas_reward { + return Err(ActorError::new( + ExitCode::ErrInsufficientFunds, + format!( + "actor current balance {} insufficient to pay gas reward {}", + rt.current_balance(), + params.gas_reward + ), + )); + } + + let miner = rt.resolve_address(¶ms.miner).ok_or_else(|| { + ActorError::new( + ExitCode::ErrIllegalState, + "failed to resolve given owner address".to_owned(), + ) + })?; + + let prior_bal = rt.current_balance(); + + let penalty: TokenAmount = rt + .transaction::<_, Result<_, String>, _>(|st: &mut State| { + let block_rew = Self::compute_block_reward( + st, + // TODO revisit this sub, should probably be checked + &prior_bal - ¶ms.gas_reward, + params.ticket_count, + ); + let total_reward = block_rew + ¶ms.gas_reward; + + // Cap the penalty at the total reward value. + let penalty = std::cmp::min(params.penalty, total_reward.clone()); + // Reduce the payable reward by the penalty. + let rew_payable = total_reward - &penalty; + if (&rew_payable + &penalty) > prior_bal { + return Err(format!( + "reward payable {} + penalty {} exceeds balance {}", + rew_payable, penalty, prior_bal + )); + } + + // Record new reward into reward map. + if rew_payable > TokenAmount::zero() { + st.add_reward( + rt.store(), + &miner, + Reward { + start_epoch: rt.curr_epoch(), + end_epoch: rt.curr_epoch() + REWARD_VESTING_PERIOD, + value: rew_payable, + amount_withdrawn: TokenAmount::zero(), + vesting_function: REWARD_VESTING_FUNCTION, + }, + )?; + } + // + Ok(penalty) + }) + .map_err(|e| ActorError::new(ExitCode::ErrIllegalState, e))?; + + // Burn the penalty + rt.send( + &*BURNT_FUNDS_ACTOR_ADDR, + METHOD_SEND, + &Serialized::default(), + &penalty, + )?; + + Ok(()) } + /// Withdraw available funds from reward map - fn withdraw_reward(_rt: &RT, _miner_in: &Address) -> Result<(), ActorError> + fn withdraw_reward(rt: &RT, miner_in: Address) -> Result<(), ActorError> where BS: BlockStore, RT: Runtime, { - // TODO - todo!(); + let maddr = rt.resolve_address(&miner_in).ok_or_else(|| { + ActorError::new( + ExitCode::ErrIllegalArgument, + "failed to resolve input address".to_owned(), + ) + })?; + + let (owner, worker) = request_miner_control_addrs(rt, &maddr)?; + + rt.validate_immediate_caller_is([owner.clone(), worker].iter()); + + let withdrawable_reward = + rt.transaction::<_, Result<_, ActorError>, _>(|st: &mut State| { + let withdrawn = st + .withdraw_reward(rt.store(), &maddr, rt.curr_epoch()) + .map_err(|e| { + ActorError::new( + ExitCode::ErrIllegalState, + format!("failed to withdraw record: {}", e), + ) + })?; + Ok(withdrawn) + })?; + + rt.send( + &owner, + METHOD_SEND, + &Serialized::default(), + &withdrawable_reward, + )?; + Ok(()) + } + + /// Withdraw available funds from reward map + fn compute_block_reward(st: &State, balance: TokenAmount, ticket_count: u64) -> TokenAmount { + let treasury = balance - &st.reward_total; + let target_rew = BLOCK_REWARD_TARGET.clone() * ticket_count; + + std::cmp::min(target_rew, treasury) } } @@ -78,12 +201,11 @@ impl ActorCode for Actor { Ok(Serialized::default()) } Some(Method::AwardBlockReward) => { - check_empty_params(params)?; - Self::award_block_reward(rt)?; + Self::award_block_reward(rt, params.deserialize()?)?; Ok(Serialized::default()) } Some(Method::WithdrawReward) => { - Self::withdraw_reward(rt, ¶ms.deserialize()?)?; + Self::withdraw_reward(rt, params.deserialize()?)?; Ok(Serialized::default()) } _ => Err(rt.abort(ExitCode::SysErrInvalidMethod, "Invalid method")), diff --git a/vm/actor/src/builtin/reward/state.rs b/vm/actor/src/builtin/reward/state.rs index 72313f2c2b99..11c65501bc24 100644 --- a/vm/actor/src/builtin/reward/state.rs +++ b/vm/actor/src/builtin/reward/state.rs @@ -5,6 +5,7 @@ use crate::Multimap; use address::Address; use cid::Cid; use clock::ChainEpoch; +use encoding::Cbor; use ipld_blockstore::BlockStore; use num_bigint::biguint_ser::{BigUintDe, BigUintSer}; use num_derive::FromPrimitive; @@ -86,19 +87,20 @@ impl State { "withdrawable amount cannot exceed previous total" ); - // Replace old reward table with remaining rewards + // Regenerate amt for multimap with updated rewards rewards.remove_all(&key)?; for rew in remaining_rewards { rewards.add(key.clone(), rew)?; } - // Replace old values with updated rewards and total + // Update rewards multimap root and total self.reward_map = rewards.root()?; self.reward_total -= &withdrawable_sum; Ok(withdrawable_sum) } } +impl Cbor for State {} impl Serialize for State { fn serialize(&self, serializer: S) -> Result where diff --git a/vm/actor/src/builtin/reward/types.rs b/vm/actor/src/builtin/reward/types.rs new file mode 100644 index 000000000000..6c4cd52ce4cd --- /dev/null +++ b/vm/actor/src/builtin/reward/types.rs @@ -0,0 +1,62 @@ +// Copyright 2020 ChainSafe Systems +// SPDX-License-Identifier: Apache-2.0, MIT + +use super::VestingFunction; +use address::Address; +use clock::ChainEpoch; +use num_bigint::biguint_ser::{BigUintDe, BigUintSer}; +use num_bigint::BigUint; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use vm::TokenAmount; + +/// Number of token units in an abstract "FIL" token. +/// The network works purely in the indivisible token amounts. This constant converts to a fixed decimal with more +/// human-friendly scale. +pub const TOKEN_PRECISION: u64 = 1_000_000_000_000_000_000; + +lazy_static! { + /// Target reward released to each block winner. + pub static ref BLOCK_REWARD_TARGET: BigUint = BigUint::from(100u8) * TOKEN_PRECISION; +} + +pub(super) const REWARD_VESTING_FUNCTION: VestingFunction = VestingFunction::None; +pub(super) const REWARD_VESTING_PERIOD: ChainEpoch = 0; + +#[derive(Clone, Debug, PartialEq)] +pub struct AwardBlockRewardParams { + pub miner: Address, + pub penalty: TokenAmount, + pub gas_reward: TokenAmount, + pub ticket_count: u64, +} + +impl Serialize for AwardBlockRewardParams { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + ( + &self.miner, + BigUintSer(&self.penalty), + BigUintSer(&self.gas_reward), + &self.ticket_count, + ) + .serialize(serializer) + } +} + +impl<'de> Deserialize<'de> for AwardBlockRewardParams { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let (miner, BigUintDe(penalty), BigUintDe(gas_reward), ticket_count) = + Deserialize::deserialize(deserializer)?; + Ok(Self { + miner, + penalty, + gas_reward, + ticket_count, + }) + } +} diff --git a/vm/actor/src/builtin/shared.rs b/vm/actor/src/builtin/shared.rs new file mode 100644 index 000000000000..9b06d042937f --- /dev/null +++ b/vm/actor/src/builtin/shared.rs @@ -0,0 +1,19 @@ +// Copyright 2020 ChainSafe Systems +// SPDX-License-Identifier: Apache-2.0, MIT + +use address::Address; +use ipld_blockstore::BlockStore; +use runtime::Runtime; +use vm::ActorError; + +pub(crate) fn request_miner_control_addrs( + _rt: &RT, + _miner_addr: &Address, +) -> Result<(Address, Address), ActorError> +where + BS: BlockStore, + RT: Runtime, +{ + // TODO finish with miner actor + todo!() +} diff --git a/vm/actor/tests/multimap_test.rs b/vm/actor/tests/multimap_test.rs index 37eb4024ac2c..7a3a2da2494e 100644 --- a/vm/actor/tests/multimap_test.rs +++ b/vm/actor/tests/multimap_test.rs @@ -59,10 +59,10 @@ fn remove_all() { let arr: Amt = mm.get(&addr1.hash_key()).unwrap().unwrap(); assert_eq!(arr.get(1), Ok(Some(88))); - mm.remove_all(addr1.hash_key()).unwrap(); + mm.remove_all(&addr1.hash_key()).unwrap(); assert_eq!(mm.get::(&addr1.hash_key()), Ok(None)); assert!(mm.get::(&addr2.hash_key()).unwrap().is_some()); - mm.remove_all(addr2.hash_key()).unwrap(); + mm.remove_all(&addr2.hash_key()).unwrap(); assert_eq!(mm.get::(&addr2.hash_key()), Ok(None)); } diff --git a/vm/runtime/src/lib.rs b/vm/runtime/src/lib.rs index 576aebc73295..52ee241868b8 100644 --- a/vm/runtime/src/lib.rs +++ b/vm/runtime/src/lib.rs @@ -83,13 +83,13 @@ pub trait Runtime { /// 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. - fn send( + fn send( &self, to: &Address, method: MethodNum, params: &Serialized, value: &TokenAmount, - ) -> Result; + ) -> 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. diff --git a/vm/src/error.rs b/vm/src/error.rs index e42aec98bac1..a86ed027a992 100644 --- a/vm/src/error.rs +++ b/vm/src/error.rs @@ -19,6 +19,14 @@ pub struct ActorError { } impl ActorError { + pub fn new(exit_code: ExitCode, msg: String) -> Self { + Self { + fatal: false, + exit_code, + msg, + } + } + pub fn is_fatal(&self) -> bool { self.fatal } From 5012422314a45d2909798f56c4b8e198d3b2c0f1 Mon Sep 17 00:00:00 2001 From: austinabell Date: Sun, 29 Mar 2020 15:17:58 -0400 Subject: [PATCH 3/5] Fix logic checks --- vm/actor/src/builtin/reward/mod.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/vm/actor/src/builtin/reward/mod.rs b/vm/actor/src/builtin/reward/mod.rs index 71324383a380..3a594004ce47 100644 --- a/vm/actor/src/builtin/reward/mod.rs +++ b/vm/actor/src/builtin/reward/mod.rs @@ -64,7 +64,7 @@ impl Actor { RT: Runtime, { rt.validate_immediate_caller_is(std::iter::once(&*SYSTEM_ACTOR_ADDR)); - if rt.current_balance() >= params.gas_reward { + if rt.current_balance() < params.gas_reward { return Err(ActorError::new( ExitCode::ErrInsufficientFunds, format!( @@ -75,6 +75,13 @@ impl Actor { )); } + if params.ticket_count == 0 { + return Err(ActorError::new( + ExitCode::ErrIllegalArgument, + "cannot give block reward for zero tickets".to_owned(), + )); + } + let miner = rt.resolve_address(¶ms.miner).ok_or_else(|| { ActorError::new( ExitCode::ErrIllegalState, @@ -88,7 +95,6 @@ impl Actor { .transaction::<_, Result<_, String>, _>(|st: &mut State| { let block_rew = Self::compute_block_reward( st, - // TODO revisit this sub, should probably be checked &prior_bal - ¶ms.gas_reward, params.ticket_count, ); From ec5671236b7a37eb10ba631760fdd118d7e123ad Mon Sep 17 00:00:00 2001 From: austinabell Date: Tue, 7 Apr 2020 16:28:59 -0400 Subject: [PATCH 4/5] resolve conflicts --- vm/actor/src/builtin/reward/mod.rs | 69 ++++++++++++++---------------- 1 file changed, 31 insertions(+), 38 deletions(-) diff --git a/vm/actor/src/builtin/reward/mod.rs b/vm/actor/src/builtin/reward/mod.rs index ad6e3d6d8052..2856a8684fd5 100644 --- a/vm/actor/src/builtin/reward/mod.rs +++ b/vm/actor/src/builtin/reward/mod.rs @@ -39,12 +39,12 @@ impl Method { pub struct Actor; impl Actor { /// Constructor for Reward actor - fn constructor(rt: &RT) -> Result<(), ActorError> + fn constructor(rt: &mut RT) -> Result<(), ActorError> where BS: BlockStore, RT: Runtime, { - rt.validate_immediate_caller_is(std::iter::once(&*SYSTEM_ACTOR_ADDR)); + rt.validate_immediate_caller_is(std::iter::once(&*SYSTEM_ACTOR_ADDR))?; let empty_root = Multimap::new(rt.store()).root().map_err(|e| { ActorError::new( @@ -53,24 +53,27 @@ impl Actor { ) })?; - rt.create(&State::new(empty_root)); + rt.create(&State::new(empty_root))?; Ok(()) } /// Mints a reward and puts into state reward map - fn award_block_reward(rt: &RT, params: AwardBlockRewardParams) -> Result<(), ActorError> + fn award_block_reward( + rt: &mut RT, + params: AwardBlockRewardParams, + ) -> Result<(), ActorError> where BS: BlockStore, RT: Runtime, { - rt.validate_immediate_caller_is(std::iter::once(&*SYSTEM_ACTOR_ADDR)); - if rt.current_balance() < params.gas_reward { + rt.validate_immediate_caller_is(std::iter::once(&*SYSTEM_ACTOR_ADDR))?; + let balance = rt.current_balance()?; + if balance < params.gas_reward { return Err(ActorError::new( ExitCode::ErrInsufficientFunds, format!( "actor current balance {} insufficient to pay gas reward {}", - rt.current_balance(), - params.gas_reward + balance, params.gas_reward ), )); } @@ -82,17 +85,13 @@ impl Actor { )); } - let miner = rt.resolve_address(¶ms.miner).ok_or_else(|| { - ActorError::new( - ExitCode::ErrIllegalState, - "failed to resolve given owner address".to_owned(), - ) - })?; + let miner = rt.resolve_address(¶ms.miner)?; - let prior_bal = rt.current_balance(); + let prior_bal = rt.current_balance()?; + let cur_epoch = rt.curr_epoch(); let penalty: TokenAmount = rt - .transaction::<_, Result<_, String>, _>(|st: &mut State| { + .transaction::<_, Result<_, String>, _>(|st: &mut State, bs| { let block_rew = Self::compute_block_reward( st, &prior_bal - ¶ms.gas_reward, @@ -114,11 +113,11 @@ impl Actor { // Record new reward into reward map. if rew_payable > TokenAmount::zero() { st.add_reward( - rt.store(), + bs, &miner, Reward { - start_epoch: rt.curr_epoch(), - end_epoch: rt.curr_epoch() + REWARD_VESTING_PERIOD, + start_epoch: cur_epoch, + end_epoch: cur_epoch + REWARD_VESTING_PERIOD, value: rew_payable, amount_withdrawn: TokenAmount::zero(), vesting_function: REWARD_VESTING_FUNCTION, @@ -127,7 +126,7 @@ impl Actor { } // Ok(penalty) - }) + })? .map_err(|e| ActorError::new(ExitCode::ErrIllegalState, e))?; // Burn the penalty @@ -142,34 +141,28 @@ impl Actor { } /// Withdraw available funds from reward map - fn withdraw_reward(rt: &RT, miner_in: Address) -> Result<(), ActorError> + fn withdraw_reward(rt: &mut RT, miner_in: Address) -> Result<(), ActorError> where BS: BlockStore, RT: Runtime, { - let maddr = rt.resolve_address(&miner_in).ok_or_else(|| { - ActorError::new( - ExitCode::ErrIllegalArgument, - "failed to resolve input address".to_owned(), - ) - })?; + let maddr = rt.resolve_address(&miner_in)?; let (owner, worker) = request_miner_control_addrs(rt, &maddr)?; - rt.validate_immediate_caller_is([owner.clone(), worker].iter()); + rt.validate_immediate_caller_is([owner.clone(), worker].iter())?; + let cur_epoch = rt.curr_epoch(); let withdrawable_reward = - rt.transaction::<_, Result<_, ActorError>, _>(|st: &mut State| { - let withdrawn = st - .withdraw_reward(rt.store(), &maddr, rt.curr_epoch()) - .map_err(|e| { - ActorError::new( - ExitCode::ErrIllegalState, - format!("failed to withdraw record: {}", e), - ) - })?; + rt.transaction::<_, Result<_, ActorError>, _>(|st: &mut State, bs| { + let withdrawn = st.withdraw_reward(bs, &maddr, cur_epoch).map_err(|e| { + ActorError::new( + ExitCode::ErrIllegalState, + format!("failed to withdraw record: {}", e), + ) + })?; Ok(withdrawn) - })?; + })??; rt.send( &owner, From d78eb174250ce6821b88764469a03fd68ed62149 Mon Sep 17 00:00:00 2001 From: austinabell Date: Tue, 7 Apr 2020 16:37:34 -0400 Subject: [PATCH 5/5] Merge master and resolve conflicts --- vm/actor/src/builtin/reward/state.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vm/actor/src/builtin/reward/state.rs b/vm/actor/src/builtin/reward/state.rs index d3b77780bdaa..3cb94255877e 100644 --- a/vm/actor/src/builtin/reward/state.rs +++ b/vm/actor/src/builtin/reward/state.rs @@ -60,7 +60,7 @@ impl State { // Iterate rewards, accumulate total and remaining reward state let mut remaining_rewards = Vec::new(); let mut withdrawable_sum = TokenAmount::from(0u8); - rewards.for_each(&key, |_, reward: Reward| { + rewards.for_each(&key, |_, reward: &Reward| { let unlocked = reward.amount_vested(curr_epoch); let withdrawable = unlocked .checked_sub(&reward.amount_withdrawn) @@ -75,7 +75,7 @@ impl State { vesting_function: reward.vesting_function, start_epoch: reward.start_epoch, end_epoch: reward.end_epoch, - value: reward.value, + value: reward.value.clone(), amount_withdrawn: unlocked, }); }