From 5f960b72e2a0a8573bd32d0b1ed95141cc2bea2c Mon Sep 17 00:00:00 2001 From: atheeshp <59333759+atheeshp@users.noreply.github.com> Date: Mon, 9 Aug 2021 21:48:19 +0530 Subject: [PATCH] feat: `--generate-only` and `--offline` flags can use keyname. (#9838) ## Description Closes: #9837 ref: #9407 --- ### 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... - [ ] 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 - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] 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) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] 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) --- CHANGELOG.md | 1 + client/context.go | 10 --- x/auth/client/testutil/suite.go | 111 ++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ce1e54c087..6c971535d82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,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. * [\#9750](https://github.com/cosmos/cosmos-sdk/pull/9750) Emit events for tx signature and sequence, so clients can now query txs by signature (`tx.signature=''`) or by address and sequence combo (`tx.acc_seq='/'`). +* [\#9837](https://github.com/cosmos/cosmos-sdk/issues/9837) `--generate-only` flag will accept the keyname now. ### API Breaking Changes diff --git a/client/context.go b/client/context.go index 9f814517355..90987355381 100644 --- a/client/context.go +++ b/client/context.go @@ -11,7 +11,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" @@ -324,15 +323,6 @@ func GetFromFields(kr keyring.Keyring, from string, genOnly bool) (sdk.AccAddres return nil, "", 0, nil } - if genOnly { - addr, err := sdk.AccAddressFromBech32(from) - if err != nil { - return nil, "", 0, errors.Wrap(err, "must provide a valid Bech32 address in generate-only mode") - } - - return addr, "", 0, nil - } - var info keyring.Info if addr, err := sdk.AccAddressFromBech32(from); err == nil { info, err = kr.KeyByAddress(addr) diff --git a/x/auth/client/testutil/suite.go b/x/auth/client/testutil/suite.go index e744916b4ff..5980e441f0b 100644 --- a/x/auth/client/testutil/suite.go +++ b/x/auth/client/testutil/suite.go @@ -30,6 +30,7 @@ import ( "github.com/cosmos/cosmos-sdk/types/tx/signing" authcli "github.com/cosmos/cosmos-sdk/x/auth/client/cli" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + bank "github.com/cosmos/cosmos-sdk/x/bank/client/cli" bankcli "github.com/cosmos/cosmos-sdk/x/bank/client/testutil" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" ) @@ -108,6 +109,116 @@ func (s *IntegrationTestSuite) TestCLIValidateSignatures() { s.Require().EqualError(err, "signatures validation failed") } +func (s *IntegrationTestSuite) TestCLISignGenOnly() { + val := s.network.Validators[0] + val2 := s.network.Validators[1] + + info, err := val.ClientCtx.Keyring.KeyByAddress(val.Address) + s.Require().NoError(err) + keyName := info.GetName() + + account, err := val.ClientCtx.AccountRetriever.GetAccount(val.ClientCtx, info.GetAddress()) + s.Require().NoError(err) + + sendTokens := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10))) + args := []string{ + keyName, // from keyname + val2.Address.String(), + sendTokens.String(), + fmt.Sprintf("--%s=true", flags.FlagGenerateOnly), // shouldn't break if we use keyname with --generate-only flag + fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), + fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), + } + generatedStd, err := clitestutil.ExecTestCLICmd(val.ClientCtx, bank.NewSendTxCmd(), args) + s.Require().NoError(err) + opFile := testutil.WriteToNewTempFile(s.T(), generatedStd.String()) + + commonArgs := []string{ + fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendTest), + fmt.Sprintf("--%s=%s", flags.FlagHome, strings.Replace(val.ClientCtx.HomeDir, "simd", "simcli", 1)), + fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), + } + + cases := []struct { + name string + args []string + expErr bool + errMsg string + }{ + { + "offline mode with account-number, sequence and keyname (valid)", + []string{ + opFile.Name(), + fmt.Sprintf("--%s=true", flags.FlagOffline), + fmt.Sprintf("--%s=%s", flags.FlagFrom, keyName), + fmt.Sprintf("--%s=%d", flags.FlagAccountNumber, account.GetAccountNumber()), + fmt.Sprintf("--%s=%d", flags.FlagSequence, account.GetSequence()), + }, + false, + "", + }, + { + "offline mode with account-number, sequence and address key (valid)", + []string{ + opFile.Name(), + fmt.Sprintf("--%s=true", flags.FlagOffline), + fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()), + fmt.Sprintf("--%s=%d", flags.FlagAccountNumber, account.GetAccountNumber()), + fmt.Sprintf("--%s=%d", flags.FlagSequence, account.GetSequence()), + }, + false, + "", + }, + { + "offline mode without account-number and keyname (invalid)", + []string{ + opFile.Name(), + fmt.Sprintf("--%s=true", flags.FlagOffline), + fmt.Sprintf("--%s=%s", flags.FlagFrom, keyName), + fmt.Sprintf("--%s=%d", flags.FlagSequence, account.GetSequence()), + }, + true, + `required flag(s) "account-number" not set`, + }, + { + "offline mode without sequence and keyname (invalid)", + []string{ + opFile.Name(), + fmt.Sprintf("--%s=true", flags.FlagOffline), + fmt.Sprintf("--%s=%s", flags.FlagFrom, keyName), + fmt.Sprintf("--%s=%d", flags.FlagAccountNumber, account.GetAccountNumber()), + }, + true, + `required flag(s) "sequence" not set`, + }, + { + "offline mode without account-number, sequence and keyname (invalid)", + []string{ + opFile.Name(), + fmt.Sprintf("--%s=%s", flags.FlagFrom, keyName), + fmt.Sprintf("--%s=true", flags.FlagOffline), + }, + true, + `required flag(s) "account-number", "sequence" not set`, + }, + } + + for _, tc := range cases { + cmd := authcli.GetSignCommand() + tmcli.PrepareBaseCmd(cmd, "", "") + out, err := clitestutil.ExecTestCLICmd(val.ClientCtx, cmd, append(tc.args, commonArgs...)) + if tc.expErr { + s.Require().Error(err) + s.Require().Contains(err.Error(), tc.errMsg) + } else { + s.Require().NoError(err) + signedTx := testutil.WriteToNewTempFile(s.T(), out.String()) + _, err := TxBroadcastExec(val.ClientCtx, signedTx.Name()) + s.Require().NoError(err) + } + } +} + func (s *IntegrationTestSuite) TestCLISignBatch() { val := s.network.Validators[0] var sendTokens = sdk.NewCoins(