From 27f2224a03db37be5f8f89e3a14a61aa2afd5328 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Tue, 3 Oct 2023 13:34:35 +0200 Subject: [PATCH 1/7] refactor: add autocli keyring wraper and simplify ProtoCodecMarshaler interface --- baseapp/block_gas_test.go | 2 - baseapp/msg_service_router_test.go | 2 +- codec/proto_codec.go | 5 +- crypto/keyring/autocli.go | 86 ++++++++++++++++++++++++++++++ runtime/module.go | 2 +- x/auth/tx/config.go | 6 +-- x/auth/tx/config/config.go | 4 +- x/auth/tx/decoder.go | 4 +- x/auth/tx/encoder.go | 2 +- 9 files changed, 98 insertions(+), 15 deletions(-) create mode 100644 crypto/keyring/autocli.go diff --git a/baseapp/block_gas_test.go b/baseapp/block_gas_test.go index 7ae5cda888c..443ebc44503 100644 --- a/baseapp/block_gas_test.go +++ b/baseapp/block_gas_test.go @@ -76,7 +76,6 @@ func TestBaseApp_BlockGas(t *testing.T) { appBuilder *runtime.AppBuilder txConfig client.TxConfig cdc codec.Codec - pcdc codec.ProtoCodecMarshaler interfaceRegistry codectypes.InterfaceRegistry err error ) @@ -97,7 +96,6 @@ func TestBaseApp_BlockGas(t *testing.T) { &interfaceRegistry, &txConfig, &cdc, - &pcdc, &appBuilder) require.NoError(t, err) diff --git a/baseapp/msg_service_router_test.go b/baseapp/msg_service_router_test.go index 23acbe99409..4d8cff45a4d 100644 --- a/baseapp/msg_service_router_test.go +++ b/baseapp/msg_service_router_test.go @@ -91,7 +91,7 @@ func TestMsgService(t *testing.T) { var ( appBuilder *runtime.AppBuilder - cdc codec.ProtoCodecMarshaler + cdc codec.Codec interfaceRegistry codectypes.InterfaceRegistry ) err := depinject.Inject( diff --git a/codec/proto_codec.go b/codec/proto_codec.go index ea8f2e2593e..76b48a835d5 100644 --- a/codec/proto_codec.go +++ b/codec/proto_codec.go @@ -23,9 +23,9 @@ import ( // ProtoCodecMarshaler defines an interface for codecs that utilize Protobuf for both // binary and JSON encoding. +// Deprecated: Use Codec instead. type ProtoCodecMarshaler interface { Codec - InterfaceRegistry() types.InterfaceRegistry } // ProtoCodec defines a codec that utilizes Protobuf for both binary and JSON @@ -35,8 +35,7 @@ type ProtoCodec struct { } var ( - _ Codec = &ProtoCodec{} - _ ProtoCodecMarshaler = &ProtoCodec{} + _ Codec = &ProtoCodec{} ) // NewProtoCodec returns a reference to a new ProtoCodec diff --git a/crypto/keyring/autocli.go b/crypto/keyring/autocli.go new file mode 100644 index 00000000000..0dd91ff60a4 --- /dev/null +++ b/crypto/keyring/autocli.go @@ -0,0 +1,86 @@ +package keyring + +import ( + signingv1beta1 "cosmossdk.io/api/cosmos/tx/signing/v1beta1" + + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" +) + +// autoCLIKeyring represents the keyring interface used by the AutoCLI. +// It purposely does not import the AutoCLI package to avoid circular dependencies. +type autoCLIKeyring interface { + // List returns the names of all keys stored in the keyring. + List() ([]string, error) + + // LookupAddressByKeyName returns the address of the key with the given name. + LookupAddressByKeyName(name string) ([]byte, error) + + // GetPubKey returns the public key of the key with the given name. + GetPubKey(name string) (cryptotypes.PubKey, error) + + // Sign signs the given bytes with the key with the given name. + Sign(name string, msg []byte, signMode signingv1beta1.SignMode) ([]byte, error) +} + +// NewAutoCLIKeyring wraps the SDK keyring and make it compatible with the AutoCLI keyring interfaces. +func NewAutoCLIKeyring(kr Keyring) (autoCLIKeyring, error) { + return &autoCLIKeyringAdapter{kr}, nil +} + +type autoCLIKeyringAdapter struct { + Keyring +} + +func (a *autoCLIKeyringAdapter) List() ([]string, error) { + list, err := a.Keyring.List() + if err != nil { + return nil, err + } + + names := make([]string, len(list)) + for i, key := range list { + names[i] = key.Name + } + + return names, nil +} + +// LookupAddressByKeyName returns the address of a key stored in the keyring +func (a *autoCLIKeyringAdapter) LookupAddressByKeyName(name string) ([]byte, error) { + record, err := a.Keyring.Key(name) + if err != nil { + return nil, err + } + + addr, err := record.GetAddress() + if err != nil { + return nil, err + } + + return addr, nil +} + +func (a *autoCLIKeyringAdapter) GetPubKey(name string) (cryptotypes.PubKey, error) { + record, err := a.Keyring.Key(name) + if err != nil { + return nil, err + } + + return record.GetPubKey() +} + +func (a *autoCLIKeyringAdapter) Sign(name string, msg []byte, signMode signingv1beta1.SignMode) ([]byte, error) { + record, err := a.Keyring.Key(name) + if err != nil { + return nil, err + } + + sdkSignMode, err := authsigning.APISignModeToInternal(signMode) + if err != nil { + return nil, err + } + + signBytes, _, err := a.Keyring.Sign(record.Name, msg, sdkSignMode) + return signBytes, err +} diff --git a/runtime/module.go b/runtime/module.go index 44b019f5809..eca766795ad 100644 --- a/runtime/module.go +++ b/runtime/module.go @@ -82,7 +82,7 @@ func ProvideApp(interfaceRegistry codectypes.InterfaceRegistry) ( codec.Codec, *codec.LegacyAmino, *AppBuilder, - codec.ProtoCodecMarshaler, + codec.Codec, *baseapp.MsgServiceRouter, appmodule.AppModule, protodesc.Resolver, diff --git a/x/auth/tx/config.go b/x/auth/tx/config.go index 1f0a05f48dc..8c80df46d93 100644 --- a/x/auth/tx/config.go +++ b/x/auth/tx/config.go @@ -22,7 +22,7 @@ type config struct { encoder sdk.TxEncoder jsonDecoder sdk.TxDecoder jsonEncoder sdk.TxEncoder - protoCodec codec.ProtoCodecMarshaler + protoCodec codec.Codec signingContext *txsigning.Context } @@ -73,7 +73,7 @@ var DefaultSignModes = []signingtypes.SignMode{ // We prefer to use depinject to provide client.TxConfig, but we permit this constructor usage. Within the SDK, // this constructor is primarily used in tests, but also sees usage in app chains like: // https://github.com/evmos/evmos/blob/719363fbb92ff3ea9649694bd088e4c6fe9c195f/encoding/config.go#L37 -func NewTxConfig(protoCodec codec.ProtoCodecMarshaler, enabledSignModes []signingtypes.SignMode, +func NewTxConfig(protoCodec codec.Codec, enabledSignModes []signingtypes.SignMode, customSignModes ...txsigning.SignModeHandler, ) client.TxConfig { txConfig, err := NewTxConfigWithOptions(protoCodec, ConfigOptions{ @@ -165,7 +165,7 @@ func NewSigningHandlerMap(configOptions ConfigOptions) (*txsigning.HandlerMap, e // 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) { +func NewTxConfigWithOptions(protoCodec codec.Codec, configOptions ConfigOptions) (client.TxConfig, error) { txConfig := &config{ protoCodec: protoCodec, } diff --git a/x/auth/tx/config/config.go b/x/auth/tx/config/config.go index 6fb7dba5014..7759530a28b 100644 --- a/x/auth/tx/config/config.go +++ b/x/auth/tx/config/config.go @@ -42,7 +42,7 @@ type ModuleInputs struct { Config *txconfigv1.Config AddressCodec address.Codec ValidatorAddressCodec runtime.ValidatorAddressCodec - ProtoCodecMarshaler codec.ProtoCodecMarshaler + Codec codec.Codec ProtoFileResolver txsigning.ProtoFileResolver // BankKeeper is the expected bank keeper to be passed to AnteHandlers BankKeeper authtypes.BankKeeper `optional:"true"` @@ -86,7 +86,7 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { txConfigOptions.TextualCoinMetadataQueryFn = NewBankKeeperCoinMetadataQueryFn(in.MetadataBankKeeper) } - txConfig, err := tx.NewTxConfigWithOptions(in.ProtoCodecMarshaler, txConfigOptions) + txConfig, err := tx.NewTxConfigWithOptions(in.Codec, txConfigOptions) if err != nil { panic(err) } diff --git a/x/auth/tx/decoder.go b/x/auth/tx/decoder.go index 16253a4ba15..d74c7f20659 100644 --- a/x/auth/tx/decoder.go +++ b/x/auth/tx/decoder.go @@ -15,7 +15,7 @@ import ( ) // DefaultTxDecoder returns a default protobuf TxDecoder using the provided Marshaler. -func DefaultTxDecoder(cdc codec.ProtoCodecMarshaler) sdk.TxDecoder { +func DefaultTxDecoder(cdc codec.Codec) sdk.TxDecoder { return func(txBytes []byte) (sdk.Tx, error) { // Make sure txBytes follow ADR-027. err := rejectNonADR027TxRaw(txBytes) @@ -79,7 +79,7 @@ func DefaultTxDecoder(cdc codec.ProtoCodecMarshaler) sdk.TxDecoder { } // DefaultJSONTxDecoder returns a default protobuf JSON TxDecoder using the provided Marshaler. -func DefaultJSONTxDecoder(cdc codec.ProtoCodecMarshaler) sdk.TxDecoder { +func DefaultJSONTxDecoder(cdc codec.Codec) sdk.TxDecoder { return func(txBytes []byte) (sdk.Tx, error) { var theTx tx.Tx err := cdc.UnmarshalJSON(txBytes, &theTx) diff --git a/x/auth/tx/encoder.go b/x/auth/tx/encoder.go index e98106a89a8..fc60b6a04e0 100644 --- a/x/auth/tx/encoder.go +++ b/x/auth/tx/encoder.go @@ -29,7 +29,7 @@ func DefaultTxEncoder() sdk.TxEncoder { } // DefaultJSONTxEncoder returns a default protobuf JSON TxEncoder using the provided Marshaler. -func DefaultJSONTxEncoder(cdc codec.ProtoCodecMarshaler) sdk.TxEncoder { +func DefaultJSONTxEncoder(cdc codec.Codec) sdk.TxEncoder { return func(tx sdk.Tx) ([]byte, error) { txWrapper, ok := tx.(*wrapper) if ok { From 2008b52782287ee9790df1233aeb228fa820c7b2 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Tue, 3 Oct 2023 13:39:07 +0200 Subject: [PATCH 2/7] updates --- client/tx/factory.go | 7 ++++++- client/tx/tx.go | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/client/tx/factory.go b/client/tx/factory.go index 52e3abf0420..e942622e06e 100644 --- a/client/tx/factory.go +++ b/client/tx/factory.go @@ -379,7 +379,12 @@ func (f Factory) PrintUnsignedTx(clientCtx client.Context, msgs ...sdk.Msg) erro return err } - json, err := clientCtx.TxConfig.TxJSONEncoder()(unsignedTx.GetTx()) + encoder := f.txConfig.TxJSONEncoder() + if encoder == nil { + return fmt.Errorf("cannot print unsigned tx: tx json encoder is nil") + } + + json, err := encoder(unsignedTx.GetTx()) if err != nil { return err } diff --git a/client/tx/tx.go b/client/tx/tx.go index c3b678b6392..1df1afe9465 100644 --- a/client/tx/tx.go +++ b/client/tx/tx.go @@ -100,7 +100,12 @@ func BroadcastTx(clientCtx client.Context, txf Factory, msgs ...sdk.Msg) error { } if !clientCtx.SkipConfirm { - txBytes, err := clientCtx.TxConfig.TxJSONEncoder()(tx.GetTx()) + encoder := txf.txConfig.TxJSONEncoder() + if encoder == nil { + return fmt.Errorf("failed to encode transaction: tx json encoder is nil") + } + + txBytes, err := encoder(tx.GetTx()) if err != nil { return err } From 68aee82db437d4f2a3bef60c108da3028d846223 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Tue, 3 Oct 2023 13:50:12 +0200 Subject: [PATCH 3/7] updates --- runtime/module.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/runtime/module.go b/runtime/module.go index eca766795ad..4440b3b8a31 100644 --- a/runtime/module.go +++ b/runtime/module.go @@ -82,7 +82,6 @@ func ProvideApp(interfaceRegistry codectypes.InterfaceRegistry) ( codec.Codec, *codec.LegacyAmino, *AppBuilder, - codec.Codec, *baseapp.MsgServiceRouter, appmodule.AppModule, protodesc.Resolver, @@ -116,7 +115,7 @@ func ProvideApp(interfaceRegistry codectypes.InterfaceRegistry) ( } appBuilder := &AppBuilder{app} - return cdc, amino, appBuilder, cdc, msgServiceRouter, appModule{app}, protoFiles, protoTypes, nil + return cdc, amino, appBuilder, msgServiceRouter, appModule{app}, protoFiles, protoTypes, nil } type AppInputs struct { From ff2eeb2947e030c6154f4255c31651add594d57e Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Tue, 3 Oct 2023 14:42:24 +0200 Subject: [PATCH 4/7] lint --- codec/proto_codec.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/codec/proto_codec.go b/codec/proto_codec.go index 76b48a835d5..9a99fc39866 100644 --- a/codec/proto_codec.go +++ b/codec/proto_codec.go @@ -34,9 +34,7 @@ type ProtoCodec struct { interfaceRegistry types.InterfaceRegistry } -var ( - _ Codec = &ProtoCodec{} -) +var _ Codec = &ProtoCodec{} // NewProtoCodec returns a reference to a new ProtoCodec func NewProtoCodec(interfaceRegistry types.InterfaceRegistry) *ProtoCodec { From ebcb88b020bf74762b397c66cc761de1a0215466 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Tue, 3 Oct 2023 15:07:35 +0200 Subject: [PATCH 5/7] updates --- x/auth/tx/config.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/x/auth/tx/config.go b/x/auth/tx/config.go index 8c80df46d93..58b536cb6ce 100644 --- a/x/auth/tx/config.go +++ b/x/auth/tx/config.go @@ -167,7 +167,11 @@ func NewSigningHandlerMap(configOptions ConfigOptions) (*txsigning.HandlerMap, e // custom sign mode handlers. If ConfigOptions is an empty struct then default values will be used. func NewTxConfigWithOptions(protoCodec codec.Codec, configOptions ConfigOptions) (client.TxConfig, error) { txConfig := &config{ - protoCodec: protoCodec, + protoCodec: protoCodec, + decoder: configOptions.ProtoDecoder, + encoder: configOptions.ProtoEncoder, + jsonDecoder: configOptions.JSONDecoder, + jsonEncoder: configOptions.JSONEncoder, } if configOptions.ProtoDecoder == nil { txConfig.decoder = DefaultTxDecoder(protoCodec) From 5dad4565e49dcc8aa0e31a3b09704997331528e3 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Tue, 3 Oct 2023 15:11:05 +0200 Subject: [PATCH 6/7] do not extract this --- crypto/keyring/autocli.go | 86 --------------------------------------- 1 file changed, 86 deletions(-) delete mode 100644 crypto/keyring/autocli.go diff --git a/crypto/keyring/autocli.go b/crypto/keyring/autocli.go deleted file mode 100644 index 0dd91ff60a4..00000000000 --- a/crypto/keyring/autocli.go +++ /dev/null @@ -1,86 +0,0 @@ -package keyring - -import ( - signingv1beta1 "cosmossdk.io/api/cosmos/tx/signing/v1beta1" - - cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" - authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" -) - -// autoCLIKeyring represents the keyring interface used by the AutoCLI. -// It purposely does not import the AutoCLI package to avoid circular dependencies. -type autoCLIKeyring interface { - // List returns the names of all keys stored in the keyring. - List() ([]string, error) - - // LookupAddressByKeyName returns the address of the key with the given name. - LookupAddressByKeyName(name string) ([]byte, error) - - // GetPubKey returns the public key of the key with the given name. - GetPubKey(name string) (cryptotypes.PubKey, error) - - // Sign signs the given bytes with the key with the given name. - Sign(name string, msg []byte, signMode signingv1beta1.SignMode) ([]byte, error) -} - -// NewAutoCLIKeyring wraps the SDK keyring and make it compatible with the AutoCLI keyring interfaces. -func NewAutoCLIKeyring(kr Keyring) (autoCLIKeyring, error) { - return &autoCLIKeyringAdapter{kr}, nil -} - -type autoCLIKeyringAdapter struct { - Keyring -} - -func (a *autoCLIKeyringAdapter) List() ([]string, error) { - list, err := a.Keyring.List() - if err != nil { - return nil, err - } - - names := make([]string, len(list)) - for i, key := range list { - names[i] = key.Name - } - - return names, nil -} - -// LookupAddressByKeyName returns the address of a key stored in the keyring -func (a *autoCLIKeyringAdapter) LookupAddressByKeyName(name string) ([]byte, error) { - record, err := a.Keyring.Key(name) - if err != nil { - return nil, err - } - - addr, err := record.GetAddress() - if err != nil { - return nil, err - } - - return addr, nil -} - -func (a *autoCLIKeyringAdapter) GetPubKey(name string) (cryptotypes.PubKey, error) { - record, err := a.Keyring.Key(name) - if err != nil { - return nil, err - } - - return record.GetPubKey() -} - -func (a *autoCLIKeyringAdapter) Sign(name string, msg []byte, signMode signingv1beta1.SignMode) ([]byte, error) { - record, err := a.Keyring.Key(name) - if err != nil { - return nil, err - } - - sdkSignMode, err := authsigning.APISignModeToInternal(signMode) - if err != nil { - return nil, err - } - - signBytes, _, err := a.Keyring.Sign(record.Name, msg, sdkSignMode) - return signBytes, err -} From 58822a0c14a719024aca5b303ca57e3ba3a8c373 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 4 Oct 2023 09:14:45 +0200 Subject: [PATCH 7/7] feedback --- client/tx/factory.go | 6 +++--- client/tx/tx.go | 2 +- codec/proto_codec.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/client/tx/factory.go b/client/tx/factory.go index e942622e06e..f21d861f3bb 100644 --- a/client/tx/factory.go +++ b/client/tx/factory.go @@ -298,10 +298,10 @@ func (f Factory) WithExtensionOptions(extOpts ...*codectypes.Any) Factory { func (f Factory) BuildUnsignedTx(msgs ...sdk.Msg) (client.TxBuilder, error) { if f.offline && f.generateOnly { if f.chainID != "" { - return nil, fmt.Errorf("chain ID cannot be used when offline and generate-only flags are set") + return nil, errors.New("chain ID cannot be used when offline and generate-only flags are set") } } else if f.chainID == "" { - return nil, fmt.Errorf("chain ID required but not specified") + return nil, errors.New("chain ID required but not specified") } fees := f.fees @@ -381,7 +381,7 @@ func (f Factory) PrintUnsignedTx(clientCtx client.Context, msgs ...sdk.Msg) erro encoder := f.txConfig.TxJSONEncoder() if encoder == nil { - return fmt.Errorf("cannot print unsigned tx: tx json encoder is nil") + return errors.New("cannot print unsigned tx: tx json encoder is nil") } json, err := encoder(unsignedTx.GetTx()) diff --git a/client/tx/tx.go b/client/tx/tx.go index 1df1afe9465..b7c66536235 100644 --- a/client/tx/tx.go +++ b/client/tx/tx.go @@ -102,7 +102,7 @@ func BroadcastTx(clientCtx client.Context, txf Factory, msgs ...sdk.Msg) error { if !clientCtx.SkipConfirm { encoder := txf.txConfig.TxJSONEncoder() if encoder == nil { - return fmt.Errorf("failed to encode transaction: tx json encoder is nil") + return errors.New("failed to encode transaction: tx json encoder is nil") } txBytes, err := encoder(tx.GetTx()) diff --git a/codec/proto_codec.go b/codec/proto_codec.go index 9a99fc39866..26259cf06f7 100644 --- a/codec/proto_codec.go +++ b/codec/proto_codec.go @@ -34,7 +34,7 @@ type ProtoCodec struct { interfaceRegistry types.InterfaceRegistry } -var _ Codec = &ProtoCodec{} +var _ Codec = (*ProtoCodec)(nil) // NewProtoCodec returns a reference to a new ProtoCodec func NewProtoCodec(interfaceRegistry types.InterfaceRegistry) *ProtoCodec {