From 8d481f55c354988945f41e913529eb22295f5951 Mon Sep 17 00:00:00 2001
From: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Date: Sun, 28 Aug 2022 13:55:54 +0100
Subject: [PATCH] Fix nomination pools pending rewards RPC (#12095)

* Fix nomination pools pending rewards RPC

* Fix node

* Update frame/nomination-pools/src/lib.rs

* Fix docs
---
 substrate/bin/node/runtime/src/lib.rs         |  2 +-
 substrate/frame/nomination-pools/src/lib.rs   | 18 ++++---
 substrate/frame/nomination-pools/src/tests.rs | 53 +++++++++++++++++++
 3 files changed, 65 insertions(+), 8 deletions(-)

diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs
index ff793a49b5c..0816fedff03 100644
--- a/substrate/bin/node/runtime/src/lib.rs
+++ b/substrate/bin/node/runtime/src/lib.rs
@@ -1842,7 +1842,7 @@ impl_runtime_apis! {
 
 	impl pallet_nomination_pools_runtime_api::NominationPoolsApi<Block, AccountId, Balance> for Runtime {
 		fn pending_rewards(member_account: AccountId) -> Balance {
-			NominationPools::pending_rewards(member_account)
+			NominationPools::pending_rewards(member_account).unwrap_or_default()
 		}
 	}
 
diff --git a/substrate/frame/nomination-pools/src/lib.rs b/substrate/frame/nomination-pools/src/lib.rs
index 62d0b3ddd55..40db4ac6485 100644
--- a/substrate/frame/nomination-pools/src/lib.rs
+++ b/substrate/frame/nomination-pools/src/lib.rs
@@ -2156,16 +2156,20 @@ pub mod pallet {
 impl<T: Config> Pallet<T> {
 	/// Returns the pending rewards for the specified `member_account`.
 	///
-	/// In the case of error the function returns balance of zero.
-	pub fn pending_rewards(member_account: T::AccountId) -> BalanceOf<T> {
+	/// In the case of error, `None` is returned.
+	pub fn pending_rewards(member_account: T::AccountId) -> Option<BalanceOf<T>> {
 		if let Some(pool_member) = PoolMembers::<T>::get(member_account) {
-			if let Some(reward_pool) = RewardPools::<T>::get(pool_member.pool_id) {
-				return pool_member
-					.pending_rewards(reward_pool.last_recorded_reward_counter())
-					.unwrap_or_default()
+			if let Some((reward_pool, bonded_pool)) = RewardPools::<T>::get(pool_member.pool_id)
+				.zip(BondedPools::<T>::get(pool_member.pool_id))
+			{
+				let current_reward_counter = reward_pool
+					.current_reward_counter(pool_member.pool_id, bonded_pool.points)
+					.ok()?;
+				return pool_member.pending_rewards(current_reward_counter).ok()
 			}
 		}
-		BalanceOf::<T>::default()
+
+		None
 	}
 
 	/// The amount of bond that MUST REMAIN IN BONDED in ALL POOLS.
diff --git a/substrate/frame/nomination-pools/src/tests.rs b/substrate/frame/nomination-pools/src/tests.rs
index 5aa8e97266e..705f8ce3a64 100644
--- a/substrate/frame/nomination-pools/src/tests.rs
+++ b/substrate/frame/nomination-pools/src/tests.rs
@@ -1285,6 +1285,59 @@ mod claim_payout {
 		});
 	}
 
+	#[test]
+	fn pending_rewards_per_member_works() {
+		ExtBuilder::default().build_and_execute(|| {
+			let ed = Balances::minimum_balance();
+
+			assert_eq!(Pools::pending_rewards(10), Some(0));
+			Balances::mutate_account(&default_reward_account(), |f| f.free += 30).unwrap();
+			assert_eq!(Pools::pending_rewards(10), Some(30));
+			assert_eq!(Pools::pending_rewards(20), None);
+
+			Balances::make_free_balance_be(&20, ed + 10);
+			assert_ok!(Pools::join(Origin::signed(20), 10, 1));
+
+			assert_eq!(Pools::pending_rewards(10), Some(30));
+			assert_eq!(Pools::pending_rewards(20), Some(0));
+
+			Balances::mutate_account(&default_reward_account(), |f| f.free += 100).unwrap();
+
+			assert_eq!(Pools::pending_rewards(10), Some(30 + 50));
+			assert_eq!(Pools::pending_rewards(20), Some(50));
+			assert_eq!(Pools::pending_rewards(30), None);
+
+			Balances::make_free_balance_be(&30, ed + 10);
+			assert_ok!(Pools::join(Origin::signed(30), 10, 1));
+
+			assert_eq!(Pools::pending_rewards(10), Some(30 + 50));
+			assert_eq!(Pools::pending_rewards(20), Some(50));
+			assert_eq!(Pools::pending_rewards(30), Some(0));
+
+			Balances::mutate_account(&default_reward_account(), |f| f.free += 60).unwrap();
+
+			assert_eq!(Pools::pending_rewards(10), Some(30 + 50 + 20));
+			assert_eq!(Pools::pending_rewards(20), Some(50 + 20));
+			assert_eq!(Pools::pending_rewards(30), Some(20));
+
+			// 10 should claim 10, 20 should claim nothing.
+			assert_ok!(Pools::claim_payout(Origin::signed(10)));
+			assert_eq!(Pools::pending_rewards(10), Some(0));
+			assert_eq!(Pools::pending_rewards(20), Some(50 + 20));
+			assert_eq!(Pools::pending_rewards(30), Some(20));
+
+			assert_ok!(Pools::claim_payout(Origin::signed(20)));
+			assert_eq!(Pools::pending_rewards(10), Some(0));
+			assert_eq!(Pools::pending_rewards(20), Some(0));
+			assert_eq!(Pools::pending_rewards(30), Some(20));
+
+			assert_ok!(Pools::claim_payout(Origin::signed(30)));
+			assert_eq!(Pools::pending_rewards(10), Some(0));
+			assert_eq!(Pools::pending_rewards(20), Some(0));
+			assert_eq!(Pools::pending_rewards(30), Some(0));
+		});
+	}
+
 	#[test]
 	fn rewards_distribution_is_fair_bond_extra() {
 		ExtBuilder::default().build_and_execute(|| {
-- 
GitLab