From bc9e2820192a53a9f14beaa34e3394975bd576f5 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Fri, 1 Feb 2019 09:55:34 -0500 Subject: [PATCH 1/7] remove tip note from gaia docs --- docs/gaia/gaiacli.md | 9 --------- 1 file changed, 9 deletions(-) diff --git a/docs/gaia/gaiacli.md b/docs/gaia/gaiacli.md index 4886167e2c07..a3ad6dc36952 100644 --- a/docs/gaia/gaiacli.md +++ b/docs/gaia/gaiacli.md @@ -233,15 +233,6 @@ gaiacli tx send \ --generate-only > unsignedSendTx.json ``` -::: tip Note -Simulation cannot be used in conjunction with tx generation only functionality -due to the fact that simulation requires a public key and generation only does -not utilize a Keybase. - -You can now sign the transaction file generated through the `--generate-only` -flag by providing your key to the following command: -::: - ```bash gaiacli tx sign \ --chain-id= \ From 93d6daefb874418513de678717bab2a817e25c23 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Fri, 1 Feb 2019 10:05:43 -0500 Subject: [PATCH 2/7] update EnrichWithGas and TxBuilder APIs to not require name for gas sim --- client/utils/rest.go | 2 +- client/utils/utils.go | 20 +++++++++---------- x/auth/client/txbuilder/txbuilder.go | 30 +++++++++------------------- 3 files changed, 19 insertions(+), 33 deletions(-) diff --git a/client/utils/rest.go b/client/utils/rest.go index 44fc746adf13..5063fb107104 100644 --- a/client/utils/rest.go +++ b/client/utils/rest.go @@ -250,7 +250,7 @@ func CompleteAndBroadcastTxREST( return } - txBldr, err = EnrichWithGas(txBldr, cliCtx, cliCtx.GetFromName(), msgs) + txBldr, err = EnrichWithGas(txBldr, cliCtx, msgs) if err != nil { WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return diff --git a/client/utils/utils.go b/client/utils/utils.go index a30a54cd135c..7d5c47400a0f 100644 --- a/client/utils/utils.go +++ b/client/utils/utils.go @@ -31,10 +31,10 @@ func CompleteAndBroadcastTxCLI(txBldr authtxb.TxBuilder, cliCtx context.CLIConte return err } - name := cliCtx.GetFromName() + fromName := cliCtx.GetFromName() if txBldr.GetSimulateAndExecute() || cliCtx.Simulate { - txBldr, err = EnrichWithGas(txBldr, cliCtx, name, msgs) + txBldr, err = EnrichWithGas(txBldr, cliCtx, msgs) if err != nil { return err } @@ -44,13 +44,13 @@ func CompleteAndBroadcastTxCLI(txBldr authtxb.TxBuilder, cliCtx context.CLIConte return nil } - passphrase, err := keys.GetPassphrase(name) + passphrase, err := keys.GetPassphrase(fromName) if err != nil { return err } // build and sign the transaction - txBytes, err := txBldr.BuildAndSign(name, passphrase, msgs) + txBytes, err := txBldr.BuildAndSign(fromName, passphrase, msgs) if err != nil { return err } @@ -62,8 +62,8 @@ func CompleteAndBroadcastTxCLI(txBldr authtxb.TxBuilder, cliCtx context.CLIConte // EnrichWithGas calculates the gas estimate that would be consumed by the // transaction and set the transaction's respective value accordingly. -func EnrichWithGas(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, name string, msgs []sdk.Msg) (authtxb.TxBuilder, error) { - _, adjusted, err := simulateMsgs(txBldr, cliCtx, name, msgs) +func EnrichWithGas(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, msgs []sdk.Msg) (authtxb.TxBuilder, error) { + _, adjusted, err := simulateMsgs(txBldr, cliCtx, msgs) if err != nil { return txBldr, err } @@ -207,8 +207,8 @@ func GetTxEncoder(cdc *codec.Codec) (encoder sdk.TxEncoder) { // nolint // SimulateMsgs simulates the transaction and returns the gas estimate and the adjusted value. -func simulateMsgs(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, name string, msgs []sdk.Msg) (estimated, adjusted uint64, err error) { - txBytes, err := txBldr.BuildWithPubKey(name, msgs) +func simulateMsgs(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, msgs []sdk.Msg) (estimated, adjusted uint64, err error) { + txBytes, err := txBldr.BuildForSim(msgs) if err != nil { return } @@ -269,9 +269,7 @@ func buildUnsignedStdTx(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, msg func buildUnsignedStdTxOffline(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, msgs []sdk.Msg) (stdTx auth.StdTx, err error) { if txBldr.GetSimulateAndExecute() { - name := cliCtx.GetFromName() - - txBldr, err = EnrichWithGas(txBldr, cliCtx, name, msgs) + txBldr, err = EnrichWithGas(txBldr, cliCtx, msgs) if err != nil { return } diff --git a/x/auth/client/txbuilder/txbuilder.go b/x/auth/client/txbuilder/txbuilder.go index ed5f4339d360..3ab1e0f29dd7 100644 --- a/x/auth/client/txbuilder/txbuilder.go +++ b/x/auth/client/txbuilder/txbuilder.go @@ -156,6 +156,8 @@ func (bldr TxBuilder) WithAccountNumber(accnum uint64) TxBuilder { // Build builds a single message to be signed from a TxBuilder given a set of // messages. It returns an error if a fee is supplied but cannot be parsed. +// +// TODO: Should consider renaming to BuildSignMsg. func (bldr TxBuilder) Build(msgs []sdk.Msg) (StdSignMsg, error) { chainID := bldr.chainID if chainID == "" { @@ -212,31 +214,17 @@ func (bldr TxBuilder) BuildAndSign(name, passphrase string, msgs []sdk.Msg) ([]b return bldr.Sign(name, passphrase, msg) } -// BuildWithPubKey builds a single message to be signed from a TxBuilder given a set of -// messages and attach the public key associated to the given name. -// It returns an error if a fee is supplied but cannot be parsed or the key cannot be -// retrieved. -func (bldr TxBuilder) BuildWithPubKey(name string, msgs []sdk.Msg) ([]byte, error) { - msg, err := bldr.Build(msgs) - if err != nil { - return nil, err - } - - keybase, err := keys.GetKeyBase() +// BuildForSim creates a StdSignMsg and encodes a transaction with the +// StdSignMsg with a single empty StdSignature for tx simulation. +func (bldr TxBuilder) BuildForSim(msgs []sdk.Msg) ([]byte, error) { + signMsg, err := bldr.Build(msgs) if err != nil { return nil, err } - info, err := keybase.Get(name) - if err != nil { - return nil, err - } - - sigs := []auth.StdSignature{{ - PubKey: info.GetPubKey(), - }} - - return bldr.txEncoder(auth.NewStdTx(msg.Msgs, msg.Fee, sigs, msg.Memo)) + // the ante handler will populate with a sentinel pubkey + sigs := []auth.StdSignature{{}} + 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 From c1d12361f8194d743a31f99754f6d4eee66310b0 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Fri, 1 Feb 2019 10:28:49 -0500 Subject: [PATCH 3/7] rename to BuildTxForSim --- client/utils/utils.go | 2 +- x/auth/client/txbuilder/txbuilder.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/client/utils/utils.go b/client/utils/utils.go index 7d5c47400a0f..2dbc9d82ffa4 100644 --- a/client/utils/utils.go +++ b/client/utils/utils.go @@ -208,7 +208,7 @@ func GetTxEncoder(cdc *codec.Codec) (encoder sdk.TxEncoder) { // nolint // SimulateMsgs simulates the transaction and returns the gas estimate and the adjusted value. func simulateMsgs(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, msgs []sdk.Msg) (estimated, adjusted uint64, err error) { - txBytes, err := txBldr.BuildForSim(msgs) + txBytes, err := txBldr.BuildTxForSim(msgs) if err != nil { return } diff --git a/x/auth/client/txbuilder/txbuilder.go b/x/auth/client/txbuilder/txbuilder.go index 3ab1e0f29dd7..331d811ba959 100644 --- a/x/auth/client/txbuilder/txbuilder.go +++ b/x/auth/client/txbuilder/txbuilder.go @@ -214,9 +214,9 @@ func (bldr TxBuilder) BuildAndSign(name, passphrase string, msgs []sdk.Msg) ([]b return bldr.Sign(name, passphrase, msg) } -// BuildForSim creates a StdSignMsg and encodes a transaction with the +// BuildTxForSim creates a StdSignMsg and encodes a transaction with the // StdSignMsg with a single empty StdSignature for tx simulation. -func (bldr TxBuilder) BuildForSim(msgs []sdk.Msg) ([]byte, error) { +func (bldr TxBuilder) BuildTxForSim(msgs []sdk.Msg) ([]byte, error) { signMsg, err := bldr.Build(msgs) if err != nil { return nil, err From 8df5dbe447b7de55c7efb9b87edf8c639f51828d Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Fri, 1 Feb 2019 10:35:14 -0500 Subject: [PATCH 4/7] support "simAndExec" mode for generate only txs --- client/utils/rest.go | 20 ++++++++++++++++++-- x/bank/client/rest/sendtx.go | 2 +- x/gov/client/rest/rest.go | 6 +++--- x/ibc/client/rest/transfer.go | 2 +- x/slashing/client/rest/tx.go | 2 +- x/staking/client/rest/tx.go | 6 +++--- 6 files changed, 27 insertions(+), 11 deletions(-) diff --git a/client/utils/rest.go b/client/utils/rest.go index 5063fb107104..63860841307d 100644 --- a/client/utils/rest.go +++ b/client/utils/rest.go @@ -308,13 +308,16 @@ func PostProcessResponse(w http.ResponseWriter, cdc *codec.Codec, response inter } // WriteGenerateStdTxResponse writes response for the generate only mode. -func WriteGenerateStdTxResponse(w http.ResponseWriter, cdc *codec.Codec, br BaseReq, msgs []sdk.Msg) { +func WriteGenerateStdTxResponse( + w http.ResponseWriter, cdc *codec.Codec, cliCtx context.CLIContext, br BaseReq, msgs []sdk.Msg, +) { + gasAdj, ok := ParseFloat64OrReturnBadRequest(w, br.GasAdjustment, client.DefaultGasAdjustment) if !ok { return } - _, gas, err := client.ParseGas(br.Gas) + simAndExec, gas, err := client.ParseGas(br.Gas) if err != nil { WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return @@ -325,6 +328,19 @@ func WriteGenerateStdTxResponse(w http.ResponseWriter, cdc *codec.Codec, br Base br.Simulate, br.ChainID, br.Memo, br.Fees, br.GasPrices, ) + if simAndExec { + if gasAdj < 0 { + WriteErrorResponse(w, http.StatusBadRequest, "gas adjustment must be a positive float") + return + } + + txBldr, err = EnrichWithGas(txBldr, cliCtx, msgs) + if err != nil { + WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) + return + } + } + stdMsg, err := txBldr.Build(msgs) if err != nil { WriteErrorResponse(w, http.StatusBadRequest, err.Error()) diff --git a/x/bank/client/rest/sendtx.go b/x/bank/client/rest/sendtx.go index d087e9316e99..b4ef7d7a2458 100644 --- a/x/bank/client/rest/sendtx.go +++ b/x/bank/client/rest/sendtx.go @@ -64,7 +64,7 @@ func SendRequestHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx context.CLIC } msg := bankclient.CreateMsg(fromAddr, toAddr, req.Amount) - utils.WriteGenerateStdTxResponse(w, cdc, req.BaseReq, []sdk.Msg{msg}) + utils.WriteGenerateStdTxResponse(w, cdc, cliCtx, req.BaseReq, []sdk.Msg{msg}) return } diff --git a/x/gov/client/rest/rest.go b/x/gov/client/rest/rest.go index b39d2f700f47..f00c187a1a96 100644 --- a/x/gov/client/rest/rest.go +++ b/x/gov/client/rest/rest.go @@ -102,7 +102,7 @@ func postProposalHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.Han } if req.BaseReq.GenerateOnly { - utils.WriteGenerateStdTxResponse(w, cdc, req.BaseReq, []sdk.Msg{msg}) + utils.WriteGenerateStdTxResponse(w, cdc, cliCtx, req.BaseReq, []sdk.Msg{msg}) return } @@ -146,7 +146,7 @@ func depositHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.HandlerF } if req.BaseReq.GenerateOnly { - utils.WriteGenerateStdTxResponse(w, cdc, req.BaseReq, []sdk.Msg{msg}) + utils.WriteGenerateStdTxResponse(w, cdc, cliCtx, req.BaseReq, []sdk.Msg{msg}) return } @@ -196,7 +196,7 @@ func voteHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.HandlerFunc } if req.BaseReq.GenerateOnly { - utils.WriteGenerateStdTxResponse(w, cdc, req.BaseReq, []sdk.Msg{msg}) + utils.WriteGenerateStdTxResponse(w, cdc, cliCtx, req.BaseReq, []sdk.Msg{msg}) return } diff --git a/x/ibc/client/rest/transfer.go b/x/ibc/client/rest/transfer.go index a92f9ea195e0..e732145e2a09 100644 --- a/x/ibc/client/rest/transfer.go +++ b/x/ibc/client/rest/transfer.go @@ -66,7 +66,7 @@ func TransferRequestHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx context. msg := ibc.IBCTransferMsg{IBCPacket: packet} if req.BaseReq.GenerateOnly { - utils.WriteGenerateStdTxResponse(w, cdc, req.BaseReq, []sdk.Msg{msg}) + utils.WriteGenerateStdTxResponse(w, cdc, cliCtx, req.BaseReq, []sdk.Msg{msg}) return } diff --git a/x/slashing/client/rest/tx.go b/x/slashing/client/rest/tx.go index 1c4266aa151d..672928f3f8bb 100644 --- a/x/slashing/client/rest/tx.go +++ b/x/slashing/client/rest/tx.go @@ -57,7 +57,7 @@ func unjailRequestHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx context.CL } if req.BaseReq.GenerateOnly { - utils.WriteGenerateStdTxResponse(w, cdc, req.BaseReq, []sdk.Msg{msg}) + utils.WriteGenerateStdTxResponse(w, cdc, cliCtx, req.BaseReq, []sdk.Msg{msg}) return } diff --git a/x/staking/client/rest/tx.go b/x/staking/client/rest/tx.go index 26b821f1d5f2..1710d340ad4a 100644 --- a/x/staking/client/rest/tx.go +++ b/x/staking/client/rest/tx.go @@ -76,7 +76,7 @@ func postDelegationsHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx context. } if req.BaseReq.GenerateOnly { - utils.WriteGenerateStdTxResponse(w, cdc, req.BaseReq, []sdk.Msg{msg}) + utils.WriteGenerateStdTxResponse(w, cdc, cliCtx, req.BaseReq, []sdk.Msg{msg}) return } @@ -121,7 +121,7 @@ func postRedelegationsHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx contex } if req.BaseReq.GenerateOnly { - utils.WriteGenerateStdTxResponse(w, cdc, req.BaseReq, []sdk.Msg{msg}) + utils.WriteGenerateStdTxResponse(w, cdc, cliCtx, req.BaseReq, []sdk.Msg{msg}) return } @@ -166,7 +166,7 @@ func postUnbondingDelegationsHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx } if req.BaseReq.GenerateOnly { - utils.WriteGenerateStdTxResponse(w, cdc, req.BaseReq, []sdk.Msg{msg}) + utils.WriteGenerateStdTxResponse(w, cdc, cliCtx, req.BaseReq, []sdk.Msg{msg}) return } From fa2e5d31a66ddf290969f7b2e99ab01b2bcb071f Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Fri, 1 Feb 2019 10:40:55 -0500 Subject: [PATCH 5/7] minor error cleanup --- client/utils/errors.go | 9 +++++++++ client/utils/rest.go | 4 ++-- client/utils/utils.go | 6 ++---- 3 files changed, 13 insertions(+), 6 deletions(-) create mode 100644 client/utils/errors.go diff --git a/client/utils/errors.go b/client/utils/errors.go new file mode 100644 index 000000000000..fe2dbe139e09 --- /dev/null +++ b/client/utils/errors.go @@ -0,0 +1,9 @@ +package utils + +import "errors" + +// common errors for CLI and REST clients +var ( + ErrInvalidGasAdjustment = errors.New("invalid gas adjustment") + ErrInvalidSigner = errors.New("tx intended signer does not match the given signer") +) diff --git a/client/utils/rest.go b/client/utils/rest.go index 63860841307d..017d5e9bf2fa 100644 --- a/client/utils/rest.go +++ b/client/utils/rest.go @@ -246,7 +246,7 @@ func CompleteAndBroadcastTxREST( if baseReq.Simulate || simAndExec { if gasAdj < 0 { - WriteErrorResponse(w, http.StatusBadRequest, "gas adjustment must be a positive float") + WriteErrorResponse(w, http.StatusBadRequest, ErrInvalidGasAdjustment.Error()) return } @@ -330,7 +330,7 @@ func WriteGenerateStdTxResponse( if simAndExec { if gasAdj < 0 { - WriteErrorResponse(w, http.StatusBadRequest, "gas adjustment must be a positive float") + WriteErrorResponse(w, http.StatusBadRequest, ErrInvalidGasAdjustment.Error()) return } diff --git a/client/utils/utils.go b/client/utils/utils.go index 2dbc9d82ffa4..6dfd9e963945 100644 --- a/client/utils/utils.go +++ b/client/utils/utils.go @@ -126,8 +126,7 @@ func SignStdTx(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, name string, // check whether the address is a signer if !isTxSigner(sdk.AccAddress(addr), stdTx.GetSigners()) { - return signedStdTx, fmt.Errorf( - "The generated transaction's intended signer does not match the given signer: %q", name) + return signedStdTx, fmt.Errorf("%s: %s", ErrInvalidSigner, name) } if !offline { @@ -155,8 +154,7 @@ func SignStdTxWithSignerAddress(txBldr authtxb.TxBuilder, cliCtx context.CLICont // check whether the address is a signer if !isTxSigner(addr, stdTx.GetSigners()) { - return signedStdTx, fmt.Errorf( - "The generated transaction's intended signer does not match the given signer: %q", name) + return signedStdTx, fmt.Errorf("%s: %s", ErrInvalidSigner, name) } if !offline { From 41142e9e75e20878a6f529885f59cee4e9bb6294 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Fri, 1 Feb 2019 10:45:36 -0500 Subject: [PATCH 6/7] add pending log entry --- PENDING.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/PENDING.md b/PENDING.md index 531509b0039b..658d2c8b9d13 100644 --- a/PENDING.md +++ b/PENDING.md @@ -44,6 +44,8 @@ IMPROVEMENTS * Automatic account number and sequence population when fields are omitted * Generate only functionality no longer requires access to a local Keybase * `from` field in the `base_req` body can be a Keybase name or account address + * [\#3423](https://github.com/cosmos/cosmos-sdk/issues/3423) Allow simulation + (auto gas) to work with generate only. * Gaia CLI (`gaiacli`) From a3656f225adce8cc2c9598ddcbfe2976a492fd35 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Fri, 1 Feb 2019 18:06:27 -0500 Subject: [PATCH 7/7] move client errors to parent client package --- client/{utils => }/errors.go | 2 +- client/utils/rest.go | 4 ++-- client/utils/utils.go | 5 +++-- 3 files changed, 6 insertions(+), 5 deletions(-) rename client/{utils => }/errors.go (93%) diff --git a/client/utils/errors.go b/client/errors.go similarity index 93% rename from client/utils/errors.go rename to client/errors.go index fe2dbe139e09..d01d574292fd 100644 --- a/client/utils/errors.go +++ b/client/errors.go @@ -1,4 +1,4 @@ -package utils +package client import "errors" diff --git a/client/utils/rest.go b/client/utils/rest.go index 017d5e9bf2fa..2926a555133d 100644 --- a/client/utils/rest.go +++ b/client/utils/rest.go @@ -246,7 +246,7 @@ func CompleteAndBroadcastTxREST( if baseReq.Simulate || simAndExec { if gasAdj < 0 { - WriteErrorResponse(w, http.StatusBadRequest, ErrInvalidGasAdjustment.Error()) + WriteErrorResponse(w, http.StatusBadRequest, client.ErrInvalidGasAdjustment.Error()) return } @@ -330,7 +330,7 @@ func WriteGenerateStdTxResponse( if simAndExec { if gasAdj < 0 { - WriteErrorResponse(w, http.StatusBadRequest, ErrInvalidGasAdjustment.Error()) + WriteErrorResponse(w, http.StatusBadRequest, client.ErrInvalidGasAdjustment.Error()) return } diff --git a/client/utils/utils.go b/client/utils/utils.go index 6dfd9e963945..03463a20825b 100644 --- a/client/utils/utils.go +++ b/client/utils/utils.go @@ -6,6 +6,7 @@ import ( "io" "os" + "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" "github.com/tendermint/go-amino" @@ -126,7 +127,7 @@ func SignStdTx(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, name string, // check whether the address is a signer if !isTxSigner(sdk.AccAddress(addr), stdTx.GetSigners()) { - return signedStdTx, fmt.Errorf("%s: %s", ErrInvalidSigner, name) + return signedStdTx, fmt.Errorf("%s: %s", client.ErrInvalidSigner, name) } if !offline { @@ -154,7 +155,7 @@ func SignStdTxWithSignerAddress(txBldr authtxb.TxBuilder, cliCtx context.CLICont // check whether the address is a signer if !isTxSigner(addr, stdTx.GetSigners()) { - return signedStdTx, fmt.Errorf("%s: %s", ErrInvalidSigner, name) + return signedStdTx, fmt.Errorf("%s: %s", client.ErrInvalidSigner, name) } if !offline {