Skip to content

Commit 9f9059c

Browse files
authored
Merge pull request #11295 from vegaprotocol/amm-sign-error
fix: volume change for price calcs is signed based on buy or sell
2 parents ff0404a + 57c59ec commit 9f9059c

File tree

8 files changed

+192
-173
lines changed

8 files changed

+192
-173
lines changed

core/execution/amm/engine_test.go

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -160,9 +160,14 @@ func testBasicSubmitOrder(t *testing.T) {
160160
Price: num.NewUint(1900),
161161
}
162162

163+
// fair-price is now 2020
164+
bb, _, ba, _ := tst.engine.BestPricesAndVolumes()
165+
assert.Equal(t, "2019", bb.String())
166+
assert.Equal(t, "2021", ba.String())
167+
163168
orders = tst.engine.SubmitOrder(agg, num.NewUint(2020), num.NewUint(1990))
164169
require.Len(t, orders, 1)
165-
assert.Equal(t, "2035", orders[0].Price.String())
170+
assert.Equal(t, "2004", orders[0].Price.String())
166171
// note that this volume being bigger than 242367 above means we've moved back to position, then flipped
167172
// sign, and took volume from the other curve.
168173
assert.Equal(t, 362325, int(orders[0].Size))
@@ -189,7 +194,7 @@ func testSubmitMarketOrder(t *testing.T) {
189194
ensurePosition(t, tst.pos, 0, num.NewUint(0))
190195
orders := tst.engine.SubmitOrder(agg, num.NewUint(1980), num.NewUint(1990))
191196
require.Len(t, orders, 1)
192-
assert.Equal(t, "2005", orders[0].Price.String())
197+
assert.Equal(t, "1994", orders[0].Price.String())
193198
assert.Equal(t, 126420, int(orders[0].Size))
194199
}
195200

@@ -309,16 +314,16 @@ func testSubmitOrderAcrossAMMBoundarySell(t *testing.T) {
309314
require.Len(t, orders, 6)
310315

311316
// first round, three orders moving all pool's to the upper boundary of the shortest
312-
assert.Equal(t, "2053", orders[0].Price.String())
313-
assert.Equal(t, "2053", orders[1].Price.String())
314-
assert.Equal(t, "2053", orders[2].Price.String())
317+
assert.Equal(t, "1949", orders[0].Price.String())
318+
assert.Equal(t, "1949", orders[1].Price.String())
319+
assert.Equal(t, "1949", orders[2].Price.String())
315320

316321
// second round, 2 orders moving all pool's to the upper boundary of the second shortest
317-
assert.Equal(t, "1925", orders[3].Price.String())
318-
assert.Equal(t, "1925", orders[4].Price.String())
322+
assert.Equal(t, "1874", orders[3].Price.String())
323+
assert.Equal(t, "1874", orders[4].Price.String())
319324

320325
// third round, 1 orders moving the last pool to its boundary
321-
assert.Equal(t, "1875", orders[5].Price.String())
326+
assert.Equal(t, "1824", orders[5].Price.String())
322327
}
323328

324329
func testBestPricesAndVolume(t *testing.T) {

core/execution/amm/pool.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -529,18 +529,23 @@ func (p *Pool) OrderbookShape(from, to *num.Uint) ([]*types.Order, []*types.Orde
529529
return buys, sells
530530
}
531531

532-
// PriceForVolume returns the price the AMM is willing to trade at to match with the given volume.
532+
// PriceForVolume returns the price the AMM is willing to trade at to match with the given volume of an incoming order.
533533
func (p *Pool) PriceForVolume(volume uint64, side types.Side) *num.Uint {
534534
x, y := p.virtualBalances(p.getPosition(), p.fairPrice(), side)
535535

536536
// dy = x*y / (x - dx) - y
537537
// where y and x are the balances on either side of the pool, and dx is the change in volume
538538
// then the trade price is dy/dx
539539
dx := num.DecimalFromInt64(int64(volume))
540+
if side == types.SideSell {
541+
// if incoming order is a sell, the AMM is buying so reducing cash balance so dx is negative
542+
dx = dx.Neg()
543+
}
544+
540545
dy := x.Mul(y).Div(x.Sub(dx)).Sub(y)
541546

542547
// dy / dx
543-
price, overflow := num.UintFromDecimal(dy.Div(dx))
548+
price, overflow := num.UintFromDecimal(dy.Div(dx).Abs())
544549
if overflow {
545550
panic("calculated negative price")
546551
}

core/execution/amm/pool_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,16 @@ func TestNotebook(t *testing.T) {
404404
ensurePosition(t, p.pos, 500, lowmid.Clone())
405405
fairPrice = p.pool.BestPrice(nil)
406406
assert.Equal(t, "1854", fairPrice.String())
407+
408+
// fair price is 2000 and the AMM quotes a best-buy at 1999 so incoming SELL should have a price <= 1999
409+
ensurePositionN(t, p.pos, 0, lowmid.Clone(), 2)
410+
price := p.pool.PriceForVolume(100, types.SideSell)
411+
assert.Equal(t, "1984", price.String())
412+
413+
// fair price is 2000 and the AMM quotes a best-buy at 2001 so incoming BUY should have a price >= 2001
414+
ensurePositionN(t, p.pos, 0, lowmid.Clone(), 2)
415+
price = p.pool.PriceForVolume(100, types.SideBuy)
416+
assert.Equal(t, "2014", price.String())
407417
}
408418

409419
type tstPool struct {

core/integration/features/amm/0090-VAMM-006-014.feature

Lines changed: 48 additions & 48 deletions
Large diffs are not rendered by default.

core/integration/features/amm/0090-VAMM-020.feature

Lines changed: 34 additions & 34 deletions
Large diffs are not rendered by default.

core/integration/features/amm/0090-VAMM-021.feature

Lines changed: 57 additions & 59 deletions
Large diffs are not rendered by default.

core/integration/features/amm/0090-VAMM-028.feature

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,7 @@ Feature: Ensure the vAMM positions follow the market correctly
257257
| vamm2-id | -74 | -304 | 0 | true |
258258
| vamm1-id | -74 | 0 | 0 | true |
259259

260+
@VAMM
260261
Scenario: 0090-VAMM-029: The volume quoted to move from price 100 to price 90 in one step is the same as the sum of the volumes to move in 10 steps of 1.
261262
# Move mid price to 90 in one go. A volume of 347 is the minimum required, 346 only gets us to 91
262263
When the parties place the following orders:
@@ -267,13 +268,13 @@ Feature: Ensure the vAMM positions follow the market correctly
267268
| 100 | TRADING_MODE_CONTINUOUS | 13915 | 1000 | 348 | 100 | 90 | 90 | 91 | 89 |
268269
And the following trades should be executed:
269270
| buyer | price | size | seller | is amm |
270-
| vamm1-id | 105 | 347 | party5 | true |
271+
| vamm1-id | 95 | 347 | party5 | true |
271272
# Check vAMM position
272273
When the network moves ahead "1" blocks
273274
Then the parties should have the following profit and loss:
274275
| party | volume | unrealised pnl | realised pnl | is amm |
275-
| party1 | 1 | 5 | 0 | |
276-
| party2 | -1 | -5 | 0 | |
276+
| party1 | 1 | -5 | 0 | |
277+
| party2 | -1 | 5 | 0 | |
277278
| party5 | -347 | 0 | 0 | |
278279
| vamm1-id | 347 | 0 | 0 | true |
279280

@@ -287,7 +288,7 @@ Feature: Ensure the vAMM positions follow the market correctly
287288
| 100 | TRADING_MODE_CONTINUOUS | 79 | 1000 | 2 | 100 | 99 | 99 | 100 | 98 |
288289
And the following trades should be executed:
289290
| buyer | price | size | seller | is amm |
290-
| vamm2-id | 100 | 1 | party6 | true |
291+
| vamm2-id | 99 | 1 | party6 | true |
291292

292293
# Move mid price to 98
293294
When the parties place the following orders:
@@ -298,7 +299,7 @@ Feature: Ensure the vAMM positions follow the market correctly
298299
| 100 | TRADING_MODE_CONTINUOUS | 1519 | 1000 | 38 | 100 | 98 | 98 | 99 | 97 |
299300
And the following trades should be executed:
300301
| buyer | price | size | seller | is amm |
301-
| vamm2-id | 100 | 36 | party6 | true |
302+
| vamm2-id | 98 | 36 | party6 | true |
302303

303304
# Move mid price to 97
304305
When the parties place the following orders:
@@ -309,7 +310,7 @@ Feature: Ensure the vAMM positions follow the market correctly
309310
| 100 | TRADING_MODE_CONTINUOUS | 2959 | 1000 | 74 | 100 | 97 | 97 | 98 | 96 |
310311
And the following trades should be executed:
311312
| buyer | price | size | seller | is amm |
312-
| vamm2-id | 98 | 36 | party6 | true |
313+
| vamm2-id | 97 | 36 | party6 | true |
313314

314315
# Move mid price to 96
315316
When the parties place the following orders:
@@ -320,7 +321,7 @@ Feature: Ensure the vAMM positions follow the market correctly
320321
| 100 | TRADING_MODE_CONTINUOUS | 4478 | 1000 | 112 | 100 | 96 | 96 | 97 | 95 |
321322
And the following trades should be executed:
322323
| buyer | price | size | seller | is amm |
323-
| vamm2-id | 97 | 38 | party6 | true |
324+
| vamm2-id | 96 | 38 | party6 | true |
324325

325326
# Move mid price to 95
326327
When the parties place the following orders:
@@ -332,7 +333,7 @@ Feature: Ensure the vAMM positions follow the market correctly
332333
And debug trades
333334
And the following trades should be executed:
334335
| buyer | price | size | seller | is amm |
335-
| vamm2-id | 96 | 37 | party6 | true |
336+
| vamm2-id | 95 | 37 | party6 | true |
336337

337338
# Move mid price to 94
338339
When the parties place the following orders:
@@ -343,7 +344,7 @@ Feature: Ensure the vAMM positions follow the market correctly
343344
| 100 | TRADING_MODE_CONTINUOUS | 7517 | 1000 | 188 | 100 | 94 | 94 | 95 | 93 |
344345
And the following trades should be executed:
345346
| buyer | price | size | seller | is amm |
346-
| vamm2-id | 95 | 39 | party6 | true |
347+
| vamm2-id | 94 | 39 | party6 | true |
347348

348349
# Move mid price to 93
349350
When the parties place the following orders:
@@ -354,7 +355,7 @@ Feature: Ensure the vAMM positions follow the market correctly
354355
| 100 | TRADING_MODE_CONTINUOUS | 9077 | 1000 | 227 | 100 | 93 | 93 | 94 | 92 |
355356
And the following trades should be executed:
356357
| buyer | price | size | seller | is amm |
357-
| vamm2-id | 94 | 39 | party6 | true |
358+
| vamm2-id | 93 | 39 | party6 | true |
358359

359360
# Move mid price to 92
360361
When the parties place the following orders:
@@ -365,7 +366,7 @@ Feature: Ensure the vAMM positions follow the market correctly
365366
| 100 | TRADING_MODE_CONTINUOUS | 10636 | 1000 | 266 | 100 | 92 | 92 | 93 | 91 |
366367
And the following trades should be executed:
367368
| buyer | price | size | seller | is amm |
368-
| vamm2-id | 93 | 39 | party6 | true |
369+
| vamm2-id | 92 | 39 | party6 | true |
369370

370371
# Move mid price to 91
371372
When the parties place the following orders:
@@ -376,7 +377,7 @@ Feature: Ensure the vAMM positions follow the market correctly
376377
| 100 | TRADING_MODE_CONTINUOUS | 12276 | 1000 | 307 | 100 | 91 | 91 | 92 | 90 |
377378
And the following trades should be executed:
378379
| buyer | price | size | seller | is amm |
379-
| vamm2-id | 92 | 41 | party6 | true |
380+
| vamm2-id | 91 | 41 | party6 | true |
380381

381382
# Move mid price to 90
382383
When the parties place the following orders:
@@ -387,16 +388,16 @@ Feature: Ensure the vAMM positions follow the market correctly
387388
| 100 | TRADING_MODE_CONTINUOUS | 13915 | 1000 | 348 | 100 | 90 | 90 | 91 | 89 |
388389
And the following trades should be executed:
389390
| buyer | price | size | seller | is amm |
390-
| vamm2-id | 91 | 41 | party6 | true |
391+
| vamm2-id | 90 | 41 | party6 | true |
391392

392393
# Make sure the volumes match, PnL is expected to be different
393394
When the network moves ahead "1" blocks
394395
Then the parties should have the following profit and loss:
395396
| party | volume | unrealised pnl | realised pnl | is amm |
396-
| party3 | 1 | -9 | 0 | |
397-
| party4 | -1 | 9 | 0 | |
398-
| party6 | -347 | 1390 | 0 | |
399-
| vamm2-id | 347 | -1390 | 0 | true |
397+
| party3 | 1 | -10 | 0 | |
398+
| party4 | -1 | 10 | 0 | |
399+
| party6 | -347 | 1354 | 0 | |
400+
| vamm2-id | 347 | -1354 | 0 | true |
400401
| vamm1-id | 347 | 0 | 0 | true |
401402

402403
@VAMM

core/integration/features/amm/0090-VAMM-rebase.feature

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -106,13 +106,13 @@ Feature: vAMM rebasing when created or amended
106106
# second AMM has its base 5 away from the first AMM so it must submit a rebasing-order
107107
And the following trades should be executed:
108108
| buyer | price | size | seller | is amm |
109-
| vamm1-id | 101 | 140 | vamm2-id | true |
109+
| vamm1-id | 98 | 140 | vamm2-id | true |
110110
Then the network moves ahead "1" blocks
111111

112112
# and now the mid-price has shifted lower to a value between the two AMM's bases 95 < 97 < 100
113113
And the market data for the market "ETH/MAR22" should be:
114114
| mark price | trading mode | mid price |
115-
| 101 | TRADING_MODE_CONTINUOUS | 97 |
115+
| 98 | TRADING_MODE_CONTINUOUS | 97 |
116116

117117

118118
@VAMM
@@ -167,7 +167,7 @@ Feature: vAMM rebasing when created or amended
167167

168168
When the parties submit the following AMM:
169169
| party | market id | amount | slippage | base | lower bound | upper bound | proposed fee |
170-
| vamm2 | ETH/MAR22 | 100000 | 0.05 | 100 | 95 | 105 | 0.03 |
170+
| vamm2 | ETH/MAR22 | 100000 | 0.05 | 100 | 95 | 105 | 0.03 |
171171
Then the AMM pool status should be:
172172
| party | market id | amount | status | base | lower bound | upper bound |
173173
| vamm2 | ETH/MAR22 | 100000 | STATUS_ACTIVE | 100 | 95 | 105 |
@@ -193,13 +193,13 @@ Feature: vAMM rebasing when created or amended
193193
# second AMM has its base 5 away from the first AMM so it must submit a rebasing-order
194194
And the following trades should be executed:
195195
| buyer | price | size | seller | is amm |
196-
| vamm1-id | 101 | 140 | vamm2-id | true |
196+
| vamm1-id | 98 | 140 | vamm2-id | true |
197197
Then the network moves ahead "1" blocks
198198

199199
# and now the mid-price has shifted lower to a value between the two AMM's bases 95 < 98 < 100
200200
And the market data for the market "ETH/MAR22" should be:
201201
| mark price | trading mode | mid price |
202-
| 101 | TRADING_MODE_CONTINUOUS | 97 |
202+
| 98 | TRADING_MODE_CONTINUOUS | 97 |
203203

204204

205205
@VAMM

0 commit comments

Comments
 (0)