Futures liquidation fee update

[artdgn]

Updates the futures liquidation related logic for the first two items of Synthetixio/issues#352:

1. Proportional liquidation fee - to incentivise liquidations of larger positions first. Tentatively set to 35 bps.
2. Proportional liquidation margin buffer - to prevent negative margin liquidation, and to allow proportional fee to be paid to the fee pool. Tentatively set at 25 bps. If this buffer will not be needed in practice for safety (or for fees), the value can be set to 0 in the future.

These two values now allow controlling the proportional liquidation margin, and the allocation of that margin upon liquidation to the liquidator and fee pool.

Other important changes in this PR:

• Adding the parameters to FuturesMarketSettings: liquidationFeeBPs and liquidationBufferBPs
• Adding new views to FuturesMarket: liquidaionFee and liquidationMargin
• Changing the logic of _liquidationPrice to calculate unrecorded funding using the current price.
• Changing the event and debt adjustment during _liquidatePosition to use the current price instead of the liquidationPrice (which may be higher or lower than actual price at that time, depending on the parameters, buffer, and oracle price).
• Sending any positive remainder of the margin to Fee Pool after deducting the liquidationFee (that goes to liquidator).

[codecov]

Codecov Report

Merging #1594 (a472c96) into futures-implementation (8defb47) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@                    Coverage Diff                     @@
##           futures-implementation    #1594      +/-   ##
==========================================================
+ Coverage                   93.87%   93.91%   +0.03%     
==========================================================
  Files                          85       85              
  Lines                        2058     2071      +13     
  Branches                      627      629       +2     
==========================================================
+ Hits                         1932     1945      +13     
  Misses                        126      126              

Impacted Files	Coverage Δ
contracts/FuturesMarketData.sol	100.00% <ø> (ø)
contracts/FuturesMarket.sol	98.42% <100.00%> (+0.09%)	⬆️
contracts/FuturesMarketSettings.sol	100.00% <100.00%> (ø)
contracts/MixinFuturesMarketSettings.sol	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8defb47...a472c96. Read the comment docs.

[liamzebedee]

This may be a dumb question but I definitely don't claim to be an expert in this area, so please, educate me. Why use the lastPrice here when we calculate the liqMargin above using the currentPrice?

[artdgn]

According to the derivation in the comments above, lastPrice is used in order to calculate the profit / loss, which is (futurePrice - lastPrice) * positionSize (which was the price the position was opened / updated at). This is because the formula is trying to solve for the futurePrice at which the position will be liquidatable. Using currentPrice would give incorrect P&L w.r.t. to position opening price (at which point the "the loan" from the debt pool was taken).

Liquidation margin is calculated using the current price because it's the best estimation we can do currently - because we can't really solve for the actual future liquidation margin because we don't know how funding in the future will affect it (because it depends on 1) time passage and future skew 2) at what times it's being recorded due to other transactions).

Previously liquidation margin was a constant parameter, but making it proportional to price & position introduces this imprecision in solving for liquidation price for the future.

The imprecision is not an issue for the actual liquidation transaction - because then futurePrice is currentPrice. It's also small when calculated close to the possible liquidation time.

The comments above the code are using trying to give more details about the derivation, but probably can be improved, any idea for what extra details needs to go there?

[artdgn]

TL;DR: I think both approaches make some sense, but both have drawbacks as well. I think the new one has better practical tradeoffs while not sacrificing any liquidation correctness.

Agree on the crux of the difference

I agree that the core difference is due to considering everything in terms of current price and current time, instead of "time traveling". And that everything should be consistent.

I've switched to using current instead of time travelling because:

• The scenario of "liquidatable in the past, but not right now" was not really addressed because the contract doesn't have that past information during the liquidation tx (I think the SIP mentioned liquidators passing in the funding index, but it's not in the contract - liquidation method only has account as input).
• Time traveling is more difficult to reason about and test exhaustively. Single time frame is easier to understand, discuss, and test. So from the egineering POV, simpler code, that still performs the important bit correctly (liquidation itself) may be preferrable to a more elegant code that is harder to maintain.
• Previous time traveling logic relied on some assumptions of 1) funding not being recorded until liquidation 2) no time passing until liquidation. Which works well in testing scenario (because we can setPrice() for next second, and have no other transactions happen), but would still have incorrect liquidation price for the realistic scenario (of price moving at real-world pace, and funding accruing and being recorded until actual liquidation).

Disagree that there are scenarios where liquidations are missed

I think there are some tradeoffs here, and I really liked the elegance of being able to derive the exact price for testing, but I think the simplicity and separation of concerns (e.g. not having to re-derive formulas in liquidation price due to liquidation fee logic changes) felt like a better tradeoff if liquidations can happen correctly in both ways.

But with this system currentPrice can still be substantially below the true first liquidation price, for example if the price is moving quickly or if liquidation is slow for some reason

In this scenario, both approaches would trigger liquidation in the same way it seems. Or do you see a scenario at which the new method doesn't liquidate a position that the previous method would? (Keeping in mind that liquidation keeper only passes the account as input, without funding index or oracle round ID).

Disagree about using a more precise calculation: it would not be more precise in practice, and it would be more complicated

That precise computation is actually still possible in the proportional buffer case, though because of the max it needs two liquidation price formulae and a case analysis. I'll send the derivation out of band.

Thanks for the calculation. There are two reason's I'd be reluctant to implement it:

1. The liquidation margin is slightly incorrect (it assumes the outer max() as proposed in another comment) and would require doing differently for the current liquidation margin (with inner max()).
2. It would make the code quite a bit more complex, and would make the manual calculations necessary for maintaining the test cases more complex as well.
3. It would strongly couple the liquidation price logic to the liquidation fee logic, which now can be encapsulated each in its own method.
4. It would make future changes to the code of either the price, the fee, or the margin very hard to reason and maintain (because of the combination of coupling and complexity of the calculation).

This might be worth it, in my opinion, if the liquidations would happen more correctly, but it seems to me that when using currentPrice the liquidation itself would happen correctly with both versions, and the only downside to the imprecision is the over-conservative estimation of future hypothetical liquidation price, which isn't too bad.

Why this "imprecision" for the hypothetical price is not harmful

This over-conservative estimation may actually have usability advantages, because e.g. the current liquidation price for a long is returned slightly higher than actual future liquidation price, which is better than the other way arround (so e.g. the UI will say liquidation price 200, where in fact you'll be liquidated at 195, which is better the the reverse).

[zyzek]

Since the _canLiquidate function does not actually use the _liquidationPrice logic, I agree that positions will always liquidate at the correct times. I also agree with all points about complexity of the code.

The only thing I really worry about is the debt pool impact. If a position is liquidated at the current price, and that price is substantially below the "true" liquidation point (in the long case), then less debt will be removed from the debt correction by the liquidation than ought to have been, since the position's notional value is evaluated to be lower at liquidation.
I imagine in most cases this will be negligible but could get larger the longer a position is unliquidated and the faster prices are moving, and I wonder about the cumulative impact of many liquidations over time. Since using the current price is basically doing the same thing as other margin exchanges with respect to liquidation point, then perhaps this is an argument for doing as they do with maintenance margin and starting with a nonzero value for the buffer parameter.

[artdgn]

Thanks, will have a more thorough think through these scenarios in terms of debt and post back my conclusions here.

[zyzek]

I'll add that other than this last point, I'm more or less satisfied that the PR looks pretty solid. As long as the debt considerations are being kept in mind, I'll leave it to full-time contributors for further review and approval (I haven't looked at test cases, config files and so on).

[artdgn]

Had a deeper look into the debt calculations:

1. It seems that for the subject of liquidations, the liquidation price actually makes no difference for the debt calculations. This is because the new position is of 0 size and 0 margin, so the price used doesn't matter. So the new position adds no debt and the old position's debt is subtracted at its last price.

// in _liquidatePosition
        _applyDebtCorrection(
            Position(0, 0, 0, price, fundingIndex),
            Position(0, position.margin, positionSize, position.lastPrice, position.fundingIndex)
        );

// _positionDebtCorrection

    function _positionDebtCorrection(Position memory position) internal view returns (int) {
        return
            int(position.margin).sub(
                position.size.multiplyDecimalRound(int(position.lastPrice).add(fundingSequence[position.fundingIndex]))
            );
    }

// _applyDebtCorrection

    function _applyDebtCorrection(Position memory newPosition, Position memory oldPosition) internal {
        int newCorrection = _positionDebtCorrection(newPosition);
        int oldCorrection = _positionDebtCorrection(oldPosition);
        _entryDebtCorrection = _entryDebtCorrection.add(newCorrection).sub(oldCorrection);
    }

2. Unrelated to the liquidation related debt calculation I don't really understand the role of the funding rate component of the debt correction calculation. It seems to me that the FR shouldn't be included in it. Because for a new position - there is no funding component in the debt added (the position is new), and for a modified position the funding is already counted in the updated margin along with the P&L (so it's double counted?). So I think it should probably be removed. I'll follow up on that out of band and if needed it will be a separate PR.

[jacko125]

Feels like the comment here at the end of line isn't needed

[artdgn]

the comment is there because it was suggested to use _max() in the reivew, and without this comment someone might have the same idea in future - and the comment will spare them the trouble :)

[jacko125]

Happy with the changes. What are the default liquidation fee ratio % and buffer % in mind that we will set for deployment ? And then these can be added to sip-80

[artdgn]

Happy with the changes. What are the default liquidation fee ratio % and buffer % in mind that we will set for deployment ? And then these can be added to sip-80

Yeah the sip will need to be updated, but perhaps worth to wait to make a single update with all the changes in this task.

The values I've set right now is:

• 35 bps for fee incentive (so $35 for each 10K in notional risk).
• 25 bps for buffer. The logic is that if actual liquidation price is on average 0.5% worse than the first liquidation price - the stakers will not be impacted.