ics02_client ConsensusState trait get root function should return Option value

[DaviRain-Su]

Feature Summary

In ibc-go, ics06-solomachine, and ics09-localhost, it's all not return value, they are return nill by default

Proposal

so, we can change fn root(&self) -> &CommitmentRoot; to fn root(&self) -> Option<&CommitmentRoot>;

[plafer]

it's all not return value, they are return nill by default

I'm less familiar with that part of the codebase, could you point to where this occurs in ibc-go?

[DaviRain-Su]

In ibc-go v5.0.0 this function return nill, https://github.com/cosmos/ibc-go/blob/28633816463b1323ccb48d197e9440c253f95a5c/modules/light-clients/06-solomachine/types/consensus_state.go#L26, but for now master have remove this function. I think this default implement return nill.

[plafer]

From what I can tell, this function was unused. In the case of the tendermint light client, the consensus state root is used in Merkle proof verifications.

If we look at the verifyMembership function for solomachine, it no longer calls ConsensusState::GetRoot; it doesn't even get the consensus state at all.

Basically, I think ibc-go's new way to do it is better; the main purpose of a ConsensusState is to provide a merkle root. If a particular client's ConsensusState doesn't provide a Merkle root, then you shouldn't even be using the consensus state in your verification algorithm.

Thank you for bringing this to our attention though; but after careful attention, I still believe our interface is appropriate as is.