From 3b9efa96d88651808af678f86fbcb8f7ad40c29d Mon Sep 17 00:00:00 2001 From: Ron Date: Tue, 27 May 2025 16:09:12 +0800 Subject: [PATCH 1/2] Snowbridge: Remove asset location check for compatibility (#8473) The `TokenIdOf` [convert](https://github.com/paritytech/polkadot-sdk/blob/4b83d24f4bc96a7b17964be94b178dd7b8f873b5/bridges/snowbridge/primitives/core/src/location.rs#L40) is XCM version-agnostic, meaning we will get the same token ID for both V5 and legacy V4 asset. However, the extra check is unnecessary, as the`ConvertAssetId::convert(&token_id).ok_or(InvalidAsset)?;` alone is sufficient to verify whether the token is registered. (cherry picked from commit 64aed49226dc592c456cb7aac126c84d5c0d78e8) --- .../pallets/inbound-queue-v2/src/mock.rs | 9 +- .../pallets/inbound-queue/src/mock.rs | 9 +- .../snowbridge/pallets/system-v2/src/lib.rs | 14 +-- .../snowbridge/pallets/system-v2/src/tests.rs | 4 - bridges/snowbridge/pallets/system/src/lib.rs | 17 +-- .../snowbridge/pallets/system/src/tests.rs | 69 ++++++++++- .../primitives/inbound-queue/src/v1.rs | 10 +- .../inbound-queue/src/v2/converter.rs | 16 +-- .../outbound-queue/src/v1/converter/mod.rs | 10 +- .../outbound-queue/src/v1/converter/tests.rs | 7 +- .../src/v2/converter/convert.rs | 7 +- .../outbound-queue/src/v2/converter/mod.rs | 4 +- .../outbound-queue/src/v2/converter/tests.rs | 7 +- .../test-utils/src/mock_converter.rs | 12 +- .../src/tests/snowbridge.rs | 108 +++++++++++++++++- .../src/bridge_to_ethereum_config.rs | 13 --- prdoc/pr_8473.prdoc | 35 ++++++ 17 files changed, 253 insertions(+), 98 deletions(-) create mode 100644 prdoc/pr_8473.prdoc diff --git a/bridges/snowbridge/pallets/inbound-queue-v2/src/mock.rs b/bridges/snowbridge/pallets/inbound-queue-v2/src/mock.rs index 919a6182af23a..436bbe4e5290c 100644 --- a/bridges/snowbridge/pallets/inbound-queue-v2/src/mock.rs +++ b/bridges/snowbridge/pallets/inbound-queue-v2/src/mock.rs @@ -14,7 +14,7 @@ use snowbridge_core::TokenId; use snowbridge_inbound_queue_primitives::{v2::MessageToXcm, Log, Proof, VerificationError}; use sp_core::H160; use sp_runtime::{ - traits::{IdentityLookup, MaybeEquivalence}, + traits::{IdentityLookup, MaybeConvert}, BuildStorage, }; use sp_std::{convert::From, default::Default, marker::PhantomData}; @@ -76,13 +76,10 @@ impl BenchmarkHelper for Test { } pub struct MockTokenIdConvert; -impl MaybeEquivalence for MockTokenIdConvert { - fn convert(_id: &TokenId) -> Option { +impl MaybeConvert for MockTokenIdConvert { + fn maybe_convert(_id: TokenId) -> Option { Some(Location::parent()) } - fn convert_back(_loc: &Location) -> Option { - None - } } pub struct MockAccountLocationConverter(PhantomData); diff --git a/bridges/snowbridge/pallets/inbound-queue/src/mock.rs b/bridges/snowbridge/pallets/inbound-queue/src/mock.rs index 1cad021f8eb7a..4a81e4243c12d 100644 --- a/bridges/snowbridge/pallets/inbound-queue/src/mock.rs +++ b/bridges/snowbridge/pallets/inbound-queue/src/mock.rs @@ -13,7 +13,7 @@ use snowbridge_core::{ use snowbridge_inbound_queue_primitives::{v1::MessageToXcm, Log, Proof, VerificationError}; use sp_core::{H160, H256}; use sp_runtime::{ - traits::{IdentifyAccount, IdentityLookup, MaybeEquivalence, Verify}, + traits::{IdentifyAccount, IdentityLookup, MaybeConvert, Verify}, BuildStorage, FixedU128, MultiSignature, }; use sp_std::{convert::From, default::Default}; @@ -214,13 +214,10 @@ impl TransactAsset for SuccessfulTransactor { } pub struct MockTokenIdConvert; -impl MaybeEquivalence for MockTokenIdConvert { - fn convert(_id: &TokenId) -> Option { +impl MaybeConvert for MockTokenIdConvert { + fn maybe_convert(_id: TokenId) -> Option { Some(Location::parent()) } - fn convert_back(_loc: &Location) -> Option { - None - } } impl inbound_queue::Config for Test { diff --git a/bridges/snowbridge/pallets/system-v2/src/lib.rs b/bridges/snowbridge/pallets/system-v2/src/lib.rs index e7bf10bfe2436..2d411c69197ee 100644 --- a/bridges/snowbridge/pallets/system-v2/src/lib.rs +++ b/bridges/snowbridge/pallets/system-v2/src/lib.rs @@ -36,10 +36,10 @@ use snowbridge_outbound_queue_primitives::{ v2::{Command, Initializer, Message, SendMessage}, OperatingMode, SendError, }; -use snowbridge_pallet_system::{ForeignToNativeId, NativeToForeignId}; +use snowbridge_pallet_system::ForeignToNativeId; use sp_core::{H160, H256}; use sp_io::hashing::blake2_256; -use sp_runtime::traits::MaybeEquivalence; +use sp_runtime::traits::MaybeConvert; use sp_std::prelude::*; use xcm::prelude::*; use xcm_executor::traits::ConvertLocation; @@ -199,7 +199,6 @@ pub mod pallet { .ok_or(Error::::LocationConversionFailed)?; if !ForeignToNativeId::::contains_key(token_id) { - NativeToForeignId::::insert(location.clone(), token_id); ForeignToNativeId::::insert(token_id, location.clone()); } @@ -254,12 +253,9 @@ pub mod pallet { } } - impl MaybeEquivalence for Pallet { - fn convert(foreign_id: &TokenId) -> Option { - ForeignToNativeId::::get(foreign_id) - } - fn convert_back(location: &Location) -> Option { - NativeToForeignId::::get(location) + impl MaybeConvert for Pallet { + fn maybe_convert(foreign_id: TokenId) -> Option { + snowbridge_pallet_system::Pallet::::maybe_convert(foreign_id) } } } diff --git a/bridges/snowbridge/pallets/system-v2/src/tests.rs b/bridges/snowbridge/pallets/system-v2/src/tests.rs index 20e4107683254..53b80fa80f31b 100644 --- a/bridges/snowbridge/pallets/system-v2/src/tests.rs +++ b/bridges/snowbridge/pallets/system-v2/src/tests.rs @@ -135,10 +135,6 @@ fn register_all_tokens_succeeds() { let foreign_token_id = EthereumSystemV2::location_to_message_origin(tc.native.clone()).unwrap(); - assert_eq!( - NativeToForeignId::::get(reanchored_location.clone()), - Some(foreign_token_id) - ); assert_eq!( ForeignToNativeId::::get(foreign_token_id), Some(reanchored_location.clone()) diff --git a/bridges/snowbridge/pallets/system/src/lib.rs b/bridges/snowbridge/pallets/system/src/lib.rs index c092177abdc3d..b8715318d56ad 100644 --- a/bridges/snowbridge/pallets/system/src/lib.rs +++ b/bridges/snowbridge/pallets/system/src/lib.rs @@ -52,7 +52,7 @@ use snowbridge_outbound_queue_primitives::{ }; use sp_core::{RuntimeDebug, H160, H256}; use sp_io::hashing::blake2_256; -use sp_runtime::{traits::MaybeEquivalence, DispatchError, SaturatedConversion}; +use sp_runtime::{traits::MaybeConvert, DispatchError, SaturatedConversion}; use sp_std::prelude::*; use xcm::prelude::*; use xcm_executor::traits::ConvertLocation; @@ -230,12 +230,7 @@ pub mod pallet { /// Lookup table for foreign token ID to native location relative to ethereum #[pallet::storage] pub type ForeignToNativeId = - StorageMap<_, Blake2_128Concat, TokenId, xcm::v5::Location, OptionQuery>; - - /// Lookup table for native location relative to ethereum to foreign token ID - #[pallet::storage] - pub type NativeToForeignId = - StorageMap<_, Blake2_128Concat, xcm::v5::Location, TokenId, OptionQuery>; + StorageMap<_, Blake2_128Concat, TokenId, Location, OptionQuery>; #[pallet::genesis_config] #[derive(frame_support::DefaultNoBound)] @@ -493,7 +488,6 @@ pub mod pallet { .ok_or(Error::::LocationConversionFailed)?; if !ForeignToNativeId::::contains_key(token_id) { - NativeToForeignId::::insert(location.clone(), token_id); ForeignToNativeId::::insert(token_id, location.clone()); } @@ -534,12 +528,9 @@ pub mod pallet { } } - impl MaybeEquivalence for Pallet { - fn convert(foreign_id: &TokenId) -> Option { + impl MaybeConvert for Pallet { + fn maybe_convert(foreign_id: TokenId) -> Option { ForeignToNativeId::::get(foreign_id) } - fn convert_back(location: &Location) -> Option { - NativeToForeignId::::get(location) - } } } diff --git a/bridges/snowbridge/pallets/system/src/tests.rs b/bridges/snowbridge/pallets/system/src/tests.rs index 93a1fba38d584..f41f0533fa60a 100644 --- a/bridges/snowbridge/pallets/system/src/tests.rs +++ b/bridges/snowbridge/pallets/system/src/tests.rs @@ -269,7 +269,6 @@ fn register_all_tokens_succeeds() { Default::default() )); - assert_eq!(NativeToForeignId::::get(tc.reanchored.clone()), Some(tc.foreign)); assert_eq!(ForeignToNativeId::::get(tc.foreign), Some(tc.reanchored.clone())); System::assert_last_event(RuntimeEvent::EthereumSystem(Event::::RegisterToken { @@ -301,3 +300,71 @@ fn register_ethereum_native_token_fails() { ); }); } + +#[test] +fn check_pna_token_id_compatibility() { + let test_cases = vec![ + // DOT + RegisterTokenTestCase { + native: Location::parent(), + reanchored: Location::new(1, GlobalConsensus(Polkadot)), + foreign: hex!("4e241583d94b5d48a27a22064cd49b2ed6f5231d2d950e432f9b7c2e0ade52b2") + .into(), + }, + // GLMR (Some Polkadot parachain currency) + RegisterTokenTestCase { + native: Location::new(1, [Parachain(2004)]), + reanchored: Location::new(1, [GlobalConsensus(Polkadot), Parachain(2004)]), + foreign: hex!("34c08fc90409b6924f0e8eabb7c2aaa0c749e23e31adad9f6d217b577737fafb") + .into(), + }, + // USDT + RegisterTokenTestCase { + native: Location::new(1, [Parachain(1000), PalletInstance(50), GeneralIndex(1984)]), + reanchored: Location::new( + 1, + [ + GlobalConsensus(Polkadot), + Parachain(1000), + PalletInstance(50), + GeneralIndex(1984), + ], + ), + foreign: hex!("14b0579be12d7d7f9971f1d4b41f0e88384b9b74799b0150d4aa6cd01afb4444") + .into(), + }, + // KSM + RegisterTokenTestCase { + native: Location::new(2, [GlobalConsensus(Kusama)]), + reanchored: Location::new(1, [GlobalConsensus(Kusama)]), + foreign: hex!("03b6054d0c576dd8391e34e1609cf398f68050c23009d19ce93c000922bcd852") + .into(), + }, + // KAR (Some Kusama parachain currency) + RegisterTokenTestCase { + native: Location::new(2, [GlobalConsensus(Kusama), Parachain(2000)]), + reanchored: Location::new(1, [GlobalConsensus(Kusama), Parachain(2000)]), + foreign: hex!("d3e39ad6ea4cee68c9741181e94098823b2ea34a467577d0875c036f0fce5be0") + .into(), + }, + ]; + for tc in test_cases.iter() { + new_test_ext(true).execute_with(|| { + let origin = RuntimeOrigin::root(); + let versioned_location: VersionedLocation = tc.native.clone().into(); + + assert_ok!(EthereumSystem::register_token( + origin, + Box::new(versioned_location), + Default::default() + )); + + assert_eq!(ForeignToNativeId::::get(tc.foreign), Some(tc.reanchored.clone())); + + System::assert_last_event(RuntimeEvent::EthereumSystem(Event::::RegisterToken { + location: tc.reanchored.clone().into(), + foreign_token_id: tc.foreign, + })); + }); + } +} diff --git a/bridges/snowbridge/primitives/inbound-queue/src/v1.rs b/bridges/snowbridge/primitives/inbound-queue/src/v1.rs index cc0edda11913b..6eba682f72f74 100644 --- a/bridges/snowbridge/primitives/inbound-queue/src/v1.rs +++ b/bridges/snowbridge/primitives/inbound-queue/src/v1.rs @@ -9,7 +9,7 @@ use frame_support::{traits::tokens::Balance as BalanceT, PalletError}; use scale_info::TypeInfo; use snowbridge_core::TokenId; use sp_core::{Get, RuntimeDebug, H160, H256}; -use sp_runtime::{traits::MaybeEquivalence, MultiAddress}; +use sp_runtime::{traits::MaybeConvert, MultiAddress}; use sp_std::prelude::*; use xcm::prelude::{Junction::AccountKey20, *}; @@ -104,7 +104,7 @@ pub struct MessageToXcm< CreateAssetCall: Get, CreateAssetDeposit: Get, Balance: BalanceT, - ConvertAssetId: MaybeEquivalence, + ConvertAssetId: MaybeConvert, EthereumUniversalLocation: Get, GlobalAssetHubLocation: Get, { @@ -171,7 +171,7 @@ where InboundQueuePalletInstance: Get, Balance: BalanceT + From, AccountId: Into<[u8; 32]>, - ConvertAssetId: MaybeEquivalence, + ConvertAssetId: MaybeConvert, EthereumUniversalLocation: Get, GlobalAssetHubLocation: Get, { @@ -230,7 +230,7 @@ where InboundQueuePalletInstance: Get, Balance: BalanceT + From, AccountId: Into<[u8; 32]>, - ConvertAssetId: MaybeEquivalence, + ConvertAssetId: MaybeConvert, EthereumUniversalLocation: Get, GlobalAssetHubLocation: Get, { @@ -426,7 +426,7 @@ where let total_fee_asset: Asset = (Location::parent(), asset_hub_fee).into(); let asset_loc = - ConvertAssetId::convert(&token_id).ok_or(ConvertMessageError::InvalidToken)?; + ConvertAssetId::maybe_convert(token_id).ok_or(ConvertMessageError::InvalidToken)?; let mut reanchored_asset_loc = asset_loc.clone(); reanchored_asset_loc diff --git a/bridges/snowbridge/primitives/inbound-queue/src/v2/converter.rs b/bridges/snowbridge/primitives/inbound-queue/src/v2/converter.rs index 6b0e9b6efb2f2..43c086e2f7448 100644 --- a/bridges/snowbridge/primitives/inbound-queue/src/v2/converter.rs +++ b/bridges/snowbridge/primitives/inbound-queue/src/v2/converter.rs @@ -10,7 +10,7 @@ use frame_support::ensure; use snowbridge_core::TokenId; use sp_core::{Get, RuntimeDebug, H160}; use sp_io::hashing::blake2_256; -use sp_runtime::{traits::MaybeEquivalence, MultiAddress}; +use sp_runtime::{traits::MaybeConvert, MultiAddress}; use sp_std::prelude::*; use xcm::{ prelude::{Junction::*, *}, @@ -103,7 +103,7 @@ where CreateAssetDeposit: Get, EthereumNetwork: Get, InboundQueueLocation: Get, - ConvertAssetId: MaybeEquivalence, + ConvertAssetId: MaybeConvert, GatewayProxyAddress: Get, EthereumUniversalLocation: Get, AssetHubFromEthereum: Get, @@ -163,7 +163,7 @@ where assets.push(AssetTransfer::ReserveDeposit(asset)); }, EthereumAsset::ForeignTokenERC20 { token_id, value } => { - let asset_loc = ConvertAssetId::convert(&token_id) + let asset_loc = ConvertAssetId::maybe_convert(*token_id) .ok_or(ConvertMessageError::InvalidAsset)?; let reanchored_asset_loc = asset_loc .reanchored(&AssetHubFromEthereum::get(), &EthereumUniversalLocation::get()) @@ -329,7 +329,7 @@ where CreateAssetDeposit: Get, EthereumNetwork: Get, InboundQueueLocation: Get, - ConvertAssetId: MaybeEquivalence, + ConvertAssetId: MaybeConvert, GatewayProxyAddress: Get, EthereumUniversalLocation: Get, AssetHubFromEthereum: Get, @@ -402,7 +402,6 @@ mod tests { add_location_override, reanchor_to_ethereum, LocationIdConvert, }; use sp_core::{H160, H256}; - use sp_runtime::traits::MaybeEquivalence; const GATEWAY_ADDRESS: [u8; 20] = hex!["eda338e4dc46038493b885327842fd3e301cab39"]; parameter_types! { @@ -420,11 +419,8 @@ mod tests { } pub struct MockFailedTokenConvert; - impl MaybeEquivalence for MockFailedTokenConvert { - fn convert(_id: &TokenId) -> Option { - None - } - fn convert_back(_loc: &Location) -> Option { + impl MaybeConvert for MockFailedTokenConvert { + fn maybe_convert(_id: TokenId) -> Option { None } } diff --git a/bridges/snowbridge/primitives/outbound-queue/src/v1/converter/mod.rs b/bridges/snowbridge/primitives/outbound-queue/src/v1/converter/mod.rs index 5a9631acfb5d9..7f6275fa331e7 100644 --- a/bridges/snowbridge/primitives/outbound-queue/src/v1/converter/mod.rs +++ b/bridges/snowbridge/primitives/outbound-queue/src/v1/converter/mod.rs @@ -13,7 +13,7 @@ use super::message::{Command, Message, SendMessage}; use frame_support::{ensure, traits::Get}; use snowbridge_core::{AgentId, ChannelId, ParaId, TokenId, TokenIdOf}; use sp_core::{H160, H256}; -use sp_runtime::traits::MaybeEquivalence; +use sp_runtime::traits::MaybeConvert; use sp_std::{iter::Peekable, marker::PhantomData, prelude::*}; use xcm::prelude::*; use xcm_executor::traits::{ConvertLocation, ExportXcm}; @@ -48,7 +48,7 @@ where EthereumNetwork: Get, OutboundQueue: SendMessage, AgentHashedDescription: ConvertLocation, - ConvertAssetId: MaybeEquivalence, + ConvertAssetId: MaybeConvert, { type Ticket = (Vec, XcmHash); @@ -194,7 +194,7 @@ struct XcmConverter<'a, ConvertAssetId, Call> { } impl<'a, ConvertAssetId, Call> XcmConverter<'a, ConvertAssetId, Call> where - ConvertAssetId: MaybeEquivalence, + ConvertAssetId: MaybeConvert, { fn new(message: &'a Xcm, ethereum_network: NetworkId, agent_id: AgentId) -> Self { Self { @@ -413,9 +413,7 @@ where let token_id = TokenIdOf::convert_location(&asset_id).ok_or(InvalidAsset)?; - let expected_asset_id = ConvertAssetId::convert(&token_id).ok_or(InvalidAsset)?; - - ensure!(asset_id == expected_asset_id, InvalidAsset); + ConvertAssetId::maybe_convert(token_id).ok_or(InvalidAsset)?; // Check if there is a SetTopic and skip over it if found. let topic_id = match_expression!(self.next()?, SetTopic(id), id).ok_or(SetTopicExpected)?; diff --git a/bridges/snowbridge/primitives/outbound-queue/src/v1/converter/tests.rs b/bridges/snowbridge/primitives/outbound-queue/src/v1/converter/tests.rs index b2b6e3c1d9954..9ce5bb8b29d30 100644 --- a/bridges/snowbridge/primitives/outbound-queue/src/v1/converter/tests.rs +++ b/bridges/snowbridge/primitives/outbound-queue/src/v1/converter/tests.rs @@ -62,13 +62,10 @@ impl SendMessageFeeProvider for MockErrOutboundQueue { } pub struct MockTokenIdConvert; -impl MaybeEquivalence for MockTokenIdConvert { - fn convert(_id: &TokenId) -> Option { +impl MaybeConvert for MockTokenIdConvert { + fn maybe_convert(_id: TokenId) -> Option { Some(Location::new(1, [GlobalConsensus(ByGenesis(WESTEND_GENESIS_HASH))])) } - fn convert_back(_loc: &Location) -> Option { - None - } } #[test] diff --git a/bridges/snowbridge/primitives/outbound-queue/src/v2/converter/convert.rs b/bridges/snowbridge/primitives/outbound-queue/src/v2/converter/convert.rs index e8b38c4e698f4..f64554e42756a 100644 --- a/bridges/snowbridge/primitives/outbound-queue/src/v2/converter/convert.rs +++ b/bridges/snowbridge/primitives/outbound-queue/src/v2/converter/convert.rs @@ -14,7 +14,7 @@ use crate::v2::{ use crate::v2::convert::XcmConverterError::{AssetResolutionFailed, FilterDoesNotConsumeAllAssets}; use sp_core::H160; -use sp_runtime::traits::MaybeEquivalence; +use sp_runtime::traits::MaybeConvert; use sp_std::{iter::Peekable, marker::PhantomData, prelude::*}; use xcm::prelude::*; use xcm_executor::traits::ConvertLocation; @@ -64,7 +64,7 @@ pub struct XcmConverter<'a, ConvertAssetId, Call> { } impl<'a, ConvertAssetId, Call> XcmConverter<'a, ConvertAssetId, Call> where - ConvertAssetId: MaybeEquivalence, + ConvertAssetId: MaybeConvert, { pub fn new(message: &'a Xcm, ethereum_network: NetworkId) -> Self { Self { @@ -174,8 +174,7 @@ where // Ensure PNA already registered let token_id = TokenIdOf::convert_location(&asset_id).ok_or(InvalidAsset)?; - let expected_asset_id = ConvertAssetId::convert(&token_id).ok_or(InvalidAsset)?; - ensure!(asset_id == expected_asset_id, InvalidAsset); + ConvertAssetId::maybe_convert(token_id).ok_or(InvalidAsset)?; commands.push(Command::MintForeignToken { token_id, recipient, amount }); } diff --git a/bridges/snowbridge/primitives/outbound-queue/src/v2/converter/mod.rs b/bridges/snowbridge/primitives/outbound-queue/src/v2/converter/mod.rs index 5da68a01626ca..c4459a05c7c51 100644 --- a/bridges/snowbridge/primitives/outbound-queue/src/v2/converter/mod.rs +++ b/bridges/snowbridge/primitives/outbound-queue/src/v2/converter/mod.rs @@ -15,7 +15,7 @@ use frame_support::{ traits::{Contains, Get, ProcessMessageError}, }; use snowbridge_core::{ParaId, TokenId}; -use sp_runtime::traits::MaybeEquivalence; +use sp_runtime::traits::MaybeConvert; use sp_std::{marker::PhantomData, ops::ControlFlow, prelude::*}; use xcm::prelude::*; use xcm_builder::{CreateMatcher, ExporterFor, MatchXcm}; @@ -53,7 +53,7 @@ where UniversalLocation: Get, EthereumNetwork: Get, OutboundQueue: SendMessage, - ConvertAssetId: MaybeEquivalence, + ConvertAssetId: MaybeConvert, AssetHubParaId: Get, { type Ticket = (Vec, XcmHash); diff --git a/bridges/snowbridge/primitives/outbound-queue/src/v2/converter/tests.rs b/bridges/snowbridge/primitives/outbound-queue/src/v2/converter/tests.rs index 58501066743bf..bb5a88c73c809 100644 --- a/bridges/snowbridge/primitives/outbound-queue/src/v2/converter/tests.rs +++ b/bridges/snowbridge/primitives/outbound-queue/src/v2/converter/tests.rs @@ -62,13 +62,10 @@ impl SendMessageFeeProvider for MockErrOutboundQueue { } pub struct MockTokenIdConvert; -impl MaybeEquivalence for MockTokenIdConvert { - fn convert(_id: &TokenId) -> Option { +impl MaybeConvert for MockTokenIdConvert { + fn maybe_convert(_id: TokenId) -> Option { Some(Location::new(1, [GlobalConsensus(ByGenesis(WESTEND_GENESIS_HASH))])) } - fn convert_back(_loc: &Location) -> Option { - None - } } #[test] diff --git a/bridges/snowbridge/test-utils/src/mock_converter.rs b/bridges/snowbridge/test-utils/src/mock_converter.rs index 333460814ec99..d40e3cf97bd49 100644 --- a/bridges/snowbridge/test-utils/src/mock_converter.rs +++ b/bridges/snowbridge/test-utils/src/mock_converter.rs @@ -2,7 +2,7 @@ // SPDX-FileCopyrightText: 2023 Snowfork use codec::Encode; -use frame_support::sp_runtime::traits::MaybeEquivalence; +use frame_support::sp_runtime::traits::MaybeConvert; use snowbridge_core::TokenIdOf; use sp_core::H256; use std::{cell::RefCell, collections::HashMap}; @@ -35,12 +35,8 @@ pub fn reanchor_to_ethereum( } pub struct LocationIdConvert; -impl MaybeEquivalence for LocationIdConvert { - fn convert(id: &H256) -> Option { - IDENTIFIER_TO_LOCATION.with(|b| b.borrow().get(id).and_then(|l| Option::from(l.clone()))) - } - fn convert_back(lol: &Location) -> Option { - LOCATION_TO_IDENTIFIER - .with(|b| b.borrow().get(&lol.encode()).and_then(|id| Option::from(*id))) +impl MaybeConvert for LocationIdConvert { + fn maybe_convert(id: H256) -> Option { + IDENTIFIER_TO_LOCATION.with(|b| b.borrow().get(&id).and_then(|l| Option::from(l.clone()))) } } diff --git a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/snowbridge.rs b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/snowbridge.rs index d2064b4f3c16b..49777d25b094b 100644 --- a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/snowbridge.rs +++ b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/snowbridge.rs @@ -22,7 +22,8 @@ use crate::{ tests::{ penpal_emulated_chain::penpal_runtime, snowbridge_common::{ - bridged_roc_at_ah_westend, ethereum, register_roc_on_bh, snowbridge_sovereign, + bridge_hub, bridged_roc_at_ah_westend, ethereum, register_roc_on_bh, + snowbridge_sovereign, }, snowbridge_v2_outbound_from_rococo::create_foreign_on_ah_westend, }, @@ -2019,3 +2020,108 @@ fn transfer_roc_from_ah_with_transfer_and_then() { ); }); } + +#[test] +fn register_pna_in_v5_while_transfer_in_v4_should_work() { + let assethub_sovereign = BridgeHubWestend::sovereign_account_id_of( + BridgeHubWestend::sibling_location_of(AssetHubWestend::para_id()), + ); + BridgeHubWestend::fund_accounts(vec![(assethub_sovereign.clone(), INITIAL_FUND)]); + + let asset_id: Location = Location { parents: 1, interior: [].into() }; + let expected_asset_id: Location = Location { + parents: 1, + interior: [GlobalConsensus(ByGenesis(WESTEND_GENESIS_HASH))].into(), + }; + + let _expected_token_id = TokenIdOf::convert_location(&expected_asset_id).unwrap(); + + let ethereum_sovereign: AccountId = snowbridge_sovereign(); + + // Register token in V5 + BridgeHubWestend::execute_with(|| { + type RuntimeOrigin = ::RuntimeOrigin; + type RuntimeEvent = ::RuntimeEvent; + + assert_ok!(::Balances::force_set_balance( + RuntimeOrigin::root(), + sp_runtime::MultiAddress::Id(BridgeHubWestendSender::get()), + INITIAL_FUND * 10, + )); + + assert_ok!(::EthereumSystem::register_token( + RuntimeOrigin::root(), + Box::new(VersionedLocation::from(asset_id.clone())), + AssetMetadata { + name: "wnd".as_bytes().to_vec().try_into().unwrap(), + symbol: "wnd".as_bytes().to_vec().try_into().unwrap(), + decimals: 12, + }, + )); + // Check that a message was sent to Ethereum to create the agent + assert_expected_events!( + BridgeHubWestend, + vec![RuntimeEvent::EthereumSystem(snowbridge_pallet_system::Event::RegisterToken { .. }) => {},] + ); + }); + + AssetHubWestend::force_xcm_version(bridge_hub(), 4); + AssetHubWestend::force_xcm_version(ethereum(), 4); + AssetHubWestend::force_default_xcm_version(Some(4)); + BridgeHubWestend::force_default_xcm_version(Some(4)); + + // Send token to Ethereum in V4 fomat + AssetHubWestend::execute_with(|| { + // LTS is V4 + use xcm::lts::{Junction::*, NetworkId::*, *}; + type RuntimeOrigin = ::RuntimeOrigin; + type RuntimeEvent = ::RuntimeEvent; + + let assets = vec![Asset { + id: AssetId(Location::parent()), + fun: Fungibility::try_from(Fungible(TOKEN_AMOUNT)).unwrap(), + }]; + let versioned_assets = VersionedAssets::V4(Assets::from(assets)); + + let destination = VersionedLocation::V4(Location::new( + 2, + [GlobalConsensus(Ethereum { chain_id: SEPOLIA_ID })], + )); + + let beneficiary = VersionedLocation::V4(Location::new( + 0, + [AccountKey20 { network: None, key: ETHEREUM_DESTINATION_ADDRESS.into() }], + )); + + assert_ok!(::PolkadotXcm::limited_reserve_transfer_assets( + RuntimeOrigin::signed(AssetHubWestendSender::get()), + Box::new(destination), + Box::new(beneficiary), + Box::new(versioned_assets), + 0, + Unlimited, + )); + + let events = AssetHubWestend::events(); + // Check that the native asset transferred to some reserved account(sovereign of Ethereum) + assert!( + events.iter().any(|event| matches!( + event, + RuntimeEvent::Balances(pallet_balances::Event::Transfer { amount, to, ..}) + if *amount == TOKEN_AMOUNT && *to == ethereum_sovereign.clone(), + )), + "native token reserved to Ethereum sovereign account." + ); + }); + + // Check that the transfer token back to Ethereum message was queue in the Ethereum + // Outbound Queue + BridgeHubWestend::execute_with(|| { + type RuntimeEvent = ::RuntimeEvent; + + assert_expected_events!( + BridgeHubWestend, + vec![RuntimeEvent::EthereumOutboundQueue(snowbridge_pallet_outbound_queue::Event::MessageQueued{ .. }) => {},] + ); + }); +} diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/bridge_to_ethereum_config.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/bridge_to_ethereum_config.rs index 2ccbfd8847232..d01021dacd063 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/bridge_to_ethereum_config.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/bridge_to_ethereum_config.rs @@ -372,7 +372,6 @@ pub mod benchmark_helpers { } pub(crate) mod migrations { - use alloc::vec::Vec; use frame_support::pallet_prelude::*; use snowbridge_core::TokenId; @@ -399,18 +398,6 @@ pub(crate) mod migrations { }; snowbridge_pallet_system::ForeignToNativeId::::translate_values(translate_westend); - let old_keys = OldNativeToForeignId::::iter_keys().collect::>(); - for old_key in old_keys { - if let Some(old_val) = OldNativeToForeignId::::get(&old_key) { - snowbridge_pallet_system::NativeToForeignId::::insert( - &xcm::v5::Location::try_from(old_key.clone()).expect("valid location"), - old_val, - ); - } - OldNativeToForeignId::::remove(old_key); - weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 2)); - } - weight } } diff --git a/prdoc/pr_8473.prdoc b/prdoc/pr_8473.prdoc new file mode 100644 index 0000000000000..f4a2294249454 --- /dev/null +++ b/prdoc/pr_8473.prdoc @@ -0,0 +1,35 @@ +title: 'Snowbridge: Remove asset location check' +doc: +- audience: Runtime Dev + description: |- + Since the TokenIdOf conversion is XCM version-agnostic and we store the TokenId as the key in storage, + checking whether the key exists is sufficient to verify if the token is registered. + There is no need to verify the asset location. +crates: +- name: snowbridge-outbound-queue-primitives + bump: patch + validate: false +- name: snowbridge-inbound-queue-primitives + bump: patch + validate: false +- name: snowbridge-test-utils + bump: patch + validate: false +- name: snowbridge-pallet-inbound-queue + bump: patch + validate: false +- name: snowbridge-pallet-inbound-queue-v2 + bump: patch + validate: false +- name: snowbridge-pallet-system + bump: patch + validate: false +- name: snowbridge-pallet-system-v2 + bump: patch + validate: false +- name: bridge-hub-westend-runtime + bump: patch + validate: false +- name: bridge-hub-westend-integration-tests + bump: patch + validate: false From 245ede78b8cd041c6c6d43da54f02c366b8af191 Mon Sep 17 00:00:00 2001 From: Ron Date: Wed, 28 May 2025 21:10:21 +0800 Subject: [PATCH 2/2] Fix breaking test (#8690) --- .../tests/bridges/bridge-hub-westend/src/tests/snowbridge.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/snowbridge.rs b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/snowbridge.rs index 49777d25b094b..ac3d700c1b4ec 100644 --- a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/snowbridge.rs +++ b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/snowbridge.rs @@ -2085,7 +2085,7 @@ fn register_pna_in_v5_while_transfer_in_v4_should_work() { let destination = VersionedLocation::V4(Location::new( 2, - [GlobalConsensus(Ethereum { chain_id: SEPOLIA_ID })], + [GlobalConsensus(Ethereum { chain_id: CHAIN_ID })], )); let beneficiary = VersionedLocation::V4(Location::new(