Multi-phase elections solution resubmission (paritytech#8290)

* not climate

* explain the intent of the bool in the unsigned phase

* remove glob imports from unsigned.rs

* add OffchainRepeat parameter to ElectionProviderMultiPhase

* migrate core logic from paritytech#7976

This is a much smaller diff than that PR contained, but I think
it contains all the essentials.

* improve formatting

* fix test build failures

* cause test to pass

* Apply suggestions from code review

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* collapse imports

* threshold acquired directly within try_acquire_offchain_lock

* add test of resubmission after interval

* add test that ocw can regenerate a failed cache when resubmitting

* ensure that OCW solutions are of the correct round

This should help prevent stale cached solutions from persisting
past the election for which they are intended.

* add test of pre-dispatch round check

* use `RawSolution.round` instead of redundantly externally

* unpack imports

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* rename `OFFCHAIN_HEAD_DB` -> `OFFCHAIN_LOCK`

* rename `mine_call` -> `mine_checked_call`

* eliminate extraneous comma

* check cached call is current before submitting

* remove unused consts introduced by bad merge.

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

* resubmit when our solution beats queued solution

* clear call cache if solution fails to submit

* use local storage; clear on ElectionFinalized

* Revert "use local storage; clear on ElectionFinalized"

This reverts commit 4b46a93.

* BROKEN: try to filter local events in OCW

* use local storage; simplify score fetching

* fix event filter

* mutate storage instead of setting it

* StorageValueRef::local isn't actually implemented yet

* add logging for some events of interest in OCW miner

* rename kill_solution -> kill_ocw_solution to avoid ambiguity

* defensive err instead of unreachable given unreachable code

* doc punctuation

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* distinguish miner errors between "out of date" and "call invalid"

* downgrade info logs -> debug

* ensure encoded call decodes as a call

* fix semantics of validation of pre-dispatch failure for wrong round

* move score check within `and_then`

* add test that offchain workers clear their cache after election

* ensure that bad ocw submissions are not retained for resubmission

* simplify fn ocw_solution_exists

* add feasibility check when restoring cached solution

should address https://github.com/paritytech/substrate/pull/8290/files#r617533358

restructures how the checks are sequenced, which simplifies legibility.

* simplify checks again

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

bin/node/runtime/src/lib.rs

@@ -471,6 +471,7 @@ 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 {
@@ -542,6 +543,7 @@ 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,10 +194,6 @@
 //! **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
@@ -224,7 +220,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, is_score_better, CompactSolution, ElectionScore,
+	assignment_ratio_to_staked_normalized, CompactSolution, ElectionScore,
 	EvaluateSupport, PerThing128, Supports, VoteWeight,
 };
 use sp_runtime::{
@@ -235,7 +231,10 @@ use sp_runtime::{
 	DispatchError, PerThing, Perbill, RuntimeDebug, SaturatedConversion,
 	traits::Bounded,
 };
-use sp_std::prelude::*;
+use sp_std::{
+	convert::TryInto,
+	prelude::*,
+};
 use sp_arithmetic::{
 	UpperOf,
 	traits::{Zero, CheckedAdd},
@@ -304,8 +303,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)),
 }
 
@@ -316,27 +323,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)
 	}
@@ -512,7 +519,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>;
+		type Event: From<Event<Self>> + IsType<<Self as frame_system::Config>::Event> + TryInto<Event<Self>>;
 
 		/// Currency type.
 		type Currency: ReservableCurrency<Self::AccountId> + Currency<Self::AccountId>;
@@ -529,6 +536,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
@@ -638,16 +652,38 @@ 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, "mine_check_and_submit execution done: {:?}", outcome);
-					}
-					Err(why) => log!(warn, "denied offchain worker: {:?}", why),
+		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);
 				}
+				_ => {}
+			}
+			// 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>();
 			}
 		}
 
@@ -784,6 +820,8 @@ 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,6 +61,7 @@ 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;
 
@@ -262,11 +263,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;
 }
 
@@ -334,6 +335,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 MinerMaxLength = MinerMaxLength;