From 81a0e25899904a1645049a116236d1ae8b6380f8 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 20 Feb 2023 20:15:33 +0700 Subject: [PATCH 1/7] rename state-toggler to bouncer --- .../nomination-pools/benchmarking/src/lib.rs | 6 +- frame/nomination-pools/fuzzer/src/call.rs | 4 +- frame/nomination-pools/src/lib.rs | 60 +++++++++---------- frame/nomination-pools/src/migration.rs | 4 +- frame/nomination-pools/src/tests.rs | 46 +++++++------- 5 files changed, 60 insertions(+), 60 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index 32708ff53e01c..675a4a8324f43 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -518,7 +518,7 @@ frame_benchmarking::benchmarks! { depositor: depositor.clone(), root: Some(depositor.clone()), nominator: Some(depositor.clone()), - state_toggler: Some(depositor.clone()), + bouncer: Some(depositor.clone()), }, } ); @@ -557,7 +557,7 @@ frame_benchmarking::benchmarks! { depositor: depositor.clone(), root: Some(depositor.clone()), nominator: Some(depositor.clone()), - state_toggler: Some(depositor.clone()), + bouncer: Some(depositor.clone()), } } ); @@ -630,7 +630,7 @@ frame_benchmarking::benchmarks! { pallet_nomination_pools::PoolRoles { depositor: root, nominator: Some(random.clone()), - state_toggler: Some(random.clone()), + bouncer: Some(random.clone()), root: Some(random), }, ) diff --git a/frame/nomination-pools/fuzzer/src/call.rs b/frame/nomination-pools/fuzzer/src/call.rs index b07903609e8ab..805cc265d96e0 100644 --- a/frame/nomination-pools/fuzzer/src/call.rs +++ b/frame/nomination-pools/fuzzer/src/call.rs @@ -143,9 +143,9 @@ fn random_call(mut rng: &mut R) -> (pools::Call, RuntimeOrigin) { let amount = random_ed_multiple(&mut rng); fund_account(&mut rng, &who); let root = who; - let state_toggler = who; + let bouncer = who; let nominator = who; - (PoolsCall::::create { amount, root, state_toggler, nominator }, origin) + (PoolsCall::::create { amount, root, bouncer, nominator }, origin) }, 7 => { // nominate diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 3cb8abedda2fb..063bd57261da4 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -120,9 +120,9 @@ //! * Depositor: creates the pool and is the initial member. They can only leave the pool once all //! other members have left. Once they fully withdraw their funds, the pool is destroyed. //! * Nominator: can select which validators the pool nominates. -//! * State-Toggler: can change the pools state and kick members if the pool is blocked. -//! * Root: can change the nominator, state-toggler, or itself and can perform any of the actions -//! the nominator or state-toggler can. +//! * Bouncer: can change the pools state and kick members if the pool is blocked. +//! * Root: can change the nominator, bouncer, or itself and can perform any of the actions +//! the nominator or bouncer can. //! //! ### Dismantling //! @@ -573,13 +573,13 @@ pub struct PoolRoles { /// Creates the pool and is the initial member. They can only leave the pool once all other /// members have left. Once they fully leave, the pool is destroyed. pub depositor: AccountId, - /// Can change the nominator, state-toggler, or itself and can perform any of the actions the - /// nominator or state-toggler can. + /// Can change the nominator, bouncer, or itself and can perform any of the actions the + /// nominator or bouncer can. pub root: Option, /// Can select which validators the pool nominates. pub nominator: Option, /// Can change the pools state and kick members if the pool is blocked. - pub state_toggler: Option, + pub bouncer: Option, } /// Pool permissions and state @@ -734,11 +734,11 @@ impl BondedPool { self.roles.root.as_ref().map_or(false, |root| root == who) } - fn is_state_toggler(&self, who: &T::AccountId) -> bool { + fn is_bouncer(&self, who: &T::AccountId) -> bool { self.roles - .state_toggler + .bouncer .as_ref() - .map_or(false, |state_toggler| state_toggler == who) + .map_or(false, |bouncer| bouncer == who) } fn can_update_roles(&self, who: &T::AccountId) -> bool { @@ -751,15 +751,15 @@ impl BondedPool { } fn can_kick(&self, who: &T::AccountId) -> bool { - self.state == PoolState::Blocked && (self.is_root(who) || self.is_state_toggler(who)) + self.state == PoolState::Blocked && (self.is_root(who) || self.is_bouncer(who)) } fn can_toggle_state(&self, who: &T::AccountId) -> bool { - (self.is_root(who) || self.is_state_toggler(who)) && !self.is_destroying() + (self.is_root(who) || self.is_bouncer(who)) && !self.is_destroying() } fn can_set_metadata(&self, who: &T::AccountId) -> bool { - self.is_root(who) || self.is_state_toggler(who) + self.is_root(who) || self.is_bouncer(who) } fn is_destroying(&self) -> bool { @@ -1407,7 +1407,7 @@ pub mod pallet { /// can never change. RolesUpdated { root: Option, - state_toggler: Option, + bouncer: Option, nominator: Option, }, /// The active balance of pool `pool_id` has been slashed to `balance`. @@ -1630,7 +1630,7 @@ pub mod pallet { /// /// # Conditions for a permissionless dispatch. /// - /// * The pool is blocked and the caller is either the root or state-toggler. This is + /// * The pool is blocked and the caller is either the root or bouncer. This is /// refereed to as a kick. /// * The pool is destroying and the member is not the depositor. /// * The pool is destroying, the member is the depositor and no other members are in the @@ -1754,7 +1754,7 @@ pub mod pallet { /// /// * The pool is in destroy mode and the target is not the depositor. /// * The target is the depositor and they are the only member in the sub pools. - /// * The pool is blocked and the caller is either the root or state-toggler. + /// * The pool is blocked and the caller is either the root or bouncer. /// /// # Conditions for permissioned dispatch /// @@ -1879,7 +1879,7 @@ pub mod pallet { /// creating multiple pools in the same extrinsic. /// * `root` - The account to set as [`PoolRoles::root`]. /// * `nominator` - The account to set as the [`PoolRoles::nominator`]. - /// * `state_toggler` - The account to set as the [`PoolRoles::state_toggler`]. + /// * `bouncer` - The account to set as the [`PoolRoles::bouncer`]. /// /// # Note /// @@ -1892,7 +1892,7 @@ pub mod pallet { #[pallet::compact] amount: BalanceOf, root: AccountIdLookupOf, nominator: AccountIdLookupOf, - state_toggler: AccountIdLookupOf, + bouncer: AccountIdLookupOf, ) -> DispatchResult { let depositor = ensure_signed(origin)?; @@ -1901,7 +1901,7 @@ pub mod pallet { Ok(*id) })?; - Self::do_create(depositor, amount, root, nominator, state_toggler, pool_id) + Self::do_create(depositor, amount, root, nominator, bouncer, pool_id) } /// Create a new delegation pool with a previously used pool id @@ -1917,7 +1917,7 @@ pub mod pallet { #[pallet::compact] amount: BalanceOf, root: AccountIdLookupOf, nominator: AccountIdLookupOf, - state_toggler: AccountIdLookupOf, + bouncer: AccountIdLookupOf, pool_id: PoolId, ) -> DispatchResult { let depositor = ensure_signed(origin)?; @@ -1925,7 +1925,7 @@ pub mod pallet { ensure!(!BondedPools::::contains_key(pool_id), Error::::PoolIdInUse); ensure!(pool_id < LastPoolId::::get(), Error::::InvalidPoolId); - Self::do_create(depositor, amount, root, nominator, state_toggler, pool_id) + Self::do_create(depositor, amount, root, nominator, bouncer, pool_id) } /// Nominate on behalf of the pool. @@ -1955,7 +1955,7 @@ pub mod pallet { /// /// The dispatch origin of this call must be either: /// - /// 1. signed by the state toggler, or the root role of the pool, + /// 1. signed by the bouncer, or the root role of the pool, /// 2. if the pool conditions to be open are NOT met (as described by `ok_to_be_open`), and /// then the state of the pool can be permissionlessly changed to `Destroying`. #[pallet::call_index(9)] @@ -1985,7 +1985,7 @@ pub mod pallet { /// Set a new metadata for the pool. /// - /// The dispatch origin of this call must be signed by the state toggler, or the root role + /// The dispatch origin of this call must be signed by the bouncer, or the root role /// of the pool. #[pallet::call_index(10)] #[pallet::weight(T::WeightInfo::set_metadata(metadata.len() as u32))] @@ -2063,7 +2063,7 @@ pub mod pallet { pool_id: PoolId, new_root: ConfigOp, new_nominator: ConfigOp, - new_state_toggler: ConfigOp, + new_bouncer: ConfigOp, ) -> DispatchResult { let mut bonded_pool = match ensure_root(origin.clone()) { Ok(()) => BondedPool::::get(pool_id).ok_or(Error::::PoolNotFound)?, @@ -2086,16 +2086,16 @@ pub mod pallet { ConfigOp::Remove => bonded_pool.roles.nominator = None, ConfigOp::Set(v) => bonded_pool.roles.nominator = Some(v), }; - match new_state_toggler { + match new_bouncer { ConfigOp::Noop => (), - ConfigOp::Remove => bonded_pool.roles.state_toggler = None, - ConfigOp::Set(v) => bonded_pool.roles.state_toggler = Some(v), + ConfigOp::Remove => bonded_pool.roles.bouncer = None, + ConfigOp::Set(v) => bonded_pool.roles.bouncer = Some(v), }; Self::deposit_event(Event::::RolesUpdated { root: bonded_pool.roles.root.clone(), nominator: bonded_pool.roles.nominator.clone(), - state_toggler: bonded_pool.roles.state_toggler.clone(), + bouncer: bonded_pool.roles.bouncer.clone(), }); bonded_pool.put(); @@ -2354,12 +2354,12 @@ impl Pallet { amount: BalanceOf, root: AccountIdLookupOf, nominator: AccountIdLookupOf, - state_toggler: AccountIdLookupOf, + bouncer: AccountIdLookupOf, pool_id: PoolId, ) -> DispatchResult { let root = T::Lookup::lookup(root)?; let nominator = T::Lookup::lookup(nominator)?; - let state_toggler = T::Lookup::lookup(state_toggler)?; + let bouncer = T::Lookup::lookup(bouncer)?; ensure!(amount >= Pallet::::depositor_min_bond(), Error::::MinimumBondNotMet); ensure!( @@ -2372,7 +2372,7 @@ impl Pallet { PoolRoles { root: Some(root), nominator: Some(nominator), - state_toggler: Some(state_toggler), + bouncer: Some(bouncer), depositor: who.clone(), }, ); diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index 4a9b6ca0fe117..53087e763b99c 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -28,7 +28,7 @@ pub mod v1 { pub depositor: AccountId, pub root: AccountId, pub nominator: AccountId, - pub state_toggler: AccountId, + pub bouncer: AccountId, } impl OldPoolRoles { @@ -37,7 +37,7 @@ pub mod v1 { depositor: self.depositor, root: Some(self.root), nominator: Some(self.nominator), - state_toggler: Some(self.state_toggler), + bouncer: Some(self.bouncer), } } } diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 7d5d418bbf2c8..bf200ecc79a05 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -37,7 +37,7 @@ macro_rules! member_unbonding_eras { } pub const DEFAULT_ROLES: PoolRoles = - PoolRoles { depositor: 10, root: Some(900), nominator: Some(901), state_toggler: Some(902) }; + PoolRoles { depositor: 10, root: Some(900), nominator: Some(901), bouncer: Some(902) }; #[test] fn test_setup_works() { @@ -2108,7 +2108,7 @@ mod unbond { .add_members(vec![(20, 20)]) .build_and_execute(|| { unsafe_set_state(1, PoolState::Blocked); - let kicker = DEFAULT_ROLES.state_toggler.unwrap(); + let kicker = DEFAULT_ROLES.bouncer.unwrap(); // cannot be kicked to above the limit. assert_noop!( @@ -2211,7 +2211,7 @@ mod unbond { // set the stage unsafe_set_state(1, PoolState::Blocked); - let kicker = DEFAULT_ROLES.state_toggler.unwrap(); + let kicker = DEFAULT_ROLES.bouncer.unwrap(); // cannot be kicked to above limit. assert_noop!( @@ -2550,7 +2550,7 @@ mod unbond { #[test] fn unbond_kick_works() { - // Kick: the pool is blocked and the caller is either the root or state-toggler. + // Kick: the pool is blocked and the caller is either the root or bouncer. ExtBuilder::default() .add_members(vec![(100, 100), (200, 200)]) .build_and_execute(|| { @@ -2559,7 +2559,7 @@ mod unbond { let bonded_pool = BondedPool::::get(1).unwrap(); assert_eq!(bonded_pool.roles.root.unwrap(), 900); assert_eq!(bonded_pool.roles.nominator.unwrap(), 901); - assert_eq!(bonded_pool.roles.state_toggler.unwrap(), 902); + assert_eq!(bonded_pool.roles.bouncer.unwrap(), 902); // When the nominator tries to kick, then its a noop assert_noop!( @@ -2587,7 +2587,7 @@ mod unbond { ] ); - // When the state toggler kicks then its ok + // When the bouncer kicks then its ok assert_ok!(Pools::fully_unbond(RuntimeOrigin::signed(902), 200)); assert_eq!( @@ -3518,7 +3518,7 @@ mod withdraw_unbonded { // Can kick as root assert_ok!(Pools::withdraw_unbonded(RuntimeOrigin::signed(900), 100, 0)); - // Can kick as state toggler + // Can kick as bouncer assert_ok!(Pools::withdraw_unbonded(RuntimeOrigin::signed(900), 200, 0)); assert_eq!(Balances::free_balance(100), 100 + 100); @@ -4108,7 +4108,7 @@ mod create { depositor: 11, root: Some(123), nominator: Some(456), - state_toggler: Some(789) + bouncer: Some(789) } } } @@ -4192,7 +4192,7 @@ mod create { amount: 20, root: 11, nominator: 11, - state_toggler: 11, + bouncer: 11, }); assert_noop!( create.dispatch(RuntimeOrigin::signed(11)), @@ -4252,7 +4252,7 @@ mod nominate { Error::::NotNominator ); - // State toggler can't nominate + // bouncer can't nominate assert_noop!( Pools::nominate(RuntimeOrigin::signed(902), 1, vec![21]), Error::::NotNominator @@ -4284,7 +4284,7 @@ mod set_state { // Given assert_ok!(BondedPool::::get(1).unwrap().ok_to_be_open()); - // Only the root and state toggler can change the state when the pool is ok to be open. + // Only the root and bouncer can change the state when the pool is ok to be open. assert_noop!( Pools::set_state(RuntimeOrigin::signed(10), 1, PoolState::Blocked), Error::::CanNotChangeState @@ -4308,7 +4308,7 @@ mod set_state { assert_eq!(BondedPool::::get(1).unwrap().state, PoolState::Blocked); - // State toggler can change state + // bouncer can change state assert_ok!(Pools::set_state(RuntimeOrigin::signed(902), 1, PoolState::Destroying)); assert_eq!(BondedPool::::get(1).unwrap().state, PoolState::Destroying); @@ -4372,7 +4372,7 @@ mod set_metadata { assert_ok!(Pools::set_metadata(RuntimeOrigin::signed(900), 1, vec![1, 1])); assert_eq!(Metadata::::get(1), vec![1, 1]); - // State toggler can set metadata + // bouncer can set metadata assert_ok!(Pools::set_metadata(RuntimeOrigin::signed(902), 1, vec![2, 2])); assert_eq!(Metadata::::get(1), vec![2, 2]); @@ -4597,7 +4597,7 @@ mod update_roles { depositor: 10, root: Some(900), nominator: Some(901), - state_toggler: Some(902) + bouncer: Some(902) }, ); @@ -4636,7 +4636,7 @@ mod update_roles { ), Error::::DoesNotHavePermission, ); - // state-toggler + // bouncer assert_noop!( Pools::update_roles( RuntimeOrigin::signed(902), @@ -4664,7 +4664,7 @@ mod update_roles { Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, Event::RolesUpdated { root: Some(5), - state_toggler: Some(7), + bouncer: Some(7), nominator: Some(6) } ] @@ -4675,7 +4675,7 @@ mod update_roles { depositor: 10, root: Some(5), nominator: Some(6), - state_toggler: Some(7) + bouncer: Some(7) }, ); @@ -4692,7 +4692,7 @@ mod update_roles { pool_events_since_last_call(), vec![Event::RolesUpdated { root: Some(1), - state_toggler: Some(3), + bouncer: Some(3), nominator: Some(2) }] ); @@ -4702,7 +4702,7 @@ mod update_roles { depositor: 10, root: Some(1), nominator: Some(2), - state_toggler: Some(3) + bouncer: Some(3) }, ); @@ -4719,7 +4719,7 @@ mod update_roles { pool_events_since_last_call(), vec![Event::RolesUpdated { root: Some(11), - state_toggler: Some(3), + bouncer: Some(3), nominator: Some(2) }] ); @@ -4730,7 +4730,7 @@ mod update_roles { depositor: 10, root: Some(11), nominator: Some(2), - state_toggler: Some(3) + bouncer: Some(3) }, ); @@ -4745,12 +4745,12 @@ mod update_roles { assert_eq!( pool_events_since_last_call(), - vec![Event::RolesUpdated { root: Some(69), state_toggler: None, nominator: None }] + vec![Event::RolesUpdated { root: Some(69), bouncer: None, nominator: None }] ); assert_eq!( BondedPools::::get(1).unwrap().roles, - PoolRoles { depositor: 10, root: Some(69), nominator: None, state_toggler: None }, + PoolRoles { depositor: 10, root: Some(69), nominator: None, bouncer: None }, ); }) } From 6b0d957f1b1e28a0f5ea7465bb897f4397c323aa Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 20 Feb 2023 20:25:35 +0700 Subject: [PATCH 2/7] add migration --- frame/nomination-pools/src/migration.rs | 93 +++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index 53087e763b99c..f1054c6f66512 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -449,3 +449,96 @@ pub mod v3 { } } } + +pub mod v4 { + use super::*; + + #[derive(Decode)] + pub struct OldBondedPoolInner { + pub points: BalanceOf, + pub state: PoolState, + pub member_counter: u32, + pub roles: OldPoolRoles, + } + + #[derive(Decode)] + pub struct OldPoolRoles { + pub depositor: AccountId, + pub root: Option, + pub nominator: Option, + pub state_toggler: Option, + } + + impl OldBondedPoolInner { + fn migrate_to_v4(self) -> BondedPoolInner { + BondedPoolInner { + member_counter: self.member_counter, + points: self.points, + state: self.state, + roles: PoolRoles { + depositor: self.roles.depositor, + root: self.roles.root, + nominator: self.roles.nominator, + bouncer: self.roles.state_toggler, + }, + } + } + } + + /// This migration changes `PoolRoles` `state_toggler` property to `bouncer`. + pub struct MigrateToV4(sp_std::marker::PhantomData); + impl OnRuntimeUpgrade for MigrateToV4 { + fn on_runtime_upgrade() -> Weight { + let current = Pallet::::current_storage_version(); + let onchain = Pallet::::on_chain_storage_version(); + + log!( + info, + "Running migration with current storage version {:?} / onchain {:?}", + current, + onchain + ); + + if current == 4 && onchain == 3 { + let mut translated = 0u64; + BondedPools::::translate::, _>(|_key, old_value| { + translated.saturating_inc(); + Some(old_value.migrate_to_v4()) + }); + + current.put::>(); + log!(info, "Upgraded {} pools, storage to version {:?}", translated, current); + + // reads: translated + onchain version. + // writes: translated + current.put + T::DbWeight::get().reads_writes(translated + 1, translated + 1) + } else { + log!(info, "Migration did not execute. This probably should be removed"); + T::DbWeight::get().reads(1) + } + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, &'static str> { + ensure!( + Pallet::::current_storage_version() > Pallet::::on_chain_storage_version(), + "the on_chain version is equal or more than the current one" + ); + Ok(Vec::new()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(_: Vec) -> Result<(), &'static str> { + // ensure all BondedPools items now contain an `inner.commission: Commission` field. + ensure!( + BondedPools::::iter().all(|(_, inner)| inner.commission.current.is_none() && + inner.commission.max.is_none() && + inner.commission.change_rate.is_none() && + inner.commission.throttle_from.is_none()), + "a commission value has been incorrectly set" + ); + ensure!(Pallet::::on_chain_storage_version() == 4, "wrong storage version"); + Ok(()) + } + } +} From 3b60a886d330d2ac2f8c501acda507c9a44131a6 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 20 Feb 2023 20:25:41 +0700 Subject: [PATCH 3/7] fmt --- frame/nomination-pools/src/lib.rs | 13 ++++------ frame/nomination-pools/src/tests.rs | 39 +++++------------------------ 2 files changed, 11 insertions(+), 41 deletions(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 063bd57261da4..19595c7f14738 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -121,8 +121,8 @@ //! other members have left. Once they fully withdraw their funds, the pool is destroyed. //! * Nominator: can select which validators the pool nominates. //! * Bouncer: can change the pools state and kick members if the pool is blocked. -//! * Root: can change the nominator, bouncer, or itself and can perform any of the actions -//! the nominator or bouncer can. +//! * Root: can change the nominator, bouncer, or itself and can perform any of the actions the +//! nominator or bouncer can. //! //! ### Dismantling //! @@ -735,10 +735,7 @@ impl BondedPool { } fn is_bouncer(&self, who: &T::AccountId) -> bool { - self.roles - .bouncer - .as_ref() - .map_or(false, |bouncer| bouncer == who) + self.roles.bouncer.as_ref().map_or(false, |bouncer| bouncer == who) } fn can_update_roles(&self, who: &T::AccountId) -> bool { @@ -1630,8 +1627,8 @@ pub mod pallet { /// /// # Conditions for a permissionless dispatch. /// - /// * The pool is blocked and the caller is either the root or bouncer. This is - /// refereed to as a kick. + /// * The pool is blocked and the caller is either the root or bouncer. This is refereed to + /// as a kick. /// * The pool is destroying and the member is not the depositor. /// * The pool is destroying, the member is the depositor and no other members are in the /// pool. diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index bf200ecc79a05..68e6877badfb4 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -4662,21 +4662,12 @@ mod update_roles { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, - Event::RolesUpdated { - root: Some(5), - bouncer: Some(7), - nominator: Some(6) - } + Event::RolesUpdated { root: Some(5), bouncer: Some(7), nominator: Some(6) } ] ); assert_eq!( BondedPools::::get(1).unwrap().roles, - PoolRoles { - depositor: 10, - root: Some(5), - nominator: Some(6), - bouncer: Some(7) - }, + PoolRoles { depositor: 10, root: Some(5), nominator: Some(6), bouncer: Some(7) }, ); // also root origin can @@ -4690,20 +4681,11 @@ mod update_roles { assert_eq!( pool_events_since_last_call(), - vec![Event::RolesUpdated { - root: Some(1), - bouncer: Some(3), - nominator: Some(2) - }] + vec![Event::RolesUpdated { root: Some(1), bouncer: Some(3), nominator: Some(2) }] ); assert_eq!( BondedPools::::get(1).unwrap().roles, - PoolRoles { - depositor: 10, - root: Some(1), - nominator: Some(2), - bouncer: Some(3) - }, + PoolRoles { depositor: 10, root: Some(1), nominator: Some(2), bouncer: Some(3) }, ); // Noop works @@ -4717,21 +4699,12 @@ mod update_roles { assert_eq!( pool_events_since_last_call(), - vec![Event::RolesUpdated { - root: Some(11), - bouncer: Some(3), - nominator: Some(2) - }] + vec![Event::RolesUpdated { root: Some(11), bouncer: Some(3), nominator: Some(2) }] ); assert_eq!( BondedPools::::get(1).unwrap().roles, - PoolRoles { - depositor: 10, - root: Some(11), - nominator: Some(2), - bouncer: Some(3) - }, + PoolRoles { depositor: 10, root: Some(11), nominator: Some(2), bouncer: Some(3) }, ); // Remove works From 6f18092f5ffe9a39a9d4cd890adff9686b8bad66 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 20 Feb 2023 20:29:29 +0700 Subject: [PATCH 4/7] bump storage version --- frame/nomination-pools/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 19595c7f14738..b1677d3d79ca3 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -1170,7 +1170,7 @@ pub mod pallet { use frame_system::{ensure_signed, pallet_prelude::*}; /// The current storage version. - const STORAGE_VERSION: StorageVersion = StorageVersion::new(3); + const STORAGE_VERSION: StorageVersion = StorageVersion::new(4); #[pallet::pallet] #[pallet::generate_store(pub(crate) trait Store)] From 135cd45d71d005e0cd21747cc47ceae3b10124a8 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 20 Feb 2023 21:33:31 +0700 Subject: [PATCH 5/7] rm migration --- frame/nomination-pools/src/migration.rs | 95 +------------------------ 1 file changed, 1 insertion(+), 94 deletions(-) diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index f1054c6f66512..37d64f2bf5e92 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -448,97 +448,4 @@ pub mod v3 { Ok(()) } } -} - -pub mod v4 { - use super::*; - - #[derive(Decode)] - pub struct OldBondedPoolInner { - pub points: BalanceOf, - pub state: PoolState, - pub member_counter: u32, - pub roles: OldPoolRoles, - } - - #[derive(Decode)] - pub struct OldPoolRoles { - pub depositor: AccountId, - pub root: Option, - pub nominator: Option, - pub state_toggler: Option, - } - - impl OldBondedPoolInner { - fn migrate_to_v4(self) -> BondedPoolInner { - BondedPoolInner { - member_counter: self.member_counter, - points: self.points, - state: self.state, - roles: PoolRoles { - depositor: self.roles.depositor, - root: self.roles.root, - nominator: self.roles.nominator, - bouncer: self.roles.state_toggler, - }, - } - } - } - - /// This migration changes `PoolRoles` `state_toggler` property to `bouncer`. - pub struct MigrateToV4(sp_std::marker::PhantomData); - impl OnRuntimeUpgrade for MigrateToV4 { - fn on_runtime_upgrade() -> Weight { - let current = Pallet::::current_storage_version(); - let onchain = Pallet::::on_chain_storage_version(); - - log!( - info, - "Running migration with current storage version {:?} / onchain {:?}", - current, - onchain - ); - - if current == 4 && onchain == 3 { - let mut translated = 0u64; - BondedPools::::translate::, _>(|_key, old_value| { - translated.saturating_inc(); - Some(old_value.migrate_to_v4()) - }); - - current.put::>(); - log!(info, "Upgraded {} pools, storage to version {:?}", translated, current); - - // reads: translated + onchain version. - // writes: translated + current.put - T::DbWeight::get().reads_writes(translated + 1, translated + 1) - } else { - log!(info, "Migration did not execute. This probably should be removed"); - T::DbWeight::get().reads(1) - } - } - - #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { - ensure!( - Pallet::::current_storage_version() > Pallet::::on_chain_storage_version(), - "the on_chain version is equal or more than the current one" - ); - Ok(Vec::new()) - } - - #[cfg(feature = "try-runtime")] - fn post_upgrade(_: Vec) -> Result<(), &'static str> { - // ensure all BondedPools items now contain an `inner.commission: Commission` field. - ensure!( - BondedPools::::iter().all(|(_, inner)| inner.commission.current.is_none() && - inner.commission.max.is_none() && - inner.commission.change_rate.is_none() && - inner.commission.throttle_from.is_none()), - "a commission value has been incorrectly set" - ); - ensure!(Pallet::::on_chain_storage_version() == 4, "wrong storage version"); - Ok(()) - } - } -} +} \ No newline at end of file From a69e3b1da574d8faed4348f0e81f3898829cf80e Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 20 Feb 2023 21:43:51 +0700 Subject: [PATCH 6/7] revert version --- frame/nomination-pools/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index b1677d3d79ca3..19595c7f14738 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -1170,7 +1170,7 @@ pub mod pallet { use frame_system::{ensure_signed, pallet_prelude::*}; /// The current storage version. - const STORAGE_VERSION: StorageVersion = StorageVersion::new(4); + const STORAGE_VERSION: StorageVersion = StorageVersion::new(3); #[pallet::pallet] #[pallet::generate_store(pub(crate) trait Store)] From adb8c21439debd2533583a64a42561eb01962050 Mon Sep 17 00:00:00 2001 From: command-bot <> Date: Mon, 20 Feb 2023 16:49:50 +0000 Subject: [PATCH 7/7] ".git/.scripts/commands/fmt/fmt.sh" --- frame/nomination-pools/src/migration.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index 37d64f2bf5e92..53087e763b99c 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -448,4 +448,4 @@ pub mod v3 { Ok(()) } } -} \ No newline at end of file +}