From 2799711ce60e0e43d0be18bbf15200f0b5fa4597 Mon Sep 17 00:00:00 2001 From: NanezX Date: Fri, 27 Oct 2023 20:23:23 -0400 Subject: [PATCH 1/5] added clear test that fail --- test/concrete/OrderBook.clear.mock.t.sol | 112 +++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 test/concrete/OrderBook.clear.mock.t.sol diff --git a/test/concrete/OrderBook.clear.mock.t.sol b/test/concrete/OrderBook.clear.mock.t.sol new file mode 100644 index 000000000..6f635632b --- /dev/null +++ b/test/concrete/OrderBook.clear.mock.t.sol @@ -0,0 +1,112 @@ +// SPDX-License-Identifier: CAL +pragma solidity =0.8.19; + +import "lib/forge-std/src/Test.sol"; + +import "test/util/abstract/OrderBookExternalMockTest.sol"; +import {LibTestAddOrder} from "test/util/lib/LibTestAddOrder.sol"; + +/// @title OrderBookClearTest +/// Tests clearing an order. +contract OrderBookClearTest is OrderBookExternalMockTest { + function testClearSimple( + address alice, + OrderConfigV2 memory aliceConfig, + uint256 aliceVaultId, + address bob, + OrderConfigV2 memory bobConfig, + uint256 bobVaultId, + address expression, + address bountyBot, + uint256 aliceBountyVaultId, + uint256 bobBountyVaultId + ) public { + // Different accounts + vm.assume(alice != bob); + vm.assume(alice != bountyBot); + vm.assume(bob != bountyBot); + + // -- Add two orders with similar IO tokens (swapped) + // Add alice order with a input token (iToken0) and output token (iToken1) + (Order memory aliceOrder, bytes32 aliceOrderHash) = _addOrderMockInternal(alice, aliceConfig, expression, iToken0, iToken1); + assertTrue(iOrderbook.orderExists(aliceOrderHash)); + + // Add bob order with a input token (iToken1) and output token (iToken0) + (Order memory bobOrder, bytes32 bobOrderHash) = _addOrderMockInternal(bob, bobConfig, expression, iToken1, iToken0); + assertTrue(iOrderbook.orderExists(bobOrderHash)); + + // 2e18 tokens will be deposit for both (alice and bob) + uint256 amount = 2e18; + + // Alice deposit his output token + _depositInternal(alice, iToken1, aliceVaultId, amount); + + // Bob deposit his output token + _depositInternal(bob, iToken0, bobVaultId, amount); + + // Since all the IO are just 1 length, the IOIndex will be zero (0). + // And vaultIds for the clearer + ClearConfig memory configClear = ClearConfig(0, 0, 0, 0, aliceBountyVaultId, bobBountyVaultId); + + // Mock the interpreter.eval that is used inside clear().calculateOrderIO() + // Produce the stack output for OB + uint256[] memory orderStack = new uint256[](2); + orderStack[0] = 1e18; // orderOutputMax + orderStack[1] = 1; // orderIORatio + vm.mockCall( + address(iInterpreter), + abi.encodeWithSelector(IInterpreterV1.eval.selector), + abi.encode(orderStack, new uint256[](0)) + ); + + // Clear the order using `bountyBot` address as caller clearer. + vm.prank(bountyBot); + iOrderbook.clear(aliceOrder, bobOrder, configClear, new SignedContextV1[](0), new SignedContextV1[](0)); + } + + /// Add an order using an owner (the caller) and modify the valid IOs to have + /// just one valid IO from an input and output tokens. + function _addOrderMockInternal( + address owner, + OrderConfigV2 memory config, + address expression, + IERC20 inputToken, + IERC20 outputToken + ) internal returns (Order memory, bytes32) { + vm.assume(config.validInputs.length > 0); + vm.assume(config.validOutputs.length > 0); + config.evaluableConfig.bytecode = hex"02000000040000000000000000"; + config.meta = new bytes(0); + + config.validInputs = _helperBuildIO(config.validInputs, address(inputToken), 18); + config.validOutputs = _helperBuildIO(config.validOutputs, address(outputToken), 18); + + return addOrderWithChecks(owner, config, expression); + } + + /// Make a deposit to the OB mocking the internal transferFrom call. + function _depositInternal(address depositor, IERC20 token, uint256 vaultId, uint256 amount) internal { + vm.prank(depositor); + vm.mockCall( + address(token), + abi.encodeWithSelector(IERC20.transferFrom.selector, depositor, address(iOrderbook), amount), + abi.encode(true) + ); + iOrderbook.deposit(address(token), vaultId, amount); + + // Check that the vaultBalance was updated + assertEq(iOrderbook.vaultBalance(depositor, address(token), vaultId), amount); + } + + /// Edit a given IO array to have only one index, with a given token and decimal. + /// This is useful to make matched Orders to do clears. + function _helperBuildIO(IO[] memory io, address newToken, uint8 newDecimals) pure internal returns (IO[] memory) { + IO[] memory ioAux = new IO[](1); + + ioAux[0] = io[0]; + ioAux[0].token = newToken; + ioAux[0].decimals = newDecimals; + + return ioAux; + } +} From e0ba59c1cbfd0e9acd75f7843fe37b7cb8209054 Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Sat, 28 Oct 2023 20:22:15 +0400 Subject: [PATCH 2/5] fix test for clear --- test/concrete/OrderBook.clear.mock.t.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/concrete/OrderBook.clear.mock.t.sol b/test/concrete/OrderBook.clear.mock.t.sol index 6f635632b..916aa3d4b 100644 --- a/test/concrete/OrderBook.clear.mock.t.sol +++ b/test/concrete/OrderBook.clear.mock.t.sol @@ -36,7 +36,7 @@ contract OrderBookClearTest is OrderBookExternalMockTest { assertTrue(iOrderbook.orderExists(bobOrderHash)); // 2e18 tokens will be deposit for both (alice and bob) - uint256 amount = 2e18; + uint256 amount = 2e18; // Alice deposit his output token _depositInternal(alice, iToken1, aliceVaultId, amount); @@ -52,7 +52,7 @@ contract OrderBookClearTest is OrderBookExternalMockTest { // Produce the stack output for OB uint256[] memory orderStack = new uint256[](2); orderStack[0] = 1e18; // orderOutputMax - orderStack[1] = 1; // orderIORatio + orderStack[1] = 1e18; // orderIORatio vm.mockCall( address(iInterpreter), abi.encodeWithSelector(IInterpreterV1.eval.selector), @@ -64,7 +64,7 @@ contract OrderBookClearTest is OrderBookExternalMockTest { iOrderbook.clear(aliceOrder, bobOrder, configClear, new SignedContextV1[](0), new SignedContextV1[](0)); } - /// Add an order using an owner (the caller) and modify the valid IOs to have + /// Add an order using an owner (the caller) and modify the valid IOs to have /// just one valid IO from an input and output tokens. function _addOrderMockInternal( address owner, From 799bc79052bcbccca691a460cfa0680b075992c1 Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Sat, 28 Oct 2023 22:00:34 +0400 Subject: [PATCH 3/5] fix clear --- src/concrete/OrderBook.sol | 25 +++++++++++++----------- test/concrete/OrderBook.clear.mock.t.sol | 11 ++++++++--- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/concrete/OrderBook.sol b/src/concrete/OrderBook.sol index f0c264acd..3f6fdf466 100644 --- a/src/concrete/OrderBook.sol +++ b/src/concrete/OrderBook.sol @@ -569,17 +569,17 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash // Emit the Clear event before `eval`. emit Clear(msg.sender, alice, bob, clearConfig); } - OrderIOCalculation memory aliceOrderIOCalculation_ = calculateOrderIO( + OrderIOCalculation memory aliceOrderIOCalculation = calculateOrderIO( alice, clearConfig.aliceInputIOIndex, clearConfig.aliceOutputIOIndex, bob.owner, bobSignedContext ); - OrderIOCalculation memory bobOrderIOCalculation_ = calculateOrderIO( + OrderIOCalculation memory bobOrderIOCalculation = calculateOrderIO( bob, clearConfig.bobInputIOIndex, clearConfig.bobOutputIOIndex, alice.owner, aliceSignedContext ); ClearStateChange memory clearStateChange = - calculateClearStateChange(aliceOrderIOCalculation_, bobOrderIOCalculation_); + calculateClearStateChange(aliceOrderIOCalculation, bobOrderIOCalculation); - recordVaultIO(alice, clearStateChange.aliceInput, clearStateChange.aliceOutput, aliceOrderIOCalculation_); - recordVaultIO(bob, clearStateChange.bobInput, clearStateChange.bobOutput, bobOrderIOCalculation_); + recordVaultIO(alice, clearStateChange.aliceInput, clearStateChange.aliceOutput, aliceOrderIOCalculation); + recordVaultIO(bob, clearStateChange.bobInput, clearStateChange.bobOutput, bobOrderIOCalculation); { // At least one of these will overflow due to negative bounties if @@ -793,16 +793,19 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash OrderIOCalculation memory aliceOrderIOCalculation, OrderIOCalculation memory bobOrderIOCalculation ) internal pure returns (ClearStateChange memory clearStateChange) { - calculateClearStateAlice(clearStateChange, aliceOrderIOCalculation, bobOrderIOCalculation); + // Calculate the clear state change for Alice. + (clearStateChange.aliceInput, clearStateChange.aliceOutput) = + calculateClearStateAlice(aliceOrderIOCalculation, bobOrderIOCalculation); + // Flip alice and bob to calculate bob's output. - calculateClearStateAlice(clearStateChange, bobOrderIOCalculation, aliceOrderIOCalculation); + (clearStateChange.bobInput, clearStateChange.bobOutput) = + calculateClearStateAlice(bobOrderIOCalculation, aliceOrderIOCalculation); } function calculateClearStateAlice( - ClearStateChange memory clearStateChange, OrderIOCalculation memory aliceOrderIOCalculation, OrderIOCalculation memory bobOrderIOCalculation - ) internal pure { + ) internal pure returns (uint256 aliceInput, uint256 aliceOutput) { // Always round IO calculations up so that the counterparty pays more. // This is the max input that bob can afford, given his own IO ratio // and maximum spend/output. @@ -818,7 +821,7 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash } // Alice's final output is the scaled version of the 18 decimal output, // rounded down to benefit Alice. - clearStateChange.aliceOutput = Output18Amount.unwrap(aliceOutputMax18).scaleN( + aliceOutput = Output18Amount.unwrap(aliceOutputMax18).scaleN( aliceOrderIOCalculation.order.validOutputs[aliceOrderIOCalculation.outputIOIndex].decimals, 0 ); @@ -826,7 +829,7 @@ contract OrderBook is IOrderBookV3, ReentrancyGuard, Multicall, OrderBookV3Flash Input18Amount aliceInput18 = Input18Amount.wrap( Output18Amount.unwrap(aliceOutputMax18).fixedPointMul(aliceOrderIOCalculation.IORatio, Math.Rounding.Up) ); - clearStateChange.aliceInput = + aliceInput = // Use bob's output decimals as alice's input decimals. // // This is only safe if we have previously checked that the decimals diff --git a/test/concrete/OrderBook.clear.mock.t.sol b/test/concrete/OrderBook.clear.mock.t.sol index 916aa3d4b..b15f445d4 100644 --- a/test/concrete/OrderBook.clear.mock.t.sol +++ b/test/concrete/OrderBook.clear.mock.t.sol @@ -28,11 +28,13 @@ contract OrderBookClearTest is OrderBookExternalMockTest { // -- Add two orders with similar IO tokens (swapped) // Add alice order with a input token (iToken0) and output token (iToken1) - (Order memory aliceOrder, bytes32 aliceOrderHash) = _addOrderMockInternal(alice, aliceConfig, expression, iToken0, iToken1); + (Order memory aliceOrder, bytes32 aliceOrderHash) = + _addOrderMockInternal(alice, aliceConfig, expression, iToken0, iToken1); assertTrue(iOrderbook.orderExists(aliceOrderHash)); // Add bob order with a input token (iToken1) and output token (iToken0) - (Order memory bobOrder, bytes32 bobOrderHash) = _addOrderMockInternal(bob, bobConfig, expression, iToken1, iToken0); + (Order memory bobOrder, bytes32 bobOrderHash) = + _addOrderMockInternal(bob, bobConfig, expression, iToken1, iToken0); assertTrue(iOrderbook.orderExists(bobOrderHash)); // 2e18 tokens will be deposit for both (alice and bob) @@ -50,6 +52,7 @@ contract OrderBookClearTest is OrderBookExternalMockTest { // Mock the interpreter.eval that is used inside clear().calculateOrderIO() // Produce the stack output for OB + console2.log("Mocking interpreter.eval"); uint256[] memory orderStack = new uint256[](2); orderStack[0] = 1e18; // orderOutputMax orderStack[1] = 1e18; // orderIORatio @@ -58,6 +61,8 @@ contract OrderBookClearTest is OrderBookExternalMockTest { abi.encodeWithSelector(IInterpreterV1.eval.selector), abi.encode(orderStack, new uint256[](0)) ); + console2.log("orderStack[0] = %s", orderStack[0]); + console2.logBytes(abi.encode(orderStack, new uint256[](0))); // Clear the order using `bountyBot` address as caller clearer. vm.prank(bountyBot); @@ -100,7 +105,7 @@ contract OrderBookClearTest is OrderBookExternalMockTest { /// Edit a given IO array to have only one index, with a given token and decimal. /// This is useful to make matched Orders to do clears. - function _helperBuildIO(IO[] memory io, address newToken, uint8 newDecimals) pure internal returns (IO[] memory) { + function _helperBuildIO(IO[] memory io, address newToken, uint8 newDecimals) internal pure returns (IO[] memory) { IO[] memory ioAux = new IO[](1); ioAux[0] = io[0]; From 66b0586c1ae99a07a3d47858142bac585cc5dfc6 Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Sat, 28 Oct 2023 22:01:05 +0400 Subject: [PATCH 4/5] lint --- test/concrete/OrderBook.clear.mock.t.sol | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/concrete/OrderBook.clear.mock.t.sol b/test/concrete/OrderBook.clear.mock.t.sol index b15f445d4..29d17534a 100644 --- a/test/concrete/OrderBook.clear.mock.t.sol +++ b/test/concrete/OrderBook.clear.mock.t.sol @@ -52,7 +52,6 @@ contract OrderBookClearTest is OrderBookExternalMockTest { // Mock the interpreter.eval that is used inside clear().calculateOrderIO() // Produce the stack output for OB - console2.log("Mocking interpreter.eval"); uint256[] memory orderStack = new uint256[](2); orderStack[0] = 1e18; // orderOutputMax orderStack[1] = 1e18; // orderIORatio @@ -61,8 +60,6 @@ contract OrderBookClearTest is OrderBookExternalMockTest { abi.encodeWithSelector(IInterpreterV1.eval.selector), abi.encode(orderStack, new uint256[](0)) ); - console2.log("orderStack[0] = %s", orderStack[0]); - console2.logBytes(abi.encode(orderStack, new uint256[](0))); // Clear the order using `bountyBot` address as caller clearer. vm.prank(bountyBot); From f9ac0d9c30703f097e46a87df49daa892fd00ead Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Mon, 30 Oct 2023 11:03:10 +0400 Subject: [PATCH 5/5] lint --- src/concrete/OrderBook.sol | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/concrete/OrderBook.sol b/src/concrete/OrderBook.sol index 3f6fdf466..50fefc1df 100644 --- a/src/concrete/OrderBook.sol +++ b/src/concrete/OrderBook.sol @@ -1,22 +1,22 @@ // SPDX-License-Identifier: CAL pragma solidity =0.8.19; -import {Math} from "lib/openzeppelin-contracts/contracts/utils/math/Math.sol"; -import {Multicall} from "lib/openzeppelin-contracts/contracts/utils/Multicall.sol"; -import {IERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/IERC20.sol"; -import {SafeERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol"; -import {ReentrancyGuard} from "lib/openzeppelin-contracts/contracts/security/ReentrancyGuard.sol"; - -import "lib/rain.math.fixedpoint/src/lib/LibFixedPointDecimalArithmeticOpenZeppelin.sol"; -import "lib/rain.math.fixedpoint/src/lib/LibFixedPointDecimalScale.sol"; -import "lib/rain.interpreter/src/lib/caller/LibEncodedDispatch.sol"; -import "lib/rain.interpreter/src/lib/caller/LibContext.sol"; +import {Math} from "openzeppelin-contracts/contracts/utils/math/Math.sol"; +import {Multicall} from "openzeppelin-contracts/contracts/utils/Multicall.sol"; +import {IERC20} from "openzeppelin-contracts/contracts/token/ERC20/IERC20.sol"; +import {SafeERC20} from "openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol"; +import {ReentrancyGuard} from "openzeppelin-contracts/contracts/security/ReentrancyGuard.sol"; + +import "rain.math.fixedpoint/lib/LibFixedPointDecimalArithmeticOpenZeppelin.sol"; +import "rain.math.fixedpoint/lib/LibFixedPointDecimalScale.sol"; +import "rain.interpreter/src/lib/caller/LibEncodedDispatch.sol"; +import "rain.interpreter/src/lib/caller/LibContext.sol"; import { DeployerDiscoverableMetaV2, DeployerDiscoverableMetaV2ConstructionConfig, LibMeta -} from "lib/rain.interpreter/src/abstract/DeployerDiscoverableMetaV2.sol"; -import "lib/rain.interpreter/src/lib/bytecode/LibBytecode.sol"; +} from "rain.interpreter/src/abstract/DeployerDiscoverableMetaV2.sol"; +import "rain.interpreter/src/lib/bytecode/LibBytecode.sol"; import "../interface/unstable/IOrderBookV3.sol"; import "../interface/unstable/IOrderBookV3OrderTaker.sol";