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

Updated outdated GenesisSupplyOffsets to match the sdk fork's bank genesis state constructor #2307

Closed
2 tasks
nicolaslara opened this issue Aug 4, 2022 · 1 comment · Fixed by #2308
Closed
2 tasks
Assignees

Comments

@nicolaslara
Copy link
Contributor

nicolaslara commented Aug 4, 2022

Background

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).

Suggested Design

Remove outdated GenesisSupplyOffsets from the sdk fork and make the necessary changes to both this repo and the sdk fork for the tests to pass

Add a new constructor to x/bank's GenesisState that uses offsets and remove the legacy parameter from the default constructor

Acceptance Criteria

  • Tests pass on both repos
  • Someone that understands the genesis state has reviewed the changes (on both repos) to make sure they're ok
@nicolaslara nicolaslara self-assigned this Aug 4, 2022
@nicolaslara nicolaslara moved this to Needs Review 🔍 in Osmosis Chain Development Aug 4, 2022
@nicolaslara nicolaslara moved this from Needs Review 🔍 to In Progress🏃 in Osmosis Chain Development Aug 4, 2022
@nicolaslara
Copy link
Contributor Author

nicolaslara commented Aug 4, 2022

This branch has my changes to the osmosis repo for this: https://github.com/osmosis-labs/osmosis/tree/supply-offset-changes-on-sdk-fork

And this one has the changes on the sdk fork: osmosis-labs/cosmos-sdk#312

Thanks to @xBalbinus for fixing one of the tests (#2274) and for @p0mvn for pointing it out to me.

@nicolaslara nicolaslara changed the title Remove outdated GenesisSupplyOffsets from the sdk fork Remove outdated GenesisSupplyOffsets from the sdk fork's bank genesis state constructor Aug 5, 2022
@nicolaslara nicolaslara changed the title Remove outdated GenesisSupplyOffsets from the sdk fork's bank genesis state constructor Updated outdated GenesisSupplyOffsets to match the sdk fork's bank genesis state constructor Aug 15, 2022
@nicolaslara nicolaslara moved this from In Progress🏃 to Needs Review 🔍 in Osmosis Chain Development Aug 15, 2022
Repository owner moved this from Needs Review 🔍 to Done ✅ in Osmosis Chain Development Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant