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

Remove Version interface and casting functions from 03-connection #4110

Merged
2 changes: 1 addition & 1 deletion docs/ibc/light-clients/localhost/connection.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
8 changes: 4 additions & 4 deletions modules/core/03-connection/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,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{
Expand Down Expand Up @@ -134,9 +134,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)
Expand Down
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 @@ -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)
Expand All @@ -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())
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
12 changes: 6 additions & 6 deletions modules/core/03-connection/keeper/handshake_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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
)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion modules/core/03-connection/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
6 changes: 3 additions & 3 deletions modules/core/03-connection/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion modules/core/03-connection/simulation/decoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
5 changes: 0 additions & 5 deletions modules/core/03-connection/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
Expand Down
4 changes: 2 additions & 2 deletions modules/core/03-connection/types/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
40 changes: 8 additions & 32 deletions modules/core/03-connection/types/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -115,14 +112,14 @@ 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{DefaultIBCVersion}
}

// 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
Expand All @@ -138,12 +135,13 @@ 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
}

Expand All @@ -158,7 +156,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 {
Expand Down Expand Up @@ -190,25 +188,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
}
23 changes: 11 additions & 12 deletions modules/core/03-connection/types/version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -41,7 +40,7 @@ func TestIsSupportedVersion(t *testing.T) {
}{
{
"version is supported",
types.ExportedVersionsToProto(types.GetCompatibleVersions())[0],
types.GetCompatibleVersions()[0],
true,
},
{
Expand All @@ -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.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},
}
Expand All @@ -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.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.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 {
Expand Down
Loading