Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate Evidence Implementation into ICS-02 #5258

Merged
merged 5 commits into from
Nov 4, 2019
Merged

Integrate Evidence Implementation into ICS-02 #5258

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8dc7ee6
implement evidence in ics-02
AdityaSripal Oct 28, 2019
4553172
fix build errors and import cycles
AdityaSripal Oct 29, 2019
13b2dcc
address fede comments
AdityaSripal Oct 30, 2019
9e5e0ec
remove unnecessary pubkey and fix init
AdityaSripal Oct 30, 2019
3340173
add tests
AdityaSripal Oct 31, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 22 additions & 21 deletions x/ibc/02-client/alias.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,21 @@ package client
import (
"github.com/cosmos/cosmos-sdk/x/ibc/02-client/keeper"
"github.com/cosmos/cosmos-sdk/x/ibc/02-client/types"
"github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/errors"
)

const (
DefaultCodespace = types.DefaultCodespace
CodeClientExists = types.CodeClientExists
CodeClientNotFound = types.CodeClientNotFound
CodeClientFrozen = types.CodeClientFrozen
CodeConsensusStateNotFound = types.CodeConsensusStateNotFound
CodeInvalidConsensusState = types.CodeInvalidConsensusState
CodeClientTypeNotFound = types.CodeClientTypeNotFound
CodeInvalidClientType = types.CodeInvalidClientType
CodeRootNotFound = types.CodeRootNotFound
CodeInvalidHeader = types.CodeInvalidHeader
CodeInvalidEvidence = types.CodeInvalidEvidence
DefaultCodespace = errors.DefaultCodespace
CodeClientExists = errors.CodeClientExists
CodeClientNotFound = errors.CodeClientNotFound
CodeClientFrozen = errors.CodeClientFrozen
CodeConsensusStateNotFound = errors.CodeConsensusStateNotFound
CodeInvalidConsensusState = errors.CodeInvalidConsensusState
CodeClientTypeNotFound = errors.CodeClientTypeNotFound
CodeInvalidClientType = errors.CodeInvalidClientType
CodeRootNotFound = errors.CodeRootNotFound
CodeInvalidHeader = errors.CodeInvalidHeader
CodeInvalidEvidence = errors.CodeInvalidEvidence
AttributeKeyClientID = types.AttributeKeyClientID
SubModuleName = types.SubModuleName
StoreKey = types.StoreKey
Expand All @@ -40,16 +41,16 @@ var (
QuerierConsensusState = keeper.QuerierConsensusState
QuerierVerifiedRoot = keeper.QuerierVerifiedRoot
RegisterCodec = types.RegisterCodec
ErrClientExists = types.ErrClientExists
ErrClientNotFound = types.ErrClientNotFound
ErrClientFrozen = types.ErrClientFrozen
ErrConsensusStateNotFound = types.ErrConsensusStateNotFound
ErrInvalidConsensus = types.ErrInvalidConsensus
ErrClientTypeNotFound = types.ErrClientTypeNotFound
ErrInvalidClientType = types.ErrInvalidClientType
ErrRootNotFound = types.ErrRootNotFound
ErrInvalidHeader = types.ErrInvalidHeader
ErrInvalidEvidence = types.ErrInvalidEvidence
ErrClientExists = errors.ErrClientExists
ErrClientNotFound = errors.ErrClientNotFound
ErrClientFrozen = errors.ErrClientFrozen
ErrConsensusStateNotFound = errors.ErrConsensusStateNotFound
ErrInvalidConsensus = errors.ErrInvalidConsensus
ErrClientTypeNotFound = errors.ErrClientTypeNotFound
ErrInvalidClientType = errors.ErrInvalidClientType
ErrRootNotFound = errors.ErrRootNotFound
ErrInvalidHeader = errors.ErrInvalidHeader
ErrInvalidEvidence = errors.ErrInvalidEvidence
ClientStatePath = types.ClientStatePath
ClientTypePath = types.ClientTypePath
ConsensusStatePath = types.ConsensusStatePath
Expand Down
15 changes: 8 additions & 7 deletions x/ibc/02-client/keeper/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"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"
)

// CreateClient creates a new client state and populates it with a given consensus
Expand All @@ -17,7 +18,7 @@ func (k Keeper) CreateClient(
) (types.State, error) {
_, found := k.GetClientState(ctx, clientID)
if found {
return types.State{}, types.ErrClientExists(k.codespace, clientID)
return types.State{}, errors.ErrClientExists(k.codespace, clientID)
}

_, found = k.GetClientType(ctx, clientID)
Expand All @@ -36,26 +37,26 @@ func (k Keeper) CreateClient(
func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.Header) error {
clientType, found := k.GetClientType(ctx, clientID)
if !found {
return sdkerrors.Wrap(types.ErrClientTypeNotFound(k.codespace), "cannot update client")
return sdkerrors.Wrap(errors.ErrClientTypeNotFound(k.codespace), "cannot update client")
}

// check that the header consensus matches the client one
if header.ClientType() != clientType {
return sdkerrors.Wrap(types.ErrInvalidConsensus(k.codespace), "cannot update client")
return sdkerrors.Wrap(errors.ErrInvalidConsensus(k.codespace), "cannot update client")
}

clientState, found := k.GetClientState(ctx, clientID)
if !found {
return sdkerrors.Wrap(types.ErrClientNotFound(k.codespace, clientID), "cannot update client")
return sdkerrors.Wrap(errors.ErrClientNotFound(k.codespace, clientID), "cannot update client")
}

if clientState.Frozen {
return sdkerrors.Wrap(types.ErrClientFrozen(k.codespace, clientID), "cannot update client")
return sdkerrors.Wrap(errors.ErrClientFrozen(k.codespace, clientID), "cannot update client")
}

consensusState, found := k.GetConsensusState(ctx, clientID)
if !found {
return sdkerrors.Wrap(types.ErrConsensusStateNotFound(k.codespace), "cannot update client")
return sdkerrors.Wrap(errors.ErrConsensusStateNotFound(k.codespace), "cannot update client")
}

if header.GetHeight() < consensusState.GetHeight() {
Expand Down Expand Up @@ -83,7 +84,7 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.H
func (k Keeper) CheckMisbehaviourAndUpdateState(ctx sdk.Context, clientID string, evidence exported.Evidence) error {
clientState, found := k.GetClientState(ctx, clientID)
if !found {
sdk.ResultFromError(types.ErrClientNotFound(k.codespace, clientID))
sdk.ResultFromError(errors.ErrClientNotFound(k.codespace, clientID))
}

err := k.checkMisbehaviour(ctx, evidence)
Expand Down
35 changes: 20 additions & 15 deletions x/ibc/02-client/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"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"
)
Expand All @@ -29,8 +31,8 @@ func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, codespace sdk.CodespaceType)
return Keeper{
storeKey: key,
cdc: cdc,
codespace: sdk.CodespaceType(fmt.Sprintf("%s/%s", codespace, types.DefaultCodespace)), // "ibc/client",
prefix: []byte(types.SubModuleName + "/"), // "client/"
codespace: sdk.CodespaceType(fmt.Sprintf("%s/%s", codespace, errors.DefaultCodespace)), // "ibc/client",
prefix: []byte(types.SubModuleName + "/"), // "client/"
}
}

Expand Down Expand Up @@ -127,25 +129,28 @@ func (k Keeper) initialize(ctx sdk.Context, clientID string, consensusState expo
}

func (k Keeper) checkMisbehaviour(ctx sdk.Context, evidence exported.Evidence) error {
// switch evidence.H1().ClientType() {
// case exported.Tendermint:
// var tmEvidence tendermint.Evidence
// _, ok := evidence.(tendermint.Evidence)
// if !ok {
// return sdkerrors.Wrap(types.ErrInvalidClientType(k.codespace), "consensus type is not Tendermint")
// }
// // TODO: pass past consensus states
// return tendermint.CheckMisbehaviour(tmEvidence)
// default:
// panic("unregistered consensus type")
// }
switch evidence.Type() {
case "tendermint":
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
var tmEvidence tendermint.Evidence
_, ok := evidence.(tendermint.Evidence)
if !ok {
return errors.ErrInvalidClientType(k.codespace, "consensus type is not Tendermint")
}
// TODO: pass past consensus states
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to double-check, but I don't think this TODO is necessary anymore

err := tendermint.CheckMisbehaviour(tmEvidence)
if err != nil {
return errors.ErrInvalidEvidence(k.codespace, err.Error())
}
default:
panic("unregistered evidence type")
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
}
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 {
return types.State{}, sdkerrors.Wrap(types.ErrClientFrozen(k.codespace, clientState.ID()), "already frozen")
return types.State{}, sdkerrors.Wrap(errors.ErrClientFrozen(k.codespace, clientState.ID()), "already frozen")
}

clientState.Frozen = true
Expand Down
7 changes: 4 additions & 3 deletions x/ibc/02-client/keeper/querier.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/ibc/02-client/types"
"github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/errors"
)

// QuerierClientState defines the sdk.Querier to query the IBC client state
Expand All @@ -20,7 +21,7 @@ func QuerierClientState(ctx sdk.Context, req abci.RequestQuery, k Keeper) ([]byt

clientState, found := k.GetClientState(ctx, params.ClientID)
if !found {
return nil, types.ErrClientTypeNotFound(k.codespace)
return nil, errors.ErrClientTypeNotFound(k.codespace)
}

bz, err := types.SubModuleCdc.MarshalJSON(clientState)
Expand All @@ -42,7 +43,7 @@ func QuerierConsensusState(ctx sdk.Context, req abci.RequestQuery, k Keeper) ([]

consensusState, found := k.GetConsensusState(ctx, params.ClientID)
if !found {
return nil, types.ErrConsensusStateNotFound(k.codespace)
return nil, errors.ErrConsensusStateNotFound(k.codespace)
}

bz, err := types.SubModuleCdc.MarshalJSON(consensusState)
Expand All @@ -64,7 +65,7 @@ func QuerierVerifiedRoot(ctx sdk.Context, req abci.RequestQuery, k Keeper) ([]by

root, found := k.GetVerifiedRoot(ctx, params.ClientID, params.Height)
if !found {
return nil, types.ErrRootNotFound(k.codespace)
return nil, errors.ErrRootNotFound(k.codespace)
}

bz, err := types.SubModuleCdc.MarshalJSON(root)
Expand Down
8 changes: 4 additions & 4 deletions x/ibc/02-client/types/errors.go → x/ibc/02-client/types/errors/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package types
package errors
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not against this change, but it breaks the standard module structure.

I like how @alexanderbez implemented the errors on the evidence module here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If i leave errors.go in types/, there wasn't a way for me to use those errors in the types/tendermint package without creating an import cycle. This is why i split it into its own package

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the codebase is a mix of using the old errors (sdk.Error defined in types), and the new errors defined in types/errors. The current ICS-02 implementation uses the old sdk.Error, should I switch it to use new error type?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current ICS-02 implementation uses the old sdk.Error, should I switch it to use new error type?

Thoughts @alexanderbez ?

Copy link
Contributor

@alexanderbez alexanderbez Oct 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the approach I've taken in the evidence module PR and for future PRs is the following:

Return error where ever and as much as possible up and until I can no longer return an error. When I can no longer return an error, I call sdk.ConvertError(err) which gives me the old (soon to be deprecated) sdk.Error.

Behind the scenes I'm returning a concrete type of sdkerrors.New(...) which fulfills the error interface and fulfills the ABCI semantics.

so tl;dr, return error using sdkerrors.New(...) as much as possible until you cant.


import (
"fmt"
Expand All @@ -8,7 +8,7 @@ import (

// client error codes
const (
DefaultCodespace sdk.CodespaceType = SubModuleName
DefaultCodespace sdk.CodespaceType = "client"

CodeClientExists sdk.CodeType = 101
CodeClientNotFound sdk.CodeType = 102
Expand Down Expand Up @@ -68,6 +68,6 @@ func ErrInvalidHeader(codespace sdk.CodespaceType) sdk.Error {
}

// ErrInvalidEvidence implements sdk.Error
func ErrInvalidEvidence(codespace sdk.CodespaceType) sdk.Error {
return sdk.NewError(codespace, CodeInvalidEvidence, "invalid evidence")
func ErrInvalidEvidence(codespace sdk.CodespaceType, msg string) sdk.Error {
return sdk.NewError(codespace, CodeInvalidEvidence, "invalid evidence: %s", msg)
}
9 changes: 5 additions & 4 deletions x/ibc/02-client/types/msgs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

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"
host "github.com/cosmos/cosmos-sdk/x/ibc/24-host"
ibctypes "github.com/cosmos/cosmos-sdk/x/ibc/types"
)
Expand Down Expand Up @@ -45,10 +46,10 @@ func (msg MsgCreateClient) ValidateBasic() sdk.Error {
return sdk.NewError(host.IBCCodeSpace, 1, fmt.Sprintf("invalid client ID: %s", err.Error()))
}
if _, err := exported.ClientTypeFromString(msg.ClientType); err != nil {
return ErrInvalidClientType(DefaultCodespace, err.Error())
return errors.ErrInvalidClientType(errors.DefaultCodespace, err.Error())
}
if msg.ConsensusState == nil {
return ErrInvalidConsensus(DefaultCodespace)
return errors.ErrInvalidConsensus(errors.DefaultCodespace)
}
if msg.Signer.Empty() {
return sdk.ErrInvalidAddress("empty address")
Expand Down Expand Up @@ -100,7 +101,7 @@ func (msg MsgUpdateClient) ValidateBasic() sdk.Error {
return sdk.NewError(host.IBCCodeSpace, 1, fmt.Sprintf("invalid client ID: %s", err.Error()))
}
if msg.Header == nil {
return ErrInvalidHeader(DefaultCodespace)
return errors.ErrInvalidHeader(errors.DefaultCodespace)
}
if msg.Signer.Empty() {
return sdk.ErrInvalidAddress("empty address")
Expand Down Expand Up @@ -150,7 +151,7 @@ func (msg MsgSubmitMisbehaviour) ValidateBasic() sdk.Error {
return sdk.NewError(host.IBCCodeSpace, 1, fmt.Sprintf("invalid client ID: %s", err.Error()))
}
if msg.Evidence == nil {
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
return ErrInvalidEvidence(DefaultCodespace)
return errors.ErrInvalidEvidence(errors.DefaultCodespace, "evidence is nil")
}
if msg.Signer.Empty() {
return sdk.ErrInvalidAddress("empty address")
Expand Down
63 changes: 60 additions & 3 deletions x/ibc/02-client/types/tendermint/misbehaviour.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,67 @@ package tendermint
import (
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"
tmtypes "github.com/tendermint/tendermint/types"
)

// Evidence is a wrapper over tendermint's DuplicateVoteEvidence
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
// that implements Evidence interface expected by ICS-02
type Evidence struct {
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
tmtypes.DuplicateVoteEvidence
ChainID string
ValPubKey crypto.PubKey
ValidatorPower int64
TotalPower int64
}

// Type implements exported.Evidence interface
func (ev Evidence) Route() string {
return exported.ClientTypeTendermint
}

// Type implements exported.Evidence interface
func (ev Evidence) Type() string {
return exported.ClientTypeTendermint
}

// String implements exported.Evidence interface
func (ev Evidence) String() string {
return ev.DuplicateVoteEvidence.String()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can add the other fields here as well. We can implement the String function by using marshal yaml to bytes and then print them. We'd need to write a test for the expected string tho.

}

// ValidateBasic implements exported.Evidence interface
func (ev Evidence) ValidateBasic() sdk.Error {
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
err := ev.DuplicateVoteEvidence.ValidateBasic()
if err != nil {
return nil
}
return errors.ErrInvalidEvidence(errors.DefaultCodespace, err.Error())
}

// 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
}

// CheckMisbehaviour checks if the evidence provided is a misbehaviour
func CheckMisbehaviour(evidence exported.Evidence) sdk.Error {
// TODO: check evidence
return nil
func CheckMisbehaviour(evidence Evidence) error {
return evidence.DuplicateVoteEvidence.Verify(evidence.ChainID, evidence.ValPubKey)
}
3 changes: 3 additions & 0 deletions x/ibc/24-host/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,7 @@ var (

// ErrInvalidPath is returned if path string is invalid
ErrInvalidPath = sdkerrors.Register(IBCCodeSpace, 2, "invalid path")

// ErrInvalidEvidence is returned if evidence is invalid
ErrInvalidEvidence = sdkerrors.Register(IBCCodeSpace, 3, "invalid evidence")
)