From 1eb27053520a148031f522a000229e769c6328d6 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Fri, 7 Oct 2022 17:12:28 +0200 Subject: [PATCH] feat: let `sign-batch` read multiple files (#13454) * feat: let `sign-batch` read multiple files * add changelog * fix gosec issue * simplify --- CHANGELOG.md | 1 + x/auth/client/cli/tx_multisign.go | 20 ++---- x/auth/client/cli/tx_sign.go | 103 +++++++++++++++--------------- x/auth/client/tx.go | 30 ++++++++- 4 files changed, 87 insertions(+), 67 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23933e95cdaf6..769c9a67175c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (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`. * (x/consensus) [#12905](https://github.com/cosmos/cosmos-sdk/pull/12905) Create a new `x/consensus` module that is now responsible for maintaining Tendermint consensus parameters instead of `x/param`. Legacy types remain in order to facilitate parameter migration from the deprecated `x/params`. App developers should ensure that they execute `baseapp.MigrateParams` during their chain upgrade. These legacy types will be removed in a future release. +* (cli) [#13454](https://github.com/cosmos/cosmos-sdk/pull/13454) `sign-batch` CLI can now read multiple transaction files. ### Improvements diff --git a/x/auth/client/cli/tx_multisign.go b/x/auth/client/cli/tx_multisign.go index 303defb6dbdb6..3f0ca9fcb4d73 100644 --- a/x/auth/client/cli/tx_multisign.go +++ b/x/auth/client/cli/tx_multisign.go @@ -29,7 +29,7 @@ type BroadcastReq struct { Mode string `json:"mode" yaml:"mode"` } -// GetSignCommand returns the sign command +// GetMultiSignCommand returns the multi-sign command func GetMultiSignCommand() *cobra.Command { cmd := &cobra.Command{ Use: "multi-sign [file] [name] [[signature]...]", @@ -258,21 +258,11 @@ func makeBatchMultisignCmd() func(cmd *cobra.Command, args []string) error { txFactory = txFactory.WithSignMode(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON) } - infile := os.Stdin - if args[0] != "-" { - infile, err = os.Open(args[0]) - defer func() { - err2 := infile.Close() - if err == nil { - err = err2 - } - }() - - if err != nil { - return fmt.Errorf("couldn't open %s: %w", args[0], err) - } + // reads tx from args[0] + scanner, err := authclient.ReadTxsFromInput(txCfg, args[0]) + if err != nil { + return err } - scanner := authclient.NewBatchScanner(txCfg, infile) k, err := getMultisigRecord(clientCtx, args[1]) if err != nil { diff --git a/x/auth/client/cli/tx_sign.go b/x/auth/client/cli/tx_sign.go index ebc2ffc4adb31..30132ec14fd3f 100644 --- a/x/auth/client/cli/tx_sign.go +++ b/x/auth/client/cli/tx_sign.go @@ -27,11 +27,11 @@ const ( // GetSignBatchCommand returns the transaction sign-batch command. func GetSignBatchCommand() *cobra.Command { cmd := &cobra.Command{ - Use: "sign-batch [file]", + Use: "sign-batch [file] ([file2]...)", Short: "Sign transaction batch files", Long: `Sign batch files of transactions generated with --generate-only. -The command processes list of transactions from file (one StdTx each line), generate -signed transactions or signatures and print their JSON encoding, delimited by '\n'. +The command processes list of transactions from a file (one StdTx each line), or multiple files. +Then generates signed transactions or signatures and print their JSON encoding, delimited by '\n'. As the signatures are generated, the command updates the account and sequence number accordingly. If the --signature-only flag is set, it will output the signature parts only. @@ -50,7 +50,7 @@ account key. It implies --signature-only. `, PreRun: preSignCmd, RunE: makeSignBatchCmd(), - Args: cobra.ExactArgs(1), + Args: cobra.MinimumNArgs(1), } cmd.Flags().String(flagMultisig, "", "Address or key name of the multisig account on behalf of which the transaction shall be signed") @@ -74,7 +74,6 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error { txFactory := tx.NewFactoryCLI(clientCtx, cmd.Flags()) txCfg := clientCtx.TxConfig printSignatureOnly, _ := cmd.Flags().GetBool(flagSigOnly) - infile := os.Stdin ms, err := cmd.Flags().GetString(flagMultisig) if err != nil { @@ -86,17 +85,14 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error { if err != nil { return err } - defer closeFunc() clientCtx.WithOutput(cmd.OutOrStdout()) - if args[0] != "-" { - infile, err = os.Open(args[0]) - if err != nil { - return err - } + // reads tx from args + scanner, err := authclient.ReadTxsFromInput(txCfg, args...) + if err != nil { + return err } - scanner := authclient.NewBatchScanner(txCfg, infile) if !clientCtx.Offline { if ms == "" { @@ -121,9 +117,9 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error { } } - appendMessagesToSingleMsg, _ := cmd.Flags().GetBool(flagAppend) - if appendMessagesToSingleMsg { - // It will combine all tx msgs and create single signed transaction + appendMessagesToSingleTx, _ := cmd.Flags().GetBool(flagAppend) + // Combines all tx msgs and create single signed transaction + if appendMessagesToSingleTx { txBuilder := clientCtx.TxConfig.NewTxBuilder() msgs := make([]sdk.Msg, 0) newGasLimit := uint64(0) @@ -151,39 +147,24 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error { // set the gasLimit txBuilder.SetGasLimit(newGasLimit) + // sign the txs 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 { + if err := sign(clientCtx, txBuilder, txFactory, from); 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 { + if err := multisigSign(clientCtx, txBuilder, txFactory, ms); 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) - } else { // It will generate signed tx for each tx for sequence := txFactory.Sequence(); scanner.Scan(); sequence++ { @@ -193,37 +174,23 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error { if err != nil { return err } + + // sign the txs 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 { + if err := sign(clientCtx, txBuilder, txFactory, from); 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 { + if err := multisigSign(clientCtx, txBuilder, txFactory, ms); 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) } } @@ -236,6 +203,40 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error { } } +func sign(clientCtx client.Context, txBuilder client.TxBuilder, txFactory tx.Factory, from string) error { + _, fromName, _, err := client.GetFromFields(clientCtx, txFactory.Keybase(), from) + if err != nil { + return fmt.Errorf("error getting account from keybase: %w", err) + } + + if err = authclient.SignTx(txFactory, clientCtx, fromName, txBuilder, true, true); err != nil { + return err + } + + return nil +} + +func multisigSign(clientCtx client.Context, txBuilder client.TxBuilder, txFactory tx.Factory, multisig string) error { + multisigAddr, _, _, err := client.GetFromFields(clientCtx, txFactory.Keybase(), multisig) + if err != nil { + return fmt.Errorf("error getting account from keybase: %w", err) + } + + if err = authclient.SignTxWithSignerAddress( + txFactory, + clientCtx, + multisigAddr, + clientCtx.GetFromName(), + txBuilder, + clientCtx.Offline, + true, + ); err != nil { + return err + } + + return nil +} + func setOutputFile(cmd *cobra.Command) (func(), error) { outputDoc, _ := cmd.Flags().GetString(flags.FlagOutputDocument) if outputDoc == "" { diff --git a/x/auth/client/tx.go b/x/auth/client/tx.go index f3f5e1b9f7232..740f401b4c1b6 100644 --- a/x/auth/client/tx.go +++ b/x/auth/client/tx.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "os" + "path/filepath" "strings" "github.com/cosmos/gogoproto/jsonpb" @@ -90,7 +91,7 @@ func SignTxWithSignerAddress(txFactory tx.Factory, clientCtx client.Context, add return tx.Sign(txFactory, name, txBuilder, overwrite) } -// Read and decode a StdTx from the given filename. Can pass "-" to read from stdin. +// Read and decode a StdTx from the given filename. Can pass "-" to read from stdin. func ReadTxFromFile(ctx client.Context, filename string) (tx sdk.Tx, err error) { var bytes []byte @@ -107,6 +108,33 @@ func ReadTxFromFile(ctx client.Context, filename string) (tx sdk.Tx, err error) return ctx.TxConfig.TxJSONDecoder()(bytes) } +// ReadTxsFromInput reads multiples txs from the given filename(s). Can pass "-" to read from stdin. +// Unlike ReadTxFromFile, this function does not decode the txs. +func ReadTxsFromInput(txCfg client.TxConfig, filenames ...string) (scanner *BatchScanner, err error) { + if len(filenames) == 0 { + return nil, fmt.Errorf("no file name provided") + } + + var infile io.Reader = os.Stdin + if filenames[0] != "-" { + buf := new(bytes.Buffer) + for _, f := range filenames { + bytes, err := os.ReadFile(filepath.Clean(f)) + if err != nil { + return nil, fmt.Errorf("couldn't read %s: %w", f, err) + } + + if _, err := buf.WriteString(string(bytes)); err != nil { + return nil, fmt.Errorf("couldn't write to merged file: %w", err) + } + } + + infile = buf + } + + return NewBatchScanner(txCfg, infile), nil +} + // NewBatchScanner returns a new BatchScanner to read newline-delimited StdTx transactions from r. func NewBatchScanner(cfg client.TxConfig, r io.Reader) *BatchScanner { return &BatchScanner{Scanner: bufio.NewScanner(r), cfg: cfg}