Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

better way to resolve Phase::Emergency via governance #10663

Merged
merged 8 commits into from
Jan 27, 2022
14 changes: 8 additions & 6 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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>;
Expand Down Expand Up @@ -647,6 +642,11 @@ impl frame_support::pallet_prelude::Get<Option<(usize, sp_npos_elections::Extend
}
}

impl onchain::Config for Runtime {
type Accuracy = Perbill;
type DataProvider = <Self as pallet_election_provider_multi_phase::Config>::DataProvider;
}

impl pallet_election_provider_multi_phase::Config for Runtime {
type Event = Event;
type Currency = Balances;
Expand All @@ -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<Self>;
type GovernanceFallback =
frame_election_provider_support::onchain::OnChainSequentialPhragmen<Self>;
type Solver = frame_election_provider_support::SequentialPhragmen<
AccountId,
pallet_election_provider_multi_phase::SolutionAccuracyOf<Self>,
Expand Down
60 changes: 57 additions & 3 deletions frame/election-provider-multi-phase/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -321,6 +323,15 @@ impl<T: Config> ElectionProvider for NoFallback<T> {
}
}

impl<T: Config> InstantElectionProvider for NoFallback<T> {
fn instant_elect(
_: Option<usize>,
_: Option<usize>,
) -> Result<Supports<T::AccountId>, Self::Error> {
Err("NoFallback.")
}
}

/// Current phase of the pallet.
#[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, Debug, TypeInfo)]
pub enum Phase<Bn> {
Expand Down Expand Up @@ -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::*;

Expand Down Expand Up @@ -669,13 +680,23 @@ 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.
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
///
/// 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,
DataProvider = Self::DataProvider,
>;

/// OCW election solution miner algorithm implementation.
type Solver: NposSolver<AccountId = Self::AccountId>;

Expand Down Expand Up @@ -1010,6 +1031,37 @@ pub mod pallet {
});
Ok(())
}

/// Trigger the governance fallback.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is OnChainSeqPhragmen the only current option that we offer out of the box that would work here? If so might be good to mention that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but anyone can write a custom one as well.

///
/// 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<T>,
maybe_max_voters: Option<u32>,
maybe_max_targets: Option<u32>,
) -> DispatchResult {
T::ForceOrigin::ensure_origin(origin)?;
ensure!(Self::current_phase().is_emergency(), <Error<T>>::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::<T>::FallbackFailed
},
)?;

let solution =
ReadySolution { supports, score: [0, 0, 0], compute: ElectionCompute::Fallback };

<QueuedSolution<T>>::put(solution);
Ok(())
}
}

#[pallet::event]
Expand Down Expand Up @@ -1060,6 +1112,8 @@ pub mod pallet {
InvalidSubmissionIndex,
/// The call is not allowed at this point.
CallNotAllowed,
/// The fallback failed
FallbackFailed,
}

#[pallet::validate_unsigned]
Expand Down
1 change: 1 addition & 0 deletions frame/election-provider-multi-phase/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,7 @@ impl crate::Config for Runtime {
type WeightInfo = DualMockWeightInfo;
type BenchmarkingConfig = TestBenchmarkingConfig;
type Fallback = MockFallback;
type GovernanceFallback = NoFallback<Self>;
type ForceOrigin = frame_system::EnsureRoot<AccountId>;
type Solution = TestNposSolution;
type VoterSnapshotPerBlock = VoterSnapshotPerBlock;
Expand Down
16 changes: 16 additions & 0 deletions frame/election-provider-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,22 @@ pub trait ElectionProvider {
fn elect() -> Result<Supports<Self::AccountId>, 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<usize>,
maybe_max_targets: Option<usize>,
) -> Result<Supports<Self::AccountId>, Self::Error>;
}

/// An election provider to be used only for testing.
#[cfg(feature = "std")]
pub struct NoElection<X>(sp_std::marker::PhantomData<X>);
Expand Down
53 changes: 40 additions & 13 deletions frame/election-provider-support/src/onchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*};
Expand Down Expand Up @@ -47,8 +47,14 @@ impl From<sp_npos_elections::Error> 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<T: Config>(PhantomData<T>);

/// Configuration trait of [`OnChainSequentialPhragmen`].
Expand All @@ -68,16 +74,17 @@ pub trait Config: frame_system::Config {
>;
}

impl<T: Config> ElectionProvider for OnChainSequentialPhragmen<T> {
type AccountId = T::AccountId;
type BlockNumber = T::BlockNumber;
type Error = Error;
type DataProvider = T::DataProvider;

fn elect() -> Result<Supports<T::AccountId>, 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<T: Config> OnChainSequentialPhragmen<T> {
fn elect_with(
maybe_max_voters: Option<usize>,
maybe_max_targets: Option<usize>,
) -> Result<Supports<T::AccountId>, Error> {
let voters = <Self as ElectionProvider>::DataProvider::voters(maybe_max_voters)
.map_err(Error::DataProvider)?;
let targets = <Self as ElectionProvider>::DataProvider::targets(maybe_max_targets)
.map_err(Error::DataProvider)?;
let desired_targets = <Self as ElectionProvider>::DataProvider::desired_targets()
.map_err(Error::DataProvider)?;

let stake_map: BTreeMap<T::AccountId, VoteWeight> = voters
.iter()
Expand All @@ -103,6 +110,26 @@ impl<T: Config> ElectionProvider for OnChainSequentialPhragmen<T> {
}
}

impl<T: Config> ElectionProvider for OnChainSequentialPhragmen<T> {
type AccountId = T::AccountId;
type BlockNumber = T::BlockNumber;
type Error = Error;
type DataProvider = T::DataProvider;

fn elect() -> Result<Supports<T::AccountId>, Self::Error> {
Self::elect_with(None, None)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a dq, but why is it safe to not put a limit on the number of voters and targets? Do we assume that if you are using OnChainSequentialPhragmen as the election provider then DataProvider::{voters, target} will always return values that won't OOM? If so we should document

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is actually NOT safe, and you should NOT use this. I'll document it a bit better.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh? If this is not safe, can you document something right here?

Copy link
Contributor Author

@kianenigma kianenigma Jan 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is documented at the type, where it is much more visible than being an inline comment in the code:

/// 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.

Note that we never use this except at genesis, as recommended.

}
}

impl<T: Config> InstantElectionProvider for OnChainSequentialPhragmen<T> {
fn instant_elect(
maybe_max_voters: Option<usize>,
maybe_max_targets: Option<usize>,
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
) -> Result<Supports<Self::AccountId>, Self::Error> {
Self::elect_with(maybe_max_voters, maybe_max_targets)
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down