FRAME: General System for Recognizing and Executing Service Work

[sam0x17]

Fixes paritytech/polkadot-sdk#206

[kianenigma]

As the associated issue is in it, I suggest keeping only one in the roadmap.)

[sam0x17]

@KiChjang do you recall when you were setting up HoldReason whether you ever ran into this issue? I based this expand_outer_task method off of your expand_outer_hold_reason method almost exactly and tried to follow all the conventions your HoldReason follows as far as I can tell.

Your version is here:

substrate/frame/support/procedural/src/construct_runtime/expand/hold_reason.rs

Lines 22 to 51 in 34f45c6

	pub fn expand_outer_hold_reason(pallet_decls: &[Pallet], scrate: &TokenStream) -> TokenStream {
	let mut conversion_fns = Vec::new();
	let mut hold_reason_variants = Vec::new();
	for decl in pallet_decls {
	if let Some(_) = decl.find_part("HoldReason") {
	let variant_name = &decl.name;
	let path = &decl.path;
	let index = decl.index;
	conversion_fns.push(expand_conversion_fn(path, variant_name));
	hold_reason_variants.push(expand_variant(index, path, variant_name));
	}
	}
	quote! {
	/// A reason for placing a hold on funds.
	#[derive(
	Copy, Clone, Eq, PartialEq, Ord, PartialOrd,
	#scrate::codec::Encode, #scrate::codec::Decode, #scrate::codec::MaxEncodedLen,
	#scrate::scale_info::TypeInfo,
	#scrate::RuntimeDebug,
	)]
	pub enum RuntimeHoldReason {
	#( #hold_reason_variants )*
	}
	#( #conversion_fns )*
	}
	}

What's happening here is for some reason decl.find_part("Task") is coming back empty, even when we are looking at a pallet like TasksExample which 100% does have a pub enum Task.

Is there anything additional I need to do to make this get picked up by find_part?

I'll be looking deeply into how pallet parts are parsed and constructed next to see why this might be going around, but wanted to double check there isn't something obvious I've missed first.

[KiChjang]

Yes, you need to an an additional variant to one of the enums in the parse module of construct_runtime.

[KiChjang]

This is the enum I'm talking about:

substrate/frame/support/procedural/src/construct_runtime/parse.rs

Lines 395 to 409 in 34f45c6

	pub enum PalletPartKeyword {
	Pallet(keyword::Pallet),
	Call(keyword::Call),
	Storage(keyword::Storage),
	Event(keyword::Event),
	Error(keyword::Error),
	Config(keyword::Config),
	Origin(keyword::Origin),
	Inherent(keyword::Inherent),
	ValidateUnsigned(keyword::ValidateUnsigned),
	FreezeReason(keyword::FreezeReason),
	HoldReason(keyword::HoldReason),
	LockId(keyword::LockId),
	SlashReason(keyword::SlashReason),
	}

In addition to modifying the above, the runtime maintainer (i.e. whoever calls construct_runtime) needs to also import the pallet part in the construct_runtime macro, e.g.

construct_runtime! {
pub enum Runtime
where
    // ...
    {
        System: frame_system,
        MyPallet: path::to::my::pallet::{Pallet, Storage, Call, Event<T>, Task},
    }
}

Note the additional Task keyword used in MyPallet. You can also use the implicit pallet part import syntax so you have less hassle (i.e. simply MyPallet: path::to::my::pallet,).

[sam0x17]

So I believe I am already doing both of these...

In PalletPartKeyword:
https://github.com/paritytech/substrate/pull/14329/files#diff-bc4d1d4294aa1ace0173ab00f6fac84463c64992cd3857099a000fb2266388cfR408

In construct_runtime!:
https://github.com/paritytech/substrate/pull/14329/files#diff-67684005af418e25ff88c2ae5b520f0c040371f1d817e03a3652e76b9485224aR1956

Perhaps I'm not in the right construct_runtime!?? @KiChjang

[sam0x17]

ahhh OK I followed your syntax now by doing this in construct_runtime!:

TasksExample: tasks_example::{Pallet, Storage, Call, Event<T>, Task},

and now I am just getting this error:

  error[E0412]: cannot find type `Task` in crate `tasks_example`
      --> /home/sam/workspace/substrate/bin/node/runtime/src/lib.rs:1883:1
       |
  652  |  |     #[compact]
       |  |_________^ not found in `tasks_example`
  ...
  1883 | // construct_runtime!(
  1884 | |      pub struct Runtime
  1885 | |      {
  1886 | |          System: frame_system,
  ...    |
  1957 | |      }
  1958 | |  );
       | |__- in this macro invocation
       |
       = note: this error originates in the macro `frame_support::construct_runtime` which comes from the expansion of the macro `construct_runtime` (in Nightly builds, run with -Z macro-backtrace for more info)
  help: consider importing one of these items
       |
  25   + use crate::tasks_example::pallet::Task;
       |
  25   + use frame_support::pallet_prelude::Task;
       |
  25   + use tasks_example::pallet::Task;
       |

  For more information about this error, try `rustc --explain E0412`.

Strangely, tasks_example::Task is perfectly accessible according to the language server, and I can see in the file that there is indeed pub use pallet::*; so yeah investigating this one now

[sam0x17]

So yeah this is super weird. I went ahead and pushed up the current code. Anyone have any idea how/why this error is happening when that type is clearly accessible from that context?

[KiChjang]

This looks wrong. Task is not declared using #[pallet::composite_enum], so it shouldn't be accessing composites here. Instead, a new field called task should exist on the Def.

[sam0x17]

ahhh that's probably it.

[gupnik]

@sam0x17 Pushed the fix here. We will need to replace it with the actual runtime ident.

[paritytech-cicd-pr]

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-check-benches
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3409306

[paritytech-cicd-pr]

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-check-benches
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3409307