diff --git a/.pending/improvements/gaia/3893-Improve-gaiacli-tx-sign-command b/.pending/improvements/gaia/3893-Improve-gaiacli-tx-sign-command new file mode 100644 index 000000000..3d8b67791 --- /dev/null +++ b/.pending/improvements/gaia/3893-Improve-gaiacli-tx-sign-command @@ -0,0 +1,4 @@ +#3893 Improve `gaiacli tx sign` command + * Add shorthand flags -a and -s for the account and sequence numbers respectively + * Mark the account and sequence numbers required during "offline" mode + * Always do an RPC query for account and sequence number during "online" mode diff --git a/client/flags.go b/client/flags.go index 28e5cc7b9..4fd899131 100644 --- a/client/flags.go +++ b/client/flags.go @@ -81,8 +81,8 @@ func PostCommands(cmds ...*cobra.Command) []*cobra.Command { for _, c := range cmds { c.Flags().Bool(FlagIndentResponse, false, "Add indent to JSON response") c.Flags().String(FlagFrom, "", "Name or address of private key with which to sign") - c.Flags().Uint64(FlagAccountNumber, 0, "AccountNumber number to sign the tx") - c.Flags().Uint64(FlagSequence, 0, "Sequence number to sign the tx") + c.Flags().Uint64P(FlagAccountNumber, "a", 0, "The account number number of the signing account (offline mode only)") + c.Flags().Uint64P(FlagSequence, "s", 0, "The sequence number of the signing account (offline mode only)") c.Flags().String(FlagMemo, "", "Memo to send along with transaction") c.Flags().String(FlagFees, "", "Fees to pay along with transaction; eg: 10uatom") c.Flags().String(FlagGasPrices, "", "Gas prices to determine the transaction fee (e.g. 10uatom)") diff --git a/client/utils/utils.go b/client/utils/utils.go index 20867df07..6e8aec8b4 100644 --- a/client/utils/utils.go +++ b/client/utils/utils.go @@ -164,15 +164,17 @@ func PrintUnsignedStdTx( return } -// SignStdTx appends a signature to a StdTx and returns a copy of a it. If appendSig +// SignStdTx appends a signature to a StdTx and returns a copy of it. If appendSig // is false, it replaces the signatures already attached with the new signature. // Don't perform online validation or lookups if offline is true. -func SignStdTx(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, name string, stdTx auth.StdTx, appendSig bool, offline bool) (auth.StdTx, error) { +func SignStdTx( + txBldr authtxb.TxBuilder, cliCtx context.CLIContext, name string, + stdTx auth.StdTx, appendSig bool, offline bool, +) (auth.StdTx, error) { + var signedStdTx auth.StdTx - keybase := txBldr.Keybase() - - info, err := keybase.Get(name) + info, err := txBldr.Keybase().Get(name) if err != nil { return signedStdTx, err } @@ -185,8 +187,7 @@ func SignStdTx(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, name string, } if !offline { - txBldr, err = populateAccountFromState( - txBldr, cliCtx, sdk.AccAddress(addr)) + txBldr, err = populateAccountFromState(txBldr, cliCtx, sdk.AccAddress(addr)) if err != nil { return signedStdTx, err } @@ -244,25 +245,21 @@ func ReadStdTxFromFile(cdc *amino.Codec, filename string) (stdTx auth.StdTx, err return } -func populateAccountFromState(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, - addr sdk.AccAddress) (authtxb.TxBuilder, error) { - if txBldr.AccountNumber() == 0 { - accNum, err := cliCtx.GetAccountNumber(addr) - if err != nil { - return txBldr, err - } - txBldr = txBldr.WithAccountNumber(accNum) +func populateAccountFromState( + txBldr authtxb.TxBuilder, cliCtx context.CLIContext, addr sdk.AccAddress, +) (authtxb.TxBuilder, error) { + + accNum, err := cliCtx.GetAccountNumber(addr) + if err != nil { + return txBldr, err } - if txBldr.Sequence() == 0 { - accSeq, err := cliCtx.GetAccountSequence(addr) - if err != nil { - return txBldr, err - } - txBldr = txBldr.WithSequence(accSeq) + accSeq, err := cliCtx.GetAccountSequence(addr) + if err != nil { + return txBldr, err } - return txBldr, nil + return txBldr.WithAccountNumber(accNum).WithSequence(accSeq), nil } // GetTxEncoder return tx encoder from global sdk configuration if ones is defined. diff --git a/x/auth/client/cli/sign.go b/x/auth/client/cli/sign.go index e30fd2e4f..6d3a9fe29 100644 --- a/x/auth/client/cli/sign.go +++ b/x/auth/client/cli/sign.go @@ -27,33 +27,35 @@ const ( flagOutfile = "output-document" ) -// GetSignCommand returns the sign command +// GetSignCommand returns the transaction sign command. func GetSignCommand(codec *amino.Codec) *cobra.Command { cmd := &cobra.Command{ Use: "sign [file]", Short: "Sign transactions generated offline", Long: `Sign transactions created with the --generate-only flag. -Read a transaction from [file], sign it, and print its JSON encoding. +It will read a transaction from [file], sign it, and print its JSON encoding. -If the flag --signature-only flag is on, it outputs a JSON representation +If the flag --signature-only flag is set, it will output a JSON representation of the generated signature only. -If the flag --validate-signatures is on, then the command would check whether all required +If the flag --validate-signatures is set, then the command would check whether all required signers have signed the transactions, whether the signatures were collected in the right order, and if the signature is valid over the given transaction. If the --offline -flag is also provided, signature validation over the transaction will be not be -performed as that will require communication with a full node. +flag is also set, signature validation over the transaction will be not be +performed as that will require RPC communication with a full node. -The --offline flag makes sure that the client will not reach out to an external node. -Thus account number or sequence number lookups will not be performed and it is -recommended to set such parameters manually. +The --offline flag makes sure that the client will not reach out to full node. +As a result, the account and sequence number queries will not be performed and +it is required to set such parameters manually. Note, invalid values will cause +the transaction to fail. The --multisig= flag generates a signature on behalf of a multisig account key. It implies --signature-only. Full multisig signed transactions may eventually be generated via the 'multisign' command. `, - RunE: makeSignCmd(codec), - Args: cobra.ExactArgs(1), + PreRun: preSignCmd, + RunE: makeSignCmd(codec), + Args: cobra.ExactArgs(1), } cmd.Flags().String( @@ -69,11 +71,22 @@ be generated via the 'multisign' command. "Print the addresses that must sign the transaction, those who have already signed it, and make sure that signatures are in the correct order", ) cmd.Flags().Bool(flagSigOnly, false, "Print only the generated signature, then exit") - cmd.Flags().Bool(flagOffline, false, "Offline mode. Do not query a full node") + cmd.Flags().Bool(flagOffline, false, "Offline mode; Do not query a full node") cmd.Flags().String(flagOutfile, "", "The document will be written to the given file instead of STDOUT") - // add the flags here and return the command - return client.PostCommands(cmd)[0] + cmd = client.PostCommands(cmd)[0] + cmd.MarkFlagRequired(client.FlagFrom) + + return cmd +} + +func preSignCmd(cmd *cobra.Command, _ []string) { + // Conditionally mark the account and sequence numbers required as no RPC + // query will be done. + if viper.GetBool(flagOffline) { + cmd.MarkFlagRequired(client.FlagAccountNumber) + cmd.MarkFlagRequired(client.FlagSequence) + } } func makeSignCmd(cdc *amino.Codec) func(cmd *cobra.Command, args []string) error { @@ -95,11 +108,6 @@ func makeSignCmd(cdc *amino.Codec) func(cmd *cobra.Command, args []string) error return nil } - from := viper.GetString(client.FlagFrom) - if from == "" { - return fmt.Errorf("required flag '%s' has not been set", client.FlagFrom) - } - // if --signature-only is on, then override --append var newTx auth.StdTx generateSignatureOnly := viper.GetBool(flagSigOnly) diff --git a/x/auth/client/txbuilder/txbuilder.go b/x/auth/client/txbuilder/txbuilder.go index 3ea6989e6..480d8fd41 100644 --- a/x/auth/client/txbuilder/txbuilder.go +++ b/x/auth/client/txbuilder/txbuilder.go @@ -245,7 +245,7 @@ func (bldr TxBuilder) BuildTxForSim(msgs []sdk.Msg) ([]byte, error) { return bldr.txEncoder(auth.NewStdTx(signMsg.Msgs, signMsg.Fee, sigs, signMsg.Memo)) } -// SignStdTx appends a signature to a StdTx and returns a copy of a it. If append +// SignStdTx appends a signature to a StdTx and returns a copy of it. If append // is false, it replaces the signatures already attached with the new signature. func (bldr TxBuilder) SignStdTx(name, passphrase string, stdTx auth.StdTx, appendSig bool) (signedStdTx auth.StdTx, err error) { stdSignature, err := MakeSignature(bldr.keybase, name, passphrase, StdSignMsg{ diff --git a/x/bank/client/cli/sendtx.go b/x/bank/client/cli/sendtx.go index 7e8a29a92..2f1d45b95 100644 --- a/x/bank/client/cli/sendtx.go +++ b/x/bank/client/cli/sendtx.go @@ -62,5 +62,9 @@ func SendTxCmd(cdc *codec.Codec) *cobra.Command { return utils.GenerateOrBroadcastMsgs(cliCtx, txBldr, []sdk.Msg{msg}, false) }, } - return client.PostCommands(cmd)[0] + + cmd = client.PostCommands(cmd)[0] + cmd.MarkFlagRequired(client.FlagFrom) + + return cmd }