Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: remove crossing hellos from 03-connection #1672

Merged
merged 10 commits into from
Jul 12, 2022
Prev Previous commit
Next Next commit
fix connection version check in conn open ack
  • Loading branch information
colin-axner committed Jul 7, 2022
commit 163133c3b7b8b6f05dc1a1c8fe3b9e0c5e795ef7
10 changes: 5 additions & 5 deletions modules/core/03-connection/keeper/handshake.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func (k Keeper) ConnOpenInit(
) (string, error) {
versions := types.GetCompatibleVersions()
if version != nil {
if !types.IsSupportedVersion(version) {
if !types.IsSupportedVersion(types.GetCompatibleVersions(), version) {
return "", sdkerrors.Wrap(types.ErrInvalidVersion, "version is not supported")
}

Expand Down Expand Up @@ -179,19 +179,19 @@ func (k Keeper) ConnOpenAck(
return sdkerrors.Wrap(types.ErrConnectionNotFound, connectionID)
}

// Verify the provided version against the previously set connection state
// connection on ChainA must be in INIT or TRYOPEN
// verify the previously set connection state
if connection.State != types.INIT {
return sdkerrors.Wrapf(
types.ErrInvalidConnectionState,
"connection state is not INIT (got %s)", connection.State.String(),
)
}

if !types.IsSupportedVersion(version) {
// ensure selected version is supported
if !types.IsSupportedVersion(types.ProtoVersionsToExported(connection.Versions), version) {
Copy link
Contributor Author

@colin-axner colin-axner Jul 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be possible to change the API's to use the proto version type (remove the switch between exported/proto). We would remove the GetVersions function from the connection interface which is fine since we only have one connection type (we can cast to connectiontypes.ConnectionEnd) which is already what occurs

Lets open an issue and do in a followup pr

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opened #1689 and #1690

return sdkerrors.Wrapf(
types.ErrInvalidConnectionState,
"the provided version is not supported %s", version,
"the selected version is not supported %s", version,
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
)
}

Expand Down
4 changes: 2 additions & 2 deletions modules/core/03-connection/types/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ func GetCompatibleVersions() []exported.Version {
// IsSupportedVersion returns true if the proposed version has a matching version
// identifier and its entire feature set is supported or the version identifier
// supports an empty feature set.
func IsSupportedVersion(proposedVersion *Version) bool {
supportedVersion, found := FindSupportedVersion(proposedVersion, GetCompatibleVersions())
func IsSupportedVersion(supportedVersions []exported.Version, proposedVersion *Version) bool {
supportedVersion, found := FindSupportedVersion(proposedVersion, supportedVersions)
if !found {
return false
}
Expand Down
2 changes: 1 addition & 1 deletion modules/core/03-connection/types/version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func TestIsSupportedVersion(t *testing.T) {
}

for _, tc := range testCases {
require.Equal(t, tc.expPass, types.IsSupportedVersion(tc.version))
require.Equal(t, tc.expPass, types.IsSupportedVersion(types.GetCompatibleVersions(), tc.version))
}
}

Expand Down