refactor!: Accept both genesis as json.RawMessage and proto.Message

[facundomedica]

Description

In this PR we go for an extension interface approach. Would like to get some feedback (especially in names, I suck at naming stuff) :)

Previous PR: https://github.com/cosmos/cosmos-sdk/pull/12700

Closes: https://github.com/cosmos/cosmos-sdk/issues/12642

---

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.

I have...

• [ ] included the correct type prefix in the PR title
• [ ] added ! to the type prefix if API or client breaking change
• [ ] targeted the correct branch (see PR Targeting)
• [ ] provided a link to the relevant issue or specification
• [ ] followed the guidelines for building modules
• [ ] included the necessary unit and integration tests
• [ ] added a changelog entry to CHANGELOG.md
• [ ] included comments for documenting Go code
• [ ] updated the relevant documentation or specification
• [ ] reviewed "Files changed" and left comments if necessary
• [ ] confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.

I have...

• [ ] confirmed the correct type prefix in the PR title
• [ ] confirmed ! in the type prefix if API or client breaking change
• [ ] confirmed all author checklist items have been addressed
• [ ] reviewed state machine logic
• [ ] reviewed API design and naming
• [ ] reviewed documentation is accurate
• [ ] reviewed tests and test coverage
• [ ] manually tested (if applicable)

[codecov[bot]]

Codecov Report

Merging #12828 (8e20bc2) into main (e2d6cbd) will increase coverage by 0.00%. The diff coverage is 55.81%.

@@           Coverage Diff           @@
##             main   #12828   +/-   ##
=======================================
  Coverage   56.25%   56.25%           
=======================================
  Files         648      644    -4     
  Lines       55031    54973   -58     
=======================================
- Hits        30956    30925   -31     
+ Misses      21581    21552   -29     
- Partials     2494     2496    +2     

Impacted Files	Coverage Δ
x/bank/module.go	63.80% <ø> (ø)
x/capability/module.go	64.38% <ø> (ø)
x/distribution/module.go	63.00% <ø> (ø)
x/evidence/module.go	45.94% <ø> (ø)
x/feegrant/module/module.go	14.63% <ø> (ø)
x/genutil/module.go	66.66% <ø> (ø)
x/group/module/module.go	50.61% <ø> (ø)
x/mint/module.go	64.89% <ø> (ø)
x/params/module.go	20.96% <ø> (ø)
x/slashing/module.go	65.16% <ø> (ø)
... and 12 more

[facundomedica]

proto.message can only be used if ORM is not used in the future, right?

Yes, @aaronc said that ( see https://github.com/cosmos/cosmos-sdk/pull/12700#issuecomment-1202520965)

[tac0turtle]

LGTM, I'm not really a fan of this. I still think we should only allow proto.message.

[aaronc]

I agree this isn't pretty but I would like to see some well reasoned rationale if we're going to only allow proto.Message which will break the ORM when it is almost ready to be marked as stable IMHO.

If we want to have a single interface an alternative is to just use interface{} and then let the manager deal with either proto.Message or json.RawMessage via reflection.

Or we can deal with the ORM's way of handling genesis with the core API but it seems like that is maybe postponed. So my preference would be to just not break things which already work for the time being.

[facundomedica]

@kocubinski

#12642 talks about simplifying the API, but this seems like a step backward to me w.r.t. maintainability, readability, especially considering the ORM requires json.RawMessage, so that API will never become legacy enough to be removed.

I agree with you, I was thinking pretty much that while finishing this PR. Adding this without having a clear path to deprecation of the json.RawMessage seems confusing; even worse if we are thinking of not deprecating json.RawMessage at all.

@aaronc can you link me to the relevant files? Would love to take a look (I don't think I'll be able to come up with a solution, but I'm curious)

LGTM, I'm not really a fan of this. I still think we should only allow proto.message.

@marbar3778 does returning an interface{} like Aaron suggested make sense to you? I mean, would that be an improvement at all in your opinion?

[aaronc]

This is the orm's json interface for a whole module: https://github.com/cosmos/cosmos-sdk/blob/main/orm/model/ormdb/module.go

It is based on this abstract json source/target interface: https://github.com/cosmos/cosmos-sdk/tree/main/orm/types/ormjson which is designed to work with the current raw JSON and with streaming files eventually when the SDK supports that. With the raw json approach, the orm basically stores all of the different table types it manages in a json map which can't directly be represented as a single proto.Message even though it contains data for several different types of proto.Message objects.

If a module uses the orm, there is very little genesis code to write compared to the current approach and I think that is beneficial.

[likhita-809]

related: #13085

[tac0turtle]

should we close this or continue because of orm?

[aaronc]

How about this approach https://github.com/cosmos/cosmos-sdk/pull/12972/files#diff-3b60c0d2bf81b2464b20c8051fea6caef3dba18fee1cea0c437cc3d969b9a210R333 ?

[facundomedica]

Let's close this one for now, as no one really likes this approach (even me). Aaron's approach sounds good to me, let's explore all the options before taking a decision :)