refactor: GetSelfConsensusState return error instead of bool

CHANGELOG.md

@@ -43,6 +43,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 ### API Breaking
 
 * (transfer) [\#517](https://github.com/cosmos/ibc-go/pull/517) Separates the ICS 26 callback functions from `AppModule` into a new type `IBCModule` for ICS 20 transfer.
+* (modules/core/02-client) [\#536](https://github.com/cosmos/ibc-go/pull/536) GetSelfConsensusState return type changed from bool to error.
 
 ### State Machine Breaking
 

modules/core/02-client/keeper/keeper.go

@@ -241,27 +241,27 @@ func (k Keeper) GetLatestClientConsensusState(ctx sdk.Context, clientID string)
 // GetSelfConsensusState introspects the (self) past historical info at a given height
 // and returns the expected consensus state at that height.
 // For now, can only retrieve self consensus states for the current revision
-func (k Keeper) GetSelfConsensusState(ctx sdk.Context, height exported.Height) (exported.ConsensusState, bool) {
+func (k Keeper) GetSelfConsensusState(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error) {
  selfHeight, ok := height.(types.Height)
  if !ok {
- return nil, false
+ return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", types.Height{}, height)
  }
  // check that height revision matches chainID revision
  revision := types.ParseChainID(ctx.ChainID())
  if revision != height.GetRevisionNumber() {
- return nil, false
+ return nil, sdkerrors.Wrapf(types.ErrInvalidHeight, "chainID revision number does not match height revision number: expected %d, got %d", revision, height.GetRevisionNumber())
  }
  histInfo, found := k.stakingKeeper.GetHistoricalInfo(ctx, int64(selfHeight.RevisionHeight))
  if !found {
- return nil, false
+ return nil, sdkerrors.Wrapf(sdkerrors.ErrNotFound, "no historical info found at height %d", selfHeight.RevisionHeight)
  }
 
  consensusState := &ibctmtypes.ConsensusState{
  Timestamp: histInfo.Header.Time,
  Root: commitmenttypes.NewMerkleRoot(histInfo.Header.GetAppHash()),
  NextValidatorsHash: histInfo.Header.NextValidatorsHash,
  }
- return consensusState, true
+ return consensusState, nil
 }
 
 // ValidateSelfClient validates the client parameters for a client of the running chain

modules/core/02-client/keeper/keeper_test.go

@@ -306,12 +306,12 @@ func (suite KeeperTestSuite) TestGetConsensusState() {
 
  for i, tc := range cases {
  tc := tc
- cs, found := suite.keeper.GetSelfConsensusState(suite.ctx, tc.height)
+ cs, err := suite.keeper.GetSelfConsensusState(suite.ctx, tc.height)
  if tc.expPass {
- suite.Require().True(found, "Case %d should have passed: %s", i, tc.name)
+ suite.Require().NoError(err, "Case %d should have passed: %s", i, tc.name)
  suite.Require().NotNil(cs, "Case %d should have passed: %s", i, tc.name)
  } else {
- suite.Require().False(found, "Case %d should have failed: %s", i, tc.name)
+ suite.Require().Error(err, "Case %d should have failed: %s", i, tc.name)
  suite.Require().Nil(cs, "Case %d should have failed: %s", i, tc.name)
  }
  }

modules/core/03-connection/keeper/handshake.go

@@ -125,9 +125,9 @@ func (k Keeper) ConnOpenTry(
  return "", err
  }
 
- expectedConsensusState, found := k.clientKeeper.GetSelfConsensusState(ctx, consensusHeight)
- if !found {
- return "", sdkerrors.Wrap(clienttypes.ErrSelfConsensusStateNotFound, consensusHeight.String())
+ expectedConsensusState, err := k.clientKeeper.GetSelfConsensusState(ctx, consensusHeight)
+ if err != nil {
+ return "", sdkerrors.Wrapf(err, "self consensus state not found for height %s", consensusHeight.String())
  }
 
  // expectedConnection defines Chain A's ConnectionEnd
@@ -250,9 +250,9 @@ func (k Keeper) ConnOpenAck(
  }
 
  // Retrieve chainA's consensus state at consensusheight
- expectedConsensusState, found := k.clientKeeper.GetSelfConsensusState(ctx, consensusHeight)
- if !found {
- return clienttypes.ErrSelfConsensusStateNotFound
+ expectedConsensusState, err := k.clientKeeper.GetSelfConsensusState(ctx, consensusHeight)
+ if err != nil {
+ return sdkerrors.Wrapf(err, "self consensus state not found for height %s", consensusHeight.String())
  }
 
  prefix := k.GetCommitmentPrefix()

modules/core/03-connection/keeper/verify_test.go

@@ -139,10 +139,10 @@ func (suite *KeeperTestSuite) TestVerifyClientConsensusState() {
 
  proof, consensusHeight := suite.chainB.QueryConsensusStateProof(path.EndpointB.ClientID)
  proofHeight := clienttypes.NewHeight(0, uint64(suite.chainB.GetContext().BlockHeight()-1))
- consensusState, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetSelfConsensusState(suite.chainA.GetContext(), consensusHeight)
- suite.Require().True(found)
+ consensusState, err := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetSelfConsensusState(suite.chainA.GetContext(), consensusHeight)
+ suite.Require().NoError(err)
 
- err := suite.chainA.App.GetIBCKeeper().ConnectionKeeper.VerifyClientConsensusState(
+ err = suite.chainA.App.GetIBCKeeper().ConnectionKeeper.VerifyClientConsensusState(
  suite.chainA.GetContext(), connection,
  malleateHeight(proofHeight, heightDiff), consensusHeight, proof, consensusState,
  )

modules/core/03-connection/types/expected_keepers.go

@@ -9,7 +9,7 @@ import (
 type ClientKeeper interface {
  GetClientState(ctx sdk.Context, clientID string) (exported.ClientState, bool)
  GetClientConsensusState(ctx sdk.Context, clientID string, height exported.Height) (exported.ConsensusState, bool)
- GetSelfConsensusState(ctx sdk.Context, height exported.Height) (exported.ConsensusState, bool)
+ GetSelfConsensusState(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error)
  ValidateSelfClient(ctx sdk.Context, clientState exported.ClientState) error
  IterateClients(ctx sdk.Context, cb func(string, exported.ClientState) bool)
  ClientStore(ctx sdk.Context, clientID string) sdk.KVStore