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 13 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
7 changes: 7 additions & 0 deletions proto/neutron/dex/limit_order_tranche.proto
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,11 @@ message LimitOrderTranche {
(gogoproto.nullable) = false,
(gogoproto.jsontag) = "price_taker_to_maker"
];
// 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/v4/utils/math.PrecDec",
(gogoproto.nullable) = false,
(gogoproto.jsontag) = "maker_price"
];
}
7 changes: 4 additions & 3 deletions proto/neutron/dex/pool_reserves.proto
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,11 @@ message PoolReserves {
(gogoproto.nullable) = false,
(gogoproto.jsontag) = "price_taker_to_maker"
];
string price_opposite_taker_to_maker = 4 [
(gogoproto.moretags) = "yaml:\"price_opposite_taker_to_maker\"",
// 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/v4/utils/math.PrecDec",
(gogoproto.nullable) = false,
(gogoproto.jsontag) = "price_opposite_taker_to_maker"
(gogoproto.jsontag) = "maker_price"
];
}
4 changes: 2 additions & 2 deletions x/dex/keeper/limit_order_tranche.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,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 +29,7 @@ func NewLimitOrderTranche(
TotalMakerDenom: math.ZeroInt(),
TotalTakerDenom: math.ZeroInt(),
ExpirationTime: goodTil,
PriceTakerToMaker: priceTakerToMaker,
MakerPrice: price,
}, nil
}

Expand Down
12 changes: 6 additions & 6 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 @@ -56,7 +56,7 @@ func (k Keeper) Swap(
// 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 @@ -148,9 +148,9 @@ func (k Keeper) TakerLimitOrderSwap(
return sdk.Coin{}, sdk.Coin{}, types.ErrLimitPriceNotSatisfied
}

truePrice := math_utils.NewPrecDecFromInt(totalOutCoin.Amount).QuoInt(totalInCoin.Amount)
truePrice := math_utils.NewPrecDecFromInt(totalInCoin.Amount).QuoInt(totalOutCoin.Amount)

if truePrice.LT(limitPrice) {
if truePrice.GT(limitPrice) {
return sdk.Coin{}, sdk.Coin{}, types.ErrLimitPriceNotSatisfied
}

Expand Down Expand Up @@ -180,9 +180,9 @@ func (k Keeper) MakerLimitOrderSwap(
remainingIn := amountIn.Sub(totalInCoin.Amount)
expectedOutMakerPortion := limitPrice.MulInt(remainingIn).Ceil()
totalExpectedOut := expectedOutMakerPortion.Add(math_utils.NewPrecDecFromInt(totalOutCoin.Amount))
truePrice := totalExpectedOut.QuoInt(amountIn)
truePrice := math_utils.NewPrecDecFromInt(amountIn).Quo(totalExpectedOut)

if truePrice.LT(limitPrice) {
if truePrice.GT(limitPrice) {
return sdk.Coin{}, sdk.Coin{}, false, types.ErrLimitPriceNotSatisfied
}
}
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/v4/x/dex/migrations/v3"
v4 "github.com/neutron-org/neutron/v4/x/dex/migrations/v4"
v5 "github.com/neutron-org/neutron/v4/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)
}
2 changes: 1 addition & 1 deletion x/dex/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func (k MsgServer) PlaceLimitOrder(
if err != nil {
return &types.MsgPlaceLimitOrderResponse{}, err
}
tickIndex := msg.TickIndexInToOut
tickIndex := -msg.TickIndexInToOut
if msg.LimitSellPrice != nil {
tickIndex, err = types.CalcTickIndexFromPrice(*msg.LimitSellPrice)
if err != nil {
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
18 changes: 8 additions & 10 deletions x/dex/keeper/place_limit_order.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"cosmossdk.io/math"
sdk "github.com/cosmos/cosmos-sdk/types"

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

Expand Down Expand Up @@ -104,23 +103,22 @@ 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)
invLimitPrice, err := types.CalcPrice(-tickIndexInToOut)
if err != nil {
return trancheKey, totalIn, swapInCoin, swapOutCoin, math.ZeroInt(), err
}

// Ensure that after rounding user will get at least 1 token out.
err = types.ValidateFairOutput(amountIn, limitPrice)
err = types.ValidateFairOutput(amountIn, invLimitPrice)
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, orderType)
swapInCoin, swapOutCoin, err = k.TakerLimitOrderSwap(ctx, *takerTradePairID, amountIn, maxAmountOut, invLimitPrice, orderType)
} else {
swapInCoin, swapOutCoin, orderFilled, err = k.MakerLimitOrderSwap(ctx, *takerTradePairID, amountIn, limitPrice)
swapInCoin, swapOutCoin, orderFilled, err = k.MakerLimitOrderSwap(ctx, *takerTradePairID, amountIn, invLimitPrice)
}
if err != nil {
return trancheKey, totalIn, swapInCoin, swapOutCoin, math.ZeroInt(), err
Expand All @@ -130,12 +128,12 @@ func (k Keeper) ExecutePlaceLimitOrder(
amountLeft = amountLeft.Sub(swapInCoin.Amount)

makerTradePairID := takerTradePairID.Reversed()
makerTickIndexTakerToMaker := tickIndexInToOut * -1
tickIndexTakerToMaker := tickIndexInToOut
var placeTranche *types.LimitOrderTranche
placeTranche, err = k.GetOrInitPlaceTranche(
ctx,
makerTradePairID,
makerTickIndexTakerToMaker,
tickIndexTakerToMaker,
goodTil,
orderType,
)
Expand All @@ -147,7 +145,7 @@ func (k Keeper) ExecutePlaceLimitOrder(
trancheUser := k.GetOrInitLimitOrderTrancheUser(
ctx,
makerTradePairID,
makerTickIndexTakerToMaker,
tickIndexTakerToMaker,
trancheKey,
orderType,
receiverAddr.String(),
Expand All @@ -161,7 +159,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, invLimitPrice)
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
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm, could you clarify why do you change a code for the previous migration?

I don't think we are supposed to change old migrations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

MustCalcPrice fn has been inverted so to achieve the same migration we must flip the sign. The output of the migration should be the same

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the question was - do we get the same migration output in 2 cases

  1. Our case. User migrates 3 -> 4 (old one) (neutron v4.2.1) and then 4 -> 5. 3 -> old 4 -> 5
  2. Imported dex module to third parthy project. User migrates 3 vesrion -> 5 version (current one). 3 -> new 4 -> 5

I dont think it's safe in terms compatibility to modify already released migrations

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, that's exactly what i meant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it's an issue. You can see that the tests are unchanged. So the migrations still produce the same output. @swelf19 @pr0n00gler

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, seems you are right. No changes in output. Dont see any problems with a migration now.

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")))
}
109 changes: 109 additions & 0 deletions x/dex/migrations/v5/store.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
package v4

import (
errorsmod "cosmossdk.io/errors"
"cosmossdk.io/store/prefix"
storetypes "cosmossdk.io/store/types"
"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/neutron-org/neutron/v4/x/dex/types"
)

// MigrateStore performs in-place store migrations.
// v5 adds a new field `MakerPrice` to all tickLiquidity. It must be calculated and added
func MigrateStore(ctx sdk.Context, cdc codec.BinaryCodec, storeKey storetypes.StoreKey) error {
if err := migrateTickLiquidityPrices(ctx, cdc, storeKey); err != nil {
return err
}

if err := migrateInactiveTranchePrices(ctx, cdc, storeKey); err != nil {
return err
}

return nil
}

type migrationUpdate struct {
key []byte
val []byte
}

func migrateTickLiquidityPrices(ctx sdk.Context, cdc codec.BinaryCodec, storeKey storetypes.StoreKey) error {
ctx.Logger().Info("Migrating TickLiquidity Prices...")

// Iterate through all tickLiquidity
store := prefix.NewStore(ctx.KVStore(storeKey), types.KeyPrefix(types.TickLiquidityKeyPrefix))
iterator := storetypes.KVStorePrefixIterator(store, []byte{})
ticksToUpdate := make([]migrationUpdate, 0)

for ; iterator.Valid(); iterator.Next() {
var tickLiq types.TickLiquidity
var updatedTickLiq types.TickLiquidity
cdc.MustUnmarshal(iterator.Value(), &tickLiq)
// Add MakerPrice
switch liquidity := tickLiq.Liquidity.(type) {
case *types.TickLiquidity_LimitOrderTranche:
liquidity.LimitOrderTranche.MakerPrice = types.MustCalcPrice(liquidity.LimitOrderTranche.Key.TickIndexTakerToMaker)
updatedTickLiq = types.TickLiquidity{Liquidity: liquidity}
case *types.TickLiquidity_PoolReserves:
poolReservesKey := liquidity.PoolReserves.Key
liquidity.PoolReserves.MakerPrice = types.MustCalcPrice(poolReservesKey.TickIndexTakerToMaker)
updatedTickLiq = types.TickLiquidity{Liquidity: liquidity}

default:
panic("Tick does not contain valid liqudityType")
}

bz := cdc.MustMarshal(&updatedTickLiq)
ticksToUpdate = append(ticksToUpdate, migrationUpdate{key: iterator.Key(), val: bz})

}

err := iterator.Close()
if err != nil {
return errorsmod.Wrap(err, "iterator failed to close during migration")
}

// Store the updated TickLiquidity
for _, v := range ticksToUpdate {
store.Set(v.key, v.val)
}

ctx.Logger().Info("Finished migrating TickLiquidity Prices...")

return nil
}

func migrateInactiveTranchePrices(ctx sdk.Context, cdc codec.BinaryCodec, storeKey storetypes.StoreKey) error {
ctx.Logger().Info("Migrating InactiveLimitOrderTranche Prices...")

// Iterate through all InactiveTranches
store := prefix.NewStore(ctx.KVStore(storeKey), types.KeyPrefix(types.InactiveLimitOrderTrancheKeyPrefix))
iterator := storetypes.KVStorePrefixIterator(store, []byte{})
ticksToUpdate := make([]migrationUpdate, 0)

for ; iterator.Valid(); iterator.Next() {
var tranche types.LimitOrderTranche
cdc.MustUnmarshal(iterator.Value(), &tranche)
// Add MakerPrice
tranche.MakerPrice = types.MustCalcPrice(tranche.Key.TickIndexTakerToMaker)

bz := cdc.MustMarshal(&tranche)
ticksToUpdate = append(ticksToUpdate, migrationUpdate{key: iterator.Key(), val: bz})
}

err := iterator.Close()
if err != nil {
return errorsmod.Wrap(err, "iterator failed to close during migration")
}

// Store the updated InactiveTranches
for _, v := range ticksToUpdate {
store.Set(v.key, v.val)
}

ctx.Logger().Info("Finished migrating InactiveLimitOrderTranche Prices...")

return nil
}
Loading
Loading