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

Migrate governance proposals from v1beta1 to v1 #1282

Closed
5 of 8 tasks
Tracked by #1310
colin-axner opened this issue Apr 21, 2022 · 6 comments · Fixed by #4620
Closed
5 of 8 tasks
Tracked by #1310

Migrate governance proposals from v1beta1 to v1 #1282

colin-axner opened this issue Apr 21, 2022 · 6 comments · Fixed by #4620
Labels
epic type: feature New features, sub-features or integrations
Milestone

Comments

@colin-axner
Copy link
Contributor

colin-axner commented Apr 21, 2022

Summary

Our existing governance proposals are based on the SDK version v1beta1 of the x/gov module. Updating to v1 of the x/gov module requires breaking changes.

Problem Definition

Instead of defining governance proposal handlers and proposal types, v1 of the gov module simply executes a list of messages. This is beneficial for many reasons, but one primary one is the ability to atomically commit multiple messages in a single governance proposal. For us this could help with:

  • updating many expired clients at once
  • simplifying UpgradeProposal (currently wraps calls to x/upgrade), an IBC chain upgrade could simply be combined with a x/upgrade message
  • delegation of responsibility for IBC related proposals to trusted groups (edit)

The new approach is also cleaner as there is less fragmentation in handlers/types.

Using v1beta1 with a MsgExecLegacyContent works alright. You can still atomically commit multiple messages with legacy code, but it does not allow for delegation of authority to trusted groups

Proposal

Readjust our proposal handlers into msg_server handler and change our proposal types to message types. The fixes are primarily for code hygiene and keeping up to date with the SDK, so on the lower priority side. This issue should be split up into smaller issues for each proposal type.

The existing governance handlers should be removed (and the registration on the router in simapp should be removed). We should add migration documentation explaining how to use the messages

Approved signers can be a parameter in IBC

Issues and tasks


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@catShaark
Copy link
Contributor

Can I try doing this ?

@crodriguezvega
Copy link
Contributor

Hey @catShaark, thanks for asking! As Colin mentions above, this issue has a low priority for us at the moment. If you want to work on it and want to create already PRs before we merge the PR for the upgrade to SDK 0.46, then you might need to end up opening your PRs again against main once the upgrade is merged, since we definitely would want to merge these things separately to main. For that reason, it might be wise to wait a bit before taking this issue...

But we see that you are very active lately in the repo: we ❤️ it and we are very thankful for your contributions! Are you planning to sustain a certain level of commitment to work on ibc-go issues? If that would be the case then I think it would make sense if we have a chat and we can discuss what other issues you could pick up that have higher priority for us. Let me know what you think!

@catShaark
Copy link
Contributor

Hey @catShaark, thanks for asking! As Colin mentions above, this issue has a low priority for us at the moment. If you want to work on it and want to create already PRs before we merge the PR for the upgrade to SDK 0.46, then you might need to end up opening your PRs again against main once the upgrade is merged, since we definitely would want to merge these things separately to main. For that reason, it might be wise to wait a bit before taking this issue...

Ohhh, thanks for letting me know that, that's why I ask you guys so that I know if I should do it or not.

But we see that you are very active lately in the repo: we ❤️ it and we are very thankful for your contributions! Are you planning to sustain a certain level of commitment to work on ibc-go issues? If that would be the case then I think it would make sense if we have a chat and we can discuss what other issues you could pick up that have higher priority for us. Let me know what you think!

Yess I'd love to consistently contribute to ibc-go cuz ibc is a very awesome piece of software.

@crodriguezvega
Copy link
Contributor

@catShaark, are you on Discord? I would like to contact you to have a chat and Github is not the greatest platform to have a build conversation... :) I am Carlos | Interchain#9176. You can find me in Cosmos Network or IBC gang discord servers.

@catShaark
Copy link
Contributor

@catShaark, are you on Discord? I would like to contact you to have a chat and Github is not the greatest platform to have a build conversation... :) I am Carlos | Interchain#9176. You can find me in Cosmos Network or IBC gang discord servers.

I'll contact you on discord, thanks so much

@crodriguezvega crodriguezvega added the type: refactor Architecture, code or CI improvements that may or may not tackle technical debt. label Sep 22, 2022
@crodriguezvega crodriguezvega moved this to Backlog in ibc-go Sep 22, 2022
@crodriguezvega crodriguezvega moved this to Backlog in ibc-go Nov 1, 2022
@crodriguezvega crodriguezvega added this to the v8.0.0 milestone Mar 27, 2023
@crodriguezvega crodriguezvega moved this from Backlog to Todo in ibc-go Mar 27, 2023
@colin-axner
Copy link
Contributor Author

When handling this issue, we should add the ability for governance to specify additional authorities. This would allow governance grant authority of specific message types. ref #3326

@crodriguezvega crodriguezvega added type: feature New features, sub-features or integrations and removed type: refactor Architecture, code or CI improvements that may or may not tackle technical debt. labels Jun 25, 2023
@crodriguezvega crodriguezvega moved this from Todo to In progress in ibc-go Sep 2, 2023
@charleenfei charleenfei moved this from In progress to In review in ibc-go Sep 12, 2023
@github-project-automation github-project-automation bot moved this from In review to Done in ibc-go Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic type: feature New features, sub-features or integrations
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants