Enforce pub calls in pallets (#9085)

* make all extrinsics public so they are available from outside * Impl * fix * more fix * more pub * few more * merge fix * fix ui test * fix ui test Co-authored-by: Alexander Popiak <alexander.popiak@parity.io> Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com> Co-authored-by: Bastian Köcher <info@kchr.de>
paritytech · Jun 12, 2021 · 9e42949 · 9e42949
1 parent 1f16a6a
commit 9e42949
Show file tree

Hide file tree

Showing 32 changed files with 154 additions and 78 deletions.
diff --git a/frame/authorship/src/lib.rs b/frame/authorship/src/lib.rs
@@ -226,7 +226,7 @@ pub mod pallet {
 	impl<T: Config> Pallet<T> {
 		/// Provide a set of uncles.
 		#[pallet::weight((0, DispatchClass::Mandatory))]
-		fn set_uncles(origin: OriginFor<T>, new_uncles: Vec<T::Header>) -> DispatchResult {
+		pub fn set_uncles(origin: OriginFor<T>, new_uncles: Vec<T::Header>) -> DispatchResult {
 			ensure_none(origin)?;
 			ensure!(new_uncles.len() <= MAX_UNCLES, Error::<T>::TooManyUncles);
 

diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs
@@ -786,7 +786,7 @@ pub mod pallet {
 		///
 		/// This check can be turned off by setting the value to `None`.
 		#[pallet::weight(T::DbWeight::get().writes(1))]
-		fn set_minimum_untrusted_score(
+		pub fn set_minimum_untrusted_score(
 			origin: OriginFor<T>,
 			maybe_next_score: Option<ElectionScore>,
 		) -> DispatchResult {

diff --git a/frame/grandpa/src/lib.rs b/frame/grandpa/src/lib.rs
@@ -190,7 +190,7 @@ pub mod pallet {
 		/// against the extracted offender. If both are valid, the offence
 		/// will be reported.
 		#[pallet::weight(T::WeightInfo::report_equivocation(key_owner_proof.validator_count()))]
-		fn report_equivocation(
+		pub fn report_equivocation(
 			origin: OriginFor<T>,
 			equivocation_proof: EquivocationProof<T::Hash, T::BlockNumber>,
 			key_owner_proof: T::KeyOwnerProof,
@@ -236,7 +236,7 @@ pub mod pallet {
 		/// will start the new authority set using the given finalized block as base.
 		/// Only callable by root.
 		#[pallet::weight(T::WeightInfo::note_stalled())]
-		fn note_stalled(
+		pub fn note_stalled(
 			origin: OriginFor<T>,
 			delay: T::BlockNumber,
 			best_finalized_block_number: T::BlockNumber,

diff --git a/frame/scheduler/src/lib.rs b/frame/scheduler/src/lib.rs
@@ -867,7 +867,7 @@ mod tests {
 		#[pallet::call]
 		impl<T: Config> Pallet<T> where <T as system::Config>::Origin: OriginTrait<PalletsOrigin = OriginCaller>  {
 			#[pallet::weight(*weight)]
-			fn log(origin: OriginFor<T>, i: u32, weight: Weight) -> DispatchResult {
+			pub fn log(origin: OriginFor<T>, i: u32, weight: Weight) -> DispatchResult {
 				Self::deposit_event(Event::Logged(i, weight));
 				LOG.with(|log| {
 					log.borrow_mut().push((origin.caller().clone(), i));
@@ -876,7 +876,7 @@ mod tests {
 			}
 
 			#[pallet::weight(*weight)]
-			fn log_without_filter(origin: OriginFor<T>, i: u32, weight: Weight) -> DispatchResult {
+			pub fn log_without_filter(origin: OriginFor<T>, i: u32, weight: Weight) -> DispatchResult {
 				Self::deposit_event(Event::Logged(i, weight));
 				LOG.with(|log| {
 					log.borrow_mut().push((origin.caller().clone(), i));

diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs
@@ -1959,7 +1959,7 @@ pub mod pallet {
 		///   Paying even a dead controller is cheaper weight-wise. We don't do any refunds here.
 		/// # </weight>
 		#[pallet::weight(T::WeightInfo::payout_stakers_alive_staked(T::MaxNominatorRewardedPerValidator::get()))]
-		pub(super) fn payout_stakers(
+		pub fn payout_stakers(
 			origin: OriginFor<T>,
 			validator_stash: T::AccountId,
 			era: EraIndex,

diff --git a/frame/sudo/src/mock.rs b/frame/sudo/src/mock.rs
@@ -45,7 +45,7 @@ pub mod logger {
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
 		#[pallet::weight(*weight)]
-		pub(crate) fn privileged_i32_log(
+		pub fn privileged_i32_log(
 			origin: OriginFor<T>,
 			i: i32,
 			weight: Weight
@@ -58,7 +58,7 @@ pub mod logger {
 		}
 
 		#[pallet::weight(*weight)]
-		pub(crate) fn non_privileged_log(
+		pub fn non_privileged_log(
 			origin: OriginFor<T>,
 			i: i32,
 			weight: Weight

diff --git a/frame/support/procedural/src/pallet/parse/call.rs b/frame/support/procedural/src/pallet/parse/call.rs
@@ -149,6 +149,18 @@ impl CallDef {
 		let mut methods = vec![];
 		for impl_item in &mut item.items {
 			if let syn::ImplItem::Method(method) = impl_item {
+				if !matches!(method.vis, syn::Visibility::Public(_)) {
+					let msg = "Invalid pallet::call, dispatchable function must be public: \
+						`pub fn`";
+
+					let span = match method.vis {
+						syn::Visibility::Inherited => method.sig.span(),
+						_ => method.vis.span(),
+					};
+
+					return Err(syn::Error::new(span, msg));
+				}
+
 				match method.sig.inputs.first() {
 					None => {
 						let msg = "Invalid pallet::call, must have at least origin arg";

diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs
@@ -1438,7 +1438,7 @@ pub mod pallet_prelude {
 /// impl<T: Config> Pallet<T> {
 /// 	/// $some_doc
 /// 	#[pallet::weight($ExpressionResultingInWeight)]
-/// 	$vis fn $fn_name(
+/// 	pub fn $fn_name(
 /// 		origin: OriginFor<T>,
 /// 		$some_arg: $some_type,
 /// 		// or with compact attribute: #[pallet::compact] $some_arg: $some_type,
@@ -1897,7 +1897,7 @@ pub mod pallet_prelude {
 /// 	impl<T: Config> Pallet<T> {
 /// 		/// Doc comment put in metadata
 /// 		#[pallet::weight(0)] // Defines weight for call (function parameters are in scope)
-/// 		fn toto(
+/// 		pub fn toto(
 /// 			origin: OriginFor<T>,
 /// 			#[pallet::compact] _foo: u32,
 /// 		) -> DispatchResultWithPostInfo {

diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs
@@ -133,11 +133,11 @@ pub mod pallet {
 
 	#[pallet::call]
 	impl<T: Config> Pallet<T>
-	where T::AccountId: From<SomeType1> + From<SomeType3> + SomeAssociation1
+		where T::AccountId: From<SomeType1> + From<SomeType3> + SomeAssociation1
 	{
 		/// Doc comment put in metadata
 		#[pallet::weight(Weight::from(*_foo))]
-		fn foo(
+		pub fn foo(
 			origin: OriginFor<T>,
 			#[pallet::compact] _foo: u32,
 			_bar: u32,
@@ -152,7 +152,7 @@ pub mod pallet {
 		/// Doc comment put in metadata
 		#[pallet::weight(1)]
 		#[frame_support::transactional]
-		fn foo_transactional(
+		pub fn foo_transactional(
 			_origin: OriginFor<T>,
 			#[pallet::compact] foo: u32,
 		) -> DispatchResultWithPostInfo {
@@ -166,7 +166,7 @@ pub mod pallet {
 
 		// Test for DispatchResult return type
 		#[pallet::weight(1)]
-		fn foo_no_post_info(
+		pub fn foo_no_post_info(
 			_origin: OriginFor<T>,
 		) -> DispatchResult {
 			Ok(())

diff --git a/frame/support/test/tests/pallet_compatibility.rs b/frame/support/test/tests/pallet_compatibility.rs
@@ -123,7 +123,7 @@ pub mod pallet {
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
 		#[pallet::weight(<T::Balance as Into<Weight>>::into(new_value.clone()))]
-		fn set_dummy(
+		pub fn set_dummy(
 			origin: OriginFor<T>,
 			#[pallet::compact] new_value: T::Balance
 		) -> DispatchResultWithPostInfo {

diff --git a/frame/support/test/tests/pallet_compatibility_instance.rs b/frame/support/test/tests/pallet_compatibility_instance.rs
@@ -113,7 +113,7 @@ pub mod pallet {
 	#[pallet::call]
 	impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		#[pallet::weight(<T::Balance as Into<Weight>>::into(new_value.clone()))]
-		fn set_dummy(
+		pub fn set_dummy(
 			origin: OriginFor<T>,
 			#[pallet::compact] new_value: T::Balance
 		) -> DispatchResultWithPostInfo {

diff --git a/frame/support/test/tests/pallet_instance.rs b/frame/support/test/tests/pallet_instance.rs
@@ -81,7 +81,7 @@ pub mod pallet {
 	impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		/// Doc comment put in metadata
 		#[pallet::weight(Weight::from(*_foo))]
-		fn foo(origin: OriginFor<T>, #[pallet::compact] _foo: u32) -> DispatchResultWithPostInfo {
+		pub fn foo(origin: OriginFor<T>, #[pallet::compact] _foo: u32) -> DispatchResultWithPostInfo {
 			let _ = origin;
 			Self::deposit_event(Event::Something(3));
 			Ok(().into())
@@ -90,7 +90,7 @@ pub mod pallet {
 		/// Doc comment put in metadata
 		#[pallet::weight(1)]
 		#[frame_support::transactional]
-		fn foo_transactional(
+		pub fn foo_transactional(
 			origin: OriginFor<T>,
 			#[pallet::compact] _foo: u32
 		) -> DispatchResultWithPostInfo {

diff --git a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.rs b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.rs
@@ -17,7 +17,7 @@ mod pallet {
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
 		#[pallet::weight(0)]
-		fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
+		pub fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
 			Ok(().into())
 		}
 	}

diff --git a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.stderr b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.stderr
@@ -1,27 +1,27 @@
 error[E0369]: binary operation `==` cannot be applied to type `&<T as pallet::Config>::Bar`
-  --> $DIR/call_argument_invalid_bound.rs:20:37
+  --> $DIR/call_argument_invalid_bound.rs:20:41
    |
-20 |         fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
-   |                                           ^
+20 |         pub fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
+   |                                               ^
    |
 help: consider further restricting this bound
    |
 17 |     #[pallet::call + std::cmp::PartialEq]
    |                    ^^^^^^^^^^^^^^^^^^^^^
 
 error[E0277]: the trait bound `<T as pallet::Config>::Bar: Clone` is not satisfied
-  --> $DIR/call_argument_invalid_bound.rs:20:37
+  --> $DIR/call_argument_invalid_bound.rs:20:41
    |
-20 |         fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
-   |                                           ^ the trait `Clone` is not implemented for `<T as pallet::Config>::Bar`
+20 |         pub fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
+   |                                               ^ the trait `Clone` is not implemented for `<T as pallet::Config>::Bar`
    |
    = note: required by `clone`
 
 error[E0277]: `<T as pallet::Config>::Bar` doesn't implement `std::fmt::Debug`
-  --> $DIR/call_argument_invalid_bound.rs:20:37
+  --> $DIR/call_argument_invalid_bound.rs:20:41
    |
-20 |         fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
-   |                                           ^ `<T as pallet::Config>::Bar` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug`
+20 |         pub fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
+   |                                               ^ `<T as pallet::Config>::Bar` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug`
    |
    = help: the trait `std::fmt::Debug` is not implemented for `<T as pallet::Config>::Bar`
    = note: required because of the requirements on the impl of `std::fmt::Debug` for `&<T as pallet::Config>::Bar`

diff --git a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.rs b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.rs
@@ -17,7 +17,7 @@ mod pallet {
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
 		#[pallet::weight(0)]
-		fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
+		pub fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
 			Ok(().into())
 		}
 	}

diff --git a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.stderr b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.stderr
@@ -1,8 +1,8 @@
 error[E0277]: the trait bound `<T as pallet::Config>::Bar: WrapperTypeDecode` is not satisfied
-   --> $DIR/call_argument_invalid_bound_2.rs:20:37
+   --> $DIR/call_argument_invalid_bound_2.rs:20:41
     |
-20  |         fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
-    |                                           ^ the trait `WrapperTypeDecode` is not implemented for `<T as pallet::Config>::Bar`
+20  |         pub fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
+    |                                               ^ the trait `WrapperTypeDecode` is not implemented for `<T as pallet::Config>::Bar`
     |
    ::: /usr/local/cargo/registry/src/github.com-1ecc6299db9ec823/parity-scale-codec-2.1.1/src/codec.rs:277:18
     |
@@ -12,10 +12,10 @@ error[E0277]: the trait bound `<T as pallet::Config>::Bar: WrapperTypeDecode` is
     = note: required because of the requirements on the impl of `Decode` for `<T as pallet::Config>::Bar`
 
 error[E0277]: the trait bound `<T as pallet::Config>::Bar: WrapperTypeEncode` is not satisfied
-   --> $DIR/call_argument_invalid_bound_2.rs:20:37
+   --> $DIR/call_argument_invalid_bound_2.rs:20:41
     |
-20  |         fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
-    |                                           ^ the trait `WrapperTypeEncode` is not implemented for `<T as pallet::Config>::Bar`
+20  |         pub fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
+    |                                               ^ the trait `WrapperTypeEncode` is not implemented for `<T as pallet::Config>::Bar`
     |
    ::: /usr/local/cargo/registry/src/github.com-1ecc6299db9ec823/parity-scale-codec-2.1.1/src/codec.rs:216:21
     |
@@ -25,29 +25,29 @@ error[E0277]: the trait bound `<T as pallet::Config>::Bar: WrapperTypeEncode` is
     = note: required because of the requirements on the impl of `pallet::_::_parity_scale_codec::Encode` for `<T as pallet::Config>::Bar`
 
 error[E0369]: binary operation `==` cannot be applied to type `&<T as pallet::Config>::Bar`
-  --> $DIR/call_argument_invalid_bound_2.rs:20:37
+  --> $DIR/call_argument_invalid_bound_2.rs:20:41
    |
-20 |         fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
-   |                                           ^
+20 |         pub fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
+   |                                               ^
    |
 help: consider further restricting this bound
    |
 17 |     #[pallet::call + std::cmp::PartialEq]
    |                    ^^^^^^^^^^^^^^^^^^^^^
 
 error[E0277]: the trait bound `<T as pallet::Config>::Bar: Clone` is not satisfied
-  --> $DIR/call_argument_invalid_bound_2.rs:20:37
+  --> $DIR/call_argument_invalid_bound_2.rs:20:41
    |
-20 |         fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
-   |                                           ^ the trait `Clone` is not implemented for `<T as pallet::Config>::Bar`
+20 |         pub fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
+   |                                               ^ the trait `Clone` is not implemented for `<T as pallet::Config>::Bar`
    |
    = note: required by `clone`
 
 error[E0277]: `<T as pallet::Config>::Bar` doesn't implement `std::fmt::Debug`
-  --> $DIR/call_argument_invalid_bound_2.rs:20:37
+  --> $DIR/call_argument_invalid_bound_2.rs:20:41
    |
-20 |         fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
-   |                                           ^ `<T as pallet::Config>::Bar` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug`
+20 |         pub fn foo(origin: OriginFor<T>, bar: T::Bar) -> DispatchResultWithPostInfo {
+   |                                               ^ `<T as pallet::Config>::Bar` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug`
    |
    = help: the trait `std::fmt::Debug` is not implemented for `<T as pallet::Config>::Bar`
    = note: required because of the requirements on the impl of `std::fmt::Debug` for `&<T as pallet::Config>::Bar`

diff --git a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.rs b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.rs
@@ -19,7 +19,7 @@ mod pallet {
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
 		#[pallet::weight(0)]
-		fn foo(origin: OriginFor<T>, bar: Bar) -> DispatchResultWithPostInfo {
+		pub fn foo(origin: OriginFor<T>, bar: Bar) -> DispatchResultWithPostInfo {
 			Ok(().into())
 		}
 	}

diff --git a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.stderr b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.stderr
@@ -1,24 +1,24 @@
 error[E0369]: binary operation `==` cannot be applied to type `&Bar`
-  --> $DIR/call_argument_invalid_bound_3.rs:22:37
+  --> $DIR/call_argument_invalid_bound_3.rs:22:41
    |
-22 |         fn foo(origin: OriginFor<T>, bar: Bar) -> DispatchResultWithPostInfo {
-   |                                           ^^^
+22 |         pub fn foo(origin: OriginFor<T>, bar: Bar) -> DispatchResultWithPostInfo {
+   |                                               ^^^
    |
    = note: an implementation of `std::cmp::PartialEq` might be missing for `&Bar`
 
 error[E0277]: the trait bound `Bar: Clone` is not satisfied
-  --> $DIR/call_argument_invalid_bound_3.rs:22:37
+  --> $DIR/call_argument_invalid_bound_3.rs:22:41
    |
-22 |         fn foo(origin: OriginFor<T>, bar: Bar) -> DispatchResultWithPostInfo {
-   |                                           ^^^ the trait `Clone` is not implemented for `Bar`
+22 |         pub fn foo(origin: OriginFor<T>, bar: Bar) -> DispatchResultWithPostInfo {
+   |                                               ^^^ the trait `Clone` is not implemented for `Bar`
    |
    = note: required by `clone`
 
 error[E0277]: `Bar` doesn't implement `std::fmt::Debug`
-  --> $DIR/call_argument_invalid_bound_3.rs:22:37
+  --> $DIR/call_argument_invalid_bound_3.rs:22:41
    |
-22 |         fn foo(origin: OriginFor<T>, bar: Bar) -> DispatchResultWithPostInfo {
-   |                                           ^^^ `Bar` cannot be formatted using `{:?}`
+22 |         pub fn foo(origin: OriginFor<T>, bar: Bar) -> DispatchResultWithPostInfo {
+   |                                               ^^^ `Bar` cannot be formatted using `{:?}`
    |
    = help: the trait `std::fmt::Debug` is not implemented for `Bar`
    = note: add `#[derive(Debug)]` or manually implement `std::fmt::Debug`

diff --git a/frame/support/test/tests/pallet_ui/call_invalid_origin_type.rs b/frame/support/test/tests/pallet_ui/call_invalid_origin_type.rs
@@ -14,7 +14,7 @@ mod pallet {
 
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
-		fn foo(origin: u8) {}
+		pub fn foo(origin: u8) {}
 	}
 }
 

diff --git a/frame/support/test/tests/pallet_ui/call_invalid_origin_type.stderr b/frame/support/test/tests/pallet_ui/call_invalid_origin_type.stderr
@@ -1,11 +1,11 @@
 error: Invalid type: expected `OriginFor<T>`
-  --> $DIR/call_invalid_origin_type.rs:17:18
+  --> $DIR/call_invalid_origin_type.rs:17:22
    |
-17 |         fn foo(origin: u8) {}
-   |                        ^^
+17 |         pub fn foo(origin: u8) {}
+   |                            ^^
 
 error: expected `OriginFor`
-  --> $DIR/call_invalid_origin_type.rs:17:18
+  --> $DIR/call_invalid_origin_type.rs:17:22
    |
-17 |         fn foo(origin: u8) {}
-   |                        ^^
+17 |         pub fn foo(origin: u8) {}
+   |                            ^^
diff --git a/frame/support/test/tests/pallet_ui/call_invalid_return.rs b/frame/support/test/tests/pallet_ui/call_invalid_return.rs
@@ -14,7 +14,7 @@ mod pallet {
 
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
-		fn foo(origin: OriginFor<T>) -> ::DispatchResult { todo!() }
+		pub fn foo(origin: OriginFor<T>) -> ::DispatchResult { todo!() }
 	}
 }
 

diff --git a/frame/support/test/tests/pallet_ui/call_invalid_return.stderr b/frame/support/test/tests/pallet_ui/call_invalid_return.stderr
@@ -1,5 +1,5 @@
 error: expected `DispatchResultWithPostInfo` or `DispatchResult`
-  --> $DIR/call_invalid_return.rs:17:35
+  --> $DIR/call_invalid_return.rs:17:39
    |
-17 |         fn foo(origin: OriginFor<T>) -> ::DispatchResult { todo!() }
-   |                                         ^^
+17 |         pub fn foo(origin: OriginFor<T>) -> ::DispatchResult { todo!() }
+   |                                             ^^