diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 498593e57a1ff..a8240679aeaee 100644 --- a/bin/node/runtime/src/lib.rs +++ b/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; diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index c59d68a33adba..e4fed277cf4fc 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/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 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 { 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 Default for Phase { } impl Phase { - /// 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> { - type Event: From> + IsType<::Event>; + type Event: From> + IsType<::Event> + TryInto>; /// Currency type. type Currency: ReservableCurrency + Currency; @@ -529,6 +536,13 @@ pub mod pallet { #[pallet::constant] type SolutionImprovementThreshold: Get; + /// 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; + /// The priority of the unsigned transaction submitted in the unsigned-phase type MinerTxPriority: Get; /// 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 >::events() + .into_iter() + .filter_map(|event_record| { + let local_event = ::Event::from(event_record.event); + local_event.try_into().ok() + }) + .find(|event| { + matches!(event, Event::ElectionFinalized(_)) + }) + .is_some() + { + unsigned::kill_ocw_solution::(); } } @@ -784,6 +820,8 @@ pub mod pallet { PreDispatchWrongWinnerCount, /// Submission was too weak, score-wise. PreDispatchWeakSubmission, + /// OCW submitted solution for wrong round + OcwCallWrongEra, } #[pallet::origin] diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index f3cf00f2ca0c8..f57836178d497 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/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; diff --git a/frame/election-provider-multi-phase/src/unsigned.rs b/frame/election-provider-multi-phase/src/unsigned.rs index 8ab3a81aa3d27..ebeae3dc472fb 100644 --- a/frame/election-provider-multi-phase/src/unsigned.rs +++ b/frame/election-provider-multi-phase/src/unsigned.rs @@ -17,22 +17,27 @@ //! The unsigned phase implementation. -use crate::*; -use frame_support::dispatch::DispatchResult; +use crate::{ + helpers, Call, CompactAccuracyOf, CompactOf, Config, + ElectionCompute, Error, FeasibilityError, Pallet, RawSolution, ReadySolution, RoundSnapshot, + SolutionOrSnapshotSize, Weight, WeightInfo, +}; +use codec::{Encode, 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_staked_to_ratio_normalized, + CompactSolution, ElectionResult, ElectionScore, assignment_ratio_to_staked_normalized, + assignment_staked_to_ratio_normalized, is_score_better, seq_phragmen, }; -use sp_runtime::{offchain::storage::StorageValueRef, traits::TrailingZeroInput}; -use sp_std::{cmp::Ordering, convert::TryFrom}; +use sp_runtime::{offchain::storage::StorageValueRef, traits::TrailingZeroInput, SaturatedConversion}; +use sp_std::{cmp::Ordering, convert::TryFrom, 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"; +pub(crate) const OFFCHAIN_LOCK: &[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"; /// A voter's fundamental data: their ID, their stake, and the list of candidates for whom they /// voted. @@ -63,6 +68,16 @@ pub enum MinerError { PreDispatchChecksFailed, /// The solution generated from the miner is not feasible. Feasibility(FeasibilityError), + /// Something went wrong fetching the lock. + Lock(&'static str), + /// Cannot restore a solution that was not stored. + NoStoredSolution, + /// Cached solution does not match the current round. + SolutionOutOfDate, + /// Cached solution is not a `submit_unsigned` call. + SolutionCallInvalid, + /// Failed to store a solution. + FailedToStoreSolution, /// There are no more voters to remove to trim the solution. NoMoreVoters, } @@ -79,25 +94,142 @@ impl From for MinerError { } } +/// Save a given call into OCW storage. +fn save_solution(call: &Call) -> Result<(), MinerError> { + let storage = StorageValueRef::persistent(&OFFCHAIN_CACHED_CALL); + match storage.mutate::<_, (), _>(|_| Ok(call.clone())) { + Ok(Ok(_)) => Ok(()), + Ok(Err(_)) => Err(MinerError::FailedToStoreSolution), + Err(_) => { + // this branch should be unreachable according to the definition of `StorageValueRef::mutate`: + // that function should only ever `Err` if the closure we pass it return an error. + // however, for safety in case the definition changes, we do not optimize the branch away + // or panic. + Err(MinerError::FailedToStoreSolution) + }, + } +} + +/// Get a saved solution from OCW storage if it exists. +fn restore_solution() -> Result, MinerError> { + StorageValueRef::persistent(&OFFCHAIN_CACHED_CALL) + .get() + .flatten() + .ok_or(MinerError::NoStoredSolution) +} + +/// Clear a saved solution from OCW storage. +pub(super) fn kill_ocw_solution() { + let mut storage = StorageValueRef::persistent(&OFFCHAIN_CACHED_CALL); + storage.clear(); +} + +/// `true` when OCW storage contains a solution +/// +/// More precise than `restore_solution::().is_ok()`; that invocation will return `false` +/// if a solution exists but cannot be decoded, whereas this just checks whether an item is present. +#[cfg(test)] +fn ocw_solution_exists() -> bool { + StorageValueRef::persistent(&OFFCHAIN_CACHED_CALL).get::>().is_some() +} + impl Pallet { - /// 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 + /// if our call's score is greater than that of the cached solution. + pub fn restore_or_compute_then_maybe_submit() -> Result<(), MinerError> { + log!( + debug, + "OCW attempting to restore or compute an unsigned solution for the current election" + ); + + let call = restore_solution::() + .and_then(|call| { + // ensure the cached call is still current before submitting + if let Call::submit_unsigned(solution, _) = &call { + // prevent errors arising from state changes in a forkful chain + Self::basic_checks(solution, "restored")?; + Ok(call) + } else { + Err(MinerError::SolutionCallInvalid) + } + }) + .or_else::(|_| { + // if not present or cache invalidated, regenerate + let (call, _) = Self::mine_checked_call()?; + save_solution(&call)?; + Ok(call) + })?; + + // the runtime will catch it and reject the transaction if the phase is wrong, but it's + // cheap and easy to check it here to ease the workload on the runtime, so: + if !Self::current_phase().is_unsigned_open() { + // don't bother submitting; it's not an error, we're just too late. + return Ok(()); + } + + // in case submission fails for any reason, `submit_call` kills the stored solution + 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> { + log!( + debug, + "OCW attempting to compute an unsigned solution for the current election" + ); + + let (call, _) = Self::mine_checked_call()?; + save_solution(&call)?; + Self::submit_call(call) + } + + /// Mine a new solution as a call. Performs all checks. + fn mine_checked_call() -> Result<(Call, ElectionScore), 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)?; + let score = raw_solution.score.clone(); + let call: Call = Call::submit_unsigned(raw_solution, witness).into(); - let call: >>::OverarchingCall = - Call::submit_unsigned(raw_solution, witness).into(); log!( - info, - "mined a solution with score {:?} and size {}", + debug, + "OCW mined a solution with score {:?} and size {}", score, call.using_encoded(|b| b.len()) ); - SubmitTransaction::>::submit_unsigned_transaction(call) - .map_err(|_| MinerError::PoolSubmissionFailed) + Ok((call, score)) + } + + fn submit_call(call: Call) -> Result<(), MinerError> { + log!( + debug, + "OCW submitting a solution as an unsigned transaction", + ); + + SubmitTransaction::>::submit_unsigned_transaction(call.into()) + .map_err(|_| { + kill_ocw_solution::(); + MinerError::PoolSubmissionFailed + }) + } + + // perform basic checks of a solution's validity + // + // Performance: note that it internally clones the provided solution. + fn basic_checks(raw_solution: &RawSolution>, solution_type: &str) -> Result<(), MinerError> { + Self::unsigned_pre_dispatch_checks(raw_solution).map_err(|err| { + log!(warn, "pre-dispatch checks fialed for {} solution: {:?}", solution_type, err); + MinerError::PreDispatchChecksFailed + })?; + + Self::feasibility_check(raw_solution.clone(), ElectionCompute::Unsigned).map_err(|err| { + log!(warn, "feasibility check failed for {} solution: {:?}", solution_type, err); + err + })?; + + Ok(()) } /// Mine a new npos solution, with all the relevant checks to make sure that it will be accepted @@ -105,26 +237,12 @@ impl Pallet { /// /// If you want an unchecked solution, use [`Pallet::mine_solution`]. /// If you want a checked solution and submit it at the same time, use - /// [`Pallet::mine_check_and_submit`]. + /// [`Pallet::mine_check_save_submit`]. pub fn mine_and_check( iters: usize, ) -> Result<(RawSolution>, SolutionOrSnapshotSize), MinerError> { let (raw_solution, witness) = Self::mine_solution(iters)?; - - // ensure that this will pass the pre-dispatch checks - Self::unsigned_pre_dispatch_checks(&raw_solution).map_err(|e| { - log!(warn, "pre-dispatch-checks failed for mined solution: {:?}", e); - MinerError::PreDispatchChecksFailed - })?; - - // ensure that this is a feasible solution - let _ = Self::feasibility_check(raw_solution.clone(), ElectionCompute::Unsigned).map_err( - |e| { - log!(warn, "feasibility-check failed for mined solution: {:?}", e); - MinerError::from(e) - }, - )?; - + Self::basic_checks(&raw_solution, "mined")?; Ok((raw_solution, witness)) } @@ -439,12 +557,14 @@ impl Pallet { /// 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> { - let storage = StorageValueRef::persistent(&OFFCHAIN_HEAD_DB); - let threshold = T::BlockNumber::from(OFFCHAIN_REPEAT); + pub(crate) fn try_acquire_offchain_lock( + now: T::BlockNumber, + ) -> Result<(), MinerError> { + let threshold = T::OffchainRepeat::get(); + let storage = StorageValueRef::persistent(&OFFCHAIN_LOCK); let mutate_stat = storage.mutate::<_, &'static str, _>(|maybe_head: Option>| { @@ -468,9 +588,9 @@ impl Pallet { // 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)), } } @@ -487,6 +607,9 @@ impl Pallet { // ensure solution is timely. Don't panic yet. This is a cheap check. ensure!(Self::current_phase().is_unsigned_open(), Error::::PreDispatchEarlySubmission); + // ensure round is current + ensure!(Self::round() == solution.round, Error::::OcwCallWrongEra); + // ensure correct number of winners. ensure!( Self::desired_targets().unwrap_or_default() @@ -511,7 +634,8 @@ impl Pallet { #[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 { @@ -597,13 +721,15 @@ mod max_weight { mod tests { use super::*; use crate::{ + CurrentPhase, InvalidTransaction, Phase, QueuedSolution, TransactionSource, + TransactionValidityError, mock::{ - assert_noop, assert_ok, ExtBuilder, Extrinsic, MinerMaxWeight, MultiPhase, Origin, - roll_to_with_ocw, roll_to, Runtime, TestCompact, TrimHelpers, trim_helpers, witness, + Call as OuterCall, ExtBuilder, Extrinsic, MinerMaxWeight, MultiPhase, Origin, Runtime, + TestCompact, TrimHelpers, roll_to, roll_to_with_ocw, trim_helpers, witness, }, }; - 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_npos_elections::IndexAssignment; use sp_runtime::{traits::ValidateUnsigned, PerU16}; @@ -845,7 +971,7 @@ mod tests { assert!(MultiPhase::current_phase().is_unsigned()); assert_eq!( - MultiPhase::mine_check_and_submit().unwrap_err(), + MultiPhase::mine_check_save_submit().unwrap_err(), MinerError::PreDispatchChecksFailed, ); }) @@ -924,6 +1050,8 @@ mod tests { fn ocw_check_prevent_duplicate() { let (mut ext, _) = ExtBuilder::default().build_offchainify(0); ext.execute_with(|| { + let offchain_repeat = ::OffchainRepeat::get(); + roll_to(25); assert!(MultiPhase::current_phase().is_unsigned()); @@ -931,21 +1059,15 @@ mod tests { assert!(MultiPhase::try_acquire_offchain_lock(25).is_ok()); // next block: rejected. - assert_noop!(MultiPhase::try_acquire_offchain_lock(26), "recently executed."); + assert_noop!(MultiPhase::try_acquire_offchain_lock(26), MinerError::Lock("recently executed.")); // allowed after `OFFCHAIN_REPEAT` - assert!(MultiPhase::try_acquire_offchain_lock((26 + OFFCHAIN_REPEAT).into()).is_ok()); + assert!(MultiPhase::try_acquire_offchain_lock((26 + offchain_repeat).into()).is_ok()); // a fork like situation: re-execute last 3. - assert_noop!( - MultiPhase::try_acquire_offchain_lock((26 + OFFCHAIN_REPEAT - 3).into()), "fork." - ); - assert_noop!( - MultiPhase::try_acquire_offchain_lock((26 + OFFCHAIN_REPEAT - 2).into()), "fork." - ); - assert_noop!( - MultiPhase::try_acquire_offchain_lock((26 + OFFCHAIN_REPEAT - 1).into()), "fork." - ); + assert!(MultiPhase::try_acquire_offchain_lock((26 + offchain_repeat - 3).into()).is_err()); + assert!(MultiPhase::try_acquire_offchain_lock((26 + offchain_repeat - 2).into()).is_err()); + assert!(MultiPhase::try_acquire_offchain_lock((26 + offchain_repeat - 1).into()).is_err()); }) } @@ -958,19 +1080,131 @@ mod tests { // we must clear the offchain storage to ensure the offchain execution check doesn't get // in the way. - let mut storage = StorageValueRef::persistent(&OFFCHAIN_HEAD_DB); + let mut storage = StorageValueRef::persistent(&OFFCHAIN_LOCK); MultiPhase::offchain_worker(24); assert!(pool.read().transactions.len().is_zero()); storage.clear(); + // creates, caches, submits without expecting previous cache value + MultiPhase::offchain_worker(25); + assert_eq!(pool.read().transactions.len(), 1); + // assume that the tx has been processed + pool.try_write().unwrap().transactions.clear(); + + // locked, but also, has previously cached. MultiPhase::offchain_worker(26); assert!(pool.read().transactions.len().is_zero()); + }) + } + + #[test] + fn ocw_clears_cache_after_election() { + let (mut ext, _pool) = ExtBuilder::default().build_offchainify(0); + ext.execute_with(|| { + roll_to(25); + assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, 25))); + + // we must clear the offchain storage to ensure the offchain execution check doesn't get + // in the way. + let mut storage = StorageValueRef::persistent(&OFFCHAIN_LOCK); storage.clear(); - // submits! + assert!(!ocw_solution_exists::(), "no solution should be present before we mine one"); + + // creates and cache a solution MultiPhase::offchain_worker(25); - assert!(!pool.read().transactions.len().is_zero()); + assert!(ocw_solution_exists::(), "a solution must be cached after running the worker"); + + // after an election, the solution must be cleared + // we don't actually care about the result of the election + roll_to(26); + let _ = MultiPhase::do_elect(); + MultiPhase::offchain_worker(26); + assert!(!ocw_solution_exists::(), "elections must clear the ocw cache"); + }) + } + + #[test] + fn ocw_resubmits_after_offchain_repeat() { + let (mut ext, pool) = ExtBuilder::default().build_offchainify(0); + ext.execute_with(|| { + const BLOCK: u64 = 25; + let block_plus = |delta: i32| ((BLOCK as i32) + delta) as u64; + let offchain_repeat = ::OffchainRepeat::get(); + + roll_to(BLOCK); + assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, BLOCK))); + + // we must clear the offchain storage to ensure the offchain execution check doesn't get + // in the way. + let mut storage = StorageValueRef::persistent(&OFFCHAIN_LOCK); + + MultiPhase::offchain_worker(block_plus(-1)); + assert!(pool.read().transactions.len().is_zero()); + storage.clear(); + + // creates, caches, submits without expecting previous cache value + MultiPhase::offchain_worker(BLOCK); + assert_eq!(pool.read().transactions.len(), 1); + let tx_cache = pool.read().transactions[0].clone(); + // assume that the tx has been processed + pool.try_write().unwrap().transactions.clear(); + + // attempts to resubmit the tx after the threshold has expired + // note that we have to add 1: the semantics forbid resubmission at + // BLOCK + offchain_repeat + MultiPhase::offchain_worker(block_plus(1 + offchain_repeat as i32)); + assert_eq!(pool.read().transactions.len(), 1); + + // resubmitted tx is identical to first submission + let tx = &pool.read().transactions[0]; + assert_eq!(&tx_cache, tx); + }) + } + + #[test] + fn ocw_regenerates_and_resubmits_after_offchain_repeat() { + let (mut ext, pool) = ExtBuilder::default().build_offchainify(0); + ext.execute_with(|| { + const BLOCK: u64 = 25; + let block_plus = |delta: i32| ((BLOCK as i32) + delta) as u64; + let offchain_repeat = ::OffchainRepeat::get(); + + roll_to(BLOCK); + assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, BLOCK))); + + // we must clear the offchain storage to ensure the offchain execution check doesn't get + // in the way. + let mut storage = StorageValueRef::persistent(&OFFCHAIN_LOCK); + + MultiPhase::offchain_worker(block_plus(-1)); + assert!(pool.read().transactions.len().is_zero()); + storage.clear(); + + // creates, caches, submits without expecting previous cache value + MultiPhase::offchain_worker(BLOCK); + assert_eq!(pool.read().transactions.len(), 1); + let tx_cache = pool.read().transactions[0].clone(); + // assume that the tx has been processed + pool.try_write().unwrap().transactions.clear(); + + // remove the cached submitted tx + // this ensures that when the resubmit window rolls around, we're ready to regenerate + // from scratch if necessary + let mut call_cache = StorageValueRef::persistent(&OFFCHAIN_CACHED_CALL); + assert!(matches!(call_cache.get::>(), Some(Some(_call)))); + call_cache.clear(); + + // attempts to resubmit the tx after the threshold has expired + // note that we have to add 1: the semantics forbid resubmission at + // BLOCK + offchain_repeat + MultiPhase::offchain_worker(block_plus(1 + offchain_repeat as i32)); + assert_eq!(pool.read().transactions.len(), 1); + + // resubmitted tx is identical to first submission + let tx = &pool.read().transactions[0]; + assert_eq!(&tx_cache, tx); }) } @@ -985,7 +1219,38 @@ mod tests { let encoded = pool.read().transactions[0].clone(); let extrinsic: Extrinsic = Decode::decode(&mut &*encoded).unwrap(); let call = extrinsic.call; - assert!(matches!(call, OuterCall::MultiPhase(Call::submit_unsigned(_, _)))); + assert!(matches!(call, OuterCall::MultiPhase(Call::submit_unsigned(..)))); + }) + } + + #[test] + fn ocw_solution_must_have_correct_round() { + let (mut ext, pool) = ExtBuilder::default().build_offchainify(0); + ext.execute_with(|| { + roll_to_with_ocw(25); + assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, 25))); + // OCW must have submitted now + // now, before we check the call, update the round + >::mutate(|round| *round += 1); + + let encoded = pool.read().transactions[0].clone(); + let extrinsic = Extrinsic::decode(&mut &*encoded).unwrap(); + let call = match extrinsic.call { + OuterCall::MultiPhase(call @ Call::submit_unsigned(..)) => call, + _ => panic!("bad call: unexpected submission"), + }; + + // Custom(3) maps to PreDispatchChecksFailed + let pre_dispatch_check_error = TransactionValidityError::Invalid(InvalidTransaction::Custom(3)); + assert_eq!( + ::validate_unsigned(TransactionSource::Local, &call) + .unwrap_err(), + pre_dispatch_check_error, + ); + assert_eq!( + ::pre_dispatch(&call).unwrap_err(), + pre_dispatch_check_error, + ); }) }