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 and autoteleport issues on Polkadot <-> Polkadot Assethub #9596

Open
prury opened this issue Feb 29, 2024 · 30 comments · Fixed by #9746
Open

Teleport and autoteleport issues on Polkadot <-> Polkadot Assethub #9596

prury opened this issue Feb 29, 2024 · 30 comments · Fixed by #9746
Labels
$ ~<50usd A-teleport related teleportation assets between para chains bug Something isn't working intern tasks intended for internal team p1 preventing everyone from using app

Comments

@prury
Copy link
Member

prury commented Feb 29, 2024

The same bug that happened on Kusama in the last month(XCM related) is also happening on Polkadot where users lose funds when trying to teleport an amount near ED.

let's disable teleport and autoteleport until there's a fix from parity

@prury prury added bug Something isn't working p1 preventing everyone from using app A-teleport related teleportation assets between para chains labels Feb 29, 2024
@vikiival vikiival added the $ ~<50usd label Feb 29, 2024
@iMac7
Copy link
Contributor

iMac7 commented Feb 29, 2024

👋

@kodabot
Copy link
Collaborator

kodabot commented Feb 29, 2024

ASSIGNED - @iMac7 🔒 LOCKED -> Friday, March 1st 2024, 17:38:52 UTC -> 24 hours

@iMac7 iMac7 mentioned this issue Feb 29, 2024
16 tasks
@yangwao
Copy link
Member

yangwao commented Feb 29, 2024

+50% of ED wouldn't help?

@iMac7
Copy link
Contributor

iMac7 commented Mar 1, 2024

+50% of ED wouldn't help?

@vikiival 👀

@vikiival
Copy link
Member

vikiival commented Mar 1, 2024

I think we would need to keep 2 * ED at least

So MAX would be = totalBalance - 2 * ED

@iMac7
Copy link
Contributor

iMac7 commented Mar 1, 2024

I think we would need to keep 2 * ED at least

So MAX would be = totalBalance - 2 * ED

How about for autoteleport, check if the amount to be teleported in > 2 * ED ?
Edit: raised the existential deposit check for teleport to 2 times the value, done this for autoteleport as well

@prury prury added the intern tasks intended for internal team label Mar 4, 2024
@prury prury unassigned iMac7 Mar 4, 2024
@prury
Copy link
Member Author

prury commented Mar 4, 2024

@kodadot/internal issue open for taking

@kodabot
Copy link
Collaborator

kodabot commented Mar 4, 2024

ASSIGNMENT EXPIRED - @iMac7 has been unassigned.

@dudo50
Copy link
Contributor

dudo50 commented Mar 11, 2024

We have checked delivery fees and found following information:

Polkadot delivery fee: 0.047DOT
PolkadotAssetHub delivery fee: 0.036DOT
Kusama delivery fee: 0.0015KSM
KusamaAssetHub delivery fee: 0.0015KSM

We are unable to confirm if these values are 100% correct so needs further testing.

@exezbcz
Copy link
Member

exezbcz commented Mar 13, 2024

so, another user reported he lost funds, @prury will confirm if its correct.

Probably let's raise the ed as teleporting from Polkadot to ahp is quite crucial for us

ad autoteleport
from what I remember, there was a feature that basically enabled the transfer of the whole amount to the chain the purchase is on to avoid funds loss. Does that cause the fund loss?

Are there any other solutions?

@prury
Copy link
Member Author

prury commented Mar 13, 2024

so, another user reported he lost funds, @prury will confirm if its correct.

yup, correct:
https://polkadot.subscan.io/extrinsic/19872686-2
https://polkadot.subscan.io/extrinsic/19881888-3

@dudo50
Copy link
Contributor

dudo50 commented Mar 13, 2024

@exezbcz

Probably let's raise the ed as teleporting from Polkadot to ahp is quite crucial for us

We would suggest to leave at least 1.5 dot (Not 1.5-fees but entire 1.5) on chain to ensure, that user won't loose funds.

Are there any other solutions?

AFAIK only one mentioned above until PolkadotJS fixes accounting for delivery fee and until Parity introduces ClaimAsset() function to recover fees. We will be implementing it in ParaSpell asap after it will be introduced: paraspell/xcm-tools#181

You will then be able to implement it to teleport for users that lost funds. With proper guide on recovering them there will be no more teleport issues further on we hope. Oh and also when that delivery fee issue is fixed.

With kind regards,
Team ParaSpell

@Jarsen136 Jarsen136 self-assigned this Mar 13, 2024
@kodabot
Copy link
Collaborator

kodabot commented Mar 13, 2024

ASSIGNED ISSUES LIMIT REACHED - @Jarsen136, you have been already assigned with 5 issues: 4042,5941,7217,8901,9046,9716,9727. Finish one of them in order to get more issues assigned!

@exezbcz
Copy link
Member

exezbcz commented Mar 18, 2024

It probably does not work guys

Andrey M:
I have lost 1 dot in my teleporter, how can I get it from there?

Andrey M:
The situation is strange, I have a pillbox on my network Polkadot Assets Hab, I wanted to change the price of NFT, but the @kodadot says that I don't have a DOT and offers a teleport from the DOT network, I clicked and the DOT disappeared

Andrey M:
dot from the dot network did not teleport to the asset hub network, it disappeared from the dot network

Andrey M:

telegram-cloud-photo-size-2-5458611517630044625-y

https://polkadot.subscan.io/extrinsic/0xc34fd7a6efcee082409d472ef4601bac3438eb382f3a6e97a8912bc006a66a80

can you have a look, please?

People should not be losing funds

@exezbcz exezbcz reopened this Mar 18, 2024
@dudo50
Copy link
Contributor

dudo50 commented Mar 18, 2024

We have checked call formatting and it seems to be correct. What we also found out is, that user sent entire funds they had from Polkadot -> AssetHubPolkadot. This was 1.03 DOT. Not sure how they managed to do this however. Wasn't the minimal deposit limit raised to 1.5 DOT?

@dudo50
Copy link
Contributor

dudo50 commented Mar 18, 2024

If you run string compare on their call - link
Compared to other random (But successful call) - link

You can see they are exactly the same except for sum and address which is expected. So there is probably bug in teleport where user somehow managed to get around 1.5 existential DOT deposit limit.

@exezbcz
Copy link
Member

exezbcz commented Mar 18, 2024

it happened on the autoteleport, maybe something is needed to do there as well?

thanks for the information @dudo50 !

@exezbcz
Copy link
Member

exezbcz commented Mar 20, 2024

can anyone have a look please? @kodadot/internal-dev

@prury
Copy link
Member Author

prury commented Mar 20, 2024

@exezbcz tried with two testing accounts i have:

Account 1: (same funds as user 1,03 DOT on Polkadot)

image

Account 2: (A bit more than user, around 1.1)

image

On both cases, i was unable to auto-teleport when trying to change price, list, buy and other interactions, only the add funds via onramp shows to me.

The only possibility that i think would be user going to teleport page and ignoring all the warnings and teleporting the full amount(still possible by ignoring the warnings)

@vikiival
Copy link
Member

Ah wasnt that caused by doubling the ED by @Jarsen136 ?

@Jarsen136
Copy link
Contributor

@exezbcz tried with two testing accounts i have:

Account 1: (same funds as user 1,03 DOT on Polkadot)

Account 2: (A bit more than user, around 1.1)

On both cases, i was unable to auto-teleport when trying to change price, list, buy and other interactions, only the add funds via onramp shows to me.

Yes, it's as expected because we have increased the ED.

image

The only possibility that i think would be user going to teleport page and ignoring all the warnings and teleporting the full amount(still possible by ignoring the warnings)

It's possible

@exezbcz
Copy link
Member

exezbcz commented Mar 21, 2024

The only possibility that i think would be user going to teleport page and ignoring all the warnings and teleporting the full amount(still possible by ignoring the warnings)

i think the user that lost funds used autoteleport - there is no warning - if there will be fundloss, it should not be enabled

so question, from what amount is the user okay if he wants to mint 1 nft from 0.5 DOT price drop? Transferring dot from polkadot to ahp

it crucial if there will be a bigger traffic - people not always have 2 dot just to mint 0.5 dot drop

@vikiival
Copy link
Member

so logical option would be - if you have like 1.2 DOT and wanna teleport 0.5 DOT,
I would simply teleport all. Hope that is possible

@exezbcz
Copy link
Member

exezbcz commented Mar 21, 2024

@vikiival that is how it was before afaik. If person have other assets on polkadot relay, that would delete that no?

@daiagi
Copy link
Contributor

daiagi commented Mar 21, 2024

@exezbcz

no. it would teleport all of it to AssetHub, avoiding unexpected loss of funds

@dudo50
Copy link
Contributor

dudo50 commented Mar 21, 2024

@vikiival that would not work due to delivery fee. If you send all your tokens, delivery fee is not accounted for and thus it will trap the assets. The only option for now is for user to have at least 1.5DOT on Polkadot and only transfer sum they have above 1.5 DOT -> eg. User has 2.8 DOT. That means they are able to safely transfer 1.3 DOT (Until the bug with delivery fee is fixed).

@dudo50
Copy link
Contributor

dudo50 commented Mar 21, 2024

Maybe upcoming runtime update to 1.1.3 will fix this issue but I doubt that because description of it is, to fix staking pallet from bricking.

@exezbcz
Copy link
Member

exezbcz commented Mar 28, 2024

aaand another one
image

@exezbcz exezbcz changed the title Disable teleport and autoteleport on Polkadot <-> Polkadot Assethub Teleport and autoteleport issues on Polkadot <-> Polkadot Assethub Mar 28, 2024
@exezbcz
Copy link
Member

exezbcz commented Mar 29, 2024

@dudo50 hello, what can we do to move it forward? I can name at least 8 users that lost funds

would creating issue help?

another one:

@dudo50
Copy link
Contributor

dudo50 commented Mar 29, 2024

@exezbcz I've checked SDK and call format, once again, SDK works as expected. The issue is, that the user somehow teleported 1.5 dot when they only had 2+ dot probably leaving them with less than deposit minimum leading to slash of account and asset trap and loss. The Kodadot devs should not allow users to teleport if they won't have at least 1.5 DOT on Polkadot after teleport happens. That way everything should work as expected. After Parity introduces claimAsset function to Runtime we will implement it to the SDK and give you guide on implementing it. For now this is only advice I can give you: Either do not allow them to go below 1.5 Dot on Polkadot or disable teleporting until PolkadotJS fixes delivery fee accounting.

With kind regards,
Team ParaSpell

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
$ ~<50usd A-teleport related teleportation assets between para chains bug Something isn't working intern tasks intended for internal team p1 preventing everyone from using app
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants