From 1197ab6e99389d36e2d1c3c104baa4618de563aa Mon Sep 17 00:00:00 2001 From: Dmitriy Kostin Date: Wed, 24 Oct 2018 19:53:05 +0300 Subject: [PATCH 1/4] Make `changeHolderPercentage` in PercentageTM a permissioned --- CLI/commands/common/permissions_list.js | 2 +- .../PercentageTransferManager.sol | 2 +- docs/permissions_list.md | 2 +- test/l_percentage_transfer_manager.js | 33 +++++++++++++++++++ 4 files changed, 36 insertions(+), 3 deletions(-) diff --git a/CLI/commands/common/permissions_list.js b/CLI/commands/common/permissions_list.js index f5779612b..4d2b58cf7 100644 --- a/CLI/commands/common/permissions_list.js +++ b/CLI/commands/common/permissions_list.js @@ -68,7 +68,7 @@ function getPermissionList() { modifyWhitelist: "WHITELIST", modifyWhitelistMulti: "WHITELIST", setAllowPrimaryIssuance: "ADMIN", - changeHolderPercentage: "ONLY_OWNER" + changeHolderPercentage: "ADMIN" }, LockupVolumeRestrictionTM: { addLockup: "ADMIN", diff --git a/contracts/modules/TransferManager/PercentageTransferManager.sol b/contracts/modules/TransferManager/PercentageTransferManager.sol index 4238cff14..d2f5e7804 100644 --- a/contracts/modules/TransferManager/PercentageTransferManager.sol +++ b/contracts/modules/TransferManager/PercentageTransferManager.sol @@ -92,7 +92,7 @@ contract PercentageTransferManager is ITransferManager { * @notice sets the maximum percentage that an individual token holder can hold * @param _maxHolderPercentage is the new maximum percentage (multiplied by 10**16) */ - function changeHolderPercentage(uint256 _maxHolderPercentage) public onlyOwner { + function changeHolderPercentage(uint256 _maxHolderPercentage) public withPerm(ADMIN) { emit ModifyHolderPercentage(maxHolderPercentage, _maxHolderPercentage); maxHolderPercentage = _maxHolderPercentage; } diff --git a/docs/permissions_list.md b/docs/permissions_list.md index 4592474ea..dc34c5a55 100644 --- a/docs/permissions_list.md +++ b/docs/permissions_list.md @@ -203,7 +203,7 @@ changeHolderPercentage() - onlyOwner() + withPerm(ADMIN) LockupVolumeRestrictionTM diff --git a/test/l_percentage_transfer_manager.js b/test/l_percentage_transfer_manager.js index 3a62463ac..d674c0f1d 100644 --- a/test/l_percentage_transfer_manager.js +++ b/test/l_percentage_transfer_manager.js @@ -22,6 +22,7 @@ contract("PercentageTransferManager", accounts => { let account_investor2; let account_investor3; let account_investor4; + let account_delegate; // investor Details let fromTime = latestTime(); @@ -92,6 +93,7 @@ contract("PercentageTransferManager", accounts => { account_investor1 = accounts[7]; account_investor2 = accounts[8]; account_investor3 = accounts[9]; + account_delegate = accounts[6]; let instances = await setUpPolymathNetwork(account_polymath, token_owner); @@ -166,6 +168,18 @@ contract("PercentageTransferManager", accounts => { let moduleData = (await I_SecurityToken.getModulesByType(2))[0]; I_GeneralTransferManager = GeneralTransferManager.at(moduleData); }); + + it("Should successfully attach the General permission manager factory with the security token", async () => { + const tx = await I_SecurityToken.addModule(I_GeneralPermissionManagerFactory.address, "0x", 0, 0, { from: token_owner }); + assert.equal(tx.logs[2].args._types[0].toNumber(), delegateManagerKey, "General Permission Manager doesn't get deployed"); + assert.equal( + web3.utils.toAscii(tx.logs[2].args._name).replace(/\u0000/g, ""), + "GeneralPermissionManager", + "GeneralPermissionManagerFactory module was not added" + ); + I_GeneralPermissionManager = GeneralPermissionManager.at(tx.logs[2].args._module); + }); + }); describe("Buy tokens using on-chain whitelist", async () => { @@ -341,6 +355,25 @@ contract("PercentageTransferManager", accounts => { assert.equal((await I_PercentageTransferManager.maxHolderPercentage()).toNumber(), 100 * 10 ** 16); }); + it("Should provide the permission", async() => { + let tx = await I_GeneralPermissionManager.changePermission( + account_delegate, + I_PercentageTransferManager.address, + "ADMIN", + true, + {from: token_owner} + ); + assert.equal(tx.logs[0].args._delegate, account_delegate); + }); + + it("Admin shuold be able to modify holder percentage to 90", async () => { + // Add the Investor in to the whitelist + // Mint some tokens + await I_PercentageTransferManager.changeHolderPercentage(90 * 10 ** 16, { from: account_delegate }); + + assert.equal((await I_PercentageTransferManager.maxHolderPercentage()).toNumber(), 90 * 10 ** 16); + }); + it("Should be able to transfer between existing token holders up to limit", async () => { await I_PercentageTransferManager.modifyWhitelist(account_investor3, false, { from: token_owner }); await I_SecurityToken.transfer(account_investor3, web3.utils.toWei("2", "ether"), { from: account_investor1 }); From dea6189a655b8bb90382819270480fb5c1f01f23 Mon Sep 17 00:00:00 2001 From: Dmitriy Kostin Date: Wed, 24 Oct 2018 22:36:01 +0300 Subject: [PATCH 2/4] Add delegate l_percentage_transfer_manager.js --- test/l_percentage_transfer_manager.js | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/test/l_percentage_transfer_manager.js b/test/l_percentage_transfer_manager.js index d674c0f1d..b79e10e22 100644 --- a/test/l_percentage_transfer_manager.js +++ b/test/l_percentage_transfer_manager.js @@ -58,6 +58,7 @@ contract("PercentageTransferManager", accounts => { const tokenDetails = "This is equity type of issuance"; const decimals = 18; const contact = "team@polymath.network"; + const delegateDetails = "Hello I am legit delegate"; // Module key const delegateManagerKey = 1; @@ -347,12 +348,9 @@ contract("PercentageTransferManager", accounts => { ) }); - it("Modify holder percentage to 100", async () => { - // Add the Investor in to the whitelist - // Mint some tokens - await I_PercentageTransferManager.changeHolderPercentage(100 * 10 ** 16, { from: token_owner }); - - assert.equal((await I_PercentageTransferManager.maxHolderPercentage()).toNumber(), 100 * 10 ** 16); + 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 provide the permission", async() => { @@ -366,12 +364,12 @@ contract("PercentageTransferManager", accounts => { assert.equal(tx.logs[0].args._delegate, account_delegate); }); - it("Admin shuold be able to modify holder percentage to 90", async () => { + it("Modify holder percentage to 100", async () => { // Add the Investor in to the whitelist // Mint some tokens - await I_PercentageTransferManager.changeHolderPercentage(90 * 10 ** 16, { from: account_delegate }); + await I_PercentageTransferManager.changeHolderPercentage(100 * 10 ** 16, { from: account_delegate }); - assert.equal((await I_PercentageTransferManager.maxHolderPercentage()).toNumber(), 90 * 10 ** 16); + assert.equal((await I_PercentageTransferManager.maxHolderPercentage()).toNumber(), 100 * 10 ** 16); }); it("Should be able to transfer between existing token holders up to limit", async () => { From 9536d8aa2ef81ea2c1846d94528b19758904d781 Mon Sep 17 00:00:00 2001 From: Dmitriy Kostin Date: Thu, 25 Oct 2018 10:07:54 +0300 Subject: [PATCH 3/4] Remove redundant getters: fixed review comments --- docs/permissions_list.md | 3 +-- test/l_percentage_transfer_manager.js | 6 ++++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/permissions_list.md b/docs/permissions_list.md index dc34c5a55..b49c95a0e 100644 --- a/docs/permissions_list.md +++ b/docs/permissions_list.md @@ -199,11 +199,10 @@ setAllowPrimaryIssuance() - withPerm(ADMIN) + withPerm(ADMIN) changeHolderPercentage() - withPerm(ADMIN) LockupVolumeRestrictionTM diff --git a/test/l_percentage_transfer_manager.js b/test/l_percentage_transfer_manager.js index b79e10e22..3f87fdcb1 100644 --- a/test/l_percentage_transfer_manager.js +++ b/test/l_percentage_transfer_manager.js @@ -348,6 +348,12 @@ contract("PercentageTransferManager", accounts => { ) }); + it("Should not be able to modify holder percentage to 100", async () => { + await catchRevert( + I_PercentageTransferManager.changeHolderPercentage(100 * 10 ** 16, { from: account_delegate }) + ) + }); + 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); From d123f37a8f88a7eca4e96a419154a80a863bc206 Mon Sep 17 00:00:00 2001 From: Mudit Gupta Date: Thu, 25 Oct 2018 12:42:56 +0530 Subject: [PATCH 4/4] Update test/l_percentage_transfer_manager.js --- test/l_percentage_transfer_manager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/l_percentage_transfer_manager.js b/test/l_percentage_transfer_manager.js index 3f87fdcb1..488be5820 100644 --- a/test/l_percentage_transfer_manager.js +++ b/test/l_percentage_transfer_manager.js @@ -348,7 +348,7 @@ contract("PercentageTransferManager", accounts => { ) }); - it("Should not be able to modify holder percentage to 100", async () => { + it("Should not be able to modify holder percentage to 100 - Unauthorized msg.sender", async () => { await catchRevert( I_PercentageTransferManager.changeHolderPercentage(100 * 10 ** 16, { from: account_delegate }) )