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

ADR 004 Geneis Migration #5589

Conversation

alexanderbez
Copy link
Contributor

@alexanderbez alexanderbez commented Jan 29, 2020

Part III (final) PR of #5572 -- genesis migration for ADR 004 slated for v0.39:

  • Remove coins from JSON encoding of existing accounts in the x/auth genesis state.
  • Add existing account coins to new balances genesis in x/bank.

For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@@ -46,15 +46,15 @@ type (

BaseAccount struct {
Address sdk.AccAddress `json:"address" yaml:"address"`
Coins sdk.Coins `json:"coins" yaml:"coins"`
Coins sdk.Coins `json:"coins,omitempty" yaml:"coins,omitempty"`
Copy link
Contributor Author

@alexanderbez alexanderbez Jan 29, 2020

Choose a reason for hiding this comment

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

I've decided to add omitempty here to avoid having to literally copy the entire file solely to remove a single field.

Copy link
Member

Choose a reason for hiding this comment

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

Like this move!

@alexanderbez alexanderbez added R4R and removed WIP labels Jan 29, 2020
@alexanderbez alexanderbez marked this pull request as ready for review January 29, 2020 15:42
@alexanderbez alexanderbez mentioned this pull request Jan 29, 2020
13 tasks
// it to v0.39 x/bank genesis state. The migration includes:
//
// - Moving balances from x/auth to x/bank genesis state.
func Migrate(bankGenState v038bank.GenesisState, authGenState v038auth.GenesisState) GenesisState {
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to ensure that this Migrate runs before auth.Migrate? Is there anything that needs to be done to ensure that?

"github.com/cosmos/cosmos-sdk/x/genutil"
)

func Migrate(appState genutil.AppMap) genutil.AppMap {
Copy link
Member

Choose a reason for hiding this comment

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

Thats done here I see.

@alexanderbez alexanderbez merged commit 59592cf into bez/5519-adr-004-bank-balances-ii Jan 30, 2020
@alexanderbez alexanderbez deleted the bez/5519-adr-004-bank-balances-gen-migration branch January 30, 2020 04:20
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.

3 participants