Skip to content
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.

Sign up for GitHub

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

fix: query account balance by ibc denom #10394

Merged
merged 8 commits into from
Nov 11, 2021
Merged

fix: query account balance by ibc denom #10394

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ee55387
fix: fix the query account balance by ibc denom
Oct 18, 2021
d2d8b53
Merge branch 'master' into sai/10381_query_balance_by_ibc_denom
gsk967 Oct 19, 2021
7777b70
Merge branch 'master' into sai/10381_query_balance_by_ibc_denom
Oct 22, 2021
6b52755
refactor: change the account balance by denom grpc-gateway to support…
Oct 22, 2021
41ae4ac
Merge remote-tracking branch 'origin/sai/10381_query_balance_by_ibc_d…
Oct 22, 2021
9439625
Merge branch 'master' into sai/10381_query_balance_by_ibc_denom
gsk967 Oct 22, 2021
ae93ce9
updated changelog
Oct 22, 2021
d60f2e9
Merge branch 'master' into sai/10381_query_balance_by_ibc_denom
tac0turtle Nov 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/core/proto-docs.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2269,7 +2269,7 @@ Query defines the gRPC querier service.

| Method Name | Request Type | Response Type | Description | HTTP Verb | Endpoint |
| ----------- | ------------ | ------------- | ------------| ------- | -------- |
| `Balance` | [QueryBalanceRequest](#cosmos.bank.v1beta1.QueryBalanceRequest) | [QueryBalanceResponse](#cosmos.bank.v1beta1.QueryBalanceResponse) | Balance queries the balance of a single coin for a single account. | GET|/cosmos/bank/v1beta1/balances/{address}/{denom}|
| `Balance` | [QueryBalanceRequest](#cosmos.bank.v1beta1.QueryBalanceRequest) | [QueryBalanceResponse](#cosmos.bank.v1beta1.QueryBalanceResponse) | Balance queries the balance of a single coin for a single account. | GET|/cosmos/bank/v1beta1/balances/{address}/{denom=**}|
| `AllBalances` | [QueryAllBalancesRequest](#cosmos.bank.v1beta1.QueryAllBalancesRequest) | [QueryAllBalancesResponse](#cosmos.bank.v1beta1.QueryAllBalancesResponse) | AllBalances queries the balance of all coins for a single account. | GET|/cosmos/bank/v1beta1/balances/{address}|
| `TotalSupply` | [QueryTotalSupplyRequest](#cosmos.bank.v1beta1.QueryTotalSupplyRequest) | [QueryTotalSupplyResponse](#cosmos.bank.v1beta1.QueryTotalSupplyResponse) | TotalSupply queries the total supply of all coins. | GET|/cosmos/bank/v1beta1/supply|
| `SupplyOf` | [QuerySupplyOfRequest](#cosmos.bank.v1beta1.QuerySupplyOfRequest) | [QuerySupplyOfResponse](#cosmos.bank.v1beta1.QuerySupplyOfResponse) | SupplyOf queries the supply of a single coin. | GET|/cosmos/bank/v1beta1/supply/{denom}|
Expand Down
2 changes: 1 addition & 1 deletion proto/cosmos/bank/v1beta1/query.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ option go_package = "github.com/cosmos/cosmos-sdk/x/bank/types";
service Query {
// Balance queries the balance of a single coin for a single account.
rpc Balance(QueryBalanceRequest) returns (QueryBalanceResponse) {
option (google.api.http).get = "/cosmos/bank/v1beta1/balances/{address}/{denom}";
option (google.api.http).get = "/cosmos/bank/v1beta1/balances/{address}/{denom=**}";
Copy link
Contributor Author

@gsk967 gsk967 Oct 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robert-zaremba
I tried the {denom=**} it is working but this change breaking the account balance route

unescaping setting for grpc-gateway runtime is added in latest grpc-gateway
https://grpc-ecosystem.github.io/grpc-gateway/docs/mapping/customizing_your_gateway/#controlling-path-parameter-unescaping

May be we can move denom params from route path to route query

if req.denom == "" || req.denom == nil  
  GetAccountBalance
else
  GetAccountBalanceByDenom

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can, but this is breaking.
Do you have an idea if fixing it without breaking an API?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No @robert-zaremba

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this is a gRPC issue? I was hoping we could just update the handler to URI decode the denom.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be possible to somehow manually resolve it? Or using:

...?denom={denom....}   // not sure what should go here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will make one route for query AllBalances and Balance together

denom and pagination both are optional params for route

if req.denom == "" || req.denom == nil  
  GetAccountBalance
else
  GetAccountBalanceByDenom 
// AllBalances queries the balance of all coins for a single account.
  rpc AccountBalance(QueryAccountBalance) returns (QueryAccountBalanceResponse) {
    option (google.api.http).get = "/cosmos/bank/v1beta1/balances/{address}";
  }

// QueryBalanceRequest is the request type for the Query/Balance RPC method.
message QueryAccountBalance {
  option (gogoproto.equal)           = false;
  option (gogoproto.goproto_getters) = false;

  // address is the address to query balances for.
  string address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];

  // denom is the coin denom to query balances for.
  string denom = 2;

 // pagination defines an optional pagination for the request.
  cosmos.base.query.v1beta1.PageRequest pagination = 3;
}


// QueryBalanceResponse is the response type for the Query/Balance RPC method.
message QueryAccountBalanceResponse {
  // balance is the balance of the coin.
  cosmos.base.v1beta1.Coin balance = 1;

// balances is the balances of all the coins.
  repeated cosmos.base.v1beta1.Coin balances = 2
      [(gogoproto.nullable) = false, (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins"];

  // pagination defines the pagination in the response.
  cosmos.base.query.v1beta1.PageResponse pagination = 3;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robert-zaremba ^^^

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robert-zaremba i think we can't do backport this because it is breaking proto change
so i changed to grpc-gateway to fix the ibc-denom issue

 rpc Balance(QueryBalanceRequest) returns (QueryBalanceResponse) {
    option (google.api.http).get = "/cosmos/bank/v1beta1/balances/{address}/by_denom";
  }

Now denom is query params to this route

{API}/cosmos/bank/v1beta1/balances/{address}/by_denom?denom={DENOM or IBC-DENOM}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robert-zaremba or @AmauryM, i'm not super familiar with the current gRPC logic. Do these proposals sound reasonable?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach sounds reasonable to me

}

// AllBalances queries the balance of all coins for a single account.
Expand Down
122 changes: 61 additions & 61 deletions x/bank/types/query.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion x/bank/types/query.pb.gw.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.