Implement pallet view function queries

[ascjones]

Closes #216.

This PR allows pallets to define a view_functions impl like so:

#[pallet::view_functions]
impl<T: Config> Pallet<T>
where
	T::AccountId: From<SomeType1> + SomeAssociation1,
{
	/// Query value no args.
	pub fn get_value() -> Option<u32> {
		SomeValue::<T>::get()
	}

	/// Query value with args.
	pub fn get_value_with_arg(key: u32) -> Option<u32> {
		SomeMap::<T>::get(key)
	}
}

QueryId

Each view function is uniquely identified by a QueryId, which for this implementation is generated by:

twox_128(pallet_name) ++ twox_128("fn_name(fnarg_types) -> return_ty")

The prefix twox_128(pallet_name) is the same as the storage prefix for pallets and take into account multiple instances of the same pallet.

The suffix is generated from the fn type signature so is guaranteed to be unique for that pallet impl. For one of the view fns in the example above it would be twox_128("get_value_with_arg(u32) -> Option<u32>"). It is a known limitation that only the type names themselves are taken into account: in the case of type aliases the signature may have the same underlying types but a different id; for generics the concrete types may be different but the signatures will remain the same.

The existing Runtime Call dispatchables are addressed by their concatenated indices pallet_index ++ call_index, and the dispatching is handled by the SCALE decoding of the RuntimeCallEnum::PalletVariant(PalletCallEnum::dispatchable_variant(payload)). For view_functions the runtime/pallet generated enum structure is replaced by implementing the DispatchQuery trait on the outer (runtime) scope, dispatching to a pallet based on the id prefix, and the inner (pallet) scope dispatching to the specific function based on the id suffix.

Future implementations could also modify/extend this scheme and routing to pallet agnostic queries.

Executing externally

These view functions can be executed externally via the system runtime api:

pub trait ViewFunctionsApi<QueryId, Query, QueryResult, Error> where
	QueryId: codec::Codec,
	Query: codec::Codec,
	QueryResult: codec::Codec,
	Error: codec::Codec,
{
	/// Execute a view function query.
	fn execute_query(query_id: QueryId, query: Query) -> Result<QueryResult, Error>;
}

XCQ

Currently there is work going on by @xlc to implement XCQ which may eventually supersede this work.

It may be that we still need the fixed function local query dispatching in addition to XCQ, in the same way that we have chain specific runtime dispatchables and XCM.

I have kept this in mind and the high level query API is agnostic to the underlying query dispatch and execution. I am just providing the implementation for the view_function definition.

Metadata

Currently I am utilizing the custom section of the frame metadata, to avoid modifying the official metadata format until this is standardized.

vs runtime_api

There are similarities with runtime_apis, some differences being:

• queries can be defined directly on pallets, so no need for boilerplate declarations and implementations
• no versioning, the QueryId will change if the signature changes.
• possibility for queries to be executed from smart contracts (see below)

Calling from contracts

Future work would be to add weight annotations to the view function queries, and a host function to pallet_contracts to allow executing these queries from contracts.

TODO

• [ ] Consistent naming (view functions pallet impl, queries, high level api?)
• [ ] End to end tests via runtime_api
• [ ] UI tests
• [ ] Mertadata tests
• [ ] Docs

[paritytech-cicd-pr]

The CI pipeline was cancelled due to failure one of the required jobs. Job name: cargo-clippy Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6425895

[kianenigma]

It may be that we still need the fixed function local query dispatching in addition to XCQ, in the same way that we have chain specific runtime dispatchables and XCM.

Exactly. XC in XCQ delineates the difference: XCQ should be useful for standards across multiple chains, and this, or in general runtime-apis can be used for chain-specific scenarios.

[kianenigma]

vs runtime_api

And one other minor difference is that sometimes a runtime may want to expose queries that are only fulfilled across multiple pallets, such as "what is my total transferrable balance".

[Polkadot-Forum]

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/wasm-view-functions/1045/9

[kianenigma]

@ascjones I am not sure super sure about how much of a requirement this is, but assuming we decide that the feature is needed, are you available to push this to the finish line?

[ascjones]

@ascjones I am not sure super sure about how much of a requirement this is, but assuming we decide that the feature is needed, are you available to push this to the finish line?

If it is required, yes I am happy to complete it.

[kianenigma]

The only possibly major shortcoming I see with the PR is the ability to extend view functions in the runtime level. Imagine a query that relies on 3 pallets, and cannot be done within the code of a single pallet.

I assume that view functions also have an aggregate enum such as RuntimeCall, right?

In that case, I think we can do a general solution here and allow all Runtime* to be extended. At least for RuntimeCall, there is also a good use-case in being able to attach a pallet-less transaction to your runtime. Might introduce some metadata related issue though. lmk what you think about this.

[ascjones]

I assume that view functions also have an aggregate enum such as RuntimeCall, right?

No, dispatching is done via a direct QueryId -> view function mapping, where the QueryId prefix is twox_128(pallet_name).

This is extendable for pallet agnostic queries where the unique prefix is generated from some other property of the view function definition.

[athei]

I think it is important to think subscribe ability into the design from the beginning. Which means essentially that the view function should declare which storage items it is touching. Maybe with an attribute and some automatically generated tests that check that it doesn't touch any other storage key.

A consequence from that is that the pallet independent view functions (defined at runtime level) should be composed of pallet view functions instead of using direct storage access. This will guarantee that the information about storage and possible migrations keeps being encapsulated within individual pallets. Otherwise I can imagine that storage migration in a pallet will break runtime wide view functions. It will also allow us to automatically derive the touched storage keys for those runtime level view functions by just taking the union of the pallet view functions they are composed of.

@ascjones Since we kinda agreed that we want this view functions independently of XCQ can you incorporate those two changes into your PR:

1. Allow for subscribeablity by requiring the view functions to declare which storage items/prefixes they access.
2. Add runtime wide view functions

The runtime wide view functions should use the same runtime function if possible. I guess just decide on how to derive the identifier. Maybe twoxx(:runtime_view:name). Or some other special character to prevent collision with pallet names.

The declared storage items per view function should then be listed in Metadata per view function so that clients can just automatically subscribe to them via existing RPCs.

[ascjones]

Which means essentially that the view function should declare which storage items it is touching. Maybe with an attribute and some automatically generated tests that check that it doesn't touch any other storage key.

Can we reuse the part of benchmarking which counts the reads/writes to get the accessed storage keys instead?

[athei]

Which means essentially that the view function should declare which storage items it is touching. Maybe with an attribute and some automatically generated tests that check that it doesn't touch any other storage key.

Can we reuse the part of benchmarking which counts the reads/writes to get the accessed storage keys instead?

The problem is that which storage keys are accessed might depent on the inputs to the view function. So you can't really automatically determine them without brute forcing every possible input. I am afraid it has to be declared manually.

[athei]

@joepetrowski pointed out that we also need to make sure that we can package common runtime level view functions in a way that they are available for every runtime. This is the case for pallet level view functions automatically since they live in their pallets crate.

[kianenigma]

I think it is important to think subscribe ability into the design from the beginning.

Yes to this..

Which means essentially that the view function should declare which storage items it is touching.

But no to this. Why does it need to edeclared at compile-time, or in code? I like the approach noted here, which allows any runtime api to become subscribe-able: https://github.com/paritytech/polkadot-sdk/issues/3594#issuecomment-2123618147

Given that the subscription can be implemented in other ways, this also invalidates this statement:

A consequence from that is that the pallet independent view functions (defined at runtime level) should be composed of pallet view functions instead of using direct storage access.

To be clear, I am not against the rule of thumb that the runtime level view fns should mostly aggregate the pallet levels, but exceptions to this might exist, and I don't want to restrict it in the code. Indeed, a runtime view fn could look like:

fn fancy42() -> {
    let d1 = pallet_1::views::fn();
    let d2 = pallet_2::views::fn();
    let arbitrary = 42;
    let raw = pallet_3::storage::get();
    // final return value is an amalgamation of all of this. 
    final_(d1, d2, arbitrary, raw)
}

For example, with the current way of things, the inflation runtime api that I have recently added will have a hard time being expressed as a runtime view function, because it depends on things beyond what is purely a "set of pallet level view fns": https://github.com/polkadot-fellows/runtimes/pull/443/files#diff-e7afb81f3cef77f8a1686085f1adaa992122790270bae77eb2e640ad40b12cf4R2137

[kianenigma]

@joepetrowski pointed out that we also need to make sure that we can package common runtime level view functions in a way that they are available for every runtime.

Yeah, this is one of the known issues of doing anything at the runtime-level, at least with our current way of amalgamating runtimes with macros.

One way we can achieve this is to spit out majority of impl_runtime_apis! { .. } as a part of construct_runtime!. I have discussed it with @gupnik in the past, but it has felt quite a macro hell that I am not sure if it is worth it.

Otherwise, an easier way would be to enhance the wasm-builder to do some checks post compilation:

1. build the wasm file, get the list of exported fns
2. query the metadata, get the list of pallets
3. match them together; if a pallet exists but the "expected" runtime api is not there, provide a compilation warning.

[kianenigma]

@ascjones About QueryId: I have solid opinions in favor of using hash (as done here) instead of enum indices.

But, given that a lot of the runtime is already working on the basis of enums, I wonder if it is short term easier to move the QueryId to be similarly based on enums. This would make the maintainers of Frontend SDKs to not need to know and implement two methods, and instead reuse the mental model of how RuntimeCall dispatch works.

In the longer future, we can move all dispatches to being hash based, perhaps.

I wish for a world where all pallet dispatching was based on hashes, but I feel like something even worse than the status quo is to need to explain to someone why e.g. pallet indices exist in construct_runtime, and how they do impact RuntimeCall, but they don't matter for view fns.

In other words, we are having now one foot here and one foot there, and that's kinda messy.

The end users of SubXT, PAPI and PJS should not feel any difference no matter if we use hashes or indices of course, or if we migrate from one to the other.

[athei]

Which means essentially that the view function should declare which storage items it is touching.

But no to this. Why does it need to edeclared at compile-time, or in code? I like the approach noted here, which allows any runtime api to become subscribe-able: #3594 (comment)

Agreed. It should work this way. You will subscribe to a runtime API with regard to a specific argument. This is why it works vs. determining the keys statically at benchmark time (as suggested by Andrew).

Given that the subscription can be implemented in other ways, this also invalidates this statement:

A consequence from that is that the pallet independent view functions (defined at runtime level) should be composed of pallet view functions instead of using direct storage access.

To be clear, I am not against the rule of thumb that the runtime level view fns should mostly aggregate the pallet levels, but exceptions to this might exist, and I don't want to restrict it in the code. Indeed, a runtime view fn could look like:

fn fancy42() -> {
    let d1 = pallet_1::views::fn();
    let d2 = pallet_2::views::fn();
    let arbitrary = 42;
    let raw = pallet_3::storage::get();
    // final return value is an amalgamation of all of this. 
    final_(d1, d2, arbitrary, raw)
}

Yeah this is fine. I just don't want them to hardcode storage keys or derive them manually. As long as they go through the pallets storage abstraction it should be fine. So yes, my initial requirement was too strict.

[bkchr]

Yeah, this is one of the known issues of doing anything at the runtime-level, at least with our current way of amalgamating runtimes with macros.

One way we can achieve this is to spit out majority of impl_runtime_apis! { .. } as a part of construct_runtime!. I have discussed it with @gupnik in the past, but it has felt quite a macro hell that I am not sure if it is worth it.

Otherwise, an easier way would be to enhance the wasm-builder to do some checks post compilation:

1. build the wasm file, get the list of exported fns

2. query the metadata, get the list of pallets

3. match them together; if a pallet exists but the "expected" runtime api is not there, provide a compilation warning.

You are mixing here runtime level with pallet level. If every pallet is exposing its own view function, how could they be missed?

"Runtime level view functions" are essentially what runtime apis are today. Exposing these view functions, based on which pallets are part of the runtime, sounds complicated. I mean assume something like a "staking view function" which would require that the staking pallets are part of the runtime. All this leads me back to what I said in the Fellowship call and these things should be implemented outside of the runtime, because there you can just check if the pallets exist etc.

BTW, with wasm only runtimes, we can also implement runtime apis in a pallet directly and not require to implement it in the runtime anymore.

[joepetrowski]

"Runtime level view functions" are essentially what runtime apis are today. Exposing these view functions, based on which pallets are part of the runtime, sounds complicated. I mean assume something like a "staking view function" which would require that the staking pallets are part of the runtime.

I don't agree with this. The runtime view API only prescribes the signature, like "you must be able to return a list of validators". The implementations can all be different. Of course Polkadot will have its own implementation based on our pallets, but another chain could implement the same function for their staking system. Nothing about a view function should require that certain pallets be present in a runtime.

[athei]

"Runtime level view functions" are essentially what runtime apis are today. Exposing these view functions, based on which pallets are part of the runtime, sounds complicated. I mean assume something like a "staking view function" which would require that the staking pallets are part of the runtime.

I don't agree with this. The runtime view API only prescribes the signature, like "you must be able to return a list of validators". The implementations can all be different. Of course Polkadot will have its own implementation based on our pallets, but another chain could implement the same function for their staking system. Nothing about a view function should require that certain pallets be present in a runtime.

I am confused. You say you are don't agree but then your argument seems to agree with the quoted text: Runtime level view functions should only prescribe a signature/spec but not an implementation. The problem with that is the same as with runtime APIs today: A lot of copy paste. Because in practice, they all implemented using the same pallets. Even though they could theoretically be implemented using different pallets.

[joepetrowski]

Ah I see, I misread then. I thought Basti was against the runtime level API because it would require certain pallets (from "assume something like a "staking view function" which would require that the staking pallets are part of the runtime").

The problem with that is the same as with runtime APIs today: A lot of copy paste. Because in practice, they all implemented using the same pallets.

It's a mix though... I mean some of the common ones (like balance(account)) will actually be at the pallet level and just imported, so no copy-paste. For higher level ones like governance and staking, how many chains even have these features? I assume there will be one chain (AH) with this implemented but it would be unimplemented or return a null value for other chains.

[xlc]

I read a few comments looks like there are some confusions.

We need a way to define an API. e.g. fungibles. Runtime and pallets needs to be able to implement such API. The implementation for such API could be different for different chains. e.g. EVM chain will use ERC20 to implement it and AH will use pallet-assets. Pallet view function creates a way to querying into the pallet. If we are able to create a unified way to invoke pallet view functions, that can become the API. e.g. both pallet-assets and pallet-erc20 can implement a same view function so a same SDK can query both of the pallet in a same way. Or if we can't, well, it is still useful, but can't be used as API. i.e. SDK will need to use pallet-assets specific code and cannot use the same code unmodified to call into pallet-erc20. But I think that's not the primary goal of this PR. It can happen later.

Also given this is somewhat related to XCQ, this is how XCQ does it:

Define API: https://github.com/open-web3-stack/XCQ/blob/master/xcq-extension-fungibles/src/lib.rs

Implements API (in runtime) https://github.com/open-web3-stack/XCQ/blob/b973132c2b8e9e342c5bcd753522fba8f36a0674/poc/runtime/src/xcq.rs#L40-L51

XCQ program (this fetch balances of some accounts and return the sum) https://github.com/open-web3-stack/XCQ/blob/master/poc/guests/sum-balance/src/main.rs

Input to the XCQ program (the accounts and action are encoded as input) https://github.com/open-web3-stack/XCQ/blob/b973132c2b8e9e342c5bcd753522fba8f36a0674/xcq-test-runner/src/main.rs#L31-L44

I will image view functions could be integrated with XCQ later to confirm XCQ extension (i.e. replace the implements API part).

[athei]

This just reads as if you agree with @joepetrowski and @bkchr that runtime level view functions should behave like runtime APIs: You define a signature/trait and then each runtime decides how to implement it.

[bkchr]

EVM chain will use ERC20 to implement it and AH will use pallet-assets.

And if a chain uses both of these pallets? :D

Generally I think that the pallet local stuff is quite easy and can be automatically exposed when adding a pallet to a runtime.

For runtime level/when you need multiple pallets having some automatic implementation is not that easy. We could maybe have some fake pallets/runtime api implementation providers which provide the implementation based on multiple pre-defined pallets.

[athei]

Agreed. Yes the runtime level part is much harder. So maybe we can break it down in two PRs and focus only on the pallet wide view functions in this PR since the design there is mostly done.

But I think we should decide on how to identify view functions now (hash vs index). I agree with @kianenigma that we should dispatch based on enum index . Not only to keep it uniform with Call but also because I realized that hashes don't really offer too much benefits: Yes they are robust against moving pallets around in the runtime but we can't do that anyways because of Call. Besides, we have index attributes to address this problem. They also don't really remove the need to look up information in the Metadata as you need to learn about the arguments anyways. So you might as well look up the index of the view function while you are there. Lastly, hashes prevent renaming of the view function which can be useful in rare occasions.

tl;dr We should use the same index/enum based dispatch mechanism as Call. But the index attribute should be mandatory on the view function. Would be weird to index based on ordering inside the pallet. @bkchr What do you think?

We should also reserve the first entry in the outer enum for runtime level view functions (which will be implemented later):

enum ViewFunction {
    RuntimeWide(u32) = 0,
    System(u32),
    Balances(u32),
    ....
}

[ascjones]

Yes they are robust against moving pallets around in the runtime but we can't do that anyways because of Call. Besides, we have index attributes to address this problem.

Different runtimes have different pallet configurations, so if a multichain UI wanted to use a standard view function it is not guaranteed to have the same enum index. However, client libs could do a string lookup for e.g. "Balances" in the metadata to determine the index.

Another benefit would be the inbuilt "versioning", a new signature would just have a different id, although this is not foolproof without proper type information.

That being said, I wouldn't be against enum based indices since it is simpler to implement, but pushing a bit of complexity to client libs.

[athei]

Different runtimes have different pallet configurations, so if a multichain UI wanted to use a standard view function it is not guaranteed to have the same enum index. However, client libs could do a string lookup for e.g. "Balances" in the metadata to determine the index.

It is a good point. Client libraries will probably use metadata to generate the view function accessors. When using indices they will have to generate one set of functions per chain. When using hashes they can just generate functions from all the metadata files they have access to and merge them while removing duplicates. I think thats much better. Assuming that pallet names are unique of course.

And even when using generated code people will refer to the function by name and signature. Changing the name will break peoples code when they upgrade the lib as it will have generated the function under a different name. So we might as well encode it into the id to make sure we never ever change it.

So yes I think hashing based on name + signatures makes it easier for client lib devs. I think it outweighs the benefits of having everything done the same way. What do you think @kianenigma ?

[bkchr]

When using hashes they can just generate functions from all the metadata files they have access to and merge them while removing duplicates. I think thats much better. Assuming that pallet names are unique of course.

I don't really get what you are saying here? Why do they need multiple functions? For what?

Pallet names are not unique across different chains, you can name your pallet whatever you like in construct_runtime.

[kianenigma]

You are mixing here runtime level with pallet level. If every pallet is exposing its own view function, how could they be missed?

@bkchr: They can be, because you might forget to put the right piece of code in impl_runtime_api. As in, even if we only have pallet-level view fns, you have to add this to your runtime:

https://github.com/ascjones/polkadot-sdk/blob/21eeba75e94cf6408c7ae1843f94142ce8f338af/substrate/bin/node/runtime/src/lib.rs#L2687-L2691

Agreed. Yes the runtime level part is much harder. So maybe we can break it down in two PRs and focus only on the pallet wide view functions in this PR since the design there is mostly done.

@athei: Second. It is hard to foresee things without getting the hands dirty.

This PR is stable enough to be built on top of. Any of us can and should go and build a few pallet-level view fns on top of this PR, and then see how it will look like if we want to amalgamate them. The view function forum post has some ideas in it both for pallet level, and runtime level view functions.

So yes I think hashing based on name + signatures makes it easier for client lib devs. I think it outweighs the benefits of having everything done the same way. What do you think @kianenigma ?

Sadly both pallet name and index are variable per-runtime, so the north star of removing complexity from client libs is not feasible. In all future scenarios, if a client lib wants to provide an abstraction over the same view function in 5 different chains, it would inevitably need to go through some effort and "speculate/hardcode" a few things. I don't think there is any solution for it.

Given this, I am also still leaning towards enum based dispatch. It is more unified, and we can pursue someday moving all dispatches to hashes, but not have one foot in each. With the time bomb of reaching the 255 limit, we will have to do it someday :)

We can then reserve a special index in both the RuntimeQueryId, and RuntimeCall and so on, to extend all Runtime* enums at the runtime level. This solves both defining a runtime-level Query, and a Call and so on.

[kianenigma]

Further comment on stability:

💡 in fact the only thing that is not variable across runtimes is the "original" (prior to cargo package renaming) crate-name of a pallet. Do we put this in the metadata? perhaps we should, and then client libs can have an easier time building abstraction. They can abstract away over pallet index or pallet name, and instead say look for an instance of "a pallet built from pallet-balances crate, no matter the name or index".

Of course, you would still have the complexity of pallet instancin, but I think working on the basis of crate name is the only reasonable step forward here.

All in all, the ordering seems to be, from "unstable" to "stable":

Pallet index << Pallet Name << Pallet Crate name

[jsdw]

in fact the only thing that is not variable across runtimes is the "original" (prior to cargo package renaming) crate-name of a pallet.

I think that identifying a pallet by the pallets name is still the way to go for client libs. This allows a crate to be reused with different config for more than one pallet without clients caring (which was done not too long ago for some pallet I can't remember off the top of my head).

I'm also happy to stick with enum index over hash; we've had to solve this problem already for eg RuntimeCalls on the client side (which was can do by looking up the correct index based on the name of the pallet / call that we want) so offhand I don't mind the same being true for making these view function calls, and don't feel like it will add a lot of complexity on the client side.

The 256 limit scares me a bit, but I expect we can migrate things over to hash based lookup by the time we get close to that (which may come more quickly once each top level call gets its own variant).