fix: --dry-run not working when using tx command (#11558)

## Description Closes: #11149 - Let `--dry-run` not use the local keyring - Fixes for required flags not shown - Make `--generate-only` behavior consistent --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [x] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [x] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [x] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable)
2022-04-11 15:50:54 +00:00
parent 56b0e837cc
commit 4f1cc3aeac
9 changed files with 150 additions and 23 deletions
@@ -248,7 +248,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
 		}
@@ -2,6 +2,7 @@ package client

 import (
 	"bufio"
+	"fmt"
 	"io"
 	"os"

@@ -52,9 +53,9 @@ type Context struct {
 	FeePayer          sdk.AccAddress
 	FeeGranter        sdk.AccAddress
 	Viper             *viper.Viper
-	
+
 	// IsAux is true when the signer is an auxiliary signer (e.g. the tipper).
-	IsAux             bool
+	IsAux bool

 	// TODO: Deprecated (remove).
 	LegacyAmino *codec.LegacyAmino
@@ -334,16 +335,31 @@ func (ctx Context) printOutput(out []byte) error {
 	return nil
 }

-// 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) {
+// GetFromFields returns a from account address, account name and keyring type, given either an address or key name.
+// If clientCtx.Simulate is true the keystore is not accessed and a valid address must be provided
+// If clientCtx.GenerateOnly is true the keystore is only accessed if a key name is provided
+func GetFromFields(clientCtx Context, kr keyring.Keyring, from string) (sdk.AccAddress, string, keyring.KeyType, error) {
 	if from == "" {
 		return nil, "", 0, nil
 	}

+	addr, err := sdk.AccAddressFromBech32(from)
+	switch {
+	case clientCtx.Simulate:
+		if err != nil {
+			return nil, "", 0, fmt.Errorf("a valid bech32 address must be provided in simulation mode: %w", err)
+		}
+
+		return addr, "", 0, nil
+
+	case clientCtx.GenerateOnly:
+		if err == nil {
+			return addr, "", 0, nil
+		}
+	}
+
 	var k *keyring.Record
-	if addr, err := sdk.AccAddressFromBech32(from); err == nil {
+	if err == nil {
 		k, err = kr.KeyByAddress(addr)
 		if err != nil {
 			return nil, "", 0, err
@@ -355,7 +371,7 @@ func GetFromFields(kr keyring.Keyring, from string, genOnly bool) (sdk.AccAddres
 		}
 	}

-	addr, err := k.GetAddress()
+	addr, err = k.GetAddress()
 	if err != nil {
 		return nil, "", 0, err
 	}
@@ -365,7 +381,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 {
 		backend = keyring.BackendMemory
 	}

@@ -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"
@@ -114,3 +116,107 @@ func TestCLIQueryConn(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, "hello", res.Message)
 }
+
+func TestGetFromFields(t *testing.T) {
+	cfg := network.DefaultConfig()
+	path := hd.CreateHDPath(118, 0, 0).String()
+
+	testCases := []struct {
+		clientCtx   client.Context
+		keyring     func() keyring.Keyring
+		from        string
+		expectedErr string
+	}{
+		{
+			keyring: func() keyring.Keyring {
+				kb := keyring.NewInMemory(cfg.Codec)
+
+				_, _, err := kb.NewMnemonic("alice", keyring.English, path, keyring.DefaultBIP39Passphrase, hd.Secp256k1)
+				require.NoError(t, err)
+
+				return kb
+			},
+			from: "alice",
+		},
+		{
+			keyring: func() keyring.Keyring {
+				kb, err := keyring.New(t.Name(), keyring.BackendTest, t.TempDir(), nil, cfg.Codec)
+				require.NoError(t, err)
+
+				_, _, err = kb.NewMnemonic("alice", keyring.English, path, keyring.DefaultBIP39Passphrase, hd.Secp256k1)
+				require.NoError(t, err)
+
+				return kb
+			},
+			from: "alice",
+		},
+		{
+			keyring: func() keyring.Keyring {
+				return keyring.NewInMemory(cfg.Codec)
+			},
+			from:        "cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5",
+			expectedErr: "key with address cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5 not found: key not found",
+		},
+		{
+			keyring: func() keyring.Keyring {
+				kb, err := keyring.New(t.Name(), keyring.BackendTest, t.TempDir(), nil, cfg.Codec)
+				require.NoError(t, err)
+				return kb
+			},
+			from:        "alice",
+			expectedErr: "alice.info: key not found",
+		},
+		{
+			keyring: func() keyring.Keyring {
+				return keyring.NewInMemory(cfg.Codec)
+			},
+			from:      "cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5",
+			clientCtx: client.Context{}.WithSimulation(true),
+		},
+		{
+			keyring: func() keyring.Keyring {
+				return keyring.NewInMemory(cfg.Codec)
+			},
+			from:        "alice",
+			clientCtx:   client.Context{}.WithSimulation(true),
+			expectedErr: "a valid bech32 address must be provided in simulation mode",
+		},
+		{
+			keyring: func() keyring.Keyring {
+				return keyring.NewInMemory(cfg.Codec)
+			},
+			from:      "cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5",
+			clientCtx: client.Context{}.WithGenerateOnly(true),
+		},
+		{
+			keyring: func() keyring.Keyring {
+				return keyring.NewInMemory(cfg.Codec)
+			},
+			from:        "alice",
+			clientCtx:   client.Context{}.WithGenerateOnly(true),
+			expectedErr: "alice.info: key not found",
+		},
+		{
+			keyring: func() keyring.Keyring {
+				kb, err := keyring.New(t.Name(), keyring.BackendTest, t.TempDir(), nil, cfg.Codec)
+				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",
+		},
+	}
+
+	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))
+		}
+	}
+}
@@ -112,8 +112,8 @@ 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(FlagGenerateOnly, false, "Build an unsigned transaction and write it to STDOUT (when enabled, the local Keybase is not accessible)")
+	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 only accessed when providing a key name)")
 	cmd.Flags().Bool(FlagOffline, false, "Offline mode (does not allow any online functionality)")
 	cmd.Flags().BoolP(FlagSkipConfirmation, "y", false, "Skip tx broadcasting prompt confirmation")
 	cmd.Flags().String(FlagKeyringBackend, DefaultKeyringBackend, "Select keyring's backend (os|file|kwallet|pass|test|memory)")
@@ -337,7 +337,7 @@ func (gr GasEstimateResponse) String() string {
 // makeAuxSignerData generates an AuxSignerData from the client inputs.
 func makeAuxSignerData(clientCtx client.Context, f Factory, msgs ...sdk.Msg) (tx.AuxSignerData, error) {
 	b := NewAuxTxBuilder()
-	fromAddress, name, _, err := client.GetFromFields(clientCtx.Keyring, clientCtx.From, false)
+	fromAddress, name, _, err := client.GetFromFields(clientCtx, clientCtx.Keyring, clientCtx.From)
 	if err != nil {
 		return tx.AuxSignerData{}, err
 	}