LooksRare Aggregator contest - ReyAdmirado's results

QA

1. use of floating pragma

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.

• ReentrancyGuard.sol#L
• OwnableTwoSteps.sol#L
• SignatureChecker.sol#L
• LowLevelETH.sol#L
• LowLevelERC20Approve.sol#L
• LowLevelERC20Transfer.sol#L
• LowLevelERC721Transfer.sol#L
• LowLevelERC1155Transfer.sol#L

2. 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.

• IERC20.sol#L5

• IERC20.sol#L7

• ILooksRareAggregator.sol#L44

3. lines are too long

Usually lines in source code are limited to 80 characters. Its advised to keep lines lower than 120 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

• SeaportProxy.sol#L35
• SeaportProxy.sol#L37

4. Scoped contracts are missing proper NatSpec comments

The scoped contracts are missing proper NatSpec comments such as @notice @dev @param on many places. It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI)

• SeaportProxy.sol#L

• ERC20EnabledLooksRareAggregator.sol#L

5. 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

• SeaportProxy.sol#L86

• LooksRareAggregator.sol#L220

6. using different Solidity Pragma in different contracts

It is better to use one Solidity compiler version across all contracts instead of different versions with different bugs and security checks.

• ReentrancyGuard.sol#L
• OwnableTwoSteps.sol#L
• SignatureChecker.sol#L
• LowLevelETH.sol#L
• LowLevelERC20Approve.sol#L
• LowLevelERC20Transfer.sol#L
• LowLevelERC721Transfer.sol#L
• LowLevelERC1155Transfer.sol#L

7. Use Underscores for Number Literals

There are multiple occasions where certain numbers have been hardcoded, either in variables or in the code itself. Large numbers can become hard to read.

• SeaportProxy.sol#L207

• LooksRareAggregator.sol#L158