Refactore simapp.MakeEncodingConfig

[robert-zaremba]

Summary

We have two functions to create a global encoding config:

• simapp.MakeEncodingConfig - it will use the latter one and register interfaces for a codec in all modules
• simappparams.MakeEncodingConfig - it will only create an internal codec based on build flags (amino or protobuf).

Problem Definition

simapp.MakeEncodingConfig souldn't be exposed - it's an implementation detail for creating an app. App codecs should be used. Using MakeEncodingConfig is source of some problems we had recently with codecs and tests.

Proposal

don't expose simapp.MakeEncodingConfig

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[colin-axner]

cc: @jackzampolin the relayer will need to update this, I saw usage of MakeEncodingConfig in chains.go

Edit: nvm I see the WIP pr that will change this to use the simappparams

[aaronc]

MakeEncodingConfig is part of simapp used for tests. It would have been a huge refactoring of lots of tests to not do something like this. If apps are depending on simapp that's a problem in and of itself. It's just for testing.

We can do something better but I don't consider this a necessary stargate backport.

Also, I imagine the complexity of this will be large. We have higher priority backports to deal with.

[robert-zaremba]

We can do something better but I don't consider this a necessary stargate backport.

I wanted to rename it, to make it clear that users should not use it and rather use the app directly. But there was objection.

The easiest way to do this task is:

• make this function private
• use app codecs instead.

[clevinson]

I agree that this also seems like quite a large refactor. The most important things for v0.40.0 (Stargate backports) are:

• UX fixes & bug fixes of issues in the current release candidate
• documentation & testing of existing features
• high priority features that we missed that are state machine breaking (and therefore would require a chain upgrade)

This issue is laregely about devUX, which is important, but not critical IMO for a backport. Let's please hold off on work for it for now until we have Stargate out the door and published.

[aaronc]

• use app codecs instead.

There's a reason why that wasn't done initially. Basically because it wasn't how tests were structured before and it will be a lot of work. To me it's clearly out of scope for a backport. Users shouldn't be depending on anything in simapp and if they do that's sort of at their own risk.

[robert-zaremba]

@aaronc - can we at least change a method name? Originally I was proposing to add a Test suffix, but there was lack of consensus. We should somehow make it clear for users to not repeat our mistakes and not use this function in tests.
discord [chat](https://discordapp.com/channels/684494798358315010/758982012861677618/768906239198363668 reference.

[robert-zaremba]

Related issue: #7310

[robert-zaremba]

another one:
Limit the creation of uncessary EncodingConfigs #7631

[aaronc]

@aaronc - can we at least change a method name?

How about MakeTestEncodingConfig?

[aaronc]

EncodingConfig itself should never be used outside SDK tests. Maybe it should be called TestEncodingConfig itself.

[robert-zaremba]

How about MakeTestEncodingConfig?

I'm OK with that. In my previous PR (Any for validator key) @amaurymartiny and @blushi didn't like it.

[robert-zaremba]

Here: #7597 (comment)