Skip to content

Commit 6dd1201

Browse files
Ank4ngithub-actions[bot]
authored andcommitted
[Staking] Cleanups and some improvements (#8701)
## Changes - Introduced a new `min_bond` value, which is the minimum of `MinValidatorBond` and `MinNominatorBond`, with a fallback to `ExistentialDeposit`. Since ED on AH is much lower than on RC, this ensures we enforce some min bonds for staking to cover storage costs for staking ledger and related data. - Added an upper bound on era duration, protecting against anomalous conditions that could otherwise lead to excessive inflation. - Some refactors to gracefully handle unexpected validator activation in RC. ## TODO - [ ] Set `MaxEraDuration` in WAH. - [ ] Port [full unbond](#3811) (will do in a separate PR) --------- Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent 79dd3c8 commit 6dd1201

9 files changed

Lines changed: 307 additions & 93 deletions

File tree

substrate/frame/staking-async/ahm-test/src/ah/mock.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,10 @@ pub fn roll_until_matches(criteria: impl Fn() -> bool, with_rc: bool) {
8383
/// Use the given `end_index` as the first session report, and increment as per needed.
8484
pub(crate) fn roll_until_next_active(mut end_index: SessionIndex) -> Vec<AccountId> {
8585
// receive enough session reports, such that we plan a new era
86-
let planned_era = pallet_staking_async::session_rotation::Rotator::<Runtime>::planning_era();
86+
let planned_era = pallet_staking_async::session_rotation::Rotator::<Runtime>::planned_era();
8787
let active_era = pallet_staking_async::session_rotation::Rotator::<Runtime>::active_era();
8888

89-
while pallet_staking_async::session_rotation::Rotator::<Runtime>::planning_era() == planned_era
90-
{
89+
while pallet_staking_async::session_rotation::Rotator::<Runtime>::planned_era() == planned_era {
9190
let report = SessionReport {
9291
end_index,
9392
activation_timestamp: None,
@@ -342,6 +341,7 @@ impl pallet_staking_async::Config for Runtime {
342341
type RewardRemainder = ();
343342
type Slash = ();
344343
type SlashDeferDuration = SlashDeferredDuration;
344+
type MaxEraDuration = ();
345345

346346
type HistoryDepth = ConstU32<7>;
347347
type MaxControllersInDeprecationBatch = ();

substrate/frame/staking-async/runtimes/parachain/src/staking.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,10 @@ parameter_types! {
244244
// of nominators.
245245
pub const MaxControllersInDeprecationBatch: u32 = 751;
246246
pub const MaxNominations: u32 = <NposCompactSolution16 as frame_election_provider_support::NposSolution>::LIMIT as u32;
247+
// Note: In WAH, this should be set closer to the ideal era duration to trigger capping more
248+
// frequently. On Kusama and Polkadot, a higher value like 7 × ideal_era_duration is more
249+
// appropriate.
250+
pub const MaxEraDuration: u64 = RelaySessionDuration::get() as u64 * RELAY_CHAIN_SLOT_DURATION_MILLIS as u64 * SessionsPerEra::get() as u64;
247251
}
248252

249253
impl pallet_staking_async::Config for Runtime {
@@ -273,6 +277,7 @@ impl pallet_staking_async::Config for Runtime {
273277
type EventListeners = (NominationPools, DelegatedStaking);
274278
type WeightInfo = weights::pallet_staking_async::WeightInfo<Runtime>;
275279
type MaxInvulnerables = frame_support::traits::ConstU32<20>;
280+
type MaxEraDuration = MaxEraDuration;
276281
type MaxDisabledValidators = ConstU32<100>;
277282
type PlanningEraOffset =
278283
pallet_staking_async::PlanningEraOffsetOf<Self, RelaySessionDuration, ConstU32<10>>;

substrate/frame/staking-async/src/benchmarking.rs

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ mod benchmarks {
229229
// clean up any existing state.
230230
clear_validators_and_nominators::<T>();
231231

232-
let origin_weight = MinNominatorBond::<T>::get().max(asset::existential_deposit::<T>());
232+
let origin_weight = Staking::<T>::min_nominator_bond();
233233

234234
// setup the worst case list scenario.
235235

@@ -318,7 +318,7 @@ mod benchmarks {
318318
// clean up any existing state.
319319
clear_validators_and_nominators::<T>();
320320

321-
let origin_weight = MinNominatorBond::<T>::get().max(asset::existential_deposit::<T>());
321+
let origin_weight = Staking::<T>::min_nominator_bond();
322322

323323
// setup a worst case list scenario. Note that we don't care about the setup of the
324324
// destination position because we are doing a removal from the list but no insert.
@@ -442,7 +442,7 @@ mod benchmarks {
442442
// clean up any existing state.
443443
clear_validators_and_nominators::<T>();
444444

445-
let origin_weight = MinNominatorBond::<T>::get().max(asset::existential_deposit::<T>());
445+
let origin_weight = Staking::<T>::min_nominator_bond();
446446

447447
// setup a worst case list scenario. Note we don't care about the destination position,
448448
// because we are just doing an insert into the origin position.
@@ -475,7 +475,7 @@ mod benchmarks {
475475
// clean up any existing state.
476476
clear_validators_and_nominators::<T>();
477477

478-
let origin_weight = MinNominatorBond::<T>::get().max(asset::existential_deposit::<T>());
478+
let origin_weight = Staking::<T>::min_nominator_bond();
479479

480480
// setup a worst case list scenario. Note that we don't care about the setup of the
481481
// destination position because we are doing a removal from the list but no insert.
@@ -633,7 +633,7 @@ mod benchmarks {
633633
// Clean up any existing state.
634634
clear_validators_and_nominators::<T>();
635635

636-
let origin_weight = MinNominatorBond::<T>::get().max(asset::existential_deposit::<T>());
636+
let origin_weight = Staking::<T>::min_nominator_bond();
637637

638638
// setup a worst case list scenario. Note that we don't care about the setup of the
639639
// destination position because we are doing a removal from the list but no insert.
@@ -734,8 +734,7 @@ mod benchmarks {
734734
// clean up any existing state.
735735
clear_validators_and_nominators::<T>();
736736

737-
let origin_weight = MinNominatorBond::<T>::get()
738-
.max(asset::existential_deposit::<T>())
737+
let origin_weight = Pallet::<T>::min_nominator_bond()
739738
// we use 100 to play friendly with the list threshold values in the mock
740739
.max(100u32.into());
741740

@@ -781,7 +780,7 @@ mod benchmarks {
781780
// clean up any existing state.
782781
clear_validators_and_nominators::<T>();
783782

784-
let origin_weight = MinNominatorBond::<T>::get().max(asset::existential_deposit::<T>());
783+
let origin_weight = Staking::<T>::min_nominator_bond();
785784

786785
// setup a worst case list scenario. Note that we don't care about the setup of the
787786
// destination position because we are doing a removal from the list but no insert.
@@ -858,7 +857,7 @@ mod benchmarks {
858857
// clean up any existing state.
859858
clear_validators_and_nominators::<T>();
860859

861-
let origin_weight = MinNominatorBond::<T>::get().max(asset::existential_deposit::<T>());
860+
let origin_weight = Staking::<T>::min_nominator_bond();
862861

863862
// setup a worst case list scenario. Note that we don't care about the setup of the
864863
// destination position because we are doing a removal from the list but no insert.
@@ -1024,14 +1023,14 @@ mod benchmarks {
10241023
let _new_validators = Rotator::<T>::legacy_insta_plan_era();
10251024
// activate the previous one
10261025
Rotator::<T>::start_era(
1027-
crate::ActiveEraInfo { index: Rotator::<T>::planning_era() - 1, start: Some(1) },
1026+
crate::ActiveEraInfo { index: Rotator::<T>::planned_era() - 1, start: Some(1) },
10281027
42, // start session index doesn't really matter,
10291028
2, // timestamp doesn't really matter
10301029
);
10311030

10321031
// ensure our offender has at least a full exposure page
10331032
let offender_exposure =
1034-
Eras::<T>::get_full_exposure(Rotator::<T>::planning_era(), &offender);
1033+
Eras::<T>::get_full_exposure(Rotator::<T>::planned_era(), &offender);
10351034
ensure!(
10361035
offender_exposure.others.len() as u32 == 2 * T::MaxExposurePageSize::get(),
10371036
"exposure not created"
@@ -1073,7 +1072,7 @@ mod benchmarks {
10731072
fn rc_on_offence(
10741073
v: Linear<2, { T::MaxValidatorSet::get() / 2 }>,
10751074
) -> Result<(), BenchmarkError> {
1076-
let initial_era = Rotator::<T>::planning_era();
1075+
let initial_era = Rotator::<T>::planned_era();
10771076
let _ = crate::testing_utils::create_validators_with_nominators_for_era::<T>(
10781077
2 * v,
10791078
// number of nominators is irrelevant here, so we hardcode these
@@ -1085,7 +1084,7 @@ mod benchmarks {
10851084

10861085
// plan new era
10871086
let new_validators = Rotator::<T>::legacy_insta_plan_era();
1088-
ensure!(Rotator::<T>::planning_era() == initial_era + 1, "era should be incremented");
1087+
ensure!(Rotator::<T>::planned_era() == initial_era + 1, "era should be incremented");
10891088
// activate the previous one
10901089
Rotator::<T>::start_era(
10911090
crate::ActiveEraInfo { index: initial_era, start: Some(1) },
@@ -1135,7 +1134,7 @@ mod benchmarks {
11351134

11361135
#[benchmark]
11371136
fn rc_on_session_report() -> Result<(), BenchmarkError> {
1138-
let initial_planned_era = Rotator::<T>::planning_era();
1137+
let initial_planned_era = Rotator::<T>::planned_era();
11391138
let initial_active_era = Rotator::<T>::active_era();
11401139

11411140
// create a small, arbitrary number of stakers. This is just for sanity of the era planning,

substrate/frame/staking-async/src/mock.rs

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -312,16 +312,29 @@ pub mod session_mock {
312312
if QueuedBufferSessions::get() == 0 {
313313
// buffer time has passed
314314
Active::set(q);
315-
Rotator::<Test>::end_session(ending, Some((Timestamp::get(), id)));
315+
<Staking as rc_client::AHStakingInterface>::on_relay_session_report(
316+
rc_client::SessionReport::new_terminal(
317+
ending,
318+
// TODO: currently we use `Eras::reward_active_era()` to set validator
319+
// points in our tests. We should improve this and find a good way to
320+
// set this value instead.
321+
vec![],
322+
Some((Timestamp::get(), id)),
323+
),
324+
);
316325
Queued::reset();
317326
QueuedId::reset();
318327
} else {
319328
QueuedBufferSessions::mutate(|s| *s -= 1);
320-
Rotator::<Test>::end_session(ending, None);
329+
<Staking as rc_client::AHStakingInterface>::on_relay_session_report(
330+
rc_client::SessionReport::new_terminal(ending, vec![], None),
331+
);
321332
}
322333
} else {
323334
// just end the session.
324-
Rotator::<Test>::end_session(ending, None);
335+
<Staking as rc_client::AHStakingInterface>::on_relay_session_report(
336+
rc_client::SessionReport::new_terminal(ending, vec![], None),
337+
);
325338
}
326339
CurrentIndex::set(ending + 1);
327340
}
@@ -385,6 +398,7 @@ ord_parameter_types! {
385398

386399
parameter_types! {
387400
pub static RemainderRatio: Perbill = Perbill::from_percent(50);
401+
pub static MaxEraDuration: u64 = time_per_era() * 7;
388402
}
389403
pub struct OneTokenPerMillisecond;
390404
impl EraPayout<Balance> for OneTokenPerMillisecond {
@@ -423,6 +437,7 @@ impl crate::pallet::pallet::Config for Test {
423437
type EventListeners = EventListenerMock;
424438
type MaxInvulnerables = ConstU32<20>;
425439
type MaxDisabledValidators = ConstU32<100>;
440+
type MaxEraDuration = MaxEraDuration;
426441
type PlanningEraOffset = PlanningEraOffset;
427442
type Filter = MockedRestrictList;
428443
type RcClientInterface = session_mock::Session;

substrate/frame/staking-async/src/pallet/impls.rs

Lines changed: 44 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,30 @@ use sp_runtime::TryRuntimeError;
7272
const NPOS_MAX_ITERATIONS_COEFFICIENT: u32 = 2;
7373

7474
impl<T: Config> Pallet<T> {
75+
/// Returns the minimum required bond for participation, considering nominators,
76+
/// and the chain’s existential deposit.
77+
///
78+
/// This function computes the smallest allowed bond among `MinValidatorBond` and
79+
/// `MinNominatorBond`, but ensures it is not below the existential deposit required to keep an
80+
/// account alive.
81+
pub(crate) fn min_chilled_bond() -> BalanceOf<T> {
82+
MinValidatorBond::<T>::get()
83+
.min(MinNominatorBond::<T>::get())
84+
.max(asset::existential_deposit::<T>())
85+
}
86+
87+
/// Returns the minimum required bond for validators, defaulting to `MinNominatorBond` if not
88+
/// set or less than `MinNominatorBond`.
89+
pub(crate) fn min_validator_bond() -> BalanceOf<T> {
90+
MinValidatorBond::<T>::get().max(Self::min_nominator_bond())
91+
}
92+
93+
/// Returns the minimum required bond for nominators, considering the chain’s existential
94+
/// deposit.
95+
pub(crate) fn min_nominator_bond() -> BalanceOf<T> {
96+
MinNominatorBond::<T>::get().max(asset::existential_deposit::<T>())
97+
}
98+
7599
/// Fetches the ledger associated with a controller or stash account, if any.
76100
pub fn ledger(account: StakingAccount<T::AccountId>) -> Result<StakingLedger<T>, Error<T>> {
77101
StakingLedger::<T>::get(account)
@@ -169,8 +193,8 @@ impl<T: Config> Pallet<T> {
169193

170194
ledger.total = ledger.total.checked_add(&extra).ok_or(ArithmeticError::Overflow)?;
171195
ledger.active = ledger.active.checked_add(&extra).ok_or(ArithmeticError::Overflow)?;
172-
// last check: the new active amount of ledger must be more than ED.
173-
ensure!(ledger.active >= asset::existential_deposit::<T>(), Error::<T>::InsufficientBond);
196+
// last check: the new active amount of ledger must be more than min bond.
197+
ensure!(ledger.active >= Self::min_chilled_bond(), Error::<T>::InsufficientBond);
174198

175199
// NOTE: ledger must be updated prior to calling `Self::weight_of`.
176200
ledger.update()?;
@@ -193,22 +217,22 @@ impl<T: Config> Pallet<T> {
193217
}
194218
let new_total = ledger.total;
195219

196-
let ed = asset::existential_deposit::<T>();
197-
let used_weight =
198-
if ledger.unlocking.is_empty() && (ledger.active < ed || ledger.active.is_zero()) {
199-
// This account must have called `unbond()` with some value that caused the active
200-
// portion to fall below existential deposit + will have no more unlocking chunks
201-
// left. We can now safely remove all staking-related information.
202-
Self::kill_stash(&ledger.stash)?;
220+
let used_weight = if ledger.unlocking.is_empty() &&
221+
(ledger.active < Self::min_chilled_bond() || ledger.active.is_zero())
222+
{
223+
// This account must have called `unbond()` with some value that caused the active
224+
// portion to fall below existential deposit + will have no more unlocking chunks
225+
// left. We can now safely remove all staking-related information.
226+
Self::kill_stash(&ledger.stash)?;
203227

204-
T::WeightInfo::withdraw_unbonded_kill()
205-
} else {
206-
// This was the consequence of a partial unbond. just update the ledger and move on.
207-
ledger.update()?;
228+
T::WeightInfo::withdraw_unbonded_kill()
229+
} else {
230+
// This was the consequence of a partial unbond. just update the ledger and move on.
231+
ledger.update()?;
208232

209-
// This is only an update, so we use less overall weight.
210-
T::WeightInfo::withdraw_unbonded_update()
211-
};
233+
// This is only an update, so we use less overall weight.
234+
T::WeightInfo::withdraw_unbonded_update()
235+
};
212236

213237
// `old_total` should never be less than the new total because
214238
// `consolidate_unlocked` strictly subtracts balance.
@@ -972,7 +996,7 @@ impl<T: Config> ElectionDataProvider for Pallet<T> {
972996

973997
#[cfg(feature = "runtime-benchmarks")]
974998
fn add_target(target: T::AccountId) {
975-
let stake = (MinValidatorBond::<T>::get() + 1u32.into()) * 100u32.into();
999+
let stake = (Self::min_validator_bond() + 1u32.into()) * 100u32.into();
9761000
<Bonded<T>>::insert(target.clone(), target.clone());
9771001
<Ledger<T>>::insert(target.clone(), StakingLedger::<T>::new(target.clone(), stake));
9781002
Self::do_add_validator(
@@ -1004,7 +1028,7 @@ impl<T: Config> ElectionDataProvider for Pallet<T> {
10041028
targets.into_iter().for_each(|v| {
10051029
let stake: BalanceOf<T> = target_stake
10061030
.and_then(|w| <BalanceOf<T>>::try_from(w).ok())
1007-
.unwrap_or_else(|| MinNominatorBond::<T>::get() * 100u32.into());
1031+
.unwrap_or_else(|| Self::min_nominator_bond() * 100u32.into());
10081032
<Bonded<T>>::insert(v.clone(), v.clone());
10091033
<Ledger<T>>::insert(v.clone(), StakingLedger::<T>::new(v.clone(), stake));
10101034
Self::do_add_validator(
@@ -1425,11 +1449,11 @@ impl<T: Config> StakingInterface for Pallet<T> {
14251449
type CurrencyToVote = T::CurrencyToVote;
14261450

14271451
fn minimum_nominator_bond() -> Self::Balance {
1428-
MinNominatorBond::<T>::get()
1452+
Self::min_nominator_bond()
14291453
}
14301454

14311455
fn minimum_validator_bond() -> Self::Balance {
1432-
MinValidatorBond::<T>::get()
1456+
Self::min_validator_bond()
14331457
}
14341458

14351459
fn stash_by_ctrl(controller: &Self::AccountId) -> Result<Self::AccountId, DispatchError> {

0 commit comments

Comments
 (0)