Omniscia WallFair Audit

TokenLock Code Style Findings

TokenLock Code Style Findings

TLK-01C: Function Optimization

TypeSeverityLocation
Gas OptimizationInformationalTokenLock.sol:L106-L118, L123-L124

Description:

The current function implementations of tokensVested and release are inefficient.

Example:

contracts/TokenLock.sol
106function tokensVested(address sender, uint256 timestamp) public view returns (uint256 vestedTokens) {
107 // returns 0 before (start time + cliff period)
108 // initial release is obtained after cliff
109 if (timestamp >= _startTime + _cliffPeriod) {
110 uint256 timeVestedSoFar = Math.min(timestamp - _startTime, _vestingPeriod);
111 uint256 stake = _stakes[sender].totalTokens;
112 // compute initial release as fraction where 10**18 is total
113 uint256 initialRelease = (stake * _initialReleaseFraction) / 10**18;
114 // return initial release + the remainder proportionally to time from vesting start
115 // mul first for best precision, v.8 compiler reverts on overflows
116 vestedTokens = ((stake - initialRelease) * timeVestedSoFar) / _vestingPeriod + initialRelease;
117 }
118}
119
120function release() public {
121 address sender = msg.sender;
122
123 uint256 unlockAmount = tokensVested(sender, block.timestamp) - unlockedTokensOf(sender);
124 UnlockState storage stake = _stakes[sender];
125
126 // this should never happen
127 assert(stake.totalTokens >= stake.unlockedTokens + unlockAmount);
128
129 stake.unlockedTokens += unlockAmount;
130
131 emit LogRelease(sender, unlockAmount);
132
133 token().safeTransfer(sender, unlockAmount);
134}

Recommendation:

The release function should read and store the stake.totalTokens value once to memory and consequently pass it in to the tokensVested function to avoid duplicate retrieval of the UnlockState struct. Additionally, the same caching should be applied to the unlockedTokens value that is retrieved by the unlockedTokensOf function redundantly.

Alleviation:

The caching optimization laid out in this exhibit has been applied to the codebase properly.

TLK-02C: Redundant User-Defined Getters

TypeSeverityLocation
Code StyleInformationalTokenLock.sol:L24, L27, L30, L33, L36L78-L96

Description:

The linked variables are declared as private and have a same-name (barring for the underscore) user-defined getter declared for them.

Example:

contracts/TokenLock.sol
78function token() public view returns (IERC20) {
79 return _token;
80}
81
82function startTime() public view returns (uint256) {
83 return _startTime;
84}
85
86function vestingPeriod() public view returns (uint256) {
87 return _vestingPeriod;
88}
89
90function initialReleaseFraction() public view returns (uint256) {
91 return _initialReleaseFraction;
92}
93
94function cliffPeriod() public view returns (uint256) {
95 return _cliffPeriod;
96}

Recommendation:

We advise the user-defined getters to be omitted from the codebase and the variables to be renamed without the underscore (_) and set as public, generating a getter function for them automatically.

Alleviation:

The development team has acknowledged this exhibit but decided to not apply its remediation in the current version of the codebase.