Skip to content

Commit

Permalink
[DNM] debug XCMP queue crash - failing decode
Browse files Browse the repository at this point in the history
  • Loading branch information
acatangiu committed Nov 1, 2023
1 parent 72a8a38 commit a290fd9
Show file tree
Hide file tree
Showing 4 changed files with 158 additions and 63 deletions.
102 changes: 53 additions & 49 deletions cumulus/pallets/xcmp-queue/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -738,58 +738,62 @@ impl<T: Config> Pallet<T> {
*messages_processed < MAX_MESSAGES_PER_BLOCK
{
last_remaining_fragments = remaining_fragments;
if let Ok(xcm) = VersionedXcm::<T::RuntimeCall>::decode_with_depth_limit(
match VersionedXcm::<T::RuntimeCall>::decode_with_depth_limit(
MAX_XCM_DECODE_DEPTH,
&mut remaining_fragments,
) {
let weight = max_weight - weight_used;
*messages_processed += 1;
match Self::handle_xcm_message(sender, sent_at, xcm, weight) {
Ok(used) => weight_used = weight_used.saturating_add(used),
Err(XcmError::WeightLimitReached(required))
if required.any_gt(max_individual_weight) =>
{
let is_under_limit =
Overweight::<T>::count() < MAX_OVERWEIGHT_MESSAGES;
weight_used.saturating_accrue(T::DbWeight::get().reads(1));
if is_under_limit {
// overweight - add to overweight queue and continue with
// message execution consuming the message.
let msg_len = last_remaining_fragments
.len()
.saturating_sub(remaining_fragments.len());
let overweight_xcm =
last_remaining_fragments[..msg_len].to_vec();
let index =
Self::stash_overweight(sender, sent_at, overweight_xcm);
let e = Event::OverweightEnqueued {
sender,
sent_at,
index,
required,
};
Self::deposit_event(e);
}
},
Err(XcmError::WeightLimitReached(required))
if required.all_lte(max_weight) =>
{
// That message didn't get processed this time because of being
// too heavy. We leave it around for next time and bail.
remaining_fragments = last_remaining_fragments;
break
},
Err(error) => {
log::error!(
"Failed to process XCMP-XCM message, caused by {:?}",
error
);
// Message looks invalid; don't attempt to retry
},
}
} else {
debug_assert!(false, "Invalid incoming XCMP message data");
remaining_fragments = &b""[..];
Ok(xcm) => {
let weight = max_weight - weight_used;
*messages_processed += 1;
match Self::handle_xcm_message(sender, sent_at, xcm, weight) {
Ok(used) => weight_used = weight_used.saturating_add(used),
Err(XcmError::WeightLimitReached(required))
if required.any_gt(max_individual_weight) =>
{
let is_under_limit =
Overweight::<T>::count() < MAX_OVERWEIGHT_MESSAGES;
weight_used.saturating_accrue(T::DbWeight::get().reads(1));
if is_under_limit {
// overweight - add to overweight queue and continue with
// message execution consuming the message.
let msg_len = last_remaining_fragments
.len()
.saturating_sub(remaining_fragments.len());
let overweight_xcm =
last_remaining_fragments[..msg_len].to_vec();
let index =
Self::stash_overweight(sender, sent_at, overweight_xcm);
let e = Event::OverweightEnqueued {
sender,
sent_at,
index,
required,
};
Self::deposit_event(e);
}
},
Err(XcmError::WeightLimitReached(required))
if required.all_lte(max_weight) =>
{
// That message didn't get processed this time because of being
// too heavy. We leave it around for next time and bail.
remaining_fragments = last_remaining_fragments;
break
},
Err(error) => {
log::error!(
"Failed to process XCMP-XCM message, caused by {:?}",
error
);
// Message looks invalid; don't attempt to retry
},
}
},
Err(e) => {
log::error!("XCMP failed to decode message, error {:?}", e);
debug_assert!(false, "Invalid incoming XCMP message data");
remaining_fragments = &b""[..];
},
}
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,5 +88,25 @@ pub fn system_para_test_args(
}
}

/// Returns a `TestArgs` instance to de used for the System Parachain accross integraton tests
pub fn para_test_args(
dest: MultiLocation,
beneficiary_id: AccountId32,
amount: Balance,
assets: MultiAssets,
asset_id: Option<u32>,
fee_asset_item: u32,
) -> TestArgs {
TestArgs {
dest,
beneficiary: AccountId32Junction { network: None, id: beneficiary_id.into() }.into(),
amount,
assets,
asset_id,
fee_asset_item,
weight_limit: WeightLimit::Unlimited,
}
}

#[cfg(test)]
mod tests;
Original file line number Diff line number Diff line change
Expand Up @@ -369,40 +369,107 @@ fn reserve_transfer_native_asset_from_system_para_to_para() {
// transfers
}

/// Limited Reserve Transfers of a local asset from System Parachain to Parachain should work
#[test]
fn limited_reserve_transfer_asset_from_system_para_to_para() {
fn debug_limited_reserve_transfer_asset_from_system_para_to_para_works() {
let amount_to_send = ASSET_MIN_BALANCE * 1000;
let assets: MultiAssets =
vec![(X2(PalletInstance(ASSETS_PALLET_ID), GeneralIndex(ASSET_ID.into())), amount_to_send)
.into()]
.into();
let fee_asset_index = 0;
do_limited_reserve_transfer_asset_from_system_para_to_para(
assets,
amount_to_send,
fee_asset_index,
);
}

#[test]
fn debug_limited_reserve_transfer_asset_from_system_para_to_para_crashes() {
let fee_amount_to_send = ASSET_HUB_ROCOCO_ED * 1000;
let assets_amount_to_send = ASSET_MIN_BALANCE * 1000;
let assets: MultiAssets = vec![
(Parent, fee_amount_to_send).into(),
(
X2(PalletInstance(ASSETS_PALLET_ID), GeneralIndex(ASSET_ID.into())),
assets_amount_to_send,
)
.into(),
]
.into();
let fee_asset_index = assets
.inner()
.iter()
.position(|r| r == &(Parent, fee_amount_to_send).into())
.unwrap() as u32;
do_limited_reserve_transfer_asset_from_system_para_to_para(
assets,
assets_amount_to_send,
fee_asset_index,
);
}

/// Limited Reserve Transfers of a local asset from System Parachain to Parachain should work
fn do_limited_reserve_transfer_asset_from_system_para_to_para(
assets_to_send: MultiAssets,
amount_to_send: u128,
fee_asset_index: u32,
) {
// Force create asset from Relay Chain and mint assets for System Parachain's sender account
AssetHubRococo::force_create_and_mint_asset(
ASSET_ID,
ASSET_MIN_BALANCE,
true,
AssetHubRococoSender::get(),
Some(Weight::from_parts(1_019_445_000, 200_000)),
ASSET_MIN_BALANCE * 1000000,
ASSET_MIN_BALANCE * 1_000_000,
);

// Init values for System Parachain
let destination = AssetHubRococo::sibling_location_of(PenpalRococoA::para_id());
let beneficiary_id = PenpalRococoAReceiver::get();
let amount_to_send = ASSET_MIN_BALANCE * 1000;
let assets =
(X2(PalletInstance(ASSETS_PALLET_ID), GeneralIndex(ASSET_ID.into())), amount_to_send)
.into();
let assets = assets_to_send;

let system_para_test_args = TestContext {
println!("fee_asset_index: {:?}", fee_asset_index);

let para_test_args = TestContext {
sender: AssetHubRococoSender::get(),
receiver: PenpalRococoAReceiver::get(),
args: system_para_test_args(destination, beneficiary_id, amount_to_send, assets, None),
args: para_test_args(
destination,
beneficiary_id,
amount_to_send,
assets,
None,
fee_asset_index,
),
};

let mut system_para_test = SystemParaToParaTest::new(system_para_test_args);
let mut test = SystemParaToParaTest::new(para_test_args);

system_para_test.set_assertion::<AssetHubRococo>(system_para_to_para_assets_assertions);
let sender_balance_before = AssetHubRococo::execute_with(|| {
type Assets = <AssetHubRococo as AssetHubRococoPallet>::Assets;
<Assets as Inspect<_>>::balance(ASSET_ID, &AssetHubRococoSender::get())
});

let receiver_balance_before = PenpalRococoA::execute_with(|| {
type Assets = <PenpalRococoA as PenpalRococoAPallet>::Assets;
<Assets as Inspect<_>>::balance(ASSET_ID, &PenpalRococoAReceiver::get())
});

println!("sender before: {:?}", sender_balance_before);
println!("receiver before: {:?}", receiver_balance_before);

// test.set_assertion::<AssetHubRococo>(system_para_to_para_assets_sender_assertions);
// test.set_assertion::<PenpalRococoA>(system_para_to_para_assets_receiver_assertions);
// TODO: Add assertions when Penpal is able to manage assets
system_para_test
.set_dispatchable::<AssetHubRococo>(system_para_to_para_limited_reserve_transfer_assets);
system_para_test.assert();
test.set_dispatchable::<AssetHubRococo>(system_para_to_para_limited_reserve_transfer_assets);
test.assert();

// // Sender's balance is reduced
// assert_eq!(sender_balance_before - amount_to_send - delivery_fees, sender_balance_after);
// // Receiver's balance is increased
// assert!(receiver_balance_after > receiver_balance_before);
}

/// Reserve Transfers of a local asset from System Parachain to Parachain should work
Expand Down
4 changes: 4 additions & 0 deletions polkadot/xcm/xcm-executor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,10 @@ impl<Config: config::Config> XcmExecutor<Config> {
assets.reanchor(&dest, reanchor_context).map_err(|()| XcmError::LocationFull)?;
let mut message = vec![ReserveAssetDeposited(assets), ClearOrigin];
message.extend(xcm.0.into_iter());
log::error!(
target: "xcm::process_instruction TransferReserveAsset",
"=== send xcm {:?}", message
);
self.send(dest, Xcm(message), FeeReason::TransferReserveAsset)?;
Ok(())
},
Expand Down

0 comments on commit a290fd9

Please sign in to comment.