osmosis
osmosis copied to clipboard
Move gov module into Osmosis
Closes: #2481
What is the purpose of the change
Moves the governance module from our SDK fork into osmosis. This will let us more easily work on features for gov, as gov is truly a chain-specific module.
This PR should do everything needed for the initial move. Currently there is a blocker on protobuf generation, where it does not work at the moment.
We need to assess a strategy for this. Options
- Change the proto path for all of these types to be 'osmosis' rather than 'cosmos'. Does this have client side impacts? My understanding is yes, but cc @pyramation
- Override the cosmos in the paths.
We generally want to aim for being query and message compatible with the SDK if possible, but with cadence mismatches.
Brief Changelog
- Move gov package from our SDK fork into this repo.
Testing and Verifying
- This brings over the gov tests as they are, which actually aren't working.
- We need to replace the tests with either Osmosis specific app usage, or by reusing the mock package in SDK gov mainline https://github.com/cosmos/cosmos-sdk/tree/3de5aa87416667305b5606e42d5617aa1126d73e/x/gov
- I prefer the latter, since then we will be able to split this into its own go mod easier & get better test caching improvements.
The first commit just copies the proto & gov folder from the SDK which could be verified by copying these and comparing the diff from https://github.com/osmosis-labs/cosmos-sdk/tree/36adfc0d988d5d67a40e55b560c564988245e7c9/x/gov
Everything else can be reviewed commit by commit
Documentation and Release Note
- Does this pull request introduce a new feature or user-facing behavior changes? yes
- Is a relevant changelog entry added to the
Unreleasedsection inCHANGELOG.md? TODO - How is the feature or change documented? in its own spec
Oh no, proto registers globals within init blobs :(
So if we want to keep cosmos import paths, we must delete gov from the osmosis fork (even more complicated)
I think this points to then making osmosis import paths :(
woaaa amazing work @ValarDragon -- I'm a bit confused on the proto PR needed (ref: https://github.com/osmosis-labs/cosmos-sdk/pull/341). Can you maybe help describe the issue on the next chain dev call? 🙏
This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you!
hey @ValarDragon @alexanderbez I think this was a tag just day before cosmosverse, I must have missed this, pardon the delay!
I think it's totally fine to move the module from cosmos -> osmosis technically, but unless the old gov messages/queries still work, it would definitely break any mintscan, keplr, osmojs voting until the protos are regenerated, any methods/apis exposing protos are also updated, and any software using those methods/apis would also need to be updated.
So in this case, I would say it would require coordination with mintscan and keplr at least.
The old messages/queries should still work under this
I think we still want to have gov in our repo, even if we switch back to mainline.
This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you!
Last commit is just trying to re-run CI
@ValarDragon Are we still good to do this?
Sorry was opened as an example to get CI to run, re-closing for now.