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

Modify 'IsFrozen' to be a 'Status' function #98

Closed
3 tasks
colin-axner opened this issue Mar 30, 2021 · 0 comments · Fixed by #140
Closed
3 tasks

Modify 'IsFrozen' to be a 'Status' function #98

colin-axner opened this issue Mar 30, 2021 · 0 comments · Fixed by #140
Assignees
Labels
needs discussion Issues that need discussion before they can be worked on
Milestone

Comments

@colin-axner
Copy link
Contributor

Summary

Modify IsFrozen to become a Status function and block any packet processing if the status of the IBC client is not active.

Problem Definition

Core IBC does not account for specific light client notions. It blocks packet processing if the client is frozen but does not check if a tendermint is expired. This can lead to users accidentally sending packets via expired clients if their user interface doesn't make an explicit check before hand. If the counterparty client is expired as well then any tokens sent are effectively lost.

Proposal

Change IsFrozen to return a Status of the client. If the Status != exported.Active, then no packet processing should occur. Alternatively, we could modify IsFrozen to be IsActive

In order for 07-tendermint to determine if a client is expired, it must have access to the latest consensus state. In most situations this can be generalized to all light clients. It is possible a light client might need access to its own client store in order to use metadata set in that store to determine the status of the client. Either signature should be fine, but in order to get the latest consensus state, we need access to the client store so maybe it is best to supply the client store in the function signature.

The benefit of using Status() as opposed to IsActive() is trivial support for a Status gRPC route. If the client state interface only supported IsActive then we would likely need to add a Status gRPC route for every light client as opposed to defining it once in 02-client.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs discussion Issues that need discussion before they can be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant