Golom contest - Bnke0x0's results

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

[L-05] address.call{value:x}() should be used instead of payable.transfer():-

1. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 154):

payable(payAddress).transfer(payAmt);

Lines of code

https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L236 https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L300#L301 https://github.com/code-423n4/2022-07-golom/blob/7bbb55fca61e6bae29e57133c1e45806cbb17aa4/contracts/core/GolomTrader.sol#L360#L361

Vulnerability details

Details & Impact

The transferFrom() method is used instead of safeTransferFrom(), presumably to save gas. I however argue that this isn’t recommended because:

1. OpenZeppelin’s documentation discourages the use of transferFrom(), use safeTransferFrom() whenever possible..

2. Given that any NFT can be used for the call option, there are a few NFTs (here’s an example) that have logic in the onERC721Received() function, which is only triggered in the safeTransferFrom() function and not in transferFrom().

Proof of Concept

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

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

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

   ERC721 nftcontract = ERC721(o.collection); nftcontract.transferFrom(msg.sender, o.signer, o.tokenId);

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

   ERC721 nftcontract = ERC721(o.collection); nftcontract.transferFrom(msg.sender, o.signer, tokenId);

Recommended Mitigation Steps

Call the safeTransferFrom() method instead of transferFrom() for NFT transfers.

Low Risk Issues

[L-01] Usage of require() instead of assert():-

1. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 483):

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

2. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 506):

   assert(idToOwner[_tokenId] == _from);

3. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 519):

   assert(idToOwner[_tokenId] == _owner);

4. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 666):

   assert(_operator != msg.sender);

5. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 679):

   assert(_to != address(0));

6. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 861):

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

7. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 977):

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

8. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 981):

   assert(_value > 0);

9. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 991):

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

10. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 1007):

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

11. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 1023):

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

12. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 1110):

    assert(_block <= block.number);

13. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 1206):

    assert(_block <= block.number);

[L-02] Unused receive() function will lock Ether in contract (If the intention is for the Ether to be used, the function should call another function, otherwise it should revert):-

1. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 315):

   receive() external payable {}

2. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 461):

   receive() external payable {}

[L-03] Missing checks for address(0x0) when assigning values to address state variables:-

1. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 870):

   voter = _voter;

2. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 53):

   token = _token;

3. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 81):

   trader = _trader;

[L-04] abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256():-

1. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 175):

   bytes32 hash = keccak256(abi.encodePacked('\x19\x01', EIP712_DOMAIN_TYPEHASH, hashStruct));

[L-05] address.call{value:x}() should be used instead of payable.transfer():-

1. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 154):

   payable(payAddress).transfer(payAmt);

[L-06] approve should be replaced with safeApprove or safeIncreaseAllowance() / safeDecreaseAllowance():-

1. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 136):

   function approve(address to, uint256 tokenId) external;

2. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 643):

   function approve(address _approved, uint256 _tokenId) public {

3. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 1232):

   approve(address(0), _tokenId);

Non-Critical

[N-01] Adding a return statement when the function defines a named return variable, is redundant:-

1. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 134):

   return myArray;

2. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 174):

   return votes;

3. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 192):

   return votes;

4. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 952):

   return _tokenId;

5. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 1056):

   return _min;

[N-02] require()/revert() statements should have descriptive reason strings:-

1. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 145):

   require(_isApprovedOrOwner(_sender, _tokenId));

2. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 88):

   require(msg.sender == trader);

3. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 144):

   require(epochs[index] < epoch);

4. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 158):

   require(epochs[index] < epoch);

5. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 220):

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

6. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 285-288):

   require( o.totalAmt * amount > (o.exchange.paymentAmt + o.prePayment.paymentAmt + o.refererrAmt) * amount + p.paymentAmt );

7. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 291):

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

8. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 293):

   require(o.orderType == 1);

9. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 295):

   require(status == 3);

10. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 296):

    require(amountRemaining >= amount);

11. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 313):

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

12. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 342):

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

13. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 345):

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

14. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 347):

    require(o.orderType == 2);

15. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 349):

    require(status == 3);

16. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 350):

    require(amountRemaining >= amount);

17. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 360):

    require(_entered_state == _not_entered);

18. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 540):

    require(_isApprovedOrOwner(_sender, _tokenId));

19. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 646):

    require(owner != address(0));

20. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 648):

    require(_approved != owner);

21. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 652):

    require(senderIsOwner || senderIsApprovedForAll);

22. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 869):

    require(msg.sender == voter);

23. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 874):

    require(msg.sender == voter);

24. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 879):

    require(msg.sender == voter);

25. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 884):

    require(msg.sender == voter);

26. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 889):

    require(msg.sender == voter);

27. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 895-897):

    require(_from != _to); require(_isApprovedOrOwner(msg.sender, _from)); require(_isApprovedOrOwner(msg.sender, _to));

28. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 927):

    require(_value > 0);

29. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 944):

    require(_value > 0);

30. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 611):

    revert(add(32, reason), mload(reason))

[N-03] 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>) ):-

1. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 3):

   pragma solidity 0.8.11;

2. File: 2022-07-golom/contracts/vote-escrow/TokenUriHelper.sol (line 3):

   pragma solidity 0.8.11;

[N-04] Event is missing indexed fields (Each event should use three indexed fields if there are three or more fields):-

1. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 29):

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

2. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 70):

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

3. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 284-291):

   event Deposit( address indexed provider, uint256 tokenId, uint256 value, uint256 indexed locktime, DepositType deposit_type, uint256 ts );

4. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 292):

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

5. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 293):

   event Supply(uint256 prevSupply, uint256 supply);

[N-05] public functions not called by the contract should be declared external instead:-

1. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 98):

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

2. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 141):

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

3. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 155):

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

4. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 172):

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

5. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 215):

   function stakerRewards(uint256 tokenid) public view returns (

[N-06] Interfaces should be moved to separate files:-

1. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 12):

   interface IVoteEscrow {

2. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 11):

   interface ERC20 {

3. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 31):

   interface VE {

[N-07] Interfaces should be moved to separate files:-

1. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 12):

   interface IVoteEscrow {

2. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 11):

   interface ERC20 {

3. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 31):

   interface VE {

4. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 172):

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

5. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 215):

   function stakerRewards(uint256 tokenid) public view returns (

[N-08] constants should be defined rather than using magic numbers:-

1. File: 2022-07-golom/contracts/governance/GolomToken.sol (line 44):

   _mint(_airdrop, 150_000_000 * 1e18);

2. File: 2022-07-golom/contracts/governance/GolomToken.sol (line 52):

   _mint(_rewardDistributor, 62_500_000 * 1e18);

3. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 48):

   uint256 constant dailyEmission = 600000 * 10**18;

4. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 57):

   uint256 constant secsInDay = 24 * 60 * 60;

5. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 100):

   function stakerRewards(uint256 tokenid) public view returns (

6. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 296-297):

   uint256 internal constant MAXTIME = 4 * 365 * 86400; int128 internal constant iMAXTIME = 4 * 365 * 86400;

[N-09] Unused file:-

1. File: 2022-07-golom/contracts/governance/GolomToken.sol (line 1):

   // SPDX-License-Identifier: MIT

2. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 1):

   // SPDX-License-Identifier: MIT

3. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 1):

   // SPDX-License-Identifier: MIT

4. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 1):

   // SPDX-License-Identifier: MIT

5. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 1):

   // SPDX-License-Identifier: MIT

6. File: 2022-07-golom/contracts/vote-escrow/TokenUriHelper.sol (line 1):

   /// [MIT License]

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

1. File: 2022-07-golom/contracts/governance/GolomToken.sol (line 2):

   pragma solidity ^0.8.11;

2. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 2):

   pragma solidity 0.8.11;

3. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 2):

   fpragma solidity 0.8.11;

4. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 3):

   pragma solidity 0.8.11;

5. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 2):

   pragma solidity 0.8.11;

6. File: 2022-07-golom/contracts/vote-escrow/TokenUriHelper.sol (line 3):

   pragma solidity 0.8.11;

[N-11] The nonReentrant modifier should occur before all other modifiers:-

1. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 209):

   ) public payable nonReentrant {

2. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 248):

   ) public nonReentrant {

3. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 312):

   function cancelOrder(Order calldata o) public nonReentrant {

4. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 323):

   function incrementNonce() external nonReentrant {

5. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 341):

   ) public nonReentrant {

6. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 990):

   function increase_unlock_time(uint256 _tokenId, uint256 _lock_duration) external nonreentrant {

7. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 1006):

   function withdraw(uint256 _tokenId) external nonreentrant {

8. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 359):

   modifier nonreentrant() {

9. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 924):

   function deposit_for(uint256 _tokenId, uint256 _value) external nonreentrant {

10. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 963):

    ) external nonreentrant returns (uint256) {

11. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 970):

    function create_lock(uint256 _value, uint256 _lock_duration) external nonreentrant returns (uint256) {

12. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 976):

    function increase_amount(uint256 _tokenId, uint256 _value) external nonreentrant {

[N-12] Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate:-

1. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 58-59):

   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

2. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 331-341):

   `/// @dev Mapping from owner address to count of his tokens. mapping(address => uint256) internal ownerToNFTokenCount;

   /// @dev Mapping from owner address to mapping of index to tokenIds mapping(address => mapping(uint256 => uint256)) internal ownerToNFTokenIdList;

   /// @dev Mapping from NFT ID to index of owner mapping(uint256 => uint256) internal tokenToOwnerIndex;

   /// @dev Mapping from owner address to mapping of operator addresses. mapping(address => mapping(address => bool)) internal ownerToOperators;`

[G-01] STATE VARIABLES ONLY SET IN THE CONSTRUCTOR SHOULD BE DECLARED IMMUTABLE:-

1. File: 2022-07-golom/contracts/governance/GolomToken.sol (line 15-18):

   ` address public minter;

uint256 public minterEnableDate;

address public pendingMinter;`         

2. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 50):

  `uint256 public MIN_VOTING_POWER_REQUIRED = 0;`

[G-02] <array>.length should not be looked up in every loop of a for-loop:-

1. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 171):

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

2. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 189):

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

3. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 199):

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

4. 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 143):

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

5. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 157):

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

6. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 180):

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

7. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 183):

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

8. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 415):

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

[G-03] Use prefix not postfix in loops (Using a prefix increment (++i) instead of a postfix increment (i++) saves gas for each loop cycle and so can have a big gas impact when the loop executes on a large number of elements.):-

1. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 171):

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

2. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 189):

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

3. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 199):

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

4. 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 143):

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

5. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 157):

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

6. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 180):

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

7. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 183):

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

8. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 415):

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

9. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 226):

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

10. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 258):

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

11. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 273):

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

[G-04] require()/revert() strings longer than 32 bytes cost extra gas:-

1. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 608):

   revert('ERC721: transfer to non ERC721Receiver implementer');

2. File: 2022-07-golom/contracts/governance/GolomToken.sol (line 24):

   require(msg.sender == minter, 'GolomToken: only reward distributor can enable');

3. File: 2022-07-golom/contracts/governance/GolomToken.sol (line 69):

   require(minterEnableDate <= block.timestamp, 'GolomToken: wait for timelock');

4. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 73):

   require(this.balanceOfNFT(tokenId) >= MIN_VOTING_POWER_REQUIRED, 'VEDelegation: Need more voting power');

5. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 99):

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

6. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 130):

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

7. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 186):

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

8. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 181):

   require(tokenowner == ve.ownerOf(tokenids[tindex]), 'Can only claim for a single Address together');

9. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 184-185):

   require(epochs[index] < epoch, 'cant claim for future epochs'); require(claimed[tokenids[tindex]][epochs[index]] == 0, 'cant claim if already claimed');

10. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 292):

    require(traderEnableDate <= block.timestamp, 'RewardDistributor: time not over yet');

11. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 309):

    require(voteEscrowEnableDate <= block.timestamp, 'RewardDistributor: time not over yet');

12. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 929):

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

13. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 945-946):

    require(unlock_time > block.timestamp, 'Can only lock until time in the future'); require(unlock_time <= block.timestamp + MAXTIME, 'Voting lock can be 4 years max');

14. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 983):

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

15. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 1227):

    require(_isApprovedOrOwner(msg.sender, _tokenId), 'caller is not owner nor approved');

[G-05] Using > 0 costs more gas than != 0 when used on a uint in a require() statement:-

1. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 927):

   require(_value > 0);

2. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 928):

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

3. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 944):

   require(_value > 0);

4. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 981):

   assert(_value > 0);

5. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 982):

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

6. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 997):

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

[G-06] It costs more gas to initialize variables to zero than to let the default of zero be applied:-

1. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 171):

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

2. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 189):

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

3. 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 143):

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

4. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 157):

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

5. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 180):

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

6. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 183):

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

7. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 415):

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

8. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 226):

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

9. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 258):

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

10. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 273):

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

[G-07] Splitting require() statements that use && saves gas:-

1. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 239):

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

2. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 538):

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

3. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 894):

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

4. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 1008):

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

[G-08] Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead:-

1. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 62):

   uint8 v;

2. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 320):

   uint8 public constant decimals = 18;

3. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 356-358):

   uint8 internal constant _not_entered = 1; uint8 internal constant _entered = 2; uint8 internal _entered_state = 1;

[G-09] Duplicated require()/revert() checks should be refactored to a modifier or function:-

1. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 130):

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

2. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 72):

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

3. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 144):

   require(epochs[index] < epoch);

4. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 173):

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

5. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 869):

   require(msg.sender == voter);

6. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 927):

   require(_value > 0);

[G-10] require() or revert() statements that check input arguments should be at the top of the function:-

1. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 608):

   revert('ERC721: transfer to non ERC721Receiver implementer');

2. File: 2022-07-golom/contracts/governance/GolomToken.sol (line 24):

   require(msg.sender == minter, 'GolomToken: only reward distributor can enable');

3. File: 2022-07-golom/contracts/governance/GolomToken.sol (line 69):

   require(minterEnableDate <= block.timestamp, 'GolomToken: wait for timelock');

4. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 73):

   require(this.balanceOfNFT(tokenId) >= MIN_VOTING_POWER_REQUIRED, 'VEDelegation: Need more voting power');

5. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 99):

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

6. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 130):

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

7. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 186):

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

8. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 181):

   require(tokenowner == ve.ownerOf(tokenids[tindex]), 'Can only claim for a single Address together');

9. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 184-185):

   require(epochs[index] < epoch, 'cant claim for future epochs'); require(claimed[tokenids[tindex]][epochs[index]] == 0, 'cant claim if already claimed');

10. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 292):

    require(traderEnableDate <= block.timestamp, 'RewardDistributor: time not over yet');

11. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 309):

    require(voteEscrowEnableDate <= block.timestamp, 'RewardDistributor: time not over yet');

12. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 929):

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

13. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 945-946):

    require(unlock_time > block.timestamp, 'Can only lock until time in the future'); require(unlock_time <= block.timestamp + MAXTIME, 'Voting lock can be 4 years max');

14. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 983):

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

15. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 1227):

    require(_isApprovedOrOwner(msg.sender, _tokenId), 'caller is not owner nor approved');

[G-11] Use custom errors rather than revert()/require() strings to save deployment gas:-

1. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 608):

   revert('ERC721: transfer to non ERC721Receiver implementer');

2. File: 2022-07-golom/contracts/governance/GolomToken.sol (line 24):

   require(msg.sender == minter, 'GolomToken: only reward distributor can enable');

3. File: 2022-07-golom/contracts/governance/GolomToken.sol (line 69):

   require(minterEnableDate <= block.timestamp, 'GolomToken: wait for timelock');

4. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 73):

   require(this.balanceOfNFT(tokenId) >= MIN_VOTING_POWER_REQUIRED, 'VEDelegation: Need more voting power');

5. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 99):

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

6. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 130):

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

7. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 186):

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

8. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 181):

   require(tokenowner == ve.ownerOf(tokenids[tindex]), 'Can only claim for a single Address together');

9. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 184-185):

   require(epochs[index] < epoch, 'cant claim for future epochs'); require(claimed[tokenids[tindex]][epochs[index]] == 0, 'cant claim if already claimed');

10. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 292):

    require(traderEnableDate <= block.timestamp, 'RewardDistributor: time not over yet');

11. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 309):

    require(voteEscrowEnableDate <= block.timestamp, 'RewardDistributor: time not over yet');

12. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 929):

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

13. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 945-946):

    require(unlock_time > block.timestamp, 'Can only lock until time in the future'); require(unlock_time <= block.timestamp + MAXTIME, 'Voting lock can be 4 years max');

14. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 983):

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

15. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 1227):

    require(_isApprovedOrOwner(msg.sender, _tokenId), 'caller is not owner nor approved');

[G-12] Functions guaranteed to revert when called by normal users can be marked payable (If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.):-

1. File: 2022-07-golom/contracts/governance/GolomToken.sol (line 36):

   function mint(address _account, uint256 _amount) external onlyMinter {

2. File: 2022-07-golom/contracts/governance/GolomToken.sol (line 42):

   function mintAirdrop(address _airdrop) external onlyOwner {

3. File: 2022-07-golom/contracts/governance/GolomToken.sol (line 50):

   function mintGenesisReward(address _rewardDistributor) external onlyOwner {

4. File: 2022-07-golom/contracts/governance/GolomToken.sol (line 58):

   function setMinter(address _minter) external onlyOwner {;

5. File: 2022-07-golom/contracts/governance/GolomToken.sol (line 65):

   function executeSetMinter() external onlyOwner {

6. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 260):

   function changeMinVotingPower(uint256 _newMinVotingPower) external onlyOwner {

7. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 98):

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

8. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 285):

   function changeTrader(address _trader) external onlyOwner {

9. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 291):

   function executeChangeTrader() external onlyOwner {

10. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 298):

    function addVoteEscrow(address _voteEscrow) external onlyOwner {

11. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 308):

    function executeAddVoteEscrow() external onlyOwner {

12. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 444):

    function setDistributor(address _distributor) external onlyOwner {

13. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 454):

    function executeSetDistributor() external onlyOwner {

[G-13] se a more recent version of solidity (Use a solidity version of at least 0.8.15 to have external calls skip contract existence checks if the external call has a return value):-

1. File: 2022-07-golom/contracts/governance/GolomToken.sol (line 2):

   pragma solidity ^0.8.11;

2. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol (line 2):

   pragma solidity 0.8.11;

3. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 2):

   fpragma solidity 0.8.11;

4. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 3):

   pragma solidity 0.8.11;

5. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 2):

   pragma solidity 0.8.11;

6. File: 2022-07-golom/contracts/vote-escrow/TokenUriHelper.sol (line 3):

   pragma solidity 0.8.11;

[G-14] Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate:-

1. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 58-59):

   mapping(address => mapping(uint256 => uint256)) public feesTrader; mapping(address => mapping(uint256 => uint256)) public feesExchange;

2. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 332-341):

   ` mapping(address => uint256) internal ownerToNFTokenCount;

/// @dev Mapping from owner address to mapping of index to tokenIds

mapping(address => mapping(uint256 => uint256)) internal ownerToNFTokenIdList;

/// @dev Mapping from NFT ID to index of owner
mapping(uint256 => uint256) internal tokenToOwnerIndex;

/// @dev Mapping from owner address to mapping of operator addresses.
mapping(address => mapping(address => bool)) internal ownerToOperators;

`

[G-15] <x> += <y> costs more gas than <x> = <x> + <y> for state variables:-

1. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 499):

   ownerToNFTokenCount[_to] += 1;

2. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 748):

   t_i += WEEK;

3. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 756):

   last_point.slope += d_slope;

4. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 768):

   _epoch += 1;

5. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 784-785):

   last_point.slope += (u_new.slope - u_old.slope); last_point.bias += (u_new.bias - u_old.bias);

6. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 803):

   old_dslope += u_old.slope;

7. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 847):

   _locked.amount += int128(int256(_value));

8. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 1168):

   t_i += WEEK;

9. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 1179):

   last_point.slope += d_slope;

10. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 805):

    old_dslope -= u_new.slope;

11. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 812):

    new_dslope -= u_new.slope;

[G-16] Empty blocks should be removed or emit something:-

1. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 313):

   fallback() external payable {}

2. File: 2022-07-golom/contracts/rewards/RewardDistributor.sol (line 315):

   receive() external payable {}

3. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 459):

   fallback() external payable {}

4. File: 2022-07-golom/contracts/core/GolomTrader.sol (line 461):

   receive() external payable {}

[G-17] Using private rather than public for constants, saves gas:-

1. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 317-320):

   string public constant name = 'veNFT';

2. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 318):

   string public constant symbol = 'veNFT';

3. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 319):

   string public constant version = '1.0.0';

4. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 320):

   uint8 public constant decimals = 18;

[G-18] Using bools for storage incurs overhead:-

1. File: 2022-07-golom/contracts/governance/GolomToken.sol (line 20):

   bool public isAirdropMinted;

2. File: 2022-07-golom/contracts/governance/GolomToken.sol (line 21):

   bool public isGenesisRewardMinted;

3. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 314):

   mapping(uint256 => bool) public voted;

4. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 341):

   mapping(address => mapping(address => bool)) internal ownerToOperators;

[G-19] Usage of require() instead of assert():-

1. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 483):

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

2. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 506):

   assert(idToOwner[_tokenId] == _from);

3. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 519):

   assert(idToOwner[_tokenId] == _owner);

4. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 666):

   assert(_operator != msg.sender);

5. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 679):

   assert(_to != address(0));

6. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 861):

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

7. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 977):

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

8. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 981):

   assert(_value > 0);

9. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 991):

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

10. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 1007):

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

11. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 1023):

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

12. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 1110):

    assert(_block <= block.number);

13. File: 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol (line 1206):

    assert(_block <= block.number);