Platform: Code4rena
Start Date: 20/05/2022
Pot Size: $1,000,000 USDC
Total HM: 4
Participants: 59
Period: 14 days
Judge: leastwood
Id: 128
League: ETH
Rank: 27/59
Findings: 2
Award: $2,326.75
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: Spearbit
Also found by: 0xsanson, Chom, IllIllI, OriDabush, Saw-mon_and_Natalie, broccoli, cccz, cmichel, csanuragjain, foobar, hack3r-0m, hickuphh3, hubble, hyh, ilan, kebabsec, mayo, oyc_109, peritoflores, rfa, scaraven, sces60107, shung, sorrynotsorry, tintin, twojoy, zkhorse, zzzitron
1892.3972 USDC - $1,892.40
Some of the contracts are using the floating pragma of >=0.8.7 and others use 0.8.13 Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively. It is also recommended to use the same solidity version for all the contracts.
References: https://swcregistry.io/docs/SWC-103 https://github.com/crytic/slither/wiki/Detector-Documentation#different-pragma-directives-are-used
Each event can have up to 3 indexed fields.
event NewConduit(address conduit, bytes32 conduitKey); //@audit address conduit can be made indexed
event ChannelUpdated(address channel, bool open); //@audit address channel can be made indexed
event OrderFulfilled( bytes32 orderHash, address indexed offerer, address indexed zone, address fulfiller, //@audit address fulfiller can be made indexed SpentItem[] offer, ReceivedItem[] consideration );
#0 - 0age
2022-06-03T04:38:48Z
need pragma for reference contracts, indexed events are more expensive and these values are not going to be used repeatedly
#1 - HardlyDifficult
2022-07-06T15:41:01Z
Floating compiler versions
Their config pins the version used, so this is a minor style preference. I often using floating personally so that when I upgrade versions the diff is just the config and therefore easy to review.
event is missing indexed fields
There is a cost to using indexed, so it should only be added where it will add value. It's not indicated which fields would be appropriate to index here.
Since none of these are clear wins, closing as invalid.
#2 - GalloDaSballo
2022-07-17T18:21:04Z
Agree with Judge and Sponsor, but would consider indexing the Conduit on creation as that could be arguably used to then list to future events on it.
Because of that I think 1 NC is valid from this report
#3 - GalloDaSballo
2022-07-19T22:14:36Z
2 NC per updated judging on #67
🌟 Selected for report: Dravee
Also found by: 0x1f8b, 0x29A, 0xalpharush, Chom, Czar102, Hawkeye, IllIllI, MaratCerby, MiloTruck, NoamYakov, OriDabush, RoiEvenHaim, Spearbit, Tadashi, TerrierLover, TomJ, asutorufos, cccz, cmichel, csanuragjain, defsec, delfin454000, djxploit, ellahi, foobar, gzeon, hake, hickuphh3, ignacio, ilan, joestakey, kaden, mayo, ming, oyc_109, peritoflores, rfa, sach1r0, sashik_eth, shung, sirhashalot, twojoy, zer0dot, zkhorse
434.3522 USDC - $434.35
Some variables are initialised with their default values which cause unnecessary gas consumption
https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/BasicOrderFulfiller.sol#L948 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/BasicOrderFulfiller.sol#L1040 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/conduit/Conduit.sol#L66 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/conduit/Conduit.sol#L130 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/CriteriaResolution.sol#L56 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/CriteriaResolution.sol#L166 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/CriteriaResolution.sol#L184 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/CriteriaResolution.sol#L199 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderCombiner.sol#L181 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderCombiner.sol#L247 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderCombiner.sol#L291 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderCombiner.sol#L373 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderCombiner.sol#L470 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderCombiner.sol#L473 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderCombiner.sol#L498 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderCombiner.sol#L577 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderCombiner.sol#L598 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderCombiner.sol#L621 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderCombiner.sol#L751 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderCombiner.sol#L754 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderFulfiller.sol#L217 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderFulfiller.sol#L306 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderFulfiller.sol#L471 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderValidator.sol#L272 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderValidator.sol#L350 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/AmountDeriver.sol#L44
https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderCombiner.sol#L229
Some of the internal functions are never called from the inherited contracts. Hence, their visibility can be changed to private.
https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/BasicOrderFulfiller.sol#L940 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/BasicOrderFulfiller.sol#L1021 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/conduit/Conduit.sol#L174 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/conduit/ConduitController.sol#L480 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/conduit/ConduitController.sol#L496 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/ConsiderationBase.sol#L131 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/CriteriaResolution.sol#L223 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/CriteriaResolution.sol#L245 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/Executor.sol#L413 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/Executor.sol#L456 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/Executor.sol#L502 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/Executor.sol#L558 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/Executor.sol#L597 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/FulfillmentApplier.sol#L202 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/FulfillmentApplier.sol#L497 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderCombiner.sol#L163 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderCombiner.sol#L451 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderCombiner.sol#L567 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderCombiner.sol#L741 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderFulfiller.sol#L161 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderValidator.sol#L451 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/SignatureVerification.sol#L120 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/ZoneInteraction.sol#L61 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/ZoneInteraction.sol#L160
It is cheaper to use calldata for the read-only arguments of external functions because then intermediate memory operations are avoided.
https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/Consideration.sol#L301 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/Consideration.sol#L385
It is more expensive to operate using booleans because before every write operation an SLOAD is executed to read the contents of the slot. Therefore, it is cheaper to use uint256 instead of bool. On the other hand, using bool is better for the code readability. Hence, it is a tradeoff to be decided by the developers. For example; _channels mapping in Conduit.sol: https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/conduit/Conduit.sol#L33
mapping(address => bool) private _channels;
can be changed to: mapping(address => uint) private _channels;
Then updateChannel() can set _channels[channel] = isOpen; //@audit isOpen can be uint with possible values of 1 and 2 Where isOpen can take values 1 and 2 for open and closed. This way more gas can be saved because SSTORE from 0 to a nonzero value costs 20000 gas, whereas SSTORE from nonzero to nonzero costs 5000 gas.
https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/conduit/Conduit.sol#L33 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/conduit/Conduit.sol#L163
#0 - HardlyDifficult
2022-06-23T21:23:25Z
Redundant initialisation with default value The optimizer should take care of this automatically.
Prefix increment/decrements are cheaper than postfix increment/decrements True, small savings
private functions are cheaper than internal functions. Private vs internal improves code readability but shouldn't impact gas costs (unlike public vs internal for example).
memory arguments can be changed to calldata True, however they appeared to have intentionally used memory here in order to encourage code reuse - e.g. https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/Consideration.sol#L230 where they are generating the array.