Tribe Turbo contest - Dravee's results

QA Report

Table of Contents:

• QA Report
  • Foreword
  • Summary
  • File: TurboMaster.sol
    • event TokenSweeped()
      • Missing @param token
  • File: TurboSafe.sol
    • event TokenSweeped()
      • Missing @param token
  • File: TurboBooster.sol
    • event BoostCapUpdatedForVault()
      • Missing @param user
    • event BoostCapUpdatedForCollateral()
      • Missing @param user
  • File: TurboClerk.sol
    • event DefaultFeePercentageUpdated()
      • Missing @param user
    • event CustomFeePercentageUpdatedForCollateral()
      • Missing @param user
    • event CustomFeePercentageUpdatedForSafe()
      • Missing @param user
  • File: TurboSavior.sol
    • event MinDebtPercentageForSavingUpdated()
      • Missing @param user

Foreword

• @audit tags

The code is annotated at multiple places with //@audit comments to pinpoint the issues. Please, pay attention to them for more details.

Summary

• It feels like the code was already audited, well done!
• This report only contains some minor missing comments (@param userand @param token)

File: TurboMaster.sol

event TokenSweeped()

Missing @param token

308:     /// @notice Emitted a token is sweeped from the Master.
309:     /// @param user The user who sweeped the token from the Master.
310:     /// @param to The recipient of the sweeped tokens. //@audit missing @param token 
311:     /// @param amount The amount of the token that was sweeped.
312:     event TokenSweeped(address indexed user, address indexed to, ERC20 indexed token, uint256 amount);

File: TurboSafe.sol

event TokenSweeped()

Missing @param token

296:     /// @notice Emitted a token is sweeped from the Safe.
297:     /// @param user The user who sweeped the token from the Safe.
298:     /// @param to The recipient of the sweeped tokens. //@audit missing @param token
299:     /// @param amount The amount of the token that was sweeped.
300:     event TokenSweeped(address indexed user, address indexed to, ERC20 indexed token, uint256 amount);

File: TurboBooster.sol

event BoostCapUpdatedForVault()

Missing @param user

51:     /// @notice Emitted when a Vault's boost cap is updated.
52:     /// @param vault The Vault who's boost cap was updated.
53:     /// @param newBoostCap The new boost cap for the Vault.  //@audit missing @param user
54:     event BoostCapUpdatedForVault(address indexed user, ERC4626 indexed vault, uint256 newBoostCap);

There don't seem to be a sensible reason to not mention @param user. It was previously mentioned, and a similar event does in fact mention it too:

File: TurboSavior.sol
89:     /// @notice Emitted a save is executed.
90:     /// @param user The user who executed the save. //@audit-ok user
91:     /// @param safe The Safe that was saved.
92:     /// @param vault The Vault that was lessed.
93:     /// @param feiAmount The amount of Fei that was lessed.
94:     event SafeSaved(address indexed user, TurboSafe indexed safe, ERC4626 indexed vault, uint256 feiAmount);

The missing comment should therefore be added

event BoostCapUpdatedForCollateral()

Missing @param user

73:     /// @notice Emitted when a collateral type's boost cap is updated.
74:     /// @param collateral The collateral type who's boost cap was updated.//@audit missing @param user
75:     /// @param newBoostCap The new boost cap for the collateral type.
76:     event BoostCapUpdatedForCollateral(address indexed user, ERC20 indexed collateral, uint256 newBoostCap);

File: TurboClerk.sol

event DefaultFeePercentageUpdated()

Missing @param user

30:     /// @notice Emitted when the default fee percentage is updated.
31:     /// @param newDefaultFeePercentage The new default fee percentage. //@audit missing @param user
32:     event DefaultFeePercentageUpdated(address indexed user, uint256 newDefaultFeePercentage);

event CustomFeePercentageUpdatedForCollateral()

Missing @param user

58:     /// @notice Emitted when a collateral's custom fee percentage is updated.
59:     /// @param collateral The collateral who's custom fee percentage was updated. //@audit missing @param user
60:     /// @param newFeePercentage The new custom fee percentage.
61:     event CustomFeePercentageUpdatedForCollateral(
62:         address indexed user,
63:         ERC20 indexed collateral,
64:         uint256 newFeePercentage
65:     );

event CustomFeePercentageUpdatedForSafe()

Missing @param user

80:     /// @notice Emitted when a Safe's custom fee percentage is updated.
81:     /// @param safe The Safe who's custom fee percentage was updated.//@audit missing @param user
82:     /// @param newFeePercentage The new custom fee percentage.
83:     event CustomFeePercentageUpdatedForSafe(address indexed user, TurboSafe indexed safe, uint256 newFeePercentage);

File: TurboSavior.sol

event MinDebtPercentageForSavingUpdated()

Missing @param user

File: TurboSavior.sol
69:     /// @notice Emitted when the minimum debt percentage for saving is updated.
70:     /// @param newDefaultFeePercentage The new minimum debt percentage for saving. //@audit missing @param user
71:     event MinDebtPercentageForSavingUpdated(address indexed user, uint256 newDefaultFeePercentage);

Gas Report

Table of Contents:

• Gas Report
  • Summary
  • Recommendation
    • Use Custom Errors instead of Revert Strings to save Gas

Summary

This code is amazingly optimized! Well done! I only have 1 suggestion: using custom errors consistently across the solution.

Recommendation

Use Custom Errors instead of Revert Strings to save Gas

Custom errors from Solidity 0.8.4 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.

Custom errors are defined using the error statement, which can be used inside and outside of contracts (including interfaces and libraries).

Instances include:

lib/solmate/ERC4626.sol:49:        require((shares = previewDeposit(amount)) != 0, "ZERO_SHARES");
lib/solmate/ERC4626.sol:106:        require((amount = previewRedeem(shares)) != 0, "ZERO_ASSETS");
src/modules/TurboClerk.sol:38:        require(newDefaultFeePercentage <= 1e18, "FEE_TOO_HIGH");
src/modules/TurboClerk.sol:72:        require(newFeePercentage <= 1e18, "FEE_TOO_HIGH");
src/modules/TurboClerk.sol:90:        require(newFeePercentage <= 1e18, "FEE_TOO_HIGH");
src/modules/TurboGibber.sol:89:        require(feiTurboCToken.repayBorrowBehalf(address(safe), feiAmount) == 0, "REPAY_FAILED");
src/modules/TurboGibber.sol:117:        require(feiTurboCToken.repayBorrowBehalf(address(safe), feiAmount) == 0, "REPAY_FAILED");
src/modules/TurboSavior.sol:77:        require(newMinDebtPercentageForSaving <= 1e18, "PERCENT_TOO_HIGH");
src/modules/TurboSavior.sol:129:        require(
src/TurboMaster.sol:208:        require(getSafeId[safe] != 0, "INVALID_SAFE");
src/TurboMaster.sol:232:        require(
src/TurboMaster.sol:258:        require(getSafeId[safe] != 0, "INVALID_SAFE");
src/TurboMaster.sol:288:        require(getSafeId[safe] != 0, "INVALID_SAFE");
src/TurboRouter.sol:38:        require(msg.sender == Auth(target).owner(), "NOT_AUTHED");
src/TurboSafe.sol:70:        require(asset != fei, "INVALID_ASSET");
src/TurboSafe.sol:79:        require(address(assetTurboCToken) != address(0), "UNSUPPORTED_ASSET");
src/TurboSafe.sol:86:        require(pool.enterMarkets(marketsToEnter)[0] == 0, "ENTER_MARKETS_FAILED");
src/TurboSafe.sol:121:                require(
src/TurboSafe.sol:140:        require(assetTurboCToken.mint(assetAmount) == 0, "MINT_FAILED");
src/TurboSafe.sol:148:        require(assetTurboCToken.redeemUnderlying(assetAmount) == 0, "REDEEM_FAILED");
src/TurboSafe.sol:173:        require(vault.asset() == fei, "NOT_FEI");
src/TurboSafe.sol:191:        require(feiTurboCToken.borrow(feiAmount) == 0, "BORROW_FAILED");
src/TurboSafe.sol:233:        if (feiAmount != 0) require(feiTurboCToken.repayBorrow(feiAmount) == 0, "REPAY_FAILED");
src/TurboSafe.sol:260:        require(getTotalFeiBoostedForVault[vault] != 0, "NO_FEI_BOOSTED");
src/TurboSafe.sol:312:        require(getTotalFeiBoostedForVault[ERC4626(address(token))] == 0 && token != assetTurboCToken, "INVALID_TOKEN");
src/TurboSafe.sol:339:        require(assetTurboCToken.redeemUnderlying(assetAmount) == 0, "REDEEM_FAILED");

I suggest replacing revert strings with custom errors.