[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 {
@@ -2774,45 +2782,6 @@ sp_api::impl_runtime_apis! {
 	}
 }
 
-#[cfg(all(test, feature = "try-runtime"))]
-mod remote_tests {
-	use super::*;
-	use frame_try_runtime::{runtime_decl_for_try_runtime::TryRuntime, UpgradeCheckSelect};
-	use remote_externalities::{
-		Builder, Mode, OfflineConfig, OnlineConfig, SnapshotConfig, Transport,
-	};
-	use std::env::var;
-
-	#[tokio::test]
-	async fn run_migrations() {
-		if var("RUN_MIGRATION_TESTS").is_err() {
-			return;
-		}
-
-		sp_tracing::try_init_simple();
-		let transport: Transport =
-			var("WS").unwrap_or("wss://westend-rpc.polkadot.io:443".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),
-						..Default::default()
-					},
-				)
-			} else {
-				Mode::Online(OnlineConfig { transport, ..Default::default() })
-			})
-			.build()
-			.await
-			.unwrap();
-		ext.execute_with(|| Runtime::on_runtime_upgrade(UpgradeCheckSelect::PreAndPost));
-	}
-}
-
 mod clean_state_migration {
 	use super::Runtime;
 	#[cfg(feature = "try-runtime")]

polkadot/runtime/westend/src/tests.rs

@@ -99,3 +99,140 @@ fn check_treasury_pallet_id() {
 		westend_runtime_constants::TREASURY_PALLET_ID
 	);
 }
+
+#[cfg(all(test, feature = "try-runtime"))]
+mod remote_tests {
+	use super::*;
+	use frame_try_runtime::{runtime_decl_for_try_runtime::TryRuntime, UpgradeCheckSelect};
+	use remote_externalities::{
+		Builder, Mode, OfflineConfig, OnlineConfig, SnapshotConfig, Transport,
+	};
+	use std::env::var;
+
+	#[tokio::test]
+	async fn run_migrations() {
+		if var("RUN_MIGRATION_TESTS").is_err() {
+			return;
+		}
+
+		sp_tracing::try_init_simple();
+		let transport: Transport =
+			var("WS").unwrap_or("wss://westend-rpc.polkadot.io:443".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),
+						..Default::default()
+					},
+				)
+			} else {
+				Mode::Online(OnlineConfig { transport, ..Default::default() })
+			})
+			.build()
+			.await
+			.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 direct_stakers = 0;
+			let mut unexpected_errors = 0;
+
+			// iterate over all pool members
+			pallet_nomination_pools::PoolMembers::<Runtime>::iter_keys().for_each(|k| {
+				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 migration = pallet_nomination_pools::Pallet::<Runtime>::migrate_delegation(
+						RuntimeOrigin::signed(alice.clone()).into(),
+						sp_runtime::MultiAddress::Id(k.clone()),
+					);
+
+					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: {}, direct_stakers: {}, unexpected_errors: {}",
+				success,
+				direct_stakers,
+				unexpected_errors
+			);
+		});
+	}
+}

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>;