Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement --offline flag #5810

Merged
merged 31 commits into from
Mar 19, 2020
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
7213e7a
add flag offline
jgimeno Mar 16, 2020
7f2dfde
change new context to take care of offline flag
jgimeno Mar 16, 2020
e5e9eb0
update cli context
jgimeno Mar 16, 2020
d64832f
update command GetCmdWithdrawAllRewards to take care of offline param
jgimeno Mar 16, 2020
b1962cd
delete trash doc created
jgimeno Mar 16, 2020
6ff0f3a
add offline cli context
jgimeno Mar 16, 2020
be4c08c
change condition on buildUnsignedStdTxOffline
jgimeno Mar 16, 2020
4754189
add new --offline flag
jgimeno Mar 16, 2020
448f7af
implement offline in broadcast command
jgimeno Mar 17, 2020
1e6aae6
update old offline command to be the generic one
jgimeno Mar 17, 2020
805dda9
update code
jgimeno Mar 17, 2020
d60146e
revert removing of parameter
jgimeno Mar 17, 2020
08ccfe5
add context test with offline
jgimeno Mar 17, 2020
e009fc3
add flag for cli context
jgimeno Mar 17, 2020
e07d848
Apply suggestions from code review
jgimeno Mar 17, 2020
ddd2e10
revert line break
jgimeno Mar 17, 2020
507779f
change error msg
jgimeno Mar 17, 2020
61f415f
Merge branch 'master' into jonathan/5448-split-generate-only-offline
jgimeno Mar 18, 2020
0621e57
Update client/context/context.go
jgimeno Mar 18, 2020
09d33f1
Update client/flags/flags.go
jgimeno Mar 18, 2020
f2c4dbc
Update x/auth/client/cli/broadcast.go
jgimeno Mar 18, 2020
4bc294e
update changelog
jgimeno Mar 18, 2020
fff39dc
update use case when generate only is provided and cannot acces keybase
jgimeno Mar 18, 2020
9a53f67
Merge branch 'jonathan/5448-split-generate-only-offline' of github.co…
jgimeno Mar 18, 2020
b73fa03
add flag info message
jgimeno Mar 18, 2020
d24af49
Merge branch 'master' into jonathan/5448-split-generate-only-offline
alexanderbez Mar 18, 2020
9166dba
add better error message when no node is set
jgimeno Mar 18, 2020
3b4a5e2
Merge branch 'jonathan/5448-split-generate-only-offline' of github.co…
jgimeno Mar 18, 2020
0607a27
fix unit test
jgimeno Mar 18, 2020
9df8a94
test broadcast
jgimeno Mar 19, 2020
30e07cd
change filepath join
jgimeno Mar 19, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,9 @@ Buffers for state serialization instead of Amino.
* (server) [\#5709](https://github.com/cosmos/cosmos-sdk/pull/5709) There are two new flags for pruning, `--pruning-keep-every`
and `--pruning-snapshot-every` as an alternative to `--pruning`. They allow to fine tune the strategy for pruning the state.
* (crypto/keys) [\#5739](https://github.com/cosmos/cosmos-sdk/pull/5739) Print an error message if the password input failed.
* (client) [\#5810](https://github.com/cosmos/cosmos-sdk/pull/5810) Added new `--offline` flag improving the user experience. This
jgimeno marked this conversation as resolved.
Show resolved Hide resolved
allow to use `--generate-only` to be called on those cases where it needs a connection but we don't need
to broadcast.

## [v0.38.1] - 2020-02-11

Expand Down
16 changes: 9 additions & 7 deletions client/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type CLIContext struct {
UseLedger bool
Simulate bool
GenerateOnly bool
Offline bool
Indent bool
SkipConfirm bool

Expand All @@ -60,14 +61,14 @@ func NewCLIContextWithInputAndFrom(input io.Reader, from string) CLIContext {
var nodeURI string
var rpc rpcclient.Client

genOnly := viper.GetBool(flags.FlagGenerateOnly)
fromAddress, fromName, err := GetFromFields(input, from, genOnly)
offline := viper.GetBool(flags.FlagOffline)
fromAddress, fromName, err := GetFromFields(input, from, offline)
if err != nil {
fmt.Printf("failed to get from fields: %v\n", err)
os.Exit(1)
}

if !genOnly {
if !offline {
nodeURI = viper.GetString(flags.FlagNode)
if nodeURI != "" {
rpc, err = rpcclient.NewHTTP(nodeURI, "/websocket")
Expand All @@ -92,7 +93,8 @@ func NewCLIContextWithInputAndFrom(input io.Reader, from string) CLIContext {
UseLedger: viper.GetBool(flags.FlagUseLedger),
BroadcastMode: viper.GetString(flags.FlagBroadcastMode),
Simulate: viper.GetBool(flags.FlagDryRun),
GenerateOnly: genOnly,
GenerateOnly: viper.GetBool(flags.FlagGenerateOnly),
Offline: offline,
FromAddress: fromAddress,
FromName: fromName,
Indent: viper.GetBool(flags.FlagIndentResponse),
Expand Down Expand Up @@ -276,14 +278,14 @@ func (ctx CLIContext) PrintOutput(toPrint interface{}) error {
}

// GetFromFields returns a from account address and Keybase name given either
// an address or key name. If genOnly is true, only a valid Bech32 cosmos
// an address or key name. If offline is true, only a valid Bech32 cosmos
// address is returned.
func GetFromFields(input io.Reader, from string, genOnly bool) (sdk.AccAddress, string, error) {
func GetFromFields(input io.Reader, from string, offline bool) (sdk.AccAddress, string, error) {
if from == "" {
return nil, "", nil
}

if genOnly {
if offline {
jgimeno marked this conversation as resolved.
Show resolved Hide resolved
addr, err := sdk.AccAddressFromBech32(from)
if err != nil {
return nil, "", errors.Wrap(err, "must provide a valid Bech32 address for generate-only")
jgimeno marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
28 changes: 28 additions & 0 deletions client/context/context_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package context

import (
"testing"

"github.com/spf13/viper"
"github.com/stretchr/testify/require"

"github.com/cosmos/cosmos-sdk/client/flags"
)

func TestCLIContext_WithOffline(t *testing.T) {
viper.Set(flags.FlagOffline, true)
viper.Set(flags.FlagNode, "tcp://localhost:26657")

ctx := NewCLIContext()
require.True(t, ctx.Offline)
require.Nil(t, ctx.Client)

viper.Reset()

viper.Set(flags.FlagOffline, false)
viper.Set(flags.FlagNode, "tcp://localhost:26657")

ctx = NewCLIContext()
require.False(t, ctx.Offline)
require.NotNil(t, ctx.Client)
}
4 changes: 3 additions & 1 deletion client/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ const (
FlagBroadcastMode = "broadcast-mode"
FlagDryRun = "dry-run"
FlagGenerateOnly = "generate-only"
FlagOffline = "offline"
FlagIndentResponse = "indent"
FlagListenAddr = "laddr"
FlagMaxOpenConnections = "max-open"
Expand Down Expand Up @@ -114,7 +115,8 @@ func PostCommands(cmds ...*cobra.Command) []*cobra.Command {
c.Flags().StringP(FlagBroadcastMode, "b", BroadcastSync, "Transaction broadcasting mode (sync|async|block)")
c.Flags().Bool(FlagTrustNode, true, "Trust connected full node (don't verify proofs for responses)")
c.Flags().Bool(FlagDryRun, false, "ignore the --gas flag and perform a simulation of a transaction, but don't broadcast it")
c.Flags().Bool(FlagGenerateOnly, false, "Build an unsigned transaction and write it to STDOUT (when enabled, the local Keybase is not accessible and the node operates offline)")
c.Flags().Bool(FlagGenerateOnly, false, "Build an unsigned transaction and write it to STDOUT")
jgimeno marked this conversation as resolved.
Show resolved Hide resolved
c.Flags().Bool(FlagOffline, false, "Offline mode. Do not query a full node")
jgimeno marked this conversation as resolved.
Show resolved Hide resolved
c.Flags().BoolP(FlagSkipConfirmation, "y", false, "Skip tx broadcasting prompt confirmation")
c.Flags().String(FlagKeyringBackend, DefaultKeyringBackend, "Select keyring's backend (os|file|test)")

Expand Down
6 changes: 6 additions & 0 deletions x/auth/client/cli/broadcast.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cli

import (
"errors"
"strings"

"github.com/spf13/cobra"
Expand All @@ -26,6 +27,11 @@ $ <appcli> tx broadcast ./mytxn.json
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) (err error) {
jgimeno marked this conversation as resolved.
Show resolved Hide resolved
cliCtx := context.NewCLIContext().WithCodec(cdc)

if cliCtx.Offline {
return errors.New("cannot broadcast tx with offline flag")
jgimeno marked this conversation as resolved.
Show resolved Hide resolved
}

stdTx, err := client.ReadStdTxFromFile(cliCtx.Codec, args[0])
if err != nil {
return
Expand Down
23 changes: 23 additions & 0 deletions x/auth/client/cli/broadcast_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package cli

import (
"testing"

"github.com/cosmos/cosmos-sdk/client/flags"

"github.com/stretchr/testify/require"

"github.com/spf13/viper"

"github.com/tendermint/go-amino"
)

func TestGetBroadcastCommand_CannotBePerformedWhenOfflineFlag(t *testing.T) {
codec := amino.NewCodec()
cmd := GetBroadcastCommand(codec)

viper.Set(flags.FlagOffline, true)

err := cmd.RunE(nil, []string{})
require.EqualError(t, err, "cannot broadcast tx with offline flag")
}
3 changes: 1 addition & 2 deletions x/auth/client/cli/tx_multisign.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ recommended to set such parameters manually.
}

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().String(flagOutfile, "", "The document will be written to the given file instead of STDOUT")

// Add the flags here and return the command
Expand Down Expand Up @@ -85,7 +84,7 @@ func makeMultiSignCmd(cdc *codec.Codec) func(cmd *cobra.Command, args []string)
cliCtx := context.NewCLIContextWithInput(inBuf).WithCodec(cdc)
txBldr := types.NewTxBuilderFromCLI(inBuf)

if !viper.GetBool(flagOffline) {
if !cliCtx.Offline {
accnum, seq, err := types.NewAccountRetriever(client.Codec, cliCtx).GetAccountNumberSequence(multisigInfo.GetAddress())
if err != nil {
return err
Expand Down
16 changes: 4 additions & 12 deletions x/auth/client/cli/tx_sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ const (
flagMultisig = "multisig"
flagAppend = "append"
flagValidateSigs = "validate-signatures"
flagOffline = "offline"
flagSigOnly = "signature-only"
flagOutfile = "output-document"
)
Expand Down Expand Up @@ -71,12 +70,7 @@ 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. --account and --sequence options would be required if offline is set",
)
cmd.Flags().String(flagOutfile, "", "The document will be written to the given file instead of STDOUT")

cmd = flags.PostCommands(cmd)[0]
cmd.MarkFlagRequired(flags.FlagFrom)

Expand All @@ -86,7 +80,7 @@ be generated via the 'multisign' command.
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) {
if viper.GetBool(flags.FlagOffline) {
cmd.MarkFlagRequired(flags.FlagAccountNumber)
cmd.MarkFlagRequired(flags.FlagSequence)
}
Expand All @@ -100,12 +94,11 @@ func makeSignCmd(cdc *codec.Codec) func(cmd *cobra.Command, args []string) error
}

inBuf := bufio.NewReader(cmd.InOrStdin())
offline := viper.GetBool(flagOffline)
cliCtx := context.NewCLIContextWithInput(inBuf).WithCodec(cdc)
txBldr := types.NewTxBuilderFromCLI(inBuf)

if viper.GetBool(flagValidateSigs) {
if !printAndValidateSigs(cliCtx, txBldr.ChainID(), stdTx, offline) {
if !printAndValidateSigs(cliCtx, txBldr.ChainID(), stdTx, cliCtx.Offline) {
return fmt.Errorf("signatures validation failed")
}

Expand All @@ -124,14 +117,13 @@ func makeSignCmd(cdc *codec.Codec) func(cmd *cobra.Command, args []string) error
if err != nil {
return err
}

newTx, err = client.SignStdTxWithSignerAddress(
txBldr, cliCtx, multisigAddr, cliCtx.GetFromName(), stdTx, offline,
txBldr, cliCtx, multisigAddr, cliCtx.GetFromName(), stdTx, cliCtx.Offline,
)
generateSignatureOnly = true
} else {
appendSig := viper.GetBool(flagAppend) && !generateSignatureOnly
newTx, err = client.SignStdTx(txBldr, cliCtx, cliCtx.GetFromName(), stdTx, appendSig, offline)
newTx, err = client.SignStdTx(txBldr, cliCtx, cliCtx.GetFromName(), stdTx, appendSig, cliCtx.Offline)
}

if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions x/auth/client/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,8 +327,8 @@ func PrepareTxBuilder(txBldr authtypes.TxBuilder, cliCtx context.CLIContext) (au

func buildUnsignedStdTxOffline(txBldr authtypes.TxBuilder, cliCtx context.CLIContext, msgs []sdk.Msg) (stdTx authtypes.StdTx, err error) {
if txBldr.SimulateAndExecute() {
if cliCtx.GenerateOnly {
return stdTx, errors.New("cannot estimate gas with generate-only")
if cliCtx.Offline {
return stdTx, errors.New("cannot estimate gas in offline mode")
}

txBldr, err = EnrichWithGas(txBldr, cliCtx, msgs)
Expand Down
4 changes: 2 additions & 2 deletions x/distribution/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ $ %s tx distribution withdraw-all-rewards --from mykey

// The transaction cannot be generated offline since it requires a query
// to get all the validators.
if cliCtx.GenerateOnly {
return fmt.Errorf("command disabled with the provided flag: %s", flags.FlagGenerateOnly)
if cliCtx.Offline {
return fmt.Errorf("cannot generate tx in offline mode")
}

msgs, err := common.WithdrawAllDelegatorRewards(cliCtx, queryRoute, delAddr)
Expand Down