dynamicFeeBps is assymetric

[hats-bug-reporter[bot]]

Github username: @0xLogos Twitter username: -- Submission hash (on-chain): 0x6bea734e2375c9a608bf88f289905faf591b9c20978f96e7d83eb5a572c4b92a Severity: high

Description: In DynamicFees library price relative diff calculated as delta / _histPrice and multipled by leverage factor to obtain final fee.

Here is how delta calculated:

// _inQuotedOrder = true for base/quote oracle and false for quote/base oracle
uint256 _delta;
if (feeType == FeeType.DEPOSIT_FEE) {
    unchecked {
        if (_inQuotedOrder && _spotPrice < _histPrice) {
            _delta = _histPrice - _spotPrice; // case 1
        } else if (!_inQuotedOrder && _spotPrice > _histPrice) {
            _delta = _spotPrice - _histPrice; // case 2
        }
    }
} else {
    unchecked {
        if (_inQuotedOrder && _spotPrice > _histPrice) {
            _delta = _spotPrice - _histPrice; // case 2
        } else if (!_inQuotedOrder && _spotPrice < _histPrice) {
            _delta = _histPrice - _spotPrice; // case 1
        }
    }
}

There is 2 cases

1. spot > hist, delta = spot - hist, diff = (spot - hist) / hist = spot/hist - 1
2. spot < hist, delta = hist - spot, diff = (hist - spot) / hist = 1 - spot/hist

In fist case when spot price increases => spot/hist ration increases to infinity => multiplier increases to infinity. But in 2 case spot when price decreases => spot/hist ratio decreases => multiplier increases to 1 at max

So calculation is inconsistent.

This is because to calculate relative diff you can use either [(max - min) / min] or [(max - min) / max]. In first case limit = infinity, in second = 1. You can choose either one, but approach must be consistent.

But here it different

• 1 case: (max - min) / min
• 2 case: (max - min) / max

Recommendation

I would suggest to use 1st formula: divide delta by max value

uint256 _fee = _delta.mulDiv(
    feeLeverageFactor * OrigamiMath.BASIS_POINTS_DIVISOR,
    _histPrice > _spotPrice ? _histPrice : _spotPrice,
    OrigamiMath.Rounding.ROUND_UP
);

[0xLogos]

Sorry, there is an error in recommendation.

It should look like this:

---

I would suggest to use 1st formula: divide delta by min value

uint256 _fee = _delta.mulDiv(
    feeLeverageFactor * OrigamiMath.BASIS_POINTS_DIVISOR,
    _histPrice < _spotPrice ? _histPrice : _spotPrice,
    OrigamiMath.Rounding.ROUND_UP
);

---

I hope you get the point, it should be consistent.

[frontier159]

@0xLogos thank you very much for the finding. We acknowledge that there's an asymmetry here, depending on the quoted order of the oracle.

However there are some practical constraints which limit the scope/impact of this significantly:

1. In your case 1, the amount which is taken as fees is restricted to only take 100% as fees as part of OrigamiMath::subtractBps(), so it cannot realistically go to infinity.
   1. investWithToken()
   2. exitToToken()
2. Further, the spot price oracle check has a valid price range restriction
3. The difference in the resulting fee because of the asymmetry is very minor. The impact from the asymmetry is that the user will be charged ever so slightly less fees.

I've put together a spreadsheet showing the difference which I think you're reporting: https://docs.google.com/spreadsheets/d/1asbO6nnHYJ6o-AinVDi5lEWzs1tWmm_IobA0kGEC1zY/edit#gid=0

Can you please confirm this adequately captures it?

• We currently use the hist price only as the denominator: row 9 (in quoted order) & 21 (not in quoted order)
• Using the min(hist, spot)as the denominator still leads to a mis-match in my opinion.
• The more accurate solution imo is to use the historic price when in quoted order, and the spot price when it's not in quoted order

[0xLogos]

It can be proved that this always holds

abs(a - b) / min(a, b) == abs(1/a - 1/b) / min(1/b, 1/b)

So quote order doesn't matter.

In the provided example my formula works as expected because in second column relative price difference is slightly lesser despite abs is the same because values themlself slightly higher. For example let f be case 1 formula: f(10, 11) = 0.1, f(1000, 1001) = 0.001

[frontier159]

In the provided example my formula works as expected because in second column relative price difference is slightly lesser despite abs is the same because values themlself slightly higher.

Taking a step back, the point is that the fees should be symmetric. What do we mean by this?

• A 0.004 cent increase in price (away from expected historic) should result a deposit fee of X
• A 0.004 cent decrease in price (away from expected historic) should result an exit fee, also of X ie X is the same in both cases.

In which case row 9 is what we're looking for (when in quoted order), row 23 (when not in quoted order)

In terms of severity and impact, it's low. This is a very small minor difference compared to the design, and there are existing guardrails which mean the a and b remain small

• For lovDSR, the expected historic price is 1 for DAI/USD, and there are min/max ranges on the price of 0.99 <= X <= 1.01. The impact to the fee is very small
• For lovStEth, it's similar with min/max ranges on the oracle and a hist price of 1 stEth/Eth