Infinity NFT Marketplace contest - 0xkatana's results

The world's most advanced NFT marketplace.

General Information

Platform: Code4rena

Start Date: 14/06/2022

Pot Size: $50,000 USDC

Total HM: 19

Participants: 99

Period: 5 days

Judge: HardlyDifficult

Total Solo HM: 4

Id: 136

League: ETH

Infinity NFT Marketplace

Findings Distribution

Researcher Performance

Rank: 90/99

Findings: 1

Award: $33.03

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest Page Github Contest Repository Github Findings Repository Infinity NFT Marketplace

Findings Information

🌟 Selected for report: IllIllI

Also found by: 0v3rf10w, 0x1f8b, 0x29A, 0xAsm0d3us, 0xDjango, 0xKitsune, 0xNazgul, 0xf15ers, 0xkatana, 0xkowloon, BowTiedWardens, Chom, ElKu, FSchmoede, Funen, GimelSec, Kaiziron, Kenshin, Lambda, MadWookie, MiloTruck, PPrieditis, Picodes, PwnedNoMore, StErMi, Tadashi, TerrierLover, TomJ, Tomio, Wayne, Waze, _Adam, antonttc, apostle0x01, asutorufos, c3phas, codexploder, defsec, delfin454000, fatherOfBlocks, hake, hansfriese, hyh, joestakey, k, kenta, oyc_109, peritoflores, reassor, rfa, robee, sach1r0, simon135, slywaters, zer0dot

Awards

33.0288 USDC - $33.03

Labels

bug

G (Gas Optimization)

External Links

Link to GitHub issue

[G-01] Redundant zero initialization

Solidity does not recognize null as a value, so uint variables are initialized to zero. Setting a uint variable to zero is redundant and can waste gas.

Recommended Mitigation Steps

Remove the redundant zero initialization uint256 i; instead of uint256 i = 0;

[G-02] Split up require statements instead of &&

Combining require statement conditions with && logic uses unnecessary gas. It is better to split up each part of the logical statement into a separate require statements

Locations where this was found include https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/core/InfinityExchange.sol#L264 https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/core/InfinityExchange.sol#L949

Recommended Mitigation Steps

Use separate require statements instead of concatenating with &&

[G-03] Use `abi.encodePacked()` not `abi.encode()`

Changing abi.encode to abi.encodePacked can save gas. abi.encode pads extra null bytes at the end of the call data which is normally unnecessary. In general, abi.encodePacked is more gas-efficient.

Locations where this was found include https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/core/InfinityExchange.sol#L107 https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/core/InfinityExchange.sol#L1173 https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/core/InfinityExchange.sol#L1191 https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/core/InfinityExchange.sol#L1207

Recommended Mitigation Steps

Change abi.encode to abi.encodePacked

[G-04] Use != 0 instead of > 0

Using > 0 uses slightly more gas than using != 0. Use != 0 when comparing uint variables to zero, which cannot hold values below zero

Locations where this was found include https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/core/InfinityExchange.sol#L392

Recommended Mitigation Steps

Replace > 0 with != 0 to save gas

[G-05] Short require strings save gas

Strings in solidity are handled in 32 byte chunks. A require string longer than 32 bytes uses more gas. Shortening these strings will save gas.

Locations where this was found include https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/core/InfinityExchange.sol#L395 https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/staking/InfinityStaker.sol#L96

Recommended Mitigation Steps

Shorten all require strings to less than 32 characters

[G-06] For loop incrementing can be unsafe

For loops that use i++ do not need to use safemath for this operation because the loop would run out of gas long before this point. Making this addition operation unsafe using unchecked saves gas.

Sample code to make the for loop increment unsafe

for (uint i = 0; i < length; i = unchecked_inc(i)) {
    // do something that doesn't change the value of i
}

function unchecked_inc(uint i) returns (uint) {
    unchecked {
        return i + 1;
    }
}

Idea borrowed from https://gist.github.com/hrkrshnn/ee8fabd532058307229d65dcd5836ddc#the-increment-in-for-loop-post-condition-can-be-made-unchecked

Recommended Mitigation Steps

Make the increment in for loops unsafe to save gas

[G-07] Use iszero assembly for zero checks

Comparing a value to zero can be done using the iszero EVM opcode. This can save gas

Source from t11s https://twitter.com/transmissions11/status/1474465495243898885

Locations where this was found include https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/staking/InfinityStaker.sol#L104 https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/staking/InfinityStaker.sol#L264 https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/core/InfinityOrderBookComplication.sol#L240 https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/core/InfinityOrderBookComplication.sol#L286 https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/core/InfinityOrderBookComplication.sol#L334 https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/core/InfinityExchange.sol#L1157

Recommended Mitigation Steps

Use the assembly iszero evm opcode to compare values to zero

[G-08] Save gas with unchecked

Use unchecked math when there is no overflow risk to save gas. Before index is decreased in remove it is checked for zero condition. This means index will not underflow and can be unchecked.

Locations where this was found include https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/staking/InfinityStaker.sol#L268 https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/staking/InfinityStaker.sol#L301 https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/staking/InfinityStaker.sol#L305 https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/staking/InfinityStaker.sol#L309

Recommended Mitigation Steps

Add unchecked around math that can't overflow for gas savings. In Solidity before 0.8.0, use the normal math operators instead of safe math functions.

[G-09] Add payable to functions that won't receive ETH

Identifying a function as payable saves gas. Functions that have a modifier like onlyOwner cannot be called by normal users and will not mistakenly receive ETH. These functions can be payable to save gas.

Recommended Mitigation Steps

Add payable to these functions for gas savings

[G-10] Add payable to constructors that won't receive ETH

Identifying a constructor as payable saves gas. Constructors should only be called by the admin or deployer and should not mistakenly receive ETH. Constructors can be payable to save gas.

Locations where this was found include https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/staking/InfinityStaker.sol#L49 https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/token/TimelockConfig.sol#L43 https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/token/InfinityToken.sol#L37 https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/core/InfinityExchange.sol#L104

Recommended Mitigation Steps

Add payable to these functions for gas savings

[G-11] Use internal function in place of modifier

An internal function can save gas vs. a modifier. A modifier inlines the code of the original function but an internal function does not.

Source https://blog.polymath.network/solidity-tips-and-tricks-to-save-gas-and-reduce-bytecode-size-c44580b218e6#dde7

Locations where this was found include https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/token/TimelockConfig.sol#L38

Recommended Mitigation Steps

Use internal functions in place of modifiers to save gas.

[G-12] Use uint not bool

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.

Recommended Mitigation Steps

Replace bool variables with uints

[G-13] Use uint256 not smaller ints

From the solidity docs

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

Recommended Mitigation Steps

Replace bool variables with uints

[G-14] Use Solidity errors instead of require

Solidity errors introduced in version 0.8.4 can save gas on revert conditions https://blog.soliditylang.org/2021/04/21/custom-errors/ https://twitter.com/PatrickAlphaC/status/1505197417884528640

Recommended Mitigation Steps

Replace require blocks with new solidity errors described in https://blog.soliditylang.org/2021/04/21/custom-errors/

[G-15] Use simple comparison in trinary logic

The comparison operators >= and <= use more gas than >, <, or ==. Replacing the >= and ≤ operators with a comparison operator that has an opcode in the EVM saves gas

The existing code is https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/staking/InfinityStaker.sol#L270

return secondsSinceStake >= durationInSeconds ? amount : 0;

A simple comparison can be used for gas savings by reversing the logic

return secondsSinceStake < durationInSeconds ? 0 : amount;

Recommended Mitigation Steps

Replace the comparison operator and reverse the logic to save gas using the suggestions above

[G-16] Use simple comparison in logic

The comparison operators >= and <= use more gas than >, <, or ==. Replacing the >= and ≤ operators with a comparison operator that has an opcode in the EVM saves gas

The existing code is https://github.com/code-423n4/2022-06-infinity/tree/main/contracts/core/InfinityExchange.sol#L311-L312

bool isTimeValid = makerOrders[i].constraints[3] <= block.timestamp &&
    makerOrders[i].constraints[4] >= block.timestamp;

A simple comparison can be used for gas savings by reversing the logic

bool isTimeValid = ! (makerOrders[i].constraints[3] > block.timestamp ||
    makerOrders[i].constraints[4] < block.timestamp);

Recommended Mitigation Steps

Replace the comparison operator and modify the logic to save gas using the suggestions above

[G-17] Non-public variables save gas

Many constant variables are public, but changing the visibility of these variables to private or internal can save gas. The TimelockConfig constants especially can be private variables.

Recommended Mitigation Steps

Declare some public variables as private or internal to save gas

[G-18] Write contracts in vyper

The contracts are all written entirely in solidity. Writing contracts with vyper instead of solidity can save gas.

Source https://twitter.com/eiber_david/status/1515737811881807876 doggo demonstrates https://twitter.com/fubuloubu/status/1528179581974417414?t=-hcq_26JFDaHdAQZ-wYxCA&s=19

Recommended Mitigation Steps

Write some or all of the contracts in vyper to save gas

#0 - nneverlander
2022-06-23T11:59:43Z

Love the comment about vyper. We want to write this in vyper in our next iteration. If you are interested in helping out, please dm me on Twitter or Discord.

Infinity NFT Marketplace contest - 0xkatana's results

The world's most advanced NFT marketplace.

General Information

Findings Distribution

Researcher Performance

External Links

[G-08] Gas Report - $33.03

Findings Information

Awards

Labels

External Links

[G-01] Redundant zero initialization

Recommended Mitigation Steps

[G-02] Split up require statements instead of &&

Recommended Mitigation Steps

[G-03] Use abi.encodePacked() not abi.encode()

Recommended Mitigation Steps

[G-04] Use != 0 instead of > 0

Recommended Mitigation Steps

[G-05] Short require strings save gas

Recommended Mitigation Steps

[G-06] For loop incrementing can be unsafe

Recommended Mitigation Steps

[G-07] Use iszero assembly for zero checks

Recommended Mitigation Steps

[G-08] Save gas with unchecked

Recommended Mitigation Steps

[G-09] Add payable to functions that won't receive ETH

Recommended Mitigation Steps

[G-10] Add payable to constructors that won't receive ETH

Recommended Mitigation Steps

[G-11] Use internal function in place of modifier

Recommended Mitigation Steps

[G-12] Use uint not bool

Recommended Mitigation Steps

[G-13] Use uint256 not smaller ints

Recommended Mitigation Steps

[G-14] Use Solidity errors instead of require

Recommended Mitigation Steps

[G-15] Use simple comparison in trinary logic

Recommended Mitigation Steps

[G-16] Use simple comparison in logic

Recommended Mitigation Steps

[G-17] Non-public variables save gas

Recommended Mitigation Steps

[G-18] Write contracts in vyper

Recommended Mitigation Steps

#0 - nneverlander2022-06-23T11:59:43Z

[G-03] Use `abi.encodePacked()` not `abi.encode()`

#0 - nneverlander
2022-06-23T11:59:43Z