From ca684aceb81011ec003968ed43503d8cf4a03618 Mon Sep 17 00:00:00 2001 From: Malte Kliemann Date: Thu, 18 Jan 2024 16:29:46 +0100 Subject: [PATCH] Implement `force_pool_exit` and disable other zrml-swaps functions (#1235) * Abstract `pool_exit` business logic into `do_*` function * Add `force_pool_exit` and test * Install call filters for zrml-swaps --- runtime/battery-station/src/lib.rs | 11 +- runtime/zeitgeist/src/lib.rs | 5 + zrml/swaps/src/lib.rs | 85 ++++++--- zrml/swaps/src/tests.rs | 282 +++++++++++++++++++++++++++++ 4 files changed, 349 insertions(+), 34 deletions(-) diff --git a/runtime/battery-station/src/lib.rs b/runtime/battery-station/src/lib.rs index b633cb4df..de55e6b7c 100644 --- a/runtime/battery-station/src/lib.rs +++ b/runtime/battery-station/src/lib.rs @@ -58,9 +58,9 @@ use zrml_prediction_markets::Call::{ }; use zrml_rikiddo::types::{EmaMarketVolume, FeeSigmoid, RikiddoSigmoidMV}; use zrml_swaps::Call::{ - pool_exit, pool_exit_with_exact_asset_amount, pool_exit_with_exact_pool_amount, pool_join, - pool_join_with_exact_asset_amount, pool_join_with_exact_pool_amount, swap_exact_amount_in, - swap_exact_amount_out, + force_pool_exit, pool_exit, pool_exit_with_exact_asset_amount, + pool_exit_with_exact_pool_amount, pool_join, pool_join_with_exact_asset_amount, + pool_join_with_exact_pool_amount, swap_exact_amount_in, swap_exact_amount_out, }; #[cfg(feature = "parachain")] use { @@ -164,7 +164,6 @@ impl Contains for ContractsCallfilter { #[derive(scale_info::TypeInfo)] pub struct IsCallable; -// Currently disables Rikiddo. impl Contains for IsCallable { fn contains(call: &RuntimeCall) -> bool { #[allow(clippy::match_like_matches_macro)] @@ -182,6 +181,10 @@ impl Contains for IsCallable { } => false, _ => true, }, + RuntimeCall::Swaps(inner_call) => match inner_call { + force_pool_exit { .. } => true, + _ => false, + }, _ => true, } } diff --git a/runtime/zeitgeist/src/lib.rs b/runtime/zeitgeist/src/lib.rs index 1ae68efe3..c57f3f4b0 100644 --- a/runtime/zeitgeist/src/lib.rs +++ b/runtime/zeitgeist/src/lib.rs @@ -123,6 +123,7 @@ impl Contains for IsCallable { use zrml_prediction_markets::Call::{ admin_move_market_to_closed, admin_move_market_to_resolved, create_market, edit_market, }; + use zrml_swaps::Call::force_pool_exit; #[allow(clippy::match_like_matches_macro)] match runtime_call { @@ -170,6 +171,10 @@ impl Contains for IsCallable { } } RuntimeCall::SimpleDisputes(_) => false, + RuntimeCall::Swaps(inner_call) => match inner_call { + force_pool_exit { .. } => true, + _ => false, + }, RuntimeCall::System(inner_call) => { match inner_call { // Some "waste" storage will never impact proper operation. diff --git a/zrml/swaps/src/lib.rs b/zrml/swaps/src/lib.rs index 6a644f4b8..64fafebad 100644 --- a/zrml/swaps/src/lib.rs +++ b/zrml/swaps/src/lib.rs @@ -129,36 +129,7 @@ mod pallet { min_assets_out: Vec>, ) -> DispatchResult { let who = ensure_signed(origin)?; - ensure!(pool_amount != Zero::zero(), Error::::ZeroAmount); - let who_clone = who.clone(); - let pool = Self::pool_by_id(pool_id)?; - // If the pool is still in use, prevent a pool drain. - Self::ensure_minimum_liquidity_shares(pool_id, &pool, pool_amount)?; - let pool_account_id = Pallet::::pool_account_id(&pool_id); - let params = PoolParams { - asset_bounds: min_assets_out, - event: |evt| Self::deposit_event(Event::PoolExit(evt)), - pool_account_id: &pool_account_id, - pool_amount, - pool_id, - pool: &pool, - transfer_asset: |amount, amount_bound, asset| { - Self::ensure_minimum_balance(pool_id, &pool, asset, amount)?; - ensure!(amount >= amount_bound, Error::::LimitOut); - T::AssetManager::transfer(asset, &pool_account_id, &who, amount)?; - Ok(()) - }, - transfer_pool: || { - Self::burn_pool_shares(pool_id, &who, pool_amount)?; - Ok(()) - }, - fee: |amount: BalanceOf| { - let exit_fee_amount = amount.bmul(Self::calc_exit_fee(&pool))?; - Ok(exit_fee_amount) - }, - who: who_clone, - }; - crate::utils::pool::<_, _, _, _, T>(params) + Self::do_pool_exit(who, pool_id, pool_amount, min_assets_out) } /// Pool - Exit with exact pool amount @@ -512,6 +483,22 @@ mod pallet { )?; Ok(Some(weight).into()) } + + #[pallet::call_index(11)] + #[pallet::weight(T::WeightInfo::pool_exit( + min_assets_out.len().min(T::MaxAssets::get().into()) as u32 + ))] + #[transactional] + pub fn force_pool_exit( + origin: OriginFor, + who: T::AccountId, + #[pallet::compact] pool_id: PoolId, + #[pallet::compact] pool_amount: BalanceOf, + min_assets_out: Vec>, + ) -> DispatchResult { + let _ = ensure_signed(origin)?; + Self::do_pool_exit(who, pool_id, pool_amount, min_assets_out) + } } #[pallet::config] @@ -747,6 +734,44 @@ mod pallet { pub(crate) type NextPoolId = StorageValue<_, PoolId, ValueQuery>; impl Pallet { + fn do_pool_exit( + who: T::AccountId, + pool_id: PoolId, + pool_amount: BalanceOf, + min_assets_out: Vec>, + ) -> DispatchResult { + ensure!(pool_amount != Zero::zero(), Error::::ZeroAmount); + let who_clone = who.clone(); + let pool = Self::pool_by_id(pool_id)?; + // If the pool is still in use, prevent a pool drain. + Self::ensure_minimum_liquidity_shares(pool_id, &pool, pool_amount)?; + let pool_account_id = Pallet::::pool_account_id(&pool_id); + let params = PoolParams { + asset_bounds: min_assets_out, + event: |evt| Self::deposit_event(Event::PoolExit(evt)), + pool_account_id: &pool_account_id, + pool_amount, + pool_id, + pool: &pool, + transfer_asset: |amount, amount_bound, asset| { + Self::ensure_minimum_balance(pool_id, &pool, asset, amount)?; + ensure!(amount >= amount_bound, Error::::LimitOut); + T::AssetManager::transfer(asset, &pool_account_id, &who, amount)?; + Ok(()) + }, + transfer_pool: || { + Self::burn_pool_shares(pool_id, &who, pool_amount)?; + Ok(()) + }, + fee: |amount: BalanceOf| { + let exit_fee_amount = amount.bmul(Self::calc_exit_fee(&pool))?; + Ok(exit_fee_amount) + }, + who: who_clone, + }; + crate::utils::pool::<_, _, _, _, T>(params) + } + pub fn get_spot_price( pool_id: &PoolId, asset_in: &AssetOf, diff --git a/zrml/swaps/src/tests.rs b/zrml/swaps/src/tests.rs index 80b8f7609..c12fbd0c1 100644 --- a/zrml/swaps/src/tests.rs +++ b/zrml/swaps/src/tests.rs @@ -704,6 +704,17 @@ fn pool_join_or_exit_raises_on_zero_value() { Error::::ZeroAmount ); + assert_noop!( + Swaps::force_pool_exit( + RuntimeOrigin::signed(CHARLIE), + ALICE, + DEFAULT_POOL_ID, + 0, + vec!(_1, _1, _1, _1) + ), + Error::::ZeroAmount + ); + assert_noop!( Swaps::pool_join_with_exact_pool_amount(alice_signed(), DEFAULT_POOL_ID, ASSET_A, 0, 0), Error::::ZeroAmount @@ -837,6 +848,113 @@ fn pool_exit_decreases_correct_pool_parameters_with_exit_fee() { }) } +#[test] +fn force_pool_exit_decreases_correct_pool_parameters() { + ExtBuilder::default().build().execute_with(|| { + ::ExitFee::set(&0u128); + frame_system::Pallet::::set_block_number(1); + create_initial_pool_with_funds_for_alice(0, true); + + assert_ok!(Swaps::pool_join(alice_signed(), DEFAULT_POOL_ID, _1, vec!(_1, _1, _1, _1),)); + + assert_ok!(Swaps::force_pool_exit( + RuntimeOrigin::signed(CHARLIE), + ALICE, + DEFAULT_POOL_ID, + _1, + vec!(_1, _1, _1, _1), + )); + + System::assert_last_event( + Event::PoolExit(PoolAssetsEvent { + assets: vec![ASSET_A, ASSET_B, ASSET_C, ASSET_D], + bounds: vec![_1, _1, _1, _1], + cpep: CommonPoolEventParams { pool_id: DEFAULT_POOL_ID, who: 0 }, + transferred: vec![_1 + 1, _1 + 1, _1 + 1, _1 + 1], + pool_amount: _1, + }) + .into(), + ); + assert_all_parameters( + [_25 + 1, _25 + 1, _25 + 1, _25 + 1], + 0, + [ + DEFAULT_LIQUIDITY - 1, + DEFAULT_LIQUIDITY - 1, + DEFAULT_LIQUIDITY - 1, + DEFAULT_LIQUIDITY - 1, + ], + DEFAULT_LIQUIDITY, + ); + }) +} + +#[test] +fn force_pool_exit_emits_correct_events() { + ExtBuilder::default().build().execute_with(|| { + frame_system::Pallet::::set_block_number(1); + create_initial_pool_with_funds_for_alice(0, true); + assert_ok!(Swaps::force_pool_exit( + RuntimeOrigin::signed(CHARLIE), + BOB, + DEFAULT_POOL_ID, + _1, + vec!(1, 2, 3, 4), + )); + let amount = _1 - BASE / 10; // Subtract 10% fees! + System::assert_last_event( + Event::PoolExit(PoolAssetsEvent { + assets: vec![ASSET_A, ASSET_B, ASSET_C, ASSET_D], + bounds: vec![1, 2, 3, 4], + cpep: CommonPoolEventParams { pool_id: DEFAULT_POOL_ID, who: BOB }, + transferred: vec![amount; 4], + pool_amount: _1, + }) + .into(), + ); + }); +} + +#[test] +fn force_pool_exit_decreases_correct_pool_parameters_with_exit_fee() { + ExtBuilder::default().build().execute_with(|| { + frame_system::Pallet::::set_block_number(1); + create_initial_pool_with_funds_for_alice(0, true); + + assert_ok!(Swaps::force_pool_exit( + RuntimeOrigin::signed(CHARLIE), + BOB, + DEFAULT_POOL_ID, + _10, + vec!(_1, _1, _1, _1), + )); + + let pool_account = Swaps::pool_account_id(&DEFAULT_POOL_ID); + let pool_shares_id = Swaps::pool_shares_id(DEFAULT_POOL_ID); + assert_eq!(Currencies::free_balance(ASSET_A, &BOB), _9); + assert_eq!(Currencies::free_balance(ASSET_B, &BOB), _9); + assert_eq!(Currencies::free_balance(ASSET_C, &BOB), _9); + assert_eq!(Currencies::free_balance(ASSET_D, &BOB), _9); + assert_eq!(Currencies::free_balance(pool_shares_id, &BOB), DEFAULT_LIQUIDITY - _10); + assert_eq!(Currencies::free_balance(ASSET_A, &pool_account), DEFAULT_LIQUIDITY - _9); + assert_eq!(Currencies::free_balance(ASSET_B, &pool_account), DEFAULT_LIQUIDITY - _9); + assert_eq!(Currencies::free_balance(ASSET_C, &pool_account), DEFAULT_LIQUIDITY - _9); + assert_eq!(Currencies::free_balance(ASSET_D, &pool_account), DEFAULT_LIQUIDITY - _9); + assert_eq!(Currencies::total_issuance(pool_shares_id), DEFAULT_LIQUIDITY - _10); + + System::assert_last_event( + Event::PoolExit(PoolAssetsEvent { + assets: vec![ASSET_A, ASSET_B, ASSET_C, ASSET_D], + bounds: vec![_1, _1, _1, _1], + cpep: CommonPoolEventParams { pool_id: DEFAULT_POOL_ID, who: BOB }, + transferred: vec![_9, _9, _9, _9], + pool_amount: _10, + }) + .into(), + ); + }) +} + #[test_case(49_999_999_665, 12_272_234_300, 0, 0; "no_fees")] #[test_case(45_082_061_850, 12_272_234_300, _1_10, 0; "with_exit_fees")] #[test_case(46_403_174_924, 11_820_024_200, 0, _1_20; "with_swap_fees")] @@ -981,6 +1099,39 @@ fn pool_exit_is_not_allowed_with_insufficient_funds() { }) } +#[test] +fn force_pool_exit_is_not_allowed_with_insufficient_funds() { + ExtBuilder::default().build().execute_with(|| { + frame_system::Pallet::::set_block_number(1); + create_initial_pool_with_funds_for_alice(0, true); + + // Alice has no pool shares! + assert_noop!( + Swaps::force_pool_exit( + RuntimeOrigin::signed(CHARLIE), + ALICE, + DEFAULT_POOL_ID, + _1, + vec!(0, 0, 0, 0) + ), + Error::::InsufficientBalance, + ); + + // Now Alice has 25 pool shares! + let _ = Currencies::deposit(Swaps::pool_shares_id(DEFAULT_POOL_ID), &ALICE, _25); + assert_noop!( + Swaps::force_pool_exit( + RuntimeOrigin::signed(CHARLIE), + ALICE, + DEFAULT_POOL_ID, + _26, + vec!(0, 0, 0, 0) + ), + Error::::InsufficientBalance, + ); + }) +} + #[test] fn pool_join_increases_correct_pool_parameters() { ExtBuilder::default().build().execute_with(|| { @@ -1120,6 +1271,16 @@ fn provided_values_len_must_equal_assets_len() { Swaps::pool_exit(alice_signed(), DEFAULT_POOL_ID, _5, vec![]), Error::::ProvidedValuesLenMustEqualAssetsLen ); + assert_noop!( + Swaps::force_pool_exit( + RuntimeOrigin::signed(CHARLIE), + ALICE, + DEFAULT_POOL_ID, + _5, + vec![] + ), + Error::::ProvidedValuesLenMustEqualAssetsLen + ); }); } @@ -1539,6 +1700,44 @@ fn join_pool_exit_pool_does_not_create_extra_tokens() { }); } +#[test] +fn join_pool_force_exit_pool_does_not_create_extra_tokens() { + ExtBuilder::default().build().execute_with(|| { + create_initial_pool_with_funds_for_alice(0, true); + + ASSETS.iter().cloned().for_each(|asset| { + let _ = Currencies::deposit(asset, &CHARLIE, _100); + }); + + let amount = 123_456_789_123; // Strange number to force rounding errors! + assert_ok!(Swaps::pool_join( + RuntimeOrigin::signed(CHARLIE), + DEFAULT_POOL_ID, + amount, + vec![_10000, _10000, _10000, _10000] + )); + assert_ok!(Swaps::force_pool_exit( + RuntimeOrigin::signed(DAVE), + CHARLIE, + DEFAULT_POOL_ID, + amount, + vec![0, 0, 0, 0] + )); + + // Check that the pool retains more tokens than before, and that Charlie loses some tokens + // due to fees. + let pool_account_id = Swaps::pool_account_id(&DEFAULT_POOL_ID); + assert_ge!(Currencies::free_balance(ASSET_A, &pool_account_id), _100); + assert_ge!(Currencies::free_balance(ASSET_B, &pool_account_id), _100); + assert_ge!(Currencies::free_balance(ASSET_C, &pool_account_id), _100); + assert_ge!(Currencies::free_balance(ASSET_D, &pool_account_id), _100); + assert_le!(Currencies::free_balance(ASSET_A, &CHARLIE), _100); + assert_le!(Currencies::free_balance(ASSET_B, &CHARLIE), _100); + assert_le!(Currencies::free_balance(ASSET_C, &CHARLIE), _100); + assert_le!(Currencies::free_balance(ASSET_D, &CHARLIE), _100); + }); +} + #[test] fn create_pool_fails_on_weight_below_minimum_weight() { ExtBuilder::default().build().execute_with(|| { @@ -1807,6 +2006,16 @@ fn pool_exit_fails_if_min_assets_out_is_violated() { Swaps::pool_exit(alice_signed(), DEFAULT_POOL_ID, _1, vec!(_1, _1, _1 + 1, _1)), Error::::LimitOut, ); + assert_noop!( + Swaps::force_pool_exit( + RuntimeOrigin::signed(CHARLIE), + ALICE, + DEFAULT_POOL_ID, + _1, + vec!(_1, _1, _1 + 1, _1) + ), + Error::::LimitOut, + ); }); } @@ -2056,6 +2265,79 @@ fn pool_exit_fails_if_liquidity_drops_too_low() { }); } +#[test] +fn force_pool_exit_fails_if_balances_drop_too_low() { + ExtBuilder::default().build().execute_with(|| { + // We drop the balances below `Swaps::min_balance(...)`, but liquidity remains above + // `Swaps::min_balance(...)`. + ::ExitFee::set(&0u128); + create_initial_pool_with_funds_for_alice(1, true); + let pool_account_id = Swaps::pool_account_id(&DEFAULT_POOL_ID); + + assert_ok!(Currencies::withdraw( + ASSET_A, + &pool_account_id, + _100 - Swaps::min_balance(ASSET_A) + )); + assert_ok!(Currencies::withdraw( + ASSET_B, + &pool_account_id, + _100 - Swaps::min_balance(ASSET_B) + )); + assert_ok!(Currencies::withdraw( + ASSET_C, + &pool_account_id, + _100 - Swaps::min_balance(ASSET_C) + )); + assert_ok!(Currencies::withdraw( + ASSET_D, + &pool_account_id, + _100 - Swaps::min_balance(ASSET_D) + )); + + // We withdraw 99% of it, leaving 0.01 of each asset, which is below minimum balance. + assert_noop!( + Swaps::force_pool_exit( + RuntimeOrigin::signed(CHARLIE), + BOB, + DEFAULT_POOL_ID, + _10, + vec![0; 4] + ), + Error::::PoolDrain, + ); + }); +} + +#[test] +fn force_pool_exit_fails_if_liquidity_drops_too_low() { + ExtBuilder::default().build().execute_with(|| { + // We drop the liquidity below `Swaps::min_balance(...)`, but balances remains above + // `Swaps::min_balance(...)`. + ::ExitFee::set(&0u128); + create_initial_pool_with_funds_for_alice(1, true); + let pool_account_id = Swaps::pool_account_id(&DEFAULT_POOL_ID); + + // There's 1000 left of each asset. + assert_ok!(Currencies::deposit(ASSET_A, &pool_account_id, _900)); + assert_ok!(Currencies::deposit(ASSET_B, &pool_account_id, _900)); + assert_ok!(Currencies::deposit(ASSET_C, &pool_account_id, _900)); + assert_ok!(Currencies::deposit(ASSET_D, &pool_account_id, _900)); + + // We withdraw too much liquidity but leave enough of each asset. + assert_noop!( + Swaps::force_pool_exit( + RuntimeOrigin::signed(CHARLIE), + BOB, + DEFAULT_POOL_ID, + _100 - Swaps::min_balance(Swaps::pool_shares_id(DEFAULT_POOL_ID)) + 1, + vec![0; 4] + ), + Error::::PoolDrain, + ); + }); +} + #[test] fn swap_exact_amount_in_fails_if_balances_drop_too_low() { ExtBuilder::default().build().execute_with(|| {