Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

Validate MultiLocation for asset-registry #224

Merged
merged 6 commits into from
Jun 12, 2023
Merged

Validate MultiLocation for asset-registry #224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
46ff168
Validate location for register_reserve_asset
valentinfernandez1 Jun 9, 2023
a9eccf9
fmt
valentinfernandez1 Jun 9, 2023
8ccaf79
fmt
valentinfernandez1 Jun 9, 2023
63318ba
fmt
valentinfernandez1 Jun 9, 2023
cda8efe
validator refactor
valentinfernandez1 Jun 9, 2023
9da09ef
fmt
valentinfernandez1 Jun 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 28 additions & 9 deletions pallets/asset-registry/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,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]
Expand Down Expand Up @@ -114,13 +114,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::<T>::WrongMultiLocation
);

Expand Down Expand Up @@ -153,6 +147,31 @@ pub mod pallet {
}
}

impl<T: Config> Pallet<T> {
//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<T: Config> xcm_primitives::AssetMultiLocationGetter<AssetIdOf<T>> for Pallet<T> {
fn get_asset_multi_location(asset_id: AssetIdOf<T>) -> Option<MultiLocation> {
AssetIdMultiLocation::<T>::get(asset_id)
Expand Down
235 changes: 169 additions & 66 deletions pallets/asset-registry/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,84 +29,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::<Test>::get(LOCAL_ASSET_ID),
Some(STATEMINE_ASSET_MULTI_LOCATION)
);
assert_eq!(
AssetMultiLocationId::<Test>::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::<Test>::get(LOCAL_ASSET_ID),
Some(STATEMINE_ASSET_MULTI_LOCATION)
);
assert_eq!(
AssetMultiLocationId::<Test>::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::<Test>::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::<Test>::AssetAlreadyRegistered
);
});
}

#[test]
fn valid_locations_succced() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add one more tests to validate relay chain native token?

MultiLocation { parents: 1, interior: Here

Copy link
Contributor Author

@valentinfernandez1 valentinfernandez1 Jun 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using Here will not work since the check is based on how this is suggested by the XCM-Format.

chain/PalletInstance() for a Frame chain with a single-asset pallet instance (such as an instance of the Balances pallet).

In order for that to work it would need to be MultiLocation { parents: 1, interior: PalletInstance(4)} where the id 4 references the rococo pallet balances.

So I'm not sure whether we should tweak the validation or maybe I`m missing something. Including that is pretty simple but I just want to make sure that we are getting it right conceptually.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But in PolkadotJS we are allowed to choose Here for the Concrete identifiers.

Screenshot 2023-06-10 at 08 24 25

My guess is that it should work

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fixed with the commit I added yesterday.

        let (split_multilocation, last_junction) = location.clone().split_last_interior();

	let check = matches!(
		last_junction,
		Some(AccountId32 { .. }) |
			Some(AccountKey20 { .. }) |
			Some(PalletInstance(_)) |
			None
	);

If the Junctions is Here the function split_interior() will return None.

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::<Test>::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::<Test>::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::<Test>::WrongMultiLocation
);

assert_noop!(
AssetRegistry::register_reserve_asset(
RuntimeOrigin::root(),
LOCAL_ASSET_ID,
invalid_general_index,
),
Error::<Test>::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::<Test>::get(LOCAL_ASSET_ID).is_none());
assert!(AssetMultiLocationId::<Test>::get(STATEMINE_ASSET_MULTI_LOCATION).is_none());
});
}
assert_ok!(AssetRegistry::unregister_reserve_asset(
RuntimeOrigin::root(),
LOCAL_ASSET_ID
));

assert!(AssetIdMultiLocation::<Test>::get(LOCAL_ASSET_ID).is_none());
assert!(AssetMultiLocationId::<Test>::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::<Test>::AssetIsNotRegistered
);
});
#[test]
fn cannot_register_unregistered_asset() {
new_test_ext().execute_with(|| {
assert_noop!(
AssetRegistry::unregister_reserve_asset(RuntimeOrigin::root(), LOCAL_ASSET_ID),
Error::<Test>::AssetIsNotRegistered
);
});
}
}