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

5 stars 0 forks source link

Gas Optimizations #391

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

GAS

[G-1] Using calldata as a pointer for read only argument

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

Using calldata to store read only array variable can save gas

[G-2] Using != operator

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#L598-L599

Using != rather than > to validate value is not 0 for uint is more effective

[G-3] Using unchecked and prefix increment for i inside for()

Occurrences: 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

Using unchecked and prefix increment is way more effective than the current implementation

RECOMMENDED MITIGATION STEP

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

[G-4] Using else statement can save gas

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

Using else statement can avoid MLOADing and validating order.isCall and order.isLong. All the rest of conditions has checked before (L495). There are no more condition will occur at L509

RECOMMENDED MITIGATION CHECK

        if ((order.isCall && isExercised) || (!order.isCall && !isExercised)) {
            // send the fee to the admin/DAO if fee is greater than 0%
            uint256 feeAmount = 0;
            if (fee > 0) {
                feeAmount = (order.strike * fee) / 1000;
                ERC20(order.baseAsset).safeTransfer(owner(), feeAmount);
            }

            ERC20(order.baseAsset).safeTransfer(msg.sender, order.strike - feeAmount);

            return;
        }

        // transfer assets from putty to owner if put is exercised or call is expired
        else{ //@audit: Use else statement here
            _transferERC20sOut(order.erc20Assets);
            _transferERC721sOut(order.erc721Assets);

            // for call options the floor token ids are saved in the long position in fillOrder(),
            // and for put options the floor tokens ids are saved in the short position in exercise()
            uint256 floorPositionId = order.isCall ? longPositionId : uint256(orderHash);
            _transferFloorsOut(order.floorTokens, positionFloorAssetTokenIds[floorPositionId]);

            return;
        }