Skip to content

Commit

Permalink
chore: Prune historical values in Validator pallet (#5292)
Browse files Browse the repository at this point in the history
* Add first attempt at pruning.

This includes:
 - Implemented for AuthorityIndex, HistoricalAuthorities, HistoricalBonds
 - Deleting values on epoch expiry
 - Migration for deleting values for *all* previously expired epochs
 - Tests for migration and epoch expiry

* Remove comments (noted in review).
  • Loading branch information
MxmUrw authored Oct 18, 2024
1 parent d9253e1 commit dee3275
Show file tree
Hide file tree
Showing 4 changed files with 166 additions and 3 deletions.
9 changes: 8 additions & 1 deletion state-chain/pallets/cf-validator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ pub enum PalletConfigUpdate {
type RuntimeRotationState<T> =
RotationState<<T as Chainflip>::ValidatorId, <T as Chainflip>::Amount>;

pub const PALLET_VERSION: StorageVersion = StorageVersion::new(3);
pub const PALLET_VERSION: StorageVersion = StorageVersion::new(4);

// Might be better to add the enum inside a struct rather than struct inside enum
#[derive(Clone, PartialEq, Eq, Default, Encode, Decode, TypeInfo, RuntimeDebugNoBound)]
Expand Down Expand Up @@ -1024,6 +1024,13 @@ impl<T: Config> Pallet<T> {
T::Bonder::update_bond(authority, EpochHistory::<T>::active_bond(authority));
}
T::EpochTransitionHandler::on_expired_epoch(epoch);

let validators = HistoricalAuthorities::<T>::take(epoch);
for validator in validators {
AuthorityIndex::<T>::remove(epoch, validator);
}
HistoricalBonds::<T>::remove(epoch);

T::ValidatorWeightInfo::expire_epoch(num_expired_authorities)
}

Expand Down
9 changes: 7 additions & 2 deletions state-chain/pallets/cf-validator/src/migrations.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
use crate::Pallet;
use cf_runtime_upgrade_utilities::PlaceholderMigration;
use cf_runtime_upgrade_utilities::{PlaceholderMigration, VersionedMigration};

pub type PalletMigration<T> = (PlaceholderMigration<Pallet<T>, 3>,);
mod delete_old_epoch_data;

pub type PalletMigration<T> = (
VersionedMigration<Pallet<T>, delete_old_epoch_data::Migration<T>, 3, 4>,
PlaceholderMigration<Pallet<T>, 4>,
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
use frame_support::{traits::OnRuntimeUpgrade, weights::Weight};

use crate::*;

pub struct Migration<T: Config>(PhantomData<T>);

impl<T: Config> OnRuntimeUpgrade for Migration<T> {
fn on_runtime_upgrade() -> Weight {
pub fn delete_all_old<Iter, Index, Item, Relevant, IterRes, Remove>(
iter: Iter,
remove: Remove,
relevant: Relevant,
) where
Iter: Fn() -> IterRes,
IterRes: Iterator<Item = Item>,
Relevant: Fn(Item) -> Option<Index>,
Remove: Fn(Index),
{
let mut old_indices = Vec::new();
for item in iter() {
if let Some(index) = relevant(item) {
old_indices.push(index);
}
}
for index in old_indices {
remove(index);
}
}

let epoch = LastExpiredEpoch::<T>::get();

delete_all_old(
HistoricalAuthorities::<T>::iter,
HistoricalAuthorities::<T>::remove,
|(e, _)| if e <= epoch { Some(e) } else { None },
);
delete_all_old(HistoricalBonds::<T>::iter, HistoricalBonds::<T>::remove, |(e, _)| {
if e <= epoch {
Some(e)
} else {
None
}
});
delete_all_old(
AuthorityIndex::<T>::iter,
|(e1, e2)| AuthorityIndex::<T>::remove(e1, e2),
|(e, e2, _)| if e <= epoch { Some((e, e2)) } else { None },
);

Weight::zero()
}

#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, DispatchError> {
Ok(vec![])
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(_state: Vec<u8>) -> Result<(), DispatchError> {
let epoch = LastExpiredEpoch::<T>::get();

assert!(!HistoricalAuthorities::<T>::iter().any(|(e, _)| e <= epoch));
assert!(!HistoricalBonds::<T>::iter().any(|(e, _)| e <= epoch));
assert!(!AuthorityIndex::<T>::iter().any(|(e, _, _)| e <= epoch));

Ok(())
}
}

#[cfg(test)]
mod migration_tests {
#[test]
fn test_migration() {
use super::*;
use crate::mock::*;

new_test_ext().execute_with(|| {
let last_expired_epoch = 1000;
LastExpiredEpoch::<Test>::set(last_expired_epoch);

// create some test values
HistoricalAuthorities::<Test>::set(last_expired_epoch - 2, vec![1, 2, 3]);
HistoricalAuthorities::<Test>::set(last_expired_epoch - 1, vec![4, 5]);
HistoricalAuthorities::<Test>::set(last_expired_epoch, vec![6, 7, 8, 9]);
HistoricalAuthorities::<Test>::set(last_expired_epoch + 1, vec![10, 11]);

HistoricalBonds::<Test>::set(last_expired_epoch - 2, 100);
HistoricalBonds::<Test>::set(last_expired_epoch - 1, 101);
HistoricalBonds::<Test>::set(last_expired_epoch, 102);
HistoricalBonds::<Test>::set(last_expired_epoch + 1, 103);

AuthorityIndex::<Test>::set(last_expired_epoch - 2, 1, Some(1));
AuthorityIndex::<Test>::set(last_expired_epoch - 2, 2, Some(2));
AuthorityIndex::<Test>::set(last_expired_epoch - 2, 3, Some(3));
AuthorityIndex::<Test>::set(last_expired_epoch - 1, 1, Some(1));
AuthorityIndex::<Test>::set(last_expired_epoch - 1, 2, Some(2));
AuthorityIndex::<Test>::set(last_expired_epoch, 3, Some(1));
AuthorityIndex::<Test>::set(last_expired_epoch, 1, Some(2));
AuthorityIndex::<Test>::set(last_expired_epoch + 1, 2, Some(1));
AuthorityIndex::<Test>::set(last_expired_epoch + 2, 3, Some(2));

#[cfg(feature = "try-runtime")]
let state = super::Migration::<Test>::pre_upgrade().unwrap();

// Perform runtime migration.
super::Migration::<Test>::on_runtime_upgrade();

#[cfg(feature = "try-runtime")]
super::Migration::<Test>::post_upgrade(state).unwrap();

// ensure that data which is not expired is kept
assert_eq!(HistoricalAuthorities::<Test>::get(last_expired_epoch + 1), vec![10, 11]);
assert_eq!(HistoricalBonds::<Test>::get(last_expired_epoch + 1), 103);
assert_eq!(AuthorityIndex::<Test>::get(last_expired_epoch + 1, 2), Some(1));
assert_eq!(AuthorityIndex::<Test>::get(last_expired_epoch + 2, 3), Some(2));
});
}
}
33 changes: 33 additions & 0 deletions state-chain/pallets/cf-validator/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,39 @@ fn historical_epochs() {
});
}

#[test]
fn expired_epoch_data_is_removed() {
new_test_ext().then_execute_with_checks(|| {
// Epoch 1
EpochHistory::<Test>::activate_epoch(&ALICE, 1);
HistoricalAuthorities::<Test>::insert(1, Vec::from([ALICE]));
HistoricalBonds::<Test>::insert(1, 10);
// Epoch 2
EpochHistory::<Test>::activate_epoch(&ALICE, 2);
HistoricalAuthorities::<Test>::insert(2, Vec::from([ALICE]));
HistoricalBonds::<Test>::insert(2, 30);
let authority_index = AuthorityIndex::<Test>::get(2, ALICE);

// Expire
ValidatorPallet::expire_epoch(1);

// Epoch 3
EpochHistory::<Test>::activate_epoch(&ALICE, 3);
HistoricalAuthorities::<Test>::insert(3, Vec::from([ALICE]));
HistoricalBonds::<Test>::insert(3, 20);

// Expect epoch 1's data to be deleted
assert!(AuthorityIndex::<Test>::try_get(1, ALICE).is_err());
assert!(HistoricalAuthorities::<Test>::try_get(1).is_err());
assert!(HistoricalBonds::<Test>::try_get(1).is_err());

// Expect epoch 2's data to be exist
assert_eq!(AuthorityIndex::<Test>::get(2, ALICE), authority_index);
assert_eq!(HistoricalAuthorities::<Test>::get(2), vec![ALICE]);
assert_eq!(HistoricalBonds::<Test>::get(2), 30);
});
}

#[test]
fn highest_bond() {
new_test_ext().then_execute_with_checks(|| {
Expand Down

0 comments on commit dee3275

Please sign in to comment.