diff --git a/docs/migrations/rest.md b/docs/migrations/rest.md index 4de2946faaf2..d418587d0866 100644 --- a/docs/migrations/rest.md +++ b/docs/migrations/rest.md @@ -16,6 +16,7 @@ Some important information concerning all legacy REST endpoints: | Legacy REST Endpoint | Description | Breaking Change | | ------------------------- | ------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `POST /txs` | Query tx by hash | Endpoint will error when trying to broadcast transactions that don't support Amino serialization (e.g. IBC txs)1. | | `GET /txs/{hash}` | Query tx by hash | Endpoint will error when trying to output transactions that don't support Amino serialization (e.g. IBC txs)1. | | `GET /txs` | Query tx by events | Endpoint will error when trying to output transactions that don't support Amino serialization (e.g. IBC txs)1. | | `GET /staking/validators` | Get all validators | BondStatus is now a protobuf enum instead of an int32, and JSON serialized using its protobuf name, so expect query parameters like `?status=BOND_STATUS_{BONDED,UNBONDED,UNBONDING}` as opposed to `?status={bonded,unbonded,unbonding}`. | @@ -87,3 +88,7 @@ Some modules expose legacy `POST` endpoints to generate unsigned transactions fo | `POST /upgrade/*` | Create unsigned `Msg`s for upgrade | N/A, use Protobuf directly | | `GET /upgrade/current` | Get the current plan | `GET /cosmos/upgrade/v1beta1/current_plan` | | `GET /upgrade/applied_plan/{name}` | Get a previously applied plan | `GET /cosmos/upgrade/v1beta1/applied/{name}` | + +## Migrating to gRPC + +Instead of hitting REST endpoints as described in the previous paragraph, the SDK also exposes a gRPC server. Any client can use gRPC instead of REST to interact with the node. An overview of different ways to communicate with a node can be found [here (TODO)](https://github.com/cosmos/cosmos-sdk/issues/7657), and a concrete tutorial for setting up a gRPC client [here (TODO)](https://github.com/cosmos/cosmos-sdk/issues/7657). diff --git a/x/auth/client/rest/decode.go b/x/auth/client/rest/decode.go index d048f91b4c39..3061d71cf488 100644 --- a/x/auth/client/rest/decode.go +++ b/x/auth/client/rest/decode.go @@ -55,9 +55,10 @@ func DecodeTxRequestHandlerFn(clientCtx client.Context) http.HandlerFunc { response := DecodeResp(stdTx) - err = checkSignModeError(w, clientCtx, response, "/cosmos/tx/v1beta1/txs/decode") + err = checkSignModeError(clientCtx, response, "/cosmos/tx/v1beta1/txs/decode") if err != nil { - // Error is already returned by checkSignModeError. + rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) + return } diff --git a/x/auth/client/rest/query.go b/x/auth/client/rest/query.go index 674e24a0082a..f9abae6b71fe 100644 --- a/x/auth/client/rest/query.go +++ b/x/auth/client/rest/query.go @@ -18,8 +18,6 @@ import ( genutilrest "github.com/cosmos/cosmos-sdk/x/genutil/client/rest" ) -const unRegisteredConcreteTypeErr = "unregistered concrete type" - // QueryAccountRequestHandlerFn is the query accountREST Handler. func QueryAccountRequestHandlerFn(storeName string, clientCtx client.Context) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { @@ -110,9 +108,10 @@ func QueryTxsRequestHandlerFn(clientCtx client.Context) http.HandlerFunc { packStdTxResponse(w, clientCtx, txRes) } - err = checkSignModeError(w, clientCtx, searchResult, "/cosmos/tx/v1beta1/txs") + err = checkSignModeError(clientCtx, searchResult, "/cosmos/tx/v1beta1/txs") if err != nil { - // Error is already returned by checkSignModeError. + rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) + return } @@ -152,9 +151,10 @@ func QueryTxRequestHandlerFn(clientCtx client.Context) http.HandlerFunc { rest.WriteErrorResponse(w, http.StatusNotFound, fmt.Sprintf("no transaction found with hash %s", hashHexStr)) } - err = checkSignModeError(w, clientCtx, output, "/cosmos/tx/v1beta1/tx/{txhash}") + err = checkSignModeError(clientCtx, output, "/cosmos/tx/v1beta1/tx/{txhash}") if err != nil { - // Error is already returned by checkSignModeError. + rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) + return } @@ -198,18 +198,22 @@ func packStdTxResponse(w http.ResponseWriter, clientCtx client.Context, txRes *s return nil } -func checkSignModeError(w http.ResponseWriter, ctx client.Context, resp interface{}, grpcEndPoint string) error { +// checkSignModeError checks if there are errors with marshalling non-amino +// txs with amino. +func checkSignModeError(ctx client.Context, resp interface{}, grpcEndPoint string) error { // LegacyAmino used intentionally here to handle the SignMode errors marshaler := ctx.LegacyAmino _, err := marshaler.MarshalJSON(resp) - if err != nil && strings.Contains(err.Error(), unRegisteredConcreteTypeErr) { - rest.WriteErrorResponse(w, http.StatusInternalServerError, - "This transaction was created with the new SIGN_MODE_DIRECT signing method, and therefore cannot be displayed"+ - " via legacy REST handlers, please use CLI or directly query the Tendermint RPC endpoint to query"+ - " this transaction. gRPC gateway endpoint is "+grpcEndPoint+". Please also see the REST endpoints migration"+ - " guide at "+clientrest.DeprecationURL+".") - return err + if err != nil { + + // If there's an unmarshalling error, we assume that it's because we're + // using amino to unmarshal a non-amino tx. + return fmt.Errorf("this transaction cannot be displayed via legacy REST endpoints, because it does not support"+ + " Amino serialization. Please either use CLI, gRPC, gRPC-gateway, or directly query the Tendermint RPC"+ + " endpoint to query this transaction. The new REST endpoint (via gRPC-gateway) is %s. Please also see the"+ + "REST endpoints migration guide at %s for more info", grpcEndPoint, clientrest.DeprecationURL) + } return nil diff --git a/x/auth/client/rest/rest_test.go b/x/auth/client/rest/rest_test.go index 889067d852a3..1d7cac6848bf 100644 --- a/x/auth/client/rest/rest_test.go +++ b/x/auth/client/rest/rest_test.go @@ -4,6 +4,7 @@ import ( "fmt" "testing" + "github.com/spf13/cobra" "github.com/stretchr/testify/suite" "github.com/cosmos/cosmos-sdk/client/flags" @@ -16,16 +17,16 @@ import ( "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/rest" + txtypes "github.com/cosmos/cosmos-sdk/types/tx" "github.com/cosmos/cosmos-sdk/types/tx/signing" authclient "github.com/cosmos/cosmos-sdk/x/auth/client" authcli "github.com/cosmos/cosmos-sdk/x/auth/client/cli" - bankcli "github.com/cosmos/cosmos-sdk/x/bank/client/testutil" - ibccli "github.com/cosmos/cosmos-sdk/x/ibc/core/04-channel/client/cli" - - txtypes "github.com/cosmos/cosmos-sdk/types/tx" rest2 "github.com/cosmos/cosmos-sdk/x/auth/client/rest" "github.com/cosmos/cosmos-sdk/x/auth/legacy/legacytx" + bankcli "github.com/cosmos/cosmos-sdk/x/bank/client/testutil" "github.com/cosmos/cosmos-sdk/x/bank/types" + ibccli "github.com/cosmos/cosmos-sdk/x/ibc/core/04-channel/client/cli" + ibcsolomachinecli "github.com/cosmos/cosmos-sdk/x/ibc/light-clients/06-solomachine/client/cli" ) type IntegrationTestSuite struct { @@ -129,51 +130,99 @@ func (s *IntegrationTestSuite) TestBroadcastTxRequest() { s.Require().NotEmpty(txRes.TxHash) } -func (s *IntegrationTestSuite) TestQueryTxByHash() { +// Helper function to test querying txs. We will use it to query StdTx and service `Msg`s. +func (s *IntegrationTestSuite) testQueryTx(txHeight int64, txHash, txRecipient string) { val0 := s.network.Validators[0] - // We broadcasted a StdTx in SetupSuite. - // we just check for a non-empty TxHash here, the actual hash will depend on the underlying tx configuration - s.Require().NotEmpty(s.stdTxRes.TxHash) - - // We now fetch the tx by hash on the `/tx/{hash}` route. - txJSON, err := rest.GetRequest(fmt.Sprintf("%s/txs/%s", val0.APIAddress, s.stdTxRes.TxHash)) - s.Require().NoError(err) + testCases := []struct { + desc string + malleate func() *sdk.TxResponse + }{ + { + "Query by hash", + func() *sdk.TxResponse { + txJSON, err := rest.GetRequest(fmt.Sprintf("%s/txs/%s", val0.APIAddress, txHash)) + s.Require().NoError(err) + + var txResAmino sdk.TxResponse + s.Require().NoError(val0.ClientCtx.LegacyAmino.UnmarshalJSON(txJSON, &txResAmino)) + return &txResAmino + }, + }, + { + "Query by height", + func() *sdk.TxResponse { + txJSON, err := rest.GetRequest(fmt.Sprintf("%s/txs?limit=10&page=1&tx.height=%d", val0.APIAddress, txHeight)) + s.Require().NoError(err) + + var searchtxResult sdk.SearchTxsResult + s.Require().NoError(val0.ClientCtx.LegacyAmino.UnmarshalJSON(txJSON, &searchtxResult)) + s.Require().Len(searchtxResult.Txs, 1) + return searchtxResult.Txs[0] + }, + }, + { + "Query by event (transfer.recipient)", + func() *sdk.TxResponse { + txJSON, err := rest.GetRequest(fmt.Sprintf("%s/txs?transfer.recipient=%s", val0.APIAddress, txRecipient)) + s.Require().NoError(err) + + var searchtxResult sdk.SearchTxsResult + s.Require().NoError(val0.ClientCtx.LegacyAmino.UnmarshalJSON(txJSON, &searchtxResult)) + s.Require().Len(searchtxResult.Txs, 1) + return searchtxResult.Txs[0] + }, + }, + } - // txJSON should contain the whole tx, we just make sure that our custom - // memo is there. - s.Require().Contains(string(txJSON), s.stdTx.Memo) + for _, tc := range testCases { + s.Run(fmt.Sprintf("Case %s", tc.desc), func() { + txResponse := tc.malleate() + + // Check that the height is correct. + s.Require().Equal(txHeight, txResponse.Height) + + // Check that the events are correct. + s.Require().Contains( + txResponse.RawLog, + fmt.Sprintf("{\"key\":\"recipient\",\"value\":\"%s\"}", txRecipient), + ) + + // Check that the Msg is correct. + stdTx, ok := txResponse.Tx.GetCachedValue().(legacytx.StdTx) + s.Require().True(ok) + msgs := stdTx.GetMsgs() + s.Require().Equal(len(msgs), 1) + msg, ok := msgs[0].(*types.MsgSend) + s.Require().True(ok) + s.Require().Equal(txRecipient, msg.ToAddress) + }) + } } -func (s *IntegrationTestSuite) TestQueryTxByHeight() { +func (s *IntegrationTestSuite) TestQueryTxWithStdTx() { val0 := s.network.Validators[0] // We broadcasted a StdTx in SetupSuite. - // we just check for a non-empty height here, as we'll need to for querying. - s.Require().NotEmpty(s.stdTxRes.Height) - - // We now fetch the tx on `/txs` route, filtering by `tx.height` - txJSON, err := rest.GetRequest(fmt.Sprintf("%s/txs?limit=10&page=1&tx.height=%d", val0.APIAddress, s.stdTxRes.Height)) - s.Require().NoError(err) + // We just check for a non-empty TxHash here, the actual hash will depend on the underlying tx configuration + s.Require().NotEmpty(s.stdTxRes.TxHash) - // txJSON should contain the whole tx, we just make sure that our custom - // memo is there. - s.Require().Contains(string(txJSON), s.stdTx.Memo) + s.testQueryTx(s.stdTxRes.Height, s.stdTxRes.TxHash, val0.Address.String()) } -func (s *IntegrationTestSuite) TestQueryTxByHashWithServiceMessage() { +func (s *IntegrationTestSuite) TestQueryTxWithServiceMessage() { val := s.network.Validators[0] sendTokens := sdk.NewInt64Coin(s.cfg.BondDenom, 10) + _, _, addr := testdata.KeyTestPubAddr() - // Right after this line, we're sending a tx. Might need to wait a block - // to refresh sequences. + // Might need to wait a block to refresh sequences from previous setups. s.Require().NoError(s.network.WaitForNextBlock()) out, err := bankcli.ServiceMsgSendExec( val.ClientCtx, val.Address, - val.Address, + addr, sdk.NewCoins(sendTokens), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), @@ -188,17 +237,7 @@ func (s *IntegrationTestSuite) TestQueryTxByHashWithServiceMessage() { s.Require().NoError(s.network.WaitForNextBlock()) - txJSON, err := rest.GetRequest(fmt.Sprintf("%s/txs/%s", val.APIAddress, txRes.TxHash)) - s.Require().NoError(err) - - var txResAmino sdk.TxResponse - s.Require().NoError(val.ClientCtx.LegacyAmino.UnmarshalJSON(txJSON, &txResAmino)) - stdTx, ok := txResAmino.Tx.GetCachedValue().(legacytx.StdTx) - s.Require().True(ok) - msgs := stdTx.GetMsgs() - s.Require().Equal(len(msgs), 1) - _, ok = msgs[0].(*types.MsgSend) - s.Require().True(ok) + s.testQueryTx(txRes.Height, txRes.TxHash, addr.String()) } func (s *IntegrationTestSuite) TestMultipleSyncBroadcastTxRequests() { @@ -303,42 +342,43 @@ func (s *IntegrationTestSuite) broadcastReq(stdTx legacytx.StdTx, mode string) ( return rest.PostRequest(fmt.Sprintf("%s/txs", val.APIAddress), "application/json", bz) } -func (s *IntegrationTestSuite) TestLegacyRestErrMessages() { +// testQueryIBCTx is a helper function to test querying txs which: +// - show an error message on legacy REST endpoints +// - succeed using gRPC +// In practise, we call this function on IBC txs. +func (s *IntegrationTestSuite) testQueryIBCTx(txRes sdk.TxResponse, cmd *cobra.Command, args []string) { val := s.network.Validators[0] - args := []string{ - "121", // dummy port-id - "channel-0", // dummy channel-id - fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), - fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), - fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()), - fmt.Sprintf("--gas=%d", flags.DefaultGasLimit), - fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()), - fmt.Sprintf("--%s=foobar", flags.FlagMemo), - } - - // created a dummy txn for IBC, eventually it fails. Our intension is to test the error message of querying a - // message which is signed with proto, since IBC won't support legacy amino at all we are considering a message from IBC module. - out, err := clitestutil.ExecTestCLICmd(val.ClientCtx, ibccli.NewChannelCloseInitCmd(), args) - s.Require().NoError(err) - - var txRes sdk.TxResponse - s.Require().NoError(val.ClientCtx.JSONMarshaler.UnmarshalJSON(out.Bytes(), &txRes)) + errMsg := "this transaction cannot be displayed via legacy REST endpoints, because it does not support" + + " Amino serialization. Please either use CLI, gRPC, gRPC-gateway, or directly query the Tendermint RPC" + + " endpoint to query this transaction. The new REST endpoint (via gRPC-gateway) is " - s.Require().NoError(s.network.WaitForNextBlock()) - - // try to fetch the txn using legacy rest, this won't work since the ibc module doesn't support amino. - txJSON, err := rest.GetRequest(fmt.Sprintf("%s/txs/%s", val.APIAddress, txRes.TxHash)) - s.Require().NoError(err) + // Test that legacy endpoint return the above error message on IBC txs. + testCases := []struct { + desc string + url string + }{ + { + "Query by hash", + fmt.Sprintf("%s/txs/%s", val.APIAddress, txRes.TxHash), + }, + { + "Query by height", + fmt.Sprintf("%s/txs?tx.height=%d", val.APIAddress, txRes.Height), + }, + } - var errResp rest.ErrorResponse - s.Require().NoError(val.ClientCtx.LegacyAmino.UnmarshalJSON(txJSON, &errResp)) + for _, tc := range testCases { + s.Run(fmt.Sprintf("Case %s", tc.desc), func() { + txJSON, err := rest.GetRequest(tc.url) + s.Require().NoError(err) - errMsg := "This transaction was created with the new SIGN_MODE_DIRECT signing method, " + - "and therefore cannot be displayed via legacy REST handlers, please use CLI or directly query the Tendermint " + - "RPC endpoint to query this transaction." + var errResp rest.ErrorResponse + s.Require().NoError(val.ClientCtx.LegacyAmino.UnmarshalJSON(txJSON, &errResp)) - s.Require().Contains(errResp.Error, errMsg) + s.Require().Contains(errResp.Error, errMsg) + }) + } // try fetching the txn using gRPC req, it will fetch info since it has proto codec. grpcJSON, err := rest.GetRequest(fmt.Sprintf("%s/cosmos/tx/v1beta1/tx/%s", val.APIAddress, txRes.TxHash)) @@ -350,7 +390,7 @@ func (s *IntegrationTestSuite) TestLegacyRestErrMessages() { // generate broadcast only txn. args = append(args, fmt.Sprintf("--%s=true", flags.FlagGenerateOnly)) - out, err = clitestutil.ExecTestCLICmd(val.ClientCtx, ibccli.NewChannelCloseInitCmd(), args) + out, err := clitestutil.ExecTestCLICmd(val.ClientCtx, cmd, args) s.Require().NoError(err) txFile, cleanup := testutil.WriteToNewTempFile(s.T(), string(out.Bytes())) @@ -368,10 +408,80 @@ func (s *IntegrationTestSuite) TestLegacyRestErrMessages() { res, err := rest.PostRequest(fmt.Sprintf("%s/txs/decode", val.APIAddress), "application/json", bz) s.Require().NoError(err) + var errResp rest.ErrorResponse s.Require().NoError(val.ClientCtx.LegacyAmino.UnmarshalJSON(res, &errResp)) s.Require().Contains(errResp.Error, errMsg) } +// TestLegacyRestErrMessages creates two IBC txs, one that fails, one that +// succeeds, and make sure we cannot query any of them (with pretty error msg). +// Our intension is to test the error message of querying a message which is +// signed with proto, since IBC won't support legacy amino at all we are +// considering a message from IBC module. +func (s *IntegrationTestSuite) TestLegacyRestErrMessages() { + val := s.network.Validators[0] + + // Write consensus json to temp file, used for an IBC message. + consensusJSON, cleanup := testutil.WriteToNewTempFile( + s.T(), + `{"public_key":{"@type":"/cosmos.crypto.secp256k1.PubKey","key":"A/3SXL2ONYaOkxpdR5P8tHTlSlPv1AwQwSFxKRee5JQW"},"diversifier":"diversifier","timestamp":"10"}`, + ) + defer cleanup() + + testCases := []struct { + desc string + cmd *cobra.Command + args []string + code uint32 + }{ + { + "Failing IBC message", + ibccli.NewChannelCloseInitCmd(), + []string{ + "121", // dummy port-id + "channel-0", // dummy channel-id + fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), + fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), + fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()), + fmt.Sprintf("--gas=%d", flags.DefaultGasLimit), + fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()), + fmt.Sprintf("--%s=foobar", flags.FlagMemo), + }, + uint32(7), + }, + { + "Successful IBC message", + ibcsolomachinecli.NewCreateClientCmd(), + []string{ + "21212121212", // dummy client-id + "1", // dummy sequence + consensusJSON.Name(), // path to consensus json, + fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), + fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), + fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()), + fmt.Sprintf("--gas=%d", flags.DefaultGasLimit), + fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()), + fmt.Sprintf("--%s=foobar", flags.FlagMemo), + }, + uint32(0), + }, + } + + for _, tc := range testCases { + s.Run(fmt.Sprintf("Case %s", tc.desc), func() { + out, err := clitestutil.ExecTestCLICmd(val.ClientCtx, tc.cmd, tc.args) + s.Require().NoError(err) + var txRes sdk.TxResponse + s.Require().NoError(val.ClientCtx.JSONMarshaler.UnmarshalJSON(out.Bytes(), &txRes)) + s.Require().Equal(tc.code, txRes.Code) + + s.Require().NoError(s.network.WaitForNextBlock()) + + s.testQueryIBCTx(txRes, tc.cmd, tc.args) + }) + } +} + func TestIntegrationTestSuite(t *testing.T) { suite.Run(t, new(IntegrationTestSuite)) }