sherlock-audit / 2023-01-ajna-judging

1 stars 0 forks source link

minhtrng - Incorrect EMA implementation leads to faulty interest rate updates #166

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

minhtrng

medium

Incorrect EMA implementation leads to faulty interest rate updates

Summary

Values in the EMA calculation are swapped compared to the whitepaper/specification. This will lead to EMA adjusting much slower to current values, leading to delayed interest updates.

Vulnerability Detail

The EMA calculation is described in the whitepaper like this:

$\text{EMA}_{t+12hr}(x) = (1-\lambda)\text{EMA}_t(x) + \lambdax_{t+12hr}$

However, in the actual implementation the values for $\text{EMA}t(x)$ and $x{t+12hr}$ are swapped:

//EMA_7D_RATE_FACTOR is equivalent to (1-lambda)
curDebtEma =
    Maths.wmul(poolState_.debt,  EMA_7D_RATE_FACTOR) +
    Maths.wmul(curDebtEma,       LAMBDA_EMA_7D
);

...

curLupColEma =
    Maths.wmul(Maths.min(lupCol, maxLupColEma), EMA_7D_RATE_FACTOR) +
    Maths.wmul(curLupColEma,                    LAMBDA_EMA_7D);

This will lead to the old EMA account for ~90% of the new EMA value, which will cause the EMA to lag behind the current value much stronger. This once again leads to the interest rate updates being delayed/wrong (i.e. updating when it shouldnt), due to using a much delayed value for target utilization.

Impact

Faulty interest rate updates.

Code Snippet

https://github.com/sherlock-audit/2023-01-ajna/blob/c9228034c74c03b42c3e10ea9c7b83c201074ab6/contracts/src/libraries/external/PoolCommons.sol#L75-L78 https://github.com/sherlock-audit/2023-01-ajna/blob/c9228034c74c03b42c3e10ea9c7b83c201074ab6/contracts/src/libraries/external/PoolCommons.sol#L86-L88

Tool used

Manual Review

Recommendation

Swap EMA_7D_RATE_FACTOR and LAMBDA_EMA_7D in both calculations shown above.

grandizzy commented 1 year ago

will fix by updating whitepaper to reflect the current implementation

hrishibhat commented 1 year ago

Considering this issue as a low, since the whitepaper needs to be updated as per the implementation