From 77d2e26f5f7900b6331577714f3add51ba00eedf Mon Sep 17 00:00:00 2001 From: Giles Cope Date: Fri, 2 Jun 2023 11:30:14 +0100 Subject: [PATCH 1/3] integrity test was only working for u32 asset ids. --- frame/asset-conversion/src/lib.rs | 36 ++++++++++++++--------------- frame/asset-conversion/src/mock.rs | 2 +- frame/asset-conversion/src/types.rs | 4 ++-- 3 files changed, 20 insertions(+), 22 deletions(-) diff --git a/frame/asset-conversion/src/lib.rs b/frame/asset-conversion/src/lib.rs index 3db694a88e2f4..bdd60b6135527 100644 --- a/frame/asset-conversion/src/lib.rs +++ b/frame/asset-conversion/src/lib.rs @@ -201,7 +201,7 @@ pub mod pallet { type WeightInfo: WeightInfo; /// The benchmarks need a way to create asset ids from u32s. - #[cfg(feature = "runtime-benchmarks")] + #[cfg(any(test, feature = "runtime-benchmarks"))] type BenchmarkHelper: BenchmarkHelper; } @@ -352,24 +352,22 @@ pub mod pallet { #[pallet::hooks] impl Hooks for Pallet { fn integrity_test() { - sp_std::if_std! { - sp_io::TestExternalities::new_empty().execute_with(|| { - // ensure that the `AccountId` is set properly and doesn't generate the same - // pool account for different pool ids. - let native = T::MultiAssetIdConverter::get_native(); - // Decode the asset ids from bytes. - let asset_1 = T::MultiAssetIdConverter::into_multiasset_id(&T::AssetId::decode(&mut vec![0u8, 0, 0, 1].as_slice()).unwrap()); - let asset_2 = T::MultiAssetIdConverter::into_multiasset_id(&T::AssetId::decode(&mut vec![255u8, 255, 255, 255].as_slice()).unwrap()); - assert!(asset_1 != asset_2, "unfortunatly decoded to be the same asset."); - let pool_account_1 = Self::get_pool_account(&(native.clone(), asset_1)); - let pool_account_2 = Self::get_pool_account(&(native, asset_2)); - assert!(sp_std::mem::size_of::() >= sp_std::mem::size_of::()); - assert!( - pool_account_1 != pool_account_2, - "AccountId should be set at least to u128" - ); - }); - } + #[cfg(test)] + sp_io::TestExternalities::new_empty().execute_with(|| { + // ensure that the `AccountId` is set properly and doesn't generate the same + // pool account for different pool ids. + let native = T::MultiAssetIdConverter::get_native(); + let asset_1 = T::MultiAssetIdConverter::into_multiasset_id(&T::BenchmarkHelper::asset_id(1)); + let asset_2 = T::MultiAssetIdConverter::into_multiasset_id(&T::BenchmarkHelper::asset_id(2)); + assert!(asset_1 != asset_2, "unfortunatly decoded to be the same asset."); + let pool_account_1 = Self::get_pool_account(&(native.clone(), asset_1)); + let pool_account_2 = Self::get_pool_account(&(native, asset_2)); + assert!(sp_std::mem::size_of::() >= sp_std::mem::size_of::()); + assert!( + pool_account_1 != pool_account_2, + "AccountId should be set at least to u128" + ); + }); } } diff --git a/frame/asset-conversion/src/mock.rs b/frame/asset-conversion/src/mock.rs index 34d2eeb273ca8..9483cfca25c76 100644 --- a/frame/asset-conversion/src/mock.rs +++ b/frame/asset-conversion/src/mock.rs @@ -178,7 +178,7 @@ impl Config for Test { type MultiAssetId = NativeOrAssetId; type MultiAssetIdConverter = NativeOrAssetIdConverter; - #[cfg(feature = "runtime-benchmarks")] + #[cfg(any(test, feature = "runtime-benchmarks"))] type BenchmarkHelper = (); } diff --git a/frame/asset-conversion/src/types.rs b/frame/asset-conversion/src/types.rs index 837b14be283ef..b370d410f82a2 100644 --- a/frame/asset-conversion/src/types.rs +++ b/frame/asset-conversion/src/types.rs @@ -50,13 +50,13 @@ pub trait MultiAssetIdConverter { } /// Benchmark Helper -#[cfg(feature = "runtime-benchmarks")] +#[cfg(any(test, feature = "runtime-benchmarks"))] pub trait BenchmarkHelper { /// Returns an asset id from a given integer. fn asset_id(asset_id: u32) -> AssetId; } -#[cfg(feature = "runtime-benchmarks")] +#[cfg(any(test, feature = "runtime-benchmarks"))] impl BenchmarkHelper for () where AssetId: From, From ac4e1ce20a012937ab1dccb58af410739dd11744 Mon Sep 17 00:00:00 2001 From: Giles Cope Date: Fri, 2 Jun 2023 11:35:36 +0100 Subject: [PATCH 2/3] cargo fmt --- frame/asset-conversion/src/lib.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/frame/asset-conversion/src/lib.rs b/frame/asset-conversion/src/lib.rs index bdd60b6135527..424b00c41e3fb 100644 --- a/frame/asset-conversion/src/lib.rs +++ b/frame/asset-conversion/src/lib.rs @@ -357,8 +357,10 @@ pub mod pallet { // ensure that the `AccountId` is set properly and doesn't generate the same // pool account for different pool ids. let native = T::MultiAssetIdConverter::get_native(); - let asset_1 = T::MultiAssetIdConverter::into_multiasset_id(&T::BenchmarkHelper::asset_id(1)); - let asset_2 = T::MultiAssetIdConverter::into_multiasset_id(&T::BenchmarkHelper::asset_id(2)); + let asset_1 = + T::MultiAssetIdConverter::into_multiasset_id(&T::BenchmarkHelper::asset_id(1)); + let asset_2 = + T::MultiAssetIdConverter::into_multiasset_id(&T::BenchmarkHelper::asset_id(2)); assert!(asset_1 != asset_2, "unfortunatly decoded to be the same asset."); let pool_account_1 = Self::get_pool_account(&(native.clone(), asset_1)); let pool_account_2 = Self::get_pool_account(&(native, asset_2)); From fc763fbc886ae72c2c3d6c669df15bd29904b1c5 Mon Sep 17 00:00:00 2001 From: Squirrel Date: Fri, 2 Jun 2023 13:04:34 +0100 Subject: [PATCH 3/3] Update frame/asset-conversion/src/lib.rs Co-authored-by: Oliver Tale-Yazdi --- frame/asset-conversion/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/asset-conversion/src/lib.rs b/frame/asset-conversion/src/lib.rs index 424b00c41e3fb..266c4eb77449f 100644 --- a/frame/asset-conversion/src/lib.rs +++ b/frame/asset-conversion/src/lib.rs @@ -361,7 +361,7 @@ pub mod pallet { T::MultiAssetIdConverter::into_multiasset_id(&T::BenchmarkHelper::asset_id(1)); let asset_2 = T::MultiAssetIdConverter::into_multiasset_id(&T::BenchmarkHelper::asset_id(2)); - assert!(asset_1 != asset_2, "unfortunatly decoded to be the same asset."); + assert!(asset_1 != asset_2, "must return different assets for different ids."); let pool_account_1 = Self::get_pool_account(&(native.clone(), asset_1)); let pool_account_2 = Self::get_pool_account(&(native, asset_2)); assert!(sp_std::mem::size_of::() >= sp_std::mem::size_of::());