QuantAMM
QuantAMM
49,600 OP
View results
Previous Next
Submission Details
Severity: low
Invalid

Missing Check for Identical `quantAMMSwapFeeTake` and `quantAMMUpliftFeeTake` Values: Oversight Could Lead to Excessive Gas Consumption and Useless Event Emissions

theirrationalone

Summary

In the UpdateWeightRunner contract, there are functions called setQuantAMMSwapFeeTake and setQuantAMMUpliftFeeTake that sets the swap and uplift fee percentage charged on every swap and uplift operations. However, there is no validation to restrict identical or redundant fee percentage updates. This oversight could result in unnecessary gas consumption and inefficient event emissions.

Vulnerability Details

While this may appear to be an admin input validation issue, there is a high likelihood that an admin might overlook the last updated fee value, especially since this function is not frequently used. Given this context, the chances of omission are high. It would, therefore, be better to include a check to restrict redundant updates.

See the code snippet below:

UpdateWeightRunner::setQuantAMMSwapFeeTake

function setQuantAMMSwapFeeTake(uint256 _quantAMMSwapFeeTake) external override {
require(msg.sender == quantammAdmin, "ONLYADMIN");
require(_quantAMMSwapFeeTake <= 1e18, "Swap fee must be less than 100%");
uint256 oldSwapFee = quantAMMSwapFeeTake;
@> // @info: Missing a check to compare oldSwapFee and newSwapFee. Without this, redundant updates may occur, resulting in inefficient event emissions.
quantAMMSwapFeeTake = _quantAMMSwapFeeTake;
​
emit SwapFeeTakeSet(oldSwapFee, _quantAMMSwapFeeTake);
}

UpdateWeightRunner::setQuantAMMUpliftFeeTake

function setQuantAMMUpliftFeeTake(uint256 _quantAMMUpliftFeeTake) external {
require(msg.sender == quantammAdmin, "ONLYADMIN");
require(_quantAMMUpliftFeeTake <= 1e18, "Uplift fee must be less than 100%");
uint256 oldSwapFee = quantAMMSwapFeeTake;
@> // @info: missing identical oldUpliftFee and newUpliftFee check, there will be inefficient event emission if the values are the same
quantAMMSwapFeeTake = _quantAMMUpliftFeeTake;
emit UpliftFeeTakeSet(oldSwapFee, _quantAMMUpliftFeeTake);
}

Explanation

Functions lack a condition to verify whether the new fee value (_quantAMMSwapFeeTake and _quantAMMUpliftFeeTake) are identical to the current fee value (quantAMMSwapFeeTake and quantAMMUpliftFeeTake). This can lead to:

  • Redundant updates to the fee value.

  • Unnecessary state changes.

  • Emission of redundant events.

Impact

  1. Unnecessary Gas Consumption:
    Redundant updates result in unnecessary state changes, incurring additional gas costs. Even if the new fee value is identical to the existing one, the contract performs a write operation to the blockchain, wasting resources.

  2. Increased Transaction Costs for Users:
    Since gas fees are proportional to the number of operations in a transaction, redundant updates will result in higher transaction costs for users (e.g., admins) without making any meaningful changes to the contract’s state.

  3. Blockchain Bloat:
    Every transaction that modifies the contract's state is recorded on-chain. Allowing redundant updates increases the size of the blockchain unnecessarily, contributing to blockchain bloat. Over time, this can affect Ethereum's scalability and performance.

  4. Unnecessary Event Emissions:
    Emitting events for redundant updates adds unnecessary logs to the blockchain, which consumes additional gas and increases blockchain storage requirements.

  5. Event Log Pollution:
    Redundant events clutter the transaction logs, making it harder to query meaningful changes. This can confuse users or developers who rely on these logs to monitor the contract's behavior.

Tools Used

Manual Review

Recommendations

Add a condition to restrict redundant and identical fee percentage updates. This will ensure efficient gas usage and prevent unnecessary event emissions.

Suggested Code Fix

Below is the updated code with the necessary validation added:

function setQuantAMMSwapFeeTake(uint256 _quantAMMSwapFeeTake) external override {
require(msg.sender == quantammAdmin, "ONLYADMIN");
require(_quantAMMSwapFeeTake <= 1e18, "Swap fee must be less than 100%");
+ require(_quantAMMSwapFeeTake != quantAMMSwapFeeTake, "SWAP_FEE_PERCENTAGE_NOT_CHANGED");
uint256 oldSwapFee = quantAMMSwapFeeTake;
quantAMMSwapFeeTake = _quantAMMSwapFeeTake;
​
emit SwapFeeTakeSet(oldSwapFee, _quantAMMSwapFeeTake);
}
​
function setQuantAMMUpliftFeeTake(uint256 _quantAMMUpliftFeeTake) external {
require(msg.sender == quantammAdmin, "ONLYADMIN");
require(_quantAMMUpliftFeeTake <= 1e18, "Uplift fee must be less than 100%");
+ require(_quantAMMUpliftFeeTake != quantAMMUpliftFeeTake, "UPLIFT_FEE_PERCENTAGE_NOT_CHANGED");
uint256 oldSwapFee = quantAMMSwapFeeTake;
quantAMMSwapFeeTake = _quantAMMUpliftFeeTake;
emit UpliftFeeTakeSet(oldSwapFee, _quantAMMUpliftFeeTake);
}

Benefits of the Fix

  1. Prevents redundant state updates and event emissions.

  2. Saves gas costs by avoiding unnecessary write operations.

  3. Reduces blockchain bloat and ensures meaningful event logs.

  4. Improves contract efficiency and user experience.

Here’s the grammatically corrected and polished version of your markdown:

Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational or Gas / Admin is trusted / Pool creation is trusted / User mistake / Suppositions

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelyhood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Support

FAQs

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

Give us feedback!