x/authz and x/feegrant don't support ledger signing

[aaronc]

We omitted supporting amino for x/authz and x/feegrant. Given that sign mode textual we'll take a bit of time, I think we should consider adding amino definitions in 0.46 so users can use these with ledger keys.

[amaury1093]

So we actually already have Amino support for authz/feegrant:

cosmos-sdk/x/authz/msgs.go

Lines 20 to 23 in e7066c4

	// For amino support.
	_ legacytx.LegacyMsg = &MsgGrant{}
	_ legacytx.LegacyMsg = &MsgRevoke{}
	_ legacytx.LegacyMsg = &MsgExec{}

cosmos-sdk/x/feegrant/msgs.go

Line 15 in e7066c4

_, _ legacytx.LegacyMsg = &MsgGrantAllowance{}, &MsgRevokeAllowance{} // For amino support.

and some people tested with ledger.

We don't have the RegisterLegacyAminoCodec function for both of them. For Amino signing (i.e. encoding), we probably don't need it, as Zaki pointed out in a slack thread. We maybe need RegisterLegacyAminoCodec for amino decoding, e.g. in the old REST api, but since that it gone maybe we can actually remove all RegisterLegacyAminoCodec (maybe).

[tac0turtle]

ledger signing is supported via cli. Its unfortunate but not sure its possible to do it in grpc?

[aaronc]

So we actually already have Amino support for authz/feegrant:

cosmos-sdk/x/authz/msgs.go

Lines 20 to 23 in e7066c4

	// For amino support.
	_ legacytx.LegacyMsg = &MsgGrant{}
	_ legacytx.LegacyMsg = &MsgRevoke{}
	_ legacytx.LegacyMsg = &MsgExec{}

cosmos-sdk/x/feegrant/msgs.go

Line 15 in e7066c4

_, _ legacytx.LegacyMsg = &MsgGrantAllowance{}, &MsgRevokeAllowance{} // For amino support.

and some people tested with ledger.

We don't have the RegisterLegacyAminoCodec function for both of them. For Amino signing (i.e. encoding), we probably don't need it, as Zaki pointed out in a slack thread. We maybe need RegisterLegacyAminoCodec for amino decoding, e.g. in the old REST api, but since that it gone maybe we can actually remove all RegisterLegacyAminoCodec (maybe).

Where does the amino name come from if we don't do RegisterConcrete?

[amaury1093]

Where does the amino name come from if we don't do RegisterConcrete?

Turns out Amino just skips the name, i.e. skips the type field, and just outputs the value. See cosmos/cosmjs#1026 (comment) for some digging done by @RiccardoM

I created a PR to fix this: #11214

[aaronc]

I guess that's both good and scary 😱