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

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));
+ }
+}

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 {