pezkuwichain/pezkuwi-subxt
1
0
Fork 0
mirror of https://github.com/pezkuwichain/pezkuwi-subxt.git synced 2026-04-27 05:47:58 +00:00
Code Issues Packages Projects Releases Wiki Activity

[fix] Bound staking ledger correctly with MaxUnlockingChunks from configuration (#12343)

Browse Source 
* used maxunlockingchunks from config

* mhl MaxUnlockingChunks

* no migration needed

* changes as per requested

* fmt

* fix tests

* fix benchmark

* warning in the doc for abrupt changes in the config

* less unnecessary details in the test

* fix tests

Co-authored-by: mrisholukamba <abdulrazzaqlukamba@gmail.com>
Co-authored-by: parity-processbot <>
This commit is contained in:
Ankan
2022-09-27 17:44:16 +02:00
committed by GitHub
parent 249316d87f
commit e6b1aae97f
5 changed files with 80 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
substrate/frame/staking/src/benchmarking.rs
+2 -2
View File
@@ -613,7 +613,7 @@ benchmarks! {
}
rebond {
let l in 1 .. MaxUnlockingChunks::get() as u32;
let l in 1 .. T::MaxUnlockingChunks::get() as u32;
// clean up any existing state.
clear_validators_and_nominators::<T>();
@@ -764,7 +764,7 @@ benchmarks! {
#[extra]
do_slash {
let l in 1 .. MaxUnlockingChunks::get() as u32;
let l in 1 .. T::MaxUnlockingChunks::get() as u32;
let (stash, controller) = create_stash_controller::<T>(0, 100, Default::default())?;
let mut staking_ledger = Ledger::<T>::get(controller.clone()).unwrap();
let unlock_chunk = UnlockChunk::<BalanceOf<T>> {
substrate/frame/staking/src/lib.rs
+1 -6
View File
@@ -301,7 +301,6 @@ mod pallet;
use codec::{Decode, Encode, HasCompact, MaxEncodedLen};
use frame_support::{
parameter_types,
traits::{Currency, Defensive, Get},
weights::Weight,
BoundedVec, CloneNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound,
@@ -349,10 +348,6 @@ 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)]
pub struct ActiveEraInfo {
@@ -465,7 +460,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>,
substrate/frame/staking/src/mock.rs
+2 -1
View File
@@ -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());
}
@@ -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;
substrate/frame/staking/src/pallet/mod.rs
+18 -9
View File
@@ -43,9 +43,9 @@ pub use impls::*;
use crate::{
slashing, weights::WeightInfo, AccountIdLookupOf, ActiveEraInfo, BalanceOf, EraPayout,
EraRewardPoints, Exposure, Forcing, MaxUnlockingChunks, NegativeImbalanceOf, Nominations,
PositiveImbalanceOf, Releases, RewardDestination, SessionInterface, StakingLedger,
UnappliedSlash, UnlockChunk, ValidatorPrefs,
EraRewardPoints, Exposure, Forcing, NegativeImbalanceOf, Nominations, PositiveImbalanceOf,
Releases, RewardDestination, SessionInterface, StakingLedger, UnappliedSlash, UnlockChunk,
ValidatorPrefs,
};
const STAKING_ID: LockIdentifier = *b"staking ";
@@ -142,8 +142,9 @@ pub mod pallet {
///
/// Note: `HistoryDepth` is used as the upper bound for the `BoundedVec`
/// item `StakingLedger.claimed_rewards`. Setting this value lower than
/// the existing value can lead to inconsistencies and will need to be
/// handled properly in a migration.
/// the existing value can lead to inconsistencies in the
/// `StakingLedger` and will need to be handled properly in a migration.
/// The test `reducing_history_depth_abrupt` shows this effect.
#[pallet::constant]
type HistoryDepth: Get<u32>;
@@ -237,8 +238,16 @@ pub mod pallet {
/// VALIDATOR.
type TargetList: SortedListProvider<Self::AccountId, Score = BalanceOf<Self>>;
/// The maximum number of `unlocking` chunks a [`StakingLedger`] can have. Effectively
/// determines how many unique eras a staker may be unbonding in.
/// The maximum number of `unlocking` chunks a [`StakingLedger`] can
/// have. Effectively determines how many unique eras a staker may be
/// unbonding in.
///
/// Note: `MaxUnlockingChunks` is used as the upper bound for the
/// `BoundedVec` item `StakingLedger.unlocking`. Setting this value
/// lower than the existing value can lead to inconsistencies in the
/// `StakingLedger` and will need to be handled properly in a runtime
/// migration. The test `reducing_max_unlocking_chunks_abrupt` shows
/// this effect.
#[pallet::constant]
type MaxUnlockingChunks: Get<u32>;
@@ -940,7 +949,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,
);
@@ -1454,7 +1463,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>,
substrate/frame/staking/src/tests.rs
+57 -4
View File
@@ -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,
@@ -1354,7 +1354,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 {
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);
@@ -1369,7 +1370,7 @@ 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
);
// can't do more.
assert_noop!(Staking::unbond(RuntimeOrigin::signed(10), 1), Error::<Test>::NoMoreChunks);
@@ -5494,7 +5495,7 @@ fn pre_bonding_era_cannot_be_claimed() {
}
#[test]
fn reducing_history_depth_without_migration() {
fn reducing_history_depth_abrupt() {
// Verifies initial conditions of mock
ExtBuilder::default().nominate(false).build_and_execute(|| {
let original_history_depth = HistoryDepth::get();
@@ -5571,3 +5572,55 @@ fn reducing_history_depth_without_migration() {
HistoryDepth::set(original_history_depth);
});
}
#[test]
fn reducing_max_unlocking_chunks_abrupt() {
// Concern is on validators only
// By Default 11, 10 are stash and ctrl and 21,20
ExtBuilder::default().build_and_execute(|| {
// given a staker at era=10 and MaxUnlockChunks set to 2
MaxUnlockingChunks::set(2);
start_active_era(10);
assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 4, 300, RewardDestination::Staked));
assert!(matches!(Staking::ledger(4), Some(_)));
// when staker unbonds
assert_ok!(Staking::unbond(RuntimeOrigin::signed(4), 20));
// then an unlocking chunk is added at `current_era + bonding_duration`
// => 10 + 3 = 13
let expected_unlocking: BoundedVec<UnlockChunk<Balance>, MaxUnlockingChunks> =
bounded_vec![UnlockChunk { value: 20 as Balance, era: 13 as EraIndex }];
assert!(matches!(Staking::ledger(4),
Some(StakingLedger {
unlocking,
..
}) if unlocking==expected_unlocking));
// when staker unbonds at next era
start_active_era(11);
assert_ok!(Staking::unbond(RuntimeOrigin::signed(4), 50));
// then another unlock chunk is added
let expected_unlocking: BoundedVec<UnlockChunk<Balance>, MaxUnlockingChunks> =
bounded_vec![UnlockChunk { value: 20, era: 13 }, UnlockChunk { value: 50, era: 14 }];
assert!(matches!(Staking::ledger(4),
Some(StakingLedger {
unlocking,
..
}) if unlocking==expected_unlocking));
// when staker unbonds further
start_active_era(12);
// then further unbonding not possible
assert_noop!(Staking::unbond(RuntimeOrigin::signed(4), 20), Error::<Test>::NoMoreChunks);
// when max unlocking chunks is reduced abruptly to a low value
MaxUnlockingChunks::set(1);
// then unbond, rebond ops are blocked with ledger in corrupt state
assert_noop!(Staking::unbond(RuntimeOrigin::signed(4), 20), Error::<Test>::NotController);
assert_noop!(Staking::rebond(RuntimeOrigin::signed(4), 100), Error::<Test>::NotController);
// reset the ledger corruption
MaxUnlockingChunks::set(2);
})
}