-
Notifications
You must be signed in to change notification settings - Fork 254
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
Add hooks to register event types for decoding #227
Conversation
event_type_registry.with_session(); | ||
event_type_registry.with_contracts(); | ||
event_type_registry.with_sudo(); | ||
register_default_type_sizes(event_type_registry); |
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.
Note that this implementation must be amended manually for new and removed pallets. It would be possible to rework the macros to do this but since this is a short term solution I think this is good enough.
# Conflicts: # .rustfmt.toml # Cargo.toml
Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
@@ -72,116 +76,118 @@ impl std::fmt::Debug for RawEvent { | |||
} | |||
} | |||
|
|||
trait TypeSegmenter: Send { | |||
pub trait TypeSegmenter: DynClone + Send + Sync { |
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.
I don't really understand why the Sync
bound is required, can you explain?
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.
data: event_data, | ||
}; | ||
/// Register a type. | ||
pub fn register_type_size<U>(&mut self, name: &str) |
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.
document that it panics if the type is already registered
@@ -162,6 +179,9 @@ pub trait Runtime: System + Sized + Send + Sync + 'static { | |||
type Signature: Verify + Encode + Send + Sync + 'static; | |||
/// Transaction extras. | |||
type Extra: SignedExtra<Self> + Send + Sync + 'static; | |||
|
|||
/// Register type sizes for this runtime | |||
fn register_type_sizes(event_type_registry: &mut EventTypeRegistry<Self>); |
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.
ok now I see why Sync
is required.
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.
LGTM
/// | ||
/// *WARNING* can lead to runtime errors if receiving events with unknown types. | ||
pub fn skip_type_sizes_check(mut self) -> Self { | ||
self.skip_type_sizes_check = false; |
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.
is this should be self.skip_type_sizes_check = true;
?
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.
Indeed looks like it should be...
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.
Fixes #201.
Provides hooks to register missing runtime type sizes either statically via the
Runtime
trait impl:Or dynamically via the
ClientBuilder
:In addition, the
build()
method on theClientBuilder
will now check for missing type sizes by default, and return anErr
if there are any missing. This can be disabled byskip_type_sizes_check
:Note
I consider this an intermediate solution, since all my efforts are currently focused towards the integrating
scale-info
with substrate which will provide richer type information for the runtimes. This would eventually remove the requirement to manually provide type information in the client.todo
check_missing_type_sizes
by default inClientBuilder::build
, which could be disabled by e.g.skip_type_size_check