ERC20VestableInTimestamp Static Analysis Findings

ERC-01S: Prohibitively Costly Removal

Description:

The inActiveSchedule function removes a schedule from the active schedule list but does so in a highly inefficient manner, potentially becoming inoperable if multiple schedules are introduced due to the block gas limit.

Example:

Copy
95function inActiveSchedule(uint256 _id) internal {
96    uint256 index = 0;
97    for (uint256 i = 0; i < allActiveSchedules.length; i++) {
98        if (allActiveSchedules[i] == _id) {
99            index = i;
100            break;
101        }
102    }
103
104    for (uint256 i = index; i < allActiveSchedules.length; i++) {
105        allActiveSchedules[index] = allActiveSchedules[
106            allActiveSchedules.length - 1
107        ];
108    }
109    allActiveSchedules.pop();
110}

Recommendation:

We advise the system to instead perform a single loop, identify the element desired to be removed and replace it with the last element of the array as order does not necessarily need to be kept.

Alleviation:

Our recommendation was applied to the codebase to the letter which now simply swaps the last element of the array with the one to be removed.

ERC-02S: Variable Shadowing

Description:

The updateVestingSchedule function contains a _name argument that shadows the ERC20._name contract variable.

Example:

Copy
138function updateVestingSchedule(
139    string memory _name,
140    uint256 _id,
141    bool _isActive,
142    uint256 _startDay,
143    uint256 _cliffDuration,
144    uint256 _duration
145) public onlyGrantor {

Recommendation:

We advise the argument to be renamed to something that does not cause a naming colission.

Alleviation:

The argument was renamed and refactored to _scheduleNames thus alleviating this exhibit.

ERC-03S: Boolean Literal Comparison

Description:

The third require check within _applyVestingSchedule compares a bool variable with the literal true.

Example:

Copy
197require(
198    vestingSchedules[_vestingSchedule].isActive == true,
199    "vesting schedule is not active"
200);

Recommendation:

We advise the bool variable to be used directly as the equality check is redundant.

Alleviation:

The check was relocated, however, the simplification we recommended has been applied thus dealing with this exhibit.

ERC-04S: Data Location Optimization

Description:

The setWhitelist function is set as public yet is not called internally by the contract and contains a memory argument.

Example:

Copy
117function setWhiteList(address[] memory _whiteListAddresses, bool _status)
118    public
119    onlyGrantor
120{
121    require(_whiteListAddresses.length > 0, "invalid length");
122    for (uint256 i = 0; i < _whiteListAddresses.length; i++) {
123        require(
124            whiteLists[_whiteListAddresses[i]] != _status,
125            "whitelist status is not change"
126        );
127        whiteLists[_whiteListAddresses[i]] = _status;
128    }
129}

Recommendation:

We advise the function's visibility to be restricted to external and its memory argument to be set as calldata greatly optimizing the function's gas cost.

Alleviation:

The data location of the white list addresses is now set to calldata greatly optimizing the codebase.