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

fix: missing line to fix upgrade client router key functionality #857

Closed
wants to merge 1 commit into from

Conversation

McDaan
Copy link

@McDaan McDaan commented May 3, 2022

Description:

The main purpose is to unfreeze the Umee <> Evmos relayer without creating new channels. It's better to update IBC client IDs than do so. The unfreezening must be made through a update-client governance proposal type, which is not working right now on Umee.

Since the team is busy due to Umeemania testnet, I wanted to make a step forward and take the initiative in regards of this issue. Only one line was missing in order to fix it (a priori). Probably this will require an upgrade (or maybe just a binary update in production), as in Evmos fixing this will require an upgrade since it's a breaking change.
Link: https://github.com/tharsis/evmos/pull/537/files)

We need to solve this as soon as possible to be able to replace the IBC client that is currently frozen for the new one, which is not frozen. By the way, as far as I can see, v3 of ibc-go is needed, so I assume that my PR change is not the only one that needs to be made.

Kind regards,
Daniel (Mandragora)

closes: #XXXX


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added appropriate labels to the PR
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

@McDaan McDaan requested a review from a team as a code owner May 3, 2022 12:02
@toteki
Copy link
Member

toteki commented May 3, 2022

I greenlit the CI checks so they can run on this PR

For context though, is this related to the mainnet + ibc + evmos thing? Add relevant information to the PR's description so we know how to evaluate it and what purpose it serves.

@McDaan
Copy link
Author

McDaan commented May 3, 2022

I greenlit the CI checks so they can run on this PR

For context though, is this related to the mainnet + ibc + evmos thing? Add relevant information to the PR's description so we know how to evaluate it and what purpose it serves.

Yes, it is related to what you are saying. Just updated the PR description with relevant information about it.

@facundomedica
Copy link
Contributor

Hey @McDaan! I think you missed some other lines, you only added the import to the app.go file. You also need to add the router handler and the IBC v3 module to the go.mod file. That being said, I'm not sure if we can just jump to IBC v3 yet 🤔

@McDaan
Copy link
Author

McDaan commented May 4, 2022

Hey @McDaan! I think you missed some other lines, you only added the import to the app.go file. You also need to add the router handler and the IBC v3 module to the go.mod file. That being said, I'm not sure if we can just jump to IBC v3 yet 🤔

Hi there @facundomedica! Yes, I am not familiarized with SDK modifications, so I took Evmos case as a example. It seems that IBC v2 can be compatible with IBC v3 one, but since I am not sure I will let you guys to decide the way to go. Anyway, thank you so much for your explanation!

Taking into account that dev team already is digging into it and created a new PR (#860) in order to implement the changes I recommended, I think we can finally close this one.

@McDaan McDaan closed this May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants