Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove superfluous Event/Extrinsic decoding APIs? #952

Closed
jsdw opened this issue May 11, 2023 · 0 comments · Fixed by #953
Closed

Remove superfluous Event/Extrinsic decoding APIs? #952

jsdw opened this issue May 11, 2023 · 0 comments · Fixed by #953

Comments

@jsdw
Copy link
Collaborator

jsdw commented May 11, 2023

Events and extrinsics both now provide:

// return the values of the fields in the event as a Composite
fn field_values(&self) -> Result<scale_value::Composite<scale_value::scale::TypeId>, Error>
// return the values of the fields in the event as some type E
fn as_event<E: StaticEvent>(&self) -> Result<Option<E>, Error>fn as_pallet_event<E: DecodeWithMetadata>(&self) -> Result<E, Error>
// try to decode the event into some specific pallet::Event
fn as_pallet_event<E: DecodeWithMetadata>(&self) -> Result<E, Error>
// try to decode the event into the root runtime::Event
fn as_root_event<E: RootEvent>(&self) -> Result<E, Error>

Ie, we have 4 different ways to decode events (and the same interface exists foer calls for consistency).

  • as_root_n is the most generally useful way to statically decode things nowadays I think, and renders as_pallet_n redundant. And moreso, as_pallet_n fns may decode things into the wrong types if the types happen to align closely enough, so I think it's a bit of a footgun. (ie if you try to decode some extrinsic into pallet_a::Call, it will succeed even if it was actually from pallet_b::Call as long as the types line up for the variant we're decoding)
  • as_n (ie as_event and as_extrinsic) are used in all of the find() functions, which are quite nice ergonomically, so I'd keep them for now just for that reason.
  • field_values (for events and extrinsics) is the only way to dynamically decode fields, which Probably makes marginally more sense than decoding the entire outer variant (you can check the variant/pallet names separately already anyway).

So, I'd propose that we remove as_pallet_extrinsic and as_pallet_event for now. I'd like to remove more, but the find() APIs are just a little too appealing to want to remove, though one can manually iterate extrinsics/events, and search/decode based on pallet and variant name)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant