diff --git a/docs/docs/01-ibc/03-apps/01-apps.md b/docs/docs/01-ibc/03-apps/01-apps.md index 2b7089171dd..02db6ef1639 100644 --- a/docs/docs/01-ibc/03-apps/01-apps.md +++ b/docs/docs/01-ibc/03-apps/01-apps.md @@ -178,6 +178,23 @@ encoded version into each handhshake call as necessary. ICS20 currently implements basic string matching with a single supported version. +### ICS4Wrapper + +The IBC application interacts with core IBC through the `ICS4Wrapper` interface for any application-initiated actions like: `SendPacket` and `WriteAcknowledgement`. This may be directly the IBCChannelKeeper or a middleware that sits between the application and the IBC ChannelKeeper. + +If the application is being wired with a custom middleware, the application **must** have its ICS4Wrapper set to the middleware directly above it on the stack through the following call: + +```go +// SetICS4Wrapper sets the ICS4Wrapper. This function may be used after +// the module's initialization to set the middleware which is above this +// module in the IBC application stack. +// The ICS4Wrapper **must** be used for sending packets and writing acknowledgements +// to ensure that the middleware can intercept and process these calls. +// Do not use the channel keeper directly to send packets or write acknowledgements +// as this will bypass the middleware. +SetICS4Wrapper(wrapper ICS4Wrapper) +``` + ### Custom Packets Modules connected by a channel must agree on what application data they are sending over the diff --git a/docs/docs/01-ibc/04-middleware/02-develop.md b/docs/docs/01-ibc/04-middleware/02-develop.md index 88042caa0a3..409d021e90d 100644 --- a/docs/docs/01-ibc/04-middleware/02-develop.md +++ b/docs/docs/01-ibc/04-middleware/02-develop.md @@ -31,6 +31,10 @@ The interfaces a middleware must implement are found [here](https://github.com/c type Middleware interface { IBCModule // middleware has access to an underlying application which may be wrapped by more middleware ICS4Wrapper // middleware has access to ICS4Wrapper which may be core IBC Channel Handler or a higher-level middleware that wraps this middleware. + + // SetUnderlyingModule sets the underlying IBC module. This function may be used after + // the middleware's initialization to set the ibc module which is below this middleware. + SetUnderlyingApplication(IBCModule) } ``` @@ -42,14 +46,12 @@ An `IBCMiddleware` struct implementing the `Middleware` interface, can be define // IBCMiddleware implements the ICS26 callbacks and ICS4Wrapper for the fee middleware given the // fee keeper and the underlying application. type IBCMiddleware struct { - app porttypes.IBCModule - keeper keeper.Keeper + keeper *keeper.Keeper } // NewIBCMiddleware creates a new IBCMiddleware given the keeper and underlying application -func NewIBCMiddleware(app porttypes.IBCModule, k keeper.Keeper) IBCMiddleware { +func NewIBCMiddleware(k *keeper.Keeper) IBCMiddleware { return IBCMiddleware{ - app: app, keeper: k, } } @@ -476,3 +478,26 @@ func GetAppVersion( ``` See [here](https://github.com/cosmos/ibc-go/blob/v7.0.0/modules/apps/29-fee/keeper/relay.go#L58-L74) an example implementation of this function for the ICS-29 Fee Middleware module. + +## Wiring Interface Requirements + +Middleware must also implement the following functions so that they can be called in the stack builder in order to correctly wire the application stack together: `SetUnderlyingApplication` and `SetICS4Wrapper`. + +```go +// SetUnderlyingModule sets the underlying IBC module. This function may be used after +// the middleware's initialization to set the ibc module which is below this middleware. +SetUnderlyingApplication(IBCModule) + +// SetICS4Wrapper sets the ICS4Wrapper. This function may be used after +// the module's initialization to set the middleware which is above this +// module in the IBC application stack. +// The ICS4Wrapper **must** be used for sending packets and writing acknowledgements +// to ensure that the middleware can intercept and process these calls. +// Do not use the channel keeper directly to send packets or write acknowledgements +// as this will bypass the middleware. +SetICS4Wrapper(wrapper ICS4Wrapper) +``` + +The middleware itself should have access to the `underlying app` (note this may be a base app or an application wrapped by layers of lower-level middleware(s)) and access to the higher layer `ICS4wrapper`. The `underlying app` gets called during the relayer initiated actions: `recvPacket`, `acknowledgePacket`, and `timeoutPacket`. The `ics4Wrapper` gets called on user-initiated actions like `sendPacket` and `writeAcknowledgement`. + +The functions above are used by the `StackBuilder` during application setup to wire the stack correctly. The stack must be wired first and have all of the wrappers and applications set correctly before transaction execution starts and packet processing begins. diff --git a/docs/docs/01-ibc/04-middleware/03-integration.md b/docs/docs/01-ibc/04-middleware/03-integration.md index 3d1ed88b83a..f1b13273dd7 100644 --- a/docs/docs/01-ibc/04-middleware/03-integration.md +++ b/docs/docs/01-ibc/04-middleware/03-integration.md @@ -25,10 +25,13 @@ The order of middleware **matters**, function calls from IBC to the application // middleware 1 and middleware 3 are stateful middleware, // perhaps implementing separate sdk.Msg and Handlers -mw1Keeper := mw1.NewKeeper(storeKey1, ..., ics4Wrapper: channelKeeper, ...) // in stack 1 & 3 +// NOTE: NewKeeper returns a pointer so that we can modify +// the keepers later after initialization +// They are all initialized to use the channelKeeper directly at the start +mw1Keeper := mw1.NewKeeper(storeKey1, ..., channelKeeper) // in stack 1 & 3 // middleware 2 is stateless -mw3Keeper1 := mw3.NewKeeper(storeKey3,..., ics4Wrapper: mw1Keeper, ...) // in stack 1 -mw3Keeper2 := mw3.NewKeeper(storeKey3,..., ics4Wrapper: channelKeeper, ...) // in stack 2 +mw3Keeper1 := mw3.NewKeeper(storeKey3,..., channelKeeper) // in stack 1 +mw3Keeper2 := mw3.NewKeeper(storeKey3,..., channelKeeper) // in stack 2 // Only create App Module **once** and register in app module // if the module maintains independent state and/or processes sdk.Msgs @@ -55,13 +58,26 @@ customIBCModule1 := custom.NewIBCModule(customKeeper1, "portCustom1") customIBCModule2 := custom.NewIBCModule(customKeeper2, "portCustom2") // create IBC stacks by combining middleware with base application +// IBC Stack builders are initialized with the IBC ChannelKeeper which is the top-level ICS4Wrapper // NOTE: since middleware2 is stateless it does not require a Keeper // stack 1 contains mw1 -> mw3 -> transfer -stack1 := mw1.NewIBCMiddleware(mw3.NewIBCMiddleware(transferIBCModule, mw3Keeper1), mw1Keeper) +stack1 := porttypes.NewStackBuilder(ibcChannelKeeper). + Base(transferIBCModule). + Next(mw3). + Next(mw1). + Build() // stack 2 contains mw3 -> mw2 -> custom1 -stack2 := mw3.NewIBCMiddleware(mw2.NewIBCMiddleware(customIBCModule1), mw3Keeper2) +stack2 := porttypes.NewStackBuilder(ibcChannelKeeper). + Base(customIBCModule1). + Next(mw2). + Next(mw3). + Build() // stack 3 contains mw2 -> mw1 -> custom2 -stack3 := mw2.NewIBCMiddleware(mw1.NewIBCMiddleware(customIBCModule2, mw1Keeper)) +stack3 := porttypes.NewStackBuilder(ibcChannelKeeper). + Base(customIBCModule2). + Next(mw1). + Next(mw2). + Build() // associate each stack with the moduleName provided by the underlying Keeper ibcRouter := porttypes.NewRouter() diff --git a/docs/docs/05-migrations/15-support-stackbuilder.md b/docs/docs/05-migrations/15-support-stackbuilder.md new file mode 100644 index 00000000000..a06c65b6439 --- /dev/null +++ b/docs/docs/05-migrations/15-support-stackbuilder.md @@ -0,0 +1,148 @@ +--- +title: Support the new StackBuilder primitive for Wiring Middlewares in the chain application +sidebar_label: Support StackBuilder Wiring +sidebar_position: 1 +slug: /migrations/support-stackbuilder +--- + +# Migration for Chains wishing to use StackBuilder + +The StackBuilder struct is a new primitive for wiring middleware in a simpler and less error-prone manner. It is not a breaking change thus the existing method of wiring middleware still works, though it is highly recommended to transition to the new wiring method. + +Refer to the [integration guide](../01-ibc/04-middleware/03-integration.md) to understand how to use this new middleware to improve middleware wiring in the chain application setup. + +# Migrations for Application Developers + +In order to be wired with the new StackBuilder primitive, applications and middlewares must implement new methods as part of their respective interfaces. + +IBC Applications must implement a new `SetICS4Wrapper` which will set the `ICS4Wrapper` through which the application will call `SendPacket` and `WriteAcknowledgement`. It is recommended that IBC applications are initialized first with the IBC ChannelKeeper directly, and then modified with a middleware ICS4Wrapper during the stack wiring. + +```go +// SetICS4Wrapper sets the ICS4Wrapper. This function may be used after +// the module's initialization to set the middleware which is above this +// module in the IBC application stack. +// The ICS4Wrapper **must** be used for sending packets and writing acknowledgements +// to ensure that the middleware can intercept and process these calls. +// Do not use the channel keeper directly to send packets or write acknowledgements +// as this will bypass the middleware. +SetICS4Wrapper(wrapper ICS4Wrapper) +``` + +Many applications have a stateful keeper that executes the logic for sending packets and writing acknowledgements. In this case, the keeper in the application must be a **pointer** reference so that it can be modified in place after initialization. + +The initialization should be modified to no longer take in an addition `ics4Wrapper` as this gets modified later by `SetICS4Wrapper`. The constructor function must also return a **pointer** reference so that it may be modified in-place by the stack builder. + +Below is an example IBCModule that supports the stack builder wiring. + +E.g. + +```go +type IBCModule struct { + keeper *keeper.Keeper +} + +// NewIBCModule creates a new IBCModule given the keeper +func NewIBCModule(k *keeper.Keeper) *IBCModule { + return &IBCModule{ + keeper: k, + } +} + +// SetICS4Wrapper sets the ICS4Wrapper. This function may be used after +// the module's initialization to set the middleware which is above this +// module in the IBC application stack. +func (im IBCModule) SetICS4Wrapper(wrapper porttypes.ICS4Wrapper) { + if wrapper == nil { + panic("ICS4Wrapper cannot be nil") + } + + im.keeper.WithICS4Wrapper(wrapper) +} + +/// Keeper file that has ICS4Wrapper internal to its own struct + +// Keeper defines the IBC fungible transfer keeper +type Keeper struct { + ... + ics4Wrapper porttypes.ICS4Wrapper + + // Keeper is initialized with ICS4Wrapper + // being equal to the top-level channelKeeper + // this can be changed by calling WithICS4Wrapper + // with a different middleware ICS4Wrapper + channelKeeper types.ChannelKeeper + ... +} + +// WithICS4Wrapper sets the ICS4Wrapper. This function may be used after +// the keepers creation to set the middleware which is above this module +// in the IBC application stack. +func (k *Keeper) WithICS4Wrapper(wrapper porttypes.ICS4Wrapper) { + k.ics4Wrapper = wrapper +} +``` + +# Migration for Middleware Developers + +Since Middleware is itself implement the IBC application interface, it must also implement `SetICS4Wrapper` in the same way as IBC applications. + +Additionally, IBC Middleware has an underlying IBC application that it calls into as well. Previously this application would be set in the middleware upon construction. With the stack builder primitive, the application is only set during upon calling `stack.Build()`. Thus, middleware is additionally responsible for implementing the new method: `SetUnderlyingApplication`: + +```go +// SetUnderlyingModule sets the underlying IBC module. This function may be used after +// the middleware's initialization to set the ibc module which is below this middleware. +SetUnderlyingApplication(IBCModule) +``` + +The initialization should not include the ICS4Wrapper and application as this gets set later. The constructor function for Middlewares **must** be modified to return a **pointer** reference so that it can be modified in place by the stack builder. + +Below is an example middleware setup: + +```go +// IBCMiddleware implements the ICS26 callbacks +type IBCMiddleware struct { + app porttypes.PacketUnmarshalerModule + ics4Wrapper porttypes.ICS4Wrapper + + // this is a stateful middleware with its own internal keeper + mwKeeper *keeper.MiddlewareKeeper + + // this is a middleware specific field + mwField any +} + +// NewIBCMiddleware creates a new IBCMiddleware given the keeper and underlying application. +// NOTE: It **must** return a pointer reference so it can be +// modified in place by the stack builder +// NOTE: We do not pass in the underlying app and ICS4Wrapper here as this happens later +func NewIBCMiddleware( + mwKeeper *keeper.MiddlewareKeeper, mwField any, +) *IBCMiddleware { + return &IBCMiddleware{ + mwKeeper: mwKeeper, + mwField, mwField, + } +} + +// SetICS4Wrapper sets the ICS4Wrapper. This function may be used after the +// middleware's creation to set the middleware which is above this module in +// the IBC application stack. +func (im *IBCMiddleware) SetICS4Wrapper(wrapper porttypes.ICS4Wrapper) { + if wrapper == nil { + panic("ICS4Wrapper cannot be nil") + } + im.mwKeeper.WithICS4Wrapper(wrapper) +} + +// SetUnderlyingApplication sets the underlying IBC module. This function may be used after +// the middleware's creation to set the ibc module which is below this middleware. +func (im *IBCMiddleware) SetUnderlyingApplication(app porttypes.IBCModule) { + if app == nil { + panic(errors.New("underlying application cannot be nil")) + } + if im.app != nil { + panic(errors.New("underlying application already set")) + } + im.app = app +} +``` diff --git a/e2e/go.mod b/e2e/go.mod index 0da46977380..feb07233902 100644 --- a/e2e/go.mod +++ b/e2e/go.mod @@ -2,6 +2,9 @@ module github.com/cosmos/ibc-go/e2e go 1.24.3 +// TODO: Remove when v11 release of interchaintest is available (that is where this one is coming from) +replace github.com/cosmos/interchain-security/v7 => github.com/cosmos/interchain-security/v7 v7.0.0-20250622154438-73c73cf686e5 + replace ( github.com/cosmos/ibc-go/modules/light-clients/08-wasm/v10 => ../modules/light-clients/08-wasm // uncomment to use the local version of ibc-go, you will need to run `go mod tidy` in e2e directory. diff --git a/e2e/go.sum b/e2e/go.sum index ffc1b5404d6..a6022558730 100644 --- a/e2e/go.sum +++ b/e2e/go.sum @@ -856,8 +856,8 @@ github.com/cosmos/iavl v1.2.4 h1:IHUrG8dkyueKEY72y92jajrizbkZKPZbMmG14QzsEkw= github.com/cosmos/iavl v1.2.4/go.mod h1:GiM43q0pB+uG53mLxLDzimxM9l/5N9UuSY3/D0huuVw= github.com/cosmos/ics23/go v0.11.0 h1:jk5skjT0TqX5e5QJbEnwXIS2yI2vnmLOgpQPeM5RtnU= github.com/cosmos/ics23/go v0.11.0/go.mod h1:A8OjxPE67hHST4Icw94hOxxFEJMBG031xIGF/JHNIY0= -github.com/cosmos/interchain-security/v7 v7.0.0-20250408210344-06e0dc6bf6d6 h1:SzJ/+uqrTsJmI+f/GqPdC4lGxgDQKYvtRCMXFdJljNM= -github.com/cosmos/interchain-security/v7 v7.0.0-20250408210344-06e0dc6bf6d6/go.mod h1:W7JHsNaZ5XoH88cKT+wuCRsXkx/Fcn2kEwzpeGdJBxI= +github.com/cosmos/interchain-security/v7 v7.0.0-20250622154438-73c73cf686e5 h1:6LnlaeVk/wHPicQG8NqElL1F1FDVGEl7xF0JXGGhgEs= +github.com/cosmos/interchain-security/v7 v7.0.0-20250622154438-73c73cf686e5/go.mod h1:9EIcx4CzKt/5/2KHtniyzt7Kz8Wgk6fdvyr+AFIUGHc= github.com/cosmos/interchaintest/v10 v10.0.0 h1:DEsXOS10x191Q3EU4RkOnyqahGCTnLaBGEN//C2MvUQ= github.com/cosmos/interchaintest/v10 v10.0.0/go.mod h1:caS4BRkAg8NkiZ8BsHEzjNBibt2OVdTctW5Ezz+Jqxs= github.com/cosmos/ledger-cosmos-go v0.14.0 h1:WfCHricT3rPbkPSVKRH+L4fQGKYHuGOK9Edpel8TYpE= diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index 99a7ae2d80f..6a13ca932c5 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -26,34 +26,53 @@ var ( // ICA controller keeper and the underlying application. type IBCMiddleware struct { app porttypes.IBCModule - keeper keeper.Keeper + keeper *keeper.Keeper } // NewIBCMiddleware creates a new IBCMiddleware given the associated keeper. // The underlying application is set to nil and authentication is assumed to // be performed by a Cosmos SDK module that sends messages to controller message server. -func NewIBCMiddleware(k keeper.Keeper) IBCMiddleware { - return IBCMiddleware{ +func NewIBCMiddleware(k *keeper.Keeper) *IBCMiddleware { + return &IBCMiddleware{ app: nil, keeper: k, } } // NewIBCMiddlewareWithAuth creates a new IBCMiddleware given the associated keeper and underlying application -func NewIBCMiddlewareWithAuth(app porttypes.IBCModule, k keeper.Keeper) IBCMiddleware { - return IBCMiddleware{ +func NewIBCMiddlewareWithAuth(app porttypes.IBCModule, k *keeper.Keeper) *IBCMiddleware { + return &IBCMiddleware{ app: app, keeper: k, } } +// SetUnderlyingApplication sets the underlying application for the middleware. +func (im *IBCMiddleware) SetUnderlyingApplication(app porttypes.IBCModule) { + if app == nil { + panic(errors.New("underlying application cannot be nil")) + } + if im.app != nil { + panic(errors.New("underlying application already set")) + } + im.app = app +} + +// SetICS4Wrapper sets the ICS4Wrapper for the middleware. +func (im *IBCMiddleware) SetICS4Wrapper(ics4Wrapper porttypes.ICS4Wrapper) { + if ics4Wrapper == nil { + panic(errors.New("ICS4Wrapper cannot be nil")) + } + im.keeper.WithICS4Wrapper(ics4Wrapper) +} + // OnChanOpenInit implements the IBCMiddleware interface // // Interchain Accounts is implemented to act as middleware for connected authentication modules on // the controller side. The connected modules may not change the controller side portID or // version. They will be allowed to perform custom logic without changing // the parameters stored within a channel struct. -func (im IBCMiddleware) OnChanOpenInit( +func (im *IBCMiddleware) OnChanOpenInit( ctx sdk.Context, order channeltypes.Order, connectionHops []string, @@ -84,7 +103,7 @@ func (im IBCMiddleware) OnChanOpenInit( } // OnChanOpenTry implements the IBCMiddleware interface -func (IBCMiddleware) OnChanOpenTry( +func (*IBCMiddleware) OnChanOpenTry( ctx sdk.Context, order channeltypes.Order, connectionHops []string, @@ -102,7 +121,7 @@ func (IBCMiddleware) OnChanOpenTry( // the controller side. The connected modules may not change the portID or // version. They will be allowed to perform custom logic without changing // the parameters stored within a channel struct. -func (im IBCMiddleware) OnChanOpenAck( +func (im *IBCMiddleware) OnChanOpenAck( ctx sdk.Context, portID, channelID string, @@ -131,7 +150,7 @@ func (im IBCMiddleware) OnChanOpenAck( } // OnChanOpenConfirm implements the IBCMiddleware interface -func (IBCMiddleware) OnChanOpenConfirm( +func (*IBCMiddleware) OnChanOpenConfirm( ctx sdk.Context, portID, channelID string, @@ -140,7 +159,7 @@ func (IBCMiddleware) OnChanOpenConfirm( } // OnChanCloseInit implements the IBCMiddleware interface -func (IBCMiddleware) OnChanCloseInit( +func (*IBCMiddleware) OnChanCloseInit( ctx sdk.Context, portID, channelID string, @@ -150,7 +169,7 @@ func (IBCMiddleware) OnChanCloseInit( } // OnChanCloseConfirm implements the IBCMiddleware interface -func (im IBCMiddleware) OnChanCloseConfirm( +func (im *IBCMiddleware) OnChanCloseConfirm( ctx sdk.Context, portID, channelID string, @@ -172,7 +191,7 @@ func (im IBCMiddleware) OnChanCloseConfirm( } // OnRecvPacket implements the IBCMiddleware interface -func (IBCMiddleware) OnRecvPacket( +func (*IBCMiddleware) OnRecvPacket( ctx sdk.Context, _ string, packet channeltypes.Packet, @@ -185,7 +204,7 @@ func (IBCMiddleware) OnRecvPacket( } // OnAcknowledgementPacket implements the IBCMiddleware interface -func (im IBCMiddleware) OnAcknowledgementPacket( +func (im *IBCMiddleware) OnAcknowledgementPacket( ctx sdk.Context, channelVersion string, packet channeltypes.Packet, @@ -210,7 +229,7 @@ func (im IBCMiddleware) OnAcknowledgementPacket( } // OnTimeoutPacket implements the IBCMiddleware interface -func (im IBCMiddleware) OnTimeoutPacket( +func (im *IBCMiddleware) OnTimeoutPacket( ctx sdk.Context, channelVersion string, packet channeltypes.Packet, @@ -237,7 +256,7 @@ func (im IBCMiddleware) OnTimeoutPacket( } // SendPacket implements the ICS4 Wrapper interface -func (IBCMiddleware) SendPacket( +func (*IBCMiddleware) SendPacket( ctx sdk.Context, sourcePort string, sourceChannel string, @@ -249,7 +268,7 @@ func (IBCMiddleware) SendPacket( } // WriteAcknowledgement implements the ICS4 Wrapper interface -func (IBCMiddleware) WriteAcknowledgement( +func (*IBCMiddleware) WriteAcknowledgement( ctx sdk.Context, packet ibcexported.PacketI, ack ibcexported.Acknowledgement, @@ -258,14 +277,14 @@ func (IBCMiddleware) WriteAcknowledgement( } // GetAppVersion returns the interchain accounts metadata. -func (im IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) { +func (im *IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) { return im.keeper.GetAppVersion(ctx, portID, channelID) } // UnmarshalPacketData attempts to unmarshal the provided packet data bytes // into an InterchainAccountPacketData. This function implements the optional // PacketDataUnmarshaler interface required for ADR 008 support. -func (im IBCMiddleware) UnmarshalPacketData(ctx sdk.Context, portID string, channelID string, bz []byte) (any, string, error) { +func (im *IBCMiddleware) UnmarshalPacketData(ctx sdk.Context, portID string, channelID string, bz []byte) (any, string, error) { var data icatypes.InterchainAccountPacketData err := data.UnmarshalJSON(bz) if err != nil { diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go index f3f895be070..1725ca21b71 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go @@ -19,6 +19,7 @@ import ( host "github.com/cosmos/ibc-go/v10/modules/core/24-host" ibcerrors "github.com/cosmos/ibc-go/v10/modules/core/errors" ibctesting "github.com/cosmos/ibc-go/v10/testing" + ibcmock "github.com/cosmos/ibc-go/v10/testing/mock" ) const invalidVersion = "invalid|version" @@ -108,6 +109,88 @@ func SetupICAPath(path *ibctesting.Path, owner string) error { return path.EndpointB.ChanOpenConfirm() } +func (s *InterchainAccountsTestSuite) TestSetUnderlyingApplication() { + var ( + app porttypes.IBCModule + mw porttypes.Middleware + ) + testCases := []struct { + name string + malleate func() + expPanic bool + }{ + { + "success", func() {}, false, + }, + { + "nil underlying app", func() { + app = nil + }, true, + }, + { + "app already set", func() { + mw.SetUnderlyingApplication(&ibcmock.IBCModule{}) + }, true, + }, + } + + for _, tc := range testCases { + s.Run(tc.name, func() { + s.SetupTest() // reset + + app = &ibcmock.IBCModule{} + mw = controller.NewIBCMiddleware(s.chainA.GetSimApp().ICAControllerKeeper) + + tc.malleate() // malleate mutates test data + + if tc.expPanic { + s.Require().Panics(func() { + mw.SetUnderlyingApplication(app) + }) + } else { + s.Require().NotPanics(func() { + mw.SetUnderlyingApplication(app) + }) + } + }) + } +} + +func (s *InterchainAccountsTestSuite) TestSetICS4Wrapper() { + var wrapper porttypes.ICS4Wrapper + mw := controller.NewIBCMiddleware(s.chainA.GetSimApp().ICAControllerKeeper) + testCases := []struct { + name string + malleate func() + expPanic bool + }{ + { + "success", func() {}, false, + }, + { + "nil ICS4Wrapper", func() { + wrapper = nil + }, true, + }, + } + for _, tc := range testCases { + s.Run(tc.name, func() { + s.SetupTest() // reset + wrapper = s.chainA.GetSimApp().GetIBCKeeper().ChannelKeeper + tc.malleate() // malleate mutates test data + if tc.expPanic { + s.Require().Panics(func() { + mw.SetICS4Wrapper(wrapper) + }) + } else { + s.Require().NotPanics(func() { + mw.SetICS4Wrapper(wrapper) + }) + } + }) + } +} + func (s *InterchainAccountsTestSuite) TestOnChanOpenInit() { var ( channel *channeltypes.Channel @@ -816,7 +899,7 @@ func (s *InterchainAccountsTestSuite) TestInFlightHandshakeRespectsGoAPICaller() s.Require().NoError(err) // attempt to start a second handshake via the controller msg server - msgServer := controllerkeeper.NewMsgServerImpl(&s.chainA.GetSimApp().ICAControllerKeeper) + msgServer := controllerkeeper.NewMsgServerImpl(s.chainA.GetSimApp().ICAControllerKeeper) msgRegisterInterchainAccount := types.NewMsgRegisterInterchainAccount(path.EndpointA.ConnectionID, s.chainA.SenderAccount.GetAddress().String(), TestVersion, ordering) res, err := msgServer.RegisterInterchainAccount(s.chainA.GetContext(), msgRegisterInterchainAccount) @@ -833,7 +916,7 @@ func (s *InterchainAccountsTestSuite) TestInFlightHandshakeRespectsMsgServerCall path.SetupConnections() // initiate a channel handshake such that channel.State == INIT - msgServer := controllerkeeper.NewMsgServerImpl(&s.chainA.GetSimApp().ICAControllerKeeper) + msgServer := controllerkeeper.NewMsgServerImpl(s.chainA.GetSimApp().ICAControllerKeeper) msgRegisterInterchainAccount := types.NewMsgRegisterInterchainAccount(path.EndpointA.ConnectionID, s.chainA.SenderAccount.GetAddress().String(), TestVersion, ordering) res, err := msgServer.RegisterInterchainAccount(s.chainA.GetContext(), msgRegisterInterchainAccount) @@ -868,7 +951,7 @@ func (s *InterchainAccountsTestSuite) TestClosedChannelReopensWithMsgServer() { channelSeq := s.chainA.GetSimApp().GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(s.chainA.GetContext()) // route a new MsgRegisterInterchainAccount in order to reopen the - msgServer := controllerkeeper.NewMsgServerImpl(&s.chainA.GetSimApp().ICAControllerKeeper) + msgServer := controllerkeeper.NewMsgServerImpl(s.chainA.GetSimApp().ICAControllerKeeper) msgRegisterInterchainAccount := types.NewMsgRegisterInterchainAccount(path.EndpointA.ConnectionID, s.chainA.SenderAccount.GetAddress().String(), path.EndpointA.ChannelConfig.Version, ordering) res, err := msgServer.RegisterInterchainAccount(s.chainA.GetContext(), msgRegisterInterchainAccount) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/genesis_test.go b/modules/apps/27-interchain-accounts/controller/keeper/genesis_test.go index 9bd77154676..6707c81fafe 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/genesis_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/genesis_test.go @@ -52,7 +52,7 @@ func (s *KeeperTestSuite) TestInitGenesis() { tc.malleate() - keeper.InitGenesis(s.chainA.GetContext(), s.chainA.GetSimApp().ICAControllerKeeper, genesisState) + keeper.InitGenesis(s.chainA.GetContext(), *s.chainA.GetSimApp().ICAControllerKeeper, genesisState) channelID, found := s.chainA.GetSimApp().ICAControllerKeeper.GetActiveChannelID(s.chainA.GetContext(), ibctesting.FirstConnectionID, TestPortID) s.Require().True(found) @@ -93,7 +93,7 @@ func (s *KeeperTestSuite) TestExportGenesis() { interchainAccAddr, exists := s.chainB.GetSimApp().ICAHostKeeper.GetInterchainAccountAddress(s.chainB.GetContext(), path.EndpointB.ConnectionID, path.EndpointA.ChannelConfig.PortID) s.Require().True(exists) - genesisState := keeper.ExportGenesis(s.chainA.GetContext(), s.chainA.GetSimApp().ICAControllerKeeper) + genesisState := keeper.ExportGenesis(s.chainA.GetContext(), *s.chainA.GetSimApp().ICAControllerKeeper) s.Require().Equal(path.EndpointA.ChannelID, genesisState.ActiveChannels[0].ChannelId) s.Require().Equal(path.EndpointA.ChannelConfig.PortID, genesisState.ActiveChannels[0].PortId) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/keeper.go b/modules/apps/27-interchain-accounts/controller/keeper/keeper.go index cebff9d3d15..3482ea6a554 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/keeper.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/keeper.go @@ -41,17 +41,19 @@ type Keeper struct { // NewKeeper creates a new interchain accounts controller Keeper instance func NewKeeper( cdc codec.Codec, storeService corestore.KVStoreService, - ics4Wrapper porttypes.ICS4Wrapper, channelKeeper icatypes.ChannelKeeper, + channelKeeper icatypes.ChannelKeeper, msgRouter icatypes.MessageRouter, authority string, -) Keeper { +) *Keeper { if strings.TrimSpace(authority) == "" { panic(errors.New("authority must be non-empty")) } - return Keeper{ - storeService: storeService, - cdc: cdc, - ics4Wrapper: ics4Wrapper, + return &Keeper{ + storeService: storeService, + cdc: cdc, + // Defaults to using the channel keeper as the ICS4Wrapper + // This can be overridden later with WithICS4Wrapper (e.g. by the middleware stack wiring) + ics4Wrapper: channelKeeper, channelKeeper: channelKeeper, msgRouter: msgRouter, authority: authority, diff --git a/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go b/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go index 1c89fdcc400..079d8ed02e9 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go @@ -120,7 +120,6 @@ func (s *KeeperTestSuite) TestNewKeeper() { s.chainA.GetSimApp().AppCodec(), runtime.NewKVStoreService(s.chainA.GetSimApp().GetKey(types.StoreKey)), s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, - s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, s.chainA.GetSimApp().MsgServiceRouter(), s.chainA.GetSimApp().ICAControllerKeeper.GetAuthority(), ) @@ -130,7 +129,6 @@ func (s *KeeperTestSuite) TestNewKeeper() { s.chainA.GetSimApp().AppCodec(), runtime.NewKVStoreService(s.chainA.GetSimApp().GetKey(types.StoreKey)), s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, - s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, s.chainA.GetSimApp().MsgServiceRouter(), "", // authority ) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/msg_server_test.go b/modules/apps/27-interchain-accounts/controller/keeper/msg_server_test.go index 120deb7eb3d..f2dc87ad387 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/msg_server_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/msg_server_test.go @@ -80,7 +80,7 @@ func (s *KeeperTestSuite) TestRegisterInterchainAccount_MsgServer() { tc.malleate() ctx := s.chainA.GetContext() - msgServer := keeper.NewMsgServerImpl(&s.chainA.GetSimApp().ICAControllerKeeper) + msgServer := keeper.NewMsgServerImpl(s.chainA.GetSimApp().ICAControllerKeeper) res, err := msgServer.RegisterInterchainAccount(ctx, msg) if tc.expErr == nil { @@ -186,7 +186,7 @@ func (s *KeeperTestSuite) TestSubmitTx() { tc.malleate() // malleate mutates test data ctx := s.chainA.GetContext() - msgServer := keeper.NewMsgServerImpl(&s.chainA.GetSimApp().ICAControllerKeeper) + msgServer := keeper.NewMsgServerImpl(s.chainA.GetSimApp().ICAControllerKeeper) res, err := msgServer.SendTx(ctx, msg) if tc.expErr == nil { diff --git a/modules/apps/27-interchain-accounts/host/ibc_module.go b/modules/apps/27-interchain-accounts/host/ibc_module.go index 484fc1b451e..7d88dcd5c8f 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module.go @@ -23,18 +23,18 @@ var ( // IBCModule implements the ICS26 interface for interchain accounts host chains type IBCModule struct { - keeper keeper.Keeper + keeper *keeper.Keeper } // NewIBCModule creates a new IBCModule given the associated keeper -func NewIBCModule(k keeper.Keeper) IBCModule { - return IBCModule{ +func NewIBCModule(k *keeper.Keeper) *IBCModule { + return &IBCModule{ keeper: k, } } // OnChanOpenInit implements the IBCModule interface -func (IBCModule) OnChanOpenInit( +func (*IBCModule) OnChanOpenInit( _ sdk.Context, _ channeltypes.Order, _ []string, @@ -47,7 +47,7 @@ func (IBCModule) OnChanOpenInit( } // OnChanOpenTry implements the IBCModule interface -func (im IBCModule) OnChanOpenTry( +func (im *IBCModule) OnChanOpenTry( ctx sdk.Context, order channeltypes.Order, connectionHops []string, @@ -64,7 +64,7 @@ func (im IBCModule) OnChanOpenTry( } // OnChanOpenAck implements the IBCModule interface -func (IBCModule) OnChanOpenAck( +func (*IBCModule) OnChanOpenAck( _ sdk.Context, _, _ string, @@ -75,7 +75,7 @@ func (IBCModule) OnChanOpenAck( } // OnChanOpenConfirm implements the IBCModule interface -func (im IBCModule) OnChanOpenConfirm( +func (im *IBCModule) OnChanOpenConfirm( ctx sdk.Context, portID, channelID string, @@ -88,7 +88,7 @@ func (im IBCModule) OnChanOpenConfirm( } // OnChanCloseInit implements the IBCModule interface -func (IBCModule) OnChanCloseInit( +func (*IBCModule) OnChanCloseInit( _ sdk.Context, _ string, _ string, @@ -98,7 +98,7 @@ func (IBCModule) OnChanCloseInit( } // OnChanCloseConfirm implements the IBCModule interface -func (im IBCModule) OnChanCloseConfirm( +func (im *IBCModule) OnChanCloseConfirm( ctx sdk.Context, portID, channelID string, @@ -107,7 +107,7 @@ func (im IBCModule) OnChanCloseConfirm( } // OnRecvPacket implements the IBCModule interface -func (im IBCModule) OnRecvPacket( +func (im *IBCModule) OnRecvPacket( ctx sdk.Context, _ string, packet channeltypes.Packet, @@ -136,7 +136,7 @@ func (im IBCModule) OnRecvPacket( } // OnAcknowledgementPacket implements the IBCModule interface -func (IBCModule) OnAcknowledgementPacket( +func (*IBCModule) OnAcknowledgementPacket( _ sdk.Context, _ string, _ channeltypes.Packet, @@ -147,7 +147,7 @@ func (IBCModule) OnAcknowledgementPacket( } // OnTimeoutPacket implements the IBCModule interface -func (IBCModule) OnTimeoutPacket( +func (*IBCModule) OnTimeoutPacket( _ sdk.Context, _ string, _ channeltypes.Packet, @@ -159,7 +159,7 @@ func (IBCModule) OnTimeoutPacket( // UnmarshalPacketData attempts to unmarshal the provided packet data bytes // into an InterchainAccountPacketData. This function implements the optional // PacketDataUnmarshaler interface required for ADR 008 support. -func (im IBCModule) UnmarshalPacketData(ctx sdk.Context, portID string, channelID string, bz []byte) (any, string, error) { +func (im *IBCModule) UnmarshalPacketData(ctx sdk.Context, portID string, channelID string, bz []byte) (any, string, error) { var data icatypes.InterchainAccountPacketData err := data.UnmarshalJSON(bz) if err != nil { @@ -173,3 +173,11 @@ func (im IBCModule) UnmarshalPacketData(ctx sdk.Context, portID string, channelI return data, version, nil } + +// SetICS4Wrapper sets the ICS4Wrapper for the IBCModule. +func (im *IBCModule) SetICS4Wrapper(wrapper porttypes.ICS4Wrapper) { + if wrapper == nil { + panic("ICS4Wrapper cannot be nil") + } + im.keeper.WithICS4Wrapper(wrapper) +} diff --git a/modules/apps/27-interchain-accounts/host/ibc_module_test.go b/modules/apps/27-interchain-accounts/host/ibc_module_test.go index aa6fa81231b..fddc39749bb 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module_test.go @@ -112,6 +112,26 @@ func SetupICAPath(path *ibctesting.Path, owner string) error { return path.EndpointB.ChanOpenConfirm() } +func (s *InterchainAccountsTestSuite) TestSetICS4Wrapper() { + s.SetupTest() // reset + + module := icahost.NewIBCModule(s.chainB.GetSimApp().ICAHostKeeper) + + s.Require().Panics(func() { + module.SetICS4Wrapper(nil) + }, "ICS4Wrapper should not be nil") + + // set ICS4Wrapper + s.Require().NotPanics(func() { + module.SetICS4Wrapper(s.chainB.GetSimApp().IBCKeeper.ChannelKeeper) + }) + + // verify ICS4Wrapper is set + ics4Wrapper := s.chainB.GetSimApp().ICAHostKeeper.GetICS4Wrapper() + s.Require().NotNil(ics4Wrapper) + s.Require().Equal(s.chainB.GetSimApp().IBCKeeper.ChannelKeeper, ics4Wrapper) +} + // Test initiating a ChanOpenInit using the host chain instead of the controller chain // ChainA is the controller chain. ChainB is the host chain func (s *InterchainAccountsTestSuite) TestChanOpenInit() { diff --git a/modules/apps/27-interchain-accounts/host/keeper/genesis_test.go b/modules/apps/27-interchain-accounts/host/keeper/genesis_test.go index c9763bea400..41448fee2cb 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/genesis_test.go +++ b/modules/apps/27-interchain-accounts/host/keeper/genesis_test.go @@ -29,7 +29,7 @@ func (s *KeeperTestSuite) TestInitGenesis() { Port: icatypes.HostPortID, } - keeper.InitGenesis(s.chainA.GetContext(), s.chainA.GetSimApp().ICAHostKeeper, genesisState) + keeper.InitGenesis(s.chainA.GetContext(), *s.chainA.GetSimApp().ICAHostKeeper, genesisState) channelID, found := s.chainA.GetSimApp().ICAHostKeeper.GetActiveChannelID(s.chainA.GetContext(), ibctesting.FirstConnectionID, TestPortID) s.Require().True(found) @@ -83,7 +83,7 @@ func (s *KeeperTestSuite) TestGenesisParams() { Params: tc.input, } if tc.expPanicMsg == "" { - keeper.InitGenesis(s.chainA.GetContext(), s.chainA.GetSimApp().ICAHostKeeper, genesisState) + keeper.InitGenesis(s.chainA.GetContext(), *s.chainA.GetSimApp().ICAHostKeeper, genesisState) channelID, found := s.chainA.GetSimApp().ICAHostKeeper.GetActiveChannelID(s.chainA.GetContext(), ibctesting.FirstConnectionID, TestPortID) s.Require().True(found) @@ -98,7 +98,7 @@ func (s *KeeperTestSuite) TestGenesisParams() { s.Require().Equal(expParams, params) } else { s.PanicsWithError(tc.expPanicMsg, func() { - keeper.InitGenesis(s.chainA.GetContext(), s.chainA.GetSimApp().ICAHostKeeper, genesisState) + keeper.InitGenesis(s.chainA.GetContext(), *s.chainA.GetSimApp().ICAHostKeeper, genesisState) }) } }) @@ -118,7 +118,7 @@ func (s *KeeperTestSuite) TestExportGenesis() { interchainAccAddr, exists := s.chainB.GetSimApp().ICAHostKeeper.GetInterchainAccountAddress(s.chainB.GetContext(), path.EndpointB.ConnectionID, path.EndpointA.ChannelConfig.PortID) s.Require().True(exists) - genesisState := keeper.ExportGenesis(s.chainB.GetContext(), s.chainB.GetSimApp().ICAHostKeeper) + genesisState := keeper.ExportGenesis(s.chainB.GetContext(), *s.chainB.GetSimApp().ICAHostKeeper) s.Require().Equal(path.EndpointB.ChannelID, genesisState.ActiveChannels[0].ChannelId) s.Require().Equal(path.EndpointA.ChannelConfig.PortID, genesisState.ActiveChannels[0].PortId) diff --git a/modules/apps/27-interchain-accounts/host/keeper/keeper.go b/modules/apps/27-interchain-accounts/host/keeper/keeper.go index c9fc5fbae04..8f4df2f03f0 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/keeper.go +++ b/modules/apps/27-interchain-accounts/host/keeper/keeper.go @@ -52,9 +52,9 @@ type Keeper struct { // NewKeeper creates a new interchain accounts host Keeper instance func NewKeeper( cdc codec.Codec, storeService corestore.KVStoreService, - ics4Wrapper porttypes.ICS4Wrapper, channelKeeper icatypes.ChannelKeeper, + channelKeeper icatypes.ChannelKeeper, accountKeeper icatypes.AccountKeeper, msgRouter icatypes.MessageRouter, queryRouter icatypes.QueryRouter, authority string, -) Keeper { +) *Keeper { // ensure ibc interchain accounts module account is set if addr := accountKeeper.GetModuleAddress(icatypes.ModuleName); addr == nil { panic(errors.New("the Interchain Accounts module account has not been set")) @@ -64,10 +64,12 @@ func NewKeeper( panic(errors.New("authority must be non-empty")) } - return Keeper{ - storeService: storeService, - cdc: cdc, - ics4Wrapper: ics4Wrapper, + return &Keeper{ + storeService: storeService, + cdc: cdc, + // Defaults to using the channel keeper as the ICS4Wrapper + // This can be overridden later with WithICS4Wrapper (e.g. by the middleware stack wiring) + ics4Wrapper: channelKeeper, channelKeeper: channelKeeper, accountKeeper: accountKeeper, msgRouter: msgRouter, diff --git a/modules/apps/27-interchain-accounts/host/keeper/keeper_test.go b/modules/apps/27-interchain-accounts/host/keeper/keeper_test.go index 5528abc5197..df670a38ac9 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/keeper_test.go +++ b/modules/apps/27-interchain-accounts/host/keeper/keeper_test.go @@ -143,7 +143,6 @@ func (s *KeeperTestSuite) TestNewKeeper() { s.chainA.GetSimApp().AppCodec(), runtime.NewKVStoreService(s.chainA.GetSimApp().GetKey(types.StoreKey)), s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, - s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, s.chainA.GetSimApp().AccountKeeper, s.chainA.GetSimApp().MsgServiceRouter(), s.chainA.GetSimApp().GRPCQueryRouter(), @@ -155,7 +154,6 @@ func (s *KeeperTestSuite) TestNewKeeper() { s.chainA.GetSimApp().AppCodec(), runtime.NewKVStoreService(s.chainA.GetSimApp().GetKey(types.StoreKey)), s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, - s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, authkeeper.AccountKeeper{}, // empty account keeper s.chainA.GetSimApp().MsgServiceRouter(), s.chainA.GetSimApp().GRPCQueryRouter(), @@ -167,7 +165,6 @@ func (s *KeeperTestSuite) TestNewKeeper() { s.chainA.GetSimApp().AppCodec(), runtime.NewKVStoreService(s.chainA.GetSimApp().GetKey(types.StoreKey)), s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, - s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, s.chainA.GetSimApp().AccountKeeper, s.chainA.GetSimApp().MsgServiceRouter(), s.chainA.GetSimApp().GRPCQueryRouter(), diff --git a/modules/apps/27-interchain-accounts/host/keeper/msg_server_test.go b/modules/apps/27-interchain-accounts/host/keeper/msg_server_test.go index 6e225bbaed1..903d835a365 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/msg_server_test.go +++ b/modules/apps/27-interchain-accounts/host/keeper/msg_server_test.go @@ -134,7 +134,7 @@ func (s *KeeperTestSuite) TestModuleQuerySafe() { tc.malleate() ctx := s.chainA.GetContext() - msgServer := keeper.NewMsgServerImpl(&s.chainA.GetSimApp().ICAHostKeeper) + msgServer := keeper.NewMsgServerImpl(s.chainA.GetSimApp().ICAHostKeeper) res, err := msgServer.ModuleQuerySafe(ctx, msg) if tc.expErr == nil { @@ -173,7 +173,7 @@ func (s *KeeperTestSuite) TestUpdateParams() { s.SetupTest() ctx := s.chainA.GetContext() - msgServer := keeper.NewMsgServerImpl(&s.chainA.GetSimApp().ICAHostKeeper) + msgServer := keeper.NewMsgServerImpl(s.chainA.GetSimApp().ICAHostKeeper) res, err := msgServer.UpdateParams(ctx, tc.msg) if tc.expErr == nil { diff --git a/modules/apps/27-interchain-accounts/types/expected_keepers.go b/modules/apps/27-interchain-accounts/types/expected_keepers.go index d0447eb62db..4f4c194d153 100644 --- a/modules/apps/27-interchain-accounts/types/expected_keepers.go +++ b/modules/apps/27-interchain-accounts/types/expected_keepers.go @@ -7,6 +7,7 @@ import ( connectiontypes "github.com/cosmos/ibc-go/v10/modules/core/03-connection/types" channeltypes "github.com/cosmos/ibc-go/v10/modules/core/04-channel/types" + porttypes "github.com/cosmos/ibc-go/v10/modules/core/05-port/types" ) // AccountKeeper defines the expected account keeper @@ -20,6 +21,7 @@ type AccountKeeper interface { // ChannelKeeper defines the expected IBC channel keeper type ChannelKeeper interface { + porttypes.ICS4Wrapper GetChannel(ctx sdk.Context, srcPort, srcChan string) (channel channeltypes.Channel, found bool) GetNextSequenceSend(ctx sdk.Context, portID, channelID string) (uint64, bool) GetConnection(ctx sdk.Context, connectionID string) (connectiontypes.ConnectionEnd, error) diff --git a/modules/apps/callbacks/ibc_middleware.go b/modules/apps/callbacks/ibc_middleware.go index a76ce36dc83..df36f7171a1 100644 --- a/modules/apps/callbacks/ibc_middleware.go +++ b/modules/apps/callbacks/ibc_middleware.go @@ -22,7 +22,7 @@ var ( // IBCMiddleware implements the ICS26 callbacks for the ibc-callbacks middleware given // the underlying application. type IBCMiddleware struct { - app porttypes.PacketUnmarshalarModule + app porttypes.PacketUnmarshalerModule ics4Wrapper porttypes.ICS4Wrapper contractKeeper types.ContractKeeper @@ -37,18 +37,8 @@ type IBCMiddleware struct { // NewIBCMiddleware creates a new IBCMiddleware given the keeper and underlying application. // The underlying application must implement the required callback interfaces. func NewIBCMiddleware( - app porttypes.IBCModule, ics4Wrapper porttypes.ICS4Wrapper, contractKeeper types.ContractKeeper, maxCallbackGas uint64, ) *IBCMiddleware { - packetDataUnmarshalerApp, ok := app.(porttypes.PacketUnmarshalarModule) - if !ok { - panic(fmt.Errorf("underlying application does not implement %T", (*porttypes.PacketUnmarshalarModule)(nil))) - } - - if ics4Wrapper == nil { - panic(errors.New("ICS4Wrapper cannot be nil")) - } - if contractKeeper == nil { panic(errors.New("contract keeper cannot be nil")) } @@ -58,20 +48,38 @@ func NewIBCMiddleware( } return &IBCMiddleware{ - app: packetDataUnmarshalerApp, - ics4Wrapper: ics4Wrapper, contractKeeper: contractKeeper, maxCallbackGas: maxCallbackGas, } } -// WithICS4Wrapper sets the ICS4Wrapper. This function may be used after the +// SetICS4Wrapper sets the ICS4Wrapper. This function may be used after the // middleware's creation to set the middleware which is above this module in // the IBC application stack. -func (im *IBCMiddleware) WithICS4Wrapper(wrapper porttypes.ICS4Wrapper) { +func (im *IBCMiddleware) SetICS4Wrapper(wrapper porttypes.ICS4Wrapper) { + if wrapper == nil { + panic("ICS4Wrapper cannot be nil") + } im.ics4Wrapper = wrapper } +// SetUnderlyingApplication sets the underlying IBC module. This function may be used after +// the middleware's creation to set the ibc module which is below this middleware. +func (im *IBCMiddleware) SetUnderlyingApplication(app porttypes.IBCModule) { + if app == nil { + panic(errors.New("underlying application cannot be nil")) + } + if im.app != nil { + panic(errors.New("underlying application already set")) + } + // the underlying application must implement the PacketUnmarshalerModule interface + pdApp, ok := app.(porttypes.PacketUnmarshalerModule) + if !ok { + panic(fmt.Errorf("underlying application must implement PacketUnmarshalerModule, got %T", app)) + } + im.app = pdApp +} + // GetICS4Wrapper returns the ICS4Wrapper. func (im *IBCMiddleware) GetICS4Wrapper() porttypes.ICS4Wrapper { return im.ics4Wrapper diff --git a/modules/apps/callbacks/ibc_middleware_test.go b/modules/apps/callbacks/ibc_middleware_test.go index a2c6298286d..3d30079e986 100644 --- a/modules/apps/callbacks/ibc_middleware_test.go +++ b/modules/apps/callbacks/ibc_middleware_test.go @@ -35,35 +35,21 @@ func (s *CallbacksTestSuite) TestNewIBCMiddleware() { { "success", func() { - _ = ibccallbacks.NewIBCMiddleware(&ibcmock.IBCModule{}, &channelkeeper.Keeper{}, simapp.ContractKeeper{}, maxCallbackGas) + _ = ibccallbacks.NewIBCMiddleware(simapp.ContractKeeper{}, maxCallbackGas) }, nil, }, - { - "panics with nil underlying app", - func() { - _ = ibccallbacks.NewIBCMiddleware(nil, &channelkeeper.Keeper{}, simapp.ContractKeeper{}, maxCallbackGas) - }, - fmt.Errorf("underlying application does not implement %T", (*porttypes.PacketUnmarshalarModule)(nil)), - }, { "panics with nil contract keeper", func() { - _ = ibccallbacks.NewIBCMiddleware(&ibcmock.IBCModule{}, &channelkeeper.Keeper{}, nil, maxCallbackGas) + _ = ibccallbacks.NewIBCMiddleware(nil, maxCallbackGas) }, errors.New("contract keeper cannot be nil"), }, - { - "panics with nil ics4Wrapper", - func() { - _ = ibccallbacks.NewIBCMiddleware(&ibcmock.IBCModule{}, nil, simapp.ContractKeeper{}, maxCallbackGas) - }, - errors.New("ICS4Wrapper cannot be nil"), - }, { "panics with zero maxCallbackGas", func() { - _ = ibccallbacks.NewIBCMiddleware(&ibcmock.IBCModule{}, &channelkeeper.Keeper{}, simapp.ContractKeeper{}, uint64(0)) + _ = ibccallbacks.NewIBCMiddleware(simapp.ContractKeeper{}, uint64(0)) }, errors.New("maxCallbackGas cannot be zero"), }, @@ -80,18 +66,38 @@ func (s *CallbacksTestSuite) TestNewIBCMiddleware() { } } -func (s *CallbacksTestSuite) TestWithICS4Wrapper() { +func (s *CallbacksTestSuite) TestSetICS4Wrapper() { s.setupChains() cbsMiddleware := ibccallbacks.IBCMiddleware{} s.Require().Nil(cbsMiddleware.GetICS4Wrapper()) - cbsMiddleware.WithICS4Wrapper(s.chainA.App.GetIBCKeeper().ChannelKeeper) + s.Require().Panics(func() { + cbsMiddleware.SetICS4Wrapper(nil) + }, "expected panic when setting nil ICS4Wrapper") + + cbsMiddleware.SetICS4Wrapper(s.chainA.App.GetIBCKeeper().ChannelKeeper) ics4Wrapper := cbsMiddleware.GetICS4Wrapper() s.Require().IsType((*channelkeeper.Keeper)(nil), ics4Wrapper) } +func (s *CallbacksTestSuite) TestSetUnderlyingApplication() { + s.setupChains() + + cbsMiddleware := ibccallbacks.IBCMiddleware{} + + s.Require().Panics(func() { + cbsMiddleware.SetUnderlyingApplication(nil) + }, "expected panic when setting nil underlying application") + + cbsMiddleware.SetUnderlyingApplication(&ibcmock.IBCModule{}) + + s.Require().Panics(func() { + cbsMiddleware.SetUnderlyingApplication(&ibcmock.IBCModule{}) + }, "expected panic when setting underlying application a second time") +} + func (s *CallbacksTestSuite) TestSendPacket() { var packetData transfertypes.FungibleTokenPacketData var callbackExecuted bool @@ -1037,7 +1043,7 @@ func (s *CallbacksTestSuite) TestUnmarshalPacketDataV1() { transferStack, ok := s.chainA.App.GetIBCKeeper().PortKeeper.Route(transfertypes.ModuleName) s.Require().True(ok) - unmarshalerStack, ok := transferStack.(porttypes.PacketUnmarshalarModule) + unmarshalerStack, ok := transferStack.(porttypes.PacketUnmarshalerModule) s.Require().True(ok) expPacketDataICS20V1 := transfertypes.FungibleTokenPacketData{ diff --git a/modules/apps/callbacks/testing/simapp/app.go b/modules/apps/callbacks/testing/simapp/app.go index 6938453c424..c49cb80e0b6 100644 --- a/modules/apps/callbacks/testing/simapp/app.go +++ b/modules/apps/callbacks/testing/simapp/app.go @@ -147,9 +147,9 @@ type SimApp struct { GovKeeper govkeeper.Keeper UpgradeKeeper *upgradekeeper.Keeper IBCKeeper *ibckeeper.Keeper // IBC Keeper must be a pointer in the app, so we can SetRouter on it correctly - ICAControllerKeeper icacontrollerkeeper.Keeper - ICAHostKeeper icahostkeeper.Keeper - TransferKeeper ibctransferkeeper.Keeper + ICAControllerKeeper *icacontrollerkeeper.Keeper + ICAHostKeeper *icahostkeeper.Keeper + TransferKeeper *ibctransferkeeper.Keeper ConsensusParamsKeeper consensusparamkeeper.Keeper // mock contract keeper used for testing @@ -334,7 +334,6 @@ func NewSimApp( app.ICAControllerKeeper = icacontrollerkeeper.NewKeeper( appCodec, runtime.NewKVStoreService(keys[icacontrollertypes.StoreKey]), app.IBCKeeper.ChannelKeeper, - app.IBCKeeper.ChannelKeeper, app.MsgServiceRouter(), authtypes.NewModuleAddress(govtypes.ModuleName).String(), ) @@ -343,7 +342,6 @@ func NewSimApp( app.ICAHostKeeper = icahostkeeper.NewKeeper( appCodec, runtime.NewKVStoreService(keys[icahosttypes.StoreKey]), app.IBCKeeper.ChannelKeeper, - app.IBCKeeper.ChannelKeeper, app.AccountKeeper, app.MsgServiceRouter(), app.GRPCQueryRouter(), authtypes.NewModuleAddress(govtypes.ModuleName).String(), ) @@ -360,7 +358,6 @@ func NewSimApp( app.TransferKeeper = ibctransferkeeper.NewKeeper( appCodec, runtime.NewKVStoreService(keys[ibctransfertypes.StoreKey]), app.IBCKeeper.ChannelKeeper, - app.IBCKeeper.ChannelKeeper, app.MsgServiceRouter(), app.AccountKeeper, app.BankKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String(), @@ -389,41 +386,27 @@ func NewSimApp( // - Transfer // create IBC module from bottom to top of stack - var transferStack porttypes.IBCModule - transferStack = transfer.NewIBCModule(app.TransferKeeper) - transferStack = ibccallbacks.NewIBCMiddleware(transferStack, app.IBCKeeper.ChannelKeeper, app.MockContractKeeper, maxCallbackGas) - var transferICS4Wrapper porttypes.ICS4Wrapper - transferICS4Wrapper, ok := transferStack.(porttypes.ICS4Wrapper) - if !ok { - panic(fmt.Errorf("cannot convert %T to %T", transferStack, transferICS4Wrapper)) - } - - // Since the callbacks middleware itself is an ics4wrapper, it needs to be passed to the transfer keeper - app.TransferKeeper.WithICS4Wrapper(transferICS4Wrapper) + transferStack := porttypes.NewIBCStackBuilder(app.IBCKeeper.ChannelKeeper) + transferStack.Base(transfer.NewIBCModule(app.TransferKeeper)).Next( + ibccallbacks.NewIBCMiddleware(app.MockContractKeeper, maxCallbackGas), + ) // Add transfer stack to IBC Router - ibcRouter.AddRoute(ibctransfertypes.ModuleName, transferStack) + ibcRouter.AddRoute(ibctransfertypes.ModuleName, transferStack.Build()) // Create Interchain Accounts Stack // SendPacket, since it is originating from the application to core IBC: // icaControllerKeeper.SendTx -> callbacks.SendPacket -> channel.SendPacket // initialize ICA module with mock module as the authentication module on the controller side - var icaControllerStack porttypes.IBCModule - icaControllerStack = ibcmock.NewIBCModule(&mockModule, ibcmock.NewIBCApp("")) - app.ICAAuthModule, ok = icaControllerStack.(ibcmock.IBCModule) - if !ok { - panic(fmt.Errorf("cannot convert %T to %T", icaControllerStack, app.ICAAuthModule)) - } - icaControllerStack = icacontroller.NewIBCMiddlewareWithAuth(icaControllerStack, app.ICAControllerKeeper) - icaControllerStack = ibccallbacks.NewIBCMiddleware(icaControllerStack, app.IBCKeeper.ChannelKeeper, app.MockContractKeeper, maxCallbackGas) - var icaICS4Wrapper porttypes.ICS4Wrapper - icaICS4Wrapper, ok = icaControllerStack.(porttypes.ICS4Wrapper) - if !ok { - panic(fmt.Errorf("cannot convert %T to %T", icaControllerStack, icaICS4Wrapper)) - } - // Since the callbacks middleware itself is an ics4wrapper, it needs to be passed to the ica controller keeper - app.ICAControllerKeeper.WithICS4Wrapper(icaICS4Wrapper) + icaControllerStack := porttypes.NewIBCStackBuilder(app.IBCKeeper.ChannelKeeper) + + icaControllerStack.Base(ibcmock.NewIBCModule(&mockModule, ibcmock.NewIBCApp(""))).Next( + icacontroller.NewIBCMiddleware(app.ICAControllerKeeper), + ).Next( + ibccallbacks.NewIBCMiddleware(app.MockContractKeeper, maxCallbackGas), + ) + icaControllerApp := icaControllerStack.Build() // RecvPacket, message that originates from core IBC and goes down to app, the flow is: // channel.RecvPacket -> icaHost.OnRecvPacket @@ -432,9 +415,9 @@ func NewSimApp( // Add host, controller & ica auth modules to IBC router ibcRouter. - AddRoute(icacontrollertypes.SubModuleName, icaControllerStack). + AddRoute(icacontrollertypes.SubModuleName, icaControllerApp). AddRoute(icahosttypes.SubModuleName, icaHostStack). - AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack) // ica with mock auth module stack route to ica (top level of middleware stack) + AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerApp) // ica with mock auth module stack route to ica (top level of middleware stack) // OnRecvPacket, message that originates from core IBC and goes down to app, the flow is the otherway // channel.RecvPacket -> callbacks.OnRecvPacket -> mockModule.OnRecvPacket @@ -482,7 +465,7 @@ func NewSimApp( // IBC modules ibc.NewAppModule(app.IBCKeeper), transfer.NewAppModule(app.TransferKeeper), - ica.NewAppModule(&app.ICAControllerKeeper, &app.ICAHostKeeper), + ica.NewAppModule(app.ICAControllerKeeper, app.ICAHostKeeper), mockModule, // IBC light clients diff --git a/modules/apps/callbacks/types/callbacks_test.go b/modules/apps/callbacks/types/callbacks_test.go index 94a4b8249e5..470351f1f26 100644 --- a/modules/apps/callbacks/types/callbacks_test.go +++ b/modules/apps/callbacks/types/callbacks_test.go @@ -594,7 +594,7 @@ func (s *CallbacksTypesTestSuite) TestGetDestSourceCallbackDataTransfer() { transferStack, ok := s.chainA.App.GetIBCKeeper().PortKeeper.Route(transfertypes.ModuleName) s.Require().True(ok) - packetUnmarshaler, ok := transferStack.(porttypes.PacketUnmarshalarModule) + packetUnmarshaler, ok := transferStack.(porttypes.PacketUnmarshalerModule) s.Require().True(ok) s.path.Setup() diff --git a/modules/apps/callbacks/v2/ibc_middleware.go b/modules/apps/callbacks/v2/ibc_middleware.go index a49fd45ef25..d31f5d628f9 100644 --- a/modules/apps/callbacks/v2/ibc_middleware.go +++ b/modules/apps/callbacks/v2/ibc_middleware.go @@ -42,7 +42,7 @@ func (rack RecvAcknowledgement) Acknowledgement() []byte { // IBCMiddleware implements the IBC v2 middleware interface // with the underlying application. type IBCMiddleware struct { - app api.PacketUnmarshalarModuleV2 + app api.PacketUnmarshalerModuleV2 writeAckWrapper api.WriteAcknowledgementWrapper contractKeeper types.ContractKeeper @@ -61,9 +61,9 @@ func NewIBCMiddleware( app api.IBCModule, writeAckWrapper api.WriteAcknowledgementWrapper, contractKeeper types.ContractKeeper, chanKeeperV2 types.ChannelKeeperV2, maxCallbackGas uint64, ) *IBCMiddleware { - packetDataUnmarshalerApp, ok := app.(api.PacketUnmarshalarModuleV2) + packetDataUnmarshalerApp, ok := app.(api.PacketUnmarshalerModuleV2) if !ok { - panic(fmt.Errorf("underlying application does not implement %T", (*api.PacketUnmarshalarModuleV2)(nil))) + panic(fmt.Errorf("underlying application does not implement %T", (*api.PacketUnmarshalerModuleV2)(nil))) } if contractKeeper == nil { diff --git a/modules/apps/callbacks/v2/ibc_middleware_test.go b/modules/apps/callbacks/v2/ibc_middleware_test.go index 704819c71a3..a93625276f2 100644 --- a/modules/apps/callbacks/v2/ibc_middleware_test.go +++ b/modules/apps/callbacks/v2/ibc_middleware_test.go @@ -49,7 +49,7 @@ func (s *CallbacksTestSuite) TestNewIBCMiddleware() { func() { _ = v2.NewIBCMiddleware(nil, &channelkeeperv2.Keeper{}, simapp.ContractKeeper{}, &channelkeeperv2.Keeper{}, maxCallbackGas) }, - fmt.Errorf("underlying application does not implement %T", (*api.PacketUnmarshalarModuleV2)(nil)), + fmt.Errorf("underlying application does not implement %T", (*api.PacketUnmarshalerModuleV2)(nil)), }, { "panics with nil contract keeper", diff --git a/modules/apps/packet-forward-middleware/ibc_middleware.go b/modules/apps/packet-forward-middleware/ibc_middleware.go index af0b410b482..2d12eb41900 100644 --- a/modules/apps/packet-forward-middleware/ibc_middleware.go +++ b/modules/apps/packet-forward-middleware/ibc_middleware.go @@ -25,13 +25,13 @@ import ( var ( _ porttypes.Middleware = &IBCMiddleware{} - _ porttypes.PacketUnmarshalarModule = &IBCMiddleware{} + _ porttypes.PacketUnmarshalerModule = &IBCMiddleware{} ) // IBCMiddleware implements the ICS26 callbacks for the forward middleware given the // forward keeper and the underlying application. type IBCMiddleware struct { - app porttypes.PacketUnmarshalarModule + app porttypes.PacketUnmarshalerModule keeper *keeper.Keeper retriesOnTimeout uint8 @@ -39,9 +39,8 @@ type IBCMiddleware struct { } // NewIBCMiddleware creates a new IBCMiddleware given the keeper and underlying application. -func NewIBCMiddleware(app porttypes.PacketUnmarshalarModule, k *keeper.Keeper, retriesOnTimeout uint8, forwardTimeout time.Duration) IBCMiddleware { - return IBCMiddleware{ - app: app, +func NewIBCMiddleware(k *keeper.Keeper, retriesOnTimeout uint8, forwardTimeout time.Duration) *IBCMiddleware { + return &IBCMiddleware{ keeper: k, retriesOnTimeout: retriesOnTimeout, forwardTimeout: forwardTimeout, @@ -49,37 +48,37 @@ func NewIBCMiddleware(app porttypes.PacketUnmarshalarModule, k *keeper.Keeper, r } // OnChanOpenInit implements the IBCModule interface. -func (im IBCMiddleware) OnChanOpenInit(ctx sdk.Context, order channeltypes.Order, connectionHops []string, portID string, channelID string, counterparty channeltypes.Counterparty, version string) (string, error) { +func (im *IBCMiddleware) OnChanOpenInit(ctx sdk.Context, order channeltypes.Order, connectionHops []string, portID string, channelID string, counterparty channeltypes.Counterparty, version string) (string, error) { return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, counterparty, version) } // OnChanOpenTry implements the IBCModule interface. -func (im IBCMiddleware) OnChanOpenTry(ctx sdk.Context, order channeltypes.Order, connectionHops []string, portID, channelID string, counterparty channeltypes.Counterparty, counterpartyVersion string) (string, error) { +func (im *IBCMiddleware) OnChanOpenTry(ctx sdk.Context, order channeltypes.Order, connectionHops []string, portID, channelID string, counterparty channeltypes.Counterparty, counterpartyVersion string) (string, error) { return im.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, counterparty, counterpartyVersion) } // OnChanOpenAck implements the IBCModule interface. -func (im IBCMiddleware) OnChanOpenAck(ctx sdk.Context, portID, channelID string, counterpartyChannelID string, counterpartyVersion string) error { +func (im *IBCMiddleware) OnChanOpenAck(ctx sdk.Context, portID, channelID string, counterpartyChannelID string, counterpartyVersion string) error { return im.app.OnChanOpenAck(ctx, portID, channelID, counterpartyChannelID, counterpartyVersion) } // OnChanOpenConfirm implements the IBCModule interface. -func (im IBCMiddleware) OnChanOpenConfirm(ctx sdk.Context, portID, channelID string) error { +func (im *IBCMiddleware) OnChanOpenConfirm(ctx sdk.Context, portID, channelID string) error { return im.app.OnChanOpenConfirm(ctx, portID, channelID) } // OnChanCloseInit implements the IBCModule interface. -func (im IBCMiddleware) OnChanCloseInit(ctx sdk.Context, portID, channelID string) error { +func (im *IBCMiddleware) OnChanCloseInit(ctx sdk.Context, portID, channelID string) error { return im.app.OnChanCloseInit(ctx, portID, channelID) } // OnChanCloseConfirm implements the IBCModule interface. -func (im IBCMiddleware) OnChanCloseConfirm(ctx sdk.Context, portID, channelID string) error { +func (im *IBCMiddleware) OnChanCloseConfirm(ctx sdk.Context, portID, channelID string) error { return im.app.OnChanCloseConfirm(ctx, portID, channelID) } // UnmarshalPacketData implements PacketDataUnmarshaler. -func (im IBCMiddleware) UnmarshalPacketData(ctx sdk.Context, portID string, channelID string, bz []byte) (any, string, error) { +func (im *IBCMiddleware) UnmarshalPacketData(ctx sdk.Context, portID string, channelID string, bz []byte) (any, string, error) { return im.app.UnmarshalPacketData(ctx, portID, channelID, bz) } @@ -138,7 +137,7 @@ func newErrorAcknowledgement(err error) channeltypes.Acknowledgement { // OnRecvPacket checks the memo field on this packet and if the metadata inside's root key indicates this packet // should be handled by the swap middleware it attempts to perform a swap. If the swap is successful // the underlying application's OnRecvPacket callback is invoked, an ack error is returned otherwise. -func (im IBCMiddleware) OnRecvPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress) ibcexported.Acknowledgement { +func (im *IBCMiddleware) OnRecvPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress) ibcexported.Acknowledgement { logger := im.keeper.Logger(ctx) var data transfertypes.FungibleTokenPacketData @@ -235,7 +234,7 @@ func (im IBCMiddleware) OnRecvPacket(ctx sdk.Context, channelVersion string, pac // receiveFunds receives funds from the packet into the override receiver // address and returns an error if the funds cannot be received. -func (im IBCMiddleware) receiveFunds(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, data transfertypes.FungibleTokenPacketData, overrideReceiver string, relayer sdk.AccAddress) error { +func (im *IBCMiddleware) receiveFunds(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, data transfertypes.FungibleTokenPacketData, overrideReceiver string, relayer sdk.AccAddress) error { overrideData := data overrideData.Receiver = overrideReceiver overrideData.Memo = "" // Memo explicitly emptied. @@ -257,7 +256,7 @@ func (im IBCMiddleware) receiveFunds(ctx sdk.Context, channelVersion string, pac } // OnAcknowledgementPacket implements the IBCModule interface. -func (im IBCMiddleware) OnAcknowledgementPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress) error { +func (im *IBCMiddleware) OnAcknowledgementPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress) error { var data transfertypes.FungibleTokenPacketData if err := transfertypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil { im.keeper.Logger(ctx).Error("packetForwardMiddleware error parsing packet data from ack packet", @@ -306,7 +305,7 @@ func (im IBCMiddleware) OnAcknowledgementPacket(ctx sdk.Context, channelVersion } // OnTimeoutPacket implements the IBCModule interface. -func (im IBCMiddleware) OnTimeoutPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress) error { +func (im *IBCMiddleware) OnTimeoutPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress) error { var data transfertypes.FungibleTokenPacketData if err := transfertypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil { im.keeper.Logger(ctx).Error("packetForwardMiddleware error parsing packet data from timeout packet", @@ -355,15 +354,34 @@ func (im IBCMiddleware) OnTimeoutPacket(ctx sdk.Context, channelVersion string, } // SendPacket implements the ICS4 Wrapper interface. -func (im IBCMiddleware) SendPacket(ctx sdk.Context, sourcePort, sourceChannel string, timeoutHeight clienttypes.Height, timeoutTimestamp uint64, data []byte) (uint64, error) { +func (im *IBCMiddleware) SendPacket(ctx sdk.Context, sourcePort, sourceChannel string, timeoutHeight clienttypes.Height, timeoutTimestamp uint64, data []byte) (uint64, error) { return im.keeper.SendPacket(ctx, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, data) } // WriteAcknowledgement implements the ICS4 Wrapper interface. -func (im IBCMiddleware) WriteAcknowledgement(ctx sdk.Context, packet ibcexported.PacketI, ack ibcexported.Acknowledgement) error { +func (im *IBCMiddleware) WriteAcknowledgement(ctx sdk.Context, packet ibcexported.PacketI, ack ibcexported.Acknowledgement) error { return im.keeper.WriteAcknowledgement(ctx, packet, ack) } -func (im IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) { +func (im *IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) { return im.keeper.GetAppVersion(ctx, portID, channelID) } + +func (im *IBCMiddleware) SetICS4Wrapper(wrapper porttypes.ICS4Wrapper) { + if wrapper == nil { + panic("ICS4Wrapper cannot be nil") + } + im.keeper.WithICS4Wrapper(wrapper) +} + +func (im *IBCMiddleware) SetUnderlyingApplication(app porttypes.IBCModule) { + if im.app != nil { + panic("underlying application already set") + } + // the underlying application must implement the PacketUnmarshalerModule interface + pdApp, ok := app.(porttypes.PacketUnmarshalerModule) + if !ok { + panic(fmt.Errorf("underlying application must implement PacketUnmarshalerModule, got %T", app)) + } + im.app = pdApp +} diff --git a/modules/apps/packet-forward-middleware/ibc_middleware_test.go b/modules/apps/packet-forward-middleware/ibc_middleware_test.go index 5e9a73265ce..a917767693c 100644 --- a/modules/apps/packet-forward-middleware/ibc_middleware_test.go +++ b/modules/apps/packet-forward-middleware/ibc_middleware_test.go @@ -12,6 +12,7 @@ import ( channeltypes "github.com/cosmos/ibc-go/v10/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v10/modules/core/05-port/types" ibctesting "github.com/cosmos/ibc-go/v10/testing" + ibcmock "github.com/cosmos/ibc-go/v10/testing/mock" ) type PFMTestSuite struct { @@ -45,6 +46,40 @@ func (s *PFMTestSuite) setupChains() { s.pathBC.Setup() } +func (s *PFMTestSuite) TestSetICS4Wrapper() { + s.setupChains() + + pfm := s.pktForwardMiddleware(s.chainA) + + s.Require().Panics(func() { + pfm.SetICS4Wrapper(nil) + }, "ICS4Wrapper cannot be nil") + + s.Require().NotPanics(func() { + pfm.SetICS4Wrapper(s.chainA.App.GetIBCKeeper().ChannelKeeper) + }, "ICS4Wrapper should be set without panic") +} + +func (s *PFMTestSuite) TestSetUnderlyingApplication() { + s.setupChains() + + pfmKeeper := s.chainA.GetSimApp().PFMKeeper + + pfm := packetforward.NewIBCMiddleware(pfmKeeper, 0, packetforwardkeeper.DefaultForwardTransferPacketTimeoutTimestamp) + + s.Require().Panics(func() { + pfm.SetUnderlyingApplication(nil) + }, "underlying application cannot be nil") + + s.Require().NotPanics(func() { + pfm.SetUnderlyingApplication(&ibcmock.IBCModule{}) + }, "underlying application should be set without panic") + + s.Require().Panics(func() { + pfm.SetUnderlyingApplication(&ibcmock.IBCModule{}) + }, "underlying application should not be set again") +} + func (s *PFMTestSuite) TestOnRecvPacket_NonfungibleToken() { s.setupChains() @@ -206,16 +241,17 @@ func (s *PFMTestSuite) TestOnRecvPacket_ForwardNoFee() { s.Require().NoError(err) } -func (s *PFMTestSuite) pktForwardMiddleware(chain *ibctesting.TestChain) packetforward.IBCMiddleware { +func (s *PFMTestSuite) pktForwardMiddleware(chain *ibctesting.TestChain) *packetforward.IBCMiddleware { pfmKeeper := chain.GetSimApp().PFMKeeper ibcModule, ok := chain.App.GetIBCKeeper().PortKeeper.Route(transfertypes.ModuleName) s.Require().True(ok) - transferStack, ok := ibcModule.(porttypes.PacketUnmarshalarModule) + transferStack, ok := ibcModule.(porttypes.PacketUnmarshalerModule) s.Require().True(ok) - ibcMiddleware := packetforward.NewIBCMiddleware(transferStack, pfmKeeper, 0, packetforwardkeeper.DefaultForwardTransferPacketTimeoutTimestamp) + ibcMiddleware := packetforward.NewIBCMiddleware(pfmKeeper, 0, packetforwardkeeper.DefaultForwardTransferPacketTimeoutTimestamp) + ibcMiddleware.SetUnderlyingApplication(transferStack) return ibcMiddleware } diff --git a/modules/apps/packet-forward-middleware/keeper/keeper.go b/modules/apps/packet-forward-middleware/keeper/keeper.go index c252eb129cc..bf6642ed37f 100644 --- a/modules/apps/packet-forward-middleware/keeper/keeper.go +++ b/modules/apps/packet-forward-middleware/keeper/keeper.go @@ -51,28 +51,30 @@ type Keeper struct { } // NewKeeper creates a new forward Keeper instance -func NewKeeper(cdc codec.BinaryCodec, storeService corestore.KVStoreService, transferKeeper types.TransferKeeper, channelKeeper types.ChannelKeeper, bankKeeper types.BankKeeper, ics4Wrapper porttypes.ICS4Wrapper, authority string) *Keeper { +func NewKeeper(cdc codec.BinaryCodec, storeService corestore.KVStoreService, transferKeeper types.TransferKeeper, channelKeeper types.ChannelKeeper, bankKeeper types.BankKeeper, authority string) *Keeper { return &Keeper{ cdc: cdc, storeService: storeService, transferKeeper: transferKeeper, - channelKeeper: channelKeeper, - bankKeeper: bankKeeper, - ics4Wrapper: ics4Wrapper, - authority: authority, + // Defaults to using the channel keeper as the ICS4Wrapper + // This can be overridden later with WithICS4Wrapper (e.g. by the middleware stack wiring) + ics4Wrapper: channelKeeper, + channelKeeper: channelKeeper, + bankKeeper: bankKeeper, + authority: authority, } } +// WithICS4Wrapper sets the ICS4Wrapper for the keeper. +func (k *Keeper) WithICS4Wrapper(ics4Wrapper porttypes.ICS4Wrapper) { + k.ics4Wrapper = ics4Wrapper +} + // GetAuthority returns the module's authority. func (k *Keeper) GetAuthority() string { return k.authority } -// SetTransferKeeper sets the transferKeeper -func (k *Keeper) SetTransferKeeper(transferKeeper types.TransferKeeper) { - k.transferKeeper = transferKeeper -} - // SetICS4Wrapper sets the ICS4 wrapper. func (k *Keeper) SetICS4Wrapper(ics4Wrapper porttypes.ICS4Wrapper) { k.ics4Wrapper = ics4Wrapper @@ -281,6 +283,7 @@ func (k *Keeper) ForwardTransferPacket(ctx sdk.Context, inFlightPacket *types.In "denom", token.Denom, "error", err, ) + // TODO: Should probably have custom errors! return errorsmod.Wrap(sdkerrors.ErrInsufficientFunds, err.Error()) } diff --git a/modules/apps/packet-forward-middleware/keeper/keeper_test.go b/modules/apps/packet-forward-middleware/keeper/keeper_test.go index 37e5853a4e0..0b1e941dcdc 100644 --- a/modules/apps/packet-forward-middleware/keeper/keeper_test.go +++ b/modules/apps/packet-forward-middleware/keeper/keeper_test.go @@ -9,11 +9,13 @@ import ( testifysuite "github.com/stretchr/testify/suite" + "github.com/cosmos/cosmos-sdk/runtime" sdk "github.com/cosmos/cosmos-sdk/types" bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper" cmtbytes "github.com/cometbft/cometbft/libs/bytes" + "github.com/cosmos/ibc-go/v10/modules/apps/packet-forward-middleware/keeper" pfmtypes "github.com/cosmos/ibc-go/v10/modules/apps/packet-forward-middleware/types" transfertypes "github.com/cosmos/ibc-go/v10/modules/apps/transfer/types" clienttypes "github.com/cosmos/ibc-go/v10/modules/core/02-client/types" @@ -163,7 +165,8 @@ func (s *KeeperTestSuite) TestForwardTransferPacket() { path := ibctesting.NewTransferPath(s.chainA, s.chainB) path.Setup() - s.chainA.GetSimApp().PFMKeeper.SetTransferKeeper(&transferMock{}) + pfmKeeper := keeper.NewKeeper(s.chainA.GetSimApp().AppCodec(), runtime.NewKVStoreService(s.chainA.GetSimApp().GetKey(pfmtypes.StoreKey)), &transferMock{}, s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, s.chainA.GetSimApp().BankKeeper, "authority") + ctx := s.chainA.GetContext() srcPacket := channeltypes.Packet{ Data: []byte{1}, @@ -195,21 +198,21 @@ func (s *KeeperTestSuite) TestForwardTransferPacket() { initialSender := s.chainA.SenderAccount.GetAddress() finalReceiver := s.chainB.SenderAccount.GetAddress() - err := s.chainA.GetSimApp().PFMKeeper.ForwardTransferPacket(ctx, nil, srcPacket, initialSender.String(), finalReceiver.String(), metadata, sdk.NewInt64Coin("denom", 1000), 2, timeout, nil, nonRefundable) + err := pfmKeeper.ForwardTransferPacket(ctx, nil, srcPacket, initialSender.String(), finalReceiver.String(), metadata, sdk.NewInt64Coin("denom", 1000), 2, timeout, nil, nonRefundable) s.Require().NoError(err) // Get the inflight packer - inflightPacket, err := s.chainA.GetSimApp().PFMKeeper.GetInflightPacket(ctx, srcPacket) + inflightPacket, err := pfmKeeper.GetInflightPacket(ctx, srcPacket) s.Require().NoError(err) s.Require().Equal(inflightPacket.RetriesRemaining, int32(retries)) // Call the same function again with inflight packet. Num retries should decrease. - err = s.chainA.GetSimApp().PFMKeeper.ForwardTransferPacket(ctx, inflightPacket, srcPacket, initialSender.String(), finalReceiver.String(), metadata, sdk.NewInt64Coin("denom", 1000), 2, timeout, nil, nonRefundable) + err = pfmKeeper.ForwardTransferPacket(ctx, inflightPacket, srcPacket, initialSender.String(), finalReceiver.String(), metadata, sdk.NewInt64Coin("denom", 1000), 2, timeout, nil, nonRefundable) s.Require().NoError(err) // Get the inflight packer - inflightPacket2, err := s.chainA.GetSimApp().PFMKeeper.GetInflightPacket(ctx, srcPacket) + inflightPacket2, err := pfmKeeper.GetInflightPacket(ctx, srcPacket) s.Require().NoError(err) s.Require().Equal(inflightPacket.RetriesRemaining, inflightPacket2.RetriesRemaining) @@ -221,7 +224,7 @@ func (s *KeeperTestSuite) TestForwardTransferPacketWithNext() { path := ibctesting.NewTransferPath(s.chainA, s.chainB) path.Setup() - s.chainA.GetSimApp().PFMKeeper.SetTransferKeeper(&transferMock{}) + pfmKeeper := keeper.NewKeeper(s.chainA.GetSimApp().AppCodec(), runtime.NewKVStoreService(s.chainA.GetSimApp().GetKey(pfmtypes.StoreKey)), &transferMock{}, s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, s.chainA.GetSimApp().BankKeeper, "authority") ctx := s.chainA.GetContext() srcPacket := channeltypes.Packet{ Data: []byte{1}, @@ -265,11 +268,11 @@ func (s *KeeperTestSuite) TestForwardTransferPacketWithNext() { initialSender := s.chainA.SenderAccount.GetAddress() finalReceiver := s.chainB.SenderAccount.GetAddress() - err := s.chainA.GetSimApp().PFMKeeper.ForwardTransferPacket(ctx, nil, srcPacket, initialSender.String(), finalReceiver.String(), metadata, sdk.NewInt64Coin("denom", 1000), 2, timeout, nil, nonRefundable) + err := pfmKeeper.ForwardTransferPacket(ctx, nil, srcPacket, initialSender.String(), finalReceiver.String(), metadata, sdk.NewInt64Coin("denom", 1000), 2, timeout, nil, nonRefundable) s.Require().NoError(err) // Verify the inflight packet was created - inflightPacket, err := s.chainA.GetSimApp().PFMKeeper.GetInflightPacket(ctx, srcPacket) + inflightPacket, err := pfmKeeper.GetInflightPacket(ctx, srcPacket) s.Require().NoError(err) s.Require().NotNil(inflightPacket) s.Require().Equal(inflightPacket.RetriesRemaining, int32(retries)) @@ -280,7 +283,7 @@ func (s *KeeperTestSuite) TestRetryTimeoutErrorGettingNext() { path := ibctesting.NewTransferPath(s.chainA, s.chainB) path.Setup() - s.chainA.GetSimApp().PFMKeeper.SetTransferKeeper(&transferMock{}) + pfmKeeper := keeper.NewKeeper(s.chainA.GetSimApp().AppCodec(), runtime.NewKVStoreService(s.chainA.GetSimApp().GetKey(pfmtypes.StoreKey)), &transferMock{}, s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, s.chainA.GetSimApp().BankKeeper, "authority") ctx := s.chainA.GetContext() // Create a transfer detail with invalid memo that will cause GetPacketMetadataFromPacketdata to fail @@ -300,7 +303,7 @@ func (s *KeeperTestSuite) TestRetryTimeoutErrorGettingNext() { Nonrefundable: false, } - err := s.chainA.GetSimApp().PFMKeeper.RetryTimeout(ctx, path.EndpointA.ChannelID, path.EndpointA.ChannelConfig.PortID, transferDetail, inFlightPacket) + err := pfmKeeper.RetryTimeout(ctx, path.EndpointA.ChannelID, path.EndpointA.ChannelConfig.PortID, transferDetail, inFlightPacket) // The function should still succeed since it only logs the error and continues s.Require().NoError(err) } diff --git a/modules/apps/packet-forward-middleware/types/expected_keepers.go b/modules/apps/packet-forward-middleware/types/expected_keepers.go index 0b48938127d..72d29fbeb72 100644 --- a/modules/apps/packet-forward-middleware/types/expected_keepers.go +++ b/modules/apps/packet-forward-middleware/types/expected_keepers.go @@ -9,6 +9,7 @@ import ( transfertypes "github.com/cosmos/ibc-go/v10/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v10/modules/core/04-channel/types" + porttypes "github.com/cosmos/ibc-go/v10/modules/core/05-port/types" ) // TransferKeeper defines the expected transfer keeper @@ -25,6 +26,7 @@ type TransferKeeper interface { // ChannelKeeper defines the expected IBC channel keeper type ChannelKeeper interface { + porttypes.ICS4Wrapper GetChannel(ctx sdk.Context, srcPort, srcChan string) (channel channeltypes.Channel, found bool) // Only used for v3 migration diff --git a/modules/apps/rate-limiting/ibc_middleware.go b/modules/apps/rate-limiting/ibc_middleware.go index be3d35679d9..b94cc89101d 100644 --- a/modules/apps/rate-limiting/ibc_middleware.go +++ b/modules/apps/rate-limiting/ibc_middleware.go @@ -1,11 +1,12 @@ package ratelimiting import ( + "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/ibc-go/v10/modules/apps/rate-limiting/keeper" clienttypes "github.com/cosmos/ibc-go/v10/modules/core/02-client/types" - channelkeeper "github.com/cosmos/ibc-go/v10/modules/core/04-channel/keeper" channeltypes "github.com/cosmos/ibc-go/v10/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v10/modules/core/05-port/types" ibcexported "github.com/cosmos/ibc-go/v10/modules/core/exported" @@ -13,58 +14,55 @@ import ( var ( _ porttypes.Middleware = (*IBCMiddleware)(nil) - _ porttypes.PacketUnmarshalarModule = (*IBCMiddleware)(nil) + _ porttypes.PacketUnmarshalerModule = (*IBCMiddleware)(nil) ) // IBCMiddleware implements the ICS26 callbacks for the rate-limiting middleware. type IBCMiddleware struct { - app porttypes.PacketUnmarshalarModule - keeper keeper.Keeper - ics4Wrapper porttypes.ICS4Wrapper + app porttypes.PacketUnmarshalerModule + keeper *keeper.Keeper } // NewIBCMiddleware creates a new IBCMiddleware given the keeper, underlying application, and channel keeper. -func NewIBCMiddleware(app porttypes.PacketUnmarshalarModule, k keeper.Keeper, ck *channelkeeper.Keeper) IBCMiddleware { - return IBCMiddleware{ - app: app, - keeper: k, - ics4Wrapper: ck, +func NewIBCMiddleware(k *keeper.Keeper) *IBCMiddleware { + return &IBCMiddleware{ + keeper: k, } } // OnChanOpenInit implements the IBCMiddleware interface. Call underlying app's OnChanOpenInit. -func (im IBCMiddleware) OnChanOpenInit(ctx sdk.Context, order channeltypes.Order, connectionHops []string, portID string, channelID string, counterparty channeltypes.Counterparty, version string) (string, error) { +func (im *IBCMiddleware) OnChanOpenInit(ctx sdk.Context, order channeltypes.Order, connectionHops []string, portID string, channelID string, counterparty channeltypes.Counterparty, version string) (string, error) { return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, counterparty, version) } // OnChanOpenTry implements the IBCMiddleware interface. Call underlying app's OnChanOpenTry. -func (im IBCMiddleware) OnChanOpenTry(ctx sdk.Context, order channeltypes.Order, connectionHops []string, portID, channelID string, counterparty channeltypes.Counterparty, counterpartyVersion string) (string, error) { +func (im *IBCMiddleware) OnChanOpenTry(ctx sdk.Context, order channeltypes.Order, connectionHops []string, portID, channelID string, counterparty channeltypes.Counterparty, counterpartyVersion string) (string, error) { return im.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, counterparty, counterpartyVersion) } // OnChanOpenAck implements the IBCMiddleware interface. Call underlying app's OnChanOpenAck. -func (im IBCMiddleware) OnChanOpenAck(ctx sdk.Context, portID, channelID string, counterpartyChannelID string, counterpartyVersion string) error { +func (im *IBCMiddleware) OnChanOpenAck(ctx sdk.Context, portID, channelID string, counterpartyChannelID string, counterpartyVersion string) error { return im.app.OnChanOpenAck(ctx, portID, channelID, counterpartyChannelID, counterpartyVersion) } // OnChanOpenConfirm implements the IBCMiddleware interface. Call underlying app's OnChanOpenConfirm. -func (im IBCMiddleware) OnChanOpenConfirm(ctx sdk.Context, portID, channelID string) error { +func (im *IBCMiddleware) OnChanOpenConfirm(ctx sdk.Context, portID, channelID string) error { return im.app.OnChanOpenConfirm(ctx, portID, channelID) } // OnChanCloseInit implements the IBCMiddleware interface. Call underlying app's OnChanCloseInit. -func (im IBCMiddleware) OnChanCloseInit(ctx sdk.Context, portID, channelID string) error { +func (im *IBCMiddleware) OnChanCloseInit(ctx sdk.Context, portID, channelID string) error { return im.app.OnChanCloseInit(ctx, portID, channelID) } // OnChanCloseConfirm implements the IBCMiddleware interface. Call underlying app's OnChanCloseConfirm. -func (im IBCMiddleware) OnChanCloseConfirm(ctx sdk.Context, portID, channelID string) error { +func (im *IBCMiddleware) OnChanCloseConfirm(ctx sdk.Context, portID, channelID string) error { return im.app.OnChanCloseConfirm(ctx, portID, channelID) } // OnRecvPacket implements the IBCMiddleware interface. // Rate limits the incoming packet. If the packet is allowed, call underlying app's OnRecvPacket. -func (im IBCMiddleware) OnRecvPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress) ibcexported.Acknowledgement { +func (im *IBCMiddleware) OnRecvPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress) ibcexported.Acknowledgement { if err := im.keeper.ReceiveRateLimitedPacket(ctx, packet); err != nil { im.keeper.Logger(ctx).Error("Receive packet rate limited", "error", err) return channeltypes.NewErrorAcknowledgement(err) @@ -77,7 +75,7 @@ func (im IBCMiddleware) OnRecvPacket(ctx sdk.Context, channelVersion string, pac // OnAcknowledgementPacket implements the IBCMiddleware interface. // If the acknowledgement was an error, revert the outflow amount. // Then, call underlying app's OnAcknowledgementPacket. -func (im IBCMiddleware) OnAcknowledgementPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress) error { +func (im *IBCMiddleware) OnAcknowledgementPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress) error { if err := im.keeper.AcknowledgeRateLimitedPacket(ctx, packet, acknowledgement); err != nil { im.keeper.Logger(ctx).Error("Rate limit OnAcknowledgementPacket failed", "error", err) } @@ -87,7 +85,7 @@ func (im IBCMiddleware) OnAcknowledgementPacket(ctx sdk.Context, channelVersion // OnTimeoutPacket implements the IBCMiddleware interface. // Revert the outflow amount. Then, call underlying app's OnTimeoutPacket. -func (im IBCMiddleware) OnTimeoutPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress) error { +func (im *IBCMiddleware) OnTimeoutPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress) error { if err := im.keeper.TimeoutRateLimitedPacket(ctx, packet); err != nil { im.keeper.Logger(ctx).Error("Rate limit OnTimeoutPacket failed", "error", err) } @@ -98,35 +96,49 @@ func (im IBCMiddleware) OnTimeoutPacket(ctx sdk.Context, channelVersion string, // SendPacket implements the ICS4 Wrapper interface. // It calls the keeper's SendRateLimitedPacket function first to check the rate limit. // If the packet is allowed, it then calls the underlying ICS4Wrapper SendPacket. -func (im IBCMiddleware) SendPacket(ctx sdk.Context, sourcePort string, sourceChannel string, timeoutHeight clienttypes.Height, timeoutTimestamp uint64, data []byte) (uint64, error) { +func (im *IBCMiddleware) SendPacket(ctx sdk.Context, sourcePort string, sourceChannel string, timeoutHeight clienttypes.Height, timeoutTimestamp uint64, data []byte) (uint64, error) { err := im.keeper.SendRateLimitedPacket(ctx, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, data) if err != nil { im.keeper.Logger(ctx).Error("ICS20 packet send was denied by rate limiter", "error", err) return 0, err } - seq, err := im.ics4Wrapper.SendPacket(ctx, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, data) - if err != nil { - return 0, err - } - - return seq, nil + return im.keeper.SendPacket(ctx, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, data) } // WriteAcknowledgement implements the ICS4 Wrapper interface. // It calls the underlying ICS4Wrapper. -func (im IBCMiddleware) WriteAcknowledgement(ctx sdk.Context, packet ibcexported.PacketI, ack ibcexported.Acknowledgement) error { - return im.ics4Wrapper.WriteAcknowledgement(ctx, packet, ack) +func (im *IBCMiddleware) WriteAcknowledgement(ctx sdk.Context, packet ibcexported.PacketI, ack ibcexported.Acknowledgement) error { + return im.keeper.WriteAcknowledgement(ctx, packet, ack) } // GetAppVersion implements the ICS4 Wrapper interface. // It calls the underlying ICS4Wrapper. -func (im IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) { - return im.ics4Wrapper.GetAppVersion(ctx, portID, channelID) +func (im *IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) { + return im.keeper.GetAppVersion(ctx, portID, channelID) } // UnmarshalPacketData implements the PacketDataUnmarshaler interface. // It defers to the underlying app to unmarshal the packet data. -func (im IBCMiddleware) UnmarshalPacketData(ctx sdk.Context, portID string, channelID string, bz []byte) (any, string, error) { +func (im *IBCMiddleware) UnmarshalPacketData(ctx sdk.Context, portID string, channelID string, bz []byte) (any, string, error) { return im.app.UnmarshalPacketData(ctx, portID, channelID, bz) } + +func (im *IBCMiddleware) SetICS4Wrapper(wrapper porttypes.ICS4Wrapper) { + if wrapper == nil { + panic("ICS4Wrapper cannot be nil") + } + im.keeper.SetICS4Wrapper(wrapper) +} + +func (im *IBCMiddleware) SetUnderlyingApplication(app porttypes.IBCModule) { + if im.app != nil { + panic("underlying application already set") + } + // the underlying application must implement the PacketUnmarshalerModule interface + pdApp, ok := app.(porttypes.PacketUnmarshalerModule) + if !ok { + panic(fmt.Errorf("underlying application must implement PacketUnmarshalerModule, got %T", app)) + } + im.app = pdApp +} diff --git a/modules/apps/rate-limiting/keeper/grpc_query_test.go b/modules/apps/rate-limiting/keeper/grpc_query_test.go index 60487292bb9..8d24614a76b 100644 --- a/modules/apps/rate-limiting/keeper/grpc_query_test.go +++ b/modules/apps/rate-limiting/keeper/grpc_query_test.go @@ -46,7 +46,7 @@ func (s *KeeperTestSuite) setupQueryRateLimitTests() []types.RateLimit { } func (s *KeeperTestSuite) TestQueryAllRateLimits() { - querier := keeper.NewQuerier(&s.chainA.GetSimApp().RateLimitKeeper) + querier := keeper.NewQuerier(s.chainA.GetSimApp().RateLimitKeeper) expectedRateLimits := s.setupQueryRateLimitTests() queryResponse, err := querier.AllRateLimits(s.chainA.GetContext(), &types.QueryAllRateLimitsRequest{}) s.Require().NoError(err) @@ -54,7 +54,7 @@ func (s *KeeperTestSuite) TestQueryAllRateLimits() { } func (s *KeeperTestSuite) TestQueryRateLimit() { - querier := keeper.NewQuerier(&s.chainA.GetSimApp().RateLimitKeeper) + querier := keeper.NewQuerier(s.chainA.GetSimApp().RateLimitKeeper) allRateLimits := s.setupQueryRateLimitTests() for _, expectedRateLimit := range allRateLimits { queryResponse, err := querier.RateLimit(s.chainA.GetContext(), &types.QueryRateLimitRequest{ @@ -67,7 +67,7 @@ func (s *KeeperTestSuite) TestQueryRateLimit() { } func (s *KeeperTestSuite) TestQueryRateLimitsByChainId() { - querier := keeper.NewQuerier(&s.chainA.GetSimApp().RateLimitKeeper) + querier := keeper.NewQuerier(s.chainA.GetSimApp().RateLimitKeeper) allRateLimits := s.setupQueryRateLimitTests() for i, expectedRateLimit := range allRateLimits { chainID := fmt.Sprintf("chain-%d", i) @@ -81,7 +81,7 @@ func (s *KeeperTestSuite) TestQueryRateLimitsByChainId() { } func (s *KeeperTestSuite) TestQueryRateLimitsByChannelOrClientId() { - querier := keeper.NewQuerier(&s.chainA.GetSimApp().RateLimitKeeper) + querier := keeper.NewQuerier(s.chainA.GetSimApp().RateLimitKeeper) allRateLimits := s.setupQueryRateLimitTests() for i, expectedRateLimit := range allRateLimits { channelID := fmt.Sprintf("channel-%d", i) @@ -95,7 +95,7 @@ func (s *KeeperTestSuite) TestQueryRateLimitsByChannelOrClientId() { } func (s *KeeperTestSuite) TestQueryAllBlacklistedDenoms() { - querier := keeper.NewQuerier(&s.chainA.GetSimApp().RateLimitKeeper) + querier := keeper.NewQuerier(s.chainA.GetSimApp().RateLimitKeeper) s.chainA.GetSimApp().RateLimitKeeper.AddDenomToBlacklist(s.chainA.GetContext(), "denom-A") s.chainA.GetSimApp().RateLimitKeeper.AddDenomToBlacklist(s.chainA.GetContext(), "denom-B") @@ -105,7 +105,7 @@ func (s *KeeperTestSuite) TestQueryAllBlacklistedDenoms() { } func (s *KeeperTestSuite) TestQueryAllWhitelistedAddresses() { - querier := keeper.NewQuerier(&s.chainA.GetSimApp().RateLimitKeeper) + querier := keeper.NewQuerier(s.chainA.GetSimApp().RateLimitKeeper) s.chainA.GetSimApp().RateLimitKeeper.SetWhitelistedAddressPair(s.chainA.GetContext(), types.WhitelistedAddressPair{ Sender: "address-A", Receiver: "address-B", diff --git a/modules/apps/rate-limiting/keeper/ics4.go b/modules/apps/rate-limiting/keeper/ics4.go new file mode 100644 index 00000000000..4930be7916d --- /dev/null +++ b/modules/apps/rate-limiting/keeper/ics4.go @@ -0,0 +1,20 @@ +package keeper + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + + clienttypes "github.com/cosmos/ibc-go/v10/modules/core/02-client/types" + "github.com/cosmos/ibc-go/v10/modules/core/exported" +) + +func (k *Keeper) SendPacket(ctx sdk.Context, sourcePort string, sourceChannel string, timeoutHeight clienttypes.Height, timeoutTimestamp uint64, data []byte) (uint64, error) { + return k.ics4Wrapper.SendPacket(ctx, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, data) +} + +func (k *Keeper) WriteAcknowledgement(ctx sdk.Context, packet exported.PacketI, ack exported.Acknowledgement) error { + return k.ics4Wrapper.WriteAcknowledgement(ctx, packet, ack) +} + +func (k *Keeper) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) { + return k.ics4Wrapper.GetAppVersion(ctx, portID, channelID) +} diff --git a/modules/apps/rate-limiting/keeper/keeper.go b/modules/apps/rate-limiting/keeper/keeper.go index dc55f238892..6f641c8104b 100644 --- a/modules/apps/rate-limiting/keeper/keeper.go +++ b/modules/apps/rate-limiting/keeper/keeper.go @@ -29,15 +29,17 @@ type Keeper struct { } // NewKeeper creates a new rate-limiting Keeper instance -func NewKeeper(cdc codec.BinaryCodec, storeService corestore.KVStoreService, ics4Wrapper porttypes.ICS4Wrapper, channelKeeper types.ChannelKeeper, clientKeeper types.ClientKeeper, bankKeeper types.BankKeeper, authority string) Keeper { +func NewKeeper(cdc codec.BinaryCodec, storeService corestore.KVStoreService, channelKeeper types.ChannelKeeper, clientKeeper types.ClientKeeper, bankKeeper types.BankKeeper, authority string) *Keeper { if strings.TrimSpace(authority) == "" { panic(errors.New("authority must be non-empty")) } - return Keeper{ - cdc: cdc, - storeService: storeService, - ics4Wrapper: ics4Wrapper, + return &Keeper{ + cdc: cdc, + storeService: storeService, + // Defaults to using the channel keeper as the ICS4Wrapper + // This can be overridden later with WithICS4Wrapper (e.g. by the middleware stack wiring) + ics4Wrapper: channelKeeper, channelKeeper: channelKeeper, clientKeeper: clientKeeper, bankKeeper: bankKeeper, diff --git a/modules/apps/rate-limiting/keeper/keeper_test.go b/modules/apps/rate-limiting/keeper/keeper_test.go index df4968f813f..643be32174f 100644 --- a/modules/apps/rate-limiting/keeper/keeper_test.go +++ b/modules/apps/rate-limiting/keeper/keeper_test.go @@ -45,7 +45,6 @@ func (s *KeeperTestSuite) TestNewKeeper() { keeper.NewKeeper( s.chainA.GetSimApp().AppCodec(), runtime.NewKVStoreService(s.chainA.GetSimApp().GetKey(ratelimittypes.StoreKey)), - s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, // This is now used as ics4Wrapper s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, s.chainA.GetSimApp().IBCKeeper.ClientKeeper, // Add clientKeeper s.chainA.GetSimApp().BankKeeper, @@ -60,7 +59,6 @@ func (s *KeeperTestSuite) TestNewKeeper() { keeper.NewKeeper( s.chainA.GetSimApp().AppCodec(), runtime.NewKVStoreService(s.chainA.GetSimApp().GetKey(ratelimittypes.StoreKey)), - s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, // ics4Wrapper s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, s.chainA.GetSimApp().IBCKeeper.ClientKeeper, // clientKeeper s.chainA.GetSimApp().BankKeeper, diff --git a/modules/apps/rate-limiting/keeper/msg_server.go b/modules/apps/rate-limiting/keeper/msg_server.go index 81d534963b2..ac9d8a05ba6 100644 --- a/modules/apps/rate-limiting/keeper/msg_server.go +++ b/modules/apps/rate-limiting/keeper/msg_server.go @@ -12,11 +12,11 @@ import ( ) type msgServer struct { - Keeper + *Keeper } // NewMsgServerImpl returns an implementation of the ratelimit MsgServer interface -func NewMsgServerImpl(keeper Keeper) types.MsgServer { +func NewMsgServerImpl(keeper *Keeper) types.MsgServer { return &msgServer{Keeper: keeper} } diff --git a/modules/apps/rate-limiting/keeper/packet_test.go b/modules/apps/rate-limiting/keeper/packet_test.go index 872805d8f3c..28406207864 100644 --- a/modules/apps/rate-limiting/keeper/packet_test.go +++ b/modules/apps/rate-limiting/keeper/packet_test.go @@ -16,7 +16,6 @@ import ( tmbytes "github.com/cometbft/cometbft/libs/bytes" - packerforwardkeeper "github.com/cosmos/ibc-go/v10/modules/apps/packet-forward-middleware/keeper" ratelimiting "github.com/cosmos/ibc-go/v10/modules/apps/rate-limiting" "github.com/cosmos/ibc-go/v10/modules/apps/rate-limiting/keeper" "github.com/cosmos/ibc-go/v10/modules/apps/rate-limiting/types" @@ -702,14 +701,9 @@ func (s *KeeperTestSuite) TestSendPacket_Allowed() { packetDataBz, err := json.Marshal(packetData) s.Require().NoError(err) - // Get the middleware instance (assuming it's accessible via SimApp - needs verification) - // We need the transfer keeper's ICS4Wrapper which *is* the packet forward middleware's keeper - shouldPFM, ok := s.chainA.GetSimApp().TransferKeeper.GetICS4Wrapper().(*packerforwardkeeper.Keeper) - s.Require().Truef(ok, "Transfer keeper's ICS4Wrapper should be the PacketForward Middleware. Found %T", shouldPFM) - // We need the transfer keeper's ICS4Wrapper which *is* the ratelimiting middleware - middleware, ok := s.chainA.GetSimApp().PFMKeeper.ICS4Wrapper().(ratelimiting.IBCMiddleware) - s.Require().Truef(ok, "PFM keeper's ICS4Wrapper should be the PacketForward Middleware. Found %T", middleware) + middleware, ok := s.chainA.GetSimApp().PFMKeeper.ICS4Wrapper().(*ratelimiting.IBCMiddleware) + s.Require().Truef(ok, "PFM's ICS4Wrapper should be the Rate Limit Middleware. Found %T", s.chainA.GetSimApp().TransferKeeper.GetICS4Wrapper()) // Directly call the middleware's SendPacket seq, err := middleware.SendPacket( @@ -767,7 +761,7 @@ func (s *KeeperTestSuite) TestSendPacket_Denied() { s.Require().NoError(err) // Get the middleware instance - middleware, ok := s.chainA.GetSimApp().PFMKeeper.ICS4Wrapper().(ratelimiting.IBCMiddleware) + middleware, ok := s.chainA.GetSimApp().PFMKeeper.ICS4Wrapper().(*ratelimiting.IBCMiddleware) s.Require().Truef(ok, "Packet forward middleware keeper's ICS4Wrapper should be the RateLimit middleware. Found: %T", middleware) // Directly call the middleware's SendPacket diff --git a/modules/apps/rate-limiting/module.go b/modules/apps/rate-limiting/module.go index 214679e9275..b3ddafb7865 100644 --- a/modules/apps/rate-limiting/module.go +++ b/modules/apps/rate-limiting/module.go @@ -94,11 +94,11 @@ func (AppModuleBasic) GetQueryCmd() *cobra.Command { // AppModule represents the AppModule for this module type AppModule struct { AppModuleBasic - keeper keeper.Keeper + keeper *keeper.Keeper } // NewAppModule creates a new rate-limiting module -func NewAppModule(k keeper.Keeper) AppModule { +func NewAppModule(k *keeper.Keeper) AppModule { return AppModule{ keeper: k, } @@ -107,7 +107,7 @@ func NewAppModule(k keeper.Keeper) AppModule { // RegisterServices registers module services. func (am AppModule) RegisterServices(cfg module.Configurator) { types.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServerImpl(am.keeper)) // Use the msgServer implementation - types.RegisterQueryServer(cfg.QueryServer(), keeper.NewQuerier(&am.keeper)) + types.RegisterQueryServer(cfg.QueryServer(), keeper.NewQuerier(am.keeper)) } // InitGenesis performs genesis initialization for the rate-limiting module. It returns diff --git a/modules/apps/rate-limiting/types/expected_keepers.go b/modules/apps/rate-limiting/types/expected_keepers.go index f99f7019c4e..916638db9b4 100644 --- a/modules/apps/rate-limiting/types/expected_keepers.go +++ b/modules/apps/rate-limiting/types/expected_keepers.go @@ -6,6 +6,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" channeltypes "github.com/cosmos/ibc-go/v10/modules/core/04-channel/types" + porttypes "github.com/cosmos/ibc-go/v10/modules/core/05-port/types" "github.com/cosmos/ibc-go/v10/modules/core/exported" ) @@ -16,6 +17,7 @@ type BankKeeper interface { // ChannelKeeper defines the expected IBC channel keeper type ChannelKeeper interface { + porttypes.ICS4Wrapper GetChannel(ctx sdk.Context, srcPort, srcChan string) (channel channeltypes.Channel, found bool) GetChannelClientState(ctx sdk.Context, portID, channelID string) (clientID string, clientState exported.ClientState, err error) GetNextSequenceSend(ctx sdk.Context, sourcePort, sourceChannel string) (uint64, bool) diff --git a/modules/apps/transfer/ibc_module.go b/modules/apps/transfer/ibc_module.go index 54f6bcd1ed3..4a1d3d401e2 100644 --- a/modules/apps/transfer/ibc_module.go +++ b/modules/apps/transfer/ibc_module.go @@ -28,12 +28,12 @@ var ( // IBCModule implements the ICS26 interface for transfer given the transfer keeper. type IBCModule struct { - keeper keeper.Keeper + keeper *keeper.Keeper } // NewIBCModule creates a new IBCModule given the keeper -func NewIBCModule(k keeper.Keeper) IBCModule { - return IBCModule{ +func NewIBCModule(k *keeper.Keeper) *IBCModule { + return &IBCModule{ keeper: k, } } @@ -80,7 +80,7 @@ func (im IBCModule) OnChanOpenInit( counterparty channeltypes.Counterparty, version string, ) (string, error) { - if err := ValidateTransferChannelParams(ctx, im.keeper, order, portID, channelID); err != nil { + if err := ValidateTransferChannelParams(ctx, *im.keeper, order, portID, channelID); err != nil { return "", err } @@ -106,7 +106,7 @@ func (im IBCModule) OnChanOpenTry( counterparty channeltypes.Counterparty, counterpartyVersion string, ) (string, error) { - if err := ValidateTransferChannelParams(ctx, im.keeper, order, portID, channelID); err != nil { + if err := ValidateTransferChannelParams(ctx, *im.keeper, order, portID, channelID); err != nil { return "", err } @@ -280,3 +280,14 @@ func (im IBCModule) UnmarshalPacketData(ctx sdk.Context, portID string, channelI ftpd, err := types.UnmarshalPacketData(bz, ics20Version, "") return ftpd, ics20Version, err } + +// SetICS4Wrapper sets the ICS4Wrapper. This function may be used after +// the module's initialization to set the middleware which is above this +// module in the IBC application stack. +func (im IBCModule) SetICS4Wrapper(wrapper porttypes.ICS4Wrapper) { + if wrapper == nil { + panic("ICS4Wrapper cannot be nil") + } + + im.keeper.WithICS4Wrapper(wrapper) +} diff --git a/modules/apps/transfer/ibc_module_test.go b/modules/apps/transfer/ibc_module_test.go index 7c873137a28..9a363a91331 100644 --- a/modules/apps/transfer/ibc_module_test.go +++ b/modules/apps/transfer/ibc_module_test.go @@ -19,6 +19,18 @@ import ( ibctesting "github.com/cosmos/ibc-go/v10/testing" ) +func (s *TransferTestSuite) TestSetICS4Wrapper() { + transferModule := transfer.NewIBCModule(s.chainA.GetSimApp().TransferKeeper) + + s.Require().Panics(func() { + transferModule.SetICS4Wrapper(nil) + }, "ICS4Wrapper cannot be nil") + + s.Require().NotPanics(func() { + transferModule.SetICS4Wrapper(s.chainA.App.GetIBCKeeper().ChannelKeeper) + }, "ICS4Wrapper can be set to a non-nil value") +} + func (s *TransferTestSuite) TestOnChanOpenInit() { var ( channel *channeltypes.Channel diff --git a/modules/apps/transfer/keeper/keeper.go b/modules/apps/transfer/keeper/keeper.go index 9b048541d71..8f7b7ea4bb2 100644 --- a/modules/apps/transfer/keeper/keeper.go +++ b/modules/apps/transfer/keeper/keeper.go @@ -43,13 +43,12 @@ type Keeper struct { func NewKeeper( cdc codec.BinaryCodec, storeService corestore.KVStoreService, - ics4Wrapper porttypes.ICS4Wrapper, channelKeeper types.ChannelKeeper, msgRouter types.MessageRouter, authKeeper types.AccountKeeper, bankKeeper types.BankKeeper, authority string, -) Keeper { +) *Keeper { // ensure ibc transfer module account is set if addr := authKeeper.GetModuleAddress(types.ModuleName); addr == nil { panic(errors.New("the IBC transfer module account has not been set")) @@ -59,10 +58,10 @@ func NewKeeper( panic(errors.New("authority must be non-empty")) } - return Keeper{ + return &Keeper{ cdc: cdc, storeService: storeService, - ics4Wrapper: ics4Wrapper, + ics4Wrapper: channelKeeper, // default ICS4Wrapper is the channel keeper channelKeeper: channelKeeper, msgRouter: msgRouter, AuthKeeper: authKeeper, diff --git a/modules/apps/transfer/keeper/keeper_test.go b/modules/apps/transfer/keeper/keeper_test.go index e9d85e69bb8..f4bc2d64b85 100644 --- a/modules/apps/transfer/keeper/keeper_test.go +++ b/modules/apps/transfer/keeper/keeper_test.go @@ -17,7 +17,7 @@ import ( authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" minttypes "github.com/cosmos/cosmos-sdk/x/mint/types" - packetforwardkeeper "github.com/cosmos/ibc-go/v10/modules/apps/packet-forward-middleware/keeper" + packetforward "github.com/cosmos/ibc-go/v10/modules/apps/packet-forward-middleware" "github.com/cosmos/ibc-go/v10/modules/apps/transfer/keeper" "github.com/cosmos/ibc-go/v10/modules/apps/transfer/types" ibctesting "github.com/cosmos/ibc-go/v10/testing" @@ -41,7 +41,7 @@ func (s *KeeperTestSuite) SetupTest() { s.chainC = s.coordinator.GetChain(ibctesting.GetChainID(3)) queryHelper := baseapp.NewQueryServerTestHelper(s.chainA.GetContext(), s.chainA.GetSimApp().InterfaceRegistry()) - types.RegisterQueryServer(queryHelper, &s.chainA.GetSimApp().TransferKeeper) + types.RegisterQueryServer(queryHelper, s.chainA.GetSimApp().TransferKeeper) } func TestKeeperTestSuite(t *testing.T) { @@ -59,7 +59,6 @@ func (s *KeeperTestSuite) TestNewKeeper() { s.chainA.GetSimApp().AppCodec(), runtime.NewKVStoreService(s.chainA.GetSimApp().GetKey(types.StoreKey)), s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, - s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, s.chainA.GetSimApp().MsgServiceRouter(), s.chainA.GetSimApp().AccountKeeper, s.chainA.GetSimApp().BankKeeper, @@ -71,7 +70,6 @@ func (s *KeeperTestSuite) TestNewKeeper() { s.chainA.GetSimApp().AppCodec(), runtime.NewKVStoreService(s.chainA.GetSimApp().GetKey(types.StoreKey)), s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, - s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, s.chainA.GetSimApp().MsgServiceRouter(), authkeeper.AccountKeeper{}, // empty account keeper s.chainA.GetSimApp().BankKeeper, @@ -83,7 +81,6 @@ func (s *KeeperTestSuite) TestNewKeeper() { s.chainA.GetSimApp().AppCodec(), runtime.NewKVStoreService(s.chainA.GetSimApp().GetKey(types.StoreKey)), s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, - s.chainA.GetSimApp().IBCKeeper.ChannelKeeper, s.chainA.GetSimApp().MsgServiceRouter(), s.chainA.GetSimApp().AccountKeeper, s.chainA.GetSimApp().BankKeeper, @@ -337,9 +334,9 @@ func (s *KeeperTestSuite) TestWithICS4Wrapper() { // test if the ics4 wrapper is the pfm keeper initially ics4Wrapper := s.chainA.GetSimApp().TransferKeeper.GetICS4Wrapper() - _, isPFMKeeper := ics4Wrapper.(*packetforwardkeeper.Keeper) + _, isPFMKeeper := ics4Wrapper.(*packetforward.IBCMiddleware) s.Require().True(isPFMKeeper) - s.Require().IsType((*packetforwardkeeper.Keeper)(nil), ics4Wrapper) + s.Require().IsType((*packetforward.IBCMiddleware)(nil), ics4Wrapper) // set the ics4 wrapper to the channel keeper s.chainA.GetSimApp().TransferKeeper.WithICS4Wrapper(nil) diff --git a/modules/apps/transfer/keeper/migrations_test.go b/modules/apps/transfer/keeper/migrations_test.go index 24259241944..83b97b93551 100644 --- a/modules/apps/transfer/keeper/migrations_test.go +++ b/modules/apps/transfer/keeper/migrations_test.go @@ -139,7 +139,7 @@ func (s *KeeperTestSuite) TestMigratorMigrateDenomTraceToDenom() { tc.malleate() - migrator := transferkeeper.NewMigrator(s.chainA.GetSimApp().TransferKeeper) + migrator := transferkeeper.NewMigrator(*s.chainA.GetSimApp().TransferKeeper) err := migrator.MigrateDenomTraceToDenom(s.chainA.GetContext()) s.Require().NoError(err) @@ -183,7 +183,7 @@ func (s *KeeperTestSuite) TestMigratorMigrateDenomTraceToDenomCorruptionDetectio s.chainA.GetSimApp().TransferKeeper.SetDenomTrace(s.chainA.GetContext(), tc.denomTrace) - migrator := transferkeeper.NewMigrator(s.chainA.GetSimApp().TransferKeeper) + migrator := transferkeeper.NewMigrator(*s.chainA.GetSimApp().TransferKeeper) s.Panics(func() { migrator.MigrateDenomTraceToDenom(s.chainA.GetContext()) //nolint:errcheck // we shouldn't check the error here because we want to ensure that a panic occurs. }) @@ -256,7 +256,7 @@ func (s *KeeperTestSuite) TestMigrateTotalEscrowForDenom() { tc.malleate() // explicitly fund escrow account - migrator := transferkeeper.NewMigrator(s.chainA.GetSimApp().TransferKeeper) + migrator := transferkeeper.NewMigrator(*s.chainA.GetSimApp().TransferKeeper) s.Require().NoError(migrator.MigrateTotalEscrowForDenom(s.chainA.GetContext())) // check that the migration set the expected amount for both native and IBC tokens @@ -435,7 +435,7 @@ func (s *KeeperTestSuite) TestMigratorMigrateMetadata() { } // run migration - migrator := transferkeeper.NewMigrator(s.chainA.GetSimApp().TransferKeeper) + migrator := transferkeeper.NewMigrator(*s.chainA.GetSimApp().TransferKeeper) err := migrator.MigrateDenomMetadata(ctx) s.Require().NoError(err) diff --git a/modules/apps/transfer/module.go b/modules/apps/transfer/module.go index eda76912196..666a86bc6a4 100644 --- a/modules/apps/transfer/module.go +++ b/modules/apps/transfer/module.go @@ -102,9 +102,9 @@ type AppModule struct { } // NewAppModule creates a new 20-transfer module -func NewAppModule(k keeper.Keeper) AppModule { +func NewAppModule(k *keeper.Keeper) AppModule { return AppModule{ - keeper: &k, + keeper: k, } } diff --git a/modules/apps/transfer/types/expected_keepers.go b/modules/apps/transfer/types/expected_keepers.go index 2182f383696..098be515908 100644 --- a/modules/apps/transfer/types/expected_keepers.go +++ b/modules/apps/transfer/types/expected_keepers.go @@ -11,6 +11,7 @@ import ( connectiontypes "github.com/cosmos/ibc-go/v10/modules/core/03-connection/types" channeltypes "github.com/cosmos/ibc-go/v10/modules/core/04-channel/types" channeltypesv2 "github.com/cosmos/ibc-go/v10/modules/core/04-channel/v2/types" + porttypes "github.com/cosmos/ibc-go/v10/modules/core/05-port/types" ) // AccountKeeper defines the contract required for account APIs. @@ -36,6 +37,7 @@ type BankKeeper interface { // ChannelKeeper defines the expected IBC channel keeper type ChannelKeeper interface { + porttypes.ICS4Wrapper GetChannel(ctx sdk.Context, srcPort, srcChan string) (channel channeltypes.Channel, found bool) GetNextSequenceSend(ctx sdk.Context, portID, channelID string) (uint64, bool) GetAllChannelsWithPortPrefix(ctx sdk.Context, portPrefix string) []channeltypes.IdentifiedChannel diff --git a/modules/apps/transfer/v2/ibc_module.go b/modules/apps/transfer/v2/ibc_module.go index 36fc2206a96..ffda9a4c392 100644 --- a/modules/apps/transfer/v2/ibc_module.go +++ b/modules/apps/transfer/v2/ibc_module.go @@ -23,14 +23,14 @@ import ( var _ api.IBCModule = (*IBCModule)(nil) // NewIBCModule creates a new IBCModule given the keeper -func NewIBCModule(k keeper.Keeper) IBCModule { +func NewIBCModule(k *keeper.Keeper) IBCModule { return IBCModule{ keeper: k, } } type IBCModule struct { - keeper keeper.Keeper + keeper *keeper.Keeper } func (im IBCModule) OnSendPacket(ctx sdk.Context, sourceChannel string, destinationChannel string, sequence uint64, payload channeltypesv2.Payload, signer sdk.AccAddress) error { diff --git a/modules/core/05-port/types/module.go b/modules/core/05-port/types/module.go index 28cfcc9447d..67f33a0b855 100644 --- a/modules/core/05-port/types/module.go +++ b/modules/core/05-port/types/module.go @@ -104,6 +104,15 @@ type IBCModule interface { packet channeltypes.Packet, relayer sdk.AccAddress, ) error + + // SetICS4Wrapper sets the ICS4Wrapper. This function may be used after + // the module's initialization to set the middleware which is above this + // module in the IBC application stack. + // The ICS4Wrapper **must** be used for sending packets and writing acknowledgements + // to ensure that the middleware can intercept and process these calls. + // Do not use the channel keeper directly to send packets or write acknowledgements + // as this will bypass the middleware. + SetICS4Wrapper(wrapper ICS4Wrapper) } // ICS4Wrapper implements the ICS4 interfaces that IBC applications use to send packets and acknowledgements. @@ -135,6 +144,10 @@ type ICS4Wrapper interface { type Middleware interface { IBCModule ICS4Wrapper + + // SetUnderlyingModule sets the underlying IBC module. This function may be used after + // the middleware's initialization to set the ibc module which is below this middleware. + SetUnderlyingApplication(IBCModule) } // PacketDataUnmarshaler defines an optional interface which allows a middleware to @@ -146,7 +159,7 @@ type PacketDataUnmarshaler interface { UnmarshalPacketData(ctx sdk.Context, portID string, channelID string, bz []byte) (any, string, error) } -type PacketUnmarshalarModule interface { +type PacketUnmarshalerModule interface { PacketDataUnmarshaler IBCModule } diff --git a/modules/core/05-port/types/stack.go b/modules/core/05-port/types/stack.go new file mode 100644 index 00000000000..90bfd7488cf --- /dev/null +++ b/modules/core/05-port/types/stack.go @@ -0,0 +1,57 @@ +package types + +type IBCStackBuilder struct { + middlewares []Middleware + baseModule IBCModule + channelKeeper ICS4Wrapper +} + +func NewIBCStackBuilder(chanKeeper ICS4Wrapper) *IBCStackBuilder { + return &IBCStackBuilder{ + channelKeeper: chanKeeper, + } +} + +func (b *IBCStackBuilder) Next(middleware Middleware) *IBCStackBuilder { + b.middlewares = append(b.middlewares, middleware) + return b +} + +func (b *IBCStackBuilder) Base(baseModule IBCModule) *IBCStackBuilder { + if baseModule == nil { + panic("base module cannot be nil") + } + if b.baseModule != nil { + panic("base module already set") + } + b.baseModule = baseModule + return b +} + +func (b *IBCStackBuilder) Build() IBCModule { + if b.baseModule == nil { + panic("base module cannot be nil") + } + if len(b.middlewares) == 0 { + panic("middlewares cannot be empty") + } + if b.channelKeeper == nil { + panic("channel keeper cannot be nil") + } + + // Build the stack by moving up the middleware list + // and setting the underlying application for each middleware + // and the ICS4wrapper for the underlying module. + underlyingModule := b.baseModule + for i := range len(b.middlewares) { + b.middlewares[i].SetUnderlyingApplication(underlyingModule) + underlyingModule.SetICS4Wrapper(b.middlewares[i]) + underlyingModule = b.middlewares[i] + } + + // set the top level channel keeper as the ICS4Wrapper + // for the lop level middleware + b.middlewares[len(b.middlewares)-1].SetICS4Wrapper(b.channelKeeper) + + return b.middlewares[len(b.middlewares)-1] +} diff --git a/modules/core/ante/ante_test.go b/modules/core/ante/ante_test.go index 77d1396d470..154ba6c1619 100644 --- a/modules/core/ante/ante_test.go +++ b/modules/core/ante/ante_test.go @@ -536,11 +536,11 @@ func (s *AnteTestSuite) TestAnteDecoratorCheckTx() { }, { "no success on one new malicious UpdateClient message and three redundant RecvPacket messages", - func(suite *AnteTestSuite) []sdk.Msg { - msgs := []sdk.Msg{suite.createMaliciousUpdateClientMessage()} + func(s *AnteTestSuite) []sdk.Msg { + msgs := []sdk.Msg{s.createMaliciousUpdateClientMessage()} for i := 1; i <= 3; i++ { - msgs = append(msgs, suite.createRecvPacketMessage(true)) + msgs = append(msgs, s.createRecvPacketMessage(true)) } return msgs diff --git a/modules/core/api/module.go b/modules/core/api/module.go index cbc22af8fb1..b1b73b40912 100644 --- a/modules/core/api/module.go +++ b/modules/core/api/module.go @@ -70,9 +70,9 @@ type PacketDataUnmarshaler interface { UnmarshalPacketData(payload channeltypesv2.Payload) (any, error) } -// PacketUnmarshalarModuleV2 is an interface that combines the IBCModuleV2 and PacketDataUnmarshaler +// PacketUnmarshalerModuleV2 is an interface that combines the IBCModuleV2 and PacketDataUnmarshaler // interfaces to assert that the underlying application supports both. -type PacketUnmarshalarModuleV2 interface { +type PacketUnmarshalerModuleV2 interface { IBCModule PacketDataUnmarshaler } diff --git a/modules/light-clients/08-wasm/testing/simapp/app.go b/modules/light-clients/08-wasm/testing/simapp/app.go index 84041075499..2a3adfbc0d1 100644 --- a/modules/light-clients/08-wasm/testing/simapp/app.go +++ b/modules/light-clients/08-wasm/testing/simapp/app.go @@ -175,10 +175,10 @@ type SimApp struct { UpgradeKeeper *upgradekeeper.Keeper AuthzKeeper authzkeeper.Keeper IBCKeeper *ibckeeper.Keeper // IBC Keeper must be a pointer in the app, so we can SetRouter on it correctly - ICAControllerKeeper icacontrollerkeeper.Keeper - ICAHostKeeper icahostkeeper.Keeper + ICAControllerKeeper *icacontrollerkeeper.Keeper + ICAHostKeeper *icahostkeeper.Keeper EvidenceKeeper evidencekeeper.Keeper - TransferKeeper ibctransferkeeper.Keeper + TransferKeeper *ibctransferkeeper.Keeper WasmClientKeeper ibcwasmkeeper.Keeper FeeGrantKeeper feegrantkeeper.Keeper GroupKeeper groupkeeper.Keeper @@ -187,7 +187,7 @@ type SimApp struct { // make IBC modules public for test purposes // these modules are never directly routed to by the IBC Router - ICAAuthModule ibcmock.IBCModule + ICAAuthModule *ibcmock.IBCModule // the module manager ModuleManager *module.Manager @@ -451,7 +451,6 @@ func newSimApp( app.ICAControllerKeeper = icacontrollerkeeper.NewKeeper( appCodec, runtime.NewKVStoreService(keys[icacontrollertypes.StoreKey]), app.IBCKeeper.ChannelKeeper, - app.IBCKeeper.ChannelKeeper, app.MsgServiceRouter(), authtypes.NewModuleAddress(govtypes.ModuleName).String(), ) @@ -460,7 +459,6 @@ func newSimApp( app.ICAHostKeeper = icahostkeeper.NewKeeper( appCodec, runtime.NewKVStoreService(keys[icahosttypes.StoreKey]), app.IBCKeeper.ChannelKeeper, - app.IBCKeeper.ChannelKeeper, app.AccountKeeper, app.MsgServiceRouter(), app.GRPCQueryRouter(), authtypes.NewModuleAddress(govtypes.ModuleName).String(), ) @@ -475,7 +473,6 @@ func newSimApp( app.TransferKeeper = ibctransferkeeper.NewKeeper( appCodec, runtime.NewKVStoreService(keys[ibctransfertypes.StoreKey]), app.IBCKeeper.ChannelKeeper, - app.IBCKeeper.ChannelKeeper, app.MsgServiceRouter(), app.AccountKeeper, app.BankKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String(), @@ -516,7 +513,7 @@ func newSimApp( var icaControllerStack porttypes.IBCModule var ok bool icaControllerStack = ibcmock.NewIBCModule(&mockModule, ibcmock.NewIBCApp("")) - app.ICAAuthModule, ok = icaControllerStack.(ibcmock.IBCModule) + app.ICAAuthModule, ok = icaControllerStack.(*ibcmock.IBCModule) if !ok { panic(fmt.Errorf("cannot convert %T into %T", icaControllerStack, app.ICAAuthModule)) } @@ -587,7 +584,7 @@ func newSimApp( // IBC modules ibc.NewAppModule(app.IBCKeeper), transfer.NewAppModule(app.TransferKeeper), - ica.NewAppModule(&app.ICAControllerKeeper, &app.ICAHostKeeper), + ica.NewAppModule(app.ICAControllerKeeper, app.ICAHostKeeper), mockModule, // IBC light clients diff --git a/simapp/app.go b/simapp/app.go index f9cea7f6b24..89c026d6385 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -169,16 +169,16 @@ type SimApp struct { UpgradeKeeper *upgradekeeper.Keeper AuthzKeeper authzkeeper.Keeper IBCKeeper *ibckeeper.Keeper // IBC Keeper must be a pointer in the app, so we can SetRouter on it correctly - ICAControllerKeeper icacontrollerkeeper.Keeper - ICAHostKeeper icahostkeeper.Keeper + ICAControllerKeeper *icacontrollerkeeper.Keeper + ICAHostKeeper *icahostkeeper.Keeper EvidenceKeeper evidencekeeper.Keeper - TransferKeeper ibctransferkeeper.Keeper + TransferKeeper *ibctransferkeeper.Keeper FeeGrantKeeper feegrantkeeper.Keeper GroupKeeper groupkeeper.Keeper ConsensusParamsKeeper consensusparamkeeper.Keeper CircuitKeeper circuitkeeper.Keeper PFMKeeper *packetforwardkeeper.Keeper - RateLimitKeeper ratelimitkeeper.Keeper + RateLimitKeeper *ratelimitkeeper.Keeper // the module manager ModuleManager *module.Manager @@ -361,8 +361,8 @@ func NewSimApp( // ICA Controller keeper app.ICAControllerKeeper = icacontrollerkeeper.NewKeeper( - appCodec, runtime.NewKVStoreService(keys[icacontrollertypes.StoreKey]), - app.IBCKeeper.ChannelKeeper, + appCodec, + runtime.NewKVStoreService(keys[icacontrollertypes.StoreKey]), app.IBCKeeper.ChannelKeeper, app.MsgServiceRouter(), govAuthority, @@ -371,27 +371,28 @@ func NewSimApp( // ICA Host keeper app.ICAHostKeeper = icahostkeeper.NewKeeper( appCodec, runtime.NewKVStoreService(keys[icahosttypes.StoreKey]), - app.IBCKeeper.ChannelKeeper, app.IBCKeeper.ChannelKeeper, app.AccountKeeper, app.MsgServiceRouter(), app.GRPCQueryRouter(), govAuthority, ) - // Create IBC Router - ibcRouter := porttypes.NewRouter() - ibcRouterV2 := ibcapi.NewRouter() - - // Middleware Stacks - - app.RateLimitKeeper = ratelimitkeeper.NewKeeper(appCodec, runtime.NewKVStoreService(keys[ratelimittypes.StoreKey]), app.IBCKeeper.ChannelKeeper, app.IBCKeeper.ChannelKeeper, app.IBCKeeper.ClientKeeper, app.BankKeeper, govAuthority) + // Transfer Keeper + app.TransferKeeper = ibctransferkeeper.NewKeeper( + appCodec, runtime.NewKVStoreService(keys[ibctransfertypes.StoreKey]), + app.IBCKeeper.ChannelKeeper, + app.MsgServiceRouter(), + app.AccountKeeper, app.BankKeeper, + authtypes.NewModuleAddress(govtypes.ModuleName).String(), + ) - // PacketForwardMiddleware must be created before TransferKeeper - app.PFMKeeper = packetforwardkeeper.NewKeeper(appCodec, runtime.NewKVStoreService(keys[packetforwardtypes.StoreKey]), nil, app.IBCKeeper.ChannelKeeper, app.BankKeeper, app.RateLimitKeeper.ICS4Wrapper(), govAuthority) + // Packet Forward Middleware keeper + app.PFMKeeper = packetforwardkeeper.NewKeeper(appCodec, runtime.NewKVStoreService(keys[packetforwardtypes.StoreKey]), app.TransferKeeper, app.IBCKeeper.ChannelKeeper, app.BankKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String()) - // Create Transfer Keeper - app.TransferKeeper = ibctransferkeeper.NewKeeper(appCodec, runtime.NewKVStoreService(keys[ibctransfertypes.StoreKey]), app.IBCKeeper.ChannelKeeper, app.IBCKeeper.ChannelKeeper, app.MsgServiceRouter(), app.AccountKeeper, app.BankKeeper, govAuthority) + app.RateLimitKeeper = ratelimitkeeper.NewKeeper(appCodec, runtime.NewKVStoreService(keys[ratelimittypes.StoreKey]), app.IBCKeeper.ChannelKeeper, app.IBCKeeper.ClientKeeper, app.BankKeeper, govAuthority) - app.PFMKeeper.SetTransferKeeper(&app.TransferKeeper) + // Create IBC Router + ibcRouter := porttypes.NewRouter() + ibcRouterV2 := ibcapi.NewRouter() // Create Transfer Stack // SendPacket, since it is originating from the application to core IBC: @@ -406,15 +407,13 @@ func NewSimApp( // - Transfer // create IBC module from bottom to top of stack - pfmMiddleware := packetforward.NewIBCMiddleware(transfer.NewIBCModule(app.TransferKeeper), app.PFMKeeper, 0, packetforwardkeeper.DefaultForwardTransferPacketTimeoutTimestamp) - transferStack := ratelimiting.NewIBCMiddleware(pfmMiddleware, app.RateLimitKeeper, app.IBCKeeper.ChannelKeeper) - - app.PFMKeeper.SetICS4Wrapper(transferStack) - app.RateLimitKeeper.SetICS4Wrapper(app.IBCKeeper.ChannelKeeper) - app.TransferKeeper.WithICS4Wrapper(app.PFMKeeper) + transferStack := porttypes.NewIBCStackBuilder(app.IBCKeeper.ChannelKeeper) + transferStack.Base(transfer.NewIBCModule(app.TransferKeeper)). + Next(packetforward.NewIBCMiddleware(app.PFMKeeper, 0, packetforwardkeeper.DefaultForwardTransferPacketTimeoutTimestamp)). + Next(ratelimiting.NewIBCMiddleware(app.RateLimitKeeper)) // Add transfer stack to IBC Router - ibcRouter.AddRoute(ibctransfertypes.ModuleName, transferStack) + ibcRouter.AddRoute(ibctransfertypes.ModuleName, transferStack.Build()) // Packet Forward Middleware Stack. @@ -484,7 +483,7 @@ func NewSimApp( // IBC modules ibc.NewAppModule(app.IBCKeeper), transfer.NewAppModule(app.TransferKeeper), - ica.NewAppModule(&app.ICAControllerKeeper, &app.ICAHostKeeper), + ica.NewAppModule(app.ICAControllerKeeper, app.ICAHostKeeper), packetforward.NewAppModule(app.PFMKeeper), ratelimiting.NewAppModule(app.RateLimitKeeper), diff --git a/testing/mock/ibc_module.go b/testing/mock/ibc_module.go index f685e304ad7..975cebd7cd2 100644 --- a/testing/mock/ibc_module.go +++ b/testing/mock/ibc_module.go @@ -31,9 +31,9 @@ type IBCModule struct { } // NewIBCModule creates a new IBCModule given the underlying mock IBC application and scopedKeeper. -func NewIBCModule(appModule *AppModule, app *IBCApp) IBCModule { +func NewIBCModule(appModule *AppModule, app *IBCApp) *IBCModule { appModule.ibcApps = append(appModule.ibcApps, app) - return IBCModule{ + return &IBCModule{ appModule: appModule, IBCApp: app, } @@ -150,3 +150,9 @@ func (IBCModule) UnmarshalPacketData(ctx sdk.Context, portID string, channelID s } return nil, "", MockApplicationCallbackError } + +func (*IBCModule) SetICS4Wrapper(wrapper porttypes.ICS4Wrapper) { + if wrapper == nil { + panic("ICS4Wrapper cannot be nil") + } +} diff --git a/testing/mock/middleware.go b/testing/mock/middleware.go index 69fc3ad9bbb..e8344833bc2 100644 --- a/testing/mock/middleware.go +++ b/testing/mock/middleware.go @@ -155,3 +155,15 @@ func (BlockUpgradeMiddleware) WriteAcknowledgement( func (BlockUpgradeMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) { return Version, true } + +// SetICS4Wrapper sets the ICS4Wrapper. This function may be used after +// the module's initialization to set the middleware which is above this +// module in the IBC application stack. +func (BlockUpgradeMiddleware) SetICS4Wrapper(wrapper porttypes.ICS4Wrapper) { + panic("unused") +} + +// SetUnderlyingApplication sets the underlying application of the middleware. +func (BlockUpgradeMiddleware) SetUnderlyingApplication(app porttypes.IBCModule) { + panic("unused") +} diff --git a/testing/simapp/app.go b/testing/simapp/app.go index fdb7ad6b2fc..09af5e1a52d 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -154,17 +154,17 @@ type SimApp struct { UpgradeKeeper *upgradekeeper.Keeper AuthzKeeper authzkeeper.Keeper IBCKeeper *ibckeeper.Keeper // IBC Keeper must be a pointer in the app, so we can SetRouter on it correctly - ICAControllerKeeper icacontrollerkeeper.Keeper - ICAHostKeeper icahostkeeper.Keeper - TransferKeeper ibctransferkeeper.Keeper + ICAControllerKeeper *icacontrollerkeeper.Keeper + ICAHostKeeper *icahostkeeper.Keeper + TransferKeeper *ibctransferkeeper.Keeper ConsensusParamsKeeper consensusparamkeeper.Keeper PFMKeeper *packetforwardkeeper.Keeper - RateLimitKeeper ratelimitkeeper.Keeper + RateLimitKeeper *ratelimitkeeper.Keeper // make IBC modules public for test purposes // these modules are never directly routed to by the IBC Router - IBCMockModule ibcmock.IBCModule - ICAAuthModule ibcmock.IBCModule + IBCMockModule *ibcmock.IBCModule + ICAAuthModule *ibcmock.IBCModule MockModuleV2A mockv2.IBCModule MockModuleV2B mockv2.IBCModule @@ -343,7 +343,6 @@ func NewSimApp( app.ICAControllerKeeper = icacontrollerkeeper.NewKeeper( appCodec, runtime.NewKVStoreService(keys[icacontrollertypes.StoreKey]), app.IBCKeeper.ChannelKeeper, - app.IBCKeeper.ChannelKeeper, app.MsgServiceRouter(), authtypes.NewModuleAddress(govtypes.ModuleName).String(), ) @@ -351,7 +350,6 @@ func NewSimApp( // ICA Host keeper app.ICAHostKeeper = icahostkeeper.NewKeeper( appCodec, runtime.NewKVStoreService(keys[icahosttypes.StoreKey]), - app.IBCKeeper.ChannelKeeper, app.IBCKeeper.ChannelKeeper, app.AccountKeeper, app.MsgServiceRouter(), app.GRPCQueryRouter(), authtypes.NewModuleAddress(govtypes.ModuleName).String(), @@ -363,13 +361,17 @@ func NewSimApp( // Middleware Stacks - app.RateLimitKeeper = ratelimitkeeper.NewKeeper(appCodec, runtime.NewKVStoreService(keys[ratelimittypes.StoreKey]), app.IBCKeeper.ChannelKeeper, app.IBCKeeper.ChannelKeeper, app.IBCKeeper.ClientKeeper, app.BankKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String()) - // PacketForwardMiddleware must be created before TransferKeeper - app.PFMKeeper = packetforwardkeeper.NewKeeper(appCodec, runtime.NewKVStoreService(keys[packetforwardtypes.StoreKey]), nil, app.IBCKeeper.ChannelKeeper, app.BankKeeper, app.IBCKeeper.ChannelKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String()) - - app.TransferKeeper = ibctransferkeeper.NewKeeper(appCodec, runtime.NewKVStoreService(keys[ibctransfertypes.StoreKey]), app.IBCKeeper.ChannelKeeper, app.IBCKeeper.ChannelKeeper, app.MsgServiceRouter(), app.AccountKeeper, app.BankKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String()) + // Create Transfer Keeper + app.TransferKeeper = ibctransferkeeper.NewKeeper( + appCodec, runtime.NewKVStoreService(keys[ibctransfertypes.StoreKey]), + app.IBCKeeper.ChannelKeeper, + app.MsgServiceRouter(), + app.AccountKeeper, app.BankKeeper, + authtypes.NewModuleAddress(govtypes.ModuleName).String(), + ) - app.PFMKeeper.SetTransferKeeper(&app.TransferKeeper) + app.RateLimitKeeper = ratelimitkeeper.NewKeeper(appCodec, runtime.NewKVStoreService(keys[ratelimittypes.StoreKey]), app.IBCKeeper.ChannelKeeper, app.IBCKeeper.ClientKeeper, app.BankKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String()) + app.PFMKeeper = packetforwardkeeper.NewKeeper(appCodec, runtime.NewKVStoreService(keys[packetforwardtypes.StoreKey]), app.TransferKeeper, app.IBCKeeper.ChannelKeeper, app.BankKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String()) // Mock Module Stack @@ -398,23 +400,17 @@ func NewSimApp( // channel.RecvPacket -> transfer.OnRecvPacket // create IBC module from bottom to top of stack - // - Core // - Rate Limit // - Packet Forward Middleware // - Transfer - transferStack := transfer.NewIBCModule(app.TransferKeeper) - transferIBCModule := packetforward.NewIBCMiddleware(transferStack, app.PFMKeeper, 0, packetforwardkeeper.DefaultForwardTransferPacketTimeoutTimestamp) - - // Create the rate-limiting middleware, wrapping the transfer IBC module - // The ICS4Wrapper is the IBC ChannelKeeper - rateLimitMiddleware := ratelimiting.NewIBCMiddleware(transferIBCModule, app.RateLimitKeeper, app.IBCKeeper.ChannelKeeper) - - app.PFMKeeper.SetICS4Wrapper(rateLimitMiddleware) - app.RateLimitKeeper.SetICS4Wrapper(app.IBCKeeper.ChannelKeeper) - app.TransferKeeper.WithICS4Wrapper(app.PFMKeeper) + transferStack := porttypes.NewIBCStackBuilder(app.IBCKeeper.ChannelKeeper) + transferApp := transfer.NewIBCModule(app.TransferKeeper) + transferStack.Base(transferApp). + Next(packetforward.NewIBCMiddleware(app.PFMKeeper, 0, packetforwardkeeper.DefaultForwardTransferPacketTimeoutTimestamp)). + Next(ratelimiting.NewIBCMiddleware(app.RateLimitKeeper)) - // Add transfer stack with rate-limiting middleware to IBC Router - ibcRouter.AddRoute(ibctransfertypes.ModuleName, rateLimitMiddleware) + // Add transfer stack to IBC Router + ibcRouter.AddRoute(ibctransfertypes.ModuleName, transferStack.Build()) // Create Interchain Accounts Stack // SendPacket, since it is originating from the application to core IBC: @@ -424,7 +420,7 @@ func NewSimApp( var icaControllerStack porttypes.IBCModule icaControllerStack = ibcmock.NewIBCModule(&mockModule, ibcmock.NewIBCApp("")) var ok bool - app.ICAAuthModule, ok = icaControllerStack.(ibcmock.IBCModule) + app.ICAAuthModule, ok = icaControllerStack.(*ibcmock.IBCModule) if !ok { panic(fmt.Errorf("cannot convert %T into %T", icaControllerStack, app.ICAAuthModule)) } @@ -490,8 +486,8 @@ func NewSimApp( // IBC modules ibc.NewAppModule(app.IBCKeeper), transfer.NewAppModule(app.TransferKeeper), - ratelimiting.NewAppModule(app.RateLimitKeeper), // Use correct package alias - ica.NewAppModule(&app.ICAControllerKeeper, &app.ICAHostKeeper), + ratelimiting.NewAppModule(app.RateLimitKeeper), + ica.NewAppModule(app.ICAControllerKeeper, app.ICAHostKeeper), mockModule, packetforward.NewAppModule(app.PFMKeeper),