From 1459170a37290b34ebb292142d4da6a8a082b8a2 Mon Sep 17 00:00:00 2001 From: Ruaridh Date: Fri, 26 May 2023 19:06:26 -0700 Subject: [PATCH] fix(evmutil): register MsgConvertCosmosCoinToERC20 on amino (#1599) * add amino registration test * register evmutil types on app amino codec * register new msg type in amino and proto, also use new register method that checks length to catch ledger issues * clarify variable naming --- app/app_test.go | 64 ++++++++++++++++++++++++++++++++++++++++ x/evmutil/module.go | 4 ++- x/evmutil/types/codec.go | 8 +++-- 3 files changed, 73 insertions(+), 3 deletions(-) diff --git a/app/app_test.go b/app/app_test.go index 2733551f..f5e447d0 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -2,11 +2,17 @@ package app import ( "encoding/json" + "fmt" "os" "sort" "testing" "time" + "github.com/cosmos/cosmos-sdk/codec" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx" + vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" + evmtypes "github.com/evmos/ethermint/x/evm/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" abci "github.com/tendermint/tendermint/abci/types" @@ -63,6 +69,64 @@ func TestExport(t *testing.T) { assert.Len(t, exportedApp.Validators, 1) // no validators set in default genesis } +// TestLegacyMsgAreAminoRegistered checks if all known msg types are registered on the app's amino codec. +// It doesn't check if they are registered on the module codecs used for signature checking. +func TestLegacyMsgAreAminoRegistered(t *testing.T) { + tApp := NewTestApp() + + lcdc := tApp.LegacyAmino() + + // Use the proto codec as the canonical list of msg types. + protoCodec := tApp.AppCodec().(*codec.ProtoCodec) + protoRegisteredMsgs := protoCodec.InterfaceRegistry().ListImplementations(sdk.MsgInterfaceProtoName) + + for i, msgName := range protoRegisteredMsgs { + // Skip msgs from dependencies that were never amino registered. + if msgName == sdk.MsgTypeURL(&evmtypes.MsgEthereumTx{}) || + msgName == sdk.MsgTypeURL(&vestingtypes.MsgCreatePeriodicVestingAccount{}) { + continue + } + + // Create an encoded json msg, then unmarshal it to instantiate the msg type. + jsonMsg := []byte(fmt.Sprintf(`{"@type": "%s"}`, msgName)) + + var msg sdk.Msg + err := protoCodec.UnmarshalInterfaceJSON(jsonMsg, &msg) + require.NoError(t, err) + + // Only check legacy msgs for amino registration. + // Only legacy msg can be signed with amino. + _, ok := msg.(legacytx.LegacyMsg) + if !ok { + continue + } + + // Check the msg is registered in amino by checking a repeat registration call panics. + panicValue, ok := catchPanic(func() { + lcdc.RegisterConcrete(interface{}(msg), fmt.Sprintf("aUniqueRegistrationName%d", i), nil) + }) + assert.True(t, ok, "registration did not panic, msg %s is not registered in amino", msgName) + if ok { + require.IsTypef(t, "", panicValue, "msg %s amino registration panicked with unexpected type", msgName) + aminoErrMsgPrefix := "TypeInfo already exists for" + require.Containsf(t, panicValue, aminoErrMsgPrefix, "msg %s amino registration panicked for unexpected reason", msgName) + } + } +} + +// catchPanic returns the panic value of the passed function. The second return indicates if the function panicked. +func catchPanic(f func()) (panicValue interface{}, didPanic bool) { + didPanic = true + + defer func() { + panicValue = recover() + }() + + f() + didPanic = false + return +} + // unmarshalJSONKeys extracts keys from the top level of a json blob. func unmarshalJSONKeys(jsonBytes []byte) ([]string, error) { var jsonMap map[string]json.RawMessage diff --git a/x/evmutil/module.go b/x/evmutil/module.go index 709795bb..15455c70 100644 --- a/x/evmutil/module.go +++ b/x/evmutil/module.go @@ -45,7 +45,9 @@ func (AppModuleBasic) Name() string { } // Registers legacy amino codec -func (AppModuleBasic) RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) {} +func (AppModuleBasic) RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) { + types.RegisterLegacyAminoCodec(cdc) +} // RegisterInterfaces registers the module's interface types func (a AppModuleBasic) RegisterInterfaces(reg cdctypes.InterfaceRegistry) { diff --git a/x/evmutil/types/codec.go b/x/evmutil/types/codec.go index 0e09ca51..8ee40e13 100644 --- a/x/evmutil/types/codec.go +++ b/x/evmutil/types/codec.go @@ -2,6 +2,7 @@ package types import ( "github.com/cosmos/cosmos-sdk/codec" + "github.com/cosmos/cosmos-sdk/codec/legacy" cdctypes "github.com/cosmos/cosmos-sdk/codec/types" cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" sdk "github.com/cosmos/cosmos-sdk/types" @@ -12,14 +13,17 @@ import ( // RegisterLegacyAminoCodec registers the necessary evmutil interfaces and concrete types // on the provided LegacyAmino codec. These types are used for Amino JSON serialization. func RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) { - cdc.RegisterConcrete(&MsgConvertCoinToERC20{}, "evmutil/MsgConvertCoinToERC20", nil) - cdc.RegisterConcrete(&MsgConvertERC20ToCoin{}, "evmutil/MsgConvertERC20ToCoin", nil) + legacy.RegisterAminoMsg(cdc, &MsgConvertCoinToERC20{}, "evmutil/MsgConvertCoinToERC20") + legacy.RegisterAminoMsg(cdc, &MsgConvertERC20ToCoin{}, "evmutil/MsgConvertERC20ToCoin") + legacy.RegisterAminoMsg(cdc, &MsgConvertCosmosCoinToERC20{}, "evmutil/MsgConvertCosmosCoinToERC20") + } func RegisterInterfaces(registry cdctypes.InterfaceRegistry) { registry.RegisterImplementations((*sdk.Msg)(nil), &MsgConvertCoinToERC20{}, &MsgConvertERC20ToCoin{}, + &MsgConvertCosmosCoinToERC20{}, ) msgservice.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc)