Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 support for spendable balances gRPC query #11417
feat: add support for spendable balances gRPC query #11417
Changes from 2 commits
9e848bf
f979395
7236e55
d4f77c0
666b04d
7a2f9fd
deecd03
ff13306
3892bf4
66c4114
c2f699e
420a2b1
4f6f8bc
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This function can be called by other transaction. So, let's charge extra gas here. Eg 20gas per iteration.
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.
Charge gas? This is a query, not a transaction.
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 know. But it can be called from a transaction, in other words, a RPC Msg method, when running can do a query call.
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 follow?
These methods are for queries only. In fact, in a subsequent PR, I'll be removing these from the
Keeper
entirely, in favor of a dedicatedQuerier
(Example). All modules should follow this approach.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.
Look at this example: https://github.com/regen-network/regen-ledger/blob/master/x/ecocredit/server/msg_server.go#L718 - The
rpc Buy
will callbank.SpendableCoins
.Technically we can create a
QueryClient
of other module, and use it when processing a transaction.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.
That doesn't make any sense. Gas and txs are not involved here.
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.
Probably I didn't express myself well enough. Here is the snippet (didn't test it but I believe it's possible):
BTW - I think this is general discussion if we should check if we should consider checking if context provides a gas meter in queries and charge gas (so feel free to merge)