From b046f04d24e246369b1bebfc7e26926e393129bf Mon Sep 17 00:00:00 2001 From: Chralt Date: Mon, 19 Aug 2024 15:59:05 +0200 Subject: [PATCH] Fix migration & it-tests for release-v0.5.4 (#1357) * update migration and it tests * add RemoveMarkets migration * improve is_corrupted check * fix clippy * update migration --- Makefile | 2 +- integration-tests/moonwall.config.json | 4 +- runtime/battery-station/src/lib.rs | 4 + runtime/common/src/lib.rs | 5 +- runtime/zeitgeist/src/lib.rs | 4 + zrml/market-commons/src/lib.rs | 2 +- zrml/market-commons/src/migrations.rs | 334 +++++++++++++++++++++++-- 7 files changed, 323 insertions(+), 32 deletions(-) diff --git a/Makefile b/Makefile index c4b4241c1..d6887defc 100644 --- a/Makefile +++ b/Makefile @@ -33,7 +33,7 @@ try-runtime-upgrade-battery-station: --execute-try-runtime try-runtime-upgrade-zeitgeist: - @$(MAKE) TRYRUNTIME_URL="wss://zeitgeist-rpc.dwellir.com:443" \ + @$(MAKE) TRYRUNTIME_URL="wss://zeitgeist.api.onfinality.io:443/public-ws" \ RUNTIME_PATH="./target/release/wbuild/zeitgeist-runtime/zeitgeist_runtime.compact.compressed.wasm" \ -- \ --execute-try-runtime diff --git a/integration-tests/moonwall.config.json b/integration-tests/moonwall.config.json index 18cec7399..55da73649 100644 --- a/integration-tests/moonwall.config.json +++ b/integration-tests/moonwall.config.json @@ -61,7 +61,7 @@ } ] }, - "envVars": ["LOG_LEVEL=debug", "VERBOSE_LOG"], + "envVars": ["LOG_LEVEL=debug", "VERBOSE_LOG=true"], "buildBlockMode": "manual", "connections": [ { @@ -106,7 +106,7 @@ } ] }, - "envVars": ["LOG_LEVEL=debug", "VERBOSE_LOG"], + "envVars": ["LOG_LEVEL=debug", "VERBOSE_LOG=true"], "buildBlockMode": "manual", "connections": [ { diff --git a/runtime/battery-station/src/lib.rs b/runtime/battery-station/src/lib.rs index 6f2069d77..10b98a779 100644 --- a/runtime/battery-station/src/lib.rs +++ b/runtime/battery-station/src/lib.rs @@ -174,6 +174,10 @@ impl Contains for IsCallable { } } +parameter_types! { + pub RemovableMarketIds: Vec = vec![879u32, 877u32, 878u32, 880u32, 882u32]; +} + decl_common_types!(); create_runtime_with_additional_pallets!( diff --git a/runtime/common/src/lib.rs b/runtime/common/src/lib.rs index d64d0c01b..ac016306c 100644 --- a/runtime/common/src/lib.rs +++ b/runtime/common/src/lib.rs @@ -86,13 +86,14 @@ macro_rules! decl_common_types { generic, DispatchError, DispatchResult, RuntimeDebug, SaturatedConversion, }; use zeitgeist_primitives::traits::{DeployPoolApi, DistributeFees, MarketCommonsPalletApi}; - use zrml_market_commons::migrations::MigrateDisputeMechanism; + use zrml_market_commons::migrations::{MigrateDisputeMechanism, RemoveMarkets}; pub type Block = generic::Block; type Address = sp_runtime::MultiAddress; - type Migrations = (MigrateDisputeMechanism); + type Migrations = + (RemoveMarkets, MigrateDisputeMechanism); pub type Executive = frame_executive::Executive< Runtime, diff --git a/runtime/zeitgeist/src/lib.rs b/runtime/zeitgeist/src/lib.rs index a29bb28f1..083ba343e 100644 --- a/runtime/zeitgeist/src/lib.rs +++ b/runtime/zeitgeist/src/lib.rs @@ -191,6 +191,10 @@ impl Contains for IsCallable { } } +parameter_types! { + pub RemovableMarketIds: Vec = vec![]; +} + decl_common_types!(); create_runtime_with_additional_pallets!(); diff --git a/zrml/market-commons/src/lib.rs b/zrml/market-commons/src/lib.rs index cce65a670..2189acbbc 100644 --- a/zrml/market-commons/src/lib.rs +++ b/zrml/market-commons/src/lib.rs @@ -57,7 +57,7 @@ mod pallet { }; /// The current storage version. - const STORAGE_VERSION: StorageVersion = StorageVersion::new(11); + const STORAGE_VERSION: StorageVersion = StorageVersion::new(13); pub(crate) type AccountIdOf = ::AccountId; pub(crate) type AssetOf = Asset>; diff --git a/zrml/market-commons/src/migrations.rs b/zrml/market-commons/src/migrations.rs index adf04862f..c532fcab4 100644 --- a/zrml/market-commons/src/migrations.rs +++ b/zrml/market-commons/src/migrations.rs @@ -17,16 +17,18 @@ // along with Zeitgeist. If not, see . use crate::{AccountIdOf, BalanceOf, Config, MarketIdOf, MomentOf, Pallet as MarketCommons}; -use alloc::vec::Vec; +use alloc::{vec, vec::Vec}; use core::marker::PhantomData; use frame_support::{ pallet_prelude::Weight, + storage::migration::get_storage_value, traits::{Get, OnRuntimeUpgrade, StorageVersion}, + Blake2_128Concat, StorageHasher, }; use frame_system::pallet_prelude::BlockNumberFor; use parity_scale_codec::{Decode, Encode, MaxEncodedLen}; use scale_info::TypeInfo; -use sp_runtime::{Perbill, RuntimeDebug, Saturating}; +use sp_runtime::{Perbill, RuntimeDebug, SaturatedConversion, Saturating}; use zeitgeist_primitives::types::{ Asset, Deadlines, EarlyClose, Market, MarketBonds, MarketCreation, MarketDisputeMechanism, MarketPeriod, MarketStatus, MarketType, OutcomeReport, Report, ScoringRule, @@ -39,14 +41,165 @@ use { sp_runtime::DispatchError, }; -#[cfg(any(feature = "try-runtime", feature = "test"))] -use frame_support::Blake2_128Concat; - -#[cfg(any(feature = "try-runtime", test))] const MARKET_COMMONS: &[u8] = b"MarketCommons"; -#[cfg(any(feature = "try-runtime", test))] const MARKETS: &[u8] = b"Markets"; +const MARKET_COMMONS_REQUIRED_STORAGE_VERSION_0: u16 = 11; +const MARKET_COMMONS_NEXT_STORAGE_VERSION_0: u16 = 12; + +#[cfg(feature = "try-runtime")] +#[frame_support::storage_alias] +pub(crate) type Markets = + StorageMap, Blake2_128Concat, MarketIdOf, OldMarketOf>; + +// ID type of the campaign asset class. +pub type CampaignAssetId = u128; + +#[derive(Clone, Copy, Debug, Decode, Default, Eq, Encode, MaxEncodedLen, PartialEq, TypeInfo)] +pub enum BaseAssetClass { + #[codec(index = 4)] + #[default] + Ztg, + + #[codec(index = 5)] + ForeignAsset(u32), + + #[codec(index = 7)] + CampaignAsset(#[codec(compact)] CampaignAssetId), +} + +#[derive(Clone, Decode, Encode, Eq, PartialEq, RuntimeDebug, TypeInfo)] +pub struct OldMarketWithCampaignBaseAsset< + AccountId, + Balance, + BlockNumber, + Moment, + BaseAssetClass, + MarketId, +> { + pub market_id: MarketId, + /// Base asset of the market. + pub base_asset: BaseAssetClass, + /// Creator of this market. + pub creator: AccountId, + /// Creation type. + pub creation: MarketCreation, + /// A fee that is charged each trade and given to the market creator. + pub creator_fee: Perbill, + /// Oracle that reports the outcome of this market. + pub oracle: AccountId, + /// Metadata for the market, usually a content address of IPFS + /// hosted JSON. Currently limited to 66 bytes (see `MaxEncodedLen` implementation) + pub metadata: Vec, + /// The type of the market. + pub market_type: MarketType, + /// Market start and end + pub period: MarketPeriod, + /// Market deadlines. + pub deadlines: Deadlines, + /// The scoring rule used for the market. + pub scoring_rule: ScoringRule, + /// The current status of the market. + pub status: MarketStatus, + /// The report of the market. Only `Some` if it has been reported. + pub report: Option>, + /// The resolved outcome. + pub resolved_outcome: Option, + /// See [`MarketDisputeMechanism`]. + pub dispute_mechanism: Option, + /// The bonds reserved for this market. + pub bonds: MarketBonds, + /// The time at which the market was closed early. + pub early_close: Option>, +} + +type CorruptedMarketOf = OldMarketWithCampaignBaseAsset< + AccountIdOf, + BalanceOf, + BlockNumberFor, + MomentOf, + BaseAssetClass, + MarketIdOf, +>; + +pub struct RemoveMarkets(PhantomData, MarketIds); + +impl OnRuntimeUpgrade for RemoveMarkets +where + T: Config, + MarketIds: Get>, +{ + fn on_runtime_upgrade() -> Weight { + let mut total_weight = T::DbWeight::get().reads(1); + let market_commons_version = StorageVersion::get::>(); + if market_commons_version != MARKET_COMMONS_REQUIRED_STORAGE_VERSION_0 { + log::info!( + "RemoveMarkets: market-commons version is {:?}, but {:?} is required", + market_commons_version, + MARKET_COMMONS_REQUIRED_STORAGE_VERSION_0, + ); + return total_weight; + } + log::info!("RemoveMarkets: Starting..."); + + let mut corrupted_markets = vec![]; + + for &market_id in MarketIds::get().iter() { + let market_id = market_id.saturated_into::>(); + let is_corrupted = || { + if let Some(market) = get_storage_value::>( + MARKET_COMMONS, + MARKETS, + &MarketIdOf::::from(market_id).using_encoded(Blake2_128Concat::hash), + ) { + matches!(market.base_asset, BaseAssetClass::CampaignAsset(_)) + } else { + false + } + }; + if crate::Markets::::contains_key(market_id) && is_corrupted() { + crate::Markets::::remove(market_id); + corrupted_markets.push(market_id); + } else { + log::warn!( + "RemoveMarkets: Market {:?} was expected to be corrupted, but isn't.", + market_id + ); + } + } + + log::info!("RemoveMarkets: Removed markets {:?}.", corrupted_markets); + let count = MarketIds::get().len() as u64; + total_weight = total_weight + .saturating_add(T::DbWeight::get().reads_writes(count.saturating_mul(2u64), count)); + + StorageVersion::new(MARKET_COMMONS_NEXT_STORAGE_VERSION_0).put::>(); + total_weight = total_weight.saturating_add(T::DbWeight::get().writes(1)); + log::info!("RemoveMarkets: Done!"); + total_weight + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, DispatchError> { + Ok(vec![]) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(_previous_state: Vec) -> Result<(), DispatchError> { + for &market_id in MarketIds::get().iter() { + let market_id = market_id.saturated_into::>(); + assert!(!crate::Markets::::contains_key(market_id)); + assert!(crate::Markets::::try_get(market_id).is_err()); + } + + log::info!("RemoveMarkets: Post-upgrade done!"); + Ok(()) + } +} + +const MARKET_COMMONS_REQUIRED_STORAGE_VERSION_1: u16 = 12; +const MARKET_COMMONS_NEXT_STORAGE_VERSION_1: u16 = 13; + #[derive(Clone, Decode, Encode, Eq, PartialEq, RuntimeDebug, TypeInfo)] pub struct OldMarket { pub market_id: MarketId, @@ -95,14 +248,6 @@ pub enum OldMarketDisputeMechanism { SimpleDisputes, } -const MARKET_COMMONS_REQUIRED_STORAGE_VERSION: u16 = 11; -const MARKET_COMMONS_NEXT_STORAGE_VERSION: u16 = 12; - -#[cfg(feature = "try-runtime")] -#[frame_support::storage_alias] -pub(crate) type Markets = - StorageMap, Blake2_128Concat, MarketIdOf, OldMarketOf>; - pub struct MigrateDisputeMechanism(PhantomData); /// Removes the `SimpleDisputes` MDM by switching markets that use `SimpleDisputes` to `Authorized`. @@ -115,11 +260,11 @@ where fn on_runtime_upgrade() -> Weight { let mut total_weight = T::DbWeight::get().reads(1); let market_commons_version = StorageVersion::get::>(); - if market_commons_version != MARKET_COMMONS_REQUIRED_STORAGE_VERSION { + if market_commons_version != MARKET_COMMONS_REQUIRED_STORAGE_VERSION_1 { log::info!( "MigrateDisputeMechanism: market-commons version is {:?}, but {:?} is required", market_commons_version, - MARKET_COMMONS_REQUIRED_STORAGE_VERSION, + MARKET_COMMONS_REQUIRED_STORAGE_VERSION_1, ); return total_weight; } @@ -158,7 +303,7 @@ where total_weight = total_weight.saturating_add(T::DbWeight::get().reads_writes(translated, translated)); - StorageVersion::new(MARKET_COMMONS_NEXT_STORAGE_VERSION).put::>(); + StorageVersion::new(MARKET_COMMONS_NEXT_STORAGE_VERSION_1).put::>(); total_weight = total_weight.saturating_add(T::DbWeight::get().writes(1)); log::info!("MigrateDisputeMechanism: Done!"); total_weight @@ -213,6 +358,7 @@ where assert_eq!(old_market.bonds, new_market.bonds); assert_eq!(old_market.early_close, new_market.early_close); } + log::info!("MigrateDisputeMechanism: Post-upgrade market count is {}!", new_market_count); Ok(()) } @@ -226,25 +372,104 @@ mod tests { MarketOf, }; use alloc::fmt::Debug; - use frame_support::{migration::put_storage_value, Blake2_128Concat, StorageHasher}; + use frame_support::{ + migration::put_storage_value, parameter_types, Blake2_128Concat, StorageHasher, + }; use parity_scale_codec::Encode; use sp_io::storage::root as storage_root; use sp_runtime::{Perbill, StateVersion}; use test_case::test_case; use zeitgeist_primitives::types::{Bond, EarlyCloseState, MarketId}; + parameter_types! { + pub RemovableMarketIds: Vec = vec![879u32, 877u32, 878u32, 880u32, 882u32]; + pub NoRemovableMarketIds: Vec = vec![]; + } + #[test] fn on_runtime_upgrade_increments_the_storage_version() { ExtBuilder::default().build().execute_with(|| { - set_up_version(); + set_up_version_remove_markets(); + RemoveMarkets::::on_runtime_upgrade(); + assert_eq!( + StorageVersion::get::>(), + MARKET_COMMONS_NEXT_STORAGE_VERSION_0 + ); MigrateDisputeMechanism::::on_runtime_upgrade(); assert_eq!( StorageVersion::get::>(), - MARKET_COMMONS_NEXT_STORAGE_VERSION + MARKET_COMMONS_NEXT_STORAGE_VERSION_1 ); }); } + #[test] + fn on_runtime_upgrade_remove_corrupted_markets_works_as_expected() { + ExtBuilder::default().build().execute_with(|| { + set_up_version_remove_markets(); + let pallet = MARKET_COMMONS; + let prefix = MARKETS; + let corrupted_market = construct_corrupt_market(); + for market_id in RemovableMarketIds::get().iter() { + let storage_hash = MarketId::from(*market_id).using_encoded(Blake2_128Concat::hash); + put_storage_value::>( + pallet, + prefix, + &storage_hash, + corrupted_market.clone(), + ); + } + + for market_id in RemovableMarketIds::get().iter() { + let market_id = MarketId::from(*market_id); + assert!(crate::Markets::::contains_key(market_id)); + } + + RemoveMarkets::::on_runtime_upgrade(); + + for market_id in RemovableMarketIds::get().iter() { + let market_id = MarketId::from(*market_id); + assert!(!crate::Markets::::contains_key(market_id)); + } + }); + } + + #[test] + fn on_runtime_upgrade_does_not_remove_valid_markets() { + ExtBuilder::default().build().execute_with(|| { + set_up_version_remove_markets(); + let pallet = MARKET_COMMONS; + let prefix = MARKETS; + let (old_market, _) = construct_old_new_tuple( + Some(OldMarketDisputeMechanism::SimpleDisputes), + Some(MarketDisputeMechanism::Authorized), + ); + for market_id in RemovableMarketIds::get().iter() { + let storage_hash = MarketId::from(*market_id).using_encoded(Blake2_128Concat::hash); + // base asset for the market is not a campaign asset, so not corrupted + put_storage_value::>( + pallet, + prefix, + &storage_hash, + old_market.clone(), + ); + } + + for market_id in RemovableMarketIds::get().iter() { + let market_id = MarketId::from(*market_id); + assert!(crate::Markets::::contains_key(market_id)); + } + + RemoveMarkets::::on_runtime_upgrade(); + + for market_id in RemovableMarketIds::get().iter() { + let market_id = MarketId::from(*market_id); + // all markets still present, because no market was in a corrupted storage layout + assert!(crate::Markets::::contains_key(market_id)); + } + }); + } + #[test_case(None, None; "none")] #[test_case( Some(OldMarketDisputeMechanism::Authorized), @@ -255,12 +480,12 @@ mod tests { Some(OldMarketDisputeMechanism::SimpleDisputes), Some(MarketDisputeMechanism::Authorized) )] - fn on_runtime_upgrade_works_as_expected( + fn on_runtime_upgrade_mdm_works_as_expected( old_scoring_rule: Option, new_scoring_rule: Option, ) { ExtBuilder::default().build().execute_with(|| { - set_up_version(); + set_up_version_mdm(); let (old_market, new_market) = construct_old_new_tuple(old_scoring_rule, new_scoring_rule); populate_test_data::>( @@ -276,7 +501,7 @@ mod tests { #[test] fn on_runtime_upgrade_is_noop_if_versions_are_not_correct() { ExtBuilder::default().build().execute_with(|| { - StorageVersion::new(MARKET_COMMONS_NEXT_STORAGE_VERSION) + StorageVersion::new(MARKET_COMMONS_NEXT_STORAGE_VERSION_1) .put::>(); let market = Market { market_id: 7, @@ -311,8 +536,13 @@ mod tests { }); } - fn set_up_version() { - StorageVersion::new(MARKET_COMMONS_REQUIRED_STORAGE_VERSION) + fn set_up_version_remove_markets() { + StorageVersion::new(MARKET_COMMONS_REQUIRED_STORAGE_VERSION_0) + .put::>(); + } + + fn set_up_version_mdm() { + StorageVersion::new(MARKET_COMMONS_REQUIRED_STORAGE_VERSION_1) .put::>(); } @@ -388,6 +618,58 @@ mod tests { (old_markets, new_market) } + fn construct_corrupt_market() -> CorruptedMarketOf { + let market_id = 0; + let base_asset = BaseAssetClass::CampaignAsset(5u128); + let creator = 1; + let creation = MarketCreation::Advised; + let creator_fee = Perbill::from_rational(2u32, 3u32); + let oracle = 4; + let metadata = vec![5; 50]; + let market_type = MarketType::Scalar(6..=7); + let period = MarketPeriod::Block(8..9); + let deadlines = Deadlines { grace_period: 10, oracle_duration: 11, dispute_duration: 12 }; + let scoring_rule = ScoringRule::AmmCdaHybrid; + let status = MarketStatus::Resolved; + let report = Some(Report { at: 13, by: 14, outcome: OutcomeReport::Categorical(15) }); + let resolved_outcome = Some(OutcomeReport::Categorical(16)); + let bonds = MarketBonds { + creation: Some(Bond { who: 17, value: 18, is_settled: true }), + oracle: Some(Bond { who: 19, value: 20, is_settled: true }), + outsider: Some(Bond { who: 21, value: 22, is_settled: true }), + dispute: Some(Bond { who: 23, value: 24, is_settled: true }), + close_request: Some(Bond { who: 25, value: 26, is_settled: true }), + close_dispute: Some(Bond { who: 27, value: 28, is_settled: true }), + }; + let early_close = Some(EarlyClose { + old: MarketPeriod::Block(29..30), + new: MarketPeriod::Block(31..32), + state: EarlyCloseState::Disputed, + }); + let dispute_mechanism: Option = + Some(OldMarketDisputeMechanism::Court); + + OldMarketWithCampaignBaseAsset { + market_id, + creator, + base_asset, + creation, + creator_fee, + oracle, + metadata, + market_type, + period, + deadlines, + scoring_rule, + status, + report, + resolved_outcome, + dispute_mechanism, + bonds, + early_close, + } + } + #[allow(unused)] fn populate_test_data(pallet: &[u8], prefix: &[u8], data: Vec) where