From 09031961a8a29c0778f35159302d722aa8a7b33a Mon Sep 17 00:00:00 2001 From: Julian Compagni Portis Date: Thu, 22 Aug 2024 17:57:39 -0400 Subject: [PATCH] Fix limit order cancel accounting --- x/dex/keeper/cancel_limit_order.go | 20 ++++-- .../integration_cancellimitorder_test.go | 64 +++++++++++++++++++ .../keeper/integration_withdrawfilled_test.go | 25 +++++++- 3 files changed, 102 insertions(+), 7 deletions(-) diff --git a/x/dex/keeper/cancel_limit_order.go b/x/dex/keeper/cancel_limit_order.go index 8c3af443e..5680c0699 100644 --- a/x/dex/keeper/cancel_limit_order.go +++ b/x/dex/keeper/cancel_limit_order.go @@ -6,6 +6,7 @@ import ( sdkerrors "cosmossdk.io/errors" 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" ) @@ -78,11 +79,22 @@ func (k Keeper) ExecuteCancelLimitOrder( makerAmountToReturn := tranche.RemoveTokenIn(trancheUser) _, takerAmountOut := tranche.Withdraw(trancheUser) - trancheUser.SharesWithdrawn = trancheUser.SharesOwned - - // Remove the canceled shares from the limitOrder + // Remove the canceled shares from the maker side of the limitOrder tranche.TotalMakerDenom = tranche.TotalMakerDenom.Sub(trancheUser.SharesOwned) - tranche.TotalTakerDenom = tranche.TotalTakerDenom.Sub(takerAmountOut) + + // Calculate total number of shares removed previously withdrawn by the user (denominated in takerDenom) + sharesWithdrawnTakerDenom := math_utils.NewPrecDecFromInt(trancheUser.SharesWithdrawn). + Quo(tranche.PriceTakerToMaker). + TruncateInt() + + // Calculate the total amount removed including prior withdrawals (denominated in takerDenom) + totalAmountOutTakerDenom := sharesWithdrawnTakerDenom.Add(takerAmountOut) + + // Decrease the tranche TotalTakerDenom by the amount being removed + tranche.TotalTakerDenom = tranche.TotalTakerDenom.Sub(totalAmountOutTakerDenom) + + // Set TrancheUser to 100% shares withdrawn + trancheUser.SharesWithdrawn = trancheUser.SharesOwned if !makerAmountToReturn.IsPositive() && !takerAmountOut.IsPositive() { return sdk.Coin{}, sdk.Coin{}, nil, sdkerrors.Wrapf(types.ErrCancelEmptyLimitOrder, "%s", tranche.Key.TrancheKey) diff --git a/x/dex/keeper/integration_cancellimitorder_test.go b/x/dex/keeper/integration_cancellimitorder_test.go index 9c12b8d0f..f72a444d6 100644 --- a/x/dex/keeper/integration_cancellimitorder_test.go +++ b/x/dex/keeper/integration_cancellimitorder_test.go @@ -384,3 +384,67 @@ func (s *DexTestSuite) TestCancelJITNextBlock() { s.aliceCancelsLimitSellFails(trancheKey, types.ErrActiveLimitOrderNotFound) s.assertAliceBalances(0, 0) } + +func (s *DexTestSuite) TestWithdrawThenCancel() { + 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.carolLimitSells("TokenB", -1, 10, types.LimitOrderType_FILL_OR_KILL) + + // WHEN alice withdraws and cancels her limit order + s.aliceWithdrawsLimitSell(trancheKey) + s.aliceCancelsLimitSell(trancheKey) + s.assertAliceBalances(45, 5) + + s.bobWithdrawsLimitSell(trancheKey) + s.assertBobBalances(0, 5) + s.bobCancelsLimitSell(trancheKey) + s.assertBobBalances(45, 5) +} + +func (s *DexTestSuite) TestWithdrawThenCancel2() { + 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.carolLimitSells("TokenB", -1, 10, types.LimitOrderType_FILL_OR_KILL) + + // WHEN alice withdraws and cancels her limit order + s.aliceWithdrawsLimitSell(trancheKey) + s.aliceCancelsLimitSell(trancheKey) + s.assertAliceBalances(45, 5) + + s.bobCancelsLimitSell(trancheKey) + s.assertBobBalances(45, 5) +} + +func (s *DexTestSuite) TestWithdrawThenCancelLowTick() { + 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", 20000, 50) + s.bobLimitSells("TokenA", 20000, 50) + + s.carolLimitSells("TokenB", -20001, 10, types.LimitOrderType_FILL_OR_KILL) + + // WHEN alice withdraws and cancels her limit order + s.aliceWithdrawsLimitSell(trancheKey) + s.aliceCancelsLimitSell(trancheKey) + s.assertAliceBalancesInt(sdkmath.NewInt(13058413), sdkmath.NewInt(4999999)) + + s.bobWithdrawsLimitSell(trancheKey) + s.assertBobBalancesInt(sdkmath.ZeroInt(), sdkmath.NewInt(4999999)) + s.bobCancelsLimitSell(trancheKey) + s.assertBobBalancesInt(sdkmath.NewInt(13058413), sdkmath.NewInt(4999999)) +} diff --git a/x/dex/keeper/integration_withdrawfilled_test.go b/x/dex/keeper/integration_withdrawfilled_test.go index 8d8ba338b..97df676c0 100644 --- a/x/dex/keeper/integration_withdrawfilled_test.go +++ b/x/dex/keeper/integration_withdrawfilled_test.go @@ -386,8 +386,27 @@ func (s *DexTestSuite) TestWithdrawPartiallyGTTFilledCancelled() { s.False(found, "Alice's LimitOrderTrancheUser not removed") } -// testcancel unfilled +func (s *DexTestSuite) TestWithdrawInactive() { + s.fundAliceBalances(10, 0) + s.fundBobBalances(0, 20) -// test withdraw expired + // GIVEN Alice places an expiring limit order of A + trancheKey := s.aliceLimitSellsGoodTil("TokenA", 0, 10, time.Now()) + + // Bob trades through half of it + s.bobLimitSells("TokenB", -1, 5) + + // Alice withdraws the profits + s.aliceWithdrawsLimitSell(trancheKey) + s.assertAliceBalances(0, 5) + + // bob swap through more + s.bobLimitSells("TokenB", -1, 4) -// how does cancel withdraw work does it call into was filled + // WHEN it is purged + s.App.DexKeeper.PurgeExpiredLimitOrders(s.Ctx, time.Now()) + + // THEN alice can withdraw the expected amount + s.aliceWithdrawsLimitSell(trancheKey) + s.assertAliceBalances(1, 9) +}