Merge pull request #19 from rainprotocol/2023-09-01-simplify-take-orders

remove input and output from take orders
rainlanguage · Sep 2, 2023 · fdb8424 · fdb8424
2 parents 8f9a582 + 3528573
commit fdb8424
Show file tree

Hide file tree

Showing 9 changed files with 150 additions and 100 deletions.
diff --git a/.gas-snapshot b/.gas-snapshot
@@ -1,47 +1,47 @@
-GenericPoolOrderBookFlashBorrowerTest:testMinimumOutput(uint256,uint256) (runs: 5096, μ: 2326260, ~: 2329525)
-GenericPoolOrderBookFlashBorrowerTest:testTakeOrdersSender() (gas: 2241348)
+GenericPoolOrderBookV3FlashBorrowerTest:testMinimumOutput((address,bool,(address,address,address),(address,uint8,uint256)[],(address,uint8,uint256)[]),uint256,uint256,uint256,uint256) (runs: 5096, μ: 2778300, ~: 2777328)
+GenericPoolOrderBookV3FlashBorrowerTest:testTakeOrdersSender((address,bool,(address,address,address),(address,uint8,uint256)[],(address,uint8,uint256)[]),uint256,uint256) (runs: 5096, μ: 2701886, ~: 2702333)
 LibOrderTest:testHashEqual((address,bool,(address,address,address),(address,uint8,uint256)[],(address,uint8,uint256)[])) (runs: 5096, μ: 194389, ~: 191140)
 LibOrderTest:testHashNotEqual((address,bool,(address,address,address),(address,uint8,uint256)[],(address,uint8,uint256)[]),(address,bool,(address,address,address),(address,uint8,uint256)[],(address,uint8,uint256)[])) (runs: 5096, μ: 298734, ~: 298554)
 OrderBookAddOrderMockTest:testAddOrderSameAccountWithDifferentConfig(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address,address) (runs: 5096, μ: 2771808, ~: 2761475)
-OrderBookAddOrderMockTest:testAddOrderTwoAccountsWithDifferentConfig(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address,address) (runs: 5096, μ: 2626700, ~: 2600883)
-OrderBookAddOrderMockTest:testAddOrderTwoAccountsWithSameConfig(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 2493275, ~: 2492266)
+OrderBookAddOrderMockTest:testAddOrderTwoAccountsWithDifferentConfig(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address,address) (runs: 5096, μ: 2626737, ~: 2600883)
+OrderBookAddOrderMockTest:testAddOrderTwoAccountsWithSameConfig(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 2492972, ~: 2492266)
 OrderBookAddOrderMockTest:testAddOrderWithCalculationsInputsAndOutputsSucceeds(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 1295793, ~: 1281958)
-OrderBookAddOrderMockTest:testAddOrderWithNonEmptyMetaEmitsMetaV1(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 1303882, ~: 1290859)
+OrderBookAddOrderMockTest:testAddOrderWithNonEmptyMetaEmitsMetaV1(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 1303904, ~: 1290881)
 OrderBookAddOrderMockTest:testAddOrderWithNonEmptyMetaReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 703690, ~: 697922)
 OrderBookAddOrderMockTest:testAddOrderWithoutCalculationsReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes)) (runs: 5096, μ: 226595, ~: 224802)
 OrderBookAddOrderMockTest:testAddOrderWithoutInputsReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes)) (runs: 5096, μ: 182418, ~: 181902)
 OrderBookAddOrderMockTest:testAddOrderWithoutOutputsReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes)) (runs: 5096, μ: 182840, ~: 182026)
-OrderBookAddOrderTest:testAddOrderRealNoHandleIOReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes)) (runs: 5096, μ: 172765, ~: 171541)
+OrderBookAddOrderTest:testAddOrderRealNoHandleIOReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes)) (runs: 5096, μ: 172787, ~: 171563)
 OrderBookAddOrderTest:testAddOrderRealNoSourcesReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes)) (runs: 5096, μ: 176045, ~: 174413)
-OrderBookAddOrderTest:testAddOrderRealOneStackCalculateReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes)) (runs: 5096, μ: 186108, ~: 184885)
+OrderBookAddOrderTest:testAddOrderRealOneStackCalculateReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes)) (runs: 5096, μ: 186130, ~: 184907)
 OrderBookAddOrderTest:testAddOrderRealThreeStackCalculate(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes)) (runs: 5096, μ: 722153, ~: 718798)
-OrderBookAddOrderTest:testAddOrderRealTwoStackCalculateReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes)) (runs: 5096, μ: 715304, ~: 711949)
+OrderBookAddOrderTest:testAddOrderRealTwoStackCalculateReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes)) (runs: 5096, μ: 715326, ~: 711971)
 OrderBookAddOrderTest:testAddOrderRealZeroStackCalculateReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes)) (runs: 5096, μ: 181302, ~: 180079)
-OrderBookDepositTest:testDepositEvent(address,uint256,uint256) (runs: 5096, μ: 38755, ~: 38755)
-OrderBookDepositTest:testDepositFail(address,uint256,uint256) (runs: 5096, μ: 8937393460516740789, ~: 8937393460516740786)
+OrderBookDepositTest:testDepositEvent(address,uint256,uint256) (runs: 5096, μ: 38710, ~: 38710)
+OrderBookDepositTest:testDepositFail(address,uint256,uint256) (runs: 5096, μ: 8937393460516740787, ~: 8937393460516740786)
 OrderBookDepositTest:testDepositGas00() (gas: 8176)
 OrderBookDepositTest:testDepositGas01() (gas: 34620)
-OrderBookDepositTest:testDepositMany((address,address,uint256,uint248)[]) (runs: 5096, μ: 5062646, ~: 4709523)
+OrderBookDepositTest:testDepositMany((address,address,uint256,uint248)[]) (runs: 5096, μ: 5162681, ~: 4894479)
 OrderBookDepositTest:testDepositOverflow(address,uint256,uint256,uint256) (runs: 5096, μ: 46645, ~: 46645)
-OrderBookDepositTest:testDepositReentrancy(address,uint256,uint256,address,uint256,uint256) (runs: 5096, μ: 495280, ~: 496632)
+OrderBookDepositTest:testDepositReentrancy(address,uint256,uint256,address,uint256,uint256) (runs: 5096, μ: 495210, ~: 496632)
 OrderBookDepositTest:testDepositSimple(address,uint256,uint256) (runs: 5096, μ: 37840, ~: 37840)
 OrderBookDepositTest:testDepositZero(address,uint256) (runs: 5096, μ: 12639, ~: 12639)
 OrderBookDepositTest:testVaultBalanceNoDeposits(address,uint256) (runs: 5096, μ: 8263, ~: 8263)
-OrderBookDepositTest:testVaultBalanceReentrant(address,uint256,uint256,address,address,uint256) (runs: 5096, μ: 494210, ~: 495964)
-OrderBookRemoveOrderMockTest:testRemoveOrderAddRemoveMulti(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 7256892, ~: 7139695)
-OrderBookRemoveOrderMockTest:testRemoveOrderDifferent(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 5070545, ~: 5046552)
-OrderBookRemoveOrderMockTest:testRemoveOrderDifferentOwners(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 4854493, ~: 4842171)
-OrderBookRemoveOrderMockTest:testRemoveOrderDifferentOwnersDifferent(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 10468846, ~: 10456300)
-OrderBookRemoveOrderMockTest:testRemoveOrderDoesNotExist(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 386856, ~: 382108)
-OrderBookRemoveOrderMockTest:testRemoveOrderOnlyOwner(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 2603327, ~: 2601305)
-OrderBookTakeOrderNoopTest:testTakeOrderNoopNonLiveOrder(address,address,(address,bool,(address,address,address),(address,uint8,uint256)[],(address,uint8,uint256)[]),uint256,uint256,(address,uint256[],bytes)) (runs: 5096, μ: 416618, ~: 413322)
-OrderBookTakeOrderNoopTest:testTakeOrderNoopNonLiveOrderTwo(address,address,(address,bool,(address,address,address),(address,uint8,uint256)[],(address,uint8,uint256)[]),(address,bool,(address,address,address),(address,uint8,uint256)[],(address,uint8,uint256)[]),uint256,uint256,uint256,uint256,(address,uint256[],bytes),(address,uint256[],bytes)) (runs: 5096, μ: 825135, ~: 821027)
-OrderBookTakeOrderNoopTest:testTakeOrderNoopZeroOrders(address,address) (runs: 5096, μ: 12738, ~: 12738)
-OrderBookTakeOrderTest:testTakeOrderPrecisionKnownBad01() (gas: 2559221)
-OrderBookWithdrawTest:testWithdrawEmptyVault(address,address,uint256,uint256) (runs: 5096, μ: 15316, ~: 15316)
-OrderBookWithdrawTest:testWithdrawFailure(address,uint256,uint256,uint256) (runs: 5096, μ: 8937393460516719573, ~: 8937393460516700429)
-OrderBookWithdrawTest:testWithdrawFullVault(address,uint256,uint256,uint256) (runs: 5096, μ: 41258, ~: 41256)
-OrderBookWithdrawTest:testWithdrawMany((bool,address,address,uint256,uint248)[]) (runs: 5096, μ: 1440671, ~: 1360132)
+OrderBookDepositTest:testVaultBalanceReentrant(address,uint256,uint256,address,address,uint256) (runs: 5096, μ: 494148, ~: 495964)
+OrderBookRemoveOrderMockTest:testRemoveOrderAddRemoveMulti(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 7256622, ~: 7139425)
+OrderBookRemoveOrderMockTest:testRemoveOrderDifferent(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 5070365, ~: 5046372)
+OrderBookRemoveOrderMockTest:testRemoveOrderDifferentOwners(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 4854788, ~: 4841901)
+OrderBookRemoveOrderMockTest:testRemoveOrderDifferentOwnersDifferent(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 10466985, ~: 10453491)
+OrderBookRemoveOrderMockTest:testRemoveOrderDoesNotExist(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 386811, ~: 382063)
+OrderBookRemoveOrderMockTest:testRemoveOrderOnlyOwner(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes),address) (runs: 5096, μ: 2603276, ~: 2601080)
+OrderBookTakeOrderNoopTest:testTakeOrderNoopNonLiveOrderOne((address,bool,(address,address,address),(address,uint8,uint256)[],(address,uint8,uint256)[]),uint256,uint256,(address,uint256[],bytes)) (runs: 5096, μ: 427777, ~: 424024)
+OrderBookTakeOrderNoopTest:testTakeOrderNoopNonLiveOrderTwo((address,bool,(address,address,address),(address,uint8,uint256)[],(address,uint8,uint256)[]),(address,bool,(address,address,address),(address,uint8,uint256)[],(address,uint8,uint256)[]),uint256,uint256,uint256,uint256,(address,uint256[],bytes),(address,uint256[],bytes)) (runs: 5096, μ: 841710, ~: 837539)
+OrderBookTakeOrderNoopTest:testTakeOrderNoopZeroOrders() (gas: 12403)
+OrderBookTakeOrderTest:testTakeOrderPrecisionKnownBad01() (gas: 2576588)
+OrderBookWithdrawTest:testWithdrawEmptyVault(address,address,uint256,uint256) (runs: 5096, μ: 15251, ~: 15251)
+OrderBookWithdrawTest:testWithdrawFailure(address,uint256,uint256,uint256) (runs: 5096, μ: 8937393460516719305, ~: 8937393460516700418)
+OrderBookWithdrawTest:testWithdrawFullVault(address,uint256,uint256,uint256) (runs: 5096, μ: 41257, ~: 41254)
+OrderBookWithdrawTest:testWithdrawMany((bool,address,address,uint256,uint248)[]) (runs: 5096, μ: 1447310, ~: 1371034)
 OrderBookWithdrawTest:testWithdrawPartialVault(address,uint256,uint256,uint256) (runs: 5096, μ: 51929, ~: 51929)
-OrderBookWithdrawTest:testWithdrawReentrant(address,uint256,uint256,address,address,uint256) (runs: 5096, μ: 506270, ~: 507997)
-OrderBookWithdrawTest:testWithdrawZero(address,address,uint256) (runs: 5096, μ: 12787, ~: 12787)
+OrderBookWithdrawTest:testWithdrawReentrant(address,uint256,uint256,address,address,uint256) (runs: 5096, μ: 506196, ~: 507997)
+OrderBookWithdrawTest:testWithdrawZero(address,address,uint256) (runs: 5096, μ: 12809, ~: 12809)
diff --git a/src/abstract/OrderBookV3ArbOrderTaker.sol b/src/abstract/OrderBookV3ArbOrderTaker.sol
@@ -127,6 +127,14 @@ abstract contract OrderBookV3ArbOrderTaker is
         nonReentrant
         onlyNotInitializing
     {
+        // Mimic what OB would do anyway if called with zero orders.
+        if (takeOrders.orders.length == 0) {
+            revert NoOrders();
+        }
+
+        address ordersInputToken = takeOrders.orders[0].order.validInputs[takeOrders.orders[0].inputIOIndex].token;
+        address ordersOutputToken = takeOrders.orders[0].order.validOutputs[takeOrders.orders[0].outputIOIndex].token;
+
         // Run the access control dispatch if it is set.
         EncodedDispatch dispatch = sI9rDispatch;
         if (EncodedDispatch.unwrap(dispatch) > 0) {
@@ -146,24 +154,24 @@ abstract contract OrderBookV3ArbOrderTaker is
             }
         }
 
-        IERC20(takeOrders.output).safeApprove(address(sOrderBook), 0);
-        IERC20(takeOrders.output).safeApprove(address(sOrderBook), type(uint256).max);
+        IERC20(ordersInputToken).safeApprove(address(sOrderBook), 0);
+        IERC20(ordersInputToken).safeApprove(address(sOrderBook), type(uint256).max);
         (uint256 totalInput, uint256 totalOutput) = sOrderBook.takeOrders(takeOrders);
         (totalInput, totalOutput);
-        IERC20(takeOrders.output).safeApprove(address(sOrderBook), 0);
+        IERC20(ordersInputToken).safeApprove(address(sOrderBook), 0);
 
         // Send all unspent input tokens to the sender.
-        uint256 inputBalance = IERC20(takeOrders.input).balanceOf(address(this));
+        uint256 inputBalance = IERC20(ordersInputToken).balanceOf(address(this));
+        if (inputBalance < minimumSenderOutput) {
+            revert MinimumOutput(minimumSenderOutput, inputBalance);
+        }
         if (inputBalance > 0) {
-            IERC20(takeOrders.input).safeTransfer(msg.sender, inputBalance);
+            IERC20(ordersInputToken).safeTransfer(msg.sender, inputBalance);
         }
         // Send all unspent output tokens to the sender.
-        uint256 outputBalance = IERC20(takeOrders.output).balanceOf(address(this));
-        if (outputBalance < minimumSenderOutput) {
-            revert MinimumOutput(minimumSenderOutput, outputBalance);
-        }
+        uint256 outputBalance = IERC20(ordersOutputToken).balanceOf(address(this));
         if (outputBalance > 0) {
-            IERC20(takeOrders.output).safeTransfer(msg.sender, outputBalance);
+            IERC20(ordersOutputToken).safeTransfer(msg.sender, outputBalance);
         }
 
         // Send any remaining gas to the sender.

diff --git a/src/abstract/OrderBookV3FlashBorrower.sol b/src/abstract/OrderBookV3FlashBorrower.sol
@@ -252,11 +252,18 @@ abstract contract OrderBookV3FlashBorrower is
         nonReentrant
         onlyNotInitializing
     {
+        // Mimic what OB would do anyway if called with zero orders.
+        if (takeOrders.orders.length == 0) {
+            revert NoOrders();
+        }
+
         // Encode everything that will be used by the flash loan callback.
         bytes memory data = abi.encode(takeOrders, exchangeData);
         // The token we receive from taking the orders is what we will use to
         // repay the flash loan.
-        address flashLoanToken = takeOrders.input;
+        address ordersOutputToken = takeOrders.orders[0].order.validOutputs[takeOrders.orders[0].outputIOIndex].token;
+        address ordersInputToken = takeOrders.orders[0].order.validInputs[takeOrders.orders[0].inputIOIndex].token;
+
         // We can't repay more than the minimum that the orders are going to
         // give us and there's no reason to borrow less.
         uint256 flashLoanAmount = takeOrders.minimumInput;
@@ -283,25 +290,25 @@ abstract contract OrderBookV3FlashBorrower is
         // Take the flash loan, which will in turn call `onFlashLoan`, which is
         // expected to process an exchange against external liq to pay back the
         // flash loan, cover the orders and remain in profit.
-        IERC20(takeOrders.output).safeApprove(address(sOrderBook), 0);
-        IERC20(takeOrders.output).safeApprove(address(sOrderBook), type(uint256).max);
-        if (!sOrderBook.flashLoan(this, flashLoanToken, flashLoanAmount, data)) {
+        IERC20(ordersInputToken).safeApprove(address(sOrderBook), 0);
+        IERC20(ordersInputToken).safeApprove(address(sOrderBook), type(uint256).max);
+        if (!sOrderBook.flashLoan(this, ordersOutputToken, flashLoanAmount, data)) {
             revert FlashLoanFailed();
         }
-        IERC20(takeOrders.output).safeApprove(address(sOrderBook), 0);
+        IERC20(ordersInputToken).safeApprove(address(sOrderBook), 0);
 
         // Send all unspent input tokens to the sender.
-        uint256 inputBalance = IERC20(takeOrders.input).balanceOf(address(this));
+        uint256 inputBalance = IERC20(ordersInputToken).balanceOf(address(this));
+        if (inputBalance < minimumSenderOutput) {
+            revert MinimumOutput(minimumSenderOutput, inputBalance);
+        }
         if (inputBalance > 0) {
-            IERC20(takeOrders.input).safeTransfer(msg.sender, inputBalance);
+            IERC20(ordersInputToken).safeTransfer(msg.sender, inputBalance);
         }
         // Send all unspent output tokens to the sender.
-        uint256 outputBalance = IERC20(takeOrders.output).balanceOf(address(this));
-        if (outputBalance < minimumSenderOutput) {
-            revert MinimumOutput(minimumSenderOutput, outputBalance);
-        }
+        uint256 outputBalance = IERC20(ordersOutputToken).balanceOf(address(this));
         if (outputBalance > 0) {
-            IERC20(takeOrders.output).safeTransfer(msg.sender, outputBalance);
+            IERC20(ordersOutputToken).safeTransfer(msg.sender, outputBalance);
         }
 
         // Send any remaining gas to the sender.

diff --git a/src/concrete/GenericPoolOrderBookV3FlashBorrower.sol b/src/concrete/GenericPoolOrderBookV3FlashBorrower.sol
@@ -34,12 +34,14 @@ contract GenericPoolOrderBookV3FlashBorrower is OrderBookV3FlashBorrower {
         (address spender, address pool, bytes memory encodedFunctionCall) =
             abi.decode(exchangeData, (address, address, bytes));
 
-        IERC20(takeOrders.input).safeApprove(spender, 0);
-        IERC20(takeOrders.input).safeApprove(spender, type(uint256).max);
+        address borrowedToken = takeOrders.orders[0].order.validOutputs[takeOrders.orders[0].outputIOIndex].token;
+
+        IERC20(borrowedToken).safeApprove(spender, 0);
+        IERC20(borrowedToken).safeApprove(spender, type(uint256).max);
         bytes memory returnData = pool.functionCallWithValue(encodedFunctionCall, address(this).balance);
         // Nothing can be done with returnData as 3156 does not support it.
         (returnData);
-        IERC20(takeOrders.input).safeApprove(spender, 0);
+        IERC20(borrowedToken).safeApprove(spender, 0);
     }
 
     /// Allow receiving gas.