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

teleport KSM token between Kusama and Basilisk #4297

Merged
merged 19 commits into from
Nov 23, 2022

Conversation

Jarsen136
Copy link
Contributor

Thank you for your contribution to the KodaDot NFT gallery.

👇 _ Let's make a quick check before the contribution.

PR Type

  • Bugfix
  • Feature
  • Refactoring

Context

Before submitting pull request, please make sure:

  • My contribution builds clean without any errors or warnings
  • I've merged recent default branch -- main and I've no conflicts
  • I've tried to respect high code quality standards
  • I've didn't break any original functionality
  • I've posted a screenshot of demonstrated change in this PR

Optional

  • I've tested it at </bsx/collection>
  • I've tested PR on mobile
  • I've written unit tests 🧪
  • I've found edge cases

Had issue bounty label?

  • Fill up your KSM address: Payout

Community participation

Screenshot 📸

  • My fix has changed something on UI; a screenshot is best to understand changes for others.

currently, the url for teleport is /teleport-bridge

@Jarsen136 Jarsen136 requested a review from a team as a code owner November 10, 2022 05:46
@Jarsen136 Jarsen136 requested review from prachi00 and removed request for a team November 10, 2022 05:46
@kodabot
Copy link
Collaborator

kodabot commented Nov 10, 2022

SUCCESS @Jarsen136 PR for issue #4035 which is assigned to you. Please wait for review and don't hesitate to grab another issue in the meantime!

@netlify
Copy link

netlify bot commented Nov 10, 2022

Deploy Preview for koda-nuxt ready!

Name Link
🔨 Latest commit 6b2311b
🔍 Latest deploy log https://app.netlify.com/sites/koda-nuxt/deploys/637cdde8f6f14d0009cf0a22
😎 Deploy Preview https://deploy-preview-4297--koda-nuxt.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@Jarsen136 Jarsen136 marked this pull request as draft November 10, 2022 05:46
@Jarsen136
Copy link
Contributor Author

Work in progress:

image

I have send 1 ksm from Kusama to Basilisk on teleport page. https://kusama.subscan.io/xcm_message/kusama-350df3d577486e2054b580f0065c872b4b4d693b
Still waiting for the confirmation at Basilisk network.

image

  • teleport ksm token from Basilisk to Kusama
  • refactor code of teleport component
  • UI redesign teleport

@yangwao
Copy link
Member

yangwao commented Nov 10, 2022

Still waiting for the confirmation at Basilisk network.

Basilisk is bricked for now and won't work for like 5-6 days for sure :|

BUT we can test it against

  • Statemine (live net)
  • Rococo at SNEK (Basilisk testnet), probably @vikiival can send you some testnet tokens there

@Jarsen136
Copy link
Contributor Author

BUT we can test it against

  • Statemine (live net)
  • Rococo at SNEK (Basilisk testnet), probably @vikiival can send you some testnet tokens there

Oh, that would be better.
My Basilisk test address is bXk6PXHj9NnYkiZ45ETrq4iLiozW8a4khe8ZmqDkdbLJNNMaE

@yangwao yangwao added the A-basilisk issues related to basilisk parachain label Nov 11, 2022
@yangwao
Copy link
Member

yangwao commented Nov 13, 2022

Once merge, we can test this one

@yangwao
Copy link
Member

yangwao commented Nov 16, 2022

It should working now? 👀

lmk whenever we can test & merge this 😎

@Jarsen136
Copy link
Contributor Author

Jarsen136 commented Nov 17, 2022

Teleport bridge path: https://deploy-preview-4297--koda-nuxt.netlify.app/teleport-bridge


  • teleport ksm token from Kusama to Basilisk (✅ Tested)

Teleport Steps:

  1. switch to rmrk network
  2. input the bsx recipient address
  3. send transaction

image


  • teleport ksm token from Basilisk to Kusama

Teleport Steps:

  1. switch to bsx network
  2. input the ksm recipient address
  3. send transaction

I have tried many times to test the functionality that on Basilisk. However, it's hard for me to connect with the Basilisk websocket network.

image

Could anyone do a teleport test from Kusama to Basilisk?

@Jarsen136
Copy link
Contributor Author

  • teleport ksm token from Basilisk to Kusama

Teleport Steps:

  1. switch to bsx network
  2. input the ksm recipient address
  3. send transaction

I have tried many times to test the functionality that on Basilisk. However, it's hard for me to connect with the Basilisk websocket network.

I have found out why it does not work properly when I teleport ksm from Basilisk.

As we could see at @paraspell/sdk here : https://github.com/paraspell/sdk/blob/7fc8b1e87033ac4e0de024f81d52f6558474531e/src/pallets/xTokens/transfer.ts#L5

Function transferParaToRelay does not prepare the result for Basilisk.
image

So, I guess we should fulfill the function transferParaToRelay for Basilisk first. 🤔 WDYT @vikiival

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 14635 lines exceeds the maximum allowed for the inline comments feature.

@vikiival
Copy link
Member

However, the network on Basilisk node seems not that stable.

what does that mean?

@Jarsen136
Copy link
Contributor Author

However, the network on Basilisk node seems not that stable.

what does that mean?

I have to wait for a long time to connect to Basilisk rpc node when I want to teleport token. I'm not sure it's related to my own network or not. It really kills my time : )
image

@Jarsen136 Jarsen136 changed the title WIP: teleport KSM token between Kusama and Basilisk teleport KSM token between Kusama and Basilisk Nov 21, 2022
@dudo50
Copy link
Contributor

dudo50 commented Nov 21, 2022

@Jarsen136 Transfer Basilisk to Relay chain does not work because you use wrong version of sdk, currently version in main branch is suited for localhost use with UI only. However if you switch to beta-pre-release branch I think you will find what you were searching for :)..
If you have any questions feel free to contact me or tag me here.

@Jarsen136
Copy link
Contributor Author

@Jarsen136 Transfer Basilisk to Relay chain does not work because you use wrong version of sdk, currently version in main branch is suited for localhost use with UI only. However if you switch to beta-pre-release branch I think you will find what you were searching for :).. If you have any questions feel free to contact me or tag me here.

Hey, @dudo50 thanks for your answer. The beta-pre-release branch would help to transfer tokens from Basilisk. However, the latest version 0.0.11 for SDK is built from the main branch. So, I guess there is no specific released version of SDK that could support transferring tokens from Basilisk, for now. Correct me if i'm wrong.

For now, I write the transfer function call code without using any SDK. If the latest version of @paraspell/sdk is released in the future, I would switch to using the SDK to support transfer tokens on more network

@dudo50
Copy link
Contributor

dudo50 commented Nov 21, 2022

@Jarsen136 there is actually it is called 0.0.11.alpha I believe, you can check it on npm.. It supports Basilisk and Kusama transfers out of the box. However I respect your decision, I would like to encourage you to check it anyways so you can see how to construct calls you seek if there is something you still need to implement :)

@Jarsen136
Copy link
Contributor Author

@Jarsen136 there is actually it is called 0.0.11.alpha I believe, you can check it on npm.. It supports Basilisk and Kusama transfers out of the box. However I respect your decision, I would like to encourage you to check it anyways so you can see how to construct calls you seek if there is something you still need to implement :)

Thanks for correcting me : ) I will use this alpha version of SDK to help to build the teleport function. I think it would do better.

@dudo50
Copy link
Contributor

dudo50 commented Nov 21, 2022

No worries @Jarsen136, it is still under construction so some things might not be clear right away. We plan on converting it into a builder pattern soon to make things as intuitive as possible.. If you have any questions feel free to tag me :)..

@Jarsen136 Jarsen136 marked this pull request as ready for review November 21, 2022 15:54
@Jarsen136
Copy link
Contributor Author

Jarsen136 commented Nov 21, 2022

After many times of testing, I think the functionality is good for testing now. @yangwao

https://deploy-preview-4297--koda-nuxt.netlify.app/teleport-bridge

image

@yangwao
Copy link
Member

yangwao commented Nov 21, 2022

i was randomly checking this pr, when was draft and suddenly green!
can test and let you know

@yangwao
Copy link
Member

yangwao commented Nov 21, 2022

@Jarsen136, maybe adding teleport-bridge to dropdown?

I would add it under Transfer for now :)
image

once this is merged,
someone can take

@Jarsen136
Copy link
Contributor Author

Jarsen136 commented Nov 21, 2022

@Jarsen136, maybe adding teleport-bridge to dropdown?

I would add it under Transfer for now :)

Done.
image

@vikiival vikiival mentioned this pull request Nov 21, 2022
Copy link
Member

@vikiival vikiival left a comment

Choose a reason for hiding this comment

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

Please use already made components

</b-select>
</b-field>

<b-field
Copy link
Member

Choose a reason for hiding this comment

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

AccountInput cries in the corner

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

</b-select>
</b-field>

<b-field class="" label-position="inside" label="Input currency amount">
Copy link
Member

Choose a reason for hiding this comment

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

BalanceInput cries in the corner

Copy link
Member

Choose a reason for hiding this comment

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

BalanceInput

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

:step="0.0001"></b-input>
</b-field>

<b-button
Copy link
Member

Choose a reason for hiding this comment

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

SubmitButton cries in third corner

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines +68 to +77
import { ApiPromise, WsProvider } from '@polkadot/api'
import { web3Enable } from '@polkadot/extension-dapp'
import '@polkadot/api-augment'
import { toDefaultAddress } from '@/utils/account'
import { getAddress } from '@/utils/extension'
import { Chain, ChainIdMap } from '@/utils/teleport'
import { notificationTypes, showNotification } from '@/utils/notification'
import useAuth from '@/composables/useAuth'
import Loader from '@/components/shared/Loader.vue'
import * as paraspell from '@paraspell/sdk'
Copy link
Member

Choose a reason for hiding this comment

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

Plese use sub-api :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm afraid that it could not use sub-api. Because If I want to teleport tokens from Kusama, I need to use ApiPromise of kusama. On the other hand, I would use ApiPromise of basilisk.

And sub-api would only return the API endpoint according to the current prefix. 👀

Copy link
Member

Choose a reason for hiding this comment

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

Comment on lines 127 to 142
const injector = await getAddress(toDefaultAddress(address))

if (fromChain.value === Chain.KUSAMA) {
const wsProvider = new WsProvider('wss://public-rpc.pinknode.io/kusama')
const apiKusama = await ApiPromise.create({ provider: wsProvider })

const promise = paraspell.xTokens.transferRelayToPara(
apiKusama,
ChainIdMap[Chain.BASILISK],
amount.value * 1e12,
toAddress.value
)

promise
.signAndSend(address, { signer: injector.signer }, transactionHandler)
.catch(errorHandler)
Copy link
Member

Choose a reason for hiding this comment

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

transactionExecutor.ts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used txCb utils function

@codeclimate
Copy link

codeclimate bot commented Nov 22, 2022

Code Climate has analyzed commit 6b2311b and detected 0 issues on this pull request.

View more on Code Climate.

@yangwao
Copy link
Member

yangwao commented Nov 23, 2022

I don't have my keys with me rn, so I believe it has worked!

pay 150 USD

@yangwao
Copy link
Member

yangwao commented Nov 23, 2022

😍 Perfect, I’ve sent the payout
💵 $150 @ 25.92 USD/KSM ~ 5.787 $KSM
🧗 Caiv9TbPz68q5dC8EcHu5xKYPRnremimGzqmEejDFNpWWLG
🔗 0x09332075fa27f7301474f43751818f0e80f7518a5cea7bb46fdc8a85e3909a6a

🪅 Let’s grab another issue and get rewarded!
🪄 github.com/kodadot/nft-gallery/issues

@yangwao yangwao added the paid pull-request has been paid label Nov 23, 2022
@yangwao yangwao merged commit 2ac5923 into kodadot:main Nov 23, 2022
@yangwao
Copy link
Member

yangwao commented Nov 23, 2022

Let's now crack on

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-basilisk issues related to basilisk parachain paid pull-request has been paid
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement teleport KSM token from Kusama to Basilisk
6 participants