chore: audit epoch (backport #24610) (#24613)

Co-authored-by: Alex | Interchain Labs <[email protected]>

Author: mergify[bot]

CHANGELOG.md

@@ -109,6 +109,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (x/auth) [#23741](https://github.com/cosmos/cosmos-sdk/pull/23741) Support legacy global AccountNumber for legacy compatibility.
 * (baseapp) [#24526](https://github.com/cosmos/cosmos-sdk/pull/24526) Fix incorrect retention height when `commitHeight` equals `minRetainBlocks`.
 * (x/protocolpool) [#24594](https://github.com/cosmos/cosmos-sdk/pull/24594) Fix NPE when initializing module via depinject.
+* (x/epochs) [#24610](https://github.com/cosmos/cosmos-sdk/pull/24610) Fix semantics of `CurrentEpochStartHeight` being set before epoch has started.
 
 ## [v0.50.13](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.13) - 2025-03-12
 

x/epochs/keeper/abci.go

@@ -13,25 +13,27 @@ func (k *Keeper) BeginBlocker(ctx sdk.Context) error {
 	start := telemetry.Now()
 	defer telemetry.ModuleMeasureSince(types.ModuleName, start, telemetry.MetricKeyBeginBlocker)
 
-	header := ctx.BlockHeader()
+	blockTime := ctx.BlockTime()
+	blockHeight := ctx.BlockHeight()
+
 	err := k.EpochInfo.Walk(
 		ctx,
 		nil,
 		func(key string, epochInfo types.EpochInfo) (stop bool, err error) {
 			// If blocktime < initial epoch start time, return
-			if header.Time.Before(epochInfo.StartTime) {
+			if blockTime.Before(epochInfo.StartTime) {
 				return false, nil
 			}
 			// if epoch counting hasn't started, signal we need to start.
 			shouldInitialEpochStart := !epochInfo.EpochCountingStarted
 
 			epochEndTime := epochInfo.CurrentEpochStartTime.Add(epochInfo.Duration)
-			shouldEpochStart := (header.Time.After(epochEndTime)) || shouldInitialEpochStart
+			shouldEpochStart := (blockTime.After(epochEndTime)) || shouldInitialEpochStart
 
 			if !shouldEpochStart {
 				return false, nil
 			}
-			epochInfo.CurrentEpochStartHeight = header.Height
+			epochInfo.CurrentEpochStartHeight = blockHeight
 
 			if shouldInitialEpochStart {
 				epochInfo.EpochCountingStarted = true

x/epochs/keeper/abci_test.go

@@ -15,10 +15,12 @@ import (
 // of their initial conditions, and subsequent block height / times.
 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
+	const (
+		defaultIdentifier = "hourly"
+		defaultDuration   = time.Hour
+		// eps is short for epsilon - in this case a negligible amount of time.
+		eps = time.Nanosecond
+	)
 
 	tests := map[string]struct {
 		// if identifier, duration is not set, we make it defaultIdentifier and defaultDuration.
@@ -69,8 +71,8 @@ func (suite *KeeperTestSuite) TestEpochInfoBeginBlockChanges() {
 		},
 		"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 that's when the timer was created on-chain
-			expEpochInfo: types.EpochInfo{StartTime: block1Time.Add(time.Second), CurrentEpoch: 0, CurrentEpochStartTime: time.Time{}, CurrentEpochStartHeight: 1},
+			// currentEpochStartHeight is 0 since it hasn't started or been triggered
+			expEpochInfo: types.EpochInfo{StartTime: block1Time.Add(time.Second), CurrentEpoch: 0, CurrentEpochStartTime: time.Time{}, CurrentEpochStartHeight: 0},
 		},
 		"StartTime in past will get ticked on first block": {
 			initialEpochInfo: types.EpochInfo{StartTime: block1Time.Add(-time.Second), CurrentEpoch: 0, CurrentEpochStartTime: time.Time{}},

x/epochs/keeper/epoch.go

@@ -33,15 +33,15 @@ func (k *Keeper) AddEpochInfo(ctx sdk.Context, epoch types.EpochInfo) error {
 	if epoch.StartTime.IsZero() {
 		epoch.StartTime = ctx.BlockTime()
 	}
-	if epoch.CurrentEpochStartHeight == 0 {
+	if epoch.CurrentEpochStartHeight == 0 && !epoch.StartTime.After(ctx.BlockTime()) {
 		epoch.CurrentEpochStartHeight = ctx.BlockHeight()
 	}
 	return k.EpochInfo.Set(ctx, epoch.Identifier, epoch)
 }
 
 // AllEpochInfos iterate through epochs to return all epochs info.
 func (k *Keeper) AllEpochInfos(ctx sdk.Context) ([]types.EpochInfo, error) {
-	epochs := []types.EpochInfo{}
+	var epochs []types.EpochInfo
 	err := k.EpochInfo.Walk(
 		ctx,
 		nil,
@@ -62,5 +62,9 @@ func (k *Keeper) NumBlocksSinceEpochStart(ctx sdk.Context, identifier string) (i
 	if err != nil {
 		return 0, fmt.Errorf("epoch with identifier %s not found", identifier)
 	}
+	if ctx.BlockTime().Before(epoch.StartTime) {
+		return 0, fmt.Errorf("epoch with identifier %s has not started yet: start time: %s", identifier, epoch.StartTime)
+	}
+
 	return ctx.BlockHeight() - epoch.CurrentEpochStartHeight, nil
 }

x/epochs/keeper/epoch_test.go

@@ -49,6 +49,27 @@ func (s *KeeperTestSuite) TestAddEpochInfo() {
 			},
 			expErr: true,
 		},
+		"start in future": {
+			addedEpochInfo: types.EpochInfo{
+				Identifier:              defaultIdentifier,
+				StartTime:               startBlockTime.Add(time.Hour),
+				Duration:                defaultDuration,
+				CurrentEpoch:            0,
+				CurrentEpochStartHeight: 0,
+				CurrentEpochStartTime:   time.Time{},
+				EpochCountingStarted:    false,
+			},
+			expEpochInfo: types.EpochInfo{
+				Identifier:              defaultIdentifier,
+				StartTime:               startBlockTime.Add(time.Hour),
+				Duration:                defaultDuration,
+				CurrentEpoch:            0,
+				CurrentEpochStartHeight: 0,
+				CurrentEpochStartTime:   time.Time{},
+				EpochCountingStarted:    false,
+			},
+			expErr: false,
+		},
 	}
 	for name, test := range tests {
 		s.Run(name, func() {
@@ -99,3 +120,89 @@ func (s *KeeperTestSuite) TestEpochLifeCycle() {
 	s.Require().Equal(allEpochs[3].Identifier, "monthly")
 	s.Require().Equal(allEpochs[4].Identifier, "week")
 }
+
+func (s *KeeperTestSuite) TestNumBlocksSinceEpochStart() {
+	s.SetupTest()
+
+	startBlockHeight := int64(100)
+	startBlockTime := time.Unix(1656907200, 0).UTC()
+	duration := time.Hour
+
+	s.Ctx = s.Ctx.WithBlockHeight(startBlockHeight).WithBlockTime(startBlockTime)
+
+	tests := map[string]struct {
+		setupEpoch        types.EpochInfo
+		advanceBlockDelta int64
+		advanceTimeDelta  time.Duration
+		expErr            bool
+		expBlocksSince    int64
+	}{
+		"same block as start": {
+			setupEpoch: types.EpochInfo{
+				Identifier:              "epoch_same_block",
+				StartTime:               startBlockTime,
+				Duration:                duration,
+				CurrentEpoch:            0,
+				CurrentEpochStartHeight: startBlockHeight,
+				CurrentEpochStartTime:   startBlockTime,
+				EpochCountingStarted:    true,
+			},
+			advanceBlockDelta: 0,
+			advanceTimeDelta:  0,
+			expErr:            false,
+			expBlocksSince:    0,
+		},
+		"after 5 blocks": {
+			setupEpoch: types.EpochInfo{
+				Identifier:              "epoch_after_five",
+				StartTime:               startBlockTime,
+				Duration:                duration,
+				CurrentEpoch:            0,
+				CurrentEpochStartHeight: startBlockHeight,
+				CurrentEpochStartTime:   startBlockTime,
+				EpochCountingStarted:    true,
+			},
+			advanceBlockDelta: 5,
+			advanceTimeDelta:  time.Minute * 5, // just to simulate realistic advancement
+			expErr:            false,
+			expBlocksSince:    5,
+		},
+		"epoch not started yet": {
+			setupEpoch: types.EpochInfo{
+				Identifier:              "epoch_future",
+				StartTime:               startBlockTime.Add(time.Hour),
+				Duration:                duration,
+				CurrentEpoch:            0,
+				CurrentEpochStartHeight: 0,
+				CurrentEpochStartTime:   time.Time{},
+				EpochCountingStarted:    false,
+			},
+			advanceBlockDelta: 0,
+			advanceTimeDelta:  0,
+			expErr:            true,
+			expBlocksSince:    0,
+		},
+	}
+
+	for name, tc := range tests {
+		s.Run(name, func() {
+			s.SetupTest()
+			s.Ctx = s.Ctx.WithBlockHeight(startBlockHeight).WithBlockTime(startBlockTime)
+
+			err := s.EpochsKeeper.AddEpochInfo(s.Ctx, tc.setupEpoch)
+			s.Require().NoError(err)
+
+			// Advance block height and time if needed
+			s.Ctx = s.Ctx.WithBlockHeight(startBlockHeight + tc.advanceBlockDelta).
+				WithBlockTime(startBlockTime.Add(tc.advanceTimeDelta))
+
+			blocksSince, err := s.EpochsKeeper.NumBlocksSinceEpochStart(s.Ctx, tc.setupEpoch.Identifier)
+			if tc.expErr {
+				s.Require().Error(err)
+			} else {
+				s.Require().NoError(err)
+				s.Require().Equal(tc.expBlocksSince, blocksSince)
+			}
+		})
+	}
+}

x/epochs/types/genesis.go

@@ -5,14 +5,11 @@ import (
 	"time"
 )
 
-// DefaultIndex is the default capability global index.
-const DefaultIndex uint64 = 1
-
 func NewGenesisState(epochs []EpochInfo) *GenesisState {
 	return &GenesisState{Epochs: epochs}
 }
 
-// DefaultGenesis returns the default Capability genesis state.
+// DefaultGenesis returns the default Epochs genesis state.
 func DefaultGenesis() *GenesisState {
 	epochs := []EpochInfo{
 		NewGenesisEpochInfo("day", time.Hour*24), // alphabetical order