Design and document gas consumption in query methods run in a Tx context

[robert-zaremba]

Summary

It is possible to call gRPC Query methods when processing Tx.

Problem Definition

Today, all query methods assume that they won't be called from a transaction. However it is technically possible to do it. In some cases heavy computation could be involved and gas won't be charged.

Example: Consider adding a new grant type, which will need to query bank spendable balances. That Msg method, when run, can call SpendableBalances Query RPC. it involve a join operation. Only store read/write coste will be charged, but we should probably charge gas for loop iterations (again, only when run from a transaction method)

Example2: Some query method could involve additional signature verification. When run from a Msg RPC, gas for doing that math operations won't be charged.

With ADR-33 this design (of using RPC Query or Msg for inter module communication) will be the right way to do something with another module.

Proposal

There are various options:

1. Allow (as it is now, and as per ADR-33) calling Query RPC. In that case, in those methods which do some additional operations, we should check if ctx.GasMeter is not nil and charge gas.
2. Disallow such behavior:
   • either by clearly documenting it in the SDK documentation and best practices
   • create other, not exportable types for methods designed to be run only in a Query context.

---

For Admin Use

• [x] Not duplicate issue
• [x] Appropriate labels applied
• [ ] Appropriate contributors tagged
• [ ] Contributor assigned/self-assigned

[alexanderbez]

I mentioned in the comment in the spendable balances query PR. I think the argument here is moot and irrelevant. Simply define the gRPC methods on a query dedicated type that just wraps the keeper. Keep the approach dead simple, which seems to somewhat line up with (2) that you proposed.

[robert-zaremba]

@alexanderbez we were talking about it during the Architecture Call last Friday and discussed few approaches I mentioned in the description.

[aaronc]

I don't think query methods should be callable in tx processing by default. We need some special annotation to allow this, probably in the protobuf file. Right now query methods are concerned non state machine breaking and could even be non deterministic. So we need to take care with an ADR 033 approach.

@fdymylja proposed an alternate approach which exposes ORM methods to other modules in a read-only manner

We should have a provision for ADR 033 methods which are only for calling internally and not exposed to clients

[robert-zaremba]

so we are leaning towards option (2.)

We should have a provision for ADR 033 methods which are only for calling internally and not exposed to clients

@ValarDragon was thinking about similar solution but for keeper. Idea is to have 2 keepers, one for queries, which can take the Msg keeper as a dependency, and another one for Msg and helper methods. Only the Msg keeper can be exposed externally

[fdymylja]

My opinion is that the system should be simplified. Things should be removed and not added.

In an ADR-33 world, where every module interacts with other module by RPC calls - which are identified by a name (ex: cosmos.bank.v1.Msg.SendCoins, cosmos.bank.v1.Query.Balances). You could have a specilized module, I proposed RBAC at the time, which at every invocation checks who the caller is (either it is a module or a normal user address) and which method (RPC) is being called and asserts if the caller has the right to move forward.

Then the authorization layer can be configurable at genesis (or somewhere else, not sure about this part, if it was in state it could be changed by governance but IDK what sort of use-cases it might empower).

an example: roles:

staking:
   executes: ["cosmos.bank.v1.Msg.SendCoinsFromModuleToModule"]
   queries: ["cosmos.bank.v1.Query.Balances"]

external_user:
   executes: ["cosmos.bank.v1.Msg.SendCoins"]
   queries: ["cosmos.bank.v1.Query.Balances"]   

role bindings:

staking-module-address: staking
user-address-1: external_user
user-address-2: external_user
user-address-3: external_user

This simplifies a lot what (module or user) can call what (rpcs either queries or msgs). With this same logic we can associate custom gas costs to Msg and Query RPCs.

I'd take this over any other keeper wrapper or stuff of sorts.

It's clean and it's declarative. And it groups modules and external users into one single entity whose scope is defined by configured authorizations.

We already have too many things to learn in the sdk with module, app module, query servers, msg servers and all of sorts. And I guess it plays nice with @aaronc idea of sdk apps as configs.

It of course requires us to migrate to ADR-033 first.

---

Also to add to this:

@fdymylja proposed an alternate approach which exposes ORM methods to other modules in a read-only manner

I think after ORM - there should be no need to implement queries anymore (maybe too optimistic eh?). Queries always map to something saved in state. ORM offers a standardized way of obtaining things from state, which is deterministic and let me stress this one more time - standardized.

[robert-zaremba]

Today, ORM is not exposed to the user. @aaronc recently told me about a graphql implementation for ORM.

But that's separate topic (we can move it to a discussion).

So here the idea presented by @fdymylja is to use a special module / contract which will manage permissions and potential gas charges whenever an inter module query call is done. I'm not sure if we really need RBAC for queries. A simple flag like 'open to other modules' would be sufficient. What other constrains would we have?

[alexanderbez]

This conversion has diverged greatly from what the initial post alludes to...