From 7806f6f7d75bb6935af6f8f964031b3b28ed848b Mon Sep 17 00:00:00 2001 From: h4x3rotab Date: Thu, 14 Jan 2021 17:56:11 +0800 Subject: [PATCH] Refactor event type decoding hand declartion Fixes #196, #181, #28 ## Dyanmic sized types Before this change, the event decoder assume all the event types have fixed sizes. Some counterexamples are: Hashes, AuthorityList. In this change, instead of decoding by skipping the fixed-length bytes, we introduce `type_segmenter` registry which decodes the raw event bytes with the actual scale codec. So variable length types can be handled correctly. ## New attribute for pallet type definition In the past, trait associated type is the only way to add types to the EventsDecoder implementation of a pallet. But in reality it's common that the events in a pallet references some types not defined in the trait associated types. Some examples are: `IdentificationTuple` and `SessionIndex` in Session pallet. In this change, we introduce more attributes to add the types: ```rust #[module] trait Pallet: System { #![event_type(SomeType)] #![event_alias(TypeNameAlias = SomeType)] #![event_alias(SomeOtherAlias = TypeWithAssociatedTypes)] } ``` ## Tested Compile with `nightly-2020-10-01`; smoke test to sync a full Phala bockchain. --- Cargo.toml | 1 + proc-macro/src/module.rs | 56 ++++++++++++++++++++++++++++++++++++++-- proc-macro/src/utils.rs | 15 +++++++++++ src/events.rs | 38 ++++++++++++++++++++++----- src/frame/session.rs | 23 ++++++++++++++--- src/frame/staking.rs | 9 ++++++- src/frame/system.rs | 4 ++- 7 files changed, 132 insertions(+), 14 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1a2297e1426..72a024af711 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,6 +38,7 @@ frame-metadata = { version = "12.0.1", package = "frame-metadata" } frame-support = { version = "2.0.1", package = "frame-support" } sp-runtime = { version = "2.0.1", package = "sp-runtime" } sp-version = { version = "2.0.1", package = "sp-version" } +sp-finality-grandpa = { version = "2.0.1", package = "sp-finality-grandpa", default-features = false } pallet-indices = { version = "2.0.1", package = "pallet-indices" } hex = "0.4.2" sp-std = "2.0.1" diff --git a/proc-macro/src/module.rs b/proc-macro/src/module.rs index b1d989235be..0e6cf41a45d 100644 --- a/proc-macro/src/module.rs +++ b/proc-macro/src/module.rs @@ -69,16 +69,61 @@ fn events_decoder_trait_name(module: &syn::Ident) -> syn::Ident { fn with_module_ident(module: &syn::Ident) -> syn::Ident { format_ident!("with_{}", module.to_string().to_snake_case()) } + +type EventAttr = utils::UniAttr; +type EventAliasAttr = utils::UniAttr>; + +/// Parses the event type definition macros within #[module] +/// +/// It supports two ways to define the associated event type: +/// +/// ``` +/// #[module] +/// trait Pallet: System { +/// #![event_type(SomeType)] +/// #![event_alias(TypeNameAlias = SomeType)] +/// #![event_alias(SomeOtherAlias = TypeWithAssociatedTypes)] +/// } +/// ``` +fn parse_event_type_attr(attr: &syn::Attribute) -> Option<(String, syn::Type)> { + let ident = utils::path_to_ident(&attr.path); + if ident == "event_type" { + let attrs: EventAttr = syn::parse2(attr.tokens.clone()) + .map_err(|err| abort!("{}", err)) + .unwrap(); + let ty = attrs.attr; + let ident_str = quote!(#ty).to_string(); + Some((ident_str, ty)) + } else if ident == "event_alias" { + let attrs: EventAliasAttr = syn::parse2(attr.tokens.clone()) + .map_err(|err| abort!("{}", err)) + .unwrap(); + let ty = attrs.attr.value; + let ident_str = attrs.attr.key.to_string(); + Some((ident_str, ty)) + } else { + None + } +} + /// Attribute macro that registers the type sizes used by the module; also sets the `MODULE` constant. pub fn module(_args: TokenStream, tokens: TokenStream) -> TokenStream { let input: Result = syn::parse2(tokens.clone()); - let input = if let Ok(input) = input { + let mut input = if let Ok(input) = input { input } else { // handle #[module(ignore)] by just returning the tokens return tokens }; + // Parse the inner attributes `event_type` and `event_alias` and remove them from the macro + // outputs. + let (other_attrs, event_types): (Vec<_>, Vec<_>) = input.attrs + .iter() + .cloned() + .partition(|attr| parse_event_type_attr(attr).is_none()); + input.attrs = other_attrs; + let subxt = utils::use_crate("substrate-subxt"); let module = &input.ident; let module_name = module.to_string(); @@ -96,7 +141,7 @@ pub fn module(_args: TokenStream, tokens: TokenStream) -> TokenStream { None } }); - let types = input.items.iter().filter_map(|item| { + let associated_types = input.items.iter().filter_map(|item| { if let syn::TraitItem::Type(ty) = item { if ignore(&ty.attrs) { return None @@ -110,6 +155,12 @@ pub fn module(_args: TokenStream, tokens: TokenStream) -> TokenStream { None } }); + let types = event_types.iter().map(|attr| { + let (ident_str, ty) = parse_event_type_attr(&attr).unwrap(); + quote! { + self.register_type_size::<#ty>(#ident_str); + } + }); quote! { #input @@ -127,6 +178,7 @@ pub fn module(_args: TokenStream, tokens: TokenStream) -> TokenStream { { fn #with_module(&mut self) { #(#bounds)* + #(#associated_types)* #(#types)* } } diff --git a/proc-macro/src/utils.rs b/proc-macro/src/utils.rs index a595d42fe6f..ae37d1ce1f3 100644 --- a/proc-macro/src/utils.rs +++ b/proc-macro/src/utils.rs @@ -214,6 +214,21 @@ impl Parse for Attr { } } +#[derive(Debug)] +pub struct UniAttr { + pub paren: syn::token::Paren, + pub attr: A, +} + +impl Parse for UniAttr { + fn parse(input: ParseStream) -> syn::Result { + let content; + let paren = syn::parenthesized!(content in input); + let attr = content.parse()?; + Ok(Self { paren, attr }) + } +} + #[cfg(test)] pub(crate) fn assert_proc_macro( result: proc_macro2::TokenStream, diff --git a/src/events.rs b/src/events.rs index 621fb7ec57c..d170b3c4bf3 100644 --- a/src/events.rs +++ b/src/events.rs @@ -28,6 +28,7 @@ use sp_runtime::{ DispatchResult, }; use std::{ + fmt, collections::{ HashMap, HashSet, @@ -72,19 +73,32 @@ impl std::fmt::Debug for RawEvent { } /// Events decoder. -#[derive(Debug)] pub struct EventsDecoder { metadata: Metadata, type_sizes: HashMap, + type_segmenters: HashMap< + String, + Box) -> Result<(), Error> + Send> + >, marker: PhantomData T>, } +impl fmt::Debug for EventsDecoder { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("EventsDecoder") + .field("metadata", &self.metadata) + .field("type_sizes", &self.type_sizes) + .finish() + } +} + impl EventsDecoder { /// Creates a new `EventsDecoder`. pub fn new(metadata: Metadata) -> Self { let mut decoder = Self { metadata, type_sizes: HashMap::new(), + type_segmenters: HashMap::new(), marker: PhantomData, }; // register default event arg type sizes for dynamic decoding of events @@ -109,6 +123,8 @@ impl EventsDecoder { decoder.register_type_size::("BlockNumber"); decoder.register_type_size::("Hash"); decoder.register_type_size::("VoteThreshold"); + // Additional types + decoder.register_type_size::<(T::BlockNumber, u32)>("TaskAddress"); decoder } @@ -119,6 +135,13 @@ impl EventsDecoder { { let size = U::default().encode().len(); self.type_sizes.insert(name.to_string(), size); + // A segmenter decodes a type from an input stream (&mut &[u8]) and returns the serialized + // type to the output stream (&mut Vec). + self.type_segmenters.insert(name.to_string(), + Box::new(|input: &mut &[u8], output: &mut Vec| -> Result<(), Error> { + U::decode(input).map_err(Error::from)?.encode_to(output); + Ok(()) + })); size } @@ -150,10 +173,10 @@ impl EventsDecoder { } } - fn decode_raw_bytes( + fn decode_raw_bytes( &self, args: &[EventArg], - input: &mut I, + input: &mut &[u8], output: &mut W, errors: &mut Vec, ) -> Result<(), Error> { @@ -188,9 +211,9 @@ impl EventsDecoder { "DispatchResult" => DispatchResult::decode(input)?, "DispatchError" => Err(DispatchError::decode(input)?), _ => { - if let Some(size) = self.type_sizes.get(name) { - let mut buf = vec![0; *size]; - input.read(&mut buf)?; + if let Some(seg) = self.type_segmenters.get(name) { + let mut buf = Vec::::new(); + seg(input, &mut buf)?; output.write(&buf); Ok(()) } else { @@ -268,9 +291,12 @@ impl EventsDecoder { } } +/// Raw event or error event #[derive(Debug)] pub enum Raw { + /// Event Event(RawEvent), + /// Error Error(RuntimeError), } diff --git a/src/frame/session.rs b/src/frame/session.rs index f5f527b81b3..f4091410cba 100644 --- a/src/frame/session.rs +++ b/src/frame/session.rs @@ -15,9 +15,15 @@ // along with substrate-subxt. If not, see . //! Session support -use crate::frame::system::{ - System, - SystemEventsDecoder as _, +use crate::frame::{ + system::{ + System, + SystemEventsDecoder as _, + }, + balances::{ + Balances, + BalancesEventsDecoder as _, + } }; use codec::Encode; use frame_support::Parameter; @@ -45,9 +51,18 @@ macro_rules! default_impl { }; } +type IdentificationTuple = ( + ::ValidatorId, + pallet_staking::Exposure<::AccountId, ::Balance> +); + /// The trait needed for this module. #[module] -pub trait Session: System { +pub trait Session: System + Balances { + #![event_alias(IdentificationTuple = IdentificationTuple)] + #![event_alias(OpaqueTimeSlot = Vec)] + #![event_alias(SessionIndex = u32)] + /// The validator account identifier type for the runtime. type ValidatorId: Parameter + Debug + Ord + Default + Send + Sync + 'static; diff --git a/src/frame/staking.rs b/src/frame/staking.rs index 06aa598143a..d1c202f9639 100644 --- a/src/frame/staking.rs +++ b/src/frame/staking.rs @@ -31,6 +31,8 @@ use std::{ marker::PhantomData, }; +use sp_finality_grandpa::AuthorityList; + pub use pallet_staking::{ ActiveEraInfo, EraIndex, @@ -42,6 +44,7 @@ pub use pallet_staking::{ ValidatorPrefs, }; + /// Rewards for the last `HISTORY_DEPTH` eras. /// If reward hasn't been set or has been removed then 0 reward is returned. #[derive(Clone, Encode, Decode, Debug, Store)] @@ -64,7 +67,11 @@ pub struct SetPayeeCall { /// The subset of the `frame::Trait` that a client must implement. #[module] -pub trait Staking: Balances {} +pub trait Staking: Balances { + #![event_alias(ElectionCompute = u8)] + #![event_type(EraIndex)] + #![event_type(AuthorityList)] +} /// Number of eras to keep in history. /// diff --git a/src/frame/system.rs b/src/frame/system.rs index b0cb80fcbb1..3b90125616a 100644 --- a/src/frame/system.rs +++ b/src/frame/system.rs @@ -164,8 +164,10 @@ pub struct SetCodeWithoutChecksCall<'a, T: System> { pub enum Phase { /// Applying an extrinsic. ApplyExtrinsic(u32), - /// The end. + /// Finalizing the block. Finalization, + /// Initializing the block. + Initialization, } /// An extrinsic completed successfully.