[Feature] Remove/disable delegate

CHANGELOG.md

@@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file.
 [__1.5.0__](https://www.npmjs.com/package/polymath-core?activeTab=readme) __15-08-18__
 
 ## Added
+* Added `deleteDelegate()` to general permission manager. It will disable the delegate but not delete the perms.
 * Migrated from `npm` to `yarn`.
 * Added `SingleTradeVolumeRestrictionManager` module
 * Added flag in `PercentageTransferManager` to allow ignoring of issuance transfers

contracts/modules/PermissionManager/GeneralPermissionManager.sol

@@ -8,7 +8,7 @@ import "../Module.sol";
  */
 contract GeneralPermissionManager is IPermissionManager, Module {
 
-    // Mapping used to hold the permissions on the modules provided to delegate, module add => delegate add => permission uint8 => bool 
+    // Mapping used to hold the permissions on the modules provided to delegate, module add => delegate add => permission bytes32 => bool 
     mapping (address => mapping (address => mapping (bytes32 => bool))) public perms;
     // Mapping hold the delagate details
     mapping (address => bytes32) public delegateDetails;
@@ -59,13 +59,29 @@ contract GeneralPermissionManager is IPermissionManager, Module {
      * @param _details Details about the delegate i.e `Belongs to financial firm`
      */
     function addDelegate(address _delegate, bytes32 _details) external withPerm(CHANGE_PERMISSION) {
+        require(_delegate != address(0), "Invalid address");
         require(_details != bytes32(0), "0 value not allowed");
         require(delegateDetails[_delegate] == bytes32(0), "Already present");
         delegateDetails[_delegate] = _details;
         allDelegates.push(_delegate);
         emit AddDelegate(_delegate, _details, now);
     }
 
+    /**
+     * @notice Used to delete a delegate
+     * @param _delegate Ethereum address of the delegate
+     */
+    function deleteDelegate(address _delegate) external withPerm(CHANGE_PERMISSION) {
+        require(delegateDetails[_delegate] != bytes32(0), "delegate does not exist");
+        for (uint256 i = 0; i < allDelegates.length; i++) {
+            if (allDelegates[i] == _delegate) {
+                allDelegates[i] = allDelegates[allDelegates.length - 1];
+                allDelegates.length = allDelegates.length - 1;
+            }
+        }
+        delete delegateDetails[_delegate];
+    }
+
     /**
      * @notice use to check if an address is a delegate or not
      * @param _potentialDelegate the address of potential delegate
@@ -97,9 +113,8 @@ contract GeneralPermissionManager is IPermissionManager, Module {
     public
     withPerm(CHANGE_PERMISSION)
     {
-        require(delegateDetails[_delegate] != bytes32(0), "Delegate is not exists");
-        perms[_module][_delegate][_perm] = _valid;
-        emit ChangePermission(_delegate, _module, _perm, _valid, now);
+        require(_delegate != address(0), "invalid address");
+        _changePermission(_delegate, _module, _perm, _valid);
     }
 
     /**
@@ -119,11 +134,12 @@ contract GeneralPermissionManager is IPermissionManager, Module {
     external
     withPerm(CHANGE_PERMISSION)
     {
-        require(_modules.length > 0 && _perms.length > 0, "0 length is not allowed");
+        require(_delegate != address(0), "invalid address");
+        require(_modules.length > 0, "0 length is not allowed");
         require(_modules.length == _perms.length, "Array length mismatch");
         require(_valids.length == _perms.length, "Array length mismatch");
-        for(uint8 i = 0; i < _perms.length; i++) {
-            changePermission(_delegate, _modules[i], _perms[i], _valids[i]);
+        for(uint256 i = 0; i < _perms.length; i++) {
+            _changePermission(_delegate, _modules[i], _perms[i], _valids[i]);
         }
     }
 
@@ -135,7 +151,7 @@ contract GeneralPermissionManager is IPermissionManager, Module {
      */
     function getAllDelegatesWithPerm(address _module, bytes32 _perm) external view returns(address[]) {
         uint256 counter = 0;
-        uint8 i = 0;
+        uint256 i = 0;
         for (i = 0; i < allDelegates.length; i++) {
             if (perms[_module][allDelegates[i]][_perm]) {
                 counter++;
@@ -164,13 +180,13 @@ contract GeneralPermissionManager is IPermissionManager, Module {
     function getAllModulesAndPermsFromTypes(address _delegate, uint8[] _types, address _tokenAddress) external view returns(address[], bytes32[]) {
         uint256 counter = 0;
         // loop through _types and get their modules from securityToken->getModulesByType
-        for (uint8 i = 0; i < _types.length; i++) {
+        for (uint256 i = 0; i < _types.length; i++) {
             address[] memory _currentTypeModules = ISecurityToken(_tokenAddress).getModulesByType(_types[i]);
             // loop through each modules to get their perms from IModule->getPermissions
-            for (uint8 j = 0; j < _currentTypeModules.length; j++){
+            for (uint256 j = 0; j < _currentTypeModules.length; j++){
                 bytes32[] memory _allModulePerms = IModule(_currentTypeModules[j]).getPermissions();
                 // loop through each perm, if it is true, push results into arrays
-                for (uint8 k = 0; k < _allModulePerms.length; k++) {
+                for (uint256 k = 0; k < _allModulePerms.length; k++) {
                     if (perms[_currentTypeModules[j]][_delegate][_allModulePerms[k]]) {
                         counter ++;
                     }
@@ -199,6 +215,26 @@ contract GeneralPermissionManager is IPermissionManager, Module {
         return(_allModules, _allPerms);
     }
 
+    /**
+     * @notice Use to provide/change the permission to the delegate corresponds to the module contract
+     * @param _delegate Ethereum address of the delegate
+     * @param _module Ethereum contract address of the module
+     * @param _perm Permission flag
+     * @param _valid Bool flag use to switch on/off the permission
+     * @return bool
+     */
+    function _changePermission(
+        address _delegate,
+        address _module,
+        bytes32 _perm,
+        bool _valid
+    )
+     internal
+    {
+        perms[_module][_delegate][_perm] = _valid;
+        emit ChangePermission(_delegate, _module, _perm, _valid, now);
+    }
+
     /**
      * @notice use to get all delegates
      * @return address[]

contracts/modules/PermissionManager/IPermissionManager.sol

@@ -14,13 +14,19 @@ interface IPermissionManager {
     */
     function checkPermission(address _delegate, address _module, bytes32 _perm) external view returns(bool);
 
-     /**
+    /**
     * @notice Use to add a delegate
     * @param _delegate Ethereum address of the delegate
     * @param _details Details about the delegate i.e `Belongs to financial firm`
     */
     function addDelegate(address _delegate, bytes32 _details) external;
 
+    /**
+    * @notice Used to delete a delegate
+    * @param _delegate Ethereum address of the delegate
+    */
+    function deleteDelegate(address _delegate) external;
+
     /**
     * @notice use to check if an address is a delegate or not
     * @param _potentialDelegate the address of potential delegate

test/g_general_permission_manager.js

@@ -209,14 +209,20 @@ contract('GeneralPermissionManager', accounts => {
         });
 
         it("Should fail in adding the delegate -- no delegate details provided", async() => {
-            catchRevert(
+            await catchRevert(
                 I_GeneralPermissionManager.addDelegate(account_delegate, '', { from: token_owner })
             );
         });
 
-        it("Should fail to provide the permission -- because delegate is not yet added", async() => {
+        it("Should fail in adding the delegate -- no delegate address provided", async() => {
+            await catchRevert(
+                I_GeneralPermissionManager.addDelegate('', delegateDetails, { from: token_owner })
+            );
+        });
+
+        it("Should fail to remove the delegate -- failed because delegate does not exisit", async() => {
             await catchRevert(
-                I_GeneralPermissionManager.changePermission(account_delegate, I_GeneralTransferManager.address, "WHITELIST", true, {from: token_owner})
+                I_GeneralPermissionManager.deleteDelegate(account_delegate, { from: token_owner})
             );
         });
 
@@ -260,6 +266,27 @@ contract('GeneralPermissionManager', accounts => {
             );
         });
 
+        it("Should fail to remove the delegate -- failed because unauthorized msg.sender", async() => {
+            await catchRevert(
+                I_GeneralPermissionManager.deleteDelegate(account_delegate, { from: account_delegate})
+            );
+        });
+
+        it("Should remove the delegate", async() => {
+            await I_GeneralPermissionManager.deleteDelegate(account_delegate, { from: token_owner})
+        });
+
+        it("Should check the permission", async () => {
+            assert.isFalse(
+                await I_GeneralPermissionManager.checkPermission.call(account_delegate, I_GeneralTransferManager.address, "WHITELIST")
+            );
+        });
+
+        it("Should successfully add the delegate", async() => {
+            let tx = await I_GeneralPermissionManager.addDelegate(account_delegate, delegateDetails, { from: token_owner});
+            assert.equal(tx.logs[0].args._delegate, account_delegate);
+        });
+
         it("Should check the delegate details", async() => {
             assert.equal(web3.utils.toAscii(await I_GeneralPermissionManager.delegateDetails.call(account_delegate))
                         .replace(/\u0000/g, ''),