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

Jump to bottom

ICS07 follow up changes #5606

Merged
merged 13 commits into from
Feb 18, 2020
Merged

ICS07 follow up changes #5606

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
104828d
ADR07 follow up changes
fedekunze Feb 3, 2020
6938cb5
add assertion checks
fedekunze Feb 3, 2020
8d68f5e
update ICS02 tests
fedekunze Feb 3, 2020
0c31bec
update ICS07 tests
fedekunze Feb 3, 2020
6db7170
add trusting and ubd period to msg
fedekunze Feb 4, 2020
7f94efe
tests
fedekunze Feb 4, 2020
77b518a
more test updates
fedekunze Feb 4, 2020
bee558d
merge ibc-alpha
fedekunze Feb 17, 2020
b34cf78
Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into fe…
AdityaSripal Feb 17, 2020
535a493
ICS07 follow ups (#5631)
AdityaSripal Feb 18, 2020
51e0491
Merge branch 'ibc-alpha' of https://github.com/cosmos/cosmos-sdk into…
fedekunze Feb 18, 2020
ccef49e
fix conflicts
fedekunze Feb 18, 2020
7d66ff1
changes from master
fedekunze Feb 18, 2020
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
19 changes: 15 additions & 4 deletions x/ibc/02-client/client/cli/tx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"io/ioutil"
"strings"
"time"

"github.com/pkg/errors"

Expand All @@ -27,15 +28,15 @@ import (
// in https://github.com/cosmos/ics/tree/master/spec/ics-002-client-semantics#create
func GetCmdCreateClient(cdc *codec.Codec) *cobra.Command {
cmd := &cobra.Command{
Use: "create [client-id] [path/to/consensus_state.json]",
Use: "create [client-id] [path/to/consensus_state.json] [trusting_period] [unbonding_period]",
Short: "create new client with a consensus state",
Long: strings.TrimSpace(fmt.Sprintf(`create new client with a specified identifier and consensus state:

Example:
$ %s tx ibc client create [client-id] [path/to/consensus_state.json] --from node0 --home ../node0/<app>cli --chain-id $CID
$ %s tx ibc client create [client-id] [path/to/consensus_state.json] [trusting_period] [unbonding_period] --from node0 --home ../node0/<app>cli --chain-id $CID
`, version.ClientName),
),
Args: cobra.ExactArgs(2),
Args: cobra.ExactArgs(4),
RunE: func(cmd *cobra.Command, args []string) error {
inBuf := bufio.NewReader(cmd.InOrStdin())
txBldr := auth.NewTxBuilderFromCLI(inBuf).WithTxEncoder(authclient.GetTxEncoder(cdc))
Expand All @@ -55,9 +56,19 @@ $ %s tx ibc client create [client-id] [path/to/consensus_state.json] --from node
}
}

trustingPeriod, err := time.ParseDuration(args[2])
if err != nil {
return err
}

ubdPeriod, err := time.ParseDuration(args[3])
if err != nil {
return err
}

msg := types.NewMsgCreateClient(
clientID, state.ClientType().String(), state,
cliCtx.GetFromAddress(),
trustingPeriod, ubdPeriod, cliCtx.GetFromAddress(),
)

if err := msg.ValidateBasic(); err != nil {
Expand Down
10 changes: 7 additions & 3 deletions x/ibc/02-client/client/rest/rest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package rest

import (
"time"

"github.com/gorilla/mux"

"github.com/cosmos/cosmos-sdk/client/context"
Expand All @@ -23,9 +25,11 @@ func RegisterRoutes(cliCtx context.CLIContext, r *mux.Router, queryRoute string)

// CreateClientReq defines the properties of a create client request's body.
type CreateClientReq struct {
BaseReq rest.BaseReq `json:"base_req" yaml:"base_req"`
ClientID string `json:"client_id" yaml:"client_id"`
ConsensusState exported.ConsensusState `json:"consensus_state" yaml:"consensus_state"`
BaseReq rest.BaseReq `json:"base_req" yaml:"base_req"`
ClientID string `json:"client_id" yaml:"client_id"`
ConsensusState exported.ConsensusState `json:"consensus_state" yaml:"consensus_state"`
TrustingPeriod time.Duration `json:"trusting_period" yaml:"trusting_period"`
UnbondingPeriod time.Duration `json:"unbonding_period" yaml:"unbonding_period"`
}

// UpdateClientReq defines the properties of a update client request's body.
Expand Down
1 change: 1 addition & 0 deletions x/ibc/02-client/client/rest/tx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ func createClientHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
req.ClientID,
req.ConsensusState.ClientType().String(),
req.ConsensusState,
req.TrustingPeriod, req.UnbondingPeriod,
fromAddr,
)

Expand Down
5 changes: 3 additions & 2 deletions x/ibc/02-client/client/utils/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,9 @@ func QueryNodeConsensusState(cliCtx context.CLIContext) (tendermint.ConsensusSta
}

state := tendermint.ConsensusState{
Root: commitment.NewRoot(commit.AppHash),
ValidatorSetHash: tmtypes.NewValidatorSet(validators.Validators).Hash(),
Timestamp: commit.Time,
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
Root: commitment.NewRoot(commit.AppHash),
ValidatorSet: tmtypes.NewValidatorSet(validators.Validators),
}

return state, height, nil
Expand Down
7 changes: 4 additions & 3 deletions x/ibc/02-client/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ func HandleMsgCreateClient(ctx sdk.Context, k Keeper, msg MsgCreateClient) (*sdk
return nil, sdkerrors.Wrap(ErrInvalidClientType, msg.ClientType)
}

_, err := k.CreateClient(ctx, msg.ClientID, clientType, msg.ConsensusState)
_, err := k.CreateClient(
ctx, msg.ClientID, clientType, msg.ConsensusState, msg.TrustingPeriod, msg.UnbondingPeriod,
)
if err != nil {
return nil, err
}
Expand All @@ -41,8 +43,7 @@ func HandleMsgCreateClient(ctx sdk.Context, k Keeper, msg MsgCreateClient) (*sdk

// HandleMsgUpdateClient defines the sdk.Handler for MsgUpdateClient
func HandleMsgUpdateClient(ctx sdk.Context, k Keeper, msg MsgUpdateClient) (*sdk.Result, error) {
err := k.UpdateClient(ctx, msg.ClientID, msg.Header)
if err != nil {
if err := k.UpdateClient(ctx, msg.ClientID, msg.Header); err != nil {
return nil, err
}

Expand Down
12 changes: 8 additions & 4 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 @@ -2,6 +2,7 @@ package keeper

import (
"fmt"
"time"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
Expand All @@ -15,6 +16,7 @@ import (
func (k Keeper) CreateClient(
ctx sdk.Context, clientID string,
clientType exported.ClientType, consensusState exported.ConsensusState,
trustingPeriod, unbondingPeriod time.Duration,
) (exported.ClientState, error) {
_, found := k.GetClientState(ctx, clientID)
if found {
Expand All @@ -26,7 +28,7 @@ func (k Keeper) CreateClient(
panic(fmt.Sprintf("client type is already defined for client %s", clientID))
}

clientState, err := k.initialize(ctx, clientID, clientType, consensusState)
clientState, err := k.initialize(ctx, clientID, clientType, consensusState, trustingPeriod, unbondingPeriod)
if err != nil {
return nil, sdkerrors.Wrapf(err, "cannot create client with ID %s", clientID)
}
Expand Down Expand Up @@ -78,9 +80,11 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.H

switch clientType {
case exported.Tendermint:
clientState, consensusState, err = tendermint.CheckValidityAndUpdateState(clientState, header, ctx.ChainID())
clientState, consensusState, err = tendermint.CheckValidityAndUpdateState(
clientState, header, ctx.ChainID(), ctx.BlockTime(),
)
default:
return sdkerrors.Wrapf(types.ErrInvalidClientType, "cannot update client with ID %s", clientID)
err = types.ErrInvalidClientType
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
}

if err != nil {
Expand Down Expand Up @@ -122,7 +126,7 @@ func (k Keeper) CheckMisbehaviourAndUpdateState(ctx sdk.Context, misbehaviour ex
switch e := misbehaviour.(type) {
case tendermint.Evidence:
clientState, err = tendermint.CheckMisbehaviourAndUpdateState(
clientState, consensusState, misbehaviour, uint64(misbehaviour.GetHeight()),
clientState, consensusState, misbehaviour, uint64(misbehaviour.GetHeight()), ctx.BlockTime(),
)

default:
Expand Down
45 changes: 22 additions & 23 deletions x/ibc/02-client/keeper/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ func (suite *KeeperTestSuite) TestCreateClient() {

if tc.expPanic {
suite.Require().Panics(func() {
suite.keeper.CreateClient(suite.ctx, tc.params.clientID, tc.params.clientType, suite.consensusState)
suite.keeper.CreateClient(suite.ctx, tc.params.clientID, tc.params.clientType, suite.consensusState, trustingPeriod, ubdPeriod)
}, "Msg %d didn't panic: %s", i, tc.msg)
} else {
clientState, err := suite.keeper.CreateClient(suite.ctx, tc.params.clientID, tc.params.clientType, suite.consensusState)
clientState, err := suite.keeper.CreateClient(suite.ctx, tc.params.clientID, tc.params.clientType, suite.consensusState, trustingPeriod, ubdPeriod)

if tc.expPass {
suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.msg)
Expand All @@ -63,7 +63,7 @@ func (suite *KeeperTestSuite) TestUpdateClient() {
expPass bool
}{
{"valid update", func() error {
_, err := suite.keeper.CreateClient(suite.ctx, testClientID, exported.Tendermint, suite.consensusState)
_, err := suite.keeper.CreateClient(suite.ctx, testClientID, exported.Tendermint, suite.consensusState, trustingPeriod, ubdPeriod)
return err
}, true},
{"client type not found", func() error {
Expand All @@ -84,7 +84,7 @@ func (suite *KeeperTestSuite) TestUpdateClient() {
return nil
}, false},
{"invalid header", func() error {
_, err := suite.keeper.CreateClient(suite.ctx, testClientID, exported.Tendermint, suite.consensusState)
_, err := suite.keeper.CreateClient(suite.ctx, testClientID, exported.Tendermint, suite.consensusState, trustingPeriod, ubdPeriod)
if err != nil {
return err
}
Expand All @@ -105,8 +105,9 @@ func (suite *KeeperTestSuite) TestUpdateClient() {

if tc.expPass {
expConsensusState := tendermint.ConsensusState{
Root: commitment.NewRoot(suite.header.AppHash),
ValidatorSetHash: suite.header.ValidatorSet.Hash(),
Timestamp: suite.header.Time,
Root: commitment.NewRoot(suite.header.AppHash),
ValidatorSet: suite.header.ValidatorSet,
}

clientState, found := suite.keeper.GetClientState(suite.ctx, testClientID)
Expand Down Expand Up @@ -153,15 +154,14 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {
{
"trusting period misbehavior should pass",
tendermint.Evidence{
Header1: tendermint.CreateTestHeader(testClientID, testClientHeight, bothValSet, suite.valSet, bothSigners),
Header2: tendermint.CreateTestHeader(testClientID, testClientHeight, bothValSet, bothValSet, bothSigners),
FromValidatorSet: bothValSet,
ChainID: testClientID,
ClientID: testClientID,
Header1: tendermint.CreateTestHeader(testClientID, testClientHeight, suite.ctx.BlockTime(), bothValSet, suite.valSet, bothSigners),
Header2: tendermint.CreateTestHeader(testClientID, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
ChainID: testClientID,
ClientID: testClientID,
},
func() error {
suite.consensusState.ValidatorSetHash = bothValSet.Hash()
_, err := suite.keeper.CreateClient(suite.ctx, testClientID, exported.Tendermint, suite.consensusState)
suite.consensusState.ValidatorSet = bothValSet
_, err := suite.keeper.CreateClient(suite.ctx, testClientID, exported.Tendermint, suite.consensusState, trustingPeriod, ubdPeriod)
return err
},
true,
Expand All @@ -175,8 +175,8 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {
{
"consensus state not found",
tendermint.Evidence{
Header1: tendermint.CreateTestHeader(testClientID, testClientHeight, bothValSet, suite.valSet, bothSigners),
Header2: tendermint.CreateTestHeader(testClientID, testClientHeight, bothValSet, bothValSet, bothSigners),
Header1: tendermint.CreateTestHeader(testClientID, testClientHeight, suite.ctx.BlockTime(), bothValSet, suite.valSet, bothSigners),
Header2: tendermint.CreateTestHeader(testClientID, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
ChainID: testClientID,
ClientID: testClientID,
},
Expand All @@ -190,8 +190,8 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {
{
"consensus state not found",
tendermint.Evidence{
Header1: tendermint.CreateTestHeader(testClientID, testClientHeight, bothValSet, suite.valSet, bothSigners),
Header2: tendermint.CreateTestHeader(testClientID, testClientHeight, bothValSet, bothValSet, bothSigners),
Header1: tendermint.CreateTestHeader(testClientID, testClientHeight, suite.ctx.BlockTime(), bothValSet, suite.valSet, bothSigners),
Header2: tendermint.CreateTestHeader(testClientID, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
ChainID: testClientID,
ClientID: testClientID,
},
Expand All @@ -205,14 +205,13 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {
{
"misbehaviour check failed",
tendermint.Evidence{
Header1: tendermint.CreateTestHeader(testClientID, testClientHeight, bothValSet, bothValSet, bothSigners),
Header2: tendermint.CreateTestHeader(testClientID, testClientHeight, altValSet, bothValSet, altSigners),
FromValidatorSet: bothValSet,
ChainID: testClientID,
ClientID: testClientID,
Header1: tendermint.CreateTestHeader(testClientID, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
Header2: tendermint.CreateTestHeader(testClientID, testClientHeight, suite.ctx.BlockTime(), altValSet, bothValSet, altSigners),
ChainID: testClientID,
ClientID: testClientID,
},
func() error {
_, err := suite.keeper.CreateClient(suite.ctx, testClientID, exported.Tendermint, suite.consensusState)
_, err := suite.keeper.CreateClient(suite.ctx, testClientID, exported.Tendermint, suite.consensusState, trustingPeriod, ubdPeriod)
return err
},
false,
Expand Down
23 changes: 17 additions & 6 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 @@ -2,6 +2,7 @@ package keeper

import (
"fmt"
"time"

"github.com/tendermint/tendermint/libs/log"
tmtypes "github.com/tendermint/tendermint/types"
Expand Down Expand Up @@ -104,8 +105,9 @@ func (k Keeper) GetSelfConsensusState(ctx sdk.Context, height uint64) (exported.
}

consensusState := tendermint.ConsensusState{
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
Root: commitment.NewRoot(histInfo.Header.AppHash),
ValidatorSetHash: tmtypes.NewValidatorSet(histInfo.ValSet.ToTmValidators()).Hash(),
Timestamp: ctx.BlockTime(),
Root: commitment.NewRoot(histInfo.Header.AppHash),
ValidatorSet: tmtypes.NewValidatorSet(histInfo.ValSet.ToTmValidators()),
}
return consensusState, true
}
Expand Down Expand Up @@ -141,16 +143,25 @@ func (k Keeper) GetAllClients(ctx sdk.Context) (states []exported.ClientState) {
// https://github.com/cosmos/ics/tree/master/spec/ics-002-client-semantics#example-implementation
func (k Keeper) initialize(
ctx sdk.Context, clientID string, clientType exported.ClientType,
consensusState exported.ConsensusState,
consensusState exported.ConsensusState, trustingPeriod, unbondingPeriod time.Duration,
) (exported.ClientState, error) {
var clientState exported.ClientState
height := uint64(ctx.BlockHeight())
Copy link
Member

Choose a reason for hiding this comment

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

Should not be using our own chain's BlockHeight here. Should be using the other chain's height.

Fixed this on my branch by adding Height field and GetHeight method to ConsensusState


var (
clientState exported.ClientState
err error
)
switch clientType {
case exported.Tendermint:
clientState = tendermint.NewClientState(clientID, height)
clientState, err = tendermint.Initialize(
clientID, consensusState, trustingPeriod, unbondingPeriod, height,
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
)
default:
return nil, types.ErrInvalidClientType
err = types.ErrInvalidClientType
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
}

if err != nil {
return nil, err
}

k.SetClientConsensusState(ctx, clientID, height, consensusState)
Expand Down
Loading