From 68f3081876ea89904ca4b41d30ef3e532e01b288 Mon Sep 17 00:00:00 2001
From: wangjj9219 <183318287@qq.com>
Date: Mon, 22 Mar 2021 10:28:28 +0800
Subject: [PATCH] refactor rewards (#412)

---
 rewards/src/default_weight.rs |  16 -----
 rewards/src/lib.rs            |  49 +++++----------
 rewards/src/mock.rs           |  36 ++---------
 rewards/src/tests.rs          | 109 +++++++---------------------------
 traits/src/rewards.rs         |  20 ++-----
 5 files changed, 44 insertions(+), 186 deletions(-)
 delete mode 100644 rewards/src/default_weight.rs

diff --git a/rewards/src/default_weight.rs b/rewards/src/default_weight.rs
deleted file mode 100644
index a2be6a5..0000000
--- a/rewards/src/default_weight.rs
+++ /dev/null
@@ -1,16 +0,0 @@
-//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 2.0.0
-
-#![allow(unused_parens)]
-#![allow(unused_imports)]
-#![allow(clippy::unnecessary_cast)]
-
-use frame_support::weights::{constants::RocksDbWeight as DbWeight, Weight};
-
-impl crate::WeightInfo for () {
-	fn on_initialize(c: u32) -> Weight {
-		(33_360_000 as Weight)
-			.saturating_add((23_139_000 as Weight).saturating_mul(c as Weight))
-			.saturating_add(DbWeight::get().reads(2 as Weight))
-			.saturating_add(DbWeight::get().reads((1 as Weight).saturating_mul(c as Weight)))
-	}
-}
diff --git a/rewards/src/lib.rs b/rewards/src/lib.rs
index 057c7e3..55be0d1 100644
--- a/rewards/src/lib.rs
+++ b/rewards/src/lib.rs
@@ -1,7 +1,6 @@
 #![allow(clippy::unused_unit)]
 #![cfg_attr(not(feature = "std"), no_std)]
 
-mod default_weight;
 mod mock;
 mod tests;
 
@@ -37,10 +36,6 @@ pub use module::*;
 pub mod module {
 	use super::*;
 
-	pub trait WeightInfo {
-		fn on_initialize(c: u32) -> Weight;
-	}
-
 	#[pallet::config]
 	pub trait Config: frame_system::Config {
 		/// The share type of pool.
@@ -64,19 +59,10 @@ pub mod module {
 			+ FixedPointOperand;
 
 		/// The reward pool ID type.
-		type PoolId: Parameter + Member + Copy + FullCodec;
+		type PoolId: Parameter + Member + Clone + FullCodec;
 
 		/// The `RewardHandler`
-		type Handler: RewardHandler<
-			Self::AccountId,
-			Self::BlockNumber,
-			Share = Self::Share,
-			Balance = Self::Balance,
-			PoolId = Self::PoolId,
-		>;
-
-		/// Weight information for extrinsics in this module.
-		type WeightInfo: WeightInfo;
+		type Handler: RewardHandler<Self::AccountId, Balance = Self::Balance, PoolId = Self::PoolId>;
 	}
 
 	/// Stores reward pool info.
@@ -95,27 +81,22 @@ pub mod module {
 	pub struct Pallet<T>(PhantomData<T>);
 
 	#[pallet::hooks]
-	impl<T: Config> Hooks<T::BlockNumber> for Pallet<T> {
-		fn on_initialize(now: T::BlockNumber) -> Weight {
-			let mut count = 0;
-			T::Handler::accumulate_reward(now, |pool, reward_to_accumulate| {
-				if !reward_to_accumulate.is_zero() {
-					count += 1;
-					Pools::<T>::mutate(pool, |pool_info| {
-						pool_info.total_rewards = pool_info.total_rewards.saturating_add(reward_to_accumulate)
-					});
-				}
-			});
-			T::WeightInfo::on_initialize(count)
-		}
-	}
+	impl<T: Config> Hooks<T::BlockNumber> for Pallet<T> {}
 
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {}
 }
 
 impl<T: Config> Pallet<T> {
-	pub fn add_share(who: &T::AccountId, pool: T::PoolId, add_amount: T::Share) {
+	pub fn accumulate_reward(pool: &T::PoolId, reward_increment: T::Balance) {
+		if !reward_increment.is_zero() {
+			Pools::<T>::mutate(pool, |pool_info| {
+				pool_info.total_rewards = pool_info.total_rewards.saturating_add(reward_increment)
+			});
+		}
+	}
+
+	pub fn add_share(who: &T::AccountId, pool: &T::PoolId, add_amount: T::Share) {
 		if add_amount.is_zero() {
 			return;
 		}
@@ -135,7 +116,7 @@ impl<T: Config> Pallet<T> {
 		});
 	}
 
-	pub fn remove_share(who: &T::AccountId, pool: T::PoolId, remove_amount: T::Share) {
+	pub fn remove_share(who: &T::AccountId, pool: &T::PoolId, remove_amount: T::Share) {
 		if remove_amount.is_zero() {
 			return;
 		}
@@ -167,7 +148,7 @@ impl<T: Config> Pallet<T> {
 		});
 	}
 
-	pub fn set_share(who: &T::AccountId, pool: T::PoolId, new_share: T::Share) {
+	pub fn set_share(who: &T::AccountId, pool: &T::PoolId, new_share: T::Share) {
 		let (share, _) = Self::share_and_withdrawn_reward(pool, who);
 
 		if new_share > share {
@@ -177,7 +158,7 @@ impl<T: Config> Pallet<T> {
 		}
 	}
 
-	pub fn claim_rewards(who: &T::AccountId, pool: T::PoolId) {
+	pub fn claim_rewards(who: &T::AccountId, pool: &T::PoolId) {
 		ShareAndWithdrawnReward::<T>::mutate(pool, who, |(share, withdrawn_rewards)| {
 			if share.is_zero() {
 				return;
diff --git a/rewards/src/mock.rs b/rewards/src/mock.rs
index 5838f30..1baf08c 100644
--- a/rewards/src/mock.rs
+++ b/rewards/src/mock.rs
@@ -16,14 +16,11 @@ pub type Balance = u64;
 pub type Share = u64;
 pub type PoolId = u32;
 pub type BlockNumber = u64;
-pub type CurrencyId = u32;
 
 pub const ALICE: AccountId = 1;
 pub const BOB: AccountId = 2;
 pub const CAROL: AccountId = 3;
 pub const DOT_POOL: PoolId = 1;
-pub const BTC_POOL: PoolId = 2;
-pub const XBTC_POOL: PoolId = 3;
 
 parameter_types! {
 	pub const BlockHashCount: u64 = 250;
@@ -59,41 +56,17 @@ thread_local! {
 }
 
 pub struct Handler;
-impl RewardHandler<AccountId, BlockNumber> for Handler {
-	type Share = Share;
+impl RewardHandler<AccountId> for Handler {
 	type Balance = Balance;
 	type PoolId = PoolId;
-	type CurrencyId = CurrencyId;
-
-	fn accumulate_reward(
-		now: BlockNumber,
-		mut callback: impl FnMut(Self::PoolId, Self::Balance),
-	) -> Vec<(Self::CurrencyId, Self::Balance)> {
-		if now % 2 == 0 {
-			let mut total_accumulated_rewards = 0;
-			let valid_pool_ids = vec![DOT_POOL, BTC_POOL];
-
-			for (pool, _) in Pools::<Runtime>::iter() {
-				if valid_pool_ids.contains(&pool) {
-					let rewards: Balance = 100;
-					callback(pool, rewards);
-					total_accumulated_rewards += rewards;
-				}
-			}
-
-			vec![(1, total_accumulated_rewards)]
-		} else {
-			vec![]
-		}
-	}
 
-	fn payout(who: &AccountId, pool: Self::PoolId, amount: Self::Balance) {
+	fn payout(who: &AccountId, pool: &Self::PoolId, amount: Self::Balance) {
 		RECEIVED_PAYOUT.with(|v| {
 			let mut old_map = v.borrow().clone();
-			if let Some(before) = old_map.get_mut(&(pool, *who)) {
+			if let Some(before) = old_map.get_mut(&(*pool, *who)) {
 				*before += amount;
 			} else {
-				old_map.insert((pool, *who), amount);
+				old_map.insert((*pool, *who), amount);
 			};
 
 			*v.borrow_mut() = old_map;
@@ -106,7 +79,6 @@ impl Config for Runtime {
 	type Balance = Balance;
 	type PoolId = PoolId;
 	type Handler = Handler;
-	type WeightInfo = ();
 }
 
 type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Runtime>;
diff --git a/rewards/src/tests.rs b/rewards/src/tests.rs
index b3d8b9d..cc2380d 100644
--- a/rewards/src/tests.rs
+++ b/rewards/src/tests.rs
@@ -18,7 +18,7 @@ fn add_share_should_work() {
 		);
 		assert_eq!(RewardsModule::share_and_withdrawn_reward(DOT_POOL, ALICE), (0, 0));
 
-		RewardsModule::add_share(&ALICE, DOT_POOL, 0);
+		RewardsModule::add_share(&ALICE, &DOT_POOL, 0);
 		assert_eq!(
 			RewardsModule::pools(DOT_POOL),
 			PoolInfo {
@@ -29,7 +29,7 @@ fn add_share_should_work() {
 		);
 		assert_eq!(RewardsModule::share_and_withdrawn_reward(DOT_POOL, ALICE), (0, 0));
 
-		RewardsModule::add_share(&ALICE, DOT_POOL, 100);
+		RewardsModule::add_share(&ALICE, &DOT_POOL, 100);
 		assert_eq!(
 			RewardsModule::pools(DOT_POOL),
 			PoolInfo {
@@ -54,7 +54,7 @@ fn add_share_should_work() {
 		);
 		assert_eq!(RewardsModule::share_and_withdrawn_reward(DOT_POOL, BOB), (0, 0));
 
-		RewardsModule::add_share(&BOB, DOT_POOL, 50);
+		RewardsModule::add_share(&BOB, &DOT_POOL, 50);
 		assert_eq!(
 			RewardsModule::pools(DOT_POOL),
 			PoolInfo {
@@ -65,7 +65,7 @@ fn add_share_should_work() {
 		);
 		assert_eq!(RewardsModule::share_and_withdrawn_reward(DOT_POOL, BOB), (50, 2500));
 
-		RewardsModule::add_share(&ALICE, DOT_POOL, 150);
+		RewardsModule::add_share(&ALICE, &DOT_POOL, 150);
 		assert_eq!(
 			RewardsModule::pools(DOT_POOL),
 			PoolInfo {
@@ -81,12 +81,12 @@ fn add_share_should_work() {
 #[test]
 fn claim_rewards_should_work() {
 	ExtBuilder::default().build().execute_with(|| {
-		RewardsModule::add_share(&ALICE, DOT_POOL, 100);
-		RewardsModule::add_share(&BOB, DOT_POOL, 100);
+		RewardsModule::add_share(&ALICE, &DOT_POOL, 100);
+		RewardsModule::add_share(&BOB, &DOT_POOL, 100);
 		Pools::<Runtime>::mutate(DOT_POOL, |pool_info| {
 			pool_info.total_rewards += 5000;
 		});
-		RewardsModule::add_share(&CAROL, DOT_POOL, 200);
+		RewardsModule::add_share(&CAROL, &DOT_POOL, 200);
 
 		assert_eq!(
 			RewardsModule::pools(DOT_POOL),
@@ -112,7 +112,7 @@ fn claim_rewards_should_work() {
 			0
 		);
 
-		RewardsModule::claim_rewards(&ALICE, DOT_POOL);
+		RewardsModule::claim_rewards(&ALICE, &DOT_POOL);
 		assert_eq!(
 			RewardsModule::pools(DOT_POOL),
 			PoolInfo {
@@ -127,7 +127,7 @@ fn claim_rewards_should_work() {
 			2500
 		);
 
-		RewardsModule::claim_rewards(&CAROL, DOT_POOL);
+		RewardsModule::claim_rewards(&CAROL, &DOT_POOL);
 		assert_eq!(
 			RewardsModule::pools(DOT_POOL),
 			PoolInfo {
@@ -142,7 +142,7 @@ fn claim_rewards_should_work() {
 			0
 		);
 
-		RewardsModule::claim_rewards(&BOB, DOT_POOL);
+		RewardsModule::claim_rewards(&BOB, &DOT_POOL);
 		assert_eq!(
 			RewardsModule::pools(DOT_POOL),
 			PoolInfo {
@@ -162,8 +162,8 @@ fn claim_rewards_should_work() {
 #[test]
 fn remove_share_should_work() {
 	ExtBuilder::default().build().execute_with(|| {
-		RewardsModule::add_share(&ALICE, DOT_POOL, 100);
-		RewardsModule::add_share(&BOB, DOT_POOL, 100);
+		RewardsModule::add_share(&ALICE, &DOT_POOL, 100);
+		RewardsModule::add_share(&BOB, &DOT_POOL, 100);
 		Pools::<Runtime>::mutate(DOT_POOL, |pool_info| {
 			pool_info.total_rewards += 10000;
 		});
@@ -188,7 +188,7 @@ fn remove_share_should_work() {
 		);
 
 		// remove amount is zero, do not claim interest
-		RewardsModule::remove_share(&ALICE, DOT_POOL, 0);
+		RewardsModule::remove_share(&ALICE, &DOT_POOL, 0);
 		assert_eq!(
 			RewardsModule::pools(DOT_POOL),
 			PoolInfo {
@@ -203,7 +203,7 @@ fn remove_share_should_work() {
 			0
 		);
 
-		RewardsModule::remove_share(&BOB, DOT_POOL, 50);
+		RewardsModule::remove_share(&BOB, &DOT_POOL, 50);
 		assert_eq!(
 			RewardsModule::pools(DOT_POOL),
 			PoolInfo {
@@ -218,7 +218,7 @@ fn remove_share_should_work() {
 			5000
 		);
 
-		RewardsModule::remove_share(&ALICE, DOT_POOL, 101);
+		RewardsModule::remove_share(&ALICE, &DOT_POOL, 101);
 		assert_eq!(
 			RewardsModule::pools(DOT_POOL),
 			PoolInfo {
@@ -248,7 +248,7 @@ fn set_share_should_work() {
 		);
 		assert_eq!(RewardsModule::share_and_withdrawn_reward(DOT_POOL, ALICE), (0, 0));
 
-		RewardsModule::set_share(&ALICE, DOT_POOL, 100);
+		RewardsModule::set_share(&ALICE, &DOT_POOL, 100);
 		assert_eq!(
 			RewardsModule::pools(DOT_POOL),
 			PoolInfo {
@@ -271,7 +271,7 @@ fn set_share_should_work() {
 			}
 		);
 
-		RewardsModule::set_share(&ALICE, DOT_POOL, 500);
+		RewardsModule::set_share(&ALICE, &DOT_POOL, 500);
 		assert_eq!(
 			RewardsModule::pools(DOT_POOL),
 			PoolInfo {
@@ -286,7 +286,7 @@ fn set_share_should_work() {
 			0
 		);
 
-		RewardsModule::set_share(&ALICE, DOT_POOL, 100);
+		RewardsModule::set_share(&ALICE, &DOT_POOL, 100);
 		assert_eq!(
 			RewardsModule::pools(DOT_POOL),
 			PoolInfo {
@@ -304,43 +304,18 @@ fn set_share_should_work() {
 }
 
 #[test]
-fn on_initialize_should_work() {
+fn accumulate_reward_should_work() {
 	ExtBuilder::default().build().execute_with(|| {
-		Pools::<Runtime>::mutate(DOT_POOL, |pool_info| {
-			pool_info.total_rewards += 100;
-		});
-		Pools::<Runtime>::mutate(BTC_POOL, |pool_info| {
-			pool_info.total_rewards += 200;
-		});
-		Pools::<Runtime>::mutate(XBTC_POOL, |pool_info| {
-			pool_info.total_rewards += 300;
-		});
 		assert_eq!(
 			RewardsModule::pools(DOT_POOL),
 			PoolInfo {
 				total_shares: 0,
-				total_rewards: 100,
-				total_withdrawn_rewards: 0,
-			}
-		);
-		assert_eq!(
-			RewardsModule::pools(BTC_POOL),
-			PoolInfo {
-				total_shares: 0,
-				total_rewards: 200,
-				total_withdrawn_rewards: 0,
-			}
-		);
-		assert_eq!(
-			RewardsModule::pools(XBTC_POOL),
-			PoolInfo {
-				total_shares: 0,
-				total_rewards: 300,
+				total_rewards: 0,
 				total_withdrawn_rewards: 0,
 			}
 		);
 
-		RewardsModule::on_initialize(1);
+		RewardsModule::accumulate_reward(&DOT_POOL, 100);
 		assert_eq!(
 			RewardsModule::pools(DOT_POOL),
 			PoolInfo {
@@ -349,47 +324,5 @@ fn on_initialize_should_work() {
 				total_withdrawn_rewards: 0,
 			}
 		);
-		assert_eq!(
-			RewardsModule::pools(BTC_POOL),
-			PoolInfo {
-				total_shares: 0,
-				total_rewards: 200,
-				total_withdrawn_rewards: 0,
-			}
-		);
-		assert_eq!(
-			RewardsModule::pools(XBTC_POOL),
-			PoolInfo {
-				total_shares: 0,
-				total_rewards: 300,
-				total_withdrawn_rewards: 0,
-			}
-		);
-
-		RewardsModule::on_initialize(2);
-		assert_eq!(
-			RewardsModule::pools(DOT_POOL),
-			PoolInfo {
-				total_shares: 0,
-				total_rewards: 200,
-				total_withdrawn_rewards: 0,
-			}
-		);
-		assert_eq!(
-			RewardsModule::pools(BTC_POOL),
-			PoolInfo {
-				total_shares: 0,
-				total_rewards: 300,
-				total_withdrawn_rewards: 0,
-			}
-		);
-		assert_eq!(
-			RewardsModule::pools(XBTC_POOL),
-			PoolInfo {
-				total_shares: 0,
-				total_rewards: 300,
-				total_withdrawn_rewards: 0,
-			}
-		);
 	});
 }
diff --git a/traits/src/rewards.rs b/traits/src/rewards.rs
index e53b905..ebf21f4 100644
--- a/traits/src/rewards.rs
+++ b/traits/src/rewards.rs
@@ -1,27 +1,15 @@
 use codec::FullCodec;
 use sp_runtime::traits::{AtLeast32BitUnsigned, MaybeSerializeDeserialize};
-use sp_std::{fmt::Debug, vec::Vec};
+use sp_std::fmt::Debug;
 
 /// Hooks to manage reward pool
-pub trait RewardHandler<AccountId, BlockNumber> {
-	/// The share type of pool
-	type Share: AtLeast32BitUnsigned + Default + Copy + MaybeSerializeDeserialize + Debug;
-
+pub trait RewardHandler<AccountId> {
 	/// The reward balance type
 	type Balance: AtLeast32BitUnsigned + Default + Copy + MaybeSerializeDeserialize + Debug;
 
 	/// The reward pool ID type
-	type PoolId: Copy + FullCodec;
-
-	/// The currency type
-	type CurrencyId: FullCodec + Eq + PartialEq + Copy + MaybeSerializeDeserialize + Debug;
-
-	/// Accumulate rewards
-	fn accumulate_reward(
-		now: BlockNumber,
-		callback: impl FnMut(Self::PoolId, Self::Balance),
-	) -> Vec<(Self::CurrencyId, Self::Balance)>;
+	type PoolId: FullCodec;
 
 	/// Payout the reward to `who`
-	fn payout(who: &AccountId, pool: Self::PoolId, amount: Self::Balance);
+	fn payout(who: &AccountId, pool: &Self::PoolId, amount: Self::Balance);
 }
-- 
GitLab