Golom contest - cRat1st0s's results

Lines of code

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L154

Vulnerability details

2022-07-golom-code4rena Report

• 2022-07-golom-code4rena Report
  • Files Description Table
  • Medium Report
  • Issues found
    • [M-01]: call() should be used instead of transfer() on an address payable
      • Impact
      • Code Affected
      • Mitigation
      • Tools used

Files Description Table

Medium Report

Issues found

[M-01]: call() should be used instead of transfer() on an address payable

Impact

The use of the deprecated transfer() function for an address will inevitably make the transaction fail when:

1. The claimer smart contract does not implement a payable function.
2. The claimer smart contract does implement a payable fallback which uses more than 2300 gas unit.
3. The claimer smart contract implements a payable fallback function that needs less than 2300 gas units but is called through proxy, raising the call’s gas usage above 2300.

Additionally, using higher than 2300 gas might be mandatory for some multisig wallets.

Code Affected

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L154

payable(payAddress).transfer(payAmt); // royalty transfer to royaltyaddress

Mitigation

Use call() instead of transfer().

Tools used

VS Code

2022-07-golom-code4rena Report

• 2022-07-golom-code4rena Report
  • Files Description Table
  • QA Report
  • Issues found
    • [L-01]: Division by zero
      • Impact
      • Code Affected
      • Mitigation
      • Tools used
    • [L-02]: Inconsistency between the code and the docs for dailyEmission
      • Impact
      • Code Affected
      • Mitigation
      • Tools used
    • [L-03]: Inconsistency between the code and the docs for rewardToken.totalSupply()
      • Impact
      • Code Affected
      • Mitigation
      • Tools used
    • [L-04]: Inconsistency between the code and the docs for Staking/Trading/Exchange Rewards
      • Impact
      • Code Affected
      • Mitigation
      • Tools used
    • [L-05]: Hardcoded WETH address
      • Impact
      • Code Affected
      • Mitigation
      • Tools used
    • [N-01]: Typos
      • Impact
      • Code Affected and Mitigation
      • Tools used
    • [N-02]: Missing SPDX identifier
      • Impact
      • Code Affected
      • Mitigation
      • Tools used
    • [N-03]: NatSpec is missing or is incomplete
      • Impact
      • Contracts Affected
      • Mitigation
      • Tools used

Files Description Table

QA Report

Issues found

[L-01]: Division by zero

Impact

tokenToEmit and stakerReward can be invalid because if there's an error in the rewardToken.totalSupply() will evaluate to divide by zero creating inconsistencies in the logic.

Code Affected

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L112-L113

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

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L114

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

Mitigation

Add a check if the value of rewardToken.totalSupply() is zero.

Tools used

VS Code

[L-02]: Inconsistency between the code and the docs for dailyEmission

Impact

Once 150M Golom tokens are given out as Airdrop and 62.5M as Genesis Rewards the daily emissions will adjust as a percentage of circulating supply and they will not be constant and equal to 600,000 tokens. So, this is breaking the project's logic for the emissions.

Code Affected

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L48

uint256 constant dailyEmission = 600000 * 10**18;

Mitigation

I suppose the docs have the correct formula and so the code must be adjusted to what is written in the docs.

Tools used

VS Code

[L-03]: Inconsistency between the code and the docs for rewardToken.totalSupply()

Impact

Golom protocol has a native token which has a maximum supply limit of 1,000,000,000 (1 Billion $GOLOM tokens) but in the function addFee the check rewardToken.totalSupply() > 1000000000 * 10**18 is insufficient. For example, if rewardToken.totalSupply() = 1000000000 * 10**18 it will pass the check and then it will mint more tokens than the maximum supply limit.

Code Affected

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L98-L103

    function addFee(address[2] memory addr, uint256 fee) public onlyTrader {
        //console.log(block.timestamp,epoch,fee);
        if (rewardToken.totalSupply() > 1000000000 * 10**18) {
            // if supply is greater then a billion dont mint anything, dont add trades
            return;
        }

Mitigation

Calculate first the rewards and if totalSupply is exceeding the maximum supply limit them up to 1 Billion.

Tools used

VS Code

[L-04]: Inconsistency between the code and the docs for Staking/Trading/Exchange Rewards

Impact

$GOLOM Allocation & Distribution for Staking/Trading/Exchange Rewards is 68.75% of the supply (687,500,000 $GOLOM) but in the function addFee there is no check that makes sure that the allocation will remain as planned.

Code Affected

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L98

function addFee(address[2] memory addr, uint256 fee) public onlyTrader {

Mitigation

Add a constant variable (totalRewards) that will be equal to 687,500,000 and in function addFee there will be a check to make sure that the rewards will not exceed the 68.75% of the supply.

Tools used

VS Code

[L-05]: Hardcoded WETH address

Impact

WETH address might change if deployed in other chains.

Code Affected

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L45

ERC20 WETH = ERC20(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2);

Mitigation

Consider injecting WETH address via the constructor.

Tools used

VS Code

[N-01]: Typos

Impact

None.

Code Affected and Mitigation

diff --git a/contracts/core/GolomTrader.sol b/contracts/core/GolomTrader.sol
index 70a18ad..85ebe27 100644
--- a/contracts/core/GolomTrader.sol
+++ b/contracts/core/GolomTrader.sol
@@ -50,14 +50,14 @@ contract GolomTrader is Ownable, ReentrancyGuard {
         address signer; // maker of order address
         uint256 orderType; // 0 if selling nft for eth , 1 if offering weth for nft,2 if offering weth for collection with special criteria root
         uint256 totalAmt; // price value of the trade // total amt maker is willing to give up per unit of amount
-        Payment exchange; // payment agreed by maker of the order to pay on succesful filling of trade this amt is subtracted from totalamt
-        Payment prePayment; // another payment , can be used for royalty, facilating trades
+        Payment exchange; // payment agreed by maker of the order to pay on successful filling of trade this amt is subtracted from totalamt
+        Payment prePayment; // another payment , can be used for royalty, facilitating trades
         bool isERC721; // standard of the collection , if 721 then true , if 1155 then false
         uint256 tokenAmt; // token amt useful if standard is 1155 if >1 means whole order can be filled tokenAmt times
         uint256 refererrAmt; // amt to pay to the address that helps in filling your order
         bytes32 root; // A merkle root derived from each valid tokenId — set to 0 to indicate a collection-level or tokenId-specific order.
         address reservedAddress; // if not address(0) , only this address can fill the order
-        uint256 nonce; // nonce of order usefull for cancelling in bulk
+        uint256 nonce; // nonce of order useful for cancelling in bulk
         uint256 deadline; // timestamp till order is valid epoch timestamp in secs
         uint8 v;
         bytes32 r;
@@ -198,7 +198,7 @@ contract GolomTrader is Ownable, ReentrancyGuard {
     /// @param o the Order struct to be filled must be orderType 0
     /// @param amount the amount of times the order is to be filled(useful for ERC1155)
     /// @param referrer referrer of the order
-    /// @param p any extra payment that the taker of this order wanna send on succesful execution of order
+    /// @param p any extra payment that the taker of this order wanna send on successful execution of order
     /// @param receiver address which will receive the NFT
     function fillAsk(
         Order calldata o,
@@ -275,7 +275,7 @@ contract GolomTrader is Ownable, ReentrancyGuard {
     /// @param o the Order struct to be filled must be orderType 1
     /// @param amount the amount of times the order is to be filled(useful for ERC1155)
     /// @param referrer referrer of the order
-    /// @param p any extra payment that the taker of this order wanna send on succesful execution of order
+    /// @param p any extra payment that the taker of this order wanna send on successful execution of order
     function fillBid(
         Order calldata o,
         uint256 amount,
@@ -285,7 +285,7 @@ contract GolomTrader is Ownable, ReentrancyGuard {
         require(
             o.totalAmt * amount >
                 (o.exchange.paymentAmt + o.prePayment.paymentAmt + o.refererrAmt) * amount + p.paymentAmt
-        ); // cause bidder eth is paying for seller payment p , dont take anything extra from seller
+        ); // cause bidder eth is paying for seller payment p , don't take anything extra from seller
         // require eth amt is sufficient
         if (o.reservedAddress != address(0)) {
             require(msg.sender == o.reservedAddress);
@@ -330,7 +330,7 @@ contract GolomTrader is Ownable, ReentrancyGuard {
     /// @param o the Order struct to be filled must be orderType 2
     /// @param amount the amount of times the order is to be filled(useful for ERC1155)
     /// @param referrer referrer of the order
-    /// @param p any extra payment that the taker of this order wanna send on succesful execution of order
+    /// @param p any extra payment that the taker of this order wanna send on successful execution of order
     function fillCriteriaBid(
         Order calldata o,
         uint256 amount,
@@ -367,11 +367,11 @@ contract GolomTrader is Ownable, ReentrancyGuard {
         _settleBalances(o, amount, referrer, p);
     }
 
-    /// @dev function to settle balances when a bid is filled succesfully
+    /// @dev function to settle balances when a bid is filled successfully
     /// @param o the Order struct to be filled must be orderType 1
     /// @param amount the amount of times the order is to be filled(useful for ERC1155)
     /// @param referrer referrer of the order
-    /// @param p any extra payment that the taker of this order wanna send on succesful execution of order
+    /// @param p any extra payment that the taker of this order wanna send on successful execution of order
     function _settleBalances(
         Order calldata o,
         uint256 amount,
diff --git a/contracts/rewards/RewardDistributor.sol b/contracts/rewards/RewardDistributor.sol
index 9e31916..9a919e1 100644
--- a/contracts/rewards/RewardDistributor.sol
+++ b/contracts/rewards/RewardDistributor.sol
@@ -58,10 +58,10 @@ contract RewardDistributor is Ownable {
     mapping(address => mapping(uint256 => uint256)) public feesTrader; // fees accumulated by address of trader per epoch
     mapping(address => mapping(uint256 => uint256)) public feesExchange; // fees accumulated by exchange of trader per epoch
     mapping(uint256 => uint256) public epochTotalFee; // total fee of epoch
-    mapping(uint256 => uint256) public rewardTrader; // reward minted each epoc for trader
-    mapping(uint256 => uint256) public rewardExchange; // reward minted each epoc for exhange
-    mapping(uint256 => uint256) public rewardLP; // reward minted each epoc for LP
-    mapping(uint256 => uint256) public rewardStaker; // reward minted each epoc for stakers
+    mapping(uint256 => uint256) public rewardTrader; // reward minted each epoch for trader
+    mapping(uint256 => uint256) public rewardExchange; // reward minted each epoch for exchange
+    mapping(uint256 => uint256) public rewardLP; // reward minted each epoch for LP
+    mapping(uint256 => uint256) public rewardStaker; // reward minted each epoch for stakers
     mapping(uint256 => uint256) public epochBeginTime; // what time previous epoch ended
     mapping(uint256 => uint256) public claimedUpto; // epoch upto which tokenid has claimed
     mapping(uint256 => mapping(uint256 => uint256)) public claimed; // epoch upto which tokenid has claimed
@@ -92,23 +92,23 @@ contract RewardDistributor is Ownable {
     // at starttime epoch 1 starts , first trade changes epoch from 0 to 1 , emits tokens stores the rewards for epoch 1 ,
     // after 1 day , first trade changes epoch from 1 to 2, changes eth in contract to weth and stores rewardstakedeth , emits tokens stores the rewards for epoch 2
 
-    /// @dev Add fees contributed by the Seller of nft and the exchange/frontend that facilated the trade
+    /// @dev Add fees contributed by the Seller of nft and the exchange/frontend that facilitated the trade
     /// @param addr the address that contributed in fees
     /// @param fee the fee contributed by these addresses
     function addFee(address[2] memory addr, uint256 fee) public onlyTrader {
         //console.log(block.timestamp,epoch,fee);
         if (rewardToken.totalSupply() > 1000000000 * 10**18) {
-            // if supply is greater then a billion dont mint anything, dont add trades
+            // if supply is greater then a billion don't mint anything, don't add trades
             return;
         }
 
         // if 24 hours have passed since last epoch change
         if (block.timestamp > startTime + (epoch) * secsInDay) {
-            // this assumes atleast 1 trade is done daily??????
+            // this assumes at least 1 trade is done daily??????
             // logic to decide how much token to emit
             // emission = daily * (1 - (balance of locker/ total supply))  full if 0 locked and 0 if all locked
             // uint256 tokenToEmit = dailyEmission * rewardToken.balanceOf()/
-            // emissions is decided by epoch begiining locked/circulating , and amount each nft gets also decided at epoch begining
+            // emissions is decided by epoch beginning locked/circulating , and amount each nft gets also decided at epoch beginning
             uint256 tokenToEmit = (dailyEmission * (rewardToken.totalSupply() - rewardToken.balanceOf(address(ve)))) /
                 rewardToken.totalSupply();
             uint256 stakerReward = (tokenToEmit * rewardToken.balanceOf(address(ve))) / rewardToken.totalSupply();
@@ -151,7 +151,7 @@ contract RewardDistributor is Ownable {
         rewardToken.transfer(addr, reward);
     }
 
-    // allows exchange that facilated the nft trades to claim there previous epoch rewards
+    // allows exchange that facilitated the nft trades to claim there previous epoch rewards
     function exchangeClaim(address addr, uint256[] memory epochs) public {
         uint256 reward = 0;
         for (uint256 index = 0; index < epochs.length; index++) {
diff --git a/contracts/vote-escrow/VoteEscrowCore.sol b/contracts/vote-escrow/VoteEscrowCore.sol
index 097cd35..997fbcb 100644
--- a/contracts/vote-escrow/VoteEscrowCore.sol
+++ b/contracts/vote-escrow/VoteEscrowCore.sol
@@ -523,7 +523,7 @@ contract VoteEscrowCore is IERC721, IERC721Metadata {
         }
     }
 
-    /// @dev Exeute transfer of a NFT.
+    /// @dev Execute transfer of a NFT.
     ///      Throws unless `msg.sender` is the current owner, an authorized operator, or the approved
     ///      address for this NFT. (NOTE: `msg.sender` not allowed in internal function so pass `_sender`.)
     ///      Throws if `_to` is the zero address.
@@ -685,7 +685,7 @@ contract VoteEscrowCore is IERC721, IERC721Metadata {
 
     /// @notice Record global and per-user data to checkpoint
     /// @param _tokenId NFT token ID. No user checkpoint if 0
-    /// @param old_locked Pevious locked amount / end lock time for the user
+    /// @param old_locked Previous locked amount / end lock time for the user
     /// @param new_locked New locked amount / end lock time for the user
     function _checkpoint(
         uint256 _tokenId,
diff --git a/contracts/vote-escrow/VoteEscrowDelegation.sol b/contracts/vote-escrow/VoteEscrowDelegation.sol
index e68e8aa..053db7b 100644
--- a/contracts/vote-escrow/VoteEscrowDelegation.sol
+++ b/contracts/vote-escrow/VoteEscrowDelegation.sol
@@ -224,7 +224,7 @@ contract VoteEscrow is VoteEscrowCore, Ownable {
     //     _writeCheckpoint(ownerTokenId, nCheckpoints, checkpoint.delegatedTokenIds);
     // }
 
-    /// @dev Exeute transfer of a NFT.
+    /// @dev Execute transfer of a NFT.
     ///      Throws unless `msg.sender` is the current owner, an authorized operator, or the approved
     ///      address for this NFT. (NOTE: `msg.sender` not allowed in internal function so pass `_sender`.)
     ///      Throws if `_to` is the zero address.

Tools used

VS Code

[N-02]: Missing SPDX identifier

Impact

The need to identify the license for open source software is critical for both reporting purposes and license compliance.

Code Affected

2022-07-golom/contracts/vote-escrow/TokenUriHelper.sol

Mitigation

Choose the appropriate license from the list.

Tools used

VS Code

[N-03]: NatSpec is missing or is incomplete

Impact

NatSpec is incomplete.

Contracts Affected

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L112

function hashPayment(Payment calldata p) private pure returns (bytes32) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L123

function _hashOrder(Order calldata o) private pure returns (bytes32) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L127

function _hashOrderinternal(Order calldata o, uint256[2] memory extra) private pure returns (bytes32) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L151

function payEther(uint256 payAmt, address payAddress) internal {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L312

function cancelOrder(Order calldata o) public nonReentrant {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L334

function fillCriteriaBid(

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L141

function traderClaim(address addr, uint256[] memory epochs) public {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L155

function exchangeClaim(address addr, uint256[] memory epochs) public {

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

function encode(bytes memory data) internal pure returns (string memory) {

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

function _tokenURI(

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

function toString(uint256 value) internal pure returns (string memory) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowDelegation.sol#L94

function _writeCheckpoint(

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowDelegation.sol#L233

 function _transferFrom(

Mitigation

Add in full NatSpec comments for all functions to have complete code documentation.

Tools used

VS Code

2022-07-golom-code4rena Report

• 2022-07-golom-code4rena Report
  • Files Description Table
  • Gas Optimizations
    • [G-01]: Variables: No need to explicitly initialize variables with default values
      • Impact
      • Code Affected:
      • Proof of Concept
      • Mitigation
      • Tools used
    • [G-02]: For-Loops: Pre-increments cost less gas compared to post-increments
      • Impact
      • Code Affected:
      • Proof of Concept
      • Mitigation
      • Tools used
    • [G-03]: For-Loops: Increments can be unchecked
      • Impact
      • Code Affected:
      • Proof of Concept
      • Mitigation
      • Tools used
    • [G-04]: For-Loops: No need to explicitly initialize variables with default values
      • Impact
      • Code Affected:
      • Mitigation
      • Tools used
    • [G-05]: Comparisons: Use != 0 rather than > 0 for unsigned integers in require() statements
      • Impact
      • Affected Code:
      • Mitigation
      • Tools used
    • [G-06]: Use multiple require statements instead of &&
      • Impact
      • Affected Code:
      • Mitigation
      • Tools used
    • [G-07]: Use Custom Errors
      • Impact
      • Mitigation
      • Tools used

Files Description Table

Gas Optimizations

[G-01]: Variables: No need to explicitly initialize variables with default values

Impact

If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0, etc depending on the data type). If you explicitly initialize it with its default value, you are just wasting gas.

Code Affected:

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L45

uint256 public epoch = 0;

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

uint256 reward = 0;

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

uint256 reward = 0;

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

uint256 reward = 0;

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

uint256 rewardEth = 0

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

uint256 reward = 0;

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

uint256 rewardEth = 0;

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

uint256 reward = 0;

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L272

uint256 reward = 0;

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowCore.sol#L735

uint256 block_slope = 0; // dblock/dt

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowCore.sol#L1042

uint256 _min = 0;

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowCore.sol#L1113

uint256 _min = 0;

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowCore.sol#L1133

uint256 d_block = 0;

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowCore.sol#L1134

uint256 d_t = 0;

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowCore.sol#L1211

uint256 dt = 0;

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowDelegation.sol#L50

uint256 public MIN_VOTING_POWER_REQUIRED = 0;

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowDelegation.sol#L147

uint256 lower = 0;

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowDelegation.sol#L170

uint256 votes = 0;

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowDelegation.sol#L189

uint256 votes = 0;

Proof of Concept

Mitigation

Do not initialize variables with default values.

Tools used

VS Code

[G-02]: For-Loops: Pre-increments cost less gas compared to post-increments

Impact

Pre-increments cost less gas compared to post-increments.

Code Affected:

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L415

for (uint256 i = 0; i < proof.length; i++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L143

for (uint256 index = 0; index < epochs.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L157

for (uint256 index = 0; index < epochs.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L180

for (uint256 tindex = 0; tindex < tokenids.length; tindex++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L183

for (uint256 index = 0; index < epochs.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L226

for (uint256 index = 0; index < epoch; index++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L258

for (uint256 index = 0; index < epoch; index++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L273

for (uint256 index = 0; index < epoch; index++) {

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

digits++;

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowDelegation.sol#L171

for (uint256 index = 0; index < delegated.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowDelegation.sol#L189

for (uint256 index = 0; index < delegatednft.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowDelegation.sol#L199

for (uint256 i; i < _array.length; i++) {

Proof of Concept

Mitigation

Change i++ to ++i.

Tools used

VS Code

[G-03]: For-Loops: Increments can be unchecked

Impact

In Solidity 0.8+, there’s a default overflow check on unsigned integers.

Code Affected:

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L415

for (uint256 i = 0; i < proof.length; i++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L143

for (uint256 index = 0; index < epochs.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L157

for (uint256 index = 0; index < epochs.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L180

for (uint256 tindex = 0; tindex < tokenids.length; tindex++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L183

for (uint256 index = 0; index < epochs.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L226

for (uint256 index = 0; index < epoch; index++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L258

for (uint256 index = 0; index < epoch; index++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L273

for (uint256 index = 0; index < epoch; index++) {

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

digits++;

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowDelegation.sol#L171

for (uint256 index = 0; index < delegated.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowDelegation.sol#L189

for (uint256 index = 0; index < delegatednft.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowDelegation.sol#L199

for (uint256 i; i < _array.length; i++) {

Proof of Concept

Mitigation

One example is the code would go from:

        for (uint256 i = 0; i < proof.length; i++) {
            bytes32 proofElement = proof[i];
            if (computedHash <= proofElement) {
                // Hash(current computed hash + current element of the proof)
                computedHash = _efficientHash(computedHash, proofElement);
            } else {
                // Hash(current element of the proof + current computed hash)
                computedHash = _efficientHash(proofElement, computedHash);
            }
        }
        if (computedHash != root) {
            revert('invalid proof');
        }

to:

        for (uint256 i = 0; i < proof.length;) {
            bytes32 proofElement = proof[i];
            if (computedHash <= proofElement) {
                // Hash(current computed hash + current element of the proof)
                computedHash = _efficientHash(computedHash, proofElement);
            } else {
                // Hash(current element of the proof + current computed hash)
                computedHash = _efficientHash(proofElement, computedHash);
            }

            unchecked {
                ++i;
            }
        }

Tools used

VS Code

[G-04]: For-Loops: No need to explicitly initialize variables with default values

Impact

If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0, etc depending on the data type). If you explicitly initialize it with its default value, you are just wasting gas.

Code Affected:

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L415

for (uint256 i = 0; i < proof.length; i++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L143

for (uint256 index = 0; index < epochs.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L157

for (uint256 index = 0; index < epochs.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L180

for (uint256 tindex = 0; tindex < tokenids.length; tindex++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L183

for (uint256 index = 0; index < epochs.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L226

for (uint256 index = 0; index < epoch; index++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L258

for (uint256 index = 0; index < epoch; index++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L273

for (uint256 index = 0; index < epoch; index++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowCore.sol#L745

for (uint256 i = 0; i < 255; ++i) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowCore.sol#L1044

for (uint256 i = 0; i < 128; ++i) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowCore.sol#L1115

for (uint256 i = 0; i < 128; ++i) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowCore.sol#L1167

for (uint256 i = 0; i < 255; ++i) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowDelegation.sol#L171

for (uint256 index = 0; index < delegated.length; index++) {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowDelegation.sol#L189

for (uint256 index = 0; index < delegatednft.length; index++) {

Mitigation

Do not initialize variables with default values.

Tools used

VS Code

[G-05]: Comparisons: Use != 0 rather than > 0 for unsigned integers in require() statements

Impact

When the optimizer is enabled, gas is wasted by doing a greater-than operation, rather than a not-equals operation inside require() statements. When using !=, the optimizer is able to avoid the EQ, ISZERO, and associated operations, by relying on the JUMPI that comes afterwards, which itself checks for zero.

Affected Code:

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowCore.sol#L927

require(_value > 0); // dev: need non-zero value

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowCore.sol#L928

require(_locked.amount > 0, 'No existing lock found');

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowCore.sol#L944

require(_value > 0); // dev: need non-zero value

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowCore.sol#L982

require(_locked.amount > 0, 'No existing lock found');

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowCore.sol#L997

require(_locked.amount > 0, 'Nothing is locked');

Mitigation

Use != 0 rather than > 0 for unsigned integers in require() statements.

Tools used

VS Code

[G-06]: Use multiple require statements instead of &&

Impact

Using multiple require statements can save gas.

Affected Code:

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowCore.sol#L538

require(attachments[_tokenId] == 0 && !voted[_tokenId], 'attached');

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowCore.sol#L894

require(attachments[_from] == 0 && !voted[_from], 'attached');

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/vote-escrow/VoteEscrowDelegation.sol#L239

require(attachments[_tokenId] == 0 && !voted[_tokenId], 'attached');

Mitigation

Use multiple require statements.

Tools used

VS Code

[G-07]: Use Custom Errors

Impact

Less expensive and able to use dynamic information in them.

Mitigation

Use custom errors.

Tools used

VS Code