ibc-rs
ibc-rs copied to clipboard
`ConsensusState` trait should not have a `root()` method, and `ClientState.verify_membership()` should not take a root
Not all client's ConsensusState
have the concept of a root (e.g. solomachine). Instead of passing the root to verify_membership
in the core handlers, we should instead fetch the root in the respective clients when needed (e.g. here in ibc-go's tendermint client).
The question this brings... was wondering what the purpose of the ConsensusState
would be if it could live and be defined without the root. Then, logically, why not include the timestamp()
in the ClientState (Particularly given that it contains the latest height as well) and move away completely from the ConsensusState
?
Isn't the Solomachine a special case? Wouldn't this generalize the concept of ConsensusState
too much?
The big difference between ClientState
and ConsensusState
is how they're stored.
-
ClientState
:clients/{identifier}/clientState
-
ConsensusState
:clients/{identifier}/consensusStates/{height}
Specifically, we remember the history of ConsensusState
s (indexed by height), whereas we only remember the latest ClientState
. So ClientState.timestamp()
would give us the timestamp of the latest ClientState
, whereas ConsensusState.timestamp()
is the timestamp associated with the ConsensusState
at a given height.
The root()
is not fundamental to IBC, whereas timestamp()
is: it is needed to verify packet timeouts when timing out a packet. So we know for sure that every ConsensusState
needs to have a timestamp.
https://github.com/cosmos/ibc-rs/issues/784#issuecomment-1650572151 related to this issue as well. Worth considering :)