From 789d565bde29cb58836dda658def7490b1163924 Mon Sep 17 00:00:00 2001 From: Vivek Pandya Date: Mon, 18 Jul 2022 09:15:45 +0530 Subject: [PATCH] Use CountedStorageMap instead of StorageMap for Jurors in court pallet (#707) * Use CountedStorageMap instead of StorageMap for Jurors in court pallet * Add migration for Jurors in Court from StorageMap to CountedStorageMap * Remove storage migration for RemoveDisputesOfResolvedMarkets --- runtime/src/lib.rs | 2 +- zrml/court/src/lib.rs | 9 +- zrml/court/src/migrations.rs | 101 ++++++++++++ zrml/prediction-markets/src/lib.rs | 1 - zrml/prediction-markets/src/migrations.rs | 189 ---------------------- 5 files changed, 107 insertions(+), 195 deletions(-) create mode 100644 zrml/court/src/migrations.rs delete mode 100644 zrml/prediction-markets/src/migrations.rs diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 83041c854..dd97996db 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -77,7 +77,7 @@ type Executive = frame_executive::Executive< frame_system::ChainContext, Runtime, AllPalletsWithSystem, - zrml_prediction_markets::migrations::RemoveDisputesOfResolvedMarkets, + zrml_court::migrations::JurorsCountedStorageMapMigration, >; type Header = generic::Header; diff --git a/zrml/court/src/lib.rs b/zrml/court/src/lib.rs index eff237363..357ceabc2 100644 --- a/zrml/court/src/lib.rs +++ b/zrml/court/src/lib.rs @@ -12,6 +12,7 @@ mod benchmarks; mod court_pallet_api; mod juror; mod juror_status; +pub mod migrations; mod mock; mod tests; pub mod weights; @@ -32,7 +33,7 @@ mod pallet { use core::marker::PhantomData; use frame_support::{ dispatch::DispatchResult, - pallet_prelude::{StorageDoubleMap, StorageMap, StorageValue, ValueQuery}, + pallet_prelude::{CountedStorageMap, StorageDoubleMap, StorageValue, ValueQuery}, traits::{ BalanceStatus, Currency, Get, Hooks, IsType, NamedReservableCurrency, Randomness, StorageVersion, @@ -58,7 +59,7 @@ mod pallet { const INITIAL_JURORS_NUM: usize = 3; const MAX_RANDOM_JURORS: usize = 13; /// The current storage version. - const STORAGE_VERSION: StorageVersion = StorageVersion::new(0); + const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); // Weight used to increase the number of jurors for subsequent disputes // of the same market const SUBSEQUENT_JURORS_FACTOR: usize = 2; @@ -93,7 +94,7 @@ mod pallet { if Jurors::::get(&who).is_some() { return Err(Error::::JurorAlreadyExists.into()); } - let jurors_num = Jurors::::iter().count(); + let jurors_num = Jurors::::count() as usize; let jurors_num_plus_one = jurors_num.checked_add(1).ok_or(ArithmeticError::Overflow)?; let stake = Self::current_required_stake(jurors_num_plus_one); CurrencyOf::::reserve_named(&RESERVE_ID, &who, stake)?; @@ -525,7 +526,7 @@ mod pallet { /// Accounts that stake funds to decide outcomes. #[pallet::storage] - pub type Jurors = StorageMap<_, Blake2_128Concat, T::AccountId, Juror>; + pub type Jurors = CountedStorageMap<_, Blake2_128Concat, T::AccountId, Juror>; /// An extra layer of pseudo randomness. #[pallet::storage] diff --git a/zrml/court/src/migrations.rs b/zrml/court/src/migrations.rs new file mode 100644 index 000000000..c44d20e15 --- /dev/null +++ b/zrml/court/src/migrations.rs @@ -0,0 +1,101 @@ +use crate::{Config, Jurors, Pallet}; +use frame_support::{ + dispatch::Weight, + log, + pallet_prelude::PhantomData, + traits::{Get, OnRuntimeUpgrade, StorageVersion}, +}; + +const COURT_REQUIRED_STORAGE_VERSION: u16 = 0; +const COURT_NEXT_STORAGE_VERSION: u16 = 1; + +pub struct JurorsCountedStorageMapMigration(PhantomData); + +impl OnRuntimeUpgrade for JurorsCountedStorageMapMigration { + fn on_runtime_upgrade() -> Weight + where + T: Config, + { + let mut total_weight = T::DbWeight::get().reads(1); + + if StorageVersion::get::>() != COURT_REQUIRED_STORAGE_VERSION { + log::info!("Skipping storage migration of jurors of court already up to date"); + return total_weight; + } + log::info!("Starting storage migration of jurors of court"); + Jurors::::initialize_counter(); + total_weight = total_weight.saturating_add( + T::DbWeight::get().writes(Jurors::::count().saturating_add(1) as u64), + ); + + StorageVersion::new(COURT_NEXT_STORAGE_VERSION).put::>(); + total_weight = total_weight.saturating_add(T::DbWeight::get().writes(1)); + + log::info!("Completed storage migration of jurors of court"); + total_weight + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result<(), &'static str> { + Ok(()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade() -> Result<(), &'static str> { + let court_storage_version = StorageVersion::get::>(); + assert_eq!( + court_storage_version, COURT_NEXT_STORAGE_VERSION, + "found unexpected court pallet storage version. Found: {:?}. Expected: {:?}", + court_storage_version, COURT_NEXT_STORAGE_VERSION, + ); + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{mock::*, Juror}; + use frame_support::Hashable; + + #[test] + fn test_on_runtime_upgrade_on_untouched_chain() { + ExtBuilder::default().build().execute_with(|| { + setup_chain(); + JurorsCountedStorageMapMigration::::on_runtime_upgrade(); + }); + } + + #[test] + fn on_runtime_upgrade_updates_storage_versions() { + ExtBuilder::default().build().execute_with(|| { + setup_chain(); + JurorsCountedStorageMapMigration::::on_runtime_upgrade(); + assert_eq!(StorageVersion::get::>(), COURT_NEXT_STORAGE_VERSION,); + }); + } + + #[test] + fn test_on_runtime_upgrade() { + ExtBuilder::default().build().execute_with(|| { + setup_chain(); + let alice_hash = ALICE.blake2_128_concat(); + let alice_juror = Juror { status: crate::JurorStatus::Ok }; + frame_support::migration::put_storage_value( + b"Court", + b"Jurors", + &alice_hash, + alice_juror.clone(), + ); + assert_eq!(Jurors::::count(), 0_u32); + JurorsCountedStorageMapMigration::::on_runtime_upgrade(); + assert_eq!(Jurors::::count(), 1_u32); + assert_eq!(Jurors::::get(&ALICE), Some(alice_juror)); + }); + } + + fn setup_chain() { + StorageVersion::new(COURT_REQUIRED_STORAGE_VERSION).put::>(); + } +} diff --git a/zrml/prediction-markets/src/lib.rs b/zrml/prediction-markets/src/lib.rs index 248a227ac..d4f7f3224 100644 --- a/zrml/prediction-markets/src/lib.rs +++ b/zrml/prediction-markets/src/lib.rs @@ -67,7 +67,6 @@ extern crate alloc; mod benchmarks; -pub mod migrations; pub mod mock; mod tests; pub mod weights; diff --git a/zrml/prediction-markets/src/migrations.rs b/zrml/prediction-markets/src/migrations.rs deleted file mode 100644 index cc51a0123..000000000 --- a/zrml/prediction-markets/src/migrations.rs +++ /dev/null @@ -1,189 +0,0 @@ -use crate::{Config, Disputes, Pallet}; -use frame_support::{ - dispatch::Weight, - log, - pallet_prelude::PhantomData, - traits::{Get, OnRuntimeUpgrade, StorageVersion}, -}; -use zeitgeist_primitives::types::MarketStatus; -use zrml_market_commons::MarketCommonsPalletApi; - -const PREDICTION_MARKETS_REQUIRED_STORAGE_VERSION: u16 = 1; -const PREDICTION_MARKETS_NEXT_STORAGE_VERSION: u16 = 2; - -pub struct RemoveDisputesOfResolvedMarkets(PhantomData); - -impl OnRuntimeUpgrade for RemoveDisputesOfResolvedMarkets { - fn on_runtime_upgrade() -> Weight - where - T: Config, - { - let mut total_weight = T::DbWeight::get().reads(1); - - if StorageVersion::get::>() != PREDICTION_MARKETS_REQUIRED_STORAGE_VERSION { - log::info!( - "Skipping storage removal of disputes of resolved markets; prediction-markets \ - already up to date" - ); - return total_weight; - } - total_weight = total_weight.saturating_add(T::DbWeight::get().reads(1)); - log::info!("Starting storage migration of RemoveDisputesOfResolvedMarkets"); - - for (market_id, market) in T::MarketCommons::market_iter() { - total_weight = total_weight.saturating_add(T::DbWeight::get().reads(1)); - - if market.status == MarketStatus::Resolved { - Disputes::::remove(market_id); - total_weight = total_weight.saturating_add(T::DbWeight::get().writes(1)); - } - } - StorageVersion::new(PREDICTION_MARKETS_NEXT_STORAGE_VERSION).put::>(); - total_weight = total_weight.saturating_add(T::DbWeight::get().writes(1)); - - log::info!("Completed storage removal of RemoveDisputesOfResolvedMarkets"); - total_weight - } - - #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result<(), &'static str> { - Ok(()) - } - - #[cfg(feature = "try-runtime")] - fn post_upgrade() -> Result<(), &'static str> { - for (market_id, market) in T::MarketCommons::market_iter() { - if market.status == MarketStatus::Resolved { - let disputes = Disputes::::get(market_id); - assert_eq!(disputes.len(), 0); - } - } - - let prediction_markets_storage_version = StorageVersion::get::>(); - assert_eq!( - prediction_markets_storage_version, PREDICTION_MARKETS_NEXT_STORAGE_VERSION, - "found unexpected prediction-markets pallet storage version. Found: {:?}. Expected: \ - {:?}", - prediction_markets_storage_version, PREDICTION_MARKETS_NEXT_STORAGE_VERSION, - ); - - Ok(()) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::{mock::*, MomentOf}; - use frame_support::assert_ok; - use orml_traits::MultiCurrency; - use zeitgeist_primitives::{ - constants::{BASE, MILLISECS_PER_BLOCK}, - types::{ - Asset, BlockNumber, MarketCreation, MarketDispute, MarketDisputeMechanism, - MarketPeriod, MarketType, MultiHash, OutcomeReport, ScoringRule, - }, - }; - - #[test] - fn test_on_runtime_upgrade_on_untouched_chain() { - ExtBuilder::default().build().execute_with(|| { - setup_chain(); - RemoveDisputesOfResolvedMarkets::::on_runtime_upgrade(); - }); - } - - #[test] - fn on_runtime_upgrade_updates_storage_versions() { - ExtBuilder::default().build().execute_with(|| { - setup_chain(); - RemoveDisputesOfResolvedMarkets::::on_runtime_upgrade(); - assert_eq!( - StorageVersion::get::>(), - PREDICTION_MARKETS_NEXT_STORAGE_VERSION - ); - }); - } - - #[test] - fn test_on_runtime_upgrade_with_sample_markets() { - ExtBuilder::default().build().execute_with(|| { - setup_chain(); - let _ = AssetManager::deposit(Asset::Ztg, &ALICE, 1_000 * BASE); - - let short_time: MomentOf = (5 * MILLISECS_PER_BLOCK).into(); - - create_test_market( - MarketPeriod::Timestamp(0..short_time), - MarketCreation::Permissionless, - MarketStatus::Resolved, - ); - - create_test_market( - MarketPeriod::Timestamp(0..short_time), - MarketCreation::Permissionless, - MarketStatus::Resolved, - ); - - // Add one simple dispute for alreay resolved market to simulate a case - // where there is pendig dispute(s) for resolver market and that should - // be cleaned in storage migration. - let market_dispute = - MarketDispute { at: 1, by: CHARLIE, outcome: OutcomeReport::Categorical(0) }; - let _res = crate::Disputes::::try_mutate(0, |disputes| { - let _ = disputes.try_push(market_dispute.clone()); - disputes.try_push(market_dispute.clone()) - }); - let _res = crate::Disputes::::try_mutate(1, |disputes| { - let _ = disputes.try_push(market_dispute.clone()); - disputes.try_push(market_dispute) - }); - - let disputes = crate::Disputes::::get(&0); - assert_eq!(disputes.len(), 2); - - let disputes = crate::Disputes::::get(&1); - assert_eq!(disputes.len(), 2); - RemoveDisputesOfResolvedMarkets::::on_runtime_upgrade(); - - assert_eq!(MarketCommons::market(&0).unwrap().status, MarketStatus::Resolved); - let disputes = crate::Disputes::::get(&0); - assert_eq!(disputes.len(), 0); - let disputes = crate::Disputes::::get(&1); - assert_eq!(disputes.len(), 0); - }); - } - - fn setup_chain() { - StorageVersion::new(PREDICTION_MARKETS_REQUIRED_STORAGE_VERSION).put::>(); - } - - fn create_test_market( - period: MarketPeriod>, - market_creation: MarketCreation, - market_status: MarketStatus, - ) { - assert_ok!(PredictionMarkets::create_market( - Origin::signed(ALICE), - BOB, - period, - gen_metadata(0), - market_creation, - MarketType::Categorical(5), - MarketDisputeMechanism::Authorized(CHARLIE), - ScoringRule::CPMM, - )); - let market_id = MarketCommons::latest_market_id().unwrap(); - assert_ok!(MarketCommons::mutate_market(&market_id, |market| { - market.status = market_status; - Ok(()) - })); - } - - fn gen_metadata(byte: u8) -> MultiHash { - let mut metadata = [byte; 50]; - metadata[0] = 0x15; - metadata[1] = 0x30; - MultiHash::Sha3_384(metadata) - } -}