diff --git a/proto/neutron/dex/limit_order_tranche_user.proto b/proto/neutron/dex/limit_order_tranche_user.proto index 5ceb413e5..89cd817b6 100644 --- a/proto/neutron/dex/limit_order_tranche_user.proto +++ b/proto/neutron/dex/limit_order_tranche_user.proto @@ -24,6 +24,7 @@ message LimitOrderTrancheUser { (gogoproto.nullable) = false, (gogoproto.jsontag) = "shares_withdrawn" ]; + // TODO: remove this in next release. It is no longer used string shares_cancelled = 7 [ (gogoproto.moretags) = "yaml:\"shares_cancelled\"", (gogoproto.customtype) = "cosmossdk.io/math.Int", diff --git a/x/dex/genesis_test.go b/x/dex/genesis_test.go index 06dae9b4e..ac944c274 100644 --- a/x/dex/genesis_test.go +++ b/x/dex/genesis_test.go @@ -27,7 +27,6 @@ func TestGenesis(t *testing.T) { Address: "fakeAddr", SharesOwned: math.NewInt(10), SharesWithdrawn: math.NewInt(0), - SharesCancelled: math.NewInt(0), }, { TradePairId: &types.TradePairID{ @@ -39,7 +38,6 @@ func TestGenesis(t *testing.T) { Address: "fakeAddr", SharesOwned: math.NewInt(10), SharesWithdrawn: math.NewInt(0), - SharesCancelled: math.NewInt(0), }, }, TickLiquidityList: []*types.TickLiquidity{ diff --git a/x/dex/keeper/core.go b/x/dex/keeper/core.go index 04e0d29fa..eb53bbd4b 100644 --- a/x/dex/keeper/core.go +++ b/x/dex/keeper/core.go @@ -509,17 +509,24 @@ func (k Keeper) CancelLimitOrderCore( return types.ErrActiveLimitOrderNotFound } - amountToCancel := tranche.RemoveTokenIn(trancheUser) - trancheUser.SharesCancelled = trancheUser.SharesCancelled.Add(amountToCancel) + makerAmountToReturn := tranche.RemoveTokenIn(trancheUser) + _, takerAmountOut := tranche.Withdraw(trancheUser) - if amountToCancel.IsPositive() { - coinOut := sdk.NewCoin(tradePairID.MakerDenom, amountToCancel) + trancheUser.SharesWithdrawn = trancheUser.SharesOwned + // Remove the canceled shares from the limitOrder + tranche.TotalMakerDenom = tranche.TotalMakerDenom.Sub(trancheUser.SharesOwned) + tranche.TotalTakerDenom = tranche.TotalTakerDenom.Sub(takerAmountOut) + + if makerAmountToReturn.IsPositive() || takerAmountOut.IsPositive() { + makerCoinOut := sdk.NewCoin(tradePairID.MakerDenom, makerAmountToReturn) + takerCoinOut := sdk.NewCoin(tradePairID.TakerDenom, takerAmountOut) + coinsOut := sdk.NewCoins(makerCoinOut, takerCoinOut) err := k.bankKeeper.SendCoinsFromModuleToAccount( ctx, types.ModuleName, callerAddr, - sdk.Coins{coinOut}, + coinsOut, ) if err != nil { return err @@ -528,6 +535,17 @@ func (k Keeper) CancelLimitOrderCore( k.SaveTrancheUser(ctx, trancheUser) k.SaveTranche(ctx, tranche) + pairID := tradePairID.MustPairID() + ctx.EventManager().EmitEvent(types.CancelLimitOrderEvent( + callerAddr, + pairID.Token0, + pairID.Token1, + tradePairID.MakerDenom, + tradePairID.TakerDenom, + coinsOut, + trancheKey, + )) + if trancheUser.OrderType.HasExpiration() { k.RemoveLimitOrderExpiration(ctx, *tranche.ExpirationTime, tranche.Key.KeyMarshal()) } @@ -535,17 +553,6 @@ func (k Keeper) CancelLimitOrderCore( return sdkerrors.Wrapf(types.ErrCancelEmptyLimitOrder, "%s", tranche.Key.TrancheKey) } - pairID := tradePairID.MustPairID() - ctx.EventManager().EmitEvent(types.CancelLimitOrderEvent( - callerAddr, - pairID.Token0, - pairID.Token1, - tradePairID.MakerDenom, - tradePairID.TakerDenom, - amountToCancel, - trancheKey, - )) - return nil } @@ -590,15 +597,13 @@ func (k Keeper) WithdrawFilledLimitOrderCore( // This is only relevant for inactive JIT and GoodTil limit orders remainingTokenIn = tranche.RemoveTokenIn(trancheUser) k.SaveInactiveTranche(ctx, tranche) - - // Treat the removed tokenIn as cancelled shares - trancheUser.SharesCancelled = trancheUser.SharesCancelled.Add(remainingTokenIn) - + // Since the order has already been filled we treat this as a complete withdrawal + trancheUser.SharesWithdrawn = trancheUser.SharesOwned } else { k.SetLimitOrderTranche(ctx, tranche) + trancheUser.SharesWithdrawn = trancheUser.SharesWithdrawn.Add(amountOutTokenIn) } - trancheUser.SharesWithdrawn = trancheUser.SharesWithdrawn.Add(amountOutTokenIn) } k.SaveTrancheUser(ctx, trancheUser) diff --git a/x/dex/keeper/integration_cancellimitorder_test.go b/x/dex/keeper/integration_cancellimitorder_test.go index 0b95f17f4..9c12b8d0f 100644 --- a/x/dex/keeper/integration_cancellimitorder_test.go +++ b/x/dex/keeper/integration_cancellimitorder_test.go @@ -191,9 +191,39 @@ func (s *DexTestSuite) TestCancelPartiallyFilled() { // WHEN alice cancels her limit order s.aliceCancelsLimitSell(trancheKey) - // Then alice gets back remaining 25 TokenA LO reserves - s.assertAliceBalances(25, 0) - s.assertDexBalances(0, 25) + // Then alice gets back remaining 25 TokenA LO reserves & 25 TokenB taker tokens + s.assertAliceBalances(25, 25) + s.assertDexBalances(0, 0) + + // Assert that the LimitOrderTrancheUser has been deleted + _, found := s.App.DexKeeper.GetLimitOrderTrancheUser(s.Ctx, s.alice.String(), trancheKey) + s.Assert().False(found) +} + +func (s *DexTestSuite) TestCancelPartiallyFilledWithdrawFails() { + s.fundAliceBalances(50, 0) + s.fundBobBalances(0, 10) + + // GIVEN alice limit sells 50 TokenA + trancheKey := s.aliceLimitSells("TokenA", 2000, 50) + // Bob swaps 10 TokenB for TokenA + s.bobLimitSells("TokenB", -2001, 10, types.LimitOrderType_FILL_OR_KILL) + + s.assertDexBalancesInt(sdkmath.NewInt(37786095), sdkmath.NewInt(10000000)) + s.assertAliceBalances(0, 0) + + // WHEN alice cancels her limit order + 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()) + + // Assert that the LimitOrderTrancheUser has been deleted + _, found := s.App.DexKeeper.GetLimitOrderTrancheUser(s.Ctx, s.alice.String(), trancheKey) + s.Assert().False(found) + + s.aliceWithdrawLimitSellFails(types.ErrValidLimitOrderTrancheNotFound, trancheKey) } func (s *DexTestSuite) TestCancelPartiallyFilledMultiUser() { @@ -215,12 +245,82 @@ func (s *DexTestSuite) TestCancelPartiallyFilledMultiUser() { s.aliceCancelsLimitSell(trancheKey) s.carolCancelsLimitSell(trancheKey) - // THEN alice gets back ~41 BIGTokenA (125 * 1/3) - s.assertAliceBalancesInt(sdkmath.NewInt(41_666_666), sdkmath.ZeroInt()) + // THEN alice gets back ~41 BIGTokenA (125 * 1/3) & ~8.3 BIGTokenB Taker tokens (25 * 1/3) + s.assertAliceBalancesInt(sdkmath.NewInt(41_666_666), sdkmath.NewInt(8333333)) + + // Carol gets back 83 TokenA (125 * 2/3) & ~16.6 BIGTokenB Taker tokens (25 * 2/3) + s.assertCarolBalancesInt(sdkmath.NewInt(83_333_333), sdkmath.NewInt(16666667)) + s.assertDexBalancesInt(sdkmath.OneInt(), sdkmath.ZeroInt()) + + // Assert that the LimitOrderTrancheUsers has been deleted + _, found := s.App.DexKeeper.GetLimitOrderTrancheUser(s.Ctx, s.alice.String(), trancheKey) + s.Assert().False(found) + _, found = s.App.DexKeeper.GetLimitOrderTrancheUser(s.Ctx, s.carol.String(), trancheKey) + s.Assert().False(found) +} + +func (s *DexTestSuite) TestCancelPartiallyFilledMultiUser2() { + s.fundAliceBalances(50, 0) + s.fundBobBalances(50, 0) + s.fundCarolBalances(0, 40) + + // // GIVEN alice and bob each limit sells 50 TokenA + trancheKey := s.aliceLimitSells("TokenA", 2000, 50) + s.bobLimitSells("TokenA", 2000, 50) + // carol swaps 20 TokenB for TokenA + s.carolLimitSells("TokenB", -2001, 20, types.LimitOrderType_FILL_OR_KILL) + + // WHEN alice cancels her limit order + 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)) + + // THEN carol swap through more of the limitorder + s.carolLimitSells("TokenB", -2001, 20, types.LimitOrderType_FILL_OR_KILL) + + // And bob can withdraw his portion + s.bobWithdrawsLimitSell(trancheKey) + s.assertBobBalancesInt(sdkmath.ZeroInt(), sdkmath.NewInt(30000000)) +} + +func (s *DexTestSuite) TestCancelFirstMultiCancel() { + s.fundAliceBalances(50, 0) + s.fundBobBalances(50, 0) + s.fundCarolBalances(0, 40) + + // // GIVEN alice and bob each limit sells 50 TokenA + trancheKey := s.aliceLimitSells("TokenA", 0, 50) + s.bobLimitSells("TokenA", 0, 50) + s.bobCancelsLimitSell(trancheKey) + // carol swaps 10 TokenB for TokenA + s.carolLimitSells("TokenB", -1, 10, types.LimitOrderType_FILL_OR_KILL) + + // WHEN alice cancels her limit order + s.aliceCancelsLimitSell(trancheKey) + + // THEN alice gets back remaining 40 tokenA 10 TokenB taker tokens + s.assertAliceBalances(40, 10) +} + +func (s *DexTestSuite) TestCancelFirstMultiWithdraw() { + s.fundAliceBalances(50, 0) + s.fundBobBalances(50, 0) + s.fundCarolBalances(0, 40) + + // // GIVEN alice and bob each limit sells 50 TokenA + trancheKey := s.aliceLimitSells("TokenA", 0, 50) + s.bobLimitSells("TokenA", 0, 50) + s.bobCancelsLimitSell(trancheKey) + // carol swaps 10 TokenB for TokenA + s.carolLimitSells("TokenB", -1, 10, types.LimitOrderType_FILL_OR_KILL) + + // WHEN alice withdraws her limit order + s.aliceWithdrawsLimitSell(trancheKey) - // Carol gets back 83 TokenA (125 * 2/3) - s.assertCarolBalancesInt(sdkmath.NewInt(83_333_333), sdkmath.ZeroInt()) - s.assertDexBalancesInt(sdkmath.OneInt(), sdkmath.NewInt(25_000_000)) + // THEN alice gets 10 TokenB + s.assertAliceBalances(0, 10) } func (s *DexTestSuite) TestCancelGoodTil() { diff --git a/x/dex/keeper/integration_placelimitorder_test.go b/x/dex/keeper/integration_placelimitorder_test.go index fcd074bdc..37c822973 100644 --- a/x/dex/keeper/integration_placelimitorder_test.go +++ b/x/dex/keeper/integration_placelimitorder_test.go @@ -271,31 +271,19 @@ func (s *DexTestSuite) TestLimitOrderPartialFillDepositCancel() { s.aliceCancelsLimitSell(trancheKey0) - s.assertAliceBalances(100, 40) + s.assertAliceBalances(110, 40) s.assertBobBalances(90, 110) - s.assertDexBalances(10, 50) + s.assertDexBalances(0, 50) s.assertCurrentTicks(math.MinInt64, 0) s.bobLimitSells("TokenA", 10, 10, types.LimitOrderType_FILL_OR_KILL) - s.assertAliceBalances(100, 40) + s.assertAliceBalances(110, 40) s.assertBobBalances(80, 120) - s.assertDexBalances(20, 40) + s.assertDexBalances(10, 40) s.aliceCancelsLimitSell(trancheKey1) - s.assertAliceBalances(100, 80) - s.assertBobBalances(80, 120) - s.assertDexBalances(20, 0) - - s.aliceWithdrawsLimitSell(trancheKey0) - - s.assertAliceBalances(110, 80) - s.assertBobBalances(80, 120) - s.assertDexBalances(10, 0) - - s.aliceWithdrawsLimitSell(trancheKey1) - s.assertAliceBalances(120, 80) s.assertBobBalances(80, 120) s.assertDexBalances(0, 0) diff --git a/x/dex/keeper/integration_withdrawfilled_test.go b/x/dex/keeper/integration_withdrawfilled_test.go index 9ddfbf3bc..8d8ba338b 100644 --- a/x/dex/keeper/integration_withdrawfilled_test.go +++ b/x/dex/keeper/integration_withdrawfilled_test.go @@ -339,9 +339,6 @@ func (s *DexTestSuite) TestWithdrawPartiallyFilledCancelled() { // AND alice cancels the remainder s.aliceCancelsLimitSell(trancheKey) - - // THEN she can withdraw the unused portion - s.aliceWithdrawsLimitSell(trancheKey) s.assertAliceBalances(1, 1) // AND her LimitOrderTrancheUser is removed diff --git a/x/dex/keeper/limit_order_tranche_user.go b/x/dex/keeper/limit_order_tranche_user.go index 13ca0fc1c..9df5c072e 100644 --- a/x/dex/keeper/limit_order_tranche_user.go +++ b/x/dex/keeper/limit_order_tranche_user.go @@ -25,7 +25,6 @@ func (k Keeper) GetOrInitLimitOrderTrancheUser( Address: receiver, SharesOwned: math.ZeroInt(), SharesWithdrawn: math.ZeroInt(), - SharesCancelled: math.ZeroInt(), TickIndexTakerToMaker: tickIndex, TradePairId: tradePairID, OrderType: orderType, diff --git a/x/dex/keeper/limit_order_tranche_user_test.go b/x/dex/keeper/limit_order_tranche_user_test.go index 9a4014f36..a5578ee43 100644 --- a/x/dex/keeper/limit_order_tranche_user_test.go +++ b/x/dex/keeper/limit_order_tranche_user_test.go @@ -24,7 +24,6 @@ func createNLimitOrderTrancheUser(keeper *keeper.Keeper, ctx sdk.Context, n int) TickIndexTakerToMaker: int64(i), SharesOwned: math.NewInt(100), SharesWithdrawn: math.ZeroInt(), - SharesCancelled: math.ZeroInt(), } items[i] = val keeper.SetLimitOrderTrancheUser(ctx, items[i]) @@ -43,7 +42,6 @@ func createNLimitOrderTrancheUserWithAddress(keeper *keeper.Keeper, ctx sdk.Cont TickIndexTakerToMaker: 0, SharesOwned: math.ZeroInt(), SharesWithdrawn: math.ZeroInt(), - SharesCancelled: math.ZeroInt(), } items[i] = val keeper.SetLimitOrderTrancheUser(ctx, items[i]) @@ -93,7 +91,7 @@ func (s *DexTestSuite) TestGetAllLimitOrders() { Address: s.alice.String(), SharesOwned: math.NewInt(10_000_000), SharesWithdrawn: math.NewInt(0), - SharesCancelled: math.NewInt(0), + SharesCancelled: math.ZeroInt(), }, LOList[0], ) @@ -104,7 +102,7 @@ func (s *DexTestSuite) TestGetAllLimitOrders() { Address: s.alice.String(), SharesOwned: math.NewInt(10_000_000), SharesWithdrawn: math.NewInt(0), - SharesCancelled: math.NewInt(0), + SharesCancelled: math.ZeroInt(), }, LOList[1], ) diff --git a/x/dex/types/events.go b/x/dex/types/events.go index bf3670855..4dd6ca2f4 100644 --- a/x/dex/types/events.go +++ b/x/dex/types/events.go @@ -157,7 +157,7 @@ func CancelLimitOrderEvent( token1 string, makerDenom string, tokenOut string, - amountOut math.Int, + coinsOut sdk.Coins, trancheKey string, ) sdk.Event { attrs := []sdk.Attribute{ @@ -168,7 +168,7 @@ func CancelLimitOrderEvent( sdk.NewAttribute(CancelLimitOrderEventToken1, token1), sdk.NewAttribute(CancelLimitOrderEventTokenIn, makerDenom), sdk.NewAttribute(CancelLimitOrderEventTokenOut, tokenOut), - sdk.NewAttribute(CancelLimitOrderEventAmountOut, amountOut.String()), + sdk.NewAttribute(CancelLimitOrderEventAmountOut, coinsOut.String()), sdk.NewAttribute(CancelLimitOrderEventTrancheKey, trancheKey), } diff --git a/x/dex/types/limit_order_tranche.go b/x/dex/types/limit_order_tranche.go index 8f6a6cb20..faea72e3d 100644 --- a/x/dex/types/limit_order_tranche.go +++ b/x/dex/types/limit_order_tranche.go @@ -135,10 +135,9 @@ func (t *LimitOrderTranche) RemoveTokenIn( trancheUser *LimitOrderTrancheUser, ) (amountToRemove math.Int) { amountUnfilled := t.AmountUnfilled() - maxAmountToRemove := amountUnfilled.MulInt(trancheUser.SharesOwned). + amountToRemove = amountUnfilled.MulInt(trancheUser.SharesOwned). QuoInt(t.TotalMakerDenom). TruncateInt() - amountToRemove = maxAmountToRemove.Sub(trancheUser.SharesCancelled) t.ReservesMakerDenom = t.ReservesMakerDenom.Sub(amountToRemove) return amountToRemove diff --git a/x/dex/types/limit_order_tranche_user.go b/x/dex/types/limit_order_tranche_user.go index 90dcf34ac..51f923210 100644 --- a/x/dex/types/limit_order_tranche_user.go +++ b/x/dex/types/limit_order_tranche_user.go @@ -1,6 +1,5 @@ package types func (l LimitOrderTrancheUser) IsEmpty() bool { - sharesRemoved := l.SharesCancelled.Add(l.SharesWithdrawn) - return sharesRemoved.Equal(l.SharesOwned) + return l.SharesWithdrawn.Equal(l.SharesOwned) }