[AHM] Cleanup and add Preimage doc (#543)

👉 To be merged into the Asset Hub Migration working branch.

Changes:
- Factor out XCM sending into common function
- Cleanup
- Add docs for the preimage pallet that I started with

- [x] Does not require a CHANGELOG entry

---------

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

.gitignore

@@ -40,3 +40,4 @@ substrate.code-workspace
 target/
 **/__pycache__/
 *.snap
+*.json

integration-tests/ahm/src/tests.rs

@@ -46,7 +46,7 @@ async fn account_migration_works() {
 	let Some((mut rc, mut ah)) = load_externalities().await else { return };
 	let para_id = ParaId::from(1000);
 
-	// Simulate 10 relay blocks and grab the DMP messages
+	// Simulate relay blocks and grab the DMP messages
 	let dmp_messages = rc.execute_with(|| {
 		let mut dmps = Vec::new();
 

pallets/ah-migrator/src/proxy.rs

@@ -51,6 +51,7 @@ impl<T: Config> Pallet<T> {
 			let Ok(proxy_type) = T::RcToProxyType::try_convert(p.proxy_type) else {
 				// This is fine, eg. `Auction` proxy is not supported on AH
 				log::warn!(target: LOG_TARGET, "Dropping unsupported proxy at index {} for {}", i, proxy.delegator.to_ss58check());
+				// TODO unreserve deposit
 				return None;
 			};
 
@@ -89,7 +90,7 @@ impl<T: Config> Pallet<T> {
 		});
 
 		let (mut count_good, mut count_bad) = (0, 0);
-		log::info!(target: LOG_TARGET, "Integrating {} proxy announcements", announcements.len());
+		log::info!(target: LOG_TARGET, "Unreserving deposits for {} proxy announcements", announcements.len());
 
 		for announcement in announcements {
 			match Self::do_receive_proxy_announcement(announcement) {
@@ -109,13 +110,14 @@ impl<T: Config> Pallet<T> {
 	pub fn do_receive_proxy_announcement(
 		announcement: RcProxyAnnouncementOf<T>,
 	) -> Result<(), Error<T>> {
+		let before = frame_system::Account::<T>::get(&announcement.depositor);
 		let missing = <T as pallet_proxy::Config>::Currency::unreserve(
 			&announcement.depositor,
 			announcement.deposit,
 		);
 
 		if !missing.is_zero() {
-			log::warn!(target: LOG_TARGET, "Could not unreserve full proxy announcement deposit for {}, missing {:?}", announcement.depositor.to_ss58check(), missing);
+			log::warn!(target: LOG_TARGET, "Could not unreserve full proxy announcement deposit for {}, missing {:?}, before {:?}", announcement.depositor.to_ss58check(), missing, before);
 		}
 
 		Ok(())

pallets/rc-migrator/Cargo.toml

@@ -26,7 +26,7 @@ pallet-multisig = { workspace = true }
 polkadot-runtime-common = { workspace = true }
 runtime-parachains = { workspace = true }
 polkadot-parachain-primitives = { workspace = true }
-
+hex-literal = { workspace = true }
 xcm = { workspace = true }
 
 [features]

pallets/rc-migrator/src/accounts.rs

@@ -219,27 +219,11 @@ impl<T: Config> Pallet<T> {
 			}
 		};
 
-		let call = types::AssetHubPalletConfig::<T>::AhmController(
-			types::AhMigratorCall::<T>::ReceiveAccounts { accounts: package },
-		);
-
-		let message = Xcm(vec![
-			Instruction::UnpaidExecution {
-				weight_limit: WeightLimit::Unlimited,
-				check_origin: None,
-			},
-			Instruction::Transact {
-				origin_kind: OriginKind::Superuser,
-				require_weight_at_most: Weight::from_all(1), // TODO
-				call: call.encode().into(),
-			},
-		]);
-
-		if let Err(_err) =
-			send_xcm::<T::SendXcm>(Location::new(0, [Junction::Parachain(1000)]), message.clone())
-		{
-			return Err(Error::TODO);
-		};
+		if !package.is_empty() {
+			Pallet::<T>::send_chunked_xcm(package, |package| {
+				types::AhMigratorCall::<T>::ReceiveAccounts { accounts: package }
+			})?;
+		}
 
 		Ok(maybe_last_key)
 	}
@@ -402,7 +386,7 @@ impl<T: Config> Pallet<T> {
 	/// Since the `reserved` and `frozen` balances will be known on a receiving side (AH) they will
 	/// be calculated there.
 	pub fn get_consumer_count(_who: &T::AccountId, _info: &AccountInfoFor<T>) -> u8 {
-		// TODO: check the pallets for provider references on Relay Chain.
+		// TODO: check the pallets for consumer references on Relay Chain.
 
 		// The following pallets increase consumers and are deployed on (Polkadot, Kusama, Westend):
 		// - `balances`: (P/K/W)
@@ -427,7 +411,7 @@ impl<T: Config> Pallet<T> {
 	pub fn get_provider_count(_who: &T::AccountId, _info: &AccountInfoFor<T>) -> u8 {
 		// TODO: check the pallets for provider references on Relay Chain.
 
-		// The following pallets increase consumers and are deployed on (Polkadot, Kusama, Westend):
+		// The following pallets increase provider and are deployed on (Polkadot, Kusama, Westend):
 		// - `crowdloan`: (P/K/W) https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/runtime/common/src/crowdloan/mod.rs#L416
 		// - `parachains_on_demand`: (P/K/W) https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/runtime/parachains/src/on_demand/mod.rs#L407
 		// - `balances`: (P/K/W) https://github.com/paritytech/polkadot-sdk/blob/master/substrate/frame/balances/src/lib.rs#L1026

pallets/rc-migrator/src/lib.rs

@@ -88,7 +88,7 @@ pub enum MigrationStage<AccountId> {
 	/// easier to swap out what stage should run next for testing.
 	AccountsMigrationDone,
 	MultisigMigrationInit,
-	MultiSigMigrationOngoing {
+	MultisigMigrationOngoing {
 		/// Last migrated key of the `Multisigs` double map.
 		last_key: Option<(AccountId, [u8; 32])>,
 	},
@@ -158,10 +158,15 @@ pub mod pallet {
 	}
 
 	#[pallet::event]
-	//#[pallet::generate_deposit(pub(crate) fn deposit_event)]
+	#[pallet::generate_deposit(pub(crate) fn deposit_event)]
 	pub enum Event<T: Config> {
-		/// TODO
-		TODO,
+		/// A stage transition has occurred.
+		StageTransition {
+			/// The old stage before the transition.
+			old: MigrationStage<T::AccountId>,
+			/// The new stage after the transition.
+			new: MigrationStage<T::AccountId>,
+		},
 	}
 
 	/// The Relay Chain migration state.
@@ -267,9 +272,9 @@ pub mod pallet {
 					Self::transition(MigrationStage::MultisigMigrationInit);
 				},
 				MigrationStage::MultisigMigrationInit => {
-					Self::transition(MigrationStage::MultiSigMigrationOngoing { last_key: None });
+					Self::transition(MigrationStage::MultisigMigrationOngoing { last_key: None });
 				},
-				MigrationStage::MultiSigMigrationOngoing { last_key } => {
+				MigrationStage::MultisigMigrationOngoing { last_key } => {
 					let res = with_transaction_opaque_err::<Option<_>, Error<T>, _>(|| {
 						TransactionOutcome::Commit(MultisigMigrator::<T>::migrate_many(
 							last_key,
@@ -287,7 +292,7 @@ pub mod pallet {
 						Ok(Some(last_key)) => {
 							// multisig migration continues with the next block
 							// TODO publish event
-							Self::transition(MigrationStage::MultiSigMigrationOngoing {
+							Self::transition(MigrationStage::MultisigMigrationOngoing {
 								last_key: Some(last_key),
 							});
 						},
@@ -364,10 +369,67 @@ pub mod pallet {
 
 	impl<T: Config> Pallet<T> {
 		/// Execute a stage transition and log it.
-		fn transition(new_stage: MigrationStage<T::AccountId>) {
+		fn transition(new: MigrationStage<T::AccountId>) {
 			let old = RcMigrationStage::<T>::get();
-			RcMigrationStage::<T>::put(&new_stage);
-			log::info!(target: LOG_TARGET, "[Block {:?}] Stage transition: {:?} -> {:?}", frame_system::Pallet::<T>::block_number(), old, new_stage);
+			RcMigrationStage::<T>::put(&new);
+			log::info!(target: LOG_TARGET, "[Block {:?}] Stage transition: {:?} -> {:?}", frame_system::Pallet::<T>::block_number(), &old, &new);
+			Self::deposit_event(Event::StageTransition { old, new });
+		}
+
+		/// Split up the items into chunks of `MAX_MSG_SIZE` and send them as separate XCM
+		/// transacts.
+		///
+		/// Will modify storage in the error path.
+		/// This is done to avoid exceeding the XCM message size limit.
+		pub fn send_chunked_xcm<E: Encode>(
+			mut items: Vec<E>,
+			create_call: impl Fn(Vec<E>) -> types::AhMigratorCall<T>,
+		) -> Result<(), Error<T>> {
+			const MAX_MSG_SIZE: u32 = 50_000; // Soft message size limit. Hard limit is about 64KiB
+			// Reverse in place so that we can use `pop` later on
+			items.reverse();
+
+			while !items.is_empty() {
+				let mut remaining_size: u32 = MAX_MSG_SIZE;
+				let mut batch = Vec::new();
+
+				while !items.is_empty() {
+					// Taking from the back as optimization is fine since we reversed
+					let item = items.last().unwrap(); // FAIL-CI no unwrap
+					let msg_size = item.encoded_size() as u32;
+					if msg_size > remaining_size {
+						break;
+					}
+					remaining_size -= msg_size;
+
+					batch.push(items.pop().unwrap()); // FAIL-CI no unwrap
+				}
+
+				log::info!(target: LOG_TARGET, "Sending batch of {} proxies", batch.len());
+				let call = types::AssetHubPalletConfig::<T>::AhmController(create_call(batch));
+
+				let message = Xcm(vec![
+					Instruction::UnpaidExecution {
+						weight_limit: WeightLimit::Unlimited,
+						check_origin: None,
+					},
+					Instruction::Transact {
+						origin_kind: OriginKind::Superuser,
+						require_weight_at_most: Weight::from_all(1), // TODO
+						call: call.encode().into(),
+					},
+				]);
+
+				if let Err(err) = send_xcm::<T::SendXcm>(
+					Location::new(0, [Junction::Parachain(1000)]),
+					message.clone(),
+				) {
+					log::error!(target: LOG_TARGET, "Error while sending XCM message: {:?}", err);
+					return Err(Error::TODO);
+				};
+			}
+
+			Ok(())
 		}
 	}
 }

pallets/rc-migrator/src/multisig.md

@@ -21,3 +21,7 @@ change the existing one. Both probably not worth it for us now.
 
 The only thing that we should do is to unlock the deposits on the AH since they were migrated to AH
 with the account state.
+
+## TODO
+- Think about what if one of the multisig members' accounts are inaccessible? Like they lost their key or something. Will this impact the possibility of the remaining multisig members to re-create the same multisig on AH?
+	- I think it does not impact it, as the [as_multi call](https://github.com/paritytech/polkadot-sdk/blob/9cdd1178f9e8da9f08a334a8cbbda435d1ffd9dd/substrate/frame/multisig/src/lib.rs#L358-L361) just accepts Account IDs as input that do not need to be accessible.

pallets/rc-migrator/src/multisig.rs

@@ -150,7 +150,9 @@ impl<T: Config> PalletMigration for MultisigMigrator<T> {
 		}
 
 		if !batch.is_empty() {
-			Self::send_batch_xcm(batch)?;
+			Pallet::<T>::send_chunked_xcm(batch, |batch| {
+				types::AhMigratorCall::<T>::ReceiveMultisigs { multisigs: batch }
+			})?;
 		}
 
 		Ok(last_key)
@@ -170,31 +172,4 @@ impl<T: Config> MultisigMigrator<T> {
 
 		Ok(RcMultisig { creator: ms.depositor, deposit: ms.deposit, details: Some(k1) })
 	}
-
-	/// Storage changes must be rolled back on error.
-	fn send_batch_xcm(multisigs: Vec<RcMultisigOf<T>>) -> Result<(), Error<T>> {
-		let call = types::AssetHubPalletConfig::<T>::AhmController(
-			types::AhMigratorCall::<T>::ReceiveMultisigs { multisigs },
-		);
-
-		let message = Xcm(vec![
-			Instruction::UnpaidExecution {
-				weight_limit: WeightLimit::Unlimited,
-				check_origin: None,
-			},
-			Instruction::Transact {
-				origin_kind: OriginKind::Superuser,
-				require_weight_at_most: Weight::from_all(1), // TODO
-				call: call.encode().into(),
-			},
-		]);
-
-		if let Err(_err) =
-			send_xcm::<T::SendXcm>(Location::new(0, [Junction::Parachain(1000)]), message.clone())
-		{
-			return Err(Error::TODO);
-		};
-
-		Ok(())
-	}
 }