Skip to content

Commit

Permalink
imp: remove Version interface and casting functions from 03-connection
Browse files Browse the repository at this point in the history
  • Loading branch information
Reecepbcups authored Jul 28, 2023
1 parent 4e1e594 commit 0bce6b5
Show file tree
Hide file tree
Showing 15 changed files with 45 additions and 82 deletions.
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

0 comments on commit 0bce6b5

Please sign in to comment.