From 7db25635bf8c5fc3bc386b0b2cadafddb4d82825 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Thu, 27 Apr 2023 13:51:18 -0300 Subject: [PATCH 1/8] feat: enable sign mode textual by default --- client/cmd.go | 6 ------ x/auth/tx/mode_handler.go | 8 ++------ 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/client/cmd.go b/client/cmd.go index 1c649ad47031..eee21e58f0e1 100644 --- a/client/cmd.go +++ b/client/cmd.go @@ -280,12 +280,6 @@ func readTxCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Context, err clientCtx = clientCtx.WithFrom(from).WithFromAddress(fromAddr).WithFromName(fromName) - // TODO Remove this once SIGN_MODE_TEXTUAL is released - // ref: https://github.com/cosmos/cosmos-sdk/issues/11970 - if keyType == keyring.TypeLedger && clientCtx.SignModeStr == flags.SignModeTextual { - return clientCtx, fmt.Errorf("SIGN_MODE_TEXTUAL is currently not supported, please follow https://github.com/cosmos/cosmos-sdk/issues/11970") - } - // If the `from` signer account is a ledger key, we need to use // SIGN_MODE_AMINO_JSON, because ledger doesn't support proto yet. // ref: https://github.com/cosmos/cosmos-sdk/issues/8109 diff --git a/x/auth/tx/mode_handler.go b/x/auth/tx/mode_handler.go index ff0b3440a4bc..ae751a066d62 100644 --- a/x/auth/tx/mode_handler.go +++ b/x/auth/tx/mode_handler.go @@ -6,6 +6,7 @@ import ( "cosmossdk.io/x/tx/signing/direct" "cosmossdk.io/x/tx/signing/directaux" "cosmossdk.io/x/tx/signing/textual" + signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing" ) @@ -25,12 +26,7 @@ var DefaultSignModes = []signingtypes.SignMode{ signingtypes.SignMode_SIGN_MODE_DIRECT, signingtypes.SignMode_SIGN_MODE_DIRECT_AUX, signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, - // We currently don't add SIGN_MODE_TEXTUAL as part of the default sign - // modes, as it's not released yet (including the Ledger app). However, - // textual's sign mode handler is already available in this package. If you - // want to use textual for **TESTING** purposes, feel free to create a - // handler that includes SIGN_MODE_TEXTUAL. - // ref: Tracking issue for SIGN_MODE_TEXTUAL https://github.com/cosmos/cosmos-sdk/issues/11970 + signingtypes.SignMode_SIGN_MODE_TEXTUAL, } // makeSignModeHandler returns the default protobuf SignModeHandler supporting From 595ddf383df78f66a227619782bb3b6beb4e9894 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Wed, 3 May 2023 09:26:10 -0300 Subject: [PATCH 2/8] progress --- x/auth/tx/config.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/x/auth/tx/config.go b/x/auth/tx/config.go index 448c9ebb0232..eb39ba791350 100644 --- a/x/auth/tx/config.go +++ b/x/auth/tx/config.go @@ -10,6 +10,7 @@ import ( "cosmossdk.io/x/tx/signing/direct" "cosmossdk.io/x/tx/signing/directaux" "cosmossdk.io/x/tx/signing/textual" + "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" @@ -53,12 +54,7 @@ var DefaultSignModes = []signingtypes.SignMode{ signingtypes.SignMode_SIGN_MODE_DIRECT, signingtypes.SignMode_SIGN_MODE_DIRECT_AUX, signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, - // We currently don't add SIGN_MODE_TEXTUAL as part of the default sign - // modes, as it's not released yet (including the Ledger app). However, - // textual's sign mode handler is already available in this package. If you - // want to use textual for **TESTING** purposes, feel free to create a - // handler that includes SIGN_MODE_TEXTUAL. - // ref: Tracking issue for SIGN_MODE_TEXTUAL https://github.com/cosmos/cosmos-sdk/issues/11970 + signingtypes.SignMode_SIGN_MODE_TEXTUAL, } // NewTxConfig returns a new protobuf TxConfig using the provided ProtoCodec and sign modes. The From 4c95a162d91489cc4e484239e9f0af9460769a95 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Wed, 3 May 2023 13:57:42 -0300 Subject: [PATCH 3/8] progress --- x/auth/tx/config.go | 3 +-- x/auth/tx/config/config.go | 5 ++++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/x/auth/tx/config.go b/x/auth/tx/config.go index eb39ba791350..74c674d07bab 100644 --- a/x/auth/tx/config.go +++ b/x/auth/tx/config.go @@ -61,8 +61,7 @@ var DefaultSignModes = []signingtypes.SignMode{ // first enabled sign mode will become the default sign mode. // // NOTE: Use NewTxConfigWithOptions to provide a custom signing handler in case the sign mode -// is not supported by default (eg: SignMode_SIGN_MODE_EIP_191), or to enable SIGN_MODE_TEXTUAL -// (for testing purposes for now). +// is not supported by default (eg: SignMode_SIGN_MODE_EIP_191), or to enable SIGN_MODE_TEXTUAL. // // 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: diff --git a/x/auth/tx/config/config.go b/x/auth/tx/config/config.go index 69629e14cd51..ad61735d66c2 100644 --- a/x/auth/tx/config/config.go +++ b/x/auth/tx/config/config.go @@ -14,6 +14,7 @@ import ( "cosmossdk.io/depinject" txsigning "cosmossdk.io/x/tx/signing" "cosmossdk.io/x/tx/signing/textual" + authcodec "github.com/cosmos/cosmos-sdk/x/auth/codec" "github.com/cosmos/cosmos-sdk/baseapp" @@ -43,6 +44,7 @@ type ModuleInputs struct { ProtoFileResolver txsigning.ProtoFileResolver // BankKeeper is the expected bank keeper to be passed to AnteHandlers BankKeeper authtypes.BankKeeper `optional:"true"` + MetadataBankKeeper BankKeeper `optional:"true"` AccountKeeper ante.AccountKeeper `optional:"true"` FeeGrantKeeper ante.FeegrantKeeper `optional:"true"` CustomSignModeHandlers func() []txsigning.SignModeHandler `optional:"true"` @@ -75,7 +77,8 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { AddressCodec: authcodec.NewBech32Codec(sdkConfig.GetBech32AccountAddrPrefix()), ValidatorAddressCodec: authcodec.NewBech32Codec(sdkConfig.GetBech32ValidatorAddrPrefix()), }, - CustomSignModes: customSignModeHandlers, + CustomSignModes: customSignModeHandlers, + TextualCoinMetadataQueryFn: NewBankKeeperCoinMetadataQueryFn(in.MetadataBankKeeper), } txConfig := tx.NewTxConfigWithOptions(in.ProtoCodecMarshaler, txConfigOptions) From 1d22be2a92fa6c2d909c3864463b88a6a85fabec Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Fri, 5 May 2023 09:23:38 -0300 Subject: [PATCH 4/8] do not add to defaultsignmodes --- client/cmd.go | 15 +++++++++++++++ simapp/simd/cmd/root.go | 4 ++++ simapp/simd/cmd/root_v2.go | 4 ++++ x/auth/tx/config.go | 3 +-- x/auth/tx/config/config.go | 5 +++++ 5 files changed, 29 insertions(+), 2 deletions(-) diff --git a/client/cmd.go b/client/cmd.go index eee21e58f0e1..e5f9f793857e 100644 --- a/client/cmd.go +++ b/client/cmd.go @@ -12,6 +12,8 @@ import ( "google.golang.org/grpc/credentials" "google.golang.org/grpc/credentials/insecure" + signingv1beta1 "cosmossdk.io/api/cosmos/tx/signing/v1beta1" + "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/crypto/keyring" sdk "github.com/cosmos/cosmos-sdk/types" @@ -280,6 +282,19 @@ func readTxCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Context, err clientCtx = clientCtx.WithFrom(from).WithFromAddress(fromAddr).WithFromName(fromName) + if keyType == keyring.TypeLedger && clientCtx.SignModeStr == flags.SignModeTextual { + textualEnabled := false + for _, v := range clientCtx.TxConfig.SignModeHandler().SupportedModes() { + if v == signingv1beta1.SignMode_SIGN_MODE_TEXTUAL { + textualEnabled = true + break + } + } + if !textualEnabled { + return clientCtx, fmt.Errorf("SIGN_MODE_TEXTUAL is not available") + } + } + // If the `from` signer account is a ledger key, we need to use // SIGN_MODE_AMINO_JSON, because ledger doesn't support proto yet. // ref: https://github.com/cosmos/cosmos-sdk/issues/8109 diff --git a/simapp/simd/cmd/root.go b/simapp/simd/cmd/root.go index bf287893e864..ef06cd699d4b 100644 --- a/simapp/simd/cmd/root.go +++ b/simapp/simd/cmd/root.go @@ -17,6 +17,7 @@ import ( "cosmossdk.io/simapp/params" confixcmd "cosmossdk.io/tools/confix/cmd" rosettaCmd "cosmossdk.io/tools/rosetta/cmd" + "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/config" "github.com/cosmos/cosmos-sdk/client/debug" @@ -31,6 +32,7 @@ import ( simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" + "github.com/cosmos/cosmos-sdk/types/tx/signing" authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" "github.com/cosmos/cosmos-sdk/x/auth/tx" txmodule "github.com/cosmos/cosmos-sdk/x/auth/tx/config" @@ -83,7 +85,9 @@ func NewRootCmd() *cobra.Command { // This needs to go after ReadFromClientConfig, as that function // sets the RPC client needed for SIGN_MODE_TEXTUAL. + enabledSignModes := append(tx.DefaultSignModes, signing.SignMode_SIGN_MODE_TEXTUAL) txConfigOpts := tx.ConfigOptions{ + EnabledSignModes: enabledSignModes, TextualCoinMetadataQueryFn: txmodule.NewGRPCCoinMetadataQueryFn(initClientCtx), } txConfigWithTextual := tx.NewTxConfigWithOptions( diff --git a/simapp/simd/cmd/root_v2.go b/simapp/simd/cmd/root_v2.go index db8c0298afce..eec1c6413f6d 100644 --- a/simapp/simd/cmd/root_v2.go +++ b/simapp/simd/cmd/root_v2.go @@ -18,6 +18,7 @@ import ( "cosmossdk.io/simapp" confixcmd "cosmossdk.io/tools/confix/cmd" rosettaCmd "cosmossdk.io/tools/rosetta/cmd" + "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/config" "github.com/cosmos/cosmos-sdk/client/debug" @@ -32,6 +33,7 @@ import ( servertypes "github.com/cosmos/cosmos-sdk/server/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" + "github.com/cosmos/cosmos-sdk/types/tx/signing" authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" "github.com/cosmos/cosmos-sdk/x/auth/tx" txmodule "github.com/cosmos/cosmos-sdk/x/auth/tx/config" @@ -93,7 +95,9 @@ func NewRootCmd() *cobra.Command { // This needs to go after ReadFromClientConfig, as that function // sets the RPC client needed for SIGN_MODE_TEXTUAL. + enabledSignModes := append(tx.DefaultSignModes, signing.SignMode_SIGN_MODE_TEXTUAL) txConfigOpts := tx.ConfigOptions{ + EnabledSignModes: enabledSignModes, TextualCoinMetadataQueryFn: txmodule.NewGRPCCoinMetadataQueryFn(initClientCtx), } txConfigWithTextual := tx.NewTxConfigWithOptions( diff --git a/x/auth/tx/config.go b/x/auth/tx/config.go index 74c674d07bab..ae6c464142c6 100644 --- a/x/auth/tx/config.go +++ b/x/auth/tx/config.go @@ -54,7 +54,6 @@ var DefaultSignModes = []signingtypes.SignMode{ signingtypes.SignMode_SIGN_MODE_DIRECT, signingtypes.SignMode_SIGN_MODE_DIRECT_AUX, signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, - signingtypes.SignMode_SIGN_MODE_TEXTUAL, } // NewTxConfig returns a new protobuf TxConfig using the provided ProtoCodec and sign modes. The @@ -63,7 +62,7 @@ var DefaultSignModes = []signingtypes.SignMode{ // NOTE: Use NewTxConfigWithOptions to provide a custom signing handler in case the sign mode // is not supported by default (eg: SignMode_SIGN_MODE_EIP_191), or to enable SIGN_MODE_TEXTUAL. // -// We prefer to use depinject to provide client.TxConfig, but we permit this constructor usage. Within the SDK, +// 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, diff --git a/x/auth/tx/config/config.go b/x/auth/tx/config/config.go index ad61735d66c2..4206e1b301e2 100644 --- a/x/auth/tx/config/config.go +++ b/x/auth/tx/config/config.go @@ -15,6 +15,8 @@ import ( txsigning "cosmossdk.io/x/tx/signing" "cosmossdk.io/x/tx/signing/textual" + signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing" + authcodec "github.com/cosmos/cosmos-sdk/x/auth/codec" "github.com/cosmos/cosmos-sdk/baseapp" @@ -67,7 +69,10 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { customSignModeHandlers = in.CustomSignModeHandlers() } sdkConfig := sdk.GetConfig() + // Append SIGN_MODE_TEXTUAL to the default sign modes. + enabledSignModes := append(tx.DefaultSignModes, signingtypes.SignMode_SIGN_MODE_TEXTUAL) txConfigOptions := tx.ConfigOptions{ + EnabledSignModes: enabledSignModes, SigningOptions: &txsigning.Options{ FileResolver: in.ProtoFileResolver, // From static config? But this is already in auth config. From 160577468db670cd76a2414f5d982353dae7fdb7 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Fri, 5 May 2023 09:35:11 -0300 Subject: [PATCH 5/8] cl++ --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b0a16134260..0d22939f9237 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Features +* [#15970](https://github.com/cosmos/cosmos-sdk/pull/15970) Enable SIGN_MODE_TEXTUAL. * (types) [#15958](https://github.com/cosmos/cosmos-sdk/pull/15958) Add `module.NewBasicManagerFromManager` for creating a basic module manager from a module manager. * (runtime) [#15818](https://github.com/cosmos/cosmos-sdk/pull/15818) Provide logger through `depinject` instead of appBuilder. * (client) [#15597](https://github.com/cosmos/cosmos-sdk/pull/15597) Add status endpoint for clients. From 1242ae96064d54718c833c6691ecf8c819630cf6 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Tue, 30 May 2023 14:30:08 +0200 Subject: [PATCH 6/8] add extra check --- x/auth/tx/config/config.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/x/auth/tx/config/config.go b/x/auth/tx/config/config.go index 4fb58383cb41..faa0ccbd0cdf 100644 --- a/x/auth/tx/config/config.go +++ b/x/auth/tx/config/config.go @@ -70,8 +70,13 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { customSignModeHandlers = in.CustomSignModeHandlers() } sdkConfig := sdk.GetConfig() - // Append SIGN_MODE_TEXTUAL to the default sign modes. - enabledSignModes := append(tx.DefaultSignModes, signingtypes.SignMode_SIGN_MODE_TEXTUAL) + + // append SIGN_MODE_TEXTUAL to the default sign modes only if bank keeper is available + enabledSignModes := tx.DefaultSignModes + if in.BankKeeper != nil { + enabledSignModes = append(enabledSignModes, signingtypes.SignMode_SIGN_MODE_TEXTUAL) + } + txConfigOptions := tx.ConfigOptions{ EnabledSignModes: enabledSignModes, SigningOptions: &txsigning.Options{ From ff1ac759420972ed5ab44aa5acedec9fba461f2c Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Tue, 30 May 2023 15:11:15 +0200 Subject: [PATCH 7/8] better nil check --- x/auth/tx/config/config.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/x/auth/tx/config/config.go b/x/auth/tx/config/config.go index faa0ccbd0cdf..70f28466eda3 100644 --- a/x/auth/tx/config/config.go +++ b/x/auth/tx/config/config.go @@ -71,14 +71,8 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { } sdkConfig := sdk.GetConfig() - // append SIGN_MODE_TEXTUAL to the default sign modes only if bank keeper is available - enabledSignModes := tx.DefaultSignModes - if in.BankKeeper != nil { - enabledSignModes = append(enabledSignModes, signingtypes.SignMode_SIGN_MODE_TEXTUAL) - } - txConfigOptions := tx.ConfigOptions{ - EnabledSignModes: enabledSignModes, + EnabledSignModes: tx.DefaultSignModes, SigningOptions: &txsigning.Options{ FileResolver: in.ProtoFileResolver, // From static config? But this is already in auth config. @@ -88,9 +82,15 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { AddressCodec: authcodec.NewBech32Codec(sdkConfig.GetBech32AccountAddrPrefix()), ValidatorAddressCodec: authcodec.NewBech32Codec(sdkConfig.GetBech32ValidatorAddrPrefix()), }, - CustomSignModes: customSignModeHandlers, - TextualCoinMetadataQueryFn: NewBankKeeperCoinMetadataQueryFn(in.MetadataBankKeeper), + CustomSignModes: customSignModeHandlers, } + + // enable SIGN_MODE_TEXTUAL only if bank keeper is available + if in.MetadataBankKeeper != nil { + txConfigOptions.EnabledSignModes = append(txConfigOptions.EnabledSignModes, signingtypes.SignMode_SIGN_MODE_TEXTUAL) + txConfigOptions.TextualCoinMetadataQueryFn = NewBankKeeperCoinMetadataQueryFn(in.MetadataBankKeeper) + } + txConfig := tx.NewTxConfigWithOptions(in.ProtoCodecMarshaler, txConfigOptions) baseAppOption := func(app *baseapp.BaseApp) { From 6f204a1ce651794cfcc8c95bad87149e8a272598 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Wed, 31 May 2023 16:42:27 +0200 Subject: [PATCH 8/8] explain why its not in default modes --- x/auth/tx/config.go | 1 + 1 file changed, 1 insertion(+) diff --git a/x/auth/tx/config.go b/x/auth/tx/config.go index bea1bfe2c482..2ce75d184503 100644 --- a/x/auth/tx/config.go +++ b/x/auth/tx/config.go @@ -55,6 +55,7 @@ var DefaultSignModes = []signingtypes.SignMode{ signingtypes.SignMode_SIGN_MODE_DIRECT, signingtypes.SignMode_SIGN_MODE_DIRECT_AUX, signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, + // signingtypes.SignMode_SIGN_MODE_TEXTUAL is not enabled by default, as it requires a x/bank keeper or gRPC connection. } // NewTxConfig returns a new protobuf TxConfig using the provided ProtoCodec and sign modes. The