diff --git a/contracts/ModuleRegistry.sol b/contracts/ModuleRegistry.sol index ef8fe279f..41c49c8a8 100644 --- a/contracts/ModuleRegistry.sol +++ b/contracts/ModuleRegistry.sol @@ -20,6 +20,8 @@ contract ModuleRegistry is IModuleRegistry, Pausable, RegistryUpdater, ReclaimTo mapping (address => address[]) public reputation; // Mapping contain the list of addresses of Module factory for a particular type mapping (uint8 => address[]) public moduleList; + // Mapping to store the index of the moduleFactory in the moduleList + mapping(address => uint8) private moduleListIndex; // contains the list of verified modules mapping (address => bool) public verified; // Contains the list of the available tags corresponds to the module type @@ -31,6 +33,8 @@ contract ModuleRegistry is IModuleRegistry, Pausable, RegistryUpdater, ReclaimTo event LogModuleRegistered(address indexed _moduleFactory, address indexed _owner); // Emit when the module get verified by the Polymath team event LogModuleVerified(address indexed _moduleFactory, bool _verified); + // Emit when a moduleFactory is removed by Polymath or moduleFactory owner + event LogModuleRemoved(address indexed _moduleFactory, address indexed _decisionMaker); constructor (address _polymathRegistry) public RegistryUpdater(_polymathRegistry) @@ -67,14 +71,49 @@ contract ModuleRegistry is IModuleRegistry, Pausable, RegistryUpdater, ReclaimTo function registerModule(address _moduleFactory) external whenNotPaused returns(bool) { require(registry[_moduleFactory] == 0, "Module factory should not be pre-registered"); IModuleFactory moduleFactory = IModuleFactory(_moduleFactory); - require(moduleFactory.getType() != 0, "Factory type should not equal to 0"); - registry[_moduleFactory] = moduleFactory.getType(); - moduleList[moduleFactory.getType()].push(_moduleFactory); + uint8 moduleType = moduleFactory.getType(); + require(moduleType != 0, "Factory moduleType should not equal to 0"); + registry[_moduleFactory] = moduleType; + moduleListIndex[_moduleFactory] = uint8(moduleList[moduleType].length); + moduleList[moduleType].push(_moduleFactory); reputation[_moduleFactory] = new address[](0); emit LogModuleRegistered (_moduleFactory, Ownable(_moduleFactory).owner()); return true; } + /** + * @notice Called by moduleFactory owner or registry curator to delete a moduleFactory + * @param _moduleFactory is the address of the module factory to be deleted + * @return bool + */ + function removeModule(address _moduleFactory) external whenNotPaused returns(bool) { + uint8 moduleType = registry[_moduleFactory]; + + require(moduleType != 0, "Module factory should be registered"); + require(msg.sender == Ownable(_moduleFactory).owner() || msg.sender == owner, + "msg.sender must be moduleFactory owner or registry curator"); + + uint8 index = moduleListIndex[_moduleFactory]; + uint8 last = uint8(moduleList[moduleType].length - 1); + address temp = moduleList[moduleType][last]; + + // pop from array and re-order + if (index != last) { + moduleList[moduleType][index] = temp; + moduleListIndex[temp] = index; + } + delete moduleList[moduleType][last]; + moduleList[moduleType].length--; + + delete registry[_moduleFactory]; + delete reputation[_moduleFactory]; + delete verified[_moduleFactory]; + delete moduleListIndex[_moduleFactory]; + + emit LogModuleRemoved (_moduleFactory, msg.sender); + return true; + } + /** * @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 diff --git a/contracts/interfaces/IModuleRegistry.sol b/contracts/interfaces/IModuleRegistry.sol index 820e8f502..d6378b209 100644 --- a/contracts/interfaces/IModuleRegistry.sol +++ b/contracts/interfaces/IModuleRegistry.sol @@ -17,6 +17,13 @@ interface IModuleRegistry { */ function registerModule(address _moduleFactory) external returns(bool); + /** + * @notice Called by moduleFactory owner or registry curator to delete a moduleFactory + * @param _moduleFactory is the address of the module factory to be deleted + * @return bool + */ + function removeModule(address _moduleFactory) external returns(bool); + /** * @notice Use to get all the tags releated to the functionality of the Module Factory. * @param _moduleType Type of module @@ -49,7 +56,7 @@ interface IModuleRegistry { /** * @notice Use to get the reputation of the Module factory * @param _factoryAddress Ethereum contract address of the module factory - * @return address array which have the list of securityToken's uses that module factory + * @return address array which have the list of securityToken's uses that module factory */ function getReputationOfFactory(address _factoryAddress) public view returns(address[]); diff --git a/test/k_module_registry.js b/test/k_module_registry.js index 22965b178..ddeb209f1 100644 --- a/test/k_module_registry.js +++ b/test/k_module_registry.js @@ -52,7 +52,8 @@ contract('ModuleRegistry', accounts => { let I_TickerRegistry; let I_FeatureRegistry; let I_SecurityTokenRegistry; - let I_CappedSTOFactory; + let I_CappedSTOFactory1; + let I_CappedSTOFactory2; let I_STFactory; let I_SecurityToken; let I_CappedSTO; @@ -244,10 +245,10 @@ contract('ModuleRegistry', accounts => { ); - I_CappedSTOFactory = await CappedSTOFactory.new(I_PolyToken.address, 0, 0, 0, { from: account_polymath }); + I_CappedSTOFactory1 = await CappedSTOFactory.new(I_PolyToken.address, 0, 0, 0, { from: account_polymath }); assert.notEqual( - I_CappedSTOFactory.address.valueOf(), + I_CappedSTOFactory1.address.valueOf(), "0x0000000000000000000000000000000000000000", "CappedSTOFactory contract was not deployed" ); @@ -307,11 +308,11 @@ contract('ModuleRegistry', accounts => { assert.equal(tx.logs[0].args._owner, account_polymath); - tx = await I_ModuleRegistry.registerModule(I_CappedSTOFactory.address, { from: account_polymath }); + tx = await I_ModuleRegistry.registerModule(I_CappedSTOFactory1.address, { from: account_polymath }); assert.equal( tx.logs[0].args._moduleFactory, - I_CappedSTOFactory.address, + I_CappedSTOFactory1.address, "CappedSTOFactory is not registerd successfully" ); @@ -373,10 +374,10 @@ contract('ModuleRegistry', accounts => { }); it("Should successfully verify the module -- false", async() => { - let tx = await I_ModuleRegistry.verifyModule(I_CappedSTOFactory.address, false, { from: account_polymath }); + let tx = await I_ModuleRegistry.verifyModule(I_CappedSTOFactory1.address, false, { from: account_polymath }); assert.equal( tx.logs[0].args._moduleFactory, - I_CappedSTOFactory.address, + I_CappedSTOFactory1.address, "Failed in verifying the module" ); assert.equal( @@ -539,7 +540,7 @@ contract('ModuleRegistry', accounts => { let bytesSTO = web3.eth.abi.encodeFunctionCall(functionSignature, [startTime, endTime, cap, rate, fundRaiseType, account_fundsReceiver]); let errorThrown = false; try { - const tx = await I_SecurityToken.addModule(I_CappedSTOFactory.address, bytesSTO, 0, 0, { from: token_owner, gas: 60000000 }); + const tx = await I_SecurityToken.addModule(I_CappedSTOFactory1.address, bytesSTO, 0, 0, { from: token_owner, gas: 60000000 }); } catch(error) { errorThrown = true; console.log(` tx revert -> Module is un-verified`.grey); @@ -549,19 +550,19 @@ contract('ModuleRegistry', accounts => { }); it("Should fail to add module because custom modules not allowed", async() => { - I_CappedSTOFactory = await CappedSTOFactory.new(I_PolyToken.address, 0, 0, 0, { from: token_owner }); + I_CappedSTOFactory2 = await CappedSTOFactory.new(I_PolyToken.address, 0, 0, 0, { from: token_owner }); assert.notEqual( - I_CappedSTOFactory.address.valueOf(), + I_CappedSTOFactory2.address.valueOf(), "0x0000000000000000000000000000000000000000", "CappedSTOFactory contract was not deployed" ); - let tx = await I_ModuleRegistry.registerModule(I_CappedSTOFactory.address, { from: token_owner }); + let tx = await I_ModuleRegistry.registerModule(I_CappedSTOFactory2.address, { from: token_owner }); assert.equal( tx.logs[0].args._moduleFactory, - I_CappedSTOFactory.address, + I_CappedSTOFactory2.address, "CappedSTOFactory is not registerd successfully" ); @@ -573,7 +574,7 @@ contract('ModuleRegistry', accounts => { let errorThrown = false; try { - tx = await I_SecurityToken.addModule(I_CappedSTOFactory.address, bytesSTO, 0, 0, { from: token_owner, gas: 60000000 }); + tx = await I_SecurityToken.addModule(I_CappedSTOFactory2.address, bytesSTO, 0, 0, { from: token_owner, gas: 60000000 }); } catch(error) { errorThrown = true; console.log(` tx revert -> Module is un-verified`.grey); @@ -593,7 +594,7 @@ contract('ModuleRegistry', accounts => { endTime = startTime + duration.days(30); let bytesSTO = web3.eth.abi.encodeFunctionCall(functionSignature, [startTime, endTime, cap, rate, fundRaiseType, account_fundsReceiver]); - let tx = await I_SecurityToken.addModule(I_CappedSTOFactory.address, bytesSTO, 0, 0, { from: token_owner, gas: 60000000 }); + let tx = await I_SecurityToken.addModule(I_CappedSTOFactory2.address, bytesSTO, 0, 0, { from: token_owner, gas: 60000000 }); assert.equal(tx.logs[2].args._type, stoKey, "CappedSTO doesn't get deployed"); assert.equal( @@ -611,6 +612,86 @@ contract('ModuleRegistry', accounts => { }); + describe("Test cases for removeModule()", async() => { + + it("Should fail if msg.sender not curator or owner", async() => { + let errorThrown = false; + try { + await I_ModuleRegistry.removeModule(I_CappedSTOFactory2.address, { from: account_temp }); + } catch(error) { + errorThrown = true; + console.log(` tx revert -> Module is un-verified`.grey); + ensureException(error); + } + assert.ok(errorThrown, message); + }); + + it("Should successfully remove module and delete data if msg.sender is curator", async() => { + let snap = await takeSnapshot(); + + let sto1 = await I_ModuleRegistry.moduleList.call(3,0); + let sto2 = await I_ModuleRegistry.moduleList.call(3,1); + + assert.equal(sto1,I_CappedSTOFactory1.address); + assert.equal(sto2,I_CappedSTOFactory2.address); + assert.equal((await I_ModuleRegistry.getModuleListOfType.call(3)).length, 2); + + let tx = await I_ModuleRegistry.removeModule(sto1, { from: account_polymath }); + + assert.equal(tx.logs[0].args._moduleFactory, sto1, "Event is not properly emitted for _moduleFactory"); + assert.equal(tx.logs[0].args._decisionMaker, account_polymath, "Event is not properly emitted for _decisionMaker"); + + let sto2_end = await I_ModuleRegistry.moduleList.call(3,0); + + // re-ordering + assert.equal(sto2_end,sto2); + // delete related data + assert.equal(await I_ModuleRegistry.registry.call(sto1), 0); + assert.equal(await I_ModuleRegistry.getReputationOfFactory.call(sto1), 0); + assert.equal((await I_ModuleRegistry.getModuleListOfType.call(3)).length, 1); + assert.equal(await I_ModuleRegistry.verified.call(sto1), false); + + await revertToSnapshot(snap); + }); + + it("Should successfully remove module and delete data if msg.sender is owner", async() => { + let sto1 = await I_ModuleRegistry.moduleList.call(3,0); + let sto2 = await I_ModuleRegistry.moduleList.call(3,1); + + assert.equal(sto1,I_CappedSTOFactory1.address); + assert.equal(sto2,I_CappedSTOFactory2.address); + assert.equal((await I_ModuleRegistry.getModuleListOfType.call(3)).length, 2); + + let tx = await I_ModuleRegistry.removeModule(sto2, { from: token_owner }); + + assert.equal(tx.logs[0].args._moduleFactory, sto2, "Event is not properly emitted for _moduleFactory"); + assert.equal(tx.logs[0].args._decisionMaker, token_owner, "Event is not properly emitted for _decisionMaker"); + + let sto1_end = await I_ModuleRegistry.moduleList.call(3,0); + + // re-ordering + assert.equal(sto1_end,sto1); + // delete related data + assert.equal(await I_ModuleRegistry.registry.call(sto2), 0); + assert.equal(await I_ModuleRegistry.getReputationOfFactory.call(sto2), 0); + assert.equal((await I_ModuleRegistry.getModuleListOfType.call(3)).length, 1); + assert.equal(await I_ModuleRegistry.verified.call(sto2), false); + }); + + it("Should fail if module already removed", async() => { + let errorThrown = false; + try { + await I_ModuleRegistry.removeModule(I_CappedSTOFactory2.address, { from: account_polymath }); + } catch(error) { + errorThrown = true; + console.log(` tx revert -> Module is un-verified`.grey); + ensureException(error); + } + assert.ok(errorThrown, message); + }); + + }); + describe("Test cases for IRegistry functionality", async() => { describe("Test cases for reclaiming funds", async() => {