You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The potential problem is using VersionedXcm::from(Xcm which uses xcm::latest::* stuff. E.g. when the relay chain is migrated to the higher XCM version than the parachains, it could lead to the undecodable and ProcessMessageError::Corrupt when processing message on the parachain here.
Everywhere where we send XCM (ChildParachainRouter, SiblingParachainRouter, ...) we use WrapVersion which uses implementation from pallet_xcm (SupportedVersion for destination or safeXcmVersion of runtime) to avoid using latest XCM version.
So the solution here is to add WrapVersion to the HRMP pallet Config and use this wrap_version for all notifications mentioned above. The result will be that we VersionedXcm would be at least safeXcmVersion or the previously stored on in SupportedVersion (if parachain communicated before with the relay chain).
Note: We added HRMP notification handler just recently here: #3696, but there could already exist custom implementations of XcmExecutor, so until this fix comes live, for them the hotfix would be to upgrade their XCM version as soon as possible according to the relay chain version (there is XCMv4 coming to the Kusama in few days).
Closes: #4003 (please
see for the problem description)
## TODO
- [x] add more tests covering `WrapVersion` corner cases (e.g. para has
lower version, ...)
- [x] regenerate benchmarks `runtime_parachains::hrmp` (fix for Rococo
is here: #4332)
## Questions / possible improvements
- [ ] A `WrapVersion` implementation for `pallet_xcm` initiates version
discovery with
[note_unknown_version](https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/xcm/pallet-xcm/src/lib.rs#L2527C5-L2527C25),
there is possibility to avoid this overhead in this HRMP case to create
new `WrapVersion` adapter for `pallet_xcm` which would not use
`note_unknown_version`. Is it worth to do it or not?
- [ ] There's a possibility to decouple XCM functionality from the HRMP
pallet, allowing any relay chain to generate its own notifications. This
approach wouldn't restrict notifications solely to the XCM. However,
it's uncertain whether it's worthwhile or desirable to do so? It means
making HRMP pallet more generic. E.g. hiding HRMP notifications behind
some trait:
```
trait HrmpNotifications {
fn on_channel_open_request(
sender: ParaId,
proposed_max_capacity: u32,
proposed_max_message_size: u32) -> primitives::DownwardMessage;
fn on_channel_accepted(recipient: ParaId) ->
primitives::DownwardMessage;
fn on_channel_closing(initiator: ParaId, sender: ParaId, recipient:
ParaId) -> primitives::DownwardMessage;
}
```
and then we could have whatever adapter, `impl HrmpNotifications for
VersionedXcmHrmpNotifications {...}`,
```
impl parachains_hrmp::Config for Runtime {
..
type HrmpNotifications = VersionedXcmHrmpNotifications;
..
}
```
---------
Co-authored-by: command-bot <>
Co-authored-by: Bastian Köcher <git@kchr.de>
The HRMP channel are handled on the relay chain and the relay chain sends notifications to the child parachains:
The potential problem is using
VersionedXcm::from(Xcm
which usesxcm::latest::*
stuff. E.g. when the relay chain is migrated to the higher XCM version than the parachains, it could lead to the undecodable andProcessMessageError::Corrupt
when processing message on the parachain here.Everywhere where we send XCM (ChildParachainRouter, SiblingParachainRouter, ...) we use
WrapVersion
which uses implementation frompallet_xcm
(SupportedVersion
for destination orsafeXcmVersion
of runtime) to avoid using latest XCM version.So the solution here is to add
WrapVersion
to the HRMP pallet Config and use thiswrap_version
for all notifications mentioned above. The result will be that weVersionedXcm
would be at leastsafeXcmVersion
or the previously stored on inSupportedVersion
(if parachain communicated before with the relay chain).Note: We added HRMP notification handler just recently here: #3696, but there could already exist custom implementations of XcmExecutor, so until this fix comes live, for them the hotfix would be to upgrade their XCM version as soon as possible according to the relay chain version (there is XCMv4 coming to the Kusama in few days).
cc: @JuaniRios
The text was updated successfully, but these errors were encountered: