StrategyV5 Code Style Findings

SV5-01C: Generic Typographic Mistake

Type	Severity	Location
Code Style		StrategyV5.sol:L467, L474

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
467* @param _amount Amount of asset to liquidate with (0 == totalPendingAssetRequest() + allocated.bp(100))

Recommendation:

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

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

The documentation was updated to reflect the 150 number that used to be utilized in the instance of the codebase during the preliminary report, however, the latest instance utilizes the 50 value as an overallocation.

As such, we advise the documentation to be updated so as to reflect this adjustment.

SV5-02C: Improper Declarations of Abstract Functions

Type	Severity	Location
Standard Conformity		StrategyV5.sol:L109-L112, L160-L162, L238-L241, L309, L316-L319, L326-L329, L336-L339, L346-L349, L356-L358, L366, L374, L524-L529

Description:

The referenced functions are meant to be virtual and implemented by derivative implementations, however, an empty declaration is present in both that would permit each to be invoked.

Example:

Copy
103/**
104 * @notice Withdraw asset function, can remove all funds in case of emergency
105 * @param _amounts Amounts of asset to withdraw
106 * @param _params Swaps calldata
107 * @return assetsRecovered Amount of asset withdrawn
108 */
109function _liquidate(
110    uint256[8] calldata _amounts, // from previewLiquidate()
111    bytes[] memory _params
112) internal virtual returns (uint256 assetsRecovered) {}

Recommendation:

We advise the functions to be declared without a code block ({}) to ensure they are overridden by derivative implementations.

Alleviation (59b75fbee1):

While some functions have been properly implemented thus rendering the empty code block observation no longer applicable, functions such as StrategyV5::_stake remain with an empty code block rendering this exhibit partially alleviated.

Alleviation (efbeab6478):

All functions have been properly updated to no longer implement a code block where applicable, rendering this exhibit fully addressed.

SV5-03C: Ineffectual Usage of Safe Arithmetics

Type	Severity	Location
Language Specific		StrategyV5.sol:L484, L507

Description:

The linked mathematical operation is guaranteed to be performed safely by surrounding conditionals evaluated in either require checks or if-else constructs.

Example:

Copy
481if (need > _amount)
482    need = _amount;
483amounts[i] = _assetToInput(need, i);
484_amount -= need;

Recommendation:

Given that safe arithmetics are toggled on by default in pragma versions of 0.8.X, we advise the linked statement to be wrapped in an unchecked code block thereby optimizing its execution cost.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

An unchecked code block has been safely introduced in both referenced instances, optimizing the code's gas cost.

SV5-04C: Inefficient Iterator Type

Type	Severity	Location
Gas Optimization		StrategyV5.sol:L186, L381, L432, L461, L477, L500

Description:

The referenced for loops utilize a uint8 variable as an iterator which is inefficient.

Example:

Copy
186for (uint8 i = 0; i < rewardLength; i++) {

Recommendation:

As the EVM is built to operate on 32-byte (256-bit) data types, we advise the iterator types to be bumped to uint256, optimizing their gas cost.

Alleviation (59b75fbee1):

Most of the instances have been properly upcast to optimize them, however, the StrategyV5::_invest and StrategyV5::_liquidate functions continue to use suboptimal operators rendering this exhibit partially alleviated.

Alleviation (efbeab6478):

The uint8 iterators in the StrategyV5::_invest and StrategyV5::_liquidate functions have been updated to their upcasted format, applying the described optimization in full.

SV5-05C: Loop Iterator Optimizations

Type	Severity	Location
Gas Optimization		StrategyV5.sol:L186, L381, L432, L461, L477, L500

Description:

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

Example:

Copy
186for (uint8 i = 0; i < rewardLength; 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 (59b75fbee1d8f3dee807c928f18be41c58b904e1):

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.

SV5-06C: Redundant Application of Access Control

Type	Severity	Location
Gas Optimization		StrategyV5.sol:L298

Description:

The StrategyV5::compound function redundantly applies the AsManageable::onlyKeeper modifier when its inner calls will perform the same validation (StrategyV5::compound -> StrategyV5::_compound -> StrategyV5::harvest -> StrategyV5::_harvest -> StrategyV5::_swapRewards).

Example:

Copy
286/**
287 * @notice Executes the compound operation in the strategy
288 * @param _amounts Amounts of inputs to compound (in asset, after harvest-> should include rewards)
289 * @param _params Generic callData for the compound operation
290 * @return iouReceived IOUs received from the compound operation
291 * @return harvestedRewards Amount of rewards harvested
292 */
293function compound(
294    uint256[8] calldata _amounts,
295    bytes[] memory _params
296)
297    external
298    onlyKeeper
299    returns (uint256 iouReceived, uint256 harvestedRewards)
300{
301    (iouReceived, harvestedRewards) = _compound(_amounts, _params);
302    emit Compound(iouReceived, block.timestamp);
303}

Recommendation:

We advise access control to be solely applied to the innermost functions, ensuring that restrictions are optimally applied.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

The access control inefficiency remains in the codebase rendering this exhibit acknowledged.

SV5-07C: Redundant Parenthesis Statement

Type	Severity	Location
Code Style		StrategyV5.sol:L147

Description:

The referenced statement is redundantly wrapped in parenthesis (()).

Example:

Copy
147if ((liquidityAvailable < _minLiquidity) && !_panic)

Recommendation:

We advise them to be safely omitted, increasing the legibility of the codebase.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

The redundant parenthesis in the referenced statement have been safely omitted.