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

fix: Remove genesis supply offsets #305

Closed
wants to merge 11 commits into from

Conversation

nicolaslara
Copy link

@nicolaslara nicolaslara commented Aug 4, 2022

Needed for osmosis-labs/osmosis#2307

What is the purpose of the change

The sdk fork is using GenesisSupplyOffsets in the banktypes.NewGenesisState() constructor. This is incompatible with the ibc-go testing package which expects the new signature (https://github.com/cosmos/ibc-go/tree/main/testing).

Brief Changelog

  • Updated the constructor to match the expected interface.
  • Added a new constructor that can be used for the previous behaviour

Testing and Verifying

Tests pass

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? (no)
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? (no)
  • How is the feature or change documented? (not applicable)

@ValarDragon
Copy link
Member

We should change NewGenesisState as done here, and make a new method for NewGenesisStateWithSupplyOffsets I think for now.

@nicolaslara nicolaslara changed the title Remove genesis supply offsets fix: Remove genesis supply offsets Aug 5, 2022
@nicolaslara
Copy link
Author

Updated according to @ValarDragon's comments. Less changes this way (even if we keep the legacy GenesisSuplyOffsets around)

@nicolaslara
Copy link
Author

@alexanderbez I tried merging main into this branch, but had issues with the integrations on the osmosis side. Are the changes from a296bdc already reflected on osmosis/main?

@nicolaslara nicolaslara marked this pull request as ready for review August 15, 2022 07:28
@nicolaslara nicolaslara requested a review from a team August 15, 2022 07:28
@nicolaslara
Copy link
Author

Closing in favor of #312

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants