Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No unbonding when signer or next signer #1031

Merged
merged 24 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 78 additions & 1 deletion pallets/staking/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand All @@ -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,
Expand Down Expand Up @@ -124,11 +136,76 @@ benchmarks! {
assert_last_event::<T>(Event::<T>::ThresholdAccountChanged(bonder, server_info).into());
}

unbond {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you take the body of this (and all the other benches added in this PR) and manually RustFmt it using the Rust Playground? This isn't done by the fmt script since it's since a decl macro.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bumping this

HCastano marked this conversation as resolved.
Show resolved Hide resolved
let c in 0 .. MAX_SIGNERS as u32;
JesseAbram marked this conversation as resolved.
Show resolved Hide resolved

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();
Expand Down
92 changes: 86 additions & 6 deletions pallets/staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,8 @@ pub mod pallet {
NotNextSigner,
ReshareNotInProgress,
AlreadyConfirmed,
NoUnbodingWhenSigner,
JesseAbram marked this conversation as resolved.
Show resolved Hide resolved
NoUnbodingWhenNextSigner,
JesseAbram marked this conversation as resolved.
Show resolved Hide resolved
}

#[pallet::event]
Expand Down Expand Up @@ -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 targeted validator sure not signer or next signe
JesseAbram marked this conversation as resolved.
Show resolved Hide resolved
#[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>::NoUnbodingWhenSigner);

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>::NoUnbodingWhenNextSigner
);
}

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
JesseAbram marked this conversation as resolved.
Show resolved Hide resolved
#[pallet::call_index(3)]
#[pallet::weight(<T as Config>::WeightInfo::chill(MAX_SIGNERS as u32))]
pub fn chill(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call it fate, but I was listening to this while reviewing 😆

let controller = ensure_signed(origin.clone())?;
let ledger =
pallet_staking::Pallet::<T>::ledger(StakingAccount::Controller(controller.clone()))
.map_err(|_| Error::<T>::NoThresholdKey)?;
HCastano marked this conversation as resolved.
Show resolved Hide resolved

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>::NoUnbodingWhenSigner);

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>::NoUnbodingWhenNextSigner
);
}

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
JesseAbram marked this conversation as resolved.
Show resolved Hide resolved
#[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,
Expand All @@ -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>::NoUnbodingWhenSigner);
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>::NoUnbodingWhenNextSigner
);
}

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() {
Expand All @@ -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>,
Expand Down Expand Up @@ -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())?;
Expand All @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion pallets/staking/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
86 changes: 85 additions & 1 deletion pallets/staking/src/tests.rs
HCastano marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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>::NoUnbodingWhenSigner
);

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>::NoUnbodingWhenNextSigner
);
});
}

Expand Down Expand Up @@ -459,3 +483,63 @@ 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>::NoUnbodingWhenSigner
);

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>::NoUnbodingWhenNextSigner
);
});
}

#[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>::NoUnbodingWhenSigner);

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>::NoUnbodingWhenNextSigner
);
});
}
Loading
Loading