Introduces Governance v2 in Moonbase Alpha #1838

4meta5 · 2022-09-30T21:14:42Z

Supercedes #1762 to add the moonbase Gov2 config. Follows conventions of paritytech/polkadot#5205 to split governance config into own folder.

This PR preserves the existing Governance config with the intention of only including the new Gov2 config for testing purposes (both Gov1 and Gov2 will be supported initially). This PR adds following pallet configs to moonbase:

pallet-preimage: allows storing the preimage of a hash on chain; we configure pallet_scheduler::Config::PreimageProvider = Preimage
pallet-whitelist: queues calls to be dispatched later by root
pallet-conviction-voting: manages tally and votes for pallet-referenda in decoupled way
pallet-referenda: pallet for executing referenda wherein referendum is a vote on whether a proposal should be dispatched from a particular origin such that each origin has its own governance parameters i.e. for root

pallet_referenda::TrackInfo {
	// Name of this track.
	name: "root",
	// A limit for the number of referenda on this track that can be being decided at once.
	// For Root origin this should generally be just one.
	max_deciding: 1,
	// Amount that must be placed on deposit before a decision can be made.
	decision_deposit: 1_000 * KILOUNIT,
	// Amount of time this must be submitted for before a decision can be made.
	prepare_period: 3 * HOURS,
	// Amount of time that a decision may take to be approved prior to cancellation.
	decision_period: 28 * DAYS,
	// Amount of time that the approval criteria must hold before it can be approved.
	confirm_period: 3 * HOURS,
	// Minimum amount of time that an approved proposal must be in the dispatch queue.
	min_enactment_period: 3 * HOURS,
	// Minimum aye votes as percentage of overall conviction-weighted votes needed for
	// approval as a function of time into decision period.
	min_approval: APP_ROOT,
	// Minimum pre-conviction aye-votes ("support") as percentage of overall population that
	// is needed for approval as a function of time into decision period.
	min_support: SUP_ROOT,
}

This PR does NOT include Fellowship configuration

No migration required. We do not need another migration to set the scheduler::Config::PreimageProvider to the newly configured preimage pallet. Democracy does not use the preimages pallet, it uses its own internal storage item. The scheduler pallet will now set the preimage pallet as the preimage provider. Pallet referenda only uses the preimage pallet through its usage of the scheduler pallet configured with PreimageProvider = Preimage.

Tracks

evolution of configured curves over decision period, TODO make graphs

Fast Track

Whitelist track is the fast track. Here are the steps:

whitelist::whitelist_call(call_hash: T::Hash) from configured WhitelistOrigin = EitherOf<Root, TechCommittee 2 of 3>
referenda::submit with Origins::WhitelistedCaller and proposal hash of whitelist::dispatch_whitelisted_call(...) with the inner call argument equal to the call_hash passed in (1)
go through referenda path until approved
must submit preimages before approved proposals will be dispatched

…igrate councils

* gov2 config init with whitelist origin configured with tech committee 2 of 3 * test * clean * fix and docs * fix * try fix * try fix * fix and clean * fix

…he identity registrar

runtime/moonbase/src/governance/origins.rs

runtime/moonbase/src/governance/referenda.rs

runtime/moonbase/src/lib.rs

notlesh · 2022-10-12T15:50:34Z

runtime/moonbase/src/governance/referenda.rs

+	type Event = Event;
+	type Call = Call;
+	type WhitelistOrigin = EitherOf<
+		EnsureRootWithSuccess<Self::AccountId, ConstU16<65535>>,


It's not clear what this ConstU16<65535> is doing, and I don't see any useful documentation around EnsureRootWithSuccess. Can you point me in the right direction?

The impl for it is:

pub struct EnsureRootWithSuccess<AccountId, Success>( sp_std::marker::PhantomData<(AccountId, Success)>, ); impl< O: Into<Result<RawOrigin<AccountId>, O>> + From<RawOrigin<AccountId>>, AccountId, Success: TypedGet, > EnsureOrigin<O> for EnsureRootWithSuccess<AccountId, Success> { type Success = Success::Type; fn try_origin(o: O) -> Result<Self::Success, O> { o.into().and_then(|o| match o { RawOrigin::Root => Ok(Success::get()), r => Err(O::from(r)), }) } #[cfg(feature = "runtime-benchmarks")] fn try_successful_origin() -> Result<O, ()> { Ok(O::from(RawOrigin::Root)) } }

It looks to me like all it will do is return Result::Ok(65535) in try_origin.

It is just the EnsureRoot required as a generic for EitherOf. I only used this constant because it is what is used in the Kusama Gov2 PR (used 5 times in that PR with EnsureRootWithSuccess i.e. https://github.com/paritytech/polkadot/pull/5205/files#diff-b2299d42ae2d30e47a174007af1e60a2bf92cec42c2628d7e419a05b2ebbe435R71-R72 )

The WhitelistOrigin = Either<EnsureRoot, TechCommitee 2 of 3>. The use of the constants and the map reduce are just to make it work. I followed the conventions of that Kusama Gov2 PR for these conventions, but I do not understand them (the constants).

notlesh · 2022-10-12T15:58:13Z

runtime/moonbase/src/governance/tracks.rs

+use super::*;
+use crate::currency::{KILOUNIT, SUPPLY_FACTOR, UNIT};
+
+const fn percent(x: i32) -> sp_runtime::FixedI64 {


I think you will get the same precision and useful range out of a Perbill with only 32 bits.

Perbill: 32 bits, range: [0, 1] with 9 decimal points of precision
FixedI64: 64 bits, range [-9223372036.854775808, 9223372036.854775807]

Both have 9 points of decimal precision, the difference is in the integer portion, which we aren't using here.

I don't see this explicitly used as storage, though. The point is irrelevant unless we're trying to save space somewhere.

This helper function is used to form the inputs for all the curves via the curve helpers make_linear and make_reciprocal. These functions take FixedI64 so that is the reason for it being returned here.

notlesh · 2022-10-12T16:00:32Z

runtime/moonbase/src/governance/tracks.rs

+			.min_support
+			.info(decision_period_days, track_info.name);
+	}
+	assert!(false);


why does this need to panic?

It is annotated by #[should_panic]. Then I comment that line out and run it to get the curve evolution for all tracks (the curve evolution output does not print unless the test fails).

If anyone has suggestions on how to better organize this, let me know. I think it's fine for now.

notlesh · 2022-10-12T16:06:20Z

runtime/moonbase/src/governance/tracks.rs

+				_ => Err(()),
+			}
+		} else if let Ok(custom_origin) = origins::Origin::try_from(id.clone()) {
+			match custom_origin {


Should this have a default branch to induce an Err just to be safe?

(This refers to the match starting on line 248, nothing to do with line 245)

The rust compiler would complain if the match did not have a branch for all enum variants.

I don't even think we can add an additional branch without the compiler complaining (because it already has a branch for every variant of the origins::Origin enum)

notlesh · 2022-10-12T16:09:49Z

runtime/moonbase/src/lib.rs

-	type WeightInfo = pallet_democracy::weights::SubstrateWeight<Runtime>;
-	type MaxProposals = ConstU32<100>;
+	type ManagerOrigin = EnsureRoot<AccountId>;
+	type MaxSize = ConstU32<{ 4096 * 1024 }>;


Is this meant to allow for up to a runtime upgrade? Or something else?

...if so, it might be safer to reduce this if we don't intend to use it for a runtime upgrade any time soon.

This was copied from Kusama so need to look into how they got this number. Maybe we don't need it so high...

Is this meant to allow for up to a runtime upgrade? Or something else?

Preimage pallet is used for all governance that goes through pallet-referenda as well as whitelisted calls in the whitelist pallet.

notlesh

Approved with some dumb questions...

runtime/moonbase/src/governance/tracks.rs

* move democracy to old and councils to own file until decided how to migrate councils * fix * Gov2 (moonbeam-foundation#1839) * gov2 config init with whitelist origin configured with tech committee 2 of 3 * test * clean * fix and docs * fix * try fix * try fix * fix and clean * fix * fix test and add curve docs above curves * revert accidental change * give power to Treasurer and rename GeneralAdmin to IdentityAdmin * finish rename of general admin to identity admin as it only manages the identity registrar * accept suggestion to use SUPPLY FACTOR in Spender definition * use supply factor for all currency * fix import * Playing with tracks values * fix build and update tracks curve evolution gist * update track curve format * improve * Configure referendum tracks Co-authored-by: Crystalin <alan@purestake.com>

4meta5 added 2 commits September 29, 2022 20:34

move democracy to old and councils to own file until decided how to m…

83c1849

…igrate councils

fix

fc4a619

4meta5 mentioned this pull request Oct 10, 2022

runtime-1900 release #1858

Closed

24 tasks

4meta5 changed the title ~~Gov2: remove old governance config~~ Gov2: move old governance config Oct 10, 2022

Merge branch 'master' into amar-gov2-rm-old-gov-config

117c231

4meta5 marked this pull request as ready for review October 10, 2022 18:13

4meta5 mentioned this pull request Oct 10, 2022

Gov2 #1839

Merged

Gov2 (#1839)

9dbe397

* gov2 config init with whitelist origin configured with tech committee 2 of 3 * test * clean * fix and docs * fix * try fix * try fix * fix and clean * fix

4meta5 changed the title ~~Gov2: move old governance config~~ Gov2 Oct 11, 2022

4meta5 added A0-pleasereview Pull request needs code review. B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Oct 11, 2022

4meta5 added 4 commits October 11, 2022 12:56

fix test and add curve docs above curves

9366f44

revert accidental change

9cdac81

give power to Treasurer and rename GeneralAdmin to IdentityAdmin

4398c07

finish rename of general admin to identity admin as it only manages t…

add036e

…he identity registrar

notlesh reviewed Oct 11, 2022

View reviewed changes

runtime/moonbase/src/governance/origins.rs Outdated Show resolved Hide resolved

accept suggestion to use SUPPLY FACTOR in Spender definition

9523952

4meta5 commented Oct 11, 2022

View reviewed changes

runtime/moonbase/src/governance/referenda.rs Outdated Show resolved Hide resolved

4meta5 commented Oct 11, 2022

View reviewed changes

runtime/moonbase/src/lib.rs Outdated Show resolved Hide resolved

4meta5 added 2 commits October 11, 2022 17:20

use supply factor for all currency

9f7a23c

fix import

149a3b2

notlesh reviewed Oct 12, 2022

View reviewed changes

girazoki approved these changes Oct 12, 2022

View reviewed changes

notlesh approved these changes Oct 12, 2022

View reviewed changes

Playing with tracks values

15de4a4

4meta5 added 3 commits October 13, 2022 10:51

fix build and update tracks curve evolution gist

2928fd5

update track curve format

dfbd066

improve

de07b35

4meta5 changed the title ~~Gov2~~ Gov2 (moonbase only) Oct 13, 2022

4meta5 commented Oct 14, 2022

View reviewed changes

runtime/moonbase/src/governance/tracks.rs Outdated Show resolved Hide resolved

Configure referendum tracks

ec96772

crystalin added A8-mergeoncegreen Pull request is reviewed well. and removed A0-pleasereview Pull request needs code review. labels Oct 18, 2022

crystalin merged commit 19e6628 into master Oct 18, 2022

crystalin deleted the amar-gov2-rm-old-gov-config branch October 18, 2022 01:08

crystalin changed the title ~~Gov2 (moonbase only)~~ Introduces Governance v2 in Moonbase Alpha Oct 20, 2022

notlesh 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 Dec 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduces Governance v2 in Moonbase Alpha #1838

Introduces Governance v2 in Moonbase Alpha #1838

4meta5 commented Sep 30, 2022 •

edited

Loading

notlesh Oct 12, 2022

4meta5 Oct 12, 2022

4meta5 Oct 12, 2022

notlesh Oct 12, 2022

notlesh Oct 12, 2022

4meta5 Oct 12, 2022

notlesh Oct 12, 2022

4meta5 Oct 12, 2022 •

edited

Loading

4meta5 Oct 12, 2022

notlesh Oct 12, 2022

notlesh Oct 12, 2022

4meta5 Oct 12, 2022 •

edited

Loading

notlesh Oct 12, 2022

notlesh Oct 12, 2022

4meta5 Oct 12, 2022

4meta5 Oct 12, 2022 •

edited

Loading

notlesh left a comment

Introduces Governance v2 in Moonbase Alpha #1838

Introduces Governance v2 in Moonbase Alpha #1838

Conversation

4meta5 commented Sep 30, 2022 • edited Loading

Tracks

Fast Track

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

4meta5 Oct 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

4meta5 Oct 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

4meta5 Oct 12, 2022 • edited Loading

Choose a reason for hiding this comment

notlesh left a comment

Choose a reason for hiding this comment

4meta5 commented Sep 30, 2022 •

edited

Loading

4meta5 Oct 12, 2022 •

edited

Loading

4meta5 Oct 12, 2022 •

edited

Loading

4meta5 Oct 12, 2022 •

edited

Loading