From c416423412e8d12d73e8d33ddeed1b8cfaa5b5b2 Mon Sep 17 00:00:00 2001 From: Kevin Davis Date: Wed, 30 Sep 2020 14:44:56 -0400 Subject: [PATCH] [R4R] harvest fixes (#673) * fix: don't allow denoms other than hard for harvest rewards * fix: parse cli flags correctly * fix: convert duration to seconds before calculating time elapsed * fix: don't distribute rewards before they start or after they end * fix: return correct message type --- x/harvest/client/cli/query.go | 16 ++++++++++++++-- x/harvest/client/cli/tx.go | 4 ++-- x/harvest/keeper/rewards.go | 11 ++++++++++- x/harvest/types/msg.go | 4 ++-- x/harvest/types/params.go | 3 +++ x/harvest/types/params_test.go | 16 ++++++++++++++++ 6 files changed, 47 insertions(+), 7 deletions(-) diff --git a/x/harvest/client/cli/query.go b/x/harvest/client/cli/query.go index 2704d6da..67645626 100644 --- a/x/harvest/client/cli/query.go +++ b/x/harvest/client/cli/query.go @@ -114,7 +114,7 @@ func queryModAccountsCmd(queryRoute string, cdc *codec.Codec) *cobra.Command { } func queryDepositsCmd(queryRoute string, cdc *codec.Codec) *cobra.Command { - return &cobra.Command{ + cmd := &cobra.Command{ Use: "deposits", Short: "query harvest module deposits with optional filters", Long: strings.TrimSpace(`query for all harvest module deposits or a specific deposit using flags: @@ -174,10 +174,16 @@ func queryDepositsCmd(queryRoute string, cdc *codec.Codec) *cobra.Command { return cliCtx.PrintOutput(deposits) }, } + cmd.Flags().Int(flags.FlagPage, 1, "pagination page to query for") + cmd.Flags().Int(flags.FlagLimit, 100, "pagination limit (max 100)") + cmd.Flags().String(flagOwner, "", "(optional) filter for deposits by owner address") + cmd.Flags().String(flagDepositDenom, "", "(optional) filter for deposits by denom") + cmd.Flags().String(flagDepositType, "", "(optional) filter for deposits by type (lp or staking)") + return cmd } func queryClaimsCmd(queryRoute string, cdc *codec.Codec) *cobra.Command { - return &cobra.Command{ + cmd := &cobra.Command{ Use: "claims", Short: "query harvest module claims with optional filters", Long: strings.TrimSpace(`query for all harvest module claims or a specific claim using flags: @@ -237,4 +243,10 @@ func queryClaimsCmd(queryRoute string, cdc *codec.Codec) *cobra.Command { return cliCtx.PrintOutput(claims) }, } + cmd.Flags().Int(flags.FlagPage, 1, "pagination page to query for") + cmd.Flags().Int(flags.FlagLimit, 100, "pagination limit (max 100)") + cmd.Flags().String(flagOwner, "", "(optional) filter for claims by owner address") + cmd.Flags().String(flagDepositDenom, "", "(optional) filter for claims by denom") + cmd.Flags().String(flagDepositType, "", "(optional) filter for claims by type (lp or staking)") + return cmd } diff --git a/x/harvest/client/cli/tx.go b/x/harvest/client/cli/tx.go index f4102891..525530b2 100644 --- a/x/harvest/client/cli/tx.go +++ b/x/harvest/client/cli/tx.go @@ -42,7 +42,7 @@ func getCmdDeposit(cdc *codec.Codec) *cobra.Command { return &cobra.Command{ Use: "deposit [amount] [deposit-type]", Short: "deposit coins to harvest", - Args: cobra.ExactArgs(3), + Args: cobra.ExactArgs(2), Example: fmt.Sprintf( `%s tx %s deposit 10000000bnb lp --from `, version.ClientName, types.ModuleName, ), @@ -67,7 +67,7 @@ func getCmdWithdraw(cdc *codec.Codec) *cobra.Command { return &cobra.Command{ Use: "withdraw [amount] [deposit-type]", Short: "withdraw coins from harvest", - Args: cobra.ExactArgs(3), + Args: cobra.ExactArgs(2), Example: fmt.Sprintf( `%s tx %s withdraw 10000000bnb lp --from `, version.ClientName, types.ModuleName, ), diff --git a/x/harvest/keeper/rewards.go b/x/harvest/keeper/rewards.go index ce350dd1..d5ed0281 100644 --- a/x/harvest/keeper/rewards.go +++ b/x/harvest/keeper/rewards.go @@ -31,6 +31,9 @@ func (k Keeper) ApplyDepositRewards(ctx sdk.Context) { if lps.End.Before(ctx.BlockTime()) { continue } + if lps.Start.After(ctx.BlockTime()) { + continue + } totalDeposited := k.GetTotalDeposited(ctx, types.LP, lps.DepositDenom) if totalDeposited.IsZero() { continue @@ -80,8 +83,11 @@ func (k Keeper) ShouldDistributeValidatorRewards(ctx sdk.Context, denom string) if denom != dds.DistributionSchedule.DepositDenom { continue } + if dds.DistributionSchedule.End.Before(ctx.BlockTime()) { + continue + } timeElapsed := sdk.NewInt(ctx.BlockTime().Unix() - previousDistributionTime.Unix()) - if timeElapsed.GTE(sdk.NewInt(int64(dds.DistributionFrequency))) { + if timeElapsed.GTE(sdk.NewInt(int64(dds.DistributionFrequency.Seconds()))) { return true } } @@ -97,6 +103,9 @@ func (k Keeper) ApplyDelegationRewards(ctx sdk.Context, denom string) { if !dds.DistributionSchedule.Active { return } + if dds.DistributionSchedule.Start.After(ctx.BlockTime()) { + return + } bondMacc := k.stakingKeeper.GetBondedPool(ctx) bondedCoinAmount := bondMacc.GetCoins().AmountOf(dds.DistributionSchedule.DepositDenom) if bondedCoinAmount.IsZero() { diff --git a/x/harvest/types/msg.go b/x/harvest/types/msg.go index a993b35a..ae0f3aba 100644 --- a/x/harvest/types/msg.go +++ b/x/harvest/types/msg.go @@ -116,8 +116,8 @@ type MsgWithdraw struct { } // NewMsgWithdraw returns a new MsgWithdraw -func NewMsgWithdraw(depositor sdk.AccAddress, amount sdk.Coin, depositType string) MsgDeposit { - return MsgDeposit{ +func NewMsgWithdraw(depositor sdk.AccAddress, amount sdk.Coin, depositType string) MsgWithdraw { + return MsgWithdraw{ Depositor: depositor, Amount: amount, DepositType: depositType, diff --git a/x/harvest/types/params.go b/x/harvest/types/params.go index 12478dc9..44cc9fb9 100644 --- a/x/harvest/types/params.go +++ b/x/harvest/types/params.go @@ -74,6 +74,9 @@ func (ds DistributionSchedule) Validate() error { if !ds.RewardsPerSecond.IsPositive() { return fmt.Errorf("reward amount must be positive, is %s for %s", ds.RewardsPerSecond, ds.DepositDenom) } + if ds.RewardsPerSecond.Denom != "hard" { + return fmt.Errorf("reward denom should be hard, is %s", ds.RewardsPerSecond.Denom) + } if ds.Start.IsZero() { return errors.New("reward period start time cannot be 0") } diff --git a/x/harvest/types/params_test.go b/x/harvest/types/params_test.go index 87852406..ce9a34e5 100644 --- a/x/harvest/types/params_test.go +++ b/x/harvest/types/params_test.go @@ -55,6 +55,22 @@ func (suite *ParamTestSuite) TestParamValidation() { expectPass: true, expectedErr: "", }, + { + name: "invalid rewards", + args: args{ + lps: types.DistributionSchedules{ + types.NewDistributionSchedule(true, "bnb", time.Date(2020, 10, 8, 14, 0, 0, 0, time.UTC), time.Date(2020, 11, 22, 14, 0, 0, 0, time.UTC), sdk.NewCoin("busd", sdk.NewInt(5000)), time.Date(2021, 11, 22, 14, 0, 0, 0, time.UTC), types.Multipliers{types.NewMultiplier(types.Small, 0, sdk.MustNewDecFromStr("0.33")), types.NewMultiplier(types.Medium, 6, sdk.MustNewDecFromStr("0.5")), types.NewMultiplier(types.Medium, 24, sdk.OneDec())}), + }, + dds: types.DelegatorDistributionSchedules{types.NewDelegatorDistributionSchedule( + types.NewDistributionSchedule(true, "bnb", time.Date(2020, 10, 8, 14, 0, 0, 0, time.UTC), time.Date(2025, 10, 8, 14, 0, 0, 0, time.UTC), sdk.NewCoin("hard", sdk.NewInt(500)), time.Date(2026, 10, 8, 14, 0, 0, 0, time.UTC), types.Multipliers{types.NewMultiplier(types.Small, 0, sdk.MustNewDecFromStr("0.33")), types.NewMultiplier(types.Medium, 6, sdk.MustNewDecFromStr("0.5")), types.NewMultiplier(types.Medium, 24, sdk.OneDec())}), + time.Hour*24, + ), + }, + active: true, + }, + expectPass: false, + expectedErr: "reward denom should be hard", + }, } for _, tc := range testCases { suite.Run(tc.name, func() {