Merge pull request #622 from open-dollar:enter-system-fix

OD-6 enterSystem, quitSystem

src/contracts/proxies/ODProxy.sol

@@ -27,9 +27,23 @@ contract ODProxy {
     _;
   }
 
+  /**
+   * @dev protocol delegatecall function
+   */
   function execute(address _target, bytes memory _data) external payable onlyOwner returns (bytes memory _response) {
     if (_target == address(0)) revert TargetAddressRequired();
 
     _response = _target.functionDelegateCall(_data);
   }
+
+  /**
+   * @dev arbitrary call function
+   * @notice prevents erc20 funds from getting stuck in proxy
+   */
+  function arbitraryExecute(
+    address _target,
+    bytes memory _data
+  ) external payable onlyOwner returns (bytes memory _response) {
+    _response = _target.functionCall(_data);
+  }
 }

src/contracts/proxies/ODSafeManager.sol

@@ -228,8 +228,9 @@ contract ODSafeManager is IODSafeManager, Authorizable, Modifiable {
   }
 
   /// @inheritdoc IODSafeManager
-  function quitSystem(uint256 _safe, address _dst) external safeAllowed(_safe) {
+  function quitSystem(uint256 _safe) external safeAllowed(_safe) {
     SAFEData memory _sData = _safeData[_safe];
+    address _dst = _sData.owner;
     ISAFEEngine.SAFE memory _safeInfo = ISAFEEngine(safeEngine).safes(_sData.collateralType, _sData.safeHandler);
     int256 _deltaCollateral = _safeInfo.lockedCollateral.toInt();
     int256 _deltaDebt = _safeInfo.generatedDebt.toInt();
@@ -240,29 +241,16 @@ contract ODSafeManager is IODSafeManager, Authorizable, Modifiable {
     _updateNfvState(_safe, _deltaCollateral, _deltaDebt);
 
     // Remove safe from owner's list (notice it doesn't erase safe ownership)
-    _usrSafes[_sData.owner].remove(_safe);
-    _usrSafesPerCollat[_sData.owner][_sData.collateralType].remove(_safe);
+    _usrSafes[_dst].remove(_safe);
+    _usrSafesPerCollat[_dst][_sData.collateralType].remove(_safe);
     emit QuitSystem(msg.sender, _safe, _dst);
   }
 
-  /// @inheritdoc IODSafeManager
-  function enterSystem(address _src, uint256 _safe) external safeAllowed(_safe) {
-    SAFEData memory _sData = _safeData[_safe];
-    ISAFEEngine.SAFE memory _safeInfo = ISAFEEngine(safeEngine).safes(_sData.collateralType, _src);
-    int256 _deltaCollateral = _safeInfo.lockedCollateral.toInt();
-    int256 _deltaDebt = _safeInfo.generatedDebt.toInt();
-    ISAFEEngine(safeEngine).transferSAFECollateralAndDebt(
-      _sData.collateralType, _src, _sData.safeHandler, _deltaCollateral, _deltaDebt
-    );
-
-    _updateNfvState(_safe, _deltaCollateral, _deltaDebt);
-    emit EnterSystem(msg.sender, _src, _safe);
-  }
-
   /// @inheritdoc IODSafeManager
   function moveSAFE(uint256 _safeSrc, uint256 _safeDst) external safeAllowed(_safeSrc) safeAllowed(_safeDst) {
     SAFEData memory _srcData = _safeData[_safeSrc];
     SAFEData memory _dstData = _safeData[_safeDst];
+    if (_dstData.safeHandler == address(0)) revert HandlerDoesNotExist();
     if (_srcData.collateralType != _dstData.collateralType) revert CollateralTypesMismatch();
     ISAFEEngine.SAFE memory _safeInfo = ISAFEEngine(safeEngine).safes(_srcData.collateralType, _srcData.safeHandler);
     int256 _deltaCollateral = _safeInfo.lockedCollateral.toInt();

src/contracts/proxies/actions/BasicActions.sol

@@ -238,13 +238,8 @@ contract BasicActions is CommonActions, IBasicActions {
   }
 
   /// @inheritdoc IBasicActions
-  function quitSystem(address _manager, uint256 _safeId, address _dst) external delegateCall {
-    ODSafeManager(_manager).quitSystem(_safeId, _dst);
-  }
-
-  /// @inheritdoc IBasicActions
-  function enterSystem(address _manager, address _src, uint256 _safeId) external delegateCall {
-    ODSafeManager(_manager).enterSystem(_src, _safeId);
+  function quitSystem(address _manager, uint256 _safeId) external delegateCall {
+    ODSafeManager(_manager).quitSystem(_safeId);
   }
 
   /// @inheritdoc IBasicActions

src/contracts/proxies/actions/GlobalSettlementActions.sol

@@ -41,7 +41,7 @@ contract GlobalSettlementActions is CommonActions, IGlobalSettlementActions {
       _safeEngine.approveSAFEModification(_manager);
     }
 
-    __manager.quitSystem(_safeId, address(this));
+    __manager.quitSystem(_safeId);
 
     // free collateral
     __globalSettlement.freeCollateral(_safeData.collateralType);

src/interfaces/proxies/IODSafeManager.sol

@@ -22,8 +22,6 @@ interface IODSafeManager {
   event TransferInternalCoins(address indexed _sender, uint256 indexed _safe, address _dst, uint256 _rad);
   /// @notice Emitted when calling quitSystem with the sender address and the method arguments
   event QuitSystem(address indexed _sender, uint256 indexed _safe, address _dst);
-  /// @notice Emitted when calling enterSystem with the sender address and the method arguments
-  event EnterSystem(address indexed _sender, address _src, uint256 indexed _safe);
   /// @notice Emitted when calling moveSAFE with the sender address and the method arguments
   event MoveSAFE(address indexed _sender, uint256 indexed _safeSrc, uint256 indexed _safeDst);
   /// @notice Emitted when calling protectSAFE with the sender address and the method arguments
@@ -184,18 +182,10 @@ interface IODSafeManager {
   function transferInternalCoins(uint256 _safe, address _dst, uint256 _rad) external;
 
   /**
-   * @notice Quit the system, migrating the safe (lockedCollateral, generatedDebt) to a different dst handler
+   * @notice Quit the system, migrating the safe (lockedCollateral, generatedDebt) to the ODProxy address
    * @param  _safe Id of the SAFE
-   * @param  _dst Address of the dst handler
    */
-  function quitSystem(uint256 _safe, address _dst) external;
-
-  /**
-   * @notice Enter the system, migrating the safe (lockedCollateral, generatedDebt) from a src handler to the safe handler
-   * @param  _src Address of the src handler
-   * @param  _safe Id of the SAFE
-   */
-  function enterSystem(address _src, uint256 _safe) external;
+  function quitSystem(uint256 _safe) external;
 
   /**
    * @notice Move a position from safeSrc handler to the safeDst handler

src/interfaces/proxies/actions/IBasicActions.sol

@@ -52,15 +52,8 @@ interface IBasicActions is ICommonActions {
   /**
    * @notice Quit the system, migrating the safe (lockedCollateral, generatedDebt) to a different dst handler
    * @param  _safe Id of the SAFE
-   * @param  _dst Address of the dst handler
    */
-  function quitSystem(address _manager, uint256 _safe, address _dst) external;
-  /**
-   * @notice Enter the system, migrating the safe (lockedCollateral, generatedDebt) from a src handler to the safe handler
-   * @param  _src Address of the src handler
-   * @param  _safe Id of the SAFE
-   */
-  function enterSystem(address _manager, address _src, uint256 _safe) external;
+  function quitSystem(address _manager, uint256 _safe) external;
   /**
    * @notice Move a position from safeSrc handler to the safeDst handler
    * @param  _safeSrc Id of the source SAFE

test/e2e/E2ENFT.t.sol

@@ -82,14 +82,8 @@ contract NFTSetup is Common {
     ODProxy(_proxy).execute(address(basicActions), payload);
   }
 
-  function quitSystem(address _proxy, uint256 _safeId, address _dst) public {
-    bytes memory payload = abi.encodeWithSelector(basicActions.quitSystem.selector, address(safeManager), _safeId, _dst);
-    ODProxy(_proxy).execute(address(basicActions), payload);
-  }
-
-  function enterSystem(address _proxy, address _src, uint256 _safeId) public {
-    bytes memory payload =
-      abi.encodeWithSelector(basicActions.enterSystem.selector, address(safeManager), _src, _safeId);
+  function quitSystem(address _proxy, uint256 _safeId) public {
+    bytes memory payload = abi.encodeWithSelector(basicActions.quitSystem.selector, address(safeManager), _safeId);
     ODProxy(_proxy).execute(address(basicActions), payload);
   }
 

test/mocks/ActionsMocks.sol

@@ -151,7 +151,7 @@ contract ODSafeManagerMock {
     return safeDataPoint;
   }
 
-  function quitSystem(uint256 _safe, address _dst) external {
+  function quitSystem(uint256 _safe) external {
     wasQuitSystemCalled = true;
   }
 

test/unit/proxies/NFTRenderer.t.sol

@@ -174,15 +174,12 @@ contract Unit_NFTRenderer_RenderParams is Base {
     }
 
     if (bytes(params.collateralJson).length == 0) {
-      // fail('No collateral returned');
       fail();
     }
     if (bytes(params.debtJson).length == 0) {
-      // fail('No debt returned');
       fail();
     }
     if (bytes(params.debtSvg).length == 0) {
-      // fail('No metaDebt returned');
       fail();
     }
   }
@@ -197,7 +194,6 @@ contract Unit_NFTRenderer_RenderParams is Base {
     assertEq(params.ratio, 0, 'incorrect ratio param');
 
     if (bytes(params.collateralJson).length == 0) {
-      // fail('No collateral string returned');
       fail();
     }
   }
@@ -210,7 +206,6 @@ contract Unit_NFTRenderer_RenderBase is Base {
     string memory returnedURI = nftRenderer.render(_data.safeId);
 
     if (bytes(returnedURI).length == 0) {
-      // fail('no URI returned');
       fail();
     }
   }

test/unit/proxies/ODProxy.t.sol

@@ -12,6 +12,10 @@ contract MockProxyTarget {
     return true;
   }
 
+  function payableCall() public payable returns (bool) {
+    return true;
+  }
+
   function callWithArgs(uint256 a, uint256 b) public pure returns (uint256) {
     return a + b;
   }
@@ -71,4 +75,48 @@ contract ODProxyTest is Test {
     vm.expectRevert();
     proxy.execute(address(target), abi.encodeWithSignature('callWillFail()'));
   }
+
+  function testArbitraryExecute() public {
+    vm.startPrank(alice);
+    bytes memory resp = proxy.arbitraryExecute(address(target), abi.encodeWithSignature('call()'));
+    assertTrue(abi.decode(resp, (bool)));
+  }
+
+  // function testArbitraryExecuteWithValue() public {
+  //   vm.startPrank(alice);
+  //   vm.deal(address(proxy), 1 ether);
+  //   bytes memory resp = proxy.arbitraryExecute(address(target), abi.encodeWithSignature('payableCall()'), 1 ether);
+  //   assertTrue(abi.decode(resp, (bool)));
+  // }
+
+  // function testArbitraryExecuteWithValueFail() public {
+  //   vm.startPrank(alice);
+  //   vm.expectRevert('Address: insufficient balance for call');
+  //   proxy.arbitraryExecute(address(target), abi.encodeWithSignature('payableCall()'), 1 ether);
+  // }
+
+  function testArbitraryExecuteWithArgs() public {
+    vm.startPrank(alice);
+    bytes memory resp =
+      proxy.arbitraryExecute(address(target), abi.encodeWithSignature('callWithArgs(uint256,uint256)', 1, 2));
+    assertEq(abi.decode(resp, (uint256)), 3);
+  }
+
+  function testArbitraryExecuteWithNoTarget() public {
+    vm.startPrank(alice);
+    vm.expectRevert('Address: call to non-contract');
+    proxy.arbitraryExecute(address(0), abi.encodeWithSignature('call()'));
+  }
+
+  function testArbitraryExecuteNonOwner() public {
+    vm.startPrank(bob);
+    vm.expectRevert(OnlyOwner.selector);
+    proxy.arbitraryExecute(address(target), abi.encodeWithSignature('call()'));
+  }
+
+  function testArbitraryExecuteWillFail() public {
+    vm.startPrank(alice);
+    vm.expectRevert();
+    proxy.arbitraryExecute(address(target), abi.encodeWithSignature('callWillFail()'));
+  }
 }

test/unit/proxies/ODSafeManager.t.sol

@@ -389,18 +389,6 @@ contract Unit_ODSafeManager_SystemManagement is Base {
     _;
   }
 
-  function test_EnterSystem(Scenario memory _scenario) public happyPath(_scenario) {
-    vm.startPrank(_scenario.aliceProxy);
-    safeManager.allowSAFE(_scenario.safeId, _scenario.aliceProxy, true);
-    vm.mockCall(
-      address(mockSafeEngine), abi.encodeWithSelector(ISAFEEngine.transferSAFECollateralAndDebt.selector), abi.encode()
-    );
-
-    vm.mockCall(address(vault721), abi.encodeWithSelector(IVault721.updateNfvState.selector), abi.encode());
-
-    safeManager.enterSystem(_scenario.aliceProxy, _scenario.safeId);
-  }
-
   function test_QuitSystem(Scenario memory _scenario) public happyPath(_scenario) {
     vm.startPrank(_scenario.aliceProxy);
     safeManager.allowSAFE(_scenario.safeId, _scenario.aliceProxy, true);
@@ -411,7 +399,7 @@ contract Unit_ODSafeManager_SystemManagement is Base {
 
     vm.mockCall(address(vault721), abi.encodeWithSelector(IVault721.updateNfvState.selector), abi.encode());
 
-    safeManager.quitSystem(_scenario.safeId, _scenario.aliceProxy);
+    safeManager.quitSystem(_scenario.safeId);
   }
 }
 

test/unit/proxies/actions/BasicActions.t.sol

@@ -98,24 +98,12 @@ contract BasicActionsTest is ActionBaseTest {
     vm.startPrank(alice);
 
     proxy.execute(
-      address(basicActions),
-      abi.encodeWithSignature('quitSystem(address,uint256,address)', address(safeManager), 1, address(0x01))
+      address(basicActions), abi.encodeWithSignature('quitSystem(address,uint256)', address(safeManager), 1)
     );
 
     assertTrue(safeManager.wasQuitSystemCalled());
   }
 
-  function test_enterSystem() public {
-    vm.startPrank(alice);
-
-    proxy.execute(
-      address(basicActions),
-      abi.encodeWithSignature('enterSystem(address,address,uint256)', address(safeManager), address(0x01), 1)
-    );
-
-    assertTrue(safeManager.wasEnterSystemCalled());
-  }
-
   function test_moveSafe() public {
     vm.startPrank(alice);