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.

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

docs: explain how to implement the four client state functions which allow for regular updates and misbehaviour handling #2939

Merged
merged 13 commits into from
Jan 10, 2023
Merged
3 changes: 0 additions & 3 deletions docs/ibc/light-clients/misbehaviour.md

This file was deleted.

2 changes: 1 addition & 1 deletion docs/ibc/light-clients/overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,4 @@ The following are considered as valid update scenarios:
- A batch of block headers which when verified inserts `N` `ConsensusState` instances for `N` unique heights.
- Evidence of misbehaviour provided by two conflicting block headers.

Learn more in the [handling client updates](./update.md) and [handling misbehaviour](./misbehaviour.md) sections.
Learn more in the [handling client updates](./update.md) and [handling misbehaviour](./updates-and-misbehaviour.md) sections.
2 changes: 1 addition & 1 deletion docs/ibc/light-clients/proofs.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<!--
order: 7
order: 8
-->
2 changes: 1 addition & 1 deletion docs/ibc/light-clients/update.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<!--
order: 4
order: 5
-->

# Implementing the `ClientMessage` interface
Expand Down
43 changes: 43 additions & 0 deletions docs/ibc/light-clients/updates-and-misbehaviour.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<!--
order: 6
-->

# Handling Updates and Misbehaviour
Copy link
Contributor

Choose a reason for hiding this comment

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

could we add pseudocode from the UpdateClient function which shows the usage of these 4 functions?

Something like:

    if err := clientState.VerifyClientMessage(clientMessage); err != nil {
        return err
    }
    
    foundMisbehaviour := clientState.CheckForMisbehaviour(clientMessage)
    if foundMisbehaviour {
        clientState.UpdateStateOnMisbehaviour(clientMessage)
        // emit misbehaviour event
        return 
    }
    
    clientState.UpdateState(clientMessage) // expects no-op on duplicate header
    // emit update event
    return
}


The functions for handling updates to a light clients and evidence of misbehaviour are all found in the [`ClientState`](https://github.com/cosmos/ibc-go/blob/v6.0.0/modules/core/exported/client.go#L40) interface, and will be discussed below.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The functions for handling updates to a light clients and evidence of misbehaviour are all found in the [`ClientState`](https://github.com/cosmos/ibc-go/blob/v6.0.0/modules/core/exported/client.go#L40) interface, and will be discussed below.
The functions for handling updates to a light client and evidence of misbehaviour are all found in the [`ClientState`](https://github.com/cosmos/ibc-go/blob/v6.0.0/modules/core/exported/client.go#L40) interface, and will be discussed below.


It is important to note that `Misbehaviour` in this particular context is referring to misbehaviour on the chain level intended to fool the light client. This will be defined by each light client.

## VerifyClientMessage
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## VerifyClientMessage
## `VerifyClientMessage`


`VerifyClientMessage` must verify a `ClientMessage`. A `ClientMessage` could be a `Header`, `Misbehaviour`, or batch update. To understand how to implement a `ClientMessage`, please refer to the [documentation](./update.md)

It must handle each type of `ClientMessage` appropriately. Calls to `CheckForMisbehaviour`, `UpdateState`, and `UpdateStateOnMisbehaviour` will assume that the content of the `ClientMessage` has been verified and can be trusted. An error should be returned if the `ClientMessage` fails to verify.

For an example of a `VerifyClientMessage` implementation, please check the [Tendermint light client](https://github.com/cosmos/ibc-go/blob/main/modules/light-clients/07-tendermint/update.go#L20)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For an example of a `VerifyClientMessage` implementation, please check the [Tendermint light client](https://github.com/cosmos/ibc-go/blob/main/modules/light-clients/07-tendermint/update.go#L20)
For an example of a `VerifyClientMessage` implementation, please check the [Tendermint light client](https://github.com/cosmos/ibc-go/blob/main/modules/light-clients/07-tendermint/update.go#L20).


## CheckForMisbehaviour
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## CheckForMisbehaviour
## `CheckForMisbehaviour`


Checks for evidence of a misbehaviour in `Header` or `Misbehaviour` type. It assumes the `ClientMessage` has already been verified.

For an example of a `CheckForMisbehaviour` implementation, please check the [Tendermint light client](https://github.com/cosmos/ibc-go/blob/main/modules/light-clients/07-tendermint/misbehaviour_handle.go#L18)
charleenfei marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For an example of a `CheckForMisbehaviour` implementation, please check the [Tendermint light client](https://github.com/cosmos/ibc-go/blob/main/modules/light-clients/07-tendermint/misbehaviour_handle.go#L18)
For an example of a `CheckForMisbehaviour` implementation, please check the [Tendermint light client](https://github.com/cosmos/ibc-go/blob/main/modules/light-clients/07-tendermint/misbehaviour_handle.go#L18).


For example, the Tendermint light client [defines `Misbehaviour`](https://github.com/cosmos/ibc-go/blob/main/modules/light-clients/07-tendermint/misbehaviour.go) as two different types of situations: a situation where two conflicting `Headers` with the same height have been submitted to update a client's `ConsensusState` within the same trusting period, or that the two conflicting `Headers` have been submitted at different heights but the consensus states are not in the correct monotonic time ordering (BFT time violation). More explicitly, updating to a new height must have a timestamp greater than the previous consensus state, or, if inserting a consensus at a past height, then time must be less than those heights which come after and greater than heights which come before.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For example, the Tendermint light client [defines `Misbehaviour`](https://github.com/cosmos/ibc-go/blob/main/modules/light-clients/07-tendermint/misbehaviour.go) as two different types of situations: a situation where two conflicting `Headers` with the same height have been submitted to update a client's `ConsensusState` within the same trusting period, or that the two conflicting `Headers` have been submitted at different heights but the consensus states are not in the correct monotonic time ordering (BFT time violation). More explicitly, updating to a new height must have a timestamp greater than the previous consensus state, or, if inserting a consensus at a past height, then time must be less than those heights which come after and greater than heights which come before.
> For example, the Tendermint light client [defines `Misbehaviour`](https://github.com/cosmos/ibc-go/blob/main/modules/light-clients/07-tendermint/misbehaviour.go) as two different types of situations: a situation where two conflicting `Headers` with the same height have been submitted to update a client's `ConsensusState` within the same trusting period, or that the two conflicting `Headers` have been submitted at different heights but the consensus states are not in the correct monotonic time ordering (BFT time violation). More explicitly, updating to a new height must have a timestamp greater than the previous consensus state, or, if inserting a consensus at a past height, then time must be less than those heights which come after and greater than heights which come before.

Maybe some extra formating to make clear this is a note?


## UpdateStateOnMisbehaviour
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## UpdateStateOnMisbehaviour
## `UpdateStateOnMisbehaviour`


`UpdateStateOnMisbehaviour` should perform appropriate state changes on a client state given that misbehaviour has been detected and verified. This method should only be called when misbehaviour is detected, as it does not perform any misbehaviour checks. Notably, it should freeze the client so that calling the `Status()` function on the associated client state no longer returns `Active`.

For an example of a `UpdateStateOnMisbehaviour` implementation, please check the [Tendermint light client](https://github.com/cosmos/ibc-go/blob/main/modules/light-clients/07-tendermint/update.go#L197)

## UpdateState
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## UpdateState
## `UpdateState`


`UpdateState` updates and stores as necessary any associated information for an IBC client, such as the `ClientState` and corresponding `ConsensusState`. It should perform a no-op on duplicate updates.

It assumes the `ClientMessage` has already been verified.

For an example of a `UpdateState` implementation, please check the [Tendermint light client](https://github.com/cosmos/ibc-go/blob/main/modules/light-clients/07-tendermint/update.go#L131)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For an example of a `UpdateState` implementation, please check the [Tendermint light client](https://github.com/cosmos/ibc-go/blob/main/modules/light-clients/07-tendermint/update.go#L131)
For an example of a `UpdateState` implementation, please check the [Tendermint light client](https://github.com/cosmos/ibc-go/blob/main/modules/light-clients/07-tendermint/update.go#L131).


## Reference Example
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if I'd go with 'reference example' as title here. Throughout the page, we already use the 07-tendermint as reference example for the light client implementation so using 'reference example' here again when looking at 02-client feels a little confusing.

How about "Putting it all together" or something similar as section title containing the pseudo code like @colin-axner mentioned above and then also refer to 02-client?


The `02-client Keeper` module in ibc-go offers a reference as to how these functions will be used to [update the client](https://github.com/cosmos/ibc-go/blob/main/modules/core/02-client/keeper/client.go#L48).
2 changes: 1 addition & 1 deletion docs/ibc/light-clients/upgrade.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<!--
order: 8
order: 7
-->