Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Prune historical values in Validator pallet #5292

Merged
merged 2 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion state-chain/pallets/cf-validator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,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 @@ -1029,6 +1029,17 @@ impl<T: Config> Pallet<T> {
T::Bonder::update_bond(authority, EpochHistory::<T>::active_bond(authority));
}
T::EpochTransitionHandler::on_expired_epoch(epoch);

// WIP: delete left-over data regarding this old epoch
// get all validators for old epoch
let validators = HistoricalAuthorities::<T>::take(epoch);
for validator in validators {
AuthorityIndex::<T>::remove(epoch, validator);
}
HistoricalBonds::<T>::remove(epoch);
// HistoricalActiveEpochs::<T>::remove(epoch);
// EpochExpiries::<T>::remove()
MxmUrw marked this conversation as resolved.
Show resolved Hide resolved

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
Loading