Merge branch 'master' into feature/compoundPicer

contracts/Controller.sol

@@ -147,9 +147,8 @@ contract Controller is ReentrancyGuard, Ownable {
         // if there is a short option and it has expired
         address calculatorModule = AddressBookInterface(addressBook).getMarginCalculator();
         MarginCalculatorInterface calculator = MarginCalculatorInterface(calculatorModule);
-        OtokenInterface otoken = OtokenInterface(vault.shortOtokens[0]);
 
-        (uint256 netValue, ) = calculator.getExcessMargin(vault, otoken.collateralAsset());
+        (uint256 netValue, ) = calculator.getExcessCollateral(vault);
         vault.collateralAmounts[0] = netValue;
         return vault;
     }
@@ -217,25 +216,25 @@ contract Controller is ReentrancyGuard, Ownable {
             Actions.ActionArgs memory action = _actions[i];
             Actions.ActionType actionType = action.actionType;
 
-            if (actionType == Actions.ActionType.OpenVault) {
+            if (
+                (actionType != Actions.ActionType.SettleVault) ||
+                (actionType != Actions.ActionType.Exercise) ||
+                (actionType != Actions.ActionType.Call)
+            ) {
                 // check if this action is manipulating the same vault as all other actions, other than SettleVault
                 (prevActionVaultId, isActionVaultStored) = _checkActionsVaults(
                     prevActionVaultId,
                     action.vaultId,
                     isActionVaultStored
                 );
-
-                _openVault(Actions._parseOpenVaultArgs(action));
             }
-            if (actionType == Actions.ActionType.DepositLongOption) {
-                // check if this action is manipulating the same vault as all other actions, other than SettleVault
-                (prevActionVaultId, isActionVaultStored) = _checkActionsVaults(
-                    prevActionVaultId,
-                    action.vaultId,
-                    isActionVaultStored
-                );
 
+            if (actionType == Actions.ActionType.OpenVault) {
+                _openVault(Actions._parseOpenVaultArgs(action));
+            } else if (actionType == Actions.ActionType.DepositLongOption) {
                 vault = _depositLong(Actions._parseDepositArgs(action));
+            } else if (actionType == Actions.ActionType.WithdrawLongOption) {
+                vault = _withdrawLong(Actions._parseWithdrawArgs(action));
             }
         }
 
@@ -247,12 +246,12 @@ contract Controller is ReentrancyGuard, Ownable {
      * @param _vault final vault state
      */
     function _verifyFinalState(MarginAccount.Vault memory _vault) internal view {
-        if (_vault.shortOtokens.length > 0) {
-            address calculatorModule = AddressBookInterface(addressBook).getMarginCalculator();
-            MarginCalculatorInterface calculator = MarginCalculatorInterface(calculatorModule);
+        address calculatorModule = AddressBookInterface(addressBook).getMarginCalculator();
+        MarginCalculatorInterface calculator = MarginCalculatorInterface(calculatorModule);
 
-            require(calculator.isValidState(_vault, _vault.shortOtokens[0]), "Controller: invalid final vault state");
-        }
+        (, bool isValidVault) = calculator.getExcessCollateral(_vault);
+
+        require(isValidVault, "Controller: invalid final vault state");
     }
 
     /**
@@ -293,6 +292,7 @@ contract Controller is ReentrancyGuard, Ownable {
      * @param _args DepositArgs structure
      */
     function _depositLong(Actions.DepositArgs memory _args) internal returns (MarginAccount.Vault memory) {
+        require(checkVaultId(_args.owner, _args.vaultId), "Controller: invalid vault id");
         require(_args.from == msg.sender, "Controller: depositor address and msg.sender address mismatch");
 
         address whitelistModule = AddressBookInterface(addressBook).getWhitelist();
@@ -322,7 +322,26 @@ contract Controller is ReentrancyGuard, Ownable {
      * @dev Only account owner or operator can withdraw long option from vault
      * @param _args WithdrawArgs structure
      */
-    // function _withdrawLong(Actions.WithdrawArgs memory _args) internal isAuthorized(_args.owner) {}
+    function _withdrawLong(Actions.WithdrawArgs memory _args)
+        internal
+        isAuthorized(msg.sender, _args.owner)
+        returns (MarginAccount.Vault memory)
+    {
+        require(checkVaultId(_args.owner, _args.vaultId), "Controller: invalid vault id");
+
+        OtokenInterface otoken = OtokenInterface(_args.asset);
+
+        require(now <= otoken.expiryTimestamp(), "Controller: can not withdraw an expired otoken");
+
+        vaults[_args.owner][_args.vaultId]._removeLong(address(otoken), _args.amount, _args.index);
+
+        address marginPoolModule = AddressBookInterface(addressBook).getMarginPool();
+        MarginPoolInterface marginPool = MarginPoolInterface(marginPoolModule);
+
+        marginPool.transferToUser(address(otoken), _args.to, _args.amount);
+
+        return vaults[_args.owner][_args.vaultId];
+    }
 
     /**
      * @notice deposit collateral asset into vault
@@ -368,4 +387,13 @@ contract Controller is ReentrancyGuard, Ownable {
     //    //Check whitelistModule.isWhitelistCallDestination(args.address)
     //    //Call args.address with args.data
     //}
+
+    /**
+     * @notice function to check the validity of a specific vault id
+     * @param _accountOwner account owner address
+     * @param _vaultId vault id
+     */
+    function checkVaultId(address _accountOwner, uint256 _vaultId) internal view returns (bool) {
+        return ((_vaultId > 0) && (_vaultId <= accountVaultCounter[_accountOwner]));
+    }
 }

contracts/MarginCalculator.sol

@@ -56,7 +56,7 @@ contract MarginCalculator is Initializable {
      * @return netValue the amount by which the margin is above or below the required amount.
      * @return isExcess true if there's excess margin in the vault. In this case, collateral can be taken out from the vault. False if there is insufficient margin and additional collateral needs to be added to the vault to create the position.
      */
-    function getExcessMargin(MarginAccount.Vault memory _vault) public view returns (uint256, bool) {
+    function getExcessCollateral(MarginAccount.Vault memory _vault) public view returns (uint256, bool) {
         // ensure the number of collateral, long and short array is valid.
         _checkIsValidSpread(_vault);
         // ensure the long asset is valid for the short asset.

contracts/MarginPool.sol

@@ -57,7 +57,7 @@ contract MarginPool {
         address _user,
         uint256 _amount
     ) public onlyController returns (bool) {
-        require(_amount > 0, "MarginPool: transferToPool amount is below or equal to 0");
+        require(_amount > 0, "MarginPool: transferToPool amount is equal to 0");
 
         // transfer val from _user to pool
         return ERC20Interface(_asset).transferFrom(_user, address(this), _amount);
@@ -77,7 +77,7 @@ contract MarginPool {
         address payable _user,
         uint256 _amount
     ) public onlyController returns (bool) {
-        require(_amount > 0, "MarginPool: transferToUser amount is below or equal to 0");
+        require(_amount > 0, "MarginPool: transferToUser amount is equal to 0");
 
         // transfer asset val from Pool to _user
         return ERC20Interface(_asset).transfer(_user, _amount);

contracts/interfaces/MarginCalculatorInterface.sol

@@ -8,10 +8,8 @@ import {MarginAccount} from "../libs/MarginAccount.sol";
 interface MarginCalculatorInterface {
     function getExpiredCashValue(address _otoken) external view returns (uint256);
 
-    function getExcessMargin(MarginAccount.Vault memory _vault, address _denominated)
+    function getExcessCollateral(MarginAccount.Vault memory _vault)
         external
         view
         returns (uint256 netValue, bool isExcess);
-
-    function isValidState(MarginAccount.Vault memory _vault, address _denominated) external view returns (bool isValid);
 }

contracts/mocks/MockMarginCalculator.sol

@@ -5,15 +5,77 @@ pragma solidity =0.6.10;
 
 pragma experimental ABIEncoderV2;
 
+import {OtokenInterface} from "../interfaces/OtokenInterface.sol";
 import {MarginAccount} from "../libs/MarginAccount.sol";
+import {FixedPointInt256} from "../libs/FixedPointInt256.sol";
+import {SignedConverter} from "../libs/SignedConverter.sol";
 
 contract MockMarginCalculator {
     // solhint-disable-ignore-no-unused-vars
-    function isValidState(MarginAccount.Vault memory finalVault, address shortOtoken)
-        external
+    function getExcessCollateral(MarginAccount.Vault memory _vault)
+        public
         view
-        returns (bool isValid)
+        returns (uint256 netValue, bool isExcess)
     {
-        return true;
+        // ensure the number of collateral, long and short array is valid.
+        _checkIsValidSpread(_vault);
+
+        // ensure the long asset is valid for the short asset.
+        _checkIsMarginableLong(_vault);
+
+        // collateral amount is always positive.
+        FixedPointInt256.FixedPointInt memory collateralAmount = _vault.collateralAmounts.length > 0
+            ? _uint256ToFixedPointInt(_vault.collateralAmounts[0])
+            : _uint256ToFixedPointInt(0);
+
+        // Vault contains no short tokens: return collateral value.
+        if ((_vault.shortOtokens.length == 0) || (_vault.shortOtokens[0] == address(0)))
+            return (SignedConverter.intToUint(collateralAmount.value), true);
+
+        return (0, true);
+    }
+
+    function _checkIsValidSpread(MarginAccount.Vault memory _vault) internal pure {
+        require(_vault.shortOtokens.length <= 1, "MarginCalculator: Too many short otokens in the vault.");
+        require(_vault.longOtokens.length <= 1, "MarginCalculator: Too many long otokens in the vault.");
+        require(_vault.collateralAssets.length <= 1, "MarginCalculator: Too many collateral assets in the vault.");
+
+        require(
+            _vault.shortOtokens.length == _vault.shortAmounts.length,
+            "MarginCalculator: Short asset and amount mismatch."
+        );
+        require(
+            _vault.longOtokens.length == _vault.longAmounts.length,
+            "MarginCalculator: Long asset and amount mismatch."
+        );
+        require(
+            _vault.collateralAssets.length == _vault.collateralAmounts.length,
+            "MarginCalculator: Collateral asset and amount mismatch."
+        );
+    }
+
+    function _checkIsMarginableLong(MarginAccount.Vault memory _vault) internal view {
+        if (_vault.longOtokens.length == 0 || _vault.shortOtokens.length == 0) return;
+
+        OtokenInterface long = OtokenInterface(_vault.longOtokens[0]);
+        OtokenInterface short = OtokenInterface(_vault.shortOtokens[0]);
+
+        require(
+            long.underlyingAsset() == short.underlyingAsset(),
+            "MarginCalculator: Long and short underlying asset mismatch"
+        );
+        require(long.strikeAsset() == short.strikeAsset(), "MarginCalculator: Long and short strike asset mismatch");
+        require(
+            long.collateralAsset() == short.collateralAsset(),
+            "MarginCalculator: Long and short collateral asset mismatch"
+        );
+        require(
+            long.expiryTimestamp() == short.expiryTimestamp(),
+            "MarginCalculator: Long and short expiry timestamp mismatch"
+        );
+    }
+
+    function _uint256ToFixedPointInt(uint256 _num) internal pure returns (FixedPointInt256.FixedPointInt memory) {
+        return FixedPointInt256.FixedPointInt(SignedConverter.uintToInt(_num));
     }
 }