Fix legacy querying tx ("unregistered type" bug)

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
 		}
 

x/auth/client/rest/query.go

@@ -17,7 +17,13 @@ import (
 	genutilrest "github.com/cosmos/cosmos-sdk/x/genutil/client/rest"
 )
 
-const unRegisteredConcreteTypeErr = "unregistered concrete type"
+// knownErrors are errors that happen on unmarshalling new proto-only txs using
+// amino. Also see https://github.com/cosmos/cosmos-sdk/issues/7639.
+var knownErrors = []string{
+	"unregistered interface",
+	"unregistered concrete type",
+	"wrong SignMode. Expected SIGN_MODE_LEGACY_AMINO_JSON, got SIGN_MODE_DIRECT",
+}
 
 // query accountREST Handler
 func QueryAccountRequestHandlerFn(storeName string, clientCtx client.Context) http.HandlerFunc {
@@ -109,9 +115,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
 		}
 
@@ -151,9 +158,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
 		}
 
@@ -197,16 +205,27 @@ 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 proto-only
+// 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)
+	if err != nil {
+		isKnownError := false
+		for _, knownError := range knownErrors {
+			if strings.Contains(err.Error(), knownError) {
+				isKnownError = true
+			}
+		}
+
+		if isKnownError {
+			return fmt.Errorf("this transaction was created with the new SIGN_MODE_DIRECT signing method, and therefore cannot be displayed"+
+				" via legacy REST handlers. 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", grpcEndPoint)
+		}
+
 		return err
 	}
 

x/auth/client/rest/rest_test.go

@@ -129,51 +129,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 +236,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() {
@@ -324,21 +362,40 @@ func (s *IntegrationTestSuite) TestLegacyRestErrMessages() {
 
 	var txRes sdk.TxResponse
 	s.Require().NoError(val.ClientCtx.JSONMarshaler.UnmarshalJSON(out.Bytes(), &txRes))
+	s.Require().NotEqual(0, txRes.Code) // It fails, see comment above.
 
 	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)
+	errMsg := "this transaction was created with the new SIGN_MODE_DIRECT signing method, and therefore cannot be displayed" +
+		" via legacy REST handlers. 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 "
 
-	var errResp rest.ErrorResponse
-	s.Require().NoError(val.ClientCtx.LegacyAmino.UnmarshalJSON(txJSON, &errResp))
+		// 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),
+		},
+	}
+
+	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))
@@ -368,6 +425,7 @@ 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)
 }