Full amino encoding support

[aaronc]

Summary

We have attempted to maintain a compatibility for amino on a few levels during the protobuf migration:

• the ability to use the AminoCodec for state encoding
• the ability to still use amino for transaction encoding by configuring the appropriate tx.Generator
• the ability to sign protobuf transactions using Amino JSON

We actually have very few tests that test that the above work properly and should.

Proposed Approaches

Simapp amino build flag

The simplest way to test a lot of this stuff with simapp might be to simply add an amino build flag which enables AminoCodec, etc. instead of protobuf and simply run the same unit tests in parallel

Test Matrix

Name	+build tags	Details	Which Tests
Full Proto	!test_amino, !test_amino_signing	simapp uses HybridMarshaler and proto tx (#6213), simcli uses proto JSON marshaler	Unit and integration tests + all sims
Proto, Amino Signing	!test_amino, test_amino_signing	Same as full proto but simcli uses amino JSON signing	Just integration tests
Full Amino	test_amino	simapp and simcli use AminoMarshaler and amino tx	Unit and integration tests

TODO

• Implement SIGN_MODE_DIRECT #6216 Full Proto & Full Amino integration tests
• unit tests
• Proto, Amino Signing integration tests

[alexanderbez]

Shouldn't Full Proto use ProtoMarshaler? Otherwise, it will still use Amino for anything JSON related. Also, maybe drop the simapp verbiage from the build tag values? (!)amino_signing and (!)amino_encoding?

[aaronc]

Shouldn't Full Proto use ProtoMarshaler?

Well, it's tricky because the JSON marshaling stuff on the server side AFAIK is only used for querier and REST endpoints which hopefully will get replaced with gRPC in the near future (which doesn't need to manually marshal anything at all). So I'd opt for using HybridMarshaler to allow for a more graceful client transition. As I recall that was the point of having a HybridMarshaler at all 😉

[aaronc]

Also, maybe drop the simapp verbiage from the build tag values? (!)amino_signing and (!)amino_encoding?

I do want to note that these build tags wouldn't affect any other code besides simapp and tests. With other code you would need to manually configure the marshalers. Thus the reason for the prefix. How about test_amino_...? This would affect JSON decoding for cli tests AFAIK also

[alexanderbez]

test_amino 👍

[clevinson]

Is it still desired to have full amino support? From last conversations, I understood the conesnsus was leaning towards dropping amino at a defined point in the future as opposed to fully supporting it as an alternate encoding.

[alexanderbez]

Last time I was involved in a conversation that included this topic, we decided that we'd punt Amino out of the SDK in 0.41. The reason being, the 0.40 (stargate) release gives folks adequate time to update their infra and services to use proto.

[tac0turtle]

closing as proto has been widely adopted, offering to go back brings a overhead we are not able to handle.