Merge pull request #1053 from gregdhill/refactor/dex

refactor: remove AssetInfo, separate bootstrap account, cleanup errors

crates/dex-general/src/fee/mock.rs

@@ -10,15 +10,15 @@ use serde::{Deserialize, Serialize};
 
 use frame_support::{parameter_types, traits::Contains, PalletId};
 use orml_traits::parameter_type_with_key;
-use sp_core::{ConstU16, ConstU32, H256};
+use sp_core::{ConstU32, H256};
 use sp_runtime::{
     testing::Header,
     traits::{BlakeTwo256, IdentityLookup},
     RuntimeDebug,
 };
 
 use crate as pallet_dex_general;
-pub use crate::{AssetBalance, AssetInfo, Config, GenerateLpAssetId, Pallet};
+pub use crate::{AssetBalance, Config, GenerateLpAssetId, Pallet, ValidateAsset};
 
 type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Test>;
 type Block = frame_system::mocking::MockBlock<Test>;
@@ -44,10 +44,12 @@ impl CurrencyId {
     }
 }
 
-impl AssetInfo for CurrencyId {
-    fn is_support(&self) -> bool {
-        match self {
-            Self::Token(_) => true,
+pub struct EnsurePairAssetImpl;
+
+impl ValidateAsset<CurrencyId> for EnsurePairAssetImpl {
+    fn validate_asset(currency_id: &CurrencyId) -> bool {
+        match currency_id {
+            CurrencyId::Token(_) => true,
             _ => false,
         }
     }
@@ -152,9 +154,9 @@ impl Config for Test {
     type MultiCurrency = Tokens;
     type PalletId = DexGeneralPalletId;
     type AssetId = CurrencyId;
+    type EnsurePairAsset = EnsurePairAssetImpl;
     type LpGenerate = PairLpIdentity;
     type WeightInfo = ();
-    type MaxSwaps = ConstU16<10>;
     type MaxBootstrapRewards = ConstU32<1000>;
     type MaxBootstrapLimits = ConstU32<1000>;
 }

crates/dex-general/src/lib.rs

@@ -42,12 +42,12 @@ mod default_weights;
 
 pub use default_weights::WeightInfo;
 pub use primitives::{
-    AssetBalance, AssetInfo, BootstrapParameter, PairMetadata, PairStatus,
+    AssetBalance, BootstrapParameter, PairMetadata, PairStatus,
     PairStatus::{Bootstrap, Disable, Trading},
     DEFAULT_FEE_RATE, FEE_ADJUSTMENT,
 };
 pub use rpc::PairInfo;
-pub use traits::{ExportDexGeneral, GenerateLpAssetId};
+pub use traits::{ExportDexGeneral, GenerateLpAssetId, ValidateAsset};
 
 #[allow(type_alias_bounds)]
 type AccountIdOf<T: Config> = <T as frame_system::Config>::AccountId;
@@ -63,11 +63,14 @@ pub mod pallet {
     pub trait Config: frame_system::Config {
         /// Because this pallet emits events, it depends on the runtime's definition of an event.
         type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;
+
         /// The trait control all currencies
         type MultiCurrency: MultiCurrency<AccountIdOf<Self>, CurrencyId = Self::AssetId, Balance = AssetBalance>;
+
         /// This pallet id.
         #[pallet::constant]
         type PalletId: Get<PalletId>;
+
         /// The asset type.
         type AssetId: FullCodec
             + Eq
@@ -76,17 +79,18 @@ pub mod pallet {
             + PartialOrd
             + Copy
             + MaybeSerializeDeserialize
-            + AssetInfo
             + Debug
             + scale_info::TypeInfo
             + MaxEncodedLen;
+
+        /// Verify the asset can be used in a pair.
+        type EnsurePairAsset: ValidateAsset<Self::AssetId>;
+
         /// Generate the AssetId for the pair.
         type LpGenerate: GenerateLpAssetId<Self::AssetId>;
+
         /// Weight information for extrinsics in this pallet.
         type WeightInfo: WeightInfo;
-        /// The maximum number of swaps allowed in routes
-        #[pallet::constant]
-        type MaxSwaps: Get<u16>;
 
         /// The maximum number of rewards that can be stored
         #[pallet::constant]
@@ -354,18 +358,12 @@ pub mod pallet {
     }
     #[pallet::error]
     pub enum Error<T> {
-        /// Require the admin who can reset the admin and receiver of the protocol fee.
-        RequireProtocolAdmin,
-        /// Require the admin candidate who can become new admin after confirm.
-        RequireProtocolAdminCandidate,
         /// Invalid fee_rate
         InvalidFeeRate,
         /// Unsupported AssetId.
         UnsupportedAssetType,
         /// Account balance must be greater than or equal to the transfer amount.
         InsufficientAssetBalance,
-        /// Account native currency balance must be greater than ExistentialDeposit.
-        NativeBalanceTooLow,
         /// Trading pair can't be created.
         DeniedCreatePair,
         /// Trading pair already exists.
@@ -390,18 +388,8 @@ pub mod pallet {
         Overflow,
         /// Transaction block number is larger than the end block number.
         Deadline,
-        /// Location given was invalid or unsupported.
-        AccountIdBadLocation,
-        /// XCM execution failed.
-        ExecutionFailed,
-        /// Transfer to self by XCM message.
-        DeniedTransferToSelf,
-        /// Not in registered parachains.
-        TargetChainNotRegistered,
         /// Can't pass the K value check
         InvariantCheckFailed,
-        /// Created pair can't create now
-        PairCreateForbidden,
         /// Pair is not in bootstrap
         NotInBootstrap,
         /// Amount of contribution is invalid.
@@ -555,7 +543,7 @@ pub mod pallet {
         ) -> DispatchResult {
             ensure_root(origin)?;
             ensure!(
-                asset_0.is_support() && asset_1.is_support(),
+                T::EnsurePairAsset::validate_asset(&asset_0) && T::EnsurePairAsset::validate_asset(&asset_1),
                 Error::<T>::UnsupportedAssetType
             );
 
@@ -627,7 +615,7 @@ pub mod pallet {
             #[pallet::compact] deadline: T::BlockNumber,
         ) -> DispatchResult {
             ensure!(
-                asset_0.is_support() && asset_1.is_support(),
+                T::EnsurePairAsset::validate_asset(&asset_0) && T::EnsurePairAsset::validate_asset(&asset_1),
                 Error::<T>::UnsupportedAssetType
             );
             let who = ensure_signed(origin)?;
@@ -672,7 +660,7 @@ pub mod pallet {
             #[pallet::compact] deadline: T::BlockNumber,
         ) -> DispatchResult {
             ensure!(
-                asset_0.is_support() && asset_1.is_support(),
+                T::EnsurePairAsset::validate_asset(&asset_0) && T::EnsurePairAsset::validate_asset(&asset_1),
                 Error::<T>::UnsupportedAssetType
             );
             let who = ensure_signed(origin)?;
@@ -711,12 +699,14 @@ pub mod pallet {
             recipient: <T::Lookup as StaticLookup>::Source,
             #[pallet::compact] deadline: T::BlockNumber,
         ) -> DispatchResult {
-            ensure!(path.iter().all(|id| id.is_support()), Error::<T>::UnsupportedAssetType);
-
             let who = ensure_signed(origin)?;
             let recipient = T::Lookup::lookup(recipient)?;
             let now = frame_system::Pallet::<T>::block_number();
             ensure!(deadline > now, Error::<T>::Deadline);
+            ensure!(
+                path.iter().all(|id| T::EnsurePairAsset::validate_asset(&id)),
+                Error::<T>::UnsupportedAssetType
+            );
 
             Self::inner_swap_exact_assets_for_assets(&who, amount_in, amount_out_min, &path, &recipient)
         }
@@ -741,7 +731,10 @@ pub mod pallet {
             recipient: <T::Lookup as StaticLookup>::Source,
             #[pallet::compact] deadline: T::BlockNumber,
         ) -> DispatchResult {
-            ensure!(path.iter().all(|id| id.is_support()), Error::<T>::UnsupportedAssetType);
+            ensure!(
+                path.iter().all(|id| T::EnsurePairAsset::validate_asset(&id)),
+                Error::<T>::UnsupportedAssetType
+            );
 
             let who = ensure_signed(origin)?;
             let recipient = T::Lookup::lookup(recipient)?;
@@ -799,7 +792,7 @@ pub mod pallet {
                             capacity_supply: (capacity_supply_0, capacity_supply_1),
                             accumulated_supply: params.accumulated_supply,
                             end_block_number: end,
-                            pair_account: Self::account_id(),
+                            pair_account: Self::bootstrap_account_id(),
                         });
 
                         // must no reward before update.
@@ -840,7 +833,7 @@ pub mod pallet {
                         capacity_supply: (capacity_supply_0, capacity_supply_1),
                         accumulated_supply: (Zero::zero(), Zero::zero()),
                         end_block_number: end,
-                        pair_account: Self::account_id(),
+                        pair_account: Self::bootstrap_account_id(),
                     });
 
                     BootstrapRewards::<T>::insert(
@@ -867,7 +860,7 @@ pub mod pallet {
             })?;
 
             Self::deposit_event(Event::BootstrapCreated {
-                bootstrap_pair_account: Self::account_id(),
+                bootstrap_pair_account: Self::bootstrap_account_id(),
                 asset_0: pair.0,
                 asset_1: pair.1,
                 total_supply_0: target_supply_0,
@@ -1001,7 +994,7 @@ pub mod pallet {
                         capacity_supply: (capacity_supply_0, capacity_supply_1),
                         accumulated_supply: params.accumulated_supply,
                         end_block_number: end,
-                        pair_account: Self::account_id(),
+                        pair_account: Self::bootstrap_account_id(),
                     });
 
                     // must no reward before update.
@@ -1084,7 +1077,7 @@ pub mod pallet {
                 for (asset_id, amount) in &charge_rewards {
                     let already_charge_amount = rewards.get(asset_id).ok_or(Error::<T>::NoRewardTokens)?;
 
-                    T::MultiCurrency::transfer(*asset_id, &who, &Self::account_id(), *amount)?;
+                    T::MultiCurrency::transfer(*asset_id, &who, &Self::bootstrap_account_id(), *amount)?;
                     let new_charge_amount = already_charge_amount.checked_add(*amount).ok_or(Error::<T>::Overflow)?;
 
                     rewards
@@ -1120,7 +1113,7 @@ pub mod pallet {
 
             BootstrapRewards::<T>::try_mutate(pair, |rewards| -> DispatchResult {
                 for (asset_id, amount) in rewards {
-                    T::MultiCurrency::transfer(*asset_id, &Self::account_id(), &recipient, *amount)?;
+                    T::MultiCurrency::transfer(*asset_id, &Self::bootstrap_account_id(), &recipient, *amount)?;
 
                     *amount = Zero::zero();
                 }

crates/dex-general/src/primitives.rs

@@ -10,10 +10,6 @@ pub type AssetBalance = u128;
 pub const DEFAULT_FEE_RATE: u128 = 30;
 pub const FEE_ADJUSTMENT: u128 = 10000;
 
-pub trait AssetInfo {
-    fn is_support(&self) -> bool;
-}
-
 /// Status for TradingPair
 #[derive(Clone, Copy, Encode, Decode, RuntimeDebug, PartialEq, Eq, MaxEncodedLen, TypeInfo)]
 pub enum PairStatus<Balance, BlockNumber, Account> {

crates/dex-general/src/swap/mock.rs

@@ -9,15 +9,15 @@ use serde::{Deserialize, Serialize};
 
 use frame_support::{parameter_types, traits::Contains, PalletId};
 use orml_traits::parameter_type_with_key;
-use sp_core::{ConstU16, ConstU32, H256};
+use sp_core::{ConstU32, H256};
 use sp_runtime::{
     testing::Header,
     traits::{BlakeTwo256, IdentityLookup},
     RuntimeDebug,
 };
 
 use crate as pallet_dex_general;
-pub use crate::{AssetBalance, AssetInfo, Config, GenerateLpAssetId, Pallet};
+pub use crate::{AssetBalance, Config, GenerateLpAssetId, Pallet, ValidateAsset};
 
 type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Test>;
 type Block = frame_system::mocking::MockBlock<Test>;
@@ -43,10 +43,12 @@ impl CurrencyId {
     }
 }
 
-impl AssetInfo for CurrencyId {
-    fn is_support(&self) -> bool {
-        match self {
-            Self::Token(_) => true,
+pub struct EnsurePairAssetImpl;
+
+impl ValidateAsset<CurrencyId> for EnsurePairAssetImpl {
+    fn validate_asset(currency_id: &CurrencyId) -> bool {
+        match currency_id {
+            CurrencyId::Token(_) => true,
             _ => false,
         }
     }
@@ -139,9 +141,9 @@ impl Config for Test {
     type MultiCurrency = Tokens;
     type PalletId = DexGeneralPalletId;
     type AssetId = CurrencyId;
+    type EnsurePairAsset = EnsurePairAssetImpl;
     type LpGenerate = PairLpIdentity;
     type WeightInfo = ();
-    type MaxSwaps = ConstU16<10>;
     type MaxBootstrapRewards = ConstU32<1000>;
     type MaxBootstrapLimits = ConstU32<1000>;
 }

crates/dex-general/src/swap/mod.rs

@@ -16,9 +16,12 @@ mod mock;
 mod tests;
 
 impl<T: Config> Pallet<T> {
-    pub(crate) fn account_id() -> T::AccountId {
-        T::PalletId::get().into_account_truncating()
+    pub(crate) fn bootstrap_account_id() -> T::AccountId {
+        // only use two byte prefix to support 16 byte account id (used by test)
+        // "modl" ++ "dex/genr" ++ "boot" is 16 bytes
+        T::PalletId::get().into_sub_account_truncating("boot")
     }
+
     /// The account ID of a pair account
     /// only use two byte prefix to support 16 byte account id (used by test)
     /// "modl" ++ "dex/genr" is 12 bytes, and 4 bytes remaining for hash of AssetId pair.
@@ -655,7 +658,7 @@ impl<T: Config> Pallet<T> {
                 .checked_add(amount_1_contribute)
                 .ok_or(Error::<T>::Overflow)?;
 
-            let pair_account = Self::account_id();
+            let pair_account = Self::bootstrap_account_id();
 
             T::MultiCurrency::transfer(pair.0, &who, &pair_account, amount_0_contribute)?;
             T::MultiCurrency::transfer(pair.1, &who, &pair_account, amount_1_contribute)?;
@@ -872,7 +875,7 @@ impl<T: Config> Pallet<T> {
 
         BootstrapPersonalSupply::<T>::try_mutate_exists((pair, &who), |contribution| -> DispatchResult {
             if let Some((amount_0_contribute, amount_1_contribute)) = contribution.take() {
-                let pair_account = Self::account_id();
+                let pair_account = Self::bootstrap_account_id();
                 T::MultiCurrency::transfer(pair.0, &pair_account, &who, amount_0_contribute)?;
                 T::MultiCurrency::transfer(pair.1, &pair_account, &who, amount_1_contribute)?;