Skip to content

Commit

Permalink
XCM builder pattern improvement - Accept impl Into<T> instead of ju…
Browse files Browse the repository at this point in the history
…st `T` (paritytech#3708)

The XCM builder pattern lets you build xcms like so:

```rust
let xcm = Xcm::builder()
    .withdraw_asset((Parent, 100u128).into())
    .buy_execution((Parent, 1u128).into())
    .deposit_asset(All.into(), AccountId32 { id: [0u8; 32], network: None }.into())
    .build();
```

All the `.into()` become quite annoying to have to write.
I accepted `impl Into<T>` instead of `T` in the generated methods from
the macro.
Now the previous example can be simplified as follows:

```rust
let xcm = Xcm::builder()
    .withdraw_asset((Parent, 100u128))
    .buy_execution((Parent, 1u128))
    .deposit_asset(All, [0u8; 32])
    .build();
```

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: command-bot <>
Co-authored-by: Adrian Catangiu <adrian@parity.io>
  • Loading branch information
3 people authored and dharjeezy committed Apr 9, 2024
1 parent da69de1 commit ecdab48
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 16 deletions.
5 changes: 2 additions & 3 deletions polkadot/xcm/pallet-xcm/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -486,9 +486,8 @@ fn claim_assets_works() {
let balances = vec![(ALICE, INITIAL_BALANCE)];
new_test_ext_with_balances(balances).execute_with(|| {
// First trap some assets.
let trapping_program = Xcm::<RuntimeCall>::builder_unsafe()
.withdraw_asset((Here, SEND_AMOUNT).into())
.build();
let trapping_program =
Xcm::<RuntimeCall>::builder_unsafe().withdraw_asset((Here, SEND_AMOUNT)).build();
// Even though assets are trapped, the extrinsic returns success.
assert_ok!(XcmPallet::execute_blob(
RuntimeOrigin::signed(ALICE),
Expand Down
18 changes: 12 additions & 6 deletions polkadot/xcm/procedural/src/builder_pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ fn generate_builder_raw_impl(name: &Ident, data_enum: &DataEnum) -> TokenStream2
.collect();
let arg_types: Vec<_> = fields.unnamed.iter().map(|field| &field.ty).collect();
quote! {
pub fn #method_name(mut self, #(#arg_names: #arg_types),*) -> Self {
pub fn #method_name(mut self, #(#arg_names: impl Into<#arg_types>),*) -> Self {
#(let #arg_names = #arg_names.into();)*
self.instructions.push(#name::<Call>::#variant_name(#(#arg_names),*));
self
}
Expand All @@ -117,7 +118,8 @@ fn generate_builder_raw_impl(name: &Ident, data_enum: &DataEnum) -> TokenStream2
let arg_names: Vec<_> = fields.named.iter().map(|field| &field.ident).collect();
let arg_types: Vec<_> = fields.named.iter().map(|field| &field.ty).collect();
quote! {
pub fn #method_name(mut self, #(#arg_names: #arg_types),*) -> Self {
pub fn #method_name(mut self, #(#arg_names: impl Into<#arg_types>),*) -> Self {
#(let #arg_names = #arg_names.into();)*
self.instructions.push(#name::<Call>::#variant_name { #(#arg_names),* });
self
}
Expand Down Expand Up @@ -188,8 +190,9 @@ fn generate_builder_impl(name: &Ident, data_enum: &DataEnum) -> Result<TokenStre
let arg_types: Vec<_> = fields.unnamed.iter().map(|field| &field.ty).collect();
quote! {
#(#docs)*
pub fn #method_name(self, #(#arg_names: #arg_types),*) -> XcmBuilder<Call, LoadedHolding> {
pub fn #method_name(self, #(#arg_names: impl Into<#arg_types>),*) -> XcmBuilder<Call, LoadedHolding> {
let mut new_instructions = self.instructions;
#(let #arg_names = #arg_names.into();)*
new_instructions.push(#name::<Call>::#variant_name(#(#arg_names),*));
XcmBuilder {
instructions: new_instructions,
Expand All @@ -203,8 +206,9 @@ fn generate_builder_impl(name: &Ident, data_enum: &DataEnum) -> Result<TokenStre
let arg_types: Vec<_> = fields.named.iter().map(|field| &field.ty).collect();
quote! {
#(#docs)*
pub fn #method_name(self, #(#arg_names: #arg_types),*) -> XcmBuilder<Call, LoadedHolding> {
pub fn #method_name(self, #(#arg_names: impl Into<#arg_types>),*) -> XcmBuilder<Call, LoadedHolding> {
let mut new_instructions = self.instructions;
#(let #arg_names = #arg_names.into();)*
new_instructions.push(#name::<Call>::#variant_name { #(#arg_names),* });
XcmBuilder {
instructions: new_instructions,
Expand Down Expand Up @@ -249,8 +253,9 @@ fn generate_builder_impl(name: &Ident, data_enum: &DataEnum) -> Result<TokenStre
fields.named.iter().map(|field| &field.ty).collect();
quote! {
#(#docs)*
pub fn #method_name(self, #(#arg_names: #arg_types),*) -> XcmBuilder<Call, AnythingGoes> {
pub fn #method_name(self, #(#arg_names: impl Into<#arg_types>),*) -> XcmBuilder<Call, AnythingGoes> {
let mut new_instructions = self.instructions;
#(let #arg_names = #arg_names.into();)*
new_instructions.push(#name::<Call>::#variant_name { #(#arg_names),* });
XcmBuilder {
instructions: new_instructions,
Expand Down Expand Up @@ -308,8 +313,9 @@ fn generate_builder_unpaid_impl(name: &Ident, data_enum: &DataEnum) -> Result<To
Ok(quote! {
impl<Call> XcmBuilder<Call, ExplicitUnpaidRequired> {
#(#docs)*
pub fn #unpaid_execution_method_name(self, #(#arg_names: #arg_types),*) -> XcmBuilder<Call, AnythingGoes> {
pub fn #unpaid_execution_method_name(self, #(#arg_names: impl Into<#arg_types>),*) -> XcmBuilder<Call, AnythingGoes> {
let mut new_instructions = self.instructions;
#(let #arg_names = #arg_names.into();)*
new_instructions.push(#name::<Call>::#unpaid_execution_ident { #(#arg_names),* });
XcmBuilder {
instructions: new_instructions,
Expand Down
14 changes: 7 additions & 7 deletions polkadot/xcm/procedural/tests/builder_pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ use xcm::latest::prelude::*;
#[test]
fn builder_pattern_works() {
let asset: Asset = (Here, 100u128).into();
let beneficiary: Location = AccountId32 { id: [0u8; 32], network: None }.into();
let beneficiary: Location = [0u8; 32].into();
let message: Xcm<()> = Xcm::builder()
.receive_teleported_asset(asset.clone().into())
.receive_teleported_asset(asset.clone())
.buy_execution(asset.clone(), Unlimited)
.deposit_asset(asset.clone().into(), beneficiary.clone())
.deposit_asset(asset.clone(), beneficiary.clone())
.build();
assert_eq!(
message,
Expand All @@ -53,8 +53,8 @@ fn default_builder_requires_buy_execution() {
// To be able to do that, we need to use the explicitly unpaid variant
let message: Xcm<()> = Xcm::builder_unpaid()
.unpaid_execution(Unlimited, None)
.withdraw_asset(asset.clone().into())
.deposit_asset(asset.clone().into(), beneficiary.clone())
.withdraw_asset(asset.clone())
.deposit_asset(asset.clone(), beneficiary.clone())
.build(); // This works
assert_eq!(
message,
Expand All @@ -68,8 +68,8 @@ fn default_builder_requires_buy_execution() {
// The other option doesn't have any limits whatsoever, so it should
// only be used when you really know what you're doing.
let message: Xcm<()> = Xcm::builder_unsafe()
.withdraw_asset(asset.clone().into())
.deposit_asset(asset.clone().into(), beneficiary.clone())
.withdraw_asset(asset.clone())
.deposit_asset(asset.clone(), beneficiary.clone())
.build();
assert_eq!(
message,
Expand Down
7 changes: 7 additions & 0 deletions polkadot/xcm/src/v4/location.rs
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,13 @@ impl<Interior: Into<Junctions>> From<AncestorThen<Interior>> for Location {
}
}

impl From<[u8; 32]> for Location {
fn from(bytes: [u8; 32]) -> Self {
let junction: Junction = bytes.into();
junction.into()
}
}

xcm_procedural::impl_conversion_functions_for_location_v4!();

#[cfg(test)]
Expand Down
31 changes: 31 additions & 0 deletions prdoc/pr_3708.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# 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: XCM builder pattern automatically converts instruction parameters.

doc:
- audience: Runtime Dev
description: |
Small quality of life improvement.
Previously, an XCM could be built like this:
```rust
let xcm = Xcm::builder()
.withdraw_asset((Parent, 100u128).into())
.buy_execution((Parent, 1u128).into())
.deposit_asset(All.into(), AccountId32 { id: [0u8; 32], network: None }.into())
.build();
```
Now, it can be built like this:
```rust
let xcm = Xcm::builder()
.withdraw_asset((Parent, 100u128))
.buy_execution((Parent, 1u128))
.deposit_asset(All, [0u8; 32])
.build();
```

crates:
- name: "xcm-procedural"
bump: minor
- name: "staging-xcm"
bump: minor

0 comments on commit ecdab48

Please sign in to comment.