diff --git a/PENDING.md b/PENDING.md index 101a306508a4..1fa2197326e1 100644 --- a/PENDING.md +++ b/PENDING.md @@ -17,6 +17,9 @@ BREAKING CHANGES * [\#3162](https://github.com/cosmos/cosmos-sdk/issues/3162) The `--gas` flag now takes `auto` instead of `simulate` in order to trigger a simulation of the tx before the actual execution. +* Gaia REST API + * [\#3176](https://github.com/cosmos/cosmos-sdk/issues/3176) `tx/sign` endpoint now expects `BaseReq` fields as nested object. + * SDK * [\#3064](https://github.com/cosmos/cosmos-sdk/issues/3064) Sanitize `sdk.Coin` denom. Coins denoms are now case insensitive, i.e. 100fooToken equals to 100FOOTOKEN. @@ -42,13 +45,16 @@ FEATURES * SDK * \#2996 Update the `AccountKeeper` to contain params used in the context of the ante handler. + * [\#3179](https://github.com/cosmos/cosmos-sdk/pull/3179) New CodeNoSignatures error code. + * Tendermint IMPROVEMENTS -* Gaia REST API (`gaiacli advanced rest-server`) +* Gaia REST API + * [\#3176](https://github.com/cosmos/cosmos-sdk/issues/3176) Validate tx/sign endpoint POST body. * Gaia CLI (`gaiacli`) @@ -72,7 +78,7 @@ IMPROVEMENTS BUG FIXES -* Gaia REST API (`gaiacli advanced rest-server`) +* Gaia REST API * Gaia CLI (`gaiacli`) * \#3141 Fix the bug in GetAccount when `len(res) == 0` and `err == nil` diff --git a/client/lcd/lcd_test.go b/client/lcd/lcd_test.go index cff3bc70dc2b..78d8aa9f29fe 100644 --- a/client/lcd/lcd_test.go +++ b/client/lcd/lcd_test.go @@ -17,6 +17,7 @@ import ( client "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/tx" + "github.com/cosmos/cosmos-sdk/client/utils" "github.com/cosmos/cosmos-sdk/cmd/gaia/app" "github.com/cosmos/cosmos-sdk/crypto/keys/mintkey" @@ -259,12 +260,10 @@ func TestCoinSendGenerateSignAndBroadcast(t *testing.T) { var signedMsg auth.StdTx payload := authrest.SignBody{ - Tx: msg, - LocalAccountName: name1, - Password: pw, - ChainID: viper.GetString(client.FlagChainID), - AccountNumber: accnum, - Sequence: sequence, + Tx: msg, + BaseReq: utils.NewBaseReq( + name1, pw, "", viper.GetString(client.FlagChainID), "", "", accnum, sequence, nil, false, false, + ), } json, err := cdc.MarshalJSON(payload) require.Nil(t, err) diff --git a/client/lcd/swagger-ui/swagger.yaml b/client/lcd/swagger-ui/swagger.yaml index 1899eb5cb709..1a92a11bece5 100644 --- a/client/lcd/swagger-ui/swagger.yaml +++ b/client/lcd/swagger-ui/swagger.yaml @@ -286,20 +286,10 @@ paths: schema: type: object properties: + base_req: + $ref: "#/definitions/BaseReq" tx: $ref: "#/definitions/StdTx" - name: - type: string - password: - type: string - chain_id: - type: string - account_number: - type: string - example: "0" - sequence: - type: string - example: "0" append_sig: type: boolean example: true diff --git a/client/lcd/test_helpers.go b/client/lcd/test_helpers.go index e6e2d2fa40a8..0bdd519f68fe 100644 --- a/client/lcd/test_helpers.go +++ b/client/lcd/test_helpers.go @@ -645,12 +645,10 @@ func getAccount(t *testing.T, port string, addr sdk.AccAddress) auth.Account { func doSign(t *testing.T, port, name, password, chainID string, accnum, sequence uint64, msg auth.StdTx) auth.StdTx { var signedMsg auth.StdTx payload := authrest.SignBody{ - Tx: msg, - LocalAccountName: name, - Password: password, - ChainID: chainID, - AccountNumber: accnum, - Sequence: sequence, + Tx: msg, + BaseReq: utils.NewBaseReq( + name, password, "", chainID, "", "", accnum, sequence, nil, false, false, + ), } json, err := cdc.MarshalJSON(payload) require.Nil(t, err) diff --git a/client/utils/rest.go b/client/utils/rest.go index e7e0f33714a9..2cdecab64c66 100644 --- a/client/utils/rest.go +++ b/client/utils/rest.go @@ -189,7 +189,7 @@ func ReadRESTReq(w http.ResponseWriter, r *http.Request, cdc *codec.Codec, req i err = cdc.UnmarshalJSON(body, req) if err != nil { - WriteErrorResponse(w, http.StatusBadRequest, err.Error()) + WriteErrorResponse(w, http.StatusBadRequest, fmt.Sprintf("failed to decode JSON payload: %s", err)) return err } diff --git a/types/errors.go b/types/errors.go index a54c9d71c528..a400fec6f95b 100644 --- a/types/errors.go +++ b/types/errors.go @@ -45,6 +45,7 @@ const ( CodeInsufficientFee CodeType = 14 CodeTooManySignatures CodeType = 15 CodeGasOverflow CodeType = 16 + CodeNoSignatures CodeType = 17 // CodespaceRoot is a codespace for error codes in this file only. // Notice that 0 is an "unset" codespace, which can be overridden with @@ -90,6 +91,8 @@ func CodeToDefaultMsg(code CodeType) string { return "insufficient fee" case CodeTooManySignatures: return "maximum numer of signatures exceeded" + case CodeNoSignatures: + return "no signatures supplied" default: return unknownCodeMsg(code) } @@ -145,6 +148,9 @@ func ErrInsufficientFee(msg string) Error { func ErrTooManySignatures(msg string) Error { return newErrorWithRootCodespace(CodeTooManySignatures, msg) } +func ErrNoSignatures(msg string) Error { + return newErrorWithRootCodespace(CodeNoSignatures, msg) +} func ErrGasOverflow(msg string) Error { return newErrorWithRootCodespace(CodeGasOverflow, msg) } diff --git a/x/auth/ante_test.go b/x/auth/ante_test.go index 78c943879320..f92e61274cee 100644 --- a/x/auth/ante_test.go +++ b/x/auth/ante_test.go @@ -71,7 +71,7 @@ func TestAnteHandlerSigErrors(t *testing.T) { require.Equal(t, expectedSigners, stdTx.GetSigners()) // Check no signatures fails - checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeUnauthorized) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeNoSignatures) // test num sigs dont match GetSigners privs, accNums, seqs = []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} diff --git a/x/auth/client/rest/sign.go b/x/auth/client/rest/sign.go index 41e95e322fc6..754a07b44e60 100644 --- a/x/auth/client/rest/sign.go +++ b/x/auth/client/rest/sign.go @@ -1,26 +1,22 @@ package rest import ( - "io/ioutil" "net/http" "github.com/cosmos/cosmos-sdk/client/context" "github.com/cosmos/cosmos-sdk/client/utils" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/crypto/keys/keyerror" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth" authtxb "github.com/cosmos/cosmos-sdk/x/auth/client/txbuilder" ) // SignBody defines the properties of a sign request's body. type SignBody struct { - Tx auth.StdTx `json:"tx"` - LocalAccountName string `json:"name"` - Password string `json:"password"` - ChainID string `json:"chain_id"` - AccountNumber uint64 `json:"account_number"` - Sequence uint64 `json:"sequence"` - AppendSig bool `json:"append_sig"` + Tx auth.StdTx `json:"tx"` + AppendSig bool `json:"append_sig"` + BaseReq utils.BaseReq `json:"base_req"` } // nolint: unparam @@ -30,21 +26,34 @@ func SignTxRequestHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.Ha return func(w http.ResponseWriter, r *http.Request) { var m SignBody - body, err := ioutil.ReadAll(r.Body) - if err != nil { + if err := utils.ReadRESTReq(w, r, cdc, &m); err != nil { utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } - err = cdc.UnmarshalJSON(body, &m) - if err != nil { + + if !m.BaseReq.ValidateBasic(w) { + return + } + + // validate tx + // discard error if it's CodeNoSignatures as the tx comes with no signatures + if err := m.Tx.ValidateBasic(); err != nil && err.Code() != sdk.CodeNoSignatures { utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return } - txBldr := authtxb.NewTxBuilder(utils.GetTxEncoder(cdc), m.AccountNumber, - m.Sequence, m.Tx.Fee.Gas, 1.0, false, m.ChainID, m.Tx.GetMemo(), m.Tx.Fee.Amount) + txBldr := authtxb.NewTxBuilder( + utils.GetTxEncoder(cdc), + m.BaseReq.AccountNumber, + m.BaseReq.Sequence, + m.Tx.Fee.Gas, + 1.0, + false, + m.BaseReq.ChainID, + m.Tx.GetMemo(), + m.Tx.Fee.Amount) - signedTx, err := txBldr.SignStdTx(m.LocalAccountName, m.Password, m.Tx, m.AppendSig) + signedTx, err := txBldr.SignStdTx(m.BaseReq.Name, m.BaseReq.Password, m.Tx, m.AppendSig) if keyerror.IsErrKeyNotFound(err) { utils.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) return diff --git a/x/auth/stdtx.go b/x/auth/stdtx.go index ceba13c6df74..07469f81b789 100644 --- a/x/auth/stdtx.go +++ b/x/auth/stdtx.go @@ -50,7 +50,7 @@ func (tx StdTx) ValidateBasic() sdk.Error { return sdk.ErrInsufficientFee(fmt.Sprintf("invalid fee %s amount provided", tx.Fee.Amount)) } if len(stdSigs) == 0 { - return sdk.ErrUnauthorized("no signers") + return sdk.ErrNoSignatures("no signers") } if len(stdSigs) != len(tx.GetSigners()) { return sdk.ErrUnauthorized("wrong number of signers") diff --git a/x/auth/stdtx_test.go b/x/auth/stdtx_test.go index 0cc6ced233ff..a1fbf4a34ba4 100644 --- a/x/auth/stdtx_test.go +++ b/x/auth/stdtx_test.go @@ -91,7 +91,7 @@ func TestTxValidateBasic(t *testing.T) { err = tx.ValidateBasic() require.Error(t, err) - require.Equal(t, sdk.CodeUnauthorized, err.Result().Code) + require.Equal(t, sdk.CodeNoSignatures, err.Result().Code) // require to fail validation when signatures do not match expected signers privs, accNums, seqs = []crypto.PrivKey{priv1}, []uint64{0, 1}, []uint64{0, 0}