Lowering XCM weights and fees

[bkontur]

Summary

The dedicated XCM-relevant benchmarks (pallet_xcm, pallet_xcm_benchmarks::fungible, pallet_xcm_benchmarks::generic) can utilize a SendXcm implementation at the end, which typically employs the WrapVersion implementation, often sourced from pallet_xcm. This also impacts all other extrinsics and benchmarks that utilize SendXcm + WrapVersion.

The pallet_xcm's WrapVersion implementation adds unnecessary overhead to the weights - 2 reads and 1 write:

        /// Storage: `PolkadotXcm::VersionDiscoveryQueue` (r:1 w:1)
	/// Proof: `PolkadotXcm::VersionDiscoveryQueue` (`max_values`: Some(1), `max_size`: None, mode: `Measured`)
	/// Storage: `PolkadotXcm::SafeXcmVersion` (r:1 w:0)
	/// Proof: `PolkadotXcm::SafeXcmVersion` (`max_values`: Some(1), `max_size`: None, mode: `Measured`)

This overhead arises from checking the compatible XCM version for the destination. If no supported version is stored, we utilize SafeXcmVersion and store the VersionDiscoveryQueue key/value for later version discovery.
This process occurs only the first time when the destination is not added to the SupportedVersion in pallet_xcm.

The idea here is that this overhead should not be borne by the end user, but rather should be part of establishing the communication channel (e.g., opening HRMP).

Proposed solution part 1 - reducing XCM weights

Implemented by: #3664

We utilize xcm_builder::EnsureDelivery and DeliveryHelper features for XCM benchmarking, as demonstrated here and here. These features ensure successful delivery for extrinsic benchmarks with the underlying SendXcm implementation.

A good example is benchmarking for AssetHubRococo, where it is essential to ensure that we can deliver to the Parent and/or to the sibling parachains. The DeliveryHelper ensures that the sender account has enough assets for delivery fees and/or opens HRMP channels.

Therefore, the straightforward solution here is to extend this DeliveryHelper by one more step, which will insert the desired XCM version into the pallet_xcm::SupportedVersion to eliminate the mentioned overhead.

Note: These EnsureDelivery features can also be directly reused for other benchmarks when needed.

Proposed solution part 2 - initiate VersionDiscoveryQueue when HRMP is accepted

Implemented by: #3696

Currently, XcmExecutor does not support the implementation for HrmpNewChannelOpenRequest / HrmpChannelAccepted / HrmpChannelClosing XCM instructions, as indicated here.

The idea here is that when a parachain receives HrmpChannelAccepted, it will add a key/value pair to the VersionDiscoveryQueue, which will automatically initiate the "XCM version negotiation" mechanism. To achieve this, we need to add a proper callback handler to the XcmExecutor's configuration.

Example:

// + tuple impl
pub trait HandleHrmpChannelNotification {
    fn on_accepted(recipient: u32);
    fn on_closing(recipient: u32);
    fn on_open_request(recipient: u32);
}

/// XcmExecutor setup
impl xcm_executor::Config for XcmConfig {
     type HrmpChannelNotificationHandler = PolkadotXcm // <- pallet_xcm::Pallet<T>
}


/// Implemenatation for `pallet_xcm`.
impl HandleHrmpChannelNotification for pallet_xcm::Pallet<T> {
   ...
}

TODO/questions

• design proper trait - one trait with function per notification or one trait per notification?
• do we need XCM rfc for that?

[bkontur]

@franciscoaguirre I would greatly appreciate your assistance and opinion on part 2, at the very least :)

[acatangiu]

Before starting on part 2, please sync up with participants of polkadot-fellows/RFCs#82 (comment) discussion, to make sure we don't duplicate work.

[franciscoaguirre]

I think 2 is the best solution. I wanted to add callbacks for those instructions already, so that any parachain can do whatever they want when they open a channel to another chain. For example they could register a derivative of their token on the other chain.
It feels like killing two birds with one stone

[franciscoaguirre]

I heard the same about Polimec, let's work with them to make sure it gets upstreamed

[bkontur]

Second part is fixed here: #4025.

First part is low prio and very nice to have, but it is better recommended to fix extrinsic:

• read actual benchmarked weight for extrinsic (AEW)
• reduce AEW about VersionDiscover weight when we don't hit version discovery scenario
• return reduced AEW from extrinsic, that way user will pay less