diff --git a/substrate/frame/collective/src/migrations/v5.rs b/substrate/frame/collective/src/migrations/v5.rs index abffba27ab528..066ef6a7bf0e2 100644 --- a/substrate/frame/collective/src/migrations/v5.rs +++ b/substrate/frame/collective/src/migrations/v5.rs @@ -19,8 +19,8 @@ use crate::{Config, Pallet}; use codec::{Decode, Encode}; use frame_support::{ pallet_prelude::{OptionQuery, StorageVersion, TypeInfo, ValueQuery}, - sp_runtime::{RuntimeDebug, Saturating}, - traits::{Get, Hash as PreimageHash, QueryPreimage, StorePreimage}, + sp_runtime::RuntimeDebug, + traits::{Get, Hash as PreimageHash, OnRuntimeUpgrade, QueryPreimage, StorePreimage}, weights::Weight, BoundedVec, Identity, }; @@ -71,55 +71,130 @@ pub struct OldVotes { pub end: BlockNumber, } -pub fn migrate, I: 'static>() -> Weight { - let storage_version = StorageVersion::get::>(); - log::info!( - target: "runtime::collective", - "Running migration for collective with storage version {:?}", - storage_version, - ); - - if storage_version <= 4 { - let mut count = 0u64; - - for (hash, proposal) in ProposalOf::::drain() { - let new_hash = >::Preimages::note(proposal.encode().into()).unwrap(); - assert_eq!(new_hash, PreimageHash::from_slice(hash.as_ref())); - count.saturating_inc(); - assert!(>::Preimages::is_requested(&new_hash)); +pub struct MigrateToV5(sp_std::marker::PhantomData<(T, I)>); +impl, I: 'static> OnRuntimeUpgrade for MigrateToV5 { + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { + log::info!("pre-migration collective v5"); + frame_support::ensure!( + StorageVersion::get::>() < 5, + "collective already migrated" + ); + let count = ProposalOf::::iter().count(); + if Proposals::::get().len() != count { + log::info!("collective proposals count inconsistency"); + } + if Members::::get().len() > >::MaxMembers::get() as usize { + log::info!("collective members exceeds MaxMembers"); } - Proposals::::kill(); + Ok((count as u32).encode()) + } + + fn on_runtime_upgrade() -> Weight { + let storage_version = StorageVersion::get::>(); + log::info!( + target: "runtime::collective", + "Running migration for collective with storage version {:?}", + storage_version, + ); + + if storage_version <= 4 { + let mut weight = T::DbWeight::get().reads(1); + + // ProposalOf + for (hash, proposal) in ProposalOf::::drain() { + weight = weight.saturating_add(T::DbWeight::get().reads_writes(1, 1)); + match >::Preimages::note(proposal.encode().into()) { + Ok(new_hash) => { + weight = weight.saturating_add(T::DbWeight::get().writes(1)); + if new_hash != PreimageHash::from_slice(hash.as_ref()) { + log::info!( + target: "runtime::collective", + "Preimage hash mismatch for proposal {:?}. Expected {:?}, got {:?}", + hash, + new_hash, + PreimageHash::from_slice(hash.as_ref()) + ); + } + if !>::Preimages::is_requested(&new_hash) { + log::info!( + target: "runtime::collective", + "Preimage for proposal {:?} was not requested", + hash, + ); + } + }, + Err(_) => { + log::info!( + target: "runtime::collective", + "Failed to note preimage for proposal {:?}", + hash, + ); + }, + } + } + + // Proposals + Proposals::::kill(); + weight = weight.saturating_add(T::DbWeight::get().writes(1)); - crate::Voting::::translate::>, _>( - |_, vote| { - count.saturating_inc(); - Some( + // Voting + for (hash, vote) in Voting::::drain() { + weight = weight.saturating_add(T::DbWeight::get().reads_writes(1, 2)); + crate::Voting::::insert( + PreimageHash::from_slice(hash.as_ref()), crate::Votes::, >::MaxMembers> { index: vote.index, threshold: vote.threshold, - ayes: vote - .ayes - .try_into() - .expect("runtime::collective migration failed, ayes overflow"), - nays: vote - .nays - .try_into() - .expect("runtime::collective migration failed, nays overflow"), + // the following operations are safe since the bound was previously enforced + // by runtime code + ayes: BoundedVec::truncate_from(vote.ayes), + nays: BoundedVec::truncate_from(vote.nays), end: vote.end, }, - ) - }, - ); + ); + } - StorageVersion::new(5).put::>(); - T::DbWeight::get().reads_writes(count, count + 2) - } else { - log::warn!( - target: "runtime::collective", - "Attempted to apply migration to V5 but failed because storage version is {:?}", - storage_version, + // Members + crate::Members::::put(BoundedVec::truncate_from(Members::::get())); + weight = weight.saturating_add(T::DbWeight::get().reads_writes(1, 1)); + + // StorageVersion + StorageVersion::new(5).put::>(); + weight = weight.saturating_add(T::DbWeight::get().writes(1)); + + weight + } else { + log::info!( + target: "runtime::collective", + "Attempted to apply migration to V5 but failed because storage version is {:?}", + storage_version, + ); + T::DbWeight::get().reads(1) + } + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { + use frame_support::ensure; + log::info!("post-migration collective v5"); + ensure!(StorageVersion::get::>() == 5, "collective migration to v5 failed"); + + match u32::decode(&mut state.as_slice()) { + Ok(count) => ensure!( + crate::Voting::::count() == count, + "collective new proposal count should equal the old count", + ), + Err(_) => return Err("the state parameter is not correct".into()), + } + + ensure!( + ProposalOf::::iter().count() == 0, + "collective v4 ProposalOf should be empty" ); - Weight::zero() + ensure!(Proposals::::get().len() == 0, "collective v4 Proposal should be empty"); + + Pallet::::do_try_state() } } diff --git a/substrate/frame/collective/src/tests.rs b/substrate/frame/collective/src/tests.rs index 8a002819e7c33..c25a6ff44884a 100644 --- a/substrate/frame/collective/src/tests.rs +++ b/substrate/frame/collective/src/tests.rs @@ -1544,10 +1544,12 @@ fn migration_v4() { }); } +#[cfg(all(feature = "try-runtime", test))] #[test] fn migration_v5() { ExtBuilder::default().build_and_execute(|| { - StorageVersion::new(4).put::(); + use frame_support::traits::OnRuntimeUpgrade; + StorageVersion::new(4).put::>(); // Setup the members. let members: Vec<::AccountId> = @@ -1562,6 +1564,7 @@ fn migration_v5() { proposal_hash ]); crate::migrations::v5::ProposalOf::::insert(proposal_hash, proposal); + ProposalCount::::put(1); let vote = crate::migrations::v5::OldVotes { index: 0, @@ -1575,9 +1578,7 @@ fn migration_v5() { crate::migrations::v5::Voting::::insert(proposal_hash, vote.clone()); // Run migration. - crate::migrations::v5::migrate::(); - // Check that the storage version is updated. - assert_eq!(StorageVersion::get::>(), 5); + assert_ok!(crate::migrations::v5::MigrateToV5::::try_on_runtime_upgrade(true)); // Check that the vote is present and bounded assert_eq!(