From 13a78c42d63f9af2a98b15df4caf73a1644cc9ec Mon Sep 17 00:00:00 2001 From: Martin Date: Mon, 3 Jul 2023 15:33:02 +0200 Subject: [PATCH 01/21] feat: use nonzerou16 type for amplification --- Cargo.lock | 2 +- pallets/stableswap/Cargo.toml | 2 +- pallets/stableswap/src/lib.rs | 26 +++++++++----- pallets/stableswap/src/tests/add_liquidity.rs | 11 +++--- pallets/stableswap/src/tests/creation.rs | 25 ++++++------- pallets/stableswap/src/tests/invariants.rs | 13 +++---- pallets/stableswap/src/tests/mock.rs | 5 +-- .../stableswap/src/tests/remove_liquidity.rs | 11 +++--- pallets/stableswap/src/tests/trades.rs | 15 ++++---- pallets/stableswap/src/tests/update_pool.rs | 35 ++++++++++--------- pallets/stableswap/src/types.rs | 3 +- 11 files changed, 82 insertions(+), 66 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c93306162..78bb43024 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7304,7 +7304,7 @@ dependencies = [ [[package]] name = "pallet-stableswap" -version = "1.3.0" +version = "1.4.0" dependencies = [ "bitflags", "frame-benchmarking", diff --git a/pallets/stableswap/Cargo.toml b/pallets/stableswap/Cargo.toml index 31e18c2cd..bcd74106e 100644 --- a/pallets/stableswap/Cargo.toml +++ b/pallets/stableswap/Cargo.toml @@ -1,6 +1,6 @@ [package] name = 'pallet-stableswap' -version = '1.3.0' +version = '1.4.0' description = 'AMM for correlated assets' authors = ['GalacticCouncil'] edition = '2021' diff --git a/pallets/stableswap/src/lib.rs b/pallets/stableswap/src/lib.rs index e1a46fa16..073171233 100644 --- a/pallets/stableswap/src/lib.rs +++ b/pallets/stableswap/src/lib.rs @@ -47,6 +47,7 @@ use frame_support::{ensure, require_transactional, transactional}; use hydradx_traits::{AccountIdFor, Registry}; use sp_runtime::traits::Zero; use sp_runtime::{ArithmeticError, DispatchError, Permill}; +use sp_std::num::NonZeroU16; use sp_std::prelude::*; pub use pallet::*; @@ -87,6 +88,7 @@ pub mod pallet { use sp_runtime::traits::Zero; use sp_runtime::ArithmeticError; use sp_runtime::Permill; + use sp_std::num::NonZeroU16; #[pallet::pallet] #[pallet::generate_store(pub(crate) trait Store)] @@ -130,7 +132,7 @@ pub mod pallet { /// Amplification inclusive range. Pool's amp can be selected from the range only. #[pallet::constant] - type AmplificationRange: Get>; + type AmplificationRange: Get>; /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; @@ -154,14 +156,14 @@ pub mod pallet { PoolCreated { pool_id: T::AssetId, assets: Vec, - amplification: u16, + amplification: NonZeroU16, trade_fee: Permill, withdraw_fee: Permill, }, /// Pool parameters has been updated. PoolUpdated { pool_id: T::AssetId, - amplification: u16, + amplification: NonZeroU16, trade_fee: Permill, withdraw_fee: Permill, }, @@ -313,6 +315,8 @@ pub mod pallet { ) -> DispatchResult { T::AuthorityOrigin::ensure_origin(origin)?; + let amplification = NonZeroU16::new(amplification).ok_or(Error::::InvalidAmplification)?; + let pool_id = Self::do_create_pool(share_asset, &assets, amplification, trade_fee, withdraw_fee)?; Self::deposit_event(Event::PoolCreated { @@ -362,7 +366,11 @@ pub mod pallet { Pools::::try_mutate(pool_id, |maybe_pool| -> DispatchResult { let mut pool = maybe_pool.as_mut().ok_or(Error::::PoolNotFound)?; - pool.amplification = amplification.unwrap_or(pool.amplification); + pool.amplification = if let Some(ampl) = amplification { + NonZeroU16::new(ampl).ok_or(Error::::InvalidAmplification)? + } else { + pool.amplification + }; ensure!( T::AmplificationRange::get().contains(&pool.amplification), Error::::InvalidAmplification @@ -477,7 +485,7 @@ pub mod pallet { share_amount, asset_idx, share_issuance, - pool.amplification.into(), + pool.amplification.get().into(), pool.withdraw_fee, ) .ok_or(ArithmeticError::Overflow)?; @@ -675,7 +683,7 @@ impl Pallet { index_in, index_out, amount_in, - pool.amplification.into(), + pool.amplification.get().into(), pool.trade_fee, ) .ok_or_else(|| ArithmeticError::Overflow.into()) @@ -703,7 +711,7 @@ impl Pallet { index_in, index_out, amount_out, - pool.amplification.into(), + pool.amplification.get().into(), pool.trade_fee, ) .ok_or_else(|| ArithmeticError::Overflow.into()) @@ -713,7 +721,7 @@ impl Pallet { fn do_create_pool( share_asset: T::AssetId, assets: &[T::AssetId], - amplification: u16, + amplification: NonZeroU16, trade_fee: Permill, withdraw_fee: Permill, ) -> Result { @@ -795,7 +803,7 @@ impl Pallet { let share_amount = hydra_dx_math::stableswap::calculate_shares::( &initial_reserves, &updated_reserves, - pool.amplification.into(), + pool.amplification.get().into(), share_issuance, ) .ok_or(ArithmeticError::Overflow)?; diff --git a/pallets/stableswap/src/tests/add_liquidity.rs b/pallets/stableswap/src/tests/add_liquidity.rs index a2a2a7685..7b9e98792 100644 --- a/pallets/stableswap/src/tests/add_liquidity.rs +++ b/pallets/stableswap/src/tests/add_liquidity.rs @@ -3,6 +3,7 @@ use crate::types::{AssetLiquidity, PoolInfo}; use crate::{assert_balance, Error}; use frame_support::{assert_noop, assert_ok}; use sp_runtime::Permill; +use std::num::NonZeroU16; #[test] fn add_initial_liquidity_should_work_when_called_first_time() { @@ -134,7 +135,7 @@ fn add_liquidity_should_work_when_initial_liquidity_has_been_provided() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -201,7 +202,7 @@ fn add_liquidity_should_work_when_order_is_not_sorted() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -268,7 +269,7 @@ fn add_liquidity_should_fail_when_providing_insufficient_liquidity() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -334,7 +335,7 @@ fn add_liquidity_should_work_when_providing_one_asset_only() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b, asset_c, asset_d].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -402,7 +403,7 @@ fn add_liquidity_should_fail_when_providing_one_asset_not_in_pool() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b, asset_c, asset_d].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, diff --git a/pallets/stableswap/src/tests/creation.rs b/pallets/stableswap/src/tests/creation.rs index f36ee1c5b..997b2c588 100644 --- a/pallets/stableswap/src/tests/creation.rs +++ b/pallets/stableswap/src/tests/creation.rs @@ -4,6 +4,7 @@ use crate::Error; use crate::Pools; use frame_support::{assert_noop, assert_ok}; use sp_runtime::Permill; +use std::num::NonZeroU16; #[test] fn create_two_asset_pool_should_work_when_assets_are_registered() { @@ -22,7 +23,7 @@ fn create_two_asset_pool_should_work_when_assets_are_registered() { RuntimeOrigin::signed(ALICE), pool_id, vec![asset_a, asset_b], - 100u16, + 100, Permill::from_percent(0), Permill::from_percent(0), )); @@ -31,7 +32,7 @@ fn create_two_asset_pool_should_work_when_assets_are_registered() { >::get(pool_id).unwrap(), PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0) } @@ -60,7 +61,7 @@ fn create_multi_asset_pool_should_work_when_assets_are_registered() { RuntimeOrigin::signed(ALICE), pool_id, vec![asset_a, asset_b, asset_c, asset_d], - 100u16, + 100, Permill::from_percent(0), Permill::from_percent(0), )); @@ -85,7 +86,7 @@ fn create_pool_should_store_assets_correctly_when_input_is_not_sorted() { .with_registered_asset("four".as_bytes().to_vec(), asset_d) .build() .execute_with(|| { - let amplification: u16 = 100; + let amplification = 100u16; assert_ok!(Stableswap::create_pool( RuntimeOrigin::signed(ALICE), @@ -100,7 +101,7 @@ fn create_pool_should_store_assets_correctly_when_input_is_not_sorted() { >::get(pool_id).unwrap(), PoolInfo { assets: vec![asset_a, asset_b, asset_c, asset_d].try_into().unwrap(), - amplification, + amplification: NonZeroU16::new(amplification).unwrap(), trade_fee: Permill::from_percent(5), withdraw_fee: Permill::from_percent(10) } @@ -116,7 +117,7 @@ fn create_pool_should_fail_when_same_assets_is_specified() { .build() .execute_with(|| { let asset_a: AssetId = 1; - let amplification: u16 = 100; + let amplification = 100u16; assert_noop!( Stableswap::create_pool( @@ -137,7 +138,7 @@ fn create_pool_should_fail_when_share_asset_is_not_registered() { let pool_id: AssetId = 100; ExtBuilder::default().build().execute_with(|| { let asset_a: AssetId = 1; - let amplification: u16 = 100; + let amplification = 100u16; assert_noop!( Stableswap::create_pool( @@ -161,7 +162,7 @@ fn create_pool_should_fail_when_share_asset_is_among_assets() { .build() .execute_with(|| { let asset_a: AssetId = 1; - let amplification: u16 = 100; + let amplification = 100u16; assert_noop!( Stableswap::create_pool( @@ -187,7 +188,7 @@ fn create_pool_should_fail_when_asset_is_not_registered() { .execute_with(|| { let registered: AssetId = 1000; let not_registered: AssetId = 2000; - let amplification: u16 = 100; + let amplification = 100u16; assert_noop!( Stableswap::create_pool( @@ -227,7 +228,7 @@ fn create_pool_should_when_same_pool_already_exists() { .with_registered_asset("two".as_bytes().to_vec(), asset_b) .build() .execute_with(|| { - let amplification: u16 = 100; + let amplification = 100u16; assert_ok!(Stableswap::create_pool( RuntimeOrigin::signed(ALICE), @@ -264,8 +265,8 @@ fn create_pool_should_fail_when_amplification_is_incorrect() { .with_registered_asset("two".as_bytes().to_vec(), asset_b) .build() .execute_with(|| { - let amplification_min: u16 = 1; - let amplification_max: u16 = 10_001; + let amplification_min = 1u16; + let amplification_max = 10_001u16; assert_noop!( Stableswap::create_pool( diff --git a/pallets/stableswap/src/tests/invariants.rs b/pallets/stableswap/src/tests/invariants.rs index a09f95c2c..7dba0ff14 100644 --- a/pallets/stableswap/src/tests/invariants.rs +++ b/pallets/stableswap/src/tests/invariants.rs @@ -2,6 +2,7 @@ use crate::tests::mock::*; use crate::types::{AssetLiquidity, PoolInfo}; use frame_support::assert_ok; use sp_runtime::{FixedU128, Permill}; +use std::num::NonZeroU16; use hydra_dx_math::stableswap::calculate_d; use proptest::prelude::*; @@ -19,8 +20,8 @@ fn asset_reserve() -> impl Strategy { RESERVE_RANGE.0..RESERVE_RANGE.1 } -fn some_amplification() -> impl Strategy { - 2..10000u16 +fn some_amplification() -> impl Strategy { + (2..10000u16).prop_map(|v| NonZeroU16::new(v).unwrap()) } fn trade_fee() -> impl Strategy { @@ -161,7 +162,7 @@ proptest! { let asset_a_reserve = Tokens::free_balance(asset_a, &pool_account); let asset_b_reserve = Tokens::free_balance(asset_b, &pool_account); - let d_prev = calculate_d::<128u8>(&[asset_a_reserve,asset_b_reserve], amplification.into()).unwrap(); + let d_prev = calculate_d::<128u8>(&[asset_a_reserve,asset_b_reserve], amplification.get().into()).unwrap(); assert_ok!(Stableswap::sell( RuntimeOrigin::signed(BOB), @@ -174,7 +175,7 @@ proptest! { let asset_a_reserve = Tokens::free_balance(asset_a, &pool_account); let asset_b_reserve = Tokens::free_balance(asset_b, &pool_account); - let d = calculate_d::<128u8>(&[asset_a_reserve,asset_b_reserve], amplification.into()).unwrap(); + let d = calculate_d::<128u8>(&[asset_a_reserve,asset_b_reserve], amplification.get().into()).unwrap(); assert!(d >= d_prev); assert!(d - d_prev <= 10u128); @@ -230,7 +231,7 @@ proptest! { let asset_a_reserve = Tokens::free_balance(asset_a, &pool_account); let asset_b_reserve = Tokens::free_balance(asset_b, &pool_account); - let d_prev = calculate_d::<128u8>(&[asset_a_reserve,asset_b_reserve], amplification.into()).unwrap(); + let d_prev = calculate_d::<128u8>(&[asset_a_reserve,asset_b_reserve], amplification.get().into()).unwrap(); assert_ok!(Stableswap::buy( RuntimeOrigin::signed(BOB), @@ -242,7 +243,7 @@ proptest! { )); let asset_a_reserve = Tokens::free_balance(asset_a, &pool_account); let asset_b_reserve = Tokens::free_balance(asset_b, &pool_account); - let d = calculate_d::<128u8>(&[asset_a_reserve,asset_b_reserve], amplification.into()).unwrap(); + let d = calculate_d::<128u8>(&[asset_a_reserve,asset_b_reserve], amplification.get().into()).unwrap(); assert!(d >= d_prev); assert!(d - d_prev <= 10u128); diff --git a/pallets/stableswap/src/tests/mock.rs b/pallets/stableswap/src/tests/mock.rs index c182e9c7f..985eadd64 100644 --- a/pallets/stableswap/src/tests/mock.rs +++ b/pallets/stableswap/src/tests/mock.rs @@ -22,6 +22,7 @@ use std::cell::RefCell; use std::collections::HashMap; use core::ops::RangeInclusive; +use std::num::NonZeroU16; use crate as pallet_stableswap; @@ -135,7 +136,7 @@ parameter_types! { pub const DAIAssetId: AssetId = DAI; pub const MinimumLiquidity: Balance = 1000; pub const MinimumTradingLimit: Balance = 1000; - pub const AmplificationRange: RangeInclusive = RangeInclusive::new(2, 10_000); + pub AmplificationRange: RangeInclusive = RangeInclusive::new(NonZeroU16::new(2).unwrap(), NonZeroU16::new(10_000).unwrap()); } impl Config for Test { @@ -241,7 +242,7 @@ impl ExtBuilder { RuntimeOrigin::signed(who), pool_id, pool.assets.clone().into(), - pool.amplification, + pool.amplification.get(), pool.trade_fee, pool.withdraw_fee, )); diff --git a/pallets/stableswap/src/tests/remove_liquidity.rs b/pallets/stableswap/src/tests/remove_liquidity.rs index fe35448c0..e6c5050b6 100644 --- a/pallets/stableswap/src/tests/remove_liquidity.rs +++ b/pallets/stableswap/src/tests/remove_liquidity.rs @@ -3,6 +3,7 @@ use crate::types::{AssetLiquidity, PoolInfo}; use crate::{assert_balance, Error}; use frame_support::{assert_noop, assert_ok}; use sp_runtime::Permill; +use std::num::NonZeroU16; #[test] fn remove_liquidity_should_work_when_withdrawing_all_shares() { @@ -24,7 +25,7 @@ fn remove_liquidity_should_work_when_withdrawing_all_shares() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b, asset_c].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -101,7 +102,7 @@ fn remove_liquidity_should_apply_fee_when_withdrawing_all_shares() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b, asset_c].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(10), }, @@ -236,7 +237,7 @@ fn remove_liquidity_should_fail_when_requested_asset_not_in_pool() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b, asset_c].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(10), }, @@ -302,7 +303,7 @@ fn remove_liquidity_should_fail_when_remaining_shares_below_min_liquidity() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b, asset_c].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(10), }, @@ -376,7 +377,7 @@ fn verify_remove_liquidity_against_research_impl() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b, asset_c, asset_d].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_float(0.003), withdraw_fee: Permill::from_float(0.003), }, diff --git a/pallets/stableswap/src/tests/trades.rs b/pallets/stableswap/src/tests/trades.rs index e7dcacdaf..cce24949b 100644 --- a/pallets/stableswap/src/tests/trades.rs +++ b/pallets/stableswap/src/tests/trades.rs @@ -1,6 +1,7 @@ use crate::tests::mock::*; use crate::types::{AssetLiquidity, PoolInfo}; use crate::{assert_balance, Error}; +use std::num::NonZeroU16; use frame_support::{assert_noop, assert_ok}; use sp_runtime::Permill; @@ -17,7 +18,7 @@ fn sell_should_work_when_correct_input_provided() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -71,7 +72,7 @@ fn buy_should_work_when_correct_input_provided() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -126,7 +127,7 @@ fn sell_with_fee_should_work_when_correct_input_provided() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_percent(10), withdraw_fee: Permill::from_percent(0), }, @@ -184,7 +185,7 @@ fn sell_should_work_when_fee_is_small() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_rational(3u32, 1000u32), withdraw_fee: Permill::from_percent(0), }, @@ -242,7 +243,7 @@ fn buy_should_work_when_fee_is_set() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_percent(10), withdraw_fee: Permill::from_percent(0), }, @@ -305,7 +306,7 @@ fn sell_should_fail_when_insufficient_amount_is_provided() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -395,7 +396,7 @@ fn buy_should_fail_when_insufficient_amount_is_provided() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, diff --git a/pallets/stableswap/src/tests/update_pool.rs b/pallets/stableswap/src/tests/update_pool.rs index 899d5acd4..7b434fcde 100644 --- a/pallets/stableswap/src/tests/update_pool.rs +++ b/pallets/stableswap/src/tests/update_pool.rs @@ -3,6 +3,7 @@ use crate::types::PoolInfo; use crate::{Error, Pools}; use frame_support::{assert_noop, assert_ok}; use sp_runtime::Permill; +use std::num::NonZeroU16; #[test] fn update_pool_should_work_when_all_parames_are_updated() { @@ -21,7 +22,7 @@ fn update_pool_should_work_when_all_parames_are_updated() { RuntimeOrigin::signed(ALICE), pool_id, vec![asset_a, asset_b], - 100u16, + 100, Permill::from_percent(0), Permill::from_percent(0), )); @@ -29,7 +30,7 @@ fn update_pool_should_work_when_all_parames_are_updated() { assert_ok!(Stableswap::update_pool( RuntimeOrigin::signed(ALICE), pool_id, - Some(55u16), + Some(55), Some(Permill::from_percent(10)), Some(Permill::from_percent(20)), )); @@ -38,7 +39,7 @@ fn update_pool_should_work_when_all_parames_are_updated() { >::get(pool_id).unwrap(), PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: 55u16, + amplification: NonZeroU16::new(55).unwrap(), trade_fee: Permill::from_percent(10), withdraw_fee: Permill::from_percent(20) } @@ -63,7 +64,7 @@ fn update_pool_should_work_when_only_amplification_is_updated() { RuntimeOrigin::signed(ALICE), pool_id, vec![asset_a, asset_b], - 100u16, + 100, Permill::from_percent(0), Permill::from_percent(0), )); @@ -71,7 +72,7 @@ fn update_pool_should_work_when_only_amplification_is_updated() { assert_ok!(Stableswap::update_pool( RuntimeOrigin::signed(ALICE), pool_id, - Some(55u16), + Some(55), None, None, )); @@ -80,7 +81,7 @@ fn update_pool_should_work_when_only_amplification_is_updated() { >::get(pool_id).unwrap(), PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: 55u16, + amplification: NonZeroU16::new(55).unwrap(), trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0) } @@ -105,7 +106,7 @@ fn update_pool_should_work_when_only_trade_fee_is_updated() { RuntimeOrigin::signed(ALICE), pool_id, vec![asset_a, asset_b], - 100u16, + 100, Permill::from_percent(0), Permill::from_percent(0), )); @@ -122,7 +123,7 @@ fn update_pool_should_work_when_only_trade_fee_is_updated() { >::get(pool_id).unwrap(), PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_percent(20), withdraw_fee: Permill::from_percent(0) } @@ -147,7 +148,7 @@ fn update_pool_should_work_when_only_withdraw_fee_is_updated() { RuntimeOrigin::signed(ALICE), pool_id, vec![asset_a, asset_b], - 100u16, + 100, Permill::from_percent(0), Permill::from_percent(0), )); @@ -164,7 +165,7 @@ fn update_pool_should_work_when_only_withdraw_fee_is_updated() { >::get(pool_id).unwrap(), PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(21) } @@ -189,7 +190,7 @@ fn update_pool_should_work_when_only_fees_is_updated() { RuntimeOrigin::signed(ALICE), pool_id, vec![asset_a, asset_b], - 100u16, + 100, Permill::from_percent(0), Permill::from_percent(0), )); @@ -206,7 +207,7 @@ fn update_pool_should_work_when_only_fees_is_updated() { >::get(pool_id).unwrap(), PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_percent(11), withdraw_fee: Permill::from_percent(21) } @@ -231,7 +232,7 @@ fn update_pool_should_fail_when_nothing_is_to_update() { RuntimeOrigin::signed(ALICE), pool_id, vec![asset_a, asset_b], - 100u16, + 100, Permill::from_percent(0), Permill::from_percent(0), )); @@ -245,7 +246,7 @@ fn update_pool_should_fail_when_nothing_is_to_update() { >::get(pool_id).unwrap(), PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: 100u16, + amplification: NonZeroU16::new(100).unwrap(), trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0) } @@ -267,7 +268,7 @@ fn update_pool_should_fail_when_pool_does_not_exists() { let pool_id = retrieve_current_asset_id(); assert_noop!( - Stableswap::update_pool(RuntimeOrigin::signed(ALICE), pool_id, Some(100u16), None, None), + Stableswap::update_pool(RuntimeOrigin::signed(ALICE), pool_id, Some(100), None, None), Error::::PoolNotFound ); }); @@ -290,13 +291,13 @@ fn update_pool_should_fail_when_amplification_is_outside_allowed_range() { RuntimeOrigin::signed(ALICE), pool_id, vec![asset_a, asset_b], - 100u16, + 100, Permill::from_percent(0), Permill::from_percent(0), )); assert_noop!( - Stableswap::update_pool(RuntimeOrigin::signed(ALICE), pool_id, Some(20_000u16), None, None), + Stableswap::update_pool(RuntimeOrigin::signed(ALICE), pool_id, Some(20_000), None, None), Error::::InvalidAmplification ); }); diff --git a/pallets/stableswap/src/types.rs b/pallets/stableswap/src/types.rs index 22b5ca8c1..53d0debd6 100644 --- a/pallets/stableswap/src/types.rs +++ b/pallets/stableswap/src/types.rs @@ -1,6 +1,7 @@ use crate::{Config, MAX_ASSETS_IN_POOL}; use sp_runtime::Permill; use sp_std::collections::btree_set::BTreeSet; +use sp_std::num::NonZeroU16; use sp_std::prelude::*; use codec::{Decode, Encode, MaxEncodedLen}; @@ -19,7 +20,7 @@ pub(crate) type Balance = u128; #[derive(Clone, PartialEq, Eq, Encode, Decode, MaxEncodedLen, TypeInfo, RuntimeDebug)] pub struct PoolInfo { pub assets: BoundedVec>, - pub amplification: u16, + pub amplification: NonZeroU16, pub trade_fee: Permill, pub withdraw_fee: Permill, } From 97c113fb0c0e21020340114693ab27a0e1b91e27 Mon Sep 17 00:00:00 2001 From: Martin Date: Mon, 3 Jul 2023 16:13:46 +0200 Subject: [PATCH 02/21] add test of amplication 0 --- pallets/stableswap/src/tests/creation.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pallets/stableswap/src/tests/creation.rs b/pallets/stableswap/src/tests/creation.rs index 997b2c588..516ce67dc 100644 --- a/pallets/stableswap/src/tests/creation.rs +++ b/pallets/stableswap/src/tests/creation.rs @@ -268,6 +268,18 @@ fn create_pool_should_fail_when_amplification_is_incorrect() { let amplification_min = 1u16; let amplification_max = 10_001u16; + assert_noop!( + Stableswap::create_pool( + RuntimeOrigin::signed(ALICE), + pool_id, + vec![asset_a, asset_b], + 0, + Permill::from_percent(0), + Permill::from_percent(0), + ), + Error::::InvalidAmplification + ); + assert_noop!( Stableswap::create_pool( RuntimeOrigin::signed(ALICE), From 204711d863bd6d6fdda2da5ace1926a80333838e Mon Sep 17 00:00:00 2001 From: Martin Date: Tue, 4 Jul 2023 17:25:56 +0200 Subject: [PATCH 03/21] reword amplification calculation - gradual change --- Cargo.lock | 5 +- math/Cargo.toml | 2 +- math/src/stableswap/math.rs | 35 +++++ math/src/stableswap/tests/amplification.rs | 41 ++++++ math/src/stableswap/tests/mod.rs | 1 + pallets/stableswap/Cargo.toml | 6 +- pallets/stableswap/src/lib.rs | 131 ++++++++++++++++-- pallets/stableswap/src/tests/add_liquidity.rs | 25 +++- pallets/stableswap/src/tests/creation.rs | 6 + pallets/stableswap/src/tests/invariants.rs | 17 ++- pallets/stableswap/src/tests/mock.rs | 10 +- .../stableswap/src/tests/remove_liquidity.rs | 25 +++- pallets/stableswap/src/tests/trades.rs | 35 ++++- pallets/stableswap/src/tests/update_pool.rs | 18 +++ pallets/stableswap/src/types.rs | 13 +- 15 files changed, 326 insertions(+), 44 deletions(-) create mode 100644 math/src/stableswap/tests/amplification.rs diff --git a/Cargo.lock b/Cargo.lock index 78bb43024..05c4d50f7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3685,7 +3685,7 @@ dependencies = [ [[package]] name = "hydra-dx-math" -version = "7.3.0" +version = "7.4.0" dependencies = [ "approx", "criterion", @@ -7304,7 +7304,7 @@ dependencies = [ [[package]] name = "pallet-stableswap" -version = "1.4.0" +version = "2.0.0" dependencies = [ "bitflags", "frame-benchmarking", @@ -7317,6 +7317,7 @@ dependencies = [ "parity-scale-codec", "proptest", "scale-info", + "serde", "sp-api", "sp-core", "sp-io", diff --git a/math/Cargo.toml b/math/Cargo.toml index 76c74d5ed..19c1b8ee2 100644 --- a/math/Cargo.toml +++ b/math/Cargo.toml @@ -6,7 +6,7 @@ license = 'Apache-2.0' name = "hydra-dx-math" description = "A collection of utilities to make performing liquidity pool calculations more convenient." repository = 'https://github.com/galacticcouncil/hydradx-math' -version = "7.3.0" +version = "7.4.0" [dependencies] primitive-types = {default-features = false, version = '0.12.0'} diff --git a/math/src/stableswap/math.rs b/math/src/stableswap/math.rs index a1b695787..a282d7938 100644 --- a/math/src/stableswap/math.rs +++ b/math/src/stableswap/math.rs @@ -4,6 +4,7 @@ use num_traits::{CheckedDiv, CheckedMul, Zero}; use primitive_types::U256; use sp_arithmetic::{FixedPointNumber, FixedU128, Permill}; use sp_std::prelude::*; +use std::ops::Div; pub const MAX_Y_ITERATIONS: u8 = 128; pub const MAX_D_ITERATIONS: u8 = 64; @@ -346,6 +347,40 @@ pub(crate) fn calculate_y(xp: &[Balance], d: Balance, amplification Balance::try_from(y).ok() } +pub fn calculate_amplification( + initial_amplification: u128, + future_amplification: u128, + initial_timestamp: u128, + future_timestamp: u128, + current_timestamp: u128, +) -> u128 { + // short circuit if timestamp are invalid + if current_timestamp < initial_timestamp || future_timestamp < initial_timestamp { + return initial_timestamp; + } + + // Short circuit if timestamp or amplification are equal + if initial_timestamp == future_timestamp || initial_amplification == future_amplification { + return future_amplification; + } + + // short circuit if already reached desired timestamp + if current_timestamp >= future_timestamp { + return future_amplification; + } + + let step = future_amplification + .abs_diff(initial_amplification) + .saturating_mul(current_timestamp.saturating_sub(initial_timestamp)) + .div(future_timestamp.saturating_sub(initial_timestamp)); + + return if future_amplification > initial_amplification { + initial_amplification.saturating_add(step) + } else { + initial_amplification.saturating_sub(step) + }; +} + #[inline] fn has_converged(v0: U256, v1: U256, precision: U256) -> bool { let diff = abs_diff(v0, v1); diff --git a/math/src/stableswap/tests/amplification.rs b/math/src/stableswap/tests/amplification.rs new file mode 100644 index 000000000..8024b3034 --- /dev/null +++ b/math/src/stableswap/tests/amplification.rs @@ -0,0 +1,41 @@ +use crate::stableswap::calculate_amplification; + +#[test] +fn calculate_amplification_should_short_circuit_when_future_and_initial_amp_are_equal() { + let result = calculate_amplification(10, 10, 0, 100, 50); + assert_eq!(result, 10); +} + +#[test] +fn calculate_amplification_should_short_circuit_when_current_timestamp_is_greater_than_future_timestamp() { + let result = calculate_amplification(10, 20, 0, 100, 150); + assert_eq!(result, 20); +} + +#[test] +fn test_calculate_amplification_increase_amplification() { + let result = calculate_amplification(10, 20, 0, 100, 50); + assert_eq!(result, 15); +} + +#[test] +fn test_calculate_amplification_decrease_amplification() { + let result = calculate_amplification(20, 10, 0, 100, 50); + assert_eq!(result, 15); +} + +#[test] +fn test_calculate_amplification_step_increase_amplification() { + for idx in 0..1000 { + let result = calculate_amplification(2000, 5000, 0, 1000, idx); + assert_eq!(result, 2000 + idx * 3); + } +} + +#[test] +fn test_calculate_amplification_step_decrease_amplification() { + for idx in 0..1000 { + let result = calculate_amplification(5000, 2000, 0, 1000, idx); + assert_eq!(result, 5000 - idx * 3); + } +} diff --git a/math/src/stableswap/tests/mod.rs b/math/src/stableswap/tests/mod.rs index e44fb5a49..4f300a451 100644 --- a/math/src/stableswap/tests/mod.rs +++ b/math/src/stableswap/tests/mod.rs @@ -1,3 +1,4 @@ +mod amplification; mod invariants; mod multi_assets; mod two_assets; diff --git a/pallets/stableswap/Cargo.toml b/pallets/stableswap/Cargo.toml index bcd74106e..fa86fe9ef 100644 --- a/pallets/stableswap/Cargo.toml +++ b/pallets/stableswap/Cargo.toml @@ -1,6 +1,6 @@ [package] name = 'pallet-stableswap' -version = '1.4.0' +version = '2.0.0' description = 'AMM for correlated assets' authors = ['GalacticCouncil'] edition = '2021' @@ -17,6 +17,7 @@ bitflags = "1.3.2" # parity scale-info = { version = "2.1.2", default-features = false, features = ["derive"] } codec = { default-features = false, features = ["derive"], package = "parity-scale-codec", version = "3.4.0" } +serde = { features = ["derive"], optional = true, version = "1.0.137" } # primitives sp-runtime = { workspace = true } @@ -55,14 +56,15 @@ runtime-benchmarks = [ "hydra-dx-math/runtime-benchmarks", ] std = [ + "serde/std", 'codec/std', + "scale-info/std", 'frame-support/std', 'frame-system/std', 'sp-runtime/std', 'sp-core/std', 'sp-io/std', 'sp-std/std', - "scale-info/std", "orml-tokens/std", "frame-benchmarking/std", "hydra-dx-math/std", diff --git a/pallets/stableswap/src/lib.rs b/pallets/stableswap/src/lib.rs index 073171233..bb7cd7b6e 100644 --- a/pallets/stableswap/src/lib.rs +++ b/pallets/stableswap/src/lib.rs @@ -45,8 +45,8 @@ extern crate core; use frame_support::pallet_prelude::{DispatchResult, Get}; use frame_support::{ensure, require_transactional, transactional}; use hydradx_traits::{AccountIdFor, Registry}; -use sp_runtime::traits::Zero; -use sp_runtime::{ArithmeticError, DispatchError, Permill}; +use sp_runtime::traits::{BlockNumberProvider, Zero}; +use sp_runtime::{ArithmeticError, DispatchError, Permill, SaturatedConversion}; use sp_std::num::NonZeroU16; use sp_std::prelude::*; @@ -85,9 +85,9 @@ pub mod pallet { use core::ops::RangeInclusive; use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; - use sp_runtime::traits::Zero; - use sp_runtime::ArithmeticError; + use sp_runtime::traits::{BlockNumberProvider, Zero}; use sp_runtime::Permill; + use sp_runtime::{ArithmeticError, SaturatedConversion}; use sp_std::num::NonZeroU16; #[pallet::pallet] @@ -99,6 +99,9 @@ pub mod pallet { /// The overarching event type. type RuntimeEvent: From> + IsType<::RuntimeEvent>; + /// Provider for the current block number. + type BlockNumberProvider: BlockNumberProvider; + /// Identifier for the class of asset. type AssetId: Member + Parameter @@ -141,7 +144,7 @@ pub mod pallet { /// Existing pools #[pallet::storage] #[pallet::getter(fn pools)] - pub type Pools = StorageMap<_, Blake2_128Concat, T::AssetId, PoolInfo>; + pub type Pools = StorageMap<_, Blake2_128Concat, T::AssetId, PoolInfo>; /// Tradability state of pool assets. #[pallet::storage] @@ -210,6 +213,13 @@ pub mod pallet { asset_id: T::AssetId, state: Tradability, }, + + /// + AmplificationUpdated { + pool_id: T::AssetId, + amplification: NonZeroU16, + block: T::BlockNumber, + }, } #[pallet::error] @@ -387,6 +397,62 @@ pub mod pallet { }) } + /// Update given stableswap pool's parameters. + /// + /// Updates one or more parameters of stablesswap pool ( amplification, trade fee, withdraw fee). + /// + /// If all parameters are none, `NothingToUpdate` error is returned. + /// + /// if pool does not exist, `PoolNotFound` is returned. + /// + /// Parameters: + /// - `origin`: Must be T::AuthorityOrigin + /// - `pool_id`: pool to update + /// - `amplification`: new pool amplification or None + /// - `trade_fee`: new trade fee or None + /// - `withdraw_fee`: new withdraw fee or None + /// + /// Emits `PoolUpdated` event if successful. + #[pallet::call_index(2)] + #[pallet::weight(::WeightInfo::update_pool())] + #[transactional] + pub fn update_amplification( + origin: OriginFor, + pool_id: T::AssetId, + future_amplification: u16, + block: T::BlockNumber, + ) -> DispatchResult { + T::AuthorityOrigin::ensure_origin(origin)?; + + Pools::::try_mutate(pool_id, |maybe_pool| -> DispatchResult { + let mut pool = maybe_pool.as_mut().ok_or(Error::::PoolNotFound)?; + + pool.amplification = pool.future_amplification; + pool.future_amplification = + NonZeroU16::new(future_amplification).ok_or(Error::::InvalidAmplification)?; + + let current_block = T::BlockNumberProvider::current_block_number(); + ensure!( + block > current_block && block > pool.future_amp_timestamp, + Error::::InvalidAmplification + ); + + pool.amp_timestamp = current_block; + pool.future_amp_timestamp = block; + + ensure!( + T::AmplificationRange::get().contains(&pool.future_amplification), + Error::::InvalidAmplification + ); + Self::deposit_event(Event::AmplificationUpdated { + pool_id, + amplification: pool.future_amplification, + block: pool.future_amp_timestamp, + }); + Ok(()) + }) + } + /// Add liquidity to selected pool. /// /// First call of `add_liquidity` adds "initial liquidity" of all assets. @@ -403,7 +469,7 @@ pub mod pallet { /// - `assets`: asset id and liquidity amount provided /// /// Emits `LiquidityAdded` event when successful. - #[pallet::call_index(2)] + #[pallet::call_index(3)] #[pallet::weight(::WeightInfo::add_liquidity())] #[transactional] pub fn add_liquidity( @@ -440,7 +506,7 @@ pub mod pallet { /// - 'share_amount': amount of shares to withdraw /// /// Emits `LiquidityRemoved` event when successful. - #[pallet::call_index(3)] + #[pallet::call_index(4)] #[pallet::weight(::WeightInfo::remove_liquidity_one_asset())] #[transactional] pub fn remove_liquidity_one_asset( @@ -480,12 +546,20 @@ pub mod pallet { Error::::InsufficientLiquidityRemaining ); + let amplification = hydra_dx_math::stableswap::calculate_amplification( + pool.amplification.get().into(), + pool.future_amplification.get().into(), + pool.amp_timestamp.saturated_into(), + pool.future_amp_timestamp.saturated_into(), + T::BlockNumberProvider::current_block_number().saturated_into(), + ); + let (amount, fee) = hydra_dx_math::stableswap::calculate_withdraw_one_asset::( &balances, share_amount, asset_idx, share_issuance, - pool.amplification.get().into(), + amplification, pool.withdraw_fee, ) .ok_or(ArithmeticError::Overflow)?; @@ -517,7 +591,7 @@ pub mod pallet { /// /// Emits `SellExecuted` event when successful. /// - #[pallet::call_index(4)] + #[pallet::call_index(5)] #[pallet::weight(::WeightInfo::sell())] #[transactional] pub fn sell( @@ -580,7 +654,7 @@ pub mod pallet { /// /// Emits `BuyExecuted` event when successful. /// - #[pallet::call_index(5)] + #[pallet::call_index(6)] #[pallet::weight(::WeightInfo::buy())] #[transactional] pub fn buy( @@ -631,7 +705,7 @@ pub mod pallet { Ok(()) } - #[pallet::call_index(6)] + #[pallet::call_index(7)] #[pallet::weight(::WeightInfo::set_asset_tradable_state())] #[transactional] pub fn set_asset_tradable_state( @@ -678,12 +752,20 @@ impl Pallet { ensure!(balances[index_in] > Balance::zero(), Error::::InsufficientLiquidity); ensure!(balances[index_out] > Balance::zero(), Error::::InsufficientLiquidity); + let amplification = hydra_dx_math::stableswap::calculate_amplification( + pool.amplification.get().into(), + pool.future_amplification.get().into(), + pool.amp_timestamp.saturated_into(), + pool.future_amp_timestamp.saturated_into(), + T::BlockNumberProvider::current_block_number().saturated_into(), + ); + hydra_dx_math::stableswap::calculate_out_given_in_with_fee::( &balances, index_in, index_out, amount_in, - pool.amplification.get().into(), + amplification, pool.trade_fee, ) .ok_or_else(|| ArithmeticError::Overflow.into()) @@ -706,12 +788,20 @@ impl Pallet { ensure!(balances[index_out] > amount_out, Error::::InsufficientLiquidity); ensure!(balances[index_in] > Balance::zero(), Error::::InsufficientLiquidity); + let amplification = hydra_dx_math::stableswap::calculate_amplification( + pool.amplification.get().into(), + pool.future_amplification.get().into(), + pool.amp_timestamp.saturated_into(), + pool.future_amp_timestamp.saturated_into(), + T::BlockNumberProvider::current_block_number().saturated_into(), + ); + hydra_dx_math::stableswap::calculate_in_given_out_with_fee::( &balances, index_in, index_out, amount_out, - pool.amplification.get().into(), + amplification, pool.trade_fee, ) .ok_or_else(|| ArithmeticError::Overflow.into()) @@ -733,6 +823,8 @@ impl Pallet { ensure!(!assets.contains(&share_asset), Error::::ShareAssetInPoolAssets); + let block_number = T::BlockNumberProvider::current_block_number(); + let mut pool_assets = assets.to_vec(); pool_assets.sort(); @@ -742,6 +834,9 @@ impl Pallet { .try_into() .map_err(|_| Error::::MaxAssetsExceeded)?, amplification, + future_amplification: amplification, + amp_timestamp: block_number, + future_amp_timestamp: block_number, trade_fee, withdraw_fee, }; @@ -799,11 +894,19 @@ impl Pallet { } } + let amplification = hydra_dx_math::stableswap::calculate_amplification( + pool.amplification.get().into(), + pool.future_amplification.get().into(), + pool.amp_timestamp.saturated_into(), + pool.future_amp_timestamp.saturated_into(), + T::BlockNumberProvider::current_block_number().saturated_into(), + ); + let share_issuance = T::Currency::total_issuance(pool_id); let share_amount = hydra_dx_math::stableswap::calculate_shares::( &initial_reserves, &updated_reserves, - pool.amplification.get().into(), + amplification, share_issuance, ) .ok_or(ArithmeticError::Overflow)?; diff --git a/pallets/stableswap/src/tests/add_liquidity.rs b/pallets/stableswap/src/tests/add_liquidity.rs index 7b9e98792..3754bd1ec 100644 --- a/pallets/stableswap/src/tests/add_liquidity.rs +++ b/pallets/stableswap/src/tests/add_liquidity.rs @@ -133,9 +133,12 @@ fn add_liquidity_should_work_when_initial_liquidity_has_been_provided() { .with_registered_asset("two".as_bytes().to_vec(), asset_b) .with_pool( ALICE, - PoolInfo:: { + PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -200,9 +203,12 @@ fn add_liquidity_should_work_when_order_is_not_sorted() { .with_registered_asset("two".as_bytes().to_vec(), 2) .with_pool( ALICE, - PoolInfo:: { + PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -267,9 +273,12 @@ fn add_liquidity_should_fail_when_providing_insufficient_liquidity() { .with_registered_asset("two".as_bytes().to_vec(), 2) .with_pool( ALICE, - PoolInfo:: { + PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -333,9 +342,12 @@ fn add_liquidity_should_work_when_providing_one_asset_only() { .with_registered_asset("four".as_bytes().to_vec(), asset_d) .with_pool( ALICE, - PoolInfo:: { + PoolInfo:: { assets: vec![asset_a, asset_b, asset_c, asset_d].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -401,9 +413,12 @@ fn add_liquidity_should_fail_when_providing_one_asset_not_in_pool() { .with_registered_asset("five".as_bytes().to_vec(), asset_e) .with_pool( ALICE, - PoolInfo:: { + PoolInfo:: { assets: vec![asset_a, asset_b, asset_c, asset_d].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, diff --git a/pallets/stableswap/src/tests/creation.rs b/pallets/stableswap/src/tests/creation.rs index 516ce67dc..d63f51def 100644 --- a/pallets/stableswap/src/tests/creation.rs +++ b/pallets/stableswap/src/tests/creation.rs @@ -33,6 +33,9 @@ fn create_two_asset_pool_should_work_when_assets_are_registered() { PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0) } @@ -102,6 +105,9 @@ fn create_pool_should_store_assets_correctly_when_input_is_not_sorted() { PoolInfo { assets: vec![asset_a, asset_b, asset_c, asset_d].try_into().unwrap(), amplification: NonZeroU16::new(amplification).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(5), withdraw_fee: Permill::from_percent(10) } diff --git a/pallets/stableswap/src/tests/invariants.rs b/pallets/stableswap/src/tests/invariants.rs index 7dba0ff14..5cb4e53d2 100644 --- a/pallets/stableswap/src/tests/invariants.rs +++ b/pallets/stableswap/src/tests/invariants.rs @@ -62,9 +62,13 @@ proptest! { .with_registered_asset("two".as_bytes().to_vec(), asset_b) .with_pool( ALICE, - PoolInfo:: { + PoolInfo:: { assets: vec![asset_a,asset_b].try_into().unwrap(), amplification, + + future_amplification: amplification, + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee, withdraw_fee: Permill::from_percent(0), }, @@ -136,9 +140,13 @@ proptest! { .with_registered_asset("two".as_bytes().to_vec(), asset_b) .with_pool( ALICE, - PoolInfo:: { + PoolInfo:: { assets: vec![asset_a,asset_b].try_into().unwrap(), amplification, + + future_amplification: amplification, + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -204,9 +212,12 @@ proptest! { .with_registered_asset("two".as_bytes().to_vec(), asset_b) .with_pool( ALICE, - PoolInfo:: { + PoolInfo:: { assets: vec![asset_a,asset_b].try_into().unwrap(), amplification, + future_amplification: amplification, + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, diff --git a/pallets/stableswap/src/tests/mock.rs b/pallets/stableswap/src/tests/mock.rs index 985eadd64..91e56c08d 100644 --- a/pallets/stableswap/src/tests/mock.rs +++ b/pallets/stableswap/src/tests/mock.rs @@ -150,6 +150,7 @@ impl Config for Test { type AmplificationRange = AmplificationRange; type MinTradingLimit = MinimumTradingLimit; type WeightInfo = (); + type BlockNumberProvider = System; } pub struct InitialLiquidity { @@ -160,7 +161,7 @@ pub struct InitialLiquidity { pub struct ExtBuilder { endowed_accounts: Vec<(AccountId, AssetId, Balance)>, registered_assets: Vec<(Vec, AssetId)>, - created_pools: Vec<(AccountId, PoolInfo, InitialLiquidity)>, + created_pools: Vec<(AccountId, PoolInfo, InitialLiquidity)>, } impl Default for ExtBuilder { @@ -196,7 +197,12 @@ impl ExtBuilder { self } - pub fn with_pool(mut self, who: AccountId, pool: PoolInfo, initial_liquidity: InitialLiquidity) -> Self { + pub fn with_pool( + mut self, + who: AccountId, + pool: PoolInfo, + initial_liquidity: InitialLiquidity, + ) -> Self { self.created_pools.push((who, pool, initial_liquidity)); self } diff --git a/pallets/stableswap/src/tests/remove_liquidity.rs b/pallets/stableswap/src/tests/remove_liquidity.rs index e6c5050b6..030c50628 100644 --- a/pallets/stableswap/src/tests/remove_liquidity.rs +++ b/pallets/stableswap/src/tests/remove_liquidity.rs @@ -23,9 +23,12 @@ fn remove_liquidity_should_work_when_withdrawing_all_shares() { .with_registered_asset("three".as_bytes().to_vec(), asset_c) .with_pool( ALICE, - PoolInfo:: { + PoolInfo:: { assets: vec![asset_a, asset_b, asset_c].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -100,9 +103,12 @@ fn remove_liquidity_should_apply_fee_when_withdrawing_all_shares() { .with_registered_asset("three".as_bytes().to_vec(), asset_c) .with_pool( ALICE, - PoolInfo:: { + PoolInfo:: { assets: vec![asset_a, asset_b, asset_c].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(10), }, @@ -235,9 +241,12 @@ fn remove_liquidity_should_fail_when_requested_asset_not_in_pool() { .with_registered_asset("three".as_bytes().to_vec(), asset_c) .with_pool( ALICE, - PoolInfo:: { + PoolInfo:: { assets: vec![asset_a, asset_b, asset_c].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(10), }, @@ -301,9 +310,12 @@ fn remove_liquidity_should_fail_when_remaining_shares_below_min_liquidity() { .with_registered_asset("three".as_bytes().to_vec(), asset_c) .with_pool( ALICE, - PoolInfo:: { + PoolInfo:: { assets: vec![asset_a, asset_b, asset_c].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(10), }, @@ -375,9 +387,12 @@ fn verify_remove_liquidity_against_research_impl() { .with_registered_asset("four".as_bytes().to_vec(), asset_d) .with_pool( ALICE, - PoolInfo:: { + PoolInfo:: { assets: vec![asset_a, asset_b, asset_c, asset_d].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_float(0.003), withdraw_fee: Permill::from_float(0.003), }, diff --git a/pallets/stableswap/src/tests/trades.rs b/pallets/stableswap/src/tests/trades.rs index cce24949b..bfdc83de6 100644 --- a/pallets/stableswap/src/tests/trades.rs +++ b/pallets/stableswap/src/tests/trades.rs @@ -16,9 +16,12 @@ fn sell_should_work_when_correct_input_provided() { .with_registered_asset("two".as_bytes().to_vec(), 2) .with_pool( ALICE, - PoolInfo:: { + PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -70,9 +73,12 @@ fn buy_should_work_when_correct_input_provided() { .with_registered_asset("two".as_bytes().to_vec(), 2) .with_pool( ALICE, - PoolInfo:: { + PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -125,9 +131,12 @@ fn sell_with_fee_should_work_when_correct_input_provided() { .with_registered_asset("two".as_bytes().to_vec(), 2) .with_pool( ALICE, - PoolInfo:: { + PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(10), withdraw_fee: Permill::from_percent(0), }, @@ -183,9 +192,12 @@ fn sell_should_work_when_fee_is_small() { .with_registered_asset("two".as_bytes().to_vec(), 2) .with_pool( ALICE, - PoolInfo:: { + PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_rational(3u32, 1000u32), withdraw_fee: Permill::from_percent(0), }, @@ -241,9 +253,12 @@ fn buy_should_work_when_fee_is_set() { .with_registered_asset("two".as_bytes().to_vec(), 2) .with_pool( ALICE, - PoolInfo:: { + PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(10), withdraw_fee: Permill::from_percent(0), }, @@ -304,9 +319,12 @@ fn sell_should_fail_when_insufficient_amount_is_provided() { .with_registered_asset("two".as_bytes().to_vec(), 2000) .with_pool( ALICE, - PoolInfo:: { + PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -394,9 +412,12 @@ fn buy_should_fail_when_insufficient_amount_is_provided() { .with_registered_asset("two".as_bytes().to_vec(), asset_b) .with_pool( ALICE, - PoolInfo:: { + PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, diff --git a/pallets/stableswap/src/tests/update_pool.rs b/pallets/stableswap/src/tests/update_pool.rs index 7b434fcde..143d05eb2 100644 --- a/pallets/stableswap/src/tests/update_pool.rs +++ b/pallets/stableswap/src/tests/update_pool.rs @@ -40,6 +40,9 @@ fn update_pool_should_work_when_all_parames_are_updated() { PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), amplification: NonZeroU16::new(55).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(10), withdraw_fee: Permill::from_percent(20) } @@ -82,6 +85,9 @@ fn update_pool_should_work_when_only_amplification_is_updated() { PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), amplification: NonZeroU16::new(55).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0) } @@ -124,6 +130,9 @@ fn update_pool_should_work_when_only_trade_fee_is_updated() { PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(20), withdraw_fee: Permill::from_percent(0) } @@ -166,6 +175,9 @@ fn update_pool_should_work_when_only_withdraw_fee_is_updated() { PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(21) } @@ -208,6 +220,9 @@ fn update_pool_should_work_when_only_fees_is_updated() { PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(11), withdraw_fee: Permill::from_percent(21) } @@ -247,6 +262,9 @@ fn update_pool_should_fail_when_nothing_is_to_update() { PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), amplification: NonZeroU16::new(100).unwrap(), + future_amplification: NonZeroU16::new(100).unwrap(), + amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0) } diff --git a/pallets/stableswap/src/types.rs b/pallets/stableswap/src/types.rs index 53d0debd6..2c97b271d 100644 --- a/pallets/stableswap/src/types.rs +++ b/pallets/stableswap/src/types.rs @@ -1,3 +1,6 @@ +#[cfg(feature = "std")] +use serde::{Deserialize, Serialize}; + use crate::{Config, MAX_ASSETS_IN_POOL}; use sp_runtime::Permill; use sp_std::collections::btree_set::BTreeSet; @@ -17,10 +20,14 @@ pub(crate) type Balance = u128; /// `assets`: pool assets /// `amplification`: amp parameter /// `fee`: trade fee to be withdrawn on sell/buy -#[derive(Clone, PartialEq, Eq, Encode, Decode, MaxEncodedLen, TypeInfo, RuntimeDebug)] -pub struct PoolInfo { +#[derive(Encode, Decode, Eq, PartialEq, Clone, RuntimeDebug, TypeInfo, MaxEncodedLen)] +#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] +pub struct PoolInfo { pub assets: BoundedVec>, pub amplification: NonZeroU16, + pub future_amplification: NonZeroU16, + pub amp_timestamp: BlockNumber, + pub future_amp_timestamp: BlockNumber, pub trade_fee: Permill, pub withdraw_fee: Permill, } @@ -34,7 +41,7 @@ where iter.all(move |x| uniq.insert(x)) } -impl PoolInfo +impl PoolInfo where AssetId: Ord + Copy, { From 174766b795afb2d66cf987fb61d1f6e62042f126 Mon Sep 17 00:00:00 2001 From: Martin Date: Tue, 4 Jul 2023 17:35:23 +0200 Subject: [PATCH 04/21] fix amp math --- math/src/stableswap/math.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/math/src/stableswap/math.rs b/math/src/stableswap/math.rs index a282d7938..c9762322b 100644 --- a/math/src/stableswap/math.rs +++ b/math/src/stableswap/math.rs @@ -356,7 +356,7 @@ pub fn calculate_amplification( ) -> u128 { // short circuit if timestamp are invalid if current_timestamp < initial_timestamp || future_timestamp < initial_timestamp { - return initial_timestamp; + return initial_amplification; } // Short circuit if timestamp or amplification are equal From 7ebc4011610d92a94bbd8e85021613b7371241c9 Mon Sep 17 00:00:00 2001 From: Martin Date: Tue, 4 Jul 2023 17:58:34 +0200 Subject: [PATCH 05/21] remove amp change from update pool call --- pallets/stableswap/src/lib.rs | 14 +-- pallets/stableswap/src/tests/update_pool.rs | 97 +++------------------ 2 files changed, 14 insertions(+), 97 deletions(-) diff --git a/pallets/stableswap/src/lib.rs b/pallets/stableswap/src/lib.rs index bb7cd7b6e..92aa1fee7 100644 --- a/pallets/stableswap/src/lib.rs +++ b/pallets/stableswap/src/lib.rs @@ -359,32 +359,22 @@ pub mod pallet { #[pallet::call_index(1)] #[pallet::weight(::WeightInfo::update_pool())] #[transactional] - pub fn update_pool( + pub fn update_pool_fees( origin: OriginFor, pool_id: T::AssetId, - amplification: Option, trade_fee: Option, withdraw_fee: Option, ) -> DispatchResult { T::AuthorityOrigin::ensure_origin(origin)?; ensure!( - amplification.is_some() || trade_fee.is_some() || withdraw_fee.is_some(), + trade_fee.is_some() || withdraw_fee.is_some(), Error::::NothingToUpdate ); Pools::::try_mutate(pool_id, |maybe_pool| -> DispatchResult { let mut pool = maybe_pool.as_mut().ok_or(Error::::PoolNotFound)?; - pool.amplification = if let Some(ampl) = amplification { - NonZeroU16::new(ampl).ok_or(Error::::InvalidAmplification)? - } else { - pool.amplification - }; - ensure!( - T::AmplificationRange::get().contains(&pool.amplification), - Error::::InvalidAmplification - ); pool.trade_fee = trade_fee.unwrap_or(pool.trade_fee); pool.withdraw_fee = withdraw_fee.unwrap_or(pool.withdraw_fee); Self::deposit_event(Event::PoolUpdated { diff --git a/pallets/stableswap/src/tests/update_pool.rs b/pallets/stableswap/src/tests/update_pool.rs index 143d05eb2..def4e72ee 100644 --- a/pallets/stableswap/src/tests/update_pool.rs +++ b/pallets/stableswap/src/tests/update_pool.rs @@ -27,10 +27,9 @@ fn update_pool_should_work_when_all_parames_are_updated() { Permill::from_percent(0), )); - assert_ok!(Stableswap::update_pool( + assert_ok!(Stableswap::update_pool_fees( RuntimeOrigin::signed(ALICE), pool_id, - Some(55), Some(Permill::from_percent(10)), Some(Permill::from_percent(20)), )); @@ -39,7 +38,7 @@ fn update_pool_should_work_when_all_parames_are_updated() { >::get(pool_id).unwrap(), PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: NonZeroU16::new(55).unwrap(), + amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), amp_timestamp: 0, future_amp_timestamp: 0, @@ -50,51 +49,6 @@ fn update_pool_should_work_when_all_parames_are_updated() { }); } -#[test] -fn update_pool_should_work_when_only_amplification_is_updated() { - let asset_a: AssetId = 1; - let asset_b: AssetId = 2; - let pool_id: AssetId = 100; - - ExtBuilder::default() - .with_endowed_accounts(vec![(ALICE, asset_a, 200 * ONE), (ALICE, asset_b, 200 * ONE)]) - .with_registered_asset("pool".as_bytes().to_vec(), pool_id) - .with_registered_asset("one".as_bytes().to_vec(), asset_a) - .with_registered_asset("two".as_bytes().to_vec(), asset_b) - .build() - .execute_with(|| { - assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), - pool_id, - vec![asset_a, asset_b], - 100, - Permill::from_percent(0), - Permill::from_percent(0), - )); - - assert_ok!(Stableswap::update_pool( - RuntimeOrigin::signed(ALICE), - pool_id, - Some(55), - None, - None, - )); - - assert_eq!( - >::get(pool_id).unwrap(), - PoolInfo { - assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: NonZeroU16::new(55).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, - future_amp_timestamp: 0, - trade_fee: Permill::from_percent(0), - withdraw_fee: Permill::from_percent(0) - } - ); - }); -} - #[test] fn update_pool_should_work_when_only_trade_fee_is_updated() { let asset_a: AssetId = 1; @@ -117,10 +71,9 @@ fn update_pool_should_work_when_only_trade_fee_is_updated() { Permill::from_percent(0), )); - assert_ok!(Stableswap::update_pool( + assert_ok!(Stableswap::update_pool_fees( RuntimeOrigin::signed(ALICE), pool_id, - None, Some(Permill::from_percent(20)), None, )); @@ -162,11 +115,10 @@ fn update_pool_should_work_when_only_withdraw_fee_is_updated() { Permill::from_percent(0), )); - assert_ok!(Stableswap::update_pool( + assert_ok!(Stableswap::update_pool_fees( RuntimeOrigin::signed(ALICE), pool_id, None, - None, Some(Permill::from_percent(21)), )); @@ -207,10 +159,9 @@ fn update_pool_should_work_when_only_fees_is_updated() { Permill::from_percent(0), )); - assert_ok!(Stableswap::update_pool( + assert_ok!(Stableswap::update_pool_fees( RuntimeOrigin::signed(ALICE), pool_id, - None, Some(Permill::from_percent(11)), Some(Permill::from_percent(21)), )); @@ -253,7 +204,7 @@ fn update_pool_should_fail_when_nothing_is_to_update() { )); assert_noop!( - Stableswap::update_pool(RuntimeOrigin::signed(ALICE), pool_id, None, None, None), + Stableswap::update_pool_fees(RuntimeOrigin::signed(ALICE), pool_id, None, None), Error::::NothingToUpdate ); @@ -286,37 +237,13 @@ fn update_pool_should_fail_when_pool_does_not_exists() { let pool_id = retrieve_current_asset_id(); assert_noop!( - Stableswap::update_pool(RuntimeOrigin::signed(ALICE), pool_id, Some(100), None, None), + Stableswap::update_pool_fees( + RuntimeOrigin::signed(ALICE), + pool_id, + Some(Permill::from_percent(1)), + None + ), Error::::PoolNotFound ); }); } - -#[test] -fn update_pool_should_fail_when_amplification_is_outside_allowed_range() { - let asset_a: AssetId = 1; - let asset_b: AssetId = 2; - let pool_id: AssetId = 100; - - ExtBuilder::default() - .with_endowed_accounts(vec![(ALICE, asset_a, 200 * ONE), (ALICE, asset_b, 200 * ONE)]) - .with_registered_asset("pool".as_bytes().to_vec(), pool_id) - .with_registered_asset("one".as_bytes().to_vec(), asset_a) - .with_registered_asset("two".as_bytes().to_vec(), asset_b) - .build() - .execute_with(|| { - assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), - pool_id, - vec![asset_a, asset_b], - 100, - Permill::from_percent(0), - Permill::from_percent(0), - )); - - assert_noop!( - Stableswap::update_pool(RuntimeOrigin::signed(ALICE), pool_id, Some(20_000), None, None), - Error::::InvalidAmplification - ); - }); -} From 2fb640973675192103bcb0d093a1c6e684e5903e Mon Sep 17 00:00:00 2001 From: Martin Date: Tue, 4 Jul 2023 18:04:43 +0200 Subject: [PATCH 06/21] rename pool params --- pallets/stableswap/src/lib.rs | 68 +++++++------------ pallets/stableswap/src/tests/add_liquidity.rs | 20 +++--- pallets/stableswap/src/tests/creation.rs | 8 +-- pallets/stableswap/src/tests/invariants.rs | 26 ++++--- pallets/stableswap/src/tests/mock.rs | 2 +- .../stableswap/src/tests/remove_liquidity.rs | 20 +++--- pallets/stableswap/src/tests/trades.rs | 28 ++++---- pallets/stableswap/src/tests/update_pool.rs | 20 +++--- pallets/stableswap/src/types.rs | 4 +- 9 files changed, 89 insertions(+), 107 deletions(-) diff --git a/pallets/stableswap/src/lib.rs b/pallets/stableswap/src/lib.rs index 92aa1fee7..5a8b1c831 100644 --- a/pallets/stableswap/src/lib.rs +++ b/pallets/stableswap/src/lib.rs @@ -164,9 +164,8 @@ pub mod pallet { withdraw_fee: Permill, }, /// Pool parameters has been updated. - PoolUpdated { + FeesUpdated { pool_id: T::AssetId, - amplification: NonZeroU16, trade_fee: Permill, withdraw_fee: Permill, }, @@ -340,22 +339,19 @@ pub mod pallet { Ok(()) } - /// Update given stableswap pool's parameters. - /// - /// Updates one or more parameters of stablesswap pool ( amplification, trade fee, withdraw fee). + /// Update pool's fees. /// - /// If all parameters are none, `NothingToUpdate` error is returned. + /// Updates pool's trade fee and/or withdraw fee. /// /// if pool does not exist, `PoolNotFound` is returned. /// /// Parameters: /// - `origin`: Must be T::AuthorityOrigin /// - `pool_id`: pool to update - /// - `amplification`: new pool amplification or None /// - `trade_fee`: new trade fee or None /// - `withdraw_fee`: new withdraw fee or None /// - /// Emits `PoolUpdated` event if successful. + /// Emits `FeesUpdated` event if successful. #[pallet::call_index(1)] #[pallet::weight(::WeightInfo::update_pool())] #[transactional] @@ -377,9 +373,8 @@ pub mod pallet { pool.trade_fee = trade_fee.unwrap_or(pool.trade_fee); pool.withdraw_fee = withdraw_fee.unwrap_or(pool.withdraw_fee); - Self::deposit_event(Event::PoolUpdated { + Self::deposit_event(Event::FeesUpdated { pool_id, - amplification: pool.amplification, trade_fee: pool.trade_fee, withdraw_fee: pool.withdraw_fee, }); @@ -387,22 +382,15 @@ pub mod pallet { }) } - /// Update given stableswap pool's parameters. - /// - /// Updates one or more parameters of stablesswap pool ( amplification, trade fee, withdraw fee). - /// - /// If all parameters are none, `NothingToUpdate` error is returned. - /// - /// if pool does not exist, `PoolNotFound` is returned. + /// Update pool's amplification. /// /// Parameters: /// - `origin`: Must be T::AuthorityOrigin /// - `pool_id`: pool to update - /// - `amplification`: new pool amplification or None - /// - `trade_fee`: new trade fee or None - /// - `withdraw_fee`: new withdraw fee or None + /// - `future_amplification`: new desired pool amplification + /// - `future_timestamp`: future block number when the amplification is updated /// - /// Emits `PoolUpdated` event if successful. + /// Emits `AmplificationUpdated` event if successful. #[pallet::call_index(2)] #[pallet::weight(::WeightInfo::update_pool())] #[transactional] @@ -410,25 +398,21 @@ pub mod pallet { origin: OriginFor, pool_id: T::AssetId, future_amplification: u16, - block: T::BlockNumber, + future_timestamp: T::BlockNumber, ) -> DispatchResult { T::AuthorityOrigin::ensure_origin(origin)?; + let current_block = T::BlockNumberProvider::current_block_number(); + ensure!(future_timestamp > current_block, Error::::InvalidAmplification); + Pools::::try_mutate(pool_id, |maybe_pool| -> DispatchResult { let mut pool = maybe_pool.as_mut().ok_or(Error::::PoolNotFound)?; - pool.amplification = pool.future_amplification; + pool.initial_amplification = pool.future_amplification; pool.future_amplification = NonZeroU16::new(future_amplification).ok_or(Error::::InvalidAmplification)?; - - let current_block = T::BlockNumberProvider::current_block_number(); - ensure!( - block > current_block && block > pool.future_amp_timestamp, - Error::::InvalidAmplification - ); - - pool.amp_timestamp = current_block; - pool.future_amp_timestamp = block; + pool.initial_amp_timestamp = current_block; + pool.future_amp_timestamp = future_timestamp; ensure!( T::AmplificationRange::get().contains(&pool.future_amplification), @@ -537,9 +521,9 @@ pub mod pallet { ); let amplification = hydra_dx_math::stableswap::calculate_amplification( - pool.amplification.get().into(), + pool.initial_amplification.get().into(), pool.future_amplification.get().into(), - pool.amp_timestamp.saturated_into(), + pool.initial_amp_timestamp.saturated_into(), pool.future_amp_timestamp.saturated_into(), T::BlockNumberProvider::current_block_number().saturated_into(), ); @@ -743,9 +727,9 @@ impl Pallet { ensure!(balances[index_out] > Balance::zero(), Error::::InsufficientLiquidity); let amplification = hydra_dx_math::stableswap::calculate_amplification( - pool.amplification.get().into(), + pool.initial_amplification.get().into(), pool.future_amplification.get().into(), - pool.amp_timestamp.saturated_into(), + pool.initial_amp_timestamp.saturated_into(), pool.future_amp_timestamp.saturated_into(), T::BlockNumberProvider::current_block_number().saturated_into(), ); @@ -779,9 +763,9 @@ impl Pallet { ensure!(balances[index_in] > Balance::zero(), Error::::InsufficientLiquidity); let amplification = hydra_dx_math::stableswap::calculate_amplification( - pool.amplification.get().into(), + pool.initial_amplification.get().into(), pool.future_amplification.get().into(), - pool.amp_timestamp.saturated_into(), + pool.initial_amp_timestamp.saturated_into(), pool.future_amp_timestamp.saturated_into(), T::BlockNumberProvider::current_block_number().saturated_into(), ); @@ -823,9 +807,9 @@ impl Pallet { .clone() .try_into() .map_err(|_| Error::::MaxAssetsExceeded)?, - amplification, + initial_amplification: amplification, future_amplification: amplification, - amp_timestamp: block_number, + initial_amp_timestamp: block_number, future_amp_timestamp: block_number, trade_fee, withdraw_fee, @@ -885,9 +869,9 @@ impl Pallet { } let amplification = hydra_dx_math::stableswap::calculate_amplification( - pool.amplification.get().into(), + pool.initial_amplification.get().into(), pool.future_amplification.get().into(), - pool.amp_timestamp.saturated_into(), + pool.initial_amp_timestamp.saturated_into(), pool.future_amp_timestamp.saturated_into(), T::BlockNumberProvider::current_block_number().saturated_into(), ); diff --git a/pallets/stableswap/src/tests/add_liquidity.rs b/pallets/stableswap/src/tests/add_liquidity.rs index 3754bd1ec..e34f580db 100644 --- a/pallets/stableswap/src/tests/add_liquidity.rs +++ b/pallets/stableswap/src/tests/add_liquidity.rs @@ -135,9 +135,9 @@ fn add_liquidity_should_work_when_initial_liquidity_has_been_provided() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), @@ -205,9 +205,9 @@ fn add_liquidity_should_work_when_order_is_not_sorted() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), @@ -275,9 +275,9 @@ fn add_liquidity_should_fail_when_providing_insufficient_liquidity() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), @@ -344,9 +344,9 @@ fn add_liquidity_should_work_when_providing_one_asset_only() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b, asset_c, asset_d].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), @@ -415,9 +415,9 @@ fn add_liquidity_should_fail_when_providing_one_asset_not_in_pool() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b, asset_c, asset_d].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), diff --git a/pallets/stableswap/src/tests/creation.rs b/pallets/stableswap/src/tests/creation.rs index d63f51def..88077d396 100644 --- a/pallets/stableswap/src/tests/creation.rs +++ b/pallets/stableswap/src/tests/creation.rs @@ -32,9 +32,9 @@ fn create_two_asset_pool_should_work_when_assets_are_registered() { >::get(pool_id).unwrap(), PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0) @@ -104,9 +104,9 @@ fn create_pool_should_store_assets_correctly_when_input_is_not_sorted() { >::get(pool_id).unwrap(), PoolInfo { assets: vec![asset_a, asset_b, asset_c, asset_d].try_into().unwrap(), - amplification: NonZeroU16::new(amplification).unwrap(), + initial_amplification: NonZeroU16::new(amplification).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(5), withdraw_fee: Permill::from_percent(10) diff --git a/pallets/stableswap/src/tests/invariants.rs b/pallets/stableswap/src/tests/invariants.rs index 5cb4e53d2..b9b6b2666 100644 --- a/pallets/stableswap/src/tests/invariants.rs +++ b/pallets/stableswap/src/tests/invariants.rs @@ -64,11 +64,10 @@ proptest! { ALICE, PoolInfo:: { assets: vec![asset_a,asset_b].try_into().unwrap(), - amplification, - - future_amplification: amplification, - amp_timestamp: 0, - future_amp_timestamp: 0, + initial_amplification: amplification, + future_amplification: amplification, + initial_amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee, withdraw_fee: Permill::from_percent(0), }, @@ -142,11 +141,10 @@ proptest! { ALICE, PoolInfo:: { assets: vec![asset_a,asset_b].try_into().unwrap(), - amplification, - - future_amplification: amplification, - amp_timestamp: 0, - future_amp_timestamp: 0, + initial_amplification: amplification, + future_amplification: amplification, + initial_amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -214,10 +212,10 @@ proptest! { ALICE, PoolInfo:: { assets: vec![asset_a,asset_b].try_into().unwrap(), - amplification, - future_amplification: amplification, - amp_timestamp: 0, - future_amp_timestamp: 0, + initial_amplification: amplification, + future_amplification: amplification, + initial_amp_timestamp: 0, + future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, diff --git a/pallets/stableswap/src/tests/mock.rs b/pallets/stableswap/src/tests/mock.rs index 91e56c08d..716e1a6b2 100644 --- a/pallets/stableswap/src/tests/mock.rs +++ b/pallets/stableswap/src/tests/mock.rs @@ -248,7 +248,7 @@ impl ExtBuilder { RuntimeOrigin::signed(who), pool_id, pool.assets.clone().into(), - pool.amplification.get(), + pool.initial_amplification.get(), pool.trade_fee, pool.withdraw_fee, )); diff --git a/pallets/stableswap/src/tests/remove_liquidity.rs b/pallets/stableswap/src/tests/remove_liquidity.rs index 030c50628..dc6c78e8a 100644 --- a/pallets/stableswap/src/tests/remove_liquidity.rs +++ b/pallets/stableswap/src/tests/remove_liquidity.rs @@ -25,9 +25,9 @@ fn remove_liquidity_should_work_when_withdrawing_all_shares() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b, asset_c].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), @@ -105,9 +105,9 @@ fn remove_liquidity_should_apply_fee_when_withdrawing_all_shares() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b, asset_c].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(10), @@ -243,9 +243,9 @@ fn remove_liquidity_should_fail_when_requested_asset_not_in_pool() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b, asset_c].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(10), @@ -312,9 +312,9 @@ fn remove_liquidity_should_fail_when_remaining_shares_below_min_liquidity() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b, asset_c].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(10), @@ -389,9 +389,9 @@ fn verify_remove_liquidity_against_research_impl() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b, asset_c, asset_d].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_float(0.003), withdraw_fee: Permill::from_float(0.003), diff --git a/pallets/stableswap/src/tests/trades.rs b/pallets/stableswap/src/tests/trades.rs index bfdc83de6..41d901202 100644 --- a/pallets/stableswap/src/tests/trades.rs +++ b/pallets/stableswap/src/tests/trades.rs @@ -18,9 +18,9 @@ fn sell_should_work_when_correct_input_provided() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), @@ -75,9 +75,9 @@ fn buy_should_work_when_correct_input_provided() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), @@ -133,9 +133,9 @@ fn sell_with_fee_should_work_when_correct_input_provided() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(10), withdraw_fee: Permill::from_percent(0), @@ -194,9 +194,9 @@ fn sell_should_work_when_fee_is_small() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_rational(3u32, 1000u32), withdraw_fee: Permill::from_percent(0), @@ -255,9 +255,9 @@ fn buy_should_work_when_fee_is_set() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(10), withdraw_fee: Permill::from_percent(0), @@ -321,9 +321,9 @@ fn sell_should_fail_when_insufficient_amount_is_provided() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), @@ -414,9 +414,9 @@ fn buy_should_fail_when_insufficient_amount_is_provided() { ALICE, PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), diff --git a/pallets/stableswap/src/tests/update_pool.rs b/pallets/stableswap/src/tests/update_pool.rs index def4e72ee..8a16fc18f 100644 --- a/pallets/stableswap/src/tests/update_pool.rs +++ b/pallets/stableswap/src/tests/update_pool.rs @@ -38,9 +38,9 @@ fn update_pool_should_work_when_all_parames_are_updated() { >::get(pool_id).unwrap(), PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(10), withdraw_fee: Permill::from_percent(20) @@ -82,9 +82,9 @@ fn update_pool_should_work_when_only_trade_fee_is_updated() { >::get(pool_id).unwrap(), PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(20), withdraw_fee: Permill::from_percent(0) @@ -126,9 +126,9 @@ fn update_pool_should_work_when_only_withdraw_fee_is_updated() { >::get(pool_id).unwrap(), PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(21) @@ -170,9 +170,9 @@ fn update_pool_should_work_when_only_fees_is_updated() { >::get(pool_id).unwrap(), PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(11), withdraw_fee: Permill::from_percent(21) @@ -212,9 +212,9 @@ fn update_pool_should_fail_when_nothing_is_to_update() { >::get(pool_id).unwrap(), PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), - amplification: NonZeroU16::new(100).unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), future_amplification: NonZeroU16::new(100).unwrap(), - amp_timestamp: 0, + initial_amp_timestamp: 0, future_amp_timestamp: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0) diff --git a/pallets/stableswap/src/types.rs b/pallets/stableswap/src/types.rs index 2c97b271d..9303a3e7c 100644 --- a/pallets/stableswap/src/types.rs +++ b/pallets/stableswap/src/types.rs @@ -24,9 +24,9 @@ pub(crate) type Balance = u128; #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] pub struct PoolInfo { pub assets: BoundedVec>, - pub amplification: NonZeroU16, + pub initial_amplification: NonZeroU16, pub future_amplification: NonZeroU16, - pub amp_timestamp: BlockNumber, + pub initial_amp_timestamp: BlockNumber, pub future_amp_timestamp: BlockNumber, pub trade_fee: Permill, pub withdraw_fee: Permill, From cb432449b75f315996e64ae087b81611aae248f2 Mon Sep 17 00:00:00 2001 From: Martin Date: Tue, 4 Jul 2023 18:06:29 +0200 Subject: [PATCH 07/21] use better error message --- pallets/stableswap/src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pallets/stableswap/src/lib.rs b/pallets/stableswap/src/lib.rs index 5a8b1c831..bafa15edf 100644 --- a/pallets/stableswap/src/lib.rs +++ b/pallets/stableswap/src/lib.rs @@ -292,6 +292,9 @@ pub mod pallet { /// Not allowed to perform an operation on given asset. NotAllowed, + + /// Future block timestamp is in the past. + InvalidTimestamp, } #[pallet::call] @@ -403,7 +406,7 @@ pub mod pallet { T::AuthorityOrigin::ensure_origin(origin)?; let current_block = T::BlockNumberProvider::current_block_number(); - ensure!(future_timestamp > current_block, Error::::InvalidAmplification); + ensure!(future_timestamp > current_block, Error::::InvalidTimestamp); Pools::::try_mutate(pool_id, |maybe_pool| -> DispatchResult { let mut pool = maybe_pool.as_mut().ok_or(Error::::PoolNotFound)?; From 683c6a5492652c7ef30ae7a72066fcf27f6e95e9 Mon Sep 17 00:00:00 2001 From: Martin Date: Tue, 4 Jul 2023 18:08:51 +0200 Subject: [PATCH 08/21] reformat --- math/src/stableswap/math.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/math/src/stableswap/math.rs b/math/src/stableswap/math.rs index c9762322b..f9fa4a911 100644 --- a/math/src/stableswap/math.rs +++ b/math/src/stableswap/math.rs @@ -355,7 +355,7 @@ pub fn calculate_amplification( current_timestamp: u128, ) -> u128 { // short circuit if timestamp are invalid - if current_timestamp < initial_timestamp || future_timestamp < initial_timestamp { + if current_timestamp < initial_timestamp || future_timestamp < initial_timestamp { return initial_amplification; } From ff4b2b3cee2e7486c736a07c986565133bf845d8 Mon Sep 17 00:00:00 2001 From: Martin Date: Tue, 4 Jul 2023 18:10:41 +0200 Subject: [PATCH 09/21] use correct non-std --- math/src/stableswap/math.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/math/src/stableswap/math.rs b/math/src/stableswap/math.rs index f9fa4a911..1cabc0188 100644 --- a/math/src/stableswap/math.rs +++ b/math/src/stableswap/math.rs @@ -3,8 +3,8 @@ use crate::types::Balance; use num_traits::{CheckedDiv, CheckedMul, Zero}; use primitive_types::U256; use sp_arithmetic::{FixedPointNumber, FixedU128, Permill}; +use sp_std::ops::Div; use sp_std::prelude::*; -use std::ops::Div; pub const MAX_Y_ITERATIONS: u8 = 128; pub const MAX_D_ITERATIONS: u8 = 64; @@ -374,11 +374,11 @@ pub fn calculate_amplification( .saturating_mul(current_timestamp.saturating_sub(initial_timestamp)) .div(future_timestamp.saturating_sub(initial_timestamp)); - return if future_amplification > initial_amplification { + if future_amplification > initial_amplification { initial_amplification.saturating_add(step) } else { initial_amplification.saturating_sub(step) - }; + } } #[inline] From 7c89defcf0f85022cf6bfa2166f8ab8a30daacac Mon Sep 17 00:00:00 2001 From: Martin Date: Tue, 4 Jul 2023 18:12:54 +0200 Subject: [PATCH 10/21] temporarily remove update pool benchmark - need to be updated for fees and ampl --- pallets/stableswap/src/benchmarks.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pallets/stableswap/src/benchmarks.rs b/pallets/stableswap/src/benchmarks.rs index 7af533293..24dce790f 100644 --- a/pallets/stableswap/src/benchmarks.rs +++ b/pallets/stableswap/src/benchmarks.rs @@ -179,7 +179,6 @@ benchmarks! { assert_eq!(T::Currency::free_balance(asset_id_to_withdraw, &lp_provider), 1296846466078107); } - sell{ let caller: T::AccountId = account("caller", 0, 1); let lp_provider: T::AccountId = account("provider", 0, 1); @@ -351,6 +350,7 @@ benchmarks! { assert_ne!(asset_tradability_old, asset_tradability_new); } + /* update_pool { let caller: T::AccountId = account("caller", 0, 1); let lp_provider: T::AccountId = account("provider", 0, 1); @@ -393,10 +393,11 @@ benchmarks! { }: _(successful_origin, pool_id, amplification_new, trade_fee_new, withdraw_fee_new) verify { let pool = crate::Pallet::::pools(pool_id).unwrap(); - assert_eq!(pool.amplification, amplification_new.unwrap()); + assert_eq!(pool.initial_amplification, amplification_new.unwrap()); assert_eq!(pool.trade_fee, trade_fee_new.unwrap()); assert_eq!(pool.withdraw_fee, withdraw_fee_new.unwrap()); } + */ impl_benchmark_test_suite!(Pallet, crate::tests::mock::ExtBuilder::default().build(), crate::tests::mock::Test); } From 1daf5d534fae70cadd8e019b41ba63230901e6ad Mon Sep 17 00:00:00 2001 From: Martin Hloska Date: Tue, 4 Jul 2023 22:06:00 +0200 Subject: [PATCH 11/21] renamed amplification fields --- math/src/stableswap/math.rs | 31 ++++---- pallets/stableswap/src/lib.rs | 73 ++++++++++--------- pallets/stableswap/src/tests/add_liquidity.rs | 30 ++++---- pallets/stableswap/src/tests/creation.rs | 12 +-- pallets/stableswap/src/tests/invariants.rs | 18 ++--- .../stableswap/src/tests/remove_liquidity.rs | 30 ++++---- pallets/stableswap/src/tests/trades.rs | 42 +++++------ pallets/stableswap/src/tests/update_pool.rs | 30 ++++---- pallets/stableswap/src/types.rs | 6 +- 9 files changed, 137 insertions(+), 135 deletions(-) diff --git a/math/src/stableswap/math.rs b/math/src/stableswap/math.rs index 1cabc0188..2d66d58ae 100644 --- a/math/src/stableswap/math.rs +++ b/math/src/stableswap/math.rs @@ -349,32 +349,27 @@ pub(crate) fn calculate_y(xp: &[Balance], d: Balance, amplification pub fn calculate_amplification( initial_amplification: u128, - future_amplification: u128, - initial_timestamp: u128, - future_timestamp: u128, - current_timestamp: u128, + final_amplification: u128, + initial_block: u128, + final_block: u128, + current_block: u128, ) -> u128 { - // short circuit if timestamp are invalid - if current_timestamp < initial_timestamp || future_timestamp < initial_timestamp { + // short circuit if block parameters are invalid or start block is not reached yet + if current_block < initial_block || final_block <= initial_block { return initial_amplification; } - // Short circuit if timestamp or amplification are equal - if initial_timestamp == future_timestamp || initial_amplification == future_amplification { - return future_amplification; + // short circuit if already reached desired block + if current_block >= final_block { + return final_amplification; } - // short circuit if already reached desired timestamp - if current_timestamp >= future_timestamp { - return future_amplification; - } - - let step = future_amplification + let step = final_amplification .abs_diff(initial_amplification) - .saturating_mul(current_timestamp.saturating_sub(initial_timestamp)) - .div(future_timestamp.saturating_sub(initial_timestamp)); + .saturating_mul(current_block.saturating_sub(initial_block)) + .div(final_block.saturating_sub(initial_block)); - if future_amplification > initial_amplification { + if final_amplification > initial_amplification { initial_amplification.saturating_add(step) } else { initial_amplification.saturating_sub(step) diff --git a/pallets/stableswap/src/lib.rs b/pallets/stableswap/src/lib.rs index bafa15edf..54d7c082f 100644 --- a/pallets/stableswap/src/lib.rs +++ b/pallets/stableswap/src/lib.rs @@ -213,11 +213,13 @@ pub mod pallet { state: Tradability, }, - /// - AmplificationUpdated { + /// AAmplification of a pool has been scheduled to change. + AmplificationChanging { pool_id: T::AssetId, - amplification: NonZeroU16, - block: T::BlockNumber, + current_amplification: NonZeroU16, + final_amplification: NonZeroU16, + start_block: T::BlockNumber, + end_block: T::BlockNumber, }, } @@ -293,8 +295,8 @@ pub mod pallet { /// Not allowed to perform an operation on given asset. NotAllowed, - /// Future block timestamp is in the past. - InvalidTimestamp, + /// Future block number is in the past. + InvalidBlock, } #[pallet::call] @@ -391,7 +393,7 @@ pub mod pallet { /// - `origin`: Must be T::AuthorityOrigin /// - `pool_id`: pool to update /// - `future_amplification`: new desired pool amplification - /// - `future_timestamp`: future block number when the amplification is updated + /// - `future_block`: future block number when the amplification is updated /// /// Emits `AmplificationUpdated` event if successful. #[pallet::call_index(2)] @@ -401,30 +403,35 @@ pub mod pallet { origin: OriginFor, pool_id: T::AssetId, future_amplification: u16, - future_timestamp: T::BlockNumber, + start_block: T::BlockNumber, + end_block: T::BlockNumber, ) -> DispatchResult { T::AuthorityOrigin::ensure_origin(origin)?; - let current_block = T::BlockNumberProvider::current_block_number(); - ensure!(future_timestamp > current_block, Error::::InvalidTimestamp); + ensure!( + end_block > start_block && end_block > current_block && start_block >= current_block, + Error::::InvalidBlock + ); Pools::::try_mutate(pool_id, |maybe_pool| -> DispatchResult { let mut pool = maybe_pool.as_mut().ok_or(Error::::PoolNotFound)?; - pool.initial_amplification = pool.future_amplification; - pool.future_amplification = + pool.initial_amplification = pool.final_amplification; + pool.final_amplification = NonZeroU16::new(future_amplification).ok_or(Error::::InvalidAmplification)?; - pool.initial_amp_timestamp = current_block; - pool.future_amp_timestamp = future_timestamp; + pool.initial_block = start_block; + pool.final_block = end_block; ensure!( - T::AmplificationRange::get().contains(&pool.future_amplification), + T::AmplificationRange::get().contains(&pool.final_amplification), Error::::InvalidAmplification ); - Self::deposit_event(Event::AmplificationUpdated { + Self::deposit_event(Event::AmplificationChanging { pool_id, - amplification: pool.future_amplification, - block: pool.future_amp_timestamp, + current_amplification: pool.initial_amplification, + final_amplification: pool.final_amplification, + start_block: pool.initial_block, + end_block: pool.final_block, }); Ok(()) }) @@ -525,9 +532,9 @@ pub mod pallet { let amplification = hydra_dx_math::stableswap::calculate_amplification( pool.initial_amplification.get().into(), - pool.future_amplification.get().into(), - pool.initial_amp_timestamp.saturated_into(), - pool.future_amp_timestamp.saturated_into(), + pool.final_amplification.get().into(), + pool.initial_block.saturated_into(), + pool.final_block.saturated_into(), T::BlockNumberProvider::current_block_number().saturated_into(), ); @@ -731,9 +738,9 @@ impl Pallet { let amplification = hydra_dx_math::stableswap::calculate_amplification( pool.initial_amplification.get().into(), - pool.future_amplification.get().into(), - pool.initial_amp_timestamp.saturated_into(), - pool.future_amp_timestamp.saturated_into(), + pool.final_amplification.get().into(), + pool.initial_block.saturated_into(), + pool.final_block.saturated_into(), T::BlockNumberProvider::current_block_number().saturated_into(), ); @@ -767,9 +774,9 @@ impl Pallet { let amplification = hydra_dx_math::stableswap::calculate_amplification( pool.initial_amplification.get().into(), - pool.future_amplification.get().into(), - pool.initial_amp_timestamp.saturated_into(), - pool.future_amp_timestamp.saturated_into(), + pool.final_amplification.get().into(), + pool.initial_block.saturated_into(), + pool.final_block.saturated_into(), T::BlockNumberProvider::current_block_number().saturated_into(), ); @@ -811,9 +818,9 @@ impl Pallet { .try_into() .map_err(|_| Error::::MaxAssetsExceeded)?, initial_amplification: amplification, - future_amplification: amplification, - initial_amp_timestamp: block_number, - future_amp_timestamp: block_number, + final_amplification: amplification, + initial_block: block_number, + final_block: block_number, trade_fee, withdraw_fee, }; @@ -873,9 +880,9 @@ impl Pallet { let amplification = hydra_dx_math::stableswap::calculate_amplification( pool.initial_amplification.get().into(), - pool.future_amplification.get().into(), - pool.initial_amp_timestamp.saturated_into(), - pool.future_amp_timestamp.saturated_into(), + pool.final_amplification.get().into(), + pool.initial_block.saturated_into(), + pool.final_block.saturated_into(), T::BlockNumberProvider::current_block_number().saturated_into(), ); diff --git a/pallets/stableswap/src/tests/add_liquidity.rs b/pallets/stableswap/src/tests/add_liquidity.rs index e34f580db..ef40da573 100644 --- a/pallets/stableswap/src/tests/add_liquidity.rs +++ b/pallets/stableswap/src/tests/add_liquidity.rs @@ -136,9 +136,9 @@ fn add_liquidity_should_work_when_initial_liquidity_has_been_provided() { PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -206,9 +206,9 @@ fn add_liquidity_should_work_when_order_is_not_sorted() { PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -276,9 +276,9 @@ fn add_liquidity_should_fail_when_providing_insufficient_liquidity() { PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -345,9 +345,9 @@ fn add_liquidity_should_work_when_providing_one_asset_only() { PoolInfo:: { assets: vec![asset_a, asset_b, asset_c, asset_d].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -416,9 +416,9 @@ fn add_liquidity_should_fail_when_providing_one_asset_not_in_pool() { PoolInfo:: { assets: vec![asset_a, asset_b, asset_c, asset_d].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, diff --git a/pallets/stableswap/src/tests/creation.rs b/pallets/stableswap/src/tests/creation.rs index 88077d396..5d8f9b688 100644 --- a/pallets/stableswap/src/tests/creation.rs +++ b/pallets/stableswap/src/tests/creation.rs @@ -33,9 +33,9 @@ fn create_two_asset_pool_should_work_when_assets_are_registered() { PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0) } @@ -105,9 +105,9 @@ fn create_pool_should_store_assets_correctly_when_input_is_not_sorted() { PoolInfo { assets: vec![asset_a, asset_b, asset_c, asset_d].try_into().unwrap(), initial_amplification: NonZeroU16::new(amplification).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(5), withdraw_fee: Permill::from_percent(10) } diff --git a/pallets/stableswap/src/tests/invariants.rs b/pallets/stableswap/src/tests/invariants.rs index b9b6b2666..3851f3445 100644 --- a/pallets/stableswap/src/tests/invariants.rs +++ b/pallets/stableswap/src/tests/invariants.rs @@ -65,9 +65,9 @@ proptest! { PoolInfo:: { assets: vec![asset_a,asset_b].try_into().unwrap(), initial_amplification: amplification, - future_amplification: amplification, - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: amplification, + initial_block: 0, + final_block: 0, trade_fee, withdraw_fee: Permill::from_percent(0), }, @@ -142,9 +142,9 @@ proptest! { PoolInfo:: { assets: vec![asset_a,asset_b].try_into().unwrap(), initial_amplification: amplification, - future_amplification: amplification, - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: amplification, + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -213,9 +213,9 @@ proptest! { PoolInfo:: { assets: vec![asset_a,asset_b].try_into().unwrap(), initial_amplification: amplification, - future_amplification: amplification, - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: amplification, + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, diff --git a/pallets/stableswap/src/tests/remove_liquidity.rs b/pallets/stableswap/src/tests/remove_liquidity.rs index dc6c78e8a..110f70801 100644 --- a/pallets/stableswap/src/tests/remove_liquidity.rs +++ b/pallets/stableswap/src/tests/remove_liquidity.rs @@ -26,9 +26,9 @@ fn remove_liquidity_should_work_when_withdrawing_all_shares() { PoolInfo:: { assets: vec![asset_a, asset_b, asset_c].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -106,9 +106,9 @@ fn remove_liquidity_should_apply_fee_when_withdrawing_all_shares() { PoolInfo:: { assets: vec![asset_a, asset_b, asset_c].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(10), }, @@ -244,9 +244,9 @@ fn remove_liquidity_should_fail_when_requested_asset_not_in_pool() { PoolInfo:: { assets: vec![asset_a, asset_b, asset_c].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(10), }, @@ -313,9 +313,9 @@ fn remove_liquidity_should_fail_when_remaining_shares_below_min_liquidity() { PoolInfo:: { assets: vec![asset_a, asset_b, asset_c].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(10), }, @@ -390,9 +390,9 @@ fn verify_remove_liquidity_against_research_impl() { PoolInfo:: { assets: vec![asset_a, asset_b, asset_c, asset_d].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_float(0.003), withdraw_fee: Permill::from_float(0.003), }, diff --git a/pallets/stableswap/src/tests/trades.rs b/pallets/stableswap/src/tests/trades.rs index 41d901202..f4909cad5 100644 --- a/pallets/stableswap/src/tests/trades.rs +++ b/pallets/stableswap/src/tests/trades.rs @@ -19,9 +19,9 @@ fn sell_should_work_when_correct_input_provided() { PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -76,9 +76,9 @@ fn buy_should_work_when_correct_input_provided() { PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -134,9 +134,9 @@ fn sell_with_fee_should_work_when_correct_input_provided() { PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(10), withdraw_fee: Permill::from_percent(0), }, @@ -195,9 +195,9 @@ fn sell_should_work_when_fee_is_small() { PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_rational(3u32, 1000u32), withdraw_fee: Permill::from_percent(0), }, @@ -256,9 +256,9 @@ fn buy_should_work_when_fee_is_set() { PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(10), withdraw_fee: Permill::from_percent(0), }, @@ -322,9 +322,9 @@ fn sell_should_fail_when_insufficient_amount_is_provided() { PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, @@ -415,9 +415,9 @@ fn buy_should_fail_when_insufficient_amount_is_provided() { PoolInfo:: { assets: vec![asset_a, asset_b].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0), }, diff --git a/pallets/stableswap/src/tests/update_pool.rs b/pallets/stableswap/src/tests/update_pool.rs index 8a16fc18f..92df573e7 100644 --- a/pallets/stableswap/src/tests/update_pool.rs +++ b/pallets/stableswap/src/tests/update_pool.rs @@ -39,9 +39,9 @@ fn update_pool_should_work_when_all_parames_are_updated() { PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(10), withdraw_fee: Permill::from_percent(20) } @@ -83,9 +83,9 @@ fn update_pool_should_work_when_only_trade_fee_is_updated() { PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(20), withdraw_fee: Permill::from_percent(0) } @@ -127,9 +127,9 @@ fn update_pool_should_work_when_only_withdraw_fee_is_updated() { PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(21) } @@ -171,9 +171,9 @@ fn update_pool_should_work_when_only_fees_is_updated() { PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(11), withdraw_fee: Permill::from_percent(21) } @@ -213,9 +213,9 @@ fn update_pool_should_fail_when_nothing_is_to_update() { PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), initial_amplification: NonZeroU16::new(100).unwrap(), - future_amplification: NonZeroU16::new(100).unwrap(), - initial_amp_timestamp: 0, - future_amp_timestamp: 0, + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, trade_fee: Permill::from_percent(0), withdraw_fee: Permill::from_percent(0) } diff --git a/pallets/stableswap/src/types.rs b/pallets/stableswap/src/types.rs index 9303a3e7c..4d753a567 100644 --- a/pallets/stableswap/src/types.rs +++ b/pallets/stableswap/src/types.rs @@ -25,9 +25,9 @@ pub(crate) type Balance = u128; pub struct PoolInfo { pub assets: BoundedVec>, pub initial_amplification: NonZeroU16, - pub future_amplification: NonZeroU16, - pub initial_amp_timestamp: BlockNumber, - pub future_amp_timestamp: BlockNumber, + pub final_amplification: NonZeroU16, + pub initial_block: BlockNumber, + pub final_block: BlockNumber, pub trade_fee: Permill, pub withdraw_fee: Permill, } From 45addda1ca9e5e3882e1d4937debb21820894c1b Mon Sep 17 00:00:00 2001 From: Martin Hloska Date: Tue, 4 Jul 2023 22:36:47 +0200 Subject: [PATCH 12/21] add additional checks when chagning amplification --- pallets/stableswap/src/lib.rs | 20 +- pallets/stableswap/src/tests/amplification.rs | 223 ++++++++++++++++++ pallets/stableswap/src/tests/mod.rs | 1 + 3 files changed, 242 insertions(+), 2 deletions(-) create mode 100644 pallets/stableswap/src/tests/amplification.rs diff --git a/pallets/stableswap/src/lib.rs b/pallets/stableswap/src/lib.rs index 54d7c082f..16a89aca1 100644 --- a/pallets/stableswap/src/lib.rs +++ b/pallets/stableswap/src/lib.rs @@ -297,6 +297,12 @@ pub mod pallet { /// Future block number is in the past. InvalidBlock, + + /// Current amplification change has not completed yet. + AmplificationChangeNotCompleted, + + /// New amplification is equal to the previous value. + SameAmplification, } #[pallet::call] @@ -402,11 +408,12 @@ pub mod pallet { pub fn update_amplification( origin: OriginFor, pool_id: T::AssetId, - future_amplification: u16, + final_amplification: u16, start_block: T::BlockNumber, end_block: T::BlockNumber, ) -> DispatchResult { T::AuthorityOrigin::ensure_origin(origin)?; + let current_block = T::BlockNumberProvider::current_block_number(); ensure!( end_block > start_block && end_block > current_block && start_block >= current_block, @@ -416,9 +423,18 @@ pub mod pallet { Pools::::try_mutate(pool_id, |maybe_pool| -> DispatchResult { let mut pool = maybe_pool.as_mut().ok_or(Error::::PoolNotFound)?; + ensure!( + pool.final_block <= current_block, + Error::::AmplificationChangeNotCompleted + ); + ensure!( + pool.final_amplification.get() != final_amplification, + Error::::SameAmplification + ); + pool.initial_amplification = pool.final_amplification; pool.final_amplification = - NonZeroU16::new(future_amplification).ok_or(Error::::InvalidAmplification)?; + NonZeroU16::new(final_amplification).ok_or(Error::::InvalidAmplification)?; pool.initial_block = start_block; pool.final_block = end_block; diff --git a/pallets/stableswap/src/tests/amplification.rs b/pallets/stableswap/src/tests/amplification.rs new file mode 100644 index 000000000..6eeaa5866 --- /dev/null +++ b/pallets/stableswap/src/tests/amplification.rs @@ -0,0 +1,223 @@ +use crate::tests::mock::*; +use crate::types::PoolInfo; +use crate::{Error, Pools}; +use frame_support::{assert_noop, assert_ok}; +use sp_runtime::traits::BlockNumberProvider; +use sp_runtime::Permill; +use std::num::NonZeroU16; + +#[test] +fn update_amplification_should_work_when_correct_params_are_provided() { + let asset_a: AssetId = 1; + let asset_b: AssetId = 2; + let pool_id: AssetId = 100; + + ExtBuilder::default() + .with_endowed_accounts(vec![(ALICE, asset_a, 200 * ONE), (ALICE, asset_b, 200 * ONE)]) + .with_registered_asset("pool".as_bytes().to_vec(), pool_id) + .with_registered_asset("one".as_bytes().to_vec(), asset_a) + .with_registered_asset("two".as_bytes().to_vec(), asset_b) + .build() + .execute_with(|| { + assert_ok!(Stableswap::create_pool( + RuntimeOrigin::signed(ALICE), + pool_id, + vec![asset_a, asset_b], + 100, + Permill::from_percent(10), + Permill::from_percent(20), + )); + + System::set_block_number(2); + let b = System::current_block_number(); + dbg!(b); + + assert_ok!(Stableswap::update_amplification( + RuntimeOrigin::signed(ALICE), + pool_id, + 1000, + 10, + 1000, + )); + + assert_eq!( + >::get(pool_id).unwrap(), + PoolInfo { + assets: vec![asset_a, asset_b].try_into().unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), + final_amplification: NonZeroU16::new(1000).unwrap(), + initial_block: 10, + final_block: 1000, + trade_fee: Permill::from_percent(10), + withdraw_fee: Permill::from_percent(20) + } + ); + }); +} + +#[test] +fn update_amplification_should_fail_when_end_block_is_before_current_block() { + let asset_a: AssetId = 1; + let asset_b: AssetId = 2; + let pool_id: AssetId = 100; + + ExtBuilder::default() + .with_endowed_accounts(vec![(ALICE, asset_a, 200 * ONE), (ALICE, asset_b, 200 * ONE)]) + .with_registered_asset("pool".as_bytes().to_vec(), pool_id) + .with_registered_asset("one".as_bytes().to_vec(), asset_a) + .with_registered_asset("two".as_bytes().to_vec(), asset_b) + .build() + .execute_with(|| { + assert_ok!(Stableswap::create_pool( + RuntimeOrigin::signed(ALICE), + pool_id, + vec![asset_a, asset_b], + 100, + Permill::from_percent(10), + Permill::from_percent(20), + )); + + System::set_block_number(5000); + + assert_noop!( + Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, 1000, 10, 1000), + Error::::InvalidBlock + ); + }); +} + +#[test] +fn update_amplification_should_fail_when_end_block_is_smaller_than_start_block() { + let asset_a: AssetId = 1; + let asset_b: AssetId = 2; + let pool_id: AssetId = 100; + + ExtBuilder::default() + .with_endowed_accounts(vec![(ALICE, asset_a, 200 * ONE), (ALICE, asset_b, 200 * ONE)]) + .with_registered_asset("pool".as_bytes().to_vec(), pool_id) + .with_registered_asset("one".as_bytes().to_vec(), asset_a) + .with_registered_asset("two".as_bytes().to_vec(), asset_b) + .build() + .execute_with(|| { + assert_ok!(Stableswap::create_pool( + RuntimeOrigin::signed(ALICE), + pool_id, + vec![asset_a, asset_b], + 100, + Permill::from_percent(10), + Permill::from_percent(20), + )); + + System::set_block_number(5000); + + assert_noop!( + Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, 1000, 20_000, 10_000), + Error::::InvalidBlock + ); + }); +} + +#[test] +fn update_amplification_should_fail_when_start_block_before_current_block() { + let asset_a: AssetId = 1; + let asset_b: AssetId = 2; + let pool_id: AssetId = 100; + + ExtBuilder::default() + .with_endowed_accounts(vec![(ALICE, asset_a, 200 * ONE), (ALICE, asset_b, 200 * ONE)]) + .with_registered_asset("pool".as_bytes().to_vec(), pool_id) + .with_registered_asset("one".as_bytes().to_vec(), asset_a) + .with_registered_asset("two".as_bytes().to_vec(), asset_b) + .build() + .execute_with(|| { + assert_ok!(Stableswap::create_pool( + RuntimeOrigin::signed(ALICE), + pool_id, + vec![asset_a, asset_b], + 100, + Permill::from_percent(10), + Permill::from_percent(20), + )); + + System::set_block_number(5000); + + assert_noop!( + Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, 1000, 4000, 10_000), + Error::::InvalidBlock + ); + }); +} + +#[test] +fn update_amplification_should_work_when_current_change_has_not_completed() { + let asset_a: AssetId = 1; + let asset_b: AssetId = 2; + let pool_id: AssetId = 100; + + ExtBuilder::default() + .with_endowed_accounts(vec![(ALICE, asset_a, 200 * ONE), (ALICE, asset_b, 200 * ONE)]) + .with_registered_asset("pool".as_bytes().to_vec(), pool_id) + .with_registered_asset("one".as_bytes().to_vec(), asset_a) + .with_registered_asset("two".as_bytes().to_vec(), asset_b) + .build() + .execute_with(|| { + assert_ok!(Stableswap::create_pool( + RuntimeOrigin::signed(ALICE), + pool_id, + vec![asset_a, asset_b], + 100, + Permill::from_percent(10), + Permill::from_percent(20), + )); + + System::set_block_number(1); + + assert_ok!(Stableswap::update_amplification( + RuntimeOrigin::signed(ALICE), + pool_id, + 1000, + 10, + 1000, + )); + + assert_eq!( + >::get(pool_id).unwrap(), + PoolInfo { + assets: vec![asset_a, asset_b].try_into().unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), + final_amplification: NonZeroU16::new(1000).unwrap(), + initial_block: 10, + final_block: 1000, + trade_fee: Permill::from_percent(10), + withdraw_fee: Permill::from_percent(20) + } + ); + System::set_block_number(500); + assert_noop!( + Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, 5000, 5010, 6000,), + Error::::AmplificationChangeNotCompleted + ); + + System::set_block_number(5000); + assert_ok!(Stableswap::update_amplification( + RuntimeOrigin::signed(ALICE), + pool_id, + 5000, + 5010, + 6000, + )); + + assert_eq!( + >::get(pool_id).unwrap(), + PoolInfo { + assets: vec![asset_a, asset_b].try_into().unwrap(), + initial_amplification: NonZeroU16::new(1000).unwrap(), + final_amplification: NonZeroU16::new(5000).unwrap(), + initial_block: 5010, + final_block: 6000, + trade_fee: Permill::from_percent(10), + withdraw_fee: Permill::from_percent(20) + } + ); + }); +} diff --git a/pallets/stableswap/src/tests/mod.rs b/pallets/stableswap/src/tests/mod.rs index 46fe16dfc..2adb89c4d 100644 --- a/pallets/stableswap/src/tests/mod.rs +++ b/pallets/stableswap/src/tests/mod.rs @@ -1,4 +1,5 @@ mod add_liquidity; +mod amplification; mod creation; mod invariants; pub(crate) mod mock; From 717e4b5a956aad1f996e2144e5820b769abc4c5e Mon Sep 17 00:00:00 2001 From: Martin Hloska Date: Tue, 4 Jul 2023 22:40:19 +0200 Subject: [PATCH 13/21] add ampl change error test scenarios --- pallets/stableswap/src/tests/amplification.rs | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/pallets/stableswap/src/tests/amplification.rs b/pallets/stableswap/src/tests/amplification.rs index 6eeaa5866..41e46fde2 100644 --- a/pallets/stableswap/src/tests/amplification.rs +++ b/pallets/stableswap/src/tests/amplification.rs @@ -221,3 +221,75 @@ fn update_amplification_should_work_when_current_change_has_not_completed() { ); }); } + +#[test] +fn update_amplification_should_fail_when_new_value_is_same_as_previous_one() { + let asset_a: AssetId = 1; + let asset_b: AssetId = 2; + let pool_id: AssetId = 100; + + ExtBuilder::default() + .with_endowed_accounts(vec![(ALICE, asset_a, 200 * ONE), (ALICE, asset_b, 200 * ONE)]) + .with_registered_asset("pool".as_bytes().to_vec(), pool_id) + .with_registered_asset("one".as_bytes().to_vec(), asset_a) + .with_registered_asset("two".as_bytes().to_vec(), asset_b) + .build() + .execute_with(|| { + assert_ok!(Stableswap::create_pool( + RuntimeOrigin::signed(ALICE), + pool_id, + vec![asset_a, asset_b], + 100, + Permill::from_percent(10), + Permill::from_percent(20), + )); + + System::set_block_number(5000); + + assert_noop!( + Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, 100, 5000, 10_000), + Error::::SameAmplification, + ); + }); +} + +#[test] +fn update_amplification_should_fail_when_new_value_is_zero_or_outside_allowed_range() { + let asset_a: AssetId = 1; + let asset_b: AssetId = 2; + let pool_id: AssetId = 100; + + ExtBuilder::default() + .with_endowed_accounts(vec![(ALICE, asset_a, 200 * ONE), (ALICE, asset_b, 200 * ONE)]) + .with_registered_asset("pool".as_bytes().to_vec(), pool_id) + .with_registered_asset("one".as_bytes().to_vec(), asset_a) + .with_registered_asset("two".as_bytes().to_vec(), asset_b) + .build() + .execute_with(|| { + assert_ok!(Stableswap::create_pool( + RuntimeOrigin::signed(ALICE), + pool_id, + vec![asset_a, asset_b], + 100, + Permill::from_percent(10), + Permill::from_percent(20), + )); + + System::set_block_number(5000); + + assert_noop!( + Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, 0, 5000, 10_000), + Error::::InvalidAmplification, + ); + + assert_noop!( + Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, 1, 5000, 10_000), + Error::::InvalidAmplification, + ); + + assert_noop!( + Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, 20_000, 5000, 10_000), + Error::::InvalidAmplification, + ); + }); +} From 9225cbe3c3c36829743a10e8d3bb5f28bf209e78 Mon Sep 17 00:00:00 2001 From: Martin Hloska Date: Tue, 4 Jul 2023 22:49:57 +0200 Subject: [PATCH 14/21] add test of changing amplification across 1000 blocks --- pallets/stableswap/src/lib.rs | 49 ++++++------------- pallets/stableswap/src/tests/amplification.rs | 42 +++++++++++++++- 2 files changed, 56 insertions(+), 35 deletions(-) diff --git a/pallets/stableswap/src/lib.rs b/pallets/stableswap/src/lib.rs index 16a89aca1..3bc326460 100644 --- a/pallets/stableswap/src/lib.rs +++ b/pallets/stableswap/src/lib.rs @@ -86,8 +86,8 @@ pub mod pallet { use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; use sp_runtime::traits::{BlockNumberProvider, Zero}; + use sp_runtime::ArithmeticError; use sp_runtime::Permill; - use sp_runtime::{ArithmeticError, SaturatedConversion}; use sp_std::num::NonZeroU16; #[pallet::pallet] @@ -546,14 +546,7 @@ pub mod pallet { Error::::InsufficientLiquidityRemaining ); - let amplification = hydra_dx_math::stableswap::calculate_amplification( - pool.initial_amplification.get().into(), - pool.final_amplification.get().into(), - pool.initial_block.saturated_into(), - pool.final_block.saturated_into(), - T::BlockNumberProvider::current_block_number().saturated_into(), - ); - + let amplification = Self::get_amplification(&pool); let (amount, fee) = hydra_dx_math::stableswap::calculate_withdraw_one_asset::( &balances, share_amount, @@ -752,14 +745,7 @@ impl Pallet { ensure!(balances[index_in] > Balance::zero(), Error::::InsufficientLiquidity); ensure!(balances[index_out] > Balance::zero(), Error::::InsufficientLiquidity); - let amplification = hydra_dx_math::stableswap::calculate_amplification( - pool.initial_amplification.get().into(), - pool.final_amplification.get().into(), - pool.initial_block.saturated_into(), - pool.final_block.saturated_into(), - T::BlockNumberProvider::current_block_number().saturated_into(), - ); - + let amplification = Self::get_amplification(&pool); hydra_dx_math::stableswap::calculate_out_given_in_with_fee::( &balances, index_in, @@ -788,14 +774,7 @@ impl Pallet { ensure!(balances[index_out] > amount_out, Error::::InsufficientLiquidity); ensure!(balances[index_in] > Balance::zero(), Error::::InsufficientLiquidity); - let amplification = hydra_dx_math::stableswap::calculate_amplification( - pool.initial_amplification.get().into(), - pool.final_amplification.get().into(), - pool.initial_block.saturated_into(), - pool.final_block.saturated_into(), - T::BlockNumberProvider::current_block_number().saturated_into(), - ); - + let amplification = Self::get_amplification(&pool); hydra_dx_math::stableswap::calculate_in_given_out_with_fee::( &balances, index_in, @@ -894,14 +873,7 @@ impl Pallet { } } - let amplification = hydra_dx_math::stableswap::calculate_amplification( - pool.initial_amplification.get().into(), - pool.final_amplification.get().into(), - pool.initial_block.saturated_into(), - pool.final_block.saturated_into(), - T::BlockNumberProvider::current_block_number().saturated_into(), - ); - + let amplification = Self::get_amplification(&pool); let share_issuance = T::Currency::total_issuance(pool_id); let share_amount = hydra_dx_math::stableswap::calculate_shares::( &initial_reserves, @@ -935,4 +907,15 @@ impl Pallet { fn pool_account(pool_id: T::AssetId) -> T::AccountId { T::ShareAccountId::from_assets(&pool_id, Some(POOL_IDENTIFIER)) } + + #[inline] + pub(crate) fn get_amplification(pool: &PoolInfo) -> u128 { + hydra_dx_math::stableswap::calculate_amplification( + pool.initial_amplification.get().into(), + pool.final_amplification.get().into(), + pool.initial_block.saturated_into(), + pool.final_block.saturated_into(), + T::BlockNumberProvider::current_block_number().saturated_into(), + ) + } } diff --git a/pallets/stableswap/src/tests/amplification.rs b/pallets/stableswap/src/tests/amplification.rs index 41e46fde2..2f3cdc3a4 100644 --- a/pallets/stableswap/src/tests/amplification.rs +++ b/pallets/stableswap/src/tests/amplification.rs @@ -29,8 +29,6 @@ fn update_amplification_should_work_when_correct_params_are_provided() { )); System::set_block_number(2); - let b = System::current_block_number(); - dbg!(b); assert_ok!(Stableswap::update_amplification( RuntimeOrigin::signed(ALICE), @@ -293,3 +291,43 @@ fn update_amplification_should_fail_when_new_value_is_zero_or_outside_allowed_ra ); }); } + +#[test] +fn amplification_should_change_when_block_changes() { + let asset_a: AssetId = 1; + let asset_b: AssetId = 2; + let pool_id: AssetId = 100; + + ExtBuilder::default() + .with_endowed_accounts(vec![(ALICE, asset_a, 200 * ONE), (ALICE, asset_b, 200 * ONE)]) + .with_registered_asset("pool".as_bytes().to_vec(), pool_id) + .with_registered_asset("one".as_bytes().to_vec(), asset_a) + .with_registered_asset("two".as_bytes().to_vec(), asset_b) + .build() + .execute_with(|| { + assert_ok!(Stableswap::create_pool( + RuntimeOrigin::signed(ALICE), + pool_id, + vec![asset_a, asset_b], + 2000, + Permill::from_percent(10), + Permill::from_percent(20), + )); + + System::set_block_number(1); + assert_ok!(Stableswap::update_amplification( + RuntimeOrigin::signed(ALICE), + pool_id, + 5000, + 10, + 1010, + )); + System::set_block_number(9); + let pool = >::get(pool_id).unwrap(); + for idx in 0..1000 { + System::set_block_number(System::current_block_number() + 1); + let amplification = crate::Pallet::::get_amplification(&pool); + assert_eq!(amplification as u16, pool.initial_amplification.get() + idx * 3); + } + }); +} From 86113c345c52c9d3cc832653c738c6a80dd0da64 Mon Sep 17 00:00:00 2001 From: Martin Hloska Date: Tue, 4 Jul 2023 23:11:24 +0200 Subject: [PATCH 15/21] add invariants tests while amp is changing --- pallets/stableswap/src/lib.rs | 2 + pallets/stableswap/src/tests/invariants.rs | 197 +++++++++++++++++++++ 2 files changed, 199 insertions(+) diff --git a/pallets/stableswap/src/lib.rs b/pallets/stableswap/src/lib.rs index 3bc326460..187931c49 100644 --- a/pallets/stableswap/src/lib.rs +++ b/pallets/stableswap/src/lib.rs @@ -900,10 +900,12 @@ impl Pallet { Ok(share_amount) } + #[inline] fn is_asset_allowed(pool_id: T::AssetId, asset_id: T::AssetId, operation: Tradability) -> bool { AssetTradability::::get(pool_id, asset_id).contains(operation) } + #[inline] fn pool_account(pool_id: T::AssetId) -> T::AccountId { T::ShareAccountId::from_assets(&pool_id, Some(POOL_IDENTIFIER)) } diff --git a/pallets/stableswap/src/tests/invariants.rs b/pallets/stableswap/src/tests/invariants.rs index 3851f3445..a32e4411b 100644 --- a/pallets/stableswap/src/tests/invariants.rs +++ b/pallets/stableswap/src/tests/invariants.rs @@ -7,6 +7,7 @@ use std::num::NonZeroU16; use hydra_dx_math::stableswap::calculate_d; use proptest::prelude::*; use proptest::proptest; +use sp_runtime::traits::BlockNumberProvider; pub const ONE: Balance = 1_000_000_000_000; @@ -24,6 +25,14 @@ fn some_amplification() -> impl Strategy { (2..10000u16).prop_map(|v| NonZeroU16::new(v).unwrap()) } +fn initial_amplification() -> impl Strategy { + (2..1000u16).prop_map(|v| NonZeroU16::new(v).unwrap()) +} + +fn final_amplification() -> impl Strategy { + (2000..10000u16).prop_map(|v| NonZeroU16::new(v).unwrap()) +} + fn trade_fee() -> impl Strategy { (0f64..50f64).prop_map(Permill::from_float) } @@ -259,3 +268,191 @@ proptest! { }); } } + +proptest! { + #![proptest_config(ProptestConfig::with_cases(100))] + #[test] + fn sell_invariants_should_hold_when_amplification_is_changing( + initial_liquidity in asset_reserve(), + amount in trade_amount(), + initial_amplification in initial_amplification(), + final_amplification in final_amplification(), + ) { + let asset_a: AssetId = 1000; + let asset_b: AssetId = 2000; + + ExtBuilder::default() + .with_endowed_accounts(vec![ + (BOB, asset_a, amount), + (ALICE, asset_a, initial_liquidity), + (ALICE, asset_b, initial_liquidity), + ]) + .with_registered_asset("one".as_bytes().to_vec(), asset_a) + .with_registered_asset("two".as_bytes().to_vec(), asset_b) + .with_pool( + ALICE, + PoolInfo:: { + assets: vec![asset_a,asset_b].try_into().unwrap(), + initial_amplification, + final_amplification: initial_amplification, + initial_block: 0, + final_block: 0, + trade_fee: Permill::from_percent(0), + withdraw_fee: Permill::from_percent(0), + }, + InitialLiquidity{ account: ALICE, assets: + vec![ + AssetLiquidity{ + asset_id: asset_a, + amount: initial_liquidity + }, + AssetLiquidity{ + asset_id: asset_b, + amount: initial_liquidity + } + ]}, + ) + .build() + .execute_with(|| { + System::set_block_number(0); + let pool_id = get_pool_id_at(0); + let pool_account = pool_account(pool_id); + + System::set_block_number(1); + assert_ok!( + Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, final_amplification.get(), 10,100) + ); + + System::set_block_number(9); + let pool = >::get(pool_id).unwrap(); + + let asset_a_balance = Tokens::free_balance(asset_a, &pool_account); + let asset_b_balance = Tokens::free_balance(asset_b, &pool_account); + let bob_a_balance = Tokens::free_balance(asset_a, &BOB); + + for _ in 0..100{ + System::set_block_number(System::current_block_number() + 1); + let amplification = crate::Pallet::::get_amplification(&pool); + + // just restore the balances + Tokens::set_balance(RuntimeOrigin::root(), pool_account, asset_a, asset_a_balance, 0).unwrap(); + Tokens::set_balance(RuntimeOrigin::root(), pool_account, asset_b, asset_b_balance, 0).unwrap(); + Tokens::set_balance(RuntimeOrigin::root(), BOB, asset_a, bob_a_balance, 0).unwrap(); + + let asset_a_reserve = Tokens::free_balance(asset_a, &pool_account); + let asset_b_reserve = Tokens::free_balance(asset_b, &pool_account); + let d_prev = calculate_d::<128u8>(&[asset_a_reserve,asset_b_reserve], amplification).unwrap(); + + assert_ok!(Stableswap::sell( + RuntimeOrigin::signed(BOB), + pool_id, + asset_a, + asset_b, + amount, + 0u128, // not interested in this + )); + + let asset_a_reserve = Tokens::free_balance(asset_a, &pool_account); + let asset_b_reserve = Tokens::free_balance(asset_b, &pool_account); + let d = calculate_d::<128u8>(&[asset_a_reserve,asset_b_reserve], amplification).unwrap(); + + assert!(d >= d_prev); + assert!(d - d_prev <= 10u128); + } + }); + } +} + +proptest! { + #![proptest_config(ProptestConfig::with_cases(100))] + #[test] + fn buy_invariants_should_hold_when_amplification_is_changing( + initial_liquidity in asset_reserve(), + amount in trade_amount(), + initial_amplification in initial_amplification(), + final_amplification in final_amplification(), + ) { + let asset_a: AssetId = 1000; + let asset_b: AssetId = 2000; + + ExtBuilder::default() + .with_endowed_accounts(vec![ + (BOB, asset_a, amount * 1000), + (ALICE, asset_a, initial_liquidity), + (ALICE, asset_b, initial_liquidity), + ]) + .with_registered_asset("one".as_bytes().to_vec(), asset_a) + .with_registered_asset("two".as_bytes().to_vec(), asset_b) + .with_pool( + ALICE, + PoolInfo:: { + assets: vec![asset_a,asset_b].try_into().unwrap(), + initial_amplification, + final_amplification: initial_amplification, + initial_block: 0, + final_block: 0, + trade_fee: Permill::from_percent(0), + withdraw_fee: Permill::from_percent(0), + }, + InitialLiquidity{ account: ALICE, assets: + vec![ + AssetLiquidity{ + asset_id: asset_a, + amount: initial_liquidity + }, + AssetLiquidity{ + asset_id: asset_b, + amount: initial_liquidity + } + ]}, + ) + .build() + .execute_with(|| { + System::set_block_number(0); + let pool_id = get_pool_id_at(0); + let pool_account = pool_account(pool_id); + + System::set_block_number(1); + assert_ok!( + Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, final_amplification.get(), 10,100) + ); + + System::set_block_number(9); + let pool = >::get(pool_id).unwrap(); + + let asset_a_balance = Tokens::free_balance(asset_a, &pool_account); + let asset_b_balance = Tokens::free_balance(asset_b, &pool_account); + let bob_a_balance = Tokens::free_balance(asset_a, &BOB); + + for _ in 0..100{ + System::set_block_number(System::current_block_number() + 1); + let amplification = crate::Pallet::::get_amplification(&pool); + + // just restore the balances + Tokens::set_balance(RuntimeOrigin::root(), pool_account, asset_a, asset_a_balance, 0).unwrap(); + Tokens::set_balance(RuntimeOrigin::root(), pool_account, asset_b, asset_b_balance, 0).unwrap(); + Tokens::set_balance(RuntimeOrigin::root(), BOB, asset_a, bob_a_balance, 0).unwrap(); + + let asset_a_reserve = Tokens::free_balance(asset_a, &pool_account); + let asset_b_reserve = Tokens::free_balance(asset_b, &pool_account); + let d_prev = calculate_d::<128u8>(&[asset_a_reserve,asset_b_reserve], amplification).unwrap(); + + assert_ok!(Stableswap::buy( + RuntimeOrigin::signed(BOB), + pool_id, + asset_b, + asset_a, + amount, + u128::MAX, // not interested in this + )); + + let asset_a_reserve = Tokens::free_balance(asset_a, &pool_account); + let asset_b_reserve = Tokens::free_balance(asset_b, &pool_account); + let d = calculate_d::<128u8>(&[asset_a_reserve,asset_b_reserve], amplification).unwrap(); + + assert!(d >= d_prev); + assert!(d - d_prev <= 10u128); + } + }); + } +} From 0958cf6c8a82fc66ed4b3b98473a31247252817a Mon Sep 17 00:00:00 2001 From: Martin Hloska Date: Tue, 4 Jul 2023 23:18:01 +0200 Subject: [PATCH 16/21] add ampl update event to create pool --- pallets/stableswap/src/lib.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pallets/stableswap/src/lib.rs b/pallets/stableswap/src/lib.rs index 187931c49..965cd37c0 100644 --- a/pallets/stableswap/src/lib.rs +++ b/pallets/stableswap/src/lib.rs @@ -347,6 +347,13 @@ pub mod pallet { withdraw_fee, }); + Self::deposit_event(Event::AmplificationChanging { + pool_id, + current_amplification: amplification, + final_amplification: amplification, + start_block: T::BlockNumberProvider::current_block_number(), + end_block: T::BlockNumberProvider::current_block_number(), + }); Ok(()) } From a4581c133c4a5bc5e406f32846184c963e60255b Mon Sep 17 00:00:00 2001 From: Martin Date: Wed, 5 Jul 2023 09:19:45 +0200 Subject: [PATCH 17/21] allow change of ongoiing amplification change --- pallets/stableswap/src/lib.rs | 18 ++++++------- pallets/stableswap/src/tests/amplification.rs | 25 ++++++++----------- 2 files changed, 17 insertions(+), 26 deletions(-) diff --git a/pallets/stableswap/src/lib.rs b/pallets/stableswap/src/lib.rs index 965cd37c0..edd4510da 100644 --- a/pallets/stableswap/src/lib.rs +++ b/pallets/stableswap/src/lib.rs @@ -296,10 +296,7 @@ pub mod pallet { NotAllowed, /// Future block number is in the past. - InvalidBlock, - - /// Current amplification change has not completed yet. - AmplificationChangeNotCompleted, + PastBlock, /// New amplification is equal to the previous value. SameAmplification, @@ -424,22 +421,21 @@ pub mod pallet { let current_block = T::BlockNumberProvider::current_block_number(); ensure!( end_block > start_block && end_block > current_block && start_block >= current_block, - Error::::InvalidBlock + Error::::PastBlock ); Pools::::try_mutate(pool_id, |maybe_pool| -> DispatchResult { let mut pool = maybe_pool.as_mut().ok_or(Error::::PoolNotFound)?; + let current_amplification = Self::get_amplification(&pool); + ensure!( - pool.final_block <= current_block, - Error::::AmplificationChangeNotCompleted - ); - ensure!( - pool.final_amplification.get() != final_amplification, + current_amplification != final_amplification as u128, Error::::SameAmplification ); - pool.initial_amplification = pool.final_amplification; + pool.initial_amplification = + NonZeroU16::new(current_amplification.saturated_into()).ok_or(Error::::InvalidAmplification)?; pool.final_amplification = NonZeroU16::new(final_amplification).ok_or(Error::::InvalidAmplification)?; pool.initial_block = start_block; diff --git a/pallets/stableswap/src/tests/amplification.rs b/pallets/stableswap/src/tests/amplification.rs index 2f3cdc3a4..e27531bc8 100644 --- a/pallets/stableswap/src/tests/amplification.rs +++ b/pallets/stableswap/src/tests/amplification.rs @@ -79,7 +79,7 @@ fn update_amplification_should_fail_when_end_block_is_before_current_block() { assert_noop!( Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, 1000, 10, 1000), - Error::::InvalidBlock + Error::::PastBlock ); }); } @@ -110,7 +110,7 @@ fn update_amplification_should_fail_when_end_block_is_smaller_than_start_block() assert_noop!( Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, 1000, 20_000, 10_000), - Error::::InvalidBlock + Error::::PastBlock ); }); } @@ -141,13 +141,13 @@ fn update_amplification_should_fail_when_start_block_before_current_block() { assert_noop!( Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, 1000, 4000, 10_000), - Error::::InvalidBlock + Error::::PastBlock ); }); } #[test] -fn update_amplification_should_work_when_current_change_has_not_completed() { +fn update_amplification_should_work_when_current_change_is_in_progress() { let asset_a: AssetId = 1; let asset_b: AssetId = 2; let pool_id: AssetId = 100; @@ -191,28 +191,23 @@ fn update_amplification_should_work_when_current_change_has_not_completed() { } ); System::set_block_number(500); - assert_noop!( - Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, 5000, 5010, 6000,), - Error::::AmplificationChangeNotCompleted - ); - System::set_block_number(5000); assert_ok!(Stableswap::update_amplification( RuntimeOrigin::signed(ALICE), pool_id, 5000, - 5010, - 6000, - )); + 501, + 1000 + ),); assert_eq!( >::get(pool_id).unwrap(), PoolInfo { assets: vec![asset_a, asset_b].try_into().unwrap(), - initial_amplification: NonZeroU16::new(1000).unwrap(), + initial_amplification: NonZeroU16::new(545).unwrap(), final_amplification: NonZeroU16::new(5000).unwrap(), - initial_block: 5010, - final_block: 6000, + initial_block: 501, + final_block: 1000, trade_fee: Permill::from_percent(10), withdraw_fee: Permill::from_percent(20) } From a2c2fbd44590938e90503d0c900f64db2e4c2836 Mon Sep 17 00:00:00 2001 From: Martin Date: Wed, 5 Jul 2023 09:42:38 +0200 Subject: [PATCH 18/21] update branch to reflect recent change in amplifcation change --- pallets/stableswap/src/benchmarks.rs | 88 +++++++++++++++++-- pallets/stableswap/src/tests/add_liquidity.rs | 4 +- pallets/stableswap/src/tests/amplification.rs | 38 ++++---- pallets/stableswap/src/tests/creation.rs | 26 +++--- pallets/stableswap/src/tests/invariants.rs | 8 +- pallets/stableswap/src/tests/mock.rs | 8 +- pallets/stableswap/src/tests/update_pool.rs | 27 +++--- 7 files changed, 132 insertions(+), 67 deletions(-) diff --git a/pallets/stableswap/src/benchmarks.rs b/pallets/stableswap/src/benchmarks.rs index 24dce790f..446dd8744 100644 --- a/pallets/stableswap/src/benchmarks.rs +++ b/pallets/stableswap/src/benchmarks.rs @@ -23,7 +23,7 @@ use frame_benchmarking::account; use frame_benchmarking::benchmarks; use frame_support::pallet_prelude::DispatchError; use frame_support::traits::EnsureOrigin; -use frame_system::RawOrigin; +use frame_system::{Pallet as System, RawOrigin}; use orml_traits::MultiCurrency; use orml_traits::MultiCurrencyExtended; use sp_runtime::Permill; @@ -222,7 +222,6 @@ benchmarks! { withdraw_fee, )?; - // Worst case is adding additional liquidity and not initial liquidity crate::Pallet::::add_liquidity(RawOrigin::Signed(caller).into(), pool_id, initial, @@ -235,6 +234,16 @@ benchmarks! { let buy_min_amount = 1_000u128; + // Worst case is when amplification is changing + crate::Pallet::::update_amplification(RawOrigin::Root.into(), + pool_id, + 1000, + 100u32.into(), + 1000u32.into(), + )?; + + System::::set_block_number(500u32.into()); + }: _(RawOrigin::Signed(seller.clone()), pool_id, asset_in, asset_out, amount_sell, buy_min_amount) verify { assert!(T::Currency::free_balance(asset_in, &seller) == 0u128); @@ -284,7 +293,6 @@ benchmarks! { withdraw_fee, )?; - // Worst case is adding additional liquidity and not initial liquidity crate::Pallet::::add_liquidity(RawOrigin::Signed(caller).into(), pool_id, initial, @@ -297,6 +305,16 @@ benchmarks! { let amount_buy = 10_000_000_000_000u128; let sell_max_limit = 20_000_000_000_000_000u128; + // Worst case is when amplification is changing + crate::Pallet::::update_amplification(RawOrigin::Root.into(), + pool_id, + 1000, + 100u32.into(), + 1000u32.into(), + )?; + + System::::set_block_number(500u32.into()); + }: _(RawOrigin::Signed(buyer.clone()), pool_id, asset_out, asset_in, amount_buy, sell_max_limit) verify { assert!(T::Currency::free_balance(asset_out, &buyer) > 0u128); @@ -350,8 +368,7 @@ benchmarks! { assert_ne!(asset_tradability_old, asset_tradability_new); } - /* - update_pool { + update_pool_fees{ let caller: T::AccountId = account("caller", 0, 1); let lp_provider: T::AccountId = account("provider", 0, 1); let initial_liquidity = 1_000_000_000_000_000u128; @@ -387,17 +404,70 @@ benchmarks! { Permill::from_percent(1), )?; - let amplification_new = Some(200_u16); let trade_fee_new = Some(Permill::from_percent(50)); let withdraw_fee_new = Some(Permill::from_percent(40)); - }: _(successful_origin, pool_id, amplification_new, trade_fee_new, withdraw_fee_new) + }: _(successful_origin, pool_id, trade_fee_new, withdraw_fee_new) verify { let pool = crate::Pallet::::pools(pool_id).unwrap(); - assert_eq!(pool.initial_amplification, amplification_new.unwrap()); assert_eq!(pool.trade_fee, trade_fee_new.unwrap()); assert_eq!(pool.withdraw_fee, withdraw_fee_new.unwrap()); } - */ + + update_amplification{ + let caller: T::AccountId = account("caller", 0, 1); + let lp_provider: T::AccountId = account("provider", 0, 1); + let initial_liquidity = 1_000_000_000_000_000u128; + let liquidity_added = 300_000_000_000_000u128; + + let mut initial: Vec> = vec![]; + let mut added_liquidity: Vec> = vec![]; + + let mut asset_ids: Vec = Vec::new() ; + for idx in 0..MAX_ASSETS_IN_POOL { + let name: Vec = idx.to_ne_bytes().to_vec(); + let asset_id = T::AssetRegistry::create_asset(&name, 1u128)?; + asset_ids.push(asset_id); + T::Currency::update_balance(asset_id, &caller, 1_000_000_000_000_000i128)?; + T::Currency::update_balance(asset_id, &lp_provider, 1_000_000_000_000_000_000_000i128)?; + initial.push(AssetLiquidity{ + asset_id, + amount: initial_liquidity + }); + added_liquidity.push(AssetLiquidity{ + asset_id, + amount: liquidity_added + }); + } + let pool_id = T::AssetRegistry::create_asset(&b"pool".to_vec(), 1u128)?; + + let successful_origin = T::AuthorityOrigin::try_successful_origin().unwrap(); + crate::Pallet::::create_pool(successful_origin.clone(), + pool_id, + asset_ids, + 100u16, + Permill::from_percent(1), + Permill::from_percent(1), + )?; + + // Worst case is when amplification is changing + crate::Pallet::::update_amplification(RawOrigin::Root.into(), + pool_id, + 1000, + 100u32.into(), + 1000u32.into(), + )?; + + System::::set_block_number(500u32.into()); + + }: _(successful_origin, pool_id, 5000, 501u32.into(), 1000u32.into()) + verify { + let pool = crate::Pallet::::pools(pool_id).unwrap(); + + assert_eq!(pool.initial_amplification, NonZeroU16::new(500).unwrap()); + assert_eq!(pool.final_amplification, NonZeroU16::new(5000).unwrap()); + assert_eq!(pool.initial_block, 501u32.into()); + assert_eq!(pool.final_block, 1000u32.into()); + } impl_benchmark_test_suite!(Pallet, crate::tests::mock::ExtBuilder::default().build(), crate::tests::mock::Test); } diff --git a/pallets/stableswap/src/tests/add_liquidity.rs b/pallets/stableswap/src/tests/add_liquidity.rs index ef40da573..f8f03b00e 100644 --- a/pallets/stableswap/src/tests/add_liquidity.rs +++ b/pallets/stableswap/src/tests/add_liquidity.rs @@ -25,7 +25,7 @@ fn add_initial_liquidity_should_work_when_called_first_time() { let amplification: u16 = 100; assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b], amplification, @@ -80,7 +80,7 @@ fn add_initial_liquidity_should_fail_when_lp_has_insufficient_balance() { let amplification: u16 = 100; assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b], amplification, diff --git a/pallets/stableswap/src/tests/amplification.rs b/pallets/stableswap/src/tests/amplification.rs index e27531bc8..f1ac99478 100644 --- a/pallets/stableswap/src/tests/amplification.rs +++ b/pallets/stableswap/src/tests/amplification.rs @@ -20,7 +20,7 @@ fn update_amplification_should_work_when_correct_params_are_provided() { .build() .execute_with(|| { assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b], 100, @@ -31,7 +31,7 @@ fn update_amplification_should_work_when_correct_params_are_provided() { System::set_block_number(2); assert_ok!(Stableswap::update_amplification( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, 1000, 10, @@ -67,7 +67,7 @@ fn update_amplification_should_fail_when_end_block_is_before_current_block() { .build() .execute_with(|| { assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b], 100, @@ -78,7 +78,7 @@ fn update_amplification_should_fail_when_end_block_is_before_current_block() { System::set_block_number(5000); assert_noop!( - Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, 1000, 10, 1000), + Stableswap::update_amplification(RuntimeOrigin::root(), pool_id, 1000, 10, 1000), Error::::PastBlock ); }); @@ -98,7 +98,7 @@ fn update_amplification_should_fail_when_end_block_is_smaller_than_start_block() .build() .execute_with(|| { assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b], 100, @@ -109,7 +109,7 @@ fn update_amplification_should_fail_when_end_block_is_smaller_than_start_block() System::set_block_number(5000); assert_noop!( - Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, 1000, 20_000, 10_000), + Stableswap::update_amplification(RuntimeOrigin::root(), pool_id, 1000, 20_000, 10_000), Error::::PastBlock ); }); @@ -129,7 +129,7 @@ fn update_amplification_should_fail_when_start_block_before_current_block() { .build() .execute_with(|| { assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b], 100, @@ -140,7 +140,7 @@ fn update_amplification_should_fail_when_start_block_before_current_block() { System::set_block_number(5000); assert_noop!( - Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, 1000, 4000, 10_000), + Stableswap::update_amplification(RuntimeOrigin::root(), pool_id, 1000, 4000, 10_000), Error::::PastBlock ); }); @@ -160,7 +160,7 @@ fn update_amplification_should_work_when_current_change_is_in_progress() { .build() .execute_with(|| { assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b], 100, @@ -171,7 +171,7 @@ fn update_amplification_should_work_when_current_change_is_in_progress() { System::set_block_number(1); assert_ok!(Stableswap::update_amplification( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, 1000, 10, @@ -193,7 +193,7 @@ fn update_amplification_should_work_when_current_change_is_in_progress() { System::set_block_number(500); assert_ok!(Stableswap::update_amplification( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, 5000, 501, @@ -229,7 +229,7 @@ fn update_amplification_should_fail_when_new_value_is_same_as_previous_one() { .build() .execute_with(|| { assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b], 100, @@ -240,7 +240,7 @@ fn update_amplification_should_fail_when_new_value_is_same_as_previous_one() { System::set_block_number(5000); assert_noop!( - Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, 100, 5000, 10_000), + Stableswap::update_amplification(RuntimeOrigin::root(), pool_id, 100, 5000, 10_000), Error::::SameAmplification, ); }); @@ -260,7 +260,7 @@ fn update_amplification_should_fail_when_new_value_is_zero_or_outside_allowed_ra .build() .execute_with(|| { assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b], 100, @@ -271,17 +271,17 @@ fn update_amplification_should_fail_when_new_value_is_zero_or_outside_allowed_ra System::set_block_number(5000); assert_noop!( - Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, 0, 5000, 10_000), + Stableswap::update_amplification(RuntimeOrigin::root(), pool_id, 0, 5000, 10_000), Error::::InvalidAmplification, ); assert_noop!( - Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, 1, 5000, 10_000), + Stableswap::update_amplification(RuntimeOrigin::root(), pool_id, 1, 5000, 10_000), Error::::InvalidAmplification, ); assert_noop!( - Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, 20_000, 5000, 10_000), + Stableswap::update_amplification(RuntimeOrigin::root(), pool_id, 20_000, 5000, 10_000), Error::::InvalidAmplification, ); }); @@ -301,7 +301,7 @@ fn amplification_should_change_when_block_changes() { .build() .execute_with(|| { assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b], 2000, @@ -311,7 +311,7 @@ fn amplification_should_change_when_block_changes() { System::set_block_number(1); assert_ok!(Stableswap::update_amplification( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, 5000, 10, diff --git a/pallets/stableswap/src/tests/creation.rs b/pallets/stableswap/src/tests/creation.rs index 5d8f9b688..5733b73af 100644 --- a/pallets/stableswap/src/tests/creation.rs +++ b/pallets/stableswap/src/tests/creation.rs @@ -20,7 +20,7 @@ fn create_two_asset_pool_should_work_when_assets_are_registered() { .build() .execute_with(|| { assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b], 100, @@ -61,7 +61,7 @@ fn create_multi_asset_pool_should_work_when_assets_are_registered() { .build() .execute_with(|| { assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b, asset_c, asset_d], 100, @@ -92,7 +92,7 @@ fn create_pool_should_store_assets_correctly_when_input_is_not_sorted() { let amplification = 100u16; assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_c, asset_d, asset_b, asset_a], amplification, @@ -127,7 +127,7 @@ fn create_pool_should_fail_when_same_assets_is_specified() { assert_noop!( Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, 3, 4, asset_a], amplification, @@ -148,7 +148,7 @@ fn create_pool_should_fail_when_share_asset_is_not_registered() { assert_noop!( Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, 3, 4], amplification, @@ -172,7 +172,7 @@ fn create_pool_should_fail_when_share_asset_is_among_assets() { assert_noop!( Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, pool_id], amplification, @@ -198,7 +198,7 @@ fn create_pool_should_fail_when_asset_is_not_registered() { assert_noop!( Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![not_registered, registered], amplification, @@ -210,7 +210,7 @@ fn create_pool_should_fail_when_asset_is_not_registered() { assert_noop!( Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![registered, not_registered], amplification, @@ -237,7 +237,7 @@ fn create_pool_should_when_same_pool_already_exists() { let amplification = 100u16; assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b], amplification, @@ -247,7 +247,7 @@ fn create_pool_should_when_same_pool_already_exists() { assert_noop!( Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b], amplification, @@ -276,7 +276,7 @@ fn create_pool_should_fail_when_amplification_is_incorrect() { assert_noop!( Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b], 0, @@ -288,7 +288,7 @@ fn create_pool_should_fail_when_amplification_is_incorrect() { assert_noop!( Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b], amplification_min, @@ -300,7 +300,7 @@ fn create_pool_should_fail_when_amplification_is_incorrect() { assert_noop!( Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b], amplification_max, diff --git a/pallets/stableswap/src/tests/invariants.rs b/pallets/stableswap/src/tests/invariants.rs index a32e4411b..920ea3ae3 100644 --- a/pallets/stableswap/src/tests/invariants.rs +++ b/pallets/stableswap/src/tests/invariants.rs @@ -270,7 +270,7 @@ proptest! { } proptest! { - #![proptest_config(ProptestConfig::with_cases(100))] + #![proptest_config(ProptestConfig::with_cases(50))] #[test] fn sell_invariants_should_hold_when_amplification_is_changing( initial_liquidity in asset_reserve(), @@ -320,7 +320,7 @@ proptest! { System::set_block_number(1); assert_ok!( - Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, final_amplification.get(), 10,100) + Stableswap::update_amplification(RuntimeOrigin::root(), pool_id, final_amplification.get(), 10,100) ); System::set_block_number(9); @@ -364,7 +364,7 @@ proptest! { } proptest! { - #![proptest_config(ProptestConfig::with_cases(100))] + #![proptest_config(ProptestConfig::with_cases(50))] #[test] fn buy_invariants_should_hold_when_amplification_is_changing( initial_liquidity in asset_reserve(), @@ -414,7 +414,7 @@ proptest! { System::set_block_number(1); assert_ok!( - Stableswap::update_amplification(RuntimeOrigin::signed(ALICE), pool_id, final_amplification.get(), 10,100) + Stableswap::update_amplification(RuntimeOrigin::root(), pool_id, final_amplification.get(), 10,100) ); System::set_block_number(9); diff --git a/pallets/stableswap/src/tests/mock.rs b/pallets/stableswap/src/tests/mock.rs index 716e1a6b2..9afa48eee 100644 --- a/pallets/stableswap/src/tests/mock.rs +++ b/pallets/stableswap/src/tests/mock.rs @@ -34,7 +34,7 @@ use frame_support::{ construct_runtime, parameter_types, traits::{ConstU32, ConstU64}, }; -use frame_system::EnsureSigned; +use frame_system::EnsureRoot; use orml_traits::parameter_type_with_key; pub use orml_traits::MultiCurrency; use sp_core::H256; @@ -145,7 +145,7 @@ impl Config for Test { type Currency = Tokens; type ShareAccountId = AccountIdConstructor; type AssetRegistry = DummyRegistry; - type AuthorityOrigin = EnsureSigned; + type AuthorityOrigin = EnsureRoot; type MinPoolLiquidity = MinimumLiquidity; type AmplificationRange = AmplificationRange; type MinTradingLimit = MinimumTradingLimit; @@ -235,7 +235,7 @@ impl ExtBuilder { let mut r: sp_io::TestExternalities = t.into(); r.execute_with(|| { - for (who, pool, initial_liquid) in self.created_pools { + for (_who, pool, initial_liquid) in self.created_pools { let pool_id = retrieve_current_asset_id(); REGISTERED_ASSETS.with(|v| { v.borrow_mut().insert(pool_id, pool_id); @@ -245,7 +245,7 @@ impl ExtBuilder { }); assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(who), + RuntimeOrigin::root(), pool_id, pool.assets.clone().into(), pool.initial_amplification.get(), diff --git a/pallets/stableswap/src/tests/update_pool.rs b/pallets/stableswap/src/tests/update_pool.rs index 92df573e7..733a08657 100644 --- a/pallets/stableswap/src/tests/update_pool.rs +++ b/pallets/stableswap/src/tests/update_pool.rs @@ -19,7 +19,7 @@ fn update_pool_should_work_when_all_parames_are_updated() { .build() .execute_with(|| { assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b], 100, @@ -28,7 +28,7 @@ fn update_pool_should_work_when_all_parames_are_updated() { )); assert_ok!(Stableswap::update_pool_fees( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, Some(Permill::from_percent(10)), Some(Permill::from_percent(20)), @@ -63,7 +63,7 @@ fn update_pool_should_work_when_only_trade_fee_is_updated() { .build() .execute_with(|| { assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b], 100, @@ -72,7 +72,7 @@ fn update_pool_should_work_when_only_trade_fee_is_updated() { )); assert_ok!(Stableswap::update_pool_fees( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, Some(Permill::from_percent(20)), None, @@ -107,7 +107,7 @@ fn update_pool_should_work_when_only_withdraw_fee_is_updated() { .build() .execute_with(|| { assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b], 100, @@ -116,7 +116,7 @@ fn update_pool_should_work_when_only_withdraw_fee_is_updated() { )); assert_ok!(Stableswap::update_pool_fees( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, None, Some(Permill::from_percent(21)), @@ -151,7 +151,7 @@ fn update_pool_should_work_when_only_fees_is_updated() { .build() .execute_with(|| { assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b], 100, @@ -160,7 +160,7 @@ fn update_pool_should_work_when_only_fees_is_updated() { )); assert_ok!(Stableswap::update_pool_fees( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, Some(Permill::from_percent(11)), Some(Permill::from_percent(21)), @@ -195,7 +195,7 @@ fn update_pool_should_fail_when_nothing_is_to_update() { .build() .execute_with(|| { assert_ok!(Stableswap::create_pool( - RuntimeOrigin::signed(ALICE), + RuntimeOrigin::root(), pool_id, vec![asset_a, asset_b], 100, @@ -204,7 +204,7 @@ fn update_pool_should_fail_when_nothing_is_to_update() { )); assert_noop!( - Stableswap::update_pool_fees(RuntimeOrigin::signed(ALICE), pool_id, None, None), + Stableswap::update_pool_fees(RuntimeOrigin::root(), pool_id, None, None), Error::::NothingToUpdate ); @@ -237,12 +237,7 @@ fn update_pool_should_fail_when_pool_does_not_exists() { let pool_id = retrieve_current_asset_id(); assert_noop!( - Stableswap::update_pool_fees( - RuntimeOrigin::signed(ALICE), - pool_id, - Some(Permill::from_percent(1)), - None - ), + Stableswap::update_pool_fees(RuntimeOrigin::root(), pool_id, Some(Permill::from_percent(1)), None), Error::::PoolNotFound ); }); From 3c038a03c4a26ba9bd9bca6a48b9b1d460b7d985 Mon Sep 17 00:00:00 2001 From: Martin Date: Wed, 5 Jul 2023 09:43:21 +0200 Subject: [PATCH 19/21] happy clippy happy life --- pallets/stableswap/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/stableswap/src/lib.rs b/pallets/stableswap/src/lib.rs index edd4510da..a6cd89c82 100644 --- a/pallets/stableswap/src/lib.rs +++ b/pallets/stableswap/src/lib.rs @@ -427,7 +427,7 @@ pub mod pallet { Pools::::try_mutate(pool_id, |maybe_pool| -> DispatchResult { let mut pool = maybe_pool.as_mut().ok_or(Error::::PoolNotFound)?; - let current_amplification = Self::get_amplification(&pool); + let current_amplification = Self::get_amplification(pool); ensure!( current_amplification != final_amplification as u128, From 2ed11b87a83337db503d2acacbf2df4c6c9ac17f Mon Sep 17 00:00:00 2001 From: Martin Hloska Date: Wed, 5 Jul 2023 19:39:33 +0200 Subject: [PATCH 20/21] update weights calls --- pallets/stableswap/src/lib.rs | 4 ++-- pallets/stableswap/src/weights.rs | 23 ++++++++++++++--------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/pallets/stableswap/src/lib.rs b/pallets/stableswap/src/lib.rs index a6cd89c82..c9ce537f1 100644 --- a/pallets/stableswap/src/lib.rs +++ b/pallets/stableswap/src/lib.rs @@ -368,7 +368,7 @@ pub mod pallet { /// /// Emits `FeesUpdated` event if successful. #[pallet::call_index(1)] - #[pallet::weight(::WeightInfo::update_pool())] + #[pallet::weight(::WeightInfo::update_pool_fees())] #[transactional] pub fn update_pool_fees( origin: OriginFor, @@ -407,7 +407,7 @@ pub mod pallet { /// /// Emits `AmplificationUpdated` event if successful. #[pallet::call_index(2)] - #[pallet::weight(::WeightInfo::update_pool())] + #[pallet::weight(::WeightInfo::update_amplification())] #[transactional] pub fn update_amplification( origin: OriginFor, diff --git a/pallets/stableswap/src/weights.rs b/pallets/stableswap/src/weights.rs index 751b870f8..37e3026c9 100644 --- a/pallets/stableswap/src/weights.rs +++ b/pallets/stableswap/src/weights.rs @@ -46,12 +46,13 @@ use sp_std::marker::PhantomData; /// Weight functions needed for pallet_stableswap. pub trait WeightInfo { fn create_pool() -> Weight; - fn update_pool() -> Weight; fn add_liquidity() -> Weight; fn remove_liquidity_one_asset() -> Weight; fn sell() -> Weight; fn buy() -> Weight; fn set_asset_tradable_state() -> Weight; + fn update_pool_fees() -> Weight; + fn update_amplification() -> Weight; } pub struct BasiliskWeight(PhantomData); @@ -63,10 +64,6 @@ impl WeightInfo for BasiliskWeight { .saturating_add(T::DbWeight::get().writes(12 as u64)) } - fn update_pool() -> Weight { - Weight::from_ref_time(0u128 as u64) - } - fn add_liquidity() -> Weight { Weight::from_ref_time(64_481_000 as u64) .saturating_add(T::DbWeight::get().reads(10 as u64)) @@ -87,10 +84,15 @@ impl WeightInfo for BasiliskWeight { .saturating_add(T::DbWeight::get().reads(9 as u64)) .saturating_add(T::DbWeight::get().writes(5 as u64)) } - fn set_asset_tradable_state() -> Weight { Weight::from_ref_time(0) } + fn update_pool_fees() -> Weight { + Weight::from_ref_time(0) + } + fn update_amplification() -> Weight { + Weight::from_ref_time(0) + } } // For backwards compatibility and tests @@ -100,9 +102,6 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(13 as u64)) .saturating_add(RocksDbWeight::get().writes(12 as u64)) } - fn update_pool() -> Weight { - Weight::from_ref_time(0u128 as u64) - } fn add_liquidity() -> Weight { Weight::from_ref_time(64_481_000 as u64) .saturating_add(RocksDbWeight::get().reads(10 as u64)) @@ -127,4 +126,10 @@ impl WeightInfo for () { fn set_asset_tradable_state() -> Weight { Weight::from_ref_time(0) } + fn update_pool_fees() -> Weight { + Weight::from_ref_time(0) + } + fn update_amplification() -> Weight { + Weight::from_ref_time(0) + } } From e574d29092afc956ad301dbfb2370e27b47ad376 Mon Sep 17 00:00:00 2001 From: Martin Date: Thu, 6 Jul 2023 08:36:12 +0200 Subject: [PATCH 21/21] simplify condition --- pallets/stableswap/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/stableswap/src/lib.rs b/pallets/stableswap/src/lib.rs index c9ce537f1..4ca76bd52 100644 --- a/pallets/stableswap/src/lib.rs +++ b/pallets/stableswap/src/lib.rs @@ -420,7 +420,7 @@ pub mod pallet { let current_block = T::BlockNumberProvider::current_block_number(); ensure!( - end_block > start_block && end_block > current_block && start_block >= current_block, + end_block > start_block && start_block >= current_block, Error::::PastBlock );