From 79eaad66603eeba6851a70484bc90ac38d479951 Mon Sep 17 00:00:00 2001 From: Robert Pirtle Date: Thu, 13 Apr 2023 09:42:01 -0700 Subject: [PATCH] bug: fix invalid accumulation time genesis validation (#1550) * bug: increase valid accumulation time to 5 years * add changelog entry * resolve test failures * update validation to only check that value is set --- CHANGELOG.md | 4 ++++ x/incentive/genesis.go | 31 ++++++++++--------------------- x/incentive/genesis_test.go | 17 ++++++++--------- 3 files changed, 22 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a1d880c9..e66bf780 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ - [#1477] Remove legacy REST endpoints. - [#1519] Remove required denom path parameter from hard grpc query endpoints. +### Bug Fixes + +- (x/incentive) [#1550] Fix validation on genesis reward accumulation time. + ## [v0.16.1] ### State Machine Breaking diff --git a/x/incentive/genesis.go b/x/incentive/genesis.go index 6f2580ff..246ffa67 100644 --- a/x/incentive/genesis.go +++ b/x/incentive/genesis.go @@ -10,12 +10,6 @@ import ( "github.com/kava-labs/kava/x/incentive/types" ) -const year = 365 * 24 * time.Hour - -// EarliestValidAccumulationTime is how far behind the genesis time an accumulation time can be for it to be valid. -// It's a safety check to ensure rewards aren't accidentally accumulated for many years on the first block (eg since Jan 1970). -var EarliestValidAccumulationTime time.Duration = year - // InitGenesis initializes the store state from a genesis state. func InitGenesis( ctx sdk.Context, @@ -49,7 +43,7 @@ func InitGenesis( k.SetUSDXMintingClaim(ctx, claim) } for _, gat := range gs.USDXRewardState.AccumulationTimes { - if err := ValidateAccumulationTime(gat.PreviousAccumulationTime, ctx.BlockTime()); err != nil { + if err := ValidateAccumulationTime(gat.PreviousAccumulationTime); err != nil { panic(err.Error()) } k.SetPreviousUSDXMintingAccrualTime(ctx, gat.CollateralType, gat.PreviousAccumulationTime) @@ -67,7 +61,7 @@ func InitGenesis( k.SetHardLiquidityProviderClaim(ctx, claim) } for _, gat := range gs.HardSupplyRewardState.AccumulationTimes { - if err := ValidateAccumulationTime(gat.PreviousAccumulationTime, ctx.BlockTime()); err != nil { + if err := ValidateAccumulationTime(gat.PreviousAccumulationTime); err != nil { panic(err.Error()) } k.SetPreviousHardSupplyRewardAccrualTime(ctx, gat.CollateralType, gat.PreviousAccumulationTime) @@ -76,7 +70,7 @@ func InitGenesis( k.SetHardSupplyRewardIndexes(ctx, mri.CollateralType, mri.RewardIndexes) } for _, gat := range gs.HardBorrowRewardState.AccumulationTimes { - if err := ValidateAccumulationTime(gat.PreviousAccumulationTime, ctx.BlockTime()); err != nil { + if err := ValidateAccumulationTime(gat.PreviousAccumulationTime); err != nil { panic(err.Error()) } k.SetPreviousHardBorrowRewardAccrualTime(ctx, gat.CollateralType, gat.PreviousAccumulationTime) @@ -90,7 +84,7 @@ func InitGenesis( k.SetDelegatorClaim(ctx, claim) } for _, gat := range gs.DelegatorRewardState.AccumulationTimes { - if err := ValidateAccumulationTime(gat.PreviousAccumulationTime, ctx.BlockTime()); err != nil { + if err := ValidateAccumulationTime(gat.PreviousAccumulationTime); err != nil { panic(err.Error()) } k.SetPreviousDelegatorRewardAccrualTime(ctx, gat.CollateralType, gat.PreviousAccumulationTime) @@ -104,7 +98,7 @@ func InitGenesis( k.SetSwapClaim(ctx, claim) } for _, gat := range gs.SwapRewardState.AccumulationTimes { - if err := ValidateAccumulationTime(gat.PreviousAccumulationTime, ctx.BlockTime()); err != nil { + if err := ValidateAccumulationTime(gat.PreviousAccumulationTime); err != nil { panic(err.Error()) } k.SetSwapRewardAccrualTime(ctx, gat.CollateralType, gat.PreviousAccumulationTime) @@ -118,7 +112,7 @@ func InitGenesis( k.SetSavingsClaim(ctx, claim) } for _, gat := range gs.SavingsRewardState.AccumulationTimes { - if err := ValidateAccumulationTime(gat.PreviousAccumulationTime, ctx.BlockTime()); err != nil { + if err := ValidateAccumulationTime(gat.PreviousAccumulationTime); err != nil { panic(err.Error()) } k.SetSavingsRewardAccrualTime(ctx, gat.CollateralType, gat.PreviousAccumulationTime) @@ -132,7 +126,7 @@ func InitGenesis( k.SetEarnClaim(ctx, claim) } for _, gat := range gs.EarnRewardState.AccumulationTimes { - if err := ValidateAccumulationTime(gat.PreviousAccumulationTime, ctx.BlockTime()); err != nil { + if err := ValidateAccumulationTime(gat.PreviousAccumulationTime); err != nil { panic(err.Error()) } k.SetEarnRewardAccrualTime(ctx, gat.CollateralType, gat.PreviousAccumulationTime) @@ -292,14 +286,9 @@ func getEarnGenesisRewardState(ctx sdk.Context, keeper keeper.Keeper) types.Gene return types.NewGenesisRewardState(ats, mris) } -func ValidateAccumulationTime(previousAccumulationTime, genesisTime time.Time) error { - if previousAccumulationTime.Before(genesisTime.Add(-1 * EarliestValidAccumulationTime)) { - return fmt.Errorf( - "found accumulation time '%s' more than '%s' behind genesis time '%s'", - previousAccumulationTime, - EarliestValidAccumulationTime, - genesisTime, - ) +func ValidateAccumulationTime(previousAccumulationTime time.Time) error { + if previousAccumulationTime.Equal(time.Time{}) { + return fmt.Errorf("accumulation time is not set") } return nil } diff --git a/x/incentive/genesis_test.go b/x/incentive/genesis_test.go index 9508c746..83a06947 100644 --- a/x/incentive/genesis_test.go +++ b/x/incentive/genesis_test.go @@ -305,13 +305,13 @@ func (suite *GenesisTestSuite) TestExportedGenesisMatchesImported() { suite.Equal(genesisState, exportedGenesisState) } -func (suite *GenesisTestSuite) TestInitGenesisPanicsWhenAccumulationTimesToLongAgo() { +func (suite *GenesisTestSuite) TestInitGenesisPanicsWhenAccumulationTimesTooLongAgo() { genesisTime := time.Date(1998, 1, 1, 0, 0, 0, 0, time.UTC) invalidRewardState := types.NewGenesisRewardState( types.AccumulationTimes{ types.NewAccumulationTime( "bnb", - genesisTime.Add(-23*incentive.EarliestValidAccumulationTime).Add(-time.Nanosecond), + time.Time{}, ), }, types.MultiRewardIndexes{}, @@ -373,7 +373,7 @@ func (suite *GenesisTestSuite) TestInitGenesisPanicsWhenAccumulationTimesToLongA ) suite.PanicsWithValue( - "found accumulation time '1975-01-06 23:59:59.999999999 +0000 UTC' more than '8760h0m0s' behind genesis time '1998-01-01 00:00:00 +0000 UTC'", + "accumulation time is not set", func() { incentive.InitGenesis( ctx, tApp.GetIncentiveKeeper(), @@ -388,13 +388,12 @@ func (suite *GenesisTestSuite) TestInitGenesisPanicsWhenAccumulationTimesToLongA } func (suite *GenesisTestSuite) TestValidateAccumulationTime() { - genTime := time.Date(1998, 1, 1, 0, 0, 0, 0, time.UTC) + // valid when set + accTime := time.Date(1998, 1, 1, 0, 0, 0, 0, time.UTC) + suite.NoError(incentive.ValidateAccumulationTime(accTime)) - err := incentive.ValidateAccumulationTime( - genTime.Add(-incentive.EarliestValidAccumulationTime).Add(-time.Nanosecond), - genTime, - ) - suite.Error(err) + // invalid when nil value + suite.Error(incentive.ValidateAccumulationTime(time.Time{})) } func TestGenesisTestSuite(t *testing.T) {