Predicate route table by Xcm

[yrong]

Context

Idea from @alistair-singh. The ExporterFor trait returns the fee to be charged to the user when exporting to Ethereum on Asset Hub. This trait exposes the XCM as message parameter that can be used to select an exporter. On Asset Hub this is configured with NetworkExportTable struct implementation. However this implementation ignores the message parameter. We need to modify this implementation so that it contains another field, a predicate which accepts Xcm<()> -> bool which acts as the filter. We can then modify the existing record in the BridgeTable to have a predicate which matches V1 XCM. And add a new record in the bridge table that matches V2 XCM. Each record can then be configured with a different fee.

The predicate for V1 XCM will look for

WithdrawAsset(...) or ReserveAssetDeposited # For erc20 and PNA respectively
ClearOrigin
BuyExecution

The predicate for V2 XCM will look for

WithdrawAsset or ReserveAssetDeposited or TeleportedAssetRevceived or * 
PayFees(fees)
ALiasOrigin

[bkontur]

Shouldn't this be better like "does not contain AliasOrigin" instead of just "true"?
Or maybe something like:

fn contains(xcm: &Xcm<()>) -> bool {
    !XcmForSnowbridgeV2::contains(xcm)
}

or even easier, you dont need XcmForSnowbridgeV1 at all, maybe just use EverythingBut:

		pub type EthereumNetworkExportTable =
			xcm_builder::NetworkWithXcmExportTable<EthereumBridgeTable, EverythingBut<XcmForSnowbridgeV2>>;

Anyway, neither XcmForSnowbridgeV2 nor XcmForSnowbridgeV1 is aligned with description:

The predicate for V1 XCM will look for

WithdrawAsset(...) or ReserveAssetDeposited # For erc20 and PNA respectively
ClearOrigin
BuyExecution

The predicate for V2 XCM will look for

WithdrawAsset or ReserveAssetDeposited or TeleportedAssetRevceived or * 
PayFees(fees)
ALiasOrigin

[yrong]

Refactor with EverythingBut in 8cb272e

neither XcmForSnowbridgeV2 nor XcmForSnowbridgeV1 is aligned with description

On AH we just inspect xcm which contains a specific instruction, we will validate the full xcm in Exporter on BH.

[bkontur]

I’m not convinced we need both NetworkExportTable and NetworkWithXcmExportTable. Why not just modify the existing NetworkExportTable to include this M / M::contains logic? Wherever NetworkExportTable is used, we can simply add NetworkExportTable<.., Everything>.

[yrong]

It's a low level primitive and the change will impact current codes/tests a lot, I'd prefer to minimize the change required.

[bkontur]

It's a low level primitive and the change will impact current codes/tests a lot, I'd prefer to minimize the change required.

I assume that at some point you will remove support for v1, correct? If so, then NetworkWithXcmExportTable will no longer be needed. Therefore, I suggest moving NetworkWithXcmExportTable to the bridges/snowbridge submodule for now.

If it needs to stay here, I would suggest modifying NetworkExportTable, yes, it is a breaking change (just like XCMv5, which this feature is waiting for, right?), but it is clearer than having both NetworkWithXcmExportTable and NetworkExportTable.

[yrong]

Moving NetworkWithXcmExportTable to the bridges/snowbridge submodule with 44dbb6c

[bkontur]

I would suggest a small optimization by reusing NetworkExportTable, so you don't need to duplicate code. Currently, your M::contains(xcm) (implemented by xcm.clone().0.matcher()) is called for every item in the list. However, the XCM instructions do not depend on the table, network, or remote location, so you are repeatedly matching the same XCM, which could be checked just once.

Suggested change

	T::get()
	.into_iter()
	.find(|item| {
	&item.remote_network == network &&
	M::contains(xcm) &&
	item.remote_location_filter
	.as_ref()
	.map(|filters| filters.iter().any(|filter| filter == remote_location))
	.unwrap_or(true)
	})
	.map(|item| (item.bridge, item.payment))
	// check `network` and `remote_location`
	let Some(bridge_with_fees) = NetworkExportTable::<T>::exporter_for(network, remote_location, xcm) else {
	return None,
	};
	// check the XCM
	if M::contains(xcm) {
	Some(bridge_with_fees)
	} else {
	None
	}

And perhaps it makes sense to check the XCM beforehand and then network, remote_location with iterating T table.

[yrong]

606355c

[alistair-singh]

Make T implement ExporterFor directly removing dependency on NetworkExportTableItem.

Suggested change

	impl<T: Get<Vec<NetworkExportTableItem>>, M: Contains<Xcm<()>>> ExporterFor
	impl<T: ExporterFor, M: Contains<Xcm<()>>> ExporterFor

[yrong]

a6dbcba

[alistair-singh]

I like the way you implemented this predicate. Maybe a better name would be:

Suggested change

	pub struct NetworkWithXcmExportTable<T, M>(core::marker::PhantomData<(T, M)>);
	pub struct XcmFilterExporter<T, M>(core::marker::PhantomData<(T, M)>);

[yrong]

47d26e8

[yrong]

Closed in favor of #6706 which is now ready for review.