paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.network/
1.82k stars 661 forks source link

bridge-hub-router should not throw xcm DestinationUnsupported error #4133

Closed yrong closed 5 months ago

yrong commented 5 months ago

Is there an existing issue?

Experiencing problems? Have you tried our Stack Exchange first?

Description of bug

bridge-hub-router pallet should not throw DestinationUnsupported error when it can't process the message to Ethereum, context and more details in https://github.com/Snowfork/polkadot-sdk/pull/140

Steps to reproduce

No response

yrong commented 5 months ago

Context

https://github.com/Snowfork/polkadot-sdk/pull/140#issue-2242988186

bkontur commented 5 months ago

Thank you for finding this. DestinationUnsupported is ok for this case, but the validation is on the wrong place :), I prepared fix here: https://github.com/paritytech/polkadot-sdk/pull/4162 but I want to add more tests tomorrow before merging.

Everywhere the checking version or wrap_version for destination is performed, we use DestinationUnsupported, which means:

        /// The given message cannot be translated into a format that the destination can be expected
    /// to interpret.
    DestinationUnsupported,

which is exactly the case here. E.g: XcmpQueue - https://github.com/paritytech/polkadot-sdk/blob/master/cumulus/pallets/xcmp-queue/src/lib.rs#L917-L918 ParentAsUmp - https://github.com/paritytech/polkadot-sdk/blob/master/cumulus/primitives/utility/src/lib.rs#L71 ChildParachainRouter - https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/runtime/common/src/xcm_sender.rs#L122

And if you check all those implementations, everywhere there is a check for reachable location, if router cannot reach the particular destination or location pattern, then it returns NotApplicable and gives opportunity to the other routers in tuple.

        /// The message and destination combination was not recognized as being reachable.
    ///
    /// This is not considered fatal: if there are alternative transport routes available, then
    /// they may be attempted.
    NotApplicable,

So the bridge-hub-router is designed exactly to be aligned with the other routers.

yrong commented 5 months ago

Cool! Thanks for confirming that.