interchaintest icon indicating copy to clipboard operation
interchaintest copied to clipboard

Add Cosmos Group module support

Open fmorency opened this issue 1 year ago • 9 comments

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

fmorency avatar May 21 '24 13:05 fmorency

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
}

reecepbcups avatar May 22 '24 14:05 reecepbcups

Thanks @Reecepbcups. This is exactly what I did in https://github.com/liftedinit/manifest-ledger/pull/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?

fmorency avatar May 22 '24 14:05 fmorency

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)

reecepbcups avatar May 22 '24 14:05 reecepbcups

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()?

fmorency avatar May 22 '24 14:05 fmorency

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)

reecepbcups avatar May 22 '24 14:05 reecepbcups

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 avatar May 22 '24 14:05 fmorency

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.

fmorency avatar May 28 '24 15:05 fmorency

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 avatar May 28 '24 17:05 reecepbcups

also ref: https://github.com/liftedinit/manifest-ledger/pull/62#discussion_r1617644379

reecepbcups avatar May 28 '24 17:05 reecepbcups