From a040ecb63b9c0d1661a07aa1be79e88ef8934e13 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Fri, 15 Sep 2023 13:12:32 -0400 Subject: [PATCH 01/13] =?UTF-8?q?=F0=9F=91=B7=F0=9F=8F=BB=E2=80=8D?= =?UTF-8?q?=E2=99=82=EF=B8=8F=20Refactor=20constants?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Engine.sol | 52 +++++--------------- src/libraries/Constants.sol | 41 +++++++++++++++ test/ConditionalOrder.t.sol | 32 ++++++------ test/utils/analysis/ConditionalOrderFees.sol | 10 ++-- test/utils/exposed/EngineExposed.sol | 18 ++++++- 5 files changed, 93 insertions(+), 60 deletions(-) create mode 100644 src/libraries/Constants.sol diff --git a/src/Engine.sol b/src/Engine.sol index ad0fb582..e3dc980a 100644 --- a/src/Engine.sol +++ b/src/Engine.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.18; import {ConditionalOrderHashLib} from "src/libraries/ConditionalOrderHashLib.sol"; +import {Constants} from "src/libraries/Constants.sol"; import {EIP712} from "src/utils/EIP712.sol"; import {IEngine, IPerpsMarketProxy} from "src/interfaces/IEngine.sol"; import {IERC20} from "src/interfaces/tokens/IERC20.sol"; @@ -23,36 +24,6 @@ contract Engine is IEngine, Multicallable, EIP712 { using ConditionalOrderHashLib for OrderDetails; using ConditionalOrderHashLib for ConditionalOrder; - /*////////////////////////////////////////////////////////////// - CONSTANTS - //////////////////////////////////////////////////////////////*/ - - /// @notice admins have permission to do everything that the account owner can - /// (including granting and revoking permissions for other addresses) except - /// for transferring account ownership - bytes32 internal constant ADMIN_PERMISSION = "ADMIN"; - - /// @notice "0" synthMarketId represents sUSD in Synthetix v3 - uint128 internal constant USD_SYNTH_ID = 0; - - /// @notice max fee that can be charged for a conditional order execution - /// @dev 50 USD - uint256 public constant UPPER_FEE_CAP = 50 ether; - - /// @notice min fee that can be charged for a conditional order execution - /// @dev 2 USD - uint256 public constant LOWER_FEE_CAP = 2 ether; - - /// @notice percentage of the simulated order fee that is charged for a conditional order execution - /// @dev denoted in BPS (basis points) where 1% = 100 BPS and 100% = 10000 BPS - uint256 public constant FEE_SCALING_FACTOR = 1000; - - /// @notice max BPS - uint256 internal constant MAX_BPS = 10_000; - - /// @notice max number of conditions that can be defined for a conditional order - uint256 internal constant MAX_CONDITIONS = 8; - /*////////////////////////////////////////////////////////////// IMMUTABLES //////////////////////////////////////////////////////////////*/ @@ -125,7 +96,7 @@ contract Engine is IEngine, Multicallable, EIP712 { returns (bool) { return PERPS_MARKET_PROXY.hasPermission( - _accountId, ADMIN_PERMISSION, _caller + _accountId, Constants.ADMIN_PERMISSION, _caller ); } @@ -255,7 +226,7 @@ contract Engine is IEngine, Multicallable, EIP712 { view returns (address synthAddress) { - synthAddress = _synthMarketId == USD_SYNTH_ID + synthAddress = _synthMarketId == Constants.USD_SYNTH_ID ? address(SUSD) : SPOT_MARKET_PROXY.getSynth(_synthMarketId); } @@ -384,13 +355,14 @@ contract Engine is IEngine, Multicallable, EIP712 { }); /// @dev calculate conditional order fee based on scaled order fees - conditionalOrderFee = (orderFees * FEE_SCALING_FACTOR) / MAX_BPS; + conditionalOrderFee = + (orderFees * Constants.FEE_SCALING_FACTOR) / Constants.MAX_BPS; /// @dev ensure conditional order fee is within bounds - if (conditionalOrderFee < LOWER_FEE_CAP) { - conditionalOrderFee = LOWER_FEE_CAP; - } else if (conditionalOrderFee > UPPER_FEE_CAP) { - conditionalOrderFee = UPPER_FEE_CAP; + if (conditionalOrderFee < Constants.LOWER_FEE_CAP) { + conditionalOrderFee = Constants.LOWER_FEE_CAP; + } else if (conditionalOrderFee > Constants.UPPER_FEE_CAP) { + conditionalOrderFee = Constants.UPPER_FEE_CAP; } /// @dev withdraw conditional order fee from account prior to executing order @@ -398,7 +370,7 @@ contract Engine is IEngine, Multicallable, EIP712 { _to: msg.sender, _synth: SUSD, _accountId: _co.orderDetails.accountId, - _synthMarketId: USD_SYNTH_ID, + _synthMarketId: Constants.USD_SYNTH_ID, _amount: -int256(conditionalOrderFee) }); @@ -476,7 +448,9 @@ contract Engine is IEngine, Multicallable, EIP712 { returns (bool) { uint256 length = _co.conditions.length; - if (length > MAX_CONDITIONS) revert MaxConditionSizeExceeded(); + if (length > Constants.MAX_CONDITIONS) { + revert MaxConditionSizeExceeded(); + } for (uint256 i = 0; i < length;) { bool success; bytes memory response; diff --git a/src/libraries/Constants.sol b/src/libraries/Constants.sol new file mode 100644 index 00000000..b078de55 --- /dev/null +++ b/src/libraries/Constants.sol @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity 0.8.18; + +/// @title Kwenta Smart Margin v3: Constants Library +/// @author JaredBorders (jaredborders@pm.me) +library Constants { + /*////////////////////////////////////////////////////////////// + CONSTANTS + //////////////////////////////////////////////////////////////*/ + + /// @notice admins have permission to do everything that the account owner can + /// (including granting and revoking permissions for other addresses) except + /// for transferring account ownership + bytes32 internal constant ADMIN_PERMISSION = "ADMIN"; + + /// @notice the permission required to commit an async order + /// @dev this permission does not allow the permission holder to modify collateral + bytes32 internal constant PERPS_COMMIT_ASYNC_ORDER_PERMISSION = + "PERPS_COMMIT_ASYNC_ORDER"; + + /// @notice "0" synthMarketId represents sUSD in Synthetix v3 + uint128 internal constant USD_SYNTH_ID = 0; + + /// @notice max fee that can be charged for a conditional order execution + /// @dev 50 USD + uint256 internal constant UPPER_FEE_CAP = 50 ether; + + /// @notice min fee that can be charged for a conditional order execution + /// @dev 2 USD + uint256 internal constant LOWER_FEE_CAP = 2 ether; + + /// @notice percentage of the simulated order fee that is charged for a conditional order execution + /// @dev denoted in BPS (basis points) where 1% = 100 BPS and 100% = 10000 BPS + uint256 internal constant FEE_SCALING_FACTOR = 1000; + + /// @notice max BPS + uint256 internal constant MAX_BPS = 10_000; + + /// @notice max number of conditions that can be defined for a conditional order + uint256 internal constant MAX_CONDITIONS = 8; +} diff --git a/test/ConditionalOrder.t.sol b/test/ConditionalOrder.t.sol index 4fb417aa..e34417d5 100644 --- a/test/ConditionalOrder.t.sol +++ b/test/ConditionalOrder.t.sol @@ -538,7 +538,7 @@ contract Execute is ConditionalOrderTest { ); uint256 marginPostConditionalOrderFee = AMOUNT - - (orderFees = orderFees * engine.FEE_SCALING_FACTOR() / 10_000); + - (orderFees = orderFees * engineExposed.FEE_SCALING_FACTOR() / 10_000); vm.expectRevert( abi.encodeWithSelector( @@ -623,7 +623,7 @@ contract Fee is ConditionalOrderTest { function test_fee_imposed_at_upper_fee_cap() public { uint256 mocked_order_fees = - engine.UPPER_FEE_CAP() * engine.FEE_SCALING_FACTOR(); + engineExposed.UPPER_FEE_CAP() * engineExposed.FEE_SCALING_FACTOR(); mock_computeOrderFees({ perpsMarketProxy: address(perpsMarketProxy), @@ -661,13 +661,13 @@ contract Fee is ConditionalOrderTest { (,, uint256 conditionalOrderFee) = engine.execute(co, signature); - assertEq(engine.UPPER_FEE_CAP(), conditionalOrderFee); - assertEq(engine.UPPER_FEE_CAP(), sUSD.balanceOf(address(this))); + assertEq(engineExposed.UPPER_FEE_CAP(), conditionalOrderFee); + assertEq(engineExposed.UPPER_FEE_CAP(), sUSD.balanceOf(address(this))); } function test_fee_imposed_above_upper_fee_cap() public { - uint256 mocked_order_fees = - engine.UPPER_FEE_CAP() * (engine.FEE_SCALING_FACTOR() + 1); + uint256 mocked_order_fees = engineExposed.UPPER_FEE_CAP() + * (engineExposed.FEE_SCALING_FACTOR() + 1); mock_computeOrderFees({ perpsMarketProxy: address(perpsMarketProxy), @@ -705,12 +705,12 @@ contract Fee is ConditionalOrderTest { (,, uint256 conditionalOrderFee) = engine.execute(co, signature); - assertEq(engine.UPPER_FEE_CAP(), conditionalOrderFee); - assertEq(engine.UPPER_FEE_CAP(), sUSD.balanceOf(address(this))); + assertEq(engineExposed.UPPER_FEE_CAP(), conditionalOrderFee); + assertEq(engineExposed.UPPER_FEE_CAP(), sUSD.balanceOf(address(this))); } function test_fee_imposed_below_upper_fee_cap() public { - uint256 mocked_order_fees = engine.UPPER_FEE_CAP(); + uint256 mocked_order_fees = engineExposed.UPPER_FEE_CAP(); mock_computeOrderFees({ perpsMarketProxy: address(perpsMarketProxy), @@ -749,11 +749,13 @@ contract Fee is ConditionalOrderTest { (,, uint256 conditionalOrderFee) = engine.execute(co, signature); assertEq( - (engine.UPPER_FEE_CAP() * engine.FEE_SCALING_FACTOR()) / 10_000, + (engineExposed.UPPER_FEE_CAP() * engineExposed.FEE_SCALING_FACTOR()) + / 10_000, conditionalOrderFee ); assertEq( - (engine.UPPER_FEE_CAP() * engine.FEE_SCALING_FACTOR()) / 10_000, + (engineExposed.UPPER_FEE_CAP() * engineExposed.FEE_SCALING_FACTOR()) + / 10_000, sUSD.balanceOf(address(this)) ); } @@ -797,8 +799,8 @@ contract Fee is ConditionalOrderTest { (,, uint256 conditionalOrderFee) = engine.execute(co, signature); - assertEq(engine.LOWER_FEE_CAP(), conditionalOrderFee); - assertEq(engine.LOWER_FEE_CAP(), sUSD.balanceOf(address(this))); + assertEq(engineExposed.LOWER_FEE_CAP(), conditionalOrderFee); + assertEq(engineExposed.LOWER_FEE_CAP(), sUSD.balanceOf(address(this))); } function test_fee_imposed_fee_cannot_be_paid() public { @@ -841,7 +843,7 @@ contract Fee is ConditionalOrderTest { sizeDelta: 10 ether }); - orderFees = orderFees * engine.FEE_SCALING_FACTOR() / 10_000; + orderFees = orderFees * engineExposed.FEE_SCALING_FACTOR() / 10_000; vm.expectRevert( abi.encodeWithSelector( @@ -891,7 +893,7 @@ contract Fee is ConditionalOrderTest { ); uint256 marginPostConditionalOrderFee = AMOUNT - - (orderFees = orderFees * engine.FEE_SCALING_FACTOR() / 10_000); + - (orderFees = orderFees * engineExposed.FEE_SCALING_FACTOR() / 10_000); vm.expectRevert( abi.encodeWithSelector( diff --git a/test/utils/analysis/ConditionalOrderFees.sol b/test/utils/analysis/ConditionalOrderFees.sol index c479b9ed..78df1d91 100644 --- a/test/utils/analysis/ConditionalOrderFees.sol +++ b/test/utils/analysis/ConditionalOrderFees.sol @@ -18,12 +18,12 @@ contract ConditionalOrderTest is Bootstrap { }); uint256 conditional_order_fee = - orderFees * engine.FEE_SCALING_FACTOR() / 10_000; + orderFees * engineExposed.FEE_SCALING_FACTOR() / 10_000; - if (conditional_order_fee < engine.LOWER_FEE_CAP()) { - console2.log("%s,", engine.LOWER_FEE_CAP() / 1e18); - } else if (conditional_order_fee > engine.UPPER_FEE_CAP()) { - console2.log("%s,", engine.UPPER_FEE_CAP() / 1e18); + if (conditional_order_fee < engineExposed.LOWER_FEE_CAP()) { + console2.log("%s,", engineExposed.LOWER_FEE_CAP() / 1e18); + } else if (conditional_order_fee > engineExposed.UPPER_FEE_CAP()) { + console2.log("%s,", engineExposed.UPPER_FEE_CAP() / 1e18); } else { console2.log("%s,", conditional_order_fee / 1e18); } diff --git a/test/utils/exposed/EngineExposed.sol b/test/utils/exposed/EngineExposed.sol index 9def0fa6..d638eb43 100644 --- a/test/utils/exposed/EngineExposed.sol +++ b/test/utils/exposed/EngineExposed.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-3.0-or-later pragma solidity 0.8.18; -import {Engine, MathLib} from "src/Engine.sol"; +import {Engine, Constants, MathLib} from "src/Engine.sol"; contract EngineExposed is Engine { using MathLib for uint256; @@ -20,4 +20,20 @@ contract EngineExposed is Engine { { return _getSynthAddress(synthMarketId); } + + function UPPER_FEE_CAP() public pure returns (uint256) { + return Constants.UPPER_FEE_CAP; + } + + function LOWER_FEE_CAP() public pure returns (uint256) { + return Constants.LOWER_FEE_CAP; + } + + function FEE_SCALING_FACTOR() public pure returns (uint256) { + return Constants.FEE_SCALING_FACTOR; + } + + function MAX_BPS() public pure returns (uint256) { + return Constants.MAX_BPS; + } } From b4ddb7d39e96b9cbf3891fa7aea52e449d373703 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Fri, 15 Sep 2023 13:31:12 -0400 Subject: [PATCH 02/13] =?UTF-8?q?=F0=9F=91=B7=F0=9F=8F=BB=E2=80=8D?= =?UTF-8?q?=E2=99=82=EF=B8=8F=20H-2:=20Rework=20delegate=20permission=20lo?= =?UTF-8?q?gic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Engine.sol | 2 +- src/interfaces/IEngine.sol | 2 ++ test/Authentication.t.sol | 8 +++++--- test/ConditionalOrder.t.sol | 10 +++++++++- test/NonceBitmap.t.sol | 2 +- test/utils/Constants.sol | 3 +++ 6 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/Engine.sol b/src/Engine.sol index e3dc980a..890fbddb 100644 --- a/src/Engine.sol +++ b/src/Engine.sol @@ -96,7 +96,7 @@ contract Engine is IEngine, Multicallable, EIP712 { returns (bool) { return PERPS_MARKET_PROXY.hasPermission( - _accountId, Constants.ADMIN_PERMISSION, _caller + _accountId, Constants.PERPS_COMMIT_ASYNC_ORDER_PERMISSION, _caller ); } diff --git a/src/interfaces/IEngine.sol b/src/interfaces/IEngine.sol index 9a0fa5e2..16612cd7 100644 --- a/src/interfaces/IEngine.sol +++ b/src/interfaces/IEngine.sol @@ -91,6 +91,8 @@ interface IEngine { /// @notice check if the msg.sender is a delegate of the account /// identified by the accountId + /// @dev a delegate is an address that has been given + /// PERPS_COMMIT_ASYNC_ORDER_PERMISSION permission /// @param _accountId the id of the account to check /// @param _caller the address to check /// @return true if the msg.sender is a delegate of the account diff --git a/test/Authentication.t.sol b/test/Authentication.t.sol index 51f7d64a..5fa1ceca 100644 --- a/test/Authentication.t.sol +++ b/test/Authentication.t.sol @@ -52,7 +52,7 @@ contract AccountDelegate is AuthenticationTest { perpsMarketProxy.grantPermission({ accountId: accountId, - permission: ADMIN_PERMISSION, + permission: PERPS_COMMIT_ASYNC_ORDER_PERMISSION, user: NEW_ACTOR }); @@ -70,7 +70,7 @@ contract AccountDelegate is AuthenticationTest { perpsMarketProxy.grantPermission({ accountId: accountId, - permission: ADMIN_PERMISSION, + permission: PERPS_COMMIT_ASYNC_ORDER_PERMISSION, user: NEW_ACTOR }); @@ -96,9 +96,11 @@ contract AccountDelegate is AuthenticationTest { ) ); + // only admin and owner can grant permission + perpsMarketProxy.grantPermission({ accountId: accountId, - permission: ADMIN_PERMISSION, + permission: PERPS_COMMIT_ASYNC_ORDER_PERMISSION, user: NEW_ACTOR }); } diff --git a/test/ConditionalOrder.t.sol b/test/ConditionalOrder.t.sol index e34417d5..abf8fc2c 100644 --- a/test/ConditionalOrder.t.sol +++ b/test/ConditionalOrder.t.sol @@ -384,9 +384,17 @@ contract VerifyConditions is ConditionalOrderTest { function test_verifyConditions_public_non_condition_isAccountDelegate() public { + vm.prank(signer); + + perpsMarketProxy.grantPermission({ + accountId: accountId, + permission: PERPS_COMMIT_ASYNC_ORDER_PERMISSION, + user: DELEGATE_1 + }); + bytes[] memory conditions = new bytes[](1); conditions[0] = abi.encodeWithSelector( - IEngine.isAccountDelegate.selector, accountId, address(engine) + IEngine.isAccountDelegate.selector, accountId, DELEGATE_1 ); IEngine.OrderDetails memory orderDetails; diff --git a/test/NonceBitmap.t.sol b/test/NonceBitmap.t.sol index 17e527d6..4db955cd 100644 --- a/test/NonceBitmap.t.sol +++ b/test/NonceBitmap.t.sol @@ -127,7 +127,7 @@ contract NonceBitmap is Bootstrap, ConditionalOrderSignature { perpsMarketProxy.grantPermission({ accountId: accountId, - permission: ADMIN_PERMISSION, + permission: PERPS_COMMIT_ASYNC_ORDER_PERMISSION, user: NEW_ACTOR }); diff --git a/test/utils/Constants.sol b/test/utils/Constants.sol index 0b4d7092..9ad4153e 100644 --- a/test/utils/Constants.sol +++ b/test/utils/Constants.sol @@ -13,6 +13,9 @@ contract Constants { bytes32 internal constant ADMIN_PERMISSION = "ADMIN"; + bytes32 internal constant PERPS_COMMIT_ASYNC_ORDER_PERMISSION = + "PERPS_COMMIT_ASYNC_ORDER"; + address internal constant MOCK_MARGIN_ENGINE = address(0xE1); address internal constant ACTOR = address(0xa1); From 924083b4d5f7992586e44e597e805e8b7475aeed Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Fri, 15 Sep 2023 13:31:46 -0400 Subject: [PATCH 03/13] =?UTF-8?q?=E2=9C=A8=20Prettify?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/Authentication.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Authentication.t.sol b/test/Authentication.t.sol index 5fa1ceca..52d7446c 100644 --- a/test/Authentication.t.sol +++ b/test/Authentication.t.sol @@ -97,7 +97,7 @@ contract AccountDelegate is AuthenticationTest { ); // only admin and owner can grant permission - + perpsMarketProxy.grantPermission({ accountId: accountId, permission: PERPS_COMMIT_ASYNC_ORDER_PERMISSION, From 3b3c3c3fa7505904417dc3319d5af6442f64bf8c Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Fri, 15 Sep 2023 14:12:12 -0400 Subject: [PATCH 04/13] =?UTF-8?q?=F0=9F=93=9A=20M-2:=20Document=20Griefing?= =?UTF-8?q?=20Attack?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Engine.sol | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Engine.sol b/src/Engine.sol index 890fbddb..5be9f258 100644 --- a/src/Engine.sol +++ b/src/Engine.sol @@ -451,6 +451,12 @@ contract Engine is IEngine, Multicallable, EIP712 { if (length > Constants.MAX_CONDITIONS) { revert MaxConditionSizeExceeded(); } + + /// @dev given that conditions are not "sanitized" prior to being called, + /// there exists a griefing vector where an infinite loop can be created + /// (i.e. a condition calls Engine.verifyConditions) + /// @custom:executor be aware and ensure that conditions are not malicious + /// to avoid wasting gas; recommend simulating call prior to executing for (uint256 i = 0; i < length;) { bool success; bytes memory response; From 8191442406a4cd3d59b36b952c31d801513a64aa Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Fri, 15 Sep 2023 14:17:25 -0400 Subject: [PATCH 05/13] =?UTF-8?q?=F0=9F=93=9A=20Q-1:=20Documentation=20Upd?= =?UTF-8?q?ate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/libraries/MathLib.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/MathLib.sol b/src/libraries/MathLib.sol index c7fc9ad6..99dfd715 100644 --- a/src/libraries/MathLib.sol +++ b/src/libraries/MathLib.sol @@ -14,7 +14,7 @@ library MathLib { /// shr(127, x): /// shifts the number x to the right by 127 bits: /// IF the number is negative, the leftmost bit (bit 127) will be 1 - /// IF the number is positive,the leftmost bit (bit 127) will be 0 + /// IF the number is positive, the leftmost bit (bit 127) will be 0 let y := shr(127, x) /// sub(xor(x, y), y): From 2718eab6dfbb20f14bac6ff8a0eeeb4df0c11cfc Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Fri, 15 Sep 2023 14:18:53 -0400 Subject: [PATCH 06/13] =?UTF-8?q?=F0=9F=97=91=EF=B8=8F=20Q-2:=20Remove=20l?= =?UTF-8?q?ibrary=20for=20OrderDetails?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Engine.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Engine.sol b/src/Engine.sol index 5be9f258..fdc8ef18 100644 --- a/src/Engine.sol +++ b/src/Engine.sol @@ -21,7 +21,6 @@ contract Engine is IEngine, Multicallable, EIP712 { using MathLib for int256; using MathLib for uint256; using SignatureCheckerLib for bytes; - using ConditionalOrderHashLib for OrderDetails; using ConditionalOrderHashLib for ConditionalOrder; /*////////////////////////////////////////////////////////////// From ec288ad042a9105f3a954c60aac72a1f58224743 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Fri, 15 Sep 2023 14:29:28 -0400 Subject: [PATCH 07/13] =?UTF-8?q?=F0=9F=93=9A=20I-1:=20Add=20conditional?= =?UTF-8?q?=20order=20documentation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/interfaces/IEngine.sol | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/interfaces/IEngine.sol b/src/interfaces/IEngine.sol index 16612cd7..c05de146 100644 --- a/src/interfaces/IEngine.sol +++ b/src/interfaces/IEngine.sol @@ -168,6 +168,17 @@ interface IEngine { CONDITIONAL ORDER MANAGEMENT //////////////////////////////////////////////////////////////*/ + /// In order for a conditional order to be committed and then executed there are a number of requirements that need to be met: + /// + /// (1) The account must have sufficient snxUSD collateral to handle the order + /// (2) The account must not have another order committed + /// (3) The order’s set `acceptablePrice` needs to be met both on committing the order and when it gets executed + /// (users should choose a value for this that is likely to execute based on the conditions set) + /// (4) The order can only be executed within Synthetix’s set settlement window + /// (5) There must be a keeper that executes a conditional order + /// + /// @notice There is no guarantee a conditional order will be executed + /// @notice execute a conditional order /// @param _co the conditional order /// @param _signature the signature of the conditional order From 3f10341de906ac9306194d91e96af51fea8ea28b Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Fri, 15 Sep 2023 14:33:24 -0400 Subject: [PATCH 08/13] =?UTF-8?q?=F0=9F=91=B7=F0=9F=8F=BB=E2=80=8D?= =?UTF-8?q?=E2=99=82=EF=B8=8F=20Update=20constructor=20Natspec?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Engine.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Engine.sol b/src/Engine.sol index fdc8ef18..c3c757c7 100644 --- a/src/Engine.sol +++ b/src/Engine.sol @@ -56,6 +56,7 @@ contract Engine is IEngine, Multicallable, EIP712 { /// @param _perpsMarketProxy Synthetix v3 perps market proxy contract /// @param _spotMarketProxy Synthetix v3 spot market proxy contract /// @param _sUSDProxy Synthetix v3 sUSD contract + /// @param _oracle pyth oracle contract used to get asset prices constructor( address _perpsMarketProxy, address _spotMarketProxy, From cacdee0ad9f11648433a79724b2f4bfa6cc9ad2d Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Fri, 15 Sep 2023 15:46:24 -0400 Subject: [PATCH 09/13] =?UTF-8?q?=F0=9F=91=B7=F0=9F=8F=BB=E2=80=8D?= =?UTF-8?q?=E2=99=82=EF=B8=8F=20Add=20condition=20selector=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gas-snapshot | 130 +++++++-------- codecov.yaml | 3 + lcov.info | 322 ++++++++++++++++++++---------------- src/Engine.sol | 42 +++-- src/interfaces/IEngine.sol | 3 + test/ConditionalOrder.t.sol | 64 +------ 6 files changed, 284 insertions(+), 280 deletions(-) create mode 100644 codecov.yaml diff --git a/.gas-snapshot b/.gas-snapshot index c9d2cb05..f601433a 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,72 +1,70 @@ -AccountDelegate:test_isAccountDelegate_account_doesnt_exist() (gas: 26446) -AccountDelegate:test_isAccountDelegate_false() (gas: 391888) -AccountDelegate:test_isAccountDelegate_true() (gas: 389857) -AccountOwner:test_isAccountOwner_account_doesnt_exist() (gas: 25010) -AccountOwner:test_isAccountOwner_false() (gas: 229778) -AccountOwner:test_isAccountOwner_true() (gas: 229768) -CanExecute:test_canExecute_false_nonce_used() (gas: 623045) -CanExecute:test_canExecute_false_trusted_executor() (gas: 43930) -CanExecute:test_canExecute_true() (gas: 43529) -CommitOrder:test_commitOrder() (gas: 378313) -CommitOrder:test_commitOrder_insufficient_collateral() (gas: 433243) -CommitOrder:test_commitOrder_invalid_market() (gas: 37323) -Conditions:test_isMarketOpen() (gas: 26784) -Conditions:test_isOrderFeeBelow() (gas: 168974) -Conditions:test_isPositionSizeAbove() (gas: 18735) -Conditions:test_isPositionSizeBelow() (gas: 18717) -Conditions:test_isPriceAbove() (gas: 19106) -Conditions:test_isPriceBelow() (gas: 19178) -Conditions:test_isTimestampAfter() (gas: 7579) -Conditions:test_isTimestampBefore() (gas: 7602) -DeploymentTest:test_deploy() (gas: 2506808) -DeploymentTest:test_deploy_oracle_zero_address() (gas: 39663) -DeploymentTest:test_deploy_perps_market_proxy_zero_address() (gas: 39559) -DeploymentTest:test_deploy_spot_market_proxy_zero_address() (gas: 39637) -DeploymentTest:test_deploy_susd_proxy_zero_address() (gas: 39650) -DepositCollateral:test_depositCollateral() (gas: 258394) -DepositCollateral:test_depositCollateral_availableMargin() (gas: 265951) -DepositCollateral:test_depositCollateral_collateralAmount() (gas: 258970) -DepositCollateral:test_depositCollateral_insufficient_balance() (gas: 55935) -DepositCollateral:test_depositCollateral_totalCollateralValue() (gas: 263324) -Execute:test_execute_CannotExecuteOrder() (gas: 38987) -Execute:test_execute_leverage_exceeded() (gas: 693504) -Execute:test_execute_order_committed() (gas: 621047) -Fee:test_fee_imposed() (gas: 627144) -Fee:test_fee_imposed_above_upper_fee_cap() (gas: 603844) -Fee:test_fee_imposed_at_upper_fee_cap() (gas: 603747) -Fee:test_fee_imposed_below_lower_fee_cap() (gas: 602161) -Fee:test_fee_imposed_below_upper_fee_cap() (gas: 604725) -Fee:test_fee_imposed_fee_cannot_be_paid() (gas: 377994) -Fee:test_fee_imposed_insufficient_collateral_for_order() (gas: 693505) +AccountDelegate:test_isAccountDelegate_account_doesnt_exist() (gas: 26494) +AccountDelegate:test_isAccountDelegate_false() (gas: 391958) +AccountDelegate:test_isAccountDelegate_true() (gas: 389927) +AccountOwner:test_isAccountOwner_account_doesnt_exist() (gas: 25032) +AccountOwner:test_isAccountOwner_false() (gas: 229800) +AccountOwner:test_isAccountOwner_true() (gas: 229790) +CanExecute:test_canExecute_false_nonce_used() (gas: 623113) +CanExecute:test_canExecute_false_trusted_executor() (gas: 43976) +CanExecute:test_canExecute_true() (gas: 43575) +CommitOrder:test_commitOrder() (gas: 378335) +CommitOrder:test_commitOrder_insufficient_collateral() (gas: 433265) +CommitOrder:test_commitOrder_invalid_market() (gas: 37345) +Conditions:test_isMarketOpen() (gas: 26828) +Conditions:test_isOrderFeeBelow() (gas: 168839) +Conditions:test_isPositionSizeAbove() (gas: 18801) +Conditions:test_isPositionSizeBelow() (gas: 18783) +Conditions:test_isPriceAbove() (gas: 19194) +Conditions:test_isPriceBelow() (gas: 18998) +Conditions:test_isTimestampAfter() (gas: 7645) +Conditions:test_isTimestampBefore() (gas: 7668) +DeploymentTest:test_deploy() (gas: 2743471) +DeploymentTest:test_deploy_oracle_zero_address() (gas: 39821) +DeploymentTest:test_deploy_perps_market_proxy_zero_address() (gas: 39717) +DeploymentTest:test_deploy_spot_market_proxy_zero_address() (gas: 39795) +DeploymentTest:test_deploy_susd_proxy_zero_address() (gas: 39808) +DepositCollateral:test_depositCollateral() (gas: 258349) +DepositCollateral:test_depositCollateral_availableMargin() (gas: 265906) +DepositCollateral:test_depositCollateral_collateralAmount() (gas: 258925) +DepositCollateral:test_depositCollateral_insufficient_balance() (gas: 55890) +DepositCollateral:test_depositCollateral_totalCollateralValue() (gas: 263279) +Execute:test_execute_CannotExecuteOrder() (gas: 39031) +Execute:test_execute_leverage_exceeded() (gas: 698025) +Execute:test_execute_order_committed() (gas: 621093) +Fee:test_fee_imposed() (gas: 627190) +Fee:test_fee_imposed_above_upper_fee_cap() (gas: 608356) +Fee:test_fee_imposed_at_upper_fee_cap() (gas: 608259) +Fee:test_fee_imposed_below_lower_fee_cap() (gas: 606657) +Fee:test_fee_imposed_below_upper_fee_cap() (gas: 609212) +Fee:test_fee_imposed_fee_cannot_be_paid() (gas: 382470) +Fee:test_fee_imposed_insufficient_collateral_for_order() (gas: 698026) MathLibTest:test_abs128() (gas: 448) MathLibTest:test_abs256() (gas: 480) MathLibTest:test_castU128() (gas: 350) MathLibTest:test_castU128_overflow() (gas: 3509) -MathLibTest:test_fuzz_abs128(int128) (runs: 256, μ: 576, ~: 603) +MathLibTest:test_fuzz_abs128(int128) (runs: 256, μ: 577, ~: 603) MathLibTest:test_fuzz_abs256(int256) (runs: 256, μ: 472, ~: 458) MathLibTest:test_isSameSign() (gas: 999) -MulticallableTest:test_multicall_depositCollateral_commitOrder() (gas: 600378) -NonceBitmap:test_hasUnorderedNonceBeenUsed() (gas: 53961) -NonceBitmap:test_invalidateUnorderedNonces() (gas: 72637) -NonceBitmap:test_invalidateUnorderedNonces_Only_Owner_Delegate() (gas: 190324) -ReduceOnly:test_reduce_only() (gas: 622681) -ReduceOnly:test_reduce_only_same_sign() (gas: 68300) -ReduceOnly:test_reduce_only_truncate_size_down() (gas: 622811) -ReduceOnly:test_reduce_only_truncate_size_up() (gas: 603624) -ReduceOnly:test_reduce_only_zero_size() (gas: 158516) -VerifyConditions:test_max_condition_size_exceeded() (gas: 45024) -VerifyConditions:test_verifyConditions_internal_non_condition_getSynthAddress() (gas: 10854) -VerifyConditions:test_verifyConditions_modify_state_createAccount() (gas: 10684) -VerifyConditions:test_verifyConditions_public_non_condition_isAccountDelegate() (gas: 28334) -VerifyConditions:test_verify_conditions_not_verified() (gas: 26803) -VerifyConditions:test_verify_conditions_verified() (gas: 129680) -VerifySignature:test_verifySignature() (gas: 23724) -VerifySignature:test_verifySignature_false_private_key() (gas: 26546) -VerifySigner:test_verifySigner() (gas: 25787) -VerifySigner:test_verifySigner_false() (gas: 28896) -WithdrawCollateral:test_withdrawCollateral() (gas: 348350) -WithdrawCollateral:test_withdrawCollateral_availableMargin() (gas: 349860) -WithdrawCollateral:test_withdrawCollateral_collateralAmount() (gas: 348860) -WithdrawCollateral:test_withdrawCollateral_insufficient_account_collateral_balance() (gas: 279608) -WithdrawCollateral:test_withdrawCollateral_totalCollateralValue() (gas: 349349) -WithdrawCollateral:test_withdrawCollateral_zero() (gas: 265953) \ No newline at end of file +MulticallableTest:test_multicall_depositCollateral_commitOrder() (gas: 600377) +NonceBitmap:test_hasUnorderedNonceBeenUsed() (gas: 53827) +NonceBitmap:test_invalidateUnorderedNonces() (gas: 72661) +NonceBitmap:test_invalidateUnorderedNonces_Only_Owner_Delegate() (gas: 190194) +ReduceOnly:test_reduce_only() (gas: 622727) +ReduceOnly:test_reduce_only_same_sign() (gas: 68346) +ReduceOnly:test_reduce_only_truncate_size_down() (gas: 622857) +ReduceOnly:test_reduce_only_truncate_size_up() (gas: 603670) +ReduceOnly:test_reduce_only_zero_size() (gas: 158562) +VerifyConditions:test_max_condition_size_exceeded() (gas: 45046) +VerifyConditions:test_verifyConditions_InvalidConditionSelector() (gas: 16013) +VerifyConditions:test_verify_conditions_not_verified() (gas: 37769) +VerifyConditions:test_verify_conditions_verified() (gas: 151632) +VerifySignature:test_verifySignature() (gas: 23770) +VerifySignature:test_verifySignature_false_private_key() (gas: 26592) +VerifySigner:test_verifySigner() (gas: 25722) +VerifySigner:test_verifySigner_false() (gas: 28831) +WithdrawCollateral:test_withdrawCollateral() (gas: 348278) +WithdrawCollateral:test_withdrawCollateral_availableMargin() (gas: 349788) +WithdrawCollateral:test_withdrawCollateral_collateralAmount() (gas: 348788) +WithdrawCollateral:test_withdrawCollateral_insufficient_account_collateral_balance() (gas: 279518) +WithdrawCollateral:test_withdrawCollateral_totalCollateralValue() (gas: 349277) +WithdrawCollateral:test_withdrawCollateral_zero() (gas: 265863) \ No newline at end of file diff --git a/codecov.yaml b/codecov.yaml new file mode 100644 index 00000000..beb17eb1 --- /dev/null +++ b/codecov.yaml @@ -0,0 +1,3 @@ +ignore: + - "test" + - "script" \ No newline at end of file diff --git a/lcov.info b/lcov.info index b764b9b3..7030a181 100644 --- a/lcov.info +++ b/lcov.info @@ -24,152 +24,168 @@ BRH:0 end_of_record TN: SF:src/Engine.sol -FN:111,Engine.isAccountOwner +FN:84,Engine._whitelistConditionSelectors +FNDA:0,Engine._whitelistConditionSelectors +DA:85,0 +DA:86,0 +DA:87,0 +DA:88,0 +DA:89,0 +DA:90,0 +DA:91,0 +DA:92,0 +FN:100,Engine.isAccountOwner FNDA:3,Engine.isAccountOwner -DA:117,39 -FN:121,Engine.isAccountDelegate -FNDA:3,Engine.isAccountDelegate -DA:127,6 -FN:132,Engine._isAccountOwnerOrDelegate +DA:106,39 +FN:110,Engine.isAccountDelegate +FNDA:2,Engine.isAccountDelegate +DA:116,5 +FN:121,Engine._isAccountOwnerOrDelegate FNDA:29,Engine._isAccountOwnerOrDelegate -DA:137,29 -DA:138,3 -FN:146,Engine.invalidateUnorderedNonces +DA:126,29 +DA:127,3 +FN:135,Engine.invalidateUnorderedNonces FNDA:4,Engine.invalidateUnorderedNonces -DA:151,4 -BRDA:151,0,0,4 -BRDA:151,0,1,- -DA:152,4 -DA:154,4 -DA:156,0 -FN:161,Engine.hasUnorderedNonceBeenUsed +DA:140,4 +BRDA:140,0,0,4 +BRDA:140,0,1,- +DA:141,4 +DA:143,4 +DA:145,0 +FN:150,Engine.hasUnorderedNonceBeenUsed FNDA:4,Engine.hasUnorderedNonceBeenUsed -DA:166,25 -DA:167,25 -DA:168,25 -FN:178,Engine._bitmapPositions +DA:155,25 +DA:156,25 +DA:157,25 +FN:167,Engine._bitmapPositions FNDA:40,Engine._bitmapPositions -DA:183,40 -DA:184,40 -FN:190,Engine._useUnorderedNonce +DA:172,40 +DA:173,40 +FN:179,Engine._useUnorderedNonce FNDA:15,Engine._useUnorderedNonce -DA:191,15 -DA:192,15 -DA:193,15 -DA:195,15 -BRDA:195,1,0,- -BRDA:195,1,1,15 -FN:203,Engine.modifyCollateral +DA:180,15 +DA:181,15 +DA:182,15 +DA:184,15 +BRDA:184,1,0,- +BRDA:184,1,1,15 +FN:192,Engine.modifyCollateral FNDA:19,Engine.modifyCollateral -DA:208,19 -DA:210,19 -BRDA:210,2,0,12 -BRDA:210,2,1,7 -DA:211,12 -DA:215,7 -BRDA:215,3,0,- -BRDA:215,3,1,7 -DA:216,7 -FN:222,Engine._depositCollateral +DA:197,19 +DA:199,19 +BRDA:199,2,0,12 +BRDA:199,2,1,7 +DA:200,12 +DA:204,7 +BRDA:204,3,0,- +BRDA:204,3,1,7 +DA:205,7 +FN:211,Engine._depositCollateral FNDA:12,Engine._depositCollateral -DA:230,12 -DA:232,11 -DA:234,11 -FN:237,Engine._withdrawCollateral +DA:219,12 +DA:221,11 +DA:223,11 +FN:226,Engine._withdrawCollateral FNDA:20,Engine._withdrawCollateral -DA:244,20 -DA:247,17 -FN:253,Engine._getSynthAddress +DA:233,20 +DA:236,17 +FN:242,Engine._getSynthAddress FNDA:19,Engine._getSynthAddress -DA:258,19 -FN:268,Engine.commitOrder +DA:247,19 +FN:257,Engine.commitOrder FNDA:4,Engine.commitOrder -DA:282,4 -BRDA:282,4,0,4 -BRDA:282,4,1,- -DA:283,4 -DA:293,0 -FN:297,Engine._commitOrder +DA:271,4 +BRDA:271,4,0,4 +BRDA:271,4,1,- +DA:272,4 +DA:282,0 +FN:286,Engine._commitOrder FNDA:16,Engine._commitOrder -DA:306,16 -FN:324,Engine.execute +DA:295,16 +FN:313,Engine.execute FNDA:16,Engine.execute -DA:337,16 -BRDA:337,5,0,1 -BRDA:337,5,1,15 -DA:340,15 -DA:344,15 -DA:347,15 -BRDA:347,6,0,2 -BRDA:347,6,1,3 -DA:348,5 -DA:353,5 -BRDA:353,7,0,1 -BRDA:353,7,1,4 -DA:354,1 -DA:358,4 -BRDA:358,8,0,1 -BRDA:358,8,1,3 -DA:359,1 -DA:365,3 -BRDA:365,9,0,2 -BRDA:365,9,1,3 -DA:375,2 -DA:381,13 +DA:326,16 +BRDA:326,5,0,1 +BRDA:326,5,1,15 +DA:329,15 +DA:333,15 +DA:336,15 +BRDA:336,6,0,2 +BRDA:336,6,1,3 +DA:337,5 +DA:342,5 +BRDA:342,7,0,1 +BRDA:342,7,1,4 +DA:343,1 +DA:347,4 +BRDA:347,8,0,1 +BRDA:347,8,1,3 +DA:348,1 +DA:354,3 +BRDA:354,9,0,2 +BRDA:354,9,1,3 +DA:364,2 +DA:370,13 +DA:376,13 +DA:380,13 +BRDA:380,10,0,6 +BRDA:380,10,1,7 +DA:381,6 +DA:382,7 +BRDA:382,11,0,2 +BRDA:382,11,1,7 +DA:383,2 DA:387,13 -DA:390,13 -BRDA:390,10,0,6 -BRDA:390,10,1,7 -DA:391,6 -DA:392,7 -BRDA:392,11,0,2 -BRDA:392,11,1,7 -DA:393,2 -DA:397,13 -DA:406,12 -FN:418,Engine.canExecute +DA:396,12 +FN:408,Engine.canExecute FNDA:5,Engine.canExecute -DA:423,21 -BRDA:423,12,0,2 -BRDA:423,12,1,19 -DA:424,2 -DA:428,19 -BRDA:428,13,0,1 -BRDA:428,13,1,18 +DA:413,21 +BRDA:413,12,0,2 +BRDA:413,12,1,19 +DA:414,2 +DA:418,19 +BRDA:418,13,0,1 +BRDA:418,13,1,18 +DA:421,18 +BRDA:421,14,0,- +BRDA:421,14,1,18 +DA:424,18 +BRDA:424,15,0,- +BRDA:424,15,1,- +DA:427,0 +BRDA:427,16,0,- +BRDA:427,16,1,- DA:431,18 -BRDA:431,14,0,- -BRDA:431,14,1,18 -DA:434,18 -BRDA:434,15,0,- -BRDA:434,15,1,- -DA:437,0 -BRDA:437,16,0,- -BRDA:437,16,1,- -DA:441,18 -BRDA:441,17,0,1 -BRDA:441,17,1,17 -DA:444,17 -FN:452,Engine.verifySigner +BRDA:431,17,0,1 +BRDA:431,17,1,17 +DA:434,17 +FN:442,Engine.verifySigner FNDA:2,Engine.verifySigner -DA:458,21 -FN:462,Engine.verifySignature +DA:448,21 +FN:452,Engine.verifySignature FNDA:2,Engine.verifySignature -DA:466,20 -FN:472,Engine.verifyConditions -FNDA:6,Engine.verifyConditions -DA:478,6 -DA:479,6 -BRDA:479,18,0,1 -BRDA:479,18,1,5 -DA:480,5 -DA:481,15 -DA:482,15 -DA:485,15 -DA:487,15 -BRDA:487,19,0,3 -BRDA:487,19,1,12 -DA:490,12 -DA:494,2 +DA:456,20 +FN:462,Engine.verifyConditions +FNDA:4,Engine.verifyConditions +DA:468,4 +DA:469,4 +BRDA:469,18,0,1 +BRDA:469,18,1,3 +DA:470,1 +DA:473,3 +DA:474,13 +DA:475,13 +DA:477,13 +DA:479,13 +BRDA:479,19,0,1 +BRDA:479,19,1,11 +DA:481,12 +DA:484,12 +BRDA:484,20,0,1 +BRDA:484,20,1,11 +DA:487,11 +DA:490,1 +DA:494,1 FN:502,Engine.isTimestampAfter FNDA:5,Engine.isTimestampAfter DA:508,5 @@ -201,12 +217,12 @@ FN:584,Engine.isOrderFeeBelow FNDA:4,Engine.isOrderFeeBelow DA:590,4 DA:595,4 -FNF:26 +FNF:27 FNH:26 -LF:85 -LH:82 -BRF:40 -BRH:31 +LF:97 +LH:86 +BRF:42 +BRH:33 end_of_record TN: SF:src/libraries/ConditionalOrderHashLib.sol @@ -392,16 +408,6 @@ BRH:0 end_of_record TN: SF:test/utils/Bootstrap.sol -FN:102,BootstrapOptimism.init -FNDA:0,BootstrapOptimism.init -DA:106,0 -DA:113,0 -DA:120,0 -FN:132,BootstrapOptimismGoerli.init -FNDA:0,BootstrapOptimismGoerli.init -DA:136,0 -DA:143,0 -DA:150,0 FN:36,Bootstrap.initializeOptimismGoerli FNDA:0,Bootstrap.initializeOptimismGoerli DA:37,0 @@ -438,6 +444,16 @@ DA:89,0 DA:90,0 DA:95,0 DA:97,0 +FN:102,BootstrapOptimism.init +FNDA:0,BootstrapOptimism.init +DA:106,0 +DA:113,0 +DA:120,0 +FN:132,BootstrapOptimismGoerli.init +FNDA:0,BootstrapOptimismGoerli.init +DA:136,0 +DA:143,0 +DA:150,0 FNF:4 FNH:0 LF:38 @@ -546,10 +562,22 @@ SF:test/utils/exposed/EngineExposed.sol FN:16,EngineExposed.getSynthAddress FNDA:0,EngineExposed.getSynthAddress DA:21,0 -FNF:1 -FNH:0 -LF:1 -LH:0 +FN:24,EngineExposed.UPPER_FEE_CAP +FNDA:9,EngineExposed.UPPER_FEE_CAP +DA:25,9 +FN:28,EngineExposed.LOWER_FEE_CAP +FNDA:2,EngineExposed.LOWER_FEE_CAP +DA:29,2 +FN:32,EngineExposed.FEE_SCALING_FACTOR +FNDA:7,EngineExposed.FEE_SCALING_FACTOR +DA:33,7 +FN:36,EngineExposed.MAX_BPS +FNDA:0,EngineExposed.MAX_BPS +DA:37,0 +FNF:5 +FNH:3 +LF:5 +LH:3 BRF:0 BRH:0 end_of_record diff --git a/src/Engine.sol b/src/Engine.sol index c3c757c7..51d07af8 100644 --- a/src/Engine.sol +++ b/src/Engine.sol @@ -48,6 +48,10 @@ contract Engine is IEngine, Multicallable, EIP712 { mapping(uint128 accountId => mapping(uint256 index => uint256 bitmap)) public nonceBitmap; + /// @notice mapping of condition selector to whether the selector is valid + /// (i.e. can be called within Engine.verifyConditions) + mapping(bytes4 selector => bool) public conditionSelector; + /*////////////////////////////////////////////////////////////// CONSTRUCTOR //////////////////////////////////////////////////////////////*/ @@ -72,6 +76,20 @@ contract Engine is IEngine, Multicallable, EIP712 { SPOT_MARKET_PROXY = ISpotMarketProxy(_spotMarketProxy); SUSD = IERC20(_sUSDProxy); ORACLE = IPyth(_oracle); + + _whitelistConditionSelectors(); + } + + /// @notice whitelist condition selectors + function _whitelistConditionSelectors() internal { + conditionSelector[IEngine.isTimestampAfter.selector] = true; + conditionSelector[IEngine.isTimestampBefore.selector] = true; + conditionSelector[IEngine.isPriceAbove.selector] = true; + conditionSelector[IEngine.isPriceBelow.selector] = true; + conditionSelector[IEngine.isMarketOpen.selector] = true; + conditionSelector[IEngine.isPositionSizeAbove.selector] = true; + conditionSelector[IEngine.isPositionSizeBelow.selector] = true; + conditionSelector[IEngine.isOrderFeeBelow.selector] = true; } /*////////////////////////////////////////////////////////////// @@ -452,22 +470,26 @@ contract Engine is IEngine, Multicallable, EIP712 { revert MaxConditionSizeExceeded(); } - /// @dev given that conditions are not "sanitized" prior to being called, - /// there exists a griefing vector where an infinite loop can be created - /// (i.e. a condition calls Engine.verifyConditions) - /// @custom:executor be aware and ensure that conditions are not malicious - /// to avoid wasting gas; recommend simulating call prior to executing for (uint256 i = 0; i < length;) { bool success; bytes memory response; - /// @dev staticcall to prevent state changes in the case a condition is malicious - (success, response) = address(this).staticcall(_co.conditions[i]); + bytes4 selector = bytes4(_co.conditions[i]); + + /// @dev checking if the selector is valid prevents the possibility of + /// a malicious condition from griefing the executor + if (conditionSelector[selector]) { + // @dev staticcall to prevent state changes in the case a condition is malicious + (success, response) = + address(this).staticcall(_co.conditions[i]); - if (!success || !abi.decode(response, (bool))) return false; + if (!success || !abi.decode(response, (bool))) return false; - unchecked { - i++; + unchecked { + i++; + } + } else { + revert InvalidConditionSelector(selector); } } diff --git a/src/interfaces/IEngine.sol b/src/interfaces/IEngine.sol index c05de146..bcd4ad4b 100644 --- a/src/interfaces/IEngine.sol +++ b/src/interfaces/IEngine.sol @@ -66,6 +66,9 @@ interface IEngine { /// @notice thrown when attempting to re-use a nonce error InvalidNonce(); + /// @notice thrown when attempting to verify a condition identified by an invalid selector + error InvalidConditionSelector(bytes4 selector); + /*////////////////////////////////////////////////////////////// EVENTS //////////////////////////////////////////////////////////////*/ diff --git a/test/ConditionalOrder.t.sol b/test/ConditionalOrder.t.sol index abf8fc2c..b819c3f4 100644 --- a/test/ConditionalOrder.t.sol +++ b/test/ConditionalOrder.t.sol @@ -381,41 +381,7 @@ contract VerifyConditions is ConditionalOrderTest { assertFalse(isVerified); } - function test_verifyConditions_public_non_condition_isAccountDelegate() - public - { - vm.prank(signer); - - perpsMarketProxy.grantPermission({ - accountId: accountId, - permission: PERPS_COMMIT_ASYNC_ORDER_PERMISSION, - user: DELEGATE_1 - }); - - bytes[] memory conditions = new bytes[](1); - conditions[0] = abi.encodeWithSelector( - IEngine.isAccountDelegate.selector, accountId, DELEGATE_1 - ); - - IEngine.OrderDetails memory orderDetails; - - IEngine.ConditionalOrder memory co = IEngine.ConditionalOrder({ - orderDetails: orderDetails, - signer: signer, - nonce: 0, - requireVerified: true, - trustedExecutor: address(0), - conditions: conditions - }); - - bool isVerified = engine.verifyConditions(co); - - assertTrue(isVerified); - } - - function test_verifyConditions_internal_non_condition_getSynthAddress() - public - { + function test_verifyConditions_InvalidConditionSelector() public { bytes[] memory conditions = new bytes[](1); conditions[0] = abi.encodeWithSignature( "_getSynthAddress(uint128 _synthMarketId)", SETH_SPOT_MARKET_ID @@ -432,29 +398,13 @@ contract VerifyConditions is ConditionalOrderTest { conditions: conditions }); - bool isVerified = engine.verifyConditions(co); - - assertFalse(isVerified); - } - - function test_verifyConditions_modify_state_createAccount() public { - bytes[] memory conditions = new bytes[](1); - conditions[0] = abi.encodeWithSignature("createAccount()"); - - IEngine.OrderDetails memory orderDetails; - - IEngine.ConditionalOrder memory co = IEngine.ConditionalOrder({ - orderDetails: orderDetails, - signer: signer, - nonce: 0, - requireVerified: true, - trustedExecutor: address(0), - conditions: conditions - }); - - bool isVerified = engine.verifyConditions(co); + vm.expectRevert( + abi.encodeWithSelector( + IEngine.InvalidConditionSelector.selector, bytes4(conditions[0]) + ) + ); - assertFalse(isVerified); + engine.verifyConditions(co); } } From 927bdd715e97a31594a7411286879422de4625ce Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Fri, 15 Sep 2023 16:17:07 -0400 Subject: [PATCH 10/13] =?UTF-8?q?=F0=9F=91=B7=F0=9F=8F=BB=E2=80=8D?= =?UTF-8?q?=E2=99=82=EF=B8=8F=20Refactor=20Engine=20&=20Improve=20Conditio?= =?UTF-8?q?n=20Selector=20Check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gas-snapshot | 50 +-- lcov.info | 379 +++++++++---------- src/Engine.sol | 106 ++++-- src/libraries/Constants.sol | 41 -- test/ConditionalOrder.t.sol | 53 ++- test/utils/analysis/ConditionalOrderFees.sol | 12 +- test/utils/exposed/EngineExposed.sol | 18 +- 7 files changed, 339 insertions(+), 320 deletions(-) delete mode 100644 src/libraries/Constants.sol diff --git a/.gas-snapshot b/.gas-snapshot index f601433a..f63badbd 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -4,13 +4,13 @@ AccountDelegate:test_isAccountDelegate_true() (gas: 389927) AccountOwner:test_isAccountOwner_account_doesnt_exist() (gas: 25032) AccountOwner:test_isAccountOwner_false() (gas: 229800) AccountOwner:test_isAccountOwner_true() (gas: 229790) -CanExecute:test_canExecute_false_nonce_used() (gas: 623113) +CanExecute:test_canExecute_false_nonce_used() (gas: 623091) CanExecute:test_canExecute_false_trusted_executor() (gas: 43976) CanExecute:test_canExecute_true() (gas: 43575) CommitOrder:test_commitOrder() (gas: 378335) CommitOrder:test_commitOrder_insufficient_collateral() (gas: 433265) CommitOrder:test_commitOrder_invalid_market() (gas: 37345) -Conditions:test_isMarketOpen() (gas: 26828) +Conditions:test_isMarketOpen() (gas: 26784) Conditions:test_isOrderFeeBelow() (gas: 168839) Conditions:test_isPositionSizeAbove() (gas: 18801) Conditions:test_isPositionSizeBelow() (gas: 18783) @@ -18,26 +18,26 @@ Conditions:test_isPriceAbove() (gas: 19194) Conditions:test_isPriceBelow() (gas: 18998) Conditions:test_isTimestampAfter() (gas: 7645) Conditions:test_isTimestampBefore() (gas: 7668) -DeploymentTest:test_deploy() (gas: 2743471) -DeploymentTest:test_deploy_oracle_zero_address() (gas: 39821) -DeploymentTest:test_deploy_perps_market_proxy_zero_address() (gas: 39717) -DeploymentTest:test_deploy_spot_market_proxy_zero_address() (gas: 39795) -DeploymentTest:test_deploy_susd_proxy_zero_address() (gas: 39808) +DeploymentTest:test_deploy() (gas: 2653558) +DeploymentTest:test_deploy_oracle_zero_address() (gas: 39840) +DeploymentTest:test_deploy_perps_market_proxy_zero_address() (gas: 39736) +DeploymentTest:test_deploy_spot_market_proxy_zero_address() (gas: 39814) +DeploymentTest:test_deploy_susd_proxy_zero_address() (gas: 39827) DepositCollateral:test_depositCollateral() (gas: 258349) DepositCollateral:test_depositCollateral_availableMargin() (gas: 265906) DepositCollateral:test_depositCollateral_collateralAmount() (gas: 258925) DepositCollateral:test_depositCollateral_insufficient_balance() (gas: 55890) DepositCollateral:test_depositCollateral_totalCollateralValue() (gas: 263279) -Execute:test_execute_CannotExecuteOrder() (gas: 39031) -Execute:test_execute_leverage_exceeded() (gas: 698025) -Execute:test_execute_order_committed() (gas: 621093) -Fee:test_fee_imposed() (gas: 627190) -Fee:test_fee_imposed_above_upper_fee_cap() (gas: 608356) -Fee:test_fee_imposed_at_upper_fee_cap() (gas: 608259) -Fee:test_fee_imposed_below_lower_fee_cap() (gas: 606657) -Fee:test_fee_imposed_below_upper_fee_cap() (gas: 609212) -Fee:test_fee_imposed_fee_cannot_be_paid() (gas: 382470) -Fee:test_fee_imposed_insufficient_collateral_for_order() (gas: 698026) +Execute:test_execute_CannotExecuteOrder() (gas: 39009) +Execute:test_execute_leverage_exceeded() (gas: 697970) +Execute:test_execute_order_committed() (gas: 621071) +Fee:test_fee_imposed() (gas: 627168) +Fee:test_fee_imposed_above_upper_fee_cap() (gas: 608109) +Fee:test_fee_imposed_at_upper_fee_cap() (gas: 608012) +Fee:test_fee_imposed_below_lower_fee_cap() (gas: 606659) +Fee:test_fee_imposed_below_upper_fee_cap() (gas: 608932) +Fee:test_fee_imposed_fee_cannot_be_paid() (gas: 382415) +Fee:test_fee_imposed_insufficient_collateral_for_order() (gas: 697971) MathLibTest:test_abs128() (gas: 448) MathLibTest:test_abs256() (gas: 480) MathLibTest:test_castU128() (gas: 350) @@ -49,15 +49,15 @@ MulticallableTest:test_multicall_depositCollateral_commitOrder() (gas: 600377) NonceBitmap:test_hasUnorderedNonceBeenUsed() (gas: 53827) NonceBitmap:test_invalidateUnorderedNonces() (gas: 72661) NonceBitmap:test_invalidateUnorderedNonces_Only_Owner_Delegate() (gas: 190194) -ReduceOnly:test_reduce_only() (gas: 622727) -ReduceOnly:test_reduce_only_same_sign() (gas: 68346) -ReduceOnly:test_reduce_only_truncate_size_down() (gas: 622857) -ReduceOnly:test_reduce_only_truncate_size_up() (gas: 603670) -ReduceOnly:test_reduce_only_zero_size() (gas: 158562) +ReduceOnly:test_reduce_only() (gas: 622705) +ReduceOnly:test_reduce_only_same_sign() (gas: 68324) +ReduceOnly:test_reduce_only_truncate_size_down() (gas: 622835) +ReduceOnly:test_reduce_only_truncate_size_up() (gas: 603648) +ReduceOnly:test_reduce_only_zero_size() (gas: 158540) VerifyConditions:test_max_condition_size_exceeded() (gas: 45046) -VerifyConditions:test_verifyConditions_InvalidConditionSelector() (gas: 16013) -VerifyConditions:test_verify_conditions_not_verified() (gas: 37769) -VerifyConditions:test_verify_conditions_verified() (gas: 151632) +VerifyConditions:test_verifyConditions_InvalidConditionSelector() (gas: 14079) +VerifyConditions:test_verify_conditions_not_verified() (gas: 29659) +VerifyConditions:test_verify_conditions_verified() (gas: 135662) VerifySignature:test_verifySignature() (gas: 23770) VerifySignature:test_verifySignature_false_private_key() (gas: 26592) VerifySigner:test_verifySigner() (gas: 25722) diff --git a/lcov.info b/lcov.info index 7030a181..25d4be15 100644 --- a/lcov.info +++ b/lcov.info @@ -1,11 +1,5 @@ TN: SF:script/Deploy.s.sol -FN:52,DeployOptimismGoerli.run -FNDA:0,DeployOptimismGoerli.run -DA:53,0 -DA:54,0 -DA:56,0 -DA:63,0 FN:14,Setup.deploySystem FNDA:0,Setup.deploySystem DA:20,0 @@ -15,6 +9,12 @@ DA:34,0 DA:35,0 DA:37,0 DA:44,0 +FN:52,DeployOptimismGoerli.run +FNDA:0,DeployOptimismGoerli.run +DA:53,0 +DA:54,0 +DA:56,0 +DA:63,0 FNF:3 FNH:0 LF:9 @@ -24,203 +24,200 @@ BRH:0 end_of_record TN: SF:src/Engine.sol -FN:84,Engine._whitelistConditionSelectors -FNDA:0,Engine._whitelistConditionSelectors -DA:85,0 -DA:86,0 -DA:87,0 -DA:88,0 -DA:89,0 -DA:90,0 -DA:91,0 -DA:92,0 -FN:100,Engine.isAccountOwner +FN:134,Engine.isAccountOwner FNDA:3,Engine.isAccountOwner -DA:106,39 -FN:110,Engine.isAccountDelegate +DA:140,39 +FN:144,Engine.isAccountDelegate FNDA:2,Engine.isAccountDelegate -DA:116,5 -FN:121,Engine._isAccountOwnerOrDelegate +DA:150,5 +FN:155,Engine._isAccountOwnerOrDelegate FNDA:29,Engine._isAccountOwnerOrDelegate -DA:126,29 -DA:127,3 -FN:135,Engine.invalidateUnorderedNonces +DA:160,29 +DA:161,3 +FN:169,Engine.invalidateUnorderedNonces FNDA:4,Engine.invalidateUnorderedNonces -DA:140,4 -BRDA:140,0,0,4 -BRDA:140,0,1,- -DA:141,4 -DA:143,4 -DA:145,0 -FN:150,Engine.hasUnorderedNonceBeenUsed +DA:174,4 +BRDA:174,0,0,4 +BRDA:174,0,1,- +DA:175,4 +DA:177,4 +DA:179,0 +FN:184,Engine.hasUnorderedNonceBeenUsed FNDA:4,Engine.hasUnorderedNonceBeenUsed -DA:155,25 -DA:156,25 -DA:157,25 -FN:167,Engine._bitmapPositions +DA:190,25 +DA:191,25 +DA:192,25 +FN:202,Engine._bitmapPositions FNDA:40,Engine._bitmapPositions -DA:172,40 -DA:173,40 -FN:179,Engine._useUnorderedNonce +DA:207,40 +DA:208,40 +FN:214,Engine._useUnorderedNonce FNDA:15,Engine._useUnorderedNonce -DA:180,15 -DA:181,15 -DA:182,15 -DA:184,15 -BRDA:184,1,0,- -BRDA:184,1,1,15 -FN:192,Engine.modifyCollateral +DA:215,15 +DA:216,15 +DA:217,15 +DA:219,15 +BRDA:219,1,0,- +BRDA:219,1,1,15 +FN:227,Engine.modifyCollateral FNDA:19,Engine.modifyCollateral -DA:197,19 -DA:199,19 -BRDA:199,2,0,12 -BRDA:199,2,1,7 -DA:200,12 -DA:204,7 -BRDA:204,3,0,- -BRDA:204,3,1,7 -DA:205,7 -FN:211,Engine._depositCollateral +DA:232,19 +DA:234,19 +BRDA:234,2,0,12 +BRDA:234,2,1,7 +DA:235,12 +DA:239,7 +BRDA:239,3,0,- +BRDA:239,3,1,7 +DA:240,7 +FN:246,Engine._depositCollateral FNDA:12,Engine._depositCollateral -DA:219,12 -DA:221,11 -DA:223,11 -FN:226,Engine._withdrawCollateral +DA:254,12 +DA:256,11 +DA:258,11 +FN:261,Engine._withdrawCollateral FNDA:20,Engine._withdrawCollateral -DA:233,20 -DA:236,17 -FN:242,Engine._getSynthAddress +DA:268,20 +DA:271,17 +FN:277,Engine._getSynthAddress FNDA:19,Engine._getSynthAddress -DA:247,19 -FN:257,Engine.commitOrder +DA:282,19 +FN:292,Engine.commitOrder FNDA:4,Engine.commitOrder -DA:271,4 -BRDA:271,4,0,4 -BRDA:271,4,1,- -DA:272,4 -DA:282,0 -FN:286,Engine._commitOrder +DA:306,4 +BRDA:306,4,0,4 +BRDA:306,4,1,- +DA:307,4 +DA:317,0 +FN:321,Engine._commitOrder FNDA:16,Engine._commitOrder -DA:295,16 -FN:313,Engine.execute +DA:330,16 +FN:348,Engine.execute FNDA:16,Engine.execute -DA:326,16 -BRDA:326,5,0,1 -BRDA:326,5,1,15 -DA:329,15 -DA:333,15 -DA:336,15 -BRDA:336,6,0,2 -BRDA:336,6,1,3 -DA:337,5 -DA:342,5 -BRDA:342,7,0,1 -BRDA:342,7,1,4 -DA:343,1 -DA:347,4 -BRDA:347,8,0,1 -BRDA:347,8,1,3 -DA:348,1 -DA:354,3 -BRDA:354,9,0,2 -BRDA:354,9,1,3 -DA:364,2 -DA:370,13 -DA:376,13 -DA:380,13 -BRDA:380,10,0,6 -BRDA:380,10,1,7 -DA:381,6 -DA:382,7 -BRDA:382,11,0,2 -BRDA:382,11,1,7 -DA:383,2 -DA:387,13 -DA:396,12 -FN:408,Engine.canExecute +DA:361,16 +BRDA:361,5,0,1 +BRDA:361,5,1,15 +DA:364,15 +DA:368,15 +DA:371,15 +BRDA:371,6,0,2 +BRDA:371,6,1,3 +DA:372,5 +DA:377,5 +BRDA:377,7,0,1 +BRDA:377,7,1,4 +DA:378,1 +DA:382,4 +BRDA:382,8,0,1 +BRDA:382,8,1,3 +DA:383,1 +DA:389,3 +BRDA:389,9,0,2 +BRDA:389,9,1,3 +DA:399,2 +DA:405,13 +DA:411,13 +DA:414,13 +BRDA:414,10,0,6 +BRDA:414,10,1,7 +DA:415,6 +DA:416,7 +BRDA:416,11,0,2 +BRDA:416,11,1,7 +DA:417,2 +DA:421,13 +DA:430,12 +FN:442,Engine.canExecute FNDA:5,Engine.canExecute -DA:413,21 -BRDA:413,12,0,2 -BRDA:413,12,1,19 -DA:414,2 -DA:418,19 -BRDA:418,13,0,1 -BRDA:418,13,1,18 -DA:421,18 -BRDA:421,14,0,- -BRDA:421,14,1,18 -DA:424,18 -BRDA:424,15,0,- -BRDA:424,15,1,- -DA:427,0 -BRDA:427,16,0,- -BRDA:427,16,1,- -DA:431,18 -BRDA:431,17,0,1 -BRDA:431,17,1,17 -DA:434,17 -FN:442,Engine.verifySigner +DA:447,21 +BRDA:447,12,0,2 +BRDA:447,12,1,19 +DA:448,2 +DA:452,19 +BRDA:452,13,0,1 +BRDA:452,13,1,18 +DA:455,18 +BRDA:455,14,0,- +BRDA:455,14,1,18 +DA:458,18 +BRDA:458,15,0,- +BRDA:458,15,1,- +DA:461,0 +BRDA:461,16,0,- +BRDA:461,16,1,- +DA:465,18 +BRDA:465,17,0,1 +BRDA:465,17,1,17 +DA:468,17 +FN:476,Engine.verifySigner FNDA:2,Engine.verifySigner -DA:448,21 -FN:452,Engine.verifySignature +DA:482,21 +FN:486,Engine.verifySignature FNDA:2,Engine.verifySignature -DA:456,20 -FN:462,Engine.verifyConditions +DA:490,20 +FN:496,Engine.verifyConditions FNDA:4,Engine.verifyConditions -DA:468,4 -DA:469,4 -BRDA:469,18,0,1 -BRDA:469,18,1,3 -DA:470,1 -DA:473,3 -DA:474,13 -DA:475,13 -DA:477,13 -DA:479,13 -BRDA:479,19,0,1 -BRDA:479,19,1,11 -DA:481,12 -DA:484,12 -BRDA:484,20,0,1 -BRDA:484,20,1,11 -DA:487,11 -DA:490,1 -DA:494,1 -FN:502,Engine.isTimestampAfter +DA:502,4 +DA:503,4 +BRDA:503,18,0,1 +BRDA:503,18,1,3 +DA:504,1 +DA:507,3 +DA:508,13 +DA:509,13 +DA:512,13 +DA:517,13 +DA:518,11 +DA:519,9 +DA:520,7 +DA:521,5 +DA:522,4 +DA:523,3 +DA:524,2 +BRDA:516,19,0,1 +BRDA:516,19,1,11 +DA:527,12 +DA:530,12 +BRDA:530,20,0,1 +BRDA:530,20,1,11 +DA:533,11 +DA:536,1 +DA:540,1 +FN:548,Engine.isTimestampAfter FNDA:5,Engine.isTimestampAfter -DA:508,5 -FN:512,Engine.isTimestampBefore +DA:554,5 +FN:558,Engine.isTimestampBefore FNDA:5,Engine.isTimestampBefore -DA:518,5 -FN:522,Engine.isPriceAbove +DA:564,5 +FN:568,Engine.isPriceAbove FNDA:6,Engine.isPriceAbove -DA:527,6 -DA:532,6 -FN:536,Engine.isPriceBelow +DA:573,6 +DA:578,6 +FN:582,Engine.isPriceBelow FNDA:6,Engine.isPriceBelow -DA:541,6 -DA:546,6 -FN:550,Engine.isMarketOpen +DA:587,6 +DA:592,6 +FN:596,Engine.isMarketOpen FNDA:3,Engine.isMarketOpen -DA:556,3 -FN:560,Engine.isPositionSizeAbove +DA:602,3 +FN:606,Engine.isPositionSizeAbove FNDA:4,Engine.isPositionSizeAbove -DA:565,4 -DA:566,4 -DA:568,4 -FN:572,Engine.isPositionSizeBelow +DA:611,4 +DA:612,4 +DA:614,4 +FN:618,Engine.isPositionSizeBelow FNDA:4,Engine.isPositionSizeBelow -DA:577,4 -DA:578,4 -DA:580,4 -FN:584,Engine.isOrderFeeBelow +DA:623,4 +DA:624,4 +DA:626,4 +FN:630,Engine.isOrderFeeBelow FNDA:4,Engine.isOrderFeeBelow -DA:590,4 -DA:595,4 -FNF:27 +DA:636,4 +DA:641,4 +FNF:26 FNH:26 -LF:97 -LH:86 +LF:96 +LH:93 BRF:42 BRH:33 end_of_record @@ -408,6 +405,11 @@ BRH:0 end_of_record TN: SF:test/utils/Bootstrap.sol +FN:102,BootstrapOptimism.init +FNDA:0,BootstrapOptimism.init +DA:106,0 +DA:113,0 +DA:120,0 FN:36,Bootstrap.initializeOptimismGoerli FNDA:0,Bootstrap.initializeOptimismGoerli DA:37,0 @@ -444,11 +446,6 @@ DA:89,0 DA:90,0 DA:95,0 DA:97,0 -FN:102,BootstrapOptimism.init -FNDA:0,BootstrapOptimism.init -DA:106,0 -DA:113,0 -DA:120,0 FN:132,BootstrapOptimismGoerli.init FNDA:0,BootstrapOptimismGoerli.init DA:136,0 @@ -545,11 +542,11 @@ DA:23,0 BRDA:23,0,0,- BRDA:23,0,1,- DA:24,0 -DA:25,0 +DA:26,0 BRDA:25,1,0,- BRDA:25,1,1,- -DA:26,0 DA:28,0 +DA:30,0 FNF:2 FNH:0 LF:12 @@ -562,17 +559,17 @@ SF:test/utils/exposed/EngineExposed.sol FN:16,EngineExposed.getSynthAddress FNDA:0,EngineExposed.getSynthAddress DA:21,0 -FN:24,EngineExposed.UPPER_FEE_CAP -FNDA:9,EngineExposed.UPPER_FEE_CAP +FN:24,EngineExposed.expose_UPPER_FEE_CAP +FNDA:9,EngineExposed.expose_UPPER_FEE_CAP DA:25,9 -FN:28,EngineExposed.LOWER_FEE_CAP -FNDA:2,EngineExposed.LOWER_FEE_CAP +FN:28,EngineExposed.expose_LOWER_FEE_CAP +FNDA:2,EngineExposed.expose_LOWER_FEE_CAP DA:29,2 -FN:32,EngineExposed.FEE_SCALING_FACTOR -FNDA:7,EngineExposed.FEE_SCALING_FACTOR +FN:32,EngineExposed.expose_FEE_SCALING_FACTOR +FNDA:7,EngineExposed.expose_FEE_SCALING_FACTOR DA:33,7 -FN:36,EngineExposed.MAX_BPS -FNDA:0,EngineExposed.MAX_BPS +FN:36,EngineExposed.expose_MAX_BPS +FNDA:0,EngineExposed.expose_MAX_BPS DA:37,0 FNF:5 FNH:3 diff --git a/src/Engine.sol b/src/Engine.sol index 51d07af8..1fc01f31 100644 --- a/src/Engine.sol +++ b/src/Engine.sol @@ -3,7 +3,6 @@ pragma solidity 0.8.18; import {ConditionalOrderHashLib} from "src/libraries/ConditionalOrderHashLib.sol"; -import {Constants} from "src/libraries/Constants.sol"; import {EIP712} from "src/utils/EIP712.sol"; import {IEngine, IPerpsMarketProxy} from "src/interfaces/IEngine.sol"; import {IERC20} from "src/interfaces/tokens/IERC20.sol"; @@ -23,6 +22,59 @@ contract Engine is IEngine, Multicallable, EIP712 { using SignatureCheckerLib for bytes; using ConditionalOrderHashLib for ConditionalOrder; + /*////////////////////////////////////////////////////////////// + CONSTANTS + //////////////////////////////////////////////////////////////*/ + + /// @notice admins have permission to do everything that the account owner can + /// (including granting and revoking permissions for other addresses) except + /// for transferring account ownership + bytes32 internal constant ADMIN_PERMISSION = "ADMIN"; + + /// @notice the permission required to commit an async order + /// @dev this permission does not allow the permission holder to modify collateral + bytes32 internal constant PERPS_COMMIT_ASYNC_ORDER_PERMISSION = + "PERPS_COMMIT_ASYNC_ORDER"; + + /// @notice "0" synthMarketId represents sUSD in Synthetix v3 + uint128 internal constant USD_SYNTH_ID = 0; + + /// @notice max fee that can be charged for a conditional order execution + /// @dev 50 USD + uint256 internal constant UPPER_FEE_CAP = 50 ether; + + /// @notice min fee that can be charged for a conditional order execution + /// @dev 2 USD + uint256 internal constant LOWER_FEE_CAP = 2 ether; + + /// @notice percentage of the simulated order fee that is charged for a conditional order execution + /// @dev denoted in BPS (basis points) where 1% = 100 BPS and 100% = 10000 BPS + uint256 internal constant FEE_SCALING_FACTOR = 1000; + + /// @notice max BPS + uint256 internal constant MAX_BPS = 10_000; + + /// @notice max number of conditions that can be defined for a conditional order + uint256 internal constant MAX_CONDITIONS = 8; + + /// @notice condition selector constant(s) + bytes4 internal constant isTimestampAfterSelector = + IEngine.isTimestampAfter.selector; + bytes4 internal constant isTimestampBeforeSelector = + IEngine.isTimestampBefore.selector; + bytes4 internal constant isPriceAboveSelector = + IEngine.isPriceAbove.selector; + bytes4 internal constant isPriceBelowSelector = + IEngine.isPriceBelow.selector; + bytes4 internal constant isMarketOpenSelector = + IEngine.isMarketOpen.selector; + bytes4 internal constant isPositionSizeAboveSelector = + IEngine.isPositionSizeAbove.selector; + bytes4 internal constant isPositionSizeBelowSelector = + IEngine.isPositionSizeBelow.selector; + bytes4 internal constant isOrderFeeBelowSelector = + IEngine.isOrderFeeBelow.selector; + /*////////////////////////////////////////////////////////////// IMMUTABLES //////////////////////////////////////////////////////////////*/ @@ -48,10 +100,6 @@ contract Engine is IEngine, Multicallable, EIP712 { mapping(uint128 accountId => mapping(uint256 index => uint256 bitmap)) public nonceBitmap; - /// @notice mapping of condition selector to whether the selector is valid - /// (i.e. can be called within Engine.verifyConditions) - mapping(bytes4 selector => bool) public conditionSelector; - /*////////////////////////////////////////////////////////////// CONSTRUCTOR //////////////////////////////////////////////////////////////*/ @@ -76,20 +124,6 @@ contract Engine is IEngine, Multicallable, EIP712 { SPOT_MARKET_PROXY = ISpotMarketProxy(_spotMarketProxy); SUSD = IERC20(_sUSDProxy); ORACLE = IPyth(_oracle); - - _whitelistConditionSelectors(); - } - - /// @notice whitelist condition selectors - function _whitelistConditionSelectors() internal { - conditionSelector[IEngine.isTimestampAfter.selector] = true; - conditionSelector[IEngine.isTimestampBefore.selector] = true; - conditionSelector[IEngine.isPriceAbove.selector] = true; - conditionSelector[IEngine.isPriceBelow.selector] = true; - conditionSelector[IEngine.isMarketOpen.selector] = true; - conditionSelector[IEngine.isPositionSizeAbove.selector] = true; - conditionSelector[IEngine.isPositionSizeBelow.selector] = true; - conditionSelector[IEngine.isOrderFeeBelow.selector] = true; } /*////////////////////////////////////////////////////////////// @@ -114,7 +148,7 @@ contract Engine is IEngine, Multicallable, EIP712 { returns (bool) { return PERPS_MARKET_PROXY.hasPermission( - _accountId, Constants.PERPS_COMMIT_ASYNC_ORDER_PERMISSION, _caller + _accountId, PERPS_COMMIT_ASYNC_ORDER_PERMISSION, _caller ); } @@ -136,7 +170,7 @@ contract Engine is IEngine, Multicallable, EIP712 { uint128 _accountId, uint256 _wordPos, uint256 _mask - ) external { + ) external override { if (_isAccountOwnerOrDelegate(_accountId, msg.sender)) { nonceBitmap[_accountId][_wordPos] |= _mask; @@ -150,6 +184,7 @@ contract Engine is IEngine, Multicallable, EIP712 { function hasUnorderedNonceBeenUsed(uint128 _accountId, uint256 _nonce) public view + override returns (bool) { (uint256 wordPos, uint256 bitPos) = _bitmapPositions(_nonce); @@ -244,7 +279,7 @@ contract Engine is IEngine, Multicallable, EIP712 { view returns (address synthAddress) { - synthAddress = _synthMarketId == Constants.USD_SYNTH_ID + synthAddress = _synthMarketId == USD_SYNTH_ID ? address(SUSD) : SPOT_MARKET_PROXY.getSynth(_synthMarketId); } @@ -373,14 +408,13 @@ contract Engine is IEngine, Multicallable, EIP712 { }); /// @dev calculate conditional order fee based on scaled order fees - conditionalOrderFee = - (orderFees * Constants.FEE_SCALING_FACTOR) / Constants.MAX_BPS; + conditionalOrderFee = (orderFees * FEE_SCALING_FACTOR) / MAX_BPS; /// @dev ensure conditional order fee is within bounds - if (conditionalOrderFee < Constants.LOWER_FEE_CAP) { - conditionalOrderFee = Constants.LOWER_FEE_CAP; - } else if (conditionalOrderFee > Constants.UPPER_FEE_CAP) { - conditionalOrderFee = Constants.UPPER_FEE_CAP; + if (conditionalOrderFee < LOWER_FEE_CAP) { + conditionalOrderFee = LOWER_FEE_CAP; + } else if (conditionalOrderFee > UPPER_FEE_CAP) { + conditionalOrderFee = UPPER_FEE_CAP; } /// @dev withdraw conditional order fee from account prior to executing order @@ -388,7 +422,7 @@ contract Engine is IEngine, Multicallable, EIP712 { _to: msg.sender, _synth: SUSD, _accountId: _co.orderDetails.accountId, - _synthMarketId: Constants.USD_SYNTH_ID, + _synthMarketId: USD_SYNTH_ID, _amount: -int256(conditionalOrderFee) }); @@ -466,7 +500,7 @@ contract Engine is IEngine, Multicallable, EIP712 { returns (bool) { uint256 length = _co.conditions.length; - if (length > Constants.MAX_CONDITIONS) { + if (length > MAX_CONDITIONS) { revert MaxConditionSizeExceeded(); } @@ -474,11 +508,21 @@ contract Engine is IEngine, Multicallable, EIP712 { bool success; bytes memory response; + // define condition selector intended to be called bytes4 selector = bytes4(_co.conditions[i]); /// @dev checking if the selector is valid prevents the possibility of /// a malicious condition from griefing the executor - if (conditionSelector[selector]) { + if ( + selector == isTimestampAfterSelector + || selector == isTimestampBeforeSelector + || selector == isPriceAboveSelector + || selector == isPriceBelowSelector + || selector == isMarketOpenSelector + || selector == isPositionSizeAboveSelector + || selector == isPositionSizeBelowSelector + || selector == isOrderFeeBelowSelector + ) { // @dev staticcall to prevent state changes in the case a condition is malicious (success, response) = address(this).staticcall(_co.conditions[i]); diff --git a/src/libraries/Constants.sol b/src/libraries/Constants.sol deleted file mode 100644 index b078de55..00000000 --- a/src/libraries/Constants.sol +++ /dev/null @@ -1,41 +0,0 @@ -// SPDX-License-Identifier: GPL-3.0-or-later -pragma solidity 0.8.18; - -/// @title Kwenta Smart Margin v3: Constants Library -/// @author JaredBorders (jaredborders@pm.me) -library Constants { - /*////////////////////////////////////////////////////////////// - CONSTANTS - //////////////////////////////////////////////////////////////*/ - - /// @notice admins have permission to do everything that the account owner can - /// (including granting and revoking permissions for other addresses) except - /// for transferring account ownership - bytes32 internal constant ADMIN_PERMISSION = "ADMIN"; - - /// @notice the permission required to commit an async order - /// @dev this permission does not allow the permission holder to modify collateral - bytes32 internal constant PERPS_COMMIT_ASYNC_ORDER_PERMISSION = - "PERPS_COMMIT_ASYNC_ORDER"; - - /// @notice "0" synthMarketId represents sUSD in Synthetix v3 - uint128 internal constant USD_SYNTH_ID = 0; - - /// @notice max fee that can be charged for a conditional order execution - /// @dev 50 USD - uint256 internal constant UPPER_FEE_CAP = 50 ether; - - /// @notice min fee that can be charged for a conditional order execution - /// @dev 2 USD - uint256 internal constant LOWER_FEE_CAP = 2 ether; - - /// @notice percentage of the simulated order fee that is charged for a conditional order execution - /// @dev denoted in BPS (basis points) where 1% = 100 BPS and 100% = 10000 BPS - uint256 internal constant FEE_SCALING_FACTOR = 1000; - - /// @notice max BPS - uint256 internal constant MAX_BPS = 10_000; - - /// @notice max number of conditions that can be defined for a conditional order - uint256 internal constant MAX_CONDITIONS = 8; -} diff --git a/test/ConditionalOrder.t.sol b/test/ConditionalOrder.t.sol index b819c3f4..40675609 100644 --- a/test/ConditionalOrder.t.sol +++ b/test/ConditionalOrder.t.sol @@ -496,7 +496,10 @@ contract Execute is ConditionalOrderTest { ); uint256 marginPostConditionalOrderFee = AMOUNT - - (orderFees = orderFees * engineExposed.FEE_SCALING_FACTOR() / 10_000); + - ( + orderFees = + orderFees * engineExposed.expose_FEE_SCALING_FACTOR() / 10_000 + ); vm.expectRevert( abi.encodeWithSelector( @@ -580,8 +583,8 @@ contract Fee is ConditionalOrderTest { } function test_fee_imposed_at_upper_fee_cap() public { - uint256 mocked_order_fees = - engineExposed.UPPER_FEE_CAP() * engineExposed.FEE_SCALING_FACTOR(); + uint256 mocked_order_fees = engineExposed.expose_UPPER_FEE_CAP() + * engineExposed.expose_FEE_SCALING_FACTOR(); mock_computeOrderFees({ perpsMarketProxy: address(perpsMarketProxy), @@ -619,13 +622,15 @@ contract Fee is ConditionalOrderTest { (,, uint256 conditionalOrderFee) = engine.execute(co, signature); - assertEq(engineExposed.UPPER_FEE_CAP(), conditionalOrderFee); - assertEq(engineExposed.UPPER_FEE_CAP(), sUSD.balanceOf(address(this))); + assertEq(engineExposed.expose_UPPER_FEE_CAP(), conditionalOrderFee); + assertEq( + engineExposed.expose_UPPER_FEE_CAP(), sUSD.balanceOf(address(this)) + ); } function test_fee_imposed_above_upper_fee_cap() public { - uint256 mocked_order_fees = engineExposed.UPPER_FEE_CAP() - * (engineExposed.FEE_SCALING_FACTOR() + 1); + uint256 mocked_order_fees = engineExposed.expose_UPPER_FEE_CAP() + * (engineExposed.expose_FEE_SCALING_FACTOR() + 1); mock_computeOrderFees({ perpsMarketProxy: address(perpsMarketProxy), @@ -663,12 +668,14 @@ contract Fee is ConditionalOrderTest { (,, uint256 conditionalOrderFee) = engine.execute(co, signature); - assertEq(engineExposed.UPPER_FEE_CAP(), conditionalOrderFee); - assertEq(engineExposed.UPPER_FEE_CAP(), sUSD.balanceOf(address(this))); + assertEq(engineExposed.expose_UPPER_FEE_CAP(), conditionalOrderFee); + assertEq( + engineExposed.expose_UPPER_FEE_CAP(), sUSD.balanceOf(address(this)) + ); } function test_fee_imposed_below_upper_fee_cap() public { - uint256 mocked_order_fees = engineExposed.UPPER_FEE_CAP(); + uint256 mocked_order_fees = engineExposed.expose_UPPER_FEE_CAP(); mock_computeOrderFees({ perpsMarketProxy: address(perpsMarketProxy), @@ -707,13 +714,17 @@ contract Fee is ConditionalOrderTest { (,, uint256 conditionalOrderFee) = engine.execute(co, signature); assertEq( - (engineExposed.UPPER_FEE_CAP() * engineExposed.FEE_SCALING_FACTOR()) - / 10_000, + ( + engineExposed.expose_UPPER_FEE_CAP() + * engineExposed.expose_FEE_SCALING_FACTOR() + ) / 10_000, conditionalOrderFee ); assertEq( - (engineExposed.UPPER_FEE_CAP() * engineExposed.FEE_SCALING_FACTOR()) - / 10_000, + ( + engineExposed.expose_UPPER_FEE_CAP() + * engineExposed.expose_FEE_SCALING_FACTOR() + ) / 10_000, sUSD.balanceOf(address(this)) ); } @@ -757,8 +768,10 @@ contract Fee is ConditionalOrderTest { (,, uint256 conditionalOrderFee) = engine.execute(co, signature); - assertEq(engineExposed.LOWER_FEE_CAP(), conditionalOrderFee); - assertEq(engineExposed.LOWER_FEE_CAP(), sUSD.balanceOf(address(this))); + assertEq(engineExposed.expose_LOWER_FEE_CAP(), conditionalOrderFee); + assertEq( + engineExposed.expose_LOWER_FEE_CAP(), sUSD.balanceOf(address(this)) + ); } function test_fee_imposed_fee_cannot_be_paid() public { @@ -801,7 +814,8 @@ contract Fee is ConditionalOrderTest { sizeDelta: 10 ether }); - orderFees = orderFees * engineExposed.FEE_SCALING_FACTOR() / 10_000; + orderFees = + orderFees * engineExposed.expose_FEE_SCALING_FACTOR() / 10_000; vm.expectRevert( abi.encodeWithSelector( @@ -851,7 +865,10 @@ contract Fee is ConditionalOrderTest { ); uint256 marginPostConditionalOrderFee = AMOUNT - - (orderFees = orderFees * engineExposed.FEE_SCALING_FACTOR() / 10_000); + - ( + orderFees = + orderFees * engineExposed.expose_FEE_SCALING_FACTOR() / 10_000 + ); vm.expectRevert( abi.encodeWithSelector( diff --git a/test/utils/analysis/ConditionalOrderFees.sol b/test/utils/analysis/ConditionalOrderFees.sol index 78df1d91..1bf50107 100644 --- a/test/utils/analysis/ConditionalOrderFees.sol +++ b/test/utils/analysis/ConditionalOrderFees.sol @@ -18,12 +18,14 @@ contract ConditionalOrderTest is Bootstrap { }); uint256 conditional_order_fee = - orderFees * engineExposed.FEE_SCALING_FACTOR() / 10_000; + orderFees * engineExposed.expose_FEE_SCALING_FACTOR() / 10_000; - if (conditional_order_fee < engineExposed.LOWER_FEE_CAP()) { - console2.log("%s,", engineExposed.LOWER_FEE_CAP() / 1e18); - } else if (conditional_order_fee > engineExposed.UPPER_FEE_CAP()) { - console2.log("%s,", engineExposed.UPPER_FEE_CAP() / 1e18); + if (conditional_order_fee < engineExposed.expose_LOWER_FEE_CAP()) { + console2.log("%s,", engineExposed.expose_LOWER_FEE_CAP() / 1e18); + } else if ( + conditional_order_fee > engineExposed.expose_UPPER_FEE_CAP() + ) { + console2.log("%s,", engineExposed.expose_UPPER_FEE_CAP() / 1e18); } else { console2.log("%s,", conditional_order_fee / 1e18); } diff --git a/test/utils/exposed/EngineExposed.sol b/test/utils/exposed/EngineExposed.sol index d638eb43..1bb9e378 100644 --- a/test/utils/exposed/EngineExposed.sol +++ b/test/utils/exposed/EngineExposed.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-3.0-or-later pragma solidity 0.8.18; -import {Engine, Constants, MathLib} from "src/Engine.sol"; +import {Engine, MathLib} from "src/Engine.sol"; contract EngineExposed is Engine { using MathLib for uint256; @@ -21,19 +21,19 @@ contract EngineExposed is Engine { return _getSynthAddress(synthMarketId); } - function UPPER_FEE_CAP() public pure returns (uint256) { - return Constants.UPPER_FEE_CAP; + function expose_UPPER_FEE_CAP() public pure returns (uint256) { + return UPPER_FEE_CAP; } - function LOWER_FEE_CAP() public pure returns (uint256) { - return Constants.LOWER_FEE_CAP; + function expose_LOWER_FEE_CAP() public pure returns (uint256) { + return LOWER_FEE_CAP; } - function FEE_SCALING_FACTOR() public pure returns (uint256) { - return Constants.FEE_SCALING_FACTOR; + function expose_FEE_SCALING_FACTOR() public pure returns (uint256) { + return FEE_SCALING_FACTOR; } - function MAX_BPS() public pure returns (uint256) { - return Constants.MAX_BPS; + function expose_MAX_BPS() public pure returns (uint256) { + return MAX_BPS; } } From 11c0541a754f066a4a04ff739c78d034cd750aa6 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Sun, 17 Sep 2023 18:32:02 -0400 Subject: [PATCH 11/13] =?UTF-8?q?=F0=9F=97=91=EF=B8=8F=20Remove=20ADMIN=5F?= =?UTF-8?q?PERMISSION?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Engine.sol | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Engine.sol b/src/Engine.sol index 1fc01f31..84a6e4a8 100644 --- a/src/Engine.sol +++ b/src/Engine.sol @@ -26,11 +26,6 @@ contract Engine is IEngine, Multicallable, EIP712 { CONSTANTS //////////////////////////////////////////////////////////////*/ - /// @notice admins have permission to do everything that the account owner can - /// (including granting and revoking permissions for other addresses) except - /// for transferring account ownership - bytes32 internal constant ADMIN_PERMISSION = "ADMIN"; - /// @notice the permission required to commit an async order /// @dev this permission does not allow the permission holder to modify collateral bytes32 internal constant PERPS_COMMIT_ASYNC_ORDER_PERMISSION = From b8206743f15129e64e8ea2cc8a85aeb6ea49a33e Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Sun, 17 Sep 2023 18:41:39 -0400 Subject: [PATCH 12/13] =?UTF-8?q?=F0=9F=91=B7=F0=9F=8F=BB=E2=80=8D?= =?UTF-8?q?=E2=99=82=EF=B8=8F=20Use=20isAuthorized()=20in=20=5FisAccountOw?= =?UTF-8?q?nerOrDelegate()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Engine.sol | 7 ++++--- src/interfaces/synthetix/IPerpsMarketProxy.sol | 10 ++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/Engine.sol b/src/Engine.sol index 84a6e4a8..bcd9c3ba 100644 --- a/src/Engine.sol +++ b/src/Engine.sol @@ -137,7 +137,7 @@ contract Engine is IEngine, Multicallable, EIP712 { /// @inheritdoc IEngine function isAccountDelegate(uint128 _accountId, address _caller) - public + external view override returns (bool) @@ -152,8 +152,9 @@ contract Engine is IEngine, Multicallable, EIP712 { view returns (bool) { - return isAccountOwner(_accountId, _caller) - || isAccountDelegate(_accountId, _caller); + return PERPS_MARKET_PROXY.isAuthorized( + _accountId, PERPS_COMMIT_ASYNC_ORDER_PERMISSION, _caller + ); } /*////////////////////////////////////////////////////////////// diff --git a/src/interfaces/synthetix/IPerpsMarketProxy.sol b/src/interfaces/synthetix/IPerpsMarketProxy.sol index 52773fd2..55598ffb 100644 --- a/src/interfaces/synthetix/IPerpsMarketProxy.sol +++ b/src/interfaces/synthetix/IPerpsMarketProxy.sol @@ -62,6 +62,16 @@ interface IPerpsMarketProxy { view returns (bool hasPermission); + /// @notice Returns `true` if `target` is authorized to `permission` for account `accountId`. + /// @param accountId The id of the account whose permission is being queried. + /// @param permission The bytes32 identifier of the permission. + /// @param target The target address whose permission is being queried. + /// @return isAuthorized A boolean with the response of the query. + function isAuthorized(uint128 accountId, bytes32 permission, address target) + external + view + returns (bool isAuthorized); + /*////////////////////////////////////////////////////////////// ASYNC ORDER MODULE //////////////////////////////////////////////////////////////*/ From 69ed84441ec874e45b7b8f1b292e27fb2f960812 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Sun, 17 Sep 2023 18:44:45 -0400 Subject: [PATCH 13/13] =?UTF-8?q?=F0=9F=93=B8=20Update=20gas-snapshot/lcov?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gas-snapshot | 68 +++++----- lcov.info | 339 +++++++++++++++++++++++++------------------------- 2 files changed, 203 insertions(+), 204 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index f63badbd..136dedb8 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,15 +1,15 @@ AccountDelegate:test_isAccountDelegate_account_doesnt_exist() (gas: 26494) -AccountDelegate:test_isAccountDelegate_false() (gas: 391958) -AccountDelegate:test_isAccountDelegate_true() (gas: 389927) +AccountDelegate:test_isAccountDelegate_false() (gas: 391959) +AccountDelegate:test_isAccountDelegate_true() (gas: 389928) AccountOwner:test_isAccountOwner_account_doesnt_exist() (gas: 25032) AccountOwner:test_isAccountOwner_false() (gas: 229800) AccountOwner:test_isAccountOwner_true() (gas: 229790) -CanExecute:test_canExecute_false_nonce_used() (gas: 623091) -CanExecute:test_canExecute_false_trusted_executor() (gas: 43976) -CanExecute:test_canExecute_true() (gas: 43575) -CommitOrder:test_commitOrder() (gas: 378335) -CommitOrder:test_commitOrder_insufficient_collateral() (gas: 433265) -CommitOrder:test_commitOrder_invalid_market() (gas: 37345) +CanExecute:test_canExecute_false_nonce_used() (gas: 623164) +CanExecute:test_canExecute_false_trusted_executor() (gas: 44049) +CanExecute:test_canExecute_true() (gas: 43648) +CommitOrder:test_commitOrder() (gas: 378408) +CommitOrder:test_commitOrder_insufficient_collateral() (gas: 433338) +CommitOrder:test_commitOrder_invalid_market() (gas: 37418) Conditions:test_isMarketOpen() (gas: 26784) Conditions:test_isOrderFeeBelow() (gas: 168839) Conditions:test_isPositionSizeAbove() (gas: 18801) @@ -18,26 +18,26 @@ Conditions:test_isPriceAbove() (gas: 19194) Conditions:test_isPriceBelow() (gas: 18998) Conditions:test_isTimestampAfter() (gas: 7645) Conditions:test_isTimestampBefore() (gas: 7668) -DeploymentTest:test_deploy() (gas: 2653558) -DeploymentTest:test_deploy_oracle_zero_address() (gas: 39840) -DeploymentTest:test_deploy_perps_market_proxy_zero_address() (gas: 39736) -DeploymentTest:test_deploy_spot_market_proxy_zero_address() (gas: 39814) -DeploymentTest:test_deploy_susd_proxy_zero_address() (gas: 39827) +DeploymentTest:test_deploy() (gas: 2684850) +DeploymentTest:test_deploy_oracle_zero_address() (gas: 39879) +DeploymentTest:test_deploy_perps_market_proxy_zero_address() (gas: 39775) +DeploymentTest:test_deploy_spot_market_proxy_zero_address() (gas: 39853) +DeploymentTest:test_deploy_susd_proxy_zero_address() (gas: 39866) DepositCollateral:test_depositCollateral() (gas: 258349) DepositCollateral:test_depositCollateral_availableMargin() (gas: 265906) DepositCollateral:test_depositCollateral_collateralAmount() (gas: 258925) DepositCollateral:test_depositCollateral_insufficient_balance() (gas: 55890) DepositCollateral:test_depositCollateral_totalCollateralValue() (gas: 263279) -Execute:test_execute_CannotExecuteOrder() (gas: 39009) -Execute:test_execute_leverage_exceeded() (gas: 697970) -Execute:test_execute_order_committed() (gas: 621071) -Fee:test_fee_imposed() (gas: 627168) -Fee:test_fee_imposed_above_upper_fee_cap() (gas: 608109) -Fee:test_fee_imposed_at_upper_fee_cap() (gas: 608012) -Fee:test_fee_imposed_below_lower_fee_cap() (gas: 606659) -Fee:test_fee_imposed_below_upper_fee_cap() (gas: 608932) -Fee:test_fee_imposed_fee_cannot_be_paid() (gas: 382415) -Fee:test_fee_imposed_insufficient_collateral_for_order() (gas: 697971) +Execute:test_execute_CannotExecuteOrder() (gas: 36391) +Execute:test_execute_leverage_exceeded() (gas: 698043) +Execute:test_execute_order_committed() (gas: 621144) +Fee:test_fee_imposed() (gas: 627241) +Fee:test_fee_imposed_above_upper_fee_cap() (gas: 608182) +Fee:test_fee_imposed_at_upper_fee_cap() (gas: 608085) +Fee:test_fee_imposed_below_lower_fee_cap() (gas: 606732) +Fee:test_fee_imposed_below_upper_fee_cap() (gas: 609005) +Fee:test_fee_imposed_fee_cannot_be_paid() (gas: 382488) +Fee:test_fee_imposed_insufficient_collateral_for_order() (gas: 698044) MathLibTest:test_abs128() (gas: 448) MathLibTest:test_abs256() (gas: 480) MathLibTest:test_castU128() (gas: 350) @@ -45,23 +45,23 @@ MathLibTest:test_castU128_overflow() (gas: 3509) MathLibTest:test_fuzz_abs128(int128) (runs: 256, μ: 577, ~: 603) MathLibTest:test_fuzz_abs256(int256) (runs: 256, μ: 472, ~: 458) MathLibTest:test_isSameSign() (gas: 999) -MulticallableTest:test_multicall_depositCollateral_commitOrder() (gas: 600377) -NonceBitmap:test_hasUnorderedNonceBeenUsed() (gas: 53827) -NonceBitmap:test_invalidateUnorderedNonces() (gas: 72661) -NonceBitmap:test_invalidateUnorderedNonces_Only_Owner_Delegate() (gas: 190194) -ReduceOnly:test_reduce_only() (gas: 622705) -ReduceOnly:test_reduce_only_same_sign() (gas: 68324) -ReduceOnly:test_reduce_only_truncate_size_down() (gas: 622835) -ReduceOnly:test_reduce_only_truncate_size_up() (gas: 603648) -ReduceOnly:test_reduce_only_zero_size() (gas: 158540) +MulticallableTest:test_multicall_depositCollateral_commitOrder() (gas: 600450) +NonceBitmap:test_hasUnorderedNonceBeenUsed() (gas: 53903) +NonceBitmap:test_invalidateUnorderedNonces() (gas: 72810) +NonceBitmap:test_invalidateUnorderedNonces_Only_Owner_Delegate() (gas: 189948) +ReduceOnly:test_reduce_only() (gas: 622778) +ReduceOnly:test_reduce_only_same_sign() (gas: 68397) +ReduceOnly:test_reduce_only_truncate_size_down() (gas: 622908) +ReduceOnly:test_reduce_only_truncate_size_up() (gas: 603721) +ReduceOnly:test_reduce_only_zero_size() (gas: 158613) VerifyConditions:test_max_condition_size_exceeded() (gas: 45046) VerifyConditions:test_verifyConditions_InvalidConditionSelector() (gas: 14079) VerifyConditions:test_verify_conditions_not_verified() (gas: 29659) VerifyConditions:test_verify_conditions_verified() (gas: 135662) VerifySignature:test_verifySignature() (gas: 23770) VerifySignature:test_verifySignature_false_private_key() (gas: 26592) -VerifySigner:test_verifySigner() (gas: 25722) -VerifySigner:test_verifySigner_false() (gas: 28831) +VerifySigner:test_verifySigner() (gas: 25801) +VerifySigner:test_verifySigner_false() (gas: 28509) WithdrawCollateral:test_withdrawCollateral() (gas: 348278) WithdrawCollateral:test_withdrawCollateral_availableMargin() (gas: 349788) WithdrawCollateral:test_withdrawCollateral_collateralAmount() (gas: 348788) diff --git a/lcov.info b/lcov.info index 25d4be15..0c10e533 100644 --- a/lcov.info +++ b/lcov.info @@ -1,14 +1,14 @@ TN: SF:script/Deploy.s.sol -FN:14,Setup.deploySystem -FNDA:0,Setup.deploySystem -DA:20,0 FN:33,DeployOptimism.run FNDA:0,DeployOptimism.run DA:34,0 DA:35,0 DA:37,0 DA:44,0 +FN:14,Setup.deploySystem +FNDA:0,Setup.deploySystem +DA:20,0 FN:52,DeployOptimismGoerli.run FNDA:0,DeployOptimismGoerli.run DA:53,0 @@ -24,200 +24,199 @@ BRH:0 end_of_record TN: SF:src/Engine.sol -FN:134,Engine.isAccountOwner +FN:129,Engine.isAccountOwner FNDA:3,Engine.isAccountOwner -DA:140,39 -FN:144,Engine.isAccountDelegate +DA:135,10 +FN:139,Engine.isAccountDelegate FNDA:2,Engine.isAccountDelegate -DA:150,5 -FN:155,Engine._isAccountOwnerOrDelegate +DA:145,2 +FN:150,Engine._isAccountOwnerOrDelegate FNDA:29,Engine._isAccountOwnerOrDelegate -DA:160,29 -DA:161,3 -FN:169,Engine.invalidateUnorderedNonces +DA:155,29 +FN:165,Engine.invalidateUnorderedNonces FNDA:4,Engine.invalidateUnorderedNonces -DA:174,4 -BRDA:174,0,0,4 -BRDA:174,0,1,- -DA:175,4 -DA:177,4 -DA:179,0 -FN:184,Engine.hasUnorderedNonceBeenUsed +DA:170,4 +BRDA:170,0,0,4 +BRDA:170,0,1,- +DA:171,4 +DA:173,4 +DA:175,0 +FN:180,Engine.hasUnorderedNonceBeenUsed FNDA:4,Engine.hasUnorderedNonceBeenUsed -DA:190,25 -DA:191,25 -DA:192,25 -FN:202,Engine._bitmapPositions +DA:186,25 +DA:187,25 +DA:188,25 +FN:198,Engine._bitmapPositions FNDA:40,Engine._bitmapPositions -DA:207,40 -DA:208,40 -FN:214,Engine._useUnorderedNonce +DA:203,40 +DA:204,40 +FN:210,Engine._useUnorderedNonce FNDA:15,Engine._useUnorderedNonce +DA:211,15 +DA:212,15 +DA:213,15 DA:215,15 -DA:216,15 -DA:217,15 -DA:219,15 -BRDA:219,1,0,- -BRDA:219,1,1,15 -FN:227,Engine.modifyCollateral +BRDA:215,1,0,- +BRDA:215,1,1,15 +FN:223,Engine.modifyCollateral FNDA:19,Engine.modifyCollateral -DA:232,19 -DA:234,19 -BRDA:234,2,0,12 -BRDA:234,2,1,7 -DA:235,12 -DA:239,7 -BRDA:239,3,0,- -BRDA:239,3,1,7 -DA:240,7 -FN:246,Engine._depositCollateral +DA:228,19 +DA:230,19 +BRDA:230,2,0,12 +BRDA:230,2,1,7 +DA:231,12 +DA:235,7 +BRDA:235,3,0,- +BRDA:235,3,1,7 +DA:236,7 +FN:242,Engine._depositCollateral FNDA:12,Engine._depositCollateral -DA:254,12 -DA:256,11 -DA:258,11 -FN:261,Engine._withdrawCollateral +DA:250,12 +DA:252,11 +DA:254,11 +FN:257,Engine._withdrawCollateral FNDA:20,Engine._withdrawCollateral -DA:268,20 -DA:271,17 -FN:277,Engine._getSynthAddress +DA:264,20 +DA:267,17 +FN:273,Engine._getSynthAddress FNDA:19,Engine._getSynthAddress -DA:282,19 -FN:292,Engine.commitOrder +DA:278,19 +FN:288,Engine.commitOrder FNDA:4,Engine.commitOrder -DA:306,4 -BRDA:306,4,0,4 -BRDA:306,4,1,- -DA:307,4 -DA:317,0 -FN:321,Engine._commitOrder +DA:302,4 +BRDA:302,4,0,4 +BRDA:302,4,1,- +DA:303,4 +DA:313,0 +FN:317,Engine._commitOrder FNDA:16,Engine._commitOrder -DA:330,16 -FN:348,Engine.execute +DA:326,16 +FN:344,Engine.execute FNDA:16,Engine.execute -DA:361,16 -BRDA:361,5,0,1 -BRDA:361,5,1,15 +DA:357,16 +BRDA:357,5,0,1 +BRDA:357,5,1,15 +DA:360,15 DA:364,15 -DA:368,15 -DA:371,15 -BRDA:371,6,0,2 -BRDA:371,6,1,3 -DA:372,5 -DA:377,5 -BRDA:377,7,0,1 -BRDA:377,7,1,4 -DA:378,1 -DA:382,4 -BRDA:382,8,0,1 -BRDA:382,8,1,3 -DA:383,1 -DA:389,3 -BRDA:389,9,0,2 -BRDA:389,9,1,3 -DA:399,2 -DA:405,13 -DA:411,13 -DA:414,13 -BRDA:414,10,0,6 -BRDA:414,10,1,7 -DA:415,6 -DA:416,7 -BRDA:416,11,0,2 -BRDA:416,11,1,7 -DA:417,2 -DA:421,13 -DA:430,12 -FN:442,Engine.canExecute +DA:367,15 +BRDA:367,6,0,2 +BRDA:367,6,1,3 +DA:368,5 +DA:373,5 +BRDA:373,7,0,1 +BRDA:373,7,1,4 +DA:374,1 +DA:378,4 +BRDA:378,8,0,1 +BRDA:378,8,1,3 +DA:379,1 +DA:385,3 +BRDA:385,9,0,2 +BRDA:385,9,1,3 +DA:395,2 +DA:401,13 +DA:407,13 +DA:410,13 +BRDA:410,10,0,6 +BRDA:410,10,1,7 +DA:411,6 +DA:412,7 +BRDA:412,11,0,2 +BRDA:412,11,1,7 +DA:413,2 +DA:417,13 +DA:426,12 +FN:438,Engine.canExecute FNDA:5,Engine.canExecute -DA:447,21 -BRDA:447,12,0,2 -BRDA:447,12,1,19 -DA:448,2 -DA:452,19 -BRDA:452,13,0,1 -BRDA:452,13,1,18 -DA:455,18 -BRDA:455,14,0,- -BRDA:455,14,1,18 -DA:458,18 -BRDA:458,15,0,- -BRDA:458,15,1,- -DA:461,0 -BRDA:461,16,0,- -BRDA:461,16,1,- -DA:465,18 -BRDA:465,17,0,1 -BRDA:465,17,1,17 -DA:468,17 -FN:476,Engine.verifySigner +DA:443,21 +BRDA:443,12,0,2 +BRDA:443,12,1,19 +DA:444,2 +DA:448,19 +BRDA:448,13,0,1 +BRDA:448,13,1,18 +DA:451,18 +BRDA:451,14,0,- +BRDA:451,14,1,18 +DA:454,18 +BRDA:454,15,0,- +BRDA:454,15,1,- +DA:457,0 +BRDA:457,16,0,- +BRDA:457,16,1,- +DA:461,18 +BRDA:461,17,0,1 +BRDA:461,17,1,17 +DA:464,17 +FN:472,Engine.verifySigner FNDA:2,Engine.verifySigner -DA:482,21 -FN:486,Engine.verifySignature +DA:478,21 +FN:482,Engine.verifySignature FNDA:2,Engine.verifySignature -DA:490,20 -FN:496,Engine.verifyConditions +DA:486,20 +FN:492,Engine.verifyConditions FNDA:4,Engine.verifyConditions -DA:502,4 -DA:503,4 -BRDA:503,18,0,1 -BRDA:503,18,1,3 -DA:504,1 -DA:507,3 +DA:498,4 +DA:499,4 +BRDA:499,18,0,1 +BRDA:499,18,1,3 +DA:500,1 +DA:503,3 +DA:504,13 +DA:505,13 DA:508,13 -DA:509,13 -DA:512,13 -DA:517,13 -DA:518,11 -DA:519,9 -DA:520,7 -DA:521,5 -DA:522,4 -DA:523,3 -DA:524,2 -BRDA:516,19,0,1 -BRDA:516,19,1,11 -DA:527,12 -DA:530,12 -BRDA:530,20,0,1 -BRDA:530,20,1,11 -DA:533,11 +DA:513,13 +DA:514,11 +DA:515,9 +DA:516,7 +DA:517,5 +DA:518,4 +DA:519,3 +DA:520,2 +BRDA:512,19,0,1 +BRDA:512,19,1,11 +DA:523,12 +DA:526,12 +BRDA:526,20,0,1 +BRDA:526,20,1,11 +DA:529,11 +DA:532,1 DA:536,1 -DA:540,1 -FN:548,Engine.isTimestampAfter +FN:544,Engine.isTimestampAfter FNDA:5,Engine.isTimestampAfter -DA:554,5 -FN:558,Engine.isTimestampBefore +DA:550,5 +FN:554,Engine.isTimestampBefore FNDA:5,Engine.isTimestampBefore -DA:564,5 -FN:568,Engine.isPriceAbove +DA:560,5 +FN:564,Engine.isPriceAbove FNDA:6,Engine.isPriceAbove -DA:573,6 -DA:578,6 -FN:582,Engine.isPriceBelow +DA:569,6 +DA:574,6 +FN:578,Engine.isPriceBelow FNDA:6,Engine.isPriceBelow -DA:587,6 -DA:592,6 -FN:596,Engine.isMarketOpen +DA:583,6 +DA:588,6 +FN:592,Engine.isMarketOpen FNDA:3,Engine.isMarketOpen -DA:602,3 -FN:606,Engine.isPositionSizeAbove +DA:598,3 +FN:602,Engine.isPositionSizeAbove FNDA:4,Engine.isPositionSizeAbove -DA:611,4 -DA:612,4 -DA:614,4 -FN:618,Engine.isPositionSizeBelow +DA:607,4 +DA:608,4 +DA:610,4 +FN:614,Engine.isPositionSizeBelow FNDA:4,Engine.isPositionSizeBelow -DA:623,4 -DA:624,4 -DA:626,4 -FN:630,Engine.isOrderFeeBelow +DA:619,4 +DA:620,4 +DA:622,4 +FN:626,Engine.isOrderFeeBelow FNDA:4,Engine.isOrderFeeBelow -DA:636,4 -DA:641,4 +DA:632,4 +DA:637,4 FNF:26 FNH:26 -LF:96 -LH:93 +LF:95 +LH:92 BRF:42 BRH:33 end_of_record @@ -410,6 +409,11 @@ FNDA:0,BootstrapOptimism.init DA:106,0 DA:113,0 DA:120,0 +FN:132,BootstrapOptimismGoerli.init +FNDA:0,BootstrapOptimismGoerli.init +DA:136,0 +DA:143,0 +DA:150,0 FN:36,Bootstrap.initializeOptimismGoerli FNDA:0,Bootstrap.initializeOptimismGoerli DA:37,0 @@ -446,11 +450,6 @@ DA:89,0 DA:90,0 DA:95,0 DA:97,0 -FN:132,BootstrapOptimismGoerli.init -FNDA:0,BootstrapOptimismGoerli.init -DA:136,0 -DA:143,0 -DA:150,0 FNF:4 FNH:0 LF:38