Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: create channel before invoking provide counterparty. #7420

Merged
merged 2 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 0 additions & 23 deletions modules/core/02-client/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 0 additions & 26 deletions modules/core/02-client/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
5 changes: 0 additions & 5 deletions modules/core/02-client/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions modules/core/04-channel/v2/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
22 changes: 11 additions & 11 deletions modules/core/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't know why this was never returned? can cherry pick to main too

}

// UpdateClient defines a rpc handler method for MsgUpdateClient.
Expand Down Expand Up @@ -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)

Expand All @@ -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
}
Expand Down
41 changes: 19 additions & 22 deletions modules/core/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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)
},
Expand All @@ -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,
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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,
Expand All @@ -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()
Expand All @@ -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,
Expand Down Expand Up @@ -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",
Expand All @@ -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,
},
Expand All @@ -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()

Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion modules/core/packet-server/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions modules/core/packet-server/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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,
Expand Down
23 changes: 23 additions & 0 deletions modules/core/packet-server/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ditto re moved

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))
}
Loading
Loading