LooksRare Aggregator contest - IllIllI's results

Summary

Low Risk Issues

Total: 7 instances over 3 issues

Non-critical Issues

Total: 49 instances over 9 issues

Low Risk Issues

[L‑01] Unused/empty receive()/fallback() function

If the intention is for the Ether to be used, the function should call another function, otherwise it should revert (e.g. require(msg.sender == address(weth))). Having no access control on the function means that someone may send Ether to the contract, and have no way to get anything back out, which is a loss of funds

There are 2 instances of this issue:

File: contracts/LooksRareAggregator.sol

222:      receive() external payable {}

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/LooksRareAggregator.sol#L222

File: contracts/proxies/SeaportProxy.sol

86:       receive() external payable {}

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/proxies/SeaportProxy.sol#L86

[L‑02] Missing checks for address(0x0) when assigning values to address state variables

There are 4 instances of this issue:

File: contracts/LooksRareAggregator.sol

122:          erc20EnabledLooksRareAggregator = _erc20EnabledLooksRareAggregator;

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/LooksRareAggregator.sol#L122

File: contracts/OwnableTwoSteps.sol

102:          potentialOwner = newPotentialOwner;

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/OwnableTwoSteps.sol#L102

File: contracts/proxies/LooksRareProxy.sol

39:           aggregator = _aggregator;

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/proxies/LooksRareProxy.sol#L39

File: contracts/proxies/SeaportProxy.sol

47:           aggregator = _aggregator;

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/proxies/SeaportProxy.sol#L47

[L‑03] Library function is vulnerable to signature malleability attacks

Use OpenZeppelin's ECDSA contract rather than calling ecrecover() directly

There is 1 instance of this issue:

File: /contracts/SignatureChecker.sol

56       function _recoverEOASigner(bytes32 hash, bytes memory signature) internal pure returns (address signer) {
57           (bytes32 r, bytes32 s, uint8 v) = _splitSignature(signature);
58   
59           // If the signature is valid (and not malleable), return the signer address
60           signer = ecrecover(hash, v, r, s);
61   
62           if (signer == address(0)) revert NullSignerAddress();
63:      }

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/SignatureChecker.sol#L56-L63

Non-critical Issues

[N‑01] Duplicate import statements

There are 4 instances of this issue:

File: contracts/LooksRareAggregator.sol

14:   import {BasicOrder, FeeData, TokenTransfer} from "./libraries/OrderStructs.sol";

15:   import {LooksRareProxy} from "./proxies/LooksRareProxy.sol";

16:   import {TokenReceiver} from "./TokenReceiver.sol";

17:   import {TokenRescuer} from "./TokenRescuer.sol";

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/LooksRareAggregator.sol#L14

[N‑02] Contract implements interface without extending the interface

Not extending the interface may lead to the wrong function signature being used, leading to unexpected behavior. If the interface is in fact being implemented, use the override keyword to indicate that fact

There is 1 instance of this issue:

File: contracts/TokenReceiver.sol

/// @audit IERC1155Receiver.onERC1155Received(), IERC1155Receiver.onERC1155BatchReceived()
4:    abstract contract TokenReceiver {

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/TokenReceiver.sol#L4

[N‑03] constants should be defined rather than using magic numbers

Even assembly can benefit from using readable constants instead of hex/numeric literals

There are 15 instances of this issue:

File: contracts/LooksRareAggregator.sol

/// @audit 10000
158:          if (bp > 10000) revert FeeTooHigh();

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/LooksRareAggregator.sol#L158

File: contracts/proxies/SeaportProxy.sol

/// @audit 10000
147:              uint256 orderFee = (orders[i].price * feeBp) / 10000;

/// @audit 10000
207:                      uint256 orderFee = (price * feeBp) / 10000;

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/proxies/SeaportProxy.sol#L147

File: contracts/SignatureChecker.sol

/// @audit 64
28:           if (signature.length == 64) {

/// @audit 0x20
31:                   r := mload(add(signature, 0x20))

/// @audit 0x40
32:                   vs := mload(add(signature, 0x40))

/// @audit 0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
33:                   s := and(vs, 0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff)

/// @audit 65
36:           } else if (signature.length == 65) {

/// @audit 0x20
38:                   r := mload(add(signature, 0x20))

/// @audit 0x40
39:                   s := mload(add(signature, 0x40))

/// @audit 0x60
40:                   v := byte(0, mload(add(signature, 0x60)))

/// @audit 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0
46:           if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) revert BadSignatureS();

/// @audit 27
/// @audit 28
48:           if (v != 27 && v != 28) revert BadSignatureV(v);

/// @audit 0x1626ba7e
80:               if (IERC1271(signer).isValidSignature(hash, signature) != 0x1626ba7e) revert InvalidSignatureERC1271();

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/SignatureChecker.sol#L28

[N‑04] Constant redefined elsewhere

Consider defining in only one contract so that values cannot become out of sync when only one location is updated. A cheap way to store constants in a single location is to create an internal constant in a library. If the variable is a local cache of another contract's value, consider making the cache variable internal or private, which will require external users to query the contract with the source of truth, so that callers don't get out of sync.

There are 3 instances of this issue:

File: contracts/proxies/LooksRareProxy.sol

/// @audit seen in contracts/ERC20EnabledLooksRareAggregator.sol 
31:       address public immutable aggregator;

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/proxies/LooksRareProxy.sol#L31

File: contracts/proxies/SeaportProxy.sol

/// @audit seen in contracts/proxies/LooksRareProxy.sol 
20:       SeaportInterface public immutable marketplace;

/// @audit seen in contracts/proxies/LooksRareProxy.sol 
21:       address public immutable aggregator;

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/proxies/SeaportProxy.sol#L20

[N‑05] Lines are too long

Usually lines in source code are limited to 80 characters. Today's screens are much larger so it's reasonable to stretch this in some cases. Since the files will most likely reside in GitHub, and GitHub starts using a scroll bar in all cases when the length is over 164 characters, the lines below should be split when they reach that length

There are 2 instances of this issue:

File: contracts/proxies/SeaportProxy.sol

8:    import {AdvancedOrder, CriteriaResolver, OrderParameters, OfferItem, ConsiderationItem, FulfillmentComponent, AdditionalRecipient} from "../libraries/seaport/ConsiderationStructs.sol";

35:           bytes32 zoneHash; // An arbitrary 32-byte value that will be supplied to the zone when fulfilling restricted orders that the zone can utilize when making a determination on whether to authorize the order

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/proxies/SeaportProxy.sol#L8

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

There are 5 instances of this issue:

File: contracts/lowLevelCallers/LowLevelERC1155Transfer.sol

2:    pragma solidity ^0.8.14;

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/lowLevelCallers/LowLevelERC1155Transfer.sol#L2

File: contracts/lowLevelCallers/LowLevelERC20Approve.sol

2:    pragma solidity ^0.8.14;

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/lowLevelCallers/LowLevelERC20Approve.sol#L2

File: contracts/lowLevelCallers/LowLevelERC20Transfer.sol

4:    pragma solidity ^0.8.14;

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/lowLevelCallers/LowLevelERC20Transfer.sol#L4

File: contracts/lowLevelCallers/LowLevelERC721Transfer.sol

2:    pragma solidity ^0.8.14;

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/lowLevelCallers/LowLevelERC721Transfer.sol#L2

File: contracts/lowLevelCallers/LowLevelETH.sol

2:    pragma solidity ^0.8.14;

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/lowLevelCallers/LowLevelETH.sol#L2

[N‑07] File is missing NatSpec

There are 7 instances of this issue:

File: contracts/interfaces/IERC1155.sol

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/interfaces/IERC1155.sol

File: contracts/interfaces/IERC20.sol

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/interfaces/IERC20.sol

File: contracts/interfaces/IERC721.sol

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/interfaces/IERC721.sol

File: contracts/libraries/OrderEnums.sol

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/libraries/OrderEnums.sol

File: contracts/libraries/OrderStructs.sol

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/libraries/OrderStructs.sol

File: contracts/libraries/seaport/ConsiderationEnums.sol

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/libraries/seaport/ConsiderationEnums.sol

File: contracts/TokenReceiver.sol

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/TokenReceiver.sol

[N‑08] NatSpec is incomplete

There are 4 instances of this issue:

File: contracts/proxies/LooksRareProxy.sol

/// @audit Missing: '@param bytes'
/// @audit Missing: '@param uint256'
/// @audit Missing: '@param address'
42        /**
43         * @notice Execute LooksRare NFT sweeps in a single transaction
44         * @dev extraData, feeBp and feeRecipient are not used
45         * @param orders Orders to be executed by LooksRare
46         * @param ordersExtraData Extra data for each order
47         * @param recipient The address to receive the purchased NFTs
48         * @param isAtomic Flag to enable atomic trades (all or nothing) or partial trades
49         */
50        function execute(
51            BasicOrder[] calldata orders,
52            bytes[] calldata ordersExtraData,
53            bytes memory,
54            address recipient,
55            bool isAtomic,
56            uint256,
57:           address

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/proxies/LooksRareProxy.sol#L42-L57

File: contracts/SignatureChecker.sol

/// @audit Missing: '@return'
54         * @param signature Bytes containing the signature (64 or 65 bytes)
55         */
56:       function _recoverEOASigner(bytes32 hash, bytes memory signature) internal pure returns (address signer) {

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/SignatureChecker.sol#L54-L56

[N‑09] Event is missing indexed fields

Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (three fields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.

There are 8 instances of this issue:

File: contracts/interfaces/IERC1155.sol

15:       event ApprovalForAll(address indexed account, address indexed operator, bool approved);

17:       event URI(string value, uint256 indexed id);

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/interfaces/IERC1155.sol#L15

File: contracts/interfaces/IERC20.sol

5:        event Transfer(address indexed from, address indexed to, uint256 value);

7:        event Approval(address indexed owner, address indexed spender, uint256 value);

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/interfaces/IERC20.sol#L5

File: contracts/interfaces/IERC721.sol

9:        event ApprovalForAll(address indexed owner, address indexed operator, bool approved);

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/interfaces/IERC721.sol#L9

File: contracts/interfaces/ILooksRareAggregator.sol

44:       event FeeUpdated(address proxy, uint256 bp, address recipient);

51:       event FunctionAdded(address indexed proxy, bytes4 selector);

58:       event FunctionRemoved(address indexed proxy, bytes4 selector);

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/interfaces/ILooksRareAggregator.sol#L44

Summary

Gas Optimizations

Total: 29 instances over 6 issues with 1003 gas saved

Gas totals use lower bounds of ranges and count two iterations of each for-loop. All values above are runtime, not deployment, values; deployment values are listed in the individual issue descriptions. The table above as well as its gas numbers do not include any of the excluded findings.

Gas Optimizations

[G‑01] Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, where appropriate

Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations.

There is 1 instance of this issue:

File: contracts/LooksRareAggregator.sol

45        mapping(address => mapping(bytes4 => bool)) private _proxyFunctionSelectors;
46:       mapping(address => FeeData) private _proxyFeeData;

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/LooksRareAggregator.sol#L45-L46

diff --git a/contracts/LooksRareAggregator.sol b/contracts/LooksRareAggregator.sol
index f215f39..a5525aa 100644
--- a/contracts/LooksRareAggregator.sol
+++ b/contracts/LooksRareAggregator.sol
@@ -156,8 +156,10 @@ contract LooksRareAggregator is
         address recipient
     ) external onlyOwner {
         if (bp > 10000) revert FeeTooHigh();
-        _proxyFeeData[proxy].bp = bp;
-        _proxyFeeData[proxy].recipient = recipient;
+
+        FeeData storage pfd = _proxyFeeData[proxy];
+        pfd.bp = bp;
+        pfd.recipient = recipient;
 
         emit FeeUpdated(proxy, bp, recipient);
     }

Note that the numbers below are wrong due to this forge bug, where forge doesn't properly track warm/cold slots in tests

diff --git a/tmp/gas_before b/tmp/gas_after
index a3222f9..705e068 100644
--- a/tmp/gas_before
+++ b/tmp/gas_after
@@ -17 +17 @@
-│ 2504484                                                        ┆ 12343           ┆        ┆        ┆        ┆         │
+│ 2504891                                                        ┆ 12345           ┆        ┆        ┆        ┆         │
@@ -47 +47 @@
-│ setFee                                                         ┆ 2945            ┆ 43626  ┆ 47171  ┆ 49171  ┆ 21      │
+│ setFee                                                         ┆ 2945            ┆ 43631  ┆ 47176  ┆ 49176  ┆ 21      │
@@ -230 +230 @@
-│ 13089423                                                                    ┆ 64155           ┆     ┆        ┆     ┆         │
+│ 13089823                                                                    ┆ 64157           ┆     ┆        ┆     ┆         │
@@ -346,0 +347 @@
+Overall gas change: 4929 (0.099%)

[G‑02] Using calldata instead of memory for read-only arguments in external functions saves gas

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, obliviates the need for such a loop in the contract code and runtime execution. Note that even if an interface defines a function as having memory arguments, it's still valid for implementation contracs to use calldata arguments instead.

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 gass-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

Note that I've also flagged instances where the function is public but can be marked as external since it's not called by the contract, and cases where a constructor is involved

There are 3 instances of this issue:

File: contracts/proxies/LooksRareProxy.sol

50        function execute(
51            BasicOrder[] calldata orders,
52            bytes[] calldata ordersExtraData,
53            bytes memory,
54            address recipient,
55            bool isAtomic,
56            uint256,
57:           address

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/proxies/LooksRareProxy.sol#L50-L57

File: contracts/TokenReceiver.sol

14        function onERC1155Received(
15            address,
16            address,
17            uint256,
18            uint256,
19            bytes memory
20:       ) external virtual returns (bytes4) {

24        function onERC1155BatchReceived(
25            address,
26            address,
27            uint256[] memory,
28            uint256[] memory,
29            bytes memory
30:       ) external virtual returns (bytes4) {

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/TokenReceiver.sol#L14-L20

diff --git a/contracts/TokenReceiver.sol b/contracts/TokenReceiver.sol
index a82e815..759d4ac 100644
--- a/contracts/TokenReceiver.sol
+++ b/contracts/TokenReceiver.sol
@@ -16,7 +16,7 @@ abstract contract TokenReceiver {
         address,
         uint256,
         uint256,
-        bytes memory
+        bytes calldata
     ) external virtual returns (bytes4) {
         return this.onERC1155Received.selector;
     }
@@ -24,9 +24,9 @@ abstract contract TokenReceiver {
     function onERC1155BatchReceived(
         address,
         address,
-        uint256[] memory,
-        uint256[] memory,
-        bytes memory
+        uint256[] calldata,
+        uint256[] calldata,
+        bytes calldata
     ) external virtual returns (bytes4) {
         return this.onERC1155BatchReceived.selector;
     }
diff --git a/contracts/proxies/LooksRareProxy.sol b/contracts/proxies/LooksRareProxy.sol
index cbce118..1ebe936 100644
--- a/contracts/proxies/LooksRareProxy.sol
+++ b/contracts/proxies/LooksRareProxy.sol
@@ -50,7 +50,7 @@ contract LooksRareProxy is IProxy, TokenRescuer, TokenTransferrer, SignatureChec
     function execute(
         BasicOrder[] calldata orders,
         bytes[] calldata ordersExtraData,
-        bytes memory,
+        bytes calldata,
         address recipient,
         bool isAtomic,
         uint256,

diff --git a/tmp/gas_before b/tmp/gas_after
index a3222f9..94f3986 100644
--- a/tmp/gas_before
+++ b/tmp/gas_after
@@ -17 +17 @@
-│ 2504484                                                        ┆ 12343           ┆        ┆        ┆        ┆         │
+│ 2420786                                                        ┆ 11925           ┆        ┆        ┆        ┆         │
@@ -27 +27 @@
-│ execute                                                        ┆ 1370            ┆ 434636 ┆ 429633 ┆ 960082 ┆ 67      │
+│ execute                                                        ┆ 1370            ┆ 434530 ┆ 429633 ┆ 960082 ┆ 67      │
@@ -31 +31 @@
-│ onERC1155Received                                              ┆ 1631            ┆ 1631   ┆ 1631   ┆ 1631   ┆ 4       │
+│ onERC1155Received                                              ┆ 1305            ┆ 1305   ┆ 1305   ┆ 1305   ┆ 4       │
@@ -56 +56 @@
-│ 1402373                                                    ┆ 6944            ┆        ┆        ┆        ┆         │
+│ 1321684                                                    ┆ 6541            ┆        ┆        ┆        ┆         │
@@ -62 +62 @@
-│ execute                                                    ┆ 268230          ┆ 373671 ┆ 361477 ┆ 503500 ┆ 4       │
+│ execute                                                    ┆ 268230          ┆ 373543 ┆ 361349 ┆ 503244 ┆ 4       │
@@ -71 +71 @@
-│ 1683078                                                      ┆ 8635            ┆        ┆        ┆        ┆         │
+│ 1699292                                                      ┆ 8716            ┆        ┆        ┆        ┆         │
@@ -75 +75 @@
-│ execute                                                      ┆ 1962            ┆ 260929 ┆ 279156 ┆ 506876 ┆ 23      │
+│ execute                                                      ┆ 1640            ┆ 260616 ┆ 278900 ┆ 506620 ┆ 23      │
@@ -230 +230 @@
-│ 13089423                                                                    ┆ 64155           ┆     ┆        ┆     ┆         │
+│ 12924886                                                                    ┆ 63334           ┆     ┆        ┆     ┆         │
@@ -288 +288 @@
-│ mint                                                    ┆ 26410           ┆ 28497 ┆ 26410  ┆ 31628 ┆ 5       │
+│ mint                                                    ┆ 26410           ┆ 28366 ┆ 26410  ┆ 31302 ┆ 5       │
@@ -346,0 +347 @@
+Overall gas change: -1174338 (-24.848%)

[G‑03] State variables should be cached in stack variables rather than re-reading them from storage

The instances below point to the second+ access of a state variable within a function. Caching of a state variable replaces each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.

There are 2 instances of this issue:

File: contracts/OwnableTwoSteps.sol

/// @audit owner on line 87
91:           emit NewOwner(owner);

/// @audit earliestOwnershipRenouncementTime on line 114
116:          emit InitiateOwnershipRenouncement(earliestOwnershipRenouncementTime);

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/OwnableTwoSteps.sol#L91

diff --git a/contracts/OwnableTwoSteps.sol b/contracts/OwnableTwoSteps.sol
index 99d8a7d..366bd03 100644
--- a/contracts/OwnableTwoSteps.sol
+++ b/contracts/OwnableTwoSteps.sol
@@ -84,11 +84,10 @@ abstract contract OwnableTwoSteps is IOwnableTwoSteps {
         if (ownershipStatus != Status.TransferInProgress) revert TransferNotInProgress();
         if (msg.sender != potentialOwner) revert WrongPotentialOwner();
 
-        owner = msg.sender;
         delete ownershipStatus;
         delete potentialOwner;
 
-        emit NewOwner(owner);
+        emit NewOwner(owner = msg.sender);
     }
 
     /**
@@ -111,9 +110,8 @@ abstract contract OwnableTwoSteps is IOwnableTwoSteps {
         if (ownershipStatus != Status.NoOngoingTransfer) revert TransferAlreadyInProgress();
 
         ownershipStatus = Status.RenouncementInProgress;
-        earliestOwnershipRenouncementTime = block.timestamp + delay;
 
-        emit InitiateOwnershipRenouncement(earliestOwnershipRenouncementTime);
+        emit InitiateOwnershipRenouncement(earliestOwnershipRenouncementTime = block.timestamp + delay);
     }
 
     /**

Note that the numbers below are wrong due to this forge bug, where forge doesn't properly track warm/cold slots in tests

diff --git a/tmp/gas_before b/tmp/gas_after
index a3222f9..f393f3c 100644
--- a/tmp/gas_before
+++ b/tmp/gas_after
@@ -174 +174 @@
-│ confirmOwnershipTransfer                                               ┆ 507             ┆ 2329  ┆ 2379   ┆ 4101  ┆ 3       │
+│ confirmOwnershipTransfer                                               ┆ 507             ┆ 2328  ┆ 2379   ┆ 4099  ┆ 3       │
@@ -180 +180 @@
-│ initiateOwnershipRenouncement                                          ┆ 529             ┆ 27941 ┆ 46039  ┆ 50039 ┆ 7       │
+│ initiateOwnershipRenouncement                                          ┆ 529             ┆ 27942 ┆ 46042  ┆ 50042 ┆ 7       │
@@ -346,0 +347 @@
+Overall gas change: 7 (0.008%)

[G‑04] Optimize names to save gas

public/external function names and public member variable names can be optimized to save gas. See this link for an example of how it works. Below are the interfaces/abstract contracts that can be optimized so that the most frequently-called functions use the least amount of gas possible during method lookup. Method IDs that have two leading zero bytes can save 128 gas each during deployment, and renaming functions to have lower method IDs will save 22 gas per call, per sorted position shifted

There are 8 instances of this issue:

File: contracts/ERC20EnabledLooksRareAggregator.sol

/// @audit execute()
15:   contract ERC20EnabledLooksRareAggregator is IERC20EnabledLooksRareAggregator, LowLevelERC20Transfer {

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/ERC20EnabledLooksRareAggregator.sol#L15

File: contracts/interfaces/IERC20EnabledLooksRareAggregator.sol

/// @audit execute()
7:    interface IERC20EnabledLooksRareAggregator {

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/interfaces/IERC20EnabledLooksRareAggregator.sol#L7

File: contracts/interfaces/ILooksRareAggregator.sol

/// @audit execute()
6:    interface ILooksRareAggregator {

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/interfaces/ILooksRareAggregator.sol#L6

File: contracts/interfaces/IProxy.sol

/// @audit execute()
6:    interface IProxy {

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/interfaces/IProxy.sol#L6

File: contracts/interfaces/SeaportInterface.sol

/// @audit fulfillBasicOrder(), fulfillOrder(), fulfillAdvancedOrder(), fulfillAvailableOrders(), fulfillAvailableAdvancedOrders(), matchOrders(), matchAdvancedOrders(), cancel(), validate(), incrementCounter(), getOrderHash(), getOrderStatus(), getCounter(), information()
28:   interface SeaportInterface {

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/interfaces/SeaportInterface.sol#L28

File: contracts/LooksRareAggregator.sol

/// @audit execute(), setERC20EnabledLooksRareAggregator(), addFunction(), removeFunction(), setFee(), approve(), supportsProxyFunction(), rescueERC721(), rescueERC1155()
25:   contract LooksRareAggregator is

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/LooksRareAggregator.sol#L25

File: contracts/OwnableTwoSteps.sol

/// @audit cancelOwnershipTransfer(), confirmOwnershipRenouncement(), confirmOwnershipTransfer(), initiateOwnershipTransfer(), initiateOwnershipRenouncement()
11:   abstract contract OwnableTwoSteps is IOwnableTwoSteps {

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/OwnableTwoSteps.sol#L11

File: contracts/TokenRescuer.sol

/// @audit rescueETH(), rescueERC20()
14:   contract TokenRescuer is OwnableTwoSteps, LowLevelETH, LowLevelERC20Transfer {

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/TokenRescuer.sol#L14

[G‑05] internal functions not called by the contract should be removed to save deployment gas

If the functions are required by an interface, the contract should inherit from that interface and use the override keyword

There are 2 instances of this issue:

File: contracts/lowLevelCallers/LowLevelERC1155Transfer.sol

23        function _executeERC1155SafeTransferFrom(
24            address collection,
25            address from,
26            address to,
27            uint256 tokenId,
28:           uint256 amount

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/lowLevelCallers/LowLevelERC1155Transfer.sol#L23-L28

File: contracts/lowLevelCallers/LowLevelETH.sol

54        function _returnETHIfAnyWithOneWeiLeft() internal {
55:           assembly {

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/lowLevelCallers/LowLevelETH.sol#L54-L55

[G‑06] Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2),DUP1(3),ISZERO(3),PUSH2(3),JUMPI(10),PUSH1(3),DUP1(3),REVERT(0),JUMPDEST(1),POP(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost

There are 13 instances of this issue:

File: contracts/LooksRareAggregator.sol

120:      function setERC20EnabledLooksRareAggregator(address _erc20EnabledLooksRareAggregator) external onlyOwner {

132:      function addFunction(address proxy, bytes4 selector) external onlyOwner {

143:      function removeFunction(address proxy, bytes4 selector) external onlyOwner {

153       function setFee(
154           address proxy,
155           uint256 bp,
156           address recipient
157:      ) external onlyOwner {

173       function approve(
174           address marketplace,
175           address currency,
176           uint256 amount
177:      ) external onlyOwner {

197       function rescueERC721(
198           address collection,
199           address to,
200           uint256 tokenId
201:      ) external onlyOwner {

213       function rescueERC1155(
214           address collection,
215           address to,
216           uint256[] calldata tokenIds,
217           uint256[] calldata amounts
218:      ) external onlyOwner {

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/LooksRareAggregator.sol#L120

File: contracts/OwnableTwoSteps.sol

51:       function cancelOwnershipTransfer() external onlyOwner {

68:       function confirmOwnershipRenouncement() external onlyOwner {

98:       function initiateOwnershipTransfer(address newPotentialOwner) external onlyOwner {

110:      function initiateOwnershipRenouncement() external onlyOwner {

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/OwnableTwoSteps.sol#L51

File: contracts/TokenRescuer.sol

22:       function rescueETH(address to) external onlyOwner {

34:       function rescueERC20(address currency, address to) external onlyOwner {

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/TokenRescuer.sol#L22

---

Excluded findings

These findings are excluded from awards calculations because there are publicly-available automated tools that find them. The valid ones appear here for completeness

Summary

Gas Optimizations

Total: 3 instances over 2 issues with 17106 gas saved

Gas totals use lower bounds of ranges and count two iterations of each for-loop. All values above are runtime, not deployment, values; deployment values are listed in the individual issue descriptions. The table above as well as its gas numbers do not include any of the excluded findings.

Gas Optimizations

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

The overheads outlined below are PER LOOP, excluding the first loop

• storage arrays incur a Gwarmaccess (100 gas)
• memory arrays use MLOAD (3 gas)
• calldata arrays use CALLDATALOAD (3 gas)

Caching the length changes each of these to a DUP<N> (3 gas), and gets rid of the extra DUP<N> needed to store the stack offset

There are 2 instances of this issue:

File: contracts/proxies/SeaportProxy.sol

/// @audit (valid but excluded finding)
126:          for (uint256 i; i < availableOrders.length; ) {

/// @audit (valid but excluded finding)
187:          for (uint256 i; i < orders.length; ) {

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/proxies/SeaportProxy.sol#L126

[G‑02] Using bools 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.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27 Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas) for the extra SLOAD, and to avoid Gsset (20000 gas) when changing from false to true, after having been true in the past

There is 1 instance of this issue:

File: contracts/LooksRareAggregator.sol

/// @audit (valid but excluded finding)
45:       mapping(address => mapping(bytes4 => bool)) private _proxyFunctionSelectors;

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/LooksRareAggregator.sol#L45