Merge pull request #81 from crescent-network/cherry-pick/380

fix: public farming plan must have same farming pool/termination address #380
crescent-network · Oct 20, 2022 · e6f9286 · e6f9286
2 parents d9dd646 + 7e06a64
commit e6f9286
Showing 4 changed files with 64 additions and 15 deletions.
diff --git a/x/farm/keeper/plan.go b/x/farm/keeper/plan.go
@@ -121,11 +121,13 @@ func (k Keeper) TerminatePlan(ctx sdk.Context, plan types.Plan) error {
 		return types.ErrPlanAlreadyTerminated
 	}
 	farmingPoolAddr := plan.GetFarmingPoolAddress()
-	balances := k.bankKeeper.SpendableCoins(ctx, farmingPoolAddr)
-	if !balances.IsZero() {
-		if err := k.bankKeeper.SendCoins(
-			ctx, farmingPoolAddr, plan.GetTerminationAddress(), balances); err != nil {
-			return err
+	if plan.FarmingPoolAddress != plan.TerminationAddress {
+		balances := k.bankKeeper.SpendableCoins(ctx, farmingPoolAddr)
+		if !balances.IsZero() {
+			if err := k.bankKeeper.SendCoins(
+				ctx, farmingPoolAddr, plan.GetTerminationAddress(), balances); err != nil {
+				return err
+			}
 		}
 	}
 	plan.IsTerminated = true

diff --git a/x/farm/keeper/proposal_handler_test.go b/x/farm/keeper/proposal_handler_test.go
@@ -1,20 +1,17 @@
 package keeper_test
 
 import (
-	sdk "github.com/cosmos/cosmos-sdk/types"
-
 	utils "github.com/crescent-network/crescent/v3/types"
 	"github.com/crescent-network/crescent/v3/x/farm/types"
 )
 
 func (s *KeeperTestSuite) TestFarmingPlanProposalHandler() {
 	farmingPoolAddr := utils.TestAddress(0)
-	termAddr := utils.TestAddress(1)
 	s.fundAddr(farmingPoolAddr, utils.ParseCoins("100_000000stake"))
 
 	pair := s.createPair("denom1", "denom2")
 	createPlanReq := types.NewCreatePlanRequest(
-		"Farming Plan #1", farmingPoolAddr, termAddr,
+		"Farming Plan #1", farmingPoolAddr, farmingPoolAddr,
 		[]types.RewardAllocation{
 			types.NewRewardAllocation(pair.Id, utils.ParseCoins("100_000000stake")),
 		}, sampleStartTime, sampleEndTime)
@@ -26,7 +23,7 @@ func (s *KeeperTestSuite) TestFarmingPlanProposalHandler() {
 	plan, found := s.keeper.GetPlan(s.ctx, 1)
 	s.Require().True(found)
 	s.Require().Equal(farmingPoolAddr.String(), plan.FarmingPoolAddress)
-	s.Require().Equal(termAddr.String(), plan.TerminationAddress)
+	s.Require().Equal(farmingPoolAddr.String(), plan.TerminationAddress)
 	s.Require().False(plan.IsPrivate)
 
 	terminatePlanReq := types.NewTerminatePlanRequest(1)
@@ -38,10 +35,8 @@ func (s *KeeperTestSuite) TestFarmingPlanProposalHandler() {
 	plan, found = s.keeper.GetPlan(s.ctx, 1)
 	s.Require().True(found)
 	s.Require().True(plan.IsTerminated)
-	// Check if the remaining balances in the farming pool has moved into
-	// the termination address.
-	s.assertEq(sdk.Coins{}, s.getBalances(farmingPoolAddr))
-	s.assertEq(utils.ParseCoins("100_000000stake"), s.getBalances(termAddr))
+	// Balances not changed.
+	s.assertEq(utils.ParseCoins("100_000000stake"), s.getBalances(farmingPoolAddr))
 
 	privPlan := s.createPrivatePlan([]types.RewardAllocation{
 		types.NewRewardAllocation(pair.Id, utils.ParseCoins("100_000000stake")),

diff --git a/x/farm/types/plan.go b/x/farm/types/plan.go
@@ -5,6 +5,8 @@ import (
 	"time"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	utils "github.com/crescent-network/crescent/v3/types"
 )
 
 const day = 24 * time.Hour
@@ -46,6 +48,13 @@ func (plan Plan) Validate() error {
 	if _, err := sdk.AccAddressFromBech32(plan.TerminationAddress); err != nil {
 		return fmt.Errorf("invalid termination address: %w", err)
 	}
+	if !plan.IsPrivate {
+		if plan.FarmingPoolAddress != plan.TerminationAddress {
+			return fmt.Errorf(
+				"farming pool address and termination address of a public plan must be same: %s != %s",
+				plan.FarmingPoolAddress, plan.TerminationAddress)
+		}
+	}
 	if err := ValidateRewardAllocations(plan.RewardAllocations); err != nil {
 		return fmt.Errorf("invalid reward allocations: %w", err)
 	}
@@ -98,7 +107,15 @@ func ValidateRewardAllocations(rewardAllocs []RewardAllocation) error {
 		if err := rewardAlloc.RewardsPerDay.Validate(); err != nil {
 			return fmt.Errorf("invalid rewards per day: %w", err)
 		}
-		// TODO: reject too big rewardsPerDay which can cause an overflow
+		overflow := false
+		utils.SafeMath(func() {
+			RewardsForBlock(rewardAlloc.RewardsPerDay, day)
+		}, func() {
+			overflow = true
+		})
+		if overflow {
+			return fmt.Errorf("too much rewards per day")
+		}
 	}
 	return nil
 }
diff --git a/x/farm/types/plan_test.go b/x/farm/types/plan_test.go
@@ -29,6 +29,29 @@ func TestPlan_Validate(t *testing.T) {
 			},
 			"too long plan description, maximum 200",
 		},
+		{
+			"invalid farming pool address",
+			func(plan *types.Plan) {
+				plan.FarmingPoolAddress = "invalidaddr"
+			},
+			"invalid farming pool address: decoding bech32 failed: invalid separator index -1",
+		},
+		{
+			"invalid termination address",
+			func(plan *types.Plan) {
+				plan.TerminationAddress = "invalidaddr"
+			},
+			"invalid termination address: decoding bech32 failed: invalid separator index -1",
+		},
+		{
+			"same farming pool address and termination address",
+			func(plan *types.Plan) {
+				plan.FarmingPoolAddress = utils.TestAddress(0).String()
+				plan.TerminationAddress = utils.TestAddress(1).String()
+				plan.IsPrivate = false
+			},
+			"farming pool address and termination address of a public plan must be same: cosmos1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqnrql8a != cosmos1qgqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqggwm7m",
+		},
 		{
 			"empty reward allocations",
 			func(plan *types.Plan) {
@@ -60,6 +83,18 @@ func TestPlan_Validate(t *testing.T) {
 			},
 			"invalid reward allocations: invalid rewards per day: coin 0stake amount is not positive",
 		},
+		{
+			"too much rewards per day",
+			func(plan *types.Plan) {
+				plan.RewardAllocations = []types.RewardAllocation{
+					{
+						PairId: 1,
+						RewardsPerDay: utils.ParseCoins("57896044618658097711785492504343953926634992332820282019728792003956564819967stake"),
+					},
+				}
+			},
+			"invalid reward allocations: too much rewards per day",
+		},
 		{
 			"duplicate pair id",
 			func(plan *types.Plan) {