incrementing provider fixes users migrating with zero balance

substrate/frame/delegated-staking/src/lib.rs

@@ -483,7 +483,8 @@ impl<T: Config> Pallet<T> {
 		let proxy_delegator = Self::generate_proxy_delegator(Agent::from(who.clone()));
 
 		// Keep proxy delegator alive until all funds are migrated.
-		frame_system::Pallet::<T>::inc_providers(&proxy_delegator.clone().get());
+		// TODO (ank4n) don't need it.
+		// frame_system::Pallet::<T>::inc_providers(&proxy_delegator.clone().get());
 
 		// Get current stake
 		let stake = T::CoreStaking::stake(who)?;
@@ -550,8 +551,6 @@ impl<T: Config> Pallet<T> {
 		let delegator = delegator.get();
 
 		let mut ledger = AgentLedger::<T>::get(&agent).ok_or(Error::<T>::NotAgent)?;
-		// try to hold the funds.
-		T::Currency::hold(&HoldReason::StakingDelegation.into(), &delegator, amount)?;
 
 		let new_delegation_amount =
 			if let Some(existing_delegation) = Delegation::<T>::get(&delegator) {
@@ -561,10 +560,15 @@ impl<T: Config> Pallet<T> {
 					.checked_add(&amount)
 					.ok_or(ArithmeticError::Overflow)?
 			} else {
+				// if this is the first time delegation, increment provider count.
+				frame_system::Pallet::<T>::inc_providers(&delegator);
 				amount
 			};
 
-		Delegation::<T>::new(&agent, new_delegation_amount).update_or_kill(&delegator);
+		// try to hold the funds.
+		T::Currency::hold(&HoldReason::StakingDelegation.into(), &delegator, amount)?;
+
+		let _ = Delegation::<T>::new(&agent, new_delegation_amount).update(&delegator);
 		ledger.total_delegated =
 			ledger.total_delegated.checked_add(&amount).ok_or(ArithmeticError::Overflow)?;
 		ledger.update(&agent);
@@ -636,7 +640,12 @@ impl<T: Config> Pallet<T> {
 			.defensive_ok_or(ArithmeticError::Overflow)?;
 
 		// remove delegator if nothing delegated anymore
-		delegation.update_or_kill(&delegator);
+		let delegation_killed = delegation.update(&delegator);
+
+		// if delegation killed, decrement provider count.
+		if delegation_killed {
+			frame_system::Pallet::<T>::dec_providers(&delegator);
+		}
 
 		let released = T::Currency::release(
 			&HoldReason::StakingDelegation.into(),
@@ -673,14 +682,19 @@ impl<T: Config> Pallet<T> {
 
 		let agent = source_delegation.agent.clone();
 		// update delegations
-		Delegation::<T>::new(&agent, amount).update_or_kill(&destination_delegator);
+		let _ = Delegation::<T>::new(&agent, amount).update(&destination_delegator);
+		frame_system::Pallet::<T>::inc_providers(&destination_delegator);
 
 		source_delegation.amount = source_delegation
 			.amount
 			.checked_sub(&amount)
 			.defensive_ok_or(Error::<T>::BadState)?;
 
-		source_delegation.update_or_kill(&source_delegator);
+		let delegation_killed = source_delegation.update(&source_delegator);
+		// if delegation killed, decrement provider count.
+		if delegation_killed {
+			frame_system::Pallet::<T>::dec_providers(&source_delegator);
+		}
 
 		// transfer the released amount to `destination_delegator`.
 		let _ = T::Currency::transfer_on_hold(
@@ -740,7 +754,8 @@ impl<T: Config> Pallet<T> {
 		agent_ledger.remove_slash(actual_slash).save();
 		delegation.amount =
 			delegation.amount.checked_sub(&actual_slash).ok_or(ArithmeticError::Overflow)?;
-		delegation.update_or_kill(&delegator);
+		// TODO(ank4n) See what to do
+		let _ = delegation.update(&delegator);
 
 		if let Some(reporter) = maybe_reporter {
 			let reward_payout: BalanceOf<T> = T::SlashRewardFraction::get() * actual_slash;

substrate/frame/delegated-staking/src/tests.rs

@@ -334,6 +334,39 @@ fn apply_pending_slash() {
 	});
 }
 
+#[test]
+fn allow_full_amount_to_be_delegated() {
+	ExtBuilder::default().build_and_execute(|| {
+		let agent: AccountId = 200;
+		let reward_acc: AccountId = 201;
+		let delegator: AccountId = 300;
+
+		// set intention to accept delegation.
+		fund(&agent, 1000);
+		assert_ok!(DelegatedStaking::register_agent(
+			RawOrigin::Signed(agent).into(),
+			reward_acc
+		));
+
+		// delegate to this account
+		fund(&delegator, 1000);
+		assert_ok!(DelegatedStaking::delegate_to_agent(
+			RawOrigin::Signed(delegator).into(),
+			agent,
+			1000
+		));
+
+		// verify
+		assert!(DelegatedStaking::is_agent(&agent));
+		assert_eq!(DelegatedStaking::stakeable_balance(Agent::from(agent)), 1000);
+		assert_eq!(
+			Balances::balance_on_hold(&HoldReason::StakingDelegation.into(), &delegator),
+			1000
+		);
+		assert_eq!(DelegatedStaking::held_balance_of(Delegator::from(delegator)), 1000);
+	});
+}
+
 /// Integration tests with pallet-staking.
 mod staking_integration {
 	use super::*;

substrate/frame/delegated-staking/src/types.rs

@@ -64,15 +64,18 @@ impl<T: Config> Delegation<T> {
 			)
 	}
 
-	/// Save self to storage. If the delegation amount is zero, remove the delegation.
-	pub(crate) fn update_or_kill(self, key: &T::AccountId) {
+	/// Save self to storage.
+	///
+	/// If the delegation amount is zero, remove the delegation and returns true.
+	pub(crate) fn update(self, key: &T::AccountId) -> bool {
 		// Clean up if no delegation left.
 		if self.amount == Zero::zero() {
 			<Delegators<T>>::remove(key);
-			return
+			return true
 		}
 
-		<Delegators<T>>::insert(key, self)
+		<Delegators<T>>::insert(key, self);
+		false
 	}
 }
 

substrate/frame/nomination-pools/test-delegate-stake/src/lib.rs

@@ -1113,8 +1113,9 @@ fn pool_migration_e2e() {
 		assert_eq!(Balances::total_balance(&21), pre_migrate_balance_21 + 10);
 
 		// MIGRATE 22
-		let pre_migrate_balance_22 = Balances::total_balance(&22);
 		assert_eq!(Balances::total_balance_on_hold(&22), 0);
+		// make balance of 22 as 0.
+		let _ = Balances::make_free_balance_be(&22, 0);
 
 		// migrate delegation for 22.
 		assert!(Pools::api_member_needs_delegate_migration(22));
@@ -1128,7 +1129,7 @@ fn pool_migration_e2e() {
 		);
 
 		// tokens moved to 22's account and held there.
-		assert_eq!(Balances::total_balance(&22), pre_migrate_balance_22 + 10);
+		assert_eq!(Balances::total_balance(&22), 10);
 		assert_eq!(Balances::total_balance_on_hold(&22), 10);
 
 		// withdraw fails since 22 only unbonds at era 8.