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.

Sign up for GitHub

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

Jump to bottom

Add sentinel localhost connection end to genesis state #3040

Merged
merged 11 commits into from
Jan 30, 2023
Merged

Add sentinel localhost connection end to genesis state #3040

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
373075c
chore: writing localhost connection end to state in InitGenesis
chatton Jan 23, 2023
4760dbc
test: adding test to ensure sentinel connection end is created
chatton Jan 23, 2023
3192280
chore: added migration function to enable localhost connection end
chatton Jan 23, 2023
4b021db
chore: fix linting issues
chatton Jan 24, 2023
68a57fb
chore: remove boolean field to specify create localhost and automatic…
chatton Jan 24, 2023
9b1ca7b
chore: updated unit tests to account for new localhost connection end
chatton Jan 24, 2023
d10b790
chore: adding docstring
chatton Jan 24, 2023
96732cb
chore: addressing PR feedback
chatton Jan 25, 2023
ee5b0b1
chore: merge feature branch
chatton Jan 25, 2023
926a026
chore: ran linter
chatton Jan 25, 2023
bda0bce
chore: performing multiple migrations in module.go
chatton Jan 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions modules/core/03-connection/genesis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, gs types.GenesisState) {
}
k.SetNextConnectionSequence(ctx, gs.NextConnectionSequence)
k.SetParams(ctx, gs.Params)

localhostConnection := k.CreateSentinelLocalhostConnection()
k.SetConnection(ctx, types.LocalhostID, localhostConnection)
}

// ExportGenesis returns the ibc connection submodule's exported genesis.
Expand Down
9 changes: 6 additions & 3 deletions modules/core/03-connection/keeper/grpc_query_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,12 @@ func (suite *KeeperTestSuite) TestQueryConnection() {
}

func (suite *KeeperTestSuite) TestQueryConnections() {
localhostConnectionEnd := suite.chainA.App.GetIBCKeeper().ConnectionKeeper.CreateSentinelLocalhostConnection()
localhostConn := types.NewIdentifiedConnection(types.LocalhostID, localhostConnectionEnd)

var (
req *types.QueryConnectionsRequest
expConnections = []*types.IdentifiedConnection{}
expConnections = []*types.IdentifiedConnection{&localhostConn}
)

testCases := []struct {
Expand Down Expand Up @@ -137,11 +140,11 @@ func (suite *KeeperTestSuite) TestQueryConnections() {
iconn2 := types.NewIdentifiedConnection(path2.EndpointA.ConnectionID, conn2)
iconn3 := types.NewIdentifiedConnection(path3.EndpointA.ConnectionID, conn3)

expConnections = []*types.IdentifiedConnection{&iconn1, &iconn2, &iconn3}
expConnections = []*types.IdentifiedConnection{&iconn1, &iconn2, &iconn3, &localhostConn}

req = &types.QueryConnectionsRequest{
Pagination: &query.PageRequest{
Limit: 3,
Limit: 4,
CountTotal: true,
},
}
Expand Down
6 changes: 6 additions & 0 deletions modules/core/03-connection/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,12 @@ func (k Keeper) GetAllConnections(ctx sdk.Context) (connections []types.Identifi
return connections
}

// CreateSentinelLocalhostConnection returns the sentinel localhost connection end.
func (k Keeper) CreateSentinelLocalhostConnection() types.ConnectionEnd {
counterparty := types.NewCounterparty(exported.Localhost, types.LocalhostID, commitmenttypes.NewMerklePrefix(k.GetCommitmentPrefix().Bytes()))
return types.NewConnectionEnd(types.OPEN, exported.Localhost, counterparty, types.ExportedVersionsToProto(types.GetCompatibleVersions()), 0)
}

// addConnectionToClient is used to add a connection identifier to the set of
// connections associated with a client.
func (k Keeper) addConnectionToClient(ctx sdk.Context, clientID, connectionID string) error {
Expand Down
22 changes: 21 additions & 1 deletion modules/core/03-connection/keeper/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/stretchr/testify/suite"

"github.com/cosmos/ibc-go/v7/modules/core/03-connection/types"
commitmenttypes "github.com/cosmos/ibc-go/v7/modules/core/23-commitment/types"
"github.com/cosmos/ibc-go/v7/modules/core/exported"
ibctesting "github.com/cosmos/ibc-go/v7/testing"
)
Expand Down Expand Up @@ -79,7 +80,10 @@ func (suite KeeperTestSuite) TestGetAllConnections() { //nolint:govet // this is
iconn1 := types.NewIdentifiedConnection(path1.EndpointA.ConnectionID, conn1)
iconn2 := types.NewIdentifiedConnection(path2.EndpointA.ConnectionID, conn2)

expConnections := []types.IdentifiedConnection{iconn1, iconn2}
localhostConnectionEnd := suite.chainA.App.GetIBCKeeper().ConnectionKeeper.CreateSentinelLocalhostConnection()
localhostConn := types.NewIdentifiedConnection(types.LocalhostID, localhostConnectionEnd)

expConnections := []types.IdentifiedConnection{iconn1, iconn2, localhostConn}

connections := suite.chainA.App.GetIBCKeeper().ConnectionKeeper.GetAllConnections(suite.chainA.GetContext())
suite.Require().Len(connections, len(expConnections))
Expand Down Expand Up @@ -156,3 +160,19 @@ func (suite *KeeperTestSuite) TestGetTimestampAtHeight() {
})
}
}

func (suite *KeeperTestSuite) TestLocalhostConnectionEndCreation() {
ctx := suite.chainA.GetContext()
connectionKeeper := suite.chainA.App.GetIBCKeeper().ConnectionKeeper
localhostConnection := connectionKeeper.CreateSentinelLocalhostConnection()
connectionKeeper.SetConnection(ctx, types.LocalhostID, localhostConnection)

connectionEnd, found := connectionKeeper.GetConnection(ctx, types.LocalhostID)

suite.Require().True(found)
suite.Require().Equal(types.OPEN, connectionEnd.State)
suite.Require().Equal(exported.Localhost, connectionEnd.ClientId)
suite.Require().Equal(types.ExportedVersionsToProto(types.GetCompatibleVersions()), connectionEnd.Versions)
expectedCounterParty := types.NewCounterparty(exported.Localhost, types.LocalhostID, commitmenttypes.NewMerklePrefix(connectionKeeper.GetCommitmentPrefix().Bytes()))
suite.Require().Equal(expectedCounterParty, connectionEnd.Counterparty)
}
24 changes: 24 additions & 0 deletions modules/core/03-connection/keeper/migrations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package keeper

import (
sdk "github.com/cosmos/cosmos-sdk/types"

connectionv7 "github.com/cosmos/ibc-go/v7/modules/core/03-connection/migrations/v7"
)

// Migrator is a struct for handling in-place store migrations.
type Migrator struct {
keeper Keeper
}

// NewMigrator returns a new Migrator.
func NewMigrator(keeper Keeper) Migrator {
return Migrator{keeper: keeper}
}

// Migrate3to4 migrates from version 3 to 4.
// This migration writes the sentinel localhost connection end to state.
func (m Migrator) Migrate3to4(ctx sdk.Context) error {
connectionv7.MigrateLocalhostConnection(ctx, m.keeper)
return nil
}
13 changes: 13 additions & 0 deletions modules/core/03-connection/migrations/v7/expected_keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package v7

import (
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/cosmos/ibc-go/v7/modules/core/03-connection/types"
)

// ConnectionKeeper expected IBC connection keeper
type ConnectionKeeper interface {
CreateSentinelLocalhostConnection() types.ConnectionEnd
SetConnection(ctx sdk.Context, connectionID string, connection types.ConnectionEnd)
}
14 changes: 14 additions & 0 deletions modules/core/03-connection/migrations/v7/localhost.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package v7

import (
sdk "github.com/cosmos/cosmos-sdk/types"

connectiontypes "github.com/cosmos/ibc-go/v7/modules/core/03-connection/types"
)

// MigrateLocalhostConnection creates the sentinel localhost connection end to enable
// localhost ibc functionality.
func MigrateLocalhostConnection(ctx sdk.Context, connectionKeeper ConnectionKeeper) {
localhostConnection := connectionKeeper.CreateSentinelLocalhostConnection()
connectionKeeper.SetConnection(ctx, connectiontypes.LocalhostID, localhostConnection)
}
13 changes: 9 additions & 4 deletions modules/core/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
ibcclient "github.com/cosmos/ibc-go/v7/modules/core/02-client"
clientkeeper "github.com/cosmos/ibc-go/v7/modules/core/02-client/keeper"
clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
connectionkeeper "github.com/cosmos/ibc-go/v7/modules/core/03-connection/keeper"
connectiontypes "github.com/cosmos/ibc-go/v7/modules/core/03-connection/types"
channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
"github.com/cosmos/ibc-go/v7/modules/core/client/cli"
Expand Down Expand Up @@ -124,9 +125,13 @@ func (am AppModule) RegisterServices(cfg module.Configurator) {
channeltypes.RegisterMsgServer(cfg.MsgServer(), am.keeper)
types.RegisterQueryService(cfg.QueryServer(), am.keeper)

m := clientkeeper.NewMigrator(am.keeper.ClientKeeper)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this is due to my lack of knowledge of how migrations work, but if we replace Migrate2to3 with Migration3to4 and a chain upgrades from v6.0.0 to v7.1.0, wouldn't it miss the Migrate2to3 migration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, my understanding was that chains needed to update to each version one by one in order to run all the migrations. I feel like it should be possible, to just register all migrations at once (as they should be idempotent) however I'm not sure if this is the case. Maybe @AdityaSripal has an answer to this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I am also not sure if the chains really need to upgrade to each version of ibc-go or if they can jump versions as long as they run the migrations of the versions in between.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if this is the case then we would preferably just add it as a migration instead of replace

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it should be possible to register multiple migrations.

err := cfg.RegisterMigration(exported.ModuleName, 2, m.Migrate2to3)
if err != nil {
clientMigrator := clientkeeper.NewMigrator(am.keeper.ClientKeeper)
if err := cfg.RegisterMigration(exported.ModuleName, 2, clientMigrator.Migrate2to3); err != nil {
panic(err)
}

connectionMigrator := connectionkeeper.NewMigrator(am.keeper.ConnectionKeeper)
if err := cfg.RegisterMigration(exported.ModuleName, 3, connectionMigrator.Migrate3to4); err != nil {
panic(err)
}
}
Expand All @@ -150,7 +155,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw
}

// ConsensusVersion implements AppModule/ConsensusVersion.
func (AppModule) ConsensusVersion() uint64 { return 3 }
func (AppModule) ConsensusVersion() uint64 { return 4 }

// BeginBlock returns the begin blocker for the ibc module.
func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {
Expand Down