Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Emit error when construct_runtime imports a non-existent pallet part #8949

Merged
48 commits merged into from
Jun 16, 2021

Conversation

KiChjang
Copy link
Contributor

@KiChjang KiChjang commented May 30, 2021

Fixes #8945.

This PR causes an error to be thrown during compilation when Substrate detects that your construct_runtime is importing a pallet part that has not been declared by the pallet in question.

The expected UI for this error looks like the following:

error: Pallet does not have #[pallet::call] defined, perhaps you should remove it from `construct_runtime`?
  --> $DIR/undefined_call_part.rs:3:1
   |
3  |   #[frame_support::pallet]
   |   ^^^^^^^^^^^^^^^^^^^^^^^^
...
12 | / construct_runtime! {
13 | |     pub enum Runtime where
14 | |         Block = Block,
15 | |         NodeBlock = Block,
...  |
20 | |     }
21 | | }
   | |_- in this macro invocation
   |
   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

polkadot companion: paritytech/polkadot#3216

@github-actions github-actions bot added the A3-in_progress Pull request is in progress. No review needed at this stage. label May 30, 2021
@KiChjang KiChjang added B5-clientnoteworthy C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. and removed B5-clientnoteworthy labels May 30, 2021
@KiChjang
Copy link
Contributor Author

I wanted to add a UI test for this, and I had the following under the construct_runtime_ui directory:

use frame_support::construct_runtime;

#[frame_support::pallet]
mod pallet {
	#[pallet::config]
	pub trait Config: frame_system::Config {}

	#[pallet::pallet]
	pub struct Pallet<T>(_);
}

construct_runtime! {
	pub enum Runtime where
		Block = Block,
		NodeBlock = Block,
		UncheckedExtrinsic = UncheckedExtrinsic
	{
		System: system::{Pallet},
		Pallet: pallet::{Pallet, Call},
	}
}

fn main() {}

This does generate the compiler error that I expect, but with a lot more junk associated with it:

error: Pallet does not have #[pallet::call] defined. Did you forget to include it?
  --> $DIR/undefined_call_part.rs:3:1
   |
3  |   #[frame_support::pallet]
   |   ^^^^^^^^^^^^^^^^^^^^^^^^
...
12 | / construct_runtime! {
13 | |     pub enum Runtime where
14 | |         Block = Block,
15 | |         NodeBlock = Block,
...  |
20 | |     }
21 | | }
   | |_- in this macro invocation
   |
   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0433]: failed to resolve: use of undeclared crate or module `system`
  --> $DIR/undefined_call_part.rs:18:11
   |
18 |         System: system::{Pallet},
   |                 ^^^^^^ use of undeclared crate or module `system`

error[E0412]: cannot find type `UncheckedExtrinsic` in this scope
  --> $DIR/undefined_call_part.rs:16:24
   |
16 |         UncheckedExtrinsic = UncheckedExtrinsic
   |                              ^^^^^^^^^^^^^^^^^^ not found in this scope
   |
help: consider importing this struct
   |
1  | use sp_runtime::generic::UncheckedExtrinsic;
   |

error[E0412]: cannot find type `Block` in this scope
  --> $DIR/undefined_call_part.rs:15:15
   |
15 |         NodeBlock = Block,
   |                     ^^^^^ not found in this scope
   |
help: consider importing one of these items
   |
1  | use sp_runtime::generic::Block;
   |
1  | use sp_runtime::testing::Block;
   |
1  | use sp_runtime::traits::Block;
   |

error[E0412]: cannot find type `Block` in this scope
  --> $DIR/undefined_call_part.rs:14:11
   |
14 |         Block = Block,
   |                 ^^^^^ not found in this scope
   |
help: consider importing one of these items
   |
1  | use sp_runtime::generic::Block;
   |
1  | use sp_runtime::testing::Block;
   |
1  | use sp_runtime::traits::Block;
   |

error[E0433]: failed to resolve: use of undeclared crate or module `system`
  --> $DIR/undefined_call_part.rs:12:1
   |
12 | / construct_runtime! {
13 | |     pub enum Runtime where
14 | |         Block = Block,
15 | |         NodeBlock = Block,
...  |
20 | |     }
21 | | }
   | |_^ not found in `system`
   |
   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider importing this enum
   |
1  | use frame_system::RawOrigin;
   |

error[E0277]: the trait bound `Runtime: pallet::Config` is not satisfied
  --> $DIR/undefined_call_part.rs:12:1
   |
12 | / construct_runtime! {
13 | |     pub enum Runtime where
14 | |         Block = Block,
15 | |         NodeBlock = Block,
...  |
20 | |     }
21 | | }
   | |_^ the trait `pallet::Config` is not implemented for `Runtime`
   |
   = note: required because of the requirements on the impl of `Callable<Runtime>` for `pallet::Pallet<Runtime>`
   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `Runtime: pallet::Config` is not satisfied
  --> $DIR/undefined_call_part.rs:12:1
   |
12 | / construct_runtime! {
13 | |     pub enum Runtime where
14 | |         Block = Block,
15 | |         NodeBlock = Block,
...  |
20 | |     }
21 | | }
   | |_^ the trait `pallet::Config` is not implemented for `Runtime`
   |
   = note: required because of the requirements on the impl of `Callable<Runtime>` for `pallet::Pallet<Runtime>`
   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `Runtime: pallet::Config` is not satisfied
  --> $DIR/undefined_call_part.rs:12:1
   |
12 | / construct_runtime! {
13 | |     pub enum Runtime where
14 | |         Block = Block,
15 | |         NodeBlock = Block,
...  |
20 | |     }
21 | | }
   | |_^ the trait `pallet::Config` is not implemented for `Runtime`
   |
   = note: required because of the requirements on the impl of `Callable<Runtime>` for `pallet::Pallet<Runtime>`
   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `Runtime: pallet::Config` is not satisfied
   --> $DIR/undefined_call_part.rs:12:1
    |
12  | / construct_runtime! {
13  | |     pub enum Runtime where
14  | |         Block = Block,
15  | |         NodeBlock = Block,
...   |
20  | |     }
21  | | }
    | |_^ the trait `pallet::Config` is not implemented for `Runtime`
    |
   ::: /home/keith/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/parity-scale-codec-2.1.0/src/codec.rs:270:19
    |
270 |   pub trait Decode: Sized {
    |                     ----- required by this bound in `Decode`
    |
    = note: required because of the requirements on the impl of `Callable<Runtime>` for `pallet::Pallet<Runtime>`
    = note: required because it appears within the type `Call`
    = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `Runtime: pallet::Config` is not satisfied
  --> $DIR/undefined_call_part.rs:12:1
   |
12 | / construct_runtime! {
13 | |     pub enum Runtime where
14 | |         Block = Block,
15 | |         NodeBlock = Block,
...  |
20 | |     }
21 | | }
   | |_^ the trait `pallet::Config` is not implemented for `Runtime`
   |
  ::: /home/keith/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/parity-scale-codec-2.1.0/src/encode_like.rs:73:41
   |
73 |   pub trait EncodeLike<T: Encode = Self>: Sized + Encode {}
   |                                           ----- required by this bound in `EncodeLike`
   |
   = note: required because of the requirements on the impl of `Callable<Runtime>` for `pallet::Pallet<Runtime>`
   = note: required because it appears within the type `Call`
   = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `Runtime: pallet::Config` is not satisfied
   --> $DIR/undefined_call_part.rs:12:1
    |
12  | / construct_runtime! {
13  | |     pub enum Runtime where
14  | |         Block = Block,
15  | |         NodeBlock = Block,
...   |
20  | |     }
21  | | }
    | |_^ the trait `pallet::Config` is not implemented for `Runtime`
    |
   ::: $RUST/core/src/clone.rs
    |
    |   pub trait Clone: Sized {
    |                    ----- required by this bound in `Clone`
    |
    = note: required because of the requirements on the impl of `Callable<Runtime>` for `pallet::Pallet<Runtime>`
    = note: required because it appears within the type `Call`
    = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `Runtime: pallet::Config` is not satisfied
  --> $DIR/undefined_call_part.rs:12:1
   |
12 | / construct_runtime! {
13 | |     pub enum Runtime where
14 | |         Block = Block,
15 | |         NodeBlock = Block,
...  |
20 | |     }
21 | | }
   | |_^ the trait `pallet::Config` is not implemented for `Runtime`
   |
   = note: required because of the requirements on the impl of `Callable<Runtime>` for `pallet::Pallet<Runtime>`
   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `Runtime: pallet::Config` is not satisfied
  --> $DIR/undefined_call_part.rs:12:1
   |
12 | / construct_runtime! {
13 | |     pub enum Runtime where
14 | |         Block = Block,
15 | |         NodeBlock = Block,
...  |
20 | |     }
21 | | }
   | |_^ the trait `pallet::Config` is not implemented for `Runtime`
   |
   = note: required because of the requirements on the impl of `Callable<Runtime>` for `pallet::Pallet<Runtime>`
   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `Runtime: pallet::Config` is not satisfied
   --> $DIR/undefined_call_part.rs:12:1
    |
12  | / construct_runtime! {
13  | |     pub enum Runtime where
14  | |         Block = Block,
15  | |         NodeBlock = Block,
...   |
20  | |     }
21  | | }
    | |_^ the trait `pallet::Config` is not implemented for `Runtime`
    |
   ::: $RUST/core/src/result.rs
    |
    |   pub enum Result<T, E> {
    |                   - required by this bound in `Result`
    |
    = note: required because of the requirements on the impl of `Callable<Runtime>` for `pallet::Pallet<Runtime>`
    = note: required because it appears within the type `Call`
    = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

@gui1117
Copy link
Contributor

gui1117 commented May 31, 2021

yes this is an improvment,

a sidenote: when we implement this: #8084 use will only give part not to include, and construct_runtime should be able to get all the available part of a pallet. Thus we can check that the "part not to include" specified by the user are sensible.

That makes me think of yet another improvement: currently when people forget to specify in their cargo.toml to compile the pallet crate with "std" feature, then the genesis config type is not available, when they are confused why the compiler say it can't find GenesisConfig.
With a similar trick as you propose we can have a macro: __is_std_enabled! which would be called before accessing the genesis config type. The macro __is_std_enabled! would fail saying that the pallet is not compile with std, in case ppl failed their cargo.toml definition.

@KiChjang KiChjang force-pushed the kckyeung/pallet-parts-forward-decl branch from 2c47b6c to d40b71e Compare June 1, 2021 23:44
@paritytech paritytech deleted a comment from cla-bot-2021 bot Jun 4, 2021
@KiChjang
Copy link
Contributor Author

KiChjang commented Jun 4, 2021

So I'm really liking the suggestion made by @shawntabrizi over the FRAME steering call, where we create a brand new construct_runtime_v2 macro and document it so that it only works for FRAMEv2 macros, instead of trying to make FRAMEv1 macros forward-compatible with this PR.

This has an intended side effect of telling people "hey, look at all the cool stuff you can do with FRAMEv2" and incentivizing them to migrate over to v2 as soon as possible.

@KiChjang KiChjang marked this pull request as ready for review June 6, 2021 05:55
@KiChjang
Copy link
Contributor Author

Ok, CI keeps choking on the tests/pallet_ui/storage_info_unsatisfied_nmap.rs test. Tried changing the test expectations for that file, but then the output matches the unchanged UI expectation. This is a real Heisenbug.

@KiChjang KiChjang requested a review from a team as a code owner June 15, 2021 00:20
@KiChjang KiChjang force-pushed the kckyeung/pallet-parts-forward-decl branch from de53045 to e6ca013 Compare June 15, 2021 06:03
@KiChjang
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Jun 16, 2021

Trying merge.

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue compiler error when construct_runtime attempts to import a non-existing pallet part
4 participants