From 4c4538947f08bdf55955230cc4ed8ee5b96d6228 Mon Sep 17 00:00:00 2001 From: dauTT Date: Mon, 3 Aug 2020 09:50:40 +0200 Subject: [PATCH] Allow empty version strings (#6909) * Allow empty version strings (left over from PR #6904) * Update spec Co-authored-by: Christopher Goes --- x/ibc/04-channel/types/msgs.go | 7 ------- x/ibc/04-channel/types/msgs_test.go | 6 +++--- x/ibc/spec/01_concepts.md | 29 ++++++++++++++++------------- 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/x/ibc/04-channel/types/msgs.go b/x/ibc/04-channel/types/msgs.go index 0b8a2065f8..8ef0be7ac4 100644 --- a/x/ibc/04-channel/types/msgs.go +++ b/x/ibc/04-channel/types/msgs.go @@ -2,7 +2,6 @@ package types import ( "encoding/base64" - "strings" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -98,9 +97,6 @@ func (msg MsgChannelOpenTry) ValidateBasic() error { if err := host.ChannelIdentifierValidator(msg.ChannelID); err != nil { return sdkerrors.Wrap(err, "invalid channel ID") } - if strings.TrimSpace(msg.CounterpartyVersion) == "" { - return sdkerrors.Wrap(ErrInvalidCounterparty, "counterparty version cannot be blank") - } if len(msg.ProofInit) == 0 { return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "cannot submit an empty proof init") } @@ -156,9 +152,6 @@ func (msg MsgChannelOpenAck) ValidateBasic() error { if err := host.ChannelIdentifierValidator(msg.ChannelID); err != nil { return sdkerrors.Wrap(err, "invalid channel ID") } - if strings.TrimSpace(msg.CounterpartyVersion) == "" { - return sdkerrors.Wrap(ErrInvalidCounterparty, "counterparty version cannot be blank") - } if len(msg.ProofTry) == 0 { return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "cannot submit an empty proof try") } diff --git a/x/ibc/04-channel/types/msgs_test.go b/x/ibc/04-channel/types/msgs_test.go index 2244e8f7db..a99c2ed265 100644 --- a/x/ibc/04-channel/types/msgs_test.go +++ b/x/ibc/04-channel/types/msgs_test.go @@ -188,14 +188,14 @@ func (suite *MsgTestSuite) TestMsgChannelOpenTry() { {testMsgs[4], false, "too short channel id"}, {testMsgs[5], false, "too long channel id"}, {testMsgs[6], false, "channel id contains non-alpha"}, - {testMsgs[7], false, "empty counterparty version"}, + {testMsgs[7], true, ""}, {testMsgs[8], false, "proof height is zero"}, {testMsgs[9], false, "invalid channel order"}, {testMsgs[10], false, "connection hops more than 1 "}, {testMsgs[11], false, "too short connection id"}, {testMsgs[12], false, "too long connection id"}, {testMsgs[13], false, "connection id contains non-alpha"}, - {testMsgs[14], true, "empty channel version"}, + {testMsgs[14], true, ""}, {testMsgs[15], false, "invalid counterparty port id"}, {testMsgs[16], false, "invalid counterparty channel id"}, {testMsgs[17], false, "empty proof"}, @@ -238,7 +238,7 @@ func (suite *MsgTestSuite) TestMsgChannelOpenAck() { {testMsgs[4], false, "too short channel id"}, {testMsgs[5], false, "too long channel id"}, {testMsgs[6], false, "channel id contains non-alpha"}, - {testMsgs[7], false, "empty counterparty version"}, + {testMsgs[7], true, ""}, {testMsgs[8], false, "empty proof"}, {testMsgs[9], false, "proof height is zero"}, } diff --git a/x/ibc/spec/01_concepts.md b/x/ibc/spec/01_concepts.md index 1645a6f1f6..3dffeb2be1 100644 --- a/x/ibc/spec/01_concepts.md +++ b/x/ibc/spec/01_concepts.md @@ -14,13 +14,13 @@ upon between the two parties. This occurs during the first 3 steps of the handshake. During `ConnOpenInit`, party A is expected to set all the versions they wish -to support within their connection state. It is expected that this set of -versions is from most preferred to least preferred. This is not a strict -requirement for the SDK implementation of IBC because the party calling -`ConnOpenTry` will greedily select the latest version it supports that the -counterparty supports as well. +to support within their connection state. It is expected that this set of +versions is from most preferred to least preferred. This is not a strict +requirement for the SDK implementation of IBC because the party calling +`ConnOpenTry` will greedily select the latest version it supports that the +counterparty supports as well. -During `ConnOpenTry`, party B will select a version from the counterparty's +During `ConnOpenTry`, party B will select a version from the counterparty's supported versions. Priority will be placed on the latest supported version. If a matching version cannot be found an error is returned. @@ -34,30 +34,30 @@ A valid connection version is considered to be in the following format: - the version tuple must be enclosed in parentheses - the feature set must be enclosed in brackets - there should be no space between the comma separting the identifier and the -feature set + feature set - the version identifier must no contain any commas - each feature must not contain any commas - each feature must be separated by commas ::: warning -A set of versions should not contain two versions with the same +A set of versions should not contain two versions with the same identifier, but differing feature sets. This will result in undefined behavior -with regards to version selection in `ConnOpenTry`. Each version in a set of +with regards to version selection in `ConnOpenTry`. Each version in a set of versions should have a unique version identifier. ::: ### Channel Version Negotation During the channel handshake procedure a version must be agreed upon between -the two parties. The selection process is largely left to the callers and +the two parties. The selection process is largely left to the callers and the verification of valid versioning must be handled by application developers in the channel handshake callbacks. -During `ChanOpenInit`, a version string is passed in and set in party A's +During `ChanOpenInit`, a version string is passed in and set in party A's channel state. During `ChanOpenTry`, a version string for party A and for party B are passed -in. The party A version string must match the version string used in +in. The party A version string must match the version string used in `ChanOpenInit` otherwise channel state verification will fail. The party B version string could be anything (even different than the proposed one by party A). However, the proposed version by party B is expected to be fully @@ -68,6 +68,9 @@ the version proposed by party B using the `MsgChanOpenAck` `CounterpartyVersion` field. The application module should throw an error if the version string is not valid. +In general empty version strings are to be considered valid options for an +application module. + Application modules may implement their own versioning system, such as semantic versioning, or they may lean upon the versioning system used for in connection version negotiation. To use the connection version semantics the application @@ -75,5 +78,5 @@ would simply pass the proto encoded version into each of the handshake calls and decode the version string into a `Version` instance to do version verification in the handshake callbacks. -Implementations which do not feel they would benefit from versioning can do +Implementations which do not feel they would benefit from versioning can do basic string matching using a single compatible version.