From 00a62fc93b1de557d7afaf3c79bb1f9175b1c094 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 6 Apr 2022 14:43:22 +0200 Subject: [PATCH 1/7] cli: fix` --dry-run` not working when using tx command --- client/cmd.go | 2 +- client/context.go | 34 ++++++++----- client/context_test.go | 76 +++++++++++++++++++++++++++++ x/auth/client/cli/tx_sign.go | 8 +-- x/bank/client/cli/tx.go | 1 + x/feegrant/client/testutil/suite.go | 2 +- 6 files changed, 105 insertions(+), 18 deletions(-) diff --git a/client/cmd.go b/client/cmd.go index 90d7689d5e2f..6f9f2d2d6824 100644 --- a/client/cmd.go +++ b/client/cmd.go @@ -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.Keyring, from, clientCtx.Simulate) if err != nil { return clientCtx, err } diff --git a/client/context.go b/client/context.go index 380b08924ac4..e18cf3245e69 100644 --- a/client/context.go +++ b/client/context.go @@ -2,6 +2,7 @@ package client import ( "bufio" + "fmt" "io" "os" @@ -16,6 +17,7 @@ import ( "github.com/cosmos/cosmos-sdk/codec" codectypes "github.com/cosmos/cosmos-sdk/codec/types" + "github.com/cosmos/cosmos-sdk/crypto/hd" "github.com/cosmos/cosmos-sdk/crypto/keyring" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -52,9 +54,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,24 +336,32 @@ 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 simulate is true a new temporary address will be generated +func GetFromFields(kr keyring.Keyring, from string, simulate bool) (sdk.AccAddress, string, keyring.KeyType, error) { if from == "" { return nil, "", 0, nil } var k *keyring.Record - if addr, err := sdk.AccAddressFromBech32(from); err == nil { - k, err = kr.KeyByAddress(addr) - if err != nil { - return nil, "", 0, err + var err error + + if !simulate { + if addr, err := sdk.AccAddressFromBech32(from); err == nil { + k, err = kr.KeyByAddress(addr) + if err != nil { + return nil, "", 0, err + } + } else { + k, err = kr.Key(from) + if err != nil { + return nil, "", 0, err + } } } else { - k, err = kr.Key(from) + k, _, err = kr.NewMnemonic(from, keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1) if err != nil { - return nil, "", 0, err + return nil, "", 0, fmt.Errorf("failed to simulate %s key: %w", from, err) } } diff --git a/client/context_test.go b/client/context_test.go index d500bc97783b..bf1eddea6ba6 100644 --- a/client/context_test.go +++ b/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" @@ -114,3 +115,78 @@ 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 { + keyring func() keyring.Keyring + from string + expectedErr string + simulate bool + }{ + { + 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 { + 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) + + _, _, err = kb.NewMnemonic("bob", keyring.English, path, keyring.DefaultBIP39Passphrase, hd.Secp256k1) + require.NoError(t, err) + + return kb + }, + from: "bob", + }, + { + keyring: func() keyring.Keyring { + kb, err := keyring.New(t.Name(), keyring.BackendTest, t.TempDir(), nil, cfg.Codec) + require.NoError(t, err) + return kb + }, + from: "bob", + expectedErr: "bob.info: key not found", + }, + { + keyring: func() keyring.Keyring { + return keyring.NewInMemory(cfg.Codec) + }, + from: "alice", + simulate: true, + }, + { + keyring: func() keyring.Keyring { + return keyring.NewInMemory(cfg.Codec) + }, + from: "cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5", + simulate: true, + }, + } + + for _, tc := range testCases { + _, _, _, err := client.GetFromFields(tc.keyring(), tc.from, tc.simulate) + if tc.expectedErr == "" { + require.NoError(t, err) + } else { + require.Equal(t, tc.expectedErr, err.Error()) + } + } +} diff --git a/x/auth/client/cli/tx_sign.go b/x/auth/client/cli/tx_sign.go index fd32d96767fe..3cd9d7cd314c 100644 --- a/x/auth/client/cli/tx_sign.go +++ b/x/auth/client/cli/tx_sign.go @@ -106,7 +106,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(txFactory.Keybase(), from, clientCtx.Simulate) if err != nil { return fmt.Errorf("error getting account from keybase: %w", err) } @@ -115,7 +115,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(txFactory.Keybase(), ms, clientCtx.Simulate) if err != nil { return fmt.Errorf("error getting account from keybase: %w", err) } @@ -235,7 +235,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(txF.Keybase(), from, clientCtx.Simulate) if err != nil { return fmt.Errorf("error getting account from keybase: %w", err) } @@ -245,7 +245,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(txFactory.Keybase(), multisig, clientCtx.Simulate) if err != nil { return fmt.Errorf("error getting account from keybase: %w", err) } diff --git a/x/bank/client/cli/tx.go b/x/bank/client/cli/tx.go index 4fee9ffd1e81..fb010c89bea2 100644 --- a/x/bank/client/cli/tx.go +++ b/x/bank/client/cli/tx.go @@ -38,6 +38,7 @@ ignored as it is implied from [from_key_or_address].`, if err != nil { return err } + toAddr, err := sdk.AccAddressFromBech32(args[1]) if err != nil { return err diff --git a/x/feegrant/client/testutil/suite.go b/x/feegrant/client/testutil/suite.go index dc2acc3cd63c..7d67c31bff78 100644 --- a/x/feegrant/client/testutil/suite.go +++ b/x/feegrant/client/testutil/suite.go @@ -316,7 +316,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.Keyring, granter.String(), clientCtx.Simulate) s.Require().Equal(fromAddr, granter) s.Require().NoError(err) From 88fc525a0ae588ad79d5b256dc2498a1002bd68a Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 6 Apr 2022 14:45:55 +0200 Subject: [PATCH 2/7] add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98eb4007ecd6..53b5c848ef77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -204,6 +204,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +* [\#11558](https://github.com/cosmos/cosmos-sdk/pull/11558) Fix `--dry-run` not working when using tx command. * [\#11354](https://github.com/cosmos/cosmos-sdk/pull/11355) Added missing pagination flag for `bank q total` query. * [\#11197](https://github.com/cosmos/cosmos-sdk/pull/11197) Signing with multisig now works with multisig address which is not in the keyring. * (makefile) [\#11285](https://github.com/cosmos/cosmos-sdk/pull/11285) Fix lint-fix make target. From 0179f5c691d68ca60a2c4509fb345d5de0072b8e Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Thu, 7 Apr 2022 12:23:23 +0200 Subject: [PATCH 3/7] implement feedback --- client/context.go | 33 ++++++++++++++++----------------- client/context_test.go | 28 ++++++++++++++++++---------- x/auth/client/cli/tx_sign.go | 6 ++++-- x/bank/client/cli/tx.go | 5 +++-- 4 files changed, 41 insertions(+), 31 deletions(-) diff --git a/client/context.go b/client/context.go index e18cf3245e69..e5b3d4dec5d2 100644 --- a/client/context.go +++ b/client/context.go @@ -17,7 +17,6 @@ import ( "github.com/cosmos/cosmos-sdk/codec" codectypes "github.com/cosmos/cosmos-sdk/codec/types" - "github.com/cosmos/cosmos-sdk/crypto/hd" "github.com/cosmos/cosmos-sdk/crypto/keyring" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -343,25 +342,25 @@ func GetFromFields(kr keyring.Keyring, from string, simulate bool) (sdk.AccAddre return nil, "", 0, nil } - var k *keyring.Record - var err error + if simulate { + addr, err := sdk.AccAddressFromBech32(from) + if err != nil { + return nil, "", 0, fmt.Errorf("a valid Bech32 address must be provided in simulation mode: %w", err) + } - if !simulate { - if addr, err := sdk.AccAddressFromBech32(from); err == nil { - k, err = kr.KeyByAddress(addr) - if err != nil { - return nil, "", 0, err - } - } else { - k, err = kr.Key(from) - if err != nil { - return nil, "", 0, err - } + return addr, "", 0, nil + } + + var k *keyring.Record + if addr, err := sdk.AccAddressFromBech32(from); err == nil { + k, err = kr.KeyByAddress(addr) + if err != nil { + return nil, "", 0, err } } else { - k, _, err = kr.NewMnemonic(from, keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1) + k, err = kr.Key(from) if err != nil { - return nil, "", 0, fmt.Errorf("failed to simulate %s key: %w", from, err) + return nil, "", 0, err } } @@ -375,7 +374,7 @@ func GetFromFields(kr keyring.Keyring, from string, simulate bool) (sdk.AccAddre // 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 } diff --git a/client/context_test.go b/client/context_test.go index bf1eddea6ba6..d715f6ecf914 100644 --- a/client/context_test.go +++ b/client/context_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "os" + "strings" "testing" "github.com/spf13/viper" @@ -122,7 +123,8 @@ func TestGetFromFields(t *testing.T) { testCases := []struct { keyring func() keyring.Keyring - from string + name string + addr string expectedErr string simulate bool }{ @@ -135,13 +137,13 @@ func TestGetFromFields(t *testing.T) { return kb }, - from: "alice", + name: "alice", }, { keyring: func() keyring.Keyring { return keyring.NewInMemory(cfg.Codec) }, - from: "cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5", + addr: "cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5", expectedErr: "key with address cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5 not found: key not found", }, { @@ -154,7 +156,7 @@ func TestGetFromFields(t *testing.T) { return kb }, - from: "bob", + name: "bob", }, { keyring: func() keyring.Keyring { @@ -162,31 +164,37 @@ func TestGetFromFields(t *testing.T) { require.NoError(t, err) return kb }, - from: "bob", + name: "bob", expectedErr: "bob.info: key not found", }, { keyring: func() keyring.Keyring { return keyring.NewInMemory(cfg.Codec) }, - from: "alice", + addr: "cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5", simulate: true, }, { keyring: func() keyring.Keyring { return keyring.NewInMemory(cfg.Codec) }, - from: "cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5", - simulate: true, + addr: "alice", + simulate: true, + expectedErr: "a valid Bech32 address must be provided in simulation mode", }, } for _, tc := range testCases { - _, _, _, err := client.GetFromFields(tc.keyring(), tc.from, tc.simulate) + var val = tc.name + if val == "" { + val = tc.addr + } + + _, _, _, err := client.GetFromFields(tc.keyring(), val, tc.simulate) if tc.expectedErr == "" { require.NoError(t, err) } else { - require.Equal(t, tc.expectedErr, err.Error()) + require.True(t, strings.HasPrefix(err.Error(), tc.expectedErr)) } } } diff --git a/x/auth/client/cli/tx_sign.go b/x/auth/client/cli/tx_sign.go index 3cd9d7cd314c..b5832ef833dd 100644 --- a/x/auth/client/cli/tx_sign.go +++ b/x/auth/client/cli/tx_sign.go @@ -54,9 +54,10 @@ account key. It implies --signature-only. cmd.Flags().String(flags.FlagOutputDocument, "", "The document will be written to the given file instead of STDOUT") cmd.Flags().Bool(flagSigOnly, true, "Print only the generated signature, then exit") cmd.Flags().String(flags.FlagChainID, "", "network chain ID") - cmd.MarkFlagRequired(flags.FlagFrom) flags.AddTxFlagsToCmd(cmd) + cmd.MarkFlagRequired(flags.FlagFrom) + return cmd } @@ -192,9 +193,10 @@ be generated via the 'multisign' command. cmd.Flags().String(flags.FlagOutputDocument, "", "The document will be written to the given file instead of STDOUT") cmd.Flags().String(flags.FlagChainID, "", "The network chain ID") cmd.Flags().Bool(flagAmino, false, "Generate Amino encoded JSON suitable for submiting to the txs REST endpoint") - cmd.MarkFlagRequired(flags.FlagFrom) flags.AddTxFlagsToCmd(cmd) + cmd.MarkFlagRequired(flags.FlagFrom) + return cmd } diff --git a/x/bank/client/cli/tx.go b/x/bank/client/cli/tx.go index fb010c89bea2..610f5d333da6 100644 --- a/x/bank/client/cli/tx.go +++ b/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 valid key cannot be used, only a valid address.`, Args: cobra.ExactArgs(3), RunE: func(cmd *cobra.Command, args []string) error { cmd.Flags().Set(flags.FlagFrom, args[0]) From b0b39a8aa9ae13243fb6996c5d0c61601a56f5e8 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Thu, 7 Apr 2022 12:38:52 +0200 Subject: [PATCH 4/7] update flags description --- client/flags/flags.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/flags/flags.go b/client/flags/flags.go index 669c487e604d..0e90fcce845a 100644 --- a/client/flags/flags.go +++ b/client/flags/flags.go @@ -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") 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)") From 70c6681caea37caf6709e826c38193adf1d1e0a7 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Thu, 7 Apr 2022 12:43:44 +0200 Subject: [PATCH 5/7] improve description --- client/context.go | 2 +- client/context_test.go | 2 +- x/bank/client/cli/tx.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/client/context.go b/client/context.go index e5b3d4dec5d2..d653dc519ab7 100644 --- a/client/context.go +++ b/client/context.go @@ -345,7 +345,7 @@ func GetFromFields(kr keyring.Keyring, from string, simulate bool) (sdk.AccAddre if simulate { addr, err := sdk.AccAddressFromBech32(from) if err != nil { - return nil, "", 0, fmt.Errorf("a valid Bech32 address must be provided in simulation mode: %w", err) + return nil, "", 0, fmt.Errorf("a valid bech32 address must be provided in simulation mode: %w", err) } return addr, "", 0, nil diff --git a/client/context_test.go b/client/context_test.go index d715f6ecf914..c9d553753a06 100644 --- a/client/context_test.go +++ b/client/context_test.go @@ -180,7 +180,7 @@ func TestGetFromFields(t *testing.T) { }, addr: "alice", simulate: true, - expectedErr: "a valid Bech32 address must be provided in simulation mode", + expectedErr: "a valid bech32 address must be provided in simulation mode", }, } diff --git a/x/bank/client/cli/tx.go b/x/bank/client/cli/tx.go index 610f5d333da6..87d6bfac4f70 100644 --- a/x/bank/client/cli/tx.go +++ b/x/bank/client/cli/tx.go @@ -31,7 +31,7 @@ func NewSendTxCmd() *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]. - When using '--dry-run' a valid key cannot be used, only a valid 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]) From ca53993e073a9c087c3f129df6425155a32ac7d8 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Fri, 8 Apr 2022 18:09:25 +0200 Subject: [PATCH 6/7] Implement feedback --- CHANGELOG.md | 2 +- client/cmd.go | 2 +- client/context.go | 18 +++++--- client/context_test.go | 72 +++++++++++++++++++---------- client/flags/flags.go | 2 +- client/tx/tx.go | 2 +- x/auth/client/cli/tx_sign.go | 8 ++-- x/feegrant/client/testutil/suite.go | 2 +- 8 files changed, 68 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53b5c848ef77..9c96e5f60433 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,7 +58,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [\#9533](https://github.com/cosmos/cosmos-sdk/pull/9533) Added a new gRPC method, `DenomOwners`, in `x/bank` to query for all account holders of a specific denomination. * (bank) [\#9618](https://github.com/cosmos/cosmos-sdk/pull/9618) Update bank.Metadata: add URI and URIHash attributes. * (store) [\#8664](https://github.com/cosmos/cosmos-sdk/pull/8664) Implementation of ADR-038 file StreamingService -* [\#9837](https://github.com/cosmos/cosmos-sdk/issues/9837) `--generate-only` flag will accept the keyname now. +* [\#9837](https://github.com/cosmos/cosmos-sdk/issues/9837) `--generate-only` flag can be used with a keyname from the keyring. * [\#10326](https://github.com/cosmos/cosmos-sdk/pull/10326) `x/authz` add all grants by granter query. * [\#10944](https://github.com/cosmos/cosmos-sdk/pull/10944) `x/authz` add all grants by grantee query * [\#10348](https://github.com/cosmos/cosmos-sdk/pull/10348) Add `fee.{payer,granter}` and `tip` fields to StdSignDoc for signing tipped transactions. diff --git a/client/cmd.go b/client/cmd.go index 6f9f2d2d6824..ce6faba09e06 100644 --- a/client/cmd.go +++ b/client/cmd.go @@ -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.Simulate) + fromAddr, fromName, keyType, err := GetFromFields(clientCtx, clientCtx.Keyring, from) if err != nil { return clientCtx, err } diff --git a/client/context.go b/client/context.go index d653dc519ab7..12f20f449853 100644 --- a/client/context.go +++ b/client/context.go @@ -336,23 +336,29 @@ 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 simulate is true a new temporary address will be generated -func GetFromFields(kr keyring.Keyring, from string, simulate bool) (sdk.AccAddress, string, keyring.KeyType, error) { +// 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 simulate { - addr, err := sdk.AccAddressFromBech32(from) + 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 @@ -364,7 +370,7 @@ func GetFromFields(kr keyring.Keyring, from string, simulate bool) (sdk.AccAddre } } - addr, err := k.GetAddress() + addr, err = k.GetAddress() if err != nil { return nil, "", 0, err } diff --git a/client/context_test.go b/client/context_test.go index c9d553753a06..0efae1e63466 100644 --- a/client/context_test.go +++ b/client/context_test.go @@ -122,11 +122,10 @@ func TestGetFromFields(t *testing.T) { path := hd.CreateHDPath(118, 0, 0).String() testCases := []struct { + clientCtx client.Context keyring func() keyring.Keyring - name string - addr string + from string expectedErr string - simulate bool }{ { keyring: func() keyring.Keyring { @@ -137,26 +136,26 @@ func TestGetFromFields(t *testing.T) { return kb }, - name: "alice", - }, - { - keyring: func() keyring.Keyring { - return keyring.NewInMemory(cfg.Codec) - }, - addr: "cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5", - expectedErr: "key with address cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5 not found: key not found", + 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("bob", keyring.English, path, keyring.DefaultBIP39Passphrase, hd.Secp256k1) + _, _, err = kb.NewMnemonic("alice", keyring.English, path, keyring.DefaultBIP39Passphrase, hd.Secp256k1) require.NoError(t, err) return kb }, - name: "bob", + 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 { @@ -164,33 +163,56 @@ func TestGetFromFields(t *testing.T) { require.NoError(t, err) return kb }, - name: "bob", - expectedErr: "bob.info: key not found", + from: "alice", + expectedErr: "alice.info: key not found", }, { keyring: func() keyring.Keyring { return keyring.NewInMemory(cfg.Codec) }, - addr: "cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5", - simulate: true, + from: "cosmos139f7kncmglres2nf3h4hc4tade85ekfr8sulz5", + clientCtx: client.Context{}.WithSimulation(true), }, { keyring: func() keyring.Keyring { return keyring.NewInMemory(cfg.Codec) }, - addr: "alice", - simulate: true, + 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 { - var val = tc.name - if val == "" { - val = tc.addr - } - - _, _, _, err := client.GetFromFields(tc.keyring(), val, tc.simulate) + _, _, _, err := client.GetFromFields(tc.clientCtx, tc.keyring(), tc.from) if tc.expectedErr == "" { require.NoError(t, err) } else { diff --git a/client/flags/flags.go b/client/flags/flags.go index 0e90fcce845a..5cf663446542 100644 --- a/client/flags/flags.go +++ b/client/flags/flags.go @@ -113,7 +113,7 @@ func AddTxFlagsToCmd(cmd *cobra.Command) { 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 (when enabled, the local Keybase is not accessible)") - cmd.Flags().Bool(FlagGenerateOnly, false, "Build an unsigned transaction and write it to STDOUT") + 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)") diff --git a/client/tx/tx.go b/client/tx/tx.go index b90da0b84be0..29a5483e471f 100644 --- a/client/tx/tx.go +++ b/client/tx/tx.go @@ -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 } diff --git a/x/auth/client/cli/tx_sign.go b/x/auth/client/cli/tx_sign.go index b5832ef833dd..2831fea729d7 100644 --- a/x/auth/client/cli/tx_sign.go +++ b/x/auth/client/cli/tx_sign.go @@ -107,7 +107,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.Simulate) + _, fromName, _, err := client.GetFromFields(clientCtx, txFactory.Keybase(), from) if err != nil { return fmt.Errorf("error getting account from keybase: %w", err) } @@ -116,7 +116,7 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error { return err } } else { - multisigAddr, _, _, err := client.GetFromFields(txFactory.Keybase(), ms, clientCtx.Simulate) + multisigAddr, _, _, err := client.GetFromFields(clientCtx, txFactory.Keybase(), ms) if err != nil { return fmt.Errorf("error getting account from keybase: %w", err) } @@ -237,7 +237,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.Simulate) + _, fromName, _, err := client.GetFromFields(clientCtx, txF.Keybase(), from) if err != nil { return fmt.Errorf("error getting account from keybase: %w", err) } @@ -247,7 +247,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.Simulate) + multisigAddr, _, _, err = client.GetFromFields(clientCtx, txFactory.Keybase(), multisig) if err != nil { return fmt.Errorf("error getting account from keybase: %w", err) } diff --git a/x/feegrant/client/testutil/suite.go b/x/feegrant/client/testutil/suite.go index 7d67c31bff78..5423a18e9503 100644 --- a/x/feegrant/client/testutil/suite.go +++ b/x/feegrant/client/testutil/suite.go @@ -316,7 +316,7 @@ func (s *IntegrationTestSuite) TestNewCmdFeeGrant() { alreadyExistedGrantee := s.addedGrantee clientCtx := val.ClientCtx - fromAddr, fromName, _, err := client.GetFromFields(clientCtx.Keyring, granter.String(), clientCtx.Simulate) + fromAddr, fromName, _, err := client.GetFromFields(clientCtx, clientCtx.Keyring, granter.String()) s.Require().Equal(fromAddr, granter) s.Require().NoError(err) From 10bf7cacc182dc546e0556cafdd8d51b0288ed00 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 11 Apr 2022 17:09:57 +0200 Subject: [PATCH 7/7] spacing Co-authored-by: Aleksandr Bezobchuk --- client/context.go | 1 + 1 file changed, 1 insertion(+) diff --git a/client/context.go b/client/context.go index 12f20f449853..1d5925b933fa 100644 --- a/client/context.go +++ b/client/context.go @@ -351,6 +351,7 @@ func GetFromFields(clientCtx Context, kr keyring.Keyring, from string) (sdk.AccA } return addr, "", 0, nil + case clientCtx.GenerateOnly: if err == nil { return addr, "", 0, nil