From f42817b182209020806081e70e65792371fbeb4b Mon Sep 17 00:00:00 2001 From: valentinfernandez1 <33705477+valentinfernandez1@users.noreply.github.com> Date: Mon, 12 Jun 2023 05:16:10 -0300 Subject: [PATCH] Validate `MultiLocation` for asset-registry (#224) * Validate location for register_reserve_asset * fmt * fmt * fmt * validator refactor * fmt --- pallets/asset-registry/src/lib.rs | 37 +++-- pallets/asset-registry/src/tests.rs | 235 ++++++++++++++++++++-------- 2 files changed, 197 insertions(+), 75 deletions(-) diff --git a/pallets/asset-registry/src/lib.rs b/pallets/asset-registry/src/lib.rs index 2ee81829..f5545aed 100644 --- a/pallets/asset-registry/src/lib.rs +++ b/pallets/asset-registry/src/lib.rs @@ -22,8 +22,8 @@ pub mod pallet { use frame_system::pallet_prelude::*; use xcm::latest::{ - Junction::{GeneralIndex, PalletInstance, Parachain}, - Junctions, MultiLocation, + Junction::{AccountId32, AccountKey20, GeneralIndex, PalletInstance}, + MultiLocation, }; #[pallet::pallet] @@ -97,13 +97,7 @@ pub mod pallet { // verify MultiLocation is valid ensure!( - matches!( - asset_multi_location, - MultiLocation { - parents: 1, - interior: Junctions::X3(Parachain(_), PalletInstance(_), GeneralIndex(_)) - } - ), + Self::valid_asset_location(&asset_multi_location), Error::::WrongMultiLocation ); @@ -137,6 +131,31 @@ pub mod pallet { } } + impl Pallet { + //Validates that the location points to an asset (Native, Frame based, Erc20) as described + // in the xcm-format: https://github.com/paritytech/xcm-format#concrete-identifiers + fn valid_asset_location(location: &MultiLocation) -> bool { + let (split_multilocation, last_junction) = location.clone().split_last_interior(); + + let check = matches!( + last_junction, + Some(AccountId32 { .. }) | + Some(AccountKey20 { .. }) | + Some(PalletInstance(_)) | + None + ); + + check | + match last_junction { + Some(GeneralIndex(_)) => { + let penultimate = split_multilocation.last(); + matches!(penultimate, Some(PalletInstance(_))) + }, + _ => false, + } + } + } + impl xcm_primitives::AssetMultiLocationGetter> for Pallet { fn get_asset_multi_location(asset_id: AssetIdOf) -> Option { AssetIdMultiLocation::::get(asset_id) diff --git a/pallets/asset-registry/src/tests.rs b/pallets/asset-registry/src/tests.rs index 2b5271ff..4430c08b 100644 --- a/pallets/asset-registry/src/tests.rs +++ b/pallets/asset-registry/src/tests.rs @@ -12,84 +12,187 @@ const STATEMINE_ASSET_MULTI_LOCATION: MultiLocation = MultiLocation { ), }; -#[test] -fn register_reserve_asset_works() { - new_test_ext().execute_with(|| { - assert_ok!(AssetRegistry::register_reserve_asset( - RuntimeOrigin::root(), - LOCAL_ASSET_ID, - STATEMINE_ASSET_MULTI_LOCATION, - )); - - assert_eq!( - AssetIdMultiLocation::::get(LOCAL_ASSET_ID), - Some(STATEMINE_ASSET_MULTI_LOCATION) - ); - assert_eq!( - AssetMultiLocationId::::get(STATEMINE_ASSET_MULTI_LOCATION), - Some(LOCAL_ASSET_ID) - ); - }); -} +mod register_reserve_assest { + use super::*; + + #[test] + fn register_reserve_asset_works() { + new_test_ext().execute_with(|| { + assert_ok!(AssetRegistry::register_reserve_asset( + RuntimeOrigin::root(), + LOCAL_ASSET_ID, + STATEMINE_ASSET_MULTI_LOCATION, + )); + + assert_eq!( + AssetIdMultiLocation::::get(LOCAL_ASSET_ID), + Some(STATEMINE_ASSET_MULTI_LOCATION) + ); + assert_eq!( + AssetMultiLocationId::::get(STATEMINE_ASSET_MULTI_LOCATION), + Some(LOCAL_ASSET_ID) + ); + }); + } -#[test] -fn cannot_register_unexisting_asset() { - new_test_ext().execute_with(|| { - let unexisting_asset_id = 9999; + #[test] + fn cannot_register_unexisting_asset() { + new_test_ext().execute_with(|| { + let unexisting_asset_id = 9999; - assert_noop!( - AssetRegistry::register_reserve_asset( + assert_noop!( + AssetRegistry::register_reserve_asset( + RuntimeOrigin::root(), + unexisting_asset_id, + STATEMINE_ASSET_MULTI_LOCATION, + ), + Error::::AssetDoesNotExist + ); + }); + } + + #[test] + fn cannot_double_register() { + new_test_ext().execute_with(|| { + assert_ok!(AssetRegistry::register_reserve_asset( RuntimeOrigin::root(), - unexisting_asset_id, + LOCAL_ASSET_ID, STATEMINE_ASSET_MULTI_LOCATION, + )); + + assert_noop!( + AssetRegistry::register_reserve_asset( + RuntimeOrigin::root(), + LOCAL_ASSET_ID, + STATEMINE_ASSET_MULTI_LOCATION, + ), + Error::::AssetAlreadyRegistered + ); + }); + } + + #[test] + fn valid_locations_succced() { + let native_frame_based_currency = + MultiLocation { parents: 1, interior: X2(Parachain(1000), PalletInstance(1)) }; + let multiasset_pallet_instance = MultiLocation { + parents: 1, + interior: X3(Parachain(1000), PalletInstance(1), GeneralIndex(2)), + }; + let relay_native_currency = MultiLocation { parents: 1, interior: Junctions::Here }; + let erc20_frame_sm_asset = MultiLocation { + parents: 1, + interior: X3( + Parachain(1000), + PalletInstance(2), + AccountId32 { network: Any, id: [0; 32] }, ), - Error::::AssetDoesNotExist - ); - }); -} + }; + let erc20_ethereum_sm_asset = MultiLocation { + parents: 1, + interior: X2(Parachain(2000), AccountKey20 { network: Any, key: [0; 20] }), + }; -#[test] -fn cannot_double_register() { - new_test_ext().execute_with(|| { - assert_ok!(AssetRegistry::register_reserve_asset( - RuntimeOrigin::root(), - LOCAL_ASSET_ID, - STATEMINE_ASSET_MULTI_LOCATION, - )); - - assert_noop!( - AssetRegistry::register_reserve_asset( + new_test_ext().execute_with(|| { + assert_ok!(AssetRegistry::register_reserve_asset( RuntimeOrigin::root(), LOCAL_ASSET_ID, - STATEMINE_ASSET_MULTI_LOCATION, + native_frame_based_currency, + )); + }); + new_test_ext().execute_with(|| { + assert_ok!(AssetRegistry::register_reserve_asset( + RuntimeOrigin::root(), + LOCAL_ASSET_ID, + multiasset_pallet_instance, + )); + }); + new_test_ext().execute_with(|| { + assert_ok!(AssetRegistry::register_reserve_asset( + RuntimeOrigin::root(), + LOCAL_ASSET_ID, + relay_native_currency, + )); + }); + new_test_ext().execute_with(|| { + assert_ok!(AssetRegistry::register_reserve_asset( + RuntimeOrigin::root(), + LOCAL_ASSET_ID, + erc20_frame_sm_asset, + )); + }); + new_test_ext().execute_with(|| { + assert_ok!(AssetRegistry::register_reserve_asset( + RuntimeOrigin::root(), + LOCAL_ASSET_ID, + erc20_ethereum_sm_asset, + )); + }); + } + + #[test] + fn invalid_locations_fail() { + let governance_location = MultiLocation { + parents: 1, + interior: X2( + Parachain(1000), + Plurality { id: BodyId::Executive, part: BodyPart::Voice }, ), - Error::::AssetAlreadyRegistered - ); - }); + }; + let invalid_general_index = + MultiLocation { parents: 1, interior: X2(Parachain(1000), GeneralIndex(1u128)) }; + + new_test_ext().execute_with(|| { + assert_noop!( + AssetRegistry::register_reserve_asset( + RuntimeOrigin::root(), + LOCAL_ASSET_ID, + governance_location, + ), + Error::::WrongMultiLocation + ); + + assert_noop!( + AssetRegistry::register_reserve_asset( + RuntimeOrigin::root(), + LOCAL_ASSET_ID, + invalid_general_index, + ), + Error::::WrongMultiLocation + ); + }) + } } -#[test] -fn unregister_reserve_asset_works() { - new_test_ext().execute_with(|| { - assert_ok!(AssetRegistry::register_reserve_asset( - RuntimeOrigin::root(), - LOCAL_ASSET_ID, - STATEMINE_ASSET_MULTI_LOCATION, - )); +mod unregister_reserve_asset { + use super::*; - assert_ok!(AssetRegistry::unregister_reserve_asset(RuntimeOrigin::root(), LOCAL_ASSET_ID)); + #[test] + fn unregister_reserve_asset_works() { + new_test_ext().execute_with(|| { + assert_ok!(AssetRegistry::register_reserve_asset( + RuntimeOrigin::root(), + LOCAL_ASSET_ID, + STATEMINE_ASSET_MULTI_LOCATION, + )); - assert!(AssetIdMultiLocation::::get(LOCAL_ASSET_ID).is_none()); - assert!(AssetMultiLocationId::::get(STATEMINE_ASSET_MULTI_LOCATION).is_none()); - }); -} + assert_ok!(AssetRegistry::unregister_reserve_asset( + RuntimeOrigin::root(), + LOCAL_ASSET_ID + )); + + assert!(AssetIdMultiLocation::::get(LOCAL_ASSET_ID).is_none()); + assert!(AssetMultiLocationId::::get(STATEMINE_ASSET_MULTI_LOCATION).is_none()); + }); + } -#[test] -fn cannot_register_unregistered_asset() { - new_test_ext().execute_with(|| { - assert_noop!( - AssetRegistry::unregister_reserve_asset(RuntimeOrigin::root(), LOCAL_ASSET_ID), - Error::::AssetIsNotRegistered - ); - }); + #[test] + fn cannot_register_unregistered_asset() { + new_test_ext().execute_with(|| { + assert_noop!( + AssetRegistry::unregister_reserve_asset(RuntimeOrigin::root(), LOCAL_ASSET_ID), + Error::::AssetIsNotRegistered + ); + }); + } }