diff --git a/Cargo.lock b/Cargo.lock index aa7d22daccb80..c469e5ffaef8c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6930,6 +6930,7 @@ dependencies = [ "sp-core", "sp-io", "sp-runtime", + "sp-std", ] [[package]] diff --git a/bin/node-template/pallets/template/Cargo.toml b/bin/node-template/pallets/template/Cargo.toml index bb6d8c511af7e..24408c7923006 100644 --- a/bin/node-template/pallets/template/Cargo.toml +++ b/bin/node-template/pallets/template/Cargo.toml @@ -20,6 +20,7 @@ scale-info = { version = "2.5.0", default-features = false, features = ["derive" frame-benchmarking = { version = "4.0.0-dev", default-features = false, optional = true, path = "../../../../frame/benchmarking" } frame-support = { version = "4.0.0-dev", default-features = false, path = "../../../../frame/support" } frame-system = { version = "4.0.0-dev", default-features = false, path = "../../../../frame/system" } +sp-std = { version = "5.0.0", default-features = false, path = "../../../../primitives/std" } [dev-dependencies] sp-core = { version = "7.0.0", path = "../../../../primitives/core" } diff --git a/client/network/src/protocol.rs b/client/network/src/protocol.rs index 0075e856e7574..2f610114a8e85 100644 --- a/client/network/src/protocol.rs +++ b/client/network/src/protocol.rs @@ -440,6 +440,23 @@ impl NetworkBehaviour for Protocol { return Poll::Ready(NetworkBehaviourAction::CloseConnection { peer_id, connection }), }; + while let Poll::Ready(Some(validation_result)) = + self.sync_substream_validations.poll_next_unpin(cx) + { + match validation_result { + Ok((peer, roles)) => { + self.peers.insert(peer, roles); + }, + Err(peer) => { + log::debug!( + target: "sub-libp2p", + "`SyncingEngine` rejected stream" + ); + self.behaviour.disconnect_peer(&peer, HARDCODED_PEERSETS_SYNC); + }, + } + } + let outcome = match event { NotificationsOut::CustomProtocolOpen { peer_id, diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 3599037104d18..1959924f2bf46 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -881,6 +881,11 @@ pub mod pallet { // configure this pallet. assert!(T::SignedMaxSubmissions::get() >= T::SignedMaxRefunds::get()); } + + #[cfg(feature = "try-runtime")] + fn try_state(_n: T::BlockNumber) -> Result<(), &'static str> { + Self::do_try_state() + } } #[pallet::call] @@ -1252,6 +1257,8 @@ pub mod pallet { pub type CurrentPhase = StorageValue<_, Phase, ValueQuery>; /// Current best solution, signed or unsigned, queued to be returned upon `elect`. + /// + /// Always sorted by score. #[pallet::storage] #[pallet::getter(fn queued_solution)] pub type QueuedSolution = @@ -1570,6 +1577,84 @@ impl Pallet { } } +#[cfg(any(feature = "try-runtime", test))] +impl Pallet { + fn do_try_state() -> Result<(), &'static str> { + Self::try_state_snapshot()?; + Self::try_state_signed_submissions_map()?; + Self::try_state_phase_off() + } + + // [`Snapshot`] state check. Invariants: + // - [`DesiredTargets`] exist IFF [`Snapshot`] is present. + // - [`SnapshotMetadata`] exist IFF [`Snapshot`] is present. + fn try_state_snapshot() -> Result<(), &'static str> { + let set = >::get().is_some() && >::get().is_some(); + + match >::take() { + Some(_) if !set => Err("If the snapshot exists, the desired targets and snapshot metadata must also exist."), + None if set => { + println!("desired targets: {:?}", >::get()); + println!("snapshot metadata: {:?}", >::get()); + Err("If the snapshot does not exists, the desired targets and snapshot metadata should also not exists") + }, + _ => Ok(()), + } + } + + // [`SignedSubmissionsMap`] state check. Invariants: + // - All [`SignedSubmissionIndices`] are present in [`SignedSubmissionsMap`], and no more; + // - [`SignedSubmissionNextIndex`] is not present in [`SignedSubmissionsMap`]; + // - [`SignedSubmissionIndices`] is sorted by election score. + fn try_state_signed_submissions_map() -> Result<(), &'static str> { + let mut best_score: ElectionScore = Default::default(); + let indices = >::get(); + + for (i, indice) in indices.iter().enumerate() { + let submission = >::get(indice.2); + if submission.is_none() { + return Err("All signed submissions indices must be part of the submissions map") + } + + if i == 0 { + best_score = indice.0 + } else { + if best_score.strict_threshold_better(indice.0, Perbill::zero()) { + return Err("Signed submission indices vector must be ordered by election score") + } + } + } + + if >::iter().nth(indices.len()).is_some() { + return Err("Signed submissions map length should be the same as the indices vec length") + } + + match >::get() { + 0 => Ok(()), + next => + if >::get(next).is_none() { + Ok(()) + } else { + Err("The next submissions index should not be in the submissions maps already") + }, + } + } + + // [`Phase::Off`] state check. Invariants: + // - If phase is `Phase::Off`, [`Snapshot`] must be none. + fn try_state_phase_off() -> Result<(), &'static str> { + match Self::current_phase().is_off() { + false => Ok(()), + true => + if >::take().is_some() { + Err("Snapshot must be none when in Phase::Off") + } else { + Ok(()) + }, + } + } +} + impl ElectionProviderBase for Pallet { type AccountId = T::AccountId; type BlockNumber = T::BlockNumber; @@ -1642,6 +1727,9 @@ mod feasibility_check { MultiPhase::feasibility_check(solution, COMPUTE), FeasibilityError::SnapshotUnavailable ); + + // restore noop snapshot to ensure post-test `try_state` checks pass. + >::set(Some(Default::default())); }) } diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 8c18777606048..94608464a36a4 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -615,7 +615,24 @@ impl ExtBuilder { } pub fn build_and_execute(self, test: impl FnOnce() -> ()) { - self.build().execute_with(test) + sp_tracing::try_init_simple(); + + let mut ext = self.build(); + ext.execute_with(test); + + // run `try-runtime` post conditions logic at the end of each test, even if + // `try-runtime` feature is disabled. + #[cfg(not(feature = "try-runtime"))] + ext.execute_with(|| { + assert_ok!(MultiPhase::do_try_state()); + }); + + #[cfg(feature = "try-runtime")] + ext.execute_with(|| { + assert_ok!(>::try_state( + System::block_number() + )); + }); } } diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index d83c94db139a4..9b217649754c6 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -325,6 +325,11 @@ pub mod pallet { T::MaxVotesPerVoter::get(), ); } + + #[cfg(feature = "try-runtime")] + fn try_state(_n: T::BlockNumber) -> Result<(), &'static str> { + Self::do_try_state() + } } #[pallet::call] @@ -893,7 +898,7 @@ impl Pallet { } /// Get the members' account ids. - fn members_ids() -> Vec { + pub(crate) fn members_ids() -> Vec { Self::members().into_iter().map(|m| m.who).collect::>() } @@ -1192,6 +1197,104 @@ impl ContainsLengthBound for Pallet { } } +#[cfg(any(feature = "try-runtime", test))] +impl Pallet { + fn do_try_state() -> Result<(), &'static str> { + Self::try_state_members()?; + Self::try_state_runners_up()?; + Self::try_state_candidates()?; + Self::try_state_candidates_runners_up_disjoint()?; + Self::try_state_members_disjoint()?; + Self::try_state_members_approval_stake() + } + + /// [`Members`] state checks. Invariants: + /// - Members are always sorted based on account ID. + fn try_state_members() -> Result<(), &'static str> { + let mut members = Members::::get().clone(); + members.sort_by_key(|m| m.who.clone()); + + if Members::::get() == members { + Ok(()) + } else { + Err("try_state checks: Members must be always sorted by account ID") + } + } + + // [`RunnersUp`] state checks. Invariants: + // - Elements are sorted based on weight (worst to best). + fn try_state_runners_up() -> Result<(), &'static str> { + let mut sorted = RunnersUp::::get(); + // worst stake first + sorted.sort_by(|a, b| a.stake.cmp(&b.stake)); + + if RunnersUp::::get() == sorted { + Ok(()) + } else { + Err("try_state checks: Runners Up must always be sorted by stake (worst to best)") + } + } + + // [`Candidates`] state checks. Invariants: + // - Always sorted based on account ID. + fn try_state_candidates() -> Result<(), &'static str> { + let mut candidates = Candidates::::get().clone(); + candidates.sort_by_key(|(c, _)| c.clone()); + + if Candidates::::get() == candidates { + Ok(()) + } else { + Err("try_state checks: Candidates must be always sorted by account ID") + } + } + // [`Candidates`] and [`RunnersUp`] state checks. Invariants: + // - Candidates and runners-ups sets are disjoint. + fn try_state_candidates_runners_up_disjoint() -> Result<(), &'static str> { + match Self::intersects(&Self::candidates_ids(), &Self::runners_up_ids()) { + true => Err("Candidates and runners up sets should always be disjoint"), + false => Ok(()), + } + } + + // [`Members`], [`Candidates`] and [`RunnersUp`] state checks. Invariants: + // - Members and candidates sets are disjoint; + // - Members and runners-ups sets are disjoint. + fn try_state_members_disjoint() -> Result<(), &'static str> { + match Self::intersects(&Pallet::::members_ids(), &Self::candidates_ids()) && + Self::intersects(&Pallet::::members_ids(), &Self::runners_up_ids()) + { + true => Err("Members set should be disjoint from candidates and runners-up sets"), + false => Ok(()), + } + } + + // [`Members`], [`RunnersUp`] and approval stake state checks. Invariants: + // - Selected members should have approval stake; + // - Selected RunnersUp should have approval stake. + fn try_state_members_approval_stake() -> Result<(), &'static str> { + match Members::::get() + .iter() + .chain(RunnersUp::::get().iter()) + .all(|s| s.stake != BalanceOf::::zero()) + { + true => Ok(()), + false => Err("Members and RunnersUp must have approval stake"), + } + } + + fn intersects(a: &[P], b: &[P]) -> bool { + a.iter().any(|e| b.contains(e)) + } + + fn candidates_ids() -> Vec { + Pallet::::candidates().iter().map(|(x, _)| x).cloned().collect::>() + } + + fn runners_up_ids() -> Vec { + Pallet::::runners_up().into_iter().map(|r| r.who).collect::>() + } +} + #[cfg(test)] mod tests { use super::*; @@ -1418,7 +1521,20 @@ mod tests { .into(); ext.execute_with(pre_conditions); ext.execute_with(test); - ext.execute_with(post_conditions) + + // run `try-runtime` post conditions logic at the end of each test, even if + // `try-runtime` feature is disabled. + #[cfg(not(feature = "try-runtime"))] + ext.execute_with(|| { + assert_ok!(Elections::do_try_state()); + }); + + #[cfg(feature = "try-runtime")] + ext.execute_with(|| { + assert_ok!(>::try_state( + System::block_number() + )); + }); } } @@ -1475,54 +1591,16 @@ mod tests { .unwrap_or_default() } - fn intersects(a: &[T], b: &[T]) -> bool { - a.iter().any(|e| b.contains(e)) - } - - fn ensure_members_sorted() { - let mut members = Elections::members().clone(); - members.sort_by_key(|m| m.who); - assert_eq!(Elections::members(), members); - } - - fn ensure_candidates_sorted() { - let mut candidates = Elections::candidates().clone(); - candidates.sort_by_key(|(c, _)| *c); - assert_eq!(Elections::candidates(), candidates); - } - fn locked_stake_of(who: &u64) -> u64 { Voting::::get(who).stake } - fn ensure_members_has_approval_stake() { - // we filter members that have no approval state. This means that even we have more seats - // than candidates, we will never ever chose a member with no votes. - assert!(Elections::members() - .iter() - .chain(Elections::runners_up().iter()) - .all(|s| s.stake != u64::zero())); - } - - fn ensure_member_candidates_runners_up_disjoint() { - // members, candidates and runners-up must always be disjoint sets. - assert!(!intersects(&members_ids(), &candidate_ids())); - assert!(!intersects(&members_ids(), &runners_up_ids())); - assert!(!intersects(&candidate_ids(), &runners_up_ids())); - } - fn pre_conditions() { System::set_block_number(1); - ensure_members_sorted(); - ensure_candidates_sorted(); - ensure_member_candidates_runners_up_disjoint(); - } - - fn post_conditions() { - ensure_members_sorted(); - ensure_candidates_sorted(); - ensure_member_candidates_runners_up_disjoint(); - ensure_members_has_approval_stake(); + Elections::try_state_members().unwrap(); + Elections::try_state_members_approval_stake().unwrap(); + Elections::try_state_candidates().unwrap(); + Elections::try_state_candidates_runners_up_disjoint().unwrap(); } fn submit_candidacy(origin: RuntimeOrigin) -> sp_runtime::DispatchResult {