diff --git a/.gas-snapshot b/.gas-snapshot index 24928c15b..2a7a1e9f3 100644 --- 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) \ No newline at end of file +OrderBookWithdrawTest:testWithdrawReentrant(address,uint256,uint256,address,address,uint256) (runs: 5096, μ: 506196, ~: 507997) +OrderBookWithdrawTest:testWithdrawZero(address,address,uint256) (runs: 5096, μ: 12809, ~: 12809) \ No newline at end of file diff --git a/src/abstract/OrderBookV3ArbOrderTaker.sol b/src/abstract/OrderBookV3ArbOrderTaker.sol index eb03b196d..5fd61f79a 100644 --- 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 index 3e466fdea..5849154e6 100644 --- 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 index 8d2aae95e..02691245c 100644 --- 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. diff --git a/src/concrete/OrderBook.sol b/src/concrete/OrderBook.sol index 0bdfc9386..8aee79b44 100644 --- a/src/concrete/OrderBook.sol +++ b/src/concrete/OrderBook.sol @@ -32,9 +32,6 @@ error ReentrancyGuardReentrantCall(); /// @param owner The owner of the order. error NotOrderOwner(address sender, address owner); -/// Thrown when take orders is called with no orders. -error NoOrders(); - /// Thrown when the input and output tokens don't match, in either direction. /// @param aliceToken The input or output of one order. /// @param bobToken The input or output of the other order that doesn't match a. @@ -345,6 +342,8 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash if (config.orders.length == 0) { revert NoOrders(); } + address expectedOrderInputToken = config.orders[0].order.validInputs[config.orders[0].inputIOIndex].token; + address expectedOrderOutputToken = config.orders[0].order.validOutputs[config.orders[0].outputIOIndex].token; uint256 i = 0; TakeOrderConfig memory takeOrderConfig; @@ -358,11 +357,13 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash if (sOrders[orderHash] == ORDER_DEAD) { emit OrderNotFound(msg.sender, order.owner, orderHash); } else { - if (order.validInputs[takeOrderConfig.inputIOIndex].token != config.output) { - revert TokenMismatch(order.validInputs[takeOrderConfig.inputIOIndex].token, config.output); + if (order.validInputs[takeOrderConfig.inputIOIndex].token != expectedOrderInputToken) { + revert TokenMismatch(order.validInputs[takeOrderConfig.inputIOIndex].token, expectedOrderInputToken); } - if (order.validOutputs[takeOrderConfig.outputIOIndex].token != config.input) { - revert TokenMismatch(order.validOutputs[takeOrderConfig.outputIOIndex].token, config.input); + if (order.validOutputs[takeOrderConfig.outputIOIndex].token != expectedOrderOutputToken) { + revert TokenMismatch( + order.validOutputs[takeOrderConfig.outputIOIndex].token, expectedOrderOutputToken + ); } OrderIOCalculation memory orderIOCalculation = calculateOrderIO( @@ -444,10 +445,11 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash // trades, which is important if the order logic itself is dependent on // external data (e.g. prices) that could be modified by the caller's // trades. - uint256 inputAmountSent = _decreaseFlashDebtThenSendToken(config.input, msg.sender, totalTakerInput); + uint256 takerInputAmountSent = + _decreaseFlashDebtThenSendToken(expectedOrderOutputToken, msg.sender, totalTakerInput); if (config.data.length > 0) { IOrderBookV3OrderTaker(msg.sender).onTakeOrders( - config.input, config.output, inputAmountSent, totalTakerOutput, config.data + expectedOrderOutputToken, expectedOrderInputToken, takerInputAmountSent, totalTakerOutput, config.data ); } @@ -455,7 +457,7 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash // We already updated vault balances before we took tokens from // `msg.sender` which is usually NOT the correct order of operations for // depositing to a vault. We rely on reentrancy guards to make this safe. - IERC20(config.output).safeTransferFrom(msg.sender, address(this), totalTakerOutput); + IERC20(expectedOrderInputToken).safeTransferFrom(msg.sender, address(this), totalTakerOutput); } } diff --git a/src/interface/unstable/IOrderBookV3.sol b/src/interface/unstable/IOrderBookV3.sol index ac82b6d89..a5794046c 100644 --- a/src/interface/unstable/IOrderBookV3.sol +++ b/src/interface/unstable/IOrderBookV3.sol @@ -5,6 +5,9 @@ import "../ierc3156/IERC3156FlashLender.sol"; import "rain.interpreter/src/lib/caller/LibEvaluable.sol"; import "rain.interpreter/src/interface/IInterpreterCallerV2.sol"; +/// Thrown when take orders is called with no orders. +error NoOrders(); + /// Configuration for a single input or output on an `Order`. /// @param token The token to either send from the owner as an output or receive /// from the counterparty to the owner as an input. The tokens are not moved @@ -67,8 +70,6 @@ struct Order { /// Config for a list of orders to take sequentially as part of a `takeOrders` /// call. -/// @param output Output token from the perspective of the order taker. -/// @param input Input token from the perspective of the order taker. /// @param minimumInput Minimum input from the perspective of the order taker. /// @param maximumInput Maximum input from the perspective of the order taker. /// @param maximumIORatio Maximum IO ratio as calculated by the order being @@ -83,8 +84,6 @@ struct Order { /// onchain actions between receiving their input tokens, before having to send /// their output tokens. struct TakeOrdersConfigV2 { - address output; - address input; uint256 minimumInput; uint256 maximumInput; uint256 maximumIORatio; diff --git a/test/concrete/GenericPoolOrderBookFlashBorrower.sender.t.sol b/test/concrete/GenericPoolOrderBookV3FlashBorrower.sender.t.sol similarity index 63% rename from test/concrete/GenericPoolOrderBookFlashBorrower.sender.t.sol rename to test/concrete/GenericPoolOrderBookV3FlashBorrower.sender.t.sol index 33e5d9f06..5ca395df1 100644 --- a/test/concrete/GenericPoolOrderBookFlashBorrower.sender.t.sol +++ b/test/concrete/GenericPoolOrderBookV3FlashBorrower.sender.t.sol @@ -62,7 +62,7 @@ contract Mock0xProxy { } } -contract GenericPoolOrderBookFlashBorrowerTest is Test { +contract GenericPoolOrderBookV3FlashBorrowerTest is Test { address immutable deployer; address immutable implementation; @@ -86,38 +86,58 @@ contract GenericPoolOrderBookFlashBorrowerTest is Test { ); } - function testTakeOrdersSender() public { - MockOrderBook ob_ = new MockOrderBook(); - Mock0xProxy proxy_ = new Mock0xProxy(); + function testTakeOrdersSender(Order memory order, uint256 inputIOIndex, uint256 outputIOIndex) public { + vm.assume(order.validInputs.length > 0); + inputIOIndex = bound(inputIOIndex, 0, order.validInputs.length - 1); + vm.assume(order.validOutputs.length > 0); + outputIOIndex = bound(outputIOIndex, 0, order.validOutputs.length - 1); - Token input_ = new Token(); - Token output_ = new Token(); + MockOrderBook ob = new MockOrderBook(); + Mock0xProxy proxy = new Mock0xProxy(); - GenericPoolOrderBookV3FlashBorrower arb_ = GenericPoolOrderBookV3FlashBorrower(Clones.clone(implementation)); - arb_.initialize( + Token takerInput = new Token(); + Token takerOutput = new Token(); + + GenericPoolOrderBookV3FlashBorrower arb = GenericPoolOrderBookV3FlashBorrower(Clones.clone(implementation)); + arb.initialize( abi.encode( OrderBookV3FlashBorrowerConfigV2( - address(ob_), EvaluableConfigV2(IExpressionDeployerV2(address(0)), "", new uint256[](0)), "" + address(ob), EvaluableConfigV2(IExpressionDeployerV2(address(0)), "", new uint256[](0)), "" ) ) ); - arb_.arb( - TakeOrdersConfigV2( - address(output_), address(input_), 0, type(uint256).max, type(uint256).max, new TakeOrderConfig[](0), "" - ), + order.validInputs[inputIOIndex].token = address(takerOutput); + order.validOutputs[outputIOIndex].token = address(takerInput); + + TakeOrderConfig[] memory orders = new TakeOrderConfig[](1); + orders[0] = TakeOrderConfig(order, inputIOIndex, outputIOIndex, new SignedContextV1[](0)); + + arb.arb( + TakeOrdersConfigV2(0, type(uint256).max, type(uint256).max, orders, ""), 0, - abi.encode(address(proxy_), address(proxy_), "") + abi.encode(address(proxy), address(proxy), "") ); } - function testMinimumOutput(uint256 minimumOutput, uint256 mintAmount) public { + function testMinimumOutput( + Order memory order, + uint256 inputIOIndex, + uint256 outputIOIndex, + uint256 minimumOutput, + uint256 mintAmount + ) public { + vm.assume(order.validInputs.length > 0); + inputIOIndex = bound(inputIOIndex, 0, order.validInputs.length - 1); + vm.assume(order.validOutputs.length > 0); + outputIOIndex = bound(outputIOIndex, 0, order.validOutputs.length - 1); + vm.assume(minimumOutput > mintAmount); MockOrderBook ob = new MockOrderBook(); Mock0xProxy proxy = new Mock0xProxy(); - Token input = new Token(); - Token output = new Token(); + Token takerInput = new Token(); + Token takerOutput = new Token(); GenericPoolOrderBookV3FlashBorrower arb = GenericPoolOrderBookV3FlashBorrower(Clones.clone(implementation)); arb.initialize( @@ -128,13 +148,17 @@ contract GenericPoolOrderBookFlashBorrowerTest is Test { ) ); - output.mint(address(arb), mintAmount); + takerOutput.mint(address(arb), mintAmount); + + order.validInputs[inputIOIndex].token = address(takerOutput); + order.validOutputs[outputIOIndex].token = address(takerInput); + + TakeOrderConfig[] memory orders = new TakeOrderConfig[](1); + orders[0] = TakeOrderConfig(order, inputIOIndex, outputIOIndex, new SignedContextV1[](0)); vm.expectRevert(abi.encodeWithSelector(MinimumOutput.selector, minimumOutput, mintAmount)); arb.arb( - TakeOrdersConfigV2( - address(output), address(input), 0, type(uint256).max, type(uint256).max, new TakeOrderConfig[](0), "" - ), + TakeOrdersConfigV2(0, type(uint256).max, type(uint256).max, orders, ""), minimumOutput, abi.encode(address(proxy), address(proxy), "") ); diff --git a/test/concrete/OrderBook.takeOrder.noop.t.sol b/test/concrete/OrderBook.takeOrder.noop.t.sol index 038feffdb..97b61acf4 100644 --- a/test/concrete/OrderBook.takeOrder.noop.t.sol +++ b/test/concrete/OrderBook.takeOrder.noop.t.sol @@ -14,9 +14,9 @@ contract OrderBookTakeOrderNoopTest is OrderBookExternalRealTest { /// Take orders makes no sense without any orders in the input array and the /// caller has full control over this so we error. - function testTakeOrderNoopZeroOrders(address input, address output) external { + function testTakeOrderNoopZeroOrders() external { TakeOrdersConfigV2 memory config = - TakeOrdersConfigV2(output, input, 0, type(uint256).max, type(uint256).max, new TakeOrderConfig[](0), ""); + TakeOrdersConfigV2(0, type(uint256).max, type(uint256).max, new TakeOrderConfig[](0), ""); vm.expectRevert(NoOrders.selector); (uint256 totalTakerInput, uint256 totalTakerOutput) = iOrderbook.takeOrders(config); (totalTakerInput, totalTakerOutput); @@ -27,14 +27,16 @@ contract OrderBookTakeOrderNoopTest is OrderBookExternalRealTest { /// have been removed by its owner. We don't want to revert the whole /// transaction in this case as there may be other orders in the input array /// in the general case. - function testTakeOrderNoopNonLiveOrder( - address input, - address output, + function testTakeOrderNoopNonLiveOrderOne( Order memory order, uint256 inputIOIndex, uint256 outputIOIndex, SignedContextV1 memory signedContext ) external { + vm.assume(order.validInputs.length > 0); + inputIOIndex = bound(inputIOIndex, 0, order.validInputs.length - 1); + vm.assume(order.validOutputs.length > 0); + outputIOIndex = bound(outputIOIndex, 0, order.validOutputs.length - 1); // We don't bound the input or output indexes as we want to allow // malformed orders to be passed in, and still show that nothing happens. SignedContextV1[] memory signedContexts = new SignedContextV1[](1); @@ -42,8 +44,7 @@ contract OrderBookTakeOrderNoopTest is OrderBookExternalRealTest { TakeOrderConfig memory orderConfig = TakeOrderConfig(order, inputIOIndex, outputIOIndex, signedContexts); TakeOrderConfig[] memory orders = new TakeOrderConfig[](1); orders[0] = orderConfig; - TakeOrdersConfigV2 memory config = - TakeOrdersConfigV2(output, input, 0, type(uint256).max, type(uint256).max, orders, ""); + TakeOrdersConfigV2 memory config = TakeOrdersConfigV2(0, type(uint256).max, type(uint256).max, orders, ""); vm.expectEmit(address(iOrderbook)); emit OrderNotFound(address(this), order.owner, order.hash()); vm.recordLogs(); @@ -56,8 +57,6 @@ contract OrderBookTakeOrderNoopTest is OrderBookExternalRealTest { /// Same as above but with two orders. function testTakeOrderNoopNonLiveOrderTwo( - address input, - address output, Order memory order1, Order memory order2, uint256 inputIOIndex1, @@ -67,6 +66,15 @@ contract OrderBookTakeOrderNoopTest is OrderBookExternalRealTest { SignedContextV1 memory signedContext1, SignedContextV1 memory signedContext2 ) external { + vm.assume(order1.validInputs.length > 0); + inputIOIndex1 = bound(inputIOIndex1, 0, order1.validInputs.length - 1); + vm.assume(order1.validOutputs.length > 0); + outputIOIndex1 = bound(outputIOIndex1, 0, order1.validOutputs.length - 1); + vm.assume(order2.validInputs.length > 0); + inputIOIndex2 = bound(inputIOIndex2, 0, order2.validInputs.length - 1); + vm.assume(order2.validOutputs.length > 0); + outputIOIndex2 = bound(outputIOIndex2, 0, order2.validOutputs.length - 1); + TakeOrdersConfigV2 memory config; { TakeOrderConfig[] memory orders; @@ -86,7 +94,7 @@ contract OrderBookTakeOrderNoopTest is OrderBookExternalRealTest { orders[1] = orderConfig2; } - config = TakeOrdersConfigV2(output, input, 0, type(uint256).max, type(uint256).max, orders, ""); + config = TakeOrdersConfigV2(0, type(uint256).max, type(uint256).max, orders, ""); } vm.recordLogs(); diff --git a/test/concrete/OrderBook.takeOrder.t.sol b/test/concrete/OrderBook.takeOrder.t.sol index ff9213176..56afbfd6a 100644 --- a/test/concrete/OrderBook.takeOrder.t.sol +++ b/test/concrete/OrderBook.takeOrder.t.sol @@ -48,7 +48,7 @@ contract OrderBookTakeOrderTest is OrderBookExternalRealTest { TakeOrderConfig[] memory orders = new TakeOrderConfig[](1); orders[0] = TakeOrderConfig(order, 0, 0, new SignedContextV1[](0)); TakeOrdersConfigV2 memory takeOrdersConfig = - TakeOrdersConfigV2(inputToken, outputToken, 0, type(uint256).max, type(uint256).max, orders, ""); + TakeOrdersConfigV2(0, type(uint256).max, type(uint256).max, orders, ""); (uint256 totalTakerInput, uint256 totalTakerOutput) = iOrderbook.takeOrders(takeOrdersConfig); assertEq(totalTakerInput, expectedTakerTotalInput); assertEq(totalTakerOutput, expectedTakerTotalOutput);