Skip to content

Commit

Permalink
Merge PR cosmos#3698: Prompt User Confirmation Prior to Signing & Bro…
Browse files Browse the repository at this point in the history
…adcasting

* Prompt user confirmation prior to sign & broadcasting
* Update confirmation message
* Update and fix existing CLI integration tests
* Implement CLI integration test for tx confirmation
* Fix order of input into tx send
  • Loading branch information
alexanderbez authored and cwgoes committed Feb 26, 2019
1 parent 250dc98 commit feb98bc
Show file tree
Hide file tree
Showing 8 changed files with 105 additions and 37 deletions.
1 change: 1 addition & 0 deletions PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ decoded automatically.

### Gaia CLI

* [\#3653] Prompt user confirmation prior to signing and broadcasting a transaction.
* [\#3670] CLI support for showing bech32 addresses in Ledger devices
* [\#3711] Update `tx sign` to use `--from` instead of the deprecated `--name`
CLI flag.
Expand Down
2 changes: 2 additions & 0 deletions client/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ type CLIContext struct {
FromAddress sdk.AccAddress
FromName string
Indent bool
SkipConfirm bool
}

// NewCLIContext returns a new initialized CLIContext with parameters from the
Expand Down Expand Up @@ -96,6 +97,7 @@ func NewCLIContext() CLIContext {
FromAddress: fromAddress,
FromName: fromName,
Indent: viper.GetBool(client.FlagIndentResponse),
SkipConfirm: viper.GetBool(client.FlagSkipConfirmation),
}
}

Expand Down
8 changes: 7 additions & 1 deletion client/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ const (
FlagSSLCertFile = "ssl-certfile"
FlagSSLKeyFile = "ssl-keyfile"
FlagOutputDocument = "output-document" // inspired by wget -O
FlagSkipConfirmation = "yes"
)

// LineBreak can be included in a command list to provide a blank line
Expand Down Expand Up @@ -89,9 +90,14 @@ func PostCommands(cmds ...*cobra.Command) []*cobra.Command {
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")
c.Flags().BoolP(FlagSkipConfirmation, "y", false, "Skip tx broadcasting prompt confirmation")

// --gas can accept integers and "simulate"
c.Flags().Var(&GasFlagVar, "gas", fmt.Sprintf(
"gas limit to set per-transaction; set to %q to calculate required gas automatically (default %d)", GasFlagAuto, DefaultGasLimit))
"gas limit to set per-transaction; set to %q to calculate required gas automatically (default %d)",
GasFlagAuto, DefaultGasLimit,
))

viper.BindPFlag(FlagTrustNode, c.Flags().Lookup(FlagTrustNode))
viper.BindPFlag(FlagUseLedger, c.Flags().Lookup(FlagUseLedger))
viper.BindPFlag(FlagNode, c.Flags().Lookup(FlagNode))
Expand Down
5 changes: 3 additions & 2 deletions client/input.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"errors"

"github.com/bgentry/speakeasy"
"github.com/mattn/go-isatty"
isatty "github.com/mattn/go-isatty"
)

// MinPassLength is the minimum acceptable password length
Expand Down Expand Up @@ -90,8 +90,9 @@ func GetCheckPassword(prompt, prompt2 string, buf *bufio.Reader) (string, error)
func GetConfirmation(prompt string, buf *bufio.Reader) (bool, error) {
for {
if inputIsTty() {
fmt.Print(fmt.Sprintf("%s [y/n]:", prompt))
fmt.Print(fmt.Sprintf("%s [Y/n]: ", prompt))
}

response, err := readLineFromBuf(buf)
if err != nil {
return false, err
Expand Down
16 changes: 16 additions & 0 deletions client/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,22 @@ func CompleteAndBroadcastTxCLI(txBldr authtxb.TxBuilder, cliCtx context.CLIConte
return nil
}

if !cliCtx.SkipConfirm {
stdSignMsg, err := txBldr.BuildSignMsg(msgs)
if err != nil {
return err
}

fmt.Fprintf(os.Stderr, "%s\n\n", cliCtx.Codec.MustMarshalJSON(stdSignMsg))

buf := client.BufferStdin()
ok, err := client.GetConfirmation("confirm transaction before signing and broadcasting", buf)
if err != nil || !ok {
fmt.Fprintf(os.Stderr, "%s\n", "cancelled transaction")
return err
}
}

passphrase, err := keys.GetPassphrase(fromName)
if err != nil {
return err
Expand Down
94 changes: 64 additions & 30 deletions cmd/gaia/cli_test/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,19 +86,19 @@ func TestGaiaCLIMinimumFees(t *testing.T) {
barAddr := f.KeyAddress(keyBar)

// Send a transaction that will get rejected
success, _, _ := f.TxSend(keyFoo, barAddr, sdk.NewInt64Coin(fee2Denom, 10))
success, _, _ := f.TxSend(keyFoo, barAddr, sdk.NewInt64Coin(fee2Denom, 10), "-y")
require.False(f.T, success)
tests.WaitForNextNBlocksTM(1, f.Port)

// Ensure tx w/ correct fees pass
txFees := fmt.Sprintf("--fees=%s", sdk.NewInt64Coin(feeDenom, 2))
success, _, _ = f.TxSend(keyFoo, barAddr, sdk.NewInt64Coin(fee2Denom, 10), txFees)
success, _, _ = f.TxSend(keyFoo, barAddr, sdk.NewInt64Coin(fee2Denom, 10), txFees, "-y")
require.True(f.T, success)
tests.WaitForNextNBlocksTM(1, f.Port)

// Ensure tx w/ improper fees fails
txFees = fmt.Sprintf("--fees=%s", sdk.NewInt64Coin(feeDenom, 1))
success, _, _ = f.TxSend(keyFoo, barAddr, sdk.NewInt64Coin(fooDenom, 10), txFees)
success, _, _ = f.TxSend(keyFoo, barAddr, sdk.NewInt64Coin(fooDenom, 10), txFees, "-y")
require.False(f.T, success)

// Cleanup testing directories
Expand All @@ -120,7 +120,7 @@ func TestGaiaCLIGasPrices(t *testing.T) {
badGasPrice, _ := sdk.NewDecFromStr("0.000003")
success, _, _ := f.TxSend(
keyFoo, barAddr, sdk.NewInt64Coin(fooDenom, 50),
fmt.Sprintf("--gas-prices=%s", sdk.NewDecCoinFromDec(feeDenom, badGasPrice)))
fmt.Sprintf("--gas-prices=%s", sdk.NewDecCoinFromDec(feeDenom, badGasPrice)), "-y")
require.False(t, success)

// wait for a block confirmation
Expand All @@ -129,7 +129,7 @@ func TestGaiaCLIGasPrices(t *testing.T) {
// sufficient gas prices (tx passes)
success, _, _ = f.TxSend(
keyFoo, barAddr, sdk.NewInt64Coin(fooDenom, 50),
fmt.Sprintf("--gas-prices=%s", sdk.NewDecCoinFromDec(feeDenom, minGasPrice)))
fmt.Sprintf("--gas-prices=%s", sdk.NewDecCoinFromDec(feeDenom, minGasPrice)), "-y")
require.True(t, success)

// wait for a block confirmation
Expand Down Expand Up @@ -171,7 +171,7 @@ func TestGaiaCLIFeesDeduction(t *testing.T) {
largeCoins := sdk.TokensFromTendermintPower(10000000)
success, _, _ = f.TxSend(
keyFoo, barAddr, sdk.NewCoin(fooDenom, largeCoins),
fmt.Sprintf("--fees=%s", sdk.NewInt64Coin(feeDenom, 2)))
fmt.Sprintf("--fees=%s", sdk.NewInt64Coin(feeDenom, 2)), "-y")
require.False(t, success)

// Wait for a block
Expand All @@ -184,7 +184,7 @@ func TestGaiaCLIFeesDeduction(t *testing.T) {
// test success (transfer = coins + fees)
success, _, _ = f.TxSend(
keyFoo, barAddr, sdk.NewInt64Coin(fooDenom, 500),
fmt.Sprintf("--fees=%s", sdk.NewInt64Coin(feeDenom, 2)))
fmt.Sprintf("--fees=%s", sdk.NewInt64Coin(feeDenom, 2)), "-y")
require.True(t, success)

f.Cleanup()
Expand All @@ -208,7 +208,7 @@ func TestGaiaCLISend(t *testing.T) {

// Send some tokens from one account to the other
sendTokens := sdk.TokensFromTendermintPower(10)
f.TxSend(keyFoo, barAddr, sdk.NewCoin(denom, sendTokens))
f.TxSend(keyFoo, barAddr, sdk.NewCoin(denom, sendTokens), "-y")
tests.WaitForNextNBlocksTM(1, f.Port)

// Ensure account balances match expected
Expand All @@ -226,7 +226,7 @@ func TestGaiaCLISend(t *testing.T) {
require.Equal(t, startTokens.Sub(sendTokens), fooAcc.GetCoins().AmountOf(denom))

// test autosequencing
f.TxSend(keyFoo, barAddr, sdk.NewCoin(denom, sendTokens))
f.TxSend(keyFoo, barAddr, sdk.NewCoin(denom, sendTokens), "-y")
tests.WaitForNextNBlocksTM(1, f.Port)

// Ensure account balances match expected
Expand All @@ -236,7 +236,7 @@ func TestGaiaCLISend(t *testing.T) {
require.Equal(t, startTokens.Sub(sendTokens.MulRaw(2)), fooAcc.GetCoins().AmountOf(denom))

// test memo
f.TxSend(keyFoo, barAddr, sdk.NewCoin(denom, sendTokens), "--memo='testmemo'")
f.TxSend(keyFoo, barAddr, sdk.NewCoin(denom, sendTokens), "--memo='testmemo'", "-y")
tests.WaitForNextNBlocksTM(1, f.Port)

// Ensure account balances match expected
Expand All @@ -248,6 +248,40 @@ func TestGaiaCLISend(t *testing.T) {
f.Cleanup()
}

func TestGaiaCLIConfirmTx(t *testing.T) {
t.Parallel()
f := InitFixtures(t)

// start gaiad server
proc := f.GDStart()
defer proc.Stop(false)

// Save key addresses for later use
fooAddr := f.KeyAddress(keyFoo)
barAddr := f.KeyAddress(keyBar)

fooAcc := f.QueryAccount(fooAddr)
startTokens := sdk.TokensFromTendermintPower(50)
require.Equal(t, startTokens, fooAcc.GetCoins().AmountOf(denom))

// send some tokens from one account to the other
sendTokens := sdk.TokensFromTendermintPower(10)
f.txSendWithConfirm(keyFoo, barAddr, sdk.NewCoin(denom, sendTokens), "Y")
tests.WaitForNextNBlocksTM(1, f.Port)

// ensure account balances match expected
barAcc := f.QueryAccount(barAddr)
require.Equal(t, sendTokens, barAcc.GetCoins().AmountOf(denom))

// send some tokens from one account to the other (cancelling confirmation)
f.txSendWithConfirm(keyFoo, barAddr, sdk.NewCoin(denom, sendTokens), "n")
tests.WaitForNextNBlocksTM(1, f.Port)

// ensure account balances match expected
barAcc = f.QueryAccount(barAddr)
require.Equal(t, sendTokens, barAcc.GetCoins().AmountOf(denom))
}

func TestGaiaCLIGasAuto(t *testing.T) {
t.Parallel()
f := InitFixtures(t)
Expand All @@ -265,31 +299,31 @@ func TestGaiaCLIGasAuto(t *testing.T) {

// Test failure with auto gas disabled and very little gas set by hand
sendTokens := sdk.TokensFromTendermintPower(10)
success, _, _ := f.TxSend(keyFoo, barAddr, sdk.NewCoin(denom, sendTokens), "--gas=10")
success, _, _ := f.TxSend(keyFoo, barAddr, sdk.NewCoin(denom, sendTokens), "--gas=10", "-y")
require.False(t, success)

// Check state didn't change
fooAcc = f.QueryAccount(fooAddr)
require.Equal(t, startTokens, fooAcc.GetCoins().AmountOf(denom))

// Test failure with negative gas
success, _, _ = f.TxSend(keyFoo, barAddr, sdk.NewCoin(denom, sendTokens), "--gas=-100")
success, _, _ = f.TxSend(keyFoo, barAddr, sdk.NewCoin(denom, sendTokens), "--gas=-100", "-y")
require.False(t, success)

// Check state didn't change
fooAcc = f.QueryAccount(fooAddr)
require.Equal(t, startTokens, fooAcc.GetCoins().AmountOf(denom))

// Test failure with 0 gas
success, _, _ = f.TxSend(keyFoo, barAddr, sdk.NewCoin(denom, sendTokens), "--gas=0")
success, _, _ = f.TxSend(keyFoo, barAddr, sdk.NewCoin(denom, sendTokens), "--gas=0", "-y")
require.False(t, success)

// Check state didn't change
fooAcc = f.QueryAccount(fooAddr)
require.Equal(t, startTokens, fooAcc.GetCoins().AmountOf(denom))

// Enable auto gas
success, stdout, stderr := f.TxSend(keyFoo, barAddr, sdk.NewCoin(denom, sendTokens), "--gas=auto")
success, stdout, stderr := f.TxSend(keyFoo, barAddr, sdk.NewCoin(denom, sendTokens), "--gas=auto", "-y")
require.NotEmpty(t, stderr)
require.True(t, success)
cdc := app.MakeCodec()
Expand Down Expand Up @@ -320,7 +354,7 @@ func TestGaiaCLICreateValidator(t *testing.T) {
consPubKey := sdk.MustBech32ifyConsPub(ed25519.GenPrivKey().PubKey())

sendTokens := sdk.TokensFromTendermintPower(10)
f.TxSend(keyFoo, barAddr, sdk.NewCoin(denom, sendTokens))
f.TxSend(keyFoo, barAddr, sdk.NewCoin(denom, sendTokens), "-y")
tests.WaitForNextNBlocksTM(1, f.Port)

barAcc := f.QueryAccount(barAddr)
Expand All @@ -342,7 +376,7 @@ func TestGaiaCLICreateValidator(t *testing.T) {
require.True(t, success)

// Create the validator
f.TxStakingCreateValidator(keyBar, consPubKey, sdk.NewCoin(denom, newValTokens))
f.TxStakingCreateValidator(keyBar, consPubKey, sdk.NewCoin(denom, newValTokens), "-y")
tests.WaitForNextNBlocksTM(1, f.Port)

// Ensure funds were deducted properly
Expand All @@ -361,7 +395,7 @@ func TestGaiaCLICreateValidator(t *testing.T) {

// unbond a single share
unbondTokens := sdk.TokensFromTendermintPower(1)
success = f.TxStakingUnbond(keyBar, unbondTokens.String(), barVal)
success = f.TxStakingUnbond(keyBar, unbondTokens.String(), barVal, "-y")
require.True(t, success)
tests.WaitForNextNBlocksTM(1, f.Port)

Expand Down Expand Up @@ -403,7 +437,7 @@ func TestGaiaCLISubmitProposal(t *testing.T) {
// Test submit generate only for submit proposal
proposalTokens := sdk.TokensFromTendermintPower(5)
success, stdout, stderr := f.TxGovSubmitProposal(
keyFoo, "Text", "Test", "test", sdk.NewCoin(denom, proposalTokens), "--generate-only")
keyFoo, "Text", "Test", "test", sdk.NewCoin(denom, proposalTokens), "--generate-only", "-y")
require.True(t, success)
require.Empty(t, stderr)
msg := unmarshalStdTx(t, stdout)
Expand All @@ -416,7 +450,7 @@ func TestGaiaCLISubmitProposal(t *testing.T) {
require.True(t, success)

// Create the proposal
f.TxGovSubmitProposal(keyFoo, "Text", "Test", "test", sdk.NewCoin(denom, proposalTokens))
f.TxGovSubmitProposal(keyFoo, "Text", "Test", "test", sdk.NewCoin(denom, proposalTokens), "-y")
tests.WaitForNextNBlocksTM(1, f.Port)

// Ensure transaction tags can be queried
Expand Down Expand Up @@ -451,7 +485,7 @@ func TestGaiaCLISubmitProposal(t *testing.T) {
require.Equal(t, 0, len(msg.GetSignatures()))

// Run the deposit transaction
f.TxGovDeposit(1, keyFoo, sdk.NewCoin(denom, depositTokens))
f.TxGovDeposit(1, keyFoo, sdk.NewCoin(denom, depositTokens), "-y")
tests.WaitForNextNBlocksTM(1, f.Port)

// test query deposit
Expand Down Expand Up @@ -486,7 +520,7 @@ func TestGaiaCLISubmitProposal(t *testing.T) {
require.Equal(t, 0, len(msg.GetSignatures()))

// Vote on the proposal
f.TxGovVote(1, gov.OptionYes, keyFoo)
f.TxGovVote(1, gov.OptionYes, keyFoo, "-y")
tests.WaitForNextNBlocksTM(1, f.Port)

// Query the vote
Expand All @@ -513,7 +547,7 @@ func TestGaiaCLISubmitProposal(t *testing.T) {
require.Equal(t, uint64(1), proposalsQuery[0].GetProposalID())

// submit a second test proposal
f.TxGovSubmitProposal(keyFoo, "Text", "Apples", "test", sdk.NewCoin(denom, proposalTokens))
f.TxGovSubmitProposal(keyFoo, "Text", "Apples", "test", sdk.NewCoin(denom, proposalTokens), "-y")
tests.WaitForNextNBlocksTM(1, f.Port)

// Test limit on proposals query
Expand All @@ -538,7 +572,7 @@ func TestGaiaCLIQueryTxPagination(t *testing.T) {
seq := accFoo.GetSequence()

for i := 1; i <= 30; i++ {
success, _, _ := f.TxSend(keyFoo, barAddr, sdk.NewInt64Coin(fooDenom, int64(i)), fmt.Sprintf("--sequence=%d", seq))
success, _, _ := f.TxSend(keyFoo, barAddr, sdk.NewInt64Coin(fooDenom, int64(i)), fmt.Sprintf("--sequence=%d", seq), "-y")
require.True(t, success)
seq++
}
Expand Down Expand Up @@ -725,7 +759,7 @@ func TestGaiaCLIMultisignInsufficientCosigners(t *testing.T) {
barAddr := f.KeyAddress(keyBar)

// Send some tokens from one account to the other
success, _, _ := f.TxSend(keyFoo, fooBarBazAddr, sdk.NewInt64Coin(denom, 10))
success, _, _ := f.TxSend(keyFoo, fooBarBazAddr, sdk.NewInt64Coin(denom, 10), "-y")
require.True(t, success)
tests.WaitForNextNBlocksTM(1, f.Port)

Expand All @@ -738,7 +772,7 @@ func TestGaiaCLIMultisignInsufficientCosigners(t *testing.T) {
defer os.Remove(unsignedTxFile.Name())

// Sign with foo's key
success, stdout, _ = f.TxSign(keyFoo, unsignedTxFile.Name(), "--multisig", fooBarBazAddr.String())
success, stdout, _ = f.TxSign(keyFoo, unsignedTxFile.Name(), "--multisig", fooBarBazAddr.String(), "-y")
require.True(t, success)

// Write the output to disk
Expand Down Expand Up @@ -810,7 +844,7 @@ func TestGaiaCLIMultisignSortSignatures(t *testing.T) {
barAddr := f.KeyAddress(keyBar)

// Send some tokens from one account to the other
success, _, _ := f.TxSend(keyFoo, fooBarBazAddr, sdk.NewInt64Coin(denom, 10))
success, _, _ := f.TxSend(keyFoo, fooBarBazAddr, sdk.NewInt64Coin(denom, 10), "-y")
require.True(t, success)
tests.WaitForNextNBlocksTM(1, f.Port)

Expand Down Expand Up @@ -872,7 +906,7 @@ func TestGaiaCLIMultisign(t *testing.T) {
bazAddr := f.KeyAddress(keyBaz)

// Send some tokens from one account to the other
success, _, _ := f.TxSend(keyFoo, fooBarBazAddr, sdk.NewInt64Coin(denom, 10))
success, _, _ := f.TxSend(keyFoo, fooBarBazAddr, sdk.NewInt64Coin(denom, 10), "-y")
require.True(t, success)
tests.WaitForNextNBlocksTM(1, f.Port)

Expand All @@ -890,15 +924,15 @@ func TestGaiaCLIMultisign(t *testing.T) {
defer os.Remove(unsignedTxFile.Name())

// Sign with foo's key
success, stdout, _ = f.TxSign(keyFoo, unsignedTxFile.Name(), "--multisig", fooBarBazAddr.String())
success, stdout, _ = f.TxSign(keyFoo, unsignedTxFile.Name(), "--multisig", fooBarBazAddr.String(), "-y")
require.True(t, success)

// Write the output to disk
fooSignatureFile := WriteToNewTempFile(t, stdout)
defer os.Remove(fooSignatureFile.Name())

// Sign with bar's key
success, stdout, _ = f.TxSign(keyBar, unsignedTxFile.Name(), "--multisig", fooBarBazAddr.String())
success, stdout, _ = f.TxSign(keyBar, unsignedTxFile.Name(), "--multisig", fooBarBazAddr.String(), "-y")
require.True(t, success)

// Write the output to disk
Expand All @@ -915,7 +949,7 @@ func TestGaiaCLIMultisign(t *testing.T) {
defer os.Remove(signedTxFile.Name())

// Validate the multisignature
success, _, _ = f.TxSign(keyFooBarBaz, signedTxFile.Name(), "--validate-signatures")
success, _, _ = f.TxSign(keyFooBarBaz, signedTxFile.Name(), "--validate-signatures", "-y")
require.True(t, success)

// Broadcast the transaction
Expand Down
Loading

0 comments on commit feb98bc

Please sign in to comment.