Pool creation should not allow duplicate assets. (#666)

* Disallow pool creation with duplicate assets.x
This fixes #497

* In create_pool() clone assets vector before sorting to be used for SomeIdenticalAssets check. Also in test for the same fund account and pass correct lenght vector for weights parameter.

* Use sorted_assets in Pool struct

zrml/swaps/src/lib.rs

@@ -807,6 +807,8 @@ mod pallet {
         /// Tried to create a pool that has more assets than the upper threshhold specified by
         /// a constant.
         TooManyAssets,
+        /// Tried to create a pool with at least two identical assets.
+        SomeIdenticalAssets,
         /// The pool does not support swapping the assets in question.
         UnsupportedTrade,
         /// The outcome asset specified as the winning asset was not found in the pool.
@@ -1343,7 +1345,7 @@ mod pallet {
         #[frame_support::transactional]
         fn create_pool(
             who: T::AccountId,
-            mut assets: Vec<Asset<T::MarketId>>,
+            assets: Vec<Asset<T::MarketId>>,
             base_asset: Asset<T::MarketId>,
             market_id: Self::MarketId,
             scoring_rule: ScoringRule,
@@ -1360,6 +1362,13 @@ mod pallet {
             let mut map = BTreeMap::new();
             let mut total_weight = 0;
             let amount_unwrapped = amount.unwrap_or_else(BalanceOf::<T>::zero);
+            let mut sorted_assets = assets.clone();
+            sorted_assets.sort();
+            let has_duplicates = sorted_assets
+                .iter()
+                .zip(sorted_assets.iter().skip(1))
+                .fold(false, |acc, (&x, &y)| acc || x == y);
+            ensure!(!has_duplicates, Error::<T>::SomeIdenticalAssets);
 
             if scoring_rule == ScoringRule::CPMM {
                 ensure!(amount.is_some(), Error::<T>::InvalidAmountArgument);
@@ -1396,11 +1405,8 @@ mod pallet {
                 let _ = T::RikiddoSigmoidFeeMarketEma::create(next_pool_id, rikiddo_instance)?;
             }
 
-            // Sort assets for future binary search, for example to check if an asset is included.
-            let sort_assets = assets.as_mut_slice();
-            sort_assets.sort();
             let pool = Pool {
-                assets,
+                assets: sorted_assets,
                 base_asset,
                 market_id,
                 pool_status: if scoring_rule == ScoringRule::CPMM {

zrml/swaps/src/tests.rs

@@ -51,6 +51,32 @@ const _105: u128 = 105 * BASE;
 const _1234: u128 = 1234 * BASE;
 const _10000: u128 = 10000 * BASE;
 
+#[test_case(vec![ASSET_A, ASSET_A]; "short vector")]
+#[test_case(vec![ASSET_A, ASSET_B, ASSET_C, ASSET_D, ASSET_E, ASSET_A]; "start and end")]
+#[test_case(vec![ASSET_A, ASSET_B, ASSET_C, ASSET_D, ASSET_E, ASSET_E]; "successive at end")]
+#[test_case(vec![ASSET_A, ASSET_B, ASSET_C, ASSET_A, ASSET_E, ASSET_D]; "start and middle")]
+fn create_pool_fails_with_duplicate_assets(assets: Vec<Asset<<Runtime as Config>::MarketId>>) {
+    ExtBuilder::default().build().execute_with(|| {
+        assets.iter().cloned().for_each(|asset| {
+            let _ = Currencies::deposit(asset, &BOB, _10000);
+        });
+        let asset_count = assets.len();
+        assert_noop!(
+            Swaps::create_pool(
+                BOB,
+                assets,
+                ASSET_A,
+                0,
+                ScoringRule::CPMM,
+                Some(0),
+                Some(<Runtime as crate::Config>::MinLiquidity::get()),
+                Some(vec![_2; asset_count]),
+            ),
+            crate::Error::<Runtime>::SomeIdenticalAssets
+        );
+    });
+}
+
 #[test]
 fn destroy_pool_fails_if_pool_does_not_exist() {
     ExtBuilder::default().build().execute_with(|| {