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

TendermintClient::verify_upgrade_and_update_state is not implemented #19

Closed
5 tasks
romac opened this issue Apr 6, 2022 · 1 comment · Fixed by #349
Closed
5 tasks

TendermintClient::verify_upgrade_and_update_state is not implemented #19

romac opened this issue Apr 6, 2022 · 1 comment · Fixed by #349
Assignees
Labels
A: bug Admin: something isn't working A: urgent Admin: high priority issue
Milestone

Comments

@romac
Copy link
Member

romac commented Apr 6, 2022

Related to: informalsystems/hermes#722

Summary

The TendermintClient::verify_upgrade_and_update_state is currently not implemented, even though it is called in the UpgradeClient handler.

https://github.com/informalsystems/ibc-rs/blob/efbf7d18512fe6efcb593de68735b7f8783fd652/modules/src/clients/ics07_tendermint/client_def.rs#L395-L403

https://github.com/informalsystems/ibc-rs/blob/efbf7d18512fe6efcb593de68735b7f8783fd652/modules/src/core/ics02_client/handler/upgrade_client.rs#L52-L57

Problem Definition

The UpgradeClient is therefore non-functional at the moment, as it will panic once the method is called.

Proposal

Implement the verify_upgrade_and_update_state method for TendermintClient.

Acceptance Criteria

The verify_upgrade_and_update_state is implemented in TendermintClient, and its behavior is captured in a test for the UpgradeClient handler.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@hu55a1n1 hu55a1n1 transferred this issue from informalsystems/hermes Sep 28, 2022
@plafer plafer added A: bug Admin: something isn't working A: urgent Admin: high priority issue labels Dec 13, 2022
@plafer
Copy link
Contributor

plafer commented Dec 13, 2022

Until we know how to implement this properly, we should return an Error instead of crashing (either at the tendermint light client level, or even at the core handler level).

Related: #141

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: bug Admin: something isn't working A: urgent Admin: high priority issue
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants