From 445a99f71037cf8cac04f598d7979e1eac058a5a Mon Sep 17 00:00:00 2001 From: Sai Kumar <17549398+gsk967@users.noreply.github.com> Date: Tue, 27 Sep 2022 04:01:58 +0530 Subject: [PATCH] feat(cli): add `--append` flag to `sign-batch` cli cmd (#13147) * feat(cli): add `multi-msg-sign` * chore: update the changelog * chore: fix the lint issue * chore: address the pr comments * chore: address the pr comments * refactor: refactored `sign-batch` to generate single signed tx * chore: updated the changelog * fix: fix the lint * test: fix the tests on auth Co-authored-by: Aleksandr Bezobchuk --- CHANGELOG.md | 1 + x/auth/client/cli/tx_sign.go | 276 ++++++++++++++++++++------------ x/auth/client/testutil/suite.go | 8 +- 3 files changed, 183 insertions(+), 102 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f314da52e52..d08dcb773d94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (ledger) [#12935](https://github.com/cosmos/cosmos-sdk/pull/12935) Generalize Ledger integration to allow for different apps or keytypes that use SECP256k1. * (x/bank) [#11981](https://github.com/cosmos/cosmos-sdk/pull/11981) Create the `SetSendEnabled` endpoint for managing the bank's SendEnabled settings. * (x/auth) [#13210](https://github.com/cosmos/cosmos-sdk/pull/13210) Add `Query/AccountInfo` endpoint for simplified access to basic account info. +* (cli) [#13147](https://github.com/cosmos/cosmos-sdk/pull/13147) Add the `--append` flag to the `sign-batch` CLI cmd to combine the messages and sign those txs which are created with `--generate-only`. ### Improvements diff --git a/x/auth/client/cli/tx_sign.go b/x/auth/client/cli/tx_sign.go index f6870c5eb3b4..ebc2ffc4adb3 100644 --- a/x/auth/client/cli/tx_sign.go +++ b/x/auth/client/cli/tx_sign.go @@ -10,6 +10,7 @@ import ( "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/client/tx" kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" + sdk "github.com/cosmos/cosmos-sdk/types" authclient "github.com/cosmos/cosmos-sdk/x/auth/client" ) @@ -20,6 +21,7 @@ const ( flagSigOnly = "signature-only" flagAmino = "amino" flagNoAutoIncrement = "no-auto-increment" + flagAppend = "append" ) // GetSignBatchCommand returns the transaction sign-batch command. @@ -53,7 +55,9 @@ account key. It implies --signature-only. 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().Bool(flagSigOnly, false, "Print only the generated signature, then exit") + cmd.Flags().Bool(flagAppend, false, "Combine all message and generate single signed transaction for broadcast.") + flags.AddTxFlagsToCmd(cmd) cmd.MarkFlagRequired(flags.FlagFrom) @@ -117,13 +121,36 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error { } } - for sequence := txFactory.Sequence(); scanner.Scan(); sequence++ { - unsignedStdTx := scanner.Tx() - txFactory = txFactory.WithSequence(sequence) - txBuilder, err := txCfg.WrapTxBuilder(unsignedStdTx) - if err != nil { - return err + appendMessagesToSingleMsg, _ := cmd.Flags().GetBool(flagAppend) + if appendMessagesToSingleMsg { + // It will combine all tx msgs and create single signed transaction + txBuilder := clientCtx.TxConfig.NewTxBuilder() + msgs := make([]sdk.Msg, 0) + newGasLimit := uint64(0) + + for scanner.Scan() { + unsignedStdTx := scanner.Tx() + fe, err := clientCtx.TxConfig.WrapTxBuilder(unsignedStdTx) + if err != nil { + return err + } + // increment the gas + newGasLimit += fe.GetTx().GetGas() + // append messages + msgs = append(msgs, unsignedStdTx.GetMsgs()...) } + // set the new appened msgs into builder + txBuilder.SetMsgs(msgs...) + + // set the memo,fees,feeGranter,feePayer from cmd flags + txBuilder.SetMemo(txFactory.Memo()) + txBuilder.SetFeeAmount(txFactory.Fees()) + txBuilder.SetFeeGranter(clientCtx.FeeGranter) + txBuilder.SetFeePayer(clientCtx.FeePayer) + + // set the gasLimit + txBuilder.SetGasLimit(newGasLimit) + if ms == "" { from, _ := cmd.Flags().GetString(flags.FlagFrom) _, fromName, _, err := client.GetFromFields(clientCtx, txFactory.Keybase(), from) @@ -156,6 +183,49 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error { } cmd.Printf("%s\n", json) + + } else { + // It will generate signed tx for each tx + for sequence := txFactory.Sequence(); scanner.Scan(); sequence++ { + unsignedStdTx := scanner.Tx() + txFactory = txFactory.WithSequence(sequence) + txBuilder, err := txCfg.WrapTxBuilder(unsignedStdTx) + if err != nil { + return err + } + if ms == "" { + from, _ := cmd.Flags().GetString(flags.FlagFrom) + _, fromName, _, err := client.GetFromFields(clientCtx, txFactory.Keybase(), from) + if err != nil { + return fmt.Errorf("error getting account from keybase: %w", err) + } + err = authclient.SignTx(txFactory, clientCtx, fromName, txBuilder, true, true) + if err != nil { + return err + } + } else { + multisigAddr, _, _, err := client.GetFromFields(clientCtx, txFactory.Keybase(), ms) + 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) + if err != nil { + return err + } + } + + if err != nil { + return err + } + + json, err := marshalSignatureJSON(txCfg, txBuilder, printSignatureOnly) + if err != nil { + return err + } + + cmd.Printf("%s\n", json) + } } if err := scanner.UnmarshalErr(); err != nil { @@ -235,129 +305,139 @@ func makeSignCmd() func(cmd *cobra.Command, args []string) error { if err != nil { return err } - f := cmd.Flags() clientCtx, txF, newTx, err := readTxAndInitContexts(clientCtx, cmd, args[0]) if err != nil { return err } - txCfg := clientCtx.TxConfig - txBuilder, err := txCfg.WrapTxBuilder(newTx) + return signTx(cmd, clientCtx, txF, newTx) + } +} + +func signTx(cmd *cobra.Command, clientCtx client.Context, txF tx.Factory, newTx sdk.Tx) error { + f := cmd.Flags() + txCfg := clientCtx.TxConfig + txBuilder, err := txCfg.WrapTxBuilder(newTx) + if err != nil { + return err + } + + printSignatureOnly, err := cmd.Flags().GetBool(flagSigOnly) + if err != nil { + return err + } + + multisig, err := cmd.Flags().GetString(flagMultisig) + if err != nil { + return err + } + + from, err := cmd.Flags().GetString(flags.FlagFrom) + if err != nil { + return err + } + + _, fromName, _, err := client.GetFromFields(clientCtx, txF.Keybase(), from) + if err != nil { + return fmt.Errorf("error getting account from keybase: %w", err) + } + + overwrite, err := f.GetBool(flagOverwrite) + if err != nil { + return err + } + + if multisig != "" { + // Bech32 decode error, maybe it's a name, we try to fetch from keyring + multisigAddr, multisigName, _, err := client.GetFromFields(clientCtx, txF.Keybase(), multisig) if err != nil { - return err + return fmt.Errorf("error getting account from keybase: %w", err) } - - printSignatureOnly, _ := cmd.Flags().GetBool(flagSigOnly) - multisig, _ := cmd.Flags().GetString(flagMultisig) + multisigkey, err := getMultisigRecord(clientCtx, multisigName) if err != nil { return err } - from, _ := cmd.Flags().GetString(flags.FlagFrom) - _, fromName, _, err := client.GetFromFields(clientCtx, txF.Keybase(), from) + multisigPubKey, err := multisigkey.GetPubKey() if err != nil { - return fmt.Errorf("error getting account from keybase: %w", err) + return err } + multisigLegacyPub := multisigPubKey.(*kmultisig.LegacyAminoPubKey) - 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(clientCtx, txF.Keybase(), multisig) - if err != nil { - return fmt.Errorf("error getting account from keybase: %w", err) - } - multisigkey, err := getMultisigRecord(clientCtx, multisigName) - if err != nil { - return err - } - multisigPubKey, err := multisigkey.GetPubKey() - if err != nil { - return err - } - multisigLegacyPub := multisigPubKey.(*kmultisig.LegacyAminoPubKey) - - fromRecord, err := clientCtx.Keyring.Key(fromName) - if err != nil { - return fmt.Errorf("error getting account from keybase: %w", err) - } - fromPubKey, err := fromRecord.GetPubKey() - if err != nil { - return err - } - - var found bool - for _, pubkey := range multisigLegacyPub.GetPubKeys() { - if pubkey.Equals(fromPubKey) { - found = true - } - } - if !found { - return fmt.Errorf("signing key is not a part of multisig key") - } - err = authclient.SignTxWithSignerAddress( - txF, clientCtx, multisigAddr, fromName, txBuilder, clientCtx.Offline, overwrite) - if err != nil { - return err - } - printSignatureOnly = true - } else { - err = authclient.SignTx(txF, clientCtx, clientCtx.GetFromName(), txBuilder, clientCtx.Offline, overwrite) + fromRecord, err := clientCtx.Keyring.Key(fromName) + if err != nil { + return fmt.Errorf("error getting account from keybase: %w", err) } + fromPubKey, err := fromRecord.GetPubKey() if err != nil { return err } - aminoJSON, err := f.GetBool(flagAmino) + var found bool + for _, pubkey := range multisigLegacyPub.GetPubKeys() { + if pubkey.Equals(fromPubKey) { + found = true + } + } + if !found { + return fmt.Errorf("signing key is not a part of multisig key") + } + err = authclient.SignTxWithSignerAddress( + txF, clientCtx, multisigAddr, fromName, txBuilder, clientCtx.Offline, overwrite) if err != nil { return err } + printSignatureOnly = true + } else { + err = authclient.SignTx(txF, clientCtx, clientCtx.GetFromName(), txBuilder, clientCtx.Offline, overwrite) + } + if err != nil { + return err + } + + aminoJSON, err := f.GetBool(flagAmino) + if err != nil { + return err + } - bMode, err := f.GetString(flags.FlagBroadcastMode) + bMode, err := f.GetString(flags.FlagBroadcastMode) + if err != nil { + return err + } + + // set output + closeFunc, err := setOutputFile(cmd) + if err != nil { + return err + } + + defer closeFunc() + clientCtx.WithOutput(cmd.OutOrStdout()) + + var json []byte + if aminoJSON { + stdTx, err := tx.ConvertTxToStdTx(clientCtx.LegacyAmino, txBuilder.GetTx()) if err != nil { return err } - - var json []byte - if aminoJSON { - stdTx, err := tx.ConvertTxToStdTx(clientCtx.LegacyAmino, txBuilder.GetTx()) - if err != nil { - return err - } - req := BroadcastReq{ - Tx: stdTx, - Mode: bMode, - } - json, err = clientCtx.LegacyAmino.MarshalJSON(req) - if err != nil { - return err - } - } else { - json, err = marshalSignatureJSON(txCfg, txBuilder, printSignatureOnly) - if err != nil { - return err - } + req := BroadcastReq{ + Tx: stdTx, + Mode: bMode, } - - outputDoc, _ := cmd.Flags().GetString(flags.FlagOutputDocument) - if outputDoc == "" { - cmd.Printf("%s\n", json) - return nil + json, err = clientCtx.LegacyAmino.MarshalJSON(req) + if err != nil { + return err } - - fp, err := os.OpenFile(outputDoc, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o644) + } else { + json, err = marshalSignatureJSON(txCfg, txBuilder, printSignatureOnly) if err != nil { return err } - defer func() { - err2 := fp.Close() - if err == nil { - err = err2 - } - }() - - _, err = fp.Write(append(json, '\n')) - return err } + + cmd.Printf("%s\n", json) + + return err } func marshalSignatureJSON(txConfig client.TxConfig, txBldr client.TxBuilder, signatureOnly bool) ([]byte, error) { diff --git a/x/auth/client/testutil/suite.go b/x/auth/client/testutil/suite.go index f262272f0df2..9ac15b7aa2de 100644 --- a/x/auth/client/testutil/suite.go +++ b/x/auth/client/testutil/suite.go @@ -1243,7 +1243,7 @@ func (s *IntegrationTestSuite) TestSignBatchMultisig() { addr1, err := account1.GetAddress() s.Require().NoError(err) // sign-batch file - res, err := TxSignBatchExec(val.ClientCtx, addr1, filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--multisig", addr.String()) + res, err := TxSignBatchExec(val.ClientCtx, addr1, filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--multisig", addr.String(), "--signature-only") s.Require().NoError(err) s.Require().Equal(1, len(strings.Split(strings.Trim(res.String(), "\n"), "\n"))) // write sigs to file @@ -1252,7 +1252,7 @@ func (s *IntegrationTestSuite) TestSignBatchMultisig() { addr2, err := account2.GetAddress() s.Require().NoError(err) // sign-batch file with account2 - res, err = TxSignBatchExec(val.ClientCtx, addr2, filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--multisig", addr.String()) + res, err = TxSignBatchExec(val.ClientCtx, addr2, filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--multisig", addr.String(), "--signature-only") s.Require().NoError(err) s.Require().Equal(1, len(strings.Split(strings.Trim(res.String(), "\n"), "\n"))) // write sigs to file2 @@ -1310,7 +1310,7 @@ func (s *IntegrationTestSuite) TestMultisignBatch() { // sign-batch file addr1, err := account1.GetAddress() s.Require().NoError(err) - res, err := TxSignBatchExec(val.ClientCtx, addr1, filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--multisig", addr.String(), 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()))) + res, err := TxSignBatchExec(val.ClientCtx, addr1, filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--multisig", addr.String(), 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())), "--signature-only") s.Require().NoError(err) s.Require().Equal(3, len(strings.Split(strings.Trim(res.String(), "\n"), "\n"))) // write sigs to file @@ -1319,7 +1319,7 @@ func (s *IntegrationTestSuite) TestMultisignBatch() { // sign-batch file with account2 addr2, err := account2.GetAddress() s.Require().NoError(err) - res, err = TxSignBatchExec(val.ClientCtx, addr2, filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--multisig", addr.String(), 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()))) + res, err = TxSignBatchExec(val.ClientCtx, addr2, filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--multisig", addr.String(), 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())), "--signature-only") s.Require().NoError(err) s.Require().Equal(3, len(strings.Split(strings.Trim(res.String(), "\n"), "\n")))