All/pre audit code fixes (#58)

* Make IUpgradableToken an interface * More descriptive name for Upgrade event param * Make events indexed * Fix too long lines * Update event name * Add mock functions ensuring that internal functions are declared as internal * Remove extra modifier from mintable
etoroxlabs · Jan 13, 2019 · d31e501 · d31e501
1 parent 063917b
commit d31e501
Show file tree

Hide file tree

Showing 8 changed files with 61 additions and 25 deletions.
diff --git a/contracts/TokenManager.sol b/contracts/TokenManager.sol
@@ -22,9 +22,11 @@ contract TokenManager is Ownable {
     mapping (bytes32 => TokenEntry) private tokens;
     bytes32[] private names;
 
-    event TokenAdded(bytes32 name, ITokenX addr);
-    event TokenDeleted(bytes32 name, ITokenX addr);
-    event TokenUpgraded(bytes32 name, ITokenX from, ITokenX to);
+    event TokenAdded(bytes32 indexed name, ITokenX indexed addr);
+    event TokenDeleted(bytes32 indexed name, ITokenX indexed addr);
+    event TokenUpgraded(bytes32 indexed name,
+                        ITokenX indexed from,
+                        ITokenX indexed to);
 
     /**
      * @dev Require that the token _name exists

diff --git a/contracts/mocks/ExternalERC20MintableMock.sol b/contracts/mocks/ExternalERC20MintableMock.sol
@@ -20,4 +20,31 @@ contract ExternalERC20MintableMock is  ExternalERC20Mintable, MinterRoleMock {
     function changeMintingRecipient(address to) public {
         _changeMintingRecipient(msg.sender, to);
     }
+
+    /** Triggers compilation error if _changeMintingRecipient
+     * function is not declared internal
+     */
+    function _changeMintingRecipient(
+        address sender,
+        address mintingRecip
+    )
+        internal
+    {
+        super._changeMintingRecipient(sender, mintingRecip);
+    }
+
+    /** Triggers compilation error if _mintExplicitSender
+     * function is not declared internal
+     */
+    function _mintExplicitSender(
+        address sender,
+        address to,
+        uint256 value
+    )
+        internal
+        requireMinter(sender)
+    {
+        super._mintExplicitSender(sender, to, value);
+    }
+
 }
diff --git a/contracts/token/ERC20/ExternalERC20Storage.sol b/contracts/token/ERC20/ExternalERC20Storage.sol
@@ -18,8 +18,9 @@ contract ExternalERC20Storage is Ownable {
 
   address private _implementor;
 
-  event StorageInitialImplementorSet(address to);
-  event StorageImplementorTransferred(address from, address to);
+  event StorageInitialImplementorSet(address indexed to);
+  event StorageImplementorTransferred(address indexed from,
+                                      address indexed to);
 
   /**
    * @dev Returns whether the sender is an implementor.

diff --git a/contracts/token/IUpgradableTokenX.sol b/contracts/token/IUpgradableTokenX.sol
@@ -1,6 +1,6 @@
 pragma solidity ^0.4.24;
 
-contract IUpgradableTokenX {
+interface IUpgradableTokenX {
 
     event Transfer(address indexed from,
                    address indexed to,
@@ -14,72 +14,78 @@ contract IUpgradableTokenX {
     function finalizeUpgrade() external;
 
     /* Taken from ERC20Detailed in openzeppelin-solidity */
-    function nameExplicitSender(address sender) public view returns(string);
+    function nameExplicitSender(address sender) external view returns(string);
 
-    function symbolExplicitSender(address sender) public view returns(string);
+    function symbolExplicitSender(address sender)
+        external
+        view
+        returns(string);
 
-    function decimalsExplicitSender(address sender) public view returns(uint8);
+    function decimalsExplicitSender(address sender)
+        external
+        view
+        returns(uint8);
 
     /* Taken from IERC20 in openzeppelin-solidity */
     function totalSupplyExplicitSender(address sender)
-        public
+        external
         view
         returns (uint256);
 
     function balanceOfExplicitSender(address sender, address who)
-        public
+        external
         view
         returns (uint256);
 
     function allowanceExplicitSender(address sender,
                                      address owner,
                                      address spender)
-        public
+        external
         view
         returns (uint256);
 
     function transferExplicitSender(address sender, address to, uint256 value)
-        public
+        external
         returns (bool);
 
     function approveExplicitSender(address sender,
                                    address spender,
                                    uint256 value)
-        public
+        external
         returns (bool);
 
     function transferFromExplicitSender(address sender,
                                         address from,
                                         address to,
                                         uint256 value)
-        public
+        external
         returns (bool);
 
     function mintExplicitSender(address sender, address to, uint256 value)
-        public
+        external
         returns (bool);
 
     function changeMintingRecipientExplicitSender(address sender,
                                                   address mintingRecip)
-        public;
+        external;
 
-    function burnExplicitSender(address sender, uint256 value) public;
+    function burnExplicitSender(address sender, uint256 value) external;
 
     function burnFromExplicitSender(address sender,
                                     address from,
                                     uint256 value)
-        public;
+        external;
 
     function increaseAllowanceExplicitSender(address sender,
                                              address spender,
                                              uint addedValue)
-        public
+        external
         returns (bool success);
 
     function decreaseAllowanceExplicitSender(address sender,
                                              address spender,
                                              uint subtractedValue)
-        public
+        external
         returns (bool success);
 
 }
diff --git a/contracts/token/TokenX.sol b/contracts/token/TokenX.sol
@@ -41,7 +41,7 @@ contract TokenX is ITokenX, TokenXExplicitSender {
         }
     }
 
-    event Upgraded(address to);
+    event Upgraded(address indexed to);
 
     function isUpgraded() public view returns (bool) {
         return upgradedToken != IUpgradableTokenX(0);

diff --git a/contracts/token/TokenXExplicitSender.sol b/contracts/token/TokenXExplicitSender.sol
@@ -32,7 +32,7 @@ contract TokenXExplicitSender is IUpgradableTokenX,
 
     bool private enabled;
 
-    event UpgradeFinalized(address c, address sender);
+    event UpgradeFinalized(address indexed upgradedFrom, address indexed sender);
 
     /**
      * @param name The name of the token

diff --git a/test/token/TokenX.events.js b/test/token/TokenX.events.js
@@ -3,7 +3,7 @@ const utils = require('./../utils.js');
 module.exports = utils.makeEventMap({
   upgrade: (contract, oldContract) => [
     { eventName: 'UpgradeFinalized',
-      paramMap: { c: oldContract,
+      paramMap: { upgradedFrom: oldContract,
                   sender: oldContract } },
     { eventName: 'Upgraded',
       paramMap: { to: contract } }] });
diff --git a/test/token/TokenXExplicitSender.events.js b/test/token/TokenXExplicitSender.events.js
@@ -3,5 +3,5 @@ const utils = require('./../utils.js');
 module.exports = utils.makeEventMap({
   finalizeUpgrade: (contract, senderAddr) => [{
     eventName: 'UpgradeFinalized',
-    paramMap: { c: contract,
+    paramMap: { upgradedFrom: contract,
                 sender: senderAddr } }] });