From e3677d763f8e9cb0fad40b848614ade924d21017 Mon Sep 17 00:00:00 2001 From: Bernardo Cardoso Date: Thu, 16 Dec 2021 23:36:50 +0000 Subject: [PATCH] Add set_cmix_id extrinsic to Staking pallet --- frame/staking/src/pallet/mod.rs | 41 ++++++++++++++++++ frame/staking/src/weights.rs | 11 +++++ frame/staking/src/xx_tests.rs | 73 +++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+) diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 569429cb13782..2a59af4d2fffd 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -672,6 +672,8 @@ pub mod pallet { ValidatorMustHaveCmixId, /// Validator commission is too low ValidatorCommissionTooLow, + /// Stash account already has a CMIX ID + StashAlreadyHasCmixId, } #[pallet::hooks] @@ -1608,6 +1610,45 @@ pub mod pallet { Self::chill_stash(&stash); Ok(()) } + + /// Set the CMIX ID of a stash, if it doesn't have one already. + /// + /// The dispatch origin for this call must be _Signed_ by the stash, not the controller. + /// + /// # + /// - Independent of the arguments. Insignificant complexity. + /// ---------- + /// Weight: O(1) + /// DB Weight: + /// - Read: Bonded, Ledger + /// - Write: Ledger + /// # + #[pallet::weight(T::WeightInfo::set_cmix_id())] + pub fn set_cmix_id( + origin: OriginFor, + cmix_id: T::Hash, + ) -> DispatchResult { + let stash = ensure_signed(origin)?; + let controller = Self::bonded(&stash).ok_or(Error::::NotStash)?; + + let mut ledger = >::get(&controller).ok_or(Error::::NotController)?; + + // Ensure cmix id is not set + if ledger.cmix_id.is_some() { + Err(Error::::StashAlreadyHasCmixId)? + } + + // Ensure cmix id is unique + if >::contains_key(&cmix_id) { + Err(Error::::ValidatorCmixIdNotUnique)? + } + + // Set cmix id and write ledger + >::insert(&cmix_id, ()); + ledger.cmix_id = Some(cmix_id); + >::insert(&controller, &ledger); + Ok(()) + } } } diff --git a/frame/staking/src/weights.rs b/frame/staking/src/weights.rs index 32c8dc80da158..fbab73e5555b9 100644 --- a/frame/staking/src/weights.rs +++ b/frame/staking/src/weights.rs @@ -73,6 +73,7 @@ pub trait WeightInfo { fn get_npos_targets(v: u32, ) -> Weight; fn set_staking_limits() -> Weight; fn chill_other() -> Weight; + fn set_cmix_id() -> Weight; } /// Weights for pallet_staking using the Substrate node and recommended hardware. @@ -442,6 +443,11 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(11 as Weight)) .saturating_add(T::DbWeight::get().writes(6 as Weight)) } + fn set_cmix_id() -> Weight { + (83_389_000 as Weight) + .saturating_add(T::DbWeight::get().reads(2 as Weight)) + .saturating_add(T::DbWeight::get().writes(1 as Weight)) + } } // For backwards compatibility and tests @@ -810,4 +816,9 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(11 as Weight)) .saturating_add(RocksDbWeight::get().writes(6 as Weight)) } + fn set_cmix_id() -> Weight { + (83_389_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(2 as Weight)) + .saturating_add(RocksDbWeight::get().writes(1 as Weight)) + } } diff --git a/frame/staking/src/xx_tests.rs b/frame/staking/src/xx_tests.rs index de3bfa61c62b1..744b25c8e78e8 100644 --- a/frame/staking/src/xx_tests.rs +++ b/frame/staking/src/xx_tests.rs @@ -99,6 +99,79 @@ fn after_full_unbond_cmix_id_is_removed() { }) } +#[test] +fn calling_set_cmix_id_correctly_stores_cmix_id() { + let stash_value = 100; + ExtBuilder::default() + .has_stakers(false) + .build_and_execute(|| { + let _ = Balances::make_free_balance_be(&10, stash_value); + + // Can't call function if not bonded + assert_noop!( + Staking::set_cmix_id(Origin::signed(10), cmix_id(10u8).unwrap()), + Error::::NotStash, + ); + + // Bond account without cmix id + assert_ok!(Staking::bond(Origin::signed(10), 11, stash_value, None)); + + // Can't call function from controller + assert_noop!( + Staking::set_cmix_id(Origin::signed(11), cmix_id(10u8).unwrap()), + Error::::NotStash, + ); + + // Set cmix id + assert_ok!(Staking::set_cmix_id(Origin::signed(10), cmix_id(10u8).unwrap())); + + // confirm cmix ID is correctly stored + assert!(CmixIds::::contains_key(&cmix_id(10u8).unwrap())); + }) +} + +#[test] +fn calling_set_cmix_id_stash_already_has_cmix_id_fails() { + let stash_value = 100; + ExtBuilder::default() + .has_stakers(false) + .build_and_execute(|| { + let _ = Balances::make_free_balance_be(&10, stash_value); + + // bond first validator with cmix id + assert_ok!(Staking::bond(Origin::signed(10), 11, stash_value, cmix_id(10u8))); + + // can't set cmix id, since it's already present + assert_noop!( + Staking::set_cmix_id(Origin::signed(10), cmix_id(11u8).unwrap()), + Error::::StashAlreadyHasCmixId, + ); + }) +} + +#[test] +fn calling_set_cmix_id_with_existing_cmix_id_fails() { + let stash_value = 100; + ExtBuilder::default() + .has_stakers(false) + .build_and_execute(|| { + let _ = Balances::make_free_balance_be(&10, stash_value); + let _ = Balances::make_free_balance_be(&20, stash_value); + + // bond first validator with cmix id + assert_ok!(Staking::bond(Origin::signed(10), 11, stash_value, cmix_id(10u8))); + + // bond second validator without + assert_ok!(Staking::bond(Origin::signed(20), 21, stash_value, None)); + + // if second vaidator tries to set existing cmix id, fails with ValidatorCmixIdNotUnique + assert_noop!( + Staking::set_cmix_id(Origin::signed(20), cmix_id(10u8).unwrap()), + Error::::ValidatorCmixIdNotUnique, + ); + }) +} + //////////////////////////////////////// // Rewards Destination // ////////////////////////////////////////