Multi-phase elections solution resubmission

bin/node/runtime/src/lib.rs

@@ -469,6 +469,7 @@ parameter_types! {
 		.get(DispatchClass::Normal)
 		.max_extrinsic.expect("Normal extrinsics have a weight limit configured; qed")
 		.saturating_sub(BlockExecutionWeight::get());
+	pub OffchainRepeat: BlockNumber = 5;
 }
 
 impl pallet_staking::Config for Runtime {
@@ -530,6 +531,7 @@ impl pallet_election_provider_multi_phase::Config for Runtime {
 	type SignedPhase = SignedPhase;
 	type UnsignedPhase = UnsignedPhase;
 	type SolutionImprovementThreshold = MinSolutionScoreBump;
+	type OffchainRepeat = OffchainRepeat;
 	type MinerMaxIterations = MinerMaxIterations;
 	type MinerMaxWeight = MinerMaxWeight;
 	type MinerTxPriority = MultiPhaseUnsignedPriority;

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

@@ -213,7 +213,7 @@ use frame_support::{
 use frame_system::{ensure_none, offchain::SendTransactionTypes};
 use sp_election_providers::{ElectionDataProvider, ElectionProvider, onchain};
 use sp_npos_elections::{
-	assignment_ratio_to_staked_normalized, is_score_better, CompactSolution, ElectionScore,
+	assignment_ratio_to_staked_normalized, CompactSolution, ElectionScore,
 	EvaluateSupport, PerThing128, Supports, VoteWeight,
 };
 use sp_runtime::{
@@ -291,8 +291,16 @@ pub enum Phase<Bn> {
 	Off,
 	/// Signed phase is open.
 	Signed,
-	/// Unsigned phase. First element is whether it is open or not, second the starting block
+	/// Unsigned phase. First element is whether it is active 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)),
 }
 
@@ -303,27 +311,27 @@ impl<Bn> Default for Phase<Bn> {
 }
 
 impl<Bn: PartialEq + Eq> Phase<Bn> {
-	/// Weather the phase is signed or not.
+	/// Whether the phase is signed or not.
 	pub fn is_signed(&self) -> bool {
 		matches!(self, Phase::Signed)
 	}
 
-	/// Weather the phase is unsigned or not.
+	/// Whether the phase is unsigned or not.
 	pub fn is_unsigned(&self) -> bool {
 		matches!(self, Phase::Unsigned(_))
 	}
 
-	/// Weather the phase is unsigned and open or not, with specific start.
+	/// Whether 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)
 	}
 
-	/// Weather the phase is unsigned and open or not.
+	/// Whether the phase is unsigned and open or not.
 	pub fn is_unsigned_open(&self) -> bool {
 		matches!(self, Phase::Unsigned((true, _)))
 	}
 
-	/// Weather the phase is off or not.
+	/// Whether the phase is off or not.
 	pub fn is_off(&self) -> bool {
 		matches!(self, Phase::Off)
 	}
@@ -514,6 +522,13 @@ 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
@@ -595,16 +610,26 @@ pub mod pallet {
 			}
 		}
 
-		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, "miner exeuction done: {:?}", outcome);
+		fn offchain_worker(now: T::BlockNumber) {
+			let threshold = T::OffchainRepeat::get();
+			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, threshold)
+						.and_then(|_| Self::mine_check_save_submit());
+					log!(info, "initial OCW output at {:?}: {:?}", now, initial_output);
+				}
+				Phase::Unsigned((true, opened)) if opened < now => {
+					if !<QueuedSolution<T>>::exists() {
+						// as long as there is no feasible solution, keep trying to submit ours
+						//
+						// the offchain_lock prevents us from spamming submissions too often.
+						let resubmit_output = Self::try_acquire_offchain_lock(now, threshold)
+							.and_then(|_| Self::restore_or_compute_then_submit());
+						log!(info, "resubmit OCW output at {:?}: {:?}", now, resubmit_output);
 					}
-					Err(why) => log!(warn, "denied offchain worker: {:?}", why),
 				}
+				_ => {},
 			}
 		}
 

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

@@ -60,6 +60,7 @@ frame_support::construct_runtime!(
 
 pub(crate) type Balance = u64;
 pub(crate) type AccountId = u64;
+pub(crate) type BlockNumber = u32;
 
 sp_npos_elections::generate_solution_type!(
 	#[compact]
@@ -201,10 +202,10 @@ 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 MockWeightInfo: bool = false;
 
-
 	pub static EpochLength: u64 = 30;
 }
 
@@ -265,6 +266,7 @@ impl crate::Config for Runtime {
 	type SignedPhase = SignedPhase;
 	type UnsignedPhase = UnsignedPhase;
 	type SolutionImprovementThreshold = SolutionImprovementThreshold;
+	type OffchainRepeat = OffchainRepeat;
 	type MinerMaxIterations = MinerMaxIterations;
 	type MinerMaxWeight = MinerMaxWeight;
 	type MinerTxPriority = MinerTxPriority;

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

@@ -17,22 +17,44 @@
 
 //! The unsigned phase implementation.
 
-use crate::*;
-use frame_support::dispatch::DispatchResult;
+use crate::{
+	helpers,
+	Call,
+	CompactAccuracyOf,
+	CompactOf,
+	CompactVoterIndexOf,
+	Config,
+	ElectionCompute,
+	Error,
+	FeasibilityError,
+	Pallet,
+	RawSolution,
+	ReadySolution,
+	RoundSnapshot,
+	SolutionOrSnapshotSize,
+	Weight,
+	WeightInfo,
+};
+use codec::Decode;
+use frame_support::{dispatch::DispatchResult, ensure, traits::Get};
 use frame_system::offchain::SubmitTransaction;
+use sp_arithmetic::Perbill;
 use sp_npos_elections::{
-	seq_phragmen, CompactSolution, ElectionResult, assignment_ratio_to_staked_normalized,
+	assignment_ratio_to_staked_normalized,
 	assignment_staked_to_ratio_normalized,
+	is_score_better,
+	seq_phragmen,
+	CompactSolution,
+	ElectionResult,
 };
 use sp_runtime::{offchain::storage::StorageValueRef, traits::TrailingZeroInput};
-use sp_std::cmp::Ordering;
+use sp_std::{cmp::Ordering, vec::Vec};
 
 /// Storage key used to store the persistent offchain worker status.
 pub(crate) const OFFCHAIN_HEAD_DB: &[u8] = b"parity/multi-phase-unsigned-election";
 
-/// The repeat threshold of the offchain worker. This means we won't run the offchain worker twice
-/// within a window of 5 blocks.
-pub(crate) const OFFCHAIN_REPEAT: u32 = 5;
+/// Storage key used to cache the solution `call`.
+pub(crate) const OFFCHAIN_CACHED_CALL: &[u8] = b"parity/multi-phase-unsigned-election/call";
 
 #[derive(Debug, Eq, PartialEq)]
 pub enum MinerError {
@@ -46,6 +68,8 @@ pub enum MinerError {
 	PreDispatchChecksFailed,
 	/// The solution generated from the miner is not feasible.
 	Feasibility(FeasibilityError),
+	/// Something went wrong fetching the lock.
+	Lock(&'static str),
 }
 
 impl From<sp_npos_elections::Error> for MinerError {
@@ -60,15 +84,48 @@ impl From<FeasibilityError> for MinerError {
 	}
 }
 
+/// Save a given call into OCW storage.
+fn save_solution<T: Config>(call: &Call<T>) {
+	let storage = StorageValueRef::persistent(&OFFCHAIN_CACHED_CALL);
+	storage.set(&call);
+}
+
+/// Get a saved solution from OCW storage if it exists.
+fn restore_solution<T: Config>() -> Option<Call<T>> {
+	StorageValueRef::persistent(&OFFCHAIN_CACHED_CALL).get().flatten()
+}
+
 impl<T: Config> Pallet<T> {
-	/// Mine a new solution, and submit it back to the chain as an unsigned transaction.
-	pub fn mine_check_and_submit() -> Result<(), MinerError> {
+	/// Attempt to restore a solution from cache. Otherwise, compute it fresh. Either way, submit.
+	pub fn restore_or_compute_then_submit() -> Result<(), MinerError> {
+		let call = match restore_solution() {
+			Some(call) => call,
+			None => {
+				let call = Self::mine_call()?;
+				save_solution(&call);
+				call
+			},
+		};
+		Self::submit_call(call)
+	}
+
+	/// Mine a new solution, cache it, and submit it back to the chain as an unsigned transaction.
+	pub fn mine_check_save_submit() -> Result<(), MinerError> {
+		let call = Self::mine_call()?;
+		save_solution(&call);
+		Self::submit_call(call)
+	}
+
+	/// Mine a new solution as a call. Performs all checks.
+	fn mine_call() -> Result<Call<T>, MinerError> {
 		let iters = Self::get_balancing_iters();
 		// get the solution, with a load of checks to ensure if submitted, IT IS ABSOLUTELY VALID.
 		let (raw_solution, witness) = Self::mine_and_check(iters)?;
+		Ok(Call::submit_unsigned(raw_solution, witness))
+	}
 
-		let call = Call::submit_unsigned(raw_solution, witness).into();
-		SubmitTransaction::<T, Call<T>>::submit_unsigned_transaction(call)
+	fn submit_call(call: Call<T>) -> Result<(), MinerError> {
+		SubmitTransaction::<T, Call<T>>::submit_unsigned_transaction(call.into())
 			.map_err(|_| MinerError::PoolSubmissionFailed)
 	}
 
@@ -148,7 +205,7 @@ impl<T: Config> Pallet<T> {
 			.map_err::<MinerError, _>(Into::into)?;
 		sp_npos_elections::reduce(&mut staked);
 
-		// convert back to ration and make compact.
+		// convert back to ratio and make compact.
 		let ratio = assignment_staked_to_ratio_normalized(staked)?;
 		let compact = <CompactOf<T>>::from_assignment(ratio, &voter_index, &target_index)?;
 
@@ -328,12 +385,11 @@ impl<T: Config> Pallet<T> {
 	/// not.
 	///
 	/// This essentially makes sure that we don't run on previous blocks in case of a re-org, and we
-	/// don't run twice within a window of length [`OFFCHAIN_REPEAT`].
+	/// don't run twice within a window of length `threshold`.
 	///
 	/// Returns `Ok(())` if offchain worker should happen, `Err(reason)` otherwise.
-	pub(crate) fn try_acquire_offchain_lock(now: T::BlockNumber) -> Result<(), &'static str> {
+	pub(crate) fn try_acquire_offchain_lock(now: T::BlockNumber, threshold: T::BlockNumber) -> Result<(), MinerError> {
 		let storage = StorageValueRef::persistent(&OFFCHAIN_HEAD_DB);
-		let threshold = T::BlockNumber::from(OFFCHAIN_REPEAT);
 
 		let mutate_stat =
 			storage.mutate::<_, &'static str, _>(|maybe_head: Option<Option<T::BlockNumber>>| {
@@ -357,9 +413,9 @@ impl<T: Config> Pallet<T> {
 			// all good
 			Ok(Ok(_)) => Ok(()),
 			// failed to write.
-			Ok(Err(_)) => Err("failed to write to offchain db."),
+			Ok(Err(_)) => Err(MinerError::Lock("failed to write to offchain db.")),
 			// fork etc.
-			Err(why) => Err(why),
+			Err(why) => Err(MinerError::Lock(why)),
 		}
 	}
 
@@ -400,7 +456,8 @@ impl<T: Config> Pallet<T> {
 #[cfg(test)]
 mod max_weight {
 	#![allow(unused_variables)]
-	use super::{mock::*, *};
+	use super::*;
+	use crate::mock::MultiPhase;
 
 	struct TestWeight;
 	impl crate::weights::WeightInfo for TestWeight {
@@ -481,12 +538,30 @@ mod max_weight {
 
 #[cfg(test)]
 mod tests {
-	use super::{
-		mock::{Origin, *},
-		Call, *,
+	use super::*;
+	use crate::{
+		mock::{
+			roll_to,
+			roll_to_with_ocw,
+			witness,
+			Call as OuterCall,
+			ExtBuilder,
+			Extrinsic,
+			MinerMaxWeight,
+			MultiPhase,
+			Origin,
+			Runtime,
+			TestCompact,
+		},
+		CurrentPhase,
+		InvalidTransaction,
+		Phase,
+		QueuedSolution,
+		TransactionSource,
+		TransactionValidityError,
 	};
-	use frame_support::{dispatch::Dispatchable, traits::OffchainWorker};
-	use mock::Call as OuterCall;
+	use frame_benchmarking::Zero;
+	use frame_support::{assert_noop, assert_ok, dispatch::Dispatchable, traits::OffchainWorker};
 	use sp_election_providers::Assignment;
 	use sp_runtime::{traits::ValidateUnsigned, PerU16};