don't read events in elections anymore. (#9898)

* dont read events in elections anymore.

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

* Fix test for Substrate#9898 (#9907)

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
Co-authored-by: Zeke Mostov <32168567+emostov@users.noreply.github.com>

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

@@ -1201,8 +1201,12 @@ impl<T: Config> Pallet<T> {
 		match current_phase {
 			Phase::Unsigned((true, opened)) if opened == now => {
 				// Mine a new solution, cache it, and attempt to submit it
-				let initial_output = Self::ensure_offchain_repeat_frequency(now)
+				let initial_output = Self::ensure_offchain_repeat_frequency(now).and_then(|_| {
-					.and_then(|_| Self::mine_check_save_submit());
+					// This is executed at the beginning of each round. Any cache is now invalid.
+					// Clear it.
+					unsigned::kill_ocw_solution::<T>();
+					Self::mine_check_save_submit()
+				});
 				log!(debug, "initial offchain thread output: {:?}", initial_output);
 			},
 			Phase::Unsigned((true, opened)) if opened < now => {
@@ -1214,20 +1218,6 @@ impl<T: Config> Pallet<T> {
 			},
 			_ => {},
 		}
-
-		// After election finalization, clear OCW solution storage.
-		//
-		// We can read the events here because offchain worker doesn't affect PoV.
-		if <frame_system::Pallet<T>>::read_events_no_consensus()
-			.into_iter()
-			.filter_map(|event_record| {
-				let local_event = <T as Config>::Event::from(event_record.event);
-				local_event.try_into().ok()
-			})
-			.any(|event| matches!(event, Event::ElectionFinalized(_)))
-		{
-			unsigned::kill_ocw_solution::<T>();
-		}
 	}
 
 	/// Logic for [`<Pallet as Hooks>::on_initialize`] when signed phase is being opened.

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

@@ -1241,35 +1241,62 @@ mod tests {
 	}
 
 	#[test]
-	fn ocw_clears_cache_after_election() {
+	fn ocw_clears_cache_on_unsigned_phase_open() {
-		let (mut ext, _pool) = ExtBuilder::default().build_offchainify(0);
+		let (mut ext, pool) = ExtBuilder::default().build_offchainify(0);
 		ext.execute_with(|| {
-			roll_to(25);
+			const BLOCK: u64 = 25;
-			assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, 25)));
+			let block_plus = |delta: u64| BLOCK + delta;
+			let offchain_repeat = <Runtime as Config>::OffchainRepeat::get();
 
-			// we must clear the offchain storage to ensure the offchain execution check doesn't get
+			roll_to(BLOCK);
-			// in the way.
+			// we are on the first block of the unsigned phase
-			let mut storage = StorageValueRef::persistent(&OFFCHAIN_LAST_BLOCK);
+			assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, BLOCK)));
-			storage.clear();
 
 			assert!(
 				!ocw_solution_exists::<Runtime>(),
 				"no solution should be present before we mine one",
 			);
 
-			// creates and cache a solution
+			// create and cache a solution on the first block of the unsigned phase
-			MultiPhase::offchain_worker(25);
+			MultiPhase::offchain_worker(BLOCK);
 			assert!(
 				ocw_solution_exists::<Runtime>(),
 				"a solution must be cached after running the worker",
 			);
 
-			// after an election, the solution must be cleared
+			// record the submitted tx,
+			let tx_cache_1 = pool.read().transactions[0].clone();
+			// and assume it has been processed.
+			pool.try_write().unwrap().transactions.clear();
+
+			// after an election, the solution is not cleared
 			// we don't actually care about the result of the election
-			roll_to(26);
 			let _ = MultiPhase::do_elect();
-			MultiPhase::offchain_worker(26);
+			MultiPhase::offchain_worker(block_plus(1));
-			assert!(!ocw_solution_exists::<Runtime>(), "elections must clear the ocw cache");
+			assert!(ocw_solution_exists::<Runtime>(), "elections does not clear the ocw cache");
+
+			// submit a solution with the offchain worker after the repeat interval
+			MultiPhase::offchain_worker(block_plus(offchain_repeat + 1));
+
+			// record the submitted tx,
+			let tx_cache_2 = pool.read().transactions[0].clone();
+			// and assume it has been processed.
+			pool.try_write().unwrap().transactions.clear();
+
+			// the OCW submitted the same solution twice since the cache was not cleared.
+			assert_eq!(tx_cache_1, tx_cache_2);
+
+			let current_block = block_plus(offchain_repeat * 2 + 2);
+			// force the unsigned phase to start on the current block.
+			CurrentPhase::<Runtime>::set(Phase::Unsigned((true, current_block)));
+
+			// clear the cache and create a solution since we are on the first block of the unsigned
+			// phase.
+			MultiPhase::offchain_worker(current_block);
+			let tx_cache_3 = pool.read().transactions[0].clone();
+
+			// the submitted solution changes because the cache was cleared.
+			assert_eq!(tx_cache_1, tx_cache_3);
 		})
 	}