Fix potential expect() panic in runtime code, two xcm must-fail tests, and filter out unneeded XTokens extrinsics

primitives/src/xcm.rs

@@ -278,7 +278,10 @@ impl<
 			Ok((asset_id, amount)) => {
 				if !amount.is_zero() {
 					Assets::mint_into(asset_id, &ReceiverAccount::get(), amount)
-						.expect("`mint_into` cannot generally fail; qed");
+						.map_err(
+							|err| log::debug!(target: "manta-xcm", "mint_into failed with {:?}", err),
+						)
+						.ok();
 				}
 			}
 			Err(_) => log::debug!(

runtime/dolphin/src/lib.rs

@@ -194,7 +194,8 @@ impl Contains<Call> for BaseFilter {
 			Call::Timestamp(_) | Call::ParachainSystem(_) | Call::System(_)
 		) {
 			// always allow core call
-			// pallet-timestamp and parachainSystem could not be filtered because they are used in commuication between releychain and parachain.
+			// pallet-timestamp and parachainSystem could not be filtered
+			// because they are used in communication between releychain and parachain.
 			return true;
 		}
 
@@ -255,10 +256,16 @@ impl Contains<Call> for BaseFilter {
 				| manta_collator_selection::Call::remove_collator{..}
 				| manta_collator_selection::Call::leave_intent{..})
 			| Call::Balances(_)
-			| Call::Preimage(_)
-			| Call::Utility(_)
+			// Everything except transfer() is filtered out until it is practically needed:
+			// orml_xtokens::Call::transfer_with_fee {..}
+			// orml_xtokens::Call::transfer_multiasset {..}
+			// orml_xtokens::Call::transfer_multiasset_with_fee {..}
+			// orml_xtokens::Call::transfer_multicurrencies  {..}
+			// orml_xtokens::Call::transfer_multiassets {..}
+			| Call::XTokens(orml_xtokens::Call::transfer {..})
 			| Call::MantaPay(_)
-			| Call::XTokens(_) => true,
+			| Call::Preimage(_)
+			| Call::Utility(_) => true,
 			// Filter XCM pallets, we only allow transfer with XTokens.
 			// Filter Assets. Assets should only be accessed by AssetManager.
 			// AssetManager is also filtered because all of its extrinsics are callable only by Root,

runtime/dolphin/tests/xcm_tests.rs

@@ -19,7 +19,7 @@
 mod xcm_mock;
 
 use codec::Encode;
-use frame_support::{assert_ok, weights::constants::WEIGHT_PER_SECOND};
+use frame_support::{assert_err, assert_ok, weights::constants::WEIGHT_PER_SECOND};
 use manta_primitives::assets::AssetLocation;
 use xcm::{latest::prelude::*, v2::Response, VersionedMultiLocation, WrapVersion};
 use xcm_mock::{parachain::PALLET_ASSET_INDEX, *};
@@ -304,7 +304,7 @@ fn reserve_transfer_relaychain_to_parachain_a_then_back() {
 fn send_para_a_native_asset_to_para_b() {
 	MockNet::reset();
 
-	// We use an opinioned source location here:
+	// We use an opinionated source location here:
 	// Ideally, we could use `here()`, however, we always prefer to use the location from
 	// `root` when possible.
 	let source_location = AssetLocation(VersionedMultiLocation::V1(MultiLocation::new(
@@ -396,6 +396,177 @@ fn send_para_a_native_asset_to_para_b() {
 	});
 }
 
+#[test]
+fn send_not_sufficient_asset_from_para_a_to_para_b() {
+	MockNet::reset();
+
+	let source_location = AssetLocation(VersionedMultiLocation::V1(MultiLocation::new(
+		1,
+		X1(Parachain(1)),
+	)));
+	let a_currency_id = 0u32;
+	let amount = 8888u128;
+	let units_per_second_at_b = 1_250_000u128;
+	let dest_weight = 1600_000u64;
+	let fee_at_b = calculate_fee(units_per_second_at_b, dest_weight);
+
+	let asset_metadata = parachain::AssetRegistrarMetadata {
+		name: b"ParaAToken".to_vec(),
+		symbol: b"ParaA".to_vec(),
+		decimals: 18,
+		evm_address: None,
+		min_balance: 1,
+		is_frozen: false,
+		is_sufficient: false,
+	};
+
+	// Register ParaA native asset in ParaA
+	ParaA::execute_with(|| {
+		assert_ok!(AssetManager::register_asset(
+			parachain::Origin::root(),
+			source_location.clone(),
+			asset_metadata.clone()
+		));
+		assert_ok!(AssetManager::set_units_per_second(
+			parachain::Origin::root(),
+			a_currency_id,
+			0u128
+		));
+		assert_eq!(
+			Some(a_currency_id),
+			parachain::AssetManager::location_asset_id(source_location.clone())
+		);
+	});
+
+	// Register ParaA native asset in ParaB
+	ParaB::execute_with(|| {
+		assert_ok!(AssetManager::register_asset(
+			parachain::Origin::root(),
+			source_location.clone(),
+			asset_metadata.clone()
+		));
+		assert_ok!(AssetManager::set_units_per_second(
+			parachain::Origin::root(),
+			a_currency_id,
+			units_per_second_at_b
+		));
+		assert_eq!(
+			Some(a_currency_id),
+			parachain::AssetManager::location_asset_id(source_location.clone())
+		);
+	});
+
+	let dest = MultiLocation {
+		parents: 1,
+		interior: X2(
+			Parachain(2),
+			AccountId32 {
+				network: NetworkId::Any,
+				id: ALICE.into(),
+			},
+		),
+	};
+
+	assert!(amount >= fee_at_b);
+	// Transfer ParaA balance to B
+	ParaA::execute_with(|| {
+		assert_ok!(parachain::XTokens::transfer(
+			parachain::Origin::signed(ALICE.into()),
+			parachain::CurrencyId::MantaCurrency(a_currency_id),
+			amount,
+			Box::new(VersionedMultiLocation::V1(dest.clone())),
+			dest_weight
+		));
+		assert_eq!(
+			parachain::Balances::free_balance(&ALICE.into()),
+			INITIAL_BALANCE - amount
+		)
+	});
+
+	ParaB::execute_with(|| {
+		// The total supply should not include the paid fee,
+		// because the XcmFeesAccount had 0 providers with is_sufficient set to false,
+		// so the mint_into() operation for the refund amount failed.
+		assert_eq!(
+			parachain::Assets::total_supply(a_currency_id),
+			amount - fee_at_b
+		);
+		assert_eq!(
+			parachain::Assets::balance(a_currency_id, &ALICE.into()),
+			amount - fee_at_b
+		);
+	});
+
+	// Setting the balance will in effect create the account
+	// incrementing its providers counter to from 0 to 1
+	ParaB::execute_with(|| {
+		assert_ok!(pallet_balances::Pallet::<parachain::Runtime>::set_balance(
+			parachain::Origin::root(),
+			parachain::AssetManager::account_id(),
+			1000000000000000,
+			1000000000000000
+		));
+	});
+
+	ParaA::execute_with(|| {
+		assert_ok!(parachain::XTokens::transfer(
+			parachain::Origin::signed(ALICE.into()),
+			parachain::CurrencyId::MantaCurrency(a_currency_id),
+			amount,
+			Box::new(VersionedMultiLocation::V1(dest.clone())),
+			dest_weight
+		));
+		assert_eq!(
+			parachain::Balances::free_balance(&ALICE.into()),
+			INITIAL_BALANCE - amount * 2
+		)
+	});
+
+	ParaB::execute_with(|| {
+		// This time we expect the total supply to be the full amount
+		// as the refund will be deposited to the XcmFeesAccount
+		assert_eq!(
+			parachain::Assets::total_supply(a_currency_id),
+			(amount - fee_at_b) + amount
+		);
+		assert_eq!(
+			parachain::Assets::balance(a_currency_id, &ALICE.into()),
+			(amount - fee_at_b) * 2
+		);
+	});
+}
+
+#[test]
+fn register_with_is_sufficient_false_and_zero_min_balance_should_fail() {
+	MockNet::reset();
+
+	let source_location = AssetLocation(VersionedMultiLocation::V1(MultiLocation::new(
+		1,
+		X1(Parachain(1)),
+	)));
+
+	let asset_metadata = parachain::AssetRegistrarMetadata {
+		name: b"ParaAToken".to_vec(),
+		symbol: b"ParaA".to_vec(),
+		decimals: 18,
+		evm_address: None,
+		min_balance: 0,
+		is_frozen: false,
+		is_sufficient: false,
+	};
+
+	ParaB::execute_with(|| {
+		assert_err!(
+			AssetManager::register_asset(
+				parachain::Origin::root(),
+				source_location.clone(),
+				asset_metadata.clone()
+			),
+			pallet_asset_manager::Error::<parachain::Runtime>::ErrorCreatingAsset
+		);
+	});
+}
+
 #[test]
 fn send_para_a_custom_asset_to_para_b() {
 	let a_currency_id: u32 = 0;
@@ -1041,6 +1212,7 @@ fn send_para_a_asset_from_para_b_to_para_c_with_trader() {
 	});
 
 	ParaB::execute_with(|| {
+		assert_eq!(parachain::Assets::total_supply(a_currency_id), amount);
 		amount = amount - fee_at_b;
 		assert_eq!(
 			parachain::Assets::balance(a_currency_id, &ALICE.into()),
@@ -1139,7 +1311,7 @@ fn receive_relay_with_insufficient_fee_payment() {
 	ParaA::execute_with(|| {
 		// ALICE gets nothing
 		assert_eq!(parachain::Assets::balance(relay_asset_id, &ALICE.into()), 0);
-		// Asset manager gets nothing, all balance stucks
+		// Asset manager gets nothing, all balance stuck
 		assert_eq!(
 			parachain::Assets::balance(relay_asset_id, AssetManager::account_id()),
 			0
@@ -1195,7 +1367,7 @@ fn receive_relay_should_fail_without_specifying_units_per_second() {
 	ParaA::execute_with(|| {
 		// ALICE gets nothing
 		assert_eq!(parachain::Assets::balance(relay_asset_id, &ALICE.into()), 0);
-		// Asset manager gets nothing, all balance stucks
+		// Asset manager gets nothing, all balance stuck
 		assert_eq!(
 			parachain::Assets::balance(relay_asset_id, AssetManager::account_id()),
 			0