Upgradable tokens

contracts/ModuleRegistry.sol

@@ -48,7 +48,9 @@ contract ModuleRegistry is IModuleRegistry, EternalStorage {
     // Emit when the Module Factory gets registered on the ModuleRegistry contract
     event ModuleRegistered(address indexed _moduleFactory, address indexed _owner);
     // Emit when the module gets verified by Polymath
-    event ModuleVerified(address indexed _moduleFactory, bool _verified);
+    event ModuleVerified(address indexed _moduleFactory);
+    // Emit when the module gets unverified by Polymath or the factory owner
+    event ModuleUnverified(address indexed _moduleFactory);
     // Emit when a ModuleFactory is removed by Polymath
     event ModuleRemoved(address indexed _moduleFactory, address indexed _decisionMaker);
     // Emit when ownership gets transferred
@@ -118,29 +120,36 @@ contract ModuleRegistry is IModuleRegistry, EternalStorage {
      * @dev The feature switch for custom modules is labelled "customModulesAllowed"
      * @param _moduleFactory is the address of the relevant module factory
      */
-    function useModule(address _moduleFactory) external {
-        // This if statement is required to be able to add modules from the token proxy contract during deployment
-        if (ISecurityTokenRegistry(getAddressValue(Encoder.getKey("securityTokenRegistry"))).isSecurityToken(msg.sender)) {
-            if (IFeatureRegistry(getAddressValue(Encoder.getKey("featureRegistry"))).getFeatureStatus("customModulesAllowed")) {
-                require(
-                    getBoolValue(Encoder.getKey("verified", _moduleFactory)) || IOwnable(_moduleFactory).owner() == IOwnable(msg.sender).owner(),
-                    "ModuleFactory must be verified or SecurityToken owner must be ModuleFactory owner"
-                );
-            } else {
-                require(getBoolValue(Encoder.getKey("verified", _moduleFactory)), "ModuleFactory must be verified");
-            }
-            require(_isCompatibleModule(_moduleFactory, msg.sender), "Version should within the compatible range of ST");
+    function useModule(address _moduleFactory, bool _isUpgrade) external {
+        if (IFeatureRegistry(getAddressValue(Encoder.getKey("featureRegistry"))).getFeatureStatus("customModulesAllowed")) {
+            require(
+                getBoolValue(Encoder.getKey("verified", _moduleFactory)) || IOwnable(_moduleFactory).owner() == IOwnable(msg.sender).owner(),
+                "ModuleFactory must be verified or SecurityToken owner must be ModuleFactory owner"
+            );
+        } else {
+            require(getBoolValue(Encoder.getKey("verified", _moduleFactory)), "ModuleFactory must be verified");
+        }
+        require(isCompatibleModule(_moduleFactory, msg.sender), "Incompatible versions");
+        // This if statement is required to be able to add modules from the STFactory contract during deployment
+        // before the token has been registered to the STR.
+        if ((!_isUpgrade) && ISecurityTokenRegistry(getAddressValue(Encoder.getKey("securityTokenRegistry"))).isSecurityToken(msg.sender)) {
             pushArray(Encoder.getKey("reputation", _moduleFactory), msg.sender);
             emit ModuleUsed(_moduleFactory, msg.sender);
         }
     }
 
-    function _isCompatibleModule(address _moduleFactory, address _securityToken) internal view returns(bool) {
+    /**
+     * @notice Check that a module and its factory are compatible
+     * @param _moduleFactory is the address of the relevant module factory
+     * @param _securityToken is the address of the relevant security token
+     * @return bool whether module and token are compatible
+     */
+    function isCompatibleModule(address _moduleFactory, address _securityToken) public view returns(bool) {
         uint8[] memory _latestVersion = ISecurityToken(_securityToken).getVersion();
         uint8[] memory _lowerBound = IModuleFactory(_moduleFactory).lowerSTVersionBounds();
         uint8[] memory _upperBound = IModuleFactory(_moduleFactory).upperSTVersionBounds();
-        bool _isLowerAllowed = VersionUtils.compareLowerBound(_lowerBound, _latestVersion);
-        bool _isUpperAllowed = VersionUtils.compareUpperBound(_upperBound, _latestVersion);
+        bool _isLowerAllowed = VersionUtils.lessThanOrEqual(_lowerBound, _latestVersion);
+        bool _isUpperAllowed = VersionUtils.greaterThanOrEqual(_upperBound, _latestVersion);
         return (_isLowerAllowed && _isUpperAllowed);
     }
 
@@ -221,12 +230,29 @@ contract ModuleRegistry is IModuleRegistry, EternalStorage {
     * @notice (The only exception to this is that the author of the module is the owner of the ST)
     * @notice -> Only if Polymath enabled the feature.
     * @param _moduleFactory is the address of the module factory to be verified
-    * @return bool
     */
-    function verifyModule(address _moduleFactory, bool _verified) external onlyOwner {
+    function verifyModule(address _moduleFactory) external onlyOwner {
+        require(getUintValue(Encoder.getKey("registry", _moduleFactory)) != uint256(0), "Module factory must be registered");
+        set(Encoder.getKey("verified", _moduleFactory), true);
+        emit ModuleVerified(_moduleFactory);
+    }
+
+    /**
+    * @notice Called by Polymath to verify Module Factories for SecurityTokens to use.
+    * @notice A module can not be used by an ST unless first approved/verified by Polymath
+    * @notice (The only exception to this is that the author of the module is the owner of the ST)
+    * @notice -> Only if Polymath enabled the feature.
+    * @param _moduleFactory is the address of the module factory to be verified
+    */
+    function unverifyModule(address _moduleFactory) external {
+        // Can be called by the registry owner, the module factory, or the module factory owner
+        bool isOwner = msg.sender == owner();
+        bool isFactoryOwner = msg.sender == IOwnable(_moduleFactory).owner();
+        bool isFactory = msg.sender == _moduleFactory;
+        require(isOwner || isFactoryOwner || isFactory, "Not authorised");
         require(getUintValue(Encoder.getKey("registry", _moduleFactory)) != uint256(0), "Module factory must be registered");
-        set(Encoder.getKey("verified", _moduleFactory), _verified);
-        emit ModuleVerified(_moduleFactory, _verified);
+        set(Encoder.getKey("verified", _moduleFactory), false);
+        emit ModuleUnverified(_moduleFactory);
     }
 
     /**
@@ -281,12 +307,13 @@ contract ModuleRegistry is IModuleRegistry, EternalStorage {
     }
 
     /**
-     * @notice Returns the reputation of the entered Module Factory
+     * @notice Returns the verified status, and reputation of the entered Module Factory
      * @param _factoryAddress is the address of the module factory
+     * @return bool indicating whether module factory is verified
      * @return address array which contains the list of securityTokens that use that module factory
      */
-    function getReputationByFactory(address _factoryAddress) external view returns(address[] memory) {
-        return getArrayAddress(Encoder.getKey("reputation", _factoryAddress));
+    function getFactoryDetails(address _factoryAddress) external view returns(bool, address[] memory) {
+        return (getBoolValue(Encoder.getKey("verified", _factoryAddress)), getArrayAddress(Encoder.getKey("reputation", _factoryAddress)));
     }
 
     /**
@@ -305,8 +332,8 @@ contract ModuleRegistry is IModuleRegistry, EternalStorage {
      * @return address array that contains the list of available addresses of module factory contracts.
      */
     function getModulesByTypeAndToken(uint8 _moduleType, address _securityToken) public view returns(address[] memory) {
-        uint256 _len = getArrayAddress(Encoder.getKey("moduleList", uint256(_moduleType))).length;
         address[] memory _addressList = getArrayAddress(Encoder.getKey("moduleList", uint256(_moduleType)));
+        uint256 _len = _addressList.length;
         bool _isCustomModuleAllowed = IFeatureRegistry(getAddressValue(Encoder.getKey("featureRegistry"))).getFeatureStatus(
             "customModulesAllowed"
         );
@@ -315,9 +342,9 @@ contract ModuleRegistry is IModuleRegistry, EternalStorage {
             if (_isCustomModuleAllowed) {
                 if (IOwnable(_addressList[i]).owner() == IOwnable(_securityToken).owner() || getBoolValue(
                     Encoder.getKey("verified", _addressList[i])
-                )) if (_isCompatibleModule(_addressList[i], _securityToken)) counter++;
+                )) if (isCompatibleModule(_addressList[i], _securityToken)) counter++;
             } else if (getBoolValue(Encoder.getKey("verified", _addressList[i]))) {
-                if (_isCompatibleModule(_addressList[i], _securityToken)) counter++;
+                if (isCompatibleModule(_addressList[i], _securityToken)) counter++;
             }
         }
         address[] memory _tempArray = new address[](counter);
@@ -327,13 +354,13 @@ contract ModuleRegistry is IModuleRegistry, EternalStorage {
                 if (IOwnable(_addressList[j]).owner() == IOwnable(_securityToken).owner() || getBoolValue(
                     Encoder.getKey("verified", _addressList[j])
                 )) {
-                    if (_isCompatibleModule(_addressList[j], _securityToken)) {
+                    if (isCompatibleModule(_addressList[j], _securityToken)) {
                         _tempArray[counter] = _addressList[j];
                         counter++;
                     }
                 }
             } else if (getBoolValue(Encoder.getKey("verified", _addressList[j]))) {
-                if (_isCompatibleModule(_addressList[j], _securityToken)) {
+                if (isCompatibleModule(_addressList[j], _securityToken)) {
                     _tempArray[counter] = _addressList[j];
                     counter++;
                 }

contracts/SecurityTokenRegistry.sol

@@ -184,7 +184,7 @@ contract SecurityTokenRegistry is EternalStorage, Proxy {
     {
         require(!getBoolValue(INITIALIZE),"Initialized");
         require(
-            _STFactory != address(0) && _owner != address(0) && _polymathRegistry != address(0) && _getterContract != address(0),
+            _owner != address(0) && _polymathRegistry != address(0) && _getterContract != address(0),
             "Invalid address"
         );
         require(_stLaunchFee != 0 && _tickerRegFee != 0, "Bad fee");
@@ -194,7 +194,8 @@ contract SecurityTokenRegistry is EternalStorage, Proxy {
         set(PAUSED, false);
         set(OWNER, _owner);
         set(POLYMATHREGISTRY, _polymathRegistry);
-        _setProtocolVersion(_STFactory, uint8(2), uint8(0), uint8(0));
+        _setProtocolFactory(_STFactory, uint8(2), uint8(0), uint8(0));
+        _setLatestVersion(uint8(2), uint8(0), uint8(0));
         set(INITIALIZE, true);
         set(STRGETTER, _getterContract);
         _updateFromRegistry();
@@ -518,7 +519,7 @@ contract SecurityTokenRegistry is EternalStorage, Proxy {
         require(_tickerOwner(ticker) == msg.sender, "Not authorised");
         /*solium-disable-next-line security/no-block-members*/
         require(getUintValue(Encoder.getKey("registeredTickers_expiryDate", ticker)) >= now, "Ticker expired");
-        _deployToken(_name, ticker, _tokenDetails, msg.sender, _divisible, _treasuryWallet, protocolVersion); 
+        _deployToken(_name, ticker, _tokenDetails, msg.sender, _divisible, _treasuryWallet, protocolVersion);
     }
 
     function _deployToken(
@@ -529,7 +530,7 @@ contract SecurityTokenRegistry is EternalStorage, Proxy {
         bool _divisible,
         address _wallet,
         uint256 _protocolVersion
-    ) 
+    )
         internal
     {
         (uint256 _usdFee, uint256 _polyFee) = _takeFee(STLAUNCHFEE);
@@ -572,7 +573,7 @@ contract SecurityTokenRegistry is EternalStorage, Proxy {
     )
         external
         onlyOwner
-    {   
+    {
         require(bytes(_name).length > 0 && bytes(_ticker).length > 0, "Bad data");
         require(bytes(_ticker).length <= 10, "Bad ticker");
         require(_deployedAt != 0 && _owner != address(0), "Bad data");
@@ -682,34 +683,49 @@ contract SecurityTokenRegistry is EternalStorage, Proxy {
     }
 
     /**
-    * @notice Changes the protocol version and the SecurityToken contract
+    * @notice Changes the SecurityToken contract for a particular factory version
     * @notice Used only by Polymath to upgrade the SecurityToken contract and add more functionalities to future versions
     * @notice Changing versions does not affect existing tokens.
     * @param _STFactoryAddress is the address of the proxy.
     * @param _major Major version of the proxy.
     * @param _minor Minor version of the proxy.
     * @param _patch Patch version of the proxy
     */
-    function setProtocolVersion(address _STFactoryAddress, uint8 _major, uint8 _minor, uint8 _patch) external onlyOwner {
+    function setProtocolFactory(address _STFactoryAddress, uint8 _major, uint8 _minor, uint8 _patch) external onlyOwner {
+        _setProtocolFactory(_STFactoryAddress, _major, _minor, _patch);
+    }
+
+    function _setProtocolFactory(address _STFactoryAddress, uint8 _major, uint8 _minor, uint8 _patch) internal {
         require(_STFactoryAddress != address(0), "Bad address");
-        _setProtocolVersion(_STFactoryAddress, _major, _minor, _patch);
+        uint8[] memory _version = new uint8[](3);
+        _version[0] = _major;
+        _version[1] = _minor;
+        _version[2] = _patch;
+        uint24 _packedVersion = VersionUtils.pack(_major, _minor, _patch);
+        //set(Encoder.getKey("latestVersion"), uint256(_packedVersion));
+        set(Encoder.getKey("protocolVersionST", uint256(_packedVersion)), _STFactoryAddress);
     }
 
     /**
-    * @notice Internal - Changes the protocol version and the SecurityToken contract
+    * @notice Changes the default protocol version
+    * @notice Used only by Polymath to upgrade the SecurityToken contract and add more functionalities to future versions
+    * @notice Changing versions does not affect existing tokens.
+    * @param _major Major version of the proxy.
+    * @param _minor Minor version of the proxy.
+    * @param _patch Patch version of the proxy
     */
-    function _setProtocolVersion(address _STFactoryAddress, uint8 _major, uint8 _minor, uint8 _patch) internal {
+    function setLatestVersion(uint8 _major, uint8 _minor, uint8 _patch) external onlyOwner {
+        _setLatestVersion(_major, _minor, _patch);
+    }
+
+    function _setLatestVersion(uint8 _major, uint8 _minor, uint8 _patch) internal {
         uint8[] memory _version = new uint8[](3);
         _version[0] = _major;
         _version[1] = _minor;
         _version[2] = _patch;
         uint24 _packedVersion = VersionUtils.pack(_major, _minor, _patch);
-        require(
-            VersionUtils.isValidVersion(VersionUtils.unpack(uint24(getUintValue(Encoder.getKey("latestVersion")))), _version),
-            "Bad version"
-        );
+        require(getAddressValue(Encoder.getKey("protocolVersionST", _packedVersion)) != address(0), "No factory");
         set(Encoder.getKey("latestVersion"), uint256(_packedVersion));
-        set(Encoder.getKey("protocolVersionST", getUintValue(Encoder.getKey("latestVersion"))), _STFactoryAddress);
     }
 
     /**

contracts/interfaces/IModuleRegistry.sol

@@ -7,8 +7,9 @@ interface IModuleRegistry {
     /**
      * @notice Called by a security token to notify the registry it is using a module
      * @param _moduleFactory is the address of the relevant module factory
+     * @param _isUpgrade whether the use is part of an existing module upgrade
      */
-    function useModule(address _moduleFactory) external;
+    function useModule(address _moduleFactory, bool _isUpgrade) external;
 
     /**
      * @notice Called by the ModuleFactory owner to register new modules for SecurityToken to use
@@ -22,20 +23,37 @@ interface IModuleRegistry {
      */
     function removeModule(address _moduleFactory) external;
 
+    /**
+     * @notice Check that a module and its factory are compatible
+     * @param _moduleFactory is the address of the relevant module factory
+     * @param _securityToken is the address of the relevant security token
+     * @return bool whether module and token are compatible
+     */
+    function isCompatibleModule(address _moduleFactory, address _securityToken) external view returns(bool);
+
     /**
     * @notice Called by Polymath to verify modules for SecurityToken to use.
     * @notice A module can not be used by an ST unless first approved/verified by Polymath
     * @notice (The only exception to this is that the author of the module is the owner of the ST - Only if enabled by the FeatureRegistry)
     * @param _moduleFactory is the address of the module factory to be registered
     */
-    function verifyModule(address _moduleFactory, bool _verified) external;
+    function verifyModule(address _moduleFactory) external;
+
+    /**
+    * @notice Called by Polymath to unverify modules for SecurityToken to use.
+    * @notice A module can not be used by an ST unless first approved/verified by Polymath
+    * @notice (The only exception to this is that the author of the module is the owner of the ST - Only if enabled by the FeatureRegistry)
+    * @param _moduleFactory is the address of the module factory to be registered
+    */
+    function unverifyModule(address _moduleFactory) external;
 
     /**
-     * @notice Used to get the reputation of a Module Factory
-     * @param _factoryAddress address of the Module Factory
-     * @return address array which has the list of securityToken's uses that module factory
+     * @notice Returns the verified status, and reputation of the entered Module Factory
+     * @param _factoryAddress is the address of the module factory
+     * @return bool indicating whether module factory is verified
+     * @return address array which contains the list of securityTokens that use that module factory
      */
-    function getReputationByFactory(address _factoryAddress) external view returns(address[] memory);
+    function getFactoryDetails(address _factoryAddress) external view returns(bool, address[] memory);
 
     /**
      * @notice Returns all the tags related to the a module type which are valid for the given token
@@ -82,7 +100,7 @@ interface IModuleRegistry {
 
     /**
      * @notice Check whether the contract operations is paused or not
-     * @return bool 
+     * @return bool
      */
     function isPaused() external view returns(bool);