From 940b048c97b7f5d0716e7174bcf07390adf7b1f7 Mon Sep 17 00:00:00 2001 From: Stephane Gosselin Date: Tue, 11 Sep 2018 13:05:00 +0200 Subject: [PATCH 1/4] add controllerTransfer --- contracts/tokens/SecurityToken.sol | 75 ++++++++++++ test/o_security_token.js | 183 ++++++++++++++++++++++++++++- 2 files changed, 253 insertions(+), 5 deletions(-) diff --git a/contracts/tokens/SecurityToken.sol b/contracts/tokens/SecurityToken.sol index 41f18def8..f9ded21b7 100644 --- a/contracts/tokens/SecurityToken.sol +++ b/contracts/tokens/SecurityToken.sol @@ -56,6 +56,11 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr // Use to permanently halt all minting bool public mintingFrozen; + // addresses whitelisted by issuer as controller + mapping (address => bool) public isController; + mapping (address => uint256) private controllerID; + address[] public controllers; + struct ModuleData { bytes32 name; address moduleAddress; @@ -110,6 +115,10 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr event Minted(address indexed to, uint256 amount); event Burnt(address indexed _burner, uint256 _value); + // Events to log controller actions + event LogChangeControllerStatus(address indexed _controller, bool _status); + event LogControllerTransfer(address indexed _controller, address indexed _from, address indexed _to, uint256 _amount, bytes _data); + // Require msg.sender to be the specified module type modifier onlyModule(uint8 _moduleType) { bool isModuleType = false; @@ -145,6 +154,14 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr _; } + /** + * @notice Revert if called by account which is not a controller + */ + modifier onlyController() { + require(isController[msg.sender]); + _; + } + /** * @notice Constructor * @param _name Name of the SecurityToken @@ -721,4 +738,62 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr return _getValueAt(checkpointBalances[_investor], _checkpointId, balanceOf(_investor)); } + /** + * @notice Use to get the list of controller addresses + * @return address array containing the list of controller addresses + */ + function getControllers() public view returns (address[]) { + return controllers; + } + + /** + * @notice Use by the issuer ot set the status of a controller addresses + * @param _controller address of the controller + * @param _status bool representing new status to set + */ + function setControllerStatus(address _controller, bool _status) public onlyOwner { + require(isController[_controller] != _status); + if (_status == true) { + isController[_controller] = true; + controllerID[_controller] = controllers.length; + controllers.push(_controller); + } else { + isController[_controller] = false; + uint256 i = controllerID[_controller]; + address last = controllers[controllers.length - 1]; + controllers[i] = last; + controllerID[last] = i; + + delete controllers[controllers.length - 1]; + delete controllerID[_controller]; + controllers.length--; + } + emit LogChangeControllerStatus(_controller, _status); + } + + /** + * @notice Use by a controller to execute a foced transfer + * @param _from address from which to take tokens + * @param _to address where to send tokens + * @param _value amount of tokens to transfer + * @param _data data attached to the transfer by controller for event + */ + function controllerTransfer(address _from, address _to, uint256 _value, bytes _data) public onlyController { + require(_value <= balances[_from]); + require(_to != address(0)); + + _adjustInvestorCount(_from, _to, _value); + _adjustBalanceCheckpoints(_from); + _adjustBalanceCheckpoints(_to); + + // Consider passing setting to record change in state but not transfer check (could be a part of standard messaging module) + // require(verifyTransfer(_from, _to, _value), "Transfer is not valid"); + + balances[_from] = balances[_from].sub(_value); + balances[_to] = balances[_to].add(_value); + + emit LogControllerTransfer(msg.sender, _from, _to, _value, _data); + emit Transfer(_from, _to, _value); + } + } diff --git a/test/o_security_token.js b/test/o_security_token.js index b37e1f020..4f274e3ff 100644 --- a/test/o_security_token.js +++ b/test/o_security_token.js @@ -38,6 +38,7 @@ contract('SecurityToken', accounts => { let account_fundsReceiver; let account_delegate; let account_temp; + let account_controller; let balanceOfReceiver; // investor Details @@ -123,16 +124,17 @@ contract('SecurityToken', accounts => { // Accounts setup account_polymath = accounts[0]; account_issuer = accounts[1]; - account_investor1 = accounts[9]; - account_investor2 = accounts[6]; - account_investor3 = accounts[7]; + account_affiliate1 = accounts[2]; + account_affiliate2 = accounts[3]; account_fundsReceiver = accounts[4]; account_delegate = accounts[5]; + account_investor2 = accounts[6]; + account_investor3 = accounts[7]; account_temp = accounts[8]; - account_affiliate1 = accounts[2]; - account_affiliate2 = accounts[3]; + account_investor1 = accounts[9]; token_owner = account_issuer; + account_controller = account_temp; // ----------- POLYMATH NETWORK Configuration ------------ @@ -1268,4 +1270,175 @@ contract('SecurityToken', accounts => { }); }); + describe("Force Transfer", async() => { + + it("Should fail to set controller status because msg.sender not owner", async() => { + let errorThrown = false; + try { + await I_SecurityToken.setControllerStatus(account_controller, true, {from: account_controller}); + } catch (error) { + console.log(` tx revert -> msg.sender not owner`.grey); + errorThrown = true; + ensureException(error); + } + assert.ok(errorThrown, message); + }); + + it("Should fail to set controller status because status unchanged", async() => { + let errorThrown = false; + try { + await I_SecurityToken.setControllerStatus(account_controller, false, {from: token_owner}); + } catch (error) { + console.log(` tx revert -> status unchanged`.grey); + errorThrown = true; + ensureException(error); + } + assert.ok(errorThrown, message); + }); + + it("Should successfully set controller status to true", async() => { + let tx1 = await I_SecurityToken.setControllerStatus(account_investor1, true, {from: token_owner}); + let tx2 = await I_SecurityToken.setControllerStatus(account_controller, true, {from: token_owner}); + let tx3 = await I_SecurityToken.setControllerStatus(account_investor2, true, {from: token_owner}); + + // check event + assert.equal(account_investor1, tx1.logs[0].args._controller, "Event not emitted as expected"); + assert.equal(true, tx1.logs[0].args._status, "Event not emitted as expected"); + assert.equal(account_controller, tx2.logs[0].args._controller, "Event not emitted as expected"); + assert.equal(true, tx2.logs[0].args._status, "Event not emitted as expected"); + assert.equal(account_investor2, tx3.logs[0].args._controller, "Event not emitted as expected"); + assert.equal(true, tx3.logs[0].args._status, "Event not emitted as expected"); + + // check status + let status1 = await I_SecurityToken.isController(account_investor1); + let status2 = await I_SecurityToken.isController(account_controller); + let status3 = await I_SecurityToken.isController(account_investor2); + assert.equal(true, status1, "Status not set correctly"); + assert.equal(true, status2, "Status not set correctly"); + assert.equal(true, status3, "Status not set correctly"); + + // check array + let controllers = await I_SecurityToken.getControllers(); + assert.equal(account_investor1, controllers[0], "Array of controllers not correctly set"); + assert.equal(account_controller, controllers[1], "Array of controllers not correctly set"); + assert.equal(account_investor2, controllers[controllers.length - 1], "Array of controllers not correctly set"); + }); + + it("Should successfully set controller status to false", async() => { + let tx1 = await I_SecurityToken.setControllerStatus(account_investor1, false, {from: token_owner}); + + // check event + assert.equal(account_investor1, tx1.logs[0].args._controller, "Event not emitted as expected"); + assert.equal(false, tx1.logs[0].args._status, "Event not emitted as expected"); + + // check status + let status1 = await I_SecurityToken.isController(account_investor1); + let status2 = await I_SecurityToken.isController(account_controller); + let status3 = await I_SecurityToken.isController(account_investor2); + assert.equal(false, status1, "Status not set correctly"); + assert.equal(true, status2, "Status not set correctly"); + assert.equal(true, status3, "Status not set correctly"); + + // check array + let controllers = await I_SecurityToken.getControllers(); + assert.equal(account_investor2, controllers[0], "Array of controllers not correctly set"); + assert.equal(account_controller, controllers[controllers.length - 1], "Array of controllers not correctly set"); + + let tx2 = await I_SecurityToken.setControllerStatus(account_investor2, false, {from: token_owner}); + + // check event + assert.equal(account_investor2, tx2.logs[0].args._controller, "Event not emitted as expected"); + assert.equal(false, tx2.logs[0].args._status, "Event not emitted as expected"); + + // check status + status1 = await I_SecurityToken.isController(account_investor1); + status2 = await I_SecurityToken.isController(account_controller); + status3 = await I_SecurityToken.isController(account_investor2); + assert.equal(false, status1, "Status not set correctly"); + assert.equal(true, status2, "Status not set correctly"); + assert.equal(false, status3, "Status not set correctly"); + + // check array + controllers = await I_SecurityToken.getControllers(); + assert.equal(account_controller, controllers[0], "Array of controllers not correctly set"); + assert.equal(account_controller, controllers[controllers.length - 1], "Array of controllers not correctly set"); + }); + + it("Should fail to controllerTransfer because not approved controller", async() => { + let errorThrown1 = false; + try { + await I_SecurityToken.controllerTransfer(account_investor1, account_investor2, web3.utils.toWei("10", "ether"), "reason", {from: account_investor1}); + } catch (error) { + console.log(` tx revert -> not approved controller`.grey); + errorThrown1 = true; + ensureException(error); + } + assert.ok(errorThrown1, message); + + let errorThrown2 = false; + try { + await I_SecurityToken.controllerTransfer(account_investor1, account_investor2, web3.utils.toWei("10", "ether"), "reason", {from: account_investor2}); + } catch (error) { + console.log(` tx revert -> not approved controller`.grey); + errorThrown2 = true; + ensureException(error); + } + assert.ok(errorThrown2, message); + }); + + it("Should fail to controllerTransfer because insufficient balance", async() => { + let errorThrown = false; + try { + await I_SecurityToken.controllerTransfer(account_investor2, account_investor1, web3.utils.toWei("10", "ether"), "reason", {from: account_controller}); + } catch (error) { + console.log(` tx revert -> insufficient balance`.grey); + errorThrown = true; + ensureException(error); + } + assert.ok(errorThrown, message); + }); + + it("Should fail to controllerTransfer because recipient is zero address", async() => { + let errorThrown = false; + try { + await I_SecurityToken.controllerTransfer(account_investor1, '0x0000000000000000000000000000000000000000', web3.utils.toWei("10", "ether"), "reason", {from: account_controller}); + } catch (error) { + console.log(` tx revert -> recipient is zero address`.grey); + errorThrown = true; + ensureException(error); + } + assert.ok(errorThrown, message); + }); + + it("Should successfully controllerTransfer", async() => { + let sender = account_investor1; + let receiver = account_investor2; + + let start_investorCount = await I_SecurityToken.investorCount.call(); + let start_balInv1 = await I_SecurityToken.balanceOf.call(account_investor1); + let start_balInv2 = await I_SecurityToken.balanceOf.call(account_investor2); + + let tx = await I_SecurityToken.controllerTransfer(account_investor1, account_investor2, web3.utils.toWei("10", "ether"), "reason", {from: account_controller}); + + let end_investorCount = await I_SecurityToken.investorCount.call(); + let end_balInv1 = await I_SecurityToken.balanceOf.call(account_investor1); + let end_balInv2 = await I_SecurityToken.balanceOf.call(account_investor2); + + assert.equal(start_investorCount.add(1).toNumber(), end_investorCount.toNumber(), "Investor count not changed"); + assert.equal(start_balInv1.sub(web3.utils.toWei("10", "ether")).toNumber(), end_balInv1.toNumber(), "Investor balance not changed"); + assert.equal(start_balInv2.add(web3.utils.toWei("10", "ether")).toNumber(), end_balInv2.toNumber(), "Investor balance not changed"); + + assert.equal(account_controller, tx.logs[0].args._controller, "Event not emitted as expected"); + assert.equal(account_investor1, tx.logs[0].args._from, "Event not emitted as expected"); + assert.equal(account_investor2, tx.logs[0].args._to, "Event not emitted as expected"); + assert.equal(web3.utils.toWei("10", "ether"), tx.logs[0].args._amount, "Event not emitted as expected"); + assert.equal("reason", web3.utils.hexToUtf8(tx.logs[0].args._data), "Event not emitted as expected"); + + assert.equal(account_investor1, tx.logs[1].args.from, "Event not emitted as expected"); + assert.equal(account_investor2, tx.logs[1].args.to, "Event not emitted as expected"); + assert.equal(web3.utils.toWei("10", "ether"), tx.logs[1].args.value, "Event not emitted as expected"); + }); + + }); + }); From 86c17a887b5fb271a95c01d529c3def781982217 Mon Sep 17 00:00:00 2001 From: Stephane Gosselin Date: Thu, 13 Sep 2018 08:47:20 -0400 Subject: [PATCH 2/4] code review changes --- contracts/FeatureRegistry.sol | 1 + contracts/interfaces/ISecurityToken.sol | 20 +++ contracts/tokens/SecurityToken.sol | 57 +++---- test/o_security_token.js | 189 +++++++++++++----------- 4 files changed, 144 insertions(+), 123 deletions(-) diff --git a/contracts/FeatureRegistry.sol b/contracts/FeatureRegistry.sol index ab9f208a2..b1ae17b84 100644 --- a/contracts/FeatureRegistry.sol +++ b/contracts/FeatureRegistry.sol @@ -24,6 +24,7 @@ contract FeatureRegistry is IFeatureRegistry, ReclaimTokens { /** * @notice change a feature status + * @dev feature status is set to false by default * @param _nameKey is the key for the feature status mapping * @param _newStatus is the new feature status */ diff --git a/contracts/interfaces/ISecurityToken.sol b/contracts/interfaces/ISecurityToken.sol index 944a611d0..5bcd94f4a 100644 --- a/contracts/interfaces/ISecurityToken.sol +++ b/contracts/interfaces/ISecurityToken.sol @@ -160,6 +160,11 @@ interface ISecurityToken { */ function freezeMinting() external; + /** + * @notice Permanently freeze controller functionality of this security token. + */ + function freezeController() external; + /** * @notice mints new tokens and assigns them to the target investors. * Can only be called by the STO attached to the token or by the Issuer (Security Token contract owner) @@ -196,4 +201,19 @@ interface ISecurityToken { uint256 _budget ) external; + /** + * @notice Use by the issuer ot set the controller addresses + * @param _controller address of the controller + */ + function setController(address _controller) external; + + /** + * @notice Use by a controller to execute a foced transfer + * @param _from address from which to take tokens + * @param _to address where to send tokens + * @param _value amount of tokens to transfer + * @param _data data attached to the transfer by controller to emit in event + */ + function controllerTransfer(address _from, address _to, uint256 _value, bytes _data) external returns(bool); + } diff --git a/contracts/tokens/SecurityToken.sol b/contracts/tokens/SecurityToken.sol index f9ded21b7..54aef31e8 100644 --- a/contracts/tokens/SecurityToken.sol +++ b/contracts/tokens/SecurityToken.sol @@ -56,10 +56,11 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr // Use to permanently halt all minting bool public mintingFrozen; - // addresses whitelisted by issuer as controller - mapping (address => bool) public isController; - mapping (address => uint256) private controllerID; - address[] public controllers; + // Use to permanently halt controller actions + bool public controllerFrozen; + + // address whitelisted by issuer as controller + address public controller; struct ModuleData { bytes32 name; @@ -116,8 +117,9 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr event Burnt(address indexed _burner, uint256 _value); // Events to log controller actions - event LogChangeControllerStatus(address indexed _controller, bool _status); + event LogSetController(address indexed _oldController, address indexed _newController); event LogControllerTransfer(address indexed _controller, address indexed _from, address indexed _to, uint256 _amount, bytes _data); + event LogFreezeController(uint256 _timestamp); // Require msg.sender to be the specified module type modifier onlyModule(uint8 _moduleType) { @@ -158,7 +160,8 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr * @notice Revert if called by account which is not a controller */ modifier onlyController() { - require(isController[msg.sender]); + require(msg.sender == controller); + require(!controllerFrozen); _; } @@ -739,36 +742,23 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr } /** - * @notice Use to get the list of controller addresses - * @return address array containing the list of controller addresses + * @notice Use by the issuer ot set the controller addresses + * @param _controller address of the controller */ - function getControllers() public view returns (address[]) { - return controllers; + function setController(address _controller) public onlyOwner { + require(!controllerFrozen); + emit LogSetController(controller, _controller); + controller = _controller; } /** - * @notice Use by the issuer ot set the status of a controller addresses - * @param _controller address of the controller - * @param _status bool representing new status to set + * @notice Permanently freeze controller functionality of this security token. */ - function setControllerStatus(address _controller, bool _status) public onlyOwner { - require(isController[_controller] != _status); - if (_status == true) { - isController[_controller] = true; - controllerID[_controller] = controllers.length; - controllers.push(_controller); - } else { - isController[_controller] = false; - uint256 i = controllerID[_controller]; - address last = controllers[controllers.length - 1]; - controllers[i] = last; - controllerID[last] = i; - - delete controllers[controllers.length - 1]; - delete controllerID[_controller]; - controllers.length--; - } - emit LogChangeControllerStatus(_controller, _status); + function freezeController() external isEnabled("freezeControllerAllowed") onlyOwner { + require(!controllerFrozen); + controllerFrozen = true; + delete controller; + emit LogFreezeController(now); } /** @@ -776,9 +766,9 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr * @param _from address from which to take tokens * @param _to address where to send tokens * @param _value amount of tokens to transfer - * @param _data data attached to the transfer by controller for event + * @param _data data attached to the transfer by controller to emit in event */ - function controllerTransfer(address _from, address _to, uint256 _value, bytes _data) public onlyController { + function controllerTransfer(address _from, address _to, uint256 _value, bytes _data) public onlyController returns(bool) { require(_value <= balances[_from]); require(_to != address(0)); @@ -794,6 +784,7 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr emit LogControllerTransfer(msg.sender, _from, _to, _value, _data); emit Transfer(_from, _to, _value); + return true; } } diff --git a/test/o_security_token.js b/test/o_security_token.js index 4f274e3ff..0ed266b9b 100644 --- a/test/o_security_token.js +++ b/test/o_security_token.js @@ -39,6 +39,7 @@ contract('SecurityToken', accounts => { let account_delegate; let account_temp; let account_controller; + let address_zero = "0x0000000000000000000000000000000000000000"; let balanceOfReceiver; // investor Details @@ -153,7 +154,7 @@ contract('SecurityToken', accounts => { assert.notEqual( I_ModuleRegistry.address.valueOf(), - "0x0000000000000000000000000000000000000000", + address_zero, "ModuleRegistry contract was not deployed" ); @@ -163,7 +164,7 @@ contract('SecurityToken', accounts => { assert.notEqual( I_GeneralTransferManagerFactory.address.valueOf(), - "0x0000000000000000000000000000000000000000", + address_zero, "GeneralTransferManagerFactory contract was not deployed" ); @@ -173,7 +174,7 @@ contract('SecurityToken', accounts => { assert.notEqual( I_GeneralPermissionManagerFactory.address.valueOf(), - "0x0000000000000000000000000000000000000000", + address_zero, "GeneralDelegateManagerFactory contract was not deployed" ); @@ -183,7 +184,7 @@ contract('SecurityToken', accounts => { assert.notEqual( I_CappedSTOFactory.address.valueOf(), - "0x0000000000000000000000000000000000000000", + address_zero, "CappedSTOFactory contract was not deployed" ); @@ -208,7 +209,7 @@ contract('SecurityToken', accounts => { assert.notEqual( I_TickerRegistry.address.valueOf(), - "0x0000000000000000000000000000000000000000", + address_zero, "TickerRegistry contract was not deployed", ); @@ -218,7 +219,7 @@ contract('SecurityToken', accounts => { assert.notEqual( I_STFactory.address.valueOf(), - "0x0000000000000000000000000000000000000000", + address_zero, "STFactory contract was not deployed", ); @@ -237,7 +238,7 @@ contract('SecurityToken', accounts => { assert.notEqual( I_SecurityTokenRegistry.address.valueOf(), - "0x0000000000000000000000000000000000000000", + address_zero, "SecurityTokenRegistry contract was not deployed", ); @@ -252,7 +253,7 @@ contract('SecurityToken', accounts => { assert.notEqual( I_FeatureRegistry.address.valueOf(), - "0x0000000000000000000000000000000000000000", + address_zero, "FeatureRegistry contract was not deployed", ); @@ -311,7 +312,7 @@ contract('SecurityToken', accounts => { assert.notEqual( I_GeneralTransferManager.address.valueOf(), - "0x0000000000000000000000000000000000000000", + address_zero, "GeneralTransferManager contract was not deployed", ); @@ -598,7 +599,7 @@ contract('SecurityToken', accounts => { it("Should get the modules of the securityToken by index (not added into the security token yet)", async () => { let moduleData = await I_SecurityToken.getModule.call(permissionManagerKey, 0); assert.equal(web3.utils.toAscii(moduleData[0]).replace(/\u0000/g, ''), ""); - assert.equal(moduleData[1], "0x0000000000000000000000000000000000000000"); + assert.equal(moduleData[1], address_zero); }); it("Should get the modules of the securityToken by name", async () => { @@ -1275,7 +1276,7 @@ contract('SecurityToken', accounts => { it("Should fail to set controller status because msg.sender not owner", async() => { let errorThrown = false; try { - await I_SecurityToken.setControllerStatus(account_controller, true, {from: account_controller}); + await I_SecurityToken.setController(account_controller, {from: account_controller}); } catch (error) { console.log(` tx revert -> msg.sender not owner`.grey); errorThrown = true; @@ -1284,84 +1285,28 @@ contract('SecurityToken', accounts => { assert.ok(errorThrown, message); }); - it("Should fail to set controller status because status unchanged", async() => { - let errorThrown = false; - try { - await I_SecurityToken.setControllerStatus(account_controller, false, {from: token_owner}); - } catch (error) { - console.log(` tx revert -> status unchanged`.grey); - errorThrown = true; - ensureException(error); - } - assert.ok(errorThrown, message); - }); - - it("Should successfully set controller status to true", async() => { - let tx1 = await I_SecurityToken.setControllerStatus(account_investor1, true, {from: token_owner}); - let tx2 = await I_SecurityToken.setControllerStatus(account_controller, true, {from: token_owner}); - let tx3 = await I_SecurityToken.setControllerStatus(account_investor2, true, {from: token_owner}); + it("Should successfully set controller", async() => { + let tx1 = await I_SecurityToken.setController(account_controller, {from: token_owner}); // check event - assert.equal(account_investor1, tx1.logs[0].args._controller, "Event not emitted as expected"); - assert.equal(true, tx1.logs[0].args._status, "Event not emitted as expected"); - assert.equal(account_controller, tx2.logs[0].args._controller, "Event not emitted as expected"); - assert.equal(true, tx2.logs[0].args._status, "Event not emitted as expected"); - assert.equal(account_investor2, tx3.logs[0].args._controller, "Event not emitted as expected"); - assert.equal(true, tx3.logs[0].args._status, "Event not emitted as expected"); + assert.equal(address_zero, tx1.logs[0].args._oldController, "Event not emitted as expected"); + assert.equal(account_controller, tx1.logs[0].args._newController, "Event not emitted as expected"); - // check status - let status1 = await I_SecurityToken.isController(account_investor1); - let status2 = await I_SecurityToken.isController(account_controller); - let status3 = await I_SecurityToken.isController(account_investor2); - assert.equal(true, status1, "Status not set correctly"); - assert.equal(true, status2, "Status not set correctly"); - assert.equal(true, status3, "Status not set correctly"); - - // check array - let controllers = await I_SecurityToken.getControllers(); - assert.equal(account_investor1, controllers[0], "Array of controllers not correctly set"); - assert.equal(account_controller, controllers[1], "Array of controllers not correctly set"); - assert.equal(account_investor2, controllers[controllers.length - 1], "Array of controllers not correctly set"); - }); - - it("Should successfully set controller status to false", async() => { - let tx1 = await I_SecurityToken.setControllerStatus(account_investor1, false, {from: token_owner}); + let tx2 = await I_SecurityToken.setController(address_zero, {from: token_owner}); // check event - assert.equal(account_investor1, tx1.logs[0].args._controller, "Event not emitted as expected"); - assert.equal(false, tx1.logs[0].args._status, "Event not emitted as expected"); - - // check status - let status1 = await I_SecurityToken.isController(account_investor1); - let status2 = await I_SecurityToken.isController(account_controller); - let status3 = await I_SecurityToken.isController(account_investor2); - assert.equal(false, status1, "Status not set correctly"); - assert.equal(true, status2, "Status not set correctly"); - assert.equal(true, status3, "Status not set correctly"); - - // check array - let controllers = await I_SecurityToken.getControllers(); - assert.equal(account_investor2, controllers[0], "Array of controllers not correctly set"); - assert.equal(account_controller, controllers[controllers.length - 1], "Array of controllers not correctly set"); + assert.equal(account_controller, tx2.logs[0].args._oldController, "Event not emitted as expected"); + assert.equal(address_zero, tx2.logs[0].args._newController, "Event not emitted as expected"); - let tx2 = await I_SecurityToken.setControllerStatus(account_investor2, false, {from: token_owner}); + let tx3 = await I_SecurityToken.setController(account_controller, {from: token_owner}); // check event - assert.equal(account_investor2, tx2.logs[0].args._controller, "Event not emitted as expected"); - assert.equal(false, tx2.logs[0].args._status, "Event not emitted as expected"); + assert.equal(address_zero, tx3.logs[0].args._oldController, "Event not emitted as expected"); + assert.equal(account_controller, tx3.logs[0].args._newController, "Event not emitted as expected"); // check status - status1 = await I_SecurityToken.isController(account_investor1); - status2 = await I_SecurityToken.isController(account_controller); - status3 = await I_SecurityToken.isController(account_investor2); - assert.equal(false, status1, "Status not set correctly"); - assert.equal(true, status2, "Status not set correctly"); - assert.equal(false, status3, "Status not set correctly"); - - // check array - controllers = await I_SecurityToken.getControllers(); - assert.equal(account_controller, controllers[0], "Array of controllers not correctly set"); - assert.equal(account_controller, controllers[controllers.length - 1], "Array of controllers not correctly set"); + let controller = await I_SecurityToken.controller.call(); + assert.equal(account_controller, controller, "Status not set correctly"); }); it("Should fail to controllerTransfer because not approved controller", async() => { @@ -1374,16 +1319,6 @@ contract('SecurityToken', accounts => { ensureException(error); } assert.ok(errorThrown1, message); - - let errorThrown2 = false; - try { - await I_SecurityToken.controllerTransfer(account_investor1, account_investor2, web3.utils.toWei("10", "ether"), "reason", {from: account_investor2}); - } catch (error) { - console.log(` tx revert -> not approved controller`.grey); - errorThrown2 = true; - ensureException(error); - } - assert.ok(errorThrown2, message); }); it("Should fail to controllerTransfer because insufficient balance", async() => { @@ -1401,7 +1336,7 @@ contract('SecurityToken', accounts => { it("Should fail to controllerTransfer because recipient is zero address", async() => { let errorThrown = false; try { - await I_SecurityToken.controllerTransfer(account_investor1, '0x0000000000000000000000000000000000000000', web3.utils.toWei("10", "ether"), "reason", {from: account_controller}); + await I_SecurityToken.controllerTransfer(account_investor1, address_zero, web3.utils.toWei("10", "ether"), "reason", {from: account_controller}); } catch (error) { console.log(` tx revert -> recipient is zero address`.grey); errorThrown = true; @@ -1439,6 +1374,80 @@ contract('SecurityToken', accounts => { assert.equal(web3.utils.toWei("10", "ether"), tx.logs[1].args.value, "Event not emitted as expected"); }); + it("Should fail to freeze controller functionality because not owner", async() => { + let errorThrown = false; + try { + await I_SecurityToken.freezeController({from: account_investor1}); + } catch (error) { + console.log(` tx revert -> not owner`.grey); + errorThrown = true; + ensureException(error); + } + assert.ok(errorThrown, message); + }); + + it("Should fail to freeze controller functionality because freezeControllerAllowed not activated", async() => { + let errorThrown = false; + try { + await I_SecurityToken.freezeController({from: token_owner}); + } catch (error) { + console.log(` tx revert -> freezeControllerAllowed not activated`.grey); + errorThrown = true; + ensureException(error); + } + assert.ok(errorThrown, message); + }); + + it("Should successfully freeze controller functionality", async() => { + let tx1 = await I_FeatureRegistry.setFeatureStatus("freezeControllerAllowed", true, {from: account_polymath}); + + // check event + assert.equal("freezeControllerAllowed", tx1.logs[0].args._nameKey, "Event not emitted as expected"); + assert.equal(true, tx1.logs[0].args._newStatus, "Event not emitted as expected"); + + let tx2 = await I_SecurityToken.freezeController({from: token_owner}); + + // check state + assert.equal(address_zero, await I_SecurityToken.controller.call(), "State not changed"); + assert.equal(true, await I_SecurityToken.controllerFrozen.call(), "State not changed"); + }); + + it("Should fail to freeze controller functionality because already frozen", async() => { + let errorThrown = false; + try { + await I_SecurityToken.freezeController({from: token_owner}); + } catch (error) { + console.log(` tx revert -> already frozen`.grey); + errorThrown = true; + ensureException(error); + } + assert.ok(errorThrown, message); + }); + + it("Should fail to set controller because controller functionality frozen", async() => { + let errorThrown = false; + try { + await I_SecurityToken.setController(account_controller, {from: token_owner}); + } catch (error) { + console.log(` tx revert -> msg.sender not owner`.grey); + errorThrown = true; + ensureException(error); + } + assert.ok(errorThrown, message); + }); + + it("Should fail to controllerTransfer because controller functionality frozen", async() => { + let errorThrown = false; + try { + await I_SecurityToken.controllerTransfer(account_investor1, account_investor2, web3.utils.toWei("10", "ether"), "reason", {from: account_controller}); + } catch (error) { + console.log(` tx revert -> recipient is zero address`.grey); + errorThrown = true; + ensureException(error); + } + assert.ok(errorThrown, message); + }); + }); }); From 0f5a5c707659cea97ecb9f71d18e0acf99ccf316 Mon Sep 17 00:00:00 2001 From: Stephane Gosselin Date: Thu, 13 Sep 2018 09:49:13 -0400 Subject: [PATCH 3/4] add verifyTransfer to event log for controllerTransfer --- contracts/tokens/SecurityToken.sol | 8 ++++---- test/o_security_token.js | 2 ++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/contracts/tokens/SecurityToken.sol b/contracts/tokens/SecurityToken.sol index 54aef31e8..81b09be7b 100644 --- a/contracts/tokens/SecurityToken.sol +++ b/contracts/tokens/SecurityToken.sol @@ -118,7 +118,7 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr // Events to log controller actions event LogSetController(address indexed _oldController, address indexed _newController); - event LogControllerTransfer(address indexed _controller, address indexed _from, address indexed _to, uint256 _amount, bytes _data); + event LogControllerTransfer(address indexed _controller, address indexed _from, address indexed _to, uint256 _amount, bool _verifyTransfer, bytes _data); event LogFreezeController(uint256 _timestamp); // Require msg.sender to be the specified module type @@ -194,6 +194,7 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr transferFunctions[bytes4(keccak256("transferFrom(address,address,uint256)"))] = true; transferFunctions[bytes4(keccak256("mint(address,uint256)"))] = true; transferFunctions[bytes4(keccak256("burn(uint256)"))] = true; + transferFunctions[bytes4(keccak256("controllerTransfer(address,address,uint256,bytes)"))] = true; } /** @@ -776,13 +777,12 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr _adjustBalanceCheckpoints(_from); _adjustBalanceCheckpoints(_to); - // Consider passing setting to record change in state but not transfer check (could be a part of standard messaging module) - // require(verifyTransfer(_from, _to, _value), "Transfer is not valid"); + bool TMcheck = verifyTransfer(_from, _to, _value); balances[_from] = balances[_from].sub(_value); balances[_to] = balances[_to].add(_value); - emit LogControllerTransfer(msg.sender, _from, _to, _value, _data); + emit LogControllerTransfer(msg.sender, _from, _to, _value, TMcheck, _data); emit Transfer(_from, _to, _value); return true; } diff --git a/test/o_security_token.js b/test/o_security_token.js index 0ed266b9b..e26773861 100644 --- a/test/o_security_token.js +++ b/test/o_security_token.js @@ -1367,6 +1367,8 @@ contract('SecurityToken', accounts => { assert.equal(account_investor1, tx.logs[0].args._from, "Event not emitted as expected"); assert.equal(account_investor2, tx.logs[0].args._to, "Event not emitted as expected"); assert.equal(web3.utils.toWei("10", "ether"), tx.logs[0].args._amount, "Event not emitted as expected"); + console.log(tx.logs[0].args._verifyTransfer); + assert.equal(false, tx.logs[0].args._verifyTransfer, "Event not emitted as expected"); assert.equal("reason", web3.utils.hexToUtf8(tx.logs[0].args._data), "Event not emitted as expected"); assert.equal(account_investor1, tx.logs[1].args.from, "Event not emitted as expected"); From 754747714797fd968838ac2b118974acb95ff080 Mon Sep 17 00:00:00 2001 From: Adam Dossa Date: Sat, 15 Sep 2018 23:17:51 +0100 Subject: [PATCH 4/4] Rename functions --- contracts/interfaces/ISecurityToken.sol | 11 +++++-- contracts/tokens/SecurityToken.sol | 35 +++++++++++------------ test/o_security_token.js | 38 ++++++++++++------------- 3 files changed, 44 insertions(+), 40 deletions(-) diff --git a/contracts/interfaces/ISecurityToken.sol b/contracts/interfaces/ISecurityToken.sol index 5bcd94f4a..834727cc0 100644 --- a/contracts/interfaces/ISecurityToken.sol +++ b/contracts/interfaces/ISecurityToken.sol @@ -202,18 +202,23 @@ interface ISecurityToken { ) external; /** - * @notice Use by the issuer ot set the controller addresses + * @notice Use by the issuer to set the controller addresses * @param _controller address of the controller */ function setController(address _controller) external; /** - * @notice Use by a controller to execute a foced transfer + * @notice Use by a controller to execute a forced transfer * @param _from address from which to take tokens * @param _to address where to send tokens * @param _value amount of tokens to transfer * @param _data data attached to the transfer by controller to emit in event */ - function controllerTransfer(address _from, address _to, uint256 _value, bytes _data) external returns(bool); + function forceTransfer(address _from, address _to, uint256 _value, bytes _data) external returns(bool); + /** + * @notice Use by the issuer to permanently disable controller functionality + * @dev enabled via feature switch "disableControllerAllowed" + */ + function disableController() external; } diff --git a/contracts/tokens/SecurityToken.sol b/contracts/tokens/SecurityToken.sol index 81b09be7b..329a916ca 100644 --- a/contracts/tokens/SecurityToken.sol +++ b/contracts/tokens/SecurityToken.sol @@ -57,7 +57,7 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr bool public mintingFrozen; // Use to permanently halt controller actions - bool public controllerFrozen; + bool public controllerDisabled; // address whitelisted by issuer as controller address public controller; @@ -118,8 +118,8 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr // Events to log controller actions event LogSetController(address indexed _oldController, address indexed _newController); - event LogControllerTransfer(address indexed _controller, address indexed _from, address indexed _to, uint256 _amount, bool _verifyTransfer, bytes _data); - event LogFreezeController(uint256 _timestamp); + event LogForceTransfer(address indexed _controller, address indexed _from, address indexed _to, uint256 _amount, bool _verifyTransfer, bytes _data); + event LogDisableController(uint256 _timestamp); // Require msg.sender to be the specified module type modifier onlyModule(uint8 _moduleType) { @@ -161,7 +161,7 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr */ modifier onlyController() { require(msg.sender == controller); - require(!controllerFrozen); + require(!controllerDisabled); _; } @@ -194,7 +194,7 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr transferFunctions[bytes4(keccak256("transferFrom(address,address,uint256)"))] = true; transferFunctions[bytes4(keccak256("mint(address,uint256)"))] = true; transferFunctions[bytes4(keccak256("burn(uint256)"))] = true; - transferFunctions[bytes4(keccak256("controllerTransfer(address,address,uint256,bytes)"))] = true; + transferFunctions[bytes4(keccak256("forceTransfer(address,address,uint256,bytes)"))] = true; } /** @@ -747,19 +747,20 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr * @param _controller address of the controller */ function setController(address _controller) public onlyOwner { - require(!controllerFrozen); + require(!controllerDisabled); emit LogSetController(controller, _controller); controller = _controller; } /** - * @notice Permanently freeze controller functionality of this security token. + * @notice Use by the issuer to permanently disable controller functionality + * @dev enabled via feature switch "disableControllerAllowed" */ - function freezeController() external isEnabled("freezeControllerAllowed") onlyOwner { - require(!controllerFrozen); - controllerFrozen = true; + function disableController() external isEnabled("disableControllerAllowed") onlyOwner { + require(!controllerDisabled); + controllerDisabled = true; delete controller; - emit LogFreezeController(now); + emit LogDisableController(now); } /** @@ -769,20 +770,18 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr * @param _value amount of tokens to transfer * @param _data data attached to the transfer by controller to emit in event */ - function controllerTransfer(address _from, address _to, uint256 _value, bytes _data) public onlyController returns(bool) { - require(_value <= balances[_from]); - require(_to != address(0)); - + function forceTransfer(address _from, address _to, uint256 _value, bytes _data) public onlyController returns(bool) { _adjustInvestorCount(_from, _to, _value); + bool verified = verifyTransfer(_from, _to, _value); _adjustBalanceCheckpoints(_from); _adjustBalanceCheckpoints(_to); - bool TMcheck = verifyTransfer(_from, _to, _value); - + require(_to != address(0)); + require(_value <= balances[_from]); balances[_from] = balances[_from].sub(_value); balances[_to] = balances[_to].add(_value); - emit LogControllerTransfer(msg.sender, _from, _to, _value, TMcheck, _data); + emit LogForceTransfer(msg.sender, _from, _to, _value, verified, _data); emit Transfer(_from, _to, _value); return true; } diff --git a/test/o_security_token.js b/test/o_security_token.js index e26773861..bd2d02f8c 100644 --- a/test/o_security_token.js +++ b/test/o_security_token.js @@ -1309,10 +1309,10 @@ contract('SecurityToken', accounts => { assert.equal(account_controller, controller, "Status not set correctly"); }); - it("Should fail to controllerTransfer because not approved controller", async() => { + it("Should fail to forceTransfer because not approved controller", async() => { let errorThrown1 = false; try { - await I_SecurityToken.controllerTransfer(account_investor1, account_investor2, web3.utils.toWei("10", "ether"), "reason", {from: account_investor1}); + await I_SecurityToken.forceTransfer(account_investor1, account_investor2, web3.utils.toWei("10", "ether"), "reason", {from: account_investor1}); } catch (error) { console.log(` tx revert -> not approved controller`.grey); errorThrown1 = true; @@ -1321,10 +1321,10 @@ contract('SecurityToken', accounts => { assert.ok(errorThrown1, message); }); - it("Should fail to controllerTransfer because insufficient balance", async() => { + it("Should fail to forceTransfer because insufficient balance", async() => { let errorThrown = false; try { - await I_SecurityToken.controllerTransfer(account_investor2, account_investor1, web3.utils.toWei("10", "ether"), "reason", {from: account_controller}); + await I_SecurityToken.forceTransfer(account_investor2, account_investor1, web3.utils.toWei("10", "ether"), "reason", {from: account_controller}); } catch (error) { console.log(` tx revert -> insufficient balance`.grey); errorThrown = true; @@ -1333,10 +1333,10 @@ contract('SecurityToken', accounts => { assert.ok(errorThrown, message); }); - it("Should fail to controllerTransfer because recipient is zero address", async() => { + it("Should fail to forceTransfer because recipient is zero address", async() => { let errorThrown = false; try { - await I_SecurityToken.controllerTransfer(account_investor1, address_zero, web3.utils.toWei("10", "ether"), "reason", {from: account_controller}); + await I_SecurityToken.forceTransfer(account_investor1, address_zero, web3.utils.toWei("10", "ether"), "reason", {from: account_controller}); } catch (error) { console.log(` tx revert -> recipient is zero address`.grey); errorThrown = true; @@ -1345,7 +1345,7 @@ contract('SecurityToken', accounts => { assert.ok(errorThrown, message); }); - it("Should successfully controllerTransfer", async() => { + it("Should successfully forceTransfer", async() => { let sender = account_investor1; let receiver = account_investor2; @@ -1353,7 +1353,7 @@ contract('SecurityToken', accounts => { let start_balInv1 = await I_SecurityToken.balanceOf.call(account_investor1); let start_balInv2 = await I_SecurityToken.balanceOf.call(account_investor2); - let tx = await I_SecurityToken.controllerTransfer(account_investor1, account_investor2, web3.utils.toWei("10", "ether"), "reason", {from: account_controller}); + let tx = await I_SecurityToken.forceTransfer(account_investor1, account_investor2, web3.utils.toWei("10", "ether"), "reason", {from: account_controller}); let end_investorCount = await I_SecurityToken.investorCount.call(); let end_balInv1 = await I_SecurityToken.balanceOf.call(account_investor1); @@ -1379,7 +1379,7 @@ contract('SecurityToken', accounts => { it("Should fail to freeze controller functionality because not owner", async() => { let errorThrown = false; try { - await I_SecurityToken.freezeController({from: account_investor1}); + await I_SecurityToken.disableController({from: account_investor1}); } catch (error) { console.log(` tx revert -> not owner`.grey); errorThrown = true; @@ -1388,12 +1388,12 @@ contract('SecurityToken', accounts => { assert.ok(errorThrown, message); }); - it("Should fail to freeze controller functionality because freezeControllerAllowed not activated", async() => { + it("Should fail to freeze controller functionality because disableControllerAllowed not activated", async() => { let errorThrown = false; try { - await I_SecurityToken.freezeController({from: token_owner}); + await I_SecurityToken.disableController({from: token_owner}); } catch (error) { - console.log(` tx revert -> freezeControllerAllowed not activated`.grey); + console.log(` tx revert -> disableControllerAllowed not activated`.grey); errorThrown = true; ensureException(error); } @@ -1401,23 +1401,23 @@ contract('SecurityToken', accounts => { }); it("Should successfully freeze controller functionality", async() => { - let tx1 = await I_FeatureRegistry.setFeatureStatus("freezeControllerAllowed", true, {from: account_polymath}); + let tx1 = await I_FeatureRegistry.setFeatureStatus("disableControllerAllowed", true, {from: account_polymath}); // check event - assert.equal("freezeControllerAllowed", tx1.logs[0].args._nameKey, "Event not emitted as expected"); + assert.equal("disableControllerAllowed", tx1.logs[0].args._nameKey, "Event not emitted as expected"); assert.equal(true, tx1.logs[0].args._newStatus, "Event not emitted as expected"); - let tx2 = await I_SecurityToken.freezeController({from: token_owner}); + let tx2 = await I_SecurityToken.disableController({from: token_owner}); // check state assert.equal(address_zero, await I_SecurityToken.controller.call(), "State not changed"); - assert.equal(true, await I_SecurityToken.controllerFrozen.call(), "State not changed"); + assert.equal(true, await I_SecurityToken.controllerDisabled.call(), "State not changed"); }); it("Should fail to freeze controller functionality because already frozen", async() => { let errorThrown = false; try { - await I_SecurityToken.freezeController({from: token_owner}); + await I_SecurityToken.disableController({from: token_owner}); } catch (error) { console.log(` tx revert -> already frozen`.grey); errorThrown = true; @@ -1438,10 +1438,10 @@ contract('SecurityToken', accounts => { assert.ok(errorThrown, message); }); - it("Should fail to controllerTransfer because controller functionality frozen", async() => { + it("Should fail to forceTransfer because controller functionality frozen", async() => { let errorThrown = false; try { - await I_SecurityToken.controllerTransfer(account_investor1, account_investor2, web3.utils.toWei("10", "ether"), "reason", {from: account_controller}); + await I_SecurityToken.forceTransfer(account_investor1, account_investor2, web3.utils.toWei("10", "ether"), "reason", {from: account_controller}); } catch (error) { console.log(` tx revert -> recipient is zero address`.grey); errorThrown = true;