fix: fixed callbacks middleware wiring (#5950)

* test: mock callback is overridable * test: added ack test * imp: fixed most tests * test: fixed all tests * docs: updated integration docs for callbacks * docs: updated docs * docs: fix broken links
cosmos · Mar 21, 2024 · ee4549b · ee4549b
1 parent e69a833
commit ee4549b
Show file tree

Hide file tree

Showing 17 changed files with 217 additions and 111 deletions.
diff --git a/docs/docs/04-middleware/02-callbacks/02-integration.md b/docs/docs/04-middleware/02-callbacks/02-integration.md
@@ -36,24 +36,24 @@ The in-line comments describe the execution flow of packets between the applicat
 ```go
 // Create Transfer Stack
 // SendPacket, since it is originating from the application to core IBC:
-// transferKeeper.SendPacket -> callbacks.SendPacket -> fee.SendPacket -> channel.SendPacket
+// transferKeeper.SendPacket -> callbacks.SendPacket -> feeKeeper.SendPacket -> channel.SendPacket
 
 // RecvPacket, message that originates from core IBC and goes down to app, the flow is the other way
-// channel.RecvPacket -> callbacks.OnRecvPacket -> fee.OnRecvPacket -> transfer.OnRecvPacket
+// channel.RecvPacket -> fee.OnRecvPacket -> callbacks.OnRecvPacket -> transfer.OnRecvPacket
 
 // transfer stack contains (from top to bottom):
-// - IBC Callbacks Middleware
 // - IBC Fee Middleware
+// - IBC Callbacks Middleware
 // - Transfer
 
 // create IBC module from bottom to top of stack
 var transferStack porttypes.IBCModule
 transferStack = transfer.NewIBCModule(app.TransferKeeper)
-transferStack = ibcfee.NewIBCMiddleware(transferStack, app.IBCFeeKeeper)
-// maxCallbackGas is a hard-coded value that is passed to the callbacks middleware
 transferStack = ibccallbacks.NewIBCMiddleware(transferStack, app.IBCFeeKeeper, app.MockContractKeeper, maxCallbackGas)
+transferICS4Wrapper := transferStack.(porttypes.ICS4Wrapper)
+transferStack = ibcfee.NewIBCMiddleware(transferStack, app.IBCFeeKeeper)
 // Since the callbacks middleware itself is an ics4wrapper, it needs to be passed to the transfer keeper
-app.TransferKeeper.WithICS4Wrapper(transferStack.(porttypes.ICS4Wrapper))
+app.TransferKeeper.WithICS4Wrapper(transferICS4Wrapper)
 
 // Add transfer stack to IBC Router
 ibcRouter.AddRoute(ibctransfertypes.ModuleName, transferStack)
@@ -70,16 +70,19 @@ The usage of `WithICS4Wrapper` after `transferStack`'s configuration is critical
 // SendPacket, since it is originating from the application to core IBC:
 // icaControllerKeeper.SendTx -> callbacks.SendPacket -> fee.SendPacket -> channel.SendPacket
 
+// initialize ICA module with mock module as the authentication module on the controller side
 var icaControllerStack porttypes.IBCModule
-icaControllerStack = icacontroller.NewIBCMiddleware(nil, app.ICAControllerKeeper)
-icaControllerStack = ibcfee.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper)
-// maxCallbackGas is a hard-coded value that is passed to the callbacks middleware
+icaControllerStack = ibcmock.NewIBCModule(&mockModule, ibcmock.NewIBCApp("", scopedICAMockKeeper))
+app.ICAAuthModule = icaControllerStack.(ibcmock.IBCModule)
+icaControllerStack = icacontroller.NewIBCMiddleware(icaControllerStack, app.ICAControllerKeeper)
 icaControllerStack = ibccallbacks.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper, app.MockContractKeeper, maxCallbackGas)
+icaICS4Wrapper := icaControllerStack.(porttypes.ICS4Wrapper)
+icaControllerStack = ibcfee.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper)
 // Since the callbacks middleware itself is an ics4wrapper, it needs to be passed to the ica controller keeper
-app.ICAControllerKeeper.WithICS4Wrapper(icaControllerStack.(porttypes.ICS4Wrapper))
+app.ICAControllerKeeper.WithICS4Wrapper(icaICS4Wrapper)
 
 // RecvPacket, message that originates from core IBC and goes down to app, the flow is:
-// channel.RecvPacket -> callbacks.OnRecvPacket -> fee.OnRecvPacket -> icaHost.OnRecvPacket
+// channel.RecvPacket -> fee.OnRecvPacket -> icaHost.OnRecvPacket
 
 var icaHostStack porttypes.IBCModule
 icaHostStack = icahost.NewIBCModule(app.ICAHostKeeper)

diff --git a/docs/versioned_docs/version-v7.3.x/04-middleware/02-callbacks/02-integration.md b/docs/versioned_docs/version-v7.3.x/04-middleware/02-callbacks/02-integration.md
@@ -36,24 +36,24 @@ The in-line comments describe the execution flow of packets between the applicat
 ```go
 // Create Transfer Stack
 // SendPacket, since it is originating from the application to core IBC:
-// transferKeeper.SendPacket -> callbacks.SendPacket -> fee.SendPacket -> channel.SendPacket
+// transferKeeper.SendPacket -> callbacks.SendPacket -> feeKeeper.SendPacket -> channel.SendPacket
 
 // RecvPacket, message that originates from core IBC and goes down to app, the flow is the other way
-// channel.RecvPacket -> callbacks.OnRecvPacket -> fee.OnRecvPacket -> transfer.OnRecvPacket
+// channel.RecvPacket -> fee.OnRecvPacket -> callbacks.OnRecvPacket -> transfer.OnRecvPacket
 
 // transfer stack contains (from top to bottom):
-// - IBC Callbacks Middleware
 // - IBC Fee Middleware
+// - IBC Callbacks Middleware
 // - Transfer
 
 // create IBC module from bottom to top of stack
 var transferStack porttypes.IBCModule
 transferStack = transfer.NewIBCModule(app.TransferKeeper)
-transferStack = ibcfee.NewIBCMiddleware(transferStack, app.IBCFeeKeeper)
-// maxCallbackGas is a hard-coded value that is passed to the callbacks middleware
 transferStack = ibccallbacks.NewIBCMiddleware(transferStack, app.IBCFeeKeeper, app.MockContractKeeper, maxCallbackGas)
+transferICS4Wrapper := transferStack.(porttypes.ICS4Wrapper)
+transferStack = ibcfee.NewIBCMiddleware(transferStack, app.IBCFeeKeeper)
 // Since the callbacks middleware itself is an ics4wrapper, it needs to be passed to the transfer keeper
-app.TransferKeeper.WithICS4Wrapper(transferStack.(porttypes.ICS4Wrapper))
+app.TransferKeeper.WithICS4Wrapper(transferICS4Wrapper)
 
 // Add transfer stack to IBC Router
 ibcRouter.AddRoute(ibctransfertypes.ModuleName, transferStack)
@@ -70,16 +70,19 @@ The usage of `WithICS4Wrapper` after `transferStack`'s configuration is critical
 // SendPacket, since it is originating from the application to core IBC:
 // icaControllerKeeper.SendTx -> callbacks.SendPacket -> fee.SendPacket -> channel.SendPacket
 
+// initialize ICA module with mock module as the authentication module on the controller side
 var icaControllerStack porttypes.IBCModule
-icaControllerStack = icacontroller.NewIBCMiddleware(nil, app.ICAControllerKeeper)
-icaControllerStack = ibcfee.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper)
-// maxCallbackGas is a hard-coded value that is passed to the callbacks middleware
+icaControllerStack = ibcmock.NewIBCModule(&mockModule, ibcmock.NewIBCApp("", scopedICAMockKeeper))
+app.ICAAuthModule = icaControllerStack.(ibcmock.IBCModule)
+icaControllerStack = icacontroller.NewIBCMiddleware(icaControllerStack, app.ICAControllerKeeper)
 icaControllerStack = ibccallbacks.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper, app.MockContractKeeper, maxCallbackGas)
+icaICS4Wrapper := icaControllerStack.(porttypes.ICS4Wrapper)
+icaControllerStack = ibcfee.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper)
 // Since the callbacks middleware itself is an ics4wrapper, it needs to be passed to the ica controller keeper
-app.ICAControllerKeeper.WithICS4Wrapper(icaControllerStack.(porttypes.ICS4Wrapper))
+app.ICAControllerKeeper.WithICS4Wrapper(icaICS4Wrapper)
 
 // RecvPacket, message that originates from core IBC and goes down to app, the flow is:
-// channel.RecvPacket -> callbacks.OnRecvPacket -> fee.OnRecvPacket -> icaHost.OnRecvPacket
+// channel.RecvPacket -> fee.OnRecvPacket -> icaHost.OnRecvPacket
 
 var icaHostStack porttypes.IBCModule
 icaHostStack = icahost.NewIBCModule(app.ICAHostKeeper)

diff --git a/docs/versioned_docs/version-v8.1.x/04-middleware/02-callbacks/02-integration.md b/docs/versioned_docs/version-v8.1.x/04-middleware/02-callbacks/02-integration.md
@@ -36,24 +36,24 @@ The in-line comments describe the execution flow of packets between the applicat
 ```go
 // Create Transfer Stack
 // SendPacket, since it is originating from the application to core IBC:
-// transferKeeper.SendPacket -> callbacks.SendPacket -> fee.SendPacket -> channel.SendPacket
+// transferKeeper.SendPacket -> callbacks.SendPacket -> feeKeeper.SendPacket -> channel.SendPacket
 
 // RecvPacket, message that originates from core IBC and goes down to app, the flow is the other way
-// channel.RecvPacket -> callbacks.OnRecvPacket -> fee.OnRecvPacket -> transfer.OnRecvPacket
+// channel.RecvPacket -> fee.OnRecvPacket -> callbacks.OnRecvPacket -> transfer.OnRecvPacket
 
 // transfer stack contains (from top to bottom):
-// - IBC Callbacks Middleware
 // - IBC Fee Middleware
+// - IBC Callbacks Middleware
 // - Transfer
 
 // create IBC module from bottom to top of stack
 var transferStack porttypes.IBCModule
 transferStack = transfer.NewIBCModule(app.TransferKeeper)
-transferStack = ibcfee.NewIBCMiddleware(transferStack, app.IBCFeeKeeper)
-// maxCallbackGas is a hard-coded value that is passed to the callbacks middleware
 transferStack = ibccallbacks.NewIBCMiddleware(transferStack, app.IBCFeeKeeper, app.MockContractKeeper, maxCallbackGas)
+transferICS4Wrapper := transferStack.(porttypes.ICS4Wrapper)
+transferStack = ibcfee.NewIBCMiddleware(transferStack, app.IBCFeeKeeper)
 // Since the callbacks middleware itself is an ics4wrapper, it needs to be passed to the transfer keeper
-app.TransferKeeper.WithICS4Wrapper(transferStack.(porttypes.ICS4Wrapper))
+app.TransferKeeper.WithICS4Wrapper(transferICS4Wrapper)
 
 // Add transfer stack to IBC Router
 ibcRouter.AddRoute(ibctransfertypes.ModuleName, transferStack)
@@ -70,16 +70,19 @@ The usage of `WithICS4Wrapper` after `transferStack`'s configuration is critical
 // SendPacket, since it is originating from the application to core IBC:
 // icaControllerKeeper.SendTx -> callbacks.SendPacket -> fee.SendPacket -> channel.SendPacket
 
+// initialize ICA module with mock module as the authentication module on the controller side
 var icaControllerStack porttypes.IBCModule
-icaControllerStack = icacontroller.NewIBCMiddleware(nil, app.ICAControllerKeeper)
-icaControllerStack = ibcfee.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper)
-// maxCallbackGas is a hard-coded value that is passed to the callbacks middleware
+icaControllerStack = ibcmock.NewIBCModule(&mockModule, ibcmock.NewIBCApp("", scopedICAMockKeeper))
+app.ICAAuthModule = icaControllerStack.(ibcmock.IBCModule)
+icaControllerStack = icacontroller.NewIBCMiddleware(icaControllerStack, app.ICAControllerKeeper)
 icaControllerStack = ibccallbacks.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper, app.MockContractKeeper, maxCallbackGas)
+icaICS4Wrapper := icaControllerStack.(porttypes.ICS4Wrapper)
+icaControllerStack = ibcfee.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper)
 // Since the callbacks middleware itself is an ics4wrapper, it needs to be passed to the ica controller keeper
-app.ICAControllerKeeper.WithICS4Wrapper(icaControllerStack.(porttypes.ICS4Wrapper))
+app.ICAControllerKeeper.WithICS4Wrapper(icaICS4Wrapper)
 
 // RecvPacket, message that originates from core IBC and goes down to app, the flow is:
-// channel.RecvPacket -> callbacks.OnRecvPacket -> fee.OnRecvPacket -> icaHost.OnRecvPacket
+// channel.RecvPacket -> fee.OnRecvPacket -> icaHost.OnRecvPacket
 
 var icaHostStack porttypes.IBCModule
 icaHostStack = icahost.NewIBCModule(app.ICAHostKeeper)

diff --git a/modules/apps/27-interchain-accounts/controller/keeper/export_test.go b/modules/apps/27-interchain-accounts/controller/keeper/export_test.go
@@ -8,14 +8,8 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types"
-	porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types"
 )
 
-// GetICS4Wrapper is a getter for the keeper's ICS4Wrapper.
-func (k *Keeper) GetICS4Wrapper() porttypes.ICS4Wrapper {
-	return k.ics4Wrapper
-}
-
 // GetAppMetadata is a wrapper around getAppMetadata to allow the function to be directly called in tests.
 func (k Keeper) GetAppMetadata(ctx sdk.Context, portID, channelID string) (icatypes.Metadata, error) {
 	return k.getAppMetadata(ctx, portID, channelID)

diff --git a/modules/apps/27-interchain-accounts/controller/keeper/keeper.go b/modules/apps/27-interchain-accounts/controller/keeper/keeper.go
@@ -72,6 +72,11 @@ func (k *Keeper) WithICS4Wrapper(wrapper porttypes.ICS4Wrapper) {
 	k.ics4Wrapper = wrapper
 }
 
+// GetICS4Wrapper returns the ICS4Wrapper.
+func (k Keeper) GetICS4Wrapper() porttypes.ICS4Wrapper {
+	return k.ics4Wrapper
+}
+
 // Logger returns the application logger, scoped to the associated module
 func (Keeper) Logger(ctx sdk.Context) log.Logger {
 	return ctx.Logger().With("module", fmt.Sprintf("x/%s-%s", exported.ModuleName, icatypes.ModuleName))

diff --git a/modules/apps/27-interchain-accounts/host/keeper/export_test.go b/modules/apps/27-interchain-accounts/host/keeper/export_test.go
@@ -8,14 +8,8 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types"
-	porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types"
 )
 
-// GetICS4Wrapper is a getter for the keeper's ICS4Wrapper.
-func (k *Keeper) GetICS4Wrapper() porttypes.ICS4Wrapper {
-	return k.ics4Wrapper
-}
-
 // GetAppMetadata is a wrapper around getAppMetadata to allow the function to be directly called in tests.
 func (k Keeper) GetAppMetadata(ctx sdk.Context, portID, channelID string) (icatypes.Metadata, error) {
 	return k.getAppMetadata(ctx, portID, channelID)

diff --git a/modules/apps/27-interchain-accounts/host/keeper/keeper.go b/modules/apps/27-interchain-accounts/host/keeper/keeper.go
@@ -80,6 +80,11 @@ func (k *Keeper) WithICS4Wrapper(wrapper porttypes.ICS4Wrapper) {
 	k.ics4Wrapper = wrapper
 }
 
+// GetICS4Wrapper returns the ICS4Wrapper.
+func (k Keeper) GetICS4Wrapper() porttypes.ICS4Wrapper {
+	return k.ics4Wrapper
+}
+
 // Logger returns the application logger, scoped to the associated module
 func (Keeper) Logger(ctx sdk.Context) log.Logger {
 	return ctx.Logger().With("module", fmt.Sprintf("x/%s-%s", exported.ModuleName, icatypes.ModuleName))

diff --git a/modules/apps/29-fee/keeper/export_test.go b/modules/apps/29-fee/keeper/export_test.go
@@ -4,14 +4,8 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	"github.com/cosmos/ibc-go/v8/modules/apps/29-fee/types"
-	porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types"
 )
 
-// GetICS4Wrapper is a getter for the keeper's ICS4Wrapper.
-func (k *Keeper) GetICS4Wrapper() porttypes.ICS4Wrapper {
-	return k.ics4Wrapper
-}
-
 // LegacyTotal is a wrapper for the legacyTotal function for testing.
 func LegacyTotal(f types.Fee) sdk.Coins {
 	return legacyTotal(f)

diff --git a/modules/apps/29-fee/keeper/keeper.go b/modules/apps/29-fee/keeper/keeper.go
@@ -57,6 +57,11 @@ func (k *Keeper) WithICS4Wrapper(wrapper porttypes.ICS4Wrapper) {
 	k.ics4Wrapper = wrapper
 }
 
+// GetICS4Wrapper returns the ICS4Wrapper.
+func (k Keeper) GetICS4Wrapper() porttypes.ICS4Wrapper {
+	return k.ics4Wrapper
+}
+
 // Logger returns a module-specific logger.
 func (Keeper) Logger(ctx sdk.Context) log.Logger {
 	return ctx.Logger().With("module", "x/"+ibcexported.ModuleName+"-"+types.ModuleName)

diff --git a/modules/apps/callbacks/export_test.go b/modules/apps/callbacks/export_test.go
@@ -8,7 +8,6 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	"github.com/cosmos/ibc-go/modules/apps/callbacks/types"
-	porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types"
 )
 
 // ProcessCallback is a wrapper around processCallback to allow the function to be directly called in tests.
@@ -18,8 +17,3 @@ func (im IBCMiddleware) ProcessCallback(
 ) error {
 	return im.processCallback(ctx, callbackType, callbackData, callbackExecutor)
 }
-
-// GetICS4Wrapper is a getter for the IBCMiddleware's ICS4Wrapper.
-func (im *IBCMiddleware) GetICS4Wrapper() porttypes.ICS4Wrapper {
-	return im.ics4Wrapper
-}