From 9a4ed681e7aa440722739d109db00dbc2ed038b4 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Mon, 8 Aug 2022 14:28:21 +0200 Subject: [PATCH] chore: remove error return in IterateConsensusStateAscending (#1896) * removing error return in tendermint store.go * updating tests and legacy code * updating changelog --- CHANGELOG.md | 3 ++- modules/core/02-client/legacy/v100/store.go | 5 +---- modules/light-clients/07-tendermint/store.go | 14 ++++---------- modules/light-clients/07-tendermint/update.go | 4 +--- modules/light-clients/07-tendermint/update_test.go | 3 +-- 5 files changed, 9 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f57f64aee5..9845d1492ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/modules/core/02-client/legacy/v100/store.go b/modules/core/02-client/legacy/v100/store.go index b59f512f92d..e83b116c956 100644 --- a/modules/core/02-client/legacy/v100/store.go +++ b/modules/core/02-client/legacy/v100/store.go @@ -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 } diff --git a/modules/light-clients/07-tendermint/store.go b/modules/light-clients/07-tendermint/store.go index 33957568e2d..e8824d06e1d 100644 --- a/modules/light-clients/07-tendermint/store.go +++ b/modules/light-clients/07-tendermint/store.go @@ -215,7 +215,7 @@ 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() @@ -223,10 +223,9 @@ func IterateConsensusStateAscending(clientStore sdk.KVStore, cb func(height expo 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. @@ -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 { @@ -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 diff --git a/modules/light-clients/07-tendermint/update.go b/modules/light-clients/07-tendermint/update.go index 39481df125a..32b5857ad19 100644 --- a/modules/light-clients/07-tendermint/update.go +++ b/modules/light-clients/07-tendermint/update.go @@ -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 { diff --git a/modules/light-clients/07-tendermint/update_test.go b/modules/light-clients/07-tendermint/update_test.go index 306133542b5..0b5b55eb1cc 100644 --- a/modules/light-clients/07-tendermint/update_test.go +++ b/modules/light-clients/07-tendermint/update_test.go @@ -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()