if transfer reward failed, put back the rewards into pool (#2201)

Author: wangjj9219

modules/incentives/src/lib.rs

@@ -285,6 +285,7 @@ pub mod module {
 		#[transactional]
 		pub fn claim_rewards(origin: OriginFor<T>, pool_id: PoolId) -> DispatchResult {
 			let who = ensure_signed(origin)?;
+
 			Self::do_claim_rewards(who, pool_id)
 		}
 
@@ -516,15 +517,29 @@ impl<T: Config> Pallet<T> {
 				let deduction_amount = deduction_rate.saturating_mul_int(pending_reward).min(pending_reward);
 				if !deduction_amount.is_zero() {
 					// re-accumulate deduction to rewards pool if deduction amount is not zero
-					<orml_rewards::Pallet<T>>::accumulate_reward(&pool_id, currency_id, deduction_amount)?;
+					let _ = <orml_rewards::Pallet<T>>::accumulate_reward(&pool_id, currency_id, deduction_amount).map_err(|e| {
+						log::error!(
+							target: "incentives",
+							"accumulate_reward: failed to accumulate reward to non-existen pool {:?}, reward_currency_id {:?}, reward_amount {:?}: {:?}",
+							pool_id, currency_id, deduction_amount, e
+						);
+					});
 				}
 				(pending_reward.saturating_sub(deduction_amount), deduction_amount)
 			};
 
-			// transfer the actual reward(pending reward exclude deduction) to user from the pool. it should not
-			// affect the process, ignore the result to continue. if it fails, just the user will not
-			// be rewarded, there will not increase user balance.
-			T::Currency::transfer(currency_id, &Self::account_id(), &who, actual_amount)?;
+			// transfer to `who` maybe fail because of the reward amount is below ED and `who` is not alive.
+			// if transfer failed, do not throw err directly and try to put the tiny reward back to pool.
+			let res = T::Currency::transfer(currency_id, &Self::account_id(), &who, actual_amount);
+			if res.is_err() {
+				let _ = <orml_rewards::Pallet<T>>::accumulate_reward(&pool_id, currency_id, actual_amount).map_err(|e| {
+					log::error!(
+						target: "incentives",
+						"accumulate_reward: failed to accumulate reward to non-existen pool {:?}, reward_currency_id {:?}, reward_amount {:?}: {:?}",
+						pool_id, currency_id, actual_amount, e
+					);
+				});
+			}
 
 			Self::deposit_event(Event::ClaimRewards {
 				who: who.clone(),

modules/incentives/src/mock.rs

@@ -89,8 +89,12 @@ impl frame_system::Config for Runtime {
 }
 
 parameter_type_with_key! {
-	pub ExistentialDeposits: |_currency_id: CurrencyId| -> Balance {
-		Default::default()
+	pub ExistentialDeposits: |currency_id: CurrencyId| -> Balance {
+
+		match currency_id {
+			CurrencyId::Token(TokenSymbol::AUSD) => 10,
+			_ => Default::default()
+		}
 	};
 }
 

modules/incentives/src/tests.rs

@@ -512,6 +512,75 @@ fn payout_works() {
 	});
 }
 
+#[test]
+fn transfer_failed_when_claim_rewards() {
+	ExtBuilder::default().build().execute_with(|| {
+		assert_ok!(TokensModule::deposit(AUSD, &VAULT::get(), 100));
+		RewardsModule::add_share(&ALICE::get(), &PoolId::Loans(BTC), 100);
+		RewardsModule::add_share(&BOB::get(), &PoolId::Loans(BTC), 100);
+		assert_ok!(RewardsModule::accumulate_reward(&PoolId::Loans(BTC), AUSD, 18));
+
+		assert_eq!(TokensModule::free_balance(AUSD, &VAULT::get()), 100);
+		assert_eq!(TokensModule::free_balance(AUSD, &ALICE::get()), 0);
+		assert_eq!(
+			RewardsModule::pool_infos(PoolId::Loans(BTC)),
+			PoolInfo {
+				total_shares: 200,
+				rewards: vec![(AUSD, (18, 0))].into_iter().collect(),
+			}
+		);
+		assert_eq!(
+			RewardsModule::shares_and_withdrawn_rewards(PoolId::Loans(BTC), ALICE::get()),
+			(100, Default::default())
+		);
+
+		// Alice claim rewards, but the rewards are put back to pool because transfer rewards failed.
+		assert_ok!(IncentivesModule::claim_rewards(
+			Origin::signed(ALICE::get()),
+			PoolId::Loans(BTC)
+		));
+
+		assert_eq!(TokensModule::free_balance(AUSD, &VAULT::get()), 100);
+		assert_eq!(TokensModule::free_balance(AUSD, &ALICE::get()), 0);
+		assert_eq!(
+			RewardsModule::pool_infos(PoolId::Loans(BTC)),
+			PoolInfo {
+				total_shares: 200,
+				rewards: vec![(AUSD, (27, 9))].into_iter().collect(),
+			}
+		);
+		assert_eq!(
+			RewardsModule::shares_and_withdrawn_rewards(PoolId::Loans(BTC), ALICE::get()),
+			(100, vec![(AUSD, 9)].into_iter().collect())
+		);
+
+		assert_eq!(TokensModule::free_balance(AUSD, &BOB::get()), 0);
+		assert_eq!(
+			RewardsModule::shares_and_withdrawn_rewards(PoolId::Loans(BTC), BOB::get()),
+			(100, Default::default())
+		);
+
+		// BOB claim reward and receive the reward.
+		assert_ok!(IncentivesModule::claim_rewards(
+			Origin::signed(BOB::get()),
+			PoolId::Loans(BTC)
+		));
+		assert_eq!(TokensModule::free_balance(AUSD, &VAULT::get()), 87);
+		assert_eq!(TokensModule::free_balance(AUSD, &BOB::get()), 13);
+		assert_eq!(
+			RewardsModule::pool_infos(PoolId::Loans(BTC)),
+			PoolInfo {
+				total_shares: 200,
+				rewards: vec![(AUSD, (27, 22))].into_iter().collect(),
+			}
+		);
+		assert_eq!(
+			RewardsModule::shares_and_withdrawn_rewards(PoolId::Loans(BTC), BOB::get()),
+			(100, vec![(AUSD, 13)].into_iter().collect())
+		);
+	});
+}
+
 #[test]
 fn claim_rewards_works() {
 	ExtBuilder::default().build().execute_with(|| {