Merge pull request #777 from sarvesh-ost/self_distruct_proxy

Destructing staker proxy via ost composer contract

contracts/gateway/OSTComposer.sol

@@ -25,15 +25,14 @@ import "./StakerProxy.sol";
 import "./EIP20GatewayInterface.sol";
 import "../lib/EIP20Interface.sol";
 import "../lib/Mutex.sol";
-import "./ComposerInterface.sol";
 
 /**
  * @title OSTComposer implements Organized contract. Reentrancy is prevented
  *        by using Mutex contract.
  *
  * @notice It facilitates the staker to get the OSTPrime on sidechains.
  */
-contract OSTComposer is Organized, Mutex, ComposerInterface {
+contract OSTComposer is Organized, Mutex {
 
     /* Constants */
 
@@ -365,19 +364,22 @@ contract OSTComposer is Organized, Mutex, ComposerInterface {
     }
 
     /**
-     * @notice It can only be called by StakerProxy contract of the staker. It
-     *         deletes the StakerProxy contract of the staker.
-     *
-     * @param _owner Owner of the StakerProxy contract.
+     * @notice It can only be called by owner of the staker proxy. It
+     *         deletes the StakerProxy contract of the staker and calls self
+     *         destruct on StakerProxy contract.
      */
-    function removeStakerProxy(
-        address _owner
-    )
+    function destructStakerProxy()
         external
-        onlyStakerProxy(_owner)
     {
+
+        StakerProxy stakerProxy = stakerProxies[msg.sender];
+        require(
+            address(stakerProxy) != address(0),
+            "Staker proxy does not exist for the caller."
+        );
         // Resetting the proxy address of the staker.
-        delete stakerProxies[_owner];
+        delete stakerProxies[msg.sender];
+        stakerProxy.selfDestruct();
     }
 
 

contracts/gateway/StakerProxy.sol

@@ -20,7 +20,6 @@ pragma solidity ^0.5.0;
 //
 // ----------------------------------------------------------------------------
 
-import "./ComposerInterface.sol";
 import "./EIP20GatewayInterface.sol";
 import "../lib/EIP20Interface.sol";
 import "../lib/Mutex.sol";
@@ -38,7 +37,7 @@ contract StakerProxy is Mutex {
     /* Storage */
 
     /** The composer that deployed this contract. */
-    ComposerInterface public composer;
+    address public composer;
 
     /** The composer deployed the StakerProxy on behalf of the owner. */
     address payable public owner;
@@ -70,13 +69,12 @@ contract StakerProxy is Mutex {
     /* Special Functions */
 
     /**
-     * @notice Must be constructed by a contract that implements the
-     *         `ComposerInterface`.
+     * @notice Must be constructed by a composer contract.
      *
      * @param _owner The owner that this proxy is deployed for.
      */
     constructor(address payable _owner) public {
-        composer = ComposerInterface(msg.sender);
+        composer = msg.sender;
         owner = _owner;
     }
 
@@ -160,9 +158,7 @@ contract StakerProxy is Mutex {
      *         to transfer all remaining token balance of this contract before
      *         calling this method.
      */
-    function selfDestruct() external onlyOwner {
-        composer.removeStakerProxy(owner);
-
+    function selfDestruct() external onlyComposer {
         selfdestruct(owner);
     }
 

contracts/gateway/ComposerInterface.sol → contracts/test/gateway/MockStakerProxy.sol

@@ -20,13 +20,29 @@ pragma solidity ^0.5.0;
 //
 // ----------------------------------------------------------------------------
 
+/**
+ * @title MockStakerProxy is a contract used for unit testing of ostComposer
+ *        method `destructStakerProxy`.
+ */
+contract MockStakerProxy {
 
-interface ComposerInterface {
+    /* Storage */
+
+    /** Flag to assert if self destruct is called. */
+    bool public selfDestruted;
+
+
+    /* Special Functions */
+
+    constructor () public {
+        selfDestruted = false;
+    }
 
     /**
-     * @notice It deletes the StakerProxy contract for the staker.
-     *
-     * @param _owner Owner of the StakerProxy contract.
+     * @notice Mock method called from ost composer during unit testing of
+     *         `destructStakerProxy`.
      */
-    function removeStakerProxy(address _owner) external;
+    function selfDestruct() external {
+        selfDestruted = true;
+    }
 }

test/gateway/ost_composer/remove_staker_proxy.js → test/gateway/ost_composer/destruct_staker_proxy.js

@@ -20,16 +20,14 @@
 
 const OSTComposer = artifacts.require('TestOSTComposer');
 const MockOrganization = artifacts.require('MockOrganization');
-const StakerProxy = artifacts.require('StakerProxy');
-const BN = require('bn.js');
+const StakerProxy = artifacts.require('MockStakerProxy');
 const Utils = require('../../test_lib/utils');
 
-contract('OSTComposer.removeStakerProxy() ', (accounts) => {
+contract('OSTComposer.destructStakerProxy() ', (accounts) => {
   let organization;
   let ostComposer;
   const stakeRequest = {};
   let stakerProxy;
-  let expectedActiveGatewayCount;
   let owner;
   let worker;
   beforeEach(async () => {
@@ -38,15 +36,13 @@ contract('OSTComposer.removeStakerProxy() ', (accounts) => {
     organization = await MockOrganization.new(owner, worker);
     stakeRequest.staker = accounts[2];
     ostComposer = await OSTComposer.new(organization.address);
-    expectedActiveGatewayCount = new BN(0);
-    stakerProxy = accounts[10];
-    await ostComposer.setStakerProxy(stakeRequest.staker, stakerProxy);
+    stakerProxy = await StakerProxy.new();
+    await ostComposer.setStakerProxy(stakeRequest.staker, stakerProxy.address);
   });
 
   it('should be able to successfully remove staker proxy', async () => {
-    const response = await ostComposer.removeStakerProxy(
-      stakeRequest.staker,
-      { from: stakerProxy },
+    const response = await ostComposer.destructStakerProxy(
+      { from: stakeRequest.staker },
     );
 
     assert.strictEqual(response.receipt.status, true, 'Receipt status is unsuccessful');
@@ -57,27 +53,22 @@ contract('OSTComposer.removeStakerProxy() ', (accounts) => {
       Utils.NULL_ADDRESS,
       'Staker\'s proxy address must be reset to null',
     );
-  });
 
-  it('should fail when non proxy address request removal of staker proxy', async () => {
-    const nonProxy = accounts[8];
-    await Utils.expectRevert(
-      ostComposer.removeStakerProxy(
-        stakeRequest.staker,
-        { from: nonProxy },
-      ),
-      'Caller is invalid proxy address.',
+    const isSelfDestructed = await stakerProxy.selfDestruted.call();
+    assert.strictEqual(
+      isSelfDestructed,
+      true,
+      'Staker proxy self destruct must be called',
     );
   });
 
   it('should fail when owner doesn\'t have any deployed staker proxy', async () => {
-    const nonOwner = accounts[12];
+    const nonProxy = accounts[8];
     await Utils.expectRevert(
-      ostComposer.removeStakerProxy(
-        nonOwner,
-        { from: stakerProxy },
+      ostComposer.destructStakerProxy(
+        { from: nonProxy },
       ),
-      'Caller is invalid proxy address.',
+      'Staker proxy does not exist for the caller.',
     );
   });
 });

test/gateway/staker_proxy/self_destruct.js

@@ -0,0 +1,57 @@
+// Copyright 2019 OpenST Ltd.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+// ----------------------------------------------------------------------------
+//
+// http://www.simpletoken.org/
+//
+// ----------------------------------------------------------------------------
+
+'use strict';
+
+const StakerProxy = artifacts.require('./StakerProxy.sol');
+const Utils = require('../../test_lib/utils');
+
+contract('StakerProxy.selfDestruct()', (accounts) => {
+  const [composer, owner, anyOtherAddress] = accounts;
+  let stakerProxy;
+
+  beforeEach(async () => {
+    stakerProxy = await StakerProxy.new(
+      owner,
+      { from: composer },
+    );
+  });
+
+  it('should allow self destruct by composer ', async () => {
+    const response = await stakerProxy.selfDestruct({ from: composer });
+
+    assert.strictEqual(response.receipt.status, true, 'Receipt status is unsuccessful');
+
+    const stakerProxyCode = await web3.eth.getCode(stakerProxy.address);
+
+    assert.strictEqual(
+      stakerProxyCode,
+      '0x',
+      'Staker proxy contract code should be 0x after self destructing',
+    );
+  });
+
+  it('should not allow self destruct by  non composer address ', async () => {
+    await Utils.expectRevert(
+      stakerProxy.selfDestruct({ from: anyOtherAddress }),
+      'This function can only be called by the composer.',
+    );
+  });
+});