fix: set public position indexes in InitGenesis and fix minor issues

- call SendCoins only when amt is all positive
crescent-network · Sep 13, 2023 · 77082a9 · 77082a9
1 parent 23c6e31
commit 77082a9
Show file tree

Hide file tree

Showing 6 changed files with 69 additions and 17 deletions.
diff --git a/x/amm/keeper/farming.go b/x/amm/keeper/farming.go
@@ -23,8 +23,10 @@ func (k Keeper) CreatePrivateFarmingPlan(
 			"maximum number of active private farming plans reached")
 	}
 	fee := k.GetPrivateFarmingPlanCreationFee(ctx)
-	if err = k.bankKeeper.SendCoinsFromAccountToModule(ctx, creatorAddr, types.ModuleName, fee); err != nil {
-		return
+	if fee.IsAllPositive() {
+		if err = k.bankKeeper.SendCoinsFromAccountToModule(ctx, creatorAddr, types.ModuleName, fee); err != nil {
+			return
+		}
 	}
 	plan, err = k.createFarmingPlan(ctx, description, nil, termAddr, rewardAllocs, startTime, endTime, true)
 	if err != nil {

diff --git a/x/amm/keeper/pool.go b/x/amm/keeper/pool.go
@@ -21,9 +21,11 @@ func (k Keeper) CreatePool(ctx sdk.Context, creatorAddr sdk.AccAddress, marketId
 	}
 
 	creationFee := k.GetPoolCreationFee(ctx)
-	if err = k.bankKeeper.SendCoinsFromAccountToModule(ctx, creatorAddr, types.ModuleName, creationFee); err != nil {
-		err = sdkerrors.Wrap(err, "insufficient pool creation fee")
-		return
+	if creationFee.IsAllPositive() {
+		if err = k.bankKeeper.SendCoinsFromAccountToModule(ctx, creatorAddr, types.ModuleName, creationFee); err != nil {
+			err = sdkerrors.Wrap(err, "insufficient pool creation fee")
+			return
+		}
 	}
 
 	// Create a new pool

diff --git a/x/amm/keeper/position.go b/x/amm/keeper/position.go
@@ -59,9 +59,11 @@ func (k Keeper) AddLiquidity(
 		ctx, pool, ownerAddr, lowerTick, upperTick, liquidity)
 
 	amt = sdk.NewCoins(sdk.NewCoin(pool.Denom0, amt0), sdk.NewCoin(pool.Denom1, amt1))
-	if err = k.bankKeeper.SendCoins(
-		ctx, fromAddr, pool.MustGetReserveAddress(), amt); err != nil {
-		return
+	if amt.IsAllPositive() {
+		if err = k.bankKeeper.SendCoins(
+			ctx, fromAddr, pool.MustGetReserveAddress(), amt); err != nil {
+			return
+		}
 	}
 
 	if err = ctx.EventManager().EmitTypedEvent(&types.EventAddLiquidity{
@@ -124,8 +126,10 @@ func (k Keeper) RemoveLiquidity(
 		if err != nil {
 			return
 		}
-		if err = k.Collect(ctx, ownerAddr, toAddr, position.Id, fee.Add(farmingRewards...)); err != nil {
-			return
+		if fee.Add(farmingRewards...).IsAllPositive() {
+			if err = k.Collect(ctx, ownerAddr, toAddr, position.Id, fee.Add(farmingRewards...)); err != nil {
+				return
+			}
 		}
 	}
 	if err = ctx.EventManager().EmitTypedEvent(&types.EventRemoveLiquidity{
@@ -164,13 +168,17 @@ func (k Keeper) Collect(
 	}
 	fee := amt.Min(position.OwedFee)
 	position.OwedFee = position.OwedFee.Sub(fee)
-	if err := k.bankKeeper.SendCoins(ctx, pool.MustGetRewardsPoolAddress(), toAddr, fee); err != nil {
-		return err
+	if fee.IsAllPositive() {
+		if err := k.bankKeeper.SendCoins(ctx, pool.MustGetRewardsPoolAddress(), toAddr, fee); err != nil {
+			return err
+		}
 	}
 	farmingRewards := amt.Sub(fee)
 	position.OwedFarmingRewards = position.OwedFarmingRewards.Sub(farmingRewards)
-	if err := k.bankKeeper.SendCoins(ctx, types.RewardsPoolAddress, toAddr, farmingRewards); err != nil {
-		return err
+	if farmingRewards.IsAllPositive() {
+		if err := k.bankKeeper.SendCoins(ctx, types.RewardsPoolAddress, toAddr, farmingRewards); err != nil {
+			return err
+		}
 	}
 	k.SetPosition(ctx, position)
 

diff --git a/x/amm/keeper/position_test.go b/x/amm/keeper/position_test.go
@@ -162,3 +162,39 @@ func (s *KeeperTestSuite) TestPositionAssets_ZeroLiquidity() {
 	s.AssertEqual(utils.ParseCoin("0ucre"), coin0)
 	s.AssertEqual(utils.ParseCoin("0uusd"), coin1)
 }
+
+func (s *KeeperTestSuite) TestRemoveSmallLiquidity() {
+	_, pool := s.CreateMarketAndPool("ucre", "uusd", utils.ParseDec("5"))
+
+	lpAddr1 := s.FundedAccount(1, enoughCoins)
+
+	position, _, amt := s.AddLiquidity(
+		lpAddr1, pool.Id, utils.ParseDec("4"), utils.ParseDec("6"),
+		utils.ParseCoins("10000ucre,50000uusd"))
+	s.AssertEqual(sdk.NewInt(211803), position.Liquidity)
+	s.AssertEqual(utils.ParseCoins("8253ucre,50000uusd"), amt)
+
+	// This will prevent removing the last liquidity from position1 to withdraw
+	// all remaining reserves.
+	lpAddr2 := s.FundedAccount(2, enoughCoins)
+	s.AddLiquidity(
+		lpAddr2, pool.Id, utils.ParseDec("3"), utils.ParseDec("7"),
+		utils.ParseCoins("10000ucre,50000uusd"))
+
+	// Removing very small amount of liquidity may cause withdrawing no assets
+	// at all.
+	position, amt = s.RemoveLiquidity(lpAddr1, position.Id, sdk.NewInt(1))
+	s.AssertEqual(sdk.NewInt(211802), position.Liquidity)
+	s.AssertEqual(sdk.Coins{}, amt)
+
+	// Thus, removing all liquidity by removing small amount many times
+	// may cause a loss in assets.
+	for {
+		position, amt = s.RemoveLiquidity(
+			lpAddr1, position.Id, utils.MinInt(sdk.NewInt(4), position.Liquidity))
+		s.AssertEqual(sdk.Coins{}, amt)
+		if position.Liquidity.IsZero() {
+			break
+		}
+	}
+}
diff --git a/x/amm/keeper/source.go b/x/amm/keeper/source.go
@@ -226,9 +226,11 @@ func (k Keeper) AfterPoolOrdersExecuted(ctx sdk.Context, pool types.Pool, result
 	accrueFees()
 	k.SetPoolState(ctx, pool.Id, poolState)
 
-	if err := k.bankKeeper.SendCoins(
-		ctx, reserveAddr, pool.MustGetRewardsPoolAddress(), accruedRewards); err != nil {
-		return err
+	if accruedRewards.IsAllPositive() {
+		if err := k.bankKeeper.SendCoins(
+			ctx, reserveAddr, pool.MustGetRewardsPoolAddress(), accruedRewards); err != nil {
+			return err
+		}
 	}
 	return nil
 }
diff --git a/x/liquidamm/keeper/genesis.go b/x/liquidamm/keeper/genesis.go
@@ -16,6 +16,8 @@ func (k Keeper) InitGenesis(ctx sdk.Context, genState types.GenesisState) {
 	}
 	for _, publicPosition := range genState.PublicPositions {
 		k.SetPublicPosition(ctx, publicPosition)
+		k.SetPublicPositionByParamsIndex(ctx, publicPosition)
+		k.SetPublicPositionsByPoolIndex(ctx, publicPosition)
 	}
 	for _, auction := range genState.RewardsAuctions {
 		k.SetRewardsAuction(ctx, auction)