Adding updates to composite router and scaffolding for LegacyIBCModule

modules/apps/callbacks/ibc_middleware_test.go

@@ -169,7 +169,14 @@ func (s *CallbacksTestSuite) TestSendPacket() {
 			cbs, ok := GetSimApp(s.chainA).IBCKeeper.PortKeeper.AppRoute(transfertypes.ModuleName)
 			s.Require().True(ok)
 
-			callbacksModule, ok := cbs[1].(ibccallbacks.IBCMiddleware) // callbacks module is routed second
+			s.Require().Len(cbs, 1, "expected 1 legacy module")
+
+			legacyModule, ok := cbs[0].(porttypes.LegacyIBCModule)
+			s.Require().True(ok, "expected there to be a single legacy ibc module")
+
+			legacyModuleCbs := legacyModule.GetCallbacks()
+
+			callbacksModule, ok := legacyModuleCbs[1].(ibccallbacks.IBCMiddleware) // callbacks module is routed second
 			s.Require().True(ok)
 
 			packetData = transfertypes.NewFungibleTokenPacketDataV2(

modules/core/05-port/keeper/keeper.go

@@ -2,7 +2,6 @@ package keeper
 
 import (
 	"fmt"
-	"strings"
 
 	"cosmossdk.io/log"
 
@@ -105,20 +104,5 @@ func (k *Keeper) Route(module string) (types.IBCModule, bool) {
 // AppRoute returns an ordered list of IBCModule callbacks for a given module name, and a boolean indicating
 // whether or not the callbacks are present.
 func (k *Keeper) AppRoute(module string) ([]types.IBCModule, bool) {
-	routes, ok := k.AppRouter.GetRoute(module)
-	if ok {
-		return routes, true
-	}
-
-	for _, prefix := range k.Keys() {
-		if strings.Contains(module, prefix) {
-			return k.AppRouter.GetRoute(prefix)
-		}
-	}
-
-	return nil, false
-}
-
-func (k *Keeper) Keys() []string {
-	return k.AppRouter.Keys()
+	return k.AppRouter.PacketRoute(module)
 }

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

@@ -0,0 +1,175 @@
+package types
+
+import (
+	errorsmod "cosmossdk.io/errors"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
+	clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
+	channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
+	ibcexported "github.com/cosmos/ibc-go/v8/modules/core/exported"
+)
+
+// TODO: IBCModule will be renamed to ClassicIBCModule in a follow up.
+type ClassicIBCModule IBCModule
+
+// LegacyIBCModule implements the ICS26 interface for transfer given the transfer keeper.
+type LegacyIBCModule struct {
+	cbs []ClassicIBCModule
+}
+
+// TODO: added this for testing purposes, we can remove later if tests are refactored.
+func (im *LegacyIBCModule) GetCallbacks() []ClassicIBCModule {
+	return im.cbs
+}
+
+// NewLegacyIBCModule creates a new IBCModule given the keeper
+func NewLegacyIBCModule(cbs ...ClassicIBCModule) ClassicIBCModule {
+	return LegacyIBCModule{
+		cbs: cbs,
+	}
+}
+
+// OnChanOpenInit implements the IBCModule interface
+func (LegacyIBCModule) OnChanOpenInit(
+	ctx sdk.Context,
+	order channeltypes.Order,
+	connectionHops []string,
+	portID string,
+	channelID string,
+	chanCap *capabilitytypes.Capability,
+	counterparty channeltypes.Counterparty,
+	version string,
+) (string, error) {
+	return "", nil
+}
+
+// OnChanOpenTry implements the IBCModule interface.
+func (LegacyIBCModule) OnChanOpenTry(
+	ctx sdk.Context,
+	order channeltypes.Order,
+	connectionHops []string,
+	portID,
+	channelID string,
+	chanCap *capabilitytypes.Capability,
+	counterparty channeltypes.Counterparty,
+	counterpartyVersion string,
+) (string, error) {
+	return "", nil
+}
+
+// OnChanOpenAck implements the IBCModule interface
+func (LegacyIBCModule) OnChanOpenAck(
+	ctx sdk.Context,
+	portID,
+	channelID string,
+	counterpartyChannelID string,
+	counterpartyVersion string,
+) error {
+	return nil
+}
+
+// OnChanOpenConfirm implements the IBCModule interface
+func (LegacyIBCModule) OnChanOpenConfirm(
+	ctx sdk.Context,
+	portID,
+	channelID string,
+) error {
+	return nil
+}
+
+// OnChanCloseInit implements the IBCModule interface
+func (LegacyIBCModule) OnChanCloseInit(
+	ctx sdk.Context,
+	portID,
+	channelID string,
+) error {
+	return nil
+}
+
+// OnChanCloseConfirm implements the IBCModule interface
+func (LegacyIBCModule) OnChanCloseConfirm(
+	ctx sdk.Context,
+	portID,
+	channelID string,
+) error {
+	return nil
+}
+
+// OnSendPacket implements the IBCModule interface.
+func (im LegacyIBCModule) OnSendPacket(
+	ctx sdk.Context,
+	portID string,
+	channelID string,
+	sequence uint64,
+	timeoutHeight clienttypes.Height,
+	timeoutTimestamp uint64,
+	dataBz []byte,
+	signer sdk.AccAddress,
+) error {
+	// to maintain backwards compatibility, OnSendPacket iterates over the callbacks in order, as they are wired from bottom to top of the stack.
+	for _, cb := range im.cbs {
+		if err := cb.OnSendPacket(ctx, portID, channelID, sequence, timeoutHeight, timeoutTimestamp, dataBz, signer); err != nil {
+			return errorsmod.Wrapf(err, "send packet callback failed for portID %s channelID %s", portID, channelID)
+		}
+	}
+	return nil
+}
+
+// OnRecvPacket implements the IBCModule interface. A successful acknowledgement
+// is returned if the packet data is successfully decoded and the receive application
+// logic returns without error.
+// A nil acknowledgement may be returned when using the packet forwarding feature. This signals to core IBC that the acknowledgement will be written asynchronously.
+func (LegacyIBCModule) OnRecvPacket(
+	ctx sdk.Context,
+	packet channeltypes.Packet,
+	relayer sdk.AccAddress,
+) ibcexported.Acknowledgement {
+	return nil
+}
+
+// OnAcknowledgementPacket implements the IBCModule interface
+func (LegacyIBCModule) OnAcknowledgementPacket(
+	ctx sdk.Context,
+	packet channeltypes.Packet,
+	acknowledgement []byte,
+	relayer sdk.AccAddress,
+) error {
+	return nil
+}
+
+// OnTimeoutPacket implements the IBCModule interface
+func (LegacyIBCModule) OnTimeoutPacket(
+	ctx sdk.Context,
+	packet channeltypes.Packet,
+	relayer sdk.AccAddress,
+) error {
+	return nil
+}
+
+// OnChanUpgradeInit implements the IBCModule interface
+func (LegacyIBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, proposedVersion string) (string, error) {
+	return "", nil
+}
+
+// OnChanUpgradeTry implements the IBCModule interface
+func (LegacyIBCModule) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, counterpartyVersion string) (string, error) {
+	return "", nil
+}
+
+// OnChanUpgradeAck implements the IBCModule interface
+func (LegacyIBCModule) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpartyVersion string) error {
+	return nil
+}
+
+// OnChanUpgradeOpen implements the IBCModule interface
+func (LegacyIBCModule) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, proposedVersion string) {
+}
+
+// UnmarshalPacketData attempts to unmarshal the provided packet data bytes
+// into a FungibleTokenPacketData. This function implements the optional
+// PacketDataUnmarshaler interface required for ADR 008 support.
+func (LegacyIBCModule) UnmarshalPacketData(ctx sdk.Context, portID, channelID string, bz []byte) (interface{}, error) {
+	return nil, nil
+}

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

@@ -5,67 +5,115 @@ package types
 import (
 	"errors"
 	"fmt"
-
-	"golang.org/x/exp/maps"
+	"strings"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
-// AppRouter contains a map from module name to an ordered list of IBCModules
-// which contains all the module-defined callbacks required by ICS-26.
+// TODO: this is a temporary constant that is subject to change based on the final spec.
+const sentinelMultiPacketData = "MultiPacketData"
+
+// AppRouter contains all the module-defined callbacks required by ICS-26
 type AppRouter struct {
-	routes map[string][]IBCModule
-	sealed bool
+	routes       map[string]IBCModule
+	legacyRoutes map[string]ClassicIBCModule
+
+	// classicRoutes facilitates the consecutive calls to AddRoute for existing modules.
+	// TODO: this should be removed once app.gos have been refactored to use AddClassicRoute.
+	classicRoutes map[string][]ClassicIBCModule
 }
 
-// NewAppRouter creates and returns a new IBCModule application router.
 func NewAppRouter() *AppRouter {
 	return &AppRouter{
-		routes: make(map[string][]IBCModule),
+		routes:        make(map[string]IBCModule),
+		legacyRoutes:  make(map[string]ClassicIBCModule),
+		classicRoutes: make(map[string][]ClassicIBCModule),
 	}
 }
 
-// Seal prevents the Router from any subsequent route handlers to be registered.
-// Seal will panic if called more than once.
-func (rtr *AppRouter) Seal() {
-	if rtr.sealed {
-		panic(errors.New("router already sealed"))
+// AddClassicRoute takes a ordered list of ClassicIBCModules and creates a LegacyIBCModule. This is then added
+// to the legacy mapping.
+func (rtr *AppRouter) AddClassicRoute(module string, cbs ...ClassicIBCModule) *AppRouter {
+	if !sdk.IsAlphaNumeric(module) {
+		panic(errors.New("route expressions can only contain alphanumeric characters"))
 	}
-	rtr.sealed = true
-}
 
-// Sealed returns a boolean signifying if the Router is sealed or not.
-func (rtr AppRouter) Sealed() bool {
-	return rtr.sealed
+	if _, ok := rtr.legacyRoutes[module]; ok {
+		panic(fmt.Errorf("route %s has already been registered", module))
+	}
+
+	if len(cbs) == 0 {
+		panic(errors.New("no callbacks provided"))
+	}
+
+	rtr.legacyRoutes[module] = NewLegacyIBCModule(cbs...)
+
+	return rtr
 }
 
 // AddRoute adds IBCModule for a given module name. It returns the Router
 // so AddRoute calls can be linked. It will panic if the Router is sealed.
 func (rtr *AppRouter) AddRoute(module string, cbs IBCModule) *AppRouter {
-	if rtr.sealed {
-		panic(fmt.Errorf("router sealed; cannot register %s route callbacks", module))
-	}
 	if !sdk.IsAlphaNumeric(module) {
 		panic(errors.New("route expressions can only contain alphanumeric characters"))
 	}
-	rtr.routes[module] = append(rtr.routes[module], cbs)
+
+	if _, ok := cbs.(ClassicIBCModule); ok {
+		rtr.classicRoutes[module] = append(rtr.classicRoutes[module], cbs)
+
+		// in order to facilitate having a single LegacyIBCModule, but also allowing for
+		// consecutive calls to AddRoute to support existing functionality, we can re-create
+		// the legacy module with the routes as they get added.
+		if classicRoutes, ok := rtr.classicRoutes[module]; ok && len(classicRoutes) > 1 {
+			rtr.legacyRoutes[module] = NewLegacyIBCModule(classicRoutes...)
+		}
+	} else {
+		rtr.routes[module] = cbs
+	}
+
 	return rtr
 }
 
-// HasRoute returns true if the Router has a module registered or false otherwise.
-func (rtr *AppRouter) HasRoute(module string) bool {
-	_, ok := rtr.routes[module]
-	return ok
+func (rtr *AppRouter) PacketRoute(module string) ([]IBCModule, bool) {
+	if module == sentinelMultiPacketData {
+		return rtr.routeMultiPacketData(module)
+	}
+	legacyModule, ok := rtr.routeToLegacyModule(module)
+	if !ok {
+		return nil, false
+	}
+	return []IBCModule{legacyModule}, true
+}
+
+// TODO: docstring once implementation is complete
+func (*AppRouter) routeMultiPacketData(module string) ([]IBCModule, bool) {
+	panic("unimplemented")
+	//  for _, pd := range packet.Data {
+	//      cbs = append(cbs, rtr.routes[pd.PortId])
+	//  }
+	// return cbs, true
 }
 
-// GetRoute returns a IBCModule for a given module.
-func (rtr *AppRouter) GetRoute(module string) ([]IBCModule, bool) {
-	if !rtr.HasRoute(module) {
-		return nil, false
+// routeToLegacyModule routes to any legacy modules which have been registered with AddClassicRoute.
+func (rtr *AppRouter) routeToLegacyModule(module string) (IBCModule, bool) {
+	route, ok := rtr.legacyRoutes[module]
+	if ok {
+		return route, true
+	}
+
+	// it's possible that some routes have been dynamically added e.g. with interchain accounts.
+	// in this case, we need to check if the module has the specified prefix.
+	for prefix := range rtr.legacyRoutes {
+		if strings.Contains(module, prefix) {
+			return rtr.legacyRoutes[prefix], true
+		}
 	}
-	return rtr.routes[module], true
+	return nil, false
 }
 
-func (rtr *AppRouter) Keys() []string {
-	return maps.Keys(rtr.routes)
+// HandshakeRoute returns the ClassicIBCModule which will implement all handshake functions
+// and is required only for those callbacks.
+func (rtr *AppRouter) HandshakeRoute(portID string) (ClassicIBCModule, bool) {
+	route, ok := rtr.routeToLegacyModule(portID)
+	return route, ok
 }

modules/core/keeper/keeper.go

@@ -92,12 +92,7 @@ func (k *Keeper) SetConsensusHost(consensusHost clienttypes.ConsensusHost) {
 // SetAppRouter sets the Router in IBC Keeper and seals it. The method panics if
 // there is an existing router that's already sealed.
 func (k *Keeper) SetAppRouter(rtr *porttypes.AppRouter) {
-	if k.PortKeeper.AppRouter != nil && k.PortKeeper.AppRouter.Sealed() {
-		panic(errors.New("cannot reset a sealed router"))
-	}
-
 	k.PortKeeper.AppRouter = rtr
-	k.PortKeeper.AppRouter.Seal()
 }
 
 // SetRouter sets the Router in IBC Keeper and seals it. The method panics if