Skip to content

Commit

Permalink
ibc: remove root from verification funcs (cosmos#7780)
Browse files Browse the repository at this point in the history
* ibc: remove root from verification funcs

* fix VerifyClientConsensusState

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
fedekunze and mergify[bot] authored Nov 2, 2020
1 parent e9801ed commit 4420fe2
Show file tree
Hide file tree
Showing 10 changed files with 25 additions and 41 deletions.
14 changes: 2 additions & 12 deletions x/ibc/core/03-connection/keeper/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,8 @@ func (k Keeper) VerifyClientState(
return sdkerrors.Wrap(clienttypes.ErrClientNotFound, clientID)
}

targetConsState, found := k.clientKeeper.GetClientConsensusState(ctx, clientID, height)
if !found {
return sdkerrors.Wrapf(clienttypes.ErrConsensusStateNotFound, "clientID: %s with height: %s", clientID, height)
}

if err := targetClient.VerifyClientState(
k.clientKeeper.ClientStore(ctx, clientID), k.cdc, targetConsState.GetRoot(), height,
k.clientKeeper.ClientStore(ctx, clientID), k.cdc, height,
connection.GetCounterparty().GetPrefix(), connection.GetCounterparty().GetClientID(), proof, clientState); err != nil {
return sdkerrors.Wrapf(err, "failed client state verification for target client: %s", connection.GetClientID())
}
Expand All @@ -52,13 +47,8 @@ func (k Keeper) VerifyClientConsensusState(
return sdkerrors.Wrap(clienttypes.ErrClientNotFound, clientID)
}

targetConsState, found := k.clientKeeper.GetClientConsensusState(ctx, clientID, height)
if !found {
return sdkerrors.Wrapf(clienttypes.ErrConsensusStateNotFound, "clientID: %s with height: %s", clientID, height)
}

if err := clientState.VerifyClientConsensusState(
k.clientKeeper.ClientStore(ctx, clientID), k.cdc, targetConsState.GetRoot(), height,
k.clientKeeper.ClientStore(ctx, clientID), k.cdc, height,
connection.GetCounterparty().GetClientID(), consensusHeight, connection.GetCounterparty().GetPrefix(), proof, consensusState,
); err != nil {
return sdkerrors.Wrapf(err, "failed consensus state verification for client (%s)", connection.GetClientID())
Expand Down
2 changes: 0 additions & 2 deletions x/ibc/core/exported/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ type ClientState interface {
VerifyClientState(
store sdk.KVStore,
cdc codec.BinaryMarshaler,
root Root,
height Height,
prefix Prefix,
counterpartyClientIdentifier string,
Expand All @@ -61,7 +60,6 @@ type ClientState interface {
VerifyClientConsensusState(
store sdk.KVStore,
cdc codec.BinaryMarshaler,
root Root,
height Height,
counterpartyClientIdentifier string,
consensusHeight Height,
Expand Down
2 changes: 0 additions & 2 deletions x/ibc/light-clients/06-solomachine/types/client_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ func (cs ClientState) VerifyUpgrade(
func (cs ClientState) VerifyClientState(
store sdk.KVStore,
cdc codec.BinaryMarshaler,
_ exported.Root,
height exported.Height,
prefix exported.Prefix,
counterpartyClientIdentifier string,
Expand Down Expand Up @@ -127,7 +126,6 @@ func (cs ClientState) VerifyClientState(
func (cs ClientState) VerifyClientConsensusState(
store sdk.KVStore,
cdc codec.BinaryMarshaler,
_ exported.Root,
height exported.Height,
counterpartyClientIdentifier string,
consensusHeight exported.Height,
Expand Down
4 changes: 2 additions & 2 deletions x/ibc/light-clients/06-solomachine/types/client_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ func (suite *SoloMachineTestSuite) TestVerifyClientState() {
}

err := tc.clientState.VerifyClientState(
suite.store, suite.chainA.Codec, nil, solomachine.GetHeight(), tc.prefix, counterpartyClientIdentifier, tc.proof, clientState,
suite.store, suite.chainA.Codec, solomachine.GetHeight(), tc.prefix, counterpartyClientIdentifier, tc.proof, clientState,
)

if tc.expPass {
Expand Down Expand Up @@ -320,7 +320,7 @@ func (suite *SoloMachineTestSuite) TestVerifyClientConsensusState() {
}

err := tc.clientState.VerifyClientConsensusState(
suite.store, suite.chainA.Codec, nil, solomachine.GetHeight(), counterpartyClientIdentifier, consensusHeight, tc.prefix, tc.proof, consensusState,
suite.store, suite.chainA.Codec, solomachine.GetHeight(), counterpartyClientIdentifier, consensusHeight, tc.prefix, tc.proof, consensusState,
)

if tc.expPass {
Expand Down
10 changes: 4 additions & 6 deletions x/ibc/light-clients/07-tendermint/types/client_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,14 +169,13 @@ func (cs ClientState) ZeroCustomFields() exported.ClientState {
func (cs ClientState) VerifyClientState(
store sdk.KVStore,
cdc codec.BinaryMarshaler,
provingRoot exported.Root,
height exported.Height,
prefix exported.Prefix,
counterpartyClientIdentifier string,
proof []byte,
clientState exported.ClientState,
) error {
merkleProof, _, err := produceVerificationArgs(store, cdc, cs, height, prefix, proof)
merkleProof, provingConsensusState, err := produceVerificationArgs(store, cdc, cs, height, prefix, proof)
if err != nil {
return err
}
Expand All @@ -201,23 +200,22 @@ func (cs ClientState) VerifyClientState(
return err
}

return merkleProof.VerifyMembership(cs.ProofSpecs, provingRoot, path, bz)
return merkleProof.VerifyMembership(cs.ProofSpecs, provingConsensusState.GetRoot(), path, bz)
}

// VerifyClientConsensusState verifies a proof of the consensus state of the
// Tendermint client stored on the target machine.
func (cs ClientState) VerifyClientConsensusState(
store sdk.KVStore,
cdc codec.BinaryMarshaler,
provingRoot exported.Root,
height exported.Height,
counterpartyClientIdentifier string,
consensusHeight exported.Height,
prefix exported.Prefix,
proof []byte,
consensusState exported.ConsensusState,
) error {
merkleProof, _, err := produceVerificationArgs(store, cdc, cs, height, prefix, proof)
merkleProof, provingConsensusState, err := produceVerificationArgs(store, cdc, cs, height, prefix, proof)
if err != nil {
return err
}
Expand All @@ -242,7 +240,7 @@ func (cs ClientState) VerifyClientConsensusState(
return err
}

if err := merkleProof.VerifyMembership(cs.ProofSpecs, provingRoot, path, bz); err != nil {
if err := merkleProof.VerifyMembership(cs.ProofSpecs, provingConsensusState.GetRoot(), path, bz); err != nil {
return err
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func (suite *TendermintTestSuite) TestVerifyClientConsensusState() {
tc := tc

err := tc.clientState.VerifyClientConsensusState(
nil, suite.cdc, tc.consensusState.Root, height, "chainA", tc.clientState.LatestHeight, tc.prefix, tc.proof, tc.consensusState,
nil, suite.cdc, height, "chainA", tc.clientState.LatestHeight, tc.prefix, tc.proof, tc.consensusState,
)

if tc.expPass {
Expand Down
6 changes: 3 additions & 3 deletions x/ibc/light-clients/07-tendermint/types/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ func GetConsensusState(store sdk.KVStore, cdc codec.BinaryMarshaler, height expo
if bz == nil {
return nil, sdkerrors.Wrapf(
clienttypes.ErrConsensusStateNotFound,
"consensus state does not exist for height %d", height,
"consensus state does not exist for height %s", height,
)
}

var consensusStateI exported.ConsensusState
if err := codec.UnmarshalAny(cdc, &consensusStateI, bz); err != nil {
consensusStateI, err := clienttypes.UnmarshalConsensusState(cdc, bz)
if err != nil {
return nil, sdkerrors.Wrapf(clienttypes.ErrInvalidConsensus, "unmarshal error: %v", err)
}

Expand Down
18 changes: 9 additions & 9 deletions x/ibc/light-clients/07-tendermint/types/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
channeltypes "github.com/cosmos/cosmos-sdk/x/ibc/core/04-channel/types"
host "github.com/cosmos/cosmos-sdk/x/ibc/core/24-host"
"github.com/cosmos/cosmos-sdk/x/ibc/core/exported"
solomachinetypes "github.com/cosmos/cosmos-sdk/x/ibc/light-clients/06-solomachine/types"
"github.com/cosmos/cosmos-sdk/x/ibc/light-clients/07-tendermint/types"
)

Expand Down Expand Up @@ -36,15 +37,14 @@ func (suite *TendermintTestSuite) TestGetConsensusState() {
store.Set(host.KeyConsensusState(height), clientStateBz)
}, false,
},
// TODO: uncomment upon merge of solomachine
// {
// "invalid consensus state (solomachine)", func() {
// // marshal and set solomachine consensus state
// store := suite.chainA.App.IBCKeeper.ClientKeeper.ClientStore(suite.chainA.GetContext(), clientA)
// consensusStateBz := suite.chainA.App.IBCKeeper.ClientKeeper.MustMarshalConsensusState(&solomachinetypes.ConsensusState{})
// store.Set(host.KeyConsensusState(height), consensusStateBz)
// }, false,
// },
{
"invalid consensus state (solomachine)", func() {
// marshal and set solomachine consensus state
store := suite.chainA.App.IBCKeeper.ClientKeeper.ClientStore(suite.chainA.GetContext(), clientA)
consensusStateBz := suite.chainA.App.IBCKeeper.ClientKeeper.MustMarshalConsensusState(&solomachinetypes.ConsensusState{})
store.Set(host.KeyConsensusState(height), consensusStateBz)
}, false,
},
}

for _, tc := range testCases {
Expand Down
4 changes: 2 additions & 2 deletions x/ibc/light-clients/09-localhost/types/client_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (cs ClientState) VerifyUpgrade(

// VerifyClientState verifies that the localhost client state is stored locally
func (cs ClientState) VerifyClientState(
store sdk.KVStore, cdc codec.BinaryMarshaler, _ exported.Root,
store sdk.KVStore, cdc codec.BinaryMarshaler,
_ exported.Height, _ exported.Prefix, _ string, _ []byte, clientState exported.ClientState,
) error {
path := host.KeyClientState()
Expand All @@ -136,7 +136,7 @@ func (cs ClientState) VerifyClientState(
// VerifyClientConsensusState returns nil since a local host client does not store consensus
// states.
func (cs ClientState) VerifyClientConsensusState(
sdk.KVStore, codec.BinaryMarshaler, exported.Root,
sdk.KVStore, codec.BinaryMarshaler,
exported.Height, string, exported.Height, exported.Prefix,
[]byte, exported.ConsensusState,
) error {
Expand Down
4 changes: 2 additions & 2 deletions x/ibc/light-clients/09-localhost/types/client_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func (suite *LocalhostTestSuite) TestVerifyClientState() {
tc.malleate()

err := tc.clientState.VerifyClientState(
suite.store, suite.cdc, nil, clienttypes.NewHeight(0, 10), nil, "", []byte{}, tc.counterparty,
suite.store, suite.cdc, clienttypes.NewHeight(0, 10), nil, "", []byte{}, tc.counterparty,
)

if tc.expPass {
Expand All @@ -114,7 +114,7 @@ func (suite *LocalhostTestSuite) TestVerifyClientState() {
func (suite *LocalhostTestSuite) TestVerifyClientConsensusState() {
clientState := types.NewClientState("chainID", clientHeight)
err := clientState.VerifyClientConsensusState(
nil, nil, nil, nil, "", nil, nil, nil, nil,
nil, nil, nil, "", nil, nil, nil, nil,
)
suite.Require().NoError(err)
}
Expand Down

0 comments on commit 4420fe2

Please sign in to comment.