From 3872184e0fbd2d80c47b1f4b841a0a121f356873 Mon Sep 17 00:00:00 2001 From: Giles Cope Date: Sat, 10 Dec 2022 12:44:50 +0000 Subject: [PATCH 1/4] multiasset enum --- bin/node/runtime/src/lib.rs | 2 +- frame/dex/src/lib.rs | 140 +++++++++++++++++++------- frame/dex/src/tests.rs | 149 +++++++++++++++++++--------- frame/dex/src/types.rs | 26 ++++- frame/support/src/traits.rs | 4 +- frame/support/src/traits/storage.rs | 4 +- 6 files changed, 232 insertions(+), 93 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 4b0a733ca6ec7..8aa59e12d5460 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -27,9 +27,9 @@ use frame_election_provider_support::{ onchain, BalancingConfig, ElectionDataProvider, SequentialPhragmen, VoteWeight, }; use frame_support::{ - instances::{Instance1, Instance2}, construct_runtime, dispatch::DispatchClass, + instances::{Instance1, Instance2}, pallet_prelude::Get, parameter_types, traits::{ diff --git a/frame/dex/src/lib.rs b/frame/dex/src/lib.rs index e9af5f5f29040..52fa3a758c948 100644 --- a/frame/dex/src/lib.rs +++ b/frame/dex/src/lib.rs @@ -110,25 +110,22 @@ pub mod pallet { type PalletId: Get; } - pub type BalanceOf = - <::Currency as InspectFungible<::AccountId>>::Balance; + pub type BalanceOf = <::Currency as InspectFungible< + ::AccountId, + >>::Balance; pub type AssetBalanceOf = <::Assets as Inspect<::AccountId>>::Balance; - pub type PoolIdOf = (::AssetId, ::AssetId); + pub type PoolIdOf = + (MultiAssetId<::AssetId>, MultiAssetId<::AssetId>); #[pallet::storage] pub type Pools = StorageMap< _, Blake2_128Concat, PoolIdOf, - PoolInfo< - T::AccountId, - T::AssetId, - T::PoolAssetId, - AssetBalanceOf, - >, + PoolInfo>, OptionQuery, >; @@ -167,8 +164,8 @@ pub mod pallet { SwapExecuted { who: T::AccountId, send_to: T::AccountId, - asset1: T::AssetId, - asset2: T::AssetId, + asset1: MultiAssetId, + asset2: MultiAssetId, pool_id: PoolIdOf, amount_in: AssetBalanceOf, amount_out: AssetBalanceOf, @@ -214,8 +211,8 @@ pub mod pallet { #[pallet::weight(0)] pub fn create_pool( origin: OriginFor, - asset1: T::AssetId, // TODO: convert into MultiToken - asset2: T::AssetId, + asset1: MultiAssetId, + asset2: MultiAssetId, ) -> DispatchResult { let sender = ensure_signed(origin)?; ensure!(asset1 != asset2, Error::::EqualAssets); @@ -253,8 +250,8 @@ pub mod pallet { #[pallet::weight(0)] pub fn add_liquidity( origin: OriginFor, - asset1: T::AssetId, - asset2: T::AssetId, + asset1: MultiAssetId, + asset2: MultiAssetId, amount1_desired: AssetBalanceOf, amount2_desired: AssetBalanceOf, amount1_min: AssetBalanceOf, @@ -307,8 +304,22 @@ pub mod pallet { } let pallet_account = Self::account_id(); - T::Assets::transfer(asset1, &sender, &pallet_account, amount1, false)?; - T::Assets::transfer(asset2, &sender, &pallet_account, amount2, false)?; + match asset1 { + MultiAssetId::Native => { + T::Currency::transfer(&sender, &pallet_account, amount1, false)?; + }, + MultiAssetId::Asset(asset1) => { + T::Assets::transfer(asset1, &sender, &pallet_account, amount1, false)?; + }, + } + match asset2 { + MultiAssetId::Native => { + T::Currency::transfer(&sender, &pallet_account, amount2, false)?; + }, + MultiAssetId::Asset(asset2) => { + T::Assets::transfer(asset2, &sender, &pallet_account, amount2, false)?; + }, + } let total_supply = T::PoolAssets::total_issuance(pool.lp_token); @@ -361,8 +372,8 @@ pub mod pallet { #[pallet::weight(0)] pub fn remove_liquidity( origin: OriginFor, - asset1: T::AssetId, - asset2: T::AssetId, + asset1: MultiAssetId, + asset2: MultiAssetId, liquidity: AssetBalanceOf, amount1_min: AssetBalanceOf, amount2_min: AssetBalanceOf, @@ -413,9 +424,22 @@ pub mod pallet { T::PoolAssets::burn_from(pool.lp_token, &pallet_account, liquidity)?; - T::Assets::transfer(asset1, &pallet_account, &withdraw_to, amount1, false)?; - T::Assets::transfer(asset2, &pallet_account, &withdraw_to, amount2, false)?; - + match asset1 { + MultiAssetId::Native => { + T::Currency::transfer(&pallet_account, &withdraw_to, amount1, false)?; + }, + MultiAssetId::Asset(asset1) => { + T::Assets::transfer(asset1, &pallet_account, &withdraw_to, amount1, false)?; + }, + } + match asset2 { + MultiAssetId::Native => { + T::Currency::transfer(&pallet_account, &withdraw_to, amount2, false)?; + }, + MultiAssetId::Asset(asset2) => { + T::Assets::transfer(asset2, &pallet_account, &withdraw_to, amount2, false)?; + }, + } pool.balance1 = reserve1 - amount1; pool.balance2 = reserve2 - amount2; @@ -436,8 +460,8 @@ pub mod pallet { #[pallet::weight(0)] pub fn swap_exact_tokens_for_tokens( origin: OriginFor, - asset1: T::AssetId, - asset2: T::AssetId, + asset1: MultiAssetId, + asset2: MultiAssetId, amount_in: AssetBalanceOf, amount_out_min: AssetBalanceOf, send_to: T::AccountId, @@ -467,13 +491,32 @@ pub mod pallet { ensure!(amount2 >= amount_out_min, Error::::InsufficientOutputAmount); let pallet_account = Self::account_id(); - T::Assets::transfer(asset1, &sender, &pallet_account, amount1, false)?; + match asset1 { + MultiAssetId::Native => { + T::Currency::transfer(&sender, &pallet_account, amount1, false)?; + }, + MultiAssetId::Asset(asset1) => { + T::Assets::transfer(asset1, &sender, &pallet_account, amount1, false)?; + }, + } let (send_asset, send_amount) = Self::validate_swap(asset1, amount2, pool_id, pool.balance1, pool.balance2)?; - T::Assets::transfer(send_asset, &pallet_account, &send_to, send_amount, false)?; - + match send_asset { + MultiAssetId::Native => { + T::Currency::transfer(&pallet_account, &send_to, send_amount, false)?; + }, + MultiAssetId::Asset(send_asset) => { + T::Assets::transfer( + send_asset, + &pallet_account, + &send_to, + send_amount, + false, + )?; + }, + } if send_asset == pool.asset1 { pool.balance1 -= send_amount; pool.balance2 += amount1; @@ -499,8 +542,8 @@ pub mod pallet { #[pallet::weight(0)] pub fn swap_tokens_for_exact_tokens( origin: OriginFor, - asset1: T::AssetId, - asset2: T::AssetId, + asset1: MultiAssetId, + asset2: MultiAssetId, amount_out: AssetBalanceOf, amount_in_max: AssetBalanceOf, send_to: T::AccountId, @@ -529,12 +572,32 @@ pub mod pallet { ensure!(amount1 <= amount_in_max, Error::::ExcessiveInputAmount); let pallet_account = Self::account_id(); - T::Assets::transfer(asset1, &sender, &pallet_account, amount1, false)?; + match asset1 { + MultiAssetId::Native => { + T::Currency::transfer(&sender, &pallet_account, amount1, false)?; + }, + MultiAssetId::Asset(asset1) => { + T::Assets::transfer(asset1, &sender, &pallet_account, amount1, false)?; + }, + } let (send_asset, send_amount) = Self::validate_swap(asset1, amount2, pool_id, pool.balance1, pool.balance2)?; - T::Assets::transfer(send_asset, &pallet_account, &send_to, send_amount, false)?; + match send_asset { + MultiAssetId::Native => { + T::Currency::transfer(&pallet_account, &send_to, send_amount, false)?; + }, + MultiAssetId::Asset(send_asset) => { + T::Assets::transfer( + send_asset, + &pallet_account, + &send_to, + send_amount, + false, + )?; + }, + } if send_asset == pool.asset1 { pool.balance1 -= send_amount; @@ -570,7 +633,10 @@ pub mod pallet { } /// Returns a pool id constructed from 2 sorted assets. - pub fn get_pool_id(asset1: T::AssetId, asset2: T::AssetId) -> PoolIdOf { + pub fn get_pool_id( + asset1: MultiAssetId, + asset2: MultiAssetId, + ) -> PoolIdOf { if asset1 <= asset2 { (asset1, asset2) } else { @@ -579,8 +645,10 @@ pub mod pallet { } pub fn quote_price(asset1: u32, asset2: u32, amount: u64) -> Option> { - let asset1 = asset1.into(); - let asset2 = asset2.into(); + let asset1: T::AssetId = asset1.into(); + let asset2: T::AssetId = asset2.into(); + let asset1 = MultiAssetId::Asset(asset1); + let asset2 = MultiAssetId::Asset(asset2); let amount = amount.into(); let pool_id = Self::get_pool_id(asset1, asset2); let (pool_asset1, _) = pool_id; @@ -676,12 +744,12 @@ pub mod pallet { } pub fn validate_swap( - asset_from: T::AssetId, + asset_from: MultiAssetId, amount_out: AssetBalanceOf, pool_id: PoolIdOf, reserve1: AssetBalanceOf, reserve2: AssetBalanceOf, - ) -> Result<(T::AssetId, AssetBalanceOf), Error> { + ) -> Result<(MultiAssetId, AssetBalanceOf), Error> { let (pool_asset1, pool_asset2) = pool_id; if asset_from == pool_asset1 { diff --git a/frame/dex/src/tests.rs b/frame/dex/src/tests.rs index 9e0cd2f0c62f2..161a858479a19 100644 --- a/frame/dex/src/tests.rs +++ b/frame/dex/src/tests.rs @@ -17,10 +17,7 @@ use crate::{mock::*, *}; -use frame_support::{ - assert_noop, assert_ok, - traits::{fungibles::InspectEnumerable, Currency}, -}; +use frame_support::{assert_noop, assert_ok, traits::fungibles::InspectEnumerable}; fn events() -> Vec> { let result = System::events() @@ -40,10 +37,12 @@ fn pools() -> Vec> { s } -fn assets() -> Vec { +fn assets() -> Vec> { // if the storage would be public: // let mut s: Vec<_> = pallet_assets::pallet::Asset::::iter().map(|x| x.0).collect(); - let mut s: Vec<_> = <::Assets>::asset_ids().collect(); + let mut s: Vec<_> = <::Assets>::asset_ids() + .map(|id| MultiAssetId::Asset(id)) + .collect(); s.sort(); s } @@ -56,19 +55,19 @@ fn pool_assets() -> Vec { s } -fn create_tokens(owner: u64, tokens: Vec) { +fn create_tokens(owner: u64, tokens: Vec>) { for token_id in tokens { - assert_ok!(Assets::force_create(RuntimeOrigin::root(), token_id, owner, true, 1)); + if let MultiAssetId::Asset(token_id) = token_id { + assert_ok!(Assets::force_create(RuntimeOrigin::root(), token_id, owner, true, 1)); + } } } -fn topup_pallet() { - let pallet_account = Dex::account_id(); - Balances::make_free_balance_be(&pallet_account, 10000); -} - -fn balance(owner: u64, token_id: u32) -> u64 { - <::Assets>::balance(token_id, owner) +fn balance(owner: u64, token_id: MultiAssetId) -> u64 { + match token_id { + MultiAssetId::Native => <::Currency>::free_balance(owner), + MultiAssetId::Asset(token_id) => <::Assets>::balance(token_id, owner), + } } fn pool_balance(owner: u64, token_id: u32) -> u64 { @@ -79,10 +78,9 @@ fn pool_balance(owner: u64, token_id: u32) -> u64 { fn create_pool_should_work() { new_test_ext().execute_with(|| { let user = 1; - let token_1 = 1; - let token_2 = 2; + let token_1 = MultiAssetId::Asset(1); + let token_2 = MultiAssetId::Asset(2); let pool_id = (token_1, token_2); - topup_pallet(); create_tokens(user, vec![token_1, token_2]); @@ -101,9 +99,8 @@ fn create_pool_should_work() { fn create_same_pool_twice_should_fail() { new_test_ext().execute_with(|| { let user = 1; - let token_1 = 1; - let token_2 = 2; - topup_pallet(); + let token_1 = MultiAssetId::Asset(1); + let token_2 = MultiAssetId::Asset(2); create_tokens(user, vec![token_1, token_2]); @@ -131,12 +128,11 @@ fn create_same_pool_twice_should_fail() { fn different_pools_should_have_different_lp_tokens() { new_test_ext().execute_with(|| { let user = 1; - let token_1 = 1; - let token_2 = 2; - let token_3 = 3; + let token_1 = MultiAssetId::Asset(1); + let token_2 = MultiAssetId::Asset(2); + let token_3 = MultiAssetId::Asset(3); let pool_id_1_2 = (token_1, token_2); let pool_id_1_3 = (token_1, token_3); - topup_pallet(); create_tokens(user, vec![token_1, token_2]); @@ -171,17 +167,16 @@ fn different_pools_should_have_different_lp_tokens() { fn add_liquidity_should_work() { new_test_ext().execute_with(|| { let user = 1; - let token_1 = 1; - let token_2 = 2; + let token_1 = MultiAssetId::Asset(1); + let token_2 = MultiAssetId::Asset(2); let pool_id = (token_1, token_2); - topup_pallet(); create_tokens(user, vec![token_1, token_2]); let lp_token = Dex::get_next_pool_asset_id(); assert_ok!(Dex::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); - assert_ok!(Assets::mint(RuntimeOrigin::signed(user), token_1, user, 1000)); - assert_ok!(Assets::mint(RuntimeOrigin::signed(user), token_2, user, 1000)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 1, user, 1000)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); assert_ok!(Dex::add_liquidity( RuntimeOrigin::signed(user), @@ -216,17 +211,16 @@ fn add_liquidity_should_work() { fn remove_liquidity_should_work() { new_test_ext().execute_with(|| { let user = 1; - let token_1 = 1; - let token_2 = 2; + let token_1 = MultiAssetId::Asset(1); + let token_2 = MultiAssetId::Asset(2); let pool_id = (token_1, token_2); - topup_pallet(); create_tokens(user, vec![token_1, token_2]); let lp_token = Dex::get_next_pool_asset_id(); assert_ok!(Dex::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); - assert_ok!(Assets::mint(RuntimeOrigin::signed(user), token_1, user, 1000)); - assert_ok!(Assets::mint(RuntimeOrigin::signed(user), token_2, user, 1000)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 1, user, 1000)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); assert_ok!(Dex::add_liquidity( RuntimeOrigin::signed(user), @@ -276,15 +270,14 @@ fn remove_liquidity_should_work() { fn quote_price_should_work() { new_test_ext().execute_with(|| { let user = 1; - let token_1 = 1; - let token_2 = 2; - topup_pallet(); + let token_1 = MultiAssetId::Asset(1); + let token_2 = MultiAssetId::Asset(2); create_tokens(user, vec![token_1, token_2]); assert_ok!(Dex::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); - assert_ok!(Assets::mint(RuntimeOrigin::signed(user), token_1, user, 1000)); - assert_ok!(Assets::mint(RuntimeOrigin::signed(user), token_2, user, 1000)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 1, user, 1000)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); assert_ok!(Dex::add_liquidity( RuntimeOrigin::signed(user), @@ -298,7 +291,7 @@ fn quote_price_should_work() { 2 )); - assert_eq!(Dex::quote_price(token_1, token_2, 3000), Some(60)); + assert_eq!(Dex::quote_price(1, 2, 3000), Some(60)); }); } @@ -306,15 +299,14 @@ fn quote_price_should_work() { fn swap_should_work() { new_test_ext().execute_with(|| { let user = 1; - let token_1 = 1; - let token_2 = 2; - topup_pallet(); + let token_1 = MultiAssetId::Asset(1); + let token_2 = MultiAssetId::Asset(2); create_tokens(user, vec![token_1, token_2]); assert_ok!(Dex::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); - assert_ok!(Assets::mint(RuntimeOrigin::signed(user), token_1, user, 1000)); - assert_ok!(Assets::mint(RuntimeOrigin::signed(user), token_2, user, 1000)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 1, user, 1000)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); let liquidity1 = 1000; let liquidity2 = 20; @@ -352,12 +344,58 @@ fn swap_should_work() { }); } +#[test] +fn swap_should_work_with_native() { + new_test_ext().execute_with(|| { + let user = 1; + let token_1 = MultiAssetId::Native; + let token_2 = MultiAssetId::Asset(2); + + create_tokens(user, vec![token_2]); + assert_ok!(Dex::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + + assert_ok!(Balances::set_balance(RuntimeOrigin::root(), user, 1000, 0)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); + + let liquidity1 = 1000; + let liquidity2 = 20; + assert_ok!(Dex::add_liquidity( + RuntimeOrigin::signed(user), + token_1, + token_2, + liquidity1, + liquidity2, + 1, + 1, + user, + 2 + )); + + let exchange_amount = 10; + assert_ok!(Dex::swap_exact_tokens_for_tokens( + RuntimeOrigin::signed(user), + token_2, + token_1, + exchange_amount, + 1, + user, + 3 + )); + + let expect_receive = + Dex::get_amount_out(&exchange_amount, &liquidity2, &liquidity1).ok().unwrap(); + let pallet_account = Dex::account_id(); + assert_eq!(balance(user, token_1), expect_receive); + assert_eq!(balance(pallet_account, token_1), liquidity1 - expect_receive); + assert_eq!(balance(pallet_account, token_2), liquidity2 + exchange_amount); + }); +} + #[test] fn same_asset_swap_should_fail() { new_test_ext().execute_with(|| { let user = 1; - let token_1 = 1; - topup_pallet(); + let token_1 = MultiAssetId::Asset(1); create_tokens(user, vec![token_1]); assert_noop!( @@ -365,7 +403,7 @@ fn same_asset_swap_should_fail() { Error::::EqualAssets ); - assert_ok!(Assets::mint(RuntimeOrigin::signed(user), token_1, user, 1000)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 1, user, 1000)); let liquidity1 = 1000; let liquidity2 = 20; @@ -397,5 +435,18 @@ fn same_asset_swap_should_fail() { ), Error::::PoolNotFound ); + + assert_noop!( + Dex::swap_exact_tokens_for_tokens( + RuntimeOrigin::signed(user), + MultiAssetId::Native, + MultiAssetId::Native, + exchange_amount, + 1, + user, + 3 + ), + Error::::PoolNotFound + ); }); } diff --git a/frame/dex/src/types.rs b/frame/dex/src/types.rs index 239485716ca30..0931e31ff7c0a 100644 --- a/frame/dex/src/types.rs +++ b/frame/dex/src/types.rs @@ -18,16 +18,36 @@ use codec::{Decode, Encode, MaxEncodedLen}; use scale_info::TypeInfo; -#[derive(Encode, Decode, Default, PartialEq, Eq, MaxEncodedLen, TypeInfo)] +#[derive( + Decode, + Encode, + Default, + PartialEq, + Eq, + MaxEncodedLen, + TypeInfo, + Clone, + Copy, + Debug, + Ord, + PartialOrd, +)] +pub enum MultiAssetId { + #[default] + Native, + Asset(AssetId), +} + +#[derive(Decode, Encode, Default, PartialEq, Eq, MaxEncodedLen, TypeInfo)] pub struct PoolInfo { /// Owner of the pool pub owner: AccountId, /// Liquidity pool asset pub lp_token: PoolAssetId, /// The first asset supported by the pool - pub asset1: AssetId, + pub asset1: MultiAssetId, /// The second asset supported by the pool - pub asset2: AssetId, + pub asset2: MultiAssetId, /// Pool balance of asset1 pub balance1: Balance, /// Pool balance of asset2 diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index f67423efb2907..e67f7a2a990ab 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -90,8 +90,8 @@ pub use hooks::{ pub mod schedule; mod storage; pub use storage::{ - Instance, PartialStorageInfoTrait, StorageInfo, StorageInfoTrait, StorageInstance, - TrackedStorageKey, WhitelistedStorageKeys, Incrementable, + Incrementable, Instance, PartialStorageInfoTrait, StorageInfo, StorageInfoTrait, + StorageInstance, TrackedStorageKey, WhitelistedStorageKeys, }; mod dispatch; diff --git a/frame/support/src/traits/storage.rs b/frame/support/src/traits/storage.rs index 95aa159767779..eddeb3852713f 100644 --- a/frame/support/src/traits/storage.rs +++ b/frame/support/src/traits/storage.rs @@ -20,8 +20,8 @@ use crate::sp_std::collections::btree_set::BTreeSet; use impl_trait_for_tuples::impl_for_tuples; pub use sp_core::storage::TrackedStorageKey; -use sp_std::prelude::*; use sp_runtime::traits::Saturating; +use sp_std::prelude::*; /// An instance of a pallet in the storage. /// @@ -144,4 +144,4 @@ pub trait Incrementable { fn initial_value() -> Self; } -impl_incrementable!(u8, u16, u32, u64, u128, i8, i16, i32, i64, i128); \ No newline at end of file +impl_incrementable!(u8, u16, u32, u64, u128, i8, i16, i32, i64, i128); From 97936d6b31601d6918a48d731cc4f567712137f5 Mon Sep 17 00:00:00 2001 From: Giles Cope Date: Sat, 10 Dec 2022 13:49:33 +0000 Subject: [PATCH 2/4] only allow native currency pairs --- frame/dex/src/lib.rs | 26 ++++++++++--- frame/dex/src/tests.rs | 83 +++++++++--------------------------------- 2 files changed, 38 insertions(+), 71 deletions(-) diff --git a/frame/dex/src/lib.rs b/frame/dex/src/lib.rs index 52fa3a758c948..56239e8c9b1c0 100644 --- a/frame/dex/src/lib.rs +++ b/frame/dex/src/lib.rs @@ -203,6 +203,8 @@ pub mod pallet { InsufficientLiquidity, /// Excessive input amount. ExcessiveInputAmount, + /// Only pools with native on one side are valid. + PoolMustContainNativeCurrency, } // Pallet's callable functions. @@ -219,6 +221,11 @@ pub mod pallet { let pool_id = Self::get_pool_id(asset1, asset2); let (asset1, asset2) = pool_id; + + if !matches!(asset1, MultiAssetId::Native) { + Err(Error::::PoolMustContainNativeCurrency)?; + } + ensure!(!Pools::::contains_key(&pool_id), Error::::PoolExists); let pallet_account = Self::account_id(); @@ -644,11 +651,20 @@ pub mod pallet { } } - pub fn quote_price(asset1: u32, asset2: u32, amount: u64) -> Option> { - let asset1: T::AssetId = asset1.into(); - let asset2: T::AssetId = asset2.into(); - let asset1 = MultiAssetId::Asset(asset1); - let asset2 = MultiAssetId::Asset(asset2); + pub fn quote_price( + asset1: Option, + asset2: Option, + amount: u64, + ) -> Option> { + let into_multi = |asset| { + if let Some(asset) = asset { + MultiAssetId::Asset(T::AssetId::from(asset)) + } else { + MultiAssetId::Native + } + }; + let asset1 = into_multi(asset1); + let asset2 = into_multi(asset2); let amount = amount.into(); let pool_id = Self::get_pool_id(asset1, asset2); let (pool_asset1, _) = pool_id; diff --git a/frame/dex/src/tests.rs b/frame/dex/src/tests.rs index 161a858479a19..d23d66b06eaf6 100644 --- a/frame/dex/src/tests.rs +++ b/frame/dex/src/tests.rs @@ -78,11 +78,11 @@ fn pool_balance(owner: u64, token_id: u32) -> u64 { fn create_pool_should_work() { new_test_ext().execute_with(|| { let user = 1; - let token_1 = MultiAssetId::Asset(1); + let token_1 = MultiAssetId::Native; let token_2 = MultiAssetId::Asset(2); let pool_id = (token_1, token_2); - create_tokens(user, vec![token_1, token_2]); + create_tokens(user, vec![token_2]); let lp_token: u32 = Dex::get_next_pool_asset_id(); assert_ok!(Dex::create_pool(RuntimeOrigin::signed(user), token_2, token_1)); @@ -90,7 +90,7 @@ fn create_pool_should_work() { assert_eq!(events(), [Event::::PoolCreated { creator: user, pool_id, lp_token }]); assert_eq!(pools(), vec![pool_id]); - assert_eq!(assets(), vec![token_1, token_2]); + assert_eq!(assets(), vec![token_2]); assert_eq!(pool_assets(), vec![lp_token]); }); } @@ -99,10 +99,10 @@ fn create_pool_should_work() { fn create_same_pool_twice_should_fail() { new_test_ext().execute_with(|| { let user = 1; - let token_1 = MultiAssetId::Asset(1); + let token_1 = MultiAssetId::Native; let token_2 = MultiAssetId::Asset(2); - create_tokens(user, vec![token_1, token_2]); + create_tokens(user, vec![token_2]); let lp_token: u32 = Dex::get_next_pool_asset_id(); assert_ok!(Dex::create_pool(RuntimeOrigin::signed(user), token_2, token_1)); @@ -128,13 +128,13 @@ fn create_same_pool_twice_should_fail() { fn different_pools_should_have_different_lp_tokens() { new_test_ext().execute_with(|| { let user = 1; - let token_1 = MultiAssetId::Asset(1); + let token_1 = MultiAssetId::Native; let token_2 = MultiAssetId::Asset(2); let token_3 = MultiAssetId::Asset(3); let pool_id_1_2 = (token_1, token_2); let pool_id_1_3 = (token_1, token_3); - create_tokens(user, vec![token_1, token_2]); + create_tokens(user, vec![token_2]); let lp_token2_1: u32 = Dex::get_next_pool_asset_id(); assert_ok!(Dex::create_pool(RuntimeOrigin::signed(user), token_2, token_1)); @@ -167,15 +167,15 @@ fn different_pools_should_have_different_lp_tokens() { fn add_liquidity_should_work() { new_test_ext().execute_with(|| { let user = 1; - let token_1 = MultiAssetId::Asset(1); + let token_1 = MultiAssetId::Native; let token_2 = MultiAssetId::Asset(2); let pool_id = (token_1, token_2); - create_tokens(user, vec![token_1, token_2]); + create_tokens(user, vec![token_2]); let lp_token = Dex::get_next_pool_asset_id(); assert_ok!(Dex::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); - assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 1, user, 1000)); + assert_ok!(Balances::set_balance(RuntimeOrigin::root(), user, 1000, 0)); assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); assert_ok!(Dex::add_liquidity( @@ -211,15 +211,15 @@ fn add_liquidity_should_work() { fn remove_liquidity_should_work() { new_test_ext().execute_with(|| { let user = 1; - let token_1 = MultiAssetId::Asset(1); + let token_1 = MultiAssetId::Native; let token_2 = MultiAssetId::Asset(2); let pool_id = (token_1, token_2); - create_tokens(user, vec![token_1, token_2]); + create_tokens(user, vec![token_2]); let lp_token = Dex::get_next_pool_asset_id(); assert_ok!(Dex::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); - assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 1, user, 1000)); + assert_ok!(Balances::set_balance(RuntimeOrigin::root(), user, 1000, 0)); assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); assert_ok!(Dex::add_liquidity( @@ -270,13 +270,13 @@ fn remove_liquidity_should_work() { fn quote_price_should_work() { new_test_ext().execute_with(|| { let user = 1; - let token_1 = MultiAssetId::Asset(1); + let token_1 = MultiAssetId::Native; let token_2 = MultiAssetId::Asset(2); - create_tokens(user, vec![token_1, token_2]); + create_tokens(user, vec![token_2]); assert_ok!(Dex::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); - assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 1, user, 1000)); + assert_ok!(Balances::set_balance(RuntimeOrigin::root(), user, 1000, 0)); assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); assert_ok!(Dex::add_liquidity( @@ -291,56 +291,7 @@ fn quote_price_should_work() { 2 )); - assert_eq!(Dex::quote_price(1, 2, 3000), Some(60)); - }); -} - -#[test] -fn swap_should_work() { - new_test_ext().execute_with(|| { - let user = 1; - let token_1 = MultiAssetId::Asset(1); - let token_2 = MultiAssetId::Asset(2); - - create_tokens(user, vec![token_1, token_2]); - assert_ok!(Dex::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); - - assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 1, user, 1000)); - assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); - - let liquidity1 = 1000; - let liquidity2 = 20; - assert_ok!(Dex::add_liquidity( - RuntimeOrigin::signed(user), - token_1, - token_2, - liquidity1, - liquidity2, - 1, - 1, - user, - 2 - )); - - assert_eq!(balance(user, token_1), 0); - - let exchange_amount = 10; - assert_ok!(Dex::swap_exact_tokens_for_tokens( - RuntimeOrigin::signed(user), - token_2, - token_1, - exchange_amount, - 1, - user, - 3 - )); - - let expect_receive = - Dex::get_amount_out(&exchange_amount, &liquidity2, &liquidity1).ok().unwrap(); - let pallet_account = Dex::account_id(); - assert_eq!(balance(user, token_1), expect_receive); - assert_eq!(balance(pallet_account, token_1), liquidity1 - expect_receive); - assert_eq!(balance(pallet_account, token_2), liquidity2 + exchange_amount); + assert_eq!(Dex::quote_price(None, Some(2), 3000), Some(60)); }); } From 68251e9d66b5409fb7a44d624d32399813d28e14 Mon Sep 17 00:00:00 2001 From: Giles Cope Date: Sun, 11 Dec 2022 12:59:12 +0000 Subject: [PATCH 3/4] added slippage tests --- frame/dex/src/tests.rs | 160 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 157 insertions(+), 3 deletions(-) diff --git a/frame/dex/src/tests.rs b/frame/dex/src/tests.rs index d23d66b06eaf6..de3bad605d8b6 100644 --- a/frame/dex/src/tests.rs +++ b/frame/dex/src/tests.rs @@ -17,7 +17,7 @@ use crate::{mock::*, *}; -use frame_support::{assert_noop, assert_ok, traits::fungibles::InspectEnumerable}; +use frame_support::{assert_err, assert_noop, assert_ok, traits::fungibles::InspectEnumerable}; fn events() -> Vec> { let result = System::events() @@ -323,6 +323,9 @@ fn swap_should_work_with_native() { )); let exchange_amount = 10; + let expect_receive = + Dex::get_amount_out(&exchange_amount, &liquidity2, &liquidity1).ok().unwrap(); + assert_ok!(Dex::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), token_2, @@ -333,8 +336,6 @@ fn swap_should_work_with_native() { 3 )); - let expect_receive = - Dex::get_amount_out(&exchange_amount, &liquidity2, &liquidity1).ok().unwrap(); let pallet_account = Dex::account_id(); assert_eq!(balance(user, token_1), expect_receive); assert_eq!(balance(pallet_account, token_1), liquidity1 - expect_receive); @@ -342,6 +343,159 @@ fn swap_should_work_with_native() { }); } +#[test] +fn swap_should_not_work_with_if_too_much_slippage() { + new_test_ext().execute_with(|| { + let user = 1; + let token_1 = MultiAssetId::Native; + let token_2 = MultiAssetId::Asset(2); + + create_tokens(user, vec![token_2]); + assert_ok!(Dex::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + + assert_ok!(Balances::set_balance(RuntimeOrigin::root(), user, 1000, 0)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); + + let liquidity1 = 1000; + let liquidity2 = 20; + assert_ok!(Dex::add_liquidity( + RuntimeOrigin::signed(user), + token_1, + token_2, + liquidity1, + liquidity2, + 1, + 1, + user, + 2 + )); + + let exchange_amount = 10; + + assert_err!( + Dex::swap_exact_tokens_for_tokens( + RuntimeOrigin::signed(user), + token_2, + token_1, + exchange_amount, + 333, // amount out min + user, + 3 + ), + Error::::InsufficientOutputAmount + ); + }); +} + +#[test] +fn swap_tokens_for_exact_tokens_should_work() { + new_test_ext().execute_with(|| { + let user = 1; + let token_1 = MultiAssetId::Native; + let token_2 = MultiAssetId::Asset(2); + let deadline = 2; + let pallet_account = Dex::account_id(); + let base1 = 1000; + + create_tokens(user, vec![token_2]); + assert_ok!(Dex::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + + assert_ok!(Balances::set_balance(RuntimeOrigin::root(), user, base1 + 1000, 0)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); + + let before1 = balance(pallet_account, token_1) + balance(user, token_1); + let before2 = balance(pallet_account, token_2) + balance(user, token_2); + + let liquidity1 = 1000; + let liquidity2 = 20; + assert_ok!(Dex::add_liquidity( + RuntimeOrigin::signed(user), + token_1, + token_2, + liquidity1, + liquidity2, + 1, + 1, + user, + deadline + )); + + assert_eq!(balance(user, token_1), 1000); + assert_eq!(balance(user, token_2), 980); + + let exchange_out2 = 1; + + let expect_in1 = + dbg!(Dex::get_amount_in(&exchange_out2, &liquidity1, &liquidity2).ok().unwrap()); + + assert_ok!(Dex::swap_tokens_for_exact_tokens( + RuntimeOrigin::signed(user), + token_1, + token_2, + exchange_out2, + 100, // amount_in_max + user, + 3 + )); + + assert_eq!(balance(user, token_1), 1000 - expect_in1); + assert_eq!(balance(pallet_account, token_1), liquidity1 + expect_in1); + assert_eq!(balance(user, token_2), 1000 - 20 + exchange_out2); + assert_eq!(balance(pallet_account, token_2), dbg!(liquidity2) - exchange_out2); + + // check invariants: + + // dot and asset totals should be preserved. + assert_eq!(before1, balance(pallet_account, token_1) + balance(user, token_1)); + assert_eq!(before2, balance(pallet_account, token_2) + balance(user, token_2)); + }); +} + +#[test] +fn swap_tokens_for_exact_tokens_should_not_work_if_too_much_slippage() { + new_test_ext().execute_with(|| { + let user = 1; + let token_1 = MultiAssetId::Native; + let token_2 = MultiAssetId::Asset(2); + let deadline = 2; + let base1 = 1000; + + create_tokens(user, vec![token_2]); + assert_ok!(Dex::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + + assert_ok!(Balances::set_balance(RuntimeOrigin::root(), user, base1 + 1000, 0)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); + + let liquidity1 = 1000; + let liquidity2 = 20; + assert_ok!(Dex::add_liquidity( + RuntimeOrigin::signed(user), + token_1, + token_2, + liquidity1, + liquidity2, + 1, + 1, + user, + deadline + )); + let exchange_out2 = 1; + + assert_err!( + Dex::swap_tokens_for_exact_tokens( + RuntimeOrigin::signed(user), + token_1, + token_2, + exchange_out2, + 52, // amount_in_max just greater than slippage. + user, + 3 + ), + >::ExcessiveInputAmount + ); + }); +} + #[test] fn same_asset_swap_should_fail() { new_test_ext().execute_with(|| { From 0a17ab5955c7a778099bb503b8342fdd383638c6 Mon Sep 17 00:00:00 2001 From: Giles Cope Date: Mon, 12 Dec 2022 12:49:17 +0000 Subject: [PATCH 4/4] transfer into separate method (Also fee not set in 2 places now.) Added test where lp and user are different users. --- frame/dex/src/lib.rs | 113 ++++++++++++----------------------------- frame/dex/src/mock.rs | 1 + frame/dex/src/tests.rs | 88 ++++++++++++++++++++++++++++++++ 3 files changed, 121 insertions(+), 81 deletions(-) diff --git a/frame/dex/src/lib.rs b/frame/dex/src/lib.rs index 56239e8c9b1c0..0072a79f275d2 100644 --- a/frame/dex/src/lib.rs +++ b/frame/dex/src/lib.rs @@ -33,7 +33,6 @@ pub use types::*; // TODO: make it configurable // TODO: weights and benchmarking. // TODO: more specific error codes. -// TODO: remove setup. pub const MIN_LIQUIDITY: u64 = 1; #[frame_support::pallet] @@ -61,6 +60,9 @@ pub mod pallet { pub trait Config: frame_system::Config { type RuntimeEvent: From> + IsType<::RuntimeEvent>; + /// Units are 10ths of a percent + type Fee: Get; + type Currency: InspectFungible + TransferFungible; @@ -222,7 +224,7 @@ pub mod pallet { let pool_id = Self::get_pool_id(asset1, asset2); let (asset1, asset2) = pool_id; - if !matches!(asset1, MultiAssetId::Native) { + if asset1 != MultiAssetId::Native { Err(Error::::PoolMustContainNativeCurrency)?; } @@ -311,22 +313,8 @@ pub mod pallet { } let pallet_account = Self::account_id(); - match asset1 { - MultiAssetId::Native => { - T::Currency::transfer(&sender, &pallet_account, amount1, false)?; - }, - MultiAssetId::Asset(asset1) => { - T::Assets::transfer(asset1, &sender, &pallet_account, amount1, false)?; - }, - } - match asset2 { - MultiAssetId::Native => { - T::Currency::transfer(&sender, &pallet_account, amount2, false)?; - }, - MultiAssetId::Asset(asset2) => { - T::Assets::transfer(asset2, &sender, &pallet_account, amount2, false)?; - }, - } + Self::transfer(asset1, &sender, &pallet_account, amount1, false)?; + Self::transfer(asset2, &sender, &pallet_account, amount2, false)?; let total_supply = T::PoolAssets::total_issuance(pool.lp_token); @@ -431,22 +419,9 @@ pub mod pallet { T::PoolAssets::burn_from(pool.lp_token, &pallet_account, liquidity)?; - match asset1 { - MultiAssetId::Native => { - T::Currency::transfer(&pallet_account, &withdraw_to, amount1, false)?; - }, - MultiAssetId::Asset(asset1) => { - T::Assets::transfer(asset1, &pallet_account, &withdraw_to, amount1, false)?; - }, - } - match asset2 { - MultiAssetId::Native => { - T::Currency::transfer(&pallet_account, &withdraw_to, amount2, false)?; - }, - MultiAssetId::Asset(asset2) => { - T::Assets::transfer(asset2, &pallet_account, &withdraw_to, amount2, false)?; - }, - } + Self::transfer(asset1, &pallet_account, &withdraw_to, amount1, false)?; + Self::transfer(asset2, &pallet_account, &withdraw_to, amount2, false)?; + pool.balance1 = reserve1 - amount1; pool.balance2 = reserve2 - amount2; @@ -498,32 +473,13 @@ pub mod pallet { ensure!(amount2 >= amount_out_min, Error::::InsufficientOutputAmount); let pallet_account = Self::account_id(); - match asset1 { - MultiAssetId::Native => { - T::Currency::transfer(&sender, &pallet_account, amount1, false)?; - }, - MultiAssetId::Asset(asset1) => { - T::Assets::transfer(asset1, &sender, &pallet_account, amount1, false)?; - }, - } + Self::transfer(asset1, &sender, &pallet_account, amount1, false)?; let (send_asset, send_amount) = Self::validate_swap(asset1, amount2, pool_id, pool.balance1, pool.balance2)?; - match send_asset { - MultiAssetId::Native => { - T::Currency::transfer(&pallet_account, &send_to, send_amount, false)?; - }, - MultiAssetId::Asset(send_asset) => { - T::Assets::transfer( - send_asset, - &pallet_account, - &send_to, - send_amount, - false, - )?; - }, - } + Self::transfer(send_asset, &pallet_account, &send_to, send_amount, false)?; + if send_asset == pool.asset1 { pool.balance1 -= send_amount; pool.balance2 += amount1; @@ -579,32 +535,12 @@ pub mod pallet { ensure!(amount1 <= amount_in_max, Error::::ExcessiveInputAmount); let pallet_account = Self::account_id(); - match asset1 { - MultiAssetId::Native => { - T::Currency::transfer(&sender, &pallet_account, amount1, false)?; - }, - MultiAssetId::Asset(asset1) => { - T::Assets::transfer(asset1, &sender, &pallet_account, amount1, false)?; - }, - } + Self::transfer(asset1, &sender, &pallet_account, amount1, false)?; let (send_asset, send_amount) = Self::validate_swap(asset1, amount2, pool_id, pool.balance1, pool.balance2)?; - match send_asset { - MultiAssetId::Native => { - T::Currency::transfer(&pallet_account, &send_to, send_amount, false)?; - }, - MultiAssetId::Asset(send_asset) => { - T::Assets::transfer( - send_asset, - &pallet_account, - &send_to, - send_amount, - false, - )?; - }, - } + Self::transfer(send_asset, &pallet_account, &send_to, send_amount, false)?; if send_asset == pool.asset1 { pool.balance1 -= send_amount; @@ -631,6 +567,20 @@ pub mod pallet { // Your Pallet's internal functions. impl Pallet { + fn transfer( + asset_id: MultiAssetId, + from: &T::AccountId, + to: &T::AccountId, + amount: AssetBalanceOf, + keep_alive: bool, + ) -> Result<::AssetBalance, DispatchError> { + match asset_id { + MultiAssetId::Native => T::Currency::transfer(from, to, amount, keep_alive), + MultiAssetId::Asset(asset_id) => + T::Assets::transfer(asset_id, from, to, amount, keep_alive), + } + } + /// The account ID of the dex pallet. /// /// This actually does computation. If you need to keep using it, then make sure you cache @@ -711,8 +661,9 @@ pub mod pallet { // TODO: extract 0.3% into config // TODO: could use Permill type - let amount_in_with_fee = - amount_in.checked_mul(&997u64.into()).ok_or(Error::::Overflow)?; + let amount_in_with_fee = amount_in + .checked_mul(&(1000u64 - T::Fee::get()).into()) + .ok_or(Error::::Overflow)?; let numerator = amount_in_with_fee.checked_mul(reserve_out).ok_or(Error::::Overflow)?; @@ -749,7 +700,7 @@ pub mod pallet { let denominator = reserve_out .checked_sub(amount_out) .ok_or(Error::::Overflow)? - .checked_mul(&997u64.into()) + .checked_mul(&(1000u64 - T::Fee::get()).into()) .ok_or(Error::::Overflow)?; numerator diff --git a/frame/dex/src/mock.rs b/frame/dex/src/mock.rs index 64b309700b4ee..ae598dedfc8c5 100644 --- a/frame/dex/src/mock.rs +++ b/frame/dex/src/mock.rs @@ -140,6 +140,7 @@ frame_support::ord_parameter_types! { impl Config for Test { type RuntimeEvent = RuntimeEvent; + type Fee = ConstU64<3>; type Currency = Balances; type AssetBalance = ::Balance; type Assets = Assets; diff --git a/frame/dex/src/tests.rs b/frame/dex/src/tests.rs index de3bad605d8b6..e3c0e0f6fe401 100644 --- a/frame/dex/src/tests.rs +++ b/frame/dex/src/tests.rs @@ -451,6 +451,94 @@ fn swap_tokens_for_exact_tokens_should_work() { }); } +#[test] +fn swap_tokens_for_exact_tokens_works_when_user_is_not_liquidity_provider() { + new_test_ext().execute_with(|| { + let user = 1; + let user2 = 2; + let token_1 = MultiAssetId::Native; + let token_2 = MultiAssetId::Asset(2); + let deadline = 2; + let pallet_account = Dex::account_id(); + let base1 = 1000; + + create_tokens(user2, vec![token_2]); + assert_ok!(Dex::create_pool(RuntimeOrigin::signed(user2), token_1, token_2)); + + assert_ok!(Balances::set_balance(RuntimeOrigin::root(), user, base1, 0)); + assert_ok!(Balances::set_balance(RuntimeOrigin::root(), user2, 1000, 0)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user2), 2, user2, 1000)); + + let before1 = + balance(pallet_account, token_1) + balance(user, token_1) + balance(user2, token_1); + let before2 = + balance(pallet_account, token_2) + balance(user, token_2) + balance(user2, token_2); + + let liquidity1 = 1000; + let liquidity2 = 20; + assert_ok!(Dex::add_liquidity( + RuntimeOrigin::signed(user2), + token_1, + token_2, + liquidity1, + liquidity2, + 1, + 1, + user2, + deadline + )); + + assert_eq!(balance(user, token_1), 1000); + assert_eq!(balance(user, token_2), 0); + + let exchange_out2 = 1; + + let expect_in1 = + dbg!(Dex::get_amount_in(&exchange_out2, &liquidity1, &liquidity2).ok().unwrap()); + + assert_ok!(Dex::swap_tokens_for_exact_tokens( + RuntimeOrigin::signed(user), + token_1, + token_2, + exchange_out2, + 100, // amount_in_max + user, + 3 + )); + + assert_eq!(balance(user, token_1), 1000 - expect_in1); + assert_eq!(balance(pallet_account, token_1), liquidity1 + expect_in1); + assert_eq!(balance(user, token_2), exchange_out2); + assert_eq!(balance(pallet_account, token_2), dbg!(liquidity2) - exchange_out2); + + // check invariants: + + // dot and asset totals should be preserved. + assert_eq!( + before1, + balance(pallet_account, token_1) + balance(user, token_1) + balance(user2, token_1) + ); + assert_eq!( + before2, + balance(pallet_account, token_2) + balance(user, token_2) + balance(user2, token_2) + ); + + assert_ok!(Dex::remove_liquidity( + RuntimeOrigin::signed(user2), + token_1, + token_2, + 9, + 0, + 0, + user2, + 2 + )); + + // assert_eq!(balance(user2, token_1), 67); + // assert_eq!(balance(user2, token_2), 981); + }); +} + #[test] fn swap_tokens_for_exact_tokens_should_not_work_if_too_much_slippage() { new_test_ext().execute_with(|| {