Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Storage migration to remove founder role #12766

Merged
Show file tree
Hide file tree
Changes from 2 commits
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
3 changes: 1 addition & 2 deletions frame/alliance/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,8 +301,7 @@ pub mod pallet {
#[pallet::constant]
type MaxAnnouncementsCount: Get<u32>;

/// The maximum number of members per member role. Should not exceed the sum of
/// `MaxFellows` and `MaxAllies`.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is used to be MaxFellows and MaxFounders, now its just per role

/// The maximum number of members per member role.
#[pallet::constant]
type MaxMembersCount: Get<u32>;

Expand Down
101 changes: 100 additions & 1 deletion frame/alliance/src/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use frame_support::{pallet_prelude::*, storage::migration, traits::OnRuntimeUpgr
use log;

/// The current storage version.
pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);
pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(2);

/// Wrapper for all migrations of this pallet.
pub fn migrate<T: Config<I>, I: 'static>() -> Weight {
Expand All @@ -31,6 +31,10 @@ pub fn migrate<T: Config<I>, I: 'static>() -> Weight {
weight = weight.saturating_add(v0_to_v1::migrate::<T, I>());
}

if onchain_version < 2 {
weight = weight.saturating_add(v1_to_v2::migrate::<T, I>());
}

STORAGE_VERSION.put::<Pallet<T, I>>();
weight = weight.saturating_add(T::DbWeight::get().writes(1));

Expand Down Expand Up @@ -77,3 +81,98 @@ mod v0_to_v1 {
T::DbWeight::get().writes(res.unique.into())
}
}

/// v1_to_v2: `Members` storage map collapses `Founder` and `Fellow` keys into one `Fellow`.
/// Total number of `Founder`s and `Fellow`s must not be higher than `T::MaxMembersCount`.
pub(crate) mod v1_to_v2 {
use super::*;
use crate::{MemberRole, Members};

/// V1 Role set.
#[derive(Copy, Clone, PartialEq, Eq, Encode, Decode, TypeInfo, MaxEncodedLen)]
pub enum MemberRoleV1 {
Founder,
Fellow,
Ally,
Retiring,
}

pub fn migrate<T: Config<I>, I: 'static>() -> Weight {
log::info!(target: LOG_TARGET, "Running migration v1_to_v2: `Members` storage map collapses `Founder` and `Fellow` keys into one `Fellow`.");
// fetch into the scope all members.
let founders_vec = take_members::<T, I>(MemberRoleV1::Founder).into_inner();
let mut fellows_vec = take_members::<T, I>(MemberRoleV1::Fellow).into_inner();
let allies = take_members::<T, I>(MemberRoleV1::Ally);
let retiring = take_members::<T, I>(MemberRoleV1::Retiring);
Comment on lines +105 to +106
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any need to include these?

if founders_vec
.len()
.saturating_add(fellows_vec.len())
.saturating_add(allies.len())
.saturating_add(retiring.len()) ==
0
{
return T::DbWeight::get().reads(4)
}
log::info!(
target: LOG_TARGET,
"Members storage v1 contains, '{}' founders, '{}' fellows, '{}' allies, '{}' retiring members.",
founders_vec.len(),
fellows_vec.len(),
allies.len(),
retiring.len(),
);
// merge founders with fellows and sort.
fellows_vec.extend(founders_vec);
fellows_vec.sort();
let fellows: BoundedVec<T::AccountId, T::MaxMembersCount> =
fellows_vec.try_into().unwrap_or_else(|_| {
log::error!(
target: LOG_TARGET,
"Merged Founders and Fellows do not fit into `T::MaxMembersCount` bound."
);
BoundedVec::default()
});
// insert members with new storage map key.
Members::<T, I>::insert(&MemberRole::Fellow, fellows.clone());
Members::<T, I>::insert(&MemberRole::Ally, allies.clone());
Members::<T, I>::insert(&MemberRole::Retiring, retiring.clone());
log::info!(
target: LOG_TARGET,
"Members storage updated with, '{}' fellows, '{}' allies, '{}' retiring members.",
fellows.len(),
allies.len(),
retiring.len(),
);
T::DbWeight::get().reads_writes(4, 4)
}

fn take_members<T: Config<I>, I: 'static>(
role: MemberRoleV1,
) -> BoundedVec<T::AccountId, T::MaxMembersCount> {
migration::take_storage_item::<
MemberRoleV1,
BoundedVec<T::AccountId, T::MaxMembersCount>,
Twox64Concat,
>(<Pallet<T, I>>::name().as_bytes(), b"Members", role)
.unwrap_or_default()
}
}

#[cfg(test)]
mod test {
use super::*;
use crate::{mock::*, MemberRole};

#[test]
fn migration_v1_to_v2_works() {
new_test_ext().execute_with(|| {
assert_ok!(Alliance::join_alliance(RuntimeOrigin::signed(4)));
assert_eq!(Alliance::members(MemberRole::Ally), vec![4]);
assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2, 3]);
v1_to_v2::migrate::<Test, ()>();
assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2, 3, 4]);
assert_eq!(Alliance::members(MemberRole::Ally), vec![]);
assert_eq!(Alliance::members(MemberRole::Retiring), vec![]);
});
}
}