From bb920dadbfa9a94bd7b4bf95ef86becc590b43ed Mon Sep 17 00:00:00 2001 From: Julian Compagni Portis Date: Thu, 5 Sep 2024 18:06:35 -0400 Subject: [PATCH 1/4] Allow cancel inactive limit orders --- x/dex/keeper/cancel_limit_order.go | 14 ++-- .../integration_cancellimitorder_test.go | 72 +++++++++++++++++-- x/dex/types/errors.go | 5 -- 3 files changed, 74 insertions(+), 17 deletions(-) diff --git a/x/dex/keeper/cancel_limit_order.go b/x/dex/keeper/cancel_limit_order.go index 8c3af443e..a45ac9177 100644 --- a/x/dex/keeper/cancel_limit_order.go +++ b/x/dex/keeper/cancel_limit_order.go @@ -59,11 +59,11 @@ func (k Keeper) ExecuteCancelLimitOrder( ) (makerCoinOut, takerCoinOut sdk.Coin, tradePairID *types.TradePairID, error error) { trancheUser, found := k.GetLimitOrderTrancheUser(ctx, callerAddr.String(), trancheKey) if !found { - return sdk.Coin{}, sdk.Coin{}, nil, types.ErrActiveLimitOrderNotFound + return sdk.Coin{}, sdk.Coin{}, nil, types.ErrValidLimitOrderTrancheNotFound } tradePairID, tickIndex := trancheUser.TradePairId, trancheUser.TickIndexTakerToMaker - tranche := k.GetLimitOrderTranche( + tranche, wasFilled, found := k.FindLimitOrderTranche( ctx, &types.LimitOrderTrancheKey{ TradePairId: tradePairID, @@ -71,8 +71,8 @@ func (k Keeper) ExecuteCancelLimitOrder( TrancheKey: trancheKey, }, ) - if tranche == nil { - return sdk.Coin{}, sdk.Coin{}, nil, types.ErrActiveLimitOrderNotFound + if !found { + return sdk.Coin{}, sdk.Coin{}, nil, types.ErrValidLimitOrderTrancheNotFound } makerAmountToReturn := tranche.RemoveTokenIn(trancheUser) @@ -89,7 +89,11 @@ func (k Keeper) ExecuteCancelLimitOrder( } k.SaveTrancheUser(ctx, trancheUser) - k.SaveTranche(ctx, tranche) + if wasFilled { + k.SaveInactiveTranche(ctx, tranche) + } else { + k.SaveTranche(ctx, tranche) + } if trancheUser.OrderType.HasExpiration() { k.RemoveLimitOrderExpiration(ctx, *tranche.ExpirationTime, tranche.Key.KeyMarshal()) diff --git a/x/dex/keeper/integration_cancellimitorder_test.go b/x/dex/keeper/integration_cancellimitorder_test.go index 9c12b8d0f..c21e96274 100644 --- a/x/dex/keeper/integration_cancellimitorder_test.go +++ b/x/dex/keeper/integration_cancellimitorder_test.go @@ -173,7 +173,7 @@ func (s *DexTestSuite) TestCancelTwiceFails() { s.assertAliceBalances(50, 50) s.assertDexBalances(0, 0) - s.aliceCancelsLimitSellFails(trancheKey, types.ErrActiveLimitOrderNotFound) + s.aliceCancelsLimitSellFails(trancheKey, types.ErrValidLimitOrderTrancheNotFound) } func (s *DexTestSuite) TestCancelPartiallyFilled() { @@ -323,6 +323,43 @@ func (s *DexTestSuite) TestCancelFirstMultiWithdraw() { s.assertAliceBalances(0, 10) } +func (s *DexTestSuite) TestCancelMultiAfterFilled() { + s.fundAliceBalances(50, 0) + s.fundBobBalances(50, 0) + s.fundCarolBalances(0, 100) + + // GIVEN alice and bob each limit sells 50 TokenA + trancheKey := s.aliceLimitSells("TokenA", 0, 50) + s.bobLimitSells("TokenA", 0, 50) + + // carol swaps through the tranche + s.carolLimitSells("TokenB", -1, 100, types.LimitOrderType_IMMEDIATE_OR_CANCEL) + + // WHEN alice and bob cancel their limit order + s.aliceCancelsLimitSell(trancheKey) + s.assertAliceBalances(0, 50) + s.bobCancelsLimitSell(trancheKey) + s.assertBobBalances(0, 50) + + // THEN they get back the expected profit + s.assertAliceBalances(0, 50) + s.assertBobBalances(0, 50) + + // AND tranche and trancheUsers are deleted + + s.App.DexKeeper.GetLimitOrderTrancheUser(s.Ctx, s.alice.String(), trancheKey) + _, _, found := s.App.DexKeeper.FindLimitOrderTranche(s.Ctx, &types.LimitOrderTrancheKey{ + TradePairId: types.MustNewTradePairID("TokenB", "TokenA"), + TickIndexTakerToMaker: 0, + TrancheKey: trancheKey, + }) + s.False(found) + _, 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) TestCancelGoodTil() { s.fundAliceBalances(50, 0) tomorrow := time.Now().AddDate(0, 0, 1) @@ -339,7 +376,7 @@ func (s *DexTestSuite) TestCancelGoodTil() { s.assertNLimitOrderExpiration(0) } -func (s *DexTestSuite) TestCancelGoodTilAfterExpirationFails() { +func (s *DexTestSuite) TestCancelGoodTilAfterExpiration() { s.fundAliceBalances(50, 0) tomorrow := time.Now().AddDate(0, 0, 1) // GIVEN alice limit sells 50 TokenA with goodTil date of tommrow @@ -350,8 +387,19 @@ func (s *DexTestSuite) TestCancelGoodTilAfterExpirationFails() { // WHEN expiration date has passed s.beginBlockWithTime(time.Now().AddDate(0, 0, 2)) - // THEN alice cancellation fails - s.aliceCancelsLimitSellFails(trancheKey, types.ErrActiveLimitOrderNotFound) + // THEN alice cancellation succeeds + s.aliceCancelsLimitSell(trancheKey) + + s.assertAliceBalances(50, 0) + + // TrancheUser and Tranche are removed + s.App.DexKeeper.GetLimitOrderTrancheUser(s.Ctx, s.alice.String(), trancheKey) + _, _, found := s.App.DexKeeper.FindLimitOrderTranche(s.Ctx, &types.LimitOrderTrancheKey{ + TradePairId: types.MustNewTradePairID("TokenB", "TokenA"), + TickIndexTakerToMaker: 0, + TrancheKey: trancheKey, + }) + s.False(found) } func (s *DexTestSuite) TestCancelJITSameBlock() { @@ -380,7 +428,17 @@ func (s *DexTestSuite) TestCancelJITNextBlock() { s.nextBlockWithTime(time.Now()) s.beginBlockWithTime(time.Now()) - // THEN alice cancellation fails - s.aliceCancelsLimitSellFails(trancheKey, types.ErrActiveLimitOrderNotFound) - s.assertAliceBalances(0, 0) + // THEN alice cancellation succeeds + s.aliceCancelsLimitSell(trancheKey) + + s.assertAliceBalances(50, 0) + + // TrancheUser and Tranche are removed + s.App.DexKeeper.GetLimitOrderTrancheUser(s.Ctx, s.alice.String(), trancheKey) + _, _, found := s.App.DexKeeper.FindLimitOrderTranche(s.Ctx, &types.LimitOrderTrancheKey{ + TradePairId: types.MustNewTradePairID("TokenB", "TokenA"), + TickIndexTakerToMaker: 0, + TrancheKey: trancheKey, + }) + s.False(found) } diff --git a/x/dex/types/errors.go b/x/dex/types/errors.go index 69211ca8f..06b5bec8c 100644 --- a/x/dex/types/errors.go +++ b/x/dex/types/errors.go @@ -69,11 +69,6 @@ var ( 1125, "MaxAmountIn in must be > 0 for swap.", ) - ErrActiveLimitOrderNotFound = sdkerrors.Register( - ModuleName, - 1128, - "No active limit found. It does not exist or has already been filled", - ) ErrZeroWithdraw = sdkerrors.Register( ModuleName, 1129, From c00327b276c19564853f819244a465cc24b85332 Mon Sep 17 00:00:00 2001 From: Julian Compagni Portis Date: Thu, 19 Sep 2024 15:17:15 -0400 Subject: [PATCH 2/4] switch to wrapped error --- x/dex/keeper/cancel_limit_order.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/dex/keeper/cancel_limit_order.go b/x/dex/keeper/cancel_limit_order.go index a45ac9177..8538313bb 100644 --- a/x/dex/keeper/cancel_limit_order.go +++ b/x/dex/keeper/cancel_limit_order.go @@ -59,7 +59,7 @@ func (k Keeper) ExecuteCancelLimitOrder( ) (makerCoinOut, takerCoinOut sdk.Coin, tradePairID *types.TradePairID, error error) { trancheUser, found := k.GetLimitOrderTrancheUser(ctx, callerAddr.String(), trancheKey) if !found { - return sdk.Coin{}, sdk.Coin{}, nil, types.ErrValidLimitOrderTrancheNotFound + return sdk.Coin{}, sdk.Coin{}, nil, sdkerrors.Wrapf(types.ErrValidLimitOrderTrancheNotFound, "%s", trancheKey) } tradePairID, tickIndex := trancheUser.TradePairId, trancheUser.TickIndexTakerToMaker @@ -72,7 +72,7 @@ func (k Keeper) ExecuteCancelLimitOrder( }, ) if !found { - return sdk.Coin{}, sdk.Coin{}, nil, types.ErrValidLimitOrderTrancheNotFound + return sdk.Coin{}, sdk.Coin{}, nil, sdkerrors.Wrapf(types.ErrValidLimitOrderTrancheNotFound, "%s", trancheKey) } makerAmountToReturn := tranche.RemoveTokenIn(trancheUser) From 94f0b32fb345076d0ec44891a8d569914c7ce980 Mon Sep 17 00:00:00 2001 From: Julian Compagni Portis Date: Thu, 19 Sep 2024 15:24:43 -0400 Subject: [PATCH 3/4] fix typo --- x/dex/types/errors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/dex/types/errors.go b/x/dex/types/errors.go index 06b5bec8c..04b845b99 100644 --- a/x/dex/types/errors.go +++ b/x/dex/types/errors.go @@ -27,7 +27,7 @@ var ( ErrValidLimitOrderTrancheNotFound = sdkerrors.Register( ModuleName, 1111, - "Limit order trache not found:", + "Limit order tranche not found:", ) // "%d", trancheKey ErrCancelEmptyLimitOrder = sdkerrors.Register( ModuleName, From 18c5e3edbfe3c4f7ac1a2aa345f533e04645e6b6 Mon Sep 17 00:00:00 2001 From: Julian Compagni Portis Date: Tue, 24 Sep 2024 11:45:15 -0400 Subject: [PATCH 4/4] fix test --- x/dex/keeper/grpc_query_simulate_cancel_limit_order_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/dex/keeper/grpc_query_simulate_cancel_limit_order_test.go b/x/dex/keeper/grpc_query_simulate_cancel_limit_order_test.go index c7144b9c1..0f45403b3 100644 --- a/x/dex/keeper/grpc_query_simulate_cancel_limit_order_test.go +++ b/x/dex/keeper/grpc_query_simulate_cancel_limit_order_test.go @@ -47,6 +47,6 @@ func (s *DexTestSuite) TestSimulateCancelLimitOrderFails() { } resp, err := s.App.DexKeeper.SimulateCancelLimitOrder(s.Ctx, req) - s.ErrorIs(err, types.ErrActiveLimitOrderNotFound) + s.ErrorIs(err, types.ErrValidLimitOrderTrancheNotFound) s.Nil(resp) }