From 058f3c8544e488e33a033d0b148c5755de8e36fd Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 7 Sep 2021 15:17:59 +0200 Subject: [PATCH 1/5] add generic middleware helper functions --- modules/core/04-channel/types/version.go | 19 ++++++++ modules/core/04-channel/types/version_test.go | 43 +++++++++++++++++++ modules/core/24-host/keys.go | 10 +++++ 3 files changed, 72 insertions(+) create mode 100644 modules/core/04-channel/types/version.go create mode 100644 modules/core/04-channel/types/version_test.go diff --git a/modules/core/04-channel/types/version.go b/modules/core/04-channel/types/version.go new file mode 100644 index 00000000000..f739cf9de27 --- /dev/null +++ b/modules/core/04-channel/types/version.go @@ -0,0 +1,19 @@ +package types + +import "strings" + +// SplitChannelVersion middleware version will split the channel version string +// into the outermost middleware version and the underlying app version. +// It will use the default delimiter `:` for middleware versions. +// In case there's no delimeter, this function returns an empty string for the middleware version (first return argument), +// and the full input as the second underlying app version. +func SplitChannelVersion(version string) (middlewareVersion, appVersion string) { + // only split out the first middleware version + splitVersions := strings.Split(version, ":") + if len(splitVersions) == 1 { + return "", version + } + middlewareVersion = splitVersions[0] + appVersion = strings.Join(splitVersions[1:], ":") + return +} diff --git a/modules/core/04-channel/types/version_test.go b/modules/core/04-channel/types/version_test.go new file mode 100644 index 00000000000..13f86a96528 --- /dev/null +++ b/modules/core/04-channel/types/version_test.go @@ -0,0 +1,43 @@ +package types_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/cosmos/ibc-go/modules/core/04-channel/types" +) + +func TestSplitVersions(t *testing.T) { + testCases := []struct { + name string + version string + mwVersion string + appVersion string + }{ + { + "single wrapped middleware", + "fee29-1:ics20-1", + "fee29-1", + "ics20-1", + }, + { + "multiple wrapped middleware", + "fee29-1:whitelist:ics20-1", + "fee29-1", + "whitelist:ics20-1", + }, + { + "no middleware", + "ics20-1", + "", + "ics20-1", + }, + } + + for _, tc := range testCases { + mwVersion, appVersion := types.SplitChannelVersion(tc.version) + require.Equal(t, tc.mwVersion, mwVersion, "middleware version is unexpected for case: %s", tc.name) + require.Equal(t, tc.appVersion, appVersion, "app version is unexpected for case: %s", tc.name) + } +} diff --git a/modules/core/24-host/keys.go b/modules/core/24-host/keys.go index ec07af5452f..8b35f4f6344 100644 --- a/modules/core/24-host/keys.go +++ b/modules/core/24-host/keys.go @@ -35,6 +35,7 @@ const ( KeyPortPrefix = "ports" KeySequencePrefix = "sequences" KeyChannelCapabilityPrefix = "capabilities" + KeyAppCapabilityPrefix = "appCapabilities" KeyNextSeqSendPrefix = "nextSequenceSend" KeyNextSeqRecvPrefix = "nextSequenceRecv" KeyNextSeqAckPrefix = "nextSequenceAck" @@ -142,6 +143,15 @@ func ChannelCapabilityPath(portID, channelID string) string { return fmt.Sprintf("%s/%s", KeyChannelCapabilityPrefix, channelPath(portID, channelID)) } +// AppCapabilityPath defines the path under which application capabilities can be issued by +// an ICS30 middleware wrapping an ICS26 application. +// Note the app capability path is only the name the middleware uses to reference a capability it issues +// to the underlying app. The underlying app will use the channel capability path to claim this capability +// from middleware just as the original capability issued by IBC is claimed by middleware using channel capability path. +func AppCapabilityPath(portID, channelID string) string { + return fmt.Sprintf("%s/%s", KeyAppCapabilityPrefix, channelPath(portID, channelID)) +} + // NextSequenceSendPath defines the next send sequence counter store path func NextSequenceSendPath(portID, channelID string) string { return fmt.Sprintf("%s/%s", KeyNextSeqSendPrefix, channelPath(portID, channelID)) From be3915a1085da5220249756a746ee756f44a2637 Mon Sep 17 00:00:00 2001 From: Aditya Date: Fri, 10 Sep 2021 12:43:17 +0200 Subject: [PATCH 2/5] Update modules/core/04-channel/types/version.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> --- modules/core/04-channel/types/version.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/core/04-channel/types/version.go b/modules/core/04-channel/types/version.go index f739cf9de27..c02d1293111 100644 --- a/modules/core/04-channel/types/version.go +++ b/modules/core/04-channel/types/version.go @@ -2,7 +2,7 @@ package types import "strings" -// SplitChannelVersion middleware version will split the channel version string +// SplitChannelVersion splits the channel version string // into the outermost middleware version and the underlying app version. // It will use the default delimiter `:` for middleware versions. // In case there's no delimeter, this function returns an empty string for the middleware version (first return argument), From 318859c00a30dfb78afbc078480446133ed4aa76 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 20 Sep 2021 18:49:15 +0200 Subject: [PATCH 3/5] remove app capability, add merge fn, changelog --- CHANGELOG.md | 1 + modules/core/04-channel/types/version.go | 12 ++++++-- modules/core/04-channel/types/version_test.go | 29 +++++++++++++++++++ modules/core/24-host/keys.go | 10 ------- 4 files changed, 40 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 226f63967b6..a5626a840d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements * [\#373](https://github.com/cosmos/ibc-go/pull/375) Added optional field `PacketCommitmentSequences` to `QueryPacketAcknowledgementsRequest` to provide filtering of packet acknowledgements +* [\#383](https://github.com/cosmos/ibc-go/pull/383) Adds helper functions for merging and splitting middleware versions from the underlying app version ### Features * [\#372](https://github.com/cosmos/ibc-go/pull/372) New CLI command `query ibc client status ` to get the current activity status of a client diff --git a/modules/core/04-channel/types/version.go b/modules/core/04-channel/types/version.go index f739cf9de27..a2696d291ed 100644 --- a/modules/core/04-channel/types/version.go +++ b/modules/core/04-channel/types/version.go @@ -2,6 +2,8 @@ package types import "strings" +const ChannelVersionDelimiter = ":" + // SplitChannelVersion middleware version will split the channel version string // into the outermost middleware version and the underlying app version. // It will use the default delimiter `:` for middleware versions. @@ -9,11 +11,17 @@ import "strings" // and the full input as the second underlying app version. func SplitChannelVersion(version string) (middlewareVersion, appVersion string) { // only split out the first middleware version - splitVersions := strings.Split(version, ":") + splitVersions := strings.Split(version, ChannelVersionDelimiter) if len(splitVersions) == 1 { return "", version } middlewareVersion = splitVersions[0] - appVersion = strings.Join(splitVersions[1:], ":") + appVersion = strings.Join(splitVersions[1:], ChannelVersionDelimiter) return } + +// MergeChannelVersions merges the provided versions together with the channel version delimiter +// the versions should be passed in from the highest-level middleware to the base application +func MergeChannelVersions(versions ...string) string { + return strings.Join(versions, ChannelVersionDelimiter) +} diff --git a/modules/core/04-channel/types/version_test.go b/modules/core/04-channel/types/version_test.go index 13f86a96528..49031c61595 100644 --- a/modules/core/04-channel/types/version_test.go +++ b/modules/core/04-channel/types/version_test.go @@ -41,3 +41,32 @@ func TestSplitVersions(t *testing.T) { require.Equal(t, tc.appVersion, appVersion, "app version is unexpected for case: %s", tc.name) } } + +func TestMergeVersions(t *testing.T) { + testCases := []struct { + name + versions []string + merged string + }{ + { + "single version", + []string{"ics20-1"}, + "ics20-1", + }, + { + "two versions", + []string{"fee29-1", "ics20-1"}, + "fee29-1:ics20-1", + }, + { + "multiple versions", + []string{"fee29-1", "whitelist", "ics20-1"}, + "fee29-1:whitelist:ics20-1", + }, + } + + for _, tc := range testCases { + actual := types.MergeChannelVersions(tc.versions...) + require.Equal(t, tc.merged, actual, "merged versions string does not equal expected value") + } +} diff --git a/modules/core/24-host/keys.go b/modules/core/24-host/keys.go index 8b35f4f6344..ec07af5452f 100644 --- a/modules/core/24-host/keys.go +++ b/modules/core/24-host/keys.go @@ -35,7 +35,6 @@ const ( KeyPortPrefix = "ports" KeySequencePrefix = "sequences" KeyChannelCapabilityPrefix = "capabilities" - KeyAppCapabilityPrefix = "appCapabilities" KeyNextSeqSendPrefix = "nextSequenceSend" KeyNextSeqRecvPrefix = "nextSequenceRecv" KeyNextSeqAckPrefix = "nextSequenceAck" @@ -143,15 +142,6 @@ func ChannelCapabilityPath(portID, channelID string) string { return fmt.Sprintf("%s/%s", KeyChannelCapabilityPrefix, channelPath(portID, channelID)) } -// AppCapabilityPath defines the path under which application capabilities can be issued by -// an ICS30 middleware wrapping an ICS26 application. -// Note the app capability path is only the name the middleware uses to reference a capability it issues -// to the underlying app. The underlying app will use the channel capability path to claim this capability -// from middleware just as the original capability issued by IBC is claimed by middleware using channel capability path. -func AppCapabilityPath(portID, channelID string) string { - return fmt.Sprintf("%s/%s", KeyAppCapabilityPrefix, channelPath(portID, channelID)) -} - // NextSequenceSendPath defines the next send sequence counter store path func NextSequenceSendPath(portID, channelID string) string { return fmt.Sprintf("%s/%s", KeyNextSeqSendPrefix, channelPath(portID, channelID)) From 5380d2a1e253b0b9e3a0ef5a3b8f778c03f85a69 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 20 Sep 2021 19:07:21 +0200 Subject: [PATCH 4/5] add back newline --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dd729883a0d..16cdee8e7e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (24-host) [\#344](https://github.com/cosmos/ibc-go/pull/344) Increase port identifier limit to 128 characters. ### Improvements + * [\#373](https://github.com/cosmos/ibc-go/pull/375) Added optional field `PacketCommitmentSequences` to `QueryPacketAcknowledgementsRequest` to provide filtering of packet acknowledgements. * [\#383](https://github.com/cosmos/ibc-go/pull/383) Adds helper functions for merging and splitting middleware versions from the underlying app version. From 3bf7daee71cae8a3c9626a7238c1fea0768b7943 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 21 Sep 2021 16:47:47 +0200 Subject: [PATCH 5/5] address colin requests --- CHANGELOG.md | 3 ++- modules/core/04-channel/types/version_test.go | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16cdee8e7e6..81eac3c769c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* [\#383](https://github.com/cosmos/ibc-go/pull/383) Adds helper functions for merging and splitting middleware versions from the underlying app version. + ### Features * [\#384](https://github.com/cosmos/ibc-go/pull/384) Added `NegotiateAppVersion` method to `IBCModule` interface supported by a gRPC query service in `05-port`. This provides routing of requests to the desired application module callback, which in turn performs application version negotiation. @@ -58,7 +60,6 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements * [\#373](https://github.com/cosmos/ibc-go/pull/375) Added optional field `PacketCommitmentSequences` to `QueryPacketAcknowledgementsRequest` to provide filtering of packet acknowledgements. -* [\#383](https://github.com/cosmos/ibc-go/pull/383) Adds helper functions for merging and splitting middleware versions from the underlying app version. ### Features diff --git a/modules/core/04-channel/types/version_test.go b/modules/core/04-channel/types/version_test.go index 1b4e89e9be9..4011aeade2c 100644 --- a/modules/core/04-channel/types/version_test.go +++ b/modules/core/04-channel/types/version_test.go @@ -53,6 +53,11 @@ func TestMergeVersions(t *testing.T) { []string{"ics20-1"}, "ics20-1", }, + { + "empty version", + []string{}, + "", + }, { "two versions", []string{"fee29-1", "ics20-1"},