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

chore: adding implementation for SendPacket message server #7383

Merged
Show file tree
Hide file tree
Changes from 2 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
13 changes: 13 additions & 0 deletions modules/core/04-channel/types/timeout.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ func NewTimeout(height clienttypes.Height, timestamp uint64) Timeout {
}
}

// NewTimeoutWithTimestamp creates a new Timeout with only the timestamp set.
func NewTimeoutWithTimestamp(timestamp uint64) Timeout {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we only care about timestamp in the eureka spec

return Timeout{
Height: clienttypes.ZeroHeight(),
Timestamp: timestamp,
}
}

// IsValid returns true if either the height or timestamp is non-zero.
func (t Timeout) IsValid() bool {
return !t.Height.IsZero() || t.Timestamp != 0
Expand All @@ -25,6 +33,11 @@ func (t Timeout) Elapsed(height clienttypes.Height, timestamp uint64) bool {
return t.heightElapsed(height) || t.timestampElapsed(timestamp)
}

// TimestampElapsed returns true if the provided timestamp is past the timeout timestamp.
func (t Timeout) TimestampElapsed(timestamp uint64) bool {
return t.timestampElapsed(timestamp)
}

// ErrTimeoutElapsed returns a timeout elapsed error indicating which timeout value
// has elapsed.
func (t Timeout) ErrTimeoutElapsed(height clienttypes.Height, timestamp uint64) error {
Expand Down
12 changes: 12 additions & 0 deletions modules/core/04-channel/v2/keeper/events.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package keeper

import (
"context"

channeltypesv2 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/types"
)

// EmitSendPacketEvents emits events for the SendPacket handler.
func EmitSendPacketEvents(ctx context.Context, packet channeltypesv2.Packet) {
// TODO: https://github.com/cosmos/ibc-go/issues/7386
}
4 changes: 3 additions & 1 deletion modules/core/04-channel/v2/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@ import (
type Keeper struct {
cdc codec.BinaryCodec
storeService corestore.KVStoreService
ClientKeeper types.ClientKeeper
}

// NewKeeper creates a new channel v2 keeper
func NewKeeper(cdc codec.BinaryCodec, storeService corestore.KVStoreService) *Keeper {
func NewKeeper(cdc codec.BinaryCodec, storeService corestore.KVStoreService, clientKeeper types.ClientKeeper) *Keeper {
return &Keeper{
cdc: cdc,
storeService: storeService,
ClientKeeper: clientKeeper,
}
}

Expand Down
53 changes: 53 additions & 0 deletions modules/core/04-channel/v2/keeper/msg_server.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package keeper

import (
"context"

errorsmod "cosmossdk.io/errors"

sdk "github.com/cosmos/cosmos-sdk/types"

channeltypesv2 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/types"
)

var _ channeltypesv2.PacketMsgServer = &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)
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)
}

signer, err := sdk.AccAddressFromBech32(msg.Signer)
if err != nil {
sdkCtx.Logger().Error("send packet failed", "error", errorsmod.Wrap(err, "Invalid address for msg Signer"))
return nil, errorsmod.Wrap(err, "Invalid address for msg Signer")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
sdkCtx.Logger().Error("send packet failed", "error", errorsmod.Wrap(err, "Invalid address for msg Signer"))
return nil, errorsmod.Wrap(err, "Invalid address for msg Signer")
sdkCtx.Logger().Error("send packet failed", "error", errorsmod.Wrap(err, "invalid address for msg Signer"))
return nil, errorsmod.Wrap(err, "invalid address for msg Signer")

}

_ = signer

// TODO: implement once app router is wired up.
// 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)
// if err != nil {
// return nil, err
// }
// }

return &channeltypesv2.MsgSendPacketResponse{Sequence: sequence}, nil
}

// RecvPacket implements the PacketMsgServer RecvPacket method.
func (k *Keeper) RecvPacket(ctx context.Context, packet *channeltypesv2.MsgRecvPacket) (*channeltypesv2.MsgRecvPacketResponse, error) {

Check failure on line 46 in modules/core/04-channel/v2/keeper/msg_server.go

View workflow job for this annotation

GitHub Actions / lint

unused-receiver: method receiver 'k' is not referenced in method's body, consider removing or renaming it as _ (revive)

Check failure on line 46 in modules/core/04-channel/v2/keeper/msg_server.go

View workflow job for this annotation

GitHub Actions / lint

unused-receiver: method receiver 'k' is not referenced in method's body, consider removing or renaming it as _ (revive)
panic("implement me")
}

// Timeout implements the PacketMsgServer Timeout method.
func (k *Keeper) Timeout(ctx context.Context, timeout *channeltypesv2.MsgTimeout) (*channeltypesv2.MsgTimeoutResponse, error) {

Check failure on line 51 in modules/core/04-channel/v2/keeper/msg_server.go

View workflow job for this annotation

GitHub Actions / lint

unused-receiver: method receiver 'k' is not referenced in method's body, consider removing or renaming it as _ (revive)

Check failure on line 51 in modules/core/04-channel/v2/keeper/msg_server.go

View workflow job for this annotation

GitHub Actions / lint

unused-receiver: method receiver 'k' is not referenced in method's body, consider removing or renaming it as _ (revive)
panic("implement me")
}
78 changes: 78 additions & 0 deletions modules/core/04-channel/v2/keeper/packet.go
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want to name this relay.go? I think this currently mimics the name in channel/keeper/ though right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I think we should call it relay.go

Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package keeper

import (
"context"
"strconv"

errorsmod "cosmossdk.io/errors"

clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we want to a new timeout type that lives just in channeltypesv2? To not need this dependency

Copy link
Contributor

Choose a reason for hiding this comment

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

lets keep track of it in issue and close if it seems overkill?

Copy link
Contributor

Choose a reason for hiding this comment

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

the timeout being referenced by the light client interface seems unavoidable to me, so I think that's the main consideration. Checkout #7055 for context on how this might look going forward as well. In #7055, you could remove the timeout type and separate legacy behaviour by providing the timestamp/height type separately

channeltypesv2 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/types"
"github.com/cosmos/ibc-go/v9/modules/core/exported"
"github.com/cosmos/ibc-go/v9/modules/core/packet-server/types"
)

// sendPacket constructs a packet from the input arguments, writes a packet commitment to state
// in order for the packet to be sent to the counterparty.
func (k *Keeper) sendPacket(
ctx context.Context,
sourceID string,
timeoutTimestamp uint64,
data []channeltypesv2.PacketData,
) (uint64, error) {
// TODO: add aliasing logic
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chatton create an issue for this!


// Lookup counterparty associated with our source channel to retrieve the destination channel
counterparty, ok := k.GetCounterparty(ctx, sourceID)
if !ok {
return 0, errorsmod.Wrap(types.ErrCounterpartyNotFound, sourceID)
}

// retrieve the sequence send for this channel
// if no packets have been sent yet, initialize the sequence to 1.
sequence, found := k.GetNextSequenceSend(ctx, counterparty.ClientId)
if !found {
sequence = 1
}

// construct packet from given fields and channel state
packet := channeltypesv2.NewPacket(sequence, sourceID, counterparty.ClientId, timeoutTimestamp, data...)

if err := packet.ValidateBasic(); err != nil {
return 0, errorsmod.Wrapf(channeltypes.ErrInvalidPacket, "constructed packet failed basic validation: %v", err)
}

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

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

latestTimestamp, err := k.ClientKeeper.GetClientTimestampAtHeight(ctx, counterparty.ClientId, latestHeight)
if err != nil {
return 0, err
}
// check if packet is timed out on the receiving chain
timeout := channeltypes.NewTimeoutWithTimestamp(timeoutTimestamp)
if timeout.TimestampElapsed(latestTimestamp) {
return 0, errorsmod.Wrap(timeout.ErrTimeoutElapsed(latestHeight, latestTimestamp), "invalid packet timeout")
}

commitment := channeltypesv2.CommitPacket(packet)

// bump the sequence and set the packet commitment so it is provable by the counterparty
k.SetNextSequenceSend(ctx, counterparty.ClientId, sequence+1)
k.SetPacketCommitment(ctx, counterparty.ClientId, packet.GetSequence(), commitment)

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

EmitSendPacketEvents(ctx, packet)

return sequence, nil
}
16 changes: 16 additions & 0 deletions modules/core/04-channel/v2/types/channel.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package types

func NewPacket(sequence uint64, sourceID, destinationID string, timeoutTimestamp uint64, data ...PacketData) Packet {
return Packet{
Sequence: sequence,
SourceId: sourceID,
DestinationId: destinationID,
TimeoutTimestamp: timeoutTimestamp,
Data: data,
}
}

func (p Packet) ValidateBasic() error {

Check failure on line 13 in modules/core/04-channel/v2/types/channel.go

View workflow job for this annotation

GitHub Actions / lint

unused-receiver: method receiver 'p' is not referenced in method's body, consider removing or renaming it as _ (revive)

Check failure on line 13 in modules/core/04-channel/v2/types/channel.go

View workflow job for this annotation

GitHub Actions / lint

unused-receiver: method receiver 'p' is not referenced in method's body, consider removing or renaming it as _ (revive)
// TODO: https://github.com/cosmos/ibc-go/issues/7385
return nil
}
10 changes: 10 additions & 0 deletions modules/core/04-channel/v2/types/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package types
Copy link
Contributor Author

Choose a reason for hiding this comment

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

duplicate from packet-server


import (
errorsmod "cosmossdk.io/errors"
)

var (
ErrInvalidCounterparty = errorsmod.Register(SubModuleName, 1, "invalid counterparty")
ErrCounterpartyNotFound = errorsmod.Register(SubModuleName, 2, "counterparty not found")
)
25 changes: 25 additions & 0 deletions modules/core/04-channel/v2/types/expected_keepers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package types
Copy link
Contributor Author

Choose a reason for hiding this comment

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

duplicate from packet-server keeper


import (
"context"

clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types"
"github.com/cosmos/ibc-go/v9/modules/core/exported"
)

type ClientKeeper interface {
// 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.
VerifyMembership(ctx context.Context, clientID string, height exported.Height, delayTimePeriod uint64, delayBlockPeriod uint64, proof []byte, path exported.Path, value []byte) error
// VerifyNonMembership retrieves the light client module for the clientID and verifies the absence of a given key at a specified height.
VerifyNonMembership(ctx context.Context, clientID string, height exported.Height, delayTimePeriod uint64, delayBlockPeriod uint64, proof []byte, path exported.Path) error
// GetClientStatus returns the status of a client given the client ID
GetClientStatus(ctx context.Context, clientID string) exported.Status
// GetClientLatestHeight returns the latest height of a client given the client ID
GetClientLatestHeight(ctx context.Context, clientID string) clienttypes.Height
// 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)
}
24 changes: 23 additions & 1 deletion modules/core/05-port/types/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ package types

import (
"context"

sdk "github.com/cosmos/cosmos-sdk/types"

clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
channeltypesv2 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/types"
"github.com/cosmos/ibc-go/v9/modules/core/exported"
)

Expand Down Expand Up @@ -108,6 +108,28 @@ type IBCModule interface {
) error
}

// IBCModuleV2 defines an interface that implements all the callbacks
// that modules must define as specified in IBC Protocol V2
type IBCModuleV2 interface {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not convinced this is the best place for this interface, open to suggestions! (Also do we want V2 in the name? )

Copy link
Contributor

Choose a reason for hiding this comment

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

It really should be referenced as api.IBCModule, I wonder if it makes sense to create the api directory now, would be a weird partial transition if you didn't move over the light client module

Main dependency concern is that if you move this api elsewhere, you may need to define interfaces or move the concrete implementations

// OnSendPacket is executed when a packet is being sent from sending chain.
// this callback is provided with the source and destination IDs, the signer, the packet sequence and the packet data
// for this specific application.
OnSendPacket(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I figure we can add them one method at a time

ctx context.Context,
sourceID string,
destinationID string,
sequence uint64,
data channeltypesv2.PacketData,
signer sdk.AccAddress,
) error

// OnRecvPacket

// OnAcknowledgementPacket

// OnTimeoutPacket
}

// UpgradableModule defines the callbacks required to perform a channel upgrade.
// Note: applications must ensure that state related to packet processing remains unmodified until the OnChanUpgradeOpen callback is executed.
// This guarantees that in-flight packets are correctly flushed using the existing channel parameters.
Expand Down
Loading