[Pools] Fix issues with member migration to DelegateStake

polkadot/runtime/westend/src/lib.rs

@@ -99,7 +99,7 @@ use sp_runtime::{
 		IdentityLookup, Keccak256, OpaqueKeys, SaturatedConversion, Verify,
 	},
 	transaction_validity::{TransactionPriority, TransactionSource, TransactionValidity},
-	ApplyExtrinsicResult, FixedU128, KeyTypeId, Perbill, Percent, Permill,
+	ApplyExtrinsicResult, FixedU128, KeyTypeId, Percent, Permill,
 };
 use sp_staking::SessionIndex;
 #[cfg(any(feature = "std", test))]
@@ -2474,6 +2474,14 @@ sp_api::impl_runtime_apis! {
 		fn member_needs_delegate_migration(member: AccountId) -> bool {
 			NominationPools::api_member_needs_delegate_migration(member)
 		}
+
+		fn member_total_balance(member: AccountId) -> Balance {
+			NominationPools::api_member_total_balance(member)
+		}
+
+		fn pool_balance(pool_id: pallet_nomination_pools::PoolId) -> Balance {
+			NominationPools::api_pool_balance(pool_id)
+		}
 	}
 
 	impl pallet_staking_runtime_api::StakingApi<Block, Balance, AccountId> for Runtime {
@@ -2811,6 +2819,114 @@ mod remote_tests {
 			.unwrap();
 		ext.execute_with(|| Runtime::on_runtime_upgrade(UpgradeCheckSelect::PreAndPost));
 	}
+
+	#[tokio::test]
+	async fn delegate_stake_migration() {
+		// Intended to be run only manually.
+		if var("RUN_MIGRATION_TESTS").is_err() {
+			return;
+		}
+		use frame_support::assert_ok;
+		sp_tracing::try_init_simple();
+
+		let transport: Transport = var("WS").unwrap_or("ws://127.0.0.1:9900".to_string()).into();
+		let maybe_state_snapshot: Option<SnapshotConfig> = var("SNAP").map(|s| s.into()).ok();
+		let mut ext = Builder::<Block>::default()
+			.mode(if let Some(state_snapshot) = maybe_state_snapshot {
+				Mode::OfflineOrElseOnline(
+					OfflineConfig { state_snapshot: state_snapshot.clone() },
+					OnlineConfig {
+						transport,
+						state_snapshot: Some(state_snapshot),
+						pallets: vec![
+							"staking".into(),
+							"system".into(),
+							"balances".into(),
+							"nomination-pools".into(),
+							"delegated-staking".into(),
+						],
+						..Default::default()
+					},
+				)
+			} else {
+				Mode::Online(OnlineConfig { transport, ..Default::default() })
+			})
+			.build()
+			.await
+			.unwrap();
+		ext.execute_with(|| {
+			// create an account with some balance
+			let alice = AccountId::from([1u8; 32]);
+			use frame_support::traits::Currency;
+			let _ = Balances::deposit_creating(&alice, 100_000 * UNITS);
+
+			// iterate over all pools
+			pallet_nomination_pools::BondedPools::<Runtime>::iter_keys().for_each(|k| {
+				if pallet_nomination_pools::Pallet::<Runtime>::api_pool_needs_delegate_migration(k)
+				{
+					assert_ok!(
+						pallet_nomination_pools::Pallet::<Runtime>::migrate_pool_to_delegate_stake(
+							RuntimeOrigin::signed(alice.clone()).into(),
+							k,
+						)
+					);
+				}
+			});
+
+			// member migration stats
+			let mut success = 0;
+			let mut members_below_ed = 0;
+			let mut direct_stakers = 0;
+			let mut unexpected_errors = 0;
+
+			// iterate over all pool members
+			pallet_nomination_pools::PoolMembers::<Runtime>::iter().for_each(|(k, member)| {
+				if pallet_nomination_pools::Pallet::<Runtime>::api_member_needs_delegate_migration(
+					k.clone(),
+				) {
+					// reasons migrations can fail:
+					let is_direct_staker = pallet_staking::Bonded::<Runtime>::contains_key(&k);
+					let member_balance_below_ed = member.total_balance() < ExistentialDeposit::get();
+
+					let migration = pallet_nomination_pools::Pallet::<Runtime>::migrate_delegation(
+						RuntimeOrigin::signed(alice.clone()).into(),
+						sp_runtime::MultiAddress::Id(k.clone()),
+					);
+
+					if member_balance_below_ed {
+						// if the member has balance below ED, the migration should fail.
+						members_below_ed += 1;
+						assert_eq!(
+							migration.unwrap_err(),
+							pallet_nomination_pools::Error::<Runtime>::MinimumBondNotMet.into()
+						);
+					} else if is_direct_staker {
+						// if the member is a direct staker, the migration should fail until pool
+						// member unstakes all funds from pallet-staking.
+						direct_stakers += 1;
+						assert_eq!(
+							migration.unwrap_err(),
+							pallet_delegated_staking::Error::<Runtime>::AlreadyStaking.into()
+						);
+					} else if migration.is_err() {
+						unexpected_errors += 1;
+						log::error!(target: "remote_test", "Unexpected error {:?} while migrating {:?}", migration.unwrap_err(), k);
+					} else {
+						success += 1;
+					}
+				}
+			});
+
+			log::info!(
+				target: "remote_test",
+				"Migration stats: success: {}, members_below_ed: {}, direct_stakers: {}, unexpected_errors: {}",
+				success,
+				members_below_ed,
+				direct_stakers,
+				unexpected_errors
+			);
+		});
+	}
 }
 
 mod clean_state_migration {

prdoc/pr_4822.prdoc

@@ -0,0 +1,25 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: Ensure as many as possible pool members can migrate to `DelegateStake`
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      1. Allows pool members to use their total balance while joining pool with `DelegateStake`.
+      2. Gates call mutating pool or member in unmigrated state.
+      3. Runtime apis for reading pool and member balance.
+
+crates:
+  - name: westend-runtime
+    bump: minor
+  - name: kitchensink-runtime
+    bump: patch
+  - name: pallet-delegated-staking
+    bump: patch
+  - name: pallet-nomination-pools
+    bump: minor
+  - name: sp-staking
+    bump: patch
+  - name: pallet-nomination-pools-runtime-api
+    bump: minor

substrate/bin/node/runtime/src/lib.rs

@@ -2782,6 +2782,14 @@ impl_runtime_apis! {
 		fn member_needs_delegate_migration(member: AccountId) -> bool {
 			NominationPools::api_member_needs_delegate_migration(member)
 		}
+
+		fn member_total_balance(member: AccountId) -> Balance {
+			NominationPools::api_member_total_balance(member)
+		}
+
+		fn pool_balance(pool_id: pallet_nomination_pools::PoolId) -> Balance {
+			NominationPools::api_pool_balance(pool_id)
+		}
 	}
 
 	impl pallet_staking_runtime_api::StakingApi<Block, Balance, AccountId> for Runtime {

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

@@ -19,7 +19,7 @@
 //! Implementations of public traits, namely [`DelegationInterface`] and [`OnStakingUpdate`].
 
 use super::*;
-use sp_staking::{Agent, DelegationInterface, DelegationMigrator, Delegator, OnStakingUpdate};
+use sp_staking::{DelegationInterface, DelegationMigrator, OnStakingUpdate};
 
 impl<T: Config> DelegationInterface for Pallet<T> {
 	type Balance = BalanceOf<T>;
@@ -36,7 +36,7 @@ impl<T: Config> DelegationInterface for Pallet<T> {
 		Delegation::<T>::get(&delegator.get()).map(|d| d.amount)
 	}
 
-	/// Delegate funds to an `Agent`.
+	/// Register a new `Agent` and delegate funds to it.
 	fn delegate(
 		who: Delegator<Self::AccountId>,
 		agent: Agent<Self::AccountId>,