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

Custom types with HasCompact #613

Open
lemunozm opened this issue Jul 11, 2024 · 7 comments
Open

Custom types with HasCompact #613

lemunozm opened this issue Jul 11, 2024 · 7 comments

Comments

@lemunozm
Copy link

lemunozm commented Jul 11, 2024

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
Copy link
Member

bkchr commented Jul 11, 2024

	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
Copy link
Author

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
Copy link
Contributor

gui1117 commented Jul 25, 2024

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:

  • introduce 2 new traits: CompactEncode and CompactDecode.
  • implement Decode and Encode for Compact<T> when T implements CompactEncode/CompactDecode
  • remove CompactAs trait, but we can probably provide an easy way forward with a macro or some functions
  • update the derive macro a bit to handle DeriveCompactAs without breaking change

@lemunozm
Copy link
Author

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: paritytech/polkadot-sdk#5004

@gui1117
Copy link
Contributor

gui1117 commented Jul 25, 2024

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
Copy link
Author

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants