LooksRare Aggregator contest - delfin454000's results

QA Report: low-risk

---

Unused/empty receive()/fallback() function

Leaving the receive() below without a revert could result in a loss of funds for a user who sends Ether to the contract (having no way to get anything back)

SeaportProxy.sol: L86

    receive() external payable {}

Similarly for the following:

LooksRareAggregator.sol: L220

---

---

Missing check for address(0x0) when assigning value to address state variable

Check for address(0x0) is missing for _aggregator:

LooksRareProxy.sol: L39

        aggregator = _aggregator;

Similarly for SeaportProxy.sol: L47

---

---

All contracts should use the same solidity pragma version

At the moment, most contracts use 0.8.17 (up-to-date version) but some are fixed to ^0.8.14 or ^0.8.0, as shown below

---

ReentrancyGuard.sol: L2

pragma solidity ^0.8.14;

The following also use pragma solidity ^0.8.14:

OwnableTwoSteps.sol: L2

SignatureChecker.sol: L2

LowLevelERC20Approve.sol: L2

LowLevelERC20Transfer.sol: L3

LowLevelERC721Transfer.sol: L4

LowLevelERC1155Transfer.sol: L2

LowLevelETH.sol: L2

---

IERC20.sol: L2

pragma solidity ^0.8.0;

The following also use pragma solidity ^0.8.0:

IERC721.sol: L2

IERC1155.sol: L2

---

---

QA Report: non-critical

Named return variables not used

---

Below, the named return variable (newCounter) is never used. In fact, function incrementCounter() never returns.

SeaportInterface.sol: L343-349

     * @notice Cancel all orders from a given offerer with a given zone in bulk
     *         by incrementing a counter. Note that only the offerer may
     *         increment the counter.
     *
     * @return newCounter The new counter.
     */
    function incrementCounter() external returns (uint256 newCounter);

---

Below, the named return variables (version, domainSeparator and conduitController) are never used. function information() never returns.

SeaportInterface.sol: L397-410

     * @notice Retrieve configuration information for this contract.
     *
     * @return version           The contract version.
     * @return domainSeparator   The domain separator for this contract.
     * @return conduitController The conduit Controller set for this contract.
     */
    function information()
        external
        view
        returns (
            string memory version,
            bytes32 domainSeparator,
            address conduitController
        );

---

Below, the named return variable (contractName) is never used. function name() never returns.

SeaportInterface.sol: L413-417

     * @notice Retrieve the name of this contract.
     *
     * @return contractName The name of this contract.
     */
    function name() external view returns (string memory contractName);

---

---

Extra-long single-line comments

In theory, comments over 80 characters should wrap using multi-line comment syntax. Even if longer comments (up to 120 characters) are acceptable and a scroll bar is provided, very long comments can interfere with readability.

Some of the long comments in LooksRare Aggregator do wrap but the treatment of long comments is inconsistent. Also, long spaces within some comments elongate them unnecessarily. Below, I divide the long comments in the contest into two types: NatSpec statements and lines that include both code and comments

---

Type 1: Long NatSpec statements

---

OwnableTwoSteps.sol: L8-9

 * @notice This contract offers transfer of ownership in two steps with potential owner having to confirm the transaction.
 *         Renouncement of the ownership is also a two-step process with a timelock since the next potential owner is address(0).

Suggestion:

 * @notice This contract offers transfer of ownership in two steps with potential owner 
 *     having to confirm the transaction. Renouncement of the ownership is also a
 *     two-step process with a timelock since the next potential owner is address(0).

---

SignatureChecker.sol: L9

 * @notice This contract is used to verify signatures for EOAs (with length of both 65 and 64 bytes) and contracts (ERC-1271).

Suggestion:

 * @notice This contract is used to verify signatures for EOAs (with length of  
 *     both 65 and 64 bytes) and contracts (ERC-1271).

---

LowLevelETH.sol: L52

     * @notice Return ETH to the original sender if any is left in the payable call but leave 1 wei of ETH in the contract.

Suggestion:

     * @notice Return ETH to the original sender if any is left in the  
     *     payable call but leave 1 wei of ETH in the contract.

---

Type 2: Extra-long lines that include both code and comments

If a line containing a comment is longer than 120 characters, it makes sense to put the comment in the line above, as shown

---

SeaportProxy.sol: L27

        FulfillmentComponent[][] considerationFulfillments; // Contains the order and item index of each consideration item

Suggestion:

        // Contains the order and item index of each consideration item
        FulfillmentComponent[][] considerationFulfillments; 

---

SeaportProxy.sol: L35

        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

Suggestion:

        // id -> Reference ID for a credit line provided by a single Lender for a given token on a Line of Credit
        bytes32 zoneHash;

---

SeaportProxy.sol: L37

        bytes32 conduitKey; // A bytes32 value that indicates what conduit, if any, should be utilized as a source for token approvals when performing transfers

Suggestion:

        // A bytes32 value that indicates what conduit, if any, should be utilized as
        //     a source for token approvals when performing transfers.
        bytes32 conduitKey; 

---

---

Duplicate import

import {LooksRareProxy} occurs twice in LooksRareAggregator.sol, thereby reducing the readability of the code

LooksRareAggregator.sol: L4-17

import {ReentrancyGuard} from "./ReentrancyGuard.sol";
import {LowLevelERC20Approve} from "./lowLevelCallers/LowLevelERC20Approve.sol";
import {LowLevelERC721Transfer} from "./lowLevelCallers/LowLevelERC721Transfer.sol";
import {LowLevelERC1155Transfer} from "./lowLevelCallers/LowLevelERC1155Transfer.sol";
import {IERC20} from "./interfaces/IERC20.sol";
import {LooksRareProxy} from "./proxies/LooksRareProxy.sol";
import {BasicOrder, TokenTransfer} from "./libraries/OrderStructs.sol";
import {TokenRescuer} from "./TokenRescuer.sol";
import {TokenReceiver} from "./TokenReceiver.sol";
import {ILooksRareAggregator} from "./interfaces/ILooksRareAggregator.sol";
import {BasicOrder, FeeData, TokenTransfer} from "./libraries/OrderStructs.sol";
import {LooksRareProxy} from "./proxies/LooksRareProxy.sol";
import {TokenReceiver} from "./TokenReceiver.sol";
import {TokenRescuer} from "./TokenRescuer.sol";

---

---

Event is missing indexed fields

Each event should use three indexed fields if there are three or more fields

---

ILooksRareAggregator.sol: L44

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

---

IERC20.sol: L5

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

---

IERC20.sol: L7

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

---

IERC721.sol: L9

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

---

IERC1155.sol: L15

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

---

---

NatSpec statement missing for function

Individual NatSpec statements are missing for the following external functions (NatSpec is not required for internal functions)

---

OrderStructs.sol: L6-17

    /**
     * @notice Execute LooksRare NFT sweeps in a single transaction
     * @dev extraData, feeBp and feeRecipient are not used
     * @param orders Orders to be executed by LooksRare
     * @param ordersExtraData Extra data for each order
     * @param recipient The address to receive the purchased NFTs
     * @param isAtomic Flag to enable atomic trades (all or nothing) or partial trades
     */
    function execute(
        BasicOrder[] calldata orders,
        bytes[] calldata ordersExtraData,
        bytes memory,
        address recipient,
        bool isAtomic,
        uint256,
        address
    ) external payable override {

Missing: @param memory

---

LooksRareAggregator.sol: L148-157

    /**
     * @param proxy Proxy to apply the fee to
     * @param bp Fee basis point
     * @param recipient Fee recipient
     */
    function setFee(
        address proxy,
        uint256 bp,
        address recipient
    ) external onlyOwner {

Missing: @notice

---

LooksRareAggregator.sol: L179-184

    /**
     * @param proxy The marketplace proxy's address
     * @param selector The marketplace proxy's function selector
     * @return Whether the marketplace proxy's function can be called from the aggregator
     */
    function supportsProxyFunction(address proxy, bytes4 selector) external view returns (bool) {

Missing: @notice

---

---

NatSpec statement missing for constructor

The constructor below is missing an @notice statement (a statement which would explain its purpose)

LooksRareProxy.sol: L33-37

    /**
     * @param _marketplace LooksRareExchange's address
     * @param _aggregator LooksRareAggregator's address
     */
    constructor(address _marketplace, address _aggregator) {

Similarly for the following constructors:

SeaportProxy.sol: L41-45

ERC20EnabledLooksRareAggregator.sol: L18-21

---

---

NatSpec statement missing for struct

The struct below is missing an @dev statement and associated explanations such as are given for other structsin the contest (see, for example, ConsiderationStructs.sol: L13-34)

OrderStructs.sol: L6-17

struct BasicOrder {
  address signer; // The order's maker
  address collection; // The address of the ERC721/ERC1155 token to be purchased
  CollectionType collectionType; // 0 for ERC721, 1 for ERC1155
  uint256[] tokenIds; // The IDs of the tokens to be purchased
  uint256[] amounts; // Always 1 when ERC721, can be > 1 if ERC1155
  uint256 price; // The *taker bid* price to pay for the order
  address currency; // The order's currency, address(0) for ETH
  uint256 startTime; // The timestamp when the order starts becoming valid
  uint256 endTime; // The timestamp when the order stops becoming valid
  bytes signature; // split to v,r,s for LooksRare
}

Similarly for the following structs:

OrderStructs.sol: L19-22

OrderStructs.sol: L24-27

SeaportProxy.sol: L25-28

SeaportProxy.sol: L30-39

---

---