diff --git a/.gas-snapshot b/.gas-snapshot index 6bf2b7c5d..24928c15b 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -3,7 +3,7 @@ GenericPoolOrderBookFlashBorrowerTest:testTakeOrdersSender() (gas: 2241348) 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, μ: 2626417, ~: 2600441) +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: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) @@ -18,27 +18,30 @@ OrderBookAddOrderTest:testAddOrderRealThreeStackCalculate(address,((address,uint OrderBookAddOrderTest:testAddOrderRealTwoStackCalculateReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes,uint256[]),bytes)) (runs: 5096, μ: 715304, ~: 711949) 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, μ: 8937393460516740787, ~: 8937393460516740786) +OrderBookDepositTest:testDepositFail(address,uint256,uint256) (runs: 5096, μ: 8937393460516740789, ~: 8937393460516740786) OrderBookDepositTest:testDepositGas00() (gas: 8176) OrderBookDepositTest:testDepositGas01() (gas: 34620) -OrderBookDepositTest:testDepositMany((address,address,uint256,uint248)[]) (runs: 5096, μ: 5110729, ~: 4755733) +OrderBookDepositTest:testDepositMany((address,address,uint256,uint248)[]) (runs: 5096, μ: 5062646, ~: 4709523) OrderBookDepositTest:testDepositOverflow(address,uint256,uint256,uint256) (runs: 5096, μ: 46645, ~: 46645) -OrderBookDepositTest:testDepositReentrancy(address,uint256,uint256,address,uint256,uint256) (runs: 5096, μ: 495105, ~: 496632) +OrderBookDepositTest:testDepositReentrancy(address,uint256,uint256,address,uint256,uint256) (runs: 5096, μ: 495280, ~: 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, μ: 494249, ~: 495964) +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, μ: 10467503, ~: 10454009) +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) -OrderBookTakeOrderTest:testTakeOrderPrecisionKnownBad01() (gas: 2556867) +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, μ: 8937393460516719607, ~: 8937393460516700429) -OrderBookWithdrawTest:testWithdrawFullVault(address,uint256,uint256,uint256) (runs: 5096, μ: 41257, ~: 41256) -OrderBookWithdrawTest:testWithdrawMany((bool,address,address,uint256,uint248)[]) (runs: 5096, μ: 1448254, ~: 1371051) +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) OrderBookWithdrawTest:testWithdrawPartialVault(address,uint256,uint256,uint256) (runs: 5096, μ: 51929, ~: 51929) -OrderBookWithdrawTest:testWithdrawReentrant(address,uint256,uint256,address,address,uint256) (runs: 5096, μ: 506329, ~: 507997) +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 diff --git a/script/DeployGenericPoolOrderBookFlashBorrower.sol b/script/DeployGenericPoolOrderBookV3FlashBorrower.sol similarity index 76% rename from script/DeployGenericPoolOrderBookFlashBorrower.sol rename to script/DeployGenericPoolOrderBookV3FlashBorrower.sol index 6ccb5c16d..f80ed08e4 100644 --- a/script/DeployGenericPoolOrderBookFlashBorrower.sol +++ b/script/DeployGenericPoolOrderBookV3FlashBorrower.sol @@ -2,13 +2,13 @@ pragma solidity =0.8.19; import "forge-std/Script.sol"; -import "src/concrete/GenericPoolOrderBookFlashBorrower.sol"; +import "src/concrete/GenericPoolOrderBookV3FlashBorrower.sol"; -/// @title DeployGenericPoolOrderBookFlashBorrower -/// @notice A script that deploys a `GenericPoolOrderBookFlashBorrower`. This is -/// intended to be run on every commit by CI to a testnet such as mumbai, then +/// @title DeployGenericPoolOrderBookV3FlashBorrower +/// @notice A script that deploys a `GenericPoolOrderBookV3FlashBorrower`. This +/// is intended to be run on every commit by CI to a testnet such as mumbai, then /// cross chain deployed to whatever mainnet is required, by users. -contract DeployGenericPoolOrderBookFlashBorrower is Script { +contract DeployGenericPoolOrderBookV3FlashBorrower is Script { /// We are avoiding using ffi here, instead forcing the script runner to /// provide the built metadata. On CI this is achieved by using the rain cli. function run(bytes memory meta) external { diff --git a/src/concrete/GenericPoolOrderBookArbOrderTaker.sol b/src/concrete/GenericPoolOrderBookV3ArbOrderTaker.sol similarity index 95% rename from src/concrete/GenericPoolOrderBookArbOrderTaker.sol rename to src/concrete/GenericPoolOrderBookV3ArbOrderTaker.sol index 35e4e8c7d..e984836a0 100644 --- a/src/concrete/GenericPoolOrderBookArbOrderTaker.sol +++ b/src/concrete/GenericPoolOrderBookV3ArbOrderTaker.sol @@ -8,7 +8,7 @@ import {Address} from "openzeppelin-contracts/contracts/utils/Address.sol"; bytes32 constant CALLER_META_HASH = bytes32(0x00); -contract GenericPoolOrderBookArbOrderTaker is OrderBookV3ArbOrderTaker { +contract GenericPoolOrderBookV3ArbOrderTaker is OrderBookV3ArbOrderTaker { using SafeERC20 for IERC20; using Address for address; diff --git a/src/concrete/GenericPoolOrderBookFlashBorrower.sol b/src/concrete/GenericPoolOrderBookV3FlashBorrower.sol similarity index 100% rename from src/concrete/GenericPoolOrderBookFlashBorrower.sol rename to src/concrete/GenericPoolOrderBookV3FlashBorrower.sol diff --git a/src/concrete/OrderBook.sol b/src/concrete/OrderBook.sol index 41b8c5de5..0bdfc9386 100644 --- a/src/concrete/OrderBook.sol +++ b/src/concrete/OrderBook.sol @@ -32,6 +32,9 @@ 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. @@ -339,6 +342,10 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash nonReentrant returns (uint256 totalTakerInput, uint256 totalTakerOutput) { + if (config.orders.length == 0) { + revert NoOrders(); + } + uint256 i = 0; TakeOrderConfig memory takeOrderConfig; Order memory order; diff --git a/src/concrete/UniV2PoolOrderBookArbOrderTaker.sol b/src/concrete/UniV2PoolOrderBookV3ArbOrderTaker.sol similarity index 96% rename from src/concrete/UniV2PoolOrderBookArbOrderTaker.sol rename to src/concrete/UniV2PoolOrderBookV3ArbOrderTaker.sol index edd1536b5..6d5fc97e8 100644 --- a/src/concrete/UniV2PoolOrderBookArbOrderTaker.sol +++ b/src/concrete/UniV2PoolOrderBookV3ArbOrderTaker.sol @@ -10,7 +10,7 @@ import {Address} from "openzeppelin-contracts/contracts/utils/Address.sol"; bytes32 constant CALLER_META_HASH = bytes32(0x00); -contract UniV2PoolOrderBookArbOrderTaker is OrderBookV3ArbOrderTaker { +contract UniV2PoolOrderBookV3ArbOrderTaker is OrderBookV3ArbOrderTaker { using SafeERC20 for IERC20; using Address for address; diff --git a/test/concrete/GenericPoolOrderBookFlashBorrower.sender.t.sol b/test/concrete/GenericPoolOrderBookFlashBorrower.sender.t.sol index 4c52b0f2b..33e5d9f06 100644 --- a/test/concrete/GenericPoolOrderBookFlashBorrower.sender.t.sol +++ b/test/concrete/GenericPoolOrderBookFlashBorrower.sender.t.sol @@ -6,9 +6,9 @@ import "openzeppelin-contracts/contracts/proxy/Clones.sol"; import "openzeppelin-contracts/contracts/token/ERC20/ERC20.sol"; import "test/util/lib/LibTestConstants.sol"; -import "test/util/lib/LibGenericPoolOrderBookFlashBorrowerConstants.sol"; +import "test/util/lib/LibGenericPoolOrderBookV3FlashBorrowerConstants.sol"; -import "src/concrete/GenericPoolOrderBookFlashBorrower.sol"; +import "src/concrete/GenericPoolOrderBookV3FlashBorrower.sol"; import "src/interface/unstable/IOrderBookV3.sol"; contract Token is ERC20 { diff --git a/test/concrete/OrderBook.takeOrder.noop.t.sol b/test/concrete/OrderBook.takeOrder.noop.t.sol new file mode 100644 index 000000000..038feffdb --- /dev/null +++ b/test/concrete/OrderBook.takeOrder.noop.t.sol @@ -0,0 +1,121 @@ +// SPDX-License-Identifier: CAL +pragma solidity =0.8.19; + +import "src/lib/LibOrder.sol"; + +import "test/util/abstract/OrderBookExternalRealTest.sol"; +import {NoOrders} from "src/concrete/OrderBook.sol"; + +/// @title OrderBookTakeOrderNoopTest +/// @notice A test harness for testing the OrderBook takeOrder function. Focuses +/// on the no-op case. +contract OrderBookTakeOrderNoopTest is OrderBookExternalRealTest { + using LibOrder for Order; + + /// 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 { + TakeOrdersConfigV2 memory config = + TakeOrdersConfigV2(output, input, 0, type(uint256).max, type(uint256).max, new TakeOrderConfig[](0), ""); + vm.expectRevert(NoOrders.selector); + (uint256 totalTakerInput, uint256 totalTakerOutput) = iOrderbook.takeOrders(config); + (totalTakerInput, totalTakerOutput); + } + + /// If there is some order in the input array but it is not live we don't + /// error as the caller may not have control over this, e.g. the order may + /// 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, + Order memory order, + uint256 inputIOIndex, + uint256 outputIOIndex, + SignedContextV1 memory signedContext + ) external { + // 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); + signedContexts[0] = signedContext; + 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, ""); + vm.expectEmit(address(iOrderbook)); + emit OrderNotFound(address(this), order.owner, order.hash()); + vm.recordLogs(); + (uint256 totalTakerInput, uint256 totalTakerOutput) = iOrderbook.takeOrders(config); + assertEq(totalTakerInput, 0); + assertEq(totalTakerOutput, 0); + Vm.Log[] memory logs = vm.getRecordedLogs(); + assertEq(logs.length, 1); + } + + /// Same as above but with two orders. + function testTakeOrderNoopNonLiveOrderTwo( + address input, + address output, + Order memory order1, + Order memory order2, + uint256 inputIOIndex1, + uint256 outputIOIndex1, + uint256 inputIOIndex2, + uint256 outputIOIndex2, + SignedContextV1 memory signedContext1, + SignedContextV1 memory signedContext2 + ) external { + TakeOrdersConfigV2 memory config; + { + TakeOrderConfig[] memory orders; + { + // 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 signedContexts1 = new SignedContextV1[](1); + signedContexts1[0] = signedContext1; + TakeOrderConfig memory orderConfig1 = + TakeOrderConfig(order1, inputIOIndex1, outputIOIndex1, signedContexts1); + SignedContextV1[] memory signedContexts2 = new SignedContextV1[](1); + signedContexts2[0] = signedContext2; + TakeOrderConfig memory orderConfig2 = + TakeOrderConfig(order2, inputIOIndex2, outputIOIndex2, signedContexts2); + orders = new TakeOrderConfig[](2); + orders[0] = orderConfig1; + orders[1] = orderConfig2; + } + + config = TakeOrdersConfigV2(output, input, 0, type(uint256).max, type(uint256).max, orders, ""); + } + + vm.recordLogs(); + { + (uint256 totalTakerInput, uint256 totalTakerOutput) = iOrderbook.takeOrders(config); + assertEq(totalTakerInput, 0); + assertEq(totalTakerOutput, 0); + } + Vm.Log[] memory logs = vm.getRecordedLogs(); + assertEq(logs.length, 2); + + { + assertEq(logs[0].topics.length, 1); + assertEq(logs[0].topics[0], bytes32(uint256(keccak256("OrderNotFound(address,address,bytes32)")))); + (address sender1, address owner1, bytes32 orderHash1) = + abi.decode(logs[0].data, (address, address, bytes32)); + assertEq(sender1, address(this)); + assertEq(owner1, order1.owner); + assertEq(orderHash1, order1.hash()); + } + + { + assertEq(logs[1].topics.length, 1); + assertEq(logs[1].topics[0], bytes32(uint256(keccak256("OrderNotFound(address,address,bytes32)")))); + (address sender2, address owner2, bytes32 orderHash2) = + abi.decode(logs[1].data, (address, address, bytes32)); + assertEq(sender2, address(this)); + assertEq(owner2, order2.owner); + assertEq(orderHash2, order2.hash()); + } + } +} diff --git a/test/util/lib/LibGenericPoolOrderBookFlashBorrowerConstants.sol b/test/util/lib/LibGenericPoolOrderBookV3FlashBorrowerConstants.sol similarity index 100% rename from test/util/lib/LibGenericPoolOrderBookFlashBorrowerConstants.sol rename to test/util/lib/LibGenericPoolOrderBookV3FlashBorrowerConstants.sol