Golom contest - IllIllI's results

Lines of code

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

Vulnerability details

Impact

Users can be blocked by other users, from being able to delegate

Proof of Concept

The _writeCheckpoint() function looks up the existing checkpoint and attempts to update its delegation array if one exists (line 104):

File: /contracts/vote-escrow/VoteEscrowDelegation.sol   #1

94       function _writeCheckpoint(
95           uint256 toTokenId,
96           uint256 nCheckpoints,
97           uint256[] memory _delegatedTokenIds
98       ) internal {
99           require(_delegatedTokenIds.length < 500, 'VVDelegation: Cannot stake more');
100  
101          Checkpoint memory oldCheckpoint = checkpoints[toTokenId][nCheckpoints - 1];
102  
103          if (nCheckpoints > 0 && oldCheckpoint.fromBlock == block.number) {
104              oldCheckpoint.delegatedTokenIds = _delegatedTokenIds;
105          } else {
106              checkpoints[toTokenId][nCheckpoints] = Checkpoint(block.number, _delegatedTokenIds);
107              numCheckpoints[toTokenId] = nCheckpoints + 1;
108          }
109:     }

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

However, because the oldCheckpoint variable is a memory variable and not a storage variable, the assignment of _delegatedTokenIds does not actually get assigned to contract storage, and only lives in the memory variable. This means the first user to delegate to a specific token in a block, is the only one able to actually have the delegation applied.

If the malicious user is a miner, that user can monitor the mempool for checkpoint updates and do a sandwich attack, where the order of transactions is:

1. attacker delegates to token X
2. user's txn delegates to token X (but silently fails due to this bug)
3. attacker calls removeDelegation() to reclaim their votes

Tools Used

Code inspection

Recommended Mitigation Steps

Use checkpoints[toTokenId][nCheckpoints] rather than oldCheckpoint.delegatedTokenIds in the assignment on line 104

Lines of code

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

Vulnerability details

Impact

Queries for historical voting power will have the wrong answer, and any DAO/reward decisions made based off it it will be wrong

Proof of Concept

removeDelegation() is marked as external, but is called as if it were public by using the this address:

File: /contracts/vote-escrow/VoteEscrowDelegation.sol   #1

233      function _transferFrom(
234          address _from,
235          address _to,
236          uint256 _tokenId,
237          address _sender
238      ) internal override {
239          require(attachments[_tokenId] == 0 && !voted[_tokenId], 'attached');
240  
241          // remove the delegation
242:         this.removeDelegation(_tokenId);

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

removeDelegation() fetches the current checkpoints, removes the delegated token (if it can find it), and then writes a new checkpoint. However, because it uses a storage variable for the lookup, and then passes it to removeElement(), which also takes in a storage variable, any changes made to the array are immediately applied to the contract's state variable, meaning that the old checkpoint is having its entry being removed, and and then a new checkpoint is written with a copy of the same storage array:

File: /contracts/vote-escrow/VoteEscrowDelegation.sol   #2

210      function removeDelegation(uint256 tokenId) external {
211          require(ownerOf(tokenId) == msg.sender, 'VEDelegation: Not allowed');
212          uint256 nCheckpoints = numCheckpoints[tokenId];
213          Checkpoint storage checkpoint = checkpoints[tokenId][nCheckpoints - 1];
214          removeElement(checkpoint.delegatedTokenIds, tokenId);
215          _writeCheckpoint(tokenId, nCheckpoints, checkpoint.delegatedTokenIds);
216:     }

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

In other words, the token is removed from the old checkpoint, the old checkpoint is saved, and a new checkpoint is added, which has the same values as the old checkpoint. This means that any prior delegations are always removed. This happens any time _transferFrom() is called, which is any time the token is transferred.

Tools Used

Code inspection

Recommended Mitigation Steps

Use memory for the checkpoint variable, as well as the argument to removeElement(), so that a copy of the state array is made and operated upon

Lines of code

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

Vulnerability details

Impact

Delegation, and thus vote tracking, accounting breaks as soon as anyone delegates to a different address than the owner address. Broken accounting means invalid vote tallies, leading to the wrong outcomes for all voting decisions

Proof of Concept

delegate() sets up delegation by fetching the checkpoints of the token to which a token is being delegated, and adds the delegated token to that checkpoint on lines 80 and 84:

File: /contracts/vote-escrow/VoteEscrowDelegation.sol   #1

71       function delegate(uint256 tokenId, uint256 toTokenId) external {
72           require(ownerOf(tokenId) == msg.sender, 'VEDelegation: Not allowed');
73           require(this.balanceOfNFT(tokenId) >= MIN_VOTING_POWER_REQUIRED, 'VEDelegation: Need more voting power');
74   
75           delegates[tokenId] = toTokenId;
76           uint256 nCheckpoints = numCheckpoints[toTokenId];
77   
78           if (nCheckpoints > 0) {
79               Checkpoint storage checkpoint = checkpoints[toTokenId][nCheckpoints - 1];
80               checkpoint.delegatedTokenIds.push(tokenId);
81               _writeCheckpoint(toTokenId, nCheckpoints, checkpoint.delegatedTokenIds);
82           } else {
83               uint256[] memory array = new uint256[](1);
84               array[0] = tokenId;
85               _writeCheckpoint(toTokenId, nCheckpoints, array);
86           }
87   
88           emit DelegateChanged(tokenId, toTokenId, msg.sender);
89:      }

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

Note that above, delgate() assigns tokenId to toTokenId's checkpoints. removeDelegation() on the other hand, removes tokenId from its own checkpoints:

File: /contracts/vote-escrow/VoteEscrowDelegation.sol   #2

208      /// @notice Remove delegation
209      /// @param tokenId TokenId of which delegation needs to be removed
210      function removeDelegation(uint256 tokenId) external {
211          require(ownerOf(tokenId) == msg.sender, 'VEDelegation: Not allowed');
212          uint256 nCheckpoints = numCheckpoints[tokenId];
213          Checkpoint storage checkpoint = checkpoints[tokenId][nCheckpoints - 1];
214          removeElement(checkpoint.delegatedTokenIds, tokenId);
215          _writeCheckpoint(tokenId, nCheckpoints, checkpoint.delegatedTokenIds);
216:     }

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

If tokenId wasn't delegated to itself, then the call to removeElement() will not fail, and a new checkpoint will be written with the token still being delegated to its old location:

File: /contracts/vote-escrow/VoteEscrowDelegation.sol   #3

198      function removeElement(uint256[] storage _array, uint256 _element) internal {
199          for (uint256 i; i < _array.length; i++) {
200              if (_array[i] == _element) {
201                  _array[i] = _array[_array.length - 1];
202                  _array.pop();
203                  break;
204              }
205          }
206:     }

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

Tools Used

Code inspection

Recommended Mitigation Steps

Use a storage mapping for tracking which toTokenId the token is currently delegated to, and remove from that token's checkpoints

Judge has assessed an item in Issue #796 as Medium risk. The relevant finding follows:

[L‑01] Use of transferFrom() rather than safeTransferFrom() for NFTs in will lead to the loss of NFTs The EIP-721 standard says the following about transferFrom():

    /// @notice Transfer ownership of an NFT -- THE CALLER IS RESPONSIBLE
    ///  TO CONFIRM THAT `_to` IS CAPABLE OF RECEIVING NFTS OR ELSE
    ///  THEY MAY BE PERMANENTLY LOST
    /// @dev Throws unless `msg.sender` is the current owner, an authorized
    ///  operator, or the approved address for this NFT. Throws if `_from` is
    ///  not the current owner. Throws if `_to` is the zero address. Throws if
    ///  `_tokenId` is not a valid NFT.
    /// @param _from The current owner of the NFT
    /// @param _to The new owner
    /// @param _tokenId The NFT to transfer
    function transferFrom(address _from, address _to, uint256 _tokenId) external payable;

https://github.com/ethereum/EIPs/blob/78e2c297611f5e92b6a5112819ab71f74041ff25/EIPS/eip-721.md?plain=1#L103-L113 Code must use the safeTransferFrom() flavor if it hasn't otherwise verified that the receiving address can handle it

There is 1 instance of this issue:

File: contracts/core/GolomTrader.sol

236:              ERC721(o.collection).transferFrom(o.signer, receiver, o.tokenId);

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

Lines of code

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

Vulnerability details

Impact

Tokens may be lost when given to non-EOA accounts

Proof of Concept

The code has the following warning for safeTransferFrom():

File: /contracts/vote-escrow/VoteEscrowCore.sol   #1

83       /**
84        * @dev Safely transfers `tokenId` token from `from` to `to`, checking first that contract recipients
85:       * are aware of the ERC721 protocol to prevent tokens from being forever locked.

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

The minting code, which creates the locks, does not do this check:

File: /contracts/vote-escrow/VoteEscrowCore.sol   #2

677      function _mint(address _to, uint256 _tokenId) internal returns (bool) {
678          // Throws if `_to` is zero address
679          assert(_to != address(0));
680          // Add NFT. Throws if `_tokenId` is owned by someone
681          _addTokenTo(_to, _tokenId);
682          emit Transfer(address(0), _to, _tokenId);
683          return true;
684:     }

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

As a separate issue, once a lock is already minted, if it's transfered to a contract, the code does call onERC721Received():

File: /contracts/vote-escrow/VoteEscrowCore.sol   #3

594      function safeTransferFrom(
595          address _from,
596          address _to,
597          uint256 _tokenId,
598          bytes memory _data
599      ) public {
600          _transferFrom(_from, _to, _tokenId, msg.sender);
601  
602          if (_isContract(_to)) {
603              // Throws if transfer destination is a contract which does not implement 'onERC721Received'
604              try IERC721Receiver(_to).onERC721Received(msg.sender, _from, _tokenId, _data) returns (bytes4) {} catch (
605                  bytes memory reason
606              ) {
607                  if (reason.length == 0) {
608                      revert('ERC721: transfer to non ERC721Receiver implementer');
609                  } else {
610                      assembly {
611                          revert(add(32, reason), mload(reason))
612                      }
613                  }
614              }
615          }
616:     }

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

While the transfer function does in fact execute onERC721Received(), it doesn't actually do a check of the bytes4 variable - it only checks for a non-zero length. The ERC721 standard says that for the function call to be valid, it must return the bytes4 function selector, otherwise it's invalid. If a user of the escrow system uses a contract that is attempting to explicitly reject NFTs by returning zero in its onERC721Received() function, the VotingEscrowCore will interpret that response as success and will transfer the NFT, potentially locking it forever. If the lock is minted to a contract, no checks are done, and the NFT can be locked forever.

Tools Used

Code inspection

Recommended Mitigation Steps

Call onERC721Received() in _mint() and ensure that the return value equals IERC721Receiver.onERC721Received.selector in both _mint() and safeTransferFrom()

Lines of code

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

Vulnerability details

Impact

Approved users are unable to call merge() or withdraw(), breaking the basic fuctionality of the protocol

Proof of Concept

withdraw() checks that the user is either the owner or the approver using the _isApprovedOrOwner() function on line 1007, and later calls _burn() on line 1026:

File: /contracts/vote-escrow/VoteEscrowCore.sol   #1

1004     /// @notice Withdraw all tokens for `_tokenId`
1005     /// @dev Only possible if the lock has expired
1006     function withdraw(uint256 _tokenId) external nonreentrant {
1007         assert(_isApprovedOrOwner(msg.sender, _tokenId));
1008         require(attachments[_tokenId] == 0 && !voted[_tokenId], 'attached');
1009 
1010         LockedBalance memory _locked = locked[_tokenId];
1011         require(block.timestamp >= _locked.end, "The lock didn't expire");
1012         uint256 value = uint256(int256(_locked.amount));
1013 
1014         locked[_tokenId] = LockedBalance(0, 0);
1015         uint256 supply_before = supply;
1016         supply = supply_before - value;
1017 
1018         // old_locked can have either expired <= timestamp or zero end
1019         // _locked has only 0 end
1020         // Both can have >= 0 amount
1021         _checkpoint(_tokenId, _locked, LockedBalance(0, 0));
1022 
1023         assert(IERC20(token).transfer(msg.sender, value));
1024 
1025         // Burn the NFT
1026         _burn(_tokenId);
1027 
1028         emit Withdraw(msg.sender, _tokenId, value, block.timestamp);
1029         emit Supply(supply_before, supply_before - value);
1030:    }

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

_burn() correctly calls _isApprovedOrOwner(), but when it does the transfer, it uses msg.sender on line 1234:

File: /contracts/vote-escrow/VoteEscrowCore.sol   #2

1226     function _burn(uint256 _tokenId) internal {
1227         require(_isApprovedOrOwner(msg.sender, _tokenId), 'caller is not owner nor approved');
1228 
1229         address owner = ownerOf(_tokenId);
1230 
1231         // Clear approval
1232         approve(address(0), _tokenId);
1233         // Remove token
1234         _removeTokenFrom(msg.sender, _tokenId);
1235         emit Transfer(owner, address(0), _tokenId);
1236:    }

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

_removeTokenFrom() will only work for the owner of the token:

File: /contracts/vote-escrow/VoteEscrowCore.sol   #3

502      /// @dev Remove a NFT from a given address
503      ///      Throws if `_from` is not the current owner.
504      function _removeTokenFrom(address _from, uint256 _tokenId) internal {
505          // Throws if `_from` is not the current owner
506:         assert(idToOwner[_tokenId] == _from);

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

merge() has the same _burn() issue on line 906:

File: /contracts/vote-escrow/VoteEscrowCore.sol   #4

893      function merge(uint256 _from, uint256 _to) external {
894          require(attachments[_from] == 0 && !voted[_from], 'attached');
895          require(_from != _to);
896          require(_isApprovedOrOwner(msg.sender, _from));
897          require(_isApprovedOrOwner(msg.sender, _to));
898  
899          LockedBalance memory _locked0 = locked[_from];
900          LockedBalance memory _locked1 = locked[_to];
901          uint256 value0 = uint256(int256(_locked0.amount));
902          uint256 end = _locked0.end >= _locked1.end ? _locked0.end : _locked1.end;
903  
904          locked[_from] = LockedBalance(0, 0);
905          _checkpoint(_from, _locked0, LockedBalance(0, 0));
906:         _burn(_from);

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

Tools Used

Code inspection

Recommended Mitigation Steps

In _burn(), change the argument to _removeTokenFrom() from msg.sender to owner

Lines of code

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

Vulnerability details

Impact

Users can get unlimited votes which leads to them:

• gaining control over governance
• getting undeserved rewards

Proof of Concept

Users are required to call delegate() to set up the initial token delegation. Thereafter, the overridden version of _transferFrom() removes the delegation on line 242:

File: /contracts/vote-escrow/VoteEscrowDelegation.sol   #1

233      function _transferFrom(
234          address _from,
235          address _to,
236          uint256 _tokenId,
237          address _sender
238      ) internal override {
239          require(attachments[_tokenId] == 0 && !voted[_tokenId], 'attached');
240  
241          // remove the delegation
242          this.removeDelegation(_tokenId);
243  
244          // Check requirements
245          require(_isApprovedOrOwner(_sender, _tokenId));
246          // Clear approval. Throws if `_from` is not the current owner
247          _clearApproval(_from, _tokenId);
248          // Remove NFT. Throws if `_tokenId` is not a valid NFT
249          _removeTokenFrom(_from, _tokenId);
250          // Add NFT
251          _addTokenTo(_to, _tokenId);
252          // Set the block of ownership transfer (for Flash NFT protection)
253          ownership_change[_tokenId] = block.number;
254          // Log the transfer
255          emit Transfer(_from, _to, _tokenId);
256:     }

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

But _burn() is not overriden, and doesn't alter delegation:

File: /contracts/vote-escrow/VoteEscrowCore.sol   #2

1226     function _burn(uint256 _tokenId) internal {
1227         require(_isApprovedOrOwner(msg.sender, _tokenId), 'caller is not owner nor approved');
1228 
1229         address owner = ownerOf(_tokenId);
1230 
1231         // Clear approval
1232         approve(address(0), _tokenId);
1233         // Remove token
1234         _removeTokenFrom(msg.sender, _tokenId);
1235         emit Transfer(owner, address(0), _tokenId);
1236:    }

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

_burn() is called by both merge() and withdraw(), so to get free votes, an attacker can deposit a token, lock it, wait for the lock to expire, withdraw() to or mege() to another address, and repeat.

Tools Used

Code inspection

Recommended Mitigation Steps

Override _burn() and have the new version call removeDelegation()

Summary

Low Risk Issues

Total: 31 instances over 10 issues

Non-critical Issues

Total: 199 instances over 26 issues

Low Risk Issues

[L‑01] Use of transferFrom() rather than safeTransferFrom() for NFTs in will lead to the loss of NFTs

The EIP-721 standard says the following about transferFrom():

    /// @notice Transfer ownership of an NFT -- THE CALLER IS RESPONSIBLE
    ///  TO CONFIRM THAT `_to` IS CAPABLE OF RECEIVING NFTS OR ELSE
    ///  THEY MAY BE PERMANENTLY LOST
    /// @dev Throws unless `msg.sender` is the current owner, an authorized
    ///  operator, or the approved address for this NFT. Throws if `_from` is
    ///  not the current owner. Throws if `_to` is the zero address. Throws if
    ///  `_tokenId` is not a valid NFT.
    /// @param _from The current owner of the NFT
    /// @param _to The new owner
    /// @param _tokenId The NFT to transfer
    function transferFrom(address _from, address _to, uint256 _tokenId) external payable;

https://github.com/ethereum/EIPs/blob/78e2c297611f5e92b6a5112819ab71f74041ff25/EIPS/eip-721.md?plain=1#L103-L113 Code must use the safeTransferFrom() flavor if it hasn't otherwise verified that the receiving address can handle it

There is 1 instance of this issue:

File: contracts/core/GolomTrader.sol

236:              ERC721(o.collection).transferFrom(o.signer, receiver, o.tokenId);

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

[L‑02] Don't use payable.transfer()/payable.send()

The use of payable.transfer() is heavily frowned upon because it can lead to the locking of funds. The transfer() call requires that the recipient is either an EOA account, or is a contract that has a payable callback. For the contract case, the transfer() call only provides 2300 gas for the contract to complete its operations. This means the following cases can cause the transfer to fail:

• The contract does not have a payable callback
• The contract's payable callback spends more than 2300 gas (which is only enough to emit something)
• The contract is called through a proxy which itself uses up the 2300 gas Use OpenZeppelin's Address.sendValue() instead

There is 1 instance of this issue:

File: contracts/core/GolomTrader.sol

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

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

[L‑03] Unused/empty receive()/fallback() function

If the intention is for the Ether to be used, the function should call another function, otherwise it should revert (e.g. require(msg.sender == address(weth)))

There are 4 instances of this issue:

File: contracts/core/GolomTrader.sol

459:      fallback() external payable {}

461:      receive() external payable {}

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

File: contracts/rewards/RewardDistributor.sol

313:      fallback() external payable {}

315:      receive() external payable {}

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

[L‑04] require() should be used instead of assert()

Prior to solidity version 0.8.0, hitting an assert consumes the remainder of the transaction's available gas rather than returning it, as require()/revert() do. assert() should be avoided even past solidity version 0.8.0 as its documentation states that "The assert function creates an error of type Panic(uint256). ... Properly functioning code should never create a Panic, not even on invalid external input. If this happens, then there is a bug in your contract which you should fix".

There are 13 instances of this issue:

File: contracts/vote-escrow/VoteEscrowCore.sol

493:          assert(idToOwner[_tokenId] == address(0));

506:          assert(idToOwner[_tokenId] == _from);

519:          assert(idToOwner[_tokenId] == _owner);

666:          assert(_operator != msg.sender);

679:          assert(_to != address(0));

861:              assert(IERC20(token).transferFrom(from, address(this), _value));

977:          assert(_isApprovedOrOwner(msg.sender, _tokenId));

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

991:          assert(_isApprovedOrOwner(msg.sender, _tokenId));

1007:         assert(_isApprovedOrOwner(msg.sender, _tokenId));

1023:         assert(IERC20(token).transfer(msg.sender, value));

1110:         assert(_block <= block.number);

1206:         assert(_block <= block.number);

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

[L‑05] Self-delegation is not automatic

Unlike some of the other functions, _mint() isn't overridden to call delegate(), which means the user may forget to do so and will miss out

There is 1 instance of this issue:

File: /contracts/vote-escrow/VoteEscrowCore.sol

677      function _mint(address _to, uint256 _tokenId) internal returns (bool) {
678          // Throws if `_to` is zero address
679          assert(_to != address(0));
680          // Add NFT. Throws if `_tokenId` is owned by someone
681          _addTokenTo(_to, _tokenId);
682          emit Transfer(address(0), _to, _tokenId);
683          return true;
684:     }

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

[L‑06] Function may run out of gas

Once the number of epochs grow to a large number, the array allocated will be large, and the number of iterations calling external functions on ve will also be large, leading to the function running out of gas

There is 1 instance of this issue:

File: /contracts/rewards/RewardDistributor.sol

215      function stakerRewards(uint256 tokenid) public view returns (
216              uint256,
217              uint256,
218              uint256[] memory
219          ){
220          require(address(ve) != address(0), ' VE not added yet');
221  
222          uint256 reward = 0;
223          uint256 rewardEth = 0;
224          uint256[] memory unclaimedepochs = new uint256[](epoch);
225          // for each epoch
226:         for (uint256 index = 0; index < epoch; index++) {

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

[L‑07] Vulnerable to cross-chain replay attacks due to static DOMAIN_SEPARATOR/domainSeparator

See this issue from a prior contest for details

There is 1 instance of this issue:

File: /contracts/core/GolomTrader.sol

101          EIP712_DOMAIN_TYPEHASH = keccak256(
102              abi.encode(
103                  keccak256('EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)'),
104                  keccak256(bytes('GOLOM.IO')),
105                  keccak256(bytes('1')),
106                  chainId,
107                  address(this)
108              )
109:         );

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

[L‑08] Wrong comment

The function description and return values are incorrectly copied from another function

There is 1 instance of this issue:

File: /contracts/rewards/RewardDistributor.sol

252      /// @dev returns unclaimed rewards of an NFT, returns (unclaimed golom rewards, unclaimed eth rewards, unclaimed epochs)
253      /// @param addr the nft id to claim rewards for all ids in the list must belong to 1 address
254      function traderRewards(address addr) public view returns (
255              uint256        
256:             ){

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

[L‑09] Missing checks for address(0x0) when assigning values to address state variables

There are 7 instances of this issue:

File: contracts/core/GolomTrader.sol

448:              pendingDistributor = _distributor;

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

File: contracts/governance/GolomToken.sol

59:           pendingMinter = _minter;

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/governance/GolomToken.sol#L59

File: contracts/rewards/RewardDistributor.sol

81:           trader = _trader;

287:          pendingTrader = _trader;

303:              pendingVoteEscrow = _voteEscrow;

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

File: contracts/vote-escrow/VoteEscrowCore.sol

870:          voter = _voter;

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

File: contracts/vote-escrow/VoteEscrowDelegation.sol

53:           token = _token;

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

[L‑10] Only a billion checkpoints available

A user can only have a billion checkpoints which, if the user is a DAO, may cause issues down the line, especially if the last checkpoint involved delegating and can thereafter not be undone

There is 1 instance of this issue:

File: contracts/contracts/VotingEscrow.sol

535:     mapping(uint => Point[1000000000]) public user_point_history; // user -> Point[user_epoch]

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L535

Non-critical Issues

[N‑01] Consider addings checks for signature malleability

Use OpenZeppelin's ECDSA contract rather than calling ecrecover() directly

There is 1 instance of this issue:

File: /contracts/core/GolomTrader.sol

176          address signaturesigner = ecrecover(hash, o.v, o.r, o.s);
177          require(signaturesigner == o.signer, 'invalid signature');
178          if (signaturesigner != o.signer) {
179              return (0, hashStruct, 0);
180:         }

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

[N‑02] ecrecover() signature validity not checked

ecrecover() returns the zero address if the signature is invalid. If the signer provided is also zero, then all incorrect signatures will be allowed

There is 1 instance of this issue:

File: /contracts/core/GolomTrader.sol

176          address signaturesigner = ecrecover(hash, o.v, o.r, o.s);
177          require(signaturesigner == o.signer, 'invalid signature');
178          if (signaturesigner != o.signer) {
179              return (0, hashStruct, 0);
180:         }

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

[N‑03] Boilerplate not replaced

There are 2 instances of this issue:

File: /contracts/governance/GolomToken.sol

5:   /// @notice Explain to an end user what this does

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/governance/GolomToken.sol#L5

File: /contracts/vote-escrow/VoteEscrowDelegation.sol

68:      /// @notice Explain to an end user what this does

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

[N‑04] Remove commented out code

There is 1 instance of this issue:

File: /contracts/vote-escrow/VoteEscrowDelegation.sol

218      // /// @notice Remove delegation by user
219      // function removeDelegationByOwner(uint256 delegatedTokenId, uint256 ownerTokenId) external {
220      //     require(ownerOf(ownerTokenId) == msg.sender, 'VEDelegation: Not allowed');
221      //     uint256 nCheckpoints = numCheckpoints[delegatedTokenId];
222      //     Checkpoint storage checkpoint = checkpoints[delegatedTokenId][nCheckpoints - 1];
223      //     removeElement(checkpoint.delegatedTokenIds, delegatedTokenId);
224      //     _writeCheckpoint(ownerTokenId, nCheckpoints, checkpoint.delegatedTokenIds);
225:     // }

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

[N‑05] Invalid/extraneous/optional function definitions in interface

There are 4 instances of this issue:

File: contracts/core/GolomTrader.sol

/// @audit withdraw(uint256) isn't defined with those arguments in the standard ERC20 definition
33:       function withdraw(uint256 wad) external;

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

File: contracts/rewards/RewardDistributor.sol

/// @audit mint(address,uint256) isn't defined with those arguments in the standard ERC20 definition
24:       function mint(address account, uint256 amount) external;

/// @audit balanceOfNFTAt(uint256,uint256) isn't defined with those arguments in the standard ERC20 definition
26:       function balanceOfNFTAt(uint256 _tokenId, uint256 _t) external view returns (uint256);

/// @audit deposit() isn't defined with those arguments in the standard ERC20 definition
28:       function deposit() external payable;

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

[N‑06] Remove include for hardhat's console

There is 1 instance of this issue:

File: contracts/rewards/RewardDistributor.sol

9:    import 'hardhat/console.sol';

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

[N‑07] Contract implements interface without extending the interface

Not extending the interface may lead to the wrong function signature being used, leading to unexpected behavior. If the interface is in fact being implemented, use the override keyword to indicate that fact

There is 1 instance of this issue:

File: contracts/vote-escrow/VoteEscrowCore.sol

/// @audit IERC721Enumerable.tokenOfOwnerByIndex()
275:  contract VoteEscrowCore is IERC721, IERC721Metadata {

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

[N‑08] require()/revert() statements should have descriptive reason strings

There are 31 instances of this issue:

File: contracts/core/GolomTrader.sol

220:              require(msg.sender == o.reservedAddress);

285           require(
286               o.totalAmt * amount >
287                   (o.exchange.paymentAmt + o.prePayment.paymentAmt + o.refererrAmt) * amount + p.paymentAmt
288:          ); // cause bidder eth is paying for seller payment p , dont take anything extra from seller

291:              require(msg.sender == o.reservedAddress);

293:          require(o.orderType == 1);

295:          require(status == 3);

296:          require(amountRemaining >= amount);

313:          require(o.signer == msg.sender);

342:          require(o.totalAmt >= o.exchange.paymentAmt + o.prePayment.paymentAmt + o.refererrAmt);

345:              require(msg.sender == o.reservedAddress);

347:          require(o.orderType == 2);

349:          require(status == 3);

350:          require(amountRemaining >= amount);

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

File: contracts/rewards/RewardDistributor.sol

88:           require(msg.sender == trader);

144:              require(epochs[index] < epoch);

158:              require(epochs[index] < epoch);

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

File: contracts/vote-escrow/VoteEscrowCore.sol

360:          require(_entered_state == _not_entered);

540:          require(_isApprovedOrOwner(_sender, _tokenId));

646:          require(owner != address(0));

648:          require(_approved != owner);

652:          require(senderIsOwner || senderIsApprovedForAll);

869:          require(msg.sender == voter);

874:          require(msg.sender == voter);

879:          require(msg.sender == voter);

884:          require(msg.sender == voter);

889:          require(msg.sender == voter);

895:          require(_from != _to);

896:          require(_isApprovedOrOwner(msg.sender, _from));

897:          require(_isApprovedOrOwner(msg.sender, _to));

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

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

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

File: contracts/vote-escrow/VoteEscrowDelegation.sol

245:          require(_isApprovedOrOwner(_sender, _tokenId));

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

[N‑09] public functions not called by the contract should be declared external instead

Contracts are allowed to override their parents' functions and change the visibility from external to public.

There are 13 instances of this issue:

File: contracts/core/GolomTrader.sol

203       function fillAsk(
204           Order calldata o,
205           uint256 amount,
206           address referrer,
207           Payment calldata p,
208           address receiver
209:      ) public payable nonReentrant {

279       function fillBid(
280           Order calldata o,
281           uint256 amount,
282           address referrer,
283           Payment calldata p
284:      ) public nonReentrant {

312:      function cancelOrder(Order calldata o) public nonReentrant {

334       function fillCriteriaBid(
335           Order calldata o,
336           uint256 amount,
337           uint256 tokenId,
338           bytes32[] calldata proof,
339           address referrer,
340           Payment calldata p
341:      ) public nonReentrant {

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

File: contracts/rewards/RewardDistributor.sol

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

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

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

172:      function multiStakerClaim(uint256[] memory tokenids, uint256[] memory epochs) public {

215       function stakerRewards(uint256 tokenid) public view returns (
216               uint256,
217               uint256,
218:              uint256[] memory

254       function traderRewards(address addr) public view returns (
255:              uint256        

269       function exchangeRewards(address addr) public view returns (
270:              uint256

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

File: contracts/vote-escrow/TokenUriHelper.sol

66        function _tokenURI(
67            uint256 _tokenId,
68            uint256 _balanceOf,
69            uint256 _locked_end,
70            uint256 _value
71:       ) public pure returns (string memory output) {

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

File: contracts/vote-escrow/VoteEscrowDelegation.sol

185:      function getPriorVotes(uint256 tokenId, uint256 blockNumber) public view returns (uint256) {

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

[N‑10] Non-assembly method available

assembly{ id := chainid() } => uint256 id = block.chainid, assembly { size := extcodesize() } => uint256 size = address().code.length There are some automated tools that will flag a project as having higher complexity if there is inline-assembly, so it's best to avoid using it where it's not necessary

There are 2 instances of this issue:

File: contracts/core/GolomTrader.sol

98:               chainId := chainid()

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

File: contracts/vote-escrow/VoteEscrowCore.sol

577:              size := extcodesize(account)

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

[N‑11] constants should be defined rather than using magic numbers

Even assembly can benefit from using readable constants instead of hex/numeric literals

There are 49 instances of this issue:

File: contracts/core/GolomTrader.sol

/// @audit 50
/// @audit 10000
212:              o.totalAmt >= o.exchange.paymentAmt + o.prePayment.paymentAmt + o.refererrAmt + (o.totalAmt * 50) / 10000,

/// @audit 3
226:          require(status == 3, 'order not valid');

/// @audit 50
/// @audit 10000
242:          payEther(((o.totalAmt * 50) / 10000) * amount, address(distributor));

/// @audit 50
254:                      (o.totalAmt * 50) /

/// @audit 10000
255:                      10000 -

/// @audit 50
/// @audit 10000
263:                  (o.totalAmt - (o.totalAmt * 50) / 10000 - o.exchange.paymentAmt - o.prePayment.paymentAmt) * amount,

/// @audit 50
/// @audit 10000
269:          distributor.addFee([o.signer, o.exchange.paymentAddress], ((o.totalAmt * 50) / 10000) * amount);

/// @audit 3
295:          require(status == 3);

/// @audit 3
349:          require(status == 3);

/// @audit 50
/// @audit 10000
381:          uint256 protocolfee = ((o.totalAmt * 50) / 10000) * amount;

/// @audit 0x00
436:              mstore(0x00, a)

/// @audit 0x20
437:              mstore(0x20, b)

/// @audit 0x00
/// @audit 0x40
438:              value := keccak256(0x00, 0x40)

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

File: contracts/governance/GolomToken.sol

/// @audit 150_000_000
/// @audit 1e18
44:           _mint(_airdrop, 150_000_000 * 1e18);

/// @audit 62_500_000
/// @audit 1e18
52:           _mint(_rewardDistributor, 62_500_000 * 1e18);

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

File: contracts/rewards/RewardDistributor.sol

/// @audit 1659211200
84:           startTime = 1659211200;

/// @audit 1000000000
/// @audit 18
100:          if (rewardToken.totalSupply() > 1000000000 * 10**18) {

/// @audit 67
/// @audit 100
120:              rewardTrader[epoch] = ((tokenToEmit - stakerReward) * 67) / 100;

/// @audit 33
/// @audit 100
121:              rewardExchange[epoch] = ((tokenToEmit - stakerReward) * 33) / 100;

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

File: contracts/vote-escrow/TokenUriHelper.sol

/// @audit 4
/// @audit 3
17:           uint256 encodedLen = 4 * ((len + 2) / 3);

/// @audit 32
20:           bytes memory result = new bytes(encodedLen + 32);

/// @audit 0xffffff
34:                   let input := and(mload(add(data, i)), 0xffffff)

/// @audit 0x3F
36:                   let out := mload(add(tablePtr, and(shr(18, input), 0x3F)))

/// @audit 0x3F
/// @audit 0xFF
38:                   out := add(out, and(mload(add(tablePtr, and(shr(12, input), 0x3F))), 0xFF))

/// @audit 0x3F
/// @audit 0xFF
40:                   out := add(out, and(mload(add(tablePtr, and(shr(6, input), 0x3F))), 0xFF))

/// @audit 0x3F
/// @audit 0xFF
42:                   out := add(out, and(mload(add(tablePtr, and(input, 0x3F))), 0xFF))

/// @audit 0x3d3d
52:                   mstore(sub(resultPtr, 2), shl(240, 0x3d3d))

/// @audit 0x3d
55:                   mstore(sub(resultPtr, 1), shl(248, 0x3d))

/// @audit 48
144:              buffer[digits] = bytes1(uint8(48 + uint256(value % 10)));

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

File: contracts/vote-escrow/VoteEscrowCore.sol

/// @audit 255
745:              for (uint256 i = 0; i < 255; ++i) {

/// @audit 128
1044:         for (uint256 i = 0; i < 128; ++i) {

/// @audit 128
1115:         for (uint256 i = 0; i < 128; ++i) {

/// @audit 255
1167:         for (uint256 i = 0; i < 255; ++i) {

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

File: contracts/vote-escrow/VoteEscrowDelegation.sol

/// @audit 500
99:           require(_delegatedTokenIds.length < 500, 'VVDelegation: Cannot stake more');

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

[N‑12] Numeric values having to do with time should use time units for readability

There are units for seconds, minutes, hours, days, and weeks, and since they're defined, they should be used

There are 5 instances of this issue:

File: contracts/rewards/RewardDistributor.sol

/// @audit 600000
48:       uint256 constant dailyEmission = 600000 * 10**18;

/// @audit 60
/// @audit 60
57:       uint256 constant secsInDay = 24 * 60 * 60;

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

File: contracts/vote-escrow/VoteEscrowCore.sol

/// @audit 86400
296:      uint256 internal constant MAXTIME = 4 * 365 * 86400;

/// @audit 86400
297:      int128 internal constant iMAXTIME = 4 * 365 * 86400;

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

[N‑13] Large multiples of ten should use scientific notation (e.g. 1e6) rather than decimal literals (e.g. 1000000), for readability

There are 2 instances of this issue:

File: contracts/rewards/RewardDistributor.sol

100:          if (rewardToken.totalSupply() > 1000000000 * 10**18) {

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

File: contracts/vote-escrow/VoteEscrowCore.sol

308:      mapping(uint256 => Point[1000000000]) public user_point_history; // user -> Point[user_epoch]

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

[N‑14] Missing event and or timelock for critical parameter change

Events help non-contract tools to track changes, and events prevent users from being surprised by changes

There are 3 instances of this issue:

File: contracts/core/GolomTrader.sol

444       function setDistributor(address _distributor) external onlyOwner {
445           if (address(distributor) == address(0)) {
446               distributor = Distributor(_distributor);
447           } else {
448               pendingDistributor = _distributor;
449               distributorEnableDate = block.timestamp + 1 days;
450           }
451:      }

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

File: contracts/governance/GolomToken.sol

58        function setMinter(address _minter) external onlyOwner {
59            pendingMinter = _minter;
60            minterEnableDate = block.timestamp + 1 days;
61:       }

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

File: contracts/vote-escrow/VoteEscrowCore.sol

868       function setVoter(address _voter) external {
869           require(msg.sender == voter);
870           voter = _voter;
871:      }

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

[N‑15] Use a more recent version of solidity

Use a solidity version of at least 0.8.12 to get string.concat() to be used instead of abi.encodePacked(<str>,<str>)

There are 2 instances of this issue:

File: contracts/core/GolomTrader.sol

3:    pragma solidity 0.8.11;

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

File: contracts/vote-escrow/TokenUriHelper.sol

3:    pragma solidity 0.8.11;

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

[N‑16] Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18)

While the compiler knows to optimize away the exponentiation, it's still better coding practice to use idioms that do not require compiler optimization, if they exist

There are 2 instances of this issue:

File: contracts/rewards/RewardDistributor.sol

48:       uint256 constant dailyEmission = 600000 * 10**18;

100:          if (rewardToken.totalSupply() > 1000000000 * 10**18) {

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

[N‑17] Inconsistent spacing in comments

Some lines use // x and some use //x. The instances below point out the usages that don't follow the majority, within each file

There are 2 instances of this issue:

File: contracts/core/GolomTrader.sol

181:          //deadline

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

File: contracts/rewards/RewardDistributor.sol

99:           //console.log(block.timestamp,epoch,fee);

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

[N‑18] Lines are too long

Usually lines in source code are limited to 80 characters. Today's screens are much larger so it's reasonable to stretch this in some cases. Since the files will most likely reside in GitHub, and GitHub starts using a scroll bar in all cases when the length is over 164 characters, the lines below should be split when they reach that length

There are 8 instances of this issue:

File: contracts/core/GolomTrader.sol

132:                          'order(address collection,uint256 tokenId,address signer,uint256 orderType,uint256 totalAmt,payment exchange,payment prePayment,bool isERC721,uint256 tokenAmt,uint256 refererrAmt,bytes32 root,address reservedAddress,uint256 nonce,uint256 deadline)payment(uint256 paymentAmt,address paymentAddress)'

329:      ///      to send ether to that address on filling the order, Match an criteria order, ensuring that the supplied proof demonstrates inclusion of the tokenId in the associated merkle root, if root is 0 then any token can be used to fill the order

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

File: contracts/rewards/RewardDistributor.sol

126:                      epochTotalFee[0] =  address(this).balance; // staking and trading rewards start at epoch 1, for epoch 0 all contract ETH balance is converted to staker rewards rewards.

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

File: contracts/vote-escrow/TokenUriHelper.sol

72:           output = '<svg xmlns="http://www.w3.org/2000/svg" width="512" height="468" viewBox="0 0 512 468" fill="none"><g clip-path="url(#clip0_2_190)"><rect width="512" height="468" rx="40" fill="#232323"/><g filter="url(#filter0_f_2_190)"><ellipse cx="256.5" cy="-132" rx="164" ry="381.5" transform="rotate(-90 256.5 -132)" fill="#FF8982"/></g>';

78:                   '</text><text y="318px" x="54px" fill="white" font-family="Lexend Deca, sans-serif" font-weight="400" font-size="24px" fill-opacity="0.64">Locked Till</text>'

86:                   '</text><text y="248px" x="54px" fill="white" font-family="Lexend Deca, sans-serif" font-weight="400" font-size="24px" fill-opacity="0.64">Voting Power</text>'

94:                   '</text><text y="391px" x="54px" fill="white" font-family="Lexend Deca, sans-serif" font-weight="400" font-size="24px" fill-opacity="0.64">Value</text>'

102:                  '</text><mask id="mask0_2_190" style="mask-type:alpha" maskUnits="userSpaceOnUse" x="399" y="73" width="58" height="58"><path d="M456.543 102.091C456.543 117.884 443.74 130.686 427.948 130.686C412.155 130.686 399.352 117.884 399.352 102.091C399.352 86.2981 412.155 73.4955 427.948 73.4955C443.74 73.4955 456.543 86.2981 456.543 102.091ZM405.91 102.091C405.91 114.262 415.777 124.128 427.948 124.128C440.118 124.128 449.985 114.262 449.985 102.091C449.985 89.9201 440.118 80.0537 427.948 80.0537C415.777 80.0537 405.91 89.9201 405.91 102.091Z" fill="#C4C4C4"/></mask><g mask="url(#mask0_2_190)"><path d="M458.138 73.4955H396.962V133.076H458.138V73.4955Z" fill="#FD7A7A"/><path d="M396.962 76.7614H458.138" stroke="black" stroke-width="0.328567"/><path d="M396.962 80.266H458.138" stroke="black" stroke-width="0.328567"/><path d="M396.962 83.7708H458.138" stroke="black" stroke-width="0.328567"/><path d="M396.962 87.2754H458.138" stroke="black" stroke-width="0.328567"/><path d="M396.962 90.7802H458.138" stroke="black" stroke-width="0.328567"/><path d="M396.962 94.2848H458.138" stroke="black" stroke-width="0.328567"/><path d="M396.962 97.7897H458.138" stroke="black" stroke-width="0.328567"/><path d="M396.962 101.294H458.138" stroke="black" stroke-width="0.328567"/><path d="M396.962 104.799H458.138" stroke="black" stroke-width="0.328567"/><path d="M396.962 108.304H458.138" stroke="black" stroke-width="0.328567"/><path d="M396.962 111.808H458.138" stroke="black" stroke-width="0.328567"/><path d="M396.962 115.313H458.138" stroke="black" stroke-width="0.328567"/><path d="M396.962 118.818H458.138" stroke="black" stroke-width="0.328567"/><path d="M396.962 122.323H458.138" stroke="black" stroke-width="0.328567"/><path d="M396.962 125.827H458.138" stroke="black" stroke-width="0.328567"/><path d="M396.962 129.332H458.138" stroke="black" stroke-width="0.328567"/></g><mask id="mask1_2_190" style="mask-type:alpha" maskUnits="userSpaceOnUse" x="399" y="73" width="58" height="58"><path d="M456.543 102.091C456.543 117.884 443.74 130.686 427.948 130.686C412.155 130.686 399.352 117.884 399.352 102.091C399.352 86.2981 412.155 73.4955 427.948 73.4955C443.74 73.4955 456.543 86.2981 456.543 102.091ZM405.91 102.091C405.91 114.262 415.777 124.128 427.948 124.128C440.118 124.128 449.985 114.262 449.985 102.091C449.985 89.9201 440.118 80.0537 427.948 80.0537C415.777 80.0537 405.91 89.9201 405.91 102.091Z" fill="white"/></mask><g mask="url(#mask1_2_190)"><path d="M456.543 102.091C456.543 117.884 443.74 130.686 427.948 130.686C412.155 130.686 399.352 117.884 399.352 102.091C399.352 86.2981 412.155 73.4955 427.948 73.4955C443.74 73.4955 456.543 86.2981 456.543 102.091ZM405.91 102.091C405.91 114.262 415.777 124.128 427.948 124.128C440.118 124.128 449.985 114.262 449.985 102.091C449.985 89.9201 440.118 80.0537 427.948 80.0537C415.777 80.0537 405.91 89.9201 405.91 102.091Z" stroke="black" stroke-width="0.876179"/><path d="M422.132 130.606H428.026" stroke="black" stroke-width="0.109522"/><path d="M422.132 73.5752H428.026" stroke="black" stroke-width="0.109522"/></g><mask id="mask2_2_190" style="mask-type:alpha" maskUnits="userSpaceOnUse" x="393" y="73" width="58" height="58"><path d="M450.648 102.091C450.648 117.884 437.845 130.686 422.053 130.686C406.26 130.686 393.457 117.884 393.457 102.091C393.457 86.2981 406.26 73.4955 422.053 73.4955C437.845 73.4955 450.648 86.2981 450.648 102.091ZM400.015 102.091C400.015 114.262 409.882 124.128 422.053 124.128C434.223 124.128 444.09 114.262 444.09 102.091C444.09 89.9201 434.223 80.0537 422.053 80.0537C409.882 80.0537 400.015 89.9201 400.015 102.091Z" fill="white"/></mask><g mask="url(#mask2_2_190)"><path d="M450.648 102.091C450.648 117.884 437.845 130.686 422.053 130.686C406.26 130.686 393.457 117.884 393.457 102.091C393.457 86.2981 406.26 73.4955 422.053 73.4955C437.845 73.4955 450.648 86.2981 450.648 102.091ZM400.015 102.091C400.015 114.262 409.882 124.128 422.053 124.128C434.223 124.128 444.09 114.262 444.09 102.091C444.09 89.9201 434.223 80.0537 422.053 80.0537C409.882 80.0537 400.015 89.9201 400.015 102.091Z" fill="white" stroke="black" stroke-width="0.876179"/></g></g><defs><filter id="filter0_f_2_190" x="-381" y="-552" width="1275" height="840" filterUnits="userSpaceOnUse" color-interpolation-filters="sRGB"><feFlood flood-opacity="0" result="BackgroundImageFix"/><feBlend mode="normal" in="SourceGraphic" in2="BackgroundImageFix" result="shape"/><feGaussianBlur stdDeviation="128" result="effect1_foregroundBlur_2_190"/></filter><clipPath id="clip0_2_190"><rect width="512" height="468" rx="40" fill="white"/></clipPath></defs></svg>'

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

[N‑19] Inconsistent method of specifying a floating pragma

Some files use >=, some use ^. The instances below are examples of the method that has the fewest instances for a specific version. Note that using >= without also specifying <= will lead to failures to compile, or external project incompatability, when the major version changes and there are breaking-changes, so ^ should be preferred regardless of the instance counts

There is 1 instance of this issue:

File: contracts/governance/GolomToken.sol

2:    pragma solidity ^0.8.11;

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

[N‑20] Variable names that consist of all capital letters should be reserved for constant/immutable variables

If the variable needs to be different based on which class it comes from, a view/pure function should be used instead (e.g. like this).

There are 2 instances of this issue:

File: contracts/core/GolomTrader.sol

45:       ERC20 WETH = ERC20(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2);

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

File: contracts/vote-escrow/VoteEscrowDelegation.sol

50:       uint256 public MIN_VOTING_POWER_REQUIRED = 0;

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

[N‑21] Non-library/interface files should use fixed compiler versions, not floating ones

There is 1 instance of this issue:

File: contracts/governance/GolomToken.sol

2:    pragma solidity ^0.8.11;

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

[N‑22] Typos

There are 24 instances of this issue:

File: contracts/core/GolomTrader.sol

/// @audit succesful
53:           Payment exchange; // payment agreed by maker of the order to pay on succesful filling of trade this amt is subtracted from totalamt

/// @audit facilating
54:           Payment prePayment; // another payment , can be used for royalty, facilating trades

/// @audit usefull
60:           uint256 nonce; // nonce of order usefull for cancelling in bulk

/// @audit succesful
201:      /// @param p any extra payment that the taker of this order wanna send on succesful execution of order

/// @audit succesful
278:      /// @param p any extra payment that the taker of this order wanna send on succesful execution of order

/// @audit succesful
333:      /// @param p any extra payment that the taker of this order wanna send on succesful execution of order

/// @audit succesfully
370:      /// @dev function to settle balances when a bid is filled succesfully

/// @audit succesful
374:      /// @param p any extra payment that the taker of this order wanna send on succesful execution of order

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

File: contracts/rewards/RewardDistributor.sol

/// @audit epoc
61:       mapping(uint256 => uint256) public rewardTrader; // reward minted each epoc for trader

/// @audit epoc
/// @audit exhange
62:       mapping(uint256 => uint256) public rewardExchange; // reward minted each epoc for exhange

/// @audit epoc
63:       mapping(uint256 => uint256) public rewardLP; // reward minted each epoc for LP

/// @audit epoc
64:       mapping(uint256 => uint256) public rewardStaker; // reward minted each epoc for stakers

/// @audit upto
66:       mapping(uint256 => uint256) public claimedUpto; // epoch upto which tokenid has claimed

/// @audit upto
67:       mapping(uint256 => mapping(uint256 => uint256)) public claimed; // epoch upto which tokenid has claimed

/// @audit facilated
95:       /// @dev Add fees contributed by the Seller of nft and the exchange/frontend that facilated the trade

/// @audit atleast
107:              // this assumes atleast 1 trade is done daily??????

/// @audit begiining
/// @audit begining
111:              // emissions is decided by epoch begiining locked/circulating , and amount each nft gets also decided at epoch begining

/// @audit facilated
154:      // allows exchange that facilated the nft trades to claim there previous epoch rewards

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

File: contracts/vote-escrow/VoteEscrowCore.sol

/// @audit blocktimes
267:   * and per block could be fairly bad b/c Ethereum changes blocktimes.

/// @audit Exeute
526:      /// @dev Exeute transfer of a NFT.

/// @audit Pevious
688:      /// @param old_locked Pevious locked amount / end lock time for the user

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

File: contracts/vote-escrow/VoteEscrowDelegation.sol

/// @audit Exeute
227:      /// @dev Exeute transfer of a NFT.

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

[N‑23] File does not contain an SPDX Identifier

There is 1 instance of this issue:

File: contracts/vote-escrow/TokenUriHelper.sol

0:    /// [MIT License]

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

[N‑24] NatSpec is incomplete

There are 19 instances of this issue:

File: contracts/core/GolomTrader.sol

/// @audit Missing: '@return'
162       ///      OrderStatus = 3 , valid order
163       /// @param o the Order struct to be validated
164       function validateOrder(Order calldata o)
165           public
166           view
167           returns (
168               uint256,
169               bytes32,
170:              uint256

/// @audit Missing: '@param tokenId'
/// @audit Missing: '@param proof'
328       /// @dev function to fill a signed order of ordertype 2 also has a payment param in case the taker wants
329       ///      to send ether to that address on filling the order, Match an criteria order, ensuring that the supplied proof demonstrates inclusion of the tokenId in the associated merkle root, if root is 0 then any token can be used to fill the order
330       /// @param o the Order struct to be filled must be orderType 2
331       /// @param amount the amount of times the order is to be filled(useful for ERC1155)
332       /// @param referrer referrer of the order
333       /// @param p any extra payment that the taker of this order wanna send on succesful execution of order
334       function fillCriteriaBid(
335           Order calldata o,
336           uint256 amount,
337           uint256 tokenId,
338           bytes32[] calldata proof,
339           address referrer,
340           Payment calldata p
341:      ) public nonReentrant {

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

File: contracts/rewards/RewardDistributor.sol

/// @audit Missing: '@return'
213       /// @dev returns unclaimed rewards of an NFT, returns (unclaimed golom rewards, unclaimed eth rewards, unclaimed epochs)
214       /// @param tokenid the nft id to claim rewards for all ids in the list must belong to 1 address
215       function stakerRewards(uint256 tokenid) public view returns (
216               uint256,
217               uint256,
218:              uint256[] memory

/// @audit Missing: '@return'
252       /// @dev returns unclaimed rewards of an NFT, returns (unclaimed golom rewards, unclaimed eth rewards, unclaimed epochs)
253       /// @param addr the nft id to claim rewards for all ids in the list must belong to 1 address
254       function traderRewards(address addr) public view returns (
255:              uint256        

/// @audit Missing: '@return'
267       /// @dev returns unclaimed golom rewards of a trader
268       /// @param addr the nft id to claim rewards for all ids in the list must belong to 1 address
269       function exchangeRewards(address addr) public view returns (
270:              uint256

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

File: contracts/vote-escrow/VoteEscrowCore.sol

/// @audit Missing: '@return'
366       /// @dev Interface identification is specified in ERC-165.
367       /// @param _interfaceID Id of the interface
368:      function supportsInterface(bytes4 _interfaceID) external view returns (bool) {

/// @audit Missing: '@return'
396       ///      Throws if `_owner` is the zero address. NFTs assigned to the zero address are considered invalid.
397       /// @param _owner Address for whom to query the balance.
398:      function _balance(address _owner) internal view returns (uint256) {

/// @audit Missing: '@return'
403       ///      Throws if `_owner` is the zero address. NFTs assigned to the zero address are considered invalid.
404       /// @param _owner Address for whom to query the balance.
405:      function balanceOf(address _owner) external view returns (uint256) {

/// @audit Missing: '@return'
409       /// @dev Returns the address of the owner of the NFT.
410       /// @param _tokenId The identifier for an NFT.
411:      function ownerOf(uint256 _tokenId) public view returns (address) {

/// @audit Missing: '@return'
415       /// @dev Get the approved address for a single NFT.
416       /// @param _tokenId ID of the NFT to query the approval of.
417:      function getApproved(uint256 _tokenId) external view returns (address) {

/// @audit Missing: '@return'
422       /// @param _owner The address that owns the NFTs.
423       /// @param _operator The address that acts on behalf of the owner.
424:      function isApprovedForAll(address _owner, address _operator) external view returns (bool) {

/// @audit Missing: '@return'
935       /// @param _lock_duration Number of seconds to lock tokens for (rounded down to nearest week)
936       /// @param _to Address to deposit
937       function _create_lock(
938           uint256 _value,
939           uint256 _lock_duration,
940           address _to
941:      ) internal returns (uint256) {

/// @audit Missing: '@return'
957       /// @param _lock_duration Number of seconds to lock tokens for (rounded down to nearest week)
958       /// @param _to Address to deposit
959       function create_lock_for(
960           uint256 _value,
961           uint256 _lock_duration,
962           address _to
963:      ) external nonreentrant returns (uint256) {

/// @audit Missing: '@return'
968       /// @param _value Amount to deposit
969       /// @param _lock_duration Number of seconds to lock tokens for (rounded down to nearest week)
970:      function create_lock(uint256 _value, uint256 _lock_duration) external nonreentrant returns (uint256) {

/// @audit Missing: '@param _tokenId'
974       /// @notice Deposit `_value` additional tokens for `_tokenId` without modifying the unlock time
975       /// @param _value Amount of tokens to deposit and add to the lock
976:      function increase_amount(uint256 _tokenId, uint256 _value) external nonreentrant {

/// @audit Missing: '@param _tokenId'
988       /// @notice Extend the unlock time for `_tokenId`
989       /// @param _lock_duration New number of seconds until tokens unlock
990:      function increase_unlock_time(uint256 _tokenId, uint256 _lock_duration) external nonreentrant {

/// @audit Missing: '@return'
1079      /// @dev Returns current token URI metadata
1080      /// @param _tokenId Token ID to fetch URI for.
1081:     function tokenURI(uint256 _tokenId) external view returns (string memory) {

/// @audit Missing: '@param t'
1189      /// @notice Calculate total voting power
1190      /// @dev Adheres to the ERC20 `totalSupply` interface for Aragon compatibility
1191      /// @return Total voting power
1192:     function totalSupplyAtT(uint256 t) public view returns (uint256) {

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

[N‑25] Event is missing indexed fields

Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (threefields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.

There are 7 instances of this issue:

File: contracts/core/GolomTrader.sol

79:       event NonceIncremented(address indexed maker, uint256 newNonce);

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

File: contracts/rewards/RewardDistributor.sol

70:       event NewEpoch(uint256 indexed epochNo, uint256 tokenMinted, uint256 rewardStaker, uint256 previousEpochFee);

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

File: contracts/vote-escrow/VoteEscrowCore.sol

67:       event ApprovalForAll(address indexed owner, address indexed operator, bool approved);

284       event Deposit(
285           address indexed provider,
286           uint256 tokenId,
287           uint256 value,
288           uint256 indexed locktime,
289           DepositType deposit_type,
290           uint256 ts
291:      );

292:      event Withdraw(address indexed provider, uint256 tokenId, uint256 value, uint256 ts);

293:      event Supply(uint256 prevSupply, uint256 supply);

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

File: contracts/vote-escrow/VoteEscrowDelegation.sol

29:       event DelegateVotesChanged(address indexed delegate, uint256 previousBalance, uint256 newBalance);

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

[N‑26] Duplicated require()/revert() checks should be refactored to a modifier or function

The compiler will inline the function, which will avoid JUMP instructions usually associated with functions

There are 14 instances of this issue:

File: contracts/core/GolomTrader.sol

291:              require(msg.sender == o.reservedAddress);

299:              require(amount == 1, 'only 1 erc721 at 1 time');

349:          require(status == 3);

350:          require(amountRemaining >= amount);

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

File: contracts/rewards/RewardDistributor.sol

158:              require(epochs[index] < epoch);

220:          require(address(ve) != address(0), ' VE not added yet');

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

File: contracts/vote-escrow/VoteEscrowCore.sol

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

874:          require(msg.sender == voter);

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

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

983:          require(_locked.end > block.timestamp, 'Cannot add to expired lock. Withdraw');

999:          require(unlock_time <= block.timestamp + MAXTIME, 'Voting lock can be 4 years max');

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

File: contracts/vote-escrow/VoteEscrowDelegation.sol

211:          require(ownerOf(tokenId) == msg.sender, 'VEDelegation: Not allowed');

186:          require(blockNumber < block.number, 'VEDelegation: not yet determined');

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