VaultPriceManager Code Style Findings

VPM-01C: Inefficient Event Argument

Type	Severity	Location
Gas Optimization		VaultPriceManager.sol:L264, L269

Description:

The referenced LimitsUpdated event will fetch the limits[_vault] data entry from storage even though a local variable containing its data points is already present in memory.

Example:

Copy
260function _setLimits(address _vault, Limit memory _limit) internal {
261    require(_limit.jump <= 1e18 && _limit.drawdown <= 1e18, InvalidLimit());
262
263
264    Limit memory oldLimit = limits[_vault];
265    if (
266        _limit.jump != oldLimit.jump || 
267        _limit.drawdown != oldLimit.drawdown
268    ) {
269        emit LimitsUpdated(_vault, limits[_vault], _limit);
270        limits[_vault] = _limit;
271    }
272}

Recommendation:

We advise the oldLimit variable to be emitted instead, optimizing the code's gas cost.

Alleviation (28f27853965de07fb79f4f2b5fed696d35120032):

The event emission was updated to utilize the local memory limit variable, optimizing its gas cost significantly.

VPM-02C: Inefficient Imposition of Access Control

Type	Severity	Location
Gas Optimization		VaultPriceManager.sol:L243, L253, L260

Description:

The VaultPriceManager::setLimits function implementations both impose the Ownable2Step::onlyOwner modifier independently even though both end up invoking the internal VaultPriceManager::_setLimits function.

Example:

Copy
240/// @notice Set vault price limits
241/// @param _vault Vault address
242/// @param _limit Price limits to set
243function setLimits(address _vault, Limit memory _limit) external onlyOwner {
244    _setLimits(_vault, _limit);
245}
246
247/// @notice Set price limits for multiple vaults
248/// @param _vaults Array of vault addresses
249/// @param _limits Array of price limits
250function setLimits(
251    address[] memory _vaults,
252    Limit[] memory _limits
253) external onlyOwner {
254    require(_vaults.length == _limits.length, InputLengthMismatch());
255    for (uint256 i; i < _vaults.length; i++) {
256        _setLimits(_vaults[i], _limits[i]);
257    }
258}
259
260function _setLimits(address _vault, Limit memory _limit) internal {

Recommendation:

We advise the VaultPriceManager::_setLimits function to be invoked, optimizing the code's bytecode size.

Alleviation (28f27853965de07fb79f4f2b5fed696d35120032):

The Ultra Yield team evaluated this exhibit and clarified that the VaultPriceManager::setLimits loop function variant would incur a higher gas cost due to imposing the access control several times in a single loop.

As such, we consider the gas benefit to outweigh the bytecode size minimization in this case and thus assess this exhibit as invalid.

VPM-03C: Inefficient mapping Lookups

Type	Severity	Location
Gas Optimization		VaultPriceManager.sol:L223, L225

Description:

The linked statements perform key-based lookup operations on mapping declarations from storage multiple times for the same key redundantly.

Example:

Copy
218function setAdmin(
219    address _vault,
220    address _admin,
221    bool _isAdmin
222) external onlyOwner {
223    if (isAdmin[_vault][_admin] != _isAdmin) {
224        emit AdminUpdated(_vault, _admin, _isAdmin);
225        isAdmin[_vault][_admin] = _isAdmin;
226    }
227}

Recommendation:

As the lookups internally perform an expensive keccak256 operation, we advise the lookups to be cached wherever possible to a single local declaration that either holds the value of the mapping in case of primitive types or holds a storage pointer to the struct contained.

As the compiler's optimizations may take care of these caching operations automatically at-times, we advise the optimization to be selectively applied, tested, and then fully adopted to ensure that the proposed caching model indeed leads to a reduction in gas costs.

Alleviation (28f27853965de07fb79f4f2b5fed696d35120032):

The Ultra Yield team opted to acknowledge this exhibit in favour of code legibility.

VPM-04C: Repetitive Value Literal

Type	Severity	Location
Code Style		VaultPriceManager.sol:L178, L181, L184, L261

Description:

The linked value literal is repeated across the codebase multiple times.

Example:

Copy
178lastPrice.mulDivDown(1e18 - limit.jump, 1e18) ||

Recommendation:

We advise it to be set to a constant variable instead, optimizing the legibility of the codebase.

In case the constant has already been declared, we advise it to be properly re-used across the code.

Alleviation (28f27853965de07fb79f4f2b5fed696d35120032):

The referenced value literal 1e18 has been properly relocated to a contract-level constant declaration labelled SCALE, optimizing the code's legibility.