Pubkey registration issue

[fedekunze]

#183 bumped the SDK version to 0.38.1 but since simulation logic was broken I had to switch to a specific commit from master. Because of this, protobuf changes also affected Ethermint and I'm now getting an annoying panic because the Ethereum Secp256k1 key is not supported by the codec:

test panicked: unmarshal to crypto.PubKey failed after 4 bytes (unrecognized prefix bytes F1658E32): F1658E324104F747CA134FFB99DE26E581A2DB33BF60DD64E43CC20FDE0594F5BE621980F4BEDC5A949E1EF2F68477428BE17CBE0700FCAD3420240E2B2FF0C14AC6109F7BB5
            goroutine 868 [running]:
            runtime/debug.Stack(0xc000544bf0, 0x4e864a0, 0xc0001af4c0)
            	/usr/local/Cellar/go/1.14/libexec/src/runtime/debug/stack.go:24 +0x9d
            github.com/stretchr/testify/suite.failOnPanic(0xc00033e120)
            	/Users/federico/go/pkg/mod/github.com/stretchr/testify@v1.5.1/suite/suite.go:63 +0x57
            panic(0x4e864a0, 0xc0001af4c0)
            	/usr/local/Cellar/go/1.14/libexec/src/runtime/panic.go:967 +0x15d
            github.com/cosmos/cosmos-sdk/x/auth/ante.SetUpContextDecorator.AnteHandle.func1(0xe48ad58, 0xc00052a300, 0xc000550d48, 0xc000550fd8)
            	/Users/federico/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.34.4-0.20200320171414-1d0967c32a76/x/auth/ante/setup.go:59 +0x25b
            panic(0x4e864a0, 0xc0001af4c0)
            	/usr/local/Cellar/go/1.14/libexec/src/runtime/panic.go:967 +0x15d
            github.com/tendermint/go-amino.(*Codec).MustUnmarshalBinaryBare(...)
            	/Users/federico/go/pkg/mod/github.com/tendermint/go-amino@v0.15.1/amino.go:358
            github.com/cosmos/cosmos-sdk/x/auth/types.StdSignature.GetPubKey(0xc0001a27d0, 0x46, 0x50, 0xc0001a2730, 0x41, 0x41, 0x4f91a80, 0xc000539800)
            	/Users/federico/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.34.4-0.20200320171414-1d0967c32a76/x/auth/types/stdtx.go:77 +0xe9
            github.com/cosmos/cosmos-sdk/x/auth/types.StdTx.GetPubKeys(0xc0001aebe0, 0x1, 0x1, 0xc0011f6520, 0x1, 0x1, 0x35b60, 0xc00036bf80, 0x2, 0x2, ...)
            	/Users/federico/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.34.4-0.20200320171414-1d0967c32a76/x/auth/types/stdtx.go:245 +0xda
            github.com/cosmos/cosmos-sdk/x/auth/ante.SetPubKeyDecorator.AnteHandle(0x5416620, 0xc00016c800, 0x5444b00, 0xc00020b300, 0x54415c0, 0xc00020b300, 0x5416620, 0xc00016c8c0, 0x5416660, 0xc00016c920, ...)
            	/Users/federico/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.34.4-0.20200320171414-1d0967c32a76/x/auth/ante/sigverify.go:67 +0xad
            github.com/cosmos/cosmos-sdk/types.ChainAnteDecorators.func1(0x54296a0, 0xc000210008, 0x543f140, 0xc00037c080, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
            	/Users/federico/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.34.4-0.20200320171414-1d0967c32a76/types/handler.go:40 +0x124
            github.com/cosmos/cosmos-sdk/x/auth/ante.ConsumeTxSizeGasDecorator.AnteHandle(0x5416620, 0xc00016c800, 0x5444b00, 0xc00020b300, 0x54415c0, 0xc00020b300, 0x5416620, 0xc00016c8c0, 0x5416660, 0xc00016c920, ...)
            	/Users/federico/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.34.4-0.20200320171414-1d0967c32a76/x/auth/ante/basic.go:145 +0x369
            github.com/cosmos/cosmos-sdk/types.ChainAnteDecorators.func1(0x54296a0, 0xc000210008, 0x543f140, 0xc00037c080, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
            	/Users/federico/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.34.4-0.20200320171414-1d0967c32a76/types/handler.go:40 +0x124
            github.com/cosmos/cosmos-sdk/x/auth/ante.ValidateMemoDecorator.AnteHandle(0x5416620, 0xc00016c800, 0x5444b00, 0xc00020b300, 0x54415c0, 0xc00020b300, 0x5416620, 0xc00016c8c0, 0x5416660, 0xc00016c920, ...)
            	/Users/federico/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.34.4-0.20200320171414-1d0967c32a76/x/auth/ante/basic.go:76 +0x351
            github.com/cosmos/cosmos-sdk/types.ChainAnteDecorators.func1(0x54296a0, 0xc000210008, 0x543f140, 0xc00037c080, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
            	/Users/federico/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.34.4-0.20200320171414-1d0967c32a76/types/handler.go:40 +0x124
            github.com/cosmos/cosmos-sdk/x/auth/ante.ValidateBasicDecorator.AnteHandle(0x54296a0, 0xc000210008, 0x543f140, 0xc00037c080, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
            	/Users/federico/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.34.4-0.20200320171414-1d0967c32a76/x/auth/ante/basic.go:38 +0x14b
            github.com/cosmos/cosmos-sdk/types.ChainAnteDecorators.func1(0x54296a0, 0xc000210008, 0x543f140, 0xc00037c080, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
            	/Users/federico/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.34.4-0.20200320171414-1d0967c32a76/types/handler.go:40 +0x124
            github.com/cosmos/cosmos-sdk/x/auth/ante.MempoolFeeDecorator.AnteHandle(0x54296a0, 0xc000210008, 0x543f140, 0xc00037c080, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
            	/Users/federico/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.34.4-0.20200320171414-1d0967c32a76/x/auth/ante/fee.go:68 +0x186
            github.com/cosmos/cosmos-sdk/types.ChainAnteDecorators.func1(0x54296a0, 0xc000210008, 0x543f140, 0xc00037c080, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
            	/Users/federico/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.34.4-0.20200320171414-1d0967c32a76/types/handler.go:40 +0x124
            github.com/cosmos/cosmos-sdk/x/auth/ante.SetUpContextDecorator.AnteHandle(0x54296a0, 0xc000210008, 0x543f140, 0xc00037c080, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
            	/Users/federico/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.34.4-0.20200320171414-1d0967c32a76/x/auth/ante/setup.go:64 +0x493
            github.com/cosmos/cosmos-sdk/types.ChainAnteDecorators.func1(0x54296a0, 0xc000210008, 0x543f140, 0xc00037c080, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
            	/Users/federico/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.34.4-0.20200320171414-1d0967c32a76/types/handler.go:40 +0x124
            github.com/cosmos/ethermint/app/ante.NewAnteHandler.func1(0x54296a0, 0xc000210008, 0x543f140, 0xc00037c080, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
            	/Users/federico/ethermint/app/ante/ante.go:56 +0xcc0
            github.com/cosmos/ethermint/app/ante_test.requireValidTx(0xc00033e120, 0xc000568320, 0x54296a0, 0xc000210008, 0x543f140, 0xc00037c080, 0x0, 0x0, 0x0, 0x0, ...)
            	/Users/federico/ethermint/app/ante/ante_test.go:26 +0x8c
            github.com/cosmos/ethermint/app/ante_test.(*AnteTestSuite).TestValidTx(0xc00032c000)
            	/Users/federico/ethermint/app/ante/ante_test.go:91 +0xbb8

I traced the error and is due to the fact that StdSignature on the SDK uses the amino codec to marshal the pubkey bytes. I then tried to register the key type to the amino codec using the following:

cryptoamino.RegisterKeyType(emintcrypto.EthPubKeySecp256k1{}, emintcrypto.PubKeyAminoName)
cryptoamino.RegisterKeyType(emintcrypto.EthPrivKeySecp256k1{}, emintcrypto.PrivKeyAminoName)

Nevertheless, this caused another issue with priorities:

test panicked: crypto.EthPubKeySecp256k1 conflicts with 2 other(s). Add it to the priority list for crypto.PubKey.
            goroutine 148 [running]:
            runtime/debug.Stack(0xc000ff38a8, 0x4e86280, 0xc000fddf30)
            	/usr/local/Cellar/go/1.14/libexec/src/runtime/debug/stack.go:24 +0x9d
            github.com/stretchr/testify/suite.failOnPanic(0xc000f3efc0)
            	/Users/federico/go/pkg/mod/github.com/stretchr/testify@v1.5.1/suite/suite.go:63 +0x57
            panic(0x4e86280, 0xc000fddf30)
            	/usr/local/Cellar/go/1.14/libexec/src/runtime/panic.go:967 +0x15d
            github.com/tendermint/go-amino.(*Codec).addCheckConflictsWithConcrete_nolock(0xc0001d2540, 0xc001018ff0)
            	/Users/federico/go/pkg/mod/github.com/tendermint/go-amino@v0.15.1/codec.go:671 +0x332
            github.com/tendermint/go-amino.(*Codec).RegisterConcrete.func1(0xc0001d2540, 0xc001018ff0)
            	/Users/federico/go/pkg/mod/github.com/tendermint/go-amino@v0.15.1/codec.go:238 +0x6c
            github.com/tendermint/go-amino.(*Codec).RegisterConcrete(0xc0001d2540, 0x4f01700, 0xc000f133e0, 0x5053b37, 0x19, 0x0)
            	/Users/federico/go/pkg/mod/github.com/tendermint/go-amino@v0.15.1/codec.go:240 +0x123
            github.com/tendermint/tendermint/crypto/encoding/amino.RegisterKeyType(0x4f01700, 0xc000f133e0, 0x5053b37, 0x19)
            	/Users/federico/go/pkg/mod/github.com/tendermint/tendermint@v0.33.2/crypto/encoding/amino/amino.go:71 +0x62
            github.com/cosmos/ethermint/codec.MakeCodec(0xc000481b30, 0x0)
            	/Users/federico/ethermint/codec/codec.go:87 +0x129
            github.com/cosmos/ethermint/app.NewEthermintApp(0x542a100, 0x5e97df8, 0x5441620, 0xc000f12380, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, ...)
            	/Users/federico/ethermint/app/ethermint.go:135 +0x58

The SDK doesn't allow you tho set amino.ConcreteOptions to define priorities. I'm still wondering what's the best way to proceed from here

[fedekunze]

ideas I have on top of my mind:

1. add support to include the concrete options on the SDK codec. Not sure if this is counterintuitive since amino is being deprecated.
2. use application-specific hybrid codec instead of the Codec defined on cosmos/cosmos-sdk/codec/amino.go.
3. not use amino at all and switch to proto. Will likely depend on SDK work and we'll have to provide support with that and ensure things don't break unexpectedly.

@alexanderbez if you have some ideas or thoughts on how to fix this and what's the best way to proceed from the SDK

[fedekunze]

opened a PR on SDK with a patch: cosmos/cosmos-sdk#5892