code-423n4 / 2022-02-tribe-turbo-findings

1 stars 0 forks source link

Gas Optimizations #3

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Title: Internal functions to private Severity: GAS

The following functions could be set private to save gas and improve code quality:

    TurboSafe.sol, afterDeposit
    TurboSafe.sol, beforeWithdraw

Title: Use calldata instead of memory Severity: GAS

Use calldata instead of memory for function parameters In some cases, having function arguments in calldata instead of memory is more optimal.

    TurboRouter.constructor (name)

Title: Unused imports Severity: GAS

In the following files there are contract imports that aren't used Import of unnecessary files costs deployment gas (and is a bad coding practice that is important to ignore)

    TurboMaster.sol, line 12, import {TurboGibber} from "./modules/TurboGibber.sol";

Title: Unnecessary constructor Severity: GAS

The following constructors are empty. (A similar issue https://github.com/code-423n4/2021-11-fei-findings/issues/12)

    TurboClerk.sol.constructor
    TurboBooster.sol.constructor

Title: Unnecessary functions Severity: GAS

The following functions are not used at all. Therefore you can remove them to save deployment gas and improve code clearness.

    TurboSafe.sol, afterDeposit
    TurboSafe.sol, beforeWithdraw

Title: Public functions to external Severity: GAS

The following functions could be set external to save gas and improve code quality. External call cost is less expensive than of public functions.

    TurboRouter.sol, withdraw
    TurboRouter.sol, redeem
    TurboRouter.sol, deposit
    TurboRouter.sol, boost
    TurboRouter.sol, createSafeAndDepositAndBoost
    TurboRouter.sol, mint
    TurboSafe.sol, totalAssets

Title: Unnecessary payable Severity: GAS

The following functions are payable but msg.value isn't used - therefore the function payable state modifier isn't necessary. Payable functions are more gas expensive than others, and it's danger the users if they send ETH by mistake.

    TurboRouter.sol, withdraw is payable but doesn't use msg.value
    TurboRouter.sol, redeem is payable but doesn't use msg.value
    TurboRouter.sol, mint is payable but doesn't use msg.value
    TurboRouter.sol, deposit is payable but doesn't use msg.value
GalloDaSballo commented 2 years ago

Internal and private no longer saves gas I don't believe you can have calldata string and use it Unused imports do not save gas Constructor are calling the parent contract, they are needed AfterDeposit et.all are used because of the inheritance from the base contract External vs public only saves gas if the parameters are dynamic arrays in memory The functions are payable because they could allow the usage of ETH as the underlying asset.

Overall I think the report was a soulless copy-paste from scripts, formatting also leaves a lot to be desired.

Because I've contested every finding, am going to rate this a 0/10