Skip to content

Rename Id Fields to Channel for Packet V2 #7428

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.

Sign up for GitHub

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

Merged
AdityaSripal merged 2 commits into from
Oct 9, 2024
Merged

Rename Id Fields to Channel for Packet V2 #7428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f5a63d3
change id fields to channel
AdityaSripal Oct 9, 2024
0a9d798
Apply suggestions from code review
AdityaSripal Oct 9, 2024
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
18 changes: 9 additions & 9 deletions modules/core/04-channel/v2/keeper/msg_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ var _ channeltypesv2.MsgServer = &Keeper{}
// SendPacket implements the PacketMsgServer SendPacket method.
func (k *Keeper) SendPacket(ctx context.Context, msg *channeltypesv2.MsgSendPacket) (*channeltypesv2.MsgSendPacketResponse, error) {
sdkCtx := sdk.UnwrapSDKContext(ctx)
sequence, err := k.sendPacket(ctx, msg.SourceId, msg.TimeoutTimestamp, msg.PacketData)
sequence, err := k.sendPacket(ctx, msg.SourceChannel, msg.TimeoutTimestamp, msg.PacketData)
if err != nil {
sdkCtx.Logger().Error("send packet failed", "source-id", msg.SourceId, "error", errorsmod.Wrap(err, "send packet failed"))
return nil, errorsmod.Wrapf(err, "send packet failed for source id: %s", msg.SourceId)
sdkCtx.Logger().Error("send packet failed", "source-id", msg.SourceChannel, "error", errorsmod.Wrap(err, "send packet failed"))
return nil, errorsmod.Wrapf(err, "send packet failed for source id: %s", msg.SourceChannel)
}

signer, err := sdk.AccAddressFromBech32(msg.Signer)
Expand All @@ -34,7 +34,7 @@ func (k *Keeper) SendPacket(ctx context.Context, msg *channeltypesv2.MsgSendPack
// https://github.com/cosmos/ibc-go/issues/7384
// for _, pd := range msg.PacketData {
// cbs := k.PortKeeper.AppRouter.Route(pd.SourcePort)
// err := cbs.OnSendPacket(ctx, msg.SourceId, sequence, msg.TimeoutTimestamp, pd, signer)
// err := cbs.OnSendPacket(ctx, msg.SourceChannel, sequence, msg.TimeoutTimestamp, pd, signer)
// if err != nil {
// return nil, err
// }
Expand All @@ -52,8 +52,8 @@ func (k *Keeper) RecvPacket(ctx context.Context, msg *channeltypesv2.MsgRecvPack
sdkCtx := sdk.UnwrapSDKContext(ctx)
err := k.recvPacket(ctx, msg.Packet, msg.ProofCommitment, msg.ProofHeight)
if err != nil {
sdkCtx.Logger().Error("receive packet failed", "source-id", msg.Packet.SourceId, "dest-id", msg.Packet.DestinationId, "error", errorsmod.Wrap(err, "send packet failed"))
return nil, errorsmod.Wrapf(err, "receive packet failed for source id: %s and destination id: %s", msg.Packet.SourceId, msg.Packet.DestinationId)
sdkCtx.Logger().Error("receive packet failed", "source-id", msg.Packet.SourceChannel, "dest-id", msg.Packet.DestinationChannel, "error", errorsmod.Wrap(err, "send packet failed"))
return nil, errorsmod.Wrapf(err, "receive packet failed for source id: %s and destination id: %s", msg.Packet.SourceChannel, msg.Packet.DestinationChannel)
}

signer, err := sdk.AccAddressFromBech32(msg.Signer)
Expand Down Expand Up @@ -81,8 +81,8 @@ func (k *Keeper) RecvPacket(ctx context.Context, msg *channeltypesv2.MsgRecvPack
func (k *Keeper) Timeout(ctx context.Context, timeout *channeltypesv2.MsgTimeout) (*channeltypesv2.MsgTimeoutResponse, error) {
sdkCtx := sdk.UnwrapSDKContext(ctx)
if err := k.timeoutPacket(ctx, timeout.Packet, timeout.ProofUnreceived, timeout.ProofHeight); err != nil {
sdkCtx.Logger().Error("Timeout packet failed", "source-id", timeout.Packet.SourceId, "destination-id", timeout.Packet.DestinationId, "error", errorsmod.Wrap(err, "timeout packet failed"))
return nil, errorsmod.Wrapf(err, "send packet failed for source id: %s and destination id: %s", timeout.Packet.SourceId, timeout.Packet.DestinationId)
sdkCtx.Logger().Error("Timeout packet failed", "source-id", timeout.Packet.SourceChannel, "destination-id", timeout.Packet.DestinationChannel, "error", errorsmod.Wrap(err, "timeout packet failed"))
return nil, errorsmod.Wrapf(err, "send packet failed for source id: %s and destination id: %s", timeout.Packet.SourceChannel, timeout.Packet.DestinationChannel)
}

signer, err := sdk.AccAddressFromBech32(timeout.Signer)
Expand All @@ -97,7 +97,7 @@ func (k *Keeper) Timeout(ctx context.Context, timeout *channeltypesv2.MsgTimeout
// https://github.com/cosmos/ibc-go/issues/7384
// for _, pd := range timeout.Packet.Data {
// cbs := k.PortKeeper.AppRouter.Route(pd.SourcePort)
// err := cbs.OnTimeoutPacket(timeout.Packet.SourceId, timeout.Packet.TimeoutTimestamp, signer)
// err := cbs.OnTimeoutPacket(timeout.Packet.SourceChannel, timeout.Packet.TimeoutTimestamp, signer)
// if err != nil {
// return err, err
// }
Expand Down
42 changes: 21 additions & 21 deletions modules/core/04-channel/v2/keeper/relay.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func (k *Keeper) sendPacket(
k.SetNextSequenceSend(ctx, sourceID, sequence+1)
k.SetPacketCommitment(ctx, sourceID, packet.GetSequence(), commitment)

k.Logger(ctx).Info("packet sent", "sequence", strconv.FormatUint(packet.Sequence, 10), "dest_id", packet.DestinationId, "src_id", packet.SourceId)
k.Logger(ctx).Info("packet sent", "sequence", strconv.FormatUint(packet.Sequence, 10), "dest_channel_id", packet.DestinationChannel, "src_channel_id", packet.SourceChannel)

EmitSendPacketEvents(ctx, packet)

Expand All @@ -114,15 +114,15 @@ func (k Keeper) recvPacket(
) error {
// Lookup counterparty associated with our channel and ensure
// that the packet was indeed sent by our counterparty.
counterparty, ok := k.GetCounterparty(ctx, packet.DestinationId)
counterparty, ok := k.GetCounterparty(ctx, packet.DestinationChannel)
if !ok {
// TODO: figure out how aliasing will work when more than one packet data is sent.
counterparty, ok = k.getV1Counterparty(ctx, packet.Data[0].DestinationPort, packet.DestinationId)
counterparty, ok = k.getV1Counterparty(ctx, packet.Data[0].DestinationPort, packet.DestinationChannel)
if !ok {
return errorsmod.Wrap(types.ErrCounterpartyNotFound, packet.DestinationId)
return errorsmod.Wrap(types.ErrCounterpartyNotFound, packet.DestinationChannel)
}
}
if counterparty.ClientId != packet.SourceId {
if counterparty.ClientId != packet.SourceChannel {
Copy link
Copy Markdown
Contributor

@DimitrisJim DimitrisJim Oct 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

not related to this but should be CounterpartyChannelId. Unsure if this applies elsewhere but feel free to fix in follow-up.

return channeltypes.ErrInvalidChannelIdentifier
}

Expand All @@ -137,7 +137,7 @@ func (k Keeper) recvPacket(
// REPLAY PROTECTION: Packet receipts will indicate that a packet has already been received
// on unordered channels. Packet receipts must not be pruned, unless it has been marked stale
// by the increase of the recvStartSequence.
_, found := k.GetPacketReceipt(ctx, packet.DestinationId, packet.Sequence)
_, found := k.GetPacketReceipt(ctx, packet.DestinationChannel, packet.Sequence)
if found {
EmitRecvPacketEvents(ctx, packet)
// This error indicates that the packet has already been relayed. Core IBC will
Expand All @@ -146,27 +146,27 @@ func (k Keeper) recvPacket(
return channeltypes.ErrNoOpMsg
}

path := hostv2.PacketCommitmentKey(packet.SourceId, packet.Sequence)
path := hostv2.PacketCommitmentKey(packet.SourceChannel, packet.Sequence)
merklePath := types.BuildMerklePath(counterparty.MerklePathPrefix, path)

commitment := channeltypesv2.CommitPacket(packet)

if err := k.ClientKeeper.VerifyMembership(
ctx,
packet.DestinationId,
packet.DestinationChannel,
Copy link
Copy Markdown
Contributor

@damiannolan damiannolan Oct 9, 2024

Choose a reason for hiding this comment

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

Likewise, lookup v2 channelEnd and use the clientID rather than channelID

Copy link
Copy Markdown
Contributor

@DimitrisJim DimitrisJim Oct 9, 2024

Choose a reason for hiding this comment

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

yes, changed this in packetserver/relay.go recently. In general, there might be some tweaks necessary in these files before tests for it pass.

Copy link
Copy Markdown
Contributor Author

@AdityaSripal AdityaSripal Oct 9, 2024

Choose a reason for hiding this comment

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

nice catch!! Though it seems this was already wrong before

Copy link
Copy Markdown
Contributor

@damiannolan damiannolan Oct 9, 2024

Choose a reason for hiding this comment

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

Yup!

proofHeight,
0, 0,
proof,
merklePath,
commitment,
); err != nil {
return errorsmod.Wrapf(err, "failed packet commitment verification for client (%s)", packet.DestinationId)
return errorsmod.Wrapf(err, "failed packet commitment verification for client (%s)", packet.DestinationChannel)
}

// Set Packet Receipt to prevent timeout from occurring on counterparty
k.SetPacketReceipt(ctx, packet.DestinationId, packet.Sequence)
k.SetPacketReceipt(ctx, packet.DestinationChannel, packet.Sequence)

k.Logger(ctx).Info("packet received", "sequence", strconv.FormatUint(packet.Sequence, 10), "src_id", packet.SourceId, "dst_id", packet.DestinationId)
k.Logger(ctx).Info("packet received", "sequence", strconv.FormatUint(packet.Sequence, 10), "src_id", packet.SourceChannel, "dst_id", packet.DestinationChannel)

EmitRecvPacketEvents(ctx, packet)

Expand All @@ -188,17 +188,17 @@ func (k Keeper) timeoutPacket(
) error {
// Lookup counterparty associated with our channel and ensure
// that the packet was indeed sent by our counterparty.
counterparty, ok := k.GetCounterparty(ctx, packet.SourceId)
counterparty, ok := k.GetCounterparty(ctx, packet.SourceChannel)
if !ok {
// TODO: figure out how aliasing will work when more than one packet data is sent.
counterparty, ok = k.getV1Counterparty(ctx, packet.Data[0].SourcePort, packet.SourceId)
counterparty, ok = k.getV1Counterparty(ctx, packet.Data[0].SourcePort, packet.SourceChannel)
if !ok {
return errorsmod.Wrap(types.ErrCounterpartyNotFound, packet.DestinationId)
return errorsmod.Wrap(types.ErrCounterpartyNotFound, packet.DestinationChannel)
}
}

// check that timeout height or timeout timestamp has passed on the other end
proofTimestamp, err := k.ClientKeeper.GetClientTimestampAtHeight(ctx, packet.SourceId, proofHeight)
proofTimestamp, err := k.ClientKeeper.GetClientTimestampAtHeight(ctx, packet.SourceChannel, proofHeight)
if err != nil {
return err
}
Expand All @@ -209,7 +209,7 @@ func (k Keeper) timeoutPacket(
}

// check that the commitment has not been cleared and that it matches the packet sent by relayer
commitment, ok := k.GetPacketCommitment(ctx, packet.SourceId, packet.Sequence)
commitment, ok := k.GetPacketCommitment(ctx, packet.SourceChannel, packet.Sequence)

if !ok {
EmitTimeoutPacketEvents(ctx, packet)
Expand All @@ -227,24 +227,24 @@ func (k Keeper) timeoutPacket(
}

// verify packet receipt absence
path := hostv2.PacketReceiptKey(packet.SourceId, packet.Sequence)
path := hostv2.PacketReceiptKey(packet.SourceChannel, packet.Sequence)
merklePath := types.BuildMerklePath(counterparty.MerklePathPrefix, path)

if err := k.ClientKeeper.VerifyNonMembership(
ctx,
packet.SourceId,
packet.SourceChannel,
Copy link
Copy Markdown
Contributor

@damiannolan damiannolan Oct 9, 2024

Choose a reason for hiding this comment

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

This should be looking up the v2 channelEnd (now counterparty) and taking the clientID from it and using it here!

Not sure if you want to do that in this PR or not

proofHeight,
0, 0,
proof,
merklePath,
); err != nil {
return errorsmod.Wrapf(err, "failed packet receipt absence verification for client (%s)", packet.SourceId)
return errorsmod.Wrapf(err, "failed packet receipt absence verification for client (%s)", packet.SourceChannel)
}

// delete packet commitment to prevent replay
k.DeletePacketCommitment(ctx, packet.SourceId, packet.Sequence)
k.DeletePacketCommitment(ctx, packet.SourceChannel, packet.Sequence)

k.Logger(ctx).Info("packet timed out", "sequence", strconv.FormatUint(packet.Sequence, 10), "src_id", packet.SourceId, "dst_id", packet.DestinationId)
k.Logger(ctx).Info("packet timed out", "sequence", strconv.FormatUint(packet.Sequence, 10), "src_channel_id", packet.SourceChannel, "dst_channel_id", packet.DestinationChannel)

EmitTimeoutPacketEvents(ctx, packet)

Expand Down
Loading