02-client-refactor added support for multiple consensus states inside a header

[nadeemb53]

A beefy or grandpa header submitted on-chain can potentially contain headers with multiple consensus states. This PR refactors the client interface to support beefy and grandpa light clients.

[bluele]

Thanks for your PR!

My understanding is that in ICS-02 and ibc-go, each client implementation currently has the responsibility for client state updates. (ref. cosmos/ibc-go#1871) I would like to apply the same design to ibc-solidity as much as possible. Would this design change help with beefy and grandpa client implementations? Also, if you have already considered it, please tell us why it was difficult.

(however, there may be some things I missed about Gas cost or EVM constraints)
cc: @siburu @3100

[siburu]

@bluele

each client implementation currently has the responsibility for client state updates

Do you mean that IClient::updateClient itself should call host.setClientState?

[bluele]

Yes. FYI, example in Go: https://github.com/cosmos/ibc-go/blob/c86d27fc280cfb342a9e4689b381e5823441b694/modules/light-clients/07-tendermint/update.go#L132

[siburu]

Aside from that, I think you should move the definition of the handwritten ConsensusUpdate.Result to a different location.
The contracts/core/types directory contains only .sol files auto-generated from .proto files.

@bluele Where is an appropriate place to move it to?

[nadeemb53]

Hey @bluele, thanks for your comment. I have implemented it according to your suggestion.

@siburu thanks for your comment, I have removed consensusUpdate.Result as it was not needed anymore.

[bluele]

hey @nadeemb53, I realized there is one problem with moving the state updates to each client implementation.

The set* functions of the host cannot be called from Light Client contracts due to onlyIBCModule modifier. To prevent this, we need to use delegatecall for the Light Client contract call.

This have already been fixed in PR #116 . Therefore, please rebase the branch with the latest main. Also, could you minimize the changes in the commits? (e.g. the changes regarding code format are not needed in this PR)

[nadeemb53]

@bluele thanks for your changes. I've rebased and followed it to further fix my PR.

[bluele]

Hi @nadeemb53, I've left some comments. Also, run CI and it looks like integration-test is failed.

[nadeemb53]

Hi @bluele , could you please run the CI?

[bluele]

@nadeemb53 done. Please squash your commits into one commit.

[nadeemb53]

done @bluele

[bluele]

@nadeemb53 Great work! I've merged this.