Skip to content
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 a compact flag to Field #42

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 30 additions & 4 deletions derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use syn::{
},
punctuated::Punctuated,
token::Comma,
AttrStyle,
Data,
DataEnum,
DataStruct,
Expand All @@ -46,6 +47,9 @@ use syn::{
Field,
Fields,
Lit,
Meta,
MetaList,
NestedMeta,
Variant,
};

Expand Down Expand Up @@ -92,7 +96,6 @@ fn generate_type(input: TokenStream2) -> Result<TokenStream2> {
.path(::scale_info::Path::new(stringify!(#ident), module_path!()))
.type_params(::scale_info::prelude::vec![ #( #generic_type_ids ),* ])
.#build_type
.into()
}
}
};
Expand All @@ -108,20 +111,43 @@ fn generate_fields(fields: &FieldsList) -> Vec<TokenStream2> {
.map(|f| {
let (ty, ident) = (&f.ty, &f.ident);
let type_name = clean_type_string(&quote!(#ty).to_string());

let compact = if is_compact(f) {
quote! {
.compact()
}
} else {
quote! {}
};
if let Some(i) = ident {
quote! {
.field_of::<#ty>(stringify!(#i), #type_name)
.field_of::<#ty>(stringify!(#i), #type_name) #compact
}
} else {
quote! {
.field_of::<#ty>(#type_name)
.field_of::<#ty>(#type_name) #compact
}
}
})
.collect()
}

/// Look for a `#[codec(compact)]` outer attribute.
fn is_compact(f: &Field) -> bool {
f.attrs.iter().any(|attr| {
let mut is_compact = false;
if attr.style == AttrStyle::Outer && attr.path.is_ident("codec") {
if let Ok(Meta::List(MetaList { nested, .. })) = attr.parse_meta() {
if let Some(NestedMeta::Meta(Meta::Path(path))) = nested.iter().next() {
if path.is_ident("compact") {
is_compact = true;
}
}
}
}
is_compact
})
}

fn clean_type_string(input: &str) -> String {
input
.replace(" ::", "::")
Expand Down
9 changes: 9 additions & 0 deletions src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,15 @@ impl<T> FieldsBuilder<T> {
pub fn finalize(self) -> Vec<Field<MetaForm>> {
self.fields
}

/// Mark last field as compact, meaning that encoding/decoding should be in the [`scale_codec::Compact`] format.
pub fn compact(mut self) -> Self {
self.fields.iter_mut().last().map(|f| {
f.compact();
f
});
self
}
}

impl FieldsBuilder<NamedFields> {
Expand Down
1 change: 0 additions & 1 deletion src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,6 @@ mod tests {
"&mut RecursiveRefs",
),
)
.into()
}
}

Expand Down
1 change: 0 additions & 1 deletion src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ fn struct_with_generics() {
.path(Path::new("MyStruct", module_path!()))
.type_params(tuple_meta_type!(T))
.composite(Fields::named().field_of::<T>("data", "T"))
.into()
}
}

Expand Down
24 changes: 22 additions & 2 deletions src/ty/fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,17 @@ pub struct Field<T: Form = MetaForm> {
ty: T::Type,
/// The name of the type of the field as it appears in the source code.
type_name: T::String,
/// This field should be encode/decoded as a
/// [`Compact`](parity_scale_codec::Compact) field
#[cfg_attr(feature = "serde", serde(skip_serializing_if = "is_false", default))]
compact: bool,
Comment on lines +89 to +92
Copy link
Contributor

@Robbepop Robbepop Jan 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think an even more compact representation for the exact same use case would be to use yet another enum variant in order to represent compacted types. The idea is to add another variant Compact(TypeDefCompact<T>) to the TypeDef enum: https://docs.rs/scale-info/0.4.1/scale_info/enum.TypeDef.html
The TypeDefCompact type internally could look like this:

pub struct TypeDefCompact<T>
where
    T: Form = MetaForm
{
    #[serde(rename = "type")]
    type_param: T::Type,
}

Then instead of requiring each type to have this bool field we'd simply have a compact variant only for those type definitions that are actually compactly encoded. Since this is kind of a special case this design would suite much better. Also it would better reflect the encoding structure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first glance I think this solution is more elegant than having the field, while still making it a first class citizen (unlike just using a type e.g. Compact<T>.

}

// Need to obey the required serde signature here
#[allow(clippy::trivially_copy_pass_by_ref)]
#[allow(dead_code)]
const fn is_false(v: &bool) -> bool {
!(*v)
}

impl IntoPortable for Field {
Expand All @@ -96,6 +107,7 @@ impl IntoPortable for Field {
name: self.name.map(|name| name.into_portable(registry)),
ty: registry.register_type(&self.ty),
type_name: self.type_name.into_portable(registry),
compact: self.compact,
}
}
}
Expand All @@ -108,11 +120,13 @@ impl Field {
name: Option<&'static str>,
ty: MetaType,
type_name: &'static str,
compact: bool,
) -> Self {
Self {
name,
ty,
type_name,
compact,
}
}

Expand All @@ -124,7 +138,7 @@ impl Field {
where
T: TypeInfo + ?Sized + 'static,
{
Self::new(Some(name), MetaType::new::<T>(), type_name)
Self::new(Some(name), MetaType::new::<T>(), type_name, false)
}

/// Creates a new unnamed field.
Expand All @@ -135,7 +149,7 @@ impl Field {
where
T: TypeInfo + ?Sized + 'static,
{
Self::new(None, MetaType::new::<T>(), type_name)
Self::new(None, MetaType::new::<T>(), type_name, false)
}
}

Expand All @@ -161,4 +175,10 @@ where
pub fn type_name(&self) -> &T::String {
&self.type_name
}

/// Set the `compact` property to true, signalling that this type is to be
/// encoded/decoded as a [`parity_scale_codec::Compact`].
pub fn compact(&mut self) {
self.compact = true;
}
}
55 changes: 53 additions & 2 deletions test_suite/tests/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@
// limitations under the License.
#![cfg_attr(not(feature = "std"), no_std)]

use scale_info::prelude::boxed::Box;

use pretty_assertions::assert_eq;
use scale::Encode;
use scale_info::{
build::*,
prelude::boxed::Box,
tuple_meta_type,
Path,
Type,
Expand Down Expand Up @@ -204,6 +204,57 @@ fn associated_types_derive_without_bounds() {
assert_type!(Assoc<ConcreteTypes>, struct_type);
}

#[test]
fn scale_compact_types_work_in_structs() {
#[allow(unused)]
#[derive(Encode, TypeInfo)]
struct Dense {
a: u8,
#[codec(compact)]
b: u16,
}

let dense = Type::builder()
.path(Path::new("Dense", "derive"))
.composite(
Fields::named()
.field_of::<u8>("a", "u8")
.field_of::<u16>("b", "u16")
.compact(),
);

assert_type!(Dense, dense);
}

#[test]
fn scale_compact_types_work_in_enums() {
#[allow(unused)]
#[derive(Encode, TypeInfo)]
enum MutilatedMultiAddress<AccountId, AccountIndex> {
Id(AccountId),
Index(#[codec(compact)] AccountIndex),
Address32([u8; 32]),
}

let ty = Type::builder()
.path(Path::new("MutilatedMultiAddress", "derive"))
.type_params(tuple_meta_type!(u8, u16))
.variant(
Variants::with_fields()
.variant("Id", Fields::unnamed().field_of::<u8>("AccountId"))
.variant(
"Index",
Fields::unnamed().field_of::<u16>("AccountIndex").compact(),
)
.variant(
"Address32",
Fields::unnamed().field_of::<[u8; 32]>("[u8; 32]"),
),
);

assert_type!(MutilatedMultiAddress<u8, u16>, ty);
}

#[rustversion::nightly]
#[test]
fn ui_tests() {
Expand Down
48 changes: 48 additions & 0 deletions test_suite/tests/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,54 @@ fn test_struct() {
}));
}

#[test]
fn test_struct_with_some_fields_marked_as_compact() {
use scale::Encode;

// #[derive(TypeInfo, Encode)]
#[derive(Encode)]
struct Dense {
#[codec(compact)]
a: u128,
b: [u8; 32],
#[codec(compact)]
c: u64,
}
use scale_info::{
build::Fields,
Path,
Type,
};
impl TypeInfo for Dense {
type Identity = Self;
fn type_info() -> Type {
Type::builder()
.path(Path::new("Dense", module_path!()))
.composite(
Fields::named()
.field_of::<u8>("a", "i32")
.compact()
.field_of::<[u8; 32]>("b", "[u8; 32]")
.field_of::<u64>("c", "u64")
.compact(),
)
}
}

assert_json_for_type::<Dense>(json![{
"path": ["json", "Dense"],
"def": {
"composite": {
"fields": [
{ "name": "a", "type": 1, "typeName": "i32", "compact": true },
{ "name": "b", "type": 2, "typeName": "[u8; 32]" },
{ "name": "c", "type": 3, "typeName": "u64", "compact": true },
],
},
}
}]);
}

#[test]
fn test_clike_enum() {
#[derive(TypeInfo)]
Expand Down