Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Module Registry removeModule() function #244

Merged
merged 8 commits into from
Sep 6, 2018
Merged

Module Registry removeModule() function #244

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a9a7dcd
add removeModule() function in ModuleRegistry.sol
Sep 3, 2018
2ca6f97
Merge remote-tracking branch 'origin/development-1.5.0' into unverify
Sep 3, 2018
7a341fe
typo in comment
Sep 3, 2018
f694238
fix naming
Sep 3, 2018
e74ca24
Merge remote-tracking branch 'refs/remotes/origin/unverify' into unve…
Sep 3, 2018
3570f49
fix index 0 workaround
Sep 3, 2018
07385c7
gas optimizations
Sep 3, 2018
e852543
Merge branch 'development-1.5.0' into unverify
pabloruiz55 Sep 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 44 additions & 3 deletions contracts/ModuleRegistry.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 moduleFacorty in the moduleList
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo => moduleFactory

mapping(address => uint8) public moduleListIndex;
// contains the list of verified modules
mapping (address => bool) public verified;
// Contains the list of the available tags corresponds to the module type
Expand All @@ -31,10 +33,16 @@ 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)
{
moduleList[1].push(address(0x0000000000000000000000000000000000000000));
moduleList[2].push(address(0x0000000000000000000000000000000000000000));
moduleList[3].push(address(0x0000000000000000000000000000000000000000));
moduleList[4].push(address(0x0000000000000000000000000000000000000000));
}

/**
Expand Down Expand Up @@ -67,14 +75,47 @@ 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 kind = moduleFactory.getType();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of "kind" use "type"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type is a reserved keyword documentation

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, moduleFactoryType or _type, don't want to be introducing different names.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay fixing

require(kind != 0, "Factory kind should not equal to 0");
registry[_moduleFactory] = kind;
moduleListIndex[_moduleFactory] = uint8(moduleList[kind].length);
moduleList[kind].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) {
require(registry[_moduleFactory] != 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];
require(index != 0, "ModuleFactory index is not valid");
uint8 kind = registry[_moduleFactory];
uint8 last = uint8(moduleList[kind].length - 1);
address temp = moduleList[kind][last];

// pop from array and re-order
moduleList[kind][index] = temp;
moduleListIndex[temp] = index;
delete moduleList[kind][last];
moduleList[kind].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
Expand Down
9 changes: 8 additions & 1 deletion contracts/interfaces/IModuleRegistry.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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[]);

Expand Down
125 changes: 111 additions & 14 deletions test/k_module_registry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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"
);
Expand Down Expand Up @@ -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"
);

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Expand All @@ -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"
);

Expand All @@ -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);
Expand All @@ -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(
Expand All @@ -611,6 +612,102 @@ 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 sto1id = await I_ModuleRegistry.moduleListIndex.call(I_CappedSTOFactory1.address);
let sto2id = await I_ModuleRegistry.moduleListIndex.call(I_CappedSTOFactory2.address);
let sto1 = await I_ModuleRegistry.moduleList.call(3,sto1id);
let sto2 = await I_ModuleRegistry.moduleList.call(3,sto2id);

assert.equal(sto1,I_CappedSTOFactory1.address);
assert.equal(sto2,I_CappedSTOFactory2.address);
assert.equal(3,(await I_ModuleRegistry.getModuleListOfType.call(3)).length);

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 sto1id_end = await I_ModuleRegistry.moduleListIndex.call(sto1);
let sto2id_end = await I_ModuleRegistry.moduleListIndex.call(sto2);
let sto1_end = await I_ModuleRegistry.moduleList.call(3,sto1id_end);
let sto2_end = await I_ModuleRegistry.moduleList.call(3,sto2id_end);

// re-ordering
assert.equal(sto2_end,sto2);
// set to zero address
assert.equal(sto1_end,'0x0000000000000000000000000000000000000000');
// delete related data
assert.equal(await I_ModuleRegistry.registry.call(sto1), 0);
assert.equal(0,(await I_ModuleRegistry.getReputationOfFactory.call(sto1)));
assert.equal(2,(await I_ModuleRegistry.getModuleListOfType.call(3)).length);
assert.equal(await I_ModuleRegistry.moduleListIndex.call(sto1), 0);
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 sto1id = await I_ModuleRegistry.moduleListIndex.call(I_CappedSTOFactory1.address);
let sto2id = await I_ModuleRegistry.moduleListIndex.call(I_CappedSTOFactory2.address);
let sto1 = await I_ModuleRegistry.moduleList.call(3,sto1id);
let sto2 = await I_ModuleRegistry.moduleList.call(3,sto2id);

assert.equal(sto1,I_CappedSTOFactory1.address);
assert.equal(sto2,I_CappedSTOFactory2.address);
assert.equal(3,(await I_ModuleRegistry.getModuleListOfType.call(3)).length);

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 sto1id_end = await I_ModuleRegistry.moduleListIndex.call(sto1);
let sto2id_end = await I_ModuleRegistry.moduleListIndex.call(sto2);
let sto1_end = await I_ModuleRegistry.moduleList.call(3,sto1id_end);
let sto2_end = await I_ModuleRegistry.moduleList.call(3,sto2id_end);

// re-ordering
assert.equal(sto2_end,'0x0000000000000000000000000000000000000000');
// set to zero address
assert.equal(sto1_end,sto1);
// delete related data
assert.equal(await I_ModuleRegistry.registry.call(sto2), 0);
assert.equal(0,(await I_ModuleRegistry.getReputationOfFactory.call(sto2)));
assert.equal(2,(await I_ModuleRegistry.getModuleListOfType.call(3)).length);
assert.equal(await I_ModuleRegistry.moduleListIndex.call(sto2), 0);
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() => {
Expand Down