Golom contest - Funen's results

1. Incorrect calculation between actual code and comment

This comment was not the same as actual code.

            // uint256 tokenToEmit = dailyEmission * rewardToken.balanceOf()/

eversince, an actual code was

            uint256 tokenToEmit = (dailyEmission * (rewardToken.totalSupply() - rewardToken.balanceOf(address(ve)))) /
                rewardToken.totalSupply();

it could be changed into :

             // uint256 tokenToEmit = dailyEmission * (rewardToken.totalSupply - rewardToken.balanceOf((ve))))/ rewardToken.totalSupply();

2. Check setMinter is not zero address

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/governance/GolomToken.sol#L58-L61

function setMinter should validate that _minter is not an empty (0x0) address.

Recommended Mitigation

require(_minter != address(0), "!_minter");

3. Avoid Floatin Pragma's

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/governance/GolomToken.sol#L2

Since it was used ^0.8.11. As the compiler can be use for example 0.8.x and consider locking at this version the same as another. It can be consider using locking the pragma version whenever possible and avoid using a floating pragma in the final deployment. Since it can be problematic, if there are publicly disclosed bugs and issues that affect the current compiler version used.

4. Set value as constant than just a number

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/governance/GolomToken.sol#L52

150_000_000

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/governance/GolomToken.sol#L44

62_500_000

5. Short the code

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L118

            epoch += 1;

6. Typo Comment

begiining into beginning

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L111

7. Incorecct License

Rather than used /// [MIT License] it can be changed into,

// SPDX-License-Identifier: MIT

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/vote-escrow/TokenUriHelper.sol#L1

8. Declare reason string

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L217

mgmtm it can be decleared what it is on above or right side.

1. Short reason string can be used for saving more gas

Every reason string takes at least 32 bytes. Use short reason strings that fits in 32 bytes or it will become more expensive.

/contracts/governance/GolomToken.sol#L24 'GolomToken: only reward distributor can enable'
/contracts/governance/GolomToken.sol#L69 'GolomToken: wait for timelock'
/contracts/vote-escrow/VoteEscrowDelegation.sol#L73 'VEDelegation: Need more voting power'
/contracts/vote-escrow/VoteEscrowDelegation.sol#L99 'VVDelegation: Cannot stake more'
/contracts/rewards/RewardDistributor.sol#L181 'Can only claim for a single Address together'
/contracts/rewards/RewardDistributor.sol#L184 'cant claim for future epochs'
/contracts/rewards/RewardDistributor.sol#L185  'cant claim if already claimed'
/contracts/rewards/RewardDistributor.sol#L292 'RewardDistributor: time not over yet'
/contracts/rewards/RewardDistributor.sol#L309 'RewardDistributor: time not over yet'

2. Saving gas by removing = 0

This implementation code can be saving more gas by removing = 0, it because If a variable was not set/initialized, it is assumed to have default value to 0

Files :

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L142

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L149

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L156

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L175-L176

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L222-L223

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L257

3. Custom Error

Custom errors can be used from Solidity 0.8.4 are cheaper than revert strings. Its cheaper deployment cost and runtime cost when the revert condition is met.

POC

https://blog.soliditylang.org/2021/04/21/custom-errors/

Occurances

/contracts/governance/GolomToken.sol#L24 
/contracts/governance/GolomToken.sol#L69 
/contracts/vote-escrow/VoteEscrowDelegation.sol#L73 
/contracts/vote-escrow/VoteEscrowDelegation.sol#L99 
/contracts/rewards/RewardDistributor.sol#L181 
/contracts/rewards/RewardDistributor.sol#L184
/contracts/rewards/RewardDistributor.sol#L185
/contracts/rewards/RewardDistributor.sol#L292 
/contracts/rewards/RewardDistributor.sol#L309

4. Change number to save more gas

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/rewards/RewardDistributor.sol#L100

used 1e27 instead of 1000000000 it can be saving more gas

another file :

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/governance/GolomToken.sol#L44

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/governance/GolomToken.sol#L52

5. Using ++i than i++ for saving more gas

Using i++ instead ++i for all the loops, the variable i is incremented using i++. It is known that implementation by using ++i costs less gas per iteration than i++.

Tools Used

Manual Review

Occurances

/contracts/rewards/RewardDistributor.sol#L143  for (uint256 index = 0; index < epochs.length; index++) {
/contracts/rewards/RewardDistributor.sol#L157 for (uint256 index = 0; index < epochs.length; index++) {
/contracts/rewards/RewardDistributor.sol#L180 for (uint256 tindex = 0; tindex < tokenids.length; tindex++) {
/contracts/rewards/RewardDistributor.sol#L183 for (uint256 index = 0; index < epochs.length; index++) {
/contracts/rewards/RewardDistributor.sol#L226 for (uint256 index = 0; index < epoch; index++) {
/contracts/rewards/RewardDistributor.sol#L258 for (uint256 index = 0; index < epoch; index++) 
/contracts/rewards/RewardDistributor.sol#L273 for (uint256 index = 0; index < epoch; index++)

6. change uint256 index = 0 into uint256 index for saving more gas

using this implementation can saving more gas for each loops.

Tool Used

Manual Review

Recommended Mitigation

Change it

Occurances

/contracts/rewards/RewardDistributor.sol#L143  for (uint256 index = 0; index < epochs.length; index++) {
/contracts/rewards/RewardDistributor.sol#L157 for (uint256 index = 0; index < epochs.length; index++) {
/contracts/rewards/RewardDistributor.sol#L180 for (uint256 tindex = 0; tindex < tokenids.length; tindex++) {
/contracts/rewards/RewardDistributor.sol#L183 for (uint256 index = 0; index < epochs.length; index++) {
/contracts/rewards/RewardDistributor.sol#L226 for (uint256 index = 0; index < epoch; index++) {
/contracts/rewards/RewardDistributor.sol#L258 for (uint256 index = 0; index < epoch; index++) 
/contracts/rewards/RewardDistributor.sol#L273 for (uint256 index = 0; index < epoch; index++)