Skip to content

Commit

Permalink
Merge pull request #11295 from vegaprotocol/amm-sign-error
Browse files Browse the repository at this point in the history
fix: volume change for price calcs is signed based on buy or sell
  • Loading branch information
peterbarrow authored May 20, 2024
2 parents ff0404a + 57c59ec commit 9f9059c
Show file tree
Hide file tree
Showing 8 changed files with 192 additions and 173 deletions.
21 changes: 13 additions & 8 deletions core/execution/amm/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,14 @@ func testBasicSubmitOrder(t *testing.T) {
Price: num.NewUint(1900),
}

// fair-price is now 2020
bb, _, ba, _ := tst.engine.BestPricesAndVolumes()
assert.Equal(t, "2019", bb.String())
assert.Equal(t, "2021", ba.String())

orders = tst.engine.SubmitOrder(agg, num.NewUint(2020), num.NewUint(1990))
require.Len(t, orders, 1)
assert.Equal(t, "2035", orders[0].Price.String())
assert.Equal(t, "2004", orders[0].Price.String())
// note that this volume being bigger than 242367 above means we've moved back to position, then flipped
// sign, and took volume from the other curve.
assert.Equal(t, 362325, int(orders[0].Size))
Expand All @@ -189,7 +194,7 @@ func testSubmitMarketOrder(t *testing.T) {
ensurePosition(t, tst.pos, 0, num.NewUint(0))
orders := tst.engine.SubmitOrder(agg, num.NewUint(1980), num.NewUint(1990))
require.Len(t, orders, 1)
assert.Equal(t, "2005", orders[0].Price.String())
assert.Equal(t, "1994", orders[0].Price.String())
assert.Equal(t, 126420, int(orders[0].Size))
}

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

// first round, three orders moving all pool's to the upper boundary of the shortest
assert.Equal(t, "2053", orders[0].Price.String())
assert.Equal(t, "2053", orders[1].Price.String())
assert.Equal(t, "2053", orders[2].Price.String())
assert.Equal(t, "1949", orders[0].Price.String())
assert.Equal(t, "1949", orders[1].Price.String())
assert.Equal(t, "1949", orders[2].Price.String())

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

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

func testBestPricesAndVolume(t *testing.T) {
Expand Down
9 changes: 7 additions & 2 deletions core/execution/amm/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -529,18 +529,23 @@ func (p *Pool) OrderbookShape(from, to *num.Uint) ([]*types.Order, []*types.Orde
return buys, sells
}

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

// dy = x*y / (x - dx) - y
// where y and x are the balances on either side of the pool, and dx is the change in volume
// then the trade price is dy/dx
dx := num.DecimalFromInt64(int64(volume))
if side == types.SideSell {
// if incoming order is a sell, the AMM is buying so reducing cash balance so dx is negative
dx = dx.Neg()
}

dy := x.Mul(y).Div(x.Sub(dx)).Sub(y)

// dy / dx
price, overflow := num.UintFromDecimal(dy.Div(dx))
price, overflow := num.UintFromDecimal(dy.Div(dx).Abs())
if overflow {
panic("calculated negative price")
}
Expand Down
10 changes: 10 additions & 0 deletions core/execution/amm/pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,16 @@ func TestNotebook(t *testing.T) {
ensurePosition(t, p.pos, 500, lowmid.Clone())
fairPrice = p.pool.BestPrice(nil)
assert.Equal(t, "1854", fairPrice.String())

// fair price is 2000 and the AMM quotes a best-buy at 1999 so incoming SELL should have a price <= 1999
ensurePositionN(t, p.pos, 0, lowmid.Clone(), 2)
price := p.pool.PriceForVolume(100, types.SideSell)
assert.Equal(t, "1984", price.String())

// fair price is 2000 and the AMM quotes a best-buy at 2001 so incoming BUY should have a price >= 2001
ensurePositionN(t, p.pos, 0, lowmid.Clone(), 2)
price = p.pool.PriceForVolume(100, types.SideBuy)
assert.Equal(t, "2014", price.String())
}

type tstPool struct {
Expand Down
96 changes: 48 additions & 48 deletions core/integration/features/amm/0090-VAMM-006-014.feature

Large diffs are not rendered by default.

68 changes: 34 additions & 34 deletions core/integration/features/amm/0090-VAMM-020.feature

Large diffs are not rendered by default.

116 changes: 57 additions & 59 deletions core/integration/features/amm/0090-VAMM-021.feature

Large diffs are not rendered by default.

35 changes: 18 additions & 17 deletions core/integration/features/amm/0090-VAMM-028.feature
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ Feature: Ensure the vAMM positions follow the market correctly
| vamm2-id | -74 | -304 | 0 | true |
| vamm1-id | -74 | 0 | 0 | true |

@VAMM
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.
# Move mid price to 90 in one go. A volume of 347 is the minimum required, 346 only gets us to 91
When the parties place the following orders:
Expand All @@ -267,13 +268,13 @@ Feature: Ensure the vAMM positions follow the market correctly
| 100 | TRADING_MODE_CONTINUOUS | 13915 | 1000 | 348 | 100 | 90 | 90 | 91 | 89 |
And the following trades should be executed:
| buyer | price | size | seller | is amm |
| vamm1-id | 105 | 347 | party5 | true |
| vamm1-id | 95 | 347 | party5 | true |
# Check vAMM position
When the network moves ahead "1" blocks
Then the parties should have the following profit and loss:
| party | volume | unrealised pnl | realised pnl | is amm |
| party1 | 1 | 5 | 0 | |
| party2 | -1 | -5 | 0 | |
| party1 | 1 | -5 | 0 | |
| party2 | -1 | 5 | 0 | |
| party5 | -347 | 0 | 0 | |
| vamm1-id | 347 | 0 | 0 | true |

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

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

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

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

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

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

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

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

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

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

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

@VAMM
Expand Down
10 changes: 5 additions & 5 deletions core/integration/features/amm/0090-VAMM-rebase.feature
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,13 @@ Feature: vAMM rebasing when created or amended
# second AMM has its base 5 away from the first AMM so it must submit a rebasing-order
And the following trades should be executed:
| buyer | price | size | seller | is amm |
| vamm1-id | 101 | 140 | vamm2-id | true |
| vamm1-id | 98 | 140 | vamm2-id | true |
Then the network moves ahead "1" blocks

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


@VAMM
Expand Down Expand Up @@ -167,7 +167,7 @@ Feature: vAMM rebasing when created or amended

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

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


@VAMM
Expand Down

0 comments on commit 9f9059c

Please sign in to comment.