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

@@ -4,8 +4,6 @@ import (
 	"fmt"
 	"testing"
 
-	"strings"
-
 	"github.com/stretchr/testify/suite"
 
 	"github.com/cosmos/cosmos-sdk/client/tx"
@@ -26,6 +24,9 @@ type IntegrationTestSuite struct {
 
 	cfg     network.Config
 	network *network.Network
+
+	stdTx    legacytx.StdTx
+	stdTxRes sdk.TxResponse
 }
 
 func (s *IntegrationTestSuite) SetupSuite() {
@@ -39,6 +40,17 @@ func (s *IntegrationTestSuite) SetupSuite() {
 
 	_, err := s.network.WaitForHeight(1)
 	s.Require().NoError(err)
+
+	// Broadcast a StdTx used for tests.
+	s.stdTx = s.createTestStdTx(s.network.Validators[0], 1)
+	res, err := s.broadcastReq(s.stdTx, "block")
+	s.Require().NoError(err)
+
+	// NOTE: this uses amino explicitly, don't migrate it!
+	s.Require().NoError(s.cfg.LegacyAmino.UnmarshalJSON(res, &s.stdTxRes))
+	s.Require().Equal(uint32(0), s.stdTxRes.Code)
+
+	s.Require().NoError(s.network.WaitForNextBlock())
 }
 
 func (s *IntegrationTestSuite) TearDownSuite() {
@@ -107,42 +119,61 @@ func (s *IntegrationTestSuite) TestBroadcastTxRequest() {
 func (s *IntegrationTestSuite) TestQueryTxByHash() {
 	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 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(txRes.TxHash)
+	s.Require().NotEmpty(s.stdTxRes.TxHash)
 
-	s.network.WaitForNextBlock()
+	// 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)
 
-	// We now fetch the tx by has on the `/tx/{hash}` route.
-	txJSON, err := rest.GetRequest(fmt.Sprintf("%s/txs/%s", val0.APIAddress, txRes.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)
+}
+
+func (s *IntegrationTestSuite) TestQueryTxByHeight() {
+	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=100&page=1&tx.height=%d", val0.APIAddress, s.stdTxRes.Height))
+	s.Require().NoError(err)
+
+	// txJSON should contain the whole tx, we just make sure that our custom
+	// memo is there.
+	s.Require().Contains(string(txJSON), s.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, s.stdTxRes.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))
+	s.Require().Contains(string(txJSON), s.stdTx.Memo)
 }
 
 func (s *IntegrationTestSuite) TestMultipleSyncBroadcastTxRequests() {
-	// First test transaction from validator should have sequence=1 (non-genesis tx)
+	// We already sent one tx in SetupSuite with sequence=1 (non-genesis tx).
+	// Therefore, we're starting this test with sequence=2.
 	testCases := []struct {
 		desc      string
 		sequence  uint64
 		shouldErr bool
 	}{
 		{
 			"First tx (correct sequence)",
-			1,
+			2,
 			false,
 		},
 		{
 			"Second tx (correct sequence)",
-			2,
+			3,
 			false,
 		},
 		{
@@ -153,7 +184,6 @@ func (s *IntegrationTestSuite) TestMultipleSyncBroadcastTxRequests() {
 	}
 	for _, tc := range testCases {
 		s.Run(fmt.Sprintf("Case %s", tc.desc), func() {
-
 			// broadcast test with sync mode, as we want to run CheckTx to verify account sequence is correct
 			stdTx := s.createTestStdTx(s.network.Validators[0], tc.sequence)
 			res, err := s.broadcastReq(stdTx, "sync")