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

Conversation

@apopiak
Copy link
Contributor

@apopiak apopiak commented Jun 17, 2020

Note: This PR is only to reproduce the error for #6379

Add duplicate dispatchable to trigger confusing error:

error: Implicit conversion to privileged function has been removed. First parameter of dispatch should be marked `origin`. For root-matching dispatch, also add `ensure_root(origin)?`.
   --> pallets/template/src/lib.rs:63:1
    |
63  | / decl_module! {
64  | |     /// The module declaration.
65  | |     pub struct Module<T: Trait> for enum Call where origin: T::Origin {
66  | |         // Initializing errors
...   |
116 | |     }
117 | | }
    | |_^
    |
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

@parity-cla-bot
Copy link

It looks like @apopiak signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@github-actions github-actions bot added the A3-in_progress Pull request is in progress. No review needed at this stage. label Jun 17, 2020
@apopiak apopiak added A1-onice B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Jun 17, 2020
@shawntabrizi
Copy link
Member

@apopiak is this specific to on_intialize or can you repro this for other such functions?

In the case of on_initialize this is a macro matching issue.

The first on_initialize() function is captured using our macro and is expected. The next one goes through our normal dispatch function logic, which does not allow any dispatchable that does not contain origin as the first parameter.

I would guess this problem only occurs with "reserved functions" like on_initialize, on_finalize, on_runtime_upgrade, etc...

Not a macro god, so idk the fix. :P

@apopiak
Copy link
Contributor Author

apopiak commented Jun 17, 2020

I originally triggered it with on_runtime_upgrade, so I guess it's likely that it's only the "reserved functions"

@shawntabrizi
Copy link
Member

Yeah. So the root of the issue is as I described. We expect one instance of on_runtime_upgrade()

This instance is handled special in our macro parsing logic.

After the first instance, the function is treated like any arbitrary function, so the name is not considered for the parsing. At this point we only consider the function parameters, and we see that this dispatchable function is missing origin, so we complain.

Its a valid issue, hopefully someone knows a fix.

@apopiak
Copy link
Contributor Author

apopiak commented Jun 17, 2020

Verified that this is not reproducible via duplicating a "regular dispatchable", so it's an issue with the "reserved functions".

@bkchr
Copy link
Member

bkchr commented Jun 17, 2020

Will be fixed by: #6384

@bkchr bkchr closed this Jun 17, 2020
@bkchr bkchr deleted the apopiak-confusing-error branch June 17, 2020 21:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants