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

staging-xcm: Use versioned_type! for VersionedXcm #4378

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
210 changes: 91 additions & 119 deletions polkadot/xcm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,16 @@ pub trait TryAs<T> {
// Macro that generated versioned wrapper types.
// NOTE: converting a v4 type into a versioned type will make it v5.
macro_rules! versioned_type {
($(#[$attr:meta])* pub enum $n:ident {
(@internal $n:ident, $v3:ty, $v4:ty, $v5:ty,) => {
impl MaxEncodedLen for $n {
fn max_encoded_len() -> usize {
<$v3>::max_encoded_len().max(<$v4>::max_encoded_len().max(<$v5>::max_encoded_len()))
}
}
};
(@internal $n:ident, $v3:ty, $v4:ty, $v5:ty, $t:ident) => {
Comment on lines +83 to +90
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
(@internal $n:ident, $v3:ty, $v4:ty, $v5:ty,) => {
impl MaxEncodedLen for $n {
fn max_encoded_len() -> usize {
<$v3>::max_encoded_len().max(<$v4>::max_encoded_len().max(<$v5>::max_encoded_len()))
}
}
};
(@internal $n:ident, $v3:ty, $v4:ty, $v5:ty, $t:ident) => {
(@maybe_max_encoded_len $n:ident, $v3:ty, $v4:ty, $v5:ty,) => {
impl MaxEncodedLen for $n {
fn max_encoded_len() -> usize {
<$v3>::max_encoded_len().max(<$v4>::max_encoded_len().max(<$v5>::max_encoded_len()))
}
}
};
(@maybe_max_encoded_len $n:ident, $v3:ty, $v4:ty, $v5:ty, $t:ident) => {

};
($(#[$attr:meta])* pub enum $n:ident$(<$($gen:ident),*>)? {
$(#[$index3:meta])+
V3($v3:ty),
$(#[$index4:meta])+
Expand All @@ -97,68 +106,69 @@ macro_rules! versioned_type {
)]
#[codec(encode_bound())]
#[codec(decode_bound())]
$(#[scale_info(bounds(), skip_type_params($($gen)+))])?
#[scale_info(replace_segment("staging_xcm", "xcm"))]
$(#[$attr])*
pub enum $n {
pub enum $n<$($($gen),*)?> {
$(#[$index3])*
V3($v3),
$(#[$index4])*
V4($v4),
$(#[$index5])*
V5($v5),
}
impl $n {
impl<$($($gen),*)?> $n<$($($gen),*)?>
{
pub fn try_as<T>(&self) -> Result<&T, ()> where Self: TryAs<T> {
<Self as TryAs<T>>::try_as(&self)
}
}
impl TryAs<$v3> for $n {
impl<$($($gen),*)?> TryAs<$v3> for $n <$($($gen),*)?>
{
fn try_as(&self) -> Result<&$v3, ()> {
match &self {
Self::V3(ref x) => Ok(x),
_ => Err(()),
}
}
}
impl TryAs<$v4> for $n {
impl<$($($gen),*)?> TryAs<$v4> for $n<$($($gen),*)?>
{
fn try_as(&self) -> Result<&$v4, ()> {
match &self {
Self::V4(ref x) => Ok(x),
_ => Err(()),
}
}
}
impl TryAs<$v5> for $n {
impl<$($($gen),*)?> TryAs<$v5> for $n<$($($gen),*)?>
{
fn try_as(&self) -> Result<&$v5, ()> {
match &self {
Self::V5(ref x) => Ok(x),
_ => Err(()),
}
}
}
impl IntoVersion for $n {
fn into_version(self, n: Version) -> Result<Self, ()> {
Ok(match n {
3 => Self::V3(self.try_into()?),
4 => Self::V4(self.try_into()?),
5 => Self::V5(self.try_into()?),
_ => return Err(()),
})
}
}
impl From<$v3> for $n {
impl<$($($gen),*)?> From<$v3> for $n <$($($gen),*)?>
{
fn from(x: $v3) -> Self {
$n::V3(x.into())
}
}
impl<T: Into<$v5>> From<T> for $n {
impl<$($($gen,),+)? T: Into<$v5>> From<T> for $n<$($($gen),*)?>
{
fn from(x: T) -> Self {
$n::V5(x.into())
}
}
impl TryFrom<$n> for $v3 {
impl<$($($gen),*)?> TryFrom<$n<$($($gen),*)?>> for $v3
$(
where $($gen: Decode + GetDispatchInfo),*
)?
{
type Error = ();
fn try_from(x: $n) -> Result<Self, ()> {
fn try_from(x: $n <$($($gen),*)?>) -> Result<Self, ()> {
use $n::*;
match x {
V3(x) => Ok(x),
Expand All @@ -170,9 +180,13 @@ macro_rules! versioned_type {
}
}
}
impl TryFrom<$n> for $v4 {
impl<$($($gen),*)?> TryFrom<$n <$($($gen),*)?>> for $v4
$(
where $($gen: Decode + GetDispatchInfo),*
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is hard coded only for VersionedXcm, since otherwise TryFrom<v4::Xcm>: v5::Xcm is not satisfied

)?
{
type Error = ();
fn try_from(x: $n) -> Result<Self, ()> {
fn try_from(x: $n<$($($gen),*)?>) -> Result<Self, ()> {
use $n::*;
match x {
V3(x) => x.try_into().map_err(|_| ()),
Expand All @@ -181,9 +195,10 @@ macro_rules! versioned_type {
}
}
}
impl TryFrom<$n> for $v5 {
impl<$($($gen),*)?> TryFrom<$n<$($($gen),*)?>> for $v5
{
type Error = ();
fn try_from(x: $n) -> Result<Self, ()> {
fn try_from(x: $n<$($($gen),*)?>) -> Result<Self, ()> {
use $n::*;
match x {
V3(x) => {
Expand All @@ -195,12 +210,26 @@ macro_rules! versioned_type {
}
}
}
impl MaxEncodedLen for $n {
fn max_encoded_len() -> usize {
<$v3>::max_encoded_len()
// internal macro that handles some edge cases
versioned_type!(@internal $n, $v3, $v4, $v5, $($($gen),+)?);
Comment on lines +213 to +214
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't help readability that having a generic implies not implementing MaxEncodedLen.

You could also match VersionedXcm specifically and not implement MaxEncodedLen in this case. Or have another optional input in the macro: @do_not_derive_max_encoded_len.

Suggested change
// internal macro that handles some edge cases
versioned_type!(@internal $n, $v3, $v4, $v5, $($($gen),+)?);
// We don't derive `MaxEncodedLen` for type with generic.
versioned_type!(@maybe_max_encoded_len $n, $v3, $v4, $v5, $($($gen),+)?);


impl<$($($gen),*)?> IntoVersion for $n<$($($gen),*)?>
$(
where $($gen: Decode + GetDispatchInfo),*
)?
{
fn into_version(self, n: Version) -> Result<Self, ()> {
Ok(match n {
3 => Self::V3(self.try_into()?),
4 => Self::V4(self.try_into()?),
5 => Self::V5(self.try_into()?),
_ => return Err(()),
})
}
}
impl IdentifyVersion for $n {

impl<$($($gen),*)?> IdentifyVersion for $n<$($($gen),*)?>
{
fn identify_version(&self) -> Version {
use $n::*;
match self {
Expand Down Expand Up @@ -310,40 +339,21 @@ versioned_type! {
}
}

/// A single XCM message, together with its version code.
#[derive(Derivative, Encode, Decode, TypeInfo)]
#[derivative(Clone(bound = ""), Eq(bound = ""), PartialEq(bound = ""), Debug(bound = ""))]
#[codec(encode_bound())]
#[codec(decode_bound())]
#[scale_info(bounds(), skip_type_params(RuntimeCall))]
#[scale_info(replace_segment("staging_xcm", "xcm"))]
pub enum VersionedXcm<RuntimeCall> {
#[codec(index = 3)]
V3(v3::Xcm<RuntimeCall>),
#[codec(index = 4)]
V4(v4::Xcm<RuntimeCall>),
#[codec(index = 5)]
V5(v5::Xcm<RuntimeCall>),
}

impl<C: Decode + GetDispatchInfo> IntoVersion for VersionedXcm<C> {
fn into_version(self, n: Version) -> Result<Self, ()> {
Ok(match n {
3 => Self::V3(self.try_into()?),
4 => Self::V4(self.try_into()?),
5 => Self::V5(self.try_into()?),
_ => return Err(()),
})
versioned_type! {
pub enum VersionedXcm<RuntimeCall> {
#[codec(index = 3)]
V3(v3::Xcm<RuntimeCall>),
#[codec(index = 4)]
V4(v4::Xcm<RuntimeCall>),
#[codec(index = 5)]
V5(v5::Xcm<RuntimeCall>),
}
}

impl<C> IdentifyVersion for VersionedXcm<C> {
fn identify_version(&self) -> Version {
match self {
Self::V3(_) => v3::VERSION,
Self::V4(_) => v4::VERSION,
Self::V5(_) => v5::VERSION,
}
/// TODO: workaround because `From<v4::Xcm>: v5::Xcm> is not satisfied
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure there is any

Suggested change
/// TODO: workaround because `From<v4::Xcm>: v5::Xcm> is not satisfied
/// `versioned_type` doesn't implement this, so we implement manually.

impl<RuntimeCall> From<v4::Xcm<RuntimeCall>> for VersionedXcm<RuntimeCall> {
fn from(x: v4::Xcm<RuntimeCall>) -> Self {
VersionedXcm::V4(x)
}
}

Expand All @@ -364,66 +374,6 @@ impl<C> VersionedXcm<C> {
}
}

impl<RuntimeCall> From<v3::Xcm<RuntimeCall>> for VersionedXcm<RuntimeCall> {
fn from(x: v3::Xcm<RuntimeCall>) -> Self {
VersionedXcm::V3(x)
}
}

impl<RuntimeCall> From<v4::Xcm<RuntimeCall>> for VersionedXcm<RuntimeCall> {
fn from(x: v4::Xcm<RuntimeCall>) -> Self {
VersionedXcm::V4(x)
}
}

impl<RuntimeCall> From<v5::Xcm<RuntimeCall>> for VersionedXcm<RuntimeCall> {
fn from(x: v5::Xcm<RuntimeCall>) -> Self {
VersionedXcm::V5(x)
}
}

impl<Call: Decode + GetDispatchInfo> TryFrom<VersionedXcm<Call>> for v3::Xcm<Call> {
type Error = ();
fn try_from(x: VersionedXcm<Call>) -> Result<Self, ()> {
use VersionedXcm::*;
match x {
V3(x) => Ok(x),
V4(x) => x.try_into(),
V5(x) => {
let v4: v4::Xcm<Call> = x.try_into()?;
v4.try_into()
},
}
}
}

impl<Call: Decode + GetDispatchInfo> TryFrom<VersionedXcm<Call>> for v4::Xcm<Call> {
type Error = ();
fn try_from(x: VersionedXcm<Call>) -> Result<Self, ()> {
use VersionedXcm::*;
match x {
V3(x) => x.try_into(),
V4(x) => Ok(x),
V5(x) => x.try_into(),
}
}
}

impl<Call: Decode + GetDispatchInfo> TryFrom<VersionedXcm<Call>> for v5::Xcm<Call> {
type Error = ();
fn try_from(x: VersionedXcm<Call>) -> Result<Self, ()> {
use VersionedXcm::*;
match x {
V3(x) => {
let v4: v4::Xcm<Call> = x.try_into()?;
v4.try_into()
},
V4(x) => x.try_into(),
V5(x) => Ok(x),
}
}
}

/// Convert an `Xcm` datum into a `VersionedXcm`, based on a destination `Location` which will
/// interpret it.
pub trait WrapVersion {
Expand Down Expand Up @@ -686,3 +636,25 @@ fn validate_xcm_nesting_works() {
.validate_xcm_nesting()
.is_err());
}

#[test]
fn test_versioned_xcm() {
let v3_xcm = VersionedXcm::V3(v3::Xcm(vec![]));
let v4_xcm = VersionedXcm::V4(v4::Xcm(vec![]));
let v5_xcm = VersionedXcm::V5(v5::Xcm(vec![]));

// Test conversion between versions
let v3_to_v4: Result<v4::Xcm<()>, ()> = v3_xcm.clone().try_into();
assert!(v3_to_v4.is_ok());

let v4_to_v5: Result<v5::Xcm<()>, ()> = v4_xcm.clone().try_into();
assert!(v4_to_v5.is_ok());

let v5_to_v3: Result<v3::Xcm<()>, ()> = v5_xcm.clone().try_into();
assert!(v5_to_v3.is_ok());

// Test identify_version
assert_eq!(v3_xcm.identify_version(), v3::VERSION);
assert_eq!(v4_xcm.identify_version(), v4::VERSION);
assert_eq!(v5_xcm.identify_version(), v5::VERSION);
}
15 changes: 15 additions & 0 deletions prdoc/pr_4378.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: "Use `versioned_type!` macro for `VersionedXcm`"

doc:
- audience: Runtime Dev
description: |
Currently, all other versioned types in the `staging-xcm` are created using `versioned_type!` macro, except for `VersionedXcm`.
This PR changes `versioned_type` macro so that it can be used for `VersionedXcm` as well, reducing a lot of duplication. This is done by adding optional
generic type param to an enum that is passed to the macro.

crates:
- name: staging-xcm
bump: patch
Loading