From 8e0e16c8b26be3a17354ddd46bb5fc3b9d172df5 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 14 Jan 2022 11:48:59 +0000 Subject: [PATCH 1/5] better way to resolve Phase::Emergency via governance --- bin/node/runtime/src/lib.rs | 14 +++-- .../election-provider-multi-phase/src/lib.rs | 57 ++++++++++++++++++- .../election-provider-multi-phase/src/mock.rs | 1 + frame/election-provider-support/src/lib.rs | 16 ++++++ .../election-provider-support/src/onchain.rs | 43 ++++++++++---- 5 files changed, 111 insertions(+), 20 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index aab813f29e0aa..b057928958b76 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -23,6 +23,7 @@ #![recursion_limit = "256"] use codec::{Decode, Encode, MaxEncodedLen}; +use frame_election_provider_support::onchain; use frame_support::{ construct_runtime, parameter_types, traits::{ @@ -528,12 +529,6 @@ parameter_types! { pub OffchainRepeat: BlockNumber = 5; } -use frame_election_provider_support::onchain; -impl onchain::Config for Runtime { - type Accuracy = Perbill; - type DataProvider = Staking; -} - pub struct StakingBenchmarkingConfig; impl pallet_staking::BenchmarkingConfig for StakingBenchmarkingConfig { type MaxNominators = ConstU32<1000>; @@ -647,6 +642,11 @@ impl frame_support::pallet_prelude::Get::DataProvider; +} + impl pallet_election_provider_multi_phase::Config for Runtime { type Event = Event; type Currency = Balances; @@ -669,6 +669,8 @@ impl pallet_election_provider_multi_phase::Config for Runtime { type DataProvider = Staking; type Solution = NposSolution16; type Fallback = pallet_election_provider_multi_phase::NoFallback; + type GovernanceFallback = + frame_election_provider_support::onchain::OnChainSequentialPhragmen; type Solver = frame_election_provider_support::SequentialPhragmen< AccountId, pallet_election_provider_multi_phase::SolutionAccuracyOf, diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 8087d22cffe69..4d117f5cafba6 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -230,7 +230,9 @@ #![cfg_attr(not(feature = "std"), no_std)] use codec::{Decode, Encode}; -use frame_election_provider_support::{ElectionDataProvider, ElectionProvider}; +use frame_election_provider_support::{ + ElectionDataProvider, ElectionProvider, InstantElectionProvider, +}; use frame_support::{ dispatch::DispatchResultWithPostInfo, ensure, @@ -321,6 +323,15 @@ impl ElectionProvider for NoFallback { } } +impl InstantElectionProvider for NoFallback { + fn instant_elect( + _: Option, + _: Option, + ) -> Result, Self::Error> { + Err("NoFallback.") + } +} + /// Current phase of the pallet. #[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, Debug, TypeInfo)] pub enum Phase { @@ -552,7 +563,7 @@ pub use pallet::*; #[frame_support::pallet] pub mod pallet { use super::*; - use frame_election_provider_support::NposSolver; + use frame_election_provider_support::{InstantElectionProvider, NposSolver}; use frame_support::{pallet_prelude::*, traits::EstimateCallFee}; use frame_system::pallet_prelude::*; @@ -669,13 +680,20 @@ pub mod pallet { + NposSolution + TypeInfo; - /// Configuration for the fallback + /// Configuration for the fallback. type Fallback: ElectionProvider< AccountId = Self::AccountId, BlockNumber = Self::BlockNumber, DataProvider = Self::DataProvider, >; + /// Configuration of the governance-only fallback. + type GovernanceFallback: InstantElectionProvider< + AccountId = Self::AccountId, + BlockNumber = Self::BlockNumber, + DataProvider = Self::DataProvider, + >; + /// OCW election solution miner algorithm implementation. type Solver: NposSolver; @@ -1010,6 +1028,37 @@ pub mod pallet { }); Ok(()) } + + /// Trigger the governance fallback. + /// + /// This can only be called when [`Phase::Emergency`] is enabled, as an alternative to + /// calling [`Call::set_emergency_election_result`]. + #[pallet::weight(T::DbWeight::get().reads_writes(1, 1))] + pub fn governance_fallback( + origin: OriginFor, + maybe_max_voters: Option, + maybe_max_targets: Option, + ) -> DispatchResult { + T::ForceOrigin::ensure_origin(origin)?; + ensure!(Self::current_phase().is_emergency(), >::CallNotAllowed); + + let maybe_max_voters = maybe_max_voters.map(|x| x as usize); + let maybe_max_targets = maybe_max_targets.map(|x| x as usize); + + let supports = + T::GovernanceFallback::instant_elect(maybe_max_voters, maybe_max_targets).map_err( + |e| { + log!(error, "GovernanceFallback failed: {:?}", e); + Error::::FallbackFailed + }, + )?; + + let solution = + ReadySolution { supports, score: [0, 0, 0], compute: ElectionCompute::Fallback }; + + >::put(solution); + Ok(()) + } } #[pallet::event] @@ -1060,6 +1109,8 @@ pub mod pallet { InvalidSubmissionIndex, /// The call is not allowed at this point. CallNotAllowed, + /// The fallback failed + FallbackFailed, } #[pallet::validate_unsigned] diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 409ebd3f1e10d..a9970299a7c35 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -420,6 +420,7 @@ impl crate::Config for Runtime { type WeightInfo = DualMockWeightInfo; type BenchmarkingConfig = TestBenchmarkingConfig; type Fallback = MockFallback; + type GovernanceFallback = NoFallback; type ForceOrigin = frame_system::EnsureRoot; type Solution = TestNposSolution; type VoterSnapshotPerBlock = VoterSnapshotPerBlock; diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index d10504c88cc67..4b582f49873ef 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -314,6 +314,22 @@ pub trait ElectionProvider { fn elect() -> Result, Self::Error>; } +/// A sub-trait of the [`ElectionProvider`] for cases where we need to be sure an election needs to +/// happen instantly, not asynchronously. +/// +/// The same `DataProvider` is assumed to be used. +/// +/// Consequently, allows for control over the amount of data that is being fetched from the +/// [`ElectionProvider::DataProvider`]. +pub trait InstantElectionProvider: ElectionProvider { + /// Elect a new set of winners, instantly, with the given given limits set on the + /// `DataProvider`. + fn instant_elect( + maybe_max_voters: Option, + maybe_max_targets: Option, + ) -> Result, Self::Error>; +} + /// An election provider to be used only for testing. #[cfg(feature = "std")] pub struct NoElection(sp_std::marker::PhantomData); diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index d325daf514757..c8c02c71d72b7 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -17,7 +17,7 @@ //! An implementation of [`ElectionProvider`] that does an on-chain sequential phragmen. -use crate::{ElectionDataProvider, ElectionProvider}; +use crate::{ElectionDataProvider, ElectionProvider, InstantElectionProvider}; use frame_support::{traits::Get, weights::DispatchClass}; use sp_npos_elections::*; use sp_std::{collections::btree_map::BTreeMap, marker::PhantomData, prelude::*}; @@ -68,16 +68,17 @@ pub trait Config: frame_system::Config { >; } -impl ElectionProvider for OnChainSequentialPhragmen { - type AccountId = T::AccountId; - type BlockNumber = T::BlockNumber; - type Error = Error; - type DataProvider = T::DataProvider; - - fn elect() -> Result, Self::Error> { - let voters = Self::DataProvider::voters(None).map_err(Error::DataProvider)?; - let targets = Self::DataProvider::targets(None).map_err(Error::DataProvider)?; - let desired_targets = Self::DataProvider::desired_targets().map_err(Error::DataProvider)?; +impl OnChainSequentialPhragmen { + fn elect_with( + maybe_max_voters: Option, + maybe_max_targets: Option, + ) -> Result, Error> { + let voters = ::DataProvider::voters(maybe_max_voters) + .map_err(Error::DataProvider)?; + let targets = ::DataProvider::targets(maybe_max_targets) + .map_err(Error::DataProvider)?; + let desired_targets = ::DataProvider::desired_targets() + .map_err(Error::DataProvider)?; let stake_map: BTreeMap = voters .iter() @@ -103,6 +104,26 @@ impl ElectionProvider for OnChainSequentialPhragmen { } } +impl ElectionProvider for OnChainSequentialPhragmen { + type AccountId = T::AccountId; + type BlockNumber = T::BlockNumber; + type Error = Error; + type DataProvider = T::DataProvider; + + fn elect() -> Result, Self::Error> { + Self::elect_with(None, None) + } +} + +impl InstantElectionProvider for OnChainSequentialPhragmen { + fn instant_elect( + maybe_max_voters: Option, + maybe_max_targets: Option, + ) -> Result, Self::Error> { + Self::elect_with(maybe_max_voters, maybe_max_targets) + } +} + #[cfg(test)] mod tests { use super::*; From 1424518edba8a79953560d0831199ffd69db7d64 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Mon, 17 Jan 2022 13:48:06 +0100 Subject: [PATCH 2/5] Update frame/election-provider-multi-phase/src/lib.rs Co-authored-by: Zeke Mostov --- frame/election-provider-multi-phase/src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 4d117f5cafba6..d54e841e01a71 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -687,7 +687,10 @@ pub mod pallet { DataProvider = Self::DataProvider, >; - /// Configuration of the governance-only fallback. + /// Configuration of the governance-only fallback. + /// + /// As a side-note, it is recommend for test-nets to use `type ElectionProvider = + /// OnChainSeqPhragmen<_>` if the test-net is not expected to have thousands of nominators. type GovernanceFallback: InstantElectionProvider< AccountId = Self::AccountId, BlockNumber = Self::BlockNumber, From 9098ff7342bfea209ce4ddd857dee2aa1c69d85a Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 18 Jan 2022 16:06:14 +0000 Subject: [PATCH 3/5] review grumbles --- frame/election-provider-multi-phase/src/lib.rs | 8 ++++---- frame/election-provider-support/src/onchain.rs | 10 ++++++++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index d54e841e01a71..fb1ca9b491f2e 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -687,10 +687,10 @@ pub mod pallet { DataProvider = Self::DataProvider, >; - /// Configuration of the governance-only fallback. - /// - /// As a side-note, it is recommend for test-nets to use `type ElectionProvider = - /// OnChainSeqPhragmen<_>` if the test-net is not expected to have thousands of nominators. + /// Configuration of the governance-only fallback. + /// + /// As a side-note, it is recommend for test-nets to use `type ElectionProvider = + /// OnChainSeqPhragmen<_>` if the test-net is not expected to have thousands of nominators. type GovernanceFallback: InstantElectionProvider< AccountId = Self::AccountId, BlockNumber = Self::BlockNumber, diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index c8c02c71d72b7..a20df9a90bf0b 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -47,8 +47,14 @@ impl From for Error { /// implementation ignores the additional data of the election data provider and gives no insight on /// how much weight was consumed. /// -/// Finally, this implementation does not impose any limits on the number of voters and targets that -/// are provided. +/// Finally, the [`ElectionProvider`] implementation of this type does not impose any limits on the +/// number of voters and targets that are fetched. This could potentially make this unsuitable for +/// execution onchain. On the other hand, the [`InstantElectionProvider`] implementation does limit +/// these inputs. +/// +/// It is advisable to use the former ([`ElectionProvider::elect`]) only at genesis, or for testing, +/// the latter [`InstantElectionProvider::instant_elect`] for onchain operations, with thoughtful +/// bounds. pub struct OnChainSequentialPhragmen(PhantomData); /// Configuration trait of [`OnChainSequentialPhragmen`]. From 93a910305edacab4bf8c4b33eda156fb1fe4851c Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Wed, 19 Jan 2022 17:05:05 +0100 Subject: [PATCH 4/5] Update frame/election-provider-support/src/onchain.rs Co-authored-by: Shawn Tabrizi --- frame/election-provider-support/src/onchain.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index a20df9a90bf0b..2d5be32ca38b3 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -123,8 +123,8 @@ impl ElectionProvider for OnChainSequentialPhragmen { impl InstantElectionProvider for OnChainSequentialPhragmen { fn instant_elect( - maybe_max_voters: Option, - maybe_max_targets: Option, + maybe_max_voters: Option, + maybe_max_targets: Option, ) -> Result, Self::Error> { Self::elect_with(maybe_max_voters, maybe_max_targets) } From 0da2906026de233610922299fd4a610aa67df905 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 19 Jan 2022 16:44:29 +0000 Subject: [PATCH 5/5] revert usize -> u32 --- frame/election-provider-support/src/onchain.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index 2d5be32ca38b3..a20df9a90bf0b 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -123,8 +123,8 @@ impl ElectionProvider for OnChainSequentialPhragmen { impl InstantElectionProvider for OnChainSequentialPhragmen { fn instant_elect( - maybe_max_voters: Option, - maybe_max_targets: Option, + maybe_max_voters: Option, + maybe_max_targets: Option, ) -> Result, Self::Error> { Self::elect_with(maybe_max_voters, maybe_max_targets) }