From 2f76d6e8673c5e37ee7a0aa3e12e9e8f70b404a6 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Wed, 22 Oct 2025 17:14:38 -0300 Subject: [PATCH 01/23] scaffold democracy pallet --- Cargo.lock | 20 +++++++++ pallets/democracy/Cargo.toml | 43 ++++++++++++++++++ pallets/democracy/src/lib.rs | 59 +++++++++++++++++++++++++ pallets/democracy/src/mock.rs | 79 ++++++++++++++++++++++++++++++++++ pallets/democracy/src/tests.rs | 10 +++++ 5 files changed, 211 insertions(+) create mode 100644 pallets/democracy/Cargo.toml create mode 100644 pallets/democracy/src/lib.rs create mode 100644 pallets/democracy/src/mock.rs create mode 100644 pallets/democracy/src/tests.rs diff --git a/Cargo.lock b/Cargo.lock index 78c1f9d102..5d2dc91fff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10692,6 +10692,26 @@ dependencies = [ "w3f-bls 0.1.3", ] +[[package]] +name = "pallet-subtensor-democracy" +version = "1.0.0" +dependencies = [ + "frame-support", + "frame-system", + "log", + "pallet-balances", + "pallet-preimage", + "pallet-scheduler", + "parity-scale-codec", + "polkadot-sdk-frame", + "scale-info", + "sp-core", + "sp-io", + "sp-runtime", + "sp-std", + "subtensor-macros", +] + [[package]] name = "pallet-subtensor-proxy" version = "40.1.0" diff --git a/pallets/democracy/Cargo.toml b/pallets/democracy/Cargo.toml new file mode 100644 index 0000000000..7a0779bec1 --- /dev/null +++ b/pallets/democracy/Cargo.toml @@ -0,0 +1,43 @@ +[package] +name = "pallet-subtensor-democracy" +version = "1.0.0" +authors = ["Bittensor Nucleus Team"] +edition.workspace = true +license = "Apache-2.0" +homepage = "https://bittensor.com" +description = "BitTensor democracy pallet" +readme = "README.md" + +[lints] +workspace = true + +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] + +[dependencies] +codec = { workspace = true, features = ["max-encoded-len"] } +frame = { workspace = true, features = ["runtime"] } +scale-info = { workspace = true, features = ["derive"] } +subtensor-macros.workspace = true +frame-support.workspace = true +frame-system.workspace = true +sp-runtime.workspace = true +sp-std.workspace = true +log.workspace = true + +[dev-dependencies] +pallet-balances = { workspace = true, default-features = true } +pallet-preimage = { workspace = true, default-features = true } +pallet-scheduler = { workspace = true, default-features = true } +sp-core = { workspace = true, default-features = true } +sp-io = { workspace = true, default-features = true } + +[features] +default = ["std"] +std = ["codec/std", "frame/std", "scale-info/std"] +runtime-benchmarks = [ + "frame/runtime-benchmarks", +] +try-runtime = [ + "frame/try-runtime", +] diff --git a/pallets/democracy/src/lib.rs b/pallets/democracy/src/lib.rs new file mode 100644 index 0000000000..dd308f799e --- /dev/null +++ b/pallets/democracy/src/lib.rs @@ -0,0 +1,59 @@ +#![cfg_attr(not(feature = "std"), no_std)] + +extern crate alloc; + +use frame_support::{ + dispatch::GetDispatchInfo, + pallet_prelude::*, + sp_runtime::traits::Dispatchable, + traits::{IsSubType, fungible}, +}; + +mod mock; +mod tests; +pub use pallet::*; + +pub type CurrencyOf = ::Currency; + +pub type BalanceOf = + as fungible::Inspect<::AccountId>>::Balance; + +#[frame_support::pallet] +#[allow(clippy::expect_used)] +pub mod pallet { + use super::*; + + const STORAGE_VERSION: StorageVersion = StorageVersion::new(0); + + #[pallet::pallet] + #[pallet::storage_version(STORAGE_VERSION)] + pub struct Pallet(_); + + #[pallet::config] + pub trait Config: frame_system::Config { + // /// The overarching call type. + type RuntimeCall: Parameter + + Dispatchable + + GetDispatchInfo + + From> + + IsSubType> + + IsType<::RuntimeCall>; + + /// The currency mechanism. + type Currency: fungible::Balanced + + fungible::Mutate; + } + + /// Accounts allowed to submit proposals. + #[pallet::storage] + pub type AllowedProposers = + StorageValue<_, BoundedVec>, ValueQuery>; + + // Active members of the triumvirate. + #[pallet::storage] + pub type Triumvirate = + StorageValue<_, BoundedVec>, ValueQuery>; + + #[pallet::call] + impl Pallet {} +} diff --git a/pallets/democracy/src/mock.rs b/pallets/democracy/src/mock.rs new file mode 100644 index 0000000000..3a8cccfd4e --- /dev/null +++ b/pallets/democracy/src/mock.rs @@ -0,0 +1,79 @@ +#![cfg(test)] +#![allow( + clippy::arithmetic_side_effects, + clippy::expect_used, + clippy::unwrap_used +)] +use frame_support::derive_impl; +use frame_system::pallet_prelude::BlockNumberFor; +use sp_core::U256; +use sp_runtime::{BuildStorage, traits::IdentityLookup}; + +use crate::{BalanceOf, pallet as pallet_democracy}; + +type Block = frame_system::mocking::MockBlock; +pub(crate) type AccountOf = ::AccountId; + +frame_support::construct_runtime!( + pub enum Test + { + System: frame_system = 1, + Balances: pallet_balances = 2, + Democracy: pallet_democracy = 3, + } +); + +#[derive_impl(frame_system::config_preludes::TestDefaultConfig)] +impl frame_system::Config for Test { + type Block = Block; + type AccountId = U256; + type AccountData = pallet_balances::AccountData; + type Lookup = IdentityLookup; +} + +#[derive_impl(pallet_balances::config_preludes::TestDefaultConfig)] +impl pallet_balances::Config for Test { + type AccountStore = System; +} + +impl pallet_democracy::Config for Test { + type RuntimeCall = RuntimeCall; + type Currency = Balances; +} + +pub(crate) struct TestState { + block_number: BlockNumberFor, + balances: Vec<(AccountOf, BalanceOf)>, +} + +impl Default for TestState { + fn default() -> Self { + Self { + block_number: 1, + balances: vec![], + } + } +} + +impl TestState { + pub(crate) fn build_and_execute(self, test: impl FnOnce()) { + let mut t = frame_system::GenesisConfig::::default() + .build_storage() + .unwrap(); + + pallet_balances::GenesisConfig:: { + balances: self + .balances + .iter() + .map(|(who, balance)| (*who, *balance)) + .collect::>(), + dev_accounts: None, + } + .assimilate_storage(&mut t) + .unwrap(); + + let mut ext = sp_io::TestExternalities::new(t); + ext.execute_with(|| System::set_block_number(self.block_number)); + ext.execute_with(test); + } +} diff --git a/pallets/democracy/src/tests.rs b/pallets/democracy/src/tests.rs new file mode 100644 index 0000000000..92ae4b0f5f --- /dev/null +++ b/pallets/democracy/src/tests.rs @@ -0,0 +1,10 @@ +#![cfg(test)] + +use crate::mock::*; + +#[test] +fn test_it_works() { + TestState::default().build_and_execute(|| { + assert!(true); + }); +} From 19c96e0527a134f2677c9e4a2b7de156277132c0 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Wed, 22 Oct 2025 20:16:18 -0300 Subject: [PATCH 02/23] added genesis config --- pallets/democracy/src/lib.rs | 78 +++++++++++++++++++++++++++++++++- pallets/democracy/src/mock.rs | 75 ++++++++++++++++++++++++-------- pallets/democracy/src/tests.rs | 58 +++++++++++++++++++++++-- 3 files changed, 188 insertions(+), 23 deletions(-) diff --git a/pallets/democracy/src/lib.rs b/pallets/democracy/src/lib.rs index dd308f799e..8e0364f9a5 100644 --- a/pallets/democracy/src/lib.rs +++ b/pallets/democracy/src/lib.rs @@ -42,18 +42,92 @@ pub mod pallet { /// The currency mechanism. type Currency: fungible::Balanced + fungible::Mutate; + + /// How many accounts allowed to submit proposals. + #[pallet::constant] + type MaxAllowedProposers: Get; } + const TRIUMVIRATE_SIZE: u32 = 3; + /// Accounts allowed to submit proposals. #[pallet::storage] pub type AllowedProposers = - StorageValue<_, BoundedVec>, ValueQuery>; + StorageValue<_, BoundedVec, ValueQuery>; // Active members of the triumvirate. #[pallet::storage] pub type Triumvirate = - StorageValue<_, BoundedVec>, ValueQuery>; + StorageValue<_, BoundedVec>, ValueQuery>; + + #[pallet::genesis_config] + #[derive(frame_support::DefaultNoBound)] + pub struct GenesisConfig { + pub allowed_proposers: Vec, + pub triumvirate: Vec, + } + + #[pallet::genesis_build] + impl BuildGenesisConfig for GenesisConfig { + fn build(&self) { + use alloc::collections::btree_set::BTreeSet; + + let allowed_proposers_set: BTreeSet<_> = self.allowed_proposers.iter().collect(); + assert_eq!( + allowed_proposers_set.len(), + self.allowed_proposers.len(), + "Allowed proposers cannot contain duplicate accounts." + ); + assert!( + self.allowed_proposers.len() <= T::MaxAllowedProposers::get() as usize, + "Allowed proposers length cannot exceed MaxAllowedProposers." + ); + + let triumvirate_set: BTreeSet<_> = self.triumvirate.iter().collect(); + assert_eq!( + triumvirate_set.len(), + self.triumvirate.len(), + "Triumvirate cannot contain duplicate accounts." + ); + assert!( + self.triumvirate.len() <= TRIUMVIRATE_SIZE as usize, + "Triumvirate length cannot exceed {TRIUMVIRATE_SIZE}." + ); + + assert!( + allowed_proposers_set.is_disjoint(&triumvirate_set), + "Allowed proposers and triumvirate must be disjoint." + ); + + Pallet::::initialize_allowed_proposers(&self.allowed_proposers); + Pallet::::initialize_triumvirate(&self.triumvirate); + } + } #[pallet::call] impl Pallet {} } + +impl Pallet { + fn initialize_allowed_proposers(allowed_proposers: &[T::AccountId]) { + if !allowed_proposers.is_empty() { + assert!( + AllowedProposers::::get().is_empty(), + "Allowed proposers are already initialized!" + ); + let mut allowed_proposers = BoundedVec::truncate_from(allowed_proposers.to_vec()); + allowed_proposers.sort(); + AllowedProposers::::put(allowed_proposers); + } + } + + fn initialize_triumvirate(triumvirate: &[T::AccountId]) { + assert!( + Triumvirate::::get().is_empty(), + "Triumvirate is already initialized!" + ); + let mut triumvirate = BoundedVec::truncate_from(triumvirate.to_vec()); + triumvirate.sort(); + Triumvirate::::put(triumvirate); + } +} diff --git a/pallets/democracy/src/mock.rs b/pallets/democracy/src/mock.rs index 3a8cccfd4e..eadaf68f89 100644 --- a/pallets/democracy/src/mock.rs +++ b/pallets/democracy/src/mock.rs @@ -4,8 +4,8 @@ clippy::expect_used, clippy::unwrap_used )] -use frame_support::derive_impl; -use frame_system::pallet_prelude::BlockNumberFor; +use frame_support::{derive_impl, parameter_types}; +use frame_system::pallet_prelude::*; use sp_core::U256; use sp_runtime::{BuildStorage, traits::IdentityLookup}; @@ -36,14 +36,21 @@ impl pallet_balances::Config for Test { type AccountStore = System; } +parameter_types! { + pub const MaxAllowedProposers: u32 = 5; +} + impl pallet_democracy::Config for Test { type RuntimeCall = RuntimeCall; type Currency = Balances; + type MaxAllowedProposers = MaxAllowedProposers; } pub(crate) struct TestState { block_number: BlockNumberFor, balances: Vec<(AccountOf, BalanceOf)>, + allowed_proposers: Vec>, + triumvirate: Vec>, } impl Default for TestState { @@ -51,29 +58,61 @@ impl Default for TestState { Self { block_number: 1, balances: vec![], + allowed_proposers: vec![U256::from(1), U256::from(2), U256::from(3)], + triumvirate: vec![U256::from(1001), U256::from(1002), U256::from(1003)], } } } impl TestState { - pub(crate) fn build_and_execute(self, test: impl FnOnce()) { - let mut t = frame_system::GenesisConfig::::default() - .build_storage() - .unwrap(); + pub(crate) fn with_block_number(mut self, block_number: BlockNumberFor) -> Self { + self.block_number = block_number; + self + } - pallet_balances::GenesisConfig:: { - balances: self - .balances - .iter() - .map(|(who, balance)| (*who, *balance)) - .collect::>(), - dev_accounts: None, - } - .assimilate_storage(&mut t) - .unwrap(); + pub(crate) fn with_balance( + mut self, + balances: Vec<(AccountOf, BalanceOf)>, + ) -> Self { + self.balances = balances; + self + } + + pub(crate) fn with_allowed_proposers( + mut self, + allowed_proposers: Vec>, + ) -> Self { + self.allowed_proposers = allowed_proposers; + self + } + + pub(crate) fn with_triumvirate(mut self, triumvirate: Vec>) -> Self { + self.triumvirate = triumvirate; + self + } - let mut ext = sp_io::TestExternalities::new(t); + pub(crate) fn build(self) -> sp_io::TestExternalities { + let mut ext: sp_io::TestExternalities = RuntimeGenesisConfig { + system: frame_system::GenesisConfig::default(), + balances: pallet_balances::GenesisConfig { + balances: self.balances, + ..Default::default() + }, + democracy: pallet_democracy::GenesisConfig { + allowed_proposers: self.allowed_proposers, + triumvirate: self.triumvirate, + }, + } + .build_storage() + .unwrap() + .into(); ext.execute_with(|| System::set_block_number(self.block_number)); - ext.execute_with(test); + ext + } + + pub(crate) fn build_and_execute(self, test: impl FnOnce()) { + self.build().execute_with(|| { + test(); + }); } } diff --git a/pallets/democracy/src/tests.rs b/pallets/democracy/src/tests.rs index 92ae4b0f5f..d8477bb6f2 100644 --- a/pallets/democracy/src/tests.rs +++ b/pallets/democracy/src/tests.rs @@ -1,10 +1,62 @@ #![cfg(test)] - +use super::*; use crate::mock::*; +use sp_core::U256; #[test] -fn test_it_works() { +fn environment_works() { TestState::default().build_and_execute(|| { - assert!(true); + assert_eq!( + AllowedProposers::::get(), + vec![U256::from(1), U256::from(2), U256::from(3)] + ); + assert_eq!( + Triumvirate::::get(), + vec![U256::from(1001), U256::from(1002), U256::from(1003)] + ); }); } + +#[test] +#[should_panic(expected = "Allowed proposers cannot contain duplicate accounts.")] +fn environment_with_duplicate_allowed_proposers_panics() { + TestState::default() + .with_allowed_proposers(vec![U256::from(1), U256::from(2), U256::from(2)]) + .build_and_execute(|| {}); +} + +#[test] +#[should_panic(expected = "Allowed proposers length cannot exceed MaxAllowedProposers.")] +fn environment_with_too_many_allowed_proposers_panics() { + let max_allowed_proposers = ::MaxAllowedProposers::get() as usize; + let allowed_proposers = (0..=max_allowed_proposers).map(|i| U256::from(i)).collect(); + TestState::default() + .with_allowed_proposers(allowed_proposers) + .build_and_execute(|| {}); +} + +#[test] +#[should_panic(expected = "Triumvirate cannot contain duplicate accounts.")] +fn environment_with_duplicate_triumvirate_panics() { + TestState::default() + .with_triumvirate(vec![U256::from(1001), U256::from(1002), U256::from(1002)]) + .build_and_execute(|| {}); +} + +#[test] +#[should_panic(expected = "Triumvirate length cannot exceed 3.")] +fn environment_with_too_many_triumvirate_panics() { + let triumvirate = (0..=3).map(|i| U256::from(i)).collect(); + TestState::default() + .with_triumvirate(triumvirate) + .build_and_execute(|| {}); +} + +#[test] +#[should_panic(expected = "Allowed proposers and triumvirate must be disjoint.")] +fn environment_with_overlapping_allowed_proposers_and_triumvirate_panics() { + TestState::default() + .with_allowed_proposers(vec![U256::from(1), U256::from(2), U256::from(3)]) + .with_triumvirate(vec![U256::from(1001), U256::from(1002), U256::from(1)]) + .build_and_execute(|| {}); +} From 98599ecb673872e51be604b6cb9036a05d971dc1 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Thu, 23 Oct 2025 19:45:29 -0300 Subject: [PATCH 03/23] added logic to set allowed proposers/triumvirate --- pallets/democracy/src/lib.rs | 139 ++++++++++++++++++++--- pallets/democracy/src/mock.rs | 21 ++-- pallets/democracy/src/tests.rs | 197 ++++++++++++++++++++++++++++++++- 3 files changed, 326 insertions(+), 31 deletions(-) diff --git a/pallets/democracy/src/lib.rs b/pallets/democracy/src/lib.rs index 8e0364f9a5..3e6def2d86 100644 --- a/pallets/democracy/src/lib.rs +++ b/pallets/democracy/src/lib.rs @@ -6,8 +6,10 @@ use frame_support::{ dispatch::GetDispatchInfo, pallet_prelude::*, sp_runtime::traits::Dispatchable, - traits::{IsSubType, fungible}, + traits::{ChangeMembers, IsSubType, fungible}, }; +use frame_system::pallet_prelude::*; +use sp_std::collections::btree_set::BTreeSet; mod mock; mod tests; @@ -43,6 +45,12 @@ pub mod pallet { type Currency: fungible::Balanced + fungible::Mutate; + /// Origin allowed to set allowed proposers. + type SetAllowedProposersOrigin: EnsureOrigin; + + /// Origin allowed to set triumvirate. + type SetTriumvirateOrigin: EnsureOrigin; + /// How many accounts allowed to submit proposals. #[pallet::constant] type MaxAllowedProposers: Get; @@ -70,25 +78,15 @@ pub mod pallet { #[pallet::genesis_build] impl BuildGenesisConfig for GenesisConfig { fn build(&self) { - use alloc::collections::btree_set::BTreeSet; - - let allowed_proposers_set: BTreeSet<_> = self.allowed_proposers.iter().collect(); - assert_eq!( - allowed_proposers_set.len(), - self.allowed_proposers.len(), - "Allowed proposers cannot contain duplicate accounts." - ); + let allowed_proposers_set = Pallet::::check_for_duplicates(&self.allowed_proposers) + .expect("Allowed proposers cannot contain duplicate accounts."); assert!( self.allowed_proposers.len() <= T::MaxAllowedProposers::get() as usize, "Allowed proposers length cannot exceed MaxAllowedProposers." ); - let triumvirate_set: BTreeSet<_> = self.triumvirate.iter().collect(); - assert_eq!( - triumvirate_set.len(), - self.triumvirate.len(), - "Triumvirate cannot contain duplicate accounts." - ); + let triumvirate_set = Pallet::::check_for_duplicates(&self.triumvirate) + .expect("Triumvirate cannot contain duplicate accounts."); assert!( self.triumvirate.len() <= TRIUMVIRATE_SIZE as usize, "Triumvirate length cannot exceed {TRIUMVIRATE_SIZE}." @@ -104,8 +102,108 @@ pub mod pallet { } } + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + pub enum Event { + TriumvirateSet, + AllowedProposersSet, + } + + #[pallet::error] + pub enum Error { + /// Duplicate accounts. + DuplicateAccounts, + /// New allowed proposers count cannot exceed MaxAllowedProposers. + TooManyAllowedProposers, + /// Triumvirate length cannot exceed 3. + InvalidTriumvirateLength, + /// Allowed proposers and triumvirate must be disjoint. + AllowedProposersAndTriumvirateMustBeDisjoint, + } + #[pallet::call] - impl Pallet {} + impl Pallet { + #[pallet::call_index(0)] + #[pallet::weight(Weight::zero())] + pub fn set_allowed_proposers( + origin: OriginFor, + mut new_allowed_proposers: BoundedVec, + ) -> DispatchResult { + T::SetAllowedProposersOrigin::ensure_origin(origin)?; + + // Check for duplicates. + let new_allowed_proposers_set = + Pallet::::check_for_duplicates(&new_allowed_proposers) + .ok_or(Error::::DuplicateAccounts)?; + + // Check for disjointness with the triumvirate. + let triumvirate = Triumvirate::::get(); + let triumvirate_set: BTreeSet<_> = triumvirate.iter().collect(); + ensure!( + triumvirate_set.is_disjoint(&new_allowed_proposers_set), + Error::::AllowedProposersAndTriumvirateMustBeDisjoint + ); + + // Sort members and get the outgoing ones. + let mut allowed_proposers = AllowedProposers::::get().to_vec(); + allowed_proposers.sort(); + new_allowed_proposers.sort(); + let (_incoming, _outgoing) = + <() as ChangeMembers>::compute_members_diff_sorted( + &allowed_proposers, + &new_allowed_proposers.to_vec(), + ); + + // TODO: Cleanup proposals/votes from the allowed proposers. + + AllowedProposers::::put(new_allowed_proposers); + + Self::deposit_event(Event::::AllowedProposersSet); + Ok(()) + } + + #[pallet::call_index(1)] + #[pallet::weight(Weight::zero())] + pub fn set_triumvirate( + origin: OriginFor, + mut new_triumvirate: BoundedVec>, + ) -> DispatchResult { + T::SetTriumvirateOrigin::ensure_origin(origin)?; + + // Check for duplicates and length. + let new_triumvirate_set = Pallet::::check_for_duplicates(&new_triumvirate) + .ok_or(Error::::DuplicateAccounts)?; + ensure!( + new_triumvirate.len() == TRIUMVIRATE_SIZE as usize, + Error::::InvalidTriumvirateLength + ); + + // Check for disjointness with the allowed proposers. + let allowed_proposers = AllowedProposers::::get(); + let allowed_proposers_set: BTreeSet<_> = allowed_proposers.iter().collect(); + ensure!( + allowed_proposers_set.is_disjoint(&new_triumvirate_set), + Error::::AllowedProposersAndTriumvirateMustBeDisjoint + ); + + // Sort members and get the outgoing ones. + let mut triumvirate = Triumvirate::::get().to_vec(); + triumvirate.sort(); + new_triumvirate.sort(); + let (_incoming, _outgoing) = + <() as ChangeMembers>::compute_members_diff_sorted( + &triumvirate, + &new_triumvirate.to_vec(), + ); + + // TODO: Cleanup proposals/votes from the triumvirate. + + Triumvirate::::put(new_triumvirate); + + Self::deposit_event(Event::::TriumvirateSet); + Ok(()) + } + } } impl Pallet { @@ -130,4 +228,13 @@ impl Pallet { triumvirate.sort(); Triumvirate::::put(triumvirate); } + + fn check_for_duplicates(accounts: &[T::AccountId]) -> Option> { + let accounts_set: BTreeSet<_> = accounts.iter().collect(); + if accounts_set.len() == accounts.len() { + Some(accounts_set) + } else { + None + } + } } diff --git a/pallets/democracy/src/mock.rs b/pallets/democracy/src/mock.rs index eadaf68f89..64af1ae9fe 100644 --- a/pallets/democracy/src/mock.rs +++ b/pallets/democracy/src/mock.rs @@ -5,7 +5,7 @@ clippy::unwrap_used )] use frame_support::{derive_impl, parameter_types}; -use frame_system::pallet_prelude::*; +use frame_system::{EnsureRoot, pallet_prelude::*}; use sp_core::U256; use sp_runtime::{BuildStorage, traits::IdentityLookup}; @@ -44,6 +44,8 @@ impl pallet_democracy::Config for Test { type RuntimeCall = RuntimeCall; type Currency = Balances; type MaxAllowedProposers = MaxAllowedProposers; + type SetAllowedProposersOrigin = EnsureRoot>; + type SetTriumvirateOrigin = EnsureRoot>; } pub(crate) struct TestState { @@ -65,19 +67,6 @@ impl Default for TestState { } impl TestState { - pub(crate) fn with_block_number(mut self, block_number: BlockNumberFor) -> Self { - self.block_number = block_number; - self - } - - pub(crate) fn with_balance( - mut self, - balances: Vec<(AccountOf, BalanceOf)>, - ) -> Self { - self.balances = balances; - self - } - pub(crate) fn with_allowed_proposers( mut self, allowed_proposers: Vec>, @@ -116,3 +105,7 @@ impl TestState { }); } } + +pub(crate) fn last_event() -> RuntimeEvent { + System::events().pop().expect("RuntimeEvent expected").event +} diff --git a/pallets/democracy/src/tests.rs b/pallets/democracy/src/tests.rs index d8477bb6f2..35de7331ef 100644 --- a/pallets/democracy/src/tests.rs +++ b/pallets/democracy/src/tests.rs @@ -1,7 +1,9 @@ #![cfg(test)] use super::*; use crate::mock::*; +use frame_support::{assert_noop, assert_ok}; use sp_core::U256; +use std::iter::repeat; #[test] fn environment_works() { @@ -17,6 +19,23 @@ fn environment_works() { }); } +#[test] +fn environment_members_are_sorted() { + TestState::default() + .with_allowed_proposers(vec![U256::from(2), U256::from(3), U256::from(1)]) + .with_triumvirate(vec![U256::from(1002), U256::from(1001), U256::from(1003)]) + .build_and_execute(|| { + assert_eq!( + AllowedProposers::::get(), + vec![U256::from(1), U256::from(2), U256::from(3)] + ); + assert_eq!( + Triumvirate::::get(), + vec![U256::from(1001), U256::from(1002), U256::from(1003)] + ); + }); +} + #[test] #[should_panic(expected = "Allowed proposers cannot contain duplicate accounts.")] fn environment_with_duplicate_allowed_proposers_panics() { @@ -46,7 +65,7 @@ fn environment_with_duplicate_triumvirate_panics() { #[test] #[should_panic(expected = "Triumvirate length cannot exceed 3.")] fn environment_with_too_many_triumvirate_panics() { - let triumvirate = (0..=3).map(|i| U256::from(i)).collect(); + let triumvirate = (1..=4).map(|i| U256::from(i)).collect(); TestState::default() .with_triumvirate(triumvirate) .build_and_execute(|| {}); @@ -60,3 +79,179 @@ fn environment_with_overlapping_allowed_proposers_and_triumvirate_panics() { .with_triumvirate(vec![U256::from(1001), U256::from(1002), U256::from(1)]) .build_and_execute(|| {}); } + +#[test] +fn set_allowed_proposers_works() { + TestState::default() + .with_allowed_proposers(vec![]) + .build_and_execute(|| { + let allowed_proposers = BoundedVec::truncate_from(vec![ + U256::from(5), + U256::from(1), + U256::from(4), + U256::from(3), + U256::from(2), + ]); + assert_eq!(AllowedProposers::::get(), vec![]); + + assert_ok!(Pallet::::set_allowed_proposers( + // SetAllowedProposersOrigin is EnsureRoot + RuntimeOrigin::root(), + allowed_proposers.clone() + )); + + assert_eq!( + AllowedProposers::::get().to_vec(), + // Sorted allowed proposers + vec![ + U256::from(1), + U256::from(2), + U256::from(3), + U256::from(4), + U256::from(5) + ] + ); + assert_eq!( + last_event(), + RuntimeEvent::Democracy(Event::::AllowedProposersSet) + ); + }); +} + +#[test] +fn set_allowed_proposers_with_bad_origin_fails() { + TestState::default() + .with_allowed_proposers(vec![]) + .build_and_execute(|| { + let allowed_proposers = + BoundedVec::truncate_from((1..=5).map(|i| U256::from(i)).collect::>()); + + assert_noop!( + Pallet::::set_allowed_proposers( + RuntimeOrigin::signed(U256::from(42)), + allowed_proposers.clone() + ), + DispatchError::BadOrigin + ); + + assert_noop!( + Pallet::::set_allowed_proposers(RuntimeOrigin::none(), allowed_proposers), + DispatchError::BadOrigin + ); + }); +} + +#[test] +fn set_allowed_proposers_with_duplicate_accounts_fails() { + TestState::default() + .with_allowed_proposers(vec![]) + .build_and_execute(|| { + let allowed_proposers = + BoundedVec::truncate_from(repeat(U256::from(1)).take(2).collect::>()); + + assert_noop!( + Pallet::::set_allowed_proposers(RuntimeOrigin::root(), allowed_proposers), + Error::::DuplicateAccounts + ); + }); +} + +#[test] +fn set_allowed_proposers_with_triumvirate_intersection_fails() { + TestState::default() + .with_allowed_proposers(vec![]) + .with_triumvirate(vec![U256::from(1), U256::from(2), U256::from(3)]) + .build_and_execute(|| { + let allowed_proposers = + BoundedVec::truncate_from((3..=8).map(|i| U256::from(i)).collect::>()); + + assert_noop!( + Pallet::::set_allowed_proposers(RuntimeOrigin::root(), allowed_proposers), + Error::::AllowedProposersAndTriumvirateMustBeDisjoint + ); + }); +} + +#[test] +fn set_triumvirate_works() { + TestState::default() + .with_triumvirate(vec![]) + .build_and_execute(|| { + let triumvirate = BoundedVec::truncate_from(vec![ + U256::from(1003), + U256::from(1001), + U256::from(1002), + ]); + assert_eq!(Triumvirate::::get(), vec![]); + + assert_ok!(Pallet::::set_triumvirate( + // SetTriumvirateOrigin is EnsureRoot + RuntimeOrigin::root(), + triumvirate.clone() + )); + + assert_eq!( + Triumvirate::::get(), + // Sorted triumvirate + vec![U256::from(1001), U256::from(1002), U256::from(1003)] + ); + assert_eq!( + last_event(), + RuntimeEvent::Democracy(Event::::TriumvirateSet) + ); + }); +} + +#[test] +fn set_triumvirate_with_bad_origin_fails() { + TestState::default() + .with_triumvirate(vec![]) + .build_and_execute(|| { + let triumvirate = BoundedVec::truncate_from( + (1..=3).map(|i| U256::from(1000 + i)).collect::>(), + ); + + assert_noop!( + Pallet::::set_triumvirate( + RuntimeOrigin::signed(U256::from(42)), + triumvirate.clone() + ), + DispatchError::BadOrigin + ); + + assert_noop!( + Pallet::::set_triumvirate(RuntimeOrigin::none(), triumvirate), + DispatchError::BadOrigin + ); + }); +} + +#[test] +fn set_triumvirate_with_duplicate_accounts_fails() { + TestState::default() + .with_triumvirate(vec![]) + .build_and_execute(|| { + let triumvirate = + BoundedVec::truncate_from(repeat(U256::from(1001)).take(2).collect::>()); + + assert_noop!( + Pallet::::set_triumvirate(RuntimeOrigin::root(), triumvirate), + Error::::DuplicateAccounts + ); + }); +} + +#[test] +fn set_triumvirate_with_allowed_proposers_intersection_fails() { + TestState::default() + .with_allowed_proposers(vec![U256::from(1), U256::from(2), U256::from(3)]) + .build_and_execute(|| { + let triumvirate = + BoundedVec::truncate_from((3..=8).map(|i| U256::from(i)).collect::>()); + + assert_noop!( + Pallet::::set_triumvirate(RuntimeOrigin::root(), triumvirate), + Error::::AllowedProposersAndTriumvirateMustBeDisjoint + ); + }); +} From 05bd1ef18d7121b4b3d06ba094dc60e5ccc79f57 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Fri, 24 Oct 2025 09:06:29 -0300 Subject: [PATCH 04/23] rename democracy to governance --- Cargo.lock | 40 +++++++++---------- pallets/{democracy => governance}/Cargo.toml | 4 +- pallets/{democracy => governance}/src/lib.rs | 0 pallets/{democracy => governance}/src/mock.rs | 8 ++-- .../{democracy => governance}/src/tests.rs | 4 +- 5 files changed, 28 insertions(+), 28 deletions(-) rename pallets/{democracy => governance}/Cargo.toml (93%) rename pallets/{democracy => governance}/src/lib.rs (100%) rename pallets/{democracy => governance}/src/mock.rs (93%) rename pallets/{democracy => governance}/src/tests.rs (98%) diff --git a/Cargo.lock b/Cargo.lock index 5d2dc91fff..563bbac7f7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9738,6 +9738,26 @@ dependencies = [ "sp-runtime", ] +[[package]] +name = "pallet-governance" +version = "1.0.0" +dependencies = [ + "frame-support", + "frame-system", + "log", + "pallet-balances", + "pallet-preimage", + "pallet-scheduler", + "parity-scale-codec", + "polkadot-sdk-frame", + "scale-info", + "sp-core", + "sp-io", + "sp-runtime", + "sp-std", + "subtensor-macros", +] + [[package]] name = "pallet-grandpa" version = "41.0.0" @@ -10692,26 +10712,6 @@ dependencies = [ "w3f-bls 0.1.3", ] -[[package]] -name = "pallet-subtensor-democracy" -version = "1.0.0" -dependencies = [ - "frame-support", - "frame-system", - "log", - "pallet-balances", - "pallet-preimage", - "pallet-scheduler", - "parity-scale-codec", - "polkadot-sdk-frame", - "scale-info", - "sp-core", - "sp-io", - "sp-runtime", - "sp-std", - "subtensor-macros", -] - [[package]] name = "pallet-subtensor-proxy" version = "40.1.0" diff --git a/pallets/democracy/Cargo.toml b/pallets/governance/Cargo.toml similarity index 93% rename from pallets/democracy/Cargo.toml rename to pallets/governance/Cargo.toml index 7a0779bec1..feba335447 100644 --- a/pallets/democracy/Cargo.toml +++ b/pallets/governance/Cargo.toml @@ -1,11 +1,11 @@ [package] -name = "pallet-subtensor-democracy" +name = "pallet-governance" version = "1.0.0" authors = ["Bittensor Nucleus Team"] edition.workspace = true license = "Apache-2.0" homepage = "https://bittensor.com" -description = "BitTensor democracy pallet" +description = "BitTensor governance pallet" readme = "README.md" [lints] diff --git a/pallets/democracy/src/lib.rs b/pallets/governance/src/lib.rs similarity index 100% rename from pallets/democracy/src/lib.rs rename to pallets/governance/src/lib.rs diff --git a/pallets/democracy/src/mock.rs b/pallets/governance/src/mock.rs similarity index 93% rename from pallets/democracy/src/mock.rs rename to pallets/governance/src/mock.rs index 64af1ae9fe..76f40d6212 100644 --- a/pallets/democracy/src/mock.rs +++ b/pallets/governance/src/mock.rs @@ -9,7 +9,7 @@ use frame_system::{EnsureRoot, pallet_prelude::*}; use sp_core::U256; use sp_runtime::{BuildStorage, traits::IdentityLookup}; -use crate::{BalanceOf, pallet as pallet_democracy}; +use crate::{BalanceOf, pallet as pallet_governance}; type Block = frame_system::mocking::MockBlock; pub(crate) type AccountOf = ::AccountId; @@ -19,7 +19,7 @@ frame_support::construct_runtime!( { System: frame_system = 1, Balances: pallet_balances = 2, - Democracy: pallet_democracy = 3, + Governance: pallet_governance = 3, } ); @@ -40,7 +40,7 @@ parameter_types! { pub const MaxAllowedProposers: u32 = 5; } -impl pallet_democracy::Config for Test { +impl pallet_governance::Config for Test { type RuntimeCall = RuntimeCall; type Currency = Balances; type MaxAllowedProposers = MaxAllowedProposers; @@ -87,7 +87,7 @@ impl TestState { balances: self.balances, ..Default::default() }, - democracy: pallet_democracy::GenesisConfig { + governance: pallet_governance::GenesisConfig { allowed_proposers: self.allowed_proposers, triumvirate: self.triumvirate, }, diff --git a/pallets/democracy/src/tests.rs b/pallets/governance/src/tests.rs similarity index 98% rename from pallets/democracy/src/tests.rs rename to pallets/governance/src/tests.rs index 35de7331ef..f5bfec553d 100644 --- a/pallets/democracy/src/tests.rs +++ b/pallets/governance/src/tests.rs @@ -113,7 +113,7 @@ fn set_allowed_proposers_works() { ); assert_eq!( last_event(), - RuntimeEvent::Democracy(Event::::AllowedProposersSet) + RuntimeEvent::Governance(Event::::AllowedProposersSet) ); }); } @@ -197,7 +197,7 @@ fn set_triumvirate_works() { ); assert_eq!( last_event(), - RuntimeEvent::Democracy(Event::::TriumvirateSet) + RuntimeEvent::Governance(Event::::TriumvirateSet) ); }); } From a5186cdd4618337f734d2e4d346c4be1f17dccae Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Fri, 24 Oct 2025 11:45:52 -0300 Subject: [PATCH 05/23] added logic to create proposals --- pallets/governance/src/lib.rs | 106 +++++++++++++-- pallets/governance/src/mock.rs | 45 ++++++- pallets/governance/src/tests.rs | 225 ++++++++++++++++++++++++++++++++ 3 files changed, 362 insertions(+), 14 deletions(-) diff --git a/pallets/governance/src/lib.rs b/pallets/governance/src/lib.rs index 3e6def2d86..679a0d6237 100644 --- a/pallets/governance/src/lib.rs +++ b/pallets/governance/src/lib.rs @@ -6,9 +6,10 @@ use frame_support::{ dispatch::GetDispatchInfo, pallet_prelude::*, sp_runtime::traits::Dispatchable, - traits::{ChangeMembers, IsSubType, fungible}, + traits::{Bounded, ChangeMembers, IsSubType, QueryPreimage, StorePreimage, fungible}, }; use frame_system::pallet_prelude::*; +use sp_runtime::{Saturating, traits::Hash}; use sp_std::collections::btree_set::BTreeSet; mod mock; @@ -20,6 +21,9 @@ pub type CurrencyOf = ::Currency; pub type BalanceOf = as fungible::Inspect<::AccountId>>::Balance; +pub type BoundedCallOf = + Bounded<::RuntimeCall, ::Hashing>; + #[frame_support::pallet] #[allow(clippy::expect_used)] pub mod pallet { @@ -45,6 +49,9 @@ pub mod pallet { type Currency: fungible::Balanced + fungible::Mutate; + /// The preimage provider which will be used to store the call to dispatch. + type Preimages: QueryPreimage + StorePreimage; + /// Origin allowed to set allowed proposers. type SetAllowedProposersOrigin: EnsureOrigin; @@ -54,6 +61,14 @@ pub mod pallet { /// How many accounts allowed to submit proposals. #[pallet::constant] type MaxAllowedProposers: Get; + + /// Maximum weight for a proposal. + #[pallet::constant] + type MaxProposalWeight: Get; + + /// Maximum number of proposals allowed to be active in parallel. + #[pallet::constant] + type MaxProposals: Get; } const TRIUMVIRATE_SIZE: u32 = 3; @@ -63,11 +78,24 @@ pub mod pallet { pub type AllowedProposers = StorageValue<_, BoundedVec, ValueQuery>; - // Active members of the triumvirate. + /// Active members of the triumvirate. #[pallet::storage] pub type Triumvirate = StorageValue<_, BoundedVec>, ValueQuery>; + #[pallet::storage] + pub type ProposalCount = StorageValue<_, u32, ValueQuery>; + + /// The hashes of the active proposals. + #[pallet::storage] + pub type Proposals = + StorageValue<_, BoundedVec, ValueQuery>; + + /// Actual proposal for a given hash. + #[pallet::storage] + pub type ProposalOf = + StorageMap<_, Identity, T::Hash, BoundedCallOf, OptionQuery>; + #[pallet::genesis_config] #[derive(frame_support::DefaultNoBound)] pub struct GenesisConfig { @@ -104,21 +132,36 @@ pub mod pallet { #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] - pub enum Event { + pub enum Event { TriumvirateSet, AllowedProposersSet, + Proposed { + account: T::AccountId, + proposal_index: u32, + proposal_hash: T::Hash, + }, } #[pallet::error] pub enum Error { - /// Duplicate accounts. + /// Duplicate accounts not allowed. DuplicateAccounts, - /// New allowed proposers count cannot exceed MaxAllowedProposers. + /// There can only be a maximum of `MaxAllowedProposers` allowed proposers. TooManyAllowedProposers, /// Triumvirate length cannot exceed 3. InvalidTriumvirateLength, /// Allowed proposers and triumvirate must be disjoint. AllowedProposersAndTriumvirateMustBeDisjoint, + /// Origin is not an allowed proposer. + NotAllowedProposer, + /// The given weight bound for the proposal was too low. + WrongProposalLength, + /// The given weight bound for the proposal was too low. + WrongProposalWeight, + /// Duplicate proposals not allowed. + DuplicateProposal, + /// There can only be a maximum of `MaxProposals` active proposals in parallel. + TooManyProposals, } #[pallet::call] @@ -131,12 +174,10 @@ pub mod pallet { ) -> DispatchResult { T::SetAllowedProposersOrigin::ensure_origin(origin)?; - // Check for duplicates. let new_allowed_proposers_set = Pallet::::check_for_duplicates(&new_allowed_proposers) .ok_or(Error::::DuplicateAccounts)?; - // Check for disjointness with the triumvirate. let triumvirate = Triumvirate::::get(); let triumvirate_set: BTreeSet<_> = triumvirate.iter().collect(); ensure!( @@ -144,7 +185,6 @@ pub mod pallet { Error::::AllowedProposersAndTriumvirateMustBeDisjoint ); - // Sort members and get the outgoing ones. let mut allowed_proposers = AllowedProposers::::get().to_vec(); allowed_proposers.sort(); new_allowed_proposers.sort(); @@ -170,7 +210,6 @@ pub mod pallet { ) -> DispatchResult { T::SetTriumvirateOrigin::ensure_origin(origin)?; - // Check for duplicates and length. let new_triumvirate_set = Pallet::::check_for_duplicates(&new_triumvirate) .ok_or(Error::::DuplicateAccounts)?; ensure!( @@ -178,7 +217,6 @@ pub mod pallet { Error::::InvalidTriumvirateLength ); - // Check for disjointness with the allowed proposers. let allowed_proposers = AllowedProposers::::get(); let allowed_proposers_set: BTreeSet<_> = allowed_proposers.iter().collect(); ensure!( @@ -186,7 +224,6 @@ pub mod pallet { Error::::AllowedProposersAndTriumvirateMustBeDisjoint ); - // Sort members and get the outgoing ones. let mut triumvirate = Triumvirate::::get().to_vec(); triumvirate.sort(); new_triumvirate.sort(); @@ -203,6 +240,53 @@ pub mod pallet { Self::deposit_event(Event::::TriumvirateSet); Ok(()) } + + #[pallet::call_index(2)] + #[pallet::weight(Weight::zero())] + pub fn propose( + origin: OriginFor, + proposal: Box<::RuntimeCall>, + #[pallet::compact] length_bound: u32, + ) -> DispatchResult { + let who = ensure_signed(origin)?; + let allowed_proposers = AllowedProposers::::get(); + ensure!( + allowed_proposers.contains(&who), + Error::::NotAllowedProposer + ); + + let proposal_len = proposal.encoded_size(); + ensure!( + proposal_len <= length_bound as usize, + Error::::WrongProposalLength + ); + let proposal_weight = proposal.get_dispatch_info().call_weight; + ensure!( + proposal_weight.all_lte(T::MaxProposalWeight::get()), + Error::::WrongProposalWeight + ); + + let proposal_hash = T::Hashing::hash_of(&proposal); + ensure!( + !ProposalOf::::contains_key(proposal_hash), + Error::::DuplicateProposal + ); + + Proposals::::try_append(proposal_hash).map_err(|_| Error::::TooManyProposals)?; + + let proposal_index = ProposalCount::::get(); + ProposalCount::::mutate(|i| i.saturating_inc()); + + let bounded_proposal = T::Preimages::bound(*proposal)?; + ProposalOf::::insert(proposal_hash, bounded_proposal); + + Self::deposit_event(Event::::Proposed { + account: who, + proposal_index, + proposal_hash, + }); + Ok(()) + } } } diff --git a/pallets/governance/src/mock.rs b/pallets/governance/src/mock.rs index 76f40d6212..8fa9d5947e 100644 --- a/pallets/governance/src/mock.rs +++ b/pallets/governance/src/mock.rs @@ -4,7 +4,7 @@ clippy::expect_used, clippy::unwrap_used )] -use frame_support::{derive_impl, parameter_types}; +use frame_support::{derive_impl, pallet_prelude::*, parameter_types}; use frame_system::{EnsureRoot, pallet_prelude::*}; use sp_core::U256; use sp_runtime::{BuildStorage, traits::IdentityLookup}; @@ -19,7 +19,9 @@ frame_support::construct_runtime!( { System: frame_system = 1, Balances: pallet_balances = 2, - Governance: pallet_governance = 3, + Preimage: pallet_preimage = 3, + Governance: pallet_governance = 4, + TestPallet: pallet_test = 5, } ); @@ -36,18 +38,55 @@ impl pallet_balances::Config for Test { type AccountStore = System; } +impl pallet_preimage::Config for Test { + type WeightInfo = pallet_preimage::weights::SubstrateWeight; + type RuntimeEvent = RuntimeEvent; + type Currency = Balances; + type ManagerOrigin = EnsureRoot>; + type Consideration = (); +} + parameter_types! { pub const MaxAllowedProposers: u32 = 5; + pub const MaxProposalWeight: Weight = Weight::from_parts(1_000_000_000_000, 0); + pub const MaxProposals: u32 = 5; } impl pallet_governance::Config for Test { type RuntimeCall = RuntimeCall; type Currency = Balances; - type MaxAllowedProposers = MaxAllowedProposers; + type Preimages = Preimage; type SetAllowedProposersOrigin = EnsureRoot>; type SetTriumvirateOrigin = EnsureRoot>; + type MaxAllowedProposers = MaxAllowedProposers; + type MaxProposalWeight = MaxProposalWeight; + type MaxProposals = MaxProposals; } +#[frame_support::pallet] +pub(crate) mod pallet_test { + use super::MaxProposalWeight; + use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::*; + + #[pallet::pallet] + pub struct Pallet(_); + + #[pallet::config] + pub trait Config: frame_system::Config + Sized {} + + #[pallet::call] + impl Pallet { + #[pallet::call_index(0)] + #[pallet::weight(MaxProposalWeight::get() * 2)] + pub fn expensive_call(_origin: OriginFor) -> DispatchResult { + Ok(()) + } + } +} + +impl pallet_test::Config for Test {} + pub(crate) struct TestState { block_number: BlockNumberFor, balances: Vec<(AccountOf, BalanceOf)>, diff --git a/pallets/governance/src/tests.rs b/pallets/governance/src/tests.rs index f5bfec553d..6fcec2c187 100644 --- a/pallets/governance/src/tests.rs +++ b/pallets/governance/src/tests.rs @@ -255,3 +255,228 @@ fn set_triumvirate_with_allowed_proposers_intersection_fails() { ); }); } + +#[test] +fn propose_works_with_inline_preimage() { + TestState::default().build_and_execute(|| { + let key_value = (b"Foobar".to_vec(), 42u32.to_be_bytes().to_vec()); + let proposal = Box::new(RuntimeCall::System( + frame_system::Call::::set_storage { + items: vec![key_value], + }, + )); + let length_bound = proposal.encoded_size() as u32; + + assert_ok!(Pallet::::propose( + RuntimeOrigin::signed(U256::from(1)), + proposal.clone(), + length_bound + )); + + let proposal_hash = ::Hashing::hash_of(&proposal); + let bounded_proposal = ::Preimages::bound(*proposal).unwrap(); + assert_eq!(Proposals::::get(), vec![proposal_hash]); + assert_eq!(ProposalCount::::get(), 1); + assert_eq!( + ProposalOf::::get(proposal_hash), + Some(bounded_proposal) + ); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::Proposed { + account: U256::from(1), + proposal_index: 0, + proposal_hash, + }) + ); + }); +} + +#[test] +fn propose_works_with_lookup_preimage() { + TestState::default().build_and_execute(|| { + let key_value = (b"Foobar".to_vec(), 42u32.to_be_bytes().to_vec()); + let proposal = Box::new(RuntimeCall::System( + frame_system::Call::::set_storage { + // We deliberately create a large proposal to avoid inlining. + items: repeat(key_value).take(50).collect::>(), + }, + )); + let length_bound = proposal.encoded_size() as u32; + println!("length_bound: {}", length_bound); + + assert_ok!(Pallet::::propose( + RuntimeOrigin::signed(U256::from(1)), + proposal.clone(), + length_bound + )); + + let proposal_hash = ::Hashing::hash_of(&proposal); + assert_eq!(Proposals::::get(), vec![proposal_hash]); + assert_eq!(ProposalCount::::get(), 1); + let stored_proposals = ProposalOf::::iter().collect::>(); + assert_eq!(stored_proposals.len(), 1); + let (stored_hash, bounded_proposal) = &stored_proposals[0]; + assert_eq!(stored_hash, &proposal_hash); + assert!(::Preimages::have(&bounded_proposal)); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::Proposed { + account: U256::from(1), + proposal_index: 0, + proposal_hash, + }) + ); + }); +} + +#[test] +fn propose_with_bad_origin_fails() { + TestState::default().build_and_execute(|| { + let proposal = Box::new(RuntimeCall::System( + frame_system::Call::::set_storage { + items: vec![(b"Foobar".to_vec(), 42u32.to_be_bytes().to_vec())], + }, + )); + let length_bound = proposal.encoded_size() as u32; + + assert_noop!( + Pallet::::propose(RuntimeOrigin::root(), proposal.clone(), length_bound), + DispatchError::BadOrigin + ); + + assert_noop!( + Pallet::::propose(RuntimeOrigin::none(), proposal.clone(), length_bound), + DispatchError::BadOrigin + ); + }); +} + +#[test] +fn propose_with_non_allowed_proposer_fails() { + TestState::default().build_and_execute(|| { + let proposal = Box::new(RuntimeCall::System( + frame_system::Call::::set_storage { + items: vec![(b"Foobar".to_vec(), 42u32.to_be_bytes().to_vec())], + }, + )); + let length_bound = proposal.encoded_size() as u32; + + assert_noop!( + Pallet::::propose( + RuntimeOrigin::signed(U256::from(42)), + proposal.clone(), + length_bound + ), + Error::::NotAllowedProposer + ); + }); +} + +#[test] +fn propose_with_incorrect_length_bound_fails() { + TestState::default().build_and_execute(|| { + let proposal = Box::new(RuntimeCall::System( + frame_system::Call::::set_storage { + items: vec![(b"Foobar".to_vec(), 42u32.to_be_bytes().to_vec())], + }, + )); + // We deliberately set the length bound to be one less than the proposal length. + let length_bound = proposal.encoded_size() as u32 - 1; + + assert_noop!( + Pallet::::propose( + RuntimeOrigin::signed(U256::from(1)), + proposal.clone(), + length_bound + ), + Error::::WrongProposalLength + ); + }); +} + +#[test] +fn propose_with_incorrect_weight_bound_fails() { + TestState::default().build_and_execute(|| { + let proposal = Box::new(RuntimeCall::TestPallet( + pallet_test::Call::::expensive_call {}, + )); + let length_bound = proposal.encoded_size() as u32; + + assert_noop!( + Pallet::::propose( + RuntimeOrigin::signed(U256::from(1)), + proposal.clone(), + length_bound + ), + Error::::WrongProposalWeight + ); + }); +} + +#[test] +fn propose_with_duplicate_proposal_fails() { + TestState::default().build_and_execute(|| { + let proposal = Box::new(RuntimeCall::System( + frame_system::Call::::set_storage { + items: vec![(b"Foobar".to_vec(), 42u32.to_be_bytes().to_vec())], + }, + )); + let length_bound = proposal.encoded_size() as u32; + + assert_ok!(Pallet::::propose( + RuntimeOrigin::signed(U256::from(1)), + proposal.clone(), + length_bound + )); + + assert_noop!( + Pallet::::propose( + RuntimeOrigin::signed(U256::from(1)), + proposal.clone(), + length_bound + ), + Error::::DuplicateProposal + ); + }); +} + +#[test] +fn propose_with_too_many_proposals_fails() { + TestState::default().build_and_execute(|| { + // Create the maximum number of proposals. + let proposals = (1..=MaxProposals::get()) + .map(|i| { + let proposal = Box::new(RuntimeCall::System( + frame_system::Call::::set_storage { + items: vec![( + format!("Foobar{}", i).as_bytes().to_vec(), + 42u32.to_be_bytes().to_vec(), + )], + }, + )); + let length_bound = proposal.encoded_size() as u32; + (proposal, length_bound) + }) + .collect::>(); + + for (proposal, length_bound) in proposals { + assert_ok!(Pallet::::propose( + RuntimeOrigin::signed(U256::from(1)), + proposal, + length_bound + )); + } + + let proposal = Box::new(RuntimeCall::System( + frame_system::Call::::set_storage { + items: vec![(b"Foobar".to_vec(), 42u32.to_be_bytes().to_vec())], + }, + )); + let length_bound = proposal.encoded_size() as u32; + assert_noop!( + Pallet::::propose(RuntimeOrigin::signed(U256::from(1)), proposal, length_bound), + Error::::TooManyProposals + ); + }); +} From e8d56ae588ee216a69bb1b9dc84f2b7f06919ceb Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Mon, 27 Oct 2025 11:37:54 -0300 Subject: [PATCH 06/23] added logic to vote on proposals + remove unused param type --- pallets/admin-utils/src/tests/mock.rs | 1 - pallets/governance/src/lib.rs | 232 +++++++++++++- pallets/governance/src/mock.rs | 39 ++- pallets/governance/src/tests.rs | 371 +++++++++++++++++++++- pallets/subtensor/src/tests/mock.rs | 1 - pallets/transaction-fee/src/tests/mock.rs | 1 - runtime/src/lib.rs | 1 - 7 files changed, 627 insertions(+), 19 deletions(-) diff --git a/pallets/admin-utils/src/tests/mock.rs b/pallets/admin-utils/src/tests/mock.rs index ed62be55d8..4ee2497f2d 100644 --- a/pallets/admin-utils/src/tests/mock.rs +++ b/pallets/admin-utils/src/tests/mock.rs @@ -381,7 +381,6 @@ parameter_types! { pub MaximumSchedulerWeight: Weight = Perbill::from_percent(80) * BlockWeights::get().max_block; pub const MaxScheduledPerBlock: u32 = 50; - pub const NoPreimagePostponement: Option = Some(10); } impl pallet_scheduler::Config for Test { diff --git a/pallets/governance/src/lib.rs b/pallets/governance/src/lib.rs index 679a0d6237..308ab4f0f8 100644 --- a/pallets/governance/src/lib.rs +++ b/pallets/governance/src/lib.rs @@ -3,29 +3,55 @@ extern crate alloc; use frame_support::{ - dispatch::GetDispatchInfo, + dispatch::{GetDispatchInfo, RawOrigin}, pallet_prelude::*, sp_runtime::traits::Dispatchable, - traits::{Bounded, ChangeMembers, IsSubType, QueryPreimage, StorePreimage, fungible}, + traits::{ + Bounded, ChangeMembers, IsSubType, QueryPreimage, StorePreimage, fungible, + schedule::{DispatchTime, Priority, v3::Named as ScheduleNamed}, + }, }; use frame_system::pallet_prelude::*; use sp_runtime::{Saturating, traits::Hash}; -use sp_std::collections::btree_set::BTreeSet; +use sp_std::{collections::btree_set::BTreeSet, vec::Vec}; mod mock; mod tests; pub use pallet::*; +const TRIUMVIRATE_SIZE: u32 = 3; + pub type CurrencyOf = ::Currency; pub type BalanceOf = as fungible::Inspect<::AccountId>>::Balance; -pub type BoundedCallOf = - Bounded<::RuntimeCall, ::Hashing>; +pub type LocalCallOf = ::RuntimeCall; + +pub type BoundedCallOf = Bounded, ::Hashing>; + +pub type PalletsOriginOf = + <::RuntimeOrigin as OriginTrait>::PalletsOrigin; + +pub type ScheduleAddressOf = + , LocalCallOf, PalletsOriginOf>>::Address; + +/// Simple index type for proposal counting. +pub type ProposalIndex = u32; + +#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)] +pub struct Votes { + /// The proposal's unique index. + index: ProposalIndex, + /// The set of triumvirate members that approved it. + ayes: BoundedVec>, + /// The set of triumvirate members that rejected it. + nays: BoundedVec>, + /// The hard end time of this vote. + end: BlockNumber, +} #[frame_support::pallet] -#[allow(clippy::expect_used)] pub mod pallet { use super::*; @@ -52,6 +78,14 @@ pub mod pallet { /// The preimage provider which will be used to store the call to dispatch. type Preimages: QueryPreimage + StorePreimage; + /// The scheduler which will be used to schedule the proposal for execution. + type Scheduler: ScheduleNamed< + BlockNumberFor, + LocalCallOf, + PalletsOriginOf, + Hasher = Self::Hashing, + >; + /// Origin allowed to set allowed proposers. type SetAllowedProposersOrigin: EnsureOrigin; @@ -69,9 +103,15 @@ pub mod pallet { /// Maximum number of proposals allowed to be active in parallel. #[pallet::constant] type MaxProposals: Get; - } - const TRIUMVIRATE_SIZE: u32 = 3; + /// Maximum number of proposals that can be scheduled for execution in parallel. + #[pallet::constant] + type MaxScheduled: Get; + + /// The duration of a motion. + #[pallet::constant] + type MotionDuration: Get>; + } /// Accounts allowed to submit proposals. #[pallet::storage] @@ -86,16 +126,26 @@ pub mod pallet { #[pallet::storage] pub type ProposalCount = StorageValue<_, u32, ValueQuery>; - /// The hashes of the active proposals. + /// The hashes of the active proposals being voted on. #[pallet::storage] pub type Proposals = StorageValue<_, BoundedVec, ValueQuery>; + /// The hashes of the proposals that have been scheduled for execution. + #[pallet::storage] + pub type Scheduled = + StorageValue<_, BoundedVec, ValueQuery>; + /// Actual proposal for a given hash. #[pallet::storage] pub type ProposalOf = StorageMap<_, Identity, T::Hash, BoundedCallOf, OptionQuery>; + /// Votes for a given proposal, if it is ongoing. + #[pallet::storage] + pub type Voting = + StorageMap<_, Identity, T::Hash, Votes>, OptionQuery>; + #[pallet::genesis_config] #[derive(frame_support::DefaultNoBound)] pub struct GenesisConfig { @@ -139,6 +189,20 @@ pub mod pallet { account: T::AccountId, proposal_index: u32, proposal_hash: T::Hash, + end: BlockNumberFor, + }, + Voted { + account: T::AccountId, + proposal_hash: T::Hash, + voted: bool, + yes: u32, + no: u32, + }, + Scheduled { + proposal_hash: T::Hash, + }, + Cancelled { + proposal_hash: T::Hash, }, } @@ -162,6 +226,22 @@ pub mod pallet { DuplicateProposal, /// There can only be a maximum of `MaxProposals` active proposals in parallel. TooManyProposals, + /// Origin is not a triumvirate member. + NotTriumvirateMember, + /// Proposal must exist. + ProposalMissing, + /// Mismatched index. + WrongProposalIndex, + /// Duplicate vote not allowed. + DuplicateVote, + /// There can only be a maximum of `MaxScheduled` proposals scheduled for execution. + TooManyScheduled, + /// There can only be a maximum of 3 votes for a proposal. + TooManyVotes, + /// Call is not available in the preimage storage. + CallUnavailable, + /// Proposal hash is not 32 bytes. + InvalidProposalHashLength, } #[pallet::call] @@ -280,13 +360,64 @@ pub mod pallet { let bounded_proposal = T::Preimages::bound(*proposal)?; ProposalOf::::insert(proposal_hash, bounded_proposal); + let now = frame_system::Pallet::::block_number(); + let end = now + T::MotionDuration::get(); + Voting::::insert( + proposal_hash, + Votes { + index: proposal_index, + ayes: BoundedVec::new(), + nays: BoundedVec::new(), + end, + }, + ); + Self::deposit_event(Event::::Proposed { account: who, proposal_index, proposal_hash, + end, }); Ok(()) } + + #[pallet::call_index(3)] + #[pallet::weight(Weight::zero())] + pub fn vote( + origin: OriginFor, + proposal_hash: T::Hash, + #[pallet::compact] proposal_index: ProposalIndex, + approve: bool, + ) -> DispatchResult { + let who = ensure_signed(origin)?; + let triumvirate = Triumvirate::::get(); + ensure!(triumvirate.contains(&who), Error::::NotTriumvirateMember); + + let proposals = Proposals::::get(); + ensure!(proposals.contains(&proposal_hash), Error::::ProposalMissing); + + Self::do_vote(&who, proposal_hash, proposal_index, approve)?; + + let voting = Voting::::get(proposal_hash).ok_or(Error::::ProposalMissing)?; + let yes_votes = voting.ayes.len() as u32; + let no_votes = voting.nays.len() as u32; + + if yes_votes >= 2 { + Self::do_schedule(proposal_hash)?; + } else if no_votes >= 2 { + Self::do_cancel(proposal_hash)?; + } else { + Self::deposit_event(Event::::Voted { + account: who, + proposal_hash, + voted: approve, + yes: yes_votes, + no: no_votes, + }); + } + + Ok(()) + } } } @@ -321,4 +452,87 @@ impl Pallet { None } } + + fn do_vote( + who: &T::AccountId, + proposal_hash: T::Hash, + index: ProposalIndex, + approve: bool, + ) -> DispatchResult { + Voting::::try_mutate(proposal_hash, |voting| -> DispatchResult { + let voting = voting.as_mut().ok_or(Error::::ProposalMissing)?; + ensure!(voting.index == index, Error::::WrongProposalIndex); + + let has_yes_vote = voting.ayes.iter().any(|a| a == who); + let has_no_vote = voting.nays.iter().any(|a| a == who); + + if approve { + if !has_yes_vote { + voting + .ayes + .try_push(who.clone()) + // Unreachable because nobody can double vote. + .map_err(|_| Error::::TooManyVotes)?; + } else { + return Err(Error::::DuplicateVote.into()); + } + if has_no_vote { + voting.nays.retain(|a| a != who); + } + } else { + if !has_no_vote { + voting + .nays + .try_push(who.clone()) + // Unreachable because nobody can double vote. + .map_err(|_| Error::::TooManyVotes)?; + } else { + return Err(Error::::DuplicateVote.into()); + } + if has_yes_vote { + voting.ayes.retain(|a| a != who); + } + } + + Ok(()) + }) + } + + fn do_schedule(proposal_hash: T::Hash) -> DispatchResult { + Proposals::::mutate(|proposals| { + proposals.retain(|h| h != &proposal_hash); + }); + Scheduled::::try_append(proposal_hash).map_err(|_| Error::::TooManyScheduled)?; + + let bounded = ProposalOf::::get(proposal_hash).ok_or(Error::::ProposalMissing)?; + ensure!(T::Preimages::have(&bounded), Error::::CallUnavailable); + + let now = frame_system::Pallet::::block_number(); + T::Scheduler::schedule_named( + proposal_hash + .as_ref() + .try_into() + // Unreachable because we expect the hash to be 32 bytes. + .map_err(|_| Error::::InvalidProposalHashLength)?, + DispatchTime::At(now + T::MotionDuration::get()), + None, + Priority::default(), + RawOrigin::Root.into(), + bounded, + )?; + + Self::deposit_event(Event::::Scheduled { proposal_hash }); + Ok(()) + } + + fn do_cancel(proposal_hash: T::Hash) -> DispatchResult { + Proposals::::mutate(|proposals| { + proposals.retain(|h| h != &proposal_hash); + }); + ProposalOf::::remove(&proposal_hash); + Voting::::remove(&proposal_hash); + + Self::deposit_event(Event::::Cancelled { proposal_hash }); + Ok(()) + } } diff --git a/pallets/governance/src/mock.rs b/pallets/governance/src/mock.rs index 8fa9d5947e..fc4157cb18 100644 --- a/pallets/governance/src/mock.rs +++ b/pallets/governance/src/mock.rs @@ -4,10 +4,10 @@ clippy::expect_used, clippy::unwrap_used )] -use frame_support::{derive_impl, pallet_prelude::*, parameter_types}; -use frame_system::{EnsureRoot, pallet_prelude::*}; +use frame_support::{derive_impl, pallet_prelude::*, parameter_types, traits::EqualPrivilegeOnly}; +use frame_system::{EnsureRoot, limits, pallet_prelude::*}; use sp_core::U256; -use sp_runtime::{BuildStorage, traits::IdentityLookup}; +use sp_runtime::{BuildStorage, Perbill, traits::IdentityLookup}; use crate::{BalanceOf, pallet as pallet_governance}; @@ -20,8 +20,9 @@ frame_support::construct_runtime!( System: frame_system = 1, Balances: pallet_balances = 2, Preimage: pallet_preimage = 3, - Governance: pallet_governance = 4, - TestPallet: pallet_test = 5, + Scheduler: pallet_scheduler = 4, + Governance: pallet_governance = 5, + TestPallet: pallet_test = 6, } ); @@ -46,21 +47,49 @@ impl pallet_preimage::Config for Test { type Consideration = (); } +parameter_types! { + pub BlockWeights: limits::BlockWeights = limits::BlockWeights::with_sensible_defaults( + Weight::from_parts(2_000_000_000_000, u64::MAX), + Perbill::from_percent(75), + ); + pub MaximumSchedulerWeight: Weight = Perbill::from_percent(80) * BlockWeights::get().max_block; + pub const MaxScheduledPerBlock: u32 = 50; +} + +impl pallet_scheduler::Config for Test { + type RuntimeOrigin = RuntimeOrigin; + type RuntimeEvent = RuntimeEvent; + type PalletsOrigin = OriginCaller; + type RuntimeCall = RuntimeCall; + type MaximumWeight = MaximumSchedulerWeight; + type ScheduleOrigin = EnsureRoot>; + type MaxScheduledPerBlock = MaxScheduledPerBlock; + type WeightInfo = pallet_scheduler::weights::SubstrateWeight; + type OriginPrivilegeCmp = EqualPrivilegeOnly; + type Preimages = Preimage; + type BlockNumberProvider = System; +} + parameter_types! { pub const MaxAllowedProposers: u32 = 5; pub const MaxProposalWeight: Weight = Weight::from_parts(1_000_000_000_000, 0); pub const MaxProposals: u32 = 5; + pub const MaxScheduled: u32 = 10; + pub const MotionDuration: BlockNumberFor = 20; } impl pallet_governance::Config for Test { type RuntimeCall = RuntimeCall; type Currency = Balances; type Preimages = Preimage; + type Scheduler = Scheduler; type SetAllowedProposersOrigin = EnsureRoot>; type SetTriumvirateOrigin = EnsureRoot>; type MaxAllowedProposers = MaxAllowedProposers; type MaxProposalWeight = MaxProposalWeight; type MaxProposals = MaxProposals; + type MaxScheduled = MaxScheduled; + type MotionDuration = MotionDuration; } #[frame_support::pallet] diff --git a/pallets/governance/src/tests.rs b/pallets/governance/src/tests.rs index 6fcec2c187..a95cd72d5d 100644 --- a/pallets/governance/src/tests.rs +++ b/pallets/governance/src/tests.rs @@ -267,6 +267,7 @@ fn propose_works_with_inline_preimage() { )); let length_bound = proposal.encoded_size() as u32; + let proposal_index = ProposalCount::::get(); assert_ok!(Pallet::::propose( RuntimeOrigin::signed(U256::from(1)), proposal.clone(), @@ -281,12 +282,23 @@ fn propose_works_with_inline_preimage() { ProposalOf::::get(proposal_hash), Some(bounded_proposal) ); + let now = frame_system::Pallet::::block_number(); + assert_eq!( + Voting::::get(proposal_hash), + Some(Votes { + index: proposal_index, + ayes: BoundedVec::new(), + nays: BoundedVec::new(), + end: now + MotionDuration::get(), + }) + ); assert_eq!( last_event(), RuntimeEvent::Governance(Event::::Proposed { account: U256::from(1), proposal_index: 0, proposal_hash, + end: now + MotionDuration::get(), }) ); }); @@ -303,8 +315,8 @@ fn propose_works_with_lookup_preimage() { }, )); let length_bound = proposal.encoded_size() as u32; - println!("length_bound: {}", length_bound); + let proposal_index = ProposalCount::::get(); assert_ok!(Pallet::::propose( RuntimeOrigin::signed(U256::from(1)), proposal.clone(), @@ -319,12 +331,23 @@ fn propose_works_with_lookup_preimage() { let (stored_hash, bounded_proposal) = &stored_proposals[0]; assert_eq!(stored_hash, &proposal_hash); assert!(::Preimages::have(&bounded_proposal)); + let now = frame_system::Pallet::::block_number(); + assert_eq!( + Voting::::get(proposal_hash), + Some(Votes { + index: proposal_index, + ayes: BoundedVec::new(), + nays: BoundedVec::new(), + end: now + MotionDuration::get(), + }) + ); assert_eq!( last_event(), RuntimeEvent::Governance(Event::::Proposed { account: U256::from(1), proposal_index: 0, proposal_hash, + end: now + MotionDuration::get(), }) ); }); @@ -480,3 +503,349 @@ fn propose_with_too_many_proposals_fails() { ); }); } + +#[test] +fn vote_aye_as_first_voter_works() { + TestState::default().build_and_execute(|| { + let (proposal_hash, proposal_index) = create_proposal(); + + let approve = true; + assert_ok!(Pallet::::vote( + RuntimeOrigin::signed(U256::from(1001)), + proposal_hash, + proposal_index, + approve + )); + + let votes = Voting::::get(proposal_hash).unwrap(); + assert_eq!(votes.ayes.to_vec(), vec![U256::from(1001)]); + assert_eq!(votes.nays.to_vec(), vec![]); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::Voted { + account: U256::from(1001), + proposal_hash, + voted: true, + yes: 1, + no: 0, + }) + ); + }); +} + +#[test] +fn vote_nay_as_first_voter_works() { + TestState::default().build_and_execute(|| { + let (proposal_hash, proposal_index) = create_proposal(); + + let approve = false; + assert_ok!(Pallet::::vote( + RuntimeOrigin::signed(U256::from(1001)), + proposal_hash, + proposal_index, + approve + )); + + let votes = Voting::::get(proposal_hash).unwrap(); + assert_eq!(votes.nays.to_vec(), vec![U256::from(1001)]); + assert_eq!(votes.ayes.to_vec(), vec![]); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::Voted { + account: U256::from(1001), + proposal_hash, + voted: false, + yes: 0, + no: 1, + }) + ); + }); +} + +#[test] +fn vote_can_be_updated() { + TestState::default().build_and_execute(|| { + let (proposal_hash, proposal_index) = create_proposal(); + + // Vote aye initially + assert_ok!(Pallet::::vote( + RuntimeOrigin::signed(U256::from(1001)), + proposal_hash, + proposal_index, + true + )); + let votes = Voting::::get(proposal_hash).unwrap(); + assert_eq!(votes.ayes.to_vec(), vec![U256::from(1001)]); + assert_eq!(votes.nays.to_vec(), vec![]); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::Voted { + account: U256::from(1001), + proposal_hash, + voted: true, + yes: 1, + no: 0, + }) + ); + + // Then vote nay, replacing the aye vote + assert_ok!(Pallet::::vote( + RuntimeOrigin::signed(U256::from(1001)), + proposal_hash, + proposal_index, + false + )); + let votes = Voting::::get(proposal_hash).unwrap(); + assert_eq!(votes.nays.to_vec(), vec![U256::from(1001)]); + assert_eq!(votes.ayes.to_vec(), vec![]); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::Voted { + account: U256::from(1001), + proposal_hash, + voted: false, + yes: 0, + no: 1, + }) + ); + + // Then vote aye again, replacing the nay vote + assert_ok!(Pallet::::vote( + RuntimeOrigin::signed(U256::from(1001)), + proposal_hash, + proposal_index, + true + )); + let votes = Voting::::get(proposal_hash).unwrap(); + assert_eq!(votes.ayes.to_vec(), vec![U256::from(1001)]); + assert_eq!(votes.nays.to_vec(), vec![]); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::Voted { + account: U256::from(1001), + proposal_hash, + voted: true, + yes: 1, + no: 0, + }) + ); + }); +} + +#[test] +fn two_aye_votes_schedule_proposal() { + TestState::default().build_and_execute(|| { + let (proposal_hash, proposal_index) = create_proposal(); + + vote_aye(U256::from(1001), proposal_hash, proposal_index); + vote_nay(U256::from(1002), proposal_hash, proposal_index); + vote_aye(U256::from(1003), proposal_hash, proposal_index); + + assert_eq!(Proposals::::get(), vec![]); + let votes = Voting::::get(proposal_hash).unwrap(); + assert_eq!( + votes.ayes.to_vec(), + vec![U256::from(1001), U256::from(1003)] + ); + assert_eq!(votes.nays.to_vec(), vec![U256::from(1002)]); + assert_eq!(Scheduled::::get(), vec![proposal_hash]); + let task_name: [u8; 32] = proposal_hash.as_ref().try_into().unwrap(); + let now = frame_system::Pallet::::block_number(); + assert_eq!( + pallet_scheduler::Lookup::::get(task_name).unwrap().0, + now + MotionDuration::get() + ); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::Scheduled { proposal_hash }) + ); + }); +} + +#[test] +fn two_nay_votes_cancel_proposal() { + TestState::default().build_and_execute(|| { + let (proposal_hash, proposal_index) = create_proposal(); + + vote_nay(U256::from(1001), proposal_hash, proposal_index); + vote_aye(U256::from(1002), proposal_hash, proposal_index); + vote_nay(U256::from(1003), proposal_hash, proposal_index); + + assert_eq!(Proposals::::get(), vec![]); + assert!(!Voting::::contains_key(proposal_hash)); + assert_eq!(Scheduled::::get(), vec![]); + assert_eq!(ProposalOf::::get(proposal_hash), None); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::Cancelled { proposal_hash }) + ); + }); +} + +#[test] +fn vote_as_bad_origin_fails() { + TestState::default().build_and_execute(|| { + let (proposal_hash, proposal_index) = create_proposal(); + + assert_noop!( + Pallet::::vote(RuntimeOrigin::root(), proposal_hash, proposal_index, true), + DispatchError::BadOrigin + ); + assert_noop!( + Pallet::::vote(RuntimeOrigin::none(), proposal_hash, proposal_index, true), + DispatchError::BadOrigin + ); + }); +} + +#[test] +fn vote_as_non_triumvirate_member_fails() { + TestState::default().build_and_execute(|| { + let (proposal_hash, proposal_index) = create_proposal(); + + assert_noop!( + Pallet::::vote( + RuntimeOrigin::signed(U256::from(42)), + proposal_hash, + proposal_index, + true + ), + Error::::NotTriumvirateMember + ); + }); +} + +#[test] +fn vote_on_missing_proposal_fails() { + TestState::default().build_and_execute(|| { + let invalid_proposal_hash = + ::Hashing::hash(b"Invalid proposal"); + assert_noop!( + Pallet::::vote( + RuntimeOrigin::signed(U256::from(1001)), + invalid_proposal_hash, + 0, + true + ), + Error::::ProposalMissing + ); + }); +} + +#[test] +fn vote_on_scheduled_proposal_fails() { + TestState::default().build_and_execute(|| { + let (proposal_hash, proposal_index) = create_proposal(); + + vote_aye(U256::from(1001), proposal_hash, proposal_index); + vote_aye(U256::from(1002), proposal_hash, proposal_index); + + assert_eq!(Proposals::::get(), vec![]); + assert_eq!(Scheduled::::get(), vec![proposal_hash]); + + assert_noop!( + Pallet::::vote( + RuntimeOrigin::signed(U256::from(1003)), + proposal_hash, + proposal_index, + true + ), + Error::::ProposalMissing + ); + }) +} + +#[test] +fn vote_on_proposal_with_wrong_index_fails() { + TestState::default().build_and_execute(|| { + let (proposal_hash, proposal_index) = create_proposal(); + + assert_noop!( + Pallet::::vote( + RuntimeOrigin::signed(U256::from(1001)), + proposal_hash, + proposal_index + 1, + true + ), + Error::::WrongProposalIndex + ); + }); +} + +#[test] +fn duplicate_vote_on_proposal_already_voted_fails() { + TestState::default().build_and_execute(|| { + let (proposal_hash, proposal_index) = create_proposal(); + + let aye_voter = RuntimeOrigin::signed(U256::from(1001)); + let approve = true; + assert_ok!(Pallet::::vote( + aye_voter.clone(), + proposal_hash, + proposal_index, + approve + )); + assert_noop!( + Pallet::::vote(aye_voter, proposal_hash, proposal_index, approve), + Error::::DuplicateVote + ); + + let nay_voter = RuntimeOrigin::signed(U256::from(1002)); + let approve = false; + assert_ok!(Pallet::::vote( + nay_voter.clone(), + proposal_hash, + proposal_index, + approve + )); + assert_noop!( + Pallet::::vote(nay_voter, proposal_hash, proposal_index, approve), + Error::::DuplicateVote + ); + }); +} + +fn create_proposal() -> (::Hash, u32) { + let proposal = Box::new(RuntimeCall::System( + frame_system::Call::::set_storage { + items: vec![(b"Foobar".to_vec(), 42u32.to_be_bytes().to_vec())], + }, + )); + let length_bound = proposal.encoded_size() as u32; + let proposal_hash = ::Hashing::hash_of(&proposal); + let proposal_index = ProposalCount::::get(); + + assert_ok!(Pallet::::propose( + RuntimeOrigin::signed(U256::from(1)), + proposal.clone(), + length_bound + )); + + (proposal_hash, proposal_index) +} + +fn vote_aye( + voter: U256, + proposal_hash: ::Hash, + proposal_index: u32, +) { + assert_ok!(Pallet::::vote( + RuntimeOrigin::signed(voter), + proposal_hash, + proposal_index, + true + )); +} + +fn vote_nay( + voter: U256, + proposal_hash: ::Hash, + proposal_index: u32, +) { + assert_ok!(Pallet::::vote( + RuntimeOrigin::signed(voter), + proposal_hash, + proposal_index, + false + )); +} diff --git a/pallets/subtensor/src/tests/mock.rs b/pallets/subtensor/src/tests/mock.rs index 73f8581d5e..9b0ffe10c3 100644 --- a/pallets/subtensor/src/tests/mock.rs +++ b/pallets/subtensor/src/tests/mock.rs @@ -341,7 +341,6 @@ parameter_types! { pub MaximumSchedulerWeight: Weight = Perbill::from_percent(80) * BlockWeights::get().max_block; pub const MaxScheduledPerBlock: u32 = 50; - pub const NoPreimagePostponement: Option = Some(10); } impl pallet_scheduler::Config for Test { diff --git a/pallets/transaction-fee/src/tests/mock.rs b/pallets/transaction-fee/src/tests/mock.rs index 8e48c2e4fc..df401aa930 100644 --- a/pallets/transaction-fee/src/tests/mock.rs +++ b/pallets/transaction-fee/src/tests/mock.rs @@ -427,7 +427,6 @@ parameter_types! { pub MaximumSchedulerWeight: Weight = Perbill::from_percent(80) * BlockWeights::get().max_block; pub const MaxScheduledPerBlock: u32 = 50; - pub const NoPreimagePostponement: Option = Some(10); } impl pallet_scheduler::Config for Test { diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 389a01a983..93ebedd273 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -793,7 +793,6 @@ parameter_types! { pub MaximumSchedulerWeight: Weight = Perbill::from_percent(80) * BlockWeights::get().max_block; pub const MaxScheduledPerBlock: u32 = 50; - pub const NoPreimagePostponement: Option = Some(10); } /// Used the compare the privilege of an origin inside the scheduler. From 97f481aedb0dfe2c69f29096f33bfbba82f075bc Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Wed, 29 Oct 2025 15:22:54 -0300 Subject: [PATCH 07/23] fix vote + emit 2 events voted and scheduled/cancelled --- pallets/governance/src/lib.rs | 25 ++++--- pallets/governance/src/mock.rs | 10 +++ pallets/governance/src/tests.rs | 122 ++++++++++++++++++++++++-------- 3 files changed, 118 insertions(+), 39 deletions(-) diff --git a/pallets/governance/src/lib.rs b/pallets/governance/src/lib.rs index 308ab4f0f8..f7a7472d87 100644 --- a/pallets/governance/src/lib.rs +++ b/pallets/governance/src/lib.rs @@ -242,6 +242,8 @@ pub mod pallet { CallUnavailable, /// Proposal hash is not 32 bytes. InvalidProposalHashLength, + /// Proposal is already scheduled. + AlreadyScheduled, } #[pallet::call] @@ -394,26 +396,29 @@ pub mod pallet { ensure!(triumvirate.contains(&who), Error::::NotTriumvirateMember); let proposals = Proposals::::get(); - ensure!(proposals.contains(&proposal_hash), Error::::ProposalMissing); - + ensure!( + proposals.contains(&proposal_hash), + Error::::ProposalMissing + ); + Self::do_vote(&who, proposal_hash, proposal_index, approve)?; let voting = Voting::::get(proposal_hash).ok_or(Error::::ProposalMissing)?; let yes_votes = voting.ayes.len() as u32; let no_votes = voting.nays.len() as u32; + Self::deposit_event(Event::::Voted { + account: who, + proposal_hash, + voted: approve, + yes: yes_votes, + no: no_votes, + }); + if yes_votes >= 2 { Self::do_schedule(proposal_hash)?; } else if no_votes >= 2 { Self::do_cancel(proposal_hash)?; - } else { - Self::deposit_event(Event::::Voted { - account: who, - proposal_hash, - voted: approve, - yes: yes_votes, - no: no_votes, - }); } Ok(()) diff --git a/pallets/governance/src/mock.rs b/pallets/governance/src/mock.rs index fc4157cb18..7719f8b342 100644 --- a/pallets/governance/src/mock.rs +++ b/pallets/governance/src/mock.rs @@ -177,3 +177,13 @@ impl TestState { pub(crate) fn last_event() -> RuntimeEvent { System::events().pop().expect("RuntimeEvent expected").event } + +pub(crate) fn last_n_events(n: usize) -> Vec { + System::events() + .into_iter() + .rev() + .take(n) + .rev() + .map(|e| e.event) + .collect() +} diff --git a/pallets/governance/src/tests.rs b/pallets/governance/src/tests.rs index a95cd72d5d..900d3d1934 100644 --- a/pallets/governance/src/tests.rs +++ b/pallets/governance/src/tests.rs @@ -464,6 +464,31 @@ fn propose_with_duplicate_proposal_fails() { }); } +#[test] +fn propose_with_already_scheduled_proposal_fails() { + TestState::default().build_and_execute(|| { + let (proposal_hash, proposal_index) = create_proposal(); + + vote_aye(U256::from(1001), proposal_hash, proposal_index); + vote_aye(U256::from(1002), proposal_hash, proposal_index); + + let proposal = Box::new(RuntimeCall::System( + frame_system::Call::::set_storage { + items: vec![(b"Foobar".to_vec(), 42u32.to_be_bytes().to_vec())], + }, + )); + let length_bound = proposal.encoded_size() as u32; + assert_noop!( + Pallet::::propose( + RuntimeOrigin::signed(U256::from(1)), + proposal.clone(), + length_bound + ), + Error::::DuplicateProposal + ); + }); +} + #[test] fn propose_with_too_many_proposals_fails() { TestState::default().build_and_execute(|| { @@ -568,12 +593,7 @@ fn vote_can_be_updated() { let (proposal_hash, proposal_index) = create_proposal(); // Vote aye initially - assert_ok!(Pallet::::vote( - RuntimeOrigin::signed(U256::from(1001)), - proposal_hash, - proposal_index, - true - )); + vote_aye(U256::from(1001), proposal_hash, proposal_index); let votes = Voting::::get(proposal_hash).unwrap(); assert_eq!(votes.ayes.to_vec(), vec![U256::from(1001)]); assert_eq!(votes.nays.to_vec(), vec![]); @@ -589,12 +609,7 @@ fn vote_can_be_updated() { ); // Then vote nay, replacing the aye vote - assert_ok!(Pallet::::vote( - RuntimeOrigin::signed(U256::from(1001)), - proposal_hash, - proposal_index, - false - )); + vote_nay(U256::from(1001), proposal_hash, proposal_index); let votes = Voting::::get(proposal_hash).unwrap(); assert_eq!(votes.nays.to_vec(), vec![U256::from(1001)]); assert_eq!(votes.ayes.to_vec(), vec![]); @@ -610,12 +625,7 @@ fn vote_can_be_updated() { ); // Then vote aye again, replacing the nay vote - assert_ok!(Pallet::::vote( - RuntimeOrigin::signed(U256::from(1001)), - proposal_hash, - proposal_index, - true - )); + vote_aye(U256::from(1001), proposal_hash, proposal_index); let votes = Voting::::get(proposal_hash).unwrap(); assert_eq!(votes.ayes.to_vec(), vec![U256::from(1001)]); assert_eq!(votes.nays.to_vec(), vec![]); @@ -640,7 +650,7 @@ fn two_aye_votes_schedule_proposal() { vote_aye(U256::from(1001), proposal_hash, proposal_index); vote_nay(U256::from(1002), proposal_hash, proposal_index); vote_aye(U256::from(1003), proposal_hash, proposal_index); - + assert_eq!(Proposals::::get(), vec![]); let votes = Voting::::get(proposal_hash).unwrap(); assert_eq!( @@ -655,8 +665,19 @@ fn two_aye_votes_schedule_proposal() { pallet_scheduler::Lookup::::get(task_name).unwrap().0, now + MotionDuration::get() ); + let events = last_n_events(3); assert_eq!( - last_event(), + events[0], + RuntimeEvent::Governance(Event::::Voted { + account: U256::from(1003), + proposal_hash, + voted: true, + yes: 2, + no: 1, + }) + ); + assert_eq!( + events[2], RuntimeEvent::Governance(Event::::Scheduled { proposal_hash }) ); }); @@ -675,8 +696,19 @@ fn two_nay_votes_cancel_proposal() { assert!(!Voting::::contains_key(proposal_hash)); assert_eq!(Scheduled::::get(), vec![]); assert_eq!(ProposalOf::::get(proposal_hash), None); + let events = last_n_events(2); assert_eq!( - last_event(), + events[0], + RuntimeEvent::Governance(Event::::Voted { + account: U256::from(1003), + proposal_hash, + voted: false, + yes: 1, + no: 2, + }) + ); + assert_eq!( + events[1], RuntimeEvent::Governance(Event::::Cancelled { proposal_hash }) ); }); @@ -736,10 +768,10 @@ fn vote_on_missing_proposal_fails() { fn vote_on_scheduled_proposal_fails() { TestState::default().build_and_execute(|| { let (proposal_hash, proposal_index) = create_proposal(); - + vote_aye(U256::from(1001), proposal_hash, proposal_index); vote_aye(U256::from(1002), proposal_hash, proposal_index); - + assert_eq!(Proposals::::get(), vec![]); assert_eq!(Scheduled::::get(), vec![proposal_hash]); @@ -805,12 +837,38 @@ fn duplicate_vote_on_proposal_already_voted_fails() { }); } -fn create_proposal() -> (::Hash, u32) { - let proposal = Box::new(RuntimeCall::System( - frame_system::Call::::set_storage { - items: vec![(b"Foobar".to_vec(), 42u32.to_be_bytes().to_vec())], - }, - )); +#[test] +fn aye_vote_on_proposal_with_too_many_scheduled_fails() { + TestState::default().build_and_execute(|| { + // We fill the scheduled proposals up to the maximum. + for i in 0..MaxScheduled::get() { + let (proposal_hash, proposal_index) = + create_custom_proposal(frame_system::Call::::set_storage { + items: vec![(b"Foobar".to_vec(), i.to_be_bytes().to_vec())], + }); + vote_aye(U256::from(1001), proposal_hash, proposal_index); + vote_aye(U256::from(1002), proposal_hash, proposal_index); + } + + let (proposal_hash, proposal_index) = create_proposal(); + + vote_aye(U256::from(1001), proposal_hash, proposal_index); + assert_noop!( + Pallet::::vote( + RuntimeOrigin::signed(U256::from(1002)), + proposal_hash, + proposal_index, + true + ), + Error::::TooManyScheduled + ); + }); +} + +fn create_custom_proposal( + call: impl Into>, +) -> (::Hash, u32) { + let proposal = Box::new(call.into()); let length_bound = proposal.encoded_size() as u32; let proposal_hash = ::Hashing::hash_of(&proposal); let proposal_index = ProposalCount::::get(); @@ -824,6 +882,12 @@ fn create_proposal() -> (::Hash, u32) { (proposal_hash, proposal_index) } +fn create_proposal() -> (::Hash, u32) { + create_custom_proposal(frame_system::Call::::set_storage { + items: vec![(b"Foobar".to_vec(), 42u32.to_be_bytes().to_vec())], + }) +} + fn vote_aye( voter: U256, proposal_hash: ::Hash, From a52430341476140955e92c5f7f84f87e08813344 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Wed, 29 Oct 2025 16:14:50 -0300 Subject: [PATCH 08/23] reorg storage items --- pallets/governance/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pallets/governance/src/lib.rs b/pallets/governance/src/lib.rs index f7a7472d87..91f0105197 100644 --- a/pallets/governance/src/lib.rs +++ b/pallets/governance/src/lib.rs @@ -131,11 +131,6 @@ pub mod pallet { pub type Proposals = StorageValue<_, BoundedVec, ValueQuery>; - /// The hashes of the proposals that have been scheduled for execution. - #[pallet::storage] - pub type Scheduled = - StorageValue<_, BoundedVec, ValueQuery>; - /// Actual proposal for a given hash. #[pallet::storage] pub type ProposalOf = @@ -146,6 +141,11 @@ pub mod pallet { pub type Voting = StorageMap<_, Identity, T::Hash, Votes>, OptionQuery>; + /// The hashes of the proposals that have been scheduled for execution. + #[pallet::storage] + pub type Scheduled = + StorageValue<_, BoundedVec, ValueQuery>; + #[pallet::genesis_config] #[derive(frame_support::DefaultNoBound)] pub struct GenesisConfig { From b5ef91b9f183eeee8da8e9b8db8b9cdc9febc4ef Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Wed, 29 Oct 2025 17:10:07 -0300 Subject: [PATCH 09/23] handle cleanup on schedule/cancel + store proposer --- pallets/governance/src/lib.rs | 30 +++++++++++++++++++----------- pallets/governance/src/tests.rs | 19 ++++++++++--------- 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/pallets/governance/src/lib.rs b/pallets/governance/src/lib.rs index 91f0105197..46bddb0bd3 100644 --- a/pallets/governance/src/lib.rs +++ b/pallets/governance/src/lib.rs @@ -126,10 +126,10 @@ pub mod pallet { #[pallet::storage] pub type ProposalCount = StorageValue<_, u32, ValueQuery>; - /// The hashes of the active proposals being voted on. + /// Tuples of account proposer and hash of the active proposals being voted on. #[pallet::storage] pub type Proposals = - StorageValue<_, BoundedVec, ValueQuery>; + StorageValue<_, BoundedVec<(T::AccountId, T::Hash), T::MaxProposals>, ValueQuery>; /// Actual proposal for a given hash. #[pallet::storage] @@ -353,8 +353,14 @@ pub mod pallet { !ProposalOf::::contains_key(proposal_hash), Error::::DuplicateProposal ); + let scheduled = Scheduled::::get(); + ensure!( + !scheduled.contains(&proposal_hash), + Error::::AlreadyScheduled + ); - Proposals::::try_append(proposal_hash).map_err(|_| Error::::TooManyProposals)?; + Proposals::::try_append((who.clone(), proposal_hash)) + .map_err(|_| Error::::TooManyProposals)?; let proposal_index = ProposalCount::::get(); ProposalCount::::mutate(|i| i.saturating_inc()); @@ -397,7 +403,7 @@ pub mod pallet { let proposals = Proposals::::get(); ensure!( - proposals.contains(&proposal_hash), + proposals.iter().any(|(_, h)| h == &proposal_hash), Error::::ProposalMissing ); @@ -504,9 +510,6 @@ impl Pallet { } fn do_schedule(proposal_hash: T::Hash) -> DispatchResult { - Proposals::::mutate(|proposals| { - proposals.retain(|h| h != &proposal_hash); - }); Scheduled::::try_append(proposal_hash).map_err(|_| Error::::TooManyScheduled)?; let bounded = ProposalOf::::get(proposal_hash).ok_or(Error::::ProposalMissing)?; @@ -526,18 +529,23 @@ impl Pallet { bounded, )?; + Self::clear_proposal(proposal_hash); + Self::deposit_event(Event::::Scheduled { proposal_hash }); Ok(()) } fn do_cancel(proposal_hash: T::Hash) -> DispatchResult { + Self::clear_proposal(proposal_hash); + Self::deposit_event(Event::::Cancelled { proposal_hash }); + Ok(()) + } + + fn clear_proposal(proposal_hash: T::Hash) { Proposals::::mutate(|proposals| { - proposals.retain(|h| h != &proposal_hash); + proposals.retain(|(_, h)| h != &proposal_hash); }); ProposalOf::::remove(&proposal_hash); Voting::::remove(&proposal_hash); - - Self::deposit_event(Event::::Cancelled { proposal_hash }); - Ok(()) } } diff --git a/pallets/governance/src/tests.rs b/pallets/governance/src/tests.rs index 900d3d1934..8e08e47741 100644 --- a/pallets/governance/src/tests.rs +++ b/pallets/governance/src/tests.rs @@ -276,7 +276,10 @@ fn propose_works_with_inline_preimage() { let proposal_hash = ::Hashing::hash_of(&proposal); let bounded_proposal = ::Preimages::bound(*proposal).unwrap(); - assert_eq!(Proposals::::get(), vec![proposal_hash]); + assert_eq!( + Proposals::::get(), + vec![(U256::from(1), proposal_hash)] + ); assert_eq!(ProposalCount::::get(), 1); assert_eq!( ProposalOf::::get(proposal_hash), @@ -324,7 +327,10 @@ fn propose_works_with_lookup_preimage() { )); let proposal_hash = ::Hashing::hash_of(&proposal); - assert_eq!(Proposals::::get(), vec![proposal_hash]); + assert_eq!( + Proposals::::get(), + vec![(U256::from(1), proposal_hash)] + ); assert_eq!(ProposalCount::::get(), 1); let stored_proposals = ProposalOf::::iter().collect::>(); assert_eq!(stored_proposals.len(), 1); @@ -484,7 +490,7 @@ fn propose_with_already_scheduled_proposal_fails() { proposal.clone(), length_bound ), - Error::::DuplicateProposal + Error::::AlreadyScheduled ); }); } @@ -652,12 +658,7 @@ fn two_aye_votes_schedule_proposal() { vote_aye(U256::from(1003), proposal_hash, proposal_index); assert_eq!(Proposals::::get(), vec![]); - let votes = Voting::::get(proposal_hash).unwrap(); - assert_eq!( - votes.ayes.to_vec(), - vec![U256::from(1001), U256::from(1003)] - ); - assert_eq!(votes.nays.to_vec(), vec![U256::from(1002)]); + assert!(!Voting::::contains_key(proposal_hash)); assert_eq!(Scheduled::::get(), vec![proposal_hash]); let task_name: [u8; 32] = proposal_hash.as_ref().try_into().unwrap(); let now = frame_system::Pallet::::block_number(); From a4ebc0a251b5388bd852e22013ff10dbe9d32b1a Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Thu, 30 Oct 2025 10:19:18 -0300 Subject: [PATCH 10/23] cleanup votes/proposals on triumvirate/proposers changes --- pallets/governance/src/lib.rs | 46 +++++++--- pallets/governance/src/tests.rs | 149 ++++++++++++++++++++++++++++++-- 2 files changed, 176 insertions(+), 19 deletions(-) diff --git a/pallets/governance/src/lib.rs b/pallets/governance/src/lib.rs index 46bddb0bd3..bcb9b5d93f 100644 --- a/pallets/governance/src/lib.rs +++ b/pallets/governance/src/lib.rs @@ -183,8 +183,15 @@ pub mod pallet { #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { - TriumvirateSet, - AllowedProposersSet, + AllowedProposersSet { + incoming: Vec, + outgoing: Vec, + removed_proposals: Vec<(T::AccountId, T::Hash)>, + }, + TriumvirateSet { + incoming: Vec, + outgoing: Vec, + }, Proposed { account: T::AccountId, proposal_index: u32, @@ -270,17 +277,28 @@ pub mod pallet { let mut allowed_proposers = AllowedProposers::::get().to_vec(); allowed_proposers.sort(); new_allowed_proposers.sort(); - let (_incoming, _outgoing) = + let (incoming, outgoing) = <() as ChangeMembers>::compute_members_diff_sorted( - &allowed_proposers, &new_allowed_proposers.to_vec(), + &allowed_proposers, ); - // TODO: Cleanup proposals/votes from the allowed proposers. + // Remove proposals from the outgoing allowed proposers. + let mut removed_proposals = vec![]; + for (proposer, proposal_hash) in Proposals::::get() { + if outgoing.contains(&proposer) { + Self::clear_proposal(proposal_hash); + removed_proposals.push((proposer, proposal_hash)); + } + } AllowedProposers::::put(new_allowed_proposers); - Self::deposit_event(Event::::AllowedProposersSet); + Self::deposit_event(Event::::AllowedProposersSet { + incoming, + outgoing, + removed_proposals, + }); Ok(()) } @@ -309,17 +327,25 @@ pub mod pallet { let mut triumvirate = Triumvirate::::get().to_vec(); triumvirate.sort(); new_triumvirate.sort(); - let (_incoming, _outgoing) = + let (incoming, outgoing) = <() as ChangeMembers>::compute_members_diff_sorted( - &triumvirate, &new_triumvirate.to_vec(), + &triumvirate, ); - // TODO: Cleanup proposals/votes from the triumvirate. + // Remove votes from the outgoing triumvirate members. + for (_proposer, proposal_hash) in Proposals::::get() { + Voting::::mutate(proposal_hash, |voting| { + if let Some(voting) = voting.as_mut() { + voting.ayes.retain(|a| !outgoing.contains(a)); + voting.nays.retain(|a| !outgoing.contains(a)); + } + }); + } Triumvirate::::put(new_triumvirate); - Self::deposit_event(Event::::TriumvirateSet); + Self::deposit_event(Event::::TriumvirateSet { incoming, outgoing }); Ok(()) } diff --git a/pallets/governance/src/tests.rs b/pallets/governance/src/tests.rs index 8e08e47741..4d618853d5 100644 --- a/pallets/governance/src/tests.rs +++ b/pallets/governance/src/tests.rs @@ -113,11 +113,73 @@ fn set_allowed_proposers_works() { ); assert_eq!( last_event(), - RuntimeEvent::Governance(Event::::AllowedProposersSet) + RuntimeEvent::Governance(Event::::AllowedProposersSet { + incoming: vec![ + U256::from(1), + U256::from(2), + U256::from(3), + U256::from(4), + U256::from(5) + ], + outgoing: vec![], + removed_proposals: vec![], + }) ); }); } +#[test] +fn set_allowed_proposers_removes_proposals_of_outgoing_proposers() { + TestState::default().build_and_execute(|| { + let (proposal_hash1, _proposal_index1) = create_custom_proposal( + U256::from(1), + frame_system::Call::::set_storage { + items: vec![(b"Foobar".to_vec(), 1i32.to_be_bytes().to_vec())], + }, + ); + let (proposal_hash2, _proposal_index2) = create_custom_proposal( + U256::from(1), + frame_system::Call::::set_storage { + items: vec![(b"Foobar".to_vec(), 2i32.to_be_bytes().to_vec())], + }, + ); + let (proposal_hash3, _proposal_index3) = create_custom_proposal( + U256::from(3), + frame_system::Call::::set_storage { + items: vec![(b"Foobar".to_vec(), 3i32.to_be_bytes().to_vec())], + }, + ); + assert_eq!( + AllowedProposers::::get(), + vec![U256::from(1), U256::from(2), U256::from(3)] + ); + + let allowed_proposers = + BoundedVec::truncate_from(vec![U256::from(2), U256::from(3), U256::from(4)]); + assert_ok!(Pallet::::set_allowed_proposers( + RuntimeOrigin::root(), + allowed_proposers.clone() + )); + + assert_eq!(AllowedProposers::::get(), allowed_proposers); + assert_eq!( + Proposals::::get(), + vec![(U256::from(3), proposal_hash3)] + ); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::AllowedProposersSet { + incoming: vec![U256::from(4)], + outgoing: vec![U256::from(1)], + removed_proposals: vec![ + (U256::from(1), proposal_hash1), + (U256::from(1), proposal_hash2) + ], + }) + ); + }); +} + #[test] fn set_allowed_proposers_with_bad_origin_fails() { TestState::default() @@ -197,11 +259,74 @@ fn set_triumvirate_works() { ); assert_eq!( last_event(), - RuntimeEvent::Governance(Event::::TriumvirateSet) + RuntimeEvent::Governance(Event::::TriumvirateSet { + incoming: vec![U256::from(1001), U256::from(1002), U256::from(1003)], + outgoing: vec![], + }) ); }); } +#[test] +fn set_triumvirate_removes_votes_of_outgoing_triumvirate_members() { + TestState::default().build_and_execute(|| { + let (proposal_hash1, proposal_index1) = create_custom_proposal( + U256::from(1), + frame_system::Call::::set_storage { + items: vec![(b"Foobar".to_vec(), 1i32.to_be_bytes().to_vec())], + }, + ); + let (proposal_hash2, proposal_index2) = create_custom_proposal( + U256::from(2), + frame_system::Call::::set_storage { + items: vec![(b"Foobar".to_vec(), 2i32.to_be_bytes().to_vec())], + }, + ); + let (proposal_hash3, proposal_index3) = create_custom_proposal( + U256::from(3), + frame_system::Call::::set_storage { + items: vec![(b"Foobar".to_vec(), 3i32.to_be_bytes().to_vec())], + }, + ); + assert_eq!( + Triumvirate::::get(), + vec![U256::from(1001), U256::from(1002), U256::from(1003)] + ); + + vote_aye(U256::from(1001), proposal_hash1, proposal_index1); + + vote_nay(U256::from(1002), proposal_hash2, proposal_index2); + vote_aye(U256::from(1003), proposal_hash2, proposal_index2); + + vote_nay(U256::from(1001), proposal_hash3, proposal_index3); + vote_aye(U256::from(1002), proposal_hash3, proposal_index3); + + let triumvirate = + BoundedVec::truncate_from(vec![U256::from(1001), U256::from(1003), U256::from(1004)]); + assert_ok!(Pallet::::set_triumvirate( + RuntimeOrigin::root(), + triumvirate.clone() + )); + assert_eq!(Triumvirate::::get(), triumvirate); + let voting1 = Voting::::get(proposal_hash1).unwrap(); + assert_eq!(voting1.ayes.to_vec(), vec![U256::from(1001)]); + assert_eq!(voting1.nays.to_vec(), vec![]); + let voting2 = Voting::::get(proposal_hash2).unwrap(); + assert_eq!(voting2.ayes.to_vec(), vec![U256::from(1003)]); + assert_eq!(voting2.nays.to_vec(), vec![]); + let voting3 = Voting::::get(proposal_hash3).unwrap(); + assert_eq!(voting3.ayes.to_vec(), vec![]); + assert_eq!(voting3.nays.to_vec(), vec![U256::from(1001)]); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::TriumvirateSet { + incoming: vec![U256::from(1004)], + outgoing: vec![U256::from(1002)], + }) + ); + }); +} + #[test] fn set_triumvirate_with_bad_origin_fails() { TestState::default() @@ -843,10 +968,12 @@ fn aye_vote_on_proposal_with_too_many_scheduled_fails() { TestState::default().build_and_execute(|| { // We fill the scheduled proposals up to the maximum. for i in 0..MaxScheduled::get() { - let (proposal_hash, proposal_index) = - create_custom_proposal(frame_system::Call::::set_storage { + let (proposal_hash, proposal_index) = create_custom_proposal( + U256::from(1), + frame_system::Call::::set_storage { items: vec![(b"Foobar".to_vec(), i.to_be_bytes().to_vec())], - }); + }, + ); vote_aye(U256::from(1001), proposal_hash, proposal_index); vote_aye(U256::from(1002), proposal_hash, proposal_index); } @@ -867,6 +994,7 @@ fn aye_vote_on_proposal_with_too_many_scheduled_fails() { } fn create_custom_proposal( + proposer: U256, call: impl Into>, ) -> (::Hash, u32) { let proposal = Box::new(call.into()); @@ -875,7 +1003,7 @@ fn create_custom_proposal( let proposal_index = ProposalCount::::get(); assert_ok!(Pallet::::propose( - RuntimeOrigin::signed(U256::from(1)), + RuntimeOrigin::signed(proposer), proposal.clone(), length_bound )); @@ -884,9 +1012,12 @@ fn create_custom_proposal( } fn create_proposal() -> (::Hash, u32) { - create_custom_proposal(frame_system::Call::::set_storage { - items: vec![(b"Foobar".to_vec(), 42u32.to_be_bytes().to_vec())], - }) + create_custom_proposal( + U256::from(1), + frame_system::Call::::set_storage { + items: vec![(b"Foobar".to_vec(), 42u32.to_be_bytes().to_vec())], + }, + ) } fn vote_aye( From 857ca731810fd3f5a405dddfdebe26f0d4243908 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Thu, 30 Oct 2025 11:39:26 -0300 Subject: [PATCH 11/23] add struct freeze --- pallets/governance/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pallets/governance/src/lib.rs b/pallets/governance/src/lib.rs index bcb9b5d93f..d59f09ed2e 100644 --- a/pallets/governance/src/lib.rs +++ b/pallets/governance/src/lib.rs @@ -14,6 +14,7 @@ use frame_support::{ use frame_system::pallet_prelude::*; use sp_runtime::{Saturating, traits::Hash}; use sp_std::{collections::btree_set::BTreeSet, vec::Vec}; +use subtensor_macros::freeze_struct; mod mock; mod tests; @@ -40,6 +41,7 @@ pub type ScheduleAddressOf = pub type ProposalIndex = u32; #[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)] +#[freeze_struct("4151e52425e670aa")] pub struct Votes { /// The proposal's unique index. index: ProposalIndex, From c9605d6be7b21a06469961ea22cc0b7d51794119 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Thu, 30 Oct 2025 17:37:09 -0300 Subject: [PATCH 12/23] added economic and building collective + basic rotation --- pallets/governance/README.md | 234 ++++++++++++++++++++++++ pallets/governance/src/lib.rs | 312 +++++++++++++++++++++++++++----- pallets/governance/src/mock.rs | 78 +++++++- pallets/governance/src/tests.rs | 92 ++++++++++ 4 files changed, 668 insertions(+), 48 deletions(-) create mode 100644 pallets/governance/README.md diff --git a/pallets/governance/README.md b/pallets/governance/README.md new file mode 100644 index 0000000000..81c29c2c9c --- /dev/null +++ b/pallets/governance/README.md @@ -0,0 +1,234 @@ +# On-Chain Governance System + +## Abstract + +This proposes a comprehensive on-chain governance system to replace the current broken governance implementation that relies on a sudo-based triumvirate multisig. The new system introduces a separation of powers model with three key components: (1) an Opentensor Foundation (OTF) account authorized to propose runtime upgrades, (2) a three-member Triumvirate that votes on proposals, and (3) two collective bodies (Economic Power and Building Power) that can delay or cancel proposals and replace Triumvirate members through a removal and appointment process. The system will be deployed in two phases: first coexisting with the current sudo implementation for validation, then fully replacing it. + +## Motivation + +The current governance system in Subtensor is broken and relies entirely on a triumvirate multisig with sudo privileges. The runtime contains dead code related to the original triumvirate collective and senate that no longer functions properly. This centralized approach creates several critical issues: + +1. **Single Point of Failure**: The sudo key represents a concentration of power with no on-chain checks or balances. +2. **Lack of Transparency**: Off-chain multisig decisions are not recorded or auditable on-chain. +3. **No Stakeholder Representation**: Major stakeholders (validators and subnet owners) have no formal mechanism to influence protocol upgrades. +4. **Technical Debt**: Dead governance code in the runtime creates maintenance burden and confusion. +5. **Trust Requirements**: The community must trust the multisig holders without cryptographic guarantees or accountability. + +This proposal addresses these issues by implementing a proper separation of powers that balances efficiency with stakeholder representation, while maintaining upgrade capability and security. + +## Specification + +### Overview + +The governance system consists of three main components working together: + +1. **Proposal Origin**: OTF-authorized account(s) +2. **Approval Body**: Triumvirate (3 members) +3. **Oversight Bodies**: Economic Power Collective (top 16 validators by total stake) and Building Power Collective (top 16 subnet owners by moving average price) + +### Actors and Roles + +#### Opentensor Foundation (OTF) Accounts + +- **Purpose**: Authorized to create runtime upgrade proposals +- **Assignment**: OTF account key(s) are configured in the runtime via governance +- **Permissions**: Can submit proposals to the main governance track +- **Constraints**: Cannot approve proposals; only the Triumvirate can approve + +**Open Questions:** +- Q1: How many OTF accounts should be authorized initially? Single account or multiple? **Multiple because safe, no power except to make proposal, one for Sam and one for other team member.** +- Q2: What happens if OTF account is compromised/lost? Can it be revoked immediately or requires full governance process? **Full governance process** +- Q3: Only one proposal active at a time? Or multiple? Different track for upgrade? **Multiple proposal at the same time but only one get through, other are cancelled** +- Q4: Who can add/remove OTF accounts? Only governance or should Triumvirate have emergency powers? +- Q5: What types of proposals can OTF submit? Only runtime upgrades or any root extrinsic? **All type of calls** +- Q6: Who validates that proposal code matches stated intent before Triumvirate votes? Share runtime WASM hash like Polkadot fellowship does? +- Q7: Would it make sense to have an extrinsic to kick the calling OTF key to avoid compromised key to submit proposals? + +#### Triumvirate + +- **Composition**: 3 distinct accounts/seats (must always maintain 3 members) +- **Role**: Vote on proposals submitted by OTF accounts +- **Voting Threshold**: 2-of-3 approval required for proposals to pass +- **Term**: Indefinite, subject to replacement by collective vote +- **Accountability**: Each seat can be replaced through collective vote process (see Replacement Mechanism) + +**Open Questions:** +- Q8: How are initial Triumvirate members selected? **Current triumvirate** +- Q9: When a member is being replaced, how is the new member selected? List of on-chain potential candidates? **Randomly from economic power collective or building power collective** +- Q10: Should Triumvirate members be known/doxxed or can they be anonymous? +- Q11: What happens if a Triumvirate member goes inactive for extended periods? **They need to accept the nomination or we rerun the nomination** +- Q12: Can Triumvirate members also be in collectives (conflict of interest)? +- Q13: What's the deadline for Triumvirate to vote? Can proposals expire? + +#### Economic Power Collective + +- **Composition**: Top 20 validators by total stake +- **Recalculation**: Membership refreshed every 2 months (432,000 blocks) +- **Powers**: + - Delay or cancel proposals approved by Triumvirate + - Replace one Triumvirate member every 6 months via single atomic vote (remove current holder + install replacement candidate, with rotating seat selection) + +**Open Questions:** +- Q14: "Total stake" - does this include delegated stake or only self-bonded? **Includes delegated stake** +- Q15: Should there be a minimum stake threshold to enter collective? **Given we select top N, should be enough to be an implicit minimum** +- Q16: What happens if validator drops out of top 20 mid-term? Immediate removal or wait for refresh? **Keep their spot until next refresh** +- Q18: Can a validator be in both Economic and Building collectives if they also own top subnet? **Yes, although imply a different key** + +#### Building Power Collective + +- **Composition**: Top 20 subnet owners by moving average (MA) price +- **Recalculation**: Membership refreshed every 2 months (432,000 blocks) +- **Powers**: + - Delay or cancel proposals approved by Triumvirate + - Replace one Triumvirate member every 6 months via single atomic vote (remove current holder + install replacement candidate, with rotating seat selection) + +**Open Questions:** +- Q19: What if subnet ownership transfers? Does collective seat transfer or recalculated when rotation happens? +- Q20: Should there be minimum subnet age requirement (prevent fresh subnets from voting)? **Maybe 3 or 4 months, or half a year, configurable** +- Q21: What if subnet is deregistered mid-term? Immediate collective removal? +- Q22: Can one entity own multiple subnets and occupy multiple collective seats? If not, how to prevent that? **Unique key only allowed on a collective** + +### Governance Process Flow + +#### Proposal Submission + +1. OTF account creates a proposal containing runtime upgrade or any root extrinsic +2. Proposal enters "Triumvirate Voting" phase +3. Voting period: 7 days (50,400 blocks) + +**Open Questions:** +- Q23: Can OTF cancel/withdraw a proposal after submission? What if they find a bug? +- Q24: Is there a queue limit? +- Q25: Who pays for proposal storage/execution? OTF, treasury, or included in proposal? + +#### Triumvirate Approval + +1. Triumvirate members cast votes (Aye/Nay) on the proposal +2. Requirement: At least 2 of 3 members must approve +3. If approved: Proposal enters "Delay Period" +4. If rejected: Proposal fails and is archived + +**Open Questions:** +- Q26: What happens if only 1 of 3 members votes within 7 days? Proposal cancels? +- Q27: Can Triumvirate members change their vote before voting period ends? +- Q28: Should there be a veto power for individual Triumvirate members for emergency stops? + +#### Delay Period (Collective Oversight) + +1. Initial Duration: 7 days (50,400 blocks) +2. Both collectives can vote to delay/cancel +3. Each collective member can cast a "Delay" vote +4. Delay votes accumulate with cumulative time delays: + - Vote 1: +12 hours (3,600 blocks at 12s/block) + - Vote 2: +1 day (7,200 blocks) + - Vote 3: +2 days (14,400 blocks) + - Vote 4: +4 days (28,800 blocks) + - Vote 5: +8 days (57,600 blocks) + - Vote 6: +16 days (115,200 blocks) + - Vote 7: +30 days (216,000 blocks) + - Vote 8: +60 days (432,000 blocks) +5. Cancellation threshold: If 9 delay votes are cast within a single collective +6. If cancelled: Proposal is terminated +7. If delay period expires without cancellation: Proposal executes automatically + +**Open Questions:** +- Q29: Are cumulative delays applied per-collective or across both collectives combined? +- Q30: Can collective members change their delay vote during the delay period? +- Q31: Should "Delay" votes require justification/reason on-chain? +- Q32: Can members vote "Support" (opposite of delay) to counter delay votes? +- Q33: Does EITHER collective reaching 9 votes cancel, or BOTH needed? + +#### Execution + +- Successful proposals execute automatically after the delay period +- Execution applies runtime upgrade or execute extrinsic +- Execution event is recorded on-chain + +**Open Questions:** +- Q34: What if execution fails due to runtime error? Who is responsible to fix? +- Q35: Can execution be delayed further if critical issue discovered on day 13? +- Q36: Should there be a final "confirm execution" step by OTF or Triumvirate? +- Q37: What if network is congested and execution can't fit in block? + +### Triumvirate Replacement Mechanism + +Each collective can replace one Triumvirate member every 6 months through a **single atomic vote**: the collective votes to replace the current seat holder with a specific new candidate. If the vote succeeds, the replacement happens immediately. The Triumvirate always maintains exactly 3 active members. + +#### Timing + +- Each collective can initiate replacement vote every 6 months (1,296,800 blocks) +- Economic and Building collectives have independent 6-month cycles +- Cooldown timer starts after vote completion (whether successful or failed) + +**Open Questions:** +- Q38: Does the 6-month timer start from genesis, from last replacement attempt, or last successful replacement? +- Q39: Can replacement be initiated early in emergency situations? +- Q40: Can a replaced member be voted back in immediately, or should there be a cooldown period? +- Q41: Should failed replacement attempts have a shorter cooldown (e.g., 1 month retry)? + +#### Rotating Seat Selection + +- Triumvirate seats are numbered: Seat 0, Seat 1, Seat 2 +- Each collective maintains an automatic rotation index +- Economic Power automatically targets the next seat in rotation: + - If last removal was Seat 0, next automatically targets Seat 1 + - If last removal was Seat 1, next automatically targets Seat 2 + - If last removal was Seat 2, next automatically targets Seat 0 +- Building Power has independent automatic rotation +- Rotation ensures no single seat is disproportionately targeted +- Collective members cannot choose which seat to target - it's determined automatically + +**Open Questions:** +- Q42: Should rotation reset if removal fails, or continue regardless? + +#### Replacement Process (Single Atomic Vote) + +The replacement happens in a single vote where the collective votes **both** to remove the current seat holder **and** to install a specific replacement candidate. This is an atomic operation - either both happen or neither happens. + +**Process:** +1. **Proposal Phase**: Any collective member can propose a replacement by submitting: + - Replacement candidate account + - Optional: Justification text + +2. **Voting Phase**: + - All collective members vote Aye/Nay on the replacement proposal + - Threshold: Simple majority (11 of 20 members) + - Voting period: 7 days (50,400 blocks) + + - **If vote succeeds**: Current seat holder immediately removed, replacement candidate immediately installed + - **If vote fails**: No change, current member remains, cooldown timer starts + +4. **Transition**: Atomic swap ensures Triumvirate always has exactly 3 members with no vacancy period + +**Open Questions:** +- Q43: From where the candidate is selected? +- Q44: Can multiple replacement proposals be submitted for the same cycle? First-come-first-served or best candidate wins? +- Q45: Can replacement vote be vetoed by OTF in emergency situations? +- Q46: What happens to in-flight proposals where replaced member already voted? +- Q47: Can a replaced member be immediately proposed as replacement for a different seat? +- Q48: Who can propose replacement candidates? Any collective member or requires threshold support? +- Q49: Should there be a minimum vetting period between proposal and voting? + +### Implementation Phases + +#### Phase 1: Coexistence (Duration: 3-6 months) + +1. Remove dead code: triumvirate collective and senate pallets and related code +2. Implement the governance as a new pallet +3. Deploy new governance pallet to runtime +4. Configure initial Triumvirate members +5. Configure OTF account(s) +6. Run new governance system in parallel with existing sudo multisig +7. All governance decisions processed through new system but sudo retains override capability +8. Monitor system performance, voting patterns, and security +9. Community review and feedback period + +#### Phase 2: Full Migration + +1. Disable sudo pallet via governance vote +2. Remove dead code: triumvirate collective and senate pallets +3. New governance system becomes sole authority +4. Emergency procedures documented and tested + +**Open Questions:** +- Q50: What constitutes "emergency" and who decides to invoke emergency procedures? diff --git a/pallets/governance/src/lib.rs b/pallets/governance/src/lib.rs index d59f09ed2e..40bdf93fac 100644 --- a/pallets/governance/src/lib.rs +++ b/pallets/governance/src/lib.rs @@ -12,7 +12,7 @@ use frame_support::{ }, }; use frame_system::pallet_prelude::*; -use sp_runtime::{Saturating, traits::Hash}; +use sp_runtime::{Percent, Saturating, traits::Hash}; use sp_std::{collections::btree_set::BTreeSet, vec::Vec}; use subtensor_macros::freeze_struct; @@ -20,7 +20,11 @@ mod mock; mod tests; pub use pallet::*; -const TRIUMVIRATE_SIZE: u32 = 3; +/// WARNING: Any changes to these 3 constants require a migration to update the `BoundedVec` in storage +/// for `Triumvirate`, `EconomicCollective`, or `BuildingCollective`. +pub const TRIUMVIRATE_SIZE: u32 = 3; +pub const ECONOMIC_COLLECTIVE_SIZE: u32 = 10; +pub const BUILDING_COLLECTIVE_SIZE: u32 = 10; pub type CurrencyOf = ::Currency; @@ -53,6 +57,42 @@ pub struct Votes { end: BlockNumber, } +#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)] +// #[freeze_struct("58071fdbad8767b6")] +pub struct CollectiveVotes { + /// The proposal's unique index. + index: ProposalIndex, + /// The set of economic collective members that approved it. + economic_ayes: BoundedVec>, + /// The set of economic collective members that rejected it. + economic_nays: BoundedVec>, + /// The set of building collective members that approved it. + building_ayes: BoundedVec>, + /// The set of building collective members that rejected it. + building_nays: BoundedVec>, +} + +#[derive( + PartialEq, + Eq, + Clone, + Encode, + Decode, + RuntimeDebug, + TypeInfo, + MaxEncodedLen, + DecodeWithMemTracking, +)] +pub enum CollectiveMember { + Economic(AccountId), + Building(AccountId), +} + +pub trait CollectiveMembersProvider { + fn get_economic_collective() -> BoundedVec>; + fn get_building_collective() -> BoundedVec>; +} + #[frame_support::pallet] pub mod pallet { use super::*; @@ -94,6 +134,9 @@ pub mod pallet { /// Origin allowed to set triumvirate. type SetTriumvirateOrigin: EnsureOrigin; + /// The collective members provider. + type CollectiveMembersProvider: CollectiveMembersProvider; + /// How many accounts allowed to submit proposals. #[pallet::constant] type MaxAllowedProposers: Get; @@ -113,6 +156,22 @@ pub mod pallet { /// The duration of a motion. #[pallet::constant] type MotionDuration: Get>; + + /// Initial scheduling delay for proposal execution. + #[pallet::constant] + type InitialSchedulingDelay: Get>; + + /// Period of time between collective rotations. + #[pallet::constant] + type CollectiveRotationPeriod: Get>; + + /// Percentage threshold for a proposal to be cancelled by a collective vote. + #[pallet::constant] + type CancellationThreshold: Get; + + /// Percentage threshold for a proposal to be fast-tracked by a collective vote. + #[pallet::constant] + type FastTrackThreshold: Get; } /// Accounts allowed to submit proposals. @@ -148,6 +207,21 @@ pub mod pallet { pub type Scheduled = StorageValue<_, BoundedVec, ValueQuery>; + /// The economic collective members (top 20 validators by total stake). + #[pallet::storage] + pub type EconomicCollective = + StorageValue<_, BoundedVec>, ValueQuery>; + + /// The building collective members (top 20 subnet owners by moving average price). + #[pallet::storage] + pub type BuildingCollective = + StorageValue<_, BoundedVec>, ValueQuery>; + + /// Collective votes for a given proposal, if it is scheduled. + #[pallet::storage] + pub type CollectiveVoting = + StorageMap<_, Identity, T::Hash, CollectiveVotes, OptionQuery>; + #[pallet::genesis_config] #[derive(frame_support::DefaultNoBound)] pub struct GenesisConfig { @@ -185,21 +259,25 @@ pub mod pallet { #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { + /// The allowed proposers have been set. AllowedProposersSet { incoming: Vec, outgoing: Vec, removed_proposals: Vec<(T::AccountId, T::Hash)>, }, + /// The triumvirate has been set. TriumvirateSet { incoming: Vec, outgoing: Vec, }, + /// A proposal has been submitted. Proposed { account: T::AccountId, proposal_index: u32, proposal_hash: T::Hash, end: BlockNumberFor, }, + /// A triumvirate member has voted on a proposal. Voted { account: T::AccountId, proposal_hash: T::Hash, @@ -207,12 +285,20 @@ pub mod pallet { yes: u32, no: u32, }, - Scheduled { - proposal_hash: T::Hash, - }, - Cancelled { + /// A collective member has voted on a proposal. + CollectiveMemberVoted { + account: CollectiveMember, proposal_hash: T::Hash, + voted: bool, + economic_yes: u32, + economic_no: u32, + building_yes: u32, + building_no: u32, }, + /// A proposal has been scheduled for execution. + Scheduled { proposal_hash: T::Hash }, + /// A proposal has been cancelled. + Cancelled { proposal_hash: T::Hash }, } #[pallet::error] @@ -253,10 +339,31 @@ pub mod pallet { InvalidProposalHashLength, /// Proposal is already scheduled. AlreadyScheduled, + /// Origin is not a collective member. + NotCollectiveMember, + /// Proposal is not scheduled. + ProposalNotScheduled, + } + + #[pallet::hooks] + impl Hooks> for Pallet { + fn on_initialize(n: BlockNumberFor) -> Weight { + let economic_collective = EconomicCollective::::get(); + let building_collective = BuildingCollective::::get(); + let is_first_run = economic_collective.is_empty() || building_collective.is_empty(); + let must_rotate = n % T::CollectiveRotationPeriod::get() == Zero::zero(); + + if is_first_run || must_rotate { + Self::do_rotate_collectives(); + } + + Weight::zero() + } } #[pallet::call] impl Pallet { + /// Set the allowed proposers. #[pallet::call_index(0)] #[pallet::weight(Weight::zero())] pub fn set_allowed_proposers( @@ -304,6 +411,7 @@ pub mod pallet { Ok(()) } + /// Set the triumvirate. #[pallet::call_index(1)] #[pallet::weight(Weight::zero())] pub fn set_triumvirate( @@ -351,6 +459,7 @@ pub mod pallet { Ok(()) } + /// Propose a new proposal. #[pallet::call_index(2)] #[pallet::weight(Weight::zero())] pub fn propose( @@ -358,12 +467,7 @@ pub mod pallet { proposal: Box<::RuntimeCall>, #[pallet::compact] length_bound: u32, ) -> DispatchResult { - let who = ensure_signed(origin)?; - let allowed_proposers = AllowedProposers::::get(); - ensure!( - allowed_proposers.contains(&who), - Error::::NotAllowedProposer - ); + let who = Self::ensure_allowed_proposer(origin)?; let proposal_len = proposal.encoded_size(); ensure!( @@ -417,6 +521,7 @@ pub mod pallet { Ok(()) } + /// Vote on a proposal as a triumvirate member. #[pallet::call_index(3)] #[pallet::weight(Weight::zero())] pub fn vote( @@ -425,9 +530,7 @@ pub mod pallet { #[pallet::compact] proposal_index: ProposalIndex, approve: bool, ) -> DispatchResult { - let who = ensure_signed(origin)?; - let triumvirate = Triumvirate::::get(); - ensure!(triumvirate.contains(&who), Error::::NotTriumvirateMember); + let who = Self::ensure_triumvirate_member(origin)?; let proposals = Proposals::::get(); ensure!( @@ -457,6 +560,49 @@ pub mod pallet { Ok(()) } + + /// Vote on a proposal as a collective member. + #[pallet::call_index(4)] + #[pallet::weight(Weight::zero())] + pub fn collective_vote( + origin: OriginFor, + proposal_hash: T::Hash, + #[pallet::compact] proposal_index: ProposalIndex, + approve: bool, + ) -> DispatchResult { + let who = Self::ensure_collective_member(origin)?; + + let scheduled = Scheduled::::get(); + ensure!( + scheduled.contains(&proposal_hash), + Error::::ProposalNotScheduled + ); + + Self::do_collective_vote(&who, proposal_hash, proposal_index, approve)?; + + let voting = CollectiveVoting::::get(proposal_hash) + .ok_or(Error::::ProposalNotScheduled)?; + let economic_yes_votes = voting.economic_ayes.len() as u32; + let economic_no_votes = voting.economic_nays.len() as u32; + let building_yes_votes = voting.building_ayes.len() as u32; + let building_no_votes = voting.building_nays.len() as u32; + + Self::deposit_event(Event::::CollectiveMemberVoted { + account: who, + proposal_hash, + voted: approve, + economic_yes: economic_yes_votes, + economic_no: economic_no_votes, + building_yes: building_yes_votes, + building_no: building_no_votes, + }); + + if economic_yes_votes >= 2 || building_yes_votes >= 2 { + Self::do_schedule(proposal_hash)?; + } + + Ok(()) + } } } @@ -502,41 +648,77 @@ impl Pallet { let voting = voting.as_mut().ok_or(Error::::ProposalMissing)?; ensure!(voting.index == index, Error::::WrongProposalIndex); - let has_yes_vote = voting.ayes.iter().any(|a| a == who); - let has_no_vote = voting.nays.iter().any(|a| a == who); - - if approve { - if !has_yes_vote { - voting - .ayes - .try_push(who.clone()) - // Unreachable because nobody can double vote. - .map_err(|_| Error::::TooManyVotes)?; - } else { - return Err(Error::::DuplicateVote.into()); - } - if has_no_vote { - voting.nays.retain(|a| a != who); - } - } else { - if !has_no_vote { - voting - .nays - .try_push(who.clone()) - // Unreachable because nobody can double vote. - .map_err(|_| Error::::TooManyVotes)?; - } else { - return Err(Error::::DuplicateVote.into()); - } - if has_yes_vote { - voting.ayes.retain(|a| a != who); - } + Self::do_vote_inner(&who, approve, &mut voting.ayes, &mut voting.nays)?; + + Ok(()) + }) + } + + fn do_collective_vote( + who: &CollectiveMember, + proposal_hash: T::Hash, + index: ProposalIndex, + approve: bool, + ) -> DispatchResult { + CollectiveVoting::::try_mutate(proposal_hash, |voting| -> DispatchResult { + let voting = voting.as_mut().ok_or(Error::::ProposalNotScheduled)?; + ensure!(voting.index == index, Error::::WrongProposalIndex); + + match who { + CollectiveMember::Economic(who) => Self::do_vote_inner( + who, + approve, + &mut voting.economic_ayes, + &mut voting.economic_nays, + )?, + CollectiveMember::Building(who) => Self::do_vote_inner( + who, + approve, + &mut voting.building_ayes, + &mut voting.building_nays, + )?, } Ok(()) }) } + fn do_vote_inner>( + who: &T::AccountId, + approve: bool, + ayes: &mut BoundedVec, + nays: &mut BoundedVec, + ) -> DispatchResult { + let has_yes_vote = ayes.iter().any(|a| a == who); + let has_no_vote = nays.iter().any(|a| a == who); + + if approve { + if !has_yes_vote { + ayes.try_push(who.clone()) + // Unreachable because nobody can double vote. + .map_err(|_| Error::::TooManyVotes)?; + } else { + return Err(Error::::DuplicateVote.into()); + } + if has_no_vote { + nays.retain(|a| a != who); + } + } else { + if !has_no_vote { + nays.try_push(who.clone()) + // Unreachable because nobody can double vote. + .map_err(|_| Error::::TooManyVotes)?; + } else { + return Err(Error::::DuplicateVote.into()); + } + if has_yes_vote { + ayes.retain(|a| a != who); + } + } + + Ok(()) + } + fn do_schedule(proposal_hash: T::Hash) -> DispatchResult { Scheduled::::try_append(proposal_hash).map_err(|_| Error::::TooManyScheduled)?; @@ -550,7 +732,7 @@ impl Pallet { .try_into() // Unreachable because we expect the hash to be 32 bytes. .map_err(|_| Error::::InvalidProposalHashLength)?, - DispatchTime::At(now + T::MotionDuration::get()), + DispatchTime::At(now + T::InitialSchedulingDelay::get()), None, Priority::default(), RawOrigin::Root.into(), @@ -576,4 +758,44 @@ impl Pallet { ProposalOf::::remove(&proposal_hash); Voting::::remove(&proposal_hash); } + + fn do_rotate_collectives() { + let economic_collective_members = T::CollectiveMembersProvider::get_economic_collective(); + let building_collective_members = T::CollectiveMembersProvider::get_building_collective(); + EconomicCollective::::put(economic_collective_members); + BuildingCollective::::put(building_collective_members); + } + + fn ensure_allowed_proposer(origin: OriginFor) -> Result { + let who = ensure_signed(origin)?; + let allowed_proposers = AllowedProposers::::get(); + ensure!( + allowed_proposers.contains(&who), + Error::::NotAllowedProposer + ); + Ok(who) + } + + fn ensure_triumvirate_member(origin: OriginFor) -> Result { + let who = ensure_signed(origin)?; + let triumvirate = Triumvirate::::get(); + ensure!(triumvirate.contains(&who), Error::::NotTriumvirateMember); + Ok(who) + } + + fn ensure_collective_member( + origin: OriginFor, + ) -> Result, DispatchError> { + let who = ensure_signed(origin)?; + let economic_collective = EconomicCollective::::get(); + let building_collective = BuildingCollective::::get(); + + if economic_collective.contains(&who) { + Ok(CollectiveMember::Economic(who)) + } else if building_collective.contains(&who) { + Ok(CollectiveMember::Building(who)) + } else { + Err(Error::::NotCollectiveMember.into()) + } + } } diff --git a/pallets/governance/src/mock.rs b/pallets/governance/src/mock.rs index 7719f8b342..49cf9af082 100644 --- a/pallets/governance/src/mock.rs +++ b/pallets/governance/src/mock.rs @@ -7,9 +7,14 @@ use frame_support::{derive_impl, pallet_prelude::*, parameter_types, traits::EqualPrivilegeOnly}; use frame_system::{EnsureRoot, limits, pallet_prelude::*}; use sp_core::U256; -use sp_runtime::{BuildStorage, Perbill, traits::IdentityLookup}; +use sp_runtime::{BuildStorage, Perbill, Percent, traits::IdentityLookup}; +use sp_std::cell::RefCell; +use std::marker::PhantomData; -use crate::{BalanceOf, pallet as pallet_governance}; +use crate::{ + BUILDING_COLLECTIVE_SIZE, BalanceOf, CollectiveMembersProvider, ECONOMIC_COLLECTIVE_SIZE, + pallet as pallet_governance, +}; type Block = frame_system::mocking::MockBlock; pub(crate) type AccountOf = ::AccountId; @@ -70,12 +75,54 @@ impl pallet_scheduler::Config for Test { type BlockNumberProvider = System; } +pub struct FakeCollectiveMembersProvider(PhantomData); +impl CollectiveMembersProvider for FakeCollectiveMembersProvider +where + T::AccountId: From>, +{ + fn get_economic_collective() -> BoundedVec> { + BoundedVec::truncate_from(ECONOMIC_COLLECTIVE.with(|c| { + c.borrow() + .iter() + .map(|a| T::AccountId::from(a.clone())) + .collect() + })) + } + fn get_building_collective() -> BoundedVec> { + BoundedVec::truncate_from(BUILDING_COLLECTIVE.with(|c| { + c.borrow() + .iter() + .map(|a| T::AccountId::from(a.clone())) + .collect() + })) + } +} + +thread_local! { + pub static ECONOMIC_COLLECTIVE: RefCell>> = const { RefCell::new(vec![]) }; + pub static BUILDING_COLLECTIVE: RefCell>> = const { RefCell::new(vec![]) }; +} + +pub fn set_next_economic_collective(members: Vec) { + assert_eq!(members.len(), ECONOMIC_COLLECTIVE_SIZE as usize); + ECONOMIC_COLLECTIVE.with_borrow_mut(|c| *c = members.clone()); +} + +pub fn set_next_building_collective(members: Vec) { + assert_eq!(members.len(), BUILDING_COLLECTIVE_SIZE as usize); + BUILDING_COLLECTIVE.with_borrow_mut(|c| *c = members.clone()); +} + parameter_types! { pub const MaxAllowedProposers: u32 = 5; pub const MaxProposalWeight: Weight = Weight::from_parts(1_000_000_000_000, 0); pub const MaxProposals: u32 = 5; pub const MaxScheduled: u32 = 10; pub const MotionDuration: BlockNumberFor = 20; + pub const InitialSchedulingDelay: BlockNumberFor = 20; + pub const CollectiveRotationPeriod: BlockNumberFor = 100; + pub const CancellationThreshold: Percent = Percent::from_percent(50); + pub FastTrackThreshold: Percent = Percent::from_rational(2u32, 3u32); // ~66.67% } impl pallet_governance::Config for Test { @@ -85,11 +132,16 @@ impl pallet_governance::Config for Test { type Scheduler = Scheduler; type SetAllowedProposersOrigin = EnsureRoot>; type SetTriumvirateOrigin = EnsureRoot>; + type CollectiveMembersProvider = FakeCollectiveMembersProvider; type MaxAllowedProposers = MaxAllowedProposers; type MaxProposalWeight = MaxProposalWeight; type MaxProposals = MaxProposals; type MaxScheduled = MaxScheduled; type MotionDuration = MotionDuration; + type InitialSchedulingDelay = InitialSchedulingDelay; + type CollectiveRotationPeriod = CollectiveRotationPeriod; + type CancellationThreshold = CancellationThreshold; + type FastTrackThreshold = FastTrackThreshold; } #[frame_support::pallet] @@ -121,6 +173,8 @@ pub(crate) struct TestState { balances: Vec<(AccountOf, BalanceOf)>, allowed_proposers: Vec>, triumvirate: Vec>, + economic_collective: BoundedVec, ConstU32>, + building_collective: BoundedVec, ConstU32>, } impl Default for TestState { @@ -130,6 +184,16 @@ impl Default for TestState { balances: vec![], allowed_proposers: vec![U256::from(1), U256::from(2), U256::from(3)], triumvirate: vec![U256::from(1001), U256::from(1002), U256::from(1003)], + economic_collective: BoundedVec::truncate_from( + (1..=ECONOMIC_COLLECTIVE_SIZE) + .map(|i| U256::from(2000 + i)) + .collect::>(), + ), + building_collective: BoundedVec::truncate_from( + (1..=BUILDING_COLLECTIVE_SIZE) + .map(|i| U256::from(3000 + i)) + .collect::>(), + ), } } } @@ -163,7 +227,11 @@ impl TestState { .build_storage() .unwrap() .into(); - ext.execute_with(|| System::set_block_number(self.block_number)); + ext.execute_with(|| { + System::set_block_number(self.block_number); + set_next_economic_collective(self.economic_collective.to_vec()); + set_next_building_collective(self.building_collective.to_vec()); + }); ext } @@ -187,3 +255,7 @@ pub(crate) fn last_n_events(n: usize) -> Vec { .map(|e| e.event) .collect() } + +pub(crate) fn run_to_block(n: BlockNumberFor) { + System::run_to_block::(n); +} diff --git a/pallets/governance/src/tests.rs b/pallets/governance/src/tests.rs index 4d618853d5..bb684e0408 100644 --- a/pallets/governance/src/tests.rs +++ b/pallets/governance/src/tests.rs @@ -393,6 +393,7 @@ fn propose_works_with_inline_preimage() { let length_bound = proposal.encoded_size() as u32; let proposal_index = ProposalCount::::get(); + assert_eq!(proposal_index, 0); assert_ok!(Pallet::::propose( RuntimeOrigin::signed(U256::from(1)), proposal.clone(), @@ -445,6 +446,7 @@ fn propose_works_with_lookup_preimage() { let length_bound = proposal.encoded_size() as u32; let proposal_index = ProposalCount::::get(); + assert_eq!(proposal_index, 0); assert_ok!(Pallet::::propose( RuntimeOrigin::signed(U256::from(1)), proposal.clone(), @@ -993,6 +995,88 @@ fn aye_vote_on_proposal_with_too_many_scheduled_fails() { }); } +#[test] +fn collective_vote_from_non_collective_member_fails() { + TestState::default().build_and_execute(|| { + let (proposal_hash, proposal_index) = create_scheduled_proposal(); + + assert_noop!( + Pallet::::collective_vote( + RuntimeOrigin::signed(U256::from(2001)), + proposal_hash, + proposal_index, + true + ), + Error::::NotCollectiveMember + ); + }); +} + +#[test] +fn collective_vote_on_non_scheduled_proposal_fails() { + TestState::default().build_and_execute(|| { + let (proposal_hash, proposal_index) = create_proposal(); + + assert_noop!( + Pallet::::collective_vote( + RuntimeOrigin::signed(U256::from(2001)), + proposal_hash, + proposal_index, + true + ), + Error::::NotCollectiveMember + ); + }); +} + +#[test] +fn collective_rotation_works() { + TestState::default().build_and_execute(|| { + let next_economic_collective = (1..=ECONOMIC_COLLECTIVE_SIZE) + .map(|i| U256::from(4000 + i)) + .collect::>(); + let next_building_collective = (1..=BUILDING_COLLECTIVE_SIZE) + .map(|i| U256::from(5000 + i)) + .collect::>(); + assert_eq!(EconomicCollective::::get().to_vec(), vec![]); + assert_eq!(BuildingCollective::::get().to_vec(), vec![]); + + // Trigger the initial collective rotation given both are empty. + run_to_block(2); + + assert_eq!( + EconomicCollective::::get().len(), + ECONOMIC_COLLECTIVE_SIZE as usize, + ); + assert_ne!( + EconomicCollective::::get().to_vec(), + next_economic_collective + ); + assert_eq!( + BuildingCollective::::get().len(), + BUILDING_COLLECTIVE_SIZE as usize, + ); + assert_ne!( + BuildingCollective::::get().to_vec(), + next_building_collective + ); + + set_next_economic_collective(next_economic_collective.clone()); + set_next_building_collective(next_building_collective.clone()); + + run_to_block(CollectiveRotationPeriod::get()); + + assert_eq!( + EconomicCollective::::get().to_vec(), + next_economic_collective + ); + assert_eq!( + BuildingCollective::::get().to_vec(), + next_building_collective + ); + }); +} + fn create_custom_proposal( proposer: U256, call: impl Into>, @@ -1020,6 +1104,14 @@ fn create_proposal() -> (::Hash, u32) { ) } +fn create_scheduled_proposal() -> (::Hash, u32) { + let (proposal_hash, proposal_index) = create_proposal(); + vote_aye(U256::from(1001), proposal_hash, proposal_index); + vote_aye(U256::from(1002), proposal_hash, proposal_index); + + (proposal_hash, proposal_index) +} + fn vote_aye( voter: U256, proposal_hash: ::Hash, From d6d92ff7715d991d8b89fc8458f2bb5b3da757a4 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Wed, 5 Nov 2025 11:04:50 -0300 Subject: [PATCH 13/23] some renaming --- pallets/governance/src/lib.rs | 20 ++++++++++---------- pallets/governance/src/tests.rs | 26 +++++++++++++------------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/pallets/governance/src/lib.rs b/pallets/governance/src/lib.rs index 40bdf93fac..eb883ff155 100644 --- a/pallets/governance/src/lib.rs +++ b/pallets/governance/src/lib.rs @@ -271,14 +271,14 @@ pub mod pallet { outgoing: Vec, }, /// A proposal has been submitted. - Proposed { + ProposalSubmitted { account: T::AccountId, proposal_index: u32, proposal_hash: T::Hash, - end: BlockNumberFor, + voting_end: BlockNumberFor, }, /// A triumvirate member has voted on a proposal. - Voted { + TriumvirateMemberVoted { account: T::AccountId, proposal_hash: T::Hash, voted: bool, @@ -296,9 +296,9 @@ pub mod pallet { building_no: u32, }, /// A proposal has been scheduled for execution. - Scheduled { proposal_hash: T::Hash }, + ProposalScheduled { proposal_hash: T::Hash }, /// A proposal has been cancelled. - Cancelled { proposal_hash: T::Hash }, + ProposalCancelled { proposal_hash: T::Hash }, } #[pallet::error] @@ -512,11 +512,11 @@ pub mod pallet { }, ); - Self::deposit_event(Event::::Proposed { + Self::deposit_event(Event::::ProposalSubmitted { account: who, proposal_index, proposal_hash, - end, + voting_end: end, }); Ok(()) } @@ -544,7 +544,7 @@ pub mod pallet { let yes_votes = voting.ayes.len() as u32; let no_votes = voting.nays.len() as u32; - Self::deposit_event(Event::::Voted { + Self::deposit_event(Event::::TriumvirateMemberVoted { account: who, proposal_hash, voted: approve, @@ -741,13 +741,13 @@ impl Pallet { Self::clear_proposal(proposal_hash); - Self::deposit_event(Event::::Scheduled { proposal_hash }); + Self::deposit_event(Event::::ProposalScheduled { proposal_hash }); Ok(()) } fn do_cancel(proposal_hash: T::Hash) -> DispatchResult { Self::clear_proposal(proposal_hash); - Self::deposit_event(Event::::Cancelled { proposal_hash }); + Self::deposit_event(Event::::ProposalCancelled { proposal_hash }); Ok(()) } diff --git a/pallets/governance/src/tests.rs b/pallets/governance/src/tests.rs index bb684e0408..e5f4dadf00 100644 --- a/pallets/governance/src/tests.rs +++ b/pallets/governance/src/tests.rs @@ -423,11 +423,11 @@ fn propose_works_with_inline_preimage() { ); assert_eq!( last_event(), - RuntimeEvent::Governance(Event::::Proposed { + RuntimeEvent::Governance(Event::::ProposalSubmitted { account: U256::from(1), proposal_index: 0, proposal_hash, - end: now + MotionDuration::get(), + voting_end: now + MotionDuration::get(), }) ); }); @@ -476,11 +476,11 @@ fn propose_works_with_lookup_preimage() { ); assert_eq!( last_event(), - RuntimeEvent::Governance(Event::::Proposed { + RuntimeEvent::Governance(Event::::ProposalSubmitted { account: U256::from(1), proposal_index: 0, proposal_hash, - end: now + MotionDuration::get(), + voting_end: now + MotionDuration::get(), }) ); }); @@ -680,7 +680,7 @@ fn vote_aye_as_first_voter_works() { assert_eq!(votes.nays.to_vec(), vec![]); assert_eq!( last_event(), - RuntimeEvent::Governance(Event::::Voted { + RuntimeEvent::Governance(Event::::TriumvirateMemberVoted { account: U256::from(1001), proposal_hash, voted: true, @@ -709,7 +709,7 @@ fn vote_nay_as_first_voter_works() { assert_eq!(votes.ayes.to_vec(), vec![]); assert_eq!( last_event(), - RuntimeEvent::Governance(Event::::Voted { + RuntimeEvent::Governance(Event::::TriumvirateMemberVoted { account: U256::from(1001), proposal_hash, voted: false, @@ -732,7 +732,7 @@ fn vote_can_be_updated() { assert_eq!(votes.nays.to_vec(), vec![]); assert_eq!( last_event(), - RuntimeEvent::Governance(Event::::Voted { + RuntimeEvent::Governance(Event::::TriumvirateMemberVoted { account: U256::from(1001), proposal_hash, voted: true, @@ -748,7 +748,7 @@ fn vote_can_be_updated() { assert_eq!(votes.ayes.to_vec(), vec![]); assert_eq!( last_event(), - RuntimeEvent::Governance(Event::::Voted { + RuntimeEvent::Governance(Event::::TriumvirateMemberVoted { account: U256::from(1001), proposal_hash, voted: false, @@ -764,7 +764,7 @@ fn vote_can_be_updated() { assert_eq!(votes.nays.to_vec(), vec![]); assert_eq!( last_event(), - RuntimeEvent::Governance(Event::::Voted { + RuntimeEvent::Governance(Event::::TriumvirateMemberVoted { account: U256::from(1001), proposal_hash, voted: true, @@ -796,7 +796,7 @@ fn two_aye_votes_schedule_proposal() { let events = last_n_events(3); assert_eq!( events[0], - RuntimeEvent::Governance(Event::::Voted { + RuntimeEvent::Governance(Event::::TriumvirateMemberVoted { account: U256::from(1003), proposal_hash, voted: true, @@ -806,7 +806,7 @@ fn two_aye_votes_schedule_proposal() { ); assert_eq!( events[2], - RuntimeEvent::Governance(Event::::Scheduled { proposal_hash }) + RuntimeEvent::Governance(Event::::ProposalScheduled { proposal_hash }) ); }); } @@ -827,7 +827,7 @@ fn two_nay_votes_cancel_proposal() { let events = last_n_events(2); assert_eq!( events[0], - RuntimeEvent::Governance(Event::::Voted { + RuntimeEvent::Governance(Event::::TriumvirateMemberVoted { account: U256::from(1003), proposal_hash, voted: false, @@ -837,7 +837,7 @@ fn two_nay_votes_cancel_proposal() { ); assert_eq!( events[1], - RuntimeEvent::Governance(Event::::Cancelled { proposal_hash }) + RuntimeEvent::Governance(Event::::ProposalCancelled { proposal_hash }) ); }); } From da4691d7f6ce095dcc41f2d3703fe240943a5ab9 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Thu, 6 Nov 2025 11:49:19 -0300 Subject: [PATCH 14/23] added collective voting conditional logic + basic tests --- pallets/governance/src/lib.rs | 90 ++++++++++++++++++++++++--------- pallets/governance/src/mock.rs | 2 +- pallets/governance/src/tests.rs | 69 ++++++++++++++++++++++--- 3 files changed, 128 insertions(+), 33 deletions(-) diff --git a/pallets/governance/src/lib.rs b/pallets/governance/src/lib.rs index eb883ff155..c764cc2183 100644 --- a/pallets/governance/src/lib.rs +++ b/pallets/governance/src/lib.rs @@ -538,9 +538,8 @@ pub mod pallet { Error::::ProposalMissing ); - Self::do_vote(&who, proposal_hash, proposal_index, approve)?; + let voting = Self::do_vote(&who, proposal_hash, proposal_index, approve)?; - let voting = Voting::::get(proposal_hash).ok_or(Error::::ProposalMissing)?; let yes_votes = voting.ayes.len() as u32; let no_votes = voting.nays.len() as u32; @@ -553,7 +552,7 @@ pub mod pallet { }); if yes_votes >= 2 { - Self::do_schedule(proposal_hash)?; + Self::do_schedule(proposal_hash, proposal_index)?; } else if no_votes >= 2 { Self::do_cancel(proposal_hash)?; } @@ -578,27 +577,55 @@ pub mod pallet { Error::::ProposalNotScheduled ); - Self::do_collective_vote(&who, proposal_hash, proposal_index, approve)?; + let voting = Self::do_collective_vote(&who, proposal_hash, proposal_index, approve)?; - let voting = CollectiveVoting::::get(proposal_hash) - .ok_or(Error::::ProposalNotScheduled)?; - let economic_yes_votes = voting.economic_ayes.len() as u32; - let economic_no_votes = voting.economic_nays.len() as u32; - let building_yes_votes = voting.building_ayes.len() as u32; - let building_no_votes = voting.building_nays.len() as u32; + let economic_yes_votes = voting.economic_ayes.len() as i32; + let economic_no_votes = voting.economic_nays.len() as i32; + let building_yes_votes = voting.building_ayes.len() as i32; + let building_no_votes = voting.building_nays.len() as i32; Self::deposit_event(Event::::CollectiveMemberVoted { account: who, proposal_hash, voted: approve, - economic_yes: economic_yes_votes, - economic_no: economic_no_votes, - building_yes: building_yes_votes, - building_no: building_no_votes, + economic_yes: economic_yes_votes as u32, + economic_no: economic_no_votes as u32, + building_yes: building_yes_votes as u32, + building_no: building_no_votes as u32, }); - if economic_yes_votes >= 2 || building_yes_votes >= 2 { - Self::do_schedule(proposal_hash)?; + let economic_net_score = economic_yes_votes.saturating_sub(economic_no_votes); + let building_net_score = building_yes_votes.saturating_sub(building_no_votes); + + let economic_fast_track_threshold = + T::FastTrackThreshold::get().mul_ceil(ECONOMIC_COLLECTIVE_SIZE); + let building_fast_track_threshold = + T::FastTrackThreshold::get().mul_ceil(BUILDING_COLLECTIVE_SIZE); + let economic_cancellation_threshold = + T::CancellationThreshold::get().mul_ceil(ECONOMIC_COLLECTIVE_SIZE); + let building_cancellation_threshold = + T::CancellationThreshold::get().mul_ceil(BUILDING_COLLECTIVE_SIZE); + + let has_reached_economic_fast_track = economic_net_score.is_positive() + && economic_net_score.abs() as u32 >= economic_fast_track_threshold; + let has_reached_building_fast_track = building_net_score.is_positive() + && building_net_score.abs() as u32 >= building_fast_track_threshold; + let should_fast_track = + has_reached_economic_fast_track || has_reached_building_fast_track; + + let has_reached_economic_cancellation = economic_net_score.is_negative() + && economic_net_score.abs() as u32 >= economic_cancellation_threshold; + let has_reached_building_cancellation = building_net_score.is_negative() + && building_net_score.abs() as u32 >= building_cancellation_threshold; + let should_cancel = + has_reached_economic_cancellation || has_reached_building_cancellation; + + if should_fast_track { + Self::do_fast_track(proposal_hash)?; + } else if should_cancel { + Self::do_cancel(proposal_hash)?; + } else { + // handle delay adjust by comparing economic/building net scores } Ok(()) @@ -643,14 +670,12 @@ impl Pallet { proposal_hash: T::Hash, index: ProposalIndex, approve: bool, - ) -> DispatchResult { - Voting::::try_mutate(proposal_hash, |voting| -> DispatchResult { + ) -> Result>, DispatchError> { + Voting::::try_mutate(proposal_hash, |voting| { let voting = voting.as_mut().ok_or(Error::::ProposalMissing)?; ensure!(voting.index == index, Error::::WrongProposalIndex); - Self::do_vote_inner(&who, approve, &mut voting.ayes, &mut voting.nays)?; - - Ok(()) + Ok(voting.clone()) }) } @@ -659,8 +684,8 @@ impl Pallet { proposal_hash: T::Hash, index: ProposalIndex, approve: bool, - ) -> DispatchResult { - CollectiveVoting::::try_mutate(proposal_hash, |voting| -> DispatchResult { + ) -> Result, DispatchError> { + CollectiveVoting::::try_mutate(proposal_hash, |voting| { let voting = voting.as_mut().ok_or(Error::::ProposalNotScheduled)?; ensure!(voting.index == index, Error::::WrongProposalIndex); @@ -679,7 +704,7 @@ impl Pallet { )?, } - Ok(()) + Ok(voting.clone()) }) } @@ -719,7 +744,7 @@ impl Pallet { Ok(()) } - fn do_schedule(proposal_hash: T::Hash) -> DispatchResult { + fn do_schedule(proposal_hash: T::Hash, proposal_index: ProposalIndex) -> DispatchResult { Scheduled::::try_append(proposal_hash).map_err(|_| Error::::TooManyScheduled)?; let bounded = ProposalOf::::get(proposal_hash).ok_or(Error::::ProposalMissing)?; @@ -741,6 +766,17 @@ impl Pallet { Self::clear_proposal(proposal_hash); + CollectiveVoting::::insert( + proposal_hash, + CollectiveVotes { + index: proposal_index, + economic_ayes: BoundedVec::new(), + economic_nays: BoundedVec::new(), + building_ayes: BoundedVec::new(), + building_nays: BoundedVec::new(), + }, + ); + Self::deposit_event(Event::::ProposalScheduled { proposal_hash }); Ok(()) } @@ -751,6 +787,10 @@ impl Pallet { Ok(()) } + fn do_fast_track(_proposal_hash: T::Hash) -> DispatchResult { + Ok(()) + } + fn clear_proposal(proposal_hash: T::Hash) { Proposals::::mutate(|proposals| { proposals.retain(|(_, h)| h != &proposal_hash); diff --git a/pallets/governance/src/mock.rs b/pallets/governance/src/mock.rs index 49cf9af082..27760461ed 100644 --- a/pallets/governance/src/mock.rs +++ b/pallets/governance/src/mock.rs @@ -228,9 +228,9 @@ impl TestState { .unwrap() .into(); ext.execute_with(|| { - System::set_block_number(self.block_number); set_next_economic_collective(self.economic_collective.to_vec()); set_next_building_collective(self.building_collective.to_vec()); + run_to_block(self.block_number); }); ext } diff --git a/pallets/governance/src/tests.rs b/pallets/governance/src/tests.rs index e5f4dadf00..f7dd1e21f1 100644 --- a/pallets/governance/src/tests.rs +++ b/pallets/governance/src/tests.rs @@ -787,6 +787,16 @@ fn two_aye_votes_schedule_proposal() { assert_eq!(Proposals::::get(), vec![]); assert!(!Voting::::contains_key(proposal_hash)); assert_eq!(Scheduled::::get(), vec![proposal_hash]); + assert_eq!( + CollectiveVoting::::get(proposal_hash), + Some(CollectiveVotes { + index: proposal_index, + economic_ayes: BoundedVec::new(), + economic_nays: BoundedVec::new(), + building_ayes: BoundedVec::new(), + building_nays: BoundedVec::new(), + }) + ); let task_name: [u8; 32] = proposal_hash.as_ref().try_into().unwrap(); let now = frame_system::Pallet::::block_number(); assert_eq!( @@ -1002,7 +1012,7 @@ fn collective_vote_from_non_collective_member_fails() { assert_noop!( Pallet::::collective_vote( - RuntimeOrigin::signed(U256::from(2001)), + RuntimeOrigin::signed(U256::from(42)), proposal_hash, proposal_index, true @@ -1024,7 +1034,57 @@ fn collective_vote_on_non_scheduled_proposal_fails() { proposal_index, true ), - Error::::NotCollectiveMember + Error::::ProposalNotScheduled + ); + }); +} + +#[test] +fn collective_vote_on_proposal_with_wrong_index_fails() { + TestState::default().build_and_execute(|| { + let (proposal_hash, _proposal_index) = create_scheduled_proposal(); + + assert_noop!( + Pallet::::collective_vote( + RuntimeOrigin::signed(U256::from(2001)), + proposal_hash, + 42, + true + ), + Error::::WrongProposalIndex + ); + }); +} + +#[test] +fn duplicate_collective_vote_on_scheduled_proposal_already_voted_fails() { + TestState::default().build_and_execute(|| { + let (proposal_hash, proposal_index) = create_scheduled_proposal(); + + let aye_voter = RuntimeOrigin::signed(U256::from(2001)); + let approve = true; + assert_ok!(Pallet::::collective_vote( + aye_voter.clone(), + proposal_hash, + proposal_index, + approve + )); + assert_noop!( + Pallet::::collective_vote(aye_voter, proposal_hash, proposal_index, approve), + Error::::DuplicateVote + ); + + let nay_voter = RuntimeOrigin::signed(U256::from(2002)); + let approve = false; + assert_ok!(Pallet::::collective_vote( + nay_voter.clone(), + proposal_hash, + proposal_index, + approve + )); + assert_noop!( + Pallet::::collective_vote(nay_voter, proposal_hash, proposal_index, approve), + Error::::DuplicateVote ); }); } @@ -1038,11 +1098,6 @@ fn collective_rotation_works() { let next_building_collective = (1..=BUILDING_COLLECTIVE_SIZE) .map(|i| U256::from(5000 + i)) .collect::>(); - assert_eq!(EconomicCollective::::get().to_vec(), vec![]); - assert_eq!(BuildingCollective::::get().to_vec(), vec![]); - - // Trigger the initial collective rotation given both are empty. - run_to_block(2); assert_eq!( EconomicCollective::::get().len(), From 0ea776b9cdd7ef4b420b90d12c8d31e86c320b5f Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Fri, 7 Nov 2025 10:35:10 -0300 Subject: [PATCH 15/23] add fast track/cancellation logic + tests --- pallets/governance/src/lib.rs | 124 +++++--- pallets/governance/src/mock.rs | 26 +- pallets/governance/src/tests.rs | 534 +++++++++++++++++++++++++------- 3 files changed, 512 insertions(+), 172 deletions(-) diff --git a/pallets/governance/src/lib.rs b/pallets/governance/src/lib.rs index c764cc2183..2766eed399 100644 --- a/pallets/governance/src/lib.rs +++ b/pallets/governance/src/lib.rs @@ -165,11 +165,11 @@ pub mod pallet { #[pallet::constant] type CollectiveRotationPeriod: Get>; - /// Percentage threshold for a proposal to be cancelled by a collective vote. + /// Percent threshold for a proposal to be cancelled by a collective vote. #[pallet::constant] type CancellationThreshold: Get; - /// Percentage threshold for a proposal to be fast-tracked by a collective vote. + /// Percent threshold for a proposal to be fast-tracked by a collective vote. #[pallet::constant] type FastTrackThreshold: Get; } @@ -299,6 +299,10 @@ pub mod pallet { ProposalScheduled { proposal_hash: T::Hash }, /// A proposal has been cancelled. ProposalCancelled { proposal_hash: T::Hash }, + /// A scheduled proposal has been fast-tracked. + ScheduledProposalFastTracked { proposal_hash: T::Hash }, + /// A scheduled proposal has been cancelled. + ScheduledProposalCancelled { proposal_hash: T::Hash }, } #[pallet::error] @@ -579,53 +583,37 @@ pub mod pallet { let voting = Self::do_collective_vote(&who, proposal_hash, proposal_index, approve)?; - let economic_yes_votes = voting.economic_ayes.len() as i32; - let economic_no_votes = voting.economic_nays.len() as i32; - let building_yes_votes = voting.building_ayes.len() as i32; - let building_no_votes = voting.building_nays.len() as i32; + let economic_yes_votes = voting.economic_ayes.len() as u32; + let economic_no_votes = voting.economic_nays.len() as u32; + let building_yes_votes = voting.building_ayes.len() as u32; + let building_no_votes = voting.building_nays.len() as u32; Self::deposit_event(Event::::CollectiveMemberVoted { account: who, proposal_hash, voted: approve, - economic_yes: economic_yes_votes as u32, - economic_no: economic_no_votes as u32, - building_yes: building_yes_votes as u32, - building_no: building_no_votes as u32, + economic_yes: economic_yes_votes, + economic_no: economic_no_votes, + building_yes: building_yes_votes, + building_no: building_no_votes, }); - let economic_net_score = economic_yes_votes.saturating_sub(economic_no_votes); - let building_net_score = building_yes_votes.saturating_sub(building_no_votes); - - let economic_fast_track_threshold = - T::FastTrackThreshold::get().mul_ceil(ECONOMIC_COLLECTIVE_SIZE); - let building_fast_track_threshold = - T::FastTrackThreshold::get().mul_ceil(BUILDING_COLLECTIVE_SIZE); - let economic_cancellation_threshold = - T::CancellationThreshold::get().mul_ceil(ECONOMIC_COLLECTIVE_SIZE); - let building_cancellation_threshold = - T::CancellationThreshold::get().mul_ceil(BUILDING_COLLECTIVE_SIZE); - - let has_reached_economic_fast_track = economic_net_score.is_positive() - && economic_net_score.abs() as u32 >= economic_fast_track_threshold; - let has_reached_building_fast_track = building_net_score.is_positive() - && building_net_score.abs() as u32 >= building_fast_track_threshold; - let should_fast_track = - has_reached_economic_fast_track || has_reached_building_fast_track; - - let has_reached_economic_cancellation = economic_net_score.is_negative() - && economic_net_score.abs() as u32 >= economic_cancellation_threshold; - let has_reached_building_cancellation = building_net_score.is_negative() - && building_net_score.abs() as u32 >= building_cancellation_threshold; - let should_cancel = - has_reached_economic_cancellation || has_reached_building_cancellation; + let should_fast_track = economic_yes_votes >= Self::economic_fast_track_threshold() + || building_yes_votes >= Self::building_fast_track_threshold(); + + let should_cancel = economic_no_votes >= Self::economic_cancellation_threshold() + || building_no_votes >= Self::building_cancellation_threshold(); + + let should_adjust_delay = !should_fast_track + && !should_cancel + && (economic_no_votes > 0 || building_no_votes > 0); if should_fast_track { Self::do_fast_track(proposal_hash)?; } else if should_cancel { - Self::do_cancel(proposal_hash)?; - } else { - // handle delay adjust by comparing economic/building net scores + Self::do_cancel_scheduled(proposal_hash)?; + } else if should_adjust_delay { + // handle delay adjustment } Ok(()) @@ -751,19 +739,19 @@ impl Pallet { ensure!(T::Preimages::have(&bounded), Error::::CallUnavailable); let now = frame_system::Pallet::::block_number(); + let name = proposal_hash + .as_ref() + .try_into() + // Unreachable because we expect the hash to be 32 bytes. + .map_err(|_| Error::::InvalidProposalHashLength)?; T::Scheduler::schedule_named( - proposal_hash - .as_ref() - .try_into() - // Unreachable because we expect the hash to be 32 bytes. - .map_err(|_| Error::::InvalidProposalHashLength)?, + name, DispatchTime::At(now + T::InitialSchedulingDelay::get()), None, Priority::default(), RawOrigin::Root.into(), bounded, )?; - Self::clear_proposal(proposal_hash); CollectiveVoting::::insert( @@ -787,7 +775,30 @@ impl Pallet { Ok(()) } - fn do_fast_track(_proposal_hash: T::Hash) -> DispatchResult { + fn do_fast_track(proposal_hash: T::Hash) -> DispatchResult { + let name = proposal_hash + .as_ref() + .try_into() + // Unreachable because we expect the hash to be 32 bytes. + .map_err(|_| Error::::InvalidProposalHashLength)?; + T::Scheduler::reschedule_named( + name, + // It will be scheduled on the next block because scheduler already ran for this block. + DispatchTime::After(Zero::zero()), + )?; + Self::clear_scheduled_proposal(proposal_hash); + Self::deposit_event(Event::::ScheduledProposalFastTracked { proposal_hash }); + Ok(()) + } + + fn do_cancel_scheduled(proposal_hash: T::Hash) -> DispatchResult { + let name = proposal_hash + .as_ref() + .try_into() + .map_err(|_| Error::::InvalidProposalHashLength)?; + T::Scheduler::cancel_named(name)?; + Self::clear_scheduled_proposal(proposal_hash); + Self::deposit_event(Event::::ScheduledProposalCancelled { proposal_hash }); Ok(()) } @@ -799,6 +810,13 @@ impl Pallet { Voting::::remove(&proposal_hash); } + fn clear_scheduled_proposal(proposal_hash: T::Hash) { + Scheduled::::mutate(|scheduled| { + scheduled.retain(|h| h != &proposal_hash); + }); + CollectiveVoting::::remove(&proposal_hash); + } + fn do_rotate_collectives() { let economic_collective_members = T::CollectiveMembersProvider::get_economic_collective(); let building_collective_members = T::CollectiveMembersProvider::get_building_collective(); @@ -838,4 +856,20 @@ impl Pallet { Err(Error::::NotCollectiveMember.into()) } } + + fn economic_fast_track_threshold() -> u32 { + T::FastTrackThreshold::get().mul_ceil(ECONOMIC_COLLECTIVE_SIZE) + } + + fn building_fast_track_threshold() -> u32 { + T::FastTrackThreshold::get().mul_ceil(BUILDING_COLLECTIVE_SIZE) as u32 + } + + fn economic_cancellation_threshold() -> u32 { + T::CancellationThreshold::get().mul_ceil(ECONOMIC_COLLECTIVE_SIZE) as u32 + } + + fn building_cancellation_threshold() -> u32 { + T::CancellationThreshold::get().mul_ceil(BUILDING_COLLECTIVE_SIZE) as u32 + } } diff --git a/pallets/governance/src/mock.rs b/pallets/governance/src/mock.rs index 27760461ed..1209897dfa 100644 --- a/pallets/governance/src/mock.rs +++ b/pallets/governance/src/mock.rs @@ -103,14 +103,20 @@ thread_local! { pub static BUILDING_COLLECTIVE: RefCell>> = const { RefCell::new(vec![]) }; } -pub fn set_next_economic_collective(members: Vec) { - assert_eq!(members.len(), ECONOMIC_COLLECTIVE_SIZE as usize); - ECONOMIC_COLLECTIVE.with_borrow_mut(|c| *c = members.clone()); +#[macro_export] +macro_rules! set_next_economic_collective { + ($members:expr) => {{ + assert_eq!($members.len(), ECONOMIC_COLLECTIVE_SIZE as usize); + ECONOMIC_COLLECTIVE.with_borrow_mut(|c| *c = $members.clone()); + }}; } -pub fn set_next_building_collective(members: Vec) { - assert_eq!(members.len(), BUILDING_COLLECTIVE_SIZE as usize); - BUILDING_COLLECTIVE.with_borrow_mut(|c| *c = members.clone()); +#[macro_export] +macro_rules! set_next_building_collective { + ($members:expr) => {{ + assert_eq!($members.len(), BUILDING_COLLECTIVE_SIZE as usize); + BUILDING_COLLECTIVE.with_borrow_mut(|c| *c = $members.clone()); + }}; } parameter_types! { @@ -121,8 +127,8 @@ parameter_types! { pub const MotionDuration: BlockNumberFor = 20; pub const InitialSchedulingDelay: BlockNumberFor = 20; pub const CollectiveRotationPeriod: BlockNumberFor = 100; - pub const CancellationThreshold: Percent = Percent::from_percent(50); - pub FastTrackThreshold: Percent = Percent::from_rational(2u32, 3u32); // ~66.67% + pub const FastTrackThreshold: Percent = Percent::from_percent(67); // ~2/3 + pub const CancellationThreshold: Percent = Percent::from_percent(51); } impl pallet_governance::Config for Test { @@ -228,8 +234,8 @@ impl TestState { .unwrap() .into(); ext.execute_with(|| { - set_next_economic_collective(self.economic_collective.to_vec()); - set_next_building_collective(self.building_collective.to_vec()); + set_next_economic_collective!(self.economic_collective.to_vec()); + set_next_building_collective!(self.building_collective.to_vec()); run_to_block(self.block_number); }); ext diff --git a/pallets/governance/src/tests.rs b/pallets/governance/src/tests.rs index f7dd1e21f1..96788e8548 100644 --- a/pallets/governance/src/tests.rs +++ b/pallets/governance/src/tests.rs @@ -92,7 +92,7 @@ fn set_allowed_proposers_works() { U256::from(3), U256::from(2), ]); - assert_eq!(AllowedProposers::::get(), vec![]); + assert!(AllowedProposers::::get().is_empty()); assert_ok!(Pallet::::set_allowed_proposers( // SetAllowedProposersOrigin is EnsureRoot @@ -131,23 +131,23 @@ fn set_allowed_proposers_works() { #[test] fn set_allowed_proposers_removes_proposals_of_outgoing_proposers() { TestState::default().build_and_execute(|| { - let (proposal_hash1, _proposal_index1) = create_custom_proposal( + let (proposal_hash1, _proposal_index1) = create_custom_proposal!( U256::from(1), frame_system::Call::::set_storage { items: vec![(b"Foobar".to_vec(), 1i32.to_be_bytes().to_vec())], - }, + } ); - let (proposal_hash2, _proposal_index2) = create_custom_proposal( + let (proposal_hash2, _proposal_index2) = create_custom_proposal!( U256::from(1), frame_system::Call::::set_storage { items: vec![(b"Foobar".to_vec(), 2i32.to_be_bytes().to_vec())], - }, + } ); - let (proposal_hash3, _proposal_index3) = create_custom_proposal( + let (proposal_hash3, _proposal_index3) = create_custom_proposal!( U256::from(3), frame_system::Call::::set_storage { items: vec![(b"Foobar".to_vec(), 3i32.to_be_bytes().to_vec())], - }, + } ); assert_eq!( AllowedProposers::::get(), @@ -244,7 +244,7 @@ fn set_triumvirate_works() { U256::from(1001), U256::from(1002), ]); - assert_eq!(Triumvirate::::get(), vec![]); + assert!(Triumvirate::::get().is_empty()); assert_ok!(Pallet::::set_triumvirate( // SetTriumvirateOrigin is EnsureRoot @@ -270,36 +270,36 @@ fn set_triumvirate_works() { #[test] fn set_triumvirate_removes_votes_of_outgoing_triumvirate_members() { TestState::default().build_and_execute(|| { - let (proposal_hash1, proposal_index1) = create_custom_proposal( + let (proposal_hash1, proposal_index1) = create_custom_proposal!( U256::from(1), frame_system::Call::::set_storage { items: vec![(b"Foobar".to_vec(), 1i32.to_be_bytes().to_vec())], - }, + } ); - let (proposal_hash2, proposal_index2) = create_custom_proposal( + let (proposal_hash2, proposal_index2) = create_custom_proposal!( U256::from(2), frame_system::Call::::set_storage { items: vec![(b"Foobar".to_vec(), 2i32.to_be_bytes().to_vec())], - }, + } ); - let (proposal_hash3, proposal_index3) = create_custom_proposal( + let (proposal_hash3, proposal_index3) = create_custom_proposal!( U256::from(3), frame_system::Call::::set_storage { items: vec![(b"Foobar".to_vec(), 3i32.to_be_bytes().to_vec())], - }, + } ); assert_eq!( Triumvirate::::get(), vec![U256::from(1001), U256::from(1002), U256::from(1003)] ); - vote_aye(U256::from(1001), proposal_hash1, proposal_index1); + vote_aye!(U256::from(1001), proposal_hash1, proposal_index1); - vote_nay(U256::from(1002), proposal_hash2, proposal_index2); - vote_aye(U256::from(1003), proposal_hash2, proposal_index2); + vote_nay!(U256::from(1002), proposal_hash2, proposal_index2); + vote_aye!(U256::from(1003), proposal_hash2, proposal_index2); - vote_nay(U256::from(1001), proposal_hash3, proposal_index3); - vote_aye(U256::from(1002), proposal_hash3, proposal_index3); + vote_nay!(U256::from(1001), proposal_hash3, proposal_index3); + vote_aye!(U256::from(1002), proposal_hash3, proposal_index3); let triumvirate = BoundedVec::truncate_from(vec![U256::from(1001), U256::from(1003), U256::from(1004)]); @@ -310,12 +310,12 @@ fn set_triumvirate_removes_votes_of_outgoing_triumvirate_members() { assert_eq!(Triumvirate::::get(), triumvirate); let voting1 = Voting::::get(proposal_hash1).unwrap(); assert_eq!(voting1.ayes.to_vec(), vec![U256::from(1001)]); - assert_eq!(voting1.nays.to_vec(), vec![]); + assert!(voting1.nays.to_vec().is_empty()); let voting2 = Voting::::get(proposal_hash2).unwrap(); assert_eq!(voting2.ayes.to_vec(), vec![U256::from(1003)]); - assert_eq!(voting2.nays.to_vec(), vec![]); + assert!(voting2.nays.to_vec().is_empty()); let voting3 = Voting::::get(proposal_hash3).unwrap(); - assert_eq!(voting3.ayes.to_vec(), vec![]); + assert!(voting3.ayes.to_vec().is_empty()); assert_eq!(voting3.nays.to_vec(), vec![U256::from(1001)]); assert_eq!( last_event(), @@ -600,10 +600,10 @@ fn propose_with_duplicate_proposal_fails() { #[test] fn propose_with_already_scheduled_proposal_fails() { TestState::default().build_and_execute(|| { - let (proposal_hash, proposal_index) = create_proposal(); + let (proposal_hash, proposal_index) = create_proposal!(); - vote_aye(U256::from(1001), proposal_hash, proposal_index); - vote_aye(U256::from(1002), proposal_hash, proposal_index); + vote_aye!(U256::from(1001), proposal_hash, proposal_index); + vote_aye!(U256::from(1002), proposal_hash, proposal_index); let proposal = Box::new(RuntimeCall::System( frame_system::Call::::set_storage { @@ -665,7 +665,7 @@ fn propose_with_too_many_proposals_fails() { #[test] fn vote_aye_as_first_voter_works() { TestState::default().build_and_execute(|| { - let (proposal_hash, proposal_index) = create_proposal(); + let (proposal_hash, proposal_index) = create_proposal!(); let approve = true; assert_ok!(Pallet::::vote( @@ -677,7 +677,7 @@ fn vote_aye_as_first_voter_works() { let votes = Voting::::get(proposal_hash).unwrap(); assert_eq!(votes.ayes.to_vec(), vec![U256::from(1001)]); - assert_eq!(votes.nays.to_vec(), vec![]); + assert!(votes.nays.to_vec().is_empty()); assert_eq!( last_event(), RuntimeEvent::Governance(Event::::TriumvirateMemberVoted { @@ -694,7 +694,7 @@ fn vote_aye_as_first_voter_works() { #[test] fn vote_nay_as_first_voter_works() { TestState::default().build_and_execute(|| { - let (proposal_hash, proposal_index) = create_proposal(); + let (proposal_hash, proposal_index) = create_proposal!(); let approve = false; assert_ok!(Pallet::::vote( @@ -706,7 +706,7 @@ fn vote_nay_as_first_voter_works() { let votes = Voting::::get(proposal_hash).unwrap(); assert_eq!(votes.nays.to_vec(), vec![U256::from(1001)]); - assert_eq!(votes.ayes.to_vec(), vec![]); + assert!(votes.ayes.to_vec().is_empty()); assert_eq!( last_event(), RuntimeEvent::Governance(Event::::TriumvirateMemberVoted { @@ -723,13 +723,13 @@ fn vote_nay_as_first_voter_works() { #[test] fn vote_can_be_updated() { TestState::default().build_and_execute(|| { - let (proposal_hash, proposal_index) = create_proposal(); + let (proposal_hash, proposal_index) = create_proposal!(); // Vote aye initially - vote_aye(U256::from(1001), proposal_hash, proposal_index); + vote_aye!(U256::from(1001), proposal_hash, proposal_index); let votes = Voting::::get(proposal_hash).unwrap(); assert_eq!(votes.ayes.to_vec(), vec![U256::from(1001)]); - assert_eq!(votes.nays.to_vec(), vec![]); + assert!(votes.nays.to_vec().is_empty()); assert_eq!( last_event(), RuntimeEvent::Governance(Event::::TriumvirateMemberVoted { @@ -742,10 +742,10 @@ fn vote_can_be_updated() { ); // Then vote nay, replacing the aye vote - vote_nay(U256::from(1001), proposal_hash, proposal_index); + vote_nay!(U256::from(1001), proposal_hash, proposal_index); let votes = Voting::::get(proposal_hash).unwrap(); assert_eq!(votes.nays.to_vec(), vec![U256::from(1001)]); - assert_eq!(votes.ayes.to_vec(), vec![]); + assert!(votes.ayes.to_vec().is_empty()); assert_eq!( last_event(), RuntimeEvent::Governance(Event::::TriumvirateMemberVoted { @@ -758,10 +758,10 @@ fn vote_can_be_updated() { ); // Then vote aye again, replacing the nay vote - vote_aye(U256::from(1001), proposal_hash, proposal_index); + vote_aye!(U256::from(1001), proposal_hash, proposal_index); let votes = Voting::::get(proposal_hash).unwrap(); assert_eq!(votes.ayes.to_vec(), vec![U256::from(1001)]); - assert_eq!(votes.nays.to_vec(), vec![]); + assert!(votes.nays.to_vec().is_empty()); assert_eq!( last_event(), RuntimeEvent::Governance(Event::::TriumvirateMemberVoted { @@ -778,13 +778,13 @@ fn vote_can_be_updated() { #[test] fn two_aye_votes_schedule_proposal() { TestState::default().build_and_execute(|| { - let (proposal_hash, proposal_index) = create_proposal(); + let (proposal_hash, proposal_index) = create_proposal!(); - vote_aye(U256::from(1001), proposal_hash, proposal_index); - vote_nay(U256::from(1002), proposal_hash, proposal_index); - vote_aye(U256::from(1003), proposal_hash, proposal_index); + vote_aye!(U256::from(1001), proposal_hash, proposal_index); + vote_nay!(U256::from(1002), proposal_hash, proposal_index); + vote_aye!(U256::from(1003), proposal_hash, proposal_index); - assert_eq!(Proposals::::get(), vec![]); + assert!(Proposals::::get().is_empty()); assert!(!Voting::::contains_key(proposal_hash)); assert_eq!(Scheduled::::get(), vec![proposal_hash]); assert_eq!( @@ -824,15 +824,15 @@ fn two_aye_votes_schedule_proposal() { #[test] fn two_nay_votes_cancel_proposal() { TestState::default().build_and_execute(|| { - let (proposal_hash, proposal_index) = create_proposal(); + let (proposal_hash, proposal_index) = create_proposal!(); - vote_nay(U256::from(1001), proposal_hash, proposal_index); - vote_aye(U256::from(1002), proposal_hash, proposal_index); - vote_nay(U256::from(1003), proposal_hash, proposal_index); + vote_nay!(U256::from(1001), proposal_hash, proposal_index); + vote_aye!(U256::from(1002), proposal_hash, proposal_index); + vote_nay!(U256::from(1003), proposal_hash, proposal_index); - assert_eq!(Proposals::::get(), vec![]); + assert!(Proposals::::get().is_empty()); assert!(!Voting::::contains_key(proposal_hash)); - assert_eq!(Scheduled::::get(), vec![]); + assert!(Scheduled::::get().is_empty()); assert_eq!(ProposalOf::::get(proposal_hash), None); let events = last_n_events(2); assert_eq!( @@ -855,7 +855,7 @@ fn two_nay_votes_cancel_proposal() { #[test] fn vote_as_bad_origin_fails() { TestState::default().build_and_execute(|| { - let (proposal_hash, proposal_index) = create_proposal(); + let (proposal_hash, proposal_index) = create_proposal!(); assert_noop!( Pallet::::vote(RuntimeOrigin::root(), proposal_hash, proposal_index, true), @@ -871,7 +871,7 @@ fn vote_as_bad_origin_fails() { #[test] fn vote_as_non_triumvirate_member_fails() { TestState::default().build_and_execute(|| { - let (proposal_hash, proposal_index) = create_proposal(); + let (proposal_hash, proposal_index) = create_proposal!(); assert_noop!( Pallet::::vote( @@ -905,12 +905,12 @@ fn vote_on_missing_proposal_fails() { #[test] fn vote_on_scheduled_proposal_fails() { TestState::default().build_and_execute(|| { - let (proposal_hash, proposal_index) = create_proposal(); + let (proposal_hash, proposal_index) = create_proposal!(); - vote_aye(U256::from(1001), proposal_hash, proposal_index); - vote_aye(U256::from(1002), proposal_hash, proposal_index); + vote_aye!(U256::from(1001), proposal_hash, proposal_index); + vote_aye!(U256::from(1002), proposal_hash, proposal_index); - assert_eq!(Proposals::::get(), vec![]); + assert!(Proposals::::get().is_empty()); assert_eq!(Scheduled::::get(), vec![proposal_hash]); assert_noop!( @@ -928,7 +928,7 @@ fn vote_on_scheduled_proposal_fails() { #[test] fn vote_on_proposal_with_wrong_index_fails() { TestState::default().build_and_execute(|| { - let (proposal_hash, proposal_index) = create_proposal(); + let (proposal_hash, proposal_index) = create_proposal!(); assert_noop!( Pallet::::vote( @@ -945,7 +945,7 @@ fn vote_on_proposal_with_wrong_index_fails() { #[test] fn duplicate_vote_on_proposal_already_voted_fails() { TestState::default().build_and_execute(|| { - let (proposal_hash, proposal_index) = create_proposal(); + let (proposal_hash, proposal_index) = create_proposal!(); let aye_voter = RuntimeOrigin::signed(U256::from(1001)); let approve = true; @@ -980,19 +980,19 @@ fn aye_vote_on_proposal_with_too_many_scheduled_fails() { TestState::default().build_and_execute(|| { // We fill the scheduled proposals up to the maximum. for i in 0..MaxScheduled::get() { - let (proposal_hash, proposal_index) = create_custom_proposal( + let (proposal_hash, proposal_index) = create_custom_proposal!( U256::from(1), frame_system::Call::::set_storage { items: vec![(b"Foobar".to_vec(), i.to_be_bytes().to_vec())], - }, + } ); - vote_aye(U256::from(1001), proposal_hash, proposal_index); - vote_aye(U256::from(1002), proposal_hash, proposal_index); + vote_aye!(U256::from(1001), proposal_hash, proposal_index); + vote_aye!(U256::from(1002), proposal_hash, proposal_index); } - let (proposal_hash, proposal_index) = create_proposal(); + let (proposal_hash, proposal_index) = create_proposal!(); - vote_aye(U256::from(1001), proposal_hash, proposal_index); + vote_aye!(U256::from(1001), proposal_hash, proposal_index); assert_noop!( Pallet::::vote( RuntimeOrigin::signed(U256::from(1002)), @@ -1008,7 +1008,7 @@ fn aye_vote_on_proposal_with_too_many_scheduled_fails() { #[test] fn collective_vote_from_non_collective_member_fails() { TestState::default().build_and_execute(|| { - let (proposal_hash, proposal_index) = create_scheduled_proposal(); + let (proposal_hash, proposal_index) = create_scheduled_proposal!(); assert_noop!( Pallet::::collective_vote( @@ -1025,7 +1025,7 @@ fn collective_vote_from_non_collective_member_fails() { #[test] fn collective_vote_on_non_scheduled_proposal_fails() { TestState::default().build_and_execute(|| { - let (proposal_hash, proposal_index) = create_proposal(); + let (proposal_hash, proposal_index) = create_proposal!(); assert_noop!( Pallet::::collective_vote( @@ -1042,7 +1042,7 @@ fn collective_vote_on_non_scheduled_proposal_fails() { #[test] fn collective_vote_on_proposal_with_wrong_index_fails() { TestState::default().build_and_execute(|| { - let (proposal_hash, _proposal_index) = create_scheduled_proposal(); + let (proposal_hash, _proposal_index) = create_scheduled_proposal!(); assert_noop!( Pallet::::collective_vote( @@ -1059,7 +1059,7 @@ fn collective_vote_on_proposal_with_wrong_index_fails() { #[test] fn duplicate_collective_vote_on_scheduled_proposal_already_voted_fails() { TestState::default().build_and_execute(|| { - let (proposal_hash, proposal_index) = create_scheduled_proposal(); + let (proposal_hash, proposal_index) = create_scheduled_proposal!(); let aye_voter = RuntimeOrigin::signed(U256::from(2001)); let approve = true; @@ -1089,6 +1089,279 @@ fn duplicate_collective_vote_on_scheduled_proposal_already_voted_fails() { }); } +#[test] +fn basic_collective_aye_vote_on_scheduled_proposal_works() { + TestState::default().build_and_execute(|| { + let (proposal_hash, proposal_index) = create_scheduled_proposal!(); + + // Add an aye vote from an economic collective member. + assert_ok!(Pallet::::collective_vote( + RuntimeOrigin::signed(U256::from(2001)), + proposal_hash, + proposal_index, + true + )); + + assert_eq!( + CollectiveVoting::::get(proposal_hash), + Some(CollectiveVotes { + index: proposal_index, + economic_ayes: BoundedVec::truncate_from(vec![U256::from(2001)]), + economic_nays: BoundedVec::new(), + building_ayes: BoundedVec::new(), + building_nays: BoundedVec::new(), + }) + ); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::CollectiveMemberVoted { + account: CollectiveMember::Economic(U256::from(2001)), + proposal_hash, + voted: true, + economic_yes: 1, + economic_no: 0, + building_yes: 0, + building_no: 0, + }) + ); + + // Add a second aye vote from a building collective member. + assert_ok!(Pallet::::collective_vote( + RuntimeOrigin::signed(U256::from(3001)), + proposal_hash, + proposal_index, + true + )); + + assert_eq!( + CollectiveVoting::::get(proposal_hash), + Some(CollectiveVotes { + index: proposal_index, + economic_ayes: BoundedVec::truncate_from(vec![U256::from(2001)]), + economic_nays: BoundedVec::new(), + building_ayes: BoundedVec::truncate_from(vec![U256::from(3001)]), + building_nays: BoundedVec::new(), + }) + ); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::CollectiveMemberVoted { + account: CollectiveMember::Building(U256::from(3001)), + proposal_hash, + voted: true, + economic_yes: 1, + economic_no: 0, + building_yes: 1, + building_no: 0, + }) + ); + }); +} + +#[test] +fn collective_vote_can_be_updated() { + TestState::default().build_and_execute(|| { + let (proposal_hash, proposal_index) = create_scheduled_proposal!(); + let economic_member = U256::from(2001); + + // Vote aye initially as an economic collective member + collective_vote_aye!(economic_member, proposal_hash, proposal_index); + let votes = CollectiveVoting::::get(proposal_hash).unwrap(); + assert_eq!(votes.economic_ayes.to_vec(), vec![economic_member]); + assert!(votes.economic_nays.to_vec().is_empty()); + assert!(votes.building_ayes.to_vec().is_empty()); + assert!(votes.building_nays.to_vec().is_empty()); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::CollectiveMemberVoted { + account: CollectiveMember::Economic(economic_member), + proposal_hash, + voted: true, + economic_yes: 1, + economic_no: 0, + building_yes: 0, + building_no: 0, + }) + ); + + // Then vote nay, replacing the aye vote + collective_vote_nay!(economic_member, proposal_hash, proposal_index); + let votes = CollectiveVoting::::get(proposal_hash).unwrap(); + assert!(votes.economic_ayes.to_vec().is_empty()); + assert_eq!(votes.economic_nays.to_vec(), vec![economic_member]); + assert!(votes.building_ayes.to_vec().is_empty()); + assert!(votes.building_nays.to_vec().is_empty()); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::CollectiveMemberVoted { + account: CollectiveMember::Economic(economic_member), + proposal_hash, + voted: false, + economic_yes: 0, + economic_no: 1, + building_yes: 0, + building_no: 0, + }) + ); + + // Then vote aye again, replacing the nay vote + collective_vote_aye!(economic_member, proposal_hash, proposal_index); + let votes = CollectiveVoting::::get(proposal_hash).unwrap(); + assert_eq!(votes.economic_ayes.to_vec(), vec![economic_member]); + assert!(votes.economic_nays.to_vec().is_empty()); + assert!(votes.building_ayes.to_vec().is_empty()); + assert!(votes.building_nays.to_vec().is_empty()); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::CollectiveMemberVoted { + account: CollectiveMember::Economic(economic_member), + proposal_hash, + voted: true, + economic_yes: 1, + economic_no: 0, + building_yes: 0, + building_no: 0, + }) + ); + + println!( + "{:?}", + pallet_scheduler::Agenda::::iter().collect::>() + ); + run_to_block(frame_system::Pallet::::block_number() + 100); + println!( + "{:?}", + pallet_scheduler::Agenda::::iter().collect::>() + ); + + let (proposal_hash, proposal_index) = create_scheduled_proposal!(); + let building_member = U256::from(3001); + + // Vote aye initially as a building collective member + collective_vote_aye!(building_member, proposal_hash, proposal_index); + let votes = CollectiveVoting::::get(proposal_hash).unwrap(); + assert!(votes.economic_ayes.to_vec().is_empty()); + assert!(votes.economic_nays.to_vec().is_empty()); + assert_eq!(votes.building_ayes.to_vec(), vec![building_member]); + assert!(votes.building_nays.to_vec().is_empty()); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::CollectiveMemberVoted { + account: CollectiveMember::Building(building_member), + proposal_hash, + voted: true, + economic_yes: 0, + economic_no: 0, + building_yes: 1, + building_no: 0, + }) + ); + + // Then vote nay, replacing the aye vote + collective_vote_nay!(building_member, proposal_hash, proposal_index); + let votes = CollectiveVoting::::get(proposal_hash).unwrap(); + assert!(votes.economic_ayes.to_vec().is_empty()); + assert!(votes.economic_nays.to_vec().is_empty()); + assert!(votes.building_ayes.to_vec().is_empty()); + assert_eq!(votes.building_nays.to_vec(), vec![building_member]); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::CollectiveMemberVoted { + account: CollectiveMember::Building(building_member), + proposal_hash, + voted: false, + economic_yes: 0, + economic_no: 0, + building_yes: 0, + building_no: 1, + }) + ); + + // Then vote aye again, replacing the nay vote + collective_vote_aye!(building_member, proposal_hash, proposal_index); + let votes = CollectiveVoting::::get(proposal_hash).unwrap(); + assert!(votes.economic_ayes.to_vec().is_empty()); + assert!(votes.economic_nays.to_vec().is_empty()); + assert_eq!(votes.building_ayes.to_vec(), vec![building_member]); + assert!(votes.building_nays.to_vec().is_empty()); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::CollectiveMemberVoted { + account: CollectiveMember::Building(building_member), + proposal_hash, + voted: true, + economic_yes: 0, + economic_no: 0, + building_yes: 1, + building_no: 0, + }) + ); + }); +} + +#[test] +fn collective_aye_votes_to_threshold_on_scheduled_proposal_fast_tracks() { + fn execute_for( + collective: impl IntoIterator::AccountId>, + collective_size: u32, + ) { + let (proposal_hash, proposal_index) = create_scheduled_proposal!(); + let threshold = FastTrackThreshold::get().mul_ceil(collective_size); + + for member in collective.into_iter().take(threshold as usize) { + collective_vote_aye!(member, proposal_hash, proposal_index); + } + + assert!(Scheduled::::get().is_empty()); + assert_eq!(CollectiveVoting::::get(proposal_hash), None); + let task_name: [u8; 32] = proposal_hash.as_ref().try_into().unwrap(); + let now = frame_system::Pallet::::block_number(); + assert_eq!( + pallet_scheduler::Lookup::::get(task_name).unwrap().0, + now + 1 + ); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::ScheduledProposalFastTracked { proposal_hash }) + ); + } + + TestState::default().build_and_execute(|| { + execute_for(EconomicCollective::::get(), ECONOMIC_COLLECTIVE_SIZE); + run_to_block(frame_system::Pallet::::block_number() + 1); + execute_for(BuildingCollective::::get(), BUILDING_COLLECTIVE_SIZE); + }); +} + +#[test] +fn collective_nay_votes_to_threshold_on_scheduled_proposal_cancels() { + fn execute_for( + collective: impl IntoIterator::AccountId>, + collective_size: u32, + ) { + let (proposal_hash, proposal_index) = create_scheduled_proposal!(); + let threshold = CancellationThreshold::get().mul_ceil(collective_size); + + for member in collective.into_iter().take(threshold as usize) { + collective_vote_nay!(member, proposal_hash, proposal_index); + } + + assert!(Scheduled::::get().is_empty()); + assert!(CollectiveVoting::::get(proposal_hash).is_none()); + let task_name: [u8; 32] = proposal_hash.as_ref().try_into().unwrap(); + assert!(pallet_scheduler::Lookup::::get(task_name).is_none()); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::ScheduledProposalCancelled { proposal_hash }) + ); + } + + TestState::default().build_and_execute(|| { + execute_for(EconomicCollective::::get(), ECONOMIC_COLLECTIVE_SIZE); + execute_for(BuildingCollective::::get(), BUILDING_COLLECTIVE_SIZE); + }); +} + #[test] fn collective_rotation_works() { TestState::default().build_and_execute(|| { @@ -1116,8 +1389,8 @@ fn collective_rotation_works() { next_building_collective ); - set_next_economic_collective(next_economic_collective.clone()); - set_next_building_collective(next_building_collective.clone()); + set_next_economic_collective!(next_economic_collective.clone()); + set_next_building_collective!(next_building_collective.clone()); run_to_block(CollectiveRotationPeriod::get()); @@ -1132,63 +1405,90 @@ fn collective_rotation_works() { }); } -fn create_custom_proposal( - proposer: U256, - call: impl Into>, -) -> (::Hash, u32) { - let proposal = Box::new(call.into()); - let length_bound = proposal.encoded_size() as u32; - let proposal_hash = ::Hashing::hash_of(&proposal); - let proposal_index = ProposalCount::::get(); - - assert_ok!(Pallet::::propose( - RuntimeOrigin::signed(proposer), - proposal.clone(), - length_bound - )); - - (proposal_hash, proposal_index) +#[macro_export] +macro_rules! create_custom_proposal { + ($proposer:expr, $call:expr) => {{ + let proposal: Box<::RuntimeCall> = Box::new($call.into()); + let length_bound = proposal.encoded_size() as u32; + let proposal_hash = ::Hashing::hash_of(&proposal); + let proposal_index = ProposalCount::::get(); + + assert_ok!(Pallet::::propose( + RuntimeOrigin::signed($proposer), + proposal.clone(), + length_bound + )); + + (proposal_hash, proposal_index) + }}; } -fn create_proposal() -> (::Hash, u32) { - create_custom_proposal( - U256::from(1), - frame_system::Call::::set_storage { - items: vec![(b"Foobar".to_vec(), 42u32.to_be_bytes().to_vec())], - }, - ) +#[macro_export] +macro_rules! create_proposal { + () => {{ + create_custom_proposal!( + U256::from(1), + frame_system::Call::::set_storage { + items: vec![(b"Foobar".to_vec(), 42u32.to_be_bytes().to_vec())], + } + ) + }}; } -fn create_scheduled_proposal() -> (::Hash, u32) { - let (proposal_hash, proposal_index) = create_proposal(); - vote_aye(U256::from(1001), proposal_hash, proposal_index); - vote_aye(U256::from(1002), proposal_hash, proposal_index); +#[macro_export] +macro_rules! create_scheduled_proposal { + () => {{ + let (proposal_hash, proposal_index) = create_proposal!(); + vote_aye!(U256::from(1001), proposal_hash, proposal_index); + vote_aye!(U256::from(1002), proposal_hash, proposal_index); + (proposal_hash, proposal_index) + }}; +} - (proposal_hash, proposal_index) +#[macro_export] +macro_rules! vote_aye { + ($voter:expr, $proposal_hash:expr, $proposal_index:expr) => {{ + assert_ok!(Pallet::::vote( + RuntimeOrigin::signed($voter), + $proposal_hash, + $proposal_index, + true + )); + }}; } -fn vote_aye( - voter: U256, - proposal_hash: ::Hash, - proposal_index: u32, -) { - assert_ok!(Pallet::::vote( - RuntimeOrigin::signed(voter), - proposal_hash, - proposal_index, - true - )); +#[macro_export] +macro_rules! vote_nay { + ($voter:expr, $proposal_hash:expr, $proposal_index:expr) => {{ + assert_ok!(Pallet::::vote( + RuntimeOrigin::signed($voter), + $proposal_hash, + $proposal_index, + false + )); + }}; } -fn vote_nay( - voter: U256, - proposal_hash: ::Hash, - proposal_index: u32, -) { - assert_ok!(Pallet::::vote( - RuntimeOrigin::signed(voter), - proposal_hash, - proposal_index, - false - )); +#[macro_export] +macro_rules! collective_vote_aye { + ($voter:expr, $proposal_hash:expr, $proposal_index:expr) => {{ + assert_ok!(Pallet::::collective_vote( + RuntimeOrigin::signed($voter), + $proposal_hash, + $proposal_index, + true + )); + }}; +} + +#[macro_export] +macro_rules! collective_vote_nay { + ($voter:expr, $proposal_hash:expr, $proposal_index:expr) => {{ + assert_ok!(Pallet::::collective_vote( + RuntimeOrigin::signed($voter), + $proposal_hash, + $proposal_index, + false + )); + }}; } From 4eecd139ec765379b17a2ab7f622297028daa656 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Fri, 7 Nov 2025 12:58:32 -0300 Subject: [PATCH 16/23] added cleanup logic + readme --- pallets/governance/README.md | 247 +++++++++++++------------------- pallets/governance/src/lib.rs | 112 ++++++++++++--- pallets/governance/src/mock.rs | 2 + pallets/governance/src/tests.rs | 25 ++-- 4 files changed, 206 insertions(+), 180 deletions(-) diff --git a/pallets/governance/README.md b/pallets/governance/README.md index 81c29c2c9c..86bb1b08ad 100644 --- a/pallets/governance/README.md +++ b/pallets/governance/README.md @@ -2,17 +2,16 @@ ## Abstract -This proposes a comprehensive on-chain governance system to replace the current broken governance implementation that relies on a sudo-based triumvirate multisig. The new system introduces a separation of powers model with three key components: (1) an Opentensor Foundation (OTF) account authorized to propose runtime upgrades, (2) a three-member Triumvirate that votes on proposals, and (3) two collective bodies (Economic Power and Building Power) that can delay or cancel proposals and replace Triumvirate members through a removal and appointment process. The system will be deployed in two phases: first coexisting with the current sudo implementation for validation, then fully replacing it. +This proposes a comprehensive on-chain governance system to replace the current broken governance implementation that relies on a sudo-based triumvirate multisig. The new system introduces a separation of powers model with three key components: (1) multiple proposer accounts (mostly controlled by OTF) to submit proposals (call executed with root privilege), (2) a three-member Triumvirate that votes on proposals, and (3) two collective bodies (Economic Power and Building Power) that can delay, cancel, or fast-track proposals and vote to replace Triumvirate members. The system will be deployed in two phases: first coexisting with the current sudo implementation for validation, then fully replacing it. ## Motivation The current governance system in Subtensor is broken and relies entirely on a triumvirate multisig with sudo privileges. The runtime contains dead code related to the original triumvirate collective and senate that no longer functions properly. This centralized approach creates several critical issues: -1. **Single Point of Failure**: The sudo key represents a concentration of power with no on-chain checks or balances. -2. **Lack of Transparency**: Off-chain multisig decisions are not recorded or auditable on-chain. +1. **Single Point of Failure**: The sudo key represents a concentration of power with no on-chain checks or balances (i.e., no blockchain-enforced voting, approval, or oversight mechanisms). +2. **Lack of Transparency**: The governance decision-making process (who voted, when, on what proposal) happens off-chain and is not recorded or auditable on-chain. While the multisig signature itself provides cryptographic proof that the threshold was met, the governance process leading to that decision is opaque. 3. **No Stakeholder Representation**: Major stakeholders (validators and subnet owners) have no formal mechanism to influence protocol upgrades. 4. **Technical Debt**: Dead governance code in the runtime creates maintenance burden and confusion. -5. **Trust Requirements**: The community must trust the multisig holders without cryptographic guarantees or accountability. This proposal addresses these issues by implementing a proper separation of powers that balances efficiency with stakeholder representation, while maintaining upgrade capability and security. @@ -20,215 +19,165 @@ This proposal addresses these issues by implementing a proper separation of powe ### Overview -The governance system consists of three main components working together: +The governance system consists of three main actors working together: -1. **Proposal Origin**: OTF-authorized account(s) -2. **Approval Body**: Triumvirate (3 members) -3. **Oversight Bodies**: Economic Power Collective (top 16 validators by total stake) and Building Power Collective (top 16 subnet owners by moving average price) +1. **Allowed Proposers**: Accounts authorized to submit proposals (mostly controlled by OTF) +2. **Triumvirate**: Approval body of 3 members that vote on proposals +3. **Economic and Building Collectives**: Oversight bodies representing major stakeholders: top 16 validators by total stake and top 16 subnet owners by moving average price respectively ### Actors and Roles -#### Opentensor Foundation (OTF) Accounts +#### Allowed Proposers (mostly OTF-controlled) -- **Purpose**: Authorized to create runtime upgrade proposals -- **Assignment**: OTF account key(s) are configured in the runtime via governance -- **Permissions**: Can submit proposals to the main governance track -- **Constraints**: Cannot approve proposals; only the Triumvirate can approve +- **Purpose**: Authorized to submit proposals (calls executed with root privilege) +- **Assignment**: Allowed proposer account keys are configured in the runtime via governance +- **Permissions**: + - Can submit proposals to the main governance track (i.e., runtime upgrade proposals or any root extrinsic) + - Can cancel or withdraw their own proposals anytime before execution (i.e., if they find a bug in the proposal code) + - Can eject its own key from the allowed proposers list (i.e., if it is lost or compromised) + - Can propose an update to the allowed proposers list via proposal flow **Open Questions:** -- Q1: How many OTF accounts should be authorized initially? Single account or multiple? **Multiple because safe, no power except to make proposal, one for Sam and one for other team member.** -- Q2: What happens if OTF account is compromised/lost? Can it be revoked immediately or requires full governance process? **Full governance process** -- Q3: Only one proposal active at a time? Or multiple? Different track for upgrade? **Multiple proposal at the same time but only one get through, other are cancelled** -- Q4: Who can add/remove OTF accounts? Only governance or should Triumvirate have emergency powers? -- Q5: What types of proposals can OTF submit? Only runtime upgrades or any root extrinsic? **All type of calls** -- Q6: Who validates that proposal code matches stated intent before Triumvirate votes? Share runtime WASM hash like Polkadot fellowship does? -- Q7: Would it make sense to have an extrinsic to kick the calling OTF key to avoid compromised key to submit proposals? +- Q1: Who can add/remove proposer accounts? Only governance or should Triumvirate have emergency powers? +- Q2: Who validates that proposal code matches stated intent before Triumvirate votes? Share runtime WASM hash like Polkadot fellowship does? #### Triumvirate -- **Composition**: 3 distinct accounts/seats (must always maintain 3 members) -- **Role**: Vote on proposals submitted by OTF accounts +- **Composition**: 3 distinct accounts (must always maintain 3 members) +- **Role**: Vote on proposals submitted by allowed proposers - **Voting Threshold**: 2-of-3 approval required for proposals to pass -- **Term**: Indefinite, subject to replacement by collective vote -- **Accountability**: Each seat can be replaced through collective vote process (see Replacement Mechanism) +- **Term**: Indefinite, subject to replacement by collective vote every 6 months (configurable) +- **Accountability**: Each member can be replaced through collective vote process (see Replacement Mechanism) +- **Permissions**: + - Can vote on proposals submitted by allowed proposers **Open Questions:** -- Q8: How are initial Triumvirate members selected? **Current triumvirate** -- Q9: When a member is being replaced, how is the new member selected? List of on-chain potential candidates? **Randomly from economic power collective or building power collective** -- Q10: Should Triumvirate members be known/doxxed or can they be anonymous? -- Q11: What happens if a Triumvirate member goes inactive for extended periods? **They need to accept the nomination or we rerun the nomination** -- Q12: Can Triumvirate members also be in collectives (conflict of interest)? -- Q13: What's the deadline for Triumvirate to vote? Can proposals expire? - -#### Economic Power Collective - -- **Composition**: Top 20 validators by total stake -- **Recalculation**: Membership refreshed every 2 months (432,000 blocks) -- **Powers**: - - Delay or cancel proposals approved by Triumvirate - - Replace one Triumvirate member every 6 months via single atomic vote (remove current holder + install replacement candidate, with rotating seat selection) - + - Q3: How to allow a triumvirate member to resign? + +#### Economic and Building Collectives + +- **Economic Collective**: Top 16 validators by total stake (including delegated stake) (configurable) +- **Building Collective**: Top 16 subnet owners by moving average price (with minimum age of 6 months) (configurable) +- **Recalculation**: Membership refreshed every 6 months (configurable) +- **Permissions**: + - Can vote aye/nay on proposals submitted by allowed proposers and approved by Triumvirate + - More than 2/3 of aye vote for any collective fast tracks the proposal (next block execution) (threshold configurable) + - More than 1/2 of nay vote for any collective cancels the proposal (threshold configurable) + - Nays votes accumulate and delay the proposal execution exponentially until cancellation (see Delay Period section) + - Can replace a Triumvirate member every 6 months via single atomic vote (remove current holder + install replacement candidate, with rotating seat selection) + - Can mark himself as eligible for nomination to the Triumvirate + - Can accept a nomination to the Triumvirate + **Open Questions:** -- Q14: "Total stake" - does this include delegated stake or only self-bonded? **Includes delegated stake** -- Q15: Should there be a minimum stake threshold to enter collective? **Given we select top N, should be enough to be an implicit minimum** -- Q16: What happens if validator drops out of top 20 mid-term? Immediate removal or wait for refresh? **Keep their spot until next refresh** -- Q18: Can a validator be in both Economic and Building collectives if they also own top subnet? **Yes, although imply a different key** - -#### Building Power Collective - -- **Composition**: Top 20 subnet owners by moving average (MA) price -- **Recalculation**: Membership refreshed every 2 months (432,000 blocks) -- **Powers**: - - Delay or cancel proposals approved by Triumvirate - - Replace one Triumvirate member every 6 months via single atomic vote (remove current holder + install replacement candidate, with rotating seat selection) - -**Open Questions:** -- Q19: What if subnet ownership transfers? Does collective seat transfer or recalculated when rotation happens? -- Q20: Should there be minimum subnet age requirement (prevent fresh subnets from voting)? **Maybe 3 or 4 months, or half a year, configurable** -- Q21: What if subnet is deregistered mid-term? Immediate collective removal? -- Q22: Can one entity own multiple subnets and occupy multiple collective seats? If not, how to prevent that? **Unique key only allowed on a collective** +- Q4: How to handle the nomination process? +- Q5: How to incentivize the collective members to vote? ### Governance Process Flow #### Proposal Submission -1. OTF account creates a proposal containing runtime upgrade or any root extrinsic +1. An allowed proposer account submits a proposal containing runtime upgrade or any root extrinsic 2. Proposal enters "Triumvirate Voting" phase -3. Voting period: 7 days (50,400 blocks) +3. Voting period: 7 days (configurable), after this period, the proposal is automatically rejected if not approved by the Triumvirate. -**Open Questions:** -- Q23: Can OTF cancel/withdraw a proposal after submission? What if they find a bug? -- Q24: Is there a queue limit? -- Q25: Who pays for proposal storage/execution? OTF, treasury, or included in proposal? +- There is a queue limit in the number of proposals that can be submitted at the same time (configurable) +- Proposal can be cancelled by the proposer before the final execution for security reasons (e.g., if they find a bug in the proposal code). +- An allowed proposer can eject its own key from the allowed proposers, removing all its submitted proposals waiting for triumvirate approval from the queue. #### Triumvirate Approval -1. Triumvirate members cast votes (Aye/Nay) on the proposal -2. Requirement: At least 2 of 3 members must approve -3. If approved: Proposal enters "Delay Period" -4. If rejected: Proposal fails and is archived +1. Triumvirate members cast votes (aye/nay) on the proposal + - 2/3 vote aye, proposal is approved: Proposal is scheduled for execution in 7 days (configurable) and enters "Delay Period" + - 2/3 vote nay, proposal is rejected: Proposal is cleaned up from storage (it was never scheduled for execution). -**Open Questions:** -- Q26: What happens if only 1 of 3 members votes within 7 days? Proposal cancels? -- Q27: Can Triumvirate members change their vote before voting period ends? -- Q28: Should there be a veto power for individual Triumvirate members for emergency stops? +- Triumvirate members can change their vote during the voting period (before the proposal is scheduled or cancelled). +- There is a queue limit in the number of scheduled proposals and in the delay period (configurable). +- If a triumvirate member is replaced, all his votes are removed from the active proposals. #### Delay Period (Collective Oversight) -1. Initial Duration: 7 days (50,400 blocks) -2. Both collectives can vote to delay/cancel -3. Each collective member can cast a "Delay" vote -4. Delay votes accumulate with cumulative time delays: - - Vote 1: +12 hours (3,600 blocks at 12s/block) - - Vote 2: +1 day (7,200 blocks) - - Vote 3: +2 days (14,400 blocks) - - Vote 4: +4 days (28,800 blocks) - - Vote 5: +8 days (57,600 blocks) - - Vote 6: +16 days (115,200 blocks) - - Vote 7: +30 days (216,000 blocks) - - Vote 8: +60 days (432,000 blocks) -5. Cancellation threshold: If 9 delay votes are cast within a single collective -6. If cancelled: Proposal is terminated -7. If delay period expires without cancellation: Proposal executes automatically +When a proposal has been approved by the Triumvirate, it is scheduled in 7 days (configurable) and enters the "Delay Period" where the Economic and Building Collectives can vote to delay, cancel or fast-track the proposal. + +1. Both collectives can vote aye/nay on the proposal +2. Delay is an exponential function of the number of nays votes, set to 1.5^n (configurable). + - Initial delay is 7 days (configurable). + - After 1 nays vote, the delay is 1.5^1 * 7 days = 10.5 days. + - After 2 nays votes, the delay is 1.5^2 * 7 days = ~16 days. + - After 3 nays votes, the delay is 1.5^3 * 7 days = ~23 days. + - After 4 nays votes, the delay is 1.5^4 * 7 days = ~35 days. + - After 5 nays votes, the delay is 1.5^5 * 7 days = ~53 days. + - After 6 nays votes, the delay is 1.5^6 * 7 days = ~80 days. + - After 7 nays votes, the delay is 1.5^7 * 7 days = ~120 days. + - After 8 nays votes, the delay is 1.5^8 * 7 days = ~180 days. + - After 9 nays votes, proposal is cancelled (given we have a collective size of 16, hence more than 1/2 of the collective votes nay). +3. If the delay period expires without cancellation: Proposal executes automatically + +- The delay is calculated based on the collective with the most nays votes (i.e., if Economic has 3 nays and Building has 1 nay, the delay is based on 3 nays = ~23 days). +- More than 2/3 of aye vote for any collective fast tracks the proposal (next block execution) (threshold configurable) +- More than 1/2 of nay vote for any collective cancels the proposal (threshold configurable) +- Collective members can change their vote during the delay period. If changing a nay vote to aye reduces the delay below the time already elapsed, the proposal executes immediately. + - **Example**: A proposal has 3 nays votes, creating a 23-day delay. After 17 days have elapsed, a collective member changes their nay vote to aye, reducing the delay to 16 days. Since 17 days have already passed (more than the new 16-day delay), the proposal executes immediately. **Open Questions:** -- Q29: Are cumulative delays applied per-collective or across both collectives combined? -- Q30: Can collective members change their delay vote during the delay period? -- Q31: Should "Delay" votes require justification/reason on-chain? -- Q32: Can members vote "Support" (opposite of delay) to counter delay votes? -- Q33: Does EITHER collective reaching 9 votes cancel, or BOTH needed? +- Q6: Should the voting be across both collectives or each collective votes independently? What if a collective decide to go rogue and fast track proposals that the other collective is against or vice versa? #### Execution -- Successful proposals execute automatically after the delay period -- Execution applies runtime upgrade or execute extrinsic -- Execution event is recorded on-chain - -**Open Questions:** -- Q34: What if execution fails due to runtime error? Who is responsible to fix? -- Q35: Can execution be delayed further if critical issue discovered on day 13? -- Q36: Should there be a final "confirm execution" step by OTF or Triumvirate? -- Q37: What if network is congested and execution can't fit in block? +- Proposals executed automatically after the delay period if not cancelled or when fast-tracked by the collectives. +- If executing fails, the proposal is not retried and is cleaned up from storage. ### Triumvirate Replacement Mechanism -Each collective can replace one Triumvirate member every 6 months through a **single atomic vote**: the collective votes to replace the current seat holder with a specific new candidate. If the vote succeeds, the replacement happens immediately. The Triumvirate always maintains exactly 3 active members. +Each collective can replace one Triumvirate member every 6 months through a **single atomic vote**: the collective votes to replace the current seat holder with a randomly selected new candidate from the eligible candidates. If the vote succeeds, the replacement happens immediately. The Triumvirate always maintains exactly 3 active members. #### Timing -- Each collective can initiate replacement vote every 6 months (1,296,800 blocks) -- Economic and Building collectives have independent 6-month cycles -- Cooldown timer starts after vote completion (whether successful or failed) +- Each collective can initiate replacement vote every 6 months (configurable) +- Economic and Building collectives have independent cycles (seat are rotated independently) **Open Questions:** -- Q38: Does the 6-month timer start from genesis, from last replacement attempt, or last successful replacement? -- Q39: Can replacement be initiated early in emergency situations? -- Q40: Can a replaced member be voted back in immediately, or should there be a cooldown period? -- Q41: Should failed replacement attempts have a shorter cooldown (e.g., 1 month retry)? +- Q7: How to have an emergency replacement vote? +- Q8: Can a replaced member be voted back in immediately, or should there be a cooldown period? #### Rotating Seat Selection - Triumvirate seats are numbered: Seat 0, Seat 1, Seat 2 -- Each collective maintains an automatic rotation index +- Each collective maintains an independent rotation index that determines which seat they target: - Economic Power automatically targets the next seat in rotation: - If last removal was Seat 0, next automatically targets Seat 1 - If last removal was Seat 1, next automatically targets Seat 2 - If last removal was Seat 2, next automatically targets Seat 0 - Building Power has independent automatic rotation - Rotation ensures no single seat is disproportionately targeted -- Collective members cannot choose which seat to target - it's determined automatically - -**Open Questions:** -- Q42: Should rotation reset if removal fails, or continue regardless? +- Collective members cannot choose which seat to target: it's determined automatically #### Replacement Process (Single Atomic Vote) -The replacement happens in a single vote where the collective votes **both** to remove the current seat holder **and** to install a specific replacement candidate. This is an atomic operation - either both happen or neither happens. +The replacement happens in a single vote where the collective votes **both** to remove the current seat holder **and** to install a specific replacement candidate. This is an atomic operation: either both happen or neither happens. **Process:** -1. **Proposal Phase**: Any collective member can propose a replacement by submitting: - - Replacement candidate account - - Optional: Justification text - -2. **Voting Phase**: - - All collective members vote Aye/Nay on the replacement proposal - - Threshold: Simple majority (11 of 20 members) - - Voting period: 7 days (50,400 blocks) - +1. **Eligibility Phase**: Collective members can mark themselves as eligible for nomination to the Triumvirate. +2. **Voting Phase**: Collective members can vote aye/nay during the voting period to replace the current seat holder. + - Threshold of more than 1/2 of the collective size (configurable) - **If vote succeeds**: Current seat holder immediately removed, replacement candidate immediately installed - - **If vote fails**: No change, current member remains, cooldown timer starts - -4. **Transition**: Atomic swap ensures Triumvirate always has exactly 3 members with no vacancy period - -**Open Questions:** -- Q43: From where the candidate is selected? -- Q44: Can multiple replacement proposals be submitted for the same cycle? First-come-first-served or best candidate wins? -- Q45: Can replacement vote be vetoed by OTF in emergency situations? -- Q46: What happens to in-flight proposals where replaced member already voted? -- Q47: Can a replaced member be immediately proposed as replacement for a different seat? -- Q48: Who can propose replacement candidates? Any collective member or requires threshold support? -- Q49: Should there be a minimum vetting period between proposal and voting? + - **If vote fails**: No change, current member remains. +3. **Selection Phase**: The replacement candidate is selected randomly from the eligible candidates. +4. **Validation Phase**: The replacement candidate validates their nomination on-chain to avoid nominating inactive members. +5. **Transition**: Atomic swap ensures Triumvirate always has exactly 3 members with no vacancy period ### Implementation Phases -#### Phase 1: Coexistence (Duration: 3-6 months) +#### Phase 1: Coexistence (Duration: TBD) 1. Remove dead code: triumvirate collective and senate pallets and related code 2. Implement the governance as a new pallet 3. Deploy new governance pallet to runtime -4. Configure initial Triumvirate members -5. Configure OTF account(s) -6. Run new governance system in parallel with existing sudo multisig -7. All governance decisions processed through new system but sudo retains override capability -8. Monitor system performance, voting patterns, and security -9. Community review and feedback period +4. Configure initial Triumvirate members and allowed proposers. +5. Run new governance system in parallel with existing sudo multisig +6. Emergency procedures documented and tested +7. Community review and feedback period #### Phase 2: Full Migration -1. Disable sudo pallet via governance vote -2. Remove dead code: triumvirate collective and senate pallets -3. New governance system becomes sole authority -4. Emergency procedures documented and tested - -**Open Questions:** -- Q50: What constitutes "emergency" and who decides to invoke emergency procedures? +1. Disable sudo pallet via governance vote (new runtime) +2. New governance system becomes sole authority \ No newline at end of file diff --git a/pallets/governance/src/lib.rs b/pallets/governance/src/lib.rs index 2766eed399..84bfa3ea10 100644 --- a/pallets/governance/src/lib.rs +++ b/pallets/governance/src/lib.rs @@ -8,7 +8,10 @@ use frame_support::{ sp_runtime::traits::Dispatchable, traits::{ Bounded, ChangeMembers, IsSubType, QueryPreimage, StorePreimage, fungible, - schedule::{DispatchTime, Priority, v3::Named as ScheduleNamed}, + schedule::{ + DispatchTime, Priority, + v3::{Named as ScheduleNamed, TaskName}, + }, }, }; use frame_system::pallet_prelude::*; @@ -165,6 +168,10 @@ pub mod pallet { #[pallet::constant] type CollectiveRotationPeriod: Get>; + /// Period of time between cleanup of proposals and scheduled proposals. + #[pallet::constant] + type CleanupPeriod: Get>; + /// Percent threshold for a proposal to be cancelled by a collective vote. #[pallet::constant] type CancellationThreshold: Get; @@ -351,17 +358,25 @@ pub mod pallet { #[pallet::hooks] impl Hooks> for Pallet { - fn on_initialize(n: BlockNumberFor) -> Weight { + fn on_initialize(now: BlockNumberFor) -> Weight { + let mut weight = Weight::zero(); + let economic_collective = EconomicCollective::::get(); let building_collective = BuildingCollective::::get(); let is_first_run = economic_collective.is_empty() || building_collective.is_empty(); - let must_rotate = n % T::CollectiveRotationPeriod::get() == Zero::zero(); + let should_rotate = now % T::CollectiveRotationPeriod::get() == Zero::zero(); + let should_cleanup = now % T::CleanupPeriod::get() == Zero::zero(); + + if is_first_run || should_rotate { + weight.saturating_accrue(Self::do_rotate_collectives()); + } - if is_first_run || must_rotate { - Self::do_rotate_collectives(); + if should_cleanup { + weight.saturating_accrue(Self::do_cleanup_proposals(now)); + weight.saturating_accrue(Self::do_cleanup_scheduled()); } - Weight::zero() + weight } } @@ -739,11 +754,7 @@ impl Pallet { ensure!(T::Preimages::have(&bounded), Error::::CallUnavailable); let now = frame_system::Pallet::::block_number(); - let name = proposal_hash - .as_ref() - .try_into() - // Unreachable because we expect the hash to be 32 bytes. - .map_err(|_| Error::::InvalidProposalHashLength)?; + let name = Self::task_name_from_hash(proposal_hash)?; T::Scheduler::schedule_named( name, DispatchTime::At(now + T::InitialSchedulingDelay::get()), @@ -776,11 +787,7 @@ impl Pallet { } fn do_fast_track(proposal_hash: T::Hash) -> DispatchResult { - let name = proposal_hash - .as_ref() - .try_into() - // Unreachable because we expect the hash to be 32 bytes. - .map_err(|_| Error::::InvalidProposalHashLength)?; + let name = Self::task_name_from_hash(proposal_hash)?; T::Scheduler::reschedule_named( name, // It will be scheduled on the next block because scheduler already ran for this block. @@ -792,10 +799,7 @@ impl Pallet { } fn do_cancel_scheduled(proposal_hash: T::Hash) -> DispatchResult { - let name = proposal_hash - .as_ref() - .try_into() - .map_err(|_| Error::::InvalidProposalHashLength)?; + let name = Self::task_name_from_hash(proposal_hash)?; T::Scheduler::cancel_named(name)?; Self::clear_scheduled_proposal(proposal_hash); Self::deposit_event(Event::::ScheduledProposalCancelled { proposal_hash }); @@ -817,11 +821,70 @@ impl Pallet { CollectiveVoting::::remove(&proposal_hash); } - fn do_rotate_collectives() { + fn do_rotate_collectives() -> Weight { + let mut weight = Weight::zero(); + let economic_collective_members = T::CollectiveMembersProvider::get_economic_collective(); let building_collective_members = T::CollectiveMembersProvider::get_building_collective(); + // TODO: handle weights + EconomicCollective::::put(economic_collective_members); BuildingCollective::::put(building_collective_members); + weight.saturating_accrue(T::DbWeight::get().writes(2)); + + weight + } + + fn do_cleanup_proposals(now: BlockNumberFor) -> Weight { + let mut weight = Weight::zero(); + + let mut proposals = Proposals::::get(); + weight.saturating_accrue(T::DbWeight::get().reads(1)); + + proposals.retain(|(_, proposal_hash)| { + let voting = Voting::::get(proposal_hash); + weight.saturating_accrue(T::DbWeight::get().reads(1)); + + match voting { + Some(voting) if voting.end > now => true, + _ => { + ProposalOf::::remove(proposal_hash); + Voting::::remove(proposal_hash); + weight.saturating_accrue(T::DbWeight::get().writes(2)); + false + } + } + }); + + Proposals::::put(proposals); + weight.saturating_accrue(T::DbWeight::get().writes(1)); + + weight + } + + fn do_cleanup_scheduled() -> Weight { + let mut weight = Weight::zero(); + + let mut scheduled = Scheduled::::get(); + weight.saturating_accrue(T::DbWeight::get().reads(1)); + + scheduled.retain( + |proposal_hash| match Self::task_name_from_hash(*proposal_hash) { + Ok(name) => { + let dispatch_time = T::Scheduler::next_dispatch_time(name); + CollectiveVoting::::remove(proposal_hash); + weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); + dispatch_time.is_ok() + } + // Unreachable because proposal hash is always 32 bytes. + Err(_) => false, + }, + ); + + Scheduled::::put(scheduled); + weight.saturating_accrue(T::DbWeight::get().writes(1)); + + weight } fn ensure_allowed_proposer(origin: OriginFor) -> Result { @@ -857,6 +920,13 @@ impl Pallet { } } + fn task_name_from_hash(proposal_hash: T::Hash) -> Result { + Ok(proposal_hash + .as_ref() + .try_into() + .map_err(|_| Error::::InvalidProposalHashLength)?) + } + fn economic_fast_track_threshold() -> u32 { T::FastTrackThreshold::get().mul_ceil(ECONOMIC_COLLECTIVE_SIZE) } diff --git a/pallets/governance/src/mock.rs b/pallets/governance/src/mock.rs index 1209897dfa..dbc869cf4a 100644 --- a/pallets/governance/src/mock.rs +++ b/pallets/governance/src/mock.rs @@ -127,6 +127,7 @@ parameter_types! { pub const MotionDuration: BlockNumberFor = 20; pub const InitialSchedulingDelay: BlockNumberFor = 20; pub const CollectiveRotationPeriod: BlockNumberFor = 100; + pub const CleanupPeriod: BlockNumberFor = 500; pub const FastTrackThreshold: Percent = Percent::from_percent(67); // ~2/3 pub const CancellationThreshold: Percent = Percent::from_percent(51); } @@ -146,6 +147,7 @@ impl pallet_governance::Config for Test { type MotionDuration = MotionDuration; type InitialSchedulingDelay = InitialSchedulingDelay; type CollectiveRotationPeriod = CollectiveRotationPeriod; + type CleanupPeriod = CleanupPeriod; type CancellationThreshold = CancellationThreshold; type FastTrackThreshold = FastTrackThreshold; } diff --git a/pallets/governance/src/tests.rs b/pallets/governance/src/tests.rs index 96788e8548..78f19a261a 100644 --- a/pallets/governance/src/tests.rs +++ b/pallets/governance/src/tests.rs @@ -1224,15 +1224,8 @@ fn collective_vote_can_be_updated() { }) ); - println!( - "{:?}", - pallet_scheduler::Agenda::::iter().collect::>() - ); - run_to_block(frame_system::Pallet::::block_number() + 100); - println!( - "{:?}", - pallet_scheduler::Agenda::::iter().collect::>() - ); + // Trigger cleanup to avoid duplicate scheduled error + run_to_block(frame_system::Pallet::::block_number() + CleanupPeriod::get()); let (proposal_hash, proposal_index) = create_scheduled_proposal!(); let building_member = U256::from(3001); @@ -1363,7 +1356,19 @@ fn collective_nay_votes_to_threshold_on_scheduled_proposal_cancels() { } #[test] -fn collective_rotation_works() { +fn cleanup_run_on_initialize() { + TestState::default().build_and_execute(|| { + let now = frame_system::Pallet::::block_number(); + run_to_block(now + CleanupPeriod::get()); + assert!(Scheduled::::get().is_empty()); + assert!(CollectiveVoting::::get(proposal_hash).is_none()); + let task_name: [u8; 32] = proposal_hash.as_ref().try_into().unwrap(); + assert!(pallet_scheduler::Lookup::::get(task_name).is_none()); + }); +} + +#[test] +fn collective_rotation_run_on_initialize() { TestState::default().build_and_execute(|| { let next_economic_collective = (1..=ECONOMIC_COLLECTIVE_SIZE) .map(|i| U256::from(4000 + i)) From d787ac3eb155953fe3ce0b57f2956eebb9714fa3 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Mon, 10 Nov 2025 12:47:43 -0300 Subject: [PATCH 17/23] make collective size 16 --- pallets/governance/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pallets/governance/src/lib.rs b/pallets/governance/src/lib.rs index 84bfa3ea10..2424cbea41 100644 --- a/pallets/governance/src/lib.rs +++ b/pallets/governance/src/lib.rs @@ -26,8 +26,8 @@ pub use pallet::*; /// WARNING: Any changes to these 3 constants require a migration to update the `BoundedVec` in storage /// for `Triumvirate`, `EconomicCollective`, or `BuildingCollective`. pub const TRIUMVIRATE_SIZE: u32 = 3; -pub const ECONOMIC_COLLECTIVE_SIZE: u32 = 10; -pub const BUILDING_COLLECTIVE_SIZE: u32 = 10; +pub const ECONOMIC_COLLECTIVE_SIZE: u32 = 16; +pub const BUILDING_COLLECTIVE_SIZE: u32 = 16; pub type CurrencyOf = ::Currency; From 802f6558b987464a63227a80ab91bd0ef07b3e9b Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Wed, 12 Nov 2025 10:00:53 -0300 Subject: [PATCH 18/23] fmt readme --- pallets/governance/README.md | 41 ++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/pallets/governance/README.md b/pallets/governance/README.md index 86bb1b08ad..692a8f90ce 100644 --- a/pallets/governance/README.md +++ b/pallets/governance/README.md @@ -31,13 +31,14 @@ The governance system consists of three main actors working together: - **Purpose**: Authorized to submit proposals (calls executed with root privilege) - **Assignment**: Allowed proposer account keys are configured in the runtime via governance -- **Permissions**: +- **Permissions**: - Can submit proposals to the main governance track (i.e., runtime upgrade proposals or any root extrinsic) - Can cancel or withdraw their own proposals anytime before execution (i.e., if they find a bug in the proposal code) - Can eject its own key from the allowed proposers list (i.e., if it is lost or compromised) - Can propose an update to the allowed proposers list via proposal flow **Open Questions:** + - Q1: Who can add/remove proposer accounts? Only governance or should Triumvirate have emergency powers? - Q2: Who validates that proposal code matches stated intent before Triumvirate votes? Share runtime WASM hash like Polkadot fellowship does? @@ -52,7 +53,8 @@ The governance system consists of three main actors working together: - Can vote on proposals submitted by allowed proposers **Open Questions:** - - Q3: How to allow a triumvirate member to resign? + +- Q3: How to allow a triumvirate member to resign? #### Economic and Building Collectives @@ -67,8 +69,9 @@ The governance system consists of three main actors working together: - Can replace a Triumvirate member every 6 months via single atomic vote (remove current holder + install replacement candidate, with rotating seat selection) - Can mark himself as eligible for nomination to the Triumvirate - Can accept a nomination to the Triumvirate - + **Open Questions:** + - Q4: How to handle the nomination process? - Q5: How to incentivize the collective members to vote? @@ -87,8 +90,9 @@ The governance system consists of three main actors working together: #### Triumvirate Approval 1. Triumvirate members cast votes (aye/nay) on the proposal - - 2/3 vote aye, proposal is approved: Proposal is scheduled for execution in 7 days (configurable) and enters "Delay Period" - - 2/3 vote nay, proposal is rejected: Proposal is cleaned up from storage (it was never scheduled for execution). + +- 2/3 vote aye, proposal is approved: Proposal is scheduled for execution in 7 days (configurable) and enters "Delay Period" +- 2/3 vote nay, proposal is rejected: Proposal is cleaned up from storage (it was never scheduled for execution). - Triumvirate members can change their vote during the voting period (before the proposal is scheduled or cancelled). - There is a queue limit in the number of scheduled proposals and in the delay period (configurable). @@ -100,16 +104,18 @@ When a proposal has been approved by the Triumvirate, it is scheduled in 7 days 1. Both collectives can vote aye/nay on the proposal 2. Delay is an exponential function of the number of nays votes, set to 1.5^n (configurable). - - Initial delay is 7 days (configurable). - - After 1 nays vote, the delay is 1.5^1 * 7 days = 10.5 days. - - After 2 nays votes, the delay is 1.5^2 * 7 days = ~16 days. - - After 3 nays votes, the delay is 1.5^3 * 7 days = ~23 days. - - After 4 nays votes, the delay is 1.5^4 * 7 days = ~35 days. - - After 5 nays votes, the delay is 1.5^5 * 7 days = ~53 days. - - After 6 nays votes, the delay is 1.5^6 * 7 days = ~80 days. - - After 7 nays votes, the delay is 1.5^7 * 7 days = ~120 days. - - After 8 nays votes, the delay is 1.5^8 * 7 days = ~180 days. - - After 9 nays votes, proposal is cancelled (given we have a collective size of 16, hence more than 1/2 of the collective votes nay). + +- Initial delay is 7 days (configurable). +- After 1 nays vote, the delay is 1.5^1 \* 7 days = 10.5 days. +- After 2 nays votes, the delay is 1.5^2 \* 7 days = ~16 days. +- After 3 nays votes, the delay is 1.5^3 \* 7 days = ~23 days. +- After 4 nays votes, the delay is 1.5^4 \* 7 days = ~35 days. +- After 5 nays votes, the delay is 1.5^5 \* 7 days = ~53 days. +- After 6 nays votes, the delay is 1.5^6 \* 7 days = ~80 days. +- After 7 nays votes, the delay is 1.5^7 \* 7 days = ~120 days. +- After 8 nays votes, the delay is 1.5^8 \* 7 days = ~180 days. +- After 9 nays votes, proposal is cancelled (given we have a collective size of 16, hence more than 1/2 of the collective votes nay). + 3. If the delay period expires without cancellation: Proposal executes automatically - The delay is calculated based on the collective with the most nays votes (i.e., if Economic has 3 nays and Building has 1 nay, the delay is based on 3 nays = ~23 days). @@ -119,6 +125,7 @@ When a proposal has been approved by the Triumvirate, it is scheduled in 7 days - **Example**: A proposal has 3 nays votes, creating a 23-day delay. After 17 days have elapsed, a collective member changes their nay vote to aye, reducing the delay to 16 days. Since 17 days have already passed (more than the new 16-day delay), the proposal executes immediately. **Open Questions:** + - Q6: Should the voting be across both collectives or each collective votes independently? What if a collective decide to go rogue and fast track proposals that the other collective is against or vice versa? #### Execution @@ -136,6 +143,7 @@ Each collective can replace one Triumvirate member every 6 months through a **si - Economic and Building collectives have independent cycles (seat are rotated independently) **Open Questions:** + - Q7: How to have an emergency replacement vote? - Q8: Can a replaced member be voted back in immediately, or should there be a cooldown period? @@ -156,6 +164,7 @@ Each collective can replace one Triumvirate member every 6 months through a **si The replacement happens in a single vote where the collective votes **both** to remove the current seat holder **and** to install a specific replacement candidate. This is an atomic operation: either both happen or neither happens. **Process:** + 1. **Eligibility Phase**: Collective members can mark themselves as eligible for nomination to the Triumvirate. 2. **Voting Phase**: Collective members can vote aye/nay during the voting period to replace the current seat holder. - Threshold of more than 1/2 of the collective size (configurable) @@ -180,4 +189,4 @@ The replacement happens in a single vote where the collective votes **both** to #### Phase 2: Full Migration 1. Disable sudo pallet via governance vote (new runtime) -2. New governance system becomes sole authority \ No newline at end of file +2. New governance system becomes sole authority From 1e7acf1813df37d5ca20e885129413ce5628766e Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Wed, 12 Nov 2025 11:33:25 -0300 Subject: [PATCH 19/23] update doc initial delay to 1h --- pallets/governance/README.md | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/pallets/governance/README.md b/pallets/governance/README.md index 692a8f90ce..855fc95f75 100644 --- a/pallets/governance/README.md +++ b/pallets/governance/README.md @@ -91,7 +91,7 @@ The governance system consists of three main actors working together: 1. Triumvirate members cast votes (aye/nay) on the proposal -- 2/3 vote aye, proposal is approved: Proposal is scheduled for execution in 7 days (configurable) and enters "Delay Period" +- 2/3 vote aye, proposal is approved: Proposal is scheduled for execution in 1 hour (configurable) and enters "Delay Period" - 2/3 vote nay, proposal is rejected: Proposal is cleaned up from storage (it was never scheduled for execution). - Triumvirate members can change their vote during the voting period (before the proposal is scheduled or cancelled). @@ -100,29 +100,29 @@ The governance system consists of three main actors working together: #### Delay Period (Collective Oversight) -When a proposal has been approved by the Triumvirate, it is scheduled in 7 days (configurable) and enters the "Delay Period" where the Economic and Building Collectives can vote to delay, cancel or fast-track the proposal. +When a proposal has been approved by the Triumvirate, it is scheduled in 1 hour (configurable) and enters the "Delay Period" where the Economic and Building Collectives can vote to delay, cancel or fast-track the proposal. 1. Both collectives can vote aye/nay on the proposal -2. Delay is an exponential function of the number of nays votes, set to 1.5^n (configurable). - -- Initial delay is 7 days (configurable). -- After 1 nays vote, the delay is 1.5^1 \* 7 days = 10.5 days. -- After 2 nays votes, the delay is 1.5^2 \* 7 days = ~16 days. -- After 3 nays votes, the delay is 1.5^3 \* 7 days = ~23 days. -- After 4 nays votes, the delay is 1.5^4 \* 7 days = ~35 days. -- After 5 nays votes, the delay is 1.5^5 \* 7 days = ~53 days. -- After 6 nays votes, the delay is 1.5^6 \* 7 days = ~80 days. -- After 7 nays votes, the delay is 1.5^7 \* 7 days = ~120 days. -- After 8 nays votes, the delay is 1.5^8 \* 7 days = ~180 days. +2. Delay is an exponential function of the number of nays votes, set to 2^n (configurable). + +- Initial delay is 1 hour (configurable). +- After 1 nays vote, the delay is 2^1 \* 1 hour = 2 hours. +- After 2 nays votes, the delay is 2^2 \* 1 hour = 4 hours. +- After 3 nays votes, the delay is 2^3 \* 1 hour = 8 hours. +- After 4 nays votes, the delay is 2^4 \* 1 hour = 16 hours. +- After 5 nays votes, the delay is 2^5 \* 1 hour = 32 hours. +- After 6 nays votes, the delay is 2^6 \* 1 hour = 64 hours. +- After 7 nays votes, the delay is 2^7 \* 1 hour = 128 hours. +- After 8 nays votes, the delay is 2^8 \* 1 hour = 256 hours. - After 9 nays votes, proposal is cancelled (given we have a collective size of 16, hence more than 1/2 of the collective votes nay). 3. If the delay period expires without cancellation: Proposal executes automatically -- The delay is calculated based on the collective with the most nays votes (i.e., if Economic has 3 nays and Building has 1 nay, the delay is based on 3 nays = ~23 days). +- The delay is calculated based on the collective with the most nays votes (i.e., if Economic has 3 nays and Building has 1 nay, the delay is based on 3 nays = 8 hours). - More than 2/3 of aye vote for any collective fast tracks the proposal (next block execution) (threshold configurable) - More than 1/2 of nay vote for any collective cancels the proposal (threshold configurable) - Collective members can change their vote during the delay period. If changing a nay vote to aye reduces the delay below the time already elapsed, the proposal executes immediately. - - **Example**: A proposal has 3 nays votes, creating a 23-day delay. After 17 days have elapsed, a collective member changes their nay vote to aye, reducing the delay to 16 days. Since 17 days have already passed (more than the new 16-day delay), the proposal executes immediately. + - **Example**: A proposal has 3 nays votes, creating a 8 hours delay. After 5 hours have elapsed, a collective member changes their nay vote to aye, reducing the delay to 4 hours. Since 5 hours have already passed (more than the new 4 hours delay), the proposal executes immediately. **Open Questions:** From 646c6fde1fa0a370705f76a212ec578342f86a17 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Wed, 12 Nov 2025 17:35:24 -0300 Subject: [PATCH 20/23] combine both collectives --- pallets/governance/src/lib.rs | 226 +++++++------- pallets/governance/src/tests.rs | 516 +++++++++++++------------------- 2 files changed, 329 insertions(+), 413 deletions(-) diff --git a/pallets/governance/src/lib.rs b/pallets/governance/src/lib.rs index 2424cbea41..fa48f8f711 100644 --- a/pallets/governance/src/lib.rs +++ b/pallets/governance/src/lib.rs @@ -29,6 +29,8 @@ pub const TRIUMVIRATE_SIZE: u32 = 3; pub const ECONOMIC_COLLECTIVE_SIZE: u32 = 16; pub const BUILDING_COLLECTIVE_SIZE: u32 = 16; +pub const TOTAL_COLLECTIVES_SIZE: u32 = ECONOMIC_COLLECTIVE_SIZE + BUILDING_COLLECTIVE_SIZE; + pub type CurrencyOf = ::Currency; pub type BalanceOf = @@ -48,8 +50,8 @@ pub type ScheduleAddressOf = pub type ProposalIndex = u32; #[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)] -#[freeze_struct("4151e52425e670aa")] -pub struct Votes { +#[freeze_struct("7b322ade3ccaaba")] +pub struct TriumvirateVotes { /// The proposal's unique index. index: ProposalIndex, /// The set of triumvirate members that approved it. @@ -61,18 +63,18 @@ pub struct Votes { } #[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)] -// #[freeze_struct("58071fdbad8767b6")] -pub struct CollectiveVotes { +#[freeze_struct("68b000ed325d45c4")] +pub struct CollectiveVotes { /// The proposal's unique index. index: ProposalIndex, - /// The set of economic collective members that approved it. - economic_ayes: BoundedVec>, - /// The set of economic collective members that rejected it. - economic_nays: BoundedVec>, - /// The set of building collective members that approved it. - building_ayes: BoundedVec>, - /// The set of building collective members that rejected it. - building_nays: BoundedVec>, + /// The set of collective members that approved it. + ayes: BoundedVec>, + /// The set of collective members that rejected it. + nays: BoundedVec>, + /// The initial dispatch time of the proposal. + initial_dispatch_time: BlockNumber, + /// The additional delay applied to the proposal on top of the initial delay. + delay: BlockNumber, } #[derive( @@ -204,10 +206,15 @@ pub mod pallet { pub type ProposalOf = StorageMap<_, Identity, T::Hash, BoundedCallOf, OptionQuery>; - /// Votes for a given proposal, if it is ongoing. + /// Triumvirate votes for a given proposal, if it is ongoing. #[pallet::storage] - pub type Voting = - StorageMap<_, Identity, T::Hash, Votes>, OptionQuery>; + pub type TriumvirateVoting = StorageMap< + _, + Identity, + T::Hash, + TriumvirateVotes>, + OptionQuery, + >; /// The hashes of the proposals that have been scheduled for execution. #[pallet::storage] @@ -224,10 +231,15 @@ pub mod pallet { pub type BuildingCollective = StorageValue<_, BoundedVec>, ValueQuery>; - /// Collective votes for a given proposal, if it is scheduled. + /// Collectives votes for a given proposal, if it is scheduled. #[pallet::storage] - pub type CollectiveVoting = - StorageMap<_, Identity, T::Hash, CollectiveVotes, OptionQuery>; + pub type CollectiveVoting = StorageMap< + _, + Identity, + T::Hash, + CollectiveVotes>, + OptionQuery, + >; #[pallet::genesis_config] #[derive(frame_support::DefaultNoBound)] @@ -294,21 +306,19 @@ pub mod pallet { }, /// A collective member has voted on a proposal. CollectiveMemberVoted { - account: CollectiveMember, + account: T::AccountId, proposal_hash: T::Hash, voted: bool, - economic_yes: u32, - economic_no: u32, - building_yes: u32, - building_no: u32, + yes: u32, + no: u32, }, - /// A proposal has been scheduled for execution. + /// A proposal has been scheduled for execution by triumvirate. ProposalScheduled { proposal_hash: T::Hash }, - /// A proposal has been cancelled. + /// A proposal has been cancelled by triumvirate. ProposalCancelled { proposal_hash: T::Hash }, - /// A scheduled proposal has been fast-tracked. + /// A scheduled proposal has been fast-tracked by collectives. ScheduledProposalFastTracked { proposal_hash: T::Hash }, - /// A scheduled proposal has been cancelled. + /// A scheduled proposal has been cancelled by collectives. ScheduledProposalCancelled { proposal_hash: T::Hash }, } @@ -464,7 +474,7 @@ pub mod pallet { // Remove votes from the outgoing triumvirate members. for (_proposer, proposal_hash) in Proposals::::get() { - Voting::::mutate(proposal_hash, |voting| { + TriumvirateVoting::::mutate(proposal_hash, |voting| { if let Some(voting) = voting.as_mut() { voting.ayes.retain(|a| !outgoing.contains(a)); voting.nays.retain(|a| !outgoing.contains(a)); @@ -521,9 +531,9 @@ pub mod pallet { let now = frame_system::Pallet::::block_number(); let end = now + T::MotionDuration::get(); - Voting::::insert( + TriumvirateVoting::::insert( proposal_hash, - Votes { + TriumvirateVotes { index: proposal_index, ayes: BoundedVec::new(), nays: BoundedVec::new(), @@ -543,7 +553,7 @@ pub mod pallet { /// Vote on a proposal as a triumvirate member. #[pallet::call_index(3)] #[pallet::weight(Weight::zero())] - pub fn vote( + pub fn vote_on_proposed( origin: OriginFor, proposal_hash: T::Hash, #[pallet::compact] proposal_index: ProposalIndex, @@ -557,7 +567,7 @@ pub mod pallet { Error::::ProposalMissing ); - let voting = Self::do_vote(&who, proposal_hash, proposal_index, approve)?; + let voting = Self::do_vote_on_proposed(&who, proposal_hash, proposal_index, approve)?; let yes_votes = voting.ayes.len() as u32; let no_votes = voting.nays.len() as u32; @@ -582,7 +592,7 @@ pub mod pallet { /// Vote on a proposal as a collective member. #[pallet::call_index(4)] #[pallet::weight(Weight::zero())] - pub fn collective_vote( + pub fn vote_on_scheduled( origin: OriginFor, proposal_hash: T::Hash, #[pallet::compact] proposal_index: ProposalIndex, @@ -596,39 +606,30 @@ pub mod pallet { Error::::ProposalNotScheduled ); - let voting = Self::do_collective_vote(&who, proposal_hash, proposal_index, approve)?; + let voting = Self::do_vote_on_scheduled(&who, proposal_hash, proposal_index, approve)?; - let economic_yes_votes = voting.economic_ayes.len() as u32; - let economic_no_votes = voting.economic_nays.len() as u32; - let building_yes_votes = voting.building_ayes.len() as u32; - let building_no_votes = voting.building_nays.len() as u32; + let yes_votes = voting.ayes.len() as u32; + let no_votes = voting.nays.len() as u32; Self::deposit_event(Event::::CollectiveMemberVoted { account: who, proposal_hash, voted: approve, - economic_yes: economic_yes_votes, - economic_no: economic_no_votes, - building_yes: building_yes_votes, - building_no: building_no_votes, + yes: yes_votes, + no: no_votes, }); - let should_fast_track = economic_yes_votes >= Self::economic_fast_track_threshold() - || building_yes_votes >= Self::building_fast_track_threshold(); - - let should_cancel = economic_no_votes >= Self::economic_cancellation_threshold() - || building_no_votes >= Self::building_cancellation_threshold(); - - let should_adjust_delay = !should_fast_track - && !should_cancel - && (economic_no_votes > 0 || building_no_votes > 0); + let should_fast_track = + yes_votes >= T::FastTrackThreshold::get().mul_ceil(TOTAL_COLLECTIVES_SIZE) as u32; + let should_cancel = + no_votes >= T::CancellationThreshold::get().mul_ceil(TOTAL_COLLECTIVES_SIZE) as u32; if should_fast_track { Self::do_fast_track(proposal_hash)?; } else if should_cancel { Self::do_cancel_scheduled(proposal_hash)?; - } else if should_adjust_delay { - // handle delay adjustment + } else { + Self::do_adjust_delay(proposal_hash, voting)?; } Ok(()) @@ -668,13 +669,13 @@ impl Pallet { } } - fn do_vote( + fn do_vote_on_proposed( who: &T::AccountId, proposal_hash: T::Hash, index: ProposalIndex, approve: bool, - ) -> Result>, DispatchError> { - Voting::::try_mutate(proposal_hash, |voting| { + ) -> Result>, DispatchError> { + TriumvirateVoting::::try_mutate(proposal_hash, |voting| { let voting = voting.as_mut().ok_or(Error::::ProposalMissing)?; ensure!(voting.index == index, Error::::WrongProposalIndex); Self::do_vote_inner(&who, approve, &mut voting.ayes, &mut voting.nays)?; @@ -682,8 +683,8 @@ impl Pallet { }) } - fn do_collective_vote( - who: &CollectiveMember, + fn do_vote_on_scheduled( + who: &T::AccountId, proposal_hash: T::Hash, index: ProposalIndex, approve: bool, @@ -691,22 +692,7 @@ impl Pallet { CollectiveVoting::::try_mutate(proposal_hash, |voting| { let voting = voting.as_mut().ok_or(Error::::ProposalNotScheduled)?; ensure!(voting.index == index, Error::::WrongProposalIndex); - - match who { - CollectiveMember::Economic(who) => Self::do_vote_inner( - who, - approve, - &mut voting.economic_ayes, - &mut voting.economic_nays, - )?, - CollectiveMember::Building(who) => Self::do_vote_inner( - who, - approve, - &mut voting.building_ayes, - &mut voting.building_nays, - )?, - } - + Self::do_vote_inner(&who, approve, &mut voting.ayes, &mut voting.nays)?; Ok(voting.clone()) }) } @@ -755,9 +741,10 @@ impl Pallet { let now = frame_system::Pallet::::block_number(); let name = Self::task_name_from_hash(proposal_hash)?; + let dispatch_time = now + T::InitialSchedulingDelay::get(); T::Scheduler::schedule_named( name, - DispatchTime::At(now + T::InitialSchedulingDelay::get()), + DispatchTime::At(dispatch_time), None, Priority::default(), RawOrigin::Root.into(), @@ -769,10 +756,10 @@ impl Pallet { proposal_hash, CollectiveVotes { index: proposal_index, - economic_ayes: BoundedVec::new(), - economic_nays: BoundedVec::new(), - building_ayes: BoundedVec::new(), - building_nays: BoundedVec::new(), + ayes: BoundedVec::new(), + nays: BoundedVec::new(), + initial_dispatch_time: dispatch_time, + delay: Zero::zero(), }, ); @@ -806,12 +793,61 @@ impl Pallet { Ok(()) } + fn do_adjust_delay( + proposal_hash: T::Hash, + mut voting: CollectiveVotes, + ) -> DispatchResult { + let net_score = voting.nays.len() as i32 - voting.ayes.len() as i32; + let now = frame_system::Pallet::::block_number(); + let name = Self::task_name_from_hash(proposal_hash)?; + + // Delay based on net opposition + let additional_delay = if new_score > 0 { + T::InitialSchedulingDelay::get() + .saturating_mul(1.5_f64.powi(net_score as u32)) + .ceil() as BlockNumberFor + } else { + Zero::zero(); + }; + + let + + if net_score > 0 { + let new_delay = 2_u64.pow(net_score as u32) * T::InitialSchedulingDelay::get(); + let is_past_new_delay = now >= voting.initial_dispatch_time + new_delay; + + // New delay is lower and we are past it, we should fast track + if new_delay < voting.delay && is_past_new_delay { + Self::do_fast_track(proposal_hash)?; + return; + } + + // New delay is higher, adjust delay + voting.delay = new_delay; + let new_dispatch_time = DispatchTime::At(voting.initial_dispatch_time + new_delay); + T::Scheduler::reschedule_named(name, new_dispatch_time)?; + } else { + // New delay is reset to 0 and we are past initial dispatch time, fast track + if now >= voting.initial_dispatch_time { + Self::do_fast_track(proposal_hash)?; + return; + } + + // New delay is reset to 0 and we are not past initial dispatch time, adjust delay + voting.delay = 0; + let new_dispatch_time = DispatchTime::At(voting.initial_dispatch_time); + T::Scheduler::reschedule_named(name, new_dispatch_time)?; + } + + Ok(()) + } + fn clear_proposal(proposal_hash: T::Hash) { Proposals::::mutate(|proposals| { proposals.retain(|(_, h)| h != &proposal_hash); }); ProposalOf::::remove(&proposal_hash); - Voting::::remove(&proposal_hash); + TriumvirateVoting::::remove(&proposal_hash); } fn clear_scheduled_proposal(proposal_hash: T::Hash) { @@ -842,14 +878,14 @@ impl Pallet { weight.saturating_accrue(T::DbWeight::get().reads(1)); proposals.retain(|(_, proposal_hash)| { - let voting = Voting::::get(proposal_hash); + let voting = TriumvirateVoting::::get(proposal_hash); weight.saturating_accrue(T::DbWeight::get().reads(1)); match voting { Some(voting) if voting.end > now => true, _ => { ProposalOf::::remove(proposal_hash); - Voting::::remove(proposal_hash); + TriumvirateVoting::::remove(proposal_hash); weight.saturating_accrue(T::DbWeight::get().writes(2)); false } @@ -904,17 +940,13 @@ impl Pallet { Ok(who) } - fn ensure_collective_member( - origin: OriginFor, - ) -> Result, DispatchError> { + fn ensure_collective_member(origin: OriginFor) -> Result { let who = ensure_signed(origin)?; let economic_collective = EconomicCollective::::get(); let building_collective = BuildingCollective::::get(); - if economic_collective.contains(&who) { - Ok(CollectiveMember::Economic(who)) - } else if building_collective.contains(&who) { - Ok(CollectiveMember::Building(who)) + if economic_collective.contains(&who) || building_collective.contains(&who) { + Ok(who) } else { Err(Error::::NotCollectiveMember.into()) } @@ -926,20 +958,4 @@ impl Pallet { .try_into() .map_err(|_| Error::::InvalidProposalHashLength)?) } - - fn economic_fast_track_threshold() -> u32 { - T::FastTrackThreshold::get().mul_ceil(ECONOMIC_COLLECTIVE_SIZE) - } - - fn building_fast_track_threshold() -> u32 { - T::FastTrackThreshold::get().mul_ceil(BUILDING_COLLECTIVE_SIZE) as u32 - } - - fn economic_cancellation_threshold() -> u32 { - T::CancellationThreshold::get().mul_ceil(ECONOMIC_COLLECTIVE_SIZE) as u32 - } - - fn building_cancellation_threshold() -> u32 { - T::CancellationThreshold::get().mul_ceil(BUILDING_COLLECTIVE_SIZE) as u32 - } } diff --git a/pallets/governance/src/tests.rs b/pallets/governance/src/tests.rs index 78f19a261a..5edc9f6bcc 100644 --- a/pallets/governance/src/tests.rs +++ b/pallets/governance/src/tests.rs @@ -293,13 +293,13 @@ fn set_triumvirate_removes_votes_of_outgoing_triumvirate_members() { vec![U256::from(1001), U256::from(1002), U256::from(1003)] ); - vote_aye!(U256::from(1001), proposal_hash1, proposal_index1); + vote_aye_on_proposed!(U256::from(1001), proposal_hash1, proposal_index1); - vote_nay!(U256::from(1002), proposal_hash2, proposal_index2); - vote_aye!(U256::from(1003), proposal_hash2, proposal_index2); + vote_nay_on_proposed!(U256::from(1002), proposal_hash2, proposal_index2); + vote_aye_on_proposed!(U256::from(1003), proposal_hash2, proposal_index2); - vote_nay!(U256::from(1001), proposal_hash3, proposal_index3); - vote_aye!(U256::from(1002), proposal_hash3, proposal_index3); + vote_nay_on_proposed!(U256::from(1001), proposal_hash3, proposal_index3); + vote_aye_on_proposed!(U256::from(1002), proposal_hash3, proposal_index3); let triumvirate = BoundedVec::truncate_from(vec![U256::from(1001), U256::from(1003), U256::from(1004)]); @@ -308,13 +308,13 @@ fn set_triumvirate_removes_votes_of_outgoing_triumvirate_members() { triumvirate.clone() )); assert_eq!(Triumvirate::::get(), triumvirate); - let voting1 = Voting::::get(proposal_hash1).unwrap(); + let voting1 = TriumvirateVoting::::get(proposal_hash1).unwrap(); assert_eq!(voting1.ayes.to_vec(), vec![U256::from(1001)]); assert!(voting1.nays.to_vec().is_empty()); - let voting2 = Voting::::get(proposal_hash2).unwrap(); + let voting2 = TriumvirateVoting::::get(proposal_hash2).unwrap(); assert_eq!(voting2.ayes.to_vec(), vec![U256::from(1003)]); assert!(voting2.nays.to_vec().is_empty()); - let voting3 = Voting::::get(proposal_hash3).unwrap(); + let voting3 = TriumvirateVoting::::get(proposal_hash3).unwrap(); assert!(voting3.ayes.to_vec().is_empty()); assert_eq!(voting3.nays.to_vec(), vec![U256::from(1001)]); assert_eq!( @@ -413,8 +413,8 @@ fn propose_works_with_inline_preimage() { ); let now = frame_system::Pallet::::block_number(); assert_eq!( - Voting::::get(proposal_hash), - Some(Votes { + TriumvirateVoting::::get(proposal_hash), + Some(TriumvirateVotes { index: proposal_index, ayes: BoundedVec::new(), nays: BoundedVec::new(), @@ -466,8 +466,8 @@ fn propose_works_with_lookup_preimage() { assert!(::Preimages::have(&bounded_proposal)); let now = frame_system::Pallet::::block_number(); assert_eq!( - Voting::::get(proposal_hash), - Some(Votes { + TriumvirateVoting::::get(proposal_hash), + Some(TriumvirateVotes { index: proposal_index, ayes: BoundedVec::new(), nays: BoundedVec::new(), @@ -602,8 +602,8 @@ fn propose_with_already_scheduled_proposal_fails() { TestState::default().build_and_execute(|| { let (proposal_hash, proposal_index) = create_proposal!(); - vote_aye!(U256::from(1001), proposal_hash, proposal_index); - vote_aye!(U256::from(1002), proposal_hash, proposal_index); + vote_aye_on_proposed!(U256::from(1001), proposal_hash, proposal_index); + vote_aye_on_proposed!(U256::from(1002), proposal_hash, proposal_index); let proposal = Box::new(RuntimeCall::System( frame_system::Call::::set_storage { @@ -663,19 +663,19 @@ fn propose_with_too_many_proposals_fails() { } #[test] -fn vote_aye_as_first_voter_works() { +fn triumirate_vote_aye_as_first_voter_works() { TestState::default().build_and_execute(|| { let (proposal_hash, proposal_index) = create_proposal!(); let approve = true; - assert_ok!(Pallet::::vote( + assert_ok!(Pallet::::vote_on_proposed( RuntimeOrigin::signed(U256::from(1001)), proposal_hash, proposal_index, approve )); - let votes = Voting::::get(proposal_hash).unwrap(); + let votes = TriumvirateVoting::::get(proposal_hash).unwrap(); assert_eq!(votes.ayes.to_vec(), vec![U256::from(1001)]); assert!(votes.nays.to_vec().is_empty()); assert_eq!( @@ -692,19 +692,19 @@ fn vote_aye_as_first_voter_works() { } #[test] -fn vote_nay_as_first_voter_works() { +fn triumvirate_vote_nay_as_first_voter_works() { TestState::default().build_and_execute(|| { let (proposal_hash, proposal_index) = create_proposal!(); let approve = false; - assert_ok!(Pallet::::vote( + assert_ok!(Pallet::::vote_on_proposed( RuntimeOrigin::signed(U256::from(1001)), proposal_hash, proposal_index, approve )); - let votes = Voting::::get(proposal_hash).unwrap(); + let votes = TriumvirateVoting::::get(proposal_hash).unwrap(); assert_eq!(votes.nays.to_vec(), vec![U256::from(1001)]); assert!(votes.ayes.to_vec().is_empty()); assert_eq!( @@ -721,13 +721,13 @@ fn vote_nay_as_first_voter_works() { } #[test] -fn vote_can_be_updated() { +fn triumvirate_vote_can_be_updated() { TestState::default().build_and_execute(|| { let (proposal_hash, proposal_index) = create_proposal!(); // Vote aye initially - vote_aye!(U256::from(1001), proposal_hash, proposal_index); - let votes = Voting::::get(proposal_hash).unwrap(); + vote_aye_on_proposed!(U256::from(1001), proposal_hash, proposal_index); + let votes = TriumvirateVoting::::get(proposal_hash).unwrap(); assert_eq!(votes.ayes.to_vec(), vec![U256::from(1001)]); assert!(votes.nays.to_vec().is_empty()); assert_eq!( @@ -742,8 +742,8 @@ fn vote_can_be_updated() { ); // Then vote nay, replacing the aye vote - vote_nay!(U256::from(1001), proposal_hash, proposal_index); - let votes = Voting::::get(proposal_hash).unwrap(); + vote_nay_on_proposed!(U256::from(1001), proposal_hash, proposal_index); + let votes = TriumvirateVoting::::get(proposal_hash).unwrap(); assert_eq!(votes.nays.to_vec(), vec![U256::from(1001)]); assert!(votes.ayes.to_vec().is_empty()); assert_eq!( @@ -758,8 +758,8 @@ fn vote_can_be_updated() { ); // Then vote aye again, replacing the nay vote - vote_aye!(U256::from(1001), proposal_hash, proposal_index); - let votes = Voting::::get(proposal_hash).unwrap(); + vote_aye_on_proposed!(U256::from(1001), proposal_hash, proposal_index); + let votes = TriumvirateVoting::::get(proposal_hash).unwrap(); assert_eq!(votes.ayes.to_vec(), vec![U256::from(1001)]); assert!(votes.nays.to_vec().is_empty()); assert_eq!( @@ -776,25 +776,23 @@ fn vote_can_be_updated() { } #[test] -fn two_aye_votes_schedule_proposal() { +fn two_triumvirate_aye_votes_schedule_proposal() { TestState::default().build_and_execute(|| { let (proposal_hash, proposal_index) = create_proposal!(); - vote_aye!(U256::from(1001), proposal_hash, proposal_index); - vote_nay!(U256::from(1002), proposal_hash, proposal_index); - vote_aye!(U256::from(1003), proposal_hash, proposal_index); + vote_aye_on_proposed!(U256::from(1001), proposal_hash, proposal_index); + vote_nay_on_proposed!(U256::from(1002), proposal_hash, proposal_index); + vote_aye_on_proposed!(U256::from(1003), proposal_hash, proposal_index); assert!(Proposals::::get().is_empty()); - assert!(!Voting::::contains_key(proposal_hash)); + assert!(!TriumvirateVoting::::contains_key(proposal_hash)); assert_eq!(Scheduled::::get(), vec![proposal_hash]); assert_eq!( CollectiveVoting::::get(proposal_hash), Some(CollectiveVotes { index: proposal_index, - economic_ayes: BoundedVec::new(), - economic_nays: BoundedVec::new(), - building_ayes: BoundedVec::new(), - building_nays: BoundedVec::new(), + ayes: BoundedVec::new(), + nays: BoundedVec::new(), }) ); let task_name: [u8; 32] = proposal_hash.as_ref().try_into().unwrap(); @@ -822,16 +820,16 @@ fn two_aye_votes_schedule_proposal() { } #[test] -fn two_nay_votes_cancel_proposal() { +fn two_triumvirate_nay_votes_cancel_proposal() { TestState::default().build_and_execute(|| { let (proposal_hash, proposal_index) = create_proposal!(); - vote_nay!(U256::from(1001), proposal_hash, proposal_index); - vote_aye!(U256::from(1002), proposal_hash, proposal_index); - vote_nay!(U256::from(1003), proposal_hash, proposal_index); + vote_nay_on_proposed!(U256::from(1001), proposal_hash, proposal_index); + vote_aye_on_proposed!(U256::from(1002), proposal_hash, proposal_index); + vote_nay_on_proposed!(U256::from(1003), proposal_hash, proposal_index); assert!(Proposals::::get().is_empty()); - assert!(!Voting::::contains_key(proposal_hash)); + assert!(!TriumvirateVoting::::contains_key(proposal_hash)); assert!(Scheduled::::get().is_empty()); assert_eq!(ProposalOf::::get(proposal_hash), None); let events = last_n_events(2); @@ -853,28 +851,38 @@ fn two_nay_votes_cancel_proposal() { } #[test] -fn vote_as_bad_origin_fails() { +fn triumvirate_vote_as_bad_origin_fails() { TestState::default().build_and_execute(|| { let (proposal_hash, proposal_index) = create_proposal!(); assert_noop!( - Pallet::::vote(RuntimeOrigin::root(), proposal_hash, proposal_index, true), + Pallet::::vote_on_proposed( + RuntimeOrigin::root(), + proposal_hash, + proposal_index, + true + ), DispatchError::BadOrigin ); assert_noop!( - Pallet::::vote(RuntimeOrigin::none(), proposal_hash, proposal_index, true), + Pallet::::vote_on_proposed( + RuntimeOrigin::none(), + proposal_hash, + proposal_index, + true + ), DispatchError::BadOrigin ); }); } #[test] -fn vote_as_non_triumvirate_member_fails() { +fn triumvirate_vote_as_non_triumvirate_member_fails() { TestState::default().build_and_execute(|| { let (proposal_hash, proposal_index) = create_proposal!(); assert_noop!( - Pallet::::vote( + Pallet::::vote_on_proposed( RuntimeOrigin::signed(U256::from(42)), proposal_hash, proposal_index, @@ -886,12 +894,12 @@ fn vote_as_non_triumvirate_member_fails() { } #[test] -fn vote_on_missing_proposal_fails() { +fn triumvirate_vote_on_missing_proposal_fails() { TestState::default().build_and_execute(|| { let invalid_proposal_hash = ::Hashing::hash(b"Invalid proposal"); assert_noop!( - Pallet::::vote( + Pallet::::vote_on_proposed( RuntimeOrigin::signed(U256::from(1001)), invalid_proposal_hash, 0, @@ -903,18 +911,18 @@ fn vote_on_missing_proposal_fails() { } #[test] -fn vote_on_scheduled_proposal_fails() { +fn triumvirate_vote_on_scheduled_proposal_fails() { TestState::default().build_and_execute(|| { let (proposal_hash, proposal_index) = create_proposal!(); - vote_aye!(U256::from(1001), proposal_hash, proposal_index); - vote_aye!(U256::from(1002), proposal_hash, proposal_index); + vote_aye_on_proposed!(U256::from(1001), proposal_hash, proposal_index); + vote_aye_on_proposed!(U256::from(1002), proposal_hash, proposal_index); assert!(Proposals::::get().is_empty()); assert_eq!(Scheduled::::get(), vec![proposal_hash]); assert_noop!( - Pallet::::vote( + Pallet::::vote_on_proposed( RuntimeOrigin::signed(U256::from(1003)), proposal_hash, proposal_index, @@ -926,12 +934,12 @@ fn vote_on_scheduled_proposal_fails() { } #[test] -fn vote_on_proposal_with_wrong_index_fails() { +fn triumvirate_vote_on_proposal_with_wrong_index_fails() { TestState::default().build_and_execute(|| { let (proposal_hash, proposal_index) = create_proposal!(); assert_noop!( - Pallet::::vote( + Pallet::::vote_on_proposed( RuntimeOrigin::signed(U256::from(1001)), proposal_hash, proposal_index + 1, @@ -943,40 +951,40 @@ fn vote_on_proposal_with_wrong_index_fails() { } #[test] -fn duplicate_vote_on_proposal_already_voted_fails() { +fn duplicate_triumvirate_vote_on_proposal_already_voted_fails() { TestState::default().build_and_execute(|| { let (proposal_hash, proposal_index) = create_proposal!(); let aye_voter = RuntimeOrigin::signed(U256::from(1001)); let approve = true; - assert_ok!(Pallet::::vote( + assert_ok!(Pallet::::vote_on_proposed( aye_voter.clone(), proposal_hash, proposal_index, approve )); assert_noop!( - Pallet::::vote(aye_voter, proposal_hash, proposal_index, approve), + Pallet::::vote_on_proposed(aye_voter, proposal_hash, proposal_index, approve), Error::::DuplicateVote ); let nay_voter = RuntimeOrigin::signed(U256::from(1002)); let approve = false; - assert_ok!(Pallet::::vote( + assert_ok!(Pallet::::vote_on_proposed( nay_voter.clone(), proposal_hash, proposal_index, approve )); assert_noop!( - Pallet::::vote(nay_voter, proposal_hash, proposal_index, approve), + Pallet::::vote_on_proposed(nay_voter, proposal_hash, proposal_index, approve), Error::::DuplicateVote ); }); } #[test] -fn aye_vote_on_proposal_with_too_many_scheduled_fails() { +fn triumvirate_aye_vote_on_proposal_with_too_many_scheduled_fails() { TestState::default().build_and_execute(|| { // We fill the scheduled proposals up to the maximum. for i in 0..MaxScheduled::get() { @@ -986,15 +994,15 @@ fn aye_vote_on_proposal_with_too_many_scheduled_fails() { items: vec![(b"Foobar".to_vec(), i.to_be_bytes().to_vec())], } ); - vote_aye!(U256::from(1001), proposal_hash, proposal_index); - vote_aye!(U256::from(1002), proposal_hash, proposal_index); + vote_aye_on_proposed!(U256::from(1001), proposal_hash, proposal_index); + vote_aye_on_proposed!(U256::from(1002), proposal_hash, proposal_index); } let (proposal_hash, proposal_index) = create_proposal!(); - vote_aye!(U256::from(1001), proposal_hash, proposal_index); + vote_aye_on_proposed!(U256::from(1001), proposal_hash, proposal_index); assert_noop!( - Pallet::::vote( + Pallet::::vote_on_proposed( RuntimeOrigin::signed(U256::from(1002)), proposal_hash, proposal_index, @@ -1006,128 +1014,41 @@ fn aye_vote_on_proposal_with_too_many_scheduled_fails() { } #[test] -fn collective_vote_from_non_collective_member_fails() { - TestState::default().build_and_execute(|| { - let (proposal_hash, proposal_index) = create_scheduled_proposal!(); - - assert_noop!( - Pallet::::collective_vote( - RuntimeOrigin::signed(U256::from(42)), - proposal_hash, - proposal_index, - true - ), - Error::::NotCollectiveMember - ); - }); -} - -#[test] -fn collective_vote_on_non_scheduled_proposal_fails() { - TestState::default().build_and_execute(|| { - let (proposal_hash, proposal_index) = create_proposal!(); - - assert_noop!( - Pallet::::collective_vote( - RuntimeOrigin::signed(U256::from(2001)), - proposal_hash, - proposal_index, - true - ), - Error::::ProposalNotScheduled - ); - }); -} - -#[test] -fn collective_vote_on_proposal_with_wrong_index_fails() { - TestState::default().build_and_execute(|| { - let (proposal_hash, _proposal_index) = create_scheduled_proposal!(); - - assert_noop!( - Pallet::::collective_vote( - RuntimeOrigin::signed(U256::from(2001)), - proposal_hash, - 42, - true - ), - Error::::WrongProposalIndex - ); - }); -} - -#[test] -fn duplicate_collective_vote_on_scheduled_proposal_already_voted_fails() { - TestState::default().build_and_execute(|| { - let (proposal_hash, proposal_index) = create_scheduled_proposal!(); - - let aye_voter = RuntimeOrigin::signed(U256::from(2001)); - let approve = true; - assert_ok!(Pallet::::collective_vote( - aye_voter.clone(), - proposal_hash, - proposal_index, - approve - )); - assert_noop!( - Pallet::::collective_vote(aye_voter, proposal_hash, proposal_index, approve), - Error::::DuplicateVote - ); - - let nay_voter = RuntimeOrigin::signed(U256::from(2002)); - let approve = false; - assert_ok!(Pallet::::collective_vote( - nay_voter.clone(), - proposal_hash, - proposal_index, - approve - )); - assert_noop!( - Pallet::::collective_vote(nay_voter, proposal_hash, proposal_index, approve), - Error::::DuplicateVote - ); - }); -} - -#[test] -fn basic_collective_aye_vote_on_scheduled_proposal_works() { +fn collective_aye_vote_on_scheduled_proposal_works() { TestState::default().build_and_execute(|| { let (proposal_hash, proposal_index) = create_scheduled_proposal!(); // Add an aye vote from an economic collective member. - assert_ok!(Pallet::::collective_vote( - RuntimeOrigin::signed(U256::from(2001)), + let economic_member = U256::from(2001); + assert_ok!(Pallet::::vote_on_scheduled( + RuntimeOrigin::signed(economic_member), proposal_hash, proposal_index, true )); - assert_eq!( CollectiveVoting::::get(proposal_hash), Some(CollectiveVotes { index: proposal_index, - economic_ayes: BoundedVec::truncate_from(vec![U256::from(2001)]), - economic_nays: BoundedVec::new(), - building_ayes: BoundedVec::new(), - building_nays: BoundedVec::new(), + ayes: BoundedVec::truncate_from(vec![economic_member]), + nays: BoundedVec::new(), }) ); assert_eq!( last_event(), RuntimeEvent::Governance(Event::::CollectiveMemberVoted { - account: CollectiveMember::Economic(U256::from(2001)), + account: economic_member, proposal_hash, voted: true, - economic_yes: 1, - economic_no: 0, - building_yes: 0, - building_no: 0, + yes: 1, + no: 0, }) ); // Add a second aye vote from a building collective member. - assert_ok!(Pallet::::collective_vote( - RuntimeOrigin::signed(U256::from(3001)), + let building_member = U256::from(3001); + assert_ok!(Pallet::::vote_on_scheduled( + RuntimeOrigin::signed(building_member), proposal_hash, proposal_index, true @@ -1137,22 +1058,18 @@ fn basic_collective_aye_vote_on_scheduled_proposal_works() { CollectiveVoting::::get(proposal_hash), Some(CollectiveVotes { index: proposal_index, - economic_ayes: BoundedVec::truncate_from(vec![U256::from(2001)]), - economic_nays: BoundedVec::new(), - building_ayes: BoundedVec::truncate_from(vec![U256::from(3001)]), - building_nays: BoundedVec::new(), + ayes: BoundedVec::truncate_from(vec![economic_member, building_member]), + nays: BoundedVec::new(), }) ); assert_eq!( last_event(), RuntimeEvent::Governance(Event::::CollectiveMemberVoted { - account: CollectiveMember::Building(U256::from(3001)), + account: building_member, proposal_hash, voted: true, - economic_yes: 1, - economic_no: 0, - building_yes: 1, - building_no: 0, + yes: 2, + no: 0, }) ); }); @@ -1165,128 +1082,50 @@ fn collective_vote_can_be_updated() { let economic_member = U256::from(2001); // Vote aye initially as an economic collective member - collective_vote_aye!(economic_member, proposal_hash, proposal_index); - let votes = CollectiveVoting::::get(proposal_hash).unwrap(); - assert_eq!(votes.economic_ayes.to_vec(), vec![economic_member]); - assert!(votes.economic_nays.to_vec().is_empty()); - assert!(votes.building_ayes.to_vec().is_empty()); - assert!(votes.building_nays.to_vec().is_empty()); - assert_eq!( - last_event(), - RuntimeEvent::Governance(Event::::CollectiveMemberVoted { - account: CollectiveMember::Economic(economic_member), - proposal_hash, - voted: true, - economic_yes: 1, - economic_no: 0, - building_yes: 0, - building_no: 0, - }) - ); - - // Then vote nay, replacing the aye vote - collective_vote_nay!(economic_member, proposal_hash, proposal_index); - let votes = CollectiveVoting::::get(proposal_hash).unwrap(); - assert!(votes.economic_ayes.to_vec().is_empty()); - assert_eq!(votes.economic_nays.to_vec(), vec![economic_member]); - assert!(votes.building_ayes.to_vec().is_empty()); - assert!(votes.building_nays.to_vec().is_empty()); - assert_eq!( - last_event(), - RuntimeEvent::Governance(Event::::CollectiveMemberVoted { - account: CollectiveMember::Economic(economic_member), - proposal_hash, - voted: false, - economic_yes: 0, - economic_no: 1, - building_yes: 0, - building_no: 0, - }) - ); - - // Then vote aye again, replacing the nay vote - collective_vote_aye!(economic_member, proposal_hash, proposal_index); - let votes = CollectiveVoting::::get(proposal_hash).unwrap(); - assert_eq!(votes.economic_ayes.to_vec(), vec![economic_member]); - assert!(votes.economic_nays.to_vec().is_empty()); - assert!(votes.building_ayes.to_vec().is_empty()); - assert!(votes.building_nays.to_vec().is_empty()); - assert_eq!( - last_event(), - RuntimeEvent::Governance(Event::::CollectiveMemberVoted { - account: CollectiveMember::Economic(economic_member), - proposal_hash, - voted: true, - economic_yes: 1, - economic_no: 0, - building_yes: 0, - building_no: 0, - }) - ); - - // Trigger cleanup to avoid duplicate scheduled error - run_to_block(frame_system::Pallet::::block_number() + CleanupPeriod::get()); - - let (proposal_hash, proposal_index) = create_scheduled_proposal!(); - let building_member = U256::from(3001); - - // Vote aye initially as a building collective member - collective_vote_aye!(building_member, proposal_hash, proposal_index); + vote_aye_on_scheduled!(economic_member, proposal_hash, proposal_index); let votes = CollectiveVoting::::get(proposal_hash).unwrap(); - assert!(votes.economic_ayes.to_vec().is_empty()); - assert!(votes.economic_nays.to_vec().is_empty()); - assert_eq!(votes.building_ayes.to_vec(), vec![building_member]); - assert!(votes.building_nays.to_vec().is_empty()); + assert_eq!(votes.ayes.to_vec(), vec![economic_member]); + assert!(votes.nays.to_vec().is_empty()); assert_eq!( last_event(), RuntimeEvent::Governance(Event::::CollectiveMemberVoted { - account: CollectiveMember::Building(building_member), + account: economic_member, proposal_hash, voted: true, - economic_yes: 0, - economic_no: 0, - building_yes: 1, - building_no: 0, + yes: 1, + no: 0, }) ); // Then vote nay, replacing the aye vote - collective_vote_nay!(building_member, proposal_hash, proposal_index); + vote_nay_on_scheduled!(economic_member, proposal_hash, proposal_index); let votes = CollectiveVoting::::get(proposal_hash).unwrap(); - assert!(votes.economic_ayes.to_vec().is_empty()); - assert!(votes.economic_nays.to_vec().is_empty()); - assert!(votes.building_ayes.to_vec().is_empty()); - assert_eq!(votes.building_nays.to_vec(), vec![building_member]); + assert!(votes.ayes.to_vec().is_empty()); + assert_eq!(votes.nays.to_vec(), vec![economic_member]); assert_eq!( last_event(), RuntimeEvent::Governance(Event::::CollectiveMemberVoted { - account: CollectiveMember::Building(building_member), + account: economic_member, proposal_hash, voted: false, - economic_yes: 0, - economic_no: 0, - building_yes: 0, - building_no: 1, + yes: 0, + no: 1, }) ); // Then vote aye again, replacing the nay vote - collective_vote_aye!(building_member, proposal_hash, proposal_index); + vote_aye_on_scheduled!(economic_member, proposal_hash, proposal_index); let votes = CollectiveVoting::::get(proposal_hash).unwrap(); - assert!(votes.economic_ayes.to_vec().is_empty()); - assert!(votes.economic_nays.to_vec().is_empty()); - assert_eq!(votes.building_ayes.to_vec(), vec![building_member]); - assert!(votes.building_nays.to_vec().is_empty()); + assert_eq!(votes.ayes.to_vec(), vec![economic_member]); + assert!(votes.nays.to_vec().is_empty()); assert_eq!( last_event(), RuntimeEvent::Governance(Event::::CollectiveMemberVoted { - account: CollectiveMember::Building(building_member), + account: economic_member, proposal_hash, voted: true, - economic_yes: 0, - economic_no: 0, - building_yes: 1, - building_no: 0, + yes: 1, + no: 0, }) ); }); @@ -1294,15 +1133,15 @@ fn collective_vote_can_be_updated() { #[test] fn collective_aye_votes_to_threshold_on_scheduled_proposal_fast_tracks() { - fn execute_for( - collective: impl IntoIterator::AccountId>, - collective_size: u32, - ) { + TestState::default().build_and_execute(|| { let (proposal_hash, proposal_index) = create_scheduled_proposal!(); - let threshold = FastTrackThreshold::get().mul_ceil(collective_size); + let threshold = FastTrackThreshold::get().mul_ceil(TOTAL_COLLECTIVES_SIZE); + let combined_collective = EconomicCollective::::get() + .into_iter() + .chain(BuildingCollective::::get().into_iter()); - for member in collective.into_iter().take(threshold as usize) { - collective_vote_aye!(member, proposal_hash, proposal_index); + for member in combined_collective.into_iter().take(threshold as usize) { + vote_aye_on_scheduled!(member, proposal_hash, proposal_index); } assert!(Scheduled::::get().is_empty()); @@ -1317,26 +1156,20 @@ fn collective_aye_votes_to_threshold_on_scheduled_proposal_fast_tracks() { last_event(), RuntimeEvent::Governance(Event::::ScheduledProposalFastTracked { proposal_hash }) ); - } - - TestState::default().build_and_execute(|| { - execute_for(EconomicCollective::::get(), ECONOMIC_COLLECTIVE_SIZE); - run_to_block(frame_system::Pallet::::block_number() + 1); - execute_for(BuildingCollective::::get(), BUILDING_COLLECTIVE_SIZE); }); } #[test] fn collective_nay_votes_to_threshold_on_scheduled_proposal_cancels() { - fn execute_for( - collective: impl IntoIterator::AccountId>, - collective_size: u32, - ) { + TestState::default().build_and_execute(|| { let (proposal_hash, proposal_index) = create_scheduled_proposal!(); - let threshold = CancellationThreshold::get().mul_ceil(collective_size); + let threshold = CancellationThreshold::get().mul_ceil(TOTAL_COLLECTIVES_SIZE); + let combined_collective = EconomicCollective::::get() + .into_iter() + .chain(BuildingCollective::::get().into_iter()); - for member in collective.into_iter().take(threshold as usize) { - collective_vote_nay!(member, proposal_hash, proposal_index); + for member in combined_collective.into_iter().take(threshold as usize) { + vote_nay_on_scheduled!(member, proposal_hash, proposal_index); } assert!(Scheduled::::get().is_empty()); @@ -1347,23 +1180,90 @@ fn collective_nay_votes_to_threshold_on_scheduled_proposal_cancels() { last_event(), RuntimeEvent::Governance(Event::::ScheduledProposalCancelled { proposal_hash }) ); - } + }); +} +#[test] +fn collective_vote_from_non_collective_member_fails() { TestState::default().build_and_execute(|| { - execute_for(EconomicCollective::::get(), ECONOMIC_COLLECTIVE_SIZE); - execute_for(BuildingCollective::::get(), BUILDING_COLLECTIVE_SIZE); + let (proposal_hash, proposal_index) = create_scheduled_proposal!(); + + assert_noop!( + Pallet::::vote_on_scheduled( + RuntimeOrigin::signed(U256::from(42)), + proposal_hash, + proposal_index, + true + ), + Error::::NotCollectiveMember + ); }); } #[test] -fn cleanup_run_on_initialize() { +fn collective_vote_on_non_scheduled_proposal_fails() { TestState::default().build_and_execute(|| { - let now = frame_system::Pallet::::block_number(); - run_to_block(now + CleanupPeriod::get()); - assert!(Scheduled::::get().is_empty()); - assert!(CollectiveVoting::::get(proposal_hash).is_none()); - let task_name: [u8; 32] = proposal_hash.as_ref().try_into().unwrap(); - assert!(pallet_scheduler::Lookup::::get(task_name).is_none()); + let (proposal_hash, proposal_index) = create_proposal!(); + + assert_noop!( + Pallet::::vote_on_scheduled( + RuntimeOrigin::signed(U256::from(2001)), + proposal_hash, + proposal_index, + true + ), + Error::::ProposalNotScheduled + ); + }); +} + +#[test] +fn collective_vote_on_proposal_with_wrong_index_fails() { + TestState::default().build_and_execute(|| { + let (proposal_hash, _proposal_index) = create_scheduled_proposal!(); + + assert_noop!( + Pallet::::vote_on_scheduled( + RuntimeOrigin::signed(U256::from(2001)), + proposal_hash, + 42, + true + ), + Error::::WrongProposalIndex + ); + }); +} + +#[test] +fn duplicate_collective_vote_on_scheduled_proposal_already_voted_fails() { + TestState::default().build_and_execute(|| { + let (proposal_hash, proposal_index) = create_scheduled_proposal!(); + + let aye_voter = RuntimeOrigin::signed(U256::from(2001)); + let approve = true; + assert_ok!(Pallet::::vote_on_scheduled( + aye_voter.clone(), + proposal_hash, + proposal_index, + approve + )); + assert_noop!( + Pallet::::vote_on_scheduled(aye_voter, proposal_hash, proposal_index, approve), + Error::::DuplicateVote + ); + + let nay_voter = RuntimeOrigin::signed(U256::from(2002)); + let approve = false; + assert_ok!(Pallet::::vote_on_scheduled( + nay_voter.clone(), + proposal_hash, + proposal_index, + approve + )); + assert_noop!( + Pallet::::vote_on_scheduled(nay_voter, proposal_hash, proposal_index, approve), + Error::::DuplicateVote + ); }); } @@ -1444,16 +1344,16 @@ macro_rules! create_proposal { macro_rules! create_scheduled_proposal { () => {{ let (proposal_hash, proposal_index) = create_proposal!(); - vote_aye!(U256::from(1001), proposal_hash, proposal_index); - vote_aye!(U256::from(1002), proposal_hash, proposal_index); + vote_aye_on_proposed!(U256::from(1001), proposal_hash, proposal_index); + vote_aye_on_proposed!(U256::from(1002), proposal_hash, proposal_index); (proposal_hash, proposal_index) }}; } #[macro_export] -macro_rules! vote_aye { +macro_rules! vote_aye_on_proposed { ($voter:expr, $proposal_hash:expr, $proposal_index:expr) => {{ - assert_ok!(Pallet::::vote( + assert_ok!(Pallet::::vote_on_proposed( RuntimeOrigin::signed($voter), $proposal_hash, $proposal_index, @@ -1463,9 +1363,9 @@ macro_rules! vote_aye { } #[macro_export] -macro_rules! vote_nay { +macro_rules! vote_nay_on_proposed { ($voter:expr, $proposal_hash:expr, $proposal_index:expr) => {{ - assert_ok!(Pallet::::vote( + assert_ok!(Pallet::::vote_on_proposed( RuntimeOrigin::signed($voter), $proposal_hash, $proposal_index, @@ -1475,9 +1375,9 @@ macro_rules! vote_nay { } #[macro_export] -macro_rules! collective_vote_aye { +macro_rules! vote_aye_on_scheduled { ($voter:expr, $proposal_hash:expr, $proposal_index:expr) => {{ - assert_ok!(Pallet::::collective_vote( + assert_ok!(Pallet::::vote_on_scheduled( RuntimeOrigin::signed($voter), $proposal_hash, $proposal_index, @@ -1487,9 +1387,9 @@ macro_rules! collective_vote_aye { } #[macro_export] -macro_rules! collective_vote_nay { +macro_rules! vote_nay_on_scheduled { ($voter:expr, $proposal_hash:expr, $proposal_index:expr) => {{ - assert_ok!(Pallet::::collective_vote( + assert_ok!(Pallet::::vote_on_scheduled( RuntimeOrigin::signed($voter), $proposal_hash, $proposal_index, From cc35013bbb219ad1b02f034a83940f281ed84920 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Thu, 13 Nov 2025 17:26:01 -0300 Subject: [PATCH 21/23] adjust delay using net score --- pallets/governance/src/lib.rs | 61 +++++++++++++++------------------ pallets/governance/src/tests.rs | 8 +++++ 2 files changed, 35 insertions(+), 34 deletions(-) diff --git a/pallets/governance/src/lib.rs b/pallets/governance/src/lib.rs index fa48f8f711..7e5086b093 100644 --- a/pallets/governance/src/lib.rs +++ b/pallets/governance/src/lib.rs @@ -320,6 +320,11 @@ pub mod pallet { ScheduledProposalFastTracked { proposal_hash: T::Hash }, /// A scheduled proposal has been cancelled by collectives. ScheduledProposalCancelled { proposal_hash: T::Hash }, + /// A scheduled proposal schedule time has been delayed by collectives. + ScheduledProposalDelayAdjusted { + proposal_hash: T::Hash, + dispatch_time: DispatchTime>, + }, } #[pallet::error] @@ -688,7 +693,7 @@ impl Pallet { proposal_hash: T::Hash, index: ProposalIndex, approve: bool, - ) -> Result, DispatchError> { + ) -> Result>, DispatchError> { CollectiveVoting::::try_mutate(proposal_hash, |voting| { let voting = voting.as_mut().ok_or(Error::::ProposalNotScheduled)?; ensure!(voting.index == index, Error::::WrongProposalIndex); @@ -795,50 +800,38 @@ impl Pallet { fn do_adjust_delay( proposal_hash: T::Hash, - mut voting: CollectiveVotes, + mut voting: CollectiveVotes>, ) -> DispatchResult { let net_score = voting.nays.len() as i32 - voting.ayes.len() as i32; - let now = frame_system::Pallet::::block_number(); - let name = Self::task_name_from_hash(proposal_hash)?; // Delay based on net opposition - let additional_delay = if new_score > 0 { - T::InitialSchedulingDelay::get() - .saturating_mul(1.5_f64.powi(net_score as u32)) - .ceil() as BlockNumberFor + let additional_delay = if net_score > 0 { + let initial_delay = T::InitialSchedulingDelay::get().into().as_u64() as f64; + let multiplier = 1.5_f64.powi(net_score as i32); + ((initial_delay * multiplier).ceil() as u32).into() } else { - Zero::zero(); + Zero::zero() }; - - let - if net_score > 0 { - let new_delay = 2_u64.pow(net_score as u32) * T::InitialSchedulingDelay::get(); - let is_past_new_delay = now >= voting.initial_dispatch_time + new_delay; + let now = frame_system::Pallet::::block_number(); + let elapsed_time = now.saturating_sub(voting.initial_dispatch_time); - // New delay is lower and we are past it, we should fast track - if new_delay < voting.delay && is_past_new_delay { - Self::do_fast_track(proposal_hash)?; - return; - } + // We are past new delay, fast track + if elapsed_time >= additional_delay { + return Self::do_fast_track(proposal_hash); + } - // New delay is higher, adjust delay - voting.delay = new_delay; - let new_dispatch_time = DispatchTime::At(voting.initial_dispatch_time + new_delay); - T::Scheduler::reschedule_named(name, new_dispatch_time)?; - } else { - // New delay is reset to 0 and we are past initial dispatch time, fast track - if now >= voting.initial_dispatch_time { - Self::do_fast_track(proposal_hash)?; - return; - } + let name = Self::task_name_from_hash(proposal_hash)?; + let dispatch_time = DispatchTime::At(voting.initial_dispatch_time + additional_delay); + T::Scheduler::reschedule_named(name, dispatch_time)?; - // New delay is reset to 0 and we are not past initial dispatch time, adjust delay - voting.delay = 0; - let new_dispatch_time = DispatchTime::At(voting.initial_dispatch_time); - T::Scheduler::reschedule_named(name, new_dispatch_time)?; - } + voting.delay = additional_delay; + CollectiveVoting::::insert(proposal_hash, voting); + Self::deposit_event(Event::::ScheduledProposalDelayAdjusted { + proposal_hash, + dispatch_time, + }); Ok(()) } diff --git a/pallets/governance/src/tests.rs b/pallets/governance/src/tests.rs index 5edc9f6bcc..b170f9b190 100644 --- a/pallets/governance/src/tests.rs +++ b/pallets/governance/src/tests.rs @@ -787,12 +787,15 @@ fn two_triumvirate_aye_votes_schedule_proposal() { assert!(Proposals::::get().is_empty()); assert!(!TriumvirateVoting::::contains_key(proposal_hash)); assert_eq!(Scheduled::::get(), vec![proposal_hash]); + let now = frame_system::Pallet::::block_number(); assert_eq!( CollectiveVoting::::get(proposal_hash), Some(CollectiveVotes { index: proposal_index, ayes: BoundedVec::new(), nays: BoundedVec::new(), + initial_dispatch_time: now + MotionDuration::get(), + delay: Zero::zero(), }) ); let task_name: [u8; 32] = proposal_hash.as_ref().try_into().unwrap(); @@ -1026,12 +1029,15 @@ fn collective_aye_vote_on_scheduled_proposal_works() { proposal_index, true )); + let now = frame_system::Pallet::::block_number(); assert_eq!( CollectiveVoting::::get(proposal_hash), Some(CollectiveVotes { index: proposal_index, ayes: BoundedVec::truncate_from(vec![economic_member]), nays: BoundedVec::new(), + initial_dispatch_time: now + MotionDuration::get(), + delay: Zero::zero(), }) ); assert_eq!( @@ -1060,6 +1066,8 @@ fn collective_aye_vote_on_scheduled_proposal_works() { index: proposal_index, ayes: BoundedVec::truncate_from(vec![economic_member, building_member]), nays: BoundedVec::new(), + initial_dispatch_time: now + MotionDuration::get(), + delay: Zero::zero(), }) ); assert_eq!( From fd8be5f3cc8850f96eda71ef2e54865edcc4cad1 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Fri, 14 Nov 2025 10:49:10 -0300 Subject: [PATCH 22/23] fix tests --- pallets/governance/src/lib.rs | 11 ++++++++--- pallets/governance/src/tests.rs | 4 ++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/pallets/governance/src/lib.rs b/pallets/governance/src/lib.rs index 7e5086b093..8692f286cc 100644 --- a/pallets/governance/src/lib.rs +++ b/pallets/governance/src/lib.rs @@ -813,13 +813,18 @@ impl Pallet { Zero::zero() }; + // No change, no need to reschedule + if voting.delay == additional_delay { + return Ok(()); + } + let now = frame_system::Pallet::::block_number(); let elapsed_time = now.saturating_sub(voting.initial_dispatch_time); // We are past new delay, fast track - if elapsed_time >= additional_delay { - return Self::do_fast_track(proposal_hash); - } + // if elapsed_time >= additional_delay { + // return Self::do_fast_track(proposal_hash); + // } let name = Self::task_name_from_hash(proposal_hash)?; let dispatch_time = DispatchTime::At(voting.initial_dispatch_time + additional_delay); diff --git a/pallets/governance/src/tests.rs b/pallets/governance/src/tests.rs index b170f9b190..689bf6595e 100644 --- a/pallets/governance/src/tests.rs +++ b/pallets/governance/src/tests.rs @@ -1111,7 +1111,7 @@ fn collective_vote_can_be_updated() { assert!(votes.ayes.to_vec().is_empty()); assert_eq!(votes.nays.to_vec(), vec![economic_member]); assert_eq!( - last_event(), + System::events().into_iter().rev().nth(3).unwrap().event, RuntimeEvent::Governance(Event::::CollectiveMemberVoted { account: economic_member, proposal_hash, @@ -1127,7 +1127,7 @@ fn collective_vote_can_be_updated() { assert_eq!(votes.ayes.to_vec(), vec![economic_member]); assert!(votes.nays.to_vec().is_empty()); assert_eq!( - last_event(), + System::events().into_iter().rev().nth(3).unwrap().event, RuntimeEvent::Governance(Event::::CollectiveMemberVoted { account: economic_member, proposal_hash, From f9c0246425c94777921c1e625994cd6de25e68eb Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Tue, 18 Nov 2025 10:10:17 -0300 Subject: [PATCH 23/23] fix fast track + tests --- pallets/governance/src/lib.rs | 27 ++-- pallets/governance/src/mock.rs | 17 +-- pallets/governance/src/tests.rs | 263 ++++++++++++++++++++++++++++---- 3 files changed, 254 insertions(+), 53 deletions(-) diff --git a/pallets/governance/src/lib.rs b/pallets/governance/src/lib.rs index 8692f286cc..60773a874c 100644 --- a/pallets/governance/src/lib.rs +++ b/pallets/governance/src/lib.rs @@ -785,7 +785,7 @@ impl Pallet { // It will be scheduled on the next block because scheduler already ran for this block. DispatchTime::After(Zero::zero()), )?; - Self::clear_scheduled_proposal(proposal_hash); + CollectiveVoting::::remove(&proposal_hash); Self::deposit_event(Event::::ScheduledProposalFastTracked { proposal_hash }); Ok(()) } @@ -804,14 +804,7 @@ impl Pallet { ) -> DispatchResult { let net_score = voting.nays.len() as i32 - voting.ayes.len() as i32; - // Delay based on net opposition - let additional_delay = if net_score > 0 { - let initial_delay = T::InitialSchedulingDelay::get().into().as_u64() as f64; - let multiplier = 1.5_f64.powi(net_score as i32); - ((initial_delay * multiplier).ceil() as u32).into() - } else { - Zero::zero() - }; + let additional_delay = Self::compute_additional_delay(net_score); // No change, no need to reschedule if voting.delay == additional_delay { @@ -822,9 +815,9 @@ impl Pallet { let elapsed_time = now.saturating_sub(voting.initial_dispatch_time); // We are past new delay, fast track - // if elapsed_time >= additional_delay { - // return Self::do_fast_track(proposal_hash); - // } + if elapsed_time > additional_delay { + return Self::do_fast_track(proposal_hash); + } let name = Self::task_name_from_hash(proposal_hash)?; let dispatch_time = DispatchTime::At(voting.initial_dispatch_time + additional_delay); @@ -956,4 +949,14 @@ impl Pallet { .try_into() .map_err(|_| Error::::InvalidProposalHashLength)?) } + + fn compute_additional_delay(net_score: i32) -> BlockNumberFor { + if net_score > 0 { + let initial_delay = T::InitialSchedulingDelay::get().into().as_u64() as f64; + let multiplier = 1.5_f64.powi(net_score.abs()); + ((initial_delay * multiplier).ceil() as u32).into() + } else { + Zero::zero() + } + } } diff --git a/pallets/governance/src/mock.rs b/pallets/governance/src/mock.rs index dbc869cf4a..2aea0165c5 100644 --- a/pallets/governance/src/mock.rs +++ b/pallets/governance/src/mock.rs @@ -250,18 +250,17 @@ impl TestState { } } -pub(crate) fn last_event() -> RuntimeEvent { - System::events().pop().expect("RuntimeEvent expected").event -} - -pub(crate) fn last_n_events(n: usize) -> Vec { +pub(crate) fn nth_last_event(n: usize) -> RuntimeEvent { System::events() .into_iter() .rev() - .take(n) - .rev() - .map(|e| e.event) - .collect() + .nth(n) + .expect("RuntimeEvent expected") + .event +} + +pub(crate) fn last_event() -> RuntimeEvent { + nth_last_event(0) } pub(crate) fn run_to_block(n: BlockNumberFor) { diff --git a/pallets/governance/src/tests.rs b/pallets/governance/src/tests.rs index 689bf6595e..8cea9e8b1a 100644 --- a/pallets/governance/src/tests.rs +++ b/pallets/governance/src/tests.rs @@ -798,15 +798,13 @@ fn two_triumvirate_aye_votes_schedule_proposal() { delay: Zero::zero(), }) ); - let task_name: [u8; 32] = proposal_hash.as_ref().try_into().unwrap(); let now = frame_system::Pallet::::block_number(); assert_eq!( - pallet_scheduler::Lookup::::get(task_name).unwrap().0, + get_scheduler_proposal_task(proposal_hash).unwrap().0, now + MotionDuration::get() ); - let events = last_n_events(3); assert_eq!( - events[0], + nth_last_event(2), RuntimeEvent::Governance(Event::::TriumvirateMemberVoted { account: U256::from(1003), proposal_hash, @@ -816,7 +814,7 @@ fn two_triumvirate_aye_votes_schedule_proposal() { }) ); assert_eq!( - events[2], + last_event(), RuntimeEvent::Governance(Event::::ProposalScheduled { proposal_hash }) ); }); @@ -834,10 +832,9 @@ fn two_triumvirate_nay_votes_cancel_proposal() { assert!(Proposals::::get().is_empty()); assert!(!TriumvirateVoting::::contains_key(proposal_hash)); assert!(Scheduled::::get().is_empty()); - assert_eq!(ProposalOf::::get(proposal_hash), None); - let events = last_n_events(2); + assert!(ProposalOf::::get(proposal_hash).is_none()); assert_eq!( - events[0], + nth_last_event(1), RuntimeEvent::Governance(Event::::TriumvirateMemberVoted { account: U256::from(1003), proposal_hash, @@ -847,7 +844,7 @@ fn two_triumvirate_nay_votes_cancel_proposal() { }) ); assert_eq!( - events[1], + last_event(), RuntimeEvent::Governance(Event::::ProposalCancelled { proposal_hash }) ); }); @@ -1083,6 +1080,205 @@ fn collective_aye_vote_on_scheduled_proposal_works() { }); } +#[test] +fn collective_votes_succession_adjust_delay_and_can_fast_track() { + TestState::default().build_and_execute(|| { + let now = frame_system::Pallet::::block_number(); + let (proposal_hash, proposal_index) = create_scheduled_proposal!(); + let voting = CollectiveVoting::::get(proposal_hash).unwrap(); + assert_eq!(voting.delay, 0); + + // Adding a nay vote increases the delay + vote_nay_on_scheduled!(U256::from(2001), proposal_hash, proposal_index); + let initial_delay = InitialSchedulingDelay::get() as f64; + let initial_dispatch_time = now + MotionDuration::get(); + let delay = (initial_delay * 1.5_f64.powi(1)).ceil() as u64; + assert_eq!( + CollectiveVoting::::get(proposal_hash), + Some(CollectiveVotes { + index: proposal_index, + ayes: BoundedVec::new(), + nays: BoundedVec::truncate_from(vec![U256::from(2001)]), + initial_dispatch_time, + delay, + }) + ); + assert_eq!( + get_scheduler_proposal_task(proposal_hash).unwrap().0, + initial_dispatch_time + delay + ); + assert_eq!( + nth_last_event(3), + RuntimeEvent::Governance(Event::::CollectiveMemberVoted { + account: U256::from(2001), + proposal_hash, + voted: false, + yes: 0, + no: 1, + }) + ); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::ScheduledProposalDelayAdjusted { + proposal_hash, + dispatch_time: DispatchTime::At(initial_dispatch_time + delay), + }) + ); + + // Adding a second nay vote increases the delay + vote_nay_on_scheduled!(U256::from(2002), proposal_hash, proposal_index); + let delay = (initial_delay * 1.5_f64.powi(2)).ceil() as u64; + assert_eq!( + CollectiveVoting::::get(proposal_hash), + Some(CollectiveVotes { + index: proposal_index, + ayes: BoundedVec::new(), + nays: BoundedVec::truncate_from(vec![U256::from(2001), U256::from(2002)]), + initial_dispatch_time, + delay, + }) + ); + assert_eq!( + get_scheduler_proposal_task(proposal_hash).unwrap().0, + initial_dispatch_time + delay + ); + assert_eq!( + nth_last_event(3), + RuntimeEvent::Governance(Event::::CollectiveMemberVoted { + account: U256::from(2002), + proposal_hash, + voted: false, + yes: 0, + no: 2, + }) + ); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::ScheduledProposalDelayAdjusted { + proposal_hash, + dispatch_time: DispatchTime::At(initial_dispatch_time + delay), + }) + ); + + // Adding a third nay vote increases the delay + vote_nay_on_scheduled!(U256::from(2003), proposal_hash, proposal_index); + let delay = (initial_delay * 1.5_f64.powi(3)).ceil() as u64; + assert_eq!( + CollectiveVoting::::get(proposal_hash), + Some(CollectiveVotes { + index: proposal_index, + ayes: BoundedVec::new(), + nays: BoundedVec::truncate_from(vec![ + U256::from(2001), + U256::from(2002), + U256::from(2003) + ]), + initial_dispatch_time, + delay, + }) + ); + assert_eq!( + get_scheduler_proposal_task(proposal_hash).unwrap().0, + initial_dispatch_time + delay + ); + assert_eq!( + nth_last_event(3), + RuntimeEvent::Governance(Event::::CollectiveMemberVoted { + account: U256::from(2003), + proposal_hash, + voted: false, + yes: 0, + no: 3, + }) + ); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::ScheduledProposalDelayAdjusted { + proposal_hash, + dispatch_time: DispatchTime::At(initial_dispatch_time + delay), + }) + ); + + // Adding a aye vote decreases the delay because net score become lower + vote_aye_on_scheduled!(U256::from(2004), proposal_hash, proposal_index); + let delay = (initial_delay * 1.5_f64.powi(2)).ceil() as u64; + assert_eq!( + CollectiveVoting::::get(proposal_hash), + Some(CollectiveVotes { + index: proposal_index, + ayes: BoundedVec::truncate_from(vec![U256::from(2004)]), + nays: BoundedVec::truncate_from(vec![ + U256::from(2001), + U256::from(2002), + U256::from(2003) + ]), + initial_dispatch_time, + delay, + }) + ); + assert_eq!( + get_scheduler_proposal_task(proposal_hash).unwrap().0, + initial_dispatch_time + delay + ); + assert_eq!( + nth_last_event(3), + RuntimeEvent::Governance(Event::::CollectiveMemberVoted { + account: U256::from(2004), + proposal_hash, + voted: true, + yes: 1, + no: 3, + }) + ); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::ScheduledProposalDelayAdjusted { + proposal_hash, + dispatch_time: DispatchTime::At(initial_dispatch_time + delay), + }) + ); + + // Now let's run some blocks until before the sheduled time + run_to_block(initial_dispatch_time + delay - 5); + // Task hasn't been executed yet + assert!(get_scheduler_proposal_task(proposal_hash).is_some()); + + // Adding a new aye vote should fast track the proposal because the delay will + // fall below the elapsed time + vote_aye_on_scheduled!(U256::from(2005), proposal_hash, proposal_index); + assert!(CollectiveVoting::::get(proposal_hash).is_none()); + let now = frame_system::Pallet::::block_number(); + assert_eq!( + get_scheduler_proposal_task(proposal_hash).unwrap().0, + // Fast track here means next block scheduling + now + 1 + ); + // The proposal is still scheduled, even if next block, we keep track of it + assert_eq!(Scheduled::::get(), vec![proposal_hash]); + assert_eq!( + nth_last_event(3), + RuntimeEvent::Governance(Event::::CollectiveMemberVoted { + account: U256::from(2005), + proposal_hash, + voted: true, + yes: 2, + no: 3, + }) + ); + assert_eq!( + last_event(), + RuntimeEvent::Governance(Event::::ScheduledProposalFastTracked { proposal_hash }) + ); + + // Now let run one block to see the proposal executed + assert_eq!(sp_io::storage::get(b"Foobar"), None); // Not executed yet + run_to_block(now + delay + 1); + assert!(get_scheduler_proposal_task(proposal_hash).is_none()); + let stored_value = 42u32.to_be_bytes().to_vec().into(); + assert_eq!(sp_io::storage::get(b"Foobar"), Some(stored_value)); // Executed + }); +} + #[test] fn collective_vote_can_be_updated() { TestState::default().build_and_execute(|| { @@ -1153,11 +1349,10 @@ fn collective_aye_votes_to_threshold_on_scheduled_proposal_fast_tracks() { } assert!(Scheduled::::get().is_empty()); - assert_eq!(CollectiveVoting::::get(proposal_hash), None); - let task_name: [u8; 32] = proposal_hash.as_ref().try_into().unwrap(); + assert!(CollectiveVoting::::get(proposal_hash).is_none()); let now = frame_system::Pallet::::block_number(); assert_eq!( - pallet_scheduler::Lookup::::get(task_name).unwrap().0, + get_scheduler_proposal_task(proposal_hash).unwrap().0, now + 1 ); assert_eq!( @@ -1182,8 +1377,7 @@ fn collective_nay_votes_to_threshold_on_scheduled_proposal_cancels() { assert!(Scheduled::::get().is_empty()); assert!(CollectiveVoting::::get(proposal_hash).is_none()); - let task_name: [u8; 32] = proposal_hash.as_ref().try_into().unwrap(); - assert!(pallet_scheduler::Lookup::::get(task_name).is_none()); + assert!(get_scheduler_proposal_task(proposal_hash).is_none()); assert_eq!( last_event(), RuntimeEvent::Governance(Event::::ScheduledProposalCancelled { proposal_hash }) @@ -1247,29 +1441,27 @@ fn duplicate_collective_vote_on_scheduled_proposal_already_voted_fails() { TestState::default().build_and_execute(|| { let (proposal_hash, proposal_index) = create_scheduled_proposal!(); - let aye_voter = RuntimeOrigin::signed(U256::from(2001)); - let approve = true; - assert_ok!(Pallet::::vote_on_scheduled( - aye_voter.clone(), - proposal_hash, - proposal_index, - approve - )); + let aye_voter = U256::from(2001); + vote_aye_on_scheduled!(aye_voter, proposal_hash, proposal_index); assert_noop!( - Pallet::::vote_on_scheduled(aye_voter, proposal_hash, proposal_index, approve), + Pallet::::vote_on_scheduled( + RuntimeOrigin::signed(aye_voter), + proposal_hash, + proposal_index, + true + ), Error::::DuplicateVote ); - let nay_voter = RuntimeOrigin::signed(U256::from(2002)); - let approve = false; - assert_ok!(Pallet::::vote_on_scheduled( - nay_voter.clone(), - proposal_hash, - proposal_index, - approve - )); + let nay_voter = U256::from(2002); + vote_nay_on_scheduled!(nay_voter, proposal_hash, proposal_index); assert_noop!( - Pallet::::vote_on_scheduled(nay_voter, proposal_hash, proposal_index, approve), + Pallet::::vote_on_scheduled( + RuntimeOrigin::signed(nay_voter), + proposal_hash, + proposal_index, + false + ), Error::::DuplicateVote ); }); @@ -1405,3 +1597,10 @@ macro_rules! vote_nay_on_scheduled { )); }}; } + +pub(crate) fn get_scheduler_proposal_task( + proposal_hash: ::Hash, +) -> Option>> { + let task_name: [u8; 32] = proposal_hash.as_ref().try_into().unwrap(); + pallet_scheduler::Lookup::::get(task_name) +}