paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.network/
1.66k stars 594 forks source link

BUG: XCM transfer, that would slash account leads to total asset loss. #3050

Closed dudo50 closed 5 months ago

dudo50 commented 6 months ago

Is there an existing issue?

Experiencing problems? Have you tried our Stack Exchange first?

Description of bug

There is huge bug, where, when account tries to transfer XCM Message that will also ~slash~ dust (cause to go below existential deposit) their account, the account will suffer total asset loss and XCM message will not get sent. This only occurs on Kusama Asset Hub, Polkadot works fine, because it does not have latest runtime.

Steps to reproduce

Transfer amount that will result in less than existential deposit on Kusama Asset Hub (Kusama untested as I have verified identity there and do not want to loose it).

Or try any Kusama DEX and try transfer MAX option. (Kusama Asset Hub -> Kusama)

Screenshot 2024-01-24 at 16 34 47

Other XCM transfers work as expected. Please, fix this bug as it could lead to huge asset losses if released on Polkadot also.

KiChjang commented 6 months ago

Which functionality did you use to transfer assets? Was it a teleport, or did you create your own XCM program? If it's the latter, can you show us your XCM program?

dudo50 commented 6 months ago

Which functionality did you use to transfer assets? Was it a teleport, or did you create your own XCM program? If it's the latter, can you show us your XCM program?

Hey @KiChjang , we used native PolkadotXCM and limited teleport function.

Screenshot 2024-01-24 at 16 34 26

Here is how the message was formatted (other messages that are formatted the same but would not slash account (amount remaining in the account after XCM is more than existential deposit) work just fine)

ggwpez commented 6 months ago

Apparently its this block: https://assethub-kusama.subscan.io/block/6284878?tab=event
And this state diff: diff.html.txt

kianenigma commented 5 months ago

I tried to emulate this issue using Chopsticks and at first attempt couldn't. Are you claiming that any teleport that leaves the sender below existential deposit on the kusama relay chain will cause a trap? I moved 5 KSM To Charlie, teleported 4.999999 to Charlie on AH and the teleport failed with a reasonable outcome, without any traps or loss of funds.

dudo50 commented 5 months ago

@kianenigma If I understand you correctly, you tried Kusama -> AssetHub transfer. We encountered the issue on AssetHub->Kusama transfer.

dudo50 commented 5 months ago

I am still able to recreate it, so it was not temporary: https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Fkusama-asset-hub-rpc.polkadot.io#/explorer/query/0xd3191feabfc50b1c6415f2fd70cd704d77e3a61558e15721d20baf874732fcc6

https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Fkusama-asset-hub-rpc.polkadot.io#/extrinsics/decode/0x8d02840084fc49ce30071ea611731838cc7736113c1ec68fbc47119be8a0805066df9b2b01988575310511e1dc64f6a1d041d9ca0be73beaf8c90c04f5949d89cbdefaf2768990d1c4853db0f4492eac27410aba4f7b2a87d46b918c49267086dc3402278b04018800001f09030100030001010084fc49ce30071ea611731838cc7736113c1ec68fbc47119be8a0805066df9b2b030400010000075ab2966e0b0000000000

NachoPal commented 5 months ago

I tried a limitedTeleport from AssetHubKusama to Kusama with Chopsticks and it worked.

~Some edge case should be triggering the issue for your case.~

Sorry, I read again the issue, I see what's the edge case

NachoPal commented 5 months ago

Can you share what was your balance when you tried to do the teleport and how much KSM you tried to teleport?

Not sure if I understand the issue, I am not able to reproduce it. I tried to teleport all my balance and it failed because a fee has to be paid beforehand. However, the sending account does not lose all its balance, just the mentioned fee.

dudo50 commented 5 months ago

You can find my approximate balance here: Around 0.04ksm. https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Fkusama-asset-hub-rpc.polkadot.io#/extrinsics/decode/0x8d02840084fc49ce30071ea611731838cc7736113c1ec68fbc47119be8a0805066df9b2b01988575310511e1dc64f6a1d041d9ca0be73beaf8c90c04f5949d89cbdefaf2768990d1c4853db0f4492eac27410aba4f7b2a87d46b918c49267086dc3402278b04018800001f09030100030001010084fc49ce30071ea611731838cc7736113c1ec68fbc47119be8a0805066df9b2b030400010000075ab2966e0b0000000000

dudo50 commented 5 months ago

Can you share what was your balance when you tried to do the teleport and how much KSM you tried to teleport?

Not sure if I understand the issue, I am not able to reproduce it. I tried to teleport all my balance and it failed because a fee has to be paid beforehand. However, the sending account does not lose all its balance, just the mentioned fee.

What was the call you used and how much was your balance/you tried to send? We are always sending *amount-xcmfee1.5**. Not the entire amount because then it would fail as expected with inability to pay for fees.

dudo50 commented 5 months ago

If you go to any dex, for eg Karura or Basilisk and go to cross-chain section and hit transfer max button for transferring KSM from KusamaAssetHub->Kusama you should be able to replicate. Be sure to not have lots of coins though.

NachoPal commented 5 months ago

You can find my approximate balance here: Around 0.04ksm. https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Fkusama-asset-hub-rpc.polkadot.io#/extrinsics/decode/0x8d02840084fc49ce30071ea611731838cc7736113c1ec68fbc47119be8a0805066df9b2b01988575310511e1dc64f6a1d041d9ca0be73beaf8c90c04f5949d89cbdefaf2768990d1c4853db0f4492eac27410aba4f7b2a87d46b918c49267086dc3402278b04018800001f09030100030001010084fc49ce30071ea611731838cc7736113c1ec68fbc47119be8a0805066df9b2b030400010000075ab2966e0b0000000000

I meant what was the balance in your account, not the amount you tried to teleport

dudo50 commented 5 months ago

You can find my approximate balance here: Around 0.04ksm. https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Fkusama-asset-hub-rpc.polkadot.io#/extrinsics/decode/0x8d02840084fc49ce30071ea611731838cc7736113c1ec68fbc47119be8a0805066df9b2b01988575310511e1dc64f6a1d041d9ca0be73beaf8c90c04f5949d89cbdefaf2768990d1c4853db0f4492eac27410aba4f7b2a87d46b918c49267086dc3402278b04018800001f09030100030001010084fc49ce30071ea611731838cc7736113c1ec68fbc47119be8a0805066df9b2b030400010000075ab2966e0b0000000000

I meant what was the balance in your account, not the amount you tried to teleport

I am unable to give you certain number as as I mentioned it was approximately same as was sent. This is the condition to catch the bug: balance-(amount+xcmfee×1.5)>0 && < ED.

dudo50 commented 5 months ago

Here is a demonstration of the bug in the video. And you can try any dex not just Karura dex..

So now you know how to replicate it I believe, because I really have no other way to show you now. I've tried my best.

https://github.com/paritytech/polkadot-sdk/assets/55763425/1abd52ea-a8e5-48e4-a081-1e384ca1dd9c

NachoPal commented 5 months ago

Thanks for the video, much clear now. I had to use exactly the same amounts to be able to reproduce the case.

Your account does not go to 0 KSM. I checked and your balance after trying the teleport was: 96,375,486. It is an UI issue because of not having enough decimals.

I think the issue was introduced in https://github.com/paritytech/polkadot-sdk/pull/1234

As per the Important Note states in the PR, the UI should have taken into consideration the new delivery fee, disallowing you from doing the teleport.

Initially you have enough balance to pay for the transaction fee + the first WithdrawAsset (amount to teleport). However, after the first withdrawal, when trying to send the message, it tries to take the fee, subsequently failing when trying to withdraw again as the holding registry does not have enough funds anymore.

I think this issue is solved by https://github.com/paritytech/polkadot-sdk/pull/2388 and https://github.com/paritytech/polkadot-sdk/pull/1222

Let me confirm it and see if we can patch it.

dudo50 commented 5 months ago

Perfect! Thanks for the great explanation! Much clearer on my end now too!

rageruun commented 5 months ago

can anyone help? Tokens stuck in parachain bridge when transmitted from the Kusama network to the Moonriver (xcKSM) image hash https://kusama.subscan.io/extrinsic/0x37b705c316c4b4b8d552964a130a552e06bad1f55171fca693e4b4e1917dd336?tab=event

KiChjang commented 5 months ago

The tokens are not stuck in the bridge, but rather in the sovereign account of parachain 2023: https://kusama.subscan.io/account/F7fq1jSB3w59f8vMShxvP5eSu3wCJbL5Am5MQ6vP6VzYLWD.

Easiest way to recover this is to contact support on parachain 2023 and show them this exact extrinsic (or this issue), and tell them that you have a failed teleport, and now some assets are stuck in their sovereign account, and that you want to retrieve them. EDIT: This is a reserve asset transfer failure however, which is not the same as the problem encountered by the issue author.

For reserve asset transfers, please wait until the patches #3050 and #3070 is landed on the relay chain and AH runtimes for it to be fixed.

For teleports, please refrain from using the send MAX functionality of all XCM UIs, since it appears that none of them took account of the delivery fees, leading to this bug. Instead, send most of your KSM and leave behind maybe 1 KSM to pay for delivery fees -- this should avoid non-payment of fees.

acatangiu commented 5 months ago

runtimes patch release fixing this https://github.com/paritytech/polkadot-sdk/issues/3050 and https://github.com/paritytech/polkadot-sdk/issues/3070 is in the works

a fix for this issue has been backported to a patch release branch, next:

rageruun commented 5 months ago

Awesome, thanks for the update!

вт, 30 янв. 2024 г. в 14:48, Adrian Catangiu @.***>:

runtimes patch release fixing this #3050 https://github.com/paritytech/polkadot-sdk/issues/3050 and #3070 https://github.com/paritytech/polkadot-sdk/issues/3070 is in the works

a fix for this issue https://github.com/paritytech/polkadot-sdk/pull/3113 has been backported to a patch release branch, next:

— Reply to this email directly, view it on GitHub https://github.com/paritytech/polkadot-sdk/issues/3050#issuecomment-1916773531, or unsubscribe https://github.com/notifications/unsubscribe-auth/AWYFUHQRLU2J7FECRVTD3SLYRDTZBAVCNFSM6AAAAABCJAH45SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJWG43TGNJTGE . You are receiving this because you commented.Message ID: @.***>

KiChjang commented 5 months ago

@acatangiu Not quite, this issue uses teleport, whereas the fixes we have resolve reserve asset transfers.

KiChjang commented 5 months ago

Sorry, it looks like @rageruun here is talking about a reserve asset transfer, which is not exactly related to what the original post is about. For teleports, what I've written above still stands.

rageruun commented 5 months ago

https://kusama.subscan.io/extrinsic/21594113-4 image

acatangiu commented 5 months ago

@acatangiu Not quite, this issue uses teleport, whereas the fixes we have resolve reserve asset transfers.

I believe XCM transactional processing also fixes/mitigates this issue.

acatangiu commented 5 months ago

This should be fixed in runtimes patch release https://github.com/polkadot-fellows/runtimes/releases/tag/v1.1.2

Pescador1551 commented 5 months ago

Hi @KiChjang @kianenigma @NachoPal @rageruun @dudo50 @acatangiu, I encountered the same issue with a transfer of KSM during a Kusama to AssetHub transport https://kusama.subscan.io/extrinsic/21678807-20 and lost 76 KSM. @KiChjang, you said the best way to resolve it is to contact parachain support 2023. Do you know where I can find this? @rageruun : Were you able to recover your funds?. All help is welcome. Thank you!

dudo50 commented 5 months ago

Sorry to hear this @Pescador1551 , the fix is currently being issued via opengov. Good luck recovering your assets!

rageruun commented 5 months ago

unfortunately not yet, if anything changes for you, please let me know, pls

пт, 2 февр. 2024 г. в 14:49, Pescador1551 @.***>:

Hi @KiChjang https://github.com/KiChjang @kianenigma https://github.com/kianenigma @NachoPal https://github.com/NachoPal @rageruun https://github.com/rageruun @dudo50 https://github.com/dudo50 @acatangiu https://github.com/acatangiu, I encountered the same issue with a transfer of KSM during a Kusama to AssetHub transport [https://kusama.subscan.io/extrinsic/21678807-20] and lost 76 KSM. @KiChjang https://github.com/KiChjang, you said the best way to resolve it is to contact parachain support 2023. Do you know where I can find this? @rageruun https://github.com/rageruun : Were you able to recover your funds?. All help is welcome. Thank you!

— Reply to this email directly, view it on GitHub https://github.com/paritytech/polkadot-sdk/issues/3050#issuecomment-1923739803, or unsubscribe https://github.com/notifications/unsubscribe-auth/AWYFUHW6MTWBW26B6CRZLNLYRTOFNAVCNFSM6AAAAABCJAH45SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRTG4ZTSOBQGM . You are receiving this because you were mentioned.Message ID: @.***>

dudo50 commented 4 months ago

@acatangiu , the issue is still not resolved and after Polkadot runtime update users are now loosing assets on Polkadot. Could you reopen? It seems to be issue with when they send XCM transfer that also dusts their account.

Example of Polkadot transaction where user lost his DOT in transfer from AssetHubPolkadot->Polkadot https://assethub-polkadot.subscan.io/extrinsic/0xe844257b68019176f815104f845c6e18efa2d85d858c95eebc2077a28fac39c4

This deeply affects experience with Polkadot for users using dApps which damages dApps name.

Edit: There is not even need for account to dust in order for asset transfer to fail. If we sent amount that leaves us exactly with minimum deposit it still fails: https://assethub-polkadot.subscan.io/extrinsic/5766838-2

Edit2: For the context other lower amount transfers work just fine.

acatangiu commented 4 months ago

this was fixed back in November in https://github.com/paritytech/polkadot-sdk/pull/2405 and first released in polkadot sdk v1.5.0

problem 1. is it wasn't backported to sdk 1.3 or 1.4 problem 2. is fellowship/runtimes is running considerably behind polkadot-sdk so the fix just hasn't hit live runtimes yet

Opened https://github.com/polkadot-fellows/runtimes/issues/210 to track inclusion of this fix in the runtime releases

dudo50 commented 4 months ago

Thanks @acatangiu for quickly letting us know! The function for asset recovery will be awesome to have!

rageruun commented 4 months ago

tnks for msg

чт, 29 февр. 2024 г. в 17:24, Dusan Morhac @.***>:

@acatangiu https://github.com/acatangiu , the issue is still not resolved and after Polkadot runtime update users are now loosing assets on Polkadot. Could you reopen? It seems to be issue with when they send XCM transfer that also dusts their account.

— Reply to this email directly, view it on GitHub https://github.com/paritytech/polkadot-sdk/issues/3050#issuecomment-1971375971, or unsubscribe https://github.com/notifications/unsubscribe-auth/AWYFUHSIIYEYUNLFGMPYUYDYV5D2NAVCNFSM6AAAAABCJAH45SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZRGM3TKOJXGE . You are receiving this because you were mentioned.Message ID: @.***>