Revert "Multi-phase elections solution resubmission (#8290)"

This reverts commit 92c15dd.

bin/node/runtime/src/lib.rs

@@ -471,7 +471,6 @@ parameter_types! {
 	pub const SlashDeferDuration: pallet_staking::EraIndex = 24 * 7; // 1/4 the bonding duration.
 	pub const RewardCurve: &'static PiecewiseLinear<'static> = &REWARD_CURVE;
 	pub const MaxNominatorRewardedPerValidator: u32 = 256;
-	pub OffchainRepeat: BlockNumber = 5;
 }
 
 impl pallet_staking::Config for Runtime {
@@ -543,7 +542,6 @@ impl pallet_election_provider_multi_phase::Config for Runtime {
 	type SignedPhase = SignedPhase;
 	type UnsignedPhase = UnsignedPhase;
 	type SolutionImprovementThreshold = SolutionImprovementThreshold;
-	type OffchainRepeat = OffchainRepeat;
 	type MinerMaxIterations = MinerMaxIterations;
 	type MinerMaxWeight = MinerMaxWeight;
 	type MinerMaxLength = MinerMaxLength;

frame/election-provider-multi-phase/src/lib.rs

@@ -194,6 +194,10 @@
 //! **Score based on (byte) size**: We should always prioritize small solutions over bigger ones, if
 //! there is a tie. Even more harsh should be to enforce the bound of the `reduce` algorithm.
 //!
+//! **Offchain resubmit**: Essentially port <https://github.com/paritytech/substrate/pull/7976> to
+//! this pallet as well. The `OFFCHAIN_REPEAT` also needs to become an adjustable parameter of the
+//! pallet.
+//!
 //! **Make the number of nominators configurable from the runtime**. Remove `sp_npos_elections`
 //! dependency from staking and the compact solution type. It should be generated at runtime, there
 //! it should be encoded how many votes each nominators have. Essentially translate
@@ -220,7 +224,7 @@ use frame_support::{
 use frame_system::{ensure_none, offchain::SendTransactionTypes};
 use frame_election_provider_support::{ElectionDataProvider, ElectionProvider, onchain};
 use sp_npos_elections::{
-	assignment_ratio_to_staked_normalized, CompactSolution, ElectionScore,
+	assignment_ratio_to_staked_normalized, is_score_better, CompactSolution, ElectionScore,
 	EvaluateSupport, PerThing128, Supports, VoteWeight,
 };
 use sp_runtime::{
@@ -231,10 +235,7 @@ use sp_runtime::{
 	DispatchError, PerThing, Perbill, RuntimeDebug, SaturatedConversion,
 	traits::Bounded,
 };
-use sp_std::{
-	convert::TryInto,
-	prelude::*,
-};
+use sp_std::prelude::*;
 use sp_arithmetic::{
 	UpperOf,
 	traits::{Zero, CheckedAdd},
@@ -303,16 +304,8 @@ pub enum Phase<Bn> {
 	Off,
 	/// Signed phase is open.
 	Signed,
-	/// Unsigned phase. First element is whether it is active or not, second the starting block
+	/// Unsigned phase. First element is whether it is open or not, second the starting block
 	/// number.
-	///
-	/// We do not yet check whether the unsigned phase is active or passive. The intent is for the
-	/// blockchain to be able to declare: "I believe that there exists an adequate signed solution,"
-	/// advising validators not to bother running the unsigned offchain worker.
-	///
-	/// As validator nodes are free to edit their OCW code, they could simply ignore this advisory
-	/// and always compute their own solution. However, by default, when the unsigned phase is passive,
-	/// the offchain workers will not bother running.
 	Unsigned((bool, Bn)),
 }
 
@@ -323,27 +316,27 @@ impl<Bn> Default for Phase<Bn> {
 }
 
 impl<Bn: PartialEq + Eq> Phase<Bn> {
-	/// Whether the phase is signed or not.
+	/// Weather the phase is signed or not.
 	pub fn is_signed(&self) -> bool {
 		matches!(self, Phase::Signed)
 	}
 
-	/// Whether the phase is unsigned or not.
+	/// Weather the phase is unsigned or not.
 	pub fn is_unsigned(&self) -> bool {
 		matches!(self, Phase::Unsigned(_))
 	}
 
-	/// Whether the phase is unsigned and open or not, with specific start.
+	/// Weather the phase is unsigned and open or not, with specific start.
 	pub fn is_unsigned_open_at(&self, at: Bn) -> bool {
 		matches!(self, Phase::Unsigned((true, real)) if *real == at)
 	}
 
-	/// Whether the phase is unsigned and open or not.
+	/// Weather the phase is unsigned and open or not.
 	pub fn is_unsigned_open(&self) -> bool {
 		matches!(self, Phase::Unsigned((true, _)))
 	}
 
-	/// Whether the phase is off or not.
+	/// Weather the phase is off or not.
 	pub fn is_off(&self) -> bool {
 		matches!(self, Phase::Off)
 	}
@@ -519,7 +512,7 @@ pub mod pallet {
 
 	#[pallet::config]
 	pub trait Config: frame_system::Config + SendTransactionTypes<Call<Self>> {
-		type Event: From<Event<Self>> + IsType<<Self as frame_system::Config>::Event> + TryInto<Event<Self>>;
+		type Event: From<Event<Self>> + IsType<<Self as frame_system::Config>::Event>;
 
 		/// Currency type.
 		type Currency: ReservableCurrency<Self::AccountId> + Currency<Self::AccountId>;
@@ -536,13 +529,6 @@ pub mod pallet {
 		#[pallet::constant]
 		type SolutionImprovementThreshold: Get<Perbill>;
 
-		/// The repeat threshold of the offchain worker.
-		///
-		/// For example, if it is 5, that means that at least 5 blocks will elapse between attempts
-		/// to submit the worker's solution.
-		#[pallet::constant]
-		type OffchainRepeat: Get<Self::BlockNumber>;
-
 		/// The priority of the unsigned transaction submitted in the unsigned-phase
 		type MinerTxPriority: Get<TransactionPriority>;
 		/// Maximum number of iteration of balancing that will be executed in the embedded miner of
@@ -652,38 +638,16 @@ pub mod pallet {
 			}
 		}
 
-		fn offchain_worker(now: T::BlockNumber) {
-			match Self::current_phase() {
-				Phase::Unsigned((true, opened)) if opened == now => {
-					// mine a new solution, cache it, and attempt to submit it
-					let initial_output = Self::try_acquire_offchain_lock(now)
-						.and_then(|_| Self::mine_check_save_submit());
-					log!(info, "initial OCW output at {:?}: {:?}", now, initial_output);
-				}
-				Phase::Unsigned((true, opened)) if opened < now => {
-					// keep trying to submit solutions. worst case, we note that the stored solution
-					// is better than our cached/computed one, and decide not to submit after all.
-					//
-					// the offchain_lock prevents us from spamming submissions too often.
-					let resubmit_output = Self::try_acquire_offchain_lock(now)
-						.and_then(|_| Self::restore_or_compute_then_maybe_submit());
-					log!(info, "resubmit OCW output at {:?}: {:?}", now, resubmit_output);
+		fn offchain_worker(n: T::BlockNumber) {
+			// We only run the OCW in the first block of the unsigned phase.
+			if Self::current_phase().is_unsigned_open_at(n) {
+				match Self::try_acquire_offchain_lock(n) {
+					Ok(_) => {
+						let outcome = Self::mine_check_and_submit().map_err(ElectionError::from);
+						log!(info, "mine_check_and_submit execution done: {:?}", outcome);
+					}
+					Err(why) => log!(warn, "denied offchain worker: {:?}", why),
 				}
-				_ => {}
-			}
-			// after election finalization, clear OCW solution storage
-			if <frame_system::Pallet<T>>::events()
-				.into_iter()
-				.filter_map(|event_record| {
-					let local_event = <T as Config>::Event::from(event_record.event);
-					local_event.try_into().ok()
-				})
-				.find(|event| {
-					matches!(event, Event::ElectionFinalized(_))
-				})
-				.is_some()
-			{
-				unsigned::kill_ocw_solution::<T>();
 			}
 		}
 
@@ -820,8 +784,6 @@ pub mod pallet {
 		PreDispatchWrongWinnerCount,
 		/// Submission was too weak, score-wise.
 		PreDispatchWeakSubmission,
-		/// OCW submitted solution for wrong round
-		OcwCallWrongEra,
 	}
 
 	#[pallet::origin]

frame/election-provider-multi-phase/src/mock.rs

@@ -61,7 +61,6 @@ frame_support::construct_runtime!(
 
 pub(crate) type Balance = u64;
 pub(crate) type AccountId = u64;
-pub(crate) type BlockNumber = u32;
 pub(crate) type VoterIndex = u32;
 pub(crate) type TargetIndex = u16;
 
@@ -263,11 +262,11 @@ parameter_types! {
 	pub static MinerMaxIterations: u32 = 5;
 	pub static MinerTxPriority: u64 = 100;
 	pub static SolutionImprovementThreshold: Perbill = Perbill::zero();
-	pub static OffchainRepeat: BlockNumber = 5;
 	pub static MinerMaxWeight: Weight = BlockWeights::get().max_block;
 	pub static MinerMaxLength: u32 = 256;
 	pub static MockWeightInfo: bool = false;
 
+
 	pub static EpochLength: u64 = 30;
 }
 
@@ -335,7 +334,6 @@ impl crate::Config for Runtime {
 	type SignedPhase = SignedPhase;
 	type UnsignedPhase = UnsignedPhase;
 	type SolutionImprovementThreshold = SolutionImprovementThreshold;
-	type OffchainRepeat = OffchainRepeat;
 	type MinerMaxIterations = MinerMaxIterations;
 	type MinerMaxWeight = MinerMaxWeight;
 	type MinerMaxLength = MinerMaxLength;