show failing case (Uniswap#99)

* show failing case * fix compilation error add TODOs about broken assert * add other assert block back in
thaixuandang · Oct 23, 2020 · 7b3b390 · 7b3b390
1 parent 7de1004
commit 7b3b390
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 11 deletions.
diff --git a/contracts/UniswapV3Pair.sol b/contracts/UniswapV3Pair.sol
@@ -357,7 +357,7 @@ contract UniswapV3Pair is IUniswapV3Pair {
         reserve0Virtual = reserve0Virtual.addi(amount0).toUint112();
         reserve1Virtual = reserve1Virtual.addi(amount1).toUint112();
 
-        // TODO remove this eventually, it's simply meant to show the direction of rounding in both cases
+        // TODO remove this eventually, it's meant to demonstrate the direction of rounding
         FixedPoint.uq112x112 memory priceNext = FixedPoint.fraction(reserve1Virtual, reserve0Virtual);
         if (amount0 > 0) {
             assert(priceNext._x >= price._x);
@@ -577,9 +577,7 @@ contract UniswapV3Pair is IUniswapV3Pair {
             // TODO ensure that there's no off-by-one error here while transitioning ticks
             if (amountInRequiredForShift > 0) {
                 // either trade fully to the next tick, or only as much as we need to
-                step.amountIn = amountInRemaining > amountInRequiredForShift
-                    ? amountInRequiredForShift
-                    : amountInRemaining;
+                step.amountIn = Math.min(amountInRequiredForShift, amountInRemaining).toUint112();
 
                 // calculate the owed output amount, given the current fee
                 step.amountOut = ((uint256(reserveOutVirtual) * step.amountIn * (PriceMath.LP_FEE_BASE - fee)) /
@@ -589,6 +587,26 @@ contract UniswapV3Pair is IUniswapV3Pair {
                         PriceMath.LP_FEE_BASE))
                     .toUint112();
 
+                // TODO remove this eventually, it's meant to ensure PriceMath.getInputToRatio is working correctly
+                {
+                FixedPoint.uq112x112 memory priceNext;
+                if (params.zeroForOne) {
+                    priceNext = FixedPoint.fraction(
+                        reserve1Virtual.sub(step.amountOut).toUint112(),
+                        (uint256(reserve0Virtual) + step.amountIn).toUint112()
+                    );
+                    // TODO this should be <=
+                    assert(priceNext._x >= step.nextPrice._x);
+                } else {
+                    priceNext = FixedPoint.fraction(
+                        (uint256(reserve1Virtual) + step.amountIn).toUint112(),
+                        reserve0Virtual.sub(step.amountOut).toUint112()
+                    );
+                    // TODO this should be >=
+                    assert(priceNext._x <= step.nextPrice._x);                    
+                }
+                }
+
                 // calculate the maximum output amount s.t. the reserves price is guaranteed to be as close as possible
                 // to the target price _without_ exceeding it
                 uint112 reserveInVirtualNext = (uint256(reserveInVirtual) + amountInRequiredForShift).toUint112();
@@ -606,7 +624,7 @@ contract UniswapV3Pair is IUniswapV3Pair {
                     reserve1Virtual = (uint256(reserve1Virtual) + step.amountIn).toUint112();
                 }
 
-                // TODO remove this eventually, it's simply meant to ensure our overshoot logic is correct
+                // TODO remove this eventually, it's meant to ensure our overshoot compensation logic is correct
                 {
                 FixedPoint.uq112x112 memory priceNext = FixedPoint.fraction(reserve1Virtual, reserve0Virtual);
                 if (params.zeroForOne) {
@@ -654,7 +672,7 @@ contract UniswapV3Pair is IUniswapV3Pair {
                         reserve1Virtual = reserve1Virtual.addi(token1VirtualDelta).toUint112();
                     }
 
-                    // TODO remove this eventually, it's simply meant to show the direction of rounding
+                    // TODO remove this eventually, it's meant to show the direction of rounding
                     {
                     FixedPoint.uq112x112 memory priceNext = FixedPoint.fraction(reserve1Virtual, reserve0Virtual);
                     if (params.zeroForOne) {

diff --git a/test/__snapshots__/UniswapV3Factory.spec.ts.snap b/test/__snapshots__/UniswapV3Factory.spec.ts.snap
@@ -1,3 +1,3 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`UniswapV3Factory #createPair gas 1`] = `4073300`;
+exports[`UniswapV3Factory #createPair gas 1`] = `4121269`;
diff --git a/test/__snapshots__/UniswapV3Pair.spec.ts.snap b/test/__snapshots__/UniswapV3Pair.spec.ts.snap
@@ -12,10 +12,10 @@ exports[`UniswapV3Pair #getVirtualSupply gas cost two votes 1`] = `5294`;
 
 exports[`UniswapV3Pair #getVirtualSupply gas cost uninitialized 1`] = `5294`;
 
-exports[`UniswapV3Pair post-initialize (fee vote 1 - 0.10%) swap0For1 gas 1`] = `105875`;
+exports[`UniswapV3Pair post-initialize (fee vote 1 - 0.10%) swap0For1 gas 1`] = `108515`;
 
-exports[`UniswapV3Pair post-initialize (fee vote 1 - 0.10%) swap0For1 gas large swap 1`] = `1630407`;
+exports[`UniswapV3Pair post-initialize (fee vote 1 - 0.10%) swap0For1 gas large swap 1`] = `1851173`;
 
-exports[`UniswapV3Pair post-initialize (fee vote 1 - 0.10%) swap1For0 gas 1`] = `92136`;
+exports[`UniswapV3Pair post-initialize (fee vote 1 - 0.10%) swap1For0 gas 1`] = `94766`;
 
-exports[`UniswapV3Pair post-initialize (fee vote 1 - 0.10%) swap1For0 gas large swap 1`] = `1567986`;
+exports[`UniswapV3Pair post-initialize (fee vote 1 - 0.10%) swap1For0 gas large swap 1`] = `1787813`;