Protocol upgrade fixes (#733)

* Update tags, types, lower & upper bounds (#725)

* Make useModule backwards compatible

* Make deployToken backwards compatible (#726)

* Make deployToken backwards compatible

* Small fix for scheduledCheckpoint

* Updated STR interface

* Removed extra event

* Fix interface mismatch

* cleaning up as per the audit recommendation (#722)

Author: maxsam4

contracts/ModuleRegistry.sol

@@ -43,7 +43,7 @@ contract ModuleRegistry is IModuleRegistry, EternalStorage {
     bytes32 constant POLYMATHREGISTRY = 0x90eeab7c36075577c7cc5ff366e389fefa8a18289b949bab3529ab4471139d4d; //keccak256("polymathRegistry")
     bytes32 constant FEATURE_REGISTRY = 0xed9ca06607835ad25ecacbcb97f2bc414d4a51ecf391b5ae42f15991227ab146; //keccak256("featureRegistry")
     bytes32 constant SECURITY_TOKEN_REGISTRY = 0x12ada4f7ee6c2b7b933330be61fefa007a1f497dc8df1b349b48071a958d7a81; //keccak256("securityTokenRegistry")
-    
+
     ///////////////
     //// Modifiers
     ///////////////
@@ -118,6 +118,18 @@ contract ModuleRegistry is IModuleRegistry, EternalStorage {
         return IFeatureRegistry(getAddressValue(FEATURE_REGISTRY)).getFeatureStatus("customModulesAllowed");
     }
 
+
+    /**
+     * @notice Called by a SecurityToken (2.x) to check if the ModuleFactory is verified or appropriate custom module
+     * @dev ModuleFactory reputation increases by one every time it is deployed(used) by a ST.
+     * @dev Any module can be added during token creation without being registered if it is defined in the token proxy deployment contract
+     * @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 {
+        useModule(_moduleFactory, false);
+    }
+
     /**
      * @notice Called by a SecurityToken to check if the ModuleFactory is verified or appropriate custom module
      * @dev ModuleFactory reputation increases by one every time it is deployed(used) by a ST.
@@ -126,7 +138,7 @@ contract ModuleRegistry is IModuleRegistry, EternalStorage {
      * @param _moduleFactory is the address of the relevant module factory
      * @param _isUpgrade whether or not the function is being called as a result of an upgrade
      */
-    function useModule(address _moduleFactory, bool _isUpgrade) external nonReentrant {
+    function useModule(address _moduleFactory, bool _isUpgrade) public nonReentrant {
         if (_customModules()) {
             require(
                 getBoolValue(Encoder.getKey("verified", _moduleFactory)) || getAddressValue(Encoder.getKey("factoryOwner", _moduleFactory))
@@ -155,8 +167,8 @@ contract ModuleRegistry is IModuleRegistry, EternalStorage {
      */
     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();
+        uint8[] memory _lowerBound = IModuleFactory(_moduleFactory).getLowerSTVersionBounds();
+        uint8[] memory _upperBound = IModuleFactory(_moduleFactory).getUpperSTVersionBounds();
         bool _isLowerAllowed = VersionUtils.lessThanOrEqual(_lowerBound, _latestVersion);
         bool _isUpperAllowed = VersionUtils.greaterThanOrEqual(_upperBound, _latestVersion);
         return (_isLowerAllowed && _isUpperAllowed);
@@ -183,7 +195,7 @@ contract ModuleRegistry is IModuleRegistry, EternalStorage {
         //Enforce type uniqueness
         uint256 i;
         uint256 j;
-        uint8[] memory moduleTypes = moduleFactory.types();
+        uint8[] memory moduleTypes = moduleFactory.getTypes();
         for (i = 1; i < moduleTypes.length; i++) {
             for (j = 0; j < i; j++) {
                 require(moduleTypes[i] != moduleTypes[j], "Type mismatch");
@@ -304,14 +316,14 @@ contract ModuleRegistry is IModuleRegistry, EternalStorage {
         uint256 i;
         uint256 j;
         for (i = 0; i < _modules.length; i++) {
-            counter = counter + IModuleFactory(_modules[i]).tags().length;
+            counter = counter + IModuleFactory(_modules[i]).getTags().length;
         }
         bytes32[] memory tags = new bytes32[](counter);
         address[] memory modules = new address[](counter);
         bytes32[] memory tempTags;
         counter = 0;
         for (i = 0; i < _modules.length; i++) {
-            tempTags = IModuleFactory(_modules[i]).tags();
+            tempTags = IModuleFactory(_modules[i]).getTags();
             for (j = 0; j < tempTags.length; j++) {
                 tags[counter] = tempTags[j];
                 modules[counter] = _modules[i];

contracts/SecurityTokenRegistry.sol

@@ -108,7 +108,7 @@ contract SecurityTokenRegistry is EternalStorage, Proxy {
     // Emit when ownership of the ticker gets changed
     event ChangeTickerOwnership(string _ticker, address indexed _oldOwner, address indexed _newOwner);
     // Emit at the time of launching a new security token of version 3.0+
-    event NewSecurityTokenCreated(
+    event NewSecurityToken(
         string _ticker,
         string _name,
         address indexed _securityTokenAddress,
@@ -173,10 +173,14 @@ contract SecurityTokenRegistry is EternalStorage, Proxy {
      * @dev Throws if called by any account other than the owner.
      */
     modifier onlyOwner() {
-        require(msg.sender == owner(), "Only owner");
+        _onlyOwner();
         _;
     }
 
+    function _onlyOwner() internal view {
+        require(msg.sender == owner(), "Only owner");
+    }
+
     modifier onlyOwnerOrSelf() {
         require(msg.sender == owner() || msg.sender == address(this), "Only owner or self");
         _;
@@ -639,7 +643,7 @@ contract SecurityTokenRegistry is EternalStorage, Proxy {
                 _ticker, _name, newSecurityTokenAddress, issuer, now, issuer, false, _polyFee
             );
         } else {
-            emit NewSecurityTokenCreated(
+            emit NewSecurityToken(
                 _ticker, _name, newSecurityTokenAddress, issuer, now, issuer, false, _usdFee, _polyFee, _protocolVersion
             );
         }
@@ -691,15 +695,24 @@ contract SecurityTokenRegistry is EternalStorage, Proxy {
         internal
         returns(address newSecurityTokenAddress)
     {
+        // In v2.x of STFactory, the final argument to deployToken is the PolymathRegistry.
+        // In v3.x of STFactory, the final argument to deployToken is the Treasury wallet.
+        uint8[] memory upperLimit = new uint8[](3);
+        upperLimit[0] = 2;
+        upperLimit[1] = 99;
+        upperLimit[2] = 99;
+        if (VersionUtils.lessThanOrEqual(VersionUtils.unpack(uint24(_protocolVersion)), upperLimit)) {
+            _wallet = getAddressValue(POLYMATHREGISTRY);
+        }
+
         newSecurityTokenAddress = ISTFactory(getAddressValue(Encoder.getKey("protocolVersionST", _protocolVersion))).deployToken(
             _name,
             _ticker,
             18,
             _tokenDetails,
             _issuer,
             _divisible,
-            _wallet,
-            getAddressValue(POLYMATHREGISTRY)
+            _wallet
         );
 
         /*solium-disable-next-line security/no-block-members*/
@@ -739,7 +752,7 @@ contract SecurityTokenRegistry is EternalStorage, Proxy {
         set(Encoder.getKey("tickerToSecurityToken", ticker), _securityToken);
         _modifyTicker(_owner, ticker, registrationTime, expiryTime, true);
         _storeSecurityTokenData(_securityToken, ticker, _tokenDetails, _deployedAt);
-        emit NewSecurityTokenCreated(
+        emit NewSecurityToken(
             ticker, ISecurityToken(_securityToken).name(), _securityToken, _owner, _deployedAt, msg.sender, true, uint256(0), uint256(0), 0
         );
     }
@@ -951,5 +964,4 @@ contract SecurityTokenRegistry is EternalStorage, Proxy {
     function owner() public view returns(address) {
         return getAddressValue(OWNER);
     }
-
 }

contracts/interfaces/IModuleFactory.sol

@@ -47,12 +47,12 @@ interface IModuleFactory {
     /**
      * @notice Type of the Module factory
      */
-    function types() external view returns(uint8[] memory moduleTypes);
+    function getTypes() external view returns(uint8[] memory moduleTypes);
 
     /**
      * @notice Get the tags related to the module factory
      */
-    function tags() external view returns(bytes32[] memory moduleTags);
+    function getTags() external view returns(bytes32[] memory moduleTags);
 
     /**
      * @notice Used to change the setup fee
@@ -83,13 +83,13 @@ interface IModuleFactory {
      * @notice Used to get the lower bound
      * @return Lower bound
      */
-    function lowerSTVersionBounds() external view returns(uint8[] memory lowerBounds);
+    function getLowerSTVersionBounds() external view returns(uint8[] memory lowerBounds);
 
     /**
      * @notice Used to get the upper bound
      * @return Upper bound
      */
-    function upperSTVersionBounds() external view returns(uint8[] memory upperBounds);
+    function getUpperSTVersionBounds() external view returns(uint8[] memory upperBounds);
 
     /**
      * @notice Updates the tags of the ModuleFactory

contracts/interfaces/IModuleRegistry.sol

@@ -27,6 +27,12 @@ interface IModuleRegistry {
     event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);
 
 
+    /**
+     * @notice Called by a security token (2.x) to notify the registry it is using a module
+     * @param _moduleFactory is the address of the relevant module factory
+     */
+    function useModule(address _moduleFactory) external;
+
     /**
      * @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

contracts/interfaces/ISTFactory.sol

@@ -23,7 +23,6 @@ interface ISTFactory {
      * @param _issuer is the owner of the Security Token
      * @param _divisible whether the token is divisible or not
      * @param _treasuryWallet Ethereum address which will holds the STs.
-     * @param _polymathRegistry is the address of the Polymath Registry contract
      */
     function deployToken(
         string calldata _name,
@@ -32,10 +31,9 @@ interface ISTFactory {
         string calldata _tokenDetails,
         address _issuer,
         bool _divisible,
-        address _treasuryWallet,
-        address _polymathRegistry
-    ) 
-    external 
+        address _treasuryWallet //In v2.x this is the Polymath Registry
+    )
+    external
     returns(address tokenAddress);
 
     /**

contracts/interfaces/ISecurityTokenRegistry.sol

@@ -24,7 +24,7 @@ interface ISecurityTokenRegistry {
     // Emit when ownership of the ticker gets changed
     event ChangeTickerOwnership(string _ticker, address indexed _oldOwner, address indexed _newOwner);
     // Emit at the time of launching a new security token of version 3.0+
-    event NewSecurityTokenCreated(
+    event NewSecurityToken(
         string _ticker,
         string _name,
         address indexed _securityTokenAddress,
@@ -244,8 +244,7 @@ interface ISecurityTokenRegistry {
         string memory tokenSymbol,
         address tokenAddress,
         string memory tokenDetails,
-        uint256 tokenTime,
-        uint8[] memory tokenVersion
+        uint256 tokenTime
     );
 
     /**

contracts/mocks/MockFactory.sol

@@ -29,7 +29,7 @@ contract MockFactory is DummySTOFactory {
     /**
      * @notice Type of the Module factory
      */
-    function types() external view returns(uint8[] memory) {
+    function getTypes() external view returns(uint8[] memory) {
         if (!typesSwitch) {
             uint8[] memory res = new uint8[](0);
             return res;

contracts/mocks/MockWrongTypeFactory.sol

@@ -28,7 +28,7 @@ contract MockWrongTypeFactory is MockBurnFactory {
     /**
      * @notice Type of the Module factory
      */
-    function types() external view returns(uint8[] memory) {
+    function getTypes() external view returns(uint8[] memory) {
         uint8[] memory res = new uint8[](1);
         res[0] = 4;
         return res;

contracts/mocks/TestSTOFactory.sol

@@ -28,7 +28,7 @@ contract TestSTOFactory is DummySTOFactory {
     /**
      * @notice Gets the tags related to the module factory
      */
-    function tags() external view returns(bytes32[] memory) {
+    function getTags() external view returns(bytes32[] memory) {
         bytes32[] memory availableTags = new bytes32[](4);
         availableTags[0] = "Test";
         availableTags[1] = "Non-refundable";

contracts/modules/Checkpoint/Voting/PLCR/PLCRVotingCheckpoint.sol

@@ -53,7 +53,7 @@ contract PLCRVotingCheckpoint is PLCRVotingCheckpointStorage, VotingCheckpoint {
         withPerm(ADMIN)
     {
         uint256 startTime = now;
-        uint256 checkpointId = ISecurityToken(securityToken).createCheckpoint();
+        uint256 checkpointId = securityToken.createCheckpoint();
         _createBallotWithCheckpoint(_commitDuration, _revealDuration, _noOfProposals, _quorumPercentage, checkpointId, startTime);
     }
 
@@ -78,7 +78,7 @@ contract PLCRVotingCheckpoint is PLCRVotingCheckpointStorage, VotingCheckpoint {
         withPerm(ADMIN)
     {
         // validate the checkpointId, It should be less than or equal to the current checkpointId of the securityToken
-        require(_checkpointId <= ISecurityToken(securityToken).currentCheckpointId(), "Invalid checkpoint Id");
+        require(_checkpointId <= securityToken.currentCheckpointId(), "Invalid checkpoint Id");
         _createBallotWithCheckpoint(_commitDuration, _revealDuration, _noOfProposals, _quorumPercentage, _checkpointId, _startTime);
     }
 
@@ -140,7 +140,7 @@ contract PLCRVotingCheckpoint is PLCRVotingCheckpointStorage, VotingCheckpoint {
         require(ballot.investorToProposal[msg.sender].secretVote == bytes32(0), "Already voted");
         require(ballot.isActive, "Inactive ballot");
         // Get the balance of the voter (i.e `msg.sender`) at the checkpoint on which ballot was created.
-        uint256 weight = ISecurityToken(securityToken).balanceOfAt(msg.sender, ballot.checkpointId);
+        uint256 weight = securityToken.balanceOfAt(msg.sender, ballot.checkpointId);
         require(weight > 0, "Zero weight is not allowed");
         // Update the storage value. Assigned `0` as vote option it will be updated when voter reveals its vote.
         ballot.investorToProposal[msg.sender] = Vote(0, _secretVote);
@@ -170,7 +170,7 @@ contract PLCRVotingCheckpoint is PLCRVotingCheckpointStorage, VotingCheckpoint {
             "Invalid vote"
         );
         // Get the balance of the voter (i.e `msg.sender`) at the checkpoint on which ballot was created.
-        uint256 weight = ISecurityToken(securityToken).balanceOfAt(msg.sender, ballot.checkpointId);
+        uint256 weight = securityToken.balanceOfAt(msg.sender, ballot.checkpointId);
         bytes32 secretVote = ballot.investorToProposal[msg.sender].secretVote;
         // update the storage values
         ballot.proposalToVotes[_choiceOfProposal] = ballot.proposalToVotes[_choiceOfProposal].add(weight);
@@ -283,7 +283,7 @@ contract PLCRVotingCheckpoint is PLCRVotingCheckpointStorage, VotingCheckpoint {
         uint256 i = 0;
         uint256 counter = 0;
         uint256 maxWeight = 0;
-        uint256 supplyAtCheckpoint = ISecurityToken(securityToken).totalSupplyAt(ballot.checkpointId);
+        uint256 supplyAtCheckpoint = securityToken.totalSupplyAt(ballot.checkpointId);
         uint256 quorumWeight = (supplyAtCheckpoint.mul(ballot.quorum)).div(10 ** 18);
         voteWeighting = new uint256[](ballot.totalProposals);
         for (i = 0; i < ballot.totalProposals; i++) {
@@ -342,7 +342,7 @@ contract PLCRVotingCheckpoint is PLCRVotingCheckpointStorage, VotingCheckpoint {
         Ballot memory ballot = ballots[_ballotId];
         return (
             ballot.quorum,
-            ISecurityToken(securityToken).totalSupplyAt(ballot.checkpointId),
+            securityToken.totalSupplyAt(ballot.checkpointId),
             ballot.checkpointId,
             ballot.startTime,
             (uint256(ballot.startTime).add(uint256(ballot.commitDuration))).add(uint256(ballot.revealDuration)),

contracts/modules/Experimental/Mixed/ScheduledCheckpoint.sol

@@ -57,6 +57,7 @@ contract ScheduledCheckpoint is ICheckpoint, TransferManager {
      */
     function addSchedule(bytes32 _name, uint256 _startTime, uint256 _interval, TimeUnit _timeUnit) external {
         _onlySecurityTokenOwner();
+        require(_name != bytes32(""), "Empty name");
         require(_startTime > now, "Start time must be in the future");
         require(schedules[_name].name == bytes32(0), "Name already in use");
         schedules[_name].name = _name;
@@ -75,6 +76,7 @@ contract ScheduledCheckpoint is ICheckpoint, TransferManager {
      */
     function removeSchedule(bytes32 _name) external {
         _onlySecurityTokenOwner();
+        require(_name != bytes32(""), "Empty name");
         require(schedules[_name].name == _name, "Name does not exist");
         uint256 index = schedules[_name].index;
         names[index] = names[names.length - 1];

contracts/modules/ModuleFactory.sol

@@ -50,14 +50,14 @@ contract ModuleFactory is IModuleFactory, Ownable {
     /**
      * @notice Type of the Module factory
      */
-    function types() external view returns(uint8[] memory) {
+    function getTypes() external view returns(uint8[] memory) {
         return typesData;
     }
 
     /**
      * @notice Get the tags related to the module factory
      */
-    function tags() external view returns(bytes32[] memory) {
+    function getTags() external view returns(bytes32[] memory) {
         return tagsData;
     }
 
@@ -154,15 +154,15 @@ contract ModuleFactory is IModuleFactory, Ownable {
      * @notice Used to get the lower bound
      * @return lower bound
      */
-    function lowerSTVersionBounds() external view returns(uint8[] memory) {
+    function getLowerSTVersionBounds() external view returns(uint8[] memory) {
         return VersionUtils.unpack(compatibleSTVersionRange["lowerBound"]);
     }
 
     /**
      * @notice Used to get the upper bound
      * @return upper bound
      */
-    function upperSTVersionBounds() external view returns(uint8[] memory) {
+    function getUpperSTVersionBounds() external view returns(uint8[] memory) {
         return VersionUtils.unpack(compatibleSTVersionRange["upperBound"]);
     }
 

contracts/modules/TransferManager/TransferManager.sol

@@ -26,7 +26,7 @@ contract TransferManager is ITransferManager, Module {
      */
     function getTokensByPartition(bytes32 _partition, address _tokenHolder, uint256 /*_additionalBalance*/) external view returns(uint256) {
         if (_partition == UNLOCKED)
-            return ISecurityToken(securityToken).balanceOf(_tokenHolder);
+            return securityToken.balanceOf(_tokenHolder);
         return uint256(0);
     }