From f9f8306af032f5d44899c24a90a79d7d6670d2ae Mon Sep 17 00:00:00 2001
From: Noah Zinsmeister <noahwz@gmail.com>
Date: Fri, 22 Nov 2019 13:14:10 -0500
Subject: [PATCH] add clarifying comments to the fixed point library

---
 contracts/UniswapV2.sol           |  3 +--
 contracts/libraries/UQ104x104.sol | 34 ++++++++++++++++++++++---------
 2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/contracts/UniswapV2.sol b/contracts/UniswapV2.sol
index 9e80ad0..1a73862 100644
--- a/contracts/UniswapV2.sol
+++ b/contracts/UniswapV2.sol
@@ -12,7 +12,7 @@ import "./token/SafeTransfer.sol";
 contract UniswapV2 is IUniswapV2, ERC20("Uniswap V2", "UNI-V2", 18, 0), SafeTransfer {
     using SafeMath128 for uint128;
     using SafeMath256 for uint256;
-    using UQ104x104 for uint208;
+    using UQ104x104 for uint232;
 
     struct TokenData {
         uint128 token0;
@@ -91,7 +91,6 @@ contract UniswapV2 is IUniswapV2, ERC20("Uniswap V2", "UNI-V2", 18, 0), SafeTran
             uint208 priceToken0 = UQ104x104.encode(reserves.token0).qdiv(UQ104x104.encode(reserves.token1));
             uint208 priceToken1 = UQ104x104.encode(reserves.token1).qdiv(UQ104x104.encode(reserves.token0));
 
-                // multiply these prices by the number of elapsed blocks and add to the accumulators
             return (
                 priceToken0Accumulated + (uint240(priceToken0) * blocksElapsed),
                 priceToken1Accumulated + (uint240(priceToken1) * blocksElapsed)
diff --git a/contracts/libraries/UQ104x104.sol b/contracts/libraries/UQ104x104.sol
index 26d0e8b..bf21bd1 100644
--- a/contracts/libraries/UQ104x104.sol
+++ b/contracts/libraries/UQ104x104.sol
@@ -1,16 +1,30 @@
 pragma solidity 0.5.12;
 
-// TODO this library is broken at the moment, and is meant only to serve as a mock
-library UQ104x104 {
-    uint208 constant Q104 = uint104(-1);
+// helpful links
+// https://en.wikipedia.org/wiki/Q_(number_format)
+// https://github.com/abdk-consulting/abdk-libraries-solidity/blob/master/ABDKMath64x64.md
+// https://github.com/gnosis/solidity-arithmetic
 
-    function encode(uint128 y) internal pure returns (uint208 z) {
-        require(y <= Q104, "encode-overflow");
-        z = y * Q104;
+library UQ104x104 {
+    uint232 constant Q104 = uint232(uint104(-1)) + 1;
+
+    // we want to encode a uint128 `y` s.t. `y := y_encoded / 2**104` (i.e. with a Q104 denominator).
+    // in other words, to encode `y` we simply multiply by `2**104`, aka Q104.
+    // however, in the case of a traditional UQ104.104, we'd store this output in a 208-bit slot,
+    // which would overflow for values of `y` in (`uint104(-1)`, `uint128(-1)`], so instead we need to
+    // store the output in 232 bits (TODO check this logic).
+    function encode(uint128 y) internal pure returns (uint232 z) {
+        return uint232(y) * Q104;
     }
-    function qdiv(uint208 x, uint208 y) internal pure returns (uint208 z) {
-        uint256 quotient = uint256(x) / uint256(y);
-        require(quotient <= Q104, "div-overflow");
-        z = uint208(quotient * Q104);
+
+    // we want to divide two modified-UQ104.104s (the outputs of encode), and return a traditional Q104.
+    // for our purposes, we'll do that by flooring the output of the division with `uint208(-1)`.
+    // (this corresponds to capping the relative prices of x and y at `1 / 2**104` and `uint208(-1) / 2**104`.)
+    // unfortunately, before we can compute `min(uint208(-1), output), we need to compute `output = x * 2**104 / y`,
+    // for which we need at least 416 bits (possibly 438 or 464? TODO think this through).
+    // for now, we just mock the function
+    function qdiv(uint232 x, uint232 y) internal pure returns (uint208 z) {
+        // TODO replace mock with real logic
+        z = uint208(x * Q104 / y);
     }
 }