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

[Bug]: server/v2 lacks support for unsafe-skip-upgrades #22238

Closed
1 task done
kocubinski opened this issue Oct 11, 2024 · 1 comment · Fixed by #22682
Closed
1 task done

[Bug]: server/v2 lacks support for unsafe-skip-upgrades #22238

kocubinski opened this issue Oct 11, 2024 · 1 comment · Fixed by #22682
Assignees
Labels
C:server/v2 Issues related to server/v2 T:Bug

Comments

@kocubinski
Copy link
Member

Is there an existing issue for this?

  • I have searched the existing issues

What happened?

It is not clear from the help text if this is possible, so I'm assuming it's not, please correct me if I'm wrong.

❯ simd start -h
...
...
      --transport string                                Transport protocol: socket, grpc (default "socket")
      --unsafe-skip-upgrades ints                       Skip a set of upgrade heights to continue the old binary

Compare this to simdv2 output:

❯ simdv2 start -h
...
...
      --server.minimum-gas-prices string                Minimum gas prices to accept for transactions; Any fee in a tx must meet this minimum (e.g. 0.01photino;0.0001stake)
# end of list

Cosmos SDK Version

main

How to reproduce?

No response

@github-project-automation github-project-automation bot moved this to 📋 Backlog in Cosmos-SDK Oct 11, 2024
@julienrbrt
Copy link
Member

Mmh, this is only useful for x/upgrade. In practice, with viper binding, setting a unsafe-skip-upgrades directly in the app.toml or config.toml will make it work with x/upgrade:

skipUpgrades := cast.ToIntSlice(in.DynamicConfig.Get(server.FlagUnsafeSkipUpgrades))

If we add it to the server/v2 config, then this flag would still be present when the x/upgrade module isn't in the chains (in baseapp world that's the case too and that's meh). Additionally, it would be prefixed by the server name, meaning we'd need to have a case for v1 and v2.

Not sure what is best 🤔

@julienrbrt julienrbrt self-assigned this Oct 25, 2024
@julienrbrt julienrbrt added the C:server/v2 Issues related to server/v2 label Nov 20, 2024
@julienrbrt julienrbrt moved this from 📋 Backlog to 🤸‍♂️ In Progress in Cosmos-SDK Nov 28, 2024
@julienrbrt julienrbrt moved this from 🤸‍♂️ In Progress to 👀 Waiting / In review in Cosmos-SDK Nov 28, 2024
@github-project-automation github-project-automation bot moved this from 👀 Waiting / In review to 🥳 Done in Cosmos-SDK Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:server/v2 Issues related to server/v2 T:Bug
Projects
Status: 🥳 Done
Development

Successfully merging a pull request may close this issue.

2 participants