[AHM] Staking async fixes for XCM and election planning

cumulus/pallets/parachain-system/src/lib.rs

@@ -1630,6 +1630,15 @@ impl<T: Config> UpwardMessageSender for Pallet<T> {
 	fn send_upward_message(message: UpwardMessage) -> Result<(u32, XcmHash), MessageSendError> {
 		Self::send_upward_message(message)
 	}
+
+	fn check_size(size: usize) -> Result<(), ()> {
+		let cfg = HostConfiguration::<T>::get().ok_or(())?;
+		if size > cfg.max_upward_message_size as usize {
+			Err(())
+		} else {
+			Ok(())
+		}
+	}
 }
 
 impl<T: Config> InspectMessageQueues for Pallet<T> {

cumulus/primitives/core/src/lib.rs

@@ -155,11 +155,19 @@ pub trait UpwardMessageSender {
 	/// be dispatched or an error if the message cannot be sent.
 	/// return the hash of the message sent
 	fn send_upward_message(msg: UpwardMessage) -> Result<(u32, XcmHash), MessageSendError>;
+
+	/// Check whether the message size is acceptable for the channel.
+	fn check_size(size: usize) -> Result<(), ()>;
 }
+
 impl UpwardMessageSender for () {
 	fn send_upward_message(_msg: UpwardMessage) -> Result<(u32, XcmHash), MessageSendError> {
 		Err(MessageSendError::NoChannel)
 	}
+
+	fn check_size(_size: usize) -> Result<(), ()> {
+		Err(())
+	}
 }
 
 /// The status of a channel.

cumulus/primitives/utility/src/lib.rs

@@ -77,6 +77,9 @@ where
 				.map_err(|()| SendError::ExceedsMaxMessageSize)?;
 			let data = versioned_xcm.encode();
 
+			// check if the `UpwardsMessageSender` may also complain about the size
+			T::check_size(data.len()).map_err(|_| SendError::ExceedsMaxMessageSize)?;
+
 			Ok((data, price))
 		} else {
 			// Anything else is unhandled. This includes a message that is not meant for us.
@@ -602,6 +605,9 @@ mod test_xcm_router {
 		fn send_upward_message(_: UpwardMessage) -> Result<(u32, XcmHash), MessageSendError> {
 			Err(MessageSendError::Other)
 		}
+		fn check_size(size: usize) -> Result<(), ()> {
+			todo!("https://github.com/paritytech/polkadot-sdk/pull/8409")
+		}
 	}
 
 	#[test]

prdoc/pr_8422.prdoc

@@ -0,0 +1,30 @@
+title: '[AHM] Staking async fixes for XCM and election planning'
+doc:
+- audience: Runtime Dev
+  description: |-
+    This PR brings a few small fixes related to the XCM messages of stkaing-async, among other small fixes:
+
+
+    * [x] Allows `xcm::validate` to check the message size, and we actually now act upon it in the `staking-async-rc/parachain-runtime`s. The code is a bit duplicate now, and there is a TOOD about how to better refactor it later.
+    * [x] Part of this work is backported separately as https://github.com/paritytech/polkadot-sdk/pull/8409
+    * [x] It brings a default `EraElectionPlannerOf` which should be the right tool to use to ensure elections always happen in time, with an educated guess based on `ElectionProvider::duration` rather than a random number.
+    * [x] It adds a few unit tests about the above
+    * [x] It silences some logs that were needlessly `INFO`, and makes the printing of some types a bit more CLI friendly.
+    * [x] Renames `type SessionDuration` in `staking-async` to `type RelaySessionDuration` for better clarity.
+crates:
+- name: cumulus-pallet-parachain-system
+  bump: minor
+- name: cumulus-primitives-core
+  bump: minor
+- name: cumulus-primitives-utility
+  bump: minor
+- name: pallet-staking-async-ah-client
+  bump: minor
+- name: pallet-staking-async-rc-client
+  bump: minor
+- name: pallet-staking-async-parachain-runtime
+  bump: minor
+- name: pallet-staking-async-rc-runtime
+  bump: minor
+- name: pallet-staking-async
+  bump: minor

substrate/frame/election-provider-multi-block/src/mock/signed.rs

@@ -25,13 +25,13 @@ use crate::{
 };
 use frame_election_provider_support::PageIndex;
 use frame_support::{
-	assert_ok, dispatch::PostDispatchInfo, parameter_types, traits::EstimateCallFee, BoundedVec,
+	assert_ok, dispatch::PostDispatchInfo, parameter_types, traits::EstimateCallFee,
 };
 use sp_npos_elections::ElectionScore;
 use sp_runtime::{traits::Zero, Perbill};
 
 parameter_types! {
-	pub static MockSignedNextSolution: Option<BoundedVec<SolutionOf<Runtime>, Pages>> = None;
+	pub static MockSignedNextSolution: Option<Vec<SolutionOf<Runtime>>> = None;
 	pub static MockSignedNextScore: Option<ElectionScore> = Default::default();
 	pub static MockSignedResults: Vec<VerificationResult> = Default::default();
 }

substrate/frame/election-provider-multi-block/src/types.rs

@@ -82,14 +82,13 @@ pub type AssignmentOf<T> =
 	CloneNoBound,
 	EqNoBound,
 	PartialEqNoBound,
-	MaxEncodedLen,
 	DefaultNoBound,
 )]
 #[codec(mel_bound(T: crate::Config))]
 #[scale_info(skip_type_params(T))]
 pub struct PagedRawSolution<T: MinerConfig> {
 	/// The individual pages.
-	pub solution_pages: BoundedVec<SolutionOf<T>, <T as MinerConfig>::Pages>,
+	pub solution_pages: Vec<SolutionOf<T>>,
 	/// The final claimed score post feasibility and concatenation of all pages.
 	pub score: ElectionScore,
 	/// The designated round.
@@ -165,6 +164,23 @@ pub trait PadSolutionPages: Sized {
 	fn pad_solution_pages(self, desired_pages: PageIndex) -> Self;
 }
 
+impl<T: Default + Clone + Debug> PadSolutionPages for Vec<T> {
+	fn pad_solution_pages(self, desired_pages: PageIndex) -> Self {
+		let desired_pages_usize = desired_pages as usize;
+		debug_assert!(self.len() <= desired_pages_usize);
+		if self.len() == desired_pages_usize {
+			return self
+		}
+
+		// we basically need to prepend the list with this many items.
+		let empty_slots = desired_pages_usize.saturating_sub(self.len());
+		sp_std::iter::repeat(Default::default())
+			.take(empty_slots)
+			.chain(self.into_iter())
+			.collect::<Vec<_>>()
+	}
+}
+
 impl<T: Default + Clone + Debug, Bound: frame_support::traits::Get<u32>> PadSolutionPages
 	for BoundedVec<T, Bound>
 {
@@ -391,8 +407,6 @@ impl<T: crate::Config> Phase<T> {
 #[cfg(test)]
 mod pagify {
 	use super::{PadSolutionPages, Pagify};
-	use frame_support::{traits::ConstU32, BoundedVec};
-	use sp_core::bounded_vec;
 
 	#[test]
 	fn pagify_works() {
@@ -410,15 +424,11 @@ mod pagify {
 	#[test]
 	fn pad_solution_pages_works() {
 		// noop if the solution is complete, as with pagify.
-		let solution: BoundedVec<_, ConstU32<3>> = bounded_vec![1u32, 2, 3];
-		assert_eq!(solution.pad_solution_pages(3).into_inner(), vec![1, 2, 3]);
+		let solution = vec![1u32, 2, 3];
+		assert_eq!(solution.pad_solution_pages(3), vec![1, 2, 3]);
 
 		// pads the solution with default if partial..
-		let solution: BoundedVec<_, ConstU32<3>> = bounded_vec![2, 3];
-		assert_eq!(solution.pad_solution_pages(3).into_inner(), vec![0, 2, 3]);
-
-		// behaves the same as `pad_solution_pages(3)`.
-		let solution: BoundedVec<_, ConstU32<3>> = bounded_vec![2, 3];
-		assert_eq!(solution.pad_solution_pages(4).into_inner(), vec![0, 2, 3]);
+		let solution = vec![2, 3];
+		assert_eq!(solution.pad_solution_pages(3), vec![0, 2, 3]);
 	}
 }

substrate/frame/election-provider-multi-block/src/unsigned/miner.rs

@@ -371,7 +371,7 @@ impl<T: MinerConfig> BaseMiner<T> {
 		}
 
 		// convert each page to a compact struct -- no more change allowed.
-		let solution_pages: BoundedVec<SolutionOf<T>, T::Pages> = paged_assignments
+		let mut solution_pages: Vec<SolutionOf<T>> = paged_assignments
 			.into_iter()
 			.enumerate()
 			.map(|(page_index, assignment_page)| {
@@ -382,12 +382,11 @@ impl<T: MinerConfig> BaseMiner<T> {
 					.ok_or(MinerError::SnapshotUnAvailable(SnapshotType::Voters(page)))?;
 
 				// one last trimming -- `MaxBackersPerWinner`, the per-page variant.
-				let trimmed_assignment_page =
-					Self::trim_supports_max_backers_per_winner_per_page(
-						assignment_page,
-						voter_snapshot_page,
-						page_index as u32,
-					)?;
+				let trimmed_assignment_page = Self::trim_supports_max_backers_per_winner_per_page(
+					assignment_page,
+					voter_snapshot_page,
+					page_index as u32,
+				)?;
 
 				let voter_index_fn = {
 					let cache = helpers::generate_voter_cache::<T, _>(&voter_snapshot_page);
@@ -401,17 +400,11 @@ impl<T: MinerConfig> BaseMiner<T> {
 				)
 				.map_err::<MinerError<T>, _>(Into::into)
 			})
-			.collect::<Result<Vec<_>, _>>()?
-			.try_into()
-			.expect("`paged_assignments` is bound by `T::Pages`; length cannot change in iter chain; qed");
+			.collect::<Result<Vec<_>, _>>()?;
 
 		// now do the length trim.
-		let mut solution_pages_unbounded = solution_pages.into_inner();
 		let _trim_length_weight =
-			Self::maybe_trim_weight_and_len(&mut solution_pages_unbounded, &voter_pages)?;
-		let solution_pages = solution_pages_unbounded
-			.try_into()
-			.expect("maybe_trim_weight_and_len cannot increase the length of its input; qed.");
+			Self::maybe_trim_weight_and_len(&mut solution_pages, &voter_pages)?;
 		miner_log!(debug, "trimmed {} voters due to length restriction.", _trim_length_weight);
 
 		// finally, wrap everything up. Assign a fake score here, since we might need to re-compute

substrate/frame/election-provider-multi-block/src/unsigned/mod.rs

@@ -165,7 +165,7 @@ mod pallet {
 			// we select the most significant pages, based on `T::MinerPages`.
 			let page_indices = crate::Pallet::<T>::msp_range_for(T::MinerPages::get() as usize);
 			<T::Verifier as Verifier>::verify_synchronous_multi(
-				paged_solution.solution_pages.into_inner(),
+				paged_solution.solution_pages,
 				page_indices,
 				claimed_score,
 			)
@@ -235,7 +235,12 @@ mod pallet {
 			assert!(
 				UnsignedWeightsOf::<T>::submit_unsigned().all_lte(T::BlockWeights::get().max_block),
 				"weight of `submit_unsigned` is too high"
-			)
+			);
+			assert!(
+				<T as Config>::MinerPages::get() as usize
+					<= <T as crate::Config>::Pages::get() as usize,
+				"number of pages in the unsigned phase is too high"
+			);
 		}
 
 		#[cfg(feature = "try-runtime")]
@@ -333,6 +338,10 @@ mod pallet {
 				paged_solution.solution_pages.len() == T::MinerPages::get() as usize,
 				CommonError::WrongPageCount
 			);
+			ensure!(
+				paged_solution.solution_pages.len() <= <T as crate::Config>::Pages::get() as usize,
+				CommonError::WrongPageCount
+			);
 
 			Ok(())
 		}

substrate/frame/election-provider-multi-block/src/verifier/tests.rs

@@ -881,7 +881,7 @@ mod multi_page_sync_verification {
 			assert_eq!(<VerifierPallet as Verifier>::queued_score(), None);
 
 			let _ = <VerifierPallet as Verifier>::verify_synchronous_multi(
-				paged.solution_pages.clone().into_inner(),
+				paged.solution_pages.clone(),
 				MultiBlock::msp_range_for(2),
 				paged.score,
 			)
@@ -909,7 +909,7 @@ mod multi_page_sync_verification {
 			assert_eq!(<VerifierPallet as Verifier>::queued_score(), None);
 
 			let _ = <VerifierPallet as Verifier>::verify_synchronous_multi(
-				paged.solution_pages.clone().into_inner(),
+				paged.solution_pages.clone(),
 				MultiBlock::msp_range_for(3),
 				paged.score,
 			)
@@ -941,7 +941,7 @@ mod multi_page_sync_verification {
 
 			assert_eq!(
 				<VerifierPallet as Verifier>::verify_synchronous_multi(
-					paged.solution_pages.clone().into_inner(),
+					paged.solution_pages.clone(),
 					MultiBlock::msp_range_for(2),
 					paged.score,
 				)
@@ -975,7 +975,7 @@ mod multi_page_sync_verification {
 
 			assert_eq!(
 				<VerifierPallet as Verifier>::verify_synchronous_multi(
-					paged.solution_pages.clone().into_inner(),
+					paged.solution_pages.clone(),
 					MultiBlock::msp_range_for(2),
 					paged.score,
 				)
@@ -1009,7 +1009,7 @@ mod multi_page_sync_verification {
 
 			hypothetically!({
 				assert_ok!(<VerifierPallet as Verifier>::verify_synchronous_multi(
-					paged.solution_pages.clone().into_inner(),
+					paged.solution_pages.clone(),
 					MultiBlock::msp_range_for(2),
 					paged.score,
 				));
@@ -1052,7 +1052,7 @@ mod multi_page_sync_verification {
 
 			assert_eq!(
 				<VerifierPallet as Verifier>::verify_synchronous_multi(
-					paged.solution_pages.clone().into_inner(),
+					paged.solution_pages.clone(),
 					MultiBlock::msp_range_for(2),
 					paged.score,
 				)

substrate/frame/staking-async/ah-client/src/lib.rs

@@ -430,7 +430,7 @@ pub mod pallet {
 			report: rc_client::ValidatorSetReport<T::AccountId>,
 		) -> DispatchResult {
 			// Ensure the origin is one of Root or whatever is representing AssetHub.
-			log!(info, "Received new validator set report {:?}", report);
+			log!(debug, "Received new validator set report {}", report);
 			T::AssetHubOrigin::ensure_origin_or_root(origin)?;
 
 			// Check the operating mode.

substrate/frame/staking-async/ahm-test/src/ah/mock.rs

@@ -316,8 +316,8 @@ impl multi_block::signed::Config for Runtime {
 parameter_types! {
 	pub static BondingDuration: u32 = 3;
 	pub static SlashDeferredDuration: u32 = 2;
-	pub static SessionsPerEra: u32 = 6;
-	pub static PlanningEraOffset: u32 = 1;
+	pub static RelaySessionsPerEra: u32 = 6;
+	pub static PlanningEraOffset: u32 = 2;
 }
 
 impl pallet_staking_async::Config for Runtime {
@@ -326,7 +326,7 @@ impl pallet_staking_async::Config for Runtime {
 
 	type AdminOrigin = EnsureRoot<AccountId>;
 	type BondingDuration = BondingDuration;
-	type SessionsPerEra = SessionsPerEra;
+	type RelaySessionsPerEra = RelaySessionsPerEra;
 	type PlanningEraOffset = PlanningEraOffset;
 
 	type Currency = Balances;

substrate/frame/staking-async/ahm-test/src/lib.rs

@@ -185,7 +185,7 @@ mod tests {
 			rc::roll_until_matches(
 				|| {
 					pallet_session::CurrentIndex::<rc::Runtime>::get() ==
-						current_session + ah::SessionsPerEra::get() + 1
+						current_session + ah::RelaySessionsPerEra::get() + 1
 				},
 				true,
 			);