Sherlock-104: PoolCommons._calculateInterestRate make decrease rate condition inline with increase condition

[grandizzy]

Description of change

High level

• see Chinmay - Mathematical Discrepancies in equations used for calculating Interest Rates sherlock-audit/2023-04-ajna-judging#104
• in PoolCommons._calculateInterestRate the condition formula for decreasing interest rate is implemented differently than condition for increasing rate. Condition for increasing rate was changed with 23e7485#diff-f180a783f7f398e0540bb5a9fee461cbec2f970bdd423c904b5e12c6ec69c325R286 to fix an overflow while condition for decreasing wasn't, hence same error can manifest
  • make decrease rate condition in PoolCommons._calculateInterestRate at par with increase rate condition, that is from

(4 * (tu - mau) > 1e18 - ((tu + mau - 1e18) ** 2) / 1e18)

change to

(4 * (tu - mau) > 1e18 - ((tu + mau - 1e18) / 1e9) ** 2)

Contract size

Pre Change

PoolCommons              -   9,082B  (36.95%)

Post Change

PoolCommons              -   9,086B  (36.97%)

Gas usage

Pre Change

| updateInterest                       | 57081           | 122919 | 112150 | 413480 | 284     |
| updateInterest                         | 138992          | 193642 | 147298 | 417497   | 14      |

Post Change

| updateInterest                       | 57081           | 122919 | 112150 | 413480 | 284     |
| updateInterest                         | 138992          | 193642 | 147298 | 417497   | 14      |

[ith-harvey]

I agree that the proposed change matches how we are performing an increase. Am curious to know the reason behind this change or the why.

Is it because this code was not inline with the spec?

[grandizzy]

I agree that the proposed change matches how we are performing an increase. Am curious to know the reason behind this change or the why.

Is it because this code was not inline with the spec?

this was changed in order to allow greater values / avoid overflow for the if clause with 23e7485#diff-f180a783f7f398e0540bb5a9fee461cbec2f970bdd423c904b5e12c6ec69c325R286 but not for the else clause

[ith-harvey]

LGTM

The links that summarize the issue may die... please copy and paste the issue into the description directly

[grandizzy]

I agree that the proposed change matches how we are performing an increase. Am curious to know the reason behind this change or the why.

Is it because this code was not inline with the spec?

updated

[EdNoepel]

Reviewed the original change to line 294, scaling before squaring, to prevent overflow. This appears consistent with that change, as stated.