The Standard

The Standard
DeFiHardhat
20,000 USDC
View results
Submission Details
Severity: low
Invalid

Memory Optimization, Gas-Efficient Error Handling, and Pragma Version Locking

Summary

This code review highlights Three areas for improvement: 1) Record both old and new values for critical parameter changes without unnecessary caching to reduce memory usage and potentially improve code readability; 2) Prioritize error over require and assert for more gas-efficient error handling.3) Consider locking pragma versions in LiquidationPool and LiquidationPoolManager contracts for consistency and security.

Vulnerability Details

1.Improve logic in setter function

Record both old and new values: When a critical parameter is modified, it is recommended to record both its previous state and its updated value. This creates a detailed change history, enabling accurate tracking of parameter modifications, informed analysis of change patterns and potential rollback to a previous state if required.

Eliminate unnecessary caching: When the old value is only required for immediate actions (e.g., event emission), access it directly within the setter to streamline code and potentially reduce memory overhead. This optimization can improve code readability and potentially reduce memory usage.

LiquidationPoolManager::setPoolFeePercentage

function setPoolFeePercentage(uint32 _poolFeePercentage) external onlyOwner {
+ emit PoolFeePercentageChanged(poolFeePercentage, _poolFeePercentage)
poolFeePercentage = _poolFeePercentage;
}

All other instances entailed:

SmartVaultManagerV5::setMintFeeRate, setBurnFeeRate, setSwapFeeRate

function setMintFeeRate(uint256 _rate) external onlyOwner {
+ emit MintFeeRateChanged(mintFeeRate, _rate)
mintFeeRate = _rate;
}
function setBurnFeeRate(uint256 _rate) external onlyOwner {
+ emit BurnFeeRateChanged(burnFeeRate, _rate)
burnFeeRate = _rate;
}
function setSwapFeeRate(uint256 _rate) external onlyOwner {
+ emit SwapFeeRateChanged(swapFeeRate, _rate)
swapFeeRate = _rate;
}

SmartVaultV3::setOwner

function setOwner(address _newOwner) external onlyVaultManager {
+ emit OwnerChanged(owner, _newOwner)
owner = _newOwner;
}

2.Fuel Efficient Error Handling for Gas Optimization

In Solidity, there are three ways to throw an exception: error, require, and assert. error is the most gas-efficient, followed by assert, and require is the most gas-consuming. Therefore, error is a good choice for both providing informative error messages to users and saving gas.

Error statements can provide more detailed error messages than require statements. This can be helpful for developers debugging their contracts or users troubleshooting problems. It can be used to handle errors more gracefully than require statements and it can be more gas-efficient than require statements.

LiquidationPool::onlyManager

modifier onlyManager {
+ if (msg.sender != manager) revert InvalidUser();
- require(msg.sender == manager, "err-invalid-user");
_;
}

All other instances entailed:

SmartVaultV3::onlyManager, onlyVaultManager, ifMinted, ifNotLiquidated

modifier onlyVaultManager {
+ if (msg.sender != manager) revert InvalidUser();
- require(msg.sender == manager, INVALID_USER);
_;
}
modifier onlyOwner {
+ if (msg.sender != manager) revert InvalidUser();
- require(msg.sender == owner, INVALID_USER);
_;
}
modifier ifMinted(uint256 _amount) {
+ if (minted < _amount) revert InsuffMinted();
- require(minted >= _amount, "err-insuff-minted");
_;
}
modifier ifNotLiquidated {
+ if (liquidated) revert Liquidated();
- require(!liquidated, "err-liquidated");
_;
}

SmartVaultV3ManagerV5::onlyLiquidator

modifier onlyLiquidator {
+ if (msg.sender != liquidator) revert InvalidLiquidator();
- require(msg.sender == liquidator, "err-invalid-liquidator");
_;
}

3.Pragma float in LiquidationPool and Liquidation Pool Manager

Both LiquidationPool and Liquidation Pool Manager contracts are floating the pragma version. Consider locking the pragma versions to ensure consistent compilation and avoid potential issues with outdated compiler features or vulnerabilities.

If these contracts are specifically designed for use by other developers (e.g., as libraries or packages), floating pragmas might be acceptable to accommodate different project setups.

LiquidationPool
Liquidation Pool Manager

+ pragma solidity 0.8.17;
- pragma solidity ^0.8.17;

Impact

Limited Tracing and Analysis: Without capturing both old and new values, accurate tracking of parameter changes, analyzing trends, and diagnosing issues becomes challenging. Reconstructing previous states becomes problematic in case of needed rollbacks, leading to delays, inaccuracies, or even an inability to revert.
Inefficient Error Handling: Reliance on require might incur higher gas costs and less informative error messages.
Security Concerns: Floating pragmas could expose contracts to vulnerabilities associated with outdated compiler versions. For libraries and packages, floating pragmas might be acceptable to allow project-specific compiler choice and seamless integration.

Tools Used

Manual

Recommendations

Capture Critical Changes by Optimized Memory Usage: Capture both old and new values for critical parameters by directly accessing old values within setter functions when feasible.
Prioritize error for exception handling: Utilize error statements for informative messages and potential gas savings.
Review pragma versions: Assess locking or floating pragmas based on contract usage and security considerations.

Updates

Lead Judging Commences

hrishibhat Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

informational/invalid

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!