No unbonding when signer or next signer (#1031)

* No unbonding when signer or next signer * test for chill and unbond * fix bench * add unbond and chill benches * update benchmarks * Apply suggestions from code review Co-authored-by: peg <peg@magmacollective.org> * clean * lint * changelog * clean * add in nominating check * Update pallets/staking/src/benchmarking.rs Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com> * add tests for nominators * clean function * fmt * fmt * fmt * Apply suggestions from code review Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com> * clean * add nominator to benchmarks * fmt --------- Co-authored-by: peg <peg@magmacollective.org> Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>
entropyxyz · Sep 27, 2024 · e1eaf12 · e1eaf12
1 parent 9facf6e
commit e1eaf12
Show file tree

Hide file tree

Showing 9 changed files with 686 additions and 70 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,6 +18,10 @@ At the moment this project **does not** adhere to
   cleaned up. A lot of storage entries, events, and extrinsics were removed from the `Registry`
   pallet. The genesis build config was also removed. Additionally, the `new/user/` HTTP endpoint in
   the TSS was removed since it was no longer necessary.
+- In [#1031](https://github.com/entropyxyz/entropy-core/pull/1031), more Staking calls were blocked
+  to go through the `staking_extention` pallet. This makes sure no funds can be unbonded from a
+  validator if they are currently in the signing comittee. This was applied to `unbond`, `chill`,
+  and `withdraw_unbonded`
 - In [#1045](https://github.com/entropyxyz/entropy-core/pull/1045), `ProgramsInfo` now takes `version_number` to maintain backwards compatibility if programs runtime is updated
 - In [#1050](https://github.com/entropyxyz/entropy-core/pull/1050), the flow for signing has changed.
   A user now sends their request to any validator that is not a signer. This will act as a relayer.
@@ -45,6 +49,7 @@ At the moment this project **does not** adhere to
 
 ### Changed
 - Fix TSS `AccountId` keys in chainspec ([#993](https://github.com/entropyxyz/entropy-core/pull/993))
+- No unbonding when signer or next signer ([#1031](https://github.com/entropyxyz/entropy-core/pull/1031))
 - Add relay tx endpoint ([#1050](https://github.com/entropyxyz/entropy-core/pull/1050))
 
 ### Removed

diff --git a/crates/client/entropy_metadata.scale b/crates/client/entropy_metadata.scale
diff --git a/pallets/staking/src/benchmarking.rs b/pallets/staking/src/benchmarking.rs
@@ -15,23 +15,25 @@
 
 //! Benchmarking setup for pallet-propgation
 #![allow(unused_imports)]
+use super::*;
+#[allow(unused_imports)]
+use crate::Pallet as Staking;
 use entropy_shared::MAX_SIGNERS;
 use frame_benchmarking::{account, benchmarks, impl_benchmark_test_suite, whitelisted_caller};
 use frame_support::{
     assert_ok, ensure,
     sp_runtime::traits::StaticLookup,
-    traits::{Currency, Get},
+    traits::{Currency, Defensive, Get},
     BoundedVec,
 };
 use frame_system::{EventRecord, RawOrigin};
 use pallet_parameters::{SignersInfo, SignersSize};
-use pallet_staking::{Pallet as FrameStaking, RewardDestination, ValidatorPrefs};
+use pallet_staking::{
+    Event as FrameStakingEvent, MaxNominationsOf, Nominations, Pallet as FrameStaking,
+    RewardDestination, ValidatorPrefs,
+};
 use sp_std::{vec, vec::Vec};
 
-use super::*;
-#[allow(unused_imports)]
-use crate::Pallet as Staking;
-
 const NULL_ARR: [u8; 32] = [0; 32];
 const SEED: u32 = 0;
 
@@ -43,6 +45,16 @@ fn assert_last_event<T: Config>(generic_event: <T as Config>::RuntimeEvent) {
     assert_eq!(event, &system_event);
 }
 
+fn assert_last_event_frame_staking<T: Config>(
+    generic_event: <T as pallet_staking::Config>::RuntimeEvent,
+) {
+    let events = frame_system::Pallet::<T>::events();
+    let system_event: <T as frame_system::Config>::RuntimeEvent = generic_event.into();
+    // compare to the last event record
+    let EventRecord { event, .. } = &events[events.len() - 1];
+    assert_eq!(event, &system_event);
+}
+
 pub fn create_validators<T: Config>(
     count: u32,
     seed: u32,
@@ -130,13 +142,88 @@ benchmarks! {
     assert_last_event::<T>(Event::<T>::ThresholdAccountChanged(bonder, server_info).into());
   }
 
+  unbond {
+    let s in 0 .. MAX_SIGNERS as u32;
+    let n in 0 .. MaxNominationsOf::<T>::get();
+
+    let caller: T::AccountId = whitelisted_caller();
+    let validator_id_res = <T as pallet_session::Config>::ValidatorId::try_from(caller.clone()).or(Err(Error::<T>::InvalidValidatorId)).unwrap();
+    let bonder: T::AccountId = account("bond", 0, SEED);
+    let threshold: T::AccountId = account("threshold", 0, SEED);
+
+    let signers = vec![validator_id_res.clone(); s as usize];
+    Signers::<T>::put(signers.clone());
+    NextSigners::<T>::put(NextSignerInfo {
+      next_signers: signers,
+      confirmations: vec![],
+    });
+
+    prep_bond_and_validate::<T>(true, caller.clone(), bonder.clone(), threshold.clone(), NULL_ARR);
+
+    let targets = BoundedVec::try_from(vec![threshold.clone(); n as usize]).unwrap();
+    let nominations = Nominations { targets, submitted_in: 0, suppressed: false };
+    pallet_staking::Nominators::<T>::insert(bonder.clone(), nominations);
+  }:  _(RawOrigin::Signed(bonder.clone()), 10u32.into())
+  verify {
+    assert_last_event_frame_staking::<T>(FrameStakingEvent::Unbonded{ stash: bonder, amount: 10u32.into() }.into() );
+
+  }
+
+  chill {
+    let c in 0 .. MAX_SIGNERS as u32;
+    let n in 0 .. MaxNominationsOf::<T>::get();
+
+    let caller: T::AccountId = whitelisted_caller();
+    let validator_id_res = <T as pallet_session::Config>::ValidatorId::try_from(caller.clone()).or(Err(Error::<T>::InvalidValidatorId)).unwrap();
+    let bonder: T::AccountId = account("bond", 0, SEED);
+    let threshold: T::AccountId = account("threshold", 0, SEED);
+
+    let signers = vec![validator_id_res.clone(); c as usize];
+    Signers::<T>::put(signers.clone());
+    NextSigners::<T>::put(NextSignerInfo {
+      next_signers: signers,
+      confirmations: vec![],
+    });
+
+    prep_bond_and_validate::<T>(true, caller.clone(), bonder.clone(), threshold.clone(), NULL_ARR);
+    let bond = <T as pallet_staking::Config>::Currency::minimum_balance() * 10u32.into();
+
+    // assume fully unbonded as slightly more weight, but not enough to handle partial unbond
+    assert_ok!(<FrameStaking<T>>::unbond(
+      RawOrigin::Signed(bonder.clone()).into(),
+      bond,
+    ));
+
+    let targets = BoundedVec::try_from(vec![threshold.clone(); n as usize]).unwrap();
+    let nominations = Nominations { targets, submitted_in: 0, suppressed: false };
+    pallet_staking::Nominators::<T>::insert(bonder.clone(), nominations);
+
+    let _ = pallet_staking::Validators::<T>::clear(100, None);
+
+  }:  _(RawOrigin::Signed(bonder.clone()))
+  verify {
+    assert_last_event_frame_staking::<T>(FrameStakingEvent::Chilled{ stash: bonder }.into() );
+
+  }
+
 
   withdraw_unbonded {
+    let c in 0 .. MAX_SIGNERS as u32;
+    let n in 0 .. MaxNominationsOf::<T>::get();
+
     let caller: T::AccountId = whitelisted_caller();
     let bonder: T::AccountId = account("bond", 0, SEED);
     let threshold: T::AccountId = account("threshold", 0, SEED);
+    let validator_id_res = <T as pallet_session::Config>::ValidatorId::try_from(caller.clone()).or(Err(Error::<T>::InvalidValidatorId)).unwrap();
 
-    prep_bond_and_validate::<T>(true, caller.clone(), bonder.clone(), threshold, NULL_ARR);
+    let signers = vec![validator_id_res.clone(); c as usize];
+    Signers::<T>::put(signers.clone());
+    NextSigners::<T>::put(NextSignerInfo {
+      next_signers: signers,
+      confirmations: vec![],
+    });
+
+    prep_bond_and_validate::<T>(true, caller.clone(), bonder.clone(), threshold.clone(), NULL_ARR);
     let bond = <T as pallet_staking::Config>::Currency::minimum_balance() * 10u32.into();
 
     // assume fully unbonded as slightly more weight, but not enough to handle partial unbond
@@ -145,6 +232,9 @@ benchmarks! {
       bond,
     ));
 
+    let targets = BoundedVec::try_from(vec![threshold.clone(); n as usize]).unwrap();
+    let nominations = Nominations { targets, submitted_in: 0, suppressed: false };
+    pallet_staking::Nominators::<T>::insert(bonder.clone(), nominations);
 
   }:  _(RawOrigin::Signed(bonder.clone()), 0u32)
   verify {

diff --git a/pallets/staking/src/lib.rs b/pallets/staking/src/lib.rs
@@ -35,7 +35,7 @@
 use core::convert::TryInto;
 
 pub use pallet::*;
-use pallet_staking::ValidatorPrefs;
+use pallet_staking::{MaxNominationsOf, ValidatorPrefs};
 #[cfg(feature = "std")]
 use serde::{Deserialize, Serialize};
 
@@ -307,6 +307,10 @@ pub mod pallet {
         NotNextSigner,
         ReshareNotInProgress,
         AlreadyConfirmed,
+        NoUnbondingWhenSigner,
+        NoUnbondingWhenNextSigner,
+        NoUnnominatingWhenSigner,
+        NoUnnominatingWhenNextSigner,
     }
 
     #[pallet::event]
@@ -400,9 +404,46 @@ pub mod pallet {
             Ok(())
         }
 
-        /// Wraps's substrate withdraw unbonded but clears extra state if fully unbonded
+        /// Wraps's Substrate's `unbond` extrinsic but checks to make sure targeted account is not a signer or next signer
         #[pallet::call_index(2)]
-        #[pallet::weight(<T as Config>::WeightInfo::withdraw_unbonded())]
+        #[pallet::weight(<T as Config>::WeightInfo::unbond(MAX_SIGNERS as u32, MaxNominationsOf::<T>::get()))]
+        pub fn unbond(
+            origin: OriginFor<T>,
+            #[pallet::compact] value: BalanceOf<T>,
+        ) -> DispatchResultWithPostInfo {
+            let controller = ensure_signed(origin.clone())?;
+            let ledger =
+                pallet_staking::Pallet::<T>::ledger(StakingAccount::Controller(controller.clone()))
+                    .map_err(|_| Error::<T>::NoThresholdKey)?;
+
+            let (signers_length, nominators_length) =
+                Self::ensure_not_signer_or_next_signer_or_nominating(&ledger.stash)?;
+
+            pallet_staking::Pallet::<T>::unbond(origin, value)?;
+
+            Ok(Some(<T as Config>::WeightInfo::unbond(signers_length, nominators_length)).into())
+        }
+
+        /// Wraps's Substrate's `chill` extrinsic but checks to make sure the targeted account is not a signer or next signer
+        #[pallet::call_index(3)]
+        #[pallet::weight(<T as Config>::WeightInfo::chill(MAX_SIGNERS as u32, MaxNominationsOf::<T>::get()))]
+        pub fn chill(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
+            let controller = ensure_signed(origin.clone())?;
+            let ledger =
+                pallet_staking::Pallet::<T>::ledger(StakingAccount::Controller(controller.clone()))
+                    .map_err(|_| Error::<T>::NoThresholdKey)?;
+
+            let (signers_length, nominators_length) =
+                Self::ensure_not_signer_or_next_signer_or_nominating(&ledger.stash)?;
+
+            pallet_staking::Pallet::<T>::chill(origin)?;
+
+            Ok(Some(<T as Config>::WeightInfo::chill(signers_length, nominators_length)).into())
+        }
+
+        /// Wraps's Substrate's `withdraw_unbonded` extrinsic but clears extra state if fully unbonded
+        #[pallet::call_index(4)]
+        #[pallet::weight(<T as Config>::WeightInfo::withdraw_unbonded(MAX_SIGNERS as u32, MaxNominationsOf::<T>::get()))]
         pub fn withdraw_unbonded(
             origin: OriginFor<T>,
             num_slashing_spans: u32,
@@ -412,8 +453,12 @@ pub mod pallet {
                 pallet_staking::Pallet::<T>::ledger(StakingAccount::Controller(controller.clone()))
                     .map_err(|_| Error::<T>::NoThresholdKey)?;
 
-            let validator_id = <T as pallet_session::Config>::ValidatorId::try_from(ledger.stash)
-                .or(Err(Error::<T>::InvalidValidatorId))?;
+            let validator_id =
+                <T as pallet_session::Config>::ValidatorId::try_from(ledger.stash.clone())
+                    .or(Err(Error::<T>::InvalidValidatorId))?;
+
+            let (signers_length, nominators_length) =
+                Self::ensure_not_signer_or_next_signer_or_nominating(&ledger.stash)?;
 
             pallet_staking::Pallet::<T>::withdraw_unbonded(origin, num_slashing_spans)?;
             // TODO: do not allow unbonding of validator if not enough validators https://github.com/entropyxyz/entropy-core/issues/942
@@ -424,15 +469,19 @@ pub mod pallet {
                 IsValidatorSynced::<T>::remove(&validator_id);
                 Self::deposit_event(Event::NodeInfoRemoved(controller));
             }
-            Ok(().into())
+            Ok(Some(<T as Config>::WeightInfo::withdraw_unbonded(
+                signers_length,
+                nominators_length,
+            ))
+            .into())
         }
 
         /// Wrap's Substrate's `staking_pallet::validate()` extrinsic, but enforces that
         /// information about a validator's threshold server is provided.
         ///
         /// Note that - just like the original `validate()` extrinsic - the effects of this are
         /// only applied in the following era.
-        #[pallet::call_index(3)]
+        #[pallet::call_index(5)]
         #[pallet::weight(<T as Config>::WeightInfo::validate())]
         pub fn validate(
             origin: OriginFor<T>,
@@ -471,7 +520,7 @@ pub mod pallet {
 
         /// Let a validator declare if their kvdb is synced or not synced
         /// `synced`: State of validator's kvdb
-        #[pallet::call_index(4)]
+        #[pallet::call_index(6)]
         #[pallet::weight(<T as Config>::WeightInfo::declare_synced())]
         pub fn declare_synced(origin: OriginFor<T>, synced: bool) -> DispatchResult {
             let who = ensure_signed(origin.clone())?;
@@ -481,7 +530,7 @@ pub mod pallet {
             Ok(())
         }
 
-        #[pallet::call_index(5)]
+        #[pallet::call_index(7)]
         #[pallet::weight(({
             <T as Config>::WeightInfo::confirm_key_reshare_confirmed(MAX_SIGNERS as u32)
             .max(<T as Config>::WeightInfo::confirm_key_reshare_completed())
@@ -531,6 +580,47 @@ pub mod pallet {
             Ok(ledger.stash)
         }
 
+        /// Ensures that the current validator is not a signer or a next signer
+        pub fn ensure_not_signer_or_next_signer_or_nominating(
+            stash: &T::AccountId,
+        ) -> Result<(u32, u32), DispatchError> {
+            let nominations = pallet_staking::Nominators::<T>::get(stash)
+                .map_or_else(Vec::new, |x| x.targets.into_inner());
+
+            let signers = Self::signers();
+
+            // Check if the validator_id or any nominated validator is in signers
+            let in_signers = |id: &T::AccountId| {
+                let validator_id = <T as pallet_session::Config>::ValidatorId::try_from(id.clone());
+                match validator_id {
+                    Ok(v_id) => signers.contains(&v_id),
+                    Err(_) => false,
+                }
+            };
+
+            ensure!(!in_signers(stash), Error::<T>::NoUnbondingWhenSigner);
+            ensure!(!nominations.iter().any(in_signers), Error::<T>::NoUnnominatingWhenSigner);
+
+            if let Some(next_signers) = Self::next_signers() {
+                let next_signers_contains = |id: &T::AccountId| {
+                    let validator_id =
+                        <T as pallet_session::Config>::ValidatorId::try_from(id.clone());
+                    match validator_id {
+                        Ok(v_id) => next_signers.next_signers.contains(&v_id),
+                        Err(_) => false,
+                    }
+                };
+
+                ensure!(!next_signers_contains(stash), Error::<T>::NoUnbondingWhenNextSigner);
+                ensure!(
+                    !nominations.iter().any(next_signers_contains),
+                    Error::<T>::NoUnnominatingWhenNextSigner
+                );
+            }
+
+            Ok((signers.len() as u32, nominations.len() as u32))
+        }
+
         pub fn get_randomness() -> ChaCha20Rng {
             let phrase = b"signer_rotation";
             // TODO: Is randomness freshness an issue here

diff --git a/pallets/staking/src/mock.rs b/pallets/staking/src/mock.rs
@@ -397,7 +397,7 @@ impl pallet_parameters::Config for Test {
 pub fn new_test_ext() -> sp_io::TestExternalities {
     let mut t = system::GenesisConfig::<Test>::default().build_storage().unwrap();
     let pallet_balances = pallet_balances::GenesisConfig::<Test> {
-        balances: vec![(1, 100), (2, 100), (3, 100), (4, 100)],
+        balances: vec![(1, 100), (2, 100), (3, 100), (4, 100), (7, 100), (8, 100), (9, 200)],
     };
     let pallet_staking_extension = pallet_staking_extension::GenesisConfig::<Test> {
         // (ValidatorID, (AccountId, X25519PublicKey, TssServerURL, VerifyingKey))