Merge PR #5508: Fix and Return Height in x/distribution REST Handlers

cosmos · Jan 10, 2020 · 2140244 · 2140244
1 parent bf41dea
commit 2140244
Show file tree

Hide file tree

Showing 5 changed files with 85 additions and 60 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -254,6 +254,7 @@ to detail this new feature and how state transitions occur.
 
 ### Bug Fixes
 
+* (rest) [\#5508](https://github.com/cosmos/cosmos-sdk/pull/5508) Fix `x/distribution` endpoints to properly return height in the response.
 * (x/genutil) [\#5499](https://github.com/cosmos/cosmos-sdk/pull/) Ensure `DefaultGenesis` returns valid and non-nil default genesis state.
 * (client) [\#5303](https://github.com/cosmos/cosmos-sdk/issues/5303) Fix ignored error in tx generate only mode.
 * (cli) [\#4763](https://github.com/cosmos/cosmos-sdk/issues/4763) Fix flag `--min-self-delegation` for staking `EditValidator`

diff --git a/x/distribution/client/cli/query.go b/x/distribution/client/cli/query.go
@@ -217,26 +217,44 @@ $ %s query distribution rewards cosmos1gghjut3ccd8ay0zduzj64hwre2fxs9ld75ru9p co
 		RunE: func(cmd *cobra.Command, args []string) error {
 			cliCtx := context.NewCLIContext().WithCodec(cdc)
 
+			// query for rewards from a particular delegation
 			if len(args) == 2 {
-				// query for rewards from a particular delegation
-				resp, err := common.QueryDelegationRewards(cliCtx, queryRoute, args[0], args[1])
+				resp, _, err := common.QueryDelegationRewards(cliCtx, queryRoute, args[0], args[1])
 				if err != nil {
 					return err
 				}
 
 				var result sdk.DecCoins
-				cdc.MustUnmarshalJSON(resp, &result)
+				if err = cdc.UnmarshalJSON(resp, &result); err != nil {
+					return fmt.Errorf("failed to unmarshal response: %w", err)
+				}
+
 				return cliCtx.PrintOutput(result)
 			}
 
+			delegatorAddr, err := sdk.AccAddressFromBech32(args[0])
+			if err != nil {
+				return err
+			}
+
+			params := types.NewQueryDelegatorParams(delegatorAddr)
+			bz, err := cdc.MarshalJSON(params)
+			if err != nil {
+				return fmt.Errorf("failed to marshal params: %w", err)
+			}
+
 			// query for delegator total rewards
-			resp, err := common.QueryDelegatorTotalRewards(cliCtx, queryRoute, args[0])
+			route := fmt.Sprintf("custom/%s/%s", queryRoute, types.QueryDelegatorTotalRewards)
+			res, _, err := cliCtx.QueryWithData(route, bz)
 			if err != nil {
 				return err
 			}
 
 			var result types.QueryDelegatorTotalRewardsResponse
-			cdc.MustUnmarshalJSON(resp, &result)
+			if err = cdc.UnmarshalJSON(res, &result); err != nil {
+				return fmt.Errorf("failed to unmarshal response: %w", err)
+			}
+
 			return cliCtx.PrintOutput(result)
 		},
 	}

diff --git a/x/distribution/client/common/common.go b/x/distribution/client/common/common.go
@@ -8,37 +8,27 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/distribution/types"
 )
 
-// QueryDelegatorTotalRewards queries delegator total rewards.
-func QueryDelegatorTotalRewards(cliCtx context.CLIContext, queryRoute, delAddr string) ([]byte, error) {
+// QueryDelegationRewards queries a delegation rewards between a delegator and a
+// validator.
+func QueryDelegationRewards(cliCtx context.CLIContext, queryRoute, delAddr, valAddr string) ([]byte, int64, error) {
 	delegatorAddr, err := sdk.AccAddressFromBech32(delAddr)
 	if err != nil {
-		return nil, err
+		return nil, 0, err
 	}
 
-	res, _, err := cliCtx.QueryWithData(
-		fmt.Sprintf("custom/%s/%s", queryRoute, types.QueryDelegatorTotalRewards),
-		cliCtx.Codec.MustMarshalJSON(types.NewQueryDelegatorParams(delegatorAddr)),
-	)
-	return res, err
-}
-
-// QueryDelegationRewards queries a delegation rewards.
-func QueryDelegationRewards(cliCtx context.CLIContext, queryRoute, delAddr, valAddr string) ([]byte, error) {
-	delegatorAddr, err := sdk.AccAddressFromBech32(delAddr)
+	validatorAddr, err := sdk.ValAddressFromBech32(valAddr)
 	if err != nil {
-		return nil, err
+		return nil, 0, err
 	}
 
-	validatorAddr, err := sdk.ValAddressFromBech32(valAddr)
+	params := types.NewQueryDelegationRewardsParams(delegatorAddr, validatorAddr)
+	bz, err := cliCtx.Codec.MarshalJSON(params)
 	if err != nil {
-		return nil, err
+		return nil, 0, fmt.Errorf("failed to marshal params: %w", err)
 	}
 
-	res, _, err := cliCtx.QueryWithData(
-		fmt.Sprintf("custom/%s/%s", queryRoute, types.QueryDelegationRewards),
-		cliCtx.Codec.MustMarshalJSON(types.NewQueryDelegationRewardsParams(delegatorAddr, validatorAddr)),
-	)
-	return res, err
+	route := fmt.Sprintf("custom/%s/%s", queryRoute, types.QueryDelegationRewards)
+	return cliCtx.QueryWithData(route, bz)
 }
 
 // QueryDelegatorValidators returns delegator's list of validators

diff --git a/x/distribution/client/common/common_test.go b/x/distribution/client/common/common_test.go
@@ -30,7 +30,7 @@ func TestQueryDelegationRewardsAddrValidation(t *testing.T) {
 	for _, tt := range tests {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			_, err := QueryDelegationRewards(ctx, "", tt.args.delAddr, tt.args.valAddr)
+			_, _, err := QueryDelegationRewards(ctx, "", tt.args.delAddr, tt.args.valAddr)
 			require.True(t, err != nil, tt.wantErr)
 		})
 	}

diff --git a/x/distribution/client/rest/query.go b/x/distribution/client/rest/query.go
@@ -73,12 +73,26 @@ func delegatorRewardsHandlerFn(cliCtx context.CLIContext, queryRoute string) htt
 			return
 		}
 
-		// query for rewards from a particular delegator
-		res, ok := checkResponseQueryDelegatorTotalRewards(w, cliCtx, queryRoute, mux.Vars(r)["delegatorAddr"])
+		delegatorAddr, ok := checkDelegatorAddressVar(w, r)
 		if !ok {
 			return
 		}
 
+		params := types.NewQueryDelegatorParams(delegatorAddr)
+		bz, err := cliCtx.Codec.MarshalJSON(params)
+		if err != nil {
+			rest.WriteErrorResponse(w, http.StatusBadRequest, fmt.Sprintf("failed to marshal params: %s", err))
+			return
+		}
+
+		route := fmt.Sprintf("custom/%s/%s", queryRoute, types.QueryDelegatorTotalRewards)
+		res, height, err := cliCtx.QueryWithData(route, bz)
+		if err != nil {
+			rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
+			return
+		}
+
+		cliCtx = cliCtx.WithHeight(height)
 		rest.PostProcessResponse(w, cliCtx, res)
 	}
 }
@@ -91,12 +105,16 @@ func delegationRewardsHandlerFn(cliCtx context.CLIContext, queryRoute string) ht
 			return
 		}
 
+		delAddr := mux.Vars(r)["delegatorAddr"]
+		valAddr := mux.Vars(r)["validatorAddr"]
+
 		// query for rewards from a particular delegation
-		res, ok := checkResponseQueryDelegationRewards(w, cliCtx, queryRoute, mux.Vars(r)["delegatorAddr"], mux.Vars(r)["validatorAddr"])
+		res, height, ok := checkResponseQueryDelegationRewards(w, cliCtx, queryRoute, delAddr, valAddr)
 		if !ok {
 			return
 		}
 
+		cliCtx = cliCtx.WithHeight(height)
 		rest.PostProcessResponse(w, cliCtx, res)
 	}
 }
@@ -147,8 +165,7 @@ func NewValidatorDistInfo(operatorAddr sdk.AccAddress, rewards sdk.DecCoins,
 // HTTP request handler to query validator's distribution information
 func validatorInfoHandlerFn(cliCtx context.CLIContext, queryRoute string) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		valAddr := mux.Vars(r)["validatorAddr"]
-		validatorAddr, ok := checkValidatorAddressVar(w, r)
+		valAddr, ok := checkValidatorAddressVar(w, r)
 		if !ok {
 			return
 		}
@@ -159,28 +176,39 @@ func validatorInfoHandlerFn(cliCtx context.CLIContext, queryRoute string) http.H
 		}
 
 		// query commission
-		commissionRes, err := common.QueryValidatorCommission(cliCtx, queryRoute, validatorAddr)
+		bz, err := common.QueryValidatorCommission(cliCtx, queryRoute, valAddr)
 		if err != nil {
 			rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
 			return
 		}
 
-		var valCom types.ValidatorAccumulatedCommission
-		cliCtx.Codec.MustUnmarshalJSON(commissionRes, &valCom)
+		var commission types.ValidatorAccumulatedCommission
+		if err := cliCtx.Codec.UnmarshalJSON(bz, &commission); err != nil {
+			rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
+			return
+		}
 
 		// self bond rewards
-		delAddr := sdk.AccAddress(validatorAddr)
-		rewardsRes, ok := checkResponseQueryDelegationRewards(w, cliCtx, queryRoute, delAddr.String(), valAddr)
+		delAddr := sdk.AccAddress(valAddr)
+		bz, height, ok := checkResponseQueryDelegationRewards(w, cliCtx, queryRoute, delAddr.String(), valAddr.String())
 		if !ok {
 			return
 		}
 
 		var rewards sdk.DecCoins
-		cliCtx.Codec.MustUnmarshalJSON(rewardsRes, &rewards)
+		if err := cliCtx.Codec.UnmarshalJSON(bz, &rewards); err != nil {
+			rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
+			return
+		}
 
-		// Prepare response
-		res := cliCtx.Codec.MustMarshalJSON(NewValidatorDistInfo(delAddr, rewards, valCom))
-		rest.PostProcessResponse(w, cliCtx, res)
+		bz, err = cliCtx.Codec.MarshalJSON(NewValidatorDistInfo(delAddr, rewards, commission))
+		if err != nil {
+			rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
+			return
+		}
+
+		cliCtx = cliCtx.WithHeight(height)
+		rest.PostProcessResponse(w, cliCtx, bz)
 	}
 }
 
@@ -199,12 +227,13 @@ func validatorRewardsHandlerFn(cliCtx context.CLIContext, queryRoute string) htt
 		}
 
 		delAddr := sdk.AccAddress(validatorAddr).String()
-		res, ok := checkResponseQueryDelegationRewards(w, cliCtx, queryRoute, delAddr, valAddr)
+		bz, height, ok := checkResponseQueryDelegationRewards(w, cliCtx, queryRoute, delAddr, valAddr)
 		if !ok {
 			return
 		}
 
-		rest.PostProcessResponse(w, cliCtx, res)
+		cliCtx = cliCtx.WithHeight(height)
+		rest.PostProcessResponse(w, cliCtx, bz)
 	}
 }
 
@@ -277,28 +306,15 @@ func outstandingRewardsHandlerFn(cliCtx context.CLIContext, queryRoute string) h
 	}
 }
 
-func checkResponseQueryDelegatorTotalRewards(
-	w http.ResponseWriter, cliCtx context.CLIContext, queryRoute, delAddr string,
-) (res []byte, ok bool) {
-
-	res, err := common.QueryDelegatorTotalRewards(cliCtx, queryRoute, delAddr)
-	if err != nil {
-		rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
-		return nil, false
-	}
-
-	return res, true
-}
-
 func checkResponseQueryDelegationRewards(
 	w http.ResponseWriter, cliCtx context.CLIContext, queryRoute, delAddr, valAddr string,
-) (res []byte, ok bool) {
+) (res []byte, height int64, ok bool) {
 
-	res, err := common.QueryDelegationRewards(cliCtx, queryRoute, delAddr, valAddr)
+	res, height, err := common.QueryDelegationRewards(cliCtx, queryRoute, delAddr, valAddr)
 	if err != nil {
 		rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
-		return nil, false
+		return nil, 0, false
 	}
 
-	return res, true
+	return res, height, true
 }