Gas Optimizations

[code423n4]

[G-00] Expensive if can be avoided

contracts/src/PuttyV2.sol:
  494              // transfer strike to owner if put is expired or call is exercised 
  495:             if ((order.isCall && isExercised) || (!order.isCall && !isExercised)) {
  496                  // send the fee to the admin/DAO if fee is greater than 0%

  508              // transfer assets from putty to owner if put is exercised or call is expired
  509:             if ((order.isCall && !isExercised) || (!order.isCall && isExercised)) {
  510                  _transferERC20sOut(order.erc20Assets);

Given that order.isCall and isExercised are boolean, there are only 4 outcomes to their Cartesian product. 2 cases are covered in the first if (L494) and so the other 2 cases can be in else of the first if, instead of computing again. Considering that first if have return; at the end, you can even skip the else. This will save gas of at least 4 reads and 1 logic op every time withdraw is called.

[G-01] ++i costs less gas compared to i++

++i costs about 5 gas less per iteration compared to i++ for unsigned integer. This statement is true even with the optimizer enabled. Summarized my results where i is used in a loop, is unsigned integer, and you safely can be changed to ++i without changing any behavior

I've found 11 locations:

contracts/src/PuttyV2.sol:
  555  
  556:             for (uint256 i = 0; i < orders.length; i++) {
  557                  positionIds[i] = fillOrder(orders[i], signatures[i], floorAssetTokenIds[i]);

  593          function _transferERC20sIn(ERC20Asset[] memory assets, address from) internal {
  594:             for (uint256 i = 0; i < assets.length; i++) {
  595                  address token = assets[i].token;

  610          function _transferERC721sIn(ERC721Asset[] memory assets, address from) internal {
  611:             for (uint256 i = 0; i < assets.length; i++) {
  612                  ERC721(assets[i].token).safeTransferFrom(from, address(this), assets[i].tokenId);

  626          ) internal {
  627:             for (uint256 i = 0; i < floorTokens.length; i++) {
  628                  ERC721(floorTokens[i]).safeTransferFrom(from, address(this), floorTokenIds[i]);

  636          function _transferERC20sOut(ERC20Asset[] memory assets) internal {
  637:             for (uint256 i = 0; i < assets.length; i++) {
  638                  ERC20(assets[i].token).safeTransfer(msg.sender, assets[i].tokenAmount);

  646          function _transferERC721sOut(ERC721Asset[] memory assets) internal {
  647:             for (uint256 i = 0; i < assets.length; i++) {
  648                  ERC721(assets[i].token).safeTransferFrom(address(this), msg.sender, assets[i].tokenId);

  657          function _transferFloorsOut(address[] memory floorTokens, uint256[] memory floorTokenIds) internal {
  658:             for (uint256 i = 0; i < floorTokens.length; i++) {
  659                  ERC721(floorTokens[i]).safeTransferFrom(address(this), msg.sender, floorTokenIds[i]);

  669          function isWhitelisted(address[] memory whitelist, address target) public pure returns (bool) {
  670:             for (uint256 i = 0; i < whitelist.length; i++) {
  671                  if (target == whitelist[i]) return true;

  727          function encodeERC20Assets(ERC20Asset[] memory arr) public pure returns (bytes memory encoded) { 
  728:             for (uint256 i = 0; i < arr.length; i++) {  
  729                  encoded = abi.encodePacked(

  741          function encodeERC721Assets(ERC721Asset[] memory arr) public pure returns (bytes memory encoded) {
  742:             for (uint256 i = 0; i < arr.length; i++) {
  743                  encoded = abi.encodePacked(

[G-02] An arrays length should be cached to save gas in for-loops

Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset). Caching the array length in the stack saves around 3 gas per iteration. I've found 10 locations:

contracts/src/PuttyV2.sol:
  555  
  556:             for (uint256 i = 0; i < orders.length; i++) {
  557                  positionIds[i] = fillOrder(orders[i], signatures[i], floorAssetTokenIds[i]);

  593          function _transferERC20sIn(ERC20Asset[] memory assets, address from) internal {
  594:             for (uint256 i = 0; i < assets.length; i++) {
  595                  address token = assets[i].token;

  610          function _transferERC721sIn(ERC721Asset[] memory assets, address from) internal {
  611:             for (uint256 i = 0; i < assets.length; i++) {
  612                  ERC721(assets[i].token).safeTransferFrom(from, address(this), assets[i].tokenId);

  626          ) internal {
  627:             for (uint256 i = 0; i < floorTokens.length; i++) {
  628                  ERC721(floorTokens[i]).safeTransferFrom(from, address(this), floorTokenIds[i]);

  636          function _transferERC20sOut(ERC20Asset[] memory assets) internal {
  637:             for (uint256 i = 0; i < assets.length; i++) {
  638                  ERC20(assets[i].token).safeTransfer(msg.sender, assets[i].tokenAmount);

  646          function _transferERC721sOut(ERC721Asset[] memory assets) internal {
  647:             for (uint256 i = 0; i < assets.length; i++) {
  648                  ERC721(assets[i].token).safeTransferFrom(address(this), msg.sender, assets[i].tokenId);

  657          function _transferFloorsOut(address[] memory floorTokens, uint256[] memory floorTokenIds) internal {
  658:             for (uint256 i = 0; i < floorTokens.length; i++) {
  659                  ERC721(floorTokens[i]).safeTransferFrom(address(this), msg.sender, floorTokenIds[i]);

  669          function isWhitelisted(address[] memory whitelist, address target) public pure returns (bool) {
  670:             for (uint256 i = 0; i < whitelist.length; i++) {
  671                  if (target == whitelist[i]) return true;

  727          function encodeERC20Assets(ERC20Asset[] memory arr) public pure returns (bytes memory encoded) {
  728:             for (uint256 i = 0; i < arr.length; i++) { 
  729                  encoded = abi.encodePacked(

  741          function encodeERC721Assets(ERC721Asset[] memory arr) public pure returns (bytes memory encoded) {
  742:             for (uint256 i = 0; i < arr.length; i++) {
  743                  encoded = abi.encodePacked(

Furthermore - it might be a waist to cache length if it's likely to be 0 - but if you think that this is the case, I would recommend: [G-02B] Check that the length of the first parameter is bigger than zero, before even calling the function, and save the function call.

593 function _transferERC20sIn(ERC20Asset[] memory assets, address from) internal {

610 function _transferERC721sIn(ERC721Asset[] memory assets, address from) internal {

622 function _transferFloorsIn(

636 function _transferERC20sOut(ERC20Asset[] memory assets) internal {

646 function _transferERC721sOut(ERC721Asset[] memory assets) internal {

657 function _transferFloorsOut(address[] memory floorTokens, uint256[] memory floorTokenIds) internal {

[G-03] Using default values is cheaper than assignment

If a variable is not set/initialized, it is assumed to have the default value, 0 for uint Explicitly initializing it with its default value is an anti-pattern and wastes gas. For example: uint8 i = 0; should be replaced with uint8 i;

I've found 11 locations:

contracts/src/PuttyV2.sol:
  496                  // send the fee to the admin/DAO if fee is greater than 0%
  497:                 uint256 feeAmount = 0;
  498                  if (fee > 0) {

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

  593          function _transferERC20sIn(ERC20Asset[] memory assets, address from) internal {
  594:             for (uint256 i = 0; i < assets.length; i++) {
  595                  address token = assets[i].token;

  610          function _transferERC721sIn(ERC721Asset[] memory assets, address from) internal {
  611:             for (uint256 i = 0; i < assets.length; i++) {
  612                  ERC721(assets[i].token).safeTransferFrom(from, address(this), assets[i].tokenId);

  626          ) internal {
  627:             for (uint256 i = 0; i < floorTokens.length; i++) {
  628                  ERC721(floorTokens[i]).safeTransferFrom(from, address(this), floorTokenIds[i]);

  636          function _transferERC20sOut(ERC20Asset[] memory assets) internal {
  637:             for (uint256 i = 0; i < assets.length; i++) {
  638                  ERC20(assets[i].token).safeTransfer(msg.sender, assets[i].tokenAmount);

  646          function _transferERC721sOut(ERC721Asset[] memory assets) internal {
  647:             for (uint256 i = 0; i < assets.length; i++) {
  648                  ERC721(assets[i].token).safeTransferFrom(address(this), msg.sender, assets[i].tokenId);

  657          function _transferFloorsOut(address[] memory floorTokens, uint256[] memory floorTokenIds) internal {
  658:             for (uint256 i = 0; i < floorTokens.length; i++) {
  659                  ERC721(floorTokens[i]).safeTransferFrom(address(this), msg.sender, floorTokenIds[i]);

  669          function isWhitelisted(address[] memory whitelist, address target) public pure returns (bool) {
  670:             for (uint256 i = 0; i < whitelist.length; i++) {
  671                  if (target == whitelist[i]) return true;

  727          function encodeERC20Assets(ERC20Asset[] memory arr) public pure returns (bytes memory encoded) {
  728:             for (uint256 i = 0; i < arr.length; i++) {
  729                  encoded = abi.encodePacked(

  741          function encodeERC721Assets(ERC721Asset[] memory arr) public pure returns (bytes memory encoded) {
  742:             for (uint256 i = 0; i < arr.length; i++) {
  743                  encoded = abi.encodePacked(

[G-04] Upgrade pragma to 0.8.15 to save gas

Across the whole solution, the declared pragma is 0.8.13 According to the release note of 0.8.15: https://blog.soliditylang.org/2022/06/15/solidity-0.8.15-release-announcement/ The benchmark shows saving of 2.5-10% Bytecode size, Saving 2-8% Deployment gas, And saving up to 6.2% Runtime gas.

[G-05] Custom errors save gas

From solidity 0.8.4 and above, Custom errors from are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met) Source: https://blog.soliditylang.org/2021/04/21/custom-errors/: Starting from Solidity v0.8.4, there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors. Until now, you could already use strings to give more information about failures (e.g., revert("Insufficient funds.");), but they are rather expensive, especially when it comes to deploy cost, and it is difficult to use dynamic information in them.

I've found 33 require locations in 2 different files:

contracts/src/PuttyV2.sol:
  213          ) {
  214:             require(_weth != address(0), "Unset weth address");
  215  

  240          function setFee(uint256 _fee) public payable onlyOwner {
  241:             require(_fee < 30, "fee must be less than 3%");
  242  

  277              // check signature is valid using EIP-712
  278:             require(SignatureChecker.isValidSignatureNow(order.maker, orderHash, signature), "Invalid signature");
  279  
  280              // check order is not cancelled
  281:             require(!cancelledOrders[orderHash], "Order has been cancelled");  
  282  
  283              // check msg.sender is allowed to fill the order
  284:             require(order.whitelist.length == 0 || isWhitelisted(order.whitelist, msg.sender), "Not whitelisted");
  285  
  286              // check duration is valid
  287:             require(order.duration < 10_000 days, "Duration too long");
  288  
  289              // check order has not expired
  290:             require(block.timestamp < order.expiration, "Order has expired");
  291  
  292              // check base asset exists
  293:             require(order.baseAsset.code.length > 0, "baseAsset is not contract");
  294  

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

  328                      // check enough ETH was sent to cover the premium
  329:                     require(msg.value == order.premium, "Incorrect ETH amount sent");
  330  

  352                      // check enough ETH was sent to cover the strike
  353:                     require(msg.value == order.strike, "Incorrect ETH amount sent");
  354  

  394              // check user owns the position
  395:             require(ownerOf(uint256(orderHash)) == msg.sender, "Not owner");
  396  
  397              // check position is long
  398:             require(order.isLong, "Can only exercise long positions");
  399  
  400              // check position has not expired
  401:             require(block.timestamp < positionExpirations[uint256(orderHash)], "Position has expired");
  402  

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

  428                      // check enough ETH was sent to cover the strike
  429:                     require(msg.value == order.strike, "Incorrect ETH amount sent");
  430  

  469              // check order is short
  470:             require(!order.isLong, "Must be short position");
  471  

  474              // check msg.sender owns the position
  475:             require(ownerOf(uint256(orderHash)) == msg.sender, "Not owner");
  476  

  480              // check long position has either been exercised or is expired
  481:             require(block.timestamp > positionExpirations[longPositionId] || isExercised, "Must be exercised or expired");
  482  

  526          function cancel(Order memory order) public {
  527:             require(msg.sender == order.maker, "Not your order");
  528  

  550          ) public returns (uint256[] memory positionIds) {
  551:             require(orders.length == signatures.length, "Length mismatch in input");
  552:             require(signatures.length == floorAssetTokenIds.length, "Length mismatch in input");
  553  

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

  764          function tokenURI(uint256 id) public view override returns (string memory) {
  765:             require(_ownerOf[id] != address(0), "URI query for NOT_MINTED token");
  766  

contracts/src/PuttyV2Nft.sol:
  11      function _mint(address to, uint256 id) internal override {
  12:         require(to != address(0), "INVALID_RECIPIENT");
  13:         require(_ownerOf[id] == address(0), "ALREADY_MINTED");
  14  

  25      ) public override {
  26:         require(from == _ownerOf[id], "WRONG_FROM");
  27:         require(to != address(0), "INVALID_RECIPIENT");
  28:         require(
  29            msg.sender == from || isApprovedForAll[from][msg.sender] || msg.sender == getApproved[id],
  30            "NOT_AUTHORIZED"
  31          );

  40      function balanceOf(address owner) public pure override returns (uint256) {
  41:         require(owner != address(0), "ZERO_ADDRESS");
  42          return type(uint256).max;

[G-06] != 0 is cheaper than > 0

!= 0 costs less gas compared to > 0 for unsigned integers even when optimizer enabled saves 6 gas

I've found 7 locations:

contracts/src/PuttyV2.sol:
  292              // check base asset exists
  293:             require(order.baseAsset.code.length > 0, "baseAsset is not contract");
  294  

  326                  // handle the case where the user uses native ETH instead of WETH to pay the premium
  327:                 if (weth == order.baseAsset && msg.value > 0) {
  328                      // check enough ETH was sent to cover the premium

  350                  // handle the case where the taker uses native ETH instead of WETH to deposit the strike
  351:                 if (weth == order.baseAsset && msg.value > 0) {
  352                      // check enough ETH was sent to cover the strike

  426                  // handle the case where the taker uses native ETH instead of WETH to pay the strike
  427:                 if (weth == order.baseAsset && msg.value > 0) {
  428                      // check enough ETH was sent to cover the strike

  497                  uint256 feeAmount = 0;
  498:                 if (fee > 0) {
  499                      feeAmount = (order.strike * fee) / 1000;

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