feat: create channel before invoking provide counterparty. (#7420)

cosmos · Oct 9, 2024 · 8e9b8cc · 8e9b8cc
1 parent a42ed56
commit 8e9b8cc
Show file tree

Hide file tree

Showing 16 changed files with 109 additions and 117 deletions.
diff --git a/modules/core/02-client/keeper/keeper.go b/modules/core/02-client/keeper/keeper.go
@@ -313,29 +313,6 @@ func (k *Keeper) GetLatestClientConsensusState(ctx context.Context, clientID str
 	return k.GetClientConsensusState(ctx, clientID, clientModule.LatestHeight(ctx, clientID))
 }
 
-// GetCreator returns the creator of the client.
-func (k *Keeper) GetCreator(ctx context.Context, clientID string) (string, bool) {
-	sdkCtx := sdk.UnwrapSDKContext(ctx) // TODO: https://github.com/cosmos/ibc-go/issues/5917
-	bz := k.ClientStore(sdkCtx, clientID).Get([]byte(types.CreatorKey))
-	if len(bz) == 0 {
-		return "", false
-	}
-
-	return string(bz), true
-}
-
-// SetCreator sets the creator of the client.
-func (k *Keeper) SetCreator(ctx context.Context, clientID, creator string) {
-	sdkCtx := sdk.UnwrapSDKContext(ctx) // TODO: https://github.com/cosmos/ibc-go/issues/5917
-	k.ClientStore(sdkCtx, clientID).Set([]byte(types.CreatorKey), []byte(creator))
-}
-
-// DeleteCreator deletes the creator associated with the client.
-func (k *Keeper) DeleteCreator(ctx context.Context, clientID string) {
-	sdkCtx := sdk.UnwrapSDKContext(ctx) // TODO: https://github.com/cosmos/ibc-go/issues/5917
-	k.ClientStore(sdkCtx, clientID).Delete([]byte(types.CreatorKey))
-}
-
 // VerifyMembership retrieves the light client module for the clientID and verifies the proof of the existence of a key-value pair at a specified height.
 func (k *Keeper) VerifyMembership(ctx context.Context, clientID string, height exported.Height, delayTimePeriod uint64, delayBlockPeriod uint64, proof []byte, path exported.Path, value []byte) error {
 	clientModule, err := k.Route(ctx, clientID)

diff --git a/modules/core/02-client/keeper/keeper_test.go b/modules/core/02-client/keeper/keeper_test.go
@@ -129,32 +129,6 @@ func (suite *KeeperTestSuite) TestSetClientState() {
 	suite.Require().Equal(clientState, retrievedState, "Client states are not equal")
 }
 
-func (suite *KeeperTestSuite) TestSetCreator() {
-	clientID := ibctesting.FirstClientID
-	expectedCreator := "test-creator"
-
-	// Set the creator for the client
-	suite.keeper.SetCreator(suite.ctx, clientID, expectedCreator)
-
-	// Retrieve the creator from the store
-	retrievedCreator, found := suite.keeper.GetCreator(suite.ctx, clientID)
-
-	// Verify that the retrieved creator matches the expected creator
-	suite.Require().True(found, "GetCreator did not return stored creator")
-	suite.Require().Equal(expectedCreator, retrievedCreator, "Creator is not retrieved correctly")
-
-	// Verify non stored creator is not found
-	retrievedCreator, found = suite.keeper.GetCreator(suite.ctx, ibctesting.SecondClientID)
-	suite.Require().False(found, "GetCreator unexpectedly returned a creator")
-	suite.Require().Empty(retrievedCreator, "Creator is not empty")
-
-	// Verify that the creator is deleted from the store
-	suite.keeper.DeleteCreator(suite.ctx, clientID)
-	retrievedCreator, found = suite.keeper.GetCreator(suite.ctx, clientID)
-	suite.Require().False(found, "GetCreator unexpectedly returned a creator")
-	suite.Require().Empty(retrievedCreator, "Creator is not empty")
-}
-
 func (suite *KeeperTestSuite) TestSetClientConsensusState() {
 	suite.keeper.SetClientConsensusState(suite.ctx, testClientID, testClientHeight, suite.consensusState)
 

diff --git a/modules/core/02-client/types/keys.go b/modules/core/02-client/types/keys.go
@@ -33,11 +33,6 @@ const (
 	// would allow any wired up light client modules to be allowed
 	AllowAllClients = "*"
 
-	// CreatorKey is the key used to store the client creator in the client store
-	// the creator key is imported from types instead of host because
-	// the creator key is not a part of the ics-24 host specification
-	CreatorKey = "creator"
-
 	// CounterpartyKey is the key used to store counterparty in the client store.
 	// the counterparty key is imported from types instead of host because
 	// the counterparty key is not a part of the ics-24 host specification

diff --git a/modules/core/04-channel/v2/types/expected_keepers.go b/modules/core/04-channel/v2/types/expected_keepers.go
@@ -19,7 +19,4 @@ type ClientKeeper interface {
 	// GetClientTimestampAtHeight returns the timestamp for a given height on the client
 	// given its client ID and height
 	GetClientTimestampAtHeight(ctx context.Context, clientID string, height exported.Height) (uint64, error)
-
-	// GetCreator returns the creator of the client denoted by the clientID.
-	GetCreator(ctx context.Context, clientID string) (string, bool)
 }
diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go
@@ -50,9 +50,7 @@ func (k *Keeper) CreateClient(goCtx context.Context, msg *clienttypes.MsgCreateC
 		return nil, err
 	}
 
-	k.ClientKeeper.SetCreator(ctx, clientID, msg.Signer)
-
-	return &clienttypes.MsgCreateClientResponse{}, nil
+	return &clienttypes.MsgCreateClientResponse{ClientId: clientID}, nil
 }
 
 // UpdateClient defines a rpc handler method for MsgUpdateClient.
@@ -154,7 +152,7 @@ func (k *Keeper) CreateChannel(goCtx context.Context, msg *packetservertypes.Msg
 	counterparty := packetservertypes.NewCounterparty(msg.ClientId, "", msg.MerklePathPrefix)
 	k.PacketServerKeeper.SetCounterparty(ctx, channelID, counterparty)
 
-	k.ClientKeeper.SetCreator(ctx, channelID, msg.Signer)
+	k.PacketServerKeeper.SetCreator(ctx, channelID, msg.Signer)
 
 	packetserverkeeper.EmitCreateChannelEvent(goCtx, channelID)
 
@@ -165,22 +163,24 @@ func (k *Keeper) CreateChannel(goCtx context.Context, msg *packetservertypes.Msg
 func (k *Keeper) ProvideCounterparty(goCtx context.Context, msg *packetservertypes.MsgProvideCounterparty) (*packetservertypes.MsgProvideCounterpartyResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
-	creator, found := k.ClientKeeper.GetCreator(ctx, msg.Counterparty.ClientId)
+	creator, found := k.PacketServerKeeper.GetCreator(ctx, msg.ChannelId)
 	if !found {
-		return nil, errorsmod.Wrap(ibcerrors.ErrUnauthorized, "client creator must be set")
+		return nil, errorsmod.Wrap(ibcerrors.ErrUnauthorized, "channel creator must be set")
 	}
 
 	if creator != msg.Signer {
-		return nil, errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "client creator (%s) must match signer (%s)", creator, msg.Signer)
+		return nil, errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "channel creator (%s) must match signer (%s)", creator, msg.Signer)
 	}
 
-	if _, ok := k.PacketServerKeeper.GetCounterparty(ctx, msg.ChannelId); ok {
-		return nil, errorsmod.Wrapf(packetservertypes.ErrInvalidCounterparty, "counterparty already exists for client %s", msg.ChannelId)
+	counterparty, ok := k.PacketServerKeeper.GetCounterparty(ctx, msg.ChannelId)
+	if !ok {
+		return nil, errorsmod.Wrapf(packetservertypes.ErrInvalidCounterparty, "counterparty must exist for channel %s", msg.ChannelId)
 	}
 
-	k.PacketServerKeeper.SetCounterparty(ctx, msg.ChannelId, msg.Counterparty)
+	counterparty.CounterpartyChannelId = msg.Counterparty.CounterpartyChannelId
+	k.PacketServerKeeper.SetCounterparty(ctx, msg.ChannelId, counterparty)
 	// Delete client creator from state as it is not needed after this point.
-	k.ClientKeeper.DeleteCreator(ctx, msg.Counterparty.ClientId)
+	k.PacketServerKeeper.DeleteCreator(ctx, msg.ChannelId)
 
 	return &packetservertypes.MsgProvideCounterpartyResponse{}, nil
 }

diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go
@@ -241,7 +241,7 @@ func (suite *KeeperTestSuite) TestRecvPacketV2() {
 				sequence, err := path.EndpointA.SendPacketV2(timeoutHeight, 0, ibcmock.Version, ibctesting.MockPacketData)
 				suite.Require().NoError(err)
 
-				packet = channeltypes.NewPacketWithVersion(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ClientID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ClientID, timeoutHeight, 0, ibcmock.Version)
+				packet = channeltypes.NewPacketWithVersion(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0, ibcmock.Version)
 			},
 			nil,
 			false,
@@ -256,7 +256,7 @@ func (suite *KeeperTestSuite) TestRecvPacketV2() {
 				sequence, err := path.EndpointA.SendPacketV2(timeoutHeight, 0, ibcmock.Version, ibctesting.MockFailPacketData)
 				suite.Require().NoError(err)
 
-				packet = channeltypes.NewPacketWithVersion(ibctesting.MockFailPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ClientID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ClientID, timeoutHeight, 0, ibcmock.Version)
+				packet = channeltypes.NewPacketWithVersion(ibctesting.MockFailPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0, ibcmock.Version)
 			},
 			nil,
 			true,
@@ -271,7 +271,7 @@ func (suite *KeeperTestSuite) TestRecvPacketV2() {
 				sequence, err := path.EndpointA.SendPacketV2(timeoutHeight, 0, ibcmock.Version, ibcmock.MockAsyncPacketData)
 				suite.Require().NoError(err)
 
-				packet = channeltypes.NewPacketWithVersion(ibcmock.MockAsyncPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ClientID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ClientID, timeoutHeight, 0, ibcmock.Version)
+				packet = channeltypes.NewPacketWithVersion(ibcmock.MockAsyncPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0, ibcmock.Version)
 			},
 			nil,
 			false,
@@ -287,7 +287,7 @@ func (suite *KeeperTestSuite) TestRecvPacketV2() {
 				sequence, err := path.EndpointA.SendPacketV2(timeoutHeight, 0, ibcmock.Version, ibctesting.MockPacketData)
 				suite.Require().NoError(err)
 
-				packet = channeltypes.NewPacketWithVersion(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ClientID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ClientID, timeoutHeight, 0, ibcmock.Version)
+				packet = channeltypes.NewPacketWithVersion(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0, ibcmock.Version)
 				err = path.EndpointB.RecvPacket(packet)
 				suite.Require().NoError(err)
 			},
@@ -311,7 +311,7 @@ func (suite *KeeperTestSuite) TestRecvPacketV2() {
 			"packet not sent",
 			func() {
 				path.SetupV2()
-				packet = channeltypes.NewPacketWithVersion(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ClientID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ClientID, timeoutHeight, 0, ibcmock.Version)
+				packet = channeltypes.NewPacketWithVersion(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0, ibcmock.Version)
 			},
 			fmt.Errorf("receive packet verification failed"),
 			false,
@@ -696,7 +696,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacketV2() {
 			sequence, err := path.EndpointA.SendPacketV2(timeoutHeight, 0, ibcmock.Version, ibctesting.MockPacketData)
 			suite.Require().NoError(err)
 
-			packet = channeltypes.NewPacketWithVersion(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ClientID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ClientID, timeoutHeight, 0, ibcmock.Version)
+			packet = channeltypes.NewPacketWithVersion(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0, ibcmock.Version)
 			err = path.EndpointB.RecvPacket(packet)
 			suite.Require().NoError(err)
 
@@ -936,7 +936,7 @@ func (suite *KeeperTestSuite) TestTimeoutPacketV2() {
 				err = path.EndpointA.UpdateClient()
 				suite.Require().NoError(err)
 
-				packet = channeltypes.NewPacketWithVersion(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ClientID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ClientID, timeoutHeight, timeoutTimestamp, ibcmock.Version)
+				packet = channeltypes.NewPacketWithVersion(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, timeoutTimestamp, ibcmock.Version)
 				packetKey = host.PacketReceiptKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
 			},
 			nil,
@@ -957,7 +957,7 @@ func (suite *KeeperTestSuite) TestTimeoutPacketV2() {
 					sequence, err := path.EndpointA.SendPacketV2(timeoutHeight, 0, ibcmock.Version, ibctesting.MockPacketData)
 					suite.Require().NoError(err)
 
-					packet = channeltypes.NewPacketWithVersion(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ClientID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ClientID, timeoutHeight, 0, ibcmock.Version)
+					packet = channeltypes.NewPacketWithVersion(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0, ibcmock.Version)
 				}
 
 				err := path.EndpointA.UpdateClient()
@@ -971,7 +971,7 @@ func (suite *KeeperTestSuite) TestTimeoutPacketV2() {
 		{
 			"success no-op: packet not sent", func() {
 				path.SetupV2()
-				packet = channeltypes.NewPacketWithVersion(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ClientID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ClientID, clienttypes.NewHeight(0, 1), 0, ibcmock.Version)
+				packet = channeltypes.NewPacketWithVersion(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, clienttypes.NewHeight(0, 1), 0, ibcmock.Version)
 				packetKey = host.PacketReceiptKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
 			},
 			nil,
@@ -1216,15 +1216,11 @@ func (suite *KeeperTestSuite) TestProvideCounterparty() {
 	}{
 		{
 			"success",
-			func() {},
-			nil,
-		},
-		{
-			"failure: unknown client identifier",
 			func() {
-				msg.Counterparty.ClientId = ibctesting.InvalidID
+				// set it before handler
+				suite.chainA.App.GetIBCKeeper().PacketServerKeeper.SetCounterparty(suite.chainA.GetContext(), msg.ChannelId, msg.Counterparty)
 			},
-			ibcerrors.ErrUnauthorized,
+			nil,
 		},
 		{
 			"failure: signer does not match creator",
@@ -1234,10 +1230,9 @@ func (suite *KeeperTestSuite) TestProvideCounterparty() {
 			ibcerrors.ErrUnauthorized,
 		},
 		{
-			"failure: counterparty already exists",
+			"failure: counterparty does not already exists",
 			func() {
-				// set it before handler
-				suite.chainA.App.GetIBCKeeper().PacketServerKeeper.SetCounterparty(suite.chainA.GetContext(), msg.ChannelId, msg.Counterparty)
+				suite.chainA.App.GetIBCKeeper().PacketServerKeeper.ChannelStore(suite.chainA.GetContext(), path.EndpointA.ChannelID).Delete([]byte(packetservertypes.CounterpartyKey))
 			},
 			packetservertypes.ErrInvalidCounterparty,
 		},
@@ -1248,9 +1243,11 @@ func (suite *KeeperTestSuite) TestProvideCounterparty() {
 		path = ibctesting.NewPath(suite.chainA, suite.chainB)
 		path.SetupClients()
 
+		suite.Require().NoError(path.EndpointA.CreateChannel())
+
 		signer := path.EndpointA.Chain.SenderAccount.GetAddress().String()
 		merklePrefix := commitmenttypesv2.NewMerklePath([]byte("mock-key"))
-		msg = packetservertypes.NewMsgProvideCounterparty(path.EndpointA.ClientID, path.EndpointB.ClientID, merklePrefix, signer)
+		msg = packetservertypes.NewMsgProvideCounterparty(path.EndpointA.ChannelID, path.EndpointB.ChannelID, merklePrefix, signer)
 
 		tc.malleate()
 
@@ -1263,11 +1260,11 @@ func (suite *KeeperTestSuite) TestProvideCounterparty() {
 			suite.Require().Nil(err)
 
 			// Assert counterparty set and creator deleted
-			counterparty, found := suite.chainA.App.GetIBCKeeper().PacketServerKeeper.GetCounterparty(suite.chainA.GetContext(), path.EndpointA.ClientID)
+			counterparty, found := suite.chainA.App.GetIBCKeeper().PacketServerKeeper.GetCounterparty(suite.chainA.GetContext(), path.EndpointA.ChannelID)
 			suite.Require().True(found)
 			suite.Require().Equal(counterparty, msg.Counterparty)
 
-			_, found = suite.chainA.App.GetIBCKeeper().ClientKeeper.GetCreator(suite.chainA.GetContext(), path.EndpointA.ClientID)
+			_, found = suite.chainA.App.GetIBCKeeper().PacketServerKeeper.GetCreator(suite.chainA.GetContext(), path.EndpointA.ClientID)
 			suite.Require().False(found)
 		} else {
 			suite.Require().Nil(resp)

diff --git a/modules/core/packet-server/keeper/grpc_query.go b/modules/core/packet-server/keeper/grpc_query.go
@@ -42,7 +42,7 @@ func (q *queryServer) Client(ctx context.Context, req *types.QueryClientRequest)
 
 	sdkCtx := sdk.UnwrapSDKContext(ctx)
 
-	creator, foundCreator := q.ClientKeeper.GetCreator(sdkCtx, req.ClientId)
+	creator, foundCreator := q.GetCreator(sdkCtx, req.ClientId)
 	counterparty, foundCounterparty := q.GetCounterparty(sdkCtx, req.ClientId)
 
 	if !foundCreator && !foundCounterparty {

diff --git a/modules/core/packet-server/keeper/grpc_query_test.go b/modules/core/packet-server/keeper/grpc_query_test.go
@@ -28,7 +28,7 @@ func (suite *KeeperTestSuite) TestQueryClient() {
 			"success",
 			func() {
 				ctx := suite.chainA.GetContext()
-				suite.chainA.App.GetIBCKeeper().ClientKeeper.SetCreator(ctx, ibctesting.FirstClientID, expCreator)
+				suite.chainA.App.GetIBCKeeper().PacketServerKeeper.SetCreator(ctx, ibctesting.FirstClientID, expCreator)
 				suite.chainA.App.GetIBCKeeper().PacketServerKeeper.SetCounterparty(ctx, ibctesting.FirstClientID, expCounterparty)
 
 				req = &types.QueryClientRequest{
@@ -55,7 +55,7 @@ func (suite *KeeperTestSuite) TestQueryClient() {
 			func() {
 				expCounterparty = types.Counterparty{}
 
-				suite.chainA.App.GetIBCKeeper().ClientKeeper.SetCreator(suite.chainA.GetContext(), ibctesting.FirstClientID, expCreator)
+				suite.chainA.App.GetIBCKeeper().PacketServerKeeper.SetCreator(suite.chainA.GetContext(), ibctesting.FirstClientID, expCreator)
 
 				req = &types.QueryClientRequest{
 					ClientId: ibctesting.FirstClientID,

diff --git a/modules/core/packet-server/keeper/keeper.go b/modules/core/packet-server/keeper/keeper.go
@@ -66,3 +66,26 @@ func (k *Keeper) GetCounterparty(ctx context.Context, clientID string) (types.Co
 	k.cdc.MustUnmarshal(bz, &counterparty)
 	return counterparty, true
 }
+
+// GetCreator returns the creator of the client.
+func (k *Keeper) GetCreator(ctx context.Context, clientID string) (string, bool) {
+	sdkCtx := sdk.UnwrapSDKContext(ctx) // TODO: https://github.com/cosmos/ibc-go/issues/5917
+	bz := k.ChannelStore(sdkCtx, clientID).Get([]byte(types.CreatorKey))
+	if len(bz) == 0 {
+		return "", false
+	}
+
+	return string(bz), true
+}
+
+// SetCreator sets the creator of the client.
+func (k *Keeper) SetCreator(ctx context.Context, clientID, creator string) {
+	sdkCtx := sdk.UnwrapSDKContext(ctx) // TODO: https://github.com/cosmos/ibc-go/issues/5917
+	k.ChannelStore(sdkCtx, clientID).Set([]byte(types.CreatorKey), []byte(creator))
+}
+
+// DeleteCreator deletes the creator associated with the client.
+func (k *Keeper) DeleteCreator(ctx context.Context, clientID string) {
+	sdkCtx := sdk.UnwrapSDKContext(ctx) // TODO: https://github.com/cosmos/ibc-go/issues/5917
+	k.ChannelStore(sdkCtx, clientID).Delete([]byte(types.CreatorKey))
+}