diff --git a/Cargo.lock b/Cargo.lock index c869f0c8dfcf3..47ac7e9a011ed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8925,10 +8925,14 @@ dependencies = [ name = "sp-npos-elections-compact" version = "3.0.0" dependencies = [ + "parity-scale-codec 2.0.1", "proc-macro-crate 1.0.0", "proc-macro2", "quote", + "sp-arithmetic", + "sp-npos-elections", "syn", + "trybuild", ] [[package]] diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 46f80cc56afd9..fcd120d000bf1 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -519,8 +519,11 @@ parameter_types! { sp_npos_elections::generate_solution_type!( #[compact] - pub struct NposCompactSolution16::(16) - // -------------------- ^^ + pub struct NposCompactSolution16::< + VoterIndex = u32, + TargetIndex = u16, + Accuracy = sp_runtime::PerU16, + >(16) ); impl pallet_election_provider_multi_phase::Config for Runtime { diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 22b5a0ac67b7a..cebd5cf06e692 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -65,7 +65,7 @@ pub(crate) type TargetIndex = u16; sp_npos_elections::generate_solution_type!( #[compact] - pub struct TestCompact::(16) + pub struct TestCompact::(16) ); /// All events of this pallet. diff --git a/primitives/npos-elections/compact/Cargo.toml b/primitives/npos-elections/compact/Cargo.toml index e2fff8e2db010..63432a36efc80 100644 --- a/primitives/npos-elections/compact/Cargo.toml +++ b/primitives/npos-elections/compact/Cargo.toml @@ -19,3 +19,9 @@ syn = { version = "1.0.58", features = ["full", "visit"] } quote = "1.0" proc-macro2 = "1.0.6" proc-macro-crate = "1.0.0" + +[dev-dependencies] +parity-scale-codec = "2.0.1" +sp-arithmetic = { path = "../../arithmetic" } +sp-npos-elections = { path = ".." } +trybuild = "1.0.41" diff --git a/primitives/npos-elections/compact/src/lib.rs b/primitives/npos-elections/compact/src/lib.rs index dd6d4de9b0241..e558ae89ca93e 100644 --- a/primitives/npos-elections/compact/src/lib.rs +++ b/primitives/npos-elections/compact/src/lib.rs @@ -52,8 +52,14 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// 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_solution_type!(pub struct TestSolution::(8)) +/// ``` +/// # use sp_npos_elections_compact::generate_solution_type; +/// # use sp_arithmetic::per_things::Perbill; +/// generate_solution_type!(pub struct TestSolution::< +/// VoterIndex = u16, +/// TargetIndex = u8, +/// Accuracy = Perbill, +/// >(8)); /// ``` /// /// The given struct provides function to convert from/to Assignment: @@ -65,11 +71,13 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error { /// 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 +/// ``` +/// # use sp_npos_elections_compact::generate_solution_type; +/// # use sp_arithmetic::per_things::Perbill; /// generate_solution_type!( /// #[compact] -/// pub struct TestSolutionCompact::(8) -/// ) +/// pub struct TestSolutionCompact::(8) +/// ); /// ``` #[proc_macro] pub fn generate_solution_type(item: TokenStream) -> TokenStream { @@ -386,7 +394,7 @@ fn check_compact_attr(input: ParseStream) -> Result { } } -/// #[compact] pub struct CompactName::() +/// #[compact] pub struct CompactName::() impl Parse for SolutionDef { fn parse(input: ParseStream) -> syn::Result { // optional #[compact] @@ -405,9 +413,22 @@ impl Parse for SolutionDef { return Err(syn_err("Must provide 3 generic args.")) } - let mut types: Vec = generics.args.iter().map(|t| + let expected_types = ["VoterIndex", "TargetIndex", "Accuracy"]; + + let mut types: Vec = generics.args.iter().zip(expected_types.iter()).map(|(t, expected)| match t { - syn::GenericArgument::Type(ty) => Ok(ty.clone()), + syn::GenericArgument::Type(ty) => { + // this is now an error + Err(syn::Error::new_spanned(ty, format!("Expected binding: `{} = ...`", expected))) + }, + syn::GenericArgument::Binding(syn::Binding{ident, ty, ..}) => { + // check that we have the right keyword for this position in the argument list + if ident == expected { + Ok(ty.clone()) + } else { + Err(syn::Error::new_spanned(ident, format!("Expected `{}`", expected))) + } + } _ => Err(syn_err("Wrong type of generic provided. Must be a `type`.")), } ).collect::>()?; @@ -436,3 +457,12 @@ impl Parse for SolutionDef { fn field_name_for(n: usize) -> Ident { Ident::new(&format!("{}{}", PREFIX, n), Span::call_site()) } + +#[cfg(test)] +mod tests { + #[test] + fn ui_fail() { + let cases = trybuild::TestCases::new(); + cases.compile_fail("tests/ui/fail/*.rs"); + } +} diff --git a/primitives/npos-elections/compact/tests/ui/fail/missing_accuracy.rs b/primitives/npos-elections/compact/tests/ui/fail/missing_accuracy.rs new file mode 100644 index 0000000000000..4bbf4960a9483 --- /dev/null +++ b/primitives/npos-elections/compact/tests/ui/fail/missing_accuracy.rs @@ -0,0 +1,9 @@ +use sp_npos_elections_compact::generate_solution_type; + +generate_solution_type!(pub struct TestSolution::< + VoterIndex = u16, + TargetIndex = u8, + Perbill, +>(8)); + +fn main() {} diff --git a/primitives/npos-elections/compact/tests/ui/fail/missing_accuracy.stderr b/primitives/npos-elections/compact/tests/ui/fail/missing_accuracy.stderr new file mode 100644 index 0000000000000..b6bb8f39ede61 --- /dev/null +++ b/primitives/npos-elections/compact/tests/ui/fail/missing_accuracy.stderr @@ -0,0 +1,5 @@ +error: Expected binding: `Accuracy = ...` + --> $DIR/missing_accuracy.rs:6:2 + | +6 | Perbill, + | ^^^^^^^ diff --git a/primitives/npos-elections/compact/tests/ui/fail/missing_target.rs b/primitives/npos-elections/compact/tests/ui/fail/missing_target.rs new file mode 100644 index 0000000000000..7d7584340713c --- /dev/null +++ b/primitives/npos-elections/compact/tests/ui/fail/missing_target.rs @@ -0,0 +1,9 @@ +use sp_npos_elections_compact::generate_solution_type; + +generate_solution_type!(pub struct TestSolution::< + VoterIndex = u16, + u8, + Accuracy = Perbill, +>(8)); + +fn main() {} diff --git a/primitives/npos-elections/compact/tests/ui/fail/missing_target.stderr b/primitives/npos-elections/compact/tests/ui/fail/missing_target.stderr new file mode 100644 index 0000000000000..d0c92c5bbd8e9 --- /dev/null +++ b/primitives/npos-elections/compact/tests/ui/fail/missing_target.stderr @@ -0,0 +1,5 @@ +error: Expected binding: `TargetIndex = ...` + --> $DIR/missing_target.rs:5:2 + | +5 | u8, + | ^^ diff --git a/primitives/npos-elections/compact/tests/ui/fail/missing_voter.rs b/primitives/npos-elections/compact/tests/ui/fail/missing_voter.rs new file mode 100644 index 0000000000000..3ad77dc104ad7 --- /dev/null +++ b/primitives/npos-elections/compact/tests/ui/fail/missing_voter.rs @@ -0,0 +1,9 @@ +use sp_npos_elections_compact::generate_solution_type; + +generate_solution_type!(pub struct TestSolution::< + u16, + TargetIndex = u8, + Accuracy = Perbill, +>(8)); + +fn main() {} diff --git a/primitives/npos-elections/compact/tests/ui/fail/missing_voter.stderr b/primitives/npos-elections/compact/tests/ui/fail/missing_voter.stderr new file mode 100644 index 0000000000000..a825d460c2fa8 --- /dev/null +++ b/primitives/npos-elections/compact/tests/ui/fail/missing_voter.stderr @@ -0,0 +1,5 @@ +error: Expected binding: `VoterIndex = ...` + --> $DIR/missing_voter.rs:4:2 + | +4 | u16, + | ^^^ diff --git a/primitives/npos-elections/compact/tests/ui/fail/no_annotations.rs b/primitives/npos-elections/compact/tests/ui/fail/no_annotations.rs new file mode 100644 index 0000000000000..aaebb857b3d8d --- /dev/null +++ b/primitives/npos-elections/compact/tests/ui/fail/no_annotations.rs @@ -0,0 +1,9 @@ +use sp_npos_elections_compact::generate_solution_type; + +generate_solution_type!(pub struct TestSolution::< + u16, + u8, + Perbill, +>(8)); + +fn main() {} diff --git a/primitives/npos-elections/compact/tests/ui/fail/no_annotations.stderr b/primitives/npos-elections/compact/tests/ui/fail/no_annotations.stderr new file mode 100644 index 0000000000000..28f1c2091546f --- /dev/null +++ b/primitives/npos-elections/compact/tests/ui/fail/no_annotations.stderr @@ -0,0 +1,5 @@ +error: Expected binding: `VoterIndex = ...` + --> $DIR/no_annotations.rs:4:2 + | +4 | u16, + | ^^^ diff --git a/primitives/npos-elections/compact/tests/ui/fail/swap_voter_target.rs b/primitives/npos-elections/compact/tests/ui/fail/swap_voter_target.rs new file mode 100644 index 0000000000000..37124256b35e4 --- /dev/null +++ b/primitives/npos-elections/compact/tests/ui/fail/swap_voter_target.rs @@ -0,0 +1,9 @@ +use sp_npos_elections_compact::generate_solution_type; + +generate_solution_type!(pub struct TestSolution::< + TargetIndex = u16, + VoterIndex = u8, + Accuracy = Perbill, +>(8)); + +fn main() {} diff --git a/primitives/npos-elections/compact/tests/ui/fail/swap_voter_target.stderr b/primitives/npos-elections/compact/tests/ui/fail/swap_voter_target.stderr new file mode 100644 index 0000000000000..5759fee7472fa --- /dev/null +++ b/primitives/npos-elections/compact/tests/ui/fail/swap_voter_target.stderr @@ -0,0 +1,5 @@ +error: Expected `VoterIndex` + --> $DIR/swap_voter_target.rs:4:2 + | +4 | TargetIndex = u16, + | ^^^^^^^^^^^ diff --git a/primitives/npos-elections/fuzzer/src/compact.rs b/primitives/npos-elections/fuzzer/src/compact.rs index 91f734bb5b7cb..a49f6a535e5f0 100644 --- a/primitives/npos-elections/fuzzer/src/compact.rs +++ b/primitives/npos-elections/fuzzer/src/compact.rs @@ -4,7 +4,11 @@ use sp_npos_elections::sp_arithmetic::Percent; use sp_runtime::codec::{Encode, Error}; fn main() { - generate_solution_type!(#[compact] pub struct InnerTestSolutionCompact::(16)); + generate_solution_type!(#[compact] pub struct InnerTestSolutionCompact::< + VoterIndex = u32, + TargetIndex = u32, + Accuracy = Percent, + >(16)); loop { fuzz!(|fuzzer_data: &[u8]| { let result_decoded: Result = diff --git a/primitives/npos-elections/src/tests.rs b/primitives/npos-elections/src/tests.rs index 7bd8565a072fd..6304e50ec5868 100644 --- a/primitives/npos-elections/src/tests.rs +++ b/primitives/npos-elections/src/tests.rs @@ -1148,7 +1148,11 @@ mod solution_type { type TestAccuracy = Percent; - generate_solution_type!(pub struct TestSolutionCompact::(16)); + generate_solution_type!(pub struct TestSolutionCompact::< + VoterIndex = u32, + TargetIndex = u8, + Accuracy = TestAccuracy, + >(16)); #[allow(dead_code)] mod __private { @@ -1158,7 +1162,7 @@ mod solution_type { use sp_arithmetic::Percent; generate_solution_type!( #[compact] - struct InnerTestSolutionCompact::(12) + struct InnerTestSolutionCompact::(12) ); } @@ -1166,7 +1170,11 @@ mod solution_type { 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)); + generate_solution_type!(pub struct InnerTestSolution::< + VoterIndex = u32, + TargetIndex = u32, + Accuracy = Percent, + >(16)); let compact = InnerTestSolution { votes1: vec![(2, 20), (4, 40)], votes2: vec![ @@ -1180,7 +1188,11 @@ mod solution_type { }; let with_compact = { - generate_solution_type!(#[compact] pub struct InnerTestSolutionCompact::(16)); + generate_solution_type!(#[compact] pub struct InnerTestSolutionCompact::< + VoterIndex = u32, + TargetIndex = u32, + Accuracy = Percent, + >(16)); let compact = InnerTestSolutionCompact { votes1: vec![(2, 20), (4, 40)], votes2: vec![