fix: minimize panics and refactor

x/farming/abci.go

@@ -13,10 +13,12 @@ import (
 func EndBlocker(ctx sdk.Context, k keeper.Keeper) {
 	defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyEndBlocker)
 
+	logger := k.Logger(ctx)
+
 	for _, plan := range k.GetPlans(ctx) {
 		if !plan.GetTerminated() && ctx.BlockTime().After(plan.GetEndTime()) {
 			if err := k.TerminatePlan(ctx, plan); err != nil {
-				panic(err)
+				logger.Error("failed to terminate plan", "plan_id", plan.GetId())
 			}
 		}
 	}

x/farming/handler_test.go

@@ -134,6 +134,6 @@ func (suite *ModuleTestSuite) TestMsgHarvest() {
 
 	balancesAfter := suite.app.BankKeeper.GetAllBalances(suite.ctx, suite.addrs[0])
 	suite.Require().True(coinsEq(balancesBefore.Add(rewards...), balancesAfter))
-	suite.Require().True(suite.app.BankKeeper.GetAllBalances(suite.ctx, suite.keeper.GetRewardsReservePoolAcc(suite.ctx)).IsZero())
+	suite.Require().True(suite.app.BankKeeper.GetAllBalances(suite.ctx, types.RewardsReserveAcc).IsZero())
 	suite.Require().True(suite.Rewards(suite.addrs[0]).IsZero())
 }

x/farming/keeper/genesis.go

@@ -100,7 +100,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, genState types.GenesisState) {
 	if err != nil {
 		panic(err)
 	}
-	rewardsPoolCoins := k.bankKeeper.GetAllBalances(ctx, k.GetRewardsReservePoolAcc(ctx))
+	rewardsPoolCoins := k.bankKeeper.GetAllBalances(ctx, types.RewardsReserveAcc)
 	if !genState.RewardPoolCoins.IsEqual(rewardsPoolCoins) {
 		panic(fmt.Sprintf("RewardPoolCoins differs from the actual value; have %s, want %s",
 			rewardsPoolCoins, genState.RewardPoolCoins))

x/farming/keeper/invariants.go

@@ -143,7 +143,7 @@ func OutstandingRewardsAmountInvariant(k Keeper) sdk.Invariant {
 			totalRewards = totalRewards.Add(rewards.Rewards...)
 			return false
 		})
-		balances := k.bankKeeper.GetAllBalances(ctx, k.GetRewardsReservePoolAcc(ctx))
+		balances := k.bankKeeper.GetAllBalances(ctx, types.RewardsReserveAcc)
 		_, hasNeg := sdk.NewDecCoinsFromCoins(balances...).SafeSub(totalRewards)
 		broken := hasNeg
 		return sdk.FormatInvariant(

x/farming/keeper/invariants_test.go

@@ -157,15 +157,15 @@ func (suite *KeeperTestSuite) TestRemainingRewardsAmountInvariant() {
 	// Send coins into the rewards reserve acc.
 	// Should be OK.
 	err := suite.app.BankKeeper.SendCoins(
-		ctx, suite.addrs[1], k.GetRewardsReservePoolAcc(ctx), sdk.NewCoins(sdk.NewInt64Coin(denom3, 1)))
+		ctx, suite.addrs[1], types.RewardsReserveAcc, sdk.NewCoins(sdk.NewInt64Coin(denom3, 1)))
 	suite.Require().NoError(err)
 	_, broken = farmingkeeper.RemainingRewardsAmountInvariant(k)(ctx)
 	suite.Require().False(broken)
 
 	// Send coins from the rewards reserve acc to another acc.
 	// Should not be OK.
 	err = suite.app.BankKeeper.SendCoins(
-		ctx, k.GetRewardsReservePoolAcc(ctx), suite.addrs[1], sdk.NewCoins(sdk.NewInt64Coin(denom3, 2)))
+		ctx, types.RewardsReserveAcc, suite.addrs[1], sdk.NewCoins(sdk.NewInt64Coin(denom3, 2)))
 	suite.Require().NoError(err)
 	_, broken = farmingkeeper.RemainingRewardsAmountInvariant(k)(ctx)
 	suite.Require().True(broken)
@@ -237,14 +237,14 @@ func (suite *KeeperTestSuite) TestOutstandingRewardsAmountInvariant() {
 
 	// Send coins into the rewards reserve acc. Should be OK.
 	err := suite.app.BankKeeper.SendCoins(
-		ctx, suite.addrs[1], k.GetRewardsReservePoolAcc(ctx), sdk.NewCoins(sdk.NewInt64Coin(denom3, 1)))
+		ctx, suite.addrs[1], types.RewardsReserveAcc, sdk.NewCoins(sdk.NewInt64Coin(denom3, 1)))
 	suite.Require().NoError(err)
 	_, broken = farmingkeeper.OutstandingRewardsAmountInvariant(k)(ctx)
 	suite.Require().False(broken)
 
 	// Send coins from the rewards reserve acc to another acc. Should not be OK.
 	err = suite.app.BankKeeper.SendCoins(
-		ctx, k.GetRewardsReservePoolAcc(ctx), suite.addrs[1], sdk.NewCoins(sdk.NewInt64Coin(denom3, 2)))
+		ctx, types.RewardsReserveAcc, suite.addrs[1], sdk.NewCoins(sdk.NewInt64Coin(denom3, 2)))
 	suite.Require().NoError(err)
 	_, broken = farmingkeeper.OutstandingRewardsAmountInvariant(k)(ctx)
 	suite.Require().True(broken)

x/farming/keeper/keeper.go

@@ -88,11 +88,6 @@ func (k Keeper) SetParams(ctx sdk.Context, params types.Params) {
 // GetCodec returns codec.Codec object used by the keeper>
 func (k Keeper) GetCodec() codec.BinaryCodec { return k.cdc }
 
-// GetRewardsReservePoolAcc returns module account for the rewards reserve pool account.
-func (k Keeper) GetRewardsReservePoolAcc(ctx sdk.Context) sdk.AccAddress { // nolint:interfacer
-	return types.RewardsReserveAcc
-}
-
 // GetFarmingFeeCollectorAcc returns module account for the farming fee collector account.
 func (k Keeper) GetFarmingFeeCollectorAcc(ctx sdk.Context) sdk.AccAddress {
 	params := k.GetParams(ctx)

x/farming/keeper/plan.go

@@ -11,15 +11,6 @@ import (
 	"github.com/tendermint/farming/x/farming/types"
 )
 
-// NewPlan sets the next plan number to a given PlanI.
-func (k Keeper) NewPlan(ctx sdk.Context, plan types.PlanI) types.PlanI {
-	if err := plan.SetId(k.GetNextPlanIdWithUpdate(ctx)); err != nil {
-		panic(err)
-	}
-
-	return plan
-}
-
 // GetPlan returns a plan for a given plan id.
 func (k Keeper) GetPlan(ctx sdk.Context, id uint64) (plan types.PlanI, found bool) {
 	store := ctx.KVStore(k.storeKey)
@@ -104,12 +95,7 @@ func (k Keeper) GetGlobalPlanId(ctx sdk.Context) uint64 {
 		id = 0
 	} else {
 		val := gogotypes.UInt64Value{}
-
-		err := k.cdc.Unmarshal(bz, &val)
-		if err != nil {
-			panic(err)
-		}
-
+		k.cdc.MustUnmarshal(bz, &val)
 		id = val.GetValue()
 	}
 	return id
@@ -239,9 +225,7 @@ func (k Keeper) TerminatePlan(ctx sdk.Context, plan types.PlanI) error {
 		}
 	}
 
-	if err := plan.SetTerminated(true); err != nil {
-		return err
-	}
+	_ = plan.SetTerminated(true)
 	k.SetPlan(ctx, plan)
 
 	ctx.EventManager().EmitEvents(sdk.Events{

x/farming/keeper/plan_test.go

@@ -4,47 +4,6 @@ import (
 	"github.com/tendermint/farming/x/farming/types"
 )
 
-//func (suite *KeeperTestSuite) TestGetSetNewPlan() {
-//	name := ""
-//	farmingPoolAddr := sdk.AccAddress("farmingPoolAddr")
-//	terminationAddr := sdk.AccAddress("terminationAddr")
-//
-//	stakingCoins := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(1000000)))
-//	coinWeights := sdk.NewDecCoins(
-//		sdk.DecCoin{Denom: "testFarmStakingCoinDenom", Amount: sdk.MustNewDecFromStr("1.0")},
-//	)
-//
-//	addrs := app.AddTestAddrs(suite.app, suite.ctx, 2, sdk.NewInt(2000000))
-//	farmerAddr := addrs[0]
-//
-//	startTime := time.Now().UTC()
-//	endTime := startTime.AddDate(1, 0, 0)
-//	basePlan := types.NewBasePlan(1, name, 1, farmingPoolAddr.String(), terminationAddr.String(), coinWeights, startTime, endTime)
-//	fixedPlan := types.NewFixedAmountPlan(basePlan, sdk.NewCoins(sdk.NewCoin("testFarmCoinDenom", sdk.NewInt(1000000))))
-//	suite.keeper.SetPlan(suite.ctx, fixedPlan)
-//
-//	planGet, found := suite.keeper.GetPlan(suite.ctx, 1)
-//	suite.Require().True(found)
-//	suite.Require().Equal(fixedPlan.Id, planGet.GetId())
-//	suite.Require().Equal(fixedPlan.FarmingPoolAddress, planGet.GetFarmingPoolAddress().String())
-//
-//	plans := suite.keeper.GetPlans(suite.ctx)
-//	suite.Require().Len(plans, 1)
-//	suite.Require().Equal(fixedPlan.Id, plans[0].GetId())
-//	suite.Require().Equal(fixedPlan.FarmingPoolAddress, plans[0].GetFarmingPoolAddress().String())
-//
-//	_, err := suite.keeper.Stake(suite.ctx, farmerAddr, stakingCoins)
-//	suite.Require().NoError(err)
-//
-//	stakings := suite.keeper.GetAllStakings(suite.ctx)
-//	stakingByFarmer, found := suite.keeper.GetStakingByFarmer(suite.ctx, farmerAddr)
-//	stakingsByDenom := suite.keeper.GetStakingsByStakingCoinDenom(suite.ctx, sdk.DefaultBondDenom)
-//
-//	suite.Require().True(found)
-//	suite.Require().Equal(stakings[0], stakingByFarmer)
-//	suite.Require().Equal(stakings, stakingsByDenom)
-//}
-
 func (suite *KeeperTestSuite) TestGlobalPlanId() {
 	globalPlanId := suite.keeper.GetGlobalPlanId(suite.ctx)
 	suite.Require().Equal(uint64(0), globalPlanId)

x/farming/keeper/proposal_handler.go

@@ -178,7 +178,7 @@ func (k Keeper) DeletePublicPlanProposal(ctx sdk.Context, proposals []types.Dele
 		}
 
 		if err := k.TerminatePlan(ctx, plan); err != nil {
-			panic(err)
+			return err
 		}
 
 		k.RemovePlan(ctx, plan)

x/farming/keeper/reward.go

@@ -231,7 +231,7 @@ func (k Keeper) WithdrawRewards(ctx sdk.Context, farmerAcc sdk.AccAddress, staki
 
 	if !rewards.IsZero() {
 		if !truncatedRewards.IsZero() {
-			if err := k.bankKeeper.SendCoins(ctx, k.GetRewardsReservePoolAcc(ctx), farmerAcc, truncatedRewards); err != nil {
+			if err := k.bankKeeper.SendCoins(ctx, types.RewardsReserveAcc, farmerAcc, truncatedRewards); err != nil {
 				return nil, err
 			}
 
@@ -274,7 +274,7 @@ func (k Keeper) WithdrawAllRewards(ctx sdk.Context, farmerAcc sdk.AccAddress) (s
 	})
 
 	if !totalRewards.IsZero() {
-		if err := k.bankKeeper.SendCoins(ctx, k.GetRewardsReservePoolAcc(ctx), farmerAcc, totalRewards); err != nil {
+		if err := k.bankKeeper.SendCoins(ctx, types.RewardsReserveAcc, farmerAcc, totalRewards); err != nil {
 			return nil, err
 		}
 	}
@@ -437,7 +437,7 @@ func (k Keeper) AllocateRewards(ctx sdk.Context) error {
 			continue
 		}
 
-		rewardsReserveAcc := k.GetRewardsReservePoolAcc(ctx)
+		rewardsReserveAcc := types.RewardsReserveAcc
 		if err := k.bankKeeper.SendCoins(ctx, allocInfo.Plan.GetFarmingPoolAddress(), rewardsReserveAcc, totalAllocCoins); err != nil {
 			return err
 		}
@@ -481,7 +481,7 @@ func (k Keeper) ValidateRemainingRewardsAmount(ctx sdk.Context) error {
 		return false
 	})
 
-	rewardsReservePoolBalances := k.bankKeeper.GetAllBalances(ctx, k.GetRewardsReservePoolAcc(ctx))
+	rewardsReservePoolBalances := k.bankKeeper.GetAllBalances(ctx, types.RewardsReserveAcc)
 	if !rewardsReservePoolBalances.IsAllGTE(remainingRewards) {
 		return types.ErrInvalidRemainingRewardsAmount
 	}
@@ -499,7 +499,7 @@ func (k Keeper) ValidateOutstandingRewardsAmount(ctx sdk.Context) error {
 		return false
 	})
 
-	rewardsReservePoolBalances := sdk.NewDecCoinsFromCoins(k.bankKeeper.GetAllBalances(ctx, k.GetRewardsReservePoolAcc(ctx))...)
+	rewardsReservePoolBalances := sdk.NewDecCoinsFromCoins(k.bankKeeper.GetAllBalances(ctx, types.RewardsReserveAcc)...)
 	_, hasNeg := rewardsReservePoolBalances.SafeSub(totalOutstandingRewards)
 	if hasNeg {
 		return types.ErrInvalidOutstandingRewardsAmount

x/farming/keeper/reward_test.go

@@ -379,7 +379,7 @@ func (suite *KeeperTestSuite) TestHarvest() {
 
 	balancesAfter := suite.app.BankKeeper.GetAllBalances(suite.ctx, suite.addrs[0])
 	suite.Require().True(coinsEq(balancesBefore.Add(rewards...), balancesAfter))
-	suite.Require().True(suite.app.BankKeeper.GetAllBalances(suite.ctx, suite.keeper.GetRewardsReservePoolAcc(suite.ctx)).IsZero())
+	suite.Require().True(suite.app.BankKeeper.GetAllBalances(suite.ctx, types.RewardsReserveAcc).IsZero())
 	suite.Require().True(suite.keeper.AllRewards(suite.ctx, suite.addrs[0]).IsZero())
 }
 

x/farming/keeper/staking.go

@@ -187,9 +187,7 @@ func (k Keeper) IncreaseTotalStakings(ctx sdk.Context, stakingCoinDenom string,
 	totalStaking.Amount = totalStaking.Amount.Add(amount)
 	k.SetTotalStakings(ctx, stakingCoinDenom, totalStaking)
 	if totalStaking.Amount.Equal(amount) {
-		if err := k.afterStakingCoinAdded(ctx, stakingCoinDenom); err != nil {
-			panic(err)
-		}
+		k.afterStakingCoinAdded(ctx, stakingCoinDenom)
 	}
 }
 
@@ -270,11 +268,10 @@ func (k Keeper) ReleaseStakingCoins(ctx sdk.Context, farmerAcc sdk.AccAddress, s
 
 // afterStakingCoinAdded is called after a new staking coin denom appeared
 // during ProcessQueuedCoins.
-func (k Keeper) afterStakingCoinAdded(ctx sdk.Context, stakingCoinDenom string) error {
+func (k Keeper) afterStakingCoinAdded(ctx sdk.Context, stakingCoinDenom string) {
 	k.SetHistoricalRewards(ctx, stakingCoinDenom, 0, types.HistoricalRewards{CumulativeUnitRewards: sdk.DecCoins{}})
 	k.SetCurrentEpoch(ctx, stakingCoinDenom, 1)
 	k.SetOutstandingRewards(ctx, stakingCoinDenom, types.OutstandingRewards{Rewards: sdk.DecCoins{}})
-	return nil
 }
 
 // afterStakingCoinRemoved is called after a staking coin denom got removed
@@ -288,7 +285,7 @@ func (k Keeper) afterStakingCoinRemoved(ctx sdk.Context, stakingCoinDenom string
 	outstanding, _ := k.GetOutstandingRewards(ctx, stakingCoinDenom)
 	coins, _ := outstanding.Rewards.TruncateDecimal() // Ignore remainder, since it cannot be sent.
 	if !coins.IsZero() {
-		if err := k.bankKeeper.SendCoins(ctx, k.GetRewardsReservePoolAcc(ctx), k.GetFarmingFeeCollectorAcc(ctx), coins); err != nil {
+		if err := k.bankKeeper.SendCoins(ctx, types.RewardsReserveAcc, k.GetFarmingFeeCollectorAcc(ctx), coins); err != nil {
 			return err
 		}
 	}