From e9bfced8c3de9737e4ec4acf05b0a33d2d3dfd36 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Thu, 20 Jul 2023 13:59:42 -0400 Subject: [PATCH 1/8] =?UTF-8?q?=F0=9F=9A=80=20Update=20script=20for=20v2.1?= =?UTF-8?q?.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- script/upgrades/{v2.0.3 => v2.1.0}/Upgrade.s.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) rename script/upgrades/{v2.0.3 => v2.1.0}/Upgrade.s.sol (97%) diff --git a/script/upgrades/v2.0.3/Upgrade.s.sol b/script/upgrades/v2.1.0/Upgrade.s.sol similarity index 97% rename from script/upgrades/v2.0.3/Upgrade.s.sol rename to script/upgrades/v2.1.0/Upgrade.s.sol index 0ea9219c..7a831c2c 100644 --- a/script/upgrades/v2.0.3/Upgrade.s.sol +++ b/script/upgrades/v2.1.0/Upgrade.s.sol @@ -33,12 +33,12 @@ import { OPTIMISM_GOERLI_UNISWAP_UNIVERSAL_ROUTER } from "script/utils/parameters/OptimismGoerliParameters.sol"; -/// @title Script to upgrade the Account implementation v2.0.2 -> v2.0.3 +/// @title Script to upgrade the Account implementation v2.0.2 -> v2.1.0 /// @author JaredBorders (jaredborders@pm.me) /// @dev steps to deploy and verify on Optimism: /// (1) load the variables in the .env file via `source .env` -/// (2) run `forge script script/upgrades/v2.0.3/Upgrade.s.sol:UpgradeAccountOptimism --rpc-url $ARCHIVE_NODE_URL_L2 --broadcast --verify -vvvv` +/// (2) run `forge script script/upgrades/v2.1.0/Upgrade.s.sol:UpgradeAccountOptimism --rpc-url $ARCHIVE_NODE_URL_L2 --broadcast --verify -vvvv` /// (3) Smart Margin Account Factory owner (i.e. Kwenta pDAO) will need to call `upgradeAccountImplementation` on the Factory with the address of the new Account implementation contract UpgradeAccountOptimism is Script { function run() public { @@ -92,7 +92,7 @@ contract UpgradeAccountOptimism is Script { /// @dev steps to deploy and verify on Optimism Goerli: /// (1) load the variables in the .env file via `source .env` -/// (2) run `forge script script/upgrades/v2.0.3/Upgrade.s.sol:UpgradeAccountOptimismGoerli --rpc-url $ARCHIVE_NODE_URL_GOERLI_L2 --broadcast --verify -vvvv` +/// (2) run `forge script script/upgrades/v2.1.0/Upgrade.s.sol:UpgradeAccountOptimismGoerli --rpc-url $ARCHIVE_NODE_URL_GOERLI_L2 --broadcast --verify -vvvv` /// (3) Smart Margin Account Factory owner (i.e. Kwenta pDAO) will need to call `upgradeAccountImplementation` on the Factory with the address of the new Account implementation contract UpgradeAccountOptimismGoerli is Script { function run() public { From 9ef29d88f56b63b793e7d3f546c5cfe252b64b06 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Thu, 20 Jul 2023 14:00:16 -0400 Subject: [PATCH 2/8] =?UTF-8?q?=E2=9C=85=20Add=20upgrade=20tests=20for=20v?= =?UTF-8?q?2.1.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/upgrades/{v2.0.3 => v2.1.0}/Upgrade.t.sol | 10 +++++----- .../{v2.0.3 => v2.1.0}/interfaces/IAccount.sol | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) rename test/upgrades/{v2.0.3 => v2.1.0}/Upgrade.t.sol (97%) rename test/upgrades/{v2.0.3 => v2.1.0}/interfaces/IAccount.sol (99%) diff --git a/test/upgrades/v2.0.3/Upgrade.t.sol b/test/upgrades/v2.1.0/Upgrade.t.sol similarity index 97% rename from test/upgrades/v2.0.3/Upgrade.t.sol rename to test/upgrades/v2.1.0/Upgrade.t.sol index 1a780f78..ca1f7867 100644 --- a/test/upgrades/v2.0.3/Upgrade.t.sol +++ b/test/upgrades/v2.1.0/Upgrade.t.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.18; import {Test} from "lib/forge-std/src/Test.sol"; -import {UpgradeAccountOptimism} from "script/upgrades/v2.0.3/Upgrade.s.sol"; +import {UpgradeAccountOptimism} from "script/upgrades/v2.1.0/Upgrade.s.sol"; import { OPTIMISM_FACTORY, OPTIMISM_PDAO, @@ -14,7 +14,7 @@ import {Factory} from "src/Factory.sol"; import {IAccount as OldAccount} from "test/upgrades/v2.0.2/interfaces/IAccount.sol"; import {IAccount as NewAccount} from - "test/upgrades/v2.0.3/interfaces/IAccount.sol"; + "test/upgrades/v2.1.0/interfaces/IAccount.sol"; import {IERC20} from "src/interfaces/token/IERC20.sol"; import {ISynth} from "test/utils/interfaces/ISynth.sol"; @@ -36,7 +36,7 @@ contract UpgradeTest is Test { 0x3BC8D34314E77c2E36948Fd8F4B353f1baDc3F6C; /*////////////////////////////////////////////////////////////// - V2.0.3 IMPLEMENTATION + V2.1.0 IMPLEMENTATION //////////////////////////////////////////////////////////////*/ address private NEW_IMPLEMENTATION; @@ -61,7 +61,7 @@ contract UpgradeTest is Test { UpgradeAccountOptimism upgradeAccountOptimism = new UpgradeAccountOptimism(); - // deploy v2.0.3 implementation + // deploy v2.1.0 implementation address implementationAddr = upgradeAccountOptimism.upgrade(); NEW_IMPLEMENTATION = payable(implementationAddr); } @@ -121,7 +121,7 @@ contract UpgradeTest is Test { * EXECUTE UPGRADE */ - // upgrade Active Account to v2.0.3 + // upgrade Active Account to v2.1.0 vm.prank(OPTIMISM_PDAO); Factory(OPTIMISM_FACTORY).upgradeAccountImplementation( address(NEW_IMPLEMENTATION) diff --git a/test/upgrades/v2.0.3/interfaces/IAccount.sol b/test/upgrades/v2.1.0/interfaces/IAccount.sol similarity index 99% rename from test/upgrades/v2.0.3/interfaces/IAccount.sol rename to test/upgrades/v2.1.0/interfaces/IAccount.sol index 96c4fc93..fd8e1a8a 100644 --- a/test/upgrades/v2.0.3/interfaces/IAccount.sol +++ b/test/upgrades/v2.1.0/interfaces/IAccount.sol @@ -4,7 +4,7 @@ pragma solidity 0.8.18; import {IPerpsV2MarketConsolidated} from "src/interfaces/synthetix/IPerpsV2MarketConsolidated.sol"; -/// @title Kwenta Smart Margin Account v2.0.3 Implementation Interface +/// @title Kwenta Smart Margin Account v2.1.0 Implementation Interface /// @author JaredBorders (jaredborders@pm.me), JChiaramonte7 (jeremy@bytecode.llc) interface IAccount { /*/////////////////////////////////////////////////////////////// From 3fd0cafbbeec7025524ba7dfa12638e1b74f71f7 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Thu, 20 Jul 2023 14:09:02 -0400 Subject: [PATCH 3/8] =?UTF-8?q?=F0=9F=AA=B2=20Fix=20import=20bug?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/utils/uniswap/V3Path.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/uniswap/V3Path.sol b/src/utils/uniswap/V3Path.sol index 2ad0b9dc..5dc594da 100644 --- a/src/utils/uniswap/V3Path.sol +++ b/src/utils/uniswap/V3Path.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.18; import {BytesLib} from "src/utils/uniswap/BytesLib.sol"; -import {Constants} from "src/utils/uniswap//Constants.sol"; +import {Constants} from "src/utils/uniswap/Constants.sol"; /// @title Functions for manipulating path data for multihop swaps library V3Path { From 01aa8ba4cfc65681f07acbebb48a42dbc0017654 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Fri, 21 Jul 2023 15:33:54 -0400 Subject: [PATCH 4/8] =?UTF-8?q?=F0=9F=91=B7=F0=9F=8F=BB=E2=80=8D=E2=99=82?= =?UTF-8?q?=EF=B8=8F=20[G-1]=20Remove=20approvals?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Account.sol | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/src/Account.sol b/src/Account.sol index c2bdff45..b0362252 100644 --- a/src/Account.sol +++ b/src/Account.sol @@ -995,6 +995,13 @@ contract Account is IAccount, Auth, OpsReady { UNISWAP //////////////////////////////////////////////////////////////*/ + /// @notice swap tokens via Uniswap V3 (sUSD <-> whitelisted token) + /// @dev assumes sufficient token allowances (i.e. Permit2 and this contract) + /// @dev non-whitelisted connector tokens will NOT cause a revert + /// (i.e. sUSD -> non-whitelisted token -> whitelisted token) + /// @param _amountIn: amount of token to swap + /// @param _amountOutMin: minimum amount of token to receive + /// @param _path: path of tokens to swap (token0 - fee - token1) /// @notice swap tokens via Uniswap V3 (sUSD <-> whitelisted token) /// @dev assumes sufficient token allowances (i.e. Permit2 and this contract) /// @dev non-whitelisted connector tokens will NOT cause a revert @@ -1023,6 +1030,10 @@ contract Account is IAccount, Auth, OpsReady { _sufficientMargin(int256(_amountIn)); recipient = msg.sender; + + // transfer sUSD to the UniversalRouter for the swap + /// @dev not using SafeERC20 because sUSD is a trusted token + IERC20(tokenIn).transfer(address(UNISWAP_UNIVERSAL_ROUTER), _amountIn); } else if ( tokenOut == address(MARGIN_ASSET) && SETTINGS.isWhitelistedTokens(tokenIn) @@ -1031,7 +1042,7 @@ contract Account is IAccount, Auth, OpsReady { /// @dev msg.sender must have approved Permit2 to spend at least the amountIn PERMIT2.transferFrom({ from: msg.sender, - to: address(this), + to: address(UNISWAP_UNIVERSAL_ROUTER), amount: _amountIn.toUint160(), token: tokenIn }); @@ -1042,18 +1053,6 @@ contract Account is IAccount, Auth, OpsReady { revert TokenSwapNotAllowed(tokenIn, tokenOut); } - // approve Permit2 to spend _amountIn of this contract's tokenIn - IERC20(tokenIn).approve(address(PERMIT2), _amountIn); - - // approve tokens to be swapped via Universal Router - PERMIT2.approve({ - token: tokenIn, - spender: address(UNISWAP_UNIVERSAL_ROUTER), - amount: _amountIn.toUint160(), - /// @dev timstamp will never overflow (i.e. maximum value of uint48 is year 8 million 921 thousand 556) - expiration: uint48(block.timestamp) - }); - _universalRouterExecute(recipient, _amountIn, _amountOutMin, _path); EVENTS.emitUniswapV3Swap({ @@ -1099,15 +1098,15 @@ contract Account is IAccount, Auth, OpsReady { uint256 _amountOutMin, bytes calldata _path ) internal { - /// @dev payerIsUser (i.e. 5th argument encoded) will always be true + /// @dev payerIsUser (i.e. 5th argument encoded) will always be false because + /// tokens are transferred to the UniversalRouter before executing the swap bytes[] memory inputs = new bytes[](1); inputs[0] = - abi.encode(_recipient, _amountIn, _amountOutMin, _path, true); + abi.encode(_recipient, _amountIn, _amountOutMin, _path, false); UNISWAP_UNIVERSAL_ROUTER.execute({ commands: abi.encodePacked(bytes1(uint8(V3_SWAP_EXACT_IN))), inputs: inputs - /// @custom:auditor removed deadline here and want increased scrutiny }); } From da5071f3b6080814388a2a4bd9eefe9209d3c9d4 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Fri, 21 Jul 2023 15:35:58 -0400 Subject: [PATCH 5/8] =?UTF-8?q?=F0=9F=97=91=EF=B8=8F=20Remove=20comments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Account.sol | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/Account.sol b/src/Account.sol index b0362252..6932ae03 100644 --- a/src/Account.sol +++ b/src/Account.sol @@ -995,13 +995,6 @@ contract Account is IAccount, Auth, OpsReady { UNISWAP //////////////////////////////////////////////////////////////*/ - /// @notice swap tokens via Uniswap V3 (sUSD <-> whitelisted token) - /// @dev assumes sufficient token allowances (i.e. Permit2 and this contract) - /// @dev non-whitelisted connector tokens will NOT cause a revert - /// (i.e. sUSD -> non-whitelisted token -> whitelisted token) - /// @param _amountIn: amount of token to swap - /// @param _amountOutMin: minimum amount of token to receive - /// @param _path: path of tokens to swap (token0 - fee - token1) /// @notice swap tokens via Uniswap V3 (sUSD <-> whitelisted token) /// @dev assumes sufficient token allowances (i.e. Permit2 and this contract) /// @dev non-whitelisted connector tokens will NOT cause a revert From 5103f83ada8d03272cb6ade3068b3416a698eef9 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Fri, 21 Jul 2023 15:39:22 -0400 Subject: [PATCH 6/8] =?UTF-8?q?=F0=9F=91=B7=F0=9F=8F=BB=E2=80=8D=E2=99=82?= =?UTF-8?q?=EF=B8=8F=20[G-2]=20Remove=20excessive=20calls=20to=20nonReentr?= =?UTF-8?q?ant?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Account.sol | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Account.sol b/src/Account.sol index 6932ae03..e35aad75 100644 --- a/src/Account.sol +++ b/src/Account.sol @@ -238,6 +238,7 @@ contract Account is IAccount, Auth, OpsReady { external payable override + nonReentrant isAccountExecutionEnabled { uint256 numCommands = _commands.length; @@ -257,10 +258,7 @@ contract Account is IAccount, Auth, OpsReady { /// @notice Decodes and executes the given command with the given inputs /// @param _command: The command type to execute /// @param _inputs: The inputs to execute the command with - function _dispatch(Command _command, bytes calldata _inputs) - internal - nonReentrant - { + function _dispatch(Command _command, bytes calldata _inputs) internal { uint256 commandIndex = uint256(_command); if (commandIndex < 4) { @@ -1026,7 +1024,9 @@ contract Account is IAccount, Auth, OpsReady { // transfer sUSD to the UniversalRouter for the swap /// @dev not using SafeERC20 because sUSD is a trusted token - IERC20(tokenIn).transfer(address(UNISWAP_UNIVERSAL_ROUTER), _amountIn); + IERC20(tokenIn).transfer( + address(UNISWAP_UNIVERSAL_ROUTER), _amountIn + ); } else if ( tokenOut == address(MARGIN_ASSET) && SETTINGS.isWhitelistedTokens(tokenIn) From c609655ced2ff6be25f7875f557f06d245328924 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Fri, 21 Jul 2023 15:42:36 -0400 Subject: [PATCH 7/8] =?UTF-8?q?=F0=9F=97=91=EF=B8=8F=20[Q-1]=20Remove=20un?= =?UTF-8?q?used=20event?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Events.sol | 9 --------- src/interfaces/IEvents.sol | 6 ------ test/utils/ConsolidatedEvents.sol | 2 -- 3 files changed, 17 deletions(-) diff --git a/src/Events.sol b/src/Events.sol index 43593ca5..78d79a1e 100644 --- a/src/Events.sol +++ b/src/Events.sol @@ -143,13 +143,4 @@ contract Events is IEvents { priceOracle: priceOracle }); } - - /// @inheritdoc IEvents - function emitExecutorFeeSet(uint256 executorFee) - external - override - onlyAccounts - { - emit ExecutorFeeSet({account: msg.sender, executorFee: executorFee}); - } } diff --git a/src/interfaces/IEvents.sol b/src/interfaces/IEvents.sol index a2c3dbff..ded3db16 100644 --- a/src/interfaces/IEvents.sol +++ b/src/interfaces/IEvents.sol @@ -147,10 +147,4 @@ interface IEvents { uint256 keeperFee, IAccount.PriceOracleUsed priceOracle ); - - /// @notice emitter when executor fee is set by the account owner - /// @param executorFee: executor fee - function emitExecutorFeeSet(uint256 executorFee) external; - - event ExecutorFeeSet(address indexed account, uint256 indexed executorFee); } diff --git a/test/utils/ConsolidatedEvents.sol b/test/utils/ConsolidatedEvents.sol index 45d91147..8846cb75 100644 --- a/test/utils/ConsolidatedEvents.sol +++ b/test/utils/ConsolidatedEvents.sol @@ -85,8 +85,6 @@ contract ConsolidatedEvents { IAccount.PriceOracleUsed priceOracle ); - event ExecutorFeeSet(address indexed account, uint256 indexed executorFee); - /*////////////////////////////////////////////////////////////// ISETTINGS //////////////////////////////////////////////////////////////*/ From 8671a8337ff24042d3efd378bcb141aca9edae09 Mon Sep 17 00:00:00 2001 From: JaredBorders Date: Fri, 21 Jul 2023 15:44:05 -0400 Subject: [PATCH 8/8] =?UTF-8?q?=F0=9F=97=91=EF=B8=8F=20[Q-2]=20Remove=20pa?= =?UTF-8?q?yable=20casting?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Account.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Account.sol b/src/Account.sol index e35aad75..c972d501 100644 --- a/src/Account.sol +++ b/src/Account.sol @@ -907,7 +907,7 @@ contract Account is IAccount, Auth, OpsReady { _transfer({_amount: fee}); } else { fee = SETTINGS.executorFee(); - (bool success,) = payable(msg.sender).call{value: fee}(""); + (bool success,) = msg.sender.call{value: fee}(""); if (!success) revert CannotPayExecutorFee(fee, msg.sender); } }