Allow for static event decoding

[geigerzaehler]

At the moment events from ExtrinsicSuccess are encoded in the RuntimeEvents struct. The RuntimeEvents struct has no relation to the runtime types. It would be preferable to instead decode the events as the real event type obtained from the runtime. More concretely

pub struct ExtrinsicSuccess<T: System> {
  // ...
  events: srml_system::EventRecord<T::Event, T::Hash>
}

This would require us to add the associated Event type to the System trait.

With this implementation we could do away with the dynamic event parsing which suffers from a couple of issues.

• It needs to fetch the metadata
• It requires substantial manual decoding logic for events
• It requires registering type sizes for custom structs.

I’d be willing to give this a stab.

[ascjones]

Apologies for the delayed response, I've been away.

Indeed having an Event type on the System trait was originally the way it was. The main motivation for the introduction of the dynamic event parsing was to remove the concrete dependency on node-runtime (or equivalent) where the top level runtime enum is defined. This is required for the library to work against any substrate based chain (with matching primitive types) without custom compilation.

I agree with the some of the issues you raise about the dynamic event parsing - we need to look at improving the api/implementation. However I'd like to keep it for now since it is required by the ink! cli: I want to be able to deploy a contract to any contract enabled chain.

However perhaps we could introduce the option back (perhaps even make it the default) of declaring a static event type. And make the dynamic parsing optional.

[geigerzaehler]

Thanks for the reply @ascjones and explaining the conflicting use case. To me it would make sense to make sense to have the static event parsing by default and build on top of that a layer that allows for dynamic event parsing.

The problem I had with the current approach is that it immediately breaks down once the assumptions that the dynamic event parser makes do not hold. And then it is impossible to work around the issue. Flipping this around would mean that every use case is supported by default although it depends on the runtime defintion.

[ascjones]

I agree, let's do it that way.

Incidentally I've seen paritytech/substrate#3964 which would make things better for including a runtime dependency for the static Event definition.

The problem I had with the current approach is that it immediately breaks down once the assumptions that the dynamic event parser makes do not hold. And then it is impossible to work around the issue

Yes I will attempt to address this - the first attempt was quick and dirty for my use case: but glad that I'm no longer the only user of this library!