extend 03-connection with a proof object for chains that can't introspect their own consensus state.

[seunlanlege]

Hey guys, Protocol Lead for composable here. We've been working on extending ibc beyond cosmos and we've been working with the core ibc team on making this happen. Through this relationship we requested the 02-client refactor and now finally we're requesting this 03-connection extension.

The Problem

03-connection as is requires that host state machines can introspect their own consensus state. Unfortunately this requirement is incompatible with every single blockchain we're working with (substrate, NEAR, ethereum). All of whom do not provide this sort of api to query past headers from within the blockchain runtime.

The solution

Inorder to keep to the IBC 03-connection specification and not skip the counterparty consensus state verification. We replace the requirement of host ConsensusState introspection, with a requirement of an on-chain map of block numbers to header hashes which can then be used to verify the proofHostConsensusState (aka full header)

[AdityaSripal]

Is there a way to signal that this is optional?

[AdityaSripal]

Also my understanding is that in your implementation the proof field here is actually a marshalled header.

This is the flow from my understanding:

1. You pass in the marshalled header here in proofHostConsensusState
2. in your handler you check that the passed in header hashes to one of your stored header hashes
3. you create a consensus state out of the header
4. you prove that the counterparty stored that consensus state in its client.

So this field shouldn't actually contain a CommitmentProof object. Instead it contains some arbitrary chain-specific data, that will be used to verify proofConsensus that is passed in above.

So I think instead of referring to it this way, it makes more sense to call this something like:

hostConsensusInfo: bytes

or something similar, and to signal in documentation that this is optional for chains to accept.

[seunlanlege]

Is there a way to signal that this is optional?

yeah there is

hostConsensusInfo: bytes

how about hostConsensusStateProof?: bytes

[AdityaSripal]

I still think it shouldn't be named proof. Because the object in question is not actually a proof.

It is metadata that is constructing the value that will get proven agains the consensusStateProof which is a separate argument to this function.

Open to a different name other than hostConsensusInfo but I do think we should avoid Proof in the name since semantically it doesn't match with the way we're using Proof arguments in IBC spec functions

[colin-axner]

Is there any other information a chain may request to extend the connection handshake info with? Or is this the only expected addition?

[seunlanlege]

This is the only addition

[colin-axner]

I'm okay with this change, but here are my reservations:

1. If implementations are going to use MsgConnectionOpenTry as the entrypoint to submitting connection relay messages, then this should be a recommendation of the spec, so it is made explicit the Msgs of ibc-go are intended to be compatible with other implementations
2. We should move the protos to the spec repo. Maybe we can leave protos in ibc-go, but copy the protos without any gogoproto annotations so it is a clean file. Any changes made to the spec protos can then manually be made on ibc-go
3. If this change is agreed upon and merged, then in ibc-go the proto file should probably specify that field as reserved. I'm unsure if there is a better construction, but we should indicate in its generation that the field must be unused/restricted when interacting with ibc-go

[seunlanlege]

Bump, I'm in favour of moving the spec to this repo. And possibly exporting generated rust, go types from this repo.

[AdityaSripal]

Approved, blocked on another approval. Perhaps @colin-axner when you can put it in your iteration. cc: @crodriguezvega

I can take on the nits in a separate PR.

Agreed with the proto approach, but that should be done in a separate PR as well

[mpoke]

LGTM