optimistic swaps (Uniswap#53)

* alternative flash lending (renting) design

* add rent interface

* fix stack too deep error

rearrange order of k condition math

ignore erroneous out of gas errors in tests

* try removing rent in favor of monolithic swap

IUniswapV2Borrower -> IUniswapV2Callee

update tests

* fix implementation

* clean up math a bit

* amount{0,1}In -> amount{0,1}InNet

* charge on all inputs, not just net

* removed unnecessary safemath

* add to != token check

don't indent in scope

rename reserve{0,1}Next -> reserve{0,1}Adjusted

* > instead of >=

simplify algebra

reserve{0,1}Adjusted -> balance{0,1}Adjusted

add comments

* add some optimistic swap test cases

contracts/UniswapV2Exchange.sol

@@ -6,6 +6,7 @@ import './libraries/Math.sol';
 import './libraries/UQ112x112.sol';
 import './interfaces/IERC20.sol';
 import './interfaces/IUniswapV2Factory.sol';
+import './interfaces/IUniswapV2Callee.sol';
 
 contract UniswapV2Exchange is IUniswapV2Exchange, UniswapV2ERC20 {
     using SafeMath  for uint;
@@ -49,13 +50,21 @@ contract UniswapV2Exchange is IUniswapV2Exchange, UniswapV2ERC20 {
 
     event Mint(address indexed sender, uint amount0, uint amount1);
     event Burn(address indexed sender, uint amount0, uint amount1, address indexed to);
-    event Swap(address indexed sender, address indexed tokenIn, uint amountIn, uint amountOut, address indexed to);
+    event Swap(
+        address indexed sender,
+        uint amount0In,
+        uint amount1In,
+        uint amount0Out,
+        uint amount1Out,
+        address indexed to
+    );
     event Sync(uint112 reserve0, uint112 reserve1);
 
     constructor() public {
         factory = msg.sender;
     }
 
+    // called once by the factory at time of deployment
     function initialize(address _token0, address _token1) external {
         require(msg.sender == factory, 'UniswapV2: FORBIDDEN'); // sufficient check
         token0 = _token0;
@@ -99,6 +108,7 @@ contract UniswapV2Exchange is IUniswapV2Exchange, UniswapV2ERC20 {
         }
     }
 
+    // this low-level function should be called from a contract which performs important safety checks
     function mint(address to) external lock returns (uint liquidity) {
         (uint112 _reserve0, uint112 _reserve1,) = getReserves(); // gas savings
         uint balance0 = IERC20(token0).balanceOf(address(this));
@@ -118,10 +128,11 @@ contract UniswapV2Exchange is IUniswapV2Exchange, UniswapV2ERC20 {
         _mint(to, liquidity);
 
         _update(balance0, balance1, _reserve0, _reserve1);
-        if (feeOn) kLast = uint(reserve0).mul(reserve1);
+        if (feeOn) kLast = uint(reserve0).mul(reserve1); // reserve0 and reserve1 are up-to-date
         emit Mint(msg.sender, amount0, amount1);
     }
 
+    // this low-level function should be called from a contract which performs important safety checks
     function burn(address to) external lock returns (uint amount0, uint amount1) {
         (uint112 _reserve0, uint112 _reserve1,) = getReserves(); // gas savings
         address _token0 = token0;                                // gas savings
@@ -142,40 +153,39 @@ contract UniswapV2Exchange is IUniswapV2Exchange, UniswapV2ERC20 {
         balance1 = IERC20(_token1).balanceOf(address(this));
 
         _update(balance0, balance1, _reserve0, _reserve1);
-        if (feeOn) kLast = uint(reserve0).mul(reserve1);
+        if (feeOn) kLast = uint(reserve0).mul(reserve1); // reserve0 and reserve1 are up-to-date
         emit Burn(msg.sender, amount0, amount1, to);
     }
 
-    function swap(address tokenIn, uint amountOut, address to) external lock {
-        require(amountOut > 0, 'UniswapV2: INSUFFICIENT_OUTPUT_AMOUNT');
+    // this low-level function should be called from a contract which performs important safety checks
+    function swap(uint amount0Out, uint amount1Out, address to, bytes calldata data) external lock {
+        require(amount0Out > 0 || amount1Out > 0, 'UniswapV2: INSUFFICIENT_OUTPUT_AMOUNT');
         (uint112 _reserve0, uint112 _reserve1,) = getReserves(); // gas savings
-        address _token0 = token0;                                // gas savings
-        address _token1 = token1;                                // gas savings
+        require(amount0Out < _reserve0 && amount1Out < _reserve1, 'UniswapV2: INSUFFICIENT_LIQUIDITY');
+
         uint balance0;
         uint balance1;
-        uint amountIn;
-
-        if (tokenIn == _token0) {
-            require(amountOut < _reserve1, 'UniswapV2: INSUFFICIENT_LIQUIDITY');
-            balance0 = IERC20(_token0).balanceOf(address(this));
-            amountIn = balance0.sub(_reserve0);
-            require(amountIn > 0, 'UniswapV2: INSUFFICIENT_INPUT_AMOUNT');
-            require(amountIn.mul(_reserve1 - amountOut).mul(997) >= amountOut.mul(_reserve0).mul(1000), 'UniswapV2: K');
-            _safeTransfer(_token1, to, amountOut);
-            balance1 = IERC20(_token1).balanceOf(address(this));
-        } else {
-            require(tokenIn == _token1, 'UniswapV2: INVALID_INPUT_TOKEN');
-            require(amountOut < _reserve0, 'UniswapV2: INSUFFICIENT_LIQUIDITY');
-            balance1 = IERC20(_token1).balanceOf(address(this));
-            amountIn = balance1.sub(_reserve1);
-            require(amountIn > 0, 'UniswapV2: INSUFFICIENT_INPUT_AMOUNT');
-            require(amountIn.mul(_reserve0 - amountOut).mul(997) >= amountOut.mul(_reserve1).mul(1000), 'UniswapV2: K');
-            _safeTransfer(_token0, to, amountOut);
-            balance0 = IERC20(_token0).balanceOf(address(this));
+        { // scope for _token{0,1}, avoids stack too deep errors
+        address _token0 = token0;
+        address _token1 = token1;
+        require(to != _token0 && to != _token1, 'UniswapV2: INVALID_TO');
+        if (amount0Out > 0) _safeTransfer(_token0, to, amount0Out); // optimistically transfer tokens
+        if (amount1Out > 0) _safeTransfer(_token1, to, amount1Out); // optimistically transfer tokens
+        if (data.length > 0) IUniswapV2Callee(to).uniswapV2Call(msg.sender, amount0Out, amount1Out, data);
+        balance0 = IERC20(_token0).balanceOf(address(this));
+        balance1 = IERC20(_token1).balanceOf(address(this));
+        }
+        uint amount0In = balance0 > _reserve0 - amount0Out ? balance0 - (_reserve0 - amount0Out) : 0;
+        uint amount1In = balance1 > _reserve1 - amount1Out ? balance1 - (_reserve1 - amount1Out) : 0;
+        require(amount0In > 0 || amount1In > 0, 'UniswapV2: INSUFFICIENT_INPUT_AMOUNT');
+        { // scope for reserve{0,1}Adjusted, avoids stack too deep errors
+        uint balance0Adjusted = balance0.mul(1000).sub(amount0In.mul(3));
+        uint balance1Adjusted = balance1.mul(1000).sub(amount1In.mul(3));
+        require(balance0Adjusted.mul(balance1Adjusted) >= uint(_reserve0).mul(_reserve1).mul(1000**2), 'UniswapV2: K');
         }
 
         _update(balance0, balance1, _reserve0, _reserve1);
-        emit Swap(msg.sender, tokenIn, amountIn, amountOut, to);
+        emit Swap(msg.sender, amount0In, amount1In, amount0Out, amount1Out, to);
     }
 
     // force balances to match reserves

contracts/interfaces/IUniswapV2Callee.sol

@@ -0,0 +1,5 @@
+pragma solidity =0.5.16;
+
+interface IUniswapV2Callee {
+    function uniswapV2Call(address sender, uint amount0, uint amount1, bytes calldata data) external;
+}

contracts/interfaces/IUniswapV2Exchange.sol

@@ -23,7 +23,14 @@ interface IUniswapV2Exchange {
 
     event Mint(address indexed sender, uint amount0, uint amount1);
     event Burn(address indexed sender, uint amount0, uint amount1, address indexed to);
-    event Swap(address indexed sender, address indexed tokenIn, uint amountIn, uint amountOut, address indexed to);
+    event Swap(
+        address indexed sender,
+        uint amount0In,
+        uint amount1In,
+        uint amount0Out,
+        uint amount1Out,
+        address indexed to
+    );
     event Sync(uint112 reserve0, uint112 reserve1);
 
     function MINIMUM_LIQUIDITY() external pure returns (uint);
@@ -37,7 +44,7 @@ interface IUniswapV2Exchange {
 
     function mint(address to) external returns (uint liquidity);
     function burn(address to) external returns (uint amount0, uint amount1);
-    function swap(address tokenIn, uint amountOut, address to) external;
+    function swap(uint amount0Out, uint amount1Out, address to, bytes calldata data) external;
     function skim(address to) external;
     function sync() external;
 

test/UniswapV2Exchange.spec.ts

@@ -12,7 +12,7 @@ const MINIMUM_LIQUIDITY = bigNumberify(10).pow(3)
 chai.use(solidity)
 
 const overrides = {
-  gasLimit: 1000000
+  gasLimit: 9999999
 }
 
 describe('UniswapV2Exchange', () => {
@@ -68,7 +68,7 @@ describe('UniswapV2Exchange', () => {
     await token1.transfer(exchange.address, token1Amount)
     await exchange.mint(wallet.address, overrides)
   }
-  const testCases: BigNumber[][] = [
+  const swapTestCases: BigNumber[][] = [
     [1, 5, 10, '1662497915624478906'],
     [1, 10, 5, '453305446940074565'],
 
@@ -78,15 +78,34 @@ describe('UniswapV2Exchange', () => {
     [1, 10, 10, '906610893880149131'],
     [1, 100, 100, '987158034397061298'],
     [1, 1000, 1000, '996006981039903216']
-  ].map(a => a.map((n, i) => (i === 3 ? bigNumberify(n) : expandTo18Decimals(n as number))))
-  testCases.forEach((testCase, i) => {
+  ].map(a => a.map(n => (typeof n === 'string' ? bigNumberify(n) : expandTo18Decimals(n))))
+  swapTestCases.forEach((swapTestCase, i) => {
     it(`getInputPrice:${i}`, async () => {
-      await addLiquidity(testCase[1], testCase[2])
-      await token0.transfer(exchange.address, testCase[0])
-      await expect(exchange.swap(token0.address, testCase[3].add(1), wallet.address, overrides)).to.be.revertedWith(
+      const [swapAmount, token0Amount, token1Amount, expectedOutputAmount] = swapTestCase
+      await addLiquidity(token0Amount, token1Amount)
+      await token0.transfer(exchange.address, swapAmount)
+      await expect(exchange.swap(0, expectedOutputAmount.add(1), wallet.address, '0x', overrides)).to.be.revertedWith(
         'UniswapV2: K'
       )
-      await exchange.swap(token0.address, testCase[3], wallet.address, overrides)
+      await exchange.swap(0, expectedOutputAmount, wallet.address, '0x', overrides)
+    })
+  })
+
+  const optimisticTestCases: BigNumber[][] = [
+    ['997000000000000000', 5, 10, 1], // given amountIn, amountOut = floor(amountIn * .997)
+    ['997000000000000000', 10, 5, 1],
+    ['997000000000000000', 5, 5, 1],
+    [1, 5, 5, '1003009027081243732'] // given amountOut, amountIn = ceiling(amountOut / .997)
+  ].map(a => a.map(n => (typeof n === 'string' ? bigNumberify(n) : expandTo18Decimals(n))))
+  optimisticTestCases.forEach((optimisticTestCase, i) => {
+    it(`optimistic:${i}`, async () => {
+      const [outputAmount, token0Amount, token1Amount, inputAmount] = optimisticTestCase
+      await addLiquidity(token0Amount, token1Amount)
+      await token0.transfer(exchange.address, inputAmount)
+      await expect(exchange.swap(outputAmount.add(1), 0, wallet.address, '0x', overrides)).to.be.revertedWith(
+        'UniswapV2: K'
+      )
+      await exchange.swap(outputAmount, 0, wallet.address, '0x', overrides)
     })
   })
 
@@ -98,13 +117,13 @@ describe('UniswapV2Exchange', () => {
     const swapAmount = expandTo18Decimals(1)
     const expectedOutputAmount = bigNumberify('1662497915624478906')
     await token0.transfer(exchange.address, swapAmount)
-    await expect(exchange.swap(token0.address, expectedOutputAmount, wallet.address, overrides))
+    await expect(exchange.swap(0, expectedOutputAmount, wallet.address, '0x', overrides))
       .to.emit(token1, 'Transfer')
       .withArgs(exchange.address, wallet.address, expectedOutputAmount)
       .to.emit(exchange, 'Sync')
       .withArgs(token0Amount.add(swapAmount), token1Amount.sub(expectedOutputAmount))
       .to.emit(exchange, 'Swap')
-      .withArgs(wallet.address, token0.address, swapAmount, expectedOutputAmount, wallet.address)
+      .withArgs(wallet.address, swapAmount, 0, 0, expectedOutputAmount, wallet.address)
 
     const reserves = await exchange.getReserves()
     expect(reserves[0]).to.eq(token0Amount.add(swapAmount))
@@ -125,13 +144,13 @@ describe('UniswapV2Exchange', () => {
     const swapAmount = expandTo18Decimals(1)
     const expectedOutputAmount = bigNumberify('453305446940074565')
     await token1.transfer(exchange.address, swapAmount)
-    await expect(exchange.swap(token1.address, expectedOutputAmount, wallet.address, overrides))
+    await expect(exchange.swap(expectedOutputAmount, 0, wallet.address, '0x', overrides))
       .to.emit(token0, 'Transfer')
       .withArgs(exchange.address, wallet.address, expectedOutputAmount)
       .to.emit(exchange, 'Sync')
       .withArgs(token0Amount.sub(expectedOutputAmount), token1Amount.add(swapAmount))
       .to.emit(exchange, 'Swap')
-      .withArgs(wallet.address, token1.address, swapAmount, expectedOutputAmount, wallet.address)
+      .withArgs(wallet.address, 0, swapAmount, expectedOutputAmount, 0, wallet.address)
 
     const reserves = await exchange.getReserves()
     expect(reserves[0]).to.eq(token0Amount.sub(expectedOutputAmount))
@@ -157,7 +176,7 @@ describe('UniswapV2Exchange', () => {
     const expectedOutputAmount = bigNumberify('453305446940074565')
     await token0.transfer(exchange.address, swapAmount)
     await mineBlock(provider, (await provider.getBlock('latest')).timestamp + 1)
-    const gasCost = await exchange.estimate.swap(token0.address, expectedOutputAmount, wallet.address, overrides)
+    const gasCost = await exchange.estimate.swap(0, expectedOutputAmount, wallet.address, '0x', overrides)
     console.log(`Gas required for swap: ${gasCost}`)
   })
 
@@ -209,7 +228,7 @@ describe('UniswapV2Exchange', () => {
     await token0.transfer(exchange.address, swapAmount)
     await mineBlock(provider, blockTimestamp + 10)
     // swap to a new price eagerly instead of syncing
-    await exchange.swap(token0.address, expandTo18Decimals(1), wallet.address, overrides) // make the price nice
+    await exchange.swap(0, expandTo18Decimals(1), wallet.address, '0x', overrides) // make the price nice
 
     expect(await exchange.price0CumulativeLast()).to.eq(initialPrice[0].mul(10))
     expect(await exchange.price1CumulativeLast()).to.eq(initialPrice[1].mul(10))
@@ -232,7 +251,7 @@ describe('UniswapV2Exchange', () => {
     const swapAmount = expandTo18Decimals(1)
     const expectedOutputAmount = bigNumberify('996006981039903216')
     await token1.transfer(exchange.address, swapAmount)
-    await exchange.swap(token1.address, expectedOutputAmount, wallet.address, overrides)
+    await exchange.swap(expectedOutputAmount, 0, wallet.address, '0x', overrides)
 
     const expectedLiquidity = expandTo18Decimals(1000)
     await exchange.transfer(exchange.address, expectedLiquidity.sub(MINIMUM_LIQUIDITY))
@@ -250,7 +269,7 @@ describe('UniswapV2Exchange', () => {
     const swapAmount = expandTo18Decimals(1)
     const expectedOutputAmount = bigNumberify('996006981039903216')
     await token1.transfer(exchange.address, swapAmount)
-    await exchange.swap(token1.address, expectedOutputAmount, wallet.address, overrides)
+    await exchange.swap(expectedOutputAmount, 0, wallet.address, '0x', overrides)
 
     const expectedLiquidity = expandTo18Decimals(1000)
     await exchange.transfer(exchange.address, expectedLiquidity.sub(MINIMUM_LIQUIDITY))

test/shared/fixtures.ts

@@ -12,8 +12,12 @@ interface FactoryFixture {
   factory: Contract
 }
 
+const overrides = {
+  gasLimit: 9999999
+}
+
 export async function factoryFixture(_: Web3Provider, [wallet]: Wallet[]): Promise<FactoryFixture> {
-  const factory = await deployContract(wallet, UniswapV2Factory, [wallet.address])
+  const factory = await deployContract(wallet, UniswapV2Factory, [wallet.address], overrides)
   return { factory }
 }
 
@@ -26,10 +30,10 @@ interface ExchangeFixture extends FactoryFixture {
 export async function exchangeFixture(provider: Web3Provider, [wallet]: Wallet[]): Promise<ExchangeFixture> {
   const { factory } = await factoryFixture(provider, [wallet])
 
-  const tokenA = await deployContract(wallet, ERC20, [expandTo18Decimals(10000)])
-  const tokenB = await deployContract(wallet, ERC20, [expandTo18Decimals(10000)])
+  const tokenA = await deployContract(wallet, ERC20, [expandTo18Decimals(10000)], overrides)
+  const tokenB = await deployContract(wallet, ERC20, [expandTo18Decimals(10000)], overrides)
 
-  await factory.createExchange(tokenA.address, tokenB.address)
+  await factory.createExchange(tokenA.address, tokenB.address, overrides)
   const exchangeAddress = await factory.getExchange(tokenA.address, tokenB.address)
   const exchange = new Contract(exchangeAddress, JSON.stringify(UniswapV2Exchange.abi), provider).connect(wallet)