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 router module to Vega upgrade #983

Closed
wants to merge 2 commits into from
Closed

Add router module to Vega upgrade #983

wants to merge 2 commits into from

Conversation

jackzampolin
Copy link
Member

No description provided.

Copy link
Contributor

@yaruwangway yaruwangway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Jack, it is great that we can include router module to gaia vega-rc2 release!
Please confirm if the following code change need to be added:

  • add routertypes.ModuleName in app.mm.SetOrderBeginBlockers and app.mm.SetOrderInitGenesis
  • add fromVM[routertypes.ModuleName] = 0 if the router module do not want to skip init genesis. like this:

    gaia/app/app.go

    Line 565 in 06d6f8f

    fromVM[authz.ModuleName] = 0
  • add the router module store in the store upgrade. like here:

    gaia/app/app.go

    Line 579 in 06d6f8f

    Added: []string{authz.ModuleName, feegrant.ModuleName},

@@ -99,6 +99,10 @@ import (
dbm "github.com/tendermint/tm-db"

gaiaappparams "github.com/cosmos/gaia/v6/app/params"
router "github.com/strangelove-ventures/packet-forward-middleware/router"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to write "router" if the alias is the same name as the package

Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to go into main then backported into the release branch. Otherwise, LGTM.

@jackzampolin
Copy link
Member Author

Ok so this is good, but I just want to make sure to test the e2e with this build. Should have this done by eow

@jackzampolin
Copy link
Member Author

Will rebase against main

@jackzampolin jackzampolin changed the base branch from vega-rc2 to main September 22, 2021 21:15
@okwme
Copy link
Contributor

okwme commented Oct 13, 2021

this work was cherry-picked by @yaruwangway and added to the vega-rc2 branch as part of #979. Going to close this PR unless there's another reason to keep it open.

@okwme okwme closed this Oct 13, 2021
@tac0turtle tac0turtle deleted the jack/vega-rc2 branch December 14, 2021 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants