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 diff --git a/misc/types.json b/misc/types.json index 44002553e..171eef842 100644 --- a/misc/types.json +++ b/misc/types.json @@ -288,7 +288,9 @@ "_enum": [ "Active", "CollectingSubsidy", - "Stale" + "Closed", + "Clean", + "Initialized" ] }, "RegistrationInfo": { 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 1444703ed..b891a30ac 100644 --- a/primitives/src/traits/swaps.rs +++ b/primitives/src/traits/swaps.rs @@ -58,6 +58,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 d4f7f3224..ed47e0c9f 100644 --- a/zrml/prediction-markets/src/lib.rs +++ b/zrml/prediction-markets/src/lib.rs @@ -672,6 +672,7 @@ mod pallet { Some(amount), Some(weights), )?; + T::Swaps::open_pool(pool_id)?; // This errors if a pool already exists! T::MarketCommons::insert_market_pool(market_id, pool_id)?; diff --git a/zrml/swaps/src/benchmarks.rs b/zrml/swaps/src/benchmarks.rs index 63b3acb40..aa6b1911e 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::AssetManager::deposit(base_asset, &caller, min_subsidy).unwrap(); diff --git a/zrml/swaps/src/lib.rs b/zrml/swaps/src/lib.rs index 93445f414..42a062336 100644 --- a/zrml/swaps/src/lib.rs +++ b/zrml/swaps/src/lib.rs @@ -371,9 +371,12 @@ 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); - Self::check_if_pool_is_active(&pool)?; let params = PoolParams { asset_bounds: max_assets_in, event: |evt| Self::deposit_event(Event::PoolJoin(evt)), @@ -766,6 +769,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, @@ -847,6 +852,8 @@ mod pallet { PoolClosed(PoolId), /// A pool was cleaned up. \[pool_id\] PoolCleanedUp(PoolId), + /// A pool was opened. \[pool_id\] + PoolActive(PoolId), /// Someone has exited a pool. \[PoolAssetsEvent\] PoolExit( PoolAssetsEvent< @@ -1423,7 +1430,7 @@ mod pallet { ); T::AssetManager::deposit(pool_shares_id, &who, amount_unwrapped)?; - let pool_status = PoolStatus::Active; + let pool_status = PoolStatus::Initialized; let total_subsidy = None; let total_weight = Some(total_weight); let weights = Some(map); @@ -1684,6 +1691,20 @@ mod pallet { }) } + fn open_pool(pool_id: PoolId) -> Result { + Self::mutate_pool(pool_id, |pool| { + ensure!( + pool.pool_status == PoolStatus::Initialized, + Error::::InvalidStateTransition + ); + pool.pool_status = PoolStatus::Active; + Ok(()) + })?; + Self::deposit_event(Event::PoolActive(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 ffc6e07e8..ae0858e5a 100644 --- a/zrml/swaps/src/tests.rs +++ b/zrml/swaps/src/tests.rs @@ -272,30 +272,47 @@ 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, Some(0), true); + 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(), + *base_asset, + 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); 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.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(), @@ -498,21 +515,88 @@ fn end_subsidy_phase_distributes_shares_and_outcome_assets() { }); } -#[test] -fn nothing_except_exit_pool_is_allowed_in_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 { @@ -521,32 +605,31 @@ fn nothing_except_exit_pool_is_allowed_in_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), - crate::Error::::PoolIsNotActive + Swaps::pool_join(Origin::signed(ALICE), pool_id, _1, vec![_10]), + crate::Error::::InvalidPoolStatus, ); assert_noop!( - Swaps::pool_exit_with_exact_pool_amount(alice_signed(), 0, ASSET_A, _1, _1_2), + Swaps::pool_exit_with_exact_asset_amount(alice_signed(), pool_id, ASSET_A, _1, _2), crate::Error::::PoolIsNotActive ); assert_noop!( - Swaps::pool_join(alice_signed(), 0, 0, vec!(_1, _1, _1, _1)), + 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, @@ -558,7 +641,7 @@ fn nothing_except_exit_pool_is_allowed_in_closed_cpmm_pools() { assert_noop!( Swaps::swap_exact_amount_out( alice_signed(), - 0, + pool_id, ASSET_A, Some(u64::MAX.into()), ASSET_B, @@ -703,15 +786,17 @@ 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. )); + 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! @@ -1492,6 +1577,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, None, false); @@ -1587,6 +1673,8 @@ fn swap_exact_amount_in_exchanges_correct_values_with_cpmm_with_fees() { Some(::MinLiquidity::get()), Some(vec!(_2, _2, _2, _2)), )); + let pool_id = 0; + assert_ok!(Swaps::open_pool(pool_id)); let asset_bound = Some(_1 / 2); let max_price = Some(_2); @@ -1596,7 +1684,7 @@ fn swap_exact_amount_in_exchanges_correct_values_with_cpmm_with_fees() { let asset_amount_out = 9_900_990_100; assert_ok!(Swaps::swap_exact_amount_in( alice_signed(), - 0, + pool_id, ASSET_A, asset_amount_in, ASSET_B, @@ -1610,7 +1698,7 @@ fn swap_exact_amount_in_exchanges_correct_values_with_cpmm_with_fees() { asset_bound, asset_in: ASSET_A, asset_out: ASSET_B, - cpep: CommonPoolEventParams { pool_id: 0, who: 0 }, + cpep: CommonPoolEventParams { pool_id, who: 0 }, max_price, }) .into(), @@ -1787,6 +1875,8 @@ fn swap_exact_amount_out_exchanges_correct_values_with_cpmm_with_fees() { Some(::MinLiquidity::get()), Some(vec!(_2, _2, _2, _2)), )); + let pool_id = 0; + assert_ok!(Swaps::open_pool(pool_id)); let asset_amount_out = _1; let asset_amount_in = 11223344556; // 10101010100 / 0.9 @@ -1794,7 +1884,7 @@ fn swap_exact_amount_out_exchanges_correct_values_with_cpmm_with_fees() { let max_price = Some(_3); assert_ok!(Swaps::swap_exact_amount_out( alice_signed(), - 0, + pool_id, ASSET_A, asset_bound, ASSET_B, @@ -1808,7 +1898,7 @@ fn swap_exact_amount_out_exchanges_correct_values_with_cpmm_with_fees() { asset_bound, asset_in: ASSET_A, asset_out: ASSET_B, - cpep: CommonPoolEventParams { pool_id: 0, who: 0 }, + cpep: CommonPoolEventParams { pool_id, who: 0 }, max_price, }) .into(), @@ -2088,7 +2178,7 @@ 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)), ), crate::Error::::BelowMinimumWeight, ); @@ -2110,7 +2200,7 @@ 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)), ), crate::Error::::AboveMaximumWeight, ); @@ -2204,6 +2294,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, Some(0), true); @@ -2229,6 +2320,55 @@ 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")] +#[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, Some(1), true); + let pool_id = 0; + assert_ok!(Swaps::mutate_pool(pool_id, |pool| { + pool.pool_status = pool_status; + Ok(()) + })); + assert_noop!(Swaps::open_pool(pool_id), 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); + 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::open_pool(pool_id)); + let pool = Swaps::pool(pool_id).unwrap(); + assert_eq!(pool.pool_status, PoolStatus::Active); + System::assert_last_event(Event::PoolActive(pool_id).into()); + }); +} + #[test] fn pool_join_fails_if_max_assets_in_is_violated() { ExtBuilder::default().build().execute_with(|| { @@ -2461,6 +2601,7 @@ fn create_initial_pool( }); } + let pool_id = Swaps::next_pool_id(); assert_ok!(Swaps::create_pool( BOB, ASSETS.iter().cloned().collect(), @@ -2475,6 +2616,9 @@ fn create_initial_pool( }, if scoring_rule == ScoringRule::CPMM { Some(vec!(_2, _2, _2, _2)) } else { None }, )); + if scoring_rule == ScoringRule::CPMM { + assert_ok!(Swaps::open_pool(pool_id)); + } } fn create_initial_pool_with_funds_for_alice(