From 261b95c2d0d335e6595dfc04fc19db7ffddc2231 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 23 Jul 2020 17:02:09 +0200 Subject: [PATCH 1/5] Fancy compact encode/decode impl for compact solution --- frame/staking/src/lib.rs | 36 +- .../npos-elections/compact/src/assignment.rs | 80 ++--- .../npos-elections/compact/src/codec.rs | 205 ++++++++++++ .../compact/src/codec_example.rs | 97 ++++++ primitives/npos-elections/compact/src/lib.rs | 189 ++++++----- .../npos-elections/compact/src/staked.rs | 212 ------------ primitives/npos-elections/src/tests.rs | 312 ++++-------------- 7 files changed, 526 insertions(+), 605 deletions(-) create mode 100644 primitives/npos-elections/compact/src/codec.rs create mode 100644 primitives/npos-elections/compact/src/codec_example.rs delete mode 100644 primitives/npos-elections/compact/src/staked.rs diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index be07c7e18a4f4..92128f08b7541 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -369,19 +369,9 @@ pub type EraIndex = u32; pub type RewardPoint = u32; // Note: Maximum nomination limit is set here -- 16. -generate_compact_solution_type!(pub GenericCompactAssignments, 16); - -/// Information regarding the active era (era in used in session). -#[derive(Encode, Decode, RuntimeDebug)] -pub struct ActiveEraInfo { - /// Index of era. - pub index: EraIndex, - /// Moment of start expressed as millisecond from `$UNIX_EPOCH`. - /// - /// Start can be none if start hasn't been set for the era yet, - /// Start is set on the first on_finalize of the era to guarantee usage of `Time`. - start: Option, -} +generate_compact_solution_type!( + pub struct CompactAssignments::(16) +); /// Accuracy used for on-chain election. pub type ChainAccuracy = Perbill; @@ -393,15 +383,23 @@ pub type OffchainAccuracy = PerU16; pub type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; -/// The compact type for election solutions. -pub type CompactAssignments = - GenericCompactAssignments; - type PositiveImbalanceOf = <::Currency as Currency<::AccountId>>::PositiveImbalance; type NegativeImbalanceOf = <::Currency as Currency<::AccountId>>::NegativeImbalance; +/// Information regarding the active era (era in used in session). +#[derive(Encode, Decode, RuntimeDebug)] +pub struct ActiveEraInfo { + /// Index of era. + pub index: EraIndex, + /// Moment of start expressed as millisecond from `$UNIX_EPOCH`. + /// + /// Start can be none if start hasn't been set for the era yet, + /// Start is set on the first on_finalize of the era to guarantee usage of `Time`. + start: Option, +} + /// Reward points of an era. Used to split era total payout between validators. /// /// This points will be used to reward validators and their respective nominators. @@ -1238,12 +1236,12 @@ decl_storage! { decl_event!( pub enum Event where Balance = BalanceOf, ::AccountId { /// The era payout has been set; the first balance is the validator-payout; the second is - /// the remainder from the maximum amount of reward. + /// the remainder from the maximum amount of reward. /// [era_index, validator_payout, remainder] EraPayout(EraIndex, Balance, Balance), /// The staker has been rewarded by this amount. [stash, amount] Reward(AccountId, Balance), - /// One validator (and its nominators) has been slashed by the given amount. + /// One validator (and its nominators) has been slashed by the given amount. /// [validator, amount] Slash(AccountId, Balance), /// An old slashing report from a prior era was discarded because it could diff --git a/primitives/npos-elections/compact/src/assignment.rs b/primitives/npos-elections/compact/src/assignment.rs index 96c68ece92a19..95d7ce2fb926e 100644 --- a/primitives/npos-elections/compact/src/assignment.rs +++ b/primitives/npos-elections/compact/src/assignment.rs @@ -15,11 +15,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Code generation for the ratio assignment type. +//! Code generation for the ratio assignment type' compact representation. use crate::field_name_for; use proc_macro2::TokenStream as TokenStream2; -use syn::GenericArgument; use quote::quote; fn from_impl(count: usize) -> TokenStream2 { @@ -27,8 +26,8 @@ fn from_impl(count: usize) -> TokenStream2 { let name = field_name_for(1); quote!(1 => compact.#name.push( ( - index_of_voter(&who).ok_or(_phragmen::Error::CompactInvalidIndex)?, - index_of_target(&distribution[0].0).ok_or(_phragmen::Error::CompactInvalidIndex)?, + index_of_voter(&who).or_invalid_index()?, + index_of_target(&distribution[0].0).or_invalid_index()?, ) ),) }; @@ -37,29 +36,29 @@ fn from_impl(count: usize) -> TokenStream2 { let name = field_name_for(2); quote!(2 => compact.#name.push( ( - index_of_voter(&who).ok_or(_phragmen::Error::CompactInvalidIndex)?, + index_of_voter(&who).or_invalid_index()?, ( - index_of_target(&distribution[0].0).ok_or(_phragmen::Error::CompactInvalidIndex)?, + index_of_target(&distribution[0].0).or_invalid_index()?, distribution[0].1, ), - index_of_target(&distribution[1].0).ok_or(_phragmen::Error::CompactInvalidIndex)?, + index_of_target(&distribution[1].0).or_invalid_index()?, ) ),) }; let from_impl_rest = (3..=count).map(|c| { let inner = (0..c-1).map(|i| - quote!((index_of_target(&distribution[#i].0).ok_or(_phragmen::Error::CompactInvalidIndex)?, distribution[#i].1),) + quote!((index_of_target(&distribution[#i].0).or_invalid_index()?, distribution[#i].1),) ).collect::(); let field_name = field_name_for(c); let last_index = c - 1; - let last = quote!(index_of_target(&distribution[#last_index].0).ok_or(_phragmen::Error::CompactInvalidIndex)?); + let last = quote!(index_of_target(&distribution[#last_index].0).or_invalid_index()?); quote!( #c => compact.#field_name.push( ( - index_of_voter(&who).ok_or(_phragmen::Error::CompactInvalidIndex)?, + index_of_voter(&who).or_invalid_index()?, [#inner], #last, ) @@ -74,15 +73,15 @@ fn from_impl(count: usize) -> TokenStream2 { ) } -fn into_impl(count: usize) -> TokenStream2 { +fn into_impl(count: usize, per_thing: syn::Type) -> TokenStream2 { let into_impl_single = { let name = field_name_for(1); quote!( for (voter_index, target_index) in self.#name { assignments.push(_phragmen::Assignment { - who: voter_at(voter_index).ok_or(_phragmen::Error::CompactInvalidIndex)?, + who: voter_at(voter_index).or_invalid_index()?, distribution: vec![ - (target_at(target_index).ok_or(_phragmen::Error::CompactInvalidIndex)?, Accuracy::one()) + (target_at(target_index).or_invalid_index()?, #per_thing::one()) ], }) } @@ -93,21 +92,21 @@ fn into_impl(count: usize) -> TokenStream2 { let name = field_name_for(2); quote!( for (voter_index, (t1_idx, p1), t2_idx) in self.#name { - if p1 >= Accuracy::one() { + if p1 >= #per_thing::one() { return Err(_phragmen::Error::CompactStakeOverflow); } // defensive only. Since Percent doesn't have `Sub`. let p2 = _phragmen::sp_arithmetic::traits::Saturating::saturating_sub( - Accuracy::one(), + #per_thing::one(), p1, ); assignments.push( _phragmen::Assignment { - who: voter_at(voter_index).ok_or(_phragmen::Error::CompactInvalidIndex)?, + who: voter_at(voter_index).or_invalid_index()?, distribution: vec![ - (target_at(t1_idx).ok_or(_phragmen::Error::CompactInvalidIndex)?, p1), - (target_at(t2_idx).ok_or(_phragmen::Error::CompactInvalidIndex)?, p2), + (target_at(t1_idx).or_invalid_index()?, p1), + (target_at(t2_idx).or_invalid_index()?, p2), ] }); } @@ -118,30 +117,30 @@ fn into_impl(count: usize) -> TokenStream2 { let name = field_name_for(c); quote!( for (voter_index, inners, t_last_idx) in self.#name { - let mut sum = Accuracy::zero(); + let mut sum = #per_thing::zero(); let mut inners_parsed = inners .iter() .map(|(ref t_idx, p)| { sum = _phragmen::sp_arithmetic::traits::Saturating::saturating_add(sum, *p); - let target = target_at(*t_idx).ok_or(_phragmen::Error::CompactInvalidIndex)?; + let target = target_at(*t_idx).or_invalid_index()?; Ok((target, *p)) }) - .collect::, _phragmen::Error>>()?; + .collect::, _phragmen::Error>>()?; - if sum >= Accuracy::one() { + if sum >= #per_thing::one() { return Err(_phragmen::Error::CompactStakeOverflow); } // defensive only. Since Percent doesn't have `Sub`. let p_last = _phragmen::sp_arithmetic::traits::Saturating::saturating_sub( - Accuracy::one(), + #per_thing::one(), sum, ); - inners_parsed.push((target_at(t_last_idx).ok_or(_phragmen::Error::CompactInvalidIndex)?, p_last)); + inners_parsed.push((target_at(t_last_idx).or_invalid_index()?, p_last)); assignments.push(_phragmen::Assignment { - who: voter_at(voter_index).ok_or(_phragmen::Error::CompactInvalidIndex)?, + who: voter_at(voter_index).or_invalid_index()?, distribution: inners_parsed, }); } @@ -157,39 +156,28 @@ fn into_impl(count: usize) -> TokenStream2 { pub(crate) fn assignment( ident: syn::Ident, - voter_type: GenericArgument, - target_type: GenericArgument, + voter_type: syn::Type, + target_type: syn::Type, + weight_type: syn::Type, count: usize, ) -> TokenStream2 { let from_impl = from_impl(count); - let into_impl = into_impl(count); + let into_impl = into_impl(count, weight_type.clone()); quote!( - impl< - #voter_type: _phragmen::codec::Codec + Default + Copy, - #target_type: _phragmen::codec::Codec + Default + Copy, - Accuracy: - _phragmen::codec::Codec + Default + Clone + _phragmen::sp_arithmetic::PerThing + - PartialOrd, - > - #ident<#voter_type, #target_type, Accuracy> - { + impl #ident { pub fn from_assignment( - assignments: Vec<_phragmen::Assignment>, + assignments: Vec<_phragmen::Assignment>, index_of_voter: FV, index_of_target: FT, ) -> Result where + A: _phragmen::IdentifierT, for<'r> FV: Fn(&'r A) -> Option<#voter_type>, for<'r> FT: Fn(&'r A) -> Option<#target_type>, - A: _phragmen::IdentifierT, { - let mut compact: #ident< - #voter_type, - #target_type, - Accuracy, - > = Default::default(); + let mut compact: #ident = Default::default(); for _phragmen::Assignment { who, distribution } in assignments { match distribution.len() { @@ -207,8 +195,8 @@ pub(crate) fn assignment( self, voter_at: impl Fn(#voter_type) -> Option, target_at: impl Fn(#target_type) -> Option, - ) -> Result>, _phragmen::Error> { - let mut assignments: Vec<_phragmen::Assignment> = Default::default(); + ) -> Result>, _phragmen::Error> { + let mut assignments: Vec<_phragmen::Assignment> = Default::default(); #into_impl Ok(assignments) } diff --git a/primitives/npos-elections/compact/src/codec.rs b/primitives/npos-elections/compact/src/codec.rs new file mode 100644 index 0000000000000..0eb0cf0a1f07c --- /dev/null +++ b/primitives/npos-elections/compact/src/codec.rs @@ -0,0 +1,205 @@ +// This file is part of Substrate. + +// Copyright (C) 2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Code generation for the ratio assignment type' encode/decode impl. + +use crate::field_name_for; +use proc_macro2::TokenStream as TokenStream2; +use quote::quote; + +pub(crate) fn codec_impl( + ident: syn::Ident, + voter_type: syn::Type, + target_type: syn::Type, + weight_type: syn::Type, + count: usize, +) -> TokenStream2 { + let encode = encode_impl(ident.clone(), count); + let decode = decode_impl(ident, voter_type, target_type, weight_type, count); + + quote! { + #encode + #decode + } +} + +fn decode_impl( + ident: syn::Ident, + voter_type: syn::Type, + target_type: syn::Type, + weight_type: syn::Type, + count: usize, +) -> TokenStream2 { + let decode_impl_single = { + let name = field_name_for(1); + quote! { + let #name = + < + Vec<(_phragmen::codec::Compact<#voter_type>, _phragmen::codec::Compact<#target_type>)> + as + _phragmen::codec::Decode + >::decode(value)?; + let #name = #name + .into_iter() + .map(|(v, t)| (v.0, t.0)) + // .map(|(v, t) (v.0, t.0)|) // Will crash the compiler in a weird way. Report + .collect::>(); + } + }; + + let decode_impl_double = { + let name = field_name_for(2); + quote! { + let #name = + < + Vec<( + _phragmen::codec::Compact<#voter_type>, + (_phragmen::codec::Compact<#target_type>, _phragmen::codec::Compact<#weight_type>), + _phragmen::codec::Compact<#target_type>, + )> + as + _phragmen::codec::Decode + >::decode(value)?; + let #name = #name + .into_iter() + .map(|(v, (t1, w), t2)| (v.0, (t1.0, w.0), t2.0)) + .collect::>(); + } + }; + + let decode_impl_rest = (3..=count).map(|c| { + let name = field_name_for(c); + + let inner_impl = (0..c-1).map(|i| + quote! { ( (inner[#i].0).0, (inner[#i].1).0 ), } + ).collect::(); + + quote! { + let #name = + < + Vec<( + _phragmen::codec::Compact<#voter_type>, + [(_phragmen::codec::Compact<#target_type>, _phragmen::codec::Compact<#weight_type>); #c], + _phragmen::codec::Compact<#target_type>, + )> + as _phragmen::codec::Decode + >::decode(value)?; + let #name = #name + .into_iter() + .map(|(v, inner, t_last)| ( + v.0, + [ #inner_impl ], + t_last.0, + )) + .collect::>(); + } + }).collect::(); + + + let all_field_names = (1..=count).map(|c| { + let name = field_name_for(c); + quote! { #name, } + }).collect::(); + + quote!( + impl _phragmen::codec::Decode for #ident { + fn decode(value: &mut I) -> Result { + #decode_impl_single + #decode_impl_double + #decode_impl_rest + + // The above code generates variables with the decoded value with the same name as + // filed names of the struct, i.e. `let votes4 = decode_value_of_votes4`. All we + // have to do is collect them into the main struct now. + Ok(#ident { #all_field_names }) + } + } + ) +} + +// General attitude is that we will convert inner values to `Compact` and then use the normal +// `Encode` implementation. +fn encode_impl(ident: syn::Ident, count: usize) -> TokenStream2 { + let encode_impl_single = { + let name = field_name_for(1); + quote! { + let #name = self.#name + .iter() + .map(|(v, t)| ( + _phragmen::codec::Compact(v.clone()), + _phragmen::codec::Compact(t.clone()), + )) + .collect::>(); + // TODO: will this work without Encode in scope? no probably. + #name.encode_to(&mut r); + } + }; + + let encode_impl_double = { + let name = field_name_for(2); + quote! { + let #name = self.#name + .iter() + .map(|(v, (t1, w), t2)| ( + _phragmen::codec::Compact(v.clone()), + ( + _phragmen::codec::Compact(t1.clone()), + _phragmen::codec::Compact(w.clone()) + ), + _phragmen::codec::Compact(t2.clone()), + )) + .collect::>(); + #name.encode_to(&mut r); + } + }; + + let encode_impl_rest = (3..=count).map(|c| { + let name = field_name_for(c); + + // we use the knowledge of the length to avoid copy_from_slice. + let inners_compact_array = (0..c-1).map(|i| + quote!{( + _phragmen::codec::Compact(inner[#i].0.clone()), + _phragmen::codec::Compact(inner[#i].1.clone()), + ),} + ).collect::(); + + quote! { + let #name = self.#name + .iter() + .map(|(v, inner, t_last)| ( + _phragmen::codec::Compact(v.clone()), + [ #inners_compact_array ], + _phragmen::codec::Compact(t_last.clone()), + )) + .collect::>(); + #name.encode_to(&mut r); + } + }).collect::(); + + quote!( + impl _phragmen::codec::Encode for #ident { + fn encode(&self) -> Vec { + let mut r = vec![]; + #encode_impl_single + #encode_impl_double + #encode_impl_rest + r + } + } + ) +} diff --git a/primitives/npos-elections/compact/src/codec_example.rs b/primitives/npos-elections/compact/src/codec_example.rs new file mode 100644 index 0000000000000..b040d6386a461 --- /dev/null +++ b/primitives/npos-elections/compact/src/codec_example.rs @@ -0,0 +1,97 @@ +//! A simpler example of hop compact encoding works. Not to be merged, just for review assistance. +//! Run with `cargo play codec_example.rs` + +//# parity-scale-codec = { version = "*", features = ["derive"] } + +use parity_scale_codec::{Encode, Decode, Compact, Input, Error}; + +#[derive(Default, Eq, PartialEq, Debug)] +struct BareSolution { + votes1: Vec<(u32, u16)>, + votes2: Vec<(u32, (u16, u64), u16)>, + votes3: Vec<(u32, [(u16, u64); 2usize], u16)>, +} + +impl Encode for BareSolution { + fn encode(&self) -> Vec { + let mut r = vec![]; + + let votes1 = self.votes1 + .iter() + .map(|(v, t)| (Compact(*v), Compact(*t))) + .collect::>(); + votes1.encode_to(&mut r); + + let votes2 = self.votes2 + .iter() + .map(|(v, (t1, w), t2)| ( + Compact(*v), + ( + Compact(*t1), + Compact(*w), + ), + Compact(*t2), + )) + .collect::>(); + votes2.encode_to(&mut r); + + let votes3 = self.votes3 + .iter() + .map(|(v, inners, t_last)| { + let inners_compact_array = [ + (Compact(inners[0].0), Compact(inners[0].1)), + (Compact(inners[1].0), Compact(inners[1].1)), + ]; + (Compact(*v), inners_compact_array, Compact(*t_last)) + }).collect::>(); + + votes3.encode_to(&mut r); + r + } +} + +impl Decode for BareSolution { + fn decode(value: &mut I) -> Result { + let votes1 = , Compact)> as Decode>::decode(value)?; + let votes1 = votes1.into_iter().map(|(v, t)| (v.0, t.0)).collect::>(); + + let votes2 = , (Compact, Compact), Compact)> as Decode>::decode(value)?; + let votes2 = votes2.into_iter().map(|(v, (t1, w), t2)| (v.0, (t1.0, w.0), t2.into())).collect::>(); + + let votes3 = , [(Compact, Compact); 2], Compact)> as Decode>::decode(value)?; + let votes3 = votes3 + .into_iter() + .map(|(v, inner, t_last)| ( + v.0, + [ + ((inner[0].0).0, (inner[0].1).0), + ((inner[1].0).0, (inner[1].1).0) + ], + t_last.0, + )) + .collect::>(); + + Ok(Self { + votes1, + votes2, + votes3, + ..Default::default() + }) + } +} + +fn main() { + let s = BareSolution { + votes1: vec![(1,2), (3, 1)], + votes2: vec![(1, (99, 33), 44), (2, (99, 33), 44)], + votes3: vec![ + (1, [(99, 33), (55, 33)], 44), + (2, [(99, 33), (55, 33)], 44), + ] + }; + + let e = s.encode(); + let d = ::decode(&mut &*e).unwrap(); + dbg!(&e); + assert_eq!(d, s); +} diff --git a/primitives/npos-elections/compact/src/lib.rs b/primitives/npos-elections/compact/src/lib.rs index 1b88ff6531081..ae27d1a066a99 100644 --- a/primitives/npos-elections/compact/src/lib.rs +++ b/primitives/npos-elections/compact/src/lib.rs @@ -21,73 +21,56 @@ use proc_macro::TokenStream; use proc_macro2::{TokenStream as TokenStream2, Span, Ident}; use proc_macro_crate::crate_name; use quote::quote; -use syn::{GenericArgument, Type, parse::{Parse, ParseStream, Result}}; +use syn::{parse::{Parse, ParseStream, Result}}; mod assignment; -mod staked; +mod codec; // prefix used for struct fields in compact. const PREFIX: &'static str = "votes"; -/// Generates a struct to store the election assignments in a compact way. The struct can only store -/// distributions up to the given input count. The given count must be greater than 2. +pub(crate) fn syn_err(message: &'static str) -> syn::Error { + syn::Error::new(Span::call_site(), message) +} + +/// Generates a struct to store the election result in a compact way. This can encode a structure +/// which is the equivalent of a `sp_npos_elections::Assignment<_>`. /// -/// ```ignore -/// // generate a struct with nominator and edge weight u128, with maximum supported -/// // edge per voter of 16. -/// generate_compact_solution_type(pub TestCompact, 16) -/// ``` +/// The following data types can be configured by the macro. /// -/// This generates: +/// - The identifier of the voter. This can be any type that supports `parity-scale-codec`'s compact +/// encoding. +/// - The identifier of the voter. This can be any type that supports `parity-scale-codec`'s compact +/// encoding. +/// - The accuracy of the ratios. This must be one of the `PerThing` types defined in +/// `sp-arithmetic`. /// -/// ```ignore -/// pub struct TestCompact { -/// votes1: Vec<(V, T)>, -/// votes2: Vec<(V, (T, W), T)>, -/// votes3: Vec<(V, [(T, W); 2usize], T)>, -/// votes4: Vec<(V, [(T, W); 3usize], T)>, -/// votes5: Vec<(V, [(T, W); 4usize], T)>, -/// votes6: Vec<(V, [(T, W); 5usize], T)>, -/// votes7: Vec<(V, [(T, W); 6usize], T)>, -/// votes8: Vec<(V, [(T, W); 7usize], T)>, -/// votes9: Vec<(V, [(T, W); 8usize], T)>, -/// votes10: Vec<(V, [(T, W); 9usize], T)>, -/// votes11: Vec<(V, [(T, W); 10usize], T)>, -/// votes12: Vec<(V, [(T, W); 11usize], T)>, -/// votes13: Vec<(V, [(T, W); 12usize], T)>, -/// votes14: Vec<(V, [(T, W); 13usize], T)>, -/// votes15: Vec<(V, [(T, W); 14usize], T)>, -/// votes16: Vec<(V, [(T, W); 15usize], T)>, -/// } -/// ``` +/// Moreover, the maximum number of edges per voter (distribution per assignment) also need to be +/// specified. Attempting to convert from/to an assignment with more distributions will fail. /// -/// The generic arguments are: -/// - `V`: identifier/index for voter (nominator) types. -/// - `T` identifier/index for candidate (validator) types. -/// - `W` weight type. /// -/// Some conversion implementations are provided by default if -/// - `W` is u128, or -/// - `W` is anything that implements `PerThing` (such as `Perbill`) +/// For example, the following generates a public struct with name `TestCompact` with `u16` voter +/// type, `u8` target type and `Perbill` accuracy with maximum of 8 edges per voter. /// -/// The ideas behind the structure are as follows: +/// ```ignore +/// generate_compact_solution_type!(pub struct TestCompact::(8)) +/// ``` /// -/// - For single distribution, no weight is stored. The weight is known to be 100%. -/// - For all the rest, the weight if the last distribution is omitted. This value can be computed -/// from the rest. +/// The given struct provides function to convert from/to Assignment: /// +/// - [`from_assignment()`]. +/// - [`fn into_assignment()`]. #[proc_macro] pub fn generate_compact_solution_type(item: TokenStream) -> TokenStream { let CompactSolutionDef { vis, ident, count, + voter_type, + target_type, + weight_type, } = syn::parse_macro_input!(item as CompactSolutionDef); - let voter_type = GenericArgument::Type(Type::Verbatim(quote!(V))); - let target_type = GenericArgument::Type(Type::Verbatim(quote!(T))); - let weight_type = GenericArgument::Type(Type::Verbatim(quote!(W))); - let imports = imports().unwrap_or_else(|e| e.to_compile_error()); let compact_def = struct_def( @@ -96,28 +79,27 @@ pub fn generate_compact_solution_type(item: TokenStream) -> TokenStream { count, voter_type.clone(), target_type.clone(), - weight_type, + weight_type.clone(), ).unwrap_or_else(|e| e.to_compile_error()); let assignment_impls = assignment::assignment( ident.clone(), voter_type.clone(), target_type.clone(), + weight_type.clone(), count, ); - let staked_impls = staked::staked( - ident, - voter_type, - target_type, - count, - ); + let codec = codec::codec_impl(ident, voter_type, target_type, weight_type, count); + + let or_invalid_index = or_invalid_index_impl(); quote!( #imports + #or_invalid_index #compact_def #assignment_impls - #staked_impls + #codec ).into() } @@ -125,25 +107,26 @@ fn struct_def( vis: syn::Visibility, ident: syn::Ident, count: usize, - voter_type: GenericArgument, - target_type: GenericArgument, - weight_type: GenericArgument, + voter_type: syn::Type, + target_type: syn::Type, + weight_type: syn::Type, ) -> Result { if count <= 2 { - Err(syn::Error::new( - Span::call_site(), - "cannot build compact solution struct with capacity less than 2." - ))? + Err(syn_err("cannot build compact solution struct with capacity less than 2."))? } let singles = { let name = field_name_for(1); - quote!(#name: Vec<(#voter_type, #target_type)>,) + quote!( + #name: Vec<(#voter_type, #target_type)>, + ) }; let doubles = { let name = field_name_for(2); - quote!(#name: Vec<(#voter_type, (#target_type, #weight_type), #target_type)>,) + quote!( + #name: Vec<(#voter_type, (#target_type, #weight_type), #target_type)>, + ) }; let rest = (3..=count).map(|c| { @@ -177,29 +160,14 @@ fn struct_def( Ok(quote! ( /// A struct to encode a election assignment in a compact way. - #[derive( - Default, - PartialEq, - Eq, - Clone, - Debug, - _phragmen::codec::Encode, - _phragmen::codec::Decode, - )] - #vis struct #ident<#voter_type, #target_type, #weight_type> { - // _marker: sp_std::marker::PhantomData, - #singles - #doubles - #rest - } + #[derive(Default, PartialEq, Eq, Clone, Debug)] + #vis struct #ident { #singles #doubles #rest } - impl<#voter_type, #target_type, #weight_type> _phragmen::VotingLimit - for #ident<#voter_type, #target_type, #weight_type> - { + impl _phragmen::VotingLimit for #ident { const LIMIT: usize = #count; } - impl<#voter_type, #target_type, #weight_type> #ident<#voter_type, #target_type, #weight_type> { + impl #ident { /// Get the length of all the assignments that this type is encoding. This is basically /// the same as the number of assignments, or the number of voters in total. pub fn len(&self) -> usize { @@ -239,20 +207,71 @@ fn imports() -> Result { } } +fn or_invalid_index_impl() -> TokenStream2 { + quote! { + // Simple Extension trait to easily convert `None` from index closures to `Err`. + trait OrInvalidIndex { + fn or_invalid_index(self) -> Result; + } + + impl OrInvalidIndex for Option { + fn or_invalid_index(self) -> Result { + self.ok_or(_phragmen::Error::CompactInvalidIndex) + } + } + } +} + struct CompactSolutionDef { vis: syn::Visibility, ident: syn::Ident, + voter_type: syn::Type, + target_type: syn::Type, + weight_type: syn::Type, count: usize, } +/// pub struct CompactName::() impl Parse for CompactSolutionDef { fn parse(input: ParseStream) -> syn::Result { + // struct let vis: syn::Visibility = input.parse()?; + let _ = ::parse(input)?; let ident: syn::Ident = input.parse()?; - let _ = ::parse(input)?; - let count_literal: syn::LitInt = input.parse()?; - let count = count_literal.base10_parse::()?; - Ok(Self { vis, ident, count } ) + + // :: + let _ = ::parse(input)?; + let generics: syn::AngleBracketedGenericArguments = input.parse()?; + + if generics.args.len() != 3 { + return Err(syn_err("Must provide 3 generic args.")) + } + + let mut types: Vec = generics.args.iter().map(|t| + match t { + syn::GenericArgument::Type(ty) => Ok(ty.clone()), + _ => Err(syn_err("Wrong type of generic provided. Must be a `type`.")), + } + ).collect::>()?; + + let weight_type = types.pop().expect("Vector of length 3 can be popped; qed"); + let target_type = types.pop().expect("Vector of length 2 can be popped; qed"); + let voter_type = types.pop().expect("Vector of length 1 can be popped; qed"); + + // () + let count_expr: syn::ExprParen = input.parse()?; + let expr = count_expr.expr; + let expr_lit = match *expr { + syn::Expr::Lit(count_lit) => count_lit.lit, + _ => return Err(syn_err("Count must be literal.")) + }; + let int_lit = match expr_lit { + syn::Lit::Int(int_lit) => int_lit, + _ => return Err(syn_err("Count must be literal.")) + }; + let count = int_lit.base10_parse::()?; + + Ok(Self { vis, ident, voter_type, target_type, weight_type, count } ) } } diff --git a/primitives/npos-elections/compact/src/staked.rs b/primitives/npos-elections/compact/src/staked.rs deleted file mode 100644 index e2680e18b6326..0000000000000 --- a/primitives/npos-elections/compact/src/staked.rs +++ /dev/null @@ -1,212 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) 2020 Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: Apache-2.0 - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//! Code generation for the staked assignment type. - -use crate::field_name_for; -use proc_macro2::{TokenStream as TokenStream2}; -use syn::{GenericArgument}; -use quote::quote; - -fn from_impl(count: usize) -> TokenStream2 { - let from_impl_single = { - let name = field_name_for(1); - quote!(1 => compact.#name.push( - ( - index_of_voter(&who).ok_or(_phragmen::Error::CompactInvalidIndex)?, - index_of_target(&distribution[0].0).ok_or(_phragmen::Error::CompactInvalidIndex)?, - ) - ),) - }; - - let from_impl_double = { - let name = field_name_for(2); - quote!(2 => compact.#name.push( - ( - index_of_voter(&who).ok_or(_phragmen::Error::CompactInvalidIndex)?, - ( - index_of_target(&distribution[0].0).ok_or(_phragmen::Error::CompactInvalidIndex)?, - distribution[0].1, - ), - index_of_target(&distribution[1].0).ok_or(_phragmen::Error::CompactInvalidIndex)?, - ) - ),) - }; - - let from_impl_rest = (3..=count).map(|c| { - let inner = (0..c-1).map(|i| - quote!((index_of_target(&distribution[#i].0).ok_or(_phragmen::Error::CompactInvalidIndex)?, distribution[#i].1),) - ).collect::(); - - let field_name = field_name_for(c); - let last_index = c - 1; - let last = quote!(index_of_target(&distribution[#last_index].0).ok_or(_phragmen::Error::CompactInvalidIndex)?); - - quote!( - #c => compact.#field_name.push( - (index_of_voter(&who).ok_or(_phragmen::Error::CompactInvalidIndex)?, [#inner], #last) - ), - ) - }).collect::(); - - quote!( - #from_impl_single - #from_impl_double - #from_impl_rest - ) -} - -fn into_impl(count: usize) -> TokenStream2 { - let into_impl_single = { - let name = field_name_for(1); - quote!( - for (voter_index, target_index) in self.#name { - let who = voter_at(voter_index).ok_or(_phragmen::Error::CompactInvalidIndex)?; - let all_stake: u128 = max_of(&who).into(); - assignments.push(_phragmen::StakedAssignment { - who, - distribution: vec![(target_at(target_index).ok_or(_phragmen::Error::CompactInvalidIndex)?, all_stake)], - }) - } - ) - }; - - let into_impl_double = { - let name = field_name_for(2); - quote!( - for (voter_index, (t1_idx, w1), t2_idx) in self.#name { - let who = voter_at(voter_index).ok_or(_phragmen::Error::CompactInvalidIndex)?; - let all_stake: u128 = max_of(&who).into(); - - if w1 >= all_stake { - return Err(_phragmen::Error::CompactStakeOverflow); - } - - // w2 is ensured to be positive. - let w2 = all_stake - w1; - assignments.push( _phragmen::StakedAssignment { - who, - distribution: vec![ - (target_at(t1_idx).ok_or(_phragmen::Error::CompactInvalidIndex)?, w1), - (target_at(t2_idx).ok_or(_phragmen::Error::CompactInvalidIndex)?, w2), - ] - }); - } - ) - }; - - let into_impl_rest = (3..=count).map(|c| { - let name = field_name_for(c); - quote!( - for (voter_index, inners, t_last_idx) in self.#name { - let who = voter_at(voter_index).ok_or(_phragmen::Error::CompactInvalidIndex)?; - let mut sum = u128::min_value(); - let all_stake: u128 = max_of(&who).into(); - - let mut inners_parsed = inners - .iter() - .map(|(ref t_idx, w)| { - sum = sum.saturating_add(*w); - let target = target_at(*t_idx).ok_or(_phragmen::Error::CompactInvalidIndex)?; - Ok((target, *w)) - }).collect::, _phragmen::Error>>()?; - - if sum >= all_stake { - return Err(_phragmen::Error::CompactStakeOverflow); - } - // w_last is proved to be positive. - let w_last = all_stake - sum; - - inners_parsed.push((target_at(t_last_idx).ok_or(_phragmen::Error::CompactInvalidIndex)?, w_last)); - - assignments.push(_phragmen::StakedAssignment { - who, - distribution: inners_parsed, - }); - } - ) - }).collect::(); - - quote!( - #into_impl_single - #into_impl_double - #into_impl_rest - ) -} - -pub(crate) fn staked( - ident: syn::Ident, - voter_type: GenericArgument, - target_type: GenericArgument, - count: usize, -) -> TokenStream2 { - - let from_impl = from_impl(count); - let into_impl = into_impl(count); - - quote!( - impl< - #voter_type: _phragmen::codec::Codec + Default + Copy, - #target_type: _phragmen::codec::Codec + Default + Copy, - > - #ident<#voter_type, #target_type, u128> - { - /// Generate self from a vector of `StakedAssignment`. - pub fn from_staked( - assignments: Vec<_phragmen::StakedAssignment>, - index_of_voter: FV, - index_of_target: FT, - ) -> Result - where - for<'r> FV: Fn(&'r A) -> Option<#voter_type>, - for<'r> FT: Fn(&'r A) -> Option<#target_type>, - A: _phragmen::IdentifierT - { - let mut compact: #ident<#voter_type, #target_type, u128> = Default::default(); - for _phragmen::StakedAssignment { who, distribution } in assignments { - match distribution.len() { - 0 => continue, - #from_impl - _ => { - return Err(_phragmen::Error::CompactTargetOverflow); - } - } - }; - Ok(compact) - } - - /// Convert self into `StakedAssignment`. The given function should return the total - /// weight of a voter. It is used to subtract the sum of all the encoded weights to - /// infer the last one. - pub fn into_staked( - self, - max_of: FM, - voter_at: impl Fn(#voter_type) -> Option, - target_at: impl Fn(#target_type) -> Option, - ) - -> Result>, _phragmen::Error> - where - for<'r> FM: Fn(&'r A) -> u64, - A: _phragmen::IdentifierT, - { - let mut assignments: Vec<_phragmen::StakedAssignment> = Default::default(); - #into_impl - Ok(assignments) - } - } - ) -} diff --git a/primitives/npos-elections/src/tests.rs b/primitives/npos-elections/src/tests.rs index 80c742117d979..2a8bbec1c69d9 100644 --- a/primitives/npos-elections/src/tests.rs +++ b/primitives/npos-elections/src/tests.rs @@ -865,23 +865,32 @@ mod compact { use super::AccountId; // these need to come from the same dev-dependency `sp-npos-elections`, not from the crate. use crate::{ - generate_compact_solution_type, VoteWeight, Assignment, StakedAssignment, - Error as PhragmenError, ExtendedBalance, + generate_compact_solution_type, Assignment, + Error as PhragmenError, }; - use sp_std::{convert::{TryInto, TryFrom}, fmt::Debug}; + use sp_std::{convert::TryInto, fmt::Debug}; use sp_arithmetic::Percent; - type Accuracy = Percent; + type TestAccuracy = Percent; - generate_compact_solution_type!(TestCompact, 16); + generate_compact_solution_type!(pub struct TestCompact::(16)); + + #[allow(dead_code)] + mod __private { + // This is just to make sure that that the compact can be generated in a scope without any + // imports. + use crate::generate_compact_solution_type; + use sp_arithmetic::Percent; + generate_compact_solution_type!(struct InnerTestCompact::(12)); + } #[test] fn compact_struct_is_codec() { - let compact = TestCompact::<_, _, _> { - votes1: vec![(2u64, 20), (4, 40)], + let compact = TestCompact { + votes1: vec![(2, 20), (4, 40)], votes2: vec![ - (1, (10, Accuracy::from_percent(80)), 11), - (5, (50, Accuracy::from_percent(85)), 51), + (1, (10, TestAccuracy::from_percent(80)), 11), + (5, (50, TestAccuracy::from_percent(85)), 51), ], ..Default::default() }; @@ -896,14 +905,8 @@ mod compact { assert_eq!(compact.edge_count(), 2 + 4); } - fn basic_ratio_test_with() where - V: codec::Codec + Copy + Default + PartialEq + Eq + TryInto + TryFrom + From + Debug, - T: codec::Codec + Copy + Default + PartialEq + Eq + TryInto + TryFrom + From + Debug, - >::Error: std::fmt::Debug, - >::Error: std::fmt::Debug, - >::Error: std::fmt::Debug, - >::Error: std::fmt::Debug, - { + #[test] + fn basic_from_and_into_compact_works_assignments() { let voters = vec![ 2 as AccountId, 4, @@ -926,44 +929,44 @@ mod compact { let assignments = vec![ Assignment { who: 2 as AccountId, - distribution: vec![(20u64, Accuracy::from_percent(100))] + distribution: vec![(20u64, TestAccuracy::from_percent(100))] }, Assignment { who: 4, - distribution: vec![(40, Accuracy::from_percent(100))], + distribution: vec![(40, TestAccuracy::from_percent(100))], }, Assignment { who: 1, distribution: vec![ - (10, Accuracy::from_percent(80)), - (11, Accuracy::from_percent(20)) + (10, TestAccuracy::from_percent(80)), + (11, TestAccuracy::from_percent(20)) ], }, Assignment { who: 5, distribution: vec![ - (50, Accuracy::from_percent(85)), - (51, Accuracy::from_percent(15)), + (50, TestAccuracy::from_percent(85)), + (51, TestAccuracy::from_percent(15)), ] }, Assignment { who: 3, distribution: vec![ - (30, Accuracy::from_percent(50)), - (31, Accuracy::from_percent(25)), - (32, Accuracy::from_percent(25)), + (30, TestAccuracy::from_percent(50)), + (31, TestAccuracy::from_percent(25)), + (32, TestAccuracy::from_percent(25)), ], }, ]; - let voter_index = |a: &AccountId| -> Option { + let voter_index = |a: &AccountId| -> Option { voters.iter().position(|x| x == a).map(TryInto::try_into).unwrap().ok() }; - let target_index = |a: &AccountId| -> Option { + let target_index = |a: &AccountId| -> Option { targets.iter().position(|x| x == a).map(TryInto::try_into).unwrap().ok() }; - let compacted = >::from_assignment( + let compacted = TestCompact::from_assignment( assignments.clone(), voter_index, target_index, @@ -979,265 +982,88 @@ mod compact { assert_eq!( compacted, TestCompact { - votes1: vec![(V::from(0u8), T::from(2u8)), (V::from(1u8), T::from(6u8))], + votes1: vec![(0, 2), (1, 6)], votes2: vec![ - (V::from(2u8), (T::from(0u8), Accuracy::from_percent(80)), T::from(1u8)), - (V::from(3u8), (T::from(7u8), Accuracy::from_percent(85)), T::from(8u8)), + (2, (0, TestAccuracy::from_percent(80)), 1), + (3, (7, TestAccuracy::from_percent(85)), 8), ], votes3: vec![ ( - V::from(4), - [(T::from(3u8), Accuracy::from_percent(50)), (T::from(4u8), Accuracy::from_percent(25))], - T::from(5u8), + 4, + [(3, TestAccuracy::from_percent(50)), (4, TestAccuracy::from_percent(25))], + 5, ), ], ..Default::default() } ); - let voter_at = |a: V| -> Option { voters.get(>::try_into(a).unwrap()).cloned() }; - let target_at = |a: T| -> Option { targets.get(>::try_into(a).unwrap()).cloned() }; - - assert_eq!( - compacted.into_assignment(voter_at, target_at).unwrap(), - assignments, - ); - } - - #[test] - fn basic_from_and_into_compact_works_assignments() { - basic_ratio_test_with::(); - basic_ratio_test_with::(); - basic_ratio_test_with::(); - } - - #[test] - fn basic_from_and_into_compact_works_staked_assignments() { - let voters = vec![ - 2 as AccountId, - 4, - 1, - 5, - 3, - ]; - let targets = vec![ - 10 as AccountId, 11, - 20, - 30, 31, 32, - 40, - 50, 51, - ]; - - let assignments = vec![ - StakedAssignment { - who: 2 as AccountId, - distribution: vec![(20, 100 as ExtendedBalance)] - }, - StakedAssignment { - who: 4, - distribution: vec![(40, 100)], - }, - StakedAssignment { - who: 1, - distribution: vec![ - (10, 80), - (11, 20) - ], - }, - StakedAssignment { - who: 5, distribution: - vec![ - (50, 85), - (51, 15), - ] - }, - StakedAssignment { - who: 3, - distribution: vec![ - (30, 50), - (31, 25), - (32, 25), - ], - }, - ]; - - let voter_index = |a: &AccountId| -> Option { - voters.iter().position(|x| x == a).map(TryInto::try_into).unwrap().ok() + let voter_at = |a: u32| -> Option { + voters.get(>::try_into(a).unwrap()).cloned() }; - let target_index = |a: &AccountId| -> Option { - targets.iter().position(|x| x == a).map(TryInto::try_into).unwrap().ok() + let target_at = |a: u8| -> Option { + targets.get(>::try_into(a).unwrap()).cloned() }; - let compacted = >::from_staked( - assignments.clone(), - voter_index, - target_index, - ).unwrap(); - assert_eq!(compacted.len(), assignments.len()); - assert_eq!( - compacted.edge_count(), - assignments.iter().fold(0, |a, b| a + b.distribution.len()), - ); - - assert_eq!( - compacted, - TestCompact { - votes1: vec![(0, 2), (1, 6)], - votes2: vec![ - (2, (0, 80), 1), - (3, (7, 85), 8), - ], - votes3: vec![ - (4, [(3, 50), (4, 25)], 5), - ], - ..Default::default() - } - ); - - let max_of_fn = |_: &AccountId| -> VoteWeight { 100 }; - let voter_at = |a: u16| -> Option { voters.get(a as usize).cloned() }; - let target_at = |a: u16| -> Option { targets.get(a as usize).cloned() }; - assert_eq!( - compacted.into_staked( - max_of_fn, - voter_at, - target_at, - ).unwrap(), + compacted.into_assignment(voter_at, target_at).unwrap(), assignments, ); } - #[test] - fn compact_into_stake_must_report_overflow() { - // The last edge which is computed from the rest should ALWAYS be positive. - // in votes2 - let compact = TestCompact:: { - votes1: Default::default(), - votes2: vec![(0, (1, 10), 2)], - ..Default::default() - }; - - let entity_at = |a: u16| -> Option { Some(a as AccountId) }; - let max_of = |_: &AccountId| -> VoteWeight { 5 }; - - assert_eq!( - compact.into_staked(&max_of, &entity_at, &entity_at).unwrap_err(), - PhragmenError::CompactStakeOverflow, - ); - - // in votes3 onwards - let compact = TestCompact:: { - votes1: Default::default(), - votes2: Default::default(), - votes3: vec![(0, [(1, 7), (2, 8)], 3)], - ..Default::default() - }; - - assert_eq!( - compact.into_staked(&max_of, &entity_at, &entity_at).unwrap_err(), - PhragmenError::CompactStakeOverflow, - ); - - // Also if equal - let compact = TestCompact:: { - votes1: Default::default(), - votes2: Default::default(), - // 5 is total, we cannot leave none for 30 here. - votes3: vec![(0, [(1, 3), (2, 2)], 3)], - ..Default::default() - }; - - assert_eq!( - compact.into_staked(&max_of, &entity_at, &entity_at).unwrap_err(), - PhragmenError::CompactStakeOverflow, - ); - } - #[test] fn compact_into_assignment_must_report_overflow() { // in votes2 - let compact = TestCompact:: { + let compact = TestCompact { votes1: Default::default(), - votes2: vec![(0, (1, Accuracy::from_percent(100)), 2)], + votes2: vec![(0, (1, TestAccuracy::from_percent(100)), 2)], ..Default::default() }; - let entity_at = |a: u16| -> Option { Some(a as AccountId) }; + let voter_at = |a: u32| -> Option { Some(a as AccountId) }; + let target_at = |a: u8| -> Option { Some(a as AccountId) }; + assert_eq!( - compact.into_assignment(&entity_at, &entity_at).unwrap_err(), + compact.into_assignment(&voter_at, &target_at).unwrap_err(), PhragmenError::CompactStakeOverflow, ); // in votes3 onwards - let compact = TestCompact:: { + let compact = TestCompact { votes1: Default::default(), votes2: Default::default(), - votes3: vec![(0, [(1, Accuracy::from_percent(70)), (2, Accuracy::from_percent(80))], 3)], + votes3: vec![(0, [(1, TestAccuracy::from_percent(70)), (2, TestAccuracy::from_percent(80))], 3)], ..Default::default() }; assert_eq!( - compact.into_assignment(&entity_at, &entity_at).unwrap_err(), + compact.into_assignment(&voter_at, &target_at).unwrap_err(), PhragmenError::CompactStakeOverflow, ); } #[test] fn target_count_overflow_is_detected() { - let assignments = vec![ - StakedAssignment { - who: 1 as AccountId, - distribution: (10..26).map(|i| (i as AccountId, i as ExtendedBalance)).collect::>(), - }, - ]; - - let entity_index = |a: &AccountId| -> Option { Some(*a as u16) }; - - let compacted = >::from_staked( - assignments.clone(), - entity_index, - entity_index, - ); - - assert!(compacted.is_ok()); - - let assignments = vec![ - StakedAssignment { - who: 1 as AccountId, - distribution: (10..27).map(|i| (i as AccountId, i as ExtendedBalance)).collect::>(), - }, - ]; - - let compacted = >::from_staked( - assignments.clone(), - entity_index, - entity_index, - ); - - assert_eq!( - compacted.unwrap_err(), - PhragmenError::CompactTargetOverflow, - ); + let voter_index = |a: &AccountId| -> Option { Some(*a as u32) }; + let target_index = |a: &AccountId| -> Option { Some(*a as u8) }; let assignments = vec![ Assignment { who: 1 as AccountId, - distribution: (10..27).map(|i| (i as AccountId, Percent::from_parts(i as u8))).collect::>(), + distribution: + (10..27) + .map(|i| (i as AccountId, Percent::from_parts(i as u8))) + .collect::>(), }, ]; - let compacted = >::from_assignment( + let compacted = TestCompact::from_assignment( assignments.clone(), - entity_index, - entity_index, - ); - - assert_eq!( - compacted.unwrap_err(), - PhragmenError::CompactTargetOverflow, + voter_index, + target_index, ); + assert_eq!(compacted.unwrap_err(), PhragmenError::CompactTargetOverflow); } #[test] @@ -1246,24 +1072,24 @@ mod compact { let targets = vec![10 as AccountId, 11]; let assignments = vec![ - StakedAssignment { + Assignment { who: 1 as AccountId, - distribution: vec![(10, 100 as ExtendedBalance), (11, 100)] + distribution: vec![(10, Percent::from_percent(50)), (11, Percent::from_percent(50))], }, - StakedAssignment { + Assignment { who: 2, distribution: vec![], }, ]; - let voter_index = |a: &AccountId| -> Option { + let voter_index = |a: &AccountId| -> Option { voters.iter().position(|x| x == a).map(TryInto::try_into).unwrap().ok() }; - let target_index = |a: &AccountId| -> Option { + let target_index = |a: &AccountId| -> Option { targets.iter().position(|x| x == a).map(TryInto::try_into).unwrap().ok() }; - let compacted = >::from_staked( + let compacted = TestCompact::from_assignment( assignments.clone(), voter_index, target_index, @@ -1273,7 +1099,7 @@ mod compact { compacted, TestCompact { votes1: Default::default(), - votes2: vec![(0, (0, 100), 1)], + votes2: vec![(0, (0, Percent::from_percent(50)), 1)], ..Default::default() } ); From 0ff952cdb0c8e6ef1dc0d61d4d87b32c1b2ac44e Mon Sep 17 00:00:00 2001 From: kianenigma Date: Mon, 3 Aug 2020 16:08:17 +0200 Subject: [PATCH 2/5] Make it optional --- frame/staking/src/lib.rs | 5 +- .../fuzzer/src/per_thing_rational.rs | 1 - .../npos-elections/compact/src/assignment.rs | 2 +- .../npos-elections/compact/src/codec.rs | 3 +- .../compact/src/codec_example.rs | 3 +- primitives/npos-elections/compact/src/lib.rs | 97 ++++++++++++------- primitives/npos-elections/src/lib.rs | 16 ++- primitives/npos-elections/src/tests.rs | 67 ++++++++++--- 8 files changed, 137 insertions(+), 57 deletions(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index f5176b2b7decb..cfab6302e0779 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -326,7 +326,7 @@ use frame_system::{ }; use sp_npos_elections::{ ExtendedBalance, Assignment, ElectionScore, ElectionResult as PrimitiveElectionResult, - build_support_map, evaluate_support, seq_phragmen, generate_compact_solution_type, + build_support_map, evaluate_support, seq_phragmen, generate_solution_type, is_score_better, VotingLimit, SupportMap, VoteWeight, }; @@ -369,7 +369,8 @@ pub type EraIndex = u32; pub type RewardPoint = u32; // Note: Maximum nomination limit is set here -- 16. -generate_compact_solution_type!( +generate_solution_type!( + #[compact] pub struct CompactAssignments::(16) ); diff --git a/primitives/arithmetic/fuzzer/src/per_thing_rational.rs b/primitives/arithmetic/fuzzer/src/per_thing_rational.rs index fc22eacc9e499..8ddbd0c6d59d9 100644 --- a/primitives/arithmetic/fuzzer/src/per_thing_rational.rs +++ b/primitives/arithmetic/fuzzer/src/per_thing_rational.rs @@ -118,6 +118,5 @@ fn assert_per_thing_equal_error(a: P, b: P, err: u128) { let a_abs = a.deconstruct().saturated_into::(); let b_abs = b.deconstruct().saturated_into::(); let diff = a_abs.max(b_abs) - a_abs.min(b_abs); - dbg!(&diff); assert!(diff <= err, "{:?} !~ {:?}", a, b); } diff --git a/primitives/npos-elections/compact/src/assignment.rs b/primitives/npos-elections/compact/src/assignment.rs index 95d7ce2fb926e..218cd4f95a76e 100644 --- a/primitives/npos-elections/compact/src/assignment.rs +++ b/primitives/npos-elections/compact/src/assignment.rs @@ -161,11 +161,11 @@ pub(crate) fn assignment( weight_type: syn::Type, count: usize, ) -> TokenStream2 { - let from_impl = from_impl(count); let into_impl = into_impl(count, weight_type.clone()); quote!( + use _phragmen::__OrInvalidIndex; impl #ident { pub fn from_assignment( assignments: Vec<_phragmen::Assignment>, diff --git a/primitives/npos-elections/compact/src/codec.rs b/primitives/npos-elections/compact/src/codec.rs index 0eb0cf0a1f07c..2f2b3a4433a5d 100644 --- a/primitives/npos-elections/compact/src/codec.rs +++ b/primitives/npos-elections/compact/src/codec.rs @@ -56,7 +56,6 @@ fn decode_impl( let #name = #name .into_iter() .map(|(v, t)| (v.0, t.0)) - // .map(|(v, t) (v.0, t.0)|) // Will crash the compiler in a weird way. Report .collect::>(); } }; @@ -93,7 +92,7 @@ fn decode_impl( < Vec<( _phragmen::codec::Compact<#voter_type>, - [(_phragmen::codec::Compact<#target_type>, _phragmen::codec::Compact<#weight_type>); #c], + [(_phragmen::codec::Compact<#target_type>, _phragmen::codec::Compact<#weight_type>); #c-1], _phragmen::codec::Compact<#target_type>, )> as _phragmen::codec::Decode diff --git a/primitives/npos-elections/compact/src/codec_example.rs b/primitives/npos-elections/compact/src/codec_example.rs index b040d6386a461..714aeebb72543 100644 --- a/primitives/npos-elections/compact/src/codec_example.rs +++ b/primitives/npos-elections/compact/src/codec_example.rs @@ -1,4 +1,4 @@ -//! A simpler example of hop compact encoding works. Not to be merged, just for review assistance. +//! A simpler example of how compact encoding works. Not to be merged, just for review assistance. //! Run with `cargo play codec_example.rs` //# parity-scale-codec = { version = "*", features = ["derive"] } @@ -92,6 +92,5 @@ fn main() { let e = s.encode(); let d = ::decode(&mut &*e).unwrap(); - dbg!(&e); assert_eq!(d, s); } diff --git a/primitives/npos-elections/compact/src/lib.rs b/primitives/npos-elections/compact/src/lib.rs index ae27d1a066a99..546b26b3c6978 100644 --- a/primitives/npos-elections/compact/src/lib.rs +++ b/primitives/npos-elections/compact/src/lib.rs @@ -33,7 +33,7 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { syn::Error::new(Span::call_site(), message) } -/// Generates a struct to store the election result in a compact way. This can encode a structure +/// Generates a struct to store the election result in a small way. This can encode a structure /// which is the equivalent of a `sp_npos_elections::Assignment<_>`. /// /// The following data types can be configured by the macro. @@ -49,37 +49,50 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// specified. Attempting to convert from/to an assignment with more distributions will fail. /// /// -/// For example, the following generates a public struct with name `TestCompact` with `u16` voter +/// For example, the following generates a public struct with name `TestSolution` with `u16` voter /// type, `u8` target type and `Perbill` accuracy with maximum of 8 edges per voter. /// /// ```ignore -/// generate_compact_solution_type!(pub struct TestCompact::(8)) +/// generate_solution_type!(pub struct TestSolution::(8)) /// ``` /// /// The given struct provides function to convert from/to Assignment: /// /// - [`from_assignment()`]. /// - [`fn into_assignment()`]. +/// +/// The generated struct is by default deriving both `Encode` and `Decode`. This is okay but could +/// lead to many 0s in the solution. If prefixed with `#[compact]`, then a custom compact encoding +/// for numbers will be used, similar to how `parity-scale-codec`'s `Compact` works. +/// +/// ```ignore +/// generate_solution_type!( +/// #[compact] +/// pub struct TestSolutionCompact::(8) +/// ) +/// ``` #[proc_macro] -pub fn generate_compact_solution_type(item: TokenStream) -> TokenStream { - let CompactSolutionDef { +pub fn generate_solution_type(item: TokenStream) -> TokenStream { + let SolutionDef { vis, ident, count, voter_type, target_type, weight_type, - } = syn::parse_macro_input!(item as CompactSolutionDef); + compact_encoding, + } = syn::parse_macro_input!(item as SolutionDef); let imports = imports().unwrap_or_else(|e| e.to_compile_error()); - let compact_def = struct_def( + let solution_struct = struct_def( vis, ident.clone(), count, voter_type.clone(), target_type.clone(), weight_type.clone(), + compact_encoding, ).unwrap_or_else(|e| e.to_compile_error()); let assignment_impls = assignment::assignment( @@ -90,16 +103,10 @@ pub fn generate_compact_solution_type(item: TokenStream) -> TokenStream { count, ); - let codec = codec::codec_impl(ident, voter_type, target_type, weight_type, count); - - let or_invalid_index = or_invalid_index_impl(); - quote!( #imports - #or_invalid_index - #compact_def + #solution_struct #assignment_impls - #codec ).into() } @@ -110,6 +117,7 @@ fn struct_def( voter_type: syn::Type, target_type: syn::Type, weight_type: syn::Type, + compact_encoding: bool, ) -> Result { if count <= 2 { Err(syn_err("cannot build compact solution struct with capacity less than 2."))? @@ -158,9 +166,27 @@ fn struct_def( ) }).collect::(); + let derives_and_maybe_compact_encoding = if compact_encoding { + // custom compact encoding. + let compact_impl = codec::codec_impl( + ident.clone(), + voter_type.clone(), + target_type.clone(), + weight_type.clone(), + count, + ); + quote!{ + #compact_impl + #[derive(Default, PartialEq, Eq, Clone, Debug)] + } + } else { + // automatically derived. + quote!(#[derive(Default, PartialEq, Eq, Clone, Debug, _phragmen::codec::Encode, _phragmen::codec::Decode)]) + }; + Ok(quote! ( /// A struct to encode a election assignment in a compact way. - #[derive(Default, PartialEq, Eq, Clone, Debug)] + #derives_and_maybe_compact_encoding #vis struct #ident { #singles #doubles #rest } impl _phragmen::VotingLimit for #ident { @@ -207,33 +233,36 @@ fn imports() -> Result { } } -fn or_invalid_index_impl() -> TokenStream2 { - quote! { - // Simple Extension trait to easily convert `None` from index closures to `Err`. - trait OrInvalidIndex { - fn or_invalid_index(self) -> Result; - } - - impl OrInvalidIndex for Option { - fn or_invalid_index(self) -> Result { - self.ok_or(_phragmen::Error::CompactInvalidIndex) - } - } - } -} - -struct CompactSolutionDef { +struct SolutionDef { vis: syn::Visibility, ident: syn::Ident, voter_type: syn::Type, target_type: syn::Type, weight_type: syn::Type, count: usize, + compact_encoding: bool, } -/// pub struct CompactName::() -impl Parse for CompactSolutionDef { +fn check_compact_attr(input: ParseStream) -> bool { + let mut attrs = input.call(syn::Attribute::parse_outer).unwrap_or_default(); + if attrs.len() == 1 { + let attr = attrs.pop().expect("Vec with len 1 can be popped."); + if attr.path.segments.len() == 1 { + let segment = attr.path.segments.first().expect("Vec with len 1 can be popped."); + if segment.ident == Ident::new("compact", Span::call_site()) { + return true + } + } + } + false +} + +/// #[compact] pub struct CompactName::() +impl Parse for SolutionDef { fn parse(input: ParseStream) -> syn::Result { + // optional #[compact] + let compact_encoding = check_compact_attr(input); + // struct let vis: syn::Visibility = input.parse()?; let _ = ::parse(input)?; @@ -271,7 +300,7 @@ impl Parse for CompactSolutionDef { }; let count = int_lit.base10_parse::()?; - Ok(Self { vis, ident, voter_type, target_type, weight_type, count } ) + Ok(Self { vis, ident, voter_type, target_type, weight_type, count, compact_encoding } ) } } diff --git a/primitives/npos-elections/src/lib.rs b/primitives/npos-elections/src/lib.rs index 2b767d7c79b94..58a69a116914f 100644 --- a/primitives/npos-elections/src/lib.rs +++ b/primitives/npos-elections/src/lib.rs @@ -60,8 +60,22 @@ pub use codec; #[doc(hidden)] pub use sp_arithmetic; +/// Simple Extension trait to easily convert `None` from index closures to `Err`. +/// +/// This is only generated and re-exported for the compact solution code to use. +#[doc(hidden)] +pub trait __OrInvalidIndex { + fn or_invalid_index(self) -> Result; +} + +impl __OrInvalidIndex for Option { + fn or_invalid_index(self) -> Result { + self.ok_or(Error::CompactInvalidIndex) + } +} + // re-export the compact solution type. -pub use sp_npos_elections_compact::generate_compact_solution_type; +pub use sp_npos_elections_compact::generate_solution_type; /// A trait to limit the number of votes per voter. The generated compact type will implement this. pub trait VotingLimit { diff --git a/primitives/npos-elections/src/tests.rs b/primitives/npos-elections/src/tests.rs index 1b92e8c20760a..8e99d2222e885 100644 --- a/primitives/npos-elections/src/tests.rs +++ b/primitives/npos-elections/src/tests.rs @@ -920,12 +920,12 @@ mod score { } } -mod compact { +mod solution_type { use codec::{Decode, Encode}; use super::AccountId; // these need to come from the same dev-dependency `sp-npos-elections`, not from the crate. use crate::{ - generate_compact_solution_type, Assignment, + generate_solution_type, Assignment, Error as PhragmenError, }; use sp_std::{convert::TryInto, fmt::Debug}; @@ -933,20 +933,59 @@ mod compact { type TestAccuracy = Percent; - generate_compact_solution_type!(pub struct TestCompact::(16)); + generate_solution_type!(pub struct TestSolutionCompact::(16)); #[allow(dead_code)] mod __private { // This is just to make sure that that the compact can be generated in a scope without any // imports. - use crate::generate_compact_solution_type; + use crate::generate_solution_type; use sp_arithmetic::Percent; - generate_compact_solution_type!(struct InnerTestCompact::(12)); + generate_solution_type!( + #[compact] + struct InnerTestSolutionCompact::(12) + ); + + } + + #[test] + fn solution_struct_works_with_and_without_compact() { + // we use u32 size to make sure compact is smaller. + let without_compact = { + generate_solution_type!(pub struct InnerTestSolution::(16)); + let compact = InnerTestSolution { + votes1: vec![(2, 20), (4, 40)], + votes2: vec![ + (1, (10, TestAccuracy::from_percent(80)), 11), + (5, (50, TestAccuracy::from_percent(85)), 51), + ], + ..Default::default() + }; + + compact.encode().len() + }; + + let with_compact = { + generate_solution_type!(#[compact] pub struct InnerTestSolutionCompact::(16)); + let compact = InnerTestSolutionCompact { + votes1: vec![(2, 20), (4, 40)], + votes2: vec![ + (1, (10, TestAccuracy::from_percent(80)), 11), + (5, (50, TestAccuracy::from_percent(85)), 51), + ], + ..Default::default() + }; + + compact.encode().len() + }; + + dbg!(with_compact, without_compact); + assert!(with_compact < without_compact); } #[test] - fn compact_struct_is_codec() { - let compact = TestCompact { + fn solution_struct_is_codec() { + let compact = TestSolutionCompact { votes1: vec![(2, 20), (4, 40)], votes2: vec![ (1, (10, TestAccuracy::from_percent(80)), 11), @@ -1026,7 +1065,7 @@ mod compact { targets.iter().position(|x| x == a).map(TryInto::try_into).unwrap().ok() }; - let compacted = TestCompact::from_assignment( + let compacted = TestSolutionCompact::from_assignment( assignments.clone(), voter_index, target_index, @@ -1041,7 +1080,7 @@ mod compact { assert_eq!( compacted, - TestCompact { + TestSolutionCompact { votes1: vec![(0, 2), (1, 6)], votes2: vec![ (2, (0, TestAccuracy::from_percent(80)), 1), @@ -1074,7 +1113,7 @@ mod compact { #[test] fn compact_into_assignment_must_report_overflow() { // in votes2 - let compact = TestCompact { + let compact = TestSolutionCompact { votes1: Default::default(), votes2: vec![(0, (1, TestAccuracy::from_percent(100)), 2)], ..Default::default() @@ -1090,7 +1129,7 @@ mod compact { ); // in votes3 onwards - let compact = TestCompact { + let compact = TestSolutionCompact { votes1: Default::default(), votes2: Default::default(), votes3: vec![(0, [(1, TestAccuracy::from_percent(70)), (2, TestAccuracy::from_percent(80))], 3)], @@ -1118,7 +1157,7 @@ mod compact { }, ]; - let compacted = TestCompact::from_assignment( + let compacted = TestSolutionCompact::from_assignment( assignments.clone(), voter_index, target_index, @@ -1149,7 +1188,7 @@ mod compact { targets.iter().position(|x| x == a).map(TryInto::try_into).unwrap().ok() }; - let compacted = TestCompact::from_assignment( + let compacted = TestSolutionCompact::from_assignment( assignments.clone(), voter_index, target_index, @@ -1157,7 +1196,7 @@ mod compact { assert_eq!( compacted, - TestCompact { + TestSolutionCompact { votes1: Default::default(), votes2: vec![(0, (0, Percent::from_percent(50)), 1)], ..Default::default() From 8c7b07a5257f9ef9ef6e3529bf208443da561dca Mon Sep 17 00:00:00 2001 From: kianenigma Date: Mon, 3 Aug 2020 16:09:20 +0200 Subject: [PATCH 3/5] Remove extra file --- .../npos-elections/compact/src/codec.rs | 1 - .../compact/src/codec_example.rs | 96 ------------------- 2 files changed, 97 deletions(-) delete mode 100644 primitives/npos-elections/compact/src/codec_example.rs diff --git a/primitives/npos-elections/compact/src/codec.rs b/primitives/npos-elections/compact/src/codec.rs index 2f2b3a4433a5d..0a475bdddcd5b 100644 --- a/primitives/npos-elections/compact/src/codec.rs +++ b/primitives/npos-elections/compact/src/codec.rs @@ -143,7 +143,6 @@ fn encode_impl(ident: syn::Ident, count: usize) -> TokenStream2 { _phragmen::codec::Compact(t.clone()), )) .collect::>(); - // TODO: will this work without Encode in scope? no probably. #name.encode_to(&mut r); } }; diff --git a/primitives/npos-elections/compact/src/codec_example.rs b/primitives/npos-elections/compact/src/codec_example.rs deleted file mode 100644 index 714aeebb72543..0000000000000 --- a/primitives/npos-elections/compact/src/codec_example.rs +++ /dev/null @@ -1,96 +0,0 @@ -//! A simpler example of how compact encoding works. Not to be merged, just for review assistance. -//! Run with `cargo play codec_example.rs` - -//# parity-scale-codec = { version = "*", features = ["derive"] } - -use parity_scale_codec::{Encode, Decode, Compact, Input, Error}; - -#[derive(Default, Eq, PartialEq, Debug)] -struct BareSolution { - votes1: Vec<(u32, u16)>, - votes2: Vec<(u32, (u16, u64), u16)>, - votes3: Vec<(u32, [(u16, u64); 2usize], u16)>, -} - -impl Encode for BareSolution { - fn encode(&self) -> Vec { - let mut r = vec![]; - - let votes1 = self.votes1 - .iter() - .map(|(v, t)| (Compact(*v), Compact(*t))) - .collect::>(); - votes1.encode_to(&mut r); - - let votes2 = self.votes2 - .iter() - .map(|(v, (t1, w), t2)| ( - Compact(*v), - ( - Compact(*t1), - Compact(*w), - ), - Compact(*t2), - )) - .collect::>(); - votes2.encode_to(&mut r); - - let votes3 = self.votes3 - .iter() - .map(|(v, inners, t_last)| { - let inners_compact_array = [ - (Compact(inners[0].0), Compact(inners[0].1)), - (Compact(inners[1].0), Compact(inners[1].1)), - ]; - (Compact(*v), inners_compact_array, Compact(*t_last)) - }).collect::>(); - - votes3.encode_to(&mut r); - r - } -} - -impl Decode for BareSolution { - fn decode(value: &mut I) -> Result { - let votes1 = , Compact)> as Decode>::decode(value)?; - let votes1 = votes1.into_iter().map(|(v, t)| (v.0, t.0)).collect::>(); - - let votes2 = , (Compact, Compact), Compact)> as Decode>::decode(value)?; - let votes2 = votes2.into_iter().map(|(v, (t1, w), t2)| (v.0, (t1.0, w.0), t2.into())).collect::>(); - - let votes3 = , [(Compact, Compact); 2], Compact)> as Decode>::decode(value)?; - let votes3 = votes3 - .into_iter() - .map(|(v, inner, t_last)| ( - v.0, - [ - ((inner[0].0).0, (inner[0].1).0), - ((inner[1].0).0, (inner[1].1).0) - ], - t_last.0, - )) - .collect::>(); - - Ok(Self { - votes1, - votes2, - votes3, - ..Default::default() - }) - } -} - -fn main() { - let s = BareSolution { - votes1: vec![(1,2), (3, 1)], - votes2: vec![(1, (99, 33), 44), (2, (99, 33), 44)], - votes3: vec![ - (1, [(99, 33), (55, 33)], 44), - (2, [(99, 33), (55, 33)], 44), - ] - }; - - let e = s.encode(); - let d = ::decode(&mut &*e).unwrap(); - assert_eq!(d, s); -} From 41ad894c80354ad44befc14ef7ba0b27cf5da908 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Thu, 6 Aug 2020 15:22:22 +0200 Subject: [PATCH 4/5] Update primitives/npos-elections/compact/src/lib.rs Co-authored-by: Guillaume Thiolliere --- primitives/npos-elections/compact/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/primitives/npos-elections/compact/src/lib.rs b/primitives/npos-elections/compact/src/lib.rs index 546b26b3c6978..ec0c26655ba1a 100644 --- a/primitives/npos-elections/compact/src/lib.rs +++ b/primitives/npos-elections/compact/src/lib.rs @@ -296,7 +296,7 @@ impl Parse for SolutionDef { }; let int_lit = match expr_lit { syn::Lit::Int(int_lit) => int_lit, - _ => return Err(syn_err("Count must be literal.")) + _ => return Err(syn_err("Count must be int literal.")) }; let count = int_lit.base10_parse::()?; From 03ce65735f76085b5da0ca76e1eefc8f3b6923ef Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 6 Aug 2020 17:36:38 +0200 Subject: [PATCH 5/5] Final fixes. --- primitives/npos-elections/compact/src/lib.rs | 23 ++++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/primitives/npos-elections/compact/src/lib.rs b/primitives/npos-elections/compact/src/lib.rs index 546b26b3c6978..50c0073d67047 100644 --- a/primitives/npos-elections/compact/src/lib.rs +++ b/primitives/npos-elections/compact/src/lib.rs @@ -40,8 +40,8 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// /// - The identifier of the voter. This can be any type that supports `parity-scale-codec`'s compact /// encoding. -/// - The identifier of the voter. This can be any type that supports `parity-scale-codec`'s compact -/// encoding. +/// - The identifier of the target. This can be any type that supports `parity-scale-codec`'s +/// compact encoding. /// - The accuracy of the ratios. This must be one of the `PerThing` types defined in /// `sp-arithmetic`. /// @@ -67,8 +67,8 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// /// ```ignore /// generate_solution_type!( -/// #[compact] -/// pub struct TestSolutionCompact::(8) +/// #[compact] +/// pub struct TestSolutionCompact::(8) /// ) /// ``` #[proc_macro] @@ -120,7 +120,7 @@ fn struct_def( compact_encoding: bool, ) -> Result { if count <= 2 { - Err(syn_err("cannot build compact solution struct with capacity less than 2."))? + Err(syn_err("cannot build compact solution struct with capacity less than 3."))? } let singles = { @@ -243,25 +243,30 @@ struct SolutionDef { compact_encoding: bool, } -fn check_compact_attr(input: ParseStream) -> bool { +fn check_compact_attr(input: ParseStream) -> Result { let mut attrs = input.call(syn::Attribute::parse_outer).unwrap_or_default(); if attrs.len() == 1 { let attr = attrs.pop().expect("Vec with len 1 can be popped."); if attr.path.segments.len() == 1 { let segment = attr.path.segments.first().expect("Vec with len 1 can be popped."); if segment.ident == Ident::new("compact", Span::call_site()) { - return true + Ok(true) + } else { + Err(syn_err("generate_solution_type macro can only accept #[compact] attribute.")) } + } else { + Err(syn_err("generate_solution_type macro can only accept #[compact] attribute.")) } + } else { + Ok(false) } - false } /// #[compact] pub struct CompactName::() impl Parse for SolutionDef { fn parse(input: ParseStream) -> syn::Result { // optional #[compact] - let compact_encoding = check_compact_attr(input); + let compact_encoding = check_compact_attr(input)?; // struct let vis: syn::Visibility = input.parse()?;