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

[Do not merge] Spike adr-8 #3359

Closed
wants to merge 1 commit into from
Closed
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
221 changes: 221 additions & 0 deletions modules/apps/08-callbacks/ibc_middleware.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
package callbacks

import (
sdk "github.com/cosmos/cosmos-sdk/types"
capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"

clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
porttypes "github.com/cosmos/ibc-go/v7/modules/core/05-port/types"
"github.com/cosmos/ibc-go/v7/modules/core/exported"
)

type Example struct {
}

var _ porttypes.Middleware = &IBCMiddleware[Example]{}

// todo: concrete type
type CallbackPacketData any

type IBCMiddleware[T CallbackPacketData] struct {
next porttypes.Middleware
decoder Decoder[T]
executor Executor[T]
limits GasLimits
}

// Decoder unpacks a raw ibc packet to the custom type
type Decoder[T CallbackPacketData] interface {
// Decode packet to custom type. An empty result skips callback execution, error aborts process
Decode(packet channeltypes.Packet) (*T, error)
}

// Executor executes the callback
type Executor[T CallbackPacketData] interface {
OnRecvPacket(ctx sdk.Context, obj T, relayer sdk.AccAddress) error
OnAcknowledgementPacket(ctx sdk.Context, obj T, acknowledgement []byte, relayer sdk.AccAddress) error
OnTimeoutPacket(ctx sdk.Context, obj T, relayer sdk.AccAddress) error
}
type GasLimits struct {
OnRecvPacketLimit *sdk.Gas
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Usage of pointers for value unset instead of 0.

OnAcknowledgementPacketLimit *sdk.Gas
OnTimeoutPacketLimit *sdk.Gas
}

// NewIBCMiddleware constructor
func NewIBCMiddleware[T CallbackPacketData](next porttypes.Middleware, decoder Decoder[T], executor Executor[T], limits GasLimits) IBCMiddleware[T] {
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 choose next to be porttypes.Middleware as I plugged in ics29 middleware here. But it may make more sense to use porttypes.IBCModule instead.

return IBCMiddleware[T]{
next: next,
decoder: decoder,
executor: executor,
limits: limits,
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 configured limits with the middleware as this is in the adr-8 example. But when implemented into the executor, then it would be even more flexible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adr-8 defines the limits read from the packet not defined here. So this is wrong

}
}

// OnRecvPacket implements the IBCMiddleware interface.
// If fees are not enabled, this callback will default to the ibc-core packet callback
func (im IBCMiddleware[T]) OnRecvPacket(
ctx sdk.Context,
packet channeltypes.Packet,
relayer sdk.AccAddress,
) exported.Acknowledgement {

ack := im.next.OnRecvPacket(ctx, packet, relayer)

// todo: limit gas
p, err := im.decoder.Decode(packet)
if err != nil {
// see comments in https://github.com/cosmos/ibc-go/blob/main/docs/architecture/adr-008-app-caller-cbs/adr-008-app-caller-cbs.md
// todo: filter out events: https://github.com/cosmos/ibc-go/issues/3358
// todo: set event with raw error for debug as defined in ErrorAcknowledgement?
return channeltypes.NewErrorAcknowledgement(err)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are some downsides with returning error ack, that are outlined well in in the adr:

// NOTE: by returning an error acknowledgement, it is assumed that the
// base IBC application on the counterparty callback stack will be able
// to properly unmarshal the error acknowledgement. It should not expect
// some custom error acknowledgement. If it does, failed acknowledgements
// will be unsuccessfully processed which can be catastrophic in processing
// refund logic.

An alternative would be raising panic to abort the process and reverting state. The data packet would eventually run into a timeout instead

}
if p != nil {
if err := im.executor.OnRecvPacket(ctx, *p, relayer); err != nil {
// see comments in https://github.com/cosmos/ibc-go/blob/main/docs/architecture/adr-008-app-caller-cbs/adr-008-app-caller-cbs.md
// todo: filter out events: https://github.com/cosmos/ibc-go/issues/3358
// todo: set event with raw error for debug as defined in ErrorAcknowledgement?
return channeltypes.NewErrorAcknowledgement(err)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above. Should the callback be able to overwrite the ack or is panic the way to go?

}
}
return ack
}

// OnAcknowledgementPacket implements the IBCMiddleware interface
// If fees are not enabled, this callback will default to the ibc-core packet callback
func (im IBCMiddleware[T]) OnAcknowledgementPacket(
ctx sdk.Context,
packet channeltypes.Packet,
acknowledgement []byte,
relayer sdk.AccAddress,
) error {
// call underlying callback
if err := im.next.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer); err != nil {
return err
}
// todo: limit gas
p, err := im.decoder.Decode(packet)
if err != nil {
return err
}
if p != nil {
return im.executor.OnAcknowledgementPacket(ctx, *p, acknowledgement, relayer)
}
return nil
}

// OnTimeoutPacket implements the IBCMiddleware interface
// If fees are not enabled, this callback will default to the ibc-core packet callback
func (im IBCMiddleware[T]) OnTimeoutPacket(
ctx sdk.Context,
packet channeltypes.Packet,
relayer sdk.AccAddress,
) error {
if err := im.next.OnTimeoutPacket(ctx, packet, relayer); err != nil {
return err
}
// todo: limit gas
p, err := im.decoder.Decode(packet)
if err != nil {
return err
}
if p != nil {
return im.executor.OnTimeoutPacket(ctx, *p, relayer)
}
return nil
}

// OnChanOpenInit implements the IBCMiddleware interface
func (im IBCMiddleware[T]) OnChanOpenInit(
ctx sdk.Context,
order channeltypes.Order,
connectionHops []string,
portID string,
channelID string,
chanCap *capabilitytypes.Capability,
counterparty channeltypes.Counterparty,
version string,
) (string, error) {
return im.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version)
}

// OnChanOpenTry implements the IBCMiddleware interface
func (im IBCMiddleware[T]) OnChanOpenTry(
ctx sdk.Context,
order channeltypes.Order,
connectionHops []string,
portID,
channelID string,
chanCap *capabilitytypes.Capability,
counterparty channeltypes.Counterparty,
counterpartyVersion string,
) (string, error) {
return im.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, counterpartyVersion)
}

// OnChanOpenAck implements the IBCMiddleware interface
func (im IBCMiddleware[T]) OnChanOpenAck(
ctx sdk.Context,
portID,
channelID string,
counterpartyChannelID string,
counterpartyVersion string,
) error {
return im.next.OnChanOpenAck(ctx, portID, channelID, counterpartyChannelID, counterpartyVersion)
}

// OnChanOpenConfirm implements the IBCMiddleware interface
func (im IBCMiddleware[T]) OnChanOpenConfirm(
ctx sdk.Context,
portID,
channelID string,
) error {
return im.next.OnChanOpenConfirm(ctx, portID, channelID)
}

// OnChanCloseInit implements the IBCMiddleware interface
func (im IBCMiddleware[T]) OnChanCloseInit(
ctx sdk.Context,
portID,
channelID string,
) error {
return im.next.OnChanCloseInit(ctx, portID, channelID)
}

// OnChanCloseConfirm implements the IBCMiddleware interface
func (im IBCMiddleware[T]) OnChanCloseConfirm(
ctx sdk.Context,
portID,
channelID string,
) error {
return im.next.OnChanCloseConfirm(ctx, portID, channelID)
}

// SendPacket implements the ICS4 Wrapper interface
func (im IBCMiddleware[T]) SendPacket(
ctx sdk.Context,
chanCap *capabilitytypes.Capability,
sourcePort string,
sourceChannel string,
timeoutHeight clienttypes.Height,
timeoutTimestamp uint64,
data []byte,
) (uint64, error) {
return im.next.SendPacket(ctx, chanCap, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, data)
}

// WriteAcknowledgement implements the ICS4 Wrapper interface
func (im IBCMiddleware[T]) WriteAcknowledgement(
ctx sdk.Context,
chanCap *capabilitytypes.Capability,
packet exported.PacketI,
ack exported.Acknowledgement,
) error {
return im.next.WriteAcknowledgement(ctx, chanCap, packet, ack)
}

// GetAppVersion returns the application version of the underlying application
func (im IBCMiddleware[T]) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
return "ics-8.0-alex", true
}
58 changes: 58 additions & 0 deletions modules/apps/08-callbacks/ics20_example_cb.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package callbacks

import (
"encoding/json"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
)

// MyICS20Payload my random payload example
type MyICS20Payload struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some example data that would be extracted from memo

SrcCallbackAddress string `json:"src_callback_address"`
DstCallbackAddress string `json:"dst_callback_address"`
}

var _ Decoder[MyICS20Payload] = &MyICS20Decoder{}

type MyICS20Decoder struct { //nolint:gofumpt
}

func (m MyICS20Decoder) Decode(packet channeltypes.Packet) (*MyICS20Payload, error) {
var data types.FungibleTokenPacketData
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This decoder is for ics-20 packets only and can not be plugged into ics-29 for example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it would not be hard to provide a generic decoder. Question is if the payload type still makes sense.
Example:

type Memoable interface {
	gogoproto.Message
	GetMemo() string
}

type MyGenericMemoDecoder [T Memoable] struct {}

func (m MyGenericMemoDecoder[T]) Decode(packet channeltypes.Packet) (*MyICS20Payload, error) {
	var data T
	if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), data); err != nil {
		return nil, err
	}
	// do we need a sanity check that type was unpacked proper? like denom not empty

	if data.GetMemo() == "" {
		return nil, nil
	}
...

if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
return nil, err
}
// do we need a sanity check that type was unpacked proper? like denom not empty

if data.Memo == "" {
return nil, nil
}
// unmarshal json to type, I prefer using concrete type here:
var r MyICS20Payload
if err := json.Unmarshal([]byte(data.Memo), &r); err != nil {
return nil, err
}
// todo: check that contains valid data
return &r, nil
}

var _ Executor[MyICS20Payload] = &MyICS20Callbacks{}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Executor for ics20 only. my assumption was that chains may want to customize the callbacks for each of the ibc apps.


type MyICS20Callbacks struct { //nolint:gofumpt
}

func (m MyICS20Callbacks) OnRecvPacket(ctx sdk.Context, obj MyICS20Payload, relayer sdk.AccAddress) error {
ctx.Logger().Info("OnRecvPacket executed")
return nil
}

func (m MyICS20Callbacks) OnAcknowledgementPacket(ctx sdk.Context, obj MyICS20Payload, acknowledgement []byte, relayer sdk.AccAddress) error {
ctx.Logger().Info("OnAcknowledgementPacket executed")
return nil
}

func (m MyICS20Callbacks) OnTimeoutPacket(ctx sdk.Context, obj MyICS20Payload, relayer sdk.AccAddress) error {
ctx.Logger().Info("OnTimeoutPacket executed")
return nil
}
8 changes: 7 additions & 1 deletion testing/simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package simapp
import (
"encoding/json"
"fmt"
callbacks "github.com/cosmos/ibc-go/v7/modules/apps/08-callbacks"
"io"
"net/http"
"os"
Expand Down Expand Up @@ -471,7 +472,12 @@ func NewSimApp(
// create IBC module from bottom to top of stack
var transferStack porttypes.IBCModule
transferStack = transfer.NewIBCModule(app.TransferKeeper)
transferStack = ibcfee.NewIBCMiddleware(transferStack, app.IBCFeeKeeper)
transferStack = callbacks.NewIBCMiddleware[callbacks.MyICS20Payload](
ibcfee.NewIBCMiddleware(transferStack, app.IBCFeeKeeper),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is how the middleware would be integrated

callbacks.MyICS20Decoder{},
callbacks.MyICS20Callbacks{},
callbacks.GasLimits{},
)

// Add transfer stack to IBC Router
ibcRouter.AddRoute(ibctransfertypes.ModuleName, transferStack)
Expand Down