Gas Optimizations - Githubissues

G01 - More efficient `if/else` statement

Next lines L342-L379 could be updated to equivalent equivalent code with fewer checks:

        if(order.isCall) {
            if (order.isLong) {
                _transferERC20sIn(order.erc20Assets, msg.sender);
                _transferERC721sIn(order.erc721Assets, msg.sender);
                _transferFloorsIn(order.floorTokens, floorAssetTokenIds, msg.sender);
                return positionId;
            } else {
                _transferERC20sIn(order.erc20Assets, order.maker);
                _transferERC721sIn(order.erc721Assets, order.maker);
                return positionId;
            }
        } else {
            if (order.isLong) {
                // handle the case where the taker uses native ETH instead of WETH to deposit the strike
                if (weth == order.baseAsset && msg.value > 0) {
                    // check enough ETH was sent to cover the strike
                    require(msg.value == order.strike, "Incorrect ETH amount sent"); 

                    // convert ETH to WETH
                    // we convert the strike ETH to WETH so that the logic in exercise() works
                    // - because exercise() assumes an ERC20 interface on the base asset.
                    IWETH(weth).deposit{value: msg.value}();
                } else {
                    ERC20(order.baseAsset).safeTransferFrom(msg.sender, address(this), order.strike);
                }

                return positionId;
            } else { 
                ERC20(order.baseAsset).safeTransferFrom(order.maker, address(this), order.strike);
                return positionId;
            }
        }

G02 - Use `unchecked` for calculations that cannot overflow/underflow

Next line could be unchecked, because due to L499 feeAmount always will be less than order.strike (since fee < 30 due to L241 ):

contracts/src/PuttyV2.sol:503   ERC20(order.baseAsset).safeTransfer(msg.sender, order.strike - feeAmount);

Next line could be unchecked since order.duration < 10 000 days(L287):

contracts/src/PuttyV2.sol:316   positionExpirations[order.isLong ? uint256(orderHash) : positionId] = block.timestamp + order.duration;

Counter i++ in for loop could be unchecked when it cann't overflow:

contracts/src/PuttyV2.sol:556   for (uint256 i = 0; i < orders.length; i++) { 

contracts/src/PuttyV2.sol:594   for (uint256 i = 0; i < assets.length; i++) { 

contracts/src/PuttyV2.sol:611   for (uint256 i = 0; i < assets.length; i++) {

contracts/src/PuttyV2.sol:627   for (uint256 i = 0; i < floorTokens.length; i++) {

contracts/src/PuttyV2.sol:637   for (uint256 i = 0; i < assets.length; i++) {

contracts/src/PuttyV2.sol:647   for (uint256 i = 0; i < assets.length; i++) {

contracts/src/PuttyV2.sol:658   for (uint256 i = 0; i < floorTokens.length; i++) {

contracts/src/PuttyV2.sol:670   for (uint256 i = 0; i < whitelist.length; i++) {

contracts/src/PuttyV2.sol:728   for (uint256 i = 0; i < arr.length; i++) {

contracts/src/PuttyV2.sol:742   for (uint256 i = 0; i < arr.length; i++) {

G03 - Prefix increment `++i` in `for` loop is cheaper than postfix increment `i++`

In next loops using ++i instead of i++ could save gas:

contracts/src/PuttyV2.sol:556   for (uint256 i = 0; i < orders.length; i++) { 

contracts/src/PuttyV2.sol:594   for (uint256 i = 0; i < assets.length; i++) { 

contracts/src/PuttyV2.sol:611   for (uint256 i = 0; i < assets.length; i++) {

contracts/src/PuttyV2.sol:627   for (uint256 i = 0; i < floorTokens.length; i++) {

contracts/src/PuttyV2.sol:637   for (uint256 i = 0; i < assets.length; i++) {

contracts/src/PuttyV2.sol:647   for (uint256 i = 0; i < assets.length; i++) {

contracts/src/PuttyV2.sol:658   for (uint256 i = 0; i < floorTokens.length; i++) {

contracts/src/PuttyV2.sol:670   for (uint256 i = 0; i < whitelist.length; i++) {

contracts/src/PuttyV2.sol:728   for (uint256 i = 0; i < arr.length; i++) {

contracts/src/PuttyV2.sol:742   for (uint256 i = 0; i < arr.length; i++) {

G04 - Caching array length before `for` loops

contracts/src/PuttyV2.sol:556   for (uint256 i = 0; i < orders.length; i++) { 

contracts/src/PuttyV2.sol:594   for (uint256 i = 0; i < assets.length; i++) { 

contracts/src/PuttyV2.sol:611   for (uint256 i = 0; i < assets.length; i++) {

contracts/src/PuttyV2.sol:627   for (uint256 i = 0; i < floorTokens.length; i++) {

contracts/src/PuttyV2.sol:637   for (uint256 i = 0; i < assets.length; i++) {

contracts/src/PuttyV2.sol:647   for (uint256 i = 0; i < assets.length; i++) {

contracts/src/PuttyV2.sol:658   for (uint256 i = 0; i < floorTokens.length; i++) {

contracts/src/PuttyV2.sol:670   for (uint256 i = 0; i < whitelist.length; i++) {

contracts/src/PuttyV2.sol:728   for (uint256 i = 0; i < arr.length; i++) {

contracts/src/PuttyV2.sol:742   for (uint256 i = 0; i < arr.length; i++) {

Caching the array length before for loop could save gas here:

G05 - Using `>0` costs more gas than `!=0` with `uint` in `require` statement

contracts/src/PuttyV2.sol:293   require(order.baseAsset.code.length > 0, "baseAsset is not contract"); 

contracts/src/PuttyV2.sol:598   require(token.code.length > 0, "ERC20: Token is not contract"); 

contracts/src/PuttyV2.sol:599   require(tokenAmount > 0, "ERC20: Amount too small");

G06 - Using custom error instead of require() string

Custom errors are available from solidity version 0.8.4. They cost cheaper than require/revert strings.

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

contracts/src/PuttyV2.sol:241   require(_fee < 30, "fee must be less than 3%"); 

contracts/src/PuttyV2.sol:278   require(SignatureChecker.isValidSignatureNow(order.maker, orderHash, signature), "Invalid signature"); 

contracts/src/PuttyV2.sol:281   require(!cancelledOrders[orderHash], "Order has been cancelled"); 

contracts/src/PuttyV2.sol:284   require(order.whitelist.length == 0 || isWhitelisted(order.whitelist, msg.sender), "Not whitelisted"); 

contracts/src/PuttyV2.sol:287   require(order.duration < 10_000 days, "Duration too long"); 

contracts/src/PuttyV2.sol:290   require(block.timestamp < order.expiration, "Order has expired"); 

contracts/src/PuttyV2.sol:293   require(order.baseAsset.code.length > 0, "baseAsset is not contract"); 

contracts/src/PuttyV2.sol:297   ? require(floorAssetTokenIds.length == order.floorTokens.length, "Wrong amount of floor tokenIds") 

contracts/src/PuttyV2.sol:298   : require(floorAssetTokenIds.length == 0, "Invalid floor tokens length"); 

contracts/src/PuttyV2.sol:353   require(msg.value == order.strike, "Incorrect ETH amount sent"); 

contracts/src/PuttyV2.sol:395   require(ownerOf(uint256(orderHash)) == msg.sender, "Not owner"); 

contracts/src/PuttyV2.sol:398   require(order.isLong, "Can only exercise long positions");  

contracts/src/PuttyV2.sol:401   require(block.timestamp < positionExpirations[uint256(orderHash)], "Position has expired"); 

contracts/src/PuttyV2.sol:405   ? require(floorAssetTokenIds.length == order.floorTokens.length, "Wrong amount of floor tokenIds") 

contracts/src/PuttyV2.sol:406   : require(floorAssetTokenIds.length == 0, "Invalid floor tokenIds length"); 

contracts/src/PuttyV2.sol:429   require(msg.value == order.strike, "Incorrect ETH amount sent"); 

contracts/src/PuttyV2.sol:470   require(!order.isLong, "Must be short position"); 

contracts/src/PuttyV2.sol:475   require(ownerOf(uint256(orderHash)) == msg.sender, "Not owner"); 

contracts/src/PuttyV2.sol:481   require(block.timestamp > positionExpirations[longPositionId] || isExercised, "Must be exercised or expired"); 

contracts/src/PuttyV2.sol:527   require(msg.sender == order.maker, "Not your order"); 

contracts/src/PuttyV2.sol:551   require(orders.length == signatures.length, "Length mismatch in input"); 

contracts/src/PuttyV2.sol:552   require(signatures.length == floorAssetTokenIds.length, "Length mismatch in input"); 

contracts/src/PuttyV2.sol:598   require(token.code.length > 0, "ERC20: Token is not contract"); 

contracts/src/PuttyV2.sol:599   require(tokenAmount > 0, "ERC20: Amount too small"); 

contracts/src/PuttyV2.sol:765   require(_ownerOf[id] != address(0), "URI query for NOT_MINTED token"); 

contracts/src/PuttyV2Nft.sol:12 require(to != address(0), "INVALID_RECIPIENT"); 

contracts/src/PuttyV2Nft.sol:13 require(_ownerOf[id] == address(0), "ALREADY_MINTED"); 

contracts/src/PuttyV2Nft.sol:26 require(from == _ownerOf[id], "WRONG_FROM"); 

contracts/src/PuttyV2Nft.sol:27 require(to != address(0), "INVALID_RECIPIENT"); 

contracts/src/PuttyV2Nft.sol:30 "NOT_AUTHORIZED"

contracts/src/PuttyV2Nft.sol:41 require(owner != address(0), "ZERO_ADDRESS");

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

Gas Optimizations #313

G01 - More efficient `if/else` statement

G02 - Use `unchecked` for calculations that cannot overflow/underflow

G03 - Prefix increment `++i` in `for` loop is cheaper than postfix increment `i++`

G04 - Caching array length before `for` loops

G05 - Using `>0` costs more gas than `!=0` with `uint` in `require` statement

G06 - Using custom error instead of require() string

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

Gas Optimizations #313

G01 - More efficient if/else statement

G02 - Use unchecked for calculations that cannot overflow/underflow

G03 - Prefix increment ++i in for loop is cheaper than postfix increment i++

G04 - Caching array length before for loops

G05 - Using >0 costs more gas than !=0 with uint in require statement

G06 - Using custom error instead of require() string

G01 - More efficient `if/else` statement

G02 - Use `unchecked` for calculations that cannot overflow/underflow

G03 - Prefix increment `++i` in `for` loop is cheaper than postfix increment `i++`

G04 - Caching array length before `for` loops

G05 - Using `>0` costs more gas than `!=0` with `uint` in `require` statement