-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Extend PalletInfoAccess with module_name and crate_version method #9690
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code is good to me (top message should be updated to reflect the current state of the PR)
I don't know if this will fix Gav request. Because contrary to method CallMetadata which list all the modules, this PR only allow to get the crate version for a specific module, it is not possible to list all modules index and crate versions as far as I can see.
(But we should be able to do so by introducing a new trait similar to CallMetadata
. The easiest would be to implement this trait on Tuple of type which implements PalletInfoAccess, and then we can simply use the type alias AllPalletsWithSystem
. (or we can implement manually like CallMetadata
))
Yeah this doesn't actually do what I need (yet). But the PR description does describe what I need. @KiChjang do you intend to get this to the point where it'a in line with the PR description? |
@gavofyork do you have an actual instance of a call on which you want to get this metadata? |
@bkchr No - I only have a string which is meant to be the pallet's name. With this string I need to be able to get all pallet instances of that name together with their index and crate version. Since a pallet instance has two associated names - the module/crate name (e.g. |
Yeah, so @KiChjang the only thing that is missing is the module/crate_name which could be added to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the name the idea was more to take the input to construct_runtime
. Because stuff like crowdloan
, claims
etc all come from the same crate.
The issue here is that whatever was provided to pub enum Runtime where
Block = Block,
NodeBlock = Block,
UncheckedExtrinsic = UncheckedExtrinsic
{
System: system::{Pallet, Call, Event<T>, Origin<T>} = 30,
NestedModule3: nested::module3::{Pallet, Call, Config, Storage, Event, Origin},
^^^^^^^^^^^^^^^
} ... is not necessarily a single identifier, and so grabbing the crate name directly from the pallet is the easiest way to disambiguate. I suppose I could also make a naming scheme where the double colons get converted to underscores, so we'd have |
If it's just going into a string, then you can keep the |
It's not such a huge issue that naming is a bit idiosyncratic when dealing with pallets that are one of several in a crate local to the project. The primary use-case for this is for standardised pallets anyway, so if they have necessarily non-standard paths then they're unlikely to be the target of this code. |
I think the pallet path given to construct_runtime shouldn't really identify anything other than the path to access this pallet. People can do renaming, or reexport etc... Instead if xcm want to check if a pallet is made from some specific pallet "blueprint" like Instead pallet could be able to declare their "blueprint" name maybe ? we can do that with an optional new attribute |
This needs another approval before it can be merged, anything else I need to add here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me, identifying a pallet kind using its rust module may be fine. Note that this doesn't include the specification of the instance, so 2 pallets can have the same module name.
Co-authored-by: Guillaume Thiolliere <[email protected]>
Needs resolving and CI failing @KiChjang |
A test would have been nice. |
Per request from @gavofyork via Element, this PR adds module names and crate versions to the
PalletInfoAccess
struct as public fields.