Merge pull request #644 from neutron-org/chore/clean_locl

Chore: clean locl

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",

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{

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,24 +535,24 @@ 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())
 		}
 	} else {
 		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)

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() {

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)

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

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,

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],
 	)

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),
 	}
 

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

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)
 }