Skip to content

Commit

Permalink
Fix epochs modules tests (#1893)
Browse files Browse the repository at this point in the history
* Fix epochs modules tests

* Remove debug code

* Fix altered API usage within superfluid

* Add changelog entry

* Add more AddEpochInfo checks

* Fix superfluid tests

* Update osmoutils/slice_helper.go

Co-authored-by: Aleksandr Bezobchuk <[email protected]>

* Changelog nit

* Apply Roman's suggestions!

Co-authored-by: Roman <[email protected]>

* add const

* Apply roman's suggestion of tests w/ non-default values

* Apply roman's comment update suggestions

* AddEpochInfo comment

* fix gomod order

Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Co-authored-by: Roman <[email protected]>
(cherry picked from commit 21f7f98)

# Conflicts:
#	CHANGELOG.md
#	go.mod
#	x/epochs/abci_test.go
#	x/epochs/keeper/epoch.go
#	x/epochs/keeper/genesis.go
#	x/superfluid/keeper/keeper.go
#	x/superfluid/keeper/twap_price_test.go
  • Loading branch information
ValarDragon authored and mergify[bot] committed Aug 4, 2022
1 parent 81d9c6b commit ca55559
Show file tree
Hide file tree
Showing 12 changed files with 188 additions and 70 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ This release contains minor CLI bug fixes.

* [#1699](https://github.com/osmosis-labs/osmosis/pull/1699) Fixes bug in sig fig rounding on spot price queries for small values

<<<<<<< HEAD
=======
#### Golang API breaks

* [#1893](https://github.com/osmosis-labs/osmosis/pull/1893) Change `EpochsKeeper.SetEpochInfo` to `AddEpochInfo`, which has more safety checks with it. (Makes it suitable to be called within upgrades)
>>>>>>> 21f7f981 (Fix epochs modules tests (#1893))
* [#1671](https://github.com/osmosis-labs/osmosis/pull/1671) Remove methods that constitute AppModuleSimulation APIs for several modules' AppModules, which implemented no-ops
* [#1671](https://github.com/osmosis-labs/osmosis/pull/1671) Add hourly epochs to `x/epochs` DefaultGenesis.
* [#1665](https://github.com/osmosis-labs/osmosis/pull/1665) Delete app/App interface, instead use simapp.App
Expand Down
5 changes: 5 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,12 @@ require (
github.com/spf13/viper v1.12.0
github.com/stretchr/testify v1.7.1
github.com/tendermint/tendermint v0.34.19
<<<<<<< HEAD
github.com/tendermint/tm-db v0.6.7
=======
github.com/tendermint/tm-db v0.6.8-0.20220506192307-f628bb5dc95b
golang.org/x/exp v0.0.0-20220613132600-b0d781184e0d
>>>>>>> 21f7f981 (Fix epochs modules tests (#1893))
google.golang.org/genproto v0.0.0-20220519153652-3a47de7e79bd
google.golang.org/grpc v1.46.2
google.golang.org/protobuf v1.28.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1473,6 +1473,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
14 changes: 14 additions & 0 deletions osmoutils/slice_helper.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package osmoutils

import (
"sort"

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

// SortSlice sorts a slice of type T elements that implement constraints.Ordered.
func SortSlice[T constraints.Ordered](s []T) {
sort.Slice(s, func(i, j int) bool {
return s[i] < s[j]
})
}
2 changes: 1 addition & 1 deletion x/epochs/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func BeginBlocker(ctx sdk.Context, k keeper.Keeper) {
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
149 changes: 102 additions & 47 deletions x/epochs/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,31 @@ import (
"github.com/osmosis-labs/osmosis/v10/x/epochs"
"github.com/osmosis-labs/osmosis/v10/x/epochs/types"

sdk "github.com/cosmos/cosmos-sdk/types"
)
"golang.org/x/exp/maps"

func TestEpochInfoChangesBeginBlockerAndInitGenesis(t *testing.T) {
var app *simapp.OsmosisApp
var ctx sdk.Context
var epochInfo types.EpochInfo
"github.com/osmosis-labs/osmosis/v7/osmoutils"
)

now := time.Now()
// 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
func (suite KeeperTestSuite) TestEpochInfoBeginBlockChanges() {
block1Time := time.Unix(1656907200, 0).UTC()
const defaultIdentifier = "hourly"
const defaultDuration = time.Hour
// eps is short for epsilon - in this case a negligible amount of time.
const eps = time.Nanosecond

tests := []struct {
expCurrentEpochStartTime time.Time
expCurrentEpochStartHeight int64
expCurrentEpoch int64
expInitialEpochStartTime time.Time
fn func()
tests := map[string]struct {
// if identifier, duration is not set, we make it defaultIdentifier and defaultDuration.
// 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
}{
<<<<<<< HEAD:x/epochs/abci_test.go
{
// Only advance 2 seconds, do not increment epoch
expCurrentEpochStartHeight: 2,
Expand Down Expand Up @@ -111,46 +119,90 @@ func TestEpochInfoChangesBeginBlockerAndInitGenesis(t *testing.T) {
numBlocksSinceStart, _ = app.EpochsKeeper.NumBlocksSinceEpochStart(ctx, "monthly")
require.Equal(t, int64(1), numBlocksSinceStart)
},
=======
"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},
},
"First block run sets start time, subsequent blocks within timer interval do not cause timer tick": {
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},
},
"Second block at exactly timer interval later does not tick": {
initialEpochInfo: types.EpochInfo{StartTime: block1Time, CurrentEpoch: 0, CurrentEpochStartTime: time.Time{}},
blockHeightTimePairs: map[int]time.Time{2: block1Time.Add(defaultDuration)},
expEpochInfo: types.EpochInfo{StartTime: block1Time, CurrentEpoch: 1, CurrentEpochStartTime: block1Time, CurrentEpochStartHeight: 1},
},
"Second block at timer interval + epsilon later does tick": {
initialEpochInfo: types.EpochInfo{StartTime: block1Time, CurrentEpoch: 0, CurrentEpochStartTime: time.Time{}},
blockHeightTimePairs: map[int]time.Time{2: block1Time.Add(defaultDuration).Add(eps)},
expEpochInfo: types.EpochInfo{StartTime: block1Time, CurrentEpoch: 2, CurrentEpochStartTime: block1Time.Add(time.Hour), CurrentEpochStartHeight: 2},
},
"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},
},
"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},
},
"Distinct identifier and duration still works": {
initialEpochInfo: types.EpochInfo{Identifier: "hello", Duration: time.Minute, StartTime: block1Time, CurrentEpoch: 0, CurrentEpochStartTime: time.Time{}},
blockHeightTimePairs: map[int]time.Time{2: block1Time.Add(time.Second), 3: block1Time.Add(time.Minute).Add(eps)},
expEpochInfo: types.EpochInfo{Identifier: "hello", Duration: time.Minute, StartTime: block1Time, CurrentEpoch: 2, CurrentEpochStartTime: block1Time.Add(time.Minute), CurrentEpochStartHeight: 3},
},
"StartTime in future won't get ticked on first block": {
initialEpochInfo: types.EpochInfo{StartTime: block1Time.Add(time.Second), CurrentEpoch: 0, CurrentEpochStartTime: time.Time{}},
// currentEpochStartHeight is 1 because thats when the timer was created on-chain
expEpochInfo: types.EpochInfo{StartTime: block1Time.Add(time.Second), CurrentEpoch: 0, CurrentEpochStartTime: time.Time{}, CurrentEpochStartHeight: 1},
},
"StartTime in past will get ticked on first block": {
initialEpochInfo: types.EpochInfo{StartTime: block1Time.Add(-time.Second), CurrentEpoch: 0, CurrentEpochStartTime: time.Time{}},
expEpochInfo: types.EpochInfo{StartTime: block1Time.Add(-time.Second), CurrentEpoch: 1, CurrentEpochStartTime: block1Time.Add(-time.Second), CurrentEpochStartHeight: 1},
>>>>>>> 21f7f981 (Fix epochs modules tests (#1893)):x/epochs/keeper/abci_test.go
},
}
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)

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,
},
},
// get sorted heights
heights := maps.Keys(test.blockHeightTimePairs)
osmoutils.SortSlice(heights)
for _, h := range heights {
// 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)
// initializeBlankEpochInfoFields set identifier, duration and epochCountingStarted if blank in epoch
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 @@ -213,6 +265,7 @@ func TestEpochStartingOneMonthAfterInitGenesis(t *testing.T) {
require.Equal(t, epochInfo.CurrentEpochStartTime.UTC().String(), now.Add(month).UTC().String())
require.Equal(t, epochInfo.EpochCountingStarted, true)
}
<<<<<<< HEAD:x/epochs/abci_test.go

// This test ensures legacy EpochInfo messages will not throw errors via InitGenesis and BeginBlocker
func TestLegacyEpochSerialization(t *testing.T) {
Expand Down Expand Up @@ -255,3 +308,5 @@ func TestLegacyEpochSerialization(t *testing.T) {

require.NotEqual(t, epochInfo.CurrentEpochStartHeight, int64(0))
}
=======
>>>>>>> 21f7f981 (Fix epochs modules tests (#1893)):x/epochs/keeper/abci_test.go
31 changes: 29 additions & 2 deletions x/epochs/keeper/epoch.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,15 @@ package keeper

import (
"fmt"
"time"

"github.com/gogo/protobuf/proto"

<<<<<<< HEAD
"github.com/osmosis-labs/osmosis/v10/x/epochs/types"
=======
"github.com/osmosis-labs/osmosis/v7/x/epochs/types"
>>>>>>> 21f7f981 (Fix epochs modules tests (#1893))

sdk "github.com/cosmos/cosmos-sdk/types"
)
Expand All @@ -25,8 +30,30 @@ 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) {
// AddEpochInfo adds a new epoch info. Will return an error if the epoch fails validation,
// or re-uses an existing identifier.
// This method also sets the start time if left unset, and sets the epoch start height.
func (k Keeper) AddEpochInfo(ctx sdk.Context, epoch types.EpochInfo) error {
err := epoch.Validate()
if err != nil {
return err
}
// Check if identifier already exists
if (k.GetEpochInfo(ctx, epoch.Identifier) != types.EpochInfo{}) {
return fmt.Errorf("epoch with identifier %s already exists", epoch.Identifier)
}

// Initialize empty and default epoch values
if epoch.StartTime.Equal(time.Time{}) {
epoch.StartTime = ctx.BlockTime()
}
epoch.CurrentEpochStartHeight = ctx.BlockHeight()
k.setEpochInfo(ctx, epoch)
return nil
}

// 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
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)
suite.Require().Len(allEpochs, 4)
Expand Down
14 changes: 7 additions & 7 deletions x/epochs/keeper/genesis.go
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
package keeper

import (
<<<<<<< HEAD
"time"

"github.com/osmosis-labs/osmosis/v10/x/epochs/types"
=======
sdk "github.com/cosmos/cosmos-sdk/types"
>>>>>>> 21f7f981 (Fix epochs modules tests (#1893))

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

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)
}

epoch.CurrentEpochStartHeight = ctx.BlockHeight()

k.SetEpochInfo(ctx, epoch)
}
}

Expand Down
4 changes: 4 additions & 0 deletions x/superfluid/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ import (

"github.com/tendermint/tendermint/libs/log"

<<<<<<< HEAD
"github.com/osmosis-labs/osmosis/v10/x/superfluid/types"
=======
"github.com/osmosis-labs/osmosis/v7/x/superfluid/types"
>>>>>>> 21f7f981 (Fix epochs modules tests (#1893))

"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down
13 changes: 8 additions & 5 deletions x/superfluid/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,20 +43,23 @@ func (suite *KeeperTestSuite) SetupTest() {
unbondingDuration,
})

// TODO: Revisit if this is needed, it was added due to another bug in testing that is now fixed.
epochIdentifier := suite.App.SuperfluidKeeper.GetEpochIdentifier(suite.Ctx)
suite.App.EpochsKeeper.SetEpochInfo(suite.Ctx, epochtypes.EpochInfo{
Identifier: epochIdentifier,
superfluidEpochIdentifer := "superfluid_epoch"
incentiveKeeperParams := suite.App.IncentivesKeeper.GetParams(suite.Ctx)
incentiveKeeperParams.DistrEpochIdentifier = superfluidEpochIdentifer
suite.App.IncentivesKeeper.SetParams(suite.Ctx, incentiveKeeperParams)
err := suite.App.EpochsKeeper.AddEpochInfo(suite.Ctx, epochtypes.EpochInfo{
Identifier: superfluidEpochIdentifer,
StartTime: startTime,
Duration: time.Hour,
CurrentEpochStartTime: startTime,
CurrentEpochStartHeight: 1,
CurrentEpoch: 1,
EpochCountingStarted: true,
})
suite.Require().NoError(err)

mintParams := suite.App.MintKeeper.GetParams(suite.Ctx)
mintParams.EpochIdentifier = epochIdentifier
mintParams.EpochIdentifier = superfluidEpochIdentifer
mintParams.DistributionProportions = minttypes.DistributionProportions{
Staking: sdk.OneDec(),
PoolIncentives: sdk.ZeroDec(),
Expand Down
Loading

0 comments on commit ca55559

Please sign in to comment.