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

Add Cosmos Group module support #1138

Open
fmorency opened this issue May 21, 2024 · 9 comments
Open

Add Cosmos Group module support #1138

fmorency opened this issue May 21, 2024 · 9 comments

Comments

@fmorency
Copy link
Contributor

The Group module is unsupported in chain/cosmos/.

  • Using ModifyGenesis to serialize a GroupPolicy currently fails, e.g.,
...
cosmos.NewGenesisKV("app_state.group.group_policies", []*group.GroupPolicyInfo{groupPolicy})
...

results in

failed to marshal genesis bytes to json: json: error calling MarshalJSON for type *types.Any: JSON marshal marshaling error for &Any{TypeUrl:/cosmos.group.v1.ThresholdDecisionPolicy,Value:[10 1 49 18 6 10 2 8 1 18 0],XXX_unrecognized:[]}, this is likely because amino is being used directly (instead of codec.LegacyAmino which is preferred) or UnpackInterfacesMessage is not defined for some type which contains a protobuf Any either directly or via one of its members
  • SubmitProposal is hardcoded to use the gov module
@Reecepbcups
Copy link
Member

Reecepbcups commented May 22, 2024

func DefaultEncoding() does not use group.AppModuleBasic{} as a register. tbh it should as its a core module, I don't see a downside

As a temp override you can add groups support with ChainConfig.EncodingConfig: extraEncoding()

Where encoding config is something like:

func extraEncoding() *testutil.TestEncodingConfig {
	cfg := cosmos.DefaultEncoding()
	grouptypes.RegisterInterfaces(cfg.InterfaceRegistry)
	return &cfg
}

@fmorency
Copy link
Contributor Author

Thanks @Reecepbcups. This is exactly what I did in liftedinit/manifest-ledger#62. However, I have no idea why but I need to add

// TODO: I have absolutely no idea why but the following block is needed in order for the GroupPolicy to get properly serialized in the ModifyGenesis function
// I don't know why the cdc.MarshalJSON is required but it is...
enc := AppEncoding()
group.RegisterInterfaces(enc.InterfaceRegistry)
cdc := codec.NewProtoCodec(enc.InterfaceRegistry)
_, err = cdc.MarshalJSON(groupPolicy)
require.NoError(t, err)

before trying to serialize the groupPolicy, otherwise I get

failed to start chains: failed to start chain group-poa: failed to marshal genesis bytes to json: json: error calling MarshalJSON for type *types.Any: JSON marshal marshaling error for &Any{TypeUrl:/cosmos.group.v1.ThresholdDecisionPolicy,Value:[10 1 49 18 6 10 2 8 10 18 0],XXX_unrecognized:[]}, this is likely because amino is being used directly (instead of codec.LegacyAmino which is preferred) or UnpackInterfacesMessage is not defined for some type which contains a protobuf Any either directly or via one of its members. To see a stacktrace of where the error is coming from, set the var Debug = true in codec/types/compat.go

Same applies to any group-related proposal trying to get serialized. I wrote a small helper function

// marshalProposal is a hackish way to ensure the prop is properly serialized
// TODO: I have absolutely no idea why but the following block is needed in order for the prop to get properly serialized
// I don't know why the cdc.MarshalJSON is required but it is...
func marshalProposal(t *testing.T, prop *group.MsgSubmitProposal) {
	enc := AppEncoding()
	group.RegisterInterfaces(enc.InterfaceRegistry)
	cdc := codec.NewProtoCodec(enc.InterfaceRegistry)
	_, err := cdc.MarshalJSON(prop)
	require.NoError(t, err)
}

I need to call before attempting to submit/vote/exec the proposal.

Any idea what's going on?

@Reecepbcups
Copy link
Member

Reecepbcups commented May 22, 2024

yea you have to use the app registry (which has all the values registered) to then Marshal/unmarshal the custom data types.

You can use chain.GetCodec() to return the already compiled codec FYI. (As long as group.RegisterInterfaces is setup in ChainConfig.EncodingConfig: extraEncoding() ) since we setup in NewCosmosChain

Then you can just chain.GetCodec().MarshalJSON(prop)

@fmorency
Copy link
Contributor Author

In my case, I added

grouptypes.RegisterInterfaces(enc.InterfaceRegistry)

in the AppEncoding() function

func AppEncoding() *sdktestutil.TestEncodingConfig {
	enc := cosmos.DefaultEncoding()

	manifesttypes.RegisterInterfaces(enc.InterfaceRegistry)
	grouptypes.RegisterInterfaces(enc.InterfaceRegistry)
	tokenfactorytypes.RegisterInterfaces(enc.InterfaceRegistry)
	poatypes.RegisterInterfaces(enc.InterfaceRegistry)

	return &enc
}

which is passed to

	LocalChainConfig = ibc.ChainConfig{
		Type:    "cosmos",
		...
		EncodingConfig: AppEncoding(),
		...
	}

However, it doesn't seem enough to get the following working

	groupGenesis := DefaultGenesis
	// Define the new Group and Group Policy to be used as the POA Admin
	groupGenesis = append(groupGenesis, cosmos.NewGenesisKV("app_state.group.group_seq", "1"))
	groupGenesis = append(groupGenesis, cosmos.NewGenesisKV("app_state.group.groups", []group.GroupInfo{groupInfo}))
	groupGenesis = append(groupGenesis, cosmos.NewGenesisKV("app_state.group.group_members", []group.GroupMember{groupMember1, groupMember2}))
	groupGenesis = append(groupGenesis, cosmos.NewGenesisKV("app_state.group.group_policy_seq", "1"))
	groupGenesis = append(groupGenesis, cosmos.NewGenesisKV("app_state.group.group_policies", []*group.GroupPolicyInfo{groupPolicy}))
...
	cfgA := LocalChainConfig
	cfgA.ModifyGenesis = cosmos.ModifyGenesis(groupGenesis)

The serialization of the Group Policy in ModifyGenesis fails with

failed to start chains: failed to start chain group-poa: failed to marshal genesis bytes to json: json: error calling MarshalJSON for type *types.Any: JSON marshal marshaling error for &Any{TypeUrl:/cosmos.group.v1.ThresholdDecisionPolicy,Value:[10 1 49 18 6 10 2 8 10 18 0],XXX_unrecognized:[]}, this is likely because amino is being used directly (instead of codec.LegacyAmino which is preferred) or UnpackInterfacesMessage is not defined for some type which contains a protobuf Any either directly or via one of its members. To see a stacktrace of where the error is coming from, set the var Debug = true in codec/types/compat.go

unless I add the call in my previous comment. I'm not sure I understand what's going on.

Is it because ModifyGenesis uses json.Marshal() instead of chain.GetCodec().MarshalJSON()?

@Reecepbcups
Copy link
Member

Yes, this is a bug on our end. It should use chain.GetCodec().MarshalJSON() instead. Great find! I think groups is the first which requires codec serialization.

Do you want to take this on? (ensure interface registry is setup on ICT before ModifyGenesis, then use chain.GetCodec().MarshalJSON()`. Ideally it should just be a few line changes around)

@fmorency
Copy link
Contributor Author

Yes, this is a bug on our end. It should use chain.GetCodec().MarshalJSON() instead. Great find! I think groups is the first which requires codec serialization.

Do you want to take this on? (ensure interface registry is setup on ICT before ModifyGenesis, then use chain.GetCodec().MarshalJSON()`. Ideally it should just be a few line changes around)

Sure thing, I can contribute a fix :)

@fmorency
Copy link
Contributor Author

Yes, this is a bug on our end. It should use chain.GetCodec().MarshalJSON() instead. Great find! I think groups is the first which requires codec serialization.

Do you want to take this on? (ensure interface registry is setup on ICT before ModifyGenesis, then use chain.GetCodec().MarshalJSON()`. Ideally it should just be a few line changes around)

I started looking into it and believe we have a chicken before the egg issue.

ModifyGenesis needs access to chain to use chain.GetCodec() but the chain is not yet created.

@Reecepbcups
Copy link
Member

hmm fun, Ill have to mess with this. I may need to have a GetCodec() with pre defined options before without using the chain then.

@Reecepbcups
Copy link
Member

also ref: liftedinit/manifest-ledger#62 (comment)

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

No branches or pull requests

2 participants