generalize query response with height (#4573)

Addition to #4536, no longer specific to account queries.
Allows for validator endpoints to return height in the response.

Closes: #4609

Author: colin-axner

.pending/improvements/sdk/4573-adds-height-in-

@@ -0,0 +1 @@
+#4573 Returns height in response for query endpoints.

types/rest/rest.go

@@ -3,6 +3,7 @@
 package rest
 
 import (
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io/ioutil"
@@ -222,9 +223,17 @@ func ParseQueryHeightOrReturnBadRequest(w http.ResponseWriter, cliCtx context.CL
 }
 
 // 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
 
+	if cliCtx.Height < 0 {
+		WriteErrorResponse(w, http.StatusInternalServerError, fmt.Errorf("negative height in response").Error())
+		return
+	}
+
 	switch response.(type) {
 	case []byte:
 		output = response.([]byte)
@@ -236,6 +245,32 @@ func PostProcessResponse(w http.ResponseWriter, cliCtx context.CLIContext, respo
 		} else {
 			output, err = cliCtx.Codec.MarshalJSON(response)
 		}
+
+		if err != nil {
+			WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
+			return
+		}
+	}
+
+	// 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
+		}
+
+		m["height"] = cliCtx.Height
+
+		if cliCtx.Indent {
+			output, err = json.MarshalIndent(m, "", "  ")
+		} else {
+			output, err = json.Marshal(m)
+		}
 		if err != nil {
 			WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return

types/rest/rest_test.go

@@ -3,14 +3,20 @@
 package rest
 
 import (
+	"encoding/json"
 	"io"
+	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
+	"strconv"
 	"testing"
 
-	"github.com/cosmos/cosmos-sdk/client/context"
 	"github.com/stretchr/testify/require"
+	"github.com/tendermint/tendermint/crypto"
+	"github.com/tendermint/tendermint/crypto/secp256k1"
 
+	"github.com/cosmos/cosmos-sdk/client/context"
+	"github.com/cosmos/cosmos-sdk/codec"
 	"github.com/cosmos/cosmos-sdk/types"
 )
 
@@ -138,6 +144,108 @@ func TestParseQueryHeight(t *testing.T) {
 	}
 }
 
+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"`
+		PubKey        crypto.PubKey    `json:"public_key"`
+		AccountNumber uint64           `json:"account_number"`
+		Sequence      uint64           `json:"sequence"`
+	}
+
+	// setup
+	ctx := context.NewCLIContext()
+	height := int64(194423)
+
+	privKey := secp256k1.GenPrivKey()
+	pubKey := privKey.PubKey()
+	addr := types.AccAddress(pubKey.Address())
+	coins := types.NewCoins(types.NewCoin("atom", types.NewInt(100)), types.NewCoin("tree", types.NewInt(125)))
+	accNumber := uint64(104)
+	sequence := uint64(32)
+
+	acc := mockAccount{addr, coins, pubKey, accNumber, sequence}
+	cdc := codec.New()
+	codec.RegisterCrypto(cdc)
+	cdc.RegisterConcrete(&mockAccount{}, "cosmos-sdk/mockAccount", nil)
+	ctx = ctx.WithCodec(cdc)
+
+	// setup expected json responses with zero height
+	jsonNoHeight, err := cdc.MarshalJSON(acc)
+	require.Nil(t, err)
+	require.NotNil(t, jsonNoHeight)
+	jsonIndentNoHeight, err := cdc.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)
+	require.Nil(t, err)
+	jsonMap, err := json.Marshal(m)
+	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)
+	// check height with indent
+	runPostProcessResponse(t, ctx, acc, jsonIndentWithHeight, 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,
+) {
+	if indent {
+		ctx.Indent = indent
+	}
+
+	// test using regular struct
+	w := httptest.NewRecorder()
+	PostProcessResponse(w, ctx, obj)
+	require.Equal(t, http.StatusOK, w.Code, w.Body)
+	resp := w.Result()
+	body, err := ioutil.ReadAll(resp.Body)
+	require.Nil(t, err)
+	require.Equal(t, expectedBody, body)
+
+	var marshalled []byte
+	if indent {
+		marshalled, err = ctx.Codec.MarshalJSONIndent(obj, "", "  ")
+	} else {
+		marshalled, err = ctx.Codec.MarshalJSON(obj)
+	}
+	require.Nil(t, err)
+
+	// test using marshalled struct
+	w = httptest.NewRecorder()
+	PostProcessResponse(w, ctx, marshalled)
+	require.Equal(t, http.StatusOK, w.Code, w.Body)
+	resp = w.Result()
+	body, err = ioutil.ReadAll(resp.Body)
+	require.Nil(t, err)
+	require.Equal(t, expectedBody, body)
+}
+
 func mustNewRequest(t *testing.T, method, url string, body io.Reader) *http.Request {
 	req, err := http.NewRequest(method, url, body)
 	require.NoError(t, err)

x/auth/client/rest/query.go

@@ -11,17 +11,9 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/rest"
 	"github.com/cosmos/cosmos-sdk/x/auth/client/utils"
-	"github.com/cosmos/cosmos-sdk/x/auth/exported"
 	"github.com/cosmos/cosmos-sdk/x/auth/types"
 )
 
-// AccountWithHeight wraps the embedded Account with the height it was queried
-// at.
-type AccountWithHeight struct {
-	exported.Account `json:"account"`
-	Height           int64 `json:"height"`
-}
-
 // query accountREST Handler
 func QueryAccountRequestHandlerFn(storeName string, cliCtx context.CLIContext) http.HandlerFunc {
 
@@ -47,13 +39,14 @@ func QueryAccountRequestHandlerFn(storeName string, cliCtx context.CLIContext) h
 			return
 		}
 
-		account, err := accGetter.GetAccount(addr)
+		account, height, err := accGetter.GetAccountWithHeight(addr)
 		if err != nil {
 			rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
 		}
 
-		rest.PostProcessResponse(w, cliCtx, AccountWithHeight{account, cliCtx.Height})
+		cliCtx = cliCtx.WithHeight(height)
+		rest.PostProcessResponse(w, cliCtx, account)
 	}
 }
 

x/auth/types/account_retriever.go

@@ -29,22 +29,30 @@ func NewAccountRetriever(querier NodeQuerier) AccountRetriever {
 // GetAccount queries for an account given an address and a block height. An
 // error is returned if the query or decoding fails.
 func (ar AccountRetriever) GetAccount(addr sdk.AccAddress) (exported.Account, error) {
+	account, _, err := ar.GetAccountWithHeight(addr)
+	return account, err
+}
+
+// GetAccountWithHeight queries for an account given an address. Returns the
+// height of the query with the account. An error is returned if the query
+// or decoding fails.
+func (ar AccountRetriever) GetAccountWithHeight(addr sdk.AccAddress) (exported.Account, int64, error) {
 	bs, err := ModuleCdc.MarshalJSON(NewQueryAccountParams(addr))
 	if err != nil {
-		return nil, err
+		return nil, 0, err
 	}
 
-	res, _, err := ar.querier.QueryWithData(fmt.Sprintf("custom/%s/%s", QuerierRoute, QueryAccount), bs)
+	res, height, err := ar.querier.QueryWithData(fmt.Sprintf("custom/%s/%s", QuerierRoute, QueryAccount), bs)
 	if err != nil {
-		return nil, err
+		return nil, 0, err
 	}
 
 	var account exported.Account
 	if err := ModuleCdc.UnmarshalJSON(res, &account); err != nil {
-		return nil, err
+		return nil, 0, err
 	}
 
-	return account, nil
+	return account, height, nil
 }
 
 // EnsureExists returns an error if no account exists for the given address else nil.

x/bank/client/rest/query.go

@@ -37,12 +37,14 @@ func QueryBalancesRequestHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
 			return
 		}
 
-		res, _, err := cliCtx.QueryWithData("custom/bank/balances", bz)
+		res, height, err := cliCtx.QueryWithData("custom/bank/balances", bz)
 		if err != nil {
 			rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
 		}
 
+		cliCtx = cliCtx.WithHeight(height)
+
 		// the query will return empty if there is no data for this account
 		if len(res) == 0 {
 			rest.PostProcessResponse(w, cliCtx, sdk.Coins{})

x/distribution/client/rest/query.go

@@ -115,12 +115,13 @@ func delegatorWithdrawalAddrHandlerFn(cliCtx context.CLIContext, queryRoute stri
 		}
 
 		bz := cliCtx.Codec.MustMarshalJSON(types.NewQueryDelegatorWithdrawAddrParams(delegatorAddr))
-		res, _, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/withdraw_addr", queryRoute), bz)
+		res, height, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/withdraw_addr", queryRoute), bz)
 		if err != nil {
 			rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
 		}
 
+		cliCtx = cliCtx.WithHeight(height)
 		rest.PostProcessResponse(w, cliCtx, res)
 	}
 }
@@ -232,7 +233,7 @@ func communityPoolHandler(cliCtx context.CLIContext, queryRoute string) http.Han
 			return
 		}
 
-		res, _, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/community_pool", queryRoute), nil)
+		res, height, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/community_pool", queryRoute), nil)
 		if err != nil {
 			rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
@@ -244,6 +245,7 @@ func communityPoolHandler(cliCtx context.CLIContext, queryRoute string) http.Han
 			return
 		}
 
+		cliCtx = cliCtx.WithHeight(height)
 		rest.PostProcessResponse(w, cliCtx, result)
 	}
 }
@@ -262,12 +264,13 @@ func outstandingRewardsHandlerFn(cliCtx context.CLIContext, queryRoute string) h
 		}
 
 		bin := cliCtx.Codec.MustMarshalJSON(types.NewQueryValidatorOutstandingRewardsParams(validatorAddr))
-		res, _, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/validator_outstanding_rewards", queryRoute), bin)
+		res, height, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/validator_outstanding_rewards", queryRoute), bin)
 		if err != nil {
 			rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
 		}
 
+		cliCtx = cliCtx.WithHeight(height)
 		rest.PostProcessResponse(w, cliCtx, res)
 	}
 }

x/distribution/types/delegator.go

@@ -14,7 +14,7 @@ import (
 type DelegatorStartingInfo struct {
 	PreviousPeriod uint64  `json:"previous_period"` // period at which the delegation should withdraw starting from
 	Stake          sdk.Dec `json:"stake"`           // amount of staking token delegated
-	Height         uint64  `json:"height"`          // height at which delegation was created
+	Height         uint64  `json:"creation_height"` // height at which delegation was created
 }
 
 // create a new DelegatorStartingInfo

x/gov/client/rest/rest.go

@@ -201,12 +201,13 @@ func queryParamsHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
 			return
 		}
 
-		res, _, err := cliCtx.QueryWithData(fmt.Sprintf("custom/gov/%s/%s", types.QueryParams, paramType), nil)
+		res, height, err := cliCtx.QueryWithData(fmt.Sprintf("custom/gov/%s/%s", types.QueryParams, paramType), nil)
 		if err != nil {
 			rest.WriteErrorResponse(w, http.StatusNotFound, err.Error())
 			return
 		}
 
+		cliCtx = cliCtx.WithHeight(height)
 		rest.PostProcessResponse(w, cliCtx, res)
 	}
 }
@@ -240,12 +241,13 @@ func queryProposalHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
 			return
 		}
 
-		res, _, err := cliCtx.QueryWithData("custom/gov/proposal", bz)
+		res, height, err := cliCtx.QueryWithData("custom/gov/proposal", bz)
 		if err != nil {
 			rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
 		}
 
+		cliCtx = cliCtx.WithHeight(height)
 		rest.PostProcessResponse(w, cliCtx, res)
 	}
 }
@@ -607,12 +609,13 @@ func queryProposalsWithParameterFn(cliCtx context.CLIContext) http.HandlerFunc {
 			return
 		}
 
-		res, _, err := cliCtx.QueryWithData("custom/gov/proposals", bz)
+		res, height, err := cliCtx.QueryWithData("custom/gov/proposals", bz)
 		if err != nil {
 			rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
 		}
 
+		cliCtx = cliCtx.WithHeight(height)
 		rest.PostProcessResponse(w, cliCtx, res)
 	}
 }
@@ -647,12 +650,13 @@ func queryTallyOnProposalHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
 			return
 		}
 
-		res, _, err := cliCtx.QueryWithData("custom/gov/tally", bz)
+		res, height, err := cliCtx.QueryWithData("custom/gov/tally", bz)
 		if err != nil {
 			rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
 		}
 
+		cliCtx = cliCtx.WithHeight(height)
 		rest.PostProcessResponse(w, cliCtx, res)
 	}
 }