From f768628bde051e2fac27d96e9b1590b6be769ce7 Mon Sep 17 00:00:00 2001 From: Elias Van Ootegem Date: Thu, 12 Oct 2023 10:52:10 +0100 Subject: [PATCH 1/3] fix: apply funding payment increment correctly Signed-off-by: Elias Van Ootegem --- CHANGELOG.md | 1 + core/risk/engine_test.go | 10 +++++----- core/risk/margins_calculation.go | 12 ++++-------- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a49da1ead..3a5afffb9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -151,6 +151,7 @@ - [402](https://github.com/vegaprotocol/OctoberACs/issues/402) - Add integration test for team rewards `0056-REWA-099` - [401](https://github.com/vegaprotocol/OctoberACs/issues/401) - Add integration test for team rewards `0056-REWA-098` - [400](https://github.com/vegaprotocol/OctoberACs/issues/400) - Add integration test for team rewards `0056-REWA-097` +- [2971](https://github.com/vegaprotocol/system-tests/issues/2971) - Fix margin calculation for `PERPS`. ### 🐛 Fixes diff --git a/core/risk/engine_test.go b/core/risk/engine_test.go index 9bdf8709eb..c9b83543bd 100644 --- a/core/risk/engine_test.go +++ b/core/risk/engine_test.go @@ -184,7 +184,7 @@ func TestNegativeMargin(t *testing.T) { riskEvts := eng.UpdateMarginsOnSettlement(ctx, []events.Margin{evt}, mtmPrice, inc) require.NotEmpty(t, riskEvts) initial := riskEvts[0].Transfer().Amount.Amount - require.Equal(t, "2", initial.String()) + require.Equal(t, "5", initial.String()) } func testMarginTopup(t *testing.T) { @@ -251,7 +251,7 @@ func TestMarginTopupPerpetual(t *testing.T) { assert.Equal(t, 1, len(resp)) mm := resp[0].MarginLevels().MaintenanceMargin - assert.Equal(t, "25", mm.String()) + assert.Equal(t, "30", mm.String()) // now do it again with the funding payment negated, the margin should be as if we were not a perp // and 5 less @@ -259,7 +259,7 @@ func TestMarginTopupPerpetual(t *testing.T) { assert.Equal(t, 1, len(resp)) mm = resp[0].MarginLevels().MaintenanceMargin - assert.Equal(t, "30", mm.String()) + assert.Equal(t, "25", mm.String()) } func testMarginNotReleasedInAuction(t *testing.T) { @@ -477,7 +477,7 @@ func TestMarginWithNoOrdersOnBook(t *testing.T) { auction: true, }, { - expectedMargin: "245", + expectedMargin: "335", positionSize: 9, buyOrders: []*risk.OrderInfo{ { @@ -510,7 +510,7 @@ func TestMarginWithNoOrdersOnBook(t *testing.T) { auction: false, }, { - expectedMargin: "238", + expectedMargin: "328", positionSize: 9, buyOrders: []*risk.OrderInfo{ { diff --git a/core/risk/margins_calculation.go b/core/risk/margins_calculation.go index 26bb8b2392..df94930a93 100644 --- a/core/risk/margins_calculation.go +++ b/core/risk/margins_calculation.go @@ -82,11 +82,8 @@ func (e *Engine) calculateMargins(m events.Margin, markPrice *num.Uint, rf types CollateralReleaseLevel: num.UintZero(), } } - // long funding payment -> requires extra margin, if false, same holds for short - longFP := inc.IsNegative() - if longFP { - inc = inc.Abs() // this can be positive or negative, we use it to pad margin levels to ensure funding can be paid either way, always add - } + // negative increment == short positions require extra margin, otherwise long requires extra margin + longFP := inc.IsPositive() mPriceDec := markPrice.ToDecimal() // calculate margin maintenance long only if riskiest is > 0 @@ -242,9 +239,8 @@ func (e *Engine) calculateMargins(m events.Margin, markPrice *num.Uint, rf types } if !inc.IsZero() && !openVolume.IsZero() { - // calculate margin increase based on position - // incD = max(0, inc * open volume) - incD := num.MaxD(num.DecimalZero(), inc.Mul(openVolume)) + // openVolume and inc are signed, but this is fine, we only apply the positive values + incD := inc.Mul(openVolume) if longFP { marginMaintenanceLng = marginMaintenanceLng.Add(incD) } else { From 488ad29c395533d0c7ee51b45ed45389ab719a6e Mon Sep 17 00:00:00 2001 From: Elias Van Ootegem Date: Thu, 12 Oct 2023 11:33:42 +0100 Subject: [PATCH 2/3] fix: code review + add integration test Signed-off-by: Elias Van Ootegem --- .../mark_to_market_perpetual.feature | 94 +++++++++++++++++++ core/risk/margins_calculation.go | 2 +- 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/core/integration/features/settlement/mark_to_market_perpetual.feature b/core/integration/features/settlement/mark_to_market_perpetual.feature index e91a3e03a8..8ad2fd8836 100644 --- a/core/integration/features/settlement/mark_to_market_perpetual.feature +++ b/core/integration/features/settlement/mark_to_market_perpetual.feature @@ -452,3 +452,97 @@ Feature: Test mark to market settlement with periodicity, takes the first scenar | party2 | ETH | ETH/DEC19 | 5270532 | 7729468 | | party3 | ETH | ETH/DEC19 | 0 | 1000000 | | party4 | ETH | ETH/DEC19 | 5270532 | 6727467 | + + @Perpetual @PerpMargin @PerpMarginBug + Scenario: Verify margins are adjusted for funding payments as expected + Given the following network parameters are set: + | name | value | + | network.markPriceUpdateMaximumFrequency | 0s | + And the parties deposit on asset's general account the following amount: + | party | asset | amount | + | party1 | ETH | 10000000 | + | party2 | ETH | 10000000 | + | party3 | ETH | 1000000 | + | party4 | ETH | 10000000 | + | aux | ETH | 100000000 | + | aux2 | ETH | 100000000 | + | lpprov | ETH | 1000000000 | + + When the parties submit the following liquidity provision: + | id | party | market id | commitment amount | fee | lp type | + | lp1 | lpprov | ETH/DEC19 | 100000000 | 0.001 | submission | + | lp1 | lpprov | ETH/DEC19 | 100000000 | 0.001 | submission | + And the parties place the following pegged iceberg orders: + | party | market id | peak size | minimum visible size | side | pegged reference | volume | offset | + | lpprov | ETH/DEC19 | 2 | 1 | buy | BID | 50 | 1 | + | lpprov | ETH/DEC19 | 2 | 1 | sell | ASK | 50 | 1 | + + # place auxiliary orders so we always have best bid and best offer as to not trigger the liquidity auction + When the parties place the following orders: + | party | market id | side | volume | price | resulting trades | type | tif | + | aux | ETH/DEC19 | buy | 1 | 49 | 0 | TYPE_LIMIT | TIF_GTC | + | aux | ETH/DEC19 | sell | 1 | 5001 | 0 | TYPE_LIMIT | TIF_GTC | + | aux2 | ETH/DEC19 | buy | 1 | 1000 | 0 | TYPE_LIMIT | TIF_GTC | + | aux | ETH/DEC19 | sell | 1 | 1000 | 0 | TYPE_LIMIT | TIF_GTC | + + And the market data for the market "ETH/DEC19" should be: + | target stake | supplied stake | + | 1100000 | 100000000 | + Then the opening auction period ends for market "ETH/DEC19" + And the trading mode should be "TRADING_MODE_CONTINUOUS" for the market "ETH/DEC19" + And the settlement account should have a balance of "0" for the market "ETH/DEC19" + + # back sure we end the block so we're in a new one after opening auction + When the network moves ahead "1" blocks + # set pegs 1001 and 999 to keep margins consistent + And the parties place the following orders: + | party | market id | side | volume | price | resulting trades | type | tif | + | party3 | ETH/DEC19 | sell | 1 | 1001 | 0 | TYPE_LIMIT | TIF_GTC | + | party4 | ETH/DEC19 | buy | 1 | 999 | 0 | TYPE_LIMIT | TIF_GTC | + | party1 | ETH/DEC19 | sell | 1 | 1000 | 0 | TYPE_LIMIT | TIF_GTC | + | party2 | ETH/DEC19 | buy | 1 | 1000 | 1 | TYPE_LIMIT | TIF_GTC | + Then the parties should have the following account balances: + | party | asset | market id | margin | general | + | party1 | ETH | ETH/DEC19 | 120000 | 9880000 | + | party2 | ETH | ETH/DEC19 | 132000 | 9867000 | + And the parties should have the following margin levels: + | party | market id | maintenance | search | initial | release | + | party1 | ETH/DEC19 | 100000 | 110000 | 120000 | 140000 | + | party2 | ETH/DEC19 | 110000 | 121000 | 132000 | 154000 | + | party3 | ETH/DEC19 | 100000 | 110000 | 120000 | 140000 | + And the settlement account should have a balance of "0" for the market "ETH/DEC19" + + # funding payment -> external TWAP is based on 1100, short parties are losing, their margin increases more + When the network moves ahead "2" blocks + Then the oracles broadcast data with block time signed with "0xCAFECAFE1": + | name | value | time offset | + | perp.ETH.value | 1100000000000000000000 | -1s | + | perp.funding.cue | 1511924180 | -1s | + Then the parties should have the following margin levels: + | party | market id | maintenance | search | initial | release | + | party1 | ETH/DEC19 | 101000 | 111100 | 121200 | 141400 | + | party2 | ETH/DEC19 | 111000 | 122100 | 133200 | 155400 | + | party3 | ETH/DEC19 | 100000 | 110000 | 120000 | 140000 | + + # Repeat the same operations, create new internal TWAP data point at the same price levels + When the network moves ahead "1" blocks + And the parties place the following orders: + | party | market id | side | volume | price | resulting trades | type | tif | + | party1 | ETH/DEC19 | sell | 1 | 1000 | 0 | TYPE_LIMIT | TIF_GTC | + | party2 | ETH/DEC19 | buy | 1 | 1000 | 1 | TYPE_LIMIT | TIF_GTC | + # Margins obviously increase as the position increased + Then the parties should have the following margin levels: + | party | market id | maintenance | search | initial | release | + | party1 | ETH/DEC19 | 201000 | 221100 | 241200 | 281400 | + | party2 | ETH/DEC19 | 221000 | 243100 | 265200 | 309400 | + And the settlement account should have a balance of "0" for the market "ETH/DEC19" + # funding payment -> external TWAP is based on 900, long parties are losing, their margin increases more + When the network moves ahead "1" blocks + Then the oracles broadcast data with block time signed with "0xCAFECAFE1": + | name | value | time offset | + | perp.ETH.value | 900000000000000000000 | -1s | + | perp.funding.cue | 1511924181 | -1s | + Then the parties should have the following margin levels: + | party | market id | maintenance | search | initial | release | + | party2 | ETH/DEC19 | 224000 | 246400 | 268800 | 313600 | + | party1 | ETH/DEC19 | 202000 | 222200 | 242400 | 282800 | diff --git a/core/risk/margins_calculation.go b/core/risk/margins_calculation.go index df94930a93..cc0e7695ab 100644 --- a/core/risk/margins_calculation.go +++ b/core/risk/margins_calculation.go @@ -240,7 +240,7 @@ func (e *Engine) calculateMargins(m events.Margin, markPrice *num.Uint, rf types if !inc.IsZero() && !openVolume.IsZero() { // openVolume and inc are signed, but this is fine, we only apply the positive values - incD := inc.Mul(openVolume) + incD := num.MaxD(num.DecimalZero(), inc.Mul(openVolume)) if longFP { marginMaintenanceLng = marginMaintenanceLng.Add(incD) } else { From 97d53077d4e97f621060f855a8f1d681a60ba2ab Mon Sep 17 00:00:00 2001 From: Elias Van Ootegem Date: Thu, 12 Oct 2023 11:42:20 +0100 Subject: [PATCH 3/3] chore: update integration test to control margins more Signed-off-by: Elias Van Ootegem --- .../settlement/mark_to_market_perpetual.feature | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/core/integration/features/settlement/mark_to_market_perpetual.feature b/core/integration/features/settlement/mark_to_market_perpetual.feature index 8ad2fd8836..47b901a289 100644 --- a/core/integration/features/settlement/mark_to_market_perpetual.feature +++ b/core/integration/features/settlement/mark_to_market_perpetual.feature @@ -510,6 +510,7 @@ Feature: Test mark to market settlement with periodicity, takes the first scenar | party1 | ETH/DEC19 | 100000 | 110000 | 120000 | 140000 | | party2 | ETH/DEC19 | 110000 | 121000 | 132000 | 154000 | | party3 | ETH/DEC19 | 100000 | 110000 | 120000 | 140000 | + | party4 | ETH/DEC19 | 110000 | 121000 | 132000 | 154000 | And the settlement account should have a balance of "0" for the market "ETH/DEC19" # funding payment -> external TWAP is based on 1100, short parties are losing, their margin increases more @@ -523,11 +524,14 @@ Feature: Test mark to market settlement with periodicity, takes the first scenar | party1 | ETH/DEC19 | 101000 | 111100 | 121200 | 141400 | | party2 | ETH/DEC19 | 111000 | 122100 | 133200 | 155400 | | party3 | ETH/DEC19 | 100000 | 110000 | 120000 | 140000 | + | party4 | ETH/DEC19 | 110000 | 121000 | 132000 | 154000 | # Repeat the same operations, create new internal TWAP data point at the same price levels When the network moves ahead "1" blocks And the parties place the following orders: | party | market id | side | volume | price | resulting trades | type | tif | + | party3 | ETH/DEC19 | sell | 1 | 1001 | 0 | TYPE_LIMIT | TIF_GTC | + | party4 | ETH/DEC19 | buy | 1 | 999 | 0 | TYPE_LIMIT | TIF_GTC | | party1 | ETH/DEC19 | sell | 1 | 1000 | 0 | TYPE_LIMIT | TIF_GTC | | party2 | ETH/DEC19 | buy | 1 | 1000 | 1 | TYPE_LIMIT | TIF_GTC | # Margins obviously increase as the position increased @@ -535,6 +539,8 @@ Feature: Test mark to market settlement with periodicity, takes the first scenar | party | market id | maintenance | search | initial | release | | party1 | ETH/DEC19 | 201000 | 221100 | 241200 | 281400 | | party2 | ETH/DEC19 | 221000 | 243100 | 265200 | 309400 | + | party3 | ETH/DEC19 | 200000 | 220000 | 240000 | 280000 | + | party4 | ETH/DEC19 | 220000 | 242000 | 264000 | 308000 | And the settlement account should have a balance of "0" for the market "ETH/DEC19" # funding payment -> external TWAP is based on 900, long parties are losing, their margin increases more When the network moves ahead "1" blocks @@ -544,5 +550,7 @@ Feature: Test mark to market settlement with periodicity, takes the first scenar | perp.funding.cue | 1511924181 | -1s | Then the parties should have the following margin levels: | party | market id | maintenance | search | initial | release | - | party2 | ETH/DEC19 | 224000 | 246400 | 268800 | 313600 | + | party2 | ETH/DEC19 | 222000 | 244200 | 266400 | 310800 | | party1 | ETH/DEC19 | 202000 | 222200 | 242400 | 282800 | + | party3 | ETH/DEC19 | 200000 | 220000 | 240000 | 280000 | + | party4 | ETH/DEC19 | 220000 | 242000 | 264000 | 308000 |