Skip to content

Commit

Permalink
a few fixes here and there
Browse files Browse the repository at this point in the history
  • Loading branch information
@girazoki
girazoki committed Oct 30, 2024
1 parent 8955fa3 commit 1b2ff17
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 29 deletions.
64 changes: 41 additions & 23 deletions pallets/external-validator-slashes/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,6 @@ pub mod pallet {
+ TryFrom<Self::AccountId>;

/// A conversion from account ID to validator ID.
///
/// Its cost must be at most one storage read.
type ValidatorIdOf: Convert<Self::AccountId, Option<Self::ValidatorId>>;

/// Number of eras that slashes are deferred by, after computation.
Expand All @@ -91,6 +89,7 @@ pub mod pallet {
#[pallet::constant]
type BondingDuration: Get<EraIndex>;

// SlashId type, used as a counter on the number of slashes
type SlashId: Default
+ FullCodec
+ TypeInfo
Expand All @@ -106,8 +105,10 @@ pub mod pallet {
/// Interface for interacting with a session pallet.
type SessionInterface: SessionInterface<Self::AccountId>;

/// Era index provider, used to fetch the active era among other thins
type EraIndexProvider: EraIndexProvider;

/// Invulnerable provider, used to get the invulnerables to know when not to slash
type InvulnerablesProvider: InvulnerablesProvider<Self::ValidatorId>;

/// The weight information of this pallet.
Expand All @@ -123,6 +124,7 @@ pub mod pallet {
ProvidedNonSlashableEra,
ActiveEraNotSet,
DeferPeriodIsOver,
ErrorComputingSlash,
}

#[pallet::pallet]
Expand All @@ -142,6 +144,7 @@ pub mod pallet {
#[pallet::unbounded]
pub type BondedEras<T: Config> = StorageValue<_, Vec<(EraIndex, SessionIndex)>, ValueQuery>;

/// A counter on the number of slashes we have performed
#[pallet::storage]
#[pallet::getter(fn next_slash_id)]
pub type NextSlashId<T: Config> = StorageValue<_, T::SlashId, ValueQuery>;
Expand All @@ -155,6 +158,7 @@ pub mod pallet {

#[pallet::call]
impl<T: Config> Pallet<T> {
/// Cancel a slash that was deferred for a later era
#[pallet::call_index(0)]
#[pallet::weight(T::WeightInfo::cancel_deferred_slash(slash_indices.len() as u32))]
pub fn cancel_deferred_slash(
Expand All @@ -166,6 +170,7 @@ pub mod pallet {

let active_era = T::EraIndexProvider::active_era().index;

// We need to be in the defer period
ensure!(
era <= active_era
.saturating_add(T::SlashDeferDuration::get().saturating_add(One::one()))
Expand All @@ -178,7 +183,7 @@ pub mod pallet {
is_sorted_and_unique(&slash_indices),
Error::<T>::NotSortedAndUnique
);

// fetch slashes for the era in which we want to defer
let mut era_slashes = Slashes::<T>::get(&era);

let last_item = slash_indices[slash_indices.len() - 1];
Expand All @@ -192,6 +197,7 @@ pub mod pallet {
era_slashes.remove(index);
}

// insert back slashes
Slashes::<T>::insert(&era, &era_slashes);
Ok(())
}
Expand All @@ -211,26 +217,33 @@ pub mod pallet {

let slash_defer_duration = T::SlashDeferDuration::get();

let window_start = active_era.saturating_sub(T::BondingDuration::get());
ensure!(era >= window_start, Error::<T>::ProvidedNonSlashableEra);
let _ = T::EraIndexProvider::era_to_session_start(era)
.ok_or(Error::<T>::ProvidedNonSlashableEra)?;

let next_slash_id = NextSlashId::<T>::get();

let slash = Slash::<T::AccountId, T::SlashId> {
slash_id: next_slash_id,
reporters: vec![],
confirmed: false,

validator,
let slash = compute_slash::<T>(
percentage,
next_slash_id,
era,
validator,
slash_defer_duration,
)
.ok_or(Error::<T>::ErrorComputingSlash)?;

// If we defer duration is 0, we immediately apply and confirm
let era_to_consider = if slash_defer_duration == 0 {
era
} else {
era.saturating_add(slash_defer_duration)
.saturating_add(One::one())
};
let mut era_slashes = Slashes::<T>::get(&era);

let mut era_slashes = Slashes::<T>::get(&era_to_consider);
era_slashes.push(slash);
Slashes::<T>::insert(
&era.saturating_add(slash_defer_duration)
.saturating_add(One::one()),
&era_slashes,
);

Slashes::<T>::insert(era_to_consider, &era_slashes);

NextSlashId::<T>::put(next_slash_id.saturating_add(One::one()));
Ok(())
}
Expand Down Expand Up @@ -337,12 +350,17 @@ where
slash_era + slash_defer_duration + 1,
);

Slashes::<T>::mutate(
slash_era
.saturating_add(slash_defer_duration)
.saturating_add(One::one()),
move |for_later| for_later.push(slash),
);
// Cover slash defer duration equal to 0
if slash_defer_duration == 0 {
Slashes::<T>::mutate(slash_era, move |for_now| for_now.push(slash));
} else {
Slashes::<T>::mutate(
slash_era
.saturating_add(slash_defer_duration)
.saturating_add(One::one()),
move |for_later| for_later.push(slash),
);
}

// Fix unwrap
next_slash_id = next_slash_id.saturating_add(One::one());
Expand Down
27 changes: 23 additions & 4 deletions pallets/external-validator-slashes/src/mock.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use {
crate as external_validator_slashes,
frame_support::{
parameter_types,
traits::{ConstU16, ConstU64},
traits::{ConstU16, ConstU64, Get},
},
frame_system as system,
sp_core::H256,
Expand Down Expand Up @@ -114,6 +114,7 @@ pub struct MockEraIndexProvider;

thread_local! {
pub static ERA_INDEX: RefCell<EraIndex> = const { RefCell::new(0) };
pub static DEFER_PERIOD: RefCell<EraIndex> = const { RefCell::new(2) };
}

impl MockEraIndexProvider {
Expand All @@ -130,7 +131,13 @@ impl EraIndexProvider for MockEraIndexProvider {
}
}
fn era_to_session_start(era_index: EraIndex) -> Option<SessionIndex> {
Some(era_index.into())
let active_era = Self::active_era().index;
if era_index > active_era || era_index < active_era.saturating_sub(BondingDuration::get()) {
None
} else {
// Else we assume eras start at the same time as sessions
Some(era_index.into())
}
}
}

Expand Down Expand Up @@ -176,16 +183,28 @@ impl InvulnerablesProvider<u64> for MockInvulnerableProvider {
}
}

pub struct DeferPeriodGetter;
impl Get<EraIndex> for DeferPeriodGetter {
fn get() -> EraIndex {
DEFER_PERIOD.with(|q| (*q.borrow()).clone())
}
}

impl DeferPeriodGetter {
pub fn with_defer_period(defer_period: EraIndex) {
DEFER_PERIOD.with(|r| *r.borrow_mut() = defer_period);
}
}

parameter_types! {
pub const DeferPeriod: u32 = 2u32;
pub const BondingDuration: u32 = 5u32;
}

impl external_validator_slashes::Config for Test {
type RuntimeEvent = RuntimeEvent;
type ValidatorId = <Self as frame_system::Config>::AccountId;
type ValidatorIdOf = IdentityValidator;
type SlashDeferDuration = DeferPeriod;
type SlashDeferDuration = DeferPeriodGetter;
type BondingDuration = BondingDuration;
type SlashId = u32;
type SessionInterface = ();
Expand Down
77 changes: 75 additions & 2 deletions pallets/external-validator-slashes/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ fn test_on_offence_injects_offences() {
);
// current era (1) + defer period + 1
let slash_era = 0
.saturating_add(crate::mock::DeferPeriod::get())
.saturating_add(crate::mock::DeferPeriodGetter::get())
.saturating_add(One::one());

assert_eq!(
Expand Down Expand Up @@ -202,13 +202,86 @@ fn test_on_offence_does_not_work_for_invulnerables() {
);
// current era (1) + defer period + 1
let slash_era = 1
.saturating_add(crate::mock::DeferPeriod::get())
.saturating_add(crate::mock::DeferPeriodGetter::get())
.saturating_add(One::one());

assert_eq!(Slashes::<Test>::get(slash_era), vec![]);
});
}

#[test]
fn defer_period_of_zero_confirms_immediately_slashes() {
new_test_ext().execute_with(|| {
crate::mock::DeferPeriodGetter::with_defer_period(0);
start_era(0, 0);
assert_ok!(ExternalValidatorSlashes::force_inject_slash(
RuntimeOrigin::root(),
0,
1u64,
Perbill::from_percent(75)
));
assert_eq!(
Slashes::<Test>::get(0),
vec![Slash {
validator: 1,
percentage: Perbill::from_percent(75),
confirmed: true,
reporters: vec![],
slash_id: 0
}]
);
});
}

#[test]
fn we_cannot_cancel_anything_with_defer_period_zero() {
new_test_ext().execute_with(|| {
crate::mock::DeferPeriodGetter::with_defer_period(0);
start_era(0, 0);
assert_ok!(ExternalValidatorSlashes::force_inject_slash(
RuntimeOrigin::root(),
0,
1u64,
Perbill::from_percent(75)
));
assert_noop!(
ExternalValidatorSlashes::cancel_deferred_slash(RuntimeOrigin::root(), 0, vec![0]),
Error::<Test>::DeferPeriodIsOver
);
});
}

#[test]
fn test_on_offence_defer_period_0() {
new_test_ext().execute_with(|| {
crate::mock::DeferPeriodGetter::with_defer_period(0);
start_era(0, 0);
start_era(1, 1);
Pallet::<Test>::on_offence(
&[OffenceDetails {
// 1 and 2 are invulnerables
offender: (3, ()),
reporters: vec![],
}],
&[Perbill::from_percent(75)],
0,
);

let slash_era = 0;

assert_eq!(
Slashes::<Test>::get(slash_era),
vec![Slash {
validator: 3,
percentage: Perbill::from_percent(75),
confirmed: true,
reporters: vec![],
slash_id: 0
}]
);
});
}

fn start_era(era_index: EraIndex, session_index: SessionIndex) {
Pallet::<Test>::on_era_start(era_index, session_index);
crate::mock::MockEraIndexProvider::with_era(era_index);
Expand Down

0 comments on commit 1b2ff17

Please sign in to comment.