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.

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

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

Merged
merged 6 commits into from
Oct 31, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Bug Fixes

* [#13673](https://github.com/cosmos/cosmos-sdk/pull/13673) Fix `--dry-run` flag not working when using tx command.
likhita-809 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

@julienrbrt julienrbrt Oct 31, 2022

Choose a reason for hiding this comment

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

@likhita-809, I've missed it but the changelog has been included to the wrong place. Can you put it under unreleased?

* [#13046](https://github.com/cosmos/cosmos-sdk/pull/13046) Fix missing return statement in BaseApp.Query.

## v0.45.7 - 2022-08-04
Expand Down
2 changes: 1 addition & 1 deletion client/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
16 changes: 11 additions & 5 deletions client/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand All @@ -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
Expand All @@ -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...)
}

Expand Down
108 changes: 108 additions & 0 deletions client/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"os"
"strings"
"testing"

"github.com/spf13/viper"
Expand All @@ -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"
Expand Down Expand Up @@ -113,3 +115,109 @@ 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 {
clientCtx client.Context
keyring func() keyring.Keyring
from string
expectedErr string
}{
likhita-809 marked this conversation as resolved.
Show resolved Hide resolved
{
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",
},
{
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",
},
{
keyring: func() keyring.Keyring {
return keyring.NewInMemory()
},
from: "cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5",
expectedErr: "key with address 8953EB4F1B47C7982A698DEB7C557D6E4F4CD923 not found: key not found",
},
{
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",
},
{
keyring: func() keyring.Keyring {
return keyring.NewInMemory()
},
from: "cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5",
clientCtx: client.Context{}.WithSimulation(true),
},
{
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",
},
{
keyring: func() keyring.Keyring {
return keyring.NewInMemory()
},
from: "cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5",
clientCtx: client.Context{}.WithGenerateOnly(true),
},
{
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",
},
{
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))
}

}
}
2 changes: 1 addition & 1 deletion client/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion client/tx/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
4 changes: 2 additions & 2 deletions crypto/keyring/keyring.go
Original file line number Diff line number Diff line change
Expand Up @@ -451,11 +451,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))
}
Expand Down
8 changes: 4 additions & 4 deletions x/auth/client/cli/tx_sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down
5 changes: 3 additions & 2 deletions x/bank/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
2 changes: 1 addition & 1 deletion x/feegrant/client/testutil/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down