From 17a5d6fd333950dbc7be3b988225e2736c4e1259 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Mon, 16 Sep 2019 14:20:16 +0300 Subject: [PATCH 01/23] types and dependencies --- Cargo.toml | 4 +++- src/lib.rs | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9e41cb5e1a..1e7fb82470 100755 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,9 +28,10 @@ git = 'https://github.com/paritytech/substrate.git' package = 'sr-io' rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b' -[dependencues.minting] +[dependencies.minting] default_features = false git = 'https://github.com/mnaamani/substrate-token-minting' +package = 'substrate-token-mint-module' branch = 'token-minting' [dev-dependencies] @@ -50,4 +51,5 @@ std = [ 'system/std', 'balances/std', 'timestamp/std', + 'minting/std', ] diff --git a/src/lib.rs b/src/lib.rs index f8c805389c..db68af1055 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,10 +8,67 @@ use srml_support::{ decl_event, decl_module, decl_storage, dispatch, ensure, StorageMap, StorageValue, }; -use minting; +use minting::{self, BalanceOf, MintId}; use system; -pub trait Trait: system::Trait + minting::Trait + Sized {} +/// Type of identifier for recipients. +type RecipientId = u64; + +/// Type for identifier for relationship representing that a recipient recieves recurring reward from a token mint +type RewardRelationshipId = u64; + +pub trait Trait: system::Trait + minting::Trait + Sized { + type PayoutStatusHandler: PayoutStatusHandler; +} + +/// Handler for aftermath of a payout attempt +pub trait PayoutStatusHandler { + fn payout_status( + id: RewardRelationshipId, + status: bool, + destination_account: T::AccountId, + amount: BalanceOf, + ); +} + +// A recipient of recurring rewards +pub struct Recipient { + /// stats + // Total payout received by this recipient + total_reward_received: BalanceOf, + + // Total payout missed for this recipient + total_reward_missed: BalanceOf, +} + +pub struct RewardRelationship { + // Identifier for receiver + recipient: RecipientId, + + // Identifier for reward source + mint_id: MintId, + + // Destination account for reward + account: T::AccountId, + + // Paid out for + amount_per_payout: BalanceOf, + + // When set, identifies block when next payout should be processed, + // otherwise there is no pending payout + next_payment_in_block: Option, + + // When set, will be the basis for automatically setting next payment, + // otherwise any upcoming payout will be a one off. + payout_interval: Option, + + /// stats + // Total payout received in this relationship + total_reward_received: BalanceOf, + + // Total payout failed in this relationship + total_reward_missed: BalanceOf, +} decl_storage! { trait Store for Module as RecurringReward { From 4b4c902282e00a1adda241554c30ca8fb5af667f Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Mon, 16 Sep 2019 16:05:17 +0300 Subject: [PATCH 02/23] state --- src/lib.rs | 47 ++++++++++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index db68af1055..d7385654d4 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,18 +4,18 @@ use rstd::prelude::*; use codec::{Decode, Encode}; -use srml_support::{ - decl_event, decl_module, decl_storage, dispatch, ensure, StorageMap, StorageValue, -}; +use srml_support::{decl_module, decl_storage, ensure, StorageMap, StorageValue}; use minting::{self, BalanceOf, MintId}; use system; /// Type of identifier for recipients. type RecipientId = u64; +pub const FIRST_RECIPIENT_ID: RecipientId = 1; /// Type for identifier for relationship representing that a recipient recieves recurring reward from a token mint type RewardRelationshipId = u64; +pub const FIRST_REWARD_RELATIONSHIP_ID: RewardRelationshipId = 1; pub trait Trait: system::Trait + minting::Trait + Sized { type PayoutStatusHandler: PayoutStatusHandler; @@ -23,25 +23,32 @@ pub trait Trait: system::Trait + minting::Trait + Sized { /// Handler for aftermath of a payout attempt pub trait PayoutStatusHandler { - fn payout_status( + fn payout_succeeded( + id: RewardRelationshipId, + destination_account: T::AccountId, + amount: BalanceOf, + ); + + fn payout_failed( id: RewardRelationshipId, - status: bool, destination_account: T::AccountId, amount: BalanceOf, ); } // A recipient of recurring rewards -pub struct Recipient { - /// stats +#[derive(Encode, Decode, Copy, Clone, Debug, Default)] +pub struct Recipient { + // stats // Total payout received by this recipient - total_reward_received: BalanceOf, + total_reward_received: Balance, // Total payout missed for this recipient - total_reward_missed: BalanceOf, + total_reward_missed: Balance, } -pub struct RewardRelationship { +#[derive(Encode, Decode, Copy, Clone, Debug, Default)] +pub struct RewardRelationship { // Identifier for receiver recipient: RecipientId, @@ -49,30 +56,36 @@ pub struct RewardRelationship { mint_id: MintId, // Destination account for reward - account: T::AccountId, + account: AccountId, // Paid out for - amount_per_payout: BalanceOf, + amount_per_payout: Balance, // When set, identifies block when next payout should be processed, // otherwise there is no pending payout - next_payment_in_block: Option, + next_payment_in_block: Option, // When set, will be the basis for automatically setting next payment, // otherwise any upcoming payout will be a one off. - payout_interval: Option, + payout_interval: Option, - /// stats + // stats // Total payout received in this relationship - total_reward_received: BalanceOf, + total_reward_received: Balance, // Total payout failed in this relationship - total_reward_missed: BalanceOf, + total_reward_missed: Balance, } decl_storage! { trait Store for Module as RecurringReward { + Recipients get(rewards): map RecipientId => Recipient>; + + NextRecipientId get(next_recipient_id): RecipientId = FIRST_RECIPIENT_ID; + + RewardRelationships get(reward_relationships): map RewardRelationshipId => RewardRelationship, T::BlockNumber>; + NextRewardRelationshipId get(next_reward_relationship_id): RewardRelationshipId = FIRST_REWARD_RELATIONSHIP_ID; } } From c2d08ecc3e09326f62701e2f369170bf5b3f09d5 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Tue, 17 Sep 2019 10:12:57 +0300 Subject: [PATCH 03/23] add_relationship --- src/lib.rs | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 105 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index d7385654d4..c53fb6f5b5 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,6 +4,7 @@ use rstd::prelude::*; use codec::{Decode, Encode}; +use runtime_primitives::traits::Zero; use srml_support::{decl_module, decl_storage, ensure, StorageMap, StorageValue}; use minting::{self, BalanceOf, MintId}; @@ -63,7 +64,7 @@ pub struct RewardRelationship { // When set, identifies block when next payout should be processed, // otherwise there is no pending payout - next_payment_in_block: Option, + next_payment_at_block: Option, // When set, will be the basis for automatically setting next payment, // otherwise any upcoming payout will be a one off. @@ -92,8 +93,109 @@ decl_storage! { decl_module! { pub struct Module for enum Call where origin: T::Origin { - fn on_finalize(now: T::BlockNumber) {} + fn on_finalize(now: T::BlockNumber) { + /* + For all RecievesFromSource found in rewardRelationships where next_payment_in_block is set and matches current block height, a call to pay_reward is made for the suitable amount, recipient and source. The next_payment_in_block is updated based on payout_interval. + + If the call succeeds, total_reward_received is incremented on both + recipient and dependency with amount_per_payout, and a call to T::PayoutStatusHandler is made. Otherwise, analogous steps for failure. + */ + } } } -impl Module {} +pub enum RewardsError { + RecipientNotFound, + RewardSourceNotFound, + BlockNumberInPast, +} + +pub enum NextPaymentSchedule { + Absolute(BlockNumber), + Relative(BlockNumber), +} + +impl Module { + /* Adds a new Recipient recipient to recipients, with identifier equal to nextRecipientId, which is also incremented, and returns the new recipient identifier. */ + pub fn add_recipient() -> RecipientId { + let next_id = Self::next_recipient_id(); + NextRecipientId::mutate(|n| { + *n += 1; + }); + >::insert(&next_id, Recipient::default()); + next_id + } + + /// Removes a mapping from reward_recipients based on the given identifier. + pub fn remove_recipient(id: RecipientId) { + >::remove(&id); + } + + // Adds a new RewardRelationship to rewardRelationships, for a given source, recipient, account, etc., with identifier equal to current nextRewardRelationshipId. Also increments nextRewardRelationshipId. + pub fn add_reward_relationship( + mint_id: MintId, + recipient: RecipientId, + account: T::AccountId, + amount_per_payout: BalanceOf, + next_payment_schedule: Option>, + payout_interval: Option, + ) -> Result { + ensure!( + >::mint_exists(mint_id), + RewardsError::RewardSourceNotFound + ); + + let next_payment_at_block = match next_payment_schedule { + Some(schedule) => match schedule { + NextPaymentSchedule::Absolute(blocknumber) => { + ensure!( + blocknumber > >::block_number(), + RewardsError::BlockNumberInPast + ); + Some(blocknumber) + } + NextPaymentSchedule::Relative(blocknumber) => { + Some(>::block_number() + blocknumber) + } + }, + None => match payout_interval { + Some(interval) => Some(>::block_number() + interval), + None => { + // No payouts will be made unless relationship is updated in future and next_payment_in_block + // is set! should this be allowed? + None + } + }, + }; + + let relationship = RewardRelationship { + mint_id, + recipient, + account, + amount_per_payout, + next_payment_at_block, + payout_interval, + total_reward_received: Zero::zero(), + total_reward_missed: Zero::zero(), + }; + + let relationship_id = Self::next_reward_relationship_id(); + NextRewardRelationshipId::mutate(|n| { + *n += 1; + }); + >::insert(relationship_id, relationship); + Ok(relationship_id) + } + + // Removes a mapping from depenencies based on given identifier. + pub fn remove_reward_relationship() {} + + /* + For RecievesFromSource found in rewardRelationships with given identifier, new valus for the following can be set + account + amount_per_payout + next_payment_in_block + payout_interval + */ + pub fn set_reward_relationship() {} +} From 733550a238840d11ba2b63f51d25ffe26f380cf1 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Tue, 17 Sep 2019 11:26:06 +0300 Subject: [PATCH 04/23] set_reward_relationship --- src/lib.rs | 54 +++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c53fb6f5b5..3cbbd5af24 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -108,6 +108,7 @@ pub enum RewardsError { RecipientNotFound, RewardSourceNotFound, BlockNumberInPast, + RewardRelationshipNotFound, } pub enum NextPaymentSchedule { @@ -187,15 +188,46 @@ impl Module { Ok(relationship_id) } - // Removes a mapping from depenencies based on given identifier. - pub fn remove_reward_relationship() {} - - /* - For RecievesFromSource found in rewardRelationships with given identifier, new valus for the following can be set - account - amount_per_payout - next_payment_in_block - payout_interval - */ - pub fn set_reward_relationship() {} + // Removes a mapping from reward relashionships based on given identifier. + pub fn remove_reward_relationship(id: RewardRelationshipId) { + >::remove(&id); + } + + // For reward relationship found with given identifier, new values can be set for + // account, payout, block number when next payout will be made and the new interval after + // the next scheduled payout. All values are optional, but updating values are combined in this + // single method to ensure atomic updates. + pub fn set_reward_relationship( + id: RewardRelationshipId, + account: Option, + payout: Option>, + next_payment_at: Option>, + payout_interval: Option>, + ) -> Result<(), RewardsError> { + ensure!( + >::exists(&id), + RewardsError::RewardRelationshipNotFound + ); + let mut relationship = Self::reward_relationships(&id); + + if let Some(account) = account { + relationship.account = account; + } + if let Some(payout) = payout { + relationship.amount_per_payout = payout; + } + if let Some(next_payout_at_block) = next_payment_at { + if let Some(blocknumber) = next_payout_at_block { + ensure!( + blocknumber > >::block_number(), + RewardsError::BlockNumberInPast + ); + } + relationship.next_payment_at_block = next_payout_at_block; + } + if let Some(payout_interval) = payout_interval { + relationship.payout_interval = payout_interval; + } + Ok(()) + } } From 3430c5757420cffe637a92d348a0b43523f6ee0b Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Tue, 17 Sep 2019 13:56:14 +0300 Subject: [PATCH 05/23] do payouts --- src/lib.rs | 93 ++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 84 insertions(+), 9 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 3cbbd5af24..3ee9b859e1 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -26,13 +26,13 @@ pub trait Trait: system::Trait + minting::Trait + Sized { pub trait PayoutStatusHandler { fn payout_succeeded( id: RewardRelationshipId, - destination_account: T::AccountId, + destination_account: &T::AccountId, amount: BalanceOf, ); fn payout_failed( id: RewardRelationshipId, - destination_account: T::AccountId, + destination_account: &T::AccountId, amount: BalanceOf, ); } @@ -80,7 +80,7 @@ pub struct RewardRelationship { decl_storage! { trait Store for Module as RecurringReward { - Recipients get(rewards): map RecipientId => Recipient>; + Recipients get(recipients): map RecipientId => Recipient>; NextRecipientId get(next_recipient_id): RecipientId = FIRST_RECIPIENT_ID; @@ -94,12 +94,7 @@ decl_module! { pub struct Module for enum Call where origin: T::Origin { fn on_finalize(now: T::BlockNumber) { - /* - For all RecievesFromSource found in rewardRelationships where next_payment_in_block is set and matches current block height, a call to pay_reward is made for the suitable amount, recipient and source. The next_payment_in_block is updated based on payout_interval. - - If the call succeeds, total_reward_received is incremented on both - recipient and dependency with amount_per_payout, and a call to T::PayoutStatusHandler is made. Otherwise, analogous steps for failure. - */ + Self::do_payouts(now); } } } @@ -230,4 +225,84 @@ impl Module { } Ok(()) } + + /* + For all relationships where next_payment_at_block is set and matches current block height, + a call to pay_reward is made for the suitable amount, recipient and source. + The next_payment_in_block is updated based on payout_interval. + If the call succeeds, total_reward_received is incremented on both + recipient and dependency with amount_per_payout, and a call to T::PayoutStatusHandler is made. + Otherwise, analogous steps for failure. + */ + fn do_payouts(now: T::BlockNumber) { + for relationship_id in FIRST_REWARD_RELATIONSHIP_ID..Self::next_reward_relationship_id() { + if !>::exists(&relationship_id) { + continue; + } + let mut relationship = Self::reward_relationships(&relationship_id); + + // recipient can be removed independantly of relationship, so ensure we have a recipient + if !>::exists(&relationship.recipient) { + continue; + } + let mut recipient = Self::recipients(&relationship.recipient); + + if let Some(blocknumber) = relationship.next_payment_at_block { + if blocknumber != now { + return; + } + + // Add the missed payout and try to pay those in addition to scheduled payout? + // let payout = relationship.total_reward_missed + relationship.amount_per_payout; + let payout = relationship.amount_per_payout; + + // try to make payment + if >::transfer_exact_tokens( + relationship.mint_id, + payout, + &relationship.account, + ) + .is_err() + { + // add only newly scheduled payout to total missed payout + relationship.total_reward_missed += relationship.amount_per_payout; + + // update recipient stats + recipient.total_reward_missed += relationship.amount_per_payout; + + T::PayoutStatusHandler::payout_failed( + relationship_id, + &relationship.account, + payout, + ); + } else { + // update payout received stats + relationship.total_reward_received += payout; + recipient.total_reward_received += payout; + + // update missed payout stats + // if relationship.total_reward_missed != Zero::zero() { + // // update recipient stats + // recipient.total_reward_missed -= relationship.total_reward_missed; + + // // clear missed reward on relationship + // relationship.total_reward_missed = Zero::zero(); + // } + T::PayoutStatusHandler::payout_succeeded( + relationship_id, + &relationship.account, + payout, + ); + } + + // update next payout blocknumber at interval if set + if let Some(payout_interval) = relationship.payout_interval { + relationship.next_payment_at_block = Some(now + payout_interval); + } + + // update relationship to storage + >::insert(&relationship_id, relationship); + } + } + } } From efe9c9995a19f5e3424b2d8c8818ae518573ca0e Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Tue, 17 Sep 2019 19:01:47 +0300 Subject: [PATCH 06/23] mock and tests setup --- src/lib.rs | 19 ++++++++++ src/mock.rs | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/tests.rs | 20 +++++++++++ 3 files changed, 139 insertions(+) create mode 100644 src/mock.rs create mode 100644 src/tests.rs diff --git a/src/lib.rs b/src/lib.rs index 3ee9b859e1..2c92e04ae1 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -10,6 +10,9 @@ use srml_support::{decl_module, decl_storage, ensure, StorageMap, StorageValue}; use minting::{self, BalanceOf, MintId}; use system; +mod mock; +mod tests; + /// Type of identifier for recipients. type RecipientId = u64; pub const FIRST_RECIPIENT_ID: RecipientId = 1; @@ -37,6 +40,22 @@ pub trait PayoutStatusHandler { ); } +impl PayoutStatusHandler for () { + fn payout_succeeded( + _id: RewardRelationshipId, + _destination_account: &T::AccountId, + _amount: BalanceOf, + ) { + } + + fn payout_failed( + _id: RewardRelationshipId, + _destination_account: &T::AccountId, + _amount: BalanceOf, + ) { + } +} + // A recipient of recurring rewards #[derive(Encode, Decode, Copy, Clone, Debug, Default)] pub struct Recipient { diff --git a/src/mock.rs b/src/mock.rs new file mode 100644 index 0000000000..1e43338c1a --- /dev/null +++ b/src/mock.rs @@ -0,0 +1,100 @@ +#![cfg(test)] + +use crate::*; + +use primitives::{Blake2Hasher, H256}; + +use crate::{Module, Trait}; +use balances; +use minting; +use runtime_primitives::{ + testing::Header, + traits::{BlakeTwo256, IdentityLookup}, + Perbill, +}; +use srml_support::{impl_outer_origin, parameter_types}; + +impl_outer_origin! { + pub enum Origin for Test {} +} + +// Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. +#[derive(Clone, PartialEq, Eq, Debug)] +pub struct Test; +parameter_types! { + pub const BlockHashCount: u64 = 250; + pub const MaximumBlockWeight: u32 = 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; + pub const AvailableBlockRatio: Perbill = Perbill::one(); + pub const MinimumPeriod: u64 = 5; +} + +impl system::Trait for Test { + type Origin = Origin; + type Index = u64; + type BlockNumber = u64; + type Call = (); + type Hash = H256; + type Hashing = BlakeTwo256; + type AccountId = u64; + type Lookup = IdentityLookup; + type Header = Header; + type WeightMultiplierUpdate = (); + type Event = (); + type BlockHashCount = BlockHashCount; + type MaximumBlockWeight = MaximumBlockWeight; + type MaximumBlockLength = MaximumBlockLength; + type AvailableBlockRatio = AvailableBlockRatio; + type Version = (); +} + +parameter_types! { + pub const ExistentialDeposit: u32 = 0; + pub const TransferFee: u32 = 0; + pub const CreationFee: u32 = 0; + pub const TransactionBaseFee: u32 = 1; + pub const TransactionByteFee: u32 = 0; + pub const InitialMembersBalance: u64 = 2000; +} + +impl balances::Trait for Test { + /// The type for recording an account's balance. + type Balance = u64; + /// What to do if an account's free balance gets zeroed. + type OnFreeBalanceZero = (); + /// What to do if a new account is created. + type OnNewAccount = (); + /// The ubiquitous event type. + type Event = (); + + type TransactionPayment = (); + type DustRemoval = (); + type TransferPayment = (); + type ExistentialDeposit = ExistentialDeposit; + type TransferFee = TransferFee; + type CreationFee = CreationFee; + type TransactionBaseFee = TransactionBaseFee; + type TransactionByteFee = TransactionByteFee; + type WeightToFee = (); +} + +impl Trait for Test { + type PayoutStatusHandler = (); +} + +impl minting::Trait for Test { + type Currency = Balances; +} + +pub fn build_test_externalities() -> runtime_io::TestExternalities { + let t = system::GenesisConfig::default() + .build_storage::() + .unwrap(); + + t.into() +} + +pub type System = system::Module; +pub type Balances = balances::Module; +pub type Rewards = Module; +pub type Minting = minting::Module; diff --git a/src/tests.rs b/src/tests.rs new file mode 100644 index 0000000000..2d5bd73e3e --- /dev/null +++ b/src/tests.rs @@ -0,0 +1,20 @@ +#![cfg(test)] + +use super::*; +use crate::mock::*; + +use runtime_io::with_externalities; + +fn create_new_mint_with_capacity(capacity: u64) -> MintId { + let mint_id = Minting::add_mint(capacity, None).ok().unwrap(); + assert!(Minting::mint_exists(mint_id)); + assert!(Minting::mint_has_capacity(mint_id, capacity)); + mint_id +} + +#[test] +fn adding_and_removing_mints() { + with_externalities(&mut build_test_externalities(), || { + let mint_id = create_new_mint_with_capacity(1000000); + }); +} From aac781c773595e94a7f0120ccc01f36e3f128a42 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Wed, 18 Sep 2019 10:02:55 +0300 Subject: [PATCH 07/23] basic tests --- src/lib.rs | 3 ++ src/tests.rs | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 2c92e04ae1..2f52866ce9 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -118,6 +118,7 @@ decl_module! { } } +#[derive(Eq, PartialEq, Debug)] pub enum RewardsError { RecipientNotFound, RewardSourceNotFound, @@ -317,6 +318,8 @@ impl Module { // update next payout blocknumber at interval if set if let Some(payout_interval) = relationship.payout_interval { relationship.next_payment_at_block = Some(now + payout_interval); + } else { + relationship.next_payment_at_block = None; } // update relationship to storage diff --git a/src/tests.rs b/src/tests.rs index 2d5bd73e3e..b9748dec33 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -2,6 +2,7 @@ use super::*; use crate::mock::*; +use srml_support::traits::Currency; use runtime_io::with_externalities; @@ -13,8 +14,97 @@ fn create_new_mint_with_capacity(capacity: u64) -> MintId { } #[test] -fn adding_and_removing_mints() { +fn adding_recipients() { with_externalities(&mut build_test_externalities(), || { + let next_id = Rewards::next_recipient_id(); + assert!(!>::exists(&next_id)); + let recipient_id = Rewards::add_recipient(); + assert!(>::exists(&next_id)); + assert_eq!(recipient_id, next_id); + assert_eq!(Rewards::next_recipient_id(), next_id + 1); + }); +} + +#[test] +fn adding_relationships() { + with_externalities(&mut build_test_externalities(), || { + let recipient_account: u64 = 1; + let mint_id = create_new_mint_with_capacity(1000000); + let recipient_id = Rewards::add_recipient(); + let interval: u64 = 600; + let next_payment_at: u64 = 2222; + let payout = 100; + + let next_relationship_id = Rewards::next_reward_relationship_id(); + let relationship = Rewards::add_reward_relationship( + mint_id, + recipient_id, + recipient_account, + payout, + Some(NextPaymentSchedule::Absolute(next_payment_at)), + Some(interval), + ); + assert!(relationship.is_ok()); + let relationship_id = relationship.ok().unwrap(); + assert_eq!(relationship_id, next_relationship_id); + assert_eq!( + Rewards::next_reward_relationship_id(), + next_relationship_id + 1 + ); + assert!(>::exists(&relationship_id)); + let relationship = Rewards::reward_relationships(&relationship_id); + assert_eq!(relationship.next_payment_at_block, Some(next_payment_at)); + assert_eq!(relationship.amount_per_payout, payout); + assert_eq!(relationship.mint_id, mint_id); + assert_eq!(relationship.account, recipient_account); + assert_eq!(relationship.payout_interval, Some(interval)); + + // mint doesn't exist + assert_eq!(Rewards::add_reward_relationship( + 111, + recipient_id, + recipient_account, + 100, + None, + None, + ).expect_err("should fail if mint doesn't exist"), RewardsError::RewardSourceNotFound); + }); +} + +#[test] +fn rewards_one_off_payout() { + with_externalities(&mut build_test_externalities(), || { + System::set_block_number(10000); + let recipient_account: u64 = 1; + Balances::deposit_creating(&recipient_account, 400); let mint_id = create_new_mint_with_capacity(1000000); + let recipient_id = Rewards::add_recipient(); + let payout: u64 = 1000; + let payout_after: u64 = 2222; + let expected_payout_at = System::block_number() + payout_after; + let relationship = Rewards::add_reward_relationship( + mint_id, + recipient_id, + recipient_account, + payout, + Some(NextPaymentSchedule::Relative(payout_after)), + None, + ); + assert!(relationship.is_ok()); + let relationship_id = relationship.ok().unwrap(); + + let relationship = Rewards::reward_relationships(&relationship_id); + assert_eq!(relationship.next_payment_at_block, Some(expected_payout_at)); + + let starting_balance = Balances::free_balance(&recipient_account); + Rewards::do_payouts(expected_payout_at); + assert_eq!( + Balances::free_balance(&recipient_account), + starting_balance + payout + ); + + let relationship = Rewards::reward_relationships(&relationship_id); + assert_eq!(relationship.total_reward_received, payout); + assert_eq!(relationship.next_payment_at_block, None); }); } From 245b1134bd089ac46a230d0540c313c3fdbde2a7 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Wed, 18 Sep 2019 10:48:17 +0300 Subject: [PATCH 08/23] more tests --- src/lib.rs | 3 +- src/tests.rs | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 103 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 2f52866ce9..56dd85a62d 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -322,7 +322,8 @@ impl Module { relationship.next_payment_at_block = None; } - // update relationship to storage + // update relationship and recipient to storage + >::insert(&relationship.recipient, recipient); >::insert(&relationship_id, relationship); } } diff --git a/src/tests.rs b/src/tests.rs index b9748dec33..90a53b48df 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -72,11 +72,11 @@ fn adding_relationships() { } #[test] -fn rewards_one_off_payout() { +fn one_off_payout() { with_externalities(&mut build_test_externalities(), || { System::set_block_number(10000); let recipient_account: u64 = 1; - Balances::deposit_creating(&recipient_account, 400); + let _ = Balances::deposit_creating(&recipient_account, 400); let mint_id = create_new_mint_with_capacity(1000000); let recipient_id = Rewards::add_recipient(); let payout: u64 = 1000; @@ -97,6 +97,13 @@ fn rewards_one_off_payout() { assert_eq!(relationship.next_payment_at_block, Some(expected_payout_at)); let starting_balance = Balances::free_balance(&recipient_account); + + // try to catch 'off by one' bugs + Rewards::do_payouts(expected_payout_at - 1); + assert_eq!(Balances::free_balance(&recipient_account), starting_balance); + Rewards::do_payouts(expected_payout_at + 1); + assert_eq!(Balances::free_balance(&recipient_account), starting_balance); + Rewards::do_payouts(expected_payout_at); assert_eq!( Balances::free_balance(&recipient_account), @@ -106,5 +113,97 @@ fn rewards_one_off_payout() { let relationship = Rewards::reward_relationships(&relationship_id); assert_eq!(relationship.total_reward_received, payout); assert_eq!(relationship.next_payment_at_block, None); + + let recipient = Rewards::recipients(&recipient_id); + assert_eq!(recipient.total_reward_received, payout); + }); +} + +#[test] +fn recurring_payout() { + with_externalities(&mut build_test_externalities(), || { + System::set_block_number(10000); + let recipient_account: u64 = 1; + let _ = Balances::deposit_creating(&recipient_account, 400); + let mint_id = create_new_mint_with_capacity(1000000); + let recipient_id = Rewards::add_recipient(); + let payout: u64 = 1000; + let payout_after: u64 = 2222; + let expected_payout_at = System::block_number() + payout_after; + let interval: u64 = 600; + let relationship = Rewards::add_reward_relationship( + mint_id, + recipient_id, + recipient_account, + payout, + Some(NextPaymentSchedule::Relative(payout_after)), + Some(interval), + ); + assert!(relationship.is_ok()); + let relationship_id = relationship.ok().unwrap(); + + let relationship = Rewards::reward_relationships(&relationship_id); + assert_eq!(relationship.next_payment_at_block, Some(expected_payout_at)); + + let starting_balance = Balances::free_balance(&recipient_account); + + let number_of_payouts = 3; + for i in 0..number_of_payouts { + Rewards::do_payouts(expected_payout_at + interval * i); + } + + assert_eq!( + Balances::free_balance(&recipient_account), + starting_balance + payout * number_of_payouts + ); + + let relationship = Rewards::reward_relationships(&relationship_id); + assert_eq!( + relationship.total_reward_received, + payout * number_of_payouts + ); + + let recipient = Rewards::recipients(&recipient_id); + assert_eq!(recipient.total_reward_received, payout * number_of_payouts); + }); +} + +#[test] +fn track_missed_payouts() { + with_externalities(&mut build_test_externalities(), || { + System::set_block_number(10000); + let recipient_account: u64 = 1; + let _ = Balances::deposit_creating(&recipient_account, 400); + let mint_id = create_new_mint_with_capacity(0); + let recipient_id = Rewards::add_recipient(); + let payout: u64 = 1000; + let payout_after: u64 = 2222; + let expected_payout_at = System::block_number() + payout_after; + let relationship = Rewards::add_reward_relationship( + mint_id, + recipient_id, + recipient_account, + payout, + Some(NextPaymentSchedule::Relative(payout_after)), + None, + ); + assert!(relationship.is_ok()); + let relationship_id = relationship.ok().unwrap(); + + let relationship = Rewards::reward_relationships(&relationship_id); + assert_eq!(relationship.next_payment_at_block, Some(expected_payout_at)); + + let starting_balance = Balances::free_balance(&recipient_account); + + Rewards::do_payouts(expected_payout_at); + assert_eq!(Balances::free_balance(&recipient_account), starting_balance); + + let relationship = Rewards::reward_relationships(&relationship_id); + assert_eq!(relationship.total_reward_received, 0); + assert_eq!(relationship.total_reward_missed, payout); + + let recipient = Rewards::recipients(&recipient_id); + assert_eq!(recipient.total_reward_received, 0); + assert_eq!(recipient.total_reward_missed, payout); }); } From 9979caecfbb6e173073efe6846ff138432bb457a Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Wed, 18 Sep 2019 10:49:51 +0300 Subject: [PATCH 09/23] fix update state when setting new relationship values --- src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib.rs b/src/lib.rs index 56dd85a62d..bc4fa854d6 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -243,6 +243,7 @@ impl Module { if let Some(payout_interval) = payout_interval { relationship.payout_interval = payout_interval; } + >::insert(&id, relationship); Ok(()) } From 53daa5bc4a19e83604ba87a4f4031531d5378d73 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Wed, 18 Sep 2019 12:44:53 +0300 Subject: [PATCH 10/23] tests: MockStatusHandler --- src/mock.rs | 44 +++++++++++++++++++++++++++++++++++++++++++- src/tests.rs | 6 ++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/mock.rs b/src/mock.rs index 1e43338c1a..b93fe15191 100644 --- a/src/mock.rs +++ b/src/mock.rs @@ -14,10 +14,50 @@ use runtime_primitives::{ }; use srml_support::{impl_outer_origin, parameter_types}; +use rstd::collections::btree_map::BTreeMap; + impl_outer_origin! { pub enum Origin for Test {} } +use std::sync::atomic::{AtomicUsize, Ordering}; + +// Simple variables to track number of invokations made to the status handler trait methods +// but this is not thread safe so must run tests with single thread: `cargo test -- --test-threads=1` +static STATUS_HANDLER_SUCCESSES: AtomicUsize = AtomicUsize::new(0); +static STATUS_HANDLER_FAILURES: AtomicUsize = AtomicUsize::new(0); + +pub struct MockStatusHandler {} +impl MockStatusHandler { + pub fn reset() { + STATUS_HANDLER_SUCCESSES.store(0, Ordering::SeqCst); + STATUS_HANDLER_FAILURES.store(0, Ordering::SeqCst); + } + pub fn successes() -> usize { + STATUS_HANDLER_SUCCESSES.load(Ordering::SeqCst) + } + pub fn failures() -> usize { + STATUS_HANDLER_FAILURES.load(Ordering::SeqCst) + } +} +impl PayoutStatusHandler for MockStatusHandler { + fn payout_succeeded( + _id: RewardRelationshipId, + _destination_account: &::AccountId, + _amount: BalanceOf, + ) { + STATUS_HANDLER_SUCCESSES.fetch_add(1, Ordering::SeqCst); + } + + fn payout_failed( + _id: RewardRelationshipId, + _destination_account: &::AccountId, + _amount: BalanceOf, + ) { + STATUS_HANDLER_FAILURES.fetch_add(1, Ordering::SeqCst); + } +} + // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. #[derive(Clone, PartialEq, Eq, Debug)] pub struct Test; @@ -79,7 +119,7 @@ impl balances::Trait for Test { } impl Trait for Test { - type PayoutStatusHandler = (); + type PayoutStatusHandler = MockStatusHandler; } impl minting::Trait for Test { @@ -87,6 +127,8 @@ impl minting::Trait for Test { } pub fn build_test_externalities() -> runtime_io::TestExternalities { + MockStatusHandler::reset(); + let t = system::GenesisConfig::default() .build_storage::() .unwrap(); diff --git a/src/tests.rs b/src/tests.rs index 90a53b48df..09b4afadca 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -104,11 +104,14 @@ fn one_off_payout() { Rewards::do_payouts(expected_payout_at + 1); assert_eq!(Balances::free_balance(&recipient_account), starting_balance); + assert_eq!(MockStatusHandler::successes(), 0); + Rewards::do_payouts(expected_payout_at); assert_eq!( Balances::free_balance(&recipient_account), starting_balance + payout ); + assert_eq!(MockStatusHandler::successes(), 1); let relationship = Rewards::reward_relationships(&relationship_id); assert_eq!(relationship.total_reward_received, payout); @@ -151,6 +154,7 @@ fn recurring_payout() { for i in 0..number_of_payouts { Rewards::do_payouts(expected_payout_at + interval * i); } + assert_eq!(MockStatusHandler::successes(), number_of_payouts as usize); assert_eq!( Balances::free_balance(&recipient_account), @@ -198,6 +202,8 @@ fn track_missed_payouts() { Rewards::do_payouts(expected_payout_at); assert_eq!(Balances::free_balance(&recipient_account), starting_balance); + assert_eq!(MockStatusHandler::failures(), 1); + let relationship = Rewards::reward_relationships(&relationship_id); assert_eq!(relationship.total_reward_received, 0); assert_eq!(relationship.total_reward_missed, payout); From 48642311a99ad73c8e63ccf014c48cca969f0c49 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Wed, 18 Sep 2019 13:29:42 +0300 Subject: [PATCH 11/23] mock: thread_local and RefCell for MockStatusHandler --- src/mock.rs | 66 +++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 46 insertions(+), 20 deletions(-) diff --git a/src/mock.rs b/src/mock.rs index b93fe15191..73472a55e5 100644 --- a/src/mock.rs +++ b/src/mock.rs @@ -14,47 +14,73 @@ use runtime_primitives::{ }; use srml_support::{impl_outer_origin, parameter_types}; -use rstd::collections::btree_map::BTreeMap; - impl_outer_origin! { pub enum Origin for Test {} } +use std::cell::RefCell; -use std::sync::atomic::{AtomicUsize, Ordering}; +struct StatusHandlerState { + successes: Vec, + failures: Vec, +} +impl StatusHandlerState { + pub fn reset(&mut self) { + self.successes = vec![]; + self.failures = vec![]; + } +} +impl Default for StatusHandlerState { + fn default() -> Self { + Self { + successes: vec![], + failures: vec![], + } + } +} -// Simple variables to track number of invokations made to the status handler trait methods -// but this is not thread safe so must run tests with single thread: `cargo test -- --test-threads=1` -static STATUS_HANDLER_SUCCESSES: AtomicUsize = AtomicUsize::new(0); -static STATUS_HANDLER_FAILURES: AtomicUsize = AtomicUsize::new(0); +thread_local!(static STATUS_HANDLER_STATE: RefCell = RefCell::new(Default::default())); pub struct MockStatusHandler {} impl MockStatusHandler { pub fn reset() { - STATUS_HANDLER_SUCCESSES.store(0, Ordering::SeqCst); - STATUS_HANDLER_FAILURES.store(0, Ordering::SeqCst); + STATUS_HANDLER_STATE.with(|cell| { + cell.borrow_mut().reset(); + }); } pub fn successes() -> usize { - STATUS_HANDLER_SUCCESSES.load(Ordering::SeqCst) + let mut value = 0; + STATUS_HANDLER_STATE.with(|cell| { + value = cell.borrow_mut().successes.len(); + }); + value } pub fn failures() -> usize { - STATUS_HANDLER_FAILURES.load(Ordering::SeqCst) + let mut value = 0; + STATUS_HANDLER_STATE.with(|cell| { + value = cell.borrow_mut().failures.len(); + }); + value } } -impl PayoutStatusHandler for MockStatusHandler { +impl PayoutStatusHandler for MockStatusHandler { fn payout_succeeded( - _id: RewardRelationshipId, - _destination_account: &::AccountId, - _amount: BalanceOf, + id: RewardRelationshipId, + _destination_account: &T::AccountId, + _amount: BalanceOf, ) { - STATUS_HANDLER_SUCCESSES.fetch_add(1, Ordering::SeqCst); + STATUS_HANDLER_STATE.with(|cell| { + cell.borrow_mut().successes.push(id); + }); } fn payout_failed( - _id: RewardRelationshipId, - _destination_account: &::AccountId, - _amount: BalanceOf, + id: RewardRelationshipId, + _destination_account: &T::AccountId, + _amount: BalanceOf, ) { - STATUS_HANDLER_FAILURES.fetch_add(1, Ordering::SeqCst); + STATUS_HANDLER_STATE.with(|cell| { + cell.borrow_mut().failures.push(id); + }); } } From d41ef9afc3d403b5643b6368c7749ee9c389beaf Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Wed, 18 Sep 2019 13:39:59 +0300 Subject: [PATCH 12/23] mocks: factor our status handler mock --- src/{mock.rs => mock/mod.rs} | 73 +++--------------------------------- src/mock/status_handler.rs | 73 ++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 68 deletions(-) rename src/{mock.rs => mock/mod.rs} (62%) create mode 100644 src/mock/status_handler.rs diff --git a/src/mock.rs b/src/mock/mod.rs similarity index 62% rename from src/mock.rs rename to src/mock/mod.rs index 73472a55e5..a592339e74 100644 --- a/src/mock.rs +++ b/src/mock/mod.rs @@ -1,10 +1,10 @@ #![cfg(test)] -use crate::*; +// use crate::*; +use crate::{Module, Trait}; use primitives::{Blake2Hasher, H256}; -use crate::{Module, Trait}; use balances; use minting; use runtime_primitives::{ @@ -14,75 +14,12 @@ use runtime_primitives::{ }; use srml_support::{impl_outer_origin, parameter_types}; +mod status_handler; +pub use status_handler::{MockStatusHandler}; + impl_outer_origin! { pub enum Origin for Test {} } -use std::cell::RefCell; - -struct StatusHandlerState { - successes: Vec, - failures: Vec, -} -impl StatusHandlerState { - pub fn reset(&mut self) { - self.successes = vec![]; - self.failures = vec![]; - } -} -impl Default for StatusHandlerState { - fn default() -> Self { - Self { - successes: vec![], - failures: vec![], - } - } -} - -thread_local!(static STATUS_HANDLER_STATE: RefCell = RefCell::new(Default::default())); - -pub struct MockStatusHandler {} -impl MockStatusHandler { - pub fn reset() { - STATUS_HANDLER_STATE.with(|cell| { - cell.borrow_mut().reset(); - }); - } - pub fn successes() -> usize { - let mut value = 0; - STATUS_HANDLER_STATE.with(|cell| { - value = cell.borrow_mut().successes.len(); - }); - value - } - pub fn failures() -> usize { - let mut value = 0; - STATUS_HANDLER_STATE.with(|cell| { - value = cell.borrow_mut().failures.len(); - }); - value - } -} -impl PayoutStatusHandler for MockStatusHandler { - fn payout_succeeded( - id: RewardRelationshipId, - _destination_account: &T::AccountId, - _amount: BalanceOf, - ) { - STATUS_HANDLER_STATE.with(|cell| { - cell.borrow_mut().successes.push(id); - }); - } - - fn payout_failed( - id: RewardRelationshipId, - _destination_account: &T::AccountId, - _amount: BalanceOf, - ) { - STATUS_HANDLER_STATE.with(|cell| { - cell.borrow_mut().failures.push(id); - }); - } -} // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. #[derive(Clone, PartialEq, Eq, Debug)] diff --git a/src/mock/status_handler.rs b/src/mock/status_handler.rs new file mode 100644 index 0000000000..cb1ed3de23 --- /dev/null +++ b/src/mock/status_handler.rs @@ -0,0 +1,73 @@ +#![cfg(test)] + +// use crate::*; +use crate::{Trait, RewardRelationshipId, PayoutStatusHandler, BalanceOf}; + +use std::cell::RefCell; + +struct StatusHandlerState { + successes: Vec, + failures: Vec, +} + +impl StatusHandlerState { + pub fn reset(&mut self) { + self.successes = vec![]; + self.failures = vec![]; + } +} + +impl Default for StatusHandlerState { + fn default() -> Self { + Self { + successes: vec![], + failures: vec![], + } + } +} + +thread_local!(static STATUS_HANDLER_STATE: RefCell = RefCell::new(Default::default())); + +pub struct MockStatusHandler {} +impl MockStatusHandler { + pub fn reset() { + STATUS_HANDLER_STATE.with(|cell| { + cell.borrow_mut().reset(); + }); + } + pub fn successes() -> usize { + let mut value = 0; + STATUS_HANDLER_STATE.with(|cell| { + value = cell.borrow_mut().successes.len(); + }); + value + } + pub fn failures() -> usize { + let mut value = 0; + STATUS_HANDLER_STATE.with(|cell| { + value = cell.borrow_mut().failures.len(); + }); + value + } +} +impl PayoutStatusHandler for MockStatusHandler { + fn payout_succeeded( + id: RewardRelationshipId, + _destination_account: &T::AccountId, + _amount: BalanceOf, + ) { + STATUS_HANDLER_STATE.with(|cell| { + cell.borrow_mut().successes.push(id); + }); + } + + fn payout_failed( + id: RewardRelationshipId, + _destination_account: &T::AccountId, + _amount: BalanceOf, + ) { + STATUS_HANDLER_STATE.with(|cell| { + cell.borrow_mut().failures.push(id); + }); + } +} From ef4bb44d3ff389bd50b412a82e249adb49549feb Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Wed, 18 Sep 2019 16:02:36 +0300 Subject: [PATCH 13/23] rustfmt --- src/mock/mod.rs | 2 +- src/mock/status_handler.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mock/mod.rs b/src/mock/mod.rs index a592339e74..562d3db6ae 100644 --- a/src/mock/mod.rs +++ b/src/mock/mod.rs @@ -15,7 +15,7 @@ use runtime_primitives::{ use srml_support::{impl_outer_origin, parameter_types}; mod status_handler; -pub use status_handler::{MockStatusHandler}; +pub use status_handler::MockStatusHandler; impl_outer_origin! { pub enum Origin for Test {} diff --git a/src/mock/status_handler.rs b/src/mock/status_handler.rs index cb1ed3de23..eb7719c95e 100644 --- a/src/mock/status_handler.rs +++ b/src/mock/status_handler.rs @@ -1,7 +1,7 @@ #![cfg(test)] // use crate::*; -use crate::{Trait, RewardRelationshipId, PayoutStatusHandler, BalanceOf}; +use crate::{BalanceOf, PayoutStatusHandler, RewardRelationshipId, Trait}; use std::cell::RefCell; From 758508df7a378f133e5a6f4daf46ec916adeaf18 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Thu, 19 Sep 2019 09:39:17 +0300 Subject: [PATCH 14/23] Cargo.toml fix package name --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 1e7fb82470..2542f0fcc4 100755 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = 'substrate-token-mint-module' +name = 'substrate-recurring-reward-module' version = '1.0.0' authors = ['Mokhtar Naamani '] edition = '2018' From 97aef804424b645c5a67c1fc75b45bb16b4b1c79 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Mon, 30 Sep 2019 17:58:07 +0300 Subject: [PATCH 15/23] update to use MintId from minting trait --- src/lib.rs | 12 ++++++------ src/mock/mod.rs | 1 + src/tests.rs | 4 ++-- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index bc4fa854d6..a79bdea436 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,7 +7,7 @@ use codec::{Decode, Encode}; use runtime_primitives::traits::Zero; use srml_support::{decl_module, decl_storage, ensure, StorageMap, StorageValue}; -use minting::{self, BalanceOf, MintId}; +use minting::{self, BalanceOf}; use system; mod mock; @@ -21,7 +21,7 @@ pub const FIRST_RECIPIENT_ID: RecipientId = 1; type RewardRelationshipId = u64; pub const FIRST_REWARD_RELATIONSHIP_ID: RewardRelationshipId = 1; -pub trait Trait: system::Trait + minting::Trait + Sized { +pub trait Trait: system::Trait + minting::Trait + minting::Trait { type PayoutStatusHandler: PayoutStatusHandler; } @@ -68,7 +68,7 @@ pub struct Recipient { } #[derive(Encode, Decode, Copy, Clone, Debug, Default)] -pub struct RewardRelationship { +pub struct RewardRelationship { // Identifier for receiver recipient: RecipientId, @@ -103,7 +103,7 @@ decl_storage! { NextRecipientId get(next_recipient_id): RecipientId = FIRST_RECIPIENT_ID; - RewardRelationships get(reward_relationships): map RewardRelationshipId => RewardRelationship, T::BlockNumber>; + RewardRelationships get(reward_relationships): map RewardRelationshipId => RewardRelationship, T::BlockNumber, T::MintId>; NextRewardRelationshipId get(next_reward_relationship_id): RewardRelationshipId = FIRST_REWARD_RELATIONSHIP_ID; } @@ -149,7 +149,7 @@ impl Module { // Adds a new RewardRelationship to rewardRelationships, for a given source, recipient, account, etc., with identifier equal to current nextRewardRelationshipId. Also increments nextRewardRelationshipId. pub fn add_reward_relationship( - mint_id: MintId, + mint_id: T::MintId, recipient: RecipientId, account: T::AccountId, amount_per_payout: BalanceOf, @@ -278,7 +278,7 @@ impl Module { let payout = relationship.amount_per_payout; // try to make payment - if >::transfer_exact_tokens( + if >::transfer_tokens( relationship.mint_id, payout, &relationship.account, diff --git a/src/mock/mod.rs b/src/mock/mod.rs index 562d3db6ae..27cb5add6f 100644 --- a/src/mock/mod.rs +++ b/src/mock/mod.rs @@ -87,6 +87,7 @@ impl Trait for Test { impl minting::Trait for Test { type Currency = Balances; + type MintId = u64; } pub fn build_test_externalities() -> runtime_io::TestExternalities { diff --git a/src/tests.rs b/src/tests.rs index 09b4afadca..8f876c6eea 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -6,10 +6,10 @@ use srml_support::traits::Currency; use runtime_io::with_externalities; -fn create_new_mint_with_capacity(capacity: u64) -> MintId { +fn create_new_mint_with_capacity(capacity: u64) -> u64 { let mint_id = Minting::add_mint(capacity, None).ok().unwrap(); assert!(Minting::mint_exists(mint_id)); - assert!(Minting::mint_has_capacity(mint_id, capacity)); + assert_eq!(Minting::mint_capacity(mint_id).ok().unwrap(), capacity); mint_id } From 1d24877876f27d729a17b4a07c40fa97863439b5 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Mon, 30 Sep 2019 18:08:36 +0300 Subject: [PATCH 16/23] update source git repo of minting module --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 2542f0fcc4..fef44aeca7 100755 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,7 +30,7 @@ rev = 'a2a0eb5398d6223e531455b4c155ef053a4a3a2b' [dependencies.minting] default_features = false -git = 'https://github.com/mnaamani/substrate-token-minting' +git = 'https://github.com/mnaamani/substrate-token-minting-module' package = 'substrate-token-mint-module' branch = 'token-minting' From 7c7a423399c14b5d73f3780472557bcabf9dc385 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Mon, 30 Sep 2019 23:44:02 +0300 Subject: [PATCH 17/23] use linked_map, make ids associated types --- src/lib.rs | 234 ++++++++++++++++++------------------- src/mock/mod.rs | 2 + src/mock/status_handler.rs | 30 ++--- src/tests.rs | 8 +- 4 files changed, 134 insertions(+), 140 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a79bdea436..a5d7d35e0a 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,9 +3,11 @@ use rstd::prelude::*; -use codec::{Decode, Encode}; -use runtime_primitives::traits::Zero; -use srml_support::{decl_module, decl_storage, ensure, StorageMap, StorageValue}; +use codec::{Codec, Decode, Encode}; +use runtime_primitives::traits::{MaybeSerializeDebug, Member, One, SimpleArithmetic, Zero}; +use srml_support::{ + decl_module, decl_storage, ensure, EnumerableStorageMap, Parameter, StorageMap, StorageValue, +}; use minting::{self, BalanceOf}; use system; @@ -13,99 +15,112 @@ use system; mod mock; mod tests; -/// Type of identifier for recipients. -type RecipientId = u64; -pub const FIRST_RECIPIENT_ID: RecipientId = 1; - -/// Type for identifier for relationship representing that a recipient recieves recurring reward from a token mint -type RewardRelationshipId = u64; -pub const FIRST_REWARD_RELATIONSHIP_ID: RewardRelationshipId = 1; - pub trait Trait: system::Trait + minting::Trait + minting::Trait { type PayoutStatusHandler: PayoutStatusHandler; + + /// Type of identifier for recipients. + type RecipientId: Parameter + + Member + + SimpleArithmetic + + Codec + + Default + + Copy + + MaybeSerializeDebug + + PartialEq; + + /// Type for identifier for relationship representing that a recipient recieves recurring reward from a token mint + type RewardRelationshipId: Parameter + + Member + + SimpleArithmetic + + Codec + + Default + + Copy + + MaybeSerializeDebug + + PartialEq; } /// Handler for aftermath of a payout attempt pub trait PayoutStatusHandler { fn payout_succeeded( - id: RewardRelationshipId, + id: T::RewardRelationshipId, destination_account: &T::AccountId, amount: BalanceOf, ); fn payout_failed( - id: RewardRelationshipId, + id: T::RewardRelationshipId, destination_account: &T::AccountId, amount: BalanceOf, ); } +/// Makes `()` empty tuple, a PayoutStatusHandler that does nothing. impl PayoutStatusHandler for () { fn payout_succeeded( - _id: RewardRelationshipId, + _id: T::RewardRelationshipId, _destination_account: &T::AccountId, _amount: BalanceOf, ) { } fn payout_failed( - _id: RewardRelationshipId, + _id: T::RewardRelationshipId, _destination_account: &T::AccountId, _amount: BalanceOf, ) { } } -// A recipient of recurring rewards +/// A recipient of recurring rewards #[derive(Encode, Decode, Copy, Clone, Debug, Default)] pub struct Recipient { // stats - // Total payout received by this recipient + /// Total payout received by this recipient total_reward_received: Balance, - // Total payout missed for this recipient + /// Total payout missed for this recipient total_reward_missed: Balance, } #[derive(Encode, Decode, Copy, Clone, Debug, Default)] -pub struct RewardRelationship { - // Identifier for receiver +pub struct RewardRelationship { + /// Identifier for receiver recipient: RecipientId, - // Identifier for reward source + /// Identifier for reward source mint_id: MintId, - // Destination account for reward + /// Destination account for reward account: AccountId, - // Paid out for + /// The payout amount at the next payout amount_per_payout: Balance, - // When set, identifies block when next payout should be processed, - // otherwise there is no pending payout + /// When set, identifies block when next payout should be processed, + /// otherwise there is no pending payout next_payment_at_block: Option, - // When set, will be the basis for automatically setting next payment, - // otherwise any upcoming payout will be a one off. + /// When set, will be the basis for automatically setting next payment, + /// otherwise any upcoming payout will be a one off. payout_interval: Option, // stats - // Total payout received in this relationship + /// Total payout received in this relationship total_reward_received: Balance, - // Total payout failed in this relationship + /// Total payout failed in this relationship total_reward_missed: Balance, } decl_storage! { trait Store for Module as RecurringReward { - Recipients get(recipients): map RecipientId => Recipient>; + Recipients get(recipients): linked_map T::RecipientId => Recipient>; - NextRecipientId get(next_recipient_id): RecipientId = FIRST_RECIPIENT_ID; + RecipientsCreated get(recipients_created): T::RecipientId; - RewardRelationships get(reward_relationships): map RewardRelationshipId => RewardRelationship, T::BlockNumber, T::MintId>; + RewardRelationships get(reward_relationships): linked_map T::RewardRelationshipId => RewardRelationship, T::BlockNumber, T::MintId, T::RecipientId>; - NextRewardRelationshipId get(next_reward_relationship_id): RewardRelationshipId = FIRST_REWARD_RELATIONSHIP_ID; + RewardRelationshipsCreated get(reward_relationships_created): T::RewardRelationshipId; } } @@ -133,29 +148,22 @@ pub enum NextPaymentSchedule { impl Module { /* Adds a new Recipient recipient to recipients, with identifier equal to nextRecipientId, which is also incremented, and returns the new recipient identifier. */ - pub fn add_recipient() -> RecipientId { - let next_id = Self::next_recipient_id(); - NextRecipientId::mutate(|n| { - *n += 1; - }); + pub fn add_recipient() -> T::RecipientId { + let next_id = Self::recipients_created(); + >::put(next_id + One::one()); >::insert(&next_id, Recipient::default()); next_id } - /// Removes a mapping from reward_recipients based on the given identifier. - pub fn remove_recipient(id: RecipientId) { - >::remove(&id); - } - // Adds a new RewardRelationship to rewardRelationships, for a given source, recipient, account, etc., with identifier equal to current nextRewardRelationshipId. Also increments nextRewardRelationshipId. pub fn add_reward_relationship( mint_id: T::MintId, - recipient: RecipientId, + recipient: T::RecipientId, account: T::AccountId, amount_per_payout: BalanceOf, next_payment_schedule: Option>, payout_interval: Option, - ) -> Result { + ) -> Result { ensure!( >::mint_exists(mint_id), RewardsError::RewardSourceNotFound @@ -195,16 +203,14 @@ impl Module { total_reward_missed: Zero::zero(), }; - let relationship_id = Self::next_reward_relationship_id(); - NextRewardRelationshipId::mutate(|n| { - *n += 1; - }); + let relationship_id = Self::reward_relationships_created(); + >::put(relationship_id + One::one()); >::insert(relationship_id, relationship); Ok(relationship_id) } // Removes a mapping from reward relashionships based on given identifier. - pub fn remove_reward_relationship(id: RewardRelationshipId) { + pub fn remove_reward_relationship(id: T::RewardRelationshipId) { >::remove(&id); } @@ -213,7 +219,7 @@ impl Module { // the next scheduled payout. All values are optional, but updating values are combined in this // single method to ensure atomic updates. pub fn set_reward_relationship( - id: RewardRelationshipId, + id: T::RewardRelationshipId, account: Option, payout: Option>, next_payment_at: Option>, @@ -256,77 +262,71 @@ impl Module { Otherwise, analogous steps for failure. */ fn do_payouts(now: T::BlockNumber) { - for relationship_id in FIRST_REWARD_RELATIONSHIP_ID..Self::next_reward_relationship_id() { - if !>::exists(&relationship_id) { - continue; - } - let mut relationship = Self::reward_relationships(&relationship_id); - + for (relationship_id, relationship) in >::enumerate() { // recipient can be removed independantly of relationship, so ensure we have a recipient if !>::exists(&relationship.recipient) { continue; } - let mut recipient = Self::recipients(&relationship.recipient); - if let Some(blocknumber) = relationship.next_payment_at_block { - if blocknumber != now { - return; - } - - // Add the missed payout and try to pay those in addition to scheduled payout? - // let payout = relationship.total_reward_missed + relationship.amount_per_payout; - let payout = relationship.amount_per_payout; - - // try to make payment - if >::transfer_tokens( - relationship.mint_id, - payout, - &relationship.account, - ) - .is_err() - { - // add only newly scheduled payout to total missed payout - relationship.total_reward_missed += relationship.amount_per_payout; - - // update recipient stats - recipient.total_reward_missed += relationship.amount_per_payout; - - T::PayoutStatusHandler::payout_failed( - relationship_id, - &relationship.account, - payout, - ); - } else { - // update payout received stats - relationship.total_reward_received += payout; - recipient.total_reward_received += payout; - - // update missed payout stats - // if relationship.total_reward_missed != Zero::zero() { - // // update recipient stats - // recipient.total_reward_missed -= relationship.total_reward_missed; - - // // clear missed reward on relationship - // relationship.total_reward_missed = Zero::zero(); - // } - T::PayoutStatusHandler::payout_succeeded( - relationship_id, - &relationship.account, - payout, - ); - } - - // update next payout blocknumber at interval if set - if let Some(payout_interval) = relationship.payout_interval { - relationship.next_payment_at_block = Some(now + payout_interval); - } else { - relationship.next_payment_at_block = None; - } - - // update relationship and recipient to storage - >::insert(&relationship.recipient, recipient); - >::insert(&relationship_id, relationship); - } + >::mutate(relationship_id, |relationship| { + >::mutate(relationship.recipient, |recipient| { + if let Some(blocknumber) = relationship.next_payment_at_block { + if blocknumber != now { + return; + } + + // Add the missed payout and try to pay those in addition to scheduled payout? + // let payout = relationship.total_reward_missed + relationship.amount_per_payout; + let payout = relationship.amount_per_payout; + + // try to make payment + if >::transfer_tokens( + relationship.mint_id, + payout, + &relationship.account, + ) + .is_err() + { + // add only newly scheduled payout to total missed payout + relationship.total_reward_missed += relationship.amount_per_payout; + + // update recipient stats + recipient.total_reward_missed += relationship.amount_per_payout; + + T::PayoutStatusHandler::payout_failed( + relationship_id, + &relationship.account, + payout, + ); + } else { + // update payout received stats + relationship.total_reward_received += payout; + recipient.total_reward_received += payout; + + // update missed payout stats + // if relationship.total_reward_missed != Zero::zero() { + // // update recipient stats + // recipient.total_reward_missed -= relationship.total_reward_missed; + + // // clear missed reward on relationship + // relationship.total_reward_missed = Zero::zero(); + // } + T::PayoutStatusHandler::payout_succeeded( + relationship_id, + &relationship.account, + payout, + ); + } + + // update next payout blocknumber at interval if set + if let Some(payout_interval) = relationship.payout_interval { + relationship.next_payment_at_block = Some(now + payout_interval); + } else { + relationship.next_payment_at_block = None; + } + } + }); + }); } } } diff --git a/src/mock/mod.rs b/src/mock/mod.rs index 27cb5add6f..df87cac531 100644 --- a/src/mock/mod.rs +++ b/src/mock/mod.rs @@ -83,6 +83,8 @@ impl balances::Trait for Test { impl Trait for Test { type PayoutStatusHandler = MockStatusHandler; + type RecipientId = u64; + type RewardRelationshipId = u64; } impl minting::Trait for Test { diff --git a/src/mock/status_handler.rs b/src/mock/status_handler.rs index eb7719c95e..652f0d288b 100644 --- a/src/mock/status_handler.rs +++ b/src/mock/status_handler.rs @@ -1,23 +1,23 @@ #![cfg(test)] // use crate::*; -use crate::{BalanceOf, PayoutStatusHandler, RewardRelationshipId, Trait}; - +use super::Test; +use crate::{BalanceOf, PayoutStatusHandler, Trait}; use std::cell::RefCell; -struct StatusHandlerState { - successes: Vec, - failures: Vec, +struct StatusHandlerState { + successes: Vec, + failures: Vec, } -impl StatusHandlerState { +impl StatusHandlerState { pub fn reset(&mut self) { self.successes = vec![]; self.failures = vec![]; } } -impl Default for StatusHandlerState { +impl Default for StatusHandlerState { fn default() -> Self { Self { successes: vec![], @@ -26,7 +26,7 @@ impl Default for StatusHandlerState { } } -thread_local!(static STATUS_HANDLER_STATE: RefCell = RefCell::new(Default::default())); +thread_local!(static STATUS_HANDLER_STATE: RefCell> = RefCell::new(Default::default())); pub struct MockStatusHandler {} impl MockStatusHandler { @@ -50,22 +50,14 @@ impl MockStatusHandler { value } } -impl PayoutStatusHandler for MockStatusHandler { - fn payout_succeeded( - id: RewardRelationshipId, - _destination_account: &T::AccountId, - _amount: BalanceOf, - ) { +impl PayoutStatusHandler for MockStatusHandler { + fn payout_succeeded(id: u64, _destination_account: &u64, _amount: u64) { STATUS_HANDLER_STATE.with(|cell| { cell.borrow_mut().successes.push(id); }); } - fn payout_failed( - id: RewardRelationshipId, - _destination_account: &T::AccountId, - _amount: BalanceOf, - ) { + fn payout_failed(id: u64, _destination_account: &u64, _amount: u64) { STATUS_HANDLER_STATE.with(|cell| { cell.borrow_mut().failures.push(id); }); diff --git a/src/tests.rs b/src/tests.rs index 8f876c6eea..dee959a267 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -16,12 +16,12 @@ fn create_new_mint_with_capacity(capacity: u64) -> u64 { #[test] fn adding_recipients() { with_externalities(&mut build_test_externalities(), || { - let next_id = Rewards::next_recipient_id(); + let next_id = Rewards::recipients_created(); assert!(!>::exists(&next_id)); let recipient_id = Rewards::add_recipient(); assert!(>::exists(&next_id)); assert_eq!(recipient_id, next_id); - assert_eq!(Rewards::next_recipient_id(), next_id + 1); + assert_eq!(Rewards::recipients_created(), next_id + 1); }); } @@ -35,7 +35,7 @@ fn adding_relationships() { let next_payment_at: u64 = 2222; let payout = 100; - let next_relationship_id = Rewards::next_reward_relationship_id(); + let next_relationship_id = Rewards::reward_relationships_created(); let relationship = Rewards::add_reward_relationship( mint_id, recipient_id, @@ -48,7 +48,7 @@ fn adding_relationships() { let relationship_id = relationship.ok().unwrap(); assert_eq!(relationship_id, next_relationship_id); assert_eq!( - Rewards::next_reward_relationship_id(), + Rewards::reward_relationships_created(), next_relationship_id + 1 ); assert!(>::exists(&relationship_id)); From 78133b4665ca7ebd415f2e0598559577b3279550 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Tue, 1 Oct 2019 00:09:13 +0300 Subject: [PATCH 18/23] code cleanup, mutate instead of insert --- src/lib.rs | 57 ++++++++++++++++++-------------------- src/mock/status_handler.rs | 2 +- 2 files changed, 28 insertions(+), 31 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a5d7d35e0a..86246b2894 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -147,7 +147,7 @@ pub enum NextPaymentSchedule { } impl Module { - /* Adds a new Recipient recipient to recipients, with identifier equal to nextRecipientId, which is also incremented, and returns the new recipient identifier. */ + /// Adds a new Recipient and returns new recipient identifier. pub fn add_recipient() -> T::RecipientId { let next_id = Self::recipients_created(); >::put(next_id + One::one()); @@ -155,7 +155,7 @@ impl Module { next_id } - // Adds a new RewardRelationship to rewardRelationships, for a given source, recipient, account, etc., with identifier equal to current nextRewardRelationshipId. Also increments nextRewardRelationshipId. + /// Adds a new RewardRelationship, for a given source mint, recipient, account. pub fn add_reward_relationship( mint_id: T::MintId, recipient: T::RecipientId, @@ -220,37 +220,37 @@ impl Module { // single method to ensure atomic updates. pub fn set_reward_relationship( id: T::RewardRelationshipId, - account: Option, - payout: Option>, - next_payment_at: Option>, - payout_interval: Option>, + new_account: Option, + new_payout: Option>, + new_next_payment_at: Option>, + new_payout_interval: Option>, ) -> Result<(), RewardsError> { ensure!( >::exists(&id), RewardsError::RewardRelationshipNotFound ); - let mut relationship = Self::reward_relationships(&id); - if let Some(account) = account { - relationship.account = account; - } - if let Some(payout) = payout { - relationship.amount_per_payout = payout; - } - if let Some(next_payout_at_block) = next_payment_at { - if let Some(blocknumber) = next_payout_at_block { - ensure!( - blocknumber > >::block_number(), - RewardsError::BlockNumberInPast - ); + >::mutate(&id, |relationship| { + if let Some(account) = new_account { + relationship.account = account; } - relationship.next_payment_at_block = next_payout_at_block; - } - if let Some(payout_interval) = payout_interval { - relationship.payout_interval = payout_interval; - } - >::insert(&id, relationship); - Ok(()) + if let Some(payout) = new_payout { + relationship.amount_per_payout = payout; + } + if let Some(next_payout_at_block) = new_next_payment_at { + if let Some(blocknumber) = next_payout_at_block { + ensure!( + blocknumber > >::block_number(), + RewardsError::BlockNumberInPast + ); + } + relationship.next_payment_at_block = next_payout_at_block; + } + if let Some(payout_interval) = new_payout_interval { + relationship.payout_interval = payout_interval; + } + Ok(()) + }) } /* @@ -263,10 +263,7 @@ impl Module { */ fn do_payouts(now: T::BlockNumber) { for (relationship_id, relationship) in >::enumerate() { - // recipient can be removed independantly of relationship, so ensure we have a recipient - if !>::exists(&relationship.recipient) { - continue; - } + assert!(>::exists(&relationship.recipient)); >::mutate(relationship_id, |relationship| { >::mutate(relationship.recipient, |recipient| { diff --git a/src/mock/status_handler.rs b/src/mock/status_handler.rs index 652f0d288b..4a4ec52385 100644 --- a/src/mock/status_handler.rs +++ b/src/mock/status_handler.rs @@ -2,7 +2,7 @@ // use crate::*; use super::Test; -use crate::{BalanceOf, PayoutStatusHandler, Trait}; +use crate::{PayoutStatusHandler, Trait}; use std::cell::RefCell; struct StatusHandlerState { From 522b0e58d4c0854b5def52ae561360f83634c070 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Tue, 1 Oct 2019 00:34:51 +0300 Subject: [PATCH 19/23] simplify adding relationship and ensure recipinet exists --- src/lib.rs | 59 ++++++++++++++++------------------------------------ src/tests.rs | 53 +++++++++++++++++++++++----------------------- 2 files changed, 45 insertions(+), 67 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 86246b2894..30eb6cdfae 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -141,11 +141,6 @@ pub enum RewardsError { RewardRelationshipNotFound, } -pub enum NextPaymentSchedule { - Absolute(BlockNumber), - Relative(BlockNumber), -} - impl Module { /// Adds a new Recipient and returns new recipient identifier. pub fn add_recipient() -> T::RecipientId { @@ -161,51 +156,33 @@ impl Module { recipient: T::RecipientId, account: T::AccountId, amount_per_payout: BalanceOf, - next_payment_schedule: Option>, + next_payment_at_block: T::BlockNumber, payout_interval: Option, ) -> Result { ensure!( >::mint_exists(mint_id), RewardsError::RewardSourceNotFound ); - - let next_payment_at_block = match next_payment_schedule { - Some(schedule) => match schedule { - NextPaymentSchedule::Absolute(blocknumber) => { - ensure!( - blocknumber > >::block_number(), - RewardsError::BlockNumberInPast - ); - Some(blocknumber) - } - NextPaymentSchedule::Relative(blocknumber) => { - Some(>::block_number() + blocknumber) - } - }, - None => match payout_interval { - Some(interval) => Some(>::block_number() + interval), - None => { - // No payouts will be made unless relationship is updated in future and next_payment_in_block - // is set! should this be allowed? - None - } - }, - }; - - let relationship = RewardRelationship { - mint_id, - recipient, - account, - amount_per_payout, - next_payment_at_block, - payout_interval, - total_reward_received: Zero::zero(), - total_reward_missed: Zero::zero(), - }; + ensure!( + >::exists(recipient), + RewardsError::RecipientNotFound + ); let relationship_id = Self::reward_relationships_created(); >::put(relationship_id + One::one()); - >::insert(relationship_id, relationship); + >::insert( + relationship_id, + RewardRelationship { + mint_id, + recipient, + account, + amount_per_payout, + next_payment_at_block: Some(next_payment_at_block), + payout_interval, + total_reward_received: Zero::zero(), + total_reward_missed: Zero::zero(), + }, + ); Ok(relationship_id) } diff --git a/src/tests.rs b/src/tests.rs index dee959a267..c02e3f6058 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -41,7 +41,7 @@ fn adding_relationships() { recipient_id, recipient_account, payout, - Some(NextPaymentSchedule::Absolute(next_payment_at)), + next_payment_at, Some(interval), ); assert!(relationship.is_ok()); @@ -60,14 +60,18 @@ fn adding_relationships() { assert_eq!(relationship.payout_interval, Some(interval)); // mint doesn't exist - assert_eq!(Rewards::add_reward_relationship( - 111, - recipient_id, - recipient_account, - 100, - None, - None, - ).expect_err("should fail if mint doesn't exist"), RewardsError::RewardSourceNotFound); + assert_eq!( + Rewards::add_reward_relationship( + 111, + recipient_id, + recipient_account, + 100, + next_payment_at, + None, + ) + .expect_err("should fail if mint doesn't exist"), + RewardsError::RewardSourceNotFound + ); }); } @@ -80,33 +84,32 @@ fn one_off_payout() { let mint_id = create_new_mint_with_capacity(1000000); let recipient_id = Rewards::add_recipient(); let payout: u64 = 1000; - let payout_after: u64 = 2222; - let expected_payout_at = System::block_number() + payout_after; + let next_payout_at: u64 = 12222; let relationship = Rewards::add_reward_relationship( mint_id, recipient_id, recipient_account, payout, - Some(NextPaymentSchedule::Relative(payout_after)), + next_payout_at, None, ); assert!(relationship.is_ok()); let relationship_id = relationship.ok().unwrap(); let relationship = Rewards::reward_relationships(&relationship_id); - assert_eq!(relationship.next_payment_at_block, Some(expected_payout_at)); + assert_eq!(relationship.next_payment_at_block, Some(next_payout_at)); let starting_balance = Balances::free_balance(&recipient_account); // try to catch 'off by one' bugs - Rewards::do_payouts(expected_payout_at - 1); + Rewards::do_payouts(next_payout_at - 1); assert_eq!(Balances::free_balance(&recipient_account), starting_balance); - Rewards::do_payouts(expected_payout_at + 1); + Rewards::do_payouts(next_payout_at + 1); assert_eq!(Balances::free_balance(&recipient_account), starting_balance); assert_eq!(MockStatusHandler::successes(), 0); - Rewards::do_payouts(expected_payout_at); + Rewards::do_payouts(next_payout_at); assert_eq!( Balances::free_balance(&recipient_account), starting_balance + payout @@ -131,28 +134,27 @@ fn recurring_payout() { let mint_id = create_new_mint_with_capacity(1000000); let recipient_id = Rewards::add_recipient(); let payout: u64 = 1000; - let payout_after: u64 = 2222; - let expected_payout_at = System::block_number() + payout_after; + let next_payout_at: u64 = 12222; let interval: u64 = 600; let relationship = Rewards::add_reward_relationship( mint_id, recipient_id, recipient_account, payout, - Some(NextPaymentSchedule::Relative(payout_after)), + next_payout_at, Some(interval), ); assert!(relationship.is_ok()); let relationship_id = relationship.ok().unwrap(); let relationship = Rewards::reward_relationships(&relationship_id); - assert_eq!(relationship.next_payment_at_block, Some(expected_payout_at)); + assert_eq!(relationship.next_payment_at_block, Some(next_payout_at)); let starting_balance = Balances::free_balance(&recipient_account); let number_of_payouts = 3; for i in 0..number_of_payouts { - Rewards::do_payouts(expected_payout_at + interval * i); + Rewards::do_payouts(next_payout_at + interval * i); } assert_eq!(MockStatusHandler::successes(), number_of_payouts as usize); @@ -181,25 +183,24 @@ fn track_missed_payouts() { let mint_id = create_new_mint_with_capacity(0); let recipient_id = Rewards::add_recipient(); let payout: u64 = 1000; - let payout_after: u64 = 2222; - let expected_payout_at = System::block_number() + payout_after; + let next_payout_at: u64 = 12222; let relationship = Rewards::add_reward_relationship( mint_id, recipient_id, recipient_account, payout, - Some(NextPaymentSchedule::Relative(payout_after)), + next_payout_at, None, ); assert!(relationship.is_ok()); let relationship_id = relationship.ok().unwrap(); let relationship = Rewards::reward_relationships(&relationship_id); - assert_eq!(relationship.next_payment_at_block, Some(expected_payout_at)); + assert_eq!(relationship.next_payment_at_block, Some(next_payout_at)); let starting_balance = Balances::free_balance(&recipient_account); - Rewards::do_payouts(expected_payout_at); + Rewards::do_payouts(next_payout_at); assert_eq!(Balances::free_balance(&recipient_account), starting_balance); assert_eq!(MockStatusHandler::failures(), 1); From e0ecf4b926be69ee64dcc3e536a28f8d960a2563 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Wed, 2 Oct 2019 10:01:41 +0300 Subject: [PATCH 20/23] ensure next_payment is always in future when setting, or creating relationship --- src/lib.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 30eb6cdfae..730285695f 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -137,7 +137,7 @@ decl_module! { pub enum RewardsError { RecipientNotFound, RewardSourceNotFound, - BlockNumberInPast, + NextPaymentNotInFuture, RewardRelationshipNotFound, } @@ -167,6 +167,10 @@ impl Module { >::exists(recipient), RewardsError::RecipientNotFound ); + ensure!( + next_payment_at_block > >::block_number(), + RewardsError::NextPaymentNotInFuture + ); let relationship_id = Self::reward_relationships_created(); >::put(relationship_id + One::one()); @@ -186,9 +190,11 @@ impl Module { Ok(relationship_id) } - // Removes a mapping from reward relashionships based on given identifier. + /// Removes a relationship from RewardRelashionships and its recipient. pub fn remove_reward_relationship(id: T::RewardRelationshipId) { - >::remove(&id); + if >::exists(&id) { + >::remove(>::take(&id).recipient); + } } // For reward relationship found with given identifier, new values can be set for @@ -218,7 +224,7 @@ impl Module { if let Some(blocknumber) = next_payout_at_block { ensure!( blocknumber > >::block_number(), - RewardsError::BlockNumberInPast + RewardsError::NextPaymentNotInFuture ); } relationship.next_payment_at_block = next_payout_at_block; From 1d545c72e34a90fef65beb2a147377882d602427 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Wed, 2 Oct 2019 16:24:15 +0300 Subject: [PATCH 21/23] don't mutate() --- src/lib.rs | 121 +++++++++++++++++++++++++++-------------------------- 1 file changed, 61 insertions(+), 60 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 730285695f..a35dbd4734 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -245,68 +245,69 @@ impl Module { Otherwise, analogous steps for failure. */ fn do_payouts(now: T::BlockNumber) { - for (relationship_id, relationship) in >::enumerate() { + for (relationship_id, ref mut relationship) in >::enumerate() { assert!(>::exists(&relationship.recipient)); - >::mutate(relationship_id, |relationship| { - >::mutate(relationship.recipient, |recipient| { - if let Some(blocknumber) = relationship.next_payment_at_block { - if blocknumber != now { - return; - } - - // Add the missed payout and try to pay those in addition to scheduled payout? - // let payout = relationship.total_reward_missed + relationship.amount_per_payout; - let payout = relationship.amount_per_payout; - - // try to make payment - if >::transfer_tokens( - relationship.mint_id, - payout, - &relationship.account, - ) - .is_err() - { - // add only newly scheduled payout to total missed payout - relationship.total_reward_missed += relationship.amount_per_payout; - - // update recipient stats - recipient.total_reward_missed += relationship.amount_per_payout; - - T::PayoutStatusHandler::payout_failed( - relationship_id, - &relationship.account, - payout, - ); - } else { - // update payout received stats - relationship.total_reward_received += payout; - recipient.total_reward_received += payout; - - // update missed payout stats - // if relationship.total_reward_missed != Zero::zero() { - // // update recipient stats - // recipient.total_reward_missed -= relationship.total_reward_missed; - - // // clear missed reward on relationship - // relationship.total_reward_missed = Zero::zero(); - // } - T::PayoutStatusHandler::payout_succeeded( - relationship_id, - &relationship.account, - payout, - ); - } - - // update next payout blocknumber at interval if set - if let Some(payout_interval) = relationship.payout_interval { - relationship.next_payment_at_block = Some(now + payout_interval); - } else { - relationship.next_payment_at_block = None; - } - } - }); - }); + let mut recipient = Self::recipients(relationship.recipient); + + if let Some(next_payment_at_block) = relationship.next_payment_at_block { + if next_payment_at_block != now { + continue; + } + + // Add the missed payout and try to pay those in addition to scheduled payout? + // let payout = relationship.total_reward_missed + relationship.amount_per_payout; + let payout = relationship.amount_per_payout; + + // try to make payment + if >::transfer_tokens( + relationship.mint_id, + payout, + &relationship.account, + ) + .is_err() + { + // add only newly scheduled payout to total missed payout + relationship.total_reward_missed += relationship.amount_per_payout; + + // update recipient stats + recipient.total_reward_missed += relationship.amount_per_payout; + + T::PayoutStatusHandler::payout_failed( + relationship_id, + &relationship.account, + payout, + ); + } else { + // update payout received stats + relationship.total_reward_received += payout; + recipient.total_reward_received += payout; + + // update missed payout stats + // if relationship.total_reward_missed != Zero::zero() { + // // update recipient stats + // recipient.total_reward_missed -= relationship.total_reward_missed; + + // // clear missed reward on relationship + // relationship.total_reward_missed = Zero::zero(); + // } + T::PayoutStatusHandler::payout_succeeded( + relationship_id, + &relationship.account, + payout, + ); + } + + // update next payout blocknumber at interval if set + if let Some(payout_interval) = relationship.payout_interval { + relationship.next_payment_at_block = Some(now + payout_interval); + } else { + relationship.next_payment_at_block = None; + } + + >::insert(relationship.recipient, recipient); + >::insert(relationship_id, relationship); + } } } } From 2cd91211e153edbf994d11e4e9f18b22060d232c Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Wed, 2 Oct 2019 16:24:52 +0300 Subject: [PATCH 22/23] fix partial mutation bug --- src/lib.rs | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a35dbd4734..68234d14fa 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -213,27 +213,29 @@ impl Module { RewardsError::RewardRelationshipNotFound ); - >::mutate(&id, |relationship| { - if let Some(account) = new_account { - relationship.account = account; - } - if let Some(payout) = new_payout { - relationship.amount_per_payout = payout; - } - if let Some(next_payout_at_block) = new_next_payment_at { - if let Some(blocknumber) = next_payout_at_block { - ensure!( - blocknumber > >::block_number(), - RewardsError::NextPaymentNotInFuture - ); - } - relationship.next_payment_at_block = next_payout_at_block; - } - if let Some(payout_interval) = new_payout_interval { - relationship.payout_interval = payout_interval; + let mut relationship = Self::reward_relationships(&id); + + if let Some(account) = new_account { + relationship.account = account; + } + if let Some(payout) = new_payout { + relationship.amount_per_payout = payout; + } + if let Some(next_payout_at_block) = new_next_payment_at { + if let Some(blocknumber) = next_payout_at_block { + ensure!( + blocknumber > >::block_number(), + RewardsError::NextPaymentNotInFuture + ); } - Ok(()) - }) + relationship.next_payment_at_block = next_payout_at_block; + } + if let Some(payout_interval) = new_payout_interval { + relationship.payout_interval = payout_interval; + } + + >::insert(&id, relationship); + Ok(()) } /* From 294c0e764168ff05a0491a926830646f3ee285a0 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Fri, 4 Oct 2019 09:12:05 +0300 Subject: [PATCH 23/23] fix method name on mint --- src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests.rs b/src/tests.rs index c02e3f6058..104fd08906 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -9,7 +9,7 @@ use runtime_io::with_externalities; fn create_new_mint_with_capacity(capacity: u64) -> u64 { let mint_id = Minting::add_mint(capacity, None).ok().unwrap(); assert!(Minting::mint_exists(mint_id)); - assert_eq!(Minting::mint_capacity(mint_id).ok().unwrap(), capacity); + assert_eq!(Minting::get_mint_capacity(mint_id).ok().unwrap(), capacity); mint_id }