From 605f47cce8f952a8de6faafbef25f35cc6fa5ab4 Mon Sep 17 00:00:00 2001 From: srdtrk Date: Mon, 14 Aug 2023 16:09:38 +0300 Subject: [PATCH 1/8] style: ran golangci-lint --- modules/apps/callbacks/callbacks_test.go | 4 ++-- modules/apps/callbacks/fee_transfer_test.go | 2 +- modules/apps/callbacks/ibc_middleware.go | 2 +- modules/apps/callbacks/ica_test.go | 2 +- modules/apps/callbacks/testing/simapp/app.go | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/modules/apps/callbacks/callbacks_test.go b/modules/apps/callbacks/callbacks_test.go index 9552fa58a6b..5f63170e47f 100644 --- a/modules/apps/callbacks/callbacks_test.go +++ b/modules/apps/callbacks/callbacks_test.go @@ -16,11 +16,11 @@ import ( dbm "github.com/cometbft/cometbft-db" "github.com/cometbft/cometbft/libs/log" + simapp "github.com/cosmos/ibc-go/modules/apps/callbacks/testing/simapp" + "github.com/cosmos/ibc-go/modules/apps/callbacks/types" icacontrollertypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/controller/types" icatypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/types" feetypes "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/types" - simapp "github.com/cosmos/ibc-go/modules/apps/callbacks/testing/simapp" - "github.com/cosmos/ibc-go/modules/apps/callbacks/types" transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" ibctesting "github.com/cosmos/ibc-go/v7/testing" ibcmock "github.com/cosmos/ibc-go/v7/testing/mock" diff --git a/modules/apps/callbacks/fee_transfer_test.go b/modules/apps/callbacks/fee_transfer_test.go index 8eeab9c0924..f50c1d76abd 100644 --- a/modules/apps/callbacks/fee_transfer_test.go +++ b/modules/apps/callbacks/fee_transfer_test.go @@ -7,8 +7,8 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" - feetypes "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/types" "github.com/cosmos/ibc-go/modules/apps/callbacks/types" + feetypes "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/types" ibctesting "github.com/cosmos/ibc-go/v7/testing" ) diff --git a/modules/apps/callbacks/ibc_middleware.go b/modules/apps/callbacks/ibc_middleware.go index bce5e40969f..a93d3350451 100644 --- a/modules/apps/callbacks/ibc_middleware.go +++ b/modules/apps/callbacks/ibc_middleware.go @@ -5,8 +5,8 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" - capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types" "github.com/cosmos/ibc-go/modules/apps/callbacks/types" + capabilitytypes "github.com/cosmos/ibc-go/modules/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" diff --git a/modules/apps/callbacks/ica_test.go b/modules/apps/callbacks/ica_test.go index 4cd39bed87a..600f278fccf 100644 --- a/modules/apps/callbacks/ica_test.go +++ b/modules/apps/callbacks/ica_test.go @@ -11,10 +11,10 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + "github.com/cosmos/ibc-go/modules/apps/callbacks/types" icacontrollertypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/controller/types" icahosttypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/host/types" icatypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/types" - "github.com/cosmos/ibc-go/modules/apps/callbacks/types" ibctesting "github.com/cosmos/ibc-go/v7/testing" ) diff --git a/modules/apps/callbacks/testing/simapp/app.go b/modules/apps/callbacks/testing/simapp/app.go index 6d9b61f75e4..7d1d6013b6b 100644 --- a/modules/apps/callbacks/testing/simapp/app.go +++ b/modules/apps/callbacks/testing/simapp/app.go @@ -96,6 +96,8 @@ import ( abci "github.com/cometbft/cometbft/abci/types" "github.com/cometbft/cometbft/libs/log" + ibccallbacks "github.com/cosmos/ibc-go/modules/apps/callbacks" + simappparams "github.com/cosmos/ibc-go/modules/apps/callbacks/testing/simapp/params" "github.com/cosmos/ibc-go/modules/capability" capabilitykeeper "github.com/cosmos/ibc-go/modules/capability/keeper" capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types" @@ -110,8 +112,6 @@ import ( ibcfee "github.com/cosmos/ibc-go/v7/modules/apps/29-fee" ibcfeekeeper "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/keeper" ibcfeetypes "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/types" - ibccallbacks "github.com/cosmos/ibc-go/modules/apps/callbacks" - simappparams "github.com/cosmos/ibc-go/modules/apps/callbacks/testing/simapp/params" transfer "github.com/cosmos/ibc-go/v7/modules/apps/transfer" ibctransferkeeper "github.com/cosmos/ibc-go/v7/modules/apps/transfer/keeper" ibctransfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" From 08747c8fb9f62c4bd71c20500fee0659e1a62a59 Mon Sep 17 00:00:00 2001 From: srdtrk Date: Mon, 14 Aug 2023 16:14:59 +0300 Subject: [PATCH 2/8] imp(callbacks): removed unused packet param from processCallback --- modules/apps/callbacks/export_test.go | 5 ++--- modules/apps/callbacks/ibc_middleware.go | 12 ++++++------ modules/apps/callbacks/ibc_middleware_test.go | 7 +------ 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/modules/apps/callbacks/export_test.go b/modules/apps/callbacks/export_test.go index fb4bba06bbc..653d5ccddcc 100644 --- a/modules/apps/callbacks/export_test.go +++ b/modules/apps/callbacks/export_test.go @@ -8,16 +8,15 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/ibc-go/modules/apps/callbacks/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" ) // ProcessCallback is a wrapper around processCallback to allow the function to be directly called in tests. func (im IBCMiddleware) ProcessCallback( - ctx sdk.Context, packet channeltypes.Packet, callbackType types.CallbackType, + ctx sdk.Context, callbackType types.CallbackType, callbackData types.CallbackData, callbackExecutor func(sdk.Context) error, ) error { - return im.processCallback(ctx, packet, callbackType, callbackData, callbackExecutor) + return im.processCallback(ctx, callbackType, callbackData, callbackExecutor) } // GetICS4Wrapper is a getter for the IBCMiddleware's ICS4Wrapper. diff --git a/modules/apps/callbacks/ibc_middleware.go b/modules/apps/callbacks/ibc_middleware.go index a93d3350451..083db7c1765 100644 --- a/modules/apps/callbacks/ibc_middleware.go +++ b/modules/apps/callbacks/ibc_middleware.go @@ -105,7 +105,7 @@ func (im IBCMiddleware) SendPacket( ) } - err = im.processCallback(ctx, reconstructedPacket, types.CallbackTypeSendPacket, callbackData, callbackExecutor) + err = im.processCallback(ctx, types.CallbackTypeSendPacket, callbackData, callbackExecutor) // contract keeper is allowed to reject the packet send. if err != nil { return 0, err @@ -144,7 +144,7 @@ func (im IBCMiddleware) OnAcknowledgementPacket( } // callback execution errors are not allowed to block the packet lifecycle, they are only used in event emissions - err = im.processCallback(ctx, packet, types.CallbackTypeAcknowledgementPacket, callbackData, callbackExecutor) + err = im.processCallback(ctx, types.CallbackTypeAcknowledgementPacket, callbackData, callbackExecutor) types.EmitCallbackEvent(ctx, packet, types.CallbackTypeAcknowledgementPacket, callbackData, err) return nil @@ -171,7 +171,7 @@ func (im IBCMiddleware) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Pac } // callback execution errors are not allowed to block the packet lifecycle, they are only used in event emissions - err = im.processCallback(ctx, packet, types.CallbackTypeTimeoutPacket, callbackData, callbackExecutor) + err = im.processCallback(ctx, types.CallbackTypeTimeoutPacket, callbackData, callbackExecutor) types.EmitCallbackEvent(ctx, packet, types.CallbackTypeTimeoutPacket, callbackData, err) return nil @@ -202,7 +202,7 @@ func (im IBCMiddleware) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet } // callback execution errors are not allowed to block the packet lifecycle, they are only used in event emissions - err = im.processCallback(ctx, packet, types.CallbackTypeReceivePacket, callbackData, callbackExecutor) + err = im.processCallback(ctx, types.CallbackTypeReceivePacket, callbackData, callbackExecutor) types.EmitCallbackEvent(ctx, packet, types.CallbackTypeReceivePacket, callbackData, err) return ack @@ -235,7 +235,7 @@ func (im IBCMiddleware) WriteAcknowledgement( } // callback execution errors are not allowed to block the packet lifecycle, they are only used in event emissions - err = im.processCallback(ctx, packet, types.CallbackTypeReceivePacket, callbackData, callbackExecutor) + err = im.processCallback(ctx, types.CallbackTypeReceivePacket, callbackData, callbackExecutor) types.EmitCallbackEvent(ctx, packet, types.CallbackTypeReceivePacket, callbackData, err) return nil @@ -248,7 +248,7 @@ func (im IBCMiddleware) WriteAcknowledgement( // - the contractExecutor runs out of gas and the relayer has not reserved gas grater than or equal to // CommitGasLimit. func (IBCMiddleware) processCallback( - ctx sdk.Context, packet ibcexported.PacketI, callbackType types.CallbackType, + ctx sdk.Context, callbackType types.CallbackType, callbackData types.CallbackData, callbackExecutor func(sdk.Context) error, ) (err error) { cachedCtx, writeFn := ctx.CacheContext() diff --git a/modules/apps/callbacks/ibc_middleware_test.go b/modules/apps/callbacks/ibc_middleware_test.go index 47f649b2282..83383eee239 100644 --- a/modules/apps/callbacks/ibc_middleware_test.go +++ b/modules/apps/callbacks/ibc_middleware_test.go @@ -820,11 +820,6 @@ func (s *CallbacksTestSuite) TestProcessCallback() { s.Run(tc.name, func() { s.SetupMockFeeTest() - // set mock packet, it is only used in logs and not in callback execution - mockPacket := channeltypes.NewPacket( - ibcmock.MockPacketData, 1, s.path.EndpointA.ChannelConfig.PortID, s.path.EndpointA.ChannelID, - s.path.EndpointB.ChannelConfig.PortID, s.path.EndpointB.ChannelID, clienttypes.NewHeight(0, 100), 0) - // set a callback data that does not allow retry callbackData = types.CallbackData{ CallbackAddress: s.chainB.SenderAccount.GetAddress().String(), @@ -853,7 +848,7 @@ func (s *CallbacksTestSuite) TestProcessCallback() { s.Require().True(ok) processCallback := func() { - err = mockCallbackStack.ProcessCallback(ctx, mockPacket, callbackType, callbackData, callbackExecutor) + err = mockCallbackStack.ProcessCallback(ctx, callbackType, callbackData, callbackExecutor) } expPass := tc.expValue == nil From 1070a915bc51475b89e8bc4e9749ec17467baf22 Mon Sep 17 00:00:00 2001 From: srdtrk Date: Mon, 14 Aug 2023 16:36:30 +0300 Subject: [PATCH 3/8] imp(callbacks): removed packet from event functions --- modules/apps/callbacks/ibc_middleware.go | 22 ++++++++++++++++----- modules/apps/callbacks/types/events.go | 16 +++++++-------- modules/apps/callbacks/types/events_test.go | 13 +++++++++++- 3 files changed, 37 insertions(+), 14 deletions(-) diff --git a/modules/apps/callbacks/ibc_middleware.go b/modules/apps/callbacks/ibc_middleware.go index 083db7c1765..9147a4ab17f 100644 --- a/modules/apps/callbacks/ibc_middleware.go +++ b/modules/apps/callbacks/ibc_middleware.go @@ -111,7 +111,7 @@ func (im IBCMiddleware) SendPacket( return 0, err } - types.EmitCallbackEvent(ctx, reconstructedPacket, types.CallbackTypeSendPacket, callbackData, nil) + types.EmitCallbackEvent(ctx, sourceChannel, sourcePort, seq, types.CallbackTypeSendPacket, callbackData, nil) return seq, nil } @@ -145,7 +145,10 @@ func (im IBCMiddleware) OnAcknowledgementPacket( // callback execution errors are not allowed to block the packet lifecycle, they are only used in event emissions err = im.processCallback(ctx, types.CallbackTypeAcknowledgementPacket, callbackData, callbackExecutor) - types.EmitCallbackEvent(ctx, packet, types.CallbackTypeAcknowledgementPacket, callbackData, err) + types.EmitCallbackEvent( + ctx, packet.GetSourceChannel(), packet.GetSourcePort(), packet.GetSequence(), + types.CallbackTypeAcknowledgementPacket, callbackData, err, + ) return nil } @@ -172,7 +175,10 @@ func (im IBCMiddleware) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Pac // callback execution errors are not allowed to block the packet lifecycle, they are only used in event emissions err = im.processCallback(ctx, types.CallbackTypeTimeoutPacket, callbackData, callbackExecutor) - types.EmitCallbackEvent(ctx, packet, types.CallbackTypeTimeoutPacket, callbackData, err) + types.EmitCallbackEvent( + ctx, packet.GetSourceChannel(), packet.GetSourcePort(), packet.GetSequence(), + types.CallbackTypeTimeoutPacket, callbackData, err, + ) return nil } @@ -203,7 +209,10 @@ func (im IBCMiddleware) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet // callback execution errors are not allowed to block the packet lifecycle, they are only used in event emissions err = im.processCallback(ctx, types.CallbackTypeReceivePacket, callbackData, callbackExecutor) - types.EmitCallbackEvent(ctx, packet, types.CallbackTypeReceivePacket, callbackData, err) + types.EmitCallbackEvent( + ctx, packet.GetDestChannel(), packet.GetDestPort(), packet.GetSequence(), + types.CallbackTypeReceivePacket, callbackData, err, + ) return ack } @@ -236,7 +245,10 @@ func (im IBCMiddleware) WriteAcknowledgement( // callback execution errors are not allowed to block the packet lifecycle, they are only used in event emissions err = im.processCallback(ctx, types.CallbackTypeReceivePacket, callbackData, callbackExecutor) - types.EmitCallbackEvent(ctx, packet, types.CallbackTypeReceivePacket, callbackData, err) + types.EmitCallbackEvent( + ctx, packet.GetDestChannel(), packet.GetDestPort(), packet.GetSequence(), + types.CallbackTypeReceivePacket, callbackData, err, + ) return nil } diff --git a/modules/apps/callbacks/types/events.go b/modules/apps/callbacks/types/events.go index ecf255047d1..6ac0e0bcb10 100644 --- a/modules/apps/callbacks/types/events.go +++ b/modules/apps/callbacks/types/events.go @@ -4,8 +4,6 @@ import ( "fmt" sdk "github.com/cosmos/cosmos-sdk/types" - - ibcexported "github.com/cosmos/ibc-go/v7/modules/core/exported" ) const ( @@ -54,7 +52,9 @@ const ( // EmitCallbackEvent emits an event for a callback func EmitCallbackEvent( ctx sdk.Context, - packet ibcexported.PacketI, + channelID, + portID string, + sequence uint64, callbackType CallbackType, callbackData CallbackData, err error, @@ -65,7 +65,7 @@ func EmitCallbackEvent( sdk.NewAttribute(AttributeKeyCallbackAddress, callbackData.CallbackAddress), sdk.NewAttribute(AttributeKeyCallbackGasLimit, fmt.Sprintf("%d", callbackData.ExecutionGasLimit)), sdk.NewAttribute(AttributeKeyCallbackCommitGasLimit, fmt.Sprintf("%d", callbackData.CommitGasLimit)), - sdk.NewAttribute(AttributeKeyCallbackSequence, fmt.Sprintf("%d", packet.GetSequence())), + sdk.NewAttribute(AttributeKeyCallbackSequence, fmt.Sprintf("%d", sequence)), } if err == nil { attributes = append(attributes, sdk.NewAttribute(AttributeKeyCallbackResult, AttributeValueCallbackSuccess)) @@ -82,14 +82,14 @@ func EmitCallbackEvent( case CallbackTypeReceivePacket: eventType = EventTypeDestinationCallback attributes = append( - attributes, sdk.NewAttribute(AttributeKeyCallbackDestPortID, packet.GetDestPort()), - sdk.NewAttribute(AttributeKeyCallbackDestChannelID, packet.GetDestChannel()), + attributes, sdk.NewAttribute(AttributeKeyCallbackDestPortID, portID), + sdk.NewAttribute(AttributeKeyCallbackDestChannelID, channelID), ) default: eventType = EventTypeSourceCallback attributes = append( - attributes, sdk.NewAttribute(AttributeKeyCallbackSourcePortID, packet.GetSourcePort()), - sdk.NewAttribute(AttributeKeyCallbackSourceChannelID, packet.GetSourceChannel()), + attributes, sdk.NewAttribute(AttributeKeyCallbackSourcePortID, portID), + sdk.NewAttribute(AttributeKeyCallbackSourceChannelID, channelID), ) } diff --git a/modules/apps/callbacks/types/events_test.go b/modules/apps/callbacks/types/events_test.go index 0638aa79884..edcbb00ab5f 100644 --- a/modules/apps/callbacks/types/events_test.go +++ b/modules/apps/callbacks/types/events_test.go @@ -186,7 +186,18 @@ func (s *CallbacksTypesTestSuite) TestEvents() { for _, tc := range testCases { newCtx := sdk.Context{}.WithEventManager(sdk.NewEventManager()) - types.EmitCallbackEvent(newCtx, tc.packet, tc.callbackType, tc.callbackData, tc.callbackError) + switch tc.callbackType { + case types.CallbackTypeReceivePacket: + types.EmitCallbackEvent( + newCtx, tc.packet.GetDestChannel(), tc.packet.GetDestPort(), + tc.packet.GetSequence(), tc.callbackType, tc.callbackData, tc.callbackError, + ) + default: + types.EmitCallbackEvent( + newCtx, tc.packet.GetSourceChannel(), tc.packet.GetSourcePort(), + tc.packet.GetSequence(), tc.callbackType, tc.callbackData, tc.callbackError, + ) + } events := newCtx.EventManager().Events().ToABCIEvents() ibctesting.AssertEvents(&s.Suite, tc.expEvents, events) } From 6210a9f355fff4feee3f2f28fc7e1a1a40b0018f Mon Sep 17 00:00:00 2001 From: srdtrk Date: Mon, 14 Aug 2023 16:46:01 +0300 Subject: [PATCH 4/8] imp(callbacks): removed packet from callbackDataGetter functions --- modules/apps/callbacks/ibc_middleware.go | 22 +++++++++++-------- modules/apps/callbacks/types/callbacks.go | 14 ++++++------ .../apps/callbacks/types/callbacks_test.go | 10 +++------ modules/apps/callbacks/types/export_test.go | 5 ++--- 4 files changed, 25 insertions(+), 26 deletions(-) diff --git a/modules/apps/callbacks/ibc_middleware.go b/modules/apps/callbacks/ibc_middleware.go index 9147a4ab17f..fb89b03b7d6 100644 --- a/modules/apps/callbacks/ibc_middleware.go +++ b/modules/apps/callbacks/ibc_middleware.go @@ -89,11 +89,7 @@ func (im IBCMiddleware) SendPacket( return 0, err } - // Reconstruct the sent packet. The destination portID and channelID are intentionally left empty as the sender information - // is only derived from the source packet information in `GetSourceCallbackData`. - reconstructedPacket := channeltypes.NewPacket(data, seq, sourcePort, sourceChannel, "", "", timeoutHeight, timeoutTimestamp) - - callbackData, err := types.GetSourceCallbackData(im.app, reconstructedPacket, ctx.GasMeter().GasRemaining(), im.maxCallbackGas) + callbackData, err := types.GetSourceCallbackData(im.app, data, sourcePort, ctx.GasMeter().GasRemaining(), im.maxCallbackGas) // SendPacket is not blocked if the packet does not opt-in to callbacks if err != nil { return seq, nil @@ -131,7 +127,9 @@ func (im IBCMiddleware) OnAcknowledgementPacket( return err } - callbackData, err := types.GetSourceCallbackData(im.app, packet, ctx.GasMeter().GasRemaining(), im.maxCallbackGas) + callbackData, err := types.GetSourceCallbackData( + im.app, packet.GetData(), packet.GetSourcePort(), ctx.GasMeter().GasRemaining(), im.maxCallbackGas, + ) // OnAcknowledgementPacket is not blocked if the packet does not opt-in to callbacks if err != nil { return nil @@ -163,7 +161,9 @@ func (im IBCMiddleware) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Pac return err } - callbackData, err := types.GetSourceCallbackData(im.app, packet, ctx.GasMeter().GasRemaining(), im.maxCallbackGas) + callbackData, err := types.GetSourceCallbackData( + im.app, packet.GetData(), packet.GetSourcePort(), ctx.GasMeter().GasRemaining(), im.maxCallbackGas, + ) // OnTimeoutPacket is not blocked if the packet does not opt-in to callbacks if err != nil { return nil @@ -197,7 +197,9 @@ func (im IBCMiddleware) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet return ack } - callbackData, err := types.GetDestCallbackData(im.app, packet, ctx.GasMeter().GasRemaining(), im.maxCallbackGas) + callbackData, err := types.GetDestCallbackData( + im.app, packet.GetData(), packet.GetSourcePort(), ctx.GasMeter().GasRemaining(), im.maxCallbackGas, + ) // OnRecvPacket is not blocked if the packet does not opt-in to callbacks if err != nil { return ack @@ -233,7 +235,9 @@ func (im IBCMiddleware) WriteAcknowledgement( return err } - callbackData, err := types.GetDestCallbackData(im.app, packet, ctx.GasMeter().GasRemaining(), im.maxCallbackGas) + callbackData, err := types.GetDestCallbackData( + im.app, packet.GetData(), packet.GetSourcePort(), ctx.GasMeter().GasRemaining(), im.maxCallbackGas, + ) // WriteAcknowledgement is not blocked if the packet does not opt-in to callbacks if err != nil { return nil diff --git a/modules/apps/callbacks/types/callbacks.go b/modules/apps/callbacks/types/callbacks.go index a868245422a..90e34cb68f7 100644 --- a/modules/apps/callbacks/types/callbacks.go +++ b/modules/apps/callbacks/types/callbacks.go @@ -68,17 +68,17 @@ type CallbackData struct { // GetSourceCallbackData parses the packet data and returns the source callback data. func GetSourceCallbackData( packetDataUnmarshaler porttypes.PacketDataUnmarshaler, - packet ibcexported.PacketI, remainingGas uint64, maxGas uint64, + packetData []byte, srcPortID string, remainingGas uint64, maxGas uint64, ) (CallbackData, error) { - return getCallbackData(packetDataUnmarshaler, packet, remainingGas, maxGas, SourceCallbackKey) + return getCallbackData(packetDataUnmarshaler, packetData, srcPortID, remainingGas, maxGas, SourceCallbackKey) } // GetDestCallbackData parses the packet data and returns the destination callback data. func GetDestCallbackData( packetDataUnmarshaler porttypes.PacketDataUnmarshaler, - packet ibcexported.PacketI, remainingGas uint64, maxGas uint64, + packetData []byte, srcPortID string, remainingGas, maxGas uint64, ) (CallbackData, error) { - return getCallbackData(packetDataUnmarshaler, packet, remainingGas, maxGas, DestinationCallbackKey) + return getCallbackData(packetDataUnmarshaler, packetData, srcPortID, remainingGas, maxGas, DestinationCallbackKey) } // getCallbackData parses the packet data and returns the callback data. @@ -87,11 +87,11 @@ func GetDestCallbackData( // address and gas limit from the callback data. func getCallbackData( packetDataUnmarshaler porttypes.PacketDataUnmarshaler, - packet ibcexported.PacketI, remainingGas, + packetData []byte, srcPortID string, remainingGas, maxGas uint64, callbackKey string, ) (CallbackData, error) { // unmarshal packet data - unmarshaledData, err := packetDataUnmarshaler.UnmarshalPacketData(packet.GetData()) + unmarshaledData, err := packetDataUnmarshaler.UnmarshalPacketData(packetData) if err != nil { return CallbackData{}, errorsmod.Wrap(ErrCannotUnmarshalPacketData, err.Error()) } @@ -117,7 +117,7 @@ func getCallbackData( if callbackKey == SourceCallbackKey { packetData, ok := unmarshaledData.(ibcexported.PacketData) if ok { - packetSender = packetData.GetPacketSender(packet.GetSourcePort()) + packetSender = packetData.GetPacketSender(srcPortID) } } diff --git a/modules/apps/callbacks/types/callbacks_test.go b/modules/apps/callbacks/types/callbacks_test.go index 554178d680c..4535127d7de 100644 --- a/modules/apps/callbacks/types/callbacks_test.go +++ b/modules/apps/callbacks/types/callbacks_test.go @@ -10,7 +10,6 @@ import ( "github.com/cosmos/ibc-go/modules/apps/callbacks/types" transfer "github.com/cosmos/ibc-go/v7/modules/apps/transfer" transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/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" ibctesting "github.com/cosmos/ibc-go/v7/testing" ibcmock "github.com/cosmos/ibc-go/v7/testing/mock" @@ -278,8 +277,7 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { tc.malleate() - testPacket := channeltypes.Packet{Data: packetData} - callbackData, err := types.GetCallbackData(packetDataUnmarshaler, testPacket, remainingGas, uint64(1_000_000), callbackKey) + callbackData, err := types.GetCallbackData(packetDataUnmarshaler, packetData, "", remainingGas, uint64(1_000_000), callbackKey) expPass := tc.expError == nil if expPass { @@ -317,8 +315,7 @@ func (s *CallbacksTypesTestSuite) TestGetSourceCallbackDataTransfer() { packetUnmarshaler := transfer.IBCModule{} - testPacket := channeltypes.Packet{Data: packetDataBytes} - callbackData, err := types.GetSourceCallbackData(packetUnmarshaler, testPacket, 2_000_000, 1_000_000) + callbackData, err := types.GetSourceCallbackData(packetUnmarshaler, packetDataBytes, "", 2_000_000, 1_000_000) s.Require().NoError(err) s.Require().Equal(expCallbackData, callbackData) } @@ -345,8 +342,7 @@ func (s *CallbacksTypesTestSuite) TestGetDestCallbackDataTransfer() { packetUnmarshaler := transfer.IBCModule{} - testPacket := channeltypes.Packet{Data: packetDataBytes} - callbackData, err := types.GetDestCallbackData(packetUnmarshaler, testPacket, 2_000_000, 1_000_000) + callbackData, err := types.GetDestCallbackData(packetUnmarshaler, packetDataBytes, "", 2_000_000, 1_000_000) s.Require().NoError(err) s.Require().Equal(expCallbackData, callbackData) } diff --git a/modules/apps/callbacks/types/export_test.go b/modules/apps/callbacks/types/export_test.go index facb96952f6..b87550dd34a 100644 --- a/modules/apps/callbacks/types/export_test.go +++ b/modules/apps/callbacks/types/export_test.go @@ -2,7 +2,6 @@ package types import ( porttypes "github.com/cosmos/ibc-go/v7/modules/core/05-port/types" - ibcexported "github.com/cosmos/ibc-go/v7/modules/core/exported" ) /* @@ -12,10 +11,10 @@ import ( // GetCallbackData is a wrapper around getCallbackData to allow the function to be directly called in tests. func GetCallbackData( packetDataUnmarshaler porttypes.PacketDataUnmarshaler, - packet ibcexported.PacketI, remainingGas uint64, + packetData []byte, srcPortID string, remainingGas, maxGas uint64, callbackKey string, ) (CallbackData, error) { - return getCallbackData(packetDataUnmarshaler, packet, remainingGas, maxGas, callbackKey) + return getCallbackData(packetDataUnmarshaler, packetData, srcPortID, remainingGas, maxGas, callbackKey) } // GetCallbackAddress is a wrapper around getCallbackAddress to allow the function to be directly called in tests. From bd5896d30c401c57c6ae84c1c55c882797407145 Mon Sep 17 00:00:00 2001 From: srdtrk Date: Mon, 14 Aug 2023 17:58:54 +0300 Subject: [PATCH 5/8] style(callbacks): reorder func arguments for more consistency --- modules/apps/callbacks/types/events.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/apps/callbacks/types/events.go b/modules/apps/callbacks/types/events.go index 6ac0e0bcb10..b05fc96faa8 100644 --- a/modules/apps/callbacks/types/events.go +++ b/modules/apps/callbacks/types/events.go @@ -52,8 +52,8 @@ const ( // EmitCallbackEvent emits an event for a callback func EmitCallbackEvent( ctx sdk.Context, - channelID, - portID string, + portID, + channelID string, sequence uint64, callbackType CallbackType, callbackData CallbackData, From ed295fbcb425d41a5356fb7489c9138b7fa6d65c Mon Sep 17 00:00:00 2001 From: srdtrk Date: Mon, 14 Aug 2023 18:01:12 +0300 Subject: [PATCH 6/8] imp(callbacks_test): using ibcmock.PortID for testing instead of empty string --- modules/apps/callbacks/types/callbacks_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/apps/callbacks/types/callbacks_test.go b/modules/apps/callbacks/types/callbacks_test.go index 4535127d7de..7faac7f9a23 100644 --- a/modules/apps/callbacks/types/callbacks_test.go +++ b/modules/apps/callbacks/types/callbacks_test.go @@ -277,7 +277,7 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { tc.malleate() - callbackData, err := types.GetCallbackData(packetDataUnmarshaler, packetData, "", remainingGas, uint64(1_000_000), callbackKey) + callbackData, err := types.GetCallbackData(packetDataUnmarshaler, packetData, ibcmock.PortID, remainingGas, uint64(1_000_000), callbackKey) expPass := tc.expError == nil if expPass { @@ -315,7 +315,7 @@ func (s *CallbacksTypesTestSuite) TestGetSourceCallbackDataTransfer() { packetUnmarshaler := transfer.IBCModule{} - callbackData, err := types.GetSourceCallbackData(packetUnmarshaler, packetDataBytes, "", 2_000_000, 1_000_000) + callbackData, err := types.GetSourceCallbackData(packetUnmarshaler, packetDataBytes, ibcmock.PortID, 2_000_000, 1_000_000) s.Require().NoError(err) s.Require().Equal(expCallbackData, callbackData) } @@ -342,7 +342,7 @@ func (s *CallbacksTypesTestSuite) TestGetDestCallbackDataTransfer() { packetUnmarshaler := transfer.IBCModule{} - callbackData, err := types.GetDestCallbackData(packetUnmarshaler, packetDataBytes, "", 2_000_000, 1_000_000) + callbackData, err := types.GetDestCallbackData(packetUnmarshaler, packetDataBytes, ibcmock.PortID, 2_000_000, 1_000_000) s.Require().NoError(err) s.Require().Equal(expCallbackData, callbackData) } From ddac0270c4d7f7399cd2ae50a90a4cac42786e90 Mon Sep 17 00:00:00 2001 From: srdtrk Date: Mon, 14 Aug 2023 18:11:08 +0300 Subject: [PATCH 7/8] style(callbacks): renamed packetData to data in 'GetCallbackData' functions --- modules/apps/callbacks/types/callbacks.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/modules/apps/callbacks/types/callbacks.go b/modules/apps/callbacks/types/callbacks.go index 90e34cb68f7..7d18259af29 100644 --- a/modules/apps/callbacks/types/callbacks.go +++ b/modules/apps/callbacks/types/callbacks.go @@ -68,17 +68,17 @@ type CallbackData struct { // GetSourceCallbackData parses the packet data and returns the source callback data. func GetSourceCallbackData( packetDataUnmarshaler porttypes.PacketDataUnmarshaler, - packetData []byte, srcPortID string, remainingGas uint64, maxGas uint64, + data []byte, srcPortID string, remainingGas uint64, maxGas uint64, ) (CallbackData, error) { - return getCallbackData(packetDataUnmarshaler, packetData, srcPortID, remainingGas, maxGas, SourceCallbackKey) + return getCallbackData(packetDataUnmarshaler, data, srcPortID, remainingGas, maxGas, SourceCallbackKey) } // GetDestCallbackData parses the packet data and returns the destination callback data. func GetDestCallbackData( packetDataUnmarshaler porttypes.PacketDataUnmarshaler, - packetData []byte, srcPortID string, remainingGas, maxGas uint64, + data []byte, srcPortID string, remainingGas, maxGas uint64, ) (CallbackData, error) { - return getCallbackData(packetDataUnmarshaler, packetData, srcPortID, remainingGas, maxGas, DestinationCallbackKey) + return getCallbackData(packetDataUnmarshaler, data, srcPortID, remainingGas, maxGas, DestinationCallbackKey) } // getCallbackData parses the packet data and returns the callback data. @@ -87,16 +87,16 @@ func GetDestCallbackData( // address and gas limit from the callback data. func getCallbackData( packetDataUnmarshaler porttypes.PacketDataUnmarshaler, - packetData []byte, srcPortID string, remainingGas, + data []byte, srcPortID string, remainingGas, maxGas uint64, callbackKey string, ) (CallbackData, error) { // unmarshal packet data - unmarshaledData, err := packetDataUnmarshaler.UnmarshalPacketData(packetData) + packetData, err := packetDataUnmarshaler.UnmarshalPacketData(data) if err != nil { return CallbackData{}, errorsmod.Wrap(ErrCannotUnmarshalPacketData, err.Error()) } - packetDataProvider, ok := unmarshaledData.(ibcexported.PacketDataProvider) + packetDataProvider, ok := packetData.(ibcexported.PacketDataProvider) if !ok { return CallbackData{}, ErrNotPacketDataProvider } @@ -115,7 +115,7 @@ func getCallbackData( // retrieve packet sender from packet data if possible and if needed var packetSender string if callbackKey == SourceCallbackKey { - packetData, ok := unmarshaledData.(ibcexported.PacketData) + packetData, ok := packetData.(ibcexported.PacketData) if ok { packetSender = packetData.GetPacketSender(srcPortID) } From ae81073a018f4738663a9a5a208f20c5bb244b50 Mon Sep 17 00:00:00 2001 From: srdtrk Date: Mon, 14 Aug 2023 18:18:27 +0300 Subject: [PATCH 8/8] fix(callbacks): reordered EmitCallbackEvents parameters during usage --- modules/apps/callbacks/ibc_middleware.go | 10 +++---- modules/apps/callbacks/types/events_test.go | 33 +++++++++++---------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/modules/apps/callbacks/ibc_middleware.go b/modules/apps/callbacks/ibc_middleware.go index fb89b03b7d6..be61c50d751 100644 --- a/modules/apps/callbacks/ibc_middleware.go +++ b/modules/apps/callbacks/ibc_middleware.go @@ -107,7 +107,7 @@ func (im IBCMiddleware) SendPacket( return 0, err } - types.EmitCallbackEvent(ctx, sourceChannel, sourcePort, seq, types.CallbackTypeSendPacket, callbackData, nil) + types.EmitCallbackEvent(ctx, sourcePort, sourceChannel, seq, types.CallbackTypeSendPacket, callbackData, nil) return seq, nil } @@ -144,7 +144,7 @@ func (im IBCMiddleware) OnAcknowledgementPacket( // callback execution errors are not allowed to block the packet lifecycle, they are only used in event emissions err = im.processCallback(ctx, types.CallbackTypeAcknowledgementPacket, callbackData, callbackExecutor) types.EmitCallbackEvent( - ctx, packet.GetSourceChannel(), packet.GetSourcePort(), packet.GetSequence(), + ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence(), types.CallbackTypeAcknowledgementPacket, callbackData, err, ) @@ -176,7 +176,7 @@ func (im IBCMiddleware) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Pac // callback execution errors are not allowed to block the packet lifecycle, they are only used in event emissions err = im.processCallback(ctx, types.CallbackTypeTimeoutPacket, callbackData, callbackExecutor) types.EmitCallbackEvent( - ctx, packet.GetSourceChannel(), packet.GetSourcePort(), packet.GetSequence(), + ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence(), types.CallbackTypeTimeoutPacket, callbackData, err, ) @@ -212,7 +212,7 @@ func (im IBCMiddleware) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet // callback execution errors are not allowed to block the packet lifecycle, they are only used in event emissions err = im.processCallback(ctx, types.CallbackTypeReceivePacket, callbackData, callbackExecutor) types.EmitCallbackEvent( - ctx, packet.GetDestChannel(), packet.GetDestPort(), packet.GetSequence(), + ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(), types.CallbackTypeReceivePacket, callbackData, err, ) @@ -250,7 +250,7 @@ func (im IBCMiddleware) WriteAcknowledgement( // callback execution errors are not allowed to block the packet lifecycle, they are only used in event emissions err = im.processCallback(ctx, types.CallbackTypeReceivePacket, callbackData, callbackExecutor) types.EmitCallbackEvent( - ctx, packet.GetDestChannel(), packet.GetDestPort(), packet.GetSequence(), + ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(), types.CallbackTypeReceivePacket, callbackData, err, ) diff --git a/modules/apps/callbacks/types/events_test.go b/modules/apps/callbacks/types/events_test.go index edcbb00ab5f..ffc99b94454 100644 --- a/modules/apps/callbacks/types/events_test.go +++ b/modules/apps/callbacks/types/events_test.go @@ -184,21 +184,24 @@ func (s *CallbacksTypesTestSuite) TestEvents() { } for _, tc := range testCases { - newCtx := sdk.Context{}.WithEventManager(sdk.NewEventManager()) + tc := tc + s.Run(tc.name, func() { + newCtx := sdk.Context{}.WithEventManager(sdk.NewEventManager()) - switch tc.callbackType { - case types.CallbackTypeReceivePacket: - types.EmitCallbackEvent( - newCtx, tc.packet.GetDestChannel(), tc.packet.GetDestPort(), - tc.packet.GetSequence(), tc.callbackType, tc.callbackData, tc.callbackError, - ) - default: - types.EmitCallbackEvent( - newCtx, tc.packet.GetSourceChannel(), tc.packet.GetSourcePort(), - tc.packet.GetSequence(), tc.callbackType, tc.callbackData, tc.callbackError, - ) - } - events := newCtx.EventManager().Events().ToABCIEvents() - ibctesting.AssertEvents(&s.Suite, tc.expEvents, events) + switch tc.callbackType { + case types.CallbackTypeReceivePacket: + types.EmitCallbackEvent( + newCtx, tc.packet.GetDestPort(), tc.packet.GetDestChannel(), + tc.packet.GetSequence(), tc.callbackType, tc.callbackData, tc.callbackError, + ) + default: + types.EmitCallbackEvent( + newCtx, tc.packet.GetSourcePort(), tc.packet.GetSourceChannel(), + tc.packet.GetSequence(), tc.callbackType, tc.callbackData, tc.callbackError, + ) + } + events := newCtx.EventManager().Events().ToABCIEvents() + ibctesting.AssertEvents(&s.Suite, tc.expEvents, events) + }) } }