Fix --dry-run for TXs

FORKED_CHANGELOG.md

@@ -42,6 +42,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 * [\#2](https://github.com/sei-protocol/sei-cosmos/pull/2) Fix GRPC bug
 * [\#88](https://github.com/sei-protocol/sei-cosmos/pull/88) Fix Rollback bug not actually rolling back CMS version
+* [https://github.com/cosmos/cosmos-sdk/commit/4f1cc3aeac7884cf1f522b87af04a455e92db7cf]
 
 ### Improvements
 

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

@@ -3,6 +3,7 @@ package client
 import (
 	"bufio"
 	"encoding/json"
+	"fmt"
 	"io"
 	"os"
 
@@ -11,7 +12,6 @@ import (
 	"gopkg.in/yaml.v2"
 
 	"github.com/gogo/protobuf/proto"
-	"github.com/pkg/errors"
 	rpcclient "github.com/tendermint/tendermint/rpc/client"
 
 	"github.com/cosmos/cosmos-sdk/codec"
@@ -330,25 +330,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
 	}
 
-	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, "must provide a valid Bech32 address in generate-only mode")
+			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 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

@@ -13,6 +13,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 +114,114 @@ func TestCLIQueryConn(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, "hello", res.Message)
 }
+
+func TestGetFromFields(t *testing.T) {
+	path := hd.CreateHDPath(118, 0, 0).String()
+	testCases := []struct {
+		name        string
+		clientCtx   client.Context
+		keyring     func() keyring.Keyring
+		from        string
+		expectedErr string
+	}{
+		{
+			name: "from 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: "from keyring",
+			keyring: func() keyring.Keyring {
+				kb, err := keyring.New(t.Name(), keyring.BackendTest, t.TempDir(), nil)
+				require.NoError(t, err)
+
+				_, _, err = kb.NewMnemonic("alice", keyring.English, path, keyring.DefaultBIP39Passphrase, hd.Secp256k1)
+				require.NoError(t, err)
+
+				return kb
+			},
+			from: "alice",
+		},
+		{
+			name: "key not found, in memory",
+			keyring: func() keyring.Keyring {
+				return keyring.NewInMemory()
+			},
+			from:        "cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5",
+			expectedErr: "key not found",
+		},
+		{
+			keyring: func() keyring.Keyring {
+				kb, err := keyring.New(t.Name(), keyring.BackendTest, t.TempDir(), nil)
+				require.NoError(t, err)
+				return kb
+			},
+			from:        "alice",
+			expectedErr: "alice.info: key not found",
+		},
+		{
+			name: "bech32 address",
+			keyring: func() keyring.Keyring {
+				return keyring.NewInMemory()
+			},
+			from:      "cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5",
+			clientCtx: client.Context{}.WithSimulation(true),
+		},
+		{
+			name: "invalid bech32 address, simulation",
+			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: "generate only mode, valid bech32 address",
+			keyring: func() keyring.Keyring {
+				return keyring.NewInMemory()
+			},
+			from:      "cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5",
+			clientCtx: client.Context{}.WithGenerateOnly(true),
+		},
+		{
+			name: "invalid bech32 address, generate only",
+			keyring: func() keyring.Keyring {
+				return keyring.NewInMemory()
+			},
+			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)
+				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 {
+		println("Running test case: ", tc.name)
+		_, _, _, err := client.GetFromFields(tc.clientCtx, tc.keyring(), tc.from)
+		if tc.expectedErr == "" {
+			require.NoError(t, err)
+		} else {
+			require.ErrorContains(t, err, tc.expectedErr)
+		}
+	}
+}

client/flags/flags.go

@@ -108,8 +108,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)")

client/tx/factory.go

@@ -279,19 +279,9 @@ func (f Factory) BuildSimTx(msgs ...sdk.Msg) ([]byte, error) {
 		return nil, err
 	}
 
-	// use the first element from the list of keys in order to generate a valid
-	// pubkey that supports multiple algorithms
-
-	var pk cryptotypes.PubKey = &secp256k1.PubKey{} // use default public key type
-
-	if 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")
-		}
-
-		// take the first info record just for simulation purposes
-		pk = infos[0].GetPubKey()
+	pk, err := f.getSimPK()
+	if err != nil {
+		return nil, err
 	}
 
 	// Create an empty signature literal as the ante handler will populate with a
@@ -310,6 +300,31 @@ func (f Factory) BuildSimTx(msgs ...sdk.Msg) ([]byte, error) {
 	return f.txConfig.TxEncoder()(txb.GetTx())
 }
 
+// getSimPK gets the public key to use for building a simulation tx.
+// Note, we should only check for keys in the keybase if we are in simulate and execute mode,
+// e.g. when using --gas=auto.
+// When using --dry-run, we are is simulation mode only and should not check the keybase.
+// Ref: https://github.com/cosmos/cosmos-sdk/issues/11283
+func (f Factory) getSimPK() (cryptotypes.PubKey, error) {
+	var (
+		pk cryptotypes.PubKey = &secp256k1.PubKey{} // use default public key type
+	)
+
+	// Use the first element from the list of keys in order to generate a valid
+	// pubkey that supports multiple algorithms.
+	if f.simulateAndExecute && f.keybase != nil {
+		records, _ := f.keybase.List()
+		if len(records) == 0 {
+			return nil, errors.New("cannot build signature for simulation, key records slice is empty")
+		}
+
+		// take the first record just for simulation purposes
+		pk = records[0].GetPubKey()
+	}
+
+	return pk, nil
+}
+
 // Prepare ensures the account defined by ctx.GetFromAddress() exists and
 // if the account number and/or the account sequence number are zero (not set),
 // they will be queried for and set on the provided Factory. A new Factory with

x/auth/client/cli/tx_sign.go

@@ -101,7 +101,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)
 				}
@@ -110,7 +110,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)
 				}
@@ -229,15 +229,15 @@ 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)
 		}
 
 		overwrite, _ := f.GetBool(flagOverwrite)
 		if multisig != "" {
 			// Bech32 decode error, maybe it's a name, we try to fetch from keyring
-			multisigAddr, multisigName, _, err := client.GetFromFields(txF.Keybase(), multisig, clientCtx.GenerateOnly)
+			multisigAddr, multisigName, _, err := client.GetFromFields(clientCtx, txF.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)