- 2022-08-01: Initial Draft
Accepted and applied in v7 of ibc-go
During the initial development of the 02-client submodule, each light client supported (06-solomachine, 07-tendermint, 09-localhost) was referenced through hardcoding. Here is an example of the code that existed in the 02-client submodule:
func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.Header) (exported.ClientState, error) {
...
switch clientType {
case exported.Tendermint:
clientState, consensusState, err = tendermint.CheckValidityAndUpdateState(
clientState, header, ctx.BlockTime(),
)
case exported.Localhost:
// override client state and update the block height
clientState = localhosttypes.NewClientState(
ctx.ChainID(), // use the chain ID from context since the client is from the running chain (i.e self).
ctx.BlockHeight(),
)
default:
err = types.ErrInvalidClientType
}To add additional light clients, code would need to be added directly to the 02-client submodule. Evidently, this would likely become problematic as IBC scaled to many chains using consensus mechanisms beyond the initial supported light clients. Issue #6064 on the SDK addressed this problem by creating a more modular 02-client submodule. The 02-client submodule would now interact with each light client via an interface. While, this change was positive in development, increasing the flexibility and adoptability of IBC, it also opened the door to new problems.
The difficulty of generalizing light clients became apparent once changes to those light clients were required. Each light client represents a different consensus algorithm which may contain a host of complexity and nuances. Here are some examples of issues which arose for light clients that are not applicable to all the light clients supported (06-solomachine, 07-tendermint, 09-localhost):
Before the launch of IBC, it was determined that the golang implementation of tendermint would not be capable of supporting non-zero height upgrades. This implies that any upgrade would require changing of the chain ID and resetting the height to 0. A chain is uniquely identified by its chain-id and validator set. Two different chain ID's can be viewed as different chains and thus a normal update produced by a validator set cannot change the chain ID. To work around the lack of support for non-zero height upgrades, an abstract height type was created along with an upgrade mechanism. This type would indicate the revision number (the number of times the chain ID has been changed) and revision height (the current height of the blockchain).
Refs:
- Issue #439 on IBC specification repository.
- Specification changes in #447
- Implementation changes for the abstract height type, SDK#7211
The initial release of the IBC module and the 07-tendermint light client implementation did not support misbehaviour detection during update nor did it prevent overwriting of previous updates.
Despite the fact that we designed the ClientState interface and developed the 07-tendermint client, we failed to detect even a duplicate update that constituted misbehaviour and thus should freeze the client.
This was fixed in PR #141 which required light client implementations to be aware that they must handle duplicate updates and misbehaviour detection.
Misbehaviour detection during updates is not applicable to the solomachine nor localhost.
It is also not obvious that CheckHeaderAndUpdateState should be performing this functionality.
The localhost has been broken since the initial version of the IBC module. The localhost tried to be developed underneath the 02-client interfaces without special exception, but this proved to be impossible. The issues were outlined in #27 and further discussed in the attempted ADR in #75. Unlike all other clients, the localhost requires access to the entire IBC store and not just the prefixed client store.
The 06-solomachine does not set the consensus states within the prefixed client store. It has a single consensus state that is stored within the client state. This causes setting of the consensus state at the 02-client level to use unnecessary storage. It also causes timeouts to fail with solo machines. Previously, the timeout logic within IBC would obtain the consensus state at the height a timeout is being proved. This is problematic for the solo machine as no consensus state is set. See issue #562 on the IBC specification repo.
New light clients may not function in a similar fashion to 06-solomachine and 07-tendermint. They may require setting many consensus states in a single update. As @seunlanlege states:
I'm in support of these changes for 2 reasons:
This would allow light clients handle batch header updates in CheckHeaderAndUpdateState, for the special case of 11-beefy proving the finality for a batch of headers is much more space and time efficient than the space/time complexity of proving each individual headers in that batch, combined.
This also allows for a single light client instance of 11-beefy be used to prove finality for every parachain connected to the relay chain (Polkadot/Kusama). We achieve this by setting the appropriate ConsensusState for individual parachain headers in CheckHeaderAndUpdateState
The IBC specification states:
If the provided header was valid, the client MUST also mutate internal state to store now-finalised consensus roots and update any necessary signature authority tracking (e.g. changes to the validator set) for future calls to the validity predicate.
The initial version of the IBC go SDK based module did not fulfill this requirement. Instead, the 02-client submodule required each light client to return the client and consensus state which should be updated in the client prefixed store. This decision lead to the issues "Solomachine doesn't set consensus states" and "New clients may want to do batch updates".
Each light client should be required to set its own client and consensus states on any update necessary. The go implementation should be changed to match the specification requirements. This will allow more flexibility for light clients to manage their own internal storage and do batch updates.
Remove GetHeight() from the header interface (as light clients now set the client/consensus states).
This results in the Header/Misbehaviour interfaces being the same.
To reduce complexity of the codebase, the Header/Misbehaviour interfaces should be merged into ClientMessage.
ClientMessage will provide the client with some authenticated information which may result in regular updates, misbehaviour detection, batch updates, or other custom functionality a light client requires.
See #668.
Split CheckHeaderAndUpdateState into 4 functions:
VerifyClientMessageCheckForMisbehaviourUpdateStateOnMisbehaviourUpdateState
VerifyClientMessage checks the that the structure of a ClientMessage is correct and that all authentication data provided is valid.
CheckForMisbehaviour checks to see if a ClientMessage is evidence of misbehaviour.
UpdateStateOnMisbehaviour freezes the client and updates its state accordingly.
UpdateState performs a regular update or a no-op on duplicate updates.
The code roughly looks like:
func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.Header) error {
...
if err := clientState.VerifyClientMessage(clientMessage); err != nil {
return err
}
foundMisbehaviour := clientState.CheckForMisbehaviour(clientMessage)
if foundMisbehaviour {
clientState.UpdateStateOnMisbehaviour(header)
// emit misbehaviour event
return
}
clientState.UpdateState(clientMessage) // expects no-op on duplicate header
// emit update event
return
}By adding GetTimestampAtHeight to the ClientState interface, we allow light clients which do non-traditional consensus state/timestamp storage to process timeouts correctly.
This fixes the issues outlined for the solo machine client.
As the complexity and the functionality grows, new verification functions will be required for additional paths.
This was explained in #684 on the specification repo.
These generic verification functions would be immediately useful for the new paths added in connection/channel upgradability as well as for custom paths defined by IBC applications such as Interchain Queries.
The old verification functions (VerifyClientState, VerifyConnection, etc) should be removed in favor of the generic verification functions.
- Flexibility for light client implementations
- Well defined interfaces and their required functionality
- Generic verification functions
- Applies changes necessary for future client/connection/channel upgrabability features
- Timeout processing for solo machines
- Reduced code complexity
- The refactor touches on sensitive areas of the ibc-go codebase
- Changing of established naming (
Header/MisbehaviourtoClientMessage)
No notable consequences
Issues:
PRs: