QuantAMM
QuantAMM
49,600 OP
View results
Previous Next
Submission Details
Severity: medium
Valid

`UpdateWeightRunner::setQuantAMMUpliftFeeTake` and `setQuantAMMSwapFeeTake` Modify the Same State Variable

4rdiii

Summary

The functions setQuantAMMUpliftFeeTake and setQuantAMMSwapFeeTake both modify the same state variable, quantAMMSwapFeeTake. If there is no distinct variable for the uplift fee (e.g., quantAMMUpliftFeeTake), this is likely an oversight. The current implementation leads to redundant and potentially confusing code, as both functions appear to manage different fees but share the same state variable.

If these functions are truly intended to manage different values, a new state variable must be introduced. Otherwise, setQuantAMMUpliftFeeTake and its associated getter should be removed to eliminate redundancy.

Vulnerability Details

Code Analysis

The state variable quantAMMSwapFeeTake is modified by both functions:

setQuantAMMSwapFeeTake Function:

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

setQuantAMMUpliftFeeTake Function:

function setQuantAMMUpliftFeeTake(uint256 _quantAMMUpliftFeeTake) external {
require(msg.sender == quantammAdmin, "ONLYADMIN");
require(_quantAMMUpliftFeeTake <= 1e18, "Uplift fee must be less than 100%");
uint256 oldSwapFee = quantAMMSwapFeeTake;
quantAMMSwapFeeTake = _quantAMMUpliftFeeTake;
​
emit UpliftFeeTakeSet(oldSwapFee, _quantAMMUpliftFeeTake);
}

Both functions modify the variable quantAMMSwapFeeTake and use it inconsistently in their respective getter functions.

Impact

  • Code Redundancy: setQuantAMMUpliftFeeTake and getQuantAMMUpliftFeeTake add no new functionality but create confusion.

  • Loss of Purpose: The distinction between swap fees and uplift fees is lost, potentially leading to errors in the protocol’s operation.

  • Misleading Emit Events: The emitted events (SwapFeeTakeSet and UpliftFeeTakeSet) suggest different actions but result in identical state changes.

Recommendations

Option 1: Introduce a New State Variable for the Uplift Fee

If these fees are intended to represent different metrics, introduce a new state variable (e.g., quantAMMUpliftFeeTake) for the uplift fee. Update the associated functions accordingly:

uint256 public quantAMMUpliftFeeTake = 0.5e18;
​
function setQuantAMMUpliftFeeTake(uint256 _quantAMMUpliftFeeTake) external {
require(msg.sender == quantammAdmin, "ONLYADMIN");
require(_quantAMMUpliftFeeTake <= 1e18, "Uplift fee must be less than 100%");
uint256 oldUpliftFee = quantAMMUpliftFeeTake;
quantAMMUpliftFeeTake = _quantAMMUpliftFeeTake;
​
emit UpliftFeeTakeSet(oldUpliftFee, _quantAMMUpliftFeeTake);
}
​
function getQuantAMMUpliftFeeTake() external view returns (uint256) {
return quantAMMUpliftFeeTake;
}

Option 2: Remove the Uplift Fee Functions

If no distinct variable for the uplift fee is intended, remove setQuantAMMUpliftFeeTake and getQuantAMMUpliftFeeTake entirely:

// Remove these functions if they are redundant
// function setQuantAMMUpliftFeeTake(uint256 _quantAMMUpliftFeeTake) external { ... }
// function getQuantAMMUpliftFeeTake() external view returns (uint256) { ... }
Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_quantAMMSwapFeeTake==quantAMMUplfitFeeTake

Likelyhood: High, calling setters or getters Impact: Low/Medium, both getters return `quantAMMSwapFeeTake` and `setQuantAMMUpliftFeeTake` modify `quantAMMUplfitFeeTake`. Real impact: those 2 values will be always the same.

Support

FAQs

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