From d582fee665b91c3d94195cddf41e0976eecbf1ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Mon, 28 Feb 2022 11:44:00 +0100 Subject: [PATCH 01/13] 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 --- modules/core/04-channel/keeper/keeper.go | 10 ++++++++++ modules/core/04-channel/keeper/keeper_test.go | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/modules/core/04-channel/keeper/keeper.go b/modules/core/04-channel/keeper/keeper.go index 65378039ad9..5c3fcf74d5a 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) } +// GetVersion gets the version for the specified channel. +func (k Keeper) GetChannelVersion(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..d7257416263 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) TestGetChannelVersion() { + // 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.GetChannelVersion(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.GetChannelVersion(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() { From a40b0125fa91490c8814a185ad7395887eaafc08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Mon, 28 Feb 2022 11:50:31 +0100 Subject: [PATCH 02/13] add docs --- docs/ibc/middleware/develop.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docs/ibc/middleware/develop.md b/docs/ibc/middleware/develop.md index 1d75e3965a8..be0c0c2890d 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 + GetChannelVersion(ctx sdk.Context, portID, channelID string) (string, bool) } ``` @@ -239,4 +240,21 @@ func SendPacket(appPacket channeltypes.Packet) { return ics4Keeper.SendPacket(packet) } + +// middleware must unwrap the channel version so the underlying application +// which calls this function receives the version it constructed. +func GetUnwrappedChannelVersion(ctx sdk.Context, portID, channelID string) (string, bool) { + version, found := ics4Keeper.GetChannelVersion(ctx, portID, channelID) + if !found { + return "", false + } + + // unwrap channel version + metadata, err := Unmarshal(version) + if err != nil { + panic() + } + + return metadata.AppVersion, true +} ``` From 816234c14035833dea78942888c7c21625c57733 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Mon, 28 Feb 2022 11:52:10 +0100 Subject: [PATCH 03/13] chore: rename GetChannelVersion to GetUnwrappedChannelVersion --- modules/core/04-channel/keeper/keeper.go | 4 ++-- modules/core/04-channel/keeper/keeper_test.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/modules/core/04-channel/keeper/keeper.go b/modules/core/04-channel/keeper/keeper.go index 5c3fcf74d5a..5ca625a380b 100644 --- a/modules/core/04-channel/keeper/keeper.go +++ b/modules/core/04-channel/keeper/keeper.go @@ -86,8 +86,8 @@ func (k Keeper) SetChannel(ctx sdk.Context, portID, channelID string, channel ty store.Set(host.ChannelKey(portID, channelID), bz) } -// GetVersion gets the version for the specified channel. -func (k Keeper) GetChannelVersion(ctx sdk.Context, portID, channelID string) (string, bool) { +// GetUnwrappedChannelVersion gets the version for the specified channel. +func (k Keeper) GetUnwrappedChannelVersion(ctx sdk.Context, portID, channelID string) (string, bool) { channel, found := k.GetChannel(ctx, portID, channelID) if !found { return "", false diff --git a/modules/core/04-channel/keeper/keeper_test.go b/modules/core/04-channel/keeper/keeper_test.go index d7257416263..314ca606dcd 100644 --- a/modules/core/04-channel/keeper/keeper_test.go +++ b/modules/core/04-channel/keeper/keeper_test.go @@ -63,12 +63,12 @@ func (suite *KeeperTestSuite) TestSetChannel() { suite.Equal(expectedCounterparty, storedChannel.Counterparty) } -func (suite *KeeperTestSuite) TestGetChannelVersion() { +func (suite *KeeperTestSuite) TestGetUnwrappedChannelVersion() { // 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.GetChannelVersion(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + version, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetUnwrappedChannelVersion(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().False(found) suite.Require().Empty(version) @@ -76,7 +76,7 @@ func (suite *KeeperTestSuite) TestGetChannelVersion() { err := path.EndpointA.ChanOpenInit() suite.NoError(err) - channelVersion, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetChannelVersion(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + channelVersion, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetUnwrappedChannelVersion(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().True(found) suite.Require().Equal(ibcmock.Version, channelVersion) } From b10307fc7b3d5f46aa319ebd5db0d2498053b7a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Mon, 28 Feb 2022 11:56:25 +0100 Subject: [PATCH 04/13] add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4508636a8c3..6ff5ad5cd58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* (middleware) [\#1022](https://github.com/cosmos/ibc-go/pull/1022) Add `GetUnwrappedChannelVersion` 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 but middleware. * (testing) [\#942](https://github.com/cosmos/ibc-go/pull/942) `NewTestChain` will create 4 validators in validator set by default. A new constructor function `NewTestChainWithValSet` is provided for test writers who want custom control over the validator set of test chains. * (testing) [\#904](https://github.com/cosmos/ibc-go/pull/904) Add `ParsePacketFromEvents` function to the testing package. Useful when sending/relaying packets via the testing package. * (testing) [\#893](https://github.com/cosmos/ibc-go/pull/893) Support custom private keys for testing. From 7e6bceb5e4c09c7653d0f6a01e062cabf1ebfdc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Mon, 28 Feb 2022 11:57:00 +0100 Subject: [PATCH 05/13] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ff5ad5cd58..1550eadf73d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,7 +63,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements -* (middleware) [\#1022](https://github.com/cosmos/ibc-go/pull/1022) Add `GetUnwrappedChannelVersion` 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 but middleware. +* (middleware) [\#1022](https://github.com/cosmos/ibc-go/pull/1022) Add `GetUnwrappedChannelVersion` 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. * (testing) [\#942](https://github.com/cosmos/ibc-go/pull/942) `NewTestChain` will create 4 validators in validator set by default. A new constructor function `NewTestChainWithValSet` is provided for test writers who want custom control over the validator set of test chains. * (testing) [\#904](https://github.com/cosmos/ibc-go/pull/904) Add `ParsePacketFromEvents` function to the testing package. Useful when sending/relaying packets via the testing package. * (testing) [\#893](https://github.com/cosmos/ibc-go/pull/893) Support custom private keys for testing. From cdeb1975bb99b22e5a241b0f8184c5bd3b43f7e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Mon, 28 Feb 2022 11:57:18 +0100 Subject: [PATCH 06/13] Update docs/ibc/middleware/develop.md --- docs/ibc/middleware/develop.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/ibc/middleware/develop.md b/docs/ibc/middleware/develop.md index be0c0c2890d..d9144118023 100644 --- a/docs/ibc/middleware/develop.md +++ b/docs/ibc/middleware/develop.md @@ -49,7 +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 - GetChannelVersion(ctx sdk.Context, portID, channelID string) (string, bool) + GetUnwrappedChannelVersion(ctx sdk.Context, portID, channelID string) (string, bool) } ``` From aedf90089be65ee5797ee19d3147123688a931c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Mon, 28 Feb 2022 11:57:33 +0100 Subject: [PATCH 07/13] Update docs/ibc/middleware/develop.md --- docs/ibc/middleware/develop.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/ibc/middleware/develop.md b/docs/ibc/middleware/develop.md index d9144118023..723225e159d 100644 --- a/docs/ibc/middleware/develop.md +++ b/docs/ibc/middleware/develop.md @@ -244,7 +244,7 @@ func SendPacket(appPacket channeltypes.Packet) { // middleware must unwrap the channel version so the underlying application // which calls this function receives the version it constructed. func GetUnwrappedChannelVersion(ctx sdk.Context, portID, channelID string) (string, bool) { - version, found := ics4Keeper.GetChannelVersion(ctx, portID, channelID) + version, found := ics4Keeper.GetUnwrappedChannelVersion(ctx, portID, channelID) if !found { return "", false } From fc3946e3ab9e40f189a4ca82cf7260efc5d80ffb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 14 Apr 2022 12:55:49 +0200 Subject: [PATCH 08/13] chore: GetUnwrappedChannelVersion -> GetAppVersion --- CHANGELOG.md | 2 +- docs/ibc/middleware/develop.md | 6 +++--- modules/core/04-channel/keeper/keeper.go | 4 ++-- modules/core/04-channel/keeper/keeper_test.go | 6 +++--- modules/core/05-port/types/module.go | 6 ++++++ 5 files changed, 15 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ecdbc3160ad..11039f1c302 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,7 +46,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements -* (middleware) [\#1022](https://github.com/cosmos/ibc-go/pull/1022) Add `GetUnwrappedChannelVersion` 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. +* (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 723225e159d..f62c0230347 100644 --- a/docs/ibc/middleware/develop.md +++ b/docs/ibc/middleware/develop.md @@ -49,7 +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 - GetUnwrappedChannelVersion(ctx sdk.Context, portID, channelID string) (string, bool) + GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) } ``` @@ -243,8 +243,8 @@ func SendPacket(appPacket channeltypes.Packet) { // middleware must unwrap the channel version so the underlying application // which calls this function receives the version it constructed. -func GetUnwrappedChannelVersion(ctx sdk.Context, portID, channelID string) (string, bool) { - version, found := ics4Keeper.GetUnwrappedChannelVersion(ctx, portID, channelID) +func GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) { + version, found := ics4Keeper.GetAppVersion(ctx, portID, channelID) if !found { return "", false } diff --git a/modules/core/04-channel/keeper/keeper.go b/modules/core/04-channel/keeper/keeper.go index 5ca625a380b..ca2f824ad48 100644 --- a/modules/core/04-channel/keeper/keeper.go +++ b/modules/core/04-channel/keeper/keeper.go @@ -86,8 +86,8 @@ func (k Keeper) SetChannel(ctx sdk.Context, portID, channelID string, channel ty store.Set(host.ChannelKey(portID, channelID), bz) } -// GetUnwrappedChannelVersion gets the version for the specified channel. -func (k Keeper) GetUnwrappedChannelVersion(ctx sdk.Context, portID, channelID string) (string, bool) { +// 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 diff --git a/modules/core/04-channel/keeper/keeper_test.go b/modules/core/04-channel/keeper/keeper_test.go index 314ca606dcd..f04664d71f4 100644 --- a/modules/core/04-channel/keeper/keeper_test.go +++ b/modules/core/04-channel/keeper/keeper_test.go @@ -63,12 +63,12 @@ func (suite *KeeperTestSuite) TestSetChannel() { suite.Equal(expectedCounterparty, storedChannel.Counterparty) } -func (suite *KeeperTestSuite) TestGetUnwrappedChannelVersion() { +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.GetUnwrappedChannelVersion(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + 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) @@ -76,7 +76,7 @@ func (suite *KeeperTestSuite) TestGetUnwrappedChannelVersion() { err := path.EndpointA.ChanOpenInit() suite.NoError(err) - channelVersion, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetUnwrappedChannelVersion(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + 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) } 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 From fa1de732d87150e5ef4f47e27070192e13da851a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 14 Apr 2022 13:41:37 +0200 Subject: [PATCH 09/13] add GetAppVersion for ics29 --- modules/apps/29-fee/ibc_module.go | 5 ++ modules/apps/29-fee/keeper/keeper.go | 5 +- modules/apps/29-fee/keeper/relay.go | 21 +++++++ modules/apps/29-fee/keeper/relay_test.go | 70 ++++++++++++++++++++++++ 4 files changed, 99 insertions(+), 2 deletions(-) diff --git a/modules/apps/29-fee/ibc_module.go b/modules/apps/29-fee/ibc_module.go index c542c5999e6..21186e4ecd8 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/keeper/keeper.go b/modules/apps/29-fee/keeper/keeper.go index e0317d3660a..cccd40acfc6 100644 --- a/modules/apps/29-fee/keeper/keeper.go +++ b/modules/apps/29-fee/keeper/keeper.go @@ -9,6 +9,7 @@ import ( "github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types" channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" + porttypes "github.com/cosmos/ibc-go/v3/modules/core/05-port/types" host "github.com/cosmos/ibc-go/v3/modules/core/24-host" ) @@ -25,7 +26,7 @@ type Keeper struct { cdc codec.BinaryCodec authKeeper types.AccountKeeper - ics4Wrapper types.ICS4Wrapper + ics4Wrapper porttypes.ICS4Wrapper channelKeeper types.ChannelKeeper portKeeper types.PortKeeper bankKeeper types.BankKeeper @@ -34,7 +35,7 @@ type Keeper struct { // NewKeeper creates a new 29-fee Keeper instance func NewKeeper( cdc codec.BinaryCodec, key sdk.StoreKey, paramSpace paramtypes.Subspace, - ics4Wrapper types.ICS4Wrapper, channelKeeper types.ChannelKeeper, portKeeper types.PortKeeper, authKeeper types.AccountKeeper, bankKeeper types.BankKeeper, + ics4Wrapper porttypes.ICS4Wrapper, channelKeeper types.ChannelKeeper, portKeeper types.PortKeeper, authKeeper types.AccountKeeper, bankKeeper types.BankKeeper, ) Keeper { return Keeper{ 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..f615015d7c7 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,71 @@ 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() { + portID = "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) + } + }) + } +} From 1f64e70f29bbecc7d4248947de226e4e20dc741f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 14 Apr 2022 14:04:55 +0200 Subject: [PATCH 10/13] add GetAppVersion to ics27 --- .../controller/ibc_module.go | 5 +++++ .../controller/ibc_module_test.go | 21 +++++++++++++++++++ .../controller/keeper/keeper.go | 5 +++++ .../types/expected_keepers.go | 1 + modules/apps/29-fee/keeper/keeper.go | 5 ++--- modules/apps/29-fee/types/expected_keepers.go | 1 + testing/endpoint.go | 9 +++++++- 7 files changed, 43 insertions(+), 4 deletions(-) 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/keeper/keeper.go b/modules/apps/29-fee/keeper/keeper.go index cccd40acfc6..e0317d3660a 100644 --- a/modules/apps/29-fee/keeper/keeper.go +++ b/modules/apps/29-fee/keeper/keeper.go @@ -9,7 +9,6 @@ import ( "github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types" channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" - porttypes "github.com/cosmos/ibc-go/v3/modules/core/05-port/types" host "github.com/cosmos/ibc-go/v3/modules/core/24-host" ) @@ -26,7 +25,7 @@ type Keeper struct { cdc codec.BinaryCodec authKeeper types.AccountKeeper - ics4Wrapper porttypes.ICS4Wrapper + ics4Wrapper types.ICS4Wrapper channelKeeper types.ChannelKeeper portKeeper types.PortKeeper bankKeeper types.BankKeeper @@ -35,7 +34,7 @@ type Keeper struct { // NewKeeper creates a new 29-fee Keeper instance func NewKeeper( cdc codec.BinaryCodec, key sdk.StoreKey, paramSpace paramtypes.Subspace, - ics4Wrapper porttypes.ICS4Wrapper, channelKeeper types.ChannelKeeper, portKeeper types.PortKeeper, authKeeper types.AccountKeeper, bankKeeper types.BankKeeper, + ics4Wrapper types.ICS4Wrapper, channelKeeper types.ChannelKeeper, portKeeper types.PortKeeper, authKeeper types.AccountKeeper, bankKeeper types.BankKeeper, ) Keeper { return Keeper{ 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/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. From 5d4544d2dcea74d378c8f2192fc2411a47a1a6b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 14 Apr 2022 14:07:27 +0200 Subject: [PATCH 11/13] add extra test for 29-fee --- modules/apps/29-fee/ibc_module_test.go | 77 ++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/modules/apps/29-fee/ibc_module_test.go b/modules/apps/29-fee/ibc_module_test.go index 1e5743f7490..b8fa70c792c 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" @@ -825,3 +826,79 @@ 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() { + portID = "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) + } + }) + } +} From b114e45741a1e8f92e7a502681ccd4081ccdf25e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 14 Apr 2022 14:09:14 +0200 Subject: [PATCH 12/13] update docs --- docs/ibc/middleware/develop.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/ibc/middleware/develop.md b/docs/ibc/middleware/develop.md index f62c0230347..705040b1db7 100644 --- a/docs/ibc/middleware/develop.md +++ b/docs/ibc/middleware/develop.md @@ -241,18 +241,21 @@ func SendPacket(appPacket channeltypes.Packet) { return ics4Keeper.SendPacket(packet) } -// middleware must unwrap the channel version so the underlying application -// which calls this function receives the version it constructed. +// 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() + panic(fmt.Errof("unable to unmarshal version: %w", err)) } return metadata.AppVersion, true From feb63aa16ee5cfd768f360fb8d1c53e9489f4323 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 14 Apr 2022 16:35:47 +0200 Subject: [PATCH 13/13] Apply suggestions from code review Co-authored-by: Sean King --- modules/apps/29-fee/ibc_module_test.go | 3 +-- modules/apps/29-fee/keeper/relay_test.go | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/modules/apps/29-fee/ibc_module_test.go b/modules/apps/29-fee/ibc_module_test.go index b8fa70c792c..640e8024f04 100644 --- a/modules/apps/29-fee/ibc_module_test.go +++ b/modules/apps/29-fee/ibc_module_test.go @@ -852,7 +852,6 @@ func (suite *FeeTestSuite) TestGetAppVersion() { 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 @@ -864,7 +863,7 @@ func (suite *FeeTestSuite) TestGetAppVersion() { { "channel does not exist", func() { - portID = "does not exist" + channelID = "does not exist" }, false, }, diff --git a/modules/apps/29-fee/keeper/relay_test.go b/modules/apps/29-fee/keeper/relay_test.go index f615015d7c7..d0a9e620f9d 100644 --- a/modules/apps/29-fee/keeper/relay_test.go +++ b/modules/apps/29-fee/keeper/relay_test.go @@ -129,7 +129,6 @@ func (suite *KeeperTestSuite) TestGetAppVersion() { 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 @@ -141,7 +140,7 @@ func (suite *KeeperTestSuite) TestGetAppVersion() { { "channel does not exist", func() { - portID = "does not exist" + channelID = "does not exist" }, false, },