Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore: Flip Dex Price Logic #NTRN-355 #624

Merged
merged 26 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
c136fd8
save price flip
jcompagni10 Jul 11, 2024
7b8592c
Core logic changes
jcompagni10 Jul 11, 2024
67b5a6f
more logic fixes
jcompagni10 Jul 12, 2024
287e173
Merge remote-tracking branch 'origin/main' into chore/reverse-dex-prices
jcompagni10 Jul 17, 2024
79db3f5
Add back old migrations + small fixes
jcompagni10 Jul 23, 2024
83221b9
misc cleanup
jcompagni10 Jul 25, 2024
4b5997b
Merge remote-tracking branch 'origin/main' into chore/reverse-dex-prices
jcompagni10 Jul 25, 2024
a8386a9
fix issues from merge
jcompagni10 Jul 25, 2024
c5ea117
Add comment to maker_price field
jcompagni10 Jul 31, 2024
7a49de3
more comments
jcompagni10 Jul 31, 2024
12433ff
proto re-gen
jcompagni10 Aug 5, 2024
7bfbbe2
Merge remote-tracking branch 'origin' into chore/reverse-dex-prices
jcompagni10 Aug 5, 2024
955ef2c
Merge remote-tracking branch 'origin/main' into chore/reverse-dex-prices
jcompagni10 Aug 20, 2024
a362a85
Merge remote-tracking branch 'origin/main' into chore/reverse-dex-prices
jcompagni10 Sep 3, 2024
032da0e
fix test math
jcompagni10 Sep 3, 2024
7e291cc
deprecate old price_taker_to_maker
jcompagni10 Sep 3, 2024
2a95035
Keeper price_opposite_taker_to_maker for compatibility
jcompagni10 Sep 19, 2024
2737dce
Merge remote-tracking branch 'origin/main' into chore/reverse-dex-prices
jcompagni10 Sep 19, 2024
5c5de96
Merge remote-tracking branch 'origin/main' into chore/reverse-dex-prices
jcompagni10 Sep 24, 2024
cfbfed9
fix merge
jcompagni10 Sep 24, 2024
3ad0aa0
Merge remote-tracking branch 'origin/main' into chore/reverse-dex-prices
jcompagni10 Sep 24, 2024
108fdb2
lint
jcompagni10 Sep 24, 2024
d7400db
Merge remote-tracking branch 'origin/main' into chore/reverse-dex-prices
jcompagni10 Oct 14, 2024
a6b2f3c
avoid extra tick flip
jcompagni10 Oct 15, 2024
c80784d
fix comments
jcompagni10 Oct 15, 2024
019edba
fmt
jcompagni10 Oct 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion proto/neutron/dex/limit_order_tranche.proto
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,19 @@ message LimitOrderTranche {
(gogoproto.stdtime) = true,
(gogoproto.nullable) = true
];
// DEPRECATED: price_taker_to_maker will be removed in future release, `maker_price` should always be used.
string price_taker_to_maker = 7 [
(gogoproto.moretags) = "yaml:\"price_taker_to_maker\"",
(gogoproto.customtype) = "github.com/neutron-org/neutron/v5/utils/math.PrecDec",
(gogoproto.nullable) = false,
(gogoproto.jsontag) = "price_taker_to_maker"
(gogoproto.jsontag) = "price_taker_to_maker",
deprecated = true
];
// This is the price of the LimitOrder denominated in the opposite token. (ie. 1 TokenA with a maker_price of 10 is worth 10 TokenB )
string maker_price = 8 [
(gogoproto.moretags) = "yaml:\"maker_price\"",
(gogoproto.customtype) = "github.com/neutron-org/neutron/v5/utils/math.PrecDec",
(gogoproto.nullable) = false,
(gogoproto.jsontag) = "maker_price"
];
}
16 changes: 14 additions & 2 deletions proto/neutron/dex/pool_reserves.proto
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,28 @@ message PoolReserves {
(gogoproto.jsontag) = "reserves_maker_denom",
(gogoproto.nullable) = false
];
// DEPRECATED: price_taker_to_maker will be removed in future release, `maker_price` should always be used.
string price_taker_to_maker = 3 [
(gogoproto.moretags) = "yaml:\"price_taker_to_maker\"",
(gogoproto.customtype) = "github.com/neutron-org/neutron/v5/utils/math.PrecDec",
(gogoproto.nullable) = false,
(gogoproto.jsontag) = "price_taker_to_maker"
(gogoproto.jsontag) = "price_taker_to_maker",
deprecated = true
];
// DEPRECATED: price_opposite_taker_maker was an internal implementation detail and will be removed in a future release.
// It is being kept strictly for backwards compatibility. The actual field value is unused.
string price_opposite_taker_to_maker = 4 [
(gogoproto.moretags) = "yaml:\"price_opposite_taker_to_maker\"",
(gogoproto.customtype) = "github.com/neutron-org/neutron/v5/utils/math.PrecDec",
(gogoproto.nullable) = false,
(gogoproto.jsontag) = "price_opposite_taker_to_maker"
(gogoproto.jsontag) = "price_opposite_taker_to_maker",
deprecated = true
];
// This is the price of the PoolReserves denominated in the opposite token. (ie. 1 TokenA with a maker_price of 10 is worth 10 TokenB )
string maker_price = 5 [
(gogoproto.moretags) = "yaml:\"maker_price\"",
(gogoproto.customtype) = "github.com/neutron-org/neutron/v5/utils/math.PrecDec",
(gogoproto.nullable) = false,
(gogoproto.jsontag) = "maker_price"
swelf19 marked this conversation as resolved.
Show resolved Hide resolved
];
}
4 changes: 3 additions & 1 deletion x/dex/keeper/grpc_query_simulate_place_limit_order.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"cosmossdk.io/errors"
sdk "github.com/cosmos/cosmos-sdk/types"

math_utils "github.com/neutron-org/neutron/v5/utils/math"
"github.com/neutron-org/neutron/v5/x/dex/types"
)

Expand Down Expand Up @@ -36,7 +37,8 @@ func (k Keeper) SimulatePlaceLimitOrder(
}
tickIndex := msg.TickIndexInToOut
if msg.LimitSellPrice != nil {
tickIndex, err = types.CalcTickIndexFromPrice(*msg.LimitSellPrice)
limitBuyPrice := math_utils.OnePrecDec().Quo(*msg.LimitSellPrice)
tickIndex, err = types.CalcTickIndexFromPrice(limitBuyPrice)
if err != nil {
return nil, errors.Wrapf(err, "invalid LimitSellPrice %s", msg.LimitSellPrice.String())
}
Expand Down
8 changes: 4 additions & 4 deletions x/dex/keeper/integration_cancellimitorder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,8 @@ func (s *DexTestSuite) TestCancelPartiallyFilledWithdrawFails() {
s.aliceCancelsLimitSell(trancheKey)

// Then alice gets back remaining ~37 BIGTokenA LO reserves & 10 BIGTokenB taker tokens
s.assertAliceBalancesInt(sdkmath.NewInt(37786094), sdkmath.NewInt(10000000))
s.assertDexBalancesInt(sdkmath.OneInt(), sdkmath.ZeroInt())
s.assertAliceBalancesInt(sdkmath.NewInt(37786094), sdkmath.NewInt(9999999))
s.assertDexBalancesInt(sdkmath.OneInt(), sdkmath.OneInt())

// Assert that the LimitOrderTrancheUser has been deleted
_, found := s.App.DexKeeper.GetLimitOrderTrancheUser(s.Ctx, s.alice.String(), trancheKey)
Expand Down Expand Up @@ -304,8 +304,8 @@ func (s *DexTestSuite) TestCancelPartiallyFilledMultiUser2() {
s.aliceCancelsLimitSell(trancheKey)

// THEN alice gets back remaining ~38 BIGTokenA LO reserves & 10 BIGTokenB taker tokens
s.assertAliceBalancesInt(sdkmath.NewInt(37786094), sdkmath.NewInt(10000000))
s.assertDexBalancesInt(sdkmath.NewInt(37786096), sdkmath.NewInt(10000000))
s.assertAliceBalancesInt(sdkmath.NewInt(37786094), sdkmath.NewInt(9999999))
s.assertDexBalancesInt(sdkmath.NewInt(37786096), sdkmath.NewInt(10000001))

// THEN carol swap through more of the limitorder
s.carolLimitSells("TokenB", -2001, 20, types.LimitOrderType_FILL_OR_KILL)
Expand Down
6 changes: 4 additions & 2 deletions x/dex/keeper/limit_order_tranche.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
storetypes "cosmossdk.io/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"

math_utils "github.com/neutron-org/neutron/v5/utils/math"
"github.com/neutron-org/neutron/v5/x/dex/types"
"github.com/neutron-org/neutron/v5/x/dex/utils"
)
Expand All @@ -18,7 +19,7 @@ func NewLimitOrderTranche(
limitOrderTrancheKey *types.LimitOrderTrancheKey,
goodTil *time.Time,
) (*types.LimitOrderTranche, error) {
priceTakerToMaker, err := limitOrderTrancheKey.PriceTakerToMaker()
price, err := limitOrderTrancheKey.Price()
if err != nil {
return nil, err
}
Expand All @@ -29,7 +30,8 @@ func NewLimitOrderTranche(
TotalMakerDenom: math.ZeroInt(),
TotalTakerDenom: math.ZeroInt(),
ExpirationTime: goodTil,
PriceTakerToMaker: priceTakerToMaker,
MakerPrice: price,
swelf19 marked this conversation as resolved.
Show resolved Hide resolved
PriceTakerToMaker: math_utils.OnePrecDec().Quo(price),
}, nil
}

Expand Down
8 changes: 4 additions & 4 deletions x/dex/keeper/liquidity.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (k Keeper) Swap(
}

// break as soon as we iterated past limitPrice
if limitPrice != nil && liq.Price().LT(*limitPrice) {
if limitPrice != nil && liq.Price().GT(*limitPrice) {
break
}

Expand All @@ -52,11 +52,11 @@ func (k Keeper) Swap(
// this avoids unnecessary iteration since outAmount will always be 0 going forward
// this also catches the normal exit case where remainingTakerDenom == 0

// NOTE: In theory this check should be: price * remainingTakerDenom < 1
// NOTE: In theory this check should be: remainingTakerDenom / price < 1
// but due to rounding and inaccuracy of fixed decimal math, it is possible
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should update the comment on ln55

// for liq.swap to use the full the amount of taker liquidity and have a leftover
// amount of the taker Denom > than 1 token worth of maker denom
if liq.Price().MulInt(remainingTakerDenom).LT(math_utils.NewPrecDec(2)) {
if math_utils.NewPrecDecFromInt(remainingTakerDenom).Quo(liq.Price()).LT(math_utils.NewPrecDec(2)) {
orderFilled = true
break
}
Expand Down Expand Up @@ -181,7 +181,7 @@ func (k Keeper) MakerLimitOrderSwap(

if totalInCoin.Amount.IsPositive() {
remainingIn := amountIn.Sub(totalInCoin.Amount)
expectedOutMakerPortion := limitPrice.MulInt(remainingIn)
expectedOutMakerPortion := math_utils.NewPrecDecFromInt(remainingIn).Quo(limitPrice)
totalExpectedOut := expectedOutMakerPortion.Add(math_utils.NewPrecDecFromInt(totalOutCoin.Amount))
truePrice := totalExpectedOut.QuoInt(amountIn)

Expand Down
4 changes: 2 additions & 2 deletions x/dex/keeper/liquidity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -483,8 +483,8 @@ func (s *DexTestSuite) TestSwap1To0LOMaxAmountNotUsed() {
tokenIn, tokenOut := s.swapWithMaxOut("TokenB", "TokenA", 8, 15)

// THEN swap should return 8 BIGTokenB in and ~8 BIGTokenA out
s.assertSwapOutputInt(tokenIn, sdkmath.NewInt(8_000_000), tokenOut, sdkmath.NewInt(8_000_800))
s.assertTickBalancesInt(sdkmath.NewInt(1_999_200), sdkmath.NewInt(8_000_000))
s.assertSwapOutputInt(tokenIn, sdkmath.NewInt(8_000_000), tokenOut, sdkmath.NewInt(8_000_799))
s.assertTickBalancesInt(sdkmath.NewInt(1_999_201), sdkmath.NewInt(8_000_000))
}

func (s *DexTestSuite) TestSwap0To1LOMaxAmountUsedMultiTick() {
Expand Down
6 changes: 6 additions & 0 deletions x/dex/keeper/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

v3 "github.com/neutron-org/neutron/v5/x/dex/migrations/v3"
v4 "github.com/neutron-org/neutron/v5/x/dex/migrations/v4"
v5 "github.com/neutron-org/neutron/v5/x/dex/migrations/v5"
)

// Migrator is a struct for handling in-place store migrations.
Expand All @@ -26,3 +27,8 @@ func (m Migrator) Migrate2to3(ctx sdk.Context) error {
func (m Migrator) Migrate3to4(ctx sdk.Context) error {
return v4.MigrateStore(ctx, m.keeper.cdc, m.keeper.storeKey)
}

// Migrate4to5 migrates from version 4 to 5.
func (m Migrator) Migrate4to5(ctx sdk.Context) error {
return v5.MigrateStore(ctx, m.keeper.cdc, m.keeper.storeKey)
}
4 changes: 3 additions & 1 deletion x/dex/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

math_utils "github.com/neutron-org/neutron/v5/utils/math"
"github.com/neutron-org/neutron/v5/x/dex/types"
)

Expand Down Expand Up @@ -136,7 +137,8 @@ func (k MsgServer) PlaceLimitOrder(
}
tickIndex := msg.TickIndexInToOut
if msg.LimitSellPrice != nil {
tickIndex, err = types.CalcTickIndexFromPrice(*msg.LimitSellPrice)
limitBuyPrice := math_utils.OnePrecDec().Quo(*msg.LimitSellPrice)
tickIndex, err = types.CalcTickIndexFromPrice(limitBuyPrice)
if err != nil {
return &types.MsgPlaceLimitOrderResponse{}, errors.Wrapf(err, "invalid LimitSellPrice %s", msg.LimitSellPrice.String())
}
Expand Down
2 changes: 1 addition & 1 deletion x/dex/keeper/multihop_swap.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func (k Keeper) HopsToRouteData(
if !found {
return routeArr, types.ErrLimitPriceNotSatisfied
}
priceAcc = priceAcc.Mul(price)
priceAcc = priceAcc.Quo(price)
routeArr[index] = MultihopStep{
tradePairID: tradePairID,
RemainingBestPrice: priceAcc,
Expand Down
23 changes: 11 additions & 12 deletions x/dex/keeper/place_limit_order.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,30 +109,29 @@ func (k Keeper) ExecutePlaceLimitOrder(
) (trancheKey string, totalIn math.Int, swapInCoin, swapOutCoin sdk.Coin, sharesIssued math.Int, err error) {
amountLeft := amountIn

var limitPrice math_utils.PrecDec
limitPrice, err = types.CalcPrice(tickIndexInToOut)

limitBuyPrice, err := types.CalcPrice(tickIndexInToOut)
if err != nil {
return trancheKey, totalIn, swapInCoin, swapOutCoin, math.ZeroInt(), err
}

// Use limitPrice for minAvgSellPrice if it has not be specified
minAvgSellPrice := limitPrice
// Use limitPrice for minAvgSellPrice if it has not been specified
minAvgSellPrice := math_utils.OnePrecDec().Quo(limitBuyPrice)

if minAvgSellPriceP != nil {
minAvgSellPrice = *minAvgSellPriceP
}

// Ensure that after rounding user will get at least 1 token out.
err = types.ValidateFairOutput(amountIn, limitPrice)
err = types.ValidateFairOutput(amountIn, limitBuyPrice)
if err != nil {
return trancheKey, totalIn, swapInCoin, swapOutCoin, math.ZeroInt(), err
}

var orderFilled bool
if orderType.IsTakerOnly() {
swapInCoin, swapOutCoin, err = k.TakerLimitOrderSwap(ctx, *takerTradePairID, amountIn, maxAmountOut, limitPrice, minAvgSellPrice, orderType)
swapInCoin, swapOutCoin, err = k.TakerLimitOrderSwap(ctx, *takerTradePairID, amountIn, maxAmountOut, limitBuyPrice, minAvgSellPrice, orderType)
} else {
swapInCoin, swapOutCoin, orderFilled, err = k.MakerLimitOrderSwap(ctx, *takerTradePairID, amountIn, limitPrice, minAvgSellPrice)
swapInCoin, swapOutCoin, orderFilled, err = k.MakerLimitOrderSwap(ctx, *takerTradePairID, amountIn, limitBuyPrice, minAvgSellPrice)
}
if err != nil {
return trancheKey, totalIn, swapInCoin, swapOutCoin, math.ZeroInt(), err
Expand All @@ -142,12 +141,12 @@ func (k Keeper) ExecutePlaceLimitOrder(
amountLeft = amountLeft.Sub(swapInCoin.Amount)

makerTradePairID := takerTradePairID.Reversed()
makerTickIndexTakerToMaker := tickIndexInToOut * -1
tickIndexTakerToMaker := tickIndexInToOut * -1
var placeTranche *types.LimitOrderTranche
placeTranche, err = k.GetOrInitPlaceTranche(
ctx,
makerTradePairID,
makerTickIndexTakerToMaker,
tickIndexTakerToMaker,
goodTil,
orderType,
)
Expand All @@ -159,7 +158,7 @@ func (k Keeper) ExecutePlaceLimitOrder(
trancheUser := k.GetOrInitLimitOrderTrancheUser(
ctx,
makerTradePairID,
makerTickIndexTakerToMaker,
tickIndexTakerToMaker,
trancheKey,
orderType,
receiverAddr.String(),
Expand All @@ -173,7 +172,7 @@ func (k Keeper) ExecutePlaceLimitOrder(
// NOTE: This does mean that a successful taker leg of the trade will be thrown away since the entire tx will fail.
// In most circumstances this seems preferable to executing the taker leg and exiting early before placing a maker
// order with the remaining liquidity.
err = types.ValidateFairOutput(amountLeft, limitPrice)
err = types.ValidateFairOutput(amountLeft, limitBuyPrice)
if err != nil {
return trancheKey, totalIn, swapInCoin, swapOutCoin, math.ZeroInt(), err
}
Expand Down
7 changes: 3 additions & 4 deletions x/dex/migrations/v4/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,11 @@ func migrateTickLiquidityPrices(ctx sdk.Context, cdc codec.BinaryCodec, storeKey
// Recalculate all prices
switch liquidity := tickLiq.Liquidity.(type) {
case *types.TickLiquidity_LimitOrderTranche:
liquidity.LimitOrderTranche.PriceTakerToMaker = types.MustCalcPrice(liquidity.LimitOrderTranche.Key.TickIndexTakerToMaker)
liquidity.LimitOrderTranche.PriceTakerToMaker = types.MustCalcPrice(-liquidity.LimitOrderTranche.Key.TickIndexTakerToMaker)
updatedTickLiq = types.TickLiquidity{Liquidity: liquidity}
case *types.TickLiquidity_PoolReserves:
poolReservesKey := liquidity.PoolReserves.Key
liquidity.PoolReserves.PriceTakerToMaker = types.MustCalcPrice(poolReservesKey.TickIndexTakerToMaker)
liquidity.PoolReserves.PriceOppositeTakerToMaker = poolReservesKey.Counterpart().MustPriceTakerToMaker()
liquidity.PoolReserves.PriceTakerToMaker = types.MustCalcPrice(-poolReservesKey.TickIndexTakerToMaker)
updatedTickLiq = types.TickLiquidity{Liquidity: liquidity}

default:
Expand Down Expand Up @@ -90,7 +89,7 @@ func migrateInactiveTranchePrices(ctx sdk.Context, cdc codec.BinaryCodec, storeK
var tranche types.LimitOrderTranche
cdc.MustUnmarshal(iterator.Value(), &tranche)
// Recalculate price
tranche.PriceTakerToMaker = types.MustCalcPrice(tranche.Key.TickIndexTakerToMaker)
tranche.PriceTakerToMaker = types.MustCalcPrice(-tranche.Key.TickIndexTakerToMaker)

bz := cdc.MustMarshal(&tranche)
ticksToUpdate = append(ticksToUpdate, migrationUpdate{key: iterator.Key(), val: bz})
Expand Down
9 changes: 4 additions & 5 deletions x/dex/migrations/v4/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,8 @@ func (suite *V4DexMigrationTestSuite) TestPriceUpdates() {
Fee: 1,
}
poolReserves := &types.PoolReserves{
Key: poolKey,
PriceTakerToMaker: math.ZeroPrecDec(),
PriceOppositeTakerToMaker: math.ZeroPrecDec(),
Key: poolKey,
PriceTakerToMaker: math.ZeroPrecDec(),
}
app.DexKeeper.SetPoolReserves(ctx, poolReserves)

Expand All @@ -60,6 +59,7 @@ func (suite *V4DexMigrationTestSuite) TestPriceUpdates() {

// Check LimitOrderTranche has correct price
newTranche := app.DexKeeper.GetLimitOrderTranche(ctx, trancheKey)

suite.True(newTranche.PriceTakerToMaker.Equal(math.MustNewPrecDecFromStr("1.005012269623051203500693815")))

// check InactiveLimitOrderTranche has correct price
Expand All @@ -68,6 +68,5 @@ func (suite *V4DexMigrationTestSuite) TestPriceUpdates() {

// Check PoolReserves has the correct prices
newPool, _ := app.DexKeeper.GetPoolReserves(ctx, poolKey)
suite.True(newPool.PriceTakerToMaker.Equal(math.MustNewPrecDecFromStr("0.002479495864288162666675923")))
suite.True(newPool.PriceOppositeTakerToMaker.Equal(math.MustNewPrecDecFromStr("403.227141612124702272520931931")))
suite.True(newPool.PriceTakerToMaker.Equal(math.MustNewPrecDecFromStr("0.002479495864288162666675934")))
}
Loading
Loading