diff --git a/frame/offences/src/lib.rs b/frame/offences/src/lib.rs index 96c7c119c2038..1c7ffeca71983 100644 --- a/frame/offences/src/lib.rs +++ b/frame/offences/src/lib.rs @@ -26,7 +26,9 @@ pub mod migration; mod mock; mod tests; -use codec::{Decode, Encode}; +use core::marker::PhantomData; + +use codec::Encode; use frame_support::weights::Weight; use sp_runtime::{traits::Hash, Perbill}; use sp_staking::{ @@ -43,12 +45,17 @@ type OpaqueTimeSlot = Vec; /// A type alias for a report identifier. type ReportIdOf = ::Hash; +const LOG_TARGET: &str = "runtime::offences"; + #[frame_support::pallet] pub mod pallet { use super::*; use frame_support::pallet_prelude::*; + const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); + #[pallet::pallet] + #[pallet::storage_version(STORAGE_VERSION)] #[pallet::without_storage_info] pub struct Pallet(_); @@ -85,21 +92,6 @@ pub mod pallet { ValueQuery, >; - /// Enumerates all reports of a kind along with the time they happened. - /// - /// All reports are sorted by the time of offence. - /// - /// Note that the actual type of this mapping is `Vec`, this is because values of - /// different types are not supported at the moment so we are doing the manual serialization. - #[pallet::storage] - pub type ReportsByKindIndex = StorageMap< - _, - Twox64Concat, - Kind, - Vec, // (O::TimeSlot, ReportIdOf) - ValueQuery, - >; - /// Events type. #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] @@ -190,7 +182,7 @@ impl Pallet { OffenceDetails { offender, reporters: reporters.clone() }, ); - storage.insert(time_slot, report_id); + storage.insert(report_id); } } @@ -225,7 +217,7 @@ struct TriageOutcome { struct ReportIndexStorage> { opaque_time_slot: OpaqueTimeSlot, concurrent_reports: Vec>, - same_kind_reports: Vec<(O::TimeSlot, ReportIdOf)>, + _phantom: PhantomData, } impl> ReportIndexStorage { @@ -233,30 +225,19 @@ impl> ReportIndexStorage { fn load(time_slot: &O::TimeSlot) -> Self { let opaque_time_slot = time_slot.encode(); - let same_kind_reports = ReportsByKindIndex::::get(&O::ID); - let same_kind_reports = - Vec::<(O::TimeSlot, ReportIdOf)>::decode(&mut &same_kind_reports[..]) - .unwrap_or_default(); - let concurrent_reports = >::get(&O::ID, &opaque_time_slot); - Self { opaque_time_slot, concurrent_reports, same_kind_reports } + Self { opaque_time_slot, concurrent_reports, _phantom: Default::default() } } /// Insert a new report to the index. - fn insert(&mut self, time_slot: &O::TimeSlot, report_id: ReportIdOf) { - // Insert the report id into the list while maintaining the ordering by the time - // slot. - let pos = self.same_kind_reports.partition_point(|(when, _)| when <= time_slot); - self.same_kind_reports.insert(pos, (time_slot.clone(), report_id)); - + fn insert(&mut self, report_id: ReportIdOf) { // Update the list of concurrent reports. self.concurrent_reports.push(report_id); } /// Dump the indexes to the storage. fn save(self) { - ReportsByKindIndex::::insert(&O::ID, self.same_kind_reports.encode()); >::insert( &O::ID, &self.opaque_time_slot, diff --git a/frame/offences/src/migration.rs b/frame/offences/src/migration.rs index 95b94ba87cc9c..07bd68407d378 100644 --- a/frame/offences/src/migration.rs +++ b/frame/offences/src/migration.rs @@ -15,11 +15,84 @@ // See the License for the specific language governing permissions and // limitations under the License. -use super::{Config, OffenceDetails, Perbill, SessionIndex}; -use frame_support::{pallet_prelude::ValueQuery, storage_alias, traits::Get, weights::Weight}; +use super::{Config, Kind, OffenceDetails, Pallet, Perbill, SessionIndex, LOG_TARGET}; +use frame_support::{ + dispatch::GetStorageVersion, + pallet_prelude::ValueQuery, + storage_alias, + traits::{Get, OnRuntimeUpgrade}, + weights::Weight, + Twox64Concat, +}; use sp_staking::offence::{DisableStrategy, OnOffenceHandler}; use sp_std::vec::Vec; +#[cfg(feature = "try-runtime")] +use frame_support::ensure; + +mod v0 { + use super::*; + + #[storage_alias] + pub type ReportsByKindIndex = StorageMap< + Pallet, + Twox64Concat, + Kind, + Vec, // (O::TimeSlot, ReportIdOf) + ValueQuery, + >; +} + +pub mod v1 { + use frame_support::traits::StorageVersion; + + use super::*; + + pub struct MigrateToV1(sp_std::marker::PhantomData); + impl OnRuntimeUpgrade for MigrateToV1 { + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, &'static str> { + let onchain = Pallet::::on_chain_storage_version(); + ensure!(onchain < 1, "pallet_offences::MigrateToV1 migration can be deleted"); + + log::info!( + target: LOG_TARGET, + "Number of reports to refund and delete: {}", + v0::ReportsByKindIndex::::iter_keys().count() + ); + + Ok(Vec::new()) + } + + fn on_runtime_upgrade() -> Weight { + let onchain = Pallet::::on_chain_storage_version(); + + if onchain > 0 { + log::info!(target: LOG_TARGET, "pallet_offences::MigrateToV1 should be removed"); + return T::DbWeight::get().reads(1) + } + + let keys_removed = v0::ReportsByKindIndex::::clear(u32::MAX, None).unique as u64; + let weight = T::DbWeight::get().reads_writes(keys_removed, keys_removed); + + StorageVersion::new(1).put::>(); + + weight + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(_state: Vec) -> Result<(), &'static str> { + let onchain = Pallet::::on_chain_storage_version(); + ensure!(onchain == 1, "pallet_offences::MigrateToV1 needs to be run"); + ensure!( + v0::ReportsByKindIndex::::iter_keys().count() == 0, + "there are some dangling reports that need to be destroyed and refunded" + ); + Ok(()) + } + } +} + /// Type of data stored as a deferred offence type DeferredOffenceOf = ( Vec::AccountId, ::IdentificationTuple>>, @@ -36,7 +109,7 @@ type DeferredOffences = pub fn remove_deferred_storage() -> Weight { let mut weight = T::DbWeight::get().reads_writes(1, 1); let deferred = >::take(); - log::info!(target: "runtime::offences", "have {} deferred offences, applying.", deferred.len()); + log::info!(target: LOG_TARGET, "have {} deferred offences, applying.", deferred.len()); for (offences, perbill, session) in deferred.iter() { let consumed = T::OnOffenceHandler::on_offence( offences, @@ -53,10 +126,32 @@ pub fn remove_deferred_storage() -> Weight { #[cfg(test)] mod test { use super::*; - use crate::mock::{new_test_ext, with_on_offence_fractions, Runtime as T}; + use crate::mock::{new_test_ext, with_on_offence_fractions, Runtime as T, KIND}; + use codec::Encode; use sp_runtime::Perbill; use sp_staking::offence::OffenceDetails; + #[test] + fn migration_to_v1_works() { + let mut ext = new_test_ext(); + + ext.execute_with(|| { + >::insert(KIND, 2u32.encode()); + assert!(>::iter_values().count() > 0); + }); + + ext.commit_all().unwrap(); + + ext.execute_with(|| { + assert_eq!( + v1::MigrateToV1::::on_runtime_upgrade(), + ::DbWeight::get().reads_writes(1, 1), + ); + + assert!(>::iter_values().count() == 0); + }) + } + #[test] fn should_resubmit_deferred_offences() { new_test_ext().execute_with(|| { diff --git a/frame/offences/src/mock.rs b/frame/offences/src/mock.rs index 27e1da8c4e636..17480be76c1d8 100644 --- a/frame/offences/src/mock.rs +++ b/frame/offences/src/mock.rs @@ -164,8 +164,3 @@ impl offence::Offence for Offence { Perbill::from_percent(5 + offenders_count * 100 / self.validator_set_count) } } - -/// Create the report id for the given `offender` and `time_slot` combination. -pub fn report_id(time_slot: u128, offender: u64) -> H256 { - Offences::report_id::(&time_slot, &offender) -} diff --git a/frame/offences/src/tests.rs b/frame/offences/src/tests.rs index 81f0f44f1b939..d525c7c3ab1d9 100644 --- a/frame/offences/src/tests.rs +++ b/frame/offences/src/tests.rs @@ -21,8 +21,8 @@ use super::*; use crate::mock::{ - new_test_ext, offence_reports, report_id, with_on_offence_fractions, Offence, Offences, - RuntimeEvent, System, KIND, + new_test_ext, offence_reports, with_on_offence_fractions, Offence, Offences, RuntimeEvent, + System, KIND, }; use frame_system::{EventRecord, Phase}; use sp_runtime::Perbill; @@ -245,48 +245,3 @@ fn should_properly_count_offences() { ); }); } - -/// We insert offences in sorted order using the time slot in the `same_kind_reports`. -/// This test ensures that it works as expected. -#[test] -fn should_properly_sort_offences() { - new_test_ext().execute_with(|| { - // given - let time_slot = 42; - assert_eq!(offence_reports(KIND, time_slot), vec![]); - - let offence1 = Offence { validator_set_count: 5, time_slot, offenders: vec![5] }; - let offence2 = Offence { validator_set_count: 5, time_slot, offenders: vec![4] }; - let offence3 = - Offence { validator_set_count: 5, time_slot: time_slot + 1, offenders: vec![6, 7] }; - let offence4 = - Offence { validator_set_count: 5, time_slot: time_slot - 1, offenders: vec![3] }; - Offences::report_offence(vec![], offence1).unwrap(); - with_on_offence_fractions(|f| { - assert_eq!(f.clone(), vec![Perbill::from_percent(25)]); - f.clear(); - }); - - // when - // report for the second time - Offences::report_offence(vec![], offence2).unwrap(); - Offences::report_offence(vec![], offence3).unwrap(); - Offences::report_offence(vec![], offence4).unwrap(); - - // then - let same_kind_reports = Vec::<(u128, sp_core::H256)>::decode( - &mut &crate::ReportsByKindIndex::::get(KIND)[..], - ) - .unwrap(); - assert_eq!( - same_kind_reports, - vec![ - (time_slot - 1, report_id(time_slot - 1, 3)), - (time_slot, report_id(time_slot, 5)), - (time_slot, report_id(time_slot, 4)), - (time_slot + 1, report_id(time_slot + 1, 6)), - (time_slot + 1, report_id(time_slot + 1, 7)), - ] - ); - }); -}