changed foreign asset mapping from hash to sequential index

Author: 0xRVE

substrate/frame/assets/precompiles/src/foreign_assets.rs

@@ -24,12 +24,11 @@ pub use pallet::*;
 pub struct ForeignAssetId<T, I = ()>(PhantomData<(T, I)>);
 impl<T: Config, I> AssetsCallback<T::AssetId, T::AccountId> for ForeignAssetId<T, I>
 where
-	T::AssetId: ToAssetIndex,
 	T: pallet::Config<ForeignAssetId = T::AssetId> + pallet_assets::Config<I>,
 	I: 'static,
 {
 	fn created(id: &T::AssetId, _: &T::AccountId) -> Result<(), ()> {
-		pallet::Pallet::<T>::insert_asset_mapping(id.to_asset_index(), id)
+		pallet::Pallet::<T>::insert_asset_mapping(id).map(|_| ())
 	}
 
 	fn destroyed(id: &T::AssetId) -> Result<(), ()> {
@@ -38,27 +37,6 @@ where
 	}
 }
 
-/// Trait to convert various types to u32 asset index used internally by the pallet.
-pub trait ToAssetIndex {
-	fn to_asset_index(&self) -> u32;
-}
-
-/// Implemented for trust-backed assets and pool assets.
-impl ToAssetIndex for u32 {
-	fn to_asset_index(&self) -> u32 {
-		*self
-	}
-}
-
-/// Implemented for foreign assets.
-impl ToAssetIndex for xcm::v5::Location {
-	fn to_asset_index(&self) -> u32 {
-		use codec::Encode;
-		let h = sp_core::hashing::blake2_256(&self.encode());
-		u32::from_le_bytes([h[0], h[1], h[2], h[3]])
-	}
-}
-
 #[frame_support::pallet]
 pub mod pallet {
 	use super::*;
@@ -68,17 +46,17 @@ pub mod pallet {
 	pub trait Config: frame_system::Config {
 		/// The foreign asset ID type. This must match the `AssetId` type used by the
 		/// `pallet_assets` instance for foreign assets.
-		type ForeignAssetId: Member
-			+ Parameter
-			+ Clone
-			+ MaybeSerializeDeserialize
-			+ MaxEncodedLen
-			+ ToAssetIndex;
+		type ForeignAssetId: Member + Parameter + Clone + MaybeSerializeDeserialize + MaxEncodedLen;
 	}
 
 	#[pallet::pallet]
 	pub struct Pallet<T>(_);
 
+	/// The next available asset index for foreign assets.
+	/// This is incremented each time a new foreign asset mapping is created.
+	#[pallet::storage]
+	pub type NextAssetIndex<T: Config> = StorageValue<_, u32, ValueQuery>;
+
 	/// Mapping an asset index (derived from the precompile address) to a `ForeignAssetId`.
 	#[pallet::storage]
 	pub type AssetIndexToForeignAssetId<T: Config> =
@@ -100,21 +78,31 @@ pub mod pallet {
 			ForeignAssetIdToAssetIndex::<T>::get(asset_id)
 		}
 
-		pub fn insert_asset_mapping(
-			asset_index: u32,
-			asset_id: &T::ForeignAssetId,
-		) -> Result<(), ()> {
-			if AssetIndexToForeignAssetId::<T>::contains_key(asset_index) {
-				log::debug!(target: LOG_TARGET, "Asset index {:?} already mapped", asset_index);
-				return Err(());
-			}
+		/// Get the next available asset index without incrementing it.
+		pub fn next_asset_index() -> u32 {
+			NextAssetIndex::<T>::get()
+		}
+
+		/// Insert a new asset mapping, allocating a sequential index.
+		/// Returns the allocated asset index on success.
+		pub fn insert_asset_mapping(asset_id: &T::ForeignAssetId) -> Result<u32, ()> {
 			if ForeignAssetIdToAssetIndex::<T>::contains_key(asset_id) {
 				log::debug!(target: LOG_TARGET, "Asset id {:?} already mapped", asset_id);
 				return Err(());
 			}
+
+			let asset_index = NextAssetIndex::<T>::get();
+			let next_index = asset_index.checked_add(1).ok_or_else(|| {
+				log::error!(target: LOG_TARGET, "Asset index overflow");
+				()
+			})?;
+
 			AssetIndexToForeignAssetId::<T>::insert(asset_index, asset_id.clone());
 			ForeignAssetIdToAssetIndex::<T>::insert(asset_id, asset_index);
-			Ok(())
+			NextAssetIndex::<T>::put(next_index);
+
+			log::debug!(target: LOG_TARGET, "Mapped asset {:?} to index {:?}", asset_id, asset_index);
+			Ok(asset_index)
 		}
 
 		pub fn remove_asset_mapping(asset_id: &T::ForeignAssetId) {

substrate/frame/assets/precompiles/src/foreign_assets_tests.rs

@@ -19,7 +19,7 @@
 
 use super::*;
 use crate::{
-	foreign_assets::{pallet::Pallet as ForeignAssetsPallet, ForeignAssetId, ToAssetIndex},
+	foreign_assets::{pallet::Pallet as ForeignAssetsPallet, ForeignAssetId},
 	mock::{new_test_ext, Test},
 };
 use frame_support::assert_ok;
@@ -29,10 +29,12 @@ use pallet_assets::AssetsCallback;
 fn asset_mapping_insert_works() {
 	new_test_ext().execute_with(|| {
 		let asset_id = 123u32;
-		let asset_index = asset_id.to_asset_index();
 
-		// Insert mapping
-		assert_ok!(ForeignAssetsPallet::<Test>::insert_asset_mapping(asset_index, &asset_id));
+		// Insert mapping - now returns the allocated index
+		let asset_index = ForeignAssetsPallet::<Test>::insert_asset_mapping(&asset_id).unwrap();
+
+		// First asset should get index 0
+		assert_eq!(asset_index, 0);
 
 		// Verify both directions of lookup work
 		assert_eq!(ForeignAssetsPallet::<Test>::asset_id_of(asset_index), Some(asset_id));
@@ -41,49 +43,51 @@ fn asset_mapping_insert_works() {
 }
 
 #[test]
-fn asset_mapping_insert_prevents_duplicate_index() {
+fn asset_mapping_insert_sequential_indices() {
 	new_test_ext().execute_with(|| {
-		let asset_id1 = 123u32;
-		let asset_id2 = 456u32;
-		let asset_index = 100u32;
-
-		// Insert first mapping
-		assert_ok!(ForeignAssetsPallet::<Test>::insert_asset_mapping(asset_index, &asset_id1));
-
-		// Try to insert different asset with same index - should fail
-		assert!(ForeignAssetsPallet::<Test>::insert_asset_mapping(asset_index, &asset_id2).is_err());
-
-		// Original mapping should still exist
-		assert_eq!(ForeignAssetsPallet::<Test>::asset_id_of(asset_index), Some(asset_id1));
+		let asset_id1 = 100u32;
+		let asset_id2 = 200u32;
+		let asset_id3 = 300u32;
+
+		// Insert mappings - should get sequential indices
+		let index1 = ForeignAssetsPallet::<Test>::insert_asset_mapping(&asset_id1).unwrap();
+		let index2 = ForeignAssetsPallet::<Test>::insert_asset_mapping(&asset_id2).unwrap();
+		let index3 = ForeignAssetsPallet::<Test>::insert_asset_mapping(&asset_id3).unwrap();
+
+		assert_eq!(index1, 0);
+		assert_eq!(index2, 1);
+		assert_eq!(index3, 2);
+
+		// Verify lookups
+		assert_eq!(ForeignAssetsPallet::<Test>::asset_id_of(0), Some(asset_id1));
+		assert_eq!(ForeignAssetsPallet::<Test>::asset_id_of(1), Some(asset_id2));
+		assert_eq!(ForeignAssetsPallet::<Test>::asset_id_of(2), Some(asset_id3));
 	});
 }
 
 #[test]
 fn asset_mapping_insert_prevents_duplicate_asset_id() {
 	new_test_ext().execute_with(|| {
 		let asset_id = 123u32;
-		let asset_index1 = 100u32;
-		let asset_index2 = 200u32;
 
 		// Insert first mapping
-		assert_ok!(ForeignAssetsPallet::<Test>::insert_asset_mapping(asset_index1, &asset_id));
+		let index1 = ForeignAssetsPallet::<Test>::insert_asset_mapping(&asset_id).unwrap();
 
-		// Try to insert same asset with different index - should fail
-		assert!(ForeignAssetsPallet::<Test>::insert_asset_mapping(asset_index2, &asset_id).is_err());
+		// Try to insert same asset again - should fail
+		assert!(ForeignAssetsPallet::<Test>::insert_asset_mapping(&asset_id).is_err());
 
 		// Original mapping should still exist
-		assert_eq!(ForeignAssetsPallet::<Test>::asset_index_of(&asset_id), Some(asset_index1));
+		assert_eq!(ForeignAssetsPallet::<Test>::asset_index_of(&asset_id), Some(index1));
 	});
 }
 
 #[test]
 fn asset_mapping_remove_works() {
 	new_test_ext().execute_with(|| {
 		let asset_id = 123u32;
-		let asset_index = asset_id.to_asset_index();
 
 		// Insert and verify
-		assert_ok!(ForeignAssetsPallet::<Test>::insert_asset_mapping(asset_index, &asset_id));
+		let asset_index = ForeignAssetsPallet::<Test>::insert_asset_mapping(&asset_id).unwrap();
 		assert_eq!(ForeignAssetsPallet::<Test>::asset_id_of(asset_index), Some(asset_id));
 
 		// Remove mapping
@@ -113,14 +117,14 @@ fn foreign_asset_callback_created_inserts_mapping() {
 	new_test_ext().execute_with(|| {
 		let asset_id = 42u32;
 		let owner = 123u64;
-		let asset_index = asset_id.to_asset_index();
 
 		// Simulate asset creation callback
 		assert_ok!(ForeignAssetId::<Test>::created(&asset_id, &owner));
 
-		// Verify mapping was inserted
+		// Verify mapping was inserted with sequential index (first asset gets 0)
+		let asset_index = ForeignAssetsPallet::<Test>::asset_index_of(&asset_id).unwrap();
+		assert_eq!(asset_index, 0);
 		assert_eq!(ForeignAssetsPallet::<Test>::asset_id_of(asset_index), Some(asset_id));
-		assert_eq!(ForeignAssetsPallet::<Test>::asset_index_of(&asset_id), Some(asset_index));
 	});
 }
 
@@ -129,10 +133,10 @@ fn foreign_asset_callback_destroyed_removes_mapping() {
 	new_test_ext().execute_with(|| {
 		let asset_id = 42u32;
 		let owner = 123u64;
-		let asset_index = asset_id.to_asset_index();
 
 		// Setup: create asset mapping via callback
 		assert_ok!(ForeignAssetId::<Test>::created(&asset_id, &owner));
+		let asset_index = ForeignAssetsPallet::<Test>::asset_index_of(&asset_id).unwrap();
 		assert_eq!(ForeignAssetsPallet::<Test>::asset_id_of(asset_index), Some(asset_id));
 
 		// Simulate asset destruction callback
@@ -148,10 +152,9 @@ fn foreign_asset_callback_destroyed_removes_mapping() {
 fn foreign_asset_id_extractor_works_with_valid_mapping() {
 	new_test_ext().execute_with(|| {
 		let asset_id = 555u32;
-		let asset_index = 0x0000_0001u32; // Will be in first 4 bytes of address
 
-		// Setup mapping
-		assert_ok!(ForeignAssetsPallet::<Test>::insert_asset_mapping(asset_index, &asset_id));
+		// Setup mapping - gets index 0
+		let asset_index = ForeignAssetsPallet::<Test>::insert_asset_mapping(&asset_id).unwrap();
 
 		// Create address with the asset index in the first 4 bytes
 		let mut address = [0u8; 20];
@@ -194,3 +197,26 @@ fn foreign_id_config_matcher_works() {
 	non_matching_address[16..18].copy_from_slice(&0x0120u16.to_be_bytes());
 	assert!(!matcher.matches(&non_matching_address));
 }
+
+#[test]
+fn next_asset_index_increments_correctly() {
+	new_test_ext().execute_with(|| {
+		// Initial state
+		assert_eq!(ForeignAssetsPallet::<Test>::next_asset_index(), 0);
+
+		// Insert first asset
+		let index1 = ForeignAssetsPallet::<Test>::insert_asset_mapping(&100u32).unwrap();
+		assert_eq!(index1, 0);
+		assert_eq!(ForeignAssetsPallet::<Test>::next_asset_index(), 1);
+
+		// Insert second asset
+		let index2 = ForeignAssetsPallet::<Test>::insert_asset_mapping(&200u32).unwrap();
+		assert_eq!(index2, 1);
+		assert_eq!(ForeignAssetsPallet::<Test>::next_asset_index(), 2);
+
+		// Insert third asset
+		let index3 = ForeignAssetsPallet::<Test>::insert_asset_mapping(&300u32).unwrap();
+		assert_eq!(index3, 2);
+		assert_eq!(ForeignAssetsPallet::<Test>::next_asset_index(), 3);
+	});
+}

substrate/frame/assets/precompiles/src/lib.rs

@@ -48,9 +48,7 @@ mod mock;
 #[cfg(test)]
 mod tests;
 
-pub use foreign_assets::{
-	pallet, pallet::Config as ForeignAssetsConfig, ForeignAssetId, ToAssetIndex,
-};
+pub use foreign_assets::{pallet, pallet::Config as ForeignAssetsConfig, ForeignAssetId};
 pub use migration::{MigrateForeignAssetPrecompileMappings, MigrationState};
 
 /// Mean of extracting the asset id from the precompile address.