Fix fetch txs by height on legacy REST endpoint

x/auth/client/rest/decode.go

@@ -6,12 +6,12 @@ import (
 	"io/ioutil"
 	"net/http"
 
-	"github.com/cosmos/cosmos-sdk/x/auth/signing"
-
 	"github.com/cosmos/cosmos-sdk/client"
 	clienttx "github.com/cosmos/cosmos-sdk/client/tx"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/cosmos/cosmos-sdk/types/rest"
 	"github.com/cosmos/cosmos-sdk/x/auth/legacy/legacytx"
+	"github.com/cosmos/cosmos-sdk/x/auth/signing"
 )
 
 type (
@@ -47,8 +47,9 @@ func DecodeTxRequestHandlerFn(clientCtx client.Context) http.HandlerFunc {
 			return
 		}
 
-		stdTx, ok := convertToStdTx(w, clientCtx, txBytes)
-		if !ok {
+		stdTx, err := convertToStdTx(w, clientCtx, txBytes)
+		if err != nil {
+			// Error is already returned by convertToStdTx.
 			return
 		}
 
@@ -61,22 +62,22 @@ func DecodeTxRequestHandlerFn(clientCtx client.Context) http.HandlerFunc {
 // convertToStdTx converts tx proto binary bytes retrieved from Tendermint into
 // a StdTx. Returns the StdTx, as well as a flag denoting if the function
 // successfully converted or not.
-func convertToStdTx(w http.ResponseWriter, clientCtx client.Context, txBytes []byte) (legacytx.StdTx, bool) {
+func convertToStdTx(w http.ResponseWriter, clientCtx client.Context, txBytes []byte) (legacytx.StdTx, error) {
 	txI, err := clientCtx.TxConfig.TxDecoder()(txBytes)
 	if rest.CheckBadRequestError(w, err) {
-		return legacytx.StdTx{}, false
+		return legacytx.StdTx{}, err
 	}
 
 	tx, ok := txI.(signing.Tx)
 	if !ok {
 		rest.WriteErrorResponse(w, http.StatusBadRequest, fmt.Sprintf("%+v is not backwards compatible with %T", tx, legacytx.StdTx{}))
-		return legacytx.StdTx{}, false
+		return legacytx.StdTx{}, sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", (signing.Tx)(nil), txI)
 	}
 
 	stdTx, err := clienttx.ConvertTxToStdTx(clientCtx.LegacyAmino, tx)
 	if rest.CheckBadRequestError(w, err) {
-		return legacytx.StdTx{}, false
+		return legacytx.StdTx{}, err
 	}
 
-	return stdTx, true
+	return stdTx, nil
 }

x/auth/client/rest/query.go

@@ -9,6 +9,7 @@ import (
 	"github.com/gorilla/mux"
 
 	"github.com/cosmos/cosmos-sdk/client"
+	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/rest"
 	authclient "github.com/cosmos/cosmos-sdk/x/auth/client"
@@ -102,6 +103,10 @@ func QueryTxsRequestHandlerFn(clientCtx client.Context) http.HandlerFunc {
 			return
 		}
 
+		for _, txRes := range searchResult.Txs {
+			packStdTxResponse(w, clientCtx, txRes)
+		}
+
 		rest.PostProcessResponseBare(w, clientCtx, searchResult)
 	}
 }
@@ -128,19 +133,17 @@ func QueryTxRequestHandlerFn(clientCtx client.Context) http.HandlerFunc {
 			return
 		}
 
-		// We just unmarshalled from Tendermint, we take the proto Tx's raw
-		// bytes, and convert them into a StdTx to be displayed.
-		txBytes := output.Tx.Value
-		stdTx, ok := convertToStdTx(w, clientCtx, txBytes)
-		if !ok {
+		err = packStdTxResponse(w, clientCtx, output)
+		if err != nil {
+			// Error is already returned by packStdTxResponse.
 			return
 		}
 
 		if output.Empty() {
 			rest.WriteErrorResponse(w, http.StatusNotFound, fmt.Sprintf("no transaction found with hash %s", hashHexStr))
 		}
 
-		rest.PostProcessResponseBare(w, clientCtx, stdTx)
+		rest.PostProcessResponseBare(w, clientCtx, output)
 	}
 }
 
@@ -161,3 +164,21 @@ func queryParamsHandler(clientCtx client.Context) http.HandlerFunc {
 		rest.PostProcessResponse(w, clientCtx, res)
 	}
 }
+
+// packStdTxResponse takes a sdk.TxResponse, converts the Tx into a StdTx, and
+// packs the StdTx again into the sdk.TxResponse Any. Amino then takes care of
+// seamlessly JSON-outputting the Any.
+func packStdTxResponse(w http.ResponseWriter, clientCtx client.Context, txRes *sdk.TxResponse) error {
+	// We just unmarshalled from Tendermint, we take the proto Tx's raw
+	// bytes, and convert them into a StdTx to be displayed.
+	txBytes := txRes.Tx.Value
+	stdTx, err := convertToStdTx(w, clientCtx, txBytes)
+	if err != nil {
+		return err
+	}
+
+	// Pack the amino stdTx into the TxResponse's Any.
+	txRes.Tx = codectypes.UnsafePackAny(stdTx)
+
+	return nil
+}

x/auth/client/rest/rest_test.go

@@ -119,7 +119,7 @@ func (s *IntegrationTestSuite) TestQueryTxByHash() {
 
 	s.network.WaitForNextBlock()
 
-	// We now fetch the tx by has on the `/tx/{hash}` route.
+	// We now fetch the tx by hash on the `/tx/{hash}` route.
 	txJSON, err := rest.GetRequest(fmt.Sprintf("%s/txs/%s", val0.APIAddress, txRes.TxHash))
 	s.Require().NoError(err)
 
@@ -128,6 +128,40 @@ func (s *IntegrationTestSuite) TestQueryTxByHash() {
 	s.Require().True(strings.Contains(string(txJSON), stdTx.Memo))
 }
 
+func (s *IntegrationTestSuite) TestQueryTxByHeight() {
+	val0 := s.network.Validators[0]
+
+	// Create and broadcast a tx.
+	stdTx := s.createTestStdTx(val0, 1) // Validator's sequence starts at 1.
+	res, err := s.broadcastReq(stdTx, "block")
+	s.Require().NoError(err)
+	var txRes sdk.TxResponse
+	// NOTE: this uses amino explicitly, don't migrate it!
+	s.Require().NoError(s.cfg.LegacyAmino.UnmarshalJSON(res, &txRes))
+	// we just check for a non-empty height here, as we'll need to for querying.
+	s.Require().NotEmpty(txRes.Height)
+
+	s.network.WaitForNextBlock()
+
+	// We now fetch the tx on `/txs` route, filtering by `tx.height`
+	txJSON, err := rest.GetRequest(fmt.Sprintf("%s/txs?limit=100&page=1&tx.height=%d", val0.APIAddress, txRes.Height))
+	s.Require().NoError(err)
+
+	// txJSON should contain the whole tx, we just make sure that our custom
+	// memo is there.
+	s.Require().True(strings.Contains(string(txJSON), stdTx.Memo))
+
+	// We now fetch the tx on `/txs` route, filtering by `height`
+	txJSON, err = rest.GetRequest(fmt.Sprintf("%s/txs?height=%d", val0.APIAddress, txRes.Height))
+	s.Require().NoError(err)
+
+	fmt.Println(string(txJSON)) // TODO This one is empty.
+
+	// txJSON should contain the whole tx, we just make sure that our custom
+	// memo is there.
+	s.Require().True(strings.Contains(string(txJSON), stdTx.Memo))
+}
+
 func (s *IntegrationTestSuite) TestMultipleSyncBroadcastTxRequests() {
 	// First test transaction from validator should have sequence=1 (non-genesis tx)
 	testCases := []struct {