Encode required origin for extrinsics in metadata

[xlc]

There are multiple incidents in both Kusama council and Polkadot council that the council motion was created with wrong number of members count make them invalid.

We should have a way to encode the required origin for extrinsics in metadata which should avoid this issue.

It will allow polkadot apps to construct correct motion without manually duplicated that information into polkadot apps repo.

One way is add fn metadata to trait EnsureOrigin. And modify decl_module macro to support new syntax to specify the origin like fn sudo(origin: EnsureRoot, call: Box<<T as Trait>::Call>)

[bkchr]

Depends on: paritytech/substrate#5678

[shawntabrizi]

An idea... needs more thought

#[meta(origin: T::Origin = T::Origin:Root)]
#[meta(fee: T::Balance = deposit_of(x).saturating_add(asdo)]

Call {

meta: {
orgin: root
fee: balance
}
}

[gui1117]

I like the idea of not strongly typed metadata associated to a call.

the metadata associated to a call would be: a name, a type and a value.

Or maybe we should type those information with:

• origin: T::Origin the only accepted origin of the call.
• origins: Vec<T::Origin> all the accepted origins
• fee: any type as balance is not a FRAME concept.

I think it is better to allow any kind of metadata with any names and types without any assumption on their signification.
This will allow user to declare their own keywords for their specific purpose.

[gui1117]

another idea would be something like this:

		#[pallet::origin(EnsureRoot)]
		#[pallet::weight(*_ratio * T::BlockWeights::get().max_block)]
		pub(crate) fn fill_block(origin: (), _ratio: Perbill) -> DispatchResult {
			Ok(())
		}

the new attribute would take type which implements EnsureOrigin, and the first argument of the call would be of type <EnsureRoot as EnsureOrigin>::Success.
The type should also implements EnsureOriginMetadata trait.

trait EnsureOriginMetadata: EnsureOrigin {
    /// Return some doc about the origin checker.
    fn meta() -> &'static str;
}

The attribute is optional, if no attribute origin is given then no check are done and metadata is None (same as now).

pros:

• the checker is automatically done when dispatching the call, so metadata can't be outdated if call is changing. they are necessarily consistent.

cons:

• the signature of the call is changing, when called from rust, the origin is not checked anymore. But I think calling dispatchable from rust is not very recommanded except in test. But even in test people can use the enum Call.

  (we can still make the macro modifies the signature of the function and replace origin type by T::Origin and add a first statement which checks the origin inside the function: let origin = EnsureRoot::ensure_origin(origin)?; But this is macro magic, and we tried to avoid this. I think it is better not to touch user input when possible, similarly to derive macro.)

EDIT: but in general allowing user to put in metadata some unstructured information associated to calls seems useful.

[stale]

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

[gui1117]

issue is still relevant

[Polkadot-Forum]

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

https://forum.polkadot.network/t/guidance-on-selecting-pre-images/2135/4

[bkchr]

This issue came up recently again, especially around the spend transaction of the Treasury pallet and finding out how much some origin can spend. IMO this is a good example that shows that origins are quite generic and can contain a lot of custom logic (a little bit similar to SignedExtensions). There are also other like EnsureRanked which are even more "complicated", because they access state to decide on the output. This is quite hard to map to something like a list of origins to pass.

So, I would propose something like this to put origins into metadata:

enum OriginMetadata {
     /// Simplest case like:
     /// `ensure_signed` => List(vec![(Origin::Signed, ())])
     /// 
     /// For a simple spend it would probably be something like:
     /// List(vec![(Origin::SmallSpender, 10), (Origin::MediumSpender, 20), (Origin::BigSpender, 30)])
     List(Vec<(Origin, Val)>),
     /// For things like `EnsureRanked` etc that are all custom logic 
     /// that we can not really express here. The argument to this type would 
     /// be an identifier. This would be similar to the identifier of a `SignedExtension` and 
     /// will enable some downstream user to match on this identifier to have the actual logic
     /// hard coded.
     Custom(Vec<u8>),
     /// Any of the checks need to succeed.
     Or(Vec<Box<OriginMetadata>>),
     /// All the checks need to succeed.
     And(Vec<Box<OriginMetadata>>),
}

I hope that I have covered all the cases that are important, but maybe I miss something.

As we have now support for adding custom things to the metadata, I would say that we do not wait for a new metadata version and instead add the information to the custom metadata section. Using Pallet_name::Call_name::Origin as key should be enough to make it unique for each runtime.

[lexnv]

Are there any strong opinions about not including this in metadata V16?

We don't expect this to land before stabilization, and we could always include it in the custom metadata fields 🙏