-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add feegrant query to see allowances from a given granter #10947
Changes from 2 commits
782a918
e6c1d01
a375d7c
d8cd1ef
449d905
29b6bf4
5805809
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,11 @@ service Query { | |
rpc Allowances(QueryAllowancesRequest) returns (QueryAllowancesResponse) { | ||
option (google.api.http).get = "/cosmos/feegrant/v1beta1/allowances/{grantee}"; | ||
} | ||
|
||
// IssuedAllowances returns all the grants given by an address | ||
rpc IssuedAllowances(QueryIssuedAllowancesRequest) returns (QueryIssuedAllowancesResponse) { | ||
option (google.api.http).get = "/cosmos/feegrant/v1beta1/issued/{granter}"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I like the Maybe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would we want to change There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would be proto-breaking, so no, though it would be the ideal choice. I still prefer There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok I will make the change |
||
} | ||
} | ||
|
||
// QueryAllowanceRequest is the request type for the Query/Allowance RPC method. | ||
|
@@ -54,3 +59,20 @@ message QueryAllowancesResponse { | |
// pagination defines an pagination for the response. | ||
cosmos.base.query.v1beta1.PageResponse pagination = 2; | ||
} | ||
|
||
// QueryIssuedAllowancesRequest is the request type for the Query/IssuedAllowances RPC method. | ||
message QueryIssuedAllowancesRequest { | ||
string granter = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"]; | ||
|
||
// pagination defines an pagination for the request. | ||
cosmos.base.query.v1beta1.PageRequest pagination = 2; | ||
} | ||
|
||
// QueryIssuedAllowancesResponse is the response type for the Query/IssuedAllowances RPC method. | ||
message QueryIssuedAllowancesResponse { | ||
// allowances that have been issued by the granter. | ||
repeated cosmos.feegrant.v1beta1.Grant allowances = 1; | ||
|
||
// pagination defines an pagination for the response. | ||
cosmos.base.query.v1beta1.PageResponse pagination = 2; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
//go:build norace | ||
// +build norace | ||
|
||
package testutil | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -197,7 +197,7 @@ func (s *IntegrationTestSuite) TestCmdGetFeeGrant() { | |
} | ||
} | ||
|
||
func (s *IntegrationTestSuite) TestCmdGetFeeGrants() { | ||
func (s *IntegrationTestSuite) TestCmdGetFeeGrantsByGrantee() { | ||
val := s.network.Validators[0] | ||
grantee := s.addedGrantee | ||
clientCtx := val.ClientCtx | ||
|
@@ -239,7 +239,7 @@ func (s *IntegrationTestSuite) TestCmdGetFeeGrants() { | |
tc := tc | ||
|
||
s.Run(tc.name, func() { | ||
cmd := cli.GetCmdQueryFeeGrants() | ||
cmd := cli.GetCmdQueryFeeGrantsByGrantee() | ||
out, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, tc.args) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you also add integration tests for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done 👍 |
||
|
||
if tc.expectErr { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -93,3 +93,42 @@ func (q Keeper) Allowances(c context.Context, req *feegrant.QueryAllowancesReque | |
|
||
return &feegrant.QueryAllowancesResponse{Allowances: grants, Pagination: pageRes}, nil | ||
} | ||
|
||
// IssuedAllowances queries all the allowances granted by the given granter | ||
func (q Keeper) IssuedAllowances(c context.Context, req *feegrant.QueryIssuedAllowancesRequest) (*feegrant.QueryIssuedAllowancesResponse, error) { | ||
if req == nil { | ||
return nil, status.Error(codes.InvalidArgument, "invalid request") | ||
} | ||
|
||
granterAddr, err := sdk.AccAddressFromBech32(req.Granter) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
ctx := sdk.UnwrapSDKContext(c) | ||
|
||
var grants []*feegrant.Grant | ||
|
||
store := ctx.KVStore(q.storeKey) | ||
pageRes, err := query.Paginate(store, req.Pagination, func(key []byte, value []byte) error { | ||
var grant feegrant.Grant | ||
|
||
granter, _ := feegrant.ParseAddressesFromFeeAllowanceKey(key) | ||
if !granter.Equals(granterAddr) { | ||
return nil | ||
} | ||
|
||
if err := q.cdc.Unmarshal(value, &grant); err != nil { | ||
return err | ||
} | ||
|
||
grants = append(grants, &grant) | ||
return nil | ||
}) | ||
Comment on lines
+113
to
+127
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes the more efficient solution would be to have a secondary index and I think in the future we may want to have the
|
||
|
||
if err != nil { | ||
return nil, status.Error(codes.Internal, err.Error()) | ||
} | ||
|
||
return &feegrant.QueryIssuedAllowancesResponse{Allowances: grants, Pagination: pageRes}, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
package feegrant_test | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/stretchr/testify/require" | ||
|
||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
"github.com/cosmos/cosmos-sdk/x/feegrant" | ||
) | ||
|
||
func TestMarshalAndUnmarshalFeegrantKey(t *testing.T) { | ||
grantee, err := sdk.AccAddressFromBech32("cosmos1qk93t4j0yyzgqgt6k5qf8deh8fq6smpn3ntu3x") | ||
require.NoError(t, err) | ||
granter, err := sdk.AccAddressFromBech32("cosmos1p9qh4ldfd6n0qehujsal4k7g0e37kel90rc4ts") | ||
require.NoError(t, err) | ||
|
||
key := feegrant.FeeAllowanceKey(granter, grantee) | ||
require.Len(t, key, len(grantee.Bytes())+len(granter.Bytes())+3) | ||
require.Equal(t, feegrant.FeeAllowancePrefixByGrantee(grantee), key[:len(grantee.Bytes())+2]) | ||
|
||
g1, g2 := feegrant.ParseAddressesFromFeeAllowanceKey(key) | ||
require.Equal(t, granter, g1) | ||
require.Equal(t, grantee, g2) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add a
//Since
comment. Any plans to backport these?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind backporting this. I'll defer the decision to others