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.

Sign up for GitHub

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

Jump to bottom

Integrating upgrade module with gaia #8

Closed
sahith-narahari opened this issue Oct 26, 2019 · 17 comments
Closed

Integrating upgrade module with gaia #8

sahith-narahari opened this issue Oct 26, 2019 · 17 comments
Assignees
@ethanfrey
Milestone
Rhine

Comments

@sahith-narahari
Copy link

Context

  • Currently the upgrade module can be tested only using simulation. I would like to propose integrating upgrade module with latest release of cosmos/gaia(v2.0.2). This would help in testing integration of upgrade module with the application.
  • The same PR can help for cosmos/gaia after upgrade PR is merged to cosmos-sdk

Proposed Design

  1. Integrate upgrade module with cosmos/gaia
  2. Create 2 releases with different version in regen-network/gaia
  3. Run a local testnet to check integration and working of upgrade module

@ethanfrey @aaronc
@aaronc
Copy link
Member

aaronc commented Oct 26, 2019 via email

@ethanfrey
Copy link

There is a relevant comment on this here cosmos#4233 (comment) (basically trying to get the info to Bez and Jack to demo).

Building those PRs on gaiad would be a nice step, definitely link to that upgrade PR when done

@ethanfrey ethanfrey added this to the Rhine milestone Oct 28, 2019
@ethanfrey
Copy link

Locations:

Work should be done on https://github.com/regen-friends/gaia in a branch gaia-upgrades. The branch should be off gaia:master or the closest code that is stable.

This should reference as dependency the branch regen-network:regen-network/upgrade-module as referenced here: cosmos#4233

If there are problems with setting up the dependencies or compiling a gaia compatible with this branch, please advice. Once finished, please make a PR from this same repo into master for internal review. Once we have internal review, we can submit the PR against cosmos/gaia

@ethanfrey
Copy link

It turns out that the most recent gaia https://github.com/cosmos/gaia/blob/release/v2.0.2/go.mod references cosmos-sdk v0.37.3, which is incompatible with current cosmos-sdk:master, and therefore the upgrade PR.

I will communicate with Jack/Bez three options:

  • They produce a gaia version off cosmos-sdk:master and we PR on that
  • They use regen-ledger v0.4.1 to v0.5.0 upgrade to see the (old version of the) PR in action - at least get a feel for how it works.
  • They don't test this in binaries

@ethanfrey ethanfrey self-assigned this Oct 29, 2019
@ethanfrey
Copy link

Please submit a PR or branch showing the proper go.mod changes and the compile error.

@sahith-narahari
Copy link
Author

https://github.com/regen-friends/gaia/tree/gaia-upgrades
This branch is from v2.0.2(master is with even older cosmos version i.e 0.34),x/genaccounts module(implemented in v0.37.3 but missing in master of cosmos-sdk) looks like the cause of problem here.

@ethanfrey
Copy link

Unblocked. Turns out the versions in go.mod are useless to follow.
And master is based off a recent master.

I updated this and https://github.com/regen-friends/gaia/tree/gaia-upgrades now builds. It is gaia:master with the one change to point to our fork with x/upgrades in it.

You should be unblocked to continue the integration work.

@ethanfrey
Copy link

We did integrate x/upgrade with regen-ledger based on the 0.34 version of the sdk. So, some things may have changed, but take a look here (and search for upgrade) to see the points where it needs to be plugged in:

https://github.com/regen-network/regen-ledger/blob/master/app/app.go

In particular here, which most modules don't do: https://github.com/regen-network/regen-ledger/blob/master/app/app.go#L220

The SetUpgradeHandler was just for the demo and not needed here. But, if you make a "post-upgrade" gaia version, it will need a SetUpgradeHandler with a chosen name and some random action to demonstrate the upgrade - here we gave tokens to the faucet as a demo.

@ethanfrey
Copy link

Once the integration is done and you want to test it, you will need the second binary (a branch on top with one more commit). And a rough description of how to do it is here: cosmos#4233

It would be good to document this better as you perform the upgrade, as someone following the instructions the first time will notice all the missing steps that are implied.

  • set voting_params.voting_period in genesis.json to something short like 1 or 2 minutes locally
  • start my chain
  • build a new binary with a dummy upgrade handler:
  • app.upgradeKeeper.SetUpgradeHandler("test1", func(ctx sdk.Context, plan upgrade.Plan) {
    // optionally test something here like a store upgrade or creating some new coins
    }
  • create a software upgrade gov proposal for that upgrade (i.e. "test1"). You've changed the cmd line params, but I do something like xrncli tx gov submit-proposal software-upgrade --upgrade-name test1 --title "test1" --description "upgrade" --upgrade-height 300 --deposit 1000tree --from my-key or --upgrade-time 2019-06-04T17:43:00Z if I'm testing based on time vs block. I aim the block or time for a minute or so after my voting window will end
  • vote yes on the proposal: xrncli tx gov vote yes --from my-key
  • wait for the blockchain to panic and spit out the upgrade message
  • start the new binary, confirm the upgrade happens --or-- alternatively do the test with https://github.com/regen-network/cosmosd for a fully automated upgrade

@sahith-narahari
Copy link
Author

We need to pass upgrade cli and rest to gov's proposal handler which is missing in current implementation of x/upgrade client.Current implementation has module_client which was used in older versions of sdk to club all client commands.

Can we change this,as per current implementation.This is blocking for integrating upgrade with gaiacli.
@aaronc

@ethanfrey
Copy link

The client was only tested with the 0.34 build, and I dragged it along into the rewrite.
Please feel free to fix the upgrade module implementation and use your client fixes for the gaia integration.

Then make a PR from your client updates to the PR branch cosmos#4233

@sahith-narahari
Copy link
Author

There's an issue with --height flag here,giving me error of "failed to load state at height",whereas replacing flag by upgrade-height solves the problem.
Maybe there's some default logic for flag height in baseapp,that's being adopted here.

@sahith-narahari
Copy link
Author

regen-friends/gaia#1
Integrated gaia but chain intialisation on local failed with consensus failure.Looks like an issue with begin block from stack trace.
Client working perfectly,tested submitting and passing proposal for software-upgrades

@sahith-narahari
Copy link
Author

@ethanfrey @aaronc, I've finished integration and tested successfully with a proposal. Also added upgrade-demo.md to sdk/docs which can be used to test the upgrade
regen-friends/gaia#2

@ethanfrey
Copy link

Okay, I will look again.
Sounds like upstream gaia:master is now working with cosmos-sdk:master, so I will likely do a bit of branch cleanup on that PR as well

@sahith-narahari
Copy link
Author

@aaronc We can close this as it's merged to cosmos/gaia
cosmos/gaia#184

@clevinson
Copy link
Member

Closng tihs ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ethanfrey ethanfrey

Labels
None yet
Projects
None yet
Milestone
Rhine
Development

No branches or pull requests
4 participants
@aaronc @clevinson @ethanfrey @sahith-narahari