From 743575ee135a90496fe1d5460099d9a5bc0f0da0 Mon Sep 17 00:00:00 2001 From: ghzlatarev Date: Tue, 22 Mar 2022 16:50:10 +0200 Subject: [PATCH 1/4] Get rid of expect() in the runtime Signed-off-by: ghzlatarev --- runtime/dolphin/tests/xcm_tests.rs | 180 ++++++++++++++++++++++++++++- runtime/primitives/src/xcm.rs | 3 +- 2 files changed, 178 insertions(+), 5 deletions(-) diff --git a/runtime/dolphin/tests/xcm_tests.rs b/runtime/dolphin/tests/xcm_tests.rs index ba81c6aaf..9e9abfafc 100644 --- a/runtime/dolphin/tests/xcm_tests.rs +++ b/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::AssetRegistarMetadata { + 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::::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::AssetRegistarMetadata { + 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::::ErrorCreatingAsset + ); + }); +} + #[test] fn send_para_a_custom_asset_to_para_b() { let a_currency_id: u32 = 0; @@ -1053,6 +1224,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()), @@ -1151,7 +1323,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 @@ -1207,7 +1379,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 diff --git a/runtime/primitives/src/xcm.rs b/runtime/primitives/src/xcm.rs index f1a3afd8a..fab511c07 100644 --- a/runtime/primitives/src/xcm.rs +++ b/runtime/primitives/src/xcm.rs @@ -278,7 +278,8 @@ 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!("mint_into failed with {:?}", err)) + .ok(); } } Err(_) => log::debug!( From d98570dd9a27c580e4bb7a49bcbda627bbf88e7a Mon Sep 17 00:00:00 2001 From: ghzlatarev Date: Tue, 22 Mar 2022 19:09:21 +0200 Subject: [PATCH 2/4] Filter out all XTokens extrinsics until transfer() until they are needed Signed-off-by: ghzlatarev --- runtime/dolphin/src/lib.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/runtime/dolphin/src/lib.rs b/runtime/dolphin/src/lib.rs index 9cb47a365..09eaf5512 100644 --- a/runtime/dolphin/src/lib.rs +++ b/runtime/dolphin/src/lib.rs @@ -247,7 +247,13 @@ impl Contains for BaseFilter { | manta_collator_selection::Call::remove_collator{..} | manta_collator_selection::Call::leave_intent{..}) | Call::Balances(_) - | Call::XTokens(_) + // 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::Utility(_) => true, // Filter XCM pallets, we only allow transfer with XTokens. // Filter Assets. Assets should only be accessed by AssetManager. From 48e447df8954efd8ca32a0d1c18791cb7404e923 Mon Sep 17 00:00:00 2001 From: ghzlatarev Date: Thu, 24 Mar 2022 15:50:25 +0200 Subject: [PATCH 3/4] Add log target Signed-off-by: ghzlatarev --- runtime/primitives/src/xcm.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/runtime/primitives/src/xcm.rs b/runtime/primitives/src/xcm.rs index fab511c07..0d4449673 100644 --- a/runtime/primitives/src/xcm.rs +++ b/runtime/primitives/src/xcm.rs @@ -278,7 +278,9 @@ impl< Ok((asset_id, amount)) => { if !amount.is_zero() { Assets::mint_into(asset_id, &ReceiverAccount::get(), amount) - .map_err(|err| log::debug!("mint_into failed with {:?}", err)) + .map_err( + |err| log::debug!(target: "manta-xcm", "mint_into failed with {:?}", err), + ) .ok(); } } From 6dcd1e2659decf3a736410f88c5592539fd1dab3 Mon Sep 17 00:00:00 2001 From: ghzlatarev Date: Mon, 4 Apr 2022 11:47:08 +0300 Subject: [PATCH 4/4] Fix tests Signed-off-by: ghzlatarev --- runtime/dolphin/tests/xcm_tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/dolphin/tests/xcm_tests.rs b/runtime/dolphin/tests/xcm_tests.rs index 0418209f8..9e333875f 100644 --- a/runtime/dolphin/tests/xcm_tests.rs +++ b/runtime/dolphin/tests/xcm_tests.rs @@ -410,7 +410,7 @@ fn send_not_sufficient_asset_from_para_a_to_para_b() { let dest_weight = 1600_000u64; let fee_at_b = calculate_fee(units_per_second_at_b, dest_weight); - let asset_metadata = parachain::AssetRegistarMetadata { + let asset_metadata = parachain::AssetRegistrarMetadata { name: b"ParaAToken".to_vec(), symbol: b"ParaA".to_vec(), decimals: 18, @@ -545,7 +545,7 @@ fn register_with_is_sufficient_false_and_zero_min_balance_should_fail() { X1(Parachain(1)), ))); - let asset_metadata = parachain::AssetRegistarMetadata { + let asset_metadata = parachain::AssetRegistrarMetadata { name: b"ParaAToken".to_vec(), symbol: b"ParaA".to_vec(), decimals: 18,