Merge pull request #1 from paritytech/giles-more

Pool assets in separate instance

Author: Squirrel

frame/dex/rpc/src/lib.rs

@@ -70,7 +70,6 @@ impl From<Error> for i32 {
 	}
 }
 
-#[async_trait]
 impl<Client, Block, Balance> DexApiServer<Balance> for Dex<Client, Block>
 where
 	Block: BlockT,

frame/dex/src/lib.rs

@@ -30,12 +30,14 @@ pub use types::*;
 
 // https://docs.uniswap.org/protocol/V2/concepts/protocol-overview/smart-contracts#minimum-liquidity
 // 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]
 pub mod pallet {
 	use super::*;
-
 	use frame_support::pallet_prelude::*;
 	use frame_system::pallet_prelude::*;
 
@@ -47,7 +49,7 @@ pub mod pallet {
 			},
 			Currency, ExistenceRequirement, ReservableCurrency,
 		},
-		transactional, PalletId,
+		PalletId,
 	};
 	use sp_runtime::traits::{
 		AccountIdConversion, AtLeast32BitUnsigned, CheckedAdd, CheckedDiv, CheckedMul, CheckedSub,
@@ -93,6 +95,13 @@ pub mod pallet {
 			+ MutateMetadata<Self::AccountId>
 			+ Transfer<Self::AccountId>;
 
+		type PoolAssets: Inspect<Self::AccountId, AssetId = Self::AssetId, Balance = Self::AssetBalance>
+			+ Create<Self::AccountId>
+			+ InspectEnumerable<Self::AccountId>
+			+ Mutate<Self::AccountId>
+			+ MutateMetadata<Self::AccountId>
+			+ Transfer<Self::AccountId>;
+
 		/// The dex's pallet id, used for deriving its sovereign account ID.
 		#[pallet::constant]
 		type PalletId: Get<PalletId>;
@@ -192,7 +201,6 @@ pub mod pallet {
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
 		#[pallet::weight(0)]
-		#[transactional]
 		pub fn setup(origin: OriginFor<T>, amount: BalanceOf<T>) -> DispatchResult {
 			let sender = ensure_signed(origin)?;
 			let pallet_account = Self::account_id();
@@ -216,7 +224,6 @@ pub mod pallet {
 		}
 
 		#[pallet::weight(0)]
-		#[transactional]
 		pub fn create_pool(
 			origin: OriginFor<T>,
 			asset1: AssetIdOf<T>, // TODO: convert into MultiToken
@@ -231,8 +238,8 @@ pub mod pallet {
 			ensure!(!Pools::<T>::contains_key(&pool_id), Error::<T>::PoolExists);
 
 			let pallet_account = Self::account_id();
-			T::Assets::create(lp_token, pallet_account.clone(), true, MIN_LIQUIDITY.into())?;
-			T::Assets::set(lp_token, &pallet_account, "LP".into(), "LP".into(), 0)?;
+			T::PoolAssets::create(lp_token, pallet_account.clone(), true, MIN_LIQUIDITY.into())?;
+			T::PoolAssets::set(lp_token, &pallet_account, "LP".into(), "LP".into(), 0)?;
 
 			let pool_info = PoolInfo {
 				owner: sender.clone(),
@@ -251,7 +258,6 @@ pub mod pallet {
 		}
 
 		#[pallet::weight(0)]
-		#[transactional]
 		pub fn add_liquidity(
 			origin: OriginFor<T>,
 			asset1: AssetIdOf<T>,
@@ -311,7 +317,7 @@ pub mod pallet {
 				T::Assets::transfer(asset1, &sender, &pallet_account, amount1, false)?;
 				T::Assets::transfer(asset2, &sender, &pallet_account, amount2, false)?;
 
-				let total_supply = T::Assets::total_issuance(pool.lp_token);
+				let total_supply = T::PoolAssets::total_issuance(pool.lp_token);
 
 				let liquidity: AssetBalanceOf<T>;
 				if total_supply.is_zero() {
@@ -321,7 +327,7 @@ pub mod pallet {
 						.integer_sqrt()
 						.checked_sub(&MIN_LIQUIDITY.into())
 						.ok_or(Error::<T>::Overflow)?;
-					T::Assets::mint_into(pool.lp_token, &pallet_account, MIN_LIQUIDITY.into())?;
+					T::PoolAssets::mint_into(pool.lp_token, &pallet_account, MIN_LIQUIDITY.into())?;
 				} else {
 					let side1 = amount1
 						.checked_mul(&total_supply)
@@ -340,7 +346,7 @@ pub mod pallet {
 
 				ensure!(liquidity > MIN_LIQUIDITY.into(), Error::<T>::InsufficientLiquidityMinted);
 
-				T::Assets::mint_into(pool.lp_token, &mint_to, liquidity)?;
+				T::PoolAssets::mint_into(pool.lp_token, &mint_to, liquidity)?;
 
 				pool.balance1 = reserve1 + amount1;
 				pool.balance2 = reserve2 + amount2;
@@ -360,7 +366,6 @@ pub mod pallet {
 		}
 
 		#[pallet::weight(0)]
-		#[transactional]
 		pub fn remove_liquidity(
 			origin: OriginFor<T>,
 			asset1: AssetIdOf<T>,
@@ -385,12 +390,12 @@ pub mod pallet {
 				let pool = maybe_pool.as_mut().ok_or(Error::<T>::PoolNotFound)?;
 
 				let pallet_account = Self::account_id();
-				T::Assets::transfer(pool.lp_token, &sender, &pallet_account, liquidity, false)?;
+				T::PoolAssets::transfer(pool.lp_token, &sender, &pallet_account, liquidity, false)?;
 
 				let reserve1 = pool.balance1;
 				let reserve2 = pool.balance2;
 
-				let total_supply = T::Assets::total_issuance(pool.lp_token);
+				let total_supply = T::PoolAssets::total_issuance(pool.lp_token);
 
 				let amount1 = liquidity
 					.checked_mul(&reserve1)
@@ -413,7 +418,7 @@ pub mod pallet {
 					Error::<T>::InsufficientAmount
 				);
 
-				T::Assets::burn_from(pool.lp_token, &pallet_account, liquidity)?;
+				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)?;
@@ -436,7 +441,6 @@ pub mod pallet {
 		}
 
 		#[pallet::weight(0)]
-		#[transactional]
 		pub fn swap_exact_tokens_for_tokens(
 			origin: OriginFor<T>,
 			asset1: AssetIdOf<T>,
@@ -500,7 +504,6 @@ pub mod pallet {
 		}
 
 		#[pallet::weight(0)]
-		#[transactional]
 		pub fn swap_tokens_for_exact_tokens(
 			origin: OriginFor<T>,
 			asset1: AssetIdOf<T>,
@@ -630,6 +633,7 @@ 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::<T>::Overflow)?;
 

frame/dex/src/mock.rs

@@ -21,7 +21,9 @@ use super::*;
 use crate as pallet_dex;
 
 use frame_support::{
-	construct_runtime, parameter_types,
+	construct_runtime,
+	instances::{Instance1, Instance2},
+	parameter_types,
 	traits::{ConstU32, ConstU64},
 	PalletId,
 };
@@ -44,7 +46,8 @@ construct_runtime!(
 	{
 		System: frame_system,
 		Balances: pallet_balances,
-		Assets: pallet_assets,
+		Assets: pallet_assets::<Instance1>,
+		PoolAssets:  pallet_assets::<Instance2>,
 		Dex: pallet_dex,
 	}
 );
@@ -88,7 +91,7 @@ impl pallet_balances::Config for Test {
 	type ReserveIdentifier = [u8; 8];
 }
 
-impl pallet_assets::Config for Test {
+impl pallet_assets::Config<Instance1> for Test {
 	type Event = Event;
 	type Balance = u64;
 	type AssetId = u32;
@@ -105,6 +108,24 @@ impl pallet_assets::Config for Test {
 	type Extra = ();
 }
 
+//TODO: limit creation only to dex pallet
+impl pallet_assets::Config<Instance2> for Test {
+	type Event = Event;
+	type Balance = u64;
+	type AssetId = u32;
+	type Currency = Balances;
+	type ForceOrigin = frame_system::EnsureRoot<u64>;
+	type AssetDeposit = ConstU64<0>;
+	type AssetAccountDeposit = ConstU64<0>;
+	type MetadataDepositBase = ConstU64<0>;
+	type MetadataDepositPerByte = ConstU64<0>;
+	type ApprovalDeposit = ConstU64<0>;
+	type StringLimit = ConstU32<50>;
+	type Freezer = ();
+	type WeightInfo = ();
+	type Extra = ();
+}
+
 parameter_types! {
 	pub const DexPalletId: PalletId = PalletId(*b"py/dexer");
 }
@@ -114,6 +135,7 @@ impl Config for Test {
 	type Currency = Balances;
 	type AssetBalance = <Self as pallet_balances::Config>::Balance;
 	type Assets = Assets;
+	type PoolAssets = PoolAssets;
 	type AssetId = u32;
 	type PalletId = DexPalletId;
 }

frame/dex/src/tests.rs

@@ -18,7 +18,7 @@
 use crate::{mock::*, *};
 
 use frame_support::{
-	assert_ok,
+	assert_noop, assert_ok,
 	traits::{fungibles::InspectEnumerable, Currency},
 };
 
@@ -27,7 +27,7 @@ fn events() -> Vec<Event<Test>> {
 		.into_iter()
 		.map(|r| r.event)
 		.filter_map(|e| if let mock::Event::Dex(inner) = e { Some(inner) } else { None })
-		.collect::<Vec<_>>();
+		.collect();
 
 	System::reset_events();
 
@@ -48,6 +48,14 @@ fn assets() -> Vec<u32> {
 	s
 }
 
+fn pool_assets() -> Vec<u32> {
+	// if the storage would be public:
+	// let mut s: Vec<_> = pallet_assets::pallet::PoolAsset::<Test>::iter().map(|x| x.0).collect();
+	let mut s: Vec<_> = <<Test as Config>::PoolAssets>::assets().collect();
+	s.sort();
+	s
+}
+
 fn create_tokens(owner: u64, tokens: Vec<u32>) {
 	for token_id in tokens {
 		assert_ok!(Assets::force_create(Origin::root(), token_id, owner, true, 1));
@@ -63,6 +71,10 @@ fn balance(owner: u64, token_id: u32) -> u64 {
 	<<Test as Config>::Assets>::balance(token_id, owner)
 }
 
+fn pool_balance(owner: u64, token_id: u32) -> u64 {
+	<<Test as Config>::PoolAssets>::balance(token_id, owner)
+}
+
 #[test]
 fn create_pool_should_work() {
 	new_test_ext().execute_with(|| {
@@ -79,7 +91,8 @@ fn create_pool_should_work() {
 
 		assert_eq!(events(), [Event::<Test>::PoolCreated { creator: user, pool_id, lp_token }]);
 		assert_eq!(pools(), vec![pool_id]);
-		assert_eq!(assets(), vec![token_1, token_2, lp_token]);
+		assert_eq!(assets(), vec![token_1, token_2]);
+		assert_eq!(pool_assets(), vec![lp_token]);
 	});
 }
 
@@ -124,7 +137,7 @@ fn add_liquidity_should_work() {
 		let pallet_account = Dex::account_id();
 		assert_eq!(balance(pallet_account, token_1), 10);
 		assert_eq!(balance(pallet_account, token_2), 10);
-		assert_eq!(balance(user, lp_token), 9);
+		assert_eq!(pool_balance(user, lp_token), 9);
 	});
 }
 
@@ -171,11 +184,11 @@ fn remove_liquidity_should_work() {
 		let pallet_account = Dex::account_id();
 		assert_eq!(balance(pallet_account, token_1), 1);
 		assert_eq!(balance(pallet_account, token_2), 1);
-		assert_eq!(balance(pallet_account, lp_token), 1);
+		assert_eq!(pool_balance(pallet_account, lp_token), 1);
 
 		assert_eq!(balance(user, token_1), 999);
 		assert_eq!(balance(user, token_2), 999);
-		assert_eq!(balance(user, lp_token), 0);
+		assert_eq!(pool_balance(user, lp_token), 0);
 	});
 }
 
@@ -260,3 +273,52 @@ fn swap_should_work() {
 		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;
+		let lp_token = 3;
+		topup_pallet();
+
+		create_tokens(user, vec![token_1]);
+		assert_noop!(
+			Dex::create_pool(Origin::signed(user), token_1, token_1, lp_token),
+			Error::<Test>::EqualAssets
+		);
+
+		assert_ok!(Assets::mint(Origin::signed(user), token_1, user, 1000));
+
+		let liquidity1 = 1000;
+		let liquidity2 = 20;
+		assert_noop!(
+			Dex::add_liquidity(
+				Origin::signed(user),
+				token_1,
+				token_1,
+				liquidity1,
+				liquidity2,
+				1,
+				1,
+				user,
+				2
+			),
+			Error::<Test>::PoolNotFound
+		);
+
+		let exchange_amount = 10;
+		assert_noop!(
+			Dex::swap_exact_tokens_for_tokens(
+				Origin::signed(user),
+				token_1,
+				token_1,
+				exchange_amount,
+				1,
+				user,
+				3
+			),
+			Error::<Test>::PoolNotFound
+		);
+	});
+}

frame/dex/src/types.rs

@@ -28,8 +28,8 @@ pub struct PoolInfo<AccountId, AssetId, Balance> {
 	pub asset1: AssetId,
 	/// The second asset supported by the pool
 	pub asset2: AssetId,
-	/// Pool balance1
+	/// Pool balance of asset1
 	pub balance1: Balance,
-	/// Pool balance2
+	/// Pool balance of asset2
 	pub balance2: Balance,
 }