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

fix: issue 9127. teleport reactivated #9529

Merged
merged 3 commits into from
Jun 17, 2023

Conversation

loanMaster
Copy link
Contributor

Hello Polkadot Apps maintainers

This PR fixes #9127
by using XCM v2 for Kusama and v1 for Polkadot teleports.

v2 is used as default if not otherwise defined.

jacogr
jacogr previously requested changes Jun 3, 2023
Copy link
Member

@jacogr jacogr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. Some small requests around the versioning maintenance.

packages/page-parachains/src/Teleport.tsx Outdated Show resolved Hide resolved
@loanMaster loanMaster requested a review from jacogr June 7, 2023 16:20
@joepetrowski
Copy link
Contributor

Polkadot and all system chains are now on V3, we really shouldn't use V1 or V2 at all.

@jacogr
Copy link
Member

jacogr commented Jun 16, 2023

Since as per Joe's comments all are (currently) on V3 could just simplify it completely -

  • only support V3
  • remove all version config

So I guess it could be just as simple as renaming the old key in the message construction to V3.

(If anything is not on V3, e.g. have not checked Encointer, just don't allow teleport functionality)

@jacogr jacogr dismissed their stale review June 16, 2023 05:47

Stale review.

@loanMaster
Copy link
Contributor Author

loanMaster commented Jun 16, 2023

I updated to xcm V3. The kusama <-> encointer network channel does not support xcm V3 and was therefore deactivated.

On Rococo the transfer works from Rococo to Asset Hub but not from Asset Hub to Rococo. Should I disable?

Also on Rococo the transferred assets from a parachain to Rococo sometimes don't show up at all, maybe related to this:
paritytech/polkadot-testnet-faucet#286
The transaction itself passes successfully.

On Westend, Kusama, Polkadot the transfers worked properly.

@jacogr: Could you review again? Thank you.

Copy link
Member

@jacogr jacogr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

It looks sane, just going through testing through this branch locally (both directions).

PS: I think Rococo is fine to leave as-is. (Since it is a testnet without any funds at risk - since it also evolves quite rapidly, having the functionality available is great, even if it temporaily doesn't quite work as expected).

Copy link
Member

@jacogr jacogr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! This is great. (And especially for checking/testing on all the affected relays)

The testing on my side went perfectly.

@polkadot-js-bot
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Jun 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update teleport functionality to XCM V2/V3
5 participants