From 7735bd44a6a21ed579e6ecfbb4c1ae7c8b93bc2d Mon Sep 17 00:00:00 2001 From: Adam Dossa Date: Tue, 24 Apr 2018 12:24:46 +0100 Subject: [PATCH] Fix test cases and add remaining require error messages --- contracts/interfaces/IModule.sol | 10 +++++----- .../GeneralPermissionManager.sol | 2 +- .../TransferManager/GeneralTransferManager.sol | 10 +++++----- contracts/tokens/SecurityToken.sol | 10 +++++----- test/general_transfer_manager.js | 16 ++++------------ truffle.js | 2 +- 6 files changed, 21 insertions(+), 29 deletions(-) diff --git a/contracts/interfaces/IModule.sol b/contracts/interfaces/IModule.sol index 842518460..c0c744943 100644 --- a/contracts/interfaces/IModule.sol +++ b/contracts/interfaces/IModule.sol @@ -27,29 +27,29 @@ contract IModule { modifier withPerm(bytes32 _perm) { bool isOwner = msg.sender == ISecurityToken(securityToken).owner(); bool isFactory = msg.sender == factory; - require(isOwner||isFactory||ISecurityToken(securityToken).checkPermission(msg.sender, address(this), _perm)); + require(isOwner||isFactory||ISecurityToken(securityToken).checkPermission(msg.sender, address(this), _perm), "Permission check failed"); _; } modifier onlyOwner { - require(msg.sender == ISecurityToken(securityToken).owner()); + require(msg.sender == ISecurityToken(securityToken).owner(), "Sender is not owner"); _; } modifier onlyFactory { - require(msg.sender == factory); + require(msg.sender == factory, "Sender is not factory"); _; } modifier onlyFactoryOwner { - require(msg.sender == IModuleFactory(factory).owner()); + require(msg.sender == IModuleFactory(factory).owner(), "Sender is not factory owner"); _; } function getPermissions() public view returns(bytes32[]); function takeFee(uint256 _amount) public withPerm(FEE_ADMIN) returns(bool) { - require(polyToken.transferFrom(address(this), IModuleFactory(factory).owner(), _amount)); + require(polyToken.transferFrom(address(this), IModuleFactory(factory).owner(), _amount), "Unable to take fee"); return true; } } diff --git a/contracts/modules/PermissionManager/GeneralPermissionManager.sol b/contracts/modules/PermissionManager/GeneralPermissionManager.sol index 4e9c420c0..451111444 100644 --- a/contracts/modules/PermissionManager/GeneralPermissionManager.sol +++ b/contracts/modules/PermissionManager/GeneralPermissionManager.sol @@ -80,7 +80,7 @@ contract GeneralPermissionManager is IPermissionManager { withPerm(CHANGE_PERMISSION) returns(bool) { - require(delegateDetails[_delegate] != bytes32(0)); + require(delegateDetails[_delegate] != bytes32(0), "Delegate details not set"); perms[_module][_delegate][_perm] = _valid; emit LogChangePermission(_delegate, _module, _perm, _valid, now); return true; diff --git a/contracts/modules/TransferManager/GeneralTransferManager.sol b/contracts/modules/TransferManager/GeneralTransferManager.sol index d15a35dc1..fd42ab8b7 100644 --- a/contracts/modules/TransferManager/GeneralTransferManager.sol +++ b/contracts/modules/TransferManager/GeneralTransferManager.sol @@ -132,8 +132,8 @@ contract GeneralTransferManager is ITransferManager { uint256[] _fromTimes, uint256[] _toTimes ) public withPerm(WHITELIST) { - require(_investors.length == _fromTimes.length); - require(_fromTimes.length == _toTimes.length); + require(_investors.length == _fromTimes.length, "Mismatched input lengths"); + require(_fromTimes.length == _toTimes.length, "Mismatched input lengths"); for (uint256 i = 0; i < _investors.length; i++) { modifyWhitelist(_investors[i], _fromTimes[i], _toTimes[i]); } @@ -151,8 +151,8 @@ contract GeneralTransferManager is ITransferManager { * @param _s issuer signature */ function modifyWhitelistSigned(address _investor, uint256 _fromTime, uint256 _toTime, uint256 _validFrom, uint256 _validTo, uint8 _v, bytes32 _r, bytes32 _s) public { - require(_validFrom <= now); - require(_validTo >= now); + require(_validFrom <= now, "ValidFrom is too early"); + require(_validTo >= now, "ValidTo is too late"); bytes32 hash = keccak256(this, _investor, _fromTime, _toTime, _validFrom, _validTo); checkSig(hash, _v, _r, _s); //Passing a _time == 0 into this function, is equivalent to removing the _investor from the whitelist @@ -164,7 +164,7 @@ contract GeneralTransferManager is ITransferManager { //Check that the signature is valid //sig should be signing - _investor, _fromTime & _toTime and be signed by the issuer address address signer = ecrecover(keccak256("\x19Ethereum Signed Message:\n32", _hash), _v, _r, _s); - require(signer == ISecurityToken(securityToken).owner() || signer == signingAddress); + require(signer == ISecurityToken(securityToken).owner() || signer == signingAddress, "Incorrect signer"); } function getPermissions() public view returns(bytes32[]) { diff --git a/contracts/tokens/SecurityToken.sol b/contracts/tokens/SecurityToken.sol index 0621f9a0f..d4c883da8 100644 --- a/contracts/tokens/SecurityToken.sol +++ b/contracts/tokens/SecurityToken.sol @@ -64,9 +64,9 @@ contract SecurityToken is ISecurityToken, StandardToken, DetailedERC20 { isModuleType = isModuleType || (modules[_moduleType][i].moduleAddress == msg.sender); } if (_fallback && !isModuleType) { - require(msg.sender == owner); + require(msg.sender == owner, "Sender is not owner"); } else { - require(isModuleType); + require(isModuleType, "Sender is not correct module type"); } _; } @@ -200,9 +200,9 @@ contract SecurityToken is ISecurityToken, StandardToken, DetailedERC20 { * @dev allows owner to approve more POLY to one of the modules */ function changeModuleBudget(uint8 _moduleType, uint8 _moduleIndex, uint256 _budget) public onlyOwner { - require(_moduleType != 0); - require(_moduleIndex < modules[_moduleType].length); - require(polyToken.approve(modules[_moduleType][_moduleIndex].moduleAddress, _budget)); + require(_moduleType != 0, "Module type cannot be zero"); + require(_moduleIndex < modules[_moduleType].length, "Incorrrect module index"); + require(polyToken.approve(modules[_moduleType][_moduleIndex].moduleAddress, _budget), "Insufficient balance to approve"); emit LogModuleBudgetChanged(_moduleType, modules[_moduleType][_moduleIndex].moduleAddress, _budget); } diff --git a/test/general_transfer_manager.js b/test/general_transfer_manager.js index 2455d4366..10134e3db 100644 --- a/test/general_transfer_manager.js +++ b/test/general_transfer_manager.js @@ -177,7 +177,7 @@ contract('GeneralTransferManager', accounts => { // Step 7: Deploy the STversionProxy contract - I_STVersion = await STVersion.new(I_GeneralTransferManagerFactory.address, I_GeneralPermissionManagerFactory.address); + I_STVersion = await STVersion.new(I_GeneralTransferManagerFactory.address); assert.notEqual( I_STVersion.address.valueOf(), @@ -240,12 +240,12 @@ contract('GeneralTransferManager', accounts => { LogAddModule.watch(function(error, log){ resolve(log);}); }); - // Verify that GeneralPermissionManager module get added successfully or not - assert.equal(log.args._type.toNumber(), 1); + // Verify that GeneralTransferManager module get added successfully or not + assert.equal(log.args._type.toNumber(), 2); assert.equal( web3.utils.toAscii(log.args._name) .replace(/\u0000/g, ''), - "GeneralPermissionManager" + "GeneralTransferManager" ); LogAddModule.stopWatching(); }); @@ -260,14 +260,6 @@ contract('GeneralTransferManager', accounts => { "GeneralTransferManager contract was not deployed", ); - moduleData = await I_SecurityToken.modules(1, 0); - I_GeneralPermissionManager = GeneralPermissionManager.at(moduleData[1]); - - assert.notEqual( - I_GeneralPermissionManager.address.valueOf(), - "0x0000000000000000000000000000000000000000", - "GeneralDelegateManager contract was not deployed", - ); }); it("Should successfully attach the STO factory with the security token", async () => { diff --git a/truffle.js b/truffle.js index f6937fec7..65590aee5 100644 --- a/truffle.js +++ b/truffle.js @@ -10,7 +10,7 @@ module.exports = { host: 'localhost', port: 8545, network_id: '*', // Match any network id - gas: 5500000, + gas: 4500000, }, mainnet: { host: 'localhost',