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

Client update fails if the client is already up-to-date #734

Closed
5 tasks
adizere opened this issue Mar 10, 2021 · 0 comments · Fixed by #723
Closed
5 tasks

Client update fails if the client is already up-to-date #734

adizere opened this issue Mar 10, 2021 · 0 comments · Fixed by #723
Assignees
Labels
I: CLI Internal: related to the relayer's CLI I: logic Internal: related to the relaying logic O: usability Objective: cause to improve the user experience (UX) and ease using the product
Milestone

Comments

@adizere
Copy link
Member

adizere commented Mar 10, 2021

Crate

relayer

Summary of Bug

If a client has a consensus state at height x and the latest height for the target chain is x, then Hermes will report success upon trying to update the consensus state of the client, despite encountering an error:

$ hermes tx raw update-client ibc-1 ibc-0 07-tendermint-3
    Finished dev [unoptimized + debuginfo] target(s) in 0.18s
     Running `target/debug/hermes tx raw update-client ibc-1 ibc-0 07-tendermint-3`
{"status":"success","result":{"ChainError":"deliver_tx reports error: log=Log(\"failed to execute message; message index: 0: cannot update client with ID 07-tendermint-3: header height ≤ consensus state height (0-400 ≤ 0-400): invalid client header\")"}}

Version & reproducing

This is easy to reproduce if the target chain is halted (pending an upgrade).
See branch adi/357_upgrade and PR #723 .

Fix description

The patch introduced in #723 solves the present issue by changing Hermes output to the following:

{"status":"error","result":"tx error: error raised while updating client: Client 07-tendermint-4 is already up-to-date with chain ibc-0@revision: 0, height: 400"}


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@adizere adizere added I: CLI Internal: related to the relayer's CLI I: logic Internal: related to the relaying logic O: usability Objective: cause to improve the user experience (UX) and ease using the product labels Mar 10, 2021
@adizere adizere added this to the 03.2021 milestone Mar 10, 2021
@adizere adizere self-assigned this Mar 10, 2021
adizere added a commit that referenced this issue Mar 11, 2021
romac added a commit that referenced this issue Mar 23, 2021
* Added domain type def.

Also cleaned-up the documentation, which looked very sloppy.
https://docs.rs/ibc/0.1.1/ibc/ics02_client/msgs/index.html

* Added partial handler & command

* Added upgrade proto files. Added Cargo.lock for proto-compiler

* Prep for query_upgraded_client_state

* Method & support for querying ugpraded client state

* Support for querying the upgraded consensus state

* Minor dev scripts enhancements & bugs

* Added guide. Uses patched Go relayer

* Proto conversion for upgrade msg. Refactored upgrade() impl

* Fix missing signer bug

* Update msg bf. upgrade. Event parsing

* Changelog. Revised guide

* Aesthetic nits based on file review

* Clarifications in the test instructions

* Documented Go relayer version in testing instructions

* Possible fix for #734

* changelog & method documentation

* Apply suggestions from code review

Co-authored-by: Romain Ruetschi <romain@informal.systems>

* Added more derived trait bounds on module events

* Adapt to newer Ics02 structure.

* Added Protobuf impl for MsgUpgradeAnyClient

* FMT

* Added upgrade-chain CLI and updated instructions

* Remove a clone and turn zero_custom_fields into a static method

* Whitespace and nitpick

* Remove obsolete TODO

Co-authored-by: Romain Ruetschi <romain@informal.systems>
Co-authored-by: Anca Zamfir <zamfiranca@gmail.com>
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this issue Sep 13, 2022
* Added domain type def.

Also cleaned-up the documentation, which looked very sloppy.
https://docs.rs/ibc/0.1.1/ibc/ics02_client/msgs/index.html

* Added partial handler & command

* Added upgrade proto files. Added Cargo.lock for proto-compiler

* Prep for query_upgraded_client_state

* Method & support for querying ugpraded client state

* Support for querying the upgraded consensus state

* Minor dev scripts enhancements & bugs

* Added guide. Uses patched Go relayer

* Proto conversion for upgrade msg. Refactored upgrade() impl

* Fix missing signer bug

* Update msg bf. upgrade. Event parsing

* Changelog. Revised guide

* Aesthetic nits based on file review

* Clarifications in the test instructions

* Documented Go relayer version in testing instructions

* Possible fix for informalsystems#734

* changelog & method documentation

* Apply suggestions from code review

Co-authored-by: Romain Ruetschi <romain@informal.systems>

* Added more derived trait bounds on module events

* Adapt to newer Ics02 structure.

* Added Protobuf impl for MsgUpgradeAnyClient

* FMT

* Added upgrade-chain CLI and updated instructions

* Remove a clone and turn zero_custom_fields into a static method

* Whitespace and nitpick

* Remove obsolete TODO

Co-authored-by: Romain Ruetschi <romain@informal.systems>
Co-authored-by: Anca Zamfir <zamfiranca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: CLI Internal: related to the relayer's CLI I: logic Internal: related to the relaying logic O: usability Objective: cause to improve the user experience (UX) and ease using the product
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant