Backport of fix Pools 6->7 migration (#2942) (#3094)

This should result as a patched `24.0.1` version for
https://crates.io/crates/pallet-nomination-pools/24.0.0.

Relates to: polkadot-fellows/runtimes#137

---------

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: Alexander Samusev <[email protected]>
Co-authored-by: Dónal Murray <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: Adrian Catangiu <[email protected]>
Co-authored-by: PG Herveou <[email protected]>

.gitlab-ci.yml

@@ -21,7 +21,7 @@ workflow:
     - if: $CI_COMMIT_BRANCH
 
 variables:
-  CI_IMAGE: !reference [.ci-unified, variables, CI_IMAGE]
+  CI_IMAGE: "docker.io/paritytech/ci-unified:bullseye-1.74.0-2023-11-01-v20231204"
   # BUILDAH_IMAGE is defined in group variables
   BUILDAH_COMMAND: "buildah --storage-driver overlay2"
   RELENG_SCRIPTS_BRANCH: "master"

polkadot/node/core/prospective-parachains/src/tests.rs

@@ -101,11 +101,9 @@ fn test_harness<T: Future<Output = VirtualOverseer>>(
 
 	let mut view = View::new();
 	let subsystem = async move {
-		loop {
-			match run_iteration(&mut context, &mut view, &Metrics(None)).await {
-				Ok(()) => break,
-				Err(e) => panic!("{:?}", e),
-			}
+		match run_iteration(&mut context, &mut view, &Metrics(None)).await {
+			Ok(()) => {},
+			Err(e) => panic!("{:?}", e),
 		}
 
 		view

polkadot/xcm/pallet-xcm/src/mock.rs

@@ -589,10 +589,23 @@ impl super::benchmarking::Config for Test {
 		let asset_amount = 10u128;
 		let fee_amount = 2u128;
 
+		let existential_deposit = ExistentialDeposit::get();
+		let caller = frame_benchmarking::whitelisted_caller();
+
+		// Give some multiple of the existential deposit
+		let balance = asset_amount + existential_deposit * 1000;
+		let _ = <Balances as frame_support::traits::Currency<_>>::make_free_balance_be(
+			&caller, balance,
+		);
 		// create sufficient foreign asset USDT
 		let usdt_initial_local_amount = fee_amount * 10;
-		let (usdt_chain, _, usdt_id_multilocation) =
-			set_up_foreign_asset(USDT_PARA_ID, None, usdt_initial_local_amount, true);
+		let (usdt_chain, _, usdt_id_multilocation) = set_up_foreign_asset(
+			USDT_PARA_ID,
+			None,
+			caller.clone(),
+			usdt_initial_local_amount,
+			true,
+		);
 
 		// native assets transfer destination is USDT chain (teleport trust only for USDT)
 		let dest = usdt_chain;
@@ -602,20 +615,13 @@ impl super::benchmarking::Config for Test {
 			// native asset to transfer (not used for fees) - local reserve
 			(MultiLocation::here(), asset_amount).into(),
 		);
-
-		let existential_deposit = ExistentialDeposit::get();
-		let caller = frame_benchmarking::whitelisted_caller();
-		// Give some multiple of the existential deposit
-		let balance = asset_amount + existential_deposit * 1000;
-		let _ = <Balances as frame_support::traits::Currency<_>>::make_free_balance_be(
-			&caller, balance,
-		);
-		// verify initial balance
+		// verify initial balances
 		assert_eq!(Balances::free_balance(&caller), balance);
+		assert_eq!(Assets::balance(usdt_id_multilocation, &caller), usdt_initial_local_amount);
 
 		// verify transferred successfully
 		let verify = Box::new(move || {
-			// verify balance after transfer, decreased by transferred amount
+			// verify balances after transfer, decreased by transferred amounts
 			assert_eq!(Balances::free_balance(&caller), balance - asset_amount);
 			assert_eq!(
 				Assets::balance(usdt_id_multilocation, &caller),

polkadot/xcm/pallet-xcm/src/tests/assets_transfer.rs

@@ -244,6 +244,7 @@ fn reserve_transfer_assets_with_paid_router_works() {
 pub(crate) fn set_up_foreign_asset(
 	reserve_para_id: u32,
 	inner_junction: Option<Junction>,
+	benficiary: AccountId,
 	initial_amount: u128,
 	is_sufficient: bool,
 ) -> (MultiLocation, AccountId, MultiLocation) {
@@ -271,7 +272,7 @@ pub(crate) fn set_up_foreign_asset(
 	assert_ok!(Assets::mint(
 		RuntimeOrigin::signed(BOB),
 		foreign_asset_id_multilocation,
-		ALICE,
+		benficiary,
 		initial_amount
 	));
 
@@ -440,6 +441,7 @@ fn destination_asset_reserve_and_local_fee_reserve_call<Call>(
 			set_up_foreign_asset(
 				FOREIGN_ASSET_RESERVE_PARA_ID,
 				Some(FOREIGN_ASSET_INNER_JUNCTION),
+				ALICE,
 				foreign_initial_amount,
 				false,
 			);
@@ -595,6 +597,7 @@ fn remote_asset_reserve_and_local_fee_reserve_call_disallowed<Call>(
 		let (_, _, foreign_asset_id_multilocation) = set_up_foreign_asset(
 			FOREIGN_ASSET_RESERVE_PARA_ID,
 			Some(FOREIGN_ASSET_INNER_JUNCTION),
+			ALICE,
 			foreign_initial_amount,
 			false,
 		);
@@ -712,6 +715,7 @@ fn local_asset_reserve_and_destination_fee_reserve_call<Call>(
 			set_up_foreign_asset(
 				USDC_RESERVE_PARA_ID,
 				Some(USDC_INNER_JUNCTION),
+				ALICE,
 				usdc_initial_local_amount,
 				true,
 			);
@@ -870,6 +874,7 @@ fn destination_asset_reserve_and_destination_fee_reserve_call<Call>(
 			set_up_foreign_asset(
 				FOREIGN_ASSET_RESERVE_PARA_ID,
 				Some(FOREIGN_ASSET_INNER_JUNCTION),
+				ALICE,
 				foreign_initial_amount,
 				true,
 			);
@@ -1013,6 +1018,7 @@ fn remote_asset_reserve_and_destination_fee_reserve_call_disallowed<Call>(
 		let (usdc_chain, _, usdc_id_multilocation) = set_up_foreign_asset(
 			USDC_RESERVE_PARA_ID,
 			Some(USDC_INNER_JUNCTION),
+			ALICE,
 			usdc_initial_local_amount,
 			true,
 		);
@@ -1022,6 +1028,7 @@ fn remote_asset_reserve_and_destination_fee_reserve_call_disallowed<Call>(
 		let (_, _, foreign_asset_id_multilocation) = set_up_foreign_asset(
 			FOREIGN_ASSET_RESERVE_PARA_ID,
 			Some(FOREIGN_ASSET_INNER_JUNCTION),
+			ALICE,
 			foreign_initial_amount,
 			false,
 		);
@@ -1135,6 +1142,7 @@ fn local_asset_reserve_and_remote_fee_reserve_call_disallowed<Call>(
 		let (_, usdc_chain_sovereign_account, usdc_id_multilocation) = set_up_foreign_asset(
 			USDC_RESERVE_PARA_ID,
 			Some(USDC_INNER_JUNCTION),
+			ALICE,
 			usdc_initial_local_amount,
 			true,
 		);
@@ -1244,6 +1252,7 @@ fn destination_asset_reserve_and_remote_fee_reserve_call_disallowed<Call>(
 		let (_, usdc_chain_sovereign_account, usdc_id_multilocation) = set_up_foreign_asset(
 			USDC_RESERVE_PARA_ID,
 			Some(USDC_INNER_JUNCTION),
+			ALICE,
 			usdc_initial_local_amount,
 			true,
 		);
@@ -1254,6 +1263,7 @@ fn destination_asset_reserve_and_remote_fee_reserve_call_disallowed<Call>(
 			set_up_foreign_asset(
 				FOREIGN_ASSET_RESERVE_PARA_ID,
 				Some(FOREIGN_ASSET_INNER_JUNCTION),
+				ALICE,
 				foreign_initial_amount,
 				false,
 			);
@@ -1383,6 +1393,7 @@ fn remote_asset_reserve_and_remote_fee_reserve_call<Call>(
 			set_up_foreign_asset(
 				USDC_RESERVE_PARA_ID,
 				Some(USDC_INNER_JUNCTION),
+				ALICE,
 				usdc_initial_local_amount,
 				true,
 			);
@@ -1526,7 +1537,7 @@ fn local_asset_reserve_and_teleported_fee_call<Call>(
 		// create sufficient foreign asset USDT
 		let usdt_initial_local_amount = 42;
 		let (usdt_chain, usdt_chain_sovereign_account, usdt_id_multilocation) =
-			set_up_foreign_asset(USDT_PARA_ID, None, usdt_initial_local_amount, true);
+			set_up_foreign_asset(USDT_PARA_ID, None, ALICE, usdt_initial_local_amount, true);
 
 		// native assets transfer destination is USDT chain (teleport trust only for USDT)
 		let dest = usdt_chain;
@@ -1675,14 +1686,15 @@ fn destination_asset_reserve_and_teleported_fee_call<Call>(
 		// create sufficient foreign asset USDT
 		let usdt_initial_local_amount = 42;
 		let (_, usdt_chain_sovereign_account, usdt_id_multilocation) =
-			set_up_foreign_asset(USDT_PARA_ID, None, usdt_initial_local_amount, true);
+			set_up_foreign_asset(USDT_PARA_ID, None, ALICE, usdt_initial_local_amount, true);
 
 		// create non-sufficient foreign asset BLA
 		let foreign_initial_amount = 142;
 		let (reserve_location, foreign_sovereign_account, foreign_asset_id_multilocation) =
 			set_up_foreign_asset(
 				FOREIGN_ASSET_RESERVE_PARA_ID,
 				Some(FOREIGN_ASSET_INNER_JUNCTION),
+				ALICE,
 				foreign_initial_amount,
 				false,
 			);
@@ -1845,13 +1857,14 @@ fn remote_asset_reserve_and_teleported_fee_reserve_call_disallowed<Call>(
 		// create sufficient foreign asset USDT
 		let usdt_initial_local_amount = 42;
 		let (usdt_chain, usdt_chain_sovereign_account, usdt_id_multilocation) =
-			set_up_foreign_asset(USDT_PARA_ID, None, usdt_initial_local_amount, true);
+			set_up_foreign_asset(USDT_PARA_ID, None, ALICE, usdt_initial_local_amount, true);
 
 		// create non-sufficient foreign asset BLA
 		let foreign_initial_amount = 142;
 		let (_, reserve_sovereign_account, foreign_asset_id_multilocation) = set_up_foreign_asset(
 			FOREIGN_ASSET_RESERVE_PARA_ID,
 			Some(FOREIGN_ASSET_INNER_JUNCTION),
+			ALICE,
 			foreign_initial_amount,
 			false,
 		);
@@ -1952,7 +1965,7 @@ fn reserve_transfer_assets_with_teleportable_asset_disallowed() {
 		// create sufficient foreign asset USDT
 		let usdt_initial_local_amount = 42;
 		let (usdt_chain, usdt_chain_sovereign_account, usdt_id_multilocation) =
-			set_up_foreign_asset(USDT_PARA_ID, None, usdt_initial_local_amount, true);
+			set_up_foreign_asset(USDT_PARA_ID, None, ALICE, usdt_initial_local_amount, true);
 
 		// transfer destination is USDT chain (foreign asset needs to go through its reserve chain)
 		let dest = usdt_chain;
@@ -2037,6 +2050,7 @@ fn intermediary_error_reverts_side_effects() {
 		let (_, usdc_chain_sovereign_account, usdc_id_multilocation) = set_up_foreign_asset(
 			USDC_RESERVE_PARA_ID,
 			Some(USDC_INNER_JUNCTION),
+			ALICE,
 			usdc_initial_local_amount,
 			true,
 		);
@@ -2106,7 +2120,7 @@ fn teleport_asset_using_local_fee_reserve_call<Call>(
 		// create non-sufficient foreign asset USDT
 		let usdt_initial_local_amount = 42;
 		let (usdt_chain, usdt_chain_sovereign_account, usdt_id_multilocation) =
-			set_up_foreign_asset(USDT_PARA_ID, None, usdt_initial_local_amount, false);
+			set_up_foreign_asset(USDT_PARA_ID, None, ALICE, usdt_initial_local_amount, false);
 
 		// transfer destination is reserve location (no teleport trust)
 		let dest = usdt_chain;
@@ -2258,14 +2272,15 @@ fn teleported_asset_using_destination_reserve_fee_call<Call>(
 			set_up_foreign_asset(
 				FOREIGN_ASSET_RESERVE_PARA_ID,
 				Some(FOREIGN_ASSET_INNER_JUNCTION),
+				ALICE,
 				foreign_initial_amount,
 				true,
 			);
 
 		// create non-sufficient foreign asset USDT
 		let usdt_initial_local_amount = 42;
 		let (_, usdt_chain_sovereign_account, usdt_id_multilocation) =
-			set_up_foreign_asset(USDT_PARA_ID, None, usdt_initial_local_amount, false);
+			set_up_foreign_asset(USDT_PARA_ID, None, ALICE, usdt_initial_local_amount, false);
 
 		// transfer destination is BLA reserve location
 		let dest = reserve_location;

prdoc/pr_2942.prdoc

@@ -0,0 +1,13 @@
+# 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: Fix pallet-nomination-pools v6 to v7 migration
+
+doc:
+  - audience: Node Dev
+    description: |
+      Restores the behaviour of the nomination pools `V6ToV7` migration so that it still works when
+      the pallet will be upgraded to V8 afterwards.
+
+crates:
+  - name: "pallet-nomination-pools"

substrate/bin/node/cli/tests/websocket_server.rs

@@ -205,8 +205,7 @@ impl WsServer {
 								Ok(soketto::Data::Text(len)) => String::from_utf8(buf[..len].to_vec())
 									.map(Message::Text)
 									.map_err(|err| Box::new(err) as Box<_>),
-								Ok(soketto::Data::Binary(len)) => Ok(buf[..len].to_vec())
-									.map(Message::Binary),
+								Ok(soketto::Data::Binary(len)) => Ok(Message::Binary(buf[..len].to_vec())),
 								Err(err) => Err(Box::new(err) as Box<_>),
 							};
 							Some((ret, (receiver, buf)))

substrate/client/consensus/babe/src/tests.rs

@@ -411,7 +411,7 @@ async fn run_one_test(mutator: impl Fn(&mut TestHeader, Stage) + Send + Sync + '
 			let mut net = net.lock();
 			net.poll(cx);
 			for p in net.peers() {
-				for (h, e) in p.failed_verifications() {
+				if let Some((h, e)) = p.failed_verifications().into_iter().next() {
 					panic!("Verification failed for {:?}: {}", h, e);
 				}
 			}

substrate/frame/nomination-pools/src/migration.rs

@@ -57,26 +57,9 @@ pub mod versioned {
 }
 
 pub mod v8 {
-	use super::*;
-
-	#[derive(Decode)]
-	pub struct OldCommission<T: Config> {
-		pub current: Option<(Perbill, T::AccountId)>,
-		pub max: Option<Perbill>,
-		pub change_rate: Option<CommissionChangeRate<BlockNumberFor<T>>>,
-		pub throttle_from: Option<BlockNumberFor<T>>,
-	}
-
-	#[derive(Decode)]
-	pub struct OldBondedPoolInner<T: Config> {
-		pub commission: OldCommission<T>,
-		pub member_counter: u32,
-		pub points: BalanceOf<T>,
-		pub roles: PoolRoles<T::AccountId>,
-		pub state: PoolState,
-	}
+	use super::{v7::V7BondedPoolInner, *};
 
-	impl<T: Config> OldBondedPoolInner<T> {
+	impl<T: Config> V7BondedPoolInner<T> {
 		fn migrate_to_v8(self) -> BondedPoolInner<T> {
 			BondedPoolInner {
 				commission: Commission {
@@ -104,7 +87,7 @@ pub mod v8 {
 
 		fn on_runtime_upgrade() -> Weight {
 			let mut translated = 0u64;
-			BondedPools::<T>::translate::<OldBondedPoolInner<T>, _>(|_key, old_value| {
+			BondedPools::<T>::translate::<V7BondedPoolInner<T>, _>(|_key, old_value| {
 				translated.saturating_inc();
 				Some(old_value.migrate_to_v8())
 			});
@@ -128,16 +111,58 @@ pub mod v8 {
 ///
 /// WARNING: This migration works under the assumption that the [`BondedPools`] cannot be inflated
 /// arbitrarily. Otherwise this migration could fail due to too high weight.
-mod v7 {
+pub(crate) mod v7 {
 	use super::*;
 
+	#[derive(Encode, Decode, MaxEncodedLen, TypeInfo, DebugNoBound, PartialEq, Clone)]
+	#[codec(mel_bound(T: Config))]
+	#[scale_info(skip_type_params(T))]
+	pub struct V7Commission<T: Config> {
+		pub current: Option<(Perbill, T::AccountId)>,
+		pub max: Option<Perbill>,
+		pub change_rate: Option<CommissionChangeRate<BlockNumberFor<T>>>,
+		pub throttle_from: Option<BlockNumberFor<T>>,
+	}
+
+	#[derive(Encode, Decode, MaxEncodedLen, TypeInfo, DebugNoBound, PartialEq, Clone)]
+	#[codec(mel_bound(T: Config))]
+	#[scale_info(skip_type_params(T))]
+	pub struct V7BondedPoolInner<T: Config> {
+		pub commission: V7Commission<T>,
+		pub member_counter: u32,
+		pub points: BalanceOf<T>,
+		pub roles: PoolRoles<T::AccountId>,
+		pub state: PoolState,
+	}
+
+	#[allow(dead_code)]
+	#[derive(RuntimeDebugNoBound)]
+	#[cfg_attr(feature = "std", derive(Clone, PartialEq))]
+	pub struct V7BondedPool<T: Config> {
+		/// The identifier of the pool.
+		id: PoolId,
+		/// The inner fields.
+		inner: V7BondedPoolInner<T>,
+	}
+
+	impl<T: Config> V7BondedPool<T> {
+		fn bonded_account(&self) -> T::AccountId {
+			Pallet::<T>::create_bonded_account(self.id)
+		}
+	}
+
+	// NOTE: We cannot put a V7 prefix here since that would change the storage key.
+	#[frame_support::storage_alias]
+	pub type BondedPools<T: Config> =
+		CountedStorageMap<Pallet<T>, Twox64Concat, PoolId, V7BondedPoolInner<T>>;
+
 	pub struct VersionUncheckedMigrateV6ToV7<T>(sp_std::marker::PhantomData<T>);
 	impl<T: Config> VersionUncheckedMigrateV6ToV7<T> {
 		fn calculate_tvl_by_total_stake() -> BalanceOf<T> {
 			BondedPools::<T>::iter()
 				.map(|(id, inner)| {
 					T::Staking::total_stake(
-						&BondedPool { id, inner: inner.clone() }.bonded_account(),
+						&V7BondedPool { id, inner: inner.clone() }.bonded_account(),
 					)
 					.unwrap_or_default()
 				})

substrate/frame/sassafras/src/data/tickets-sort.md

@@ -267,7 +267,7 @@ buffer (i.e. how many tickets we retain from the last processed segment)
 | 3 | 14 |
 | 2 | 17 |
 | 1 | 9 |
-| 0 | 13
+| 0 | 13 |
 
 # Graph of the same data