Dispatch Macro Enhancements

[gavofyork]

• 1. Introduce ability to not have origin parameter at all (dispatch pre-executes system::ensure_root(origin)?)
• 2. Introduce logic for arbitrary origin matching in the macro

Notes

1. fn definitions in decl_module that do not include an origin term should be assumed to be privileged and automatically ensure! that origin == Root.

Before:

pub struct Module<T: Trait>;
pub enum Call where aux: T::PublicAux {
	fn propose_spend(aux, value: T::Balance, beneficiary: T::AccountId) -> Result = 0;
}
pub enum PrivCall {
	fn set_pot(new_pot: T::Balance) -> Result = 0;
}

After:

pub struct Module<T: Trait> where origin: T::Origin {
	fn propose_spend(origin, value: T::Balance, beneficiary: T::AccountId) -> Result = 0;
	fn set_pot(new_pot: T::Balance) -> Result = 1;
}

2. This can be done by ensuring that the impl_dispatch places the ensure! in the match dispatcher before dispatching in the cases where no origin is provided. A more sophisticated macro would allow origin to be required to match a particular value:

Before:

use {T::Origin::council as CouncilOrigin, council::Origin::Members};
use {T::Origin::system as SystemOrigin, system::Origin::Signed};
pub struct Module<T: Trait> where origin: T::Origin {
	fn propose(origin, value: T::Balance, beneficiary: T::AccountId) -> Result = 0;
	fn accept(origin, index: ProposalIndex) -> Result = 1;
}
impl<T: Trait> Module<T> {
	fn propose(origin: T::Origin, index: ProposalIndex) -> Result {
		match origin {
			SystemOrigin(Signed(&who)) => {
				ensure!(Self::check_account(who), "not a good account");
				Self::do_some_stuff(who)
			}
			_ => return Err("invalid origin"),
		}
	}
	fn accept(origin: T::Origin, index: ProposalIndex) -> Result {
		match origin {
			CouncilOrigin(Members(n)) if n >= 2 => {
				// ...
			}
			_ => return Err("invalid origin"),
		}
	}
}

After:

use {T::Origin::council as CouncilOrigin, council::Origin::Members};
pub struct Module<T: Trait> where origin: T::Origin {
	fn propose(SystemOrigin(Signed(who)), value: T::Balance, beneficiary: T::AccountId) -> Result = 0;
	fn accept(CouncilOrigin(Members(n)) if n >= 2, index: ProposalIndex) -> Result = 1;
}
impl<T: Trait> Module<T> {
	fn propose(who: &T::AccountId, index: ProposalIndex) -> Result {
		ensure!(Self::check_account(who), "not a good account");
		Self::do_some_stuff(who)
	}
	fn accept(index: ProposalIndex) -> Result {
		// ...
	}
}

[guanqun]

@bkchr the first problem sounds interesting, if you don't mind, can I have one day to look at that?

[bkchr]

I don't mind, do it if you want. I started with a different task :)

[guanqun]

Thanks. :) Let me practice a bit macro-fu.

[guanqun]

Should we use static way as in the current code CouncilOrigin(Members(_2)) or use the CouncilOrigin(Members(n)) if n >= 2 as outlined above?

[bkchr]

The second one would be more flexible. Depending on what you can achieve, the second would be really nice.

[guanqun]

I'd say these two are actually the same, even for the second one, the "n" is not exposed to the function at all. The logic is determined when we write the macro.

[gavofyork]

Will move this to another milestone for now since I think we've got the low-hanging fruit.

[guanqun]

Agreed, I tried a few different options for item 2, but I'm not quite satisfied with that approach. I'll leave this to @bkchr :)

[kianenigma]

This seems very much out of date

definitions in decl_module that do not include an origin term should be assumed to be privileged and automatically ensure! that origin == Root.

we had this and removed it later in a PR to avoid confusion.

[kianenigma]

@thiolliere close?

[gui1117]

@thiolliere close?

We may still want to improve origin matching, but I think this issue can be closed in favor of paritytech/polkadot-sdk#349