From d37ad94d630851d49dbc44b261e890265d0f4774 Mon Sep 17 00:00:00 2001 From: maltekliemann Date: Sun, 3 Jul 2022 11:18:26 +0200 Subject: [PATCH 01/10] Add `PoolStatus::Open` --- primitives/src/traits/swaps.rs | 3 ++ zrml/prediction-markets/src/lib.rs | 2 + zrml/swaps/src/lib.rs | 21 +++++++++- zrml/swaps/src/tests.rs | 63 ++++++++++++++++++++++++++---- 4 files changed, 79 insertions(+), 10 deletions(-) diff --git a/primitives/src/traits/swaps.rs b/primitives/src/traits/swaps.rs index 1444703ed..9fd345979 100644 --- a/primitives/src/traits/swaps.rs +++ b/primitives/src/traits/swaps.rs @@ -32,6 +32,7 @@ pub trait Swaps { swap_fee: Option, amount: Option, weights: Option>, + active: Option, ) -> Result; /// Close the specified pool. @@ -58,6 +59,8 @@ pub trait Swaps { /// * `pool_id`: Unique pool identifier associated with the pool to be destroyed. fn destroy_pool_in_subsidy_phase(pool_id: PoolId) -> Result; + fn open_pool(pool_id: PoolId) -> Result; + /// Pool - Exit with exact pool amount /// /// Takes an asset from `pool_id` and transfers to `origin`. Differently from `pool_exit`, diff --git a/zrml/prediction-markets/src/lib.rs b/zrml/prediction-markets/src/lib.rs index 5dcbba938..1b745444c 100644 --- a/zrml/prediction-markets/src/lib.rs +++ b/zrml/prediction-markets/src/lib.rs @@ -637,6 +637,7 @@ mod pallet { Some(Zero::zero()), Some(amount), Some(weights), + Some(true), )?; // This errors if a pool already exists! @@ -2071,6 +2072,7 @@ mod pallet { None, None, None, + None, )?; // This errors if a pool already exists! diff --git a/zrml/swaps/src/lib.rs b/zrml/swaps/src/lib.rs index 3004c68ac..8c8ed94ec 100644 --- a/zrml/swaps/src/lib.rs +++ b/zrml/swaps/src/lib.rs @@ -372,7 +372,6 @@ mod pallet { let pool = Self::pool_by_id(pool_id)?; let pool_account_id = Pallet::::pool_account_id(pool_id); - Self::check_if_pool_is_active(&pool)?; let params = PoolParams { asset_bounds: max_assets_in, event: |evt| Self::deposit_event(Event::PoolJoin(evt)), @@ -769,6 +768,8 @@ mod pallet { InvalidStateTransition, /// Could not create CPMM pool since no weights were supplied. InvalidWeightArgument, + /// Could not create CPMM pool since no status was supplied. + InvalidStatusArgument, /// A transferal of funds into a swaps pool was above a threshhold specified by the sender. LimitIn, /// Subsidy amount is too small. @@ -839,6 +840,8 @@ mod pallet { PoolClosed(PoolId), /// A pool was cleaned up. \[pool_id\] PoolCleanedUp(PoolId), + /// A pool was opened. \[pool_id\] + PoolOpen(PoolId), /// Someone has exited a pool. \[PoolAssetsEvent\] PoolExit( PoolAssetsEvent< @@ -1347,6 +1350,7 @@ mod pallet { swap_fee: Option>, amount: Option>, weights: Option>, + active: Option, ) -> Result { ensure!(assets.len() <= usize::from(T::MaxAssets::get()), Error::::TooManyAssets); ensure!(assets.len() >= usize::from(T::MinAssets::get()), Error::::TooFewAssets); @@ -1379,6 +1383,7 @@ mod pallet { &assets, &weights_unwrapped, )?; + let active_unwrapped = active.ok_or(Error::::InvalidStatusArgument)?; for (asset, weight) in assets.iter().copied().zip(weights_unwrapped) { let free_balance = T::Shares::free_balance(asset, &who); @@ -1399,7 +1404,8 @@ mod pallet { ); T::Shares::deposit(pool_shares_id, &who, amount_unwrapped)?; - let pool_status = PoolStatus::Active; + let pool_status = + if active_unwrapped { PoolStatus::Active } else { PoolStatus::Closed }; let total_subsidy = None; let total_weight = Some(total_weight); let weights = Some(map); @@ -1654,6 +1660,17 @@ mod pallet { }) } + fn open_pool(pool_id: PoolId) -> Result { + Self::mutate_pool(pool_id, |pool| { + ensure!(pool.pool_status == PoolStatus::Closed, Error::::InvalidStateTransition); + pool.pool_status = PoolStatus::Active; + Ok(()) + })?; + Self::deposit_event(Event::PoolOpen(pool_id)); + // TODO(#603): Fix weight calculation! + Ok(T::DbWeight::get().reads_writes(1, 1)) + } + /// Pool - Exit with exact pool amount /// /// Takes an asset from `pool_id` and transfers to `origin`. Differently from `pool_exit`, diff --git a/zrml/swaps/src/tests.rs b/zrml/swaps/src/tests.rs index da91f5837..c778c9eb9 100644 --- a/zrml/swaps/src/tests.rs +++ b/zrml/swaps/src/tests.rs @@ -72,6 +72,7 @@ fn create_pool_fails_with_duplicate_assets(assets: Vec Some(0), Some(::MinLiquidity::get()), Some(vec![_2; asset_count]), + Some(true), ), crate::Error::::SomeIdenticalAssets ); @@ -467,7 +468,7 @@ fn end_subsidy_phase_distributes_shares_and_outcome_assets() { } #[test] -fn nothing_except_exit_pool_is_allowed_in_closed_cpmm_pools() { +fn single_asset_operations_and_swaps_fail_on_closed_cpmm_pools() { ExtBuilder::default().build().execute_with(|| { use zeitgeist_primitives::traits::Swaps as _; create_initial_pool_with_funds_for_alice(ScoringRule::CPMM, true); @@ -498,10 +499,6 @@ fn nothing_except_exit_pool_is_allowed_in_closed_cpmm_pools() { Swaps::pool_exit_with_exact_pool_amount(alice_signed(), 0, ASSET_A, _1, _1_2), crate::Error::::PoolIsNotActive ); - assert_noop!( - Swaps::pool_join(alice_signed(), 0, 0, vec!(_1, _1, _1, _1)), - crate::Error::::PoolIsNotActive - ); assert_noop!( Swaps::pool_join_with_exact_asset_amount(alice_signed(), 0, ASSET_E, 1, 1), crate::Error::::PoolIsNotActive @@ -624,7 +621,8 @@ fn pool_join_amount_satisfies_max_in_ratio_constraints() { ScoringRule::CPMM, Some(0), Some(::MinLiquidity::get()), - Some(vec!(_2, _2, _2, _5)) // Asset weights don't divide total weight. + Some(vec!(_2, _2, _2, _5)), // Asset weights don't divide total weight. + Some(true), )); assert_ok!(Currencies::deposit(ASSET_D, &ALICE, u64::MAX.into())); @@ -1755,6 +1753,7 @@ fn create_pool_fails_on_too_many_assets() { Some(0), Some(::MinLiquidity::get()), Some(weights), + Some(true), ), crate::Error::::TooManyAssets ); @@ -1774,6 +1773,7 @@ fn create_pool_fails_on_too_few_assets() { Some(0), Some(::MinLiquidity::get()), Some(vec!(_2, _2, _2, _2)), + Some(true), ), crate::Error::::TooFewAssets ); @@ -1793,6 +1793,7 @@ fn create_pool_fails_if_base_asset_is_not_in_asset_vector() { Some(0), Some(::MinLiquidity::get()), Some(vec!(_2, _2, _2)), + Some(true), ), crate::Error::::BaseAssetNotFound ); @@ -1846,7 +1847,8 @@ fn create_pool_fails_on_weight_below_minimum_weight() { ScoringRule::CPMM, Some(0), Some(::MinLiquidity::get()), - Some(vec!(_2, ::MinWeight::get() - 1, _2, _2)) + Some(vec!(_2, ::MinWeight::get() - 1, _2, _2)), + Some(true), ), crate::Error::::BelowMinimumWeight, ); @@ -1868,7 +1870,8 @@ fn create_pool_fails_on_weight_above_maximum_weight() { ScoringRule::CPMM, Some(0), Some(::MinLiquidity::get()), - Some(vec!(_2, ::MaxWeight::get() + 1, _2, _2)) + Some(vec!(_2, ::MaxWeight::get() + 1, _2, _2)), + Some(true), ), crate::Error::::AboveMaximumWeight, ); @@ -1892,6 +1895,7 @@ fn create_pool_fails_on_total_weight_above_maximum_total_weight() { Some(0), Some(::MinLiquidity::get()), Some(vec![weight; 4]), + Some(true), ), crate::Error::::MaxTotalWeight, ); @@ -1914,6 +1918,7 @@ fn create_pool_fails_on_insufficient_liquidity() { Some(0), Some(::MinLiquidity::get() - 1), Some(vec!(_2, _2, _2, _2)), + Some(true), ), crate::Error::::InsufficientLiquidity, ); @@ -1935,6 +1940,7 @@ fn create_pool_transfers_the_correct_amount_of_tokens() { Some(0), Some(_1234), Some(vec!(_2, _2, _2, _2)), + Some(true), )); let pool_shares_id = Swaps::pool_shares_id(0); @@ -1987,6 +1993,45 @@ fn close_pool_succeeds_and_emits_correct_event_if_pool_exists() { }); } +#[test] +fn open_pool_fails_if_pool_does_not_exist() { + ExtBuilder::default().build().execute_with(|| { + assert_noop!(Swaps::open_pool(0), crate::Error::::PoolDoesNotExist); + }); +} + +#[test_case(PoolStatus::Active; "active")] +#[test_case(PoolStatus::Clean; "clean")] +#[test_case(PoolStatus::CollectingSubsidy; "collecting_subsidy")] +fn open_pool_fails_if_pool_is_not_closed(pool_status: PoolStatus) { + ExtBuilder::default().build().execute_with(|| { + create_initial_pool(ScoringRule::CPMM, true); + let pool_id = 0; + assert_ok!(Swaps::mutate_pool(pool_id, |pool| { + pool.pool_status = pool_status; + Ok(()) + })); + assert_noop!(Swaps::open_pool(0), crate::Error::::InvalidStateTransition); + }); +} + +#[test] +fn open_pool_succeeds_and_emits_correct_event_if_pool_exists() { + ExtBuilder::default().build().execute_with(|| { + frame_system::Pallet::::set_block_number(1); + create_initial_pool(ScoringRule::CPMM, true); + let pool_id = 0; + assert_ok!(Swaps::mutate_pool(pool_id, |pool| { + pool.pool_status = PoolStatus::Closed; + Ok(()) + })); + assert_ok!(Swaps::open_pool(pool_id)); + let pool = Swaps::pool(pool_id).unwrap(); + assert_eq!(pool.pool_status, PoolStatus::Active); + System::assert_last_event(Event::PoolOpen(pool_id).into()); + }); +} + #[test] fn pool_join_fails_if_max_assets_in_is_violated() { ExtBuilder::default().build().execute_with(|| { @@ -2107,6 +2152,7 @@ fn create_pool_correctly_associates_weights_with_assets() { Some(0), Some(::MinLiquidity::get()), Some(vec!(_1, _2, _3, _4)), + Some(true), )); let pool = Swaps::pool(0).unwrap(); let pool_weights = pool.weights.unwrap(); @@ -2141,6 +2187,7 @@ fn create_initial_pool(scoring_rule: ScoringRule, deposit: bool) { None }, if scoring_rule == ScoringRule::CPMM { Some(vec!(_2, _2, _2, _2)) } else { None }, + if scoring_rule == ScoringRule::CPMM { Some(true) } else { None }, )); } From 494c5fc32f19add60ebec2f18aa8aff5e966a9e4 Mon Sep 17 00:00:00 2001 From: maltekliemann Date: Mon, 4 Jul 2022 14:15:26 +0200 Subject: [PATCH 02/10] Fix benchmark --- zrml/swaps/src/benchmarks.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/zrml/swaps/src/benchmarks.rs b/zrml/swaps/src/benchmarks.rs index 2a78b7a75..1143d33cf 100644 --- a/zrml/swaps/src/benchmarks.rs +++ b/zrml/swaps/src/benchmarks.rs @@ -102,6 +102,7 @@ fn bench_create_pool( if scoring_rule == ScoringRule::CPMM { Some(Zero::zero()) } else { None }, if scoring_rule == ScoringRule::CPMM { Some(T::MinLiquidity::get()) } else { None }, if scoring_rule == ScoringRule::CPMM { Some(weights) } else { None }, + if scoring_rule == ScoringRule::CPMM { Some(true) } else { None }, ) .unwrap(); let pool_id = >::get() - 1; From 7dcbaad1f0b33411c9816c9ca54fb400693577ca Mon Sep 17 00:00:00 2001 From: maltekliemann Date: Wed, 6 Jul 2022 17:26:36 +0200 Subject: [PATCH 03/10] Add new pool status `Initialized` --- primitives/src/pool_status.rs | 2 + primitives/src/traits/swaps.rs | 1 - zrml/prediction-markets/src/lib.rs | 3 +- zrml/swaps/src/benchmarks.rs | 1 - zrml/swaps/src/lib.rs | 14 +++--- zrml/swaps/src/tests.rs | 71 +++++++++++++++++++----------- 6 files changed, 55 insertions(+), 37 deletions(-) diff --git a/primitives/src/pool_status.rs b/primitives/src/pool_status.rs index a22b41320..122415d2b 100644 --- a/primitives/src/pool_status.rs +++ b/primitives/src/pool_status.rs @@ -23,4 +23,6 @@ pub enum PoolStatus { Closed, /// The pool has been cleaned up, usually after the corresponding market has been resolved. Clean, + /// The pool has just been created. + Initialized, } diff --git a/primitives/src/traits/swaps.rs b/primitives/src/traits/swaps.rs index 9fd345979..b891a30ac 100644 --- a/primitives/src/traits/swaps.rs +++ b/primitives/src/traits/swaps.rs @@ -32,7 +32,6 @@ pub trait Swaps { swap_fee: Option, amount: Option, weights: Option>, - active: Option, ) -> Result; /// Close the specified pool. diff --git a/zrml/prediction-markets/src/lib.rs b/zrml/prediction-markets/src/lib.rs index 1b745444c..aaeaa8391 100644 --- a/zrml/prediction-markets/src/lib.rs +++ b/zrml/prediction-markets/src/lib.rs @@ -637,8 +637,8 @@ mod pallet { Some(Zero::zero()), Some(amount), Some(weights), - Some(true), )?; + T::Swaps::open_pool(pool_id)?; // This errors if a pool already exists! T::MarketCommons::insert_market_pool(market_id, pool_id)?; @@ -2072,7 +2072,6 @@ mod pallet { None, None, None, - None, )?; // This errors if a pool already exists! diff --git a/zrml/swaps/src/benchmarks.rs b/zrml/swaps/src/benchmarks.rs index 1143d33cf..2a78b7a75 100644 --- a/zrml/swaps/src/benchmarks.rs +++ b/zrml/swaps/src/benchmarks.rs @@ -102,7 +102,6 @@ fn bench_create_pool( if scoring_rule == ScoringRule::CPMM { Some(Zero::zero()) } else { None }, if scoring_rule == ScoringRule::CPMM { Some(T::MinLiquidity::get()) } else { None }, if scoring_rule == ScoringRule::CPMM { Some(weights) } else { None }, - if scoring_rule == ScoringRule::CPMM { Some(true) } else { None }, ) .unwrap(); let pool_id = >::get() - 1; diff --git a/zrml/swaps/src/lib.rs b/zrml/swaps/src/lib.rs index 8c8ed94ec..ed7031013 100644 --- a/zrml/swaps/src/lib.rs +++ b/zrml/swaps/src/lib.rs @@ -841,7 +841,7 @@ mod pallet { /// A pool was cleaned up. \[pool_id\] PoolCleanedUp(PoolId), /// A pool was opened. \[pool_id\] - PoolOpen(PoolId), + PoolActive(PoolId), /// Someone has exited a pool. \[PoolAssetsEvent\] PoolExit( PoolAssetsEvent< @@ -1350,7 +1350,6 @@ mod pallet { swap_fee: Option>, amount: Option>, weights: Option>, - active: Option, ) -> Result { ensure!(assets.len() <= usize::from(T::MaxAssets::get()), Error::::TooManyAssets); ensure!(assets.len() >= usize::from(T::MinAssets::get()), Error::::TooFewAssets); @@ -1383,7 +1382,6 @@ mod pallet { &assets, &weights_unwrapped, )?; - let active_unwrapped = active.ok_or(Error::::InvalidStatusArgument)?; for (asset, weight) in assets.iter().copied().zip(weights_unwrapped) { let free_balance = T::Shares::free_balance(asset, &who); @@ -1404,8 +1402,7 @@ mod pallet { ); T::Shares::deposit(pool_shares_id, &who, amount_unwrapped)?; - let pool_status = - if active_unwrapped { PoolStatus::Active } else { PoolStatus::Closed }; + let pool_status = PoolStatus::Initialized; let total_subsidy = None; let total_weight = Some(total_weight); let weights = Some(map); @@ -1662,11 +1659,14 @@ mod pallet { fn open_pool(pool_id: PoolId) -> Result { Self::mutate_pool(pool_id, |pool| { - ensure!(pool.pool_status == PoolStatus::Closed, Error::::InvalidStateTransition); + ensure!( + pool.pool_status == PoolStatus::Initialized, + Error::::InvalidStateTransition + ); pool.pool_status = PoolStatus::Active; Ok(()) })?; - Self::deposit_event(Event::PoolOpen(pool_id)); + Self::deposit_event(Event::PoolActive(pool_id)); // TODO(#603): Fix weight calculation! Ok(T::DbWeight::get().reads_writes(1, 1)) } diff --git a/zrml/swaps/src/tests.rs b/zrml/swaps/src/tests.rs index c778c9eb9..44458e4c4 100644 --- a/zrml/swaps/src/tests.rs +++ b/zrml/swaps/src/tests.rs @@ -72,7 +72,6 @@ fn create_pool_fails_with_duplicate_assets(assets: Vec Some(0), Some(::MinLiquidity::get()), Some(vec![_2; asset_count]), - Some(true), ), crate::Error::::SomeIdenticalAssets ); @@ -249,7 +248,20 @@ fn create_pool_generates_a_new_pool_with_correct_parameters_for_cpmm() { let next_pool_before = Swaps::next_pool_id(); assert_eq!(next_pool_before, 0); - create_initial_pool(ScoringRule::CPMM, true); + let amount = ::MinLiquidity::get(); + ASSETS.iter().cloned().for_each(|asset| { + assert_ok!(Currencies::deposit(asset, &BOB, amount)); + }); + assert_ok!(Swaps::create_pool( + BOB, + ASSETS.iter().cloned().collect(), + ASSETS.last().unwrap().clone(), + 0, + ScoringRule::CPMM, + Some(1), + Some(amount), + Some(vec!(_4, _3, _2, _1)), + )); let next_pool_after = Swaps::next_pool_id(); assert_eq!(next_pool_after, 1); @@ -258,21 +270,21 @@ fn create_pool_generates_a_new_pool_with_correct_parameters_for_cpmm() { assert_eq!(pool.assets, ASSETS.iter().cloned().collect::>()); assert_eq!(pool.scoring_rule, ScoringRule::CPMM); - assert_eq!(pool.swap_fee.unwrap(), 0); + assert_eq!(pool.swap_fee, Some(1)); assert_eq!(pool.total_subsidy, None); - assert_eq!(pool.total_weight.unwrap(), _8); + assert_eq!(pool.total_weight.unwrap(), _10); - assert_eq!(*pool.weights.as_ref().unwrap().get(&ASSET_A).unwrap(), _2); - assert_eq!(*pool.weights.as_ref().unwrap().get(&ASSET_B).unwrap(), _2); + assert_eq!(*pool.weights.as_ref().unwrap().get(&ASSET_A).unwrap(), _4); + assert_eq!(*pool.weights.as_ref().unwrap().get(&ASSET_B).unwrap(), _3); assert_eq!(*pool.weights.as_ref().unwrap().get(&ASSET_C).unwrap(), _2); - assert_eq!(*pool.weights.as_ref().unwrap().get(&ASSET_D).unwrap(), _2); + assert_eq!(*pool.weights.as_ref().unwrap().get(&ASSET_D).unwrap(), _1); let pool_account = Swaps::pool_account_id(0); System::assert_last_event( Event::PoolCreate( CommonPoolEventParams { pool_id: next_pool_before, who: BOB }, pool, - ::MinLiquidity::get(), + amount, pool_account, ) .into(), @@ -622,15 +634,16 @@ fn pool_join_amount_satisfies_max_in_ratio_constraints() { Some(0), Some(::MinLiquidity::get()), Some(vec!(_2, _2, _2, _5)), // Asset weights don't divide total weight. - Some(true), )); + let pool_id = 0; + assert_ok!(Swaps::open_pool(pool_id)); assert_ok!(Currencies::deposit(ASSET_D, &ALICE, u64::MAX.into())); assert_noop!( Swaps::pool_join_with_exact_pool_amount( alice_signed(), - 0, + pool_id, ASSET_A, _100, _10000 // Don't care how much we have to pay! @@ -1402,6 +1415,7 @@ fn clean_up_pool_handles_rikiddo_pools_properly() { #[test_case(PoolStatus::Active; "active")] #[test_case(PoolStatus::Clean; "clean")] #[test_case(PoolStatus::CollectingSubsidy; "collecting_subsidy")] +#[test_case(PoolStatus::Initialized; "initialized")] fn clean_up_pool_fails_if_pool_is_not_closed(pool_status: PoolStatus) { ExtBuilder::default().build().execute_with(|| { create_initial_pool(ScoringRule::RikiddoSigmoidFeeMarketEma, false); @@ -1753,7 +1767,6 @@ fn create_pool_fails_on_too_many_assets() { Some(0), Some(::MinLiquidity::get()), Some(weights), - Some(true), ), crate::Error::::TooManyAssets ); @@ -1773,7 +1786,6 @@ fn create_pool_fails_on_too_few_assets() { Some(0), Some(::MinLiquidity::get()), Some(vec!(_2, _2, _2, _2)), - Some(true), ), crate::Error::::TooFewAssets ); @@ -1793,7 +1805,6 @@ fn create_pool_fails_if_base_asset_is_not_in_asset_vector() { Some(0), Some(::MinLiquidity::get()), Some(vec!(_2, _2, _2)), - Some(true), ), crate::Error::::BaseAssetNotFound ); @@ -1848,7 +1859,6 @@ fn create_pool_fails_on_weight_below_minimum_weight() { Some(0), Some(::MinLiquidity::get()), Some(vec!(_2, ::MinWeight::get() - 1, _2, _2)), - Some(true), ), crate::Error::::BelowMinimumWeight, ); @@ -1871,7 +1881,6 @@ fn create_pool_fails_on_weight_above_maximum_weight() { Some(0), Some(::MinLiquidity::get()), Some(vec!(_2, ::MaxWeight::get() + 1, _2, _2)), - Some(true), ), crate::Error::::AboveMaximumWeight, ); @@ -1895,7 +1904,6 @@ fn create_pool_fails_on_total_weight_above_maximum_total_weight() { Some(0), Some(::MinLiquidity::get()), Some(vec![weight; 4]), - Some(true), ), crate::Error::::MaxTotalWeight, ); @@ -1918,7 +1926,6 @@ fn create_pool_fails_on_insufficient_liquidity() { Some(0), Some(::MinLiquidity::get() - 1), Some(vec!(_2, _2, _2, _2)), - Some(true), ), crate::Error::::InsufficientLiquidity, ); @@ -1940,7 +1947,6 @@ fn create_pool_transfers_the_correct_amount_of_tokens() { Some(0), Some(_1234), Some(vec!(_2, _2, _2, _2)), - Some(true), )); let pool_shares_id = Swaps::pool_shares_id(0); @@ -1968,6 +1974,7 @@ fn close_pool_fails_if_pool_does_not_exist() { #[test_case(PoolStatus::Closed; "closed")] #[test_case(PoolStatus::Clean; "clean")] #[test_case(PoolStatus::CollectingSubsidy; "collecting_subsidy")] +#[test_case(PoolStatus::Initialized; "initialized")] fn close_pool_fails_if_pool_is_not_active(pool_status: PoolStatus) { ExtBuilder::default().build().execute_with(|| { create_initial_pool(ScoringRule::CPMM, true); @@ -2003,6 +2010,7 @@ fn open_pool_fails_if_pool_does_not_exist() { #[test_case(PoolStatus::Active; "active")] #[test_case(PoolStatus::Clean; "clean")] #[test_case(PoolStatus::CollectingSubsidy; "collecting_subsidy")] +#[test_case(PoolStatus::Closed; "closed")] fn open_pool_fails_if_pool_is_not_closed(pool_status: PoolStatus) { ExtBuilder::default().build().execute_with(|| { create_initial_pool(ScoringRule::CPMM, true); @@ -2019,16 +2027,25 @@ fn open_pool_fails_if_pool_is_not_closed(pool_status: PoolStatus) { fn open_pool_succeeds_and_emits_correct_event_if_pool_exists() { ExtBuilder::default().build().execute_with(|| { frame_system::Pallet::::set_block_number(1); - create_initial_pool(ScoringRule::CPMM, true); + let amount = ::MinLiquidity::get(); + ASSETS.iter().cloned().for_each(|asset| { + assert_ok!(Currencies::deposit(asset, &BOB, amount)); + }); + assert_ok!(Swaps::create_pool( + BOB, + vec![ASSET_D, ASSET_B, ASSET_C, ASSET_A], + ASSET_A, + 0, + ScoringRule::CPMM, + Some(0), + Some(amount), + Some(vec!(_1, _2, _3, _4)), + )); let pool_id = 0; - assert_ok!(Swaps::mutate_pool(pool_id, |pool| { - pool.pool_status = PoolStatus::Closed; - Ok(()) - })); assert_ok!(Swaps::open_pool(pool_id)); let pool = Swaps::pool(pool_id).unwrap(); assert_eq!(pool.pool_status, PoolStatus::Active); - System::assert_last_event(Event::PoolOpen(pool_id).into()); + System::assert_last_event(Event::PoolActive(pool_id).into()); }); } @@ -2152,7 +2169,6 @@ fn create_pool_correctly_associates_weights_with_assets() { Some(0), Some(::MinLiquidity::get()), Some(vec!(_1, _2, _3, _4)), - Some(true), )); let pool = Swaps::pool(0).unwrap(); let pool_weights = pool.weights.unwrap(); @@ -2174,6 +2190,7 @@ fn create_initial_pool(scoring_rule: ScoringRule, deposit: bool) { }); } + let pool_id = Swaps::next_pool_id(); assert_ok!(Swaps::create_pool( BOB, ASSETS.iter().cloned().collect(), @@ -2187,8 +2204,10 @@ fn create_initial_pool(scoring_rule: ScoringRule, deposit: bool) { None }, if scoring_rule == ScoringRule::CPMM { Some(vec!(_2, _2, _2, _2)) } else { None }, - if scoring_rule == ScoringRule::CPMM { Some(true) } else { None }, )); + if scoring_rule == ScoringRule::CPMM { + assert_ok!(Swaps::open_pool(pool_id)); + } } fn create_initial_pool_with_funds_for_alice(scoring_rule: ScoringRule, deposit: bool) { From 66cde95e305f2eca1e2fff0f323df46cd5a0f7db Mon Sep 17 00:00:00 2001 From: maltekliemann Date: Wed, 6 Jul 2022 18:02:25 +0200 Subject: [PATCH 04/10] Fix benchmark --- zrml/swaps/src/benchmarks.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/zrml/swaps/src/benchmarks.rs b/zrml/swaps/src/benchmarks.rs index 2a78b7a75..5aa69d7af 100644 --- a/zrml/swaps/src/benchmarks.rs +++ b/zrml/swaps/src/benchmarks.rs @@ -106,6 +106,10 @@ fn bench_create_pool( .unwrap(); let pool_id = >::get() - 1; + if scoring_rule == ScoringRule::CPMM { + let _ = Pallet::::open_pool(pool_id); + } + if subsidize { let min_subsidy = T::MinSubsidy::get(); T::Shares::deposit(base_asset, &caller, min_subsidy).unwrap(); From 1ca7260194e61289d03b38c1bd1a6f1cb32c3959 Mon Sep 17 00:00:00 2001 From: maltekliemann Date: Thu, 7 Jul 2022 12:06:34 +0200 Subject: [PATCH 05/10] Fix `types.json` --- misc/types.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/misc/types.json b/misc/types.json index c7b8b4a43..7f118d17b 100644 --- a/misc/types.json +++ b/misc/types.json @@ -288,7 +288,9 @@ "_enum": [ "Active", "CollectingSubsidy", - "Stale" + "Closed", + "Clean", + "Initialized" ] }, "RegistrationInfo": { From 2a5dbe0325d3c817cf2e4df3563118aa9e8dd40b Mon Sep 17 00:00:00 2001 From: Malte Kliemann Date: Wed, 13 Jul 2022 13:17:35 +0200 Subject: [PATCH 06/10] Update zrml/swaps/src/lib.rs Co-authored-by: Chralt --- zrml/swaps/src/lib.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/zrml/swaps/src/lib.rs b/zrml/swaps/src/lib.rs index 7cde0429c..e17bba197 100644 --- a/zrml/swaps/src/lib.rs +++ b/zrml/swaps/src/lib.rs @@ -771,8 +771,6 @@ mod pallet { InvalidStateTransition, /// Could not create CPMM pool since no weights were supplied. InvalidWeightArgument, - /// Could not create CPMM pool since no status was supplied. - InvalidStatusArgument, /// A transferal of funds into a swaps pool was above a threshhold specified by the sender. LimitIn, /// Subsidy amount is too small. From 876c8543976e59ff85fcf88e315a42eefb0a9ca0 Mon Sep 17 00:00:00 2001 From: Malte Kliemann Date: Wed, 13 Jul 2022 13:17:49 +0200 Subject: [PATCH 07/10] Update zrml/swaps/src/tests.rs Co-authored-by: Chralt --- zrml/swaps/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zrml/swaps/src/tests.rs b/zrml/swaps/src/tests.rs index 248a868ba..a23dcb9f2 100644 --- a/zrml/swaps/src/tests.rs +++ b/zrml/swaps/src/tests.rs @@ -2265,7 +2265,7 @@ fn open_pool_fails_if_pool_is_not_closed(pool_status: PoolStatus) { pool.pool_status = pool_status; Ok(()) })); - assert_noop!(Swaps::open_pool(0), crate::Error::::InvalidStateTransition); + assert_noop!(Swaps::open_pool(pool_id), crate::Error::::InvalidStateTransition); }); } From 54204c9affbbde4dd321e82413ce02125e59bf47 Mon Sep 17 00:00:00 2001 From: maltekliemann Date: Wed, 13 Jul 2022 13:23:57 +0200 Subject: [PATCH 08/10] Extend pool parameters checks --- zrml/swaps/src/tests.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/zrml/swaps/src/tests.rs b/zrml/swaps/src/tests.rs index a23dcb9f2..65c01875c 100644 --- a/zrml/swaps/src/tests.rs +++ b/zrml/swaps/src/tests.rs @@ -273,13 +273,14 @@ fn create_pool_generates_a_new_pool_with_correct_parameters_for_cpmm() { assert_eq!(next_pool_before, 0); let amount = ::MinLiquidity::get(); + let base_asset = ASSETS.last().unwrap(); ASSETS.iter().cloned().for_each(|asset| { assert_ok!(Currencies::deposit(asset, &BOB, amount)); }); assert_ok!(Swaps::create_pool( BOB, ASSETS.iter().cloned().collect(), - ASSETS.last().unwrap().clone(), + *base_asset, 0, ScoringRule::CPMM, Some(1), @@ -292,7 +293,10 @@ fn create_pool_generates_a_new_pool_with_correct_parameters_for_cpmm() { let pool = Swaps::pools(0).unwrap(); - assert_eq!(pool.assets, ASSETS.iter().cloned().collect::>()); + assert_eq!(pool.assets, ASSETS); + assert_eq!(pool.base_asset, *base_asset); + assert_eq!(pool.market_id, 0); + assert_eq!(pool.pool_status, PoolStatus::Initialized); assert_eq!(pool.scoring_rule, ScoringRule::CPMM); assert_eq!(pool.swap_fee, Some(1)); assert_eq!(pool.total_subsidy, None); From 95a7add9c2f844018e11c8fc9f195426a2ae135b Mon Sep 17 00:00:00 2001 From: maltekliemann Date: Thu, 14 Jul 2022 19:39:26 +0200 Subject: [PATCH 09/10] Fix status errors --- zrml/swaps/src/lib.rs | 6 +++ zrml/swaps/src/tests.rs | 98 +++++++++++++++++++++++++++++++++++------ 2 files changed, 90 insertions(+), 14 deletions(-) diff --git a/zrml/swaps/src/lib.rs b/zrml/swaps/src/lib.rs index e17bba197..80443a74d 100644 --- a/zrml/swaps/src/lib.rs +++ b/zrml/swaps/src/lib.rs @@ -370,6 +370,10 @@ mod pallet { ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let pool = Self::pool_by_id(pool_id)?; + ensure!( + matches!(pool.pool_status, PoolStatus::Initialized | PoolStatus::Active), + Error::::InvalidPoolStatus, + ); let pool_account_id = Pallet::::pool_account_id(pool_id); let params = PoolParams { @@ -764,6 +768,8 @@ mod pallet { InvalidAmountArgument, /// Could not create CPMM pool since no fee was supplied. InvalidFeeArgument, + /// Dispatch called on pool with invalid status. + InvalidPoolStatus, /// A function that is only valid for pools with specific scoring rules was called for a /// pool with another scoring rule. InvalidScoringRule, diff --git a/zrml/swaps/src/tests.rs b/zrml/swaps/src/tests.rs index 65c01875c..ae0858e5a 100644 --- a/zrml/swaps/src/tests.rs +++ b/zrml/swaps/src/tests.rs @@ -515,21 +515,88 @@ fn end_subsidy_phase_distributes_shares_and_outcome_assets() { }); } -#[test] -fn single_asset_operations_and_swaps_fail_on_closed_cpmm_pools() { +#[test_case(PoolStatus::Initialized; "Initialized")] +#[test_case(PoolStatus::Closed; "Closed")] +#[test_case(PoolStatus::Clean; "Clean")] +fn single_asset_operations_and_swaps_fail_on_invalid_status_before_clean(status: PoolStatus) { ExtBuilder::default().build().execute_with(|| { - use zeitgeist_primitives::traits::Swaps as _; create_initial_pool_with_funds_for_alice(ScoringRule::CPMM, Some(0), true); + let pool_id = 0; // For this test, we need to give Alice some pool shares, as well. We don't do this in // `create_initial_pool_...` so that there are exacly 100 pool shares, making computations // in other tests easier. - let _ = Currencies::deposit(Swaps::pool_shares_id(0), &ALICE, _25); - assert_ok!(Swaps::pool_join(alice_signed(), 0, _1, vec!(_1, _1, _1, _1),)); + assert_ok!(Currencies::deposit(Swaps::pool_shares_id(0), &ALICE, _25)); + assert_ok!(Swaps::mutate_pool(pool_id, |pool| { + pool.pool_status = status; + Ok(()) + })); - assert_ok!(Swaps::close_pool(0)); + assert_noop!( + Swaps::pool_exit_with_exact_asset_amount(alice_signed(), pool_id, ASSET_A, _1, _2), + crate::Error::::PoolIsNotActive + ); + assert_noop!( + Swaps::pool_exit_with_exact_pool_amount(alice_signed(), pool_id, ASSET_A, _1, _1_2), + crate::Error::::PoolIsNotActive + ); + assert_noop!( + Swaps::pool_join_with_exact_asset_amount(alice_signed(), pool_id, ASSET_E, 1, 1), + crate::Error::::PoolIsNotActive + ); + assert_noop!( + Swaps::pool_join_with_exact_pool_amount(alice_signed(), pool_id, ASSET_E, 1, 1), + crate::Error::::PoolIsNotActive + ); + assert_ok!(Currencies::deposit(ASSET_A, &ALICE, u64::MAX.into())); + assert_noop!( + Swaps::swap_exact_amount_in( + alice_signed(), + pool_id, + ASSET_A, + u64::MAX.into(), + ASSET_B, + Some(_1), + Some(_1), + ), + crate::Error::::PoolIsNotActive + ); + assert_noop!( + Swaps::swap_exact_amount_out( + alice_signed(), + pool_id, + ASSET_A, + Some(u64::MAX.into()), + ASSET_B, + _1, + Some(_1), + ), + crate::Error::::PoolIsNotActive + ); + }); +} + +#[test] +fn pool_join_fails_if_pool_is_closed() { + ExtBuilder::default().build().execute_with(|| { + create_initial_pool_with_funds_for_alice(ScoringRule::CPMM, Some(0), true); + let pool_id = 0; + assert_ok!(Swaps::close_pool(pool_id)); + assert_noop!( + Swaps::pool_join(Origin::signed(ALICE), pool_id, _1, vec![_1, _1, _1, _1]), + crate::Error::::InvalidPoolStatus, + ); + }); +} + +#[test] +fn most_operations_fail_if_pool_is_clean() { + ExtBuilder::default().build().execute_with(|| { + create_initial_pool_with_funds_for_alice(ScoringRule::CPMM, Some(0), true); + let pool_id = 0; + assert_ok!(Swaps::close_pool(pool_id)); assert_ok!(Swaps::clean_up_pool( &MarketType::Categorical(0), - 0, + pool_id, &OutcomeReport::Categorical(if let Asset::CategoricalOutcome(_, idx) = ASSET_A { idx } else { @@ -538,28 +605,31 @@ fn single_asset_operations_and_swaps_fail_on_closed_cpmm_pools() { &Default::default() )); - assert_ok!(Swaps::pool_exit(alice_signed(), 0, _1, vec!(_1_2, _1_2))); assert_noop!( - Swaps::pool_exit_with_exact_asset_amount(alice_signed(), 0, ASSET_A, _1, _2), + Swaps::pool_join(Origin::signed(ALICE), pool_id, _1, vec![_10]), + crate::Error::::InvalidPoolStatus, + ); + assert_noop!( + Swaps::pool_exit_with_exact_asset_amount(alice_signed(), pool_id, ASSET_A, _1, _2), crate::Error::::PoolIsNotActive ); assert_noop!( - Swaps::pool_exit_with_exact_pool_amount(alice_signed(), 0, ASSET_A, _1, _1_2), + Swaps::pool_exit_with_exact_pool_amount(alice_signed(), pool_id, ASSET_A, _1, _1_2), crate::Error::::PoolIsNotActive ); assert_noop!( - Swaps::pool_join_with_exact_asset_amount(alice_signed(), 0, ASSET_E, 1, 1), + Swaps::pool_join_with_exact_asset_amount(alice_signed(), pool_id, ASSET_E, 1, 1), crate::Error::::PoolIsNotActive ); assert_noop!( - Swaps::pool_join_with_exact_pool_amount(alice_signed(), 0, ASSET_E, 1, 1), + Swaps::pool_join_with_exact_pool_amount(alice_signed(), pool_id, ASSET_E, 1, 1), crate::Error::::PoolIsNotActive ); assert_ok!(Currencies::deposit(ASSET_A, &ALICE, u64::MAX.into())); assert_noop!( Swaps::swap_exact_amount_in( alice_signed(), - 0, + pool_id, ASSET_A, u64::MAX.into(), ASSET_B, @@ -571,7 +641,7 @@ fn single_asset_operations_and_swaps_fail_on_closed_cpmm_pools() { assert_noop!( Swaps::swap_exact_amount_out( alice_signed(), - 0, + pool_id, ASSET_A, Some(u64::MAX.into()), ASSET_B, From db426719167ccd4ba59e4f0b0e7c2a6fcf3d45a9 Mon Sep 17 00:00:00 2001 From: maltekliemann Date: Sun, 17 Jul 2022 14:40:52 +0200 Subject: [PATCH 10/10] Update changelog --- docs/changelog_for_devs.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/changelog_for_devs.md b/docs/changelog_for_devs.md index 46b06fe87..0b87cc540 100644 --- a/docs/changelog_for_devs.md +++ b/docs/changelog_for_devs.md @@ -1,3 +1,10 @@ +# v0.3.5 + +- Added `Initialized` status for pools. A pool now starts in `Initialized` + status and must be opened using `Swaps::open_pool`. While the pool is + `Initialized`, it is allowed to call `pool_join` and `pool_exit`, but trading + and single-asset operations are prohibited. + # v0.3.4 - Implemented swap fees for CPMM pools. This means that the following extrinsics