4naly3er-2022-11-non-fungible.md

Report

Gas Optimizations

	Issue	Instances
[GAS-1]	Cache array length outside of loop	2
[GAS-2]	Use != 0 instead of > 0 for unsigned integer comparison	1
[GAS-3]	Don't initialize variables with default value	6
[GAS-4]	Using bools for storage incurs overhead	2
[GAS-5]	Use Custom Errors	20
[GAS-6]	++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)	4
[GAS-7]	Using private rather than public for constants, saves gas	5

[GAS-1] Cache array length outside of loop

If not cached, the solidity compiler will always read the length of the array during each iteration. That is, if it is a storage array, this is an extra sload operation (100 additional extra gas for each iteration except for the first) and if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first).

Instances (2):

File: Exchange.sol

307:         for (uint8 i = 0; i < orders.length; i++) {

598:         for (uint8 i = 0; i < fees.length; i++) {

[GAS-2] Use != 0 instead of > 0 for unsigned integer comparison

Instances (1):

File: Exchange.sol

412:         if (order.order.extraParams.length > 0 && order.order.extraParams[0] == 0x01) {

[GAS-3] Don't initialize variables with default value

Instances (6):

File: Exchange.sol

147:     uint256 public remainingETH = 0;

177:         for (uint8 i=0; i < executionsLength; i++) {

184:         for (uint8 i = 0; i < executionsLength; i++) {

307:         for (uint8 i = 0; i < orders.length; i++) {

597:         uint256 totalFee = 0;

598:         for (uint8 i = 0; i < fees.length; i++) {

[GAS-4] Using bools for storage incurs overhead

Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the past. See source.

Instances (2):

File: Exchange.sol

85:     mapping(bytes32 => bool) public cancelledOrFilled;

146:     bool public isInternal = false;

[GAS-5] Use Custom Errors

Source Instead of using error strings, to reduce deployment and runtime cost, you should use Custom Errors. This would save both deployment and runtime cost.

Instances (20):

File: Exchange.sol

36:         require(isOpen == 1, "Closed");

49:         require(isInternal, "This function should not be called directly");

245:         require(_validateSignatures(sell, sellHash), "Sell failed authorization");

246:         require(_validateSignatures(buy, buyHash), "Buy failed authorization");

248:         require(_validateOrderParameters(sell.order, sellHash), "Sell has invalid parameters");

249:         require(_validateOrderParameters(buy.order, buyHash), "Buy has invalid parameters");

295:         require(!cancelledOrFilled[hash], "Order already cancelled or filled");

327:         require(address(_executionDelegate) != address(0), "Address cannot be zero");

336:         require(address(_policyManager) != address(0), "Address cannot be zero");

345:         require(_oracle != address(0), "Address cannot be zero");

414:             require(block.number - order.blockNumber < blockRange, "Signed block number out of range");

523:         require(v == 27 || v == 28, "Invalid v parameter");

545:             require(policyManager.isPolicyWhitelisted(sell.matchingPolicy), "Policy is not whitelisted");

549:             require(policyManager.isPolicyWhitelisted(buy.matchingPolicy), "Policy is not whitelisted");

552:         require(canMatch, "Orders cannot be matched");

604:         require(totalFee <= price, "Total amount of fees are more than the price");

630:             require(to != address(0), "Transfer to zero address");

632:             require(success, "ETH transfer failed");

636:             require(success, "Pool transfer failed");

641:             revert("Invalid payment token");

[GAS-6] ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)

Saves 5 gas per loop

Instances (4):

File: Exchange.sol

177:         for (uint8 i=0; i < executionsLength; i++) {

184:         for (uint8 i = 0; i < executionsLength; i++) {

307:         for (uint8 i = 0; i < orders.length; i++) {

598:         for (uint8 i = 0; i < fees.length; i++) {

[GAS-7] Using private rather than public for constants, saves gas

If needed, the values can be read from the verified contract source code, or if there are multiple values there can be a single getter function that returns a tuple of the values of all currently-public constants. Saves 3406-3606 gas in deployment gas due to the compiler not having to create non-payable getter functions for deployment calldata, not having to store the bytes of the value outside of where it's used, and not adding another entry to the method ID table

Instances (5):

File: Exchange.sol

70:     string public constant NAME = "Exchange";

71:     string public constant VERSION = "1.0";

72:     uint256 public constant INVERSE_BASIS_POINT = 10_000;

73:     address public constant WETH = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2;

74:     address public constant POOL = 0xF66CfDf074D2FFD6A4037be3A669Ed04380Aef2B;

Low Issues

	Issue	Instances
[L-1]	Unsafe ERC20 operation(s)	1

[L-1] Unsafe ERC20 operation(s)

Instances (1):

File: Exchange.sol

635:             bool success = IPool(POOL).transferFrom(from, to, amount);