Proxying pause, unpause and paused calls (#92)

etoroxlabs · Feb 5, 2019 · 618f7a5 · 618f7a5
1 parent 5b4f59b
commit 618f7a5
Show file tree

Hide file tree

Showing 8 changed files with 88 additions and 36 deletions.
diff --git a/contracts/mocks/ERC20Mock.sol b/contracts/mocks/ERC20Mock.sol
@@ -293,5 +293,4 @@ contract ERC20Mock is ERC20 {
     {
         return super._burnFrom(originSender, account, value);
     }
-
 }
diff --git a/contracts/mocks/RevertingProxy.sol b/contracts/mocks/RevertingProxy.sol
@@ -178,4 +178,18 @@ contract RevertingProxy is IETokenProxy {
         revert("changeMintingRecipient");
     }
 
+    function pauseProxy(address sender) external {
+        sender;
+        revert("pause");
+    }
+
+    function unpauseProxy(address sender) external {
+        sender;
+        revert("unpause");
+    }
+
+    function pausedProxy(address sender) external view returns (bool) {
+        sender;
+        revert("paused");
+    }
 }
diff --git a/contracts/token/EToken.sol b/contracts/token/EToken.sol
@@ -221,8 +221,7 @@ contract EToken is IEToken, ETokenProxy {
      * @dev Proxies call to new token if this token is upgraded
      * @param value The amount of token to be burned.
      */
-    function burn(uint256 value) public
-    {
+    function burn(uint256 value) public {
         if (isUpgraded()) {
             getUpgradedToken().burnProxy(msg.sender, value);
         } else {
@@ -237,8 +236,7 @@ contract EToken is IEToken, ETokenProxy {
      * @param from address The address which you want to send tokens from
      * @param value uint256 The amount of token to be burned
      */
-    function burnFrom(address from, uint256 value) public
-    {
+    function burnFrom(address from, uint256 value) public {
         if (isUpgraded()) {
             getUpgradedToken().burnFromProxy(msg.sender, from, value);
         } else {
@@ -323,25 +321,21 @@ contract EToken is IEToken, ETokenProxy {
 
     /**
      * Allows a pauser to pause the current token.
-     * @dev This function will _not_ be proxied to the new
-     * token if this token is upgraded
      */
     function pause() public {
         if (isUpgraded()) {
-            revert("Token is upgraded. Call pause from new token.");
+            getUpgradedToken().pauseProxy(msg.sender);
         } else {
             pauseGuarded(msg.sender);
         }
     }
 
     /**
      * Allows a pauser to unpause the current token.
-     * @dev This function will _not_ be proxied to the new
-     * token if this token is upgraded
      */
     function unpause() public {
         if (isUpgraded()) {
-            revert("Token is upgraded. Call unpause from new token.");
+            getUpgradedToken().unpauseProxy(msg.sender);
         } else {
             unpauseGuarded(msg.sender);
         }
@@ -352,7 +346,7 @@ contract EToken is IEToken, ETokenProxy {
      */
     function paused() public view returns (bool) {
         if (isUpgraded()) {
-            revert("Token is upgraded. Call paused from new token.");
+            return getUpgradedToken().pausedProxy(msg.sender);
         } else {
             return pausedGuarded(msg.sender);
         }

diff --git a/contracts/token/ETokenProxy.sol b/contracts/token/ETokenProxy.sol
@@ -320,4 +320,33 @@ contract ETokenProxy is IETokenProxy, ETokenGuarded {
         }
     }
 
+    /** Like EToken.pause but proxies calls as described
+        in the documentation for the declaration of this contract. */
+    function pauseProxy(address sender) external {
+        if (isUpgraded()) {
+            getUpgradedToken().pauseProxy(sender);
+        } else {
+            pauseGuarded(sender);
+        }
+    }
+
+    /** Like EToken.unpause but proxies calls as described
+        in the documentation for the declaration of this contract. */
+    function unpauseProxy(address sender) external {
+        if (isUpgraded()) {
+            getUpgradedToken().unpauseProxy(sender);
+        } else {
+            unpauseGuarded(sender);
+        }
+    }
+
+    /** Like EToken.paused but proxies calls as described
+        in the documentation for the declaration of this contract. */
+    function pausedProxy(address sender) external view returns (bool) {
+        if (isUpgraded()) {
+            return getUpgradedToken().pausedProxy(sender);
+        } else {
+            return pausedGuarded(sender);
+        }
+    }
 }
diff --git a/contracts/token/IETokenProxy.sol b/contracts/token/IETokenProxy.sol
@@ -83,5 +83,11 @@ interface IETokenProxy {
         external
         returns (bool success);
 
+    function pauseProxy(address sender) external;
+
+    function unpauseProxy(address sender) external;
+
+    function pausedProxy(address sender) external view returns (bool);
+
     function finalizeUpgrade() external;
 }
diff --git a/contracts/token/access/ETokenGuarded.sol b/contracts/token/access/ETokenGuarded.sol
@@ -348,9 +348,10 @@ contract ETokenGuarded is
     function pauseGuarded(address originSender)
         internal
         isEnabled
-        requireOwner(originSender)
+        requireIsPauser(originSender)
+        whenNotPaused
     {
-        _pause();
+        _pause(originSender);
     }
 
     /**
@@ -361,9 +362,10 @@ contract ETokenGuarded is
     function unpauseGuarded(address originSender)
         internal
         isEnabled
-        requireOwner(originSender)
+        requireIsPauser(originSender)
+        whenPaused
     {
-        _unpause();
+        _unpause(originSender);
     }
 
     /**
@@ -374,11 +376,11 @@ contract ETokenGuarded is
     function pausedGuarded(address originSender)
         internal
         view
+        isEnabled
         returns (bool)
     {
         // Silence warnings
         originSender;
         return _paused();
     }
-
 }
diff --git a/contracts/token/access/Pausable.sol b/contracts/token/access/Pausable.sol
@@ -39,19 +39,34 @@ contract Pausable is PauserRole {
         _;
     }
 
+    /**
+     * @dev Modifier to make a function callable if a specified account is pauser.
+     * @param account the address of the account to check
+     */
+    modifier requireIsPauser(address account) {
+        require(isPauser(account));
+        _;
+    }
+
     /**
      * @dev Called by the owner to pause, triggers stopped state
+     * @param originSender the original sender of this method
      */
-    function _pause() internal onlyPauser whenNotPaused {
+    function _pause(address originSender)
+        internal
+    {
         paused_ = true;
-        emit Paused(msg.sender);
+        emit Paused(originSender);
     }
 
     /**
      * @dev Called by the owner to unpause, returns to normal state
+     * @param originSender the original sender of this method
      */
-    function _unpause() internal onlyPauser whenPaused {
+    function _unpause(address originSender)
+        internal
+    {
         paused_ = false;
-        emit Unpaused(msg.sender);
+        emit Unpaused(originSender);
     }
 }
diff --git a/test/token/EToken.test.js b/test/token/EToken.test.js
@@ -55,7 +55,8 @@ const otherOps = [
   ['mint', [util.ZERO_ADDRESS, 0]],
   ['changeMintingRecipient', [util.ZERO_ADDRESS]],
   ['pause', []],
-  ['unpause', []]
+  ['unpause', []],
+  ['paused', []]
 ];
 
 function unupgradedTokenBehavior () {
@@ -80,18 +81,6 @@ function proxyTokenBehavior () {
   });
 }
 
-function proxyPausableBehavior () {
-  ['pause', 'unpause', 'paused'].forEach(function (f) {
-    describe(f, function () {
-      it('reverts when token is upgraded', async function () {
-        await util.assertRevertsReason(
-          this.token[f](),
-          `Token is upgraded. Call ${f} from new token.`);
-      });
-    });
-  });
-}
-
 function ERC20Permissions (owner, whitelisted, other, other1, blacklisted, blacklisted1,
                            blackwhite, blackwhite1) {
   describe('token permissions', function () {
@@ -405,7 +394,6 @@ contract('EToken', async function (
         shouldBehaveLikeERC20PublicAPI(owner, whitelisted, whitelisted1);
         shouldBehaveLikeERC20Mintable(owner, minter, [user], mintingRecipientAccount);
         shouldBehaveLikeERC20Burnable(owner, 100, [burner]);
-        proxyPausableBehavior();
         ERC20Permissions(owner, whitelisted, user, user1,
                          blacklisted, blacklisted1,
                          blackwhite, blackwhite1);
@@ -501,15 +489,20 @@ contract('EToken', async function (
       ['burnFrom', [owner, 1], minter],
       ['increaseAllowance', [minter, 1]],
       ['decreaseAllowance', [minter, 1]],
-      ['changeMintingRecipient', [minter]]
+      ['changeMintingRecipient', [minter]],
+      ['pause', []],
+      ['unpause', [], undefined, (t) => t.pause()], // Pause before testing unpause
+      ['paused', []]
     ];
 
     upgradeOps.forEach(function (u) {
       const name = u[0];
       const params = u[1];
       const from = u[2] === undefined ? owner : u[2];
+      const preFn = u[3];
 
       it(`should proxy ${name} method`, async function () {
+        preFn && await preFn(this.token);
         await this.token[name](...params, { from });
         await this.upgrade();
         await util.assertRevertsReason(this.token[name](...params, { from }), name);