From 70e9f8e920fbd8353b640641eca20286d1cb5865 Mon Sep 17 00:00:00 2001
From: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Date: Fri, 23 Dec 2022 08:04:47 +0100
Subject: [PATCH] Fix fast-unstake for accounts with slashing (#12963)
* Fix fast-unstake for accounts with slashing
* ".git/.scripts/fmt.sh" 1
* fmt
* fix
* fix weight tracking
* Adds tests for withdraw_unbonded with slashing
* Removes tests for withdraw_unbonded with slashing
* ".git/.scripts/fmt.sh"
* Adds slash spans calculation test for withdraw_unbonded
Co-authored-by: command-bot <>
Co-authored-by: gpestana <g6pestana@gmail.com>
---
substrate/frame/fast-unstake/src/lib.rs | 14 +++---
substrate/frame/staking/src/pallet/impls.rs | 2 +-
substrate/frame/staking/src/pallet/mod.rs | 3 +-
substrate/frame/staking/src/tests.rs | 51 +++++++++++++++++++++
4 files changed, 62 insertions(+), 8 deletions(-)
diff --git a/substrate/frame/fast-unstake/src/lib.rs b/substrate/frame/fast-unstake/src/lib.rs
index 7f226826cbc..f2faeebc134 100644
--- a/substrate/frame/fast-unstake/src/lib.rs
+++ b/substrate/frame/fast-unstake/src/lib.rs
@@ -91,7 +91,7 @@ pub mod pallet {
DispatchResult,
};
use sp_staking::{EraIndex, StakingInterface};
- use sp_std::{prelude::*, vec::Vec};
+ use sp_std::{collections::btree_set::BTreeSet, prelude::*, vec::Vec};
pub use weights::WeightInfo;
#[derive(scale_info::TypeInfo, codec::Encode, codec::Decode, codec::MaxEncodedLen)]
@@ -429,9 +429,9 @@ pub mod pallet {
}
};
- let check_stash = |stash, deposit, eras_checked: &mut u32| {
+ let check_stash = |stash, deposit, eras_checked: &mut BTreeSet<EraIndex>| {
let is_exposed = unchecked_eras_to_check.iter().any(|e| {
- eras_checked.saturating_inc();
+ eras_checked.insert(*e);
T::Staking::is_exposed_in_era(&stash, e)
});
@@ -452,7 +452,7 @@ pub mod pallet {
<T as Config>::WeightInfo::on_idle_unstake()
} else {
// eras checked so far.
- let mut eras_checked = 0u32;
+ let mut eras_checked = BTreeSet::<EraIndex>::new();
let pre_length = stashes.len();
let stashes: BoundedVec<(T::AccountId, BalanceOf<T>), T::BatchSize> = stashes
@@ -468,7 +468,7 @@ pub mod pallet {
log!(
debug,
"checked {:?} eras, pre stashes: {:?}, post: {:?}",
- eras_checked,
+ eras_checked.len(),
pre_length,
post_length,
);
@@ -489,7 +489,9 @@ pub mod pallet {
},
}
- <T as Config>::WeightInfo::on_idle_check(validator_count * eras_checked)
+ <T as Config>::WeightInfo::on_idle_check(
+ validator_count * eras_checked.len() as u32,
+ )
}
}
}
diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs
index a7190d70c70..db9aeba6fb5 100644
--- a/substrate/frame/staking/src/pallet/impls.rs
+++ b/substrate/frame/staking/src/pallet/impls.rs
@@ -1569,7 +1569,7 @@ impl<T: Config> StakingInterface for Pallet<T> {
}
fn force_unstake(who: Self::AccountId) -> sp_runtime::DispatchResult {
- let num_slashing_spans = Self::slashing_spans(&who).iter().count() as u32;
+ let num_slashing_spans = Self::slashing_spans(&who).map_or(0, |s| s.iter().count() as u32);
Self::force_unstake(RawOrigin::Root.into(), who.clone(), num_slashing_spans)
}
diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs
index 1d5babe7ffa..f04d13d05f4 100644
--- a/substrate/frame/staking/src/pallet/mod.rs
+++ b/substrate/frame/staking/src/pallet/mod.rs
@@ -982,7 +982,8 @@ pub mod pallet {
// `BondingDuration` to proceed with the unbonding.
let maybe_withdraw_weight = {
if unlocking == T::MaxUnlockingChunks::get() as usize {
- let real_num_slashing_spans = Self::slashing_spans(&controller).iter().count();
+ let real_num_slashing_spans =
+ Self::slashing_spans(&controller).map_or(0, |s| s.iter().count());
Some(Self::do_withdraw_unbonded(&controller, real_num_slashing_spans as u32)?)
} else {
None
diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs
index fc6fc68e66d..5c2766deee3 100644
--- a/substrate/frame/staking/src/tests.rs
+++ b/substrate/frame/staking/src/tests.rs
@@ -5725,3 +5725,54 @@ fn scale_validator_count_errors() {
);
})
}
+
+mod staking_interface {
+ use frame_support::storage::with_storage_layer;
+ use sp_staking::StakingInterface;
+
+ use super::*;
+
+ #[test]
+ fn force_unstake_with_slash_works() {
+ ExtBuilder::default().build_and_execute(|| {
+ // without slash
+ let _ = with_storage_layer::<(), _, _>(|| {
+ // bond an account, can unstake
+ assert_eq!(Staking::bonded(&11), Some(10));
+ assert_ok!(<Staking as StakingInterface>::force_unstake(11));
+ Err(DispatchError::from("revert"))
+ });
+
+ // bond again and add a slash, still can unstake.
+ assert_eq!(Staking::bonded(&11), Some(10));
+ add_slash(&11);
+ assert_ok!(<Staking as StakingInterface>::force_unstake(11));
+ });
+ }
+
+ #[test]
+ fn do_withdraw_unbonded_with_wrong_slash_spans_works_as_expected() {
+ ExtBuilder::default().build_and_execute(|| {
+ on_offence_now(
+ &[OffenceDetails {
+ offender: (11, Staking::eras_stakers(active_era(), 11)),
+ reporters: vec![],
+ }],
+ &[Perbill::from_percent(100)],
+ );
+
+ assert_eq!(Staking::bonded(&11), Some(10));
+
+ assert_noop!(
+ Staking::withdraw_unbonded(RuntimeOrigin::signed(10), 0),
+ Error::<Test>::IncorrectSlashingSpans
+ );
+
+ let num_slashing_spans = Staking::slashing_spans(&11).map_or(0, |s| s.iter().count());
+ assert_ok!(Staking::withdraw_unbonded(
+ RuntimeOrigin::signed(10),
+ num_slashing_spans as u32
+ ));
+ });
+ }
+}
--
GitLab