From 393af6c64517d13d1d77393be96b7a2e1f245167 Mon Sep 17 00:00:00 2001 From: Matt Kocubinski Date: Wed, 7 Jun 2023 09:54:38 -0500 Subject: [PATCH] refactor(x/auth): clean up signing init code for reusability (#16433) --- simapp/simd/cmd/root.go | 5 +- simapp/simd/cmd/root_v2.go | 6 +- x/auth/ante/sigverify_test.go | 8 +- x/auth/tx/config.go | 164 ++++++++++++++++++++++------------ x/auth/tx/config/config.go | 5 +- 5 files changed, 125 insertions(+), 63 deletions(-) diff --git a/simapp/simd/cmd/root.go b/simapp/simd/cmd/root.go index 149a36387e..2de3d5d5ec 100644 --- a/simapp/simd/cmd/root.go +++ b/simapp/simd/cmd/root.go @@ -90,10 +90,13 @@ func NewRootCmd() *cobra.Command { EnabledSignModes: enabledSignModes, TextualCoinMetadataQueryFn: txmodule.NewGRPCCoinMetadataQueryFn(initClientCtx), } - txConfigWithTextual := tx.NewTxConfigWithOptions( + txConfigWithTextual, err := tx.NewTxConfigWithOptions( codec.NewProtoCodec(encodingConfig.InterfaceRegistry), txConfigOpts, ) + if err != nil { + return err + } initClientCtx = initClientCtx.WithTxConfig(txConfigWithTextual) if err := client.SetCmdClientContextHandler(initClientCtx, cmd); err != nil { diff --git a/simapp/simd/cmd/root_v2.go b/simapp/simd/cmd/root_v2.go index e66ef3be03..3dd21f92b6 100644 --- a/simapp/simd/cmd/root_v2.go +++ b/simapp/simd/cmd/root_v2.go @@ -101,11 +101,13 @@ func NewRootCmd() *cobra.Command { EnabledSignModes: enabledSignModes, TextualCoinMetadataQueryFn: txmodule.NewGRPCCoinMetadataQueryFn(initClientCtx), } - txConfigWithTextual := tx.NewTxConfigWithOptions( + txConfigWithTextual, err := tx.NewTxConfigWithOptions( codec.NewProtoCodec(interfaceRegistry), txConfigOpts, ) - + if err != nil { + return err + } initClientCtx = initClientCtx.WithTxConfig(txConfigWithTextual) if err := client.SetCmdClientContextHandler(initClientCtx, cmd); err != nil { return err diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index 78dcc5802f..cd803f3fac 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/require" storetypes "cosmossdk.io/store/types" + authsign "github.com/cosmos/cosmos-sdk/x/auth/signing" "github.com/cosmos/cosmos-sdk/codec" @@ -136,10 +137,12 @@ func TestSigVerification(t *testing.T) { TextualCoinMetadataQueryFn: txmodule.NewGRPCCoinMetadataQueryFn(suite.clientCtx), EnabledSignModes: enabledSignModes, } - suite.clientCtx.TxConfig = authtx.NewTxConfigWithOptions( + var err error + suite.clientCtx.TxConfig, err = authtx.NewTxConfigWithOptions( codec.NewProtoCodec(suite.encCfg.InterfaceRegistry), txConfigOpts, ) + require.NoError(t, err) suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() // make block height non-zero to ensure account numbers part of signBytes @@ -171,10 +174,11 @@ func TestSigVerification(t *testing.T) { TextualCoinMetadataQueryFn: txmodule.NewBankKeeperCoinMetadataQueryFn(suite.txBankKeeper), EnabledSignModes: enabledSignModes, } - anteTxConfig := authtx.NewTxConfigWithOptions( + anteTxConfig, err := authtx.NewTxConfigWithOptions( codec.NewProtoCodec(suite.encCfg.InterfaceRegistry), txConfigOpts, ) + require.NoError(t, err) svd := ante.NewSigVerificationDecorator(suite.accountKeeper, anteTxConfig.SignModeHandler()) antehandler := sdk.ChainAnteDecorators(spkd, svd) defaultSignMode, err := authsign.APISignModeToInternal(anteTxConfig.SignModeHandler().DefaultMode()) diff --git a/x/auth/tx/config.go b/x/auth/tx/config.go index 2ce75d1845..1f0a05f48d 100644 --- a/x/auth/tx/config.go +++ b/x/auth/tx/config.go @@ -3,8 +3,6 @@ package tx import ( "fmt" - "google.golang.org/protobuf/reflect/protoregistry" - txsigning "cosmossdk.io/x/tx/signing" "cosmossdk.io/x/tx/signing/aminojson" "cosmossdk.io/x/tx/signing/direct" @@ -48,6 +46,14 @@ type ConfigOptions struct { TextualCoinMetadataQueryFn textual.CoinMetadataQueryFn // CustomSignModes are the custom sign modes that will be added to the txsigning.HandlerMap. CustomSignModes []txsigning.SignModeHandler + // ProtoDecoder is the decoder that will be used to decode protobuf transactions. + ProtoDecoder sdk.TxDecoder + // ProtoEncoder is the encoder that will be used to encode protobuf transactions. + ProtoEncoder sdk.TxEncoder + // JSONDecoder is the decoder that will be used to decode json transactions. + JSONDecoder sdk.TxDecoder + // JSONEncoder is the encoder that will be used to encode json transactions. + JSONEncoder sdk.TxEncoder } // DefaultSignModes are the default sign modes enabled for protobuf transactions. @@ -70,62 +76,54 @@ var DefaultSignModes = []signingtypes.SignMode{ func NewTxConfig(protoCodec codec.ProtoCodecMarshaler, enabledSignModes []signingtypes.SignMode, customSignModes ...txsigning.SignModeHandler, ) client.TxConfig { - return NewTxConfigWithOptions(protoCodec, ConfigOptions{ + txConfig, err := NewTxConfigWithOptions(protoCodec, ConfigOptions{ EnabledSignModes: enabledSignModes, CustomSignModes: customSignModes, }) + if err != nil { + panic(err) + } + return txConfig } -// NewTxConfigWithOptions returns a new protobuf TxConfig using the provided ProtoCodec, ConfigOptions and -// custom sign mode handlers. If ConfigOptions is an empty struct then default values will be used. -func NewTxConfigWithOptions(protoCodec codec.ProtoCodecMarshaler, configOptions ConfigOptions) client.TxConfig { - txConfig := &config{ - decoder: DefaultTxDecoder(protoCodec), - encoder: DefaultTxEncoder(), - jsonDecoder: DefaultJSONTxDecoder(protoCodec), - jsonEncoder: DefaultJSONTxEncoder(protoCodec), - protoCodec: protoCodec, - } +// NewDefaultSigningOptions returns the sdk default signing options used by x/tx. This includes account and +// validator address prefix enabled codecs. +func NewDefaultSigningOptions() (*txsigning.Options, error) { + sdkConfig := sdk.GetConfig() + return &txsigning.Options{ + AddressCodec: authcodec.NewBech32Codec(sdkConfig.GetBech32AccountAddrPrefix()), + ValidatorAddressCodec: authcodec.NewBech32Codec(sdkConfig.GetBech32ValidatorAddrPrefix()), + }, nil +} - opts := &configOptions - if opts.SigningHandler != nil { - txConfig.handler = opts.SigningHandler - return txConfig - } - - signingOpts := opts.SigningOptions - if signingOpts == nil { - signingOpts = &txsigning.Options{} - } - if signingOpts.TypeResolver == nil { - signingOpts.TypeResolver = protoregistry.GlobalTypes - } - if signingOpts.FileResolver == nil { - signingOpts.FileResolver = protoCodec.InterfaceRegistry() - } - if len(opts.EnabledSignModes) == 0 { - opts.EnabledSignModes = DefaultSignModes - } - - if opts.SigningContext == nil { - sdkConfig := sdk.GetConfig() - if signingOpts.AddressCodec == nil { - signingOpts.AddressCodec = authcodec.NewBech32Codec(sdkConfig.GetBech32AccountAddrPrefix()) - } - if signingOpts.ValidatorAddressCodec == nil { - signingOpts.ValidatorAddressCodec = authcodec.NewBech32Codec(sdkConfig.GetBech32ValidatorAddrPrefix()) - } - var err error - opts.SigningContext, err = txsigning.NewContext(*signingOpts) +// NewSigningHandlerMap returns a new txsigning.HandlerMap using the provided ConfigOptions. +// It is recommended to use types.InterfaceRegistry in the field ConfigOptions.FileResolver as shown in +// NewTxConfigWithOptions but this fn does not enforce it. +func NewSigningHandlerMap(configOptions ConfigOptions) (*txsigning.HandlerMap, error) { + var err error + configOpts := &configOptions + if configOpts.SigningOptions == nil { + configOpts.SigningOptions, err = NewDefaultSigningOptions() if err != nil { - panic(err) + return nil, err + } + } + if configOpts.SigningContext == nil { + configOpts.SigningContext, err = txsigning.NewContext(*configOpts.SigningOptions) + if err != nil { + return nil, err } } - txConfig.signingContext = opts.SigningContext - lenSignModes := len(configOptions.EnabledSignModes) - handlers := make([]txsigning.SignModeHandler, lenSignModes+len(opts.CustomSignModes)) - for i, m := range configOptions.EnabledSignModes { + signingOpts := configOpts.SigningOptions + + if len(configOpts.EnabledSignModes) == 0 { + configOpts.EnabledSignModes = DefaultSignModes + } + + lenSignModes := len(configOpts.EnabledSignModes) + handlers := make([]txsigning.SignModeHandler, lenSignModes+len(configOpts.CustomSignModes)) + for i, m := range configOpts.EnabledSignModes { var err error switch m { case signingtypes.SignMode_SIGN_MODE_DIRECT: @@ -133,10 +131,10 @@ func NewTxConfigWithOptions(protoCodec codec.ProtoCodecMarshaler, configOptions case signingtypes.SignMode_SIGN_MODE_DIRECT_AUX: handlers[i], err = directaux.NewSignModeHandler(directaux.SignModeHandlerOptions{ TypeResolver: signingOpts.TypeResolver, - SignersContext: opts.SigningContext, + SignersContext: configOpts.SigningContext, }) if err != nil { - panic(err) + return nil, err } case signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON: handlers[i] = aminojson.NewSignModeHandler(aminojson.SignModeHandlerOptions{ @@ -145,24 +143,76 @@ func NewTxConfigWithOptions(protoCodec codec.ProtoCodecMarshaler, configOptions }) case signingtypes.SignMode_SIGN_MODE_TEXTUAL: handlers[i], err = textual.NewSignModeHandler(textual.SignModeOptions{ - CoinMetadataQuerier: opts.TextualCoinMetadataQueryFn, + CoinMetadataQuerier: configOpts.TextualCoinMetadataQueryFn, FileResolver: signingOpts.FileResolver, TypeResolver: signingOpts.TypeResolver, }) - if opts.TextualCoinMetadataQueryFn == nil { - panic("cannot enable SIGN_MODE_TEXTUAL without a TextualCoinMetadataQueryFn") + if configOpts.TextualCoinMetadataQueryFn == nil { + return nil, fmt.Errorf("cannot enable SIGN_MODE_TEXTUAL without a TextualCoinMetadataQueryFn") } if err != nil { - panic(err) + return nil, err } } } - for i, m := range opts.CustomSignModes { + for i, m := range configOpts.CustomSignModes { handlers[i+lenSignModes] = m } - txConfig.handler = txsigning.NewHandlerMap(handlers...) - return txConfig + handler := txsigning.NewHandlerMap(handlers...) + return handler, nil +} + +// NewTxConfigWithOptions returns a new protobuf TxConfig using the provided ProtoCodec, ConfigOptions and +// custom sign mode handlers. If ConfigOptions is an empty struct then default values will be used. +func NewTxConfigWithOptions(protoCodec codec.ProtoCodecMarshaler, configOptions ConfigOptions) (client.TxConfig, error) { + txConfig := &config{ + protoCodec: protoCodec, + } + if configOptions.ProtoDecoder == nil { + txConfig.decoder = DefaultTxDecoder(protoCodec) + } + if configOptions.ProtoEncoder == nil { + txConfig.encoder = DefaultTxEncoder() + } + if configOptions.JSONDecoder == nil { + txConfig.jsonDecoder = DefaultJSONTxDecoder(protoCodec) + } + if configOptions.JSONEncoder == nil { + txConfig.jsonEncoder = DefaultJSONTxEncoder(protoCodec) + } + + var err error + opts := &configOptions + if opts.SigningContext == nil { + signingOpts := configOptions.SigningOptions + if signingOpts == nil { + signingOpts, err = NewDefaultSigningOptions() + if err != nil { + return nil, err + } + } + if signingOpts.FileResolver == nil { + signingOpts.FileResolver = protoCodec.InterfaceRegistry() + } + opts.SigningContext, err = txsigning.NewContext(*signingOpts) + if err != nil { + return nil, err + } + } + txConfig.signingContext = opts.SigningContext + + if opts.SigningHandler != nil { + txConfig.handler = opts.SigningHandler + return txConfig, nil + } + + txConfig.handler, err = NewSigningHandlerMap(configOptions) + if err != nil { + return nil, err + } + + return txConfig, nil } func (g config) NewTxBuilder() client.TxBuilder { diff --git a/x/auth/tx/config/config.go b/x/auth/tx/config/config.go index 70f28466ed..b249dbf943 100644 --- a/x/auth/tx/config/config.go +++ b/x/auth/tx/config/config.go @@ -91,7 +91,10 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { txConfigOptions.TextualCoinMetadataQueryFn = NewBankKeeperCoinMetadataQueryFn(in.MetadataBankKeeper) } - txConfig := tx.NewTxConfigWithOptions(in.ProtoCodecMarshaler, txConfigOptions) + txConfig, err := tx.NewTxConfigWithOptions(in.ProtoCodecMarshaler, txConfigOptions) + if err != nil { + panic(err) + } baseAppOption := func(app *baseapp.BaseApp) { // AnteHandlers