Golom contest - Dravee's results

Lines of code

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

Vulnerability details

Impact

The state isn't updated

Proof of Concept

See @audit:

File: VoteEscrowDelegation.sol
094:     function _writeCheckpoint(
095:         uint256 toTokenId,
096:         uint256 nCheckpoints,
097:         uint256[] memory _delegatedTokenIds
098:     ) internal {
099:         require(_delegatedTokenIds.length < 500, 'VVDelegation: Cannot stake more');
100: 
101:         Checkpoint memory oldCheckpoint = checkpoints[toTokenId][nCheckpoints - 1]; //@audit using memory instead of storage
102: 
103:         if (nCheckpoints > 0 && oldCheckpoint.fromBlock == block.number) {
104:             oldCheckpoint.delegatedTokenIds = _delegatedTokenIds; //@audit writing to memory (which basically isn't doing anything here). To update this value: "storage" should be used
105:         } else {
106:             checkpoints[toTokenId][nCheckpoints] = Checkpoint(block.number, _delegatedTokenIds);
107:             numCheckpoints[toTokenId] = nCheckpoints + 1;
108:         }
109:     }

Recommended Mitigation Steps

Use the storage keyword L101

Lines of code

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

Vulnerability details

This is a classic Code4rena issue:

• https://github.com/code-423n4/2021-04-meebits-findings/issues/2
• https://github.com/code-423n4/2021-10-tally-findings/issues/20
• https://github.com/code-423n4/2022-01-openleverage-findings/issues/75

Impact

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

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

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

Impacted lines:

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

Recommended Mitigation

Use call() instead of transfer(), but be sure to respect the CEI pattern and/or add re-entrancy guards, as several hacks already happened in the past due to this recommendation not being fully understood.

Relevant links: https://twitter.com/hacxyk/status/1520715516490379264?s=21&t=fnhDkcC3KpE_kJE8eLiE2A https://twitter.com/hacxyk/status/1520715536325218304?s=21&t=fnhDkcC3KpE_kJE8eLiE2A https://twitter.com/hacxyk/status/1520370441705037824?s=21&t=fnhDkcC3KpE_kJE8eLiE2A https://twitter.com/Hacxyk/status/1521949933380595712

Lines of code

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

Vulnerability details

The transferFrom keyword is used instead of safeTransferFrom. If the arbitrary address is a contract and is not aware of the incoming ERC721 token, the sent token could be locked.

Consider transitioning from transferFrom to safeTransferFrom here:

core/GolomTrader.sol:236:            ERC721(o.collection).transferFrom(o.signer, receiver, o.tokenId);
core/GolomTrader.sol:301:            nftcontract.transferFrom(msg.sender, o.signer, o.tokenId);
core/GolomTrader.sol:361:            nftcontract.transferFrom(msg.sender, o.signer, tokenId);

Overview

Table of Contents

• 1. Low Risk Issues
  • 1.1. Known vulnerabilities exist in currently used @openzeppelin/contracts version
  • 1.2. Wrong amount emitted
  • 1.3. Unused/empty receive() function
  • 1.4. Use a 2-step ownership transfer pattern
  • 1.5. A magic number should be documented and explained. Use a constant instead
  • 1.6. require() should be used for checking error conditions on inputs and return values while assert() should be used for invariant checking
• 2. Non-Critical Issues
  • 2.1. It's better to emit after all processing is done
  • 2.2. Typos
  • 2.3. Missing friendly revert strings
  • 2.4. 1000000000 should be changed to 1e9 for readability reasons
  • 2.5. Use a more recent version of solidity
  • 2.6. Use string.concat() or bytes.concat()
  • 2.7. Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18)
  • 2.8. Non-library/interface files should use fixed compiler versions, not floating ones

1. Low Risk Issues

1.1. Known vulnerabilities exist in currently used @openzeppelin/contracts version

As some known vulnerabilities exist in the current @openzeppelin/contracts version, consider updating package.json with at least @openzeppelin/contracts@4.7.1 here:

• https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/package.json#L24

File: package.json
24:     "@openzeppelin/contracts": "^4.5.0",

While vulnerabilities are known (one touching ERC165Checker), the current scope isn't affected (this might not hold true for the whole solution as the known vulns do seem relevant)

1.2. Wrong amount emitted

As arbitrary ERC20 tokens can be used, the amount here should be calculated every time to take into consideration a possible fee-on-transfer or deflation. Also, it's a good practice for the future of the solution.

Affected code:

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

File: VoteEscrowCore.sol
860:         if (_value != 0 && deposit_type != DepositType.MERGE_TYPE) {
861:             assert(IERC20(token).transferFrom(from, address(this), _value)); //@audit FoT would introduce an erroneous emission
862:         }
863: 
864:         emit Deposit(from, _tokenId, _value, _locked.end, deposit_type, block.timestamp);
865:         emit Supply(supply_before, supply_before + _value);
866:     }

Use the balance before and after the transfer to calculate the received amount instead of assuming that it would be equal to the amount passed as a parameter. Or explicitly document that such tokens shouldn't be used and won't be supported

1.3. Unused/empty receive() function

If the intention is for the Ether to be used, the function should call another function, otherwise it should revert

core/GolomTrader.sol:461:    receive() external payable {}
rewards/RewardDistributor.sol:315:    receive() external payable {}

1.4. Use a 2-step ownership transfer pattern

Contracts inheriting from OpenZeppelin's libraries have the default transferOwnership() function (a one-step process). It's possible that the onlyOwner role mistakenly transfers ownership to a wrong address, resulting in a loss of the onlyOwner role. Consider overriding the default transferOwnership() function to first nominate an address as the pendingOwner and implementing an acceptOwnership() function which is called by the pendingOwner to confirm the transfer.

core/GolomTrader.sol:5:import '@openzeppelin/contracts/access/Ownable.sol';
core/GolomTrader.sol:40:contract GolomTrader is Ownable, ReentrancyGuard {
core/GolomTrader.sol:94:        _transferOwnership(_governance);
core/GolomTrader.sol:444:    function setDistributor(address _distributor) external onlyOwner {
core/GolomTrader.sol:454:    function executeSetDistributor() external onlyOwner {

governance/GolomToken.sol:11:import '@openzeppelin/contracts/access/Ownable.sol';
governance/GolomToken.sol:14:contract GolomToken is ERC20Votes, Ownable {
governance/GolomToken.sol:29:        _transferOwnership(_governance); // set the new owner
governance/GolomToken.sol:42:    function mintAirdrop(address _airdrop) external onlyOwner {
governance/GolomToken.sol:50:    function mintGenesisReward(address _rewardDistributor) external onlyOwner {
governance/GolomToken.sol:58:    function setMinter(address _minter) external onlyOwner {
governance/GolomToken.sol:65:    function executeSetMinter() external onlyOwner {

rewards/RewardDistributor.sol:8:import '@openzeppelin/contracts/access/Ownable.sol';
rewards/RewardDistributor.sol:43:contract RewardDistributor is Ownable {
rewards/RewardDistributor.sol:83:        _transferOwnership(_governance); // set the new owner
rewards/RewardDistributor.sol:285:    function changeTrader(address _trader) external onlyOwner {
rewards/RewardDistributor.sol:291:    function executeChangeTrader() external onlyOwner {
rewards/RewardDistributor.sol:298:    function addVoteEscrow(address _voteEscrow) external onlyOwner {
rewards/RewardDistributor.sol:308:    function executeAddVoteEscrow() external onlyOwner {

vote-escrow/VoteEscrowDelegation.sol:8:import '@openzeppelin/contracts/access/Ownable.sol';
vote-escrow/VoteEscrowDelegation.sol:20:contract VoteEscrow is VoteEscrowCore, Ownable {
vote-escrow/VoteEscrowDelegation.sol:260:    function changeMinVotingPower(uint256 _newMinVotingPower) external onlyOwner {

1.5. A magic number should be documented and explained. Use a constant instead

rewards/RewardDistributor.sol:84:        startTime = 1659211200;
rewards/RewardDistributor.sol:120:            rewardTrader[epoch] = ((tokenToEmit - stakerReward) * 67) / 100;
rewards/RewardDistributor.sol:121:            rewardExchange[epoch] = ((tokenToEmit - stakerReward) * 33) / 100;

Consider using constant variables as this would make the code more maintainable and readable while costing nothing gas-wise (constants are replaced by their value at compile-time).

1.6. require() should be used for checking error conditions on inputs and return values while assert() should be used for invariant checking

Properly functioning code should never reach a failing assert statement, unless there is a bug in your contract you should fix. Here, I believe the assert should be a require or a revert:

vote-escrow/VoteEscrowCore.sol:493:        assert(idToOwner[_tokenId] == address(0));
vote-escrow/VoteEscrowCore.sol:506:        assert(idToOwner[_tokenId] == _from);
vote-escrow/VoteEscrowCore.sol:519:        assert(idToOwner[_tokenId] == _owner);
vote-escrow/VoteEscrowCore.sol:666:        assert(_operator != msg.sender);
vote-escrow/VoteEscrowCore.sol:679:        assert(_to != address(0));
vote-escrow/VoteEscrowCore.sol:861:            assert(IERC20(token).transferFrom(from, address(this), _value));
vote-escrow/VoteEscrowCore.sol:977:        assert(_isApprovedOrOwner(msg.sender, _tokenId));
vote-escrow/VoteEscrowCore.sol:981:        assert(_value > 0); // dev: need non-zero value
vote-escrow/VoteEscrowCore.sol:991:        assert(_isApprovedOrOwner(msg.sender, _tokenId));
vote-escrow/VoteEscrowCore.sol:1007:        assert(_isApprovedOrOwner(msg.sender, _tokenId));
vote-escrow/VoteEscrowCore.sol:1023:        assert(IERC20(token).transfer(msg.sender, value));
vote-escrow/VoteEscrowCore.sol:1110:        assert(_block <= block.number);
vote-escrow/VoteEscrowCore.sol:1206:        assert(_block <= block.number);

As the Solidity version is > 0.8.* the remaining gas would still be refunded in case of failure.

2. Non-Critical Issues

2.1. It's better to emit after all processing is done

contracts/core/GolomTrader.sol:
  306:         emit OrderFilled(msg.sender, o.signer, 1, hashStruct, o.totalAmt * amount);
  307          _settleBalances(o, amount, referrer, p);

  366:         emit OrderFilled(msg.sender, o.signer, 2, hashStruct, o.totalAmt * amount);
  367          _settleBalances(o, amount, referrer, p);

2.2. Typos

• totalamt, succesful

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

• facilating

core/GolomTrader.sol:54:        Payment prePayment; // another payment , can be used for royalty, facilating trades

• succesful, succesfully

core/GolomTrader.sol:201:    /// @param p any extra payment that the taker of this order wanna send on succesful execution of order
core/GolomTrader.sol:333:    /// @param p any extra payment that the taker of this order wanna send on succesful execution of order
core/GolomTrader.sol:370:    /// @dev function to settle balances when a bid is filled succesfully
core/GolomTrader.sol:374:    /// @param p any extra payment that the taker of this order wanna send on succesful execution of order

• exhange

rewards/RewardDistributor.sol:62:    mapping(uint256 => uint256) public rewardExchange; // reward minted each epoc for exhange

2.3. Missing friendly revert strings

Here, a friendly message should exist for users to understand what went wrong:

core/GolomTrader.sol:220:            require(msg.sender == o.reservedAddress);
core/GolomTrader.sol:291:            require(msg.sender == o.reservedAddress);
core/GolomTrader.sol:293:        require(o.orderType == 1);
core/GolomTrader.sol:295:        require(status == 3);
core/GolomTrader.sol:296:        require(amountRemaining >= amount);
core/GolomTrader.sol:313:        require(o.signer == msg.sender);
core/GolomTrader.sol:342:        require(o.totalAmt >= o.exchange.paymentAmt + o.prePayment.paymentAmt + o.refererrAmt);
core/GolomTrader.sol:345:            require(msg.sender == o.reservedAddress);
core/GolomTrader.sol:347:        require(o.orderType == 2);
core/GolomTrader.sol:349:        require(status == 3);
core/GolomTrader.sol:350:        require(amountRemaining >= amount);
rewards/RewardDistributor.sol:88:        require(msg.sender == trader);
rewards/RewardDistributor.sol:144:            require(epochs[index] < epoch);
rewards/RewardDistributor.sol:158:            require(epochs[index] < epoch);
vote-escrow/VoteEscrowCore.sol:360:        require(_entered_state == _not_entered);
vote-escrow/VoteEscrowCore.sol:540:        require(_isApprovedOrOwner(_sender, _tokenId));
vote-escrow/VoteEscrowCore.sol:646:        require(owner != address(0));
vote-escrow/VoteEscrowCore.sol:648:        require(_approved != owner);
vote-escrow/VoteEscrowCore.sol:652:        require(senderIsOwner || senderIsApprovedForAll);
vote-escrow/VoteEscrowCore.sol:869:        require(msg.sender == voter);
vote-escrow/VoteEscrowCore.sol:874:        require(msg.sender == voter);
vote-escrow/VoteEscrowCore.sol:879:        require(msg.sender == voter);
vote-escrow/VoteEscrowCore.sol:884:        require(msg.sender == voter);
vote-escrow/VoteEscrowCore.sol:889:        require(msg.sender == voter);
vote-escrow/VoteEscrowCore.sol:895:        require(_from != _to);
vote-escrow/VoteEscrowCore.sol:896:        require(_isApprovedOrOwner(msg.sender, _from));
vote-escrow/VoteEscrowCore.sol:897:        require(_isApprovedOrOwner(msg.sender, _to));
vote-escrow/VoteEscrowDelegation.sol:245:        require(_isApprovedOrOwner(_sender, _tokenId));

2.4. 1000000000 should be changed to 1e9 for readability reasons

rewards/RewardDistributor.sol:100:        if (rewardToken.totalSupply() > 1000000000 * 10**18) {
vote-escrow/VoteEscrowCore.sol:308:    mapping(uint256 => Point[1000000000]) public user_point_history; // user -> Point[user_epoch]

2.5. Use a more recent version of solidity

Use a solidity version of at least 0.8.4 to get bytes.concat() instead of abi.encodePacked(<bytes>,<bytes>)

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

core/GolomTrader.sol:3:pragma solidity 0.8.11;
core/GolomTrader.sol:175:        bytes32 hash = keccak256(abi.encodePacked('\x19\x01', EIP712_DOMAIN_TYPEHASH, hashStruct));
governance/GolomToken.sol:2:pragma solidity ^0.8.11;
rewards/RewardDistributor.sol:2:pragma solidity 0.8.11;
vote-escrow/TokenUriHelper.sol:3:pragma solidity 0.8.11;
vote-escrow/TokenUriHelper.sol:74:            abi.encodePacked(
vote-escrow/TokenUriHelper.sol:82:            abi.encodePacked(
vote-escrow/TokenUriHelper.sol:90:            abi.encodePacked(
vote-escrow/TokenUriHelper.sol:98:            abi.encodePacked(
vote-escrow/TokenUriHelper.sol:109:                    abi.encodePacked(
vote-escrow/TokenUriHelper.sol:125:        output = string(abi.encodePacked('data:application/json;base64,', json));
vote-escrow/VoteEscrowCore.sol:2:pragma solidity 0.8.11;
vote-escrow/VoteEscrowDelegation.sol:2:pragma solidity 0.8.11;

2.6. Use string.concat() or bytes.concat()

Solidity version 0.8.4 introduces bytes.concat() (vs abi.encodePacked(<bytes>,<bytes>)) Solidity version 0.8.12 introduces string.concat() (vs abi.encodePacked(<str>,<str>))

core/GolomTrader.sol:3:pragma solidity 0.8.11;
core/GolomTrader.sol:175:        bytes32 hash = keccak256(abi.encodePacked('\x19\x01', EIP712_DOMAIN_TYPEHASH, hashStruct));
vote-escrow/TokenUriHelper.sol:3:pragma solidity 0.8.11;
vote-escrow/TokenUriHelper.sol:74:            abi.encodePacked(
vote-escrow/TokenUriHelper.sol:82:            abi.encodePacked(
vote-escrow/TokenUriHelper.sol:90:            abi.encodePacked(
vote-escrow/TokenUriHelper.sol:98:            abi.encodePacked(
vote-escrow/TokenUriHelper.sol:109:                    abi.encodePacked(
vote-escrow/TokenUriHelper.sol:125:        output = string(abi.encodePacked('data:application/json;base64,', json));

2.7. Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18)

rewards/RewardDistributor.sol:48:    uint256 constant dailyEmission = 600000 * 10**18;
rewards/RewardDistributor.sol:100:        if (rewardToken.totalSupply() > 1000000000 * 10**18) {

2.8. Non-library/interface files should use fixed compiler versions, not floating ones

governance/GolomToken.sol:2:pragma solidity ^0.8.11;

Overview

Table of Contents:

• 1. State variables only set in the constructor should be declared immutable
• 2. Use of this instead of marking as public an external function
• 3. Unchecking arithmetics operations that can't underflow/overflow
• 4. Tightly pack storage variables
• 5. Caching storage values in memory
  • 5.1. contracts/core/GolomTrader.sol
  • 5.2. contracts/rewards/RewardDistributor.sol
  • 5.3. contracts/vote-escrow/VoteEscrowCore.sol
• 6. Should use result of pre-increment instead of reading from storage
• 7. Use of the memory keyword when storage should be used
• 8. Use calldata instead of memory
• 9. A modifier used only once and not being inherited should be inlined to save gas
• 10. Multiple address mappings can be combined in a struct
• 11. Pre-Solidity 0.8.13: > 0 is less efficient than != 0 for unsigned integers (with proof)
• 12. Splitting require() statements that use && saves gas
• 13. Using bool for storage incurs overhead
• 14. Use shift right/left instead of division/multiplication if possible
• 15. <array>.length should not be looked up in every loop of a for-loop
• 16. ++i costs less gas compared to i++ or i += 1 (same for --i vs i-- or i -= 1)
• 17. Increments/decrements can be unchecked in for-loops
• 18. Public functions to external
• 19. It costs more gas to initialize variables with their default value than letting the default value be applied
• 20. Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18)
• 21. Optimizations with assembly
  • 21.1. Use assembly for math (add, sub, mul, div)
  • 21.2. Use assembly to check for address(0)
  • 21.3. Use assembly to write storage values
• 22. Use Custom Errors instead of Revert Strings to save Gas
• 23. (Not recommended, but true) Functions guaranteed to revert when called by normal users can be marked payable

1. State variables only set in the constructor should be declared immutable

Variables only set in the constructor and never edited afterwards should be marked as immutable, as it would avoid the expensive storage-writing operation in the constructor (around 20 000 gas per variable) and replace the expensive storage-reading operations (around 2100 gas per reading) to a less expensive value reading (3 gas)

contracts/core/GolomTrader.sol:
  45:     ERC20 WETH = ERC20(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2); //@audit gas : should be immutable

contracts/rewards/RewardDistributor.sol:
  68:     ERC20 public rewardToken; //@audit gas : should be immutable
  69:     ERC20 public weth; //@audit gas : should be immutable

contracts/vote-escrow/VoteEscrowCore.sol:
  300:     address public token;  //@audit gas : too bad that this can't be made immutable (only set once in VoteEscrowDelegation's constructor). This is set in another contract, so it cannot be made immutable anywhere. I'm still raising the sponsor's awareness on the potential gain though

2. Use of this instead of marking as public an external function

Using this. is like making an expensive external call:

contracts/vote-escrow/VoteEscrowDelegation.sol:
   73:         require(this.balanceOfNFT(tokenId) >= MIN_VOTING_POWER_REQUIRED, 'VEDelegation: Need more voting power');
  172:             votes = votes + this.balanceOfNFT(delegated[index]);
  190:             votes = votes + this.balanceOfAtNFT(delegatednft[index], blockNumber);
  242:         this.removeDelegation(_tokenId);

Consider marking those as public in VoteEscrowDelegation.sol:

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

1093:     function balanceOfNFT(uint256 _tokenId) external view returns (uint256) {
1094:         if (ownership_change[_tokenId] == block.number) return 0;
1095:         return _balanceOfNFT(_tokenId, block.timestamp);
1096:     }

1098:     function balanceOfNFTAt(uint256 _tokenId, uint256 _t) external view returns (uint256) {
1099:         return _balanceOfNFT(_tokenId, _t);
1100:     }

3. Unchecking arithmetics operations that can't underflow/overflow

Solidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers. When an overflow or an underflow isn't possible (as an example, when a comparison is made before the arithmetic operation), some gas can be saved by using an unchecked block: https://docs.soliditylang.org/en/v0.8.10/control-structures.html#checked-or-unchecked-arithmetic

Consider wrapping with an unchecked block here (around 25 gas saved per instance):

• contracts/core/GolomTrader.sol:

  189          if (filled[hashStruct] >= o.tokenAmt) {
  190              // handles erc1155
  191              return (2, hashStruct, 0);
  192          }
  193:         return (3, hashStruct, o.tokenAmt - filled[hashStruct]); //@audit should be unchecked due to L189

• contracts/vote-escrow/VoteEscrowDelegation.sol:

   78          if (nCheckpoints > 0) {
   79:             Checkpoint storage checkpoint = checkpoints[toTokenId][nCheckpoints - 1]; //@audit should be unchecked due to L78

  119:         return nCheckpoints > 0 ? checkpoints[tokenId][nCheckpoints - 1].delegatedTokenIds : myArray; //@audit should be unchecked

  133          if (nCheckpoints == 0) {
  134              return myArray;
  135          }
  136  
  137          // First check most recent balance
  138:         if (checkpoints[nftId][nCheckpoints - 1].fromBlock <= blockNumber) {  //@audit should be unchecked due to L133
  139              return checkpoints[nftId][nCheckpoints - 1].delegatedTokenIds;  //@audit should be unchecked due to L133
  140          }
...
  148:         uint256 upper = nCheckpoints - 1;  //@audit should be unchecked due to L133

4. Tightly pack storage variables

Here, struct LendingInfo can be tightly packed:

File: GolomTrader.sol
47:     struct Order {
48:         address collection; // NFT contract address
49:         uint256 tokenId; // order for which tokenId of the collection
50:         address signer; // maker of order address
51:         uint256 orderType; // 0 if selling nft for eth , 1 if offering weth for nft,2 if offering weth for collection with special criteria root
52:         uint256 totalAmt; // price value of the trade // total amt maker is willing to give up per unit of amount
53:         Payment exchange; // payment agreed by maker of the order to pay on succesful filling of trade this amt is subtracted from totalamt
54:         Payment prePayment; // another payment , can be used for royalty, facilating trades
55:         bool isERC721; // standard of the collection , if 721 then true , if 1155 then false
56:         uint256 tokenAmt; // token amt useful if standard is 1155 if >1 means whole order can be filled tokenAmt times
57:         uint256 refererrAmt; // amt to pay to the address that helps in filling your order
58:         bytes32 root; // A merkle root derived from each valid tokenId — set to 0 to indicate a collection-level or tokenId-specific order.
- 59:         address reservedAddress; // if not address(0) , only this address can fill the order
60:         uint256 nonce; // nonce of order usefull for cancelling in bulk
61:         uint256 deadline; // timestamp till order is valid epoch timestamp in secs
+ 61:         address reservedAddress; // if not address(0) , only this address can fill the order
62:         uint8 v;
63:         bytes32 r;
64:         bytes32 s;
65:     }

Which would save 1 storage slot.

5. Caching storage values in memory

The code can be optimized by minimizing the number of SLOADs.

SLOADs are expensive (100 gas after the 1st one) compared to MLOADs/MSTOREs (3 gas each). Storage values read multiple times should instead be cached in memory the first time (costing 1 SLOAD) and then read from this cache to avoid multiple SLOADs.

5.1. contracts/core/GolomTrader.sol

• filled[hashStruct]

  189:         if (filled[hashStruct] >= o.tokenAmt) { //@audit SLOAD filled[hashStruct] 
  193:         return (3, hashStruct, o.tokenAmt - filled[hashStruct]); //@audit SLOAD filled[hashStruct] 

• WETH

  382:         WETH.transferFrom(o.signer, address(this), o.totalAmt * amount); //@audit SLOAD WETH
  383:         WETH.withdraw(o.totalAmt * amount); //@audit SLOAD WETH

5.2. contracts/rewards/RewardDistributor.sol

• epoch, rewardToken

  106:         if (block.timestamp > startTime + (epoch) * secsInDay) {  //@audit SLOAD epoch
  112:             uint256 tokenToEmit = (dailyEmission * (rewardToken.totalSupply() - rewardToken.balanceOf(address(ve)))) /  //@audit SLOAD rewardToken
  113:                 rewardToken.totalSupply(); //@audit SLOAD rewardToken
  114:             uint256 stakerReward = (tokenToEmit * rewardToken.balanceOf(address(ve))) / rewardToken.totalSupply(); //@audit SLOAD rewardToken
  117:             uint256 previousEpochFee = epochTotalFee[epoch]; //@audit SLOAD epoch
  118:             epoch = epoch + 1; //@audit SLOAD epoch
  119:             rewardStaker[epoch] = stakerReward; //@audit SLOAD epoch
  120:             rewardTrader[epoch] = ((tokenToEmit - stakerReward) * 67) / 100; //@audit SLOAD epoch
  121:             rewardExchange[epoch] = ((tokenToEmit - stakerReward) * 33) / 100; //@audit SLOAD epoch
  122:             rewardToken.mint(address(this), tokenToEmit);  //@audit SLOAD rewardToken
  123:             epochBeginTime[epoch] = block.number; //@audit SLOAD epoch
  125:                 if (epoch == 1){ //@audit SLOAD epoch
  132:             emit NewEpoch(epoch, tokenToEmit, stakerReward, previousEpochFee); //@audit SLOAD epoch
  134:         feesTrader[addr[0]][epoch] = feesTrader[addr[0]][epoch] + fee; //@audit SLOAD epoch
  135:         feesExchange[addr[1]][epoch] = feesExchange[addr[1]][epoch] + fee; //@audit SLOAD epoch
  136:         epochTotalFee[epoch] = epochTotalFee[epoch] + fee; //@audit SLOAD epoch

• ve, epoch, epochBeginTime[1], epochBeginTime[index]

  173:         require(address(ve) != address(0), ' VE not added yet'); //@audit SLOAD ve
  177:         address tokenowner = ve.ownerOf(tokenids[0]); //@audit SLOAD ve
  181:             require(tokenowner == ve.ownerOf(tokenids[tindex]), 'Can only claim for a single Address together'); //@audit SLOAD ve
  191:                             ve.balanceOfAtNFT(tokenids[tindex], epochBeginTime[1])) / //@audit SLOAD epochBeginTime[1] + ve
  192:                         ve.totalSupplyAt(epochBeginTime[1]); //@audit SLOAD epochBeginTime[1] + ve
  197:                         (rewardStaker[epochs[index]] * ve.balanceOfAtNFT(tokenids[tindex], epochBeginTime[epochs[index]])) / //@audit SLOAD epochBeginTime[1]
  198:                         ve.totalSupplyAt(epochBeginTime[epochs[index]]); //@audit SLOAD epochBeginTime[1]
  202:                             ve.balanceOfAtNFT(tokenids[tindex], epochBeginTime[epochs[index]])) / //@audit SLOAD epochBeginTime[1] + ve
  203:                         ve.totalSupplyAt(epochBeginTime[epochs[index]]); //@audit SLOAD epochBeginTime[1] + ve

• ve, epoch, epochBeginTime[1], epochBeginTime[index]

  220:         require(address(ve) != address(0), ' VE not added yet'); //@audit SLOAD ve
  224:         uint256[] memory unclaimedepochs = new uint256[](epoch);//@audit SLOAD epoch
  226:         for (uint256 index = 0; index < epoch; index++) {//@audit SLOAD epoch
  233:                             ve.balanceOfAtNFT(tokenid, epochBeginTime[1])) / //@audit SLOAD ve + epochBeginTime[1]
  234:                         ve.totalSupplyAt(epochBeginTime[1]); //@audit SLOAD ve + epochBeginTime[1]
  239:                         (rewardStaker[index] * ve.balanceOfAtNFT(tokenid, epochBeginTime[index])) / //@audit SLOAD ve + epochBeginTime[index]
  240:                         ve.totalSupplyAt(epochBeginTime[index]); //@audit SLOAD ve + epochBeginTime[index]
  244:                             ve.balanceOfAtNFT(tokenid, epochBeginTime[index])) / //@audit SLOAD ve + epochBeginTime[index]
  245:                         ve.totalSupplyAt(epochBeginTime[index]); //@audit SLOAD ve + epochBeginTime[index]

5.3. contracts/vote-escrow/VoteEscrowCore.sol

• idToOwner[_tokenId]

  644:         address owner = idToOwner[_tokenId];//@audit SLOAD idToOwner[_tokenId]
  650:         bool senderIsOwner = (idToOwner[_tokenId] == msg.sender); //@audit SLOAD idToOwner[_tokenId]

6. Should use result of pre-increment instead of reading from storage

This can be done in 1 step as a pre-increment first increments and then returns the value.

File: VoteEscrowCore.sol
- 948:         ++tokenId;
- 949:         uint256 _tokenId = tokenId; 
+ 949:         uint256 _tokenId = ++tokenId; 

7. Use of the memory keyword when storage should be used

When copying a state struct in memory, there are as many SLOADs and MSTOREs as there are slots.

Consider using a storage pointer instead of memory location here, as the 2nd element of Checkpoint is actually an array getting copied in memory (while the storage pointer could be returned just like L160):

File: VoteEscrowDelegation.sol
149:         while (upper > lower) {
150:             uint256 center = upper - (upper - lower) / 2; // ceil, avoiding overflow
151:             Checkpoint memory cp = checkpoints[nftId][center]; //@audit gas should use storage
152:             if (cp.fromBlock == blockNumber) {
153:                 return cp.delegatedTokenIds;
154:             } else if (cp.fromBlock < blockNumber) {
155:                 lower = center;
156:             } else {
157:                 upper = center - 1;
158:             }
159:         }
160:         return checkpoints[nftId][lower].delegatedTokenIds;

8. Use calldata instead of memory

When a function with a memory array is called externally, the abi.decode() step has to use a for-loop to copy each index of the calldata to the memory index. Each iteration of this for-loop costs at least 60 gas (i.e. 60 * <mem_array>.length). Using calldata directly bypasses this loop.

If the array is passed to an internal function which passes the array to another internal function where the array is modified and therefore memory is used in the external call, it's still more gas-efficient to use calldata when the external function uses modifiers, since the modifiers may prevent the internal functions from being called. Structs have the same overhead as an array of length one

Affected code (around 60 gas per occurence):

core/GolomTrader.sol:127:    function _hashOrderinternal(Order calldata o, uint256[2] memory extra) private pure returns (bytes32) {
rewards/RewardDistributor.sol:98:    function addFee(address[2] memory addr, uint256 fee) public onlyTrader {
rewards/RewardDistributor.sol:141:    function traderClaim(address addr, uint256[] memory epochs) public {
rewards/RewardDistributor.sol:155:    function exchangeClaim(address addr, uint256[] memory epochs) public {
rewards/RewardDistributor.sol:172:    function multiStakerClaim(uint256[] memory tokenids, uint256[] memory epochs) public {

9. A modifier used only once and not being inherited should be inlined to save gas

Affected code:

• onlyTrader

contracts/rewards/RewardDistributor.sol:
  87:     modifier onlyTrader() {
  88          require(msg.sender == trader);
  89          _;
  90      }
...
  98:     function addFee(address[2] memory addr, uint256 fee) public onlyTrader {

• onlyMinter

contracts/governance/GolomToken.sol:
  23:     modifier onlyMinter() {
  24          require(msg.sender == minter, 'GolomToken: only reward distributor can enable');
  25          _;
  26      }
...
  36:     function mint(address _account, uint256 _amount) external onlyMinter {

10. Multiple address mappings can be combined in a struct

Computing storage costs ~42 gas, and this could be saved per access due to not having to recalculate the key's keccak256 hash.

rewards/RewardDistributor.sol:58:    mapping(address => mapping(uint256 => uint256)) public feesTrader; // fees accumulated by address of trader per epoch
rewards/RewardDistributor.sol:59:    mapping(address => mapping(uint256 => uint256)) public feesExchange; // fees accumulated by exchange of trader per epoch

vote-escrow/VoteEscrowCore.sol:332:    mapping(address => uint256) internal ownerToNFTokenCount;
vote-escrow/VoteEscrowCore.sol:335:    mapping(address => mapping(uint256 => uint256)) internal ownerToNFTokenIdList;
vote-escrow/VoteEscrowCore.sol:341:    mapping(address => mapping(address => bool)) internal ownerToOperators;

11. Pre-Solidity 0.8.13: > 0 is less efficient than != 0 for unsigned integers (with proof)

Up until Solidity 0.8.13: != 0 costs less gas compared to > 0 for unsigned integers in require statements with the optimizer enabled (6 gas)

Proof: While it may seem that > 0 is cheaper than !=, this is only true without the optimizer enabled and outside a require statement. If you enable the optimizer AND you're in a require statement, this will save gas. You can see this tweet for more proofs: https://twitter.com/gzeon/status/1485428085885640706

Consider changing > 0 with != 0 here:

vote-escrow/VoteEscrowCore.sol:927:        require(_value > 0); // dev: need non-zero value
vote-escrow/VoteEscrowCore.sol:928:        require(_locked.amount > 0, 'No existing lock found');
vote-escrow/VoteEscrowCore.sol:944:        require(_value > 0); // dev: need non-zero value
vote-escrow/VoteEscrowCore.sol:981:        assert(_value > 0); // dev: need non-zero value
vote-escrow/VoteEscrowCore.sol:982:        require(_locked.amount > 0, 'No existing lock found');
vote-escrow/VoteEscrowCore.sol:997:        require(_locked.amount > 0, 'Nothing is locked');

Also, please enable the Optimizer.

12. Splitting require() statements that use && saves gas

See this issue which describes the fact that there is a larger deployment gas cost, but with enough runtime calls, the change ends up being cheaper

Affected code (saving around 3 gas per instance):

vote-escrow/VoteEscrowCore.sol:538:        require(attachments[_tokenId] == 0 && !voted[_tokenId], 'attached');
vote-escrow/VoteEscrowCore.sol:894:        require(attachments[_from] == 0 && !voted[_from], 'attached');
vote-escrow/VoteEscrowCore.sol:1008:        require(attachments[_tokenId] == 0 && !voted[_tokenId], 'attached');
vote-escrow/VoteEscrowDelegation.sol:239:        require(attachments[_tokenId] == 0 && !voted[_tokenId], 'attached');

13. Using bool for storage incurs overhead

Booleans are more expensive than uint256 or any type that takes up a full word because each write operation emits an extra SLOAD to first read the slot's contents, replace the bits taken up by the boolean, and then write back. This is the compiler's defense against contract upgrades and pointer aliasing, and it cannot be disabled.

core/GolomTrader.sol:55:        bool isERC721; // standard of the collection , if 721 then true , if 1155 then false
governance/GolomToken.sol:20:    bool public isAirdropMinted;
governance/GolomToken.sol:21:    bool public isGenesisRewardMinted;

14. Use shift right/left instead of division/multiplication if possible

While the DIV / MUL opcode uses 5 gas, the SHR / SHL opcode only uses 3 gas. Furthermore, beware that Solidity's division operation also includes a division-by-0 prevention which is bypassed using shifting. Eventually, overflow checks are never performed for shift operations as they are done for arithmetic operations. Instead, the result is always truncated, so the calculation can be unchecked in Solidity version 0.8+

• Use >> 1 instead of / 2
• Use >> 2 instead of / 4
• Use << 3 instead of * 8

Affected code (saves around 2 gas + 20 for unchecked per instance):

vote-escrow/VoteEscrowCore.sol:1049:            uint256 _mid = (_min + _max + 1) / 2;
vote-escrow/VoteEscrowCore.sol:1120:            uint256 _mid = (_min + _max + 1) / 2;
vote-escrow/VoteEscrowDelegation.sol:150:            uint256 center = upper - (upper - lower) / 2; // ceil, avoiding overflow

15. <array>.length should not be looked up in every loop of a for-loop

Reading array length at each iteration of the loop consumes more gas than necessary.

In the best case scenario (length read on a memory variable), caching the array length in the stack saves around 3 gas per iteration. In the worst case scenario (external calls at each iteration), the amount of gas wasted can be massive.

Here, consider storing the array's length in a variable before the for-loop, and use this new variable instead:

core/GolomTrader.sol:415:        for (uint256 i = 0; i < proof.length; i++) {
rewards/RewardDistributor.sol:143:        for (uint256 index = 0; index < epochs.length; index++) {
rewards/RewardDistributor.sol:157:        for (uint256 index = 0; index < epochs.length; index++) {
rewards/RewardDistributor.sol:180:        for (uint256 tindex = 0; tindex < tokenids.length; tindex++) {
rewards/RewardDistributor.sol:183:            for (uint256 index = 0; index < epochs.length; index++) {
vote-escrow/VoteEscrowDelegation.sol:171:        for (uint256 index = 0; index < delegated.length; index++) {
vote-escrow/VoteEscrowDelegation.sol:189:        for (uint256 index = 0; index < delegatednft.length; index++) {
vote-escrow/VoteEscrowDelegation.sol:199:        for (uint256 i; i < _array.length; i++) {

16. ++i costs less gas compared to i++ or i += 1 (same for --i vs i-- or i -= 1)

Pre-increments and pre-decrements are cheaper.

For a uint256 i variable, the following is true with the Optimizer enabled at 10k:

Increment:

• i += 1 is the most expensive form
• i++ costs 6 gas less than i += 1
• ++i costs 5 gas less than i++ (11 gas less than i += 1)

Decrement:

• i -= 1 is the most expensive form
• i-- costs 11 gas less than i -= 1
• --i costs 5 gas less than i-- (16 gas less than i -= 1)

Note that post-increments (or post-decrements) return the old value before incrementing or decrementing, hence the name post-increment:

uint i = 1;  
uint j = 2;
require(j == i++, "This will be false as i is incremented after the comparison");

However, pre-increments (or pre-decrements) return the new value:

uint i = 1;  
uint j = 2;
require(j == ++i, "This will be true as i is incremented before the comparison");

In the pre-increment case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2.

Affected code:

core/GolomTrader.sol:415:        for (uint256 i = 0; i < proof.length; i++) {
rewards/RewardDistributor.sol:143:        for (uint256 index = 0; index < epochs.length; index++) {
rewards/RewardDistributor.sol:157:        for (uint256 index = 0; index < epochs.length; index++) {
rewards/RewardDistributor.sol:180:        for (uint256 tindex = 0; tindex < tokenids.length; tindex++) {
rewards/RewardDistributor.sol:183:            for (uint256 index = 0; index < epochs.length; index++) {
rewards/RewardDistributor.sol:226:        for (uint256 index = 0; index < epoch; index++) {
rewards/RewardDistributor.sol:258:        for (uint256 index = 0; index < epoch; index++) {
rewards/RewardDistributor.sol:273:        for (uint256 index = 0; index < epoch; index++) {
vote-escrow/TokenUriHelper.sol:138:            digits++;
vote-escrow/TokenUriHelper.sol:143:            digits -= 1;
vote-escrow/VoteEscrowCore.sol:499:        ownerToNFTokenCount[_to] += 1;
vote-escrow/VoteEscrowCore.sol:512:        ownerToNFTokenCount[_from] -= 1;
vote-escrow/VoteEscrowCore.sol:768:                _epoch += 1;
vote-escrow/VoteEscrowDelegation.sol:171:        for (uint256 index = 0; index < delegated.length; index++) {
vote-escrow/VoteEscrowDelegation.sol:189:        for (uint256 index = 0; index < delegatednft.length; index++) {
vote-escrow/VoteEscrowDelegation.sol:199:        for (uint256 i; i < _array.length; i++) {

Consider using pre-increments and pre-decrements where they are relevant (meaning: not where post-increments/decrements logic are relevant).

17. Increments/decrements can be unchecked in for-loops

In Solidity 0.8+, there's a default overflow check on unsigned integers. It's possible to uncheck this in for-loops and save some gas at each iteration, but at the cost of some code readability, as this uncheck cannot be made inline.

ethereum/solidity#10695

Consider wrapping with an unchecked block here (around 25 gas saved per instance):

core/GolomTrader.sol:415:        for (uint256 i = 0; i < proof.length; i++) {
rewards/RewardDistributor.sol:143:        for (uint256 index = 0; index < epochs.length; index++) {
rewards/RewardDistributor.sol:157:        for (uint256 index = 0; index < epochs.length; index++) {
rewards/RewardDistributor.sol:180:        for (uint256 tindex = 0; tindex < tokenids.length; tindex++) {
rewards/RewardDistributor.sol:183:            for (uint256 index = 0; index < epochs.length; index++) {
rewards/RewardDistributor.sol:226:        for (uint256 index = 0; index < epoch; index++) {
rewards/RewardDistributor.sol:258:        for (uint256 index = 0; index < epoch; index++) {
rewards/RewardDistributor.sol:273:        for (uint256 index = 0; index < epoch; index++) {
vote-escrow/VoteEscrowCore.sol:745:            for (uint256 i = 0; i < 255; ++i) {
vote-escrow/VoteEscrowCore.sol:1044:        for (uint256 i = 0; i < 128; ++i) {
vote-escrow/VoteEscrowCore.sol:1115:        for (uint256 i = 0; i < 128; ++i) {
vote-escrow/VoteEscrowCore.sol:1167:        for (uint256 i = 0; i < 255; ++i) {
vote-escrow/VoteEscrowDelegation.sol:171:        for (uint256 index = 0; index < delegated.length; index++) {
vote-escrow/VoteEscrowDelegation.sol:189:        for (uint256 index = 0; index < delegatednft.length; index++) {
vote-escrow/VoteEscrowDelegation.sol:199:        for (uint256 i; i < _array.length; i++) {

The change would be:

- for (uint256 i; i < numIterations; i++) {
+ for (uint256 i; i < numIterations;) {
 // ...  
+   unchecked { ++i; }
}  

The same can be applied with decrements (which should use break when i == 0).

The risk of overflow is non-existant for uint256 here.

18. Public functions to external

An external call cost is less expensive than one of a public function. The following functions could be set external to save gas and improve code quality (extracted from Slither, relevant to the current scope).

fillBid(GolomTrader.Order,uint256,address,GolomTrader.Payment) should be declared external:
 - GolomTrader.fillBid(GolomTrader.Order,uint256,address,GolomTrader.Payment) (contracts/core/GolomTrader.sol#279-308)
cancelOrder(GolomTrader.Order) should be declared external:
 - GolomTrader.cancelOrder(GolomTrader.Order) (contracts/core/GolomTrader.sol#312-317)
fillCriteriaBid(GolomTrader.Order,uint256,uint256,bytes32[],address,GolomTrader.Payment) should be declared external:
 - GolomTrader.fillCriteriaBid(GolomTrader.Order,uint256,uint256,bytes32[],address,GolomTrader.Payment) (contracts/core/GolomTrader.sol#334-368)
traderClaim(address,uint256[]) should be declared external:
 - RewardDistributor.traderClaim(address,uint256[]) (contracts/rewards/RewardDistributor.sol#141-152)
exchangeClaim(address,uint256[]) should be declared external:
 - RewardDistributor.exchangeClaim(address,uint256[]) (contracts/rewards/RewardDistributor.sol#155-166)
multiStakerClaim(uint256[],uint256[]) should be declared external:
 - RewardDistributor.multiStakerClaim(uint256[],uint256[]) (contracts/rewards/RewardDistributor.sol#172-210)
stakerRewards(uint256) should be declared external:
 - RewardDistributor.stakerRewards(uint256) (contracts/rewards/RewardDistributor.sol#215-250)
traderRewards(address) should be declared external:
 - RewardDistributor.traderRewards(address) (contracts/rewards/RewardDistributor.sol#254-265)
exchangeRewards(address) should be declared external:
 - RewardDistributor.exchangeRewards(address) (contracts/rewards/RewardDistributor.sol#269-280)
getPriorVotes(uint256,uint256) should be declared external:
 - VoteEscrow.getPriorVotes(uint256,uint256) (contracts/vote-escrow/VoteEscrowDelegation.sol#185-193)

19. It costs more gas to initialize variables with their default value than letting the default value be applied

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for address...). Explicitly initializing it with its default value is an anti-pattern and wastes gas (especially true for storage values)

Affected code:

rewards/RewardDistributor.sol:45:    uint256 public epoch = 0;
vote-escrow/VoteEscrowDelegation.sol:50:    uint256 public MIN_VOTING_POWER_REQUIRED = 0;

Consider removing explicit initializations for default values.

20. Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18)

rewards/RewardDistributor.sol:48:    uint256 constant dailyEmission = 600000 * 10**18;
rewards/RewardDistributor.sol:100:        if (rewardToken.totalSupply() > 1000000000 * 10**18) {

21. Optimizations with assembly

The original warden who proved these type of findings is 0xKitsune. Clone the repo 0xKitsune/gas-lab, copy/paste the contract examples and run forge test --gas-report to replicate the gas reports with the optimizer turned on and set to 10000 runs.

21.1. Use assembly for math (add, sub, mul, div)

Use assembly for math instead of Solidity. You can check for overflow/underflow in assembly to ensure safety. If using Solidity versions < 0.8.0 and you are using Safemath, you can gain significant gas savings by using assembly to calculate values and checking for overflow/underflow.

POC Contract:

contract GasTest is DSTest {
    Contract0 c0;
    Contract1 c1;
    Contract2 c2;
    Contract3 c3;
    Contract4 c4;
    Contract5 c5;
    Contract6 c6;
    Contract7 c7;

    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
        c2 = new Contract2();
        c3 = new Contract3();
        c4 = new Contract4();
        c5 = new Contract5();
        c6 = new Contract6();
        c7 = new Contract7();
    }

    function testGas() public {
        c0.addTest(34598345, 100);
        c1.addAssemblyTest(34598345, 100);
        c2.subTest(34598345, 100);
        c3.subAssemblyTest(34598345, 100);
        c4.mulTest(34598345, 100);
        c5.mulAssemblyTest(34598345, 100);
        c6.divTest(34598345, 100);
        c7.divAssemblyTest(34598345, 100);
    }
}

contract Contract0 {
    //addition in Solidity
    function addTest(uint256 a, uint256 b) public pure {
        uint256 c = a + b;
    }
}

contract Contract1 {
    //addition in assembly
    function addAssemblyTest(uint256 a, uint256 b) public pure {
        assembly {
            let c := add(a, b)

            if lt(c, a) {
                mstore(0x00, "overflow")
                revert(0x00, 0x20)
            }
        }
    }
}

contract Contract2 {
    //subtraction in Solidity
    function subTest(uint256 a, uint256 b) public pure {
        uint256 c = a - b;
    }
}

contract Contract3 {
    //subtraction in assembly
    function subAssemblyTest(uint256 a, uint256 b) public pure {
        assembly {
            let c := sub(a, b)

            if gt(c, a) {
                mstore(0x00, "underflow")
                revert(0x00, 0x20)
            }
        }
    }
}

contract Contract4 {
    //multiplication in Solidity
    function mulTest(uint256 a, uint256 b) public pure {
        uint256 c = a * b;
    }
}

contract Contract5 {
    //multiplication in assembly
    function mulAssemblyTest(uint256 a, uint256 b) public pure {
        assembly {
            let c := mul(a, b)

            if lt(c, a) {
                mstore(0x00, "overflow")
                revert(0x00, 0x20)
            }
        }
    }
}

contract Contract6 {
    //division in Solidity
    function divTest(uint256 a, uint256 b) public pure {
        uint256 c = a * b;
    }
}

contract Contract7 {
    //division in assembly
    function divAssemblyTest(uint256 a, uint256 b) public pure {
        assembly {
            let c := div(a, b)

            if gt(c, a) {
                mstore(0x00, "underflow")
                revert(0x00, 0x20)
            }
        }
    }
}

POC Gas Report:

╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract0 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 40493              ┆ 233             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ addTest            ┆ 303             ┆ 303 ┆ 303    ┆ 303 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯
╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract1 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 37087              ┆ 216             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ addAssemblyTest    ┆ 263             ┆ 263 ┆ 263    ┆ 263 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯
╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract2 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 40293              ┆ 232             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ subTest            ┆ 300             ┆ 300 ┆ 300    ┆ 300 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯
╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract3 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 37287              ┆ 217             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ subAssemblyTest    ┆ 263             ┆ 263 ┆ 263    ┆ 263 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯
╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract4 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 41893              ┆ 240             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ mulTest            ┆ 325             ┆ 325 ┆ 325    ┆ 325 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯
╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract5 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 37087              ┆ 216             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ mulAssemblyTest    ┆ 265             ┆ 265 ┆ 265    ┆ 265 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯
╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract6 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 41893              ┆ 240             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ divTest            ┆ 325             ┆ 325 ┆ 325    ┆ 325 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯
╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract7 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 37287              ┆ 217             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ divAssemblyTest    ┆ 265             ┆ 265 ┆ 265    ┆ 265 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯

Affected code:

core/GolomTrader.sol:193:        return (3, hashStruct, o.tokenAmt - filled[hashStruct]);
core/GolomTrader.sol:212:            o.totalAmt >= o.exchange.paymentAmt + o.prePayment.paymentAmt + o.refererrAmt + (o.totalAmt * 50) / 10000,
core/GolomTrader.sol:217:        require(msg.value >= o.totalAmt * amount + p.paymentAmt, 'mgmtm');
core/GolomTrader.sol:229:        filled[hashStruct] = filled[hashStruct] + amount;
core/GolomTrader.sol:242:        payEther(((o.totalAmt * 50) / 10000) * amount, address(distributor));
core/GolomTrader.sol:245:        payEther(o.exchange.paymentAmt * amount, o.exchange.paymentAddress);
core/GolomTrader.sol:248:        payEther(o.prePayment.paymentAmt * amount, o.prePayment.paymentAddress);
core/GolomTrader.sol:251:            payEther(o.refererrAmt * amount, referrer);
core/GolomTrader.sol:254:                    (o.totalAmt * 50) /
core/GolomTrader.sol:258:                    o.refererrAmt) * amount,
core/GolomTrader.sol:263:                (o.totalAmt - (o.totalAmt * 50) / 10000 - o.exchange.paymentAmt - o.prePayment.paymentAmt) * amount,
core/GolomTrader.sol:269:        distributor.addFee([o.signer, o.exchange.paymentAddress], ((o.totalAmt * 50) / 10000) * amount);
core/GolomTrader.sol:270:        emit OrderFilled(o.signer, msg.sender, 0, hashStruct, o.totalAmt * amount);
core/GolomTrader.sol:286:            o.totalAmt * amount >
core/GolomTrader.sol:287:                (o.exchange.paymentAmt + o.prePayment.paymentAmt + o.refererrAmt) * amount + p.paymentAmt
core/GolomTrader.sol:297:        filled[hashStruct] = filled[hashStruct] + amount;
core/GolomTrader.sol:306:        emit OrderFilled(msg.sender, o.signer, 1, hashStruct, o.totalAmt * amount);
core/GolomTrader.sol:315:        filled[hashStruct] = o.tokenAmt + 1;
core/GolomTrader.sol:342:        require(o.totalAmt >= o.exchange.paymentAmt + o.prePayment.paymentAmt + o.refererrAmt);
core/GolomTrader.sol:352:        filled[hashStruct] = filled[hashStruct] + amount;
core/GolomTrader.sol:366:        emit OrderFilled(msg.sender, o.signer, 2, hashStruct, o.totalAmt * amount);
core/GolomTrader.sol:381:        uint256 protocolfee = ((o.totalAmt * 50) / 10000) * amount;
core/GolomTrader.sol:382:        WETH.transferFrom(o.signer, address(this), o.totalAmt * amount);
core/GolomTrader.sol:383:        WETH.withdraw(o.totalAmt * amount);
core/GolomTrader.sol:385:        payEther(o.exchange.paymentAmt * amount, o.exchange.paymentAddress);
core/GolomTrader.sol:386:        payEther(o.prePayment.paymentAmt * amount, o.prePayment.paymentAddress);
core/GolomTrader.sol:388:            payEther(o.refererrAmt * amount, referrer);
core/GolomTrader.sol:390:                (o.totalAmt - protocolfee - o.exchange.paymentAmt - o.prePayment.paymentAmt - o.refererrAmt) *
core/GolomTrader.sol:397:                (o.totalAmt - protocolfee - o.exchange.paymentAmt - o.prePayment.paymentAmt) * amount - p.paymentAmt,
core/GolomTrader.sol:449:            distributorEnableDate = block.timestamp + 1 days;
governance/GolomToken.sol:44:        _mint(_airdrop, 150_000_000 * 1e18);
governance/GolomToken.sol:52:        _mint(_rewardDistributor, 62_500_000 * 1e18);
governance/GolomToken.sol:60:        minterEnableDate = block.timestamp + 1 days;
rewards/RewardDistributor.sol:100:        if (rewardToken.totalSupply() > 1000000000 * 10**18) {
rewards/RewardDistributor.sol:106:        if (block.timestamp > startTime + (epoch) * secsInDay) {
rewards/RewardDistributor.sol:112:            uint256 tokenToEmit = (dailyEmission * (rewardToken.totalSupply() - rewardToken.balanceOf(address(ve)))) /
rewards/RewardDistributor.sol:114:            uint256 stakerReward = (tokenToEmit * rewardToken.balanceOf(address(ve))) / rewardToken.totalSupply();
rewards/RewardDistributor.sol:118:            epoch = epoch + 1;
rewards/RewardDistributor.sol:120:            rewardTrader[epoch] = ((tokenToEmit - stakerReward) * 67) / 100;
rewards/RewardDistributor.sol:121:            rewardExchange[epoch] = ((tokenToEmit - stakerReward) * 33) / 100;
rewards/RewardDistributor.sol:134:        feesTrader[addr[0]][epoch] = feesTrader[addr[0]][epoch] + fee;
rewards/RewardDistributor.sol:135:        feesExchange[addr[1]][epoch] = feesExchange[addr[1]][epoch] + fee;
rewards/RewardDistributor.sol:136:        epochTotalFee[epoch] = epochTotalFee[epoch] + fee;
rewards/RewardDistributor.sol:147:                (rewardTrader[epochs[index]] * feesTrader[addr][epochs[index]]) /
rewards/RewardDistributor.sol:161:                (rewardExchange[epochs[index]] * feesExchange[addr][epochs[index]]) /
rewards/RewardDistributor.sol:197:                        (rewardStaker[epochs[index]] * ve.balanceOfAtNFT(tokenids[tindex], epochBeginTime[epochs[index]])) /
rewards/RewardDistributor.sol:239:                        (rewardStaker[index] * ve.balanceOfAtNFT(tokenid, epochBeginTime[index])) /
rewards/RewardDistributor.sol:261:                (rewardTrader[index] * feesTrader[addr][index]) /
rewards/RewardDistributor.sol:276:                (rewardExchange[index] * feesExchange[addr][index]) /
rewards/RewardDistributor.sol:286:        traderEnableDate = block.timestamp + 1 days;
rewards/RewardDistributor.sol:302:            voteEscrowEnableDate = block.timestamp + 1 days;
vote-escrow/TokenUriHelper.sol:17:        uint256 encodedLen = 4 * ((len + 2) / 3);
vote-escrow/TokenUriHelper.sol:20:        bytes memory result = new bytes(encodedLen + 32);
vote-escrow/TokenUriHelper.sol:144:            buffer[digits] = bytes1(uint8(48 + uint256(value % 10)));
vote-escrow/VoteEscrowCore.sol:20:# 1 +        /
vote-escrow/VoteEscrowCore.sol:262:    int128 slope; // # -dweight / dt
vote-escrow/VoteEscrowCore.sol:464:        uint256 current_count = _balance(_from) - 1;
vote-escrow/VoteEscrowCore.sol:705:                u_old.slope = old_locked.amount / iMAXTIME;
vote-escrow/VoteEscrowCore.sol:706:                u_old.bias = u_old.slope * int128(int256(old_locked.end - block.timestamp));
vote-escrow/VoteEscrowCore.sol:709:                u_new.slope = new_locked.amount / iMAXTIME;
vote-escrow/VoteEscrowCore.sol:710:                u_new.bias = u_new.slope * int128(int256(new_locked.end - block.timestamp));
vote-escrow/VoteEscrowCore.sol:737:            block_slope = (MULTIPLIER * (block.number - last_point.blk)) / (block.timestamp - last_point.ts);
vote-escrow/VoteEscrowCore.sol:744:            uint256 t_i = (last_checkpoint / WEEK) * WEEK;
vote-escrow/VoteEscrowCore.sol:755:                last_point.bias -= last_point.slope * int128(int256(t_i - last_checkpoint));
vote-escrow/VoteEscrowCore.sol:767:                last_point.blk = initial_last_point.blk + (block_slope * (t_i - initial_last_point.ts)) / MULTIPLIER;
vote-escrow/VoteEscrowCore.sol:784:            last_point.slope += (u_new.slope - u_old.slope);
vote-escrow/VoteEscrowCore.sol:785:            last_point.bias += (u_new.bias - u_old.bias);
vote-escrow/VoteEscrowCore.sol:818:            uint256 user_epoch = user_point_epoch[_tokenId] + 1;
vote-escrow/VoteEscrowCore.sol:843:        supply = supply_before + _value;
vote-escrow/VoteEscrowCore.sol:865:        emit Supply(supply_before, supply_before + _value);
vote-escrow/VoteEscrowCore.sol:885:        attachments[_tokenId] = attachments[_tokenId] + 1;
vote-escrow/VoteEscrowCore.sol:890:        attachments[_tokenId] = attachments[_tokenId] - 1;
vote-escrow/VoteEscrowCore.sol:942:        uint256 unlock_time = ((block.timestamp + _lock_duration) / WEEK) * WEEK; // Locktime is rounded down to weeks
vote-escrow/VoteEscrowCore.sol:946:        require(unlock_time <= block.timestamp + MAXTIME, 'Voting lock can be 4 years max');
vote-escrow/VoteEscrowCore.sol:994:        uint256 unlock_time = ((block.timestamp + _lock_duration) / WEEK) * WEEK; // Locktime is rounded down to weeks
vote-escrow/VoteEscrowCore.sol:999:        require(unlock_time <= block.timestamp + MAXTIME, 'Voting lock can be 4 years max');
vote-escrow/VoteEscrowCore.sol:1016:        supply = supply_before - value;
vote-escrow/VoteEscrowCore.sol:1029:        emit Supply(supply_before, supply_before - value);
vote-escrow/VoteEscrowCore.sol:1049:            uint256 _mid = (_min + _max + 1) / 2;
vote-escrow/VoteEscrowCore.sol:1053:                _max = _mid - 1;
vote-escrow/VoteEscrowCore.sol:1071:            last_point.bias -= last_point.slope * int128(int256(_t) - int256(last_point.ts));
vote-escrow/VoteEscrowCore.sol:1120:            uint256 _mid = (_min + _max + 1) / 2;
vote-escrow/VoteEscrowCore.sol:1124:                _max = _mid - 1;
vote-escrow/VoteEscrowCore.sol:1136:            Point memory point_1 = point_history[_epoch + 1];
vote-escrow/VoteEscrowCore.sol:1137:            d_block = point_1.blk - point_0.blk;
vote-escrow/VoteEscrowCore.sol:1138:            d_t = point_1.ts - point_0.ts;
vote-escrow/VoteEscrowCore.sol:1140:            d_block = block.number - point_0.blk;
vote-escrow/VoteEscrowCore.sol:1141:            d_t = block.timestamp - point_0.ts;
vote-escrow/VoteEscrowCore.sol:1145:            block_time += (d_t * (_block - point_0.blk)) / d_block;
vote-escrow/VoteEscrowCore.sol:1148:        upoint.bias -= upoint.slope * int128(int256(block_time - upoint.ts));
vote-escrow/VoteEscrowCore.sol:1166:        uint256 t_i = (last_point.ts / WEEK) * WEEK;
vote-escrow/VoteEscrowCore.sol:1175:            last_point.bias -= last_point.slope * int128(int256(t_i - last_point.ts));
vote-escrow/VoteEscrowCore.sol:1213:            Point memory point_next = point_history[target_epoch + 1];
vote-escrow/VoteEscrowCore.sol:1215:                dt = ((_block - point.blk) * (point_next.ts - point.ts)) / (point_next.blk - point.blk);
vote-escrow/VoteEscrowCore.sol:1219:                dt = ((_block - point.blk) * (block.timestamp - point.ts)) / (block.number - point.blk);
vote-escrow/VoteEscrowCore.sol:1223:        return _supply_at(point, point.ts + dt);
vote-escrow/VoteEscrowDelegation.sol:79:            Checkpoint storage checkpoint = checkpoints[toTokenId][nCheckpoints - 1];
vote-escrow/VoteEscrowDelegation.sol:101:        Checkpoint memory oldCheckpoint = checkpoints[toTokenId][nCheckpoints - 1];
vote-escrow/VoteEscrowDelegation.sol:107:            numCheckpoints[toTokenId] = nCheckpoints + 1;
vote-escrow/VoteEscrowDelegation.sol:138:        if (checkpoints[nftId][nCheckpoints - 1].fromBlock <= blockNumber) {
vote-escrow/VoteEscrowDelegation.sol:139:            return checkpoints[nftId][nCheckpoints - 1].delegatedTokenIds;
vote-escrow/VoteEscrowDelegation.sol:148:        uint256 upper = nCheckpoints - 1;
vote-escrow/VoteEscrowDelegation.sol:150:            uint256 center = upper - (upper - lower) / 2; // ceil, avoiding overflow
vote-escrow/VoteEscrowDelegation.sol:157:                upper = center - 1;
vote-escrow/VoteEscrowDelegation.sol:172:            votes = votes + this.balanceOfNFT(delegated[index]);
vote-escrow/VoteEscrowDelegation.sol:190:            votes = votes + this.balanceOfAtNFT(delegatednft[index], blockNumber);
vote-escrow/VoteEscrowDelegation.sol:201:                _array[i] = _array[_array.length - 1];
vote-escrow/VoteEscrowDelegation.sol:213:        Checkpoint storage checkpoint = checkpoints[tokenId][nCheckpoints - 1];

21.2. Use assembly to check for address(0)

contract GasTest is DSTest {
    Contract0 c0;
    Contract1 c1;

    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
    }

    function testGas() public view {
        c0.ownerNotZero(address(this));
        c1.assemblyOwnerNotZero(address(this));
    }
}

contract Contract0 {
    function ownerNotZero(address _addr) public pure {
        require(_addr != address(0), "zero address)");
    }
}

contract Contract1 {
    function assemblyOwnerNotZero(address _addr) public pure {
        assembly {
            if iszero(_addr) {
                mstore(0x00, "zero address")
                revert(0x00, 0x20)
            }
        }
    }
}

POC Gas Report:

╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract0 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 61311              ┆ 338             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ ownerNotZero       ┆ 258             ┆ 258 ┆ 258    ┆ 258 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯
╭──────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract1 contract   ┆                 ┆     ┆        ┆     ┆         │
╞══════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost      ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 44893                ┆ 255             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name        ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ assemblyOwnerNotZero ┆ 252             ┆ 252 ┆ 252    ┆ 252 ┆ 1       │
╰──────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯

Affected code:

core/GolomTrader.sol:219:        if (o.reservedAddress != address(0)) {
core/GolomTrader.sol:231:        if (receiver == address(0)) {
core/GolomTrader.sol:250:        if (o.refererrAmt > 0 && referrer != address(0)) {
core/GolomTrader.sol:290:        if (o.reservedAddress != address(0)) {
core/GolomTrader.sol:344:        if (o.reservedAddress != address(0)) {
core/GolomTrader.sol:387:        if (o.refererrAmt > 0 && referrer != address(0)) {
core/GolomTrader.sol:445:        if (address(distributor) == address(0)) {
governance/GolomToken.sol:66:        if (minter == address(0)) {
rewards/RewardDistributor.sol:173:        require(address(ve) != address(0), ' VE not added yet');
rewards/RewardDistributor.sol:220:        require(address(ve) != address(0), ' VE not added yet');
rewards/RewardDistributor.sol:299:        if (address(ve) == address(0)) {
vote-escrow/VoteEscrowCore.sol:493:        assert(idToOwner[_tokenId] == address(0));
vote-escrow/VoteEscrowCore.sol:520:        if (idToApprovals[_tokenId] != address(0)) {
vote-escrow/VoteEscrowCore.sol:646:        require(owner != address(0));
vote-escrow/VoteEscrowCore.sol:679:        assert(_to != address(0));
vote-escrow/VoteEscrowCore.sol:682:        emit Transfer(address(0), _to, _tokenId);
vote-escrow/VoteEscrowCore.sol:1082:        require(idToOwner[_tokenId] != address(0), 'Query for nonexistent token');
vote-escrow/VoteEscrowCore.sol:1235:        emit Transfer(owner, address(0), _tokenId);
vote-escrow/VoteEscrowDelegation.sol:63:        emit Transfer(address(0), address(this), tokenId);
vote-escrow/VoteEscrowDelegation.sol:65:        emit Transfer(address(this), address(0), tokenId);

21.3. Use assembly to write storage values

contract GasTest is DSTest {
    Contract0 c0;
    Contract1 c1;

    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
    }

    function testGas() public {
        c0.updateOwner(0x158B28A1b1CB1BE12C6bD8f5a646a0e3B2024734);
        c1.assemblyUpdateOwner(0x158B28A1b1CB1BE12C6bD8f5a646a0e3B2024734);
    }
}

contract Contract0 {
    address owner = 0xb4c79daB8f259C7Aee6E5b2Aa729821864227e84;

    function updateOwner(address newOwner) public {
        owner = newOwner;
    }
}

contract Contract1 {
    address owner = 0xb4c79daB8f259C7Aee6E5b2Aa729821864227e84;

    function assemblyUpdateOwner(address newOwner) public {
        assembly {
            sstore(owner.slot, newOwner)
        }
    }
}

POC Gas Report:

╭────────────────────┬─────────────────┬──────┬────────┬──────┬─────────╮
│ Contract0 contract ┆                 ┆      ┆        ┆      ┆         │
╞════════════════════╪═════════════════╪══════╪════════╪══════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 60623              ┆ 261             ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg  ┆ median ┆ max  ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ updateOwner        ┆ 5302            ┆ 5302 ┆ 5302   ┆ 5302 ┆ 1       │
╰────────────────────┴─────────────────┴──────┴────────┴──────┴─────────╯
╭────────────────────┬─────────────────┬──────┬────────┬──────┬─────────╮
│ Contract1 contract ┆                 ┆      ┆        ┆      ┆         │
╞════════════════════╪═════════════════╪══════╪════════╪══════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 54823              ┆ 232             ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg  ┆ median ┆ max  ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ assemblyUpdateOwner┆ 5236            ┆ 5236 ┆ 5236   ┆ 5236 ┆ 1       │
╰────────────────────┴─────────────────┴──────┴────────┴──────┴─────────╯

Affected code:

core/GolomTrader.sol:444:    function setDistributor(address _distributor) external onlyOwner {
governance/GolomToken.sol:58:    function setMinter(address _minter) external onlyOwner {
vote-escrow/VoteEscrowCore.sol:664:    function setApprovalForAll(address _operator, bool _approved) external {
vote-escrow/VoteEscrowCore.sol:868:    function setVoter(address _voter) external {

22. Use Custom Errors instead of Revert Strings to save Gas

Solidity 0.8.4 introduced custom errors. They are more gas efficient than revert strings, when it comes to deploy cost as well as runtime cost when the revert condition is met. Use custom errors instead of revert strings for gas savings.

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met).

Additionally, custom errors can be used inside and outside of contracts (including interfaces and libraries).

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

Starting from Solidity v0.8.4, there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors. Until now, you could already use strings to give more information about failures (e.g., revert("Insufficient funds.");), but they are rather expensive, especially when it comes to deploy cost, and it is difficult to use dynamic information in them.

POC Contract:

contract GasTest is DSTest {
    Contract0 c0;
    Contract1 c1;

    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
    }

    function testFailGas() public {
        c0.stringErrorMessage();
        c1.customErrorMessage();
    }
}

contract Contract0 {
    function stringErrorMessage() public {
        bool check = false;
        require(check, "error message");
    }
}

contract Contract1 {
    error CustomError();

    function customErrorMessage() public {
        bool check = false;
        if (!check) {
            revert CustomError();
        }
    }
}

POC Gas Report:

╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract0 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 34087              ┆ 200             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ stringErrorMessage ┆ 218             ┆ 218 ┆ 218    ┆ 218 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯
╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract1 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 26881              ┆ 164             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ customErrorMessage ┆ 161             ┆ 161 ┆ 161    ┆ 161 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯

Consider replacing all revert strings with custom errors in the solution.

core/GolomTrader.sol:177:        require(signaturesigner == o.signer, 'invalid signature');
core/GolomTrader.sol:211:        require(
core/GolomTrader.sol:217:        require(msg.value >= o.totalAmt * amount + p.paymentAmt, 'mgmtm');
core/GolomTrader.sol:220:            require(msg.sender == o.reservedAddress);
core/GolomTrader.sol:222:        require(o.orderType == 0, 'invalid orderType');
core/GolomTrader.sol:226:        require(status == 3, 'order not valid');
core/GolomTrader.sol:227:        require(amountRemaining >= amount, 'order already filled');
core/GolomTrader.sol:235:            require(amount == 1, 'only 1 erc721 at 1 time');
core/GolomTrader.sol:285:        require(
core/GolomTrader.sol:291:            require(msg.sender == o.reservedAddress);
core/GolomTrader.sol:293:        require(o.orderType == 1);
core/GolomTrader.sol:295:        require(status == 3);
core/GolomTrader.sol:296:        require(amountRemaining >= amount);
core/GolomTrader.sol:299:            require(amount == 1, 'only 1 erc721 at 1 time');
core/GolomTrader.sol:313:        require(o.signer == msg.sender);
core/GolomTrader.sol:342:        require(o.totalAmt >= o.exchange.paymentAmt + o.prePayment.paymentAmt + o.refererrAmt);
core/GolomTrader.sol:345:            require(msg.sender == o.reservedAddress);
core/GolomTrader.sol:347:        require(o.orderType == 2);
core/GolomTrader.sol:349:        require(status == 3);
core/GolomTrader.sol:350:        require(amountRemaining >= amount);
core/GolomTrader.sol:359:            require(amount == 1, 'only 1 erc721 at 1 time');
core/GolomTrader.sol:455:        require(distributorEnableDate <= block.timestamp, 'not allowed');
governance/GolomToken.sol:24:        require(msg.sender == minter, 'GolomToken: only reward distributor can enable');
governance/GolomToken.sol:43:        require(!isAirdropMinted, 'already minted');
governance/GolomToken.sol:51:        require(!isGenesisRewardMinted, 'already minted');
governance/GolomToken.sol:69:            require(minterEnableDate <= block.timestamp, 'GolomToken: wait for timelock');
rewards/RewardDistributor.sol:88:        require(msg.sender == trader);
rewards/RewardDistributor.sol:144:            require(epochs[index] < epoch);
rewards/RewardDistributor.sol:158: