Skip to content

Commit

Permalink
[xcm]: Use latest Versioned* instead of V4 + bridges doc nits (#4528
Browse files Browse the repository at this point in the history
)

Co-authored-by: Svyatoslav Nikolsky <[email protected]>
  • Loading branch information
bkontur and svyatonik authored May 21, 2024
1 parent 278486f commit b00e168
Show file tree
Hide file tree
Showing 13 changed files with 107 additions and 79 deletions.
19 changes: 8 additions & 11 deletions bridges/bin/runtime-common/src/messages_benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ pub fn generate_xcm_builder_bridge_message_sample(
move |expected_message_size| -> MessagePayload {
// For XCM bridge hubs, it is the message that
// will be pushed further to some XCM queue (XCMP/UMP)
let location = xcm::VersionedInteriorLocation::V4(destination.clone());
let location = xcm::VersionedInteriorLocation::from(destination.clone());
let location_encoded_size = location.encoded_size();

// we don't need to be super-precise with `expected_size` here
Expand All @@ -294,16 +294,13 @@ pub fn generate_xcm_builder_bridge_message_sample(
expected_message_size, location_encoded_size, xcm_size, xcm_data_size,
);

let xcm = xcm::VersionedXcm::<()>::V4(
vec![Instruction::<()>::ExpectPallet {
index: 0,
name: vec![42; xcm_data_size],
module_name: vec![],
crate_major: 0,
min_crate_minor: 0,
}]
.into(),
);
let xcm = xcm::VersionedXcm::<()>::from(Xcm(vec![Instruction::<()>::ExpectPallet {
index: 0,
name: vec![42; xcm_data_size],
module_name: vec![],
crate_major: 0,
min_crate_minor: 0,
}]));

// this is the `BridgeMessage` from polkadot xcm builder, but it has no constructor
// or public fields, so just tuple
Expand Down
2 changes: 1 addition & 1 deletion bridges/modules/beefy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ pub mod pallet {
/// Pallet owner has the right to halt all pallet operations and then resume it. If it is
/// `None`, then there are no direct ways to halt/resume pallet operations, but other
/// runtime methods may still be used to do that (i.e. `democracy::referendum` to update halt
/// flag directly or calling `halt_operations`).
/// flag directly or calling `set_operating_mode`).
#[pallet::storage]
pub type PalletOwner<T: Config<I>, I: 'static = ()> =
StorageValue<_, T::AccountId, OptionQuery>;
Expand Down
2 changes: 1 addition & 1 deletion bridges/modules/grandpa/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ pub mod pallet {
/// Pallet owner has a right to halt all pallet operations and then resume it. If it is
/// `None`, then there are no direct ways to halt/resume pallet operations, but other
/// runtime methods may still be used to do that (i.e. democracy::referendum to update halt
/// flag directly or call the `halt_operations`).
/// flag directly or call the `set_operating_mode`).
#[pallet::storage]
pub type PalletOwner<T: Config<I>, I: 'static = ()> =
StorageValue<_, T::AccountId, OptionQuery>;
Expand Down
12 changes: 7 additions & 5 deletions bridges/modules/messages/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,13 @@ There may be a special account in every runtime where the messages module is dep
owner', is like a module-level sudo account - he's able to halt and resume all module operations without requiring
runtime upgrade. Calls that are related to this account are:
- `fn set_owner()`: current module owner may call it to transfer "ownership" to another account;
- `fn halt_operations()`: the module owner (or sudo account) may call this function to stop all module operations. After
this call, all message-related transactions will be rejected until further `resume_operations` call'. This call may be
used when something extraordinary happens with the bridge;
- `fn resume_operations()`: module owner may call this function to resume bridge operations. The module will resume its
regular operations after this call.
- `fn set_operating_mode()`: the module owner (or sudo account) may call this function to pause/resume
pallet operations. Owner may halt the pallet by calling this method with
`MessagesOperatingMode::Basic(BasicOperatingMode::Halted)` argument - all message-related
transactions will be rejected. Owner may then resume pallet operations by passing the
`MessagesOperatingMode::Basic(BasicOperatingMode::Normal)` argument. There's also
`MessagesOperatingMode::RejectingOutboundMessages` pallet mode, where it still accepts all incoming
messages, but all outbound messages are rejected.

If pallet owner is not defined, the governance may be used to make those calls.

Expand Down
2 changes: 1 addition & 1 deletion bridges/modules/messages/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ pub mod pallet {
/// Pallet owner has a right to halt all pallet operations and then resume it. If it is
/// `None`, then there are no direct ways to halt/resume pallet operations, but other
/// runtime methods may still be used to do that (i.e. democracy::referendum to update halt
/// flag directly or call the `halt_operations`).
/// flag directly or call the `set_operating_mode`).
#[pallet::storage]
#[pallet::getter(fn module_owner)]
pub type PalletOwner<T: Config<I>, I: 'static = ()> = StorageValue<_, T::AccountId>;
Expand Down
2 changes: 1 addition & 1 deletion bridges/modules/parachains/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ pub mod pallet {
/// Pallet owner has a right to halt all pallet operations and then resume them. If it is
/// `None`, then there are no direct ways to halt/resume pallet operations, but other
/// runtime methods may still be used to do that (i.e. democracy::referendum to update halt
/// flag directly or call the `halt_operations`).
/// flag directly or call the `set_operating_mode`).
#[pallet::storage]
pub type PalletOwner<T: Config<I>, I: 'static = ()> =
StorageValue<_, T::AccountId, OptionQuery>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1350,7 +1350,7 @@ impl_runtime_apis! {
let forwarded_xcms = xcm_config::XcmRouter::get_messages();
let events: Vec<RuntimeEvent> = System::read_events_no_consensus().map(|record| record.event.clone()).collect();
Ok(ExtrinsicDryRunEffects {
local_xcm: local_xcm.map(VersionedXcm::<()>::V4),
local_xcm: local_xcm.map(VersionedXcm::<()>::from),
forwarded_xcms,
emitted_events: events,
execution_result: result,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1386,7 +1386,7 @@ impl_runtime_apis! {
let forwarded_xcms = xcm_config::XcmRouter::get_messages();
let events: Vec<RuntimeEvent> = System::read_events_no_consensus().map(|record| record.event.clone()).collect();
Ok(ExtrinsicDryRunEffects {
local_xcm: local_xcm.map(VersionedXcm::<()>::V4),
local_xcm: local_xcm.map(VersionedXcm::<()>::from),
forwarded_xcms,
emitted_events: events,
execution_result: result,
Expand Down
34 changes: 21 additions & 13 deletions cumulus/parachains/runtimes/testing/penpal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -849,24 +849,32 @@ impl_runtime_apis! {

impl xcm_fee_payment_runtime_api::fees::XcmPaymentApi<Block> for Runtime {
fn query_acceptable_payment_assets(xcm_version: xcm::Version) -> Result<Vec<VersionedAssetId>, XcmPaymentApiError> {
if !matches!(xcm_version, 3 | 4) {
return Err(XcmPaymentApiError::UnhandledXcmVersion);
}
Ok([VersionedAssetId::V4(xcm_config::RelayLocation::get().into())]
let acceptable = vec![
// native token
VersionedAssetId::from(AssetLocationId(xcm_config::RelayLocation::get()))
];

Ok(acceptable
.into_iter()
.filter_map(|asset| asset.into_version(xcm_version).ok())
.collect())
}

fn query_weight_to_asset_fee(weight: Weight, asset: VersionedAssetId) -> Result<u128, XcmPaymentApiError> {
let local_asset = VersionedAssetId::V4(xcm_config::RelayLocation::get().into());
let asset = asset
.into_version(4)
.map_err(|_| XcmPaymentApiError::VersionedConversionFailed)?;

if asset != local_asset { return Err(XcmPaymentApiError::AssetNotFound); }

Ok(WeightToFee::weight_to_fee(&weight))
match asset.try_as::<AssetLocationId>() {
Ok(asset_id) if asset_id.0 == xcm_config::RelayLocation::get() => {
// for native token
Ok(WeightToFee::weight_to_fee(&weight))
},
Ok(asset_id) => {
log::trace!(target: "xcm::xcm_fee_payment_runtime_api", "query_weight_to_asset_fee - unhandled asset_id: {asset_id:?}!");
Err(XcmPaymentApiError::AssetNotFound)
},
Err(_) => {
log::trace!(target: "xcm::xcm_fee_payment_runtime_api", "query_weight_to_asset_fee - failed to convert asset: {asset:?}!");
Err(XcmPaymentApiError::VersionedConversionFailed)
}
}
}

fn query_xcm_weight(message: VersionedXcm<()>) -> Result<Weight, XcmPaymentApiError> {
Expand Down Expand Up @@ -897,7 +905,7 @@ impl_runtime_apis! {
let forwarded_xcms = xcm_config::XcmRouter::get_messages();
let events: Vec<RuntimeEvent> = System::read_events_no_consensus().map(|record| record.event.clone()).collect();
Ok(ExtrinsicDryRunEffects {
local_xcm: local_xcm.map(VersionedXcm::<()>::V4),
local_xcm: local_xcm.map(VersionedXcm::<()>::from),
forwarded_xcms,
emitted_events: events,
execution_result: result,
Expand Down
2 changes: 1 addition & 1 deletion polkadot/runtime/rococo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1823,7 +1823,7 @@ sp_api::impl_runtime_apis! {
let forwarded_xcms = xcm_config::XcmRouter::get_messages();
let events: Vec<RuntimeEvent> = System::read_events_no_consensus().map(|record| record.event.clone()).collect();
Ok(ExtrinsicDryRunEffects {
local_xcm: local_xcm.map(VersionedXcm::<()>::V4),
local_xcm: local_xcm.map(VersionedXcm::<()>::from),
forwarded_xcms,
emitted_events: events,
execution_result: result,
Expand Down
2 changes: 1 addition & 1 deletion polkadot/runtime/westend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2257,7 +2257,7 @@ sp_api::impl_runtime_apis! {
let forwarded_xcms = xcm_config::XcmRouter::get_messages();
let events: Vec<RuntimeEvent> = System::read_events_no_consensus().map(|record| record.event.clone()).collect();
Ok(ExtrinsicDryRunEffects {
local_xcm: local_xcm.map(VersionedXcm::<()>::V4),
local_xcm: local_xcm.map(VersionedXcm::<()>::from),
forwarded_xcms,
emitted_events: events,
execution_result: result,
Expand Down
61 changes: 34 additions & 27 deletions polkadot/xcm/xcm-fee-payment-runtime-api/tests/fee_estimation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,15 @@ fn fee_estimation_for_teleport() {
let runtime_api = client.runtime_api();
let extrinsic = TestXt::new(
RuntimeCall::XcmPallet(pallet_xcm::Call::transfer_assets {
dest: Box::new(VersionedLocation::V4((Parent, Parachain(1000)).into())),
beneficiary: Box::new(VersionedLocation::V4(
AccountId32 { id: [0u8; 32], network: None }.into(),
)),
assets: Box::new(VersionedAssets::V4(
vec![(Here, 100u128).into(), (Parent, 20u128).into()].into(),
)),
dest: Box::new(VersionedLocation::from((Parent, Parachain(1000)))),
beneficiary: Box::new(VersionedLocation::from(AccountId32 {
id: [0u8; 32],
network: None,
})),
assets: Box::new(VersionedAssets::from(vec![
(Here, 100u128).into(),
(Parent, 20u128).into(),
])),
fee_asset_item: 1, // Fees are paid with the RelayToken
weight_limit: Unlimited,
}),
Expand All @@ -69,7 +71,7 @@ fn fee_estimation_for_teleport() {

assert_eq!(
dry_run_effects.local_xcm,
Some(VersionedXcm::V4(
Some(VersionedXcm::from(
Xcm::builder_unsafe()
.withdraw_asset((Parent, 20u128))
.burn_asset((Parent, 20u128))
Expand All @@ -89,8 +91,8 @@ fn fee_estimation_for_teleport() {
assert_eq!(
dry_run_effects.forwarded_xcms,
vec![(
VersionedLocation::V4(send_destination.clone()),
vec![VersionedXcm::V4(send_message.clone())],
VersionedLocation::from(send_destination.clone()),
vec![VersionedXcm::from(send_message.clone())],
),],
);

Expand Down Expand Up @@ -153,7 +155,7 @@ fn fee_estimation_for_teleport() {
.query_weight_to_asset_fee(
H256::zero(),
weight,
VersionedAssetId::V4(HereLocation::get().into()),
VersionedAssetId::from(AssetId(HereLocation::get())),
)
.unwrap()
.unwrap();
Expand All @@ -168,7 +170,7 @@ fn fee_estimation_for_teleport() {
.query_delivery_fees(H256::zero(), destination.clone(), remote_message.clone())
.unwrap()
.unwrap();
assert_eq!(delivery_fees, VersionedAssets::V4((Here, 20u128).into()));
assert_eq!(delivery_fees, VersionedAssets::from((Here, 20u128)));

// This would have to be the runtime API of the destination,
// which we have the location for.
Expand All @@ -182,7 +184,7 @@ fn fee_estimation_for_teleport() {
.query_weight_to_asset_fee(
H256::zero(),
remote_execution_weight,
VersionedAssetId::V4(HereLocation::get().into()),
VersionedAssetId::from(AssetId(HereLocation::get())),
)
.unwrap()
.unwrap();
Expand Down Expand Up @@ -216,11 +218,12 @@ fn dry_run_reserve_asset_transfer() {
let runtime_api = client.runtime_api();
let extrinsic = TestXt::new(
RuntimeCall::XcmPallet(pallet_xcm::Call::transfer_assets {
dest: Box::new(VersionedLocation::V4((Parent, Parachain(1000)).into())),
beneficiary: Box::new(VersionedLocation::V4(
AccountId32 { id: [0u8; 32], network: None }.into(),
)),
assets: Box::new(VersionedAssets::V4((Parent, 100u128).into())),
dest: Box::new(VersionedLocation::from((Parent, Parachain(1000)))),
beneficiary: Box::new(VersionedLocation::from(AccountId32 {
id: [0u8; 32],
network: None,
})),
assets: Box::new(VersionedAssets::from((Parent, 100u128))),
fee_asset_item: 0,
weight_limit: Unlimited,
}),
Expand All @@ -231,7 +234,7 @@ fn dry_run_reserve_asset_transfer() {

assert_eq!(
dry_run_effects.local_xcm,
Some(VersionedXcm::V4(
Some(VersionedXcm::from(
Xcm::builder_unsafe()
.withdraw_asset((Parent, 100u128))
.burn_asset((Parent, 100u128))
Expand All @@ -251,8 +254,8 @@ fn dry_run_reserve_asset_transfer() {
assert_eq!(
dry_run_effects.forwarded_xcms,
vec![(
VersionedLocation::V4(send_destination.clone()),
vec![VersionedXcm::V4(send_message.clone())],
VersionedLocation::from(send_destination.clone()),
vec![VersionedXcm::from(send_message.clone())],
),],
);

Expand Down Expand Up @@ -310,11 +313,15 @@ fn dry_run_xcm() {
let client = TestClient;
let runtime_api = client.runtime_api();
let xcm_weight = runtime_api
.query_xcm_weight(H256::zero(), VersionedXcm::V4(xcm_to_weigh.clone().into()))
.query_xcm_weight(H256::zero(), VersionedXcm::from(xcm_to_weigh.clone().into()))
.unwrap()
.unwrap();
let execution_fees = runtime_api
.query_weight_to_asset_fee(H256::zero(), xcm_weight, VersionedAssetId::V4(Here.into()))
.query_weight_to_asset_fee(
H256::zero(),
xcm_weight,
VersionedAssetId::from(AssetId(Here.into())),
)
.unwrap()
.unwrap();
let xcm = Xcm::<RuntimeCall>::builder_unsafe()
Expand All @@ -331,16 +338,16 @@ fn dry_run_xcm() {
let dry_run_effects = runtime_api
.dry_run_xcm(
H256::zero(),
VersionedLocation::V4(AccountIndex64 { index: 1, network: None }.into()),
VersionedXcm::V4(xcm),
VersionedLocation::from([AccountIndex64 { index: 1, network: None }]),
VersionedXcm::from(xcm),
)
.unwrap()
.unwrap();
assert_eq!(
dry_run_effects.forwarded_xcms,
vec![(
VersionedLocation::V4((Parent, Parachain(2100)).into()),
vec![VersionedXcm::V4(
VersionedLocation::from((Parent, Parachain(2100))),
vec![VersionedXcm::from(
Xcm::<()>::builder_unsafe()
.reserve_asset_deposited((
(Parent, Parachain(2000)),
Expand Down
44 changes: 29 additions & 15 deletions polkadot/xcm/xcm-fee-payment-runtime-api/tests/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,23 +429,37 @@ impl sp_api::ProvideRuntimeApi<Block> for TestClient {
sp_api::mock_impl_runtime_apis! {
impl XcmPaymentApi<Block> for RuntimeApi {
fn query_acceptable_payment_assets(xcm_version: XcmVersion) -> Result<Vec<VersionedAssetId>, XcmPaymentApiError> {
if xcm_version != 4 { return Err(XcmPaymentApiError::UnhandledXcmVersion) };
Ok(vec![VersionedAssetId::V4(HereLocation::get().into())])
Ok(vec![
VersionedAssetId::from(AssetId(HereLocation::get()))
.into_version(xcm_version)
.map_err(|_| XcmPaymentApiError::VersionedConversionFailed)?
])
}

fn query_xcm_weight(message: VersionedXcm<()>) -> Result<Weight, XcmPaymentApiError> {
XcmPallet::query_xcm_weight(message)
}

fn query_weight_to_asset_fee(weight: Weight, asset: VersionedAssetId) -> Result<u128, XcmPaymentApiError> {
let local_asset = VersionedAssetId::V4(HereLocation::get().into());
let asset = asset
.into_version(4)
.map_err(|_| XcmPaymentApiError::VersionedConversionFailed)?;

if asset != local_asset { return Err(XcmPaymentApiError::AssetNotFound); }

Ok(WeightToFee::weight_to_fee(&weight))
match asset.try_as::<AssetId>() {
Ok(asset_id) if asset_id.0 == HereLocation::get() => {
Ok(WeightToFee::weight_to_fee(&weight))
},
Ok(asset_id) => {
log::trace!(
target: "xcm::XcmPaymentApi::query_weight_to_asset_fee",
"query_weight_to_asset_fee - unhandled asset_id: {asset_id:?}!"
);
Err(XcmPaymentApiError::AssetNotFound)
},
Err(_) => {
log::trace!(
target: "xcm::XcmPaymentApi::query_weight_to_asset_fee",
"query_weight_to_asset_fee - failed to convert asset: {asset:?}!"
);
Err(XcmPaymentApiError::VersionedConversionFailed)
}
}
}

fn query_delivery_fees(destination: VersionedLocation, message: VersionedXcm<()>) -> Result<VersionedAssets, XcmPaymentApiError> {
Expand All @@ -471,12 +485,12 @@ sp_api::mock_impl_runtime_apis! {
let forwarded_xcms = sent_xcm()
.into_iter()
.map(|(location, message)| (
VersionedLocation::V4(location),
vec![VersionedXcm::V4(message)],
VersionedLocation::from(location),
vec![VersionedXcm::from(message)],
)).collect();
let events: Vec<RuntimeEvent> = System::events().iter().map(|record| record.event.clone()).collect();
Ok(ExtrinsicDryRunEffects {
local_xcm: local_xcm.map(VersionedXcm::<()>::V4),
local_xcm: local_xcm.map(VersionedXcm::<()>::from),
forwarded_xcms,
emitted_events: events,
execution_result: result,
Expand Down Expand Up @@ -511,8 +525,8 @@ sp_api::mock_impl_runtime_apis! {
let forwarded_xcms = sent_xcm()
.into_iter()
.map(|(location, message)| (
VersionedLocation::V4(location),
vec![VersionedXcm::V4(message)],
VersionedLocation::from(location),
vec![VersionedXcm::from(message)],
)).collect();
let events: Vec<RuntimeEvent> = System::events().iter().map(|record| record.event.clone()).collect();
Ok(XcmDryRunEffects {
Expand Down

0 comments on commit b00e168

Please sign in to comment.