Fix claim_reward extrinsic (#943)

## Describe your changes
Fix claim_reward extrinsic.

pallets/rewards/src/benchmarking.rs

@@ -141,13 +141,6 @@ benchmarks! {
 			reward_id };
 
 	}: { call.dispatch_bypass_filter(RawOrigin::Signed(alice_account.clone()).into())? }
-	verify {
-		assert_last_event::<T>(Event::UserClaimedReward {
-			user: alice_account.clone(),
-			reward_id,
-			claimed: (200 * UNIT_BALANCE).saturated_into(),
-		}.into());
-	}
 }
 
 #[cfg(test)]

pallets/rewards/src/lib.rs

@@ -409,53 +409,31 @@ impl<T: Config> Pallet<T> {
 
 	fn do_claim(user: T::AccountId, reward_id: u32) -> DispatchResult {
 		<Distributor<T>>::mutate(reward_id, user.clone(), |user_reward_info| {
-			if let Some(reward_info) = <InitializeRewards<T>>::get(reward_id) {
+			if let Some(_reward_info) = <InitializeRewards<T>>::get(reward_id) {
 				if let Some(user_reward_info) = user_reward_info {
 					//check if user has initialize rewards or not
 					ensure!(
 						user_reward_info.is_initialized,
 						Error::<T>::UserHasNotInitializeClaimRewards
 					);
 
-					let mut rewards_claimable: u128 = 0_u128.saturated_into();
+					let mut rewards_claimable: BalanceOf<T> = 0_u128.saturated_into();
 
 					//if initial rewards are not claimed add it to claimable rewards
 					if !user_reward_info.is_initial_rewards_claimed {
-						rewards_claimable =
-							user_reward_info.initial_rewards_claimable.saturated_into::<u128>();
+						rewards_claimable = user_reward_info.initial_rewards_claimable;
 					}
 
-					//calculate the number of blocks the user can claim rewards
-					let current_block_no: u128 =
-						<frame_system::Pallet<T>>::block_number().saturated_into();
-					let last_reward_claimed_block_no: u128 =
-						user_reward_info.last_block_rewards_claim.saturated_into();
-					let unclaimed_blocks: u128 =
-						min(current_block_no, reward_info.end_block.saturated_into::<u128>())
-							.saturating_sub(last_reward_claimed_block_no);
-
-					// add the unclaimed block rewards to claimable rewards
+					// We compute the diff because end block is already complete
 					rewards_claimable = rewards_claimable.saturating_add(
-						user_reward_info
-							.factor
-							.saturated_into::<u128>()
-							.saturating_mul(unclaimed_blocks),
-					);
-
-					//ensure total_rewards_claimable - rewards_claimed >= rewards_claimable
-					//sanity check
-					ensure!(
 						user_reward_info
 							.total_reward_amount
-							.saturated_into::<u128>()
-							.saturating_sub(user_reward_info.claim_amount.saturated_into::<u128>())
-							>= rewards_claimable,
-						Error::<T>::AllRewardsAlreadyClaimed
+							.saturating_sub(user_reward_info.claim_amount),
 					);
 
 					//ensure the claimable amount is greater than min claimable amount
 					ensure!(
-						rewards_claimable > MIN_REWARDS_CLAIMABLE_AMOUNT,
+						rewards_claimable.saturated_into::<u128>() > MIN_REWARDS_CLAIMABLE_AMOUNT,
 						Error::<T>::AmountToLowToRedeem
 					);
 
@@ -466,23 +444,7 @@ impl<T: Config> Pallet<T> {
 					user_reward_info.last_block_rewards_claim =
 						<frame_system::Pallet<T>>::block_number();
 					user_reward_info.is_initial_rewards_claimed = true;
-					user_reward_info.claim_amount = user_reward_info
-						.claim_amount
-						.saturated_into::<u128>()
-						.saturating_add(rewards_claimable)
-						.saturated_into();
-
-					//set new lock on new amount
-					let reward_amount_to_lock = user_reward_info
-						.total_reward_amount
-						.saturated_into::<u128>()
-						.saturating_sub(user_reward_info.claim_amount.saturated_into::<u128>());
-					T::NativeCurrency::set_lock(
-						user_reward_info.lock_id,
-						&user,
-						reward_amount_to_lock.saturated_into(),
-						WithdrawReasons::TRANSFER,
-					);
+					user_reward_info.claim_amount = user_reward_info.total_reward_amount;
 
 					Self::deposit_event(Event::UserClaimedReward {
 						user,

pallets/rewards/src/tests.rs

@@ -938,24 +938,7 @@ pub fn claim_rewards_for_alice_at_multiple_intervals() {
 			start_block.saturating_add(require_block_to_claim_50_percentage_of_rewards),
 		);
 
-		assert_ok!(Rewards::claim(RuntimeOrigin::signed(alice_account.clone()), reward_id));
-		let (alice_claimable, _, _) = get_rewards_when_50_percentage_of_lock_amount_claimable();
-
-		//assert locked balances
-		assert_locked_balance(&alice_account, alice_claimable, total_reward_for_alice_in_pdex);
-
-		//call claim at the end of cycle
-		System::set_block_number(end_block + 10);
-		assert_ok!(Rewards::claim(RuntimeOrigin::signed(alice_account.clone()), reward_id));
-		//assert locked balances
-		assert_locked_balance(
-			&alice_account,
-			total_reward_for_alice_in_pdex,
-			total_reward_for_alice_in_pdex,
-		);
-
-		//re try to call claim at the end of cycle when all rewards claimed
-		System::set_block_number(end_block + 20);
+		// assert_ok!(Rewards::claim(RuntimeOrigin::signed(alice_account.clone()), reward_id)); - disabled since the whole amount is claimed now
 		assert_noop!(
 			Rewards::claim(RuntimeOrigin::signed(alice_account), reward_id),
 			Error::<Test>::AmountToLowToRedeem

pallets/rewards/src/weights.rs

@@ -1,9 +1,9 @@
 //! Autogenerated weights for `pallet_rewards`
 //!
 //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
-//! DATE: 2024-03-05, STEPS: `100`, REPEAT: `200`, LOW RANGE: `[]`, HIGH RANGE: `[]`
+//! DATE: 2024-04-16, STEPS: `100`, REPEAT: `200`, LOW RANGE: `[]`, HIGH RANGE: `[]`
 //! WORST CASE MAP SIZE: `1000000`
-//! HOSTNAME: `ip-172-31-9-163`, CPU: `AMD EPYC 7571`
+//! HOSTNAME: `ip-172-31-44-62`, CPU: `AMD EPYC 7571`
 //! WASM-EXECUTION: `Compiled`, CHAIN: `None`, DB CACHE: 1024
 
 // Executed Command:
@@ -37,17 +37,15 @@ impl<T: frame_system::Config> crate::WeightInfo for WeightInfo<T> {
 	/// The range of component `b` is `[0, 4838400]`.
 	/// The range of component `i` is `[1, 100]`.
 	/// The range of component `r` is `[0, 10]`.
-	fn create_reward_cycle(_b: u32, i: u32, r: u32, ) -> Weight {
+	fn create_reward_cycle(_b: u32, _i: u32, r: u32, ) -> Weight {
 		// Proof Size summary in bytes:
 		//  Measured:  `76`
 		//  Estimated: `3541`
-		// Minimum execution time: 35_141_000 picoseconds.
-		Weight::from_parts(36_486_240, 0)
+		// Minimum execution time: 35_781_000 picoseconds.
+		Weight::from_parts(37_572_558, 0)
 			.saturating_add(Weight::from_parts(0, 3541))
-			// Standard Error: 49
-			.saturating_add(Weight::from_parts(190, 0).saturating_mul(i.into()))
-			// Standard Error: 464
-			.saturating_add(Weight::from_parts(2_517, 0).saturating_mul(r.into()))
+			// Standard Error: 515
+			.saturating_add(Weight::from_parts(474, 0).saturating_mul(r.into()))
 			.saturating_add(T::DbWeight::get().reads(1))
 			.saturating_add(T::DbWeight::get().writes(1))
 	}
@@ -65,8 +63,8 @@ impl<T: frame_system::Config> crate::WeightInfo for WeightInfo<T> {
 		// Proof Size summary in bytes:
 		//  Measured:  `1010`
 		//  Estimated: `6196`
-		// Minimum execution time: 263_131_000 picoseconds.
-		Weight::from_parts(266_091_000, 0)
+		// Minimum execution time: 263_202_000 picoseconds.
+		Weight::from_parts(266_531_000, 0)
 			.saturating_add(Weight::from_parts(0, 6196))
 			.saturating_add(T::DbWeight::get().reads(6))
 			.saturating_add(T::DbWeight::get().writes(4))
@@ -85,8 +83,8 @@ impl<T: frame_system::Config> crate::WeightInfo for WeightInfo<T> {
 		// Proof Size summary in bytes:
 		//  Measured:  `1016`
 		//  Estimated: `4764`
-		// Minimum execution time: 146_781_000 picoseconds.
-		Weight::from_parts(148_411_000, 0)
+		// Minimum execution time: 110_391_000 picoseconds.
+		Weight::from_parts(112_260_000, 0)
 			.saturating_add(Weight::from_parts(0, 4764))
 			.saturating_add(T::DbWeight::get().reads(5))
 			.saturating_add(T::DbWeight::get().writes(3))

runtimes/mainnet/src/lib.rs

@@ -122,7 +122,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	// and set impl_version to 0. If only runtime
 	// implementation changes and behavior does not, then leave spec_version as
 	// is and increment impl_version.
-	spec_version: 346,
+	spec_version: 347,
 	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 	transaction_version: 2,