From c0c76e2d04f01ef1370ebbf8fbed9c8edce21253 Mon Sep 17 00:00:00 2001 From: Reece Williams Date: Mon, 17 Jul 2023 20:16:13 -0500 Subject: [PATCH 1/4] Remove Version interface abstraction and casting --- .../ibc/light-clients/localhost/connection.md | 2 +- .../03-connection/keeper/grpc_query_test.go | 8 ++-- .../core/03-connection/keeper/handshake.go | 10 ++-- .../03-connection/keeper/handshake_test.go | 12 ++--- modules/core/03-connection/keeper/keeper.go | 2 +- .../core/03-connection/keeper/keeper_test.go | 6 +-- .../03-connection/simulation/decoder_test.go | 2 +- modules/core/03-connection/types/codec.go | 5 -- .../core/03-connection/types/connection.go | 4 +- modules/core/03-connection/types/version.go | 47 ++++++------------- .../core/03-connection/types/version_test.go | 25 +++++----- modules/core/exported/connection.go | 7 --- modules/core/keeper/msg_server.go | 2 +- .../09-localhost/client_state_test.go | 2 +- testing/values.go | 2 +- 15 files changed, 52 insertions(+), 84 deletions(-) diff --git a/docs/ibc/light-clients/localhost/connection.md b/docs/ibc/light-clients/localhost/connection.md index 33251bde432..301d93455b5 100644 --- a/docs/ibc/light-clients/localhost/connection.md +++ b/docs/ibc/light-clients/localhost/connection.md @@ -16,7 +16,7 @@ The `ConnectionEnd` and its `Counterparty` both reference the `09-localhost` cli // CreateSentinelLocalhostConnection creates and sets the sentinel localhost connection end in the IBC store. func (k Keeper) CreateSentinelLocalhostConnection(ctx sdk.Context) { counterparty := types.NewCounterparty(exported.LocalhostClientID, exported.LocalhostConnectionID, commitmenttypes.NewMerklePrefix(k.GetCommitmentPrefix().Bytes())) - connectionEnd := types.NewConnectionEnd(types.OPEN, exported.LocalhostClientID, counterparty, types.ExportedVersionsToProto(types.GetCompatibleVersions()), 0) + connectionEnd := types.NewConnectionEnd(types.OPEN, exported.LocalhostClientID, counterparty, types.GetCompatibleVersions(), 0) k.SetConnection(ctx, exported.LocalhostConnectionID, connectionEnd) } diff --git a/modules/core/03-connection/keeper/grpc_query_test.go b/modules/core/03-connection/keeper/grpc_query_test.go index 0573e3e02aa..3c535727922 100644 --- a/modules/core/03-connection/keeper/grpc_query_test.go +++ b/modules/core/03-connection/keeper/grpc_query_test.go @@ -55,7 +55,7 @@ func (suite *KeeperTestSuite) TestQueryConnection() { suite.Require().NoError(err) counterparty := types.NewCounterparty(path.EndpointB.ClientID, "", suite.chainB.GetPrefix()) - expConnection = types.NewConnectionEnd(types.INIT, path.EndpointA.ClientID, counterparty, types.ExportedVersionsToProto(types.GetCompatibleVersions()), 500) + expConnection = types.NewConnectionEnd(types.INIT, path.EndpointA.ClientID, counterparty, types.GetCompatibleVersions(), 500) suite.chainA.App.GetIBCKeeper().ConnectionKeeper.SetConnection(suite.chainA.GetContext(), path.EndpointA.ConnectionID, expConnection) req = &types.QueryConnectionRequest{ @@ -135,9 +135,9 @@ func (suite *KeeperTestSuite) TestQueryConnections() { // counterparty connection id is blank after open init counterparty3 := types.NewCounterparty(path3.EndpointB.ClientID, "", suite.chainB.GetPrefix()) - conn1 := types.NewConnectionEnd(types.OPEN, path1.EndpointA.ClientID, counterparty1, types.ExportedVersionsToProto(types.GetCompatibleVersions()), 0) - conn2 := types.NewConnectionEnd(types.OPEN, path2.EndpointA.ClientID, counterparty2, types.ExportedVersionsToProto(types.GetCompatibleVersions()), 0) - conn3 := types.NewConnectionEnd(types.INIT, path3.EndpointA.ClientID, counterparty3, types.ExportedVersionsToProto(types.GetCompatibleVersions()), 0) + conn1 := types.NewConnectionEnd(types.OPEN, path1.EndpointA.ClientID, counterparty1, types.GetCompatibleVersions(), 0) + conn2 := types.NewConnectionEnd(types.OPEN, path2.EndpointA.ClientID, counterparty2, types.GetCompatibleVersions(), 0) + conn3 := types.NewConnectionEnd(types.INIT, path3.EndpointA.ClientID, counterparty3, types.GetCompatibleVersions(), 0) iconn1 := types.NewIdentifiedConnection(path1.EndpointA.ConnectionID, conn1) iconn2 := types.NewIdentifiedConnection(path2.EndpointA.ConnectionID, conn2) diff --git a/modules/core/03-connection/keeper/handshake.go b/modules/core/03-connection/keeper/handshake.go index 61e50b7a55d..60267beff36 100644 --- a/modules/core/03-connection/keeper/handshake.go +++ b/modules/core/03-connection/keeper/handshake.go @@ -31,7 +31,7 @@ func (k Keeper) ConnOpenInit( return "", errorsmod.Wrap(types.ErrInvalidVersion, "version is not supported") } - versions = []exported.Version{version} + versions = []*types.Version{version} } clientState, found := k.clientKeeper.GetClientState(ctx, clientID) @@ -49,7 +49,7 @@ func (k Keeper) ConnOpenInit( } // connection defines chain A's ConnectionEnd - connection := types.NewConnectionEnd(types.INIT, clientID, counterparty, types.ExportedVersionsToProto(versions), delayPeriod) + connection := types.NewConnectionEnd(types.INIT, clientID, counterparty, versions, delayPeriod) k.SetConnection(ctx, connectionID, connection) k.Logger(ctx).Info("connection state updated", "connection-id", connectionID, "previous-state", types.UNINITIALIZED.String(), "new-state", types.INIT.String()) @@ -73,7 +73,7 @@ func (k Keeper) ConnOpenTry( delayPeriod uint64, clientID string, // clientID of chainA clientState exported.ClientState, // clientState that chainA has for chainB - counterpartyVersions []exported.Version, // supported versions of chain A + counterpartyVersions []*types.Version, // supported versions of chain A proofInit []byte, // proof that chainA stored connectionEnd in state (on ConnOpenInit) proofClient []byte, // proof that chainA stored a light client of chainB proofConsensus []byte, // proof that chainA stored chainB's consensus state at consensus height @@ -108,7 +108,7 @@ func (k Keeper) ConnOpenTry( // NOTE: chainA and chainB must have the same delay period prefix := k.GetCommitmentPrefix() expectedCounterparty := types.NewCounterparty(clientID, "", commitmenttypes.NewMerklePrefix(prefix.Bytes())) - expectedConnection := types.NewConnectionEnd(types.INIT, counterparty.ClientId, expectedCounterparty, types.ExportedVersionsToProto(counterpartyVersions), delayPeriod) + expectedConnection := types.NewConnectionEnd(types.INIT, counterparty.ClientId, expectedCounterparty, counterpartyVersions, delayPeriod) // chain B picks a version from Chain A's available versions that is compatible // with Chain B's supported IBC versions. PickVersion will select the intersection @@ -197,7 +197,7 @@ func (k Keeper) ConnOpenAck( } // ensure selected version is supported - if !types.IsSupportedVersion(types.ProtoVersionsToExported(connection.Versions), version) { + if !types.IsSupportedVersion(connection.Versions, version) { return errorsmod.Wrapf( types.ErrInvalidConnectionState, "the counterparty selected version %s is not supported by versions selected on INIT", version, diff --git a/modules/core/03-connection/keeper/handshake_test.go b/modules/core/03-connection/keeper/handshake_test.go index 5751c9e531a..c67d5c9bf31 100644 --- a/modules/core/03-connection/keeper/handshake_test.go +++ b/modules/core/03-connection/keeper/handshake_test.go @@ -33,7 +33,7 @@ func (suite *KeeperTestSuite) TestConnOpenInit() { emptyConnBID = true }, true}, {"success with non empty version", func() { - version = types.ExportedVersionsToProto(types.GetCompatibleVersions())[0] + version = types.GetCompatibleVersions()[0] }, true}, {"success with non zero delayPeriod", func() { delayPeriod = uint64(time.Hour.Nanoseconds()) @@ -96,7 +96,7 @@ func (suite *KeeperTestSuite) TestConnOpenTry() { var ( path *ibctesting.Path delayPeriod uint64 - versions []exported.Version + versions []*types.Version consensusHeight exported.Height counterpartyClient exported.ClientState ) @@ -181,7 +181,7 @@ func (suite *KeeperTestSuite) TestConnOpenTry() { counterpartyClient = suite.chainA.GetClientState(path.EndpointA.ClientID) version := types.NewVersion("0.0", nil) - versions = []exported.Version{version} + versions = []*types.Version{version} }, false}, {"connection state verification failed", func() { // chainA connection not created @@ -478,9 +478,9 @@ func (suite *KeeperTestSuite) TestConnOpenAck() { for _, tc := range testCases { tc := tc suite.Run(tc.msg, func() { - suite.SetupTest() // reset - version = types.ExportedVersionsToProto(types.GetCompatibleVersions())[0] // must be explicitly changed in malleate - consensusHeight = clienttypes.ZeroHeight() // must be explicitly changed in malleate + suite.SetupTest() // reset + version = types.GetCompatibleVersions()[0] // must be explicitly changed in malleate + consensusHeight = clienttypes.ZeroHeight() // must be explicitly changed in malleate path = ibctesting.NewPath(suite.chainA, suite.chainB) suite.coordinator.SetupClients(path) diff --git a/modules/core/03-connection/keeper/keeper.go b/modules/core/03-connection/keeper/keeper.go index 2de788c3310..220e09f30c4 100644 --- a/modules/core/03-connection/keeper/keeper.go +++ b/modules/core/03-connection/keeper/keeper.go @@ -201,7 +201,7 @@ func (k Keeper) GetAllConnections(ctx sdk.Context) (connections []types.Identifi // CreateSentinelLocalhostConnection creates and sets the sentinel localhost connection end in the IBC store. func (k Keeper) CreateSentinelLocalhostConnection(ctx sdk.Context) { counterparty := types.NewCounterparty(exported.LocalhostClientID, exported.LocalhostConnectionID, commitmenttypes.NewMerklePrefix(k.GetCommitmentPrefix().Bytes())) - connectionEnd := types.NewConnectionEnd(types.OPEN, exported.LocalhostClientID, counterparty, types.ExportedVersionsToProto(types.GetCompatibleVersions()), 0) + connectionEnd := types.NewConnectionEnd(types.OPEN, exported.LocalhostClientID, counterparty, types.GetCompatibleVersions(), 0) k.SetConnection(ctx, exported.LocalhostConnectionID, connectionEnd) } diff --git a/modules/core/03-connection/keeper/keeper_test.go b/modules/core/03-connection/keeper/keeper_test.go index 5f4e5b3f244..37f185a6d66 100644 --- a/modules/core/03-connection/keeper/keeper_test.go +++ b/modules/core/03-connection/keeper/keeper_test.go @@ -74,8 +74,8 @@ func (suite KeeperTestSuite) TestGetAllConnections() { //nolint:govet // this is counterpartyB0 := types.NewCounterparty(path1.EndpointB.ClientID, path1.EndpointB.ConnectionID, suite.chainB.GetPrefix()) // connection B0 counterpartyB1 := types.NewCounterparty(path2.EndpointB.ClientID, path2.EndpointB.ConnectionID, suite.chainB.GetPrefix()) // connection B1 - conn1 := types.NewConnectionEnd(types.OPEN, path1.EndpointA.ClientID, counterpartyB0, types.ExportedVersionsToProto(types.GetCompatibleVersions()), 0) // A0 - B0 - conn2 := types.NewConnectionEnd(types.OPEN, path2.EndpointA.ClientID, counterpartyB1, types.ExportedVersionsToProto(types.GetCompatibleVersions()), 0) // A1 - B1 + conn1 := types.NewConnectionEnd(types.OPEN, path1.EndpointA.ClientID, counterpartyB0, types.GetCompatibleVersions(), 0) // A0 - B0 + conn2 := types.NewConnectionEnd(types.OPEN, path2.EndpointA.ClientID, counterpartyB1, types.GetCompatibleVersions(), 0) // A1 - B1 iconn1 := types.NewIdentifiedConnection(path1.EndpointA.ConnectionID, conn1) iconn2 := types.NewIdentifiedConnection(path2.EndpointA.ConnectionID, conn2) @@ -172,7 +172,7 @@ func (suite *KeeperTestSuite) TestLocalhostConnectionEndCreation() { suite.Require().True(found) suite.Require().Equal(types.OPEN, connectionEnd.State) suite.Require().Equal(exported.LocalhostClientID, connectionEnd.ClientId) - suite.Require().Equal(types.ExportedVersionsToProto(types.GetCompatibleVersions()), connectionEnd.Versions) + suite.Require().Equal(types.GetCompatibleVersions(), connectionEnd.Versions) expectedCounterParty := types.NewCounterparty(exported.LocalhostClientID, exported.LocalhostConnectionID, commitmenttypes.NewMerklePrefix(connectionKeeper.GetCommitmentPrefix().Bytes())) suite.Require().Equal(expectedCounterParty, connectionEnd.Counterparty) } diff --git a/modules/core/03-connection/simulation/decoder_test.go b/modules/core/03-connection/simulation/decoder_test.go index 8cb2ae5f3ef..0a80a8edf34 100644 --- a/modules/core/03-connection/simulation/decoder_test.go +++ b/modules/core/03-connection/simulation/decoder_test.go @@ -22,7 +22,7 @@ func TestDecodeStore(t *testing.T) { connection := types.ConnectionEnd{ ClientId: "clientidone", - Versions: types.ExportedVersionsToProto(types.GetCompatibleVersions()), + Versions: types.GetCompatibleVersions(), } paths := types.ClientPaths{ diff --git a/modules/core/03-connection/types/codec.go b/modules/core/03-connection/types/codec.go index d6c1c1b2693..4deebb33401 100644 --- a/modules/core/03-connection/types/codec.go +++ b/modules/core/03-connection/types/codec.go @@ -22,11 +22,6 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) { (*exported.CounterpartyConnectionI)(nil), &Counterparty{}, ) - registry.RegisterInterface( - "ibc.core.connection.v1.Version", - (*exported.Version)(nil), - &Version{}, - ) registry.RegisterImplementations( (*sdk.Msg)(nil), &MsgConnectionOpenInit{}, diff --git a/modules/core/03-connection/types/connection.go b/modules/core/03-connection/types/connection.go index 03735cccced..e202d5b2c45 100644 --- a/modules/core/03-connection/types/connection.go +++ b/modules/core/03-connection/types/connection.go @@ -38,8 +38,8 @@ func (c ConnectionEnd) GetCounterparty() exported.CounterpartyConnectionI { } // GetVersions implements the Connection interface -func (c ConnectionEnd) GetVersions() []exported.Version { - return ProtoVersionsToExported(c.Versions) +func (c ConnectionEnd) GetVersions() []*Version { + return c.Versions } // GetDelayPeriod implements the Connection interface diff --git a/modules/core/03-connection/types/version.go b/modules/core/03-connection/types/version.go index c4310c4dfa9..3d154715e5c 100644 --- a/modules/core/03-connection/types/version.go +++ b/modules/core/03-connection/types/version.go @@ -6,7 +6,6 @@ import ( errorsmod "cosmossdk.io/errors" collections "github.com/cosmos/ibc-go/v7/internal/collections" - "github.com/cosmos/ibc-go/v7/modules/core/exported" ) var ( @@ -31,8 +30,6 @@ var ( } ) -var _ exported.Version = (*Version)(nil) - // NewVersion returns a new instance of Version. func NewVersion(identifier string, features []string) *Version { return &Version{ @@ -73,7 +70,7 @@ func ValidateVersion(version *Version) error { // proposed version is supported by this chain. If the feature set is // empty it verifies that this is allowed for the specified version // identifier. -func (version Version) VerifyProposedVersion(proposedVersion exported.Version) error { +func (version Version) VerifyProposedVersion(proposedVersion *Version) error { if proposedVersion.GetIdentifier() != version.GetIdentifier() { return errorsmod.Wrapf( ErrVersionNegotiationFailed, @@ -102,7 +99,7 @@ func (version Version) VerifyProposedVersion(proposedVersion exported.Version) e // VerifySupportedFeature takes in a version and feature string and returns // true if the feature is supported by the version and false otherwise. -func VerifySupportedFeature(version exported.Version, feature string) bool { +func VerifySupportedFeature(version *Version, feature string) bool { for _, f := range version.GetFeatures() { if f == feature { return true @@ -115,14 +112,19 @@ func VerifySupportedFeature(version exported.Version, feature string) bool { // versions for the caller chain's connection end. The latest supported // version should be first element and the set should descend to the oldest // supported version. -func GetCompatibleVersions() []exported.Version { - return []exported.Version{DefaultIBCVersion} +func GetCompatibleVersions() []*Version { + return []*Version{ + { + Identifier: DefaultIBCVersionIdentifier, + Features: SupportedOrderings, + }, + } } // 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(supportedVersions []exported.Version, proposedVersion *Version) bool { +func IsSupportedVersion(supportedVersions []*Version, proposedVersion *Version) bool { supportedVersion, found := FindSupportedVersion(proposedVersion, supportedVersions) if !found { return false @@ -138,13 +140,14 @@ func IsSupportedVersion(supportedVersions []exported.Version, proposedVersion *V // FindSupportedVersion returns the version with a matching version identifier // if it exists. The returned boolean is true if the version is found and // false otherwise. -func FindSupportedVersion(version exported.Version, supportedVersions []exported.Version) (exported.Version, bool) { +func FindSupportedVersion(version *Version, supportedVersions []*Version) (*Version, bool) { for _, supportedVersion := range supportedVersions { if version.GetIdentifier() == supportedVersion.GetIdentifier() { return supportedVersion, true } } - return nil, false + + return &Version{}, false } // PickVersion iterates over the descending ordered set of compatible IBC @@ -158,7 +161,7 @@ func FindSupportedVersion(version exported.Version, supportedVersions []exported // // CONTRACT: PickVersion must only provide a version that is in the // intersection of the supported versions and the counterparty versions. -func PickVersion(supportedVersions, counterpartyVersions []exported.Version) (*Version, error) { +func PickVersion(supportedVersions, counterpartyVersions []*Version) (*Version, error) { for _, supportedVersion := range supportedVersions { // check if the source version is supported by the counterparty if counterpartyVersion, found := FindSupportedVersion(supportedVersion, counterpartyVersions); found { @@ -190,25 +193,3 @@ func GetFeatureSetIntersection(sourceFeatureSet, counterpartyFeatureSet []string return featureSet } - -// ExportedVersionsToProto casts a slice of the Version interface to a slice -// of the Version proto definition. -func ExportedVersionsToProto(exportedVersions []exported.Version) []*Version { - versions := make([]*Version, len(exportedVersions)) - for i := range exportedVersions { - versions[i] = exportedVersions[i].(*Version) - } - - return versions -} - -// ProtoVersionsToExported converts a slice of the Version proto definition to -// the Version interface. -func ProtoVersionsToExported(versions []*Version) []exported.Version { - exportedVersions := make([]exported.Version, len(versions)) - for i := range versions { - exportedVersions[i] = versions[i] - } - - return exportedVersions -} diff --git a/modules/core/03-connection/types/version_test.go b/modules/core/03-connection/types/version_test.go index 9aa4abf9259..e30d8381a09 100644 --- a/modules/core/03-connection/types/version_test.go +++ b/modules/core/03-connection/types/version_test.go @@ -6,7 +6,6 @@ import ( "github.com/stretchr/testify/require" "github.com/cosmos/ibc-go/v7/modules/core/03-connection/types" - "github.com/cosmos/ibc-go/v7/modules/core/exported" ibctesting "github.com/cosmos/ibc-go/v7/testing" ) @@ -41,7 +40,7 @@ func TestIsSupportedVersion(t *testing.T) { }{ { "version is supported", - types.ExportedVersionsToProto(types.GetCompatibleVersions())[0], + types.GetCompatibleVersions()[0], true, }, { @@ -65,14 +64,14 @@ func TestFindSupportedVersion(t *testing.T) { testCases := []struct { name string version *types.Version - supportedVersions []exported.Version + supportedVersions []*types.Version expVersion *types.Version expFound bool }{ {"valid supported version", types.DefaultIBCVersion, types.GetCompatibleVersions(), types.DefaultIBCVersion, true}, {"empty (invalid) version", &types.Version{}, types.GetCompatibleVersions(), &types.Version{}, false}, - {"empty supported versions", types.DefaultIBCVersion, []exported.Version{}, &types.Version{}, false}, - {"desired version is last", types.DefaultIBCVersion, []exported.Version{types.NewVersion("1.1", nil), types.NewVersion("2", []string{"ORDER_UNORDERED"}), types.NewVersion("3", nil), types.DefaultIBCVersion}, types.DefaultIBCVersion, true}, + {"empty supported versions", types.DefaultIBCVersion, []*types.Version{}, &types.Version{}, false}, + {"desired version is last", types.DefaultIBCVersion, []*types.Version{types.NewVersion("1", nil), types.NewVersion("2", []string{"ORDER_UNORDERED"}), types.NewVersion("3", nil)}, types.DefaultIBCVersion, true}, {"desired version identifier with different feature set", types.NewVersion(types.DefaultIBCVersionIdentifier, []string{"ORDER_DAG"}), types.GetCompatibleVersions(), types.DefaultIBCVersion, true}, {"version not supported", types.NewVersion("2", []string{"ORDER_DAG"}), types.GetCompatibleVersions(), &types.Version{}, false}, } @@ -84,7 +83,7 @@ func TestFindSupportedVersion(t *testing.T) { require.True(t, found, "test case %d: %s", i, tc.name) } else { require.False(t, found, "test case: %s", tc.name) - require.Nil(t, version, "test case: %s", tc.name) + require.Equal(t, version, &types.Version{}, "test case: %s", tc.name) } } } @@ -92,17 +91,17 @@ func TestFindSupportedVersion(t *testing.T) { func TestPickVersion(t *testing.T) { testCases := []struct { name string - supportedVersions []exported.Version - counterpartyVersions []exported.Version + supportedVersions []*types.Version + counterpartyVersions []*types.Version expVer *types.Version expPass bool }{ {"valid default ibc version", types.GetCompatibleVersions(), types.GetCompatibleVersions(), types.DefaultIBCVersion, true}, - {"valid version in counterparty versions", types.GetCompatibleVersions(), []exported.Version{types.NewVersion("version1", nil), types.NewVersion("2.0.0", []string{"ORDER_UNORDERED-ZK"}), types.DefaultIBCVersion}, types.DefaultIBCVersion, true}, - {"valid identifier match but empty feature set not allowed", types.GetCompatibleVersions(), []exported.Version{types.NewVersion(types.DefaultIBCVersionIdentifier, []string{"DAG", "ORDERED-ZK", "UNORDERED-zk]"})}, types.NewVersion(types.DefaultIBCVersionIdentifier, nil), false}, - {"empty counterparty versions", types.GetCompatibleVersions(), []exported.Version{}, &types.Version{}, false}, - {"non-matching counterparty versions", types.GetCompatibleVersions(), []exported.Version{types.NewVersion("2.0.0", nil)}, &types.Version{}, false}, - {"non-matching counterparty versions (uses ordered channels only) contained in supported versions (uses unordered channels only)", []exported.Version{types.NewVersion(types.DefaultIBCVersionIdentifier, []string{"ORDER_UNORDERED"})}, []exported.Version{types.NewVersion(types.DefaultIBCVersionIdentifier, []string{"ORDER_ORDERED"})}, &types.Version{}, false}, + {"valid version in counterparty versions", []*types.Version{types.NewVersion("2.0.0", []string{"ORDER_UNORDERED-ZK"})}, []*types.Version{types.NewVersion("version1", nil), types.NewVersion("2.0.0", []string{"ORDER_UNORDERED-ZK"})}, types.DefaultIBCVersion, true}, + {"valid identifier match but empty feature set not allowed", types.GetCompatibleVersions(), []*types.Version{types.NewVersion(types.DefaultIBCVersionIdentifier, []string{"DAG", "ORDERED-ZK", "UNORDERED-zk]"})}, types.NewVersion(types.DefaultIBCVersionIdentifier, nil), false}, + {"empty counterparty versions", types.GetCompatibleVersions(), []*types.Version{}, &types.Version{}, false}, + {"non-matching counterparty versions", types.GetCompatibleVersions(), []*types.Version{types.NewVersion("2.0.0", nil)}, &types.Version{}, false}, + {"non-matching counterparty versions (uses ordered channels only) contained in supported versions (uses unordered channels only)", []*types.Version{types.NewVersion(types.DefaultIBCVersionIdentifier, []string{"ORDER_UNORDERED"})}, []*types.Version{types.NewVersion(types.DefaultIBCVersionIdentifier, []string{"ORDER_ORDERED"})}, &types.Version{}, false}, } for i, tc := range testCases { diff --git a/modules/core/exported/connection.go b/modules/core/exported/connection.go index c418e8497a7..a8341a255a9 100644 --- a/modules/core/exported/connection.go +++ b/modules/core/exported/connection.go @@ -19,10 +19,3 @@ type CounterpartyConnectionI interface { GetPrefix() Prefix ValidateBasic() error } - -// Version defines an IBC version used in connection handshake negotiation. -type Version interface { - GetIdentifier() string - GetFeatures() []string - VerifyProposedVersion(Version) error -} diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index dc2690e4e97..4916403edb8 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -122,7 +122,7 @@ func (k Keeper) ConnectionOpenTry(goCtx context.Context, msg *connectiontypes.Ms if _, err := k.ConnectionKeeper.ConnOpenTry( ctx, msg.Counterparty, msg.DelayPeriod, msg.ClientId, targetClient, - connectiontypes.ProtoVersionsToExported(msg.CounterpartyVersions), msg.ProofInit, msg.ProofClient, msg.ProofConsensus, + msg.CounterpartyVersions, msg.ProofInit, msg.ProofClient, msg.ProofConsensus, msg.ProofHeight, msg.ConsensusHeight, ); err != nil { return nil, errorsmod.Wrap(err, "connection handshake open try failed") diff --git a/modules/light-clients/09-localhost/client_state_test.go b/modules/light-clients/09-localhost/client_state_test.go index b11034a87e3..f241c400d85 100644 --- a/modules/light-clients/09-localhost/client_state_test.go +++ b/modules/light-clients/09-localhost/client_state_test.go @@ -139,7 +139,7 @@ func (suite *LocalhostTestSuite) TestVerifyMembership() { connectiontypes.OPEN, exported.LocalhostClientID, connectiontypes.NewCounterparty(exported.LocalhostClientID, exported.LocalhostConnectionID, suite.chain.GetPrefix()), - connectiontypes.ExportedVersionsToProto(connectiontypes.GetCompatibleVersions()), 0, + connectiontypes.GetCompatibleVersions(), 0, ) suite.chain.GetSimApp().GetIBCKeeper().ConnectionKeeper.SetConnection(suite.chain.GetContext(), exported.LocalhostConnectionID, connectionEnd) diff --git a/testing/values.go b/testing/values.go index 3e933053c23..291cf9a44b4 100644 --- a/testing/values.go +++ b/testing/values.go @@ -57,7 +57,7 @@ var ( UpgradePath = []string{"upgrade", "upgradedIBCState"} - ConnectionVersion = connectiontypes.ExportedVersionsToProto(connectiontypes.GetCompatibleVersions())[0] + ConnectionVersion = connectiontypes.GetCompatibleVersions()[0] MockAcknowledgement = mock.MockAcknowledgement.Acknowledgement() MockPacketData = mock.MockPacketData From 8eed0a5062d9dc90fd85f4cb561587416bbf0fda Mon Sep 17 00:00:00 2001 From: Reece Williams Date: Mon, 17 Jul 2023 22:31:15 -0500 Subject: [PATCH 2/4] Fix accidently modified SupportedVersion test --- modules/core/03-connection/types/version_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/core/03-connection/types/version_test.go b/modules/core/03-connection/types/version_test.go index e30d8381a09..1696f68308d 100644 --- a/modules/core/03-connection/types/version_test.go +++ b/modules/core/03-connection/types/version_test.go @@ -71,7 +71,7 @@ func TestFindSupportedVersion(t *testing.T) { {"valid supported version", types.DefaultIBCVersion, types.GetCompatibleVersions(), types.DefaultIBCVersion, true}, {"empty (invalid) version", &types.Version{}, types.GetCompatibleVersions(), &types.Version{}, false}, {"empty supported versions", types.DefaultIBCVersion, []*types.Version{}, &types.Version{}, false}, - {"desired version is last", types.DefaultIBCVersion, []*types.Version{types.NewVersion("1", nil), types.NewVersion("2", []string{"ORDER_UNORDERED"}), types.NewVersion("3", nil)}, types.DefaultIBCVersion, true}, + {"desired version is last", types.DefaultIBCVersion, []*types.Version{types.NewVersion("1.1", nil), types.NewVersion("2", []string{"ORDER_UNORDERED"}), types.NewVersion("3", nil), types.DefaultIBCVersion}, types.DefaultIBCVersion, true}, {"desired version identifier with different feature set", types.NewVersion(types.DefaultIBCVersionIdentifier, []string{"ORDER_DAG"}), types.GetCompatibleVersions(), types.DefaultIBCVersion, true}, {"version not supported", types.NewVersion("2", []string{"ORDER_DAG"}), types.GetCompatibleVersions(), &types.Version{}, false}, } From 1c457eef962bc66b647d6167bcef8587d0eb8f43 Mon Sep 17 00:00:00 2001 From: Reece Williams Date: Mon, 17 Jul 2023 22:34:16 -0500 Subject: [PATCH 3/4] Fix accidental modified PickVersion test --- modules/core/03-connection/types/version_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/core/03-connection/types/version_test.go b/modules/core/03-connection/types/version_test.go index 1696f68308d..0d202ab886e 100644 --- a/modules/core/03-connection/types/version_test.go +++ b/modules/core/03-connection/types/version_test.go @@ -97,8 +97,8 @@ func TestPickVersion(t *testing.T) { expPass bool }{ {"valid default ibc version", types.GetCompatibleVersions(), types.GetCompatibleVersions(), types.DefaultIBCVersion, true}, - {"valid version in counterparty versions", []*types.Version{types.NewVersion("2.0.0", []string{"ORDER_UNORDERED-ZK"})}, []*types.Version{types.NewVersion("version1", nil), types.NewVersion("2.0.0", []string{"ORDER_UNORDERED-ZK"})}, types.DefaultIBCVersion, true}, - {"valid identifier match but empty feature set not allowed", types.GetCompatibleVersions(), []*types.Version{types.NewVersion(types.DefaultIBCVersionIdentifier, []string{"DAG", "ORDERED-ZK", "UNORDERED-zk]"})}, types.NewVersion(types.DefaultIBCVersionIdentifier, nil), false}, + {"valid version in counterparty versions", types.GetCompatibleVersions(), []*types.Version{types.NewVersion("version1", nil), types.NewVersion("2.0.0", []string{"ORDER_UNORDERED-ZK"}), types.DefaultIBCVersion}, types.DefaultIBCVersion, true}, + {"valid identifier match but empty feature set not allowed", types.GetCompatibleVersions(), []*types.Version{types.NewVersion(types.DefaultIBCVersionIdentifier, []string{"DAG", "ORDERED-ZK", "UNORDERED-zk]"}), types.DefaultIBCVersion}, types.DefaultIBCVersion, false}, {"empty counterparty versions", types.GetCompatibleVersions(), []*types.Version{}, &types.Version{}, false}, {"non-matching counterparty versions", types.GetCompatibleVersions(), []*types.Version{types.NewVersion("2.0.0", nil)}, &types.Version{}, false}, {"non-matching counterparty versions (uses ordered channels only) contained in supported versions (uses unordered channels only)", []*types.Version{types.NewVersion(types.DefaultIBCVersionIdentifier, []string{"ORDER_UNORDERED"})}, []*types.Version{types.NewVersion(types.DefaultIBCVersionIdentifier, []string{"ORDER_ORDERED"})}, &types.Version{}, false}, From 13e7becbfdad8226fb31e34ee5e211dff73ab76d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 27 Jul 2023 13:35:36 +0200 Subject: [PATCH 4/4] review: apply requested changes --- modules/core/03-connection/types/version.go | 9 ++------- modules/core/03-connection/types/version_test.go | 4 ++-- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/modules/core/03-connection/types/version.go b/modules/core/03-connection/types/version.go index 3d154715e5c..6efcdb76569 100644 --- a/modules/core/03-connection/types/version.go +++ b/modules/core/03-connection/types/version.go @@ -113,12 +113,7 @@ func VerifySupportedFeature(version *Version, feature string) bool { // version should be first element and the set should descend to the oldest // supported version. func GetCompatibleVersions() []*Version { - return []*Version{ - { - Identifier: DefaultIBCVersionIdentifier, - Features: SupportedOrderings, - }, - } + return []*Version{DefaultIBCVersion} } // IsSupportedVersion returns true if the proposed version has a matching version @@ -147,7 +142,7 @@ func FindSupportedVersion(version *Version, supportedVersions []*Version) (*Vers } } - return &Version{}, false + return nil, false } // PickVersion iterates over the descending ordered set of compatible IBC diff --git a/modules/core/03-connection/types/version_test.go b/modules/core/03-connection/types/version_test.go index 0d202ab886e..9830781ddaf 100644 --- a/modules/core/03-connection/types/version_test.go +++ b/modules/core/03-connection/types/version_test.go @@ -83,7 +83,7 @@ func TestFindSupportedVersion(t *testing.T) { require.True(t, found, "test case %d: %s", i, tc.name) } else { require.False(t, found, "test case: %s", tc.name) - require.Equal(t, version, &types.Version{}, "test case: %s", tc.name) + require.Nil(t, version, "test case: %s", tc.name) } } } @@ -98,7 +98,7 @@ func TestPickVersion(t *testing.T) { }{ {"valid default ibc version", types.GetCompatibleVersions(), types.GetCompatibleVersions(), types.DefaultIBCVersion, true}, {"valid version in counterparty versions", types.GetCompatibleVersions(), []*types.Version{types.NewVersion("version1", nil), types.NewVersion("2.0.0", []string{"ORDER_UNORDERED-ZK"}), types.DefaultIBCVersion}, types.DefaultIBCVersion, true}, - {"valid identifier match but empty feature set not allowed", types.GetCompatibleVersions(), []*types.Version{types.NewVersion(types.DefaultIBCVersionIdentifier, []string{"DAG", "ORDERED-ZK", "UNORDERED-zk]"}), types.DefaultIBCVersion}, types.DefaultIBCVersion, false}, + {"valid identifier match but empty feature set not allowed", types.GetCompatibleVersions(), []*types.Version{types.NewVersion(types.DefaultIBCVersionIdentifier, []string{"DAG", "ORDERED-ZK", "UNORDERED-zk]"})}, types.NewVersion(types.DefaultIBCVersionIdentifier, nil), false}, {"empty counterparty versions", types.GetCompatibleVersions(), []*types.Version{}, &types.Version{}, false}, {"non-matching counterparty versions", types.GetCompatibleVersions(), []*types.Version{types.NewVersion("2.0.0", nil)}, &types.Version{}, false}, {"non-matching counterparty versions (uses ordered channels only) contained in supported versions (uses unordered channels only)", []*types.Version{types.NewVersion(types.DefaultIBCVersionIdentifier, []string{"ORDER_UNORDERED"})}, []*types.Version{types.NewVersion(types.DefaultIBCVersionIdentifier, []string{"ORDER_ORDERED"})}, &types.Version{}, false},