Merge PR cosmos#4777: Fix Height Queries

.pending/breaking/_All-REST-responses-

@@ -0,0 +1,2 @@
+All REST responses now wrap the original resource/result. The response
+will contain two fields: height and result.

.pending/bugfixes/_Return-height-in-re

@@ -0,0 +1 @@
+Return height in responses when querying against BaseApp

baseapp/baseapp.go

@@ -463,6 +463,7 @@ func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) (res abc
 			return abci.ResponseQuery{
 				Code:      uint32(sdk.CodeOK),
 				Codespace: string(sdk.CodespaceRoot),
+				Height:    req.Height,
 				Value:     []byte(app.appVersion),
 			}
 
@@ -474,6 +475,7 @@ func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) (res abc
 		return abci.ResponseQuery{
 			Code:      uint32(sdk.CodeOK),
 			Codespace: string(sdk.CodespaceRoot),
+			Height:    req.Height,
 			Value:     value,
 		}
 	}
@@ -482,7 +484,7 @@ func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) (res abc
 	return sdk.ErrUnknownRequest(msg).QueryResult()
 }
 
-func handleQueryStore(app *BaseApp, path []string, req abci.RequestQuery) (res abci.ResponseQuery) {
+func handleQueryStore(app *BaseApp, path []string, req abci.RequestQuery) abci.ResponseQuery {
 	// "/store" prefix for store queries
 	queryable, ok := app.cms.(sdk.Queryable)
 	if !ok {
@@ -491,7 +493,11 @@ func handleQueryStore(app *BaseApp, path []string, req abci.RequestQuery) (res a
 	}
 
 	req.Path = "/" + strings.Join(path[1:], "/")
-	return queryable.Query(req)
+
+	resp := queryable.Query(req)
+	resp.Height = req.Height
+
+	return resp
 }
 
 func handleQueryP2P(app *BaseApp, path []string, _ abci.RequestQuery) (res abci.ResponseQuery) {
@@ -503,9 +509,11 @@ func handleQueryP2P(app *BaseApp, path []string, _ abci.RequestQuery) (res abci.
 			switch typ {
 			case "addr":
 				return app.FilterPeerByAddrPort(arg)
+
 			case "id":
 				return app.FilterPeerByID(arg)
 			}
+
 		default:
 			msg := "Expected second parameter to be filter"
 			return sdk.ErrUnknownRequest(msg).QueryResult()
@@ -554,13 +562,15 @@ func handleQueryCustom(app *BaseApp, path []string, req abci.RequestQuery) (res
 		return abci.ResponseQuery{
 			Code:      uint32(err.Code()),
 			Codespace: string(err.Codespace()),
+			Height:    req.Height,
 			Log:       err.ABCILog(),
 		}
 	}
 
 	return abci.ResponseQuery{
-		Code:  uint32(sdk.CodeOK),
-		Value: resBytes,
+		Code:   uint32(sdk.CodeOK),
+		Height: req.Height,
+		Value:  resBytes,
 	}
 }
 

client/context/query.go

@@ -81,6 +81,16 @@ func (ctx CLIContext) query(path string, key cmn.HexBytes) (res []byte, height i
 		return res, height, err
 	}
 
+	// When a client did not provide a query height, manually query for it so it can
+	// be injected downstream into responses.
+	if ctx.Height == 0 {
+		status, err := node.Status()
+		if err != nil {
+			return res, height, err
+		}
+		ctx = ctx.WithHeight(status.SyncInfo.LatestBlockHeight)
+	}
+
 	opts := rpcclient.ABCIQueryOptions{
 		Height: ctx.Height,
 		Prove:  !ctx.TrustNode,

types/rest/rest.go

@@ -24,6 +24,21 @@ const (
 	DefaultLimit = 30 // should be consistent with tendermint/tendermint/rpc/core/pipe.go:19
 )
 
+// ResponseWithHeight defines a response object type that wraps an original
+// response with a height.
+type ResponseWithHeight struct {
+	Height int64           `json:"height"`
+	Result json.RawMessage `json:"result"`
+}
+
+// NewResponseWithHeight creates a new ResponseWithHeight instance
+func NewResponseWithHeight(height int64, result json.RawMessage) ResponseWithHeight {
+	return ResponseWithHeight{
+		Height: height,
+		Result: result,
+	}
+}
+
 // GasEstimateResponse defines a response definition for tx gas estimation.
 type GasEstimateResponse struct {
 	GasEstimate uint64 `json:"gas_estimate"`
@@ -222,28 +237,27 @@ func ParseQueryHeightOrReturnBadRequest(w http.ResponseWriter, cliCtx context.CL
 	return cliCtx, true
 }
 
-// PostProcessResponse performs post processing for a REST response.
-// If the height is greater than zero it will be injected into the body
-// of the response. An internal server error is written to the response
-// if the height is negative or an encoding/decoding error occurs.
-func PostProcessResponse(w http.ResponseWriter, cliCtx context.CLIContext, response interface{}) {
-	var output []byte
+// PostProcessResponse performs post processing for a REST response. The result
+// returned to clients will contain two fields, the height at which the resource
+// was queried at and the original result.
+func PostProcessResponse(w http.ResponseWriter, cliCtx context.CLIContext, resp interface{}) {
+	var result []byte
 
 	if cliCtx.Height < 0 {
 		WriteErrorResponse(w, http.StatusInternalServerError, fmt.Errorf("negative height in response").Error())
 		return
 	}
 
-	switch response.(type) {
+	switch resp.(type) {
 	case []byte:
-		output = response.([]byte)
+		result = resp.([]byte)
 
 	default:
 		var err error
 		if cliCtx.Indent {
-			output, err = cliCtx.Codec.MarshalJSONIndent(response, "", "  ")
+			result, err = cliCtx.Codec.MarshalJSONIndent(resp, "", "  ")
 		} else {
-			output, err = cliCtx.Codec.MarshalJSON(response)
+			result, err = cliCtx.Codec.MarshalJSON(resp)
 		}
 
 		if err != nil {
@@ -252,29 +266,22 @@ func PostProcessResponse(w http.ResponseWriter, cliCtx context.CLIContext, respo
 		}
 	}
 
-	// inject the height into the response by:
-	// - decoding into a map
-	// - adding the height to the map
-	// - encoding using standard JSON library
-	if cliCtx.Height > 0 {
-		m := make(map[string]interface{})
-		err := json.Unmarshal(output, &m)
-		if err != nil {
-			WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
-			return
-		}
+	wrappedResp := NewResponseWithHeight(cliCtx.Height, result)
 
-		m["height"] = cliCtx.Height
+	var (
+		output []byte
+		err    error
+	)
 
-		if cliCtx.Indent {
-			output, err = json.MarshalIndent(m, "", "  ")
-		} else {
-			output, err = json.Marshal(m)
-		}
-		if err != nil {
-			WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
-			return
-		}
+	if cliCtx.Indent {
+		output, err = cliCtx.Codec.MarshalJSONIndent(wrappedResp, "", "  ")
+	} else {
+		output, err = cliCtx.Codec.MarshalJSON(wrappedResp)
+	}
+
+	if err != nil {
+		WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
+		return
 	}
 
 	w.Header().Set("Content-Type", "application/json")

types/rest/rest_test.go

@@ -3,12 +3,10 @@
 package rest
 
 import (
-	"encoding/json"
 	"io"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
-	"strconv"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -148,6 +146,7 @@ func TestProcessPostResponse(t *testing.T) {
 	// mock account
 	// PubKey field ensures amino encoding is used first since standard
 	// JSON encoding will panic on crypto.PubKey
+
 	type mockAccount struct {
 		Address       types.AccAddress `json:"address"`
 		Coins         types.Coins      `json:"coins"`
@@ -173,48 +172,35 @@ func TestProcessPostResponse(t *testing.T) {
 	cdc.RegisterConcrete(&mockAccount{}, "cosmos-sdk/mockAccount", nil)
 	ctx = ctx.WithCodec(cdc)
 
-	// setup expected json responses with zero height
-	jsonNoHeight, err := cdc.MarshalJSON(acc)
+	// setup expected results
+	jsonNoIndent, err := ctx.Codec.MarshalJSON(acc)
 	require.Nil(t, err)
-	require.NotNil(t, jsonNoHeight)
-	jsonIndentNoHeight, err := cdc.MarshalJSONIndent(acc, "", "  ")
+	jsonWithIndent, err := ctx.Codec.MarshalJSONIndent(acc, "", "  ")
 	require.Nil(t, err)
-	require.NotNil(t, jsonIndentNoHeight)
-
-	// decode into map to order alphabetically
-	m := make(map[string]interface{})
-	err = json.Unmarshal(jsonNoHeight, &m)
+	respNoIndent := NewResponseWithHeight(height, jsonNoIndent)
+	respWithIndent := NewResponseWithHeight(height, jsonWithIndent)
+	expectedNoIndent, err := ctx.Codec.MarshalJSON(respNoIndent)
 	require.Nil(t, err)
-	jsonMap, err := json.Marshal(m)
+	expectedWithIndent, err := ctx.Codec.MarshalJSONIndent(respWithIndent, "", "  ")
 	require.Nil(t, err)
-	jsonWithHeight := append(append([]byte(`{"height":`), []byte(strconv.Itoa(int(height))+",")...), jsonMap[1:]...)
-	jsonIndentMap, err := json.MarshalIndent(m, "", "  ")
-	jsonIndentWithHeight := append(append([]byte(`{`+"\n "+` "height": `), []byte(strconv.Itoa(int(height))+",")...), jsonIndentMap[1:]...)
 
 	// check that negative height writes an error
 	w := httptest.NewRecorder()
 	ctx = ctx.WithHeight(-1)
 	PostProcessResponse(w, ctx, acc)
 	require.Equal(t, http.StatusInternalServerError, w.Code)
 
-	// check that zero height returns expected response
-	ctx = ctx.WithHeight(0)
-	runPostProcessResponse(t, ctx, acc, jsonNoHeight, false)
-	// check zero height with indent
-	runPostProcessResponse(t, ctx, acc, jsonIndentNoHeight, true)
 	// check that height returns expected response
 	ctx = ctx.WithHeight(height)
-	runPostProcessResponse(t, ctx, acc, jsonWithHeight, false)
+	runPostProcessResponse(t, ctx, acc, expectedNoIndent, false)
 	// check height with indent
-	runPostProcessResponse(t, ctx, acc, jsonIndentWithHeight, true)
+	runPostProcessResponse(t, ctx, acc, expectedWithIndent, true)
 }
 
 // asserts that ResponseRecorder returns the expected code and body
 // runs PostProcessResponse on the objects regular interface and on
 // the marshalled struct.
-func runPostProcessResponse(t *testing.T, ctx context.CLIContext, obj interface{},
-	expectedBody []byte, indent bool,
-) {
+func runPostProcessResponse(t *testing.T, ctx context.CLIContext, obj interface{}, expectedBody []byte, indent bool) {
 	if indent {
 		ctx.Indent = indent
 	}