Merge pull request #295 from northwesternfintech/mm-overflow-fix

Added better overflow fix in market maker by using float ratio
northwesternfintech · Oct 9, 2024 · cf51530 · cf51530
2 parents 4da713c + 0d78947
commit cf51530
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 19 deletions.
diff --git a/exchange/src/common/types/decimal.cpp b/exchange/src/common/types/decimal.cpp
@@ -6,7 +6,7 @@ template <std::uint8_t Scale>
 Decimal<Scale>
 Decimal<Scale>::operator-() const
 {
-    return -value_;
+    return Decimal{-value_};
 }
 
 template <std::uint8_t Scale>
@@ -27,28 +27,28 @@ template <std::uint8_t Scale>
 Decimal<Scale>
 Decimal<Scale>::operator-(const Decimal& other) const
 {
-    return value_ - other.value_;
+    return Decimal{value_ - other.value_};
 }
 
 template <std::uint8_t Scale>
 Decimal<Scale>
 Decimal<Scale>::operator+(const Decimal& other) const
 {
-    return value_ + other.value_;
+    return Decimal{value_ + other.value_};
 }
 
 template <std::uint8_t Scale>
 Decimal<Scale>
 Decimal<Scale>::operator/(const Decimal& other) const
 {
-    return value_ * MULTIPLIER / other.value_;
+    return Decimal{value_ * MULTIPLIER / other.value_};
 }
 
 template <std::uint8_t Scale>
 Decimal<Scale>
 Decimal<Scale>::operator*(const Decimal& other) const
 {
-    return (value_ * other.value_) / MULTIPLIER;
+    return Decimal{(value_ * other.value_) / MULTIPLIER};
 }
 
 template <std::uint8_t Scale>
@@ -101,14 +101,15 @@ Decimal<Scale>
 Decimal<Scale>::difference(const Decimal& other) const
 {
     if (value_ >= other.value_) {
-        return value_ - other.value_;
+        return Decimal{value_ - other.value_};
     }
-    return other.value_ - value_;
+    return Decimal{other.value_ - value_};
 }
 
 template class Decimal<PRICE_DECIMAL_PLACES>;
 
 #if PRICE_DECIMAL_PLACES != QUANTITY_DECIMAL_PLACES
 template class Decimal<QUANTITY_DECIMAL_PLACES>;
 #endif
+
 } // namespace nutc::common
diff --git a/exchange/src/common/types/decimal.hpp b/exchange/src/common/types/decimal.hpp
@@ -24,6 +24,7 @@ pow10(int pow)
 }
 } // namespace detail
 
+// THIS CLASS PRIORITIZES PRECISION OVER AVOIDING OVERFLOW
 template <std::uint8_t Scale>
 class Decimal {
     using decimal_type = std::int64_t;
@@ -56,7 +57,7 @@ class Decimal {
     Decimal difference(const Decimal& other) const;
 
 private:
-    constexpr Decimal(decimal_type value) : value_(value) {}
+    constexpr explicit Decimal(decimal_type value) : value_(value) {}
 
     static constexpr bool
     double_within_bounds(double value)
@@ -110,19 +111,23 @@ struct hash<nutc::common::Decimal<Scale>> {
 // TODO: add unit tests
 template <std::uint8_t Scale>
 class numeric_limits<nutc::common::Decimal<Scale>> {
+    using scaled_decimal = nutc::common::Decimal<Scale>;
+
 public:
-    static nutc::common::Decimal<Scale>
+    static scaled_decimal
     max()
     {
-        return std::numeric_limits<
-            typename nutc::common::Decimal<Scale>::decimal_type>::max();
+        return scaled_decimal{
+            std::numeric_limits<typename scaled_decimal::decimal_type>::max()
+        };
     }
 
-    static nutc::common::Decimal<Scale>
+    static scaled_decimal
     min()
     {
-        return std::numeric_limits<
-            typename nutc::common::Decimal<Scale>::decimal_type>::min();
+        return scaled_decimal{
+            std::numeric_limits<typename scaled_decimal::decimal_type>::min()
+        };
     }
 };
 

diff --git a/exchange/src/exchange/bots/bot_container.cpp b/exchange/src/exchange/bots/bot_container.cpp
@@ -18,7 +18,7 @@ BotContainer::generate_orders(
 {
     variance_calculator_.record_price(midprice);
 
-    decimal_price cumulative_interest_limit{};
+	common::decimal_price cumulative_interest_limit{};
     common::decimal_quantity cumulative_quantity_held{};
 
     for (const auto& bot : bots_) {

diff --git a/exchange/src/exchange/bots/bot_types/market_maker.cpp b/exchange/src/exchange/bots/bot_types/market_maker.cpp
@@ -64,8 +64,9 @@ MarketMakerBot::place_orders_(
 decimal_price
 MarketMakerBot::calculate_lean_percent(const shared_bot_state& state)
 {
-    auto lean_price = state.CUMULATIVE_QUANTITY_HELD / state.CUMULATIVE_INTEREST_LIMIT;
-    return lean_price * state.MIDPRICE;
+    double ratio = double{state.CUMULATIVE_QUANTITY_HELD}
+                   / double{state.CUMULATIVE_INTEREST_LIMIT};
+    return ratio * static_cast<double>(state.MIDPRICE);
 }
 
 // TODO: clean up

diff --git a/exchange/test/src/unit/types/decimal.cpp b/exchange/test/src/unit/types/decimal.cpp
@@ -18,6 +18,19 @@ TEST(UnitDecimalPrice, AssignmentOperator)
     EXPECT_EQ(second, 5.0);
 }
 
+TEST(UnitDecimalPrice, TestCommutativity)
+{
+    decimal_price three = 3.0;
+    decimal_price four = 4.0;
+    decimal_price six = 6.0;
+
+	decimal_price twelve = three*four;
+	EXPECT_EQ(double{twelve/six}, 2.0);
+
+	decimal_price point_five = three/six;
+	EXPECT_EQ(double{point_five*four}, 2.0);
+}
+
 TEST(UnitDecimalPrice, UnaryNegate)
 {
     decimal_price first = 5.0;
@@ -45,8 +58,8 @@ TEST(UnitDecimalPrice, DecimalDecimalDivision)
 {
     decimal_price first = 10.0;
     decimal_price second = 2.0;
-    double third{first / second};
-    EXPECT_EQ(third, 5.0);
+	decimal_price third = first/second;
+    EXPECT_EQ(double{third}, 5.0);
 }
 
 TEST(UnitDecimalPrice, DecimalDecimalMultiplication)