fix: --dry-run flag not working when using tx cmd (cosmos#13673)

CHANGELOG.md

@@ -112,8 +112,13 @@ Reverted #12437 due to API breaking changes.
 
 ### Bug Fixes
 
+* [#13673](https://github.com/cosmos/cosmos-sdk/pull/13673) Fix `--dry-run` flag not working when using tx command.
 * [#13046](https://github.com/cosmos/cosmos-sdk/pull/13046) Fix missing return statement in BaseApp.Query.
 
+### API Breaking Changes
+
+* [#13673](https://github.com/cosmos/cosmos-sdk/pull/13673) The `GetFromFields` function now takes `Context` as an argument and removes `genOnly`.
+
 ## v0.45.7 - 2022-08-04
 
 ### Features

client/cmd.go

@@ -235,7 +235,7 @@ func readTxCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Context, err
 
 	if clientCtx.From == "" || flagSet.Changed(flags.FlagFrom) {
 		from, _ := flagSet.GetString(flags.FlagFrom)
-		fromAddr, fromName, keyType, err := GetFromFields(clientCtx.Keyring, from, clientCtx.GenerateOnly)
+		fromAddr, fromName, keyType, err := GetFromFields(clientCtx, clientCtx.Keyring, from)
 		if err != nil {
 			return clientCtx, err
 		}

client/context.go

@@ -333,13 +333,19 @@ func (ctx Context) printOutput(out []byte) error {
 // GetFromFields returns a from account address, account name and keyring type, given either
 // an address or key name. If genOnly is true, only a valid Bech32 cosmos
 // address is returned.
-func GetFromFields(kr keyring.Keyring, from string, genOnly bool) (sdk.AccAddress, string, keyring.KeyType, error) {
+func GetFromFields(clientCtx Context, kr keyring.Keyring, from string) (sdk.AccAddress, string, keyring.KeyType, error) {
 	if from == "" {
 		return nil, "", 0, nil
 	}
 
-	if genOnly {
-		addr, err := sdk.AccAddressFromBech32(from)
+	addr, err := sdk.AccAddressFromBech32(from)
+	switch {
+	case clientCtx.Simulate:
+		if err != nil {
+			return nil, "", 0, errors.Wrap(err, "a valid bech32 address must be provided in simulation mode")
+		}
+		return addr, "", 0, nil
+	case clientCtx.GenerateOnly:
 		if err != nil {
 			return nil, "", 0, errors.Wrap(err, "must provide a valid Bech32 address in generate-only mode")
 		}
@@ -348,7 +354,7 @@ func GetFromFields(kr keyring.Keyring, from string, genOnly bool) (sdk.AccAddres
 	}
 
 	var info keyring.Info
-	if addr, err := sdk.AccAddressFromBech32(from); err == nil {
+	if err == nil {
 		info, err = kr.KeyByAddress(addr)
 		if err != nil {
 			return nil, "", 0, err
@@ -365,7 +371,7 @@ func GetFromFields(kr keyring.Keyring, from string, genOnly bool) (sdk.AccAddres
 
 // NewKeyringFromBackend gets a Keyring object from a backend
 func NewKeyringFromBackend(ctx Context, backend string) (keyring.Keyring, error) {
-	if ctx.GenerateOnly || ctx.Simulate {
+	if ctx.Simulate {
 		return keyring.New(sdk.KeyringServiceName(), keyring.BackendMemory, ctx.KeyringDir, ctx.Input, ctx.KeyringOptions...)
 	}
 

client/context_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"os"
+	"strings"
 	"testing"
 
 	"github.com/spf13/viper"
@@ -13,6 +14,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/client/flags"
 	"github.com/cosmos/cosmos-sdk/codec"
 	"github.com/cosmos/cosmos-sdk/codec/types"
+	"github.com/cosmos/cosmos-sdk/crypto/hd"
 	"github.com/cosmos/cosmos-sdk/crypto/keyring"
 	"github.com/cosmos/cosmos-sdk/testutil/network"
 	"github.com/cosmos/cosmos-sdk/testutil/testdata"
@@ -113,3 +115,119 @@ func TestCLIQueryConn(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, "hello", res.Message)
 }
+
+func TestGetFromFields(t *testing.T) {
+	ctx := client.Context{}
+	path := hd.CreateHDPath(118, 0, 0).String()
+
+	testCases := []struct {
+		name        string
+		clientCtx   client.Context
+		keyring     func() keyring.Keyring
+		from        string
+		expectedErr string
+	}{
+		{
+			name: "valid key alice from memory keyring",
+			keyring: func() keyring.Keyring {
+				kb := keyring.NewInMemory()
+
+				_, _, err := kb.NewMnemonic("alice", keyring.English, path, keyring.DefaultBIP39Passphrase, hd.Secp256k1)
+				require.NoError(t, err)
+
+				return kb
+			},
+			from: "alice",
+		},
+		{
+			name: "valid key alice from test keyring",
+			keyring: func() keyring.Keyring {
+				kb, err := keyring.New(t.Name(), keyring.BackendTest, t.TempDir(), nil, ctx.KeyringOptions...)
+				require.NoError(t, err)
+
+				_, _, err = kb.NewMnemonic("alice", keyring.English, path, keyring.DefaultBIP39Passphrase, hd.Secp256k1)
+				require.NoError(t, err)
+
+				return kb
+			},
+			from: "alice",
+		},
+		{
+			name: "address not present in memory keyring",
+			keyring: func() keyring.Keyring {
+				return keyring.NewInMemory()
+			},
+			from:        "cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5",
+			expectedErr: "key with address 8953EB4F1B47C7982A698DEB7C557D6E4F4CD923 not found: key not found",
+		},
+		{
+			name: "key is not from test keyring",
+			keyring: func() keyring.Keyring {
+				kb, err := keyring.New(t.Name(), keyring.BackendTest, t.TempDir(), nil, ctx.KeyringOptions...)
+				require.NoError(t, err)
+				return kb
+			},
+			from:        "alice",
+			expectedErr: "alice.info: key not found",
+		},
+		{
+			name: "valid bech32 address is allowed when simulation is true",
+			keyring: func() keyring.Keyring {
+				return keyring.NewInMemory()
+			},
+			from:      "cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5",
+			clientCtx: client.Context{}.WithSimulation(true),
+		},
+		{
+			name: "only bech32 address is allowed as from key in simulation mode",
+			keyring: func() keyring.Keyring {
+				return keyring.NewInMemory()
+			},
+			from:        "alice",
+			clientCtx:   client.Context{}.WithSimulation(true),
+			expectedErr: "a valid bech32 address must be provided in simulation mode",
+		},
+		{
+			name: "valid bech32 address is allowed when generate-only is true",
+			keyring: func() keyring.Keyring {
+				return keyring.NewInMemory()
+			},
+			from:      "cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5",
+			clientCtx: client.Context{}.WithGenerateOnly(true),
+		},
+		{
+			name: "only bech32 address is allowed as from key in generate-only mode",
+			keyring: func() keyring.Keyring {
+				return keyring.NewInMemory()
+			},
+			from:        "alice",
+			clientCtx:   client.Context{}.WithGenerateOnly(true),
+			expectedErr: "must provide a valid Bech32 address in generate-only mode",
+		},
+		{
+			name: "only bech32 address is allowed as from key even when key is present in the keyring",
+			keyring: func() keyring.Keyring {
+				kb, err := keyring.New(t.Name(), keyring.BackendTest, t.TempDir(), nil, ctx.KeyringOptions...)
+				require.NoError(t, err)
+
+				_, _, err = kb.NewMnemonic("alice", keyring.English, path, keyring.DefaultBIP39Passphrase, hd.Secp256k1)
+				require.NoError(t, err)
+
+				return kb
+			},
+			clientCtx:   client.Context{}.WithGenerateOnly(true),
+			from:        "alice",
+			expectedErr: "must provide a valid Bech32 address in generate-only mode",
+		},
+	}
+
+	for _, tc := range testCases {
+		_, _, _, err := client.GetFromFields(tc.clientCtx, tc.keyring(), tc.from)
+		if tc.expectedErr == "" {
+			require.NoError(t, err)
+		} else {
+			require.True(t, strings.HasPrefix(err.Error(), tc.expectedErr))
+		}
+
+	}
+}

client/flags/flags.go

@@ -107,7 +107,7 @@ func AddTxFlagsToCmd(cmd *cobra.Command) {
 	cmd.Flags().Bool(FlagUseLedger, false, "Use a connected Ledger device")
 	cmd.Flags().Float64(FlagGasAdjustment, DefaultGasAdjustment, "adjustment factor to be multiplied against the estimate returned by the tx simulation; if the gas limit is set manually this flag is ignored ")
 	cmd.Flags().StringP(FlagBroadcastMode, "b", BroadcastSync, "Transaction broadcasting mode (sync|async|block)")
-	cmd.Flags().Bool(FlagDryRun, false, "ignore the --gas flag and perform a simulation of a transaction, but don't broadcast it")
+	cmd.Flags().Bool(FlagDryRun, false, "ignore the --gas flag and perform a simulation of a transaction, but don't broadcast it (when enabled, the local Keybase is not accessible)")
 	cmd.Flags().Bool(FlagGenerateOnly, false, "Build an unsigned transaction and write it to STDOUT (when enabled, the local Keybase is not accessible)")
 	cmd.Flags().Bool(FlagOffline, false, "Offline mode (does not allow any online functionality")
 	cmd.Flags().BoolP(FlagSkipConfirmation, "y", false, "Skip tx broadcasting prompt confirmation")

client/tx/factory.go

@@ -284,7 +284,7 @@ func (f Factory) BuildSimTx(msgs ...sdk.Msg) ([]byte, error) {
 
 	var pk cryptotypes.PubKey = &secp256k1.PubKey{} // use default public key type
 
-	if f.keybase != nil {
+	if f.simulateAndExecute && f.keybase != nil {
 		infos, _ := f.keybase.List()
 		if len(infos) == 0 {
 			return nil, errors.New("cannot build signature for simulation, key infos slice is empty")

crypto/keyring/keyring.go

@@ -450,11 +450,11 @@ func (ks keystore) Delete(uid string) error {
 func (ks keystore) KeyByAddress(address sdk.Address) (Info, error) {
 	ik, err := ks.db.Get(addrHexKeyAsString(address))
 	if err != nil {
-		return nil, wrapKeyNotFound(err, fmt.Sprint("key with address", address, "not found"))
+		return nil, wrapKeyNotFound(err, fmt.Sprint("key with address ", address, " not found"))
 	}
 
 	if len(ik.Data) == 0 {
-		return nil, wrapKeyNotFound(err, fmt.Sprint("key with address", address, "not found"))
+		return nil, wrapKeyNotFound(err, fmt.Sprint("key with address ", address, " not found"))
 	}
 	return ks.key(string(ik.Data))
 }

x/auth/client/cli/tx_sign.go

@@ -99,7 +99,7 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error {
 			}
 			if ms == "" {
 				from, _ := cmd.Flags().GetString(flags.FlagFrom)
-				_, fromName, _, err := client.GetFromFields(txFactory.Keybase(), from, clientCtx.GenerateOnly)
+				_, fromName, _, err := client.GetFromFields(clientCtx, txFactory.Keybase(), from)
 				if err != nil {
 					return fmt.Errorf("error getting account from keybase: %w", err)
 				}
@@ -108,7 +108,7 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error {
 					return err
 				}
 			} else {
-				multisigAddr, _, _, err := client.GetFromFields(txFactory.Keybase(), ms, clientCtx.GenerateOnly)
+				multisigAddr, _, _, err := client.GetFromFields(clientCtx, txFactory.Keybase(), ms)
 				if err != nil {
 					return fmt.Errorf("error getting account from keybase: %w", err)
 				}
@@ -228,7 +228,7 @@ func makeSignCmd() func(cmd *cobra.Command, args []string) error {
 			return err
 		}
 		from, _ := cmd.Flags().GetString(flags.FlagFrom)
-		_, fromName, _, err := client.GetFromFields(txF.Keybase(), from, clientCtx.GenerateOnly)
+		_, fromName, _, err := client.GetFromFields(clientCtx, txF.Keybase(), from)
 		if err != nil {
 			return fmt.Errorf("error getting account from keybase: %w", err)
 		}
@@ -238,7 +238,7 @@ func makeSignCmd() func(cmd *cobra.Command, args []string) error {
 			multisigAddr, err := sdk.AccAddressFromBech32(multisig)
 			if err != nil {
 				// Bech32 decode error, maybe it's a name, we try to fetch from keyring
-				multisigAddr, _, _, err = client.GetFromFields(txFactory.Keybase(), multisig, clientCtx.GenerateOnly)
+				multisigAddr, _, _, err = client.GetFromFields(clientCtx, txFactory.Keybase(), multisig)
 				if err != nil {
 					return fmt.Errorf("error getting account from keybase: %w", err)
 				}

x/bank/client/cli/tx.go

@@ -29,8 +29,9 @@ func NewTxCmd() *cobra.Command {
 func NewSendTxCmd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use: "send [from_key_or_address] [to_address] [amount]",
-		Short: `Send funds from one account to another. Note, the'--from' flag is
-ignored as it is implied from [from_key_or_address].`,
+		Short: `Send funds from one account to another. 
+		Note, the'--from' flag is ignored as it is implied from [from_key_or_address].
+		When using '--dry-run' a key name cannot be used, only a bech32 address.`,
 		Args: cobra.ExactArgs(3),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			cmd.Flags().Set(flags.FlagFrom, args[0])

x/feegrant/client/testutil/suite.go

@@ -313,7 +313,7 @@ func (s *IntegrationTestSuite) TestNewCmdFeeGrant() {
 	alreadyExistedGrantee := s.addedGrantee
 	clientCtx := val.ClientCtx
 
-	fromAddr, fromName, _, err := client.GetFromFields(clientCtx.Keyring, granter.String(), clientCtx.GenerateOnly)
+	fromAddr, fromName, _, err := client.GetFromFields(clientCtx, clientCtx.Keyring, granter.String())
 	s.Require().Equal(fromAddr, granter)
 	s.Require().NoError(err)