From dbde0d1cf0a9169801b919405ef272a78288f6b3 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Tue, 12 Nov 2019 18:25:21 +0100 Subject: [PATCH 01/33] ibc client evidence route --- simapp/app.go | 6 +- x/ibc/02-client/alias.go | 11 +- x/ibc/02-client/client/cli/tx.go | 44 ------- x/ibc/02-client/exported/exported.go | 29 +---- x/ibc/02-client/handler.go | 39 +++--- x/ibc/02-client/keeper/client.go | 33 +++-- x/ibc/02-client/keeper/keeper.go | 19 --- x/ibc/02-client/types/codec.go | 1 - x/ibc/02-client/types/events.go | 2 +- x/ibc/02-client/types/keys.go | 8 +- x/ibc/02-client/types/msgs.go | 53 -------- x/ibc/02-client/types/msgs_test.go | 122 +++++++++--------- .../types/tendermint/misbehaviour.go | 37 +++--- x/ibc/handler.go | 3 - 14 files changed, 138 insertions(+), 269 deletions(-) diff --git a/simapp/app.go b/simapp/app.go index 3248b1a08f5e..03c062389409 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -24,6 +24,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/genutil" "github.com/cosmos/cosmos-sdk/x/gov" "github.com/cosmos/cosmos-sdk/x/ibc" + ibcclient "github.com/cosmos/cosmos-sdk/x/ibc/02-client" ibctransfer "github.com/cosmos/cosmos-sdk/x/ibc/20-transfer" "github.com/cosmos/cosmos-sdk/x/mint" "github.com/cosmos/cosmos-sdk/x/params" @@ -193,8 +194,9 @@ func NewSimApp( evidenceKeeper := evidence.NewKeeper( app.cdc, keys[evidence.StoreKey], app.subspaces[evidence.ModuleName], evidence.DefaultCodespace, ) - evidenceRouter := evidence.NewRouter() - // TODO: Register evidence routes. + evidenceRouter := evidence.NewRouter(). + AddRoute(ibcclient.RouterKey, ibcclient.HandlerClientMisbehaviour(app.IBCKeeper.ClientKeeper)) + evidenceKeeper.SetRouter(evidenceRouter) app.EvidenceKeeper = *evidenceKeeper diff --git a/x/ibc/02-client/alias.go b/x/ibc/02-client/alias.go index 00519d9da512..0cccbbdf3b63 100644 --- a/x/ibc/02-client/alias.go +++ b/x/ibc/02-client/alias.go @@ -61,24 +61,21 @@ var ( KeyRoot = types.KeyRoot NewMsgCreateClient = types.NewMsgCreateClient NewMsgUpdateClient = types.NewMsgUpdateClient - NewMsgSubmitMisbehaviour = types.NewMsgSubmitMisbehaviour NewQueryClientStateParams = types.NewQueryClientStateParams NewQueryCommitmentRootParams = types.NewQueryCommitmentRootParams NewClientState = types.NewClientState // variable aliases - SubModuleCdc = types.SubModuleCdc - EventTypeCreateClient = types.EventTypeCreateClient - EventTypeUpdateClient = types.EventTypeUpdateClient - EventTypeSubmitMisbehaviour = types.EventTypeSubmitMisbehaviour - AttributeValueCategory = types.AttributeValueCategory + SubModuleCdc = types.SubModuleCdc + EventTypeCreateClient = types.EventTypeCreateClient + EventTypeUpdateClient = types.EventTypeUpdateClient + AttributeValueCategory = types.AttributeValueCategory ) type ( Keeper = keeper.Keeper MsgCreateClient = types.MsgCreateClient MsgUpdateClient = types.MsgUpdateClient - MsgSubmitMisbehaviour = types.MsgSubmitMisbehaviour QueryClientStateParams = types.QueryClientStateParams QueryCommitmentRootParams = types.QueryCommitmentRootParams State = types.State diff --git a/x/ibc/02-client/client/cli/tx.go b/x/ibc/02-client/client/cli/tx.go index 4cbc97b39842..86def52e76b5 100644 --- a/x/ibc/02-client/client/cli/tx.go +++ b/x/ibc/02-client/client/cli/tx.go @@ -126,47 +126,3 @@ $ %s tx ibc client update [client-id] [path/to/header.json] --from node0 --home } return cmd } - -// GetCmdSubmitMisbehaviour defines the command to submit a misbehaviour to invalidate -// previous state roots and prevent future updates as defined in -// https://github.com/cosmos/ics/tree/master/spec/ics-002-client-semantics#misbehaviour -func GetCmdSubmitMisbehaviour(cdc *codec.Codec) *cobra.Command { - cmd := &cobra.Command{ - Use: "misbehaviour [client-it] [path/to/evidence.json]", - Short: "submit a client misbehaviour", - Long: strings.TrimSpace(fmt.Sprintf(`submit a client misbehaviour to invalidate to invalidate previous state roots and prevent future updates: - -Example: -$ %s tx ibc client misbehaviour [client-id] [path/to/evidence.json] --from node0 --home ../node0/cli --chain-id $CID - `, version.ClientName), - ), - Args: cobra.ExactArgs(2), - RunE: func(cmd *cobra.Command, args []string) error { - txBldr := auth.NewTxBuilderFromCLI().WithTxEncoder(utils.GetTxEncoder(cdc)) - cliCtx := context.NewCLIContext().WithCodec(cdc) - - clientID := args[0] - - var evidence exported.Evidence - if err := cdc.UnmarshalJSON([]byte(args[1]), &evidence); err != nil { - fmt.Fprintf(os.Stderr, "failed to unmarshall input into struct, checking for file...") - - contents, err := ioutil.ReadFile(args[1]) - if err != nil { - return fmt.Errorf("error opening proof file: %v", err) - } - if err := cdc.UnmarshalJSON(contents, &evidence); err != nil { - return fmt.Errorf("error unmarshalling evidence file: %v", err) - } - } - - msg := types.NewMsgSubmitMisbehaviour(clientID, evidence, cliCtx.GetFromAddress()) - if err := msg.ValidateBasic(); err != nil { - return err - } - - return utils.GenerateOrBroadcastMsgs(cliCtx, txBldr, []sdk.Msg{msg}) - }, - } - return cmd -} diff --git a/x/ibc/02-client/exported/exported.go b/x/ibc/02-client/exported/exported.go index e611b94301fa..0d95a4fe2a53 100644 --- a/x/ibc/02-client/exported/exported.go +++ b/x/ibc/02-client/exported/exported.go @@ -4,10 +4,7 @@ import ( "encoding/json" "fmt" - sdk "github.com/cosmos/cosmos-sdk/types" - - cmn "github.com/tendermint/tendermint/libs/common" - + evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported" commitment "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment" ) @@ -25,32 +22,10 @@ type ConsensusState interface { CheckValidityAndUpdateState(Header) (ConsensusState, error) } -// Evidence from ADR 009: Evidence Module -// TODO: use evidence module interface once merged -type Evidence interface { - Route() string - Type() string - String() string - Hash() cmn.HexBytes - ValidateBasic() sdk.Error - - // The consensus address of the malicious validator at time of infraction - GetConsensusAddress() sdk.ConsAddress - - // Height at which the infraction occurred - GetHeight() int64 - - // The total power of the malicious validator at time of infraction - GetValidatorPower() int64 - - // The total validator set power at time of infraction - GetTotalPower() int64 -} - // Misbehaviour defines a specific consensus kind and an evidence type Misbehaviour interface { ClientType() ClientType - Evidence() Evidence + Evidence() evidenceexported.Evidence } // Header is the consensus state update information diff --git a/x/ibc/02-client/handler.go b/x/ibc/02-client/handler.go index 1b1e1807b54a..d8f0511cbb56 100644 --- a/x/ibc/02-client/handler.go +++ b/x/ibc/02-client/handler.go @@ -1,8 +1,13 @@ package client import ( + "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" - exported "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" + "github.com/cosmos/cosmos-sdk/x/evidence" + evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported" + "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" + "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/tendermint" ) // HandleMsgCreateClient defines the sdk.Handler for MsgCreateClient @@ -12,7 +17,6 @@ func HandleMsgCreateClient(ctx sdk.Context, k Keeper, msg MsgCreateClient) sdk.R return sdk.ResultFromError(ErrInvalidClientType(DefaultCodespace, err.Error())) } - // TODO: should we create an event with the new client state id ? _, err = k.CreateClient(ctx, msg.ClientID, clientType, msg.ConsensusState) if err != nil { return sdk.ResultFromError(err) @@ -55,24 +59,17 @@ func HandleMsgUpdateClient(ctx sdk.Context, k Keeper, msg MsgUpdateClient) sdk.R return sdk.Result{Events: ctx.EventManager().Events()} } -// HandleMsgSubmitMisbehaviour defines the sdk.Handler for MsgSubmitMisbehaviour -func HandleMsgSubmitMisbehaviour(ctx sdk.Context, k Keeper, msg MsgSubmitMisbehaviour) sdk.Result { - err := k.CheckMisbehaviourAndUpdateState(ctx, msg.ClientID, msg.Evidence) - if err != nil { - return sdk.ResultFromError(err) - } +// HandlerClientMisbehaviour defines the Evidence module handler for submitting a +// light client misbehaviour. +func HandlerClientMisbehaviour(k Keeper) evidence.Handler { + return func(ctx sdk.Context, evidence evidenceexported.Evidence) error { + switch e := evidence.(type) { + case tendermint.Evidence: + return k.CheckMisbehaviourAndUpdateState(ctx, evidence) - ctx.EventManager().EmitEvents(sdk.Events{ - sdk.NewEvent( - EventTypeSubmitMisbehaviour, - sdk.NewAttribute(AttributeKeyClientID, msg.ClientID), - ), - sdk.NewEvent( - sdk.EventTypeMessage, - sdk.NewAttribute(sdk.AttributeKeyModule, AttributeValueCategory), - sdk.NewAttribute(sdk.AttributeKeySender, msg.Signer.String()), - ), - }) - - return sdk.Result{Events: ctx.EventManager().Events()} + default: + errMsg := fmt.Sprintf("unrecognized IBC client evidence type: %T", e) + return sdk.ErrUnknownRequest(errMsg) + } + } } diff --git a/x/ibc/02-client/keeper/client.go b/x/ibc/02-client/keeper/client.go index 3ee7a53143bf..29f44db7ac7b 100644 --- a/x/ibc/02-client/keeper/client.go +++ b/x/ibc/02-client/keeper/client.go @@ -5,9 +5,11 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/errors" + "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/tendermint" ) // CreateClient creates a new client state and populates it with a given consensus @@ -73,23 +75,38 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.H // CheckMisbehaviourAndUpdateState checks for client misbehaviour and freezes the // client if so. -func (k Keeper) CheckMisbehaviourAndUpdateState(ctx sdk.Context, clientID string, evidence exported.Evidence) error { - clientState, found := k.GetClientState(ctx, clientID) +// +// NOTE: In the first implementation, only Tendermint misbehaviour evidence is +// supported. +func (k Keeper) CheckMisbehaviourAndUpdateState(ctx sdk.Context, evidence evidenceexported.Evidence) error { + tmEvidence, ok := evidence.(tendermint.Evidence) + if !ok { + return errors.ErrInvalidClientType(k.codespace, "consensus type is not Tendermint") + } + + clientState, found := k.GetClientState(ctx, tmEvidence.ClientID) if !found { - sdk.ResultFromError(errors.ErrClientNotFound(k.codespace, clientID)) + sdk.ResultFromError(errors.ErrClientNotFound(k.codespace, tmEvidence.ClientID)) } - err := k.checkMisbehaviour(ctx, evidence) - if err != nil { - return err + if err := tendermint.CheckMisbehaviour(tmEvidence); err != nil { + return errors.ErrInvalidEvidence(k.codespace, err.Error()) } - clientState, err = k.freeze(ctx, clientState) + clientState, err := k.freeze(ctx, clientState) if err != nil { return err } k.SetClientState(ctx, clientState) - k.Logger(ctx).Info(fmt.Sprintf("client %s frozen due to misbehaviour", clientID)) + k.Logger(ctx).Info(fmt.Sprintf("client %s frozen due to misbehaviour", tmEvidence.ClientID)) + + ctx.EventManager().EmitEvent( + sdk.NewEvent( + types.EventTypeSubmitMisbehaviour, + sdk.NewAttribute(types.AttributeKeyClientID, tmEvidence.ClientID), + ), + ) + return nil } diff --git a/x/ibc/02-client/keeper/keeper.go b/x/ibc/02-client/keeper/keeper.go index 3911c1f98f0e..b0ed38bac919 100644 --- a/x/ibc/02-client/keeper/keeper.go +++ b/x/ibc/02-client/keeper/keeper.go @@ -12,7 +12,6 @@ import ( "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/errors" - "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/tendermint" commitment "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment" ibctypes "github.com/cosmos/cosmos-sdk/x/ibc/types" ) @@ -127,24 +126,6 @@ func (k Keeper) initialize(ctx sdk.Context, clientID string, consensusState expo return clientState } -func (k Keeper) checkMisbehaviour(ctx sdk.Context, evidence exported.Evidence) error { - switch evidence.Type() { - case exported.ClientTypeTendermint: - var tmEvidence tendermint.Evidence - _, ok := evidence.(tendermint.Evidence) - if !ok { - return errors.ErrInvalidClientType(k.codespace, "consensus type is not Tendermint") - } - err := tendermint.CheckMisbehaviour(tmEvidence) - if err != nil { - return errors.ErrInvalidEvidence(k.codespace, err.Error()) - } - default: - panic(fmt.Sprintf("unregistered evidence type: %s", evidence.Type())) - } - return nil -} - // freeze updates the state of the client in the event of a misbehaviour func (k Keeper) freeze(ctx sdk.Context, clientState types.State) (types.State, error) { if clientState.Frozen { diff --git a/x/ibc/02-client/types/codec.go b/x/ibc/02-client/types/codec.go index 551403e6ad89..2bb41865cc08 100644 --- a/x/ibc/02-client/types/codec.go +++ b/x/ibc/02-client/types/codec.go @@ -12,7 +12,6 @@ var SubModuleCdc = codec.New() // RegisterCodec registers the IBC client interfaces and types func RegisterCodec(cdc *codec.Codec) { cdc.RegisterInterface((*exported.ConsensusState)(nil), nil) - cdc.RegisterInterface((*exported.Evidence)(nil), nil) cdc.RegisterInterface((*exported.Header)(nil), nil) cdc.RegisterInterface((*exported.Misbehaviour)(nil), nil) diff --git a/x/ibc/02-client/types/events.go b/x/ibc/02-client/types/events.go index 7e7f9a508cee..a83b52f07367 100644 --- a/x/ibc/02-client/types/events.go +++ b/x/ibc/02-client/types/events.go @@ -15,7 +15,7 @@ const ( var ( EventTypeCreateClient = MsgCreateClient{}.Type() EventTypeUpdateClient = MsgUpdateClient{}.Type() - EventTypeSubmitMisbehaviour = MsgSubmitMisbehaviour{}.Type() + EventTypeSubmitMisbehaviour = "submit_misbehaviour" AttributeValueCategory = fmt.Sprintf("%s_%s", ibctypes.ModuleName, SubModuleName) ) diff --git a/x/ibc/02-client/types/keys.go b/x/ibc/02-client/types/keys.go index eba16d0c169c..ad99879a457e 100644 --- a/x/ibc/02-client/types/keys.go +++ b/x/ibc/02-client/types/keys.go @@ -6,16 +6,16 @@ import ( const ( // SubModuleName defines the IBC client name - SubModuleName = "client" + SubModuleName string = "client" // StoreKey is the store key string for IBC client - StoreKey = SubModuleName + StoreKey string = SubModuleName // RouterKey is the message route for IBC client - RouterKey = SubModuleName + RouterKey string = SubModuleName // QuerierRoute is the querier route for IBC client - QuerierRoute = SubModuleName + QuerierRoute string = SubModuleName ) // KVStore key prefixes diff --git a/x/ibc/02-client/types/msgs.go b/x/ibc/02-client/types/msgs.go index f09e126666ff..f42faf001c6d 100644 --- a/x/ibc/02-client/types/msgs.go +++ b/x/ibc/02-client/types/msgs.go @@ -116,56 +116,3 @@ func (msg MsgUpdateClient) GetSignBytes() []byte { func (msg MsgUpdateClient) GetSigners() []sdk.AccAddress { return []sdk.AccAddress{msg.Signer} } - -// MsgSubmitMisbehaviour defines a message to update an IBC client -type MsgSubmitMisbehaviour struct { - ClientID string `json:"id" yaml:"id"` - Evidence exported.Evidence `json:"evidence" yaml:"evidence"` - Signer sdk.AccAddress `json:"address" yaml:"address"` -} - -// NewMsgSubmitMisbehaviour creates a new MsgSubmitMisbehaviour instance -func NewMsgSubmitMisbehaviour(id string, evidence exported.Evidence, signer sdk.AccAddress) MsgSubmitMisbehaviour { - return MsgSubmitMisbehaviour{ - ClientID: id, - Evidence: evidence, - Signer: signer, - } -} - -// Route implements sdk.Msg -func (msg MsgSubmitMisbehaviour) Route() string { - return ibctypes.RouterKey -} - -// Type implements sdk.Msg -func (msg MsgSubmitMisbehaviour) Type() string { - return "submit_misbehaviour" -} - -// ValidateBasic implements sdk.Msg -func (msg MsgSubmitMisbehaviour) ValidateBasic() sdk.Error { - if err := host.DefaultClientIdentifierValidator(msg.ClientID); err != nil { - return sdk.ConvertError(err) - } - if msg.Evidence == nil { - return sdk.ConvertError(errors.ErrInvalidEvidence(errors.DefaultCodespace, "evidence is nil")) - } - if err := msg.Evidence.ValidateBasic(); err != nil { - return sdk.ConvertError(errors.ErrInvalidEvidence(errors.DefaultCodespace, err.Error())) - } - if msg.Signer.Empty() { - return sdk.ErrInvalidAddress("empty address") - } - return nil -} - -// GetSignBytes implements sdk.Msg -func (msg MsgSubmitMisbehaviour) GetSignBytes() []byte { - return sdk.MustSortJSON(SubModuleCdc.MustMarshalJSON(msg)) -} - -// GetSigners implements sdk.Msg -func (msg MsgSubmitMisbehaviour) GetSigners() []sdk.AccAddress { - return []sdk.AccAddress{msg.Signer} -} diff --git a/x/ibc/02-client/types/msgs_test.go b/x/ibc/02-client/types/msgs_test.go index f7e022ecc89c..4887532a4940 100644 --- a/x/ibc/02-client/types/msgs_test.go +++ b/x/ibc/02-client/types/msgs_test.go @@ -6,11 +6,9 @@ import ( "github.com/stretchr/testify/require" "github.com/tendermint/tendermint/crypto/secp256k1" - cmn "github.com/tendermint/tendermint/libs/common" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" - "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/errors" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/tendermint" ) @@ -79,63 +77,63 @@ func TestMsgUpdateClient(t *testing.T) { } } -var _ exported.Evidence = mockEvidence{} - -const mockStr = "mock" - -// mock GoodEvidence -type mockEvidence struct{} - -// Implement Evidence interface -func (me mockEvidence) Route() string { return mockStr } -func (me mockEvidence) Type() string { return mockStr } -func (me mockEvidence) String() string { return mockStr } -func (me mockEvidence) Hash() cmn.HexBytes { return cmn.HexBytes([]byte(mockStr)) } -func (me mockEvidence) ValidateBasic() sdk.Error { return nil } -func (me mockEvidence) GetConsensusAddress() sdk.ConsAddress { return sdk.ConsAddress{} } -func (me mockEvidence) GetHeight() int64 { return 3 } -func (me mockEvidence) GetValidatorPower() int64 { return 3 } -func (me mockEvidence) GetTotalPower() int64 { return 5 } - -// mock bad evidence -type mockBadEvidence struct { - mockEvidence -} - -// Override ValidateBasic -func (mbe mockBadEvidence) ValidateBasic() sdk.Error { - return sdk.ConvertError(errors.ErrInvalidEvidence(errors.DefaultCodespace, "invalid evidence")) -} - -func TestMsgSubmitMisbehaviour(t *testing.T) { - privKey := secp256k1.GenPrivKey() - signer := sdk.AccAddress(privKey.PubKey().Address()) - testMsgs := []MsgSubmitMisbehaviour{ - NewMsgSubmitMisbehaviour(exported.ClientTypeTendermint, mockEvidence{}, signer), // valid msg - NewMsgSubmitMisbehaviour("badClient", mockEvidence{}, signer), // bad client id - NewMsgSubmitMisbehaviour(exported.ClientTypeTendermint, nil, signer), // nil evidence - NewMsgSubmitMisbehaviour(exported.ClientTypeTendermint, mockBadEvidence{}, signer), // invalid evidence - NewMsgSubmitMisbehaviour(exported.ClientTypeTendermint, mockEvidence{}, sdk.AccAddress{}), // empty signer - } - - cases := []struct { - msg MsgSubmitMisbehaviour - expPass bool - errMsg string - }{ - {testMsgs[0], true, ""}, - {testMsgs[1], false, "invalid client id passed"}, - {testMsgs[2], false, "Nil Evidence passed"}, - {testMsgs[3], false, "Invalid Evidence passed"}, - {testMsgs[4], false, "Empty address passed"}, - } - - for i, tc := range cases { - err := tc.msg.ValidateBasic() - if tc.expPass { - require.Nil(t, err, "Msg %d failed: %v", i, err) - } else { - require.NotNil(t, err, "Invalid Msg %d passed: %s", i, tc.errMsg) - } - } -} +// var _ exported.Evidence = mockEvidence{} + +// const mockStr = "mock" + +// // mock GoodEvidence +// type mockEvidence struct{} + +// // Implement Evidence interface +// func (me mockEvidence) Route() string { return mockStr } +// func (me mockEvidence) Type() string { return mockStr } +// func (me mockEvidence) String() string { return mockStr } +// func (me mockEvidence) Hash() cmn.HexBytes { return cmn.HexBytes([]byte(mockStr)) } +// func (me mockEvidence) ValidateBasic() sdk.Error { return nil } +// func (me mockEvidence) GetConsensusAddress() sdk.ConsAddress { return sdk.ConsAddress{} } +// func (me mockEvidence) GetHeight() int64 { return 3 } +// func (me mockEvidence) GetValidatorPower() int64 { return 3 } +// func (me mockEvidence) GetTotalPower() int64 { return 5 } + +// // mock bad evidence +// type mockBadEvidence struct { +// mockEvidence +// } + +// // Override ValidateBasic +// func (mbe mockBadEvidence) ValidateBasic() sdk.Error { +// return sdk.ConvertError(errors.ErrInvalidEvidence(errors.DefaultCodespace, "invalid evidence")) +// } + +// func TestMsgSubmitMisbehaviour(t *testing.T) { +// privKey := secp256k1.GenPrivKey() +// signer := sdk.AccAddress(privKey.PubKey().Address()) +// testMsgs := []MsgSubmitMisbehaviour{ +// NewMsgSubmitMisbehaviour(exported.ClientTypeTendermint, mockEvidence{}, signer), // valid msg +// NewMsgSubmitMisbehaviour("badClient", mockEvidence{}, signer), // bad client id +// NewMsgSubmitMisbehaviour(exported.ClientTypeTendermint, nil, signer), // nil evidence +// NewMsgSubmitMisbehaviour(exported.ClientTypeTendermint, mockBadEvidence{}, signer), // invalid evidence +// NewMsgSubmitMisbehaviour(exported.ClientTypeTendermint, mockEvidence{}, sdk.AccAddress{}), // empty signer +// } + +// cases := []struct { +// msg MsgSubmitMisbehaviour +// expPass bool +// errMsg string +// }{ +// {testMsgs[0], true, ""}, +// {testMsgs[1], false, "invalid client id passed"}, +// {testMsgs[2], false, "Nil Evidence passed"}, +// {testMsgs[3], false, "Invalid Evidence passed"}, +// {testMsgs[4], false, "Empty address passed"}, +// } + +// for i, tc := range cases { +// err := tc.msg.ValidateBasic() +// if tc.expPass { +// require.Nil(t, err, "Msg %d failed: %v", i, err) +// } else { +// require.NotNil(t, err, "Invalid Msg %d passed: %s", i, tc.errMsg) +// } +// } +// } diff --git a/x/ibc/02-client/types/tendermint/misbehaviour.go b/x/ibc/02-client/types/tendermint/misbehaviour.go index 1edde8ec1c04..e1988500c152 100644 --- a/x/ibc/02-client/types/tendermint/misbehaviour.go +++ b/x/ibc/02-client/types/tendermint/misbehaviour.go @@ -5,37 +5,40 @@ import ( yaml "gopkg.in/yaml.v2" - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" - "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/errors" - "github.com/tendermint/tendermint/crypto/tmhash" cmn "github.com/tendermint/tendermint/libs/common" tmtypes "github.com/tendermint/tendermint/types" + + sdk "github.com/cosmos/cosmos-sdk/types" + evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported" + "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" + "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/errors" + host "github.com/cosmos/cosmos-sdk/x/ibc/24-host" ) -var _ exported.Evidence = Evidence{} +var _ evidenceexported.Evidence = Evidence{} // Evidence is a wrapper over tendermint's DuplicateVoteEvidence // that implements Evidence interface expected by ICS-02 type Evidence struct { *tmtypes.DuplicateVoteEvidence + ClientID string `json:"client_id" yaml:"client_id"` ChainID string `json:"chain_id" yaml:"chain_id"` ValidatorPower int64 `json:"val_power" yaml:"val_power"` TotalPower int64 `json:"total_power" yaml:"total_power"` } -// Type implements exported.Evidence interface +// Route implements Evidence interface func (ev Evidence) Route() string { return exported.ClientTypeTendermint } -// Type implements exported.Evidence interface +// Type implements Evidence interface func (ev Evidence) Type() string { return exported.ClientTypeTendermint } -// String implements exported.Evidence interface +// String implements Evidence interface func (ev Evidence) String() string { bz, err := yaml.Marshal(ev) if err != nil { @@ -44,30 +47,30 @@ func (ev Evidence) String() string { return string(bz) } -// Hash implements exported.Evidence interface +// Hash implements Evidence interface func (ev Evidence) Hash() cmn.HexBytes { return tmhash.Sum(SubModuleCdc.MustMarshalBinaryBare(ev)) } -// ValidateBasic implements exported.Evidence interface -func (ev Evidence) ValidateBasic() sdk.Error { +// ValidateBasic implements Evidence interface +func (ev Evidence) ValidateBasic() error { if ev.DuplicateVoteEvidence == nil { - return sdk.ConvertError(errors.ErrInvalidEvidence(errors.DefaultCodespace, "duplicate evidence is nil")) + return errors.ErrInvalidEvidence(errors.DefaultCodespace, "duplicate evidence is nil") } err := ev.DuplicateVoteEvidence.ValidateBasic() if err != nil { - return sdk.ConvertError(errors.ErrInvalidEvidence(errors.DefaultCodespace, err.Error())) + return errors.ErrInvalidEvidence(errors.DefaultCodespace, err.Error()) } if ev.ChainID == "" { - return sdk.ConvertError(errors.ErrInvalidEvidence(errors.DefaultCodespace, "chainID is empty")) + return errors.ErrInvalidEvidence(errors.DefaultCodespace, "chainID is empty") } if ev.ValidatorPower <= 0 { - return sdk.ConvertError(errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("Invalid Validator Power: %d", ev.ValidatorPower))) + return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("invalid Validator Power: %d", ev.ValidatorPower)) } if ev.TotalPower < ev.ValidatorPower { - return sdk.ConvertError(errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("Invalid Total Power: %d", ev.TotalPower))) + return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("invalid Total Power: %d", ev.TotalPower)) } - return nil + return host.DefaultClientIdentifierValidator(ev.ClientID) } // GetConsensusAddress implements exported.Evidence interface diff --git a/x/ibc/handler.go b/x/ibc/handler.go index 07fc9f831be8..1a1e2d0189be 100644 --- a/x/ibc/handler.go +++ b/x/ibc/handler.go @@ -23,9 +23,6 @@ func NewHandler(k Keeper) sdk.Handler { case client.MsgUpdateClient: return client.HandleMsgUpdateClient(ctx, k.ClientKeeper, msg) - case client.MsgSubmitMisbehaviour: - return client.HandleMsgSubmitMisbehaviour(ctx, k.ClientKeeper, msg) - // IBC connection msgs case connection.MsgConnectionOpenInit: return connection.HandleMsgConnectionOpenInit(ctx, k.ConnectionKeeper, msg) From a7e05887e14277a9d2327ae99586059fb7f5e0f9 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Wed, 13 Nov 2019 11:18:13 +0100 Subject: [PATCH 02/33] split evidence from misbehaviour --- x/ibc/02-client/exported/exported.go | 2 +- x/ibc/02-client/handler.go | 2 +- x/ibc/02-client/keeper/client.go | 12 +-- x/ibc/02-client/types/tendermint/codec.go | 1 + x/ibc/02-client/types/tendermint/doc.go | 4 +- x/ibc/02-client/types/tendermint/evidence.go | 91 ++++++++++++++++ .../types/tendermint/evidence_test.go | 63 +++++++++++ .../types/tendermint/misbehaviour.go | 100 ++++-------------- .../types/tendermint/misbehaviour_test.go | 63 ----------- 9 files changed, 187 insertions(+), 151 deletions(-) create mode 100644 x/ibc/02-client/types/tendermint/evidence.go create mode 100644 x/ibc/02-client/types/tendermint/evidence_test.go diff --git a/x/ibc/02-client/exported/exported.go b/x/ibc/02-client/exported/exported.go index 0d95a4fe2a53..da41da7fd377 100644 --- a/x/ibc/02-client/exported/exported.go +++ b/x/ibc/02-client/exported/exported.go @@ -25,7 +25,7 @@ type ConsensusState interface { // Misbehaviour defines a specific consensus kind and an evidence type Misbehaviour interface { ClientType() ClientType - Evidence() evidenceexported.Evidence + GetEvidence() evidenceexported.Evidence } // Header is the consensus state update information diff --git a/x/ibc/02-client/handler.go b/x/ibc/02-client/handler.go index d8f0511cbb56..595656a86c20 100644 --- a/x/ibc/02-client/handler.go +++ b/x/ibc/02-client/handler.go @@ -64,7 +64,7 @@ func HandleMsgUpdateClient(ctx sdk.Context, k Keeper, msg MsgUpdateClient) sdk.R func HandlerClientMisbehaviour(k Keeper) evidence.Handler { return func(ctx sdk.Context, evidence evidenceexported.Evidence) error { switch e := evidence.(type) { - case tendermint.Evidence: + case tendermint.Misbehaviour: return k.CheckMisbehaviourAndUpdateState(ctx, evidence) default: diff --git a/x/ibc/02-client/keeper/client.go b/x/ibc/02-client/keeper/client.go index 29f44db7ac7b..83f3cbc98957 100644 --- a/x/ibc/02-client/keeper/client.go +++ b/x/ibc/02-client/keeper/client.go @@ -79,17 +79,17 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.H // NOTE: In the first implementation, only Tendermint misbehaviour evidence is // supported. func (k Keeper) CheckMisbehaviourAndUpdateState(ctx sdk.Context, evidence evidenceexported.Evidence) error { - tmEvidence, ok := evidence.(tendermint.Evidence) + misbehaviour, ok := evidence.(tendermint.Misbehaviour) if !ok { return errors.ErrInvalidClientType(k.codespace, "consensus type is not Tendermint") } - clientState, found := k.GetClientState(ctx, tmEvidence.ClientID) + clientState, found := k.GetClientState(ctx, misbehaviour.ClientID) if !found { - sdk.ResultFromError(errors.ErrClientNotFound(k.codespace, tmEvidence.ClientID)) + sdk.ResultFromError(errors.ErrClientNotFound(k.codespace, misbehaviour.ClientID)) } - if err := tendermint.CheckMisbehaviour(tmEvidence); err != nil { + if err := tendermint.CheckMisbehaviour(misbehaviour); err != nil { return errors.ErrInvalidEvidence(k.codespace, err.Error()) } @@ -99,12 +99,12 @@ func (k Keeper) CheckMisbehaviourAndUpdateState(ctx sdk.Context, evidence eviden } k.SetClientState(ctx, clientState) - k.Logger(ctx).Info(fmt.Sprintf("client %s frozen due to misbehaviour", tmEvidence.ClientID)) + k.Logger(ctx).Info(fmt.Sprintf("client %s frozen due to misbehaviour", misbehaviour.ClientID)) ctx.EventManager().EmitEvent( sdk.NewEvent( types.EventTypeSubmitMisbehaviour, - sdk.NewAttribute(types.AttributeKeyClientID, tmEvidence.ClientID), + sdk.NewAttribute(types.AttributeKeyClientID, misbehaviour.ClientID), ), ) diff --git a/x/ibc/02-client/types/tendermint/codec.go b/x/ibc/02-client/types/tendermint/codec.go index 46814fec2479..02673b8c13b6 100644 --- a/x/ibc/02-client/types/tendermint/codec.go +++ b/x/ibc/02-client/types/tendermint/codec.go @@ -10,6 +10,7 @@ var SubModuleCdc = codec.New() func RegisterCodec(cdc *codec.Codec) { cdc.RegisterConcrete(ConsensusState{}, "ibc/client/tendermint/ConsensusState", nil) cdc.RegisterConcrete(Header{}, "ibc/client/tendermint/Header", nil) + cdc.RegisterConcrete(Misbehaviour{}, "ibc/client/tendermint/Misbehaviour", nil) cdc.RegisterConcrete(Evidence{}, "ibc/client/tendermint/Evidence", nil) } diff --git a/x/ibc/02-client/types/tendermint/doc.go b/x/ibc/02-client/types/tendermint/doc.go index f0e27c7696a7..26aa430a82da 100644 --- a/x/ibc/02-client/types/tendermint/doc.go +++ b/x/ibc/02-client/types/tendermint/doc.go @@ -1,5 +1,5 @@ /* -Package tendermint implements a concrete `ConsensusState`, `Header` and `Equivocation` -for the Tendermint consensus algorithm. +Package tendermint implements a concrete `ConsensusState`, `Header`, +`Misbehaviour` and `Equivocation` types for the Tendermint consensus light client. */ package tendermint diff --git a/x/ibc/02-client/types/tendermint/evidence.go b/x/ibc/02-client/types/tendermint/evidence.go new file mode 100644 index 000000000000..bd334fb687e9 --- /dev/null +++ b/x/ibc/02-client/types/tendermint/evidence.go @@ -0,0 +1,91 @@ +package tendermint + +import ( + "fmt" + + yaml "gopkg.in/yaml.v2" + + "github.com/tendermint/tendermint/crypto/tmhash" + cmn "github.com/tendermint/tendermint/libs/common" + tmtypes "github.com/tendermint/tendermint/types" + + sdk "github.com/cosmos/cosmos-sdk/types" + evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported" + "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/errors" +) + +var _ evidenceexported.Evidence = Evidence{} + +// Evidence is a wrapper over tendermint's DuplicateVoteEvidence +// that implements Evidence interface expected by ICS-02 +type Evidence struct { + *tmtypes.DuplicateVoteEvidence + ChainID string `json:"chain_id" yaml:"chain_id"` + ValidatorPower int64 `json:"val_power" yaml:"val_power"` + TotalPower int64 `json:"total_power" yaml:"total_power"` +} + +// Route implements Evidence interface +func (ev Evidence) Route() string { + return "client" +} + +// Type implements Evidence interface +func (ev Evidence) Type() string { + return "tendermint" +} + +// String implements Evidence interface +func (ev Evidence) String() string { + bz, err := yaml.Marshal(ev) + if err != nil { + panic(err) + } + return string(bz) +} + +// Hash implements Evidence interface +func (ev Evidence) Hash() cmn.HexBytes { + return tmhash.Sum(SubModuleCdc.MustMarshalBinaryBare(ev)) +} + +// ValidateBasic implements Evidence interface +func (ev Evidence) ValidateBasic() error { + if ev.DuplicateVoteEvidence == nil { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, "duplicate evidence is nil") + } + err := ev.DuplicateVoteEvidence.ValidateBasic() + if err != nil { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, err.Error()) + } + if ev.ChainID == "" { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, "chainID is empty") + } + if ev.ValidatorPower <= 0 { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("invalid Validator Power: %d", ev.ValidatorPower)) + } + if ev.TotalPower < ev.ValidatorPower { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("invalid Total Power: %d", ev.TotalPower)) + } + return nil +} + +// GetConsensusAddress implements exported.Evidence interface +func (ev Evidence) GetConsensusAddress() sdk.ConsAddress { + return sdk.ConsAddress(ev.DuplicateVoteEvidence.Address()) +} + +// GetHeight implements exported.Evidence interface +func (ev Evidence) GetHeight() int64 { + return ev.DuplicateVoteEvidence.Height() +} + +// GetValidatorPower implements exported.Evidence interface +func (ev Evidence) GetValidatorPower() int64 { + return ev.ValidatorPower +} + +// GetTotalPower implements exported.Evidence interface +func (ev Evidence) GetTotalPower() int64 { + return ev.TotalPower +} diff --git a/x/ibc/02-client/types/tendermint/evidence_test.go b/x/ibc/02-client/types/tendermint/evidence_test.go new file mode 100644 index 000000000000..c8695c0459da --- /dev/null +++ b/x/ibc/02-client/types/tendermint/evidence_test.go @@ -0,0 +1,63 @@ +package tendermint + +import ( + "testing" + + "github.com/stretchr/testify/require" + + yaml "gopkg.in/yaml.v2" +) + +func TestString(t *testing.T) { + dupEv := randomDuplicatedVoteEvidence() + ev := Evidence{ + DuplicateVoteEvidence: dupEv, + ChainID: "mychain", + ValidatorPower: 10, + TotalPower: 50, + } + + byteStr, err := yaml.Marshal(ev) + require.Nil(t, err) + require.Equal(t, string(byteStr), ev.String(), "Evidence String method does not work as expected") + +} + +func TestValidateBasic(t *testing.T) { + dupEv := randomDuplicatedVoteEvidence() + + // good evidence + ev := Evidence{ + DuplicateVoteEvidence: dupEv, + ChainID: "mychain", + ValidatorPower: 10, + TotalPower: 50, + } + + err := ev.ValidateBasic() + require.Nil(t, err, "good evidence failed on ValidateBasic: %v", err) + + // invalid duplicate evidence + ev.DuplicateVoteEvidence.VoteA = nil + err = ev.ValidateBasic() + require.NotNil(t, err, "invalid duplicate evidence passed on ValidateBasic") + + // reset duplicate evidence to be valid, and set empty chainID + dupEv = randomDuplicatedVoteEvidence() + ev.DuplicateVoteEvidence = dupEv + ev.ChainID = "" + err = ev.ValidateBasic() + require.NotNil(t, err, "invalid chain-id passed on ValidateBasic") + + // reset chainID and set 0 validator power + ev.ChainID = "mychain" + ev.ValidatorPower = 0 + err = ev.ValidateBasic() + require.NotNil(t, err, "invalid validator power passed on ValidateBasic") + + // reset validator power and set invalid total power + ev.ValidatorPower = 10 + ev.TotalPower = 9 + err = ev.ValidateBasic() + require.NotNil(t, err, "invalid total power passed on ValidateBasic") +} diff --git a/x/ibc/02-client/types/tendermint/misbehaviour.go b/x/ibc/02-client/types/tendermint/misbehaviour.go index e1988500c152..2a4cd0bde4aa 100644 --- a/x/ibc/02-client/types/tendermint/misbehaviour.go +++ b/x/ibc/02-client/types/tendermint/misbehaviour.go @@ -1,99 +1,43 @@ package tendermint import ( - "fmt" - - yaml "gopkg.in/yaml.v2" - - "github.com/tendermint/tendermint/crypto/tmhash" - cmn "github.com/tendermint/tendermint/libs/common" - tmtypes "github.com/tendermint/tendermint/types" - - sdk "github.com/cosmos/cosmos-sdk/types" evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" - "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/errors" host "github.com/cosmos/cosmos-sdk/x/ibc/24-host" ) -var _ evidenceexported.Evidence = Evidence{} - -// Evidence is a wrapper over tendermint's DuplicateVoteEvidence -// that implements Evidence interface expected by ICS-02 -type Evidence struct { - *tmtypes.DuplicateVoteEvidence - ClientID string `json:"client_id" yaml:"client_id"` - ChainID string `json:"chain_id" yaml:"chain_id"` - ValidatorPower int64 `json:"val_power" yaml:"val_power"` - TotalPower int64 `json:"total_power" yaml:"total_power"` -} - -// Route implements Evidence interface -func (ev Evidence) Route() string { - return exported.ClientTypeTendermint -} +var _ exported.Misbehaviour = Misbehaviour{} +var _ evidenceexported.Evidence = Misbehaviour{} -// Type implements Evidence interface -func (ev Evidence) Type() string { - return exported.ClientTypeTendermint +// Misbehaviour contains an evidence that a +type Misbehaviour struct { + *Evidence + ClientID string `json:"client_id" yaml:"client_id"` } -// String implements Evidence interface -func (ev Evidence) String() string { - bz, err := yaml.Marshal(ev) - if err != nil { - panic(err) - } - return string(bz) +// ClientType is Tendermint light client +func (m Misbehaviour) ClientType() exported.ClientType { + return exported.Tendermint } -// Hash implements Evidence interface -func (ev Evidence) Hash() cmn.HexBytes { - return tmhash.Sum(SubModuleCdc.MustMarshalBinaryBare(ev)) +// GetEvidence returns the evidence to handle a light client misbehaviour +func (m Misbehaviour) GetEvidence() evidenceexported.Evidence { + return m.Evidence } -// ValidateBasic implements Evidence interface -func (ev Evidence) ValidateBasic() error { - if ev.DuplicateVoteEvidence == nil { - return errors.ErrInvalidEvidence(errors.DefaultCodespace, "duplicate evidence is nil") - } - err := ev.DuplicateVoteEvidence.ValidateBasic() - if err != nil { - return errors.ErrInvalidEvidence(errors.DefaultCodespace, err.Error()) - } - if ev.ChainID == "" { - return errors.ErrInvalidEvidence(errors.DefaultCodespace, "chainID is empty") +// ValidateBasic performs the basic validity checks for the evidence and the +// client ID. +func (m Misbehaviour) ValidateBasic() error { + if err := m.Evidence.ValidateBasic(); err != nil { + return err } - if ev.ValidatorPower <= 0 { - return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("invalid Validator Power: %d", ev.ValidatorPower)) - } - if ev.TotalPower < ev.ValidatorPower { - return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("invalid Total Power: %d", ev.TotalPower)) - } - return host.DefaultClientIdentifierValidator(ev.ClientID) -} - -// GetConsensusAddress implements exported.Evidence interface -func (ev Evidence) GetConsensusAddress() sdk.ConsAddress { - return sdk.ConsAddress(ev.DuplicateVoteEvidence.Address()) -} - -// GetHeight implements exported.Evidence interface -func (ev Evidence) GetHeight() int64 { - return ev.DuplicateVoteEvidence.Height() -} - -// GetValidatorPower implements exported.Evidence interface -func (ev Evidence) GetValidatorPower() int64 { - return ev.ValidatorPower -} -// GetTotalPower implements exported.Evidence interface -func (ev Evidence) GetTotalPower() int64 { - return ev.TotalPower + return host.DefaultClientIdentifierValidator(m.ClientID) } // CheckMisbehaviour checks if the evidence provided is a misbehaviour -func CheckMisbehaviour(evidence Evidence) error { - return evidence.DuplicateVoteEvidence.Verify(evidence.ChainID, evidence.DuplicateVoteEvidence.PubKey) +func CheckMisbehaviour(m Misbehaviour) error { + return m.Evidence.DuplicateVoteEvidence.Verify( + m.Evidence.ChainID, m.Evidence.DuplicateVoteEvidence.PubKey, + ) } diff --git a/x/ibc/02-client/types/tendermint/misbehaviour_test.go b/x/ibc/02-client/types/tendermint/misbehaviour_test.go index f5e75e12f785..1b7ad5eda026 100644 --- a/x/ibc/02-client/types/tendermint/misbehaviour_test.go +++ b/x/ibc/02-client/types/tendermint/misbehaviour_test.go @@ -1,64 +1 @@ package tendermint - -import ( - "testing" - - "github.com/stretchr/testify/require" - - yaml "gopkg.in/yaml.v2" -) - -func TestString(t *testing.T) { - dupEv := randomDuplicatedVoteEvidence() - ev := Evidence{ - DuplicateVoteEvidence: dupEv, - ChainID: "mychain", - ValidatorPower: 10, - TotalPower: 50, - } - - byteStr, err := yaml.Marshal(ev) - require.Nil(t, err) - require.Equal(t, string(byteStr), ev.String(), "Evidence String method does not work as expected") - -} - -func TestValidateBasic(t *testing.T) { - dupEv := randomDuplicatedVoteEvidence() - - // good evidence - ev := Evidence{ - DuplicateVoteEvidence: dupEv, - ChainID: "mychain", - ValidatorPower: 10, - TotalPower: 50, - } - - err := ev.ValidateBasic() - require.Nil(t, err, "good evidence failed on ValidateBasic: %v", err) - - // invalid duplicate evidence - ev.DuplicateVoteEvidence.VoteA = nil - err = ev.ValidateBasic() - require.NotNil(t, err, "invalid duplicate evidence passed on ValidateBasic") - - // reset duplicate evidence to be valid, and set empty chainID - dupEv = randomDuplicatedVoteEvidence() - ev.DuplicateVoteEvidence = dupEv - ev.ChainID = "" - err = ev.ValidateBasic() - require.NotNil(t, err, "invalid chain-id passed on ValidateBasic") - - // reset chainID and set 0 validator power - ev.ChainID = "mychain" - ev.ValidatorPower = 0 - err = ev.ValidateBasic() - require.NotNil(t, err, "invalid validator power passed on ValidateBasic") - - // reset validator power and set invalid total power - ev.ValidatorPower = 10 - ev.TotalPower = 9 - err = ev.ValidateBasic() - require.NotNil(t, err, "invalid total power passed on ValidateBasic") - -} From 33ae02e29030d114cb918f6557d0b2ec69647611 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Wed, 13 Nov 2019 11:24:14 +0100 Subject: [PATCH 03/33] clean up client events --- x/ibc/02-client/types/events.go | 6 +++--- x/ibc/02-client/types/msgs.go | 11 +++++++++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/x/ibc/02-client/types/events.go b/x/ibc/02-client/types/events.go index a83b52f07367..6b02bfa5eaf8 100644 --- a/x/ibc/02-client/types/events.go +++ b/x/ibc/02-client/types/events.go @@ -13,9 +13,9 @@ const ( // IBC client events vars var ( - EventTypeCreateClient = MsgCreateClient{}.Type() - EventTypeUpdateClient = MsgUpdateClient{}.Type() - EventTypeSubmitMisbehaviour = "submit_misbehaviour" + EventTypeCreateClient = TypeMsgCreateClient + EventTypeUpdateClient = TypeMsgUpdateClient + EventTypeSubmitMisbehaviour = TypeClientMisbehaviour AttributeValueCategory = fmt.Sprintf("%s_%s", ibctypes.ModuleName, SubModuleName) ) diff --git a/x/ibc/02-client/types/msgs.go b/x/ibc/02-client/types/msgs.go index f42faf001c6d..6dc615b27ec7 100644 --- a/x/ibc/02-client/types/msgs.go +++ b/x/ibc/02-client/types/msgs.go @@ -8,6 +8,13 @@ import ( ibctypes "github.com/cosmos/cosmos-sdk/x/ibc/types" ) +// Message types for the IBC client +const ( + TypeMsgCreateClient string = "create_client" + TypeMsgUpdateClient string = "update_client" + TypeClientMisbehaviour string = "client_misbehaviour" +) + var _ sdk.Msg = MsgCreateClient{} // MsgCreateClient defines a message to create an IBC client @@ -35,7 +42,7 @@ func (msg MsgCreateClient) Route() string { // Type implements sdk.Msg func (msg MsgCreateClient) Type() string { - return "create_client" + return TypeMsgCreateClient } // ValidateBasic implements sdk.Msg @@ -90,7 +97,7 @@ func (msg MsgUpdateClient) Route() string { // Type implements sdk.Msg func (msg MsgUpdateClient) Type() string { - return "update_client" + return TypeMsgUpdateClient } // ValidateBasic implements sdk.Msg From 94790bfbb25e589e942909ac88628d01b53ddafa Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Wed, 13 Nov 2019 12:05:26 +0100 Subject: [PATCH 04/33] test misbehaviour and evidence --- x/ibc/02-client/types/tendermint/codec.go | 1 + x/ibc/02-client/types/tendermint/evidence.go | 2 +- .../types/tendermint/evidence_test.go | 126 +++++++++++++----- .../types/tendermint/misbehaviour.go | 5 + .../types/tendermint/misbehaviour_test.go | 76 +++++++++++ 5 files changed, 174 insertions(+), 36 deletions(-) diff --git a/x/ibc/02-client/types/tendermint/codec.go b/x/ibc/02-client/types/tendermint/codec.go index 02673b8c13b6..94dcdf4f2d0c 100644 --- a/x/ibc/02-client/types/tendermint/codec.go +++ b/x/ibc/02-client/types/tendermint/codec.go @@ -8,6 +8,7 @@ var SubModuleCdc = codec.New() // RegisterCodec registers the Tendermint types func RegisterCodec(cdc *codec.Codec) { + codec.RegisterCrypto(cdc) cdc.RegisterConcrete(ConsensusState{}, "ibc/client/tendermint/ConsensusState", nil) cdc.RegisterConcrete(Header{}, "ibc/client/tendermint/Header", nil) cdc.RegisterConcrete(Misbehaviour{}, "ibc/client/tendermint/Misbehaviour", nil) diff --git a/x/ibc/02-client/types/tendermint/evidence.go b/x/ibc/02-client/types/tendermint/evidence.go index bd334fb687e9..ec3cd4832e2e 100644 --- a/x/ibc/02-client/types/tendermint/evidence.go +++ b/x/ibc/02-client/types/tendermint/evidence.go @@ -32,7 +32,7 @@ func (ev Evidence) Route() string { // Type implements Evidence interface func (ev Evidence) Type() string { - return "tendermint" + return "client_misbehaviour" } // String implements Evidence interface diff --git a/x/ibc/02-client/types/tendermint/evidence_test.go b/x/ibc/02-client/types/tendermint/evidence_test.go index c8695c0459da..006571eb6d24 100644 --- a/x/ibc/02-client/types/tendermint/evidence_test.go +++ b/x/ibc/02-client/types/tendermint/evidence_test.go @@ -3,12 +3,16 @@ package tendermint import ( "testing" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/crypto/tmhash" + cmn "github.com/tendermint/tendermint/libs/common" + tmtypes "github.com/tendermint/tendermint/types" yaml "gopkg.in/yaml.v2" ) -func TestString(t *testing.T) { +func TestEvienceString(t *testing.T) { dupEv := randomDuplicatedVoteEvidence() ev := Evidence{ DuplicateVoteEvidence: dupEv, @@ -20,44 +24,96 @@ func TestString(t *testing.T) { byteStr, err := yaml.Marshal(ev) require.Nil(t, err) require.Equal(t, string(byteStr), ev.String(), "Evidence String method does not work as expected") - } -func TestValidateBasic(t *testing.T) { +func TestEvidenceValidateBasic(t *testing.T) { dupEv := randomDuplicatedVoteEvidence() - // good evidence - ev := Evidence{ - DuplicateVoteEvidence: dupEv, - ChainID: "mychain", - ValidatorPower: 10, - TotalPower: 50, + testCases := []struct { + message string + evidence Evidence + expectErr bool + }{ + { + "valid evidence", + Evidence{ + DuplicateVoteEvidence: dupEv, + ChainID: "mychain", + ValidatorPower: 10, + TotalPower: 50, + }, + false, + }, + { + "invalid duplicate vote evidence", + Evidence{ + DuplicateVoteEvidence: &tmtypes.DuplicateVoteEvidence{ + PubKey: dupEv.PubKey, + VoteA: nil, + VoteB: dupEv.VoteB, + }, + ChainID: "mychain", + ValidatorPower: 10, + TotalPower: 50, + }, + true, + }, + { + "empty duplicate vote evidence", + Evidence{ + DuplicateVoteEvidence: nil, + ChainID: "mychain", + ValidatorPower: 10, + TotalPower: 50, + }, + true, + }, + { + "empty chain ID", + Evidence{ + DuplicateVoteEvidence: dupEv, + ChainID: "", + ValidatorPower: 10, + TotalPower: 50, + }, + true, + }, + { + "invalid validator power", + Evidence{ + DuplicateVoteEvidence: dupEv, + ChainID: "mychain", + ValidatorPower: 0, + TotalPower: 50, + }, + true, + }, + { + "validator power > total power", + Evidence{ + DuplicateVoteEvidence: dupEv, + ChainID: "mychain", + ValidatorPower: 100, + TotalPower: 50, + }, + true, + }, } - err := ev.ValidateBasic() - require.Nil(t, err, "good evidence failed on ValidateBasic: %v", err) - - // invalid duplicate evidence - ev.DuplicateVoteEvidence.VoteA = nil - err = ev.ValidateBasic() - require.NotNil(t, err, "invalid duplicate evidence passed on ValidateBasic") - - // reset duplicate evidence to be valid, and set empty chainID - dupEv = randomDuplicatedVoteEvidence() - ev.DuplicateVoteEvidence = dupEv - ev.ChainID = "" - err = ev.ValidateBasic() - require.NotNil(t, err, "invalid chain-id passed on ValidateBasic") - - // reset chainID and set 0 validator power - ev.ChainID = "mychain" - ev.ValidatorPower = 0 - err = ev.ValidateBasic() - require.NotNil(t, err, "invalid validator power passed on ValidateBasic") - - // reset validator power and set invalid total power - ev.ValidatorPower = 10 - ev.TotalPower = 9 - err = ev.ValidateBasic() - require.NotNil(t, err, "invalid total power passed on ValidateBasic") + for i, tc := range testCases { + + require.Equal(t, tc.evidence.Route(), "client", "unexpected evidence route for tc #%d", i) + require.Equal(t, tc.evidence.Type(), "client_misbehaviour", "unexpected evidence type for tc #%d", i) + require.Equal(t, tc.evidence.GetValidatorPower(), tc.evidence.ValidatorPower, "unexpected val power for tc #%d", i) + require.Equal(t, tc.evidence.GetTotalPower(), tc.evidence.TotalPower, "unexpected total power for tc #%d", i) + require.Equal(t, tc.evidence.Hash(), cmn.HexBytes(tmhash.Sum(SubModuleCdc.MustMarshalBinaryBare(tc.evidence))), "unexpected evidence hash for tc #%d", i) + + if tc.expectErr { + require.Error(t, tc.evidence.ValidateBasic(), "expected error for tc #%d", i) + } else { + require.Equal(t, tc.evidence.GetHeight(), tc.evidence.DuplicateVoteEvidence.Height(), "unexpected height for tc #%d", i) + require.Equal(t, tc.evidence.GetConsensusAddress().String(), sdk.ConsAddress(tc.evidence.DuplicateVoteEvidence.Address()).String(), "unexpected cons addr for tc #%d", i) + require.NoError(t, tc.evidence.ValidateBasic(), "unexpected error for tc #%d", i) + } + } } diff --git a/x/ibc/02-client/types/tendermint/misbehaviour.go b/x/ibc/02-client/types/tendermint/misbehaviour.go index 2a4cd0bde4aa..58f577ba358f 100644 --- a/x/ibc/02-client/types/tendermint/misbehaviour.go +++ b/x/ibc/02-client/types/tendermint/misbehaviour.go @@ -3,6 +3,7 @@ package tendermint import ( evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" + "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/errors" host "github.com/cosmos/cosmos-sdk/x/ibc/24-host" ) @@ -28,6 +29,10 @@ func (m Misbehaviour) GetEvidence() evidenceexported.Evidence { // ValidateBasic performs the basic validity checks for the evidence and the // client ID. func (m Misbehaviour) ValidateBasic() error { + if m.Evidence == nil { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, "empty evidence") + } + if err := m.Evidence.ValidateBasic(); err != nil { return err } diff --git a/x/ibc/02-client/types/tendermint/misbehaviour_test.go b/x/ibc/02-client/types/tendermint/misbehaviour_test.go index 1b7ad5eda026..ea9adf7ea9e5 100644 --- a/x/ibc/02-client/types/tendermint/misbehaviour_test.go +++ b/x/ibc/02-client/types/tendermint/misbehaviour_test.go @@ -1 +1,77 @@ package tendermint + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" +) + +func TestMisbehaviour(t *testing.T) { + testCases := []struct { + message string + misbehaviour Misbehaviour + expectErr bool + }{ + { + "valid misbehaviour", + Misbehaviour{ + Evidence: &Evidence{ + DuplicateVoteEvidence: randomDuplicatedVoteEvidence(), + ChainID: "mychain", + ValidatorPower: 10, + TotalPower: 50, + }, + ClientID: "ibcclientzero", + }, + false, + }, + { + "empty misbehaviour evidence", + Misbehaviour{ + Evidence: nil, + ClientID: "ibcclientzero", + }, + true, + }, + { + "empty misbehaviour evidence", + Misbehaviour{ + Evidence: &Evidence{ + DuplicateVoteEvidence: randomDuplicatedVoteEvidence(), + ChainID: "mychain", + ValidatorPower: 100, + TotalPower: 50, + }, + ClientID: "ibcclientzero", + }, + true, + }, + { + "invalid client ID", + Misbehaviour{ + Evidence: &Evidence{ + DuplicateVoteEvidence: randomDuplicatedVoteEvidence(), + ChainID: "mychain", + ValidatorPower: 10, + TotalPower: 50, + }, + ClientID: " ", + }, + true, + }, + } + + for i, tc := range testCases { + require.Equal(t, tc.misbehaviour.ClientType().String(), exported.ClientTypeTendermint, "unexpected misbehaviour client type for tc #%d", i) + require.Equal(t, tc.misbehaviour.GetEvidence(), tc.misbehaviour.Evidence, "unexpected evidence for tc #%d", i) + + if tc.expectErr { + require.Error(t, tc.misbehaviour.ValidateBasic(), "expected error for tc #%d", i) + } else { + require.NoError(t, tc.misbehaviour.ValidateBasic(), "unexpected error for tc #%d", i) + require.NoError(t, CheckMisbehaviour(tc.misbehaviour)) + } + } +} From f7db0fed3c9f82246824c5a3ffe9528e3981d478 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Wed, 13 Nov 2019 12:08:29 +0100 Subject: [PATCH 05/33] remove comments --- x/ibc/02-client/types/msgs_test.go | 61 ------------------------------ 1 file changed, 61 deletions(-) diff --git a/x/ibc/02-client/types/msgs_test.go b/x/ibc/02-client/types/msgs_test.go index 4887532a4940..72f4407f2319 100644 --- a/x/ibc/02-client/types/msgs_test.go +++ b/x/ibc/02-client/types/msgs_test.go @@ -76,64 +76,3 @@ func TestMsgUpdateClient(t *testing.T) { } } } - -// var _ exported.Evidence = mockEvidence{} - -// const mockStr = "mock" - -// // mock GoodEvidence -// type mockEvidence struct{} - -// // Implement Evidence interface -// func (me mockEvidence) Route() string { return mockStr } -// func (me mockEvidence) Type() string { return mockStr } -// func (me mockEvidence) String() string { return mockStr } -// func (me mockEvidence) Hash() cmn.HexBytes { return cmn.HexBytes([]byte(mockStr)) } -// func (me mockEvidence) ValidateBasic() sdk.Error { return nil } -// func (me mockEvidence) GetConsensusAddress() sdk.ConsAddress { return sdk.ConsAddress{} } -// func (me mockEvidence) GetHeight() int64 { return 3 } -// func (me mockEvidence) GetValidatorPower() int64 { return 3 } -// func (me mockEvidence) GetTotalPower() int64 { return 5 } - -// // mock bad evidence -// type mockBadEvidence struct { -// mockEvidence -// } - -// // Override ValidateBasic -// func (mbe mockBadEvidence) ValidateBasic() sdk.Error { -// return sdk.ConvertError(errors.ErrInvalidEvidence(errors.DefaultCodespace, "invalid evidence")) -// } - -// func TestMsgSubmitMisbehaviour(t *testing.T) { -// privKey := secp256k1.GenPrivKey() -// signer := sdk.AccAddress(privKey.PubKey().Address()) -// testMsgs := []MsgSubmitMisbehaviour{ -// NewMsgSubmitMisbehaviour(exported.ClientTypeTendermint, mockEvidence{}, signer), // valid msg -// NewMsgSubmitMisbehaviour("badClient", mockEvidence{}, signer), // bad client id -// NewMsgSubmitMisbehaviour(exported.ClientTypeTendermint, nil, signer), // nil evidence -// NewMsgSubmitMisbehaviour(exported.ClientTypeTendermint, mockBadEvidence{}, signer), // invalid evidence -// NewMsgSubmitMisbehaviour(exported.ClientTypeTendermint, mockEvidence{}, sdk.AccAddress{}), // empty signer -// } - -// cases := []struct { -// msg MsgSubmitMisbehaviour -// expPass bool -// errMsg string -// }{ -// {testMsgs[0], true, ""}, -// {testMsgs[1], false, "invalid client id passed"}, -// {testMsgs[2], false, "Nil Evidence passed"}, -// {testMsgs[3], false, "Invalid Evidence passed"}, -// {testMsgs[4], false, "Empty address passed"}, -// } - -// for i, tc := range cases { -// err := tc.msg.ValidateBasic() -// if tc.expPass { -// require.Nil(t, err, "Msg %d failed: %v", i, err) -// } else { -// require.NotNil(t, err, "Invalid Msg %d passed: %s", i, tc.errMsg) -// } -// } -// } From e32f3048b38c8eac101eac012bc24f0b77f7c38f Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Wed, 13 Nov 2019 17:27:34 +0100 Subject: [PATCH 06/33] remove frozen comments from demo --- x/ibc/02-client/keeper/keeper.go | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/x/ibc/02-client/keeper/keeper.go b/x/ibc/02-client/keeper/keeper.go index b0ed38bac919..922749cf1243 100644 --- a/x/ibc/02-client/keeper/keeper.go +++ b/x/ibc/02-client/keeper/keeper.go @@ -145,12 +145,14 @@ func (k Keeper) VerifyMembership( path commitment.PathI, value []byte, ) bool { - // XXX: commented out for demo - /* - if clientState.Frozen { - return false - } - */ + clientState, found := k.GetClientState(ctx, clientID) + if !found { + return false + } + + if clientState.Frozen { + return false + } root, found := k.GetVerifiedRoot(ctx, clientID, height) if !found { @@ -168,12 +170,15 @@ func (k Keeper) VerifyNonMembership( proof commitment.ProofI, path commitment.PathI, ) bool { - // XXX: commented out for demo - /* - if clientState.Frozen { - return false - } - */ + clientState, found := k.GetClientState(ctx, clientID) + if !found { + return false + } + + if clientState.Frozen { + return false + } + root, found := k.GetVerifiedRoot(ctx, clientID, height) if !found { return false From 0c5ee0a1c08f32cacbd79538eb2ec90f3d0efcb0 Mon Sep 17 00:00:00 2001 From: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Date: Thu, 14 Nov 2019 11:39:00 +0100 Subject: [PATCH 07/33] Update x/ibc/02-client/types/tendermint/evidence_test.go Co-Authored-By: Aditya --- x/ibc/02-client/types/tendermint/evidence_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/ibc/02-client/types/tendermint/evidence_test.go b/x/ibc/02-client/types/tendermint/evidence_test.go index 006571eb6d24..bdc5e360d28b 100644 --- a/x/ibc/02-client/types/tendermint/evidence_test.go +++ b/x/ibc/02-client/types/tendermint/evidence_test.go @@ -12,7 +12,7 @@ import ( yaml "gopkg.in/yaml.v2" ) -func TestEvienceString(t *testing.T) { +func TestEvidenceString(t *testing.T) { dupEv := randomDuplicatedVoteEvidence() ev := Evidence{ DuplicateVoteEvidence: dupEv, From 806136a0c5a7d803e501e30824b2a53fc96daa31 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 15 Nov 2019 14:40:18 -0800 Subject: [PATCH 08/33] change evidence to detect malicious chain --- x/evidence/exported/evidence.go | 6 ++ x/ibc/02-client/types/tendermint/evidence.go | 76 +++++++++++-------- .../types/tendermint/misbehaviour.go | 4 +- 3 files changed, 51 insertions(+), 35 deletions(-) diff --git a/x/evidence/exported/evidence.go b/x/evidence/exported/evidence.go index 1c6adfdf8c0a..9c7f8180ef54 100644 --- a/x/evidence/exported/evidence.go +++ b/x/evidence/exported/evidence.go @@ -14,6 +14,12 @@ type Evidence interface { String() string Hash() cmn.HexBytes ValidateBasic() error +} + +// ValidatorEvidence extends Evidence interface to define contract +// for evidence against malicious validators +type ValidatorEvidence interface { + Evidence // The consensus address of the malicious validator at time of infraction GetConsensusAddress() sdk.ConsAddress diff --git a/x/ibc/02-client/types/tendermint/evidence.go b/x/ibc/02-client/types/tendermint/evidence.go index ec3cd4832e2e..71d0ef5384a1 100644 --- a/x/ibc/02-client/types/tendermint/evidence.go +++ b/x/ibc/02-client/types/tendermint/evidence.go @@ -2,6 +2,8 @@ package tendermint import ( "fmt" + "reflect" + "sort" yaml "gopkg.in/yaml.v2" @@ -9,7 +11,6 @@ import ( cmn "github.com/tendermint/tendermint/libs/common" tmtypes "github.com/tendermint/tendermint/types" - sdk "github.com/cosmos/cosmos-sdk/types" evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/errors" ) @@ -19,10 +20,9 @@ var _ evidenceexported.Evidence = Evidence{} // Evidence is a wrapper over tendermint's DuplicateVoteEvidence // that implements Evidence interface expected by ICS-02 type Evidence struct { - *tmtypes.DuplicateVoteEvidence - ChainID string `json:"chain_id" yaml:"chain_id"` - ValidatorPower int64 `json:"val_power" yaml:"val_power"` - TotalPower int64 `json:"total_power" yaml:"total_power"` + Header1 Header + Header2 Header + ChainID string } // Route implements Evidence interface @@ -51,41 +51,53 @@ func (ev Evidence) Hash() cmn.HexBytes { // ValidateBasic implements Evidence interface func (ev Evidence) ValidateBasic() error { - if ev.DuplicateVoteEvidence == nil { - return errors.ErrInvalidEvidence(errors.DefaultCodespace, "duplicate evidence is nil") + if err := ev.Header1.ValidateBasic(ev.ChainID); err != nil { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, err.Error()) } - err := ev.DuplicateVoteEvidence.ValidateBasic() - if err != nil { + if err := ev.Header2.ValidateBasic(ev.ChainID); err != nil { return errors.ErrInvalidEvidence(errors.DefaultCodespace, err.Error()) } - if ev.ChainID == "" { - return errors.ErrInvalidEvidence(errors.DefaultCodespace, "chainID is empty") + if ev.Header1.ValidatorSet == nil || ev.Header2.ValidatorSet == nil { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, "validator set in header is nil") } - if ev.ValidatorPower <= 0 { - return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("invalid Validator Power: %d", ev.ValidatorPower)) + if ev.Header1.Height != ev.Header2.Height { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, "headers in evidence are on different heights") } - if ev.TotalPower < ev.ValidatorPower { - return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("invalid Total Power: %d", ev.TotalPower)) + if ev.Header1.Commit.BlockID.Equals(ev.Header2.Commit.BlockID) { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, "Headers commit to same blockID") } - return nil -} -// GetConsensusAddress implements exported.Evidence interface -func (ev Evidence) GetConsensusAddress() sdk.ConsAddress { - return sdk.ConsAddress(ev.DuplicateVoteEvidence.Address()) -} + // Ensure that validator sets that voted on differing headers are the same validators + // even if headers have different proposers + // Require Validators are sorted first + sort.Sort(tmtypes.ValidatorsByAddress(ev.Header1.ValidatorSet.Validators)) + sort.Sort(tmtypes.ValidatorsByAddress(ev.Header2.ValidatorSet.Validators)) + valSet1 := ev.Header1.ValidatorSet.Validators + valSet2 := ev.Header2.ValidatorSet.Validators + if len(valSet1) != len(valSet2) { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("ValidatorSets have different lengths: valSet1: %s, valSet2: %s", + tmtypes.ValidatorListString(valSet1), tmtypes.ValidatorListString(valSet2))) + } + for i, v := range valSet1 { + if !reflect.DeepEqual(v, valSet2[i]) { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("ValidatorSets are different at index %d. v1: %s, v2: %s", i, *v, *valSet2[i])) + } + } -// GetHeight implements exported.Evidence interface -func (ev Evidence) GetHeight() int64 { - return ev.DuplicateVoteEvidence.Height() -} + // Convert commits to vote-sets given the validator set so we can check if they both have 2/3 power + voteSet1 := tmtypes.CommitToVoteSet(ev.ChainID, ev.Header1.Commit, ev.Header1.ValidatorSet) + voteSet2 := tmtypes.CommitToVoteSet(ev.ChainID, ev.Header2.Commit, ev.Header2.ValidatorSet) -// GetValidatorPower implements exported.Evidence interface -func (ev Evidence) GetValidatorPower() int64 { - return ev.ValidatorPower -} + blockID1, ok1 := voteSet1.TwoThirdsMajority() + blockID2, ok2 := voteSet2.TwoThirdsMajority() + + // Check that ValidatorSet did indeed commit to two different headers + if !ok1 || !blockID1.Equals(ev.Header1.Commit.BlockID) { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, "ValidatorSet did not commit to Header1") + } + if !ok2 || !blockID2.Equals(ev.Header2.Commit.BlockID) { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, "ValidatorSet did not commit to Header2") + } -// GetTotalPower implements exported.Evidence interface -func (ev Evidence) GetTotalPower() int64 { - return ev.TotalPower + return nil } diff --git a/x/ibc/02-client/types/tendermint/misbehaviour.go b/x/ibc/02-client/types/tendermint/misbehaviour.go index 58f577ba358f..6be8d6bd0477 100644 --- a/x/ibc/02-client/types/tendermint/misbehaviour.go +++ b/x/ibc/02-client/types/tendermint/misbehaviour.go @@ -42,7 +42,5 @@ func (m Misbehaviour) ValidateBasic() error { // CheckMisbehaviour checks if the evidence provided is a misbehaviour func CheckMisbehaviour(m Misbehaviour) error { - return m.Evidence.DuplicateVoteEvidence.Verify( - m.Evidence.ChainID, m.Evidence.DuplicateVoteEvidence.PubKey, - ) + return m.ValidateBasic() } From 6c9fe6ddea1e91914d036b1f2251d3802a35adce Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 18 Nov 2019 10:39:54 -0800 Subject: [PATCH 09/33] remove unnecessary sort --- x/ibc/02-client/types/tendermint/evidence.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x/ibc/02-client/types/tendermint/evidence.go b/x/ibc/02-client/types/tendermint/evidence.go index 71d0ef5384a1..edfc125b3a70 100644 --- a/x/ibc/02-client/types/tendermint/evidence.go +++ b/x/ibc/02-client/types/tendermint/evidence.go @@ -3,7 +3,6 @@ package tendermint import ( "fmt" "reflect" - "sort" yaml "gopkg.in/yaml.v2" @@ -51,6 +50,7 @@ func (ev Evidence) Hash() cmn.HexBytes { // ValidateBasic implements Evidence interface func (ev Evidence) ValidateBasic() error { + // ValidateBasic on both validators if err := ev.Header1.ValidateBasic(ev.ChainID); err != nil { return errors.ErrInvalidEvidence(errors.DefaultCodespace, err.Error()) } @@ -60,9 +60,11 @@ func (ev Evidence) ValidateBasic() error { if ev.Header1.ValidatorSet == nil || ev.Header2.ValidatorSet == nil { return errors.ErrInvalidEvidence(errors.DefaultCodespace, "validator set in header is nil") } + // Ensure that Heights are the same if ev.Header1.Height != ev.Header2.Height { return errors.ErrInvalidEvidence(errors.DefaultCodespace, "headers in evidence are on different heights") } + // Ensure that Commit Hashes are different if ev.Header1.Commit.BlockID.Equals(ev.Header2.Commit.BlockID) { return errors.ErrInvalidEvidence(errors.DefaultCodespace, "Headers commit to same blockID") } @@ -70,8 +72,6 @@ func (ev Evidence) ValidateBasic() error { // Ensure that validator sets that voted on differing headers are the same validators // even if headers have different proposers // Require Validators are sorted first - sort.Sort(tmtypes.ValidatorsByAddress(ev.Header1.ValidatorSet.Validators)) - sort.Sort(tmtypes.ValidatorsByAddress(ev.Header2.ValidatorSet.Validators)) valSet1 := ev.Header1.ValidatorSet.Validators valSet2 := ev.Header2.ValidatorSet.Validators if len(valSet1) != len(valSet2) { From baac2e45cb1e8cc6b17ab0459d28c26c6daa5f11 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 21 Nov 2019 11:19:54 -0800 Subject: [PATCH 10/33] fix evidence and persist committers to check misbehaviour --- x/evidence/exported/evidence.go | 6 +- x/ibc/02-client/exported/exported.go | 10 ++ x/ibc/02-client/keeper/client.go | 15 ++- x/ibc/02-client/keeper/keeper.go | 25 ++++ x/ibc/02-client/types/errors/errors.go | 22 +++- x/ibc/02-client/types/keys.go | 14 ++- x/ibc/02-client/types/tendermint/committer.go | 17 +++ .../types/tendermint/consensus_state.go | 16 ++- .../types/tendermint/consensus_state_test.go | 3 +- x/ibc/02-client/types/tendermint/evidence.go | 23 +--- .../types/tendermint/evidence_test.go | 118 ------------------ x/ibc/02-client/types/tendermint/header.go | 22 ++++ .../types/tendermint/misbehaviour.go | 19 ++- .../types/tendermint/tendermint_test.go | 1 + 14 files changed, 160 insertions(+), 151 deletions(-) create mode 100644 x/ibc/02-client/types/tendermint/committer.go diff --git a/x/evidence/exported/evidence.go b/x/evidence/exported/evidence.go index 9c7f8180ef54..79ce2bec737f 100644 --- a/x/evidence/exported/evidence.go +++ b/x/evidence/exported/evidence.go @@ -14,6 +14,9 @@ type Evidence interface { String() string Hash() cmn.HexBytes ValidateBasic() error + + // Height at which the infraction occurred + GetHeight() int64 } // ValidatorEvidence extends Evidence interface to define contract @@ -24,9 +27,6 @@ type ValidatorEvidence interface { // The consensus address of the malicious validator at time of infraction GetConsensusAddress() sdk.ConsAddress - // Height at which the infraction occurred - GetHeight() int64 - // The total power of the malicious validator at time of infraction GetValidatorPower() int64 diff --git a/x/ibc/02-client/exported/exported.go b/x/ibc/02-client/exported/exported.go index da41da7fd377..a596c0c1bda7 100644 --- a/x/ibc/02-client/exported/exported.go +++ b/x/ibc/02-client/exported/exported.go @@ -17,6 +17,9 @@ type ConsensusState interface { // which is used for key-value pair verification. GetRoot() commitment.RootI + // GetCommitter returns the committer that committed the consensus state + GetCommitter() Committer + // CheckValidityAndUpdateState returns the updated consensus state // only if the header is a descendent of this consensus state. CheckValidityAndUpdateState(Header) (ConsensusState, error) @@ -31,9 +34,16 @@ type Misbehaviour interface { // Header is the consensus state update information type Header interface { ClientType() ClientType + GetCommitter() Committer GetHeight() uint64 } +// Committer defines the type that is responsible for +// updating the consensusState at a given height +type Committer interface { + ClientType() ClientType +} + // ClientType defines the type of the consensus algorithm type ClientType byte diff --git a/x/ibc/02-client/keeper/client.go b/x/ibc/02-client/keeper/client.go index 83f3cbc98957..7f974848b676 100644 --- a/x/ibc/02-client/keeper/client.go +++ b/x/ibc/02-client/keeper/client.go @@ -29,6 +29,7 @@ func (k Keeper) CreateClient( } clientState := k.initialize(ctx, clientID, consensusState) + k.SetCommitter(ctx, clientID, consensusState.GetHeight(), consensusState.GetCommitter()) k.SetVerifiedRoot(ctx, clientID, consensusState.GetHeight(), consensusState.GetRoot()) k.SetClientState(ctx, clientState) k.SetClientType(ctx, clientID, clientType) @@ -68,6 +69,7 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.H } k.SetConsensusState(ctx, clientID, consensusState) + k.SetCommitter(ctx, clientID, consensusState.GetHeight(), consensusState.GetCommitter()) k.SetVerifiedRoot(ctx, clientID, consensusState.GetHeight(), consensusState.GetRoot()) k.Logger(ctx).Info(fmt.Sprintf("client %s updated to height %d", clientID, consensusState.GetHeight())) return nil @@ -86,10 +88,19 @@ func (k Keeper) CheckMisbehaviourAndUpdateState(ctx sdk.Context, evidence eviden clientState, found := k.GetClientState(ctx, misbehaviour.ClientID) if !found { - sdk.ResultFromError(errors.ErrClientNotFound(k.codespace, misbehaviour.ClientID)) + return errors.ErrClientNotFound(k.codespace, misbehaviour.ClientID) } - if err := tendermint.CheckMisbehaviour(misbehaviour); err != nil { + committer, found := k.GetCommitter(ctx, misbehaviour.ClientID, misbehaviour.GetHeight()) + if !found { + return errors.ErrCommitterNotFound(k.codespace, fmt.Sprintf("committer not found for height %d", misbehaviour.GetHeight())) + } + tmCommitter, ok := committer.(tendermint.Committer) + if !ok { + return errors.ErrInvalidCommitter(k.codespace, "committer type is not Tendermint") + } + + if err := tendermint.CheckMisbehaviour(tmCommitter, misbehaviour); err != nil { return errors.ErrInvalidEvidence(k.codespace, err.Error()) } diff --git a/x/ibc/02-client/keeper/keeper.go b/x/ibc/02-client/keeper/keeper.go index 922749cf1243..0d09cf792e9c 100644 --- a/x/ibc/02-client/keeper/keeper.go +++ b/x/ibc/02-client/keeper/keeper.go @@ -118,6 +118,31 @@ func (k Keeper) SetVerifiedRoot(ctx sdk.Context, clientID string, height uint64, store.Set(types.KeyRoot(clientID, height), bz) } +// GetCommitter will get the Committer of a particular client at the oldest height +// that is less than or equal to the height passed in +func (k Keeper) GetCommitter(ctx sdk.Context, clientID string, height uint64) (exported.Committer, bool) { + store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixClient) + + iter := store.ReverseIterator(types.KeyCommitter(clientID, height), types.KeyCommitter(clientID, 0)) + + if !iter.Valid() { + return nil, false + } + + bz := iter.Value() + var committer exported.Committer + k.cdc.MustUnmarshalBinaryLengthPrefixed(bz, &committer) + return committer, true +} + +// SetCommitter sets a committer from a particular height to +// a particular client +func (k Keeper) SetCommitter(ctx sdk.Context, clientID string, height uint64, committer exported.Committer) { + store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixClient) + bz := k.cdc.MustMarshalBinaryLengthPrefixed(committer) + store.Set(types.KeyCommitter(clientID, height), bz) +} + // State returns a new client state with a given id as defined in // https://github.com/cosmos/ics/tree/master/spec/ics-002-client-semantics#example-implementation func (k Keeper) initialize(ctx sdk.Context, clientID string, consensusState exported.ConsensusState) types.State { diff --git a/x/ibc/02-client/types/errors/errors.go b/x/ibc/02-client/types/errors/errors.go index 1e9510df6c44..1a7272ca1fcb 100644 --- a/x/ibc/02-client/types/errors/errors.go +++ b/x/ibc/02-client/types/errors/errors.go @@ -19,8 +19,10 @@ const ( CodeClientTypeNotFound sdk.CodeType = 205 CodeInvalidClientType sdk.CodeType = 206 CodeRootNotFound sdk.CodeType = 207 - CodeInvalidHeader sdk.CodeType = 207 + CodeInvalidHeader sdk.CodeType = 208 CodeInvalidEvidence sdk.CodeType = 209 + CodeCommitterNotFound sdk.CodeType = 210 + CodeInvalidCommitter sdk.CodeType = 211 ) // ErrClientExists implements sdk.Error @@ -112,3 +114,21 @@ func ErrInvalidEvidence(codespace sdk.CodespaceType, msg string) error { fmt.Sprintf("invalid evidence: %s", msg), ) } + +// ErrCommitterNotFound implements sdk.Error +func ErrCommitterNotFound(codespace sdk.CodespaceType, msg string) error { + return sdkerrors.New( + string(codespace), + uint32(CodeCommitterNotFound), + fmt.Sprintf("invalid evidence: %s", msg), + ) +} + +// ErrInvalidEvidence implements sdk.Error +func ErrInvalidCommitter(codespace sdk.CodespaceType, msg string) error { + return sdkerrors.New( + string(codespace), + uint32(CodeInvalidCommitter), + fmt.Sprintf("invalid evidence: %s", msg), + ) +} diff --git a/x/ibc/02-client/types/keys.go b/x/ibc/02-client/types/keys.go index ad99879a457e..bb0cf5b4a40c 100644 --- a/x/ibc/02-client/types/keys.go +++ b/x/ibc/02-client/types/keys.go @@ -44,11 +44,17 @@ func ConsensusStatePath(clientID string) string { } // RootPath takes an Identifier and returns a Path under which to -// store the consensus state of a client. +// store the root for a particular height of a client. func RootPath(clientID string, height uint64) string { return fmt.Sprintf("clients/%s/roots/%d", clientID, height) } +// CommitterPath takes an Identifier and returns a Path under which +// to store the committer of a client at a particular height +func CommitterPath(clientID string, height uint64) string { + return fmt.Sprintf("clients/%s/committer/%d", clientID, height) +} + // KeyClientState returns the store key for a particular client state func KeyClientState(clientID string) []byte { return []byte(ClientStatePath(clientID)) @@ -70,3 +76,9 @@ func KeyConsensusState(clientID string) []byte { func KeyRoot(clientID string, height uint64) []byte { return []byte(RootPath(clientID, height)) } + +// KeyValidatorSet returns the store key for a validator of a particular +// client at a given height. Key => clientID||height +func KeyCommitter(clientID string, height uint64) []byte { + return []byte(CommitterPath(clientID, height)) +} diff --git a/x/ibc/02-client/types/tendermint/committer.go b/x/ibc/02-client/types/tendermint/committer.go new file mode 100644 index 000000000000..d6ce6049b882 --- /dev/null +++ b/x/ibc/02-client/types/tendermint/committer.go @@ -0,0 +1,17 @@ +package tendermint + +import ( + tmtypes "github.com/tendermint/tendermint/types" + + "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" +) + +var _ exported.Committer = Committer{} + +type Committer struct { + *tmtypes.ValidatorSet +} + +func (c Committer) ClientType() exported.ClientType { + return exported.Tendermint +} diff --git a/x/ibc/02-client/types/tendermint/consensus_state.go b/x/ibc/02-client/types/tendermint/consensus_state.go index f57887045a2f..379049ae9f95 100644 --- a/x/ibc/02-client/types/tendermint/consensus_state.go +++ b/x/ibc/02-client/types/tendermint/consensus_state.go @@ -20,6 +20,7 @@ type ConsensusState struct { ChainID string `json:"chain_id" yaml:"chain_id"` Height uint64 `json:"height" yaml:"height"` // NOTE: defined as 'sequence' in the spec Root commitment.RootI `json:"root" yaml:"root"` + ValidatorSet *tmtypes.ValidatorSet `json:"validator_set" yaml"validator_set"` NextValidatorSet *tmtypes.ValidatorSet `json:"next_validator_set" yaml:"next_validator_set"` // contains the PublicKey } @@ -38,6 +39,10 @@ func (cs ConsensusState) GetRoot() commitment.RootI { return cs.Root } +func (cs ConsensusState) GetCommitter() exported.Committer { + return Committer{cs.ValidatorSet} +} + // CheckValidityAndUpdateState checks if the provided header is valid and updates // the consensus state if appropriate func (cs ConsensusState) CheckValidityAndUpdateState(header exported.Header) (exported.ConsensusState, error) { @@ -64,6 +69,11 @@ func (cs ConsensusState) checkValidity(header Header) error { ) } + // basic consistency check + if err := header.ValidateBasic(cs.ChainID); err != nil { + return err + } + // check if the hash from the consensus set and header // matches nextHash := cs.NextValidatorSet.Hash() @@ -78,11 +88,6 @@ func (cs ConsensusState) checkValidity(header Header) error { return lerr.ErrUnexpectedValidators(header.NextValidatorsHash, nextHash) } - // basic consistency check - if err := header.ValidateBasic(cs.ChainID); err != nil { - return err - } - // abortTransactionUnless(consensusState.publicKey.verify(header.signature)) return header.ValidatorSet.VerifyFutureCommit( cs.NextValidatorSet, cs.ChainID, header.Commit.BlockID, header.Height, header.Commit, @@ -93,6 +98,7 @@ func (cs ConsensusState) checkValidity(header Header) error { func (cs ConsensusState) update(header Header) ConsensusState { cs.Height = header.GetHeight() cs.Root = commitment.NewRoot(header.AppHash) + cs.ValidatorSet = header.ValidatorSet cs.NextValidatorSet = header.NextValidatorSet return cs } diff --git a/x/ibc/02-client/types/tendermint/consensus_state_test.go b/x/ibc/02-client/types/tendermint/consensus_state_test.go index 2f4129226228..166dac70d148 100644 --- a/x/ibc/02-client/types/tendermint/consensus_state_test.go +++ b/x/ibc/02-client/types/tendermint/consensus_state_test.go @@ -40,7 +40,8 @@ func (suite *TendermintTestSuite) TestCheckUpdate() { require.Equal(suite.T(), suite.header.GetHeight(), cs.GetHeight(), "height not updated") require.Equal(suite.T(), suite.header.AppHash.Bytes(), cs.GetRoot().GetHash(), "root not updated") tmCS, _ := cs.(ConsensusState) - require.Equal(suite.T(), suite.header.NextValidatorSet, tmCS.NextValidatorSet, "validator set did not update") + require.Equal(suite.T(), suite.header.ValidatorSet, tmCS.ValidatorSet, "validator set did not update") + require.Equal(suite.T(), suite.header.NextValidatorSet, tmCS.NextValidatorSet, "next validator set did not update") // make header invalid so update should be unsuccessful suite.SetupTest() diff --git a/x/ibc/02-client/types/tendermint/evidence.go b/x/ibc/02-client/types/tendermint/evidence.go index edfc125b3a70..a0b6d503266c 100644 --- a/x/ibc/02-client/types/tendermint/evidence.go +++ b/x/ibc/02-client/types/tendermint/evidence.go @@ -1,9 +1,6 @@ package tendermint import ( - "fmt" - "reflect" - yaml "gopkg.in/yaml.v2" "github.com/tendermint/tendermint/crypto/tmhash" @@ -48,6 +45,11 @@ func (ev Evidence) Hash() cmn.HexBytes { return tmhash.Sum(SubModuleCdc.MustMarshalBinaryBare(ev)) } +// GetHeight returns the height at which misbehaviour occurred +func (ev Evidence) GetHeight() int64 { + return ev.Header1.Height +} + // ValidateBasic implements Evidence interface func (ev Evidence) ValidateBasic() error { // ValidateBasic on both validators @@ -69,21 +71,6 @@ func (ev Evidence) ValidateBasic() error { return errors.ErrInvalidEvidence(errors.DefaultCodespace, "Headers commit to same blockID") } - // Ensure that validator sets that voted on differing headers are the same validators - // even if headers have different proposers - // Require Validators are sorted first - valSet1 := ev.Header1.ValidatorSet.Validators - valSet2 := ev.Header2.ValidatorSet.Validators - if len(valSet1) != len(valSet2) { - return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("ValidatorSets have different lengths: valSet1: %s, valSet2: %s", - tmtypes.ValidatorListString(valSet1), tmtypes.ValidatorListString(valSet2))) - } - for i, v := range valSet1 { - if !reflect.DeepEqual(v, valSet2[i]) { - return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("ValidatorSets are different at index %d. v1: %s, v2: %s", i, *v, *valSet2[i])) - } - } - // Convert commits to vote-sets given the validator set so we can check if they both have 2/3 power voteSet1 := tmtypes.CommitToVoteSet(ev.ChainID, ev.Header1.Commit, ev.Header1.ValidatorSet) voteSet2 := tmtypes.CommitToVoteSet(ev.ChainID, ev.Header2.Commit, ev.Header2.ValidatorSet) diff --git a/x/ibc/02-client/types/tendermint/evidence_test.go b/x/ibc/02-client/types/tendermint/evidence_test.go index bdc5e360d28b..8b137891791f 100644 --- a/x/ibc/02-client/types/tendermint/evidence_test.go +++ b/x/ibc/02-client/types/tendermint/evidence_test.go @@ -1,119 +1 @@ -package tendermint -import ( - "testing" - - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/stretchr/testify/require" - "github.com/tendermint/tendermint/crypto/tmhash" - cmn "github.com/tendermint/tendermint/libs/common" - tmtypes "github.com/tendermint/tendermint/types" - - yaml "gopkg.in/yaml.v2" -) - -func TestEvidenceString(t *testing.T) { - dupEv := randomDuplicatedVoteEvidence() - ev := Evidence{ - DuplicateVoteEvidence: dupEv, - ChainID: "mychain", - ValidatorPower: 10, - TotalPower: 50, - } - - byteStr, err := yaml.Marshal(ev) - require.Nil(t, err) - require.Equal(t, string(byteStr), ev.String(), "Evidence String method does not work as expected") -} - -func TestEvidenceValidateBasic(t *testing.T) { - dupEv := randomDuplicatedVoteEvidence() - - testCases := []struct { - message string - evidence Evidence - expectErr bool - }{ - { - "valid evidence", - Evidence{ - DuplicateVoteEvidence: dupEv, - ChainID: "mychain", - ValidatorPower: 10, - TotalPower: 50, - }, - false, - }, - { - "invalid duplicate vote evidence", - Evidence{ - DuplicateVoteEvidence: &tmtypes.DuplicateVoteEvidence{ - PubKey: dupEv.PubKey, - VoteA: nil, - VoteB: dupEv.VoteB, - }, - ChainID: "mychain", - ValidatorPower: 10, - TotalPower: 50, - }, - true, - }, - { - "empty duplicate vote evidence", - Evidence{ - DuplicateVoteEvidence: nil, - ChainID: "mychain", - ValidatorPower: 10, - TotalPower: 50, - }, - true, - }, - { - "empty chain ID", - Evidence{ - DuplicateVoteEvidence: dupEv, - ChainID: "", - ValidatorPower: 10, - TotalPower: 50, - }, - true, - }, - { - "invalid validator power", - Evidence{ - DuplicateVoteEvidence: dupEv, - ChainID: "mychain", - ValidatorPower: 0, - TotalPower: 50, - }, - true, - }, - { - "validator power > total power", - Evidence{ - DuplicateVoteEvidence: dupEv, - ChainID: "mychain", - ValidatorPower: 100, - TotalPower: 50, - }, - true, - }, - } - - for i, tc := range testCases { - - require.Equal(t, tc.evidence.Route(), "client", "unexpected evidence route for tc #%d", i) - require.Equal(t, tc.evidence.Type(), "client_misbehaviour", "unexpected evidence type for tc #%d", i) - require.Equal(t, tc.evidence.GetValidatorPower(), tc.evidence.ValidatorPower, "unexpected val power for tc #%d", i) - require.Equal(t, tc.evidence.GetTotalPower(), tc.evidence.TotalPower, "unexpected total power for tc #%d", i) - require.Equal(t, tc.evidence.Hash(), cmn.HexBytes(tmhash.Sum(SubModuleCdc.MustMarshalBinaryBare(tc.evidence))), "unexpected evidence hash for tc #%d", i) - - if tc.expectErr { - require.Error(t, tc.evidence.ValidateBasic(), "expected error for tc #%d", i) - } else { - require.Equal(t, tc.evidence.GetHeight(), tc.evidence.DuplicateVoteEvidence.Height(), "unexpected height for tc #%d", i) - require.Equal(t, tc.evidence.GetConsensusAddress().String(), sdk.ConsAddress(tc.evidence.DuplicateVoteEvidence.Address()).String(), "unexpected cons addr for tc #%d", i) - require.NoError(t, tc.evidence.ValidateBasic(), "unexpected error for tc #%d", i) - } - } -} diff --git a/x/ibc/02-client/types/tendermint/header.go b/x/ibc/02-client/types/tendermint/header.go index c2d9cfebc9c5..375f23311931 100644 --- a/x/ibc/02-client/types/tendermint/header.go +++ b/x/ibc/02-client/types/tendermint/header.go @@ -3,7 +3,9 @@ package tendermint import ( tmtypes "github.com/tendermint/tendermint/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" + clienterrors "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/errors" ) var _ exported.Header = Header{} @@ -20,9 +22,29 @@ func (h Header) ClientType() exported.ClientType { return exported.Tendermint } +// GetCommitter returns the ValidatorSet that committed header +func (h Header) GetCommitter() exported.Committer { + return Committer{h.ValidatorSet} +} + // GetHeight returns the current height // // NOTE: also referred as `sequence` func (h Header) GetHeight() uint64 { return uint64(h.Height) } + +// ValidateBasic calls the SignedHeader ValidateBasic function +// and checks that validatorsets are not nil +func (h Header) ValidateBasic(chainID string) error { + if err := h.SignedHeader.ValidateBasic(chainID); err != nil { + return err + } + if h.ValidatorSet == nil { + return sdkerrors.Wrap(clienterrors.ErrInvalidHeader(clienterrors.DefaultCodespace), "ValidatorSet is nil") + } + if h.NextValidatorSet == nil { + return sdkerrors.Wrap(clienterrors.ErrInvalidHeader(clienterrors.DefaultCodespace), "NextValidatorSet is nil") + } + return nil +} diff --git a/x/ibc/02-client/types/tendermint/misbehaviour.go b/x/ibc/02-client/types/tendermint/misbehaviour.go index 6be8d6bd0477..dceb3d16d329 100644 --- a/x/ibc/02-client/types/tendermint/misbehaviour.go +++ b/x/ibc/02-client/types/tendermint/misbehaviour.go @@ -41,6 +41,21 @@ func (m Misbehaviour) ValidateBasic() error { } // CheckMisbehaviour checks if the evidence provided is a misbehaviour -func CheckMisbehaviour(m Misbehaviour) error { - return m.ValidateBasic() +func CheckMisbehaviour(trustedCommitter Committer, m Misbehaviour) error { + if err := m.ValidateBasic(); err != nil { + return err + } + // check that the validator sets on both headers are valid given the last trusted validatorset + // less than or equal to evidence height + trustedValSet := trustedCommitter.ValidatorSet + if err := trustedValSet.VerifyFutureCommit(m.Evidence.Header1.ValidatorSet, m.Evidence.ChainID, + m.Evidence.Header1.Commit.BlockID, m.Evidence.Header1.Height, m.Evidence.Header1.Commit); err != nil { + return err + } + if err := trustedValSet.VerifyFutureCommit(m.Evidence.Header2.ValidatorSet, m.Evidence.ChainID, + m.Evidence.Header2.Commit.BlockID, m.Evidence.Header2.Height, m.Evidence.Header2.Commit); err != nil { + return err + } + + return nil } diff --git a/x/ibc/02-client/types/tendermint/tendermint_test.go b/x/ibc/02-client/types/tendermint/tendermint_test.go index 11817f73e9ff..9d7f7d774779 100644 --- a/x/ibc/02-client/types/tendermint/tendermint_test.go +++ b/x/ibc/02-client/types/tendermint/tendermint_test.go @@ -72,6 +72,7 @@ func (suite *TendermintTestSuite) SetupTest() { ChainID: "mychain", Height: 3, Root: root, + ValidatorSet: valSet, NextValidatorSet: valSet, } From 948c4c40f9fd46c42712ffbb08186695e0c8042f Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 21 Nov 2019 12:52:13 -0800 Subject: [PATCH 11/33] minor fixes and remove incorrect tests --- x/ibc/02-client/keeper/client.go | 2 +- x/ibc/02-client/types/tendermint/evidence.go | 3 - .../types/tendermint/evidence_test.go | 1 - .../types/tendermint/misbehaviour_test.go | 77 ------------------- .../types/tendermint/tendermint_test.go | 2 +- .../02-client/types/tendermint/test_utils.go | 29 ------- 6 files changed, 2 insertions(+), 112 deletions(-) delete mode 100644 x/ibc/02-client/types/tendermint/evidence_test.go delete mode 100644 x/ibc/02-client/types/tendermint/misbehaviour_test.go diff --git a/x/ibc/02-client/keeper/client.go b/x/ibc/02-client/keeper/client.go index 7f974848b676..9b72ef4a2c2c 100644 --- a/x/ibc/02-client/keeper/client.go +++ b/x/ibc/02-client/keeper/client.go @@ -91,7 +91,7 @@ func (k Keeper) CheckMisbehaviourAndUpdateState(ctx sdk.Context, evidence eviden return errors.ErrClientNotFound(k.codespace, misbehaviour.ClientID) } - committer, found := k.GetCommitter(ctx, misbehaviour.ClientID, misbehaviour.GetHeight()) + committer, found := k.GetCommitter(ctx, misbehaviour.ClientID, uint64(misbehaviour.GetHeight())) if !found { return errors.ErrCommitterNotFound(k.codespace, fmt.Sprintf("committer not found for height %d", misbehaviour.GetHeight())) } diff --git a/x/ibc/02-client/types/tendermint/evidence.go b/x/ibc/02-client/types/tendermint/evidence.go index a0b6d503266c..5dea8a11fd9e 100644 --- a/x/ibc/02-client/types/tendermint/evidence.go +++ b/x/ibc/02-client/types/tendermint/evidence.go @@ -59,9 +59,6 @@ func (ev Evidence) ValidateBasic() error { if err := ev.Header2.ValidateBasic(ev.ChainID); err != nil { return errors.ErrInvalidEvidence(errors.DefaultCodespace, err.Error()) } - if ev.Header1.ValidatorSet == nil || ev.Header2.ValidatorSet == nil { - return errors.ErrInvalidEvidence(errors.DefaultCodespace, "validator set in header is nil") - } // Ensure that Heights are the same if ev.Header1.Height != ev.Header2.Height { return errors.ErrInvalidEvidence(errors.DefaultCodespace, "headers in evidence are on different heights") diff --git a/x/ibc/02-client/types/tendermint/evidence_test.go b/x/ibc/02-client/types/tendermint/evidence_test.go deleted file mode 100644 index 8b137891791f..000000000000 --- a/x/ibc/02-client/types/tendermint/evidence_test.go +++ /dev/null @@ -1 +0,0 @@ - diff --git a/x/ibc/02-client/types/tendermint/misbehaviour_test.go b/x/ibc/02-client/types/tendermint/misbehaviour_test.go deleted file mode 100644 index ea9adf7ea9e5..000000000000 --- a/x/ibc/02-client/types/tendermint/misbehaviour_test.go +++ /dev/null @@ -1,77 +0,0 @@ -package tendermint - -import ( - "testing" - - "github.com/stretchr/testify/require" - - "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" -) - -func TestMisbehaviour(t *testing.T) { - testCases := []struct { - message string - misbehaviour Misbehaviour - expectErr bool - }{ - { - "valid misbehaviour", - Misbehaviour{ - Evidence: &Evidence{ - DuplicateVoteEvidence: randomDuplicatedVoteEvidence(), - ChainID: "mychain", - ValidatorPower: 10, - TotalPower: 50, - }, - ClientID: "ibcclientzero", - }, - false, - }, - { - "empty misbehaviour evidence", - Misbehaviour{ - Evidence: nil, - ClientID: "ibcclientzero", - }, - true, - }, - { - "empty misbehaviour evidence", - Misbehaviour{ - Evidence: &Evidence{ - DuplicateVoteEvidence: randomDuplicatedVoteEvidence(), - ChainID: "mychain", - ValidatorPower: 100, - TotalPower: 50, - }, - ClientID: "ibcclientzero", - }, - true, - }, - { - "invalid client ID", - Misbehaviour{ - Evidence: &Evidence{ - DuplicateVoteEvidence: randomDuplicatedVoteEvidence(), - ChainID: "mychain", - ValidatorPower: 10, - TotalPower: 50, - }, - ClientID: " ", - }, - true, - }, - } - - for i, tc := range testCases { - require.Equal(t, tc.misbehaviour.ClientType().String(), exported.ClientTypeTendermint, "unexpected misbehaviour client type for tc #%d", i) - require.Equal(t, tc.misbehaviour.GetEvidence(), tc.misbehaviour.Evidence, "unexpected evidence for tc #%d", i) - - if tc.expectErr { - require.Error(t, tc.misbehaviour.ValidateBasic(), "expected error for tc #%d", i) - } else { - require.NoError(t, tc.misbehaviour.ValidateBasic(), "unexpected error for tc #%d", i) - require.NoError(t, CheckMisbehaviour(tc.misbehaviour)) - } - } -} diff --git a/x/ibc/02-client/types/tendermint/tendermint_test.go b/x/ibc/02-client/types/tendermint/tendermint_test.go index e6a2b25fad4a..e05ae1f70e55 100644 --- a/x/ibc/02-client/types/tendermint/tendermint_test.go +++ b/x/ibc/02-client/types/tendermint/tendermint_test.go @@ -24,7 +24,7 @@ func (suite *TendermintTestSuite) SetupTest() { privVal := tmtypes.NewMockPV() val := tmtypes.NewValidator(privVal.GetPubKey(), 10) valSet := tmtypes.NewValidatorSet([]*tmtypes.Validator{val}) - suite.header = MakeHeader(3, valSet, valSet, []tmtypes.PrivValidator{privVal}) + suite.header = MakeHeader(4, valSet, valSet, []tmtypes.PrivValidator{privVal}) root := commitment.NewRoot(tmhash.Sum([]byte("my root"))) cs := ConsensusState{ diff --git a/x/ibc/02-client/types/tendermint/test_utils.go b/x/ibc/02-client/types/tendermint/test_utils.go index ac25a13fabbf..7e62b1e04181 100644 --- a/x/ibc/02-client/types/tendermint/test_utils.go +++ b/x/ibc/02-client/types/tendermint/test_utils.go @@ -21,35 +21,6 @@ func makeBlockID(hash []byte, partSetSize int, partSetHash []byte) tmtypes.Block } -func makeVote(val tmtypes.PrivValidator, chainID string, valIndex int, height int64, round, step int, blockID tmtypes.BlockID) *tmtypes.Vote { - addr := val.GetPubKey().Address() - v := &tmtypes.Vote{ - ValidatorAddress: addr, - ValidatorIndex: valIndex, - Height: height, - Round: round, - Type: tmtypes.SignedMsgType(step), - BlockID: blockID, - } - err := val.SignVote(chainID, v) - if err != nil { - panic(err) - } - return v -} - -func randomDuplicatedVoteEvidence() *tmtypes.DuplicateVoteEvidence { - val := tmtypes.NewMockPV() - blockID := makeBlockID(tmhash.Sum([]byte("blockhash")), 1000, tmhash.Sum([]byte("partshash"))) - blockID2 := makeBlockID(tmhash.Sum([]byte("blockhash2")), 1000, tmhash.Sum([]byte("partshash"))) - const chainID = "gaia" - return &tmtypes.DuplicateVoteEvidence{ - PubKey: val.GetPubKey(), - VoteA: makeVote(val, chainID, 0, 10, 2, 1, blockID), - VoteB: makeVote(val, chainID, 0, 10, 2, 1, blockID2), - } -} - func MakeHeader(height int64, valSet *tmtypes.ValidatorSet, nextValSet *tmtypes.ValidatorSet, signers []tmtypes.PrivValidator) Header { vsetHash := valSet.Hash() nextHash := nextValSet.Hash() From 0d31c59e3554f8fc742917875c38693d9e9d64c4 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 22 Nov 2019 00:06:53 -0800 Subject: [PATCH 12/33] add evidence tests --- x/ibc/02-client/keeper/client_test.go | 4 +- x/ibc/02-client/keeper/keeper_test.go | 2 +- .../types/tendermint/consensus_state.go | 2 +- x/ibc/02-client/types/tendermint/evidence.go | 32 ++-- .../types/tendermint/evidence_test.go | 146 ++++++++++++++++++ .../types/tendermint/tendermint_test.go | 2 +- .../02-client/types/tendermint/test_utils.go | 8 +- 7 files changed, 178 insertions(+), 18 deletions(-) create mode 100644 x/ibc/02-client/types/tendermint/evidence_test.go diff --git a/x/ibc/02-client/keeper/client_test.go b/x/ibc/02-client/keeper/client_test.go index 923a6e710981..4f46b887e3d0 100644 --- a/x/ibc/02-client/keeper/client_test.go +++ b/x/ibc/02-client/keeper/client_test.go @@ -60,10 +60,10 @@ func (suite *KeeperTestSuite) TestUpdateClient() { suite.keeper.SetClientState(suite.ctx, clientState) }, true}, {"past height", func() { - suite.header = tendermint.MakeHeader(2, suite.valSet, suite.valSet, []tmtypes.PrivValidator{suite.privVal}) + suite.header = tendermint.MakeHeader("gaia", 2, suite.valSet, suite.valSet, []tmtypes.PrivValidator{suite.privVal}) }, true}, {"validatorHash incorrect", func() { - suite.header = tendermint.MakeHeader(4, altValSet, suite.valSet, altSigners) + suite.header = tendermint.MakeHeader("gaia", 4, altValSet, suite.valSet, altSigners) }, true}, {"nextHash incorrect", func() { suite.header.NextValidatorSet = altValSet diff --git a/x/ibc/02-client/keeper/keeper_test.go b/x/ibc/02-client/keeper/keeper_test.go index da81be0edb12..9fdc554edbba 100644 --- a/x/ibc/02-client/keeper/keeper_test.go +++ b/x/ibc/02-client/keeper/keeper_test.go @@ -48,7 +48,7 @@ func (suite *KeeperTestSuite) SetupTest() { validator := tmtypes.NewValidator(suite.privVal.GetPubKey(), 1) suite.valSet = tmtypes.NewValidatorSet([]*tmtypes.Validator{validator}) - suite.header = tendermint.MakeHeader(4, suite.valSet, suite.valSet, []tmtypes.PrivValidator{suite.privVal}) + suite.header = tendermint.MakeHeader("gaia", 4, suite.valSet, suite.valSet, []tmtypes.PrivValidator{suite.privVal}) suite.consensusState = tendermint.ConsensusState{ ChainID: testClientID, diff --git a/x/ibc/02-client/types/tendermint/consensus_state.go b/x/ibc/02-client/types/tendermint/consensus_state.go index 379049ae9f95..1f5d4aa6672a 100644 --- a/x/ibc/02-client/types/tendermint/consensus_state.go +++ b/x/ibc/02-client/types/tendermint/consensus_state.go @@ -20,7 +20,7 @@ type ConsensusState struct { ChainID string `json:"chain_id" yaml:"chain_id"` Height uint64 `json:"height" yaml:"height"` // NOTE: defined as 'sequence' in the spec Root commitment.RootI `json:"root" yaml:"root"` - ValidatorSet *tmtypes.ValidatorSet `json:"validator_set" yaml"validator_set"` + ValidatorSet *tmtypes.ValidatorSet `json:"validator_set" yaml:"validator_set"` NextValidatorSet *tmtypes.ValidatorSet `json:"next_validator_set" yaml:"next_validator_set"` // contains the PublicKey } diff --git a/x/ibc/02-client/types/tendermint/evidence.go b/x/ibc/02-client/types/tendermint/evidence.go index 5dea8a11fd9e..4d5e2310a7bf 100644 --- a/x/ibc/02-client/types/tendermint/evidence.go +++ b/x/ibc/02-client/types/tendermint/evidence.go @@ -1,6 +1,8 @@ package tendermint import ( + "fmt" + yaml "gopkg.in/yaml.v2" "github.com/tendermint/tendermint/crypto/tmhash" @@ -68,20 +70,32 @@ func (ev Evidence) ValidateBasic() error { return errors.ErrInvalidEvidence(errors.DefaultCodespace, "Headers commit to same blockID") } + if err1 := ValidCommit(ev.ChainID, ev.Header1.Commit, ev.Header1.ValidatorSet); err1 != nil { + return err1 + } + if err2 := ValidCommit(ev.ChainID, ev.Header2.Commit, ev.Header2.ValidatorSet); err2 != nil { + return err2 + } + + return nil +} + +func ValidCommit(chainID string, commit *tmtypes.Commit, valSet *tmtypes.ValidatorSet) (err error) { + defer func() { + if r := recover(); r != nil { + err = errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("invalid commit: %v", r)) + } + }() + // Convert commits to vote-sets given the validator set so we can check if they both have 2/3 power - voteSet1 := tmtypes.CommitToVoteSet(ev.ChainID, ev.Header1.Commit, ev.Header1.ValidatorSet) - voteSet2 := tmtypes.CommitToVoteSet(ev.ChainID, ev.Header2.Commit, ev.Header2.ValidatorSet) + voteSet := tmtypes.CommitToVoteSet(chainID, commit, valSet) - blockID1, ok1 := voteSet1.TwoThirdsMajority() - blockID2, ok2 := voteSet2.TwoThirdsMajority() + blockID, ok := voteSet.TwoThirdsMajority() - // Check that ValidatorSet did indeed commit to two different headers - if !ok1 || !blockID1.Equals(ev.Header1.Commit.BlockID) { + // Check that ValidatorSet did indeed commit to blockID in Commit + if !ok || !blockID.Equals(commit.BlockID) { return errors.ErrInvalidEvidence(errors.DefaultCodespace, "ValidatorSet did not commit to Header1") } - if !ok2 || !blockID2.Equals(ev.Header2.Commit.BlockID) { - return errors.ErrInvalidEvidence(errors.DefaultCodespace, "ValidatorSet did not commit to Header2") - } return nil } diff --git a/x/ibc/02-client/types/tendermint/evidence_test.go b/x/ibc/02-client/types/tendermint/evidence_test.go new file mode 100644 index 000000000000..bbfed2802a52 --- /dev/null +++ b/x/ibc/02-client/types/tendermint/evidence_test.go @@ -0,0 +1,146 @@ +package tendermint + +import ( + "bytes" + "fmt" + + "github.com/stretchr/testify/require" + + "github.com/tendermint/tendermint/crypto/tmhash" + tmtypes "github.com/tendermint/tendermint/types" +) + +func (suite *TendermintTestSuite) TestValidateBasic() { + altPrivVal := tmtypes.NewMockPV() + altVal := tmtypes.NewValidator(altPrivVal.GetPubKey(), 4) + + altValSet := tmtypes.NewValidatorSet(append(suite.valSet.Validators, altVal)) + wrongValSet := tmtypes.NewValidatorSet([]*tmtypes.Validator{altVal}) + + signers := []tmtypes.PrivValidator{suite.privVal} + var bothSigners []tmtypes.PrivValidator + if bytes.Compare(altPrivVal.GetPubKey().Address(), suite.privVal.GetPubKey().Address()) == -1 { + bothSigners = []tmtypes.PrivValidator{altPrivVal, suite.privVal} + } else { + bothSigners = []tmtypes.PrivValidator{suite.privVal, altPrivVal} + } + + altSigners := []tmtypes.PrivValidator{altPrivVal} + + testCases := []struct { + name string + evidence Evidence + malleateEvidence func(ev *Evidence) + expErr bool + }{ + { + "valid evidence", + Evidence{ + Header1: suite.header, + Header2: MakeHeader("gaia", 4, suite.valSet, altValSet, signers), + ChainID: "gaia", + }, + func(ev *Evidence) {}, + false, + }, + { + "wrong chainID on header1", + Evidence{ + Header1: suite.header, + Header2: MakeHeader("ethermint", 4, suite.valSet, altValSet, signers), + ChainID: "ethermint", + }, + func(ev *Evidence) {}, + true, + }, + { + "wrong chainID on header2", + Evidence{ + Header1: suite.header, + Header2: MakeHeader("ethermint", 4, suite.valSet, altValSet, signers), + ChainID: "gaia", + }, + func(ev *Evidence) {}, + true, + }, + { + "mismatched heights", + Evidence{ + Header1: suite.header, + Header2: MakeHeader("gaia", 6, suite.valSet, altValSet, signers), + ChainID: "gaia", + }, + func(ev *Evidence) {}, + true, + }, + { + "mismatched heights", + Evidence{ + Header1: suite.header, + Header2: MakeHeader("gaia", 6, suite.valSet, altValSet, signers), + ChainID: "gaia", + }, + func(ev *Evidence) {}, + true, + }, + { + "same block id", + Evidence{ + Header1: suite.header, + Header2: suite.header, + ChainID: "gaia", + }, + func(ev *Evidence) {}, + true, + }, + { + "header doesn't have 2/3 majority", + Evidence{ + Header1: suite.header, + Header2: MakeHeader("gaia", 4, altValSet, altValSet, bothSigners), + ChainID: "gaia", + }, + func(ev *Evidence) { + fmt.Printf("ValidatorSet: %v\n\n", wrongValSet) + fmt.Printf("Signer: %v\n\n", altSigners) + wrongVoteSet := tmtypes.NewVoteSet("gaia", ev.Header2.Height, 1, tmtypes.PrecommitType, wrongValSet) + var err error + ev.Header2.Commit, err = tmtypes.MakeCommit(ev.Header2.Commit.BlockID, ev.Header2.Height, ev.Header2.Commit.Round(), wrongVoteSet, altSigners) + if err != nil { + panic(err) + } + }, + true, + }, + { + "validators sign off on wrong commit", + Evidence{ + Header1: suite.header, + Header2: MakeHeader("gaia", 4, altValSet, altValSet, bothSigners), + ChainID: "gaia", + }, + func(ev *Evidence) { + ev.Header2.Commit.BlockID = makeBlockID(tmhash.Sum([]byte("other_hash")), 3, tmhash.Sum([]byte("other_partset"))) + }, + true, + }, + } + + for _, tc := range testCases { + suite.Run(tc.name, func() { + // reset suite for each subtest + suite.SetupTest() + fmt.Println(tc.name) + + tc.malleateEvidence(&tc.evidence) + + err := tc.evidence.ValidateBasic() + + if tc.expErr { + require.NotNil(suite.T(), err, "ValidateBasic did not throw error for invalid evidence") + } else { + require.Nil(suite.T(), err, "ValidateBasic returned error on valid evidence: %s", err) + } + }) + } +} diff --git a/x/ibc/02-client/types/tendermint/tendermint_test.go b/x/ibc/02-client/types/tendermint/tendermint_test.go index e05ae1f70e55..7f8b8e0e863c 100644 --- a/x/ibc/02-client/types/tendermint/tendermint_test.go +++ b/x/ibc/02-client/types/tendermint/tendermint_test.go @@ -24,7 +24,7 @@ func (suite *TendermintTestSuite) SetupTest() { privVal := tmtypes.NewMockPV() val := tmtypes.NewValidator(privVal.GetPubKey(), 10) valSet := tmtypes.NewValidatorSet([]*tmtypes.Validator{val}) - suite.header = MakeHeader(4, valSet, valSet, []tmtypes.PrivValidator{privVal}) + suite.header = MakeHeader("gaia", 4, valSet, valSet, []tmtypes.PrivValidator{privVal}) root := commitment.NewRoot(tmhash.Sum([]byte("my root"))) cs := ConsensusState{ diff --git a/x/ibc/02-client/types/tendermint/test_utils.go b/x/ibc/02-client/types/tendermint/test_utils.go index 7e62b1e04181..bc2b998c2e71 100644 --- a/x/ibc/02-client/types/tendermint/test_utils.go +++ b/x/ibc/02-client/types/tendermint/test_utils.go @@ -21,13 +21,13 @@ func makeBlockID(hash []byte, partSetSize int, partSetHash []byte) tmtypes.Block } -func MakeHeader(height int64, valSet *tmtypes.ValidatorSet, nextValSet *tmtypes.ValidatorSet, signers []tmtypes.PrivValidator) Header { +func MakeHeader(chainID string, height int64, valSet *tmtypes.ValidatorSet, nextValSet *tmtypes.ValidatorSet, signers []tmtypes.PrivValidator) Header { vsetHash := valSet.Hash() nextHash := nextValSet.Hash() timestamp := time.Date(math.MaxInt64, 0, 0, 0, 0, 0, math.MaxInt64, time.UTC) tmHeader := tmtypes.Header{ Version: version.Consensus{Block: 2, App: 2}, - ChainID: "gaia", + ChainID: chainID, Height: height, Time: timestamp, NumTxs: 100, @@ -41,11 +41,11 @@ func MakeHeader(height int64, valSet *tmtypes.ValidatorSet, nextValSet *tmtypes. AppHash: tmhash.Sum([]byte("app_hash")), LastResultsHash: tmhash.Sum([]byte("last_results_hash")), EvidenceHash: tmhash.Sum([]byte("evidence_hash")), - ProposerAddress: signers[0].GetPubKey().Address(), + ProposerAddress: valSet.Proposer.Address, } hhash := tmHeader.Hash() blockID := makeBlockID(hhash, 3, tmhash.Sum([]byte("part_set"))) - voteSet := tmtypes.NewVoteSet("gaia", height, 1, tmtypes.PrecommitType, valSet) + voteSet := tmtypes.NewVoteSet(chainID, height, 1, tmtypes.PrecommitType, valSet) commit, err := tmtypes.MakeCommit(blockID, height, 1, voteSet, signers) if err != nil { panic(err) From 85ab5e1009a8875a5344a9bf8100a169c17ccb21 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 22 Nov 2019 00:09:18 -0800 Subject: [PATCH 13/33] remove debug statements --- x/ibc/02-client/types/tendermint/evidence_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/x/ibc/02-client/types/tendermint/evidence_test.go b/x/ibc/02-client/types/tendermint/evidence_test.go index bbfed2802a52..8448edd8b8fe 100644 --- a/x/ibc/02-client/types/tendermint/evidence_test.go +++ b/x/ibc/02-client/types/tendermint/evidence_test.go @@ -2,7 +2,6 @@ package tendermint import ( "bytes" - "fmt" "github.com/stretchr/testify/require" @@ -101,8 +100,6 @@ func (suite *TendermintTestSuite) TestValidateBasic() { ChainID: "gaia", }, func(ev *Evidence) { - fmt.Printf("ValidatorSet: %v\n\n", wrongValSet) - fmt.Printf("Signer: %v\n\n", altSigners) wrongVoteSet := tmtypes.NewVoteSet("gaia", ev.Header2.Height, 1, tmtypes.PrecommitType, wrongValSet) var err error ev.Header2.Commit, err = tmtypes.MakeCommit(ev.Header2.Commit.BlockID, ev.Header2.Height, ev.Header2.Commit.Round(), wrongVoteSet, altSigners) @@ -130,7 +127,6 @@ func (suite *TendermintTestSuite) TestValidateBasic() { suite.Run(tc.name, func() { // reset suite for each subtest suite.SetupTest() - fmt.Println(tc.name) tc.malleateEvidence(&tc.evidence) From 22faf51e8191ab64372698b529df23358db40603 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 22 Nov 2019 14:13:29 -0800 Subject: [PATCH 14/33] cleanup evidence test --- .../types/tendermint/evidence_test.go | 32 ++++++++----------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/x/ibc/02-client/types/tendermint/evidence_test.go b/x/ibc/02-client/types/tendermint/evidence_test.go index 8448edd8b8fe..b09d57d99b67 100644 --- a/x/ibc/02-client/types/tendermint/evidence_test.go +++ b/x/ibc/02-client/types/tendermint/evidence_test.go @@ -13,10 +13,13 @@ func (suite *TendermintTestSuite) TestValidateBasic() { altPrivVal := tmtypes.NewMockPV() altVal := tmtypes.NewValidator(altPrivVal.GetPubKey(), 4) - altValSet := tmtypes.NewValidatorSet(append(suite.valSet.Validators, altVal)) - wrongValSet := tmtypes.NewValidatorSet([]*tmtypes.Validator{altVal}) + // Create bothValSet with both suite validator and altVal + bothValSet := tmtypes.NewValidatorSet(append(suite.valSet.Validators, altVal)) + // Create alternative validator set with only altVal + altValSet := tmtypes.NewValidatorSet([]*tmtypes.Validator{altVal}) signers := []tmtypes.PrivValidator{suite.privVal} + // Create signer array and ensure it is in same order as bothValSet var bothSigners []tmtypes.PrivValidator if bytes.Compare(altPrivVal.GetPubKey().Address(), suite.privVal.GetPubKey().Address()) == -1 { bothSigners = []tmtypes.PrivValidator{altPrivVal, suite.privVal} @@ -36,7 +39,7 @@ func (suite *TendermintTestSuite) TestValidateBasic() { "valid evidence", Evidence{ Header1: suite.header, - Header2: MakeHeader("gaia", 4, suite.valSet, altValSet, signers), + Header2: MakeHeader("gaia", 4, suite.valSet, bothValSet, signers), ChainID: "gaia", }, func(ev *Evidence) {}, @@ -46,7 +49,7 @@ func (suite *TendermintTestSuite) TestValidateBasic() { "wrong chainID on header1", Evidence{ Header1: suite.header, - Header2: MakeHeader("ethermint", 4, suite.valSet, altValSet, signers), + Header2: MakeHeader("ethermint", 4, suite.valSet, bothValSet, signers), ChainID: "ethermint", }, func(ev *Evidence) {}, @@ -56,7 +59,7 @@ func (suite *TendermintTestSuite) TestValidateBasic() { "wrong chainID on header2", Evidence{ Header1: suite.header, - Header2: MakeHeader("ethermint", 4, suite.valSet, altValSet, signers), + Header2: MakeHeader("ethermint", 4, suite.valSet, bothValSet, signers), ChainID: "gaia", }, func(ev *Evidence) {}, @@ -66,17 +69,7 @@ func (suite *TendermintTestSuite) TestValidateBasic() { "mismatched heights", Evidence{ Header1: suite.header, - Header2: MakeHeader("gaia", 6, suite.valSet, altValSet, signers), - ChainID: "gaia", - }, - func(ev *Evidence) {}, - true, - }, - { - "mismatched heights", - Evidence{ - Header1: suite.header, - Header2: MakeHeader("gaia", 6, suite.valSet, altValSet, signers), + Header2: MakeHeader("gaia", 6, suite.valSet, bothValSet, signers), ChainID: "gaia", }, func(ev *Evidence) {}, @@ -96,11 +89,12 @@ func (suite *TendermintTestSuite) TestValidateBasic() { "header doesn't have 2/3 majority", Evidence{ Header1: suite.header, - Header2: MakeHeader("gaia", 4, altValSet, altValSet, bothSigners), + Header2: MakeHeader("gaia", 4, bothValSet, bothValSet, bothSigners), ChainID: "gaia", }, func(ev *Evidence) { - wrongVoteSet := tmtypes.NewVoteSet("gaia", ev.Header2.Height, 1, tmtypes.PrecommitType, wrongValSet) + // voteSet contains only altVal which is less than 2/3 of total power (4/14) + wrongVoteSet := tmtypes.NewVoteSet("gaia", ev.Header2.Height, 1, tmtypes.PrecommitType, altValSet) var err error ev.Header2.Commit, err = tmtypes.MakeCommit(ev.Header2.Commit.BlockID, ev.Header2.Height, ev.Header2.Commit.Round(), wrongVoteSet, altSigners) if err != nil { @@ -113,7 +107,7 @@ func (suite *TendermintTestSuite) TestValidateBasic() { "validators sign off on wrong commit", Evidence{ Header1: suite.header, - Header2: MakeHeader("gaia", 4, altValSet, altValSet, bothSigners), + Header2: MakeHeader("gaia", 4, bothValSet, bothValSet, bothSigners), ChainID: "gaia", }, func(ev *Evidence) { From 5a10ee0a6655dfda9c42e3a8dcd19299a1cce6df Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 22 Nov 2019 17:35:22 -0800 Subject: [PATCH 15/33] start misbehaviour tests --- .../types/tendermint/evidence_test.go | 2 +- .../types/tendermint/misbehaviour_test.go | 169 ++++++++++++++++++ 2 files changed, 170 insertions(+), 1 deletion(-) create mode 100644 x/ibc/02-client/types/tendermint/misbehaviour_test.go diff --git a/x/ibc/02-client/types/tendermint/evidence_test.go b/x/ibc/02-client/types/tendermint/evidence_test.go index b09d57d99b67..bcdfb7ea83f9 100644 --- a/x/ibc/02-client/types/tendermint/evidence_test.go +++ b/x/ibc/02-client/types/tendermint/evidence_test.go @@ -9,7 +9,7 @@ import ( tmtypes "github.com/tendermint/tendermint/types" ) -func (suite *TendermintTestSuite) TestValidateBasic() { +func (suite *TendermintTestSuite) TestEvidenceValidateBasic() { altPrivVal := tmtypes.NewMockPV() altVal := tmtypes.NewValidator(altPrivVal.GetPubKey(), 4) diff --git a/x/ibc/02-client/types/tendermint/misbehaviour_test.go b/x/ibc/02-client/types/tendermint/misbehaviour_test.go new file mode 100644 index 000000000000..d6e23b6cbf25 --- /dev/null +++ b/x/ibc/02-client/types/tendermint/misbehaviour_test.go @@ -0,0 +1,169 @@ +package tendermint + +import ( + "bytes" + + "github.com/stretchr/testify/require" + tmtypes "github.com/tendermint/tendermint/types" +) + +func (suite *TendermintTestSuite) TestMisbehaviourValidateBasic() { + altPrivVal := tmtypes.NewMockPV() + altVal := tmtypes.NewValidator(altPrivVal.GetPubKey(), 4) + + // Create bothValSet with both suite validator and altVal + bothValSet := tmtypes.NewValidatorSet(append(suite.valSet.Validators, altVal)) + + signers := []tmtypes.PrivValidator{suite.privVal} + testCases := []struct { + name string + evidence *Evidence + clientID string + expErr bool + }{ + { + "valid misbehavior", + &Evidence{ + Header1: suite.header, + Header2: MakeHeader("gaia", 4, suite.valSet, bothValSet, signers), + ChainID: "gaia", + }, + "gaiamainnet", + false, + }, + { + "nil evidence", + nil, + "gaiamainnet", + true, + }, + { + "invalid evidence", + &Evidence{ + Header1: suite.header, + Header2: suite.header, + ChainID: "gaia", + }, + "gaiamainnet", + true, + }, + { + "invalid ClientID", + &Evidence{ + Header1: MakeHeader("gaia123??", 4, suite.valSet, suite.valSet, signers), + Header2: MakeHeader("gaia123?", 4, suite.valSet, suite.valSet, signers), + ChainID: "gaia123?", + }, + "gaia123?", + true, + }, + } + + for _, tc := range testCases { + suite.Run(tc.name, func() { + misbehaviour := Misbehaviour{ + Evidence: tc.evidence, + ClientID: tc.clientID, + } + + err := misbehaviour.ValidateBasic() + + if tc.expErr { + require.NotNil(suite.T(), err, "Invalid Misbehaviour passed ValidateBasic") + } else { + require.Nil(suite.T(), err, "Valid Misbehaviour failed ValidateBasic: %v", err) + } + }) + } +} + +func (suite *TendermintTestSuite) TestCheckMisbehaviour() { + altPrivVal := tmtypes.NewMockPV() + altVal := tmtypes.NewValidator(altPrivVal.GetPubKey(), 4) + + // Create bothValSet with both suite validator and altVal + bothValSet := tmtypes.NewValidatorSet(append(suite.valSet.Validators, altVal)) + // Create alternative validator set with only altVal + altValSet := tmtypes.NewValidatorSet([]*tmtypes.Validator{altVal}) + + signers := []tmtypes.PrivValidator{suite.privVal} + // Create signer array and ensure it is in same order as bothValSet + var bothSigners []tmtypes.PrivValidator + if bytes.Compare(altPrivVal.GetPubKey().Address(), suite.privVal.GetPubKey().Address()) == -1 { + bothSigners = []tmtypes.PrivValidator{altPrivVal, suite.privVal} + } else { + bothSigners = []tmtypes.PrivValidator{suite.privVal, altPrivVal} + } + + altSigners := []tmtypes.PrivValidator{altPrivVal} + + testCases := []struct { + name string + evidence *Evidence + clientID string + committer Committer + expErr bool + }{ + { + "misbehavior should pass", + &Evidence{ + Header1: MakeHeader("gaia", 4, bothValSet, suite.valSet, bothSigners), + Header2: MakeHeader("gaia", 4, bothValSet, bothValSet, signers), + ChainID: "gaia", + }, + "gaiamainnet", + Committer{suite.valSet}, + false, + }, + { + "first valset has too much change", + &Evidence{ + Header1: MakeHeader("gaia", 4, altValSet, bothValSet, altSigners), + Header2: MakeHeader("gaia", 4, bothValSet, bothValSet, bothSigners), + ChainID: "gaia", + }, + "gaiamainnet", + Committer{suite.valSet}, + true, + }, + { + "second valset has too much change", + &Evidence{ + Header1: MakeHeader("gaia", 4, bothValSet, bothValSet, bothSigners), + Header2: MakeHeader("gaia", 4, altValSet, bothValSet, altSigners), + ChainID: "gaia", + }, + "gaiamainnet", + Committer{suite.valSet}, + true, + }, + { + "both valsets have too much change", + &Evidence{ + Header1: MakeHeader("gaia", 4, altValSet, altValSet, altSigners), + Header2: MakeHeader("gaia", 4, altValSet, bothValSet, altSigners), + ChainID: "gaia", + }, + "gaiamainnet", + Committer{suite.valSet}, + true, + }, + } + + for _, tc := range testCases { + suite.Run(tc.name, func() { + misbehaviour := Misbehaviour{ + Evidence: tc.evidence, + ClientID: tc.clientID, + } + + err := CheckMisbehaviour(tc.committer, misbehaviour) + + if tc.expErr { + require.NotNil(suite.T(), err, "CheckMisbehaviour passed unexpectedly") + } else { + require.Nil(suite.T(), err, "CheckMisbehaviour failed unexpectedly: %v", err) + } + }) + } +} From 3d3c3c71bf70bb2ba1babcee2942e2086cc6b71e Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 25 Nov 2019 14:52:44 -0800 Subject: [PATCH 16/33] fix nondeterministic bug --- x/ibc/02-client/types/tendermint/misbehaviour_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/ibc/02-client/types/tendermint/misbehaviour_test.go b/x/ibc/02-client/types/tendermint/misbehaviour_test.go index d6e23b6cbf25..1069481a9f3a 100644 --- a/x/ibc/02-client/types/tendermint/misbehaviour_test.go +++ b/x/ibc/02-client/types/tendermint/misbehaviour_test.go @@ -86,7 +86,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { // Create alternative validator set with only altVal altValSet := tmtypes.NewValidatorSet([]*tmtypes.Validator{altVal}) - signers := []tmtypes.PrivValidator{suite.privVal} // Create signer array and ensure it is in same order as bothValSet var bothSigners []tmtypes.PrivValidator if bytes.Compare(altPrivVal.GetPubKey().Address(), suite.privVal.GetPubKey().Address()) == -1 { @@ -108,7 +107,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { "misbehavior should pass", &Evidence{ Header1: MakeHeader("gaia", 4, bothValSet, suite.valSet, bothSigners), - Header2: MakeHeader("gaia", 4, bothValSet, bothValSet, signers), + Header2: MakeHeader("gaia", 4, bothValSet, bothValSet, bothSigners), ChainID: "gaia", }, "gaiamainnet", From 373601a8e5069b6554194e4c2c47069cbce7a3a7 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 26 Nov 2019 11:10:34 -0800 Subject: [PATCH 17/33] add same height and next height checks in misbehaviour --- x/ibc/02-client/exported/exported.go | 1 + x/ibc/02-client/types/tendermint/committer.go | 6 +++ .../types/tendermint/consensus_state.go | 6 ++- x/ibc/02-client/types/tendermint/header.go | 6 ++- .../types/tendermint/misbehaviour.go | 27 +++++++++++- .../types/tendermint/misbehaviour_test.go | 41 ++++++++++--------- 6 files changed, 64 insertions(+), 23 deletions(-) diff --git a/x/ibc/02-client/exported/exported.go b/x/ibc/02-client/exported/exported.go index a596c0c1bda7..d9e97b5dab31 100644 --- a/x/ibc/02-client/exported/exported.go +++ b/x/ibc/02-client/exported/exported.go @@ -42,6 +42,7 @@ type Header interface { // updating the consensusState at a given height type Committer interface { ClientType() ClientType + GetHeight() uint64 } // ClientType defines the type of the consensus algorithm diff --git a/x/ibc/02-client/types/tendermint/committer.go b/x/ibc/02-client/types/tendermint/committer.go index d6ce6049b882..86615430c0dd 100644 --- a/x/ibc/02-client/types/tendermint/committer.go +++ b/x/ibc/02-client/types/tendermint/committer.go @@ -10,8 +10,14 @@ var _ exported.Committer = Committer{} type Committer struct { *tmtypes.ValidatorSet + Height uint64 + NextValSetHash []byte } func (c Committer) ClientType() exported.ClientType { return exported.Tendermint } + +func (c Committer) GetHeight() uint64 { + return c.Height +} diff --git a/x/ibc/02-client/types/tendermint/consensus_state.go b/x/ibc/02-client/types/tendermint/consensus_state.go index 1f5d4aa6672a..4c81d2cf4437 100644 --- a/x/ibc/02-client/types/tendermint/consensus_state.go +++ b/x/ibc/02-client/types/tendermint/consensus_state.go @@ -40,7 +40,11 @@ func (cs ConsensusState) GetRoot() commitment.RootI { } func (cs ConsensusState) GetCommitter() exported.Committer { - return Committer{cs.ValidatorSet} + return Committer{ + ValidatorSet: cs.ValidatorSet, + Height: cs.Height, + NextValSetHash: cs.NextValidatorSet.Hash(), + } } // CheckValidityAndUpdateState checks if the provided header is valid and updates diff --git a/x/ibc/02-client/types/tendermint/header.go b/x/ibc/02-client/types/tendermint/header.go index 375f23311931..133c1741e56e 100644 --- a/x/ibc/02-client/types/tendermint/header.go +++ b/x/ibc/02-client/types/tendermint/header.go @@ -24,7 +24,11 @@ func (h Header) ClientType() exported.ClientType { // GetCommitter returns the ValidatorSet that committed header func (h Header) GetCommitter() exported.Committer { - return Committer{h.ValidatorSet} + return Committer{ + ValidatorSet: h.ValidatorSet, + Height: uint64(h.Height), + NextValSetHash: h.NextValidatorsHash, + } } // GetHeight returns the current height diff --git a/x/ibc/02-client/types/tendermint/misbehaviour.go b/x/ibc/02-client/types/tendermint/misbehaviour.go index dceb3d16d329..c001cb1806b0 100644 --- a/x/ibc/02-client/types/tendermint/misbehaviour.go +++ b/x/ibc/02-client/types/tendermint/misbehaviour.go @@ -1,6 +1,9 @@ package tendermint import ( + "bytes" + "fmt" + evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/errors" @@ -45,9 +48,31 @@ func CheckMisbehaviour(trustedCommitter Committer, m Misbehaviour) error { if err := m.ValidateBasic(); err != nil { return err } + + trustedValSet := trustedCommitter.ValidatorSet + + // Evidence is on same height as trustedCommiter. ValidatorSets must be the same + if trustedCommitter.GetHeight() == uint64(m.GetHeight()) { + if bytes.Equal(trustedValSet.Hash(), m.Evidence.Header1.ValidatorSet.Hash()) || + !bytes.Equal(trustedValSet.Hash(), m.Evidence.Header2.ValidatorSet.Hash()) { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("validator set is not valid for height: %d", m.GetHeight())) + } + return nil + } + + // Evidence is on next height of trustedCommitter. Evidence Validator Set must match + // trustedCommitter's NextValSetHash + if trustedCommitter.GetHeight()+1 == uint64(m.GetHeight()) { + if !bytes.Equal(trustedCommitter.NextValSetHash, m.Evidence.Header1.ValidatorSet.Hash()) || + !bytes.Equal(trustedCommitter.NextValSetHash, m.Evidence.Header2.ValidatorSet.Hash()) { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("validator set is not valid for height: %d", m.GetHeight())) + } + return nil + } + + // Evidence is within trusting period. ValidatorSet must have 2/3 similarity with trustedCommitter ValidatorSet // check that the validator sets on both headers are valid given the last trusted validatorset // less than or equal to evidence height - trustedValSet := trustedCommitter.ValidatorSet if err := trustedValSet.VerifyFutureCommit(m.Evidence.Header1.ValidatorSet, m.Evidence.ChainID, m.Evidence.Header1.Commit.BlockID, m.Evidence.Header1.Height, m.Evidence.Header1.Commit); err != nil { return err diff --git a/x/ibc/02-client/types/tendermint/misbehaviour_test.go b/x/ibc/02-client/types/tendermint/misbehaviour_test.go index 1069481a9f3a..f69791126563 100644 --- a/x/ibc/02-client/types/tendermint/misbehaviour_test.go +++ b/x/ibc/02-client/types/tendermint/misbehaviour_test.go @@ -50,8 +50,8 @@ func (suite *TendermintTestSuite) TestMisbehaviourValidateBasic() { { "invalid ClientID", &Evidence{ - Header1: MakeHeader("gaia123??", 4, suite.valSet, suite.valSet, signers), - Header2: MakeHeader("gaia123?", 4, suite.valSet, suite.valSet, signers), + Header1: MakeHeader("gaia123??", 5, suite.valSet, suite.valSet, signers), + Header2: MakeHeader("gaia123?", 5, suite.valSet, suite.valSet, signers), ChainID: "gaia123?", }, "gaia123?", @@ -96,55 +96,56 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { altSigners := []tmtypes.PrivValidator{altPrivVal} + committer := Committer{ + ValidatorSet: suite.valSet, + Height: 3, + NextValSetHash: suite.valSet.Hash(), + } + testCases := []struct { - name string - evidence *Evidence - clientID string - committer Committer - expErr bool + name string + evidence *Evidence + clientID string + expErr bool }{ { "misbehavior should pass", &Evidence{ - Header1: MakeHeader("gaia", 4, bothValSet, suite.valSet, bothSigners), - Header2: MakeHeader("gaia", 4, bothValSet, bothValSet, bothSigners), + Header1: MakeHeader("gaia", 5, bothValSet, suite.valSet, bothSigners), + Header2: MakeHeader("gaia", 5, bothValSet, bothValSet, bothSigners), ChainID: "gaia", }, "gaiamainnet", - Committer{suite.valSet}, false, }, { "first valset has too much change", &Evidence{ - Header1: MakeHeader("gaia", 4, altValSet, bothValSet, altSigners), - Header2: MakeHeader("gaia", 4, bothValSet, bothValSet, bothSigners), + Header1: MakeHeader("gaia", 5, altValSet, bothValSet, altSigners), + Header2: MakeHeader("gaia", 5, bothValSet, bothValSet, bothSigners), ChainID: "gaia", }, "gaiamainnet", - Committer{suite.valSet}, true, }, { "second valset has too much change", &Evidence{ - Header1: MakeHeader("gaia", 4, bothValSet, bothValSet, bothSigners), - Header2: MakeHeader("gaia", 4, altValSet, bothValSet, altSigners), + Header1: MakeHeader("gaia", 5, bothValSet, bothValSet, bothSigners), + Header2: MakeHeader("gaia", 5, altValSet, bothValSet, altSigners), ChainID: "gaia", }, "gaiamainnet", - Committer{suite.valSet}, true, }, { "both valsets have too much change", &Evidence{ - Header1: MakeHeader("gaia", 4, altValSet, altValSet, altSigners), - Header2: MakeHeader("gaia", 4, altValSet, bothValSet, altSigners), + Header1: MakeHeader("gaia", 5, altValSet, altValSet, altSigners), + Header2: MakeHeader("gaia", 5, altValSet, bothValSet, altSigners), ChainID: "gaia", }, "gaiamainnet", - Committer{suite.valSet}, true, }, } @@ -156,7 +157,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { ClientID: tc.clientID, } - err := CheckMisbehaviour(tc.committer, misbehaviour) + err := CheckMisbehaviour(committer, misbehaviour) if tc.expErr { require.NotNil(suite.T(), err, "CheckMisbehaviour passed unexpectedly") From d420a0eb95a158e0c77ba0598de1e029b5dc002e Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 26 Nov 2019 12:43:35 -0800 Subject: [PATCH 18/33] fix bugs --- .../types/tendermint/misbehaviour.go | 2 +- .../types/tendermint/misbehaviour_test.go | 49 +++++++++++++++++-- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/x/ibc/02-client/types/tendermint/misbehaviour.go b/x/ibc/02-client/types/tendermint/misbehaviour.go index c001cb1806b0..8f879ec7570d 100644 --- a/x/ibc/02-client/types/tendermint/misbehaviour.go +++ b/x/ibc/02-client/types/tendermint/misbehaviour.go @@ -53,7 +53,7 @@ func CheckMisbehaviour(trustedCommitter Committer, m Misbehaviour) error { // Evidence is on same height as trustedCommiter. ValidatorSets must be the same if trustedCommitter.GetHeight() == uint64(m.GetHeight()) { - if bytes.Equal(trustedValSet.Hash(), m.Evidence.Header1.ValidatorSet.Hash()) || + if !bytes.Equal(trustedValSet.Hash(), m.Evidence.Header1.ValidatorSet.Hash()) || !bytes.Equal(trustedValSet.Hash(), m.Evidence.Header2.ValidatorSet.Hash()) { return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("validator set is not valid for height: %d", m.GetHeight())) } diff --git a/x/ibc/02-client/types/tendermint/misbehaviour_test.go b/x/ibc/02-client/types/tendermint/misbehaviour_test.go index f69791126563..4a7f57116ba1 100644 --- a/x/ibc/02-client/types/tendermint/misbehaviour_test.go +++ b/x/ibc/02-client/types/tendermint/misbehaviour_test.go @@ -69,7 +69,7 @@ func (suite *TendermintTestSuite) TestMisbehaviourValidateBasic() { err := misbehaviour.ValidateBasic() if tc.expErr { - require.NotNil(suite.T(), err, "Invalid Misbehaviour passed ValidateBasic") + suite.Error(err, "Invalid Misbehaviour passed ValidateBasic") } else { require.Nil(suite.T(), err, "Valid Misbehaviour failed ValidateBasic: %v", err) } @@ -94,6 +94,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { bothSigners = []tmtypes.PrivValidator{suite.privVal, altPrivVal} } + signers := []tmtypes.PrivValidator{suite.privVal} altSigners := []tmtypes.PrivValidator{altPrivVal} committer := Committer{ @@ -109,7 +110,47 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { expErr bool }{ { - "misbehavior should pass", + "same height misbehaviour should pass", + &Evidence{ + Header1: MakeHeader("gaia", 3, suite.valSet, altValSet, signers), + Header2: MakeHeader("gaia", 3, suite.valSet, bothValSet, signers), + ChainID: "gaia", + }, + "gaiamainnet", + false, + }, + { + "same height misbehaviour with wrong valSet", + &Evidence{ + Header1: MakeHeader("gaia", 3, bothValSet, altValSet, bothSigners), + Header2: MakeHeader("gaia", 3, bothValSet, bothValSet, bothSigners), + ChainID: "gaia", + }, + "gaiamainnet", + true, + }, + { + "next height misbehaviour should pass", + &Evidence{ + Header1: MakeHeader("gaia", 4, suite.valSet, altValSet, signers), + Header2: MakeHeader("gaia", 4, suite.valSet, bothValSet, signers), + ChainID: "gaia", + }, + "gaiamainnet", + false, + }, + { + "next height misbehaviour with wrong valSet", + &Evidence{ + Header1: MakeHeader("gaia", 4, bothValSet, altValSet, bothSigners), + Header2: MakeHeader("gaia", 4, bothValSet, bothValSet, bothSigners), + ChainID: "gaia", + }, + "gaiamainnet", + true, + }, + { + "trusting period misbehavior should pass", &Evidence{ Header1: MakeHeader("gaia", 5, bothValSet, suite.valSet, bothSigners), Header2: MakeHeader("gaia", 5, bothValSet, bothValSet, bothSigners), @@ -160,9 +201,9 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { err := CheckMisbehaviour(committer, misbehaviour) if tc.expErr { - require.NotNil(suite.T(), err, "CheckMisbehaviour passed unexpectedly") + suite.Error(err, "CheckMisbehaviour passed unexpectedly") } else { - require.Nil(suite.T(), err, "CheckMisbehaviour failed unexpectedly: %v", err) + suite.NoError(err, "CheckMisbehaviour failed unexpectedly: %v", err) } }) } From 4b1aa96c23e396056875dfb8898b688719b135a6 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 26 Nov 2019 12:50:41 -0800 Subject: [PATCH 19/33] apply fede review suggestions --- .../02-client/types/tendermint/consensus_state_test.go | 10 +++++----- x/ibc/02-client/types/tendermint/evidence_test.go | 6 ++---- x/ibc/02-client/types/tendermint/misbehaviour_test.go | 3 +-- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/x/ibc/02-client/types/tendermint/consensus_state_test.go b/x/ibc/02-client/types/tendermint/consensus_state_test.go index 74aff1eebeca..778de45fa3a8 100644 --- a/x/ibc/02-client/types/tendermint/consensus_state_test.go +++ b/x/ibc/02-client/types/tendermint/consensus_state_test.go @@ -9,12 +9,12 @@ import ( func (suite *TendermintTestSuite) TestCheckValidity() { // valid header err := suite.cs.checkValidity(suite.header) - require.Nil(suite.T(), err, "validity failed") + suite.NoError(err, "validity failed") // switch out header ValidatorsHash suite.header.ValidatorsHash = tmhash.Sum([]byte("hello")) err = suite.cs.checkValidity(suite.header) - require.NotNil(suite.T(), err, "validator hash is wrong") + suite.Error(err, "validator hash is wrong") // reset suite and make header.NextValidatorSet different // from NextValidatorSetHash @@ -23,13 +23,13 @@ func (suite *TendermintTestSuite) TestCheckValidity() { val := tmtypes.NewValidator(privVal.GetPubKey(), 5) suite.header.NextValidatorSet = tmtypes.NewValidatorSet([]*tmtypes.Validator{val}) err = suite.cs.checkValidity(suite.header) - require.NotNil(suite.T(), err, "header's next validator set is not consistent with hash") + suite.Error(err, "header's next validator set is not consistent with hash") // reset and make header fail validatebasic suite.SetupTest() suite.header.ChainID = "not_gaia" err = suite.cs.checkValidity(suite.header) - require.NotNil(suite.T(), err, "invalid header should fail ValidateBasic") + suite.Error(err, "invalid header should fail ValidateBasic") } func (suite *TendermintTestSuite) TestCheckUpdate() { @@ -48,6 +48,6 @@ func (suite *TendermintTestSuite) TestCheckUpdate() { suite.header.ChainID = "not_gaia" cs, err = suite.cs.CheckValidityAndUpdateState(suite.header) - require.NotNil(suite.T(), err) + suite.Error(err) require.Nil(suite.T(), cs) } diff --git a/x/ibc/02-client/types/tendermint/evidence_test.go b/x/ibc/02-client/types/tendermint/evidence_test.go index bcdfb7ea83f9..af3589550031 100644 --- a/x/ibc/02-client/types/tendermint/evidence_test.go +++ b/x/ibc/02-client/types/tendermint/evidence_test.go @@ -3,8 +3,6 @@ package tendermint import ( "bytes" - "github.com/stretchr/testify/require" - "github.com/tendermint/tendermint/crypto/tmhash" tmtypes "github.com/tendermint/tendermint/types" ) @@ -127,9 +125,9 @@ func (suite *TendermintTestSuite) TestEvidenceValidateBasic() { err := tc.evidence.ValidateBasic() if tc.expErr { - require.NotNil(suite.T(), err, "ValidateBasic did not throw error for invalid evidence") + suite.Error(err, "ValidateBasic did not throw error for invalid evidence") } else { - require.Nil(suite.T(), err, "ValidateBasic returned error on valid evidence: %s", err) + suite.NoError(err, "ValidateBasic returned error on valid evidence: %s", err) } }) } diff --git a/x/ibc/02-client/types/tendermint/misbehaviour_test.go b/x/ibc/02-client/types/tendermint/misbehaviour_test.go index 4a7f57116ba1..57ce93eebf1c 100644 --- a/x/ibc/02-client/types/tendermint/misbehaviour_test.go +++ b/x/ibc/02-client/types/tendermint/misbehaviour_test.go @@ -3,7 +3,6 @@ package tendermint import ( "bytes" - "github.com/stretchr/testify/require" tmtypes "github.com/tendermint/tendermint/types" ) @@ -71,7 +70,7 @@ func (suite *TendermintTestSuite) TestMisbehaviourValidateBasic() { if tc.expErr { suite.Error(err, "Invalid Misbehaviour passed ValidateBasic") } else { - require.Nil(suite.T(), err, "Valid Misbehaviour failed ValidateBasic: %v", err) + suite.NoError(err, "Valid Misbehaviour failed ValidateBasic: %v", err) } }) } From 09d834b0fa742a07a101ff56dfb79e724bf00ee6 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 26 Nov 2019 13:08:34 -0800 Subject: [PATCH 20/33] finish code review changes --- x/ibc/02-client/types/codec.go | 2 ++ x/ibc/02-client/types/tendermint/codec.go | 1 + x/ibc/02-client/types/tendermint/evidence.go | 10 +++++----- x/ibc/02-client/types/tendermint/misbehaviour.go | 4 ++-- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/x/ibc/02-client/types/codec.go b/x/ibc/02-client/types/codec.go index b6322238e2ef..ed923f5cbc89 100644 --- a/x/ibc/02-client/types/codec.go +++ b/x/ibc/02-client/types/codec.go @@ -12,6 +12,7 @@ var SubModuleCdc *codec.Codec // RegisterCodec registers the IBC client interfaces and types func RegisterCodec(cdc *codec.Codec) { cdc.RegisterInterface((*exported.ConsensusState)(nil), nil) + cdc.RegisterInterface((*exported.Committer)(nil), nil) cdc.RegisterInterface((*exported.Header)(nil), nil) cdc.RegisterInterface((*exported.Misbehaviour)(nil), nil) @@ -19,6 +20,7 @@ func RegisterCodec(cdc *codec.Codec) { cdc.RegisterConcrete(MsgUpdateClient{}, "ibc/client/MsgUpdateClient", nil) cdc.RegisterConcrete(tendermint.ConsensusState{}, "ibc/client/tendermint/ConsensusState", nil) + cdc.RegisterConcrete(tendermint.Committer{}, "ibc/client/tendermint/Committer", nil) cdc.RegisterConcrete(tendermint.Header{}, "ibc/client/tendermint/Header", nil) cdc.RegisterConcrete(tendermint.Evidence{}, "ibc/client/tendermint/Evidence", nil) diff --git a/x/ibc/02-client/types/tendermint/codec.go b/x/ibc/02-client/types/tendermint/codec.go index 94dcdf4f2d0c..a546ed313cef 100644 --- a/x/ibc/02-client/types/tendermint/codec.go +++ b/x/ibc/02-client/types/tendermint/codec.go @@ -9,6 +9,7 @@ var SubModuleCdc = codec.New() // RegisterCodec registers the Tendermint types func RegisterCodec(cdc *codec.Codec) { codec.RegisterCrypto(cdc) + cdc.RegisterConcrete(Committer{}, "ibc/client/tendermint/Committer", nil) cdc.RegisterConcrete(ConsensusState{}, "ibc/client/tendermint/ConsensusState", nil) cdc.RegisterConcrete(Header{}, "ibc/client/tendermint/Header", nil) cdc.RegisterConcrete(Misbehaviour{}, "ibc/client/tendermint/Misbehaviour", nil) diff --git a/x/ibc/02-client/types/tendermint/evidence.go b/x/ibc/02-client/types/tendermint/evidence.go index 4d5e2310a7bf..850c21b4fb1c 100644 --- a/x/ibc/02-client/types/tendermint/evidence.go +++ b/x/ibc/02-client/types/tendermint/evidence.go @@ -18,9 +18,9 @@ var _ evidenceexported.Evidence = Evidence{} // Evidence is a wrapper over tendermint's DuplicateVoteEvidence // that implements Evidence interface expected by ICS-02 type Evidence struct { - Header1 Header - Header2 Header - ChainID string + Header1 Header `json: "header1" yaml: "header1"` + Header2 Header `json: "header2" yaml: "header2"` + ChainID string `json: "chain_id" yaml: "chain_id"` } // Route implements Evidence interface @@ -56,10 +56,10 @@ func (ev Evidence) GetHeight() int64 { func (ev Evidence) ValidateBasic() error { // ValidateBasic on both validators if err := ev.Header1.ValidateBasic(ev.ChainID); err != nil { - return errors.ErrInvalidEvidence(errors.DefaultCodespace, err.Error()) + return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("Header1 failed ValidateBasic: %v", err)) } if err := ev.Header2.ValidateBasic(ev.ChainID); err != nil { - return errors.ErrInvalidEvidence(errors.DefaultCodespace, err.Error()) + return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("Header2 failed ValidateBasic: %v", err)) } // Ensure that Heights are the same if ev.Header1.Height != ev.Header2.Height { diff --git a/x/ibc/02-client/types/tendermint/misbehaviour.go b/x/ibc/02-client/types/tendermint/misbehaviour.go index 8f879ec7570d..a4c02b88a119 100644 --- a/x/ibc/02-client/types/tendermint/misbehaviour.go +++ b/x/ibc/02-client/types/tendermint/misbehaviour.go @@ -75,11 +75,11 @@ func CheckMisbehaviour(trustedCommitter Committer, m Misbehaviour) error { // less than or equal to evidence height if err := trustedValSet.VerifyFutureCommit(m.Evidence.Header1.ValidatorSet, m.Evidence.ChainID, m.Evidence.Header1.Commit.BlockID, m.Evidence.Header1.Height, m.Evidence.Header1.Commit); err != nil { - return err + return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("validator set in Header1 has too much change from last known committer: %v", err)) } if err := trustedValSet.VerifyFutureCommit(m.Evidence.Header2.ValidatorSet, m.Evidence.ChainID, m.Evidence.Header2.Commit.BlockID, m.Evidence.Header2.Height, m.Evidence.Header2.Commit); err != nil { - return err + return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("validator set in Header2 has too much change from last known committer: %v", err)) } return nil From 3b3cb992bac6318fec5603cb4efe4755a786627c Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 27 Nov 2019 15:42:58 -0800 Subject: [PATCH 21/33] fix GetCommitter and write keeper-level misbehaviour tests --- x/ibc/02-client/keeper/client_test.go | 105 ++++++++++++++++++++++++-- x/ibc/02-client/keeper/keeper.go | 17 +++-- x/ibc/02-client/keeper/keeper_test.go | 46 +++++++++++ 3 files changed, 155 insertions(+), 13 deletions(-) diff --git a/x/ibc/02-client/keeper/client_test.go b/x/ibc/02-client/keeper/client_test.go index 4f46b887e3d0..82ca52721eb3 100644 --- a/x/ibc/02-client/keeper/client_test.go +++ b/x/ibc/02-client/keeper/client_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + "bytes" "fmt" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" @@ -18,7 +19,7 @@ const ( func (suite *KeeperTestSuite) TestCreateClient() { // Test Valid CreateClient state, err := suite.keeper.CreateClient(suite.ctx, testClientID, exported.Tendermint, suite.consensusState) - require.Nil(suite.T(), err, "CreateClient failed") + suite.NoError(err, "CreateClient failed") // Test ClientState stored correctly expectedState := types.State{ @@ -35,7 +36,7 @@ func (suite *KeeperTestSuite) TestCreateClient() { // Test that trying to CreateClient on existing client fails _, err = suite.keeper.CreateClient(suite.ctx, testClientID, exported.Tendermint, suite.consensusState) - require.NotNil(suite.T(), err, "CreateClient on existing client: %s passed", testClientID) + suite.Error(err, "CreateClient on existing client: %s passed", testClientID) } func (suite *KeeperTestSuite) TestUpdateClient() { @@ -83,23 +84,24 @@ func (suite *KeeperTestSuite) TestUpdateClient() { suite.SetupTest() _, err := suite.keeper.CreateClient(suite.ctx, testClientID, exported.Tendermint, suite.consensusState) - require.Nil(suite.T(), err, "CreateClient failed") + suite.NoError(err, "CreateClient failed") tc.malleate() err = suite.keeper.UpdateClient(suite.ctx, testClientID, suite.header) retrievedConsState, _ := suite.keeper.GetConsensusState(suite.ctx, testClientID) tmConsState, _ := retrievedConsState.(tendermint.ConsensusState) + tmConsState.ValidatorSet.TotalVotingPower() tmConsState.NextValidatorSet.TotalVotingPower() retrievedRoot, _ := suite.keeper.GetVerifiedRoot(suite.ctx, testClientID, suite.consensusState.GetHeight()+1) if tc.expErr { - require.NotNil(suite.T(), err, "Invalid UpdateClient passed", tc.name) + suite.Error(err, "Invalid UpdateClient passed", tc.name) // require no state changes occurred require.Equal(suite.T(), suite.consensusState, tmConsState, "Consensus state changed after invalid UpdateClient") require.Nil(suite.T(), retrievedRoot, "Root added for new height after invalid UpdateClient") } else { - require.Nil(suite.T(), err, "Valid UpdateClient failed", tc.name) + suite.NoError(err, "Valid UpdateClient failed", tc.name) // require state changes were performed correctly require.Equal(suite.T(), suite.header.GetHeight(), retrievedConsState.GetHeight(), "height not updated correctly") @@ -111,3 +113,96 @@ func (suite *KeeperTestSuite) TestUpdateClient() { }) } } + +func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { + _, err := suite.keeper.CreateClient(suite.ctx, "gaiamainnet", exported.Tendermint, suite.consensusState) + suite.NoError(err, "CreateClient failed") + + altPrivVal := tmtypes.NewMockPV() + altVal := tmtypes.NewValidator(altPrivVal.GetPubKey(), 4) + + // Create bothValSet with both suite validator and altVal + bothValSet := tmtypes.NewValidatorSet(append(suite.valSet.Validators, altVal)) + // Create alternative validator set with only altVal + altValSet := tmtypes.NewValidatorSet([]*tmtypes.Validator{altVal}) + + // Create signer array and ensure it is in same order as bothValSet + var bothSigners []tmtypes.PrivValidator + if bytes.Compare(altPrivVal.GetPubKey().Address(), suite.privVal.GetPubKey().Address()) == -1 { + bothSigners = []tmtypes.PrivValidator{altPrivVal, suite.privVal} + } else { + bothSigners = []tmtypes.PrivValidator{suite.privVal, altPrivVal} + } + + altSigners := []tmtypes.PrivValidator{altPrivVal} + + testCases := []struct { + name string + evidence *tendermint.Evidence + clientID string + expErr bool + }{ + { + "trusting period misbehavior should pass", + &tendermint.Evidence{ + Header1: tendermint.MakeHeader("gaia", 5, bothValSet, suite.valSet, bothSigners), + Header2: tendermint.MakeHeader("gaia", 5, bothValSet, bothValSet, bothSigners), + ChainID: "gaia", + }, + "gaiamainnet", + false, + }, + { + "first valset has too much change", + &tendermint.Evidence{ + Header1: tendermint.MakeHeader("gaia", 5, altValSet, bothValSet, altSigners), + Header2: tendermint.MakeHeader("gaia", 5, bothValSet, bothValSet, bothSigners), + ChainID: "gaia", + }, + "gaiamainnet", + true, + }, + { + "second valset has too much change", + &tendermint.Evidence{ + Header1: tendermint.MakeHeader("gaia", 5, bothValSet, bothValSet, bothSigners), + Header2: tendermint.MakeHeader("gaia", 5, altValSet, bothValSet, altSigners), + ChainID: "gaia", + }, + "gaiamainnet", + true, + }, + { + "both valsets have too much change", + &tendermint.Evidence{ + Header1: tendermint.MakeHeader("gaia", 5, altValSet, altValSet, altSigners), + Header2: tendermint.MakeHeader("gaia", 5, altValSet, bothValSet, altSigners), + ChainID: "gaia", + }, + "gaiamainnet", + true, + }, + } + + for _, tc := range testCases { + suite.Run(tc.name, func() { + misbehaviour := tendermint.Misbehaviour{ + Evidence: tc.evidence, + ClientID: tc.clientID, + } + + err = suite.keeper.CheckMisbehaviourAndUpdateState(suite.ctx, misbehaviour) + + if tc.expErr { + suite.Error(err, "CheckMisbehaviour passed unexpectedly") + } else { + suite.NoError(err, "CheckMisbehaviour failed unexpectedly: %v", err) + } + + // reset Frozen flag to false + clientState, _ := suite.keeper.GetClientState(suite.ctx, tc.clientID) + clientState.Frozen = false + suite.keeper.SetClientState(suite.ctx, clientState) + }) + } +} diff --git a/x/ibc/02-client/keeper/keeper.go b/x/ibc/02-client/keeper/keeper.go index 0d09cf792e9c..e5c4feb26263 100644 --- a/x/ibc/02-client/keeper/keeper.go +++ b/x/ibc/02-client/keeper/keeper.go @@ -123,16 +123,17 @@ func (k Keeper) SetVerifiedRoot(ctx sdk.Context, clientID string, height uint64, func (k Keeper) GetCommitter(ctx sdk.Context, clientID string, height uint64) (exported.Committer, bool) { store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixClient) - iter := store.ReverseIterator(types.KeyCommitter(clientID, height), types.KeyCommitter(clientID, 0)) + var committer exported.Committer - if !iter.Valid() { - return nil, false + // TODO: Replace this for-loop with a ReverseIterator for efficiency + for i := height; i > 0; i-- { + bz := store.Get(types.KeyCommitter(clientID, i)) + if bz != nil { + k.cdc.MustUnmarshalBinaryLengthPrefixed(bz, &committer) + return committer, true + } } - - bz := iter.Value() - var committer exported.Committer - k.cdc.MustUnmarshalBinaryLengthPrefixed(bz, &committer) - return committer, true + return nil, false } // SetCommitter sets a committer from a particular height to diff --git a/x/ibc/02-client/keeper/keeper_test.go b/x/ibc/02-client/keeper/keeper_test.go index 9fdc554edbba..83ad1cc166e7 100644 --- a/x/ibc/02-client/keeper/keeper_test.go +++ b/x/ibc/02-client/keeper/keeper_test.go @@ -4,6 +4,7 @@ import ( "testing" abci "github.com/tendermint/tendermint/abci/types" + "github.com/tendermint/tendermint/crypto/tmhash" tmtypes "github.com/tendermint/tendermint/types" "github.com/cosmos/cosmos-sdk/codec" @@ -54,6 +55,7 @@ func (suite *KeeperTestSuite) SetupTest() { ChainID: testClientID, Height: 3, Root: commitment.NewRoot([]byte("hash")), + ValidatorSet: suite.valSet, NextValidatorSet: suite.valSet, } } @@ -87,6 +89,7 @@ func (suite *KeeperTestSuite) TestSetConsensusState() { require.True(suite.T(), ok, "GetConsensusState failed") tmConsState, _ := retrievedConsState.(tendermint.ConsensusState) // force recalculation of unexported totalVotingPower so we can compare consensusState + tmConsState.ValidatorSet.TotalVotingPower() tmConsState.NextValidatorSet.TotalVotingPower() require.Equal(suite.T(), suite.consensusState, tmConsState, "ConsensusState not stored correctly") } @@ -100,3 +103,46 @@ func (suite *KeeperTestSuite) TestSetVerifiedRoot() { require.True(suite.T(), ok, "GetVerifiedRoot failed") require.Equal(suite.T(), root, retrievedRoot, "Root stored incorrectly") } + +func (suite KeeperTestSuite) TestSetCommitter() { + committer := tendermint.Committer{ + ValidatorSet: suite.valSet, + Height: 3, + NextValSetHash: suite.valSet.Hash(), + } + nextCommitter := tendermint.Committer{ + ValidatorSet: suite.valSet, + Height: 6, + NextValSetHash: tmhash.Sum([]byte("next_hash")), + } + + suite.keeper.SetCommitter(suite.ctx, "gaia", 3, committer) + suite.keeper.SetCommitter(suite.ctx, "gaia", 6, nextCommitter) + + for i := 0; i < 3; i++ { + committer, ok := suite.keeper.GetCommitter(suite.ctx, "gaia", uint64(i)) + require.False(suite.T(), ok, "GetCommitter passed on nonexistent height: %d", i) + require.Nil(suite.T(), committer, "GetCommitter returned committer on nonexistent height: %d", i) + } + + for i := 3; i < 6; i++ { + recv, ok := suite.keeper.GetCommitter(suite.ctx, "gaia", uint64(i)) + tmRecv, _ := recv.(tendermint.Committer) + if tmRecv.ValidatorSet != nil { + tmRecv.ValidatorSet.TotalVotingPower() + } + require.True(suite.T(), ok, "GetCommitter failed on existing height: %d", i) + require.Equal(suite.T(), committer, recv, "GetCommitter returned committer on nonexistent height: %d", i) + } + + for i := 6; i < 9; i++ { + recv, ok := suite.keeper.GetCommitter(suite.ctx, "gaia", uint64(i)) + tmRecv, _ := recv.(tendermint.Committer) + if tmRecv.ValidatorSet != nil { + tmRecv.ValidatorSet.TotalVotingPower() + } + require.True(suite.T(), ok, "GetCommitter failed on existing height: %d", i) + require.Equal(suite.T(), nextCommitter, recv, "GetCommitter returned committer on nonexistent height: %d", i) + } + +} From 4764655d0fc6f6a23128eacbd54c456ec733a335 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 27 Nov 2019 15:46:02 -0800 Subject: [PATCH 22/33] remove incorrect special case checking --- .../types/tendermint/misbehaviour.go | 20 --------- .../types/tendermint/misbehaviour_test.go | 41 ------------------- 2 files changed, 61 deletions(-) diff --git a/x/ibc/02-client/types/tendermint/misbehaviour.go b/x/ibc/02-client/types/tendermint/misbehaviour.go index a4c02b88a119..ad11be35bb6d 100644 --- a/x/ibc/02-client/types/tendermint/misbehaviour.go +++ b/x/ibc/02-client/types/tendermint/misbehaviour.go @@ -1,7 +1,6 @@ package tendermint import ( - "bytes" "fmt" evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported" @@ -51,25 +50,6 @@ func CheckMisbehaviour(trustedCommitter Committer, m Misbehaviour) error { trustedValSet := trustedCommitter.ValidatorSet - // Evidence is on same height as trustedCommiter. ValidatorSets must be the same - if trustedCommitter.GetHeight() == uint64(m.GetHeight()) { - if !bytes.Equal(trustedValSet.Hash(), m.Evidence.Header1.ValidatorSet.Hash()) || - !bytes.Equal(trustedValSet.Hash(), m.Evidence.Header2.ValidatorSet.Hash()) { - return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("validator set is not valid for height: %d", m.GetHeight())) - } - return nil - } - - // Evidence is on next height of trustedCommitter. Evidence Validator Set must match - // trustedCommitter's NextValSetHash - if trustedCommitter.GetHeight()+1 == uint64(m.GetHeight()) { - if !bytes.Equal(trustedCommitter.NextValSetHash, m.Evidence.Header1.ValidatorSet.Hash()) || - !bytes.Equal(trustedCommitter.NextValSetHash, m.Evidence.Header2.ValidatorSet.Hash()) { - return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("validator set is not valid for height: %d", m.GetHeight())) - } - return nil - } - // Evidence is within trusting period. ValidatorSet must have 2/3 similarity with trustedCommitter ValidatorSet // check that the validator sets on both headers are valid given the last trusted validatorset // less than or equal to evidence height diff --git a/x/ibc/02-client/types/tendermint/misbehaviour_test.go b/x/ibc/02-client/types/tendermint/misbehaviour_test.go index 57ce93eebf1c..3def035aee98 100644 --- a/x/ibc/02-client/types/tendermint/misbehaviour_test.go +++ b/x/ibc/02-client/types/tendermint/misbehaviour_test.go @@ -93,7 +93,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { bothSigners = []tmtypes.PrivValidator{suite.privVal, altPrivVal} } - signers := []tmtypes.PrivValidator{suite.privVal} altSigners := []tmtypes.PrivValidator{altPrivVal} committer := Committer{ @@ -108,46 +107,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { clientID string expErr bool }{ - { - "same height misbehaviour should pass", - &Evidence{ - Header1: MakeHeader("gaia", 3, suite.valSet, altValSet, signers), - Header2: MakeHeader("gaia", 3, suite.valSet, bothValSet, signers), - ChainID: "gaia", - }, - "gaiamainnet", - false, - }, - { - "same height misbehaviour with wrong valSet", - &Evidence{ - Header1: MakeHeader("gaia", 3, bothValSet, altValSet, bothSigners), - Header2: MakeHeader("gaia", 3, bothValSet, bothValSet, bothSigners), - ChainID: "gaia", - }, - "gaiamainnet", - true, - }, - { - "next height misbehaviour should pass", - &Evidence{ - Header1: MakeHeader("gaia", 4, suite.valSet, altValSet, signers), - Header2: MakeHeader("gaia", 4, suite.valSet, bothValSet, signers), - ChainID: "gaia", - }, - "gaiamainnet", - false, - }, - { - "next height misbehaviour with wrong valSet", - &Evidence{ - Header1: MakeHeader("gaia", 4, bothValSet, altValSet, bothSigners), - Header2: MakeHeader("gaia", 4, bothValSet, bothValSet, bothSigners), - ChainID: "gaia", - }, - "gaiamainnet", - true, - }, { "trusting period misbehavior should pass", &Evidence{ From 40cc70fa3233c446ddfe43f259e0ae4e80fce4cb Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 3 Dec 2019 10:38:13 -0800 Subject: [PATCH 23/33] save --- x/ibc/02-client/keeper/client_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/x/ibc/02-client/keeper/client_test.go b/x/ibc/02-client/keeper/client_test.go index 82ca52721eb3..16d61388cabc 100644 --- a/x/ibc/02-client/keeper/client_test.go +++ b/x/ibc/02-client/keeper/client_test.go @@ -115,9 +115,6 @@ func (suite *KeeperTestSuite) TestUpdateClient() { } func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { - _, err := suite.keeper.CreateClient(suite.ctx, "gaiamainnet", exported.Tendermint, suite.consensusState) - suite.NoError(err, "CreateClient failed") - altPrivVal := tmtypes.NewMockPV() altVal := tmtypes.NewValidator(altPrivVal.GetPubKey(), 4) @@ -136,6 +133,12 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { altSigners := []tmtypes.PrivValidator{altPrivVal} + _, err := suite.keeper.CreateClient(suite.ctx, "gaiamainnet", exported.Tendermint, suite.consensusState) + suite.NoError(err, "CreateClient failed") + + err = suite.keeper.UpdateClient(suite.ctx, "gaiamainnet", tendermint.MakeHeader("gaia", 10, bothValSet, bothValSet, bothSigners)) + suite.NoError(err, "UpdateClient failed") + testCases := []struct { name string evidence *tendermint.Evidence From 51ed94ee9852941944b2d9a4ae0bc60d4f0dae16 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 3 Dec 2019 14:14:56 -0800 Subject: [PATCH 24/33] final fixes --- x/ibc/02-client/exported/exported.go | 2 ++ x/ibc/02-client/types/tendermint/evidence.go | 5 +++++ x/ibc/02-client/types/tendermint/evidence_test.go | 1 + x/ibc/02-client/types/tendermint/misbehaviour_test.go | 2 ++ 4 files changed, 10 insertions(+) diff --git a/x/ibc/02-client/exported/exported.go b/x/ibc/02-client/exported/exported.go index d9e97b5dab31..9a5f2bc2e89f 100644 --- a/x/ibc/02-client/exported/exported.go +++ b/x/ibc/02-client/exported/exported.go @@ -40,6 +40,8 @@ type Header interface { // Committer defines the type that is responsible for // updating the consensusState at a given height +// +// In Tendermint, this is the ValidatorSet at the given height type Committer interface { ClientType() ClientType GetHeight() uint64 diff --git a/x/ibc/02-client/types/tendermint/evidence.go b/x/ibc/02-client/types/tendermint/evidence.go index 850c21b4fb1c..ba7637c71e6f 100644 --- a/x/ibc/02-client/types/tendermint/evidence.go +++ b/x/ibc/02-client/types/tendermint/evidence.go @@ -80,6 +80,11 @@ func (ev Evidence) ValidateBasic() error { return nil } +// ValidCommit checks if the given commit is a valid commit from the passed-in validatorset +// +// CommitToVoteSet will panic if the commit cannot be converted to a valid voteset given the validatorset +// This implies that someone tried to submit evidence that wasn't actually committed by the validatorset +// thus we should return an error here and reject the evidence rather than panicing. func ValidCommit(chainID string, commit *tmtypes.Commit, valSet *tmtypes.ValidatorSet) (err error) { defer func() { if r := recover(); r != nil { diff --git a/x/ibc/02-client/types/tendermint/evidence_test.go b/x/ibc/02-client/types/tendermint/evidence_test.go index af3589550031..09f40499b458 100644 --- a/x/ibc/02-client/types/tendermint/evidence_test.go +++ b/x/ibc/02-client/types/tendermint/evidence_test.go @@ -116,6 +116,7 @@ func (suite *TendermintTestSuite) TestEvidenceValidateBasic() { } for _, tc := range testCases { + tc := tc // pin for scopelint suite.Run(tc.name, func() { // reset suite for each subtest suite.SetupTest() diff --git a/x/ibc/02-client/types/tendermint/misbehaviour_test.go b/x/ibc/02-client/types/tendermint/misbehaviour_test.go index 3def035aee98..f69a6d12884e 100644 --- a/x/ibc/02-client/types/tendermint/misbehaviour_test.go +++ b/x/ibc/02-client/types/tendermint/misbehaviour_test.go @@ -59,6 +59,7 @@ func (suite *TendermintTestSuite) TestMisbehaviourValidateBasic() { } for _, tc := range testCases { + tc := tc // pin for scopelint suite.Run(tc.name, func() { misbehaviour := Misbehaviour{ Evidence: tc.evidence, @@ -150,6 +151,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { } for _, tc := range testCases { + tc := tc // pin for scopelint suite.Run(tc.name, func() { misbehaviour := Misbehaviour{ Evidence: tc.evidence, From 213002d01e3a76b1085fe980676586cdfc2ffb18 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 3 Dec 2019 15:57:09 -0800 Subject: [PATCH 25/33] save --- x/ibc/02-client/keeper/client_test.go | 2 ++ x/ibc/02-client/types/tendermint/evidence.go | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/x/ibc/02-client/keeper/client_test.go b/x/ibc/02-client/keeper/client_test.go index 16d61388cabc..2f36d68be00d 100644 --- a/x/ibc/02-client/keeper/client_test.go +++ b/x/ibc/02-client/keeper/client_test.go @@ -79,6 +79,7 @@ func (suite *KeeperTestSuite) TestUpdateClient() { } for _, tc := range cases { + tc := tc // pin for scopelint suite.Run(fmt.Sprintf("Case %s", tc.name), func() { // Reset suite on each subtest suite.SetupTest() @@ -188,6 +189,7 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { } for _, tc := range testCases { + tc := tc // pin for scopelint suite.Run(tc.name, func() { misbehaviour := tendermint.Misbehaviour{ Evidence: tc.evidence, diff --git a/x/ibc/02-client/types/tendermint/evidence.go b/x/ibc/02-client/types/tendermint/evidence.go index ba7637c71e6f..a8d1154a27ae 100644 --- a/x/ibc/02-client/types/tendermint/evidence.go +++ b/x/ibc/02-client/types/tendermint/evidence.go @@ -18,9 +18,9 @@ var _ evidenceexported.Evidence = Evidence{} // Evidence is a wrapper over tendermint's DuplicateVoteEvidence // that implements Evidence interface expected by ICS-02 type Evidence struct { - Header1 Header `json: "header1" yaml: "header1"` - Header2 Header `json: "header2" yaml: "header2"` - ChainID string `json: "chain_id" yaml: "chain_id"` + Header1 Header `json:"header1" yaml:"header1"` + Header2 Header `json:"header2" yaml:"header2"` + ChainID string `json:"chain_id" yaml:"chain_id"` } // Route implements Evidence interface From 6ae0e28e351e6b13bb6d1c5e60ad4fc6f4a04a09 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 3 Dec 2019 16:05:04 -0800 Subject: [PATCH 26/33] fix conflict --- x/ibc/02-client/client/cli/tx.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/ibc/02-client/client/cli/tx.go b/x/ibc/02-client/client/cli/tx.go index 197f7271b011..e677bb57b794 100644 --- a/x/ibc/02-client/client/cli/tx.go +++ b/x/ibc/02-client/client/cli/tx.go @@ -16,6 +16,7 @@ import ( "github.com/cosmos/cosmos-sdk/version" "github.com/cosmos/cosmos-sdk/x/auth" "github.com/cosmos/cosmos-sdk/x/auth/client/utils" + evidence "github.com/cosmos/cosmos-sdk/x/evidence/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types" ) @@ -133,7 +134,7 @@ $ %s tx ibc client misbehaviour [client-id] [path/to/evidence.json] --from node0 clientID := args[0] - var evidence exported.Evidence + var evidence evidence.Evidence if err := cdc.UnmarshalJSON([]byte(args[1]), &evidence); err != nil { fmt.Fprintf(os.Stderr, "failed to unmarshall input into struct, checking for file...") From d151f461aa9d87f2082e47ad36d38a231e12a8e3 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 3 Dec 2019 20:28:53 -0800 Subject: [PATCH 27/33] fix conflicts and add back submit misbehaviour msg --- x/ibc/02-client/client/rest/rest.go | 3 +- x/ibc/02-client/types/msgs.go | 54 +++++++++++++++++++++++++ x/ibc/02-client/types/msgs_test.go | 62 +++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+), 1 deletion(-) diff --git a/x/ibc/02-client/client/rest/rest.go b/x/ibc/02-client/client/rest/rest.go index fb832f63a69a..007c18427952 100644 --- a/x/ibc/02-client/client/rest/rest.go +++ b/x/ibc/02-client/client/rest/rest.go @@ -5,6 +5,7 @@ import ( "github.com/cosmos/cosmos-sdk/client/context" "github.com/cosmos/cosmos-sdk/types/rest" + evidence "github.com/cosmos/cosmos-sdk/x/evidence/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" ) @@ -35,5 +36,5 @@ type UpdateClientReq struct { // SubmitMisbehaviourReq defines the properties of a submit misbehaviour request's body. type SubmitMisbehaviourReq struct { BaseReq rest.BaseReq `json:"base_req" yaml:"base_req"` - Evidence exported.Evidence `json:"evidence" yaml:"evidence"` + Evidence evidence.Evidence `json:"evidence" yaml:"evidence"` } diff --git a/x/ibc/02-client/types/msgs.go b/x/ibc/02-client/types/msgs.go index 6dc615b27ec7..fc326039eb54 100644 --- a/x/ibc/02-client/types/msgs.go +++ b/x/ibc/02-client/types/msgs.go @@ -2,6 +2,7 @@ package types import ( sdk "github.com/cosmos/cosmos-sdk/types" + evidence "github.com/cosmos/cosmos-sdk/x/evidence/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/errors" host "github.com/cosmos/cosmos-sdk/x/ibc/24-host" @@ -123,3 +124,56 @@ func (msg MsgUpdateClient) GetSignBytes() []byte { func (msg MsgUpdateClient) GetSigners() []sdk.AccAddress { return []sdk.AccAddress{msg.Signer} } + +// MsgSubmitMisbehaviour defines a message to update an IBC client +type MsgSubmitMisbehaviour struct { + ClientID string `json:"id" yaml:"id"` + Evidence evidence.Evidence `json:"evidence" yaml:"evidence"` + Signer sdk.AccAddress `json:"address" yaml:"address"` +} + +// NewMsgSubmitMisbehaviour creates a new MsgSubmitMisbehaviour instance +func NewMsgSubmitMisbehaviour(id string, evidence evidence.Evidence, signer sdk.AccAddress) MsgSubmitMisbehaviour { + return MsgSubmitMisbehaviour{ + ClientID: id, + Evidence: evidence, + Signer: signer, + } +} + +// Route implements sdk.Msg +func (msg MsgSubmitMisbehaviour) Route() string { + return ibctypes.RouterKey +} + +// Type implements sdk.Msg +func (msg MsgSubmitMisbehaviour) Type() string { + return "submit_misbehaviour" +} + +// ValidateBasic implements sdk.Msg +func (msg MsgSubmitMisbehaviour) ValidateBasic() sdk.Error { + if err := host.DefaultClientIdentifierValidator(msg.ClientID); err != nil { + return sdk.ConvertError(err) + } + if msg.Evidence == nil { + return sdk.ConvertError(errors.ErrInvalidEvidence(errors.DefaultCodespace, "evidence is nil")) + } + if err := msg.Evidence.ValidateBasic(); err != nil { + return sdk.ConvertError(errors.ErrInvalidEvidence(errors.DefaultCodespace, err.Error())) + } + if msg.Signer.Empty() { + return sdk.ErrInvalidAddress("empty address") + } + return nil +} + +// GetSignBytes implements sdk.Msg +func (msg MsgSubmitMisbehaviour) GetSignBytes() []byte { + return sdk.MustSortJSON(SubModuleCdc.MustMarshalJSON(msg)) +} + +// GetSigners implements sdk.Msg +func (msg MsgSubmitMisbehaviour) GetSigners() []sdk.AccAddress { + return []sdk.AccAddress{msg.Signer} +} diff --git a/x/ibc/02-client/types/msgs_test.go b/x/ibc/02-client/types/msgs_test.go index 72f4407f2319..8e9f59108e03 100644 --- a/x/ibc/02-client/types/msgs_test.go +++ b/x/ibc/02-client/types/msgs_test.go @@ -6,12 +6,41 @@ import ( "github.com/stretchr/testify/require" "github.com/tendermint/tendermint/crypto/secp256k1" + cmn "github.com/tendermint/tendermint/libs/common" sdk "github.com/cosmos/cosmos-sdk/types" + evidence "github.com/cosmos/cosmos-sdk/x/evidence/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" + "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/errors" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/tendermint" ) +var _ evidence.Evidence = mockEvidence{} +var _ evidence.Evidence = mockBadEvidence{} + +const mockStr = "mock" + +// mock GoodEvidence +type mockEvidence struct{} + +// Implement Evidence interface +func (me mockEvidence) Route() string { return mockStr } +func (me mockEvidence) Type() string { return mockStr } +func (me mockEvidence) String() string { return mockStr } +func (me mockEvidence) Hash() cmn.HexBytes { return cmn.HexBytes([]byte(mockStr)) } +func (me mockEvidence) ValidateBasic() error { return nil } +func (me mockEvidence) GetHeight() int64 { return 3 } + +// mock bad evidence +type mockBadEvidence struct { + mockEvidence +} + +// Override ValidateBasic +func (mbe mockBadEvidence) ValidateBasic() error { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, "invalid evidence") +} + func TestMsgCreateClientValidateBasic(t *testing.T) { cs := tendermint.ConsensusState{} privKey := secp256k1.GenPrivKey() @@ -76,3 +105,36 @@ func TestMsgUpdateClient(t *testing.T) { } } } + +func TestMsgSubmitMisbehaviour(t *testing.T) { + privKey := secp256k1.GenPrivKey() + signer := sdk.AccAddress(privKey.PubKey().Address()) + testMsgs := []MsgSubmitMisbehaviour{ + NewMsgSubmitMisbehaviour(exported.ClientTypeTendermint, mockEvidence{}, signer), // valid msg + NewMsgSubmitMisbehaviour("badClient", mockEvidence{}, signer), // bad client id + NewMsgSubmitMisbehaviour(exported.ClientTypeTendermint, nil, signer), // nil evidence + NewMsgSubmitMisbehaviour(exported.ClientTypeTendermint, mockBadEvidence{}, signer), // invalid evidence + NewMsgSubmitMisbehaviour(exported.ClientTypeTendermint, mockEvidence{}, sdk.AccAddress{}), // empty signer + } + + cases := []struct { + msg MsgSubmitMisbehaviour + expPass bool + errMsg string + }{ + {testMsgs[0], true, ""}, + {testMsgs[1], false, "invalid client id passed"}, + {testMsgs[2], false, "Nil Evidence passed"}, + {testMsgs[3], false, "Invalid Evidence passed"}, + {testMsgs[4], false, "Empty address passed"}, + } + + for i, tc := range cases { + err := tc.msg.ValidateBasic() + if tc.expPass { + require.Nil(t, err, "Msg %d failed: %v", i, err) + } else { + require.NotNil(t, err, "Invalid Msg %d passed: %s", i, tc.errMsg) + } + } +} From d1386b91363da2ea6f9f607eb823afdada2e2310 Mon Sep 17 00:00:00 2001 From: Aditya Date: Wed, 4 Dec 2019 10:18:43 -0800 Subject: [PATCH 28/33] Apply suggestions from code review Co-Authored-By: Federico Kunze <31522760+fedekunze@users.noreply.github.com> --- x/ibc/02-client/client/cli/tx.go | 4 ++-- x/ibc/02-client/client/rest/rest.go | 4 ++-- x/ibc/02-client/keeper/client.go | 1 + x/ibc/02-client/keeper/keeper_test.go | 3 +++ x/ibc/02-client/types/msgs.go | 6 +++--- x/ibc/02-client/types/msgs_test.go | 6 +++--- x/ibc/02-client/types/tendermint/misbehaviour.go | 5 +++-- 7 files changed, 17 insertions(+), 12 deletions(-) diff --git a/x/ibc/02-client/client/cli/tx.go b/x/ibc/02-client/client/cli/tx.go index e677bb57b794..9e666f8f926b 100644 --- a/x/ibc/02-client/client/cli/tx.go +++ b/x/ibc/02-client/client/cli/tx.go @@ -16,7 +16,7 @@ import ( "github.com/cosmos/cosmos-sdk/version" "github.com/cosmos/cosmos-sdk/x/auth" "github.com/cosmos/cosmos-sdk/x/auth/client/utils" - evidence "github.com/cosmos/cosmos-sdk/x/evidence/exported" + evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types" ) @@ -134,7 +134,7 @@ $ %s tx ibc client misbehaviour [client-id] [path/to/evidence.json] --from node0 clientID := args[0] - var evidence evidence.Evidence + var evidence evidenceexported.Evidence if err := cdc.UnmarshalJSON([]byte(args[1]), &evidence); err != nil { fmt.Fprintf(os.Stderr, "failed to unmarshall input into struct, checking for file...") diff --git a/x/ibc/02-client/client/rest/rest.go b/x/ibc/02-client/client/rest/rest.go index 007c18427952..51f8ce762bcb 100644 --- a/x/ibc/02-client/client/rest/rest.go +++ b/x/ibc/02-client/client/rest/rest.go @@ -5,7 +5,7 @@ import ( "github.com/cosmos/cosmos-sdk/client/context" "github.com/cosmos/cosmos-sdk/types/rest" - evidence "github.com/cosmos/cosmos-sdk/x/evidence/exported" + evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" ) @@ -36,5 +36,5 @@ type UpdateClientReq struct { // SubmitMisbehaviourReq defines the properties of a submit misbehaviour request's body. type SubmitMisbehaviourReq struct { BaseReq rest.BaseReq `json:"base_req" yaml:"base_req"` - Evidence evidence.Evidence `json:"evidence" yaml:"evidence"` + Evidence evidenceexported.Evidence `json:"evidence" yaml:"evidence"` } diff --git a/x/ibc/02-client/keeper/client.go b/x/ibc/02-client/keeper/client.go index 9b72ef4a2c2c..96ff0bbe06b6 100644 --- a/x/ibc/02-client/keeper/client.go +++ b/x/ibc/02-client/keeper/client.go @@ -95,6 +95,7 @@ func (k Keeper) CheckMisbehaviourAndUpdateState(ctx sdk.Context, evidence eviden if !found { return errors.ErrCommitterNotFound(k.codespace, fmt.Sprintf("committer not found for height %d", misbehaviour.GetHeight())) } + tmCommitter, ok := committer.(tendermint.Committer) if !ok { return errors.ErrInvalidCommitter(k.codespace, "committer type is not Tendermint") diff --git a/x/ibc/02-client/keeper/keeper_test.go b/x/ibc/02-client/keeper/keeper_test.go index 83ad1cc166e7..da79777dae50 100644 --- a/x/ibc/02-client/keeper/keeper_test.go +++ b/x/ibc/02-client/keeper/keeper_test.go @@ -119,6 +119,7 @@ func (suite KeeperTestSuite) TestSetCommitter() { suite.keeper.SetCommitter(suite.ctx, "gaia", 3, committer) suite.keeper.SetCommitter(suite.ctx, "gaia", 6, nextCommitter) + // fetch the commiter on each respective height for i := 0; i < 3; i++ { committer, ok := suite.keeper.GetCommitter(suite.ctx, "gaia", uint64(i)) require.False(suite.T(), ok, "GetCommitter passed on nonexistent height: %d", i) @@ -129,6 +130,7 @@ func (suite KeeperTestSuite) TestSetCommitter() { recv, ok := suite.keeper.GetCommitter(suite.ctx, "gaia", uint64(i)) tmRecv, _ := recv.(tendermint.Committer) if tmRecv.ValidatorSet != nil { + // update validator set's power tmRecv.ValidatorSet.TotalVotingPower() } require.True(suite.T(), ok, "GetCommitter failed on existing height: %d", i) @@ -139,6 +141,7 @@ func (suite KeeperTestSuite) TestSetCommitter() { recv, ok := suite.keeper.GetCommitter(suite.ctx, "gaia", uint64(i)) tmRecv, _ := recv.(tendermint.Committer) if tmRecv.ValidatorSet != nil { + // update validator set's power tmRecv.ValidatorSet.TotalVotingPower() } require.True(suite.T(), ok, "GetCommitter failed on existing height: %d", i) diff --git a/x/ibc/02-client/types/msgs.go b/x/ibc/02-client/types/msgs.go index fc326039eb54..6f0aa7959ac7 100644 --- a/x/ibc/02-client/types/msgs.go +++ b/x/ibc/02-client/types/msgs.go @@ -2,7 +2,7 @@ package types import ( sdk "github.com/cosmos/cosmos-sdk/types" - evidence "github.com/cosmos/cosmos-sdk/x/evidence/exported" + evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/errors" host "github.com/cosmos/cosmos-sdk/x/ibc/24-host" @@ -128,12 +128,12 @@ func (msg MsgUpdateClient) GetSigners() []sdk.AccAddress { // MsgSubmitMisbehaviour defines a message to update an IBC client type MsgSubmitMisbehaviour struct { ClientID string `json:"id" yaml:"id"` - Evidence evidence.Evidence `json:"evidence" yaml:"evidence"` + Evidence evidenceexported.Evidence `json:"evidence" yaml:"evidence"` Signer sdk.AccAddress `json:"address" yaml:"address"` } // NewMsgSubmitMisbehaviour creates a new MsgSubmitMisbehaviour instance -func NewMsgSubmitMisbehaviour(id string, evidence evidence.Evidence, signer sdk.AccAddress) MsgSubmitMisbehaviour { +func NewMsgSubmitMisbehaviour(id string, evidence evidenceexported.Evidence, signer sdk.AccAddress) MsgSubmitMisbehaviour { return MsgSubmitMisbehaviour{ ClientID: id, Evidence: evidence, diff --git a/x/ibc/02-client/types/msgs_test.go b/x/ibc/02-client/types/msgs_test.go index 8e9f59108e03..b29633420ed4 100644 --- a/x/ibc/02-client/types/msgs_test.go +++ b/x/ibc/02-client/types/msgs_test.go @@ -9,14 +9,14 @@ import ( cmn "github.com/tendermint/tendermint/libs/common" sdk "github.com/cosmos/cosmos-sdk/types" - evidence "github.com/cosmos/cosmos-sdk/x/evidence/exported" + evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/errors" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/tendermint" ) -var _ evidence.Evidence = mockEvidence{} -var _ evidence.Evidence = mockBadEvidence{} +var _ evidenceexported.Evidence = mockEvidence{} +var _ evidenceexported.Evidence = mockBadEvidence{} const mockStr = "mock" diff --git a/x/ibc/02-client/types/tendermint/misbehaviour.go b/x/ibc/02-client/types/tendermint/misbehaviour.go index ad11be35bb6d..5b04b6e3ec0c 100644 --- a/x/ibc/02-client/types/tendermint/misbehaviour.go +++ b/x/ibc/02-client/types/tendermint/misbehaviour.go @@ -12,7 +12,8 @@ import ( var _ exported.Misbehaviour = Misbehaviour{} var _ evidenceexported.Evidence = Misbehaviour{} -// Misbehaviour contains an evidence that a +// Misbehaviour contains evidence that a light client submitted a different header from +// a full node at the same height. type Misbehaviour struct { *Evidence ClientID string `json:"client_id" yaml:"client_id"` @@ -42,7 +43,7 @@ func (m Misbehaviour) ValidateBasic() error { return host.DefaultClientIdentifierValidator(m.ClientID) } -// CheckMisbehaviour checks if the evidence provided is a misbehaviour +// CheckMisbehaviour checks if the evidence provided is a valid light client misbehaviour func CheckMisbehaviour(trustedCommitter Committer, m Misbehaviour) error { if err := m.ValidateBasic(); err != nil { return err From 57c22b4201d0822dea095ee399ba966eed4f09c5 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 4 Dec 2019 10:20:16 -0800 Subject: [PATCH 29/33] save --- x/ibc/02-client/client/cli/tx.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/ibc/02-client/client/cli/tx.go b/x/ibc/02-client/client/cli/tx.go index e677bb57b794..9e666f8f926b 100644 --- a/x/ibc/02-client/client/cli/tx.go +++ b/x/ibc/02-client/client/cli/tx.go @@ -16,7 +16,7 @@ import ( "github.com/cosmos/cosmos-sdk/version" "github.com/cosmos/cosmos-sdk/x/auth" "github.com/cosmos/cosmos-sdk/x/auth/client/utils" - evidence "github.com/cosmos/cosmos-sdk/x/evidence/exported" + evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types" ) @@ -134,7 +134,7 @@ $ %s tx ibc client misbehaviour [client-id] [path/to/evidence.json] --from node0 clientID := args[0] - var evidence evidence.Evidence + var evidence evidenceexported.Evidence if err := cdc.UnmarshalJSON([]byte(args[1]), &evidence); err != nil { fmt.Fprintf(os.Stderr, "failed to unmarshall input into struct, checking for file...") From 0202ce4f6b01f52b5d052332fa75f88c2ef4430d Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 4 Dec 2019 10:41:13 -0800 Subject: [PATCH 30/33] add godocs and fix test --- x/ibc/02-client/keeper/client_test.go | 3 --- x/ibc/02-client/types/tendermint/committer.go | 9 ++++++--- x/ibc/02-client/types/tendermint/consensus_state.go | 1 + 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/x/ibc/02-client/keeper/client_test.go b/x/ibc/02-client/keeper/client_test.go index 98e0f306a50d..c99757d3eacb 100644 --- a/x/ibc/02-client/keeper/client_test.go +++ b/x/ibc/02-client/keeper/client_test.go @@ -138,9 +138,6 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { _, err := suite.keeper.CreateClient(suite.ctx, "gaiamainnet", exported.Tendermint, suite.consensusState) suite.NoError(err, "CreateClient failed") - err = suite.keeper.UpdateClient(suite.ctx, "gaiamainnet", tendermint.MakeHeader("gaia", 10, bothValSet, bothValSet, bothSigners)) - suite.NoError(err, "UpdateClient failed") - testCases := []struct { name string evidence *tendermint.Evidence diff --git a/x/ibc/02-client/types/tendermint/committer.go b/x/ibc/02-client/types/tendermint/committer.go index 86615430c0dd..ccf798feb570 100644 --- a/x/ibc/02-client/types/tendermint/committer.go +++ b/x/ibc/02-client/types/tendermint/committer.go @@ -8,16 +8,19 @@ import ( var _ exported.Committer = Committer{} +// Committer definites a Tendermint Committer type Committer struct { - *tmtypes.ValidatorSet - Height uint64 - NextValSetHash []byte + *tmtypes.ValidatorSet `json:"validator_set" yaml:"validator_set"` + Height uint64 `json:"height" yaml:"height"` + NextValSetHash []byte `json:"next_valset_hash" yaml:"next_valset_hash"` } +// Implement exported.Committer interface func (c Committer) ClientType() exported.ClientType { return exported.Tendermint } +// Implement exported.Committer interface func (c Committer) GetHeight() uint64 { return c.Height } diff --git a/x/ibc/02-client/types/tendermint/consensus_state.go b/x/ibc/02-client/types/tendermint/consensus_state.go index 4c81d2cf4437..25c2f8d990f8 100644 --- a/x/ibc/02-client/types/tendermint/consensus_state.go +++ b/x/ibc/02-client/types/tendermint/consensus_state.go @@ -39,6 +39,7 @@ func (cs ConsensusState) GetRoot() commitment.RootI { return cs.Root } +// GetCommitter returns the commmitter that committed the ConsensusState func (cs ConsensusState) GetCommitter() exported.Committer { return Committer{ ValidatorSet: cs.ValidatorSet, From f0d6eac577ac58ebb810159b623b6ecf53e81f1e Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 4 Dec 2019 11:51:38 -0800 Subject: [PATCH 31/33] fix test panics in other modules --- x/ibc/03-connection/keeper/handshake_test.go | 8 +++++--- x/ibc/03-connection/keeper/keeper_test.go | 13 +++++++++--- x/ibc/04-channel/keeper/handshake_test.go | 10 +++++++--- x/ibc/04-channel/keeper/keeper_test.go | 13 +++++++++--- x/ibc/20-transfer/handler_test.go | 21 ++++++++++++++------ x/ibc/20-transfer/keeper/keeper_test.go | 13 +++++++++--- x/ibc/20-transfer/keeper/relay_test.go | 8 +++++--- 7 files changed, 62 insertions(+), 24 deletions(-) diff --git a/x/ibc/03-connection/keeper/handshake_test.go b/x/ibc/03-connection/keeper/handshake_test.go index fede95f47fe9..4e39fbd8de23 100644 --- a/x/ibc/03-connection/keeper/handshake_test.go +++ b/x/ibc/03-connection/keeper/handshake_test.go @@ -267,9 +267,11 @@ func (suite *KeeperTestSuite) createClient(clientID string) { suite.ctx = suite.app.BaseApp.NewContext(false, abci.Header{}) consensusState := tendermint.ConsensusState{ - ChainID: chainID, - Height: uint64(commitID.Version), - Root: commitment.NewRoot(commitID.Hash), + ChainID: chainID, + Height: uint64(commitID.Version), + Root: commitment.NewRoot(commitID.Hash), + ValidatorSet: suite.valSet, + NextValidatorSet: suite.valSet, } _, err := suite.app.IBCKeeper.ClientKeeper.CreateClient(suite.ctx, clientID, clientType, consensusState) diff --git a/x/ibc/03-connection/keeper/keeper_test.go b/x/ibc/03-connection/keeper/keeper_test.go index bf6dfe0d7839..904f02ca16d8 100644 --- a/x/ibc/03-connection/keeper/keeper_test.go +++ b/x/ibc/03-connection/keeper/keeper_test.go @@ -5,6 +5,7 @@ import ( "github.com/stretchr/testify/suite" abci "github.com/tendermint/tendermint/abci/types" + tmtypes "github.com/tendermint/tendermint/types" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/simapp" @@ -29,9 +30,10 @@ const ( type KeeperTestSuite struct { suite.Suite - cdc *codec.Codec - ctx sdk.Context - app *simapp.SimApp + cdc *codec.Codec + ctx sdk.Context + app *simapp.SimApp + valSet *tmtypes.ValidatorSet } func (suite *KeeperTestSuite) SetupTest() { @@ -41,6 +43,11 @@ func (suite *KeeperTestSuite) SetupTest() { suite.cdc = app.Codec() suite.ctx = app.BaseApp.NewContext(isCheckTx, abci.Header{}) suite.app = app + + privVal := tmtypes.NewMockPV() + + validator := tmtypes.NewValidator(privVal.GetPubKey(), 1) + suite.valSet = tmtypes.NewValidatorSet([]*tmtypes.Validator{validator}) } func TestKeeperTestSuite(t *testing.T) { diff --git a/x/ibc/04-channel/keeper/handshake_test.go b/x/ibc/04-channel/keeper/handshake_test.go index efd704c28d21..98a39778e467 100644 --- a/x/ibc/04-channel/keeper/handshake_test.go +++ b/x/ibc/04-channel/keeper/handshake_test.go @@ -22,11 +22,15 @@ func (suite *KeeperTestSuite) createClient() { suite.ctx = suite.app.BaseApp.NewContext(false, abci.Header{}) consensusState := clienttypestm.ConsensusState{ - ChainID: testChainID, - Height: uint64(commitID.Version), - Root: commitment.NewRoot(commitID.Hash), + ChainID: testChainID, + Height: uint64(commitID.Version), + Root: commitment.NewRoot(commitID.Hash), + ValidatorSet: suite.valSet, + NextValidatorSet: suite.valSet, } + _ = consensusState.GetCommitter() + _, err := suite.app.IBCKeeper.ClientKeeper.CreateClient(suite.ctx, testClient, testClientType, consensusState) suite.NoError(err) } diff --git a/x/ibc/04-channel/keeper/keeper_test.go b/x/ibc/04-channel/keeper/keeper_test.go index 299b0ab9c5e5..fa011b3ae104 100644 --- a/x/ibc/04-channel/keeper/keeper_test.go +++ b/x/ibc/04-channel/keeper/keeper_test.go @@ -5,6 +5,7 @@ import ( "github.com/stretchr/testify/suite" abci "github.com/tendermint/tendermint/abci/types" + tmtypes "github.com/tendermint/tendermint/types" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/simapp" @@ -32,9 +33,10 @@ const ( type KeeperTestSuite struct { suite.Suite - cdc *codec.Codec - ctx sdk.Context - app *simapp.SimApp + cdc *codec.Codec + ctx sdk.Context + app *simapp.SimApp + valSet *tmtypes.ValidatorSet } func (suite *KeeperTestSuite) SetupTest() { @@ -45,6 +47,11 @@ func (suite *KeeperTestSuite) SetupTest() { suite.ctx = app.BaseApp.NewContext(isCheckTx, abci.Header{}) suite.app = app + privVal := tmtypes.NewMockPV() + + validator := tmtypes.NewValidator(privVal.GetPubKey(), 1) + suite.valSet = tmtypes.NewValidatorSet([]*tmtypes.Validator{validator}) + suite.createClient() } diff --git a/x/ibc/20-transfer/handler_test.go b/x/ibc/20-transfer/handler_test.go index 7e0f430ddaa9..2b07fbe5151a 100644 --- a/x/ibc/20-transfer/handler_test.go +++ b/x/ibc/20-transfer/handler_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/suite" abci "github.com/tendermint/tendermint/abci/types" + tmtypes "github.com/tendermint/tendermint/types" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/simapp" @@ -50,9 +51,10 @@ var ( type HandlerTestSuite struct { suite.Suite - cdc *codec.Codec - ctx sdk.Context - app *simapp.SimApp + cdc *codec.Codec + ctx sdk.Context + app *simapp.SimApp + valSet *tmtypes.ValidatorSet } func (suite *HandlerTestSuite) SetupTest() { @@ -63,6 +65,11 @@ func (suite *HandlerTestSuite) SetupTest() { suite.ctx = app.BaseApp.NewContext(isCheckTx, abci.Header{}) suite.app = app + privVal := tmtypes.NewMockPV() + + validator := tmtypes.NewValidator(privVal.GetPubKey(), 1) + suite.valSet = tmtypes.NewValidatorSet([]*tmtypes.Validator{validator}) + suite.createClient() suite.createConnection(connection.OPEN) } @@ -75,9 +82,11 @@ func (suite *HandlerTestSuite) createClient() { suite.ctx = suite.app.BaseApp.NewContext(false, abci.Header{}) consensusState := clienttypestm.ConsensusState{ - ChainID: testChainID, - Height: uint64(commitID.Version), - Root: commitment.NewRoot(commitID.Hash), + ChainID: testChainID, + Height: uint64(commitID.Version), + Root: commitment.NewRoot(commitID.Hash), + ValidatorSet: suite.valSet, + NextValidatorSet: suite.valSet, } _, err := suite.app.IBCKeeper.ClientKeeper.CreateClient(suite.ctx, testClient, testClientType, consensusState) diff --git a/x/ibc/20-transfer/keeper/keeper_test.go b/x/ibc/20-transfer/keeper/keeper_test.go index b7370a34d29f..cc67265bb1f8 100644 --- a/x/ibc/20-transfer/keeper/keeper_test.go +++ b/x/ibc/20-transfer/keeper/keeper_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/suite" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto" + tmtypes "github.com/tendermint/tendermint/types" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/simapp" @@ -46,9 +47,10 @@ var ( type KeeperTestSuite struct { suite.Suite - cdc *codec.Codec - ctx sdk.Context - app *simapp.SimApp + cdc *codec.Codec + ctx sdk.Context + app *simapp.SimApp + valSet *tmtypes.ValidatorSet } func (suite *KeeperTestSuite) SetupTest() { @@ -59,6 +61,11 @@ func (suite *KeeperTestSuite) SetupTest() { suite.ctx = app.BaseApp.NewContext(isCheckTx, abci.Header{}) suite.app = app + privVal := tmtypes.NewMockPV() + + validator := tmtypes.NewValidator(privVal.GetPubKey(), 1) + suite.valSet = tmtypes.NewValidatorSet([]*tmtypes.Validator{validator}) + suite.createClient() suite.createConnection(connection.OPEN) } diff --git a/x/ibc/20-transfer/keeper/relay_test.go b/x/ibc/20-transfer/keeper/relay_test.go index fe20d2b26062..c28b4cc9db00 100644 --- a/x/ibc/20-transfer/keeper/relay_test.go +++ b/x/ibc/20-transfer/keeper/relay_test.go @@ -23,9 +23,11 @@ func (suite *KeeperTestSuite) createClient() { suite.ctx = suite.app.BaseApp.NewContext(false, abci.Header{}) consensusState := clienttypestm.ConsensusState{ - ChainID: testChainID, - Height: uint64(commitID.Version), - Root: commitment.NewRoot(commitID.Hash), + ChainID: testChainID, + Height: uint64(commitID.Version), + Root: commitment.NewRoot(commitID.Hash), + ValidatorSet: suite.valSet, + NextValidatorSet: suite.valSet, } _, err := suite.app.IBCKeeper.ClientKeeper.CreateClient(suite.ctx, testClient, testClientType, consensusState) From 9858a8f508908a63a157d3fabdfc9cddaf0f9437 Mon Sep 17 00:00:00 2001 From: Aditya Date: Wed, 4 Dec 2019 11:55:27 -0800 Subject: [PATCH 32/33] Update x/ibc/02-client/keeper/client.go Co-Authored-By: Bot from GolangCI <42910462+golangcibot@users.noreply.github.com> --- x/ibc/02-client/keeper/client.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/ibc/02-client/keeper/client.go b/x/ibc/02-client/keeper/client.go index 96ff0bbe06b6..9b72ef4a2c2c 100644 --- a/x/ibc/02-client/keeper/client.go +++ b/x/ibc/02-client/keeper/client.go @@ -95,7 +95,6 @@ func (k Keeper) CheckMisbehaviourAndUpdateState(ctx sdk.Context, evidence eviden if !found { return errors.ErrCommitterNotFound(k.codespace, fmt.Sprintf("committer not found for height %d", misbehaviour.GetHeight())) } - tmCommitter, ok := committer.(tendermint.Committer) if !ok { return errors.ErrInvalidCommitter(k.codespace, "committer type is not Tendermint") From a5b42e42bf3d33a1f3a589b797e5bc49df52b518 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 4 Dec 2019 12:33:47 -0800 Subject: [PATCH 33/33] add back aliases --- x/ibc/02-client/alias.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x/ibc/02-client/alias.go b/x/ibc/02-client/alias.go index 0cccbbdf3b63..4713a4e77dee 100644 --- a/x/ibc/02-client/alias.go +++ b/x/ibc/02-client/alias.go @@ -61,6 +61,7 @@ var ( KeyRoot = types.KeyRoot NewMsgCreateClient = types.NewMsgCreateClient NewMsgUpdateClient = types.NewMsgUpdateClient + NewMsgSubmitMibehaviour = types.NewMsgSubmitMisbehaviour NewQueryClientStateParams = types.NewQueryClientStateParams NewQueryCommitmentRootParams = types.NewQueryCommitmentRootParams NewClientState = types.NewClientState @@ -76,6 +77,7 @@ type ( Keeper = keeper.Keeper MsgCreateClient = types.MsgCreateClient MsgUpdateClient = types.MsgUpdateClient + MsgSubmitMisbehaviour = types.MsgSubmitMisbehaviour QueryClientStateParams = types.QueryClientStateParams QueryCommitmentRootParams = types.QueryCommitmentRootParams State = types.State