Merge pull request #184 from Kwenta/hotfix/max-lev-fee

hotfix: add try/catch return to margin pulling call in imposeFee

README.md

@@ -178,12 +178,15 @@ forge test --fork-url $(grep ARCHIVE_NODE_URL_L2 .env | cut -d '=' -f2) --match-
 8. Call `Factory.upgradeAccountImplementation` with new `Account` address (can be done on etherscan)
 > Only factory owner can do this
 9. Update `./deploy-addresses/optimism-goerli.json` with new `Account` address
-10. Ensure testnet accounts are updated and functional (ensure state is correct)
-11. Run script and deploy to Mainnet
-12. Call `Factory.upgradeAccountImplementation` with new `Account` address (can be done on etherscan)
+10. Update `utils/parameters/OptimismGoerliParameters.sol` with new `Account` address
+11. Ensure testnet accounts are updated and functional (ensure state is correct)
+12. Run script and deploy to Mainnet
+13. Call `Factory.upgradeAccountImplementation` with new `Account` address (can be done on etherscan)
 > Only factory owner can do this (pDAO)
-13. Update `./deploy-addresses/optimism.json` with new `Account` address
-14. Ensure mainnet accounts are updated and functional (ensure state is correct)
+14. Update `./deploy-addresses/optimism.json` with new `Account` address
+15. Update `utils/parameters/OptimismParameters.sol` with new `Account` address
+16. Double-check and update any other fields necessary on Parameters constant file. (for example if there is a new deployer address)
+17. Ensure mainnet accounts are updated and functional (ensure state is correct)
 
 ## External Conditional Order Executors
 > As of SM v2.1.0, public actors can execute conditional orders and receive a fee for doing so

deploy-addresses/optimism.json

@@ -1,6 +1,6 @@
 {
  "prod": {
- "Account": "0x0f716Fc517955863824CD9317603E4795EDfffb4",
+ "Account": "0xbada5ec9fa0568e0cd9D252a0744E6b6b52E438C",
  "Events": "0x6B32d15a6Cb77ea227A6Fb19532b2de542c45AC6",
  "Factory": "0x8234F990b149Ae59416dc260305E565e5DAfEb54",
  "Settings": "0xf36003a5dd0B17D51ca1525857dEf220E579447D"

script/upgrades/v2.1.5/Upgrade.s.sol

@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+pragma solidity 0.8.18;
+
+import {Script} from "lib/forge-std/src/Script.sol";
+
+import {IAddressResolver} from "script/utils/interfaces/IAddressResolver.sol";
+
+import {Account} from "src/Account.sol";
+import {Events} from "src/Events.sol";
+import {Settings} from "src/Settings.sol";
+import {IAccount} from "src/interfaces/IAccount.sol";
+
+import {
+ OPTIMISM_GELATO,
+ OPTIMISM_OPS,
+ FUTURES_MARKET_MANAGER,
+ OPTIMISM_FACTORY,
+ OPTIMISM_SYNTHETIX_ADDRESS_RESOLVER,
+ OPTIMISM_UNISWAP_PERMIT2,
+ OPTIMISM_UNISWAP_UNIVERSAL_ROUTER,
+ OPTIMISM_SETTINGS,
+ OPTIMISM_EVENTS,
+ PERPS_V2_EXCHANGE_RATE,
+ PROXY_SUSD,
+ SYSTEM_STATUS,
+ OPTIMISM_PDAO,
+ OPTIMISM_DEPLOYER,
+ OPTIMISM_USDC,
+ OPTIMISM_DAI,
+ OPTIMISM_USDT,
+ OPTIMISM_LUSD
+} from "script/utils/parameters/OptimismParameters.sol";
+
+import {
+ OPTIMISM_SEPOLIA_DEPLOYER,
+ OPTIMISM_SEPOLIA_SYNTHETIX_ADDRESS_RESOLVER,
+ OPTIMISM_SEPOLIA_GELATO,
+ OPTIMISM_SEPOLIA_OPS,
+ OPTIMISM_SEPOLIA_FACTORY,
+ OPTIMISM_SEPOLIA_UNISWAP_UNIVERSAL_ROUTER,
+ OPTIMISM_SEPOLIA_UNISWAP_PERMIT2
+} from "script/utils/parameters/OptimismSepoliaParameters.sol";
+
+/// @title Script to upgrade the Account implementation v2.1.4 -> v2.1.5
+/// @author JaredBorders ([email protected])
+
+/// @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.1.5/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 {
+ uint256 deployerPrivateKey = vm.envUint("DEPLOYER_PRIVATE_KEY");
+ vm.startBroadcast(deployerPrivateKey);
+
+ upgrade();
+
+ vm.stopBroadcast();
+ }
+
+ function upgrade() public returns (address implementation) {
+ IAddressResolver addressResolver =
+ IAddressResolver(OPTIMISM_SYNTHETIX_ADDRESS_RESOLVER);
+
+ address marginAsset = addressResolver.getAddress({name: PROXY_SUSD});
+ address perpsV2ExchangeRate =
+ addressResolver.getAddress({name: PERPS_V2_EXCHANGE_RATE});
+ address futuresMarketManager =
+ addressResolver.getAddress({name: FUTURES_MARKET_MANAGER});
+ address systemStatus = addressResolver.getAddress({name: SYSTEM_STATUS});
+
+ IAccount.AccountConstructorParams memory params = IAccount
+ .AccountConstructorParams({
+ factory: OPTIMISM_FACTORY,
+ events: OPTIMISM_EVENTS,
+ marginAsset: marginAsset,
+ perpsV2ExchangeRate: perpsV2ExchangeRate,
+ futuresMarketManager: futuresMarketManager,
+ systemStatus: systemStatus,
+ gelato: OPTIMISM_GELATO,
+ ops: OPTIMISM_OPS,
+ settings: OPTIMISM_SETTINGS,
+ universalRouter: OPTIMISM_UNISWAP_UNIVERSAL_ROUTER,
+ permit2: OPTIMISM_UNISWAP_PERMIT2
+ });
+
+ implementation = address(new Account(params));
+ }
+}

script/utils/parameters/OptimismParameters.sol

@@ -4,8 +4,8 @@ pragma solidity 0.8.18;
 /// @dev for Synthetix addresses see:
 /// https://github.com/Synthetixio/synthetix-docs/blob/master/content/addresses.md#mainnet-optimism-l2
 
-// v2.1.3 deployer
-address constant OPTIMISM_DEPLOYER = 0x12d970154Ac171293323f20757130d5731850deB;
+// v2.1.5 deployer
+address constant OPTIMISM_DEPLOYER = 0x264bF33f2442001dC6bE0c3FC777df5495b8A5e7;
 
 address constant OPTIMISM_PDAO = 0xe826d43961a87fBE71C91d9B73F7ef9b16721C07;
 
@@ -16,9 +16,9 @@ address constant OPTIMISM_GELATO = 0x01051113D81D7d6DA508462F2ad6d7fD96cF42Ef;
 
 address constant OPTIMISM_OPS = 0x340759c8346A1E6Ed92035FB8B6ec57cE1D82c2c;
 
-// v2.1.4
+// v2.1.5
 address constant OPTIMISM_IMPLEMENTATION =
- 0x0f716Fc517955863824CD9317603E4795EDfffb4;
+ 0xbada5ec9fa0568e0cd9D252a0744E6b6b52E438C;
 
 // released with v2.1.4 implementation (used by v2.1.*)
 address constant OPTIMISM_EVENTS = 0x6B32d15a6Cb77ea227A6Fb19532b2de542c45AC6;

src/Account.sol

@@ -39,7 +39,7 @@ contract Account is IAccount, Auth, OpsReady {
  //////////////////////////////////////////////////////////////*/
 
  /// @inheritdoc IAccount
- bytes32 public constant VERSION = "2.1.4";
+ bytes32 public constant VERSION = "2.1.5";
 
  /// @notice tracking code used when modifying positions
  bytes32 internal constant TRACKING_CODE = "KWENTA";
@@ -617,6 +617,23 @@ contract Account is IAccount, Auth, OpsReady {
  IPerpsV2MarketConsolidated(_market).transferMargin(_amount);
  }
 
+ /// @notice deposit/withdraw margin to/from a Synthetix PerpsV2 Market
+ /// @param _market: address of market
+ /// @param _amount: amount of margin to deposit/withdraw
+ function _perpsV2ModifyMarginNoExternalRevert(
+ address _market,
+ int256 _amount
+ ) internal returns (bool) {
+ if (_amount > 0) {
+ _sufficientMargin(_amount);
+ }
+ try IPerpsV2MarketConsolidated(_market).transferMargin(_amount) {
+ return true;
+ } catch {
+ return false;
+ }
+ }
+
  /// @notice withdraw margin from market back to this account
  /// @dev this will *not* fail if market has zero margin
  function _perpsV2WithdrawAllMargin(address _market) internal {
@@ -657,7 +674,11 @@ contract Account is IAccount, Auth, OpsReady {
 
  /// @dev this will revert if market does not
  /// have sufficient available margin
- _perpsV2ModifyMargin(_market, -int256(difference));
+ bool success = _perpsV2ModifyMarginNoExternalRevert(
+ _market, -int256(difference)
+ );
+ /// @dev breakout of fee impose if market reverts (max leverage scenario)
+ if (!success) return;
  }
 
  // impose fee on account from account margin
@@ -942,7 +963,7 @@ contract Account is IAccount, Auth, OpsReady {
  execAddress: address(this),
  execData: abi.encodeCall(
  this.executeConditionalOrder, conditionalOrderId
-  ),
+ ),
  moduleData: moduleData,
  feeToken: ETH
  });

test/integration/orderFlowFee.behavior.t.sol

@@ -410,6 +410,113 @@ contract OrderFlowFeeTest is Test, ConsolidatedEvents {
  assertEq(treasuryPreBalance, treasuryPostBalance);
  }
 
+ /// Verifies that transaction does not revert and charges a fee if there is sufficient margin to cover for orderFlowFee
+ /// @dev Synthetix makes transaction reverts if the resulting position is too large, outside the max leverage, or is liquidating.
+ function test_imposeOrderFlowFee_on_valid_close() public {
+ uint256 treasuryPreBalance = sUSD.balanceOf(settings.TREASURY());
+
+ /// @dev Ensures first trade doesn't charge a fee
+ uint256 testOrderFlowFee = 0; // 0%
+ settings.setOrderFlowFee(testOrderFlowFee);
+
+ // create a long position in the ETH market
+ address market = getMarketAddressFromKey(sETHPERP);
+
+ /// Deposit all margin so that account has no margin to cover orderFlowFee
+ int256 marginDelta = int256(AMOUNT); // 10_000 dollars
+ int256 sizeDelta = 5 ether; // ~ Estimated notional $9,396.3052993
+ // Leverage is .94x
+
+ (uint256 desiredFillPrice,) =
+ IPerpsV2MarketConsolidated(market).assetPrice();
+
+ submitAtomicOrder(sETHPERP, marginDelta, sizeDelta, desiredFillPrice);
+
+ // Margin after fees charged: ~ $7182.31
+
+ // Set orderflow fee high to easily test that transaction fails if neither account or market has sufficient margin to cover for fees
+ testOrderFlowFee = 10_000; // 10%
+ settings.setOrderFlowFee(testOrderFlowFee);
+
+ // Fee is 90% of notional = $936 fee
+ // Margin would be 7182.31 - 936 = 6246.31
+ // Remaining leverage is 1.5x
+ // Resulting in valid fee charge
+
+ // define close position order
+ IAccount.Command[] memory commands = new IAccount.Command[](1);
+ commands[0] = IAccount.Command.PERPS_V2_CLOSE_POSITION;
+ bytes[] memory inputs = new bytes[](1);
+ desiredFillPrice -= 1 ether;
+ inputs[0] = abi.encode(market, desiredFillPrice);
+
+ // close position
+ account.execute(commands, inputs);
+
+ // Assert position is closed
+ IPerpsV2MarketConsolidated.Position memory position =
+ account.getPosition(sETHPERP);
+ assertEq(0, position.size);
+
+ uint256 treasuryPostBalance = sUSD.balanceOf(settings.TREASURY());
+
+ // Assert treasury balance is now greater than before
+ assertLt(treasuryPreBalance, treasuryPostBalance);
+ }
+
+ /// Verifies that transaction does not revert and does not charge a fee if Synthetix trade reverts (due to max leverage etc..)
+ /// @dev Synthetix makes transaction reverts if the resulting position is too large, outside the max leverage, or is liquidating.
+ function test_imposeOrderFlowFee_market_revert_bypass() public {
+ uint256 treasuryPreBalance = sUSD.balanceOf(settings.TREASURY());
+
+ /// @dev Ensures first trade doesn't charge a fee
+ uint256 testOrderFlowFee = 0; // 0%
+ settings.setOrderFlowFee(testOrderFlowFee);
+
+ // create a long position in the ETH market
+ address market = getMarketAddressFromKey(sETHPERP);
+
+ /// Deposit all margin so that account has no margin to cover orderFlowFee
+ int256 marginDelta = int256(AMOUNT); // 10_000 dollars
+ int256 sizeDelta = 5 ether; // ~ Estimated notional $9,396.3052993
+ // Leverage is .94x
+
+ (uint256 desiredFillPrice,) =
+ IPerpsV2MarketConsolidated(market).assetPrice();
+
+ submitAtomicOrder(sETHPERP, marginDelta, sizeDelta, desiredFillPrice);
+
+ // Margin after fees charged: ~ $7182.31
+
+ // Set orderflow fee high to easily test that transaction fails if neither account or market has sufficient margin to cover for fees
+ testOrderFlowFee = 75_000; // 75%
+ settings.setOrderFlowFee(testOrderFlowFee);
+
+ // Fee is 75% of notional = $7,047 fee
+ // Margin would be $7182.31 - $7,047 = $135.31
+ // Resulting in 69x leverage
+
+ // define close position order
+ IAccount.Command[] memory commands = new IAccount.Command[](1);
+ commands[0] = IAccount.Command.PERPS_V2_CLOSE_POSITION;
+ bytes[] memory inputs = new bytes[](1);
+ desiredFillPrice -= 1 ether;
+ inputs[0] = abi.encode(market, desiredFillPrice);
+
+ // close position
+ account.execute(commands, inputs);
+
+ // Assert position is closed
+ IPerpsV2MarketConsolidated.Position memory position =
+ account.getPosition(sETHPERP);
+ assertEq(0, position.size);
+
+ uint256 treasuryPostBalance = sUSD.balanceOf(settings.TREASURY());
+
+ // Assert that no overflowfee was distributed
+ assertEq(treasuryPreBalance, treasuryPostBalance);
+ }
+
  /// Verifies that the correct Event is emitted with correct fee value
  function test_imposeOrderFlowFee_event() public {
  // create a long position in the ETH market

test/unit/Account.t.sol

@@ -119,7 +119,7 @@ contract AccountTest is Test, ConsolidatedEvents {
  //////////////////////////////////////////////////////////////*/
 
  function test_GetVersion() public view {
- assert(account.VERSION() == "2.1.4");
+ assert(account.VERSION() == "2.1.5");
  }
 
  function test_GetTrackingCode() public view {