From 1c5f539f1aeaf0f18a57a7dd62cc4704fa339202 Mon Sep 17 00:00:00 2001 From: Mudit Gupta Date: Fri, 15 Mar 2019 18:25:09 +0530 Subject: [PATCH 1/5] MATM optimizations --- .../ManualApprovalTransferManager.sol | 138 +++++++++--------- 1 file changed, 72 insertions(+), 66 deletions(-) diff --git a/contracts/modules/TransferManager/ManualApprovalTransferManager.sol b/contracts/modules/TransferManager/ManualApprovalTransferManager.sol index f660ac639..f53259eed 100644 --- a/contracts/modules/TransferManager/ManualApprovalTransferManager.sol +++ b/contracts/modules/TransferManager/ManualApprovalTransferManager.sol @@ -72,7 +72,7 @@ contract ManualApprovalTransferManager is ITransferManager { return bytes4(0); } - /** + /** * @notice Used to verify the transfer transaction and allow a manually approved transqaction to bypass other restrictions * @param _from Address of the sender * @param _to Address of the receiver @@ -82,13 +82,14 @@ contract ManualApprovalTransferManager is ITransferManager { function verifyTransfer(address _from, address _to, uint256 _amount, bytes /* _data */, bool _isTransfer) public returns(Result) { // function must only be called by the associated security token if _isTransfer == true require(_isTransfer == false || msg.sender == securityToken, "Sender is not the owner"); - - if (!paused && approvalIndex[_from][_to] != 0) { - uint256 index = approvalIndex[_from][_to] - 1; + uint256 index = approvalIndex[_from][_to]; + if (!paused && index != 0) { + index--; //Actual index is stored index - 1. ManualApproval storage approval = approvals[index]; - if ((approval.expiryTime >= now) && (approval.allowance >= _amount)) { + uint256 allowance = approval.allowance; + if ((approval.expiryTime >= now) && (allowance >= _amount)) { if (_isTransfer) { - approval.allowance = approval.allowance.sub(_amount); + approval.allowance = allowance - _amount; } return Result.VALID; } @@ -110,8 +111,8 @@ contract ManualApprovalTransferManager is ITransferManager { uint256 _allowance, uint256 _expiryTime, bytes32 _description - ) - external + ) + external withPerm(TRANSFER_APPROVAL) { _addManualApproval(_from, _to, _allowance, _expiryTime, _description); @@ -121,8 +122,9 @@ contract ManualApprovalTransferManager is ITransferManager { require(_to != address(0), "Invalid to address"); require(_expiryTime > now, "Invalid expiry time"); require(_allowance > 0, "Invalid allowance"); - if (approvalIndex[_from][_to] != 0) { - uint256 index = approvalIndex[_from][_to] - 1; + uint256 index = approvalIndex[_from][_to]; + if (index != 0) { + index--; //Actual index is stored index - 1. require(approvals[index].expiryTime < now || approvals[index].allowance == 0, "Approval already exists"); _revokeManualApproval(_from, _to); } @@ -135,7 +137,7 @@ contract ManualApprovalTransferManager is ITransferManager { * @notice Adds mutiple manual approvals in batch * @param _from is the address array from which transfers are approved * @param _to is the address array to which transfers are approved - * @param _allowances is the array of approved amounts + * @param _allowances is the array of approved amounts * @param _expiryTimes is the array of the times until which eath transfer is allowed * @param _descriptions is the description array for these manual approvals */ @@ -145,8 +147,8 @@ contract ManualApprovalTransferManager is ITransferManager { uint256[] _allowances, uint256[] _expiryTimes, bytes32[] _descriptions - ) - external + ) + external withPerm(TRANSFER_APPROVAL) { _checkInputLengthArray(_from, _to, _allowances, _expiryTimes, _descriptions); @@ -160,69 +162,70 @@ contract ManualApprovalTransferManager is ITransferManager { * @param _from is the address from which transfers are approved * @param _to is the address to which transfers are approved * @param _expiryTime is the time until which the transfer is allowed - * @param _changedAllowance is the changed allowance + * @param _changeInAllowance is the change in allowance * @param _description Description about the manual approval - * @param _change uint values which tells whether the allowances will be increased (1) or decreased (0) + * @param _increase tells whether the allowances will be increased (true) or decreased (false). * or any value when there is no change in allowances */ function modifyManualApproval( address _from, address _to, uint256 _expiryTime, - uint256 _changedAllowance, + uint256 _changeInAllowance, bytes32 _description, - uint8 _change - ) + bool _increase + ) external withPerm(TRANSFER_APPROVAL) { - _modifyManualApproval(_from, _to, _expiryTime, _changedAllowance, _description, _change); + _modifyManualApproval(_from, _to, _expiryTime, _changeInAllowance, _description, _increase); } function _modifyManualApproval( address _from, address _to, uint256 _expiryTime, - uint256 _changedAllowance, + uint256 _changeInAllowance, bytes32 _description, - uint8 _change - ) - internal + bool _increase + ) + internal { require(_to != address(0), "Invalid to address"); /*solium-disable-next-line security/no-block-members*/ require(_expiryTime > now, "Invalid expiry time"); - require(approvalIndex[_from][_to] != 0, "Approval not present"); - uint256 index = approvalIndex[_from][_to] - 1; + uint256 index = approvalIndex[_from][_to]; + require(index != 0, "Approval not present"); + index--; //Index is stored in an incremented form. 0 represnts non existant. ManualApproval storage approval = approvals[index]; - require(approval.allowance != 0 && approval.expiryTime > now, "Not allowed"); - uint256 currentAllowance = approval.allowance; - uint256 newAllowance; - if (_change == 1) { - // Allowance get increased - newAllowance = currentAllowance.add(_changedAllowance); - approval.allowance = newAllowance; - } else if (_change == 0) { - // Allowance get decreased - if (_changedAllowance > currentAllowance) { - newAllowance = 0; - approval.allowance = newAllowance; + uint256 allowance = approval.allowance; + uint256 expiryTime = approval.expiryTime; + require(allowance != 0 && expiryTime > now, "Not allowed"); + + if (_changeInAllowance > 0) { + if (_increase) { + // Allowance get increased + allowance = allowance.add(_changeInAllowance); } else { - newAllowance = currentAllowance.sub(_changedAllowance); - approval.allowance = newAllowance; + // Allowance get decreased + if (_changeInAllowance >= allowance) { + allowance = 0; + } else { + allowance = allowance - _changeInAllowance; + } } - } else { - // No change in the Allowance - newAllowance = currentAllowance; + approval.allowance = allowance; } + // Greedy storage technique - if (approval.expiryTime != _expiryTime) { + if (expiryTime != _expiryTime) { approval.expiryTime = _expiryTime; } if (approval.description != _description) { approval.description = _description; } - emit ModifyManualApproval(_from, _to, _expiryTime, newAllowance, _description, msg.sender); + + emit ModifyManualApproval(_from, _to, _expiryTime, allowance, _description, msg.sender); } /** @@ -230,9 +233,9 @@ contract ManualApprovalTransferManager is ITransferManager { * @param _from is the address array from which transfers are approved * @param _to is the address array to which transfers are approved * @param _expiryTimes is the array of the times until which eath transfer is allowed - * @param _changedAllowances is the array of approved amounts + * @param _changedAllowances is the array of approved amounts * @param _descriptions is the description array for these manual approvals - * @param _changes Array of uint values which tells whether the allowances will be increased (1) or decreased (0) + * @param _increase Array of bool values which tells whether the allowances will be increased (true) or decreased (false) * or any value when there is no change in allowances */ function modifyManualApprovalMulti( @@ -241,15 +244,15 @@ contract ManualApprovalTransferManager is ITransferManager { uint256[] _expiryTimes, uint256[] _changedAllowances, bytes32[] _descriptions, - uint8[] _changes + bool[] _increase ) public withPerm(TRANSFER_APPROVAL) { _checkInputLengthArray(_from, _to, _changedAllowances, _expiryTimes, _descriptions); - require(_changes.length == _changedAllowances.length, "Input length array mismatch"); + require(_increase.length == _changedAllowances.length, "Input length array mismatch"); for (uint256 i = 0; i < _from.length; i++) { - _modifyManualApproval(_from[i], _to[i], _expiryTimes[i], _changedAllowances[i], _descriptions[i], _changes[i]); + _modifyManualApproval(_from[i], _to[i], _expiryTimes[i], _changedAllowances[i], _descriptions[i], _increase[i]); } } @@ -263,13 +266,14 @@ contract ManualApprovalTransferManager is ITransferManager { } function _revokeManualApproval(address _from, address _to) internal { - require(approvalIndex[_from][_to] != 0, "Approval not exist"); - + uint256 index = approvalIndex[_from][_to]; + require(index != 0, "Approval does not exist"); + index--; //Actual index is stored index - 1. + uint256 lastApprovalIndex = approvals.length - 1; // find the record in active approvals array & delete it - uint256 index = approvalIndex[_from][_to] - 1; - if (index != approvals.length -1) { - approvals[index] = approvals[approvals.length -1]; - approvalIndex[approvals[index].from][approvals[index].to] = index + 1; + if (index != lastApprovalIndex) { + approvals[index] = approvals[lastApprovalIndex]; + approvalIndex[approvals[index].from][approvals[index].to] = index + 1; } delete approvalIndex[_from][_to]; approvals.length--; @@ -294,9 +298,9 @@ contract ManualApprovalTransferManager is ITransferManager { uint256[] _expiryTimes, uint256[] _allowances, bytes32[] _descriptions - ) + ) internal - pure + pure { require(_from.length == _to.length && _to.length == _allowances.length && @@ -308,7 +312,7 @@ contract ManualApprovalTransferManager is ITransferManager { /** * @notice Returns the all active approvals corresponds to an address - * @param _user Address of the holder corresponds to whom list of manual approvals + * @param _user Address of the holder corresponds to whom list of manual approvals * need to return * @return address[] addresses from * @return address[] addresses to @@ -318,7 +322,8 @@ contract ManualApprovalTransferManager is ITransferManager { */ function getActiveApprovalsToUser(address _user) external view returns(address[], address[], uint256[], uint256[], bytes32[]) { uint256 counter = 0; - for (uint256 i = 0; i < approvals.length; i++) { + uint256 approvalsLength = approvals.length; + for (uint256 i = 0; i < approvalsLength; i++) { if ((approvals[i].from == _user || approvals[i].to == _user) && approvals[i].expiryTime >= now) counter ++; @@ -331,7 +336,7 @@ contract ManualApprovalTransferManager is ITransferManager { bytes32[] memory description = new bytes32[](counter); counter = 0; - for (i = 0; i < approvals.length; i++) { + for (i = 0; i < approvalsLength; i++) { if ((approvals[i].from == _user || approvals[i].to == _user) && approvals[i].expiryTime >= now) { @@ -341,7 +346,7 @@ contract ManualApprovalTransferManager is ITransferManager { expiryTime[counter]=approvals[i].expiryTime; description[counter]=approvals[i].description; counter ++; - } + } } return (from, to, allowance, expiryTime, description); } @@ -355,8 +360,9 @@ contract ManualApprovalTransferManager is ITransferManager { * @return uint256 Description provided to the approval */ function getApprovalDetails(address _from, address _to) external view returns(uint256, uint256, bytes32) { - if (approvalIndex[_from][_to] != 0) { - uint256 index = approvalIndex[_from][_to] - 1; + uint256 index = approvalIndex[_from][_to]; + if (index != 0) { + index--; if (index < approvals.length) { ManualApproval storage approval = approvals[index]; return( @@ -366,7 +372,6 @@ contract ManualApprovalTransferManager is ITransferManager { ); } } - return (uint256(0), uint256(0), bytes32(0)); } /** @@ -390,8 +395,9 @@ contract ManualApprovalTransferManager is ITransferManager { uint256[] memory allowance = new uint256[](approvals.length); uint256[] memory expiryTime = new uint256[](approvals.length); bytes32[] memory description = new bytes32[](approvals.length); + uint256 approvalsLength = approvals.length; - for (uint256 i = 0; i < approvals.length; i++) { + for (uint256 i = 0; i < approvalsLength; i++) { from[i]=approvals[i].from; to[i]=approvals[i].to; @@ -402,7 +408,7 @@ contract ManualApprovalTransferManager is ITransferManager { } return (from, to, allowance, expiryTime, description); - + } /** From 329b9c585c5abb1cd37497a984f8aa66f453ccce Mon Sep 17 00:00:00 2001 From: Mudit Gupta Date: Fri, 15 Mar 2019 18:52:09 +0530 Subject: [PATCH 2/5] Added code comments --- .../GeneralTransferManager.sol | 127 +++++++++--------- .../ManualApprovalTransferManager.sol | 11 +- .../storage/GeneralTransferManagerStorage.sol | 8 +- 3 files changed, 75 insertions(+), 71 deletions(-) diff --git a/contracts/modules/TransferManager/GeneralTransferManager.sol b/contracts/modules/TransferManager/GeneralTransferManager.sol index dc7eabd4b..ff3c9fd23 100644 --- a/contracts/modules/TransferManager/GeneralTransferManager.sol +++ b/contracts/modules/TransferManager/GeneralTransferManager.sol @@ -24,19 +24,19 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag // Emit when there is change in the flag variable called signingAddress event ChangeSigningAddress(address _signingAddress); // Emit when investor details get modified related to their whitelisting - event ChangeDefaults(uint64 _defaultFromTime, uint64 _defaultToTime); + event ChangeDefaults(uint64 _defaultCanSendTokensAfter, uint64 _defaultCanReceiveTokensAfter); - // _fromTime is the time from which the _investor can send tokens - // _toTime is the time from which the _investor can receive tokens - // if allowAllWhitelistIssuances is TRUE, then _toTime is ignored when receiving tokens from the issuance address - // if allowAllWhitelistTransfers is TRUE, then _toTime and _fromTime is ignored when sending or receiving tokens + // _canSendTokensAfter is the time from which the _investor can send tokens + // _canReceiveTokensAfter is the time from which the _investor can receive tokens + // if allowAllWhitelistIssuances is TRUE, then _canReceiveTokensAfter is ignored when receiving tokens from the issuance address + // if allowAllWhitelistTransfers is TRUE, then _canReceiveTokensAfter and _canSendTokensAfter is ignored when sending or receiving tokens // in any case, any investor sending or receiving tokens, must have a _expiryTime in the future event ModifyWhitelist( address indexed _investor, uint256 _dateAdded, address indexed _addedBy, - uint256 _fromTime, - uint256 _toTime, + uint256 _canSendTokensAfter, + uint256 _canReceiveTokensAfter, uint256 _expiryTime, bool _canBuyFromSTO ); @@ -60,14 +60,17 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag } /** - * @notice Used to change the default times used when fromTime / toTime are zero - * @param _defaultFromTime default for zero fromTime - * @param _defaultToTime default for zero toTime + * @notice Used to change the default times used when canSendTokensAfter / canReceiveTokensAfter are zero + * @param _defaultCanSendTokensAfter default for zero canSendTokensAfter + * @param _defaultCanReceiveTokensAfter default for zero canReceiveTokensAfter */ - function changeDefaults(uint64 _defaultFromTime, uint64 _defaultToTime) public withPerm(FLAGS) { - defaults.fromTime = _defaultFromTime; - defaults.toTime = _defaultToTime; - emit ChangeDefaults(_defaultFromTime, _defaultToTime); + function changeDefaults(uint64 _defaultCanSendTokensAfter, uint64 _defaultCanReceiveTokensAfter) public withPerm(FLAGS) { + /* 0 values are also allowed as they represent that the Issuer + does not want a default value for these variables. + 0 is also the default value of these variables */ + defaults.canSendTokensAfter = _defaultCanSendTokensAfter; + defaults.canReceiveTokensAfter = _defaultCanReceiveTokensAfter; + emit ChangeDefaults(_defaultCanSendTokensAfter, _defaultCanReceiveTokensAfter); } /** @@ -75,6 +78,7 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag * @param _issuanceAddress new address for the issuance */ function changeIssuanceAddress(address _issuanceAddress) public withPerm(FLAGS) { + // address(0x0) is also a valid value and in most cases, the address that issues tokens is 0x0. issuanceAddress = _issuanceAddress; emit ChangeIssuanceAddress(_issuanceAddress); } @@ -84,6 +88,9 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag * @param _signingAddress new address for the signing */ function changeSigningAddress(address _signingAddress) public withPerm(FLAGS) { + /* address(0x0) is also a valid value as an Issuer might want to + give this permission to nobody (except their own address). + 0x0 is also the default value */ signingAddress = _signingAddress; emit ChangeSigningAddress(_signingAddress); } @@ -156,7 +163,7 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag return (_onWhitelist(_to) && _onWhitelist(_from)) ? Result.VALID : Result.NA; } - (uint64 adjustedFromTime, uint64 adjustedToTime) = _adjustTimes(whitelist[_from].fromTime, whitelist[_to].toTime); + (uint64 adjustedCanSendTokensAfter, uint64 adjustedCanReceiveTokensAfter) = _adjustTimes(whitelist[_from].canSendTokensAfter, whitelist[_to].canReceiveTokensAfter); if (_from == issuanceAddress) { // Possible STO transaction, but investor not allowed to purchased from STO if ((whitelist[_to].canBuyFromSTO == 0) && _isSTOAttached()) { @@ -166,14 +173,14 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag if (allowAllWhitelistIssuances) { return _onWhitelist(_to) ? Result.VALID : Result.NA; } else { - return (_onWhitelist(_to) && (adjustedToTime <= uint64(now))) ? Result.VALID : Result.NA; + return (_onWhitelist(_to) && (adjustedCanReceiveTokensAfter <= uint64(now))) ? Result.VALID : Result.NA; } } //Anyone on the whitelist can transfer provided the blocknumber is large enough /*solium-disable-next-line security/no-block-members*/ - return ((_onWhitelist(_from) && (adjustedFromTime <= uint64(now))) && - (_onWhitelist(_to) && (adjustedToTime <= uint64(now)))) ? Result.VALID : Result.NA; /*solium-disable-line security/no-block-members*/ + return ((_onWhitelist(_from) && (adjustedCanSendTokensAfter <= uint64(now))) && + (_onWhitelist(_to) && (adjustedCanReceiveTokensAfter <= uint64(now)))) ? Result.VALID : Result.NA; /*solium-disable-line security/no-block-members*/ } return Result.NA; } @@ -181,36 +188,36 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag /** * @notice Adds or removes addresses from the whitelist. * @param _investor is the address to whitelist - * @param _fromTime is the moment when the sale lockup period ends and the investor can freely sell his tokens - * @param _toTime is the moment when the purchase lockup period ends and the investor can freely purchase tokens from others + * @param _canSendTokensAfter is the moment when the sale lockup period ends and the investor can freely sell his tokens + * @param _canReceiveTokensAfter is the moment when the purchase lockup period ends and the investor can freely purchase tokens from others * @param _expiryTime is the moment till investors KYC will be validated. After that investor need to do re-KYC * @param _canBuyFromSTO is used to know whether the investor is restricted investor or not. */ function modifyWhitelist( address _investor, - uint256 _fromTime, - uint256 _toTime, + uint256 _canSendTokensAfter, + uint256 _canReceiveTokensAfter, uint256 _expiryTime, bool _canBuyFromSTO ) public withPerm(WHITELIST) { - _modifyWhitelist(_investor, _fromTime, _toTime, _expiryTime, _canBuyFromSTO); + _modifyWhitelist(_investor, _canSendTokensAfter, _canReceiveTokensAfter, _expiryTime, _canBuyFromSTO); } /** * @notice Adds or removes addresses from the whitelist. * @param _investor is the address to whitelist - * @param _fromTime is the moment when the sale lockup period ends and the investor can freely sell his tokens - * @param _toTime is the moment when the purchase lockup period ends and the investor can freely purchase tokens from others + * @param _canSendTokensAfter is the moment when the sale lockup period ends and the investor can freely sell his tokens + * @param _canReceiveTokensAfter is the moment when the purchase lockup period ends and the investor can freely purchase tokens from others * @param _expiryTime is the moment till investors KYC will be validated. After that investor need to do re-KYC * @param _canBuyFromSTO is used to know whether the investor is restricted investor or not. */ function _modifyWhitelist( address _investor, - uint256 _fromTime, - uint256 _toTime, + uint256 _canSendTokensAfter, + uint256 _canReceiveTokensAfter, uint256 _expiryTime, bool _canBuyFromSTO ) @@ -224,39 +231,39 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag if (whitelist[_investor].added == uint8(0)) { investors.push(_investor); } - whitelist[_investor] = TimeRestriction(uint64(_fromTime), uint64(_toTime), uint64(_expiryTime), canBuyFromSTO, uint8(1)); - emit ModifyWhitelist(_investor, now, msg.sender, _fromTime, _toTime, _expiryTime, _canBuyFromSTO); + whitelist[_investor] = TimeRestriction(uint64(_canSendTokensAfter), uint64(_canReceiveTokensAfter), uint64(_expiryTime), canBuyFromSTO, uint8(1)); + emit ModifyWhitelist(_investor, now, msg.sender, _canSendTokensAfter, _canReceiveTokensAfter, _expiryTime, _canBuyFromSTO); } /** * @notice Adds or removes addresses from the whitelist. * @param _investors List of the addresses to whitelist - * @param _fromTimes An array of the moment when the sale lockup period ends and the investor can freely sell his tokens - * @param _toTimes An array of the moment when the purchase lockup period ends and the investor can freely purchase tokens from others + * @param _canSendTokensAfters An array of the moment when the sale lockup period ends and the investor can freely sell his tokens + * @param _canReceiveTokensAfters An array of the moment when the purchase lockup period ends and the investor can freely purchase tokens from others * @param _expiryTimes An array of the moment till investors KYC will be validated. After that investor need to do re-KYC * @param _canBuyFromSTO An array of boolean values */ function modifyWhitelistMulti( address[] _investors, - uint256[] _fromTimes, - uint256[] _toTimes, + uint256[] _canSendTokensAfters, + uint256[] _canReceiveTokensAfters, uint256[] _expiryTimes, bool[] _canBuyFromSTO ) public withPerm(WHITELIST) { - require(_investors.length == _fromTimes.length, "Mismatched input lengths"); - require(_fromTimes.length == _toTimes.length, "Mismatched input lengths"); - require(_toTimes.length == _expiryTimes.length, "Mismatched input lengths"); - require(_canBuyFromSTO.length == _toTimes.length, "Mismatched input length"); + require(_investors.length == _canSendTokensAfters.length, "Mismatched input lengths"); + require(_canSendTokensAfters.length == _canReceiveTokensAfters.length, "Mismatched input lengths"); + require(_canReceiveTokensAfters.length == _expiryTimes.length, "Mismatched input lengths"); + require(_canBuyFromSTO.length == _canReceiveTokensAfters.length, "Mismatched input length"); for (uint256 i = 0; i < _investors.length; i++) { - _modifyWhitelist(_investors[i], _fromTimes[i], _toTimes[i], _expiryTimes[i], _canBuyFromSTO[i]); + _modifyWhitelist(_investors[i], _canSendTokensAfters[i], _canReceiveTokensAfters[i], _expiryTimes[i], _canBuyFromSTO[i]); } } /** * @notice Adds or removes addresses from the whitelist - can be called by anyone with a valid signature * @param _investor is the address to whitelist - * @param _fromTime is the moment when the sale lockup period ends and the investor can freely sell his tokens - * @param _toTime is the moment when the purchase lockup period ends and the investor can freely purchase tokens from others + * @param _canSendTokensAfter is the moment when the sale lockup period ends and the investor can freely sell his tokens + * @param _canReceiveTokensAfter is the moment when the purchase lockup period ends and the investor can freely purchase tokens from others * @param _expiryTime is the moment till investors KYC will be validated. After that investor need to do re-KYC * @param _canBuyFromSTO is used to know whether the investor is restricted investor or not. * @param _validFrom is the time that this signature is valid from @@ -268,8 +275,8 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag */ function modifyWhitelistSigned( address _investor, - uint256 _fromTime, - uint256 _toTime, + uint256 _canSendTokensAfter, + uint256 _canReceiveTokensAfter, uint256 _expiryTime, bool _canBuyFromSTO, uint256 _validFrom, @@ -286,10 +293,10 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag require(!nonceMap[_investor][_nonce], "Already used signature"); nonceMap[_investor][_nonce] = true; bytes32 hash = keccak256( - abi.encodePacked(this, _investor, _fromTime, _toTime, _expiryTime, _canBuyFromSTO, _validFrom, _validTo, _nonce) + abi.encodePacked(this, _investor, _canSendTokensAfter, _canReceiveTokensAfter, _expiryTime, _canBuyFromSTO, _validFrom, _validTo, _nonce) ); _checkSig(hash, _v, _r, _s); - _modifyWhitelist(_investor, _fromTime, _toTime, _expiryTime, _canBuyFromSTO); + _modifyWhitelist(_investor, _canSendTokensAfter, _canReceiveTokensAfter, _expiryTime, _canBuyFromSTO); } /** @@ -297,7 +304,7 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag */ function _checkSig(bytes32 _hash, uint8 _v, bytes32 _r, bytes32 _s) internal view { //Check that the signature is valid - //sig should be signing - _investor, _fromTime, _toTime & _expiryTime and be signed by the issuer address + //sig should be signing - _investor, _canSendTokensAfter, _canReceiveTokensAfter & _expiryTime and be signed by the issuer address address signer = ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", _hash)), _v, _r, _s); require(signer == Ownable(securityToken).owner() || signer == signingAddress, "Incorrect signer"); } @@ -322,16 +329,16 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag /** * @notice Internal function to adjust times using default values */ - function _adjustTimes(uint64 _fromTime, uint64 _toTime) internal view returns(uint64, uint64) { - uint64 adjustedFromTime = _fromTime; - uint64 adjustedToTime = _toTime; - if (_fromTime == 0) { - adjustedFromTime = defaults.fromTime; + function _adjustTimes(uint64 _canSendTokensAfter, uint64 _canReceiveTokensAfter) internal view returns(uint64, uint64) { + uint64 adjustedCanSendTokensAfter = _canSendTokensAfter; + uint64 adjustedCanReceiveTokensAfter = _canReceiveTokensAfter; + if (_canSendTokensAfter == 0) { + adjustedCanSendTokensAfter = defaults.canSendTokensAfter; } - if (_toTime == 0) { - adjustedToTime = defaults.toTime; + if (_canReceiveTokensAfter == 0) { + adjustedCanReceiveTokensAfter = defaults.canReceiveTokensAfter; } - return (adjustedFromTime, adjustedToTime); + return (adjustedCanSendTokensAfter, adjustedCanReceiveTokensAfter); } /** @@ -345,9 +352,9 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag * @dev Returns list of all investors data */ function getAllInvestorsData() external view returns(address[], uint256[], uint256[], uint256[], bool[]) { - (uint256[] memory fromTimes, uint256[] memory toTimes, uint256[] memory expiryTimes, bool[] memory canBuyFromSTOs) + (uint256[] memory canSendTokensAfters, uint256[] memory canReceiveTokensAfters, uint256[] memory expiryTimes, bool[] memory canBuyFromSTOs) = _investorsData(investors); - return (investors, fromTimes, toTimes, expiryTimes, canBuyFromSTOs); + return (investors, canSendTokensAfters, canReceiveTokensAfters, expiryTimes, canBuyFromSTOs); } @@ -359,13 +366,13 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag } function _investorsData(address[] _investors) internal view returns(uint256[], uint256[], uint256[], bool[]) { - uint256[] memory fromTimes = new uint256[](_investors.length); - uint256[] memory toTimes = new uint256[](_investors.length); + uint256[] memory canSendTokensAfters = new uint256[](_investors.length); + uint256[] memory canReceiveTokensAfters = new uint256[](_investors.length); uint256[] memory expiryTimes = new uint256[](_investors.length); bool[] memory canBuyFromSTOs = new bool[](_investors.length); for (uint256 i = 0; i < _investors.length; i++) { - fromTimes[i] = whitelist[_investors[i]].fromTime; - toTimes[i] = whitelist[_investors[i]].toTime; + canSendTokensAfters[i] = whitelist[_investors[i]].canSendTokensAfter; + canReceiveTokensAfters[i] = whitelist[_investors[i]].canReceiveTokensAfter; expiryTimes[i] = whitelist[_investors[i]].expiryTime; if (whitelist[_investors[i]].canBuyFromSTO == 0) { canBuyFromSTOs[i] = false; @@ -373,7 +380,7 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag canBuyFromSTOs[i] = true; } } - return (fromTimes, toTimes, expiryTimes, canBuyFromSTOs); + return (canSendTokensAfters, canReceiveTokensAfters, expiryTimes, canBuyFromSTOs); } /** diff --git a/contracts/modules/TransferManager/ManualApprovalTransferManager.sol b/contracts/modules/TransferManager/ManualApprovalTransferManager.sol index f53259eed..5e95f6513 100644 --- a/contracts/modules/TransferManager/ManualApprovalTransferManager.sol +++ b/contracts/modules/TransferManager/ManualApprovalTransferManager.sol @@ -9,12 +9,6 @@ import "openzeppelin-solidity/contracts/math/SafeMath.sol"; contract ManualApprovalTransferManager is ITransferManager { using SafeMath for uint256; - //Address from which issuances come - address public issuanceAddress = address(0); - - //Address which can sign whitelist changes - address public signingAddress = address(0); - bytes32 public constant TRANSFER_APPROVAL = "TRANSFER_APPROVAL"; //Manual approval is an allowance (that has been approved) with an expiry time @@ -27,7 +21,10 @@ contract ManualApprovalTransferManager is ITransferManager { } mapping (address => mapping (address => uint256)) public approvalIndex; - // An array to track all approvals + + // An array to track all approvals. It is an unbounded array but it's not a problem as + // it is never looped through in an onchain call. It is defined as an Array instead of mapping + // just to make it easier for users to fetch list of all approvals through constant functions. ManualApproval[] public approvals; event AddManualApproval( diff --git a/contracts/storage/GeneralTransferManagerStorage.sol b/contracts/storage/GeneralTransferManagerStorage.sol index cca7b5dd1..485164c0f 100644 --- a/contracts/storage/GeneralTransferManagerStorage.sol +++ b/contracts/storage/GeneralTransferManagerStorage.sol @@ -16,8 +16,8 @@ contract GeneralTransferManagerStorage { //from and to timestamps that an investor can send / receive tokens respectively struct TimeRestriction { - uint64 fromTime; - uint64 toTime; + uint64 canSendTokensAfter; + uint64 canReceiveTokensAfter; uint64 expiryTime; uint8 canBuyFromSTO; uint8 added; @@ -25,8 +25,8 @@ contract GeneralTransferManagerStorage { // Allows all TimeRestrictions to be offset struct Defaults { - uint64 fromTime; - uint64 toTime; + uint64 canSendTokensAfter; + uint64 canReceiveTokensAfter; } // Offset to be applied to all timings (except KYC expiry) From 6a5cf39d90647e7c2b4275de733f443be0b35f2f Mon Sep 17 00:00:00 2001 From: Mudit Gupta Date: Fri, 15 Mar 2019 18:54:40 +0530 Subject: [PATCH 3/5] Updated tests --- test/j_manual_approval_transfer_manager.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/j_manual_approval_transfer_manager.js b/test/j_manual_approval_transfer_manager.js index 49a2f9645..985d471ca 100644 --- a/test/j_manual_approval_transfer_manager.js +++ b/test/j_manual_approval_transfer_manager.js @@ -459,7 +459,7 @@ contract("ManualApprovalTransferManager", accounts => { latestTime() + duration.days(2), web3.utils.toWei("5"), "New Description", - 0, + false, { from: token_owner } @@ -493,9 +493,9 @@ contract("ManualApprovalTransferManager", accounts => { account_investor1, account_investor4, expiryTimeMA, - web3.utils.toWei("5"), + 0, "New Description", - 45, + true, { from: token_owner } @@ -529,7 +529,7 @@ contract("ManualApprovalTransferManager", accounts => { expiryTimeMA, web3.utils.toWei("4"), "New Description", - 1, + true, { from: token_owner } @@ -557,7 +557,7 @@ contract("ManualApprovalTransferManager", accounts => { expiryTimeMA, web3.utils.toWei("1"), "New Description", - 0, + false, { from: token_owner } @@ -592,7 +592,7 @@ contract("ManualApprovalTransferManager", accounts => { expiryTimeMA, web3.utils.toWei("5"), "New Description", - 0, + false, { from: token_owner } From 8b2bd1d32f7d4970e8432cbc16b38f902848d2a1 Mon Sep 17 00:00:00 2001 From: Mudit Gupta Date: Fri, 15 Mar 2019 19:01:33 +0530 Subject: [PATCH 4/5] Updated names --- .../GeneralTransferManager.sol | 120 +++++++++--------- .../storage/GeneralTransferManagerStorage.sol | 10 +- 2 files changed, 66 insertions(+), 64 deletions(-) diff --git a/contracts/modules/TransferManager/GeneralTransferManager.sol b/contracts/modules/TransferManager/GeneralTransferManager.sol index ff3c9fd23..2f70f194b 100644 --- a/contracts/modules/TransferManager/GeneralTransferManager.sol +++ b/contracts/modules/TransferManager/GeneralTransferManager.sol @@ -24,19 +24,19 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag // Emit when there is change in the flag variable called signingAddress event ChangeSigningAddress(address _signingAddress); // Emit when investor details get modified related to their whitelisting - event ChangeDefaults(uint64 _defaultCanSendTokensAfter, uint64 _defaultCanReceiveTokensAfter); + event ChangeDefaults(uint64 _defaultCanSendAfter, uint64 _defaultCanReceiveAfter); - // _canSendTokensAfter is the time from which the _investor can send tokens - // _canReceiveTokensAfter is the time from which the _investor can receive tokens - // if allowAllWhitelistIssuances is TRUE, then _canReceiveTokensAfter is ignored when receiving tokens from the issuance address - // if allowAllWhitelistTransfers is TRUE, then _canReceiveTokensAfter and _canSendTokensAfter is ignored when sending or receiving tokens + // _canSendAfter is the time from which the _investor can send tokens + // _canReceiveAfter is the time from which the _investor can receive tokens + // if allowAllWhitelistIssuances is TRUE, then _canReceiveAfter is ignored when receiving tokens from the issuance address + // if allowAllWhitelistTransfers is TRUE, then _canReceiveAfter and _canSendAfter is ignored when sending or receiving tokens // in any case, any investor sending or receiving tokens, must have a _expiryTime in the future event ModifyWhitelist( address indexed _investor, uint256 _dateAdded, address indexed _addedBy, - uint256 _canSendTokensAfter, - uint256 _canReceiveTokensAfter, + uint256 _canSendAfter, + uint256 _canReceiveAfter, uint256 _expiryTime, bool _canBuyFromSTO ); @@ -60,17 +60,17 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag } /** - * @notice Used to change the default times used when canSendTokensAfter / canReceiveTokensAfter are zero - * @param _defaultCanSendTokensAfter default for zero canSendTokensAfter - * @param _defaultCanReceiveTokensAfter default for zero canReceiveTokensAfter + * @notice Used to change the default times used when canSendAfter / canReceiveAfter are zero + * @param _defaultCanSendAfter default for zero canSendAfter + * @param _defaultCanReceiveAfter default for zero canReceiveAfter */ - function changeDefaults(uint64 _defaultCanSendTokensAfter, uint64 _defaultCanReceiveTokensAfter) public withPerm(FLAGS) { + function changeDefaults(uint64 _defaultCanSendAfter, uint64 _defaultCanReceiveAfter) public withPerm(FLAGS) { /* 0 values are also allowed as they represent that the Issuer does not want a default value for these variables. 0 is also the default value of these variables */ - defaults.canSendTokensAfter = _defaultCanSendTokensAfter; - defaults.canReceiveTokensAfter = _defaultCanReceiveTokensAfter; - emit ChangeDefaults(_defaultCanSendTokensAfter, _defaultCanReceiveTokensAfter); + defaults.canSendAfter = _defaultCanSendAfter; + defaults.canReceiveAfter = _defaultCanReceiveAfter; + emit ChangeDefaults(_defaultCanSendAfter, _defaultCanReceiveAfter); } /** @@ -163,7 +163,7 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag return (_onWhitelist(_to) && _onWhitelist(_from)) ? Result.VALID : Result.NA; } - (uint64 adjustedCanSendTokensAfter, uint64 adjustedCanReceiveTokensAfter) = _adjustTimes(whitelist[_from].canSendTokensAfter, whitelist[_to].canReceiveTokensAfter); + (uint64 adjustedCanSendAfter, uint64 adjustedCanReceiveAfter) = _adjustTimes(whitelist[_from].canSendAfter, whitelist[_to].canReceiveAfter); if (_from == issuanceAddress) { // Possible STO transaction, but investor not allowed to purchased from STO if ((whitelist[_to].canBuyFromSTO == 0) && _isSTOAttached()) { @@ -173,14 +173,14 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag if (allowAllWhitelistIssuances) { return _onWhitelist(_to) ? Result.VALID : Result.NA; } else { - return (_onWhitelist(_to) && (adjustedCanReceiveTokensAfter <= uint64(now))) ? Result.VALID : Result.NA; + return (_onWhitelist(_to) && (adjustedCanReceiveAfter <= uint64(now))) ? Result.VALID : Result.NA; } } //Anyone on the whitelist can transfer provided the blocknumber is large enough /*solium-disable-next-line security/no-block-members*/ - return ((_onWhitelist(_from) && (adjustedCanSendTokensAfter <= uint64(now))) && - (_onWhitelist(_to) && (adjustedCanReceiveTokensAfter <= uint64(now)))) ? Result.VALID : Result.NA; /*solium-disable-line security/no-block-members*/ + return ((_onWhitelist(_from) && (adjustedCanSendAfter <= uint64(now))) && + (_onWhitelist(_to) && (adjustedCanReceiveAfter <= uint64(now)))) ? Result.VALID : Result.NA; /*solium-disable-line security/no-block-members*/ } return Result.NA; } @@ -188,36 +188,36 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag /** * @notice Adds or removes addresses from the whitelist. * @param _investor is the address to whitelist - * @param _canSendTokensAfter is the moment when the sale lockup period ends and the investor can freely sell his tokens - * @param _canReceiveTokensAfter is the moment when the purchase lockup period ends and the investor can freely purchase tokens from others + * @param _canSendAfter the moment when the sale lockup period ends and the investor can freely sell or transfer away their tokens + * @param _canReceiveAfter the moment when the purchase lockup period ends and the investor can freely purchase or receive from others * @param _expiryTime is the moment till investors KYC will be validated. After that investor need to do re-KYC * @param _canBuyFromSTO is used to know whether the investor is restricted investor or not. */ function modifyWhitelist( address _investor, - uint256 _canSendTokensAfter, - uint256 _canReceiveTokensAfter, + uint256 _canSendAfter, + uint256 _canReceiveAfter, uint256 _expiryTime, bool _canBuyFromSTO ) public withPerm(WHITELIST) { - _modifyWhitelist(_investor, _canSendTokensAfter, _canReceiveTokensAfter, _expiryTime, _canBuyFromSTO); + _modifyWhitelist(_investor, _canSendAfter, _canReceiveAfter, _expiryTime, _canBuyFromSTO); } /** * @notice Adds or removes addresses from the whitelist. * @param _investor is the address to whitelist - * @param _canSendTokensAfter is the moment when the sale lockup period ends and the investor can freely sell his tokens - * @param _canReceiveTokensAfter is the moment when the purchase lockup period ends and the investor can freely purchase tokens from others + * @param _canSendAfter is the moment when the sale lockup period ends and the investor can freely sell his tokens + * @param _canReceiveAfter is the moment when the purchase lockup period ends and the investor can freely purchase tokens from others * @param _expiryTime is the moment till investors KYC will be validated. After that investor need to do re-KYC * @param _canBuyFromSTO is used to know whether the investor is restricted investor or not. */ function _modifyWhitelist( address _investor, - uint256 _canSendTokensAfter, - uint256 _canReceiveTokensAfter, + uint256 _canSendAfter, + uint256 _canReceiveAfter, uint256 _expiryTime, bool _canBuyFromSTO ) @@ -231,39 +231,39 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag if (whitelist[_investor].added == uint8(0)) { investors.push(_investor); } - whitelist[_investor] = TimeRestriction(uint64(_canSendTokensAfter), uint64(_canReceiveTokensAfter), uint64(_expiryTime), canBuyFromSTO, uint8(1)); - emit ModifyWhitelist(_investor, now, msg.sender, _canSendTokensAfter, _canReceiveTokensAfter, _expiryTime, _canBuyFromSTO); + whitelist[_investor] = TimeRestriction(uint64(_canSendAfter), uint64(_canReceiveAfter), uint64(_expiryTime), canBuyFromSTO, uint8(1)); + emit ModifyWhitelist(_investor, now, msg.sender, _canSendAfter, _canReceiveAfter, _expiryTime, _canBuyFromSTO); } /** * @notice Adds or removes addresses from the whitelist. * @param _investors List of the addresses to whitelist - * @param _canSendTokensAfters An array of the moment when the sale lockup period ends and the investor can freely sell his tokens - * @param _canReceiveTokensAfters An array of the moment when the purchase lockup period ends and the investor can freely purchase tokens from others + * @param _canSendAfters An array of the moment when the sale lockup period ends and the investor can freely sell his tokens + * @param _canReceiveAfters An array of the moment when the purchase lockup period ends and the investor can freely purchase tokens from others * @param _expiryTimes An array of the moment till investors KYC will be validated. After that investor need to do re-KYC * @param _canBuyFromSTO An array of boolean values */ function modifyWhitelistMulti( address[] _investors, - uint256[] _canSendTokensAfters, - uint256[] _canReceiveTokensAfters, + uint256[] _canSendAfters, + uint256[] _canReceiveAfters, uint256[] _expiryTimes, bool[] _canBuyFromSTO ) public withPerm(WHITELIST) { - require(_investors.length == _canSendTokensAfters.length, "Mismatched input lengths"); - require(_canSendTokensAfters.length == _canReceiveTokensAfters.length, "Mismatched input lengths"); - require(_canReceiveTokensAfters.length == _expiryTimes.length, "Mismatched input lengths"); - require(_canBuyFromSTO.length == _canReceiveTokensAfters.length, "Mismatched input length"); + require(_investors.length == _canSendAfters.length, "Mismatched input lengths"); + require(_canSendAfters.length == _canReceiveAfters.length, "Mismatched input lengths"); + require(_canReceiveAfters.length == _expiryTimes.length, "Mismatched input lengths"); + require(_canBuyFromSTO.length == _canReceiveAfters.length, "Mismatched input length"); for (uint256 i = 0; i < _investors.length; i++) { - _modifyWhitelist(_investors[i], _canSendTokensAfters[i], _canReceiveTokensAfters[i], _expiryTimes[i], _canBuyFromSTO[i]); + _modifyWhitelist(_investors[i], _canSendAfters[i], _canReceiveAfters[i], _expiryTimes[i], _canBuyFromSTO[i]); } } /** * @notice Adds or removes addresses from the whitelist - can be called by anyone with a valid signature * @param _investor is the address to whitelist - * @param _canSendTokensAfter is the moment when the sale lockup period ends and the investor can freely sell his tokens - * @param _canReceiveTokensAfter is the moment when the purchase lockup period ends and the investor can freely purchase tokens from others + * @param _canSendAfter is the moment when the sale lockup period ends and the investor can freely sell his tokens + * @param _canReceiveAfter is the moment when the purchase lockup period ends and the investor can freely purchase tokens from others * @param _expiryTime is the moment till investors KYC will be validated. After that investor need to do re-KYC * @param _canBuyFromSTO is used to know whether the investor is restricted investor or not. * @param _validFrom is the time that this signature is valid from @@ -275,8 +275,8 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag */ function modifyWhitelistSigned( address _investor, - uint256 _canSendTokensAfter, - uint256 _canReceiveTokensAfter, + uint256 _canSendAfter, + uint256 _canReceiveAfter, uint256 _expiryTime, bool _canBuyFromSTO, uint256 _validFrom, @@ -293,10 +293,10 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag require(!nonceMap[_investor][_nonce], "Already used signature"); nonceMap[_investor][_nonce] = true; bytes32 hash = keccak256( - abi.encodePacked(this, _investor, _canSendTokensAfter, _canReceiveTokensAfter, _expiryTime, _canBuyFromSTO, _validFrom, _validTo, _nonce) + abi.encodePacked(this, _investor, _canSendAfter, _canReceiveAfter, _expiryTime, _canBuyFromSTO, _validFrom, _validTo, _nonce) ); _checkSig(hash, _v, _r, _s); - _modifyWhitelist(_investor, _canSendTokensAfter, _canReceiveTokensAfter, _expiryTime, _canBuyFromSTO); + _modifyWhitelist(_investor, _canSendAfter, _canReceiveAfter, _expiryTime, _canBuyFromSTO); } /** @@ -304,7 +304,7 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag */ function _checkSig(bytes32 _hash, uint8 _v, bytes32 _r, bytes32 _s) internal view { //Check that the signature is valid - //sig should be signing - _investor, _canSendTokensAfter, _canReceiveTokensAfter & _expiryTime and be signed by the issuer address + //sig should be signing - _investor, _canSendAfter, _canReceiveAfter & _expiryTime and be signed by the issuer address address signer = ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", _hash)), _v, _r, _s); require(signer == Ownable(securityToken).owner() || signer == signingAddress, "Incorrect signer"); } @@ -329,16 +329,16 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag /** * @notice Internal function to adjust times using default values */ - function _adjustTimes(uint64 _canSendTokensAfter, uint64 _canReceiveTokensAfter) internal view returns(uint64, uint64) { - uint64 adjustedCanSendTokensAfter = _canSendTokensAfter; - uint64 adjustedCanReceiveTokensAfter = _canReceiveTokensAfter; - if (_canSendTokensAfter == 0) { - adjustedCanSendTokensAfter = defaults.canSendTokensAfter; + function _adjustTimes(uint64 _canSendAfter, uint64 _canReceiveAfter) internal view returns(uint64, uint64) { + uint64 adjustedCanSendAfter = _canSendAfter; + uint64 adjustedCanReceiveAfter = _canReceiveAfter; + if (_canSendAfter == 0) { + adjustedCanSendAfter = defaults.canSendAfter; } - if (_canReceiveTokensAfter == 0) { - adjustedCanReceiveTokensAfter = defaults.canReceiveTokensAfter; + if (_canReceiveAfter == 0) { + adjustedCanReceiveAfter = defaults.canReceiveAfter; } - return (adjustedCanSendTokensAfter, adjustedCanReceiveTokensAfter); + return (adjustedCanSendAfter, adjustedCanReceiveAfter); } /** @@ -352,9 +352,9 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag * @dev Returns list of all investors data */ function getAllInvestorsData() external view returns(address[], uint256[], uint256[], uint256[], bool[]) { - (uint256[] memory canSendTokensAfters, uint256[] memory canReceiveTokensAfters, uint256[] memory expiryTimes, bool[] memory canBuyFromSTOs) + (uint256[] memory canSendAfters, uint256[] memory canReceiveAfters, uint256[] memory expiryTimes, bool[] memory canBuyFromSTOs) = _investorsData(investors); - return (investors, canSendTokensAfters, canReceiveTokensAfters, expiryTimes, canBuyFromSTOs); + return (investors, canSendAfters, canReceiveAfters, expiryTimes, canBuyFromSTOs); } @@ -366,13 +366,13 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag } function _investorsData(address[] _investors) internal view returns(uint256[], uint256[], uint256[], bool[]) { - uint256[] memory canSendTokensAfters = new uint256[](_investors.length); - uint256[] memory canReceiveTokensAfters = new uint256[](_investors.length); + uint256[] memory canSendAfters = new uint256[](_investors.length); + uint256[] memory canReceiveAfters = new uint256[](_investors.length); uint256[] memory expiryTimes = new uint256[](_investors.length); bool[] memory canBuyFromSTOs = new bool[](_investors.length); for (uint256 i = 0; i < _investors.length; i++) { - canSendTokensAfters[i] = whitelist[_investors[i]].canSendTokensAfter; - canReceiveTokensAfters[i] = whitelist[_investors[i]].canReceiveTokensAfter; + canSendAfters[i] = whitelist[_investors[i]].canSendAfter; + canReceiveAfters[i] = whitelist[_investors[i]].canReceiveAfter; expiryTimes[i] = whitelist[_investors[i]].expiryTime; if (whitelist[_investors[i]].canBuyFromSTO == 0) { canBuyFromSTOs[i] = false; @@ -380,7 +380,7 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag canBuyFromSTOs[i] = true; } } - return (canSendTokensAfters, canReceiveTokensAfters, expiryTimes, canBuyFromSTOs); + return (canSendAfters, canReceiveAfters, expiryTimes, canBuyFromSTOs); } /** diff --git a/contracts/storage/GeneralTransferManagerStorage.sol b/contracts/storage/GeneralTransferManagerStorage.sol index 485164c0f..87f886ae7 100644 --- a/contracts/storage/GeneralTransferManagerStorage.sol +++ b/contracts/storage/GeneralTransferManagerStorage.sol @@ -16,8 +16,10 @@ contract GeneralTransferManagerStorage { //from and to timestamps that an investor can send / receive tokens respectively struct TimeRestriction { - uint64 canSendTokensAfter; - uint64 canReceiveTokensAfter; + //the moment when the sale lockup period ends and the investor can freely sell or transfer away their tokens + uint64 canSendAfter; + //the moment when the purchase lockup period ends and the investor can freely purchase or receive from others + uint64 canReceiveAfter; uint64 expiryTime; uint8 canBuyFromSTO; uint8 added; @@ -25,8 +27,8 @@ contract GeneralTransferManagerStorage { // Allows all TimeRestrictions to be offset struct Defaults { - uint64 canSendTokensAfter; - uint64 canReceiveTokensAfter; + uint64 canSendAfter; + uint64 canReceiveAfter; } // Offset to be applied to all timings (except KYC expiry) From 3e0d2cce79fda072e89958fd2a60d84b6d8451fd Mon Sep 17 00:00:00 2001 From: Mudit Gupta Date: Fri, 15 Mar 2019 19:58:53 +0530 Subject: [PATCH 5/5] Added overflow check --- .../modules/TransferManager/GeneralTransferManager.sol | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/contracts/modules/TransferManager/GeneralTransferManager.sol b/contracts/modules/TransferManager/GeneralTransferManager.sol index 2f70f194b..0d2f96700 100644 --- a/contracts/modules/TransferManager/GeneralTransferManager.sol +++ b/contracts/modules/TransferManager/GeneralTransferManager.sol @@ -231,6 +231,12 @@ contract GeneralTransferManager is GeneralTransferManagerStorage, ITransferManag if (whitelist[_investor].added == uint8(0)) { investors.push(_investor); } + require( + uint64(_canSendAfter) == _canSendAfter && + uint64(_canReceiveAfter) == _canReceiveAfter && + uint64(_expiryTime) == _expiryTime, + "uint64 overflow" + ); whitelist[_investor] = TimeRestriction(uint64(_canSendAfter), uint64(_canReceiveAfter), uint64(_expiryTime), canBuyFromSTO, uint8(1)); emit ModifyWhitelist(_investor, now, msg.sender, _canSendAfter, _canReceiveAfter, _expiryTime, _canBuyFromSTO); }