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

Bring back the upgrade module #174

Closed
RiccardoM opened this issue May 21, 2020 · 13 comments · Fixed by #239
Closed

Bring back the upgrade module #174

RiccardoM opened this issue May 21, 2020 · 13 comments · Fixed by #239
Assignees
Labels
kind/enhancement Enhance an already existing feature; no "New feature" to add
Milestone

Comments

@RiccardoM
Copy link
Contributor

Taken from #142

Currently inside Desmos we were not making usage of the governance and upgrade modules. The first one was not used at all, while the second one was plugged in a couple of versions ago but never properly tested.

I've decided to remove them temporary inside v0.4.0 to avoid having any problem inside our testnets, but we should definitely bring them back in the future.

With this issue I want to track the bringing back of the upgrade module.

@RiccardoM RiccardoM added the kind/enhancement Enhance an already existing feature; no "New feature" to add label May 21, 2020
@RiccardoM RiccardoM added this to the v0.7.0 milestone May 21, 2020
@RiccardoM
Copy link
Contributor Author

Note that when bringing back this module, a micro test should also be performed locally in order to test the upgrading feature, and such test steps should be provided to everyone that might want to try it out as well.

RiccardoM added a commit that referenced this issue May 22, 2020
RiccardoM added a commit that referenced this issue May 26, 2020
See PR #174

(cherry picked from commit 3f849c5)
@RiccardoM
Copy link
Contributor Author

After discussing with @bragaz, we've found out that the x/upgrade module has some problems currently:

  • It will not work unless the nodes run the binary with the --prune=nothing option that makes them archives. This problem is tracked inside issue #5746 inside Cosmos repository.

  • Currently there are still some minor bugs, especially with transactions commands. This is tracked inside issue #6284 and PR #6285.

It looks like the overall module has some issues open related to it (see here and it's tarted to be released inside Cosmos 0.39.

That being said, I'm going to put this issue on hold until those things have been resolved.

@RiccardoM RiccardoM removed this from the v0.7.0 milestone May 27, 2020
@RiccardoM RiccardoM added the status/waiting-upstream This issue is waiting for a dependency to be updated in order to be worked on label May 27, 2020
@anilcse
Copy link

anilcse commented May 27, 2020

@RiccardoM I have been testing the upgrade module from quite a time now. We have tested it on multiple testnets. Meta issue and test scripts need to be handled, but they are not blockers. The critical issue is with pruning. 0.39 should have a fix for this issue. I can help in testing the features

@leobragaz
Copy link
Contributor

leobragaz commented May 27, 2020

@anilcse Is There a date set for the 0.39?
Btw, thanks for the answer! 🙏

@RiccardoM RiccardoM added this to the v0.10.0 milestone Jul 8, 2020
@RiccardoM RiccardoM removed the status/waiting-upstream This issue is waiting for a dependency to be updated in order to be worked on label Jul 8, 2020
@RiccardoM RiccardoM assigned leobragaz and RiccardoM and unassigned leobragaz Jul 13, 2020
@RiccardoM
Copy link
Contributor Author

Hi @anilcse, thanks for your willingness in helping us. I've tried implementing the upgrade module inside our code (this has been done inside the riccardo/upgrade-module branch). Unluckily, I got the following errros when trying to create a proposal:

$ desmoscli tx gov submit-proposal software-upgrade "test" --upgrade-height=100 --from jack
ERROR: invalid proposal content: proposal title cannot be blank [cosmos/cosmos-sdk@v0.38.5/x/gov/types/content.go:38]
$ desmoscli tx gov submit-proposal software-upgrade "test" --upgrade-height=100 --from jack --upgrade-info --help
ERROR: unknown flag: --upgrade-info

Do you have any idea why this might happen? The Cosmos SDK version we're using is v0.38.5

@anilcse
Copy link

anilcse commented Jul 17, 2020

You just need to add "title" flag, it's a mandatory flag for this command.
desmoscli tx gov submit-proposal software-upgrade "test" --title "test software upgrade proposal" --description "something about the proposal here" --upgrade-height=100 --from jack

Then name and title looks bit ambiguous but they have different functionality here.
name suggests the upgrade-name and it will be used inside upgrade-hanlder and should be an unique active proposal name. Title is meta for the proposal.

@RiccardoM
Copy link
Contributor Author

@anilcse Thank you very much! 🙏 Do you by any chance have the reference to some piece of code that performs some migrations inside an upgrade handler that you used in the past? I would like to understand how to handle possibly breaking changes migrations

@anilcse
Copy link

anilcse commented Jul 17, 2020

@RiccardoM you can check these PRs for some example upgrade handlers:
regen-network/regen-ledger#98
akash-network/node#598

These are just example handlers, we don't use them to mint/update stake in general cases. Most of the times the handler will be empty as in: https://github.com/ovrclk/akash/pull/561/files

We can use the handler to update params as well.

@RiccardoM
Copy link
Contributor Author

@anilcse Thank you! Do you by any chance have also some upgrade proposals examples that allow to auto-download the new binary?

@anilcse
Copy link

anilcse commented Jul 17, 2020

@anilcse Thank you! Do you by any chance have also some upgrade proposals examples that allow to auto-download the new binary?

I didn't try that but you can keep the binary information inside info object I guess and cosmosd can take care of the upgrade process automatically.

@RiccardoM
Copy link
Contributor Author

@anilcse I've tried setting up cosmosd to properly handle the upgrades, and I've set DAEMON_RESTART_AFTER_UPGRADE=on. When the upgrade height is reached, the new binary is started and I get the following:

E[2020-07-20|15:40:34.636] BINARY UPDATED BEFORE TRIGGER! UPGRADE "test" - in binary but not executed on chain module=main 
panic: BINARY UPDATED BEFORE TRIGGER! UPGRADE "test" - in binary but not executed on chain

Do you have any idea why that might happen?

@anilcse
Copy link

anilcse commented Jul 20, 2020

Are you running multiple validators? Looks like you stopped old binary before the upgrade height. Or else, you might be facing issues with pruning. If you are using 0.38.x, make sure to use pruning: nothing and otherwise you will face state load issues for upgrades(cosmos/cosmos-sdk#5746)

@RiccardoM
Copy link
Contributor Author

@anilcse I was running a local, single validator node chain. You were right, it was due to me starting the node without specifying --pruning nothing. After setting it the problem was solved and the test upgrade was succesful, Thank you very much! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Enhance an already existing feature; no "New feature" to add
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants