Skip to content

Commit

Permalink
chore: remove error return in IterateConsensusStateAscending (#1896)
Browse files Browse the repository at this point in the history
* removing error return in tendermint store.go

* updating tests and legacy code

* updating changelog
  • Loading branch information
damiannolan authored Aug 8, 2022
1 parent fc4bfaf commit 9a4ed68
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 20 deletions.
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

0 comments on commit 9a4ed68

Please sign in to comment.