TeaVaultV3PortfolioHelper Code Style Findings

TVH-01C: Deprecated Revert Statements

Type	Severity	Location
Code Style		TeaVaultV3PortfolioHelper.sol:L411, L416, L439, L444, L463

Description:

The referenced statements represent empty revert instructions which are deprecated.

Example:

Copy
437if (success) {
438    // shouldn't happen, revert
439    revert();
440}
441else {
442    if (returndata.length == 0) {
443        // no result, revert
444        revert();
445    }
446
447    (depositedAmount0, depositedAmount1) = abi.decode(returndata, (uint256, uint256));
448}

Recommendation:

We advise custom error declarations to be introduced and emitted in each case, clearly depicting why the revert statement is issued and increasing code clarity.

Alleviation (302b96f324a88038a0872015466cd43783c14543):

The Teahouse Finance team stated that they have opted to use revert statements due to the functions' simulating behaviour. As such, we consider this exhibit nullified as we consider it standard to retain revert statements in such instances.

TVH-02C: Generic Typographic Mistake

Type	Severity	Location
Code Style		TeaVaultV3PortfolioHelper.sol:L32

Description:

The referenced line contains a typographical mistake (i.e. private variable without an underscore prefix) or generic documentational error (i.e. copy-paste) that should be corrected.

Example:

Copy
32address private vault;

Recommendation:

We advise this to be done so to enhance the legibility of the codebase.

Alleviation (302b96f324a88038a0872015466cd43783c14543):

The Teahouse Finance team stated that an underscore introduced to the vault variable would cause a conflict with the input argument of the TeaVaultV3PortfolioHelper::multicall function.

We would like to note that input arguments are usually suffixed with an underscore rather than prefixed for this exact purpose, and consider the exhibit acknowledged.

TVH-03C: Loop Iterator Optimizations

Type	Severity	Location
Gas Optimization		TeaVaultV3PortfolioHelper.sol:L64, L103, L128, L149, L155, L168, L176, L193, L205, L380

Description:

The linked for loops increment / decrement their iterator "safely" due to Solidity's built - in safe arithmetics (post-0.8.X).

Example:

Copy
64for (uint256 i; i < tokensLength; i++) {

Recommendation:

We advise the increment / decrement operations to be performed in an unchecked code block as the last statement within each for loop to optimize their execution cost.

Alleviation (302b96f324a88038a0872015466cd43783c14543):

The referenced loop iterator increment statements have been relocated at the end of each respective for loop's body and have been unwrapped in an unchecked code block, optimizing their gas cost.

TVH-04C: Potentially Redundant Rescue Functions

Type	Severity	Location
Code Style		TeaVaultV3PortfolioHelper.sol:L388-L391, L393-L396

Description:

The referenced rescue functions can already be simulated via the TeaVaultV3PortfolioHelper::multicall function by supplying a single call to TeaVaultV3PortfolioHelper::convertWETH followed by a call to TeaVaultV3PortfolioHelper::refundTokens with the tokens desired to be rescued.

Example:

Copy
367/// @inheritdoc ITeaVaultV3PortfolioHelper
368function convertWETH() external payable onlyInMulticall {
369    uint256 balance = weth9.balanceOf(address(this));
370    if (balance > 0) {
371        weth9.withdraw(balance);
372    }
373}
374
375/// @inheritdoc ITeaVaultV3PortfolioHelper
376function refundTokens(
377    address[] calldata _tokens
378) external payable onlyInMulticall {
379    uint256 tokensLength = _tokens.length;
380    for (uint256 i; i < tokensLength; i++) {
381        uint256 balance = IERC20(_tokens[i]).balanceOf(address(this));
382        if (balance > 0) {
383            IERC20(_tokens[i]).safeTransfer(msg.sender, balance);
384        }
385    }
386}    
387
388/// @inheritdoc ITeaVaultV3PortfolioHelper
389function rescueEth(uint256 _amount) external onlyOwner {
390    Address.sendValue(payable(msg.sender), _amount);
391}
392
393/// @inheritdoc ITeaVaultV3PortfolioHelper
394function rescueFund(address _token, uint256 _amount) external onlyOwner {
395    IERC20(_token).safeTransfer(msg.sender, _amount);
396}

Recommendation:

We advise the functions to be revisited and potentially omitted, optimizing the code's overall structure.

Alleviation (302b96f324a88038a0872015466cd43783c14543):

The Teahouse Finance team acknowledged that the rescue functions are potentially redundant but they wish to retain the code as is.

TVH-05C: Unreachable Code

Type	Severity	Location
Code Style		TeaVaultV3PortfolioHelper.sol:L135-L137

Description:

The referenced else clause is unreachable as the if-else-if chain evaluates all possible cases of an enum.

Example:

Copy
111if (_vaultType == VaultType.TeaVaultV3Pair) {
112    IERC20 token0 = IERC20(ITeaVaultV3Pair(_vault).assetToken0());
113    IERC20 token1 = IERC20(ITeaVaultV3Pair(_vault).assetToken1());
114
115    uint256 balance = token0.balanceOf(address(this));
116    if (balance > 0) {
117        token0.safeTransfer(msg.sender, balance);
118    }
119
120    balance = token1.balanceOf(address(this));
121    if (balance > 0) {
122        token1.safeTransfer(msg.sender, balance);
123    }
124}
125else if (_vaultType == VaultType.TeaVaultV3Portfolio) {
126    ERC20Upgradeable[] memory vaultTokens = ITeaVaultV3Portfolio(vault).getAssets();
127    tokensLength = vaultTokens.length;
128    for (uint256 i; i < tokensLength; i++) {
129        uint256 balance = vaultTokens[i].balanceOf(address(this));
130        if (balance > 0) {
131            vaultTokens[i].safeTransfer(msg.sender, balance);
132        }
133    }
134}
135else {
136    revert InvalidVaultType();
137}

Recommendation:

We advise it to be omitted, optimizing the code's overall bytecode and clarity.

Alleviation (302b96f324a88038a0872015466cd43783c14543):

The unreachable code has been commented out, highlighting that the compiler will ensure the input _vaultType is a valid enum value. As dead code is no longer present in the contract's code, we consider this exhibit addressed.