fix: Staking delegations should return empty list instead of rpc error when no records found (#9423)

* return empty list instead of rpc error when no records found for staking delegations * fix grpc query DelegatorDelegations tests * remove response code tests for staking delegations * fix failing tests * change staking delegations response code to 200 in grpc test * add staking delegations response code tests to TestQueryDelegatorDelegationsGRPC * add address without delegations testcase * add changes to grpc query tests of delegatorDelegations * remove getRequest unused function from x/staking/client/rest/grpc_query_test.go * minor fixes * add testcases for request with no delegations * address review comments * add changelog Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
2021-06-01 20:47:44 +05:30 · 2021-06-01 20:47:44 +05:30 · da87ab0d36
commit da87ab0d36
parent 98fbdbd5b8
4 changed files with 104 additions and 80 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -127,6 +127,7 @@ if input key is empty, or input data contains empty key.

 ### Improvements

+* (x/staking) [\#9423](https://github.com/cosmos/cosmos-sdk/pull/9423) Staking delegations now returns empty list instead of rpc error when no records found.
 * (baseapp, types) [#\9390](https://github.com/cosmos/cosmos-sdk/pull/9390) Add current block header hash to `Context`
 * (x/bank) [\#8614](https://github.com/cosmos/cosmos-sdk/issues/8614) Add `Name` and `Symbol` fields to denom metadata
 * (x/auth) [\#8522](https://github.com/cosmos/cosmos-sdk/pull/8522) Allow to query all stored accounts
--- a/x/staking/client/rest/grpc_query_test.go
+++ b/x/staking/client/rest/grpc_query_test.go
@ -4,8 +4,6 @@ package rest_test

 import (
 	"fmt"
-	"io/ioutil"
-	"net/http"
 	"testing"

 	"github.com/gogo/protobuf/proto"
@ -414,39 +412,15 @@ func (s *IntegrationTestSuite) TestQueryUnbondingDelegationGRPC() {
 	}
 }

-func (s *IntegrationTestSuite) TestQueryDelegationsResponseCode() {
-	val := s.network.Validators[0]
-
-	// Create new account in the keyring.
-	info, _, err := val.ClientCtx.Keyring.NewMnemonic("test", keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1)
-	s.Require().NoError(err)
-	newAddr := sdk.AccAddress(info.GetPubKey().Address())
-
-	s.T().Log("expect 404 error for address without delegations")
-	res, statusCode, err := getRequest(fmt.Sprintf("%s/cosmos/staking/v1beta1/delegations/%s", val.APIAddress, newAddr.String()))
-	s.Require().NoError(err)
-	s.Require().Contains(string(res), "\"code\": 5")
-	s.Require().Equal(404, statusCode)
-}
-
-func getRequest(url string) ([]byte, int, error) {
-	res, err := http.Get(url) // nolint:gosec
-	body, err := ioutil.ReadAll(res.Body)
-	if err != nil {
-		return nil, res.StatusCode, err
-	}
-
-	if err = res.Body.Close(); err != nil {
-		return nil, res.StatusCode, err
-	}
-
-	return body, res.StatusCode, nil
-}
-
 func (s *IntegrationTestSuite) TestQueryDelegatorDelegationsGRPC() {
 	val := s.network.Validators[0]
 	baseURL := val.APIAddress

+	// Create new account in the keyring for address without delegations.
+	info, _, err := val.ClientCtx.Keyring.NewMnemonic("test", keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1)
+	s.Require().NoError(err)
+	newAddr := sdk.AccAddress(info.GetPubKey().Address())
+
 	testCases := []struct {
 		name         string
 		url          string
@ -486,6 +460,19 @@ func (s *IntegrationTestSuite) TestQueryDelegatorDelegationsGRPC() {
 				Pagination: &query.PageResponse{Total: 1},
 			},
 		},
+		{
+			"address without delegations",
+			fmt.Sprintf("%s/cosmos/staking/v1beta1/delegations/%s", baseURL, newAddr.String()),
+			map[string]string{
+				grpctypes.GRPCBlockHeightHeader: "1",
+			},
+			false,
+			&types.QueryDelegatorDelegationsResponse{},
+			&types.QueryDelegatorDelegationsResponse{
+				DelegationResponses: types.DelegationResponses{},
+				Pagination:          &query.PageResponse{Total: 0},
+			},
+		},
 	}

 	for _, tc := range testCases {
--- a/x/staking/keeper/grpc_query.go
+++ b/x/staking/keeper/grpc_query.go
@ -281,11 +281,6 @@ func (k Querier) DelegatorDelegations(c context.Context, req *types.QueryDelegat
 		return nil, status.Error(codes.Internal, err.Error())
 	}

-	if delegations == nil {
-		return nil, status.Errorf(
-			codes.NotFound,
-			"unable to find delegations for address %s", req.DelegatorAddr)
-	}
 	delegationResps, err := DelegationsToDelegationResponses(ctx, k.Keeper, delegations)
 	if err != nil {
 		return nil, status.Error(codes.Internal, err.Error())
--- a/x/staking/keeper/grpc_query_test.go
+++ b/x/staking/keeper/grpc_query_test.go
@ -35,7 +35,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryValidators() {
 			len(vals),
 			false,
 		},
-		{"empty status returns all the validators",
+		{
+			"empty status returns all the validators",
 			func() {
 				req = &types.QueryValidatorsRequest{Status: ""}
 			},
@ -52,7 +53,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryValidators() {
 			0,
 			false,
 		},
-		{"valid request",
+		{
+			"valid request",
 			func() {
 				req = &types.QueryValidatorsRequest{Status: types.Bonded.String(),
 					Pagination: &query.PageRequest{Limit: 1, CountTotal: true}}
@ -101,7 +103,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryValidator() {
 			},
 			false,
 		},
-		{"valid request",
+		{
+			"valid request",
 			func() {
 				req = &types.QueryValidatorRequest{ValidatorAddr: vals[0].OperatorAddress}
 			},
@ -141,7 +144,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryDelegatorValidators() {
 			},
 			false,
 		},
-		{"valid request",
+		{
+			"valid request",
 			func() {
 				req = &types.QueryDelegatorValidatorsRequest{
 					DelegatorAddr: addrs[0].String(),
@ -185,7 +189,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryDelegatorValidator() {
 			},
 			false,
 		},
-		{"invalid delegator, validator pair",
+		{
+			"invalid delegator, validator pair",
 			func() {
 				req = &types.QueryDelegatorValidatorRequest{
 					DelegatorAddr: addr.String(),
@ -194,7 +199,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryDelegatorValidator() {
 			},
 			false,
 		},
-		{"valid request",
+		{
+			"valid request",
 			func() {
 				req = &types.QueryDelegatorValidatorRequest{
 					DelegatorAddr: addr.String(),
@ -235,13 +241,15 @@ func (suite *KeeperTestSuite) TestGRPCQueryDelegation() {
 		malleate func()
 		expPass  bool
 	}{
-		{"empty request",
+		{
+			"empty request",
 			func() {
 				req = &types.QueryDelegationRequest{}
 			},
 			false,
 		},
-		{"invalid validator, delegator pair",
+		{
+			"invalid validator, delegator pair",
 			func() {
 				req = &types.QueryDelegationRequest{
 					DelegatorAddr: addrAcc1.String(),
@ -250,7 +258,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryDelegation() {
 			},
 			false,
 		},
-		{"valid request",
+		{
+			"valid request",
 			func() {
 				req = &types.QueryDelegationRequest{DelegatorAddr: addrAcc.String(), ValidatorAddr: addrVal}
 			},
@ -285,27 +294,42 @@ func (suite *KeeperTestSuite) TestGRPCQueryDelegatorDelegations() {
 	var req *types.QueryDelegatorDelegationsRequest

 	testCases := []struct {
-		msg      string
-		malleate func()
-		expPass  bool
+		msg       string
+		malleate  func()
+		onSuccess func(suite *KeeperTestSuite, response *types.QueryDelegatorDelegationsResponse)
+		expErr    bool
 	}{
-		{"empty request",
+		{
+			"empty request",
 			func() {
 				req = &types.QueryDelegatorDelegationsRequest{}
 			},
-			false,
-		}, {"invalid request",
+			func(suite *KeeperTestSuite, response *types.QueryDelegatorDelegationsResponse) {},
+			true,
+		},
+		{
+			"valid request with no delegations",
 			func() {
 				req = &types.QueryDelegatorDelegationsRequest{DelegatorAddr: addrs[4].String()}
 			},
+			func(suite *KeeperTestSuite, response *types.QueryDelegatorDelegationsResponse) {
+				suite.Equal(uint64(0), response.Pagination.Total)
+				suite.Len(response.DelegationResponses, 0)
+			},
 			false,
 		},
-		{"valid request",
+		{
+			"valid request",
 			func() {
 				req = &types.QueryDelegatorDelegationsRequest{DelegatorAddr: addrAcc.String(),
 					Pagination: &query.PageRequest{Limit: 1, CountTotal: true}}
 			},
-			true,
+			func(suite *KeeperTestSuite, response *types.QueryDelegatorDelegationsResponse) {
+				suite.Equal(uint64(2), response.Pagination.Total)
+				suite.Len(response.DelegationResponses, 1)
+				suite.Equal(sdk.NewCoin(sdk.DefaultBondDenom, delegation.Shares.TruncateInt()), response.DelegationResponses[0].Balance)
+			},
+			false,
 		},
 	}

@ -313,14 +337,11 @@ func (suite *KeeperTestSuite) TestGRPCQueryDelegatorDelegations() {
 		suite.Run(fmt.Sprintf("Case %s", tc.msg), func() {
 			tc.malleate()
 			res, err := queryClient.DelegatorDelegations(gocontext.Background(), req)
-			if tc.expPass {
-				suite.Equal(uint64(2), res.Pagination.Total)
-				suite.Len(res.DelegationResponses, 1)
-				suite.Equal(1, len(res.DelegationResponses))
-				suite.Equal(sdk.NewCoin(sdk.DefaultBondDenom, delegation.Shares.TruncateInt()), res.DelegationResponses[0].Balance)
-			} else {
+			if tc.expErr {
 				suite.Error(err)
-				suite.Nil(res)
+			} else {
+				suite.NoError(err)
+				tc.onSuccess(suite, res)
 			}
 		})
 	}
@ -344,21 +365,24 @@ func (suite *KeeperTestSuite) TestGRPCQueryValidatorDelegations() {
 		expPass  bool
 		expErr   bool
 	}{
-		{"empty request",
+		{
+			"empty request",
 			func() {
 				req = &types.QueryValidatorDelegationsRequest{}
 			},
 			false,
 			true,
 		},
-		{"invalid validator delegator pair",
+		{
+			"invalid validator delegator pair",
 			func() {
 				req = &types.QueryValidatorDelegationsRequest{ValidatorAddr: addrVal2.String()}
 			},
 			false,
 			false,
 		},
-		{"valid request",
+		{
+			"valid request",
 			func() {
 				req = &types.QueryValidatorDelegationsRequest{ValidatorAddr: addrVal1,
 					Pagination: &query.PageRequest{Limit: 1, CountTotal: true}}
@ -409,19 +433,22 @@ func (suite *KeeperTestSuite) TestGRPCQueryUnbondingDelegation() {
 		malleate func()
 		expPass  bool
 	}{
-		{"empty request",
+		{
+			"empty request",
 			func() {
 				req = &types.QueryUnbondingDelegationRequest{}
 			},
 			false,
 		},
-		{"invalid request",
+		{
+			"invalid request",
 			func() {
 				req = &types.QueryUnbondingDelegationRequest{}
 			},
 			false,
 		},
-		{"valid request",
+		{
+			"valid request",
 			func() {
 				req = &types.QueryUnbondingDelegationRequest{
 					DelegatorAddr: addrAcc2.String(), ValidatorAddr: addrVal2}
@ -469,21 +496,24 @@ func (suite *KeeperTestSuite) TestGRPCQueryDelegatorUnbondingDelegations() {
 		expPass  bool
 		expErr   bool
 	}{
-		{"empty request",
+		{
+			"empty request",
 			func() {
 				req = &types.QueryDelegatorUnbondingDelegationsRequest{}
 			},
 			false,
 			true,
 		},
-		{"invalid request",
+		{
+			"invalid request",
 			func() {
 				req = &types.QueryDelegatorUnbondingDelegationsRequest{DelegatorAddr: addrAcc1.String()}
 			},
 			false,
 			false,
 		},
-		{"valid request",
+		{
+			"valid request",
 			func() {
 				req = &types.QueryDelegatorUnbondingDelegationsRequest{DelegatorAddr: addrAcc.String(),
 					Pagination: &query.PageRequest{Limit: 1, CountTotal: true}}
@ -544,25 +574,29 @@ func (suite *KeeperTestSuite) TestGRPCQueryHistoricalInfo() {
 		malleate func()
 		expPass  bool
 	}{
-		{"empty request",
+		{
+			"empty request",
 			func() {
 				req = &types.QueryHistoricalInfoRequest{}
 			},
 			false,
 		},
-		{"invalid request with negative height",
+		{
+			"invalid request with negative height",
 			func() {
 				req = &types.QueryHistoricalInfoRequest{Height: -1}
 			},
 			false,
 		},
-		{"valid request with old height",
+		{
+			"valid request with old height",
 			func() {
 				req = &types.QueryHistoricalInfoRequest{Height: 4}
 			},
 			false,
 		},
-		{"valid request with current height",
+		{
+			"valid request with current height",
 			func() {
 				req = &types.QueryHistoricalInfoRequest{Height: 5}
 			},
@ -612,14 +646,16 @@ func (suite *KeeperTestSuite) TestGRPCQueryRedelegations() {
 		expPass  bool
 		expErr   bool
 	}{
-		{"request redelegations for non existent addr",
+		{
+			"request redelegations for non existent addr",
 			func() {
 				req = &types.QueryRedelegationsRequest{DelegatorAddr: addrAcc.String()}
 			},
 			false,
 			false,
 		},
-		{"request redelegations with non existent pairs",
+		{
+			"request redelegations with non existent pairs",
 			func() {
 				req = &types.QueryRedelegationsRequest{DelegatorAddr: addrAcc.String(), SrcValidatorAddr: val3.String(),
 					DstValidatorAddr: val4.String()}
@ -627,7 +663,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryRedelegations() {
 			false,
 			true,
 		},
-		{"request redelegations with delegatoraddr, sourceValAddr, destValAddr",
+		{
+			"request redelegations with delegatoraddr, sourceValAddr, destValAddr",
 			func() {
 				req = &types.QueryRedelegationsRequest{
 					DelegatorAddr: addrAcc1.String(), SrcValidatorAddr: val1.OperatorAddress,
@ -636,7 +673,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryRedelegations() {
 			true,
 			false,
 		},
-		{"request redelegations with delegatoraddr and sourceValAddr",
+		{
+			"request redelegations with delegatoraddr and sourceValAddr",
 			func() {
 				req = &types.QueryRedelegationsRequest{
 					DelegatorAddr: addrAcc1.String(), SrcValidatorAddr: val1.OperatorAddress,
@ -645,7 +683,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryRedelegations() {
 			true,
 			false,
 		},
-		{"query redelegations with sourceValAddr only",
+		{
+			"query redelegations with sourceValAddr only",
 			func() {
 				req = &types.QueryRedelegationsRequest{
 					SrcValidatorAddr: val1.GetOperator().String(),
@ -695,13 +734,15 @@ func (suite *KeeperTestSuite) TestGRPCQueryValidatorUnbondingDelegations() {
 		malleate func()
 		expPass  bool
 	}{
-		{"empty request",
+		{
+			"empty request",
 			func() {
 				req = &types.QueryValidatorUnbondingDelegationsRequest{}
 			},
 			false,
 		},
-		{"valid request",
+		{
+			"valid request",
 			func() {
 				req = &types.QueryValidatorUnbondingDelegationsRequest{
 					ValidatorAddr: val1.GetOperator().String(),