Papr contest - eyexploit's results

Unused custom errors should be removed

Larger the code base, larger the bytecode will be generated. These bytecode is responsible for total deployment cost. Unused custom errors will only increase the size of bytecode and hence the deployment cost. Its better to remove them if not getting used.

https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/NFTEDA/NFTEDA.sol#L18

Use safeTransferFrom for single NFT transfers to save the gas.

safeTransferFrom are well optimized code, can be used to save the gas. And its also mentioned in the natspec format below. But didn't really applied by the developer.

    /// @notice adds collateral to msg.senders vault for collateral.addr
    /// @dev use safeTransferFrom to save gas if only sending one NFT
    /// @param collateral collateral to add
    function addCollateral(IPaprController.Collateral[] calldata collateral) external;

    /// @inheritdoc IPaprController
    function addCollateral(IPaprController.Collateral[] calldata collateralArr) external override {
        for (uint256 i = 0; i < collateralArr.length;) {
            _addCollateralToVault(msg.sender, collateralArr[i]);
            collateralArr[i].addr.transferFrom(msg.sender, address(this), collateralArr[i].id);
            unchecked {
                ++i;
            }
        }
    }

MerkleProof can be used to avoid high gas consumption in setAllowedCollateral

Modifying state by iterating over each and every element of an array is highly gas consuming process and doing for larger number make it worse.

In future, we might deal with hundred of collateral assets and in order to make them into allowed collateral list, it might exceed the gas limit and revert.

An efficient call would be implementing Merkle tree whitelisting mechanism. Its saves a lot of gas, and it is easier to implement as well. You only need to submit merkleProof in order to validate the collateralAddr.

https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L365-L379

Use of unchecked can save gas where computation is known to be overflow/underflow safe

The codebase already uses the unchecked{} primitive to save gas where computation is known to be overflow/underflow safe. There are a few more places where this can be applied.

Proof of Concept :

https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L546 https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L550