cosmos / ibc-rs

Rust implementation of the Inter-Blockchain Communication (IBC) protocol.
Apache License 2.0
182 stars 73 forks source link

ics07: implement `TryFrom<ConsensusState, Error = ClientError>` for `ConsensusStateType` #1201

Closed rnbguy closed 2 months ago

rnbguy commented 2 months ago

Feature Summary

If implemented, we can use ConsensuState directly in ibc-client-tendermint-cw without a new AnyConsensusState.

https://github.com/cosmos/ibc-rs/blob/67d0735d3acc41ddadf8b20eb4b77c02f3707435/ibc-clients/ics07-tendermint/cw-contract/src/client_type.rs#L15-L18

Proposal

Add the following trait implementation in ibc-client-tendermint,

impl TryFrom<ConsensusState> for ConsensusStateType {
    type Error = ClientError;
    fn try_from(value: ConsensusState) -> Result<Self, Self::Error> {
        Ok(value.0)
    }
}

and use ConsensusState directly in ibc-client-tendermint-cw.

impl<'a> ClientType<'a> for TendermintClient {
    type ClientState = ClientState;
    type ConsensusState = ConsensusState;
}

Note that the TryFrom<ConsensusState> never fails. But we can't use From<ConsensusState>, as it implies TryFrom<ConsensusState, Error = Infallible>. And ConsensusStateDecoder requires TryFrom<ConsensusState, Error = ClientError>.

https://github.com/cosmos/ibc-rs/blob/67d0735d3acc41ddadf8b20eb4b77c02f3707435/ibc-core/ics02-client/context/src/consensus_state.rs#L11

This is a step towards #1169. Also, refer to #1203 for a discussion about removing the concrete ClientError type.

rnbguy commented 2 months ago

This will also improve the rollkit-ibc implementation.

seanchen1991 commented 2 months ago

Just a note for posterity: we did end up opting for a From<ConsensusState> for ConsensusStateType impl, which was enabled after implementing From<Infallible> for ClientError.