Review fixes

pallets/ah-migrator/src/account_translation.rs

@@ -40,7 +40,6 @@ impl<T: Config> Pallet<T> {
 	}
 
 	/// Translate the account if its a parachain sovereign account.
-	#[cfg(any(feature = "polkadot-ahm", feature = "kusama-ahm"))]
 	pub fn maybe_sovereign_translate(account: &T::AccountId) -> Option<T::AccountId> {
 		let Some(new) = crate::sovereign_account_translation::SOV_TRANSLATIONS
 			.binary_search_by_key(account, |((rc_acc, _), _)| rc_acc.clone())
@@ -65,13 +64,7 @@ impl<T: Config> Pallet<T> {
 		Some(new)
 	}
 
-	#[cfg(not(any(feature = "polkadot-ahm", feature = "kusama-ahm")))]
-	fn maybe_sovereign_translate(account: &T::AccountId) -> Option<T::AccountId> {
-		None
-	}
-
 	/// Translate the account if its derived from a parachain sovereign account.
-	#[cfg(any(feature = "polkadot-ahm", feature = "kusama-ahm"))]
 	pub fn maybe_derived_translate(account: &T::AccountId) -> Option<T::AccountId> {
 		let Some((new, idx)) = crate::sovereign_account_translation::DERIVED_TRANSLATIONS
 			.binary_search_by_key(account, |((rc_acc, _), _, _)| rc_acc.clone())
@@ -95,9 +88,4 @@ impl<T: Config> Pallet<T> {
 
 		Some(new.clone())
 	}
-
-	#[cfg(not(any(feature = "polkadot-ahm", feature = "kusama-ahm")))]
-	fn maybe_derived_translate(account: &T::AccountId) -> Option<T::AccountId> {
-		None
-	}
 }

pallets/ah-migrator/src/lib.rs

@@ -1084,7 +1084,7 @@ pub mod pallet {
 		}
 
 		/// XCM send call identical to the [`pallet_xcm::Pallet::send`] call but with the
-		/// [Config::SendXcm] router which will be able to send messages to the Asset Hub during
+		/// [Config::SendXcm] router which will be able to send messages to the Relay Chain during
 		/// the migration.
 		#[pallet::call_index(111)]
 		#[pallet::weight({ Weight::from_parts(10_000_000, 1000) })]
@@ -1261,8 +1261,6 @@ pub mod pallet {
 
 		/// Send a single XCM message.
 		pub fn send_xcm(call: types::RcMigratorCall) -> Result<(), Error<T>> {
-			log::debug!(target: LOG_TARGET, "Sending XCM message");
-
 			let call = types::RcPalletConfig::RcmController(call);
 
 			let message = Xcm(vec![

pallets/ah-migrator/src/multisig.rs

@@ -21,18 +21,26 @@ use hex_literal::hex;
 #[cfg(feature = "std")]
 use pallet_rc_migrator::types::AccountIdOf;
 
-/// These multisigs have historical issues where the deposit is missing for the creator.
+/// These multisigs have historical issues where the deposit is missing on the RC side.
+///
+/// We just ignore those.
+#[cfg(feature = "polkadot-ahm")]
 const KNOWN_BAD_MULTISIGS: &[AccountId32] = &[
 	AccountId32::new(hex!("e64d5c0de81b9c960c1dd900ad2a5d9d91c8a683e60dd1308e6bc7f80ea3b25f")),
 	AccountId32::new(hex!("d55ec415b6703ddf7bec9d5c02a0b642f1f5bd068c6b3c50c2145544046f1491")),
 	AccountId32::new(hex!("c2ff4f84b7fcee1fb04b4a97800e72321a4bc9939d456ad48d971127fc661c48")),
 	AccountId32::new(hex!("0a8933d3f2164648399cc48cb8bb8c915abb94a2164c40ad6b48cee005f1cb6e")),
 	AccountId32::new(hex!("ebe3cd53e580c4cd88acec1c952585b50a44a9b697d375ff648fee582ae39d38")),
-	AccountId32::new(hex!("e64d5c0de81b9c960c1dd900ad2a5d9d91c8a683e60dd1308e6bc7f80ea3b25f")),
 	AccountId32::new(hex!("caafae0aaa6333fcf4dc193146945fe8e4da74aa6c16d481eef0ca35b8279d73")),
 	AccountId32::new(hex!("d429458e57ba6e9b21688441ff292c7cf82700550446b061a6c5dec306e1ef05")),
 ];
 
+#[cfg(feature = "kusama-ahm")]
+const KNOWN_BAD_MULTISIGS: &[AccountId32] = &[
+	AccountId32::new(hex!("48df9c1a60044840351ef0fbe6b9713ee070578b26a74eb5637b06ac05505f66")),
+	AccountId32::new(hex!("e64d5c0de81b9c960c1dd900ad2a5d9d91c8a683e60dd1308e6bc7f80ea3b25f")),
+];
+
 impl<T: Config> Pallet<T> {
 	pub fn do_receive_multisigs(multisigs: Vec<RcMultisigOf<T>>) -> Result<(), Error<T>> {
 		Self::deposit_event(Event::BatchReceived {
@@ -64,43 +72,19 @@ impl<T: Config> Pallet<T> {
 		// Translate the creator account from RC to AH format
 		let translated_creator = Self::translate_account_rc_to_ah(multisig.creator.clone());
 
-		// Translate the details account (derived multisig account) if present.
-		// NOTE: There are instances where we expect the translation to be a no-op. It's acceptable
-		// to retain it for now and remove it later if we determine it is consistently a no-op.
-		let translated_details = multisig
-			.details
-			.as_ref()
-			.map(|details_account| Self::translate_account_rc_to_ah(details_account.clone()));
-
-		log::trace!(target: LOG_TARGET, "Integrating multisig {}, deposit: {:?}, details: {:?} -> {:?}",
-			translated_creator.to_ss58check(),
-			multisig.deposit,
-			multisig.details.as_ref().map(|d| d.to_ss58check()),
-			translated_details.as_ref().map(|d| d.to_ss58check())
-		);
-
 		let missing = <T as pallet_multisig::Config>::Currency::unreserve(
 			&translated_creator,
 			multisig.deposit,
 		);
 
 		if missing != Default::default() {
 			if KNOWN_BAD_MULTISIGS.contains(&multisig.creator) {
-				log::warn!(
-					target: LOG_TARGET,
-					"Failed to unreserve deposit for known bad multisig {}, missing: {:?}, account: {:?}",
-					translated_creator.to_ss58check(),
-					missing,
-					frame_system::Account::<T>::get(&translated_creator)
-				);
+				// This is "fine"
 			} else {
-				log::error!(
-					target: LOG_TARGET,
-					"Failed to unreserve deposit for multisig {}, missing: {:?}, details: {:?} -> {:?}",
+				debug_assert!(
+					false,
+					"Failed to unreserve deposit for multisig {}",
 					translated_creator.to_ss58check(),
-					missing,
-					multisig.details.as_ref().map(|d| d.to_ss58check()),
-					translated_details.as_ref().map(|d| d.to_ss58check())
 				);
 			}
 

pallets/rc-migrator/src/accounts.rs

@@ -732,7 +732,7 @@ impl<T: Config> AccountsMigrator<T> {
 	/// weight.
 	pub fn obtain_free_proxy_candidates() -> (Option<u32>, Weight) {
 		if PureProxyCandidatesMigrated::<T>::iter_keys().next().is_some() {
-			log::info!(target: LOG_TARGET, "Init pure proxy candidates already ran, skipping");
+			defensive!("Init pure proxy candidates already ran, skipping");
 			return (None, T::DbWeight::get().reads(1));
 		}
 
@@ -758,7 +758,7 @@ impl<T: Config> AccountsMigrator<T> {
 	/// Should be executed once before the migration starts.
 	pub fn obtain_rc_accounts() -> Weight {
 		if RcAccounts::<T>::iter_keys().next().is_some() {
-			log::info!(target: LOG_TARGET, "Init accounts migration already ran, skipping");
+			defensive!("Init accounts migration already ran, skipping");
 			return T::DbWeight::get().reads(1);
 		}
 

pallets/rc-migrator/src/lib.rs

@@ -814,7 +814,7 @@ pub mod pallet {
 	pub type PendingXcmQueries<T: Config> =
 		StorageMap<_, Twox64Concat, QueryId, T::Hash, OptionQuery>;
 
-	/// The DMP queue priority.
+	/// Manual override for `type UnprocessedMsgBuffer: Get<u32>`. Look there for docs.
 	#[pallet::storage]
 	pub type UnprocessedMsgBuffer<T: Config> = StorageValue<_, u32, OptionQuery>;
 
@@ -2446,6 +2446,7 @@ pub mod pallet {
 			}
 
 			if batch_count > MAX_XCM_MSG_PER_BLOCK {
+				debug_assert!(false, "Unreachable: we always remaining len before pushing");
 				log::warn!(
 					target: LOG_TARGET,
 					"Maximum number of XCM messages ({}) to migrate per block exceeded, current msg count: {}",
@@ -2463,8 +2464,6 @@ pub mod pallet {
 		/// ### Parameters:
 		/// - call - the call to send
 		pub fn send_xcm(call: types::AhMigratorCall<T>) -> Result<(), Error<T>> {
-			log::info!(target: LOG_TARGET, "Sending XCM message");
-
 			let call = types::AssetHubPalletConfig::<T>::AhmController(call);
 
 			let message = Xcm(vec![

pallets/rc-migrator/src/multisig.rs

@@ -79,8 +79,6 @@ pub struct RcMultisig<AccountId, Balance> {
 	pub creator: AccountId,
 	/// Amount of the deposit.
 	pub deposit: Balance,
-	/// Optional details field to debug. Can be `None` in prod. Contains the derived account.
-	pub details: Option<AccountId>,
 }
 
 pub type RcMultisigOf<T> = RcMultisig<AccountIdOf<T>, BalanceOf<T>>;
@@ -172,11 +170,7 @@ impl<T: Config> PalletMigration for MultisigMigrator<T> {
 
 			log::debug!(target: LOG_TARGET, "Migrating multisigs of acc {k1:?}");
 
-			batch.push(RcMultisig {
-				creator: multisig.depositor,
-				deposit: multisig.deposit,
-				details: Some(k1.clone()),
-			});
+			batch.push(RcMultisig { creator: multisig.depositor, deposit: multisig.deposit });
 
 			aliases::Multisigs::<T>::remove(&k1, &k2);
 			last_key = Some((k1, k2));

pallets/rc-migrator/src/types.rs

@@ -448,7 +448,7 @@ impl<T: Encode> XcmBatch<T> {
 	pub fn push(&mut self, message: T) {
 		let message_size = message.encoded_size() as u32;
 		if message_size > MAX_XCM_SIZE {
-			defensive_assert!(true, "Message is too large to be added to the batch");
+			defensive_assert!(false, "Message is too large to be added to the batch");
 		}
 
 		match self.sized_batches.back_mut() {

relay/kusama/src/ah_migration/phase1.rs

@@ -73,7 +73,8 @@ impl Get<RuntimeHoldReason> for StakingDelegationReason {
 ///       Start        End
 /// ```
 ///
-/// This call returns a 2-tuple to indicate whether a call is enabled during these periods.
+/// This call returns a 2-tuple to indicate whether a call is enabled during these periods. The
+/// Start period contains our Warmup period and the End period contains our Cool-off period.
 pub fn call_allowed_status(call: &<Runtime as frame_system::Config>::RuntimeCall) -> (bool, bool) {
 	use RuntimeCall::*;
 	const ON: bool = true;
@@ -106,11 +107,13 @@ pub fn call_allowed_status(call: &<Runtime as frame_system::Config>::RuntimeCall
 		VoterList(..) => (OFF, OFF),
 		NominationPools(..) => (OFF, OFF),
 		FastUnstake(..) => (OFF, OFF),
-		Configuration(..) => (ON, ON), /* TODO allow this to be called by fellow origin during the migration https://github.com/polkadot-fellows/runtimes/pull/559#discussion_r1928794490 */
+		Configuration(..) => (ON, ON),
 		ParasShared(parachains_shared::Call::__Ignore { .. }) => (ON, ON), // Has no calls
 		ParaInclusion(parachains_inclusion::Call::__Ignore { .. }) => (ON, ON), // Has no calls
-		ParaInherent(..) => (ON, ON),  // only inherents
-		Paras(..) => (ON, ON),         /* Only root and one security relevant call: */
+		ParaInherent(..) => (ON, ON),                                      // only inherents
+		Paras(..) => (ON, ON),                                             /* Only root and one
+		                                                                     * security relevant
+		                                                                     * call: */
 		// `include_pvf_check_statement`
 		Initializer(..) => (ON, ON), // Only root calls. Fine to keep.
 		Hrmp(..) => (ON, ON),        /* open close hrmp channels by parachains or root force. */

relay/polkadot/src/ah_migration/phase1.rs

@@ -73,7 +73,8 @@ impl Get<RuntimeHoldReason> for StakingDelegationReason {
 ///       Start        End
 /// ```
 ///
-/// This call returns a 2-tuple to indicate whether a call is enabled during these periods.
+/// This call returns a 2-tuple to indicate whether a call is enabled during these periods. The
+/// Start period contains our Warmup period and the End period contains our Cool-off period.
 pub fn call_allowed_status(call: &<Runtime as frame_system::Config>::RuntimeCall) -> (bool, bool) {
 	use RuntimeCall::*;
 	const ON: bool = true;
@@ -106,11 +107,13 @@ pub fn call_allowed_status(call: &<Runtime as frame_system::Config>::RuntimeCall
 		VoterList(..) => (OFF, OFF),
 		NominationPools(..) => (OFF, OFF),
 		FastUnstake(..) => (OFF, OFF),
-		Configuration(..) => (ON, ON), /* TODO allow this to be called by fellow origin during the migration https://github.com/polkadot-fellows/runtimes/pull/559#discussion_r1928794490 */
+		Configuration(..) => (ON, ON),
 		ParasShared(parachains_shared::Call::__Ignore { .. }) => (ON, ON), // Has no calls
 		ParaInclusion(parachains_inclusion::Call::__Ignore { .. }) => (ON, ON), // Has no calls
-		ParaInherent(..) => (ON, ON),  // only inherents
-		Paras(..) => (ON, ON),         /* Only root and one security relevant call: */
+		ParaInherent(..) => (ON, ON),                                      // only inherents
+		Paras(..) => (ON, ON),                                             /* Only root and one
+		                                                                     * security relevant
+		                                                                     * call: */
 		// `include_pvf_check_statement`
 		Initializer(..) => (ON, ON), // Only root calls. Fine to keep.
 		Hrmp(..) => (ON, ON),        /* open close hrmp channels by parachains or root force. */

system-parachains/asset-hubs/asset-hub-kusama/src/ah_migration/call_filter.rs

@@ -77,7 +77,8 @@ impl Contains<<Runtime as frame_system::Config>::RuntimeCall> for CallsEnabledAf
 ///       Start        End
 /// ```
 ///
-/// This call returns a 3-tuple to indicate whether a call is enabled during these periods.
+/// This call returns a 3-tuple to indicate whether a call is enabled during these periods. The
+/// Start period contains our Warmup period and the End period contains our Cool-off period.
 pub fn call_allowed_status(
 	call: &<Runtime as frame_system::Config>::RuntimeCall,
 ) -> (bool, bool, bool) {

system-parachains/asset-hubs/asset-hub-kusama/src/xcm_config.rs

@@ -35,8 +35,8 @@ use frame_support::{
 	traits::{
 		fungible::HoldConsideration,
 		tokens::imbalance::{ResolveAssetTo, ResolveTo},
-		ConstU32, Contains, ContainsPair, Equals, Everything, FromContains, LinearStoragePrice,
-		PalletInfoAccess,
+		ConstU32, Contains, ContainsPair, Defensive, Equals, Everything, FromContains,
+		LinearStoragePrice, PalletInfoAccess,
 	},
 };
 use frame_system::EnsureRoot;
@@ -90,7 +90,7 @@ parameter_types! {
 	// Account address: `5Gzx76VEMzLpMp9HBarpkJ62WMSNeRfdD1jLjpvpZtY37Wum`
 	pub PreMigrationRelayTreasuryPalletAccount: AccountId =
 		LocationToAccountId::convert_location(&RelayTreasuryLocation::get())
-			.unwrap_or(crate::treasury::TreasuryAccount::get());
+			.defensive_unwrap_or(crate::treasury::TreasuryAccount::get());
 	pub PostMigrationTreasuryAccount: AccountId = crate::treasury::TreasuryAccount::get();
 	/// The Checking Account along with the indication that the local chain is able to mint tokens.
 	pub TeleportTracking: Option<(AccountId, MintLocation)> = crate::AhMigrator::teleport_tracking();
@@ -330,7 +330,6 @@ pub type Barrier = TrailingSetTopicAsId<
 /// We only waive fees for system functions, which these locations represent.
 pub type WaivedLocations = (
 	Equals<RootLocation>,
-	Equals<ParentLocation>,
 	RelayOrOtherSystemParachains<AllSiblingSystemParachains, Runtime>,
 	Equals<RelayTreasuryLocation>,
 	FellowshipEntities,

system-parachains/asset-hubs/asset-hub-polkadot/src/ah_migration/call_filter.rs

@@ -77,7 +77,8 @@ impl Contains<<Runtime as frame_system::Config>::RuntimeCall> for CallsEnabledAf
 ///       Start        End
 /// ```
 ///
-/// This call returns a 3-tuple to indicate whether a call is enabled during these periods.
+/// This call returns a 3-tuple to indicate whether a call is enabled during these periods. The
+/// Start period contains our Warmup period and the End period contains our Cool-off period.
 pub fn call_allowed_status(
 	call: &<Runtime as frame_system::Config>::RuntimeCall,
 ) -> (bool, bool, bool) {

system-parachains/asset-hubs/asset-hub-polkadot/src/lib.rs

@@ -105,7 +105,7 @@ use xcm::latest::prelude::*;
 use xcm_runtime_apis::{
 	dry_run::{CallDryRunEffects, Error as XcmDryRunApiError, XcmDryRunEffects},
 	fees::Error as XcmPaymentApiError,
-}; // Change for Async Backing https://github.com/polkadot-fellows/runtimes/pull/763
+};
 
 #[cfg(feature = "std")]
 use sp_version::NativeVersion;

system-parachains/asset-hubs/asset-hub-polkadot/src/xcm_config.rs

@@ -402,7 +402,6 @@ pub type Barrier = TrailingSetTopicAsId<
 /// We only waive fees for system functions, which these locations represent.
 pub type WaivedLocations = (
 	Equals<RootLocation>,
-	Equals<ParentLocation>,
 	RelayOrOtherSystemParachains<AllSiblingSystemParachains, Runtime>,
 	Equals<RelayTreasuryLocation>,
 	FellowshipEntities,