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.

Sign up for GitHub

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

Jump to bottom

fix(simapp): textual wiring #18242

Merged
merged 5 commits into from
Oct 24, 2023
Merged

fix(simapp): textual wiring #18242

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
70282a1
fix(simapp): textual wiring
julienrbrt Oct 24, 2023
edae272
update docs
julienrbrt Oct 24, 2023
eb28327
autocli fixes
julienrbrt Oct 24, 2023
57d05ee
updates
julienrbrt Oct 24, 2023
a461239
update upgrading.md
julienrbrt Oct 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 4 additions & 12 deletions client/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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")
}
}
Expand All @@ -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.
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
if clientCtx.SignModeStr == "" || !flagSet.Changed(flags.FlagSignMode) {
clientCtx = clientCtx.WithSignModeStr(flags.SignModeDirectAux)
}
Expand Down
4 changes: 1 addition & 3 deletions client/tx/factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
7 changes: 2 additions & 5 deletions client/tx/tx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
Expand Down
28 changes: 16 additions & 12 deletions client/v2/autocli/msg.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We removed this check here that !slices.Contains(b.TxConfigOpts.EnabledSignModes, signing.SignMode_SIGN_MODE_TEXTUAL), was that intentional?

Copy link
Member Author

@julienrbrt julienrbrt Oct 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it was intentional.
In the client (autocli or root.go), we always need to overwrite the TxConfigOpts.TextualCoinMetadataQueryFn to use NewGRPCCoinMetadataQueryFn. If an app is using depinject, that value will using the bank keeper, which is only correct for the server (app.go)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it and thank you!

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())))
Expand Down
2 changes: 1 addition & 1 deletion client/v2/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
14 changes: 10 additions & 4 deletions simapp/simd/cmd/root_v2.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -47,7 +47,6 @@ func NewRootCmd() *cobra.Command {
ProvideKeyring,
),
),
&txConfigOpts,
&autoCliOpts,
&moduleBasicManager,
&clientCtx,
Expand Down Expand Up @@ -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,
Expand All @@ -110,7 +109,6 @@ func ProvideClientContext(
clientCtx := client.Context{}.
WithCodec(appCodec).
WithInterfaceRegistry(interfaceRegistry).
WithTxConfig(txConfig).
WithLegacyAmino(legacyAmino).
WithInput(os.Stdin).
WithAccountRetriever(types.AccountRetriever{}).
Expand All @@ -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
}

Expand Down
8 changes: 4 additions & 4 deletions x/auth/tx/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand All @@ -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)
Expand Down
Loading