diff --git a/CHANGELOG.md b/CHANGELOG.md index 8da21edad21..a72dc61c05a 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/docs/ibc/middleware/develop.md b/docs/ibc/middleware/develop.md index 1d75e3965a8..705040b1db7 100644 --- a/docs/ibc/middleware/develop.md +++ b/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 +} ``` diff --git a/modules/apps/27-interchain-accounts/controller/ibc_module.go b/modules/apps/27-interchain-accounts/controller/ibc_module.go index c00c9f5d1c2..325ec70959b 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_module.go +++ b/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) +} diff --git a/modules/apps/27-interchain-accounts/controller/ibc_module_test.go b/modules/apps/27-interchain-accounts/controller/ibc_module_test.go index c0163e954e1..608996515c1 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_module_test.go +++ b/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) +} diff --git a/modules/apps/27-interchain-accounts/controller/keeper/keeper.go b/modules/apps/27-interchain-accounts/controller/keeper/keeper.go index 87af9ae9c6f..86099b954bd 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/keeper.go +++ b/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) diff --git a/modules/apps/27-interchain-accounts/types/expected_keepers.go b/modules/apps/27-interchain-accounts/types/expected_keepers.go index 4c6a1708e43..8fb0b743cda 100644 --- a/modules/apps/27-interchain-accounts/types/expected_keepers.go +++ b/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 diff --git a/modules/apps/29-fee/ibc_module.go b/modules/apps/29-fee/ibc_module.go index e89b95cb4de..6ed9faa1f34 100644 --- a/modules/apps/29-fee/ibc_module.go +++ b/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) +} diff --git a/modules/apps/29-fee/ibc_module_test.go b/modules/apps/29-fee/ibc_module_test.go index cb6a89d13b7..3f5cf64635a 100644 --- a/modules/apps/29-fee/ibc_module_test.go +++ b/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) + } + }) + } +} diff --git a/modules/apps/29-fee/keeper/relay.go b/modules/apps/29-fee/keeper/relay.go index 34c473e2e83..476497bfbbb 100644 --- a/modules/apps/29-fee/keeper/relay.go +++ b/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 +} diff --git a/modules/apps/29-fee/keeper/relay_test.go b/modules/apps/29-fee/keeper/relay_test.go index c853babd527..d0a9e620f9d 100644 --- a/modules/apps/29-fee/keeper/relay_test.go +++ b/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) + } + }) + } +} diff --git a/modules/apps/29-fee/types/expected_keepers.go b/modules/apps/29-fee/types/expected_keepers.go index 3a58f0706b3..1d9d3439b07 100644 --- a/modules/apps/29-fee/types/expected_keepers.go +++ b/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 diff --git a/modules/core/04-channel/keeper/keeper.go b/modules/core/04-channel/keeper/keeper.go index 65378039ad9..ca2f824ad48 100644 --- a/modules/core/04-channel/keeper/keeper.go +++ b/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) diff --git a/modules/core/04-channel/keeper/keeper_test.go b/modules/core/04-channel/keeper/keeper_test.go index 60888f11c3c..f04664d71f4 100644 --- a/modules/core/04-channel/keeper/keeper_test.go +++ b/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() { diff --git a/modules/core/05-port/types/module.go b/modules/core/05-port/types/module.go index 9f754fe0a3e..e6ba8f3449b 100644 --- a/modules/core/05-port/types/module.go +++ b/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 diff --git a/testing/endpoint.go b/testing/endpoint.go index 607f7a16843..02c4e9aac39 100644 --- a/testing/endpoint.go +++ b/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.