diff --git a/x/farming/keeper/proposal_handler.go b/x/farming/keeper/proposal_handler.go index c84fe8b0..af6a68bf 100644 --- a/x/farming/keeper/proposal_handler.go +++ b/x/farming/keeper/proposal_handler.go @@ -48,7 +48,7 @@ func (k Keeper) AddPublicPlanProposal(ctx sdk.Context, proposals []*types.AddReq return err } - if p.EpochAmount.IsAllPositive() { + if p.IsForFixedAmountPlan() { msg := types.NewMsgCreateFixedAmountPlan( p.GetName(), farmingPoolAddrAcc, @@ -65,8 +65,7 @@ func (k Keeper) AddPublicPlanProposal(ctx sdk.Context, proposals []*types.AddReq logger := k.Logger(ctx) logger.Info("created public fixed amount plan", "fixed_amount_plan", plan) - - } else if p.EpochRatio.IsPositive() { + } else { msg := types.NewMsgCreateRatioPlan( p.GetName(), farmingPoolAddrAcc, @@ -76,10 +75,6 @@ func (k Keeper) AddPublicPlanProposal(ctx sdk.Context, proposals []*types.AddReq p.EpochRatio, ) - if err = msg.ValidateBasic(); err != nil { - return err - } - plan, err := k.CreateRatioPlan(ctx, msg, farmingPoolAddrAcc, terminationAcc, types.PlanTypePublic) if err != nil { return err @@ -101,123 +96,72 @@ func (k Keeper) UpdatePublicPlanProposal(ctx sdk.Context, proposals []*types.Upd return sdkerrors.Wrapf(sdkerrors.ErrNotFound, "plan %d is not found", p.GetPlanId()) } - if p.EpochAmount.IsAllPositive() { - if p.GetName() != "" { - if err := plan.SetName(p.GetName()); err != nil { - return err - } + if p.GetName() != "" { + if err := plan.SetName(p.GetName()); err != nil { + return err } + } - if p.GetFarmingPoolAddress() != "" { - farmingPoolAddrAcc, err := sdk.AccAddressFromBech32(p.GetFarmingPoolAddress()) - if err != nil { - return err - } - if err := plan.SetFarmingPoolAddress(farmingPoolAddrAcc); err != nil { - return err - } + if p.GetFarmingPoolAddress() != "" { + farmingPoolAddrAcc, err := sdk.AccAddressFromBech32(p.GetFarmingPoolAddress()) + if err != nil { + return err } - - if p.GetTerminationAddress() != "" { - terminationAddrAcc, err := sdk.AccAddressFromBech32(p.GetTerminationAddress()) - if err != nil { - return err - } - if err := plan.SetTerminationAddress(terminationAddrAcc); err != nil { - return err - } + if err := plan.SetFarmingPoolAddress(farmingPoolAddrAcc); err != nil { + return err } + } - if p.GetStakingCoinWeights() != nil { - if err := plan.SetStakingCoinWeights(p.GetStakingCoinWeights()); err != nil { - return err - } + if p.GetTerminationAddress() != "" { + terminationAddrAcc, err := sdk.AccAddressFromBech32(p.GetTerminationAddress()) + if err != nil { + return err + } + if err := plan.SetTerminationAddress(terminationAddrAcc); err != nil { + return err } + } - if p.GetStartTime() != nil { - if err := plan.SetStartTime(*p.GetStartTime()); err != nil { - return err - } + if p.GetStakingCoinWeights() != nil { + if err := plan.SetStakingCoinWeights(p.GetStakingCoinWeights()); err != nil { + return err } + } - if p.GetEndTime() != nil { - if err := plan.SetEndTime(*p.GetEndTime()); err != nil { - return err - } + if p.GetStartTime() != nil { + if err := plan.SetStartTime(*p.GetStartTime()); err != nil { + return err } + } - // change the plan to fixed amount plan if an epoch amount exists - if p.GetEpochAmount().IsAllPositive() { - plan = types.NewFixedAmountPlan(plan.GetBasePlan(), p.GetEpochAmount()) + if p.GetEndTime() != nil { + if err := plan.SetEndTime(*p.GetEndTime()); err != nil { + return err } + } - k.SetPlan(ctx, plan) + if p.IsForFixedAmountPlan() { + // change the plan to fixed amount plan + plan = types.NewFixedAmountPlan(plan.GetBasePlan(), p.GetEpochAmount()) logger := k.Logger(ctx) logger.Info("updated public fixed amount plan", "fixed_amount_plan", plan) - } else if p.EpochRatio.IsPositive() { - if p.GetName() != "" { - if err := plan.SetName(p.GetName()); err != nil { - return err - } - } - - if p.GetFarmingPoolAddress() != "" { - farmingPoolAddrAcc, err := sdk.AccAddressFromBech32(p.GetFarmingPoolAddress()) - if err != nil { - return err - } - if err := plan.SetFarmingPoolAddress(farmingPoolAddrAcc); err != nil { - return err - } - } - - if p.GetTerminationAddress() != "" { - terminationAddrAcc, err := sdk.AccAddressFromBech32(p.GetTerminationAddress()) - if err != nil { - return err - } - if err := plan.SetTerminationAddress(terminationAddrAcc); err != nil { - return err - } - } - - if p.GetStakingCoinWeights() != nil { - if err := plan.SetStakingCoinWeights(p.GetStakingCoinWeights()); err != nil { - return err - } - } - - if p.GetStartTime() != nil { - if err := plan.SetStartTime(*p.GetStartTime()); err != nil { - return err - } - } - - if p.GetEndTime() != nil { - if err := plan.SetEndTime(*p.GetEndTime()); err != nil { - return err - } - } - - // change the plan to ratio plan if an epoch ratio exists - if p.EpochRatio.IsPositive() { - plan = types.NewRatioPlan(plan.GetBasePlan(), p.EpochRatio) - } - - k.SetPlan(ctx, plan) + } else if p.IsForRatioPlan() { + // change the plan to ratio plan + plan = types.NewRatioPlan(plan.GetBasePlan(), p.EpochRatio) logger := k.Logger(ctx) logger.Info("updated public ratio plan", "ratio_plan", plan) - } + + k.SetPlan(ctx, plan) } return nil } -// DeletePublicPlanProposal delets public plan proposal once the governance proposal is passed. +// DeletePublicPlanProposal deletes public plan proposal once the governance proposal is passed. func (k Keeper) DeletePublicPlanProposal(ctx sdk.Context, proposals []*types.DeleteRequestProposal) error { for _, p := range proposals { plan, found := k.GetPlan(ctx, p.GetPlanId()) diff --git a/x/farming/keeper/proposal_handler_test.go b/x/farming/keeper/proposal_handler_test.go index bbe0bc2d..1e9de7ae 100644 --- a/x/farming/keeper/proposal_handler_test.go +++ b/x/farming/keeper/proposal_handler_test.go @@ -29,7 +29,7 @@ func (suite *KeeperTestSuite) TestValidateAddPublicPlanProposal() { types.ParseTime("2021-08-01T00:00:00Z"), types.ParseTime("2021-08-30T00:00:00Z"), sdk.NewCoins(sdk.NewInt64Coin(denom3, 100_000_000)), - sdk.ZeroDec(), + sdk.Dec{}, )}, nil, }, @@ -125,7 +125,7 @@ func (suite *KeeperTestSuite) TestValidateAddPublicPlanProposal() { sdk.NewCoins(sdk.NewInt64Coin(denom3, 1)), sdk.NewDec(1), )}, - sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "only one of epoch amount or epoch ratio must be provided"), + sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "exactly one of epoch amount or epoch ratio must be provided"), }, { "epoch amount & epoch ratio case #2", @@ -142,7 +142,7 @@ func (suite *KeeperTestSuite) TestValidateAddPublicPlanProposal() { sdk.NewCoins(), sdk.ZeroDec(), )}, - sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "only one of epoch amount or epoch ratio must be provided"), + sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "exactly one of epoch amount or epoch ratio must be provided"), }, } { suite.Run(tc.name, func() { @@ -155,7 +155,9 @@ func (suite *KeeperTestSuite) TestValidateAddPublicPlanProposal() { } err := proposal.ValidateBasic() - if err == nil { + if tc.expectedErr == nil { + suite.NoError(err) + err := keeper.HandlePublicPlanProposal(suite.ctx, suite.keeper, proposal) suite.Require().NoError(err) @@ -320,7 +322,7 @@ func (suite *KeeperTestSuite) TestValidateUpdatePublicPlanProposal() { sdk.NewCoins(sdk.NewInt64Coin("stake", 100_000)), plan.(*types.RatioPlan).EpochRatio, )}, - sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "only one of epoch amount or epoch ratio must be provided"), + sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "at most one of epoch amount or epoch ratio must be provided"), }, { "epoch amount & epoch ratio case #2", @@ -333,10 +335,10 @@ func (suite *KeeperTestSuite) TestValidateUpdatePublicPlanProposal() { plan.GetStakingCoinWeights(), plan.GetStartTime(), plan.GetEndTime(), - sdk.NewCoins(sdk.NewInt64Coin("stake", 0)), + sdk.Coins{sdk.NewInt64Coin("stake", 0)}, sdk.ZeroDec(), )}, - sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "only one of epoch amount or epoch ratio must be provided"), + sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "invalid epoch amount: coin 0stake amount is not positive"), }, } { suite.Run(tc.name, func() { @@ -349,7 +351,9 @@ func (suite *KeeperTestSuite) TestValidateUpdatePublicPlanProposal() { } err := proposal.ValidateBasic() - if err == nil { + if tc.expectedErr == nil { + suite.NoError(err) + err := keeper.HandlePublicPlanProposal(suite.ctx, suite.keeper, proposal) suite.Require().NoError(err) diff --git a/x/farming/simulation/proposals.go b/x/farming/simulation/proposals.go index f28447d2..20d042a0 100644 --- a/x/farming/simulation/proposals.go +++ b/x/farming/simulation/proposals.go @@ -15,7 +15,7 @@ import ( /* [TODO]: We need to come up with better ways to simulate public plan proposals. - Currently, the details are igored and only basic logics are written to simulate. + Currently, the details are ignored and only basic logics are written to simulate. These are some of the following considerations that i think need to be discussed and addressed: 1. Randomize staking coin weights (single or multiple denoms) @@ -78,7 +78,6 @@ func SimulateAddPublicPlanProposal(ak types.AccountKeeper, bk types.BankKeeper, StartTime: ctx.BlockTime(), EndTime: ctx.BlockTime().AddDate(0, 1, 0), EpochAmount: sdk.NewCoins(sdk.NewInt64Coin(poolCoins[r.Intn(3)].Denom, int64(simtypes.RandIntBetween(r, 10_000_000, 1_000_000_000)))), - EpochRatio: sdk.ZeroDec(), } addRequests := []*types.AddRequestProposal{req} diff --git a/x/farming/types/msgs.go b/x/farming/types/msgs.go index 87e43161..5f2fa1ac 100644 --- a/x/farming/types/msgs.go +++ b/x/farming/types/msgs.go @@ -57,15 +57,9 @@ func (msg MsgCreateFixedAmountPlan) ValidateBasic() error { if !msg.EndTime.After(msg.StartTime) { return sdkerrors.Wrapf(ErrInvalidPlanEndTime, "end time %s must be greater than start time %s", msg.EndTime.Format(time.RFC3339), msg.StartTime.Format(time.RFC3339)) } - if msg.StakingCoinWeights.Empty() { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "staking coin weights must not be empty") - } - if err := msg.StakingCoinWeights.Validate(); err != nil { + if err := ValidateStakingCoinTotalWeights(msg.StakingCoinWeights); err != nil { return err } - if ok := ValidateStakingCoinTotalWeights(msg.StakingCoinWeights); !ok { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "total weight must be 1") - } if msg.EpochAmount.Empty() { return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "epoch amount must not be empty") } @@ -125,17 +119,11 @@ func (msg MsgCreateRatioPlan) ValidateBasic() error { if !msg.EndTime.After(msg.StartTime) { return sdkerrors.Wrapf(ErrInvalidPlanEndTime, "end time %s must be greater than start time %s", msg.EndTime.Format(time.RFC3339), msg.StartTime.Format(time.RFC3339)) } - if msg.StakingCoinWeights.Empty() { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "staking coin weights must not be empty") - } - if err := msg.StakingCoinWeights.Validate(); err != nil { + if err := ValidateStakingCoinTotalWeights(msg.StakingCoinWeights); err != nil { return err } - if ok := ValidateStakingCoinTotalWeights(msg.StakingCoinWeights); !ok { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "total weight must be 1") - } - if !msg.EpochRatio.IsPositive() { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "invalid epoch ratio") + if err := ValidateEpochRatio(msg.EpochRatio); err != nil { + return err } if msg.EpochRatio.GT(sdk.NewDec(1)) { return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "invalid epoch ratio") diff --git a/x/farming/types/msgs_test.go b/x/farming/types/msgs_test.go index c91e2595..101d9fca 100644 --- a/x/farming/types/msgs_test.go +++ b/x/farming/types/msgs_test.go @@ -119,7 +119,7 @@ func TestMsgCreateRatioPlan(t *testing.T) { ), }, { - "invalid epoch ratio: invalid request", + "epoch ratio must be positive: -1.000000000000000000: invalid request", types.NewMsgCreateRatioPlan( name, creatorAddr, stakingCoinWeights, startTime, endTime, sdk.NewDec(-1), diff --git a/x/farming/types/plan.go b/x/farming/types/plan.go index da70d66b..cea75f1b 100644 --- a/x/farming/types/plan.go +++ b/x/farming/types/plan.go @@ -177,15 +177,8 @@ func (plan BasePlan) Validate() error { if len(plan.Name) > MaxNameLength { return sdkerrors.Wrapf(ErrInvalidPlanNameLength, "plan name cannot be longer than max length of %d", MaxNameLength) } - - if plan.StakingCoinWeights.Empty() { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "staking coin weights must not be empty") - } - if err := plan.StakingCoinWeights.Validate(); err != nil { - return sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid staking coin weights: %v", err) - } - if ok := ValidateStakingCoinTotalWeights(plan.StakingCoinWeights); !ok { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "total weight must be 1") + if err := ValidateStakingCoinTotalWeights(plan.StakingCoinWeights); err != nil { + return err } if !plan.EndTime.After(plan.StartTime) { return sdkerrors.Wrapf(ErrInvalidPlanEndTime, "end time %s must be greater than start time %s", plan.EndTime, plan.StartTime) @@ -301,6 +294,25 @@ func ValidateTotalEpochRatio(i interface{}) error { return nil } +// ValidateEpochRatio validate a epoch ratio that must be positive and less than 1. +func ValidateEpochRatio(epochRatio sdk.Dec) error { + if !epochRatio.IsPositive() { + return sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "epoch ratio must be positive: %s", epochRatio) + } + if epochRatio.GT(sdk.OneDec()) { + return sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "epoch ratio must be less than 1: %s", epochRatio) + } + return nil +} + +// ValidateEpochAmount validate a epoch amount that must be valid coins. +func ValidateEpochAmount(epochAmount sdk.Coins) error { + if err := epochAmount.Validate(); err != nil { + return sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid epoch amount: %v", err) + } + return nil +} + // PackPlan converts PlanI to Any func PackPlan(plan PlanI) (*codectypes.Any, error) { any, err := codectypes.NewAnyWithValue(plan) @@ -349,12 +361,21 @@ func UnpackPlans(plansAny []*codectypes.Any) ([]PlanI, error) { } // ValidateStakingCoinTotalWeights validates the total staking coin weights must be equal to 1. -func ValidateStakingCoinTotalWeights(weights sdk.DecCoins) bool { +func ValidateStakingCoinTotalWeights(weights sdk.DecCoins) error { + if weights.Empty() { + return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "staking coin weights must not be empty") + } + if err := weights.Validate(); err != nil { + return sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid staking coin weights: %v", err) + } totalWeight := sdk.ZeroDec() for _, w := range weights { totalWeight = totalWeight.Add(w.Amount) } - return totalWeight.Equal(sdk.NewDec(1)) + if !totalWeight.Equal(sdk.OneDec()) { + return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "total weight must be 1") + } + return nil } // IsPlanActiveAt returns if the plan is active at given time t. diff --git a/x/farming/types/plan_test.go b/x/farming/types/plan_test.go index c75f5ea5..d296b008 100644 --- a/x/farming/types/plan_test.go +++ b/x/farming/types/plan_test.go @@ -373,44 +373,63 @@ func TestIsPlanActiveAt(t *testing.T) { func TestValidateStakingCoinTotalWeights(t *testing.T) { for _, tc := range []struct { + name string stakingCoinWeights sdk.DecCoins - valid bool + expectedErr string }{ { + "nil case", nil, - false, + "staking coin weights must not be empty: invalid request", }, { + "empty case", sdk.DecCoins{}, - false, + "staking coin weights must not be empty: invalid request", }, { + "valid case 1", sdk.NewDecCoins(sdk.NewInt64DecCoin("stake1", 1)), - true, + "", }, { + "valid case 2", sdk.NewDecCoins( sdk.NewDecCoinFromDec("stake1", sdk.NewDecWithPrec(5, 1)), sdk.NewDecCoinFromDec("stake2", sdk.NewDecWithPrec(5, 1)), ), - true, + "", }, { + "invalid case 1", sdk.NewDecCoins( sdk.NewDecCoinFromDec("stake1", sdk.NewDecWithPrec(3, 1)), sdk.NewDecCoinFromDec("stake2", sdk.NewDecWithPrec(6, 1)), ), - false, + "total weight must be 1: invalid request", }, { + "invalid case 2", sdk.NewDecCoins( sdk.NewDecCoinFromDec("stake1", sdk.NewDecWithPrec(5, 1)), sdk.NewDecCoinFromDec("stake2", sdk.NewDecWithPrec(6, 1)), ), - false, + "total weight must be 1: invalid request", + }, + { + "invalid case 3", + sdk.DecCoins{ + sdk.DecCoin{Denom: "stake1", Amount: sdk.NewDec(-1)}, + }, + "invalid staking coin weights: coin -1.000000000000000000stake1 amount is not positive: invalid request", }, } { - require.Equal(t, tc.valid, types.ValidateStakingCoinTotalWeights(tc.stakingCoinWeights)) + err := types.ValidateStakingCoinTotalWeights(tc.stakingCoinWeights) + if tc.expectedErr == "" { + require.Nil(t, err) + } else { + require.Equal(t, tc.expectedErr, err.Error()) + } } } diff --git a/x/farming/types/proposal.go b/x/farming/types/proposal.go index 2ee89edd..680f682e 100644 --- a/x/farming/types/proposal.go +++ b/x/farming/types/proposal.go @@ -105,7 +105,7 @@ func NewAddRequestProposal( } func (p *AddRequestProposal) IsForFixedAmountPlan() bool { - return !p.EpochAmount.IsZero() + return !p.EpochAmount.Empty() } func (p *AddRequestProposal) IsForRatioPlan() bool { @@ -113,6 +113,9 @@ func (p *AddRequestProposal) IsForRatioPlan() bool { } func (p *AddRequestProposal) Validate() error { + if p.Name == "" { + return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "plan name must not be empty") + } if len(p.Name) > MaxNameLength { return sdkerrors.Wrapf(ErrInvalidPlanNameLength, "plan name cannot be longer than max length of %d", MaxNameLength) } @@ -122,20 +125,26 @@ func (p *AddRequestProposal) Validate() error { if _, err := sdk.AccAddressFromBech32(p.TerminationAddress); err != nil { return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid termination address %q: %v", p.TerminationAddress, err) } - if p.StakingCoinWeights.Empty() { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "staking coin weights must not be empty") - } - if err := p.StakingCoinWeights.Validate(); err != nil { - return sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid staking coin weights: %v", err) - } - if ok := ValidateStakingCoinTotalWeights(p.StakingCoinWeights); !ok { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "total weight must be 1") + if err := ValidateStakingCoinTotalWeights(p.StakingCoinWeights); err != nil { + return err } if !p.EndTime.After(p.StartTime) { return sdkerrors.Wrapf(ErrInvalidPlanEndTime, "end time %s must be greater than start time %s", p.EndTime, p.StartTime) } - if p.IsForFixedAmountPlan() == p.IsForRatioPlan() { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "only one of epoch amount or epoch ratio must be provided") + + isForFixedAmountPlan := p.IsForFixedAmountPlan() + isForRatioPlan := p.IsForRatioPlan() + switch { + case isForFixedAmountPlan == isForRatioPlan: + return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "exactly one of epoch amount or epoch ratio must be provided") + case isForFixedAmountPlan: + if err := ValidateEpochAmount(p.EpochAmount); err != nil { + return err + } + case isForRatioPlan: + if err := ValidateEpochRatio(p.EpochRatio); err != nil { + return err + } } return nil } @@ -166,7 +175,7 @@ func NewUpdateRequestProposal( } func (p *UpdateRequestProposal) IsForFixedAmountPlan() bool { - return !p.EpochAmount.IsZero() + return !p.EpochAmount.Empty() } func (p *UpdateRequestProposal) IsForRatioPlan() bool { @@ -180,28 +189,39 @@ func (p *UpdateRequestProposal) Validate() error { if len(p.Name) > MaxNameLength { return sdkerrors.Wrapf(ErrInvalidPlanNameLength, "plan name cannot be longer than max length of %d", MaxNameLength) } - if _, err := sdk.AccAddressFromBech32(p.FarmingPoolAddress); err != nil { - return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid farming pool address %q: %v", p.FarmingPoolAddress, err) - } - if _, err := sdk.AccAddressFromBech32(p.TerminationAddress); err != nil { - return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid termination address %q: %v", p.TerminationAddress, err) - } - if p.StakingCoinWeights.Empty() { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "staking coin weights must not be empty") + if p.FarmingPoolAddress != "" { + if _, err := sdk.AccAddressFromBech32(p.FarmingPoolAddress); err != nil { + return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid farming pool address %q: %v", p.FarmingPoolAddress, err) + } } - if err := p.StakingCoinWeights.Validate(); err != nil { - return sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid staking coin weights: %v", err) + if p.TerminationAddress != "" { + if _, err := sdk.AccAddressFromBech32(p.TerminationAddress); err != nil { + return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid termination address %q: %v", p.TerminationAddress, err) + } } - if ok := ValidateStakingCoinTotalWeights(p.StakingCoinWeights); !ok { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "total weight must be 1") + if p.StakingCoinWeights != nil { + if err := ValidateStakingCoinTotalWeights(p.StakingCoinWeights); err != nil { + return err + } } if p.StartTime != nil && p.EndTime != nil { if !p.EndTime.After(*p.StartTime) { return sdkerrors.Wrapf(ErrInvalidPlanEndTime, "end time %s must be greater than start time %s", p.EndTime, p.StartTime) } } - if p.IsForFixedAmountPlan() == p.IsForRatioPlan() { - return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "only one of epoch amount or epoch ratio must be provided") + isForFixedAmountPlan := p.IsForFixedAmountPlan() + isForRatioPlan := p.IsForRatioPlan() + switch { + case isForFixedAmountPlan && isForRatioPlan: + return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "at most one of epoch amount or epoch ratio must be provided") + case isForFixedAmountPlan: + if err := ValidateEpochAmount(p.EpochAmount); err != nil { + return err + } + case isForRatioPlan: + if err := ValidateEpochRatio(p.EpochRatio); err != nil { + return err + } } return nil } diff --git a/x/farming/types/proposal_test.go b/x/farming/types/proposal_test.go index 1d5d020b..08d57bb7 100644 --- a/x/farming/types/proposal_test.go +++ b/x/farming/types/proposal_test.go @@ -59,7 +59,7 @@ func TestPublicPlanProposal_ValidateBasic(t *testing.T) { "description", []*types.AddRequestProposal{ { - Name: "", + Name: "name", FarmingPoolAddress: sdk.AccAddress(crypto.AddressHash([]byte("address1"))).String(), TerminationAddress: sdk.AccAddress(crypto.AddressHash([]byte("address1"))).String(), StakingCoinWeights: sdk.NewDecCoins(sdk.NewInt64DecCoin("stake1", 1)), @@ -119,21 +119,21 @@ func TestAddRequestProposal_Validate(t *testing.T) { func(proposal *types.AddRequestProposal) { proposal.EpochRatio = sdk.NewDecWithPrec(5, 2) }, - "only one of epoch amount or epoch ratio must be provided: invalid request", + "exactly one of epoch amount or epoch ratio must be provided: invalid request", }, { "ambiguous plan type #2", func(proposal *types.AddRequestProposal) { proposal.EpochAmount = nil }, - "only one of epoch amount or epoch ratio must be provided: invalid request", + "exactly one of epoch amount or epoch ratio must be provided: invalid request", }, { "empty name", func(proposal *types.AddRequestProposal) { proposal.Name = "" }, - "", + "plan name must not be empty: invalid request", }, { "too long name", @@ -190,6 +190,36 @@ func TestAddRequestProposal_Validate(t *testing.T) { }, "end time 2021-09-01 00:00:00 +0000 UTC must be greater than start time 2021-10-01 00:00:00 +0000 UTC: invalid plan end time", }, + { + "empty epoch amount", + func(proposal *types.AddRequestProposal) { + proposal.EpochAmount = sdk.NewCoins() + }, + "exactly one of epoch amount or epoch ratio must be provided: invalid request", + }, + { + "invalid epoch amount", + func(proposal *types.AddRequestProposal) { + proposal.EpochAmount = sdk.Coins{sdk.NewInt64Coin("reward1", 0)} + }, + "invalid epoch amount: coin 0reward1 amount is not positive: invalid request", + }, + { + "zero epoch ratio", + func(proposal *types.AddRequestProposal) { + proposal.EpochAmount = nil + proposal.EpochRatio = sdk.ZeroDec() + }, + "exactly one of epoch amount or epoch ratio must be provided: invalid request", + }, + { + "too big epoch ratio", + func(proposal *types.AddRequestProposal) { + proposal.EpochAmount = nil + proposal.EpochRatio = sdk.NewDec(2) + }, + "epoch ratio must be less than 1: 2.000000000000000000: invalid request", + }, } { t.Run(tc.name, func(t *testing.T) { proposal := types.NewAddRequestProposal( @@ -233,18 +263,18 @@ func TestUpdateRequestProposal_Validate(t *testing.T) { "", }, { - "ambiguous plan type #1", + "not updating distribution info", func(proposal *types.UpdateRequestProposal) { - proposal.EpochRatio = sdk.NewDecWithPrec(5, 2) + proposal.EpochAmount = nil }, - "only one of epoch amount or epoch ratio must be provided: invalid request", + "", }, { - "ambiguous plan type #2", + "ambiguous plan type", func(proposal *types.UpdateRequestProposal) { - proposal.EpochAmount = nil + proposal.EpochRatio = sdk.NewDecWithPrec(5, 2) }, - "only one of epoch amount or epoch ratio must be provided: invalid request", + "at most one of epoch amount or epoch ratio must be provided: invalid request", }, { "invalid plan id", @@ -267,6 +297,13 @@ func TestUpdateRequestProposal_Validate(t *testing.T) { }, "plan name cannot be longer than max length of 140: invalid plan name length", }, + { + "not updating farming pool addr", + func(proposal *types.UpdateRequestProposal) { + proposal.FarmingPoolAddress = "" + }, + "", + }, { "invalid farming pool addr", func(proposal *types.UpdateRequestProposal) { @@ -274,6 +311,13 @@ func TestUpdateRequestProposal_Validate(t *testing.T) { }, "invalid farming pool address \"invalid\": decoding bech32 failed: invalid bech32 string length 7: invalid address", }, + { + "not updating termination addr", + func(proposal *types.UpdateRequestProposal) { + proposal.TerminationAddress = "" + }, + "", + }, { "invalid termination addr", func(proposal *types.UpdateRequestProposal) { @@ -282,10 +326,17 @@ func TestUpdateRequestProposal_Validate(t *testing.T) { "invalid termination address \"invalid\": decoding bech32 failed: invalid bech32 string length 7: invalid address", }, { - "invalid staking coin weights - empty", + "not updating staking coin weights", func(proposal *types.UpdateRequestProposal) { proposal.StakingCoinWeights = nil }, + "", + }, + { + "empty staking coin weights", + func(proposal *types.UpdateRequestProposal) { + proposal.StakingCoinWeights = sdk.NewDecCoins() + }, "staking coin weights must not be empty: invalid request", }, { @@ -307,6 +358,14 @@ func TestUpdateRequestProposal_Validate(t *testing.T) { }, "total weight must be 1: invalid request", }, + { + "not updating start/end time", + func(proposal *types.UpdateRequestProposal) { + proposal.StartTime = nil + proposal.EndTime = nil + }, + "", + }, { "invalid start/end time", func(proposal *types.UpdateRequestProposal) { @@ -333,6 +392,36 @@ func TestUpdateRequestProposal_Validate(t *testing.T) { }, "", }, + { + "empty epoch amount", + func(proposal *types.UpdateRequestProposal) { + proposal.EpochAmount = sdk.NewCoins() + }, + "", + }, + { + "invalid epoch amount", + func(proposal *types.UpdateRequestProposal) { + proposal.EpochAmount = sdk.Coins{sdk.NewInt64Coin("reward1", 0)} + }, + "invalid epoch amount: coin 0reward1 amount is not positive: invalid request", + }, + { + "zero epoch ratio", + func(proposal *types.UpdateRequestProposal) { + proposal.EpochAmount = nil + proposal.EpochRatio = sdk.ZeroDec() + }, + "", + }, + { + "too big epoch ratio", + func(proposal *types.UpdateRequestProposal) { + proposal.EpochAmount = nil + proposal.EpochRatio = sdk.NewDec(2) + }, + "epoch ratio must be less than 1: 2.000000000000000000: invalid request", + }, } { t.Run(tc.name, func(t *testing.T) { proposal := types.NewUpdateRequestProposal(