fix: terminate plan when deleting public plan proposal (#162)

* fix: add termination plan logic * test: add test code * docs: update proposal docs * chore: apply from code review * chore: remove TODO comment and apply feedback * test: add emit events verification * test: verify EventTypePlanTerminated is emitted
tendermint · Oct 13, 2021 · bd64f31 · bd64f31
1 parent f994143
commit bd64f31
Show file tree

Hide file tree

Showing 4 changed files with 142 additions and 54 deletions.
diff --git a/x/farming/keeper/plan.go b/x/farming/keeper/plan.go
@@ -240,10 +240,12 @@ func (k Keeper) CreateRatioPlan(ctx sdk.Context, msg *types.MsgCreateRatioPlan,
 // TerminatePlan sends all remaining coins in the plan's farming pool to
 // the termination address and mark the plan as terminated.
 func (k Keeper) TerminatePlan(ctx sdk.Context, plan types.PlanI) error {
-	balances := k.bankKeeper.GetAllBalances(ctx, plan.GetFarmingPoolAddress())
-	if balances.IsAllPositive() {
-		if err := k.bankKeeper.SendCoins(ctx, plan.GetFarmingPoolAddress(), plan.GetTerminationAddress(), balances); err != nil {
-			return err
+	if plan.GetFarmingPoolAddress().String() != plan.GetTerminationAddress().String() {
+		balances := k.bankKeeper.GetAllBalances(ctx, plan.GetFarmingPoolAddress())
+		if balances.IsAllPositive() {
+			if err := k.bankKeeper.SendCoins(ctx, plan.GetFarmingPoolAddress(), plan.GetTerminationAddress(), balances); err != nil {
+				return err
+			}
 		}
 	}
 
@@ -258,7 +260,6 @@ func (k Keeper) TerminatePlan(ctx sdk.Context, plan types.PlanI) error {
 			sdk.NewAttribute(types.AttributeKeyPlanId, strconv.FormatUint(plan.GetId(), 10)),
 			sdk.NewAttribute(types.AttributeKeyFarmingPoolAddress, plan.GetFarmingPoolAddress().String()),
 			sdk.NewAttribute(types.AttributeKeyTerminationAddress, plan.GetTerminationAddress().String()),
-			// TODO: add refunded coins?
 		),
 	})
 

diff --git a/x/farming/keeper/proposal_handler.go b/x/farming/keeper/proposal_handler.go
@@ -225,6 +225,10 @@ func (k Keeper) DeletePublicPlanProposal(ctx sdk.Context, proposals []*types.Del
 			return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "plan %d is not found", p.GetPlanId())
 		}
 
+		if err := k.TerminatePlan(ctx, plan); err != nil {
+			panic(err)
+		}
+
 		k.RemovePlan(ctx, plan)
 
 		logger := k.Logger(ctx)

diff --git a/x/farming/keeper/proposal_handler_test.go b/x/farming/keeper/proposal_handler_test.go
@@ -1,6 +1,8 @@
 package keeper_test
 
 import (
+	"fmt"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 
@@ -406,80 +408,153 @@ func (suite *KeeperTestSuite) TestValidateDeletePublicPlanProposal() {
 
 func (suite *KeeperTestSuite) TestUpdatePlanType() {
 	// create a ratio public plan
-	addRequests := []*types.AddRequestProposal{
-		types.NewAddRequestProposal(
-			"testPlan",
-			suite.addrs[0].String(),
-			suite.addrs[0].String(),
-			sdk.NewDecCoins(
-				sdk.NewDecCoinFromDec(denom1, sdk.NewDecWithPrec(3, 1)), // 30%
-				sdk.NewDecCoinFromDec(denom2, sdk.NewDecWithPrec(7, 1)), // 70%
-			),
-			types.ParseTime("2021-08-01T00:00:00Z"),
-			types.ParseTime("2021-08-30T00:00:00Z"),
-			nil,
-			sdk.NewDecWithPrec(10, 2), // 10%
-		),
-	}
-
 	err := keeper.HandlePublicPlanProposal(
 		suite.ctx,
 		suite.keeper,
-		types.NewPublicPlanProposal("testTitle", "testDescription", addRequests, nil, nil),
+		types.NewPublicPlanProposal("testTitle", "testDescription", []*types.AddRequestProposal{
+			types.NewAddRequestProposal(
+				"testPlan",
+				suite.addrs[0].String(),
+				suite.addrs[0].String(),
+				sdk.NewDecCoins(
+					sdk.NewDecCoinFromDec(denom1, sdk.NewDecWithPrec(3, 1)),
+					sdk.NewDecCoinFromDec(denom2, sdk.NewDecWithPrec(7, 1)),
+				),
+				types.ParseTime("0001-01-01T00:00:00Z"),
+				types.ParseTime("9999-12-31T00:00:00Z"),
+				sdk.NewCoins(),
+				sdk.NewDecWithPrec(10, 2),
+			),
+		}, nil, nil),
 	)
 	suite.Require().NoError(err)
 
 	plan, found := suite.keeper.GetPlan(suite.ctx, uint64(1))
 	suite.Require().Equal(true, found)
 	suite.Require().Equal(plan.(*types.RatioPlan).EpochRatio, sdk.NewDecWithPrec(10, 2))
 
-	// update the ratio plan to fixed amount plan type
-	updateRequests := []*types.UpdateRequestProposal{
-		types.NewUpdateRequestProposal(
-			plan.GetId(),
-			plan.GetName(),
-			plan.GetFarmingPoolAddress().String(),
-			plan.GetTerminationAddress().String(),
-			plan.GetStakingCoinWeights(),
-			plan.GetStartTime(),
-			plan.GetEndTime(),
-			sdk.NewCoins(sdk.NewInt64Coin("stake", 100_000)),
-			sdk.ZeroDec(),
-		)}
-
+	// update the ratio plan type to fixed amount plan type
 	err = keeper.HandlePublicPlanProposal(
 		suite.ctx,
 		suite.keeper,
-		types.NewPublicPlanProposal("testTitle", "testDescription", nil, updateRequests, nil),
+		types.NewPublicPlanProposal("testTitle", "testDescription", nil, []*types.UpdateRequestProposal{
+			types.NewUpdateRequestProposal(
+				plan.GetId(),
+				plan.GetName(),
+				plan.GetFarmingPoolAddress().String(),
+				plan.GetTerminationAddress().String(),
+				plan.GetStakingCoinWeights(),
+				plan.GetStartTime(),
+				plan.GetEndTime(),
+				sdk.NewCoins(sdk.NewInt64Coin("stake", 100_000)),
+				sdk.ZeroDec(),
+			),
+		}, nil),
 	)
 	suite.Require().NoError(err)
 
 	plan, found = suite.keeper.GetPlan(suite.ctx, uint64(1))
 	suite.Require().Equal(true, found)
 	suite.Require().Equal(plan.(*types.FixedAmountPlan).EpochAmount, sdk.NewCoins(sdk.NewInt64Coin("stake", 100_000)))
 
-	// update the fixed amount plan back to ratio plan
-	updateRequests = []*types.UpdateRequestProposal{
-		types.NewUpdateRequestProposal(
-			plan.GetId(),
-			plan.GetName(),
-			plan.GetFarmingPoolAddress().String(),
-			plan.GetTerminationAddress().String(),
-			plan.GetStakingCoinWeights(),
-			plan.GetStartTime(),
-			plan.GetEndTime(),
-			nil,
-			sdk.NewDecWithPrec(7, 2), // 7%
-		)}
-
+	// update back to ratio plan with different epoch ratio
 	err = keeper.HandlePublicPlanProposal(
 		suite.ctx,
 		suite.keeper,
-		types.NewPublicPlanProposal("testTitle", "testDescription", nil, updateRequests, nil),
+		types.NewPublicPlanProposal("testTitle", "testDescription", nil, []*types.UpdateRequestProposal{
+			types.NewUpdateRequestProposal(
+				plan.GetId(),
+				plan.GetName(),
+				plan.GetFarmingPoolAddress().String(),
+				plan.GetTerminationAddress().String(),
+				plan.GetStakingCoinWeights(),
+				plan.GetStartTime(),
+				plan.GetEndTime(),
+				nil,
+				sdk.NewDecWithPrec(7, 2), // 7%
+			),
+		}, nil),
 	)
 	suite.Require().NoError(err)
 
 	plan, found = suite.keeper.GetPlan(suite.ctx, uint64(1))
 	suite.Require().Equal(true, found)
 	suite.Require().Equal(plan.(*types.RatioPlan).EpochRatio, sdk.NewDecWithPrec(7, 2))
 }
+
+func (suite *KeeperTestSuite) TestDeletePublicPlan() {
+	for _, tc := range []struct {
+		name             string
+		farmingPoolAddr  sdk.AccAddress
+		terminationAddr  sdk.AccAddress
+		expectedBalances sdk.Coins
+	}{
+		{
+			"farming pool address and termination address are equal",
+			suite.addrs[0],
+			suite.addrs[0],
+			initialBalances,
+		},
+		{
+			"farming pool address and termination address are not equal",
+			suite.addrs[1],
+			suite.addrs[2],
+			sdk.Coins{},
+		},
+	} {
+		suite.Run(tc.name, func() {
+			cacheCtx, _ := suite.ctx.CacheContext()
+
+			// create a public plan
+			err := keeper.HandlePublicPlanProposal(
+				cacheCtx,
+				suite.keeper,
+				types.NewPublicPlanProposal("testTitle", "testDescription", []*types.AddRequestProposal{
+					types.NewAddRequestProposal(
+						"testPlan",
+						tc.farmingPoolAddr.String(),
+						tc.terminationAddr.String(),
+						sdk.NewDecCoins(
+							sdk.NewDecCoinFromDec(denom1, sdk.NewDecWithPrec(3, 1)),
+							sdk.NewDecCoinFromDec(denom2, sdk.NewDecWithPrec(7, 1)),
+						),
+						types.ParseTime("0001-01-01T00:00:00Z"),
+						types.ParseTime("9999-12-31T00:00:00Z"),
+						sdk.NewCoins(sdk.NewInt64Coin(denom3, 100_000_000)),
+						sdk.ZeroDec(),
+					),
+				}, nil, nil),
+			)
+			suite.Require().NoError(err)
+
+			plans := suite.keeper.GetPlans(cacheCtx)
+
+			// delete the plan
+			err = keeper.HandlePublicPlanProposal(
+				cacheCtx,
+				suite.keeper,
+				types.NewPublicPlanProposal("testTitle", "testDescription", nil, nil, []*types.DeleteRequestProposal{
+					types.NewDeleteRequestProposal(plans[0].GetId()),
+				}),
+			)
+			suite.Require().NoError(err)
+
+			// the plan should be successfully removed and coins meet the expected balances
+			_, found := suite.keeper.GetPlan(cacheCtx, plans[0].GetId())
+			suite.Require().Equal(false, found)
+			suite.Require().Equal(tc.expectedBalances, suite.app.BankKeeper.GetAllBalances(cacheCtx, tc.farmingPoolAddr))
+
+			isPlanTerminatedEventType := false
+			for _, e := range cacheCtx.EventManager().ABCIEvents() {
+				if e.Type == types.EventTypePlanTerminated {
+					suite.Require().Equal(fmt.Sprint(plans[0].GetId()), string(e.Attributes[0].Value))
+					suite.Require().Equal(tc.farmingPoolAddr.String(), string(e.Attributes[1].Value))
+					suite.Require().Equal(tc.terminationAddr.String(), string(e.Attributes[2].Value))
+					isPlanTerminatedEventType = true
+					break
+				}
+			}
+			suite.Require().True(isPlanTerminatedEventType, "plan_terminated events should be emitted")
+		})
+	}
+}
diff --git a/x/farming/spec/08_proposal.md b/x/farming/spec/08_proposal.md
@@ -2,7 +2,13 @@
 
 # Proposal
 
-The farming module contains the following public plan governance proposal that receives one of the following requests. A request `AddRequestProposal` that creates a public farming plan, a request `UpdateRequestProposal` that updates the plan, and a request `DeleteRequestProposal` that deletes the plan. For most cases, it expects that a single request is used, but note that `PublicPlanProposal` accepts more than a single request to cover broader use cases. Also,
+The farming module contains the following public plan governance proposal that receives one of the following requests. 
+
+- `AddRequestProposal` is the request proposal that requests the module to create a public farming plan. You can either input epoch amount `EpochAmount` or epoch ratio `EpochRatio`. Depending on which value of the parameter you input, it creates the following plan type `FixedAmountPlan` or `RatioPlan`.
+
+- `UpdateRequestProposal` is the request proposal that requests the module to update the plan. You can also update the plan type. 
+
+- `DeleteRequestProposal` is the request proposal that requests the module to delete the plan. It sends all remaining coins in the plan's farming pool `FarmingPoolAddress` to the termination address `TerminationAddress` and mark the plan as terminated.
 
 ## PublicPlanProposal
 
@@ -26,7 +32,7 @@ type PublicPlanProposal struct {
 
 ## AddRequestProposal
 
-Note that when requesting `AddRequestProposal` depending on which field is passed, either `EpochAmount` or `EpochRatio`, it creates a `FixedAmountPlan` or `RatioPlan`.
+You can either input epoch amount `EpochAmount` or epoch ratio `EpochRatio`. Depending on which value of the parameter you input, it creates the following plan type `FixedAmountPlan` or `RatioPlan`.
 
 ```go
 // AddRequestProposal details a proposal for creating a public plan.
@@ -82,6 +88,8 @@ type UpdateRequestProposal struct {
 
 ## DeleteRequestProposal
 
+The request requests the module to delete the plan and it sends all remaining coins in the plan's farming pool `FarmingPoolAddress` to the termination address `TerminationAddress` and mark the plan as terminated.
+
 ```go
 // DeleteRequestProposal details a proposal for deleting an existing public plan.
 type DeleteRequestProposal struct {