search

code-423n4 / 2022-07-golom-findings

2 stars 1 forks source link

QA Report #796

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Summary

Low Risk Issues

Issue Instances
[L‑01] Use of transferFrom() rather than safeTransferFrom() for NFTs in will lead to the loss of NFTs 1
[L‑02] Don't use payable.transfer()/payable.send() 1
[L‑03] Unused/empty receive()/fallback() function 4
[L‑04] require() should be used instead of assert() 13
[L‑05] Self-delegation is not automatic 1
[L‑06] Function may run out of gas 1
[L‑07] Vulnerable to cross-chain replay attacks due to static DOMAIN_SEPARATOR/domainSeparator 1
[L‑08] Wrong comment 1
[L‑09] Missing checks for address(0x0) when assigning values to address state variables 7
[L‑10] Only a billion checkpoints available 1

Total: 31 instances over 10 issues

Non-critical Issues

Issue Instances
[N‑01] Consider addings checks for signature malleability 1
[N‑02] ecrecover() signature validity not checked 1
[N‑03] Boilerplate not replaced 2
[N‑04] Remove commented out code 1
[N‑05] Invalid/extraneous/optional function definitions in interface 4
[N‑06] Remove include for hardhat's console 1
[N‑07] Contract implements interface without extending the interface 1
[N‑08] require()/revert() statements should have descriptive reason strings 31
[N‑09] public functions not called by the contract should be declared external instead 13
[N‑10] Non-assembly method available 2
[N‑11] constants should be defined rather than using magic numbers 49
[N‑12] Numeric values having to do with time should use time units for readability 5
[N‑13] Large multiples of ten should use scientific notation (e.g. 1e6) rather than decimal literals (e.g. 1000000), for readability 2
[N‑14] Missing event and or timelock for critical parameter change 3
[N‑15] Use a more recent version of solidity 2
[N‑16] Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18) 2
[N‑17] Inconsistent spacing in comments 2
[N‑18] Lines are too long 8
[N‑19] Inconsistent method of specifying a floating pragma 1
[N‑20] Variable names that consist of all capital letters should be reserved for constant/immutable variables 2
[N‑21] Non-library/interface files should use fixed compiler versions, not floating ones 1
[N‑22] Typos 24
[N‑23] File does not contain an SPDX Identifier 1
[N‑24] NatSpec is incomplete 19
[N‑25] Event is missing indexed fields 7
[N‑26] Duplicated require()/revert() checks should be refactored to a modifier or function 14

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:

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

dmvt commented 1 year ago

L-01 - upgraded N-14, N-17, N-22, N-24 - should be low risk