Implement `migrateClient` function in `ClientState`

[crodriguezvega]

Based on feedback received on Ethan Frey's audit of 08-wasm, the CheckSubsituteAndUpdateState function of the ClientState interface is overloaded (updates the client state, the consensus state and its associated metadata) and error prone. We acknowledge this and see the benefit of splitting this API into 2 different functions.

This issue addresses the implementation of the first function, one that would update only the client state. For the time being, the 02-client ClientState interface function is not modified and this new function may be implemented only in 08-wasm's own ClientState implementation and be used in its implementation of CheckSubsituteAndUpdateState.

Possible pseudo-code for the function:

func (cs ClientState) migrateClient(store storetypes.KVStore, substituteClient exported.ClientState) error {
  substituteClientState, ok := substituteClient.(*ClientState)
  if !ok {
    return errorsmod.Wrapf(
      clienttypes.ErrInvalidClient,
      fmt.Sprintf("invalid substitute client state. expected type %T, got %T", &ClientState{}, substituteClient),
    )
  }

  // 08-wasm's implementation of Status will check that the code hash in substituteClientState
  // is in the allow list of code hashes 
  if status := substituteClientState.Status(ctx, substituteClientStore, cdc); status != exported.Active {
    return errorsmod.Wrapf(types.ErrClientNotActive, "cannot migrate to client with status %s", status)
  }

  payload := sudoMsg{
    MigrateClient: &migrateClientMsg{ // new payload message
      ClientState: substituteClientState,
    },
  }

  _, err := wasmCall[emptyResult](ctx, store, &cs, payload)
  return err
}

Contracts will need to implement the function to handle the migrateClientMsg message.

We also agreed that we could use the client recovery flow to migrate to a new contract, if needed (e.g. in the situation that a new contract needs to be deployed to fix a bug). However, as I am writing this I just realised that for this to work we need the light client with the existing contract to expire first (or make it not active somehow), since we check in 02-client's RecoverClient that the subject client is not active. For this reason I am adding the needs discussion label to discuss how to handle this.

[crodriguezvega]

Thinking about flows where we can use wasm's feature of contract migration.

Context

In x/wasm contract migration requires the contract to have an address, which is passed in the env parameter. In 08-wasm we currently do not have addresses for the light client contracts. In x/wasm the contract address is generated during contract instantiation, each contract instance gets its own contract address.

In x/wasm there is MsgMigrateContract with a field contract that is the address of the contract that should be migrated. The code_id field references the new contract that migrates to. MsgInstantiateContract has an admin field that specify who can perform migrations and this is stored in the ContractInfo during contract instantiation.

During migration it is checked that the caller (the signer of MsgMigrateContract can modify the contract (it uses the admin address stored in ContractInto during instantiation. In case of governance, the admin address does not matter, since the governance policy will allow migration if the signer is the governance module.

To execute the migration, the env variable passed to wasmVM.Migrate contains the address of the contract that should be migrated. In the call to wasmVM.Migrate the code hash of the new contract is also passed.

Proposals

If we want to use the migrate feature of wasmVM we need to have an address for each contract that is instantiated. In the Initialize function we instantiate the contract, so we can generate a contract address using the code hash and a similar trick as we do when we generate the interchain account address. In the Initialize function we also have access to the client store, so we could store the contract address under a contracAddress key. The contract address that we generate in Initialize should then be passed to wasmVM.Instantiate in the env parameter.

x/wasm exposes contract addresses to the external world and implements an elaborate permission scheme for migration, but in 08-wasm we could keep it simple and do not use contract addresses outside of the module (i.e. addresses would only be known and used inside the module).

Using wasm migration in migrateClient

In migrateClient:

• We check that the code hash of the substitute client is in the list of allowed code hashes by calling Status on the substitute client.
• We retrieve the contract address for the instance of the subject client ID.
• If the code hash of the subject and substitute clients are different, then we call wasmVM.Migrate with the contract address and the new code hash. If the code hashes are the same, we can skip this step.
• We sudo call MigrateClient on the contract to replace the client state of the subject with the client state of the substitute.

In order to use the client recovery flow for contract migration we would need to either change the check in 02-client's RecoverClient that enforces the subject client to not be active or execute in the same governance proposal that would execute RecoverClient another message that would make the subject client not active first, so that we can progress beyond the check in 02-client. Maybe there other alternatives as well.

Using a dedicated endpoint for migration

We could have a new message (e.g. MsgMigrateContract) and RPC endpoint (e.g. MigrateContract). MsgMigrateContract could have the following fields:

• The client ID that should be migrated to the new wasm code.
• The code hash of the contract we will migrate to.
• We could already add a msg field with the bytes that can be passed to the contract at migration, even if we don't use them right now.

We don't need to have a field for the code hash of the contract we migrate from, since we have the client ID and with it we can access the client store and retrieve the contract address.that we need to use in the call to wasmVM.Migrate.

In MigrateContract:

• We check that the new code hash is in the list of allowed code hashes. Since this RPC endpoint is in 08-wasm, we have access to the module store and we can do this check.
• If the old and new code hashes are the same, then we can exit early.
• We retrieve the contract address for the instance of the client ID using its own client-prefixed store.
• We call wasmVM.Migrate with the contract address and the new code hash.
• We update the code hash in the client state.

The main advantage of this approach is that we do not require modifications to 02-client to change the check that enforces subject clients to not be active if they are going to be recovered, or an extra message in the governance proposal to make the client not active.

The disadvantage is that we still need to deal with the situation where a client is recoverd and the new client state has a new code hash. We could either enforce in migrateClient that the code hash cannot change during client recovery, or alternatively implement the proposal for migrateClient above (i.e. we migrate the contract and update client state).

[srdtrk]

I'm in favour of using wasmVM.Migrate. Isn't it possible for us to use the clientID (ex 08-wasm-0) as the contract address? We could also make this a more general thing (for example providing the clientID as contract address to the Env)

[crodriguezvega]

We discussed and decided to implement the new RPC endpoint for migrating the byte code of a contract (for which I will open a new issue). We will close this issue and use https://github.com/cosmos/ibc-go/issues/4862 for the implementation changes needed in CheckSubstituteAndUpdateState.