-
Notifications
You must be signed in to change notification settings - Fork 2.6k
better way to resolve Phase::Emergency
via governance
#10663
Conversation
@@ -1010,6 +1028,37 @@ pub mod pallet { | |||
}); | |||
Ok(()) | |||
} | |||
|
|||
/// Trigger the governance fallback. |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
type DataProvider = T::DataProvider; | ||
|
||
fn elect() -> Result<Supports<T::AccountId>, Self::Error> { | ||
Self::elect_with(None, None) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
Overall looks good just want to make sure I understand #10663 (comment) |
Co-authored-by: Zeke Mostov <z.mostov@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure this makes sense to me.
Basically, in a really bad situation, we just pick the first N people rather than doing any fancy calculation which may stall us.
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
…ction-onchain-backup
…ubstrate into kiz-election-onchain-backup
@georgesdib would you like to prepare the appropriate companions for this one as well? should be fairly understandable for you. |
I just created that under: Polkadot companion: paritytech/polkadot#4757 cumulus seem to build fine. |
…ction-onchain-backup
bot merge |
Error: Github API says paritytech/polkadot#4757 is not mergeable |
bot merge |
…0663) * better way to resolve Phase::Emergency via governance * Update frame/election-provider-multi-phase/src/lib.rs Co-authored-by: Zeke Mostov <z.mostov@gmail.com> * review grumbles * Update frame/election-provider-support/src/onchain.rs Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com> * revert usize -> u32 Co-authored-by: Zeke Mostov <z.mostov@gmail.com> Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
…0663) * better way to resolve Phase::Emergency via governance * Update frame/election-provider-multi-phase/src/lib.rs Co-authored-by: Zeke Mostov <z.mostov@gmail.com> * review grumbles * Update frame/election-provider-support/src/onchain.rs Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com> * revert usize -> u32 Co-authored-by: Zeke Mostov <z.mostov@gmail.com> Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
…0663) * better way to resolve Phase::Emergency via governance * Update frame/election-provider-multi-phase/src/lib.rs Co-authored-by: Zeke Mostov <z.mostov@gmail.com> * review grumbles * Update frame/election-provider-support/src/onchain.rs Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com> * revert usize -> u32 Co-authored-by: Zeke Mostov <z.mostov@gmail.com> Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
This allows chain that are stuck in
Phase::Emergency
to resolve it more easily, without the need to rely on the (really hard to usestaking-miner
).This also closes #8348, we can now have bounded on-chain election. Although, using it in a chain that does not have bags-list is kinda stupid, because you are basically relying on random order of nominators, if you pass
Some
to the call parameters.Nonetheless, for most testnets, this is a big relief. When chain goes to
Phase::emergency
, call this new call withNone
and it should become unblocked by the next epoch.As a side-note, I want to highly recommend to all test-nets to actually use
type ElectionProvider = OnChainSeqPhragmen<_>
in the first place, if their test-net is not expected to have thousands of nominators.Polkadot companion: paritytech/polkadot#4757