From c173455354da2f96b4b03e6101e512d15e53a4b7 Mon Sep 17 00:00:00 2001 From: Alistair Singh Date: Tue, 12 Dec 2023 00:25:35 +0200 Subject: [PATCH 1/7] add basic filter --- .../primitives/router/src/outbound/mod.rs | 37 ++- .../primitives/router/src/outbound/tests.rs | 213 +++++++++--------- 2 files changed, 138 insertions(+), 112 deletions(-) diff --git a/parachain/primitives/router/src/outbound/mod.rs b/parachain/primitives/router/src/outbound/mod.rs index 8260cbe89..16fa64513 100644 --- a/parachain/primitives/router/src/outbound/mod.rs +++ b/parachain/primitives/router/src/outbound/mod.rs @@ -9,7 +9,10 @@ use core::slice::Iter; use codec::{Decode, Encode}; -use frame_support::{ensure, traits::Get}; +use frame_support::{ + ensure, + traits::{ContainsPair, Get}, +}; use snowbridge_core::{ outbound::{AgentExecuteCommand, Command, Message, SendMessage}, ChannelId, ParaId, @@ -24,15 +27,31 @@ pub struct EthereumBlobExporter< EthereumNetwork, OutboundQueue, AgentHashedDescription, ->(PhantomData<(UniversalLocation, EthereumNetwork, OutboundQueue, AgentHashedDescription)>); - -impl ExportXcm - for EthereumBlobExporter -where + ExportFilter, +>( + PhantomData<( + UniversalLocation, + EthereumNetwork, + OutboundQueue, + AgentHashedDescription, + ExportFilter, + )>, +); + +impl + ExportXcm + for EthereumBlobExporter< + UniversalLocation, + EthereumNetwork, + OutboundQueue, + AgentHashedDescription, + ExportFilter, + > where UniversalLocation: Get, EthereumNetwork: Get, OutboundQueue: SendMessage, AgentHashedDescription: ConvertLocation, + ExportFilter: ContainsPair, { type Ticket = (Vec, XcmHash); @@ -111,6 +130,12 @@ where command: Command::AgentExecute { agent_id, command: agent_execute_command }, }; + // filter out message + if !ExportFilter::contains(&dest, &outbound_message) { + log::error!(target: "xcm::ethereum_blob_exporter", "unroutable due to being filtered. '{local_sub_location:?}'"); + return Err(SendError::Unroutable) + } + // validate the message let (ticket, fee) = OutboundQueue::validate(&outbound_message).map_err(|err| { log::error!(target: "xcm::ethereum_blob_exporter", "OutboundQueue validation of message failed. {err:?}"); diff --git a/parachain/primitives/router/src/outbound/tests.rs b/parachain/primitives/router/src/outbound/tests.rs index f64ad8698..b57646853 100644 --- a/parachain/primitives/router/src/outbound/tests.rs +++ b/parachain/primitives/router/src/outbound/tests.rs @@ -1,4 +1,4 @@ -use frame_support::parameter_types; +use frame_support::{assert_ok, parameter_types, traits::Everything}; use hex_literal::hex; use snowbridge_core::outbound::{Fee, SendError, SendMessageFeeProvider}; @@ -66,14 +66,14 @@ fn exporter_validate_with_unknown_network_yields_not_applicable() { let mut destination: Option = None; let mut message: Option> = None; - let result = EthereumBlobExporter::< - UniversalLocation, - BridgedNetwork, - MockOkOutboundQueue, - AgentIdOf, - >::validate( - network, channel, &mut universal_source, &mut destination, &mut message - ); + let result = + EthereumBlobExporter::< + UniversalLocation, + BridgedNetwork, + MockOkOutboundQueue, + AgentIdOf, + Everything, + >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::NotApplicable)); } @@ -85,14 +85,14 @@ fn exporter_validate_with_invalid_destination_yields_missing_argument() { let mut destination: Option = None; let mut message: Option> = None; - let result = EthereumBlobExporter::< - UniversalLocation, - BridgedNetwork, - MockOkOutboundQueue, - AgentIdOf, - >::validate( - network, channel, &mut universal_source, &mut destination, &mut message - ); + let result = + EthereumBlobExporter::< + UniversalLocation, + BridgedNetwork, + MockOkOutboundQueue, + AgentIdOf, + Everything, + >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::MissingArgument)); } @@ -106,14 +106,14 @@ fn exporter_validate_with_x8_destination_yields_not_applicable() { )); let mut message: Option> = None; - let result = EthereumBlobExporter::< - UniversalLocation, - BridgedNetwork, - MockOkOutboundQueue, - AgentIdOf, - >::validate( - network, channel, &mut universal_source, &mut destination, &mut message - ); + let result = + EthereumBlobExporter::< + UniversalLocation, + BridgedNetwork, + MockOkOutboundQueue, + AgentIdOf, + Everything, + >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::NotApplicable)); } @@ -125,14 +125,14 @@ fn exporter_validate_without_universal_source_yields_missing_argument() { let mut destination: Option = Here.into(); let mut message: Option> = None; - let result = EthereumBlobExporter::< - UniversalLocation, - BridgedNetwork, - MockOkOutboundQueue, - AgentIdOf, - >::validate( - network, channel, &mut universal_source, &mut destination, &mut message - ); + let result = + EthereumBlobExporter::< + UniversalLocation, + BridgedNetwork, + MockOkOutboundQueue, + AgentIdOf, + Everything, + >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::MissingArgument)); } @@ -144,14 +144,14 @@ fn exporter_validate_without_global_universal_location_yields_unroutable() { let mut destination: Option = Here.into(); let mut message: Option> = None; - let result = EthereumBlobExporter::< - UniversalLocation, - BridgedNetwork, - MockOkOutboundQueue, - AgentIdOf, - >::validate( - network, channel, &mut universal_source, &mut destination, &mut message - ); + let result = + EthereumBlobExporter::< + UniversalLocation, + BridgedNetwork, + MockOkOutboundQueue, + AgentIdOf, + Everything, + >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::Unroutable)); } @@ -163,14 +163,14 @@ fn exporter_validate_without_global_bridge_location_yields_not_applicable() { let mut destination: Option = Here.into(); let mut message: Option> = None; - let result = EthereumBlobExporter::< - UniversalLocation, - BridgedNetwork, - MockOkOutboundQueue, - AgentIdOf, - >::validate( - network, channel, &mut universal_source, &mut destination, &mut message - ); + let result = + EthereumBlobExporter::< + UniversalLocation, + BridgedNetwork, + MockOkOutboundQueue, + AgentIdOf, + Everything, + >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::NotApplicable)); } @@ -183,14 +183,14 @@ fn exporter_validate_with_remote_universal_source_yields_not_applicable() { let mut destination: Option = Here.into(); let mut message: Option> = None; - let result = EthereumBlobExporter::< - UniversalLocation, - BridgedNetwork, - MockOkOutboundQueue, - AgentIdOf, - >::validate( - network, channel, &mut universal_source, &mut destination, &mut message - ); + let result = + EthereumBlobExporter::< + UniversalLocation, + BridgedNetwork, + MockOkOutboundQueue, + AgentIdOf, + Everything, + >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::NotApplicable)); } @@ -202,14 +202,14 @@ fn exporter_validate_without_para_id_in_source_yields_missing_argument() { let mut destination: Option = Here.into(); let mut message: Option> = None; - let result = EthereumBlobExporter::< - UniversalLocation, - BridgedNetwork, - MockOkOutboundQueue, - AgentIdOf, - >::validate( - network, channel, &mut universal_source, &mut destination, &mut message - ); + let result = + EthereumBlobExporter::< + UniversalLocation, + BridgedNetwork, + MockOkOutboundQueue, + AgentIdOf, + Everything, + >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::MissingArgument)); } @@ -222,14 +222,14 @@ fn exporter_validate_complex_para_id_in_source_yields_missing_argument() { let mut destination: Option = Here.into(); let mut message: Option> = None; - let result = EthereumBlobExporter::< - UniversalLocation, - BridgedNetwork, - MockOkOutboundQueue, - AgentIdOf, - >::validate( - network, channel, &mut universal_source, &mut destination, &mut message - ); + let result = + EthereumBlobExporter::< + UniversalLocation, + BridgedNetwork, + MockOkOutboundQueue, + AgentIdOf, + Everything, + >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::MissingArgument)); } @@ -242,14 +242,14 @@ fn exporter_validate_without_xcm_message_yields_missing_argument() { let mut destination: Option = Here.into(); let mut message: Option> = None; - let result = EthereumBlobExporter::< - UniversalLocation, - BridgedNetwork, - MockOkOutboundQueue, - AgentIdOf, - >::validate( - network, channel, &mut universal_source, &mut destination, &mut message - ); + let result = + EthereumBlobExporter::< + UniversalLocation, + BridgedNetwork, + MockOkOutboundQueue, + AgentIdOf, + Everything, + >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::MissingArgument)); } @@ -289,14 +289,14 @@ fn exporter_validate_with_max_target_fee_yields_unroutable() { .into(), ); - let result = EthereumBlobExporter::< - UniversalLocation, - BridgedNetwork, - MockOkOutboundQueue, - AgentIdOf, - >::validate( - network, channel, &mut universal_source, &mut destination, &mut message - ); + let result = + EthereumBlobExporter::< + UniversalLocation, + BridgedNetwork, + MockOkOutboundQueue, + AgentIdOf, + Everything, + >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::Unroutable)); } @@ -316,14 +316,14 @@ fn exporter_validate_with_unparsable_xcm_yields_unroutable() { let mut message: Option> = Some(vec![WithdrawAsset(fees), BuyExecution { fees: fee, weight_limit: Unlimited }].into()); - let result = EthereumBlobExporter::< - UniversalLocation, - BridgedNetwork, - MockOkOutboundQueue, - AgentIdOf, - >::validate( - network, channel, &mut universal_source, &mut destination, &mut message - ); + let result = + EthereumBlobExporter::< + UniversalLocation, + BridgedNetwork, + MockOkOutboundQueue, + AgentIdOf, + Everything, + >::validate(network, channel, &mut universal_source, &mut destination, &mut message); assert_eq!(result, Err(XcmSendError::Unroutable)); } @@ -362,16 +362,16 @@ fn exporter_validate_xcm_success_case_1() { .into(), ); - let result = EthereumBlobExporter::< - UniversalLocation, - BridgedNetwork, - MockOkOutboundQueue, - AgentIdOf, - >::validate( - network, channel, &mut universal_source, &mut destination, &mut message - ); + let result = + EthereumBlobExporter::< + UniversalLocation, + BridgedNetwork, + MockOkOutboundQueue, + AgentIdOf, + Everything, + >::validate(network, channel, &mut universal_source, &mut destination, &mut message); - assert!(result.is_ok()); + assert_ok!(result); } #[test] @@ -381,6 +381,7 @@ fn exporter_deliver_with_submit_failure_yields_unroutable() { BridgedNetwork, MockErrOutboundQueue, AgentIdOf, + Everything, >::deliver((hex!("deadbeef").to_vec(), XcmHash::default())); assert_eq!(result, Err(XcmSendError::Transport("other transport error"))) } From a88caf66bc495c2f42f732e3db4bfc4c09f99dd3 Mon Sep 17 00:00:00 2001 From: Alistair Singh Date: Tue, 12 Dec 2023 00:46:38 +0200 Subject: [PATCH 2/7] tests --- .../primitives/router/src/outbound/tests.rs | 119 ++++++++++++------ 1 file changed, 84 insertions(+), 35 deletions(-) diff --git a/parachain/primitives/router/src/outbound/tests.rs b/parachain/primitives/router/src/outbound/tests.rs index b57646853..bdeb5ad01 100644 --- a/parachain/primitives/router/src/outbound/tests.rs +++ b/parachain/primitives/router/src/outbound/tests.rs @@ -1,4 +1,7 @@ -use frame_support::{assert_ok, parameter_types, traits::Everything}; +use frame_support::{ + assert_err, assert_ok, parameter_types, + traits::{Everything, Nothing}, +}; use hex_literal::hex; use snowbridge_core::outbound::{Fee, SendError, SendMessageFeeProvider}; @@ -74,7 +77,7 @@ fn exporter_validate_with_unknown_network_yields_not_applicable() { AgentIdOf, Everything, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); - assert_eq!(result, Err(XcmSendError::NotApplicable)); + assert_err!(result, XcmSendError::NotApplicable); } #[test] @@ -93,7 +96,7 @@ fn exporter_validate_with_invalid_destination_yields_missing_argument() { AgentIdOf, Everything, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); - assert_eq!(result, Err(XcmSendError::MissingArgument)); + assert_err!(result, XcmSendError::MissingArgument); } #[test] @@ -114,7 +117,7 @@ fn exporter_validate_with_x8_destination_yields_not_applicable() { AgentIdOf, Everything, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); - assert_eq!(result, Err(XcmSendError::NotApplicable)); + assert_err!(result, XcmSendError::NotApplicable); } #[test] @@ -133,7 +136,7 @@ fn exporter_validate_without_universal_source_yields_missing_argument() { AgentIdOf, Everything, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); - assert_eq!(result, Err(XcmSendError::MissingArgument)); + assert_err!(result, XcmSendError::MissingArgument); } #[test] @@ -152,7 +155,7 @@ fn exporter_validate_without_global_universal_location_yields_unroutable() { AgentIdOf, Everything, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); - assert_eq!(result, Err(XcmSendError::Unroutable)); + assert_err!(result, XcmSendError::Unroutable); } #[test] @@ -171,7 +174,7 @@ fn exporter_validate_without_global_bridge_location_yields_not_applicable() { AgentIdOf, Everything, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); - assert_eq!(result, Err(XcmSendError::NotApplicable)); + assert_err!(result, XcmSendError::NotApplicable); } #[test] @@ -191,7 +194,7 @@ fn exporter_validate_with_remote_universal_source_yields_not_applicable() { AgentIdOf, Everything, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); - assert_eq!(result, Err(XcmSendError::NotApplicable)); + assert_err!(result, XcmSendError::NotApplicable); } #[test] @@ -210,7 +213,7 @@ fn exporter_validate_without_para_id_in_source_yields_missing_argument() { AgentIdOf, Everything, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); - assert_eq!(result, Err(XcmSendError::MissingArgument)); + assert_err!(result, XcmSendError::MissingArgument); } #[test] @@ -230,7 +233,7 @@ fn exporter_validate_complex_para_id_in_source_yields_missing_argument() { AgentIdOf, Everything, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); - assert_eq!(result, Err(XcmSendError::MissingArgument)); + assert_err!(result, XcmSendError::MissingArgument); } #[test] @@ -250,7 +253,7 @@ fn exporter_validate_without_xcm_message_yields_missing_argument() { AgentIdOf, Everything, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); - assert_eq!(result, Err(XcmSendError::MissingArgument)); + assert_err!(result, XcmSendError::MissingArgument); } #[test] @@ -298,7 +301,7 @@ fn exporter_validate_with_max_target_fee_yields_unroutable() { Everything, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); - assert_eq!(result, Err(XcmSendError::Unroutable)); + assert_err!(result, XcmSendError::Unroutable); } #[test] @@ -325,7 +328,7 @@ fn exporter_validate_with_unparsable_xcm_yields_unroutable() { Everything, >::validate(network, channel, &mut universal_source, &mut destination, &mut message); - assert_eq!(result, Err(XcmSendError::Unroutable)); + assert_err!(result, XcmSendError::Unroutable); } #[test] @@ -374,6 +377,52 @@ fn exporter_validate_xcm_success_case_1() { assert_ok!(result); } +#[test] +fn exporter_validate_with_filter_yields_unroutable() { + let network = BridgedNetwork::get(); + let mut destination: Option = Here.into(); + + let mut universal_source: Option = + Some(X2(GlobalConsensus(Polkadot), Parachain(1000))); + + let token_address: [u8; 20] = hex!("1000000000000000000000000000000000000000"); + let beneficiary_address: [u8; 20] = hex!("2000000000000000000000000000000000000000"); + + let channel: u32 = 0; + let assets: MultiAssets = vec![MultiAsset { + id: Concrete(X1(AccountKey20 { network: None, key: token_address }).into()), + fun: Fungible(1000), + }] + .into(); + let fee = assets.clone().get(0).unwrap().clone(); + let filter: MultiAssetFilter = assets.clone().into(); + + let mut message: Option> = Some( + vec![ + WithdrawAsset(assets.clone()), + ClearOrigin, + BuyExecution { fees: fee, weight_limit: Unlimited }, + DepositAsset { + assets: filter, + beneficiary: X1(AccountKey20 { network: None, key: beneficiary_address }).into(), + }, + SetTopic([0; 32]), + ] + .into(), + ); + + let result = + EthereumBlobExporter::< + UniversalLocation, + BridgedNetwork, + MockOkOutboundQueue, + AgentIdOf, + Nothing, + >::validate(network, channel, &mut universal_source, &mut destination, &mut message); + + assert_err!(result, XcmSendError::Unroutable); +} + #[test] fn exporter_deliver_with_submit_failure_yields_unroutable() { let result = EthereumBlobExporter::< @@ -383,7 +432,7 @@ fn exporter_deliver_with_submit_failure_yields_unroutable() { AgentIdOf, Everything, >::deliver((hex!("deadbeef").to_vec(), XcmHash::default())); - assert_eq!(result, Err(XcmSendError::Transport("other transport error"))) + assert_err!(result, XcmSendError::Transport("other transport error")) } #[test] @@ -418,7 +467,7 @@ fn xcm_converter_convert_success() { amount: 1000, }; let result = converter.convert(); - assert_eq!(result, Ok((expected_payload, [0; 32]))); + assert_ok!(result, (expected_payload, [0; 32])); } #[test] @@ -451,7 +500,7 @@ fn xcm_converter_convert_without_buy_execution_yields_success() { amount: 1000, }; let result = converter.convert(); - assert_eq!(result, Ok((expected_payload, [0; 32]))); + assert_ok!(result, (expected_payload, [0; 32])); } #[test] @@ -486,7 +535,7 @@ fn xcm_converter_convert_with_wildcard_all_asset_filter_succeeds() { amount: 1000, }; let result = converter.convert(); - assert_eq!(result, Ok((expected_payload, [0; 32]))); + assert_ok!(result, (expected_payload, [0; 32])); } #[test] @@ -522,7 +571,7 @@ fn xcm_converter_convert_with_fees_less_than_reserve_yields_success() { amount: 1000, }; let result = converter.convert(); - assert_eq!(result, Ok((expected_payload, [0; 32]))); + assert_ok!(result, (expected_payload, [0; 32])); } #[test] @@ -551,7 +600,7 @@ fn xcm_converter_convert_without_set_topic_yields_set_topic_expected() { .into(); let mut converter = XcmConverter::new(&message, &network); let result = converter.convert(); - assert_eq!(result.err(), Some(XcmConverterError::SetTopicExpected)); + assert_err!(result, XcmConverterError::SetTopicExpected); } #[test] @@ -568,7 +617,7 @@ fn xcm_converter_convert_with_partial_message_yields_unexpected_end_of_xcm() { let mut converter = XcmConverter::new(&message, &network); let result = converter.convert(); - assert_eq!(result.err(), Some(XcmConverterError::UnexpectedEndOfXcm)); + assert_err!(result, XcmConverterError::UnexpectedEndOfXcm); } #[test] @@ -602,7 +651,7 @@ fn xcm_converter_with_different_fee_asset_fails() { .into(); let mut converter = XcmConverter::new(&message, &network); let result = converter.convert(); - assert_eq!(result.err(), Some(XcmConverterError::InvalidFeeAsset)); + assert_err!(result, XcmConverterError::InvalidFeeAsset); } #[test] @@ -633,7 +682,7 @@ fn xcm_converter_with_fees_greater_than_reserve_fails() { .into(); let mut converter = XcmConverter::new(&message, &network); let result = converter.convert(); - assert_eq!(result.err(), Some(XcmConverterError::InvalidFeeAsset)); + assert_err!(result, XcmConverterError::InvalidFeeAsset); } #[test] @@ -645,7 +694,7 @@ fn xcm_converter_convert_with_empty_xcm_yields_unexpected_end_of_xcm() { let mut converter = XcmConverter::new(&message, &network); let result = converter.convert(); - assert_eq!(result.err(), Some(XcmConverterError::UnexpectedEndOfXcm)); + assert_err!(result, XcmConverterError::UnexpectedEndOfXcm); } #[test] @@ -677,7 +726,7 @@ fn xcm_converter_convert_with_extra_instructions_yields_end_of_xcm_message_expec let mut converter = XcmConverter::new(&message, &network); let result = converter.convert(); - assert_eq!(result.err(), Some(XcmConverterError::EndOfXcmMessageExpected)); + assert_err!(result, XcmConverterError::EndOfXcmMessageExpected); } #[test] @@ -707,7 +756,7 @@ fn xcm_converter_convert_without_withdraw_asset_yields_withdraw_expected() { let mut converter = XcmConverter::new(&message, &network); let result = converter.convert(); - assert_eq!(result.err(), Some(XcmConverterError::WithdrawAssetExpected)); + assert_err!(result, XcmConverterError::WithdrawAssetExpected); } #[test] @@ -732,7 +781,7 @@ fn xcm_converter_convert_without_withdraw_asset_yields_deposit_expected() { let mut converter = XcmConverter::new(&message, &network); let result = converter.convert(); - assert_eq!(result.err(), Some(XcmConverterError::DepositAssetExpected)); + assert_err!(result, XcmConverterError::DepositAssetExpected); } #[test] @@ -765,7 +814,7 @@ fn xcm_converter_convert_without_assets_yields_no_reserve_assets() { let mut converter = XcmConverter::new(&message, &network); let result = converter.convert(); - assert_eq!(result.err(), Some(XcmConverterError::NoReserveAssets)); + assert_err!(result, XcmConverterError::NoReserveAssets); } #[test] @@ -803,7 +852,7 @@ fn xcm_converter_convert_with_two_assets_yields_too_many_assets() { let mut converter = XcmConverter::new(&message, &network); let result = converter.convert(); - assert_eq!(result.err(), Some(XcmConverterError::TooManyAssets)); + assert_err!(result, XcmConverterError::TooManyAssets); } #[test] @@ -834,7 +883,7 @@ fn xcm_converter_convert_without_consuming_filter_yields_filter_does_not_consume let mut converter = XcmConverter::new(&message, &network); let result = converter.convert(); - assert_eq!(result.err(), Some(XcmConverterError::FilterDoesNotConsumeAllAssets)); + assert_err!(result, XcmConverterError::FilterDoesNotConsumeAllAssets); } #[test] @@ -865,7 +914,7 @@ fn xcm_converter_convert_with_zero_amount_asset_yields_zero_asset_transfer() { let mut converter = XcmConverter::new(&message, &network); let result = converter.convert(); - assert_eq!(result.err(), Some(XcmConverterError::ZeroAssetTransfer)); + assert_err!(result, XcmConverterError::ZeroAssetTransfer); } #[test] @@ -895,7 +944,7 @@ fn xcm_converter_convert_non_ethereum_asset_yields_asset_resolution_failed() { let mut converter = XcmConverter::new(&message, &network); let result = converter.convert(); - assert_eq!(result.err(), Some(XcmConverterError::AssetResolutionFailed)); + assert_err!(result, XcmConverterError::AssetResolutionFailed); } #[test] @@ -928,7 +977,7 @@ fn xcm_converter_convert_non_ethereum_chain_asset_yields_asset_resolution_failed let mut converter = XcmConverter::new(&message, &network); let result = converter.convert(); - assert_eq!(result.err(), Some(XcmConverterError::AssetResolutionFailed)); + assert_err!(result, XcmConverterError::AssetResolutionFailed); } #[test] @@ -961,7 +1010,7 @@ fn xcm_converter_convert_non_ethereum_chain_yields_asset_resolution_failed() { let mut converter = XcmConverter::new(&message, &network); let result = converter.convert(); - assert_eq!(result.err(), Some(XcmConverterError::AssetResolutionFailed)); + assert_err!(result, XcmConverterError::AssetResolutionFailed); } #[test] @@ -998,7 +1047,7 @@ fn xcm_converter_convert_with_non_ethereum_beneficiary_yields_beneficiary_resolu let mut converter = XcmConverter::new(&message, &network); let result = converter.convert(); - assert_eq!(result.err(), Some(XcmConverterError::BeneficiaryResolutionFailed)); + assert_err!(result, XcmConverterError::BeneficiaryResolutionFailed); } #[test] @@ -1034,5 +1083,5 @@ fn xcm_converter_convert_with_non_ethereum_chain_beneficiary_yields_beneficiary_ let mut converter = XcmConverter::new(&message, &network); let result = converter.convert(); - assert_eq!(result.err(), Some(XcmConverterError::BeneficiaryResolutionFailed)); + assert_err!(result, XcmConverterError::BeneficiaryResolutionFailed); } From 0140122ce788d496c8940ab9819f73d83ad4c56d Mon Sep 17 00:00:00 2001 From: Alistair Singh Date: Tue, 12 Dec 2023 01:28:58 +0200 Subject: [PATCH 3/7] add runtime tests --- parachain/runtime/tests/src/lib.rs | 24 +++++++++- parachain/runtime/tests/src/test_cases.rs | 53 +++-------------------- 2 files changed, 28 insertions(+), 49 deletions(-) diff --git a/parachain/runtime/tests/src/lib.rs b/parachain/runtime/tests/src/lib.rs index 7375f84c8..946941adc 100644 --- a/parachain/runtime/tests/src/lib.rs +++ b/parachain/runtime/tests/src/lib.rs @@ -8,6 +8,7 @@ mod test_cases; use asset_hub_rococo_runtime::xcm_config::bridging::to_ethereum::DefaultBridgeHubEthereumBaseFee; use bridge_hub_rococo_runtime::{xcm_config::XcmConfig, Runtime, RuntimeEvent, SessionKeys}; use codec::Decode; +use cumulus_primitives_core::XcmError::{FailedToTransactAsset, NotHoldingFees, Unroutable}; use parachains_common::{AccountId, AuraId}; use sp_core::H160; use sp_keyring::AccountKeyring::Alice; @@ -38,6 +39,21 @@ pub fn transfer_token_to_ethereum_works() { ) } +#[test] +pub fn transfer_token_from_non_assethub_to_ethereum_unroutable() { + test_cases::send_transfer_token_message_failure::( + collator_session_keys(), + 1013, + 1001, + BridgeHubEthereumBaseFeeInROC::get() + 1_000_000_000, + H160::random(), + H160::random(), + // fee not enough + 1_000_000_000, + Unroutable, + ) +} + #[test] pub fn unpaid_transfer_token_to_ethereum_fails_with_barrier() { test_cases::send_unpaid_transfer_token_message::( @@ -51,25 +67,29 @@ pub fn unpaid_transfer_token_to_ethereum_fails_with_barrier() { #[test] pub fn transfer_token_to_ethereum_fee_not_enough() { - test_cases::send_transfer_token_message_fee_not_enough::( + test_cases::send_transfer_token_message_failure::( collator_session_keys(), 1013, 1000, + BridgeHubEthereumBaseFeeInROC::get() + 1_000_000_000, H160::random(), H160::random(), // fee not enough 1_000_000_000, + NotHoldingFees, ) } #[test] pub fn transfer_token_to_ethereum_insufficient_fund() { - test_cases::send_transfer_token_message_insufficient_fund::( + test_cases::send_transfer_token_message_failure::( collator_session_keys(), 1013, 1000, + 1_000_000_000, H160::random(), H160::random(), DefaultBridgeHubEthereumBaseFee::get(), + FailedToTransactAsset("InsufficientBalance"), ) } diff --git a/parachain/runtime/tests/src/test_cases.rs b/parachain/runtime/tests/src/test_cases.rs index c2c227451..0c921787a 100644 --- a/parachain/runtime/tests/src/test_cases.rs +++ b/parachain/runtime/tests/src/test_cases.rs @@ -15,7 +15,7 @@ use xcm::latest::prelude::*; use xcm_executor::XcmExecutor; // Re-export test_case from `parachains-runtimes-test-utils` pub use parachains_runtimes_test_utils::test_cases::change_storage_constant_by_governance_works; -use xcm::v3::Error::{Barrier, FailedToTransactAsset, NotHoldingFees}; +use xcm::v3::Error::{self, Barrier}; type RuntimeHelper = parachains_runtimes_test_utils::RuntimeHelper; @@ -240,13 +240,15 @@ pub fn send_unpaid_transfer_token_message( }); } -pub fn send_transfer_token_message_fee_not_enough( +pub fn send_transfer_token_message_failure( collator_session_key: CollatorSessionKeys, runtime_para_id: u32, assethub_parachain_id: u32, + initial_amount: u128, weth_contract_address: H160, destination_address: H160, fee_amount: u128, + expected_error: Error, ) where Runtime: frame_system::Config + pallet_balances::Config @@ -267,10 +269,7 @@ pub fn send_transfer_token_message_fee_not_enough( .build() .execute_with(|| { // fund asset hub sovereign account enough so it can pay fees - initial_fund::( - assethub_parachain_id, - DefaultBridgeHubEthereumBaseFee::get() + 1_000_000_000, - ); + initial_fund::(assethub_parachain_id, initial_amount); let outcome = send_transfer_token_message::( assethub_parachain_id, @@ -279,46 +278,6 @@ pub fn send_transfer_token_message_fee_not_enough( fee_amount, ); // check err is NotHoldingFees - assert_err!(outcome.ensure_complete(), NotHoldingFees); - }); -} - -pub fn send_transfer_token_message_insufficient_fund( - collator_session_key: CollatorSessionKeys, - runtime_para_id: u32, - assethub_parachain_id: u32, - weth_contract_address: H160, - destination_address: H160, - fee_amount: u128, -) where - Runtime: frame_system::Config - + pallet_balances::Config - + pallet_session::Config - + pallet_xcm::Config - + parachain_info::Config - + pallet_collator_selection::Config - + cumulus_pallet_parachain_system::Config - + snowbridge_outbound_queue::Config, - XcmConfig: xcm_executor::Config, - ValidatorIdOf: From>, -{ - ExtBuilder::::default() - .with_collators(collator_session_key.collators()) - .with_session_keys(collator_session_key.session_keys()) - .with_para_id(runtime_para_id.into()) - .with_tracing() - .build() - .execute_with(|| { - // initial fund not enough - initial_fund::(assethub_parachain_id, 1_000_000_000); - - let outcome = send_transfer_token_message::( - assethub_parachain_id, - weth_contract_address, - destination_address, - fee_amount, - ); - // check err is FailedToTransactAsset("InsufficientBalance") - assert_err!(outcome.ensure_complete(), FailedToTransactAsset("InsufficientBalance")); + assert_err!(outcome.ensure_complete(), expected_error); }); } From c32246e2fa4bb01fca9d3e842905ece815eb1b61 Mon Sep 17 00:00:00 2001 From: Alistair Singh Date: Tue, 12 Dec 2023 01:41:00 +0200 Subject: [PATCH 4/7] fix tests --- parachain/primitives/router/src/outbound/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/parachain/primitives/router/src/outbound/mod.rs b/parachain/primitives/router/src/outbound/mod.rs index 16fa64513..5e2c61946 100644 --- a/parachain/primitives/router/src/outbound/mod.rs +++ b/parachain/primitives/router/src/outbound/mod.rs @@ -131,8 +131,8 @@ impl Date: Sat, 16 Dec 2023 15:55:34 +0200 Subject: [PATCH 5/7] change name to local origin --- .../primitives/router/src/outbound/mod.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/parachain/primitives/router/src/outbound/mod.rs b/parachain/primitives/router/src/outbound/mod.rs index 5e2c61946..ab7da13dd 100644 --- a/parachain/primitives/router/src/outbound/mod.rs +++ b/parachain/primitives/router/src/outbound/mod.rs @@ -76,7 +76,7 @@ impl para_id, _ => { - log::error!(target: "xcm::ethereum_blob_exporter", "could not get parachain id from universal source '{local_sub:?}'."); + log::error!(target: "xcm::ethereum_blob_exporter", "could not get parachain id from universal source '{local_origin:?}'."); return Err(SendError::MissingArgument) }, }; @@ -112,12 +112,12 @@ impl id, None => { - log::error!(target: "xcm::ethereum_blob_exporter", "unroutable due to not being able to create agent id. '{local_sub_location:?}'"); + log::error!(target: "xcm::ethereum_blob_exporter", "unroutable due to not being able to create agent id. '{local_origin_location:?}'"); return Err(SendError::Unroutable) }, }; @@ -131,8 +131,8 @@ impl Date: Sat, 16 Dec 2023 16:05:47 +0200 Subject: [PATCH 6/7] fix tests --- parachain/runtime/tests/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/parachain/runtime/tests/src/lib.rs b/parachain/runtime/tests/src/lib.rs index 946941adc..c2811f7ab 100644 --- a/parachain/runtime/tests/src/lib.rs +++ b/parachain/runtime/tests/src/lib.rs @@ -45,7 +45,7 @@ pub fn transfer_token_from_non_assethub_to_ethereum_unroutable() { collator_session_keys(), 1013, 1001, - BridgeHubEthereumBaseFeeInROC::get() + 1_000_000_000, + DefaultBridgeHubEthereumBaseFee::get() + 1_000_000_000, H160::random(), H160::random(), // fee not enough @@ -71,7 +71,7 @@ pub fn transfer_token_to_ethereum_fee_not_enough() { collator_session_keys(), 1013, 1000, - BridgeHubEthereumBaseFeeInROC::get() + 1_000_000_000, + DefaultBridgeHubEthereumBaseFee::get() + 1_000_000_000, H160::random(), H160::random(), // fee not enough From b6fafe46e38ba95e3d4ba58ae2399f475016e06b Mon Sep 17 00:00:00 2001 From: Alistair Singh Date: Sun, 17 Dec 2023 11:56:54 +0200 Subject: [PATCH 7/7] update polkadot-sdk --- polkadot-sdk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkadot-sdk b/polkadot-sdk index 00e8eb2e0..6252d96cb 160000 --- a/polkadot-sdk +++ b/polkadot-sdk @@ -1 +1 @@ -Subproject commit 00e8eb2e04375260d728f143352e21cbf7a4436c +Subproject commit 6252d96cb9555e8b407df8dd27b0b8ef359b74ef