[FRAME Core] Add support for `view_functions`

[bkchr]

It is basically the idea from @xlc: https://forum.polkadot.network/t/wasm-view-functions/1045

I think we should start low and only add support for these view functions. For this we should add some new attribute to the FRAME macros pallet::view_function. It would look similar to this:

#[pallet::view_function]
impl<T: Config> Pallet<T> {
    pub fn account_balances(account: T::AccountId) -> Vec<T::Balance> {
         // query the storage etc
    }
}

This code will then generate some declarative macro implement_view_function:

macro_rules implement_view_function {
    ( $PALLET_RUNTIME_NAME:ident ) => {
        #[cfg(not(feature = "std"))]
    #[no_mangle]
    pub unsafe fn $PALLET_RUNTIME_NAME __ account_balances(input_data: *mut u8, input_len: usize) -> u64 {
        let mut #input = if input_len == 0 {
            &[0u8; 0]
        } else {
            unsafe {
                $crate::slice::from_raw_parts(input_data, input_len)
            }
        };

        let output = $PALLET_RUNTIME_NAME::account_balances(Decode::decode(input).unwrap());
        $crate::to_substrate_wasm_fn_return_value(&output)
    }
    },
}

This implement_view_function macro is then called by construct_runtime! to generate these functions. As we prefix each view function with the name of the pallet in the runtime there will be no conflicts for different pallet instances.

This only solves the runtime side. The node side can be implemented later, but requires more thinking.

[ggwpez]

Would this be implemented on all runtimes per default, or feature gated to keep them small?

These functions are run in a new WASM instance, so even if someone changed storage, it would be disregarded, or?
We could probably make it panic in that case via ReadOnlyExternalities for better Dev-EX.

Decode::decode(input).unwrap())

How does this work with multiple inputs? Just some tuple iterator, huh?

I think we should directly think about versioning them, maybe similar to the runtime APIs. That will make it easier to communicate breaking changes in their arg/return types.

[bkchr]

How does this work with multiple inputs? Just some tuple iterator, huh?

Multiple arguments are just a tuple. That is the same way it works for the the runtime api function as well. Basically the extern functions are like runtime api functions.

Would this be implemented on all runtimes per default, or feature gated to keep them small?

On all. Maybe we could make it disablable per pallet in construct_runime if requested.

These functions are run in a new WASM instance, so even if someone changed storage, it would be disregarded, or? We could probably make it panic in that case via ReadOnlyExternalities for better Dev-EX.

All runtime api functions work this way. We don't need to panic or anything. There are also valid points of temp modifications. The node also should not see any of that and the node doesn't need to know. You can use the state_call rpc to call these functions as they are similar to runtime api functions. You just need to know their name, parameters and return value. (Yes we should add them to the metadata as well).

I think we should directly think about versioning them, maybe similar to the runtime APIs. That will make it easier to communicate breaking changes in their arg/return types.

If they are part of the metadata, it should be enough. As a developer you can also do some kind of manual versioning, by just creating a new function and keep the old one.

[athei]

This is definitely good for replacing all those getter RuntimeAPIs which are mostly boilerplate. However, it looks to me as a more comfortable way to define a subset of RuntimeAPIs. I think we should take it a step further.

The design as described won't allow a contract to use those queries because they can only be called from outside the runtime and not from inside. So we probably want to design it more like a Call. We construct a type we might call Getter which is basically like Call. We then implement a new trait Query (like Dispatchable) on it. All the getters are then called via a single Wasm exported function which accepts Getter. From within the runtime we can call it directly by using Query::query(). This will allow a contract to pass an encoded Query to the runtime.

It is basically just the read-only version of Dispatchable.

[bkchr]

I like the idea! Sounds like a good solution to solve the issue for contracts and ui in one go!

[kianenigma]

One downside I see with using tuples for this as @athei explained is that we would have to deal with a similar set of issues related preventing breaking changes. The index of the pallet and function will matter, similar to calls.

Runtime-apis, which are addressable via their name and are thus independent of the runtime pallet ordering had this extra advantage. I always assumed view functions need to have the same property.

Not sure if it is a big issue, just something to be aware of.

[athei]

Good point. Maybe we should incorporate more robustness and versioning. If it works well we can backport it to Call.

[bkchr]

We could use a "hash based" index.

[juangirini]

@bkchr given the fact that we want to push the use of XCM/XCQ to cal into the runtime, do you think this issue is still relevant?

[bkchr]

XCQ will not replace this. As XCM is not replacing pallet calls. We for sure can do the same here and introduce some kind of querying language, instead of exposing individual functions as described here in the issue. (which was already explained by Alex above)

[athei]

XCQ will not replace this. As XCM is not replacing pallet calls.

I had a brief discussion with @gavofyork about this in Malmö. I noted that Call seems redundant in a world with XCM. As far as I understood it is that in an ideal world transactions would contain XCM programs instead of a Call.

[bkchr]

Yes, but XCM as a standard will probably never abstract over all available functionality of every available chain. And as a standard it will also never be able to evolve that fast as chains itself.

[athei]

I think this is exactly the point of XCM. To be generic enough that functionality can be added without amending the standard. At least eventually. That said, it might still be worthwhile to have view functions. However, they would not be useable from contracts because we need a stable target. And this is exactly what XCM is for.

[kianenigma]

I am doubtful if this is still needed as a priority feature. It can remain in our backlog and someday tackled, but not immediately.

@juangirini what would be great if you and/or someone else already has a good grasp of this and make it a mentor-able issue? or is it too big for that?

[xlc]

please, makes this a top priority issue

[juangirini]

I am doubtful if this is still needed as a priority feature. It can remain in our backlog and someday tackled, but not immediately.

@juangirini what would be great if you and/or someone else already has a good grasp of this and make it a mentor-able issue? or is it too big for that?

I haven't had a proper look at this yet tbh, so I couldn't have an estimate in size yet. But big does not necessarily mean that can't be done by contributors outside of Parity.

please, makes this a top priority issue

@xlc Maybe someone from your team is interested in collaborating to this?

[xlc]

We don’t have any extra capacity. Also this issue is definitely not suitable for someone not super familiar with Substrate.

[pgherveou]

We could use a "hash based" index.

hashing sounds like a good idea, we just need to make sure that we take into account instantiable pallet, as there could be more than 1 occurrence of a pallet in construct_runtime.

This is definitely good for replacing all those getter RuntimeAPIs which are mostly boilerplate. However, it looks to me as a more comfortable way to define a subset of RuntimeAPIs. I think we should take it a step further.

The design as described won't allow a contract to use those queries because they can only be called from outside the runtime and not from inside. So we probably want to design it more like a Call. We construct a type we might call Getter which is basically like Call. We then implement a new trait Query (like Dispatchable) on it. All the getters are then called via a single Wasm exported function which accepts Getter. From within the runtime we can call it directly by using Query::query(). This will allow a contract to pass an encoded Query to the runtime.

It is basically just the read-only version of Dispatchable.

The outer enum pallet sounds like a good idea but unlike dispatchable call we not only have to encode a call but also decode a generic Response type.

Is using Vec<u8> for the return type our best option here?

pub trait Query {
    fn query(&self) -> Vec<u8>
}

From within the runtime we can call it directly by using Query::query()

Can't we simply call the view function like any other fns.

let balance = Self::account_balance(id);

From the perspective of an ink! contract, I imagine we would need to wrap the Getter into the outer enum, and then decode the response using SCALE, something along these lines:

let getter = Balances::Getter::account_balance{ account: id }.into();
let balance: T::Balance = self.env().query(getter).decode()?;

[bkchr]

Is using Vec<u8> for the return type our best option here?

We probably also want to have this typed. For contracts this isn't that useful, but for UIs etc to get the type.

[pgherveou]

I mean the types are defined by the view functions inside the #[pallet::view_functions], but the Query trait is implemented by the macro and just here to (de)serialize the data, right?

I imagine that metadata and type annotations will derive from this macro as well

[bkchr]

Ahh yeah makes sense! So we have pallet level Query and runtime level RuntimeQuery? And we implement the query trait for the runtime RuntimeQuery?

[pgherveou]

Yes, from what Alex wrote above, I imagine that the macro would implement the Query trait for the Pallet and Runtime, just like we have for RuntimeCall today

[Polkadot-Forum]

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

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

[ascjones]

I have been investigating this issue, at the same time Bryan @xlc has begun designing and developing an XCQ PoC.

There is some discussion above about whether view_functions as specified here would be necessary if/when XCQ is implemented.

Yes, but XCM as a standard will probably never abstract over all available functionality of every available chain. And as a standard it will also never be able to evolve that fast as chains itself.

The design proposes to solve this issue with the extension model and polkavm programs for querying. This may or may not be viable but there is WIP to investigate this direction.

I am at a point where I have to decide where to direct my efforts:

1. Go ahead and implement view_function as specified here. More straightforward to implement up front, but could be superseded eventually by a more powerful XCQ implementation.
2. Assist with design/development of XCQ. More involved, larger future payoff.

At the moment I am leaning towards 2.. What does everybody else think?

[athei]

I think view_functions can be thought of a special case of an XCQ program. Instead of encoding a PolkaVM program we encode a Query data structure similar to Call. I think all of the infrastructure needed for both approaches are the same. Essentially only the payload is different. Like we still need RPC's for off chain usage. And an XCM instruction for on-chain usage.

I think maybe you can implement 1. in a way that it is agnostic to the payload (Query vs. XCM query) except for the runtime.

[xlc]

XCQ will cover all use cases of view functions.

Instead of encoding a PolkaVM program we encode a Query data structure similar to Call.

The XCQ will provide host calls to invoke extensions and one potential implementation will be calling the host call with such Query data structure so basically we can make XCQ a wrapper of view functions.

However, that’s just one of the potential implementation. There could be other ways and each alternatives will have different trade offs. That’s something I need to figure out by analyzing the pros and cons of each potential solution and decide the best suitable one.

So I don’t really know if we should continue do the view function as it is. It is possible the work can be directly integrated into XCQ but it is also possible the work will be obsoleted by XCQ. We need to spend more time defining XCQ spec before we can answer this question.

[athei]

XCQ will cover all use cases of view functions.

This is understood. Just trying to find out if view_functions can be a stepping stone on the way to XCQ. Or if we should go for XCQ directly.