Skip to content

Commit

Permalink
Fix test cases and add remaining require error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
adamdossa committed Apr 24, 2018
1 parent 12889e6 commit 7735bd4
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 29 deletions.
10 changes: 5 additions & 5 deletions contracts/interfaces/IModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 5 additions & 5 deletions contracts/modules/TransferManager/GeneralTransferManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
Expand All @@ -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
Expand All @@ -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[]) {
Expand Down
10 changes: 5 additions & 5 deletions contracts/tokens/SecurityToken.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
_;
}
Expand Down Expand Up @@ -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);
}

Expand Down
16 changes: 4 additions & 12 deletions test/general_transfer_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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();
});
Expand All @@ -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 () => {
Expand Down
2 changes: 1 addition & 1 deletion truffle.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module.exports = {
host: 'localhost',
port: 8545,
network_id: '*', // Match any network id
gas: 5500000,
gas: 4500000,
},
mainnet: {
host: 'localhost',
Expand Down

0 comments on commit 7735bd4

Please sign in to comment.