diff --git a/modules/light-clients/07-tendermint/types/client_state.go b/modules/light-clients/07-tendermint/types/client_state.go index 51f826979fd..f0d0e79972f 100644 --- a/modules/light-clients/07-tendermint/types/client_state.go +++ b/modules/light-clients/07-tendermint/types/client_state.go @@ -76,8 +76,8 @@ func (cs ClientState) Status( } // get latest consensus state from clientStore to check for expiry - consState, err := GetConsensusState(clientStore, cdc, cs.GetLatestHeight()) - if err != nil { + consState, found := GetConsensusState(clientStore, cdc, cs.GetLatestHeight()) + if !found { // if the client state does not have an associated consensus state for its latest height // then it must be expired return exported.Expired @@ -574,9 +574,9 @@ func produceVerificationArgs( return commitmenttypes.MerkleProof{}, nil, sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "failed to unmarshal proof into commitment merkle proof") } - consensusState, err = GetConsensusState(store, cdc, height) - if err != nil { - return commitmenttypes.MerkleProof{}, nil, sdkerrors.Wrap(err, "please ensure the proof was constructed against a height that exists on the client") + consensusState, found := GetConsensusState(store, cdc, height) + if !found { + return commitmenttypes.MerkleProof{}, nil, sdkerrors.Wrap(clienttypes.ErrConsensusStateNotFound, "please ensure the proof was constructed against a height that exists on the client") } return merkleProof, consensusState, nil diff --git a/modules/light-clients/07-tendermint/types/misbehaviour_handle.go b/modules/light-clients/07-tendermint/types/misbehaviour_handle.go index 7dce230b6f7..71a8e5284f1 100644 --- a/modules/light-clients/07-tendermint/types/misbehaviour_handle.go +++ b/modules/light-clients/07-tendermint/types/misbehaviour_handle.go @@ -80,14 +80,14 @@ func (cs *ClientState) verifyMisbehaviour(ctx sdk.Context, clientStore sdk.KVSto // Regardless of the type of misbehaviour, ensure that both headers are valid and would have been accepted by light-client // Retrieve trusted consensus states for each Header in misbehaviour - tmConsensusState1, err := GetConsensusState(clientStore, cdc, misbehaviour.Header1.TrustedHeight) - if err != nil { - return sdkerrors.Wrapf(err, "could not get trusted consensus state from clientStore for Header1 at TrustedHeight: %s", misbehaviour.Header1.TrustedHeight) + tmConsensusState1, found := GetConsensusState(clientStore, cdc, misbehaviour.Header1.TrustedHeight) + if !found { + return sdkerrors.Wrapf(clienttypes.ErrConsensusStateNotFound, "could not get trusted consensus state from clientStore for Header1 at TrustedHeight: %s", misbehaviour.Header1.TrustedHeight) } - tmConsensusState2, err := GetConsensusState(clientStore, cdc, misbehaviour.Header2.TrustedHeight) - if err != nil { - return sdkerrors.Wrapf(err, "could not get trusted consensus state from clientStore for Header2 at TrustedHeight: %s", misbehaviour.Header2.TrustedHeight) + tmConsensusState2, found := GetConsensusState(clientStore, cdc, misbehaviour.Header2.TrustedHeight) + if !found { + return sdkerrors.Wrapf(clienttypes.ErrConsensusStateNotFound, "could not get trusted consensus state from clientStore for Header2 at TrustedHeight: %s", misbehaviour.Header2.TrustedHeight) } // Check the validity of the two conflicting headers against their respective diff --git a/modules/light-clients/07-tendermint/types/proposal_handle.go b/modules/light-clients/07-tendermint/types/proposal_handle.go index c01ec6fd8b7..563acce2c02 100644 --- a/modules/light-clients/07-tendermint/types/proposal_handle.go +++ b/modules/light-clients/07-tendermint/types/proposal_handle.go @@ -62,9 +62,9 @@ func (cs ClientState) CheckSubstituteAndUpdateState( // starting from initial height and ending on the latest height (inclusive) height := substituteClientState.GetLatestHeight() - consensusState, err := GetConsensusState(substituteClientStore, cdc, height) - if err != nil { - return nil, sdkerrors.Wrap(err, "unable to retrieve latest consensus state for substitute client") + consensusState, found := GetConsensusState(substituteClientStore, cdc, height) + if !found { + return nil, sdkerrors.Wrap(clienttypes.ErrConsensusStateNotFound, "unable to retrieve latest consensus state for substitute client") } setConsensusState(subjectClientStore, cdc, consensusState, height) diff --git a/modules/light-clients/07-tendermint/types/store.go b/modules/light-clients/07-tendermint/types/store.go index f2c954cc029..fabd29f3161 100644 --- a/modules/light-clients/07-tendermint/types/store.go +++ b/modules/light-clients/07-tendermint/types/store.go @@ -8,7 +8,6 @@ import ( "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/store/prefix" sdk "github.com/cosmos/cosmos-sdk/types" - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types" host "github.com/cosmos/ibc-go/v3/modules/core/24-host" @@ -57,31 +56,16 @@ func setConsensusState(clientStore sdk.KVStore, cdc codec.BinaryCodec, consensus clientStore.Set(key, val) } -// GetConsensusState retrieves the consensus state from the client prefixed -// store. An error is returned if the consensus state does not exist. -func GetConsensusState(store sdk.KVStore, cdc codec.BinaryCodec, height exported.Height) (*ConsensusState, error) { +// GetConsensusState retrieves the consensus state from the client prefixed store. +// If the ConsensusState does not exist in state for the provided height a nil value and false boolean flag is returned +func GetConsensusState(store sdk.KVStore, cdc codec.BinaryCodec, height exported.Height) (*ConsensusState, bool) { bz := store.Get(host.ConsensusStateKey(height)) if bz == nil { - return nil, sdkerrors.Wrapf( - clienttypes.ErrConsensusStateNotFound, - "consensus state does not exist for height %s", height, - ) - } - - consensusStateI, err := clienttypes.UnmarshalConsensusState(cdc, bz) - if err != nil { - return nil, sdkerrors.Wrapf(clienttypes.ErrInvalidConsensus, "unmarshal error: %v", err) - } - - consensusState, ok := consensusStateI.(*ConsensusState) - if !ok { - return nil, sdkerrors.Wrapf( - clienttypes.ErrInvalidConsensus, - "invalid consensus type %T, expected %T", consensusState, &ConsensusState{}, - ) + return nil, false } - return consensusState, nil + consensusStateI := clienttypes.MustUnmarshalConsensusState(cdc, bz) + return consensusStateI.(*ConsensusState), true } // deleteConsensusState deletes the consensus state at the given height @@ -299,9 +283,8 @@ func PruneAllExpiredConsensusStates( var heights []exported.Height pruneCb := func(height exported.Height) bool { - consState, err := GetConsensusState(clientStore, cdc, height) - // this error should never occur - if err != nil { + consState, found := GetConsensusState(clientStore, cdc, height) + if !found { // consensus state should always be found return true } diff --git a/modules/light-clients/07-tendermint/types/store_test.go b/modules/light-clients/07-tendermint/types/store_test.go index 22a8d069794..28877a49386 100644 --- a/modules/light-clients/07-tendermint/types/store_test.go +++ b/modules/light-clients/07-tendermint/types/store_test.go @@ -23,15 +23,16 @@ func (suite *TendermintTestSuite) TestGetConsensusState() { name string malleate func() expPass bool + expPanic bool }{ { - "success", func() {}, true, + "success", func() {}, true, false, }, { "consensus state not found", func() { // use height with no consensus state set height = height.(clienttypes.Height).Increment() - }, false, + }, false, false, }, { "not a consensus state interface", func() { @@ -39,7 +40,7 @@ func (suite *TendermintTestSuite) TestGetConsensusState() { store := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID) clientStateBz := suite.chainA.App.GetIBCKeeper().ClientKeeper.MustMarshalClientState(&types.ClientState{}) store.Set(host.ConsensusStateKey(height), clientStateBz) - }, false, + }, false, true, }, { "invalid consensus state (solomachine)", func() { @@ -47,7 +48,7 @@ func (suite *TendermintTestSuite) TestGetConsensusState() { store := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID) consensusStateBz := suite.chainA.App.GetIBCKeeper().ClientKeeper.MustMarshalConsensusState(&solomachinetypes.ConsensusState{}) store.Set(host.ConsensusStateKey(height), consensusStateBz) - }, false, + }, false, true, }, } @@ -64,16 +65,26 @@ func (suite *TendermintTestSuite) TestGetConsensusState() { tc.malleate() // change vars as necessary + if tc.expPanic { + suite.Require().Panics(func() { + store := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID) + types.GetConsensusState(store, suite.chainA.Codec, height) + }) + + return + } + store := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID) - consensusState, err := types.GetConsensusState(store, suite.chainA.Codec, height) + consensusState, found := types.GetConsensusState(store, suite.chainA.Codec, height) if tc.expPass { - suite.Require().NoError(err) + suite.Require().True(found) + expConsensusState, found := suite.chainA.GetConsensusState(path.EndpointA.ClientID, height) suite.Require().True(found) suite.Require().Equal(expConsensusState, consensusState) } else { - suite.Require().Error(err) + suite.Require().False(found) suite.Require().Nil(consensusState) } }) diff --git a/modules/light-clients/07-tendermint/types/update.go b/modules/light-clients/07-tendermint/types/update.go index 0a25cbc2518..ecd88312882 100644 --- a/modules/light-clients/07-tendermint/types/update.go +++ b/modules/light-clients/07-tendermint/types/update.go @@ -147,9 +147,9 @@ func (cs *ClientState) verifyHeader( currentTimestamp := ctx.BlockTime() // Retrieve trusted consensus states for each Header in misbehaviour - consState, err := GetConsensusState(clientStore, cdc, header.TrustedHeight) - if err != nil { - return sdkerrors.Wrapf(err, "could not get trusted consensus state from clientStore for Header at TrustedHeight: %s", header.TrustedHeight) + consState, found := GetConsensusState(clientStore, cdc, header.TrustedHeight) + if !found { + return sdkerrors.Wrapf(clienttypes.ErrConsensusStateNotFound, "could not get trusted consensus state from clientStore for Header at TrustedHeight: %s", header.TrustedHeight) } if err := checkTrustedHeader(header, consState); err != nil { @@ -282,10 +282,10 @@ func (cs ClientState) pruneOldestConsensusState(ctx sdk.Context, cdc codec.Binar ) pruneCb := func(height exported.Height) bool { - consState, err := GetConsensusState(clientStore, cdc, height) + consState, found := GetConsensusState(clientStore, cdc, height) // this error should never occur - if err != nil { - pruneError = err + if !found { + pruneError = sdkerrors.Wrapf(clienttypes.ErrConsensusStateNotFound, "failed to retrieve consensus state at height: %s", height) return true } diff --git a/modules/light-clients/07-tendermint/types/upgrade.go b/modules/light-clients/07-tendermint/types/upgrade.go index eab2e3681df..c3910aa8816 100644 --- a/modules/light-clients/07-tendermint/types/upgrade.go +++ b/modules/light-clients/07-tendermint/types/upgrade.go @@ -67,9 +67,9 @@ func (cs ClientState) VerifyUpgradeAndUpdateState( // Must prove against latest consensus state to ensure we are verifying against latest upgrade plan // This verifies that upgrade is intended for the provided revision, since committed client must exist // at this consensus state - consState, err := GetConsensusState(clientStore, cdc, lastHeight) - if err != nil { - return sdkerrors.Wrap(err, "could not retrieve consensus state for lastHeight") + consState, found := GetConsensusState(clientStore, cdc, lastHeight) + if !found { + return sdkerrors.Wrap(clienttypes.ErrConsensusStateNotFound, "could not retrieve consensus state for lastHeight") } // Verify client proof