diff --git a/proto/ibc/core/client/v1/genesis.proto b/proto/ibc/core/client/v1/genesis.proto index b1c4247c94a..06b4bbd0648 100644 --- a/proto/ibc/core/client/v1/genesis.proto +++ b/proto/ibc/core/client/v1/genesis.proto @@ -20,4 +20,6 @@ message GenesisState { Params params = 3 [(gogoproto.nullable) = false]; // create localhost on initialization bool create_localhost = 4 [(gogoproto.moretags) = "yaml:\"create_localhost\""]; + // the sequence for the next generated client identifier + uint64 next_client_sequence = 5 [(gogoproto.moretags) = "yaml:\"next_client_sequence\""]; } diff --git a/proto/ibc/core/client/v1/tx.proto b/proto/ibc/core/client/v1/tx.proto index 1019e15a034..a30ec8bbf10 100644 --- a/proto/ibc/core/client/v1/tx.proto +++ b/proto/ibc/core/client/v1/tx.proto @@ -27,15 +27,13 @@ message MsgCreateClient { option (gogoproto.equal) = false; option (gogoproto.goproto_getters) = false; - // client unique identifier - string client_id = 1 [(gogoproto.moretags) = "yaml:\"client_id\""]; // light client state - google.protobuf.Any client_state = 2 [(gogoproto.moretags) = "yaml:\"client_state\""]; + google.protobuf.Any client_state = 1 [(gogoproto.moretags) = "yaml:\"client_state\""]; // consensus state associated with the client that corresponds to a given // height. - google.protobuf.Any consensus_state = 3 [(gogoproto.moretags) = "yaml:\"consensus_state\""]; + google.protobuf.Any consensus_state = 2 [(gogoproto.moretags) = "yaml:\"consensus_state\""]; // signer address - string signer = 4; + string signer = 3; } // MsgCreateClientResponse defines the Msg/CreateClient response type. diff --git a/types/query/query.pb.go b/types/query/query.pb.go index c04a6bde32b..266d337002e 100644 --- a/types/query/query.pb.go +++ b/types/query/query.pb.go @@ -783,7 +783,7 @@ type Module struct { Path string `protobuf:"bytes,1,opt,name=path,proto3" json:"path,omitempty"` // module version Version string `protobuf:"bytes,2,opt,name=version,proto3" json:"version,omitempty"` - //checksum + // checksum Sum string `protobuf:"bytes,3,opt,name=sum,proto3" json:"sum,omitempty"` } diff --git a/x/auth/client/rest/rest_test.go b/x/auth/client/rest/rest_test.go index 1d7cac6848b..671df87a8b9 100644 --- a/x/auth/client/rest/rest_test.go +++ b/x/auth/client/rest/rest_test.go @@ -453,7 +453,6 @@ func (s *IntegrationTestSuite) TestLegacyRestErrMessages() { "Successful IBC message", ibcsolomachinecli.NewCreateClientCmd(), []string{ - "21212121212", // dummy client-id "1", // dummy sequence consensusJSON.Name(), // path to consensus json, fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), diff --git a/x/ibc/core/02-client/genesis.go b/x/ibc/core/02-client/genesis.go index ef00930f0b8..6e77b20e36f 100644 --- a/x/ibc/core/02-client/genesis.go +++ b/x/ibc/core/02-client/genesis.go @@ -7,7 +7,6 @@ import ( "github.com/cosmos/cosmos-sdk/x/ibc/core/02-client/keeper" "github.com/cosmos/cosmos-sdk/x/ibc/core/02-client/types" "github.com/cosmos/cosmos-sdk/x/ibc/core/exported" - localhosttypes "github.com/cosmos/cosmos-sdk/x/ibc/light-clients/09-localhost/types" ) // InitGenesis initializes the ibc client submodule's state from a provided genesis @@ -39,29 +38,10 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, gs types.GenesisState) { } } - if !gs.CreateLocalhost { - return - } - - // NOTE: return if the localhost client was already imported. The chain-id and - // block height will be overwriten to the correct values during BeginBlock. - if _, found := k.GetClientState(ctx, exported.Localhost); found { - return - } - - // client id is always "localhost" - revision := types.ParseChainID(ctx.ChainID()) - clientState := localhosttypes.NewClientState( - ctx.ChainID(), types.NewHeight(revision, uint64(ctx.BlockHeight())), - ) + k.SetNextClientSequence(ctx, gs.NextClientSequence) - if err := clientState.Validate(); err != nil { - panic(err) - } - - if err := k.CreateClient(ctx, exported.Localhost, clientState, nil); err != nil { - panic(err) - } + // NOTE: localhost creation is specifically disallowed for the time being. + // Issue: https://github.com/cosmos/cosmos-sdk/issues/7871 } // ExportGenesis returns the ibc client submodule's exported genesis. diff --git a/x/ibc/core/02-client/keeper/client.go b/x/ibc/core/02-client/keeper/client.go index ddb742563c1..49d5a04eb36 100644 --- a/x/ibc/core/02-client/keeper/client.go +++ b/x/ibc/core/02-client/keeper/client.go @@ -15,20 +15,17 @@ import ( // // CONTRACT: ClientState was constructed correctly from given initial consensusState func (k Keeper) CreateClient( - ctx sdk.Context, clientID string, clientState exported.ClientState, consensusState exported.ConsensusState, -) error { + ctx sdk.Context, clientState exported.ClientState, consensusState exported.ConsensusState, +) (string, error) { params := k.GetParams(ctx) if !params.IsAllowedClient(clientState.ClientType()) { - return sdkerrors.Wrapf( + return "", sdkerrors.Wrapf( types.ErrInvalidClientType, "client state type %s is not registered in the allowlist", clientState.ClientType(), ) } - _, found := k.GetClientState(ctx, clientID) - if found { - return sdkerrors.Wrapf(types.ErrClientExists, "cannot create client with ID %s", clientID) - } + clientID := k.GenerateClientIdentifier(ctx, clientState.ClientType()) // check if consensus state is nil in case the created client is Localhost if consensusState != nil { @@ -46,7 +43,7 @@ func (k Keeper) CreateClient( ) }() - return nil + return clientID, nil } // UpdateClient updates the consensus state and the state root from a provided header. diff --git a/x/ibc/core/02-client/keeper/client_test.go b/x/ibc/core/02-client/keeper/client_test.go index 736a34d1e48..3d772761cc5 100644 --- a/x/ibc/core/02-client/keeper/client_test.go +++ b/x/ibc/core/02-client/keeper/client_test.go @@ -19,36 +19,23 @@ import ( func (suite *KeeperTestSuite) TestCreateClient() { cases := []struct { - msg string - clientID string - expPass bool - expPanic bool + msg string + clientState exported.ClientState + expPass bool }{ - {"success", testClientID, true, false}, - {"client ID exists", testClientID, false, false}, + {"success", ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), true}, + {"client type not supported", localhosttypes.NewClientState(testChainID, clienttypes.NewHeight(0, 1)), false}, } for i, tc := range cases { - tc := tc - i := i - if tc.expPanic { - suite.Require().Panics(func() { - clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false) - suite.keeper.CreateClient(suite.ctx, tc.clientID, clientState, suite.consensusState) - }, "Msg %d didn't panic: %s", i, tc.msg) - } else { - clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false) - if tc.expPass { - suite.Require().NotNil(clientState, "valid test case %d failed: %s", i, tc.msg) - } - // If we were able to NewClientState clientstate successfully, try persisting it to state - err := suite.keeper.CreateClient(suite.ctx, tc.clientID, clientState, suite.consensusState) - if tc.expPass { - suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.msg) - } else { - suite.Require().Error(err, "invalid test case %d passed: %s", i, tc.msg) - } + clientID, err := suite.keeper.CreateClient(suite.ctx, tc.clientState, suite.consensusState) + if tc.expPass { + suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.msg) + suite.Require().NotNil(clientID, "valid test case %d failed: %s", i, tc.msg) + } else { + suite.Require().Error(err, "invalid test case %d passed: %s", i, tc.msg) + suite.Require().Equal("", clientID, "invalid test case %d passed: %s", i, tc.msg) } } } @@ -72,6 +59,8 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() { var ( updateHeader *ibctmtypes.Header clientState *ibctmtypes.ClientState + clientID string + err error ) cases := []struct { @@ -81,7 +70,7 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() { }{ {"valid update", func() error { clientState = ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false) - err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState) + clientID, err = suite.keeper.CreateClient(suite.ctx, 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() @@ -89,17 +78,17 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() { Timestamp: suite.now.Add(time.Minute), NextValidatorsHash: suite.valSetHash, } - suite.keeper.SetClientConsensusState(suite.ctx, testClientID, incrementedClientHeight, intermediateConsState) + suite.keeper.SetClientConsensusState(suite.ctx, clientID, incrementedClientHeight, intermediateConsState) clientState.LatestHeight = incrementedClientHeight - suite.keeper.SetClientState(suite.ctx, testClientID, clientState) + suite.keeper.SetClientState(suite.ctx, clientID, clientState) updateHeader = createFutureUpdateFn(suite) return err }, true}, {"valid past update", func() error { clientState = ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false) - err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState) + clientID, err = suite.keeper.CreateClient(suite.ctx, clientState, suite.consensusState) suite.Require().NoError(err) height1 := types.NewHeight(0, 1) @@ -109,7 +98,7 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() { Timestamp: suite.past, NextValidatorsHash: suite.valSetHash, } - suite.keeper.SetClientConsensusState(suite.ctx, testClientID, height1, prevConsState) + suite.keeper.SetClientConsensusState(suite.ctx, clientID, height1, prevConsState) height2 := types.NewHeight(0, 2) @@ -118,7 +107,7 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() { Timestamp: suite.past.Add(time.Minute), NextValidatorsHash: suite.valSetHash, } - suite.keeper.SetClientConsensusState(suite.ctx, testClientID, height2, intermediateConsState) + suite.keeper.SetClientConsensusState(suite.ctx, clientID, height2, intermediateConsState) // updateHeader will fill in consensus state between prevConsState and suite.consState // clientState should not be updated @@ -147,7 +136,7 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() { {"valid past update before client was frozen", func() error { clientState = ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false) clientState.FrozenHeight = types.NewHeight(0, testClientHeight.RevisionHeight-1) - err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState) + clientID, err = suite.keeper.CreateClient(suite.ctx, clientState, suite.consensusState) suite.Require().NoError(err) height1 := types.NewHeight(0, 1) @@ -157,7 +146,7 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() { Timestamp: suite.past, NextValidatorsHash: suite.valSetHash, } - suite.keeper.SetClientConsensusState(suite.ctx, testClientID, height1, prevConsState) + suite.keeper.SetClientConsensusState(suite.ctx, clientID, height1, prevConsState) // updateHeader will fill in consensus state between prevConsState and suite.consState // clientState should not be updated @@ -166,7 +155,7 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() { }, true}, {"invalid header", func() error { clientState = ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false) - err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState) + _, err := suite.keeper.CreateClient(suite.ctx, clientState, suite.consensusState) suite.Require().NoError(err) updateHeader = createPastUpdateFn(suite) @@ -179,13 +168,14 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() { i := i suite.Run(fmt.Sprintf("Case %s", tc.name), func() { suite.SetupTest() + clientID = testClientID // must be explicitly changed err := tc.malleate() suite.Require().NoError(err) suite.ctx = suite.ctx.WithBlockTime(updateHeader.Header.Time.Add(time.Minute)) - err = suite.keeper.UpdateClient(suite.ctx, testClientID, updateHeader) + err = suite.keeper.UpdateClient(suite.ctx, clientID, updateHeader) if tc.expPass { suite.Require().NoError(err, err) @@ -196,10 +186,10 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() { NextValidatorsHash: updateHeader.Header.NextValidatorsHash, } - newClientState, found := suite.keeper.GetClientState(suite.ctx, testClientID) + newClientState, found := suite.keeper.GetClientState(suite.ctx, clientID) suite.Require().True(found, "valid test case %d failed: %s", i, tc.name) - consensusState, found := suite.keeper.GetClientConsensusState(suite.ctx, testClientID, updateHeader.GetHeight()) + consensusState, found := suite.keeper.GetClientConsensusState(suite.ctx, clientID, updateHeader.GetHeight()) suite.Require().True(found, "valid test case %d failed: %s", i, tc.name) // Determine if clientState should be updated or not @@ -399,6 +389,11 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { } func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { + var ( + clientID string + err error + ) + altPrivVal := ibctestingmock.NewPV() altPubKey, err := altPrivVal.GetPubKey() suite.Require().NoError(err) @@ -437,12 +432,12 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { &ibctmtypes.Misbehaviour{ Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight), testClientHeight, altTime, bothValSet, bothValSet, bothSigners), Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight), testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), - ClientId: testClientID, + ClientId: clientID, }, func() error { suite.consensusState.NextValidatorsHash = bothValsHash clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false) - err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState) + clientID, err = suite.keeper.CreateClient(suite.ctx, clientState, suite.consensusState) return err }, @@ -453,22 +448,22 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { &ibctmtypes.Misbehaviour{ Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight), testClientHeight, altTime, bothValSet, valSet, bothSigners), Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight), testClientHeight, suite.ctx.BlockTime(), bothValSet, valSet, bothSigners), - ClientId: testClientID, + ClientId: clientID, }, func() error { suite.consensusState.NextValidatorsHash = valsHash clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false) - err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState) + clientID, err = suite.keeper.CreateClient(suite.ctx, clientState, suite.consensusState) // store intermediate consensus state to check that trustedHeight does not need to be highest consensus state before header height intermediateConsState := &ibctmtypes.ConsensusState{ Timestamp: suite.now.Add(time.Minute), NextValidatorsHash: suite.valSetHash, } - suite.keeper.SetClientConsensusState(suite.ctx, testClientID, heightPlus3, intermediateConsState) + suite.keeper.SetClientConsensusState(suite.ctx, clientID, heightPlus3, intermediateConsState) clientState.LatestHeight = heightPlus3 - suite.keeper.SetClientState(suite.ctx, testClientID, clientState) + suite.keeper.SetClientState(suite.ctx, clientID, clientState) return err }, @@ -479,22 +474,22 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { &ibctmtypes.Misbehaviour{ Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight), testClientHeight, altTime, bothValSet, valSet, bothSigners), Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight), heightPlus3, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), - ClientId: testClientID, + ClientId: clientID, }, func() error { suite.consensusState.NextValidatorsHash = valsHash clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false) - err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState) + clientID, err = suite.keeper.CreateClient(suite.ctx, clientState, suite.consensusState) // store trusted consensus state for Header2 intermediateConsState := &ibctmtypes.ConsensusState{ Timestamp: suite.now.Add(time.Minute), NextValidatorsHash: bothValsHash, } - suite.keeper.SetClientConsensusState(suite.ctx, testClientID, heightPlus3, intermediateConsState) + suite.keeper.SetClientConsensusState(suite.ctx, clientID, heightPlus3, intermediateConsState) clientState.LatestHeight = heightPlus3 - suite.keeper.SetClientState(suite.ctx, testClientID, clientState) + suite.keeper.SetClientState(suite.ctx, clientID, clientState) return err }, @@ -505,12 +500,12 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { &ibctmtypes.Misbehaviour{ Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight), heightPlus3, altTime, bothValSet, bothValSet, bothSigners), Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight), testClientHeight, suite.ctx.BlockTime(), bothValSet, valSet, bothSigners), - ClientId: testClientID, + ClientId: clientID, }, func() error { suite.consensusState.NextValidatorsHash = valsHash clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false) - err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState) + clientID, err = suite.keeper.CreateClient(suite.ctx, clientState, suite.consensusState) // intermediate consensus state at height + 3 is not created return err }, @@ -521,12 +516,12 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { &ibctmtypes.Misbehaviour{ Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight), testClientHeight, altTime, bothValSet, valSet, bothSigners), Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight), heightPlus3, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), - ClientId: testClientID, + ClientId: clientID, }, func() error { suite.consensusState.NextValidatorsHash = valsHash clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false) - err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState) + clientID, err = suite.keeper.CreateClient(suite.ctx, clientState, suite.consensusState) // intermediate consensus state at height + 3 is not created return err }, @@ -543,15 +538,15 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { &ibctmtypes.Misbehaviour{ Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight), testClientHeight, altTime, bothValSet, bothValSet, bothSigners), Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight), testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), - ClientId: testClientID, + ClientId: clientID, }, func() error { suite.consensusState.NextValidatorsHash = bothValsHash clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false) - err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState) + clientID, err = suite.keeper.CreateClient(suite.ctx, clientState, suite.consensusState) clientState.FrozenHeight = types.NewHeight(0, 1) - suite.keeper.SetClientState(suite.ctx, testClientID, clientState) + suite.keeper.SetClientState(suite.ctx, clientID, clientState) return err }, @@ -562,14 +557,14 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { &ibctmtypes.Misbehaviour{ Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight), testClientHeight, altTime, bothValSet, bothValSet, bothSigners), Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight), testClientHeight, suite.ctx.BlockTime(), altValSet, bothValSet, altSigners), - ClientId: testClientID, + ClientId: clientID, }, func() error { clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false) if err != nil { return err } - err = suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState) + clientID, err = suite.keeper.CreateClient(suite.ctx, clientState, suite.consensusState) return err }, @@ -580,18 +575,22 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { for i, tc := range testCases { tc := tc i := i + suite.Run(tc.name, func() { - suite.SetupTest() // reset + suite.SetupTest() // reset + clientID = testClientID // must be explicitly changed err := tc.malleate() suite.Require().NoError(err) + tc.misbehaviour.ClientId = clientID + err = suite.keeper.CheckMisbehaviourAndUpdateState(suite.ctx, tc.misbehaviour) if tc.expPass { suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.name) - clientState, found := suite.keeper.GetClientState(suite.ctx, testClientID) + clientState, found := suite.keeper.GetClientState(suite.ctx, clientID) suite.Require().True(found, "valid test case %d failed: %s", i, tc.name) suite.Require().True(clientState.IsFrozen(), "valid test case %d failed: %s", i, tc.name) suite.Require().Equal(tc.misbehaviour.GetHeight(), clientState.GetFrozenHeight(), diff --git a/x/ibc/core/02-client/keeper/keeper.go b/x/ibc/core/02-client/keeper/keeper.go index accf48bb14c..18cb4afd68d 100644 --- a/x/ibc/core/02-client/keeper/keeper.go +++ b/x/ibc/core/02-client/keeper/keeper.go @@ -50,6 +50,16 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger { return ctx.Logger().With("module", fmt.Sprintf("x/%s/%s", host.ModuleName, types.SubModuleName)) } +// GenerateClientIdentifier returns the next client identifier. +func (k Keeper) GenerateClientIdentifier(ctx sdk.Context, clientType string) string { + nextClientSeq := k.GetNextClientSequence(ctx) + clientID := types.FormatClientIdentifier(clientType, nextClientSeq) + + nextClientSeq++ + k.SetNextClientSequence(ctx, nextClientSeq) + return clientID +} + // GetClientState gets a particular client from the store func (k Keeper) GetClientState(ctx sdk.Context, clientID string) (exported.ClientState, bool) { store := k.ClientStore(ctx, clientID) @@ -87,6 +97,24 @@ func (k Keeper) SetClientConsensusState(ctx sdk.Context, clientID string, height store.Set(host.ConsensusStateKey(height), k.MustMarshalConsensusState(consensusState)) } +// GetNextClientSequence gets the next client sequence from the store. +func (k Keeper) GetNextClientSequence(ctx sdk.Context) uint64 { + store := ctx.KVStore(k.storeKey) + bz := store.Get([]byte(types.KeyNextClientSequence)) + if bz == nil { + panic("next client sequence is nil") + } + + return sdk.BigEndianToUint64(bz) +} + +// SetNextClientSequence sets the next client sequence to the store. +func (k Keeper) SetNextClientSequence(ctx sdk.Context, sequence uint64) { + store := ctx.KVStore(k.storeKey) + bz := sdk.Uint64ToBigEndian(sequence) + store.Set([]byte(types.KeyNextClientSequence), bz) +} + // IterateConsensusStates provides an iterator over all stored consensus states. // objects. For each State object, cb will be called. If the cb returns true, // the iterator will close and stop. diff --git a/x/ibc/core/02-client/keeper/keeper_test.go b/x/ibc/core/02-client/keeper/keeper_test.go index 45ff4f674e3..ec3c0229ca5 100644 --- a/x/ibc/core/02-client/keeper/keeper_test.go +++ b/x/ibc/core/02-client/keeper/keeper_test.go @@ -30,9 +30,9 @@ const ( testChainID = "gaiahub-0" testChainIDRevision1 = "gaiahub-1" - testClientID = "gaiachain" - testClientID2 = "ethbridge" - testClientID3 = "ethermint" + testClientID = "tendermint-0" + testClientID2 = "tendermint-1" + testClientID3 = "tendermint-2" height = 5 diff --git a/x/ibc/core/02-client/types/client.go b/x/ibc/core/02-client/types/client.go index 305b07cec47..1c44d6e2b9c 100644 --- a/x/ibc/core/02-client/types/client.go +++ b/x/ibc/core/02-client/types/client.go @@ -2,11 +2,15 @@ package types import ( "fmt" + "math" "sort" + "strings" proto "github.com/gogo/protobuf/proto" codectypes "github.com/cosmos/cosmos-sdk/codec/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + host "github.com/cosmos/cosmos-sdk/x/ibc/core/24-host" "github.com/cosmos/cosmos-sdk/x/ibc/core/exported" ) @@ -80,3 +84,28 @@ func NewConsensusStateWithHeight(height Height, consensusState exported.Consensu func (cswh ConsensusStateWithHeight) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error { return unpacker.UnpackAny(cswh.ConsensusState, new(exported.ConsensusState)) } + +// ValidateClientType validates the client type. It cannot be blank or empty. It must be a valid +// client identifier when used with '0' or the maximum uint64 as the sequence. +func ValidateClientType(clientType string) error { + if strings.TrimSpace(clientType) == "" { + return sdkerrors.Wrap(ErrInvalidClientType, "client type cannot be blank") + } + + smallestPossibleClientID := FormatClientIdentifier(clientType, 0) + largestPossibleClientID := FormatClientIdentifier(clientType, math.MaxUint64) + + // IsValidClientID will check client type format and if the sequence is a uint64 + if !IsValidClientID(smallestPossibleClientID) { + return sdkerrors.Wrap(ErrInvalidClientType, "") + } + + if err := host.ClientIdentifierValidator(smallestPossibleClientID); err != nil { + return sdkerrors.Wrap(err, "client type results in smallest client identifier being invalid") + } + if err := host.ClientIdentifierValidator(largestPossibleClientID); err != nil { + return sdkerrors.Wrap(err, "client type results in largest client identifier being invalid") + } + + return nil +} diff --git a/x/ibc/core/02-client/types/client_test.go b/x/ibc/core/02-client/types/client_test.go index 409ab53050a..2dfd3967d28 100644 --- a/x/ibc/core/02-client/types/client_test.go +++ b/x/ibc/core/02-client/types/client_test.go @@ -1,6 +1,10 @@ package types_test import ( + "testing" + + "github.com/stretchr/testify/require" + "github.com/cosmos/cosmos-sdk/x/ibc/core/02-client/types" "github.com/cosmos/cosmos-sdk/x/ibc/core/exported" ibctesting "github.com/cosmos/cosmos-sdk/x/ibc/testing" @@ -54,3 +58,30 @@ func (suite *TypesTestSuite) TestMarshalConsensusStateWithHeight() { }) } } + +func TestValidateClientType(t *testing.T) { + testCases := []struct { + name string + clientType string + expPass bool + }{ + {"valid", "tendermint", true}, + {"valid solomachine", "solomachine-v1", true}, + {"too large", "tenderminttenderminttenderminttenderminttendermintt", false}, + {"too short", "t", false}, + {"blank id", " ", false}, + {"empty id", "", false}, + {"ends with dash", "tendermint-", false}, + } + + for _, tc := range testCases { + + err := types.ValidateClientType(tc.clientType) + + if tc.expPass { + require.NoError(t, err, tc.name) + } else { + require.Error(t, err, tc.name) + } + } +} diff --git a/x/ibc/core/02-client/types/genesis.go b/x/ibc/core/02-client/types/genesis.go index 4da2f5e9206..f7939c47893 100644 --- a/x/ibc/core/02-client/types/genesis.go +++ b/x/ibc/core/02-client/types/genesis.go @@ -67,23 +67,25 @@ func (ccs ClientConsensusStates) UnpackInterfaces(unpacker codectypes.AnyUnpacke // NewGenesisState creates a GenesisState instance. func NewGenesisState( clients []IdentifiedClientState, clientsConsensus ClientsConsensusStates, - params Params, createLocalhost bool, + params Params, createLocalhost bool, nextClientSequence uint64, ) GenesisState { return GenesisState{ - Clients: clients, - ClientsConsensus: clientsConsensus, - Params: params, - CreateLocalhost: createLocalhost, + Clients: clients, + ClientsConsensus: clientsConsensus, + Params: params, + CreateLocalhost: createLocalhost, + NextClientSequence: nextClientSequence, } } // DefaultGenesisState returns the ibc client submodule's default genesis state. func DefaultGenesisState() GenesisState { return GenesisState{ - Clients: []IdentifiedClientState{}, - ClientsConsensus: ClientsConsensusStates{}, - Params: DefaultParams(), - CreateLocalhost: false, + Clients: []IdentifiedClientState{}, + ClientsConsensus: ClientsConsensusStates{}, + Params: DefaultParams(), + CreateLocalhost: false, + NextClientSequence: 0, } } diff --git a/x/ibc/core/02-client/types/genesis.pb.go b/x/ibc/core/02-client/types/genesis.pb.go index 4c72edfe5e7..becfa1baba1 100644 --- a/x/ibc/core/02-client/types/genesis.pb.go +++ b/x/ibc/core/02-client/types/genesis.pb.go @@ -32,6 +32,8 @@ type GenesisState struct { Params Params `protobuf:"bytes,3,opt,name=params,proto3" json:"params"` // create localhost on initialization CreateLocalhost bool `protobuf:"varint,4,opt,name=create_localhost,json=createLocalhost,proto3" json:"create_localhost,omitempty" yaml:"create_localhost"` + // the sequence for the next generated client identifier + NextClientSequence uint64 `protobuf:"varint,5,opt,name=next_client_sequence,json=nextClientSequence,proto3" json:"next_client_sequence,omitempty" yaml:"next_client_sequence"` } func (m *GenesisState) Reset() { *m = GenesisState{} } @@ -95,6 +97,13 @@ func (m *GenesisState) GetCreateLocalhost() bool { return false } +func (m *GenesisState) GetNextClientSequence() uint64 { + if m != nil { + return m.NextClientSequence + } + return 0 +} + func init() { proto.RegisterType((*GenesisState)(nil), "ibc.core.client.v1.GenesisState") } @@ -102,30 +111,32 @@ func init() { func init() { proto.RegisterFile("ibc/core/client/v1/genesis.proto", fileDescriptor_bcd0c0f1f2e6a91a) } var fileDescriptor_bcd0c0f1f2e6a91a = []byte{ - // 362 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x74, 0x91, 0xc1, 0x4e, 0xea, 0x40, - 0x14, 0x86, 0xdb, 0x0b, 0xe1, 0xde, 0x94, 0x9b, 0x88, 0x8d, 0xd1, 0x06, 0x93, 0xb6, 0xe9, 0x0a, - 0x17, 0xcc, 0x08, 0x2e, 0x34, 0x2c, 0x4b, 0xa2, 0x31, 0x71, 0xa1, 0x75, 0xe7, 0x86, 0xb4, 0xc3, - 0x58, 0x26, 0xb6, 0x1d, 0xd2, 0x33, 0x10, 0x79, 0x05, 0x57, 0xc6, 0xc7, 0xf0, 0x49, 0x58, 0xb2, - 0x74, 0x85, 0x06, 0xde, 0x80, 0x27, 0x30, 0xed, 0x14, 0x17, 0x80, 0xab, 0x39, 0xf9, 0xe7, 0xff, - 0xfe, 0xff, 0x24, 0x47, 0xb3, 0x59, 0x40, 0x30, 0xe1, 0x29, 0xc5, 0x24, 0x62, 0x34, 0x11, 0x78, - 0xdc, 0xc2, 0x21, 0x4d, 0x28, 0x30, 0x40, 0xc3, 0x94, 0x0b, 0xae, 0xeb, 0x2c, 0x20, 0x28, 0x73, - 0x20, 0xe9, 0x40, 0xe3, 0x56, 0xdd, 0xda, 0x41, 0x15, 0xbf, 0x39, 0x54, 0x3f, 0x08, 0x79, 0xc8, - 0xf3, 0x11, 0x67, 0x93, 0x54, 0x9d, 0x97, 0x92, 0xf6, 0xff, 0x4a, 0x86, 0xdf, 0x0b, 0x5f, 0x50, - 0x9d, 0x68, 0x7f, 0x25, 0x06, 0x86, 0x6a, 0x97, 0x1a, 0xd5, 0xf6, 0x09, 0xda, 0x6e, 0x43, 0xd7, - 0x7d, 0x9a, 0x08, 0xf6, 0xc8, 0x68, 0xbf, 0x9b, 0x6b, 0x39, 0xeb, 0x9a, 0xd3, 0xb9, 0xa5, 0xbc, - 0x7f, 0x5a, 0x87, 0x3b, 0xbf, 0xc1, 0x5b, 0x27, 0xeb, 0x6f, 0xaa, 0xb6, 0x5f, 0xcc, 0x3d, 0xc2, - 0x13, 0xa0, 0x09, 0x8c, 0xc0, 0xf8, 0xf3, 0x7b, 0x9f, 0x8c, 0xe9, 0xae, 0xad, 0x32, 0xcf, 0xed, - 0x64, 0x7d, 0xab, 0xb9, 0x65, 0x4c, 0xfc, 0x38, 0xea, 0x38, 0x5b, 0x89, 0x4e, 0xb6, 0x8b, 0x44, - 0x61, 0x83, 0xf5, 0x6a, 0x64, 0x43, 0xd7, 0x2f, 0xb4, 0xca, 0xd0, 0x4f, 0xfd, 0x18, 0x8c, 0x92, - 0xad, 0x36, 0xaa, 0xed, 0xfa, 0xae, 0x45, 0x6e, 0x73, 0x87, 0x5b, 0xce, 0x9a, 0xbd, 0xc2, 0xaf, - 0x5f, 0x6a, 0x35, 0x92, 0x52, 0x5f, 0xd0, 0x5e, 0xc4, 0x89, 0x1f, 0x0d, 0x38, 0x08, 0xa3, 0x6c, - 0xab, 0x8d, 0x7f, 0xee, 0xf1, 0x6a, 0x6e, 0x1d, 0x15, 0xdb, 0x6d, 0x38, 0x1c, 0x6f, 0x4f, 0x4a, - 0x37, 0x6b, 0xc5, 0xbd, 0x9b, 0x2e, 0x4c, 0x75, 0xb6, 0x30, 0xd5, 0xaf, 0x85, 0xa9, 0xbe, 0x2e, - 0x4d, 0x65, 0xb6, 0x34, 0x95, 0x8f, 0xa5, 0xa9, 0x3c, 0x9c, 0x87, 0x4c, 0x0c, 0x46, 0x01, 0x22, - 0x3c, 0xc6, 0x84, 0x43, 0xcc, 0xa1, 0x78, 0x9a, 0xd0, 0x7f, 0xc2, 0xcf, 0xf8, 0xe7, 0xf8, 0xa7, - 0xed, 0x66, 0x71, 0x7f, 0x31, 0x19, 0x52, 0x08, 0x2a, 0xf9, 0x99, 0xcf, 0xbe, 0x03, 0x00, 0x00, - 0xff, 0xff, 0x8d, 0xa4, 0x74, 0xd6, 0x55, 0x02, 0x00, 0x00, + // 400 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x74, 0x92, 0xc1, 0x8e, 0x9a, 0x40, + 0x1c, 0xc6, 0x99, 0x6a, 0x6d, 0x83, 0x4d, 0x6a, 0x27, 0xa6, 0x25, 0x9a, 0x00, 0xe1, 0x44, 0x0f, + 0x32, 0xd5, 0x1e, 0xda, 0x78, 0xc4, 0xa4, 0x4d, 0x93, 0x1e, 0x2a, 0xbd, 0xf5, 0x42, 0x60, 0x9c, + 0x22, 0x29, 0x30, 0x96, 0x19, 0x8d, 0xbe, 0xc5, 0x66, 0x1f, 0x63, 0x9f, 0xc4, 0xa3, 0xc7, 0xbd, + 0x2c, 0xbb, 0xd1, 0x37, 0xf0, 0x09, 0x36, 0x30, 0xe3, 0x1e, 0x94, 0x3d, 0xf1, 0xcf, 0x37, 0xbf, + 0xef, 0xfb, 0xfe, 0x21, 0x7f, 0xd5, 0x8c, 0x43, 0x8c, 0x30, 0xcd, 0x09, 0xc2, 0x49, 0x4c, 0x32, + 0x8e, 0x56, 0x43, 0x14, 0x91, 0x8c, 0xb0, 0x98, 0x39, 0x8b, 0x9c, 0x72, 0x0a, 0x61, 0x1c, 0x62, + 0xa7, 0x24, 0x1c, 0x41, 0x38, 0xab, 0x61, 0xcf, 0xa8, 0x71, 0xc9, 0xd7, 0xca, 0xd4, 0xeb, 0x46, + 0x34, 0xa2, 0xd5, 0x88, 0xca, 0x49, 0xa8, 0xd6, 0x5d, 0x43, 0x7d, 0xf3, 0x5d, 0x84, 0xff, 0xe6, + 0x01, 0x27, 0x10, 0xab, 0xaf, 0x84, 0x8d, 0x69, 0xc0, 0x6c, 0xd8, 0xed, 0xd1, 0x47, 0xe7, 0xb2, + 0xcd, 0xf9, 0x31, 0x23, 0x19, 0x8f, 0xff, 0xc6, 0x64, 0x36, 0xa9, 0xb4, 0xca, 0xeb, 0xea, 0xdb, + 0xc2, 0x50, 0x6e, 0xee, 0x8d, 0xf7, 0xb5, 0xcf, 0xcc, 0x3b, 0x25, 0xc3, 0x6b, 0xa0, 0xbe, 0x93, + 0xb3, 0x8f, 0x69, 0xc6, 0x48, 0xc6, 0x96, 0x4c, 0x7b, 0xf1, 0x7c, 0x9f, 0x88, 0x99, 0x9c, 0x50, + 0x91, 0xe7, 0x8e, 0xcb, 0xbe, 0x63, 0x61, 0x68, 0x9b, 0x20, 0x4d, 0xc6, 0xd6, 0x45, 0xa2, 0x55, + 0xee, 0x22, 0xac, 0xec, 0xcc, 0xeb, 0x75, 0xf0, 0x99, 0x0e, 0xbf, 0xaa, 0xad, 0x45, 0x90, 0x07, + 0x29, 0xd3, 0x1a, 0x26, 0xb0, 0xdb, 0xa3, 0x5e, 0xdd, 0x22, 0xbf, 0x2a, 0xc2, 0x6d, 0x96, 0xcd, + 0x9e, 0xe4, 0xe1, 0x37, 0xb5, 0x83, 0x73, 0x12, 0x70, 0xe2, 0x27, 0x14, 0x07, 0xc9, 0x9c, 0x32, + 0xae, 0x35, 0x4d, 0x60, 0xbf, 0x76, 0xfb, 0xc7, 0xc2, 0xf8, 0x20, 0xb7, 0x3b, 0x23, 0x2c, 0xef, + 0xad, 0x90, 0x7e, 0x9e, 0x14, 0x38, 0x55, 0xbb, 0x19, 0x59, 0x73, 0x5f, 0xd4, 0xf9, 0x8c, 0xfc, + 0x5f, 0x92, 0x0c, 0x13, 0xed, 0xa5, 0x09, 0xec, 0xa6, 0x6b, 0x1c, 0x0b, 0xa3, 0x2f, 0xb2, 0xea, + 0x28, 0xcb, 0x83, 0xa5, 0x2c, 0x7f, 0xb8, 0x14, 0xdd, 0xe9, 0x76, 0xaf, 0x83, 0xdd, 0x5e, 0x07, + 0x0f, 0x7b, 0x1d, 0x5c, 0x1d, 0x74, 0x65, 0x77, 0xd0, 0x95, 0xdb, 0x83, 0xae, 0xfc, 0xf9, 0x12, + 0xc5, 0x7c, 0xbe, 0x0c, 0x1d, 0x4c, 0x53, 0x84, 0x29, 0x4b, 0x29, 0x93, 0x9f, 0x01, 0x9b, 0xfd, + 0x43, 0x6b, 0xf4, 0x74, 0x4f, 0x9f, 0x46, 0x03, 0x79, 0x52, 0x7c, 0xb3, 0x20, 0x2c, 0x6c, 0x55, + 0x97, 0xf3, 0xf9, 0x31, 0x00, 0x00, 0xff, 0xff, 0x7c, 0xcd, 0xe7, 0x85, 0xa8, 0x02, 0x00, 0x00, } func (m *GenesisState) Marshal() (dAtA []byte, err error) { @@ -148,6 +159,11 @@ func (m *GenesisState) MarshalToSizedBuffer(dAtA []byte) (int, error) { _ = i var l int _ = l + if m.NextClientSequence != 0 { + i = encodeVarintGenesis(dAtA, i, uint64(m.NextClientSequence)) + i-- + dAtA[i] = 0x28 + } if m.CreateLocalhost { i-- if m.CreateLocalhost { @@ -233,6 +249,9 @@ func (m *GenesisState) Size() (n int) { if m.CreateLocalhost { n += 2 } + if m.NextClientSequence != 0 { + n += 1 + sovGenesis(uint64(m.NextClientSequence)) + } return n } @@ -392,6 +411,25 @@ func (m *GenesisState) Unmarshal(dAtA []byte) error { } } m.CreateLocalhost = bool(v != 0) + case 5: + if wireType != 0 { + return fmt.Errorf("proto: wrong wireType = %d for field NextClientSequence", wireType) + } + m.NextClientSequence = 0 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowGenesis + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + m.NextClientSequence |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } default: iNdEx = preIndex skippy, err := skipGenesis(dAtA[iNdEx:]) diff --git a/x/ibc/core/02-client/types/genesis_test.go b/x/ibc/core/02-client/types/genesis_test.go index 7d131bae619..81eff78335d 100644 --- a/x/ibc/core/02-client/types/genesis_test.go +++ b/x/ibc/core/02-client/types/genesis_test.go @@ -90,6 +90,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { }, types.NewParams(exported.Tendermint, exported.Localhost), false, + 0, ), expPass: true, }, @@ -119,6 +120,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { }, types.NewParams(exported.Tendermint), false, + 0, ), expPass: false, }, @@ -134,6 +136,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { nil, types.NewParams(exported.Tendermint), false, + 0, ), expPass: false, }, @@ -163,6 +166,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { }, types.NewParams(exported.Tendermint), false, + 0, ), expPass: false, }, @@ -192,6 +196,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { }, types.NewParams(exported.Tendermint), false, + 0, ), expPass: false, }, @@ -221,6 +226,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { }, types.NewParams(exported.Tendermint), false, + 0, ), expPass: false, }, @@ -250,6 +256,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { }, types.NewParams(exported.Solomachine), false, + 0, ), expPass: false, }, @@ -279,6 +286,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { }, types.NewParams(" "), false, + 0, ), expPass: false, }, @@ -308,6 +316,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { }, types.NewParams(" "), true, + 0, ), expPass: false, }, @@ -337,6 +346,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { }, types.NewParams(exported.Tendermint), true, + 0, ), expPass: false, }, diff --git a/x/ibc/core/02-client/types/height.go b/x/ibc/core/02-client/types/height.go index d507ddea8c8..9ac6fec7559 100644 --- a/x/ibc/core/02-client/types/height.go +++ b/x/ibc/core/02-client/types/height.go @@ -17,7 +17,7 @@ var _ exported.Height = (*Height)(nil) // IsRevisionFormat checks if a chainID is in the format required for parsing revisions // The chainID must be in the form: `{chainID}-{revision} // 24-host may enforce stricter checks on chainID -var IsRevisionFormat = regexp.MustCompile(`^.+[^-]-{1}[1-9][0-9]*$`).MatchString +var IsRevisionFormat = regexp.MustCompile(`^.*[^-]-{1}[1-9][0-9]*$`).MatchString // ZeroHeight is a helper function which returns an uninitialized height. func ZeroHeight() Height { diff --git a/x/ibc/core/02-client/types/height_test.go b/x/ibc/core/02-client/types/height_test.go index f2615c8c1d7..a455b7f58d4 100644 --- a/x/ibc/core/02-client/types/height_test.go +++ b/x/ibc/core/02-client/types/height_test.go @@ -104,6 +104,7 @@ func TestParseChainID(t *testing.T) { formatted bool }{ {"gaiamainnet-3", 3, true}, + {"a-1", 1, true}, {"gaia-mainnet-40", 40, true}, {"gaiamainnet-3-39", 39, true}, {"gaiamainnet--", 0, false}, @@ -111,10 +112,13 @@ func TestParseChainID(t *testing.T) { {"gaiamainnet--4", 0, false}, {"gaiamainnet-3.4", 0, false}, {"gaiamainnet", 0, false}, + {"a--1", 0, false}, + {"-1", 0, false}, + {"--1", 0, false}, } for i, tc := range cases { - require.Equal(t, tc.formatted, types.IsRevisionFormat(tc.chainID), "case %d does not match expected format", i) + require.Equal(t, tc.formatted, types.IsRevisionFormat(tc.chainID), "id %s does not match expected format", tc.chainID) revision := types.ParseChainID(tc.chainID) require.Equal(t, tc.revision, revision, "case %d returns incorrect revision", i) diff --git a/x/ibc/core/02-client/types/keys.go b/x/ibc/core/02-client/types/keys.go index f360393335d..321f5e3ffa3 100644 --- a/x/ibc/core/02-client/types/keys.go +++ b/x/ibc/core/02-client/types/keys.go @@ -1,5 +1,15 @@ package types +import ( + "fmt" + "regexp" + "strconv" + "strings" + + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + host "github.com/cosmos/cosmos-sdk/x/ibc/core/24-host" +) + const ( // SubModuleName defines the IBC client name SubModuleName string = "client" @@ -9,4 +19,47 @@ const ( // QuerierRoute is the querier route for IBC client QuerierRoute string = SubModuleName + + // KeyNextClientSequence is the key used to store the next client sequence in + // the keeper. + KeyNextClientSequence = "nextClientSequence" ) + +// FormatClientIdentifier returns the client identifier with the sequence appended. +// This is a SDK specific format not enforced by IBC protocol. +func FormatClientIdentifier(clientType string, sequence uint64) string { + return fmt.Sprintf("%s-%d", clientType, sequence) +} + +// IsClientIDFormat checks if a clientID is in the format required on the SDK for +// parsing client identifiers. The client identifier must be in the form: `{client-type}-{N} +var IsClientIDFormat = regexp.MustCompile(`^.*[^-]-[0-9]{1,20}$`).MatchString + +// IsValidClientID checks if the clientID is valid and can be parsed into the client +// identifier format. +func IsValidClientID(clientID string) bool { + _, _, err := ParseClientIdentifier(clientID) + return err == nil +} + +// ParseClientIdentifier parses the client type and sequence from the client identifier. +func ParseClientIdentifier(clientID string) (string, uint64, error) { + if !IsClientIDFormat(clientID) { + return "", 0, sdkerrors.Wrapf(host.ErrInvalidID, "invalid client identifier %s is not in format: `{client-type}-{N}`", clientID) + } + + splitStr := strings.Split(clientID, "-") + lastIndex := len(splitStr) - 1 + + clientType := strings.Join(splitStr[:lastIndex], "-") + if strings.TrimSpace(clientType) == "" { + return "", 0, sdkerrors.Wrap(host.ErrInvalidID, "client identifier must be in format: `{client-type}-{N}` and client type cannot be blank") + } + + sequence, err := strconv.ParseUint(splitStr[lastIndex], 10, 64) + if err != nil { + return "", 0, sdkerrors.Wrap(err, "failed to parse client identifier sequence") + } + + return clientType, sequence, nil +} diff --git a/x/ibc/core/02-client/types/keys_test.go b/x/ibc/core/02-client/types/keys_test.go new file mode 100644 index 00000000000..dbe56657ad8 --- /dev/null +++ b/x/ibc/core/02-client/types/keys_test.go @@ -0,0 +1,53 @@ +package types_test + +import ( + "math" + "testing" + + "github.com/cosmos/cosmos-sdk/x/ibc/core/02-client/types" + "github.com/stretchr/testify/require" +) + +// tests ParseClientIdentifier and IsValidClientID +func TestParseClientIdentifier(t *testing.T) { + testCases := []struct { + name string + clientID string + clientType string + expSeq uint64 + expPass bool + }{ + {"valid 0", "tendermint-0", "tendermint", 0, true}, + {"valid 1", "tendermint-1", "tendermint", 1, true}, + {"valid solemachine", "solomachine-v1-1", "solomachine-v1", 1, true}, + {"valid large sequence", types.FormatClientIdentifier("tendermint", math.MaxUint64), "tendermint", math.MaxUint64, true}, + {"valid short client type", "t-0", "t", 0, true}, + // one above uint64 max + {"invalid uint64", "tendermint-18446744073709551616", "tendermint", 0, false}, + // uint64 == 20 characters + {"invalid large sequence", "tendermint-2345682193567182931243", "tendermint", 0, false}, + {"missing dash", "tendermint0", "tendermint", 0, false}, + {"blank id", " ", " ", 0, false}, + {"empty id", "", "", 0, false}, + {"negative sequence", "tendermint--1", "tendermint", 0, false}, + {"invalid format", "tendermint-tm", "tendermint", 0, false}, + {"empty clientype", " -100", "tendermint", 0, false}, + } + + for _, tc := range testCases { + + clientType, seq, err := types.ParseClientIdentifier(tc.clientID) + valid := types.IsValidClientID(tc.clientID) + require.Equal(t, tc.expSeq, seq, tc.clientID) + + if tc.expPass { + require.NoError(t, err, tc.name) + require.True(t, valid) + require.Equal(t, tc.clientType, clientType) + } else { + require.Error(t, err, tc.name, tc.clientID) + require.False(t, valid) + require.Equal(t, "", clientType) + } + } +} diff --git a/x/ibc/core/02-client/types/msgs.go b/x/ibc/core/02-client/types/msgs.go index d1d084ddadd..1e884123d74 100644 --- a/x/ibc/core/02-client/types/msgs.go +++ b/x/ibc/core/02-client/types/msgs.go @@ -31,7 +31,7 @@ var ( // NewMsgCreateClient creates a new MsgCreateClient instance //nolint:interfacer func NewMsgCreateClient( - id string, clientState exported.ClientState, consensusState exported.ConsensusState, signer sdk.AccAddress, + clientState exported.ClientState, consensusState exported.ConsensusState, signer sdk.AccAddress, ) (*MsgCreateClient, error) { anyClientState, err := PackClientState(clientState) @@ -45,7 +45,6 @@ func NewMsgCreateClient( } return &MsgCreateClient{ - ClientId: id, ClientState: anyClientState, ConsensusState: anyConsensusState, Signer: signer.String(), @@ -75,17 +74,20 @@ func (msg MsgCreateClient) ValidateBasic() error { if err := clientState.Validate(); err != nil { return err } - if clientState.ClientType() == exported.Localhost || msg.ClientId == exported.Localhost { + if clientState.ClientType() == exported.Localhost { return sdkerrors.Wrap(ErrInvalidClient, "localhost client can only be created on chain initialization") } consensusState, err := UnpackConsensusState(msg.ConsensusState) if err != nil { return err } - if err := consensusState.ValidateBasic(); err != nil { - return err + if clientState.ClientType() != consensusState.ClientType() { + return sdkerrors.Wrap(ErrInvalidClientType, "client type for client state and consensus state do not match") } - return host.ClientIdentifierValidator(msg.ClientId) + if err := ValidateClientType(clientState.ClientType()); err != nil { + return sdkerrors.Wrap(err, "client type does not meet naming constraints") + } + return consensusState.ValidateBasic() } // GetSignBytes implements sdk.Msg. The function will panic since it is used diff --git a/x/ibc/core/02-client/types/msgs_test.go b/x/ibc/core/02-client/types/msgs_test.go index 7d2da65d2cd..e42725bae26 100644 --- a/x/ibc/core/02-client/types/msgs_test.go +++ b/x/ibc/core/02-client/types/msgs_test.go @@ -12,7 +12,6 @@ import ( "github.com/cosmos/cosmos-sdk/x/ibc/core/exported" solomachinetypes "github.com/cosmos/cosmos-sdk/x/ibc/light-clients/06-solomachine/types" ibctmtypes "github.com/cosmos/cosmos-sdk/x/ibc/light-clients/07-tendermint/types" - localhosttypes "github.com/cosmos/cosmos-sdk/x/ibc/light-clients/09-localhost/types" ibctesting "github.com/cosmos/cosmos-sdk/x/ibc/testing" ) @@ -50,14 +49,14 @@ func (suite *TypesTestSuite) TestMarshalMsgCreateClient() { { "solo machine client", func() { soloMachine := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "solomachine", "", 2) - msg, err = types.NewMsgCreateClient(soloMachine.ClientID, soloMachine.ClientState(), soloMachine.ConsensusState(), suite.chainA.SenderAccount.GetAddress()) + msg, err = types.NewMsgCreateClient(soloMachine.ClientState(), soloMachine.ConsensusState(), suite.chainA.SenderAccount.GetAddress()) suite.Require().NoError(err) }, }, { "tendermint client", func() { tendermintClient := ibctmtypes.NewClientState(suite.chainA.ChainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false) - msg, err = types.NewMsgCreateClient("tendermint", tendermintClient, suite.chainA.CurrentTMClientHeader().ConsensusState(), suite.chainA.SenderAccount.GetAddress()) + msg, err = types.NewMsgCreateClient(tendermintClient, suite.chainA.CurrentTMClientHeader().ConsensusState(), suite.chainA.SenderAccount.GetAddress()) suite.Require().NoError(err) }, }, @@ -98,18 +97,11 @@ func (suite *TypesTestSuite) TestMsgCreateClient_ValidateBasic() { malleate func() expPass bool }{ - { - "invalid client-id", - func() { - msg.ClientId = "" - }, - false, - }, { "valid - tendermint client", func() { tendermintClient := ibctmtypes.NewClientState(suite.chainA.ChainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false) - msg, err = types.NewMsgCreateClient("tendermint", tendermintClient, suite.chainA.CurrentTMClientHeader().ConsensusState(), suite.chainA.SenderAccount.GetAddress()) + msg, err = types.NewMsgCreateClient(tendermintClient, suite.chainA.CurrentTMClientHeader().ConsensusState(), suite.chainA.SenderAccount.GetAddress()) suite.Require().NoError(err) }, true, @@ -117,7 +109,7 @@ func (suite *TypesTestSuite) TestMsgCreateClient_ValidateBasic() { { "invalid tendermint client", func() { - msg, err = types.NewMsgCreateClient("tendermint", &ibctmtypes.ClientState{}, suite.chainA.CurrentTMClientHeader().ConsensusState(), suite.chainA.SenderAccount.GetAddress()) + msg, err = types.NewMsgCreateClient(&ibctmtypes.ClientState{}, suite.chainA.CurrentTMClientHeader().ConsensusState(), suite.chainA.SenderAccount.GetAddress()) suite.Require().NoError(err) }, false, @@ -133,7 +125,7 @@ func (suite *TypesTestSuite) TestMsgCreateClient_ValidateBasic() { "failed to unpack consensus state", func() { tendermintClient := ibctmtypes.NewClientState(suite.chainA.ChainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false) - msg, err = types.NewMsgCreateClient("tendermint", tendermintClient, suite.chainA.CurrentTMClientHeader().ConsensusState(), suite.chainA.SenderAccount.GetAddress()) + msg, err = types.NewMsgCreateClient(tendermintClient, suite.chainA.CurrentTMClientHeader().ConsensusState(), suite.chainA.SenderAccount.GetAddress()) suite.Require().NoError(err) msg.ConsensusState = nil }, @@ -150,7 +142,7 @@ func (suite *TypesTestSuite) TestMsgCreateClient_ValidateBasic() { "valid - solomachine client", func() { soloMachine := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "solomachine", "", 2) - msg, err = types.NewMsgCreateClient(soloMachine.ClientID, soloMachine.ClientState(), soloMachine.ConsensusState(), suite.chainA.SenderAccount.GetAddress()) + msg, err = types.NewMsgCreateClient(soloMachine.ClientState(), soloMachine.ConsensusState(), suite.chainA.SenderAccount.GetAddress()) suite.Require().NoError(err) }, true, @@ -159,7 +151,7 @@ func (suite *TypesTestSuite) TestMsgCreateClient_ValidateBasic() { "invalid solomachine client", func() { soloMachine := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "solomachine", "", 2) - msg, err = types.NewMsgCreateClient(soloMachine.ClientID, &solomachinetypes.ClientState{}, soloMachine.ConsensusState(), suite.chainA.SenderAccount.GetAddress()) + msg, err = types.NewMsgCreateClient(&solomachinetypes.ClientState{}, soloMachine.ConsensusState(), suite.chainA.SenderAccount.GetAddress()) suite.Require().NoError(err) }, false, @@ -168,16 +160,17 @@ func (suite *TypesTestSuite) TestMsgCreateClient_ValidateBasic() { "invalid solomachine consensus state", func() { soloMachine := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "solomachine", "", 2) - msg, err = types.NewMsgCreateClient(soloMachine.ClientID, soloMachine.ClientState(), &solomachinetypes.ConsensusState{}, suite.chainA.SenderAccount.GetAddress()) + msg, err = types.NewMsgCreateClient(soloMachine.ClientState(), &solomachinetypes.ConsensusState{}, suite.chainA.SenderAccount.GetAddress()) suite.Require().NoError(err) }, false, }, { - "unsupported - localhost client", + "invalid - client state and consensus state client types do not match", func() { - localhostClient := localhosttypes.NewClientState(suite.chainA.ChainID, types.NewHeight(0, uint64(suite.chainA.LastHeader.Header.Height))) - msg, err = types.NewMsgCreateClient("localhost", localhostClient, suite.chainA.LastHeader.ConsensusState(), suite.chainA.SenderAccount.GetAddress()) + tendermintClient := ibctmtypes.NewClientState(suite.chainA.ChainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false) + soloMachine := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "solomachine", "", 2) + msg, err = types.NewMsgCreateClient(tendermintClient, soloMachine.ConsensusState(), suite.chainA.SenderAccount.GetAddress()) suite.Require().NoError(err) }, false, diff --git a/x/ibc/core/02-client/types/tx.pb.go b/x/ibc/core/02-client/types/tx.pb.go index 2a1a6b8885c..b0bf22d0cc1 100644 --- a/x/ibc/core/02-client/types/tx.pb.go +++ b/x/ibc/core/02-client/types/tx.pb.go @@ -31,15 +31,13 @@ const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package // MsgCreateClient defines a message to create an IBC client type MsgCreateClient struct { - // client unique identifier - ClientId string `protobuf:"bytes,1,opt,name=client_id,json=clientId,proto3" json:"client_id,omitempty" yaml:"client_id"` // light client state - ClientState *types.Any `protobuf:"bytes,2,opt,name=client_state,json=clientState,proto3" json:"client_state,omitempty" yaml:"client_state"` + ClientState *types.Any `protobuf:"bytes,1,opt,name=client_state,json=clientState,proto3" json:"client_state,omitempty" yaml:"client_state"` // consensus state associated with the client that corresponds to a given // height. - ConsensusState *types.Any `protobuf:"bytes,3,opt,name=consensus_state,json=consensusState,proto3" json:"consensus_state,omitempty" yaml:"consensus_state"` + ConsensusState *types.Any `protobuf:"bytes,2,opt,name=consensus_state,json=consensusState,proto3" json:"consensus_state,omitempty" yaml:"consensus_state"` // signer address - Signer string `protobuf:"bytes,4,opt,name=signer,proto3" json:"signer,omitempty"` + Signer string `protobuf:"bytes,3,opt,name=signer,proto3" json:"signer,omitempty"` } func (m *MsgCreateClient) Reset() { *m = MsgCreateClient{} } @@ -374,45 +372,45 @@ func init() { func init() { proto.RegisterFile("ibc/core/client/v1/tx.proto", fileDescriptor_cb5dc4651eb49a04) } var fileDescriptor_cb5dc4651eb49a04 = []byte{ - // 598 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xe4, 0x55, 0x3f, 0x6f, 0xd3, 0x4e, - 0x18, 0x8e, 0x9b, 0xdf, 0x2f, 0x6a, 0xaf, 0x81, 0x56, 0x26, 0xb4, 0xa9, 0xab, 0xda, 0x95, 0xe9, - 0x10, 0x44, 0xeb, 0x23, 0x61, 0x00, 0x75, 0x23, 0x9d, 0x18, 0x22, 0x51, 0x57, 0x0c, 0xb0, 0x04, - 0xff, 0xb9, 0x5e, 0xac, 0x26, 0xbe, 0xc8, 0x67, 0x47, 0xcd, 0x37, 0x60, 0x44, 0x82, 0x0f, 0x50, - 0x31, 0xf0, 0x59, 0x18, 0x3b, 0x30, 0x30, 0x45, 0x28, 0x59, 0x98, 0xf3, 0x09, 0x90, 0xef, 0x1c, - 0xcb, 0x76, 0xed, 0x28, 0x82, 0x91, 0xc9, 0x7e, 0xef, 0x7d, 0xee, 0x79, 0x9e, 0x7b, 0xdf, 0xf7, - 0x6c, 0xb0, 0xef, 0x98, 0x16, 0xb4, 0x88, 0x87, 0xa0, 0xd5, 0x77, 0x90, 0xeb, 0xc3, 0x51, 0x13, - 0xfa, 0xd7, 0xda, 0xd0, 0x23, 0x3e, 0x11, 0x45, 0xc7, 0xb4, 0xb4, 0x30, 0xa9, 0xf1, 0xa4, 0x36, - 0x6a, 0x4a, 0x35, 0x4c, 0x30, 0x61, 0x69, 0x18, 0xbe, 0x71, 0xa4, 0xb4, 0x87, 0x09, 0xc1, 0x7d, - 0x04, 0x59, 0x64, 0x06, 0x97, 0xd0, 0x70, 0xc7, 0x51, 0x4a, 0xc9, 0x51, 0x88, 0xe8, 0x18, 0x40, - 0xfd, 0xb4, 0x06, 0xb6, 0x3a, 0x14, 0x9f, 0x79, 0xc8, 0xf0, 0xd1, 0x19, 0xcb, 0x88, 0x4d, 0xb0, - 0xc1, 0x31, 0x5d, 0xc7, 0xae, 0x0b, 0x87, 0x42, 0x63, 0xa3, 0x5d, 0x9b, 0x4f, 0x94, 0xed, 0xb1, - 0x31, 0xe8, 0x9f, 0xaa, 0x71, 0x4a, 0xd5, 0xd7, 0xf9, 0xfb, 0x2b, 0x5b, 0x7c, 0x0d, 0xaa, 0xd1, - 0x3a, 0xf5, 0x0d, 0x1f, 0xd5, 0xd7, 0x0e, 0x85, 0xc6, 0x66, 0xab, 0xa6, 0x71, 0x67, 0xda, 0xc2, - 0x99, 0xf6, 0xd2, 0x1d, 0xb7, 0x77, 0xe7, 0x13, 0xe5, 0x41, 0x8a, 0x8b, 0xed, 0x51, 0xf5, 0x4d, - 0x1e, 0x5e, 0x84, 0x91, 0xf8, 0x16, 0x6c, 0x59, 0xc4, 0xa5, 0xc8, 0xa5, 0x01, 0x8d, 0x48, 0xcb, - 0x4b, 0x48, 0xa5, 0xf9, 0x44, 0xd9, 0x89, 0x48, 0xd3, 0xdb, 0x54, 0xfd, 0x7e, 0xbc, 0xc2, 0xa9, - 0x77, 0x40, 0x85, 0x3a, 0xd8, 0x45, 0x5e, 0xfd, 0xbf, 0xf0, 0x70, 0x7a, 0x14, 0x9d, 0xae, 0x7f, - 0xb8, 0x51, 0x4a, 0xbf, 0x6e, 0x94, 0x92, 0xba, 0x07, 0x76, 0x33, 0x45, 0xd1, 0x11, 0x1d, 0x86, - 0x2c, 0xea, 0x67, 0x81, 0x15, 0xec, 0xcd, 0xd0, 0xfe, 0xab, 0x82, 0x1d, 0x83, 0x4a, 0x0f, 0x19, - 0x36, 0xf2, 0x96, 0x95, 0x4a, 0x8f, 0x30, 0x09, 0xc7, 0xe5, 0xa5, 0x8e, 0x93, 0xae, 0x62, 0xc7, - 0xdf, 0xcb, 0x60, 0x9b, 0xe5, 0xb0, 0x67, 0xd8, 0xff, 0x4a, 0x8f, 0xcf, 0x41, 0x6d, 0xe8, 0x11, - 0x72, 0xd9, 0x0d, 0xf8, 0xb1, 0xbb, 0x5c, 0x97, 0x75, 0xbc, 0xda, 0x56, 0xe6, 0x13, 0x65, 0x9f, - 0x33, 0xe5, 0xa1, 0x54, 0x5d, 0x64, 0xcb, 0xe9, 0x92, 0x5d, 0x81, 0x83, 0x0c, 0x38, 0xe3, 0xfd, - 0x7f, 0xc6, 0xdd, 0x98, 0x4f, 0x94, 0xa3, 0x5c, 0xee, 0xac, 0x67, 0x29, 0x25, 0x52, 0x34, 0xa3, - 0x95, 0x82, 0x8e, 0x4b, 0xa0, 0x9e, 0xed, 0x6a, 0xdc, 0xf2, 0xaf, 0x02, 0x78, 0xd8, 0xa1, 0xf8, - 0x22, 0x30, 0x07, 0x8e, 0xdf, 0x71, 0xa8, 0x89, 0x7a, 0xc6, 0xc8, 0x21, 0x81, 0xf7, 0x27, 0x7d, - 0x7f, 0x01, 0xaa, 0x83, 0x04, 0xc5, 0xd2, 0x81, 0x4d, 0x21, 0x57, 0x18, 0x5b, 0x05, 0x1c, 0xe4, - 0xfa, 0x5c, 0x9c, 0xa4, 0xf5, 0xa5, 0x0c, 0xca, 0x1d, 0x8a, 0xc5, 0xf7, 0xa0, 0x9a, 0xfa, 0x46, - 0x3d, 0xd2, 0xee, 0x7e, 0x1e, 0xb5, 0xcc, 0x9d, 0x95, 0x9e, 0xac, 0x00, 0x5a, 0x28, 0x85, 0x0a, - 0xa9, 0x4b, 0x5d, 0xa4, 0x90, 0x04, 0x15, 0x2a, 0xe4, 0x5d, 0x44, 0xd1, 0x02, 0xf7, 0xd2, 0x13, - 0x75, 0x54, 0xb8, 0x3b, 0x81, 0x92, 0x8e, 0x57, 0x41, 0xc5, 0x22, 0x1e, 0x10, 0x73, 0xda, 0xfe, - 0xb8, 0x80, 0xe3, 0x2e, 0x54, 0x6a, 0xae, 0x0c, 0x5d, 0x68, 0xb6, 0xcf, 0xbf, 0x4d, 0x65, 0xe1, - 0x76, 0x2a, 0x0b, 0x3f, 0xa7, 0xb2, 0xf0, 0x71, 0x26, 0x97, 0x6e, 0x67, 0x72, 0xe9, 0xc7, 0x4c, - 0x2e, 0xbd, 0x7b, 0x8e, 0x1d, 0xbf, 0x17, 0x98, 0x9a, 0x45, 0x06, 0xd0, 0x22, 0x74, 0x40, 0x68, - 0xf4, 0x38, 0xa1, 0xf6, 0x15, 0xbc, 0x86, 0xf1, 0xef, 0xe9, 0x69, 0xeb, 0x24, 0xfa, 0x43, 0xf9, - 0xe3, 0x21, 0xa2, 0x66, 0x85, 0x8d, 0xd5, 0xb3, 0xdf, 0x01, 0x00, 0x00, 0xff, 0xff, 0xd2, 0x19, - 0x59, 0x52, 0x23, 0x07, 0x00, 0x00, + // 601 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xb4, 0x55, 0x3f, 0x6f, 0xd3, 0x40, + 0x1c, 0x8d, 0x1b, 0x88, 0x9a, 0x6b, 0xa0, 0x95, 0x09, 0x6d, 0xea, 0xaa, 0x76, 0x64, 0x3a, 0x04, + 0xd1, 0xfa, 0x48, 0x18, 0x40, 0xdd, 0x48, 0x27, 0x86, 0x48, 0xd4, 0x15, 0x03, 0x2c, 0xc1, 0x7f, + 0xae, 0x97, 0x53, 0x13, 0x5f, 0xe4, 0xb3, 0xa3, 0xe6, 0x1b, 0x30, 0x32, 0xf0, 0x01, 0x2a, 0x06, + 0x3e, 0x0b, 0x63, 0x07, 0x06, 0xa6, 0xa8, 0x4a, 0x16, 0xe6, 0x7c, 0x02, 0x14, 0x9f, 0x13, 0x62, + 0xd7, 0x8e, 0x2c, 0xa0, 0x53, 0x7c, 0xfe, 0xbd, 0x7b, 0xef, 0xf7, 0xf2, 0x7e, 0xe7, 0x03, 0x7b, + 0xc4, 0xb4, 0xa0, 0x45, 0x5d, 0x04, 0xad, 0x2e, 0x41, 0x8e, 0x07, 0x07, 0x75, 0xe8, 0x5d, 0x6a, + 0x7d, 0x97, 0x7a, 0x54, 0x14, 0x89, 0x69, 0x69, 0xb3, 0xa2, 0xc6, 0x8b, 0xda, 0xa0, 0x2e, 0x95, + 0x31, 0xc5, 0x34, 0x28, 0xc3, 0xd9, 0x13, 0x47, 0x4a, 0xbb, 0x98, 0x52, 0xdc, 0x45, 0x30, 0x58, + 0x99, 0xfe, 0x39, 0x34, 0x9c, 0x61, 0x58, 0x52, 0x12, 0x14, 0x42, 0xba, 0x00, 0xa0, 0xde, 0x08, + 0x60, 0xb3, 0xc5, 0xf0, 0x89, 0x8b, 0x0c, 0x0f, 0x9d, 0x04, 0x15, 0xf1, 0x2d, 0x28, 0x71, 0x4c, + 0x9b, 0x79, 0x86, 0x87, 0x2a, 0x42, 0x55, 0xa8, 0x6d, 0x34, 0xca, 0x1a, 0x97, 0xd1, 0xe6, 0x32, + 0xda, 0x6b, 0x67, 0xd8, 0xdc, 0x99, 0x8e, 0x94, 0x47, 0x43, 0xa3, 0xd7, 0x3d, 0x56, 0x97, 0xf7, + 0xa8, 0xfa, 0x06, 0x5f, 0x9e, 0xcd, 0x56, 0xe2, 0x7b, 0xb0, 0x69, 0x51, 0x87, 0x21, 0x87, 0xf9, + 0x2c, 0x24, 0x5d, 0x5b, 0x41, 0x2a, 0x4d, 0x47, 0xca, 0x76, 0x48, 0x1a, 0xdd, 0xa6, 0xea, 0x0f, + 0x17, 0x6f, 0x38, 0xf5, 0x36, 0x28, 0x30, 0x82, 0x1d, 0xe4, 0x56, 0xf2, 0x55, 0xa1, 0x56, 0xd4, + 0xc3, 0xd5, 0xf1, 0xfa, 0xa7, 0x2b, 0x25, 0xf7, 0xeb, 0x4a, 0xc9, 0xa9, 0xbb, 0x60, 0x27, 0xe6, + 0x50, 0x47, 0xac, 0x3f, 0x63, 0x51, 0xbf, 0x70, 0xf7, 0xef, 0xfa, 0xf6, 0x1f, 0xf7, 0x75, 0x50, + 0x0c, 0x9d, 0x10, 0x3b, 0xb0, 0x5e, 0x6c, 0x96, 0xa7, 0x23, 0x65, 0x2b, 0x62, 0x92, 0xd8, 0xaa, + 0xbe, 0xce, 0x9f, 0xdf, 0xd8, 0xe2, 0x21, 0x28, 0x74, 0x90, 0x61, 0x23, 0x77, 0x95, 0x2b, 0x3d, + 0xc4, 0x64, 0xee, 0x78, 0xb9, 0xab, 0x45, 0xc7, 0x3f, 0xf2, 0x60, 0x2b, 0xa8, 0x61, 0xd7, 0xb0, + 0xff, 0xa1, 0xe5, 0x78, 0xc6, 0x6b, 0x77, 0x91, 0x71, 0xfe, 0x3f, 0x65, 0x7c, 0x0a, 0xca, 0x7d, + 0x97, 0xd2, 0xf3, 0xb6, 0xcf, 0x6d, 0xb7, 0xb9, 0x6e, 0xe5, 0x5e, 0x55, 0xa8, 0x95, 0x9a, 0xca, + 0x74, 0xa4, 0xec, 0x71, 0xa6, 0x24, 0x94, 0xaa, 0x8b, 0xc1, 0xeb, 0xe8, 0x5f, 0x76, 0x01, 0xf6, + 0x63, 0xe0, 0x58, 0xef, 0xf7, 0x03, 0xee, 0xda, 0x74, 0xa4, 0x1c, 0x24, 0x72, 0xc7, 0x7b, 0x96, + 0x22, 0x22, 0x69, 0x33, 0x5a, 0x48, 0x49, 0x5c, 0x02, 0x95, 0x78, 0xaa, 0x8b, 0xc8, 0xbf, 0x09, + 0xe0, 0x71, 0x8b, 0xe1, 0x33, 0xdf, 0xec, 0x11, 0xaf, 0x45, 0x98, 0x89, 0x3a, 0xc6, 0x80, 0x50, + 0xdf, 0xfd, 0x9b, 0xdc, 0x5f, 0x81, 0x52, 0x6f, 0x89, 0x62, 0xe5, 0xc0, 0x46, 0x90, 0x19, 0xc6, + 0x56, 0x01, 0xfb, 0x89, 0x7d, 0xce, 0x9d, 0x34, 0xbe, 0xe6, 0x41, 0xbe, 0xc5, 0xb0, 0xf8, 0x11, + 0x94, 0x22, 0x1f, 0x9c, 0x27, 0xda, 0xed, 0x6f, 0x9d, 0x16, 0x3b, 0xb3, 0xd2, 0xb3, 0x0c, 0xa0, + 0xb9, 0xd2, 0x4c, 0x21, 0x72, 0xa8, 0xd3, 0x14, 0x96, 0x41, 0xa9, 0x0a, 0x49, 0x07, 0x51, 0xb4, + 0xc0, 0x83, 0xe8, 0x44, 0x1d, 0xa4, 0xee, 0x5e, 0x42, 0x49, 0x87, 0x59, 0x50, 0x0b, 0x11, 0x17, + 0x88, 0x09, 0xb1, 0x3f, 0x4d, 0xe1, 0xb8, 0x0d, 0x95, 0xea, 0x99, 0xa1, 0x73, 0xcd, 0xe6, 0xe9, + 0xf7, 0xb1, 0x2c, 0x5c, 0x8f, 0x65, 0xe1, 0x66, 0x2c, 0x0b, 0x9f, 0x27, 0x72, 0xee, 0x7a, 0x22, + 0xe7, 0x7e, 0x4e, 0xe4, 0xdc, 0x87, 0x97, 0x98, 0x78, 0x1d, 0xdf, 0xd4, 0x2c, 0xda, 0x83, 0x16, + 0x65, 0x3d, 0xca, 0xc2, 0x9f, 0x23, 0x66, 0x5f, 0xc0, 0x4b, 0xb8, 0xb8, 0x6b, 0x9e, 0x37, 0x8e, + 0xc2, 0xeb, 0xc6, 0x1b, 0xf6, 0x11, 0x33, 0x0b, 0xc1, 0x58, 0xbd, 0xf8, 0x1d, 0x00, 0x00, 0xff, + 0xff, 0xf4, 0xf1, 0xa7, 0x9a, 0xf0, 0x06, 0x00, 0x00, } // Reference imports to suppress errors if they are not otherwise used. @@ -636,7 +634,7 @@ func (m *MsgCreateClient) MarshalToSizedBuffer(dAtA []byte) (int, error) { copy(dAtA[i:], m.Signer) i = encodeVarintTx(dAtA, i, uint64(len(m.Signer))) i-- - dAtA[i] = 0x22 + dAtA[i] = 0x1a } if m.ConsensusState != nil { { @@ -648,7 +646,7 @@ func (m *MsgCreateClient) MarshalToSizedBuffer(dAtA []byte) (int, error) { i = encodeVarintTx(dAtA, i, uint64(size)) } i-- - dAtA[i] = 0x1a + dAtA[i] = 0x12 } if m.ClientState != nil { { @@ -660,13 +658,6 @@ func (m *MsgCreateClient) MarshalToSizedBuffer(dAtA []byte) (int, error) { i = encodeVarintTx(dAtA, i, uint64(size)) } i-- - dAtA[i] = 0x12 - } - if len(m.ClientId) > 0 { - i -= len(m.ClientId) - copy(dAtA[i:], m.ClientId) - i = encodeVarintTx(dAtA, i, uint64(len(m.ClientId))) - i-- dAtA[i] = 0xa } return len(dAtA) - i, nil @@ -954,10 +945,6 @@ func (m *MsgCreateClient) Size() (n int) { } var l int _ = l - l = len(m.ClientId) - if l > 0 { - n += 1 + l + sovTx(uint64(l)) - } if m.ClientState != nil { l = m.ClientState.Size() n += 1 + l + sovTx(uint64(l)) @@ -1120,38 +1107,6 @@ func (m *MsgCreateClient) Unmarshal(dAtA []byte) error { } switch fieldNum { case 1: - if wireType != 2 { - return fmt.Errorf("proto: wrong wireType = %d for field ClientId", wireType) - } - var stringLen uint64 - for shift := uint(0); ; shift += 7 { - if shift >= 64 { - return ErrIntOverflowTx - } - if iNdEx >= l { - return io.ErrUnexpectedEOF - } - b := dAtA[iNdEx] - iNdEx++ - stringLen |= uint64(b&0x7F) << shift - if b < 0x80 { - break - } - } - intStringLen := int(stringLen) - if intStringLen < 0 { - return ErrInvalidLengthTx - } - postIndex := iNdEx + intStringLen - if postIndex < 0 { - return ErrInvalidLengthTx - } - if postIndex > l { - return io.ErrUnexpectedEOF - } - m.ClientId = string(dAtA[iNdEx:postIndex]) - iNdEx = postIndex - case 2: if wireType != 2 { return fmt.Errorf("proto: wrong wireType = %d for field ClientState", wireType) } @@ -1187,7 +1142,7 @@ func (m *MsgCreateClient) Unmarshal(dAtA []byte) error { return err } iNdEx = postIndex - case 3: + case 2: if wireType != 2 { return fmt.Errorf("proto: wrong wireType = %d for field ConsensusState", wireType) } @@ -1223,7 +1178,7 @@ func (m *MsgCreateClient) Unmarshal(dAtA []byte) error { return err } iNdEx = postIndex - case 4: + case 3: if wireType != 2 { return fmt.Errorf("proto: wrong wireType = %d for field Signer", wireType) } diff --git a/x/ibc/core/03-connection/keeper/handshake_test.go b/x/ibc/core/03-connection/keeper/handshake_test.go index eb9ca5c401b..d70fd013dcc 100644 --- a/x/ibc/core/03-connection/keeper/handshake_test.go +++ b/x/ibc/core/03-connection/keeper/handshake_test.go @@ -41,8 +41,9 @@ func (suite *KeeperTestSuite) TestConnOpenInit() { version = &types.Version{} }, false}, {"couldn't add connection to client", func() { - // swap client identifiers to result in client that does not exist - clientB, clientA = suite.coordinator.SetupClients(suite.chainA, suite.chainB, exported.Tendermint) + clientA, clientB = suite.coordinator.SetupClients(suite.chainA, suite.chainB, exported.Tendermint) + // set clientA to invalid client identifier + clientA = "clientidentifier" }, false}, } diff --git a/x/ibc/core/03-connection/types/keys.go b/x/ibc/core/03-connection/types/keys.go index c44602203e5..65af565c2ab 100644 --- a/x/ibc/core/03-connection/types/keys.go +++ b/x/ibc/core/03-connection/types/keys.go @@ -2,10 +2,10 @@ package types import ( "fmt" - "strconv" - "strings" + "regexp" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + host "github.com/cosmos/cosmos-sdk/x/ibc/core/24-host" ) const ( @@ -30,11 +30,17 @@ const ( ) // FormatConnectionIdentifier returns the connection identifier with the sequence appended. +// This is a SDK specific format not enforced by IBC protocol. func FormatConnectionIdentifier(sequence uint64) string { return fmt.Sprintf("%s%d", ConnectionPrefix, sequence) } -// IsValidConnectionID return true if the connection identifier is valid. +// IsConnectionIDFormat checks if a connectionID is in the format required on the SDK for +// parsing connection identifiers. The connection identifier must be in the form: `connection-{N} +var IsConnectionIDFormat = regexp.MustCompile(`^connection-[0-9]{1,20}$`).MatchString + +// IsValidConnectionID checks if the connection identifier is valid and can be parsed to +// the connection identifier format. func IsValidConnectionID(connectionID string) bool { _, err := ParseConnectionSequence(connectionID) return err == nil @@ -42,18 +48,14 @@ func IsValidConnectionID(connectionID string) bool { // ParseConnectionSequence parses the connection sequence from the connection identifier. func ParseConnectionSequence(connectionID string) (uint64, error) { - if !strings.HasPrefix(connectionID, ConnectionPrefix) { - return 0, sdkerrors.Wrapf(ErrInvalidConnectionIdentifier, "doesn't contain prefix `%s`", ConnectionPrefix) - } - - splitStr := strings.Split(connectionID, ConnectionPrefix) - if len(splitStr) != 2 { - return 0, sdkerrors.Wrap(ErrInvalidConnectionIdentifier, "connection identifier must be in format: `connection-{N}`") + if !IsConnectionIDFormat(connectionID) { + return 0, sdkerrors.Wrap(host.ErrInvalidID, "connection identifier is not in the format: `connection-{N}`") } - sequence, err := strconv.ParseUint(splitStr[1], 10, 64) + sequence, err := host.ParseIdentifier(connectionID, ConnectionPrefix) if err != nil { - return 0, sdkerrors.Wrap(err, "failed to parse connection identifier sequence") + return 0, sdkerrors.Wrap(err, "invalid connection identifier") } + return sequence, nil } diff --git a/x/ibc/core/03-connection/types/keys_test.go b/x/ibc/core/03-connection/types/keys_test.go index 261bd2895fb..c899dc3c53a 100644 --- a/x/ibc/core/03-connection/types/keys_test.go +++ b/x/ibc/core/03-connection/types/keys_test.go @@ -1,6 +1,7 @@ package types_test import ( + "math" "testing" "github.com/cosmos/cosmos-sdk/x/ibc/core/03-connection/types" @@ -17,10 +18,13 @@ func TestParseConnectionSequence(t *testing.T) { }{ {"valid 0", "connection-0", 0, true}, {"valid 1", "connection-1", 1, true}, - {"valid large sequence", "connection-234568219356718293", 234568219356718293, true}, + {"valid large sequence", types.FormatConnectionIdentifier(math.MaxUint64), math.MaxUint64, true}, + // one above uint64 max + {"invalid uint64", "connection-18446744073709551616", 0, false}, // uint64 == 20 characters {"invalid large sequence", "connection-2345682193567182931243", 0, false}, {"capital prefix", "Connection-0", 0, false}, + {"double prefix", "connection-connection-0", 0, false}, {"missing dash", "connection0", 0, false}, {"blank id", " ", 0, false}, {"empty id", "", 0, false}, diff --git a/x/ibc/core/04-channel/types/keys.go b/x/ibc/core/04-channel/types/keys.go index b6c5745dda1..d3a6cde24d5 100644 --- a/x/ibc/core/04-channel/types/keys.go +++ b/x/ibc/core/04-channel/types/keys.go @@ -2,10 +2,10 @@ package types import ( "fmt" - "strconv" - "strings" + "regexp" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + host "github.com/cosmos/cosmos-sdk/x/ibc/core/24-host" ) const ( @@ -30,11 +30,17 @@ const ( ) // FormatChannelIdentifier returns the channel identifier with the sequence appended. +// This is a SDK specific format not enforced by IBC protocol. func FormatChannelIdentifier(sequence uint64) string { return fmt.Sprintf("%s%d", ChannelPrefix, sequence) } -// IsValidChannelID return true if the channel identifier is valid. +// IsChannelIDFormat checks if a channelID is in the format required on the SDK for +// parsing channel identifiers. The channel identifier must be in the form: `channel-{N} +var IsChannelIDFormat = regexp.MustCompile(`^channel-[0-9]{1,20}$`).MatchString + +// IsValidChannelID checks if a channelID is valid and can be parsed to the channel +// identifier format. func IsValidChannelID(channelID string) bool { _, err := ParseChannelSequence(channelID) return err == nil @@ -42,18 +48,14 @@ func IsValidChannelID(channelID string) bool { // ParseChannelSequence parses the channel sequence from the channel identifier. func ParseChannelSequence(channelID string) (uint64, error) { - if !strings.HasPrefix(channelID, ChannelPrefix) { - return 0, sdkerrors.Wrapf(ErrInvalidChannelIdentifier, "doesn't contain prefix `%s`", ChannelPrefix) - } - - splitStr := strings.Split(channelID, ChannelPrefix) - if len(splitStr) != 2 { - return 0, sdkerrors.Wrap(ErrInvalidChannelIdentifier, "channel identifier must be in format: `channel-{N}`") + if !IsChannelIDFormat(channelID) { + return 0, sdkerrors.Wrap(host.ErrInvalidID, "channel identifier is not in the format: `channel-{N}`") } - sequence, err := strconv.ParseUint(splitStr[1], 10, 64) + sequence, err := host.ParseIdentifier(channelID, ChannelPrefix) if err != nil { - return 0, sdkerrors.Wrap(err, "failed to parse channel identifier sequence") + return 0, sdkerrors.Wrap(err, "invalid channel identifier") } + return sequence, nil } diff --git a/x/ibc/core/04-channel/types/keys_test.go b/x/ibc/core/04-channel/types/keys_test.go index 418174cb040..86e4e61aa2c 100644 --- a/x/ibc/core/04-channel/types/keys_test.go +++ b/x/ibc/core/04-channel/types/keys_test.go @@ -18,6 +18,8 @@ func TestParseChannelSequence(t *testing.T) { {"valid 0", "channel-0", 0, true}, {"valid 1", "channel-1", 1, true}, {"valid large sequence", "channel-234568219356718293", 234568219356718293, true}, + // one above uint64 max + {"invalid uint64", "channel-18446744073709551616", 0, false}, // uint64 == 20 characters {"invalid large sequence", "channel-2345682193567182931243", 0, false}, {"capital prefix", "Channel-0", 0, false}, diff --git a/x/ibc/core/24-host/parse.go b/x/ibc/core/24-host/parse.go index 7e6301a391f..8c3459500d9 100644 --- a/x/ibc/core/24-host/parse.go +++ b/x/ibc/core/24-host/parse.go @@ -1,11 +1,37 @@ package host import ( + "strconv" "strings" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) +// ParseIdentifier parses the sequence from the identifier using the provided prefix. This function +// does not need to be used by counterparty chains. SDK generated connection and channel identifiers +// are required to use this format. +func ParseIdentifier(identifier, prefix string) (uint64, error) { + if !strings.HasPrefix(identifier, prefix) { + return 0, sdkerrors.Wrapf(ErrInvalidID, "identifier doesn't contain prefix `%s`", prefix) + } + + splitStr := strings.Split(identifier, prefix) + if len(splitStr) != 2 { + return 0, sdkerrors.Wrapf(ErrInvalidID, "identifier must be in format: `%s{N}`", prefix) + } + + // sanity check + if splitStr[0] != "" { + return 0, sdkerrors.Wrapf(ErrInvalidID, "identifier must begin with prefix %s", prefix) + } + + sequence, err := strconv.ParseUint(splitStr[1], 10, 64) + if err != nil { + return 0, sdkerrors.Wrap(err, "failed to parse identifier sequence") + } + return sequence, nil +} + // ParseConnectionPath returns the connection ID from a full path. It returns // an error if the provided path is invalid. func ParseConnectionPath(path string) (string, error) { diff --git a/x/ibc/core/24-host/parse_test.go b/x/ibc/core/24-host/parse_test.go new file mode 100644 index 00000000000..cbee37ddbcb --- /dev/null +++ b/x/ibc/core/24-host/parse_test.go @@ -0,0 +1,47 @@ +package host_test + +import ( + "math" + "testing" + + connectiontypes "github.com/cosmos/cosmos-sdk/x/ibc/core/03-connection/types" + host "github.com/cosmos/cosmos-sdk/x/ibc/core/24-host" + "github.com/stretchr/testify/require" +) + +func TestParseIdentifier(t *testing.T) { + testCases := []struct { + name string + identifier string + prefix string + expSeq uint64 + expPass bool + }{ + {"valid 0", "connection-0", "connection-", 0, true}, + {"valid 1", "connection-1", "connection-", 1, true}, + {"valid large sequence", connectiontypes.FormatConnectionIdentifier(math.MaxUint64), "connection-", math.MaxUint64, true}, + // one above uint64 max + {"invalid uint64", "connection-18446744073709551616", "connection-", 0, false}, + // uint64 == 20 characters + {"invalid large sequence", "connection-2345682193567182931243", "conenction-", 0, false}, + {"capital prefix", "Connection-0", "connection-", 0, false}, + {"double prefix", "connection-connection-0", "connection-", 0, false}, + {"doesn't have prefix", "connection-0", "prefix", 0, false}, + {"missing dash", "connection0", "connection-", 0, false}, + {"blank id", " ", "connection-", 0, false}, + {"empty id", "", "connection-", 0, false}, + {"negative sequence", "connection--1", "connection-", 0, false}, + } + + for _, tc := range testCases { + + seq, err := host.ParseIdentifier(tc.identifier, tc.prefix) + require.Equal(t, tc.expSeq, seq) + + if tc.expPass { + require.NoError(t, err, tc.name) + } else { + require.Error(t, err, tc.name) + } + } +} diff --git a/x/ibc/core/genesis_test.go b/x/ibc/core/genesis_test.go index 29a26aad740..bc4c5834d3e 100644 --- a/x/ibc/core/genesis_test.go +++ b/x/ibc/core/genesis_test.go @@ -97,6 +97,7 @@ func (suite *IBCTestSuite) TestValidateGenesis() { }, clienttypes.NewParams(exported.Tendermint, exported.Localhost), true, + 0, ), ConnectionGenesis: connectiontypes.NewGenesisState( []connectiontypes.IdentifiedConnection{ @@ -154,6 +155,7 @@ func (suite *IBCTestSuite) TestValidateGenesis() { nil, clienttypes.NewParams(exported.Tendermint), false, + 0, ), ConnectionGenesis: connectiontypes.DefaultGenesisState(), }, @@ -239,6 +241,7 @@ func (suite *IBCTestSuite) TestInitGenesis() { }, clienttypes.NewParams(exported.Tendermint, exported.Localhost), true, + 0, ), ConnectionGenesis: connectiontypes.NewGenesisState( []connectiontypes.IdentifiedConnection{ diff --git a/x/ibc/core/keeper/msg_server.go b/x/ibc/core/keeper/msg_server.go index 2b655ff9a2f..78f822e4d5a 100644 --- a/x/ibc/core/keeper/msg_server.go +++ b/x/ibc/core/keeper/msg_server.go @@ -32,14 +32,15 @@ func (k Keeper) CreateClient(goCtx context.Context, msg *clienttypes.MsgCreateCl return nil, err } - if err = k.ClientKeeper.CreateClient(ctx, msg.ClientId, clientState, consensusState); err != nil { + clientID, err := k.ClientKeeper.CreateClient(ctx, clientState, consensusState) + if err != nil { return nil, err } ctx.EventManager().EmitEvents(sdk.Events{ sdk.NewEvent( clienttypes.EventTypeCreateClient, - sdk.NewAttribute(clienttypes.AttributeKeyClientID, msg.ClientId), + sdk.NewAttribute(clienttypes.AttributeKeyClientID, clientID), sdk.NewAttribute(clienttypes.AttributeKeyClientType, clientState.ClientType()), sdk.NewAttribute(clienttypes.AttributeKeyConsensusHeight, clientState.GetLatestHeight().String()), ), diff --git a/x/ibc/core/spec/01_concepts.md b/x/ibc/core/spec/01_concepts.md index c3059e5f587..045508999d3 100644 --- a/x/ibc/core/spec/01_concepts.md +++ b/x/ibc/core/spec/01_concepts.md @@ -10,11 +10,11 @@ this [document](https://github.com/cosmos/ics/blob/master/ibc/1_IBC_TERMINOLOGY. ## Client Creation, Updates, and Upgrades IBC clients are on chain light clients. The light client is responsible for verifying -counterparty state. A light client can be created by any user submitting a client -identifier and a valid initial `ClientState` and `ConsensusState`. The client identifier -must not already be used. Clients are given a client identifier prefixed store to -store their associated client state and consensus states. Consensus states are -stored using their associated height. +counterparty state. A light client can be created by any user submitting a valid initial +`ClientState` and `ConsensusState`. The client identifier is auto generated using the +client type and the global client counter appended in the format: `{client-type}-{N}`. +Clients are given a client identifier prefixed store to store their associated client +state and consensus states. Consensus states are stored using their associated height. Clients can be updated by any user submitting a valid `Header`. The client state callback to `CheckHeaderAndUpdateState` is responsible for verifying the header against previously diff --git a/x/ibc/core/spec/02_state.md b/x/ibc/core/spec/02_state.md index 54eb41bc4bc..2c85a525a95 100644 --- a/x/ibc/core/spec/02_state.md +++ b/x/ibc/core/spec/02_state.md @@ -13,9 +13,12 @@ The client type is not stored since it can be obtained through the client state. | "0/" | "clients/{identifier}/clientState" | ClientState | | "0/" | "clients/{identifier}/consensusStates/{height}" | ConsensusState | | "0/" | "clients/{identifier}/connections" | []string | +| "0/" | "nextClientSequence | uint64 | | "0/" | "connections/{identifier}" | ConnectionEnd | +| "0/" | "nextConnectionSequence" | uint64 | | "0/" | "ports/{identifier}" | CapabilityKey | | "0/" | "channelEnds/ports/{identifier}/channels/{identifier}" | ChannelEnd | +| "0/" | "nextChannelSequence" | uint64 | | "0/" | "capabilities/ports/{identifier}/channels/{identifier}" | CapabilityKey | | "0/" | "nextSequenceSend/ports/{identifier}/channels/{identifier}" | uint64 | | "0/" | "nextSequenceRecv/ports/{identifier}/channels/{identifier}" | uint64 | diff --git a/x/ibc/core/spec/03_state_transitions.md b/x/ibc/core/spec/03_state_transitions.md index de31957d66e..be3b508b795 100644 --- a/x/ibc/core/spec/03_state_transitions.md +++ b/x/ibc/core/spec/03_state_transitions.md @@ -9,7 +9,7 @@ The described state transitions assume successful message exection. ## Create Client `MsgCreateClient` will initialize and store a `ClientState` and `ConsensusState` in the sub-store -created using the given client identifier. +created using a generated client identifier. ## Update Client diff --git a/x/ibc/core/spec/04_messages.md b/x/ibc/core/spec/04_messages.md index 34d68200b28..3728e6d6f32 100644 --- a/x/ibc/core/spec/04_messages.md +++ b/x/ibc/core/spec/04_messages.md @@ -14,7 +14,6 @@ A light client is created using the `MsgCreateClient`. ```go type MsgCreateClient struct { - ClientId string ClientState *types.Any // proto-packed client state ConsensusState *types.Any // proto-packed consensus state Signer sdk.AccAddress @@ -23,13 +22,11 @@ type MsgCreateClient struct { This message is expected to fail if: -- `ClientId` is invalid (see naming requirements) - `ClientState` is empty or invalid - `ConsensusState` is empty or invalid - `Signer` is empty -- A light client with the provided id and type already exist -The message creates and stores a light client with an initial consensus state for the given client +The message creates and stores a light client with an initial consensus state using a generated client identifier. ### MsgUpdateClient @@ -112,7 +109,6 @@ A connection is initialized on a light client using the `MsgConnectionOpenInit`. ```go type MsgConnectionOpenInit struct { ClientId string - ConnectionId string Counterparty Counterparty Version string Signer sdk.AccAddress @@ -121,7 +117,6 @@ type MsgConnectionOpenInit struct { This message is expected to fail if: - `ClientId` is invalid (see naming requirements) -- `ConnectionId` is invalid (see naming requirements) - `Counterparty` is empty - 'Version' is not empty and invalid - `Signer` is empty @@ -138,8 +133,7 @@ using the `MsgConnectionOpenTry`. ```go type MsgConnectionOpenTry struct { ClientId string - DesiredConnectionId string - CounterpartyChosenConnectionId string + PreviousConnectionId string ClientState *types.Any // proto-packed counterparty client Counterparty Counterparty CounterpartyVersions []string @@ -155,8 +149,7 @@ type MsgConnectionOpenTry struct { This message is expected to fail if: - `ClientId` is invalid (see naming requirements) -- `DesiredConnectionId` is invalid (see naming requirements) -- `CounterpartyChosenConnectionId` is not empty and doesn't match `DesiredConnectionId` +- `PreviousConnectionId` is not empty and invalid (see naming requirements) - `ClientState` is not a valid client of the executing chain - `Counterparty` is empty - `CounterpartyVersions` is empty @@ -167,15 +160,13 @@ This message is expected to fail if: - `ConsensusHeight` is zero - `Signer` is empty - A Client hasn't been created for the given ID -- A Connection for the given ID already exists +- If a previous connection exists but does not match the supplied parameters. - `ProofInit` does not prove that the counterparty connection is in state INIT - `ProofClient` does not prove that the counterparty has stored the `ClientState` provided in message - `ProofConsensus` does not prove that the counterparty has the correct consensus state for this chain -The message creates a connection for the given ID with an TRYOPEN State. The `CounterpartyChosenConnectionID` -represents the connection ID the counterparty set under `connection.Counterparty.ConnectionId` -to represent the connection ID this chain should use. An empty string indicates the connection -identifier is flexible and gives this chain an opportunity to choose its own identifier. +The message creates a connection for a generated connection ID with an TRYOPEN State. If a previous +connection already exists, it updates the connection state from INIT to TRYOPEN. ### MsgConnectionOpenAck @@ -251,7 +242,6 @@ message. ```go type MsgChannelOpenInit struct { PortId string - ChannelId string Channel Channel Signer sdk.AccAddress } @@ -260,12 +250,11 @@ type MsgChannelOpenInit struct { This message is expected to fail if: - `PortId` is invalid (see naming requirements) -- `ChannelId` is invalid (see naming requirements) - `Channel` is empty - `Signer` is empty - A Channel End exists for the given Channel ID and Port ID -The message creates a channel on chain A with an INIT state for the given Channel ID +The message creates a channel on chain A with an INIT state for a generated Channel ID and Port ID. ### MsgChannelOpenTry @@ -276,8 +265,7 @@ the `MsgChannelOpenTry` message. ```go type MsgChannelOpenTry struct { PortId string - DesiredChannelId string - CounterpartyChosenChannelId string + PreviousChannelId string Channel Channel CounterpartyVersion string ProofInit []byte @@ -289,21 +277,18 @@ type MsgChannelOpenTry struct { This message is expected to fail if: - `PortId` is invalid (see naming requirements) -- `DesiredChannelId` is invalid (see naming requirements) -- `CounterpartyChosenChannelId` is not empty and not equal to `ChannelId` +- `PreviousChannelId` is not empty and invalid (see naming requirements) - `Channel` is empty - `CounterpartyVersion` is empty - `ProofInit` is empty - `ProofHeight` is zero - `Signer` is empty -- A Channel End exists for the given Channel and Port ID +- A previous channel exists and does not match the provided parameters. - `ProofInit` does not prove that the counterparty's Channel state is in INIT -The message creates a channel on chain B with an TRYOPEN state for the given Channel ID -and Port ID. The `CounterpartyChosenChannelId` represents the channel ID the counterparty set under -`connection.Counterparty.ChannelId` to represent the channel ID this chain should use. -An empty string indicates the channel identifier is flexible and gives this chain an -opportunity to choose its own identifier. +The message creates a channel on chain B with an TRYOPEN state for using a generated Channel ID +and given Port ID if the previous channel does not already exist. Otherwise it udates the +previous channel state from INIT to TRYOPEN. ### MsgChannelOpenAck diff --git a/x/ibc/light-clients/06-solomachine/client/cli/tx.go b/x/ibc/light-clients/06-solomachine/client/cli/tx.go index 37db768a302..8d1709fc279 100644 --- a/x/ibc/light-clients/06-solomachine/client/cli/tx.go +++ b/x/ibc/light-clients/06-solomachine/client/cli/tx.go @@ -24,12 +24,12 @@ const ( // NewCreateClientCmd defines the command to create a new solo machine client. func NewCreateClientCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "create [client-id] [sequence] [path/to/consensus_state.json]", + Use: "create [sequence] [path/to/consensus_state.json]", Short: "create new solo machine client", Long: `create a new solo machine client with the specified identifier and public key - ConsensusState json example: {"public_key":{"@type":"/cosmos.crypto.secp256k1.PubKey","key":"A/3SXL2ONYaOkxpdR5P8tHTlSlPv1AwQwSFxKRee5JQW"},"diversifier":"diversifier","timestamp":"10"}`, - Example: fmt.Sprintf("%s tx ibc %s create [client-id] [sequence] [path/to/consensus_state] --from node0 --home ../node0/cli --chain-id $CID", version.AppName, types.SubModuleName), - Args: cobra.ExactArgs(3), + Example: fmt.Sprintf("%s tx ibc %s create [sequence] [path/to/consensus_state] --from node0 --home ../node0/cli --chain-id $CID", version.AppName, types.SubModuleName), + Args: cobra.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { clientCtx := client.GetClientContextFromCmd(cmd) clientCtx, err := client.ReadTxCommandFlags(clientCtx, cmd.Flags()) @@ -37,9 +37,7 @@ func NewCreateClientCmd() *cobra.Command { return err } - clientID := args[0] - - sequence, err := strconv.ParseUint(args[1], 10, 64) + sequence, err := strconv.ParseUint(args[0], 10, 64) if err != nil { return err } @@ -48,10 +46,10 @@ func NewCreateClientCmd() *cobra.Command { // attempt to unmarshal consensus state argument consensusState := &types.ConsensusState{} - if err := cdc.UnmarshalJSON([]byte(args[2]), consensusState); err != nil { + if err := cdc.UnmarshalJSON([]byte(args[1]), consensusState); err != nil { // check for file path if JSON input is not provided - contents, err := ioutil.ReadFile(args[2]) + contents, err := ioutil.ReadFile(args[1]) if err != nil { return errors.Wrap(err, "neither JSON input nor path to .json file for consensus state were provided") } @@ -64,7 +62,7 @@ func NewCreateClientCmd() *cobra.Command { allowUpdateAfterProposal, _ := cmd.Flags().GetBool(flagAllowUpdateAfterProposal) clientState := types.NewClientState(sequence, consensusState, allowUpdateAfterProposal) - msg, err := clienttypes.NewMsgCreateClient(clientID, clientState, consensusState, clientCtx.GetFromAddress()) + msg, err := clienttypes.NewMsgCreateClient(clientState, consensusState, clientCtx.GetFromAddress()) if err != nil { return err } diff --git a/x/ibc/light-clients/07-tendermint/client/cli/tx.go b/x/ibc/light-clients/07-tendermint/client/cli/tx.go index 3bd2f0c0b73..04a274aceca 100644 --- a/x/ibc/light-clients/07-tendermint/client/cli/tx.go +++ b/x/ibc/light-clients/07-tendermint/client/cli/tx.go @@ -34,15 +34,15 @@ const ( // in https://github.com/cosmos/ics/tree/master/spec/ics-002-client-semantics#create func NewCreateClientCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "create [client-id] [path/to/consensus_state.json] [trusting_period] [unbonding_period] [max_clock_drift]", + Use: "create [path/to/consensus_state.json] [trusting_period] [unbonding_period] [max_clock_drift]", Short: "create new tendermint client", Long: `Create a new tendermint IBC client. - 'trust-level' flag can be a fraction (eg: '1/3') or 'default' - 'proof-specs' flag can be JSON input, a path to a .json file or 'default' - 'upgrade-path' flag is a string specifying the upgrade path for this chain where a future upgraded client will be stored. The path is a comma-separated list representing the keys in order of the keyPath to the committed upgraded client. e.g. 'upgrade/upgradedClient'`, - Example: fmt.Sprintf("%s tx ibc %s create [client-id] [path/to/consensus_state.json] [trusting_period] [unbonding_period] [max_clock_drift] --trust-level default --consensus-params [path/to/consensus-params.json] --proof-specs [path/to/proof-specs.json] --upgrade-path upgrade/upgradedClient --from node0 --home ../node0/cli --chain-id $CID", version.AppName, types.SubModuleName), - Args: cobra.ExactArgs(5), + Example: fmt.Sprintf("%s tx ibc %s create [path/to/consensus_state.json] [trusting_period] [unbonding_period] [max_clock_drift] --trust-level default --consensus-params [path/to/consensus-params.json] --proof-specs [path/to/proof-specs.json] --upgrade-path upgrade/upgradedClient --from node0 --home ../node0/cli --chain-id $CID", version.AppName, types.SubModuleName), + Args: cobra.ExactArgs(4), RunE: func(cmd *cobra.Command, args []string) error { clientCtx := client.GetClientContextFromCmd(cmd) clientCtx, err := client.ReadTxCommandFlags(clientCtx, cmd.Flags()) @@ -50,15 +50,13 @@ func NewCreateClientCmd() *cobra.Command { return err } - clientID := args[0] - cdc := codec.NewProtoCodec(clientCtx.InterfaceRegistry) legacyAmino := codec.NewLegacyAmino() var header *types.Header - if err := cdc.UnmarshalJSON([]byte(args[1]), header); err != nil { + if err := cdc.UnmarshalJSON([]byte(args[0]), header); err != nil { // check for file path if JSON input is not provided - contents, err := ioutil.ReadFile(args[1]) + contents, err := ioutil.ReadFile(args[0]) if err != nil { return errors.New("neither JSON input nor path to .json file were provided for consensus header") } @@ -83,17 +81,17 @@ func NewCreateClientCmd() *cobra.Command { } } - trustingPeriod, err := time.ParseDuration(args[2]) + trustingPeriod, err := time.ParseDuration(args[1]) if err != nil { return err } - ubdPeriod, err := time.ParseDuration(args[3]) + ubdPeriod, err := time.ParseDuration(args[2]) if err != nil { return err } - maxClockDrift, err := time.ParseDuration(args[4]) + maxClockDrift, err := time.ParseDuration(args[3]) if err != nil { return err } @@ -137,7 +135,7 @@ func NewCreateClientCmd() *cobra.Command { consensusState := header.ConsensusState() msg, err := clienttypes.NewMsgCreateClient( - clientID, clientState, consensusState, clientCtx.GetFromAddress(), + clientState, consensusState, clientCtx.GetFromAddress(), ) if err != nil { return err diff --git a/x/ibc/testing/chain.go b/x/ibc/testing/chain.go index b63e72c3e9d..4dc8ec4f25e 100644 --- a/x/ibc/testing/chain.go +++ b/x/ibc/testing/chain.go @@ -369,8 +369,8 @@ func (chain *TestChain) GetPrefix() commitmenttypes.MerklePrefix { // NewClientID appends a new clientID string in the format: // ClientFor -func (chain *TestChain) NewClientID(counterpartyChainID string) string { - clientID := "client" + strconv.Itoa(len(chain.ClientIDs)) + "For" + counterpartyChainID +func (chain *TestChain) NewClientID(clientType string) string { + clientID := fmt.Sprintf("%s-%s", clientType, strconv.Itoa(len(chain.ClientIDs))) chain.ClientIDs = append(chain.ClientIDs, clientID) return clientID } @@ -460,7 +460,7 @@ func (chain *TestChain) ConstructMsgCreateClient(counterparty *TestChain, client } msg, err := clienttypes.NewMsgCreateClient( - clientID, clientState, consensusState, chain.SenderAccount.GetAddress(), + clientState, consensusState, chain.SenderAccount.GetAddress(), ) require.NoError(chain.t, err) return msg diff --git a/x/ibc/testing/chain_test.go b/x/ibc/testing/chain_test.go index bf5af6de636..361a9c4c15a 100644 --- a/x/ibc/testing/chain_test.go +++ b/x/ibc/testing/chain_test.go @@ -33,6 +33,7 @@ func TestCreateSortedSignerArray(t *testing.T) { // smaller address validator1.Address = []byte{1} + validator2.Address = []byte{2} validator2.VotingPower = 1 expected = []tmtypes.PrivValidator{privVal1, privVal2} diff --git a/x/ibc/testing/coordinator.go b/x/ibc/testing/coordinator.go index aecdcd4b704..95b59a1db9b 100644 --- a/x/ibc/testing/coordinator.go +++ b/x/ibc/testing/coordinator.go @@ -95,7 +95,7 @@ func (coord *Coordinator) CreateClient( ) (clientID string, err error) { coord.CommitBlock(source, counterparty) - clientID = source.NewClientID(counterparty.ChainID) + clientID = source.NewClientID(clientType) switch clientType { case exported.Tendermint: