ValidateGenesis before InitGenesis without double unmarshalling

[julienrbrt]

Currently, as @akhilkumarpilli noticed, ValidateGenesis is not called before the InitGenesis.
It is only called at gentx:

cosmos-sdk/x/genutil/client/cli/gentx.go

Lines 90 to 97 in 7d5ab19

	var genesisState map[string]json.RawMessage
	if err = json.Unmarshal(appGenesis.AppState, &genesisState); err != nil {
	return errors.Wrap(err, "failed to unmarshal genesis state")
	}
	if err = genMM.ValidateGenesis(genesisState); err != nil {
	return errors.Wrap(err, "failed to validate genesis state")
	}

Meaning, if a chain would change its genesis in an invalid way, after gentxs and before collect-txs, the chain will start without any issue.

Option 1: Call genesis validation logic inside init genesis
This option is great as it prevents the double unmarshalling, however, then you end-up needing to define your validation logic twice.
The SDK has avoided double validation in the past by deprecating ValidateBasic for messages, this feels like a step backward if we recommend double validation somewhere else.

Option 2: Call validate genesis before init genesis in module manager
This option is best for UX as users will not need any change in their module code. The sdk would call the validation function before calling init genesis. The issue is that it leads to double unmarshalling with the current core (v1 beta) apis:

• cosmos-sdk/x/gov/module.go

  Lines 181 to 187 in 7d5ab19

  	func (am AppModule) InitGenesis(ctx context.Context, data json.RawMessage) error {
  	var genesisState v1.GenesisState
  	if err := am.cdc.UnmarshalJSON(data, &genesisState); err != nil {
  	return err
  	}
  	return InitGenesis(ctx, am.accountKeeper, am.bankKeeper, am.keeper, &genesisState)
  	}

• cosmos-sdk/x/gov/module.go

  Lines 169 to 177 in 7d5ab19

  	// ValidateGenesis performs genesis state validation for the gov module.
  	func (am AppModule) ValidateGenesis(bz json.RawMessage) error {
  	var data v1.GenesisState
  	if err := am.cdc.UnmarshalJSON(bz, &data); err != nil {
  	return fmt.Errorf("failed to unmarshal %s genesis state: %w", govtypes.ModuleName, err)
  	}
  	return v1.ValidateGenesis(am.accountKeeper.AddressCodec(), &data)

  
  This is not acceptable for hard-forked chain, and wastes compute.

Option 3: Add UnmarshalGenesis to core API and call validate genesis and init genesis in module manager
There is a long-standing proposal of adding an extra step in the genesis apis, namely UnmarshalGenesis.
This would perfectly solve the issue of Option 2, and Option 3 and Option 2 can then solve the problem.
The issue that we are late in the release cycle for core v1 / v0.52 (but still before rc, so still possible).

Option 4: Keep the status quo, and address this issue in the future core AutoGenesis.
This would be as well an improvement over the previous auto genesis that had no validate genesis function.

Personally, I think it is best to do that in the module manager so that no change is required for users.
However, we may be too late for changing anything yet. I do think it is something we should solve as we cannot expect users no to forget to call <appd> genesis validate even if we documented it perfectly.