Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix epochs modules tests #1893

Merged
merged 15 commits into from
Jun 29, 2022
Merged

Fix epochs modules tests #1893

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
23d9904
Fix epochs modules tests
ValarDragon Jun 28, 2022
8915190
Remove debug code
ValarDragon Jun 28, 2022
1de1759
Fix altered API usage within superfluid
ValarDragon Jun 28, 2022
f4ef09a
Add changelog entry
ValarDragon Jun 28, 2022
d6f4c29
Add more AddEpochInfo checks
ValarDragon Jun 28, 2022
c520bae
Fix superfluid tests
ValarDragon Jun 28, 2022
d7c3f6b
Update osmoutils/slice_helper.go
ValarDragon Jun 28, 2022
4020b2b
Changelog nit
ValarDragon Jun 28, 2022
b0e4c16
Apply Roman's suggestions!
ValarDragon Jun 29, 2022
d576203
add const
ValarDragon Jun 29, 2022
5941e9d
Apply roman's suggestion of tests w/ non-default values
ValarDragon Jun 29, 2022
23c067f
Apply roman's comment update suggestions
ValarDragon Jun 29, 2022
fd72bbe
AddEpochInfo comment
ValarDragon Jun 29, 2022
844758e
fix gomod order
ValarDragon Jun 29, 2022
e90e497
Add eps comment
ValarDragon Jun 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ require (
mvdan.cc/gofumpt v0.3.1
)

require golang.org/x/exp v0.0.0-20220613132600-b0d781184e0d
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved

require (
4d63.com/gochecknoglobals v0.1.0 // indirect
filippo.io/edwards25519 v1.0.0-beta.2 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1484,6 +1484,8 @@ golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u0
golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM=
golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU=
golang.org/x/exp v0.0.0-20200331195152-e8c3332aa8e5/go.mod h1:4M0jN8W1tt0AVLNr8HDosyJCDCDuyL9N9+3m7wDWgKw=
golang.org/x/exp v0.0.0-20220613132600-b0d781184e0d h1:vtUKgx8dahOomfFzLREU8nSv25YHnTgLBn4rDnWZdU0=
golang.org/x/exp v0.0.0-20220613132600-b0d781184e0d/go.mod h1:Kr81I6Kryrl9sr8s2FK3vxD90NdsKWRuOIl2O4CvYbA=
golang.org/x/exp/typeparams v0.0.0-20220218215828-6cf2b201936e h1:qyrTQ++p1afMkO4DPEeLGq/3oTsdlvdH4vqZUBWzUKM=
golang.org/x/exp/typeparams v0.0.0-20220218215828-6cf2b201936e/go.mod h1:AbB0pIl9nAr9wVwH+Z2ZpaocVmF5I4GyWCDIsVjR0bk=
golang.org/x/image v0.0.0-20180708004352-c73c2afc3b81/go.mod h1:ux5Hcp/YLpHSI86hEcLt0YII63i6oz57MZXIpbrjZUs=
Expand Down
13 changes: 13 additions & 0 deletions osmoutils/slice_helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package osmoutils

import (
"sort"

"golang.org/x/exp/constraints"
)

func SortSlice[T constraints.Ordered](s []T) {
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved
sort.Slice(s, func(i, j int) bool {
return s[i] < s[j]
})
}
2 changes: 1 addition & 1 deletion x/epochs/keeper/abci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (k Keeper) BeginBlocker(ctx sdk.Context) {
sdk.NewAttribute(types.AttributeEpochStartTime, fmt.Sprintf("%d", epochInfo.CurrentEpochStartTime.Unix())),
),
)
k.SetEpochInfo(ctx, epochInfo)
k.setEpochInfo(ctx, epochInfo)
k.BeforeEpochStart(ctx, epochInfo.Identifier, epochInfo.CurrentEpoch)

return false
Expand Down
248 changes: 79 additions & 169 deletions x/epochs/keeper/abci_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,146 +10,98 @@ import (
simapp "github.com/osmosis-labs/osmosis/v7/app"
"github.com/osmosis-labs/osmosis/v7/x/epochs/types"

sdk "github.com/cosmos/cosmos-sdk/types"
)

func TestEpochInfoChangesBeginBlockerAndInitGenesis(t *testing.T) {
var app *simapp.OsmosisApp
var ctx sdk.Context
var epochInfo types.EpochInfo
"golang.org/x/exp/maps"

now := time.Now()
"github.com/osmosis-labs/osmosis/v7/osmoutils"
)

tests := []struct {
expCurrentEpochStartTime time.Time
expCurrentEpochStartHeight int64
expCurrentEpoch int64
expInitialEpochStartTime time.Time
fn func()
// This test is responsible for testing how epochs increment based off
// of their initial conditions, and subsequent block height / times.
//
// TODO: Make a new test for init genesis logic
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved
func (suite KeeperTestSuite) TestEpochInfoBeginBlockChanges() {
block1Time := time.Unix(1656907200, 0).UTC()
// We run all tests with the same identifier and same duration
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved
defaultIdentifier := "hourly"
defaultDuration := time.Hour
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is eps? I'm assumming "epsilon"? Can we add a comment to explain why we need this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this this value for epsilon is significant enough for a tick? From what I see, this is an implementation detail of the time package used for comparing time. Can we add a comment explaining this please?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is also slightly off for some reason. Referring to line 27

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eps is epsilon. Its for an insignifcant time difference, its not a detail of the time package, but for bounds checking.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Nanosecond was the smallest time that my auto-completer could fill in. Could really be any unit of time)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying. I think converting this into a comment would be useful 👍

eps := time.Nanosecond
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved

tests := map[string]struct {
// if identifier, duration is not set, we make it defaultIdentifier and defaultDuration.
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved
// EpochCountingStarted, if unspecified, is inferred by CurrentEpoch == 0
// StartTime is inferred to be block1Time if left blank.
initialEpochInfo types.EpochInfo
blockHeightTimePairs map[int]time.Time
expEpochInfo types.EpochInfo
}{
{
// Only advance 2 seconds, do not increment epoch
expCurrentEpochStartHeight: 2,
expCurrentEpochStartTime: now,
expCurrentEpoch: 1,
expInitialEpochStartTime: now,
fn: func() {
ctx = ctx.WithBlockHeight(2).WithBlockTime(now.Add(time.Second))
app.EpochsKeeper.BeginBlocker(ctx)
epochInfo = app.EpochsKeeper.GetEpochInfo(ctx, "monthly")
},
"First block running at exactly start time sets epoch tick": {
initialEpochInfo: types.EpochInfo{StartTime: block1Time, CurrentEpoch: 0, CurrentEpochStartTime: time.Time{}},
expEpochInfo: types.EpochInfo{StartTime: block1Time, CurrentEpoch: 1, CurrentEpochStartTime: block1Time, CurrentEpochStartHeight: 1},
},
{
expCurrentEpochStartHeight: 2,
expCurrentEpochStartTime: now,
expCurrentEpoch: 1,
expInitialEpochStartTime: now,
fn: func() {
ctx = ctx.WithBlockHeight(2).WithBlockTime(now.Add(time.Second))
app.EpochsKeeper.BeginBlocker(ctx)
epochInfo = app.EpochsKeeper.GetEpochInfo(ctx, "monthly")
},
"First block run sets start time, subsequent blocks within interval do not affect": {
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved
initialEpochInfo: types.EpochInfo{StartTime: block1Time, CurrentEpoch: 0, CurrentEpochStartTime: time.Time{}},
blockHeightTimePairs: map[int]time.Time{2: block1Time.Add(time.Second), 3: block1Time.Add(time.Minute), 4: block1Time.Add(30 * time.Minute)},
expEpochInfo: types.EpochInfo{StartTime: block1Time, CurrentEpoch: 1, CurrentEpochStartTime: block1Time, CurrentEpochStartHeight: 1},
},
{
expCurrentEpochStartHeight: 2,
expCurrentEpochStartTime: now,
expCurrentEpoch: 1,
expInitialEpochStartTime: now,
fn: func() {
ctx = ctx.WithBlockHeight(2).WithBlockTime(now.Add(time.Second))
app.EpochsKeeper.BeginBlocker(ctx)
ctx = ctx.WithBlockHeight(3).WithBlockTime(now.Add(time.Hour * 24 * 31))
app.EpochsKeeper.BeginBlocker(ctx)
epochInfo = app.EpochsKeeper.GetEpochInfo(ctx, "monthly")
},
"Second block at exactly interval later does not tick": {
p0mvn marked this conversation as resolved.
Show resolved Hide resolved
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved
initialEpochInfo: types.EpochInfo{StartTime: block1Time, CurrentEpoch: 0, CurrentEpochStartTime: time.Time{}},
blockHeightTimePairs: map[int]time.Time{2: block1Time.Add(time.Hour)},
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved
expEpochInfo: types.EpochInfo{StartTime: block1Time, CurrentEpoch: 1, CurrentEpochStartTime: block1Time, CurrentEpochStartHeight: 1},
},
// Test that incrementing _exactly_ 1 month increments the epoch count.
{
expCurrentEpochStartHeight: 3,
expCurrentEpochStartTime: now.Add(time.Hour * 24 * 31),
expCurrentEpoch: 2,
expInitialEpochStartTime: now,
fn: func() {
ctx = ctx.WithBlockHeight(2).WithBlockTime(now.Add(time.Second))
app.EpochsKeeper.BeginBlocker(ctx)
ctx = ctx.WithBlockHeight(3).WithBlockTime(now.Add(time.Hour * 24 * 32))
app.EpochsKeeper.BeginBlocker(ctx)
epochInfo = app.EpochsKeeper.GetEpochInfo(ctx, "monthly")
},
"Second block at interval + epsilon later does tick": {
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved
initialEpochInfo: types.EpochInfo{StartTime: block1Time, CurrentEpoch: 0, CurrentEpochStartTime: time.Time{}},
blockHeightTimePairs: map[int]time.Time{2: block1Time.Add(time.Hour).Add(eps)},
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved
expEpochInfo: types.EpochInfo{StartTime: block1Time, CurrentEpoch: 2, CurrentEpochStartTime: block1Time.Add(time.Hour), CurrentEpochStartHeight: 2},
},
{
expCurrentEpochStartHeight: 3,
expCurrentEpochStartTime: now.Add(time.Hour * 24 * 31),
expCurrentEpoch: 2,
expInitialEpochStartTime: now,
fn: func() {
ctx = ctx.WithBlockHeight(2).WithBlockTime(now.Add(time.Second))
app.EpochsKeeper.BeginBlocker(ctx)
ctx = ctx.WithBlockHeight(3).WithBlockTime(now.Add(time.Hour * 24 * 32))
app.EpochsKeeper.BeginBlocker(ctx)
ctx.WithBlockHeight(4).WithBlockTime(now.Add(time.Hour * 24 * 33))
app.EpochsKeeper.BeginBlocker(ctx)
epochInfo = app.EpochsKeeper.GetEpochInfo(ctx, "monthly")
},
"Downtime recovery (many intervals), first block causes 1 tick and sets current start time 1 interval ahead": {
initialEpochInfo: types.EpochInfo{StartTime: block1Time, CurrentEpoch: 0, CurrentEpochStartTime: time.Time{}},
blockHeightTimePairs: map[int]time.Time{2: block1Time.Add(24 * time.Hour)},
expEpochInfo: types.EpochInfo{StartTime: block1Time, CurrentEpoch: 2, CurrentEpochStartTime: block1Time.Add(time.Hour), CurrentEpochStartHeight: 2},
},
{
expCurrentEpochStartHeight: 3,
expCurrentEpochStartTime: now.Add(time.Hour * 24 * 31),
expCurrentEpoch: 2,
expInitialEpochStartTime: now,
fn: func() {
ctx = ctx.WithBlockHeight(2).WithBlockTime(now.Add(time.Second))
app.EpochsKeeper.BeginBlocker(ctx)
ctx = ctx.WithBlockHeight(3).WithBlockTime(now.Add(time.Hour * 24 * 32))
app.EpochsKeeper.BeginBlocker(ctx)
numBlocksSinceStart, _ := app.EpochsKeeper.NumBlocksSinceEpochStart(ctx, "monthly")
require.Equal(t, int64(0), numBlocksSinceStart)
ctx = ctx.WithBlockHeight(4).WithBlockTime(now.Add(time.Hour * 24 * 33))
app.EpochsKeeper.BeginBlocker(ctx)
epochInfo = app.EpochsKeeper.GetEpochInfo(ctx, "monthly")
numBlocksSinceStart, _ = app.EpochsKeeper.NumBlocksSinceEpochStart(ctx, "monthly")
require.Equal(t, int64(1), numBlocksSinceStart)
},
"Downtime recovery (many intervals), second block is at tick 2, w/ start time 2 intervals ahead": {
initialEpochInfo: types.EpochInfo{StartTime: block1Time, CurrentEpoch: 0, CurrentEpochStartTime: time.Time{}},
blockHeightTimePairs: map[int]time.Time{2: block1Time.Add(24 * time.Hour), 3: block1Time.Add(24 * time.Hour).Add(eps)},
expEpochInfo: types.EpochInfo{StartTime: block1Time, CurrentEpoch: 3, CurrentEpochStartTime: block1Time.Add(2 * time.Hour), CurrentEpochStartHeight: 3},
},
"Many blocks between first and second tick": {
initialEpochInfo: types.EpochInfo{StartTime: block1Time, CurrentEpoch: 1, CurrentEpochStartTime: block1Time},
blockHeightTimePairs: map[int]time.Time{2: block1Time.Add(time.Second), 3: block1Time.Add(2 * time.Second), 4: block1Time.Add(time.Hour).Add(eps)},
expEpochInfo: types.EpochInfo{StartTime: block1Time, CurrentEpoch: 2, CurrentEpochStartTime: block1Time.Add(time.Hour), CurrentEpochStartHeight: 4},
},
}

for _, test := range tests {
app = simapp.Setup(false)
ctx = app.BaseApp.NewContext(false, tmproto.Header{})

// On init genesis, default epochs information is set
// To check init genesis again, should make it fresh status
epochInfos := app.EpochsKeeper.AllEpochInfos(ctx)
for _, epochInfo := range epochInfos {
app.EpochsKeeper.DeleteEpochInfo(ctx, epochInfo.Identifier)
}

ctx = ctx.WithBlockHeight(1).WithBlockTime(now)
// check init genesis
app.EpochsKeeper.InitGenesis(ctx, types.GenesisState{
Epochs: []types.EpochInfo{
{
Identifier: "monthly",
StartTime: time.Time{},
Duration: time.Hour * 24 * 31,
CurrentEpoch: 0,
CurrentEpochStartHeight: ctx.BlockHeight(),
CurrentEpochStartTime: time.Time{},
EpochCountingStarted: false,
},
},
for name, test := range tests {
suite.Run(name, func() {
suite.SetupTest()
suite.Ctx = suite.Ctx.WithBlockHeight(1).WithBlockTime(block1Time)
initialEpoch := initializeBlankEpochInfoFields(test.initialEpochInfo, defaultIdentifier, defaultDuration)
suite.App.EpochsKeeper.AddEpochInfo(suite.Ctx, initialEpoch)
suite.App.EpochsKeeper.BeginBlocker(suite.Ctx)

// get sorted heights
heights := maps.Keys(test.blockHeightTimePairs)
osmoutils.SortSlice(heights)
for _, h := range heights {
Comment on lines +95 to +97
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This go 1.18 deterministic iteration over maps API 😍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love it

// for each height in order, run begin block
suite.Ctx = suite.Ctx.WithBlockHeight(int64(h)).WithBlockTime(test.blockHeightTimePairs[h])
suite.App.EpochsKeeper.BeginBlocker(suite.Ctx)
}
expEpoch := initializeBlankEpochInfoFields(test.expEpochInfo, initialEpoch.Identifier, initialEpoch.Duration)
actEpoch := suite.App.EpochsKeeper.GetEpochInfo(suite.Ctx, initialEpoch.Identifier)
suite.Require().Equal(expEpoch, actEpoch)
})
}
}

test.fn()

require.Equal(t, epochInfo.Identifier, "monthly")
require.Equal(t, epochInfo.StartTime.UTC().String(), test.expInitialEpochStartTime.UTC().String())
require.Equal(t, epochInfo.Duration, time.Hour*24*31)
require.Equal(t, epochInfo.CurrentEpoch, test.expCurrentEpoch)
require.Equal(t, epochInfo.CurrentEpochStartHeight, test.expCurrentEpochStartHeight)
require.Equal(t, epochInfo.CurrentEpochStartTime.UTC().String(), test.expCurrentEpochStartTime.UTC().String())
require.Equal(t, epochInfo.EpochCountingStarted, true)
// set identifier, duration and epochCountingStarted if blank in epoch
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved
func initializeBlankEpochInfoFields(epoch types.EpochInfo, identifier string, duration time.Duration) types.EpochInfo {
if epoch.Identifier == "" {
epoch.Identifier = identifier
}
if epoch.Duration == time.Duration(0) {
epoch.Duration = duration
}
epoch.EpochCountingStarted = (epoch.CurrentEpoch != 0)
return epoch
}

func TestEpochStartingOneMonthAfterInitGenesis(t *testing.T) {
Expand Down Expand Up @@ -212,45 +164,3 @@ func TestEpochStartingOneMonthAfterInitGenesis(t *testing.T) {
require.Equal(t, epochInfo.CurrentEpochStartTime.UTC().String(), now.Add(month).UTC().String())
require.Equal(t, epochInfo.EpochCountingStarted, true)
}

// This test ensures legacy EpochInfo messages will not throw errors via InitGenesis and BeginBlocker
func TestLegacyEpochSerialization(t *testing.T) {
Comment on lines -216 to -217
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is deadcode / for an obsoleted concern. (it was for the v4 upgrade, when we added some more fields to this struct, and deleted a dead one)

// Legacy Epoch Info message - without CurrentEpochStartHeight property
legacyEpochInfo := types.EpochInfo{
Identifier: "monthly",
StartTime: time.Time{},
Duration: time.Hour * 24 * 31,
CurrentEpoch: 0,
CurrentEpochStartTime: time.Time{},
EpochCountingStarted: false,
}

now := time.Now()
app := simapp.Setup(false)
ctx := app.BaseApp.NewContext(false, tmproto.Header{})

// On init genesis, default epochs information is set
// To check init genesis again, should make it fresh status
epochInfos := app.EpochsKeeper.AllEpochInfos(ctx)
for _, epochInfo := range epochInfos {
app.EpochsKeeper.DeleteEpochInfo(ctx, epochInfo.Identifier)
}

ctx = ctx.WithBlockHeight(1).WithBlockTime(now)

// check init genesis
app.EpochsKeeper.InitGenesis(ctx, types.GenesisState{
Epochs: []types.EpochInfo{legacyEpochInfo},
})

// Do not increment epoch
ctx = ctx.WithBlockHeight(2).WithBlockTime(now.Add(time.Second))
app.EpochsKeeper.BeginBlocker(ctx)

// Increment epoch
ctx = ctx.WithBlockHeight(3).WithBlockTime(now.Add(time.Hour * 24 * 32))
app.EpochsKeeper.BeginBlocker(ctx)
epochInfo := app.EpochsKeeper.GetEpochInfo(ctx, "monthly")

require.NotEqual(t, epochInfo.CurrentEpochStartHeight, int64(0))
}
21 changes: 19 additions & 2 deletions x/epochs/keeper/epoch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package keeper

import (
"fmt"
"time"

"github.com/gogo/protobuf/proto"

"github.com/osmosis-labs/osmosis/v7/x/epochs/types"

sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -24,8 +26,23 @@ func (k Keeper) GetEpochInfo(ctx sdk.Context, identifier string) types.EpochInfo
return epoch
}

// SetEpochInfo set epoch info.
func (k Keeper) SetEpochInfo(ctx sdk.Context, epoch types.EpochInfo) {
// setEpochInfo set epoch info.
func (k Keeper) AddEpochInfo(ctx sdk.Context, epoch types.EpochInfo) error {
err := epoch.Validate()
if err != nil {
return err
}
// Initialize empty epoch values via Cosmos SDK
if epoch.StartTime.Equal(time.Time{}) {
epoch.StartTime = ctx.BlockTime()
}
epoch.CurrentEpochStartHeight = ctx.BlockHeight()
k.setEpochInfo(ctx, epoch)
return nil
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved
}

// setEpochInfo set epoch info.
func (k Keeper) setEpochInfo(ctx sdk.Context, epoch types.EpochInfo) {
store := ctx.KVStore(k.storeKey)
value, err := proto.Marshal(&epoch)
if err != nil {
Expand Down
8 changes: 6 additions & 2 deletions x/epochs/keeper/epoch_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,13 @@ func (suite *KeeperTestSuite) TestEpochLifeCycle() {
suite.SetupTest()

epochInfo := types.NewGenesisEpochInfo("monthly", time.Hour*24*30)
suite.App.EpochsKeeper.SetEpochInfo(suite.Ctx, epochInfo)
suite.App.EpochsKeeper.AddEpochInfo(suite.Ctx, epochInfo)
epochInfoSaved := suite.App.EpochsKeeper.GetEpochInfo(suite.Ctx, "monthly")
suite.Require().Equal(epochInfo, epochInfoSaved)
// setup expected epoch info
expectedEpochInfo := epochInfo
expectedEpochInfo.StartTime = suite.Ctx.BlockTime()
expectedEpochInfo.CurrentEpochStartHeight = suite.Ctx.BlockHeight()
suite.Require().Equal(expectedEpochInfo, epochInfoSaved)

allEpochs := suite.App.EpochsKeeper.AllEpochInfos(suite.Ctx)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense splitting up the AllEpochInfos test into its own test fixture? It seems that we are testing 2 separate methods in one fixture

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really sure theres much to add as a general test here that'd be of value, versus us making this test have documentation for what exactly its doing. I guess we could provide a list of []types.EpochInfo and then call AllEpochInfos on it

suite.Require().Len(allEpochs, 4)
Expand Down
12 changes: 3 additions & 9 deletions x/epochs/keeper/genesis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package keeper

import (
"time"

sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/osmosis-labs/osmosis/v7/x/epochs/types"
Expand All @@ -11,14 +9,10 @@ import (
func (k Keeper) InitGenesis(ctx sdk.Context, genState types.GenesisState) {
// set epoch info from genesis
for _, epoch := range genState.Epochs {
// Initialize empty epoch values via Cosmos SDK
if epoch.StartTime.Equal(time.Time{}) {
epoch.StartTime = ctx.BlockTime()
err := k.AddEpochInfo(ctx, epoch)
if err != nil {
panic(err)
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved
}

epoch.CurrentEpochStartHeight = ctx.BlockHeight()

k.SetEpochInfo(ctx, epoch)
}
}

Expand Down
Loading