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.

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

chore: remove error return in IterateConsensusStateAscending #1896

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (06-solomachine) [\#1100](https://github.com/cosmos/ibc-go/pull/1100) Deprecate `ClientId` field in 06-solomachine `Misbehaviour` type.
* (07-tendermint) [\#1097](https://github.com/cosmos/ibc-go/pull/1097) Remove `GetClientID` function from 07-tendermint `Misbehaviour` type.
* (07-tendermint) [\#1097](https://github.com/cosmos/ibc-go/pull/1097) Deprecate `ClientId` field in 07-tendermint `Misbehaviour` type.
* (modules/core/exported) [\#1107](https://github.com/cosmos/ibc-go/pull/1107) Merging the `Header` and `Misbehaviour` interfaces into a single `ClientMessage` type
* (modules/core/exported) [\#1107](https://github.com/cosmos/ibc-go/pull/1107) Merging the `Header` and `Misbehaviour` interfaces into a single `ClientMessage` type.
* (07-tendermint) [\#1896](https://github.com/cosmos/ibc-go/pull/1896) Remove error return from `IterateConsensusStateAscending` in `07-tendermint`.

### State Machine Breaking

Expand Down
5 changes: 1 addition & 4 deletions modules/core/02-client/legacy/v100/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,7 @@ func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.Binar
// add iteration keys so pruning will be successful
addConsensusMetadata(ctx, clientStore)

if err = ibctm.PruneAllExpiredConsensusStates(ctx, clientStore, cdc, tmClientState); err != nil {
return err
}

ibctm.PruneAllExpiredConsensusStates(ctx, clientStore, cdc, tmClientState)
default:
continue
}
Expand Down
14 changes: 4 additions & 10 deletions modules/light-clients/07-tendermint/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,18 +215,17 @@ func GetHeightFromIterationKey(iterKey []byte) exported.Height {

// IterateConsensusStateAscending iterates through the consensus states in ascending order. It calls the provided
// callback on each height, until stop=true is returned.
func IterateConsensusStateAscending(clientStore sdk.KVStore, cb func(height exported.Height) (stop bool)) error {
func IterateConsensusStateAscending(clientStore sdk.KVStore, cb func(height exported.Height) (stop bool)) {
iterator := sdk.KVStorePrefixIterator(clientStore, []byte(KeyIterateConsensusStatePrefix))
defer iterator.Close()

for ; iterator.Valid(); iterator.Next() {
iterKey := iterator.Key()
height := GetHeightFromIterationKey(iterKey)
if cb(height) {
return nil
break
}
}
return nil
}

// GetNextConsensusState returns the lowest consensus state that is larger than the given height.
Expand Down Expand Up @@ -278,7 +277,7 @@ func GetPreviousConsensusState(clientStore sdk.KVStore, cdc codec.BinaryCodec, h
func PruneAllExpiredConsensusStates(
ctx sdk.Context, clientStore sdk.KVStore,
cdc codec.BinaryCodec, clientState *ClientState,
) (err error) {
) {
var heights []exported.Height

pruneCb := func(height exported.Height) bool {
Expand All @@ -294,17 +293,12 @@ func PruneAllExpiredConsensusStates(
return false
}

err = IterateConsensusStateAscending(clientStore, pruneCb)
if err != nil {
return err
}
IterateConsensusStateAscending(clientStore, pruneCb)

for _, height := range heights {
deleteConsensusState(clientStore, height)
deleteConsensusMetadata(clientStore, height)
}

return nil
}

// Helper function for GetNextConsensusState and GetPreviousConsensusState
Expand Down
4 changes: 1 addition & 3 deletions modules/light-clients/07-tendermint/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,7 @@ func (cs ClientState) pruneOldestConsensusState(ctx sdk.Context, cdc codec.Binar
return true
}

if err := IterateConsensusStateAscending(clientStore, pruneCb); err != nil {
panic(err)
}
IterateConsensusStateAscending(clientStore, pruneCb)

// if pruneHeight is set, delete consensus state and metadata
if pruneHeight != nil {
Expand Down
3 changes: 1 addition & 2 deletions modules/light-clients/07-tendermint/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -488,8 +488,7 @@ func (suite *TendermintTestSuite) TestPruneConsensusState() {
}
ctx := path.EndpointA.Chain.GetContext()
clientStore := path.EndpointA.Chain.App.GetIBCKeeper().ClientKeeper.ClientStore(ctx, path.EndpointA.ClientID)
err := ibctm.IterateConsensusStateAscending(clientStore, getFirstHeightCb)
suite.Require().Nil(err)
ibctm.IterateConsensusStateAscending(clientStore, getFirstHeightCb)

// this height will be expired but not pruned
path.EndpointA.UpdateClient()
Expand Down