search

code-423n4 / 2022-06-putty-findings

5 stars 0 forks source link

Gas Optimizations #430

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Summary

Title Counts
[Gas-1] Use != 0 instead of > 0 on uint variables 7
[Gas-2] No need to set 0 on uint variables 11
[Gas-3] Gas cost of ++i is smaller than i++ 10
[Gas-4] Use custom errors 33
[Gas-5] Unnecessary Strings.sol import at PuttyV2Nft.sol 1
[Gas-6] Flipping the boolean check 1
[Gas-7] Not splitting the strings 1

[Gas-1] Use != 0 instead of > 0 on uint variables

uint variables will never be lower than 0. Therefore, > 0 and != 0 have same meanings. Using != 0 can reduce the gas deployment cost, so it is worth using != 0 wherever possible.

Here are list of the gas improvements by using != 0.

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L293

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L327

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L351

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L427

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L498

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L598

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L599

[Gas-2] No need to set 0 on uint variables

The default value of uint varibles are 0. Therefore, there is no need to set 0 on uint variables. Not setting 0 on uint variables can reduce the deployment gas cost.

Here are list of the gas improvements.

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L497

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L556

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L594

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L611

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L627

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L637

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L647

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L658

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L670

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L728

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L742

[Gas-3] Gas cost of ++i is smaller than i++

Here are gas optimization opportunities.

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L556

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L594

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L611

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L627

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L637

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L647

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L658

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L670

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L728

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L742

[Gas-4] Use custom errors

Using custom errors can reduce the gas cost.

File: 2022-06-putty/contracts/src/PuttyV2Nft.sol
  11,9:         require(to != address(0), "INVALID_RECIPIENT");
  12,9:         require(_ownerOf[id] == address(0), "ALREADY_MINTED");
  25,9:         require(from == _ownerOf[id], "WRONG_FROM");
  26,9:         require(to != address(0), "INVALID_RECIPIENT");
  27,9:         require(
  40,9:         require(owner != address(0), "ZERO_ADDRESS");
File: 2022-06-putty/contracts/src/PuttyV2.sol
  214,9:         require(_weth != address(0), "Unset weth address");
  241,9:         require(_fee < 30, "fee must be less than 3%");
  278,9:         require(SignatureChecker.isValidSignatureNow(order.maker, orderHash, signature), "Invalid signature");
  281,9:         require(!cancelledOrders[orderHash], "Order has been cancelled");
  284,9:         require(order.whitelist.length == 0 || isWhitelisted(order.whitelist, msg.sender), "Not whitelisted");
  287,9:         require(order.duration < 10_000 days, "Duration too long");
  290,9:         require(block.timestamp < order.expiration, "Order has expired");
  293,9:         require(order.baseAsset.code.length > 0, "baseAsset is not contract");
  297,15:             ? require(floorAssetTokenIds.length == order.floorTokens.length, "Wrong amount of floor tokenIds")
  298,15:             : require(floorAssetTokenIds.length == 0, "Invalid floor tokens length");
  329,17:                 require(msg.value == order.premium, "Incorrect ETH amount sent");
  353,17:                 require(msg.value == order.strike, "Incorrect ETH amount sent");
  395,9:         require(ownerOf(uint256(orderHash)) == msg.sender, "Not owner");
  398,9:         require(order.isLong, "Can only exercise long positions");
  401,9:         require(block.timestamp < positionExpirations[uint256(orderHash)], "Position has expired");
  405,15:             ? require(floorAssetTokenIds.length == order.floorTokens.length, "Wrong amount of floor tokenIds")
  406,15:             : require(floorAssetTokenIds.length == 0, "Invalid floor tokenIds length");
  429,17:                 require(msg.value == order.strike, "Incorrect ETH amount sent");
  470,9:         require(!order.isLong, "Must be short position");
  475,9:         require(ownerOf(uint256(orderHash)) == msg.sender, "Not owner");
  481,9:         require(block.timestamp > positionExpirations[longPositionId] || isExercised, "Must be exercised or expired");
  527,9:         require(msg.sender == order.maker, "Not your order");
  551,9:         require(orders.length == signatures.length, "Length mismatch in input");
  552,9:         require(signatures.length == floorAssetTokenIds.length, "Length mismatch in input");
  598,13:             require(token.code.length > 0, "ERC20: Token is not contract");
  599,13:             require(tokenAmount > 0, "ERC20: Amount too small");
  765,9:         require(_ownerOf[id] != address(0), "URI query for NOT_MINTED token");

[Gas-5] Unnecessary Strings.sol import at PuttyV2Nft.sol

Import of Strings.sol is not necessary. It can remove this line completely.

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2Nft.sol#L5

[Gas-6] Flip the boolean check to reduce gas cost

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L404-L406

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

It uses !order.isCall but just removing ! and changing order can reduce a gas cost.

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

[Gas-7] Not splitting the strings

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L101-L122

    bytes32 public constant ORDER_TYPE_HASH =
        keccak256(
            abi.encodePacked(
                "Order(",
                "address maker,",
                "bool isCall,",
                "bool isLong,",
                "address baseAsset,",
                "uint256 strike,",
                "uint256 premium,",
                "uint256 duration,",
                "uint256 expiration,",
                "uint256 nonce,",
                "address[] whitelist,",
                "address[] floorTokens,",
                "ERC20Asset[] erc20Assets,",
                "ERC721Asset[] erc721Assets",
                ")",
                "ERC20Asset(address token,uint256 tokenAmount)",
                "ERC721Asset(address token,uint256 tokenId)"
            )
        );

abi.encodePacked is used to combine the strings. This is good for the readability, but combining the strings using abi.encodePacked can add gas cost. This part can be written to reduce the gas cost, but this sacrifices the readability a bit.

    bytes32 public constant ORDER_TYPE_HASH =
        keccak256(
            "Order(address maker,bool isCall,bool isLong,address baseAsset,uint256 strike,uint256 premium,uint256 duration,uint256 expiration,uint256 nonce,address[] whitelist,address[] floorTokens,ERC20Asset[] erc20Assets,ERC721Asset[] erc721Assets)ERC20Asset(address token,uint256 tokenAmount)ERC721Asset(address token,uint256 tokenId)"
        );