Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

used MaxUnlockingChunks from config #12289

Closed
Show file tree
Hide file tree
Changes from 4 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
6 changes: 2 additions & 4 deletions frame/staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,9 +349,7 @@ type NegativeImbalanceOf<T> = <<T as Config>::Currency as Currency<

type AccountIdLookupOf<T> = <<T as frame_system::Config>::Lookup as StaticLookup>::Source;

parameter_types! {
pub MaxUnlockingChunks: u32 = 32;
}


/// Information regarding the active era (era in used in session).
#[derive(Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)]
Expand Down Expand Up @@ -465,7 +463,7 @@ pub struct StakingLedger<T: Config> {
/// Any balance that is becoming free, which may eventually be transferred out of the stash
/// (assuming it doesn't get slashed first). It is assumed that this will be treated as a first
/// in, first out queue where the new (higher value) eras get pushed on the back.
pub unlocking: BoundedVec<UnlockChunk<BalanceOf<T>>, MaxUnlockingChunks>,
pub unlocking: BoundedVec<UnlockChunk<BalanceOf<T>>, T::MaxUnlockingChunks>,
/// List of eras for which the stakers behind a validator have claimed rewards. Only updated
/// for validators.
pub claimed_rewards: BoundedVec<EraIndex, T::HistoryDepth>,
Expand Down
3 changes: 2 additions & 1 deletion frame/staking/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ parameter_types! {
pub static BagThresholds: &'static [sp_npos_elections::VoteWeight] = &THRESHOLDS;
pub static MaxNominations: u32 = 16;
pub static HistoryDepth: u32 = 80;
pub static MaxUnlockingChunks: u32 = 32;
pub static RewardOnUnbalanceWasCalled: bool = false;
pub static LedgerSlashPerEra: (BalanceOf<Test>, BTreeMap<EraIndex, BalanceOf<Test>>) = (Zero::zero(), BTreeMap::new());
}
Expand Down Expand Up @@ -301,7 +302,7 @@ impl crate::pallet::pallet::Config for Test {
// NOTE: consider a macro and use `UseNominatorsAndValidatorsMap<Self>` as well.
type VoterList = VoterBagsList;
type TargetList = UseValidatorsMap<Self>;
type MaxUnlockingChunks = ConstU32<32>;
type MaxUnlockingChunks = MaxUnlockingChunks;
type HistoryDepth = HistoryDepth;
type OnStakerSlash = OnStakerSlashMock<Test>;
type BenchmarkingConfig = TestBenchmarkingConfig;
Expand Down
6 changes: 3 additions & 3 deletions frame/staking/src/pallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub use impls::*;

use crate::{
slashing, weights::WeightInfo, AccountIdLookupOf, ActiveEraInfo, BalanceOf, EraPayout,
EraRewardPoints, Exposure, Forcing, MaxUnlockingChunks, NegativeImbalanceOf, Nominations,
EraRewardPoints, Exposure, Forcing, NegativeImbalanceOf, Nominations,
PositiveImbalanceOf, Releases, RewardDestination, SessionInterface, StakingLedger,
UnappliedSlash, UnlockChunk, ValidatorPrefs,
};
Expand Down Expand Up @@ -942,7 +942,7 @@ pub mod pallet {
let controller = ensure_signed(origin)?;
let mut ledger = Self::ledger(&controller).ok_or(Error::<T>::NotController)?;
ensure!(
ledger.unlocking.len() < MaxUnlockingChunks::get() as usize,
ledger.unlocking.len() < T::MaxUnlockingChunks::get() as usize,
Error::<T>::NoMoreChunks,
);

Expand Down Expand Up @@ -1456,7 +1456,7 @@ pub mod pallet {
/// - Bounded by `MaxUnlockingChunks`.
/// - Storage changes: Can't increase storage, only decrease it.
/// # </weight>
#[pallet::weight(T::WeightInfo::rebond(MaxUnlockingChunks::get() as u32))]
#[pallet::weight(T::WeightInfo::rebond(T::MaxUnlockingChunks::get() as u32))]
pub fn rebond(
origin: OriginFor<T>,
#[pallet::compact] value: BalanceOf<T>,
Expand Down
83 changes: 80 additions & 3 deletions frame/staking/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

//! Tests for the module.

use super::{ConfigOp, Event, MaxUnlockingChunks, *};
use super::{ConfigOp, Event, *};
use frame_election_provider_support::{ElectionProvider, SortedListProvider, Support};
use frame_support::{
assert_noop, assert_ok, assert_storage_noop, bounded_vec,
Expand Down Expand Up @@ -1346,7 +1346,8 @@ fn too_many_unbond_calls_should_not_work() {
ExtBuilder::default().build_and_execute(|| {
let mut current_era = 0;
// locked at era MaxUnlockingChunks - 1 until 3
for i in 0..MaxUnlockingChunks::get() - 1 {
//maxunlockingchunks
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//maxunlockingchunks

for i in 0..<<Test as Config>::MaxUnlockingChunks as Get::<u32>>::get()- 1 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for i in 0..<<Test as Config>::MaxUnlockingChunks as Get::<u32>>::get()- 1 {
for i in 0..<<Test as Config>::MaxUnlockingChunks as Get<u32>>::get()-1 {

// There is only 1 chunk per era, so we need to be in a new era to create a chunk.
current_era = i as u32;
mock::start_active_era(current_era);
Expand All @@ -1361,7 +1362,8 @@ fn too_many_unbond_calls_should_not_work() {
assert_ok!(Staking::unbond(RuntimeOrigin::signed(10), 1));
assert_eq!(
Staking::ledger(&10).unwrap().unlocking.len(),
MaxUnlockingChunks::get() as usize

<<Test as Config>::MaxUnlockingChunks as Get::<u32>>::get() as usize
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<<Test as Config>::MaxUnlockingChunks as Get::<u32>>::get() as usize
<<Test as Config>::MaxUnlockingChunks as Get<u32>>::get() as usize

);
// can't do more.
assert_noop!(Staking::unbond(RuntimeOrigin::signed(10), 1), Error::<Test>::NoMoreChunks);
Expand Down Expand Up @@ -5560,3 +5562,78 @@ fn reducing_history_depth_without_migration() {
HistoryDepth::set(original_history_depth);
});
}

#[test]
fn change_max_unlocking_chunks_effect(){
//Concern is on validators only
//By Default 11, 10 are stash and ctrl and 21,20
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//Concern is on validators only
//By Default 11, 10 are stash and ctrl and 21,20
// Concern is on validators only
// By Default 11, 10 are stash and ctrl and 21,20

Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix all comments accordingly.

ExtBuilder::default()
.build_and_execute(||{
//initial state
assert_eq!(MaxUnlockingChunks::get(),32);

//starting an era
let current_era = 10;
start_active_era(current_era);
//bond some funds
assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 4, 300, RewardDestination::Staked));
assert_eq!(Staking::ledger(4),Some(StakingLedger {
stash: 3,
total: 300,
active: 300,
unlocking: Default::default(),
claimed_rewards: bounded_vec![0,1,2,3,4,5,6,7,8,9],
}));
assert_ok!(Staking::unbond(RuntimeOrigin::signed(4),20));
assert_eq!(Staking::ledger(4),Some(StakingLedger {
stash: 3,
total: 300,
active: 280,
unlocking: bounded_vec![UnlockChunk{value:20,era:13}],
claimed_rewards: bounded_vec![0,1,2,3,4,5,6,7,8,9],
}));
assert_ok!(Staking::unbond(RuntimeOrigin::signed(4),50));
assert_eq!(Staking::ledger(4),Some(StakingLedger {
stash: 3,
total: 300,
active: 230,
unlocking: bounded_vec![UnlockChunk{value:70,era:13}],
claimed_rewards: bounded_vec![0,1,2,3,4,5,6,7,8,9],
}));
assert_ok!(Staking::unbond(RuntimeOrigin::signed(4),20));

// changing MaxUnlockingChunks without migration it works
MaxUnlockingChunks::set(10);

assert_eq!(MaxUnlockingChunks::get(),10);
assert_ok!(Staking::unbond(RuntimeOrigin::signed(4),30));
assert_eq!(Staking::ledger(4),Some(StakingLedger {
stash: 3,
total: 300,
active: 180,
unlocking: bounded_vec![UnlockChunk{value:120,era:13}] ,
claimed_rewards: bounded_vec![0,1,2,3,4,5,6,7,8,9],
}));


MaxUnlockingChunks::set(3);

assert_eq!(MaxUnlockingChunks::get(),3);
assert_ok!(Staking::unbond(RuntimeOrigin::signed(4),20));
assert_ok!(Staking::unbond(RuntimeOrigin::signed(4),20));
assert_eq!(Staking::ledger(4),Some(StakingLedger {
stash: 3,
total: 300,
active: 140,
unlocking: bounded_vec![UnlockChunk{value:160,era:13}] ,
claimed_rewards: bounded_vec![0,1,2,3,4,5,6,7,8,9],
}));
//rebonding without migration
assert_ok!(Staking::rebond(RuntimeOrigin::signed(4),100));
MaxUnlockingChunks::set(1);
assert_ok!(Staking::rebond(RuntimeOrigin::signed(4),120));
assert_ok!(Staking::unbond(RuntimeOrigin::signed(4),20));
// No effect on storage items changing MaxUnlockingChunks
// Note decreasing to zero affects rebond and unbond funtions
})
}