No unbonding when signer or next signer

CHANGELOG.md

@@ -14,6 +14,7 @@ At the moment this project **does not** adhere to
   fields, `pallet_staking_extension::initial_signers`, `pallet_parameters::total_signers`, and
   `pallet_parameters::threshold`, which are used to set up the initial threshold signing
   configuration for the network.
+- in [#1031](https://github.com/entropyxyz/entropy-core/pull/1031), more Staking calls were blocked to go through staking_extention. 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`
 
 ### Added
 - Jumpstart network ([#918](https://github.com/entropyxyz/entropy-core/pull/918))
@@ -32,6 +33,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))
 
 ### Removed
 -  Remove `prune_registration` extrinsic ([#1022](https://github.com/entropyxyz/entropy-core/pull/1022))

pallets/staking/src/benchmarking.rs

@@ -24,7 +24,9 @@ use frame_support::{
 };
 use frame_system::{EventRecord, RawOrigin};
 use pallet_parameters::{SignersInfo, SignersSize};
-use pallet_staking::{Pallet as FrameStaking, RewardDestination, ValidatorPrefs};
+use pallet_staking::{
+    Event as FrameStakingEvent, Pallet as FrameStaking, RewardDestination, ValidatorPrefs,
+};
 use sp_std::{vec, vec::Vec};
 
 use super::*;
@@ -42,6 +44,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,
@@ -124,11 +136,76 @@ benchmarks! {
     assert_last_event::<T>(Event::<T>::ThresholdAccountChanged(bonder, server_info).into());
   }
 
+  unbond {
+    let c in 0 .. MAX_SIGNERS as u32;
+
+    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, NULL_ARR);
+
+
+  }:  _(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 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, 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,
+    ));
+
+
+  }:  _(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 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();
+
+    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, NULL_ARR);
     let bond = <T as pallet_staking::Config>::Currency::minimum_balance() * 10u32.into();

pallets/staking/src/lib.rs

@@ -304,6 +304,8 @@ pub mod pallet {
         NotNextSigner,
         ReshareNotInProgress,
         AlreadyConfirmed,
+        NoUnbondingWhenSigner,
+        NoUnbondingWhenNextSigner,
     }
 
     #[pallet::event]
@@ -397,9 +399,74 @@ pub mod pallet {
             Ok(())
         }
 
-        /// Wraps's substrate withdraw unbonded but clears extra state if fully unbonded
+        /// Wraps's substrate unbond but checks to make sure targeted validator 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))]
+        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 validator_id = <T as pallet_session::Config>::ValidatorId::try_from(ledger.stash)
+                .or(Err(Error::<T>::InvalidValidatorId))?;
+
+            let signers = Self::signers();
+            ensure!(!signers.contains(&validator_id), Error::<T>::NoUnbondingWhenSigner);
+
+            let next_signers = Self::next_signers();
+            if next_signers.is_some() {
+                ensure!(
+                    !next_signers
+                        .ok_or(Error::<T>::ReshareNotInProgress)?
+                        .next_signers
+                        .contains(&validator_id),
+                    Error::<T>::NoUnbondingWhenNextSigner
+                );
+            }
+
+            pallet_staking::Pallet::<T>::unbond(origin, value)?;
+
+            Ok(Some(<T as Config>::WeightInfo::unbond(signers.len() as u32)).into())
+        }
+
+        /// Wraps's substrate chill but checks to make targeted validator sure not signer or next signer
+        #[pallet::call_index(3)]
+        #[pallet::weight(<T as Config>::WeightInfo::chill(MAX_SIGNERS as u32))]
+        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 validator_id = <T as pallet_session::Config>::ValidatorId::try_from(ledger.stash)
+                .or(Err(Error::<T>::InvalidValidatorId))?;
+
+            let signers = Self::signers();
+            ensure!(!signers.contains(&validator_id), Error::<T>::NoUnbondingWhenSigner);
+
+            let next_signers = Self::next_signers();
+            if next_signers.is_some() {
+                ensure!(
+                    !next_signers
+                        .ok_or(Error::<T>::ReshareNotInProgress)?
+                        .next_signers
+                        .contains(&validator_id),
+                    Error::<T>::NoUnbondingWhenNextSigner
+                );
+            }
+
+            pallet_staking::Pallet::<T>::chill(origin)?;
+
+            Ok(Some(<T as Config>::WeightInfo::chill(signers.len() as u32)).into())
+        }
+
+        /// Wraps's substrate withdraw unbonded but clears extra state if fully unbonded
+        #[pallet::call_index(4)]
+        #[pallet::weight(<T as Config>::WeightInfo::withdraw_unbonded(MAX_SIGNERS as u32))]
         pub fn withdraw_unbonded(
             origin: OriginFor<T>,
             num_slashing_spans: u32,
@@ -412,6 +479,19 @@ pub mod pallet {
             let validator_id = <T as pallet_session::Config>::ValidatorId::try_from(ledger.stash)
                 .or(Err(Error::<T>::InvalidValidatorId))?;
 
+            let signers = Self::signers();
+            ensure!(!signers.contains(&validator_id), Error::<T>::NoUnbondingWhenSigner);
+            let next_signers = Self::next_signers();
+            if next_signers.is_some() {
+                ensure!(
+                    !next_signers
+                        .ok_or(Error::<T>::ReshareNotInProgress)?
+                        .next_signers
+                        .contains(&validator_id),
+                    Error::<T>::NoUnbondingWhenNextSigner
+                );
+            }
+
             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
             if pallet_staking::Pallet::<T>::bonded(&controller).is_none() {
@@ -421,15 +501,15 @@ 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.len() as u32)).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>,
@@ -468,7 +548,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())?;
@@ -478,7 +558,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())

pallets/staking/src/mock.rs

@@ -396,7 +396,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)],
     };
     let pallet_staking_extension = pallet_staking_extension::GenesisConfig::<Test> {
         // (ValidatorID, (AccountId, X25519PublicKey, TssServerURL))

pallets/staking/src/tests.rs

@@ -242,7 +242,7 @@ fn it_will_not_allow_existing_tss_account_when_changing_threshold_account() {
 #[test]
 fn it_deletes_when_no_bond_left() {
     new_test_ext().execute_with(|| {
-        Signers::<Test>::put(vec![5, 6]);
+        Signers::<Test>::put(vec![5, 6, 7]);
         start_active_era(1);
         assert_ok!(FrameStaking::bond(
             RuntimeOrigin::signed(2),
@@ -311,6 +311,30 @@ fn it_deletes_when_no_bond_left() {
         assert_eq!(Staking::threshold_to_stash(3), None);
         // validator no longer synced
         assert_eq!(Staking::is_validator_synced(2), false);
+
+        assert_ok!(FrameStaking::bond(
+            RuntimeOrigin::signed(7),
+            100u64,
+            pallet_staking::RewardDestination::Account(1),
+        ));
+
+        assert_noop!(
+            Staking::withdraw_unbonded(RuntimeOrigin::signed(7), 0),
+            Error::<Test>::NoUnbondingWhenSigner
+        );
+
+        assert_ok!(FrameStaking::bond(
+            RuntimeOrigin::signed(8),
+            100u64,
+            pallet_staking::RewardDestination::Account(1),
+        ));
+
+        NextSigners::<Test>::put(NextSignerInfo { next_signers: vec![8], confirmations: vec![] });
+
+        assert_noop!(
+            Staking::withdraw_unbonded(RuntimeOrigin::signed(8), 0),
+            Error::<Test>::NoUnbondingWhenNextSigner
+        );
     });
 }
 
@@ -459,3 +483,66 @@ fn it_confirms_keyshare() {
         assert_eq!(Staking::signers(), [6, 5], "next signers rotated into current signers");
     });
 }
+
+#[test]
+fn it_stops_unbonded_when_signer_or_next_signer() {
+    new_test_ext().execute_with(|| {
+        Signers::<Test>::put(vec![7]);
+        start_active_era(1);
+
+        assert_ok!(FrameStaking::bond(
+            RuntimeOrigin::signed(7),
+            100u64,
+            pallet_staking::RewardDestination::Account(1),
+        ));
+
+        assert_noop!(
+            Staking::unbond(RuntimeOrigin::signed(7), 0),
+            Error::<Test>::NoUnbondingWhenSigner
+        );
+
+        assert_ok!(FrameStaking::bond(
+            RuntimeOrigin::signed(8),
+            100u64,
+            pallet_staking::RewardDestination::Account(1),
+        ));
+
+        NextSigners::<Test>::put(NextSignerInfo { next_signers: vec![8], confirmations: vec![] });
+        assert_noop!(
+            Staking::unbond(RuntimeOrigin::signed(8), 0),
+            Error::<Test>::NoUnbondingWhenNextSigner
+        );
+    });
+}
+
+#[test]
+fn it_stops_chill_when_signer_or_next_signer() {
+    new_test_ext().execute_with(|| {
+        Signers::<Test>::put(vec![7]);
+        start_active_era(1);
+
+        assert_ok!(FrameStaking::bond(
+            RuntimeOrigin::signed(7),
+            100u64,
+            pallet_staking::RewardDestination::Account(1),
+        ));
+
+        assert_noop!(
+            Staking::chill(RuntimeOrigin::signed(7)),
+            Error::<Test>::NoUnbondingWhenSigner
+        );
+
+        assert_ok!(FrameStaking::bond(
+            RuntimeOrigin::signed(8),
+            100u64,
+            pallet_staking::RewardDestination::Account(1),
+        ));
+
+        NextSigners::<Test>::put(NextSignerInfo { next_signers: vec![8], confirmations: vec![] });
+
+        assert_noop!(
+            Staking::chill(RuntimeOrigin::signed(8)),
+            Error::<Test>::NoUnbondingWhenNextSigner
+        );
+    });
+}