From c91f48edff6d977f50be7e108a7f21c7003b5e8f Mon Sep 17 00:00:00 2001 From: Malte Kliemann Date: Wed, 7 Feb 2024 11:32:51 +0100 Subject: [PATCH 1/7] Use `BoundedBTreeMap` in neo-swaps `Pool` struct --- zrml/neo-swaps/src/lib.rs | 10 ++--- zrml/neo-swaps/src/tests/sell.rs | 4 +- zrml/neo-swaps/src/types/max_assets.rs | 26 +++++++++++++ zrml/neo-swaps/src/types/mod.rs | 2 + zrml/neo-swaps/src/types/pool.rs | 51 ++++++++------------------ 5 files changed, 50 insertions(+), 43 deletions(-) create mode 100644 zrml/neo-swaps/src/types/max_assets.rs diff --git a/zrml/neo-swaps/src/lib.rs b/zrml/neo-swaps/src/lib.rs index ed0a3f30a..974dc031f 100644 --- a/zrml/neo-swaps/src/lib.rs +++ b/zrml/neo-swaps/src/lib.rs @@ -38,11 +38,11 @@ pub use pallet::*; #[frame_support::pallet] mod pallet { use crate::{ - consts::{LN_NUMERICAL_LIMIT, MAX_ASSETS}, + consts::LN_NUMERICAL_LIMIT, liquidity_tree::types::{BenchmarkInfo, LiquidityTree, LiquidityTreeError}, math::{Math, MathOps}, traits::{pool_operations::PoolOperations, LiquiditySharesManager}, - types::{FeeDistribution, Pool}, + types::{FeeDistribution, MaxAssets, Pool}, weights::*, }; use alloc::{collections::BTreeMap, vec, vec::Vec}; @@ -99,7 +99,7 @@ mod pallet { pub(crate) type MarketIdOf = <::MarketCommons as MarketCommonsPalletApi>::MarketId; pub(crate) type LiquidityTreeOf = LiquidityTree::MaxLiquidityTreeDepth>; - pub(crate) type PoolOf = Pool>; + pub(crate) type PoolOf = Pool, MaxAssets>; #[pallet::config] pub trait Config: frame_system::Config { @@ -833,7 +833,7 @@ mod pallet { ensure!(market.scoring_rule == ScoringRule::Lmsr, Error::::InvalidTradingMechanism); let asset_count = spot_prices.len(); ensure!(asset_count as u16 == market.outcomes(), Error::::IncorrectVecLen); - ensure!(market.outcomes() <= MAX_ASSETS, Error::::AssetCountAboveMax); + ensure!(market.outcomes() as u32 <= MaxAssets::get(), Error::::AssetCountAboveMax); ensure!(swap_fee >= MIN_SWAP_FEE.saturated_into(), Error::::SwapFeeBelowMin); ensure!(swap_fee <= T::MaxSwapFee::get(), Error::::SwapFeeAboveMax); ensure!( @@ -869,7 +869,7 @@ mod pallet { let collateral = market.base_asset; let pool = Pool { account_id: pool_account_id.clone(), - reserves: reserves.clone(), + reserves: reserves.clone().try_into().map_err(|_| Error::::Unexpected)?, collateral, liquidity_parameter, liquidity_shares_manager: LiquidityTree::new(who.clone(), amount)?, diff --git a/zrml/neo-swaps/src/tests/sell.rs b/zrml/neo-swaps/src/tests/sell.rs index ae6aa501f..45022ff6d 100644 --- a/zrml/neo-swaps/src/tests/sell.rs +++ b/zrml/neo-swaps/src/tests/sell.rs @@ -267,7 +267,7 @@ fn sell_fails_if_price_is_too_low() { // speaking this leaves the pool in an inconsistent state (reserve recorded in the `Pool` // struct is smaller than actual reserve), but this doesn't matter in this test. NeoSwaps::try_mutate_pool(&market_id, |pool| { - pool.reserves.insert(asset_in, 11 * pool.liquidity_parameter); + pool.reserves.try_insert(asset_in, 11 * pool.liquidity_parameter).unwrap(); Ok(()) }) .unwrap(); @@ -306,7 +306,7 @@ fn sell_fails_if_price_is_pushed_below_threshold() { NeoSwaps::try_mutate_pool(&market_id, |pool| { // The price is right at the brink here. Any further shift and sells won't be accepted // anymore. - pool.reserves.insert(asset_in, 10 * pool.liquidity_parameter); + pool.reserves.try_insert(asset_in, 10 * pool.liquidity_parameter).unwrap(); Ok(()) }) .unwrap(); diff --git a/zrml/neo-swaps/src/types/max_assets.rs b/zrml/neo-swaps/src/types/max_assets.rs new file mode 100644 index 000000000..1a5e96b7c --- /dev/null +++ b/zrml/neo-swaps/src/types/max_assets.rs @@ -0,0 +1,26 @@ +// Copyright 2023-2024 Forecasting Technologies LTD. +// +// This file is part of Zeitgeist. +// +// Zeitgeist is free software: you can redistribute it and/or modify it +// under the terms of the GNU General Public License as published by the +// Free Software Foundation, either version 3 of the License, or (at +// your option) any later version. +// +// Zeitgeist is distributed in the hope that it will be useful, but +// WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zeitgeist. If not, see . + +use sp_runtime::traits::Get; + +pub(crate) struct MaxAssets; + +impl Get for MaxAssets { + fn get() -> u32 { + 128 + } +} diff --git a/zrml/neo-swaps/src/types/mod.rs b/zrml/neo-swaps/src/types/mod.rs index 30734e23e..14da6c7fc 100644 --- a/zrml/neo-swaps/src/types/mod.rs +++ b/zrml/neo-swaps/src/types/mod.rs @@ -16,7 +16,9 @@ // along with Zeitgeist. If not, see . mod fee_distribution; +mod max_assets; mod pool; pub(crate) use fee_distribution::*; +pub(crate) use max_assets::*; pub(crate) use pool::*; diff --git a/zrml/neo-swaps/src/types/pool.rs b/zrml/neo-swaps/src/types/pool.rs index 301dc89d0..bf355919a 100644 --- a/zrml/neo-swaps/src/types/pool.rs +++ b/zrml/neo-swaps/src/types/pool.rs @@ -16,37 +16,43 @@ // along with Zeitgeist. If not, see . use crate::{ - consts::{EXP_NUMERICAL_LIMIT, MAX_ASSETS}, + consts::EXP_NUMERICAL_LIMIT, math::{Math, MathOps}, pallet::{AssetOf, BalanceOf, Config}, traits::{LiquiditySharesManager, PoolOperations}, Error, }; -use alloc::{collections::BTreeMap, vec::Vec}; +use alloc::vec::Vec; +use frame_support::{storage::bounded_btree_map::BoundedBTreeMap, CloneNoBound}; use parity_scale_codec::{Decode, Encode, MaxEncodedLen}; use scale_info::TypeInfo; use sp_runtime::{ - traits::{CheckedAdd, CheckedSub}, + traits::{CheckedAdd, CheckedSub, Get}, DispatchError, DispatchResult, RuntimeDebug, SaturatedConversion, Saturating, }; -#[derive(Clone, Decode, Encode, Eq, PartialEq, RuntimeDebug, TypeInfo)] -#[scale_info(skip_type_params(T))] -pub struct Pool +#[derive(CloneNoBound, Decode, Encode, Eq, MaxEncodedLen, PartialEq, RuntimeDebug, TypeInfo)] +#[scale_info(skip_type_params(S, T))] +pub struct Pool where - LSM: LiquiditySharesManager, + T: Config, + LSM: Clone + LiquiditySharesManager, + S: Get, { pub account_id: T::AccountId, - pub reserves: BTreeMap, BalanceOf>, + pub reserves: BoundedBTreeMap, BalanceOf, S>, pub collateral: AssetOf, pub liquidity_parameter: BalanceOf, pub liquidity_shares_manager: LSM, pub swap_fee: BalanceOf, } -impl + TypeInfo> PoolOperations for Pool +impl PoolOperations for Pool where + T: Config, BalanceOf: SaturatedConversion, + LSM: Clone + LiquiditySharesManager + TypeInfo, + S: Get, { fn assets(&self) -> Vec> { self.reserves.keys().cloned().collect() @@ -117,30 +123,3 @@ where Math::::calculate_buy_ln_argument(reserve, amount_in, self.liquidity_parameter) } } - -// TODO(#1214): Replace BTreeMap with BoundedBTreeMap and remove the unnecessary `MaxEncodedLen` -// implementation. -impl> MaxEncodedLen for Pool -where - T::AccountId: MaxEncodedLen, - AssetOf: MaxEncodedLen, - BalanceOf: MaxEncodedLen, - LSM: MaxEncodedLen, -{ - fn max_encoded_len() -> usize { - let len_account_id = T::AccountId::max_encoded_len(); - let len_reserves = 1usize.saturating_add((MAX_ASSETS as usize).saturating_mul( - >::max_encoded_len().saturating_add(BalanceOf::::max_encoded_len()), - )); - let len_collateral = AssetOf::::max_encoded_len(); - let len_liquidity_parameter = BalanceOf::::max_encoded_len(); - let len_liquidity_shares_manager = LSM::max_encoded_len(); - let len_swap_fee = BalanceOf::::max_encoded_len(); - len_account_id - .saturating_add(len_reserves) - .saturating_add(len_collateral) - .saturating_add(len_liquidity_parameter) - .saturating_add(len_liquidity_shares_manager) - .saturating_add(len_swap_fee) - } -} From b122292da79c7ff0d89fa7939c91fe0c93c84fca Mon Sep 17 00:00:00 2001 From: Malte Kliemann Date: Wed, 7 Feb 2024 14:59:42 +0100 Subject: [PATCH 2/7] Migrate to new pool struct --- runtime/common/src/lib.rs | 3 +- zrml/neo-swaps/src/consts.rs | 2 +- zrml/neo-swaps/src/lib.rs | 2 +- zrml/neo-swaps/src/migration.rs | 233 ++++++++++++++++++++++++ zrml/neo-swaps/src/mock.rs | 2 +- zrml/neo-swaps/src/tests/deploy_pool.rs | 2 +- zrml/neo-swaps/src/types/max_assets.rs | 3 +- zrml/neo-swaps/src/types/pool.rs | 17 +- 8 files changed, 252 insertions(+), 12 deletions(-) diff --git a/runtime/common/src/lib.rs b/runtime/common/src/lib.rs index 12e5a57cb..37abc22b4 100644 --- a/runtime/common/src/lib.rs +++ b/runtime/common/src/lib.rs @@ -55,12 +55,13 @@ macro_rules! decl_common_types { use orml_traits::MultiCurrency; use sp_runtime::{generic, DispatchError, DispatchResult, SaturatedConversion}; use zeitgeist_primitives::traits::{DeployPoolApi, DistributeFees, MarketCommonsPalletApi}; + use zrml_neo_swaps::migration::MigratePoolReservesToBoundedBTreeMap; pub type Block = generic::Block; type Address = sp_runtime::MultiAddress; - type Migrations = (); + type Migrations = (MigratePoolReservesToBoundedBTreeMap); pub type Executive = frame_executive::Executive< Runtime, diff --git a/zrml/neo-swaps/src/consts.rs b/zrml/neo-swaps/src/consts.rs index 19d0af5f7..270c9fc1a 100644 --- a/zrml/neo-swaps/src/consts.rs +++ b/zrml/neo-swaps/src/consts.rs @@ -22,7 +22,7 @@ pub(crate) const EXP_NUMERICAL_LIMIT: u128 = 10; /// Numerical lower limit for ln arguments (fixed point number). pub(crate) const LN_NUMERICAL_LIMIT: u128 = BASE / 10; /// The maximum number of assets allowed in a pool. -pub(crate) const MAX_ASSETS: u16 = 128; +pub(crate) const MAX_ASSETS: u32 = 128; pub(crate) const _1: u128 = BASE; pub(crate) const _2: u128 = 2 * _1; diff --git a/zrml/neo-swaps/src/lib.rs b/zrml/neo-swaps/src/lib.rs index 974dc031f..698c61e26 100644 --- a/zrml/neo-swaps/src/lib.rs +++ b/zrml/neo-swaps/src/lib.rs @@ -74,7 +74,7 @@ mod pallet { }; use zrml_market_commons::MarketCommonsPalletApi; - pub(crate) const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); + pub(crate) const STORAGE_VERSION: StorageVersion = StorageVersion::new(2); // These should not be config parameters to avoid misconfigurations. pub(crate) const EXIT_FEE: u128 = CENT / 10; diff --git a/zrml/neo-swaps/src/migration.rs b/zrml/neo-swaps/src/migration.rs index fb9ed7fb7..2aa4996f9 100644 --- a/zrml/neo-swaps/src/migration.rs +++ b/zrml/neo-swaps/src/migration.rs @@ -15,6 +15,239 @@ // You should have received a copy of the GNU General Public License // along with Zeitgeist. If not, see . +use crate::{ + traits::LiquiditySharesManager, types::Pool, AssetOf, BalanceOf, Config, LiquidityTreeOf, + Pallet, Pools, +}; +use alloc::collections::BTreeMap; +use core::marker::PhantomData; +use frame_support::{ + dispatch::Weight, + log, + traits::{Get, OnRuntimeUpgrade, StorageVersion}, + RuntimeDebug, +}; +use parity_scale_codec::{Decode, Encode}; +use scale_info::TypeInfo; +use sp_runtime::Saturating; + +cfg_if::cfg_if! { + if #[cfg(feature = "try-runtime")] { + use crate::{MarketIdOf}; + use alloc::{format, vec::Vec}; + use frame_support::{migration::storage_key_iter, pallet_prelude::Twox64Concat}; + } +} + +cfg_if::cfg_if! { + if #[cfg(any(feature = "try-runtime", test))] { + const NEO_SWAPS: &[u8] = b"NeoSwaps"; + const POOLS: &[u8] = b"Pools"; + } +} + +const NEO_SWAPS_REQUIRED_STORAGE_VERSION: u16 = 1; +const NEO_SWAPS_NEXT_STORAGE_VERSION: u16 = NEO_SWAPS_REQUIRED_STORAGE_VERSION + 1; + +#[derive(Clone, Decode, Encode, Eq, PartialEq, RuntimeDebug, TypeInfo)] +#[scale_info(skip_type_params(T))] +pub struct OldPool +where + T: Config, + LSM: LiquiditySharesManager, +{ + pub account_id: T::AccountId, + pub reserves: BTreeMap, BalanceOf>, + pub collateral: AssetOf, + pub liquidity_parameter: BalanceOf, + pub liquidity_shares_manager: LSM, + pub swap_fee: BalanceOf, +} + +type OldPoolOf = OldPool>; + +pub struct MigratePoolReservesToBoundedBTreeMap(PhantomData); + +impl OnRuntimeUpgrade for MigratePoolReservesToBoundedBTreeMap +where + T: Config, +{ + fn on_runtime_upgrade() -> Weight { + let mut total_weight = T::DbWeight::get().reads(1); + let neo_swaps_version = StorageVersion::get::>(); + if neo_swaps_version != NEO_SWAPS_REQUIRED_STORAGE_VERSION { + log::info!( + "MigratePoolReservesToBoundedBTreeMap: neo-swaps version is {:?}, but {:?} is \ + required", + neo_swaps_version, + NEO_SWAPS_REQUIRED_STORAGE_VERSION, + ); + return total_weight; + } + log::info!("MigratePoolReservesToBoundedBTreeMap: Starting..."); + let mut translated = 0u64; + Pools::::translate::, _>(|_, pool| { + // Can't fail unless `MaxAssets` is misconfigured. If it fails after all, we delete the + // pool. This may seem drastic, but is actually cleaner than trying some half-baked + // recovery and allows us to do a manual recovery of funds. + let reserves = pool.reserves.try_into().ok()?; + translated.saturating_inc(); + Some(Pool { + account_id: pool.account_id, + reserves, + collateral: pool.collateral, + liquidity_parameter: pool.liquidity_parameter, + liquidity_shares_manager: pool.liquidity_shares_manager, + swap_fee: pool.swap_fee, + }) + }); + log::info!("MigratePoolReservesToBoundedBTreeMap: Upgraded {} pools.", translated); + total_weight = + total_weight.saturating_add(T::DbWeight::get().reads_writes(translated, translated)); + StorageVersion::new(NEO_SWAPS_NEXT_STORAGE_VERSION).put::>(); + total_weight = total_weight.saturating_add(T::DbWeight::get().writes(1)); + log::info!("MigratePoolReservesToBoundedBTreeMap: Done!"); + total_weight + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, &'static str> { + let old_pools = + storage_key_iter::, OldPoolOf, Twox64Concat>(NEO_SWAPS, POOLS) + .collect::>(); + Ok(old_pools.encode()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(previous_state: Vec) -> Result<(), &'static str> { + let old_pools: BTreeMap, OldPoolOf> = + Decode::decode(&mut &previous_state[..]) + .map_err(|_| "Failed to decode state: Invalid state")?; + let new_pool_count = Pools::::iter().count(); + assert_eq!(old_pools.len(), new_pool_count); + for (market_id, new_pool) in Pools::::iter() { + let old_pool = + old_pools.get(&market_id).expect(&format!("Pool {:?} not found", market_id)[..]); + assert_eq!(new_pool.account_id, old_pool.account_id); + assert_eq!(new_pool.reserves.into_inner(), old_pool.reserves); + assert_eq!(new_pool.collateral, old_pool.collateral); + assert_eq!(new_pool.liquidity_parameter, old_pool.liquidity_parameter); + assert_eq!(new_pool.liquidity_shares_manager, old_pool.liquidity_shares_manager); + assert_eq!(new_pool.swap_fee, old_pool.swap_fee); + } + log::info!( + "MigratePoolReservesToBoundedBTreeMap: Post-upgrade pool count is {}!", + new_pool_count + ); + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{ + liquidity_tree::types::LiquidityTree, + mock::{ExtBuilder, Runtime}, + MarketIdOf, PoolOf, Pools, + }; + use alloc::collections::BTreeMap; + use frame_support::{ + dispatch::fmt::Debug, migration::put_storage_value, storage_root, StateVersion, + StorageHasher, Twox64Concat, + }; + use parity_scale_codec::Encode; + use zeitgeist_primitives::types::Asset; + + #[test] + fn on_runtime_upgrade_increments_the_storage_version() { + ExtBuilder::default().build().execute_with(|| { + set_up_version(); + MigratePoolReservesToBoundedBTreeMap::::on_runtime_upgrade(); + assert_eq!(StorageVersion::get::>(), NEO_SWAPS_NEXT_STORAGE_VERSION); + }); + } + + #[test] + fn on_runtime_upgrade_is_noop_if_versions_are_not_correct() { + ExtBuilder::default().build().execute_with(|| { + StorageVersion::new(NEO_SWAPS_NEXT_STORAGE_VERSION).put::>(); + let (_, new_pools) = construct_old_new_tuple(); + populate_test_data::, PoolOf>( + NEO_SWAPS, POOLS, new_pools, + ); + let tmp = storage_root(StateVersion::V1); + MigratePoolReservesToBoundedBTreeMap::::on_runtime_upgrade(); + assert_eq!(tmp, storage_root(StateVersion::V1)); + }); + } + + #[test] + fn on_runtime_upgrade_correctly_updates_markets() { + ExtBuilder::default().build().execute_with(|| { + set_up_version(); + let (old_pools, new_pools) = construct_old_new_tuple(); + populate_test_data::, OldPoolOf>( + NEO_SWAPS, POOLS, old_pools, + ); + MigratePoolReservesToBoundedBTreeMap::::on_runtime_upgrade(); + let actual = Pools::get(0u128).unwrap(); + assert_eq!(actual, new_pools[0]); + }); + } + + fn set_up_version() { + StorageVersion::new(NEO_SWAPS_REQUIRED_STORAGE_VERSION).put::>(); + } + + fn construct_old_new_tuple() -> (Vec>, Vec>) { + let account_id = 1; + let mut old_reserves = BTreeMap::new(); + old_reserves.insert(Asset::CategoricalOutcome(2, 3), 4); + let new_reserves = old_reserves.clone().try_into().unwrap(); + let collateral = Asset::Ztg; + let liquidity_parameter = 5; + let swap_fee = 6; + let total_shares = 7; + let fees = 8; + + let mut liquidity_shares_manager = LiquidityTree::new(account_id, total_shares).unwrap(); + liquidity_shares_manager.nodes.get_mut(0).unwrap().fees = fees; + + let old_pool = OldPoolOf { + account_id, + reserves: old_reserves, + collateral, + liquidity_parameter, + liquidity_shares_manager: liquidity_shares_manager.clone(), + swap_fee, + }; + let new_pool = Pool { + account_id, + reserves: new_reserves, + collateral, + liquidity_parameter, + liquidity_shares_manager, + swap_fee, + }; + (vec![old_pool], vec![new_pool]) + } + + #[allow(unused)] + fn populate_test_data(pallet: &[u8], prefix: &[u8], data: Vec) + where + H: StorageHasher, + K: TryFrom + Encode, + V: Encode + Clone, + >::Error: Debug, + { + for (key, value) in data.iter().enumerate() { + let storage_hash = utility::key_to_hash::(K::try_from(key).unwrap()); + put_storage_value::(pallet, prefix, &storage_hash, (*value).clone()); + } + } +} + mod utility { use alloc::vec::Vec; use frame_support::StorageHasher; diff --git a/zrml/neo-swaps/src/mock.rs b/zrml/neo-swaps/src/mock.rs index a6d519fd8..8466e155e 100644 --- a/zrml/neo-swaps/src/mock.rs +++ b/zrml/neo-swaps/src/mock.rs @@ -98,7 +98,7 @@ parameter_types! { pub const OracleBond: Balance = 0; pub const ValidityBond: Balance = 0; pub const DisputeBond: Balance = 0; - pub const MaxCategories: u16 = MAX_ASSETS + 1; + pub const MaxCategories: u16 = MAX_ASSETS as u16 + 1; } pub struct DeployPoolNoop; diff --git a/zrml/neo-swaps/src/tests/deploy_pool.rs b/zrml/neo-swaps/src/tests/deploy_pool.rs index 689dd76c0..b6d2ae4a1 100644 --- a/zrml/neo-swaps/src/tests/deploy_pool.rs +++ b/zrml/neo-swaps/src/tests/deploy_pool.rs @@ -262,7 +262,7 @@ fn deploy_pool_fails_on_asset_count_above_max() { let market_id = create_market( ALICE, BASE_ASSET, - MarketType::Categorical(category_count), + MarketType::Categorical(category_count as u16), ScoringRule::Lmsr, ); let liquidity = _10; diff --git a/zrml/neo-swaps/src/types/max_assets.rs b/zrml/neo-swaps/src/types/max_assets.rs index 1a5e96b7c..b939bc3d4 100644 --- a/zrml/neo-swaps/src/types/max_assets.rs +++ b/zrml/neo-swaps/src/types/max_assets.rs @@ -16,11 +16,12 @@ // along with Zeitgeist. If not, see . use sp_runtime::traits::Get; +use crate::consts::MAX_ASSETS; pub(crate) struct MaxAssets; impl Get for MaxAssets { fn get() -> u32 { - 128 + MAX_ASSETS } } diff --git a/zrml/neo-swaps/src/types/pool.rs b/zrml/neo-swaps/src/types/pool.rs index bf355919a..683088840 100644 --- a/zrml/neo-swaps/src/types/pool.rs +++ b/zrml/neo-swaps/src/types/pool.rs @@ -22,21 +22,26 @@ use crate::{ traits::{LiquiditySharesManager, PoolOperations}, Error, }; -use alloc::vec::Vec; -use frame_support::{storage::bounded_btree_map::BoundedBTreeMap, CloneNoBound}; +use alloc::{fmt::Debug, vec::Vec}; +use frame_support::{ + storage::bounded_btree_map::BoundedBTreeMap, CloneNoBound, PartialEqNoBound, + RuntimeDebugNoBound, +}; use parity_scale_codec::{Decode, Encode, MaxEncodedLen}; use scale_info::TypeInfo; use sp_runtime::{ traits::{CheckedAdd, CheckedSub, Get}, - DispatchError, DispatchResult, RuntimeDebug, SaturatedConversion, Saturating, + DispatchError, DispatchResult, SaturatedConversion, Saturating, }; -#[derive(CloneNoBound, Decode, Encode, Eq, MaxEncodedLen, PartialEq, RuntimeDebug, TypeInfo)] +#[derive( + CloneNoBound, Decode, Encode, Eq, MaxEncodedLen, PartialEqNoBound, RuntimeDebugNoBound, TypeInfo, +)] #[scale_info(skip_type_params(S, T))] pub struct Pool where T: Config, - LSM: Clone + LiquiditySharesManager, + LSM: Clone + Debug + LiquiditySharesManager + PartialEq, S: Get, { pub account_id: T::AccountId, @@ -51,7 +56,7 @@ impl PoolOperations for Pool where T: Config, BalanceOf: SaturatedConversion, - LSM: Clone + LiquiditySharesManager + TypeInfo, + LSM: Clone + Debug + LiquiditySharesManager + TypeInfo + PartialEq, S: Get, { fn assets(&self) -> Vec> { From 36edc48b55868ea7752bfb4574e405d0254e8ac9 Mon Sep 17 00:00:00 2001 From: Malte Kliemann Date: Wed, 7 Feb 2024 16:25:59 +0100 Subject: [PATCH 3/7] Fix formatting --- zrml/neo-swaps/src/types/max_assets.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zrml/neo-swaps/src/types/max_assets.rs b/zrml/neo-swaps/src/types/max_assets.rs index b939bc3d4..d85ebdd05 100644 --- a/zrml/neo-swaps/src/types/max_assets.rs +++ b/zrml/neo-swaps/src/types/max_assets.rs @@ -15,8 +15,8 @@ // You should have received a copy of the GNU General Public License // along with Zeitgeist. If not, see . -use sp_runtime::traits::Get; use crate::consts::MAX_ASSETS; +use sp_runtime::traits::Get; pub(crate) struct MaxAssets; From 8415c118c8fe564683f4868fb04f77def3d84f35 Mon Sep 17 00:00:00 2001 From: Malte Kliemann Date: Fri, 5 Apr 2024 19:22:20 +0200 Subject: [PATCH 4/7] Update zrml/neo-swaps/src/types/max_assets.rs Co-authored-by: Harald Heckmann --- zrml/neo-swaps/src/types/max_assets.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zrml/neo-swaps/src/types/max_assets.rs b/zrml/neo-swaps/src/types/max_assets.rs index d85ebdd05..9a9ce0ebd 100644 --- a/zrml/neo-swaps/src/types/max_assets.rs +++ b/zrml/neo-swaps/src/types/max_assets.rs @@ -1,4 +1,4 @@ -// Copyright 2023-2024 Forecasting Technologies LTD. +// Copyright 2024 Forecasting Technologies LTD. // // This file is part of Zeitgeist. // From 9c7d94edeca54627f07b3a77f52e8ca82752ac25 Mon Sep 17 00:00:00 2001 From: Malte Kliemann Date: Sun, 7 Apr 2024 18:21:12 +0200 Subject: [PATCH 5/7] Prettify imports --- zrml/prediction-markets/src/mock.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/zrml/prediction-markets/src/mock.rs b/zrml/prediction-markets/src/mock.rs index 8b784b3a8..44a2e9e94 100644 --- a/zrml/prediction-markets/src/mock.rs +++ b/zrml/prediction-markets/src/mock.rs @@ -34,8 +34,6 @@ use frame_support::{ }, }; use frame_system::{EnsureRoot, EnsureSigned, EnsureSignedBy}; -#[cfg(feature = "parachain")] -use orml_asset_registry::AssetMetadata; use parity_scale_codec::Compact; use sp_arithmetic::per_things::Percent; use sp_runtime::{ @@ -44,8 +42,6 @@ use sp_runtime::{ DispatchError, DispatchResult, }; use std::cell::RefCell; -#[cfg(feature = "parachain")] -use zeitgeist_primitives::types::XcmAsset; use zeitgeist_primitives::{ constants::mock::{ AddOutcomePeriod, AggregationPeriod, AppealBond, AppealPeriod, AssetsAccountDeposit, @@ -73,6 +69,9 @@ use zeitgeist_primitives::{ }, }; +#[cfg(feature = "parachain")] +use {orml_asset_registry::AssetMetadata, zeitgeist_primitives::types::XcmAsset}; + pub(super) const ON_PROPOSAL_STORAGE: [u8; 4] = [0x09, 0x09, 0x00, 0x00]; pub(super) const ON_ACTIVATION_STORAGE: [u8; 4] = [0x09, 0x09, 0x00, 0x01]; pub(super) const ON_CLOSURE_STORAGE: [u8; 4] = [0x09, 0x09, 0x00, 0x02]; From 5d75af2b1f81d6cf0581d45b6fe00cc6df1f9ea1 Mon Sep 17 00:00:00 2001 From: Malte Kliemann Date: Sun, 7 Apr 2024 20:21:50 +0200 Subject: [PATCH 6/7] Clean up type conversions --- zrml/neo-swaps/src/lib.rs | 60 +++++++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 22 deletions(-) diff --git a/zrml/neo-swaps/src/lib.rs b/zrml/neo-swaps/src/lib.rs index 87a16877a..2c94b0f48 100644 --- a/zrml/neo-swaps/src/lib.rs +++ b/zrml/neo-swaps/src/lib.rs @@ -267,6 +267,8 @@ mod pallet { LiquidityTreeError(LiquidityTreeError), /// The relative value of a new LP position is too low. MinRelativeLiquidityThresholdViolated, + /// Narrowing type conversion occurred. + NarrowingConversion, } #[derive(Decode, Encode, Eq, PartialEq, PalletError, RuntimeDebug, TypeInfo)] @@ -311,7 +313,7 @@ mod pallet { /// Depends on the implementation of `CompleteSetOperationsApi` and `ExternalFees`; when /// using the canonical implementations, the runtime complexity is `O(asset_count)`. #[pallet::call_index(0)] - #[pallet::weight(T::WeightInfo::buy(*asset_count as u32))] + #[pallet::weight(T::WeightInfo::buy((*asset_count).saturated_into()))] #[transactional] pub fn buy( origin: OriginFor, @@ -325,7 +327,7 @@ mod pallet { let asset_count_real = T::MarketCommons::market(&market_id)?.outcomes(); ensure!(asset_count == asset_count_real, Error::::IncorrectAssetCount); Self::do_buy(who, market_id, asset_out, amount_in, min_amount_out)?; - Ok(Some(T::WeightInfo::buy(asset_count as u32)).into()) + Ok(Some(T::WeightInfo::buy(asset_count.into())).into()) } /// Sell outcome tokens to the specified market. @@ -355,7 +357,7 @@ mod pallet { /// Depends on the implementation of `CompleteSetOperationsApi` and `ExternalFees`; when /// using the canonical implementations, the runtime complexity is `O(asset_count)`. #[pallet::call_index(1)] - #[pallet::weight(T::WeightInfo::sell(*asset_count as u32))] + #[pallet::weight(T::WeightInfo::sell((*asset_count).saturated_into()))] #[transactional] pub fn sell( origin: OriginFor, @@ -369,7 +371,7 @@ mod pallet { let asset_count_real = T::MarketCommons::market(&market_id)?.outcomes(); ensure!(asset_count == asset_count_real, Error::::IncorrectAssetCount); Self::do_sell(who, market_id, asset_in, amount_in, min_amount_out)?; - Ok(Some(T::WeightInfo::sell(asset_count as u32)).into()) + Ok(Some(T::WeightInfo::sell(asset_count.into())).into()) } /// Join the liquidity pool for the specified market. @@ -396,9 +398,9 @@ mod pallet { /// providers in the pool. #[pallet::call_index(2)] #[pallet::weight( - T::WeightInfo::join_in_place(max_amounts_in.len() as u32) - .max(T::WeightInfo::join_reassigned(max_amounts_in.len() as u32)) - .max(T::WeightInfo::join_leaf(max_amounts_in.len() as u32)) + T::WeightInfo::join_in_place(max_amounts_in.len().saturated_into()) + .max(T::WeightInfo::join_reassigned(max_amounts_in.len().saturated_into())) + .max(T::WeightInfo::join_leaf(max_amounts_in.len().saturated_into())) )] #[transactional] pub fn join( @@ -409,7 +411,11 @@ mod pallet { ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let asset_count = T::MarketCommons::market(&market_id)?.outcomes(); - ensure!(max_amounts_in.len() == asset_count as usize, Error::::IncorrectVecLen); + let asset_count_usize: usize = asset_count.into(); + // Ensure that the conversion in the weight calculation doesn't saturate. + let _: u32 = + max_amounts_in.len().try_into().map_err(|_| Error::::NarrowingConversion)?; + ensure!(max_amounts_in.len() == asset_count_usize, Error::::IncorrectVecLen); Self::do_join(who, market_id, pool_shares_amount, max_amounts_in) } @@ -446,7 +452,7 @@ mod pallet { /// pool's liquidity tree, or, equivalently, `log_2(m)` where `m` is the number of liquidity /// providers in the pool. #[pallet::call_index(3)] - #[pallet::weight(T::WeightInfo::exit(min_amounts_out.len() as u32))] + #[pallet::weight(T::WeightInfo::exit(min_amounts_out.len().saturated_into()))] #[transactional] pub fn exit( origin: OriginFor, @@ -456,9 +462,12 @@ mod pallet { ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let asset_count = T::MarketCommons::market(&market_id)?.outcomes(); - ensure!(min_amounts_out.len() == asset_count as usize, Error::::IncorrectVecLen); + let asset_count_u32: u32 = asset_count.into(); + let min_amounts_out_len: u32 = + min_amounts_out.len().try_into().map_err(|_| Error::::NarrowingConversion)?; + ensure!(min_amounts_out_len == asset_count_u32, Error::::IncorrectVecLen); Self::do_exit(who, market_id, pool_shares_amount_out, min_amounts_out)?; - Ok(Some(T::WeightInfo::exit(asset_count as u32)).into()) + Ok(Some(T::WeightInfo::exit(min_amounts_out_len)).into()) } /// Withdraw swap fees from the specified market. @@ -510,7 +519,7 @@ mod pallet { /// /// `O(n)` where `n` is the number of assets in the pool. #[pallet::call_index(5)] - #[pallet::weight(T::WeightInfo::deploy_pool(spot_prices.len() as u32))] + #[pallet::weight(T::WeightInfo::deploy_pool(spot_prices.len().saturated_into()))] #[transactional] pub fn deploy_pool( origin: OriginFor, @@ -521,9 +530,12 @@ mod pallet { ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let asset_count = T::MarketCommons::market(&market_id)?.outcomes(); - ensure!(spot_prices.len() == asset_count as usize, Error::::IncorrectVecLen); + let asset_count_u32: u32 = asset_count.into(); + let spot_prices_len: u32 = + spot_prices.len().try_into().map_err(|_| Error::::NarrowingConversion)?; + ensure!(spot_prices_len == asset_count_u32, Error::::IncorrectVecLen); Self::do_deploy_pool(who, market_id, amount, spot_prices, swap_fee)?; - Ok(Some(T::WeightInfo::deploy_pool(asset_count as u32)).into()) + Ok(Some(T::WeightInfo::deploy_pool(spot_prices_len)).into()) } } @@ -672,8 +684,10 @@ mod pallet { ensure!(pool_shares_amount != Zero::zero(), Error::::ZeroAmount); let market = T::MarketCommons::market(&market_id)?; ensure!(market.status == MarketStatus::Active, Error::::MarketNotActive); - let asset_count = max_amounts_in.len() as u32; - ensure!(asset_count == market.outcomes() as u32, Error::::IncorrectAssetCount); + let asset_count_u16: u16 = + max_amounts_in.len().try_into().map_err(|_| Error::::NarrowingConversion)?; + let asset_count_u32: u32 = asset_count_u16.into(); + ensure!(asset_count_u16 == market.outcomes(), Error::::IncorrectAssetCount); let benchmark_info = Self::try_mutate_pool(&market_id, |pool| { let ratio = pool_shares_amount.bdiv_ceil(pool.liquidity_shares_manager.total_shares()?)?; @@ -712,9 +726,9 @@ mod pallet { Ok(benchmark_info) })?; let weight = match benchmark_info { - BenchmarkInfo::InPlace => T::WeightInfo::join_in_place(asset_count), - BenchmarkInfo::Reassigned => T::WeightInfo::join_reassigned(asset_count), - BenchmarkInfo::Leaf => T::WeightInfo::join_leaf(asset_count), + BenchmarkInfo::InPlace => T::WeightInfo::join_in_place(asset_count_u32), + BenchmarkInfo::Reassigned => T::WeightInfo::join_reassigned(asset_count_u32), + BenchmarkInfo::Leaf => T::WeightInfo::join_leaf(asset_count_u32), }; Ok((Some(weight)).into()) } @@ -832,9 +846,11 @@ mod pallet { let market = T::MarketCommons::market(&market_id)?; ensure!(market.status == MarketStatus::Active, Error::::MarketNotActive); ensure!(market.scoring_rule == ScoringRule::Lmsr, Error::::InvalidTradingMechanism); - let asset_count = spot_prices.len(); - ensure!(asset_count as u16 == market.outcomes(), Error::::IncorrectVecLen); - ensure!(market.outcomes() as u32 <= MaxAssets::get(), Error::::AssetCountAboveMax); + let asset_count_u16: u16 = + spot_prices.len().try_into().map_err(|_| Error::::NarrowingConversion)?; + let asset_count_u32: u32 = asset_count_u16.into(); + ensure!(asset_count_u16 == market.outcomes(), Error::::IncorrectVecLen); + ensure!(asset_count_u32 <= MaxAssets::get(), Error::::AssetCountAboveMax); ensure!(swap_fee >= MIN_SWAP_FEE.saturated_into(), Error::::SwapFeeBelowMin); ensure!(swap_fee <= T::MaxSwapFee::get(), Error::::SwapFeeAboveMax); ensure!( From 58ccf5ed907f245ad57fd3fc6edde95274119aa7 Mon Sep 17 00:00:00 2001 From: Malte Kliemann Date: Mon, 8 Apr 2024 21:53:24 +0200 Subject: [PATCH 7/7] Fix merge --- zrml/prediction-markets/src/mock.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/zrml/prediction-markets/src/mock.rs b/zrml/prediction-markets/src/mock.rs index 70cf236b3..392a7d2df 100644 --- a/zrml/prediction-markets/src/mock.rs +++ b/zrml/prediction-markets/src/mock.rs @@ -42,8 +42,6 @@ use sp_runtime::{ DispatchError, DispatchResult, }; use std::cell::RefCell; -#[cfg(feature = "parachain")] -use zeitgeist_primitives::types::XcmAsset; use zeitgeist_primitives::{ constants::mock::{ AddOutcomePeriod, AggregationPeriod, AppealBond, AppealPeriod, AssetsAccountDeposit,