From cb45b9e182aca7c8ca46854fbef089df13539c4e Mon Sep 17 00:00:00 2001 From: Francisco Aguirre Date: Wed, 29 May 2024 20:57:17 +0100 Subject: [PATCH] Change `XcmDryRunApi::dry_run_extrinsic` to take a call instead (#4621) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the new `XcmDryRunApi` runtime API introduced in https://github.com/paritytech/polkadot-sdk/pull/3872. Taking an extrinsic means the frontend has to sign first to dry-run and once again to submit. This is bad UX which is solved by taking an `origin` and a `call`. This also has the benefit of being able to dry-run as any account, since it needs no signature. This is a breaking change since I changed `dry_run_extrinsic` to `dry_run_call`, however, this API is still only on testnets. The crates are bumped accordingly. As a part of this PR, I changed the name of the API from `XcmDryRunApi` to just `DryRunApi`, since it can be used for general dry-running :) Step towards https://github.com/paritytech/polkadot-sdk/issues/690. Example of calling the API with PAPI, not the best code, just testing :) ```ts // We just build a call, the arguments make it look very big though. const call = localApi.tx.XcmPallet.transfer_assets({ dest: XcmVersionedLocation.V4({ parents: 0, interior: XcmV4Junctions.X1(XcmV4Junction.Parachain(1000)) }), beneficiary: XcmVersionedLocation.V4({ parents: 0, interior: XcmV4Junctions.X1(XcmV4Junction.AccountId32({ network: undefined, id: Binary.fromBytes(encodeAccount(account.address)) })) }), weight_limit: XcmV3WeightLimit.Unlimited(), assets: XcmVersionedAssets.V4([{ id: { parents: 0, interior: XcmV4Junctions.Here() }, fun: XcmV3MultiassetFungibility.Fungible(1_000_000_000_000n) } ]), fee_asset_item: 0, }); // We call the API passing in a signed origin const result = await localApi.apis.XcmDryRunApi.dry_run_call( WestendRuntimeOriginCaller.system(DispatchRawOrigin.Signed(account.address)), call.decodedCall ); if (result.success && result.value.execution_result.success) { // We find the forwarded XCM we want. The first one going to AssetHub in this case. const xcmsToAssetHub = result.value.forwarded_xcms.find(([location, _]) => ( location.type === "V4" && location.value.parents === 0 && location.value.interior.type === "X1" && location.value.interior.value.type === "Parachain" && location.value.interior.value.value === 1000 ))!; // We can even find the delivery fees for that forwarded XCM. const deliveryFeesQuery = await localApi.apis.XcmPaymentApi.query_delivery_fees(xcmsToAssetHub[0], xcmsToAssetHub[1][0]); if (deliveryFeesQuery.success) { const amount = deliveryFeesQuery.value.type === "V4" && deliveryFeesQuery.value.value[0].fun.type === "Fungible" && deliveryFeesQuery.value.value[0].fun.value.valueOf() || 0n; // We store them in state somewhere. setDeliveryFees(formatAmount(BigInt(amount))); } } ``` --------- Co-authored-by: Bastian Köcher --- polkadot/xcm/pallet-xcm/src/lib.rs | 71 ++++++++++++++++-------------- 1 file changed, 39 insertions(+), 32 deletions(-) diff --git a/polkadot/xcm/pallet-xcm/src/lib.rs b/polkadot/xcm/pallet-xcm/src/lib.rs index 8f67e6e7d9496..913199f7e0919 100644 --- a/polkadot/xcm/pallet-xcm/src/lib.rs +++ b/polkadot/xcm/pallet-xcm/src/lib.rs @@ -836,7 +836,7 @@ pub mod pallet { if Self::request_version_notify(dest).is_ok() { // TODO: correct weights. weight_used.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); - break + break; } } } @@ -884,8 +884,9 @@ pub mod pallet { timeout, maybe_match_querier: Some(Location::here().into()), }, - VersionNotifier { origin, is_active } => - QueryStatus::VersionNotifier { origin, is_active }, + VersionNotifier { origin, is_active } => { + QueryStatus::VersionNotifier { origin, is_active } + }, Ready { response, at } => QueryStatus::Ready { response, at }, } } @@ -1689,8 +1690,9 @@ impl Pallet { fees, weight_limit, )?, - TransferType::RemoteReserve(_) => - return Err(Error::::InvalidAssetUnsupportedReserve.into()), + TransferType::RemoteReserve(_) => { + return Err(Error::::InvalidAssetUnsupportedReserve.into()) + }, }; FeesHandling::Separate { local_xcm, remote_xcm } }; @@ -2239,7 +2241,7 @@ impl Pallet { } weight_used.saturating_accrue(sv_migrate_weight); if weight_used.any_gte(weight_cutoff) { - return (weight_used, Some(stage)) + return (weight_used, Some(stage)); } } } @@ -2253,7 +2255,7 @@ impl Pallet { } weight_used.saturating_accrue(vn_migrate_weight); if weight_used.any_gte(weight_cutoff) { - return (weight_used, Some(stage)) + return (weight_used, Some(stage)); } } } @@ -2275,7 +2277,7 @@ impl Pallet { // We don't early return here since we need to be certain that we // make some progress. weight_used.saturating_accrue(vnt_already_notified_weight); - continue + continue; }, }; let response = Response::Version(xcm_version); @@ -2301,7 +2303,7 @@ impl Pallet { weight_used.saturating_accrue(vnt_notify_weight); if weight_used.any_gte(weight_cutoff) { let last = Some(iter.last_raw_key().into()); - return (weight_used, Some(NotifyCurrentTargets(last))) + return (weight_used, Some(NotifyCurrentTargets(last))); } } stage = MigrateAndNotifyOldTargets; @@ -2319,9 +2321,9 @@ impl Pallet { }); weight_used.saturating_accrue(vnt_migrate_fail_weight); if weight_used.any_gte(weight_cutoff) { - return (weight_used, Some(stage)) + return (weight_used, Some(stage)); } - continue + continue; }, }; @@ -2362,7 +2364,7 @@ impl Pallet { weight_used.saturating_accrue(vnt_notify_migrate_weight); } if weight_used.any_gte(weight_cutoff) { - return (weight_used, Some(stage)) + return (weight_used, Some(stage)); } } } @@ -2688,7 +2690,7 @@ impl Pallet { // if migration has been already scheduled, everything is ok and data will be eventually // migrated if CurrentMigration::::exists() { - return Ok(()) + return Ok(()); } // if migration has NOT been scheduled yet, we need to check all operational data @@ -2985,7 +2987,7 @@ impl VersionChangeNotifier for Pallet { impl DropAssets for Pallet { fn drop_assets(origin: &Location, assets: AssetsInHolding, _context: &XcmContext) -> Weight { if assets.is_empty() { - return Weight::zero() + return Weight::zero(); } let versioned = VersionedAssets::from(Assets::from(assets)); let hash = BlakeTwo256::hash_of(&(&origin, &versioned)); @@ -3009,11 +3011,12 @@ impl ClaimAssets for Pallet { ) -> bool { let mut versioned = VersionedAssets::from(assets.clone()); match ticket.unpack() { - (0, [GeneralIndex(i)]) => + (0, [GeneralIndex(i)]) => { versioned = match versioned.into_version(*i as u32) { Ok(v) => v, Err(()) => return false, - }, + } + }, (0, []) => (), _ => return false, }; @@ -3028,7 +3031,7 @@ impl ClaimAssets for Pallet { origin: origin.clone(), assets: versioned, }); - return true + return true; } } @@ -3039,15 +3042,17 @@ impl OnResponse for Pallet { querier: Option<&Location>, ) -> bool { match Queries::::get(query_id) { - Some(QueryStatus::Pending { responder, maybe_match_querier, .. }) => - Location::try_from(responder).map_or(false, |r| origin == &r) && - maybe_match_querier.map_or(true, |match_querier| { + Some(QueryStatus::Pending { responder, maybe_match_querier, .. }) => { + Location::try_from(responder).map_or(false, |r| origin == &r) + && maybe_match_querier.map_or(true, |match_querier| { Location::try_from(match_querier).map_or(false, |match_querier| { querier.map_or(false, |q| q == &match_querier) }) - }), - Some(QueryStatus::VersionNotifier { origin: r, .. }) => - Location::try_from(r).map_or(false, |r| origin == &r), + }) + }, + Some(QueryStatus::VersionNotifier { origin: r, .. }) => { + Location::try_from(r).map_or(false, |r| origin == &r) + }, _ => false, } } @@ -3074,7 +3079,7 @@ impl OnResponse for Pallet { query_id, expected_location: Some(o), }); - return Weight::zero() + return Weight::zero(); }, _ => { Self::deposit_event(Event::InvalidResponder { @@ -3083,7 +3088,7 @@ impl OnResponse for Pallet { expected_location: None, }); // TODO #3735: Correct weight for this. - return Weight::zero() + return Weight::zero(); }, }; // TODO #3735: Check max_weight is correct. @@ -3116,7 +3121,7 @@ impl OnResponse for Pallet { origin: origin.clone(), query_id, }); - return Weight::zero() + return Weight::zero(); }, }; if querier.map_or(true, |q| q != &match_querier) { @@ -3126,7 +3131,7 @@ impl OnResponse for Pallet { expected_querier: match_querier, maybe_actual_querier: querier.cloned(), }); - return Weight::zero() + return Weight::zero(); } } let responder = match Location::try_from(responder) { @@ -3136,7 +3141,7 @@ impl OnResponse for Pallet { origin: origin.clone(), query_id, }); - return Weight::zero() + return Weight::zero(); }, }; if origin != responder { @@ -3145,7 +3150,7 @@ impl OnResponse for Pallet { query_id, expected_location: Some(responder), }); - return Weight::zero() + return Weight::zero(); } return match maybe_notify { Some((pallet_index, call_index)) => { @@ -3167,7 +3172,7 @@ impl OnResponse for Pallet { max_budgeted_weight: max_weight, }; Self::deposit_event(e); - return Weight::zero() + return Weight::zero(); } let dispatch_origin = Origin::Response(origin.clone()).into(); match call.dispatch(dispatch_origin) { @@ -3204,7 +3209,7 @@ impl OnResponse for Pallet { Queries::::insert(query_id, QueryStatus::Ready { response, at }); Weight::zero() }, - } + }; }, _ => { let e = Event::UnexpectedResponse { origin: origin.clone(), query_id }; @@ -3313,7 +3318,9 @@ where caller.try_into().and_then(|o| match o { Origin::Xcm(ref location) if F::contains(&location.clone().try_into().map_err(|_| o.clone().into())?) => - Ok(location.clone().try_into().map_err(|_| o.clone().into())?), + { + Ok(location.clone().try_into().map_err(|_| o.clone().into())?) + }, Origin::Xcm(location) => Err(Origin::Xcm(location).into()), o => Err(o.into()), })