Skip to content

Commit

Permalink
refactor: change counterparty structure. (#7328)
Browse files Browse the repository at this point in the history
* refactor: change counterparty structure.

* refactor: add channel id to Counterparty.

* refactor: use client identifier in relay functions.

* chore: suffix _id to identifier.
  • Loading branch information
DimitrisJim authored Sep 30, 2024
1 parent f7cf24d commit f2f46bc
Show file tree
Hide file tree
Showing 12 changed files with 169 additions and 84 deletions.
10 changes: 5 additions & 5 deletions modules/core/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func (k *Keeper) IBCSoftwareUpgrade(goCtx context.Context, msg *clienttypes.MsgI
func (k *Keeper) ProvideCounterparty(goCtx context.Context, msg *packetservertypes.MsgProvideCounterparty) (*packetservertypes.MsgProvideCounterpartyResponse, error) {
ctx := sdk.UnwrapSDKContext(goCtx)

creator, found := k.ClientKeeper.GetCreator(ctx, msg.ClientId)
creator, found := k.ClientKeeper.GetCreator(ctx, msg.Counterparty.ClientId)
if !found {
return nil, errorsmod.Wrap(ibcerrors.ErrUnauthorized, "client creator must be set")
}
Expand All @@ -157,13 +157,13 @@ func (k *Keeper) ProvideCounterparty(goCtx context.Context, msg *packetservertyp
return nil, errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "client creator (%s) must match signer (%s)", creator, msg.Signer)
}

if _, ok := k.PacketServerKeeper.GetCounterparty(ctx, msg.ClientId); ok {
return nil, errorsmod.Wrapf(packetservertypes.ErrInvalidCounterparty, "counterparty already exists for client %s", msg.ClientId)
if _, ok := k.PacketServerKeeper.GetCounterparty(ctx, msg.ChannelId); ok {
return nil, errorsmod.Wrapf(packetservertypes.ErrInvalidCounterparty, "counterparty already exists for client %s", msg.ChannelId)
}

k.PacketServerKeeper.SetCounterparty(ctx, msg.ClientId, msg.Counterparty)
k.PacketServerKeeper.SetCounterparty(ctx, msg.ChannelId, msg.Counterparty)
// Delete client creator from state as it is not needed after this point.
k.ClientKeeper.DeleteCreator(ctx, msg.ClientId)
k.ClientKeeper.DeleteCreator(ctx, msg.Counterparty.ClientId)

return &packetservertypes.MsgProvideCounterpartyResponse{}, nil
}
Expand Down
4 changes: 2 additions & 2 deletions modules/core/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1222,7 +1222,7 @@ func (suite *KeeperTestSuite) TestProvideCounterparty() {
{
"failure: unknown client identifier",
func() {
msg.ClientId = ibctesting.InvalidID
msg.Counterparty.ClientId = ibctesting.InvalidID
},
ibcerrors.ErrUnauthorized,
},
Expand All @@ -1237,7 +1237,7 @@ func (suite *KeeperTestSuite) TestProvideCounterparty() {
"failure: counterparty already exists",
func() {
// set it before handler
suite.chainA.App.GetIBCKeeper().PacketServerKeeper.SetCounterparty(suite.chainA.GetContext(), msg.ClientId, msg.Counterparty)
suite.chainA.App.GetIBCKeeper().PacketServerKeeper.SetCounterparty(suite.chainA.GetContext(), msg.ChannelId, msg.Counterparty)
},
packetservertypes.ErrInvalidCounterparty,
},
Expand Down
4 changes: 2 additions & 2 deletions modules/core/packet-server/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ The [counterparty-merkle-path-prefix] is a comma-separated list of hex-encoded s
return err
}

counterparty := types.NewCounterparty(counterpartyClientIdentifier, counterpartyMerklePathPrefix)
counterparty := types.NewCounterparty(clientIdentifier, counterpartyClientIdentifier, counterpartyMerklePathPrefix)
msg := types.MsgProvideCounterparty{
ClientId: clientIdentifier,
ChannelId: clientIdentifier,
Counterparty: counterparty,
Signer: clientCtx.GetFromAddress().String(),
}
Expand Down
31 changes: 18 additions & 13 deletions modules/core/packet-server/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ func (k Keeper) SendPacket(
if !ok {
return 0, errorsmod.Wrap(types.ErrCounterpartyNotFound, sourceChannel)
}
destChannel := counterparty.ClientId
destChannel := counterparty.CounterpartyChannelId
clientID := counterparty.ClientId

// retrieve the sequence send for this channel
// if no packets have been sent yet, initialize the sequence to 1.
Expand All @@ -54,17 +55,17 @@ func (k Keeper) SendPacket(
}

// check that the client of counterparty chain is still active
if status := k.ClientKeeper.GetClientStatus(ctx, sourceChannel); status != exported.Active {
return 0, errorsmod.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", sourceChannel, status)
if status := k.ClientKeeper.GetClientStatus(ctx, clientID); status != exported.Active {
return 0, errorsmod.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status)
}

// retrieve latest height and timestamp of the client of counterparty chain
latestHeight := k.ClientKeeper.GetClientLatestHeight(ctx, sourceChannel)
latestHeight := k.ClientKeeper.GetClientLatestHeight(ctx, clientID)
if latestHeight.IsZero() {
return 0, errorsmod.Wrapf(clienttypes.ErrInvalidHeight, "cannot send packet using client (%s) with zero height", sourceChannel)
return 0, errorsmod.Wrapf(clienttypes.ErrInvalidHeight, "cannot send packet using client (%s) with zero height", clientID)
}

latestTimestamp, err := k.ClientKeeper.GetClientTimestampAtHeight(ctx, sourceChannel, latestHeight)
latestTimestamp, err := k.ClientKeeper.GetClientTimestampAtHeight(ctx, clientID, latestHeight)
if err != nil {
return 0, err
}
Expand Down Expand Up @@ -113,9 +114,11 @@ func (k Keeper) RecvPacket(
if !ok {
return "", errorsmod.Wrap(types.ErrCounterpartyNotFound, packet.DestinationChannel)
}
if counterparty.ClientId != packet.SourceChannel {

if counterparty.CounterpartyChannelId != packet.SourceChannel {
return "", channeltypes.ErrInvalidChannelIdentifier
}
clientID := counterparty.ClientId

// check if packet timed out by comparing it with the latest height of the chain
sdkCtx := sdk.UnwrapSDKContext(ctx) // TODO: https://github.com/cosmos/ibc-go/issues/5917
Expand Down Expand Up @@ -144,14 +147,14 @@ func (k Keeper) RecvPacket(

if err := k.ClientKeeper.VerifyMembership(
ctx,
packet.DestinationChannel,
clientID,
proofHeight,
0, 0,
proof,
merklePath,
commitment,
); err != nil {
return "", errorsmod.Wrapf(err, "failed packet commitment verification for client (%s)", packet.DestinationChannel)
return "", errorsmod.Wrapf(err, "failed packet commitment verification for client (%s)", clientID)
}

// Set Packet Receipt to prevent timeout from occurring on counterparty
Expand Down Expand Up @@ -248,6 +251,7 @@ func (k Keeper) AcknowledgePacket(
if counterparty.ClientId != packet.DestinationChannel {
return "", channeltypes.ErrInvalidChannelIdentifier
}
clientID := counterparty.ClientId

commitment := k.ChannelKeeper.GetPacketCommitment(ctx, packet.SourcePort, packet.SourceChannel, packet.Sequence)
if len(commitment) == 0 {
Expand All @@ -272,14 +276,14 @@ func (k Keeper) AcknowledgePacket(

if err := k.ClientKeeper.VerifyMembership(
ctx,
packet.SourceChannel,
clientID,
proofHeight,
0, 0,
proofAcked,
merklePath,
channeltypes.CommitAcknowledgement(acknowledgement),
); err != nil {
return "", errorsmod.Wrapf(err, "failed packet acknowledgement verification for client (%s)", packet.SourceChannel)
return "", errorsmod.Wrapf(err, "failed packet acknowledgement verification for client (%s)", clientID)
}

k.ChannelKeeper.DeletePacketCommitment(ctx, packet.SourcePort, packet.SourceChannel, packet.Sequence)
Expand Down Expand Up @@ -318,6 +322,7 @@ func (k Keeper) TimeoutPacket(
if counterparty.ClientId != packet.DestinationChannel {
return "", channeltypes.ErrInvalidChannelIdentifier
}
clientID := counterparty.ClientId

// check that timeout height or timeout timestamp has passed on the other end
proofTimestamp, err := k.ClientKeeper.GetClientTimestampAtHeight(ctx, packet.SourceChannel, proofHeight)
Expand Down Expand Up @@ -354,13 +359,13 @@ func (k Keeper) TimeoutPacket(

if err := k.ClientKeeper.VerifyNonMembership(
ctx,
packet.SourceChannel,
clientID,
proofHeight,
0, 0,
proof,
merklePath,
); err != nil {
return "", errorsmod.Wrapf(err, "failed packet receipt absence verification for client (%s)", packet.SourceChannel)
return "", errorsmod.Wrapf(err, "failed packet receipt absence verification for client (%s)", clientID)
}

// delete packet commitment to prevent replay
Expand Down
11 changes: 8 additions & 3 deletions modules/core/packet-server/types/counterparty.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ import (
)

// NewCounterparty creates a new Counterparty instance
func NewCounterparty(clientID string, merklePathPrefix commitmenttypes.MerklePath) Counterparty {
func NewCounterparty(clientID, counterpartyChannelID string, merklePathPrefix commitmenttypes.MerklePath) Counterparty {
return Counterparty{
ClientId: clientID,
MerklePathPrefix: merklePathPrefix,
ClientId: clientID,
CounterpartyChannelId: counterpartyChannelID,
MerklePathPrefix: merklePathPrefix,
}
}

Expand All @@ -21,6 +22,10 @@ func (c Counterparty) Validate() error {
return err
}

if err := host.ChannelIdentifierValidator(c.CounterpartyChannelId); err != nil {
return err
}

if err := c.MerklePathPrefix.ValidateAsPrefix(); err != nil {
return errorsmod.Wrap(ErrInvalidCounterparty, err.Error())
}
Expand Down
102 changes: 79 additions & 23 deletions modules/core/packet-server/types/counterparty.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit f2f46bc

Please sign in to comment.