Pricegraph: Store order remaining amount and user balances as integers (

#1417)

instead of floats.

While working on a using a BigRational type for ExchangeRate in a different branch I noticed
that this change also makes sense and could be refactored it into its own PR.

With the BigRational change Flow::capacity and min_trade would also become u128s.

Benchmark
<details>Pricegraph::read        time:   [5.6543 ms 5.6597 ms 5.6654 ms]
                        change: [-0.6112% -0.4534% -0.2954%] (p = 0.00 < 0.05)
                        Change within noise threshold.

Pricegraph::transitive_orderbook/5298183
                        time:   [5.5705 ms 5.5748 ms 5.5794 ms]
                        change: [+0.7394% +0.8957% +1.0439%] (p = 0.00 < 0.05)
                        Change within noise threshold.

Pricegraph::estimate_limit_price/100000000000000000
                        time:   [47.531 us 47.607 us 47.670 us]
                        change: [+8.3493% +8.4750% +8.5964%] (p = 0.00 < 0.05)
                        Performance has regressed.

Pricegraph::estimate_limit_price/1000000000000000000
                        time:   [48.837 us 48.874 us 48.922 us]
                        change: [+1.8688% +2.1218% +2.3802%] (p = 0.00 < 0.05)
                        Performance has regressed.

Pricegraph::estimate_limit_price/10000000000000000000
                        time:   [94.161 us 94.197 us 94.240 us]
                        change: [+2.0211% +2.2254% +2.4099%] (p = 0.00 < 0.05)
                        Performance has regressed.

Pricegraph::estimate_limit_price/100000000000000000000
                        time:   [192.46 us 192.72 us 193.05 us]
                        change: [+1.5835% +1.8608% +2.0939%] (p = 0.00 < 0.05)
                        Performance has regressed.

Pricegraph::estimate_limit_price/1000000000000000000000
                        time:   [477.25 us 477.63 us 478.07 us]
                        change: [+7.6418% +7.8084% +7.9628%] (p = 0.00 < 0.05)
                        Performance has regressed.

Pricegraph::order_for_limit_price/200
                        time:   [42.607 us 42.744 us 42.921 us]
                        change: [+4.3753% +4.8784% +5.4368%] (p = 0.00 < 0.05)
                        Performance has regressed.

Pricegraph::order_for_limit_price/190
                        time:   [122.03 us 122.29 us 122.61 us]
                        change: [+1.9331% +2.3399% +2.6983%] (p = 0.00 < 0.05)
                        Performance has regressed.

Pricegraph::order_for_limit_price/180
                        time:   [208.11 us 208.35 us 208.58 us]
                        change: [+2.1229% +2.5886% +3.0075%] (p = 0.00 < 0.05)
                        Performance has regressed.

Pricegraph::order_for_limit_price/150
                        time:   [452.56 us 452.84 us 453.11 us]
                        change: [+2.9362% +3.0678% +3.1991%] (p = 0.00 < 0.05)
                        Performance has regressed.

Pricegraph::order_for_limit_price/100
                        time:   [509.79 us 510.08 us 510.46 us]
                        change: [+6.5567% +6.7537% +6.9483%] (p = 0.00 < 0.05)
                        Performance has regressed.
</details>

Benchmark now with balance as U256
<details>
Pricegraph::read        time:   [5.9581 ms 5.9664 ms 5.9754 ms]
                        change: [+2.0925% +2.6443% +3.1203%] (p = 0.00 < 0.05)
                        Performance has regressed.

Pricegraph::transitive_orderbook/5298183
                        time:   [5.8476 ms 5.8520 ms 5.8571 ms]
                        change: [+2.1179% +2.6124% +3.0897%] (p = 0.00 < 0.05)
                        Performance has regressed.

Pricegraph::estimate_limit_price/100000000000000000
                        time:   [47.819 us 47.846 us 47.873 us]
                        change: [+8.1399% +8.8444% +9.4816%] (p = 0.00 < 0.05)
                        Performance has regressed.

Pricegraph::estimate_limit_price/1000000000000000000
                        time:   [49.410 us 49.513 us 49.632 us]
                        change: [+1.9095% +2.8321% +3.6865%] (p = 0.00 < 0.05)
                        Performance has regressed.

Pricegraph::estimate_limit_price/10000000000000000000
                        time:   [96.026 us 96.119 us 96.240 us]
                        change: [+1.8636% +2.7113% +3.3794%] (p = 0.00 < 0.05)
                        Performance has regressed.

Pricegraph::estimate_limit_price/100000000000000000000
                        time:   [197.57 us 197.73 us 197.95 us]
                        change: [-0.4474% +0.8413% +2.0184%] (p = 0.18 > 0.05)
                        No change in performance detected.

Pricegraph::estimate_limit_price/1000000000000000000000
                        time:   [488.48 us 489.17 us 490.14 us]
                        change: [+7.3693% +8.1000% +8.7454%] (p = 0.00 < 0.05)
                        Performance has regressed.

Pricegraph::order_for_limit_price/200
                        time:   [41.826 us 41.905 us 42.000 us]
                        change: [+3.6122% +4.2047% +4.7273%] (p = 0.00 < 0.05)
                        Performance has regressed.

Pricegraph::order_for_limit_price/190
                        time:   [121.74 us 121.89 us 122.08 us]
                        change: [-1.0053% -0.2611% +0.3924%] (p = 0.48 > 0.05)
                        No change in performance detected.

Pricegraph::order_for_limit_price/180
                        time:   [213.23 us 213.31 us 213.41 us]
                        change: [+0.1689% +1.2980% +2.2842%] (p = 0.01 < 0.05)
                        Change within noise threshold.

Pricegraph::order_for_limit_price/150
                        time:   [464.76 us 465.16 us 465.65 us]
                        change: [+5.2298% +5.4621% +5.6739%] (p = 0.00 < 0.05)
                        Performance has regressed.

Pricegraph::order_for_limit_price/100
                        time:   [521.98 us 522.61 us 523.40 us]
                        change: [+8.5424% +8.7334% +8.9668%] (p = 0.00 < 0.05)
                        Performance has regressed.
</details>

### Test Plan
Existing unit tests.

pricegraph/src/api/price_estimation.rs

@@ -16,7 +16,7 @@ impl Pricegraph {
     /// the ratio between buy and sell amounts, with implicit fees.
     pub fn estimate_limit_price(&self, pair: TokenPair, max_sell_amount: f64) -> Option<f64> {
         if !num::is_strictly_positive_and_finite(max_sell_amount)
-            || num::is_dust_amount(max_sell_amount)
+            || num::is_dust_amount(max_sell_amount as u128)
         {
             return None;
         }
@@ -75,7 +75,7 @@ impl Pricegraph {
         // be overlapping with existing orders such that an executed buy amount
         // could be found greater than the dust amount.
         let min_buy_amount = max_sell_amount * price;
-        if num::is_dust_amount(min_buy_amount) {
+        if num::is_dust_amount(min_buy_amount as u128) {
             return None;
         }
 

pricegraph/src/lib.rs

@@ -24,7 +24,7 @@ const FEE_TOKEN: TokenId = 0;
 /// solution. Orders with effective sell amounts smaller than this amount can
 /// safely be ignored, and transitive orders with flows that trade amounts
 /// smaller than this are not considered for price estimates.
-pub const MIN_AMOUNT: f64 = 10_000.0;
+pub const MIN_AMOUNT: u128 = 10_000;
 
 /// API entry point for computing price estimates and transitive orderbooks for
 /// a give auction.

pricegraph/src/num.rs

@@ -80,7 +80,7 @@ pub fn is_strictly_positive_and_finite(value: f64) -> bool {
 
 /// Returns true if an amount is considered a dust amount. See `MIN_AMOUNT`
 /// documentation for more details.
-pub fn is_dust_amount(amount: f64) -> bool {
+pub fn is_dust_amount(amount: u128) -> bool {
     amount < MIN_AMOUNT
 }
 

pricegraph/src/orderbook.rs

@@ -16,7 +16,7 @@ mod weight;
 
 pub use self::flow::{Flow, Ring};
 pub use self::iter::TransitiveOrders;
-use self::order::{Order, OrderCollector, OrderMap};
+use self::order::{Amount, Order, OrderCollector, OrderMap};
 pub use self::reduced::ReducedOrderbook;
 pub use self::scalar::{ExchangeRate, LimitPrice};
 use self::user::{User, UserMap};
@@ -29,6 +29,7 @@ use crate::graph::subgraph::{ControlFlow, Subgraphs};
 use crate::num;
 use petgraph::graph::{DiGraph, EdgeIndex, NodeIndex};
 use petgraph::visit::NodeIndexable;
+use primitive_types::U256;
 use std::cmp;
 use std::f64;
 use thiserror::Error;
@@ -328,7 +329,7 @@ impl Orderbook {
             transitive_xrate *= order.exchange_rate;
             max_xrate = cmp::max(max_xrate, transitive_xrate);
 
-            let sell_amount = order.get_effective_amount(&self.users);
+            let sell_amount = order.get_effective_amount(&self.users) as f64;
             capacity = num::min(capacity, sell_amount * transitive_xrate.value());
         }
 
@@ -355,23 +356,19 @@ impl Orderbook {
 
             // NOTE: `capacity` is expressed in the buy token, so we need to
             // divide by the exchange rate to get the sell amount being filled.
-            let fill_amount = flow.capacity / transitive_xrate.value();
-
-            order.amount -= fill_amount;
-            debug_assert!(
-                order.amount >= -num::max_rounding_error(fill_amount),
-                "remaining amount underflow for order {}-{}",
-                order.user,
-                order.id,
-            );
-
+            let fill_amount = (flow.capacity / transitive_xrate.value()) as u128;
             let new_balance = user.deduct_from_balance(pair.sell, fill_amount);
 
             if num::is_dust_amount(new_balance) {
                 user.clear_balance(pair.sell);
                 self.update_projection_graph_node(pair.sell);
-            } else if num::is_dust_amount(order.amount) {
-                self.update_projection_graph_edge(pair);
+            } else if let Amount::Remaining(amount) = &mut order.amount {
+                // TODO: add a debug assert to see that we are not over filling orders.
+                // Will do this when we use BigRational.
+                *amount = amount.saturating_sub(fill_amount);
+                if num::is_dust_amount(*amount) {
+                    self.update_projection_graph_edge(pair);
+                }
             }
         }
 
@@ -438,7 +435,8 @@ fn format_path(path: &[NodeIndex]) -> String {
 /// for trades
 fn is_dust_order(element: &Element) -> bool {
     num::is_dust_amount(element.remaining_sell_amount as _)
-        || num::is_dust_amount(num::u256_to_f64(element.balance))
+        || (num::is_dust_amount(element.balance.low_u128())
+            && element.balance < U256::from(u128::MAX))
 }
 
 /// An error indicating an invalid operation was performed on an overlapping
@@ -710,40 +708,40 @@ mod tests {
         assert_eq!(orderbook.num_orders(), 3);
 
         let transitive_xrate_0_1 = FEE_FACTOR;
-        assert_approx_eq!(
+        assert_eq!(
             orderbook
                 .orders
                 .best_order_for_pair(TokenPair { buy: 0, sell: 1 })
                 .unwrap()
                 .amount,
-            1_000_000.0 - flow.capacity / transitive_xrate_0_1
+            Amount::Remaining(1_000_000 - (flow.capacity / transitive_xrate_0_1) as u128)
         );
-        assert_approx_eq!(
+        assert_eq!(
             orderbook.users[&user_id(1)].balance_of(1),
-            1_000_000.0 - flow.capacity / transitive_xrate_0_1
+            1_000_000 - (flow.capacity / transitive_xrate_0_1) as u128
         );
 
-        assert_approx_eq!(
+        assert_eq!(
             orderbook
                 .orders
                 .best_order_for_pair(TokenPair { buy: 1, sell: 2 })
                 .unwrap()
                 .amount,
-            1_000_000.0
+            Amount::Remaining(1_000_000),
         );
-        assert_approx_eq!(orderbook.users[&user_id(5)].balance_of(2), 1_000_000.0);
+        assert_eq!(orderbook.users[&user_id(5)].balance_of(2), 1_000_000);
 
-        assert_approx_eq!(
+        assert_eq!(
             orderbook
                 .orders
                 .best_order_for_pair(TokenPair { buy: 3, sell: 4 })
                 .unwrap()
                 .amount,
-            2_000_000.0 - flow.capacity / flow.exchange_rate.value()
+            Amount::Remaining(2_000_000 - (flow.capacity / flow.exchange_rate.value()) as u128)
         );
-        assert_approx_eq!(
+        assert_eq!(
             orderbook.users[&user_id(4)].balance_of(4),
-            10_000_000.0 - flow.capacity / flow.exchange_rate.value()
+            10_000_000 - (flow.capacity / flow.exchange_rate.value()) as u128
         );
     }
 

pricegraph/src/orderbook/flow.rs

@@ -32,7 +32,7 @@ impl Flow {
 
     /// Returns true if this flow is a dust trade.
     pub fn is_dust_trade(&self) -> bool {
-        num::is_dust_amount(self.min_trade)
+        num::is_dust_amount(self.min_trade as u128)
     }
 }
 

pricegraph/src/orderbook/order.rs

@@ -2,9 +2,7 @@
 
 use super::{map::Map, ExchangeRate, LimitPrice, UserMap};
 use crate::encoding::{Element, OrderId, TokenId, TokenPair, UserId};
-use crate::num;
 use std::cmp::Reverse;
-use std::f64;
 
 /// A type for collecting orders and building an order map that garantees that
 /// per-pair orders are sorted for optimal access.
@@ -111,6 +109,13 @@ impl OrderMap {
     }
 }
 
+/// The remaining amount in an order can be unlimited or fixed.
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub enum Amount {
+    Unlimited,
+    Remaining(u128),
+}
+
 /// A single order with a reference to the user.
 ///
 /// Note that we approximate amounts and prices with floating point numbers.
@@ -127,7 +132,7 @@ pub struct Order {
     /// The maximum capacity for this order, this is equivalent to the order's
     /// remaining sell amount. Note that orders are also limited by their user's
     /// balance.
-    pub amount: f64,
+    pub amount: Amount,
     /// The effective exchange rate for this order.
     pub exchange_rate: ExchangeRate,
 }
@@ -136,9 +141,9 @@ impl Order {
     /// Creates a new order from an ID and an orderbook element.
     pub fn new(element: &Element) -> Option<Self> {
         let amount = if is_unbounded(&element) {
-            f64::INFINITY
+            Amount::Unlimited
         } else {
-            element.remaining_sell_amount as _
+            Amount::Remaining(element.remaining_sell_amount)
         };
         let exchange_rate = LimitPrice::from_fraction(&element.price)?.exchange_rate();
 
@@ -154,9 +159,12 @@ impl Order {
     /// Retrieves the effective remaining amount for this order based on user
     /// balances. This is the minimum between the remaining order amount and
     /// the user's sell token balance.
-    pub fn get_effective_amount(&self, users: &UserMap) -> f64 {
+    pub fn get_effective_amount(&self, users: &UserMap) -> u128 {
         let balance = users[&self.user].balance_of(self.pair.sell);
-        num::min(self.amount, balance)
+        match self.amount {
+            Amount::Unlimited => balance,
+            Amount::Remaining(amount) => amount.min(balance),
+        }
     }
 }
 

pricegraph/src/orderbook/user.rs

@@ -2,7 +2,7 @@
 
 use super::map::{self, Map};
 use crate::encoding::{Element, TokenId, UserId};
-use crate::num;
+use primitive_types::U256;
 
 /// A type definiton for a mapping between user IDs to user data.
 pub type UserMap = Map<UserId, User>;
@@ -11,7 +11,7 @@ pub type UserMap = Map<UserId, User>;
 #[derive(Clone, Debug, Default)]
 pub struct User {
     /// User balances per token.
-    balances: Map<TokenId, f64>,
+    balances: Map<TokenId, U256>,
 }
 
 impl User {
@@ -20,40 +20,37 @@ impl User {
     pub fn set_balance(&mut self, element: &Element) {
         self.balances
             .entry(element.pair.sell)
-            .or_insert_with(|| num::u256_to_f64(element.balance));
+            .or_insert_with(|| element.balance);
     }
 
     /// Return's the user's balance for the specified token.
-    pub fn balance_of(&self, token: TokenId) -> f64 {
-        self.balances.get(&token).copied().unwrap_or(0.0)
+    /// Panics if the user doesn't have a balance.
+    pub fn balance_of(&self, token: TokenId) -> u128 {
+        self.balances
+            .get(&token)
+            .map(u256_to_u128_saturating)
+            .unwrap_or(0)
     }
 
     /// Deducts an amount from the balance for the given token. Returns the new
-    /// balance or `None` if the user no longer has any balance.
-    pub fn deduct_from_balance(&mut self, token: TokenId, amount: f64) -> f64 {
+    /// balance.
+    pub fn deduct_from_balance(&mut self, token: TokenId, amount: u128) -> u128 {
         if let map::Entry::Occupied(mut entry) = self.balances.entry(token) {
             let balance = entry.get_mut();
-            *balance -= amount;
-
-            debug_assert!(
-                *balance >= -num::max_rounding_error(amount),
-                "user balance underflow for token {}",
-                token,
-            );
-
-            if *balance > 0.0 {
-                *balance
-            } else {
+            *balance = balance.saturating_sub(U256::from(amount));
+            if balance.is_zero() {
                 entry.remove_entry();
-                0.0
+                0
+            } else {
+                u256_to_u128_saturating(balance)
             }
         } else {
             debug_assert!(
                 false,
                 "deducted amount from user with empty balace for token {}",
                 token,
             );
-            0.0
+            0
         }
     }
 
@@ -62,3 +59,26 @@ impl User {
         self.balances.remove(&token);
     }
 }
+
+fn u256_to_u128_saturating(u256: &U256) -> u128 {
+    u256.min(&u128::MAX.into()).low_u128()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn u256_to_u128() {
+        assert_eq!(
+            u256_to_u128_saturating(&(u128::MAX - 1).into()),
+            u128::MAX - 1
+        );
+        assert_eq!(u256_to_u128_saturating(&u128::MAX.into()), u128::MAX);
+        assert_eq!(
+            u256_to_u128_saturating(&(U256::from(u128::MAX) + U256::from(1))),
+            u128::MAX
+        );
+        assert_eq!(u256_to_u128_saturating(&U256::MAX), u128::MAX);
+    }
+}

pricegraph/src/orderbook/weight.rs

@@ -135,7 +135,7 @@ mod tests {
         assert_eq!(-f64::MAX as i128, i128::MIN);
 
         const MAX_TOKENS: f64 = (1 << 16) as _;
-        let max_xrate = 2.0f64.powi(128) / MIN_AMOUNT;
+        let max_xrate = 2.0f64.powi(128) / MIN_AMOUNT as f64;
 
         let max_total_weight = {
             let weight = max_xrate.log2() * MAX_TOKENS * FIXED_24X104_SCALING_FACTOR;