Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: enable sign mode textual by default #15970

Merged
merged 15 commits into from
Jun 1, 2023
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
15 changes: 12 additions & 3 deletions client/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -280,10 +282,17 @@ 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")
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
Expand Down
8 changes: 6 additions & 2 deletions simapp/simd/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import (
rosettaCmd "cosmossdk.io/tools/rosetta/cmd"
cmtcfg "github.com/cometbft/cometbft/config"
dbm "github.com/cosmos/cosmos-db"
"github.com/spf13/cobra"
"github.com/spf13/viper"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/config"
"github.com/cosmos/cosmos-sdk/client/debug"
Expand All @@ -29,15 +32,14 @@ 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"
"github.com/cosmos/cosmos-sdk/x/auth/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
"github.com/cosmos/cosmos-sdk/x/crisis"
genutilcli "github.com/cosmos/cosmos-sdk/x/genutil/client/cli"
"github.com/spf13/cobra"
"github.com/spf13/viper"
)

// NewRootCmd creates a new root command for simd. It is called once in the
Expand Down Expand Up @@ -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(
Expand Down
4 changes: 4 additions & 0 deletions simapp/simd/cmd/root_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -33,6 +34,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"
Expand Down Expand Up @@ -94,7 +96,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(
Expand Down
13 changes: 4 additions & 9 deletions x/auth/tx/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -54,22 +55,16 @@ var DefaultSignModes = []signingtypes.SignMode{
signingtypes.SignMode_SIGN_MODE_DIRECT,
signingtypes.SignMode_SIGN_MODE_DIRECT_AUX,
signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON,
facundomedica marked this conversation as resolved.
Show resolved Hide resolved
// 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 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
// 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,
// 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,
Expand Down
13 changes: 13 additions & 0 deletions x/auth/tx/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ import (
"cosmossdk.io/core/appmodule"
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"
Expand Down Expand Up @@ -44,6 +47,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"`
Expand All @@ -66,7 +70,9 @@ func ProvideModule(in ModuleInputs) ModuleOutputs {
customSignModeHandlers = in.CustomSignModeHandlers()
}
sdkConfig := sdk.GetConfig()

txConfigOptions := tx.ConfigOptions{
EnabledSignModes: tx.DefaultSignModes,
SigningOptions: &txsigning.Options{
FileResolver: in.ProtoFileResolver,
// From static config? But this is already in auth config.
Expand All @@ -78,6 +84,13 @@ func ProvideModule(in ModuleInputs) ModuleOutputs {
},
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) {
Expand Down