From cd382e8af3e644bbfff6f0bad506d7df1a866652 Mon Sep 17 00:00:00 2001 From: Inkvi Date: Fri, 12 Jul 2024 12:33:12 -0700 Subject: [PATCH] Use hardcoded client prefixes A client can exist without connections hence relying on connections is not a fully valid approach --- modules/core/02-client/genesis.go | 8 +-- modules/core/02-client/keeper/keeper.go | 58 +++++++++++++------ modules/core/02-client/keeper/keeper_test.go | 9 ++- .../02-client/migrations/v7/genesis_test.go | 4 +- modules/core/02-client/types/genesis_test.go | 2 +- modules/core/03-connection/keeper/keeper.go | 38 +++++------- modules/core/genesis.go | 2 +- modules/core/migrations/v7/genesis_test.go | 4 +- 8 files changed, 70 insertions(+), 55 deletions(-) diff --git a/modules/core/02-client/genesis.go b/modules/core/02-client/genesis.go index 90848906870..904870bf443 100644 --- a/modules/core/02-client/genesis.go +++ b/modules/core/02-client/genesis.go @@ -4,8 +4,6 @@ import ( "fmt" sdk "github.com/cosmos/cosmos-sdk/types" - connectionkeeper "github.com/cosmos/ibc-go/v7/modules/core/03-connection/keeper" - "github.com/cosmos/ibc-go/v7/modules/core/02-client/keeper" "github.com/cosmos/ibc-go/v7/modules/core/02-client/types" "github.com/cosmos/ibc-go/v7/modules/core/exported" @@ -56,8 +54,8 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, gs types.GenesisState) { } // ExportGenesis returns the ibc client submodule's exported genesis. -func ExportGenesis(ctx sdk.Context, clientKeeper keeper.Keeper, connectionKeeper connectionkeeper.Keeper) types.GenesisState { - genClients := clientKeeper.GetAllGenesisClients(ctx, connectionKeeper) +func ExportGenesis(ctx sdk.Context, clientKeeper keeper.Keeper) types.GenesisState { + genClients := clientKeeper.GetAllGenesisClients(ctx) clientsMetadata, err := clientKeeper.GetAllClientMetadata(ctx, genClients) if err != nil { panic(err) @@ -65,7 +63,7 @@ func ExportGenesis(ctx sdk.Context, clientKeeper keeper.Keeper, connectionKeeper return types.GenesisState{ Clients: genClients, ClientsMetadata: clientsMetadata, - ClientsConsensus: clientKeeper.GetAllConsensusStates(ctx, connectionKeeper), + ClientsConsensus: clientKeeper.GetAllConsensusStates(ctx), Params: clientKeeper.GetParams(ctx), // Warning: CreateLocalhost is deprecated CreateLocalhost: false, diff --git a/modules/core/02-client/keeper/keeper.go b/modules/core/02-client/keeper/keeper.go index a872505df7f..7f93d62c934 100644 --- a/modules/core/02-client/keeper/keeper.go +++ b/modules/core/02-client/keeper/keeper.go @@ -14,8 +14,6 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" - connectionkeeper "github.com/cosmos/ibc-go/v7/modules/core/03-connection/keeper" - "github.com/cosmos/ibc-go/v7/modules/core/02-client/types" commitmenttypes "github.com/cosmos/ibc-go/v7/modules/core/23-commitment/types" host "github.com/cosmos/ibc-go/v7/modules/core/24-host" @@ -24,6 +22,9 @@ import ( localhost "github.com/cosmos/ibc-go/v7/modules/light-clients/09-localhost" ) +// ClientPrefixes is a list of all client prefixes used in the IBC module +var ClientPrefixes = []string{"opstack", "sim-test", "optimism", "tendermint", exported.Solomachine, exported.Tendermint, exported.Localhost, exported.Virtual} + // Keeper represents a type that grants read and write permissions to any client // state information type Keeper struct { @@ -35,7 +36,13 @@ type Keeper struct { } // NewKeeper creates a new NewKeeper instance -func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, paramSpace paramtypes.Subspace, sk types.StakingKeeper, uk types.UpgradeKeeper) Keeper { +func NewKeeper( + cdc codec.BinaryCodec, + key storetypes.StoreKey, + paramSpace paramtypes.Subspace, + sk types.StakingKeeper, + uk types.UpgradeKeeper, +) Keeper { // set KeyTable if it has not already been set if !paramSpace.HasKeyTable() { paramSpace = paramSpace.WithKeyTable(types.ParamKeyTable()) @@ -95,7 +102,11 @@ func (k Keeper) SetClientState(ctx sdk.Context, clientID string, clientState exp } // GetClientConsensusState gets the stored consensus state from a client at a given height. -func (k Keeper) GetClientConsensusState(ctx sdk.Context, clientID string, height exported.Height) (exported.ConsensusState, bool) { +func (k Keeper) GetClientConsensusState( + ctx sdk.Context, + clientID string, + height exported.Height, +) (exported.ConsensusState, bool) { store := k.ClientStore(ctx, clientID) bz := store.Get(host.ConsensusStateKey(height)) if len(bz) == 0 { @@ -108,7 +119,12 @@ func (k Keeper) GetClientConsensusState(ctx sdk.Context, clientID string, height // SetClientConsensusState sets a ConsensusState to a particular client at the given // height -func (k Keeper) SetClientConsensusState(ctx sdk.Context, clientID string, height exported.Height, consensusState exported.ConsensusState) { +func (k Keeper) SetClientConsensusState( + ctx sdk.Context, + clientID string, + height exported.Height, + consensusState exported.ConsensusState, +) { store := k.ClientStore(ctx, clientID) store.Set(host.ConsensusStateKey(height), k.MustMarshalConsensusState(consensusState)) } @@ -134,7 +150,11 @@ func (k Keeper) SetNextClientSequence(ctx sdk.Context, sequence uint64) { // IterateConsensusStates provides an iterator over all stored consensus states. // objects. For each State object, cb will be called. If the cb returns true, // the iterator will close and stop. -func (k Keeper) IterateConsensusStates(ctx sdk.Context, prefix []byte, cb func(clientID string, cs types.ConsensusStateWithHeight) bool) { +func (k Keeper) IterateConsensusStates( + ctx sdk.Context, + prefix []byte, + cb func(clientID string, cs types.ConsensusStateWithHeight) bool, +) { store := ctx.KVStore(k.storeKey) iterator := sdk.KVStorePrefixIterator(store, prefix) @@ -158,12 +178,10 @@ func (k Keeper) IterateConsensusStates(ctx sdk.Context, prefix []byte, cb func(c } // GetAllGenesisClients returns all the clients in state with their client ids returned as IdentifiedClientState -func (k Keeper) GetAllGenesisClients(ctx sdk.Context, connectionKeeper connectionkeeper.Keeper) types.IdentifiedClientStates { - clientIds := connectionKeeper.GetAllClientIds(ctx) - +func (k Keeper) GetAllGenesisClients(ctx sdk.Context) types.IdentifiedClientStates { var genClients types.IdentifiedClientStates - for _, clientID := range clientIds { - k.IterateClientStates(ctx, []byte(clientID), func(clientID string, cs exported.ClientState) bool { + for _, clientPrefix := range ClientPrefixes { + k.IterateClientStates(ctx, []byte(clientPrefix), func(clientID string, cs exported.ClientState) bool { genClients = append(genClients, types.NewIdentifiedClientState(clientID, cs)) return false }) @@ -175,7 +193,10 @@ func (k Keeper) GetAllGenesisClients(ctx sdk.Context, connectionKeeper connectio // GetAllClientMetadata will take a list of IdentifiedClientState and return a list // of IdentifiedGenesisMetadata necessary for exporting and importing client metadata // into the client store. -func (k Keeper) GetAllClientMetadata(ctx sdk.Context, genClients []types.IdentifiedClientState) ([]types.IdentifiedGenesisMetadata, error) { +func (k Keeper) GetAllClientMetadata( + ctx sdk.Context, + genClients []types.IdentifiedClientState, +) ([]types.IdentifiedGenesisMetadata, error) { genMetadata := make([]types.IdentifiedGenesisMetadata, 0) for _, ic := range genClients { cs, err := types.UnpackClientState(ic.ClientState) @@ -216,13 +237,12 @@ func (k Keeper) SetAllClientMetadata(ctx sdk.Context, genMetadata []types.Identi } // GetAllConsensusStates returns all stored client consensus states. -func (k Keeper) GetAllConsensusStates(ctx sdk.Context, connectionKeeper connectionkeeper.Keeper) types.ClientsConsensusStates { +func (k Keeper) GetAllConsensusStates(ctx sdk.Context) types.ClientsConsensusStates { clientConsStates := make(types.ClientsConsensusStates, 0) mapClientIDToConsStateIdx := make(map[string]int) - clientIds := connectionKeeper.GetAllClientIds(ctx) - for _, _clientId := range clientIds { - prefix := host.PrefixedClientStoreKey([]byte(_clientId)) + for _, clientPrefix := range ClientPrefixes { + prefix := host.PrefixedClientStoreKey([]byte(clientPrefix)) k.IterateConsensusStates(ctx, prefix, func(clientID string, cs types.ConsensusStateWithHeight) bool { idx, ok := mapClientIDToConsStateIdx[clientID] if ok { @@ -374,7 +394,11 @@ func (k Keeper) SetUpgradedConsensusState(ctx sdk.Context, planHeight int64, bz // IterateClientStates provides an iterator over all stored light client State // objects. For each State object, cb will be called. If the cb returns true, // the iterator will close and stop. -func (k Keeper) IterateClientStates(ctx sdk.Context, prefix []byte, cb func(clientID string, cs exported.ClientState) bool) { +func (k Keeper) IterateClientStates( + ctx sdk.Context, + prefix []byte, + cb func(clientID string, cs exported.ClientState) bool, +) { store := ctx.KVStore(k.storeKey) iterator := sdk.KVStorePrefixIterator(store, host.PrefixedClientStoreKey(prefix)) diff --git a/modules/core/02-client/keeper/keeper_test.go b/modules/core/02-client/keeper/keeper_test.go index 644dc0e8fdc..e87805f5d56 100644 --- a/modules/core/02-client/keeper/keeper_test.go +++ b/modules/core/02-client/keeper/keeper_test.go @@ -253,7 +253,7 @@ func (suite KeeperTestSuite) TestGetAllGenesisClients() { //nolint:govet // this expGenClients[i] = types.NewIdentifiedClientState(clientIDs[i], expClients[i]) } - genClients := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetAllGenesisClients(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ConnectionKeeper) + genClients := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetAllGenesisClients(suite.chainA.GetContext()) suite.Require().Equal(expGenClients.Sort(), genClients) } @@ -385,7 +385,7 @@ func (suite KeeperTestSuite) TestGetAllConsensusStates() { //nolint:govet // thi }), }.Sort() - consStates := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetAllConsensusStates(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ConnectionKeeper) + consStates := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetAllConsensusStates(suite.chainA.GetContext()) suite.Require().Equal(expConsensusStates, consStates, "%s \n\n%s", expConsensusStates, consStates) } @@ -452,7 +452,10 @@ func (suite KeeperTestSuite) TestIterateClientStates() { //nolint:govet // this tc := tc suite.Run(tc.name, func() { var clientIDs []string - suite.chainA.GetSimApp().IBCKeeper.ClientKeeper.IterateClientStates(suite.chainA.GetContext(), tc.prefix, func(clientID string, _ exported.ClientState) bool { + suite.chainA.GetSimApp().IBCKeeper.ClientKeeper.IterateClientStates(suite.chainA.GetContext(), tc.prefix, func( + clientID string, + _ exported.ClientState, + ) bool { clientIDs = append(clientIDs, clientID) return false }) diff --git a/modules/core/02-client/migrations/v7/genesis_test.go b/modules/core/02-client/migrations/v7/genesis_test.go index f5c7d619d4c..79e285ba58a 100644 --- a/modules/core/02-client/migrations/v7/genesis_test.go +++ b/modules/core/02-client/migrations/v7/genesis_test.go @@ -33,7 +33,7 @@ func (suite *MigrationsV7TestSuite) TestMigrateGenesisSolomachine() { solomachine := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "06-solomachine-0", "testing", 1) solomachineMulti := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "06-solomachine-1", "testing", 4) - clientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper, suite.chainA.App.GetIBCKeeper().ConnectionKeeper) + clientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper) // manually generate old proto buf definitions and set in genesis // NOTE: we cannot use 'ExportGenesis' for the solo machines since we are @@ -108,7 +108,7 @@ func (suite *MigrationsV7TestSuite) TestMigrateGenesisSolomachine() { // NOTE: tendermint clients are not pruned in genesis so the test should not have expired tendermint clients err := v7.MigrateStore(suite.chainA.GetContext(), suite.chainA.GetSimApp().GetKey(ibcexported.StoreKey), suite.chainA.App.AppCodec(), suite.chainA.GetSimApp().IBCKeeper.ClientKeeper) suite.Require().NoError(err) - expectedClientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper, suite.chainA.App.GetIBCKeeper().ConnectionKeeper) + expectedClientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper) cdc, ok := suite.chainA.App.AppCodec().(codec.ProtoCodecMarshaler) suite.Require().True(ok) diff --git a/modules/core/02-client/types/genesis_test.go b/modules/core/02-client/types/genesis_test.go index 1139bf0eed3..41254d148e3 100644 --- a/modules/core/02-client/types/genesis_test.go +++ b/modules/core/02-client/types/genesis_test.go @@ -34,7 +34,7 @@ func (suite *TypesTestSuite) TestMarshalGenesisState() { err := path.EndpointA.UpdateClient() suite.Require().NoError(err) - genesis := client.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper, suite.chainA.App.GetIBCKeeper().ConnectionKeeper) + genesis := client.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper) bz, err := cdc.MarshalJSON(&genesis) suite.Require().NoError(err) diff --git a/modules/core/03-connection/keeper/keeper.go b/modules/core/03-connection/keeper/keeper.go index 7b360620b73..6ad2a0aafa2 100644 --- a/modules/core/03-connection/keeper/keeper.go +++ b/modules/core/03-connection/keeper/keeper.go @@ -7,6 +7,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" + clientkeeper "github.com/cosmos/ibc-go/v7/modules/core/02-client/keeper" clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types" "github.com/cosmos/ibc-go/v7/modules/core/03-connection/types" @@ -27,7 +28,12 @@ type Keeper struct { } // NewKeeper creates a new IBC connection Keeper instance -func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, paramSpace paramtypes.Subspace, ck types.ClientKeeper) Keeper { +func NewKeeper( + cdc codec.BinaryCodec, + key storetypes.StoreKey, + paramSpace paramtypes.Subspace, + ck types.ClientKeeper, +) Keeper { // set KeyTable if it has not already been set if !paramSpace.HasKeyTable() { paramSpace = paramSpace.WithKeyTable(types.ParamKeyTable()) @@ -92,7 +98,11 @@ func (k Keeper) SetConnection(ctx sdk.Context, connectionID string, connection t // GetTimestampAtHeight returns the timestamp in nanoseconds of the consensus state at the // given height. -func (k Keeper) GetTimestampAtHeight(ctx sdk.Context, connection types.ConnectionEnd, height exported.Height) (uint64, error) { +func (k Keeper) GetTimestampAtHeight( + ctx sdk.Context, + connection types.ConnectionEnd, + height exported.Height, +) (uint64, error) { clientState, found := k.clientKeeper.GetClientState(ctx, connection.GetClientID()) if !found { return 0, sdkerrors.Wrapf( @@ -152,12 +162,9 @@ func (k Keeper) SetNextConnectionSequence(ctx sdk.Context, sequence uint64) { // will ignore the clients that haven't initialized a connection handshake since // no paths are stored. func (k Keeper) GetAllClientConnectionPaths(ctx sdk.Context) []types.ConnectionPaths { - clientIds := k.GetAllClientIds(ctx) - var allConnectionPaths []types.ConnectionPaths - for _, _clientId := range clientIds { - prefix := host.PrefixedClientStoreKey([]byte(_clientId)) - k.clientKeeper.IterateClientStates(ctx, prefix, func(clientID string, cs exported.ClientState) bool { + for _, clientPrefix := range clientkeeper.ClientPrefixes { + k.clientKeeper.IterateClientStates(ctx, []byte(clientPrefix), func(clientID string, cs exported.ClientState) bool { paths, found := k.GetClientConnectionPaths(ctx, clientID) if !found { // continue when connection handshake is not initialized @@ -172,23 +179,6 @@ func (k Keeper) GetAllClientConnectionPaths(ctx sdk.Context) []types.ConnectionP return allConnectionPaths } -func (k Keeper) GetAllClientIds(ctx sdk.Context) []string { - connections := k.GetAllConnections(ctx) - clientIds := make([]string, 0) - clientIdSet := make(map[string]bool) // To avoid duplicate client IDs - - for _, connection := range connections { - if connection.ClientId == "polymer-0" { - continue - } - if !clientIdSet[connection.ClientId] { - clientIdSet[connection.ClientId] = true - clientIds = append(clientIds, connection.ClientId) - } - } - return clientIds -} - // IterateConnections provides an iterator over all ConnectionEnd objects. // For each ConnectionEnd, cb will be called. If the cb returns true, the // iterator will close and stop. diff --git a/modules/core/genesis.go b/modules/core/genesis.go index 790372f15ff..42661193665 100644 --- a/modules/core/genesis.go +++ b/modules/core/genesis.go @@ -21,7 +21,7 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, gs *types.GenesisState) { // ExportGenesis returns the ibc exported genesis. func ExportGenesis(ctx sdk.Context, k keeper.Keeper) *types.GenesisState { return &types.GenesisState{ - ClientGenesis: client.ExportGenesis(ctx, k.ClientKeeper, k.ConnectionKeeper), + ClientGenesis: client.ExportGenesis(ctx, k.ClientKeeper), ConnectionGenesis: connection.ExportGenesis(ctx, k.ConnectionKeeper), ChannelGenesis: channel.ExportGenesis(ctx, k.ChannelKeeper), } diff --git a/modules/core/migrations/v7/genesis_test.go b/modules/core/migrations/v7/genesis_test.go index a1a0c0d3a32..1a040534ff8 100644 --- a/modules/core/migrations/v7/genesis_test.go +++ b/modules/core/migrations/v7/genesis_test.go @@ -60,7 +60,7 @@ func (suite *MigrationsV7TestSuite) TestMigrateGenesisSolomachine() { solomachine := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "06-solomachine-0", "testing", 1) solomachineMulti := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "06-solomachine-1", "testing", 4) - clientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper, suite.chainA.App.GetIBCKeeper().ConnectionKeeper) + clientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper) // manually generate old proto buf definitions and set in genesis // NOTE: we cannot use 'ExportGenesis' for the solo machines since we are @@ -134,7 +134,7 @@ func (suite *MigrationsV7TestSuite) TestMigrateGenesisSolomachine() { // NOTE: tendermint clients are not pruned in genesis so the test should not have expired tendermint clients err := clientv7.MigrateStore(suite.chainA.GetContext(), suite.chainA.GetSimApp().GetKey(ibcexported.StoreKey), suite.chainA.App.AppCodec(), suite.chainA.GetSimApp().IBCKeeper.ClientKeeper) suite.Require().NoError(err) - expectedClientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper, suite.chainA.App.GetIBCKeeper().ConnectionKeeper) + expectedClientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper) cdc := suite.chainA.App.AppCodec().(*codec.ProtoCodec)