kodadot / nft-gallery

Generative Art Marketplace
https://kodadot.xyz
MIT License
611 stars 351 forks source link

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

Open prury opened 4 months ago

prury commented 4 months ago

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

iMac7 commented 4 months ago

:wave:

kodabot commented 4 months ago

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

yangwao commented 4 months ago

+50% of ED wouldn't help?

iMac7 commented 4 months ago

+50% of ED wouldn't help?

@vikiival 👀

vikiival commented 4 months ago

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

So MAX would be = totalBalance - 2 * ED

iMac7 commented 4 months ago

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 commented 3 months ago

@kodadot/internal issue open for taking

kodabot commented 3 months ago

ASSIGNMENT EXPIRED - @iMac7 has been unassigned.

dudo50 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

@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: https://github.com/paraspell/xcm-tools/issues/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

kodabot commented 3 months ago

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 commented 3 months ago

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

dudo50 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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

thanks for the information @dudo50 !

exezbcz commented 3 months ago

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

prury commented 3 months ago

@exezbcz tried with two testing accounts i have:

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

![image](https://github.com/kodadot/nft-gallery/assets/36627808/044421e6-92a2-4805-9adc-39753b6886c7)

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

![image](https://github.com/kodadot/nft-gallery/assets/36627808/64bb0086-7b44-49f3-a00a-4fa6df293d26)

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 commented 3 months ago

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

Jarsen136 commented 3 months ago

@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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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

daiagi commented 3 months ago

@exezbcz

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

dudo50 commented 3 months ago

@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 commented 3 months ago

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 commented 3 months ago

aaand another one image

exezbcz commented 3 months ago

@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 commented 3 months ago

@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