From 97ddbafb36d2dc94a8ee46f71b38de4df78fa4db Mon Sep 17 00:00:00 2001 From: dongsam Date: Tue, 2 Nov 2021 18:59:03 +0900 Subject: [PATCH 1/8] fix: refactor collectible budgets function --- x/budget/keeper/budget.go | 19 ++++++++++--------- x/budget/keeper/budget_test.go | 2 +- x/budget/types/params.go | 2 +- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/x/budget/keeper/budget.go b/x/budget/keeper/budget.go index 1a9ade3..b1c7280 100644 --- a/x/budget/keeper/budget.go +++ b/x/budget/keeper/budget.go @@ -10,7 +10,11 @@ import ( // CollectBudgets collects all the valid budgets registered in params.Budgets and // distributes the total collected coins to collection address. func (k Keeper) CollectBudgets(ctx sdk.Context) error { - budgets := k.CollectibleBudgets(ctx) + params := k.GetParams(ctx) + var budgets []types.Budget + if params.EpochBlocks > 0 && ctx.BlockHeight()%int64(params.EpochBlocks) == 0 { + budgets = k.CollectibleBudgets(ctx, params.Budgets) + } if len(budgets) == 0 { return nil } @@ -69,14 +73,11 @@ func (k Keeper) CollectBudgets(ctx sdk.Context) error { // CollectibleBudgets returns scan through the budgets registered in params.Budgets // and returns only the valid and not expired budgets. -func (k Keeper) CollectibleBudgets(ctx sdk.Context) (budgets []types.Budget) { - params := k.GetParams(ctx) - if params.EpochBlocks > 0 && ctx.BlockHeight()%int64(params.EpochBlocks) == 0 { - for _, budget := range params.Budgets { - err := budget.Validate() - if err == nil && budget.Collectible(ctx.BlockTime()) { - budgets = append(budgets, budget) - } +func (k Keeper) CollectibleBudgets(ctx sdk.Context, budgets []types.Budget) (collectibleBudgets []types.Budget) { + for _, budget := range budgets { + err := budget.Validate() + if err == nil && budget.Collectible(ctx.BlockTime()) { + collectibleBudgets = append(collectibleBudgets, budget) } } return diff --git a/x/budget/keeper/budget_test.go b/x/budget/keeper/budget_test.go index 7563861..1394d6a 100644 --- a/x/budget/keeper/budget_test.go +++ b/x/budget/keeper/budget_test.go @@ -384,7 +384,7 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() { height += 1 suite.ctx = suite.ctx.WithBlockHeight(int64(height)) suite.ctx = suite.ctx.WithBlockTime(tc.nextBlockTime) - budgets := suite.keeper.CollectibleBudgets(suite.ctx) + budgets := suite.keeper.CollectibleBudgets(suite.ctx, params.Budgets) suite.Require().Len(budgets, tc.collectibleBudgetCount) // BeginBlocker diff --git a/x/budget/types/params.go b/x/budget/types/params.go index 14caa8e..9ed1ca2 100644 --- a/x/budget/types/params.go +++ b/x/budget/types/params.go @@ -85,7 +85,7 @@ func ValidateBudgets(i interface{}) error { } names[budget.Name] = true } - + // TODO: Verifying only the date range budgetsBySourceMap := GetBudgetsBySourceMap(budgets) for addr, budgets := range budgetsBySourceMap { if budgets.TotalRate.GT(sdk.OneDec()) { From a658eb9a8d0c3fe679e2f955504998c49fdcfd4d Mon Sep 17 00:00:00 2001 From: dongsam Date: Wed, 3 Nov 2021 17:47:17 +0900 Subject: [PATCH 2/8] fix: add overlap checking on budget validation --- x/budget/keeper/budget_test.go | 30 +++++++++++++++++++++ x/budget/types/params.go | 26 +++++++++++++----- x/budget/types/utils.go | 4 +++ x/budget/types/utils_test.go | 49 ++++++++++++++++++++++++++++++++++ 4 files changed, 103 insertions(+), 6 deletions(-) diff --git a/x/budget/keeper/budget_test.go b/x/budget/keeper/budget_test.go index 1394d6a..0b3a300 100644 --- a/x/budget/keeper/budget_test.go +++ b/x/budget/keeper/budget_test.go @@ -355,6 +355,36 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() { mustParseRFC3339("2021-10-01T00:00:00Z"), nil, }, + { + "add budget 4 without date range overlap", + testProposal(proposal.ParamChange{ + Subspace: types.ModuleName, + Key: string(types.KeyBudgets), + Value: `[ + { + "name": "gravity-dex-farming-20213Q-20313Q", + "rate": "0.500000000000000000", + "budget_source_address": "cosmos17xpfvakm2amg962yls6f84z3kell8c5lserqta", + "collection_address": "cosmos1228ryjucdpdv3t87rxle0ew76a56ulvnfst0hq0sscd3nafgjpqqkcxcky", + "start_time": "2021-09-01T00:00:00Z", + "end_time": "2031-09-30T00:00:00Z" + }, + { + "name": "gravity-dex-farming-4", + "rate": "1.000000000000000000", + "budget_source_address": "cosmos17xpfvakm2amg962yls6f84z3kell8c5lserqta", + "collection_address": "cosmos17avp6xs5c8ycqzy20yv99ccxwunu32e507kpm8ql5nfg47pzj9qqxhujxr", + "start_time": "2021-09-30T00:00:01Z", + "end_time": "2021-12-10T00:00:00Z" + } + ]`, + }), + 2, + 1, + mustParseRFC3339("2021-09-29T00:00:00Z"), + mustParseRFC3339("2021-10-01T00:00:00Z"), + nil, + }, } { suite.Run(tc.name, func() { proposalJson := paramscutils.ParamChangeProposalJSON{} diff --git a/x/budget/types/params.go b/x/budget/types/params.go index 9ed1ca2..9372b7c 100644 --- a/x/budget/types/params.go +++ b/x/budget/types/params.go @@ -85,13 +85,27 @@ func ValidateBudgets(i interface{}) error { } names[budget.Name] = true } - // TODO: Verifying only the date range budgetsBySourceMap := GetBudgetsBySourceMap(budgets) - for addr, budgets := range budgetsBySourceMap { - if budgets.TotalRate.GT(sdk.OneDec()) { - return sdkerrors.Wrapf( - ErrInvalidTotalBudgetRate, - "total rate for budget source address %s must not exceed 1: %v", addr, budgets.TotalRate) + for addr, budgetsBySource := range budgetsBySourceMap { + if budgetsBySource.TotalRate.GT(sdk.OneDec()) { + // If the TotalRate of Budgets with the same BudgetSourceAddress exceeds 1, + // recalculate and verify the TotalRate of Budgets with overlapping time ranges. + for _, budget := range budgetsBySource.Budgets { + if budgetsBySource.TotalRate.GTE(sdk.OneDec()) { + totalRate := sdk.ZeroDec() + for _, budgetToCheck := range budgetsBySource.Budgets { + if DateRageOverlap(budget.StartTime, budget.EndTime, budgetToCheck.StartTime, budgetToCheck.EndTime) { + totalRate = totalRate.Add(budgetToCheck.Rate) + } + } + if totalRate.GTE(sdk.OneDec()) { + return sdkerrors.Wrapf( + ErrInvalidTotalBudgetRate, + "total rate for budget source address %s must not exceed 1: %v", addr, totalRate) + } + } + } + } } return nil diff --git a/x/budget/types/utils.go b/x/budget/types/utils.go index f660741..9732b11 100644 --- a/x/budget/types/utils.go +++ b/x/budget/types/utils.go @@ -10,3 +10,7 @@ func ParseTime(s string) time.Time { } return t } + +func DateRageOverlap(startTimeA, endTimeA, startTimeB, endTimeB time.Time) bool { + return !startTimeA.After(endTimeB) && !endTimeA.Before(startTimeB) +} diff --git a/x/budget/types/utils_test.go b/x/budget/types/utils_test.go index b8bce23..defe338 100644 --- a/x/budget/types/utils_test.go +++ b/x/budget/types/utils_test.go @@ -18,3 +18,52 @@ func TestParseTime(t *testing.T) { require.PanicsWithError(t, err.Error(), func() { types.ParseTime(errorCase) }) require.Equal(t, normalRes, types.ParseTime(normalCase)) } + +func TestDateRageOverlap(t *testing.T) { + testCases := []struct { + name string + expectedResult bool + startTimeA time.Time + endTimeA time.Time + startTimeB time.Time + endTimeB time.Time + }{ + { + "same range", + true, + types.ParseTime("2021-12-31T00:00:00Z"), + types.ParseTime("2021-12-31T00:00:00Z"), + types.ParseTime("2021-12-31T00:00:00Z"), + types.ParseTime("2021-12-31T00:00:00Z"), + }, + { + "overlap with start", + true, + types.ParseTime("2021-10-05T00:00:00Z"), + types.ParseTime("2021-12-31T00:00:00Z"), + types.ParseTime("2021-10-05T00:00:00Z"), + types.ParseTime("2021-11-10T00:00:00Z"), + }, + { + "overlap with start 2", + true, + types.ParseTime("2021-10-05T00:00:00Z"), + types.ParseTime("2021-11-10T00:00:00Z"), + types.ParseTime("2021-10-05T00:00:00Z"), + types.ParseTime("2021-12-31T00:00:00Z"), + }, + { + "overlap 1 sec", + true, + types.ParseTime("2021-10-05T00:00:00Z"), + types.ParseTime("2021-11-10T00:00:01Z"), + types.ParseTime("2021-11-10T00:00:00Z"), + types.ParseTime("2021-12-31T00:00:00Z"), + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.expectedResult, types.DateRageOverlap(tc.startTimeA, tc.endTimeA, tc.startTimeB, tc.endTimeB)) + }) + } +} From c7e009cbba68b7863c898bc1726e6b21253ebb7f Mon Sep 17 00:00:00 2001 From: dongsam Date: Wed, 3 Nov 2021 19:35:53 +0900 Subject: [PATCH 3/8] fix: update validation logic and refactor mustParseRFC3339 and add testcase --- x/budget/keeper/budget_test.go | 37 +++++++++++++++--------------- x/budget/keeper/grpc_query_test.go | 18 +++++++-------- x/budget/keeper/keeper_test.go | 37 +++++++++++------------------- x/budget/simulation/genesis.go | 4 ++-- x/budget/types/params.go | 20 ++++++++-------- x/budget/types/params_test.go | 26 +++++++++++++++++++-- x/budget/types/utils.go | 8 ++++--- x/budget/types/utils_test.go | 36 ++++++++++++++--------------- 8 files changed, 99 insertions(+), 87 deletions(-) diff --git a/x/budget/keeper/budget_test.go b/x/budget/keeper/budget_test.go index 0b3a300..74ecda7 100644 --- a/x/budget/keeper/budget_test.go +++ b/x/budget/keeper/budget_test.go @@ -184,7 +184,7 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() { params := suite.keeper.GetParams(suite.ctx) suite.keeper.SetParams(suite.ctx, params) height := 1 - suite.ctx = suite.ctx.WithBlockTime(mustParseRFC3339("2021-08-01T00:00:00Z")) + suite.ctx = suite.ctx.WithBlockTime(types.MustParseRFC3339("2021-08-01T00:00:00Z")) suite.ctx = suite.ctx.WithBlockHeight(int64(height)) for _, tc := range []struct { @@ -196,7 +196,6 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() { nextBlockTime time.Time expErr error }{ - { "add budget 1", testProposal(proposal.ParamChange{ @@ -215,8 +214,8 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() { }), 1, 0, - mustParseRFC3339("2021-08-01T00:00:00Z"), - mustParseRFC3339("2021-08-01T00:00:00Z"), + types.MustParseRFC3339("2021-08-01T00:00:00Z"), + types.MustParseRFC3339("2021-08-01T00:00:00Z"), nil, }, { @@ -245,8 +244,8 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() { }), 2, 2, - mustParseRFC3339("2021-09-03T00:00:00Z"), - mustParseRFC3339("2021-09-03T00:00:00Z"), + types.MustParseRFC3339("2021-09-03T00:00:00Z"), + types.MustParseRFC3339("2021-09-03T00:00:00Z"), nil, }, { @@ -283,8 +282,8 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() { }), 0, 0, - mustParseRFC3339("2021-09-29T00:00:00Z"), - mustParseRFC3339("2021-09-30T00:00:00Z"), + types.MustParseRFC3339("2021-09-29T00:00:00Z"), + types.MustParseRFC3339("2021-09-30T00:00:00Z"), types.ErrInvalidTotalBudgetRate, }, { @@ -321,8 +320,8 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() { }), 0, 0, - mustParseRFC3339("2021-10-01T00:00:00Z"), - mustParseRFC3339("2021-10-01T00:00:00Z"), + types.MustParseRFC3339("2021-10-01T00:00:00Z"), + types.MustParseRFC3339("2021-10-01T00:00:00Z"), types.ErrInvalidTotalBudgetRate, }, { @@ -351,8 +350,8 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() { }), 2, 2, - mustParseRFC3339("2021-10-01T00:00:00Z"), - mustParseRFC3339("2021-10-01T00:00:00Z"), + types.MustParseRFC3339("2021-10-01T00:00:00Z"), + types.MustParseRFC3339("2021-10-01T00:00:00Z"), nil, }, { @@ -374,15 +373,15 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() { "rate": "1.000000000000000000", "budget_source_address": "cosmos17xpfvakm2amg962yls6f84z3kell8c5lserqta", "collection_address": "cosmos17avp6xs5c8ycqzy20yv99ccxwunu32e507kpm8ql5nfg47pzj9qqxhujxr", - "start_time": "2021-09-30T00:00:01Z", - "end_time": "2021-12-10T00:00:00Z" + "start_time": "2031-09-30T00:00:01Z", + "end_time": "2031-12-10T00:00:00Z" } ]`, }), 2, 1, - mustParseRFC3339("2021-09-29T00:00:00Z"), - mustParseRFC3339("2021-10-01T00:00:00Z"), + types.MustParseRFC3339("2021-09-29T00:00:00Z"), + types.MustParseRFC3339("2021-09-30T00:00:00Z"), nil, }, } { @@ -448,8 +447,8 @@ func (suite *KeeperTestSuite) TestTotalCollectedCoins() { Rate: sdk.NewDecWithPrec(5, 2), // 5% BudgetSourceAddress: suite.budgetSourceAddrs[0].String(), CollectionAddress: suite.collectionAddrs[0].String(), - StartTime: mustParseRFC3339("0000-01-01T00:00:00Z"), - EndTime: mustParseRFC3339("9999-12-31T00:00:00Z"), + StartTime: types.MustParseRFC3339("0000-01-01T00:00:00Z"), + EndTime: types.MustParseRFC3339("9999-12-31T00:00:00Z"), } params := suite.keeper.GetParams(suite.ctx) @@ -462,7 +461,7 @@ func (suite *KeeperTestSuite) TestTotalCollectedCoins() { collectedCoins := suite.keeper.GetTotalCollectedCoins(suite.ctx, "budget1") suite.Require().Equal(sdk.Coins(nil), collectedCoins) - suite.ctx = suite.ctx.WithBlockTime(mustParseRFC3339("2021-08-31T00:00:00Z")) + suite.ctx = suite.ctx.WithBlockTime(types.MustParseRFC3339("2021-08-31T00:00:00Z")) err := suite.keeper.CollectBudgets(suite.ctx) suite.Require().NoError(err) diff --git a/x/budget/keeper/grpc_query_test.go b/x/budget/keeper/grpc_query_test.go index 68c2cc3..5fcd39a 100644 --- a/x/budget/keeper/grpc_query_test.go +++ b/x/budget/keeper/grpc_query_test.go @@ -21,32 +21,32 @@ func (suite *KeeperTestSuite) TestGRPCBudgets() { Rate: sdk.NewDecWithPrec(5, 2), BudgetSourceAddress: suite.budgetSourceAddrs[0].String(), CollectionAddress: suite.collectionAddrs[0].String(), - StartTime: mustParseRFC3339("0000-01-01T00:00:00Z"), - EndTime: mustParseRFC3339("9999-12-31T00:00:00Z"), + StartTime: types.MustParseRFC3339("0000-01-01T00:00:00Z"), + EndTime: types.MustParseRFC3339("9999-12-31T00:00:00Z"), }, { Name: "budget2", Rate: sdk.NewDecWithPrec(5, 2), BudgetSourceAddress: suite.budgetSourceAddrs[0].String(), CollectionAddress: suite.collectionAddrs[1].String(), - StartTime: mustParseRFC3339("0000-01-01T00:00:00Z"), - EndTime: mustParseRFC3339("9999-12-31T00:00:00Z"), + StartTime: types.MustParseRFC3339("0000-01-01T00:00:00Z"), + EndTime: types.MustParseRFC3339("9999-12-31T00:00:00Z"), }, { Name: "budget3", Rate: sdk.NewDecWithPrec(5, 2), BudgetSourceAddress: suite.budgetSourceAddrs[1].String(), CollectionAddress: suite.collectionAddrs[0].String(), - StartTime: mustParseRFC3339("0000-01-01T00:00:00Z"), - EndTime: mustParseRFC3339("9999-12-31T00:00:00Z"), + StartTime: types.MustParseRFC3339("0000-01-01T00:00:00Z"), + EndTime: types.MustParseRFC3339("9999-12-31T00:00:00Z"), }, { Name: "budget4", Rate: sdk.NewDecWithPrec(5, 2), BudgetSourceAddress: suite.budgetSourceAddrs[1].String(), CollectionAddress: suite.collectionAddrs[1].String(), - StartTime: mustParseRFC3339("0000-01-01T00:00:00Z"), - EndTime: mustParseRFC3339("9999-12-31T00:00:00Z"), + StartTime: types.MustParseRFC3339("0000-01-01T00:00:00Z"), + EndTime: types.MustParseRFC3339("9999-12-31T00:00:00Z"), }, } @@ -57,7 +57,7 @@ func (suite *KeeperTestSuite) TestGRPCBudgets() { balance := suite.app.BankKeeper.GetAllBalances(suite.ctx, suite.budgetSourceAddrs[0]) expectedCoins, _ := sdk.NewDecCoinsFromCoins(balance...).MulDec(sdk.NewDecWithPrec(5, 2)).TruncateDecimal() - suite.ctx = suite.ctx.WithBlockTime(mustParseRFC3339("2021-08-31T00:00:00Z")) + suite.ctx = suite.ctx.WithBlockTime(types.MustParseRFC3339("2021-08-31T00:00:00Z")) err := suite.keeper.CollectBudgets(suite.ctx) suite.Require().NoError(err) diff --git a/x/budget/keeper/keeper_test.go b/x/budget/keeper/keeper_test.go index 32a9dea..95848e2 100644 --- a/x/budget/keeper/keeper_test.go +++ b/x/budget/keeper/keeper_test.go @@ -2,7 +2,6 @@ package keeper_test import ( "testing" - "time" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" @@ -91,56 +90,56 @@ func (suite *KeeperTestSuite) SetupTest() { Rate: sdk.MustNewDecFromStr("0.5"), BudgetSourceAddress: suite.budgetSourceAddrs[0].String(), CollectionAddress: suite.collectionAddrs[0].String(), - StartTime: mustParseRFC3339("0000-01-01T00:00:00Z"), - EndTime: mustParseRFC3339("9999-12-31T00:00:00Z"), + StartTime: types.MustParseRFC3339("0000-01-01T00:00:00Z"), + EndTime: types.MustParseRFC3339("9999-12-31T00:00:00Z"), }, { Name: "budget2", Rate: sdk.MustNewDecFromStr("0.5"), BudgetSourceAddress: suite.budgetSourceAddrs[0].String(), CollectionAddress: suite.collectionAddrs[1].String(), - StartTime: mustParseRFC3339("0000-01-01T00:00:00Z"), - EndTime: mustParseRFC3339("9999-12-31T00:00:00Z"), + StartTime: types.MustParseRFC3339("0000-01-01T00:00:00Z"), + EndTime: types.MustParseRFC3339("9999-12-31T00:00:00Z"), }, { Name: "budget3", Rate: sdk.MustNewDecFromStr("1.0"), BudgetSourceAddress: suite.budgetSourceAddrs[1].String(), CollectionAddress: suite.collectionAddrs[2].String(), - StartTime: mustParseRFC3339("0000-01-01T00:00:00Z"), - EndTime: mustParseRFC3339("9999-12-31T00:00:00Z"), + StartTime: types.MustParseRFC3339("0000-01-01T00:00:00Z"), + EndTime: types.MustParseRFC3339("9999-12-31T00:00:00Z"), }, { Name: "budget4", Rate: sdk.MustNewDecFromStr("1"), BudgetSourceAddress: suite.budgetSourceAddrs[2].String(), CollectionAddress: suite.collectionAddrs[3].String(), - StartTime: mustParseRFC3339("0000-01-01T00:00:00Z"), - EndTime: mustParseRFC3339("0000-01-01T00:00:00Z"), + StartTime: types.MustParseRFC3339("0000-01-01T00:00:00Z"), + EndTime: types.MustParseRFC3339("0000-01-01T00:00:00Z"), }, { Name: "budget5", Rate: sdk.MustNewDecFromStr("0.5"), BudgetSourceAddress: suite.budgetSourceAddrs[3].String(), CollectionAddress: suite.collectionAddrs[0].String(), - StartTime: mustParseRFC3339("0000-01-01T00:00:00Z"), - EndTime: mustParseRFC3339("9999-12-31T00:00:00Z"), + StartTime: types.MustParseRFC3339("0000-01-01T00:00:00Z"), + EndTime: types.MustParseRFC3339("9999-12-31T00:00:00Z"), }, { Name: "budget6", Rate: sdk.MustNewDecFromStr("0.5"), BudgetSourceAddress: suite.budgetSourceAddrs[3].String(), CollectionAddress: suite.collectionAddrs[1].String(), - StartTime: mustParseRFC3339("0000-01-01T00:00:00Z"), - EndTime: mustParseRFC3339("9999-12-31T00:00:00Z"), + StartTime: types.MustParseRFC3339("0000-01-01T00:00:00Z"), + EndTime: types.MustParseRFC3339("9999-12-31T00:00:00Z"), }, { Name: "gravity-dex-farming-20213Q-20313Q", Rate: sdk.MustNewDecFromStr("0.5"), BudgetSourceAddress: suite.budgetSourceAddrs[5].String(), CollectionAddress: suite.collectionAddrs[5].String(), - StartTime: mustParseRFC3339("2021-09-01T00:00:00Z"), - EndTime: mustParseRFC3339("2031-09-30T00:00:00Z"), + StartTime: types.MustParseRFC3339("2021-09-01T00:00:00Z"), + EndTime: types.MustParseRFC3339("2031-09-30T00:00:00Z"), }, } } @@ -149,14 +148,6 @@ func coinsEq(exp, got sdk.Coins) (bool, string, string, string) { return exp.IsEqual(got), "expected:\t%v\ngot:\t\t%v", exp.String(), got.String() } -func mustParseRFC3339(s string) time.Time { - t, err := time.Parse(time.RFC3339, s) - if err != nil { - panic(err) - } - return t -} - func mustParseCoinsNormalized(coinStr string) (coins sdk.Coins) { coins, err := sdk.ParseCoinsNormalized(coinStr) if err != nil { diff --git a/x/budget/simulation/genesis.go b/x/budget/simulation/genesis.go index 5317f87..a6d2a33 100644 --- a/x/budget/simulation/genesis.go +++ b/x/budget/simulation/genesis.go @@ -36,8 +36,8 @@ func GenBudgets(r *rand.Rand) []types.Budget { Rate: sdk.NewDecFromIntWithPrec(sdk.NewInt(int64(simtypes.RandIntBetween(r, 1, 4))), 1), // 10~30% BudgetSourceAddress: "cosmos17xpfvakm2amg962yls6f84z3kell8c5lserqta", // Cosmos Hub's FeeCollector module account CollectionAddress: sdk.AccAddress(address.Module(types.ModuleName, []byte("GravityDEXFarmingBudget"))).String(), - StartTime: types.ParseTime("2000-01-01T00:00:00Z"), - EndTime: types.ParseTime("9999-12-31T00:00:00Z"), + StartTime: types.MustParseRFC3339("2000-01-01T00:00:00Z"), + EndTime: types.MustParseRFC3339("9999-12-31T00:00:00Z"), } ranBudgets = append(ranBudgets, budget) } diff --git a/x/budget/types/params.go b/x/budget/types/params.go index 9372b7c..d0a887b 100644 --- a/x/budget/types/params.go +++ b/x/budget/types/params.go @@ -91,19 +91,17 @@ func ValidateBudgets(i interface{}) error { // If the TotalRate of Budgets with the same BudgetSourceAddress exceeds 1, // recalculate and verify the TotalRate of Budgets with overlapping time ranges. for _, budget := range budgetsBySource.Budgets { - if budgetsBySource.TotalRate.GTE(sdk.OneDec()) { - totalRate := sdk.ZeroDec() - for _, budgetToCheck := range budgetsBySource.Budgets { - if DateRageOverlap(budget.StartTime, budget.EndTime, budgetToCheck.StartTime, budgetToCheck.EndTime) { - totalRate = totalRate.Add(budgetToCheck.Rate) - } - } - if totalRate.GTE(sdk.OneDec()) { - return sdkerrors.Wrapf( - ErrInvalidTotalBudgetRate, - "total rate for budget source address %s must not exceed 1: %v", addr, totalRate) + totalRate := sdk.ZeroDec() + for _, budgetToCheck := range budgetsBySource.Budgets { + if DateRageOverlap(budget.StartTime, budget.EndTime, budgetToCheck.StartTime, budgetToCheck.EndTime) { + totalRate = totalRate.Add(budgetToCheck.Rate) } } + if totalRate.GT(sdk.OneDec()) { + return sdkerrors.Wrapf( + ErrInvalidTotalBudgetRate, + "total rate for budget source address %s must not exceed 1: %v", addr, totalRate) + } } } diff --git a/x/budget/types/params_test.go b/x/budget/types/params_test.go index 9505101..cc4960b 100644 --- a/x/budget/types/params_test.go +++ b/x/budget/types/params_test.go @@ -31,7 +31,7 @@ func TestValidateBudgets(t *testing.T) { budgets := []types.Budget{ { Name: "test", - Rate: sdk.NewDec(1), + Rate: sdk.OneDec(), BudgetSourceAddress: tAddr1.String(), CollectionAddress: cAddr1.String(), StartTime: time.Time{}, @@ -39,7 +39,7 @@ func TestValidateBudgets(t *testing.T) { }, { Name: "test2", - Rate: sdk.NewDec(1), + Rate: sdk.OneDec(), BudgetSourceAddress: tAddr2.String(), CollectionAddress: cAddr2.String(), StartTime: time.Time{}, @@ -61,6 +61,22 @@ func TestValidateBudgets(t *testing.T) { StartTime: time.Time{}, EndTime: time.Time{}, }, + { + Name: "test4", + Rate: sdk.OneDec(), + BudgetSourceAddress: tAddr2.String(), + CollectionAddress: cAddr2.String(), + StartTime: types.MustParseRFC3339("2021-08-01T00:00:00Z"), + EndTime: types.MustParseRFC3339("2021-08-20T00:00:00Z"), + }, + { + Name: "test5", + Rate: sdk.MustNewDecFromStr("0.1"), + BudgetSourceAddress: tAddr2.String(), + CollectionAddress: cAddr2.String(), + StartTime: types.MustParseRFC3339("2021-08-19T00:00:00Z"), + EndTime: types.MustParseRFC3339("2021-08-25T00:00:00Z"), + }, } err := types.ValidateBudgets(budgets[:2]) @@ -69,6 +85,12 @@ func TestValidateBudgets(t *testing.T) { err = types.ValidateBudgets(budgets[:3]) require.ErrorIs(t, err, types.ErrInvalidTotalBudgetRate) + err = types.ValidateBudgets(budgets[3:5]) + require.NoError(t, err) + + err = types.ValidateBudgets(budgets[4:6]) + require.ErrorIs(t, err, types.ErrInvalidTotalBudgetRate) + err = types.ValidateBudgets(budgets) require.ErrorIs(t, err, types.ErrDuplicateBudgetName) } diff --git a/x/budget/types/utils.go b/x/budget/types/utils.go index 9732b11..4fd907a 100644 --- a/x/budget/types/utils.go +++ b/x/budget/types/utils.go @@ -1,9 +1,11 @@ package types -import "time" +import ( + "time" +) -// ParseTime parses string time to time in RFC3339 format. -func ParseTime(s string) time.Time { +// MustParseRFC3339 parses string time to time in RFC3339 format. +func MustParseRFC3339(s string) time.Time { t, err := time.Parse(time.RFC3339, s) if err != nil { panic(err) diff --git a/x/budget/types/utils_test.go b/x/budget/types/utils_test.go index defe338..a0c6a74 100644 --- a/x/budget/types/utils_test.go +++ b/x/budget/types/utils_test.go @@ -15,8 +15,8 @@ func TestParseTime(t *testing.T) { require.NoError(t, err) errorCase := "9999-12-31T00:00:00_ErrorCase" _, err = time.Parse(time.RFC3339, errorCase) - require.PanicsWithError(t, err.Error(), func() { types.ParseTime(errorCase) }) - require.Equal(t, normalRes, types.ParseTime(normalCase)) + require.PanicsWithError(t, err.Error(), func() { types.MustParseRFC3339(errorCase) }) + require.Equal(t, normalRes, types.MustParseRFC3339(normalCase)) } func TestDateRageOverlap(t *testing.T) { @@ -31,34 +31,34 @@ func TestDateRageOverlap(t *testing.T) { { "same range", true, - types.ParseTime("2021-12-31T00:00:00Z"), - types.ParseTime("2021-12-31T00:00:00Z"), - types.ParseTime("2021-12-31T00:00:00Z"), - types.ParseTime("2021-12-31T00:00:00Z"), + types.MustParseRFC3339("2021-12-31T00:00:00Z"), + types.MustParseRFC3339("2021-12-31T00:00:00Z"), + types.MustParseRFC3339("2021-12-31T00:00:00Z"), + types.MustParseRFC3339("2021-12-31T00:00:00Z"), }, { "overlap with start", true, - types.ParseTime("2021-10-05T00:00:00Z"), - types.ParseTime("2021-12-31T00:00:00Z"), - types.ParseTime("2021-10-05T00:00:00Z"), - types.ParseTime("2021-11-10T00:00:00Z"), + types.MustParseRFC3339("2021-10-05T00:00:00Z"), + types.MustParseRFC3339("2021-12-31T00:00:00Z"), + types.MustParseRFC3339("2021-10-05T00:00:00Z"), + types.MustParseRFC3339("2021-11-10T00:00:00Z"), }, { "overlap with start 2", true, - types.ParseTime("2021-10-05T00:00:00Z"), - types.ParseTime("2021-11-10T00:00:00Z"), - types.ParseTime("2021-10-05T00:00:00Z"), - types.ParseTime("2021-12-31T00:00:00Z"), + types.MustParseRFC3339("2021-10-05T00:00:00Z"), + types.MustParseRFC3339("2021-11-10T00:00:00Z"), + types.MustParseRFC3339("2021-10-05T00:00:00Z"), + types.MustParseRFC3339("2021-12-31T00:00:00Z"), }, { "overlap 1 sec", true, - types.ParseTime("2021-10-05T00:00:00Z"), - types.ParseTime("2021-11-10T00:00:01Z"), - types.ParseTime("2021-11-10T00:00:00Z"), - types.ParseTime("2021-12-31T00:00:00Z"), + types.MustParseRFC3339("2021-10-05T00:00:00Z"), + types.MustParseRFC3339("2021-11-10T00:00:01Z"), + types.MustParseRFC3339("2021-11-10T00:00:00Z"), + types.MustParseRFC3339("2021-12-31T00:00:00Z"), }, } for _, tc := range testCases { From 2fe6c6a567e9337dc8ffed5d495302fb50317ce1 Mon Sep 17 00:00:00 2001 From: dongsam Date: Fri, 5 Nov 2021 13:57:24 +0900 Subject: [PATCH 4/8] fix: refactor CollectibleBudgets and add test cases --- x/budget/keeper/budget.go | 14 +----- x/budget/keeper/budget_test.go | 2 +- x/budget/types/budget.go | 11 +++++ x/budget/types/params_test.go | 79 +++++++++++++++++++++------------- 4 files changed, 62 insertions(+), 44 deletions(-) diff --git a/x/budget/keeper/budget.go b/x/budget/keeper/budget.go index b1c7280..2f02602 100644 --- a/x/budget/keeper/budget.go +++ b/x/budget/keeper/budget.go @@ -13,7 +13,7 @@ func (k Keeper) CollectBudgets(ctx sdk.Context) error { params := k.GetParams(ctx) var budgets []types.Budget if params.EpochBlocks > 0 && ctx.BlockHeight()%int64(params.EpochBlocks) == 0 { - budgets = k.CollectibleBudgets(ctx, params.Budgets) + budgets = types.CollectibleBudgets(params.Budgets, ctx.BlockTime()) } if len(budgets) == 0 { return nil @@ -71,18 +71,6 @@ func (k Keeper) CollectBudgets(ctx sdk.Context) error { return nil } -// CollectibleBudgets returns scan through the budgets registered in params.Budgets -// and returns only the valid and not expired budgets. -func (k Keeper) CollectibleBudgets(ctx sdk.Context, budgets []types.Budget) (collectibleBudgets []types.Budget) { - for _, budget := range budgets { - err := budget.Validate() - if err == nil && budget.Collectible(ctx.BlockTime()) { - collectibleBudgets = append(collectibleBudgets, budget) - } - } - return -} - // GetTotalCollectedCoins returns total collected coins for a budget. func (k Keeper) GetTotalCollectedCoins(ctx sdk.Context, budgetName string) sdk.Coins { store := ctx.KVStore(k.storeKey) diff --git a/x/budget/keeper/budget_test.go b/x/budget/keeper/budget_test.go index 74ecda7..8ce4451 100644 --- a/x/budget/keeper/budget_test.go +++ b/x/budget/keeper/budget_test.go @@ -413,7 +413,7 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() { height += 1 suite.ctx = suite.ctx.WithBlockHeight(int64(height)) suite.ctx = suite.ctx.WithBlockTime(tc.nextBlockTime) - budgets := suite.keeper.CollectibleBudgets(suite.ctx, params.Budgets) + budgets := suite.keeper.CollectibleBudgets(params.Budgets, suite.ctx.BlockTime()) suite.Require().Len(budgets, tc.collectibleBudgetCount) // BeginBlocker diff --git a/x/budget/types/budget.go b/x/budget/types/budget.go index f22ddf8..dd0400f 100644 --- a/x/budget/types/budget.go +++ b/x/budget/types/budget.go @@ -63,6 +63,17 @@ func (budget Budget) Collectible(blockTime time.Time) bool { return !budget.StartTime.After(blockTime) && budget.EndTime.After(blockTime) } +// CollectibleBudgets returns only the valid and started and not expired budgets based on the given block time. +func CollectibleBudgets(budgets []Budget, blockTime time.Time) (collectibleBudgets []Budget) { + for _, budget := range budgets { + err := budget.Validate() + if err == nil && budget.Collectible(blockTime) { + collectibleBudgets = append(collectibleBudgets, budget) + } + } + return +} + // ValidateName is the default validation function for Budget.Name. // A budget name only allows alphabet letters(`A-Z, a-z`), digit numbers(`0-9`), and `-`. // It doesn't allow spaces and the maximum length is 50 characters. diff --git a/x/budget/types/params_test.go b/x/budget/types/params_test.go index cc4960b..421f3ba 100644 --- a/x/budget/types/params_test.go +++ b/x/budget/types/params_test.go @@ -12,23 +12,12 @@ import ( "github.com/tendermint/budget/x/budget/types" ) -func TestParams(t *testing.T) { - require.IsType(t, paramstypes.KeyTable{}, types.ParamKeyTable()) - - defaultParams := types.DefaultParams() - - paramsStr := `epoch_blocks: 1 -budgets: [] -` - require.Equal(t, paramsStr, defaultParams.String()) -} - -func TestValidateBudgets(t *testing.T) { - cAddr1 := sdk.AccAddress(address.Module(types.ModuleName, []byte("collectionAddr1"))) - cAddr2 := sdk.AccAddress(address.Module(types.ModuleName, []byte("collectionAddr2"))) - tAddr1 := sdk.AccAddress(address.Module(types.ModuleName, []byte("budgetSourceAddr1"))) - tAddr2 := sdk.AccAddress(address.Module(types.ModuleName, []byte("budgetSourceAddr2"))) - budgets := []types.Budget{ +var ( + cAddr1 = sdk.AccAddress(address.Module(types.ModuleName, []byte("collectionAddr1"))) + cAddr2 = sdk.AccAddress(address.Module(types.ModuleName, []byte("collectionAddr2"))) + tAddr1 = sdk.AccAddress(address.Module(types.ModuleName, []byte("budgetSourceAddr1"))) + tAddr2 = sdk.AccAddress(address.Module(types.ModuleName, []byte("budgetSourceAddr2"))) + budgets = []types.Budget{ { Name: "test", Rate: sdk.OneDec(), @@ -38,28 +27,28 @@ func TestValidateBudgets(t *testing.T) { EndTime: time.Time{}, }, { - Name: "test2", + Name: "test1", Rate: sdk.OneDec(), BudgetSourceAddress: tAddr2.String(), CollectionAddress: cAddr2.String(), - StartTime: time.Time{}, - EndTime: time.Time{}, + StartTime: types.MustParseRFC3339("2021-07-01T00:00:00Z"), + EndTime: types.MustParseRFC3339("2021-07-10T00:00:00Z"), }, { - Name: "test3", + Name: "test2", Rate: sdk.MustNewDecFromStr("0.1"), BudgetSourceAddress: tAddr2.String(), CollectionAddress: cAddr2.String(), - StartTime: time.Time{}, - EndTime: time.Time{}, + StartTime: types.MustParseRFC3339("2021-07-01T00:00:00Z"), + EndTime: types.MustParseRFC3339("2021-07-10T00:00:00Z"), }, { Name: "test3", Rate: sdk.MustNewDecFromStr("0.1"), BudgetSourceAddress: tAddr2.String(), CollectionAddress: cAddr2.String(), - StartTime: time.Time{}, - EndTime: time.Time{}, + StartTime: types.MustParseRFC3339("2021-08-01T00:00:00Z"), + EndTime: types.MustParseRFC3339("2021-08-10T00:00:00Z"), }, { Name: "test4", @@ -78,23 +67,53 @@ func TestValidateBudgets(t *testing.T) { EndTime: types.MustParseRFC3339("2021-08-25T00:00:00Z"), }, } +) + +func TestParams(t *testing.T) { + require.IsType(t, paramstypes.KeyTable{}, types.ParamKeyTable()) + + defaultParams := types.DefaultParams() + + paramsStr := `epoch_blocks: 1 +budgets: [] +` + require.Equal(t, paramsStr, defaultParams.String()) +} - err := types.ValidateBudgets(budgets[:2]) +func TestValidateBudgets(t *testing.T) { + err := types.ValidateBudgets([]types.Budget{budgets[0], budgets[1]}) require.NoError(t, err) - err = types.ValidateBudgets(budgets[:3]) + err = types.ValidateBudgets([]types.Budget{budgets[0], budgets[1], budgets[2]}) require.ErrorIs(t, err, types.ErrInvalidTotalBudgetRate) - err = types.ValidateBudgets(budgets[3:5]) + err = types.ValidateBudgets([]types.Budget{budgets[1], budgets[4]}) require.NoError(t, err) - err = types.ValidateBudgets(budgets[4:6]) + err = types.ValidateBudgets([]types.Budget{budgets[4], budgets[5]}) require.ErrorIs(t, err, types.ErrInvalidTotalBudgetRate) - err = types.ValidateBudgets(budgets) + err = types.ValidateBudgets([]types.Budget{budgets[3], budgets[3]}) require.ErrorIs(t, err, types.ErrDuplicateBudgetName) } +func TestCollectibleBudgets(t *testing.T) { + collectibleBudgets := types.CollectibleBudgets([]types.Budget{budgets[0], budgets[1]}, types.MustParseRFC3339("2021-07-05T00:00:00Z")) + require.Len(t, collectibleBudgets, 1) + + collectibleBudgets = types.CollectibleBudgets([]types.Budget{budgets[0], budgets[1], budgets[2]}, types.MustParseRFC3339("2021-07-05T00:00:00Z")) + require.Len(t, collectibleBudgets, 2) + + collectibleBudgets = types.CollectibleBudgets([]types.Budget{budgets[4], budgets[5]}, types.MustParseRFC3339("2021-08-18T00:00:00Z")) + require.Len(t, collectibleBudgets, 1) + + collectibleBudgets = types.CollectibleBudgets([]types.Budget{budgets[4], budgets[5]}, types.MustParseRFC3339("2021-08-19T00:00:00Z")) + require.Len(t, collectibleBudgets, 2) + + collectibleBudgets = types.CollectibleBudgets([]types.Budget{budgets[4], budgets[5]}, types.MustParseRFC3339("2021-08-20T00:00:00Z")) + require.Len(t, collectibleBudgets, 1) +} + func TestValidateEpochBlocks(t *testing.T) { err := types.ValidateEpochBlocks(uint32(0)) require.NoError(t, err) From ecabb85fd158a77e0c549e1469738caaeed45161 Mon Sep 17 00:00:00 2001 From: dongsam Date: Fri, 5 Nov 2021 13:59:22 +0900 Subject: [PATCH 5/8] chore: refactor missed part --- x/budget/keeper/budget_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/budget/keeper/budget_test.go b/x/budget/keeper/budget_test.go index 8ce4451..ddd741f 100644 --- a/x/budget/keeper/budget_test.go +++ b/x/budget/keeper/budget_test.go @@ -413,7 +413,7 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() { height += 1 suite.ctx = suite.ctx.WithBlockHeight(int64(height)) suite.ctx = suite.ctx.WithBlockTime(tc.nextBlockTime) - budgets := suite.keeper.CollectibleBudgets(params.Budgets, suite.ctx.BlockTime()) + budgets := types.CollectibleBudgets(params.Budgets, suite.ctx.BlockTime()) suite.Require().Len(budgets, tc.collectibleBudgetCount) // BeginBlocker From bbc760e62488b14be2b2876ce4bd0495cb90147f Mon Sep 17 00:00:00 2001 From: Hanjun Kim Date: Fri, 5 Nov 2021 15:45:26 +0900 Subject: [PATCH 6/8] test: add more tests for `DateRangesOverlap` also fix typo `DateRageOverlap` -> `DateRangesOverlap` --- x/budget/types/params.go | 2 +- x/budget/types/params_test.go | 17 +++++----- x/budget/types/utils.go | 6 ++-- x/budget/types/utils_test.go | 61 ++++++++++++++++++++++------------- 4 files changed, 52 insertions(+), 34 deletions(-) diff --git a/x/budget/types/params.go b/x/budget/types/params.go index d0a887b..d1aece8 100644 --- a/x/budget/types/params.go +++ b/x/budget/types/params.go @@ -93,7 +93,7 @@ func ValidateBudgets(i interface{}) error { for _, budget := range budgetsBySource.Budgets { totalRate := sdk.ZeroDec() for _, budgetToCheck := range budgetsBySource.Budgets { - if DateRageOverlap(budget.StartTime, budget.EndTime, budgetToCheck.StartTime, budgetToCheck.EndTime) { + if DateRangesOverlap(budget.StartTime, budget.EndTime, budgetToCheck.StartTime, budgetToCheck.EndTime) { totalRate = totalRate.Add(budgetToCheck.Rate) } } diff --git a/x/budget/types/params_test.go b/x/budget/types/params_test.go index cc4960b..d254a63 100644 --- a/x/budget/types/params_test.go +++ b/x/budget/types/params_test.go @@ -2,7 +2,6 @@ package types_test import ( "testing" - "time" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/address" @@ -34,32 +33,32 @@ func TestValidateBudgets(t *testing.T) { Rate: sdk.OneDec(), BudgetSourceAddress: tAddr1.String(), CollectionAddress: cAddr1.String(), - StartTime: time.Time{}, - EndTime: time.Time{}, + StartTime: types.MustParseRFC3339("2021-08-01T00:00:00Z"), + EndTime: types.MustParseRFC3339("2021-08-03T00:00:00Z"), }, { Name: "test2", Rate: sdk.OneDec(), BudgetSourceAddress: tAddr2.String(), CollectionAddress: cAddr2.String(), - StartTime: time.Time{}, - EndTime: time.Time{}, + StartTime: types.MustParseRFC3339("2021-08-03T00:00:00Z"), + EndTime: types.MustParseRFC3339("2021-08-05T00:00:00Z"), }, { Name: "test3", Rate: sdk.MustNewDecFromStr("0.1"), BudgetSourceAddress: tAddr2.String(), CollectionAddress: cAddr2.String(), - StartTime: time.Time{}, - EndTime: time.Time{}, + StartTime: types.MustParseRFC3339("2021-08-04T00:00:00Z"), + EndTime: types.MustParseRFC3339("2021-08-06T00:00:00Z"), }, { Name: "test3", Rate: sdk.MustNewDecFromStr("0.1"), BudgetSourceAddress: tAddr2.String(), CollectionAddress: cAddr2.String(), - StartTime: time.Time{}, - EndTime: time.Time{}, + StartTime: types.MustParseRFC3339("2021-07-25T00:00:00Z"), + EndTime: types.MustParseRFC3339("2021-07-30T00:00:00Z"), }, { Name: "test4", diff --git a/x/budget/types/utils.go b/x/budget/types/utils.go index 4fd907a..82a8e28 100644 --- a/x/budget/types/utils.go +++ b/x/budget/types/utils.go @@ -13,6 +13,8 @@ func MustParseRFC3339(s string) time.Time { return t } -func DateRageOverlap(startTimeA, endTimeA, startTimeB, endTimeB time.Time) bool { - return !startTimeA.After(endTimeB) && !endTimeA.Before(startTimeB) +// DateRangesOverlap returns true if two date ranges overlap each other. +// End time is exclusive and start time is inclusive. +func DateRangesOverlap(startTimeA, endTimeA, startTimeB, endTimeB time.Time) bool { + return startTimeA.Before(endTimeB) && endTimeA.After(startTimeB) } diff --git a/x/budget/types/utils_test.go b/x/budget/types/utils_test.go index a0c6a74..d86bdbe 100644 --- a/x/budget/types/utils_test.go +++ b/x/budget/types/utils_test.go @@ -19,7 +19,7 @@ func TestParseTime(t *testing.T) { require.Equal(t, normalRes, types.MustParseRFC3339(normalCase)) } -func TestDateRageOverlap(t *testing.T) { +func TestDateRangesOverlap(t *testing.T) { testCases := []struct { name string expectedResult bool @@ -29,41 +29,58 @@ func TestDateRageOverlap(t *testing.T) { endTimeB time.Time }{ { - "same range", + "not overlapping", + false, + types.MustParseRFC3339("2021-12-01T00:00:00Z"), + types.MustParseRFC3339("2021-12-02T00:00:00Z"), + types.MustParseRFC3339("2021-12-03T00:00:00Z"), + types.MustParseRFC3339("2021-12-04T00:00:00Z"), + }, + { + "same end time and start time", + false, + types.MustParseRFC3339("2021-12-01T00:00:00Z"), + types.MustParseRFC3339("2021-12-02T00:00:00Z"), + types.MustParseRFC3339("2021-12-02T00:00:00Z"), + types.MustParseRFC3339("2021-12-03T00:00:00Z"), + }, + { + "end time and start time differs by a little amount", true, - types.MustParseRFC3339("2021-12-31T00:00:00Z"), - types.MustParseRFC3339("2021-12-31T00:00:00Z"), - types.MustParseRFC3339("2021-12-31T00:00:00Z"), - types.MustParseRFC3339("2021-12-31T00:00:00Z"), + types.MustParseRFC3339("2021-12-01T00:00:00Z"), + types.MustParseRFC3339("2021-12-02T00:00:00.001Z"), + types.MustParseRFC3339("2021-12-02T00:00:00Z"), + types.MustParseRFC3339("2021-12-03T00:00:00Z"), }, { - "overlap with start", + "overlap #1", true, - types.MustParseRFC3339("2021-10-05T00:00:00Z"), - types.MustParseRFC3339("2021-12-31T00:00:00Z"), - types.MustParseRFC3339("2021-10-05T00:00:00Z"), - types.MustParseRFC3339("2021-11-10T00:00:00Z"), + types.MustParseRFC3339("2021-12-01T00:00:00Z"), + types.MustParseRFC3339("2021-12-03T00:00:00Z"), + types.MustParseRFC3339("2021-12-02T00:00:00Z"), + types.MustParseRFC3339("2021-12-04T00:00:00Z"), }, { - "overlap with start 2", + "overlap #2 - same ranges", true, - types.MustParseRFC3339("2021-10-05T00:00:00Z"), - types.MustParseRFC3339("2021-11-10T00:00:00Z"), - types.MustParseRFC3339("2021-10-05T00:00:00Z"), - types.MustParseRFC3339("2021-12-31T00:00:00Z"), + types.MustParseRFC3339("2021-12-01T00:00:00Z"), + types.MustParseRFC3339("2021-12-03T00:00:00Z"), + types.MustParseRFC3339("2021-12-01T00:00:00Z"), + types.MustParseRFC3339("2021-12-03T00:00:00Z"), }, { - "overlap 1 sec", + "overlap #3 - one includes another", true, - types.MustParseRFC3339("2021-10-05T00:00:00Z"), - types.MustParseRFC3339("2021-11-10T00:00:01Z"), - types.MustParseRFC3339("2021-11-10T00:00:00Z"), - types.MustParseRFC3339("2021-12-31T00:00:00Z"), + types.MustParseRFC3339("2021-12-02T00:00:00Z"), + types.MustParseRFC3339("2021-12-03T00:00:00Z"), + types.MustParseRFC3339("2021-12-01T00:00:00Z"), + types.MustParseRFC3339("2021-12-04T00:00:00Z"), }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - require.Equal(t, tc.expectedResult, types.DateRageOverlap(tc.startTimeA, tc.endTimeA, tc.startTimeB, tc.endTimeB)) + require.Equal(t, tc.expectedResult, types.DateRangesOverlap(tc.startTimeA, tc.endTimeA, tc.startTimeB, tc.endTimeB)) + require.Equal(t, tc.expectedResult, types.DateRangesOverlap(tc.startTimeB, tc.endTimeB, tc.startTimeA, tc.endTimeA)) }) } } From e2ca0dc1ebfddebb603ea2564515249052a1bd90 Mon Sep 17 00:00:00 2001 From: dongsam Date: Fri, 5 Nov 2021 16:49:53 +0900 Subject: [PATCH 7/8] fix: remove duplicated validating logic --- x/budget/types/budget.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/budget/types/budget.go b/x/budget/types/budget.go index dd0400f..dc48eaa 100644 --- a/x/budget/types/budget.go +++ b/x/budget/types/budget.go @@ -66,8 +66,7 @@ func (budget Budget) Collectible(blockTime time.Time) bool { // CollectibleBudgets returns only the valid and started and not expired budgets based on the given block time. func CollectibleBudgets(budgets []Budget, blockTime time.Time) (collectibleBudgets []Budget) { for _, budget := range budgets { - err := budget.Validate() - if err == nil && budget.Collectible(blockTime) { + if budget.Collectible(blockTime) { collectibleBudgets = append(collectibleBudgets, budget) } } From 36cbc69160b2f20aa2570ab39c89d59705c352a8 Mon Sep 17 00:00:00 2001 From: Hanjun Kim Date: Fri, 5 Nov 2021 16:51:58 +0900 Subject: [PATCH 8/8] fix: budget end time should be after budget start time --- x/budget/keeper/keeper_test.go | 2 +- x/budget/types/budget.go | 2 +- x/budget/types/errors.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x/budget/keeper/keeper_test.go b/x/budget/keeper/keeper_test.go index 95848e2..b95e29b 100644 --- a/x/budget/keeper/keeper_test.go +++ b/x/budget/keeper/keeper_test.go @@ -115,7 +115,7 @@ func (suite *KeeperTestSuite) SetupTest() { BudgetSourceAddress: suite.budgetSourceAddrs[2].String(), CollectionAddress: suite.collectionAddrs[3].String(), StartTime: types.MustParseRFC3339("0000-01-01T00:00:00Z"), - EndTime: types.MustParseRFC3339("0000-01-01T00:00:00Z"), + EndTime: types.MustParseRFC3339("0000-01-02T00:00:00Z"), }, { Name: "budget5", diff --git a/x/budget/types/budget.go b/x/budget/types/budget.go index f22ddf8..e570a8b 100644 --- a/x/budget/types/budget.go +++ b/x/budget/types/budget.go @@ -45,7 +45,7 @@ func (budget Budget) Validate() error { return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid budget source address %s: %v", budget.BudgetSourceAddress, err) } - if budget.EndTime.Before(budget.StartTime) { + if !budget.EndTime.After(budget.StartTime) { return ErrInvalidStartEndTime } diff --git a/x/budget/types/errors.go b/x/budget/types/errors.go index e96cad5..e556985 100644 --- a/x/budget/types/errors.go +++ b/x/budget/types/errors.go @@ -7,7 +7,7 @@ import ( // Sentinel errors for the budget module. var ( ErrInvalidBudgetName = sdkerrors.Register(ModuleName, 2, "budget name only allows letters, digits, and dash(-) without spaces and the maximum length is 50") - ErrInvalidStartEndTime = sdkerrors.Register(ModuleName, 3, "budget end time should not be earlier than budget start time") + ErrInvalidStartEndTime = sdkerrors.Register(ModuleName, 3, "budget end time must be after the start time") ErrInvalidBudgetRate = sdkerrors.Register(ModuleName, 4, "invalid budget rate") ErrInvalidTotalBudgetRate = sdkerrors.Register(ModuleName, 5, "invalid total rate of the budgets with the same budget source address") ErrDuplicateBudgetName = sdkerrors.Register(ModuleName, 6, "duplicate budget name")