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

Add migrations for solo machine protobuf types #1863

Closed
4 of 8 tasks
colin-axner opened this issue Aug 2, 2022 · 3 comments · Fixed by #2897
Closed
4 of 8 tasks

Add migrations for solo machine protobuf types #1863

colin-axner opened this issue Aug 2, 2022 · 3 comments · Fixed by #2897

Comments

@colin-axner
Copy link
Contributor

colin-axner commented Aug 2, 2022

Summary

Todo:


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@colin-axner
Copy link
Contributor Author

remove Allow boolean in solo machine client state as well

@crodriguezvega crodriguezvega moved this to In progress in ibc-go Aug 4, 2022
@crodriguezvega crodriguezvega moved this from In progress to Todo in ibc-go Aug 19, 2022
@crodriguezvega crodriguezvega moved this to Todo in ibc-go Nov 1, 2022
@colin-axner colin-axner removed their assignment Nov 7, 2022
@colin-axner
Copy link
Contributor Author

colin-axner commented Nov 16, 2022

I did some initial work for this since the issue lacked sufficient information to get started. https://github.com/cosmos/ibc-go/tree/colin/1863-v7-migrations

What I have done:

  • reused v1 to v2 consensus version migrations and renamed them to v2 to v3
  • moved localhost migration to v2 to v3 migrations and made it automatic
  • moved all migrations into 02-client/migrations/v7
  • removed addition of metadata (used in v1 to v2)
  • updated solomachine migration to be v2 to v3 proto definition
  • simplified genesis migration

What's left?

  • review tests/cleanup
  • probably move localhost_test.go into store_test.go
  • split tendermint pruning into separate migration function
  • add logging

Note: It will not be possible to upgrade from stargate -> v7

@colin-axner
Copy link
Contributor Author

Further thoughts:

Let's move tendermint expired consensus state pruning into an optional migration which chains can call directly. I like the idea of optimizations being optional and required migrations being automatic. Iterating over all clients/consensus states can actually take quite a bit of time (a year ago it took like an hour), so I think it makes sense to skip as automatic. It could also be reused during any upgrade. Only iterating over solo machine/localhost clients should be a lot faster!

Lets also add logging to the expired tendermint pruning. Might be nice to log the number of consensus states pruned. This can inform a chain whether it is worth running the migration or not since even if 2 consensus states are pruned, all consensus states will be iterated

@colin-axner colin-axner self-assigned this Nov 21, 2022
@colin-axner colin-axner moved this from Todo to In progress in ibc-go Nov 24, 2022
@colin-axner colin-axner moved this from In progress to In review in ibc-go Nov 24, 2022
Repository owner moved this from In review to Todo in ibc-go Dec 7, 2022
@damiannolan damiannolan moved this from Todo to Done in ibc-go Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done 🥳
Development

Successfully merging a pull request may close this issue.

1 participant