Introduce #[pallet::call_index] attribute to dispatchables

[KiChjang]

Fixes #10408.

Per title, this PR adds a new attribute #[pallet::call_index($idx)] to dispatchable functions, which can be used to fix the codec order of the corresponding enum variant in the Call enum.

[KiChjang]

There isn't any logic to check whether an index is duplicated, perhaps I should first include that.

[kianenigma]

There isn't any logic to check whether an index is duplicated, perhaps I should first include that.

Wouldn't the rust compiler already raise an error if you do that? If so, you mean a better error message, rather than letting the user deal with the rustc error?

[kianenigma]

There isn't any logic to check whether an index is duplicated, perhaps I should first include that.

Wouldn't the rust compiler already raise an error if you do that? If so, you mean a better error message, rather than letting the user deal with the rustc error?

UPDATE: having a peek at the code, I don't see where the new value is used. Eventually, the functions that are annotated with #pallet::call are used to generate enum Call { Fn1(..), Fn2(..) }. We should just change this to enum Call { Fn1(..) = x, Fn2(..) = y } and let rustc handle the errors for us as well. I belive this is how the construct_runtime also works.

[KiChjang]

@kianenigma #10408 (comment) is why we can't assign discriminants to non-unit enum variants. In short, it's an unsupported Rust feature.

[KiChjang]

construct_runtime in fact uses #[codec(index = $idx)] as well, instead of enum discriminants.

[shawntabrizi]

can you briefly add a description here of what happens if you use the call index in some placed, but not in others?

Can we make it so that call index must be used everywhere or nowhere?

[shawntabrizi]

Honestly, probably call index should just be mandatory. We could allow users to ignore it if they use pallet::dev_mode

[kianenigma]

In the spirit of keeping the FRAME boilerplate minimal, I don't think mandating call-indices should be mandatory. I like how it works in construct_runtime and find it intuitive: You have the default order, and you can overwrite anything that you want. Duplicates will be detected at compile time.

[shawntabrizi]

This looks good to me as is, however I am coming to the conclusion that we may want to handle calls differently in the future.

What if rather than having the call enum be generated by including a bunch of functions inside #[pallet::call], we instead start by the user simply creating an explicit call enum with just names.

Then we would implement traits on top of each enum, for example Dispatchable which then maps each enum variant to the function it executes, WeightInfo which maps each enum variant to the weight function, and even things like PreExecute, a function or set of checks which is executed before the main function, PostExecute, and whatever other million of things we want.

Basically, by hiding the call enum from the developer, we have not simplified things much, but have made extending calls much less flexible.

Also, this would allow us to have call function definitions in completely different files, and stuff like that...

just an idea

[shawntabrizi]

bot rebase

[kianenigma]

Thanks! cc @joepetrowski

[KiChjang]

bot merge