From c7afd2c30d0f8b63a9fa5901e5db11c65d38de5b Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Mon, 21 Jun 2021 15:43:44 +0530 Subject: [PATCH 1/6] accept key names for multisig --- x/auth/client/cli/tx_sign.go | 40 +++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/x/auth/client/cli/tx_sign.go b/x/auth/client/cli/tx_sign.go index 3e8c0599faa0..5a274784f1fd 100644 --- a/x/auth/client/cli/tx_sign.go +++ b/x/auth/client/cli/tx_sign.go @@ -9,7 +9,6 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/client/tx" - sdk "github.com/cosmos/cosmos-sdk/types" authclient "github.com/cosmos/cosmos-sdk/x/auth/client" "github.com/cosmos/cosmos-sdk/x/auth/client/rest" ) @@ -48,7 +47,7 @@ account key. It implies --signature-only. Args: cobra.ExactArgs(1), } - cmd.Flags().String(flagMultisig, "", "Address of the multisig account on behalf of which the transaction shall be signed") + cmd.Flags().String(flagMultisig, "", "Address or Key of the multisig account on behalf of which the transaction shall be signed") 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") @@ -68,14 +67,10 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error { txCfg := clientCtx.TxConfig printSignatureOnly, _ := cmd.Flags().GetBool(flagSigOnly) infile := os.Stdin - var multisigAddr sdk.AccAddress - // validate multisig address if there's any - if ms, _ := cmd.Flags().GetString(flagMultisig); ms != "" { - multisigAddr, err = sdk.AccAddressFromBech32(ms) - if err != nil { - return err - } + ms, err := cmd.Flags().GetString(flagMultisig) + if err != nil { + return err } // prepare output document @@ -102,7 +97,7 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error { if err != nil { return err } - if multisigAddr.Empty() { + if ms == "" { from, _ := cmd.Flags().GetString(flags.FlagFrom) _, fromName, _, err := client.GetFromFields(txFactory.Keybase(), from, clientCtx.GenerateOnly) if err != nil { @@ -113,8 +108,12 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error { return err } } else { + multisigAddr, multisigName, _, err := client.GetFromFields(txFactory.Keybase(), ms, clientCtx.GenerateOnly) + if err != nil { + return fmt.Errorf("error getting account from keybase: %w", err) + } err = authclient.SignTxWithSignerAddress( - txFactory, clientCtx, multisigAddr, clientCtx.GetFromName(), txBuilder, clientCtx.Offline, true) + txFactory, clientCtx, multisigAddr, multisigName, txBuilder, clientCtx.Offline, true) } if err != nil { @@ -177,7 +176,7 @@ be generated via the 'multisign' command. Args: cobra.ExactArgs(1), } - cmd.Flags().String(flagMultisig, "", "Address of the multisig account on behalf of which the transaction shall be signed") + cmd.Flags().String(flagMultisig, "", "Address or Key of the multisig account on behalf of which the transaction shall be signed") cmd.Flags().Bool(flagOverwrite, false, "Overwrite existing signatures with a new one. If disabled, new signature will be appended") cmd.Flags().Bool(flagSigOnly, false, "Print only the signatures") cmd.Flags().String(flags.FlagOutputDocument, "", "The document will be written to the given file instead of STDOUT") @@ -213,6 +212,7 @@ func makeSignCmd() func(cmd *cobra.Command, args []string) error { return err } + txFactory := tx.NewFactoryCLI(clientCtx, cmd.Flags()) txCfg := clientCtx.TxConfig txBuilder, err := txCfg.WrapTxBuilder(newTx) if err != nil { @@ -220,7 +220,10 @@ func makeSignCmd() func(cmd *cobra.Command, args []string) error { } printSignatureOnly, _ := cmd.Flags().GetBool(flagSigOnly) - multisigAddrStr, _ := cmd.Flags().GetString(flagMultisig) + multisig, _ := cmd.Flags().GetString(flagMultisig) + if err != nil { + return err + } from, _ := cmd.Flags().GetString(flags.FlagFrom) _, fromName, _, err := client.GetFromFields(txF.Keybase(), from, clientCtx.GenerateOnly) if err != nil { @@ -228,17 +231,16 @@ func makeSignCmd() func(cmd *cobra.Command, args []string) error { } overwrite, _ := f.GetBool(flagOverwrite) - if multisigAddrStr != "" { - var multisigAddr sdk.AccAddress - multisigAddr, err = sdk.AccAddressFromBech32(multisigAddrStr) + if multisig != "" { + multisigAddr, multisigName, _, err := client.GetFromFields(txFactory.Keybase(), multisig, clientCtx.GenerateOnly) if err != nil { - return err + return fmt.Errorf("error getting account from keybase: %w", err) } err = authclient.SignTxWithSignerAddress( - txF, clientCtx, multisigAddr, fromName, txBuilder, clientCtx.Offline, overwrite) + txF, clientCtx, multisigAddr, multisigName, txBuilder, clientCtx.Offline, overwrite) printSignatureOnly = true } else { - err = authclient.SignTx(txF, clientCtx, clientCtx.GetFromName(), txBuilder, clientCtx.Offline, overwrite) + err = authclient.SignTx(txF, clientCtx, fromName, txBuilder, clientCtx.Offline, overwrite) } if err != nil { return err From 74313e68578f6d313af29854cb9469e3af15f2b3 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Mon, 21 Jun 2021 16:16:10 +0530 Subject: [PATCH 2/6] fix failing tests --- x/auth/client/cli/tx_sign.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/x/auth/client/cli/tx_sign.go b/x/auth/client/cli/tx_sign.go index 5a274784f1fd..c398e0ec34da 100644 --- a/x/auth/client/cli/tx_sign.go +++ b/x/auth/client/cli/tx_sign.go @@ -108,12 +108,15 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error { return err } } else { - multisigAddr, multisigName, _, err := client.GetFromFields(txFactory.Keybase(), ms, clientCtx.GenerateOnly) + multisigAddr, _, _, err := client.GetFromFields(txFactory.Keybase(), ms, clientCtx.GenerateOnly) if err != nil { return fmt.Errorf("error getting account from keybase: %w", err) } err = authclient.SignTxWithSignerAddress( - txFactory, clientCtx, multisigAddr, multisigName, txBuilder, clientCtx.Offline, true) + txFactory, clientCtx, multisigAddr, clientCtx.GetFromName(), txBuilder, clientCtx.Offline, true) + if err != nil { + return err + } } if err != nil { @@ -232,15 +235,18 @@ func makeSignCmd() func(cmd *cobra.Command, args []string) error { overwrite, _ := f.GetBool(flagOverwrite) if multisig != "" { - multisigAddr, multisigName, _, err := client.GetFromFields(txFactory.Keybase(), multisig, clientCtx.GenerateOnly) + multisigAddr, _, _, err := client.GetFromFields(txFactory.Keybase(), multisig, clientCtx.GenerateOnly) if err != nil { return fmt.Errorf("error getting account from keybase: %w", err) } err = authclient.SignTxWithSignerAddress( - txF, clientCtx, multisigAddr, multisigName, txBuilder, clientCtx.Offline, overwrite) + txF, clientCtx, multisigAddr, fromName, txBuilder, clientCtx.Offline, overwrite) + if err != nil { + return err + } printSignatureOnly = true } else { - err = authclient.SignTx(txF, clientCtx, fromName, txBuilder, clientCtx.Offline, overwrite) + err = authclient.SignTx(txF, clientCtx, clientCtx.GetFromName(), txBuilder, clientCtx.Offline, overwrite) } if err != nil { return err From 9b1d26c1b8b1dee743fbeb8ec24d845ddce67954 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Mon, 21 Jun 2021 18:41:58 +0530 Subject: [PATCH 3/6] add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5fc142f4bd4..8bcd33b29189 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -133,6 +133,7 @@ if input key is empty, or input data contains empty key. ### Improvements +* (x/auth) [\#9553] (https://github.com/cosmos/cosmos-sdk/pull/9553) `multisig` flag now accepts both name and address. * (gRPC-Web) [\#9493](https://github.com/cosmos/cosmos-sdk/pull/9493) Add `EnableUnsafeCORS` flag to grpc-web config. * (x/params) [\#9481](https://github.com/cosmos/cosmos-sdk/issues/9481) Speedup simulator for parameter change proposals. * (store) [\#9403](https://github.com/cosmos/cosmos-sdk/pull/9403) Add `RefundGas` function to `GasMeter` interface From 4adef3a22e7fd9813e13fed185b051256327970d Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Tue, 22 Jun 2021 18:27:02 +0530 Subject: [PATCH 4/6] address review cooments --- CHANGELOG.md | 2 +- x/auth/client/cli/tx_sign.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index af6e9d620fe1..80fcc8ef0a6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -129,7 +129,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements -* (x/auth) [\#9553] (https://github.com/cosmos/cosmos-sdk/pull/9553) `multisig` flag now accepts both name and address. +* (x/auth) [\#9553] (https://github.com/cosmos/cosmos-sdk/pull/9553) The `--multisig` flag now accepts both a name and address. * (gRPC-Web) [\#9493](https://github.com/cosmos/cosmos-sdk/pull/9493) Add `EnableUnsafeCORS` flag to grpc-web config. * (x/params) [\#9481](https://github.com/cosmos/cosmos-sdk/issues/9481) Speedup simulator for parameter change proposals. * (x/staking) [\#9423](https://github.com/cosmos/cosmos-sdk/pull/9423) Staking delegations now returns empty list instead of rpc error when no records found. diff --git a/x/auth/client/cli/tx_sign.go b/x/auth/client/cli/tx_sign.go index c398e0ec34da..833c034758f5 100644 --- a/x/auth/client/cli/tx_sign.go +++ b/x/auth/client/cli/tx_sign.go @@ -47,7 +47,7 @@ account key. It implies --signature-only. Args: cobra.ExactArgs(1), } - cmd.Flags().String(flagMultisig, "", "Address or Key of the multisig account on behalf of which the transaction shall be signed") + cmd.Flags().String(flagMultisig, "", "Address or key name of the multisig account on behalf of which the transaction shall be signed") 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") @@ -179,7 +179,7 @@ be generated via the 'multisign' command. Args: cobra.ExactArgs(1), } - cmd.Flags().String(flagMultisig, "", "Address or Key of the multisig account on behalf of which the transaction shall be signed") + cmd.Flags().String(flagMultisig, "", "Address or key name of the multisig account on behalf of which the transaction shall be signed") cmd.Flags().Bool(flagOverwrite, false, "Overwrite existing signatures with a new one. If disabled, new signature will be appended") cmd.Flags().Bool(flagSigOnly, false, "Print only the signatures") cmd.Flags().String(flags.FlagOutputDocument, "", "The document will be written to the given file instead of STDOUT") From 99f988b9138cf7eff598979bd5e769f93dff79d0 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Wed, 23 Jun 2021 15:15:21 +0530 Subject: [PATCH 5/6] add tests for multisig key name --- x/auth/client/testutil/suite.go | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/x/auth/client/testutil/suite.go b/x/auth/client/testutil/suite.go index 9456260c89ef..96157c07ebcc 100644 --- a/x/auth/client/testutil/suite.go +++ b/x/auth/client/testutil/suite.go @@ -129,7 +129,7 @@ func (s *IntegrationTestSuite) TestCLISignBatch() { s.Require().NoError(err) s.Require().Equal(3, len(strings.Split(strings.Trim(res.String(), "\n"), "\n"))) - // sign-batch file + // sign-batch file signature only res, err = TxSignBatchExec(val.ClientCtx, val.Address, outputFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--signature-only") s.Require().NoError(err) s.Require().Equal(3, len(strings.Split(strings.Trim(res.String(), "\n"), "\n"))) @@ -756,11 +756,19 @@ func (s *IntegrationTestSuite) TestSignBatchMultisig() { res, err = TxSignBatchExec(val.ClientCtx, account2.GetAddress(), filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--multisig", multisigInfo.GetAddress().String()) s.Require().NoError(err) s.Require().Equal(1, len(strings.Split(strings.Trim(res.String(), "\n"), "\n"))) - // write sigs to file2 file2 := testutil.WriteToNewTempFile(s.T(), res.String()) - _, err = TxMultiSignExec(val.ClientCtx, multisigInfo.GetName(), filename.Name(), file1.Name(), file2.Name()) + + //sign-batch file with multisig key name + res, err = TxSignBatchExec(val.ClientCtx, account1.GetAddress(), filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--multisig", multisigInfo.GetName()) + s.Require().NoError(err) + s.Require().Equal(1, len(strings.Split(strings.Trim(res.String(), "\n"), "\n"))) + // write sigs to file3 + file3 := testutil.WriteToNewTempFile(s.T(), res.String()) + + _, err = TxMultiSignExec(val.ClientCtx, multisigInfo.GetName(), filename.Name(), file1.Name(), file2.Name(), file3.Name()) s.Require().NoError(err) + } func (s *IntegrationTestSuite) TestMultisignBatch() { @@ -821,7 +829,15 @@ func (s *IntegrationTestSuite) TestMultisignBatch() { // multisign the file file2 := testutil.WriteToNewTempFile(s.T(), res.String()) - res, err = TxMultiSignBatchExec(val.ClientCtx, filename.Name(), multisigInfo.GetName(), file1.Name(), file2.Name()) + + //sign-batch file with multisig key name + res, err = TxSignBatchExec(val.ClientCtx, account1.GetAddress(), filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--multisig", multisigInfo.GetName(), fmt.Sprintf("--%s", flags.FlagOffline), fmt.Sprintf("--%s=%s", flags.FlagAccountNumber, fmt.Sprint(account.GetAccountNumber())), fmt.Sprintf("--%s=%s", flags.FlagSequence, fmt.Sprint(account.GetSequence()))) + s.Require().NoError(err) + s.Require().Equal(3, len(strings.Split(strings.Trim(res.String(), "\n"), "\n"))) + // write sigs to file + file3 := testutil.WriteToNewTempFile(s.T(), res.String()) + + res, err = TxMultiSignBatchExec(val.ClientCtx, filename.Name(), multisigInfo.GetName(), file1.Name(), file2.Name(), file3.Name()) s.Require().NoError(err) signedTxs := strings.Split(strings.Trim(res.String(), "\n"), "\n") From 4cc6afa7bcb4dfb82668db7706a9012316750cc1 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Wed, 23 Jun 2021 15:25:47 +0530 Subject: [PATCH 6/6] fix lint issues --- x/auth/client/testutil/suite.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/auth/client/testutil/suite.go b/x/auth/client/testutil/suite.go index 96157c07ebcc..773f8dba1ae9 100644 --- a/x/auth/client/testutil/suite.go +++ b/x/auth/client/testutil/suite.go @@ -759,7 +759,7 @@ func (s *IntegrationTestSuite) TestSignBatchMultisig() { // write sigs to file2 file2 := testutil.WriteToNewTempFile(s.T(), res.String()) - //sign-batch file with multisig key name + // sign-batch file with multisig key name res, err = TxSignBatchExec(val.ClientCtx, account1.GetAddress(), filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--multisig", multisigInfo.GetName()) s.Require().NoError(err) s.Require().Equal(1, len(strings.Split(strings.Trim(res.String(), "\n"), "\n"))) @@ -830,7 +830,7 @@ func (s *IntegrationTestSuite) TestMultisignBatch() { // multisign the file file2 := testutil.WriteToNewTempFile(s.T(), res.String()) - //sign-batch file with multisig key name + // sign-batch file with multisig key name res, err = TxSignBatchExec(val.ClientCtx, account1.GetAddress(), filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--multisig", multisigInfo.GetName(), fmt.Sprintf("--%s", flags.FlagOffline), fmt.Sprintf("--%s=%s", flags.FlagAccountNumber, fmt.Sprint(account.GetAccountNumber())), fmt.Sprintf("--%s=%s", flags.FlagSequence, fmt.Sprint(account.GetSequence()))) s.Require().NoError(err) s.Require().Equal(3, len(strings.Split(strings.Trim(res.String(), "\n"), "\n")))