Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SubscribeVersion from the Relay will be blocked by new parachains #1756

Closed
mrshiposha opened this issue Sep 29, 2023 · 5 comments · Fixed by #3955
Closed

SubscribeVersion from the Relay will be blocked by new parachains #1756

mrshiposha opened this issue Sep 29, 2023 · 5 comments · Fixed by #3955
Assignees
Labels
I2-bug The node fails to follow expected behavior. T6-XCM This PR/Issue is related to XCM.

Comments

@mrshiposha
Copy link
Contributor

mrshiposha commented Sep 29, 2023

The Relay chain uses the WithUniqueTopic router, which adds a SetTopic instruction to every XCM message the Relay sends if the message doesn't end with such an instruction already.

This could break the XCM version subscriptions for new parachains that didn't receive the SubscribeVersion in the past.

The SubscribeVersion message will also be augmented with the SetTopic instruction. Currently, the AllowSubscriptionsFrom barrier used by the parachains forbids any message that contains anything but the SubscribeVersion/UnsubscribeVersion instruction. So, any SubscribeVersion or UnsubscribeVersion from the Relay chain will be blocked.

@mrshiposha mrshiposha added I10-unconfirmed Issue might be valid, but it's not yet known. I2-bug The node fails to follow expected behavior. labels Sep 29, 2023
@bkchr
Copy link
Member

bkchr commented Sep 29, 2023

CC @franciscoaguirre

@franciscoaguirre
Copy link
Contributor

That sounds bad, I'll look into it

@franciscoaguirre franciscoaguirre added the T6-XCM This PR/Issue is related to XCM. label Mar 25, 2024
@acatangiu acatangiu moved this from Draft to Next to pick up in Parity Roadmap Mar 28, 2024
@acatangiu acatangiu removed the I10-unconfirmed Issue might be valid, but it's not yet known. label Mar 28, 2024
@bkontur bkontur moved this from Next to pick up to In-Progress in Parity Roadmap Apr 2, 2024
@bkontur
Copy link
Contributor

bkontur commented Apr 2, 2024

@mrshiposha Yes, you're right. AllowSubscriptionsFrom accepts just SubscribeVersion / UnsubscribeVersion, but there is a TrailingSetTopicAsId barrier, which is exactly designed to handle this SetTopic added by WithUniqueTopic router. Therefore, the parachain should wrap its barrier with TrailingSetTopicAsId, for example: https://github.com/paritytech/polkadot-sdk/blob/master/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs#L485-L506. Anyway, the parachain still has the possibility to write custom barriers according to its needs.

And it is not a specific problem of the RelayChain, this can happen between any chains when using the WithUniqueTopic wrapper for SendXcm between them.

I've just added more tests for these barriers here: #3955.

@mrshiposha
Copy link
Contributor Author

Oh, I forgot I filed this issue :)
Thanks for the response, @bkontur! I used the TrailingSetTopicAsId barrier in the Unique Network chain as a fix but forgot to comment here.

It's great to have tests to check that version subscriptions work. IIRC, I filed this issue because the subscription mechanism was unexpectedly broken in our chain when Relay suddenly changed its behavior.

github-merge-queue bot pushed a commit that referenced this issue Apr 3, 2024
@github-project-automation github-project-automation bot moved this from In-Progress to Done in Parity Roadmap Apr 3, 2024
@bkontur
Copy link
Contributor

bkontur commented Apr 3, 2024

I filed this issue because the subscription mechanism was unexpectedly broken in our chain when Relay suddenly changed its behavior.

@mrshiposha Thank you for reporting! Yeah, got it. I hope that similar issues will be avoided with our improved release notes, including this kind of breaking change.

Ank4n pushed a commit that referenced this issue Apr 9, 2024
dharjeezy pushed a commit to dharjeezy/polkadot-sdk that referenced this issue Apr 9, 2024
bkchr pushed a commit that referenced this issue Apr 10, 2024
* Define SimpleRuntimeVersion

* Set R/WococoBridgeHub bundle runtime version
@acatangiu acatangiu moved this to Done in Bridges + XCM Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I2-bug The node fails to follow expected behavior. T6-XCM This PR/Issue is related to XCM.
Projects
Status: Done
Status: Done
5 participants