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

Support non IBC upgrade genesis restarts #1152

Closed
5 tasks
Tracked by #3197
colin-axner opened this issue Jul 6, 2021 · 3 comments · Fixed by #3210
Closed
5 tasks
Tracked by #3197

Support non IBC upgrade genesis restarts #1152

colin-axner opened this issue Jul 6, 2021 · 3 comments · Fixed by #3210
Assignees
Labels
I: CLI Internal: related to the relayer's CLI
Milestone

Comments

@colin-axner
Copy link
Contributor

Crate

relayer

Summary

When a chain undergoes a genesis restart which does not utilize an IBC upgrade proposal, the chain history will be reset. Hermes will attempt to get the trusted header after the restart, but it was erased during the genesis restart:

Jul 06 10:48:19.529 ERROR failed during a client operation: error raised while updating client: failed building header with error: Light client error for RPC address network1-1: Internal error: height 26 is not available, lowest height is 1101 (code: -32603):

Problem Definition

While using in process upgrades are ideal, chains will still opt for genesis restarts from time to time. Not all of these restarts will require an IBC upgrade. This is a bit of an edge case scenario, but if it does occur then it'd be a pain to try to fix live since it would need to be fixed before the client expires

Proposal

I'm not sure what the best solution is. Short term patches might include creating a genesis restart specific command to update a client by querying a node which hasn't been reset for the trusted header. Alternatively there could be a command to rely on historical entries, but these might be pruned after 24 hours. The best solution would likely rely on a cache of old headers. I think the trusted header must have been referenced at some point in the past to update the client. Maybe keeping this header around could be the solution, but I imagine this might be a larger change.

I mostly wanted to open this issue as food for thought in case there are other ongoing structural changes that might allow for this to be handled smoothly

Acceptance Criteria

Hermes can update clients after non IBC upgrade genesis restarts


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@adizere adizere added this to the 08.2021 milestone Jul 6, 2021
@adizere adizere modified the milestones: 08.2021, Backlog Aug 3, 2021
@adizere adizere self-assigned this Aug 8, 2022
@adizere adizere modified the milestones: Backlog, v1.1 Aug 8, 2022
@sainoe
Copy link
Contributor

sainoe commented Aug 10, 2022

Hi, I have encountered this same issue in the context of building a testnet for Interchain Security.

The steps to reproduce the issue:

  1. Clone the IS-testnet repo
git clone https://github.com/sainoe/IS-testnet
cd ./is-testnet/docker/
  1. Run the container and the testnet
docker build --tag is-testnet .
make run-testnet
  1. Restart the provider chain
make restart-provider
  1. Delegate tokens (send a IBC packet through CCV)
make delegate
  1. Check the error message in the relayer logs
make relayer-logs

Note that sometime the relayer has to be restarted using make restart-relayer to observe the issue.

@romac romac modified the milestones: v1.1, v1.3 Oct 27, 2022
@seanchen1991 seanchen1991 modified the milestones: v1.3, Backlog Dec 13, 2022
@sainoe sainoe modified the milestones: Backlog, v1.3 Jan 4, 2023
@adizere adizere removed their assignment Feb 7, 2023
@romac romac modified the milestones: v1.3, v1.4 Feb 17, 2023
@seanchen1991 seanchen1991 added the I: CLI Internal: related to the relayer's CLI label Mar 17, 2023
@romac romac modified the milestones: v1.4, v1.5 Mar 29, 2023
romac added a commit that referenced this issue Apr 6, 2023
Closes: #1152

This PR adds two optional flags to the `hermes update client` command which allow the update to retrieve blocks from a separate Node when updating a client.

The two flags are:

* `--archive-address`
* `--restart-height`

If one of the flags is set, the other must also be set.

## Testing

It is possible to test this solution using the branch `luca_joss/setup-with-gm` from the forked repo, https://github.com/ljoss17/ICS-testnet/tree/main, with the following instructions: https://github.com/ljoss17/ICS-testnet/blob/luca_joss/setup-with-gm/docker/TEST_RESTART.md

---------

* WIP: adding a new flag to specify an address when updating client using CLI

* Add a flag to specify halted height

* Add documentation related to genesis restart without IBC upgrade proposal

* Improved address selection when 'archive-address' is given

* Handle case where the light client needs to do bisection across the restart height (#3227)

* Rename 'halted-height' to 'restart-height'

* Add unclog entry

---------

Co-authored-by: Romain Ruetschi <romain@informal.systems>
@romac
Copy link
Member

romac commented Apr 6, 2023

@colin-axner @sainoe @adizere Should you ever run into this again, we now added two new flags to update client which would let you update a client across the restart by using an archive node and specifying its RPC address as well as the restart height (ie. the lowest block available on the node that underwent the restart):

hermes update client --host-chain consumer --client 07-tendermint-0 --archive-addr http://some.archive.node --restart-height 1101

Hermes will then use the archive node to fetch blocks below 1101 and the live node for the blocks at and above 1101, which enables it to update the client across the restart height.

@colin-axner
Copy link
Contributor Author

Thank you @romac! This is so cool! Thanks y'all for your work 🙏

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
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

6 participants