Fix offchain election to respect the weight

bin/node/runtime/src/lib.rs

@@ -441,13 +441,13 @@ parameter_types! {
 	pub const MaxIterations: u32 = 10;
 	// 0.05%. The higher the value, the more strict solution acceptance becomes.
 	pub MinSolutionScoreBump: Perbill = Perbill::from_rational_approximation(5u32, 10_000);
-	// The unsigned solution is operational, so as long as the average on_initialize weights are
-	// less than `MaximumBlockWeight * (1 - T::AvailableRatio)`, it can consume at most most this
-	// amount.
+	// The unsigned solution is mandatory, and this is the absolute maximum weight that we allow for
+	// it.
 	pub OffchainSolutionWeightLimit: Weight =
 		MaximumBlockWeight::get()
 			.saturating_sub(BlockExecutionWeight::get())
-			.saturating_sub(ExtrinsicBaseWeight::get());
+			.saturating_sub(ExtrinsicBaseWeight::get())
+			.saturating_sub(AVERAGE_ON_INITIALIZE_WEIGHT * MaximumBlockWeight::get());
 }
 
 impl pallet_staking::Trait for Runtime {

frame/staking/src/lib.rs

@@ -1239,6 +1239,8 @@ decl_error! {
 		OffchainElectionBogusScore,
 		/// The election size is invalid.
 		OffchainElectionBogusElectionSize,
+		/// The election weight is invalid.
+		OffchainElectionBogusElectionWeight,
 		/// The call is not allowed at the given time due to restrictions of election period.
 		CallNotAllowed,
 		/// Incorrect previous history depth input provided.
@@ -1346,12 +1348,12 @@ decl_module! {
 			if Self::era_election_status().is_open_at(now) {
 				let offchain_status = set_check_offchain_execution_status::<T>(now);
 				if let Err(why) = offchain_status {
-					log!(debug, "skipping offchain worker in open election window due to [{}]", why);
+					log!(warn, "💸 skipping offchain worker in open election window due to [{}]", why);
 				} else {
 					if let Err(e) = compute_offchain_election::<T>() {
 						log!(error, "💸 Error in election offchain worker: {:?}", e);
 					} else {
-						log!(debug, "Executed offchain worker thread without errors.");
+						log!(debug, "💸 Executed offchain worker thread without errors.");
 					}
 				}
 			}
@@ -1380,17 +1382,6 @@ decl_module! {
 				)
 			);
 
-			// Offchain solution is operational, so it can consume all the block weight except for
-			// `System::BlockExecutionWeight`. So setting the limit above this is invalid.
-			assert!(
-				T::OffchainSolutionWeightLimit::get() <=
-				(
-					<T as frame_system::Trait>::MaximumBlockWeight::get() -
-					<T as frame_system::Trait>::BlockExecutionWeight::get() -
-					<T as frame_system::Trait>::ExtrinsicBaseWeight::get()
-				)
-			);
-
 			use sp_runtime::UpperOf;
 			// see the documentation of `Assignment::try_normalize`. Now we can ensure that this
 			// will always return `Ok`.
@@ -2159,15 +2150,15 @@ decl_module! {
 		/// transaction in the block.
 		///
 		/// # <weight>
-		/// See `crate::weight` module.
+		/// The weight of this call is not enforced by the system module and is enforced in this
+		/// module directly.
 		/// # </weight>
-		#[weight = (
-			T::WeightInfo::submit_solution_better(
+		#[weight = (T::WeightInfo::submit_solution_better(
 			size.validators.into(),
 			size.nominators.into(),
 			compact.len() as u32,
 			winners.len() as u32,
-		), frame_support::weights::DispatchClass::Operational)]
+		), frame_support::weights::DispatchClass::Mandatory)]
 		pub fn submit_election_solution_unsigned(
 			origin,
 			winners: Vec<ValidatorIndex>,
@@ -2177,6 +2168,13 @@ decl_module! {
 			size: ElectionSize,
 		) -> DispatchResultWithPostInfo {
 			ensure_none(origin)?;
+			// The signed solution is `Normal` and the system module checks the weight. But for this
+			// mandatory one, we need to do it.
+			ensure!(
+				Self::check_unsigned_solution_weight(&winners, &compact, &size),
+				Error::<T>::OffchainElectionBogusElectionWeight.with_weight(0),
+			);
+
 			let adjustments = Self::check_and_replace_solution(
 				winners,
 				compact,
@@ -2190,6 +2188,7 @@ decl_module! {
 				effectively depriving the validators from their authoring reward. Hence, this panic
 				is expected."
 			);
+
 			Ok(adjustments)
 		}
 	}
@@ -2209,6 +2208,21 @@ impl<T: Trait> Module<T> {
 		)
 	}
 
+	/// Check the weight of an unsigned solution. Returns true of the weight is below the designated
+	/// limit .
+	pub fn check_unsigned_solution_weight(
+		winners: &Vec<ValidatorIndex>,
+		compact: &CompactAssignments,
+		size: &ElectionSize,
+	) -> bool {
+		T::WeightInfo::submit_solution_better(
+			size.validators.into(),
+			size.nominators.into(),
+			compact.len() as u32,
+			winners.len() as u32,
+		) <= T::OffchainSolutionWeightLimit::get()
+	}
+
 	/// Dump the list of validators and nominators into vectors and keep them on-chain.
 	///
 	/// This data is used to efficiently evaluate election results. returns `true` if the operation
@@ -3101,7 +3115,6 @@ impl<T: Trait> Module<T> {
 	pub fn set_slash_reward_fraction(fraction: Perbill) {
 		SlashRewardFraction::put(fraction);
 	}
-
 }
 
 /// In this implementation `new_session(session)` must be called before `end_session(session-1)`
@@ -3365,11 +3378,11 @@ impl<T: Trait> frame_support::unsigned::ValidateUnsigned for Module<T> {
 	type Call = Call<T>;
 	fn validate_unsigned(source: TransactionSource, call: &Self::Call) -> TransactionValidity {
 		if let Call::submit_election_solution_unsigned(
-			_,
-			_,
+			winners,
+			compact,
 			score,
 			era,
-			_,
+			size,
 		) = call {
 			use offchain_election::DEFAULT_LONGEVITY;
 
@@ -3382,17 +3395,22 @@ impl<T: Trait> frame_support::unsigned::ValidateUnsigned for Module<T> {
 				}
 			}
 
+			// discard solution if it is too big.
+			if !Self::check_unsigned_solution_weight(winners, compact, size) {
+				return Err(InvalidTransaction::ExhaustsResources.into())
+			}
+
 			if let Err(error_with_post_info) = Self::pre_dispatch_checks(*score, *era) {
 				let invalid = to_invalid(error_with_post_info);
 				log!(
 					debug,
-					"validate unsigned pre dispatch checks failed due to error #{:?}.",
+					"💸 validate unsigned pre dispatch checks failed due to error #{:?}.",
 					invalid,
 				);
-				return invalid .into();
+				return invalid.into();
 			}
 
-			log!(debug, "validateUnsigned succeeded for a solution at era {}.", era);
+			log!(debug, "💸 validateUnsigned succeeded for a solution at era {}.", era);
 
 			ValidTransaction::with_tag_prefix("StakingOffchain")
 				// The higher the score[0], the better a solution is.

frame/staking/src/offchain_election.rs

@@ -127,7 +127,7 @@ pub(crate) fn compute_offchain_election<T: Trait>() -> Result<(), OffchainElecti
 
 	crate::log!(
 		info,
-		"prepared a seq-phragmen solution with {} balancing iterations and score {:?}",
+		"💸 prepared a seq-phragmen solution with {} balancing iterations and score {:?}",
 		iters,
 		score,
 	);
@@ -281,54 +281,57 @@ pub fn trim_to_weight<T: Trait, FN>(
 where
 	for<'r> FN: Fn(&'r T::AccountId) -> Option<NominatorIndex>,
 {
-	if let Some(to_remove) = compact.len().checked_sub(maximum_allowed_voters as usize) {
-		// grab all voters and sort them by least stake.
-		let mut voters_sorted = <Nominators<T>>::iter()
-			.map(|(who, _)| {
-				(
-					who.clone(),
-					<Module<T>>::slashable_balance_of_vote_weight(&who),
-				)
-			})
-			.collect::<Vec<_>>();
-		voters_sorted.sort_by_key(|(_, y)| *y);
-
-		// start removing from the least stake. Iterate until we know enough have been removed.
-		let mut removed = 0;
-		for (maybe_index, _stake) in voters_sorted
-			.iter()
-			.map(|(who, stake)| (nominator_index(&who), stake))
-		{
-			let index = maybe_index.ok_or(OffchainElectionError::NominatorSnapshotCorrupt)?;
-			if compact.remove_voter(index) {
-				crate::log!(
-					debug,
-					"removed a voter at index {} with stake {:?} from compact to reduce the size",
-					index,
-					_stake,
-				);
-				removed += 1
-			}
+	match compact.len().checked_sub(maximum_allowed_voters as usize) {
+		Some(to_remove) if to_remove > 0 => {
+			// grab all voters and sort them by least stake.
+			let mut voters_sorted = <Nominators<T>>::iter()
+				.map(|(who, _)| {
+					(
+						who.clone(),
+						<Module<T>>::slashable_balance_of_vote_weight(&who),
+					)
+				})
+				.collect::<Vec<_>>();
+			voters_sorted.sort_by_key(|(_, y)| *y);
+
+			// start removing from the least stake. Iterate until we know enough have been removed.
+			let mut removed = 0;
+			for (maybe_index, _stake) in voters_sorted
+				.iter()
+				.map(|(who, stake)| (nominator_index(&who), stake))
+			{
+				let index = maybe_index.ok_or(OffchainElectionError::NominatorSnapshotCorrupt)?;
+				if compact.remove_voter(index) {
+					crate::log!(
+						trace,
+						"💸 removed a voter at index {} with stake {:?} from compact to reduce the size",
+						index,
+						_stake,
+					);
+					removed += 1
+				}
 
-			if removed >= to_remove {
-				break;
+				if removed >= to_remove {
+					break;
+				}
 			}
-		}
 
-		crate::log!(
-			warn,
-			"{} voters of {} had to be removed from compact solution due to size limits.",
-			removed,
-			compact.len() + removed,
-		);
-		Ok(compact)
-	} else {
-		// nada, return as-is
-		crate::log!(
-			info,
-			"Compact solution did not get trimmed due to block weight limits.",
-		);
-		Ok(compact)
+			crate::log!(
+					warn,
+					"💸 {} nominators out of {} had to be removed from compact solution due to size limits.",
+					removed,
+					compact.len() + removed,
+				);
+			Ok(compact)
+		}
+		_ => {
+			// nada, return as-is
+			crate::log!(
+				info,
+				"💸 Compact solution did not get trimmed due to block weight limits.",
+			);
+			Ok(compact)
+		}
 	}
 }
 
@@ -415,7 +418,7 @@ where
 	let maximum_allowed_voters =
 		maximum_compact_len::<T::WeightInfo>(winners.len() as u32, size, maximum_weight);
 
-	crate::log!(info, "Maximum weight = {:?} // current weight = {:?} // maximum voters = {:?} // current votes = {:?}",
+	crate::log!(debug, "💸 Maximum weight = {:?} // current weight = {:?} // maximum voters = {:?} // current votes = {:?}",
 		maximum_weight,
 		T::WeightInfo::submit_solution_better(
 				size.validators.into(),