From e25557f3a69eba36668b460280085cd266ba77f8 Mon Sep 17 00:00:00 2001 From: dsshap Date: Tue, 5 Mar 2024 13:55:26 -0500 Subject: [PATCH 1/2] moving functions around --- src/settled-cash/CrossMarginCashEngine.sol | 213 +++++++++--------- .../CrossMarginPhysicalEngine.sol | 160 ++++++------- 2 files changed, 193 insertions(+), 180 deletions(-) diff --git a/src/settled-cash/CrossMarginCashEngine.sol b/src/settled-cash/CrossMarginCashEngine.sol index 000a70c..963c734 100644 --- a/src/settled-cash/CrossMarginCashEngine.sol +++ b/src/settled-cash/CrossMarginCashEngine.sol @@ -148,51 +148,60 @@ contract CrossMarginCashEngine is } /*/////////////////////////////////////////////////////////////// - Override Upgrade Permission + Execute and BatchExecute //////////////////////////////////////////////////////////////*/ /** - * @dev Upgradable by the owner. - * + * @notice gets access status of an address + * @dev if whitelist address is not set, it ignores this + * @param _address address */ - function _authorizeUpgrade(address /*newImplementation*/ ) internal view override { - _checkOwner(); + function _checkPermissioned(address _address) internal view { + if (!authority.canCall(_address, address(this), msg.sig)) revert NoAccess(); } - /*/////////////////////////////////////////////////////////////// - External Functions - //////////////////////////////////////////////////////////////*/ - /** - * @notice sets the Collateralizable Mask for a pair of assets - * @param _asset0 the address of the asset 0 - * @param _asset1 the address of the asset 1 - * @param _value is margin-able + * @notice execute multiple actions on one subAccounts + * @dev also check access of msg.sender */ - function setCollateralizable(address _asset0, address _asset1, bool _value) external { - _checkOwner(); - - uint256 collateralId = grappa.assetIds(_asset0); - uint256 mask = 1 << grappa.assetIds(_asset1); - - if (_value) collateralizable[collateralId] |= mask; - else collateralizable[collateralId] &= ~mask; - - emit CollateralizableSet(_asset0, _asset1, _value); - } + function _execute(address _subAccount, ActionArgs[] calldata actions) internal { + _assertCallerHasAccess(_subAccount); - /** - * @dev check if a pair of assets are collateralizable - */ - function isCollateralizable(address _asset0, address _asset1) external view returns (bool) { - return _isCollateralizable(grappa.assetIds(_asset0), grappa.assetIds(_asset1)); - } + // update the account storage and do external calls on the flight + for (uint256 i; i < actions.length;) { + if (actions[i].action == ActionType.AddCollateral) { + _addCollateral(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.RemoveCollateral) { + _removeCollateral(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.MintShort) { + _mintOption(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.MintShortIntoAccount) { + _mintOptionIntoAccount(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.BurnShort) { + _burnOption(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.BurnShortInAccount) { + _burnOptionFromAccount(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.TransferLong) { + _transferLong(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.TransferShort) { + _transferShort(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.TransferCollateral) { + _transferCollateral(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.AddLong) { + _addOption(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.RemoveLong) { + _removeOption(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.SettleAccount) { + _settle(_subAccount); + } else { + revert CM_UnsupportedAction(); + } - /** - * @dev check if a pair of assets are collateralizable - */ - function isCollateralizable(uint8 _asset0, uint8 _asset1) external view returns (bool) { - return _isCollateralizable(_asset0, _asset1); + // increase i without checking overflow + unchecked { + ++i; + } + } } /** @@ -262,21 +271,6 @@ contract CrossMarginCashEngine is return _getMinCollateral(account); } - /** - * @notice move an account to someone else - * @dev expected to be call by account owner - * @param _subAccount the id of subaccount to transfer - * @param _newSubAccount the id of receiving account - */ - function transferAccount(address _subAccount, address _newSubAccount) external { - if (!_isPrimaryAccountFor(msg.sender, _subAccount)) revert NoAccess(); - - if (!accounts[_newSubAccount].isEmpty()) revert CM_AccountIsNotEmpty(); - accounts[_newSubAccount] = accounts[_subAccount]; - - delete accounts[_subAccount]; - } - /** * @dev view function to get all shorts, longs and collaterals */ @@ -563,58 +557,6 @@ contract CrossMarginCashEngine is * ========================================================= * */ - /** - * @notice gets access status of an address - * @dev if whitelist address is not set, it ignores this - * @param _address address - */ - function _checkPermissioned(address _address) internal view { - if (!authority.canCall(_address, address(this), msg.sig)) revert NoAccess(); - } - - /** - * @notice execute multiple actions on one subAccounts - * @dev also check access of msg.sender - */ - function _execute(address _subAccount, ActionArgs[] calldata actions) internal { - _assertCallerHasAccess(_subAccount); - - // update the account storage and do external calls on the flight - for (uint256 i; i < actions.length;) { - if (actions[i].action == ActionType.AddCollateral) { - _addCollateral(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.RemoveCollateral) { - _removeCollateral(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.MintShort) { - _mintOption(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.MintShortIntoAccount) { - _mintOptionIntoAccount(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.BurnShort) { - _burnOption(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.BurnShortInAccount) { - _burnOptionFromAccount(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.TransferLong) { - _transferLong(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.TransferShort) { - _transferShort(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.TransferCollateral) { - _transferCollateral(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.AddLong) { - _addOption(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.RemoveLong) { - _removeOption(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.SettleAccount) { - _settle(_subAccount); - } else { - revert CM_UnsupportedAction(); - } - - // increase i without checking overflow - unchecked { - ++i; - } - } - } /** * @dev get minimum collateral requirement for an account @@ -644,4 +586,71 @@ contract CrossMarginCashEngine is ) ); } + + /*/////////////////////////////////////////////////////////////// + Collateralizable Functions + //////////////////////////////////////////////////////////////*/ + + /** + * @notice sets the Collateralizable Mask for a pair of assets + * @param _asset0 the address of the asset 0 + * @param _asset1 the address of the asset 1 + * @param _value is margin-able + */ + function setCollateralizable(address _asset0, address _asset1, bool _value) external { + _checkOwner(); + + uint256 collateralId = grappa.assetIds(_asset0); + uint256 mask = 1 << grappa.assetIds(_asset1); + + if (_value) collateralizable[collateralId] |= mask; + else collateralizable[collateralId] &= ~mask; + + emit CollateralizableSet(_asset0, _asset1, _value); + } + + /** + * @dev check if a pair of assets are collateralizable + */ + function isCollateralizable(address _asset0, address _asset1) external view returns (bool) { + return _isCollateralizable(grappa.assetIds(_asset0), grappa.assetIds(_asset1)); + } + + /** + * @dev check if a pair of assets are collateralizable + */ + function isCollateralizable(uint8 _asset0, uint8 _asset1) external view returns (bool) { + return _isCollateralizable(_asset0, _asset1); + } + + /*/////////////////////////////////////////////////////////////// + Transfer Account Functions + //////////////////////////////////////////////////////////////*/ + + /** + * @notice move an account to someone else + * @dev expected to be call by account owner + * @param _subAccount the id of subaccount to transfer + * @param _newSubAccount the id of receiving account + */ + function transferAccount(address _subAccount, address _newSubAccount) external { + if (!_isPrimaryAccountFor(msg.sender, _subAccount)) revert NoAccess(); + + if (!accounts[_newSubAccount].isEmpty()) revert CM_AccountIsNotEmpty(); + accounts[_newSubAccount] = accounts[_subAccount]; + + delete accounts[_subAccount]; + } + + /*/////////////////////////////////////////////////////////////// + Override Upgrade Permission + //////////////////////////////////////////////////////////////*/ + + /** + * @dev Upgradable by the owner. + * + */ + function _authorizeUpgrade(address /*newImplementation*/ ) internal view override { + _checkOwner(); + } } diff --git a/src/settled-physical/CrossMarginPhysicalEngine.sol b/src/settled-physical/CrossMarginPhysicalEngine.sol index c6e87d1..58bb7f6 100644 --- a/src/settled-physical/CrossMarginPhysicalEngine.sol +++ b/src/settled-physical/CrossMarginPhysicalEngine.sol @@ -129,20 +129,63 @@ contract CrossMarginPhysicalEngine is } /*/////////////////////////////////////////////////////////////// - Override Upgrade Permission + Execute and BatchExecute //////////////////////////////////////////////////////////////*/ /** - * @dev Upgradable by the owner. - * + * @notice gets access status of an address + * @dev if whitelist address is not set, it ignores this + * @param _address address */ - function _authorizeUpgrade(address /*newImplementation*/ ) internal view override { - _checkOwner(); + function _checkPermissioned(address _address) internal view { + if (!authority.canCall(_address, address(this), msg.sig)) revert NoAccess(); } - /*/////////////////////////////////////////////////////////////// - External Functions - //////////////////////////////////////////////////////////////*/ + /** + * @notice execute multiple actions on one subAccounts + * @dev also check access of msg.sender + */ + function _execute(address _subAccount, ActionArgs[] calldata actions) internal { + _assertCallerHasAccess(_subAccount); + + // update the account storage and do external calls on the flight + for (uint256 i; i < actions.length;) { + if (actions[i].action == ActionType.AddCollateral) { + _addCollateral(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.RemoveCollateral) { + _removeCollateral(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.MintShort) { + _mintOption(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.MintShortIntoAccount) { + _mintOptionIntoAccount(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.BurnShort) { + _burnOption(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.BurnShortInAccount) { + _burnOptionFromAccount(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.TransferLong) { + _transferLong(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.TransferShort) { + _transferShort(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.TransferCollateral) { + _transferCollateral(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.AddLong) { + _addOption(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.RemoveLong) { + _removeOption(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.ExerciseToken) { + _exerciseToken(_subAccount, actions[i].data); + } else if (actions[i].action == ActionType.SettleAccount) { + _settle(_subAccount); + } else { + revert CM_UnsupportedAction(); + } + + // increase i without checking overflow + unchecked { + ++i; + } + } + } /** * @notice batch execute on multiple subAccounts @@ -269,21 +312,6 @@ contract CrossMarginPhysicalEngine is return _getMinCollateral(account); } - /** - * @notice move an account to someone else - * @dev expected to be call by account owner - * @param _subAccount the id of subaccount to transfer - * @param _newSubAccount the id of receiving account - */ - function transferAccount(address _subAccount, address _newSubAccount) external { - if (!_isPrimaryAccountFor(msg.sender, _subAccount)) revert NoAccess(); - - if (!accounts[_newSubAccount].isEmpty()) revert CM_AccountIsNotEmpty(); - accounts[_newSubAccount] = accounts[_subAccount]; - - delete accounts[_subAccount]; - } - /** * @dev view function to get all shorts, longs and collaterals */ @@ -634,61 +662,6 @@ contract CrossMarginPhysicalEngine is * ========================================================= * */ - /** - * @notice gets access status of an address - * @dev if whitelist address is not set, it ignores this - * @param _address address - */ - function _checkPermissioned(address _address) internal view { - if (!authority.canCall(_address, address(this), msg.sig)) revert NoAccess(); - } - - /** - * @notice execute multiple actions on one subAccounts - * @dev also check access of msg.sender - */ - function _execute(address _subAccount, ActionArgs[] calldata actions) internal { - _assertCallerHasAccess(_subAccount); - - // update the account storage and do external calls on the flight - for (uint256 i; i < actions.length;) { - if (actions[i].action == ActionType.AddCollateral) { - _addCollateral(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.RemoveCollateral) { - _removeCollateral(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.MintShort) { - _mintOption(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.MintShortIntoAccount) { - _mintOptionIntoAccount(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.BurnShort) { - _burnOption(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.BurnShortInAccount) { - _burnOptionFromAccount(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.TransferLong) { - _transferLong(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.TransferShort) { - _transferShort(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.TransferCollateral) { - _transferCollateral(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.AddLong) { - _addOption(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.RemoveLong) { - _removeOption(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.ExerciseToken) { - _exerciseToken(_subAccount, actions[i].data); - } else if (actions[i].action == ActionType.SettleAccount) { - _settle(_subAccount); - } else { - revert CM_UnsupportedAction(); - } - - // increase i without checking overflow - unchecked { - ++i; - } - } - } - /** * @dev get minimum collateral requirement for an account */ @@ -754,4 +727,35 @@ contract CrossMarginPhysicalEngine is ) ); } + + /*/////////////////////////////////////////////////////////////// + Transfer Account Functions + //////////////////////////////////////////////////////////////*/ + + /** + * @notice move an account to someone else + * @dev expected to be call by account owner + * @param _subAccount the id of subaccount to transfer + * @param _newSubAccount the id of receiving account + */ + function transferAccount(address _subAccount, address _newSubAccount) external { + if (!_isPrimaryAccountFor(msg.sender, _subAccount)) revert NoAccess(); + + if (!accounts[_newSubAccount].isEmpty()) revert CM_AccountIsNotEmpty(); + accounts[_newSubAccount] = accounts[_subAccount]; + + delete accounts[_subAccount]; + } + + /*/////////////////////////////////////////////////////////////// + Override Upgrade Permission + //////////////////////////////////////////////////////////////*/ + + /** + * @dev Upgradable by the owner. + * + */ + function _authorizeUpgrade(address /*newImplementation*/ ) internal view override { + _checkOwner(); + } } From 8da767d91bebfbfb4c4062b2c2e1db5c22ffe737 Mon Sep 17 00:00:00 2001 From: dsshap Date: Tue, 5 Mar 2024 13:56:05 -0500 Subject: [PATCH 2/2] fmt and snapshot --- src/settled-cash/CrossMarginCashEngine.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/src/settled-cash/CrossMarginCashEngine.sol b/src/settled-cash/CrossMarginCashEngine.sol index 963c734..9de3e2e 100644 --- a/src/settled-cash/CrossMarginCashEngine.sol +++ b/src/settled-cash/CrossMarginCashEngine.sol @@ -557,7 +557,6 @@ contract CrossMarginCashEngine is * ========================================================= * */ - /** * @dev get minimum collateral requirement for an account */