Skip to content

Commit

Permalink
consensus/cometbft: Fail ImmutableState creation if version is missing
Browse files Browse the repository at this point in the history
  • Loading branch information
ptrus committed Feb 8, 2025
1 parent cdf14c0 commit 0eb80e6
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 8 deletions.
8 changes: 8 additions & 0 deletions .changelog/6040.bugfix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
go/consensus/cometbft: Fail ImmutableState creation if version is missing

Previously, when an `ImmutableState` was requested for a block version that
didn't exist, the function would silently default to the latest available
block. This could lead to inconsistencies since clients might receive state
for a different block than expected. With this change, calls to create
an `ImmutableState` for a missing version now explicitly fail with a
"version not found" error, ensuring that such cases are handled properly.
21 changes: 16 additions & 5 deletions go/consensus/cometbft/abci/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,28 +278,39 @@ func (s *applicationState) GetEpoch(ctx context.Context, blockHeight int64) (bea
return s.timeSource.GetEpoch(ctx, blockHeight)
}

func (s *applicationState) GetCurrentEpoch(ctx context.Context) (beacon.EpochTime, error) {
func (s *applicationState) getCurrentEpoch(ctx context.Context, inCommitPhase bool) (beacon.EpochTime, error) {
blockHeight := s.BlockHeight()
if blockHeight == 0 {
return beacon.EpochInvalid, nil
}
latestStateHeight := blockHeight
// If we are not in the commit phase, then the latest state height is the next height.
// Otherwise, it is the current height.
if !inCommitPhase {
latestStateHeight++
}

// Check if there is an epoch transition scheduled for the current height. This should be taken
// into account when GetCurrentEpoch is called before the time keeping app does the transition.
future, err := s.timeSource.GetFutureEpoch(ctx, blockHeight+1)
future, err := s.timeSource.GetFutureEpoch(ctx, latestStateHeight)
if err != nil {
return beacon.EpochInvalid, fmt.Errorf("failed to get future epoch for height %d: %w", blockHeight+1, err)
return beacon.EpochInvalid, fmt.Errorf("failed to get future epoch for height %d: %w", latestStateHeight, err)
}
if future != nil && future.Height == blockHeight+1 {
return future.Epoch, nil
}

currentEpoch, err := s.timeSource.GetEpoch(ctx, blockHeight+1)
currentEpoch, err := s.timeSource.GetEpoch(ctx, latestStateHeight)
if err != nil {
return beacon.EpochInvalid, fmt.Errorf("failed to get epoch for height %d: %w", blockHeight+1, err)
return beacon.EpochInvalid, fmt.Errorf("failed to get epoch for height %d: %w", latestStateHeight, err)

Check warning on line 305 in go/consensus/cometbft/abci/state.go

View check run for this annotation

Codecov / codecov/patch

go/consensus/cometbft/abci/state.go#L305

Added line #L305 was not covered by tests
}
return currentEpoch, nil
}

func (s *applicationState) GetCurrentEpoch(ctx context.Context) (beacon.EpochTime, error) {
return s.getCurrentEpoch(ctx, false)
}

func (s *applicationState) EpochChanged(ctx *api.Context) (bool, beacon.EpochTime) {
blockHeight := s.BlockHeight()
if blockHeight == 0 {
Expand Down
2 changes: 1 addition & 1 deletion go/consensus/cometbft/abci/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func (mux *abciMux) maybeHaltForUpgrade() {
}

// Determine the epoch that will become the current epoch in the next block.
epoch, err := mux.state.GetCurrentEpoch(context.Background())
epoch, err := mux.state.getCurrentEpoch(context.Background(), true)
if err != nil {
mux.logger.Warn("failed to get current epoch",
"err", err,
Expand Down
5 changes: 4 additions & 1 deletion go/consensus/cometbft/api/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,10 @@ func NewImmutableState(ctx context.Context, state ApplicationQueryState, version
if state.BlockHeight() == 0 {
return nil, consensus.ErrNoCommittedBlocks
}
if version <= 0 || version > state.BlockHeight() {
if version > state.BlockHeight() {
return nil, consensus.ErrVersionNotFound

Check warning on line 298 in go/consensus/cometbft/api/state.go

View check run for this annotation

Codecov / codecov/patch

go/consensus/cometbft/api/state.go#L298

Added line #L298 was not covered by tests
}
if version <= 0 {
version = state.BlockHeight()
}

Expand Down
2 changes: 1 addition & 1 deletion go/consensus/cometbft/apps/staking/slashing.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func onEvidenceByzantineConsensus(
// validator from being scheduled in the next epoch.
if penalty.FreezeInterval > 0 {
var epoch beacon.EpochTime
epoch, err = ctx.AppState().GetEpoch(context.Background(), ctx.BlockHeight()+1)
epoch, err = ctx.AppState().GetEpoch(context.Background(), ctx.BlockHeight())
if err != nil {
return err
}
Expand Down

0 comments on commit 0eb80e6

Please sign in to comment.