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

kianenigma · 2022-01-14T11:54:37Z

This allows chain that are stuck in Phase::Emergency to resolve it more easily, without the need to rely on the (really hard to use staking-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 with None 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

frame/election-provider-multi-phase/src/lib.rs

emostov · 2022-01-14T20:34:21Z

frame/election-provider-multi-phase/src/lib.rs

@@ -1010,6 +1028,37 @@ pub mod pallet {
 			});
 			Ok(())
 		}
+
+		/// Trigger the governance fallback.


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

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

emostov · 2022-01-14T20:44:46Z

frame/election-provider-support/src/onchain.rs

+	type DataProvider = T::DataProvider;
+
+	fn elect() -> Result<Supports<T::AccountId>, Self::Error> {
+		Self::elect_with(None, None)


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

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

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

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.

emostov · 2022-01-14T20:47:17Z

Overall looks good just want to make sure I understand #10663 (comment)

Co-authored-by: Zeke Mostov <z.mostov@gmail.com>

frame/election-provider-multi-phase/src/lib.rs

shawntabrizi

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.

frame/election-provider-support/src/onchain.rs

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

…ction-onchain-backup

…ubstrate into kiz-election-onchain-backup

kianenigma · 2022-01-20T07:40:49Z

@georgesdib would you like to prepare the appropriate companions for this one as well? should be fairly understandable for you.

georgesdib · 2022-01-20T23:09:04Z

@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

kianenigma · 2022-01-26T17:05:58Z

bot merge

paritytech-processbot · 2022-01-26T17:06:05Z

Error: Github API says paritytech/polkadot#4757 is not mergeable

kianenigma · 2022-01-27T10:00:37Z

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>

better way to resolve Phase::Emergency via governance

8e0e16c

kianenigma added A0-please_review Pull request needs code review. B7-runtimenoteworthy C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Jan 14, 2022

kianenigma requested review from andresilva, shawntabrizi and emostov January 14, 2022 11:54

emostov reviewed Jan 14, 2022

View reviewed changes

frame/election-provider-multi-phase/src/lib.rs Show resolved Hide resolved

emostov reviewed Jan 14, 2022

View reviewed changes

Update frame/election-provider-multi-phase/src/lib.rs

1424518

Co-authored-by: Zeke Mostov <z.mostov@gmail.com>

kianenigma added this to In progress in Runtime via automation Jan 17, 2022

kianenigma added this to In progress. in NPoS and Elections 💸 🗳 (NEW: https://github.com/orgs/paritytech/projects/33) via automation Jan 17, 2022

kianenigma moved this from In progress to Please Review in Runtime Jan 17, 2022

emostov approved these changes Jan 17, 2022

View reviewed changes

shawntabrizi reviewed Jan 17, 2022

View reviewed changes

frame/election-provider-multi-phase/src/lib.rs Outdated Show resolved Hide resolved

review grumbles

9098ff7

kianenigma requested a review from shawntabrizi January 18, 2022 16:09

Runtime automation moved this from Please Review to Needs Audit Jan 19, 2022

shawntabrizi approved these changes Jan 19, 2022

View reviewed changes

shawntabrizi reviewed Jan 19, 2022

View reviewed changes

frame/election-provider-support/src/onchain.rs Show resolved Hide resolved

Update frame/election-provider-support/src/onchain.rs

93a9103

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

kianenigma added D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited and removed D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Jan 19, 2022

kianenigma added 3 commits January 19, 2022 16:40

Merge branch 'master' of github.com:paritytech/substrate into kiz-ele…

4078d87

…ction-onchain-backup

Merge branch 'kiz-election-onchain-backup' of github.com:paritytech/s…

aee7481

…ubstrate into kiz-election-onchain-backup

revert usize -> u32

0da2906

georgesdib mentioned this pull request Jan 20, 2022

Companion for better way to resolve Phase::Emergency via governance #10663 paritytech/polkadot#4757

Merged

jakoblell added D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Jan 25, 2022

Merge branch 'master' of github.com:paritytech/substrate into kiz-ele…

0cb18f1

…ction-onchain-backup

paritytech-processbot bot merged commit da534d7 into master Jan 27, 2022

paritytech-processbot bot deleted the kiz-election-onchain-backup branch January 27, 2022 10:00

Runtime automation moved this from Needs Audit to Done Jan 27, 2022

NPoS and Elections 💸 🗳 (NEW: https://github.com/orgs/paritytech/projects/33) automation moved this from In progress. to Done Jan 27, 2022

shawntabrizi moved this from Done to Archive in Runtime Feb 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

kianenigma commented Jan 14, 2022 •

edited

Loading

emostov Jan 14, 2022

kianenigma Jan 17, 2022

emostov Jan 14, 2022

kianenigma Jan 17, 2022

shawntabrizi Jan 19, 2022

kianenigma Jan 19, 2022 •

edited

Loading

emostov commented Jan 14, 2022

shawntabrizi left a comment

kianenigma commented Jan 20, 2022

georgesdib commented Jan 20, 2022

kianenigma commented Jan 26, 2022

paritytech-processbot bot commented Jan 26, 2022

kianenigma commented Jan 27, 2022

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

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

Conversation

kianenigma commented Jan 14, 2022 • edited Loading

emostov Jan 14, 2022

Choose a reason for hiding this comment

kianenigma Jan 17, 2022

Choose a reason for hiding this comment

emostov Jan 14, 2022

Choose a reason for hiding this comment

kianenigma Jan 17, 2022

Choose a reason for hiding this comment

shawntabrizi Jan 19, 2022

Choose a reason for hiding this comment

kianenigma Jan 19, 2022 • edited Loading

Choose a reason for hiding this comment

emostov commented Jan 14, 2022

shawntabrizi left a comment

Choose a reason for hiding this comment

kianenigma commented Jan 20, 2022

georgesdib commented Jan 20, 2022

kianenigma commented Jan 26, 2022

paritytech-processbot bot commented Jan 26, 2022

kianenigma commented Jan 27, 2022

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

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

kianenigma commented Jan 14, 2022 •

edited

Loading

kianenigma Jan 19, 2022 •

edited

Loading