Fix CI + one integration test for bridge-hub-kusama (#397)

Closes: #396

This PR:
- fixes CI test pipelines to block PRs when tests fail. See more details
[here](#396).
- Fixes an integration test for `bridge-hub-kusama-integration-tests`
that was not caught due to a previous CI bug.
- Includes some minor cleanup, changing `Versioned*::V4` to
`Versioned*::from` to use the latest version (for easier migration to
`xcm:v5`).

<!-- Remember that you can run `/merge` to enable auto-merge in the PR
-->

<!-- Remember to modify the changelog. If you don't need to modify it,
you can check the following box.
Instead, if you have already modified it, simply delete the following
line. -->

## TODO

- [x] fix the integration tests
- [X] Does not require a CHANGELOG entry

---------

Co-authored-by: Javier Bullrich <javier@bullrich.dev>

.github/workflows/changelog.yml

@@ -1,6 +1,6 @@
 name: Verify Changelog
 
-# If you modify more test jobs, ensure that you add them as required to the job "confirmTestPassed"
+# If you modify more test jobs, ensure that you add them as required to the job "confirmChangelogChecksPassed"
 # which is located at the end of this file (more info in the job)
 
 on:
@@ -16,7 +16,7 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-
+  # Job required by "confirmChangelogChecksPassed"
   verify-changelog-updated:
     name: Verify that Changelog is Updated
     runs-on: ubuntu-latest
@@ -32,6 +32,7 @@ jobs:
         if: steps.changed.outputs.matched != 'true' && !contains(github.event.pull_request.body, '[x] Does not require a CHANGELOG entry')
         run: echo "::error::CHANGELOG.md has not been modified. Either modify the file or check the checkbox in the body" && exit 1
 
+  # Job required by "confirmChangelogChecksPassed"
   verify-changelog-valid:
     name: Verify that Changelog is valid
     runs-on: ubuntu-latest
@@ -50,10 +51,10 @@ jobs:
   # If you add more jobs, remember to add them to the "needs" array.
   confirmChangelogChecksPassed:
     runs-on: ubuntu-latest
-    name: All changelog tests passed
+    name: All changelog checks passed
     # If any new job gets added, be sure to add it to this list
     needs:
       - verify-changelog-updated
       - verify-changelog-valid
     steps:
-      - run: echo '### Good job! All the tests passed 🚀' >> $GITHUB_STEP_SUMMARY
+      - run: echo '### Good job! All the changelog checks passed 🚀' >> $GITHUB_STEP_SUMMARY

.github/workflows/test.yml

@@ -45,7 +45,6 @@ jobs:
   # Job required by "confirmTestPassed"
   runtime-test:
     needs: [runtime-matrix]
-    continue-on-error: true
     runs-on: ubuntu-22.04
     strategy:
       matrix:
@@ -97,7 +96,6 @@ jobs:
   # Job required by "confirmTestPassed"
   integration-test:
     needs: [integration-test-matrix]
-    continue-on-error: true
     runs-on: ubuntu-22.04
     strategy:
       matrix:

integration-tests/emulated/helpers/src/lib.rs

@@ -63,7 +63,7 @@ macro_rules! test_relay_is_trusted_teleporter {
 						weight_limit: weight_limit.clone(),
 					});
 					let mut delivery_fees_amount = 0;
-					let mut remote_message = VersionedXcm::V4(Xcm(Vec::new()));
+					let mut remote_message = VersionedXcm::from(Xcm(Vec::new()));
 					<$sender_relay>::execute_with(|| {
 						type Runtime = <$sender_relay as Chain>::Runtime;
 						type OriginCaller = <$sender_relay as Chain>::OriginCaller;
@@ -75,7 +75,7 @@ macro_rules! test_relay_is_trusted_teleporter {
 							.forwarded_xcms
 							.iter()
 							.find(|(destination, _)| {
-								*destination == VersionedLocation::V4(Location::new(0, [Parachain(<$receiver_para>::para_id().into())]))
+								*destination == VersionedLocation::from(Location::new(0, [Parachain(<$receiver_para>::para_id().into())]))
 							})
 							.unwrap();
 						assert_eq!(messages_to_query.len(), 1);
@@ -85,7 +85,7 @@ macro_rules! test_relay_is_trusted_teleporter {
 								.unwrap();
 						let latest_delivery_fees: Assets = delivery_fees.clone().try_into().unwrap();
 						let Fungible(inner_delivery_fees_amount) = latest_delivery_fees.inner()[0].fun else {
-							unreachable!("asset is fungible");
+							unreachable!("asset is non-fungible");
 						};
 						delivery_fees_amount = inner_delivery_fees_amount;
 					});
@@ -201,7 +201,7 @@ macro_rules! test_parachain_is_trusted_teleporter_for_relay {
 			});
 			// These will be filled in the closure.
 			let mut delivery_fees_amount = 0;
-			let mut remote_message = VersionedXcm::V4(Xcm(Vec::new()));
+			let mut remote_message = VersionedXcm::from(Xcm(Vec::new()));
 			<$sender_para>::execute_with(|| {
 				type Runtime = <$sender_para as Chain>::Runtime;
 				type OriginCaller = <$sender_para as Chain>::OriginCaller;
@@ -213,7 +213,7 @@ macro_rules! test_parachain_is_trusted_teleporter_for_relay {
 					.forwarded_xcms
 					.iter()
 					.find(|(destination, _)| {
-						*destination == VersionedLocation::V4(Location::new(1, []))
+						*destination == VersionedLocation::from(Location::parent())
 					})
 					.unwrap();
 				assert_eq!(messages_to_query.len(), 1);
@@ -224,7 +224,7 @@ macro_rules! test_parachain_is_trusted_teleporter_for_relay {
 				let latest_delivery_fees: Assets = delivery_fees.clone().try_into().unwrap();
 				delivery_fees_amount = if let Some(first_asset) = latest_delivery_fees.inner().first() {
 					let Fungible(inner_delivery_fees_amount) = first_asset.fun else {
-						unreachable!("asset is fungible");
+						unreachable!("asset is non-fungible");
 					};
 					inner_delivery_fees_amount
 				} else {

integration-tests/emulated/tests/assets/asset-hub-kusama/src/tests/treasury.rs

@@ -70,11 +70,12 @@ fn spend_ksm_on_asset_hub() {
 		let teleport_call = RuntimeCall::Utility(pallet_utility::Call::<Runtime>::dispatch_as {
 			as_origin: bx!(OriginCaller::system(RawOrigin::Signed(treasury_account))),
 			call: bx!(RuntimeCall::XcmPallet(pallet_xcm::Call::<Runtime>::teleport_assets {
-				dest: bx!(VersionedLocation::V4(asset_hub_location.clone())),
-				beneficiary: bx!(VersionedLocation::V4(treasury_location)),
-				assets: bx!(VersionedAssets::V4(
-					Asset { id: native_asset.clone().into(), fun: treasury_balance.into() }.into()
-				)),
+				dest: bx!(VersionedLocation::from(asset_hub_location.clone())),
+				beneficiary: bx!(VersionedLocation::from(treasury_location)),
+				assets: bx!(VersionedAssets::from(Assets::from(Asset {
+					id: native_asset.clone().into(),
+					fun: treasury_balance.into()
+				}))),
 				fee_asset_item: 0,
 			})),
 		});
@@ -114,7 +115,7 @@ fn spend_ksm_on_asset_hub() {
 				asset_id: native_asset_on_asset_hub.into(),
 			}),
 			amount: treasury_spend_balance,
-			beneficiary: bx!(VersionedLocation::V4(alice_location)),
+			beneficiary: bx!(VersionedLocation::from(alice_location)),
 			valid_from: None,
 		});
 

integration-tests/emulated/tests/assets/asset-hub-kusama/src/tests/xcm_fee_estimation.rs

@@ -76,7 +76,7 @@ fn multi_hop_works() {
 
 	// We get them from the PenpalA closure.
 	let mut delivery_fees_amount = 0;
-	let mut remote_message = VersionedXcm::V4(Xcm(Vec::new()));
+	let mut remote_message = VersionedXcm::from(Xcm(Vec::new()));
 	<PenpalA as TestExt>::execute_with(|| {
 		type Runtime = <PenpalA as Chain>::Runtime;
 		type OriginCaller = <PenpalA as Chain>::OriginCaller;
@@ -89,7 +89,7 @@ fn multi_hop_works() {
 			.forwarded_xcms
 			.iter()
 			.find(|(destination, _)| {
-				*destination == VersionedLocation::V4(Location::new(1, [Parachain(1000)]))
+				*destination == VersionedLocation::from(Location::new(1, [Parachain(1000)]))
 			})
 			.unwrap();
 		assert_eq!(messages_to_query.len(), 1);
@@ -103,7 +103,7 @@ fn multi_hop_works() {
 	// These are set in the AssetHub closure.
 	let mut intermediate_execution_fees = 0;
 	let mut intermediate_delivery_fees_amount = 0;
-	let mut intermediate_remote_message = VersionedXcm::V4(Xcm::<()>(Vec::new()));
+	let mut intermediate_remote_message = VersionedXcm::from(Xcm::<()>(Vec::new()));
 	<AssetHubKusama as TestExt>::execute_with(|| {
 		type Runtime = <AssetHubKusama as Chain>::Runtime;
 		type RuntimeCall = <AssetHubKusama as Chain>::RuntimeCall;
@@ -112,13 +112,14 @@ fn multi_hop_works() {
 		let weight = Runtime::query_xcm_weight(remote_message.clone()).unwrap();
 		intermediate_execution_fees = Runtime::query_weight_to_asset_fee(
 			weight,
-			VersionedAssetId::V4(Location::new(1, []).into()),
+			VersionedAssetId::from(AssetId(Location::parent())),
 		)
 		.unwrap();
 
 		// We have to do this to turn `VersionedXcm<()>` into `VersionedXcm<RuntimeCall>`.
-		let xcm_program =
-			VersionedXcm::V4(Xcm::<RuntimeCall>::from(remote_message.clone().try_into().unwrap()));
+		let xcm_program = VersionedXcm::from(Xcm::<RuntimeCall>::from(
+			remote_message.clone().try_into().unwrap(),
+		));
 
 		// Now we get the delivery fees to the final destination.
 		let result =
@@ -127,7 +128,7 @@ fn multi_hop_works() {
 			.forwarded_xcms
 			.iter()
 			.find(|(destination, _)| {
-				*destination == VersionedLocation::V4(Location::new(1, [Parachain(2001)]))
+				*destination == VersionedLocation::from(Location::new(1, [Parachain(2001)]))
 			})
 			.unwrap();
 		// There's actually two messages here.
@@ -150,9 +151,11 @@ fn multi_hop_works() {
 		type Runtime = <PenpalA as Chain>::Runtime;
 
 		let weight = Runtime::query_xcm_weight(intermediate_remote_message.clone()).unwrap();
-		final_execution_fees =
-			Runtime::query_weight_to_asset_fee(weight, VersionedAssetId::V4(Parent.into()))
-				.unwrap();
+		final_execution_fees = Runtime::query_weight_to_asset_fee(
+			weight,
+			VersionedAssetId::from(AssetId(Parent.into())),
+		)
+		.unwrap();
 	});
 
 	// Dry-running is done.
@@ -220,7 +223,7 @@ fn sender_assertions(test: ParaToParaThroughAHTest) {
 			RuntimeEvent::ForeignAssets(
 				pallet_assets::Event::Burned { asset_id, owner, balance }
 			) => {
-				asset_id: *asset_id == Location::new(1, []),
+				asset_id: *asset_id == Location::parent(),
 				owner: *owner == test.sender.account_id,
 				balance: *balance == test.args.amount,
 			},
@@ -254,7 +257,7 @@ fn receiver_assertions(test: ParaToParaThroughAHTest) {
 			RuntimeEvent::ForeignAssets(
 				pallet_assets::Event::Issued { asset_id, owner, .. }
 			) => {
-				asset_id: *asset_id == Location::new(1, []),
+				asset_id: *asset_id == Location::parent(),
 				owner: *owner == test.receiver.account_id,
 			},
 		]
@@ -264,7 +267,7 @@ fn receiver_assertions(test: ParaToParaThroughAHTest) {
 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 {
-		unreachable!("asset is fungible");
+		unreachable!("asset is non-fungible");
 	};
 	amount
 }
@@ -293,7 +296,7 @@ fn transfer_assets_para_to_para_through_ah_call(
 		dest: bx!(test.args.dest.into()),
 		assets: bx!(test.args.assets.clone().into()),
 		assets_transfer_type: bx!(TransferType::RemoteReserve(asset_hub_location.clone().into())),
-		remote_fees_id: bx!(VersionedAssetId::V4(AssetId(Location::new(1, [])))),
+		remote_fees_id: bx!(VersionedAssetId::from(AssetId(Location::parent()))),
 		fees_transfer_type: bx!(TransferType::RemoteReserve(asset_hub_location.into())),
 		custom_xcm_on_dest: bx!(VersionedXcm::from(custom_xcm_on_dest)),
 		weight_limit: test.args.weight_limit,

integration-tests/emulated/tests/assets/asset-hub-polkadot/src/tests/xcm_fee_estimation.rs

@@ -77,7 +77,7 @@ fn multi_hop_works() {
 
 	// We get them from the PenpalB closure.
 	let mut delivery_fees_amount = 0;
-	let mut remote_message = VersionedXcm::V4(Xcm(Vec::new()));
+	let mut remote_message = VersionedXcm::from(Xcm(Vec::new()));
 	<PenpalB as TestExt>::execute_with(|| {
 		type Runtime = <PenpalB as Chain>::Runtime;
 		type OriginCaller = <PenpalB as Chain>::OriginCaller;
@@ -90,7 +90,7 @@ fn multi_hop_works() {
 			.forwarded_xcms
 			.iter()
 			.find(|(destination, _)| {
-				*destination == VersionedLocation::V4(Location::new(1, [Parachain(1000)]))
+				*destination == VersionedLocation::from(Location::new(1, [Parachain(1000)]))
 			})
 			.unwrap();
 		assert_eq!(messages_to_query.len(), 1);
@@ -104,7 +104,7 @@ fn multi_hop_works() {
 	// These are set in the AssetHub closure.
 	let mut intermediate_execution_fees = 0;
 	let mut intermediate_delivery_fees_amount = 0;
-	let mut intermediate_remote_message = VersionedXcm::V4(Xcm::<()>(Vec::new()));
+	let mut intermediate_remote_message = VersionedXcm::from(Xcm::<()>(Vec::new()));
 	<AssetHubPolkadot as TestExt>::execute_with(|| {
 		type Runtime = <AssetHubPolkadot as Chain>::Runtime;
 		type RuntimeCall = <AssetHubPolkadot as Chain>::RuntimeCall;
@@ -113,13 +113,14 @@ fn multi_hop_works() {
 		let weight = Runtime::query_xcm_weight(remote_message.clone()).unwrap();
 		intermediate_execution_fees = Runtime::query_weight_to_asset_fee(
 			weight,
-			VersionedAssetId::V4(Location::new(1, []).into()),
+			VersionedAssetId::from(AssetId(Location::parent())),
 		)
 		.unwrap();
 
 		// We have to do this to turn `VersionedXcm<()>` into `VersionedXcm<RuntimeCall>`.
-		let xcm_program =
-			VersionedXcm::V4(Xcm::<RuntimeCall>::from(remote_message.clone().try_into().unwrap()));
+		let xcm_program = VersionedXcm::from(Xcm::<RuntimeCall>::from(
+			remote_message.clone().try_into().unwrap(),
+		));
 
 		// Now we get the delivery fees to the final destination.
 		let result =
@@ -128,7 +129,7 @@ fn multi_hop_works() {
 			.forwarded_xcms
 			.iter()
 			.find(|(destination, _)| {
-				*destination == VersionedLocation::V4(Location::new(1, [Parachain(2000)]))
+				*destination == VersionedLocation::from(Location::new(1, [Parachain(2000)]))
 			})
 			.unwrap();
 		// There's actually two messages here.
@@ -151,9 +152,11 @@ fn multi_hop_works() {
 		type Runtime = <PenpalA as Chain>::Runtime;
 
 		let weight = Runtime::query_xcm_weight(intermediate_remote_message.clone()).unwrap();
-		final_execution_fees =
-			Runtime::query_weight_to_asset_fee(weight, VersionedAssetId::V4(Parent.into()))
-				.unwrap();
+		final_execution_fees = Runtime::query_weight_to_asset_fee(
+			weight,
+			VersionedAssetId::from(AssetId(Location::parent())),
+		)
+		.unwrap();
 	});
 
 	// Dry-running is done.
@@ -221,7 +224,7 @@ fn sender_assertions(test: ParaToParaThroughAHTest) {
 			RuntimeEvent::ForeignAssets(
 				pallet_assets::Event::Burned { asset_id, owner, balance }
 			) => {
-				asset_id: *asset_id == Location::new(1, []),
+				asset_id: *asset_id == Location::parent(),
 				owner: *owner == test.sender.account_id,
 				balance: *balance == test.args.amount,
 			},
@@ -255,7 +258,7 @@ fn receiver_assertions(test: ParaToParaThroughAHTest) {
 			RuntimeEvent::ForeignAssets(
 				pallet_assets::Event::Issued { asset_id, owner, .. }
 			) => {
-				asset_id: *asset_id == Location::new(1, []),
+				asset_id: *asset_id == Location::parent(),
 				owner: *owner == test.receiver.account_id,
 			},
 		]
@@ -265,7 +268,7 @@ fn receiver_assertions(test: ParaToParaThroughAHTest) {
 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 {
-		unreachable!("asset is fungible");
+		unreachable!("asset is non-fungible");
 	};
 	amount
 }
@@ -294,7 +297,7 @@ fn transfer_assets_para_to_para_through_ah_call(
 		dest: bx!(test.args.dest.into()),
 		assets: bx!(test.args.assets.clone().into()),
 		assets_transfer_type: bx!(TransferType::RemoteReserve(asset_hub_location.clone().into())),
-		remote_fees_id: bx!(VersionedAssetId::V4(AssetId(Location::new(1, [])))),
+		remote_fees_id: bx!(VersionedAssetId::from(AssetId(Location::parent()))),
 		fees_transfer_type: bx!(TransferType::RemoteReserve(asset_hub_location.into())),
 		custom_xcm_on_dest: bx!(VersionedXcm::from(custom_xcm_on_dest)),
 		weight_limit: test.args.weight_limit,