Fixed missing onlyProxy requirements for proxied pause, unpause and p…

…aused (#95) Also, added tests to catch the same error in the future.
etoroxlabs · Feb 9, 2019 · c6fb4c7 · c6fb4c7
1 parent 49fbdc4
commit c6fb4c7
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 5 deletions.
diff --git a/contracts/token/ETokenProxy.sol b/contracts/token/ETokenProxy.sol
@@ -322,7 +322,11 @@ 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 {
+    function pauseProxy(address sender)
+        external
+        isEnabled
+        onlyProxy
+    {
         if (isUpgraded()) {
             getUpgradedToken().pauseProxy(sender);
         } else {
@@ -332,7 +336,11 @@ contract ETokenProxy is IETokenProxy, ETokenGuarded {
 
     /** Like EToken.unpause but proxies calls as described
         in the documentation for the declaration of this contract. */
-    function unpauseProxy(address sender) external {
+    function unpauseProxy(address sender)
+        external
+        isEnabled
+        onlyProxy
+    {
         if (isUpgraded()) {
             getUpgradedToken().unpauseProxy(sender);
         } else {
@@ -342,7 +350,13 @@ contract ETokenProxy is IETokenProxy, ETokenGuarded {
 
     /** 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) {
+    function pausedProxy(address sender)
+        external
+        view
+        isEnabled
+        onlyProxy
+        returns (bool)
+    {
         if (isUpgraded()) {
             return getUpgradedToken().pausedProxy(sender);
         } else {

diff --git a/test/token/EToken.test.js b/test/token/EToken.test.js
@@ -1,4 +1,4 @@
-/* global artifacts, web3, contract */
+/* global artifacts, web3, contract, assert */
 /* eslint-env mocha */
 
 'use strict';
@@ -39,9 +39,13 @@ const explicitSenderOps = [
   ['transferFromProxy', [util.ZERO_ADDRESS, util.ZERO_ADDRESS, util.ZERO_ADDRESS, 0]],
   ['increaseAllowanceProxy', [util.ZERO_ADDRESS, util.ZERO_ADDRESS, 0]],
   ['decreaseAllowanceProxy', [util.ZERO_ADDRESS, util.ZERO_ADDRESS, 0]],
+  ['mintProxy', [util.ZERO_ADDRESS, util.ZERO_ADDRESS, 0]],
   ['burnProxy', [util.ZERO_ADDRESS, 0]],
   ['burnFromProxy', [util.ZERO_ADDRESS, util.ZERO_ADDRESS, 0]],
-  ['changeMintingRecipientProxy', [util.ZERO_ADDRESS, util.ZERO_ADDRESS]]
+  ['changeMintingRecipientProxy', [util.ZERO_ADDRESS, util.ZERO_ADDRESS]],
+  ['pauseProxy', [util.ZERO_ADDRESS]],
+  ['unpauseProxy', [util.ZERO_ADDRESS]],
+  ['pausedProxy', [util.ZERO_ADDRESS]]
 ];
 
 const otherOps = [
@@ -372,6 +376,22 @@ contract('EToken', async function (
         identifiesAsNewToken();
 
         describe('Upgraded token rejects unauthorized for proxy sender functions', function () {
+          it('should test for all the proxy methods', async function () {
+            const methodsToTest = explicitSenderOps.map(o => o[0]).sort();
+            const proxyMethods = Object.entries(this.token)
+              .filter(
+                ([key, value]) => key.endsWith('Proxy') &&
+                      typeof value === 'function'
+              )
+              .map(o => o[0])
+              .sort();
+
+            assert.deepEqual(
+              methodsToTest,
+              proxyMethods,
+              'Not all proxy methods are tested');
+          });
+
           explicitSenderOps.forEach(function (op) {
             it(`${op[0]} reverts`, async function () {
               await util.assertRevertsReason(upgradeToken[op[0]](...op[1], { from: owner }),