feat: Add GetAppVersion to ICS4Wrapper (#1022)

* feat: Add ICS4Wrapper function GetChannelVersion

Add a function to 04-channel keeper which can be used in the ICS4Wrapper interface for obtaining the unwrapped channel verison

* add docs

* chore: rename GetChannelVersion to GetUnwrappedChannelVersion

* add changelog entry

* Update CHANGELOG.md

* Update docs/ibc/middleware/develop.md

* Update docs/ibc/middleware/develop.md

* chore: GetUnwrappedChannelVersion -> GetAppVersion

* add GetAppVersion for ics29

* add GetAppVersion to ics27

* add extra test for 29-fee

* update docs

* Apply suggestions from code review

Co-authored-by: Sean King <seantking@users.noreply.github.com>

Co-authored-by: Sean King <seantking@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

CHANGELOG.md

@@ -48,6 +48,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvements
 
+* (middleware) [\#1022](https://github.com/cosmos/ibc-go/pull/1022) Add `GetAppVersion` to the ICS4Wrapper interface. This function should be used by IBC applications to obtain their own version since the version set in the channel structure may be wrapped many times by middleware. 
 * (modules/core/04-channel) [\#1160](https://github.com/cosmos/ibc-go/pull/1160) Improve `uint64 -> string` performance in `Logger`.
 * (modules/core/04-channel) [\#1232](https://github.com/cosmos/ibc-go/pull/1232) Updating params on `NewPacketId` and moving to bottom of file.   
 

docs/ibc/middleware/develop.md

@@ -49,6 +49,7 @@ type Middleware interface {
 type ICS4Wrapper interface {
     SendPacket(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet exported.Packet) error
     WriteAcknowledgement(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet exported.Packet, ack []byte) error
+    GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) 
 }
 ```
 
@@ -239,4 +240,24 @@ func SendPacket(appPacket channeltypes.Packet) {
 
     return ics4Keeper.SendPacket(packet)
 }
+
+// middleware must return the underlying application version 
+func GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
+    version, found := ics4Keeper.GetAppVersion(ctx, portID, channelID)
+    if !found {
+        return "", false
+    }
+
+    if !MiddlewareEnabled {
+        return version, true
+    }
+
+    // unwrap channel version
+    metadata, err := Unmarshal(version)
+    if err != nil {
+        panic(fmt.Errof("unable to unmarshal version: %w", err))
+    }
+
+    return metadata.AppVersion, true
+}
 ```

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

@@ -163,3 +163,8 @@ func (im IBCModule) OnTimeoutPacket(
 
 	return im.app.OnTimeoutPacket(ctx, packet, relayer)
 }
+
+// GetAppVersion returns the interchain accounts metadata.
+func (im IBCModule) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
+	return im.keeper.GetAppVersion(ctx, portID, channelID)
+}

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

@@ -9,6 +9,7 @@ import (
 	"github.com/stretchr/testify/suite"
 	"github.com/tendermint/tendermint/crypto"
 
+	icacontroller "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/controller"
 	"github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/controller/types"
 	icatypes "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/types"
 	clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types"
@@ -702,3 +703,23 @@ func (suite *InterchainAccountsTestSuite) TestSingleHostMultipleControllers() {
 		})
 	}
 }
+
+func (suite *InterchainAccountsTestSuite) TestGetAppVersion() {
+	path := NewICAPath(suite.chainA, suite.chainB)
+	suite.coordinator.SetupConnections(path)
+
+	err := SetupICAPath(path, TestOwnerAddress)
+	suite.Require().NoError(err)
+
+	module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID)
+	suite.Require().NoError(err)
+
+	cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module)
+	suite.Require().True(ok)
+
+	controllerModule := cbs.(icacontroller.IBCModule)
+
+	appVersion, found := controllerModule.GetAppVersion(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+	suite.Require().True(found)
+	suite.Require().Equal(path.EndpointA.ChannelConfig.Version, appVersion)
+}

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

@@ -102,6 +102,11 @@ func (k Keeper) ClaimCapability(ctx sdk.Context, cap *capabilitytypes.Capability
 	return k.scopedKeeper.ClaimCapability(ctx, cap, name)
 }
 
+// GetAppVersion calls the ICS4Wrapper GetAppVersion function.
+func (k Keeper) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
+	return k.ics4Wrapper.GetAppVersion(ctx, portID, channelID)
+}
+
 // GetActiveChannelID retrieves the active channelID from the store, keyed by the provided connectionID and portID
 func (k Keeper) GetActiveChannelID(ctx sdk.Context, connectionID, portID string) (string, bool) {
 	store := ctx.KVStore(k.storeKey)

modules/apps/27-interchain-accounts/types/expected_keepers.go

@@ -21,6 +21,7 @@ type AccountKeeper interface {
 // ICS4Wrapper defines the expected ICS4Wrapper for middleware
 type ICS4Wrapper interface {
 	SendPacket(ctx sdk.Context, channelCap *capabilitytypes.Capability, packet ibcexported.PacketI) error
+	GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool)
 }
 
 // ChannelKeeper defines the expected IBC channel keeper

modules/apps/29-fee/ibc_module.go

@@ -267,3 +267,8 @@ func (im IBCModule) OnTimeoutPacket(
 	// call underlying callback
 	return im.app.OnTimeoutPacket(ctx, packet, relayer)
 }
+
+// GetAppVersion returns the application version of the underlying application
+func (im IBCModule) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
+	return im.keeper.GetAppVersion(ctx, portID, channelID)
+}

modules/apps/29-fee/ibc_module_test.go

@@ -6,6 +6,7 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
 
+	fee "github.com/cosmos/ibc-go/v3/modules/apps/29-fee"
 	"github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types"
 	transfertypes "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types"
 	channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
@@ -834,3 +835,78 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {
 		})
 	}
 }
+
+func (suite *FeeTestSuite) TestGetAppVersion() {
+	var (
+		portID        string
+		channelID     string
+		expAppVersion string
+	)
+	testCases := []struct {
+		name     string
+		malleate func()
+		expFound bool
+	}{
+		{
+			"success for fee enabled channel",
+			func() {
+				expAppVersion = ibcmock.Version
+			},
+			true,
+		},
+		{
+			"success for non fee enabled channel",
+			func() {
+				path := ibctesting.NewPath(suite.chainA, suite.chainB)
+				path.EndpointA.ChannelConfig.PortID = ibctesting.MockFeePort
+				path.EndpointB.ChannelConfig.PortID = ibctesting.MockFeePort
+				// by default a new path uses a non fee channel
+				suite.coordinator.Setup(path)
+				portID = path.EndpointA.ChannelConfig.PortID
+				channelID = path.EndpointA.ChannelID
+
+				expAppVersion = ibcmock.Version
+			},
+			true,
+		},
+		{
+			"channel does not exist",
+			func() {
+				channelID = "does not exist"
+			},
+			false,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		suite.Run(tc.name, func() {
+			suite.SetupTest()
+			suite.coordinator.Setup(suite.path)
+
+			portID = suite.path.EndpointA.ChannelConfig.PortID
+			channelID = suite.path.EndpointA.ChannelID
+
+			// malleate test case
+			tc.malleate()
+
+			module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.MockFeePort)
+			suite.Require().NoError(err)
+
+			cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module)
+			suite.Require().True(ok)
+
+			feeModule := cbs.(fee.IBCModule)
+
+			appVersion, found := feeModule.GetAppVersion(suite.chainA.GetContext(), portID, channelID)
+
+			if tc.expFound {
+				suite.Require().True(found)
+				suite.Require().Equal(expAppVersion, appVersion)
+			} else {
+				suite.Require().False(found)
+				suite.Require().Empty(appVersion)
+			}
+		})
+	}
+}

modules/apps/29-fee/keeper/relay.go

@@ -1,6 +1,8 @@
 package keeper
 
 import (
+	"fmt"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
@@ -42,3 +44,22 @@ func (k Keeper) WriteAcknowledgement(ctx sdk.Context, chanCap *capabilitytypes.C
 	// ics4Wrapper may be core IBC or higher-level middleware
 	return k.ics4Wrapper.WriteAcknowledgement(ctx, chanCap, packet, ack)
 }
+
+// GetAppVersion returns the underlying application version.
+func (k Keeper) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
+	version, found := k.ics4Wrapper.GetAppVersion(ctx, portID, channelID)
+	if !found {
+		return "", false
+	}
+
+	if !k.IsFeeEnabled(ctx, portID, channelID) {
+		return version, true
+	}
+
+	var metadata types.Metadata
+	if err := types.ModuleCdc.UnmarshalJSON([]byte(version), &metadata); err != nil {
+		panic(fmt.Errorf("unable to unmarshal metadata for fee enabled channel: %w", err))
+	}
+
+	return metadata.AppVersion, true
+}

modules/apps/29-fee/keeper/relay_test.go

@@ -4,6 +4,8 @@ import (
 	"github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types"
 	clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types"
 	channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
+	ibctesting "github.com/cosmos/ibc-go/v3/testing"
+	ibcmock "github.com/cosmos/ibc-go/v3/testing/mock"
 )
 
 func (suite *KeeperTestSuite) TestWriteAcknowledgementAsync() {
@@ -101,3 +103,70 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgementAsyncFeeDisabled() {
 	packetAck, _ := suite.chainB.GetSimApp().GetIBCKeeper().ChannelKeeper.GetPacketAcknowledgement(suite.chainB.GetContext(), packet.DestinationPort, packet.DestinationChannel, 1)
 	suite.Require().Equal(packetAck, channeltypes.CommitAcknowledgement(ack.Acknowledgement()))
 }
+
+func (suite *KeeperTestSuite) TestGetAppVersion() {
+	var (
+		portID        string
+		channelID     string
+		expAppVersion string
+	)
+	testCases := []struct {
+		name     string
+		malleate func()
+		expFound bool
+	}{
+		{
+			"success for fee enabled channel",
+			func() {
+				expAppVersion = ibcmock.Version
+			},
+			true,
+		},
+		{
+			"success for non fee enabled channel",
+			func() {
+				path := ibctesting.NewPath(suite.chainA, suite.chainB)
+				path.EndpointA.ChannelConfig.PortID = ibctesting.MockFeePort
+				path.EndpointB.ChannelConfig.PortID = ibctesting.MockFeePort
+				// by default a new path uses a non fee channel
+				suite.coordinator.Setup(path)
+				portID = path.EndpointA.ChannelConfig.PortID
+				channelID = path.EndpointA.ChannelID
+
+				expAppVersion = ibcmock.Version
+			},
+			true,
+		},
+		{
+			"channel does not exist",
+			func() {
+				channelID = "does not exist"
+			},
+			false,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		suite.Run(tc.name, func() {
+			suite.SetupTest()
+			suite.coordinator.Setup(suite.path)
+
+			portID = suite.path.EndpointA.ChannelConfig.PortID
+			channelID = suite.path.EndpointA.ChannelID
+
+			// malleate test case
+			tc.malleate()
+
+			appVersion, found := suite.chainA.GetSimApp().IBCFeeKeeper.GetAppVersion(suite.chainA.GetContext(), portID, channelID)
+
+			if tc.expFound {
+				suite.Require().True(found)
+				suite.Require().Equal(expAppVersion, appVersion)
+			} else {
+				suite.Require().False(found)
+				suite.Require().Empty(appVersion)
+			}
+		})
+	}
+}

modules/apps/29-fee/types/expected_keepers.go

@@ -18,6 +18,7 @@ type AccountKeeper interface {
 type ICS4Wrapper interface {
 	WriteAcknowledgement(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI, acknowledgement ibcexported.Acknowledgement) error
 	SendPacket(ctx sdk.Context, channelCap *capabilitytypes.Capability, packet ibcexported.PacketI) error
+	GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool)
 }
 
 // ChannelKeeper defines the expected IBC channel keeper

modules/core/04-channel/keeper/keeper.go

@@ -86,6 +86,16 @@ func (k Keeper) SetChannel(ctx sdk.Context, portID, channelID string, channel ty
 	store.Set(host.ChannelKey(portID, channelID), bz)
 }
 
+// GetAppVersion gets the version for the specified channel.
+func (k Keeper) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
+	channel, found := k.GetChannel(ctx, portID, channelID)
+	if !found {
+		return "", false
+	}
+
+	return channel.Version, true
+}
+
 // GetNextChannelSequence gets the next channel sequence from the store.
 func (k Keeper) GetNextChannelSequence(ctx sdk.Context) uint64 {
 	store := ctx.KVStore(k.storeKey)

modules/core/04-channel/keeper/keeper_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
 	ibctesting "github.com/cosmos/ibc-go/v3/testing"
+	ibcmock "github.com/cosmos/ibc-go/v3/testing/mock"
 )
 
 // KeeperTestSuite is a testing suite to test keeper functions.
@@ -62,6 +63,24 @@ func (suite *KeeperTestSuite) TestSetChannel() {
 	suite.Equal(expectedCounterparty, storedChannel.Counterparty)
 }
 
+func (suite *KeeperTestSuite) TestGetAppVersion() {
+	// create client and connections on both chains
+	path := ibctesting.NewPath(suite.chainA, suite.chainB)
+	suite.coordinator.SetupConnections(path)
+
+	version, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetAppVersion(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+	suite.Require().False(found)
+	suite.Require().Empty(version)
+
+	// init channel
+	err := path.EndpointA.ChanOpenInit()
+	suite.NoError(err)
+
+	channelVersion, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetAppVersion(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+	suite.Require().True(found)
+	suite.Require().Equal(ibcmock.Version, channelVersion)
+}
+
 // TestGetAllChannels creates multiple channels on chain A through various connections
 // and tests their retrieval. 2 channels are on connA0 and 1 channel is on connA1
 func (suite KeeperTestSuite) TestGetAllChannels() {

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

@@ -113,6 +113,12 @@ type ICS4Wrapper interface {
 		packet exported.PacketI,
 		ack exported.Acknowledgement,
 	) error
+
+	GetAppVersion(
+		ctx sdk.Context,
+		portID,
+		channelID string,
+	) (string, bool)
 }
 
 // Middleware must implement IBCModule to wrap communication from core IBC to underlying application

testing/endpoint.go

@@ -328,7 +328,14 @@ func (endpoint *Endpoint) ChanOpenAck() error {
 		proof, height,
 		endpoint.Chain.SenderAccount.GetAddress().String(),
 	)
-	return endpoint.Chain.sendMsgs(msg)
+
+	if err = endpoint.Chain.sendMsgs(msg); err != nil {
+		return err
+	}
+
+	endpoint.ChannelConfig.Version = endpoint.GetChannel().Version
+
+	return nil
 }
 
 // ChanOpenConfirm will construct and execute a MsgChannelOpenConfirm on the associated endpoint.