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

Fork x/gov #4

Merged
merged 12 commits into from
Feb 9, 2024
Merged

Conversation

tbruyelle
Copy link
Collaborator

@tbruyelle tbruyelle commented Feb 9, 2024

Copy cosmos-sdk/x/gov to x/gov to allow some customization.

Closes #5

  • copy cosmos-sdk/x/gov
  • change package path to github.com/atomone-hub/govgen/v1/x/gov
  • remove migration and legacy stuff
  • use legacy router because proposal handlers from other modules returns the legacy handlers
  • fork cosmos-sdk/simapp so gov tests can compile (need to replace simapp.GovKeeper by the govgen one)
  • update error code from registered errors to avoid registration panic because of duplicate
  • fix duplicate proto registration (by replacing x/gov/types with legacy one in the imports)
     $ gotest ./x/gov/
     2024/02/09 15:30:06 proto: duplicate proto type registered: cosmos.gov.v1beta1.GenesisState
     panic: proto: duplicate enum registered: cosmos.gov.v1beta1.VoteOption
     
     goroutine 1 [running]:
     github.com/gogo/protobuf/proto.RegisterEnum(...)
     	/home/tom/go/pkg/mod/github.com/regen-network/[email protected]/proto/properties.go:520
     github.com/cosmos/cosmos-sdk/x/gov/types.init.3()
     	/home/tom/go/pkg/mod/github.com/cosmos/[email protected]/x/gov/types/gov.pb.go:497 +0x3d4
     FAIL	github.com/atomone-hub/govgen/v1/x/gov	0.015s
     FAIL
    
    Since both x/gov/types and cosmos-sdk/x/gov/types are imported, the proto registration happens twice for the same type name. We might want to change the type name/path, but if we do, won't we have a problem with clients using the legacy type?

@tbruyelle tbruyelle changed the title cp cosmos-sdk/x/gov x/gov Fork x/gov Feb 9, 2024
@tbruyelle tbruyelle changed the base branch from main to feat/custom-gov-module February 9, 2024 14:33
x/gov/abci.go Dismissed Show dismissed Hide dismissed
x/gov/abci.go Dismissed Show dismissed Hide dismissed
x/gov/module.go Dismissed Show dismissed Hide dismissed
x/gov/keeper/tally.go Dismissed Show dismissed Hide dismissed
x/gov/keeper/keeper.go Dismissed Show dismissed Hide dismissed
x/gov/keeper/keeper.go Dismissed Show dismissed Hide dismissed
x/gov/keeper/proposal.go Dismissed Show dismissed Hide dismissed
x/gov/types/keys.go Fixed Show fixed Hide fixed
@tbruyelle tbruyelle marked this pull request as ready for review February 9, 2024 16:00
Copy link
Collaborator

@giunatale giunatale left a comment

Choose a reason for hiding this comment

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

I've removed the need for depending on simapp in x/gov for testing. Switched over using the govgen app.

merging this code so we can go ahead with the two parallel feat planned.
We'll fix and or change this module if needed when we do the next steps

@giunatale giunatale merged commit b5f3fec into feat/custom-gov-module Feb 9, 2024
7 of 10 checks passed
@giunatale giunatale deleted the tbruyelle/chore/import-sdk-gov branch February 9, 2024 19:53
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.

2 participants