ConsensusState trait should not have a root() method, and ClientState.verify_membership() should not take a root

[plafer]

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).

[Farhad-Shabani]

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?

[plafer]

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 ConsensusStates (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.

[Farhad-Shabani]

#784 (comment) related to this issue as well. Worth considering :)