Introduce #[pallet::call_index] attribute to dispatchables (#11381)

* Introduce #[pallet::call_index] attribute to dispatchables

* cargo fmt

* Add more docs and prevent duplicates of call indices

* Add UI test for conflicting call indices

* cargo fmt

Co-authored-by: parity-processbot <>

frame/support/procedural/src/pallet/expand/call.rs

@@ -42,6 +42,7 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
 	let pallet_ident = &def.pallet_struct.pallet;
 
 	let fn_name = methods.iter().map(|method| &method.name).collect::<Vec<_>>();
+	let call_index = methods.iter().map(|method| method.call_index).collect::<Vec<_>>();
 	let new_call_variant_fn_name = fn_name
 		.iter()
 		.map(|fn_name| quote::format_ident!("new_call_variant_{}", fn_name))
@@ -177,6 +178,7 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
 			),
 			#(
 				#( #[doc = #fn_doc] )*
+				#[codec(index = #call_index)]
 				#fn_name {
 					#(
 						#[allow(missing_docs)]

frame/support/procedural/src/pallet/parse/call.rs

@@ -18,13 +18,15 @@
 use super::helper;
 use frame_support_procedural_tools::get_doc_literals;
 use quote::ToTokens;
+use std::collections::HashMap;
 use syn::spanned::Spanned;
 
 /// List of additional token to be used for parsing.
 mod keyword {
 	syn::custom_keyword!(Call);
 	syn::custom_keyword!(OriginFor);
 	syn::custom_keyword!(weight);
+	syn::custom_keyword!(call_index);
 	syn::custom_keyword!(compact);
 	syn::custom_keyword!(T);
 	syn::custom_keyword!(pallet);
@@ -55,14 +57,17 @@ pub struct CallVariantDef {
 	pub args: Vec<(bool, syn::Ident, Box<syn::Type>)>,
 	/// Weight formula.
 	pub weight: syn::Expr,
+	/// Call index of the dispatchable.
+	pub call_index: u8,
 	/// Docs, used for metadata.
 	pub docs: Vec<syn::Lit>,
 }
 
 /// Attributes for functions in call impl block.
-/// Parse for `#[pallet::weight(expr)]`
-pub struct FunctionAttr {
-	weight: syn::Expr,
+/// Parse for `#[pallet::weight(expr)]` or `#[pallet::call_index(expr)]
+pub enum FunctionAttr {
+	CallIndex(u8),
+	Weight(syn::Expr),
 }
 
 impl syn::parse::Parse for FunctionAttr {
@@ -72,11 +77,22 @@ impl syn::parse::Parse for FunctionAttr {
 		syn::bracketed!(content in input);
 		content.parse::<keyword::pallet>()?;
 		content.parse::<syn::Token![::]>()?;
-		content.parse::<keyword::weight>()?;
 
-		let weight_content;
-		syn::parenthesized!(weight_content in content);
-		Ok(FunctionAttr { weight: weight_content.parse::<syn::Expr>()? })
+		let lookahead = content.lookahead1();
+		if lookahead.peek(keyword::weight) {
+			content.parse::<keyword::weight>()?;
+			let weight_content;
+			syn::parenthesized!(weight_content in content);
+			Ok(FunctionAttr::Weight(weight_content.parse::<syn::Expr>()?))
+		} else if lookahead.peek(keyword::call_index) {
+			content.parse::<keyword::call_index>()?;
+			let call_index_content;
+			syn::parenthesized!(call_index_content in content);
+			let index = call_index_content.parse::<syn::LitInt>()?;
+			Ok(FunctionAttr::CallIndex(index.base10_parse()?))
+		} else {
+			Err(lookahead.error())
+		}
 	}
 }
 
@@ -145,6 +161,8 @@ impl CallDef {
 		}
 
 		let mut methods = vec![];
+		let mut indices = HashMap::new();
+		let mut last_index: Option<u8> = None;
 		for impl_item in &mut item.items {
 			if let syn::ImplItem::Method(method) = impl_item {
 				if !matches!(method.vis, syn::Visibility::Public(_)) {
@@ -182,18 +200,58 @@ impl CallDef {
 					return Err(syn::Error::new(method.sig.span(), msg))
 				}
 
-				let mut call_var_attrs: Vec<FunctionAttr> =
-					helper::take_item_pallet_attrs(&mut method.attrs)?;
-
-				if call_var_attrs.len() != 1 {
-					let msg = if call_var_attrs.is_empty() {
+				let (mut weight_attrs, mut call_idx_attrs): (Vec<FunctionAttr>, Vec<FunctionAttr>) =
+					helper::take_item_pallet_attrs(&mut method.attrs)?.into_iter().partition(
+						|attr| {
+							if let FunctionAttr::Weight(_) = attr {
+								true
+							} else {
+								false
+							}
+						},
+					);
+
+				if weight_attrs.len() != 1 {
+					let msg = if weight_attrs.is_empty() {
 						"Invalid pallet::call, requires weight attribute i.e. `#[pallet::weight($expr)]`"
 					} else {
 						"Invalid pallet::call, too many weight attributes given"
 					};
 					return Err(syn::Error::new(method.sig.span(), msg))
 				}
-				let weight = call_var_attrs.pop().unwrap().weight;
+				let weight = match weight_attrs.pop().unwrap() {
+					FunctionAttr::Weight(w) => w,
+					_ => unreachable!("checked during creation of the let binding"),
+				};
+
+				if call_idx_attrs.len() > 1 {
+					let msg = "Invalid pallet::call, too many call_index attributes given";
+					return Err(syn::Error::new(method.sig.span(), msg))
+				}
+				let call_index = call_idx_attrs.pop().map(|attr| match attr {
+					FunctionAttr::CallIndex(idx) => idx,
+					_ => unreachable!("checked during creation of the let binding"),
+				});
+
+				let final_index = match call_index {
+					Some(i) => i,
+					None =>
+						last_index.map_or(Some(0), |idx| idx.checked_add(1)).ok_or_else(|| {
+							let msg = "Call index doesn't fit into u8, index is 256";
+							syn::Error::new(method.sig.span(), msg)
+						})?,
+				};
+				last_index = Some(final_index);
+
+				if let Some(used_fn) = indices.insert(final_index, method.sig.ident.clone()) {
+					let msg = format!(
+						"Call indices are conflicting: Both functions {} and {} are at index {}",
+						used_fn, method.sig.ident, final_index,
+					);
+					let mut err = syn::Error::new(used_fn.span(), &msg);
+					err.combine(syn::Error::new(method.sig.ident.span(), msg));
+					return Err(err)
+				}
 
 				let mut args = vec![];
 				for arg in method.sig.inputs.iter_mut().skip(1) {
@@ -223,7 +281,13 @@ impl CallDef {
 
 				let docs = get_doc_literals(&method.attrs);
 
-				methods.push(CallVariantDef { name: method.sig.ident.clone(), weight, args, docs });
+				methods.push(CallVariantDef {
+					name: method.sig.ident.clone(),
+					weight,
+					call_index: final_index,
+					args,
+					docs,
+				});
 			} else {
 				let msg = "Invalid pallet::call, only method accepted";
 				return Err(syn::Error::new(impl_item.span(), msg))

frame/support/src/lib.rs

@@ -1553,6 +1553,15 @@ pub mod pallet_prelude {
 /// used using `#[pallet::compact]`, function must return `DispatchResultWithPostInfo` or
 /// `DispatchResult`.
 ///
+/// Each dispatchable may also be annotated with the `#[pallet::call_index($idx)]` attribute,
+/// which defines and sets the codec index for the dispatchable function in the `Call` enum.
+///
+/// All call indexes start from 0, until it encounters a dispatchable function with a defined
+/// call index. The dispatchable function that lexically follows the function with a defined
+/// call index will have that call index, but incremented by 1, e.g. if there are 3
+/// dispatchable functions `fn foo`, `fn bar` and `fn qux` in that order, and only `fn bar` has
+/// a call index of 10, then `fn qux` will have an index of 11, instead of 1.
+///
 /// All arguments must implement `Debug`, `PartialEq`, `Eq`, `Decode`, `Encode`, `Clone`. For
 /// ease of use, bound the trait `Member` available in frame_support::pallet_prelude.
 ///
@@ -1566,16 +1575,18 @@ pub mod pallet_prelude {
 /// **WARNING**: modifying dispatchables, changing their order, removing some must be done with
 /// care. Indeed this will change the outer runtime call type (which is an enum with one
 /// variant per pallet), this outer runtime call can be stored on-chain (e.g. in
-/// pallet-scheduler). Thus migration might be needed.
+/// pallet-scheduler). Thus migration might be needed. To mitigate against some of this, the
+/// `#[pallet::call_index($idx)]` attribute can be used to fix the order of the dispatchable so
+/// that the `Call` enum encoding does not change after modification.
 ///
 /// ### Macro expansion
 ///
-/// The macro create an enum `Call` with one variant per dispatchable. This enum implements:
+/// The macro creates an enum `Call` with one variant per dispatchable. This enum implements:
 /// `Clone`, `Eq`, `PartialEq`, `Debug` (with stripped implementation in `not("std")`),
 /// `Encode`, `Decode`, `GetDispatchInfo`, `GetCallName`, `UnfilteredDispatchable`.
 ///
-/// The macro implement on `Pallet`, the `Callable` trait and a function `call_functions` which
-/// returns the dispatchable metadatas.
+/// The macro implement the `Callable` trait on `Pallet` and a function `call_functions` which
+/// returns the dispatchable metadata.
 ///
 /// # Extra constants: `#[pallet::extra_constants]` optional
 ///

frame/support/test/tests/pallet_ui/call_conflicting_indices.rs

@@ -0,0 +1,24 @@
+#[frame_support::pallet]
+mod pallet {
+	use frame_support::pallet_prelude::DispatchResultWithPostInfo;
+
+	#[pallet::config]
+	pub trait Config: frame_system::Config {}
+
+	#[pallet::pallet]
+	pub struct Pallet<T>(core::marker::PhantomData<T>);
+
+	#[pallet::call]
+	impl<T: Config> Pallet<T> {
+		#[pallet::weight(0)]
+		#[pallet::call_index(10)]
+		pub fn foo(origin: OriginFor<T>) -> DispatchResultWithPostInfo {}
+
+		#[pallet::weight(0)]
+		#[pallet::call_index(10)]
+		pub fn bar(origin: OriginFor<T>) -> DispatchResultWithPostInfo {}
+	}
+}
+
+fn main() {
+}

frame/support/test/tests/pallet_ui/call_conflicting_indices.stderr

@@ -0,0 +1,11 @@
+error: Call indices are conflicting: Both functions foo and bar are at index 10
+  --> tests/pallet_ui/call_conflicting_indices.rs:15:10
+   |
+15 |         pub fn foo(origin: OriginFor<T>) -> DispatchResultWithPostInfo {}
+   |                ^^^
+
+error: Call indices are conflicting: Both functions foo and bar are at index 10
+  --> tests/pallet_ui/call_conflicting_indices.rs:19:10
+   |
+19 |         pub fn bar(origin: OriginFor<T>) -> DispatchResultWithPostInfo {}
+   |                ^^^

frame/support/test/tests/pallet_ui/call_invalid_attr.rs

@@ -0,0 +1,20 @@
+#[frame_support::pallet]
+mod pallet {
+	use frame_support::pallet_prelude::DispatchResultWithPostInfo;
+	use frame_system::pallet_prelude::OriginFor;
+
+	#[pallet::config]
+	pub trait Config: frame_system::Config {}
+
+	#[pallet::pallet]
+	pub struct Pallet<T>(core::marker::PhantomData<T>);
+
+	#[pallet::call]
+	impl<T: Config> Pallet<T> {
+		#[pallet::weird_attr]
+		pub fn foo(origin: OriginFor<T>) -> DispatchResultWithPostInfo {}
+	}
+}
+
+fn main() {
+}

frame/support/test/tests/pallet_ui/call_invalid_attr.stderr

@@ -0,0 +1,5 @@
+error: expected `weight` or `call_index`
+  --> tests/pallet_ui/call_invalid_attr.rs:14:13
+   |
+14 |         #[pallet::weird_attr]
+   |                   ^^^^^^^^^^

frame/support/test/tests/pallet_ui/call_invalid_index.rs

@@ -0,0 +1,21 @@
+#[frame_support::pallet]
+mod pallet {
+	use frame_support::pallet_prelude::DispatchResultWithPostInfo;
+	use frame_system::pallet_prelude::OriginFor;
+
+	#[pallet::config]
+	pub trait Config: frame_system::Config {}
+
+	#[pallet::pallet]
+	pub struct Pallet<T>(core::marker::PhantomData<T>);
+
+	#[pallet::call]
+	impl<T: Config> Pallet<T> {
+		#[pallet::weight(0)]
+		#[pallet::call_index(256)]
+		pub fn foo(origin: OriginFor<T>) -> DispatchResultWithPostInfo {}
+	}
+}
+
+fn main() {
+}

frame/support/test/tests/pallet_ui/call_invalid_index.stderr

@@ -0,0 +1,5 @@
+error: number too large to fit in target type
+  --> tests/pallet_ui/call_invalid_index.rs:15:24
+   |
+15 |         #[pallet::call_index(256)]
+   |                              ^^^

frame/support/test/tests/pallet_ui/call_multiple_call_index.rs

@@ -0,0 +1,22 @@
+#[frame_support::pallet]
+mod pallet {
+	use frame_support::pallet_prelude::DispatchResultWithPostInfo;
+	use frame_system::pallet_prelude::OriginFor;
+
+	#[pallet::config]
+	pub trait Config: frame_system::Config {}
+
+	#[pallet::pallet]
+	pub struct Pallet<T>(core::marker::PhantomData<T>);
+
+	#[pallet::call]
+	impl<T: Config> Pallet<T> {
+		#[pallet::weight(0)]
+		#[pallet::call_index(1)]
+		#[pallet::call_index(2)]
+		pub fn foo(origin: OriginFor<T>) -> DispatchResultWithPostInfo {}
+	}
+}
+
+fn main() {
+}

frame/support/test/tests/pallet_ui/call_multiple_call_index.stderr

@@ -0,0 +1,5 @@
+error: Invalid pallet::call, too many call_index attributes given
+  --> tests/pallet_ui/call_multiple_call_index.rs:17:7
+   |
+17 |         pub fn foo(origin: OriginFor<T>) -> DispatchResultWithPostInfo {}
+   |             ^^