From 799832fa2e4b10d7c239e052e254f76ddc61d9be Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Wed, 19 Jul 2023 14:06:44 +0400 Subject: [PATCH 1/6] wip on remove order tests --- .gitmodules | 24 +++++--- lib/forge-std | 2 +- lib/rain.datacontract | 1 + lib/rain.factory | 2 +- lib/rain.interpreter | 2 +- lib/rain.lib.memkv | 1 + src/concrete/OrderBook.sol | 62 ++++++++++----------- src/interface/unstable/IOrderBookV3.sol | 23 +++++--- test/concrete/OrderBook.addOrder.mock.t.sol | 37 +++++++++--- test/util/abstract/IOrderBookV3Stub.sol | 4 +- 10 files changed, 94 insertions(+), 64 deletions(-) create mode 160000 lib/rain.datacontract create mode 160000 lib/rain.lib.memkv diff --git a/.gitmodules b/.gitmodules index d05161205..653918ad6 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,21 +1,27 @@ [submodule "lib/rain.math.fixedpoint"] path = lib/rain.math.fixedpoint url = https://github.com/rainprotocol/rain.math.fixedpoint -[submodule "lib/rain.factory"] - path = lib/rain.factory - url = https://github.com/rainprotocol/rain.factory -[submodule "lib/forge-std"] - path = lib/forge-std - url = https://github.com/foundry-rs/forge-std [submodule "lib/openzeppelin-contracts"] path = lib/openzeppelin-contracts url = https://github.com/OpenZeppelin/openzeppelin-contracts -[submodule "lib/rain.interpreter"] - path = lib/rain.interpreter - url = https://github.com/rainprotocol/rain.interpreter [submodule "lib/rain.metadata"] path = lib/rain.metadata url = https://github.com/rainprotocol/rain.metadata [submodule "lib/rain.erc1820"] path = lib/rain.erc1820 url = https://github.com/rainprotocol/rain.erc1820 +[submodule "lib/rain.interpreter"] + path = lib/rain.interpreter + url = https://github.com/rainprotocol/rain.interpreter +[submodule "lib/rain.factory"] + path = lib/rain.factory + url = https://github.com/rainprotocol/rain.factory +[submodule "lib/forge-std"] + path = lib/forge-std + url = https://github.com/foundry-rs/forge-std +[submodule "lib/rain.datacontract"] + path = lib/rain.datacontract + url = https://github.com/rainprotocol/rain.datacontract +[submodule "lib/rain.lib.memkv"] + path = lib/rain.lib.memkv + url = https://github.com/rainprotocol/rain.lib.memkv diff --git a/lib/forge-std b/lib/forge-std index e8a047e3f..74cfb77e3 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit e8a047e3f40f13fa37af6fe14e6e06283d9a060e +Subproject commit 74cfb77e308dd188d2f58864aaf44963ae6b88b1 diff --git a/lib/rain.datacontract b/lib/rain.datacontract new file mode 160000 index 000000000..04e400b01 --- /dev/null +++ b/lib/rain.datacontract @@ -0,0 +1 @@ +Subproject commit 04e400b0175667017a9d3a3b8eb079615f74c908 diff --git a/lib/rain.factory b/lib/rain.factory index 7e0d17abc..b4ef3953d 160000 --- a/lib/rain.factory +++ b/lib/rain.factory @@ -1 +1 @@ -Subproject commit 7e0d17abc3b1f16a1e5d2ad48215bd21e2fb7eef +Subproject commit b4ef3953dd3cc35593e6e1e6736741cf923f850f diff --git a/lib/rain.interpreter b/lib/rain.interpreter index 083250d7d..0ee6e6052 160000 --- a/lib/rain.interpreter +++ b/lib/rain.interpreter @@ -1 +1 @@ -Subproject commit 083250d7d8ebff0cafbd9e59a36dfd48cb7e5421 +Subproject commit 0ee6e605210ea76ea74c68a92411e1f242ae322a diff --git a/lib/rain.lib.memkv b/lib/rain.lib.memkv new file mode 160000 index 000000000..818f262f9 --- /dev/null +++ b/lib/rain.lib.memkv @@ -0,0 +1 @@ +Subproject commit 818f262f91ac4402d9c800d333ae512874bca6e5 diff --git a/src/concrete/OrderBook.sol b/src/concrete/OrderBook.sol index 98c618a36..10c3a51bc 100644 --- a/src/concrete/OrderBook.sol +++ b/src/concrete/OrderBook.sol @@ -108,13 +108,6 @@ uint256 constant CONTEXT_VAULT_IO_ROWS = 5; /// @dev Hash of the caller contract metadata for construction. bytes32 constant CALLER_META_HASH = bytes32(0xd55ed91accdfd893ecc4028057ab2894d6eb88b88f59a27f0b73eaef92d20430); -/// @dev Value that signifies that an order is live in the internal mapping. -/// Anything nonzero is equally useful. -uint256 constant LIVE_ORDER = 1; - -/// @dev Value that signifies that an order is dead in the internal mapping. -uint256 constant DEAD_ORDER = 0; - /// All information resulting from an order calculation that allows for vault IO /// to be calculated and applied, then the handle IO entrypoint to be dispatched. /// @param outputMax The UNSCALED maximum output calculated by the order @@ -167,13 +160,13 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookFlashLe /// All hashes of all active orders. There's nothing interesting in the value /// it's just nonzero if the order is live. The key is the hash of the order. /// Removing an order sets the value back to zero so it is identical to the - /// order never existing and gives a gas refund on removal. + /// order never existing. /// The order hash includes its owner so there's no need to build a multi /// level mapping, each order hash MUST uniquely identify the order globally. /// order hash => order is live // Solhint and slither disagree on this. Slither wins. //solhint-disable-next-line private-vars-leading-underscore - mapping(bytes32 => uint256) internal sOrders; + mapping(bytes32 => bool) internal sOrders; /// @dev Vault balances are stored in a mapping of owner => token => vault ID /// This gives 1:1 parity with the `IOrderBookV1` interface but keeping the @@ -247,7 +240,7 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookFlashLe } /// @inheritdoc IOrderBookV3 - function addOrder(OrderConfig calldata config) external nonReentrant { + function addOrder(OrderConfig calldata config) external nonReentrant returns (bool stateChanged) { if (config.evaluableConfig.sources.length == 0) { revert OrderNoSources(msg.sender); } @@ -281,38 +274,39 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookFlashLe ); bytes32 orderHash = order.hash(); - // Check that the order is not already live. As the order hash includes - // the expression address, this can only happen if the deployer returns - // the same expression address for multiple deployments. This is - // technically possible but not something we want to support. - if (sOrders[orderHash] != DEAD_ORDER) { - revert OrderExists(msg.sender, orderHash); - } - //slither-disable-next-line reentrancy-benign - sOrders[orderHash] = LIVE_ORDER; - emit AddOrder(msg.sender, config.evaluableConfig.deployer, order, orderHash); - - // We only emit the meta event if there is meta to emit. We do require - // that the meta self describes as a Rain meta document. - if (config.meta.length > 0) { - LibMeta.checkMetaUnhashed(config.meta); - emit MetaV1(msg.sender, uint256(orderHash), config.meta); + // If the order is not dead we return early without state changes. + if (!sOrders[orderHash]) { + stateChanged = true; + + //slither-disable-next-line reentrancy-benign + sOrders[orderHash] = true; + emit AddOrder(msg.sender, config.evaluableConfig.deployer, order, orderHash); + + // We only emit the meta event if there is meta to emit. We do require + // that the meta self describes as a Rain meta document. + if (config.meta.length > 0) { + LibMeta.checkMetaUnhashed(config.meta); + emit MetaV1(msg.sender, uint256(orderHash), config.meta); + } } } /// @inheritdoc IOrderBookV3 function orderExists(bytes32 orderHash) external view override returns (bool) { - return sOrders[orderHash] == LIVE_ORDER; + return sOrders[orderHash]; } /// @inheritdoc IOrderBookV3 - function removeOrder(Order calldata order) external nonReentrant { + function removeOrder(Order calldata order) external nonReentrant returns (bool stateChanged) { if (msg.sender != order.owner) { revert NotOrderOwner(msg.sender, order.owner); } - bytes32 orderHash_ = order.hash(); - delete (sOrders[orderHash_]); - emit RemoveOrder(msg.sender, order, orderHash_); + bytes32 orderHash = order.hash(); + if (sOrders[orderHash]) { + stateChanged = true; + sOrders[orderHash] = false; + emit RemoveOrder(msg.sender, order, orderHash); + } } /// @inheritdoc IOrderBookV3 @@ -329,7 +323,7 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookFlashLe takeOrder = config.orders[i]; order = takeOrder.order; bytes32 orderHash = order.hash(); - if (sOrders[orderHash] == DEAD_ORDER) { + if (!sOrders[orderHash]) { emit OrderNotFound(msg.sender, order.owner, orderHash); } else { if (order.validInputs[takeOrder.inputIOIndex].token != config.output) { @@ -420,11 +414,11 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookFlashLe // If either order is dead the clear is a no-op other than emitting // `OrderNotFound`. Returning rather than erroring makes it easier to // bulk clear using `Multicall`. - if (sOrders[alice.hash()] == DEAD_ORDER) { + if (!sOrders[alice.hash()]) { emit OrderNotFound(msg.sender, alice.owner, alice.hash()); return; } - if (sOrders[bob.hash()] == DEAD_ORDER) { + if (!sOrders[bob.hash()]) { emit OrderNotFound(msg.sender, bob.owner, bob.hash()); return; } diff --git a/src/interface/unstable/IOrderBookV3.sol b/src/interface/unstable/IOrderBookV3.sol index 41c710ad0..02fad7bec 100644 --- a/src/interface/unstable/IOrderBookV3.sol +++ b/src/interface/unstable/IOrderBookV3.sol @@ -278,7 +278,8 @@ struct ClearStateChange { /// - adding an order MUST revert if there is no handle IO entrypoint. /// - adding an order MUST revert if there are no inputs. /// - adding an order MUST revert if there are no outputs. -/// - adding an order MUST revert if the order already exists. +/// - adding and removing orders MUST return a boolean indicating if the state +/// changed. /// - new `orderExists` method. interface IOrderBookV3 is IERC3156FlashLender, IInterpreterCallerV2 { /// MUST be thrown by `deposit` if the amount is zero. @@ -311,11 +312,6 @@ interface IOrderBookV3 is IERC3156FlashLender, IInterpreterCallerV2 { /// @param sender `msg.sender` adding the order. error OrderNoOutputs(address sender); - /// MUST be thrown by `addOrder` if the order already exists. - /// @param sender `msg.sender` adding the order. - /// @param orderHash The hash of the order that already exists. - error OrderExists(address sender, bytes32 orderHash); - /// Some tokens have been deposited to a vault. /// @param sender `msg.sender` depositing tokens. Delegated deposits are NOT /// supported. @@ -474,11 +470,18 @@ interface IOrderBookV3 is IERC3156FlashLender, IInterpreterCallerV2 { /// evaluation on the interpreter, which MUST prevent the order from /// clearing. /// - /// MUST revert with `OrderExists` if the order already exists. /// MUST revert with `OrderNoInputs` if the order has no inputs. /// MUST revert with `OrderNoOutputs` if the order has no outputs. + /// + /// If the order already exists, the order book MUST NOT change state, which + /// includes not emitting an event. Instead it MUST return false. If the + /// order book modifies state it MUST emit an `AddOrder` event and return + /// true. + /// /// @param config All config required to build an `Order`. - function addOrder(OrderConfig calldata config) external; + /// @return stateChanged True if the order was added, false if it already + /// existed. + function addOrder(OrderConfig calldata config) external returns (bool stateChanged); /// Returns true if the order exists, false otherwise. /// @param orderHash The hash of the order to check. @@ -491,7 +494,9 @@ interface IOrderBookV3 is IERC3156FlashLender, IInterpreterCallerV2 { /// transaction will complete with that order hash definitely, redundantly /// not live. /// @param order The `Order` data exactly as it was added. - function removeOrder(Order calldata order) external; + /// @return stateChanged True if the order was removed, false if it did not + /// exist. + function removeOrder(Order calldata order) external returns (bool stateChanged); /// Allows `msg.sender` to attempt to fill a list of orders in sequence /// without needing to place their own order and clear them. This works like diff --git a/test/concrete/OrderBook.addOrder.mock.t.sol b/test/concrete/OrderBook.addOrder.mock.t.sol index a533eee8e..fadfed247 100644 --- a/test/concrete/OrderBook.addOrder.mock.t.sol +++ b/test/concrete/OrderBook.addOrder.mock.t.sol @@ -34,7 +34,7 @@ contract OrderBookAddOrderMockTest is OrderBookExternalMockTest, IMetaV1 { vm.record(); vm.recordLogs(); vm.prank(owner); - iOrderbook.addOrder(config); + assertTrue(iOrderbook.addOrder(config)); // MetaV1 is NOT emitted if the meta is empty. assertEq(vm.getRecordedLogs().length, config.meta.length > 0 ? 2 : 1); (bytes32[] memory reads, bytes32[] memory writes) = vm.accesses(address(iOrderbook)); @@ -44,17 +44,25 @@ contract OrderBookAddOrderMockTest is OrderBookExternalMockTest, IMetaV1 { assertEq(writes.length, 3); assertTrue(iOrderbook.orderExists(orderHash)); - // Adding the same order again MUST revert. This MAY be impossible to - // encounter for a real expression deployer, as the deployer MAY NOT - // return the same address twice, but it is possible to mock. - vm.prank(owner); - vm.expectRevert(abi.encodeWithSelector(OrderExists.selector, owner, orderHash)); + // Adding the same order again MUST NOT change state. This MAY be + // impossible to encounter for a real expression deployer, as the + // deployer MAY NOT return the same address twice, but it is possible to + // mock. vm.mockCall( address(iDeployer), abi.encodeWithSelector(IExpressionDeployerV1.deployExpression.selector), abi.encode(iInterpreter, iStore, expression) ); - iOrderbook.addOrder(config); + vm.record(); + vm.recordLogs(); + vm.prank(owner); + assertFalse(iOrderbook.addOrder(config)); + assertEq(vm.getRecordedLogs().length, 0); + (reads, writes) = vm.accesses(address(iOrderbook)); + // 3x for reentrancy guard, 1x for dead order check. + assertEq(reads.length, 4); + // 2x for reentrancy guard. + assertEq(writes.length, 2); return (order, orderHash); } @@ -139,6 +147,21 @@ contract OrderBookAddOrderMockTest is OrderBookExternalMockTest, IMetaV1 { assertTrue(!iOrderbook.orderExists(orderHash)); } + function testDebugZ() external { + IO[] memory inputs = new IO[](1); + inputs[0] = IO(address(0), 0, 0); + IO[] memory outputs = new IO[](1); + outputs[0] = IO(address(0), 0, 0); + OrderConfig memory config = + OrderConfig( + inputs, + outputs, + EvaluableConfig(IExpressionDeployerV1(address(0)), new bytes[](0), new uint256[](0)), + new bytes(0) + ); + iOrderbook.addOrder(config); + } + /// Adding a valid order with a non-empty meta MUST emit MetaV1 if the meta /// is self describing as a rain meta document. function testAddOrderWithNonEmptyMetaEmitsMetaV1(address owner, OrderConfig memory config, address expression) diff --git a/test/util/abstract/IOrderBookV3Stub.sol b/test/util/abstract/IOrderBookV3Stub.sol index ee6a55d29..6deeeb341 100644 --- a/test/util/abstract/IOrderBookV3Stub.sol +++ b/test/util/abstract/IOrderBookV3Stub.sol @@ -5,7 +5,7 @@ import "src/interface/unstable/IOrderBookV3.sol"; abstract contract IOrderBookV3Stub is IOrderBookV3 { /// @inheritdoc IOrderBookV3 - function addOrder(OrderConfig calldata) external pure { + function addOrder(OrderConfig calldata) external pure returns (bool) { revert("addOrder"); } @@ -15,7 +15,7 @@ abstract contract IOrderBookV3Stub is IOrderBookV3 { } /// @inheritdoc IOrderBookV3 - function removeOrder(Order calldata) external pure { + function removeOrder(Order calldata) external pure returns (bool) { revert("removeOrder"); } From 558e4f469c5af3bb560e516ab6085a285047c3b7 Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Thu, 20 Jul 2023 19:04:48 +0400 Subject: [PATCH 2/6] snapshot --- .gas-snapshot | 54 ++--- .gitmodules | 6 +- lib/rain.datacontract | 2 +- src/concrete/OrderBook.sol | 35 ++-- test/concrete/OrderBook.addOrder.mock.t.sol | 86 +------- test/concrete/OrderBook.addOrder.t.sol | 12 +- .../concrete/OrderBook.removeOrder.mock.t.sol | 193 ++++++++++++++++++ test/concrete/OrderBook.withdraw.t.sol | 2 +- .../abstract/OrderBookExternalMockTest.sol | 100 ++++++++- test/util/lib/LibTestAddOrder.sol | 28 ++- 10 files changed, 383 insertions(+), 135 deletions(-) create mode 100644 test/concrete/OrderBook.removeOrder.mock.t.sol diff --git a/.gas-snapshot b/.gas-snapshot index f204c8137..305ccaeb9 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,35 +1,41 @@ -GenericPoolOrderBookFlashBorrowerTest:testMinimumOutput(uint256,uint256) (runs: 5096, μ: 2315773, ~: 2319046) +GenericPoolOrderBookFlashBorrowerTest:testMinimumOutput(uint256,uint256) (runs: 5096, μ: 2315742, ~: 2319046) GenericPoolOrderBookFlashBorrowerTest:testTakeOrdersSender() (gas: 2230881) -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, μ: 3332897, ~: 3311290) -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, μ: 3178640, ~: 3183991) -OrderBookAddOrderMockTest:testAddOrderTwoAccountsWithSameConfig(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes),address) (runs: 5096, μ: 2984203, ~: 2983512) -OrderBookAddOrderMockTest:testAddOrderWithCalculationsInputsAndOutputsSucceeds(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes),address) (runs: 5096, μ: 1552081, ~: 1543479) -OrderBookAddOrderMockTest:testAddOrderWithNonEmptyMetaEmitsMetaV1(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes),address) (runs: 5096, μ: 1561961, ~: 1553408) -OrderBookAddOrderMockTest:testAddOrderWithNonEmptyMetaReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes),address) (runs: 5096, μ: 861676, ~: 859777) -OrderBookAddOrderMockTest:testAddOrderWithoutCalculationsReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes)) (runs: 5096, μ: 285903, ~: 284875) -OrderBookAddOrderMockTest:testAddOrderWithoutInputsReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes)) (runs: 5096, μ: 297793, ~: 295925) -OrderBookAddOrderMockTest:testAddOrderWithoutOutputsReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes)) (runs: 5096, μ: 298512, ~: 296468) -OrderBookAddOrderTest:testAddOrderRealNoHandleIOReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes)) (runs: 5096, μ: 249422, ~: 248374) -OrderBookAddOrderTest:testAddOrderRealNoSourcesReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes)) (runs: 5096, μ: 241014, ~: 239965) -OrderBookAddOrderTest:testAddOrderRealOneStackCalculateReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes)) (runs: 5096, μ: 272576, ~: 271479) -OrderBookAddOrderTest:testAddOrderRealThreeStackCalculate(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes)) (runs: 5096, μ: 1677570, ~: 1667344) -OrderBookAddOrderTest:testAddOrderRealTwoStackCalculateReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes)) (runs: 5096, μ: 1671739, ~: 1661513) -OrderBookAddOrderTest:testAddOrderRealZeroStackCalculateReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes)) (runs: 5096, μ: 268146, ~: 267050) +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, μ: 3327619, ~: 3300143) +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, μ: 3171805, ~: 3178905) +OrderBookAddOrderMockTest:testAddOrderTwoAccountsWithSameConfig(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes),address) (runs: 5096, μ: 2973414, ~: 2969001) +OrderBookAddOrderMockTest:testAddOrderWithCalculationsInputsAndOutputsSucceeds(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes),address) (runs: 5096, μ: 1553370, ~: 1544769) +OrderBookAddOrderMockTest:testAddOrderWithNonEmptyMetaEmitsMetaV1(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes),address) (runs: 5096, μ: 1563247, ~: 1554684) +OrderBookAddOrderMockTest:testAddOrderWithNonEmptyMetaReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes),address) (runs: 5096, μ: 863899, ~: 862000) +OrderBookAddOrderMockTest:testAddOrderWithoutCalculationsReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes)) (runs: 5096, μ: 288136, ~: 287123) +OrderBookAddOrderMockTest:testAddOrderWithoutInputsReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes)) (runs: 5096, μ: 300009, ~: 298141) +OrderBookAddOrderMockTest:testAddOrderWithoutOutputsReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes)) (runs: 5096, μ: 300728, ~: 298684) +OrderBookAddOrderTest:testAddOrderRealNoHandleIOReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes)) (runs: 5096, μ: 247907, ~: 247005) +OrderBookAddOrderTest:testAddOrderRealNoSourcesReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes)) (runs: 5096, μ: 239466, ~: 238574) +OrderBookAddOrderTest:testAddOrderRealOneStackCalculateReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes)) (runs: 5096, μ: 271077, ~: 270308) +OrderBookAddOrderTest:testAddOrderRealThreeStackCalculate(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes)) (runs: 5096, μ: 1675201, ~: 1668189) +OrderBookAddOrderTest:testAddOrderRealTwoStackCalculateReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes)) (runs: 5096, μ: 1669370, ~: 1662358) +OrderBookAddOrderTest:testAddOrderRealZeroStackCalculateReverts(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes)) (runs: 5096, μ: 266647, ~: 265877) OrderBookDepositTest:testDepositEvent(address,uint256,uint256) (runs: 5096, μ: 38710, ~: 38710) -OrderBookDepositTest:testDepositFail(address,uint256,uint256) (runs: 5096, μ: 8937393460516740785, ~: 8937393460516740786) +OrderBookDepositTest:testDepositFail(address,uint256,uint256) (runs: 5096, μ: 8937393460516740741, ~: 8937393460516740727) OrderBookDepositTest:testDepositGas00() (gas: 8176) OrderBookDepositTest:testDepositGas01() (gas: 34620) -OrderBookDepositTest:testDepositMany((address,address,uint256,uint248)[]) (runs: 5096, μ: 5023411, ~: 4663258) +OrderBookDepositTest:testDepositMany((address,address,uint256,uint248)[]) (runs: 5096, μ: 5464154, ~: 5358203) OrderBookDepositTest:testDepositOverflow(address,uint256,uint256,uint256) (runs: 5096, μ: 46645, ~: 46645) -OrderBookDepositTest:testDepositReentrancy(address,uint256,uint256,address,uint256,uint256) (runs: 5096, μ: 495175, ~: 496632) +OrderBookDepositTest:testDepositReentrancy(address,uint256,uint256,address,uint256,uint256) (runs: 5096, μ: 494421, ~: 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, μ: 494155, ~: 495964) +OrderBookDepositTest:testVaultBalanceReentrant(address,uint256,uint256,address,address,uint256) (runs: 5096, μ: 493269, ~: 495964) +OrderBookRemoveOrderMockTest:testRemoveOrderAddRemoveMulti(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes),address) (runs: 5096, μ: 7852729, ~: 7760163) +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, μ: 5595387, ~: 5533281) +OrderBookRemoveOrderMockTest:testRemoveOrderDifferentOwners(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes),address) (runs: 5096, μ: 5323719, ~: 5284644) +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, μ: 11257905, ~: 11141262) +OrderBookRemoveOrderMockTest:testRemoveOrderDoesNotExist(address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes),address) (runs: 5096, μ: 451197, ~: 448009) +OrderBookRemoveOrderMockTest:testRemoveOrderOnlyOwner(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes),address) (runs: 5096, μ: 2868562, ~: 2850555) OrderBookWithdrawTest:testWithdrawEmptyVault(address,address,uint256,uint256) (runs: 5096, μ: 15251, ~: 15251) -OrderBookWithdrawTest:testWithdrawFailure(address,uint256,uint256,uint256) (runs: 5096, μ: 8937393460516719508, ~: 8937393460516700411) -OrderBookWithdrawTest:testWithdrawFullVault(address,uint256,uint256,uint256) (runs: 5096, μ: 41245, ~: 41244) -OrderBookWithdrawTest:testWithdrawMany((bool,address,address,uint256,uint248)[]) (runs: 5096, μ: 1392625, ~: 1325216) +OrderBookWithdrawTest:testWithdrawFailure(address,uint256,uint256,uint256) (runs: 5096, μ: 8937393460516719412, ~: 8937393460516700411) +OrderBookWithdrawTest:testWithdrawFullVault(address,uint256,uint256,uint256) (runs: 5096, μ: 41228, ~: 41225) +OrderBookWithdrawTest:testWithdrawMany((bool,address,address,uint256,uint248)[]) (runs: 5096, μ: 1429599, ~: 1390000) OrderBookWithdrawTest:testWithdrawPartialVault(address,uint256,uint256,uint256) (runs: 5096, μ: 51914, ~: 51914) -OrderBookWithdrawTest:testWithdrawReentrant(address,uint256,uint256,address,address,uint256) (runs: 5096, μ: 506222, ~: 507991) +OrderBookWithdrawTest:testWithdrawReentrant(address,uint256,uint256,address,address,uint256) (runs: 5096, μ: 505472, ~: 507991) OrderBookWithdrawTest:testWithdrawZero(address,address,uint256) (runs: 5096, μ: 12809, ~: 12809) \ No newline at end of file diff --git a/.gitmodules b/.gitmodules index 653918ad6..9231ba630 100644 --- a/.gitmodules +++ b/.gitmodules @@ -19,9 +19,9 @@ [submodule "lib/forge-std"] path = lib/forge-std url = https://github.com/foundry-rs/forge-std -[submodule "lib/rain.datacontract"] - path = lib/rain.datacontract - url = https://github.com/rainprotocol/rain.datacontract [submodule "lib/rain.lib.memkv"] path = lib/rain.lib.memkv url = https://github.com/rainprotocol/rain.lib.memkv +[submodule "lib/rain.datacontract"] + path = lib/rain.datacontract + url = https://github.com/rainprotocol/rain.datacontract diff --git a/lib/rain.datacontract b/lib/rain.datacontract index 04e400b01..723bc2f27 160000 --- a/lib/rain.datacontract +++ b/lib/rain.datacontract @@ -1 +1 @@ -Subproject commit 04e400b0175667017a9d3a3b8eb079615f74c908 +Subproject commit 723bc2f273955071e598023ee325d495fc624089 diff --git a/src/concrete/OrderBook.sol b/src/concrete/OrderBook.sol index 10c3a51bc..fc0425789 100644 --- a/src/concrete/OrderBook.sol +++ b/src/concrete/OrderBook.sol @@ -44,6 +44,15 @@ error MinimumInput(uint256 minimumInput, uint256 input); /// @param owner The owner of both orders. error SameOwner(address owner); +/// @dev Stored value for a live order. NOT a boolean because storing a boolean +/// is more expensive than storing a uint256. +uint256 constant ORDER_LIVE = 1; + +/// @dev Stored value for a dead order. `0` is chosen because it is the default +/// value for a mapping, which means all orders are dead unless explicitly made +/// live. +uint256 constant ORDER_DEAD = 0; + /// @dev Entrypoint to a calculate the amount and ratio of an order. SourceIndex constant CALCULATE_ORDER_ENTRYPOINT = SourceIndex.wrap(0); /// @dev Entrypoint to handle the final internal vault movements resulting from @@ -166,7 +175,7 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookFlashLe /// order hash => order is live // Solhint and slither disagree on this. Slither wins. //solhint-disable-next-line private-vars-leading-underscore - mapping(bytes32 => bool) internal sOrders; + mapping(bytes32 => uint256) internal sOrders; /// @dev Vault balances are stored in a mapping of owner => token => vault ID /// This gives 1:1 parity with the `IOrderBookV1` interface but keeping the @@ -207,6 +216,11 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookFlashLe return sVaultBalances[owner][token][vaultId]; } + /// @inheritdoc IOrderBookV3 + function orderExists(bytes32 orderHash) external view override nonReentrantView returns (bool) { + return sOrders[orderHash] == ORDER_LIVE; + } + /// @inheritdoc IOrderBookV3 function deposit(address token, uint256 vaultId, uint256 amount) external nonReentrant { if (amount == 0) { @@ -275,11 +289,11 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookFlashLe bytes32 orderHash = order.hash(); // If the order is not dead we return early without state changes. - if (!sOrders[orderHash]) { + if (sOrders[orderHash] == ORDER_DEAD) { stateChanged = true; //slither-disable-next-line reentrancy-benign - sOrders[orderHash] = true; + sOrders[orderHash] = ORDER_LIVE; emit AddOrder(msg.sender, config.evaluableConfig.deployer, order, orderHash); // We only emit the meta event if there is meta to emit. We do require @@ -291,20 +305,15 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookFlashLe } } - /// @inheritdoc IOrderBookV3 - function orderExists(bytes32 orderHash) external view override returns (bool) { - return sOrders[orderHash]; - } - /// @inheritdoc IOrderBookV3 function removeOrder(Order calldata order) external nonReentrant returns (bool stateChanged) { if (msg.sender != order.owner) { revert NotOrderOwner(msg.sender, order.owner); } bytes32 orderHash = order.hash(); - if (sOrders[orderHash]) { + if (sOrders[orderHash] == ORDER_LIVE) { stateChanged = true; - sOrders[orderHash] = false; + sOrders[orderHash] = ORDER_DEAD; emit RemoveOrder(msg.sender, order, orderHash); } } @@ -323,7 +332,7 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookFlashLe takeOrder = config.orders[i]; order = takeOrder.order; bytes32 orderHash = order.hash(); - if (!sOrders[orderHash]) { + if (sOrders[orderHash] == ORDER_DEAD) { emit OrderNotFound(msg.sender, order.owner, orderHash); } else { if (order.validInputs[takeOrder.inputIOIndex].token != config.output) { @@ -414,11 +423,11 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookFlashLe // If either order is dead the clear is a no-op other than emitting // `OrderNotFound`. Returning rather than erroring makes it easier to // bulk clear using `Multicall`. - if (!sOrders[alice.hash()]) { + if (sOrders[alice.hash()] == ORDER_DEAD) { emit OrderNotFound(msg.sender, alice.owner, alice.hash()); return; } - if (!sOrders[bob.hash()]) { + if (sOrders[bob.hash()] == ORDER_DEAD) { emit OrderNotFound(msg.sender, bob.owner, bob.hash()); return; } diff --git a/test/concrete/OrderBook.addOrder.mock.t.sol b/test/concrete/OrderBook.addOrder.mock.t.sol index fadfed247..d951c39ae 100644 --- a/test/concrete/OrderBook.addOrder.mock.t.sol +++ b/test/concrete/OrderBook.addOrder.mock.t.sol @@ -1,71 +1,12 @@ // SPDX-License-Identifier: CAL pragma solidity =0.8.19; -import "rain.metadata/LibMeta.sol"; - import "test/util/abstract/OrderBookExternalMockTest.sol"; import "test/util/lib/LibTestAddOrder.sol"; /// @title OrderBookAddOrderMockTest /// @notice Tests the addOrder function of the OrderBook contract. -contract OrderBookAddOrderMockTest is OrderBookExternalMockTest, IMetaV1 { - /// Boilerplate to add an order with a mocked deployer and checks events and - /// storage accesses. - function addOrderWithChecks(address owner, OrderConfig memory config, address expression) - internal - returns (Order memory, bytes32) - { - config.evaluableConfig.deployer = iDeployer; - (Order memory order, bytes32 orderHash) = - LibTestAddOrder.expectedOrder(owner, config, iInterpreter, iStore, expression); - assertTrue(!iOrderbook.orderExists(orderHash)); - vm.mockCall( - address(iDeployer), - abi.encodeWithSelector(IExpressionDeployerV1.deployExpression.selector), - abi.encode(iInterpreter, iStore, expression) - ); - vm.expectEmit(false, false, false, true); - emit AddOrder(owner, iDeployer, order, orderHash); - if (config.meta.length > 0) { - vm.expectEmit(false, false, true, false); - // The subject of the meta is the order hash. - emit MetaV1(owner, uint256(orderHash), config.meta); - } - vm.record(); - vm.recordLogs(); - vm.prank(owner); - assertTrue(iOrderbook.addOrder(config)); - // MetaV1 is NOT emitted if the meta is empty. - assertEq(vm.getRecordedLogs().length, config.meta.length > 0 ? 2 : 1); - (bytes32[] memory reads, bytes32[] memory writes) = vm.accesses(address(iOrderbook)); - // 3x for reentrancy guard, 1x for dead order check, 1x for live write. - assertEq(reads.length, 5); - // 2x for reentrancy guard, 1x for live write. - assertEq(writes.length, 3); - assertTrue(iOrderbook.orderExists(orderHash)); - - // Adding the same order again MUST NOT change state. This MAY be - // impossible to encounter for a real expression deployer, as the - // deployer MAY NOT return the same address twice, but it is possible to - // mock. - vm.mockCall( - address(iDeployer), - abi.encodeWithSelector(IExpressionDeployerV1.deployExpression.selector), - abi.encode(iInterpreter, iStore, expression) - ); - vm.record(); - vm.recordLogs(); - vm.prank(owner); - assertFalse(iOrderbook.addOrder(config)); - assertEq(vm.getRecordedLogs().length, 0); - (reads, writes) = vm.accesses(address(iOrderbook)); - // 3x for reentrancy guard, 1x for dead order check. - assertEq(reads.length, 4); - // 2x for reentrancy guard. - assertEq(writes.length, 2); - return (order, orderHash); - } - +contract OrderBookAddOrderMockTest is OrderBookExternalMockTest { /// Adding an order without calculations MUST revert. function testAddOrderWithoutCalculationsReverts(address owner, OrderConfig memory config) public { vm.prank(owner); @@ -147,21 +88,6 @@ contract OrderBookAddOrderMockTest is OrderBookExternalMockTest, IMetaV1 { assertTrue(!iOrderbook.orderExists(orderHash)); } - function testDebugZ() external { - IO[] memory inputs = new IO[](1); - inputs[0] = IO(address(0), 0, 0); - IO[] memory outputs = new IO[](1); - outputs[0] = IO(address(0), 0, 0); - OrderConfig memory config = - OrderConfig( - inputs, - outputs, - EvaluableConfig(IExpressionDeployerV1(address(0)), new bytes[](0), new uint256[](0)), - new bytes(0) - ); - iOrderbook.addOrder(config); - } - /// Adding a valid order with a non-empty meta MUST emit MetaV1 if the meta /// is self describing as a rain meta document. function testAddOrderWithNonEmptyMetaEmitsMetaV1(address owner, OrderConfig memory config, address expression) @@ -190,7 +116,7 @@ contract OrderBookAddOrderMockTest is OrderBookExternalMockTest, IMetaV1 { address expression ) public { vm.assume(alice != bob); - vm.assume(LibTestAddOrder.conformConfig(config, iDeployer)); + LibTestAddOrder.conformConfig(config, iDeployer); (Order memory aliceOrder, bytes32 aliceOrderHash) = addOrderWithChecks(alice, config, expression); (Order memory bobOrder, bytes32 bobOrderHash) = addOrderWithChecks(bob, config, expression); (aliceOrder); @@ -209,8 +135,8 @@ contract OrderBookAddOrderMockTest is OrderBookExternalMockTest, IMetaV1 { address bobExpression ) public { vm.assume(alice != bob); - vm.assume(LibTestAddOrder.conformConfig(aliceConfig, iDeployer)); - vm.assume(LibTestAddOrder.conformConfig(bobConfig, iDeployer)); + LibTestAddOrder.conformConfig(aliceConfig, iDeployer); + LibTestAddOrder.conformConfig(bobConfig, iDeployer); (Order memory aliceOrder, bytes32 aliceOrderHash) = addOrderWithChecks(alice, aliceConfig, aliceExpression); (Order memory bobOrder, bytes32 bobOrderHash) = addOrderWithChecks(bob, bobConfig, bobExpression); (aliceOrder); @@ -227,8 +153,8 @@ contract OrderBookAddOrderMockTest is OrderBookExternalMockTest, IMetaV1 { address expressionOne, address expressionTwo ) public { - vm.assume(LibTestAddOrder.conformConfig(configOne, iDeployer)); - vm.assume(LibTestAddOrder.conformConfig(configTwo, iDeployer)); + LibTestAddOrder.conformConfig(configOne, iDeployer); + LibTestAddOrder.conformConfig(configTwo, iDeployer); (Order memory expectedOrderOne, bytes32 expectedOrderOneHash) = LibTestAddOrder.expectedOrder(alice, configOne, iInterpreter, iStore, expressionOne); (Order memory expectedOrderTwo, bytes32 expectedOrderTwoHash) = diff --git a/test/concrete/OrderBook.addOrder.t.sol b/test/concrete/OrderBook.addOrder.t.sol index b4994cfd9..4ca326dd4 100644 --- a/test/concrete/OrderBook.addOrder.t.sol +++ b/test/concrete/OrderBook.addOrder.t.sol @@ -9,7 +9,7 @@ import "test/util/lib/LibTestAddOrder.sol"; contract OrderBookAddOrderTest is OrderBookExternalRealTest { /// No sources reverts as we need at least a calculate expression. function testAddOrderRealNoSourcesReverts(address owner, OrderConfig memory config) public { - vm.assume(LibTestAddOrder.conformConfig(config, iDeployer)); + LibTestAddOrder.conformConfig(config, iDeployer); config.evaluableConfig.sources = new bytes[](0); vm.expectRevert(abi.encodeWithSelector(OrderNoSources.selector, owner)); vm.prank(owner); @@ -18,7 +18,7 @@ contract OrderBookAddOrderTest is OrderBookExternalRealTest { /// No handle IO reverts. function testAddOrderRealNoHandleIOReverts(address owner, OrderConfig memory config) public { - vm.assume(LibTestAddOrder.conformConfig(config, iDeployer)); + LibTestAddOrder.conformConfig(config, iDeployer); (bytes[] memory sources, uint256[] memory constants) = IParserV1(address(iDeployer)).parse(":;"); (constants); config.evaluableConfig.sources = sources; @@ -29,7 +29,7 @@ contract OrderBookAddOrderTest is OrderBookExternalRealTest { /// A stack of 0 for calculate order reverts. function testAddOrderRealZeroStackCalculateReverts(address owner, OrderConfig memory config) public { - vm.assume(LibTestAddOrder.conformConfig(config, iDeployer)); + LibTestAddOrder.conformConfig(config, iDeployer); (bytes[] memory sources, uint256[] memory constants) = IParserV1(address(iDeployer)).parse(":;:;"); (constants); config.evaluableConfig.sources = sources; @@ -40,7 +40,7 @@ contract OrderBookAddOrderTest is OrderBookExternalRealTest { /// A stack of 1 for calculate order reverts. function testAddOrderRealOneStackCalculateReverts(address owner, OrderConfig memory config) public { - vm.assume(LibTestAddOrder.conformConfig(config, iDeployer)); + LibTestAddOrder.conformConfig(config, iDeployer); (bytes[] memory sources, uint256[] memory constants) = IParserV1(address(iDeployer)).parse("_:block-timestamp();:;"); (constants); @@ -52,7 +52,7 @@ contract OrderBookAddOrderTest is OrderBookExternalRealTest { /// A stack of 2 for calculate order deploys. function testAddOrderRealTwoStackCalculateReverts(address owner, OrderConfig memory config) public { - vm.assume(LibTestAddOrder.conformConfig(config, iDeployer)); + LibTestAddOrder.conformConfig(config, iDeployer); (bytes[] memory sources, uint256[] memory constants) = IParserV1(address(iDeployer)).parse("_ _:block-timestamp() chain-id();:;"); (constants); @@ -63,7 +63,7 @@ contract OrderBookAddOrderTest is OrderBookExternalRealTest { /// A stack of 3 for calculate order deploys. function testAddOrderRealThreeStackCalculate(address owner, OrderConfig memory config) public { - vm.assume(LibTestAddOrder.conformConfig(config, iDeployer)); + LibTestAddOrder.conformConfig(config, iDeployer); (bytes[] memory sources, uint256[] memory constants) = IParserV1(address(iDeployer)).parse("_ _ _:block-timestamp() chain-id() block-number();:;"); (constants); diff --git a/test/concrete/OrderBook.removeOrder.mock.t.sol b/test/concrete/OrderBook.removeOrder.mock.t.sol new file mode 100644 index 000000000..a5419ff35 --- /dev/null +++ b/test/concrete/OrderBook.removeOrder.mock.t.sol @@ -0,0 +1,193 @@ +// SPDX-License-Identifier: CAL +pragma solidity =0.8.19; + +import "test/util/abstract/OrderBookExternalMockTest.sol"; + +/// @title OrderBookRemoveOrderMockTest +/// @notice A contract to test the OrderBook removeOrder function. +contract OrderBookRemoveOrderMockTest is OrderBookExternalMockTest { + /// An order MUST ONLY be removable by its owner. + function testRemoveOrderOnlyOwner(address alice, address bob, OrderConfig memory config, address expression) + external + { + LibTestAddOrder.conformConfig(config, iDeployer); + vm.assume(alice != bob); + + (Order memory expectedOrder, bytes32 expectedOrderHash) = + LibTestAddOrder.expectedOrder(alice, config, iInterpreter, iStore, expression); + + // It will revert even if the order has not been added yet. + vm.expectRevert(abi.encodeWithSelector(NotOrderOwner.selector, bob, alice)); + vm.prank(bob); + iOrderbook.removeOrder(expectedOrder); + + // And will revert after the order is added. + (Order memory order, bytes32 orderHash) = addOrderWithChecks(alice, config, expression); + assertEq(orderHash, expectedOrderHash); + + vm.expectRevert(abi.encodeWithSelector(NotOrderOwner.selector, bob, alice)); + vm.prank(bob); + iOrderbook.removeOrder(order); + + // Alice can remove the order. + removeOrderWithChecks(alice, order); + + // It will revert even after the order has been removed. + vm.expectRevert(abi.encodeWithSelector(NotOrderOwner.selector, bob, alice)); + vm.prank(bob); + iOrderbook.removeOrder(order); + } + + /// The same order can be added and removed multiple times. + function testRemoveOrderAddRemoveMulti(address alice, OrderConfig memory config, address expression) external { + LibTestAddOrder.conformConfig(config, iDeployer); + + Order memory order; + bytes32 orderHashA; + bytes32 orderHashB; + // Each iteration is quite slow so 3 is about as much as we want to do. + for (uint256 i = 0; i < 3; i++) { + (order, orderHashB) = addOrderWithChecks(alice, config, expression); + removeOrderWithChecks(alice, order); + if (i > 0) { + assertEq(orderHashA, orderHashB); + } + orderHashA = orderHashB; + } + } + + /// An order MUST NOT change state if it does not exist. + function testRemoveOrderDoesNotExist(address alice, OrderConfig memory config, address expression) external { + LibTestAddOrder.conformConfig(config, iDeployer); + (Order memory order, bytes32 orderHash) = + LibTestAddOrder.expectedOrder(alice, config, iInterpreter, iStore, expression); + assertFalse(iOrderbook.orderExists(orderHash)); + vm.record(); + vm.recordLogs(); + vm.prank(alice); + assertFalse(iOrderbook.removeOrder(order)); + assertEq(vm.getRecordedLogs().length, 0); + (bytes32[] memory reads, bytes32[] memory writes) = vm.accesses(address(iOrderbook)); + // 3x for reentrancy guard, 1x for dead order check. + assertEq(reads.length, 4); + // 2x for reentrancy guard. + assertEq(writes.length, 2); + } + + /// Can add and remove different orders. + function testRemoveOrderDifferent( + address alice, + OrderConfig memory configOne, + address expressionOne, + OrderConfig memory configTwo, + address expressionTwo + ) external { + LibTestAddOrder.conformConfig(configOne, iDeployer); + LibTestAddOrder.conformConfig(configTwo, iDeployer); + + (Order memory expectedOrderOne, bytes32 expectedOrderHashOne) = + LibTestAddOrder.expectedOrder(alice, configOne, iInterpreter, iStore, expressionOne); + (Order memory expectedOrderTwo, bytes32 expectedOrderHashTwo) = + LibTestAddOrder.expectedOrder(alice, configTwo, iInterpreter, iStore, expressionTwo); + (expectedOrderOne); + (expectedOrderTwo); + vm.assume(expectedOrderHashOne != expectedOrderHashTwo); + + (Order memory orderOne, bytes32 orderHashOne) = addOrderWithChecks(alice, configOne, expressionOne); + (Order memory orderTwo, bytes32 orderHashTwo) = addOrderWithChecks(alice, configTwo, expressionTwo); + assertEq(orderHashOne, expectedOrderHashOne); + assertEq(orderHashTwo, expectedOrderHashTwo); + removeOrderWithChecks(alice, orderOne); + removeOrderWithChecks(alice, orderTwo); + } + + /// Different owners can add and remove the same order. + function testRemoveOrderDifferentOwners(address alice, address bob, OrderConfig memory config, address expression) + external + { + LibTestAddOrder.conformConfig(config, iDeployer); + vm.assume(alice != bob); + (Order memory orderAlice, bytes32 orderHashAlice) = addOrderWithChecks(alice, config, expression); + (Order memory orderBob, bytes32 orderHashBob) = addOrderWithChecks(bob, config, expression); + assertTrue(orderHashAlice != orderHashBob); + + // Owners can't interfere with each other. + vm.expectRevert(abi.encodeWithSelector(NotOrderOwner.selector, alice, bob)); + vm.prank(alice); + iOrderbook.removeOrder(orderBob); + + vm.expectRevert(abi.encodeWithSelector(NotOrderOwner.selector, bob, alice)); + vm.prank(bob); + iOrderbook.removeOrder(orderAlice); + + removeOrderWithChecks(alice, orderAlice); + removeOrderWithChecks(bob, orderBob); + } + + /// Different owners can add and remove different orders. + function testRemoveOrderDifferentOwnersDifferent( + address alice, + address bob, + OrderConfig memory configOne, + address expressionOne, + OrderConfig memory configTwo, + address expressionTwo + ) external { + { + LibTestAddOrder.conformConfig(configOne, iDeployer); + LibTestAddOrder.conformConfig(configTwo, iDeployer); + vm.assume(alice != bob); + + // Ensure the configs are different. + (Order memory expectedOrderOne, bytes32 expectedOrderHashOne) = + LibTestAddOrder.expectedOrder(address(0), configOne, iInterpreter, iStore, expressionOne); + (Order memory expectedOrderTwo, bytes32 expectedOrderHashTwo) = + LibTestAddOrder.expectedOrder(address(0), configTwo, iInterpreter, iStore, expressionTwo); + (expectedOrderOne); + (expectedOrderTwo); + vm.assume(expectedOrderHashOne != expectedOrderHashTwo); + } + + Order memory orderAliceOne; + Order memory orderBobOne; + Order memory orderAliceTwo; + Order memory orderBobTwo; + { + bytes32 orderHashAliceOne; + bytes32 orderHashBobOne; + bytes32 orderHashAliceTwo; + bytes32 orderHashBobTwo; + + (orderAliceOne, orderHashAliceOne) = addOrderWithChecks(alice, configOne, expressionOne); + (orderBobOne, orderHashBobOne) = addOrderWithChecks(bob, configOne, expressionOne); + (orderAliceTwo, orderHashAliceTwo) = addOrderWithChecks(alice, configTwo, expressionTwo); + (orderBobTwo, orderHashBobTwo) = addOrderWithChecks(bob, configTwo, expressionTwo); + assertTrue(orderHashAliceOne != orderHashAliceTwo); + assertTrue(orderHashAliceOne != orderHashBobOne); + assertTrue(orderHashAliceOne != orderHashBobTwo); + assertTrue(orderHashAliceTwo != orderHashBobOne); + assertTrue(orderHashAliceTwo != orderHashBobTwo); + assertTrue(orderHashBobOne != orderHashBobTwo); + } + + // Owners can't interfere with each other. + vm.expectRevert(abi.encodeWithSelector(NotOrderOwner.selector, alice, bob)); + vm.prank(alice); + iOrderbook.removeOrder(orderBobOne); + vm.expectRevert(abi.encodeWithSelector(NotOrderOwner.selector, alice, bob)); + vm.prank(alice); + iOrderbook.removeOrder(orderBobTwo); + + vm.expectRevert(abi.encodeWithSelector(NotOrderOwner.selector, bob, alice)); + vm.prank(bob); + iOrderbook.removeOrder(orderAliceOne); + vm.expectRevert(abi.encodeWithSelector(NotOrderOwner.selector, bob, alice)); + vm.prank(bob); + iOrderbook.removeOrder(orderAliceTwo); + + removeOrderWithChecks(alice, orderAliceOne); + removeOrderWithChecks(bob, orderBobOne); + removeOrderWithChecks(alice, orderAliceTwo); + removeOrderWithChecks(bob, orderBobTwo); + } +} diff --git a/test/concrete/OrderBook.withdraw.t.sol b/test/concrete/OrderBook.withdraw.t.sol index b5061bb16..d0dd71e35 100644 --- a/test/concrete/OrderBook.withdraw.t.sol +++ b/test/concrete/OrderBook.withdraw.t.sol @@ -143,7 +143,7 @@ contract OrderBookWithdrawTest is OrderBookExternalMockTest { for (uint256 i = 0; i < actions.length; i++) { // Deposit and withdrawal amounts must be positive. vm.assume(actions[i].amount > 0); - assumeNoPrecompiles(actions[i].token); + assumeNotPrecompile(actions[i].token); // Avoid errors from attempting to etch the orderbook. vm.assume(actions[i].token != address(iOrderbook)); } diff --git a/test/util/abstract/OrderBookExternalMockTest.sol b/test/util/abstract/OrderBookExternalMockTest.sol index 3fb576b5f..454e15239 100644 --- a/test/util/abstract/OrderBookExternalMockTest.sol +++ b/test/util/abstract/OrderBookExternalMockTest.sol @@ -2,11 +2,14 @@ pragma solidity =0.8.19; import "forge-std/Test.sol"; + import "rain.interpreter/interface/IExpressionDeployerV1.sol"; +import "rain.metadata/LibMeta.sol"; import "test/util/lib/LibTestConstants.sol"; import "test/util/lib/LibOrderBookConstants.sol"; import "test/util/abstract/IOrderBookV3Stub.sol"; +import "test/util/lib/LibTestAddOrder.sol"; import "src/concrete/OrderBook.sol"; @@ -21,7 +24,7 @@ import "src/concrete/OrderBook.sol"; /// /// Inherits from Test so that it can be used as a base contract for other tests. /// Implements IOrderBookV2 so that it has access to all the relevant events. -abstract contract OrderBookExternalMockTest is Test, IOrderBookV3Stub { +abstract contract OrderBookExternalMockTest is Test, IMetaV1, IOrderBookV3Stub { IInterpreterV1 immutable iInterpreter; IInterpreterStoreV1 immutable iStore; IExpressionDeployerV1 immutable iDeployer; @@ -55,4 +58,99 @@ abstract contract OrderBookExternalMockTest is Test, IOrderBookV3Stub { vm.etch(address(iToken1), REVERTING_MOCK_BYTECODE); vm.resumeGasMetering(); } + + /// Boilerplate to add an order with a mocked deployer and checks events and + /// storage accesses. + function addOrderWithChecks(address owner, OrderConfig memory config, address expression) + internal + returns (Order memory, bytes32) + { + config.evaluableConfig.deployer = iDeployer; + (Order memory order, bytes32 orderHash) = + LibTestAddOrder.expectedOrder(owner, config, iInterpreter, iStore, expression); + assertTrue(!iOrderbook.orderExists(orderHash)); + vm.mockCall( + address(iDeployer), + abi.encodeWithSelector(IExpressionDeployerV1.deployExpression.selector), + abi.encode(iInterpreter, iStore, expression) + ); + vm.expectEmit(false, false, false, true); + emit AddOrder(owner, iDeployer, order, orderHash); + if (config.meta.length > 0) { + vm.expectEmit(false, false, true, false); + // The subject of the meta is the order hash. + emit MetaV1(owner, uint256(orderHash), config.meta); + } + vm.record(); + vm.recordLogs(); + vm.prank(owner); + assertTrue(iOrderbook.addOrder(config)); + // MetaV1 is NOT emitted if the meta is empty. + assertEq(vm.getRecordedLogs().length, config.meta.length > 0 ? 2 : 1); + (bytes32[] memory reads, bytes32[] memory writes) = vm.accesses(address(iOrderbook)); + // 3x for reentrancy guard, 1x for dead order check, 1x for live write. + assertEq(reads.length, 5); + // 2x for reentrancy guard, 1x for live write. + assertEq(writes.length, 3); + assertTrue(iOrderbook.orderExists(orderHash)); + + // Adding the same order again MUST NOT change state. This MAY be + // impossible to encounter for a real expression deployer, as the + // deployer MAY NOT return the same address twice, but it is possible to + // mock. + vm.mockCall( + address(iDeployer), + abi.encodeWithSelector(IExpressionDeployerV1.deployExpression.selector), + abi.encode(iInterpreter, iStore, expression) + ); + vm.record(); + vm.recordLogs(); + vm.prank(owner); + assertFalse(iOrderbook.addOrder(config)); + assertEq(vm.getRecordedLogs().length, 0); + (reads, writes) = vm.accesses(address(iOrderbook)); + // 3x for reentrancy guard, 1x for dead order check. + assertEq(reads.length, 4); + // 2x for reentrancy guard. + assertEq(writes.length, 2); + assertTrue(iOrderbook.orderExists(orderHash)); + + return (order, orderHash); + } + + /// Boilerplate to remove an order with a mocked deployer and checks events + /// and storage accesses. + function removeOrderWithChecks(address owner, Order memory order) internal { + bytes32 orderHash = LibOrder.hash(order); + // This check assumes the order exists before we try to remove it. + assertTrue(iOrderbook.orderExists(orderHash)); + vm.expectEmit(false, false, false, true); + emit RemoveOrder(owner, order, orderHash); + vm.record(); + vm.recordLogs(); + vm.prank(owner); + // An order was removed so this is true as there is a state change. + assertTrue(iOrderbook.removeOrder(order)); + assertEq(vm.getRecordedLogs().length, 1); + (bytes32[] memory reads, bytes32[] memory writes) = vm.accesses(address(iOrderbook)); + // 3x for reentrancy guard, 1x for dead order check, 1x for dead write. + assertEq(reads.length, 5); + // 2x for reentrancy guard, 1x for dead write. + assertEq(writes.length, 3); + assertFalse(iOrderbook.orderExists(orderHash)); + + // Removing the same order again MUST NOT change state. + vm.record(); + vm.recordLogs(); + vm.prank(owner); + // There is no state change so this is false. + assertFalse(iOrderbook.removeOrder(order)); + assertEq(vm.getRecordedLogs().length, 0); + (reads, writes) = vm.accesses(address(iOrderbook)); + // 3x for reentrancy guard, 1x for dead order check. + assertEq(reads.length, 4); + // 2x for reentrancy guard. + assertEq(writes.length, 2); + assertFalse(iOrderbook.orderExists(orderHash)); + } } diff --git a/test/util/lib/LibTestAddOrder.sol b/test/util/lib/LibTestAddOrder.sol index 47d3920c2..bc2f41210 100644 --- a/test/util/lib/LibTestAddOrder.sol +++ b/test/util/lib/LibTestAddOrder.sol @@ -28,17 +28,33 @@ library LibTestAddOrder { return (order, LibOrder.hash(order)); } - /// Valid config has a few requirements. Mutates the config in place and - /// returns a bool that indicates whether the config is otherwise valid. The - /// bool should be passed directly to `vm.assume`. - function conformConfig(OrderConfig memory config, IExpressionDeployerV1 deployer) internal pure returns (bool) { + /// Valid config has a few requirements. Mutates the config in place. + /// Anything that doesn't meet the requirements will just be set to 0 values + /// as this is faster than forcing the fuzzer to rebuild with assume. + function conformConfig(OrderConfig memory config, IExpressionDeployerV1 deployer) internal pure { if (config.meta.length > 0) { // This is a bit of a hack, but it's the easiest way to get a valid // meta document. config.meta = abi.encodePacked(META_MAGIC_NUMBER_V1, config.meta); } config.evaluableConfig.deployer = deployer; - return config.evaluableConfig.sources.length >= 2 && config.validInputs.length > 0 - && config.validOutputs.length > 0; + if (config.validInputs.length == 0) { + config.validInputs = new IO[](1); + config.validInputs[0] = IO(address(0), 0, 0); + } + if (config.validOutputs.length == 0) { + config.validOutputs = new IO[](1); + config.validOutputs[0] = IO(address(0), 0, 0); + } + if (config.evaluableConfig.sources.length == 0) { + config.evaluableConfig.sources = new bytes[](2); + config.evaluableConfig.sources[0] = new bytes(0); + config.evaluableConfig.sources[1] = new bytes(0); + } else if (config.evaluableConfig.sources.length == 1) { + bytes[] memory sources = new bytes[](2); + sources[0] = config.evaluableConfig.sources[0]; + sources[1] = new bytes(0); + config.evaluableConfig.sources = sources; + } } } From 1a59a690af89feac7c16cba18f87b1f7cab896e3 Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Thu, 20 Jul 2023 19:15:49 +0400 Subject: [PATCH 3/6] test lib order --- .gas-snapshot | 2 ++ test/lib/LibOrder.t.sol | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 test/lib/LibOrder.t.sol diff --git a/.gas-snapshot b/.gas-snapshot index 305ccaeb9..d19e78ca1 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,5 +1,7 @@ GenericPoolOrderBookFlashBorrowerTest:testMinimumOutput(uint256,uint256) (runs: 5096, μ: 2315742, ~: 2319046) GenericPoolOrderBookFlashBorrowerTest:testTakeOrdersSender() (gas: 2230881) +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, μ: 3327619, ~: 3300143) 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, μ: 3171805, ~: 3178905) OrderBookAddOrderMockTest:testAddOrderTwoAccountsWithSameConfig(address,address,((address,uint8,uint256)[],(address,uint8,uint256)[],(address,bytes[],uint256[]),bytes),address) (runs: 5096, μ: 2973414, ~: 2969001) diff --git a/test/lib/LibOrder.t.sol b/test/lib/LibOrder.t.sol new file mode 100644 index 000000000..cf3b00467 --- /dev/null +++ b/test/lib/LibOrder.t.sol @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: CAL +pragma solidity =0.8.19; + +import "forge-std/Test.sol"; + +import "src/lib/LibOrder.sol"; + +/// @title LibOrderTest +/// Exercises the LibOrder library. +contract LibOrderTest is Test { + /// Hashing should always produce the same result for the same input. + function testHashEqual(Order memory a) public { + assertTrue(LibOrder.hash(a) == LibOrder.hash(a)); + } + + /// Hashing should always produce different results for different inputs. + function testHashNotEqual(Order memory a, Order memory b) public { + assertTrue(LibOrder.hash(a) != LibOrder.hash(b)); + } +} From 919ac5288571bc734e521c512179d8652b8cf14f Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Thu, 20 Jul 2023 19:25:57 +0400 Subject: [PATCH 4/6] workflows with specific nightly --- .github/workflows/ci-deploy.yaml | 4 +-- .../workflows/{qakit-docs.yaml => docs.yaml} | 4 +-- .../{qakit-slither.yaml => slither.yaml} | 4 +-- .github/workflows/snapshot.yml | 28 +++++++++++++++++++ .../workflows/{qakit-test.yaml => test.yaml} | 13 ++++----- 5 files changed, 39 insertions(+), 14 deletions(-) rename .github/workflows/{qakit-docs.yaml => docs.yaml} (78%) rename .github/workflows/{qakit-slither.yaml => slither.yaml} (73%) create mode 100644 .github/workflows/snapshot.yml rename .github/workflows/{qakit-test.yaml => test.yaml} (72%) diff --git a/.github/workflows/ci-deploy.yaml b/.github/workflows/ci-deploy.yaml index af3f6daf3..aaab68b8e 100644 --- a/.github/workflows/ci-deploy.yaml +++ b/.github/workflows/ci-deploy.yaml @@ -14,8 +14,8 @@ jobs: uses: DeterminateSystems/nix-installer-action@v4 - name: Install Foundry uses: foundry-rs/foundry-toolchain@v1 - with: - version: nightly + # https://github.com/foundry-rs/foundry/issues/3440#issuecomment-1641853058 + version: nightly-cc5637a979050c39b3d06bc4cc6134f0591ee8d0 - name: Install rain run: nix profile install github:rainprotocol/rain.cli - run: rain --version diff --git a/.github/workflows/qakit-docs.yaml b/.github/workflows/docs.yaml similarity index 78% rename from .github/workflows/qakit-docs.yaml rename to .github/workflows/docs.yaml index bff578909..e718dc12b 100644 --- a/.github/workflows/qakit-docs.yaml +++ b/.github/workflows/docs.yaml @@ -13,8 +13,8 @@ jobs: uses: actions/checkout@v3 - name: Install Foundry uses: foundry-rs/foundry-toolchain@v1 - with: - version: nightly + # https://github.com/foundry-rs/foundry/issues/3440#issuecomment-1641853058 + version: nightly-cc5637a979050c39b3d06bc4cc6134f0591ee8d0 - name: Generate docs run: forge doc -b - name: Deploy to GitHub Pages diff --git a/.github/workflows/qakit-slither.yaml b/.github/workflows/slither.yaml similarity index 73% rename from .github/workflows/qakit-slither.yaml rename to .github/workflows/slither.yaml index 362abeab8..4c2b808ff 100644 --- a/.github/workflows/qakit-slither.yaml +++ b/.github/workflows/slither.yaml @@ -9,8 +9,8 @@ jobs: # I found the installer flakey on CI. - name: Install Foundry uses: foundry-rs/foundry-toolchain@v1 - with: - version: nightly + # https://github.com/foundry-rs/foundry/issues/3440#issuecomment-1641853058 + version: nightly-cc5637a979050c39b3d06bc4cc6134f0591ee8d0 - name: Use Node.js uses: actions/setup-node@v3 diff --git a/.github/workflows/snapshot.yml b/.github/workflows/snapshot.yml new file mode 100644 index 000000000..ae2750d13 --- /dev/null +++ b/.github/workflows/snapshot.yml @@ -0,0 +1,28 @@ +name: Gas snapshot + +on: [push] + +env: + FOUNDRY_PROFILE: ci + +jobs: + check: + strategy: + fail-fast: true + + name: Gas snapshot + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + with: + submodules: recursive + + - name: Install Foundry + uses: foundry-rs/foundry-toolchain@v1 + with: + # https://github.com/foundry-rs/foundry/issues/3440#issuecomment-1641853058 + version: nightly-cc5637a979050c39b3d06bc4cc6134f0591ee8d0 + + - name: Check gas snapshots + run: forge snapshot --check + id: snapshot diff --git a/.github/workflows/qakit-test.yaml b/.github/workflows/test.yaml similarity index 72% rename from .github/workflows/qakit-test.yaml rename to .github/workflows/test.yaml index 3bf7ff394..dd231b19c 100644 --- a/.github/workflows/qakit-test.yaml +++ b/.github/workflows/test.yaml @@ -1,4 +1,4 @@ -name: QAKit test +name: Tests on: [push] @@ -10,7 +10,7 @@ jobs: strategy: fail-fast: true - name: QAKit Tests + name: Tests runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 @@ -20,7 +20,8 @@ jobs: - name: Install Foundry uses: foundry-rs/foundry-toolchain@v1 with: - version: nightly + # https://github.com/foundry-rs/foundry/issues/3440#issuecomment-1641853058 + version: nightly-cc5637a979050c39b3d06bc4cc6134f0591ee8d0 - name: Run Forge fmt run: forge fmt --check @@ -32,8 +33,4 @@ jobs: - name: Run Forge tests run: forge test -vvv --gas-report - id: test - - - name: Check gas snapshots - run: forge snapshot --check - id: snapshot + id: test \ No newline at end of file From 43058c4ae79a4e9eb7fc4b85112a7a69d7b304f4 Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Thu, 20 Jul 2023 19:27:56 +0400 Subject: [PATCH 5/6] lint --- .github/workflows/slither.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/slither.yaml b/.github/workflows/slither.yaml index 4c2b808ff..9356936cb 100644 --- a/.github/workflows/slither.yaml +++ b/.github/workflows/slither.yaml @@ -1,7 +1,7 @@ -name: QAKit Slither Analysis +name: Slither Analysis on: [push] jobs: - analyze: + slither-analyze: runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 From 16525a4987c411f33b5866e40945dd1459fa94c7 Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Thu, 20 Jul 2023 19:30:37 +0400 Subject: [PATCH 6/6] fix workflows --- .github/workflows/docs.yaml | 1 + .github/workflows/slither.yaml | 1 + 2 files changed, 2 insertions(+) diff --git a/.github/workflows/docs.yaml b/.github/workflows/docs.yaml index e718dc12b..d44bb4c79 100644 --- a/.github/workflows/docs.yaml +++ b/.github/workflows/docs.yaml @@ -13,6 +13,7 @@ jobs: uses: actions/checkout@v3 - name: Install Foundry uses: foundry-rs/foundry-toolchain@v1 + with: # https://github.com/foundry-rs/foundry/issues/3440#issuecomment-1641853058 version: nightly-cc5637a979050c39b3d06bc4cc6134f0591ee8d0 - name: Generate docs diff --git a/.github/workflows/slither.yaml b/.github/workflows/slither.yaml index 9356936cb..7d9b4b10e 100644 --- a/.github/workflows/slither.yaml +++ b/.github/workflows/slither.yaml @@ -9,6 +9,7 @@ jobs: # I found the installer flakey on CI. - name: Install Foundry uses: foundry-rs/foundry-toolchain@v1 + with: # https://github.com/foundry-rs/foundry/issues/3440#issuecomment-1641853058 version: nightly-cc5637a979050c39b3d06bc4cc6134f0591ee8d0 - name: Use Node.js