use new port router with OnAcknowledgementPacket

modules/apps/27-interchain-accounts/controller/ibc_middleware.go

@@ -227,16 +227,6 @@ func (im IBCMiddleware) OnAcknowledgementPacket(
 		return types.ErrControllerSubModuleDisabled
 	}
 
-	connectionID, err := im.keeper.GetConnectionID(ctx, packet.GetSourcePort(), packet.GetSourceChannel())
-	if err != nil {
-		return err
-	}
-
-	// call underlying app's OnAcknowledgementPacket callback.
-	if im.app != nil && im.keeper.IsMiddlewareEnabled(ctx, packet.GetSourcePort(), connectionID) {
-		return im.app.OnAcknowledgementPacket(ctx, channelVersion, packet, acknowledgement, relayer)
-	}
-
 	return nil
 }
 
@@ -352,3 +342,9 @@ func (IBCMiddleware) UnwrapVersionUnsafe(version string) (string, string, error)
 	// ignore underlying app version
 	return version, "", nil
 }
+
+// UnwrapVersionSafe returns the version. Interchain accounts does not wrap versions.
+func (IBCMiddleware) UnwrapVersionSafe(ctx sdk.Context, portID, channelID, version string) (string, string) {
+	// ignore underlying app version
+	return version, ""
+}

modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go

@@ -515,50 +515,22 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() {
 }
 
 func (suite *InterchainAccountsTestSuite) TestOnAcknowledgementPacket() {
-	var (
-		path     *ibctesting.Path
-		isNilApp bool
-	)
+	var path *ibctesting.Path
 
 	testCases := []struct {
 		msg      string
 		malleate func()
-		expPass  bool
+		expError error
 	}{
 		{
 			"success",
 			func() {},
-			true,
+			nil,
 		},
 		{
 			"controller submodule disabled", func() {
 				suite.chainA.GetSimApp().ICAControllerKeeper.SetParams(suite.chainA.GetContext(), types.NewParams(false))
-			}, false,
-		},
-		{
-			"ICA auth module callback fails", func() {
-				suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnAcknowledgementPacket = func(
-					ctx sdk.Context, channelVersion string, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress,
-				) error {
-					return fmt.Errorf("mock ica auth fails")
-				}
-			}, false,
-		},
-		{
-			"nil underlying app", func() {
-				isNilApp = true
-			}, true,
-		},
-		{
-			"middleware disabled", func() {
-				suite.chainA.GetSimApp().ICAControllerKeeper.DeleteMiddlewareEnabled(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ConnectionID)
-
-				suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnAcknowledgementPacket = func(
-					ctx sdk.Context, channelVersion string, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress,
-				) error {
-					return fmt.Errorf("error should be unreachable")
-				}
-			}, true,
+			}, types.ErrControllerSubModuleDisabled,
 		},
 	}
 
@@ -568,7 +540,6 @@ func (suite *InterchainAccountsTestSuite) TestOnAcknowledgementPacket() {
 
 			suite.Run(tc.msg, func() {
 				suite.SetupTest() // reset
-				isNilApp = false
 
 				path = NewICAPath(suite.chainA, suite.chainB, ordering)
 				path.SetupConnections()
@@ -589,22 +560,22 @@ func (suite *InterchainAccountsTestSuite) TestOnAcknowledgementPacket() {
 
 				tc.malleate() // malleate mutates test data
 
-				module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID)
-				suite.Require().NoError(err)
-
-				cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(module)
+				cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.AppRouter.PacketRoute(path.EndpointA.ChannelConfig.PortID)
 				suite.Require().True(ok)
 
-				if isNilApp {
-					cbs = controller.NewIBCMiddleware(suite.chainA.GetSimApp().ICAControllerKeeper)
-				}
+				legacyModule, ok := cbs[0].(*porttypes.LegacyIBCModule)
+				suite.Require().True(ok, "expected there to be a single legacy ibc module")
+
+				legacyModuleCbs := legacyModule.GetCallbacks()
+				controllerModule, ok := legacyModuleCbs[1].(controller.IBCMiddleware) // controller module is routed second
+				suite.Require().True(ok)
 
-				err = cbs.OnAcknowledgementPacket(suite.chainA.GetContext(), path.EndpointA.GetChannel().Version, packet, []byte("ack"), nil)
+				err = controllerModule.OnAcknowledgementPacket(suite.chainA.GetContext(), path.EndpointA.GetChannel().Version, packet, []byte("ack"), nil)
 
-				if tc.expPass {
+				if tc.expError == nil {
 					suite.Require().NoError(err)
 				} else {
-					suite.Require().Error(err)
+					suite.Require().ErrorIs(err, tc.expError)
 				}
 			})
 		}

modules/apps/27-interchain-accounts/host/ibc_module_test.go

@@ -305,7 +305,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenConfirm() {
 				suite.Require().True(ok, "expected there to be a single legacy ibc module")
 
 				legacyModuleCbs := legacyModule.GetCallbacks()
-				hostModule, ok := legacyModuleCbs[0].(icahost.IBCModule) // fee module is routed second
+				hostModule, ok := legacyModuleCbs[0].(icahost.IBCModule)
 				suite.Require().True(ok)
 
 				err = hostModule.OnChanOpenConfirm(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
@@ -554,10 +554,14 @@ func (suite *InterchainAccountsTestSuite) TestOnAcknowledgementPacket() {
 
 				tc.malleate() // malleate mutates test data
 
-				module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID)
-				suite.Require().NoError(err)
+				cbs, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.AppRouter.PacketRoute(path.EndpointB.ChannelConfig.PortID)
+				suite.Require().True(ok)
 
-				cbs, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(module)
+				legacyModule, ok := cbs[0].(*porttypes.LegacyIBCModule)
+				suite.Require().True(ok, "expected there to be a single legacy ibc module")
+
+				legacyModuleCbs := legacyModule.GetCallbacks()
+				hostModule, ok := legacyModuleCbs[0].(icahost.IBCModule)
 				suite.Require().True(ok)
 
 				packet := channeltypes.NewPacket(
@@ -571,7 +575,7 @@ func (suite *InterchainAccountsTestSuite) TestOnAcknowledgementPacket() {
 					0,
 				)
 
-				err = cbs.OnAcknowledgementPacket(suite.chainB.GetContext(), path.EndpointB.GetChannel().Version, packet, []byte("ackBytes"), nil)
+				err = hostModule.OnAcknowledgementPacket(suite.chainB.GetContext(), path.EndpointB.GetChannel().Version, packet, []byte("ackBytes"), nil)
 
 				if tc.expPass {
 					suite.Require().NoError(err)

modules/apps/29-fee/ibc_middleware.go

@@ -197,11 +197,9 @@ func (im IBCMiddleware) OnAcknowledgementPacket(
 	relayer sdk.AccAddress,
 ) error {
 	if !im.keeper.IsFeeEnabled(ctx, packet.SourcePort, packet.SourceChannel) {
-		return im.app.OnAcknowledgementPacket(ctx, channelVersion, packet, acknowledgement, relayer)
+		return nil
 	}
 
-	appVersion := unwrapAppVersion(channelVersion)
-
 	var ack types.IncentivizedAcknowledgement
 	if err := json.Unmarshal(acknowledgement, &ack); err != nil {
 		return errorsmod.Wrapf(ibcerrors.ErrInvalidType, "cannot unmarshal ICS-29 incentivized packet acknowledgement %v: %s", ack, err)
@@ -216,14 +214,13 @@ func (im IBCMiddleware) OnAcknowledgementPacket(
 		// for fee enabled channels
 		//
 		// Please see ADR 004 for more information.
-		return im.app.OnAcknowledgementPacket(ctx, appVersion, packet, ack.AppAcknowledgement, relayer)
+		return nil
 	}
 
 	packetID := channeltypes.NewPacketID(packet.SourcePort, packet.SourceChannel, packet.Sequence)
 	feesInEscrow, found := im.keeper.GetFeesInEscrow(ctx, packetID)
 	if !found {
-		// call underlying callback
-		return im.app.OnAcknowledgementPacket(ctx, appVersion, packet, ack.AppAcknowledgement, relayer)
+		return nil
 	}
 
 	payee, found := im.keeper.GetPayeeAddress(ctx, relayer.String(), packet.SourceChannel)
@@ -237,9 +234,7 @@ func (im IBCMiddleware) OnAcknowledgementPacket(
 	}
 
 	im.keeper.DistributePacketFeesOnAcknowledgement(ctx, ack.ForwardRelayerAddress, payeeAddr, feesInEscrow.PacketFees, packetID)
-
-	// call underlying callback
-	return im.app.OnAcknowledgementPacket(ctx, appVersion, packet, ack.AppAcknowledgement, relayer)
+	return nil
 }
 
 // OnTimeoutPacket implements the IBCMiddleware interface
@@ -401,3 +396,30 @@ func (IBCMiddleware) UnwrapVersionUnsafe(version string) (string, string, error)
 
 	return metadata.FeeVersion, metadata.AppVersion, nil
 }
+
+// UnwrapVersionSafe unwraps a version contextually by relying on storage and the given portID and channelID.
+func (im IBCMiddleware) UnwrapVersionSafe(ctx sdk.Context, portID, channelID, version string) (string, string) {
+	if !im.keeper.IsFeeEnabled(ctx, portID, channelID) {
+		return "", version
+	}
+	metadata, err := types.MetadataFromVersion(version)
+	if err != nil {
+		// This should not happen, as it would mean that the channel is broken. Only a severe bug would cause this.
+		panic(errorsmod.Wrap(err, "failed to unwrap app version from channel version"))
+	}
+	return metadata.FeeVersion, metadata.AppVersion
+}
+
+// UnwrapAcknowledgement unwraps an acnkowledgement contextually by relying on storage and the given portID and channelID.
+func (im IBCMiddleware) UnwrapAcknowledgement(ctx sdk.Context, portID, channelID string, ack []byte) ([]byte, []byte) {
+	if !im.keeper.IsFeeEnabled(ctx, portID, channelID) {
+		return nil, ack
+	}
+
+	var incentivizedAck types.IncentivizedAcknowledgement
+	if err := json.Unmarshal(ack, &incentivizedAck); err != nil {
+		panic(errorsmod.Wrap(err, "failed to unwrap acknowledgement"))
+	}
+
+	return ack, incentivizedAck.AppAcknowledgement
+}

modules/apps/29-fee/ibc_middleware_test.go

@@ -710,16 +710,6 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
 			false,
 			func() {},
 		},
-		{
-			"application callback fails",
-			func() {
-				suite.chainA.GetSimApp().FeeMockModule.IBCApp.OnAcknowledgementPacket = func(_ sdk.Context, _ string, _ channeltypes.Packet, _ []byte, _ sdk.AccAddress) error {
-					return fmt.Errorf("mock fee app callback fails")
-				}
-			},
-			false,
-			func() {},
-		},
 	}
 
 	for _, tc := range testCases {

modules/apps/callbacks/ibc_middleware.go

@@ -125,12 +125,6 @@ func (im IBCMiddleware) OnAcknowledgementPacket(
 	acknowledgement []byte,
 	relayer sdk.AccAddress,
 ) error {
-	// we first call the underlying app to handle the acknowledgement
-	err := im.app.OnAcknowledgementPacket(ctx, channelVersion, packet, acknowledgement, relayer)
-	if err != nil {
-		return err
-	}
-
 	// OnAcknowledgementPacket is not blocked if the packet does not opt-in to callbacks
 	callbackData, err := types.GetSourceCallbackData(ctx, im.app, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetData(), im.maxCallbackGas)
 	if err != nil {
@@ -348,7 +342,7 @@ func (IBCMiddleware) OnChanOpenConfirm(ctx sdk.Context, portID, channelID string
 }
 
 // OnChanCloseInit defers to the underlying application
-func (im IBCMiddleware) OnChanCloseInit(ctx sdk.Context, portID, channelID string) error {
+func (IBCMiddleware) OnChanCloseInit(ctx sdk.Context, portID, channelID string) error {
 	return nil
 }
 

modules/apps/callbacks/ibc_middleware_test.go

@@ -258,14 +258,6 @@ func (s *CallbacksTestSuite) TestOnAcknowledgementPacket() {
 			callbackSuccess,
 			nil,
 		},
-		{
-			"failure: underlying app OnAcknolwedgePacket fails",
-			func() {
-				ack = []byte("invalid ack")
-			},
-			noExecution,
-			ibcerrors.ErrUnknownRequest,
-		},
 		{
 			"success: no-op on callback data is not valid",
 			func() {
@@ -345,11 +337,19 @@ func (s *CallbacksTestSuite) TestOnAcknowledgementPacket() {
 			tc.malleate()
 
 			// callbacks module is routed as top level middleware
-			transferStack, ok := s.chainA.App.GetIBCKeeper().PortKeeper.Route(transfertypes.ModuleName)
+			cbs, ok := s.chainA.App.GetIBCKeeper().PortKeeper.AppRouter.PacketRoute(transfertypes.ModuleName)
+			s.Require().True(ok)
+
+			legacyModule, ok := cbs[0].(*porttypes.LegacyIBCModule)
+			s.Require().True(ok, "expected there to be a single legacy ibc module")
+
+			legacyModuleCbs := legacyModule.GetCallbacks()
+
+			callbacksModule, ok := legacyModuleCbs[1].(ibccallbacks.IBCMiddleware) // callbacks module is routed second
 			s.Require().True(ok)
 
 			onAcknowledgementPacket := func() error {
-				return transferStack.OnAcknowledgementPacket(ctx, s.path.EndpointA.GetChannel().Version, packet, ack, s.chainA.SenderAccount.GetAddress())
+				return callbacksModule.OnAcknowledgementPacket(ctx, s.path.EndpointA.GetChannel().Version, packet, ack, s.chainA.SenderAccount.GetAddress())
 			}
 
 			switch tc.expError {
@@ -388,7 +388,7 @@ func (s *CallbacksTestSuite) TestOnAcknowledgementPacket() {
 				s.Require().Equal(uint8(1), sourceStatefulCounter)
 
 				expEvent, exists := GetExpectedEvent(
-					ctx, transferStack.(porttypes.PacketDataUnmarshaler), gasLimit, packet.Data, packet.SourcePort,
+					ctx, callbacksModule, gasLimit, packet.Data, packet.SourcePort,
 					packet.SourcePort, packet.SourceChannel, packet.Sequence, types.CallbackTypeAcknowledgementPacket, nil,
 				)
 				s.Require().True(exists)

modules/core/05-port/types/ibc_legacy_module.go

@@ -234,13 +234,32 @@ func (LegacyIBCModule) OnRecvPacket(
 }
 
 // OnAcknowledgementPacket implements the IBCModule interface
-func (LegacyIBCModule) OnAcknowledgementPacket(
+func (im *LegacyIBCModule) OnAcknowledgementPacket(
 	ctx sdk.Context,
 	channelVersion string,
 	packet channeltypes.Packet,
 	acknowledgement []byte,
 	relayer sdk.AccAddress,
 ) error {
+	for i := len(im.cbs) - 1; i >= 0; i-- {
+		var (
+			cbVersion = channelVersion
+			cbAck     = acknowledgement
+		)
+
+		if wrapper, ok := im.cbs[i].(VersionWrapper); ok {
+			cbVersion, channelVersion = wrapper.UnwrapVersionSafe(ctx, packet.SourcePort, packet.SourceChannel, cbVersion)
+		}
+
+		if wrapper, ok := im.cbs[i].(AcknowledgementWrapper); ok {
+			cbAck, acknowledgement = wrapper.UnwrapAcknowledgement(ctx, packet.SourcePort, packet.SourceChannel, cbAck)
+		}
+
+		err := im.cbs[i].OnAcknowledgementPacket(ctx, cbVersion, packet, cbAck, relayer)
+		if err != nil {
+			return errorsmod.Wrap(err, "acknowledge packet callback failed")
+		}
+	}
 	return nil
 }
 

modules/core/05-port/types/module.go

@@ -136,7 +136,11 @@ type VersionWrapper interface {
 	// UnwrapVersionUnsafe will be used during opening handshakes and channel upgrades when the version
 	// is still being negotiated.
 	UnwrapVersionUnsafe(string) (cbVersion, underlyingAppVersion string, err error)
-	// UnwrapVersionSafe(ctx sdk.Context, portID, channelID, version string) (appVersion, version string)
+	UnwrapVersionSafe(ctx sdk.Context, portID, channelID, version string) (cbVersion, underlyingAppVersion string)
+}
+
+type AcknowledgementWrapper interface {
+	UnwrapAcknowledgement(ctx sdk.Context, portID, channelID string, acknowledgment []byte) (cbAcknowledgement, underlyingAppAcknowledgement []byte)
 }
 
 // UpgradableModule defines the callbacks required to perform a channel upgrade.

modules/core/keeper/msg_server.go

@@ -614,7 +614,7 @@ func (k *Keeper) Acknowledgement(goCtx context.Context, msg *channeltypes.MsgAck
 	}
 
 	// Retrieve callbacks from router
-	cbs, ok := k.PortKeeper.Route(msg.Packet.SourcePort)
+	cbs, ok := k.PortKeeper.AppRouter.PacketRoute(msg.Packet.SourcePort)
 	if !ok {
 		ctx.Logger().Error("acknowledgement failed", "port-id", msg.Packet.SourcePort, "error", errorsmod.Wrapf(porttypes.ErrInvalidRoute, "route not found to module: %s", msg.Packet.SourcePort))
 		return nil, errorsmod.Wrapf(porttypes.ErrInvalidRoute, "route not found to module: %s", msg.Packet.SourcePort)
@@ -640,10 +640,12 @@ func (k *Keeper) Acknowledgement(goCtx context.Context, msg *channeltypes.MsgAck
 	}
 
 	// Perform application logic callback
-	err = cbs.OnAcknowledgementPacket(ctx, channelVersion, msg.Packet, msg.Acknowledgement, relayer)
-	if err != nil {
-		ctx.Logger().Error("acknowledgement failed", "port-id", msg.Packet.SourcePort, "channel-id", msg.Packet.SourceChannel, "error", errorsmod.Wrap(err, "acknowledge packet callback failed"))
-		return nil, errorsmod.Wrap(err, "acknowledge packet callback failed")
+	for _, cb := range cbs {
+		err = cb.OnAcknowledgementPacket(ctx, channelVersion, msg.Packet, msg.Acknowledgement, relayer)
+		if err != nil {
+			ctx.Logger().Error("acknowledgement failed", "port-id", msg.Packet.SourcePort, "channel-id", msg.Packet.SourceChannel, "error", errorsmod.Wrap(err, "acknowledge packet callback failed"))
+			return nil, errorsmod.Wrap(err, "acknowledge packet callback failed")
+		}
 	}
 
 	defer telemetry.ReportAcknowledgePacket(msg.Packet)