Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ICS2: Replace specific VerifyFunctions with generic VerifyMembership and VerifyNonMembership #684

Closed
AdityaSripal opened this issue Mar 10, 2022 · 2 comments · Fixed by #687
Labels
tao Transport, authentication, & ordering layer.

Comments

@AdityaSripal
Copy link
Member

Rationale

Currently ICS-2 clients must create a custom verify function for each thing that needs to be proved by IBC: packets, receipts, acknowledgements, connections, etc.

This design causes a number of pain points when we want to make any changes to the protocol. If we wish to add a new verification method to enable a new feature, we must change the ICS-3 and ICS-2 interfaces.

See examples: #680 and #681, similar for #621 and #677

We can think of the IBC stack as being composed of 3 semi-independent layers: Light Clients, Core IBC, Applications

Ideally all of these can be upgraded independently without having changes reflected in the other layers.

The ConnectionEnd struct is what negotiates the IBC version, which will then determine what specific parts of state will need to be verified by the clients.

So it makes sense to encapsulate the specifics of what needs to be verified in the struct that is negotiating the IBC version and then delegating verification of just the generic proof to the clients.

For example, I am currently working on #636 which is a core IBC feature and it introduces a new channel type. This requires a new verification method. So in addition to making core IBC support it by making changes to ICS-3 and ICS-4. There would also need to be changes to the ClientState interface to add this new method.

Thus for a chain to support this new feature, they will need to upgrade their core IBC implementation and also any light clients implementation must also upgrade to add the new bespoke verification method.

It would be much better if we could just create generic verify methods in the ClientState interface. Then future verification methods can be trivially added to core IBC without having to also upgrade the light clients as well.

We can also change the proof paths much easier by just upgrading the connection (and thus bumping the core IBC version) without upgrading the light client implementations as well.

Proposal

  1. Introduce new generic verification methods to the ClientState interface:
function VerifyMembership(
    clientState: ClientState,
    height: Height,
    proof: CommitmentProof,
    path: CommitmentPath,
    value: bytes,
) error

function VerifyNonMembership(
    clientState: ClientState,
    height: Height,
    proof: CommitmentProof,
    path: CommitmentPath,
) error

The connection is then completely responsible for creating the commitment path using the CommitmentPrefix and the ICS24 paths, as well as marshalling the value into bytes.

This will make it much easier to upgrade the IBC protocol over time.

  1. Use the generic methods for future verification methods/

  2. Slowly deprecate the usage of the specific verification methods in favor of the generic ones, so that we can change the proof paths later if we decide to

cc: @colin-axner @mpoke @colin-axner

@colin-axner
Copy link
Contributor

Very much in favor of this direction. Excellent proposal. It also makes on chain light clients more general purpose, we could optionally expose usage of client verification via 02-client

@mpoke
Copy link
Contributor

mpoke commented Mar 14, 2022

I agree. This would indeed simplify things.

I'm curious if we could also have a generic method for Proof construction, i.e., queryAndProve...().

@mpoke mpoke changed the title Replace specific VerifyFunctions in ICS-2 with generic VerifyMembership and VerifyNonMembership ICS2: Replace specific VerifyFunctions with generic VerifyMembership and VerifyNonMembership Mar 17, 2022
@mpoke mpoke added the tao Transport, authentication, & ordering layer. label Mar 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tao Transport, authentication, & ordering layer.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants