Refine combinatorial betting (#1372)

* Add numerical thresholds to combinatorial betting

* Add protected `exp` for normal bets

* Ensure correctness of partitions

* Check partitions

primitives/src/constants/base_multiples.rs

@@ -60,6 +60,8 @@ pub const _1_5: u128 = _1 / 5;
 pub const _1_6: u128 = _1 / 6;
 pub const _5_6: u128 = _5 / 6;
 
+pub const _1_7: u128 = _1 / 7;
+
 pub const _1_10: u128 = _1 / 10;
 pub const _2_10: u128 = _2 / 10;
 pub const _3_10: u128 = _3 / 10;

zrml/neo-swaps/src/lib.rs

@@ -46,7 +46,11 @@ mod pallet {
         types::{FeeDistribution, MaxAssets, Pool},
         weights::*,
     };
-    use alloc::{collections::BTreeMap, vec, vec::Vec};
+    use alloc::{
+        collections::{BTreeMap, BTreeSet},
+        vec,
+        vec::Vec,
+    };
     use core::marker::PhantomData;
     use frame_support::{
         dispatch::DispatchResultWithPostInfo,
@@ -297,6 +301,10 @@ mod pallet {
         MinRelativeLiquidityThresholdViolated,
         /// Narrowing type conversion occurred.
         NarrowingConversion,
+
+        /// The buy/sell/keep partition specified is empty, or contains overlaps or assets that don't
+        /// belong to the market.
+        InvalidPartition,
     }
 
     #[derive(Decode, Encode, Eq, PartialEq, PalletError, RuntimeDebug, TypeInfo)]
@@ -1027,13 +1035,21 @@ mod pallet {
             let market = T::MarketCommons::market(&market_id)?;
             ensure!(market.status == MarketStatus::Active, Error::<T>::MarketNotActive);
             Self::try_mutate_pool(&market_id, |pool| {
-                for asset in buy.iter().chain(sell.iter()) {
-                    ensure!(pool.contains(asset), Error::<T>::AssetNotFound);
+                // Ensure that `buy` and `sell` partition are disjoint, only contain assets from
+                // the market and don't contain dupliates.
+                ensure!(!buy.is_empty(), Error::<T>::InvalidPartition);
+                ensure!(!sell.is_empty(), Error::<T>::InvalidPartition);
+                for asset in buy.iter() {
+                    ensure!(!sell.contains(asset), Error::<T>::InvalidPartition);
+                    ensure!(market.outcome_assets().contains(asset), Error::<T>::InvalidPartition);
                 }
-
-                // TODO Ensure that buy, sell partition the assets!
-
-                // TODO Ensure that numerical limits are observed.
+                for asset in sell.iter() {
+                    ensure!(market.outcome_assets().contains(asset), Error::<T>::InvalidPartition);
+                }
+                let buy_set = buy.iter().collect::<BTreeSet<_>>();
+                let sell_set = sell.iter().collect::<BTreeSet<_>>();
+                ensure!(buy_set.len() == buy.len(), Error::<T>::InvalidPartition);
+                ensure!(sell_set.len() == sell.len(), Error::<T>::InvalidPartition);
 
                 let FeeDistribution {
                     remaining: amount_in_minus_fees,
@@ -1100,13 +1116,30 @@ mod pallet {
             let market = T::MarketCommons::market(&market_id)?;
             ensure!(market.status == MarketStatus::Active, Error::<T>::MarketNotActive);
             Self::try_mutate_pool(&market_id, |pool| {
-                for asset in buy.iter().chain(sell.iter()).chain(keep.iter()) {
-                    ensure!(pool.contains(asset), Error::<T>::AssetNotFound);
+                // Ensure that `buy` and `sell` partition are disjoint and only contain assets from
+                // the market.
+                ensure!(!buy.is_empty(), Error::<T>::InvalidPartition);
+                ensure!(!sell.is_empty(), Error::<T>::InvalidPartition);
+                for asset in buy.iter() {
+                    ensure!(!keep.contains(asset), Error::<T>::InvalidPartition);
+                    ensure!(!sell.contains(asset), Error::<T>::InvalidPartition);
+                    ensure!(market.outcome_assets().contains(asset), Error::<T>::InvalidPartition);
                 }
-
-                // TODO Ensure that buy, sell partition the assets!
-
-                // TODO Ensure that numerical limits are observed.
+                for asset in sell.iter() {
+                    ensure!(!keep.contains(asset), Error::<T>::InvalidPartition);
+                    ensure!(market.outcome_assets().contains(asset), Error::<T>::InvalidPartition);
+                }
+                for asset in keep.iter() {
+                    ensure!(market.outcome_assets().contains(asset), Error::<T>::InvalidPartition);
+                }
+                let buy_set = buy.iter().collect::<BTreeSet<_>>();
+                let keep_set = keep.iter().collect::<BTreeSet<_>>();
+                let sell_set = sell.iter().collect::<BTreeSet<_>>();
+                ensure!(buy_set.len() == buy.len(), Error::<T>::InvalidPartition);
+                ensure!(keep_set.len() == keep.len(), Error::<T>::InvalidPartition);
+                ensure!(sell_set.len() == sell.len(), Error::<T>::InvalidPartition);
+                let total_assets = buy.len().saturating_add(keep.len()).saturating_add(sell.len());
+                ensure!(total_assets == market.outcomes() as usize, Error::<T>::InvalidPartition);
 
                 // This is the amount of collateral the user will receive in the end, or,
                 // equivalently, the amount of each asset in `sell` that the user intermittently

zrml/neo-swaps/src/math/types/combo_math.rs

@@ -31,9 +31,8 @@ use typenum::U80;
 type Fractional = U80;
 type FixedType = FixedU128<Fractional>;
 
-/// The point at which 32.44892769177272
-#[allow(dead_code)] // TODO Block calls that go outside of these bounds.
-const EXP_OVERFLOW_THRESHOLD: FixedType = FixedType::from_bits(0x20_72EC_ECDA_6EBE_EACC_40C7);
+/// The point at which `exp` values become too large, 32.44892769177272.
+const EXP_NUMERICAL_THRESHOLD: FixedType = FixedType::from_bits(0x20_72EC_ECDA_6EBE_EACC_40C7);
 
 pub(crate) struct ComboMath<T>(PhantomData<T>);
 
@@ -134,12 +133,18 @@ mod detail {
         value.to_fixed_decimal(DECIMALS).ok()
     }
 
+    /// Calculates `exp(value)` but returns `None` if `value` lies outside of the numerical
+    /// boundaries.
+    fn protected_exp(value: FixedType, neg: bool) -> Option<FixedType> {
+        if value < EXP_NUMERICAL_THRESHOLD { exp(value, neg).ok() } else { None }
+    }
+
     /// Returns `\sum_{r \in R} e^{-r/b}`, where `R` denotes `reserves` and `b` denotes `liquidity`.
-    /// The result is `None` if and only if one of the `exp` calculations has failed.
+    /// The result is `None` if and only if any of the `exp` calculations has failed.
     fn exp_sum(reserves: Vec<FixedType>, liquidity: FixedType) -> Option<FixedType> {
         reserves
             .iter()
-            .map(|r| exp(r.checked_div(liquidity)?, true).ok())
+            .map(|r| protected_exp(r.checked_div(liquidity)?, true))
             .collect::<Option<Vec<_>>>()?
             .iter()
             .try_fold(FixedType::zero(), |acc, &val| acc.checked_add(val))
@@ -222,7 +227,7 @@ mod detail {
         let exp_sum_buy = exp_sum(buy, liquidity)?;
         let exp_sum_sell = exp_sum(sell, liquidity)?;
         let amount_in_div_liquidity = amount_in.checked_div(liquidity)?;
-        let exp_of_minus_amount_in: FixedType = exp(amount_in_div_liquidity, true).ok()?;
+        let exp_of_minus_amount_in: FixedType = protected_exp(amount_in_div_liquidity, true)?;
         let exp_of_minus_amount_in_times_exp_sum_sell =
             exp_of_minus_amount_in.checked_mul(exp_sum_sell)?;
         let numerator = exp_sum_buy
@@ -249,7 +254,7 @@ mod detail {
         let numerator = exp_sum_buy.checked_add(exp_sum_sell)?;
         let delta = amount_buy.checked_sub(amount_sell)?;
         let delta_div_liquidity = delta.checked_div(liquidity)?;
-        let exp_delta: FixedType = exp(delta_div_liquidity, false).ok()?;
+        let exp_delta: FixedType = protected_exp(delta_div_liquidity, false)?;
         let exp_delta_times_exp_sum_sell = exp_delta.checked_mul(exp_sum_sell)?;
         let denominator = exp_sum_buy.checked_add(exp_delta_times_exp_sum_sell)?;
         let ln_arg = numerator.checked_div(denominator)?;

zrml/neo-swaps/src/math/types/math.rs

@@ -35,7 +35,7 @@ type Fractional = U80;
 type FixedType = FixedU128<Fractional>;
 
 // 32.44892769177272
-const EXP_OVERFLOW_THRESHOLD: FixedType = FixedType::from_bits(0x20_72EC_ECDA_6EBE_EACC_40C7);
+const EXP_NUMERICAL_THRESHOLD: FixedType = FixedType::from_bits(0x20_72EC_ECDA_6EBE_EACC_40C7);
 
 pub(crate) struct Math<T>(PhantomData<T>);
 
@@ -242,6 +242,12 @@ mod detail {
         value.to_fixed_decimal(DECIMALS).ok()
     }
 
+    /// Calculates `exp(value)` but returns `None` if `value` lies outside of the numerical
+    /// boundaries.
+    fn protected_exp(value: FixedType, neg: bool) -> Option<FixedType> {
+        if value < EXP_NUMERICAL_THRESHOLD { exp(value, neg).ok() } else { None }
+    }
+
     fn calculate_swap_amount_out_for_buy_fixed(
         reserve: FixedType,
         amount_in: FixedType,
@@ -264,8 +270,8 @@ mod detail {
             // Ensure that if the reserve is zero, we don't accidentally return a non-zero value.
             return None;
         }
-        let exp_neg_x_over_b: FixedType = exp(amount_in.checked_div(liquidity)?, true).ok()?;
-        let exp_r_over_b = exp(reserve.checked_div(liquidity)?, false).ok()?;
+        let exp_neg_x_over_b: FixedType = protected_exp(amount_in.checked_div(liquidity)?, true)?;
+        let exp_r_over_b = protected_exp(reserve.checked_div(liquidity)?, false)?;
         let inside_ln = exp_neg_x_over_b
             .checked_add(exp_r_over_b)?
             .checked_sub(FixedType::checked_from_num(1)?)?;
@@ -278,7 +284,7 @@ mod detail {
         reserve: FixedType,
         liquidity: FixedType,
     ) -> Option<FixedType> {
-        exp(reserve.checked_div(liquidity)?, true).ok()
+        protected_exp(reserve.checked_div(liquidity)?, true)
     }
 
     fn calculate_reserve_from_spot_prices_fixed(
@@ -308,10 +314,10 @@ mod detail {
         amount_in: FixedType,
         liquidity: FixedType,
     ) -> Option<FixedType> {
-        let exp_x_over_b: FixedType = exp(amount_in.checked_div(liquidity)?, false).ok()?;
+        let exp_x_over_b: FixedType = protected_exp(amount_in.checked_div(liquidity)?, false)?;
         let r_over_b = reserve.checked_div(liquidity)?;
-        let exp_neg_r_over_b = if r_over_b < EXP_OVERFLOW_THRESHOLD {
-            exp(reserve.checked_div(liquidity)?, true).ok()?
+        let exp_neg_r_over_b = if r_over_b < EXP_NUMERICAL_THRESHOLD {
+            protected_exp(reserve.checked_div(liquidity)?, true)?
         } else {
             FixedType::checked_from_num(0)? // Underflow to zero.
         };
@@ -573,7 +579,7 @@ mod tests {
     #[test_case(true, FixedType::from_str("0.000000000000008083692034").unwrap())]
     fn exp_does_not_overflow_or_underflow(neg: bool, expected: FixedType) {
         let result: FixedType =
-            exp(FixedType::checked_from_num(EXP_OVERFLOW_THRESHOLD).unwrap(), neg).unwrap();
+            exp(FixedType::checked_from_num(EXP_NUMERICAL_THRESHOLD).unwrap(), neg).unwrap();
         assert_eq!(result, expected);
     }
 

zrml/neo-swaps/src/tests/combo_buy.rs

@@ -19,6 +19,7 @@ use super::*;
 #[cfg(not(feature = "parachain"))]
 use sp_runtime::{DispatchError, TokenError};
 use test_case::test_case;
+use zeitgeist_primitives::types::Asset::CategoricalOutcome;
 
 // Example taken from
 // https://docs.gnosis.io/conditionaltokens/docs/introduction3/#an-example-with-lmsr
@@ -210,33 +211,6 @@ fn combo_buy_fails_on_pool_not_found() {
     });
 }
 
-#[test_case(MarketType::Categorical(2))]
-#[test_case(MarketType::Scalar(0..=1))]
-fn combo_buy_fails_on_asset_not_found(market_type: MarketType) {
-    ExtBuilder::default().build().execute_with(|| {
-        let market_id = create_market_and_deploy_pool(
-            ALICE,
-            BASE_ASSET,
-            market_type,
-            _10,
-            vec![_1_2, _1_2],
-            CENT,
-        );
-        assert_noop!(
-            NeoSwaps::combo_buy(
-                RuntimeOrigin::signed(BOB),
-                market_id,
-                2,
-                vec![Asset::CategoricalOutcome(market_id, 2)],
-                vec![Asset::CategoricalOutcome(market_id, 1)],
-                _1,
-                0
-            ),
-            Error::<Runtime>::AssetNotFound,
-        );
-    });
-}
-
 #[test]
 fn combo_buy_fails_on_insufficient_funds() {
     ExtBuilder::default().build().execute_with(|| {
@@ -293,3 +267,35 @@ fn combo_buy_fails_on_amount_out_below_min() {
         );
     });
 }
+
+#[test_case(vec![0], vec![0]; "overlap")]
+#[test_case(vec![], vec![0, 1]; "empty_buy")]
+#[test_case(vec![2, 3], vec![]; "empty_sell")]
+#[test_case(vec![0, 2, 3], vec![1, 3, 4]; "overlap2")]
+#[test_case(vec![0, 1, 3, 1], vec![2]; "duplicate_buy")]
+#[test_case(vec![0, 1, 3], vec![4, 2, 4]; "duplicate_sell")]
+#[test_case(vec![999], vec![0, 1, 2, 3, 4]; "out_of_bounds_buy")]
+#[test_case(vec![0, 1, 3], vec![999]; "out_of_bounds_sell")]
+fn combo_buy_fails_on_invalid_partition(indices_buy: Vec<u16>, indices_sell: Vec<u16>) {
+    ExtBuilder::default().build().execute_with(|| {
+        let market_id = create_market_and_deploy_pool(
+            ALICE,
+            BASE_ASSET,
+            MarketType::Categorical(5),
+            _10,
+            vec![_1_5, _1_5, _1_5, _1_5, _1_5],
+            CENT,
+        );
+        let amount_in = _1;
+        assert_ok!(AssetManager::deposit(BASE_ASSET, &BOB, amount_in));
+
+        let buy = indices_buy.into_iter().map(|i| CategoricalOutcome(market_id, i)).collect();
+        let sell = indices_sell.into_iter().map(|i| CategoricalOutcome(market_id, i)).collect();
+
+        // Buying 1 at price of .5 will return less than 2 outcomes due to slippage.
+        assert_noop!(
+            NeoSwaps::combo_buy(RuntimeOrigin::signed(BOB), market_id, 5, buy, sell, amount_in, 0),
+            Error::<Runtime>::InvalidPartition,
+        );
+    });
+}

zrml/neo-swaps/src/tests/combo_sell.rs

@@ -17,6 +17,7 @@
 
 use super::*;
 use test_case::test_case;
+use zeitgeist_primitives::types::Asset::CategoricalOutcome;
 
 #[test]
 fn combo_sell_works() {
@@ -215,36 +216,6 @@ fn combo_sell_fails_on_pool_not_found() {
     });
 }
 
-// TODO Needs to be expanded.
-#[test_case(MarketType::Categorical(2))]
-#[test_case(MarketType::Scalar(0..=1))]
-fn combo_sell_fails_on_asset_not_found(market_type: MarketType) {
-    ExtBuilder::default().build().execute_with(|| {
-        let market_id = create_market_and_deploy_pool(
-            ALICE,
-            BASE_ASSET,
-            market_type,
-            _10,
-            vec![_1_2, _1_2],
-            CENT,
-        );
-        assert_noop!(
-            NeoSwaps::combo_sell(
-                RuntimeOrigin::signed(BOB),
-                market_id,
-                2,
-                vec![Asset::CategoricalOutcome(market_id, 3)],
-                vec![Asset::CategoricalOutcome(market_id, 5)],
-                vec![Asset::CategoricalOutcome(market_id, 4)],
-                _1,
-                0,
-                u128::MAX,
-            ),
-            Error::<Runtime>::AssetNotFound,
-        );
-    });
-}
-
 #[test]
 fn combo_sell_fails_on_insufficient_funds() {
     ExtBuilder::default().build().execute_with(|| {
@@ -307,3 +278,52 @@ fn combo_sell_fails_on_amount_out_below_min() {
         );
     });
 }
+
+#[test_case(vec![], vec![], vec![2]; "empty_buy")]
+#[test_case(vec![0], vec![], vec![]; "empty_sell")]
+#[test_case(vec![0, 1], vec![2, 1], vec![3, 4]; "buy_keep_overlap")]
+#[test_case(vec![0, 1], vec![2, 4], vec![3, 1]; "buy_sell_overlap")]
+#[test_case(vec![0, 1], vec![2, 4], vec![4, 3]; "keep_sell_overlap")]
+#[test_case(vec![0, 1, 999], vec![2, 4], vec![5, 3]; "out_of_bounds_buy")]
+#[test_case(vec![0, 1], vec![2, 4, 999], vec![5, 3]; "out_of_bounds_keep")]
+#[test_case(vec![0, 1], vec![2, 4], vec![5, 999, 3]; "out_of_bounds_sell")]
+#[test_case(vec![0, 6, 1, 6], vec![2, 4], vec![5, 3]; "duplicate_buy")]
+#[test_case(vec![0, 1], vec![2, 2, 4], vec![5, 3]; "duplicate_keep")]
+#[test_case(vec![0, 1], vec![2, 4], vec![5, 3, 6, 6, 6]; "duplicate_sell")]
+fn combo_buy_fails_on_invalid_partition(
+    indices_buy: Vec<u16>,
+    indices_keep: Vec<u16>,
+    indices_sell: Vec<u16>,
+) {
+    ExtBuilder::default().build().execute_with(|| {
+        println!("{:?}", _1_7);
+        let market_id = create_market_and_deploy_pool(
+            ALICE,
+            BASE_ASSET,
+            MarketType::Categorical(7),
+            _10,
+            vec![_1_7, _1_7, _1_7, _1_7, _1_7, _1_7, _1_7 + 4],
+            CENT,
+        );
+
+        let buy = indices_buy.into_iter().map(|i| CategoricalOutcome(market_id, i)).collect();
+        let keep = indices_keep.into_iter().map(|i| CategoricalOutcome(market_id, i)).collect();
+        let sell = indices_sell.into_iter().map(|i| CategoricalOutcome(market_id, i)).collect();
+
+        // Buying 1 at price of .5 will return less than 2 outcomes due to slippage.
+        assert_noop!(
+            NeoSwaps::combo_sell(
+                RuntimeOrigin::signed(BOB),
+                market_id,
+                7,
+                buy,
+                keep,
+                sell,
+                _2,
+                _1,
+                0
+            ),
+            Error::<Runtime>::InvalidPartition,
+        );
+    });
+}