paritytech / parity-scale-codec

Lightweight, efficient, binary serialization and deserialization codec
Apache License 2.0
243 stars 95 forks source link

Custom types with `HasCompact` #613

Open lemunozm opened 1 month ago

lemunozm commented 1 month ago

Hi, I have a custom wrapper type that, in terms of encoding/decoding, should behave exactly as its underlying type:

type MyWrapper<T>;

I'm able to implement Encode/Decode successfully, nevertheless, if this type is used as follows:

struct Foo {
    #[codec(compact)]
    a: MyWrapper<u64>,
}

The compile throws an error. Basically, it requires that CompactRef<'a, MyWrapper<u64>> implements Encode/Decode. But I can not do that because both CompactRef and Encode/Decode are foreign to my crate. I can neither implement HasCompact directly because it's already implemented for T in the library, and to fulfill the conditions, I need the CompactRef implementation.

Is any way to create compactables custom types?

bkchr commented 1 month ago
    impl<T> CompactAs for MyWrapper<T> {
        type As = T;
        fn encode_as(&self) -> &Self::As {
            self.0
        }
        fn decode_from(v: Self::As) -> Result<Self, Error> {
            Ok(Self(v))
        }
    }

    impl<T> From<Compact<MyWrapper<T>>> for MyWrapper<T> {
        fn from(v: Compact<Self>) -> Self {
            Self(v.0)
        }
    }

This should do the trick

lemunozm commented 1 month ago

Thanks for your fast answer!

I've already tried that, but didn't work:

#[codec(compact)]
^ the trait `parity_scale_codec::Encode` is not implemented for `CompactRef<'_, num_wrapper::NumWrapper<u64, ()>>`, which is required by `parity_scale_codec::Compact<num_wrapper::NumWrapper<u64, ()>>: EncodeAsRef<'_, num_wrapper::NumWrapper<u64, ()>>`

Basically Encode is only implemented for Compact if already exist a CompactRef for that type that implements Encode, which is not the case for MyWrapper<T>, and can not be done in an external crate due to the foreign restrictions implementing foreign traits with foreign types.

gui1117 commented 1 month ago

Indeed CompactAs only support types which encode into/decode from a SCALE-encoded integer. it can add more constraint on the integer in the decode_from implementation (like non-zero types for example)

If we want to support any type to have a Compact encoding then maybe we should:

lemunozm commented 1 month ago

Thanks for come with a solution!

As a note, I've found this "wall" because BaseArithmetic trait from Substrate requires a HasCompact trait. Not sure if any other user would want to create custom compact types for other reasons.

So maybe, for those who came here for the same issue as me, the easiest way to pass through is allowing non-compact types in the BaseArithmetic trait (just removing the requirement?)

The related issue regarding this: https://github.com/paritytech/polkadot-sdk/issues/5004

gui1117 commented 1 month ago

I see, Note that if you only want to have HasCompact for NumWrapper for concrete num types (NumWrapper<u8, I> to NumWrapper<u128, I>) then it is possible. Just not in a generic way.

use parity_scale_codec::*;

pub struct NumWrapper {
    inner_type: u64,
}

impl From<Compact<NumWrapper>> for NumWrapper {
    fn from(c: Compact<NumWrapper>) -> Self {
        c.0
    }
}

impl CompactAs for NumWrapper {
    type As = u64;

    fn encode_as(&self) -> &Self::As {
        &self.inner_type
    }
    fn decode_from(d: Self::As) -> Result<Self, Error> {
        // Allow to do some more check
        Ok(NumWrapper { inner_type: d })
    }
}

fn foo<T: HasCompact>(t: T) -> T {
    let e = <<T as HasCompact>::Type as EncodeAsRef<T>>::RefType::from(&t).encode();
    <<T as HasCompact>::Type as Decode>::decode(&mut &e[..]).unwrap().into()
}
lemunozm commented 1 month ago

Wow! I was always testing with a generic NumWrapper<T, I>, but it's true that for concrete types it works. Thanks for pointing out this!

Where exactly is the part that makes the generic wrapper impossible? I'm not able to see it in the crate. Where does this limitation come from?

lemunozm commented 1 month ago

Ok it seems that when I implement the CompactAs as follows:

impl<T: CompactAs> CompactAs for NumWrapper<T> {
..
}

the requirement of CompactRef appears, but if I write:

impl<T> CompactAs for NumWrapper<T> {
..
}

it compiles