Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Improve MultiLocation conversion functions in xcm-procedural #3690

Merged
3 commits merged into from
Aug 23, 2021
Merged

Improve MultiLocation conversion functions in xcm-procedural #3690

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
31a37be
Use fully-qualified paths to reference core traits in proc macro
KiChjang Aug 23, 2021
5478213
Ensure a canonicalized v0 MultiLocation first before attempting to co…
KiChjang Aug 23, 2021
9a3bf7e
Fix failing test
KiChjang Aug 23, 2021
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
2 changes: 1 addition & 1 deletion xcm/procedural/src/v0/multilocation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ fn generate_conversion_from_v1() -> TokenStream {
.collect::<TokenStream>();

quote! {
impl TryFrom<crate::v1::MultiLocation> for MultiLocation {
impl core::convert::TryFrom<crate::v1::MultiLocation> for MultiLocation {
type Error = ();
fn try_from(v1: crate::v1::MultiLocation) -> core::result::Result<Self, ()> {
let mut res = MultiLocation::Null;
Expand Down
10 changes: 6 additions & 4 deletions xcm/procedural/src/v1/multilocation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ fn generate_conversion_from_v0() -> TokenStream {
if #( #parent_idents.is_parent() )&&* =>
Ok(MultiLocation {
parents: #parent_count,
interior: #junction_variant( #( #junction_idents.try_into()? ),* ),
interior: #junction_variant( #( core::convert::TryInto::try_into(#junction_idents)? ),* ),
}),
}
})
Expand All @@ -184,16 +184,18 @@ fn generate_conversion_from_v0() -> TokenStream {
Ok(MultiLocation::ancestor(#num_ancestors)),
#intermediate_match_arms
crate::v0::MultiLocation::#variant( #(#idents),* ) =>
Ok( #variant( #( #idents.try_into()? ),* ).into() ),
Ok( #variant( #( core::convert::TryInto::try_into(#idents)? ),* ).into() ),
}
})
.collect::<TokenStream>();

quote! {
impl TryFrom<crate::v0::MultiLocation> for MultiLocation {
impl core::convert::TryFrom<crate::v0::MultiLocation> for MultiLocation {
type Error = ();
fn try_from(v0: crate::v0::MultiLocation) -> core::result::Result<Self, ()> {
fn try_from(mut v0: crate::v0::MultiLocation) -> core::result::Result<Self, ()> {
use Junctions::*;

v0.canonicalize();
Copy link
Copy Markdown
Member

@bkchr bkchr Aug 23, 2021

Choose a reason for hiding this comment

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

This wasn't called before?

Copy link
Copy Markdown
Contributor Author

@KiChjang KiChjang Aug 23, 2021

Choose a reason for hiding this comment

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

Yeah it wasn't, even before the migration to the proc macro.

Copy link
Copy Markdown
Member

@bkchr bkchr Aug 23, 2021

Choose a reason for hiding this comment

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

What I should have asked, do you added this by intention? :D

Copy link
Copy Markdown
Contributor Author

@KiChjang KiChjang Aug 23, 2021

Choose a reason for hiding this comment

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

Yes, I thought that the conversion should always try and canonicalize the MultiLocation first, since it's an infallible operation. However, it does change the internal representation of the MultiLocation irreversibly, but I think that's ok because the non-canonical representation would've been "erroneous" anyway.

match v0 {
crate::v0::MultiLocation::Null => Ok(Here.into()),
#match_variants
Expand Down
2 changes: 1 addition & 1 deletion xcm/src/v0/multi_location.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
//! Cross-Consensus Message format data structures.

use super::Junction;
use core::{convert::TryFrom, mem, result};
use core::{mem, result};
use parity_scale_codec::{self, Decode, Encode};

/// A relative path between state-bearing consensus systems.
Expand Down
5 changes: 1 addition & 4 deletions xcm/src/v1/multilocation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@
//! Cross-Consensus Message format data structures.

use super::Junction;
use core::{
convert::{TryFrom, TryInto},
mem, result,
};
use core::{convert::TryFrom, mem, result};
use parity_scale_codec::{Decode, Encode};

/// A relative path between state-bearing consensus systems.
Expand Down