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 10, 2025
1 parent cdf14c0 commit 0e95d83
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 5 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.
14 changes: 11 additions & 3 deletions go/consensus/cometbft/abci/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,17 +283,25 @@ func (s *applicationState) GetCurrentEpoch(ctx context.Context) (beacon.EpochTim
if blockHeight == 0 {
return beacon.EpochInvalid, nil
}

latestHeight := blockHeight
if abciCtx := api.FromCtx(ctx); abciCtx != nil {
// If request was made from an ABCI application context, then use blockHeight + 1,
// to fetch the epoch at current (future) height.
latestHeight++
}

// 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, latestHeight)
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", latestHeight, 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, latestHeight)
if err != nil {
return beacon.EpochInvalid, fmt.Errorf("failed to get epoch for height %d: %w", blockHeight+1, 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
}
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 0e95d83

Please sign in to comment.