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

refactor: GetSelfConsensusState return error instead of bool #536

Merged
Merged
Show file tree
Hide file tree
Changes from 4 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
10 changes: 5 additions & 5 deletions modules/core/02-client/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.ErrInvalidType.Wrapf("unable cast %T to %T", height, types.Height{})
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
}
// 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
Expand Down
6 changes: 3 additions & 3 deletions modules/core/02-client/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
12 changes: 6 additions & 6 deletions modules/core/03-connection/keeper/handshake.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
6 changes: 3 additions & 3 deletions modules/core/03-connection/keeper/verify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down
2 changes: 1 addition & 1 deletion modules/core/03-connection/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down