Gas Optimizations

[code423n4]

2022-06-putty gas report

MAKING SOME CONSTANTS AS NON-PUBLIC TO SAVE GAS

Reducing from public to private or internal can save gas when a constant isn't used outside of its contract. The suggestion: changing the visibility from public to internal or private here:

89-101

    bytes32 public constant ERC721ASSET_TYPE_HASH =
        keccak256(abi.encodePacked("ERC721Asset(address token,uint256 tokenId)"));

    /**
        @dev ERC20Asset type hash used for EIP-712 encoding.
     */
    bytes32 public constant ERC20ASSET_TYPE_HASH =
        keccak256(abi.encodePacked("ERC20Asset(address token,uint256 tokenAmount)"));

    /**
        @dev ERC721Asset type hash used for EIP-712 encoding.
     */
    bytes32 public constant ORDER_TYPE_HASH =
        keccak256(

PUBLIC FUNCTIONS TO EXTERNAL

Functions only called outside the contract could be declared external. The following functions could be set external/internal/private to save gas and improve code quality. External call cost is less expensive than of public functions.

These could be set to private:

228:
    function setBaseURI(string memory _baseURI) public payable onlyOwner {

240:
    function setFee(uint256 _fee) public payable onlyOwner {

These could be set to external:

389:
    function exercise(Order memory order, uint256[] calldata floorAssetTokenIds) public payable {

466:
    function withdraw(Order memory order) public {

546:
    function batchFillOrder(
        Order[] memory orders,
        bytes[] calldata signatures,
        uint256[][] memory floorAssetTokenIds
    ) public returns (uint256[] memory positionIds) {

573: 
    function acceptCounterOffer(
        Order memory order,
        bytes calldata signature,
        Order memory originalOrder
    ) public payable returns (uint256 positionId) {

Beside, CALLDATA could be used instead of MEMORY for these external functions.

FOR-LOOPS GAS-SAVING

There are several for loops can be improved to save gas

556:
        for (uint256 i = 0; i < orders.length; i++) {
            positionIds[i] = fillOrder(orders[i], signatures[i], floorAssetTokenIds[i]);
        }

594:
        for (uint256 i = 0; i < assets.length; i++) {

611:
        for (uint256 i = 0; i < assets.length; i++) {

627:
        for (uint256 i = 0; i < floorTokens.length; i++) {

637:
        for (uint256 i = 0; i < assets.length; i++) {

647:
        for (uint256 i = 0; i < assets.length; i++) {

658:
        for (uint256 i = 0; i < floorTokens.length; i++) {

670:
        for (uint256 i = 0; i < whitelist.length; i++) {

728:
        for (uint256 i = 0; i < arr.length; i++) {

742:
        for (uint256 i = 0; i < arr.length; i++) {

1. LOOP LENGTH COULD BE CACHED

suggestion: cache the for loop length to memory before the loop.

2. ++I COSTS LESS GAS COMPARED TO I++ OR I += 1

suggestion: using ++i instead of i++ to increment the value of an uint variable.

3. NO NEED TO EXPLICITLY INITIALIZE VARIABLES WITH DEFAULT VALUES

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for address…). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

suggestion:

for (uint i = 0; i < lenth; ++i)

can be written as

for (uint i ; i < lenth; ++i)

FOR LOOP SUMMARY

The for loops can be written as follows, take one example:

uint length = assets.length;
for (uint i; i < length;) {
    // ... 
    unchecked { ++i; } 
}

> 0 can be replaced with != 0

!= 0 is a cheaper operation compared to > 0, when dealing with uint.

293:
        require(order.baseAsset.code.length > 0, "baseAsset is not contract");

327:
            if (weth == order.baseAsset && msg.value > 0) {

351:
            if (weth == order.baseAsset && msg.value > 0) {

427:
            if (weth == order.baseAsset && msg.value > 0) {

598-599:
            require(token.code.length > 0, "ERC20: Token is not contract");
            require(tokenAmount > 0, "ERC20: Amount too small");

DUPLICATED REQUIRE() CHECKS COULD BE REFACTORED TO A MODIFIER OR FUNCTION

296:
        order.isCall && order.isLong
            ? require(floorAssetTokenIds.length == order.floorTokens.length, "Wrong amount of floor tokenIds")
            : require(floorAssetTokenIds.length == 0, "Invalid floor tokens length");

405:
        !order.isCall
            ? require(floorAssetTokenIds.length == order.floorTokens.length, "Wrong amount of floor tokenIds")
            : require(floorAssetTokenIds.length == 0, "Invalid floor tokenIds length");

353:
                require(msg.value == order.strike, "Incorrect ETH amount sent");

429:
                require(msg.value == order.strike, "Incorrect ETH amount sent");

Delete state variable if won't be used later

'isApprovedForAll[from][msg.sender]' can be deleted, like which is done for 'getApproved[id]'.

PyttyV2Nft.sol
     function transferFrom(
        \\ ...