From fd93ee77e4fd6a1e47bb5c9e6bffa1a58bc68fd7 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Tue, 24 Oct 2023 23:14:10 +0200 Subject: [PATCH] fix(simapp): textual wiring (#18242) --- UPGRADING.md | 38 +++++++++++++++++++------------------- client/cmd.go | 16 ++++------------ client/tx/factory.go | 4 +--- client/tx/tx.go | 7 ++----- client/v2/autocli/msg.go | 28 ++++++++++++++++------------ client/v2/go.mod | 2 +- simapp/simd/cmd/root_v2.go | 14 ++++++++++---- x/auth/tx/config/config.go | 8 ++++---- 8 files changed, 57 insertions(+), 60 deletions(-) diff --git a/UPGRADING.md b/UPGRADING.md index 01c827975730..73fbc12a4e99 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -34,22 +34,6 @@ Refer to SimApp `root_v2.go` and `root.go` for an example with an app v2 and a l ### Modules -#### `x/group` - -Group was spun out into its own `go.mod`. To import it use `cosmossdk.io/x/group` - -#### `x/gov` - -Gov was spun out into its own `go.mod`. To import it use `cosmossdk.io/x/gov` - -#### `x/distribution` - -Distribution was spun out into its own `go.mod`. To import it use `cosmossdk.io/x/distribution` - -#### Params - -A standalone Go module was created and it is accessible at "cosmossdk.io/x/params". - #### `**all**` ##### Genesis Interface @@ -74,10 +58,24 @@ Most of Cosmos SDK modules have migrated to [collections](https://docs.cosmos.ne Many functions have been removed due to this changes as the API can be smaller thanks to collections. For modules that have migrated, verify you are checking against `collections.ErrNotFound` when applicable. +#### `x/group` + +Group was spun out into its own `go.mod`. To import it use `cosmossdk.io/x/group` + +#### `x/gov` + +Gov was spun out into its own `go.mod`. To import it use `cosmossdk.io/x/gov` + #### `x/distribution` +Distribution was spun out into its own `go.mod`. To import it use `cosmossdk.io/x/distribution` + The existing chains using x/distribution module needs to add the new x/protocolpool module. +#### `x/params` + +A standalone Go module was created and it is accessible at "cosmossdk.io/x/params". + #### `x/protocolpool` Introducing a new `x/protocolpool` module to handle community pool funds. Its store must be added while upgrading to v0.51.x @@ -420,7 +418,7 @@ This sign mode does not allow offline signing When using (legacy) application wiring, the following must be added to `app.go` after setting the app's bank keeper: -```golang +```go enabledSignModes := append(tx.DefaultSignModes, sigtypes.SignMode_SIGN_MODE_TEXTUAL) txConfigOpts := tx.ConfigOptions{ EnabledSignModes: enabledSignModes, @@ -436,9 +434,11 @@ When using (legacy) application wiring, the following must be added to `app.go` app.txConfig = txConfig ``` +When using `depinject` / `app v2`, **it's enabled by default** if there's a bank keeper present. + And in the application client (usually `root.go`): -```golang +```go if !clientCtx.Offline { txConfigOpts.EnabledSignModes = append(txConfigOpts.EnabledSignModes, signing.SignMode_SIGN_MODE_TEXTUAL) txConfigOpts.TextualCoinMetadataQueryFn = txmodule.NewGRPCCoinMetadataQueryFn(clientCtx) @@ -453,7 +453,7 @@ And in the application client (usually `root.go`): } ``` -When using `depinject` / `app v2`, **it's enabled by default** if there's a bank keeper present. +When using `depinject` / `app v2`, the a tx config should be recreated from the `txConfigOpts` to use `NewGRPCCoinMetadataQueryFn` instead of depending on the bank keeper (that is used in the server). To learn more see the [docs](https://docs.cosmos.network/main/learn/advanced/transactions#sign_mode_textual) and the [ADR-050](https://docs.cosmos.network/main/build/architecture/adr-050-sign-mode-textual). diff --git a/client/cmd.go b/client/cmd.go index 4837bddea557..49a52d462858 100644 --- a/client/cmd.go +++ b/client/cmd.go @@ -8,6 +8,7 @@ import ( "github.com/cockroachdb/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" + "golang.org/x/exp/slices" "google.golang.org/grpc" "google.golang.org/grpc/credentials" "google.golang.org/grpc/credentials/insecure" @@ -283,14 +284,7 @@ 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 { + if !slices.Contains(clientCtx.TxConfig.SignModeHandler().SupportedModes(), signingv1beta1.SignMode_SIGN_MODE_TEXTUAL) { return clientCtx, fmt.Errorf("SIGN_MODE_TEXTUAL is not available") } } @@ -311,14 +305,12 @@ func readTxCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Context, err isAux, _ := flagSet.GetBool(flags.FlagAux) clientCtx = clientCtx.WithAux(isAux) if isAux { - // If the user didn't explicitly set an --output flag, use JSON by - // default. + // If the user didn't explicitly set an --output flag, use JSON by default. if clientCtx.OutputFormat == "" || !flagSet.Changed(flags.FlagOutput) { clientCtx = clientCtx.WithOutputFormat(flags.OutputFormatJSON) } - // If the user didn't explicitly set a --sign-mode flag, use - // DIRECT_AUX by default. + // If the user didn't explicitly set a --sign-mode flag, use DIRECT_AUX by default. if clientCtx.SignModeStr == "" || !flagSet.Changed(flags.FlagSignMode) { clientCtx = clientCtx.WithSignModeStr(flags.SignModeDirectAux) } diff --git a/client/tx/factory.go b/client/tx/factory.go index b8e9bab4fd8a..cb02c95d570b 100644 --- a/client/tx/factory.go +++ b/client/tx/factory.go @@ -57,10 +57,8 @@ func NewFactoryCLI(clientCtx client.Context, flagSet *pflag.FlagSet) (Factory, e return Factory{}, fmt.Errorf("failed to bind flags to viper: %w", err) } - signModeStr := clientCtx.SignModeStr - signMode := signing.SignMode_SIGN_MODE_UNSPECIFIED - switch signModeStr { + switch clientCtx.SignModeStr { case flags.SignModeDirect: signMode = signing.SignMode_SIGN_MODE_DIRECT case flags.SignModeLegacyAminoJSON: diff --git a/client/tx/tx.go b/client/tx/tx.go index b9335ee8cc0e..f71111829627 100644 --- a/client/tx/tx.go +++ b/client/tx/tx.go @@ -126,8 +126,7 @@ func BroadcastTx(clientCtx client.Context, txf Factory, msgs ...sdk.Msg) error { } } - err = Sign(clientCtx.CmdContext, txf, clientCtx.FromName, tx, true) - if err != nil { + if err = Sign(clientCtx.CmdContext, txf, clientCtx.FromName, tx, true); err != nil { return err } @@ -323,9 +322,7 @@ func Sign(ctx context.Context, txf Factory, name string, txBuilder client.TxBuil return err } - bytesToSign, err := authsigning.GetSignBytesAdapter( - ctx, txf.txConfig.SignModeHandler(), - signMode, signerData, txBuilder.GetTx()) + bytesToSign, err := authsigning.GetSignBytesAdapter(ctx, txf.txConfig.SignModeHandler(), signMode, signerData, txBuilder.GetTx()) if err != nil { return err } diff --git a/client/v2/autocli/msg.go b/client/v2/autocli/msg.go index 5e0db2fb25ef..5bad6e1817ff 100644 --- a/client/v2/autocli/msg.go +++ b/client/v2/autocli/msg.go @@ -6,7 +6,6 @@ import ( "github.com/cockroachdb/errors" "github.com/spf13/cobra" - "golang.org/x/exp/slices" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/types/dynamicpb" @@ -115,21 +114,26 @@ func (b *Builder) BuildMsgMethodCommand(descriptor protoreflect.MethodDescriptor return err } - // enable sign mode textual and config tx options - if !clientCtx.Offline && !slices.Contains(b.TxConfigOpts.EnabledSignModes, signing.SignMode_SIGN_MODE_TEXTUAL) { + clientCtx = clientCtx.WithCmdContext(cmd.Context()) + clientCtx = clientCtx.WithOutput(cmd.OutOrStdout()) + + // enable sign mode textual + // the config is always overwritten as we need to have set the flags to the client context + // this ensures that the context has the correct client. + if !clientCtx.Offline { b.TxConfigOpts.EnabledSignModes = append(b.TxConfigOpts.EnabledSignModes, signing.SignMode_SIGN_MODE_TEXTUAL) b.TxConfigOpts.TextualCoinMetadataQueryFn = authtxconfig.NewGRPCCoinMetadataQueryFn(clientCtx) - } - txConfig, err := authtx.NewTxConfigWithOptions( - codec.NewProtoCodec(clientCtx.InterfaceRegistry), - b.TxConfigOpts, - ) - if err != nil { - return err + txConfig, err := authtx.NewTxConfigWithOptions( + codec.NewProtoCodec(clientCtx.InterfaceRegistry), + b.TxConfigOpts, + ) + if err != nil { + return err + } + + clientCtx = clientCtx.WithTxConfig(txConfig) } - clientCtx = clientCtx.WithTxConfig(txConfig) - clientCtx.Output = cmd.OutOrStdout() // set signer to signer field if empty fd := input.Descriptor().Fields().ByName(protoreflect.Name(flag.GetSignerFieldName(input.Descriptor()))) diff --git a/client/v2/go.mod b/client/v2/go.mod index 35e3f2a5837e..35a3aa9a6d29 100644 --- a/client/v2/go.mod +++ b/client/v2/go.mod @@ -13,7 +13,6 @@ require ( github.com/cosmos/gogoproto v1.4.11 github.com/spf13/cobra v1.7.0 github.com/spf13/pflag v1.0.5 - golang.org/x/exp v0.0.0-20231006140011-7918f672742d google.golang.org/grpc v1.59.0 google.golang.org/protobuf v1.31.0 gotest.tools/v3 v3.5.1 @@ -140,6 +139,7 @@ require ( go.etcd.io/bbolt v1.3.7 // indirect go.uber.org/multierr v1.11.0 // indirect golang.org/x/crypto v0.14.0 // indirect + golang.org/x/exp v0.0.0-20231006140011-7918f672742d // indirect golang.org/x/net v0.17.0 // indirect golang.org/x/sync v0.4.0 // indirect golang.org/x/sys v0.13.0 // indirect diff --git a/simapp/simd/cmd/root_v2.go b/simapp/simd/cmd/root_v2.go index 74cfdcced4ee..47113624b1cf 100644 --- a/simapp/simd/cmd/root_v2.go +++ b/simapp/simd/cmd/root_v2.go @@ -24,13 +24,13 @@ import ( simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" "github.com/cosmos/cosmos-sdk/types/module" "github.com/cosmos/cosmos-sdk/x/auth/tx" + authtxconfig "github.com/cosmos/cosmos-sdk/x/auth/tx/config" "github.com/cosmos/cosmos-sdk/x/auth/types" ) // NewRootCmd creates a new root command for simd. It is called once in the main function. func NewRootCmd() *cobra.Command { var ( - txConfigOpts tx.ConfigOptions autoCliOpts autocli.AppOptions moduleBasicManager module.BasicManager clientCtx client.Context @@ -47,7 +47,6 @@ func NewRootCmd() *cobra.Command { ProvideKeyring, ), ), - &txConfigOpts, &autoCliOpts, &moduleBasicManager, &clientCtx, @@ -99,7 +98,7 @@ func NewRootCmd() *cobra.Command { func ProvideClientContext( appCodec codec.Codec, interfaceRegistry codectypes.InterfaceRegistry, - txConfig client.TxConfig, + txConfigOpts tx.ConfigOptions, legacyAmino *codec.LegacyAmino, addressCodec address.Codec, validatorAddressCodec runtime.ValidatorAddressCodec, @@ -110,7 +109,6 @@ func ProvideClientContext( clientCtx := client.Context{}. WithCodec(appCodec). WithInterfaceRegistry(interfaceRegistry). - WithTxConfig(txConfig). WithLegacyAmino(legacyAmino). WithInput(os.Stdin). WithAccountRetriever(types.AccountRetriever{}). @@ -127,6 +125,14 @@ func ProvideClientContext( panic(err) } + // re-create the tx config grpc instead of bank keeper + txConfigOpts.TextualCoinMetadataQueryFn = authtxconfig.NewGRPCCoinMetadataQueryFn(clientCtx) + txConfig, err := tx.NewTxConfigWithOptions(clientCtx.Codec, txConfigOpts) + if err != nil { + panic(err) + } + clientCtx = clientCtx.WithTxConfig(txConfig) + return clientCtx } diff --git a/x/auth/tx/config/config.go b/x/auth/tx/config/config.go index 7759530a28b6..09cf60c756f4 100644 --- a/x/auth/tx/config/config.go +++ b/x/auth/tx/config/config.go @@ -157,9 +157,7 @@ func newAnteHandler(txConfig client.TxConfig, in ModuleInputs) (sdk.AnteHandler, // NewBankKeeperCoinMetadataQueryFn creates a new Textual struct using the given // BankKeeper to retrieve coin metadata. // -// Note: Once we switch to ADR-033, and keepers become ADR-033 clients to each -// other, this function could probably be deprecated in favor of -// `NewTextualWithGRPCConn`. +// This function should be used in the server (app.go) and is already injected thanks to app wiring for app_v2. func NewBankKeeperCoinMetadataQueryFn(bk BankKeeper) textual.CoinMetadataQueryFn { return func(ctx context.Context, denom string) (*bankv1beta1.Metadata, error) { res, err := bk.DenomMetadataV2(ctx, &bankv1beta1.QueryDenomMetadataRequest{Denom: denom}) @@ -178,7 +176,9 @@ func NewBankKeeperCoinMetadataQueryFn(bk BankKeeper) textual.CoinMetadataQueryFn // Example: // // clientCtx := client.GetClientContextFromCmd(cmd) -// txt := tx.NewTextualWithGRPCConn(clientCtxx) +// txt := tx.NewTextualWithGRPCConn(clientCtx) +// +// This should be used in the client (root.go) of an application. func NewGRPCCoinMetadataQueryFn(grpcConn grpc.ClientConnInterface) textual.CoinMetadataQueryFn { return func(ctx context.Context, denom string) (*bankv1beta1.Metadata, error) { bankQueryClient := bankv1beta1.NewQueryClient(grpcConn)