From 1528eda222469d135d0002e9033332b35d50146b Mon Sep 17 00:00:00 2001 From: DimitrisJim Date: Wed, 22 May 2024 12:53:34 +0300 Subject: [PATCH] review: address feedback. --- modules/apps/29-fee/ibc_middleware_test.go | 1 + modules/apps/callbacks/ibc_middleware.go | 2 ++ modules/apps/callbacks/ibc_middleware_test.go | 7 ++++-- .../apps/callbacks/types/callbacks_test.go | 11 +++++++++- modules/apps/callbacks/types/export_test.go | 22 ------------------- 5 files changed, 18 insertions(+), 25 deletions(-) diff --git a/modules/apps/29-fee/ibc_middleware_test.go b/modules/apps/29-fee/ibc_middleware_test.go index ac19ace089a..dc59f277d3f 100644 --- a/modules/apps/29-fee/ibc_middleware_test.go +++ b/modules/apps/29-fee/ibc_middleware_test.go @@ -1573,6 +1573,7 @@ func (suite *FeeTestSuite) TestPacketDataUnmarshalerInterface() { feeModule, ok := cbs.(porttypes.PacketDataUnmarshaler) suite.Require().True(ok) + // Context, port identifier, channel identifier are not used in current wiring of fee. packetData, err := feeModule.UnmarshalPacketData(suite.chainA.GetContext(), "", "", ibcmock.MockPacketData) suite.Require().NoError(err) suite.Require().Equal(ibcmock.MockPacketData, packetData) diff --git a/modules/apps/callbacks/ibc_middleware.go b/modules/apps/callbacks/ibc_middleware.go index 768e43371b0..816305913d8 100644 --- a/modules/apps/callbacks/ibc_middleware.go +++ b/modules/apps/callbacks/ibc_middleware.go @@ -99,7 +99,9 @@ func (im IBCMiddleware) SendPacket( return 0, err } + // packet is created withouth destination information present, GetSourceCallbackData does not use these. packet := channeltypes.NewPacket(data, seq, sourcePort, sourceChannel, "", "", timeoutHeight, timeoutTimestamp) + callbackData, err := types.GetSourceCallbackData(ctx, im.app, packet, im.maxCallbackGas) // SendPacket is not blocked if the packet does not opt-in to callbacks if err != nil { diff --git a/modules/apps/callbacks/ibc_middleware_test.go b/modules/apps/callbacks/ibc_middleware_test.go index 286f58684d1..63ba76be482 100644 --- a/modules/apps/callbacks/ibc_middleware_test.go +++ b/modules/apps/callbacks/ibc_middleware_test.go @@ -1003,15 +1003,18 @@ func (s *CallbacksTestSuite) TestUnmarshalPacketData() { Memo: fmt.Sprintf(`{"src_callback": {"address": "%s"}, "dest_callback": {"address":"%s"}}`, ibctesting.TestAccAddress, ibctesting.TestAccAddress), } + portID := s.path.EndpointA.ChannelConfig.PortID + channelID := s.path.EndpointA.ChannelID + // Unmarshal ICS20 v1 packet data data := expPacketDataICS20V1.GetBytes() - packetData, err := unmarshalerStack.UnmarshalPacketData(s.chainA.GetContext(), "", "", data) + packetData, err := unmarshalerStack.UnmarshalPacketData(s.chainA.GetContext(), portID, channelID, data) s.Require().NoError(err) s.Require().Equal(expPacketDataICS20V2, packetData) // Unmarshal ICS20 v1 packet data data = expPacketDataICS20V2.GetBytes() - packetData, err = unmarshalerStack.UnmarshalPacketData(s.chainA.GetContext(), "", "", data) + packetData, err = unmarshalerStack.UnmarshalPacketData(s.chainA.GetContext(), portID, channelID, data) s.Require().NoError(err) s.Require().Equal(expPacketDataICS20V2, packetData) } diff --git a/modules/apps/callbacks/types/callbacks_test.go b/modules/apps/callbacks/types/callbacks_test.go index d3d4af4edc9..754118dda90 100644 --- a/modules/apps/callbacks/types/callbacks_test.go +++ b/modules/apps/callbacks/types/callbacks_test.go @@ -286,7 +286,16 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { ctx := s.chain.GetContext().WithGasMeter(gasMeter) packet := channeltypes.NewPacket(packetData, 0, ibcmock.PortID, "", "", "", clienttypes.ZeroHeight(), 0) - callbackData, err := types.GetCallbackData(ctx, packetDataUnmarshaler, packet, remainingGas, uint64(1_000_000), callbackKey) + + var ( + callbackData types.CallbackData + err error + ) + if callbackKey == types.DestinationCallbackKey { + callbackData, err = types.GetDestCallbackData(ctx, packetDataUnmarshaler, packet, uint64(1_000_000)) + } else { + callbackData, err = types.GetSourceCallbackData(ctx, packetDataUnmarshaler, packet, uint64(1_000_000)) + } expPass := tc.expError == nil if expPass { diff --git a/modules/apps/callbacks/types/export_test.go b/modules/apps/callbacks/types/export_test.go index e5d1a12a4f2..5b0a32508e3 100644 --- a/modules/apps/callbacks/types/export_test.go +++ b/modules/apps/callbacks/types/export_test.go @@ -1,31 +1,9 @@ package types -import ( - sdk "github.com/cosmos/cosmos-sdk/types" - - channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" - porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types" -) - /* This file is to allow for unexported functions to be accessible to the testing package. */ -// GetCallbackData is a wrapper around getCallbackData to allow the function to be directly called in tests. -func GetCallbackData( - ctx sdk.Context, - packetDataUnmarshaler porttypes.PacketDataUnmarshaler, - packet channeltypes.Packet, - remainingGas, - maxGas uint64, callbackKey string, -) (CallbackData, error) { - // TODO(jim): Probably just refactor single occurrence of this in tests - if callbackKey == DestinationCallbackKey { - return GetDestCallbackData(ctx, packetDataUnmarshaler, packet, maxGas) - } - return GetSourceCallbackData(ctx, packetDataUnmarshaler, packet, maxGas) -} - // GetCallbackAddress is a wrapper around getCallbackAddress to allow the function to be directly called in tests. func GetCallbackAddress(callbackData map[string]interface{}) string { return getCallbackAddress(callbackData)