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

DifferenceMomentumUpdateRule Asymmetric Handling of Negative Prices

iepathos

Summary

The DifferenceMomentumUpdateRule contract handles negative prices asymmetrically compared to positive prices, leading to inconsistent weight calculations. This asymmetry is significantly larger than both MomentumUpdateRule and AntiMomentumUpdateRule, with a delta of 0.0875 (compared to 0.01875 and 0.017 respectively). While ChainlinkOracle enforces positive prices (require(data > 0)), negative prices can still occur through the project's MultiHopOracle which performs mathematical operations that may result in negative values. This could cause severe pool imbalances in pools using MultiHopOracle or similar oracles that support negative prices.

Vulnerability Details

Location: pkg/pool-quantamm/contracts/rules/DifferenceMomentumUpdateRule.sol

The issue occurs in the weight calculation where negative prices produce asymmetric results compared to equivalent positive prices:

//1/p(t) · ∂p(t)/∂t used in both the main part of the equation and normalisation
locals.newWeights[locals.i] = ONE.div(locals.denominator).mul(int256(locals.newWeights[locals.i]));

When handling negative prices (which can occur through MultiHopOracle's mathematical operations), the sign propagates through multiple calculations affecting both the normalization factor and final weights. The asymmetry is amplified by the dual moving average system (MACD-style calculation).

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.26;
​
import "forge-std/Test.sol";
import "../../../contracts/mock/MockRuleInvoker.sol";
import "../../../contracts/mock/mockRules/MockDifferenceMomentumRule.sol";
import "../../../contracts/mock/MockPool.sol";
import "../utils.t.sol";
​
contract QuantammDifferenceMomentumNegativePriceTest is Test, QuantAMMTestUtils {
MockDifferenceMomentumRule internal rule;
MockPool internal mockPool;
function setUp() public {
rule = new MockDifferenceMomentumRule(address(this));
mockPool = new MockPool(3600, PRBMathSD59x18.fromInt(1), address(rule));
mockPool.setNumberOfAssets(2);
}
​
function testDifferenceMomentumNegativePriceSymmetry() public {
// Setup initial values for both short and long moving averages
int256[] memory initialValues = new int256[]();
initialValues[0] = 1e18;
initialValues[1] = 1e18;
​
// Empty array for intermediate values
int256[] memory emptyArray = new int256[]();
​
rule.initialisePoolRuleIntermediateValues(
address(mockPool),
initialValues,
emptyArray, // Added intermediate values parameter
mockPool.numAssets()
);
​
int256[] memory prevWeights = new int256[]();
prevWeights[0] = 0.5e18;
prevWeights[1] = 0.5e18;
​
int256[] memory positivePrices = new int256[]();
positivePrices[0] = 2e18;
positivePrices[1] = 1e18;
​
int256[] memory negativePrices = new int256[]();
negativePrices[0] = -2e18;
negativePrices[1] = 1e18;
​
// Parameters: kappa and lambda_short
int256[][] memory parameters = new int256[][]();
parameters[0] = new int256[]();
parameters[0][0] = 0.1e18; // kappa
parameters[1] = new int256[]();
parameters[1][0] = 0.5e18; // lambda_short
​
// Lambda_long for the pool
int128[] memory lambdas = new int128[]();
lambdas[0] = int128(0.1e18); // lambda_long
​
// Test with positive prices
rule.CalculateUnguardedWeights(
prevWeights,
positivePrices,
address(mockPool),
parameters,
lambdas,
initialValues // Using initial values as long-term moving average
);
int256[] memory positiveWeights = rule.GetResultWeights();
​
// Test with negative prices
rule.CalculateUnguardedWeights(
prevWeights,
negativePrices,
address(mockPool),
parameters,
lambdas,
initialValues // Using initial values as long-term moving average
);
int256[] memory negativeWeights = rule.GetResultWeights();
​
// Log results
emit log_named_decimal_int("Positive price weight 0", positiveWeights[0], 18);
emit log_named_decimal_int("Positive price weight 1", positiveWeights[1], 18);
emit log_named_decimal_int("Negative price weight 0", negativeWeights[0], 18);
emit log_named_decimal_int("Negative price weight 1", negativeWeights[1], 18);
​
// Validate weight ranges
require(positiveWeights[0] >= 0 && positiveWeights[0] <= 1e18, "Invalid positive weight range");
require(negativeWeights[0] >= 0 && negativeWeights[0] <= 1e18, "Invalid negative weight range");
// Check symmetry (should be equal for positive and negative prices)
assertApproxEqAbs(positiveWeights[0], negativeWeights[0], 1e16, "Weight asymmetry detected");
}
}

Test Results:

Positive price weight 0: 0.525000000000000000
Positive price weight 1: 0.475000000000000000
Negative price weight 0: 0.437500000000000000
Negative price weight 1: 0.562500000000000000

Note that the asymmetry (0.0875) is significantly larger than in both MomentumUpdateRule (0.01875) and AntiMomentumUpdateRule (0.017). The weight difference is more than 8.7%, which could lead to substantial pool imbalances.

Impact

  • Severe asymmetric weight calculations for positive vs negative prices

  • Largest asymmetry among all momentum-based rules (0.0875 or 8.7%)

  • Inconsistent pool behavior depending on price sign

  • Could lead to significant weight distribution imbalances

  • May create substantial arbitrage opportunities due to predictable asymmetry

  • Breaks mathematical symmetry expected in price calculations

  • Could compound with other weight calculation issues

  • Particularly concerning due to interaction between short and long-term moving averages amplifying the asymmetry

Recommendations

  1. Modify price handling to maintain symmetry:

// Handle sign separately from magnitude
int256 sign = locals.denominator >= 0 ? ONE : -ONE;
int256 magnitude = locals.denominator >= 0 ? locals.denominator : -locals.denominator;
locals.newWeights[locals.i] = sign.mul(ONE.div(magnitude)).mul(int256(locals.newWeights[locals.i]));

  1. Consider architectural improvements:

    • Add explicit sign handling throughout calculations

    • Implement symmetry validation in tests

    • Add invariant checks for price sign handling

    • Consider using absolute values for intermediate calculations

    • Add documentation about price sign handling expectations

    • Consider extracting common price handling logic to a shared library

    • Add specific checks for MACD-style calculations with negative prices

  2. Add comprehensive tests:

    • Test symmetry with various price magnitudes

    • Test edge cases with extreme price values

    • Add property-based tests for price sign handling

    • Test interactions between short and long moving averages

    • Test compounding effects over multiple updates

    • Test interactions with other momentum-based rules

    • Add specific tests for MACD divergence scenarios

Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

invalid_getData_negative_or_zero_price

Multihop will call ChainlinkOracle and the check is in it: `require(data > 0, "INVLDDATA");` MultiHop is just here to combine Chainlinks feed when there is no direct USD price feed for a token.

Support

FAQs

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

Give us feedback!