Skip to content

Fix assert_expected_events macro #7913

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 37 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
5a8028c
fix assert_expected_events macro
claravanstaden Mar 13, 2025
d6ab120
rename var
claravanstaden Mar 13, 2025
372b07b
add actual event data
claravanstaden Mar 13, 2025
f05b6bf
fix test
claravanstaden Mar 13, 2025
0d070f9
revert println
claravanstaden Mar 13, 2025
18e57e6
cleanup
claravanstaden Mar 13, 2025
fce326b
Merge branch 'master' into fix-assert-expected-events-macro
claravanstaden Mar 13, 2025
e75b2a2
fix test
claravanstaden Mar 13, 2025
4c92d5d
Merge remote-tracking branch 'origin/fix-assert-expected-events-macro…
claravanstaden Mar 13, 2025
720886d
Merge branch 'master' into fix-assert-expected-events-macro
claravanstaden Mar 14, 2025
d259c9e
fix tests
claravanstaden Apr 1, 2025
2652401
fix more tests
claravanstaden Apr 1, 2025
8bb78db
more test fixes
claravanstaden Apr 1, 2025
27ea4a9
fix test
claravanstaden Apr 1, 2025
a10fd08
more tests
claravanstaden Apr 1, 2025
690aefe
more test fixes
claravanstaden Apr 1, 2025
f49210c
more test fixes
claravanstaden Apr 1, 2025
0032000
test progress
claravanstaden Apr 1, 2025
ae3d1af
more fixes
claravanstaden Apr 2, 2025
b67cf8b
fix bh rococo tests
claravanstaden Apr 2, 2025
8f87f35
more fixes
claravanstaden Apr 2, 2025
3ed071c
Merge branch 'master' into fix-assert-expected-events-macro
claravanstaden Apr 2, 2025
74fbd17
more test fixes
claravanstaden Apr 2, 2025
f9fbd5a
Merge branch 'master' into fix-assert-expected-events-macro
claravanstaden Apr 2, 2025
6e521e6
prdoc
claravanstaden Apr 2, 2025
702d913
Merge remote-tracking branch 'origin/fix-assert-expected-events-macro…
claravanstaden Apr 2, 2025
4e1c25c
fmt
claravanstaden Apr 2, 2025
4543c05
fix semver
claravanstaden Apr 2, 2025
3f67188
remove debug prints
claravanstaden Apr 2, 2025
4525128
Merge branch 'master' into fix-assert-expected-events-macro
claravanstaden Apr 3, 2025
c564b8c
Merge branch 'master' into fix-assert-expected-events-macro
claravanstaden Apr 3, 2025
e0e856f
Merge branch 'master' into fix-assert-expected-events-macro
acatangiu Apr 10, 2025
457f7e6
Update prdoc/pr_7913.prdoc
acatangiu Apr 10, 2025
22171ce
Update prdoc/pr_7913.prdoc
acatangiu Apr 10, 2025
1c415d1
Merge branch 'master' into fix-assert-expected-events-macro
claravanstaden Apr 10, 2025
ee72eae
pr comments
claravanstaden Apr 15, 2025
82e3384
revert unnecessary clone
claravanstaden Apr 15, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,16 @@ pub fn non_fee_asset(assets: &Assets, fee_idx: usize) -> Option<(Location, u128)
Some((asset.id.0, asset_amount))
}

/// Helper method to get the fee asset used in multiple assets transfer
pub fn fee_asset(assets: &Assets, fee_idx: usize) -> Option<(Location, u128)> {
let asset = assets.get(fee_idx)?;
let asset_amount = match asset.fun {
Fungible(amount) => amount,
_ => return None,
};
Some((asset.id.0.clone(), asset_amount))
}

pub fn get_amount_from_versioned_assets(assets: VersionedAssets) -> u128 {
let latest_assets: Assets = assets.try_into().unwrap();
let Fungible(amount) = latest_assets.inner()[0].fun else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,17 @@ mod imports {
TestArgs, TestContext, TestExt,
},
xcm_helpers::{
get_amount_from_versioned_assets, non_fee_asset, xcm_transact_paid_execution,
fee_asset, get_amount_from_versioned_assets, non_fee_asset, xcm_transact_paid_execution,
},
ASSETS_PALLET_ID, RESERVABLE_ASSET_ID, XCM_V3,
PenpalATeleportableAssetLocation, ASSETS_PALLET_ID, RESERVABLE_ASSET_ID, XCM_V3,
};
pub use parachains_common::Balance;
pub use rococo_system_emulated_network::{
asset_hub_rococo_emulated_chain::{
asset_hub_rococo_runtime::{
self,
xcm_config::{
self as ahr_xcm_config, TokenLocation as RelayLocation,
self as ahr_xcm_config, TokenLocation as RelayLocation, TreasuryAccount,
XcmConfig as AssetHubRococoXcmConfig,
},
AssetConversionOrigin as AssetHubRococoAssetConversionOrigin,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -737,8 +737,10 @@ fn transfer_native_asset_from_relay_to_para_through_asset_hub() {
}
fn penpal_assertions(t: RelayToParaThroughAHTest) {
type RuntimeEvent = <PenpalA as Chain>::RuntimeEvent;
let expected_id =
t.args.assets.into_inner().first().unwrap().id.0.clone().try_into().unwrap();
// Assets in t are relative to the relay chain. The asset here should be relative to
// Penpal, so parents: 1.
let expected_id: Location = Location { parents: 1, interior: Here };

assert_expected_events!(
PenpalA,
vec![
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use sp_core::{crypto::get_public_from_string_or_panic, sr25519};
fn relay_to_para_sender_assertions(t: RelayToParaTest) {
type RuntimeEvent = <Rococo as Chain>::RuntimeEvent;

Rococo::assert_xcm_pallet_attempted_complete(Some(Weight::from_parts(864_610_000, 8_799)));
Rococo::assert_xcm_pallet_attempted_complete(Some(Weight::from_parts(350_000_000, 7000)));

assert_expected_events!(
Rococo,
Expand All @@ -41,7 +41,7 @@ fn relay_to_para_sender_assertions(t: RelayToParaTest) {

fn para_to_relay_sender_assertions(t: ParaToRelayTest) {
type RuntimeEvent = <PenpalA as Chain>::RuntimeEvent;
PenpalA::assert_xcm_pallet_attempted_complete(Some(Weight::from_parts(864_610_000, 8_799)));
PenpalA::assert_xcm_pallet_attempted_complete(Some(Weight::from_parts(2_000_000_000, 140_000)));
assert_expected_events!(
PenpalA,
vec![
Expand Down Expand Up @@ -141,6 +141,34 @@ pub fn system_para_to_para_receiver_assertions(t: SystemParaToParaTest) {
}
}

pub fn system_para_to_penpal_receiver_assertions(t: SystemParaToParaTest) {
type RuntimeEvent = <PenpalA as Chain>::RuntimeEvent;

PenpalA::assert_xcmp_queue_success(None);
for asset in t.args.assets.into_inner().into_iter() {
let mut expected_id: Location = asset.id.0.try_into().unwrap();
let relative_id = match expected_id {
Location { parents: 1, interior: Here } => expected_id,
_ => {
expected_id
.push_front_interior(Parachain(AssetHubRococo::para_id().into()))
.unwrap();
Location::new(1, expected_id.interior().clone())
},
};

assert_expected_events!(
PenpalA,
vec![
RuntimeEvent::ForeignAssets(pallet_assets::Event::Issued { asset_id, owner, .. }) => {
asset_id: *asset_id == relative_id,
owner: *owner == t.receiver.account_id,
},
]
);
}
}

pub fn para_to_system_para_sender_assertions(t: ParaToSystemParaTest) {
type RuntimeEvent = <PenpalA as Chain>::RuntimeEvent;
PenpalA::assert_xcm_pallet_attempted_complete(None);
Expand Down Expand Up @@ -195,7 +223,11 @@ pub fn para_to_system_para_receiver_assertions(t: ParaToSystemParaTest) {
type RuntimeEvent = <AssetHubRococo as Chain>::RuntimeEvent;
AssetHubRococo::assert_xcmp_queue_success(None);

let sov_acc_of_penpal = AssetHubRococo::sovereign_account_id_of(t.args.dest.clone());
let sov_acc_of_penpal = AssetHubRococo::sovereign_account_id_of(Location::new(
1,
Parachain(PenpalA::para_id().into()),
));

for (idx, asset) in t.args.assets.into_inner().into_iter().enumerate() {
let expected_id = asset.id.0.clone().try_into().unwrap();
let asset_amount = if let Fungible(a) = asset.fun { Some(a) } else { None }.unwrap();
Expand Down Expand Up @@ -255,6 +287,7 @@ fn system_para_to_para_assets_sender_assertions(t: SystemParaToParaTest) {
864_610_000,
8799,
)));

assert_expected_events!(
AssetHubRococo,
vec![
Expand All @@ -269,11 +302,9 @@ fn system_para_to_para_assets_sender_assertions(t: SystemParaToParaTest) {
),
amount: *amount == t.args.amount,
},
// Native asset to pay for fees is transferred to Parachain's Sovereign account
// Native asset to pay for fees is transferred to Treasury
RuntimeEvent::Balances(pallet_balances::Event::Minted { who, .. }) => {
who: *who == AssetHubRococo::sovereign_account_id_of(
t.args.dest.clone()
),
who: *who == TreasuryAccount::get(),
},
// Delivery fees are paid
RuntimeEvent::PolkadotXcm(
Expand All @@ -287,7 +318,7 @@ fn para_to_system_para_assets_sender_assertions(t: ParaToSystemParaTest) {
type RuntimeEvent = <PenpalA as Chain>::RuntimeEvent;
let system_para_native_asset_location = RelayLocation::get();
let reservable_asset_location = PenpalLocalReservableFromAssetHub::get();
PenpalA::assert_xcm_pallet_attempted_complete(Some(Weight::from_parts(864_610_000, 8799)));
PenpalA::assert_xcm_pallet_attempted_complete(Some(Weight::from_parts(2_000_000_000, 140000)));
assert_expected_events!(
PenpalA,
vec![
Expand Down Expand Up @@ -412,9 +443,8 @@ fn para_to_para_asset_hub_hop_assertions(t: ParaToParaThroughAHTest) {
let sov_penpal_a_on_ah = AssetHubRococo::sovereign_account_id_of(
AssetHubRococo::sibling_location_of(PenpalA::para_id()),
);
let sov_penpal_b_on_ah = AssetHubRococo::sovereign_account_id_of(
AssetHubRococo::sibling_location_of(PenpalB::para_id()),
);

let (_, asset_amount) = fee_asset(&t.args.assets, t.args.fee_asset_item as usize).unwrap();

assert_expected_events!(
AssetHubRococo,
Expand All @@ -424,13 +454,7 @@ fn para_to_para_asset_hub_hop_assertions(t: ParaToParaThroughAHTest) {
pallet_assets::Event::Burned { owner, balance, .. }
) => {
owner: *owner == sov_penpal_a_on_ah,
balance: *balance == t.args.amount,
},
// Deposited to receiver parachain SA
RuntimeEvent::Assets(
pallet_assets::Event::Deposited { who, .. }
) => {
who: *who == sov_penpal_b_on_ah,
Comment on lines -430 to -433
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this because I can't figure out to which account these funds are deposited. The assets are deposited and then an asset conversion takes place, could it be related to the assets conversion pallet? I tried some combinations (like the asset conversion pallet index, account, neither worked).

I figured since these tests didn't really work, I would rather remove the assertion and it can be re-added, than having this buggy macro continue to be used without it actually working.

balance: *balance == asset_amount,
},
RuntimeEvent::MessageQueue(
pallet_message_queue::Event::Processed { success: true, .. }
Expand Down Expand Up @@ -804,7 +828,7 @@ fn reserve_transfer_native_asset_from_asset_hub_to_para() {

// Set assertions and dispatchables
test.set_assertion::<AssetHubRococo>(system_para_to_para_sender_assertions);
test.set_assertion::<PenpalA>(system_para_to_para_receiver_assertions);
test.set_assertion::<PenpalA>(system_para_to_penpal_receiver_assertions);
test.set_dispatchable::<AssetHubRococo>(system_para_to_para_reserve_transfer_assets);
test.assert();

Expand Down Expand Up @@ -1326,7 +1350,7 @@ fn reserve_transfer_usdt_from_asset_hub_to_para() {
});

test.set_assertion::<AssetHubRococo>(system_para_to_para_sender_assertions);
test.set_assertion::<PenpalA>(system_para_to_para_receiver_assertions);
test.set_assertion::<PenpalA>(system_para_to_penpal_receiver_assertions);
test.set_dispatchable::<AssetHubRococo>(system_para_to_para_reserve_transfer_assets);
test.assert();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ fn para_origin_assertions(t: SystemParaToRelayTest) {
type RuntimeEvent = <AssetHubRococo as Chain>::RuntimeEvent;

AssetHubRococo::assert_xcm_pallet_attempted_complete(Some(Weight::from_parts(
720_053_000,
7_203,
730_053_000,
4_000,
)));

AssetHubRococo::assert_parachain_system_ump_sent();
Expand Down Expand Up @@ -76,12 +76,12 @@ fn penpal_to_ah_foreign_assets_receiver_assertions(t: ParaToSystemParaTest) {
let sov_penpal_on_ahr = AssetHubRococo::sovereign_account_id_of(
AssetHubRococo::sibling_location_of(PenpalA::para_id()),
);
let (expected_foreign_asset_id, expected_foreign_asset_amount) =
let (_, expected_foreign_asset_amount) =
non_fee_asset(&t.args.assets, t.args.fee_asset_item as usize).unwrap();
let (_, fee_asset_amount) = fee_asset(&t.args.assets, t.args.fee_asset_item as usize).unwrap();

AssetHubRococo::assert_xcmp_queue_success(None);

println!("expected_foreign_asset_id: {:?}", expected_foreign_asset_id);
assert_expected_events!(
AssetHubRococo,
vec![
Expand All @@ -90,13 +90,13 @@ fn penpal_to_ah_foreign_assets_receiver_assertions(t: ParaToSystemParaTest) {
pallet_balances::Event::Burned { who, amount }
) => {
who: *who == sov_penpal_on_ahr.clone().into(),
amount: *amount == t.args.amount,
amount: *amount == fee_asset_amount,
},
RuntimeEvent::Balances(pallet_balances::Event::Minted { who, .. }) => {
who: *who == t.receiver.account_id,
},
RuntimeEvent::ForeignAssets(pallet_assets::Event::Issued { asset_id, owner, amount }) => {
asset_id: *asset_id == expected_foreign_asset_id,
asset_id: *asset_id == PenpalATeleportableAssetLocation::get(),
owner: *owner == t.receiver.account_id,
amount: *amount == expected_foreign_asset_amount,
},
Expand All @@ -110,6 +110,7 @@ fn ah_to_penpal_foreign_assets_sender_assertions(t: SystemParaToParaTest) {
AssetHubRococo::assert_xcm_pallet_attempted_complete(None);
let (expected_foreign_asset_id, expected_foreign_asset_amount) =
non_fee_asset(&t.args.assets, t.args.fee_asset_item as usize).unwrap();
let (_, fee_asset_amount) = fee_asset(&t.args.assets, t.args.fee_asset_item as usize).unwrap();
assert_expected_events!(
AssetHubRococo,
vec![
Expand All @@ -121,7 +122,7 @@ fn ah_to_penpal_foreign_assets_sender_assertions(t: SystemParaToParaTest) {
to: *to == AssetHubRococo::sovereign_account_id_of(
t.args.dest.clone()
),
amount: *amount == t.args.amount,
amount: *amount == fee_asset_amount,
},
// foreign asset is burned locally as part of teleportation
RuntimeEvent::ForeignAssets(pallet_assets::Event::Burned { asset_id, owner, balance }) => {
Expand Down Expand Up @@ -159,10 +160,9 @@ fn ah_to_penpal_foreign_assets_receiver_assertions(t: SystemParaToParaTest) {
amount: *amount == expected_asset_amount,
},
// native asset for fee is deposited to receiver
RuntimeEvent::ForeignAssets(pallet_assets::Event::Issued { asset_id, owner, amount }) => {
RuntimeEvent::ForeignAssets(pallet_assets::Event::Issued { asset_id, owner, .. }) => {
asset_id: *asset_id == system_para_native_asset_location,
owner: *owner == t.receiver.account_id,
amount: *amount == expected_asset_amount,
},
]
);
Expand Down Expand Up @@ -291,6 +291,7 @@ pub fn do_bidirectional_teleport_foreign_assets_between_para_and_asset_hub_using
_ => unreachable!(),
};
let asset_amount_to_send = ASSET_HUB_ROCOCO_ED * 1000;

let asset_owner = PenpalAssetOwner::get();
let system_para_native_asset_location = RelayLocation::get();
let sender = PenpalASender::get();
Expand Down Expand Up @@ -457,6 +458,7 @@ pub fn do_bidirectional_teleport_foreign_assets_between_para_and_asset_hub_using
fee_asset_index,
),
};

let mut ah_to_penpal = SystemParaToParaTest::new(ah_to_penpal_test_args);

let ah_sender_balance_before = ah_to_penpal.sender.balance;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ fn hop_assertions(test: ParaToParaThroughAHTest) {
RuntimeEvent::Balances(
pallet_balances::Event::Burned { amount, .. }
) => {
amount: *amount == test.args.amount,
amount: *amount > test.args.amount * 90/100,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@franciscoaguirre for multi_hop_pay_fees_works, the expected amount for the burn event is 1000000000000, but the actual amount in the event is 966873920000. Probably a part of it was used to pay for fees? I want to update the tests but just make sure with you if it is OK. I want to remove the amount or change it to something like: amount: *amount >= test.args.amount *.9 (i.e. the amount burned should at least be higher than 90% of the test amount).

},
]
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,17 @@ mod imports {
TestArgs, TestContext, TestExt,
},
xcm_helpers::{
get_amount_from_versioned_assets, non_fee_asset, xcm_transact_paid_execution,
fee_asset, get_amount_from_versioned_assets, non_fee_asset, xcm_transact_paid_execution,
},
ASSETS_PALLET_ID, RESERVABLE_ASSET_ID, USDT_ID, XCM_V3,
PenpalATeleportableAssetLocation, ASSETS_PALLET_ID, RESERVABLE_ASSET_ID, USDT_ID, XCM_V3,
};
pub use parachains_common::{AccountId, Balance};
pub use westend_system_emulated_network::{
asset_hub_westend_emulated_chain::{
asset_hub_westend_runtime::{
self,
xcm_config::{
self as ahw_xcm_config, WestendLocation as RelayLocation,
self as ahw_xcm_config, TreasuryAccount, WestendLocation as RelayLocation,
XcmConfig as AssetHubWestendXcmConfig,
},
AssetConversionOrigin as AssetHubWestendAssetConversionOrigin,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -738,13 +738,11 @@ fn transfer_native_asset_from_relay_to_penpal_through_asset_hub() {
}
fn penpal_assertions(t: RelayToParaThroughAHTest) {
type RuntimeEvent = <PenpalA as Chain>::RuntimeEvent;
let expected_id =
t.args.assets.into_inner().first().unwrap().id.0.clone().try_into().unwrap();
assert_expected_events!(
PenpalA,
vec![
RuntimeEvent::ForeignAssets(pallet_assets::Event::Issued { asset_id, owner, .. }) => {
asset_id: *asset_id == expected_id,
asset_id: *asset_id == Location::new(1, Here),
owner: *owner == t.receiver.account_id,
},
]
Expand Down
Loading
Loading