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

Inefficient if check in validParameters function. Leading to excessive GAS consumption.

theirrationalone

Summary

In the DifferenceMomentumUpdateRule contract, there's a function called validParameters which is used to check whether parameters are valid or not which are necessary to calculate and update weights. The function is all good and doing its job perfectly. However, function contains an inefficient if check.

Vulnerability Details

DifferenceMomentumUpdateRule::validParameters:

function validParameters(int256[][] calldata _parameters) external pure override returns (bool) {
if (_parameters.length != 2) {
return false;
}
​
// lambda has to be less that int128 max value
int256[] memory shortLambda = _parameters[1];
for (uint256 i; i < shortLambda.length;) {
@> // @info: inefficient if statement because
@> // type(int128).max is already greater than int256(1e18)
@> if (shortLambda[i] > int256(type(int128).max)) {
return false;
}
if (shortLambda[i] < int256(0)) {
return false;
}
@> if (shortLambda[i] > int256(1e18)) {
return false;
}
unchecked {
++i;
}
}
​
int256[] memory kappa = _parameters[0];
uint16 valid = uint16(kappa.length) > 0 ? 1 : 0;
for (uint256 i; i < kappa.length;) {
if (kappa[i] == 0) {
unchecked {
valid = 0;
}
break;
}
unchecked {
++i;
}
}
return valid == 1;
}

Did you see the bug above, chisel helped to know that 1e18 is smaller than type(int128).max, int256(1e18) < type(int128).max. So we should only check if shortLambda[i] is greater than int256(1e18) or not, comparing shortLambda[i] > int256(type(int128).max is totally worthless and is a complete waste of GAS.

Impact

Excessive wastage of GAS

Tools Used

Manual review

Recommendations

Please remove that inefficient if check and then we're good to go.

DifferenceMomentumUpdateRule::validParameters:

function validParameters(int256[][] calldata _parameters) external pure override returns (bool) {
if (_parameters.length != 2) {
return false;
}
​
// lambda has to be less that int128 max value
int256[] memory shortLambda = _parameters[1];
for (uint256 i; i < shortLambda.length;) {
- if (shortLambda[i] > int256(type(int128).max)) {
- return false;
- }
if (shortLambda[i] < int256(0)) {
return false;
}
if (shortLambda[i] > int256(1e18)) {
return false;
}
unchecked {
++i;
}
}
​
int256[] memory kappa = _parameters[0];
uint16 valid = uint16(kappa.length) > 0 ? 1 : 0;
for (uint256 i; i < kappa.length;) {
if (kappa[i] == 0) {
unchecked {
valid = 0;
}
break;
}
unchecked {
++i;
}
}
return valid == 1;
}
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!