Skip to content

Commit

Permalink
Make clients use proto Height (#7184)
Browse files Browse the repository at this point in the history 
* start using Height in TM client

* fix client tests

* fix client tests

* fix connection tests

* fix rest of tests

* Apply suggestions from code review

Co-authored-by: colin axnér <[email protected]>

* fix simple issues

* add and use LTE and GTE methods

* remove TM-specific height semantics from 02-client

* fix lint and build

Co-authored-by: Federico Kunze <[email protected]>
Co-authored-by: colin axnér <[email protected]>
  • Loading branch information
@AdityaSripal @fedekunze @colin-axner
3 people authored Aug 31, 2020
1 parent fc75d14 commit d208d2b
Show file tree
Hide file tree
Showing 46 changed files with 1,613 additions and 1,054 deletions.
17 changes: 17 additions & 0 deletions proto/ibc/client/client.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,20 @@ message ClientConsensusStates {
// consensus states associated with the client
repeated google.protobuf.Any consensus_states = 2 [(gogoproto.moretags) = "yaml:\"consensus_states\""];
}

// Height is a monotonically increasing data type
// that can be compared against another Height for the purposes of updating and freezing clients
//
// Normally the EpochHeight is incremented at each height while keeping epoch number the same
// However some consensus algorithms may choose to reset the height in certain conditions
// e.g. hard forks, state-machine breaking changes
// In these cases, the epoch number is incremented so that height continues to be monitonically increasing
// even as the EpochHeight gets reset
message Height {
option (gogoproto.goproto_stringer) = false;

// the epoch that the client is currently on
uint64 epoch_number = 1 [(gogoproto.moretags) = "yaml:\"epoch_number\""];
// the height within the given epoch
uint64 epoch_height = 2 [(gogoproto.moretags) = "yaml:\"epoch_height\""];
}
5 changes: 4 additions & 1 deletion proto/ibc/localhost/localhost.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ syntax = "proto3";
package ibc.localhost;

import "gogoproto/gogo.proto";
import "ibc/client/client.proto";

option go_package = "github.com/cosmos/cosmos-sdk/x/ibc/09-localhost/types";

Expand All @@ -24,5 +25,7 @@ message ClientState {
(gogoproto.moretags) = "yaml:\"chain_id\""
];
// self latest block height
uint64 height = 3;
ibc.client.Height height = 3 [
(gogoproto.nullable) = false
];
}
20 changes: 16 additions & 4 deletions proto/ibc/tendermint/tendermint.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import "tendermint/types/types.proto";
import "confio/proofs.proto";
import "google/protobuf/duration.proto";
import "google/protobuf/timestamp.proto";
import "ibc/client/client.proto";
import "ibc/commitment/commitment.proto";
import "gogoproto/gogo.proto";

Expand Down Expand Up @@ -41,9 +42,15 @@ message ClientState {
(gogoproto.moretags) = "yaml:\"max_clock_drift\""
];
// Block height when the client was frozen due to a misbehaviour
uint64 frozen_height = 6 [(gogoproto.moretags) = "yaml:\"frozen_height\""];
ibc.client.Height frozen_height = 6 [
(gogoproto.nullable) = false,
(gogoproto.moretags) = "yaml:\"frozen_height\""
];
// Latest height the client was updated to
uint64 latest_height = 7 [(gogoproto.moretags) = "yaml:\"latest_height\""];
ibc.client.Height latest_height = 7 [
(gogoproto.nullable) = false,
(gogoproto.moretags) = "yaml:\"latest_height\""
];
// Proof specifications used in verifying counterparty state
repeated ics23.ProofSpec proof_specs = 8
[(gogoproto.moretags) = "yaml:\"proof_specs\""];
Expand All @@ -60,7 +67,9 @@ message ConsensusState {
// commitment root (i.e app hash)
ibc.commitment.MerkleRoot root = 2 [(gogoproto.nullable) = false];
// height at which the consensus state was stored.
uint64 height = 3;
ibc.client.Height height = 3 [
(gogoproto.nullable) = false
];
bytes next_validators_hash = 4 [
(gogoproto.casttype) =
"github.com/tendermint/tendermint/libs/bytes.HexBytes",
Expand Down Expand Up @@ -106,7 +115,10 @@ message Header {

.tendermint.types.ValidatorSet validator_set = 2
[(gogoproto.moretags) = "yaml:\"validator_set\""];
uint64 trusted_height = 3 [(gogoproto.moretags) = "yaml:\"trusted_height\""];
ibc.client.Height trusted_height = 3 [
(gogoproto.nullable) = false,
(gogoproto.moretags) = "yaml:\"trusted_height\""
];
.tendermint.types.ValidatorSet trusted_validators = 4
[(gogoproto.moretags) = "yaml:\"trusted_validators\""];
}
Expand Down
11 changes: 11 additions & 0 deletions x/ibc/02-client/exported/exported.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,17 @@ type MsgSubmitMisbehaviour interface {
GetMisbehaviour() Misbehaviour
}

// Height is a wrapper interface over clienttypes.Height
// all clients must use the concrete implementation in types
type Height interface {
IsZero() bool
LT(Height) bool
LTE(Height) bool
EQ(Height) bool
GT(Height) bool
GTE(Height) bool
}

// ClientType defines the type of the consensus algorithm
type ClientType byte

Expand Down
6 changes: 5 additions & 1 deletion x/ibc/02-client/genesis.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,11 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, gs types.GenesisState) {
}

// client id is always "localhost"
clientState := localhosttypes.NewClientState(ctx.ChainID(), ctx.BlockHeight())
// Hardcode 0 as epoch number for now
// TODO: Retrieve epoch from chain-id
clientState := localhosttypes.NewClientState(
ctx.ChainID(), types.NewHeight(0, uint64(ctx.BlockHeight())),
)

_, err := k.CreateClient(ctx, exported.ClientTypeLocalHost, clientState, nil)
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion x/ibc/02-client/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ func HandleMsgCreateClient(ctx sdk.Context, k keeper.Keeper, msg exported.MsgCre
// localhost is a special case that must initialize client state
// from context and not from msg
case *localhosttypes.MsgCreateClient:
clientState = localhosttypes.NewClientState(ctx.ChainID(), ctx.BlockHeight())
selfHeight := types.NewHeight(0, uint64(ctx.BlockHeight()))
clientState = localhosttypes.NewClientState(ctx.ChainID(), selfHeight)
// Localhost consensus height is chain's blockheight
consensusHeight = uint64(ctx.BlockHeight())
default:
Expand Down
68 changes: 35 additions & 33 deletions x/ibc/02-client/keeper/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
tmtypes "github.com/tendermint/tendermint/types"

"github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported"
"github.com/cosmos/cosmos-sdk/x/ibc/02-client/types"
ibctmtypes "github.com/cosmos/cosmos-sdk/x/ibc/07-tendermint/types"
localhosttypes "github.com/cosmos/cosmos-sdk/x/ibc/09-localhost/types"
commitmenttypes "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment/types"
Expand Down Expand Up @@ -80,14 +81,15 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() {
_, err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState)

// store intermediate consensus state to check that trustedHeight does not need to be highest consensus state before header height
incrementedClientHeight := testClientHeight.Increment()
intermediateConsState := &ibctmtypes.ConsensusState{
Height: testClientHeight + 1,
Height: incrementedClientHeight,
Timestamp: suite.now.Add(time.Minute),
NextValidatorsHash: suite.valSetHash,
}
suite.keeper.SetClientConsensusState(suite.ctx, testClientID, testClientHeight+1, intermediateConsState)
suite.keeper.SetClientConsensusState(suite.ctx, testClientID, incrementedClientHeight.EpochHeight, intermediateConsState)

clientState.LatestHeight = testClientHeight + 1
clientState.LatestHeight = incrementedClientHeight
suite.keeper.SetClientState(suite.ctx, testClientID, clientState)

updateHeader = createFutureUpdateFn(suite)
Expand All @@ -100,15 +102,15 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() {

// store previous consensus state
prevConsState := &ibctmtypes.ConsensusState{
Height: 1,
Height: types.NewHeight(0, 1),
Timestamp: suite.past,
NextValidatorsHash: suite.valSetHash,
}
suite.keeper.SetClientConsensusState(suite.ctx, testClientID, 1, prevConsState)

// store intermediate consensus state to check that trustedHeight does not need to be hightest consensus state before header height
intermediateConsState := &ibctmtypes.ConsensusState{
Height: 2,
Height: types.NewHeight(0, 2),
Timestamp: suite.past.Add(time.Minute),
NextValidatorsHash: suite.valSetHash,
}
Expand Down Expand Up @@ -145,7 +147,7 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() {
return nil
}, false},
{"frozen client before update", func() error {
clientState = &ibctmtypes.ClientState{FrozenHeight: 1, LatestHeight: testClientHeight}
clientState = &ibctmtypes.ClientState{FrozenHeight: types.NewHeight(0, 1), LatestHeight: testClientHeight}
suite.keeper.SetClientState(suite.ctx, testClientID, clientState)
suite.keeper.SetClientType(suite.ctx, testClientID, exported.Tendermint)
updateHeader = createFutureUpdateFn(suite)
Expand All @@ -154,13 +156,13 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() {
}, false},
{"valid past update before client was frozen", func() error {
clientState = ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs())
clientState.FrozenHeight = testClientHeight - 1
clientState.FrozenHeight = types.NewHeight(0, testClientHeight.EpochHeight-1)
_, err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState)
suite.Require().NoError(err)

// store previous consensus state
prevConsState := &ibctmtypes.ConsensusState{
Height: 1,
Height: types.NewHeight(0, 1),
Timestamp: suite.past,
NextValidatorsHash: suite.valSetHash,
}
Expand Down Expand Up @@ -198,7 +200,7 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() {
suite.Require().NoError(err, err)

expConsensusState := &ibctmtypes.ConsensusState{
Height: updateHeader.GetHeight(),
Height: types.NewHeight(0, updateHeader.GetHeight()),
Timestamp: updateHeader.GetTime(),
Root: commitmenttypes.NewMerkleRoot(updateHeader.Header.GetAppHash()),
NextValidatorsHash: updateHeader.Header.NextValidatorsHash,
Expand Down Expand Up @@ -231,7 +233,7 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() {
}

func (suite *KeeperTestSuite) TestUpdateClientLocalhost() {
var localhostClient exported.ClientState = localhosttypes.NewClientState(suite.header.Header.GetChainID(), suite.ctx.BlockHeight())
var localhostClient exported.ClientState = localhosttypes.NewClientState(suite.header.Header.GetChainID(), types.NewHeight(0, uint64(suite.ctx.BlockHeight())))

suite.ctx = suite.ctx.WithBlockHeight(suite.ctx.BlockHeight() + 1)

Expand Down Expand Up @@ -274,8 +276,8 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {
{
"trusting period misbehavior should pass",
&ibctmtypes.Misbehaviour{
Header1: ibctmtypes.CreateTestHeader(testChainID, testClientHeight, testClientHeight, altTime, bothValSet, bothValSet, bothSigners),
Header2: ibctmtypes.CreateTestHeader(testChainID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
Header1: ibctmtypes.CreateTestHeader(testChainID, height, height, altTime, bothValSet, bothValSet, bothSigners),
Header2: ibctmtypes.CreateTestHeader(testChainID, height, height, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
ChainId: testChainID,
ClientId: testClientID,
},
Expand All @@ -291,8 +293,8 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {
{
"misbehavior at later height should pass",
&ibctmtypes.Misbehaviour{
Header1: ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight, altTime, bothValSet, valSet, bothSigners),
Header2: ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight, suite.ctx.BlockTime(), bothValSet, valSet, bothSigners),
Header1: ibctmtypes.CreateTestHeader(testChainID, height+5, height, altTime, bothValSet, valSet, bothSigners),
Header2: ibctmtypes.CreateTestHeader(testChainID, height+5, height, suite.ctx.BlockTime(), bothValSet, valSet, bothSigners),
ChainId: testChainID,
ClientId: testClientID,
},
Expand All @@ -303,13 +305,13 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {

// store intermediate consensus state to check that trustedHeight does not need to be highest consensus state before header height
intermediateConsState := &ibctmtypes.ConsensusState{
Height: testClientHeight + 3,
Height: types.NewHeight(0, height+3),
Timestamp: suite.now.Add(time.Minute),
NextValidatorsHash: suite.valSetHash,
}
suite.keeper.SetClientConsensusState(suite.ctx, testClientID, testClientHeight+3, intermediateConsState)
suite.keeper.SetClientConsensusState(suite.ctx, testClientID, height+3, intermediateConsState)

clientState.LatestHeight = testClientHeight + 3
clientState.LatestHeight = types.NewHeight(0, height+3)
suite.keeper.SetClientState(suite.ctx, testClientID, clientState)

return err
Expand All @@ -319,8 +321,8 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {
{
"misbehavior at later height with different trusted heights should pass",
&ibctmtypes.Misbehaviour{
Header1: ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight, altTime, bothValSet, valSet, bothSigners),
Header2: ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight+3, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
Header1: ibctmtypes.CreateTestHeader(testChainID, height+5, height, altTime, bothValSet, valSet, bothSigners),
Header2: ibctmtypes.CreateTestHeader(testChainID, height+5, height+3, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
ChainId: testChainID,
ClientId: testClientID,
},
Expand All @@ -331,13 +333,13 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {

// store trusted consensus state for Header2
intermediateConsState := &ibctmtypes.ConsensusState{
Height: testClientHeight + 3,
Height: types.NewHeight(0, height+3),
Timestamp: suite.now.Add(time.Minute),
NextValidatorsHash: bothValsHash,
}
suite.keeper.SetClientConsensusState(suite.ctx, testClientID, testClientHeight+3, intermediateConsState)
suite.keeper.SetClientConsensusState(suite.ctx, testClientID, height+3, intermediateConsState)

clientState.LatestHeight = testClientHeight + 3
clientState.LatestHeight = types.NewHeight(0, height+3)
suite.keeper.SetClientState(suite.ctx, testClientID, clientState)

return err
Expand All @@ -347,8 +349,8 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {
{
"misbehaviour fails validatebasic",
&ibctmtypes.Misbehaviour{
Header1: ibctmtypes.CreateTestHeader(testChainID, testClientHeight+1, testClientHeight, altTime, bothValSet, bothValSet, bothSigners),
Header2: ibctmtypes.CreateTestHeader(testChainID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
Header1: ibctmtypes.CreateTestHeader(testChainID, height+1, height, altTime, bothValSet, bothValSet, bothSigners),
Header2: ibctmtypes.CreateTestHeader(testChainID, height, height, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
ChainId: testChainID,
ClientId: testClientID,
},
Expand All @@ -364,8 +366,8 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {
{
"trusted ConsensusState1 not found",
&ibctmtypes.Misbehaviour{
Header1: ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight+3, altTime, bothValSet, bothValSet, bothSigners),
Header2: ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight, suite.ctx.BlockTime(), bothValSet, valSet, bothSigners),
Header1: ibctmtypes.CreateTestHeader(testChainID, height+5, height+3, altTime, bothValSet, bothValSet, bothSigners),
Header2: ibctmtypes.CreateTestHeader(testChainID, height+5, height, suite.ctx.BlockTime(), bothValSet, valSet, bothSigners),
ChainId: testChainID,
ClientId: testClientID,
},
Expand All @@ -381,8 +383,8 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {
{
"trusted ConsensusState2 not found",
&ibctmtypes.Misbehaviour{
Header1: ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight, altTime, bothValSet, valSet, bothSigners),
Header2: ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight+3, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
Header1: ibctmtypes.CreateTestHeader(testChainID, height+5, height, altTime, bothValSet, valSet, bothSigners),
Header2: ibctmtypes.CreateTestHeader(testChainID, height+5, height+3, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
ChainId: testChainID,
ClientId: testClientID,
},
Expand All @@ -405,8 +407,8 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {
{
"client already frozen at earlier height",
&ibctmtypes.Misbehaviour{
Header1: ibctmtypes.CreateTestHeader(testChainID, testClientHeight, testClientHeight, altTime, bothValSet, bothValSet, bothSigners),
Header2: ibctmtypes.CreateTestHeader(testChainID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
Header1: ibctmtypes.CreateTestHeader(testChainID, height, height, altTime, bothValSet, bothValSet, bothSigners),
Header2: ibctmtypes.CreateTestHeader(testChainID, height, height, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
ChainId: testChainID,
ClientId: testClientID,
},
Expand All @@ -415,7 +417,7 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {
clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs())
_, err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState)

clientState.FrozenHeight = 1
clientState.FrozenHeight = types.NewHeight(0, 1)
suite.keeper.SetClientState(suite.ctx, testClientID, clientState)

return err
Expand All @@ -426,8 +428,8 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {
{
"misbehaviour check failed",
&ibctmtypes.Misbehaviour{
Header1: ibctmtypes.CreateTestHeader(testChainID, testClientHeight, testClientHeight, altTime, bothValSet, bothValSet, bothSigners),
Header2: ibctmtypes.CreateTestHeader(testChainID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), altValSet, bothValSet, altSigners),
Header1: ibctmtypes.CreateTestHeader(testChainID, height, height, altTime, bothValSet, bothValSet, bothSigners),
Header2: ibctmtypes.CreateTestHeader(testChainID, height, height, suite.ctx.BlockTime(), altValSet, bothValSet, altSigners),
ChainId: testChainID,
ClientId: testClientID,
},
Expand Down
Loading

0 comments on commit d208d2b

Please sign in to comment.