feat(statemachine)!: Add allow all client wildcard to AllowedClients …

…param (#5429)

* add wildcard allow all client

* minor

* fix lint

* minor and doc update

* review comments

* if allow all clients is present, no other client types should be in the allow list

* clean up code to allow wasm client type

* remove unused variable

* Fix build failures, tweak err message.

* modify allow clients list in genesis with feature releases

* chore: adding v8.1 to minor versions in e2e feat releases struct

* update docs

* chore: doc lint fixes

---------

Co-authored-by: GnaD <[email protected]>
Co-authored-by: Du Nguyen <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>
Co-authored-by: DimitrisJim <[email protected]>
Co-authored-by: Damian Nolan <[email protected]>
Co-authored-by: Đỗ Việt Hoàng <[email protected]>
Co-authored-by: Charly <[email protected]>

docs/docs/01-ibc/11-troubleshooting.md

@@ -12,4 +12,4 @@ slug: /ibc/troubleshooting
 If it is being reported that a client state is unauthorized, this is due to the client type not being present
 in the [`AllowedClients`](https://github.com/cosmos/ibc-go/blob/v6.0.0/modules/core/02-client/types/client.pb.go#L345) array.
 
-Unless the client type is present in this array, all usage of clients of this type will be prevented.
+Unless the client type is present in this array or the `AllowAllClients` wildcard (`"*"`) is used, all usage of clients of this type will be prevented.

docs/docs/03-light-clients/01-developer-guide/09-setup.md

@@ -131,3 +131,5 @@ where `proposal.json` contains:
   "deposit": "100stake"
 }
 ```
+
+If the `AllowedClients` list contains a single element that is equal to the wildcard `"*"`, then all client types are allowed and it is thus not necessary to submit a governance proposal to update the parameter.

docs/docs/03-light-clients/02-localhost/02-integration.md

@@ -10,11 +10,10 @@ slug: /ibc/light-clients/localhost/integration
 
 The 09-localhost light client module registers codec types within the core IBC module. This differs from other light client module implementations which are expected to register codec types using the `AppModuleBasic` interface.
 
-The localhost client is added to the 02-client submodule param [`allowed_clients`](https://github.com/cosmos/ibc-go/blob/v7.0.0/proto/ibc/core/client/v1/client.proto#L102) by default in ibc-go.
+The localhost client is implicitly enabled by using the `AllowAllClients` wildcard (`"*"`) in the 02-client submodule default value for param  [`allowed_clients`](https://github.com/cosmos/ibc-go/blob/v7.0.0/proto/ibc/core/client/v1/client.proto#L102).
 
 ```go
-var (
-  // DefaultAllowedClients are the default clients for the AllowedClients parameter.
-  DefaultAllowedClients = []string{exported.Solomachine, exported.Tendermint, exported.Localhost}
-)
+// DefaultAllowedClients are the default clients for the AllowedClients parameter.
+// By default it allows all client types.
+var DefaultAllowedClients = []string{AllowAllClients}
 ```

docs/docs/03-light-clients/04-wasm/03-integration.md

@@ -307,7 +307,7 @@ app.WasmClientKeeper = ibcwasmkeeper.NewKeeperWithVM(
 
 ## Updating `AllowedClients`
 
-In order to use the `08-wasm` module chains must update the [`AllowedClients` parameter in the 02-client submodule](https://github.com/cosmos/ibc-go/blob/v8.0.0/proto/ibc/core/client/v1/client.proto#L64) of core IBC. This can be configured directly in the application upgrade handler with the sample code below:
+If the chain's 02-client submodule parameter `AllowedClients` contains the single wildcard `"*"` element, then it is not necessary to do anything in order to allow the creation of `08-wasm` clients. However, if the parameter contains a list of client types (e.g. `["06-solomachine", "07-tendermint"]`), then in order to use the `08-wasm` module chains must update the [`AllowedClients` parameter](https://github.com/cosmos/ibc-go/blob/v8.0.0/proto/ibc/core/client/v1/client.proto#L64) of core IBC. This can be configured directly in the application upgrade handler with the sample code below:
 
 ```go
 import (

docs/params/params.md

@@ -8,14 +8,17 @@ slug: /params.md
 
 The 02-client submodule contains the following parameters:
 
-| Key              | Type     | Default Value                                     |
-| ---------------- | -------- | ------------------------------------------------- |
-| `AllowedClients` | []string | `"06-solomachine","07-tendermint","09-localhost"` |
+| Key              | Type     | Default Value |
+| ---------------- | -------- | ------------- |
+| `AllowedClients` | []string | `"*"`         |
 
 ### AllowedClients
 
-The allowed clients parameter defines an allowlist of client types supported by the chain. A client
-that is not registered on this list will fail upon creation or on genesis validation. Note that,
-since the client type is an arbitrary string, chains they must not register two light clients which
-return the same value for the `ClientType()` function, otherwise the allowlist check can be
+The allowed clients parameter defines an allow list of client types supported by the chain. The 
+default value is a single-element list containing the `AllowAllClients` wildcard (`"*"`). When the
+wilcard is used, then all client types are supported by default. Alternatively, the parameter
+may be set with a list of client types (e.g. `"06-solomachine","07-tendermint","09-localhost"`).
+A client type that is not registered on this list will fail upon creation or on genesis validation.
+Note that, since the client type is an arbitrary string, chains must not register two light clients
+which return the same value for the `ClientType()` function, otherwise the allow list check can be
 bypassed.

e2e/tests/wasm/grandpa_test.go

@@ -23,9 +23,6 @@ import (
 
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
-	paramsproposaltypes "github.com/cosmos/cosmos-sdk/x/params/types/proposal"
-
-	cmtjson "github.com/cometbft/cometbft/libs/json"
 
 	"github.com/cosmos/ibc-go/e2e/testsuite"
 	"github.com/cosmos/ibc-go/e2e/testvalues"
@@ -128,11 +125,6 @@ func (s *GrandpaTestSuite) TestMsgTransfer_Succeeds_GrandpaContract() {
 	err = r.GeneratePath(ctx, eRep, cosmosChain.Config().ChainID, polkadotChain.Config().ChainID, pathName)
 	s.Require().NoError(err)
 
-	t.Run("add 08-wasm to list of allowed clients", func(t *testing.T) {
-		allowedClients := s.queryAllowedClientsParam(ctx, cosmosChain)
-		s.allowWasmClients(ctx, cosmosChain, cosmosWallet, allowedClients)
-	})
-
 	// Create new clients
 	err = r.CreateClients(ctx, eRep, pathName, ibc.DefaultClientOpts())
 	s.Require().NoError(err)
@@ -285,11 +277,6 @@ func (s *GrandpaTestSuite) TestMsgTransfer_TimesOut_GrandpaContract() {
 	err = r.GeneratePath(ctx, eRep, cosmosChain.Config().ChainID, polkadotChain.Config().ChainID, pathName)
 	s.Require().NoError(err)
 
-	t.Run("add 08-wasm to list of allowed clients", func(t *testing.T) {
-		allowedClients := s.queryAllowedClientsParam(ctx, cosmosChain)
-		s.allowWasmClients(ctx, cosmosChain, cosmosWallet, allowedClients)
-	})
-
 	// Create new clients
 	err = r.CreateClients(ctx, eRep, pathName, ibc.DefaultClientOpts())
 	s.Require().NoError(err)
@@ -354,7 +341,6 @@ func (s *GrandpaTestSuite) TestMsgTransfer_TimesOut_GrandpaContract() {
 // * Migrates the wasm client contract
 func (s *GrandpaTestSuite) TestMsgMigrateContract_Success_GrandpaContract() {
 	ctx := context.Background()
-	t := s.T()
 
 	chainA, chainB := s.GetGrandpaTestChains()
 
@@ -392,11 +378,6 @@ func (s *GrandpaTestSuite) TestMsgMigrateContract_Success_GrandpaContract() {
 	err = r.GeneratePath(ctx, eRep, cosmosChain.Config().ChainID, polkadotChain.Config().ChainID, pathName)
 	s.Require().NoError(err)
 
-	t.Run("add 08-wasm to list of allowed clients", func(t *testing.T) {
-		allowedClients := s.queryAllowedClientsParam(ctx, cosmosChain)
-		s.allowWasmClients(ctx, cosmosChain, cosmosWallet, allowedClients)
-	})
-
 	// Create new clients
 	err = r.CreateClients(ctx, eRep, pathName, ibc.DefaultClientOpts())
 	s.Require().NoError(err)
@@ -447,7 +428,6 @@ func (s *GrandpaTestSuite) TestMsgMigrateContract_Success_GrandpaContract() {
 // * Migrates the wasm client contract with a contract that will always fail migration
 func (s *GrandpaTestSuite) TestMsgMigrateContract_ContractError_GrandpaContract() {
 	ctx := context.Background()
-	t := s.T()
 
 	chainA, chainB := s.GetGrandpaTestChains()
 
@@ -484,11 +464,6 @@ func (s *GrandpaTestSuite) TestMsgMigrateContract_ContractError_GrandpaContract(
 	err = r.GeneratePath(ctx, eRep, cosmosChain.Config().ChainID, polkadotChain.Config().ChainID, pathName)
 	s.Require().NoError(err)
 
-	t.Run("add 08-wasm to list of allowed clients", func(t *testing.T) {
-		allowedClients := s.queryAllowedClientsParam(ctx, cosmosChain)
-		s.allowWasmClients(ctx, cosmosChain, cosmosWallet, allowedClients)
-	})
-
 	// Create new clients
 	err = r.CreateClients(ctx, eRep, pathName, ibc.DefaultClientOpts())
 	s.Require().NoError(err)
@@ -541,7 +516,6 @@ func (s *GrandpaTestSuite) TestMsgMigrateContract_ContractError_GrandpaContract(
 // This contract modifies the unbonding period to 1600s with the trusting period being calculated as (unbonding period / 3).
 func (s *GrandpaTestSuite) TestRecoverClient_Succeeds_GrandpaContract() {
 	ctx := context.Background()
-	t := s.T()
 
 	// set the trusting period to a value which will still be valid upon client creation, but invalid before the first update
 	// the contract uses 1600s as the unbonding period with the trusting period evaluating to (unbonding period / 3)
@@ -585,11 +559,6 @@ func (s *GrandpaTestSuite) TestRecoverClient_Succeeds_GrandpaContract() {
 	err = r.GeneratePath(ctx, eRep, cosmosChain.Config().ChainID, polkadotChain.Config().ChainID, pathName)
 	s.Require().NoError(err)
 
-	t.Run("add 08-wasm to list of allowed clients", func(t *testing.T) {
-		allowedClients := s.queryAllowedClientsParam(ctx, cosmosChain)
-		s.allowWasmClients(ctx, cosmosChain, cosmosWallet, allowedClients)
-	})
-
 	// create client pair with subject (bad trusting period)
 	subjectClientID := clienttypes.FormatClientIdentifier(wasmtypes.Wasm, 0)
 	// TODO: The hyperspace relayer makes no use of create client opts
@@ -797,48 +766,3 @@ func (s *GrandpaTestSuite) GetGrandpaTestChains() (ibc.Chain, ibc.Chain) {
 		options.ChainBSpec.EncodingConfig = testsuite.SDKEncodingConfig()
 	})
 }
-
-// queryAllowedClientsParam queries the on-chain allowed clients param for 02-client
-func (s *GrandpaTestSuite) queryAllowedClientsParam(ctx context.Context, chain ibc.Chain) []string {
-	if testvalues.SelfParamsFeatureReleases.IsSupported(chain.Config().Images[0].Version) {
-		queryClient := s.GetChainGRCPClients(chain).ClientQueryClient
-		res, err := queryClient.ClientParams(ctx, &clienttypes.QueryClientParamsRequest{})
-		s.Require().NoError(err)
-
-		return res.Params.AllowedClients
-	}
-	queryClient := s.GetChainGRCPClients(chain).ParamsQueryClient
-	res, err := queryClient.Params(ctx, &paramsproposaltypes.QueryParamsRequest{
-		Subspace: ibcexported.ModuleName,
-		Key:      string(clienttypes.KeyAllowedClients),
-	})
-	s.Require().NoError(err)
-
-	var allowedClients []string
-	err = cmtjson.Unmarshal([]byte(res.Param.Value), &allowedClients)
-	s.Require().NoError(err)
-
-	return allowedClients
-}
-
-// allowWasmClients adds 08-wasm to the on-chain allowed clients param for 02-client
-func (s *GrandpaTestSuite) allowWasmClients(ctx context.Context, chain ibc.Chain, wallet ibc.Wallet, allowedClients []string) {
-	govModuleAddress, err := s.QueryModuleAccountAddress(ctx, govtypes.ModuleName, chain)
-	s.Require().NoError(err)
-	s.Require().NotNil(govModuleAddress)
-
-	allowedClients = append(allowedClients, wasmtypes.Wasm)
-	if testvalues.SelfParamsFeatureReleases.IsSupported(chain.Config().Images[0].Version) {
-		msg := clienttypes.NewMsgUpdateParams(govModuleAddress.String(), clienttypes.NewParams(allowedClients...))
-		s.ExecuteAndPassGovV1Proposal(ctx, msg, chain, wallet)
-	} else {
-		value, err := cmtjson.Marshal(allowedClients)
-		s.Require().NoError(err)
-		changes := []paramsproposaltypes.ParamChange{
-			paramsproposaltypes.NewParamChange(ibcexported.ModuleName, string(clienttypes.KeyAllowedClients), string(value)),
-		}
-
-		proposal := paramsproposaltypes.NewParameterChangeProposal(ibctesting.Title, ibctesting.Description, changes)
-		s.ExecuteAndPassGovV1Beta1Proposal(ctx, chain, wallet, proposal)
-	}
-}

e2e/testsuite/testconfig.go

@@ -27,6 +27,10 @@ import (
 	"github.com/cosmos/ibc-go/e2e/relayer"
 	"github.com/cosmos/ibc-go/e2e/semverutil"
 	"github.com/cosmos/ibc-go/e2e/testvalues"
+	wasmtypes "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types"
+	clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
+	ibcexported "github.com/cosmos/ibc-go/v8/modules/core/exported"
+	ibctypes "github.com/cosmos/ibc-go/v8/modules/core/types"
 )
 
 const (
@@ -532,7 +536,7 @@ func getGenesisModificationFunction(cc ChainConfig) func(ibc.ChainConfig, []byte
 		return defaultGovv1ModifyGenesis(version)
 	}
 
-	return defaultGovv1Beta1ModifyGenesis()
+	return defaultGovv1Beta1ModifyGenesis(version)
 }
 
 // defaultGovv1ModifyGenesis will only modify governance params to ensure the voting period and minimum deposit
@@ -554,9 +558,16 @@ func defaultGovv1ModifyGenesis(version string) func(ibc.ChainConfig, []byte) ([]
 		if err != nil {
 			return nil, err
 		}
-
 		appState[govtypes.ModuleName] = govGenBz
 
+		if !testvalues.AllowAllClientsWildcardFeatureReleases.IsSupported(version) {
+			ibcGenBz, err := modifyClientGenesisAppState(chainConfig, appState[ibcexported.ModuleName])
+			if err != nil {
+				return nil, err
+			}
+			appState[ibcexported.ModuleName] = ibcGenBz
+		}
+
 		appGenesis.AppState, err = json.Marshal(appState)
 		if err != nil {
 			return nil, err
@@ -581,7 +592,7 @@ func defaultGovv1ModifyGenesis(version string) func(ibc.ChainConfig, []byte) ([]
 
 // defaultGovv1Beta1ModifyGenesis will only modify governance params to ensure the voting period and minimum deposit
 // // are functional for e2e testing purposes.
-func defaultGovv1Beta1ModifyGenesis() func(ibc.ChainConfig, []byte) ([]byte, error) {
+func defaultGovv1Beta1ModifyGenesis(version string) func(ibc.ChainConfig, []byte) ([]byte, error) {
 	const appStateKey = "app_state"
 	return func(chainConfig ibc.ChainConfig, genbz []byte) ([]byte, error) {
 		genesisDocMap := map[string]interface{}{}
@@ -611,6 +622,24 @@ func defaultGovv1Beta1ModifyGenesis() func(ibc.ChainConfig, []byte) ([]byte, err
 			return nil, fmt.Errorf("failed to unmarshal gov genesis bytes into map: %w", err)
 		}
 
+		if !testvalues.AllowAllClientsWildcardFeatureReleases.IsSupported(version) {
+			ibcModuleBytes, err := json.Marshal(appStateMap[ibcexported.ModuleName])
+			if err != nil {
+				return nil, fmt.Errorf("failed to extract ibc genesis bytes: %s", err)
+			}
+
+			ibcGenesisBytes, err := modifyClientGenesisAppState(chainConfig, ibcModuleBytes)
+			if err != nil {
+				return nil, err
+			}
+
+			ibcModuleGenesisMap := map[string]interface{}{}
+			err = json.Unmarshal(ibcGenesisBytes, &ibcModuleGenesisMap)
+			if err != nil {
+				return nil, fmt.Errorf("failed to unmarshal gov genesis bytes into map: %w", err)
+			}
+		}
+
 		appStateMap[govtypes.ModuleName] = govModuleGenesisMap
 		genesisDocMap[appStateKey] = appStateMap
 
@@ -673,3 +702,24 @@ func modifyGovv1Beta1AppState(chainConfig ibc.ChainConfig, govAppState []byte) (
 
 	return govGenBz, nil
 }
+
+// modifyClientGenesisAppState takes the existing ibc app state and marshals it to a ibc GenesisState.
+func modifyClientGenesisAppState(chainConfig ibc.ChainConfig, ibcAppState []byte) ([]byte, error) {
+	cfg := testutil.MakeTestEncodingConfig()
+
+	cdc := codec.NewProtoCodec(cfg.InterfaceRegistry)
+	clienttypes.RegisterInterfaces(cfg.InterfaceRegistry)
+
+	ibcGenesisState := &ibctypes.GenesisState{}
+	if err := cdc.UnmarshalJSON(ibcAppState, ibcGenesisState); err != nil {
+		return nil, fmt.Errorf("failed to unmarshal genesis bytes into client genesis state: %w", err)
+	}
+
+	ibcGenesisState.ClientGenesis.Params.AllowedClients = append(ibcGenesisState.ClientGenesis.Params.AllowedClients, wasmtypes.Wasm)
+	ibcGenBz, err := cdc.MarshalJSON(ibcGenesisState)
+	if err != nil {
+		return nil, fmt.Errorf("failed to marshal gov genesis state: %w", err)
+	}
+
+	return ibcGenBz, nil
+}

e2e/testvalues/values.go

@@ -91,3 +91,11 @@ var LocalhostClientFeatureReleases = semverutil.FeatureReleases{
 		"v7.1",
 	},
 }
+
+// AllowAllClientsWildcardFeatureReleases represents the releases the allow all clients wildcard was released in.
+var AllowAllClientsWildcardFeatureReleases = semverutil.FeatureReleases{
+	MajorVersion: "v9",
+	MinorVersions: []string{
+		"v8.1",
+	},
+}

modules/core/02-client/types/keys.go

@@ -28,6 +28,10 @@ const (
 
 	// ParamsKey is the store key for the IBC client parameters
 	ParamsKey = "clientParams"
+
+	// AllowAllClients is the value that if set in AllowedClients param
+	// would allow any wired up light client modules to be allowed
+	AllowAllClients = "*"
 )
 
 // FormatClientIdentifier returns the client identifier with the sequence appended.

modules/core/02-client/types/params.go

@@ -4,12 +4,11 @@ import (
 	"fmt"
 	"slices"
 	"strings"
-
-	"github.com/cosmos/ibc-go/v8/modules/core/exported"
 )
 
 // DefaultAllowedClients are the default clients for the AllowedClients parameter.
-var DefaultAllowedClients = []string{exported.Solomachine, exported.Tendermint, exported.Localhost}
+// By default it allows all client types.
+var DefaultAllowedClients = []string{AllowAllClients}
 
 // NewParams creates a new parameter configuration for the ibc client module
 func NewParams(allowedClients ...string) Params {
@@ -30,11 +29,27 @@ func (p Params) Validate() error {
 
 // IsAllowedClient checks if the given client type is registered on the allowlist.
 func (p Params) IsAllowedClient(clientType string) bool {
+	// Still need to check for blank client type
+	if strings.TrimSpace(clientType) == "" {
+		return false
+	}
+
+	// Check for allow all client wildcard
+	// If exist then allow all type of client
+	if len(p.AllowedClients) == 1 && p.AllowedClients[0] == AllowAllClients {
+		return true
+	}
+
 	return slices.Contains(p.AllowedClients, clientType)
 }
 
 // validateClients checks that the given clients are not blank and there are no duplicates.
+// If AllowAllClients wildcard (*) is used, then there should no other client types in the allow list
 func validateClients(clients []string) error {
+	if slices.Contains(clients, AllowAllClients) && len(clients) > 1 {
+		return fmt.Errorf("allow list must have only one element because the allow all clients wildcard (%s) is present", AllowAllClients)
+	}
+
 	foundClients := make(map[string]bool, len(clients))
 	for i, clientType := range clients {
 		if strings.TrimSpace(clientType) == "" {

modules/core/02-client/types/params_test.go

@@ -19,6 +19,8 @@ func TestIsAllowedClient(t *testing.T) {
 		{"success: valid client with custom params", exported.Tendermint, NewParams(exported.Tendermint), true},
 		{"success: invalid blank client", " ", DefaultParams(), false},
 		{"success: invalid client with custom params", exported.Localhost, NewParams(exported.Tendermint), false},
+		{"success: wildcard allow all clients", "test-client-type", NewParams(AllowAllClients), true},
+		{"success: wildcard allow all clients with blank client", " ", NewParams(AllowAllClients), false},
 	}
 
 	for _, tc := range testCases {
@@ -37,6 +39,7 @@ func TestValidateParams(t *testing.T) {
 		{"custom params", NewParams(exported.Tendermint), true},
 		{"blank client", NewParams(" "), false},
 		{"duplicate clients", NewParams(exported.Tendermint, exported.Tendermint), false},
+		{"allow all clients plus valid client", NewParams(AllowAllClients, exported.Tendermint), false},
 	}
 
 	for _, tc := range testCases {

modules/light-clients/08-wasm/keeper/keeper_test.go

@@ -79,8 +79,6 @@ func (suite *KeeperTestSuite) SetupWasmWithMockVM() {
 
 	suite.coordinator = ibctesting.NewCoordinator(suite.T(), 1)
 	suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1))
-
-	wasmtesting.AllowWasmClients(suite.chainA)
 }
 
 func (suite *KeeperTestSuite) setupWasmWithMockVM() (ibctesting.TestingApp, map[string]json.RawMessage) {